From 56b954af25f0c20905ea322fa72369985346d940 Mon Sep 17 00:00:00 2001 From: Glenn Sheasby Date: Mon, 22 Jun 2020 11:49:24 +0100 Subject: [PATCH 1/6] Add compatibility backfill methods for optional header params --- .../services/Retrofit2ServiceGenerator.java | 24 +++++++++---- .../src/test/resources/example-service.yml | 15 ++++++++ .../api/TestServiceRetrofit.java.retrofit | 35 +++++++++++++++++++ .../TestServiceRetrofit.java.retrofit.prefix | 35 +++++++++++++++++++ 4 files changed, 102 insertions(+), 7 deletions(-) diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/services/Retrofit2ServiceGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/services/Retrofit2ServiceGenerator.java index 2c366d033..8af40d409 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/services/Retrofit2ServiceGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/services/Retrofit2ServiceGenerator.java @@ -19,6 +19,7 @@ import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Lists; import com.palantir.conjure.java.ConjureAnnotations; import com.palantir.conjure.java.Options; import com.palantir.conjure.java.types.DefaultClassNameVisitor; @@ -201,9 +202,14 @@ private HttpPath replaceEncodedPathArgs(HttpPath httpPath) { private List generateCompatibilityBackfillServiceMethods( EndpointDefinition endpointDef, TypeMapper returnTypeMapper, TypeMapper argumentTypeMapper) { Set encodedPathArgs = extractEncodedPathArgs(endpointDef.getHttpPath()); + List headerArgs = new ArrayList<>(); List queryArgs = new ArrayList<>(); for (ArgumentDefinition arg : endpointDef.getArgs()) { + if (arg.getParamType().accept(ParameterTypeVisitor.IS_HEADER) + && arg.getType().accept(DefaultableTypeVisitor.INSTANCE)) { + headerArgs.add(arg); + } if (arg.getParamType().accept(ParameterTypeVisitor.IS_QUERY) && arg.getType().accept(DefaultableTypeVisitor.INSTANCE)) { queryArgs.add(arg); @@ -211,13 +217,17 @@ private List generateCompatibilityBackfillServiceMethods( } List alternateMethods = new ArrayList<>(); - for (int i = 0; i < queryArgs.size(); i++) { - alternateMethods.add(createCompatibilityBackfillMethod( - endpointDef, - returnTypeMapper, - argumentTypeMapper, - encodedPathArgs, - queryArgs.subList(i, queryArgs.size()))); + for (int j = 0; j <= headerArgs.size(); j++) { + for (int i = 0; i <= queryArgs.size(); i++) { + List extraHeaderArgs = headerArgs.subList(j, headerArgs.size()); + List extraQueryArgs = queryArgs.subList(i, queryArgs.size()); + List extraArgs = Lists.newArrayList(extraHeaderArgs); + extraArgs.addAll(extraQueryArgs); + if (!extraArgs.isEmpty()) { + alternateMethods.add(createCompatibilityBackfillMethod( + endpointDef, returnTypeMapper, argumentTypeMapper, encodedPathArgs, extraArgs)); + } + } } return alternateMethods; diff --git a/conjure-java-core/src/test/resources/example-service.yml b/conjure-java-core/src/test/resources/example-service.yml index afc70d92a..861bbbfff 100644 --- a/conjure-java-core/src/test/resources/example-service.yml +++ b/conjure-java-core/src/test/resources/example-service.yml @@ -224,6 +224,21 @@ services: type: optional param-type: query + testOptionalHeaderParam: + http: POST /test-optional-header-param + args: + query: string + maybeHeader: + type: optional + param-type: header + param-id: MaybeHeader + implicit: + type: rid + param-type: query + maybeQuery: + type: optional + param-type: query + testBoolean: http: GET /boolean returns: boolean diff --git a/conjure-java-core/src/test/resources/test/api/TestServiceRetrofit.java.retrofit b/conjure-java-core/src/test/resources/test/api/TestServiceRetrofit.java.retrofit index e697d0068..bf036188a 100644 --- a/conjure-java-core/src/test/resources/test/api/TestServiceRetrofit.java.retrofit +++ b/conjure-java-core/src/test/resources/test/api/TestServiceRetrofit.java.retrofit @@ -135,6 +135,15 @@ public interface TestServiceRetrofit { @Query("optionalEnd") Optional optionalEnd, @Body String query); + @POST("./catalog/test-optional-header-param") + @Headers({"hr-path-template: /catalog/test-optional-header-param", "Accept: application/json"}) + ListenableFuture testOptionalHeaderParam( + @Header("Authorization") AuthHeader authHeader, + @Header("MaybeHeader") Optional maybeHeader, + @Query("implicit") ResourceIdentifier implicit, + @Query("maybeQuery") Optional maybeQuery, + @Body String query); + @GET("./catalog/boolean") @Headers({"hr-path-template: /catalog/boolean", "Accept: application/json"}) ListenableFuture testBoolean(@Header("Authorization") AuthHeader authHeader); @@ -230,6 +239,32 @@ public interface TestServiceRetrofit { testNoResponseQueryParams(authHeader, something, implicit, optionalMiddle, setEnd, Optional.empty(), query); } + @Deprecated + default void testOptionalHeaderParam( + @Header("Authorization") AuthHeader authHeader, + @Query("implicit") ResourceIdentifier implicit, + @Body String query) { + testOptionalHeaderParam(authHeader, Optional.empty(), implicit, Optional.empty(), query); + } + + @Deprecated + default void testOptionalHeaderParam( + @Header("Authorization") AuthHeader authHeader, + @Query("implicit") ResourceIdentifier implicit, + @Query("maybeQuery") Optional maybeQuery, + @Body String query) { + testOptionalHeaderParam(authHeader, Optional.empty(), implicit, maybeQuery, query); + } + + @Deprecated + default void testOptionalHeaderParam( + @Header("Authorization") AuthHeader authHeader, + @Header("MaybeHeader") Optional maybeHeader, + @Query("implicit") ResourceIdentifier implicit, + @Body String query) { + testOptionalHeaderParam(authHeader, maybeHeader, implicit, Optional.empty(), query); + } + @Deprecated default void testOptionalIntegerAndDouble(@Header("Authorization") AuthHeader authHeader) { testOptionalIntegerAndDouble(authHeader, OptionalInt.empty(), OptionalDouble.empty()); diff --git a/conjure-java-core/src/test/resources/test/api/TestServiceRetrofit.java.retrofit.prefix b/conjure-java-core/src/test/resources/test/api/TestServiceRetrofit.java.retrofit.prefix index 9b5bf230f..1f3c27edd 100644 --- a/conjure-java-core/src/test/resources/test/api/TestServiceRetrofit.java.retrofit.prefix +++ b/conjure-java-core/src/test/resources/test/api/TestServiceRetrofit.java.retrofit.prefix @@ -135,6 +135,15 @@ public interface TestServiceRetrofit { @Query("optionalEnd") Optional optionalEnd, @Body String query); + @POST("./catalog/test-optional-header-param") + @Headers({"hr-path-template: /catalog/test-optional-header-param", "Accept: application/json"}) + ListenableFuture testOptionalHeaderParam( + @Header("Authorization") AuthHeader authHeader, + @Header("MaybeHeader") Optional maybeHeader, + @Query("implicit") ResourceIdentifier implicit, + @Query("maybeQuery") Optional maybeQuery, + @Body String query); + @GET("./catalog/boolean") @Headers({"hr-path-template: /catalog/boolean", "Accept: application/json"}) ListenableFuture testBoolean(@Header("Authorization") AuthHeader authHeader); @@ -230,6 +239,32 @@ public interface TestServiceRetrofit { testNoResponseQueryParams(authHeader, something, implicit, optionalMiddle, setEnd, Optional.empty(), query); } + @Deprecated + default void testOptionalHeaderParam( + @Header("Authorization") AuthHeader authHeader, + @Query("implicit") ResourceIdentifier implicit, + @Body String query) { + testOptionalHeaderParam(authHeader, Optional.empty(), implicit, Optional.empty(), query); + } + + @Deprecated + default void testOptionalHeaderParam( + @Header("Authorization") AuthHeader authHeader, + @Query("implicit") ResourceIdentifier implicit, + @Query("maybeQuery") Optional maybeQuery, + @Body String query) { + testOptionalHeaderParam(authHeader, Optional.empty(), implicit, maybeQuery, query); + } + + @Deprecated + default void testOptionalHeaderParam( + @Header("Authorization") AuthHeader authHeader, + @Header("MaybeHeader") Optional maybeHeader, + @Query("implicit") ResourceIdentifier implicit, + @Body String query) { + testOptionalHeaderParam(authHeader, maybeHeader, implicit, Optional.empty(), query); + } + @Deprecated default void testOptionalIntegerAndDouble(@Header("Authorization") AuthHeader authHeader) { testOptionalIntegerAndDouble(authHeader, OptionalInt.empty(), OptionalDouble.empty()); From eb77f79e7663ec3fd45541d7f07df84745c1ed08 Mon Sep 17 00:00:00 2001 From: Glenn Sheasby Date: Mon, 22 Jun 2020 12:04:45 +0100 Subject: [PATCH 2/6] fix compile --- .../conjure/java/services/Retrofit2ServiceGenerator.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/services/Retrofit2ServiceGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/services/Retrofit2ServiceGenerator.java index 8af40d409..37082bc0b 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/services/Retrofit2ServiceGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/services/Retrofit2ServiceGenerator.java @@ -19,7 +19,6 @@ import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Lists; import com.palantir.conjure.java.ConjureAnnotations; import com.palantir.conjure.java.Options; import com.palantir.conjure.java.types.DefaultClassNameVisitor; @@ -221,7 +220,7 @@ private List generateCompatibilityBackfillServiceMethods( for (int i = 0; i <= queryArgs.size(); i++) { List extraHeaderArgs = headerArgs.subList(j, headerArgs.size()); List extraQueryArgs = queryArgs.subList(i, queryArgs.size()); - List extraArgs = Lists.newArrayList(extraHeaderArgs); + List extraArgs = new ArrayList<>(extraHeaderArgs); extraArgs.addAll(extraQueryArgs); if (!extraArgs.isEmpty()) { alternateMethods.add(createCompatibilityBackfillMethod( From 0602a27c5dc8b2a686927c9042b3c9137e5756ee Mon Sep 17 00:00:00 2001 From: Glenn Sheasby Date: Mon, 22 Jun 2020 14:22:44 +0100 Subject: [PATCH 3/6] Fix dialogue and undertow tests --- .../com/palantir/conjure/java/TestBase.java | 2 +- .../api/DialogueTestEndpoints.java.dialogue | 32 +++++++++++ ...DialogueTestEndpoints.java.dialogue.prefix | 32 +++++++++++ .../test/api/TestService.java.undertow | 7 +++ .../test/api/TestService.java.undertow.prefix | 7 +++ .../test/api/TestServiceAsync.java.dialogue | 37 +++++++++++++ .../api/TestServiceAsync.java.dialogue.prefix | 37 +++++++++++++ .../api/TestServiceBlocking.java.dialogue | 18 ++++++ .../TestServiceBlocking.java.dialogue.prefix | 18 ++++++ .../api/TestServiceEndpoints.java.undertow | 55 +++++++++++++++++++ .../TestServiceEndpoints.java.undertow.prefix | 55 +++++++++++++++++++ 11 files changed, 299 insertions(+), 1 deletion(-) diff --git a/conjure-java-core/src/test/java/com/palantir/conjure/java/TestBase.java b/conjure-java-core/src/test/java/com/palantir/conjure/java/TestBase.java index d14a49a02..05ec42610 100644 --- a/conjure-java-core/src/test/java/com/palantir/conjure/java/TestBase.java +++ b/conjure-java-core/src/test/java/com/palantir/conjure/java/TestBase.java @@ -56,7 +56,7 @@ protected static void validateGeneratorOutput(List files, Path outputDir, Files.deleteIfExists(output); Files.copy(file, output); } - assertThat(readFromFile(file)).isEqualTo(readFromFile(output)); + assertThat(readFromFile(file)).describedAs(output.toString()).isEqualTo(readFromFile(output)); } } } diff --git a/conjure-java-core/src/test/resources/test/api/DialogueTestEndpoints.java.dialogue b/conjure-java-core/src/test/resources/test/api/DialogueTestEndpoints.java.dialogue index 510d9e191..4fd8b9405 100644 --- a/conjure-java-core/src/test/resources/test/api/DialogueTestEndpoints.java.dialogue +++ b/conjure-java-core/src/test/resources/test/api/DialogueTestEndpoints.java.dialogue @@ -515,6 +515,38 @@ enum DialogueTestEndpoints implements Endpoint { } }, + testOptionalHeaderParam { + private final PathTemplate pathTemplate = PathTemplate.builder() + .fixed("catalog") + .fixed("test-optional-header-param") + .build(); + + @Override + public void renderPath(Map params, UrlBuilder url) { + pathTemplate.fill(params, url); + } + + @Override + public HttpMethod httpMethod() { + return HttpMethod.POST; + } + + @Override + public String serviceName() { + return "TestService"; + } + + @Override + public String endpointName() { + return "testOptionalHeaderParam"; + } + + @Override + public String version() { + return VERSION; + } + }, + testBoolean { private final PathTemplate pathTemplate = PathTemplate.builder().fixed("catalog").fixed("boolean").build(); diff --git a/conjure-java-core/src/test/resources/test/api/DialogueTestEndpoints.java.dialogue.prefix b/conjure-java-core/src/test/resources/test/api/DialogueTestEndpoints.java.dialogue.prefix index 24c39c379..fb02414c4 100644 --- a/conjure-java-core/src/test/resources/test/api/DialogueTestEndpoints.java.dialogue.prefix +++ b/conjure-java-core/src/test/resources/test/api/DialogueTestEndpoints.java.dialogue.prefix @@ -515,6 +515,38 @@ enum DialogueTestEndpoints implements Endpoint { } }, + testOptionalHeaderParam { + private final PathTemplate pathTemplate = PathTemplate.builder() + .fixed("catalog") + .fixed("test-optional-header-param") + .build(); + + @Override + public void renderPath(Map params, UrlBuilder url) { + pathTemplate.fill(params, url); + } + + @Override + public HttpMethod httpMethod() { + return HttpMethod.POST; + } + + @Override + public String serviceName() { + return "TestService"; + } + + @Override + public String endpointName() { + return "testOptionalHeaderParam"; + } + + @Override + public String version() { + return VERSION; + } + }, + testBoolean { private final PathTemplate pathTemplate = PathTemplate.builder().fixed("catalog").fixed("boolean").build(); diff --git a/conjure-java-core/src/test/resources/test/api/TestService.java.undertow b/conjure-java-core/src/test/resources/test/api/TestService.java.undertow index 6c26c3718..2ad0fcdc4 100644 --- a/conjure-java-core/src/test/resources/test/api/TestService.java.undertow +++ b/conjure-java-core/src/test/resources/test/api/TestService.java.undertow @@ -77,6 +77,13 @@ public interface TestService { Optional optionalEnd, String query); + void testOptionalHeaderParam( + AuthHeader authHeader, + Optional maybeHeader, + ResourceIdentifier implicit, + Optional maybeQuery, + String query); + boolean testBoolean(AuthHeader authHeader); double testDouble(AuthHeader authHeader); diff --git a/conjure-java-core/src/test/resources/test/api/TestService.java.undertow.prefix b/conjure-java-core/src/test/resources/test/api/TestService.java.undertow.prefix index 2cd601ad4..8acebe433 100644 --- a/conjure-java-core/src/test/resources/test/api/TestService.java.undertow.prefix +++ b/conjure-java-core/src/test/resources/test/api/TestService.java.undertow.prefix @@ -77,6 +77,13 @@ public interface TestService { Optional optionalEnd, String query); + void testOptionalHeaderParam( + AuthHeader authHeader, + Optional maybeHeader, + ResourceIdentifier implicit, + Optional maybeQuery, + String query); + boolean testBoolean(AuthHeader authHeader); double testDouble(AuthHeader authHeader); diff --git a/conjure-java-core/src/test/resources/test/api/TestServiceAsync.java.dialogue b/conjure-java-core/src/test/resources/test/api/TestServiceAsync.java.dialogue index 10713b8fc..eab203740 100644 --- a/conjure-java-core/src/test/resources/test/api/TestServiceAsync.java.dialogue +++ b/conjure-java-core/src/test/resources/test/api/TestServiceAsync.java.dialogue @@ -95,6 +95,13 @@ public interface TestServiceAsync { Optional optionalEnd, String query); + ListenableFuture testOptionalHeaderParam( + AuthHeader authHeader, + Optional maybeHeader, + ResourceIdentifier implicit, + Optional maybeQuery, + String query); + ListenableFuture testBoolean(AuthHeader authHeader); ListenableFuture testDouble(AuthHeader authHeader); @@ -209,6 +216,15 @@ public interface TestServiceAsync { private final Deserializer testNoResponseQueryParamsDeserializer = _runtime.bodySerDe().emptyBodyDeserializer(); + private final Serializer testOptionalHeaderParamSerializer = + _runtime.bodySerDe().serializer(new TypeMarker() {}); + + private final EndpointChannel testOptionalHeaderParamChannel = + _runtime.clients().bind(_channel, DialogueTestEndpoints.testOptionalHeaderParam); + + private final Deserializer testOptionalHeaderParamDeserializer = + _runtime.bodySerDe().emptyBodyDeserializer(); + private final EndpointChannel testBooleanChannel = _runtime.clients().bind(_channel, DialogueTestEndpoints.testBoolean); @@ -430,6 +446,27 @@ public interface TestServiceAsync { testNoResponseQueryParamsDeserializer); } + @Override + public ListenableFuture testOptionalHeaderParam( + AuthHeader authHeader, + Optional maybeHeader, + ResourceIdentifier implicit, + Optional maybeQuery, + String query) { + Request.Builder _request = Request.builder(); + _request.putHeaderParams("Authorization", authHeader.toString()); + _request.body(testOptionalHeaderParamSerializer.serialize(query)); + if (maybeHeader.isPresent()) { + _request.putHeaderParams("MaybeHeader", _plainSerDe.serializeString(maybeHeader.get())); + } + _request.putQueryParams("implicit", _plainSerDe.serializeRid(implicit)); + if (maybeQuery.isPresent()) { + _request.putQueryParams("maybeQuery", _plainSerDe.serializeRid(maybeQuery.get())); + } + return _runtime.clients() + .call(testOptionalHeaderParamChannel, _request.build(), testOptionalHeaderParamDeserializer); + } + @Override public ListenableFuture testBoolean(AuthHeader authHeader) { Request.Builder _request = Request.builder(); diff --git a/conjure-java-core/src/test/resources/test/api/TestServiceAsync.java.dialogue.prefix b/conjure-java-core/src/test/resources/test/api/TestServiceAsync.java.dialogue.prefix index bf747cba4..032104e9f 100644 --- a/conjure-java-core/src/test/resources/test/api/TestServiceAsync.java.dialogue.prefix +++ b/conjure-java-core/src/test/resources/test/api/TestServiceAsync.java.dialogue.prefix @@ -95,6 +95,13 @@ public interface TestServiceAsync { Optional optionalEnd, String query); + ListenableFuture testOptionalHeaderParam( + AuthHeader authHeader, + Optional maybeHeader, + ResourceIdentifier implicit, + Optional maybeQuery, + String query); + ListenableFuture testBoolean(AuthHeader authHeader); ListenableFuture testDouble(AuthHeader authHeader); @@ -209,6 +216,15 @@ public interface TestServiceAsync { private final Deserializer testNoResponseQueryParamsDeserializer = _runtime.bodySerDe().emptyBodyDeserializer(); + private final Serializer testOptionalHeaderParamSerializer = + _runtime.bodySerDe().serializer(new TypeMarker() {}); + + private final EndpointChannel testOptionalHeaderParamChannel = + _runtime.clients().bind(_channel, DialogueTestEndpoints.testOptionalHeaderParam); + + private final Deserializer testOptionalHeaderParamDeserializer = + _runtime.bodySerDe().emptyBodyDeserializer(); + private final EndpointChannel testBooleanChannel = _runtime.clients().bind(_channel, DialogueTestEndpoints.testBoolean); @@ -430,6 +446,27 @@ public interface TestServiceAsync { testNoResponseQueryParamsDeserializer); } + @Override + public ListenableFuture testOptionalHeaderParam( + AuthHeader authHeader, + Optional maybeHeader, + ResourceIdentifier implicit, + Optional maybeQuery, + String query) { + Request.Builder _request = Request.builder(); + _request.putHeaderParams("Authorization", authHeader.toString()); + _request.body(testOptionalHeaderParamSerializer.serialize(query)); + if (maybeHeader.isPresent()) { + _request.putHeaderParams("MaybeHeader", _plainSerDe.serializeString(maybeHeader.get())); + } + _request.putQueryParams("implicit", _plainSerDe.serializeRid(implicit)); + if (maybeQuery.isPresent()) { + _request.putQueryParams("maybeQuery", _plainSerDe.serializeRid(maybeQuery.get())); + } + return _runtime.clients() + .call(testOptionalHeaderParamChannel, _request.build(), testOptionalHeaderParamDeserializer); + } + @Override public ListenableFuture testBoolean(AuthHeader authHeader) { Request.Builder _request = Request.builder(); diff --git a/conjure-java-core/src/test/resources/test/api/TestServiceBlocking.java.dialogue b/conjure-java-core/src/test/resources/test/api/TestServiceBlocking.java.dialogue index b4bcc3849..5c03be842 100644 --- a/conjure-java-core/src/test/resources/test/api/TestServiceBlocking.java.dialogue +++ b/conjure-java-core/src/test/resources/test/api/TestServiceBlocking.java.dialogue @@ -85,6 +85,13 @@ public interface TestServiceBlocking { Optional optionalEnd, String query); + void testOptionalHeaderParam( + AuthHeader authHeader, + Optional maybeHeader, + ResourceIdentifier implicit, + Optional maybeQuery, + String query); + boolean testBoolean(AuthHeader authHeader); double testDouble(AuthHeader authHeader); @@ -199,6 +206,17 @@ public interface TestServiceBlocking { authHeader, something, implicit, optionalMiddle, setEnd, optionalEnd, query)); } + @Override + public void testOptionalHeaderParam( + AuthHeader authHeader, + Optional maybeHeader, + ResourceIdentifier implicit, + Optional maybeQuery, + String query) { + _runtime.clients() + .block(delegate.testOptionalHeaderParam(authHeader, maybeHeader, implicit, maybeQuery, query)); + } + @Override public boolean testBoolean(AuthHeader authHeader) { return _runtime.clients().block(delegate.testBoolean(authHeader)); diff --git a/conjure-java-core/src/test/resources/test/api/TestServiceBlocking.java.dialogue.prefix b/conjure-java-core/src/test/resources/test/api/TestServiceBlocking.java.dialogue.prefix index 9365469e1..964bc4938 100644 --- a/conjure-java-core/src/test/resources/test/api/TestServiceBlocking.java.dialogue.prefix +++ b/conjure-java-core/src/test/resources/test/api/TestServiceBlocking.java.dialogue.prefix @@ -85,6 +85,13 @@ public interface TestServiceBlocking { Optional optionalEnd, String query); + void testOptionalHeaderParam( + AuthHeader authHeader, + Optional maybeHeader, + ResourceIdentifier implicit, + Optional maybeQuery, + String query); + boolean testBoolean(AuthHeader authHeader); double testDouble(AuthHeader authHeader); @@ -199,6 +206,17 @@ public interface TestServiceBlocking { authHeader, something, implicit, optionalMiddle, setEnd, optionalEnd, query)); } + @Override + public void testOptionalHeaderParam( + AuthHeader authHeader, + Optional maybeHeader, + ResourceIdentifier implicit, + Optional maybeQuery, + String query) { + _runtime.clients() + .block(delegate.testOptionalHeaderParam(authHeader, maybeHeader, implicit, maybeQuery, query)); + } + @Override public boolean testBoolean(AuthHeader authHeader) { return _runtime.clients().block(delegate.testBoolean(authHeader)); 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 91b04eda9..9b272a28a 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 @@ -64,6 +64,7 @@ public final class TestServiceEndpoints implements UndertowService { new TestParamEndpoint(runtime, delegate), new TestQueryParamsEndpoint(runtime, delegate), new TestNoResponseQueryParamsEndpoint(runtime, delegate), + new TestOptionalHeaderParamEndpoint(runtime, delegate), new TestBooleanEndpoint(runtime, delegate), new TestDoubleEndpoint(runtime, delegate), new TestIntegerEndpoint(runtime, delegate), @@ -847,6 +848,60 @@ public final class TestServiceEndpoints implements UndertowService { } } + private static final class TestOptionalHeaderParamEndpoint implements HttpHandler, Endpoint { + private final UndertowRuntime runtime; + + private final TestService delegate; + + private final Deserializer deserializer; + + TestOptionalHeaderParamEndpoint(UndertowRuntime runtime, TestService delegate) { + this.runtime = runtime; + this.delegate = delegate; + this.deserializer = runtime.bodySerDe().deserializer(new TypeMarker() {}); + } + + @Override + public void handleRequest(HttpServerExchange exchange) throws IOException { + AuthHeader authHeader = runtime.auth().header(exchange); + String query = deserializer.deserialize(exchange); + HeaderMap headerParams = exchange.getRequestHeaders(); + Optional maybeHeader = + runtime.plainSerDe().deserializeOptionalString(headerParams.get("MaybeHeader")); + Map> queryParams = exchange.getQueryParameters(); + ResourceIdentifier implicit = runtime.plainSerDe().deserializeRid(queryParams.get("implicit")); + Optional maybeQuery = + runtime.plainSerDe().deserializeOptionalRid(queryParams.get("maybeQuery")); + delegate.testOptionalHeaderParam(authHeader, maybeHeader, implicit, maybeQuery, query); + exchange.setStatusCode(StatusCodes.NO_CONTENT); + } + + @Override + public HttpString method() { + return Methods.POST; + } + + @Override + public String template() { + return "/catalog/test-optional-header-param"; + } + + @Override + public String serviceName() { + return "TestService"; + } + + @Override + public String name() { + return "testOptionalHeaderParam"; + } + + @Override + public HttpHandler handler() { + return this; + } + } + private static final class TestBooleanEndpoint implements HttpHandler, Endpoint { private final UndertowRuntime runtime; 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 fdf398b5b..4f28a5a14 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 @@ -64,6 +64,7 @@ public final class TestServiceEndpoints implements UndertowService { new TestParamEndpoint(runtime, delegate), new TestQueryParamsEndpoint(runtime, delegate), new TestNoResponseQueryParamsEndpoint(runtime, delegate), + new TestOptionalHeaderParamEndpoint(runtime, delegate), new TestBooleanEndpoint(runtime, delegate), new TestDoubleEndpoint(runtime, delegate), new TestIntegerEndpoint(runtime, delegate), @@ -847,6 +848,60 @@ public final class TestServiceEndpoints implements UndertowService { } } + private static final class TestOptionalHeaderParamEndpoint implements HttpHandler, Endpoint { + private final UndertowRuntime runtime; + + private final TestService delegate; + + private final Deserializer deserializer; + + TestOptionalHeaderParamEndpoint(UndertowRuntime runtime, TestService delegate) { + this.runtime = runtime; + this.delegate = delegate; + this.deserializer = runtime.bodySerDe().deserializer(new TypeMarker() {}); + } + + @Override + public void handleRequest(HttpServerExchange exchange) throws IOException { + AuthHeader authHeader = runtime.auth().header(exchange); + String query = deserializer.deserialize(exchange); + HeaderMap headerParams = exchange.getRequestHeaders(); + Optional maybeHeader = + runtime.plainSerDe().deserializeOptionalString(headerParams.get("MaybeHeader")); + Map> queryParams = exchange.getQueryParameters(); + ResourceIdentifier implicit = runtime.plainSerDe().deserializeRid(queryParams.get("implicit")); + Optional maybeQuery = + runtime.plainSerDe().deserializeOptionalRid(queryParams.get("maybeQuery")); + delegate.testOptionalHeaderParam(authHeader, maybeHeader, implicit, maybeQuery, query); + exchange.setStatusCode(StatusCodes.NO_CONTENT); + } + + @Override + public HttpString method() { + return Methods.POST; + } + + @Override + public String template() { + return "/catalog/test-optional-header-param"; + } + + @Override + public String serviceName() { + return "TestService"; + } + + @Override + public String name() { + return "testOptionalHeaderParam"; + } + + @Override + public HttpHandler handler() { + return this; + } + } + private static final class TestBooleanEndpoint implements HttpHandler, Endpoint { private final UndertowRuntime runtime; From 3dc160029d92ecacd94e1e9d5ab8689c93a181dc Mon Sep 17 00:00:00 2001 From: Glenn Sheasby Date: Mon, 22 Jun 2020 14:32:54 +0100 Subject: [PATCH 4/6] Jersey: add compatibility backfill for optional headers --- .../java/services/JerseyServiceGenerator.java | 19 +++++++++++--- .../test/api/TestService.java.jersey | 26 +++++++++++++++++++ .../test/api/TestService.java.jersey.prefix | 26 +++++++++++++++++++ .../TestService.java.jersey_require_not_null | 26 +++++++++++++++++++ 4 files changed, 94 insertions(+), 3 deletions(-) diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/services/JerseyServiceGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/services/JerseyServiceGenerator.java index 2553d6593..507b80e67 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/services/JerseyServiceGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/services/JerseyServiceGenerator.java @@ -186,9 +186,14 @@ private MethodSpec generateServiceMethod( /** Provides a linear expansion of optional query arguments to improve Java back-compat. */ private List generateCompatibilityBackfillServiceMethods( EndpointDefinition endpointDef, TypeMapper returnTypeMapper, TypeMapper argumentTypeMapper) { + List headerArgs = new ArrayList<>(); List queryArgs = new ArrayList<>(); for (ArgumentDefinition arg : endpointDef.getArgs()) { + if (arg.getParamType().accept(ParameterTypeVisitor.IS_HEADER) + && arg.getType().accept(DefaultableTypeVisitor.INSTANCE)) { + headerArgs.add(arg); + } if (arg.getParamType().accept(ParameterTypeVisitor.IS_QUERY) && arg.getType().accept(DefaultableTypeVisitor.INSTANCE)) { queryArgs.add(arg); @@ -196,9 +201,17 @@ private List generateCompatibilityBackfillServiceMethods( } List alternateMethods = new ArrayList<>(); - for (int i = 0; i < queryArgs.size(); i++) { - alternateMethods.add(createCompatibilityBackfillMethod( - endpointDef, returnTypeMapper, argumentTypeMapper, queryArgs.subList(i, queryArgs.size()))); + for (int j = 0; j <= headerArgs.size(); j++) { + for (int i = 0; i <= queryArgs.size(); i++) { + List extraHeaderArgs = headerArgs.subList(j, headerArgs.size()); + List extraQueryArgs = queryArgs.subList(i, queryArgs.size()); + List extraArgs = new ArrayList<>(extraHeaderArgs); + extraArgs.addAll(extraQueryArgs); + if (!extraArgs.isEmpty()) { + alternateMethods.add(createCompatibilityBackfillMethod( + endpointDef, returnTypeMapper, argumentTypeMapper, extraArgs)); + } + } } return alternateMethods; diff --git a/conjure-java-core/src/test/resources/test/api/TestService.java.jersey b/conjure-java-core/src/test/resources/test/api/TestService.java.jersey index 5966b3eaf..13ce4f56e 100644 --- a/conjure-java-core/src/test/resources/test/api/TestService.java.jersey +++ b/conjure-java-core/src/test/resources/test/api/TestService.java.jersey @@ -150,6 +150,15 @@ public interface TestService { @QueryParam("optionalEnd") Optional optionalEnd, @NotNull String query); + @POST + @Path("catalog/test-optional-header-param") + void testOptionalHeaderParam( + @HeaderParam("Authorization") @NotNull AuthHeader authHeader, + @HeaderParam("MaybeHeader") Optional maybeHeader, + @QueryParam("implicit") ResourceIdentifier implicit, + @QueryParam("maybeQuery") Optional maybeQuery, + @NotNull String query); + @GET @Path("catalog/boolean") boolean testBoolean(@HeaderParam("Authorization") @NotNull AuthHeader authHeader); @@ -239,6 +248,23 @@ public interface TestService { testNoResponseQueryParams(authHeader, something, implicit, optionalMiddle, setEnd, Optional.empty(), query); } + @Deprecated + default void testOptionalHeaderParam(AuthHeader authHeader, ResourceIdentifier implicit, String query) { + testOptionalHeaderParam(authHeader, Optional.empty(), implicit, Optional.empty(), query); + } + + @Deprecated + default void testOptionalHeaderParam( + AuthHeader authHeader, ResourceIdentifier implicit, Optional maybeQuery, String query) { + testOptionalHeaderParam(authHeader, Optional.empty(), implicit, maybeQuery, query); + } + + @Deprecated + default void testOptionalHeaderParam( + AuthHeader authHeader, Optional maybeHeader, ResourceIdentifier implicit, String query) { + testOptionalHeaderParam(authHeader, maybeHeader, implicit, Optional.empty(), query); + } + @Deprecated default void testOptionalIntegerAndDouble(AuthHeader authHeader) { testOptionalIntegerAndDouble(authHeader, OptionalInt.empty(), OptionalDouble.empty()); diff --git a/conjure-java-core/src/test/resources/test/api/TestService.java.jersey.prefix b/conjure-java-core/src/test/resources/test/api/TestService.java.jersey.prefix index 9725d0f00..04e8e3be1 100644 --- a/conjure-java-core/src/test/resources/test/api/TestService.java.jersey.prefix +++ b/conjure-java-core/src/test/resources/test/api/TestService.java.jersey.prefix @@ -148,6 +148,15 @@ public interface TestService { @QueryParam("optionalEnd") Optional optionalEnd, String query); + @POST + @Path("catalog/test-optional-header-param") + void testOptionalHeaderParam( + @HeaderParam("Authorization") AuthHeader authHeader, + @HeaderParam("MaybeHeader") Optional maybeHeader, + @QueryParam("implicit") ResourceIdentifier implicit, + @QueryParam("maybeQuery") Optional maybeQuery, + String query); + @GET @Path("catalog/boolean") boolean testBoolean(@HeaderParam("Authorization") AuthHeader authHeader); @@ -237,6 +246,23 @@ public interface TestService { testNoResponseQueryParams(authHeader, something, implicit, optionalMiddle, setEnd, Optional.empty(), query); } + @Deprecated + default void testOptionalHeaderParam(AuthHeader authHeader, ResourceIdentifier implicit, String query) { + testOptionalHeaderParam(authHeader, Optional.empty(), implicit, Optional.empty(), query); + } + + @Deprecated + default void testOptionalHeaderParam( + AuthHeader authHeader, ResourceIdentifier implicit, Optional maybeQuery, String query) { + testOptionalHeaderParam(authHeader, Optional.empty(), implicit, maybeQuery, query); + } + + @Deprecated + default void testOptionalHeaderParam( + AuthHeader authHeader, Optional maybeHeader, ResourceIdentifier implicit, String query) { + testOptionalHeaderParam(authHeader, maybeHeader, implicit, Optional.empty(), query); + } + @Deprecated default void testOptionalIntegerAndDouble(AuthHeader authHeader) { testOptionalIntegerAndDouble(authHeader, OptionalInt.empty(), OptionalDouble.empty()); diff --git a/conjure-java-core/src/test/resources/test/api/TestService.java.jersey_require_not_null b/conjure-java-core/src/test/resources/test/api/TestService.java.jersey_require_not_null index 5966b3eaf..13ce4f56e 100644 --- a/conjure-java-core/src/test/resources/test/api/TestService.java.jersey_require_not_null +++ b/conjure-java-core/src/test/resources/test/api/TestService.java.jersey_require_not_null @@ -150,6 +150,15 @@ public interface TestService { @QueryParam("optionalEnd") Optional optionalEnd, @NotNull String query); + @POST + @Path("catalog/test-optional-header-param") + void testOptionalHeaderParam( + @HeaderParam("Authorization") @NotNull AuthHeader authHeader, + @HeaderParam("MaybeHeader") Optional maybeHeader, + @QueryParam("implicit") ResourceIdentifier implicit, + @QueryParam("maybeQuery") Optional maybeQuery, + @NotNull String query); + @GET @Path("catalog/boolean") boolean testBoolean(@HeaderParam("Authorization") @NotNull AuthHeader authHeader); @@ -239,6 +248,23 @@ public interface TestService { testNoResponseQueryParams(authHeader, something, implicit, optionalMiddle, setEnd, Optional.empty(), query); } + @Deprecated + default void testOptionalHeaderParam(AuthHeader authHeader, ResourceIdentifier implicit, String query) { + testOptionalHeaderParam(authHeader, Optional.empty(), implicit, Optional.empty(), query); + } + + @Deprecated + default void testOptionalHeaderParam( + AuthHeader authHeader, ResourceIdentifier implicit, Optional maybeQuery, String query) { + testOptionalHeaderParam(authHeader, Optional.empty(), implicit, maybeQuery, query); + } + + @Deprecated + default void testOptionalHeaderParam( + AuthHeader authHeader, Optional maybeHeader, ResourceIdentifier implicit, String query) { + testOptionalHeaderParam(authHeader, maybeHeader, implicit, Optional.empty(), query); + } + @Deprecated default void testOptionalIntegerAndDouble(AuthHeader authHeader) { testOptionalIntegerAndDouble(authHeader, OptionalInt.empty(), OptionalDouble.empty()); From 0a1240b44d311fb70c8c908d76492a522d2ced94 Mon Sep 17 00:00:00 2001 From: Glenn Sheasby Date: Mon, 22 Jun 2020 13:32:54 +0000 Subject: [PATCH 5/6] Add generated changelog entries --- changelog/@unreleased/pr-967.v2.yml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 changelog/@unreleased/pr-967.v2.yml diff --git a/changelog/@unreleased/pr-967.v2.yml b/changelog/@unreleased/pr-967.v2.yml new file mode 100644 index 000000000..78933190b --- /dev/null +++ b/changelog/@unreleased/pr-967.v2.yml @@ -0,0 +1,7 @@ +type: improvement +improvement: + description: When optional header params are added, Retrofit and Jersey generated + code now includes deprecated methods without these headers - similar to those + generated when optional query params are added. + links: + - https://github.com/palantir/conjure-java/pull/967 From 7209bdeae477c5ef73c54e2747c220142beeb0dc Mon Sep 17 00:00:00 2001 From: Glenn Sheasby Date: Thu, 25 Jun 2020 14:48:23 +0100 Subject: [PATCH 6/6] Prevent generated signatures from clashing --- .../java/services/JerseyServiceGenerator.java | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/services/JerseyServiceGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/services/JerseyServiceGenerator.java index 507b80e67..f6dc85fe2 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/services/JerseyServiceGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/services/JerseyServiceGenerator.java @@ -200,6 +200,39 @@ private List generateCompatibilityBackfillServiceMethods( } } + if (!headerArgs.isEmpty()) { + // ensure the correct ordering of parameters by creating the complete sorted parameter list + List sortedParams = createServiceMethodParameters(endpointDef, argumentTypeMapper, false); + List sortedArgs = sortedParams.stream() + .map(param -> endpointDef.getArgs().stream() + .filter(arg -> arg.getArgName().get().equals(param.name)) + .findFirst()) + .filter(Optional::isPresent) + .map(Optional::get) + .collect(Collectors.toList()); + + Optional firstParameterAfterLastHeader = Optional.empty(); + for (int i = 0; i < sortedArgs.size(); i++) { + ArgumentDefinition arg = sortedArgs.get(i); + boolean isOptionalHeaderArg = arg.getParamType().accept(ParameterTypeVisitor.IS_HEADER) + && arg.getType().accept(DefaultableTypeVisitor.INSTANCE); + if (isOptionalHeaderArg) { + if (i < (sortedArgs.size() - 1)) { + firstParameterAfterLastHeader = Optional.of(sortedArgs.get(i + 1)); + } else { + firstParameterAfterLastHeader = Optional.empty(); + } + } + } + + if (!firstParameterAfterLastHeader.isPresent() + || firstParameterAfterLastHeader.get().getType().accept(DefaultableTypeVisitor.INSTANCE)) { + // The last optional header is not followed by a non-optional parameter, so we can't guarantee + // uniqueness of signatures if we generate backfill methods for headers. + headerArgs = ImmutableList.of(); + } + } + List alternateMethods = new ArrayList<>(); for (int j = 0; j <= headerArgs.size(); j++) { for (int i = 0; i <= queryArgs.size(); i++) {