diff --git a/conjure-core/src/main/java/com/palantir/conjure/defs/validator/EndpointDefinitionValidator.java b/conjure-core/src/main/java/com/palantir/conjure/defs/validator/EndpointDefinitionValidator.java index 641e80336..eab013aa9 100644 --- a/conjure-core/src/main/java/com/palantir/conjure/defs/validator/EndpointDefinitionValidator.java +++ b/conjure-core/src/main/java/com/palantir/conjure/defs/validator/EndpointDefinitionValidator.java @@ -25,6 +25,7 @@ import com.palantir.conjure.spec.ArgumentDefinition; import com.palantir.conjure.spec.ArgumentName; import com.palantir.conjure.spec.EndpointDefinition; +import com.palantir.conjure.spec.EndpointError; import com.palantir.conjure.spec.ExternalReference; import com.palantir.conjure.spec.HttpMethod; import com.palantir.conjure.spec.ListType; @@ -64,7 +65,8 @@ public enum EndpointDefinitionValidator implements ConjureContextualValidator { + @Override + public void validate(EndpointDefinition definition) { + Set endpointErrorNames = new HashSet<>(); + for (EndpointError endpointErrorDef : definition.getErrors()) { + String errorName = endpointErrorDef.getError().getName(); + Preconditions.checkArgument( + endpointErrorNames.add(errorName), + "Error '%s' is declared multiple times in endpoint '%s'", + errorName, + describe(definition)); + } + } + } + private static String describe(EndpointDefinition endpoint) { return String.format( "%s{http: %s %s}", endpoint.getEndpointName(), endpoint.getHttpMethod(), endpoint.getHttpPath()); diff --git a/conjure-core/src/main/java/com/palantir/conjure/parser/services/EndpointError.java b/conjure-core/src/main/java/com/palantir/conjure/parser/services/EndpointError.java index 65aca2f9c..cc6231efc 100644 --- a/conjure-core/src/main/java/com/palantir/conjure/parser/services/EndpointError.java +++ b/conjure-core/src/main/java/com/palantir/conjure/parser/services/EndpointError.java @@ -16,19 +16,14 @@ package com.palantir.conjure.parser.services; -import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.databind.DeserializationContext; -import com.fasterxml.jackson.databind.JsonDeserializer; -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.annotation.JsonCreator; import com.palantir.conjure.defs.ConjureImmutablesStyle; -import com.palantir.conjure.parser.services.EndpointError.EndpointErrorDeserializer; +import com.palantir.conjure.parser.services.ImmutableEndpointError.Json; import com.palantir.conjure.parser.types.ConjureType; import com.palantir.parsec.ParseException; -import java.io.IOException; import java.util.Optional; import org.immutables.value.Value; -@JsonDeserialize(using = EndpointErrorDeserializer.class) @Value.Immutable @ConjureImmutablesStyle public interface EndpointError { @@ -40,21 +35,17 @@ static EndpointError of(ConjureType errorName) { return ImmutableEndpointError.builder().error(errorName).build(); } - // TODO(pm): see if there's something we can do with @JsonCreator. - final class EndpointErrorDeserializer extends JsonDeserializer { - @Override - public EndpointError deserialize(JsonParser parser, DeserializationContext _context) throws IOException { - - String candidate = parser.getValueAsString(); - if (candidate != null) { - try { - return EndpointError.of(ConjureType.fromString(candidate)); - } catch (ParseException e) { - throw new RuntimeException(e); - } - } - - return ImmutableEndpointError.fromJson(parser.readValueAs(ImmutableEndpointError.Json.class)); + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + static EndpointError fromString(String error) { + try { + return EndpointError.of(ConjureType.fromString(error)); + } catch (ParseException e) { + throw new RuntimeException(e); } } + + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + static EndpointError fromJson(Json json) { + return ImmutableEndpointError.fromJson(json); + } } diff --git a/conjure-core/src/test/java/com/palantir/conjure/defs/ConjureDefTest.java b/conjure-core/src/test/java/com/palantir/conjure/defs/ConjureDefTest.java index d6f65dad8..0e50fdf53 100644 --- a/conjure-core/src/test/java/com/palantir/conjure/defs/ConjureDefTest.java +++ b/conjure-core/src/test/java/com/palantir/conjure/defs/ConjureDefTest.java @@ -23,6 +23,7 @@ import com.palantir.conjure.exceptions.ConjureRuntimeException; import com.palantir.conjure.parser.ConjureParser; import com.palantir.conjure.spec.ConjureDefinition; +import com.palantir.conjure.spec.Documentation; import com.palantir.conjure.spec.EndpointError; import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; import java.io.File; @@ -110,12 +111,17 @@ public void testEndpointErrorsCanBeImported() { assertThat(serviceDefinition.getEndpoints()) .singleElement() .satisfies(endpointDefinition -> assertThat(endpointDefinition.getErrors()) - .extracting(EndpointError::getError) + // .extracting(EndpointError::getError) .containsExactlyInAnyOrder( - // Invalid argument should be from the `test.api` package that was imported. - com.palantir.conjure.spec.TypeName.of("InvalidArgument", "test.api"), - com.palantir.conjure.spec.TypeName.of( - "Error2", "test.api.with.imported.errors"))); + EndpointError.builder() + .error(com.palantir.conjure.spec.TypeName.of( + "Error2", "test.api.with.imported.errors")) + .build(), + // The InvalidArgument is imported from the `test.api` package. + EndpointError.of( + com.palantir.conjure.spec.TypeName.of( + "InvalidArgument", "test.api"), + Documentation.of("Docs for the imported error")))); }); } } diff --git a/conjure-core/src/test/java/com/palantir/conjure/defs/validator/EndpointDefinitionTest.java b/conjure-core/src/test/java/com/palantir/conjure/defs/validator/EndpointDefinitionTest.java index 0d016a930..8701fd5b4 100644 --- a/conjure-core/src/test/java/com/palantir/conjure/defs/validator/EndpointDefinitionTest.java +++ b/conjure-core/src/test/java/com/palantir/conjure/defs/validator/EndpointDefinitionTest.java @@ -26,6 +26,7 @@ import com.palantir.conjure.spec.BodyParameterType; import com.palantir.conjure.spec.Documentation; import com.palantir.conjure.spec.EndpointDefinition; +import com.palantir.conjure.spec.EndpointError; import com.palantir.conjure.spec.EndpointName; import com.palantir.conjure.spec.HeaderParameterType; import com.palantir.conjure.spec.HttpMethod; @@ -43,6 +44,7 @@ import com.palantir.conjure.spec.TypeDefinition; import com.palantir.conjure.spec.TypeName; import com.palantir.conjure.visitor.DealiasingTypeVisitor; +import java.util.List; import java.util.stream.Collectors; import org.junit.jupiter.api.Test; @@ -351,4 +353,27 @@ public void testNoUnsupportedHttpMethod() { + HttpMethod.values().stream().map(HttpMethod::toString).collect(Collectors.joining("|")) + "), but received 'UNKNOWN' in endpoint 'test{http: UNKNOWN /}'."); } + + @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) + .docs(Documentation.of("docs")) + .build(), + EndpointError.builder() + .error(errorType) + .docs(Documentation.of("different docs but same error")) + .build())) + .build(); + + assertThatThrownBy(() -> EndpointDefinitionValidator.validateAll(definition, emptyDealiasingVisitor)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Error 'Error1' is declared multiple times in endpoint 'test{http: GET /get}'"); + } } diff --git a/conjure-core/src/test/resources/example-imported-endpoint-error.yml b/conjure-core/src/test/resources/example-imported-endpoint-error.yml index 1b1ffebac..6888ab21b 100644 --- a/conjure-core/src/test/resources/example-imported-endpoint-error.yml +++ b/conjure-core/src/test/resources/example-imported-endpoint-error.yml @@ -20,5 +20,6 @@ services: http: GET /string returns: string errors: - - imports.InvalidArgument + - error: imports.InvalidArgument + docs: Docs for the imported error - Error2 diff --git a/conjure.schema.json b/conjure.schema.json index dbb7c432e..c3b54e2f4 100644 --- a/conjure.schema.json +++ b/conjure.schema.json @@ -526,7 +526,10 @@ "docs": { "$ref": "#/definitions/DocString" } - } + }, + "required": [ + "error" + ] }, "DocString": { "type": "string" diff --git a/docs/spec/conjure_definitions.md b/docs/spec/conjure_definitions.md index 098a40be3..07e4d2394 100644 --- a/docs/spec/conjure_definitions.md +++ b/docs/spec/conjure_definitions.md @@ -30,6 +30,7 @@ The Conjure compiler requires each file to conform to the [ConjureSourceFile][] - [ServiceDefinition][] - [AuthDefinition][] - [EndpointDefinition][] + - [EndpointError][] - [ArgumentDefinition][] - [ArgumentDefinition.ParamType][] - [DocString][] @@ -278,6 +279,15 @@ safe‑args | Map[`string` → [FieldDefinition][] or [ConjureT unsafe‑args | Map[`string` → [FieldDefinition][] or [ConjureType][]] | **REQUIRED**. A map from argument names to type names. These arguments are considered unsafe in accordance with the SLS specification. If the value of the field is a `string` it MUST be a type name that exists within the Conjure definition. docs | [DocString][] | Documentation for the type. [CommonMark syntax](http://spec.commonmark.org/) MAY be used for rich text representation. +**Example:** + +```yaml +CategoryNotFound: + namespace: com.example + code: NOT_FOUND + safe-args: + allCategories: list +``` ## ErrorCode [ErrorCode]: #errorcode @@ -347,6 +357,7 @@ Field | Type | Description http | `string` | **REQUIRED** The operation and path for the endpoint. It MUST follow the shorthand ` `, where `` is one of GET, DELETE, POST, or PUT, and `` is a [PathString][]. auth | [AuthDefinition][] | The authentication mechanism for the endpoint. Overrides `default-auth` in [ServiceDefinition][]. returns | [ConjureType][] | The name of the return type of the endpoint. The value MUST be a type name that exists within the Conjure definition. If not specified, then the endpoint does not return a value. +errors | List[[EndpointError][]] | The errors that this endpoint may return. The errors listed here should be closely tied to problems that uniquely arise from the generating a response to the endpoint, which clients are expected to handle. args | Map[`string` → [ArgumentDefinition][] or [ConjureType][]] | A map between argument names and argument definitions. If the value of the field is a `string` it MUST be a type name that exists within the Conjure definition. Furthermore, if a `string` the argument will default to `auto` [ArgumentDefinition.ParamType][]. docs | [DocString][] | Documentation for the endpoint. [CommonMark syntax](http://spec.commonmark.org/) MAY be used for rich text representation. deprecated | [DocString][] | Documentation for the deprecation of the endpoint. [CommonMark syntax](http://spec.commonmark.org/) MAY be used for rich text representation. @@ -362,7 +373,11 @@ createRecipe: param-type: body type: Recipe returns: RecipeId + errors: + - error: CategoryNotFound + docs: A category with the provided category ID was not found. ``` +See the example in the [ErrorDefinition][] section for the definition of `CategoryNotFound` ## ArgumentDefinition [ArgumentDefinition]: #argumentdefinition @@ -417,6 +432,14 @@ A field describing the type of an endpoint parameter. It is a `string` which MUS - `header`: defined as a header parameter. - `query`: defined as a querystring parameter. +## EndpointError +[EndpointError]: #endpointerror +A reference to an [ErrorDefinition][] associated with a service endpoint. + +Field | Type | Description +---|:---:|--- +error | [TypeName][] | **REQUIRED**. A reference to a Conjure-defined [ErrorDefinition][]. +docs | [DocString][] | Documentation for the argument. [CommonMark syntax](http://spec.commonmark.org/) MAY be used for rich text representation. ## DocString [DocString]: #docstring @@ -476,4 +499,4 @@ services: # Safety may be declared on primitive endpoint arguments as well as # aliases, object fields, and union types. safety: safe -``` \ No newline at end of file +``` diff --git a/docs/spec/intermediate_representation.md b/docs/spec/intermediate_representation.md index 4ae691267..5dbc9c6d2 100644 --- a/docs/spec/intermediate_representation.md +++ b/docs/spec/intermediate_representation.md @@ -364,6 +364,9 @@ parameter name. Each argument definition may have a "docs" key containing string Each argument definition may also have a "markers" key, consisting of a list of [types](#representation-of-conjure-types) that serve as additional metadata for the argument. - "returns": a [representation](#representation-of-conjure-types) of the return type of the endpoint +- "errors": a list of endpoint errors. Each endpoint error is a reference to a Conjure-defined [error type](#errors). +The endpoint error is either a string representing the name of the referenced error, or an object with an "error" field +representing the name of the error and an optional "docs" field containing a string with documentation for the error. - "docs": string documentation for the endpoint - "deprecated": a string explanation indicating the endpoint is deprecated and why