bugfix: fix ImmutableKeyValuePairs bugs (open-telemetry#4573)

* fix bug * change the fix * add new cases and changed the way to fix the bug * fix some describe words Co-authored-by: John Watson <[email protected]> * modify some describe mistake Co-authored-by: John Watson <[email protected]> * run spotlessApply Co-authored-by: John Watson <[email protected]>
LWogan · Jul 6, 2022 · 51fb582 · 51fb582
1 parent d2a8304
commit 51fb582
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 0 deletions.
diff --git a/api/all/src/main/java/io/opentelemetry/api/internal/ImmutableKeyValuePairs.java b/api/all/src/main/java/io/opentelemetry/api/internal/ImmutableKeyValuePairs.java
@@ -211,6 +211,12 @@ private static <K> Object[] dedupe(Object[] data, Comparator<K> keyComparator) {
       // Skip entries with null value, we do it here because we want them to overwrite and remove
       // entries with same key that we already added.
       if (value == null) {
+        // When the value is null, there are two cases:
+        // 1. next key is the same as the current one, it may cause ArrayIndexOutOfBoundsException,
+        // so we reset the previous key to null to avoid this
+        // 2. next key is different than the current one; In this case, whether the previous key is
+        // null or not will have no impact.
+        previousKey = null;
         continue;
       }
       previousKey = key;

diff --git a/api/all/src/test/java/io/opentelemetry/api/internal/ImmutableKeyValuePairsTest.java b/api/all/src/test/java/io/opentelemetry/api/internal/ImmutableKeyValuePairsTest.java
@@ -21,13 +21,32 @@ void get() {
     assertThat(new TestPairs(new Object[] {"one", 55, "two", "b"}).get("one")).isEqualTo(55);
     assertThat(new TestPairs(new Object[] {"one", 55, "two", "b"}).get("two")).isEqualTo("b");
     assertThat(new TestPairs(new Object[] {"one", 55, "two", "b"}).get("three")).isNull();
+    assertThat(new TestPairs(new Object[] {"one", 55, "one", null, "one", 66}).get("one"))
+        .isEqualTo(66);
+    assertThat(
+            new TestPairs(new Object[] {"one", 55, "one", null, "one", 66, "one", null}).get("two"))
+        .isNull();
+    assertThat(
+            new TestPairs(new Object[] {"one", 55, "two", "b", "one", null, "one", 66}).get("one"))
+        .isEqualTo(66);
+    assertThat(
+            new TestPairs(new Object[] {"one", 55, "two", 66, "two", null, "two", 77}).get("two"))
+        .isEqualTo(77);
   }
 
   @Test
   void size() {
     assertThat(new TestPairs(new Object[0]).size()).isEqualTo(0);
     assertThat(new TestPairs(new Object[] {"one", 55}).size()).isEqualTo(1);
     assertThat(new TestPairs(new Object[] {"one", 55, "two", "b"}).size()).isEqualTo(2);
+    assertThat(new TestPairs(new Object[] {"one", 55, "one", null, "one", 66}).size()).isEqualTo(1);
+    assertThat(new TestPairs(new Object[] {"one", 55, "one", null, "one", 66, "one", null}).size())
+        .isEqualTo(0);
+    assertThat(new TestPairs(new Object[] {"one", 55, "two", "b", "one", null, "one", 66}).size())
+        .isEqualTo(2);
+    assertThat(new TestPairs(new Object[] {"one", 55, "two", 66, "two", null}).size()).isEqualTo(1);
+    assertThat(new TestPairs(new Object[] {"one", 55, "two", 66, "two", null, "two", 77}).size())
+        .isEqualTo(2);
   }
 
   @Test