Skip to content

Commit 5323882

Browse files
ianbotsfianbotsf
authored
fix: correctly check equality for CaseInsensitiveMap (#1235)
1 parent 9d6857b commit 5323882

File tree

9 files changed

+291
-22
lines changed

9 files changed

+291
-22
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"id": "4820850c-8916-47f5-a7e1-8880e6a00d22",
3+
"type": "bugfix",
4+
"description": "Fix errors in equality checks for `CaseInsensitiveMap` which affect `Headers` and `ValuesMap` implementations"
5+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,7 @@ public class aws/smithy/kotlin/runtime/collections/ValuesMapImpl : aws/smithy/ko
306306
public fun getAll (Ljava/lang/String;)Ljava/util/List;
307307
public fun getCaseInsensitiveName ()Z
308308
protected final fun getValues ()Ljava/util/Map;
309+
public fun hashCode ()I
309310
public fun isEmpty ()Z
310311
public fun names ()Ljava/util/Set;
311312
}

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

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,6 @@ package aws.smithy.kotlin.runtime.collections
66

77
import aws.smithy.kotlin.runtime.InternalApi
88

9-
private class CaseInsensitiveString(val s: String) {
10-
val hash: Int = s.lowercase().hashCode()
11-
override fun hashCode(): Int = hash
12-
override fun equals(other: Any?): Boolean = other is CaseInsensitiveString && other.s.equals(s, ignoreCase = true)
13-
override fun toString(): String = s
14-
}
15-
16-
private fun String.toInsensitive(): CaseInsensitiveString =
17-
CaseInsensitiveString(this)
18-
199
/**
2010
* Map of case-insensitive [String] to [Value]
2111
*/
@@ -30,17 +20,17 @@ internal class CaseInsensitiveMap<Value> : MutableMap<String, Value> {
3020

3121
override fun containsValue(value: Value): Boolean = impl.containsValue(value)
3222

33-
override fun get(key: String): Value? = impl.get(key.toInsensitive())
23+
override fun get(key: String): Value? = impl[key.toInsensitive()]
3424

3525
override fun isEmpty(): Boolean = impl.isEmpty()
3626

3727
override val entries: MutableSet<MutableMap.MutableEntry<String, Value>>
3828
get() = impl.entries.map {
39-
Entry(it.key.s, it.value)
29+
Entry(it.key.normalized, it.value)
4030
}.toMutableSet()
4131

4232
override val keys: MutableSet<String>
43-
get() = impl.keys.map { it.s }.toMutableSet()
33+
get() = CaseInsensitiveMutableStringSet(impl.keys)
4434

4535
override val values: MutableCollection<Value>
4636
get() = impl.values
@@ -57,6 +47,12 @@ internal class CaseInsensitiveMap<Value> : MutableMap<String, Value> {
5747

5848
override fun remove(key: String): Value? = impl.remove(key.toInsensitive())
5949

50+
override fun hashCode() = impl.hashCode()
51+
52+
override fun equals(other: Any?) = other is CaseInsensitiveMap<*> && impl == other.impl
53+
54+
override fun toString() = impl.toString()
55+
6056
private class Entry<Key, Value>(
6157
override val key: Key,
6258
override var value: Value,

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package aws.smithy.kotlin.runtime.collections
2+
3+
internal class CaseInsensitiveMutableStringSet(
4+
initialValues: Iterable<CaseInsensitiveString> = setOf(),
5+
) : MutableSet<String> {
6+
private val delegate = initialValues.toMutableSet()
7+
8+
override fun add(element: String) = delegate.add(element.toInsensitive())
9+
override fun clear() = delegate.clear()
10+
override fun contains(element: String) = delegate.contains(element.toInsensitive())
11+
override fun containsAll(elements: Collection<String>) = elements.all { it in this }
12+
override fun equals(other: Any?) = other is CaseInsensitiveMutableStringSet && delegate == other.delegate
13+
override fun hashCode() = delegate.hashCode()
14+
override fun isEmpty() = delegate.isEmpty()
15+
override fun remove(element: String) = delegate.remove(element.toInsensitive())
16+
override val size: Int get() = delegate.size
17+
override fun toString() = delegate.toString()
18+
19+
override fun addAll(elements: Collection<String>) =
20+
elements.fold(false) { modified, item -> add(item) || modified }
21+
22+
override fun iterator() = object : MutableIterator<String> {
23+
val delegate = this@CaseInsensitiveMutableStringSet.delegate.iterator()
24+
override fun hasNext() = delegate.hasNext()
25+
override fun next() = delegate.next().normalized
26+
override fun remove() = delegate.remove()
27+
}
28+
29+
override fun removeAll(elements: Collection<String>) =
30+
elements.fold(false) { modified, item -> remove(item) || modified }
31+
32+
override fun retainAll(elements: Collection<String>): Boolean {
33+
val insensitiveElements = elements.map { it.toInsensitive() }.toSet()
34+
val toRemove = delegate.filterNot { it in insensitiveElements }
35+
return toRemove.fold(false) { modified, item -> delegate.remove(item) || modified }
36+
}
37+
}
38+
39+
internal fun CaseInsensitiveMutableStringSet(initialValues: Iterable<String>) =
40+
CaseInsensitiveMutableStringSet(initialValues.map { it.toInsensitive() })

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package aws.smithy.kotlin.runtime.collections
2+
3+
internal class CaseInsensitiveString(val original: String) {
4+
val normalized = original.lowercase()
5+
override fun hashCode() = normalized.hashCode()
6+
override fun equals(other: Any?) = other is CaseInsensitiveString && normalized == other.normalized
7+
override fun toString() = original
8+
}
9+
10+
internal fun String.toInsensitive(): CaseInsensitiveString =
11+
CaseInsensitiveString(this)

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -89,15 +89,13 @@ public open class ValuesMapImpl<T>(
8989

9090
override fun isEmpty(): Boolean = values.isEmpty()
9191

92-
override fun equals(other: Any?): Boolean =
93-
other is ValuesMap<*> &&
94-
caseInsensitiveName == other.caseInsensitiveName &&
95-
names().let { names ->
96-
if (names.size != other.names().size) {
97-
return false
98-
}
99-
names.all { getAll(it) == other.getAll(it) }
100-
}
92+
override fun equals(other: Any?): Boolean = when (other) {
93+
is ValuesMapImpl<*> -> caseInsensitiveName == other.caseInsensitiveName && values == other.values
94+
is ValuesMap<*> -> caseInsensitiveName == other.caseInsensitiveName && entries() == other.entries()
95+
else -> false
96+
}
97+
98+
override fun hashCode(): Int = values.hashCode()
10199

102100
private fun Map<String, List<T>>.deepCopyValues(): Map<String, List<T>> = mapValues { (_, v) -> v.toList() }
103101
}

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

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ package aws.smithy.kotlin.runtime.collections
66

77
import kotlin.test.Test
88
import kotlin.test.assertEquals
9+
import kotlin.test.assertFalse
10+
import kotlin.test.assertTrue
911

1012
class CaseInsensitiveMapTest {
1113
@Test
@@ -18,4 +20,68 @@ class CaseInsensitiveMapTest {
1820
assertEquals("json", map["content-type"])
1921
assertEquals("json", map["CONTENT-TYPE"])
2022
}
23+
24+
@Test
25+
fun testContains() {
26+
val map = CaseInsensitiveMap<String>()
27+
map["A"] = "apple"
28+
map["B"] = "banana"
29+
map["C"] = "cherry"
30+
31+
assertTrue("C" in map)
32+
assertTrue("c" in map)
33+
assertFalse("D" in map)
34+
}
35+
36+
@Test
37+
fun testKeysContains() {
38+
val map = CaseInsensitiveMap<String>()
39+
map["A"] = "apple"
40+
map["B"] = "banana"
41+
map["C"] = "cherry"
42+
val keys = map.keys
43+
44+
assertTrue("C" in keys)
45+
assertTrue("c" in keys)
46+
assertFalse("D" in keys)
47+
}
48+
49+
@Test
50+
fun testEquality() {
51+
val left = CaseInsensitiveMap<String>()
52+
left["A"] = "apple"
53+
left["B"] = "banana"
54+
left["C"] = "cherry"
55+
56+
val right = CaseInsensitiveMap<String>()
57+
right["c"] = "cherry"
58+
right["b"] = "banana"
59+
right["a"] = "apple"
60+
61+
assertEquals(left, right)
62+
}
63+
64+
@Test
65+
fun testEntriesEquality() {
66+
val left = CaseInsensitiveMap<String>()
67+
left["A"] = "apple"
68+
left["B"] = "banana"
69+
left["C"] = "cherry"
70+
71+
val right = CaseInsensitiveMap<String>()
72+
right["c"] = "cherry"
73+
right["b"] = "banana"
74+
right["a"] = "apple"
75+
76+
assertEquals(left.entries, right.entries)
77+
}
78+
79+
@Test
80+
fun testToString() {
81+
val map = CaseInsensitiveMap<String>()
82+
map["A"] = "apple"
83+
map["B"] = "banana"
84+
map["C"] = "cherry"
85+
assertEquals("{A=apple, B=banana, C=cherry}", map.toString())
86+
}
2187
}

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

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
package aws.smithy.kotlin.runtime.collections
2+
3+
import kotlin.test.*
4+
5+
private val input = setOf("APPLE", "banana", "cHeRrY")
6+
private val variations = (input + input.map { it.lowercase() } + input.map { it.uppercase() })
7+
private val disjoint = setOf("durIAN", "ELdeRBerRY", "FiG")
8+
private val subset = input - "APPLE"
9+
private val intersecting = subset + disjoint
10+
11+
class CaseInsensitiveMutableStringSetTest {
12+
private fun assertSize(size: Int, set: CaseInsensitiveMutableStringSet) {
13+
assertEquals(size, set.size)
14+
val emptyAsserter: (Boolean) -> Unit = if (size == 0) ::assertTrue else ::assertFalse
15+
emptyAsserter(set.isEmpty())
16+
}
17+
18+
@Test
19+
fun testInitialization() {
20+
val set = CaseInsensitiveMutableStringSet(input)
21+
assertSize(3, set)
22+
}
23+
24+
@Test
25+
fun testAdd() {
26+
val set = CaseInsensitiveMutableStringSet(input)
27+
set += "durIAN"
28+
assertSize(4, set)
29+
}
30+
31+
@Test
32+
fun testAddAll() {
33+
val set = CaseInsensitiveMutableStringSet(input)
34+
assertFalse(set.addAll(set))
35+
36+
val intersecting = input + "durian"
37+
assertTrue(set.addAll(intersecting))
38+
assertSize(4, set)
39+
}
40+
41+
@Test
42+
fun testClear() {
43+
val set = CaseInsensitiveMutableStringSet(input)
44+
set.clear()
45+
assertSize(0, set)
46+
}
47+
48+
@Test
49+
fun testContains() {
50+
val set = CaseInsensitiveMutableStringSet(input)
51+
variations.forEach { assertTrue("Set should contain element $it") { it in set } }
52+
53+
assertFalse("durian" in set)
54+
}
55+
56+
@Test
57+
fun testContainsAll() {
58+
val set = CaseInsensitiveMutableStringSet(input)
59+
assertTrue(set.containsAll(variations))
60+
61+
val intersecting = input + "durian"
62+
assertFalse(set.containsAll(intersecting))
63+
}
64+
65+
@Test
66+
fun testEquality() {
67+
val left = CaseInsensitiveMutableStringSet(input)
68+
val right = CaseInsensitiveMutableStringSet(input)
69+
assertEquals(left, right)
70+
71+
left -= "apple"
72+
assertNotEquals(left, right)
73+
74+
right -= "ApPlE"
75+
assertEquals(left, right)
76+
}
77+
78+
@Test
79+
fun testIterator() {
80+
val set = CaseInsensitiveMutableStringSet(input)
81+
val iterator = set.iterator()
82+
83+
assertTrue(iterator.hasNext())
84+
assertEquals("apple", iterator.next())
85+
86+
assertTrue(iterator.hasNext())
87+
assertEquals("banana", iterator.next())
88+
iterator.remove()
89+
assertSize(2, set)
90+
91+
assertTrue(iterator.hasNext())
92+
assertEquals("cherry", iterator.next())
93+
94+
assertFalse(iterator.hasNext())
95+
assertTrue(set.containsAll(input - "banana"))
96+
}
97+
98+
@Test
99+
fun testRemove() {
100+
val set = CaseInsensitiveMutableStringSet(input)
101+
set -= "BANANA"
102+
assertSize(2, set)
103+
}
104+
105+
@Test
106+
fun testRemoveAll() {
107+
val set = CaseInsensitiveMutableStringSet(input)
108+
assertFalse(set.removeAll(disjoint))
109+
110+
assertTrue(set.removeAll(intersecting))
111+
assertSize(1, set)
112+
assertTrue("apple" in set)
113+
}
114+
115+
@Test
116+
fun testRetainAll() {
117+
val set = CaseInsensitiveMutableStringSet(input)
118+
assertFalse(set.retainAll(set))
119+
assertSize(3, set)
120+
121+
assertTrue(set.retainAll(intersecting))
122+
assertSize(2, set)
123+
assertTrue(set.containsAll(subset))
124+
}
125+
}

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package aws.smithy.kotlin.runtime.collections
2+
3+
import kotlin.test.Test
4+
import kotlin.test.assertEquals
5+
import kotlin.test.assertNotEquals
6+
7+
class CaseInsensitiveStringTest {
8+
@Test
9+
fun testEquality() {
10+
val left = "Banana".toInsensitive()
11+
val right = "baNAna".toInsensitive()
12+
assertEquals(left, right)
13+
assertNotEquals<Any>("Banana", left)
14+
assertNotEquals<Any>("baNAna", right)
15+
16+
val nonMatching = "apple".toInsensitive()
17+
assertNotEquals(nonMatching, left)
18+
assertNotEquals(nonMatching, right)
19+
}
20+
21+
@Test
22+
fun testProperties() {
23+
val s = "BANANA".toInsensitive()
24+
assertEquals("BANANA", s.original)
25+
assertEquals("banana", s.normalized)
26+
}
27+
}

0 commit comments

Comments
 (0)