Skip to content

Commit

Permalink
Added support for optional http_response_override yaml (#1362)
Browse files Browse the repository at this point in the history
  • Loading branch information
amorton authored Aug 27, 2024
1 parent 0ebe9e3 commit 668069a
Show file tree
Hide file tree
Showing 11 changed files with 232 additions and 29 deletions.
8 changes: 8 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@
<type>pom</type>
<scope>import</scope>
</dependency>
<dependency>
<!-- Support for Optional used when loading error config seehttps://github.com/FasterXML/jackson-modules-java8/tree/2.18/datatypes -->
<groupId>com.fasterxml.jackson.datatype</groupId>
<artifactId>jackson-datatype-jdk8</artifactId>
<version>${jackson.version}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
<dependency>
<groupId>${quarkus.platform.group-id}</groupId>
<artifactId>${quarkus.platform.artifact-id}</artifactId>
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -40,6 +42,17 @@
public abstract class APIException extends RuntimeException
implements Supplier<CommandResponseError> {

// 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;

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -61,7 +62,7 @@ public static ErrorConfig getInstance() {
private Map<String, String> 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";
Expand All @@ -71,6 +72,7 @@ public ErrorConfig(
@JsonProperty("snippets") List<Snippet> snippets,
@JsonProperty("request_errors") List<ErrorDetail> requestErrors,
@JsonProperty("server_errors") List<ErrorDetail> 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);
Expand All @@ -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}. <b>NOTE:</b>
* 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<Integer> httpResponseOverride) {

public ErrorDetail {
if (scope == null) {
Expand All @@ -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");
}
}

Expand All @@ -133,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;
}
}

/**
Expand Down Expand Up @@ -199,8 +223,7 @@ public Map<String, String> getSnippetVars() {
// want the map to be immutable because we hand it out
snippetVars =
Map.copyOf(
snippets.stream()
.collect(Collectors.toMap(s -> SNIPPET_VAR_PREFIX + s.name(), Snippet::body)));
snippets.stream().collect(Collectors.toMap(s -> s.variableName(), Snippet::body)));
}
return snippetVars;
}
Expand Down Expand Up @@ -246,6 +269,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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.stargate.sgv2.jsonapi.exception.playing;

import java.util.Optional;
import java.util.UUID;

/**
Expand All @@ -14,14 +15,22 @@
* <p>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.
*
* <p>
* <p><b>NOTE:</b> 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
* @param scope
* @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<Integer> httpResponseOverride) {}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -38,6 +39,7 @@
* }
* </pre>
*
* @param <T> 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.
Expand All @@ -46,15 +48,16 @@
* @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 <T> 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<T extends APIException>(
Constructor<T> constructor,
ErrorFamily family,
ErrorScope scope,
String code,
String title,
String messageTemplate) {
String messageTemplate,
Optional<Integer> httpResponseOverride) {

public T toException(Map<String, String> values) {
var errorInstance = toInstance(values);
Expand Down Expand Up @@ -100,7 +103,8 @@ private ErrorInstance toInstance(Map<String, String> 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);
}

/**
Expand Down Expand Up @@ -146,6 +150,12 @@ public static <T extends APIException> ErrorTemplate<T> load(
family, scope, code)));

return new ErrorTemplate<T>(
constructor, family, scope, code, errorConfig.title(), errorConfig.body());
constructor,
family,
scope,
code,
errorConfig.title(),
errorConfig.body(),
errorConfig.httpResponseOverride());
}
}
4 changes: 4 additions & 0 deletions src/main/resources/errors.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 668069a

Please sign in to comment.