From 756f6150ef1f6ed844bd6f788d60142806e92eef Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Wed, 9 Oct 2024 15:56:02 +1300 Subject: [PATCH] Use WarningException, expand CommandResultBuilder Changes the warnings associated commands to use the new ApiExcpetions, and `warnings` in the return status now returns a list of error object V2. To do that needed to improve the way CommandResult was built, so it always had a status map so the CommandProcessor could append a warning. To do that expanded the CommandResultBuilder added for the OperationAttempts, removed the many overloads used for CommandResult, and updated all creation of the CommandResult to use the builder. See also https://github.com/stargate/data-api/issues/1518 to continue this --- .../api/model/command/CommandError.java | 84 ++++++++++ .../api/model/command/CommandErrorV2.java | 147 ++++++++++++++++++ .../api/model/command/CommandResult.java | 140 +++++------------ .../model/command/CommandResultBuilder.java | 56 ++++--- .../api/model/command/DeprecatedCommand.java | 19 +-- .../api/model/command/ResponseData.java | 69 ++++++++ .../challenge/impl/ErrorChallengeSender.java | 4 +- .../APIExceptionCommandErrorBuilder.java | 48 +++++- .../jsonapi/exception/JsonApiException.java | 14 +- .../jsonapi/exception/WarningException.java | 3 +- .../ConstraintViolationExceptionMapper.java | 10 +- .../ThrowableCommandResultSupplier.java | 12 +- .../WebApplicationExceptionMapper.java | 6 +- .../service/operation/InsertAttemptPage.java | 11 +- .../operation/InsertOperationPage.java | 43 +++-- .../service/operation/OperationAttempt.java | 12 +- .../service/operation/ReadAttemptPage.java | 10 +- .../service/operation/ReadOperationPage.java | 29 ++-- .../service/operation/SchemaAttemptPage.java | 9 +- .../collections/CountOperationPage.java | 10 +- .../collections/DeleteOperationPage.java | 35 +++-- .../collections/EstimatedCountResult.java | 5 +- .../FindCollectionsCollectionOperation.java | 11 +- .../collections/SchemaChangeResult.java | 5 +- .../ServiceRegistrationResult.java | 3 +- .../UpdateCollectionOperationPage.java | 43 +++-- .../FindEmbeddingProvidersOperation.java | 7 +- .../keyspaces/FindKeyspacesOperation.java | 11 +- .../tables/DeleteTableOperation.java | 8 +- .../tables/TableReadAttemptBuilder.java | 4 +- .../tables/UpdateTableOperation.java | 10 +- .../service/processor/CommandProcessor.java | 17 +- .../processor/MeteredCommandProcessor.java | 10 +- src/main/resources/errors.yaml | 11 ++ .../CommandResultToRestResponseTest.java | 44 ++++-- .../APIExceptionCommandErrorBuilderTest.java | 55 ++++++- .../MeteredCommandProcessorTest.java | 11 +- 37 files changed, 697 insertions(+), 329 deletions(-) create mode 100644 src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandError.java create mode 100644 src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandErrorV2.java create mode 100644 src/main/java/io/stargate/sgv2/jsonapi/api/model/command/ResponseData.java diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandError.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandError.java new file mode 100644 index 000000000..5af72cade --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandError.java @@ -0,0 +1,84 @@ +package io.stargate.sgv2.jsonapi.api.model.command; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonInclude; +import jakarta.ws.rs.core.Response; +import java.util.*; +import org.eclipse.microprofile.openapi.annotations.media.Schema; + +/** + * Intended to replace the {@link CommandResult.Error} with something that has all the fields in an + * error. + * + *

This super class is the legacy V1 structure, and the subclass {@link CommandErrorV2} is the + * new V2 structure. Once we have fully removed the {@link CommandResult.Error} we can remove this + * class and just have the V2 structure. + * + *

This object is intended to be used in the CommandResult, and is serialised into the JSON + * response. + * + *

aaron - 9 -oct-2024 - This is initially only used with the Warnings in the CommandResult, + * further work needed to use it in all places. Warnings are in the status, and not described on the + * swagger for the CommandResult. This is why all the decorators from the original class are not + * here yet, but they have things to ignore flagged just incase + */ +public class CommandError { + + private final String errorCode; + private final String message; + private final Response.Status httpStatus; + private final String errorClass; + private final Map metricsTags; + + public CommandError( + String errorCode, + String message, + Response.Status httpStatus, + String errorClass, + Map metricsTags) { + + this.errorCode = requireNoNullOrBlank(errorCode, "errorCode cannot be null or blank"); + this.message = requireNoNullOrBlank(message, "message cannot be null or blank"); + this.httpStatus = Objects.requireNonNull(httpStatus, "httpStatus cannot be null"); + + this.metricsTags = metricsTags == null ? Collections.emptyMap() : Map.copyOf(metricsTags); + // errorClass is not required, it is only passed when we are in debug mode + // normalise to null if blank + this.errorClass = errorClass == null || errorClass.isBlank() ? null : errorClass; + } + + protected static String requireNoNullOrBlank(String value, String message) { + if (Objects.isNull(value) || value.isBlank()) { + throw new IllegalArgumentException(message); + } + return value; + } + + /** + * @return + */ + public String errorCode() { + return errorCode; + } + + public String message() { + return message; + } + + @JsonIgnore + public Response.Status httpStatus() { + return httpStatus; + } + + @JsonIgnore + @Schema(hidden = true) + public Map metricsTags() { + return metricsTags; + } + + @Schema(hidden = true) + @JsonInclude(JsonInclude.Include.NON_NULL) + public String exceptionClass() { + return errorClass; + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandErrorV2.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandErrorV2.java new file mode 100644 index 000000000..41e8b4dcf --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandErrorV2.java @@ -0,0 +1,147 @@ +package io.stargate.sgv2.jsonapi.api.model.command; + +import jakarta.ws.rs.core.Response; +import java.util.Map; +import java.util.Objects; +import java.util.UUID; + +/** + * See {@link CommandError} for why this class exists. + * + *

Use either {@link #builderV1()} or {@link #builderV2()} to get te builder to create an + * instance of this class. + */ +public class CommandErrorV2 extends CommandError { + + private final String family; + private final String scope; + private final String title; + private final UUID id; + + public CommandErrorV2( + String errorCode, + String message, + Response.Status httpStatus, + String errorClass, + Map metricsTags, + String family, + String scope, + String title, + UUID id) { + super(errorCode, message, httpStatus, errorClass, metricsTags); + this.family = requireNoNullOrBlank(family, "family cannot be null or blank"); + this.scope = scope == null ? "" : scope; + this.title = requireNoNullOrBlank(title, "title cannot be null or blank"); + this.id = Objects.requireNonNull(id, "id cannot be null"); + } + + /** Create a new builder for the {@link CommandError} that represents the V1 error object. */ + public static Builder builderV1() { + return new Builder<>(true); + } + + /** Create a new builder for the {@link CommandErrorV2} that represents the V2 error object. */ + public static Builder builderV2() { + return new Builder<>(false); + } + + public String family() { + return family; + } + + public String scope() { + return scope; + } + + public String title() { + return title; + } + + public UUID id() { + return id; + } + + public static class Builder { + private String errorCode; + private String message; + private Response.Status httpStatus; + private String errorClass; + private Map metricsTags; + private String family; + private String scope; + private String title; + private UUID id; + + private final boolean v1Error; + + Builder(boolean v1Error) { + this.v1Error = v1Error; + } + + public Builder errorCode(String errorCode) { + this.errorCode = errorCode; + return this; + } + + public Builder message(String message) { + this.message = message; + return this; + } + + public Builder httpStatus(Response.Status httpStatus) { + this.httpStatus = httpStatus; + return this; + } + + public Builder errorClass(String errorClass) { + this.errorClass = errorClass; + return this; + } + + public Builder metricsTags(Map metricsTags) { + this.metricsTags = metricsTags; + return this; + } + + public Builder family(String family) { + this.family = family; + return this; + } + + public Builder scope(String scope) { + this.scope = scope; + return this; + } + + public Builder title(String title) { + this.title = title; + return this; + } + + public Builder id(UUID id) { + this.id = id; + return this; + } + + public T build() { + return v1Error + ? unchecked(new CommandError(errorCode, message, httpStatus, errorClass, metricsTags)) + : unchecked( + new CommandErrorV2( + errorCode, + message, + httpStatus, + errorClass, + metricsTags, + family, + scope, + title, + id)); + } + + @SuppressWarnings("unchecked") + private T unchecked(CommandError error) { + return (T) error; + } + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResult.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResult.java index c1ead5541..b910644af 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResult.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResult.java @@ -3,9 +3,7 @@ import com.fasterxml.jackson.annotation.JsonAnyGetter; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; -import com.fasterxml.jackson.databind.JsonNode; import io.stargate.sgv2.jsonapi.exception.ErrorCodeV1; -import jakarta.validation.constraints.NotNull; import jakarta.ws.rs.core.Response; import java.util.*; import org.eclipse.microprofile.openapi.annotations.enums.SchemaType; @@ -29,7 +27,7 @@ public record CommandResult( description = "A response data holding documents that were returned as the result of a command.", nullable = true, - oneOf = {CommandResult.MultiResponseData.class, CommandResult.SingleResponseData.class}) + oneOf = {ResponseData.MultiResponseData.class, ResponseData.SingleResponseData.class}) ResponseData data, @Schema( description = @@ -44,105 +42,51 @@ public record CommandResult( implementation = String.class, nullable = true) }) + @JsonInclude(JsonInclude.Include.NON_EMPTY) Map status, @JsonInclude(JsonInclude.Include.NON_EMPTY) @Schema(nullable = true) List errors) { - /** - * Constructor for only specifying the {@link MultiResponseData}. - * - * @param responseData {@link MultiResponseData} - */ - public CommandResult(ResponseData responseData) { - this(responseData, null, null); - } - - /** - * Constructor for specifying the {@link MultiResponseData} and statuses. - * - * @param responseData {@link MultiResponseData} - * @param status Map of status information. - */ - public CommandResult(ResponseData responseData, Map status) { - this(responseData, status, null); - } + public CommandResult { - /** - * Constructor for only specifying the status. - * - * @param status Map of status information. - */ - public CommandResult(Map status) { - this(null, status, null); + // make both of these not null, so they can be mutated if needed + // the decorators on the record fields tell Jackson to exclude when they are null or empty + if (null == status) { + status = new HashMap<>(); + } + if (null == errors) { + errors = new ArrayList<>(); + } } /** - * Constructor for only specifying the errors. + * Get a builder for the {@link CommandResult} for a single document response, see {@link + * CommandResultBuilder} * - * @param errors List of errors. + *

NOTE: aaron 9-oct-2024 I kept the errorObjectV2 and debugMode params to make it clear + * how inconsistency we are configuring these settings. Ultimately useErrorObjectV2 will go away, + * but we will still have the debugMode setting. I will create ticket so that we create the + * builder in resolver or similar and then pass it around rather than creating in many places. + * Also the {@link io.stargate.sgv2.jsonapi.service.operation.OperationAttemptPageBuilder} is how + * things will turn out. */ - public CommandResult(List errors) { - this(null, null, errors); + public static CommandResultBuilder singleDocumentBuilder( + boolean useErrorObjectV2, boolean debugMode) { + return new CommandResultBuilder( + CommandResultBuilder.ResponseType.SINGLE_DOCUMENT, useErrorObjectV2, debugMode); } - public interface ResponseData { - - /** - * @return Simple shared method to get the response documents. Usually used only in tests, - * ignored in JSON response. - */ - @JsonIgnore - List getResponseDocuments(); - } - - /** - * Response data object that's included in the {@link CommandResult}, for a single document - * responses. - * - * @param document Document. - */ - @Schema(description = "Response data for a single document commands.") - public record SingleResponseData( - @NotNull - @Schema( - description = "Document that resulted from a command.", - type = SchemaType.OBJECT, - implementation = Object.class, - nullable = true) - JsonNode document) - implements ResponseData { - - /** {@inheritDoc} */ - @Override - public List getResponseDocuments() { - return List.of(document); - } + /** See {@link #singleDocumentBuilder(boolean, boolean)} */ + public static CommandResultBuilder multiDocumentBuilder( + boolean useErrorObjectV2, boolean debugMode) { + return new CommandResultBuilder( + CommandResultBuilder.ResponseType.MULTI_DOCUMENT, useErrorObjectV2, debugMode); } - /** - * Response data object that's included in the {@link CommandResult}, for multi document - * responses. - * - * @param documents Documents. - * @param nextPageState Optional next page state. - */ - @Schema(description = "Response data for multiple documents commands.") - public record MultiResponseData( - @NotNull - @Schema( - description = "Documents that resulted from a command.", - type = SchemaType.ARRAY, - implementation = Object.class, - minItems = 0) - List documents, - @Schema(description = "Next page state for pagination.", nullable = true) - String nextPageState) - implements ResponseData { - - /** {@inheritDoc} */ - @Override - public List getResponseDocuments() { - return documents; - } + /** See {@link #singleDocumentBuilder(boolean, boolean)} */ + public static CommandResultBuilder statusOnlyBuilder( + boolean useErrorObjectV2, boolean debugMode) { + return new CommandResultBuilder( + CommandResultBuilder.ResponseType.STATUS_ONLY, useErrorObjectV2, debugMode); } /** @@ -202,19 +146,11 @@ public RestResponse toRestResponse() { * returned a new CommandResult with warning message added in status map * * @param warning message - * @return CommandResult */ - public CommandResult withWarning(String warning) { - Map newStatus = new HashMap<>(); - if (status != null) { - newStatus.putAll(status); - } - List newWarnings = - newStatus.get(CommandStatus.WARNINGS) != null - ? new ArrayList<>((List) newStatus.get(CommandStatus.WARNINGS)) - : new ArrayList<>(); - newWarnings.add(warning); - newStatus.put(CommandStatus.WARNINGS, newWarnings); - return new CommandResult(this.data, newStatus, errors); + public void addWarning(CommandErrorV2 warning) { + List warnings = + (List) + status.computeIfAbsent(CommandStatus.WARNINGS, k -> new ArrayList<>()); + warnings.add(warning); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResultBuilder.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResultBuilder.java index f06fc3555..f37ba7cf2 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResultBuilder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/CommandResultBuilder.java @@ -4,12 +4,18 @@ import io.stargate.sgv2.jsonapi.exception.APIException; import io.stargate.sgv2.jsonapi.exception.APIExceptionCommandErrorBuilder; import io.stargate.sgv2.jsonapi.exception.mappers.ThrowableToErrorMapper; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.function.Function; - +import java.util.*; + +/** + * A basic builder pattern for creating a {@link CommandResult} object, so we create it easily and + * more reliably. + * + *

See https://github.com/stargate/data-api/issues/1518 for future changes. + * + *

This is trying to codify the different ways the CommandResult was created, and it still needs + * to handle the migration from the old error object to the new error object. It should be improved + * in the ticket above, and once we finish the migration, we can remove the old error object. + */ public class CommandResultBuilder { public enum ResponseType { @@ -21,7 +27,8 @@ public enum ResponseType { private final Map cmdStatus = new HashMap<>(); private final List cmdErrors = new ArrayList<>(); private final List documents = new ArrayList<>(); - private final List warnings = new ArrayList<>(); + // Warnings are always the V2 error structure + private final List warnings = new ArrayList<>(); private String nextPageState = null; @@ -34,10 +41,9 @@ public enum ResponseType { private final boolean useErrorObjectV2; // Created in the Ctor - private final Function apiExceptionToError; + private final APIExceptionCommandErrorBuilder apiExceptionToError; - public CommandResultBuilder( - ResponseType responseType, boolean useErrorObjectV2, boolean debugMode) { + CommandResultBuilder(ResponseType responseType, boolean useErrorObjectV2, boolean debugMode) { this.responseType = responseType; this.useErrorObjectV2 = useErrorObjectV2; this.debugMode = debugMode; @@ -50,12 +56,22 @@ public CommandResultBuilder addStatus(CommandStatus status, Object value) { return this; } - public CommandResultBuilder addThrowable(Throwable thorwable) { - var error = throwableToCommandError(thorwable); - return addCommandError(error); + public CommandResultBuilder addThrowable(List throwables) { + Objects.requireNonNull(throwables, "throwables cannot be null").forEach(this::addThrowable); + return this; + } + + public CommandResultBuilder addThrowable(Throwable throwable) { + var error = throwableToCommandError(throwable); + return addCommandResultError(error); + } + + public CommandResultBuilder addCommandResultError(List errors) { + Objects.requireNonNull(errors, "errors cannot be null").forEach(this::addCommandResultError); + return this; } - public CommandResultBuilder addCommandError(CommandResult.Error error) { + public CommandResultBuilder addCommandResultError(CommandResult.Error error) { cmdErrors.add(error); return this; } @@ -70,8 +86,10 @@ public CommandResultBuilder addDocuments(List documents) { return this; } - public CommandResultBuilder addWarning(String warning) { - warnings.add(warning); + public CommandResultBuilder addWarning(APIException warning) { + warnings.add( + apiExceptionToError.buildCommandErrorV2( + Objects.requireNonNull(warning, "warning cannot be null"))); return this; } @@ -89,7 +107,7 @@ public CommandResult.Error throwableToCommandError(Throwable throwable) { // new v2 error object, with family etc. // the builder will handle the debug mode and extended errors settings to return a V1 or V2 // error - return apiExceptionToError.apply(apiException); + return apiExceptionToError.buildLegacyCommandResultError(apiException); } // the mapper handles error object v2 part @@ -132,8 +150,8 @@ public CommandResult build() { case SINGLE_DOCUMENT -> documents.isEmpty() ? null - : new CommandResult.SingleResponseData(documents.getFirst()); - case MULTI_DOCUMENT -> new CommandResult.MultiResponseData(documents, nextPageState); + : new ResponseData.SingleResponseData(documents.getFirst()); + case MULTI_DOCUMENT -> new ResponseData.MultiResponseData(documents, nextPageState); case STATUS_ONLY -> null; }; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/DeprecatedCommand.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/DeprecatedCommand.java index 01db3ceec..c5121f3d0 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/DeprecatedCommand.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/DeprecatedCommand.java @@ -1,6 +1,7 @@ package io.stargate.sgv2.jsonapi.api.model.command; -import io.stargate.sgv2.jsonapi.api.model.command.impl.*; +import io.stargate.sgv2.jsonapi.exception.APIException; +import io.stargate.sgv2.jsonapi.exception.WarningException; /** * All deprecated commands will implement this interface. It has default deprecation message @@ -16,16 +17,10 @@ public interface DeprecatedCommand extends Command { */ Command.CommandName useCommandName(); - /** - * A warning message will be added to commandResult when deprecated command is used. Template: - * This ${commandName} has been deprecated and will be removed in future releases, use - * ${useCommandName} instead. - * - * @return String - */ - default String getDeprecationMessage() { - return String.format( - "This %s has been deprecated and will be removed in future releases, use %s instead.", - this.commandName().getApiName(), useCommandName().getApiName()); + /** A warning message will be added to commandResult when deprecated command is used. */ + default APIException getDeprecationWarning() { + return WarningException.Code.DEPRECATED_COMMAND.get( + "deprecatedCommand", this.commandName().getApiName(), + "replacementCommand", useCommandName().getApiName()); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/ResponseData.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/ResponseData.java new file mode 100644 index 000000000..9c4ad0709 --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/ResponseData.java @@ -0,0 +1,69 @@ +package io.stargate.sgv2.jsonapi.api.model.command; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.databind.JsonNode; +import jakarta.validation.constraints.NotNull; +import java.util.List; +import org.eclipse.microprofile.openapi.annotations.enums.SchemaType; +import org.eclipse.microprofile.openapi.annotations.media.Schema; + +public interface ResponseData { + + /** + * @return Simple shared method to get the response documents. Usually used only in tests, ignored + * in JSON response. + */ + @JsonIgnore + List getResponseDocuments(); + + /** + * Response data object that's included in the {@link CommandResult}, for a single document + * responses. + * + * @param document Document. + */ + @Schema(description = "Response data for a single document commands.") + record SingleResponseData( + @NotNull + @Schema( + description = "Document that resulted from a command.", + type = SchemaType.OBJECT, + implementation = Object.class, + nullable = true) + JsonNode document) + implements ResponseData { + + /** {@inheritDoc} */ + @Override + public List getResponseDocuments() { + return List.of(document); + } + } + + /** + * Response data object that's included in the {@link CommandResult}, for multi document + * responses. + * + * @param documents Documents. + * @param nextPageState Optional next page state. + */ + @Schema(description = "Response data for multiple documents commands.") + record MultiResponseData( + @NotNull + @Schema( + description = "Documents that resulted from a command.", + type = SchemaType.ARRAY, + implementation = Object.class, + minItems = 0) + List documents, + @Schema(description = "Next page state for pagination.", nullable = true) + String nextPageState) + implements ResponseData { + + /** {@inheritDoc} */ + @Override + public List getResponseDocuments() { + return documents; + } + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/security/challenge/impl/ErrorChallengeSender.java b/src/main/java/io/stargate/sgv2/jsonapi/api/security/challenge/impl/ErrorChallengeSender.java index c76abd7dc..1a165b66c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/security/challenge/impl/ErrorChallengeSender.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/security/challenge/impl/ErrorChallengeSender.java @@ -13,7 +13,6 @@ import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import java.util.Collections; -import java.util.List; import java.util.function.BiFunction; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,7 +44,8 @@ public ErrorChallengeSender(ObjectMapper objectMapper) { CommandResult.Error error = new CommandResult.Error( message, Collections.emptyMap(), Collections.emptyMap(), Response.Status.UNAUTHORIZED); - commandResult = new CommandResult(List.of(error)); + commandResult = + CommandResult.statusOnlyBuilder(false, false).addCommandResultError(error).build(); } /** {@inheritDoc} */ diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/APIExceptionCommandErrorBuilder.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/APIExceptionCommandErrorBuilder.java index 91d2a2005..3fdf5026c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/APIExceptionCommandErrorBuilder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/APIExceptionCommandErrorBuilder.java @@ -1,20 +1,21 @@ package io.stargate.sgv2.jsonapi.exception; +import io.stargate.sgv2.jsonapi.api.model.command.CommandErrorV2; import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.config.constants.ErrorObjectV2Constants; import jakarta.ws.rs.core.Response; import java.util.HashMap; import java.util.Map; -import java.util.function.Function; /** * Builder that creates a {@link CommandResult.Error} from an {@link APIException}. * *

This class encapsulates the mapping between the APIException and the API tier to keep it out - * of the core exception classes. + * of the core exception classes. NOTE: aaron 9-oct-2024 needed to tweak this class to work + * with the new CommandErrorV2, once we have rolled out the use of CommandErrorV2 everywhere we can + * remove the legacy CommandResult.Error */ -public class APIExceptionCommandErrorBuilder - implements Function { +public class APIExceptionCommandErrorBuilder { private final boolean debugEnabled; private final boolean returnErrorObjectV2; @@ -40,8 +41,7 @@ public APIExceptionCommandErrorBuilder(boolean debugEnabled, boolean returnError * @param apiException the exception that is going to be returned. * @return a {@link CommandResult.Error} that represents the apiException. */ - @Override - public CommandResult.Error apply(APIException apiException) { + public CommandResult.Error buildLegacyCommandResultError(APIException apiException) { // Note, in the old JsonApiException the code also traverses the cause, we do not want to do // that in // error objects V2 because the proper error is created by the template etc. @@ -75,6 +75,42 @@ public CommandResult.Error apply(APIException apiException) { Response.Status.fromStatusCode(apiException.httpStatus)); } + /** + * Create a new instance that will create a {@link CommandErrorV2} that represents the + * apiException. + * + * @param apiException the exception that is going to be returned. + * @return a {@link CommandErrorV2} that represents the apiException. + */ + public CommandErrorV2 buildCommandErrorV2(APIException apiException) { + if (!returnErrorObjectV2) { + // aaron - oct 9 - I know this seems silly, we are in the process on moving all the errors to + // the V2 + // i added this function to be used with WARNING errors, once we have rolled out the use of + // CommandErrorV2 + // everywhere we wont need this test and there will be one build function + throw new IllegalStateException( + "Cannot build a V2 error object when returnErrorObjectV2 is false"); + } + + var builder = CommandErrorV2.builderV2(); + + if (debugEnabled) { + builder.errorClass(apiException.getClass().getSimpleName()); + } + + return builder + .errorCode(apiException.code) + .message(apiException.body) + .httpStatus(Response.Status.fromStatusCode(apiException.httpStatus)) + .metricsTags(tagsForMetrics(apiException)) + .family(apiException.family.name()) + .scope(apiException.scope) + .title(apiException.title) + .id(apiException.errorId) + .build(); + } + private Map tagsForMetrics(APIException apiException) { // These tags must be backwards compatible with how we tracked before return Map.of( diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/JsonApiException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/JsonApiException.java index 86d3b06b3..9f273e6ed 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/JsonApiException.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/JsonApiException.java @@ -134,17 +134,17 @@ public CommandResult get() { if (message == null) { message = errorCode.getMessage(); } - // construct and return - CommandResult.Error error = getCommandResultError(message, httpStatus); + var builder = CommandResult.statusOnlyBuilder(false, false); + + // construct and return + builder.addCommandResultError(getCommandResultError(message, httpStatus)); // handle cause as well Throwable cause = getCause(); - if (null == cause) { - return new CommandResult(List.of(error)); - } else { - CommandResult.Error causeError = ThrowableToErrorMapper.getMapperFunction().apply(cause); - return new CommandResult(List.of(error, causeError)); + if (null != cause) { + builder.addCommandResultError(ThrowableToErrorMapper.getMapperFunction().apply(cause)); } + return builder.build(); } public CommandResult.Error getCommandResultError(String message, Response.Status status) { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/WarningException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/WarningException.java index 7daeb2b68..e19419bff 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/WarningException.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/WarningException.java @@ -17,7 +17,8 @@ public enum Code implements ErrorCode { MISSING_INDEX, NOT_EQUALS_UNSUPPORTED_BY_INDEXING, ZERO_FILTER_OPERATIONS, - INCOMPLETE_PRIMARY_KEY_FILTER; + INCOMPLETE_PRIMARY_KEY_FILTER, + DEPRECATED_COMMAND; private final ErrorTemplate template; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ConstraintViolationExceptionMapper.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ConstraintViolationExceptionMapper.java index 2ee65ca57..29ca731be 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ConstraintViolationExceptionMapper.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ConstraintViolationExceptionMapper.java @@ -7,7 +7,6 @@ import jakarta.validation.ConstraintViolation; import jakarta.validation.ConstraintViolationException; import jakarta.ws.rs.core.Response; -import java.util.List; import java.util.Map; import java.util.Set; import org.jboss.resteasy.reactive.RestResponse; @@ -35,11 +34,14 @@ public RestResponse constraintViolationException( ConstraintViolationException exception) { // map all violations to errors Set> violations = exception.getConstraintViolations(); - List errors = - violations.stream().map(ConstraintViolationExceptionMapper::getError).distinct().toList(); + var builder = CommandResult.statusOnlyBuilder(false, false); + violations.stream() + .map(ConstraintViolationExceptionMapper::getError) + .distinct() + .forEach(builder::addCommandResultError); // return result - return new CommandResult(errors).toRestResponse(); + return builder.build().toRestResponse(); } private static CommandResult.Error getError(ConstraintViolation violation) { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ThrowableCommandResultSupplier.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ThrowableCommandResultSupplier.java index c7d569b83..090319f12 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ThrowableCommandResultSupplier.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ThrowableCommandResultSupplier.java @@ -1,7 +1,6 @@ package io.stargate.sgv2.jsonapi.exception.mappers; import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; -import java.util.List; import java.util.function.Supplier; /** @@ -14,13 +13,14 @@ public record ThrowableCommandResultSupplier(Throwable t) implements Supplier webApplicationExceptionMapper(WebApplicationE var errorBuilder = new APIExceptionCommandErrorBuilder( debugModeConfig.enabled(), operationsConfig.extendError()); - return RestResponse.ok(new CommandResult(List.of(errorBuilder.apply(ae)))); + return RestResponse.ok( + CommandResult.statusOnlyBuilder(false, false) + .addCommandResultError(errorBuilder.buildLegacyCommandResultError(ae)) + .build()); } CommandResult commandResult = new ThrowableCommandResultSupplier(toReport).get(); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/InsertAttemptPage.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/InsertAttemptPage.java index e68d9009a..8c4094853 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/InsertAttemptPage.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/InsertAttemptPage.java @@ -123,7 +123,7 @@ private void buildPerDocumentResult() { new InsertionResult(attempt.docRowID().orElseThrow(), InsertionStatus.SKIPPED, null); } - seenErrors.forEach(resultBuilder::addCommandError); + seenErrors.forEach(resultBuilder::addCommandResultError); resultBuilder.addStatus(CommandStatus.DOCUMENT_RESPONSES, Arrays.asList(results)); maybeAddSchema(); } @@ -192,11 +192,10 @@ public Builder returnDocumentResponses(boolean returnDocumentResponses) @Override public InsertAttemptPage getOperationPage() { - var resultBuilder = - new CommandResultBuilder( - CommandResultBuilder.ResponseType.STATUS_ONLY, useErrorObjectV2, debugMode); - - return new InsertAttemptPage<>(attempts, resultBuilder, returnDocumentResponses); + return new InsertAttemptPage<>( + attempts, + CommandResult.statusOnlyBuilder(useErrorObjectV2, debugMode), + returnDocumentResponses); } } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/InsertOperationPage.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/InsertOperationPage.java index 47d25f0ec..8524dd6eb 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/InsertOperationPage.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/InsertOperationPage.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonPropertyOrder; import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; +import io.stargate.sgv2.jsonapi.api.model.command.CommandResultBuilder; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; import io.stargate.sgv2.jsonapi.exception.APIException; import io.stargate.sgv2.jsonapi.exception.APIExceptionCommandErrorBuilder; @@ -11,7 +12,6 @@ import io.stargate.sgv2.jsonapi.service.shredding.DocRowIdentifer; import java.util.*; import java.util.function.BiConsumer; -import java.util.function.Function; import java.util.function.Supplier; /** @@ -45,7 +45,7 @@ public class InsertOperationPage private final boolean useErrorObjectV2; // Created in the Ctor - private final Function apiExceptionToError; + private final APIExceptionCommandErrorBuilder apiExceptionToError; /** Create an instance that has debug false and useErrorIbhectV2 false */ public InsertOperationPage( @@ -102,11 +102,8 @@ public CommandResult get() { // TODO AARON used to only sort the success list when not returning detailed responses, check OK Collections.sort(successfulInsertions); - if (!returnDocumentResponses) { - return nonPerDocumentResult(); - } - - return perDocumentResult(); + var builder = CommandResult.statusOnlyBuilder(useErrorObjectV2, debugMode); + return returnDocumentResponses ? perDocumentResult(builder) : nonPerDocumentResult(builder); } /** @@ -116,7 +113,7 @@ public CommandResult get() { * * @return Command result */ - private CommandResult perDocumentResult() { + private CommandResult perDocumentResult(CommandResultBuilder builder) { // New style output: detailed responses. InsertionResult[] results = new InsertionResult[allInsertions.size()]; List errors = new ArrayList<>(); @@ -126,6 +123,7 @@ private CommandResult perDocumentResult() { results[okInsertion.position()] = new InsertionResult(okInsertion.docRowID().orElseThrow(), InsertionStatus.OK, null); } + // Second: failed insertions; output in order of insertion for (var failedInsertion : failedInsertions) { // TODO AARON - confirm the null handling in the getError @@ -152,11 +150,10 @@ private CommandResult perDocumentResult() { allInsertions.get(i).docRowID().orElseThrow(), InsertionStatus.SKIPPED, null); } } - Map status = new HashMap<>(); - status.put(CommandStatus.DOCUMENT_RESPONSES, Arrays.asList(results)); - maybeAddSchema(status); + builder.addStatus(CommandStatus.DOCUMENT_RESPONSES, Arrays.asList(results)); + maybeAddSchema(builder); - return new CommandResult(null, status, errors); + return builder.build(); } /** @@ -166,12 +163,9 @@ private CommandResult perDocumentResult() { * * @return Command result */ - private CommandResult nonPerDocumentResult() { + private CommandResult nonPerDocumentResult(CommandResultBuilder builder) { - List errors = - failedInsertions.isEmpty() - ? null - : failedInsertions.stream().map(this::getErrorObject).toList(); + failedInsertions.stream().map(this::getErrorObject).forEach(builder::addCommandResultError); // Note: See DocRowIdentifer, it has an attribute that will be called for JSON serialization List insertedIds = @@ -180,11 +174,10 @@ private CommandResult nonPerDocumentResult() { .map(Optional::orElseThrow) .toList(); - Map status = new HashMap<>(); - status.put(CommandStatus.INSERTED_IDS, insertedIds); - maybeAddSchema(status); + builder.addStatus(CommandStatus.INSERTED_IDS, insertedIds); + maybeAddSchema(builder); - return new CommandResult(null, status, errors); + return builder.build(); } /** @@ -194,16 +187,16 @@ private CommandResult nonPerDocumentResult() { *

Uses the first, not the first successful, because we may fail to do an insert but will still * have the _id or PK to report. * - * @param status Map to add the status to + * @param builder CommandResult builder to add the status to */ - private void maybeAddSchema(Map status) { + private void maybeAddSchema(CommandResultBuilder builder) { if (allInsertions.isEmpty()) { return; } allInsertions .getFirst() .schemaDescription() - .ifPresent(o -> status.put(CommandStatus.PRIMARY_KEY_SCHEMA, o)); + .ifPresent(o -> builder.addStatus(CommandStatus.PRIMARY_KEY_SCHEMA, o)); } /** @@ -216,7 +209,7 @@ private CommandResult.Error getErrorObject(InsertAttempt insertAttempt) // new v2 error object, with family etc. // the builder will handle the debug mode and extended errors settings to return a V1 or V2 // error - return apiExceptionToError.apply(apiException); + return apiExceptionToError.buildLegacyCommandResultError(apiException); } if (useErrorObjectV2) { return getErrorObjectV2(throwable); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/OperationAttempt.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/OperationAttempt.java index 1bbd8834e..68a1a01b5 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/OperationAttempt.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/OperationAttempt.java @@ -2,6 +2,7 @@ import com.datastax.oss.driver.api.core.cql.AsyncResultSet; import io.smallrye.mutiny.Uni; +import io.stargate.sgv2.jsonapi.exception.APIException; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CommandQueryExecutor; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.DriverExceptionHandler; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.SchemaObject; @@ -68,7 +69,7 @@ public final boolean isTerminal() { protected final UUID attemptId = UUID.randomUUID(); - private final List warnings = new ArrayList<>(); + private final List warnings = new ArrayList<>(); private Throwable failure; // Keep this private, so sub-classes set through setter incase we need to syncronize later @@ -447,11 +448,8 @@ public SubT maybeAddFailure(Throwable throwable) { * * @param warning The warning message to add, cannot be null or blank. */ - public void addWarning(String warning) { - if (warning == null || warning.isBlank()) { - throw new IllegalArgumentException("warning cannot be null or blank"); - } - warnings.add(warning); + public void addWarning(APIException warning) { + warnings.add(Objects.requireNonNull(warning, "warning cannot be null")); } /** @@ -462,7 +460,7 @@ public void addWarning(String warning) { * * @return An unmodifiable list of warnings, never null */ - public List warnings() { + public List warnings() { return List.copyOf(warnings); } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttemptPage.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttemptPage.java index cba4b3ee8..c24293849 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttemptPage.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttemptPage.java @@ -1,5 +1,6 @@ package io.stargate.sgv2.jsonapi.service.operation; +import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandResultBuilder; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CqlPagingState; @@ -93,12 +94,9 @@ public ReadAttemptPage getOperationPage() { }; var resultBuilder = - new CommandResultBuilder( - singleResponse - ? CommandResultBuilder.ResponseType.SINGLE_DOCUMENT - : CommandResultBuilder.ResponseType.MULTI_DOCUMENT, - useErrorObjectV2, - debugMode); + singleResponse + ? CommandResult.singleDocumentBuilder(useErrorObjectV2, debugMode) + : CommandResult.multiDocumentBuilder(useErrorObjectV2, debugMode); return new ReadAttemptPage<>( attempts, resultBuilder, pagingState, includeSortVector, sortVector); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadOperationPage.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadOperationPage.java index 8315c8143..e910d9641 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadOperationPage.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadOperationPage.java @@ -1,11 +1,8 @@ package io.stargate.sgv2.jsonapi.service.operation; -import com.fasterxml.jackson.databind.JsonNode; import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; -import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.function.Supplier; /** @@ -33,20 +30,22 @@ public record ReadOperationPage( @Override public CommandResult get() { - Map status = - includeSortVector ? Collections.singletonMap(CommandStatus.SORT_VECTOR, vector) : null; + var builder = + singleResponse + ? CommandResult.singleDocumentBuilder(false, false) + : CommandResult.multiDocumentBuilder(false, false); - List jsonDocs = - documentSources.stream() - .limit(singleResponse ? 1 : Long.MAX_VALUE) - .map(DocumentSource::get) - .toList(); + if (includeSortVector) { + builder.addStatus(CommandStatus.SORT_VECTOR, vector); + } else { + builder.nextPageState(nextPageState); + } - var responseData = - singleResponse - ? new CommandResult.SingleResponseData(jsonDocs.isEmpty() ? null : jsonDocs.get(0)) - : new CommandResult.MultiResponseData(jsonDocs, nextPageState); + documentSources.stream() + .limit(singleResponse ? 1 : Long.MAX_VALUE) + .map(DocumentSource::get) + .forEach(builder::addDocument); - return new CommandResult(responseData, status); + return builder.build(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/SchemaAttemptPage.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/SchemaAttemptPage.java index 24890af92..66793631c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/SchemaAttemptPage.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/SchemaAttemptPage.java @@ -1,5 +1,6 @@ package io.stargate.sgv2.jsonapi.service.operation; +import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandResultBuilder; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.SchemaObject; @@ -35,12 +36,8 @@ public static class Builder @Override public SchemaAttemptPage getOperationPage() { - - var resultBuilder = - new CommandResultBuilder( - CommandResultBuilder.ResponseType.STATUS_ONLY, useErrorObjectV2, debugMode); - - return new SchemaAttemptPage<>(attempts, resultBuilder); + return new SchemaAttemptPage<>( + attempts, CommandResult.statusOnlyBuilder(useErrorObjectV2, debugMode)); } } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CountOperationPage.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CountOperationPage.java index e261960b5..1b565a3ce 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CountOperationPage.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/CountOperationPage.java @@ -2,16 +2,18 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; -import java.util.Map; import java.util.function.Supplier; public record CountOperationPage(long count, boolean moreData) implements Supplier { @Override public CommandResult get() { + + var builder = + CommandResult.statusOnlyBuilder(false, false) + .addStatus(CommandStatus.COUNTED_DOCUMENT, count); if (moreData) { - return new CommandResult( - Map.of(CommandStatus.COUNTED_DOCUMENT, count(), CommandStatus.MORE_DATA, true)); + builder.addStatus(CommandStatus.MORE_DATA, true); } - return new CommandResult(Map.of(CommandStatus.COUNTED_DOCUMENT, count())); + return builder.build(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/DeleteOperationPage.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/DeleteOperationPage.java index 0f9b32d25..0a9bcb3c9 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/DeleteOperationPage.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/DeleteOperationPage.java @@ -11,7 +11,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; -import java.util.Map; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -32,9 +31,16 @@ public record DeleteOperationPage( @Override public CommandResult get() { + // aaron - 9 octo 2024 - this class had multiple return statements, which is ok but when I + // changed to + // use the CommandResultBuilder, I left the structure as it was to reduce the amount of changes. + // when we move to use OperationAttempt for the collection commands we can refactor if (deletedInformation == null) { - return new CommandResult(null, Map.of(CommandStatus.DELETED_COUNT, -1), null); + return CommandResult.singleDocumentBuilder(false, false) + .addStatus(CommandStatus.DELETED_COUNT, -1) + .build(); } + int deletedCount = (int) deletedInformation.stream() @@ -76,15 +82,20 @@ public CommandResult get() { // return the result // note that we always target a single document to be returned // thus fixed to the SingleResponseData - if (moreData) - return new CommandResult( - deletedDoc.isEmpty() ? null : new CommandResult.SingleResponseData(deletedDoc.get(0)), - Map.of(CommandStatus.DELETED_COUNT, deletedCount, CommandStatus.MORE_DATA, true), - errors.isEmpty() ? null : errors); - else - return new CommandResult( - deletedDoc.isEmpty() ? null : new CommandResult.SingleResponseData(deletedDoc.get(0)), - Map.of(CommandStatus.DELETED_COUNT, deletedCount), - errors.isEmpty() ? null : errors); + + // aaron 9-oct-2024 the original code had this to create the "ResponseData"for the command + // result + // which looks like it would be statusOnly if there were no docs, otherwise singleDoc + // deletedDoc.isEmpty() ? null : new ResponseData.SingleResponseData(deletedDoc.get(0)), + var builder = + deletedDoc.isEmpty() + ? CommandResult.statusOnlyBuilder(false, false) + : CommandResult.singleDocumentBuilder(false, false).addDocument(deletedDoc.getFirst()); + + builder.addStatus(CommandStatus.DELETED_COUNT, deletedCount).addCommandResultError(errors); + if (moreData) { + builder.addStatus(CommandStatus.MORE_DATA, true); + } + return builder.build(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/EstimatedCountResult.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/EstimatedCountResult.java index ab97b04f1..13456eb32 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/EstimatedCountResult.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/EstimatedCountResult.java @@ -2,12 +2,13 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; -import java.util.Map; import java.util.function.Supplier; public record EstimatedCountResult(long count) implements Supplier { @Override public CommandResult get() { - return new CommandResult(Map.of(CommandStatus.COUNTED_DOCUMENT, count())); + return CommandResult.statusOnlyBuilder(false, false) + .addStatus(CommandStatus.COUNTED_DOCUMENT, count) + .build(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionsCollectionOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionsCollectionOperation.java index bf497d8f9..c6feca762 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionsCollectionOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/FindCollectionsCollectionOperation.java @@ -17,7 +17,6 @@ import io.stargate.sgv2.jsonapi.service.schema.collections.CollectionSchemaObject; import io.stargate.sgv2.jsonapi.service.schema.collections.CollectionTableMatcher; import java.util.List; -import java.util.Map; import java.util.function.Supplier; /** @@ -96,20 +95,20 @@ private record Result(boolean explain, List collections) @Override public CommandResult get() { + + var builder = CommandResult.statusOnlyBuilder(false, false); if (explain) { final List createCollectionCommands = collections.stream() .map(CollectionSchemaObject::collectionSettingToCreateCollectionCommand) .toList(); - Map statuses = - Map.of(CommandStatus.EXISTING_COLLECTIONS, createCollectionCommands); - return new CommandResult(statuses); + builder.addStatus(CommandStatus.EXISTING_COLLECTIONS, createCollectionCommands); } else { List tables = collections.stream().map(schemaObject -> schemaObject.name().table()).toList(); - Map statuses = Map.of(CommandStatus.EXISTING_COLLECTIONS, tables); - return new CommandResult(statuses); + builder.addStatus(CommandStatus.EXISTING_COLLECTIONS, tables); } + return builder.build(); } } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/SchemaChangeResult.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/SchemaChangeResult.java index 740a280c9..b71f2a079 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/SchemaChangeResult.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/SchemaChangeResult.java @@ -2,12 +2,13 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; -import java.util.Map; import java.util.function.Supplier; public record SchemaChangeResult(boolean schemaChanged) implements Supplier { @Override public CommandResult get() { - return new CommandResult(Map.of(CommandStatus.OK, schemaChanged ? 1 : 0)); + return CommandResult.statusOnlyBuilder(false, false) + .addStatus(CommandStatus.OK, schemaChanged ? 1 : 0) + .build(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/ServiceRegistrationResult.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/ServiceRegistrationResult.java index df5b1c798..70e0517e7 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/ServiceRegistrationResult.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/ServiceRegistrationResult.java @@ -2,12 +2,11 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; -import java.util.Map; import java.util.function.Supplier; public record ServiceRegistrationResult() implements Supplier { @Override public CommandResult get() { - return new CommandResult(Map.of(CommandStatus.OK, 1)); + return CommandResult.singleDocumentBuilder(false, false).addStatus(CommandStatus.OK, 1).build(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/UpdateCollectionOperationPage.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/UpdateCollectionOperationPage.java index 4041ac1d2..63cce04c5 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/UpdateCollectionOperationPage.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/collections/UpdateCollectionOperationPage.java @@ -9,7 +9,6 @@ import io.stargate.sgv2.jsonapi.util.ExceptionUtil; import java.util.ArrayList; import java.util.Collection; -import java.util.EnumMap; import java.util.List; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -29,13 +28,22 @@ public CommandResult get() { final DocumentId[] upsertedId = new DocumentId[1]; List updatedDocs = new ArrayList<>(updatedDocuments().size()); + var builder = + returnDocs + ? CommandResult.singleDocumentBuilder(false, false) + : CommandResult.multiDocumentBuilder(false, false); + // aggregate the errors by error code or error class Multimap groupedErrorUpdates = ArrayListMultimap.create(); updatedDocuments.forEach( update -> { - if (update.upserted()) upsertedId[0] = update.id(); - if (returnDocs) updatedDocs.add(update.document()); + if (update.upserted()) { + upsertedId[0] = update.id(); + } + if (returnDocs) { + updatedDocs.add(update.document()); + } // if (update.error() != null) { String key = ExceptionUtil.getThrowableGroupingKey(update.error()); @@ -56,28 +64,29 @@ public CommandResult get() { ExceptionUtil.getError( ERROR, documentIds, updatedDocuments.stream().findFirst().get().error())); }); - EnumMap updateStatus = new EnumMap<>(CommandStatus.class); + if (upsertedId[0] != null) { - updateStatus.put(CommandStatus.UPSERTED_ID, upsertedId[0]); + builder.addStatus(CommandStatus.UPSERTED_ID, upsertedId[0]); } - updateStatus.put(CommandStatus.MATCHED_COUNT, matchedCount()); - updateStatus.put(CommandStatus.MODIFIED_COUNT, modifiedCount()); + builder.addStatus(CommandStatus.MATCHED_COUNT, matchedCount()); + builder.addStatus(CommandStatus.MODIFIED_COUNT, modifiedCount()); if (pagingState != null) { - updateStatus.put(CommandStatus.MORE_DATA, true); - updateStatus.put(CommandStatus.PAGE_STATE, pagingState); + builder.addStatus(CommandStatus.MORE_DATA, true); + builder.addStatus(CommandStatus.PAGE_STATE, pagingState); } + // aaron - 9-oct-2024 - these two line comments were below.... // note that we always target a single document to be returned // thus fixed to the SingleResponseData - if (returnDocs) { - JsonNode node = updatedDocs.size() > 0 ? updatedDocs.get(0) : null; - return new CommandResult( - new CommandResult.SingleResponseData(node), - updateStatus, - errors.isEmpty() ? null : errors); - } else { - return new CommandResult(null, updateStatus, errors.isEmpty() ? null : errors); + // ... but I think they are wrong, because it would previously pass null for the data if + // returnedDocs was false + // so at the top of the function we make the appropriate builder + // (comment could have been about how it handles have zero docs and returnDocs is true) + if (returnDocs && !updatedDocs.isEmpty()) { + builder.addDocument(updatedDocs.getFirst()); } + builder.addCommandResultError(errors); + return builder.build(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/embeddings/FindEmbeddingProvidersOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/embeddings/FindEmbeddingProvidersOperation.java index 7abbf225f..2bdb65eea 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/embeddings/FindEmbeddingProvidersOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/embeddings/FindEmbeddingProvidersOperation.java @@ -40,9 +40,10 @@ private record Result(Map embeddingProviders) @Override public CommandResult get() { - Map statuses = - Map.of(CommandStatus.EXISTING_VECTOR_PROVIDERS, embeddingProviders); - return new CommandResult(statuses); + + return CommandResult.statusOnlyBuilder(false, false) + .addStatus(CommandStatus.EXISTING_VECTOR_PROVIDERS, embeddingProviders) + .build(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/keyspaces/FindKeyspacesOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/keyspaces/FindKeyspacesOperation.java index 17acbc055..3cf349e57 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/keyspaces/FindKeyspacesOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/keyspaces/FindKeyspacesOperation.java @@ -8,7 +8,6 @@ import io.stargate.sgv2.jsonapi.service.cqldriver.executor.QueryExecutor; import io.stargate.sgv2.jsonapi.service.operation.Operation; import java.util.List; -import java.util.Map; import java.util.function.Supplier; /** @@ -65,11 +64,11 @@ private record Result(List keyspaces, boolean useKeyspaceNaming) @Override public CommandResult get() { - Map statuses = - useKeyspaceNaming - ? Map.of(CommandStatus.EXISTING_KEYSPACES, keyspaces) - : Map.of(CommandStatus.EXISTING_NAMESPACES, keyspaces); - return new CommandResult(statuses); + + var statusKey = + useKeyspaceNaming ? CommandStatus.EXISTING_KEYSPACES : CommandStatus.EXISTING_NAMESPACES; + + return CommandResult.statusOnlyBuilder(false, false).addStatus(statusKey, keyspaces).build(); } } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/DeleteTableOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/DeleteTableOperation.java index 4784a315f..a7ca94a32 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/DeleteTableOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/DeleteTableOperation.java @@ -14,7 +14,6 @@ import io.stargate.sgv2.jsonapi.service.operation.query.WhereCQLClause; import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.function.Supplier; import org.slf4j.Logger; @@ -67,12 +66,13 @@ public Uni> execute( resultSet -> Uni.createFrom() .item( - new Supplier>() { + new Supplier<>() { @Override public Supplier get() { return () -> - new CommandResult( - null, Map.of(CommandStatus.DELETED_COUNT, -1), List.of()); + CommandResult.singleDocumentBuilder(false, false) + .addStatus(CommandStatus.DELETED_COUNT, -1) + .build(); } })); } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableReadAttemptBuilder.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableReadAttemptBuilder.java index 6b9d065c0..48e323eeb 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableReadAttemptBuilder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableReadAttemptBuilder.java @@ -111,9 +111,7 @@ public ReadAttempt build(WhereCQLClause