Update docs. Review changes

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<En
     NO_OPTIONAL_BINARY_BODY_PARAM_VALIDATOR(new NoOptionalBinaryBodyParamValidator()),
     PARAMETER_NAME(new ParameterNameValidator()),
     PARAM_ID(new ParamIdValidator()),
-    NO_UNSUPPORTED_HTTP_METHOD(new NoUnsupportedHttpMethodValidator());
+    NO_UNSUPPORTED_HTTP_METHOD(new NoUnsupportedHttpMethodValidator()),
+    NO_DUPLICATE_ENDPOINT_ERRORS(new NoDuplicateEndpointErrorsValidation());
 
     private static final Logger log = LoggerFactory.getLogger(EndpointDefinitionValidator.class);
 
@@ -471,6 +473,22 @@ public void validate(EndpointDefinition definition) {
         }
     }
 
+    @com.google.errorprone.annotations.Immutable
+    private static final class NoDuplicateEndpointErrorsValidation implements ConjureValidator<EndpointDefinition> {
+        @Override
+        public void validate(EndpointDefinition definition) {
+            Set<String> 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());

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<EndpointError> {
-        @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);
+    }
 }

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}'");
+    }
 }

conjure.schema.json

@@ -526,7 +526,10 @@
         "docs": {
           "$ref": "#/definitions/DocString"
         }
-      }
+      },
+      "required": [
+        "error"
+      ]
     },
     "DocString": {
       "type": "string"

docs/spec/conjure_definitions.md

@@ -342,15 +342,16 @@ A field describing an authentication mechanism. It is a `string` which MUST be o
 [EndpointDefinition]: #endpointdefinition
 An object representing an endpoint. An endpoint describes a method, arguments and return type.
 
-Field | Type | Description
----|:---:|---
-http | `string` | **REQUIRED** The operation and path for the endpoint. It MUST follow the shorthand `<method> <path>`, where `<method>` is one of GET, DELETE, POST, or PUT, and `<path>` 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.
+Field |                                   Type                                   | Description
+------|:------------------------------------------------------------------------:|---
+http  |                                 `string`                                 | **REQUIRED** The operation and path for the endpoint. It MUST follow the shorthand `<method> <path>`, where `<method>` is one of GET, DELETE, POST, or PUT, and `<path>` 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 |                            [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` &rarr; [ArgumentDefinition][]&nbsp;or&nbsp;[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.
-tags | Set[`string`] | Set of tags that serves as additional metadata for the endpoint.
+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.
+tags |                              Set[`string`]                               | Set of tags that serves as additional metadata for the endpoint.
 
 **Example:**
 ```yaml
@@ -417,6 +418,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

docs/spec/intermediate_representation.md

@@ -364,6 +364,7 @@ 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 reference to a Conjure-defined [error type](#errors).
 - "docs": string documentation for the endpoint
 - "deprecated": a string explanation indicating the endpoint is deprecated and why