Skip to content

Commit 172327b

Browse files
authored
Ensure custom KProperty include the name in the hashcode (#1710)
The `pathCache` utilised the string representation of the KProperty instance. The custom implementations didn't include the calculated `path` value, this can lead to naming collisions in the `pathCache` key. This commit adds `hashCode` and `equals` methods to ensure the `name` value is included in our custom implementations of `KProperty1`. Finally, the cache key now uses the `hashCode` for the cacheKey. JAVA-5868
1 parent 9f8174d commit 172327b

File tree

3 files changed

+35
-3
lines changed

3 files changed

+35
-3
lines changed

driver-kotlin-extensions/src/main/kotlin/com/mongodb/kotlin/client/model/Properties.kt

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import kotlin.reflect.jvm.javaField
3232
import org.bson.codecs.pojo.annotations.BsonId
3333
import org.bson.codecs.pojo.annotations.BsonProperty
3434

35-
private val pathCache: MutableMap<String, String> by lazySoft { ConcurrentHashMap<String, String>() }
35+
private val pathCache: MutableMap<Int, String> by lazySoft { ConcurrentHashMap<Int, String>() }
3636

3737
/** Returns a composed property. For example Friend::address / Address::postalCode = "address.postalCode". */
3838
public operator fun <T0, T1, T2> KProperty1<T0, T1?>.div(p2: KProperty1<T1, T2?>): KProperty1<T0, T2?> =
@@ -71,8 +71,7 @@ public fun <T> KProperty<T>.path(): String {
7171
return if (this is KPropertyPath<*, T>) {
7272
this.name
7373
} else {
74-
pathCache.computeIfAbsent(this.toString()) {
75-
74+
pathCache.computeIfAbsent(hashCode()) {
7675
// Check serial name - Note kotlinx.serialization.SerialName may not be on the class
7776
// path
7877
val serialName =

driver-kotlin-extensions/src/main/kotlin/com/mongodb/kotlin/client/property/KPropertyPath.kt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ package com.mongodb.kotlin.client.property
2020

2121
import com.mongodb.annotations.Sealed
2222
import com.mongodb.kotlin.client.model.path
23+
import java.util.Objects
2324
import kotlin.reflect.KParameter
2425
import kotlin.reflect.KProperty1
2526
import kotlin.reflect.KType
@@ -84,6 +85,15 @@ public open class KPropertyPath<T, R>(
8485
override fun callBy(args: Map<KParameter, Any?>): R = unSupportedOperation()
8586
override fun get(receiver: T): R = unSupportedOperation()
8687
override fun getDelegate(receiver: T): Any? = unSupportedOperation()
88+
override fun hashCode(): Int = Objects.hash(previous, property, name)
89+
override fun equals(other: Any?): Boolean {
90+
if (this === other) return true
91+
if (javaClass != other?.javaClass) return false
92+
other as KPropertyPath<*, *>
93+
return Objects.equals(previous, other.previous) &&
94+
Objects.equals(property, other.property) &&
95+
Objects.equals(name, other.name)
96+
}
8797

8898
public companion object {
8999

@@ -121,6 +131,13 @@ public open class KPropertyPath<T, R>(
121131
override fun get(receiver: T): R = unSupportedOperation()
122132
override fun getDelegate(receiver: T): Any? = unSupportedOperation()
123133
override fun invoke(p1: T): R = unSupportedOperation()
134+
override fun hashCode(): Int = Objects.hash(previous, name)
135+
override fun equals(other: Any?): Boolean {
136+
if (this === other) return true
137+
if (javaClass != other?.javaClass) return false
138+
other as CustomProperty<*, *>
139+
return Objects.equals(previous, other.previous) && Objects.equals(name, other.name)
140+
}
124141
}
125142

126143
/** Provides "fake" property with custom name. */

driver-kotlin-extensions/src/test/kotlin/com/mongodb/kotlin/client/model/KPropertiesTest.kt

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,4 +139,20 @@ class KPropertiesTest {
139139
assertThrows<UnsupportedOperationException> { property.get(restaurant) }
140140
assertThrows<UnsupportedOperationException> { property.getDelegate(restaurant) }
141141
}
142+
143+
@Test
144+
fun testNoCacheCollisions() {
145+
for (i in 1.rangeTo(25_000)) {
146+
assertEquals("reviews.$i", Restaurant::reviews.pos(i).path())
147+
assertEquals("reviews.$[identifier$i]", Restaurant::reviews.filteredPosOp("identifier$i").path())
148+
assertEquals("localeMap.$i", Restaurant::localeMap.keyProjection(i).path())
149+
150+
val x = i / 2
151+
assertEquals(
152+
"reviews.$[identifier$x].rating",
153+
(Restaurant::reviews.filteredPosOp("identifier$x") / Review::score).path())
154+
assertEquals("reviews.$x.rating", (Restaurant::reviews.pos(x) / Review::score).path())
155+
assertEquals("localeMap.$x.rating", (Restaurant::localeMap.keyProjection(x) / Review::score).path())
156+
}
157+
}
142158
}

0 commit comments

Comments
 (0)