review changes

Author: Pritham Marupaka

conjure-java-core/src/integrationInput/java/dialogue/com/palantir/product/DialogueEteEndpoints.java

@@ -18,9 +18,9 @@
 import com.google.common.base.CaseFormat;
 import com.google.common.collect.ImmutableMap;
 import com.palantir.conjure.java.Options;
+import com.palantir.conjure.java.api.errors.ConjureErrorParameterFormats;
 import com.palantir.conjure.java.services.Auth;
 import com.palantir.conjure.java.util.ErrorGenerationUtils;
-import com.palantir.conjure.java.util.ErrorGenerationUtils.PackageToErrorDefinitionsMapping;
 import com.palantir.conjure.java.util.Packages;
 import com.palantir.conjure.java.util.Primitives;
 import com.palantir.conjure.spec.ArgumentDefinition;
@@ -82,21 +82,21 @@ public final class DefaultStaticFactoryMethodGenerator implements StaticFactoryM
     private final ParameterTypeMapper parameterTypes;
     private final ReturnTypeMapper returnTypes;
     private final StaticFactoryMethodType methodType;
-    private final PackageToErrorDefinitionsMapping packageToErrorDefinitionsMapping;
+    private final List<ErrorDefinition> errorDefinitions;
 
     public DefaultStaticFactoryMethodGenerator(
             Options options,
             TypeNameResolver typeNameResolver,
             ParameterTypeMapper parameterTypes,
             ReturnTypeMapper returnTypes,
             StaticFactoryMethodType methodType,
-            PackageToErrorDefinitionsMapping packageToErrorDefinitionsMapping) {
+            List<ErrorDefinition> errorDefinitions) {
         this.options = options;
         this.typeNameResolver = typeNameResolver;
         this.parameterTypes = parameterTypes;
         this.returnTypes = returnTypes;
         this.methodType = methodType;
-        this.packageToErrorDefinitionsMapping = packageToErrorDefinitionsMapping;
+        this.errorDefinitions = errorDefinitions;
     }
 
     @Override
@@ -110,8 +110,7 @@ public MethodSpec generate(ServiceDefinition def) {
                 .build());
 
         if (options.generateErrorParameterFormatRespectingDialogueInterfaces()) {
-            impl.addMethod(createHelperToConstructExceptionDeserializerArgs(
-                    def.getServiceName().getPackage()));
+            impl.addMethod(createHelperToConstructExceptionDeserializerArgs());
         }
 
         def.getEndpoints().forEach(endpoint -> {
@@ -142,8 +141,14 @@ public MethodSpec generate(ServiceDefinition def) {
         return method;
     }
 
-    private MethodSpec createHelperToConstructExceptionDeserializerArgs(String serviceDefinitionPackageName) {
-        List<ErrorDefinition> errorDefinitions = packageToErrorDefinitionsMapping.get(serviceDefinitionPackageName);
+    private ClassName getClassNameInErrorsPackage(ErrorDefinition errorDef, String name) {
+        return ClassName.get(
+                Packages.getPrefixedPackage(errorDef.getErrorName().getPackage(), options.packagePrefix()),
+                ErrorGenerationUtils.errorTypesClassName(errorDef.getNamespace()),
+                name);
+    }
+
+    private MethodSpec createHelperToConstructExceptionDeserializerArgs() {
         TypeVariableName typeVariableT = TypeVariableName.get("T");
         MethodSpec.Builder methodBuilder = MethodSpec.methodBuilder("createExceptionDeserializerArgs")
                 .addTypeVariable(typeVariableT)
@@ -157,18 +162,10 @@ private MethodSpec createHelperToConstructExceptionDeserializerArgs(String servi
         // Add exceptions from error definitions
         for (ErrorDefinition errorDef : errorDefinitions) {
             String errorName = errorDef.getErrorName().getName();
-            ClassName errorClass = ClassName.get(
-                    Packages.getPrefixedPackage(errorDef.getErrorName().getPackage(), options.packagePrefix()),
-                    ErrorGenerationUtils.errorTypesClassName(errorDef.getNamespace()),
-                    CaseFormat.UPPER_CAMEL.to(CaseFormat.UPPER_UNDERSCORE, errorName));
-            ClassName serializableErrorClass = ClassName.get(
-                    Packages.getPrefixedPackage(errorDef.getErrorName().getPackage(), options.packagePrefix()),
-                    ErrorGenerationUtils.errorTypesClassName(errorDef.getNamespace()),
-                    errorName + "SerializableError");
-            ClassName exceptionClass = ClassName.get(
-                    Packages.getPrefixedPackage(errorDef.getErrorName().getPackage(), options.packagePrefix()),
-                    ErrorGenerationUtils.errorTypesClassName(errorDef.getNamespace()),
-                    errorName + "Exception");
+            ClassName errorClass = getClassNameInErrorsPackage(
+                    errorDef, CaseFormat.UPPER_CAMEL.to(CaseFormat.UPPER_UNDERSCORE, errorName));
+            ClassName serializableErrorClass = getClassNameInErrorsPackage(errorDef, errorName + "SerializableError");
+            ClassName exceptionClass = getClassNameInErrorsPackage(errorDef, errorName + "Exception");
             exceptions.add(
                     ".exception($T.name(), new $T<$T>() {}, new $T<$T>() {})",
                     errorClass,
@@ -308,9 +305,9 @@ private MethodSpec clientImpl(EndpointDefinition def) {
             methodBuilder.addCode(CodeBlock.builder()
                     .beginControlFlow("if ($L.bodySerDe().errorParameterFormat().isPresent())", RUNTIME)
                     .add(
-                            "$L.putHeaderParams($S," + " $L.bodySerDe().errorParameterFormat().get().toString());",
+                            "$L.putHeaderParams($S, $L.bodySerDe().errorParameterFormat().get().toString());",
                             REQUEST,
-                            "Accept-Conjure-Error-Parameter-Format", // TODO(pm): make this public.
+                            ConjureErrorParameterFormats.ACCEPT_CONJURE_ERROR_PARAMETER_FORMAT_HEADER,
                             RUNTIME)
                     .endControlFlow()
                     .build());

conjure-java-core/src/integrationInput/java/dialogue/com/palantir/product/EteServiceAsync.java

@@ -22,7 +22,6 @@
 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.PackageToErrorDefinitionsMapping;
 import com.palantir.conjure.java.util.TypeFunctions;
 import com.palantir.conjure.spec.ConjureDefinition;
 import com.palantir.conjure.spec.ServiceDefinition;
@@ -71,23 +70,21 @@ public Stream<JavaFile> generate(ConjureDefinition conjureDefinition) {
         TypeNameResolver typeNameResolver = typeName -> Preconditions.checkNotNull(
                 types.get(typeName), "Referenced unknown TypeName", SafeArg.of("typeName", typeName));
 
-        PackageToErrorDefinitionsMapping packageToErrorDefinitionsMapping =
-                PackageToErrorDefinitionsMapping.from(conjureDefinition);
         StaticFactoryMethodGenerator asyncGenerator = new DefaultStaticFactoryMethodGenerator(
                 options,
                 typeNameResolver,
                 parameterMapper,
                 new ReturnTypeMapper(returnTypes),
                 StaticFactoryMethodType.ASYNC,
-                packageToErrorDefinitionsMapping);
+                conjureDefinition.getErrors());
 
         StaticFactoryMethodGenerator blockingGenerator = new DefaultStaticFactoryMethodGenerator(
                 options,
                 typeNameResolver,
                 parameterMapper,
                 new ReturnTypeMapper(returnTypes),
                 StaticFactoryMethodType.BLOCKING,
-                packageToErrorDefinitionsMapping);
+                conjureDefinition.getErrors());
 
         return conjureDefinition.getServices().stream()
                 .flatMap(serviceDef -> generateFilesForService(

conjure-java-core/src/integrationInput/java/dialogue/com/palantir/product/EteServiceBlocking.java

@@ -67,32 +67,6 @@ public static String errorParametersClassName(String errorName) {
         return errorName + "Parameters";
     }
 
-    public static ParameterSpec buildParameterWithSafetyAnnotationAndJsonProperty(
-            TypeMapper typeMapper, FieldDefinition argDefinition, boolean isSafe) {
-        ParameterSpec.Builder parameterBuilder =
-                buildParameterWithSafetyAnnotationInternal(typeMapper, argDefinition, isSafe, true);
-        parameterBuilder.addAnnotation(AnnotationSpec.builder(JsonProperty.class)
-                .addMember("value", "$S", argDefinition.getFieldName().get())
-                .build());
-        return parameterBuilder.build();
-    }
-
-    /**
-     * A mapping from a package name to the list of errors defined within that package. This is used when attempting to
-     * deserialize error responses from endpoints.
-     */
-    public record PackageToErrorDefinitionsMapping(Map<String, List<ErrorDefinition>> packageToErrors) {
-        public static PackageToErrorDefinitionsMapping from(ConjureDefinition definition) {
-            Map<String, List<ErrorDefinition>> fromDefinition = definition.getErrors().stream()
-                    .collect(Collectors.groupingBy(error -> error.getErrorName().getPackage(), Collectors.toList()));
-            return new PackageToErrorDefinitionsMapping(fromDefinition);
-        }
-
-        public List<ErrorDefinition> get(String packageName) {
-            return packageToErrors.getOrDefault(packageName, ImmutableList.of());
-        }
-    }
-
     public record DeclaredEndpointErrors(Set<ErrorTypeName> errors) {
         public static DeclaredEndpointErrors from(ConjureDefinition definition) {
             return new DeclaredEndpointErrors(definition.getServices().stream()

conjure-java-core/src/integrationInput/java/jersey/com/palantir/product/EteService.java

@@ -33,7 +33,6 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.MethodOrderer;
 import org.junit.jupiter.api.Order;
 import org.junit.jupiter.api.Test;
@@ -69,7 +68,6 @@ void testGeneratedCode(ParameterizedTestCase testCase) throws IOException {
 
     @Test
     @Order(2)
-    @Disabled
     void validateOnlyRegisteredPackagesPresent() {
         File referenceFilesFolder = new File(REFERENCE_FILES_FOLDER);
         File[] files = referenceFilesFolder.listFiles();

conjure-java-core/src/integrationInput/java/undertow/com/palantir/product/UndertowEteService.java

@@ -220,8 +220,9 @@ services:
       errorParameterSerialization:
         http: GET /errors/serialization
         docs: |
-          This endpoint is used to test that that error parameters serialized as JSON or using `Objects.toString` are 
-          both handled correctly by the clients that return "rich" exceptions (which are sub-types of `RemoteException`).
+          This endpoint is used to test that error parameters serialized as JSON or using `Objects.toString` are 
+          both handled correctly by the clients that can deserialize "rich" exceptions (which are sub-types of 
+          `RemoteException`).
         args:
           headerParameter:
             param-type: header