From c45c4d188f2fe41aaa236253039b10e95f4bd744 Mon Sep 17 00:00:00 2001 From: Patrick Koenig Date: Wed, 5 Dec 2018 14:23:01 -0800 Subject: [PATCH 1/5] Add Produces annotation for binary alias return types --- .../java/services/JerseyServiceGenerator.java | 37 ++++++-- .../services/Retrofit2ServiceGenerator.java | 15 +-- .../JerseyMethodTypeClassNameVisitor.java | 77 --------------- .../JerseyReturnTypeClassNameVisitor.java | 95 ------------------- ...r.java => MethodTypeClassNameVisitor.java} | 15 +-- ...r.java => ReturnTypeClassNameVisitor.java} | 37 ++++++-- .../conjure/java/types/TypeMapper.java | 3 +- .../java/util/BinaryReturnTypeResolver.java | 66 ------------- .../test/api/TestService.java.jersey | 1 + .../TestService.java.jersey_require_not_null | 1 + 10 files changed, 75 insertions(+), 272 deletions(-) delete mode 100644 conjure-java-core/src/main/java/com/palantir/conjure/java/types/JerseyMethodTypeClassNameVisitor.java delete mode 100644 conjure-java-core/src/main/java/com/palantir/conjure/java/types/JerseyReturnTypeClassNameVisitor.java rename conjure-java-core/src/main/java/com/palantir/conjure/java/types/{Retrofit2MethodTypeClassNameVisitor.java => MethodTypeClassNameVisitor.java} (82%) rename conjure-java-core/src/main/java/com/palantir/conjure/java/types/{Retrofit2ReturnTypeClassNameVisitor.java => ReturnTypeClassNameVisitor.java} (65%) delete mode 100644 conjure-java-core/src/main/java/com/palantir/conjure/java/util/BinaryReturnTypeResolver.java 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 0cf9a5e3e..9d9240f46 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 @@ -23,8 +23,8 @@ import com.google.common.collect.Lists; import com.palantir.conjure.java.ConjureAnnotations; import com.palantir.conjure.java.FeatureFlags; -import com.palantir.conjure.java.types.JerseyMethodTypeClassNameVisitor; -import com.palantir.conjure.java.types.JerseyReturnTypeClassNameVisitor; +import com.palantir.conjure.java.types.MethodTypeClassNameVisitor; +import com.palantir.conjure.java.types.ReturnTypeClassNameVisitor; import com.palantir.conjure.java.types.TypeMapper; import com.palantir.conjure.spec.ArgumentDefinition; import com.palantir.conjure.spec.AuthType; @@ -52,7 +52,9 @@ import com.squareup.javapoet.JavaFile; import com.squareup.javapoet.MethodSpec; import com.squareup.javapoet.ParameterSpec; +import com.squareup.javapoet.TypeName; import com.squareup.javapoet.TypeSpec; +import java.io.InputStream; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -65,11 +67,18 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; import javax.lang.model.element.Modifier; +import javax.ws.rs.core.Response; +import javax.ws.rs.core.StreamingOutput; import org.apache.commons.lang3.StringUtils; public final class JerseyServiceGenerator implements ServiceGenerator { + private static final ClassName NOT_NULL = ClassName.get("javax.validation.constraints", "NotNull"); + private static final ClassName BINARY_METHOD_TYPE = ClassName.get(InputStream.class); + private static final ClassName BINARY_RETURN_TYPE_RESPONSE = ClassName.get(Response.class); + private static final ClassName BINARY_RETURN_TYPE_OUTPUT = ClassName.get(StreamingOutput.class); + private final Set experimentalFeatures; public JerseyServiceGenerator() { @@ -82,11 +91,15 @@ public JerseyServiceGenerator(Set experimentalFeatures) { @Override public Set generate(ConjureDefinition conjureDefinition) { + ClassName binaryReturnType = experimentalFeatures.contains(FeatureFlags.JerseyBinaryAsResponse) + ? BINARY_RETURN_TYPE_RESPONSE + : BINARY_RETURN_TYPE_OUTPUT; TypeMapper returnTypeMapper = new TypeMapper( conjureDefinition.getTypes(), - types -> new JerseyReturnTypeClassNameVisitor(types, experimentalFeatures)); + ReturnTypeClassNameVisitor.createFactory(binaryReturnType)); TypeMapper methodTypeMapper = new TypeMapper( - conjureDefinition.getTypes(), JerseyMethodTypeClassNameVisitor::new); + conjureDefinition.getTypes(), + MethodTypeClassNameVisitor.createFactory(BINARY_METHOD_TYPE)); return conjureDefinition.getServices().stream() .map(serviceDef -> generateService(serviceDef, returnTypeMapper, methodTypeMapper)) .collect(Collectors.toSet()); @@ -129,10 +142,15 @@ private MethodSpec generateServiceMethod( EndpointDefinition endpointDef, TypeMapper returnTypeMapper, TypeMapper methodTypeMapper) { + TypeName returnType = endpointDef.getReturns() + .map(returnTypeMapper::getClassName) + .orElse(ClassName.VOID); + MethodSpec.Builder methodBuilder = MethodSpec.methodBuilder(endpointDef.getEndpointName().get()) .addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT) .addAnnotation(httpMethodToClassName(endpointDef.getHttpMethod().get().name())) - .addParameters(createServiceMethodParameters(endpointDef, methodTypeMapper, true)); + .addParameters(createServiceMethodParameters(endpointDef, methodTypeMapper, true)) + .returns(returnType); // @Path("") is invalid in Feign JaxRs and equivalent to absent on an endpoint method String rawHttpPath = endpointDef.getHttpPath().get(); @@ -143,15 +161,16 @@ private MethodSpec generateServiceMethod( .build()); } - if (endpointDef.getReturns().map(type -> type.accept(TypeVisitor.IS_BINARY)).orElse(false)) { + if (returnType.equals(BINARY_RETURN_TYPE_OUTPUT) || returnType.equals(BINARY_RETURN_TYPE_RESPONSE)) { methodBuilder.addAnnotation(AnnotationSpec.builder(ClassName.get("javax.ws.rs", "Produces")) .addMember("value", "$T.APPLICATION_OCTET_STREAM", ClassName.get("javax.ws.rs.core", "MediaType")) .build()); } boolean consumesTypeIsBinary = endpointDef.getArgs().stream() - .anyMatch(arg -> arg.getType().accept(TypeVisitor.IS_BINARY) - && arg.getParamType().accept(ParameterTypeVisitor.IS_BODY)); + .map(ArgumentDefinition::getType) + .map(methodTypeMapper::getClassName) + .anyMatch(BINARY_METHOD_TYPE::equals); if (consumesTypeIsBinary) { methodBuilder.addAnnotation(AnnotationSpec.builder(ClassName.get("javax.ws.rs", "Consumes")) @@ -165,8 +184,6 @@ private MethodSpec generateServiceMethod( ServiceGenerator.getJavaDoc(endpointDef).ifPresent( content -> methodBuilder.addJavadoc("$L", content)); - endpointDef.getReturns().ifPresent(type -> methodBuilder.returns(returnTypeMapper.getClassName(type))); - return methodBuilder.build(); } 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 f9893ad4d..1eca42b74 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 @@ -23,8 +23,8 @@ import com.google.common.collect.Lists; import com.palantir.conjure.java.ConjureAnnotations; import com.palantir.conjure.java.FeatureFlags; -import com.palantir.conjure.java.types.Retrofit2MethodTypeClassNameVisitor; -import com.palantir.conjure.java.types.Retrofit2ReturnTypeClassNameVisitor; +import com.palantir.conjure.java.types.MethodTypeClassNameVisitor; +import com.palantir.conjure.java.types.ReturnTypeClassNameVisitor; import com.palantir.conjure.java.types.TypeMapper; import com.palantir.conjure.spec.ArgumentDefinition; import com.palantir.conjure.spec.ArgumentName; @@ -69,6 +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 ClassName BINARY_METHOD_TYPE = ClassName.get("okhttp3", "RequestBody"); private static final ClassName BINARY_RETURN_TYPE = ClassName.get("okhttp3", "ResponseBody"); private static final Logger log = LoggerFactory.getLogger(Retrofit2ServiceGenerator.class); @@ -85,10 +86,12 @@ public Retrofit2ServiceGenerator(Set experimentalFeatures) { @Override public Set generate(ConjureDefinition conjureDefinition) { - TypeMapper returnTypeMapper = - new TypeMapper(conjureDefinition.getTypes(), Retrofit2ReturnTypeClassNameVisitor::new); - TypeMapper methodTypeMapper = - new TypeMapper(conjureDefinition.getTypes(), Retrofit2MethodTypeClassNameVisitor::new); + TypeMapper returnTypeMapper = new TypeMapper( + conjureDefinition.getTypes(), + ReturnTypeClassNameVisitor.createFactory(BINARY_RETURN_TYPE)); + TypeMapper methodTypeMapper = new TypeMapper( + conjureDefinition.getTypes(), + MethodTypeClassNameVisitor.createFactory(BINARY_METHOD_TYPE)); return conjureDefinition.getServices().stream() .map(serviceDef -> generateService(serviceDef, returnTypeMapper, methodTypeMapper)) .collect(Collectors.toSet()); diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/JerseyMethodTypeClassNameVisitor.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/JerseyMethodTypeClassNameVisitor.java deleted file mode 100644 index a033805ae..000000000 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/JerseyMethodTypeClassNameVisitor.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * (c) Copyright 2018 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.types; - -import com.palantir.conjure.spec.ExternalReference; -import com.palantir.conjure.spec.ListType; -import com.palantir.conjure.spec.MapType; -import com.palantir.conjure.spec.OptionalType; -import com.palantir.conjure.spec.PrimitiveType; -import com.palantir.conjure.spec.SetType; -import com.palantir.conjure.spec.TypeDefinition; -import com.squareup.javapoet.ClassName; -import com.squareup.javapoet.TypeName; -import java.io.InputStream; -import java.util.List; - -public final class JerseyMethodTypeClassNameVisitor implements ClassNameVisitor { - - private final DefaultClassNameVisitor delegate; - - public JerseyMethodTypeClassNameVisitor(List types) { - this.delegate = new DefaultClassNameVisitor(types); - } - - @Override - public TypeName visitList(ListType type) { - return delegate.visitList(type); - } - - @Override - public TypeName visitMap(MapType type) { - return delegate.visitMap(type); - } - - @Override - public TypeName visitOptional(OptionalType type) { - return delegate.visitOptional(type); - } - - @Override - public TypeName visitPrimitive(PrimitiveType type) { - if (type.get() == PrimitiveType.Value.BINARY) { - return ClassName.get(InputStream.class); - } else { - return delegate.visitPrimitive(type); - } - } - - @Override - public TypeName visitReference(com.palantir.conjure.spec.TypeName type) { - return delegate.visitReference(type); - } - - @Override - public TypeName visitExternal(ExternalReference type) { - return delegate.visitExternal(type); - } - - @Override - public TypeName visitSet(SetType type) { - return delegate.visitSet(type); - } -} diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/JerseyReturnTypeClassNameVisitor.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/JerseyReturnTypeClassNameVisitor.java deleted file mode 100644 index 6088f20e2..000000000 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/JerseyReturnTypeClassNameVisitor.java +++ /dev/null @@ -1,95 +0,0 @@ -/* - * (c) Copyright 2018 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.types; - -import com.palantir.conjure.java.FeatureFlags; -import com.palantir.conjure.java.util.BinaryReturnTypeResolver; -import com.palantir.conjure.spec.ExternalReference; -import com.palantir.conjure.spec.ListType; -import com.palantir.conjure.spec.MapType; -import com.palantir.conjure.spec.OptionalType; -import com.palantir.conjure.spec.PrimitiveType; -import com.palantir.conjure.spec.SetType; -import com.palantir.conjure.spec.TypeDefinition; -import com.palantir.conjure.visitor.TypeDefinitionVisitor; -import com.squareup.javapoet.ClassName; -import com.squareup.javapoet.TypeName; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.function.Function; -import java.util.stream.Collectors; -import javax.ws.rs.core.Response; -import javax.ws.rs.core.StreamingOutput; - -public final class JerseyReturnTypeClassNameVisitor implements ClassNameVisitor { - - private final boolean binaryAsResponse; - private final DefaultClassNameVisitor delegate; - private final Map types; - - public JerseyReturnTypeClassNameVisitor(List types, Set featureFlags) { - this.types = types.stream().collect( - Collectors.toMap(t -> t.accept(TypeDefinitionVisitor.TYPE_NAME), Function.identity())); - this.delegate = new DefaultClassNameVisitor(types); - this.binaryAsResponse = featureFlags.contains(FeatureFlags.JerseyBinaryAsResponse); - } - - @Override - public TypeName visitList(ListType type) { - return delegate.visitList(type); - } - - @Override - public TypeName visitMap(MapType type) { - return delegate.visitMap(type); - } - - @Override - public TypeName visitOptional(OptionalType type) { - return delegate.visitOptional(type); - } - - @Override - public TypeName visitPrimitive(PrimitiveType type) { - if (type.get() == PrimitiveType.Value.BINARY) { - if (binaryAsResponse) { - return ClassName.get(Response.class); - } else { - return ClassName.get(StreamingOutput.class); - } - } - return delegate.visitPrimitive(type); - } - - @Override - public TypeName visitReference(com.palantir.conjure.spec.TypeName type) { - return BinaryReturnTypeResolver.resolveReturnReferenceType(types, type, - binaryAsResponse ? ClassName.get(Response.class) : ClassName.get(StreamingOutput.class)); - } - - @Override - public TypeName visitExternal(ExternalReference type) { - return delegate.visitExternal(type); - } - - @Override - public TypeName visitSet(SetType type) { - return delegate.visitSet(type); - } - -} diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/Retrofit2MethodTypeClassNameVisitor.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/MethodTypeClassNameVisitor.java similarity index 82% rename from conjure-java-core/src/main/java/com/palantir/conjure/java/types/Retrofit2MethodTypeClassNameVisitor.java rename to conjure-java-core/src/main/java/com/palantir/conjure/java/types/MethodTypeClassNameVisitor.java index 256eedc48..4843cd2f1 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/Retrofit2MethodTypeClassNameVisitor.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/MethodTypeClassNameVisitor.java @@ -27,14 +27,18 @@ import com.squareup.javapoet.TypeName; import java.util.List; -public final class Retrofit2MethodTypeClassNameVisitor implements ClassNameVisitor { - - private static final ClassName REQUEST_BODY_TYPE = ClassName.get("okhttp3", "RequestBody"); +public final class MethodTypeClassNameVisitor implements ClassNameVisitor { private final DefaultClassNameVisitor delegate; + private final ClassName binaryClassName; + + public static Factory createFactory(ClassName binaryClassName) { + return types -> new MethodTypeClassNameVisitor(types, binaryClassName); + } - public Retrofit2MethodTypeClassNameVisitor(List types) { + MethodTypeClassNameVisitor(List types, ClassName binaryClassName) { this.delegate = new DefaultClassNameVisitor(types); + this.binaryClassName = binaryClassName; } @Override @@ -55,7 +59,7 @@ public TypeName visitOptional(OptionalType type) { @Override public TypeName visitPrimitive(PrimitiveType type) { if (type.get() == PrimitiveType.Value.BINARY) { - return REQUEST_BODY_TYPE; + return binaryClassName; } else { return delegate.visitPrimitive(type); } @@ -75,5 +79,4 @@ public TypeName visitExternal(ExternalReference type) { public TypeName visitSet(SetType type) { return delegate.visitSet(type); } - } diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/Retrofit2ReturnTypeClassNameVisitor.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ReturnTypeClassNameVisitor.java similarity index 65% rename from conjure-java-core/src/main/java/com/palantir/conjure/java/types/Retrofit2ReturnTypeClassNameVisitor.java rename to conjure-java-core/src/main/java/com/palantir/conjure/java/types/ReturnTypeClassNameVisitor.java index 27276d05d..133c7e927 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/Retrofit2ReturnTypeClassNameVisitor.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ReturnTypeClassNameVisitor.java @@ -16,13 +16,13 @@ package com.palantir.conjure.java.types; -import com.palantir.conjure.java.util.BinaryReturnTypeResolver; import com.palantir.conjure.spec.ExternalReference; import com.palantir.conjure.spec.ListType; import com.palantir.conjure.spec.MapType; import com.palantir.conjure.spec.OptionalType; import com.palantir.conjure.spec.PrimitiveType; import com.palantir.conjure.spec.SetType; +import com.palantir.conjure.spec.Type; import com.palantir.conjure.spec.TypeDefinition; import com.palantir.conjure.visitor.TypeDefinitionVisitor; import com.squareup.javapoet.ClassName; @@ -32,17 +32,22 @@ import java.util.function.Function; import java.util.stream.Collectors; -public final class Retrofit2ReturnTypeClassNameVisitor implements ClassNameVisitor { - - private static final ClassName RESPONSE_BODY_TYPE = ClassName.get("okhttp3", "ResponseBody"); +public final class ReturnTypeClassNameVisitor implements ClassNameVisitor { private final DefaultClassNameVisitor delegate; private final Map types; + private final ClassName binaryClassName; + + public static Factory createFactory(ClassName binaryClassName) { + return types -> new ReturnTypeClassNameVisitor(types, binaryClassName); + } - public Retrofit2ReturnTypeClassNameVisitor(List types) { - this.types = types.stream().collect( - Collectors.toMap(t -> t.accept(TypeDefinitionVisitor.TYPE_NAME), Function.identity())); + private ReturnTypeClassNameVisitor(List types, ClassName binaryClassName) { this.delegate = new DefaultClassNameVisitor(types); + this.types = types.stream().collect(Collectors.toMap( + t -> t.accept(TypeDefinitionVisitor.TYPE_NAME), + Function.identity())); + this.binaryClassName = binaryClassName; } @Override @@ -63,7 +68,7 @@ public TypeName visitOptional(OptionalType type) { @Override public TypeName visitPrimitive(PrimitiveType type) { if (type.get() == PrimitiveType.Value.BINARY) { - return RESPONSE_BODY_TYPE; + return binaryClassName; } else { return delegate.visitPrimitive(type); } @@ -71,7 +76,20 @@ public TypeName visitPrimitive(PrimitiveType type) { @Override public TypeName visitReference(com.palantir.conjure.spec.TypeName type) { - return BinaryReturnTypeResolver.resolveReturnReferenceType(types, type, RESPONSE_BODY_TYPE); + if (!types.containsKey(type)) { + throw new IllegalStateException("Unknown LocalReferenceType type: " + type); + } + + TypeDefinition def = types.get(type); + if (def.accept(TypeDefinitionVisitor.IS_ALIAS)) { + Type aliasType = def.accept(TypeDefinitionVisitor.ALIAS).getAlias(); + TypeName aliasTypeName = aliasType.accept(this); + if (aliasTypeName.equals(binaryClassName)) { + return aliasTypeName; + } + } + + return delegate.visitReference(type); } @Override @@ -83,5 +101,4 @@ public TypeName visitExternal(ExternalReference type) { public TypeName visitSet(SetType type) { return delegate.visitSet(type); } - } diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/TypeMapper.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/TypeMapper.java index 5f850bd48..10aa8ae27 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/TypeMapper.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/TypeMapper.java @@ -37,8 +37,7 @@ public TypeMapper(List types) { this(types, DefaultClassNameVisitor::new); } - public TypeMapper(List types, - Factory classNameVisitorFactory) { + public TypeMapper(List types, Factory classNameVisitorFactory) { this.types = types.stream().collect( Collectors.toMap(t -> t.accept(TypeDefinitionVisitor.TYPE_NAME), Function.identity())); this.classNameVisitorFactory = classNameVisitorFactory; diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/util/BinaryReturnTypeResolver.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/util/BinaryReturnTypeResolver.java deleted file mode 100644 index f79f0ae33..000000000 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/util/BinaryReturnTypeResolver.java +++ /dev/null @@ -1,66 +0,0 @@ -/* - * (c) Copyright 2018 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.util; - -import com.palantir.conjure.spec.AliasDefinition; -import com.palantir.conjure.spec.Type; -import com.palantir.conjure.spec.TypeDefinition; -import com.palantir.conjure.visitor.TypeDefinitionVisitor; -import com.palantir.conjure.visitor.TypeVisitor; -import com.squareup.javapoet.ClassName; -import com.squareup.javapoet.TypeName; -import java.util.Map; - -public final class BinaryReturnTypeResolver { - - private BinaryReturnTypeResolver() {} - - /** - * Recursively examines nested local alias references to see if the leaf node is of a binary type. If so, - * this method resolves it to a binary type supplied by the {@code binaryTypeName}, otherwise it returns the - * immediate local reference {@code {@link TypeName}}. - * - * @param types - a map of name to conjure definition - * @param type - the given type to be resolved - * @param binaryTypeName - the return binary type name if the leaf node is of a binary type - * @return resolved return reference type - */ - public static TypeName resolveReturnReferenceType( - Map types, - com.palantir.conjure.spec.TypeName type, - TypeName binaryTypeName) { - - if (!types.containsKey(type)) { - throw new IllegalStateException("Unknown LocalReferenceType type: " + type); - } - - TypeDefinition def = types.get(type); - while (def.accept(TypeDefinitionVisitor.IS_ALIAS)) { - AliasDefinition aliasDefinition = def.accept(TypeDefinitionVisitor.ALIAS); - Type conjureType = aliasDefinition.getAlias(); - if (conjureType.accept(TypeVisitor.IS_REFERENCE)) { - def = types.get(conjureType.accept(TypeVisitor.REFERENCE)); - } else if (conjureType.accept(TypeVisitor.IS_BINARY)) { - return binaryTypeName; - } else { - break; - } - } - - return ClassName.get(type.getPackage(), type.getName()); - } -} 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 d21fbd453..02f283bd6 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 @@ -65,6 +65,7 @@ public interface TestService { @GET @Path("catalog/datasets/{datasetRid}/raw-aliased") + @Produces(MediaType.APPLICATION_OCTET_STREAM) StreamingOutput getAliasedRawData( @HeaderParam("Authorization") @NotNull AuthHeader authHeader, @PathParam("datasetRid") @Safe ResourceIdentifier datasetRid); 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 d21fbd453..02f283bd6 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 @@ -65,6 +65,7 @@ public interface TestService { @GET @Path("catalog/datasets/{datasetRid}/raw-aliased") + @Produces(MediaType.APPLICATION_OCTET_STREAM) StreamingOutput getAliasedRawData( @HeaderParam("Authorization") @NotNull AuthHeader authHeader, @PathParam("datasetRid") @Safe ResourceIdentifier datasetRid); From c36522c9a0b75c2f21a4c931656677066c6e9f1a Mon Sep 17 00:00:00 2001 From: Patrick Koenig Date: Mon, 7 Jan 2019 11:54:44 -0800 Subject: [PATCH 2/5] Comments --- .../java/services/JerseyServiceGenerator.java | 32 +++++++++---------- .../services/Retrofit2ServiceGenerator.java | 22 ++++++------- ...java => ArgumentTypeClassNameVisitor.java} | 6 ++-- 3 files changed, 30 insertions(+), 30 deletions(-) rename conjure-java-core/src/main/java/com/palantir/conjure/java/types/{MethodTypeClassNameVisitor.java => ArgumentTypeClassNameVisitor.java} (90%) 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 9d9240f46..b6939e914 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 @@ -23,7 +23,7 @@ import com.google.common.collect.Lists; import com.palantir.conjure.java.ConjureAnnotations; import com.palantir.conjure.java.FeatureFlags; -import com.palantir.conjure.java.types.MethodTypeClassNameVisitor; +import com.palantir.conjure.java.types.ArgumentTypeClassNameVisitor; import com.palantir.conjure.java.types.ReturnTypeClassNameVisitor; import com.palantir.conjure.java.types.TypeMapper; import com.palantir.conjure.spec.ArgumentDefinition; @@ -75,7 +75,7 @@ public final class JerseyServiceGenerator implements ServiceGenerator { private static final ClassName NOT_NULL = ClassName.get("javax.validation.constraints", "NotNull"); - private static final ClassName BINARY_METHOD_TYPE = ClassName.get(InputStream.class); + private static final ClassName BINARY_ARGUMENT_TYPE = ClassName.get(InputStream.class); private static final ClassName BINARY_RETURN_TYPE_RESPONSE = ClassName.get(Response.class); private static final ClassName BINARY_RETURN_TYPE_OUTPUT = ClassName.get(StreamingOutput.class); @@ -97,16 +97,16 @@ public Set generate(ConjureDefinition conjureDefinition) { TypeMapper returnTypeMapper = new TypeMapper( conjureDefinition.getTypes(), ReturnTypeClassNameVisitor.createFactory(binaryReturnType)); - TypeMapper methodTypeMapper = new TypeMapper( + TypeMapper argumentTypeMapper = new TypeMapper( conjureDefinition.getTypes(), - MethodTypeClassNameVisitor.createFactory(BINARY_METHOD_TYPE)); + ArgumentTypeClassNameVisitor.createFactory(BINARY_ARGUMENT_TYPE)); return conjureDefinition.getServices().stream() - .map(serviceDef -> generateService(serviceDef, returnTypeMapper, methodTypeMapper)) + .map(serviceDef -> generateService(serviceDef, returnTypeMapper, argumentTypeMapper)) .collect(Collectors.toSet()); } private JavaFile generateService(ServiceDefinition serviceDefinition, - TypeMapper returnTypeMapper, TypeMapper methodTypeMapper) { + TypeMapper returnTypeMapper, TypeMapper argumentTypeMapper) { TypeSpec.Builder serviceBuilder = TypeSpec.interfaceBuilder(serviceDefinition.getServiceName().getName()) .addModifiers(Modifier.PUBLIC) .addAnnotation(AnnotationSpec.builder(ClassName.get("javax.ws.rs", "Consumes")) @@ -124,12 +124,12 @@ private JavaFile generateService(ServiceDefinition serviceDefinition, serviceBuilder.addJavadoc("$L", StringUtils.appendIfMissing(docs.get(), "\n"))); serviceBuilder.addMethods(serviceDefinition.getEndpoints().stream() - .map(endpoint -> generateServiceMethod(endpoint, returnTypeMapper, methodTypeMapper)) + .map(endpoint -> generateServiceMethod(endpoint, returnTypeMapper, argumentTypeMapper)) .collect(Collectors.toList())); serviceBuilder.addMethods(serviceDefinition.getEndpoints().stream() .map(endpoint -> generateCompatibilityBackfillServiceMethods(endpoint, returnTypeMapper, - methodTypeMapper)) + argumentTypeMapper)) .flatMap(Collection::stream) .collect(Collectors.toList())); @@ -141,7 +141,7 @@ private JavaFile generateService(ServiceDefinition serviceDefinition, private MethodSpec generateServiceMethod( EndpointDefinition endpointDef, TypeMapper returnTypeMapper, - TypeMapper methodTypeMapper) { + TypeMapper argumentTypeMapper) { TypeName returnType = endpointDef.getReturns() .map(returnTypeMapper::getClassName) .orElse(ClassName.VOID); @@ -149,7 +149,7 @@ private MethodSpec generateServiceMethod( MethodSpec.Builder methodBuilder = MethodSpec.methodBuilder(endpointDef.getEndpointName().get()) .addModifiers(Modifier.PUBLIC, Modifier.ABSTRACT) .addAnnotation(httpMethodToClassName(endpointDef.getHttpMethod().get().name())) - .addParameters(createServiceMethodParameters(endpointDef, methodTypeMapper, true)) + .addParameters(createServiceMethodParameters(endpointDef, argumentTypeMapper, true)) .returns(returnType); // @Path("") is invalid in Feign JaxRs and equivalent to absent on an endpoint method @@ -169,8 +169,8 @@ private MethodSpec generateServiceMethod( boolean consumesTypeIsBinary = endpointDef.getArgs().stream() .map(ArgumentDefinition::getType) - .map(methodTypeMapper::getClassName) - .anyMatch(BINARY_METHOD_TYPE::equals); + .map(argumentTypeMapper::getClassName) + .anyMatch(BINARY_ARGUMENT_TYPE::equals); if (consumesTypeIsBinary) { methodBuilder.addAnnotation(AnnotationSpec.builder(ClassName.get("javax.ws.rs", "Consumes")) @@ -191,7 +191,7 @@ private MethodSpec generateServiceMethod( private List generateCompatibilityBackfillServiceMethods( EndpointDefinition endpointDef, TypeMapper returnTypeMapper, - TypeMapper methodTypeMapper) { + TypeMapper argumentTypeMapper) { List args = Lists.newArrayList(); List queryArgs = Lists.newArrayList(); @@ -210,7 +210,7 @@ private List generateCompatibilityBackfillServiceMethods( alternateMethods.add(createCompatibilityBackfillMethod( endpointDef, returnTypeMapper, - methodTypeMapper, + argumentTypeMapper, queryArgs.subList(i, queryArgs.size()))); } @@ -220,10 +220,10 @@ private List generateCompatibilityBackfillServiceMethods( private MethodSpec createCompatibilityBackfillMethod( EndpointDefinition endpointDef, TypeMapper returnTypeMapper, - TypeMapper methodTypeMapper, + TypeMapper argumentTypeMapper, List extraArgs) { // ensure the correct ordering of parameters by creating the complete sorted parameter list - List sortedParams = createServiceMethodParameters(endpointDef, methodTypeMapper, false); + List sortedParams = createServiceMethodParameters(endpointDef, argumentTypeMapper, false); List> sortedMaybeExtraArgs = sortedParams.stream().map(param -> extraArgs.stream() .filter(arg -> arg.getArgName().get().equals(param.name)) 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 1eca42b74..ab44cf7c7 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 @@ -23,7 +23,7 @@ import com.google.common.collect.Lists; import com.palantir.conjure.java.ConjureAnnotations; import com.palantir.conjure.java.FeatureFlags; -import com.palantir.conjure.java.types.MethodTypeClassNameVisitor; +import com.palantir.conjure.java.types.ArgumentTypeClassNameVisitor; import com.palantir.conjure.java.types.ReturnTypeClassNameVisitor; import com.palantir.conjure.java.types.TypeMapper; import com.palantir.conjure.spec.ArgumentDefinition; @@ -69,7 +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 ClassName BINARY_METHOD_TYPE = ClassName.get("okhttp3", "RequestBody"); + private static final ClassName BINARY_ARGUMENT_TYPE = ClassName.get("okhttp3", "RequestBody"); private static final ClassName BINARY_RETURN_TYPE = ClassName.get("okhttp3", "ResponseBody"); private static final Logger log = LoggerFactory.getLogger(Retrofit2ServiceGenerator.class); @@ -89,16 +89,16 @@ public Set generate(ConjureDefinition conjureDefinition) { TypeMapper returnTypeMapper = new TypeMapper( conjureDefinition.getTypes(), ReturnTypeClassNameVisitor.createFactory(BINARY_RETURN_TYPE)); - TypeMapper methodTypeMapper = new TypeMapper( + TypeMapper argumentTypeMapper = new TypeMapper( conjureDefinition.getTypes(), - MethodTypeClassNameVisitor.createFactory(BINARY_METHOD_TYPE)); + ArgumentTypeClassNameVisitor.createFactory(BINARY_ARGUMENT_TYPE)); return conjureDefinition.getServices().stream() - .map(serviceDef -> generateService(serviceDef, returnTypeMapper, methodTypeMapper)) + .map(serviceDef -> generateService(serviceDef, returnTypeMapper, argumentTypeMapper)) .collect(Collectors.toSet()); } private JavaFile generateService(ServiceDefinition serviceDefinition, - TypeMapper returnTypeMapper, TypeMapper methodTypeMapper) { + TypeMapper returnTypeMapper, TypeMapper argumentTypeMapper) { TypeSpec.Builder serviceBuilder = TypeSpec.interfaceBuilder(serviceName(serviceDefinition)) .addModifiers(Modifier.PUBLIC) .addAnnotation(ConjureAnnotations.getConjureGeneratedAnnotation(Retrofit2ServiceGenerator.class)); @@ -108,7 +108,7 @@ private JavaFile generateService(ServiceDefinition serviceDefinition, }); serviceBuilder.addMethods(serviceDefinition.getEndpoints().stream() - .map(endpoint -> generateServiceMethod(endpoint, returnTypeMapper, methodTypeMapper)) + .map(endpoint -> generateServiceMethod(endpoint, returnTypeMapper, argumentTypeMapper)) .collect(Collectors.toList())); return JavaFile.builder(serviceDefinition.getServiceName().getPackage(), serviceBuilder.build()) @@ -133,7 +133,7 @@ private ClassName getReturnType() { private MethodSpec generateServiceMethod( EndpointDefinition endpointDef, TypeMapper returnTypeMapper, - TypeMapper methodTypeMapper) { + TypeMapper argumentTypeMapper) { TypeName returnType = endpointDef.getReturns() .map(returnTypeMapper::getClassName) .orElse(ClassName.VOID); @@ -165,7 +165,7 @@ private MethodSpec generateServiceMethod( getAuthParameter(endpointDef.getAuth()).ifPresent(methodBuilder::addParameter); methodBuilder.addParameters(endpointDef.getArgs().stream() - .map(arg -> createEndpointParameter(methodTypeMapper, encodedPathArgs, arg)) + .map(arg -> createEndpointParameter(argumentTypeMapper, encodedPathArgs, arg)) .collect(Collectors.toList())); return methodBuilder.build(); @@ -201,11 +201,11 @@ private HttpPath replaceEncodedPathArgs(HttpPath httpPath) { } private ParameterSpec createEndpointParameter( - TypeMapper methodTypeMapper, + TypeMapper argumentTypeMapper, Set encodedPathArgs, ArgumentDefinition def) { ParameterSpec.Builder param = ParameterSpec.builder( - methodTypeMapper.getClassName(def.getType()), + argumentTypeMapper.getClassName(def.getType()), def.getArgName().get()); ParameterType paramType = def.getParamType(); if (paramType.accept(ParameterTypeVisitor.IS_PATH)) { diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/MethodTypeClassNameVisitor.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ArgumentTypeClassNameVisitor.java similarity index 90% rename from conjure-java-core/src/main/java/com/palantir/conjure/java/types/MethodTypeClassNameVisitor.java rename to conjure-java-core/src/main/java/com/palantir/conjure/java/types/ArgumentTypeClassNameVisitor.java index 4843cd2f1..2d4ac3cb1 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/MethodTypeClassNameVisitor.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ArgumentTypeClassNameVisitor.java @@ -27,16 +27,16 @@ import com.squareup.javapoet.TypeName; import java.util.List; -public final class MethodTypeClassNameVisitor implements ClassNameVisitor { +public final class ArgumentTypeClassNameVisitor implements ClassNameVisitor { private final DefaultClassNameVisitor delegate; private final ClassName binaryClassName; public static Factory createFactory(ClassName binaryClassName) { - return types -> new MethodTypeClassNameVisitor(types, binaryClassName); + return types -> new ArgumentTypeClassNameVisitor(types, binaryClassName); } - MethodTypeClassNameVisitor(List types, ClassName binaryClassName) { + private ArgumentTypeClassNameVisitor(List types, ClassName binaryClassName) { this.delegate = new DefaultClassNameVisitor(types); this.binaryClassName = binaryClassName; } From f44cced40e5dce4958665e14dc28ffdb000fbd6b Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Wed, 9 Jan 2019 16:02:52 +0000 Subject: [PATCH 3/5] Don't take 'Factory' constructor param --- .../java/services/JerseyServiceGenerator.java | 4 +- .../services/Retrofit2ServiceGenerator.java | 4 +- .../services/UndertowServiceGenerator.java | 8 ++- .../types/ArgumentTypeClassNameVisitor.java | 2 +- .../conjure/java/types/ClassNameVisitor.java | 49 +++++++++++++++++++ .../types/ReturnTypeClassNameVisitor.java | 2 +- .../conjure/java/types/TypeMapper.java | 18 +++---- 7 files changed, 70 insertions(+), 17 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 d99b98ce0..33be83769 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 @@ -96,10 +96,10 @@ public Set generate(ConjureDefinition conjureDefinition) { : BINARY_RETURN_TYPE_OUTPUT; TypeMapper returnTypeMapper = new TypeMapper( conjureDefinition.getTypes(), - ReturnTypeClassNameVisitor.createFactory(binaryReturnType)); + ReturnTypeClassNameVisitor.createFactory(binaryReturnType).create(conjureDefinition.getTypes())); TypeMapper argumentTypeMapper = new TypeMapper( conjureDefinition.getTypes(), - ArgumentTypeClassNameVisitor.createFactory(BINARY_ARGUMENT_TYPE)); + ArgumentTypeClassNameVisitor.createFactory(BINARY_ARGUMENT_TYPE).create(conjureDefinition.getTypes())); return conjureDefinition.getServices().stream() .map(serviceDef -> generateService(serviceDef, returnTypeMapper, argumentTypeMapper)) .collect(Collectors.toSet()); 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 e9178fde4..905d4fefc 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 @@ -88,10 +88,10 @@ public Retrofit2ServiceGenerator(Set experimentalFeatures) { public Set generate(ConjureDefinition conjureDefinition) { TypeMapper returnTypeMapper = new TypeMapper( conjureDefinition.getTypes(), - ReturnTypeClassNameVisitor.createFactory(BINARY_RETURN_TYPE)); + ReturnTypeClassNameVisitor.createFactory(BINARY_RETURN_TYPE).create(conjureDefinition.getTypes())); TypeMapper argumentTypeMapper = new TypeMapper( conjureDefinition.getTypes(), - ArgumentTypeClassNameVisitor.createFactory(BINARY_ARGUMENT_TYPE)); + ArgumentTypeClassNameVisitor.createFactory(BINARY_ARGUMENT_TYPE).create(conjureDefinition.getTypes())); return conjureDefinition.getServices().stream() .map(serviceDef -> generateService(serviceDef, returnTypeMapper, argumentTypeMapper)) .collect(Collectors.toSet()); diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/services/UndertowServiceGenerator.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/services/UndertowServiceGenerator.java index 7c795643f..8db8bbfcf 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/services/UndertowServiceGenerator.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/services/UndertowServiceGenerator.java @@ -53,8 +53,12 @@ public UndertowServiceGenerator(Set experimentalFeatures) { public Set generate(ConjureDefinition conjureDefinition) { return conjureDefinition.getServices().stream() .flatMap(serviceDef -> generateService(serviceDef, conjureDefinition.getTypes(), - new TypeMapper(conjureDefinition.getTypes(), UndertowRequestBodyClassNameVisitor::new), - new TypeMapper(conjureDefinition.getTypes(), UndertowReturnValueClassNameVisitor::new)) + new TypeMapper( + conjureDefinition.getTypes(), + new UndertowRequestBodyClassNameVisitor(conjureDefinition.getTypes())), + new TypeMapper( + conjureDefinition.getTypes(), + new UndertowReturnValueClassNameVisitor(conjureDefinition.getTypes()))) .stream()).collect(Collectors.toSet()); } diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ArgumentTypeClassNameVisitor.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ArgumentTypeClassNameVisitor.java index 2d4ac3cb1..b66b2164b 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ArgumentTypeClassNameVisitor.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ArgumentTypeClassNameVisitor.java @@ -36,7 +36,7 @@ public static Factory createFactory(ClassName binaryClassName) { return types -> new ArgumentTypeClassNameVisitor(types, binaryClassName); } - private ArgumentTypeClassNameVisitor(List types, ClassName binaryClassName) { + public ArgumentTypeClassNameVisitor(List types, ClassName binaryClassName) { this.delegate = new DefaultClassNameVisitor(types); this.binaryClassName = binaryClassName; } diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ClassNameVisitor.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ClassNameVisitor.java index e4cb6f75f..be6525f4b 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ClassNameVisitor.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ClassNameVisitor.java @@ -16,6 +16,12 @@ package com.palantir.conjure.java.types; +import com.palantir.conjure.spec.ExternalReference; +import com.palantir.conjure.spec.ListType; +import com.palantir.conjure.spec.MapType; +import com.palantir.conjure.spec.OptionalType; +import com.palantir.conjure.spec.PrimitiveType; +import com.palantir.conjure.spec.SetType; import com.palantir.conjure.spec.Type; import com.palantir.conjure.spec.TypeDefinition; import com.squareup.javapoet.TypeName; @@ -34,4 +40,47 @@ interface Factory { default TypeName visitUnknown(String unknownType) { throw new IllegalStateException("Unknown type:" + unknownType); } + + static ClassNameVisitor specializePrimitiveBinary(ClassNameVisitor delegate, TypeName binaryClassName) { + return new ClassNameVisitor() { + @Override + public TypeName visitPrimitive(PrimitiveType value) { + if (value.get() == PrimitiveType.Value.BINARY) { + return binaryClassName; + } else { + return delegate.visitPrimitive(value); + } + } + + @Override + public TypeName visitOptional(OptionalType value) { + return delegate.visitOptional(value); + } + + @Override + public TypeName visitList(ListType value) { + return delegate.visitList(value); + } + + @Override + public TypeName visitSet(SetType value) { + return delegate.visitSet(value); + } + + @Override + public TypeName visitMap(MapType value) { + return delegate.visitMap(value); + } + + @Override + public TypeName visitReference(com.palantir.conjure.spec.TypeName value) { + return delegate.visitReference(value); + } + + @Override + public TypeName visitExternal(ExternalReference value) { + return delegate.visitExternal(value); + } + }; + } } diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ReturnTypeClassNameVisitor.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ReturnTypeClassNameVisitor.java index 133c7e927..6a51b859c 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ReturnTypeClassNameVisitor.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ReturnTypeClassNameVisitor.java @@ -42,7 +42,7 @@ public static Factory createFactory(ClassName binaryClassName) { return types -> new ReturnTypeClassNameVisitor(types, binaryClassName); } - private ReturnTypeClassNameVisitor(List types, ClassName binaryClassName) { + public ReturnTypeClassNameVisitor(List types, ClassName binaryClassName) { this.delegate = new DefaultClassNameVisitor(types); this.types = types.stream().collect(Collectors.toMap( t -> t.accept(TypeDefinitionVisitor.TYPE_NAME), diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/TypeMapper.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/TypeMapper.java index 10aa8ae27..ee95569a2 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/TypeMapper.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/TypeMapper.java @@ -16,12 +16,10 @@ package com.palantir.conjure.java.types; -import com.palantir.conjure.java.types.ClassNameVisitor.Factory; import com.palantir.conjure.spec.Type; import com.palantir.conjure.spec.TypeDefinition; import com.palantir.conjure.visitor.TypeDefinitionVisitor; import com.squareup.javapoet.TypeName; -import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Optional; @@ -31,16 +29,18 @@ public final class TypeMapper { private final Map types; - private final Factory classNameVisitorFactory; + private final ClassNameVisitor classNameVisitor; public TypeMapper(List types) { - this(types, DefaultClassNameVisitor::new); + this(types, new DefaultClassNameVisitor(types)); } - public TypeMapper(List types, Factory classNameVisitorFactory) { - this.types = types.stream().collect( - Collectors.toMap(t -> t.accept(TypeDefinitionVisitor.TYPE_NAME), Function.identity())); - this.classNameVisitorFactory = classNameVisitorFactory; + public TypeMapper(List types, ClassNameVisitor classNameVisitor) { + this.types = types.stream() + .collect(Collectors.toMap( + t -> t.accept(TypeDefinitionVisitor.TYPE_NAME), + Function.identity())); + this.classNameVisitor = classNameVisitor; } public Optional getType(com.palantir.conjure.spec.TypeName typeName) { @@ -48,6 +48,6 @@ public Optional getType(com.palantir.conjure.spec.TypeName typeN } public TypeName getClassName(Type type) { - return type.accept(classNameVisitorFactory.create(new ArrayList<>(types.values()))); + return type.accept(classNameVisitor); } } From bfbc2d55d334f6bfe23401803c7468bcb98babaa Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Wed, 9 Jan 2019 16:04:49 +0000 Subject: [PATCH 4/5] Delete 'Factory' entirely --- .../conjure/java/services/JerseyServiceGenerator.java | 4 ++-- .../conjure/java/services/Retrofit2ServiceGenerator.java | 4 ++-- .../conjure/java/types/ArgumentTypeClassNameVisitor.java | 4 ---- .../com/palantir/conjure/java/types/ClassNameVisitor.java | 6 ------ .../conjure/java/types/ReturnTypeClassNameVisitor.java | 4 ---- 5 files changed, 4 insertions(+), 18 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 33be83769..4f6b0a5a3 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 @@ -96,10 +96,10 @@ public Set generate(ConjureDefinition conjureDefinition) { : BINARY_RETURN_TYPE_OUTPUT; TypeMapper returnTypeMapper = new TypeMapper( conjureDefinition.getTypes(), - ReturnTypeClassNameVisitor.createFactory(binaryReturnType).create(conjureDefinition.getTypes())); + new ReturnTypeClassNameVisitor(conjureDefinition.getTypes(), binaryReturnType)); TypeMapper argumentTypeMapper = new TypeMapper( conjureDefinition.getTypes(), - ArgumentTypeClassNameVisitor.createFactory(BINARY_ARGUMENT_TYPE).create(conjureDefinition.getTypes())); + new ArgumentTypeClassNameVisitor(conjureDefinition.getTypes(), BINARY_ARGUMENT_TYPE)); return conjureDefinition.getServices().stream() .map(serviceDef -> generateService(serviceDef, returnTypeMapper, argumentTypeMapper)) .collect(Collectors.toSet()); 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 905d4fefc..f91dcccdc 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 @@ -88,10 +88,10 @@ public Retrofit2ServiceGenerator(Set experimentalFeatures) { public Set generate(ConjureDefinition conjureDefinition) { TypeMapper returnTypeMapper = new TypeMapper( conjureDefinition.getTypes(), - ReturnTypeClassNameVisitor.createFactory(BINARY_RETURN_TYPE).create(conjureDefinition.getTypes())); + new ReturnTypeClassNameVisitor(conjureDefinition.getTypes(), BINARY_RETURN_TYPE)); TypeMapper argumentTypeMapper = new TypeMapper( conjureDefinition.getTypes(), - ArgumentTypeClassNameVisitor.createFactory(BINARY_ARGUMENT_TYPE).create(conjureDefinition.getTypes())); + new ArgumentTypeClassNameVisitor(conjureDefinition.getTypes(), BINARY_ARGUMENT_TYPE)); return conjureDefinition.getServices().stream() .map(serviceDef -> generateService(serviceDef, returnTypeMapper, argumentTypeMapper)) .collect(Collectors.toSet()); diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ArgumentTypeClassNameVisitor.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ArgumentTypeClassNameVisitor.java index b66b2164b..0823f585a 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ArgumentTypeClassNameVisitor.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ArgumentTypeClassNameVisitor.java @@ -32,10 +32,6 @@ public final class ArgumentTypeClassNameVisitor implements ClassNameVisitor { private final DefaultClassNameVisitor delegate; private final ClassName binaryClassName; - public static Factory createFactory(ClassName binaryClassName) { - return types -> new ArgumentTypeClassNameVisitor(types, binaryClassName); - } - public ArgumentTypeClassNameVisitor(List types, ClassName binaryClassName) { this.delegate = new DefaultClassNameVisitor(types); this.binaryClassName = binaryClassName; diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ClassNameVisitor.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ClassNameVisitor.java index be6525f4b..242ddfbd1 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ClassNameVisitor.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ClassNameVisitor.java @@ -23,19 +23,13 @@ import com.palantir.conjure.spec.PrimitiveType; import com.palantir.conjure.spec.SetType; import com.palantir.conjure.spec.Type; -import com.palantir.conjure.spec.TypeDefinition; import com.squareup.javapoet.TypeName; -import java.util.List; /** * Maps a conjure type into the corresponding java type. */ public interface ClassNameVisitor extends Type.Visitor { - interface Factory { - ClassNameVisitor create(List types); - } - @Override default TypeName visitUnknown(String unknownType) { throw new IllegalStateException("Unknown type:" + unknownType); diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ReturnTypeClassNameVisitor.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ReturnTypeClassNameVisitor.java index 6a51b859c..078b7cc49 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ReturnTypeClassNameVisitor.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ReturnTypeClassNameVisitor.java @@ -38,10 +38,6 @@ public final class ReturnTypeClassNameVisitor implements ClassNameVisitor { private final Map types; private final ClassName binaryClassName; - public static Factory createFactory(ClassName binaryClassName) { - return types -> new ReturnTypeClassNameVisitor(types, binaryClassName); - } - public ReturnTypeClassNameVisitor(List types, ClassName binaryClassName) { this.delegate = new DefaultClassNameVisitor(types); this.types = types.stream().collect(Collectors.toMap( From 5caabf7feffe1aaae6443dc103de64c22d6a54b7 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Wed, 9 Jan 2019 16:15:11 +0000 Subject: [PATCH 5/5] ArgumentTypeClassNameVisitor -> SpecializeBinaryClassNameVisitor --- .../java/services/JerseyServiceGenerator.java | 10 +++- .../services/Retrofit2ServiceGenerator.java | 9 +++- .../conjure/java/types/ClassNameVisitor.java | 49 ------------------ ... => SpecializeBinaryClassNameVisitor.java} | 51 +++++++++---------- 4 files changed, 39 insertions(+), 80 deletions(-) rename conjure-java-core/src/main/java/com/palantir/conjure/java/types/{ArgumentTypeClassNameVisitor.java => SpecializeBinaryClassNameVisitor.java} (51%) 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 4f6b0a5a3..c5f653132 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 @@ -23,8 +23,9 @@ import com.google.common.collect.Lists; import com.palantir.conjure.java.ConjureAnnotations; import com.palantir.conjure.java.FeatureFlags; -import com.palantir.conjure.java.types.ArgumentTypeClassNameVisitor; +import com.palantir.conjure.java.types.DefaultClassNameVisitor; import com.palantir.conjure.java.types.ReturnTypeClassNameVisitor; +import com.palantir.conjure.java.types.SpecializeBinaryClassNameVisitor; import com.palantir.conjure.java.types.TypeMapper; import com.palantir.conjure.spec.ArgumentDefinition; import com.palantir.conjure.spec.AuthType; @@ -94,12 +95,17 @@ public Set generate(ConjureDefinition conjureDefinition) { ClassName binaryReturnType = experimentalFeatures.contains(FeatureFlags.JerseyBinaryAsResponse) ? BINARY_RETURN_TYPE_RESPONSE : BINARY_RETURN_TYPE_OUTPUT; + TypeMapper returnTypeMapper = new TypeMapper( conjureDefinition.getTypes(), new ReturnTypeClassNameVisitor(conjureDefinition.getTypes(), binaryReturnType)); + TypeMapper argumentTypeMapper = new TypeMapper( conjureDefinition.getTypes(), - new ArgumentTypeClassNameVisitor(conjureDefinition.getTypes(), BINARY_ARGUMENT_TYPE)); + new SpecializeBinaryClassNameVisitor( + new DefaultClassNameVisitor(conjureDefinition.getTypes()), + BINARY_ARGUMENT_TYPE)); + return conjureDefinition.getServices().stream() .map(serviceDef -> generateService(serviceDef, returnTypeMapper, argumentTypeMapper)) .collect(Collectors.toSet()); 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 f91dcccdc..16985f190 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 @@ -23,8 +23,9 @@ import com.google.common.collect.Lists; import com.palantir.conjure.java.ConjureAnnotations; import com.palantir.conjure.java.FeatureFlags; -import com.palantir.conjure.java.types.ArgumentTypeClassNameVisitor; +import com.palantir.conjure.java.types.DefaultClassNameVisitor; import com.palantir.conjure.java.types.ReturnTypeClassNameVisitor; +import com.palantir.conjure.java.types.SpecializeBinaryClassNameVisitor; import com.palantir.conjure.java.types.TypeMapper; import com.palantir.conjure.spec.ArgumentDefinition; import com.palantir.conjure.spec.ArgumentName; @@ -89,9 +90,13 @@ public Set generate(ConjureDefinition conjureDefinition) { TypeMapper returnTypeMapper = new TypeMapper( conjureDefinition.getTypes(), new ReturnTypeClassNameVisitor(conjureDefinition.getTypes(), BINARY_RETURN_TYPE)); + TypeMapper argumentTypeMapper = new TypeMapper( conjureDefinition.getTypes(), - new ArgumentTypeClassNameVisitor(conjureDefinition.getTypes(), BINARY_ARGUMENT_TYPE)); + new SpecializeBinaryClassNameVisitor( + new DefaultClassNameVisitor(conjureDefinition.getTypes()), + BINARY_ARGUMENT_TYPE)); + return conjureDefinition.getServices().stream() .map(serviceDef -> generateService(serviceDef, returnTypeMapper, argumentTypeMapper)) .collect(Collectors.toSet()); diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ClassNameVisitor.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ClassNameVisitor.java index 242ddfbd1..31c380c9f 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ClassNameVisitor.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ClassNameVisitor.java @@ -16,12 +16,6 @@ package com.palantir.conjure.java.types; -import com.palantir.conjure.spec.ExternalReference; -import com.palantir.conjure.spec.ListType; -import com.palantir.conjure.spec.MapType; -import com.palantir.conjure.spec.OptionalType; -import com.palantir.conjure.spec.PrimitiveType; -import com.palantir.conjure.spec.SetType; import com.palantir.conjure.spec.Type; import com.squareup.javapoet.TypeName; @@ -34,47 +28,4 @@ public interface ClassNameVisitor extends Type.Visitor { default TypeName visitUnknown(String unknownType) { throw new IllegalStateException("Unknown type:" + unknownType); } - - static ClassNameVisitor specializePrimitiveBinary(ClassNameVisitor delegate, TypeName binaryClassName) { - return new ClassNameVisitor() { - @Override - public TypeName visitPrimitive(PrimitiveType value) { - if (value.get() == PrimitiveType.Value.BINARY) { - return binaryClassName; - } else { - return delegate.visitPrimitive(value); - } - } - - @Override - public TypeName visitOptional(OptionalType value) { - return delegate.visitOptional(value); - } - - @Override - public TypeName visitList(ListType value) { - return delegate.visitList(value); - } - - @Override - public TypeName visitSet(SetType value) { - return delegate.visitSet(value); - } - - @Override - public TypeName visitMap(MapType value) { - return delegate.visitMap(value); - } - - @Override - public TypeName visitReference(com.palantir.conjure.spec.TypeName value) { - return delegate.visitReference(value); - } - - @Override - public TypeName visitExternal(ExternalReference value) { - return delegate.visitExternal(value); - } - }; - } } diff --git a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ArgumentTypeClassNameVisitor.java b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/SpecializeBinaryClassNameVisitor.java similarity index 51% rename from conjure-java-core/src/main/java/com/palantir/conjure/java/types/ArgumentTypeClassNameVisitor.java rename to conjure-java-core/src/main/java/com/palantir/conjure/java/types/SpecializeBinaryClassNameVisitor.java index 0823f585a..a0ce6ce7f 100644 --- a/conjure-java-core/src/main/java/com/palantir/conjure/java/types/ArgumentTypeClassNameVisitor.java +++ b/conjure-java-core/src/main/java/com/palantir/conjure/java/types/SpecializeBinaryClassNameVisitor.java @@ -1,5 +1,5 @@ /* - * (c) Copyright 2018 Palantir Technologies Inc. All rights reserved. + * (c) Copyright 2019 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. @@ -22,57 +22,54 @@ import com.palantir.conjure.spec.OptionalType; import com.palantir.conjure.spec.PrimitiveType; import com.palantir.conjure.spec.SetType; -import com.palantir.conjure.spec.TypeDefinition; -import com.squareup.javapoet.ClassName; import com.squareup.javapoet.TypeName; -import java.util.List; -public final class ArgumentTypeClassNameVisitor implements ClassNameVisitor { +public final class SpecializeBinaryClassNameVisitor implements ClassNameVisitor { - private final DefaultClassNameVisitor delegate; - private final ClassName binaryClassName; + private final ClassNameVisitor delegate; + private final TypeName binaryClassName; - public ArgumentTypeClassNameVisitor(List types, ClassName binaryClassName) { - this.delegate = new DefaultClassNameVisitor(types); + public SpecializeBinaryClassNameVisitor(ClassNameVisitor delegate, TypeName binaryClassName) { + this.delegate = delegate; this.binaryClassName = binaryClassName; } @Override - public TypeName visitList(ListType type) { - return delegate.visitList(type); + public TypeName visitPrimitive(PrimitiveType value) { + if (value.get() == PrimitiveType.Value.BINARY) { + return binaryClassName; + } else { + return delegate.visitPrimitive(value); + } } @Override - public TypeName visitMap(MapType type) { - return delegate.visitMap(type); + public TypeName visitOptional(OptionalType value) { + return delegate.visitOptional(value); } @Override - public TypeName visitOptional(OptionalType type) { - return delegate.visitOptional(type); + public TypeName visitList(ListType value) { + return delegate.visitList(value); } @Override - public TypeName visitPrimitive(PrimitiveType type) { - if (type.get() == PrimitiveType.Value.BINARY) { - return binaryClassName; - } else { - return delegate.visitPrimitive(type); - } + public TypeName visitSet(SetType value) { + return delegate.visitSet(value); } @Override - public TypeName visitReference(com.palantir.conjure.spec.TypeName type) { - return delegate.visitReference(type); + public TypeName visitMap(MapType value) { + return delegate.visitMap(value); } @Override - public TypeName visitExternal(ExternalReference type) { - return delegate.visitExternal(type); + public TypeName visitReference(com.palantir.conjure.spec.TypeName value) { + return delegate.visitReference(value); } @Override - public TypeName visitSet(SetType type) { - return delegate.visitSet(type); + public TypeName visitExternal(ExternalReference value) { + return delegate.visitExternal(value); } }