Skip to content

Commit 05f862d

Browse files
author
Pritham Marupaka
committed
review changes
1 parent e231424 commit 05f862d

File tree

8 files changed

+30
-63
lines changed

8 files changed

+30
-63
lines changed

‎conjure-java-core/src/main/java/com/palantir/conjure/java/services/dialogue/DefaultStaticFactoryMethodGenerator.java‎

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@
1818
import com.google.common.base.CaseFormat;
1919
import com.google.common.collect.ImmutableMap;
2020
import com.palantir.conjure.java.Options;
21+
import com.palantir.conjure.java.api.errors.ConjureErrorParameterFormats;
2122
import com.palantir.conjure.java.services.Auth;
2223
import com.palantir.conjure.java.util.ErrorGenerationUtils;
23-
import com.palantir.conjure.java.util.ErrorGenerationUtils.PackageToErrorDefinitionsMapping;
2424
import com.palantir.conjure.java.util.Packages;
2525
import com.palantir.conjure.java.util.Primitives;
2626
import com.palantir.conjure.spec.ArgumentDefinition;
@@ -82,21 +82,21 @@ public final class DefaultStaticFactoryMethodGenerator implements StaticFactoryM
8282
private final ParameterTypeMapper parameterTypes;
8383
private final ReturnTypeMapper returnTypes;
8484
private final StaticFactoryMethodType methodType;
85-
private final PackageToErrorDefinitionsMapping packageToErrorDefinitionsMapping;
85+
private final List<ErrorDefinition> errorDefinitions;
8686

8787
public DefaultStaticFactoryMethodGenerator(
8888
Options options,
8989
TypeNameResolver typeNameResolver,
9090
ParameterTypeMapper parameterTypes,
9191
ReturnTypeMapper returnTypes,
9292
StaticFactoryMethodType methodType,
93-
PackageToErrorDefinitionsMapping packageToErrorDefinitionsMapping) {
93+
List<ErrorDefinition> errorDefinitions) {
9494
this.options = options;
9595
this.typeNameResolver = typeNameResolver;
9696
this.parameterTypes = parameterTypes;
9797
this.returnTypes = returnTypes;
9898
this.methodType = methodType;
99-
this.packageToErrorDefinitionsMapping = packageToErrorDefinitionsMapping;
99+
this.errorDefinitions = errorDefinitions;
100100
}
101101

102102
@Override
@@ -110,8 +110,7 @@ public MethodSpec generate(ServiceDefinition def) {
110110
.build());
111111

112112
if (options.generateErrorParameterFormatRespectingDialogueInterfaces()) {
113-
impl.addMethod(createHelperToConstructExceptionDeserializerArgs(
114-
def.getServiceName().getPackage()));
113+
impl.addMethod(createHelperToConstructExceptionDeserializerArgs());
115114
}
116115

117116
def.getEndpoints().forEach(endpoint -> {
@@ -142,8 +141,14 @@ public MethodSpec generate(ServiceDefinition def) {
142141
return method;
143142
}
144143

145-
private MethodSpec createHelperToConstructExceptionDeserializerArgs(String serviceDefinitionPackageName) {
146-
List<ErrorDefinition> errorDefinitions = packageToErrorDefinitionsMapping.get(serviceDefinitionPackageName);
144+
private ClassName getClassNameInErrorsPackage(ErrorDefinition errorDef, String name) {
145+
return ClassName.get(
146+
Packages.getPrefixedPackage(errorDef.getErrorName().getPackage(), options.packagePrefix()),
147+
ErrorGenerationUtils.errorTypesClassName(errorDef.getNamespace()),
148+
name);
149+
}
150+
151+
private MethodSpec createHelperToConstructExceptionDeserializerArgs() {
147152
TypeVariableName typeVariableT = TypeVariableName.get("T");
148153
MethodSpec.Builder methodBuilder = MethodSpec.methodBuilder("createExceptionDeserializerArgs")
149154
.addTypeVariable(typeVariableT)
@@ -157,18 +162,10 @@ private MethodSpec createHelperToConstructExceptionDeserializerArgs(String servi
157162
// Add exceptions from error definitions
158163
for (ErrorDefinition errorDef : errorDefinitions) {
159164
String errorName = errorDef.getErrorName().getName();
160-
ClassName errorClass = ClassName.get(
161-
Packages.getPrefixedPackage(errorDef.getErrorName().getPackage(), options.packagePrefix()),
162-
ErrorGenerationUtils.errorTypesClassName(errorDef.getNamespace()),
163-
CaseFormat.UPPER_CAMEL.to(CaseFormat.UPPER_UNDERSCORE, errorName));
164-
ClassName serializableErrorClass = ClassName.get(
165-
Packages.getPrefixedPackage(errorDef.getErrorName().getPackage(), options.packagePrefix()),
166-
ErrorGenerationUtils.errorTypesClassName(errorDef.getNamespace()),
167-
errorName + "SerializableError");
168-
ClassName exceptionClass = ClassName.get(
169-
Packages.getPrefixedPackage(errorDef.getErrorName().getPackage(), options.packagePrefix()),
170-
ErrorGenerationUtils.errorTypesClassName(errorDef.getNamespace()),
171-
errorName + "Exception");
165+
ClassName errorClass = getClassNameInErrorsPackage(
166+
errorDef, CaseFormat.UPPER_CAMEL.to(CaseFormat.UPPER_UNDERSCORE, errorName));
167+
ClassName serializableErrorClass = getClassNameInErrorsPackage(errorDef, errorName + "SerializableError");
168+
ClassName exceptionClass = getClassNameInErrorsPackage(errorDef, errorName + "Exception");
172169
exceptions.add(
173170
".exception($T.name(), new $T<$T>() {}, new $T<$T>() {})",
174171
errorClass,
@@ -308,9 +305,9 @@ private MethodSpec clientImpl(EndpointDefinition def) {
308305
methodBuilder.addCode(CodeBlock.builder()
309306
.beginControlFlow("if ($L.bodySerDe().errorParameterFormat().isPresent())", RUNTIME)
310307
.add(
311-
"$L.putHeaderParams($S," + " $L.bodySerDe().errorParameterFormat().get().toString());",
308+
"$L.putHeaderParams($S, $L.bodySerDe().errorParameterFormat().get().toString());",
312309
REQUEST,
313-
"Accept-Conjure-Error-Parameter-Format", // TODO(pm): make this public.
310+
ConjureErrorParameterFormats.ACCEPT_CONJURE_ERROR_PARAMETER_FORMAT_HEADER,
314311
RUNTIME)
315312
.endControlFlow()
316313
.build());

‎conjure-java-core/src/main/java/com/palantir/conjure/java/services/dialogue/DialogueServiceGenerator.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import com.palantir.conjure.java.types.SafetyEvaluator;
2323
import com.palantir.conjure.java.types.SpecializeBinaryClassNameVisitor;
2424
import com.palantir.conjure.java.types.TypeMapper;
25-
import com.palantir.conjure.java.util.ErrorGenerationUtils.PackageToErrorDefinitionsMapping;
2625
import com.palantir.conjure.java.util.TypeFunctions;
2726
import com.palantir.conjure.spec.ConjureDefinition;
2827
import com.palantir.conjure.spec.ServiceDefinition;
@@ -71,23 +70,21 @@ public Stream<JavaFile> generate(ConjureDefinition conjureDefinition) {
7170
TypeNameResolver typeNameResolver = typeName -> Preconditions.checkNotNull(
7271
types.get(typeName), "Referenced unknown TypeName", SafeArg.of("typeName", typeName));
7372

74-
PackageToErrorDefinitionsMapping packageToErrorDefinitionsMapping =
75-
PackageToErrorDefinitionsMapping.from(conjureDefinition);
7673
StaticFactoryMethodGenerator asyncGenerator = new DefaultStaticFactoryMethodGenerator(
7774
options,
7875
typeNameResolver,
7976
parameterMapper,
8077
new ReturnTypeMapper(returnTypes),
8178
StaticFactoryMethodType.ASYNC,
82-
packageToErrorDefinitionsMapping);
79+
conjureDefinition.getErrors());
8380

8481
StaticFactoryMethodGenerator blockingGenerator = new DefaultStaticFactoryMethodGenerator(
8582
options,
8683
typeNameResolver,
8784
parameterMapper,
8885
new ReturnTypeMapper(returnTypes),
8986
StaticFactoryMethodType.BLOCKING,
90-
packageToErrorDefinitionsMapping);
87+
conjureDefinition.getErrors());
9188

9289
return conjureDefinition.getServices().stream()
9390
.flatMap(serviceDef -> generateFilesForService(

‎conjure-java-core/src/main/java/com/palantir/conjure/java/util/ErrorGenerationUtils.java‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -67,32 +67,6 @@ public static String errorParametersClassName(String errorName) {
6767
return errorName + "Parameters";
6868
}
6969

70-
public static ParameterSpec buildParameterWithSafetyAnnotationAndJsonProperty(
71-
TypeMapper typeMapper, FieldDefinition argDefinition, boolean isSafe) {
72-
ParameterSpec.Builder parameterBuilder =
73-
buildParameterWithSafetyAnnotationInternal(typeMapper, argDefinition, isSafe, true);
74-
parameterBuilder.addAnnotation(AnnotationSpec.builder(JsonProperty.class)
75-
.addMember("value", "$S", argDefinition.getFieldName().get())
76-
.build());
77-
return parameterBuilder.build();
78-
}
79-
80-
/**
81-
* A mapping from a package name to the list of errors defined within that package. This is used when attempting to
82-
* deserialize error responses from endpoints.
83-
*/
84-
public record PackageToErrorDefinitionsMapping(Map<String, List<ErrorDefinition>> packageToErrors) {
85-
public static PackageToErrorDefinitionsMapping from(ConjureDefinition definition) {
86-
Map<String, List<ErrorDefinition>> fromDefinition = definition.getErrors().stream()
87-
.collect(Collectors.groupingBy(error -> error.getErrorName().getPackage(), Collectors.toList()));
88-
return new PackageToErrorDefinitionsMapping(fromDefinition);
89-
}
90-
91-
public List<ErrorDefinition> get(String packageName) {
92-
return packageToErrors.getOrDefault(packageName, ImmutableList.of());
93-
}
94-
}
95-
9670
public record DeclaredEndpointErrors(Set<ErrorTypeName> errors) {
9771
public static DeclaredEndpointErrors from(ConjureDefinition definition) {
9872
return new DeclaredEndpointErrors(definition.getServices().stream()

‎conjure-java-core/src/test/java/com/palantir/conjure/java/parameterized/ParameterizedConjureGenerationTest.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
import java.util.Set;
3434
import java.util.stream.Collectors;
3535
import java.util.stream.Stream;
36-
import org.junit.jupiter.api.Disabled;
3736
import org.junit.jupiter.api.MethodOrderer;
3837
import org.junit.jupiter.api.Order;
3938
import org.junit.jupiter.api.Test;
@@ -69,7 +68,6 @@ void testGeneratedCode(ParameterizedTestCase testCase) throws IOException {
6968

7069
@Test
7170
@Order(2)
72-
@Disabled
7371
void validateOnlyRegisteredPackagesPresent() {
7472
File referenceFilesFolder = new File(REFERENCE_FILES_FOLDER);
7573
File[] files = referenceFilesFolder.listFiles();

‎conjure-java-core/src/test/resources/ete-service.yml‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,9 @@ services:
220220
errorParameterSerialization:
221221
http: GET /errors/serialization
222222
docs: |
223-
This endpoint is used to test that that error parameters serialized as JSON or using `Objects.toString` are
224-
both handled correctly by the clients that return "rich" exceptions (which are sub-types of `RemoteException`).
223+
This endpoint is used to test that error parameters serialized as JSON or using `Objects.toString` are
224+
both handled correctly by the clients that can deserialize "rich" exceptions (which are sub-types of
225+
`RemoteException`).
225226
args:
226227
headerParameter:
227228
param-type: header

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ public static final class GenerateCommand implements Runnable {
252252
description =
253253
"Enabling this flag will lead to the generation of Dialogue client interfaces that are able to"
254254
+ " send a header to servers specifying the Conjure error parameter serialization format"
255-
+ " they expect, and create custom exceptions foreach error defined in the service's"
255+
+ " they expect, and create custom exceptions for each error defined in the service's"
256256
+ " package.")
257257
private boolean generateErrorParameterFormatRespectingDialogueInterfaces;
258258

‎versions.lock‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,11 @@ com.palantir.conjure:conjure-api-objects:4.50.0 (3 constraints: 9327531f)
4444

4545
com.palantir.conjure:conjure-generator-common:4.50.0 (2 constraints: fd13a782)
4646

47-
com.palantir.conjure.java.api:errors:2.63.0 (8 constraints: d4954976)
47+
com.palantir.conjure.java.api:errors:2.64.0 (8 constraints: a394547a)
4848

49-
com.palantir.conjure.java.api:service-config:2.63.0 (6 constraints: 6967b93b)
49+
com.palantir.conjure.java.api:service-config:2.64.0 (6 constraints: 37668fd8)
5050

51-
com.palantir.conjure.java.api:ssl-config:2.63.0 (5 constraints: e6507242)
51+
com.palantir.conjure.java.api:ssl-config:2.64.0 (5 constraints: b54faa2d)
5252

5353
com.palantir.conjure.java.runtime:client-config:8.24.0 (6 constraints: ae6b4e29)
5454

@@ -240,7 +240,7 @@ com.netflix.feign:feign-jackson:8.18.0 (1 constraints: c718909e)
240240

241241
com.palantir.conjure:conjure-core:4.50.0 (1 constraints: 3b054b3b)
242242

243-
com.palantir.conjure.java.api:test-utils:2.63.0 (1 constraints: 70064153)
243+
com.palantir.conjure.java.api:test-utils:2.64.0 (1 constraints: 3e054f3b)
244244

245245
com.palantir.conjure.java.runtime:conjure-java-annotations:8.24.0 (1 constraints: c418889e)
246246

‎versions.props‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ com.palantir.conjure.verification:* = 0.19.0
1111
com.palantir.conjure:* = 4.50.0
1212
com.palantir.deadlines:* = 0.15.0
1313
com.palantir.goethe:* = 0.16.0
14-
com.palantir.conjure.java.api:* = 2.63.0-rc1
14+
com.palantir.conjure.java.api:* = 2.64.0
1515
com.palantir.dialogue:* = 6.8.0-7-gde83dba
1616
com.palantir.human-readable-types:* = 1.7.0
1717
com.palantir.javapoet:javapoet = 0.7.0

0 commit comments

Comments
 (0)