diff --git a/changelog/@unreleased/pr-2016.v2.yml b/changelog/@unreleased/pr-2016.v2.yml new file mode 100644 index 000000000..52527ceae --- /dev/null +++ b/changelog/@unreleased/pr-2016.v2.yml @@ -0,0 +1,7 @@ +type: improvement +improvement: + description: |- + UrlBuilder supports adding multiple path segments at once. + Deprecation metric is only created when a deprecated endpoint is invoked. + links: + - https://github.com/palantir/dialogue/pull/2016 diff --git a/dialogue-core/src/main/java/com/palantir/dialogue/core/BaseUrl.java b/dialogue-core/src/main/java/com/palantir/dialogue/core/BaseUrl.java index b454a8f87..91e4fd0d6 100644 --- a/dialogue-core/src/main/java/com/palantir/dialogue/core/BaseUrl.java +++ b/dialogue-core/src/main/java/com/palantir/dialogue/core/BaseUrl.java @@ -20,6 +20,7 @@ import com.google.common.base.CharMatcher; import com.google.common.base.Joiner; import com.google.common.base.Strings; +import com.google.common.collect.Collections2; import com.google.common.collect.ListMultimap; import com.google.common.collect.Multimap; import com.google.common.collect.Multimaps; @@ -36,6 +37,7 @@ import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; @@ -154,6 +156,12 @@ public DefaultUrlBuilder pathSegment(String thePath) { return this; } + @Override + public DefaultUrlBuilder pathSegments(Collection paths) { + this.pathSegments.addAll(Collections2.transform(paths, BaseUrl.UrlEncoder::encodePathSegment)); + return this; + } + /** * URL-encodes the given query parameter name and value and adds them to the list of query parameters. Note that * no guarantee is made regarding the ordering of query parameters in the resulting URL. diff --git a/dialogue-core/src/main/java/com/palantir/dialogue/core/DeprecationWarningChannel.java b/dialogue-core/src/main/java/com/palantir/dialogue/core/DeprecationWarningChannel.java index 19778dfde..945d53655 100644 --- a/dialogue-core/src/main/java/com/palantir/dialogue/core/DeprecationWarningChannel.java +++ b/dialogue-core/src/main/java/com/palantir/dialogue/core/DeprecationWarningChannel.java @@ -19,6 +19,7 @@ import com.codahale.metrics.Meter; import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; +import com.google.common.base.Suppliers; import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.ListenableFuture; import com.palantir.dialogue.Endpoint; @@ -32,6 +33,7 @@ import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import java.time.Duration; import java.util.Optional; +import java.util.function.Supplier; import org.immutables.value.Value; /** @@ -73,19 +75,19 @@ public ListenableFuture execute(Request request) { } private FutureCallback createCallback(String channelName, Endpoint endpoint) { - Meter meter = metrics.deprecations(endpoint.serviceName()); - + // lazily create meter metric name only if deprecated endpoint is accessed + Supplier meterSupplier = Suppliers.memoize(() -> metrics.deprecations(endpoint.serviceName())); return DialogueFutures.onSuccess(response -> { if (response == null) { return; } Optional maybeHeader = response.getFirstHeader("deprecation"); - if (!maybeHeader.isPresent()) { + if (maybeHeader.isEmpty()) { return; } - meter.mark(); + meterSupplier.get().mark(); if (log.isWarnEnabled() && tryAcquire(channelName, endpoint)) { log.warn( "Using a deprecated endpoint when connecting to service", diff --git a/dialogue-core/src/test/java/com/palantir/dialogue/core/UrlBuilderTest.java b/dialogue-core/src/test/java/com/palantir/dialogue/core/UrlBuilderTest.java index 85d4c0a4d..af9a9dee0 100644 --- a/dialogue-core/src/test/java/com/palantir/dialogue/core/UrlBuilderTest.java +++ b/dialogue-core/src/test/java/com/palantir/dialogue/core/UrlBuilderTest.java @@ -23,6 +23,7 @@ import com.palantir.dialogue.TestEndpoint; import java.net.MalformedURLException; import java.net.URL; +import java.util.List; import java.util.UUID; import org.junit.jupiter.api.Test; @@ -65,6 +66,8 @@ public void encodesPaths() throws Exception { assertThat(minimalUrl().pathSegment("foo").build().toString()).isEqualTo("http://host:80/foo"); assertThat(minimalUrl().pathSegment("foo").pathSegment("bar").build().toString()) .isEqualTo("http://host:80/foo/bar"); + assertThat(minimalUrl().pathSegments(List.of("foo", "bar")).build().toString()) + .isEqualTo("http://host:80/foo/bar"); assertThat(minimalUrl().pathSegment("foo/bar").build().toString()).isEqualTo("http://host:80/foo%2Fbar"); assertThat(minimalUrl() .pathSegment("!@#$%^&*()_+{}[]|\\|\"':;/?.>,<~`") @@ -72,6 +75,12 @@ public void encodesPaths() throws Exception { .toString()) .isEqualTo("http://host:80/%21%40%23%24%25%5E%26%2A%28%29_%2B%7B%7D" + "%5B%5D%7C%5C%7C%22%27%3A%3B%2F%3F.%3E%2C%3C~%60"); + assertThat(minimalUrl() + .pathSegments(List.of("!@#$%^&*()_+{}", "[]|\\|\"':;/?.>,<~`")) + .build() + .toString()) + .isEqualTo("http://host:80/%21%40%23%24%25%5E%26%2A%28%29_%2B%7B%7D" + + "/%5B%5D%7C%5C%7C%22%27%3A%3B%2F%3F.%3E%2C%3C~%60"); } @Test @@ -83,6 +92,12 @@ public void handlesEmptyPathSegments() throws Exception { .build() .toString()) .isEqualTo("http://host:80///bar"); + assertThat(minimalUrl() + .pathSegments(List.of("", "")) + .pathSegment("bar") + .build() + .toString()) + .isEqualTo("http://host:80///bar"); } @Test diff --git a/dialogue-target/src/main/java/com/palantir/dialogue/PathTemplate.java b/dialogue-target/src/main/java/com/palantir/dialogue/PathTemplate.java index 116f1d665..b4dafd73a 100644 --- a/dialogue-target/src/main/java/com/palantir/dialogue/PathTemplate.java +++ b/dialogue-target/src/main/java/com/palantir/dialogue/PathTemplate.java @@ -80,7 +80,7 @@ public void fill(ListMultimap parameters, UrlBuilder url) { if (segment.fixed != null) { url.pathSegment(segment.fixed); } else { - parameters.get(segment.variable).forEach(url::pathSegment); + url.pathSegments(parameters.get(segment.variable)); } } } diff --git a/dialogue-target/src/main/java/com/palantir/dialogue/UrlBuilder.java b/dialogue-target/src/main/java/com/palantir/dialogue/UrlBuilder.java index 452e31b93..d01ff545e 100644 --- a/dialogue-target/src/main/java/com/palantir/dialogue/UrlBuilder.java +++ b/dialogue-target/src/main/java/com/palantir/dialogue/UrlBuilder.java @@ -16,11 +16,18 @@ package com.palantir.dialogue; +import java.util.Collection; + public interface UrlBuilder { /** URL-encodes the given path segment and adds it to the list of segments. */ UrlBuilder pathSegment(String thePath); + default UrlBuilder pathSegments(Collection paths) { + paths.forEach(this::pathSegment); + return this; + } + /** * URL-encodes the given query parameter name and value and adds them to the list of query parameters. Note that * no guarantee is made regarding the ordering of query parameters in the resulting URL.