From 4610af8b4f3c7a5fe18decf629f63234a0e07bbf Mon Sep 17 00:00:00 2001 From: Fabian Windheuser Date: Fri, 20 Oct 2023 15:29:10 +0100 Subject: [PATCH 1/2] Add test --- .../product/DialogueEteEndpoints.java | 35 +++++++++++++ .../java/com/palantir/product/EteService.java | 9 ++++ .../com/palantir/product/EteServiceAsync.java | 23 ++++++++ .../palantir/product/EteServiceBlocking.java | 23 ++++++++ .../palantir/product/EteServiceEndpoints.java | 52 +++++++++++++++++++ .../palantir/product/EteServiceRetrofit.java | 9 ++++ .../palantir/product/UndertowEteService.java | 5 ++ .../palantir/conjure/java/EteResource.java | 5 ++ .../conjure/java/UndertowServiceEteTest.java | 6 +++ .../src/test/resources/ete-service.yml | 12 +++++ 10 files changed, 179 insertions(+) diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/DialogueEteEndpoints.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/DialogueEteEndpoints.java index 66738b2dc..efde5df93 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/DialogueEteEndpoints.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/DialogueEteEndpoints.java @@ -431,6 +431,41 @@ public String version() { } }, + pathParamRegex { + private final PathTemplate pathTemplate = PathTemplate.builder() + .fixed("base") + .fixed("path") + .variable("paramOne") + .variable("paramTwo") + .variable("paramThree") + .build(); + + @Override + public void renderPath(ListMultimap params, UrlBuilder url) { + pathTemplate.fill(params, url); + } + + @Override + public HttpMethod httpMethod() { + return HttpMethod.GET; + } + + @Override + public String serviceName() { + return "EteService"; + } + + @Override + public String endpointName() { + return "pathParamRegex"; + } + + @Override + public String version() { + return "1.2.3"; + } + }, + optionalExternalLongQuery { private final PathTemplate pathTemplate = PathTemplate.builder() .fixed("base") diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/EteService.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/EteService.java index a047bb81e..791175261 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/EteService.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/EteService.java @@ -111,6 +111,15 @@ public interface EteService { @ClientEndpoint(method = "GET", path = "/base/externalLong/{param}") long externalLongPath(@HeaderParam("Authorization") @NotNull AuthHeader authHeader, @PathParam("param") long param); + @GET + @Path("base/path/{paramOne}/{paramTwo:.+}/{paramThree:.*}") + @ClientEndpoint(method = "GET", path = "/base/path/{paramOne}/{paramTwo:.+}/{paramThree:.*}") + String pathParamRegex( + @HeaderParam("Authorization") @NotNull AuthHeader authHeader, + @PathParam("paramOne") String paramOne, + @PathParam("paramTwo") String paramTwo, + @PathParam("paramThree") String paramThree); + @GET @Path("base/optionalExternalLong") @ClientEndpoint(method = "GET", path = "/base/optionalExternalLong") diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceAsync.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceAsync.java index ff3dd8f3e..3d4bb4820 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceAsync.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceAsync.java @@ -124,6 +124,12 @@ public interface EteServiceAsync { @ClientEndpoint(method = "GET", path = "/base/externalLong/{param}") ListenableFuture externalLongPath(AuthHeader authHeader, long param); + /** + * @apiNote {@code GET /base/path/{paramOne}/{paramTwo:.+}/{paramThree:.*}} + */ + @ClientEndpoint(method = "GET", path = "/base/path/{paramOne}/{paramTwo:.+}/{paramThree:.*}") + ListenableFuture pathParamRegex(AuthHeader authHeader, String paramOne, String paramTwo, String paramThree); + /** * @apiNote {@code GET /base/optionalExternalLong} */ @@ -320,6 +326,12 @@ static EteServiceAsync of(EndpointChannelFactory _endpointChannelFactory, Conjur private final Deserializer externalLongPathDeserializer = _runtime.bodySerDe().deserializer(new TypeMarker() {}); + private final EndpointChannel pathParamRegexChannel = + _endpointChannelFactory.endpoint(DialogueEteEndpoints.pathParamRegex); + + private final Deserializer pathParamRegexDeserializer = + _runtime.bodySerDe().deserializer(new TypeMarker() {}); + private final EndpointChannel optionalExternalLongQueryChannel = _endpointChannelFactory.endpoint(DialogueEteEndpoints.optionalExternalLongQuery); @@ -543,6 +555,17 @@ public ListenableFuture externalLongPath(AuthHeader authHeader, long param return _runtime.clients().call(externalLongPathChannel, _request.build(), externalLongPathDeserializer); } + @Override + public ListenableFuture pathParamRegex( + AuthHeader authHeader, String paramOne, String paramTwo, String paramThree) { + Request.Builder _request = Request.builder(); + _request.putHeaderParams("Authorization", authHeader.toString()); + _request.putPathParams("paramOne", _plainSerDe.serializeString(paramOne)); + _request.putPathParams("paramTwo", _plainSerDe.serializeString(paramTwo)); + _request.putPathParams("paramThree", _plainSerDe.serializeString(paramThree)); + return _runtime.clients().call(pathParamRegexChannel, _request.build(), pathParamRegexDeserializer); + } + @Override public ListenableFuture> optionalExternalLongQuery( AuthHeader authHeader, Optional param) { diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceBlocking.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceBlocking.java index 08bc418f3..b1f23775f 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceBlocking.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceBlocking.java @@ -125,6 +125,12 @@ public interface EteServiceBlocking { @ClientEndpoint(method = "GET", path = "/base/externalLong/{param}") long externalLongPath(AuthHeader authHeader, long param); + /** + * @apiNote {@code GET /base/path/{paramOne}/{paramTwo:.+}/{paramThree:.*}} + */ + @ClientEndpoint(method = "GET", path = "/base/path/{paramOne}/{paramTwo:.+}/{paramThree:.*}") + String pathParamRegex(AuthHeader authHeader, String paramOne, String paramTwo, String paramThree); + /** * @apiNote {@code GET /base/optionalExternalLong} */ @@ -316,6 +322,12 @@ static EteServiceBlocking of(EndpointChannelFactory _endpointChannelFactory, Con private final Deserializer externalLongPathDeserializer = _runtime.bodySerDe().deserializer(new TypeMarker() {}); + private final EndpointChannel pathParamRegexChannel = + _endpointChannelFactory.endpoint(DialogueEteEndpoints.pathParamRegex); + + private final Deserializer pathParamRegexDeserializer = + _runtime.bodySerDe().deserializer(new TypeMarker() {}); + private final EndpointChannel optionalExternalLongQueryChannel = _endpointChannelFactory.endpoint(DialogueEteEndpoints.optionalExternalLongQuery); @@ -542,6 +554,17 @@ public long externalLongPath(AuthHeader authHeader, long param) { .callBlocking(externalLongPathChannel, _request.build(), externalLongPathDeserializer); } + @Override + public String pathParamRegex(AuthHeader authHeader, String paramOne, String paramTwo, String paramThree) { + Request.Builder _request = Request.builder(); + _request.putHeaderParams("Authorization", authHeader.toString()); + _request.putPathParams("paramOne", _plainSerDe.serializeString(paramOne)); + _request.putPathParams("paramTwo", _plainSerDe.serializeString(paramTwo)); + _request.putPathParams("paramThree", _plainSerDe.serializeString(paramThree)); + return _runtime.clients() + .callBlocking(pathParamRegexChannel, _request.build(), pathParamRegexDeserializer); + } + @Override public Optional optionalExternalLongQuery(AuthHeader authHeader, Optional param) { Request.Builder _request = Request.builder(); diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceEndpoints.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceEndpoints.java index 396640060..32cf100d4 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceEndpoints.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceEndpoints.java @@ -59,6 +59,7 @@ public List endpoints(UndertowRuntime runtime) { new BinaryEndpoint(runtime, delegate), new PathEndpoint(runtime, delegate), new ExternalLongPathEndpoint(runtime, delegate), + new PathParamRegexEndpoint(runtime, delegate), new OptionalExternalLongQueryEndpoint(runtime, delegate), new NotNullBodyEndpoint(runtime, delegate), new AliasOneEndpoint(runtime, delegate), @@ -695,6 +696,57 @@ public HttpHandler handler() { } } + private static final class PathParamRegexEndpoint implements HttpHandler, Endpoint { + private final UndertowRuntime runtime; + + private final UndertowEteService delegate; + + private final Serializer serializer; + + PathParamRegexEndpoint(UndertowRuntime runtime, UndertowEteService delegate) { + this.runtime = runtime; + this.delegate = delegate; + this.serializer = runtime.bodySerDe().serializer(new TypeMarker() {}, this); + } + + @Override + public void handleRequest(HttpServerExchange exchange) throws IOException { + AuthHeader authHeader = runtime.auth().header(exchange); + Map pathParams = + exchange.getAttachment(PathTemplateMatch.ATTACHMENT_KEY).getParameters(); + String paramOne = runtime.plainSerDe().deserializeString(pathParams.get("paramOne")); + String paramTwo = runtime.plainSerDe().deserializeString(pathParams.get("paramTwo")); + String paramThree = runtime.plainSerDe().deserializeString(pathParams.get("paramThree")); + String result = delegate.pathParamRegex(authHeader, paramOne, paramTwo, paramThree); + serializer.serialize(result, exchange); + } + + @Override + public HttpString method() { + return Methods.GET; + } + + @Override + public String template() { + return "/base/path/{paramOne}/{paramTwo:.+}/{paramThree:.*}"; + } + + @Override + public String serviceName() { + return "EteService"; + } + + @Override + public String name() { + return "pathParamRegex"; + } + + @Override + public HttpHandler handler() { + return this; + } + } + private static final class OptionalExternalLongQueryEndpoint implements HttpHandler, Endpoint { private final UndertowRuntime runtime; diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceRetrofit.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceRetrofit.java index 7f2920c15..c4a4d21e6 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceRetrofit.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceRetrofit.java @@ -107,6 +107,15 @@ public interface EteServiceRetrofit { @ClientEndpoint(method = "GET", path = "/base/externalLong/{param}") ListenableFuture externalLongPath(@Header("Authorization") AuthHeader authHeader, @Path("param") long param); + @GET("./base/path/{paramOne}/{paramTwo}/{paramThree}") + @Headers({"hr-path-template: /base/path/{paramOne}/{paramTwo}/{paramThree}", "Accept: application/json"}) + @ClientEndpoint(method = "GET", path = "/base/path/{paramOne}/{paramTwo:.+}/{paramThree:.*}") + ListenableFuture pathParamRegex( + @Header("Authorization") AuthHeader authHeader, + @Path("paramOne") String paramOne, + @Path(value = "paramTwo", encoded = true) String paramTwo, + @Path(value = "paramThree", encoded = true) String paramThree); + @GET("./base/optionalExternalLong") @Headers({"hr-path-template: /base/optionalExternalLong", "Accept: application/json"}) @ClientEndpoint(method = "GET", path = "/base/optionalExternalLong") diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/UndertowEteService.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/UndertowEteService.java index 00c9665ad..9cf7d0bf4 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/UndertowEteService.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/UndertowEteService.java @@ -88,6 +88,11 @@ public interface UndertowEteService { */ long externalLongPath(AuthHeader authHeader, long param); + /** + * @apiNote {@code GET /base/path/{paramOne}/{paramTwo:.+}/{paramThree:.*}} + */ + String pathParamRegex(AuthHeader authHeader, String paramOne, String paramTwo, String paramThree); + /** * @apiNote {@code GET /base/optionalExternalLong} */ diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/EteResource.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/EteResource.java index dc60ae42b..05c02e757 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/EteResource.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/EteResource.java @@ -104,6 +104,11 @@ public long externalLongPath(AuthHeader _authHeader, long param) { return param; } + @Override + public String pathParamRegex(AuthHeader authHeader, String paramOne, String paramTwo, String paramThree) { + return paramOne + "," + paramTwo + "," + paramThree; + } + @Override public Optional optionalExternalLongQuery(AuthHeader _authHeader, Optional param) { return param; diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/UndertowServiceEteTest.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/UndertowServiceEteTest.java index 735114ecd..02a5e9646 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/UndertowServiceEteTest.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/UndertowServiceEteTest.java @@ -428,6 +428,12 @@ public void testSpaceInPathParam() { .isEqualTo(expected); } + @Test + public void testRegexPath() { + assertThat(client.pathParamRegex(AuthHeader.valueOf("bearer"), "foo", "bar", "baz")) + .isEqualTo("foo,bar,baz"); + } + @Test public void testBinaryOptionalEmptyResponse() { Optional response = diff --git a/conjure-java-core/src/test/resources/ete-service.yml b/conjure-java-core/src/test/resources/ete-service.yml index 3e92dc928..d48714c8f 100644 --- a/conjure-java-core/src/test/resources/ete-service.yml +++ b/conjure-java-core/src/test/resources/ete-service.yml @@ -111,6 +111,18 @@ services: param: Long returns: Long + # The Conjure spec technically supports path params regexes even though they are unused + pathParamRegex: + http: GET /path/{paramOne}/{paramTwo:.+}/{paramThree:.*} + args: + paramOne: + type: string + paramTwo: + type: string + paramThree: + type: string + returns: string + optionalExternalLongQuery: http: GET /optionalExternalLong args: From bde72602aa106e48dc9a4068df4eda6be7a5b668 Mon Sep 17 00:00:00 2001 From: Fabian Windheuser Date: Fri, 20 Oct 2023 15:05:14 +0100 Subject: [PATCH 2/2] Ignore undertow regex path params --- .../palantir/product/EteServiceEndpoints.java | 2 +- .../UndertowServiceHandlerGenerator.java | 22 ++++++++- .../UndertowServiceHandlerGeneratorTest.java | 46 +++++++++++++++++++ .../api/TestServiceEndpoints.java.undertow | 2 +- .../TestServiceEndpoints.java.undertow.prefix | 2 +- 5 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 conjure-java-core/src/test/java/com/palantir/conjure/java/services/UndertowServiceHandlerGeneratorTest.java diff --git a/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceEndpoints.java b/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceEndpoints.java index 32cf100d4..d7bbab305 100644 --- a/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceEndpoints.java +++ b/conjure-java-core/src/integrationInput/java/com/palantir/product/EteServiceEndpoints.java @@ -728,7 +728,7 @@ public HttpString method() { @Override public String template() { - return "/base/path/{paramOne}/{paramTwo:.+}/{paramThree:.*}"; + return "/base/path/{paramOne}/{paramTwo}/{paramThree}"; } @Override diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/services/UndertowServiceHandlerGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/services/UndertowServiceHandlerGenerator.java index 54f6622f4..c58bee931 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/services/UndertowServiceHandlerGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/services/UndertowServiceHandlerGenerator.java @@ -16,6 +16,7 @@ package com.palantir.conjure.java.services; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -53,6 +54,7 @@ import com.palantir.conjure.spec.EndpointName; import com.palantir.conjure.spec.ExternalReference; import com.palantir.conjure.spec.HeaderAuthType; +import com.palantir.conjure.spec.HttpPath; import com.palantir.conjure.spec.ListType; import com.palantir.conjure.spec.LogSafety; import com.palantir.conjure.spec.OptionalType; @@ -95,11 +97,16 @@ import java.util.Optional; import java.util.Set; import java.util.function.Function; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.lang.model.element.Modifier; import org.apache.commons.lang3.StringUtils; final class UndertowServiceHandlerGenerator { + private static final Pattern REGEX_PATH_TEMPLATES = + Pattern.compile("\\{([a-zA-Z0-9]+)" + "(" + Pattern.quote(":.+") + "|" + Pattern.quote(":.*") + ")" + "}"); + private static final String EXCHANGE_VAR_NAME = "exchange"; private static final String DELEGATE_VAR_NAME = "delegate"; private static final String RUNTIME_VAR_NAME = "runtime"; @@ -329,7 +336,7 @@ private TypeSpec generateEndpointHandler( .addModifiers(Modifier.PUBLIC) .addAnnotation(Override.class) .returns(String.class) - .addStatement("return $1S", endpointDefinition.getHttpPath()) + .addStatement("return $1S", normalizeHttpPathTemplates(endpointDefinition.getHttpPath())) .build()) .addMethod(MethodSpec.methodBuilder("serviceName") .addModifiers(Modifier.PUBLIC) @@ -1168,4 +1175,17 @@ && requiresRequestContext(endpoint, evaluator))) { } return value; } + + /** + * The Conjure spec allows for regex path param validations ("{param:.+}" and "{param:.*}"), however Undertow does + * not support these for path templates. Based on that, we strip these validation regexes from the http path + * template. + * @see "https://github.com/palantir/conjure-java/pull/2119" + */ + @VisibleForTesting + static HttpPath normalizeHttpPathTemplates(HttpPath httpPath) { + Matcher matcher = REGEX_PATH_TEMPLATES.matcher(httpPath.get()); + String normalized = matcher.replaceAll("{$1}"); + return HttpPath.of(normalized); + } } diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/services/UndertowServiceHandlerGeneratorTest.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/services/UndertowServiceHandlerGeneratorTest.java new file mode 100644 index 000000000..58aaa11c6 --- /dev/null +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/services/UndertowServiceHandlerGeneratorTest.java @@ -0,0 +1,46 @@ +/* + * (c) Copyright 2023 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.conjure.java.services; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.palantir.conjure.spec.HttpPath; +import java.util.stream.Stream; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +final class UndertowServiceHandlerGeneratorTest { + + @ParameterizedTest + @MethodSource("normalizes_http_path_templates") + void normalizes_http_path_templates(String given, String expected) { + HttpPath normalized = UndertowServiceHandlerGenerator.normalizeHttpPathTemplates(HttpPath.of(given)); + + assertThat(normalized).isEqualTo(HttpPath.of(expected)); + } + + private static Stream normalizes_http_path_templates() { + return Stream.of( + Arguments.of("{param:.+}", "{param}"), + Arguments.of("{param:.*}", "{param}"), + Arguments.of("{param}", "{param}"), + // We ignore any other regex, because those are not allowed by the Conjure spec. + Arguments.of("{param:[a-zA-Z0-9]+}", "{param:[a-zA-Z0-9]+}"), + Arguments.of("/foo/{paramA:.*}/{paramB:.+}/{paramC}", "/foo/{paramA}/{paramB}/{paramC}")); + } +} diff --git a/conjure-java-core/src/test/resources/test/api/TestServiceEndpoints.java.undertow b/conjure-java-core/src/test/resources/test/api/TestServiceEndpoints.java.undertow index 5bf79213e..bf66a498b 100644 --- a/conjure-java-core/src/test/resources/test/api/TestServiceEndpoints.java.undertow +++ b/conjure-java-core/src/test/resources/test/api/TestServiceEndpoints.java.undertow @@ -668,7 +668,7 @@ public final class TestServiceEndpoints implements UndertowService { @Override public String template() { - return "/catalog/datasets/{datasetRid}/branches/{branch:.+}/resolve"; + return "/catalog/datasets/{datasetRid}/branches/{branch}/resolve"; } @Override diff --git a/conjure-java-core/src/test/resources/test/api/TestServiceEndpoints.java.undertow.prefix b/conjure-java-core/src/test/resources/test/api/TestServiceEndpoints.java.undertow.prefix index 71691ac8f..a9dc19087 100644 --- a/conjure-java-core/src/test/resources/test/api/TestServiceEndpoints.java.undertow.prefix +++ b/conjure-java-core/src/test/resources/test/api/TestServiceEndpoints.java.undertow.prefix @@ -668,7 +668,7 @@ public final class TestServiceEndpoints implements UndertowService { @Override public String template() { - return "/catalog/datasets/{datasetRid}/branches/{branch:.+}/resolve"; + return "/catalog/datasets/{datasetRid}/branches/{branch}/resolve"; } @Override