[improvement] FeatureFlag to make Response the return type for binary Jersey Responses (#37)

<!-- PR title should start with '[fix]', '[improvement]' or '[break]' if this PR would cause a patch, minor or major SemVer bump. Omit the prefix if this PR doesn't warrant a standalone release. -->

## Before this PR
Currently binary response types are rendered as `StreamingOutput`. There's a bug in Jersey which causes a failure part way through streaming back that response to appear as a successful response to the client: eclipse-ee4j/jersey#3850

## After this PR
By rendering `Response` as the return type we are still able to use `StreamingOutput` as the body of the response, but we are also able to include the `Content-Length` header when we know the size of the response. Including the content length allows clients to determine there was an error, even when the stream appeared to have been closed successfully.

Author: ellisjoe

conjure-java-core/src/main/java/com/palantir/conjure/java/FeatureFlags.java

@@ -16,7 +16,9 @@
 
 package com.palantir.conjure.java;
 
+import com.palantir.conjure.java.services.JerseyServiceGenerator;
 import com.palantir.conjure.java.services.Retrofit2ServiceGenerator;
+import javax.ws.rs.core.Response;
 
 public enum FeatureFlags {
     /**
@@ -25,4 +27,9 @@ public enum FeatureFlags {
      */
     RetrofitCompletableFutures,
 
+    /**
+     * Instructs the {@link JerseyServiceGenerator} to generate binary response endpoints with the {@link Response}
+     * type.
+     */
+    JerseyBinaryAsResponse,
 }

conjure-java-core/src/main/java/com/palantir/conjure/java/services/JerseyServiceGenerator.java

@@ -19,8 +19,10 @@
 import static com.google.common.base.Preconditions.checkArgument;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import com.palantir.conjure.java.ConjureAnnotations;
+import com.palantir.conjure.java.FeatureFlags;
 import com.palantir.conjure.java.types.JerseyMethodTypeClassNameVisitor;
 import com.palantir.conjure.java.types.JerseyReturnTypeClassNameVisitor;
 import com.palantir.conjure.java.types.TypeMapper;
@@ -65,13 +67,21 @@
 public final class JerseyServiceGenerator implements ServiceGenerator {
 
 
-    public JerseyServiceGenerator() {}
+    private final Set<FeatureFlags> experimentalFeatures;
+
+    public JerseyServiceGenerator() {
+        this(ImmutableSet.of());
+    }
+
+    public JerseyServiceGenerator(Set<FeatureFlags> experimentalFeatures) {
+        this.experimentalFeatures = ImmutableSet.copyOf(experimentalFeatures);
+    }
 
     @Override
     public Set<JavaFile> generate(ConjureDefinition conjureDefinition) {
         TypeMapper returnTypeMapper = new TypeMapper(
                 conjureDefinition.getTypes(),
-                types -> new JerseyReturnTypeClassNameVisitor(types));
+                types -> new JerseyReturnTypeClassNameVisitor(types, experimentalFeatures));
         TypeMapper methodTypeMapper = new TypeMapper(
                 conjureDefinition.getTypes(), JerseyMethodTypeClassNameVisitor::new);
         return conjureDefinition.getServices().stream()

conjure-java-core/src/main/java/com/palantir/conjure/java/types/JerseyReturnTypeClassNameVisitor.java

@@ -16,6 +16,7 @@
 
 package com.palantir.conjure.java.types;
 
+import com.palantir.conjure.java.FeatureFlags;
 import com.palantir.conjure.spec.ExternalReference;
 import com.palantir.conjure.spec.ListType;
 import com.palantir.conjure.spec.MapType;
@@ -26,13 +27,18 @@
 import com.squareup.javapoet.ClassName;
 import com.squareup.javapoet.TypeName;
 import java.util.List;
+import java.util.Set;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.core.StreamingOutput;
 
 public final class JerseyReturnTypeClassNameVisitor implements ClassNameVisitor {
 
     private final DefaultClassNameVisitor delegate;
+    private final boolean binaryAsResponse;
 
-    public JerseyReturnTypeClassNameVisitor(List<TypeDefinition> types) {
+    public JerseyReturnTypeClassNameVisitor(List<TypeDefinition> types, Set<FeatureFlags> featureFlags) {
         this.delegate = new DefaultClassNameVisitor(types);
+        this.binaryAsResponse = featureFlags.contains(FeatureFlags.JerseyBinaryAsResponse);
     }
 
     @Override
@@ -53,7 +59,11 @@ public TypeName visitOptional(OptionalType type) {
     @Override
     public TypeName visitPrimitive(PrimitiveType type) {
         if (type.get() == PrimitiveType.Value.BINARY) {
-            return ClassName.get(javax.ws.rs.core.StreamingOutput.class);
+            if (binaryAsResponse) {
+                return ClassName.get(Response.class);
+            } else {
+                return ClassName.get(StreamingOutput.class);
+            }
         }
         return delegate.visitPrimitive(type);
     }

conjure-java-core/src/test/java/com/palantir/conjure/java/JerseyServiceGeneratorTests.java

@@ -19,6 +19,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.palantir.conjure.defs.Conjure;
 import com.palantir.conjure.java.services.JerseyServiceGenerator;
 import com.palantir.conjure.spec.ConjureDefinition;
@@ -87,6 +88,27 @@ public void testBinaryReturnInputStream() throws IOException {
         }
     }
 
+    @Test
+    public void testBinaryReturnResponse() throws IOException {
+        ConjureDefinition def = Conjure.parse(
+                ImmutableList.of(new File("src/test/resources/example-binary.yml")));
+        List<Path> files = new JerseyServiceGenerator(ImmutableSet.of(FeatureFlags.JerseyBinaryAsResponse))
+                .emit(def, folder.getRoot());
+
+        for (Path file : files) {
+            if (Boolean.valueOf(System.getProperty("recreate", "false"))) {
+                Path output = Paths.get(
+                        "src/test/resources/test/api/" + file.getFileName() + ".jersey.binary_as_response");
+                Files.delete(output);
+                Files.copy(file, output);
+            }
+
+            assertThat(TestUtils.readFromFile(file)).isEqualTo(
+                    TestUtils.readFromFile(Paths.get(
+                            "src/test/resources/test/api/" + file.getFileName() + ".jersey.binary_as_response")));
+        }
+    }
+
     private void testServiceGeneration(String conjureFile) throws IOException {
         ConjureDefinition def = Conjure.parse(
                 ImmutableList.of(new File("src/test/resources/" + conjureFile + ".yml")));

conjure-java-core/src/test/resources/test/api/TestService.java.jersey.binary_as_response

@@ -0,0 +1,19 @@
+package test.api;
+
+import javax.annotation.Generated;
+import javax.ws.rs.Consumes;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+
+@Consumes(MediaType.APPLICATION_JSON)
+@Produces(MediaType.APPLICATION_JSON)
+@Path("/")
+@Generated("com.palantir.conjure.java.services.JerseyServiceGenerator")
+public interface TestService {
+    @GET
+    @Produces(MediaType.APPLICATION_OCTET_STREAM)
+    Response getBinary();
+}

conjure-java/src/main/java/com/palantir/conjure/java/cli/CliConfiguration.java

@@ -30,6 +30,7 @@ public abstract class CliConfiguration {
     public static final String JERSEY_OPTION = "jersey";
     public static final String RETROFIT_OPTION = "retrofit";
     public static final String RETROFIT_COMPLETABLE_FUTURES = "retrofitCompletableFutures";
+    public static final String JERSEY_BINARY_AS_RESPONSE = "jerseyBinaryAsResponse";
 
     abstract File target();
 
@@ -91,6 +92,9 @@ static CliConfiguration of(String target, String outputDirectory, Option[] optio
                 case RETROFIT_COMPLETABLE_FUTURES:
                     flagsBuilder.add(FeatureFlags.RetrofitCompletableFutures);
                     break;
+                case JERSEY_BINARY_AS_RESPONSE:
+                    flagsBuilder.add(FeatureFlags.JerseyBinaryAsResponse);
+                    break;
                 default:
                     break;
             }

conjure-java/src/main/java/com/palantir/conjure/java/cli/ConjureJavaCli.java

@@ -57,6 +57,8 @@ static CliConfiguration parseCliConfiguration(String[] args) {
                 "Generate retrofit interfaces for streaming/async clients"));
         options.addOption(new Option(CliConfiguration.RETROFIT_COMPLETABLE_FUTURES,
                 "Generate retrofit services which return Java8 CompletableFuture instead of OkHttp Call"));
+        options.addOption(new Option(CliConfiguration.JERSEY_BINARY_AS_RESPONSE,
+                "Generate jersey interfaces which return Response instead of StreamingOutput"));
 
         try {
             CommandLine cmd = parser.parse(options, args, false);
@@ -75,7 +77,7 @@ static void generate(CliConfiguration config) {
         try {
             ConjureDefinition conjureDefinition = OBJECT_MAPPER.readValue(config.target(), ConjureDefinition.class);
             TypeGenerator typeGenerator = new ObjectGenerator();
-            ServiceGenerator jerseyGenerator = new JerseyServiceGenerator();
+            ServiceGenerator jerseyGenerator = new JerseyServiceGenerator(config.featureFlags());
             ServiceGenerator retrofitGenerator = new Retrofit2ServiceGenerator(config.featureFlags());
 
             if (config.generateObjects()) {

conjure-java/src/test/java/com/palantir/conjure/java/cli/ConjureJavaCliTest.java

@@ -64,13 +64,16 @@ public void parseFeatureFlags() {
                 targetFile.getAbsolutePath(),
                 folder.getRoot().getAbsolutePath(),
                 "--objects",
-                "--retrofitCompletableFutures"
+                "--retrofitCompletableFutures",
+                "--jerseyBinaryAsResponse"
         };
         CliConfiguration expectedConfiguration = CliConfiguration.builder()
                 .target(targetFile)
                 .outputDirectory(folder.getRoot())
                 .generateObjects(true)
-                .featureFlags(ImmutableSet.of(FeatureFlags.RetrofitCompletableFutures))
+                .featureFlags(ImmutableSet.of(
+                        FeatureFlags.RetrofitCompletableFutures,
+                        FeatureFlags.JerseyBinaryAsResponse))
                 .build();
         assertThat(ConjureJavaCli.parseCliConfiguration(args)).isEqualTo(expectedConfiguration);
     }