diff --git a/pom.xml b/pom.xml index 9aa2ece650..23d869b0d1 100644 --- a/pom.xml +++ b/pom.xml @@ -135,6 +135,10 @@ io.quarkus quarkus-hibernate-validator + + io.quarkus + quarkus-logging-json + jakarta.validation jakarta.validation-api @@ -143,6 +147,11 @@ org.apache.commons commons-lang3 + + org.apache.commons + commons-text + 1.12.0 + com.bpodgursky jbool_expressions @@ -189,10 +198,18 @@ java-driver-query-builder ${driver.version} + - io.quarkus - quarkus-logging-json + com.fasterxml.jackson.dataformat + jackson-dataformat-yaml + + + com.fasterxml.jackson.datatype + jackson-datatype-jdk8 + + diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/impl/CreateCollectionCommand.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/impl/CreateCollectionCommand.java index 49c2b086e0..5b1f5dc896 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/impl/CreateCollectionCommand.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/impl/CreateCollectionCommand.java @@ -137,7 +137,7 @@ public record VectorizeConfig( @Nullable @Schema( description = - "Optional parameters that match the template provided for the provider", + "Optional parameters that match the messageTemplate provided for the provider", type = SchemaType.OBJECT) @JsonProperty("parameters") @JsonInclude(JsonInclude.Include.NON_NULL) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/security/HeaderBasedAuthenticationMechanism.java b/src/main/java/io/stargate/sgv2/jsonapi/api/security/HeaderBasedAuthenticationMechanism.java index 31adf80de9..8d90af5118 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/security/HeaderBasedAuthenticationMechanism.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/security/HeaderBasedAuthenticationMechanism.java @@ -20,7 +20,6 @@ import static io.stargate.sgv2.jsonapi.config.constants.HttpConstants.AUTHENTICATION_TOKEN_HEADER_NAME; import static io.stargate.sgv2.jsonapi.config.constants.HttpConstants.DEPRECATED_AUTHENTICATION_TOKEN_HEADER_NAME; -import io.netty.handler.codec.http.HttpResponseStatus; import io.quarkus.security.identity.IdentityProviderManager; import io.quarkus.security.identity.SecurityIdentity; import io.quarkus.security.identity.request.AuthenticationRequest; @@ -32,6 +31,7 @@ import io.stargate.sgv2.jsonapi.api.security.challenge.impl.ErrorChallengeSender; import io.vertx.ext.web.RoutingContext; import jakarta.enterprise.inject.Instance; +import jakarta.ws.rs.core.Response; import java.util.Collections; import java.util.Optional; import java.util.Set; @@ -84,7 +84,7 @@ public Uni authenticate( @Override public Uni getChallenge(RoutingContext context) { return Uni.createFrom() - .item(new ChallengeData(HttpResponseStatus.UNAUTHORIZED.code(), null, null)); + .item(new ChallengeData(Response.Status.UNAUTHORIZED.getStatusCode(), null, null)); } /** 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 new file mode 100644 index 0000000000..47fc6cc9c2 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/APIException.java @@ -0,0 +1,130 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +import jakarta.ws.rs.core.Response; +import java.util.Objects; +import java.util.Optional; +import java.util.UUID; +import java.util.function.Supplier; + +/** + * Base for all exceptions returned from the API for external use (as opposed to ones only used + * internally) + * + *

All errors are of a {@link ErrorFamily}, this class should not be used directly, one of the + * subclasses should be used. There are further categorised to be errors have an optional {@link + * ErrorScope}, that groups errors of a similar source together. Finally, the error has an {@link + * ErrorCode} that is unique within the family and scope. + * + *

To facilitate better error messages we template the messages in a {@link ErrorTemplate} that + * is loaded from a properties file. The body for the error may change with each instance of the + * exception, for example to include the number of filters that were included in a request. + * + *

The process of adding a new Error Code is: + * + *

+ * + *

    + *
  • Decide what {@link ErrorFamily} the code belongs to. + *
  • Decide if the error has a {@link ErrorScope}, such as errors with Embedding Providers, if + * it does not then use {@link ErrorScope#NONE}. + *
  • Decide on the {@link ErrorCode}, it should be unique within the Family and Scope + * combination. + *
  • Add the error to file read by {@link ErrorTemplate} to define the title and templated body + * body. + *
  • Add the error code to the Code enum for the Exception class, such as {@link + * FilterException.Code} or {@link RequestException.Code} with the same name. When the enum is + * instantiated at JVM start the error template is loaded. + *
  • Create instances using the methods on {@link ErrorCode}. + *
+ * + * To get the Error to be returned in the {@link + * io.stargate.sgv2.jsonapi.api.model.command.CommandResult} call the {@link #get()} method to get a + * {@link CommandResponseError} that contains the subset of information we want to return. + */ +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 = Response.Status.OK.getStatusCode(); + + /** + * 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; + + /** The family of the error. */ + public final ErrorFamily family; + + /** + * Optional scope of the error, inside the family. + * + *

Never {@code null}, uses "" for no scope. See {@link ErrorScope} + */ + public final String scope; + + /** Unique code for this error, codes should be unique within the Family and Scope combination. */ + public final String code; + + /** Title of this exception, the same title is used for all instances of the error code. */ + public final String title; + + /** + * Message body for this instance of the error. + * + *

Messages may be unique for each instance of the error code, they are typically driven from + * the {@link ErrorTemplate}. + * + *

This is the processed body to be returned to the client. NOT called body to avoid confusion + * with getMessage() on the 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, Optional.empty())); + } + + @Override + public CommandResponseError get() { + return null; + } + + /** + * Overrides to return the {@link #body} of the error. Using the body as this is effectively the + * message, the structure we want to return to the in the API is the {@link CommandResponseError} + * from {@link #get()} + * + * @return + */ + @Override + public String getMessage() { + return body; + } + + @Override + public String toString() { + return String.format( + "%s{errorId=%s, family=%s, scope='%s', code='%s', title='%s', body='%s'}", + getClass().getSimpleName(), errorId, family, scope, code, title, body); + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/CommandResponseError.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/CommandResponseError.java new file mode 100644 index 0000000000..88db9aa384 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/CommandResponseError.java @@ -0,0 +1,6 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +// TODO has all the fields we want to return in the error object in the request +// TODO: where do we implement error coalescing , here or in in the APIException +public record CommandResponseError( + String family, String scope, String code, String title, String message) {} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/DatabaseException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/DatabaseException.java new file mode 100644 index 0000000000..0983bec672 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/DatabaseException.java @@ -0,0 +1,23 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +public class DatabaseException extends ServerException { + public DatabaseException(ErrorInstance errorInstance) { + super(errorInstance); + } + + public enum Code implements ErrorCode { + FAKE; + + private final ErrorTemplate template; + + Code() { + template = + ErrorTemplate.load(DatabaseException.class, ErrorFamily.SERVER, Scope.DATABASE, name()); + } + + @Override + public ErrorTemplate template() { + return template; + } + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/EmbeddingProviderException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/EmbeddingProviderException.java new file mode 100644 index 0000000000..805220a2ca --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/EmbeddingProviderException.java @@ -0,0 +1,28 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +public class EmbeddingProviderException extends ServerException { + public EmbeddingProviderException(ErrorInstance errorInstance) { + super(errorInstance); + } + + public enum Code implements ErrorCode { + CLIENT_ERROR, + SERVER_ERROR; + + private final ErrorTemplate template; + + Code() { + template = + ErrorTemplate.load( + EmbeddingProviderException.class, + ErrorFamily.SERVER, + Scope.EMBEDDING_PROVIDER, + name()); + } + + @Override + public ErrorTemplate template() { + return template; + } + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorCode.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorCode.java new file mode 100644 index 0000000000..946b20ffbd --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorCode.java @@ -0,0 +1,75 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +import com.google.common.base.Preconditions; +import java.util.HashMap; +import java.util.Map; + +/** + * Interface for any enum that represents an error code to implement. + * + *

This interface is used because multiple ENUM's define the scopes, the interface creates a way + * to treat the values from the different ENUM's in a consistent way. + * + *

The interface makes it easy for code to get an instance of the exception the code represents, + * built using the templates error information in {@link ErrorTemplate} that is loaded at startup. + * + *

With this interface code creates an instance of the exception by calling any of the {@link + * #get()} overloads for example: + * + *

+ *   FilterException.Code.MULTIPLE_ID_FILTER.get("variable_name", "value");
+ *   RequestException.Code.FAKE_CODE.get();
+ * 
+ * + * @param Type of the {@link APIException} the error code creates. + */ +public interface ErrorCode { + + /** + * Gets an instance of the {@link APIException} the error code represents without providing any + * substitution values for the error body. + * + * @return Instance of {@link APIException} the error code represents. + */ + default T get() { + return get(Map.of()); + } + + /** + * Gets an instance of the {@link APIException} the error code represents, providing substitution + * values for the error body as a param array. + * + * @param values Substitution values for the error body. The array length must be a multiple of 2, + * each pair of strings is treated as a key-value pair for example ["key-1", "value-1", + * "key-2", "value-2"] + * @return Instance of {@link APIException} the error code represents. + */ + default T get(String... values) { + Preconditions.checkArgument( + values.length % 2 == 0, "Length of the values must be a multiple of 2"); + Map valuesMap = new HashMap<>(values.length / 2); + for (int i = 0; i < values.length; i += 2) { + valuesMap.put(values[i], values[i + 1]); + } + return get(valuesMap); + } + + /** + * Gets an instance of the {@link APIException} the error code represents, providing substitution + * values for the error body as a param array. + * + * @param values May of substitution values for the error body. + * @return Instance of {@link APIException} the error code represents. + */ + default T get(Map values) { + return template().toException(values); + } + + /** + * ENUM Implementers must return a non-null {@link ErrorTemplate} that is used to build an + * instance of the Exception the code represents. + * + * @return {@link ErrorTemplate} for the error code. + */ + ErrorTemplate template(); +} 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 new file mode 100644 index 0000000000..1a9a4fd909 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfig.java @@ -0,0 +1,301 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.core.JacksonException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.PropertyNamingStrategies; +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; +import com.google.common.annotations.VisibleForTesting; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +/** + * Java objects to hold the config from the errors config yaml file for use by {@link ErrorTemplate} + * + *

See the {@link #DEFAULT_ERROR_CONFIG_FILE} for description of what the yaml should look like. + * + *

To use this, call {@link #getInstance()} in code, this will cause the yaml file to be loaded + * if needed. The file will be loaded from {@link #DEFAULT_ERROR_CONFIG_FILE} resource in the JAR. + * Then use {@link #getErrorDetail(ErrorFamily, String, String)} if you want the template, or the + * use {@link #getSnippetVars()} when running templates. + * + *

If you need more control use the {@link #initializeFromYamlResource(String)} before any code + * causes the config to be read or {@link #unsafeInitializeFromYamlResource(String)} . + * + *

A {@link IllegalStateException} will be raised if an attempt is made to re-read the config. + * + *

Using a class rather than a record so we can cache converting the snippets to a map for use in + * the templates. + */ +public class ErrorConfig { + + /** + * Method to get the configured ErrorConfig instance that has the data from the file + * + * @return Configured ErrorConfig instance + */ + public static ErrorConfig getInstance() { + return CACHE.get(CACHE_KEY); + } + + private final List snippets; + private final List requestErrors; + private final List serverErrors; + + // Lazy loaded map of the snippets for use in the templates + private Map snippetVars; + + // Prefix used when adding snippets to the variables for a template. + private static final String SNIPPET_VAR_PREFIX = "SNIPPET."; + + // TIDY: move this to the config sections + public static final String DEFAULT_ERROR_CONFIG_FILE = "errors.yaml"; + + @JsonCreator + 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); + this.serverErrors = serverErrors == null ? List.of() : List.copyOf(serverErrors); + } + + /** + * ErrorDetail is a record to hold the template for a particular error. + * + *

Does not have {@link ErrorFamily} because we have that as the only hierarchy in the config + * file, it is two different lists in the parent class. + * + * @param scope + * @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, + Optional httpResponseOverride) { + + public ErrorDetail { + if (scope == null) { + scope = ErrorScope.NONE.scope(); + } + // scope can be empty, if not empty must be snake case + if (!scope.isBlank()) { + requireSnakeCase(scope, "scope"); + } + + Objects.requireNonNull(code, "code cannot be null"); + requireSnakeCase(code, "code"); + + Objects.requireNonNull(title, "title cannot be null"); + if (title.isBlank()) { + throw new IllegalArgumentException("title cannot be blank"); + } + + Objects.requireNonNull(body, "body cannot be null"); + if (body.isBlank()) { + throw new IllegalArgumentException("body cannot be blank"); + } + + Objects.requireNonNull(httpResponseOverride, "httpResponseOverride cannot be null"); + } + } + + /** + * Snippet is a record to hold the template for a particular snippet. + * + *

+ * + * @param name + * @param body + */ + public record Snippet(String name, String body) { + + public Snippet { + Objects.requireNonNull(name, "name cannot be null"); + requireSnakeCase(name, "name"); + + Objects.requireNonNull(body, "body cannot be null"); + if (body.isBlank()) { + 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; + } + } + + /** + * See {@link #getSnippetVars()} for the cached map of snippets vars. + * + * @return + */ + @VisibleForTesting + List snippets() { + return snippets; + } + + /** + * Helper to optionally get a {@link ErrorDetail} for use by a {@link ErrorTemplate} + * + * @param family + * @param scope + * @param code + * @return + */ + public Optional getErrorDetail(ErrorFamily family, String scope, String code) { + + var errors = + switch (family) { + case REQUEST -> requestErrors; + case SERVER -> serverErrors; + }; + return errors.stream() + .filter(e -> e.code().equals(code) && e.scope().equals(scope)) + .findFirst(); + } + + /** + * Returns a map of the snippets for use in the templates. + * + *

The map is cached, recommend use this rather than call {@link #snippets()} for every error + * + * @return Map of snippets for use in templates + */ + protected Map getSnippetVars() { + + if (snippetVars == null) { + // NOTE: Potential race condition, should be OK because the data won't change and we are only + // writing. + // want the map to be immutable because we hand it out + snippetVars = + Map.copyOf( + snippets.stream().collect(Collectors.toMap(s -> s.variableName(), Snippet::body))); + } + return snippetVars; + } + + // Reusable Pattern for UPPER_SNAKE_CASE_2 - allows alpha and digits + private static final Pattern UPPER_SNAKE_CASE_PATTERN = + Pattern.compile("^[A-Z0-9]+(_[A-Z0-9]+)*$"); + + private static void requireSnakeCase(String value, String name) { + if (!UPPER_SNAKE_CASE_PATTERN.matcher(value).matches()) { + throw new IllegalArgumentException( + name + " must be in UPPER_SNAKE_CASE_1 format, got: " + value); + } + } + + // there is a single item in the cache + private static final Object CACHE_KEY = new Object(); + + // Using a Caffeine cache even though there is a single instance of the ErrorConfig read from disk + // so we can either lazy load when we first need it using default file or load from a + // different file or yaml string for tests etc. + private static final LoadingCache CACHE = + Caffeine.newBuilder().build(key -> readFromYamlResource(DEFAULT_ERROR_CONFIG_FILE)); + + private static ErrorConfig maybeCacheErrorConfig(ErrorConfig errorConfig) { + + if (CACHE.getIfPresent(CACHE_KEY) != null) { + throw new IllegalStateException("ErrorConfig already initialised"); + } + CACHE.put(CACHE_KEY, errorConfig); + return errorConfig; + } + + @VisibleForTesting + /** + * Configures a new {@link ErrorConfig} using the data in the YAML string. + * + *

NOTE: does not "initialize" the class, just creates a new instance with the data from + * the YAML string. Use the initializeFrom methods for that, or let the default behaviour kicking. + */ + static ErrorConfig readFromYamlString(String yaml) throws JacksonException { + + // This is only going to happen once at system start, ok to create a new mapper + final ObjectMapper mapper = + YAMLMapper.builder() + // module for Optional support see + // https://github.com/FasterXML/jackson-modules-java8/tree/2.18/datatypes + .addModule(new Jdk8Module()) + .propertyNamingStrategy(PropertyNamingStrategies.KEBAB_CASE) + .build(); + return mapper.readValue(yaml, ErrorConfig.class); + } + + /** + * Create a new {@link ErrorConfig} using the YAML contents of the resource file, does not + * initialise. This is for use by the cache loader. + * + * @param path + * @return + * @throws IOException + */ + private static ErrorConfig readFromYamlResource(String path) throws IOException { + + URL resourceURL = ErrorConfig.class.getClassLoader().getResource(path); + if (resourceURL == null) { + throw new FileNotFoundException("ErrorConfig Resource not found: " + path); + } + URI resourceURI; + try { + resourceURI = resourceURL.toURI(); + } catch (URISyntaxException e) { + throw new IOException("ErrorConfig Resource " + path + " has Invalid URI: " + resourceURL, e); + } + return readFromYamlString(Files.readString(Paths.get(resourceURI))); + } + + /** + * Create a new {@link ErrorConfig} using the YAML contents of the resource file and initialise it + * as the static config. + */ + @VisibleForTesting + static ErrorConfig initializeFromYamlResource(String path) throws IOException { + return maybeCacheErrorConfig(readFromYamlResource(path)); + } + + /** + * Clears the current config and replaces it with the file at path. DO NOT USE + * in regular code, only for testing. + */ + @VisibleForTesting + static ErrorConfig unsafeInitializeFromYamlResource(String path) throws IOException { + CACHE.invalidate(CACHE_KEY); + return initializeFromYamlResource(path); + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorFamily.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorFamily.java new file mode 100644 index 0000000000..b04342dc7c --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorFamily.java @@ -0,0 +1,14 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +/** + * Top level hierarchy for all exceptions thrown by the API. + * + *

See {@link APIException} + */ +public enum ErrorFamily { + /** See {@link RequestException} */ + REQUEST, + + /** See {@link ServerException} */ + SERVER; +} 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 new file mode 100644 index 0000000000..201ea65a45 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorInstance.java @@ -0,0 +1,36 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +import java.util.Optional; +import java.util.UUID; + +/** + * The information to use when creating an error, included as a record to make it easier to pass + * around between the template and the subclasses of the {@link APIException}. + * + *

Created by the {@link ErrorTemplate} and accepeted by the {@link APIException}. + * + *

Not intended to replace the {@link APIException} or be anything that is used outside the + * exception package. + * + *

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 + * @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, + Optional httpResponseOverride) {} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorScope.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorScope.java new file mode 100644 index 0000000000..68d441c7c1 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorScope.java @@ -0,0 +1,25 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +/** + * Interface for any enum that represents an error scope to implement. + * + *

This is used to group errors together in a {@link ErrorFamily}. + * + *

This interface is used because multiple ENUM's define the scopes, the interface creates a way + * to treat the values from the different ENUM's in a consistent way. + * + *

See {@link APIException} + */ +@FunctionalInterface +public interface ErrorScope { + + /** The NONE scope is used to represent the absence of a scope. */ + ErrorScope NONE = () -> ""; + + /** + * Implementing ENUM's must return a unique string that represents the scope. + * + * @return String representing the scope. + */ + String scope(); +} 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 new file mode 100644 index 0000000000..ba2e3db40d --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorTemplate.java @@ -0,0 +1,161 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +import java.lang.reflect.Constructor; +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; + +/** + * A template for creating an {@link APIException}, that is associated with an Error Code enum so + * the {@link io.stargate.sgv2.jsonapi.exception.ErrorCode} interface can easily create the + * exception. + * + *

Instances are normally created by reading a config file, see {@link #load(Class, ErrorFamily, + * ErrorScope, String)} + * + *

Example usage with an Error Code ENUM: + * + *

+ * + *

+ *     public enum Code implements ErrorCode {
+ *
+ *     MULTIPLE_ID_FILTER,
+ *     FILTER_FIELDS_LIMIT_VIOLATION;
+ *
+ *     private final ErrorTemplate template;
+ *
+ *     Code() {
+ *       template = ErrorTemplate.load(FilterException.class, FAMILY, SCOPE, name());
+ *     }
+ *
+ *     @Override
+ *     public ErrorTemplate template() {
+ *       return template;
+ *     }
+ *   }
+ * 
+ * + * @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. + * @param scope {@link ErrorScope} the error belongs to. + * @param code SNAKE_CASE error code for the error. + * @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 httpResponseOverride If present, overrides the default HTTP 200 response code for errors. + */ +public record ErrorTemplate( + Constructor constructor, + ErrorFamily family, + ErrorScope scope, + String code, + String title, + String messageTemplate, + Optional httpResponseOverride) { + + public T toException(Map values) { + var errorInstance = toInstance(values); + + try { + return constructor().newInstance(errorInstance); + } catch (InstantiationException | IllegalAccessException | InvocationTargetException e) { + throw new IllegalStateException( + "Failed to create a new instance of " + constructor().getDeclaringClass().getSimpleName(), + e); + } + } + + /** + * Uses the template to create a {@link ErrorInstance} with the provided values. + * + *

As well as the values provided in the params, the template can use "snippets" that are + * defined in the {@link ErrorConfig#DEFAULT_ERROR_CONFIG_FILE} file. See the file for infor on + * how to define them. + * + *

+ * + * @param values The values to use in the template for the error body. + * @return {@link ErrorInstance} created from the template. + * @throws UnresolvedErrorTemplateVariable if the template string for the body of the exception + * has variables e.g ${my_var} that are not in the values passed in + * or not in the {@link ErrorConfig#getSnippetVars()} from the error config. + */ + private ErrorInstance toInstance(Map values) { + + // use the apache string substitution to replace the variables in the messageTemplate + Map allValues = new HashMap<>(values); + allValues.putAll(ErrorConfig.getInstance().getSnippetVars()); + var subs = + new StringSubstitutor(allValues) + .setEnableUndefinedVariableException( + true); // set so IllegalArgumentException thrown if template var missing a value + + String msg; + try { + msg = subs.replace(messageTemplate); + } catch (IllegalArgumentException e) { + throw new UnresolvedErrorTemplateVariable(this, e.getMessage()); + } + + return new ErrorInstance( + UUID.randomUUID(), family, scope, code, title, msg, httpResponseOverride); + } + + /** + * Creates a {@link ErrorTemplate} by loading content from the {@link + * ErrorConfig#DEFAULT_ERROR_CONFIG_FILE} file. + * + *

Checks that the `T` class has a constructor that takes a single {@link ErrorInstance} + * parameter. + * + *

+ * + * @param exceptionClass The class of the exception the template creates. + * @param family The {@link ErrorFamily} the error belongs to. + * @param scope The {@link ErrorScope} the error belongs to. + * @param code The SNAKE_CASE error code for the error. + * @return {@link ErrorTemplate} that the Error Code num can provide to the {@link + * io.stargate.sgv2.jsonapi.exception.ErrorCode} interface. + * @param Type of the {@link APIException} the error code creates. + * @throws IllegalArgumentException if the exceptionClass does not have the + * constructor needed. + */ + public static ErrorTemplate load( + Class exceptionClass, ErrorFamily family, ErrorScope scope, String code) { + + final Constructor constructor; + try { + constructor = exceptionClass.getConstructor(ErrorInstance.class); + } catch (NoSuchMethodException | SecurityException e) { + throw new IllegalArgumentException( + "Failed to find constructor that accepts APIException.ErrorInstance.class for the exception class: " + + exceptionClass.getSimpleName(), + e); + } + + var errorConfig = + ErrorConfig.getInstance() + .getErrorDetail(family, scope.scope(), code) + .orElseThrow( + () -> + new IllegalArgumentException( + String.format( + "Could not find error config for family=%s, scope=%s, code=%s", + family, scope, code))); + + return new ErrorTemplate<>( + constructor, + family, + scope, + code, + errorConfig.title(), + errorConfig.body(), + errorConfig.httpResponseOverride()); + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/FilterException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/FilterException.java new file mode 100644 index 0000000000..15fe033667 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/FilterException.java @@ -0,0 +1,29 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +/** + * Errors related to the filter clause in a request. + * + *

See {@link APIException} for steps to add a new code. + */ +public class FilterException extends RequestException { + public FilterException(ErrorInstance errorInstance) { + super(errorInstance); + } + + public enum Code implements ErrorCode { + MULTIPLE_ID_FILTER, + FIELDS_LIMIT_VIOLATION; + + private final ErrorTemplate template; + + Code() { + template = + ErrorTemplate.load(FilterException.class, ErrorFamily.SERVER, Scope.FILTER, name()); + } + + @Override + public ErrorTemplate template() { + return template; + } + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/RequestException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/RequestException.java new file mode 100644 index 0000000000..b639109c9c --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/RequestException.java @@ -0,0 +1,45 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +/** + * Base for any errors that are from the {@link ErrorFamily#REQUEST} family, these are errors + * related to the structure of the request. + * + *

Scope are defined in {@lnk Scope} and each represents a subclass of this class. + * + *

The {@link Code} in this class is for error codes that do not have a scope. + * + *

See {@link APIException} + */ +public class RequestException extends APIException { + + public RequestException(ErrorInstance errorInstance) { + super(errorInstance); + } + + public enum Scope implements ErrorScope { + /** See {@link FilterException} */ + FILTER; + + @Override + public String scope() { + return name(); + } + } + + public enum Code implements ErrorCode { + // TODO: remove fake error code, just here so it compiles + FAKE_CODE; + + private final ErrorTemplate template; + + Code() { + template = + ErrorTemplate.load(RequestException.class, ErrorFamily.REQUEST, ErrorScope.NONE, name()); + } + + @Override + public ErrorTemplate template() { + return template; + } + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ServerException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ServerException.java new file mode 100644 index 0000000000..03e4c7c35f --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/ServerException.java @@ -0,0 +1,44 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +/** + * Base for any errors that are from the {@link ErrorFamily#SERVER} family, these are server side + * errors not related to the structure of the request itself. + * + *

Scope are defined in {@link Scope} and each represents a subclass of this class. + * + *

See {@link APIException} + */ +public class ServerException extends APIException { + public ServerException(ErrorInstance errorInstance) { + super(errorInstance); + } + + public enum Scope implements ErrorScope { + /** See {@link DatabaseException} */ + DATABASE, + /** See {@link EmbeddingProviderException} */ + EMBEDDING_PROVIDER; + + @Override + public String scope() { + return name(); + } + } + + public enum Code implements ErrorCode { + // TODO: remove fake error code, just here so it compiles + FAKE_CODE; + + private final ErrorTemplate template; + + Code() { + template = + ErrorTemplate.load(ServerException.class, ErrorFamily.SERVER, ErrorScope.NONE, name()); + } + + @Override + public ErrorTemplate template() { + return template; + } + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/UnresolvedErrorTemplateVariable.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/UnresolvedErrorTemplateVariable.java new file mode 100644 index 0000000000..d4d9414962 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/playing/UnresolvedErrorTemplateVariable.java @@ -0,0 +1,35 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +/** + * Raised by the Exception code when a template used to build an error message has a variable in it + * that is not resolved by the values passed when the exception is created. + * + *

E.G. if the template is "too many ${foo}" and the values passed are "bar" and "baz" then this + * exception is raised. + * + *

This SHOULD NOT happen in production code, exceptions should be triggered in testing + * and this exception is raised if the template and the code do not match. Note that it is OK for + * the code to provide more variables than the template uses, but not the other way around. + */ +public class UnresolvedErrorTemplateVariable extends RuntimeException { + + public final ErrorTemplate errorTemplate; + + /** + * Create a new instance. + * + *

This is the message the Apache libs will generate: + * String.format("Cannot resolve variable '%s' (enableSubstitutionInVariables=%s).", + * varName, substitutionInVariablesEnabled)) + * + * + * @param errorTemplate The template we were processing when the error occurred. + * @param message The Apache {@link org.apache.commons.text.StringSubstitutor} raises a {@link + * IllegalArgumentException} with the message above, pass this as the message to this + * exception. + */ + public UnresolvedErrorTemplateVariable(ErrorTemplate errorTemplate, String message) { + super(message); + this.errorTemplate = errorTemplate; + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/embedding/operation/EmbeddingProvider.java b/src/main/java/io/stargate/sgv2/jsonapi/service/embedding/operation/EmbeddingProvider.java index 7644846238..24e9b29706 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/embedding/operation/EmbeddingProvider.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/embedding/operation/EmbeddingProvider.java @@ -116,13 +116,13 @@ protected static boolean acceptsTitanAIDimensions(String modelName) { } /** - * Helper method to replace parameters in a template string with values from a map: placeholders - * are of form {@code {parameterName}} and matching value to look for in the map is String {@code - * "parameterName"}. + * Helper method to replace parameters in a messageTemplate string with values from a map: + * placeholders are of form {@code {parameterName}} and matching value to look for in the map is + * String {@code "parameterName"}. * * @param template Template with placeholders to replace - * @param parameters Parameters to replace in the template - * @return Processed template with replaced parameters + * @param parameters Parameters to replace in the messageTemplate + * @return Processed messageTemplate with replaced parameters */ protected String replaceParameters(String template, Map parameters) { final int len = template.length(); diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml new file mode 100644 index 0000000000..ebbe6505ff --- /dev/null +++ b/src/main/resources/errors.yaml @@ -0,0 +1,83 @@ +# Data API Error Objects V2 +# +# This file contain the error messages that are returned by the Data API +# +# The error descriptions in this file are ready by the APIException classes to create the content for the errors. +# That is, only errors defined in the code are read from this file, see the APIException class in the code +# for a description of how the errors are defined and manged. +# +# Errors have the following hierarchy: +# Family -> (optional) Scope -> Code +# +# 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. +# * 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. +# * Code: A unique string identifying the error. +# +# All values are strings and must be UPPER_SNAKE_CASE_1 supporting upper case alpha and digits. +# +# FILE LAYOUT +# =========== +# +# "snippets" is a list of text snippets than can be included in any error body, the snippets are included in the +# variables when running the tempalte for the body of the error. Snippets are referenced using `${SNIPET.}` +# where is the name of the snippet key. +# Each snippet has: +# - 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. +# 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. +# +# NOTE: Please keep the entries sorted on their name for snippets, or scope and code for errors. Please add a +# new line after each entry, using the `|-` to trim trailing newlines. + +snippets: + - name: CONTACT_SUPPORT + body: |- + Please contact support if the issue persists. + +request-errors: + - scope: FILTER + code: MULTIPLE_ID_FILTER + title: the title for the error + body: |- + big long message with ${vars} in it + - scope: + code: DOCUMENT_UNPARSEABLE + title: Unable to parse the document + body: |- + big long message with ${vars} in it. + + ${SNIPPET.CONTACT_SUPPORT} + +server-errors: + - scope: DATABASE + code: SERVER_READ_FAILED + title: the title for the error + body: big long body with ${vars} in it + + - scope: EMBEDDING_PROVIDER + code: CLIENT_ERROR + title: The Embedding Provider returned a HTTP client error + body: |- + Provider: ${provider}; HTTP Status: ${httpStatus}; Error Message: ${errorMessage} + + - scope: EMBEDDING_PROVIDER + code: SERVER_ERROR + title: The Embedding Provider returned a HTTP client error + body: |- + Provider: ${provider}; HTTP Status: ${httpStatus}; Error Message: ${errorMessage} \ No newline at end of file diff --git a/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/BadExceptionTemplateTest.java b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/BadExceptionTemplateTest.java new file mode 100644 index 0000000000..269a90378a --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/BadExceptionTemplateTest.java @@ -0,0 +1,78 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** + * Test creating the new API Exceptions using YAML templating in the file {@link + * ErrorTestData#TEST_ERROR_CONFIG_FILE} + */ +public class BadExceptionTemplateTest { + + @BeforeAll + public static void setup() throws IOException { + ErrorConfig.unsafeInitializeFromYamlResource(ErrorTestData.MISSING_CTOR_ERROR_CONFIG_FILE); + } + + @AfterAll + public static void teardown() throws IOException { + ErrorConfig.unsafeInitializeFromYamlResource(ErrorConfig.DEFAULT_ERROR_CONFIG_FILE); + } + + @Test + public void missingCtorFails() { + + /// This is tricky, because the exception code is designed to be easy and work. So if our test + // references the ErrorCode enum for a bad exception it will fail but fail as the ENUM is being + // constructed which we cannot catch. + // So need to make the template manually as a workaround, this is what the error code enum would + // do + // but we CANNOT cause the Code enum to be constructed as it will fail. + + // the error ID must match the one in the config file, from setUP + var error = + assertThrows( + IllegalArgumentException.class, + () -> + ErrorTemplate.load( + MissingCtorException.class, + MissingCtorException.FAMILY, + MissingCtorException.SCOPE, + "EXCEPTION_MISSING_CTOR"), + "Trying to make a template for an exception that is missing the required CTOR should fail"); + + assertThat(error) + .message() + .startsWith( + "Failed to find constructor that accepts APIException.ErrorInstance.class for the exception class"); + } + + @Test + public void failingCtorFails() { + + var error = + assertThrows( + IllegalStateException.class, + FailingCtorScopeException.Code.EXCEPTION_FAILING_CTOR::get, + "Trying to make a template for an exception that fails in the constructor should fail"); + + assertThat(error).message().startsWith("Failed to create a new instance of"); + + var reflectionError = error.getCause(); + assertThat(reflectionError) + .as("Error from template wraps the InvocationTargetException from reflection") + .isInstanceOf(InvocationTargetException.class); + + var rootCause = reflectionError.getCause(); + assertThat(rootCause) + .as("Root cause is the exception thrown in the constructor") + .isInstanceOf(RuntimeException.class) + .hasMessage("BANG"); + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorCodeTest.java b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorCodeTest.java new file mode 100644 index 0000000000..77dd4a02d8 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorCodeTest.java @@ -0,0 +1,69 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; + +import java.io.IOException; +import java.util.Map; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** + * Testing the methods on the {@link ErrorCode} interface, not testing the actual error codes. + * + *

Uses config in {@link ErrorTestData#TEST_ERROR_CONFIG_FILE} + */ +public class ErrorCodeTest { + + private final ErrorTestData TEST_DATA = new ErrorTestData(); + + @BeforeAll + public static void setup() throws IOException { + ErrorConfig.unsafeInitializeFromYamlResource(ErrorTestData.TEST_ERROR_CONFIG_FILE); + } + + @AfterAll + public static void teardown() throws IOException { + ErrorConfig.unsafeInitializeFromYamlResource(ErrorConfig.DEFAULT_ERROR_CONFIG_FILE); + } + + @Test + public void allGetWithVarsReturnSame() { + var errorCode = TestScopeException.Code.SCOPED_REQUEST_ERROR; + + var errorFromParamArgs = + assertDoesNotThrow( + () -> + errorCode.get( + "name", TEST_DATA.VAR_NAME, + "value", TEST_DATA.VAR_VALUE)); + + var errorFromMap = + assertDoesNotThrow( + () -> + errorCode.get( + Map.of( + "name", TEST_DATA.VAR_NAME, + "value", TEST_DATA.VAR_VALUE)), + String.format("Creating exception with template %s", errorCode.template())); + + assertThat(errorFromParamArgs) + .describedAs("Error from param args and error from map are same ignoring errorId") + .usingRecursiveComparison() + .ignoringFields("errorId") + .isEqualTo(errorFromMap); + + assertThat(errorFromParamArgs.errorId) + .describedAs("errorId is different between from param args and map") + .isNotEqualTo(errorFromMap.errorId); + } + + @Test + public void getNoVars() { + var errorCode = TestRequestException.Code.NO_VARIABLES_TEMPLATE; + + assertDoesNotThrow( + () -> errorCode.get(), "Can create error that does not have vars in the template "); + } +} 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 new file mode 100644 index 0000000000..66acd75abe --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorConfigTest.java @@ -0,0 +1,267 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; + +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; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +public class ErrorConfigTest { + + @Test + public void initializeFromResource() { + + // test that we load from a resource, and that it can only happen once + // we will assume other tests that just look for an exception from normal code will cause the + // autoloading to happen + // Have to use Unsafe first because cannot guarantee the order tests are run, another test could + // have set it + assertDoesNotThrow( + () -> ErrorConfig.unsafeInitializeFromYamlResource(ErrorConfig.DEFAULT_ERROR_CONFIG_FILE), + "Can load the error config from yaml resource file"); + + assertThrows( + IllegalStateException.class, + () -> ErrorConfig.initializeFromYamlResource(ErrorConfig.DEFAULT_ERROR_CONFIG_FILE), + "Can only load the error config from yaml resource file once"); + + assertDoesNotThrow( + ErrorConfig::getInstance, + "Can get the error config instance after loading from yaml resource file"); + } + + @Test + public void readErrorsYaml() throws Exception { + String yaml = + """ + request-errors: + - 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: + code: TEST_ERROR_ID_2 + title: This error has no scope + body: |- + Line 1 of body + + Line 2 of body + server-errors: + - scope: TEST_SCOPE_3 + code: TEST_ERROR_ID_2 + title: the title for the error + body: big long body with ${vars} in it"""; + + var errorConfig = ErrorConfig.readFromYamlString(yaml); + + assertThat(errorConfig.getErrorDetail(ErrorFamily.REQUEST, "TEST_SCOPE_1", "TEST_ERROR_ID_1")) + .isPresent() + .get() + .satisfies( + e -> { + assertThat(e.scope()).isEqualTo("TEST_SCOPE_1"); + 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")) + .isPresent() + .get() + .satisfies( + e -> { + assertThat(e.scope()).isEmpty(); + 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")) + .isPresent() + .get() + .satisfies( + e -> { + assertThat(e.scope()).isEqualTo("TEST_SCOPE_3"); + 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, + Optional httpStatusOverride, + Class errorClass, + String message) { + + T error = + assertThrows( + errorClass, + () -> new ErrorConfig.ErrorDetail(scope, code, title, body, httpStatusOverride), + "Error Type"); + + assertThat(error.getMessage()).as("Error Message").isEqualTo(message); + } + + private static Stream errorDetailTestCases() { + return Stream.of( + Arguments.of( + "not_snake_scope", + "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", + 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", + 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", + "title", + null, + Optional.empty(), + NullPointerException.class, + "body cannot be null"), + Arguments.of( + "SCOPE", + "CODE", + "title", + "", + Optional.empty(), + IllegalArgumentException.class, + "body cannot be blank"), + Arguments.of( + "SCOPE", + "CODE", + "title", + "body", + null, + NullPointerException.class, + "httpResponseOverride cannot be null")); + } + + @Test + public void readSnippetYaml() throws Exception { + String yaml = + """ + snippets: + - name: SNIPPET_1 + body: |- + Snippet 1 body + - name: SNIPPET_2 + body: |- + Snippet 2 body + + multi line + """; + + var errorConfig = ErrorConfig.readFromYamlString(yaml); + + assertThat( + errorConfig.snippets().stream().filter(e -> e.name().equals("SNIPPET_1")).findFirst()) + .isPresent() + .get() + .satisfies( + s -> { + assertThat(s.name()).isEqualTo("SNIPPET_1"); + assertThat(s.body()).isEqualTo("Snippet 1 body"); + assertThat(errorConfig.getSnippetVars()).containsKey(s.variableName()); + }); + assertThat( + errorConfig.snippets().stream().filter(e -> e.name().equals("SNIPPET_2")).findFirst()) + .isPresent() + .get() + .satisfies( + s -> { + assertThat(s.name()).isEqualTo("SNIPPET_2"); + assertThat(s.body()).isEqualTo("Snippet 2 body\n\nmulti line"); + assertThat(errorConfig.getSnippetVars()).containsKey(s.variableName()); + }); + } + + @ParameterizedTest + @MethodSource("snippetValidationTestCases") + public void snippetValidation( + String name, String body, Class errorClass, String message) { + + T error = assertThrows(errorClass, () -> new ErrorConfig.Snippet(name, body), "Error Type"); + assertEquals(message, error.getMessage(), "Error Message"); + } + + private static Stream snippetValidationTestCases() { + return Stream.of( + Arguments.of(null, "body", NullPointerException.class, "name cannot be null"), + Arguments.of( + "not_snake_name", + "body", + IllegalArgumentException.class, + "name must be in UPPER_SNAKE_CASE_1 format, got: not_snake_name"), + Arguments.of("NAME", null, NullPointerException.class, "body cannot be null"), + Arguments.of("NAME", "", IllegalArgumentException.class, "body cannot be blank")); + } + + @Test + public void snippetsVarsAreImmutable() { + + // do not care where the config comes from + var snippetVars = ErrorConfig.getInstance().getSnippetVars(); + + assertThrows(UnsupportedOperationException.class, () -> snippetVars.put("new", "value")); + assertThrows(UnsupportedOperationException.class, snippetVars::clear); + } + + @Test + public void missingConfigFile() { + assertThrows( + FileNotFoundException.class, + () -> ErrorConfig.unsafeInitializeFromYamlResource("missing.yaml"), + "Error when loading a missing config file"); + } +} 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 new file mode 100644 index 0000000000..c484f39d06 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorTemplateTest.java @@ -0,0 +1,239 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import java.util.Map; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** + * Test creating the new API Exceptions using YAML templating in the file {@link + * ErrorTestData#TEST_ERROR_CONFIG_FILE} + */ +public class ErrorTemplateTest { + + private final ErrorTestData TEST_DATA = new ErrorTestData(); + + @BeforeAll + public static void setup() throws IOException { + ErrorConfig.unsafeInitializeFromYamlResource(ErrorTestData.TEST_ERROR_CONFIG_FILE); + } + + @AfterAll + public static void teardown() throws IOException { + ErrorConfig.unsafeInitializeFromYamlResource(ErrorConfig.DEFAULT_ERROR_CONFIG_FILE); + } + + /** + * Stnadard way to create an exception from template, that expects the name and value params + * + * @param errorCode + * @return + * @param + */ + private T createException(ErrorCode errorCode) { + + T error = + assertDoesNotThrow( + () -> + errorCode.get( + Map.of( + "name", TEST_DATA.VAR_NAME, + "value", TEST_DATA.VAR_VALUE)), + String.format("Creating exception with template %s", errorCode.template())); + + // basic test that the name and value got put into the body + assertThat(error) + .isNotNull() + .satisfies( + e -> { + assertThat(e.body).contains(TEST_DATA.VAR_NAME); + assertThat(e.body).contains(TEST_DATA.VAR_VALUE); + }); + 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() { + + var error = createException(TestScopeException.Code.SCOPED_REQUEST_ERROR); + assertThat(error) + .isNotNull() + .satisfies( + e -> { + 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 " + + TEST_DATA.VAR_NAME + + " and " + + TEST_DATA.VAR_VALUE + + " in it"); + }); + } + + @Test + public void unscopedRequestError() { + + var error = createException(TestRequestException.Code.UNSCOPED_REQUEST_ERROR); + assertThat(error) + .isNotNull() + .satisfies( + e -> { + assertError( + e, + TestRequestException.FAMILY, + "", + TestRequestException.Code.UNSCOPED_REQUEST_ERROR.name(), + "An unscoped request error", + null); + assertThat(e.body) + .contains( + "Multi line with " + + TEST_DATA.VAR_NAME + + " and " + + TEST_DATA.VAR_VALUE + + " in it.\n" + + "And a snippet below:\n" + + "Please contact support if the issue persists."); + }); + } + + @Test + public void missingTemplateVars() { + + var error = + assertThrows( + UnresolvedErrorTemplateVariable.class, + () -> TestScopeException.Code.SCOPED_REQUEST_ERROR.get("name", TEST_DATA.VAR_NAME), + "Missing Vars"); + + assertThat(error) + .as("Error message is for the missing var") + .message() + .startsWith("Cannot resolve variable 'value'"); + + assertThat(error.errorTemplate) + .as("Error template is the one we triggered") + .isNotNull() + .isEqualTo(TestScopeException.Code.SCOPED_REQUEST_ERROR.template()); + } + + @Test + public void apiExceptionMessageIsBody() { + + var error = createException(TestRequestException.Code.UNSCOPED_REQUEST_ERROR); + assertThat(error) + .isNotNull() + .satisfies( + e -> { + assertThat(e.getMessage()).isEqualTo(e.body); + }); + } + + @Test + public void apiExceptionToString() { + + var error = createException(TestRequestException.Code.UNSCOPED_REQUEST_ERROR); + + assertThat(error) + .describedAs("toString should contain all the fields") + .isNotNull() + .satisfies( + e -> { + assertThat(e.toString()).contains(e.getClass().getSimpleName()); + assertThat(e.toString()).contains(e.family.name()); + assertThat(e.toString()).contains(e.scope); + assertThat(e.toString()).contains(e.code); + assertThat(e.toString()).contains(e.title); + assertThat(e.toString()).contains(e.body); + }); + } + + @Test + public void unknownErrorCode() { + + var error = + assertThrows( + IllegalArgumentException.class, + () -> + ErrorTemplate.load( + TestScopeException.class, + TestScopeException.FAMILY, + TestScopeException.SCOPE, + TEST_DATA.RANDOM_STRING), + "Error code in enum but not in yaml"); + + assertThat(error) + .message() + .startsWith( + String.format( + "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/ErrorTestData.java b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorTestData.java new file mode 100644 index 0000000000..84e1827ddd --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/ErrorTestData.java @@ -0,0 +1,19 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +/** + * Re-usable test data for tests on the exceptions, so we dont need to use inheritance where it may + * not make sense. + */ +public class ErrorTestData { + + public static final String TEST_ERROR_CONFIG_FILE = "test_errors.yaml"; + + public static final String MISSING_CTOR_ERROR_CONFIG_FILE = "invalid_exception_errors.yaml"; + + // Variables for templates + public final String VAR_NAME = "name-" + System.currentTimeMillis(); + public final String VAR_VALUE = "value-" + System.currentTimeMillis(); + + // Just a random string to use when needed + public final String RANDOM_STRING = "random-" + System.currentTimeMillis(); +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/FailingCtorScopeException.java b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/FailingCtorScopeException.java new file mode 100644 index 0000000000..d818f1c321 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/FailingCtorScopeException.java @@ -0,0 +1,27 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +/** An exception throws an error when you try to create it See {@link BadExceptionTemplateTest} */ +public class FailingCtorScopeException extends TestRequestException { + + public static final Scope SCOPE = Scope.FAILING_CTOR; + + public FailingCtorScopeException(ErrorInstance errorInstance) { + super(errorInstance); + throw new RuntimeException("BANG"); + } + + public enum Code implements ErrorCode { + EXCEPTION_FAILING_CTOR; + + private final ErrorTemplate template; + + Code() { + template = ErrorTemplate.load(FailingCtorScopeException.class, FAMILY, SCOPE, name()); + } + + @Override + public ErrorTemplate template() { + return template; + } + } +} 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 new file mode 100644 index 0000000000..a0684ca833 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/MissingCtorException.java @@ -0,0 +1,30 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +import java.util.Optional; + +/** + * An exception that is missing the CTOR we need for the templating system. + * + *

This class still has to have a store that calls the super, so it's kind of hard to actually + * make this mistake + * + *

See {@link BadExceptionTemplateTest} + */ +public class MissingCtorException extends TestRequestException { + + public static final Scope SCOPE = Scope.MISSING_CTOR; + + // This ctor is just here to make it compile + public MissingCtorException() { + 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 + // public MissingCtorException(ErrorInstance errorInstance) { + // super(errorInstance); + // } + + // there is no Code enum to avoid confusion, see the test class for why +} 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 new file mode 100644 index 0000000000..e14c400c7d --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/TestRequestException.java @@ -0,0 +1,54 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +/** + * Base for any errors that are from the {@link ErrorFamily#REQUEST} family, these are errors + * related to the structure of the request. + * + *

Scope are defined in {@lnk Scope} and each represents a subclass of this class. + * + *

The {@link Code} in this class is for error codes that do not have a scope. + * + *

See {@link APIException} + */ +public class TestRequestException extends APIException { + + public static final ErrorFamily FAMILY = ErrorFamily.REQUEST; + + public TestRequestException(ErrorInstance errorInstance) { + super(errorInstance); + } + + public TestRequestException( + ErrorFamily family, ErrorScope scope, String code, String title, String message) { + super(family, scope, code, title, message); + } + + public enum Scope implements ErrorScope { + /** See {@link TestScopeException} */ + TEST_REQUEST_SCOPE, + MISSING_CTOR, + FAILING_CTOR; + + @Override + public String scope() { + return name(); + } + } + + public enum Code implements ErrorCode { + UNSCOPED_REQUEST_ERROR, + NO_VARIABLES_TEMPLATE, + HTTP_OVERRIDE; + + private final ErrorTemplate template; + + Code() { + template = ErrorTemplate.load(TestRequestException.class, FAMILY, ErrorScope.NONE, name()); + } + + @Override + public ErrorTemplate template() { + return template; + } + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/TestScopeException.java b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/TestScopeException.java new file mode 100644 index 0000000000..a507967879 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/exception/playing/TestScopeException.java @@ -0,0 +1,30 @@ +package io.stargate.sgv2.jsonapi.exception.playing; + +/** + * Errors related to the filter clause in a request. + * + *

See {@link APIException} for steps to add a new code. + */ +public class TestScopeException extends TestRequestException { + + public static final Scope SCOPE = Scope.TEST_REQUEST_SCOPE; + + public TestScopeException(ErrorInstance errorInstance) { + super(errorInstance); + } + + public enum Code implements ErrorCode { + SCOPED_REQUEST_ERROR; + + private final ErrorTemplate template; + + Code() { + template = ErrorTemplate.load(TestScopeException.class, FAMILY, SCOPE, name()); + } + + @Override + public ErrorTemplate template() { + return template; + } + } +} diff --git a/src/test/resources/invalid_exception_errors.yaml b/src/test/resources/invalid_exception_errors.yaml new file mode 100644 index 0000000000..7a2fb8c94c --- /dev/null +++ b/src/test/resources/invalid_exception_errors.yaml @@ -0,0 +1,14 @@ +# For use when testing the exception classes that do not have ctor or break etc. + +request-errors: + - scope: MISSING_CTOR + code: EXCEPTION_MISSING_CTOR + title: test for exception class missing the ctor needed for templating + body: |- + Scopes are represented by Exception classes, so needs a new scope + + - scope: FAILING_CTOR + code: EXCEPTION_FAILING_CTOR + title: test for exception class that throws error on construction + body: |- + Scopes are represented by Exception classes, so needs a new scope diff --git a/src/test/resources/test_errors.yaml b/src/test/resources/test_errors.yaml new file mode 100644 index 0000000000..4d1912c12e --- /dev/null +++ b/src/test/resources/test_errors.yaml @@ -0,0 +1,40 @@ +# Define some errors just for testing + +snippets: + - name: CONTACT_SUPPORT + body: |- + Please contact support if the issue persists. + +request-errors: + - scope: TEST_REQUEST_SCOPE + code: SCOPED_REQUEST_ERROR + title: A scoped request error + body: |- + long body with ${name} and ${value} in it + + - scope: MISSING_CTOR + code: EXCEPTION_MISSING_CTOR + title: test for exception class missing the ctor needed for templating + body: |- + Scopes are represented by Exception classes, so needs a new scope + + - scope: + code: UNSCOPED_REQUEST_ERROR + title: An unscoped request error + body: |- + Multi line with ${name} and ${value} in it. + And a snippet below: + ${SNIPPET.CONTACT_SUPPORT} + + - scope: + code: NO_VARIABLES_TEMPLATE + title: A body with no template variables + body: |- + 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