From 23c0e6cebeffc698f8ee03b8079db5c7798a68cc Mon Sep 17 00:00:00 2001 From: Patrick Koenig Date: Tue, 4 Dec 2018 10:52:32 -0800 Subject: [PATCH] Apply correct header/streaming annotations to retrofit clients --- .../services/Retrofit2ServiceGenerator.java | 38 +++++++++---------- .../src/test/resources/example-service.yml | 7 ++++ .../test/api/TestService.java.jersey | 7 ++++ .../TestService.java.jersey_require_not_null | 7 ++++ .../api/TestServiceRetrofit.java.retrofit | 18 +++++++-- ...eRetrofit.java.retrofit_completable_future | 18 +++++++-- ...ceRetrofit.java.retrofit_listenable_future | 18 +++++++-- 7 files changed, 81 insertions(+), 32 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 bbb1ffd41..f9893ad4d 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 @@ -47,6 +47,7 @@ import com.squareup.javapoet.MethodSpec; import com.squareup.javapoet.ParameterSpec; import com.squareup.javapoet.ParameterizedTypeName; +import com.squareup.javapoet.TypeName; import com.squareup.javapoet.TypeSpec; import java.util.List; import java.util.Optional; @@ -68,8 +69,7 @@ public final class Retrofit2ServiceGenerator implements ServiceGenerator { private static final ClassName CALL_TYPE = ClassName.get("retrofit2", "Call"); private static final String AUTH_HEADER_NAME = "Authorization"; - private static final String JSON_MEDIA_TYPE = MediaType.APPLICATION_JSON; - private static final String OCTET_STREAM_MEDIA_TYPE = MediaType.APPLICATION_OCTET_STREAM; + private static final ClassName BINARY_RETURN_TYPE = ClassName.get("okhttp3", "ResponseBody"); private static final Logger log = LoggerFactory.getLogger(Retrofit2ServiceGenerator.class); @@ -131,25 +131,23 @@ private MethodSpec generateServiceMethod( EndpointDefinition endpointDef, TypeMapper returnTypeMapper, TypeMapper methodTypeMapper) { + TypeName returnType = endpointDef.getReturns() + .map(returnTypeMapper::getClassName) + .orElse(ClassName.VOID); + Set encodedPathArgs = extractEncodedPathArgs(endpointDef.getHttpPath()); HttpPath endpointPathWithoutRegex = replaceEncodedPathArgs(endpointDef.getHttpPath()); MethodSpec.Builder methodBuilder = MethodSpec.methodBuilder(endpointDef.getEndpointName().get()) .addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT) .addAnnotation(AnnotationSpec.builder(httpMethodToClassName(endpointDef.getHttpMethod().get().name())) .addMember("value", "$S", "." + endpointPathWithoutRegex) + .build()) + .addAnnotation(AnnotationSpec.builder(ClassName.get("retrofit2.http", "Headers")) + .addMember("value", "$S", "hr-path-template: " + endpointPathWithoutRegex) + .addMember("value", "$S", "Accept: " + getReturnMediaType(returnType)) .build()); - AnnotationSpec.Builder headersBuilder = AnnotationSpec.builder(ClassName.get("retrofit2.http", "Headers")) - .addMember("value", "$S", "hr-path-template: " + endpointPathWithoutRegex); - endpointDef.getReturns().ifPresent(type -> { - String mediaType = type.accept(TypeVisitor.IS_BINARY) - ? MediaType.APPLICATION_OCTET_STREAM - : MediaType.APPLICATION_JSON; - headersBuilder.addMember("value", "$S", "Accept: " + mediaType); - }); - methodBuilder.addAnnotation(headersBuilder.build()); - - if (endpointDef.getReturns().map(type -> type.accept(TypeVisitor.IS_BINARY)).orElse(false)) { + if (returnType.equals(BINARY_RETURN_TYPE)) { methodBuilder.addAnnotation(AnnotationSpec.builder(ClassName.get("retrofit2.http", "Streaming")).build()); } @@ -159,13 +157,7 @@ private MethodSpec generateServiceMethod( ServiceGenerator.getJavaDoc(endpointDef).ifPresent( content -> methodBuilder.addJavadoc("$L", content)); - if (endpointDef.getReturns().isPresent()) { - methodBuilder.returns( - ParameterizedTypeName.get(getReturnType(), - returnTypeMapper.getClassName(endpointDef.getReturns().get()).box())); - } else { - methodBuilder.returns(ParameterizedTypeName.get(getReturnType(), ClassName.get(Void.class))); - } + methodBuilder.returns(ParameterizedTypeName.get(getReturnType(), returnType.box())); getAuthParameter(endpointDef.getAuth()).ifPresent(methodBuilder::addParameter); @@ -260,6 +252,12 @@ private Optional getAuthParameter(Optional auth) { throw new IllegalStateException("Unrecognized auth type: " + auth.get()); } + private static String getReturnMediaType(TypeName returnType) { + return returnType.equals(BINARY_RETURN_TYPE) + ? MediaType.APPLICATION_OCTET_STREAM + : MediaType.APPLICATION_JSON; + } + private static Set createMarkers(TypeMapper typeMapper, List markers) { checkArgument(markers.stream().allMatch(type -> type.accept(TypeVisitor.IS_REFERENCE)), "Markers must refer to reference types."); diff --git a/conjure-java-core/src/test/resources/example-service.yml b/conjure-java-core/src/test/resources/example-service.yml index 0f33d769c..ce92f9b0a 100644 --- a/conjure-java-core/src/test/resources/example-service.yml +++ b/conjure-java-core/src/test/resources/example-service.yml @@ -122,6 +122,13 @@ services: type: binary param-type: body + uploadAliasedRawData: + http: POST /datasets/upload-raw-aliased + args: + input: + type: NestedAliasedBinary + param-type: body + getBranches: http: GET /datasets/{datasetRid}/branches args: 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 f4205c361..d21fbd453 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 @@ -2,6 +2,7 @@ package com.palantir.another; import com.palantir.product.AliasedString; import com.palantir.product.CreateDatasetRequest; +import com.palantir.product.NestedAliasedBinary; import com.palantir.product.datasets.BackingFileSystem; import com.palantir.product.datasets.Dataset; import com.palantir.redaction.Safe; @@ -87,6 +88,12 @@ public interface TestService { @HeaderParam("Authorization") @NotNull AuthHeader authHeader, @NotNull InputStream input); + @POST + @Path("catalog/datasets/upload-raw-aliased") + void uploadAliasedRawData( + @HeaderParam("Authorization") @NotNull AuthHeader authHeader, + @NotNull NestedAliasedBinary input); + @GET @Path("catalog/datasets/{datasetRid}/branches") Set getBranches( 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 f4205c361..d21fbd453 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 @@ -2,6 +2,7 @@ package com.palantir.another; import com.palantir.product.AliasedString; import com.palantir.product.CreateDatasetRequest; +import com.palantir.product.NestedAliasedBinary; import com.palantir.product.datasets.BackingFileSystem; import com.palantir.product.datasets.Dataset; import com.palantir.redaction.Safe; @@ -87,6 +88,12 @@ public interface TestService { @HeaderParam("Authorization") @NotNull AuthHeader authHeader, @NotNull InputStream input); + @POST + @Path("catalog/datasets/upload-raw-aliased") + void uploadAliasedRawData( + @HeaderParam("Authorization") @NotNull AuthHeader authHeader, + @NotNull NestedAliasedBinary input); + @GET @Path("catalog/datasets/{datasetRid}/branches") Set getBranches( 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 688851a2d..ddc07038a 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 @@ -2,6 +2,7 @@ package com.palantir.another; import com.palantir.product.AliasedString; import com.palantir.product.CreateDatasetRequest; +import com.palantir.product.NestedAliasedBinary; import com.palantir.product.datasets.BackingFileSystem; import com.palantir.product.datasets.Dataset; import com.palantir.ri.ResourceIdentifier; @@ -66,8 +67,9 @@ public interface TestServiceRetrofit { @GET("./catalog/datasets/{datasetRid}/raw-aliased") @Headers({ "hr-path-template: /catalog/datasets/{datasetRid}/raw-aliased", - "Accept: application/json" + "Accept: application/octet-stream" }) + @Streaming Call getAliasedRawData( @Header("Authorization") AuthHeader authHeader, @Path("datasetRid") ResourceIdentifier datasetRid); @@ -91,10 +93,15 @@ public interface TestServiceRetrofit { @Path("datasetRid") ResourceIdentifier datasetRid); @POST("./catalog/datasets/upload-raw") - @Headers("hr-path-template: /catalog/datasets/upload-raw") + @Headers({"hr-path-template: /catalog/datasets/upload-raw", "Accept: application/json"}) Call uploadRawData( @Header("Authorization") AuthHeader authHeader, @Body RequestBody input); + @POST("./catalog/datasets/upload-raw-aliased") + @Headers({"hr-path-template: /catalog/datasets/upload-raw-aliased", "Accept: application/json"}) + Call uploadAliasedRawData( + @Header("Authorization") AuthHeader authHeader, @Body NestedAliasedBinary input); + @GET("./catalog/datasets/{datasetRid}/branches") @Headers({ "hr-path-template: /catalog/datasets/{datasetRid}/branches", @@ -150,7 +157,10 @@ public interface TestServiceRetrofit { @Query("optionalEnd") Optional optionalEnd); @POST("./catalog/test-no-response-query-params") - @Headers("hr-path-template: /catalog/test-no-response-query-params") + @Headers({ + "hr-path-template: /catalog/test-no-response-query-params", + "Accept: application/json" + }) Call testNoResponseQueryParams( @Header("Authorization") AuthHeader authHeader, @Body String query, @@ -178,7 +188,7 @@ public interface TestServiceRetrofit { @Header("Authorization") AuthHeader authHeader, @Body Optional maybeString); @GET("./catalog/optional-integer-double") - @Headers("hr-path-template: /catalog/optional-integer-double") + @Headers({"hr-path-template: /catalog/optional-integer-double", "Accept: application/json"}) Call testOptionalIntegerAndDouble( @Header("Authorization") AuthHeader authHeader, @Query("maybeInteger") OptionalInt maybeInteger, diff --git a/conjure-java-core/src/test/resources/test/api/TestServiceRetrofit.java.retrofit_completable_future b/conjure-java-core/src/test/resources/test/api/TestServiceRetrofit.java.retrofit_completable_future index a3f071ea5..559c0723d 100644 --- a/conjure-java-core/src/test/resources/test/api/TestServiceRetrofit.java.retrofit_completable_future +++ b/conjure-java-core/src/test/resources/test/api/TestServiceRetrofit.java.retrofit_completable_future @@ -2,6 +2,7 @@ package com.palantir.another; import com.palantir.product.AliasedString; import com.palantir.product.CreateDatasetRequest; +import com.palantir.product.NestedAliasedBinary; import com.palantir.product.datasets.BackingFileSystem; import com.palantir.product.datasets.Dataset; import com.palantir.ri.ResourceIdentifier; @@ -66,8 +67,9 @@ public interface TestServiceRetrofit { @GET("./catalog/datasets/{datasetRid}/raw-aliased") @Headers({ "hr-path-template: /catalog/datasets/{datasetRid}/raw-aliased", - "Accept: application/json" + "Accept: application/octet-stream" }) + @Streaming CompletableFuture getAliasedRawData( @Header("Authorization") AuthHeader authHeader, @Path("datasetRid") ResourceIdentifier datasetRid); @@ -91,10 +93,15 @@ public interface TestServiceRetrofit { @Path("datasetRid") ResourceIdentifier datasetRid); @POST("./catalog/datasets/upload-raw") - @Headers("hr-path-template: /catalog/datasets/upload-raw") + @Headers({"hr-path-template: /catalog/datasets/upload-raw", "Accept: application/json"}) CompletableFuture uploadRawData( @Header("Authorization") AuthHeader authHeader, @Body RequestBody input); + @POST("./catalog/datasets/upload-raw-aliased") + @Headers({"hr-path-template: /catalog/datasets/upload-raw-aliased", "Accept: application/json"}) + CompletableFuture uploadAliasedRawData( + @Header("Authorization") AuthHeader authHeader, @Body NestedAliasedBinary input); + @GET("./catalog/datasets/{datasetRid}/branches") @Headers({ "hr-path-template: /catalog/datasets/{datasetRid}/branches", @@ -150,7 +157,10 @@ public interface TestServiceRetrofit { @Query("optionalEnd") Optional optionalEnd); @POST("./catalog/test-no-response-query-params") - @Headers("hr-path-template: /catalog/test-no-response-query-params") + @Headers({ + "hr-path-template: /catalog/test-no-response-query-params", + "Accept: application/json" + }) CompletableFuture testNoResponseQueryParams( @Header("Authorization") AuthHeader authHeader, @Body String query, @@ -178,7 +188,7 @@ public interface TestServiceRetrofit { @Header("Authorization") AuthHeader authHeader, @Body Optional maybeString); @GET("./catalog/optional-integer-double") - @Headers("hr-path-template: /catalog/optional-integer-double") + @Headers({"hr-path-template: /catalog/optional-integer-double", "Accept: application/json"}) CompletableFuture testOptionalIntegerAndDouble( @Header("Authorization") AuthHeader authHeader, @Query("maybeInteger") OptionalInt maybeInteger, diff --git a/conjure-java-core/src/test/resources/test/api/TestServiceRetrofit.java.retrofit_listenable_future b/conjure-java-core/src/test/resources/test/api/TestServiceRetrofit.java.retrofit_listenable_future index c2db14cca..98393fb4f 100644 --- a/conjure-java-core/src/test/resources/test/api/TestServiceRetrofit.java.retrofit_listenable_future +++ b/conjure-java-core/src/test/resources/test/api/TestServiceRetrofit.java.retrofit_listenable_future @@ -3,6 +3,7 @@ package com.palantir.another; import com.google.common.util.concurrent.ListenableFuture; import com.palantir.product.AliasedString; import com.palantir.product.CreateDatasetRequest; +import com.palantir.product.NestedAliasedBinary; import com.palantir.product.datasets.BackingFileSystem; import com.palantir.product.datasets.Dataset; import com.palantir.ri.ResourceIdentifier; @@ -66,8 +67,9 @@ public interface TestServiceRetrofit { @GET("./catalog/datasets/{datasetRid}/raw-aliased") @Headers({ "hr-path-template: /catalog/datasets/{datasetRid}/raw-aliased", - "Accept: application/json" + "Accept: application/octet-stream" }) + @Streaming ListenableFuture getAliasedRawData( @Header("Authorization") AuthHeader authHeader, @Path("datasetRid") ResourceIdentifier datasetRid); @@ -91,10 +93,15 @@ public interface TestServiceRetrofit { @Path("datasetRid") ResourceIdentifier datasetRid); @POST("./catalog/datasets/upload-raw") - @Headers("hr-path-template: /catalog/datasets/upload-raw") + @Headers({"hr-path-template: /catalog/datasets/upload-raw", "Accept: application/json"}) ListenableFuture uploadRawData( @Header("Authorization") AuthHeader authHeader, @Body RequestBody input); + @POST("./catalog/datasets/upload-raw-aliased") + @Headers({"hr-path-template: /catalog/datasets/upload-raw-aliased", "Accept: application/json"}) + ListenableFuture uploadAliasedRawData( + @Header("Authorization") AuthHeader authHeader, @Body NestedAliasedBinary input); + @GET("./catalog/datasets/{datasetRid}/branches") @Headers({ "hr-path-template: /catalog/datasets/{datasetRid}/branches", @@ -150,7 +157,10 @@ public interface TestServiceRetrofit { @Query("optionalEnd") Optional optionalEnd); @POST("./catalog/test-no-response-query-params") - @Headers("hr-path-template: /catalog/test-no-response-query-params") + @Headers({ + "hr-path-template: /catalog/test-no-response-query-params", + "Accept: application/json" + }) ListenableFuture testNoResponseQueryParams( @Header("Authorization") AuthHeader authHeader, @Body String query, @@ -178,7 +188,7 @@ public interface TestServiceRetrofit { @Header("Authorization") AuthHeader authHeader, @Body Optional maybeString); @GET("./catalog/optional-integer-double") - @Headers("hr-path-template: /catalog/optional-integer-double") + @Headers({"hr-path-template: /catalog/optional-integer-double", "Accept: application/json"}) ListenableFuture testOptionalIntegerAndDouble( @Header("Authorization") AuthHeader authHeader, @Query("maybeInteger") OptionalInt maybeInteger,