Skip to content

Commit

Permalink
review change
Browse files Browse the repository at this point in the history
  • Loading branch information
Pritham Marupaka committed Nov 5, 2024
1 parent 46d15a8 commit e334bee
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 28 deletions.
7 changes: 6 additions & 1 deletion conjure-api/src/main/conjure/conjure-api.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,12 @@ types:
docs: Should be in lowerCamelCase.
EndpointError:
fields:
error: TypeName
name:
type: string
docs: A reference to an error definition.
package:
type: string
docs: A period-delimited string of package names.
namespace: ErrorNamespace
docs: optional<Documentation>
HttpMethod:
Expand Down
3 changes: 2 additions & 1 deletion conjure-core/src/main/java/com/palantir/conjure/defs/ConjureParserUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,8 @@ private static EndpointDefinition parseEndpoint(
ErrorResolutionResult errorResolutionResult =
endpointErrorResolver.resolve(endpointError.error());
return EndpointError.builder()
.error(errorResolutionResult.typeName())
.name(errorResolutionResult.errorName())
.package_(errorResolutionResult.package_())
.namespace(errorResolutionResult.namespace())
.docs(endpointError.docs().map(Documentation::of))
.build();
Expand Down
11 changes: 5 additions & 6 deletions conjure-core/src/main/java/com/palantir/conjure/defs/EndpointErrorResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ final class EndpointErrorResolver implements ConjureTypeVisitor<ErrorResolutionR
}

record ErrorResolutionResult(
com.palantir.conjure.spec.TypeName typeName, com.palantir.conjure.spec.ErrorNamespace namespace) {}
String errorName, String package_, com.palantir.conjure.spec.ErrorNamespace namespace) {}

ErrorResolutionResult resolve(ConjureType conjureType) {
return conjureType.visit(this);
Expand Down Expand Up @@ -98,11 +98,10 @@ private static ErrorResolutionResult resolveInternal(
throw new SafeIllegalArgumentException("Unknown error", SafeArg.of("error", name.name()));
}
return new ErrorResolutionResult(
com.palantir.conjure.spec.TypeName.of(
name.name(),
ConjureParserUtils.parsePackageOrElseThrow(
errorDefinition.conjurePackage(),
defaultConjurePackage.map(ConjureParserUtils::parseConjurePackage))),
name.name(),
ConjureParserUtils.parsePackageOrElseThrow(
errorDefinition.conjurePackage(),
defaultConjurePackage.map(ConjureParserUtils::parseConjurePackage)),
com.palantir.conjure.spec.ErrorNamespace.of(
errorDefinition.namespace().name()));
}
Expand Down
2 changes: 1 addition & 1 deletion ...e-core/src/main/java/com/palantir/conjure/defs/validator/EndpointDefinitionValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ private static final class NoDuplicateEndpointErrorsValidation implements Conjur
public void validate(EndpointDefinition definition) {
Set<String> endpointErrorNameAndNamespaces = new HashSet<>();
for (EndpointError endpointErrorDef : definition.getErrors()) {
String errorName = endpointErrorDef.getError().getName();
String errorName = endpointErrorDef.getName();
String errorNamespace = endpointErrorDef.getNamespace().get();
String errorUniqueId = errorName + ":" + errorNamespace;
Preconditions.checkArgument(
Expand Down
29 changes: 16 additions & 13 deletions conjure-core/src/test/java/com/palantir/conjure/defs/ConjureDefTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,23 +113,26 @@ public void testEndpointErrorsCanBeImported() {
.satisfies(endpointDefinition -> assertThat(endpointDefinition.getErrors())
.containsExactlyInAnyOrder(
EndpointError.builder()
.error(com.palantir.conjure.spec.TypeName.of(
"Error2", "test.api.with.imported.errors"))
.name("Error2")
.package_("test.api.with.imported.errors")
.namespace(com.palantir.conjure.spec.ErrorNamespace.of(
"TestNamespace"))
.build(),
// The InvalidArgument is imported from the `test.api` package.
EndpointError.of(
com.palantir.conjure.spec.TypeName.of(
"InvalidArgument", "test.api"),
com.palantir.conjure.spec.ErrorNamespace.of("Test"),
Documentation.of("Docs for the imported error")),
EndpointError.of(
com.palantir.conjure.spec.TypeName.of(
"InvalidArgument", "test.api.with.imported.errors"),
com.palantir.conjure.spec.ErrorNamespace.of("OtherNamespace"),
Documentation.of("An error with the same name is imported from "
+ "test-service.yml, but has a different namespace."))));
EndpointError.builder()
.name("InvalidArgument")
.package_("test.api")
.namespace(com.palantir.conjure.spec.ErrorNamespace.of("Test"))
.docs(Documentation.of("Docs for the imported error"))
.build(),
EndpointError.builder()
.name("InvalidArgument")
.package_("test.api.with.imported.errors")
.namespace(com.palantir.conjure.spec.ErrorNamespace.of(
"OtherNamespace"))
.docs(Documentation.of("An error with the same name is imported"
+ " from test-service.yml, but has a different namespace."))
.build()));
});
}
}
14 changes: 8 additions & 6 deletions conjure-core/src/test/java/com/palantir/conjure/defs/validator/EndpointDefinitionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,19 +357,20 @@ public void testNoUnsupportedHttpMethod() {

@Test
public void testDuplicateEndpointErrorsAreInvalid() {
TypeName errorType = TypeName.of("Error1", "test.api");
EndpointDefinition definition = EndpointDefinition.builder()
.endpointName(ENDPOINT_NAME)
.httpMethod(HttpMethod.GET)
.httpPath(HttpPath.of("/get"))
.errors(List.of(
EndpointError.builder()
.error(errorType)
.name("Error1")
.package_("test.api")
.docs(Documentation.of("docs"))
.namespace(ErrorNamespace.of("Test"))
.build(),
EndpointError.builder()
.error(errorType)
.name("Error1")
.package_("test.api")
.namespace(ErrorNamespace.of("Test"))
.docs(Documentation.of("different docs but same error"))
.build()))
Expand All @@ -384,18 +385,19 @@ public void testDuplicateEndpointErrorsAreInvalid() {

@Test
public void testErrorsAreIdentifiedByNameAndNamespace() {
TypeName errorType = TypeName.of("Error1", "test.api");
EndpointDefinition definition = EndpointDefinition.builder()
.endpointName(ENDPOINT_NAME)
.httpMethod(HttpMethod.GET)
.httpPath(HttpPath.of("/get"))
.errors(List.of(
EndpointError.builder()
.error(errorType)
.name("Error1")
.package_("test.api")
.namespace(ErrorNamespace.of("Test"))
.build(),
EndpointError.builder()
.error(errorType)
.name("Error1")
.package_("test.api")
.namespace(ErrorNamespace.of("TestTwo"))
.build()))
.build();
Expand Down

0 comments on commit e334bee

Please sign in to comment.