diff --git a/spectator-api/src/main/java/com/netflix/spectator/api/ArrayTagSet.java b/spectator-api/src/main/java/com/netflix/spectator/api/ArrayTagSet.java index 3f14e6bc5..7b0b34a51 100644 --- a/spectator-api/src/main/java/com/netflix/spectator/api/ArrayTagSet.java +++ b/spectator-api/src/main/java/com/netflix/spectator/api/ArrayTagSet.java @@ -104,34 +104,40 @@ boolean isEmpty() { } /** Add a new tag to the set. */ - @SuppressWarnings("PMD.AvoidArrayLoops") ArrayTagSet add(String k, String v) { Preconditions.checkNotNull(k, "key"); Preconditions.checkNotNull(v, "value"); if (length == 0) { return new ArrayTagSet(new String[] {k, v}); - } else { - String[] newTags = new String[length + 2]; - int i = 0; - for (; i < length && tags[i].compareTo(k) < 0; i += 2) { - newTags[i] = tags[i]; - newTags[i + 1] = tags[i + 1]; + } + + int idx = 0; + int cmp = -1; + while (idx < length) { + cmp = tags[idx].compareTo(k); + if (cmp >= 0) { + break; } - if (i < length && tags[i].equals(k)) { - // Override - newTags[i++] = k; - newTags[i++] = v; - System.arraycopy(tags, i, newTags, i, length - i); - i = length; - } else { - // Insert - newTags[i] = k; - newTags[i + 1] = v; - System.arraycopy(tags, i, newTags, i + 2, length - i); - i = newTags.length; + idx += 2; + } + + // Update an existing tag + if (cmp == 0) { + if (tags[idx + 1].equals(v)) { + return this; } - return new ArrayTagSet(newTags, i); + String[] newTags = Arrays.copyOf(tags, length); + newTags[idx + 1] = v; + return new ArrayTagSet(newTags); } + + String[] newTags = new String[length + 2]; + newTags[idx] = k; + newTags[idx + 1] = v; + + System.arraycopy(tags, 0, newTags, 0, idx); + System.arraycopy(tags, idx, newTags, idx + 2, length - idx); + return new ArrayTagSet(newTags); } /** Add a new tag to the set. */ diff --git a/spectator-api/src/test/java/com/netflix/spectator/api/ArrayTagSetTest.java b/spectator-api/src/test/java/com/netflix/spectator/api/ArrayTagSetTest.java index 6a4e168f5..d3586127f 100644 --- a/spectator-api/src/test/java/com/netflix/spectator/api/ArrayTagSetTest.java +++ b/spectator-api/src/test/java/com/netflix/spectator/api/ArrayTagSetTest.java @@ -640,4 +640,39 @@ public void testCreateWithSortedMapCustomComparatorNaturalOrder() { List tagList = StreamSupport.stream(tags.spliterator(), false).collect(Collectors.toList()); Assertions.assertEquals(Arrays.asList(Tag.of("a", "v1"), Tag.of("b", "v2"), Tag.of("c", "v3"), Tag.of("d", "v4")), tagList); } + + @Test + public void testAddBeginning() { + ArrayTagSet tags = ArrayTagSet.create("a", "v1", "b", "v2", "c", "v3"); + ArrayTagSet updated = tags.add("0", "v0"); + Assertions.assertEquals(ArrayTagSet.create("0", "v0", "a", "v1", "b", "v2", "c", "v3"), updated); + } + + @Test + public void testAddEnd() { + ArrayTagSet tags = ArrayTagSet.create("a", "v1", "b", "v2", "c", "v3"); + ArrayTagSet updated = tags.add("d", "v4"); + Assertions.assertEquals(ArrayTagSet.create("a", "v1", "b", "v2", "c", "v3", "d", "v4"), updated); + } + + @Test + public void testAddMiddle() { + ArrayTagSet tags = ArrayTagSet.create("a", "v1", "b", "v2", "d", "v4"); + ArrayTagSet updated = tags.add("c", "v3"); + Assertions.assertEquals(ArrayTagSet.create("a", "v1", "b", "v2", "c", "v3", "d", "v4"), updated); + } + + @Test + public void testAddUpdatesExisting() { + ArrayTagSet tags = ArrayTagSet.create("a", "v1", "b", "v2", "c", "v3"); + ArrayTagSet updated = tags.add("c", "v3-updated"); + Assertions.assertEquals(ArrayTagSet.create("a", "v1", "b", "v2", "c", "v3-updated"), updated); + } + + @Test + public void testAddUpdatesExistingWithSameTag() { + ArrayTagSet tags = ArrayTagSet.create("a", "v1", "b", "v2", "c", "v3"); + ArrayTagSet updated = tags.add("c", "v3"); + Assertions.assertSame(tags, updated); + } }