Skip to content

[fix] Add Produces annotation for binary alias return types #131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bulldozer-bot merged 6 commits into from
Jan 9, 2019
Merged

[fix] Add Produces annotation for binary alias return types #131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c45c4d1
Add Produces annotation for binary alias return types
pkoenig10 Dec 5, 2018
c36522c
Comments
pkoenig10 Jan 7, 2019
fb4f979
Merge branch 'develop' into pkoenig10-streaming
iamdanfox Jan 9, 2019
f44cced
Don't take 'Factory' constructor param
iamdanfox Jan 9, 2019
bfbc2d5
Delete 'Factory' entirely
iamdanfox Jan 9, 2019
5caabf7
ArgumentTypeClassNameVisitor -> SpecializeBinaryClassNameVisitor
iamdanfox Jan 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 43 additions & 20 deletions ...re-java-core/src/main/java/com/palantir/conjure/java/services/JerseyServiceGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.JerseyMethodTypeClassNameVisitor;
import com.palantir.conjure.java.types.JerseyReturnTypeClassNameVisitor;
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;
Expand Down Expand Up @@ -52,7 +53,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;
Expand All @@ -65,11 +68,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_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);

private final Set<FeatureFlags> experimentalFeatures;

public JerseyServiceGenerator() {
Expand All @@ -82,18 +92,27 @@ public JerseyServiceGenerator(Set<FeatureFlags> experimentalFeatures) {

@Override
public Set<JavaFile> 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));
TypeMapper methodTypeMapper = new TypeMapper(
conjureDefinition.getTypes(), JerseyMethodTypeClassNameVisitor::new);
new ReturnTypeClassNameVisitor(conjureDefinition.getTypes(), binaryReturnType));

TypeMapper argumentTypeMapper = new TypeMapper(
conjureDefinition.getTypes(),
new SpecializeBinaryClassNameVisitor(
new DefaultClassNameVisitor(conjureDefinition.getTypes()),
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"))
Expand All @@ -111,12 +130,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()));

Expand All @@ -129,11 +148,16 @@ 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);

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
String rawHttpPath = endpointDef.getHttpPath().get();
Expand All @@ -144,15 +168,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(argumentTypeMapper::getClassName)
.anyMatch(BINARY_ARGUMENT_TYPE::equals);

if (consumesTypeIsBinary) {
methodBuilder.addAnnotation(AnnotationSpec.builder(ClassName.get("javax.ws.rs", "Consumes"))
Expand All @@ -166,16 +191,14 @@ private MethodSpec generateServiceMethod(
ServiceGenerator.getJavaDoc(endpointDef).ifPresent(
content -> methodBuilder.addJavadoc("$L", content));

endpointDef.getReturns().ifPresent(type -> methodBuilder.returns(returnTypeMapper.getClassName(type)));

return methodBuilder.build();
}

/** Provides a linear expansion of optional query arguments to improve Java back-compat. */
private List<MethodSpec> generateCompatibilityBackfillServiceMethods(
EndpointDefinition endpointDef,
TypeMapper returnTypeMapper,
TypeMapper methodTypeMapper) {
TypeMapper argumentTypeMapper) {

List<ArgumentDefinition> queryArgs = Lists.newArrayList();

Expand All @@ -191,7 +214,7 @@ private List<MethodSpec> generateCompatibilityBackfillServiceMethods(
alternateMethods.add(createCompatibilityBackfillMethod(
endpointDef,
returnTypeMapper,
methodTypeMapper,
argumentTypeMapper,
queryArgs.subList(i, queryArgs.size())));
}

Expand All @@ -201,10 +224,10 @@ private List<MethodSpec> generateCompatibilityBackfillServiceMethods(
private MethodSpec createCompatibilityBackfillMethod(
EndpointDefinition endpointDef,
TypeMapper returnTypeMapper,
TypeMapper methodTypeMapper,
TypeMapper argumentTypeMapper,
List<ArgumentDefinition> extraArgs) {
// ensure the correct ordering of parameters by creating the complete sorted parameter list
List<ParameterSpec> sortedParams = createServiceMethodParameters(endpointDef, methodTypeMapper, false);
List<ParameterSpec> sortedParams = createServiceMethodParameters(endpointDef, argumentTypeMapper, false);
List<Optional<ArgumentDefinition>> sortedMaybeExtraArgs = sortedParams.stream().map(param ->
extraArgs.stream()
.filter(arg -> arg.getArgName().get().equals(param.name))
Expand Down
34 changes: 21 additions & 13 deletions ...java-core/src/main/java/com/palantir/conjure/java/services/Retrofit2ServiceGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.Retrofit2MethodTypeClassNameVisitor;
import com.palantir.conjure.java.types.Retrofit2ReturnTypeClassNameVisitor;
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;
Expand Down Expand Up @@ -69,6 +70,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_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);
Expand All @@ -85,17 +87,23 @@ public Retrofit2ServiceGenerator(Set<FeatureFlags> experimentalFeatures) {

@Override
public Set<JavaFile> 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(),
new ReturnTypeClassNameVisitor(conjureDefinition.getTypes(), BINARY_RETURN_TYPE));

TypeMapper argumentTypeMapper = new TypeMapper(
conjureDefinition.getTypes(),
new SpecializeBinaryClassNameVisitor(
new DefaultClassNameVisitor(conjureDefinition.getTypes()),
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));
Expand All @@ -105,7 +113,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())
Expand All @@ -131,7 +139,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);
Expand Down Expand Up @@ -163,7 +171,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();
Expand Down Expand Up @@ -199,11 +207,11 @@ private HttpPath replaceEncodedPathArgs(HttpPath httpPath) {
}

private ParameterSpec createEndpointParameter(
TypeMapper methodTypeMapper,
TypeMapper argumentTypeMapper,
Set<ArgumentName> 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)) {
Expand Down
8 changes: 6 additions & 2 deletions ...-java-core/src/main/java/com/palantir/conjure/java/services/UndertowServiceGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,12 @@ public UndertowServiceGenerator(Set<FeatureFlags> experimentalFeatures) {
public Set<JavaFile> 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());
}

Expand Down
6 changes: 0 additions & 6 deletions conjure-java-core/src/main/java/com/palantir/conjure/java/types/ClassNameVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,13 @@
package com.palantir.conjure.java.types;

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<TypeName> {

interface Factory {
ClassNameVisitor create(List<TypeDefinition> types);
}

@Override
default TypeName visitUnknown(String unknownType) {
throw new IllegalStateException("Unknown type:" + unknownType);
Expand Down
95 changes: 0 additions & 95 deletions ...-core/src/main/java/com/palantir/conjure/java/types/JerseyReturnTypeClassNameVisitor.java
View file
Open in desktop

This file was deleted.

Loading