From 51fb582a3604f6a56aef636a6ceaa2d51bc4e165 Mon Sep 17 00:00:00 2001 From: xiangtianyu Date: Wed, 6 Jul 2022 11:07:57 +0800 Subject: [PATCH] bugfix: fix ImmutableKeyValuePairs bugs (#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 * modify some describe mistake Co-authored-by: John Watson * run spotlessApply Co-authored-by: John Watson --- .../api/internal/ImmutableKeyValuePairs.java | 6 ++++++ .../internal/ImmutableKeyValuePairsTest.java | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+) 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 index 333a9bdf91f..270b9bd4f9c 100644 --- 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 Object[] dedupe(Object[] data, Comparator 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 index 9e63ca39be5..e2dd0a159b8 100644 --- 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,6 +21,17 @@ 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 @@ -28,6 +39,14 @@ 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