fix: correctly check equality for CaseInsensitiveMap

.changes/4820850c-8916-47f5-a7e1-8880e6a00d22.json

@@ -0,0 +1,5 @@
+{
+    "id": "4820850c-8916-47f5-a7e1-8880e6a00d22",
+    "type": "bugfix",
+    "description": "Fix errors in equality checks for `CaseInsensitiveMap` which affect `Headers` and `ValuesMap` implementations"
+}

runtime/runtime-core/api/runtime-core.api

@@ -306,6 +306,7 @@ public class aws/smithy/kotlin/runtime/collections/ValuesMapImpl : aws/smithy/ko
 	public fun getAll (Ljava/lang/String;)Ljava/util/List;
 	public fun getCaseInsensitiveName ()Z
 	protected final fun getValues ()Ljava/util/Map;
+	public fun hashCode ()I
 	public fun isEmpty ()Z
 	public fun names ()Ljava/util/Set;
 }

runtime/runtime-core/common/src/aws/smithy/kotlin/runtime/collections/CaseInsensitiveMap.kt

@@ -6,16 +6,6 @@ package aws.smithy.kotlin.runtime.collections
 
 import aws.smithy.kotlin.runtime.InternalApi
 
-private class CaseInsensitiveString(val s: String) {
-    val hash: Int = s.lowercase().hashCode()
-    override fun hashCode(): Int = hash
-    override fun equals(other: Any?): Boolean = other is CaseInsensitiveString && other.s.equals(s, ignoreCase = true)
-    override fun toString(): String = s
-}
-
-private fun String.toInsensitive(): CaseInsensitiveString =
-    CaseInsensitiveString(this)
-
 /**
  * Map of case-insensitive [String] to [Value]
  */
@@ -30,17 +20,17 @@ internal class CaseInsensitiveMap<Value> : MutableMap<String, Value> {
 
     override fun containsValue(value: Value): Boolean = impl.containsValue(value)
 
-    override fun get(key: String): Value? = impl.get(key.toInsensitive())
+    override fun get(key: String): Value? = impl[key.toInsensitive()]
 
     override fun isEmpty(): Boolean = impl.isEmpty()
 
     override val entries: MutableSet<MutableMap.MutableEntry<String, Value>>
         get() = impl.entries.map {
-            Entry(it.key.s, it.value)
+            Entry(it.key.normalized, it.value)
         }.toMutableSet()
 
     override val keys: MutableSet<String>
-        get() = impl.keys.map { it.s }.toMutableSet()
+        get() = CaseInsensitiveMutableStringSet(impl.keys)
 
     override val values: MutableCollection<Value>
         get() = impl.values
@@ -57,6 +47,12 @@ internal class CaseInsensitiveMap<Value> : MutableMap<String, Value> {
 
     override fun remove(key: String): Value? = impl.remove(key.toInsensitive())
 
+    override fun hashCode() = impl.hashCode()
+
+    override fun equals(other: Any?) = other is CaseInsensitiveMap<*> && impl == other.impl
+
+    override fun toString() = impl.toString()
+
     private class Entry<Key, Value>(
         override val key: Key,
         override var value: Value,

runtime/runtime-core/common/src/aws/smithy/kotlin/runtime/collections/CaseInsensitiveMutableStringSet.kt

@@ -0,0 +1,40 @@
+package aws.smithy.kotlin.runtime.collections
+
+internal class CaseInsensitiveMutableStringSet(
+    initialValues: Iterable<CaseInsensitiveString> = setOf(),
+) : MutableSet<String> {
+    private val delegate = initialValues.toMutableSet()
+
+    override fun add(element: String) = delegate.add(element.toInsensitive())
+    override fun clear() = delegate.clear()
+    override fun contains(element: String) = delegate.contains(element.toInsensitive())
+    override fun containsAll(elements: Collection<String>) = elements.all { it in this }
+    override fun equals(other: Any?) = other is CaseInsensitiveMutableStringSet && delegate == other.delegate
+    override fun hashCode() = delegate.hashCode()
+    override fun isEmpty() = delegate.isEmpty()
+    override fun remove(element: String) = delegate.remove(element.toInsensitive())
+    override val size: Int get() = delegate.size
+    override fun toString() = delegate.toString()
+
+    override fun addAll(elements: Collection<String>) =
+        elements.fold(false) { modified, item -> add(item) || modified }
+
+    override fun iterator() = object : MutableIterator<String> {
+        val delegate = [email protected]()
+        override fun hasNext() = delegate.hasNext()
+        override fun next() = delegate.next().normalized
+        override fun remove() = delegate.remove()
+    }
+
+    override fun removeAll(elements: Collection<String>) =
+        elements.fold(false) { modified, item -> remove(item) || modified }
+
+    override fun retainAll(elements: Collection<String>): Boolean {
+        val insensitiveElements = elements.map { it.toInsensitive() }.toSet()
+        val toRemove = delegate.filterNot { it in insensitiveElements }
+        return toRemove.fold(false) { modified, item -> delegate.remove(item) || modified }
+    }
+}
+
+internal fun CaseInsensitiveMutableStringSet(initialValues: Iterable<String>) =
+    CaseInsensitiveMutableStringSet(initialValues.map { it.toInsensitive() })

runtime/runtime-core/common/src/aws/smithy/kotlin/runtime/collections/CaseInsensitiveString.kt

@@ -0,0 +1,11 @@
+package aws.smithy.kotlin.runtime.collections
+
+internal class CaseInsensitiveString(val original: String) {
+    val normalized = original.lowercase()
+    override fun hashCode() = normalized.hashCode()
+    override fun equals(other: Any?) = other is CaseInsensitiveString && normalized == other.normalized
+    override fun toString() = original
+}
+
+internal fun String.toInsensitive(): CaseInsensitiveString =
+    CaseInsensitiveString(this)

runtime/runtime-core/common/src/aws/smithy/kotlin/runtime/collections/ValuesMap.kt

@@ -89,15 +89,13 @@ public open class ValuesMapImpl<T>(
 
     override fun isEmpty(): Boolean = values.isEmpty()
 
-    override fun equals(other: Any?): Boolean =
-        other is ValuesMap<*> &&
-            caseInsensitiveName == other.caseInsensitiveName &&
-            names().let { names ->
-                if (names.size != other.names().size) {
-                    return false
-                }
-                names.all { getAll(it) == other.getAll(it) }
-            }
+    override fun equals(other: Any?): Boolean = when (other) {
+        is ValuesMapImpl<*> -> caseInsensitiveName == other.caseInsensitiveName && values == other.values
+        is ValuesMap<*> -> caseInsensitiveName == other.caseInsensitiveName && entries() == other.entries()
+        else -> false
+    }
+
+    override fun hashCode(): Int = values.hashCode()
 
     private fun Map<String, List<T>>.deepCopyValues(): Map<String, List<T>> = mapValues { (_, v) -> v.toList() }
 }

runtime/runtime-core/common/test/aws/smithy/kotlin/runtime/collections/CaseInsensitiveMapTest.kt

@@ -6,6 +6,8 @@ package aws.smithy.kotlin.runtime.collections
 
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertTrue
 
 class CaseInsensitiveMapTest {
     @Test
@@ -18,4 +20,68 @@ class CaseInsensitiveMapTest {
         assertEquals("json", map["content-type"])
         assertEquals("json", map["CONTENT-TYPE"])
     }
+
+    @Test
+    fun testContains() {
+        val map = CaseInsensitiveMap<String>()
+        map["A"] = "apple"
+        map["B"] = "banana"
+        map["C"] = "cherry"
+
+        assertTrue("C" in map)
+        assertTrue("c" in map)
+        assertFalse("D" in map)
+    }
+
+    @Test
+    fun testKeysContains() {
+        val map = CaseInsensitiveMap<String>()
+        map["A"] = "apple"
+        map["B"] = "banana"
+        map["C"] = "cherry"
+        val keys = map.keys
+
+        assertTrue("C" in keys)
+        assertTrue("c" in keys)
+        assertFalse("D" in keys)
+    }
+
+    @Test
+    fun testEquality() {
+        val left = CaseInsensitiveMap<String>()
+        left["A"] = "apple"
+        left["B"] = "banana"
+        left["C"] = "cherry"
+
+        val right = CaseInsensitiveMap<String>()
+        right["c"] = "cherry"
+        right["b"] = "banana"
+        right["a"] = "apple"
+
+        assertEquals(left, right)
+    }
+
+    @Test
+    fun testEntriesEquality() {
+        val left = CaseInsensitiveMap<String>()
+        left["A"] = "apple"
+        left["B"] = "banana"
+        left["C"] = "cherry"
+
+        val right = CaseInsensitiveMap<String>()
+        right["c"] = "cherry"
+        right["b"] = "banana"
+        right["a"] = "apple"
+
+        assertEquals(left.entries, right.entries)
+    }
+
+    @Test
+    fun testToString() {
+        val map = CaseInsensitiveMap<String>()
+        map["A"] = "apple"
+        map["B"] = "banana"
+        map["C"] = "cherry"
+        assertEquals("{A=apple, B=banana, C=cherry}", map.toString())
+    }
 }

runtime/runtime-core/common/test/aws/smithy/kotlin/runtime/collections/CaseInsensitiveMutableStringSetTest.kt

@@ -0,0 +1,125 @@
+package aws.smithy.kotlin.runtime.collections
+
+import kotlin.test.*
+
+private val input = setOf("APPLE", "banana", "cHeRrY")
+private val variations = (input + input.map { it.lowercase() } + input.map { it.uppercase() })
+private val disjoint = setOf("durIAN", "ELdeRBerRY", "FiG")
+private val subset = input - "APPLE"
+private val intersecting = subset + disjoint
+
+class CaseInsensitiveMutableStringSetTest {
+    private fun assertSize(size: Int, set: CaseInsensitiveMutableStringSet) {
+        assertEquals(size, set.size)
+        val emptyAsserter: (Boolean) -> Unit = if (size == 0) ::assertTrue else ::assertFalse
+        emptyAsserter(set.isEmpty())
+    }
+
+    @Test
+    fun testInitialization() {
+        val set = CaseInsensitiveMutableStringSet(input)
+        assertSize(3, set)
+    }
+
+    @Test
+    fun testAdd() {
+        val set = CaseInsensitiveMutableStringSet(input)
+        set += "durIAN"
+        assertSize(4, set)
+    }
+
+    @Test
+    fun testAddAll() {
+        val set = CaseInsensitiveMutableStringSet(input)
+        assertFalse(set.addAll(set))
+
+        val intersecting = input + "durian"
+        assertTrue(set.addAll(intersecting))
+        assertSize(4, set)
+    }
+
+    @Test
+    fun testClear() {
+        val set = CaseInsensitiveMutableStringSet(input)
+        set.clear()
+        assertSize(0, set)
+    }
+
+    @Test
+    fun testContains() {
+        val set = CaseInsensitiveMutableStringSet(input)
+        variations.forEach { assertTrue("Set should contain element $it") { it in set } }
+
+        assertFalse("durian" in set)
+    }
+
+    @Test
+    fun testContainsAll() {
+        val set = CaseInsensitiveMutableStringSet(input)
+        assertTrue(set.containsAll(variations))
+
+        val intersecting = input + "durian"
+        assertFalse(set.containsAll(intersecting))
+    }
+
+    @Test
+    fun testEquality() {
+        val left = CaseInsensitiveMutableStringSet(input)
+        val right = CaseInsensitiveMutableStringSet(input)
+        assertEquals(left, right)
+
+        left -= "apple"
+        assertNotEquals(left, right)
+
+        right -= "ApPlE"
+        assertEquals(left, right)
+    }
+
+    @Test
+    fun testIterator() {
+        val set = CaseInsensitiveMutableStringSet(input)
+        val iterator = set.iterator()
+
+        assertTrue(iterator.hasNext())
+        assertEquals("apple", iterator.next())
+
+        assertTrue(iterator.hasNext())
+        assertEquals("banana", iterator.next())
+        iterator.remove()
+        assertSize(2, set)
+
+        assertTrue(iterator.hasNext())
+        assertEquals("cherry", iterator.next())
+
+        assertFalse(iterator.hasNext())
+        assertTrue(set.containsAll(input - "banana"))
+    }
+
+    @Test
+    fun testRemove() {
+        val set = CaseInsensitiveMutableStringSet(input)
+        set -= "BANANA"
+        assertSize(2, set)
+    }
+
+    @Test
+    fun testRemoveAll() {
+        val set = CaseInsensitiveMutableStringSet(input)
+        assertFalse(set.removeAll(disjoint))
+
+        assertTrue(set.removeAll(intersecting))
+        assertSize(1, set)
+        assertTrue("apple" in set)
+    }
+
+    @Test
+    fun testRetainAll() {
+        val set = CaseInsensitiveMutableStringSet(input)
+        assertFalse(set.retainAll(set))
+        assertSize(3, set)
+
+        assertTrue(set.retainAll(intersecting))
+        assertSize(2, set)
+        assertTrue(set.containsAll(subset))
+    }
+}

runtime/runtime-core/common/test/aws/smithy/kotlin/runtime/collections/CaseInsensitiveStringTest.kt

@@ -0,0 +1,27 @@
+package aws.smithy.kotlin.runtime.collections
+
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNotEquals
+
+class CaseInsensitiveStringTest {
+    @Test
+    fun testEquality() {
+        val left = "Banana".toInsensitive()
+        val right = "baNAna".toInsensitive()
+        assertEquals(left, right)
+        assertNotEquals<Any>("Banana", left)
+        assertNotEquals<Any>("baNAna", right)
+
+        val nonMatching = "apple".toInsensitive()
+        assertNotEquals(nonMatching, left)
+        assertNotEquals(nonMatching, right)
+    }
+
+    @Test
+    fun testProperties() {
+        val s = "BANANA".toInsensitive()
+        assertEquals("BANANA", s.original)
+        assertEquals("banana", s.normalized)
+    }
+}