From 03de64559ee53f14b4595f81c5fb7de03145da39 Mon Sep 17 00:00:00 2001 From: Ross Lawley Date: Wed, 7 May 2025 09:10:00 +0100 Subject: [PATCH 1/3] Ensure custom KProperty include the name in the hashcode 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 is explicitly created using the same default `Any.toString()` implementation to ensure that all implementations of `KProperty1` use the same logic to create the cacheKey. JAVA-5868 --- .../mongodb/kotlin/client/model/Properties.kt | 3 +-- .../kotlin/client/property/KPropertyPath.kt | 17 +++++++++++++++++ .../kotlin/client/model/KPropertiesTest.kt | 14 ++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/driver-kotlin-extensions/src/main/kotlin/com/mongodb/kotlin/client/model/Properties.kt b/driver-kotlin-extensions/src/main/kotlin/com/mongodb/kotlin/client/model/Properties.kt index fc8a4e94e87..d82085be81b 100644 --- a/driver-kotlin-extensions/src/main/kotlin/com/mongodb/kotlin/client/model/Properties.kt +++ b/driver-kotlin-extensions/src/main/kotlin/com/mongodb/kotlin/client/model/Properties.kt @@ -71,8 +71,7 @@ public fun KProperty.path(): String { return if (this is KPropertyPath<*, T>) { this.name } else { - pathCache.computeIfAbsent(this.toString()) { - + pathCache.computeIfAbsent("${this.javaClass.getSimpleName()}@${Integer.toHexString(hashCode())}") { // Check serial name - Note kotlinx.serialization.SerialName may not be on the class // path val serialName = diff --git a/driver-kotlin-extensions/src/main/kotlin/com/mongodb/kotlin/client/property/KPropertyPath.kt b/driver-kotlin-extensions/src/main/kotlin/com/mongodb/kotlin/client/property/KPropertyPath.kt index a460266f098..1aaa3f622e9 100644 --- a/driver-kotlin-extensions/src/main/kotlin/com/mongodb/kotlin/client/property/KPropertyPath.kt +++ b/driver-kotlin-extensions/src/main/kotlin/com/mongodb/kotlin/client/property/KPropertyPath.kt @@ -20,6 +20,7 @@ package com.mongodb.kotlin.client.property import com.mongodb.annotations.Sealed import com.mongodb.kotlin.client.model.path +import java.util.Objects import kotlin.reflect.KParameter import kotlin.reflect.KProperty1 import kotlin.reflect.KType @@ -84,6 +85,15 @@ public open class KPropertyPath( override fun callBy(args: Map): R = unSupportedOperation() override fun get(receiver: T): R = unSupportedOperation() override fun getDelegate(receiver: T): Any? = unSupportedOperation() + override fun hashCode(): Int = Objects.hash(previous, property, name) + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (javaClass != other?.javaClass) return false + other as KPropertyPath<*, *> + return Objects.equals(previous, other.previous) && + Objects.equals(property, other.property) && + Objects.equals(name, other.name) + } public companion object { @@ -121,6 +131,13 @@ public open class KPropertyPath( override fun get(receiver: T): R = unSupportedOperation() override fun getDelegate(receiver: T): Any? = unSupportedOperation() override fun invoke(p1: T): R = unSupportedOperation() + override fun hashCode(): Int = Objects.hash(previous, name) + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (javaClass != other?.javaClass) return false + other as CustomProperty<*, *> + return Objects.equals(previous, other.previous) && Objects.equals(name, other.name) + } } /** Provides "fake" property with custom name. */ diff --git a/driver-kotlin-extensions/src/test/kotlin/com/mongodb/kotlin/client/model/KPropertiesTest.kt b/driver-kotlin-extensions/src/test/kotlin/com/mongodb/kotlin/client/model/KPropertiesTest.kt index 0007c6251ea..2fb6bd7c706 100644 --- a/driver-kotlin-extensions/src/test/kotlin/com/mongodb/kotlin/client/model/KPropertiesTest.kt +++ b/driver-kotlin-extensions/src/test/kotlin/com/mongodb/kotlin/client/model/KPropertiesTest.kt @@ -139,4 +139,18 @@ class KPropertiesTest { assertThrows { property.get(restaurant) } assertThrows { property.getDelegate(restaurant) } } + + @Test + fun testNoCacheCollisions() { + for (i in 1.rangeTo(25_000)) { + assertEquals("reviews.$i", Restaurant::reviews.pos(i).path()) + assertEquals("reviews.$[identifier$i]", Restaurant::reviews.filteredPosOp("identifier$i").path()) + assertEquals("localeMap.$i", Restaurant::localeMap.keyProjection(i).path()) + + val x = i / 2 + assertEquals("reviews.$[identifier$x].rating", (Restaurant::reviews.filteredPosOp("identifier$x") / Review::score).path()) + assertEquals("reviews.$x.rating", (Restaurant::reviews.pos(x) / Review::score).path()) + assertEquals("localeMap.$x.rating", (Restaurant::localeMap.keyProjection(x) / Review::score).path()) + } + } } From 0764544ecb0b5eef04629b97b80c0a729fc5fa4d Mon Sep 17 00:00:00 2001 From: Ross Lawley Date: Mon, 12 May 2025 09:54:40 +0100 Subject: [PATCH 2/3] PR - simplify the cache key --- .../main/kotlin/com/mongodb/kotlin/client/model/Properties.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/driver-kotlin-extensions/src/main/kotlin/com/mongodb/kotlin/client/model/Properties.kt b/driver-kotlin-extensions/src/main/kotlin/com/mongodb/kotlin/client/model/Properties.kt index d82085be81b..97ebae27d63 100644 --- a/driver-kotlin-extensions/src/main/kotlin/com/mongodb/kotlin/client/model/Properties.kt +++ b/driver-kotlin-extensions/src/main/kotlin/com/mongodb/kotlin/client/model/Properties.kt @@ -32,7 +32,7 @@ import kotlin.reflect.jvm.javaField import org.bson.codecs.pojo.annotations.BsonId import org.bson.codecs.pojo.annotations.BsonProperty -private val pathCache: MutableMap by lazySoft { ConcurrentHashMap() } +private val pathCache: MutableMap by lazySoft { ConcurrentHashMap() } /** Returns a composed property. For example Friend::address / Address::postalCode = "address.postalCode". */ public operator fun KProperty1.div(p2: KProperty1): KProperty1 = @@ -71,7 +71,7 @@ public fun KProperty.path(): String { return if (this is KPropertyPath<*, T>) { this.name } else { - pathCache.computeIfAbsent("${this.javaClass.getSimpleName()}@${Integer.toHexString(hashCode())}") { + pathCache.computeIfAbsent(hashCode()) { // Check serial name - Note kotlinx.serialization.SerialName may not be on the class // path val serialName = From b345e4bba99b47bfce7aada26160f8bfb74b21fa Mon Sep 17 00:00:00 2001 From: Ross Lawley Date: Mon, 12 May 2025 10:00:26 +0100 Subject: [PATCH 3/3] Spotless update --- .../com/mongodb/kotlin/client/model/KPropertiesTest.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/driver-kotlin-extensions/src/test/kotlin/com/mongodb/kotlin/client/model/KPropertiesTest.kt b/driver-kotlin-extensions/src/test/kotlin/com/mongodb/kotlin/client/model/KPropertiesTest.kt index 2fb6bd7c706..51e09675d72 100644 --- a/driver-kotlin-extensions/src/test/kotlin/com/mongodb/kotlin/client/model/KPropertiesTest.kt +++ b/driver-kotlin-extensions/src/test/kotlin/com/mongodb/kotlin/client/model/KPropertiesTest.kt @@ -144,11 +144,13 @@ class KPropertiesTest { fun testNoCacheCollisions() { for (i in 1.rangeTo(25_000)) { assertEquals("reviews.$i", Restaurant::reviews.pos(i).path()) - assertEquals("reviews.$[identifier$i]", Restaurant::reviews.filteredPosOp("identifier$i").path()) + assertEquals("reviews.$[identifier$i]", Restaurant::reviews.filteredPosOp("identifier$i").path()) assertEquals("localeMap.$i", Restaurant::localeMap.keyProjection(i).path()) val x = i / 2 - assertEquals("reviews.$[identifier$x].rating", (Restaurant::reviews.filteredPosOp("identifier$x") / Review::score).path()) + assertEquals( + "reviews.$[identifier$x].rating", + (Restaurant::reviews.filteredPosOp("identifier$x") / Review::score).path()) assertEquals("reviews.$x.rating", (Restaurant::reviews.pos(x) / Review::score).path()) assertEquals("localeMap.$x.rating", (Restaurant::localeMap.keyProjection(x) / Review::score).path()) }