From 366abbb898aa3f817962f0a1c2202f321e2367b0 Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Tue, 27 Aug 2024 12:41:23 +1200 Subject: [PATCH 1/5] Added support for optional http_response_override yaml When set it is passed through to the APIException creation and set as a field on the exception that can be used later when we start handling these errors and working out the response code. --- .../exception/playing/APIException.java | 20 +++++- .../exception/playing/ErrorConfig.java | 20 +++++- .../exception/playing/ErrorInstance.java | 13 +++- .../exception/playing/ErrorTemplate.java | 18 +++-- .../exception/playing/ErrorConfigTest.java | 69 +++++++++++++++++-- .../playing/MissingCtorException.java | 6 +- src/test/resources/test_errors.yaml | 9 ++- 7 files changed, 138 insertions(+), 17 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/APIException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/APIException.java index 4e9d08f94d..374f7e51d6 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/APIException.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/APIException.java @@ -1,5 +1,7 @@ package io.stargate.sgv2.jsonapi.exception.playing; +import java.util.Objects; +import java.util.Optional; import java.util.UUID; import java.util.function.Supplier; @@ -40,6 +42,17 @@ public abstract class APIException extends RuntimeException implements Supplier { + // All errors default to 200 HTTP status code, because we have partial failure modes. + // There are some overrides, e.g. a server timeout may be a 500, this is managed in the + // error config. See ErrorTemplate. + public static final int DEFAULT_HTTP_RESPONSE = 200; + + /** + * HTTP Response code for this error. NOTE: Not using enum from quarkus because do not want + * references to the HTTP framework this deep into the command processing + */ + public final int httpResponse; + /** Unique identifier for this error instance. */ public final UUID errorId; @@ -71,17 +84,22 @@ public abstract class APIException extends RuntimeException public final String body; public APIException(ErrorInstance errorInstance) { + Objects.requireNonNull(errorInstance, "ErrorInstance cannot be null"); + this.errorId = errorInstance.errorId(); this.family = errorInstance.family(); this.scope = errorInstance.scope().scope(); this.code = errorInstance.code(); this.title = errorInstance.title(); this.body = errorInstance.body(); + Objects.requireNonNull( + errorInstance.httpResponseOverride(), "httpResponseOverride cannot be null"); + this.httpResponse = errorInstance.httpResponseOverride().orElse(DEFAULT_HTTP_RESPONSE); } public APIException( ErrorFamily family, ErrorScope scope, String code, String title, String body) { - this(new ErrorInstance(UUID.randomUUID(), family, scope, code, title, body)); + this(new ErrorInstance(UUID.randomUUID(), family, scope, code, title, body, Optional.empty())); } @Override diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java index 0f1752493e..dfa06e4f7f 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java @@ -6,6 +6,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.PropertyNamingStrategies; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; +import com.fasterxml.jackson.datatype.jdk8.Jdk8Module; import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.cache.LoadingCache; import com.google.common.annotations.VisibleForTesting; @@ -71,6 +72,7 @@ public ErrorConfig( @JsonProperty("snippets") List snippets, @JsonProperty("request_errors") List requestErrors, @JsonProperty("server_errors") List serverErrors) { + // defensive immutable copies because this config can be shared this.snippets = snippets == null ? List.of() : List.copyOf(snippets); this.requestErrors = requestErrors == null ? List.of() : List.copyOf(requestErrors); @@ -87,8 +89,19 @@ public ErrorConfig( * @param code * @param title * @param body + * @param httpResponseOverride Optional override for the HTTP response code for this error, only + * needs to be set if different from {@link APIException#DEFAULT_HTTP_RESPONSE}. NOTE: + * there is no checking that this is a well known HTTP status code, as we do not want to + * depend on classes like {@link jakarta.ws.rs.core.Response.Status} in this class and if we + * want to return a weird status this class should not limit that. It would be handled higher + * up the stack and tracked with Integration Tests. */ - public record ErrorDetail(String scope, String code, String title, String body) { + public record ErrorDetail( + String scope, + String code, + String title, + String body, + Optional httpResponseOverride) { public ErrorDetail { if (scope == null) { @@ -111,6 +124,8 @@ public record ErrorDetail(String scope, String code, String title, String body) if (body.isBlank()) { throw new IllegalArgumentException("body cannot be blank"); } + + Objects.requireNonNull(httpResponseOverride, "httpResponseOverride cannot be null"); } } @@ -246,6 +261,9 @@ public static ErrorConfig readFromYamlString(String yaml) throws JsonProcessingE // This is only going to happen once at system start, ok to create a new mapper ObjectMapper mapper = new ObjectMapper(new YAMLFactory()); + // module for Optional support see + // https://github.com/FasterXML/jackson-modules-java8/tree/2.18/datatypes + mapper.registerModule(new Jdk8Module()); mapper.setPropertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE); return mapper.readValue(yaml, ErrorConfig.class); } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorInstance.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorInstance.java index 51588e4234..201ea65a45 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorInstance.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorInstance.java @@ -1,5 +1,6 @@ package io.stargate.sgv2.jsonapi.exception.playing; +import java.util.Optional; import java.util.UUID; /** @@ -14,7 +15,8 @@ *

Will make it easier if / when we add more info to an error such as links to documentation. * They can be passed from the template through the ErrorInstance into to the APIException. * - *

+ *

NOTE: keeping variable checking out of this class for now, it's just a way to pass a + * single param rather than many. * * @param errorId * @param family @@ -22,6 +24,13 @@ * @param code * @param title * @param body + * @param httpResponseOverride */ public record ErrorInstance( - UUID errorId, ErrorFamily family, ErrorScope scope, String code, String title, String body) {} + UUID errorId, + ErrorFamily family, + ErrorScope scope, + String code, + String title, + String body, + Optional httpResponseOverride) {} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorTemplate.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorTemplate.java index 472bd10baf..9422cfd52e 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorTemplate.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorTemplate.java @@ -4,6 +4,7 @@ import java.lang.reflect.InvocationTargetException; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import java.util.UUID; import org.apache.commons.text.StringSubstitutor; @@ -38,6 +39,7 @@ * } * * + * @param The type of the {@link APIException} the template creates. * @param constructor A constructor accepts a single parameter of {@link ErrorInstance} and returns * an instance of the `T` type. * @param family {@link ErrorFamily} the error belongs to. @@ -46,7 +48,7 @@ * @param title Title of the error, does not change between instances. * @param messageTemplate A template for the error body, with variables to be replaced at runtime * using the {@link StringSubstitutor} from Apache Commons Text. - * @param The type of the {@link APIException} the template creates. + * @param httpResponseOverride If present, overrides the default HTTP 200 response code for errors. */ public record ErrorTemplate( Constructor constructor, @@ -54,7 +56,8 @@ public record ErrorTemplate( ErrorScope scope, String code, String title, - String messageTemplate) { + String messageTemplate, + Optional httpResponseOverride) { public T toException(Map values) { var errorInstance = toInstance(values); @@ -100,7 +103,8 @@ private ErrorInstance toInstance(Map values) { throw new UnresolvedErrorTemplateVariable(this, e.getMessage()); } - return new ErrorInstance(UUID.randomUUID(), family, scope, code, title, msg); + return new ErrorInstance( + UUID.randomUUID(), family, scope, code, title, msg, httpResponseOverride); } /** @@ -146,6 +150,12 @@ public static ErrorTemplate load( family, scope, code))); return new ErrorTemplate( - constructor, family, scope, code, errorConfig.title(), errorConfig.body()); + constructor, + family, + scope, + code, + errorConfig.title(), + errorConfig.body(), + errorConfig.httpResponseOverride()); } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfigTest.java b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfigTest.java index 74503517af..59ca6b9bf9 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfigTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfigTest.java @@ -6,6 +6,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import java.io.FileNotFoundException; +import java.util.Optional; import java.util.stream.Stream; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -44,6 +45,7 @@ public void readErrorsYaml() throws JsonProcessingException { - scope: TEST_SCOPE_1 code: TEST_ERROR_ID_1 title: the title for the error + http_response_override: 501 body: |- big long body with ${vars} in it - scope: @@ -70,6 +72,7 @@ public void readErrorsYaml() throws JsonProcessingException { assertThat(e.code()).isEqualTo("TEST_ERROR_ID_1"); assertThat(e.title()).isEqualTo("the title for the error"); assertThat(e.body()).isEqualTo("big long body with ${vars} in it"); + assertThat(e.httpResponseOverride()).isPresent().get().isEqualTo(501); }); assertThat(errorConfig.getErrorDetail(ErrorFamily.REQUEST, "", "TEST_ERROR_ID_2")) @@ -81,6 +84,7 @@ public void readErrorsYaml() throws JsonProcessingException { assertThat(e.code()).isEqualTo("TEST_ERROR_ID_2"); assertThat(e.title()).isEqualTo("This error has no scope"); assertThat(e.body()).isEqualTo("Line 1 of body\n\nLine 2 of body"); + assertThat(e.httpResponseOverride()).isEmpty(); }); assertThat(errorConfig.getErrorDetail(ErrorFamily.SERVER, "TEST_SCOPE_3", "TEST_ERROR_ID_2")) @@ -92,17 +96,26 @@ public void readErrorsYaml() throws JsonProcessingException { assertThat(e.code()).isEqualTo("TEST_ERROR_ID_2"); assertThat(e.title()).isEqualTo("the title for the error"); assertThat(e.body()).isEqualTo("big long body with ${vars} in it"); + assertThat(e.httpResponseOverride()).isEmpty(); }); } @ParameterizedTest @MethodSource("errorDetailTestCases") public void errorDetailValidation( - String scope, String code, String title, String body, Class errorClass, String message) { + String scope, + String code, + String title, + String body, + Optional httpStatusOverride, + Class errorClass, + String message) { T error = assertThrows( - errorClass, () -> new ErrorConfig.ErrorDetail(scope, code, title, body), "Error Type"); + errorClass, + () -> new ErrorConfig.ErrorDetail(scope, code, title, body, httpStatusOverride), + "Error Type"); assertThat(error.getMessage()).as("Error Message").isEqualTo(message); } @@ -114,25 +127,65 @@ private static Stream errorDetailTestCases() { "CODE", "title", "body", + Optional.empty(), IllegalArgumentException.class, "scope must be in UPPER_SNAKE_CASE_1 format, got: not_snake_scope"), Arguments.of( - "SCOPE", null, "title", "body", NullPointerException.class, "code cannot be null"), + "SCOPE", + null, + "title", + "body", + Optional.empty(), + NullPointerException.class, + "code cannot be null"), Arguments.of( "SCOPE", "not snake code", "title", "body", + Optional.empty(), IllegalArgumentException.class, "code must be in UPPER_SNAKE_CASE_1 format, got: not snake code"), Arguments.of( - "SCOPE", "CODE", null, "body", NullPointerException.class, "title cannot be null"), + "SCOPE", + "CODE", + null, + "body", + Optional.empty(), + NullPointerException.class, + "title cannot be null"), + Arguments.of( + "SCOPE", + "CODE", + "", + "body", + Optional.empty(), + IllegalArgumentException.class, + "title cannot be blank"), Arguments.of( - "SCOPE", "CODE", "", "body", IllegalArgumentException.class, "title cannot be blank"), + "SCOPE", + "CODE", + "title", + null, + Optional.empty(), + NullPointerException.class, + "body cannot be null"), Arguments.of( - "SCOPE", "CODE", "title", null, NullPointerException.class, "body cannot be null"), + "SCOPE", + "CODE", + "title", + "", + Optional.empty(), + IllegalArgumentException.class, + "body cannot be blank"), Arguments.of( - "SCOPE", "CODE", "title", "", IllegalArgumentException.class, "body cannot be blank")); + "SCOPE", + "CODE", + "title", + "body", + null, + NullPointerException.class, + "httpResponseOverride cannot be null")); } @Test @@ -160,6 +213,7 @@ public void readSnippetYaml() throws JsonProcessingException { s -> { assertThat(s.name()).isEqualTo("SNIPPET_1"); assertThat(s.body()).isEqualTo("Snippet 1 body"); + assertThat(errorConfig.getSnippetVars()).containsKey(s.name()); }); assertThat( errorConfig.snippets().stream().filter(e -> e.name().equals("SNIPPET_2")).findFirst()) @@ -169,6 +223,7 @@ public void readSnippetYaml() throws JsonProcessingException { s -> { assertThat(s.name()).isEqualTo("SNIPPET_2"); assertThat(s.body()).isEqualTo("Snippet 2 body\n\nmulti line"); + assertThat(errorConfig.getSnippetVars()).containsKey(s.name()); }); } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/MissingCtorException.java b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/MissingCtorException.java index 9c78784260..a0684ca833 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/MissingCtorException.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/MissingCtorException.java @@ -1,5 +1,7 @@ package io.stargate.sgv2.jsonapi.exception.playing; +import java.util.Optional; + /** * An exception that is missing the CTOR we need for the templating system. * @@ -14,7 +16,9 @@ public class MissingCtorException extends TestRequestException { // This ctor is just here to make it compile public MissingCtorException() { - super(new ErrorInstance(null, ErrorFamily.REQUEST, SCOPE, "FAKE", "title", "body")); + super( + new ErrorInstance( + null, ErrorFamily.REQUEST, SCOPE, "FAKE", "title", "body", Optional.empty())); } // This is the CTOR that is missing, kept commented out to show what is missing diff --git a/src/test/resources/test_errors.yaml b/src/test/resources/test_errors.yaml index cee9024fef..0cc7fe021b 100644 --- a/src/test/resources/test_errors.yaml +++ b/src/test/resources/test_errors.yaml @@ -30,4 +30,11 @@ request_errors: code: NO_VARIABLES_TEMPLATE title: A body with no template variables body: |- - Body with no variables. \ No newline at end of file + Body with no variables. + + - scope: + code: HTTP_OVERRIDE + http_response_override: 500 + title: An error that overrides the HTTP response + body: |- + body with ${name} and ${value} in it \ No newline at end of file From eb9b0996de7fe8810e8fb6dedddfd804bca15de4 Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Tue, 27 Aug 2024 12:41:45 +1200 Subject: [PATCH 2/5] fmt fixes --- pom.xml | 8 ++ src/main/resources/errors.yaml | 4 + .../exception/playing/ErrorTemplateTest.java | 77 +++++++++++++++++-- .../playing/TestRequestException.java | 3 +- 4 files changed, 83 insertions(+), 9 deletions(-) diff --git a/pom.xml b/pom.xml index c723676f0a..a751044b82 100644 --- a/pom.xml +++ b/pom.xml @@ -56,6 +56,14 @@ pom import + + + com.fasterxml.jackson.datatype + jackson-datatype-jdk8 + ${jackson.version} + pom + import + ${quarkus.platform.group-id} ${quarkus.platform.artifact-id} diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml index 19c8430fec..1dfa126858 100644 --- a/src/main/resources/errors.yaml +++ b/src/main/resources/errors.yaml @@ -34,6 +34,10 @@ # Each error object has: # - scope: UPPER_SNAKE_CASE_1 # - code: UPPER_SNAKE_CASE_1 +# - http_response_override: (optional) The HTTP status code to return when this error is thrown. If not present, the +# default status code is 200 for most things. This is not returned in the error object JSON +# It is used to override the HTTP status code in the response. +# NOTE: NO checking is done to confirm this is a valid HTTP status code. # - title: A short title for the error, that must not change between instances of the error. # - body: A longer body that may contain ${vars} to be passed by the code when created, and references to snippets. # This can be a multi line string, recommend using the `|-` to trim trailing newlines. diff --git a/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorTemplateTest.java b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorTemplateTest.java index a51b65591d..c484f39d06 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorTemplateTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorTemplateTest.java @@ -57,6 +57,43 @@ private T createException(ErrorCode errorCode) { return error; } + /** + * Re-usable to Test various properties on an error + * + * @param error + * @param family + * @param scope + * @param code + * @param title + * @param httpResponseOverride + * @param + */ + private void assertError( + T error, + ErrorFamily family, + String scope, + String code, + String title, + Integer httpResponseOverride) { + // Does not accept a template because that would not catch the template being wrong + // pass in the values that error should have given the code etc. + + assertThat(error) + .isNotNull() + .satisfies( + e -> { + assertThat(e.family).isEqualTo(family); + assertThat(e.scope).isEqualTo(scope); + assertThat(e.code).isEqualTo(code); + assertThat(e.title).isEqualTo(title); + if (null == httpResponseOverride) { + assertThat(e.httpResponse).isEqualTo(APIException.DEFAULT_HTTP_RESPONSE); + } else { + assertThat(e.httpResponse).isEqualTo(httpResponseOverride); + } + }); + } + @Test public void scopedRequestError() { @@ -65,10 +102,13 @@ public void scopedRequestError() { .isNotNull() .satisfies( e -> { - assertThat(e.family).isEqualTo(ErrorFamily.REQUEST); - assertThat(e.scope).isEqualTo(TestRequestException.Scope.TEST_REQUEST_SCOPE.name()); - assertThat(e.code).isEqualTo(TestScopeException.Code.SCOPED_REQUEST_ERROR.name()); - assertThat(e.title).isEqualTo("A scoped request error"); + assertError( + e, + TestScopeException.FAMILY, + TestScopeException.SCOPE.name(), + TestScopeException.Code.SCOPED_REQUEST_ERROR.name(), + "A scoped request error", + null); assertThat(e.body) .contains( "long body with " @@ -87,10 +127,13 @@ public void unscopedRequestError() { .isNotNull() .satisfies( e -> { - assertThat(e.family).isEqualTo(ErrorFamily.REQUEST); - assertThat(e.scope).isBlank(); - assertThat(e.code).isEqualTo(TestRequestException.Code.UNSCOPED_REQUEST_ERROR.name()); - assertThat(e.title).isEqualTo("An unscoped request error"); + assertError( + e, + TestRequestException.FAMILY, + "", + TestRequestException.Code.UNSCOPED_REQUEST_ERROR.name(), + "An unscoped request error", + null); assertThat(e.body) .contains( "Multi line with " @@ -175,4 +218,22 @@ public void unknownErrorCode() { "Could not find error config for family=%s, scope=%s, code=%s", TestScopeException.FAMILY, TestScopeException.SCOPE, TEST_DATA.RANDOM_STRING)); } + + @Test + public void httpResponseOverride() { + var error = createException(TestRequestException.Code.HTTP_OVERRIDE); + + assertThat(error) + .isNotNull() + .satisfies( + e -> { + assertError( + e, + TestRequestException.FAMILY, + "", + TestRequestException.Code.HTTP_OVERRIDE.name(), + "An error that overrides the HTTP response", + 500); + }); + } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/TestRequestException.java b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/TestRequestException.java index 618e096391..e14c400c7d 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/TestRequestException.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/TestRequestException.java @@ -37,7 +37,8 @@ public String scope() { public enum Code implements ErrorCode { UNSCOPED_REQUEST_ERROR, - NO_VARIABLES_TEMPLATE; + NO_VARIABLES_TEMPLATE, + HTTP_OVERRIDE; private final ErrorTemplate template; From 9019e6bf9e209c02492cb0bee456f285f33fd28d Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Tue, 27 Aug 2024 14:58:03 +1200 Subject: [PATCH 3/5] Fix broken test was a badly written test --- .../sgv2/jsonapi/exception/playing/ErrorConfig.java | 13 +++++++++++-- .../jsonapi/exception/playing/ErrorConfigTest.java | 4 ++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java index dfa06e4f7f..5c1fe1afc0 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java @@ -62,7 +62,7 @@ public static ErrorConfig getInstance() { private Map snippetVars; // Prefix used when adding snippets to the variables for a template. - private static final String SNIPPET_VAR_PREFIX = "SNIPPET."; + public static final String SNIPPET_VAR_PREFIX = "SNIPPET."; // TIDY: move this to the config sections public static final String DEFAULT_ERROR_CONFIG_FILE = "errors.yaml"; @@ -148,6 +148,15 @@ public record Snippet(String name, String body) { throw new IllegalArgumentException("body cannot be blank"); } } + + /** + * Name to use for this snippet when substituting into templates. + * + * @return + */ + public String variableName(){ + return SNIPPET_VAR_PREFIX + name; + } } /** @@ -215,7 +224,7 @@ public Map getSnippetVars() { snippetVars = Map.copyOf( snippets.stream() - .collect(Collectors.toMap(s -> SNIPPET_VAR_PREFIX + s.name(), Snippet::body))); + .collect(Collectors.toMap(s -> s.variableName(), Snippet::body))); } return snippetVars; } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfigTest.java b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfigTest.java index 59ca6b9bf9..bebf76a61c 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfigTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfigTest.java @@ -213,7 +213,7 @@ public void readSnippetYaml() throws JsonProcessingException { s -> { assertThat(s.name()).isEqualTo("SNIPPET_1"); assertThat(s.body()).isEqualTo("Snippet 1 body"); - assertThat(errorConfig.getSnippetVars()).containsKey(s.name()); + assertThat(errorConfig.getSnippetVars()).containsKey(s.variableName()); }); assertThat( errorConfig.snippets().stream().filter(e -> e.name().equals("SNIPPET_2")).findFirst()) @@ -223,7 +223,7 @@ public void readSnippetYaml() throws JsonProcessingException { s -> { assertThat(s.name()).isEqualTo("SNIPPET_2"); assertThat(s.body()).isEqualTo("Snippet 2 body\n\nmulti line"); - assertThat(errorConfig.getSnippetVars()).containsKey(s.name()); + assertThat(errorConfig.getSnippetVars()).containsKey(s.variableName()); }); } From c80a5991cdc12bbe061e86f4013cd9b91425d58c Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Tue, 27 Aug 2024 15:01:48 +1200 Subject: [PATCH 4/5] fmt fix --- .../stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java index 5c1fe1afc0..24ad7737c2 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java @@ -154,7 +154,7 @@ public record Snippet(String name, String body) { * * @return */ - public String variableName(){ + public String variableName() { return SNIPPET_VAR_PREFIX + name; } } @@ -223,8 +223,7 @@ public Map getSnippetVars() { // want the map to be immutable because we hand it out snippetVars = Map.copyOf( - snippets.stream() - .collect(Collectors.toMap(s -> s.variableName(), Snippet::body))); + snippets.stream().collect(Collectors.toMap(s -> s.variableName(), Snippet::body))); } return snippetVars; } From 83b448efec580af21691e3461e919d78134636d7 Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Wed, 28 Aug 2024 10:34:28 +1200 Subject: [PATCH 5/5] Change error.yaml format to use kebab-case moved from camel_case to internal standard --- .../sgv2/jsonapi/exception/playing/ErrorConfig.java | 10 +++++----- src/main/resources/errors.yaml | 10 +++++----- .../jsonapi/exception/playing/ErrorConfigTest.java | 6 +++--- src/test/resources/invalid_exception_errors.yaml | 2 +- src/test/resources/test_errors.yaml | 4 ++-- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java index 24ad7737c2..dbcc5af160 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java @@ -5,7 +5,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.PropertyNamingStrategies; -import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; +import com.fasterxml.jackson.dataformat.yaml.YAMLMapper; import com.fasterxml.jackson.datatype.jdk8.Jdk8Module; import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.cache.LoadingCache; @@ -70,8 +70,8 @@ public static ErrorConfig getInstance() { @JsonCreator public ErrorConfig( @JsonProperty("snippets") List snippets, - @JsonProperty("request_errors") List requestErrors, - @JsonProperty("server_errors") List serverErrors) { + @JsonProperty("request-errors") List requestErrors, + @JsonProperty("server-errors") List serverErrors) { // defensive immutable copies because this config can be shared this.snippets = snippets == null ? List.of() : List.copyOf(snippets); @@ -268,11 +268,11 @@ private static ErrorConfig maybeCacheErrorConfig(ErrorConfig errorConfig) { public static ErrorConfig readFromYamlString(String yaml) throws JsonProcessingException { // This is only going to happen once at system start, ok to create a new mapper - ObjectMapper mapper = new ObjectMapper(new YAMLFactory()); + ObjectMapper mapper = new YAMLMapper(); // module for Optional support see // https://github.com/FasterXML/jackson-modules-java8/tree/2.18/datatypes mapper.registerModule(new Jdk8Module()); - mapper.setPropertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE); + mapper.setPropertyNamingStrategy(PropertyNamingStrategies.KEBAB_CASE); return mapper.readValue(yaml, ErrorConfig.class); } diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml index 1dfa126858..ebbe6505ff 100644 --- a/src/main/resources/errors.yaml +++ b/src/main/resources/errors.yaml @@ -12,7 +12,7 @@ # Where # * Family: Identifies if the error relates to the client request or the server processing, analogous # to the 4XX and 5XX HTTP status codes. Supported values are REQUEST or SERVER. In this file they are -# represented by the request_errors and server_errors keys. +# represented by the request-errors and server-errors keys. # * Scope: Optionally identifies the part of the request or server processing that caused the fault, for example "FILTER" # when there is a problem in the filter clause. Scope generally map to a concrete APIException class such as # FilterException. @@ -30,11 +30,11 @@ # - name: UPPER_SNAKE_CASE_1 # - body: A string with the text of the snippet, recommend using the `|-` to trim trailing newlines. # -# "request_errors" and "server_errors" are lists of error objects, for the REQUEST and SERVER family respectively. +# "request-errors" and "server-errors" are lists of error objects, for the REQUEST and SERVER family respectively. # Each error object has: # - scope: UPPER_SNAKE_CASE_1 # - code: UPPER_SNAKE_CASE_1 -# - http_response_override: (optional) The HTTP status code to return when this error is thrown. If not present, the +# - http-response-override: (optional) The HTTP status code to return when this error is thrown. If not present, the # default status code is 200 for most things. This is not returned in the error object JSON # It is used to override the HTTP status code in the response. # NOTE: NO checking is done to confirm this is a valid HTTP status code. @@ -50,7 +50,7 @@ snippets: body: |- Please contact support if the issue persists. -request_errors: +request-errors: - scope: FILTER code: MULTIPLE_ID_FILTER title: the title for the error @@ -64,7 +64,7 @@ request_errors: ${SNIPPET.CONTACT_SUPPORT} -server_errors: +server-errors: - scope: DATABASE code: SERVER_READ_FAILED title: the title for the error diff --git a/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfigTest.java b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfigTest.java index bebf76a61c..d740678ffe 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfigTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfigTest.java @@ -41,11 +41,11 @@ public void initializeFromResource() throws JsonProcessingException { public void readErrorsYaml() throws JsonProcessingException { String yaml = """ - request_errors: + request-errors: - scope: TEST_SCOPE_1 code: TEST_ERROR_ID_1 title: the title for the error - http_response_override: 501 + http-response-override: 501 body: |- big long body with ${vars} in it - scope: @@ -55,7 +55,7 @@ public void readErrorsYaml() throws JsonProcessingException { Line 1 of body Line 2 of body - server_errors: + server-errors: - scope: TEST_SCOPE_3 code: TEST_ERROR_ID_2 title: the title for the error diff --git a/src/test/resources/invalid_exception_errors.yaml b/src/test/resources/invalid_exception_errors.yaml index 8612172148..7a2fb8c94c 100644 --- a/src/test/resources/invalid_exception_errors.yaml +++ b/src/test/resources/invalid_exception_errors.yaml @@ -1,6 +1,6 @@ # For use when testing the exception classes that do not have ctor or break etc. -request_errors: +request-errors: - scope: MISSING_CTOR code: EXCEPTION_MISSING_CTOR title: test for exception class missing the ctor needed for templating diff --git a/src/test/resources/test_errors.yaml b/src/test/resources/test_errors.yaml index 0cc7fe021b..4d1912c12e 100644 --- a/src/test/resources/test_errors.yaml +++ b/src/test/resources/test_errors.yaml @@ -5,7 +5,7 @@ snippets: body: |- Please contact support if the issue persists. -request_errors: +request-errors: - scope: TEST_REQUEST_SCOPE code: SCOPED_REQUEST_ERROR title: A scoped request error @@ -34,7 +34,7 @@ request_errors: - scope: code: HTTP_OVERRIDE - http_response_override: 500 + http-response-override: 500 title: An error that overrides the HTTP response body: |- body with ${name} and ${value} in it \ No newline at end of file