Properly handle optional<binary> endpoint return type and binary-related aliases (#1052)

Author: BenjaminLowry

changelog/@unreleased/pr-1052.v2.yml

@@ -0,0 +1,6 @@
+type: fix
+fix:
+  description: Properly handle optional<binary> endpoint return type and binary-related
+    aliases
+  links:
+  - https://github.com/palantir/conjure-python/pull/1052

conjure-python-core/src/main/java/com/palantir/conjure/python/client/ClientGenerator.java

@@ -31,7 +31,6 @@
 import com.palantir.conjure.python.types.MyPyTypeNameVisitor;
 import com.palantir.conjure.python.types.PythonTypeNameVisitor;
 import com.palantir.conjure.spec.EndpointDefinition;
-import com.palantir.conjure.spec.PrimitiveType;
 import com.palantir.conjure.spec.ServiceDefinition;
 import com.palantir.conjure.spec.Type;
 import com.palantir.conjure.visitor.DealiasingTypeVisitor;
@@ -125,14 +124,29 @@ private PythonEndpointDefinition generateEndpoint(
                 .params(params)
                 .pythonReturnType(endpointDef.getReturns().map(type -> type.accept(pythonTypeNameVisitor)))
                 .myPyReturnType(endpointDef.getReturns().map(type -> type.accept(myPyTypeNameVisitor)))
+                // Set to true iff the de-aliased type is binary.
                 .isRequestBinary(endpointDef.getArgs().stream()
                         .anyMatch(argumentDef -> argumentDef.getParamType().accept(ParameterTypeVisitor.IS_BODY)
-                                && argumentDef.getType().accept(TypeVisitor.IS_BINARY)))
+                                && dealiasingTypeVisitor
+                                        .dealias(argumentDef.getType())
+                                        .fold(_typeDefinition -> false, type -> type.accept(TypeVisitor.IS_BINARY))))
+                // Set to true iff 1) the de-aliased type is binary or 2) the de-aliased type is optional<binary> (both
+                // outer and inner type de-aliased).
                 .isResponseBinary(endpointDef
                         .getReturns()
-                        // We do not need to handle alias of binary since they are treated differently over the wire
-                        .map(rt -> rt.accept(TypeVisitor.IS_PRIMITIVE)
-                                && rt.accept(TypeVisitor.PRIMITIVE).get() == PrimitiveType.Value.BINARY)
+                        .map(rt -> dealiasingTypeVisitor
+                                .dealias(rt)
+                                .fold(
+                                        _typeDefinition -> false,
+                                        type -> type.accept(TypeVisitor.IS_BINARY)
+                                                || (type.accept(TypeVisitor.IS_OPTIONAL)
+                                                        && dealiasingTypeVisitor
+                                                                .dealias(type.accept(TypeVisitor.OPTIONAL)
+                                                                        .getItemType())
+                                                                .fold(
+                                                                        _typeDefinition -> false,
+                                                                        itemType -> itemType.accept(
+                                                                                TypeVisitor.IS_BINARY)))))
                         .orElse(false))
                 .isOptionalReturnType(endpointDef
                         .getReturns()

conjure-python-core/src/main/java/com/palantir/conjure/python/poet/PythonEndpointDefinition.java

@@ -225,22 +225,21 @@ default void emit(PythonPoetWriter poetWriter) {
             poetWriter.decreaseIndent();
 
             poetWriter.writeLine();
+            if (isOptionalReturnType()) {
+                poetWriter.writeIndentedLine("if _response.status_code == 204:");
+                poetWriter.increaseIndent();
+                poetWriter.writeIndentedLine("return None");
+                poetWriter.decreaseIndent();
+            }
             if (isResponseBinary()) {
                 poetWriter.writeIndentedLine("_raw = _response.raw");
                 poetWriter.writeIndentedLine("_raw.decode_content = True");
                 poetWriter.writeIndentedLine("return _raw");
             } else if (pythonReturnType().isPresent()) {
                 poetWriter.writeIndentedLine("_decoder = ConjureDecoder()");
-                if (isOptionalReturnType()) {
-                    poetWriter.writeIndentedLine(
-                            "return None if _response.status_code == 204 else _decoder.decode(_response.json()"
-                                    + ", %s, self._return_none_for_unknown_union_types)",
-                            pythonReturnType().get());
-                } else {
-                    poetWriter.writeIndentedLine(
-                            "return _decoder.decode(_response.json(), %s, self._return_none_for_unknown_union_types)",
-                            pythonReturnType().get());
-                }
+                poetWriter.writeIndentedLine(
+                        "return _decoder.decode(_response.json(), %s, self._return_none_for_unknown_union_types)",
+                        pythonReturnType().get());
             } else {
                 poetWriter.writeIndentedLine("return");
             }

conjure-python-core/src/test/resources/services/expected/package_name/_impl.py

@@ -52,6 +52,12 @@ types:
           - value: PYPI
           - value: CONDA
 
+      BinaryAlias:
+        alias: binary
+
+      OptionalStringAlias:
+        alias: optional<string>
+
 services:
   TestService:
     name: Test Service
@@ -98,6 +104,10 @@ services:
              - Safe
         returns: binary
 
+      getAliasedRawData:
+        http: GET /get-aliased-raw
+        returns: BinaryAlias
+
       maybeGetRawData:
         http: GET /datasets/{datasetRid}/raw-maybe
         args:
@@ -107,13 +117,33 @@ services:
              - Safe
         returns: optional<binary>
 
+      maybeGetAliasedRawData:
+        http: GET /datasets/{datasetRid}/raw-maybe-alias
+        args:
+          datasetRid:
+            type: rid
+            markers:
+              - Safe
+        returns: optional<BinaryAlias>
+
+      getAliasedReturn:
+        http: GET /aliased-return
+        returns: OptionalStringAlias
+
       uploadRawData:
         http: POST /datasets/upload-raw
         args:
           input:
             type: binary
             param-type: body
 
+      uploadAliasedRawData:
+        http: POST /datasets/upload-raw-alias
+        args:
+          input:
+            type: BinaryAlias
+            param-type: body
+
       uploadPythonPackage:
         http: POST /data/python/{uploadType}
         args: