From fb493a1d3a4ff3e97c53b067860908d363175782 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Mon, 1 Aug 2022 15:33:55 -0400 Subject: [PATCH 1/5] MetricName TagMap supports replacement of existing entries --- .../tritium/metrics/registry/TagMap.java | 8 ++++++-- .../tritium/metrics/registry/TagMapTest.java | 18 ++++++++++++++---- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/TagMap.java b/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/TagMap.java index 5be84b73d..8b79267f5 100644 --- a/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/TagMap.java +++ b/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/TagMap.java @@ -18,7 +18,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Ordering; -import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Arrays; @@ -122,7 +121,12 @@ TagMap withEntry(String key, String value) { String current = local[newPosition]; int comparisonResult = current.compareTo(key); if (comparisonResult == 0) { - throw new SafeIllegalArgumentException("Base must not contain the extra key that is to be added"); + if (Objects.equals(local[newPosition + 1], value)) { + return this; + } + String[] newArray = local.clone(); + newArray[newPosition + 1] = value; + return new TagMap(newArray); } if (comparisonResult > 0) { break; diff --git a/tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/TagMapTest.java b/tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/TagMapTest.java index bc2616049..22c90ffbf 100644 --- a/tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/TagMapTest.java +++ b/tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/TagMapTest.java @@ -17,8 +17,8 @@ package com.palantir.tritium.metrics.registry; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Ordering; import java.util.AbstractMap.SimpleImmutableEntry; @@ -53,12 +53,22 @@ void testSingleton() { @Test void testExtraEntryKeyAlreadyExists() { - TagMap map = TagMap.of(Collections.singletonMap("foo", "bar")); - assertThatThrownBy(() -> map.withEntry("foo", "baz")).isInstanceOf(IllegalArgumentException.class); + TagMap original = TagMap.of(Collections.singletonMap("foo", "bar")); + TagMap updated = original.withEntry("foo", "baz"); + assertThat(updated) + .isEqualTo(ImmutableMap.of("foo", "baz")) + .as("Original must not be mutated") + .isNotSameAs(original); + } + + @Test + void testUpdateWithExisting() { + TagMap map = TagMap.EMPTY.withEntry("foo", "bar"); + assertThat(map.withEntry("foo", "bar")).isSameAs(map); } @Test - public void testNaturalOrder() { + void testNaturalOrder() { assertThat(TagMap.isNaturalOrder(Ordering.natural())).isTrue(); assertThat(TagMap.isNaturalOrder(Comparator.naturalOrder())).isTrue(); From d1fa7e262e2858c2b85c1f41301bb1e2ffafc0f6 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Mon, 1 Aug 2022 15:38:11 -0400 Subject: [PATCH 2/5] docs --- .../main/java/com/palantir/tritium/metrics/registry/TagMap.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/TagMap.java b/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/TagMap.java index 8b79267f5..1769d8687 100644 --- a/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/TagMap.java +++ b/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/TagMap.java @@ -113,7 +113,7 @@ private static String[] toArray(Map data) { return values; } - /** Returns a new {@link TagMap} with on additional entry. */ + /** Returns a new {@link TagMap} with on additional or updated entry. */ TagMap withEntry(String key, String value) { String[] local = this.values; int newPosition = 0; From c8935838cecb800704adeeb60dee0a108a3f6e39 Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Mon, 1 Aug 2022 20:00:37 +0000 Subject: [PATCH 3/5] Add generated changelog entries --- changelog/@unreleased/pr-1537.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-1537.v2.yml diff --git a/changelog/@unreleased/pr-1537.v2.yml b/changelog/@unreleased/pr-1537.v2.yml new file mode 100644 index 000000000..f5cd4e791 --- /dev/null +++ b/changelog/@unreleased/pr-1537.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: MetricName TagMap supports replacement of existing entries + links: + - https://github.com/palantir/tritium/pull/1537 From 9c16dd87375f9b430d355fd825fcdfda0250ea68 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Mon, 1 Aug 2022 16:30:27 -0400 Subject: [PATCH 4/5] Update tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/TagMap.java --- .../main/java/com/palantir/tritium/metrics/registry/TagMap.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/TagMap.java b/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/TagMap.java index 1769d8687..5145b7365 100644 --- a/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/TagMap.java +++ b/tritium-registry/src/main/java/com/palantir/tritium/metrics/registry/TagMap.java @@ -113,7 +113,7 @@ private static String[] toArray(Map data) { return values; } - /** Returns a new {@link TagMap} with on additional or updated entry. */ + /** Returns a new {@link TagMap} with an additional or updated entry. */ TagMap withEntry(String key, String value) { String[] local = this.values; int newPosition = 0; From 8182584f681489ceb8580630711dd9a537a2087d Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Mon, 1 Aug 2022 16:31:40 -0400 Subject: [PATCH 5/5] Update tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/TagMapTest.java Co-authored-by: David Schlosnagle --- .../palantir/tritium/metrics/registry/TagMapTest.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/TagMapTest.java b/tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/TagMapTest.java index 22c90ffbf..afb476b38 100644 --- a/tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/TagMapTest.java +++ b/tritium-registry/src/test/java/com/palantir/tritium/metrics/registry/TagMapTest.java @@ -56,9 +56,17 @@ void testExtraEntryKeyAlreadyExists() { TagMap original = TagMap.of(Collections.singletonMap("foo", "bar")); TagMap updated = original.withEntry("foo", "baz"); assertThat(updated) + .hasSize(1) + .containsEntry("foo", "baz") .isEqualTo(ImmutableMap.of("foo", "baz")) .as("Original must not be mutated") - .isNotSameAs(original); + .isNotSameAs(original) + .isNotEqualTo(original); + assertThat(original) + .hasSize(1) + .as("Original must not be mutated") + .containsEntry("foo", "bar") + .isNotEqualTo(updated); } @Test