Don't create parameter records when there are no parameters for error

Author: Pritham Marupaka

conjure-java-core/src/integrationInput/java/com/palantir/another/EndpointSpecificErrors.java

@@ -179,8 +179,9 @@ default boolean excludeDialogueAsyncInterfaces() {
     }
 
     /**
-     * TODO(pm): flesh this out more. For errors associated with endpoints, this option enables the generated Dialogue
-     *  clients to return a result type.
+     * If enabled, endpoints that have associated errors will return a result type: a sealed interface permitting
+     * subclasses for the endpoint's return value, and each endpoint error. Each endpoint error is a subclass of
+     * {@link com.palantir.conjure.java.lib.internal.ConjureErrors.BaseEndpointError}.
      */
     @Value.Default
     default boolean generateDialogueEndpointErrorResultTypes() {

conjure-java-core/src/integrationInput/java/com/palantir/product/EndpointSpecificTwoErrors.java

@@ -181,13 +181,18 @@ private Optional<FieldSpec> deserializer(
         if (isBinaryOrOptionalBinary(className, returnTypes) && !options.generateDialogueEndpointErrorResultTypes()) {
             return Optional.empty();
         }
-        ParameterizedTypeName deserializerType =
-                ParameterizedTypeName.get(ClassName.get(Deserializer.class), responseType);
+
+        ParameterizedTypeName deserializerType;
+        if (shouldGenerateDialogueEndpointErrorResultTypesForEndpoint(options, endpointDef)) {
+            deserializerType = ParameterizedTypeName.get(ClassName.get(Deserializer.class), responseType);
+        } else {
+            deserializerType = ParameterizedTypeName.get(ClassName.get(Deserializer.class), className);
+        }
 
         CodeBlock initializer = CodeBlock.of(
                 "$L.bodySerDe().$L",
                 StaticFactoryMethodGenerator.RUNTIME,
-                options.generateDialogueEndpointErrorResultTypes()
+                shouldGenerateDialogueEndpointErrorResultTypesForEndpoint(options, endpointDef)
                         ? constructDeserializerWithEndpointErrors(endpointDef, className, responseType)
                         : constructDeserializer(type, className));
 
@@ -239,6 +244,12 @@ private static boolean isBinaryOrOptionalBinary(TypeName className, ReturnTypeMa
         return isBinary(className, returnTypes) || isOptionalBinary(className, returnTypes);
     }
 
+    private static boolean shouldGenerateDialogueEndpointErrorResultTypesForEndpoint(
+            Options options, EndpointDefinition endpointDefinition) {
+        return options.generateDialogueEndpointErrorResultTypes()
+                && !endpointDefinition.getErrors().isEmpty();
+    }
+
     private static boolean isBinary(TypeName className, ReturnTypeMapper returnTypes) {
         return className.equals(returnTypes.baseType(Type.primitive(PrimitiveType.BINARY)));
     }
@@ -280,13 +291,14 @@ private MethodSpec clientImpl(ClassName className, EndpointDefinition def) {
                 .build();
         String codeBlock = methodType.switchBy(
                 "$L.clients().callBlocking($L, $L.build(), $L);", "$L.clients().call($L, $L.build" + "(), $L);");
+        boolean generateResultTypes = shouldGenerateDialogueEndpointErrorResultTypesForEndpoint(options, def);
         CodeBlock execute = CodeBlock.of(
                 codeBlock,
                 StaticFactoryMethodGenerator.RUNTIME,
                 Names.endpointChannel(def),
                 REQUEST,
                 def.getReturns()
-                        .filter(type -> !options.generateDialogueEndpointErrorResultTypes()
+                        .filter(type -> !generateResultTypes
                                 && isBinaryOrOptionalBinary(returnTypes.baseType(type), returnTypes))
                         .map(type -> StaticFactoryMethodGenerator.RUNTIME
                                 + (isOptionalBinary(returnTypes.baseType(type), returnTypes)
@@ -295,20 +307,15 @@ && isBinaryOrOptionalBinary(returnTypes.baseType(type), returnTypes))
                         .orElseGet(() -> def.getEndpointName().get() + "Deserializer"));
 
         methodBuilder.addCode(request);
-        methodBuilder.addCode(methodType.switchBy(
-                def.getReturns().isPresent()
-                                || (options.generateDialogueEndpointErrorResultTypes()
-                                        && !def.getErrors().isEmpty())
-                        ? "return "
-                        : "",
-                "return "));
+        methodBuilder.addCode(
+                methodType.switchBy(def.getReturns().isPresent() || generateResultTypes ? "return " : "", "return "));
         methodBuilder.addCode(execute);
 
         return methodBuilder.build();
     }
 
     private TypeName getReturnType(EndpointDefinition def, ClassName className) {
-        if (options.generateDialogueEndpointErrorResultTypes()
+        if (shouldGenerateDialogueEndpointErrorResultTypesForEndpoint(options, def)
                 && !def.getErrors().isEmpty()) {
             ClassName returnType = ClassName.get(
                     className.packageName(),

conjure-java-core/src/integrationInput/java/com/palantir/product/ErrorServiceAsync.java

@@ -25,16 +25,19 @@
 import com.google.errorprone.annotations.MustBeClosed;
 import com.palantir.conjure.java.ConjureAnnotations;
 import com.palantir.conjure.java.Options;
+import com.palantir.conjure.java.lib.internal.ConjureErrors;
 import com.palantir.conjure.java.lib.internal.ConjureErrors.BaseEndpointError;
 import com.palantir.conjure.java.services.IsUndertowAsyncMarkerVisitor;
 import com.palantir.conjure.java.services.ServiceGenerators;
 import com.palantir.conjure.java.services.ServiceGenerators.EndpointErrorsJavaDoc;
 import com.palantir.conjure.java.services.ServiceGenerators.EndpointJavaDocGenerationOptions;
 import com.palantir.conjure.java.services.ServiceGenerators.RequestLineJavaDoc;
 import com.palantir.conjure.java.util.ErrorGenerationUtils;
+import com.palantir.conjure.java.util.ErrorGenerationUtils.ErrorsAndParameters;
 import com.palantir.conjure.java.util.Packages;
 import com.palantir.conjure.spec.EndpointDefinition;
 import com.palantir.conjure.spec.EndpointError;
+import com.palantir.conjure.spec.ErrorTypeName;
 import com.palantir.conjure.spec.ServiceDefinition;
 import com.palantir.conjure.visitor.TypeVisitor;
 import com.palantir.dialogue.Channel;
@@ -66,12 +69,17 @@ public final class DialogueInterfaceGenerator {
     private final Options options;
     private final ParameterTypeMapper parameterTypes;
     private final ReturnTypeMapper returnTypes;
+    private final ErrorsAndParameters errorsAndParameters;
 
     public DialogueInterfaceGenerator(
-            Options options, ParameterTypeMapper parameterTypes, ReturnTypeMapper returnTypes) {
+            Options options,
+            ParameterTypeMapper parameterTypes,
+            ReturnTypeMapper returnTypes,
+            ErrorsAndParameters errorsAndParameters) {
         this.options = options;
         this.parameterTypes = parameterTypes;
         this.returnTypes = returnTypes;
+        this.errorsAndParameters = errorsAndParameters;
     }
 
     public JavaFile generateBlocking(
@@ -190,29 +198,7 @@ private TypeSpec responseTypeForEndpoint(String packageName, ClassName className
         // Create a record for each of the endpoint's errors
         List<TypeSpec> errorTypes = new ArrayList<>();
         for (EndpointError endpointError : endpointDef.getErrors()) {
-            String errorTypeName = CaseFormat.UPPER_CAMEL.to(
-                    CaseFormat.UPPER_UNDERSCORE, endpointError.getError().getName());
-            ClassName errorTypesClassName = ClassName.get(
-                    endpointError.getError().getPackage(),
-                    ErrorGenerationUtils.errorTypesClassName(
-                            endpointError.getError().getNamespace()),
-                    errorTypeName);
-            ClassName parametersClassName = ClassName.get(
-                    endpointError.getError().getPackage(),
-                    ErrorGenerationUtils.errorTypesClassName(
-                            endpointError.getError().getNamespace()),
-                    ErrorGenerationUtils.errorParametersClassName(
-                            endpointError.getError().getName()));
-            TypeSpec endpointErrorType = TypeSpec.classBuilder(ClassName.get(
-                            packageName,
-                            responseTypeName.simpleName(),
-                            endpointError.getError().getName()))
-                    .addModifiers(Modifier.PUBLIC, Modifier.STATIC, Modifier.FINAL)
-                    .addSuperinterface(responseTypeName)
-                    .superclass(ParameterizedTypeName.get(ClassName.get(BaseEndpointError.class), parametersClassName))
-                    .addMethod(errorTypeConstructor(parametersClassName, errorTypesClassName))
-                    .build();
-            errorTypes.add(endpointErrorType);
+            errorTypes.add(constructEndpointErrorType(endpointError, packageName, responseTypeName));
         }
 
         // Get type from typespec
@@ -229,6 +215,39 @@ private TypeSpec responseTypeForEndpoint(String packageName, ClassName className
                 .build();
     }
 
+    private TypeSpec constructEndpointErrorType(
+            EndpointError endpointError, String packageName, ClassName responseTypeName) {
+        String errorTypeName = CaseFormat.UPPER_CAMEL.to(
+                CaseFormat.UPPER_UNDERSCORE, endpointError.getError().getName());
+        ClassName errorTypesClassName = ClassName.get(
+                endpointError.getError().getPackage(),
+                ErrorGenerationUtils.errorTypesClassName(
+                        endpointError.getError().getNamespace()),
+                errorTypeName);
+
+        TypeSpec.Builder endpointErrorTypeBuilder = TypeSpec.classBuilder(ClassName.get(
+                        packageName,
+                        responseTypeName.simpleName(),
+                        endpointError.getError().getName()))
+                .addModifiers(Modifier.PUBLIC, Modifier.STATIC, Modifier.FINAL)
+                .addSuperinterface(responseTypeName);
+        ClassName parametersClassName;
+        if (errorsAndParameters.hasParameters(endpointError.getError())) {
+            parametersClassName = ClassName.get(
+                    endpointError.getError().getPackage(),
+                    ErrorGenerationUtils.errorTypesClassName(
+                            endpointError.getError().getNamespace()),
+                    ErrorGenerationUtils.errorParametersClassName(
+                            endpointError.getError().getName()));
+        } else {
+            parametersClassName = ClassName.get(ConjureErrors.EmptyErrorParameters.class);
+        }
+        endpointErrorTypeBuilder
+                .superclass(ParameterizedTypeName.get(ClassName.get(BaseEndpointError.class), parametersClassName))
+                .addMethod(errorTypeConstructor(endpointError.getError(), parametersClassName, errorTypesClassName));
+        return endpointErrorTypeBuilder.build();
+    }
+
     private TypeSpec createSuccessRecord(
             String packageName, ClassName className, ClassName responseTypeName, EndpointDefinition endpointDef) {
         ClassName successTypeClassName =
@@ -263,7 +282,8 @@ private TypeSpec createSuccessRecord(
         return successRecordBuilder.build();
     }
 
-    private MethodSpec errorTypeConstructor(ClassName parametersClassName, ClassName errorTypesClassName) {
+    private MethodSpec errorTypeConstructor(
+            ErrorTypeName errorTypeName, ClassName parametersClassName, ClassName errorTypesClassName) {
         MethodSpec.Builder ctorBuilder = MethodSpec.constructorBuilder()
                 .addAnnotation(JsonCreator.class)
                 .addParameter(ParameterSpec.builder(ClassName.get(String.class), "errorCode")
@@ -275,16 +295,20 @@ private MethodSpec errorTypeConstructor(ClassName parametersClassName, ClassName
                         .addAnnotation(AnnotationSpec.builder(JsonProperty.class)
                                 .addMember("value", "$S", "errorInstanceId")
                                 .build())
-                        .build())
-                // TODO(pm): Only add the parameters if the error has them. Plumb in error definitions.
-                .addParameter(ParameterSpec.builder(parametersClassName, "parameters")
-                        .addAnnotation(AnnotationSpec.builder(JsonProperty.class)
-                                .addMember("value", "$S", "parameters")
-                                .build())
-                        .build())
-                .addStatement(
-                        // TODO(pm): Perhaps these shouldn't be literals but names ($N).
-                        "super(errorCode, $T.name(), errorInstanceId, parameters)", errorTypesClassName);
+                        .build());
+        if (errorsAndParameters.hasParameters(errorTypeName)) {
+            ctorBuilder.addParameter(ParameterSpec.builder(parametersClassName, "parameters")
+                    .addAnnotation(AnnotationSpec.builder(JsonProperty.class)
+                            .addMember("value", "$S", "parameters")
+                            .build())
+                    .build());
+            ctorBuilder.addStatement("super(errorCode, $T.name(), errorInstanceId, parameters)", errorTypesClassName);
+        } else {
+            ctorBuilder.addStatement(
+                    "super(errorCode, $T.name(), errorInstanceId, $T.EMPTY_ERROR_PARAMETERS_INSTANCE)",
+                    errorTypesClassName,
+                    ClassName.get(ConjureErrors.class));
+        }
         return ctorBuilder.build();
     }
 

conjure-java-core/src/integrationInput/java/com/palantir/product/ErrorServiceBlocking.java

@@ -22,6 +22,7 @@
 import com.palantir.conjure.java.types.SafetyEvaluator;
 import com.palantir.conjure.java.types.SpecializeBinaryClassNameVisitor;
 import com.palantir.conjure.java.types.TypeMapper;
+import com.palantir.conjure.java.util.ErrorGenerationUtils.ErrorsAndParameters;
 import com.palantir.conjure.java.util.TypeFunctions;
 import com.palantir.conjure.spec.ConjureDefinition;
 import com.palantir.conjure.spec.ServiceDefinition;
@@ -64,8 +65,9 @@ public Stream<JavaFile> generate(ConjureDefinition conjureDefinition) {
                         new DefaultClassNameVisitor(types.keySet(), options), types, ClassName.get(InputStream.class)));
         ParameterTypeMapper parameterMapper = new ParameterTypeMapper(parameterTypes, safetyEvaluator);
 
-        DialogueInterfaceGenerator interfaceGenerator =
-                new DialogueInterfaceGenerator(options, parameterMapper, new ReturnTypeMapper(returnTypes));
+        ErrorsAndParameters errorsAndParameters = ErrorsAndParameters.from(conjureDefinition);
+        DialogueInterfaceGenerator interfaceGenerator = new DialogueInterfaceGenerator(
+                options, parameterMapper, new ReturnTypeMapper(returnTypes), errorsAndParameters);
 
         TypeNameResolver typeNameResolver = typeName -> Preconditions.checkNotNull(
                 types.get(typeName), "Referenced unknown TypeName", SafeArg.of("typeName", typeName));