From 756f6150ef1f6ed844bd6f788d60142806e92eef Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Wed, 9 Oct 2024 15:56:02 +1300 Subject: [PATCH 01/19] 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 0000000000..5af72cadec --- /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 0000000000..41e8b4dcfe --- /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 c1ead5541b..b910644afc 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 f06fc3555b..f37ba7cf23 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 01db3ceec5..c5121f3d0d 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 0000000000..9c4ad07091 --- /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 c76abd7dc8..1a165b66c5 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 91d2a20054..3fdf5026cc 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 86d3b06b3e..9f273e6edc 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 7daeb2b687..e19419bff9 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 2ee65ca570..29ca731beb 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 c7d569b83e..090319f126 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 e68d9009a0..8c4094853f 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 47d25f0ec2..8524dd6ebf 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 1bbd8834e4..68a1a01b5a 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 cba4b3ee88..c24293849c 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 8315c8143f..e910d96410 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 24890af923..66793631c9 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 e261960b55..1b565a3ce2 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 0f9b32d25e..0a9bcb3c9f 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 ab97b04f1f..13456eb323 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 bf497d8f98..c6feca7620 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 740a280c9a..b71f2a0798 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 df5b1c798f..70e0517e76 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 4041ac1d26..63cce04c54 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 7abbf225f9..2bdb65eeac 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 17acbc055a..3cf349e57b 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 4784a315f9..a7ca94a326 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 6b9d065c05..48e323eebc 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 whereCQLClause; + private final OrderByCqlClause orderByCqlClause; private final CqlOptions whereCQLClause, + OrderByCqlClause orderByCqlClause, CqlOptions whereCQLClaus tableSchemaObject, selectCQLClause, whereCQLClause, + orderByCqlClause, atttemptCqlOptions, pagingState, documentSourceSupplier); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindCommandResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindCommandResolver.java index 1c75f57496..7ddbb6d0a4 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindCommandResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindCommandResolver.java @@ -15,6 +15,7 @@ import io.stargate.sgv2.jsonapi.service.operation.*; import io.stargate.sgv2.jsonapi.service.operation.collections.CollectionReadType; import io.stargate.sgv2.jsonapi.service.operation.collections.FindCollectionOperation; +import io.stargate.sgv2.jsonapi.service.operation.filters.table.codecs.JSONCodecRegistries; import io.stargate.sgv2.jsonapi.service.operation.query.CQLOption; import io.stargate.sgv2.jsonapi.service.operation.query.DBLogicalExpression; import io.stargate.sgv2.jsonapi.service.operation.tables.*; @@ -22,6 +23,8 @@ import io.stargate.sgv2.jsonapi.service.resolver.matcher.CollectionFilterResolver; import io.stargate.sgv2.jsonapi.service.resolver.matcher.FilterResolver; import io.stargate.sgv2.jsonapi.service.resolver.matcher.TableFilterResolver; +import io.stargate.sgv2.jsonapi.service.resolver.sort.SortClauseResolver; +import io.stargate.sgv2.jsonapi.service.resolver.sort.TableSortClauseResolver; import io.stargate.sgv2.jsonapi.service.schema.collections.CollectionSchemaObject; import io.stargate.sgv2.jsonapi.util.SortClauseUtil; import jakarta.enterprise.context.ApplicationScoped; @@ -41,6 +44,7 @@ public class FindCommandResolver implements CommandResolver { private final FilterResolver collectionFilterResolver; private final FilterResolver tableFilterResolver; + private final SortClauseResolver tableSortClauseResolver; @Inject public FindCommandResolver( @@ -58,6 +62,8 @@ public FindCommandResolver( this.collectionFilterResolver = new CollectionFilterResolver<>(operationsConfig); this.tableFilterResolver = new TableFilterResolver<>(operationsConfig); + this.tableSortClauseResolver = + new TableSortClauseResolver<>(operationsConfig, JSONCodecRegistries.DEFAULT_REGISTRY); } @Override @@ -72,6 +78,7 @@ public Operation resolveTableCommand(CommandContext ctx, Find Optional.ofNullable(command.options()) .map(FindCommand.Options::limit) .orElse(Integer.MAX_VALUE); + var cqlPageState = Optional.ofNullable(command.options()) .map(options -> CqlPagingState.from(options.pageState())) @@ -81,8 +88,10 @@ public Operation resolveTableCommand(CommandContext ctx, Find TableRowProjection.fromDefinition( objectMapper, command.tableProjectionDefinition(), ctx.schemaObject()); + var orderBy = tableSortClauseResolver.resolve(ctx, command); + var builder = - new TableReadAttemptBuilder(ctx.schemaObject(), projection, projection) + new TableReadAttemptBuilder(ctx.schemaObject(), projection, projection, orderBy) .addBuilderOption(CQLOption.ForSelect.limit(limit)) .addStatementOption(CQLOption.ForStatement.pageSize(operationsConfig.defaultPageSize())) .addPagingState(cqlPageState); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindOneCommandResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindOneCommandResolver.java index 24f47f583f..e7b4f21ba8 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindOneCommandResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindOneCommandResolver.java @@ -13,6 +13,7 @@ import io.stargate.sgv2.jsonapi.service.operation.*; import io.stargate.sgv2.jsonapi.service.operation.collections.CollectionReadType; import io.stargate.sgv2.jsonapi.service.operation.collections.FindCollectionOperation; +import io.stargate.sgv2.jsonapi.service.operation.filters.table.codecs.JSONCodecRegistries; import io.stargate.sgv2.jsonapi.service.operation.query.CQLOption; import io.stargate.sgv2.jsonapi.service.operation.query.DBLogicalExpression; import io.stargate.sgv2.jsonapi.service.operation.tables.*; @@ -20,6 +21,8 @@ import io.stargate.sgv2.jsonapi.service.resolver.matcher.CollectionFilterResolver; import io.stargate.sgv2.jsonapi.service.resolver.matcher.FilterResolver; import io.stargate.sgv2.jsonapi.service.resolver.matcher.TableFilterResolver; +import io.stargate.sgv2.jsonapi.service.resolver.sort.SortClauseResolver; +import io.stargate.sgv2.jsonapi.service.resolver.sort.TableSortClauseResolver; import io.stargate.sgv2.jsonapi.service.schema.collections.CollectionSchemaObject; import io.stargate.sgv2.jsonapi.util.SortClauseUtil; import jakarta.enterprise.context.ApplicationScoped; @@ -37,6 +40,7 @@ public class FindOneCommandResolver implements CommandResolver { private final FilterResolver collectionFilterResolver; private final FilterResolver tableFilterResolver; + private final SortClauseResolver tableSortClauseResolver; @Inject public FindOneCommandResolver( @@ -55,6 +59,8 @@ public FindOneCommandResolver( this.collectionFilterResolver = new CollectionFilterResolver<>(operationsConfig); this.tableFilterResolver = new TableFilterResolver<>(operationsConfig); + this.tableSortClauseResolver = + new TableSortClauseResolver<>(operationsConfig, JSONCodecRegistries.DEFAULT_REGISTRY); } @Override @@ -70,8 +76,10 @@ public Operation resolveTableCommand( TableRowProjection.fromDefinition( objectMapper, command.tableProjectionDefinition(), ctx.schemaObject()); + var orderBy = tableSortClauseResolver.resolve(ctx, command); + var builder = - new TableReadAttemptBuilder(ctx.schemaObject(), projection, projection) + new TableReadAttemptBuilder(ctx.schemaObject(), projection, projection, orderBy) .addBuilderOption(CQLOption.ForSelect.limit(1)); // TODO, we may want the ability to resolve API filter clause into multiple diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/SortClauseResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/SortClauseResolver.java index b65d3fda05..633e5148eb 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/SortClauseResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/SortClauseResolver.java @@ -2,17 +2,27 @@ import io.stargate.sgv2.jsonapi.api.model.command.Command; import io.stargate.sgv2.jsonapi.api.model.command.Sortable; -import io.stargate.sgv2.jsonapi.api.model.command.Updatable; import io.stargate.sgv2.jsonapi.config.OperationsConfig; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.SchemaObject; import io.stargate.sgv2.jsonapi.service.operation.query.OrderByCqlClause; -import io.stargate.sgv2.jsonapi.service.operation.query.UpdateValuesCQLClause; import io.stargate.sgv2.jsonapi.service.resolver.ClauseResolver; -public abstract class SortClauseResolver +/** + * Base for classes that resolve a sort clause on a {@link Command} into something the Operation can + * use to add ORDER BY to a query. + * + *

NOTE: this is for adding the CQL order by, while still do some in memory sorting for CQL that + * is handled differently. + * + * @param The type of the {@link Command} that is being resolved. + * @param The type of the {@link SchemaObject} that {@link Command} command is operating + * on. + */ +public abstract class SortClauseResolver< + CmdT extends Command & Sortable, SchemaT extends SchemaObject> extends ClauseResolver { - protected SortClauseResolver(OperationsConfig operationsConfig){ + protected SortClauseResolver(OperationsConfig operationsConfig) { super(operationsConfig); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/TableSortClauseResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/TableSortClauseResolver.java index f8f8cd1328..9c35dd74ae 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/TableSortClauseResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/TableSortClauseResolver.java @@ -1,68 +1,100 @@ package io.stargate.sgv2.jsonapi.service.resolver.sort; -import com.datastax.oss.driver.api.core.metadata.schema.TableMetadata; +import static io.stargate.sgv2.jsonapi.exception.ErrorFormatters.*; +import static io.stargate.sgv2.jsonapi.util.CqlIdentifierUtil.cqlIdentifierFromUserInput; + import io.stargate.sgv2.jsonapi.api.model.command.Command; import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; import io.stargate.sgv2.jsonapi.api.model.command.Sortable; import io.stargate.sgv2.jsonapi.api.model.command.clause.sort.SortExpression; import io.stargate.sgv2.jsonapi.config.OperationsConfig; -import io.stargate.sgv2.jsonapi.exception.catchable.UnknownColumnException; +import io.stargate.sgv2.jsonapi.exception.SortException; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.TableSchemaObject; import io.stargate.sgv2.jsonapi.service.operation.filters.table.codecs.JSONCodecRegistry; import io.stargate.sgv2.jsonapi.service.operation.query.OrderByCqlClause; -import io.stargate.sgv2.jsonapi.service.shredding.CqlNamedValue; -import org.jboss.resteasy.reactive.server.core.RuntimeExceptionMapper; - +import io.stargate.sgv2.jsonapi.service.operation.tables.TableANNOrderByCQlClause; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiTypeName; +import io.stargate.sgv2.jsonapi.util.CqlVectorUtil; import java.util.Objects; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; -import static io.stargate.sgv2.jsonapi.util.CqlIdentifierUtil.cqlIdentifierFromUserInput; - -public class TableSortClauseResolver extends SortClauseResolver { +/** + * Resolves a sort clause to deterimine if we want to apply a CQL ORDER BY clause to the operation. + * + *

Not all sort clauses result in a CQL order by, some may result in in memory sorting. + */ +public class TableSortClauseResolver + extends SortClauseResolver { + private static final Logger LOGGER = LoggerFactory.getLogger(TableSortClauseResolver.class); - private final JSONCodecRegistry codecRegistry; - public TableSortClauseResolver(OperationsConfig operationsConfig, JSONCodecRegistry codecRegistry) { + public TableSortClauseResolver( + OperationsConfig operationsConfig, JSONCodecRegistry codecRegistry) { super(operationsConfig); - this.codecRegistry = Objects.requireNonNull(codecRegistry, "codecRegistry must not be null"); } @Override public OrderByCqlClause resolve(CommandContext commandContext, CmdT command) { + Objects.requireNonNull(commandContext, "commandContext is required"); + Objects.requireNonNull(command, "command is required"); + var apiTableDef = commandContext.schemaObject().apiTableDef(); var sortClause = command.sortClause(); if (sortClause == null || sortClause.isEmpty()) { return OrderByCqlClause.NO_OP; } var vectorSorts = sortClause.tableVectorSorts(); - if (vectorSorts.size() > 1){ - throw new RuntimeException("TODO: throw too many ANN sorts"); + if (vectorSorts.isEmpty()) { + // For now, only supporting vector sort + return OrderByCqlClause.NO_OP; } - if (vectorSorts.size() == 1){ - var vectorSort = vectorSorts.getFirst(); - - - return new TableANNOrderByCQlClause(commandContext.getSchemaObject(), vectorSort.column(), vectorSort.vector()); + if (vectorSorts.size() > 1) { + throw SortException.Code.MORE_THAN_ONE_VECTOR_SORT.get( + errVars( + commandContext.schemaObject(), + map -> { + map.put( + "vectorColumns", + errFmtApiColumnDef(apiTableDef.allColumns().filterBy(ApiTypeName.VECTOR))); + map.put( + "sortColumns", + errFmtJoin(vectorSorts.stream().map(SortExpression::path).toList())); + })); } - } - private OrderByCqlClause resolveTableVectorSort(TableMetadata tableMetadata, SortExpression sortExpression) { + var vectorSort = vectorSorts.getFirst(); + var sortIdentifier = cqlIdentifierFromUserInput(vectorSort.path()); + var apiColumnDef = apiTableDef.allColumns().get(sortIdentifier); + if (apiColumnDef == null) { + throw SortException.Code.CANNOT_SORT_UNKNOWN_COLUMNS.get( + errVars( + commandContext.schemaObject(), + map -> { + map.put("allColumns", errFmtApiColumnDef(apiTableDef.allColumns())); + map.put("unknownColumns", errFmt(sortIdentifier)); + })); + } - var column = cqlIdentifierFromUserInput(sortExpression.path()); - CqlNamedValue cqlNamedValue = null; - try { - var codec = codecRegistry.codecToCQL(tableMetadata, column, sortExpression.vector()); - cqlNamedValue = new CqlNamedValue(tableMetadata, codec.toCQL(rawJsonValue)); - } catch (UnknownColumnException e) { - // this should not happen, we checked above but the codecs are written to be very safe and - // will check and throw - throw ServerException.Code.UNEXPECTED_SERVER_ERROR.get(errVars(e)); - } catch (MissingJSONCodecException e) { - unsupportedErrors.put(metadata, e); - } catch (ToCQLCodecException e) { - codecErrors.put(metadata, e); + if (apiColumnDef.type().typeName() != ApiTypeName.VECTOR) { + throw SortException.Code.CANNOT_VECTOR_SORT_NON_VECTOR_COLUMNS.get( + errVars( + commandContext.schemaObject(), + map -> { + map.put( + "vectorColumns", + errFmtApiColumnDef(apiTableDef.allColumns().filterBy(ApiTypeName.VECTOR))); + map.put("sortColumns", errFmt(sortIdentifier)); + })); } - } + // This is a bit of a hack, we should be using the codecs to convert but for now the Sort + // deserialization + // turns the JSON array into a float array, so we can just use that. + // Needs more refactoring to change how it works + var cqlVector = CqlVectorUtil.floatsToCqlVector(vectorSort.vector()); + return new TableANNOrderByCQlClause(apiColumnDef, cqlVector); + } } diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml index acef21dde0..c0e655a859 100644 --- a/src/main/resources/errors.yaml +++ b/src/main/resources/errors.yaml @@ -46,7 +46,9 @@ # new line after each entry, using the `|-` to trim trailing newlines. # ================================================================================================================ -# Snippets +# ================================================================================================================ +# SNIPPETS +# ================================================================================================================ # ================================================================================================================ snippets: @@ -68,10 +70,21 @@ snippets: body: |- Resend the request using only defined columns. + - name: VECTOR_SORT_EXPLANATION + body: |- + A vector sort in the sort clause identifies the vector column by name and then provides either: + - The vector as an array of decimal numbers. + - The vector as a base64 encoded `{"$binary": "base64-encoded-vector"}` object. + - A string to be vectorized if enabled for the column. +# ================================================================================================================ +# ================================================================================================================ +# REQUEST Errors +# ================================================================================================================ +# ================================================================================================================ # ================================================================================================================ -# Request Errors +# Family: REQUEST Scope: NONE # ================================================================================================================ request-errors: @@ -84,6 +97,10 @@ request-errors: ${SNIPPET.CONTACT_SUPPORT} +# ================================================================================================================ +# Family: REQUEST Scope: DOCUMENT +# ================================================================================================================ + # DOCUMENT request errors - scope: DOCUMENT code: MISSING_PRIMARY_KEY_COLUMNS @@ -132,7 +149,10 @@ request-errors: Resend the request using only supported column values. - # FILTER request errors +# ================================================================================================================ +# Family: REQUEST Scope: FILTER +# ================================================================================================================ + - scope: FILTER code: INVALID_FILTER title: Invalid filter @@ -223,7 +243,10 @@ request-errors: - Missing Partition Keys: ${missingPartitionKeys}. - Out of order Partition Sort Keys: ${outOfOrderClusteringKeys}. - # UPDATE clause errors +# ================================================================================================================ +# Family: REQUEST Scope: UPDATE +# ================================================================================================================ + - scope: UPDATE code: UNKNOWN_TABLE_COLUMNS title: Cannot update on unknown table columns @@ -259,7 +282,9 @@ request-errors: Supported update operations are ${supportedUpdateOperations}. - # Warnings + # ================================================================================================================ + # Family: REQUEST Scope: WARNING + # ================================================================================================================ - scope: WARNING code: MISSING_INDEX @@ -354,7 +379,9 @@ request-errors: ${SNIPPET.INEFFICIENT_QUERY} - # Schema Errors + # ================================================================================================================ + # Family: REQUEST Scope: SCHEMA + # ================================================================================================================ # aaron errors - scope: SCHEMA @@ -619,12 +646,51 @@ request-errors: body: |- Column `${column}` doesn't exist in the table. +# ================================================================================================================ +# Family: REQUEST Scope: SORT +# ================================================================================================================ + - scope: SORT + code: MORE_THAN_ONE_VECTOR_SORT + title: More than one vector sort provided + body: |- + The command used a sort clause with more than one vector sort, only one vector sort is allowed. + + ${SNIPPET.VECTOR_SORT_EXPLANATION} + + The table ${keyspace}.${table} defines the vector columns: ${vectorColumns}. + The command attempted to sort on the columns: ${sortColumns}. + + Resend the command with only one vector sort. + - scope: SORT + code: CANNOT_SORT_UNKNOWN_COLUMNS + title: Sorted columns are not present in the table schema + body: |- + The command attempted to sort using columns that are not in the table schema. + + The table ${keyspace}.${table} defines the columns: ${allColumns}. + The command attempted to sort the unknown columns: ${unknownColumns}. + + Resend the command using only columns that are defined in the table schema. + - scope: SORT + code: CANNOT_VECTOR_SORT_NON_VECTOR_COLUMNS + title: Vector sort requested for non-vector columns + body: |- + The command attemtped to vector sort columns that are not of vector type. + + ${SNIPPET.VECTOR_SORT_EXPLANATION} + + The table ${keyspace}.${table} defines the columns: ${vectorColumns}. + The command attempted to sort the non vector columns: ${sortColumns}. + + Resend the command using only vector columns for vector sorting. # ================================================================================================================ -# Server Errors +# ================================================================================================================ +# Server Errors +# ================================================================================================================ # ================================================================================================================ server-errors: diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttemptTestData.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttemptTestData.java index ecf801dd74..21033a0526 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttemptTestData.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttemptTestData.java @@ -8,6 +8,7 @@ import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CqlPagingState; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.TableSchemaObject; import io.stargate.sgv2.jsonapi.service.operation.query.CqlOptions; +import io.stargate.sgv2.jsonapi.service.operation.query.OrderByCqlClause; import io.stargate.sgv2.jsonapi.service.operation.tables.TableDriverExceptionHandler; public class ReadAttemptTestData extends OperationAttemptTestData { @@ -41,6 +42,7 @@ private OperationAttemptFixture newReadAttemptFixture( testData.schemaObject().emptyTableSchemaObject(), testData.selectCQLClause().selectAllFromTable(), testData.whereCQLClause().emptySelect(), + OrderByCqlClause.NO_OP, new CqlOptions<>(), CqlPagingState.EMPTY, null, diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/TestReadAttempt.java b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/TestReadAttempt.java index ce9954828f..f58f4e71ca 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/operation/TestReadAttempt.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/operation/TestReadAttempt.java @@ -5,6 +5,7 @@ import io.stargate.sgv2.jsonapi.service.cqldriver.executor.CqlPagingState; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.TableSchemaObject; import io.stargate.sgv2.jsonapi.service.operation.query.CqlOptions; +import io.stargate.sgv2.jsonapi.service.operation.query.OrderByCqlClause; import io.stargate.sgv2.jsonapi.service.operation.query.SelectCQLClause; import io.stargate.sgv2.jsonapi.service.operation.query.WhereCQLClause; @@ -17,6 +18,7 @@ public class TestReadAttempt extends ReadAttempt { TableSchemaObject schemaObject, SelectCQLClause selectCQLClause, WhereCQLClause cqlOptions, CqlPagingState pagingState, DocumentSourceSupplier documentSourceSupplier, @@ -26,6 +28,7 @@ public class TestReadAttempt extends ReadAttempt { schemaObject, selectCQLClause, whereCQLClause, + orderByCqlClause, cqlOptions, pagingState, documentSourceSupplier); From 879d16d72c0f998c1f8f2b54e97a5b946cd5102c Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Fri, 1 Nov 2024 10:42:58 +1300 Subject: [PATCH 14/19] Added integration tests --- .../model/command/clause/sort/SortClause.java | 12 +- .../deserializers/SortClauseDeserializer.java | 20 +- .../sgv2/jsonapi/exception/SortException.java | 2 + .../service/operation/ReadAttempt.java | 2 +- .../operation/query/OrderByCqlClause.java | 10 + .../tables/TableANNOrderByCQlClause.java | 5 + .../operation/tables/TableInsertAttempt.java | 2 +- .../service/resolver/FindCommandResolver.java | 2 + .../sort/TableSortClauseResolver.java | 60 ++++- .../service/schema/tables/ApiColumnDef.java | 6 +- .../schema/tables/ApiColumnDefContainer.java | 26 ++- .../service/schema/tables/ApiVectorType.java | 4 + src/main/resources/errors.yaml | 34 ++- .../AbstractKeyspaceIntegrationTestBase.java | 2 +- .../tables/AnnSortTableIntegrationTest.java | 210 ++++++++++++++++++ .../ProjectionSchemaIntegrationTest.java | 69 +++--- .../api/v1/util/DataApiCommandSenders.java | 2 - .../api/v1/util/DataApiResponseValidator.java | 9 + .../api/v1/util/KeyspaceTemplates.java | 21 ++ .../jsonapi/api/v1/util/TableTemplates.java | 104 +++++++-- .../jsonapi/api/v1/util/TemplateRunner.java | 2 +- .../api/v1/util/TestDataScenarios.java | 116 ---------- .../AllScalarTypesTableScenario.java | 55 +++++ .../v1/util/scenarios/TestDataScenario.java | 154 +++++++++++++ .../VectorDimension5TableScenario.java | 94 ++++++++ .../jsonapi/fixtures/data/DefaultData.java | 25 ++- .../jsonapi/fixtures/data/FixtureData.java | 10 + 27 files changed, 868 insertions(+), 190 deletions(-) create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java delete mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TestDataScenarios.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/AllScalarTypesTableScenario.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/TestDataScenario.java create mode 100644 src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/VectorDimension5TableScenario.java diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/SortClause.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/SortClause.java index ce37d337b7..f19140e795 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/SortClause.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/sort/SortClause.java @@ -33,7 +33,17 @@ public boolean isEmpty() { } public List tableVectorSorts() { - return sortExpressions.stream().filter(SortExpression::isTableVectorSort).toList(); + return sortExpressions == null + ? List.of() + : sortExpressions.stream().filter(SortExpression::isTableVectorSort).toList(); + } + + public List nonTableVectorSorts() { + return sortExpressions == null + ? List.of() + : sortExpressions.stream() + .filter(sortExpression -> !sortExpression.isTableVectorSort()) + .toList(); } public boolean hasVsearchClause() { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/SortClauseDeserializer.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/SortClauseDeserializer.java index 8e26131dee..9e662a75a8 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/SortClauseDeserializer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/SortClauseDeserializer.java @@ -88,7 +88,14 @@ public SortClause deserialize(JsonParser parser, DeserializationContext ctxt) th // TODO: aaron 17-oct-2024 - this break seems unneeded as above it checks if there is only 1 // field, leaving for now break; - + } else if (inner.getValue().isArray()) { + // TODO: HACK: quick support for tables, if the value is an array we will assume the column + // is a vector then need to check on table pathway that the sort is correct. + // NOTE: does not check if there are more than one sort expression, the + // TableSortClauseResolver will take care of that so we can get proper ApiExceptions + // this is also why we do not break the look here + sortExpressions.add( + SortExpression.tableVectorSort(path, arrayNodeToVector((ArrayNode) inner.getValue()))); } else { if (path.isBlank()) { throw ErrorCodeV1.INVALID_SORT_CLAUSE_PATH.toApiException( @@ -100,16 +107,7 @@ public SortClause deserialize(JsonParser parser, DeserializationContext ctxt) th "sort clause path ('%s') contains character(s) not allowed", path); } - // TODO: HACK: quick support for tables, if the value is an array we will assume the column - // is a vector then need to check on table pathway that the sort is correct. - // the code above checks for $vector and $vectorize - if (inner.getValue().isArray()) { - sortExpressions.add( - SortExpression.tableVectorSort( - path, arrayNodeToVector((ArrayNode) inner.getValue()))); - continue; - - } else if (!inner.getValue().isInt() + if (!inner.getValue().isInt() || !(inner.getValue().intValue() == 1 || inner.getValue().intValue() == -1)) { throw ErrorCodeV1.INVALID_SORT_CLAUSE_VALUE.toApiException( "Sort ordering value can only be `1` for ascending or `-1` for descending (not `%s`)", diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java index 09450d0fc7..e63d38e634 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java @@ -16,6 +16,8 @@ public SortException(ErrorInstance errorInstance) { public enum Code implements ErrorCode { CANNOT_SORT_UNKNOWN_COLUMNS, CANNOT_VECTOR_SORT_NON_VECTOR_COLUMNS, + CANNOT_VECTOR_SORT_NON_INDEXED_VECTOR_COLUMNS, + CANNOT_MIX_VECTOR_AND_NON_VECTOR_SORT, MORE_THAN_ONE_VECTOR_SORT, ; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttempt.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttempt.java index 1ccaafc985..514aa2fb5a 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttempt.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/ReadAttempt.java @@ -194,7 +194,7 @@ public Optional schemaDescription() { "Unsupported columns in the result set: %s" .formatted(errFmtApiColumnDef(readApiColumns.filterByUnsupported()))); } - return Optional.of(readApiColumns.toColumnsDef()); + return Optional.of(readApiColumns.toColumnsDesc()); } // This is a simple container for the result set so we can set one variable in the onSuccess diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/query/OrderByCqlClause.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/query/OrderByCqlClause.java index e7aa428456..c7d93484c3 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/query/OrderByCqlClause.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/query/OrderByCqlClause.java @@ -23,4 +23,14 @@ public interface OrderByCqlClause extends Function, CQLClause { // No Op implementation just returns the select, use this when no order by is needed OrderByCqlClause NO_OP = select -> select; + + /** + * If true, this means that the query will need to be sorted in memory after the query is + * executed. + * + * @return true if the query needs to be sorted in memory + */ + default boolean inMemorySortNeeded() { + return true; + } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableANNOrderByCQlClause.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableANNOrderByCQlClause.java index 47f91bb214..d21b278be7 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableANNOrderByCQlClause.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableANNOrderByCQlClause.java @@ -34,4 +34,9 @@ public TableANNOrderByCQlClause(ApiColumnDef apiColumnDef, CqlVector vect public Select apply(Select select) { return select.orderByAnnOf(apiColumnDef.name(), vector); } + + @Override + public boolean inMemorySortNeeded() { + return false; + } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableInsertAttempt.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableInsertAttempt.java index 6442c018d2..fc380a223c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableInsertAttempt.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableInsertAttempt.java @@ -51,6 +51,6 @@ public Optional schemaDescription() { "Unsupported columns primary key: %s" + apiColumns.filterByUnsupported()); } - return Optional.of(apiColumns.toColumnsDef()); + return Optional.of(apiColumns.toColumnsDesc()); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindCommandResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindCommandResolver.java index 7ddbb6d0a4..07f5b85863 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindCommandResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/FindCommandResolver.java @@ -90,6 +90,8 @@ public Operation resolveTableCommand(CommandContext ctx, Find var orderBy = tableSortClauseResolver.resolve(ctx, command); + // make the Sorter resolver and pass ing the order by clause + var builder = new TableReadAttemptBuilder(ctx.schemaObject(), projection, projection, orderBy) .addBuilderOption(CQLOption.ForSelect.limit(limit)) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/TableSortClauseResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/TableSortClauseResolver.java index 9c35dd74ae..28a04ab19b 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/TableSortClauseResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/sort/TableSortClauseResolver.java @@ -3,6 +3,8 @@ import static io.stargate.sgv2.jsonapi.exception.ErrorFormatters.*; import static io.stargate.sgv2.jsonapi.util.CqlIdentifierUtil.cqlIdentifierFromUserInput; +import com.datastax.oss.driver.api.core.CqlIdentifier; +import com.datastax.oss.driver.api.core.metadata.schema.IndexMetadata; import io.stargate.sgv2.jsonapi.api.model.command.Command; import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; import io.stargate.sgv2.jsonapi.api.model.command.Sortable; @@ -13,16 +15,19 @@ import io.stargate.sgv2.jsonapi.service.operation.filters.table.codecs.JSONCodecRegistry; import io.stargate.sgv2.jsonapi.service.operation.query.OrderByCqlClause; import io.stargate.sgv2.jsonapi.service.operation.tables.TableANNOrderByCQlClause; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiColumnDef; import io.stargate.sgv2.jsonapi.service.schema.tables.ApiTypeName; import io.stargate.sgv2.jsonapi.util.CqlVectorUtil; +import java.util.List; import java.util.Objects; +import java.util.Optional; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** * Resolves a sort clause to deterimine if we want to apply a CQL ORDER BY clause to the operation. * - *

Not all sort clauses result in a CQL order by, some may result in in memory sorting. + *

Not all sort clauses result in a CQL order by, some may result in memory sorting. */ public class TableSortClauseResolver extends SortClauseResolver { @@ -38,6 +43,7 @@ public OrderByCqlClause resolve(CommandContext commandContext Objects.requireNonNull(commandContext, "commandContext is required"); Objects.requireNonNull(command, "command is required"); + // NOTE: Currently only supporting vector sort, next PR will deal with clustering key sorting var apiTableDef = commandContext.schemaObject().apiTableDef(); var sortClause = command.sortClause(); if (sortClause == null || sortClause.isEmpty()) { @@ -64,6 +70,25 @@ public OrderByCqlClause resolve(CommandContext commandContext })); } + // we have one vector sort - cannot have any other sorting + var nonVectorSorts = sortClause.nonTableVectorSorts(); + if (!nonVectorSorts.isEmpty()) { + throw SortException.Code.CANNOT_MIX_VECTOR_AND_NON_VECTOR_SORT.get( + errVars( + commandContext.schemaObject(), + map -> { + map.put( + "vectorColumns", + errFmtApiColumnDef(apiTableDef.allColumns().filterBy(ApiTypeName.VECTOR))); + map.put( + "sortVectorColumns", + errFmtJoin(vectorSorts.stream().map(SortExpression::path).toList())); + map.put( + "sortNonVectorColumns", + errFmtJoin(nonVectorSorts.stream().map(SortExpression::path).toList())); + })); + } + var vectorSort = vectorSorts.getFirst(); var sortIdentifier = cqlIdentifierFromUserInput(vectorSort.path()); var apiColumnDef = apiTableDef.allColumns().get(sortIdentifier); @@ -89,6 +114,23 @@ public OrderByCqlClause resolve(CommandContext commandContext })); } + // HACK - waiting for index support on the APiTableDef + var optionalIndexMetadata = findIndexMetadata(commandContext.schemaObject(), apiColumnDef); + if (optionalIndexMetadata.isEmpty()) { + throw SortException.Code.CANNOT_VECTOR_SORT_NON_INDEXED_VECTOR_COLUMNS.get( + errVars( + commandContext.schemaObject(), + map -> { + map.put( + "vectorColumns", + errFmtApiColumnDef(apiTableDef.allColumns().filterBy(ApiTypeName.VECTOR))); + map.put( + "indexedColumns", + errFmtJoin(indexedVectorColumns(commandContext.schemaObject()))); + map.put("sortColumns", errFmt(sortIdentifier)); + })); + } + // This is a bit of a hack, we should be using the codecs to convert but for now the Sort // deserialization // turns the JSON array into a float array, so we can just use that. @@ -97,4 +139,20 @@ public OrderByCqlClause resolve(CommandContext commandContext return new TableANNOrderByCQlClause(apiColumnDef, cqlVector); } + + private Optional findIndexMetadata( + TableSchemaObject schemaObject, ApiColumnDef targetColumn) { + return schemaObject.tableMetadata().getIndexes().values().stream() + .filter(index -> index.getTarget().equals(targetColumn.name().asInternal())) + .findFirst(); + } + + private List indexedVectorColumns(TableSchemaObject schemaObject) { + + var apiVectorColumns = schemaObject.apiTableDef().allColumns().filterBy(ApiTypeName.VECTOR); + return schemaObject.tableMetadata().getIndexes().values().stream() + .map(IndexMetadata::getTarget) + .filter(target -> apiVectorColumns.containsKey(CqlIdentifier.fromInternal(target))) + .toList(); + } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiColumnDef.java b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiColumnDef.java index 5c2c86dfc3..4ef746301c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiColumnDef.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiColumnDef.java @@ -43,9 +43,9 @@ public class ApiColumnDef { private final CqlIdentifier name; private final ApiDataType type; - private ApiColumnDef(CqlIdentifier name, ApiDataType type) { - this.name = name; - this.type = type; + public ApiColumnDef(CqlIdentifier name, ApiDataType type) { + this.name = Objects.requireNonNull(name, "name is must not be null"); + this.type = Objects.requireNonNull(type, "type is must not be null"); } /** diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiColumnDefContainer.java b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiColumnDefContainer.java index d474b9ab22..7576dcd162 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiColumnDefContainer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiColumnDefContainer.java @@ -20,6 +20,9 @@ public class ApiColumnDefContainer extends LinkedHashMap { private static final Logger LOGGER = LoggerFactory.getLogger(ApiColumnDefContainer.class); + private static final ApiColumnDefContainer IMMUTABLE_EMPTY = + new ApiColumnDefContainer(0).toUnmodifiable(); + public static final CqlColumnFactory FROM_CQL_FACTORY = new CqlColumnFactory(); public static final ColumnDescFactory FROM_COLUMN_DESC_FACTORY = new ColumnDescFactory(); @@ -44,6 +47,27 @@ public ApiColumnDefContainer(List columnDefs) { columnDefs.forEach(this::put); } + public static ApiColumnDefContainer of() { + return IMMUTABLE_EMPTY; + } + + public static ApiColumnDefContainer of(ApiColumnDef col1) { + return new ApiColumnDefContainer(List.of(col1)).toUnmodifiable(); + } + + public static ApiColumnDefContainer of(ApiColumnDef col1, ApiColumnDef col2) { + return new ApiColumnDefContainer(List.of(col1, col2)).toUnmodifiable(); + } + + public static ApiColumnDefContainer of(ApiColumnDef col1, ApiColumnDef col2, ApiColumnDef col3) { + return new ApiColumnDefContainer(List.of(col1, col2, col3)).toUnmodifiable(); + } + + public static ApiColumnDefContainer of( + ApiColumnDef col1, ApiColumnDef col2, ApiColumnDef col3, ApiColumnDef col4) { + return new ApiColumnDefContainer(List.of(col1, col2, col3, col4)).toUnmodifiable(); + } + public ApiColumnDefContainer toUnmodifiable() { return this instanceof UnmodifiableApiColumnDefContainer ? this @@ -91,7 +115,7 @@ public Map getVectorizeDefs() { columnDef -> ((ApiVectorType) columnDef.type()).getVectorizeDefinition())); } - public ColumnsDescContainer toColumnsDef() { + public ColumnsDescContainer toColumnsDesc() { ColumnsDescContainer columnsDesc = new ColumnsDescContainer(size()); forEach((name, columnDef) -> columnsDesc.put(name, columnDef.type().columnDesc())); return columnsDesc; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiVectorType.java b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiVectorType.java index 6daf6b76b5..6be665426d 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiVectorType.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/schema/tables/ApiVectorType.java @@ -59,6 +59,10 @@ public VectorizeDefinition getVectorizeDefinition() { return vectorizeDefinition; } + public static ApiVectorType from(int dimension) { + return from(dimension, null); + } + public static ApiVectorType from(int dimension, VectorizeDefinition vectorizeDefinition) { // Sanity check diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml index c0e655a859..3ccec4defc 100644 --- a/src/main/resources/errors.yaml +++ b/src/main/resources/errors.yaml @@ -678,7 +678,7 @@ request-errors: code: CANNOT_VECTOR_SORT_NON_VECTOR_COLUMNS title: Vector sort requested for non-vector columns body: |- - The command attemtped to vector sort columns that are not of vector type. + The command attempted to vector sort columns that are not of vector type. ${SNIPPET.VECTOR_SORT_EXPLANATION} @@ -687,6 +687,38 @@ request-errors: Resend the command using only vector columns for vector sorting. + - scope: SORT + code: CANNOT_VECTOR_SORT_NON_INDEXED_VECTOR_COLUMNS + title: Vector sort requested for vector columns that are not indexed + body: |- + The command attempted to vector sort vector columns that are not indexed. + + Vector sorting is only supported on vector columns that have been indexed. + + ${SNIPPET.VECTOR_SORT_EXPLANATION} + + The table ${keyspace}.${table} defines the vector columns: ${vectorColumns}. + And has indexes on the vector columns: ${indexedColumns}. + The command attempted to sort vector columns: ${sortColumns}. + + Resend the command using only vector columns that have been indexed. + + - scope: SORT + code: CANNOT_MIX_VECTOR_AND_NON_VECTOR_SORT + title: Vector sort requested for vector column and non vector columns + body: |- + The command attempted to vector sort vector columns and other non vector columns. + + Vector sorts must use a single vector column, and cannot include any other sort columns. + + ${SNIPPET.VECTOR_SORT_EXPLANATION} + + The table ${keyspace}.${table} defines the vector columns: ${vectorColumns}. + The command attempted to sort the vector columns: ${sortVectorColumns}. + The command attempted to sort the non-vector columns: ${sortNonVectorColumns}. + + Resend the command using only vector columns that have been indexed. + # ================================================================================================================ # ================================================================================================================ # Server Errors diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/AbstractKeyspaceIntegrationTestBase.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/AbstractKeyspaceIntegrationTestBase.java index ff654e6176..e165d0693c 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/AbstractKeyspaceIntegrationTestBase.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/AbstractKeyspaceIntegrationTestBase.java @@ -35,7 +35,7 @@ public abstract class AbstractKeyspaceIntegrationTestBase { // keyspace automatically created in this test - protected final String keyspaceName = "ks" + RandomStringUtils.randomAlphanumeric(16); + protected static final String keyspaceName = "ks" + RandomStringUtils.randomAlphanumeric(16); @BeforeAll public static void enableLog() { diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java new file mode 100644 index 0000000000..1f97b78284 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java @@ -0,0 +1,210 @@ +package io.stargate.sgv2.jsonapi.api.v1.tables; + +import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.assertTableCommand; + +import io.quarkus.test.common.WithTestResource; +import io.quarkus.test.junit.QuarkusIntegrationTest; +import io.stargate.sgv2.jsonapi.api.model.command.Command; +import io.stargate.sgv2.jsonapi.api.v1.util.scenarios.VectorDimension5TableScenario; +import io.stargate.sgv2.jsonapi.exception.SortException; +import io.stargate.sgv2.jsonapi.testresource.DseTestResource; +import java.util.ArrayList; +import java.util.Map; +import java.util.stream.Stream; +import org.junit.jupiter.api.*; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.testcontainers.shaded.com.google.common.collect.ImmutableMap; + +@QuarkusIntegrationTest +@WithTestResource(value = DseTestResource.class, restrictToAnnotatedClass = false) +@TestClassOrder(ClassOrderer.OrderAnnotation.class) +public class AnnSortTableIntegrationTest extends AbstractTableIntegrationTestBase { + + private static final String TABLE_NAME = "annSortTableTest"; + private static final VectorDimension5TableScenario SCENARIO = + new VectorDimension5TableScenario(keyspaceName, TABLE_NAME); + + @BeforeAll + public final void createScenario() { + SCENARIO.create(); + } + + @AfterAll + public final void dropScenario() { + SCENARIO.drop(); + } + + private static Stream findCommandNames() { + + var commands = new ArrayList(); + commands.add(Arguments.of(Command.CommandName.FIND)); + commands.add(Arguments.of(Command.CommandName.FIND_ONE)); + return commands.stream(); + } + + @ParameterizedTest + @MethodSource("findCommandNames") + public void findUnindexedVector(Command.CommandName commandName) { + + var sort = + ImmutableMap.of( + SCENARIO.fieldName(VectorDimension5TableScenario.UNINDEXED_VECTOR_COL), + SCENARIO.columnValue(VectorDimension5TableScenario.UNINDEXED_VECTOR_COL)); + + assertTableCommand(keyspaceName, TABLE_NAME) + .templated() + .find(commandName, null, null, sort) + .hasSingleApiError( + SortException.Code.CANNOT_VECTOR_SORT_NON_INDEXED_VECTOR_COLUMNS, + SortException.class, + "And has indexes on the vector columns: %s.\nThe command attempted to sort vector columns: %s" + .formatted(SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + SCENARIO.fieldName(VectorDimension5TableScenario.UNINDEXED_VECTOR_COL))); + } + + @ParameterizedTest + @MethodSource("findCommandNames") + public void findMoreThanOneVector(Command.CommandName commandName) { + + var sort = + ImmutableMap.of( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + SCENARIO.columnValue(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + SCENARIO.fieldName(VectorDimension5TableScenario.UNINDEXED_VECTOR_COL), + SCENARIO.columnValue(VectorDimension5TableScenario.UNINDEXED_VECTOR_COL)); + + assertTableCommand(keyspaceName, TABLE_NAME) + .templated() + .find(commandName, null, null, sort) + .hasSingleApiError( + SortException.Code.MORE_THAN_ONE_VECTOR_SORT, + SortException.class, + "The command attempted to sort on the columns: %s, %s." + .formatted( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + SCENARIO.fieldName(VectorDimension5TableScenario.UNINDEXED_VECTOR_COL))); + } + + @ParameterizedTest + @MethodSource("findCommandNames") + public void findUnknownVectorColumn(Command.CommandName commandName) { + + var sort = + ImmutableMap.of( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL) + "unknown", + SCENARIO.columnValue(VectorDimension5TableScenario.INDEXED_VECTOR_COL)); + + assertTableCommand(keyspaceName, TABLE_NAME) + .templated() + .find(commandName, null, null, sort) + .hasSingleApiError( + SortException.Code.CANNOT_SORT_UNKNOWN_COLUMNS, + SortException.class, + "The command attempted to sort the unknown columns: %s." + .formatted( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL) + + "unknown")); + } + + @ParameterizedTest + @MethodSource("findCommandNames") + public void findNonVectorCol(Command.CommandName commandName) { + + var sort = + ImmutableMap.of( + SCENARIO.fieldName(VectorDimension5TableScenario.CONTENT_COL), + SCENARIO.columnValue(VectorDimension5TableScenario.INDEXED_VECTOR_COL)); + + assertTableCommand(keyspaceName, TABLE_NAME) + .templated() + .find(commandName, null, null, sort) + .hasSingleApiError( + SortException.Code.CANNOT_VECTOR_SORT_NON_VECTOR_COLUMNS, + SortException.class, + "The command attempted to sort the non vector columns: %s." + .formatted(SCENARIO.fieldName(VectorDimension5TableScenario.CONTENT_COL))); + } + + @ParameterizedTest + @MethodSource("findCommandNames") + public void findSortVectorAndNon(Command.CommandName commandName) { + + var sort = + ImmutableMap.of( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + SCENARIO.columnValue(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + SCENARIO.fieldName(VectorDimension5TableScenario.CONTENT_COL), + 1); // asc sort + + assertTableCommand(keyspaceName, TABLE_NAME) + .templated() + .find(commandName, null, null, sort) + .hasSingleApiError( + SortException.Code.CANNOT_MIX_VECTOR_AND_NON_VECTOR_SORT, + SortException.class, + "The command attempted to sort the vector columns: %s.\nThe command attempted to sort the non-vector columns: %s." + .formatted( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + SCENARIO.fieldName(VectorDimension5TableScenario.CONTENT_COL))); + } + + @ParameterizedTest + @MethodSource("findCommandNames") + public void findRandomVector(Command.CommandName commandName) { + // Doing a sort for a vector we do not know if is in the table + + var sort = + ImmutableMap.of( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + SCENARIO.columnValue(VectorDimension5TableScenario.INDEXED_VECTOR_COL)); + + var limit = 3; + Map options = + commandName == Command.CommandName.FIND ? ImmutableMap.of("limit", limit) : null; + + var validator = + assertTableCommand(keyspaceName, TABLE_NAME) + .templated() + .find(commandName, null, null, sort, options) + .wasSuccessful(); + + if (commandName == Command.CommandName.FIND) { + validator.hasDocuments(limit); + } else { + validator.hasSingleDocument(); + } + } + + @ParameterizedTest + @MethodSource("findCommandNames") + public void findKnownVector(Command.CommandName commandName) { + // Doing a sort for a vector we know the vector is in the table, we can match on the expected + // doc + + var sort = + ImmutableMap.of( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + (Object) VectorDimension5TableScenario.KNOWN_VECTOR); + + var limit = 1; + Map options = + commandName == Command.CommandName.FIND ? ImmutableMap.of("limit", limit) : null; + + var validator = + assertTableCommand(keyspaceName, TABLE_NAME) + .templated() + .find(commandName, null, null, sort, options) + .wasSuccessful(); + + if (commandName == Command.CommandName.FIND) { + validator + .hasDocuments(limit) + .hasDocumentInPosition(0, VectorDimension5TableScenario.KNOWN_VECTOR_ROW_JSON); + + } else { + validator.hasSingleDocument(VectorDimension5TableScenario.KNOWN_VECTOR_ROW_JSON); + } + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/ProjectionSchemaIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/ProjectionSchemaIntegrationTest.java index 5cebd114f9..030a735961 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/ProjectionSchemaIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/ProjectionSchemaIntegrationTest.java @@ -2,20 +2,19 @@ import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.*; +import com.datastax.oss.driver.api.core.CqlIdentifier; import io.quarkus.test.common.WithTestResource; import io.quarkus.test.junit.QuarkusIntegrationTest; import io.stargate.sgv2.jsonapi.api.model.command.Command; import io.stargate.sgv2.jsonapi.api.v1.util.DataApiResponseValidator; -import io.stargate.sgv2.jsonapi.api.v1.util.TestDataScenarios; +import io.stargate.sgv2.jsonapi.api.v1.util.scenarios.AllScalarTypesTableScenario; import io.stargate.sgv2.jsonapi.fixtures.types.ApiDataTypesForTesting; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiColumnDefContainer; import io.stargate.sgv2.jsonapi.service.schema.tables.ApiDataTypeDefs; -import io.stargate.sgv2.jsonapi.service.schema.tables.PrimitiveApiDataTypeDef; import io.stargate.sgv2.jsonapi.testresource.DseTestResource; import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.function.Function; -import java.util.stream.Collectors; import java.util.stream.Stream; import org.junit.jupiter.api.*; import org.junit.jupiter.params.ParameterizedTest; @@ -32,38 +31,46 @@ */ public class ProjectionSchemaIntegrationTest extends AbstractTableIntegrationTestBase { - static final String TABLE_NAME = "projectionSchemaTable"; + private static final String TABLE_NAME = "projectionSchemaTable"; + private static final AllScalarTypesTableScenario TEST_DATA_SCENARIO = + new AllScalarTypesTableScenario(keyspaceName, TABLE_NAME); @BeforeAll - public final void createDefaultTables() { - TEST_DATA_SCENARIOS.allScalarTypesRowsWithNulls(keyspaceName, TABLE_NAME); + public final void createScenario() { + TEST_DATA_SCENARIO.create(); + } + + @AfterAll + public final void dropScenario() { + TEST_DATA_SCENARIO.drop(); } private static Stream findWithProjectionSchemaTests() { var commands = List.of(Command.CommandName.FIND, Command.CommandName.FIND_ONE); - // we ask to project columns of these types - List> typeCombinations = + // we ask to project these columns + List projections = List.of( - List.of(), // for the default no projection - ApiDataTypesForTesting.ALL_SCALAR_TYPES_FOR_CREATE, - List.of(ApiDataTypeDefs.DURATION), - List.of(ApiDataTypeDefs.TEXT, ApiDataTypeDefs.DURATION), - List.of(ApiDataTypeDefs.TEXT, ApiDataTypeDefs.DURATION, ApiDataTypeDefs.INT)); + ApiColumnDefContainer.of(), // for the default no projection + TEST_DATA_SCENARIO.nonPkColumns, + ApiColumnDefContainer.of(TEST_DATA_SCENARIO.columnForDatatype(ApiDataTypeDefs.TEXT)), + ApiColumnDefContainer.of( + TEST_DATA_SCENARIO.columnForDatatype(ApiDataTypeDefs.TEXT), + TEST_DATA_SCENARIO.columnForDatatype(ApiDataTypeDefs.DURATION)), + ApiColumnDefContainer.of( + TEST_DATA_SCENARIO.columnForDatatype(ApiDataTypeDefs.TEXT), + TEST_DATA_SCENARIO.columnForDatatype(ApiDataTypeDefs.DURATION), + TEST_DATA_SCENARIO.columnForDatatype(ApiDataTypeDefs.INT))); var combinations = new ArrayList(); for (var command : commands) { - for (var types : typeCombinations) { - - var columns = - types.stream() - .collect(Collectors.toMap(TestDataScenarios::columnName, Function.identity())); - combinations.add(Arguments.of(command, columns)); + for (var projection : projections) { + combinations.add(Arguments.of(command, projection)); } // specials combinations.add( - Arguments.of(command, Map.of(TestDataScenarios.ID_COL, ApiDataTypeDefs.TEXT))); + Arguments.of(command, ApiColumnDefContainer.of(TEST_DATA_SCENARIO.primaryKey))); } return combinations.stream(); @@ -72,13 +79,16 @@ private static Stream findWithProjectionSchemaTests() { @ParameterizedTest @MethodSource("findWithProjectionSchemaTests") public void findFilterWithProjectionSchema( - Command.CommandName commandName, Map columns) { + Command.CommandName commandName, ApiColumnDefContainer projectionColumns) { // row-1 is the row with all the values set var validator = assertTableCommand(keyspaceName, TABLE_NAME) .templated() - .find(commandName, Map.of("id", "row-1"), columns.keySet().stream().toList()) + .find( + commandName, + Map.of("id", "row-1"), + projectionColumns.keySet().stream().map(CqlIdentifier::asInternal).toList()) .wasSuccessful() .hasProjectionSchema(); @@ -90,18 +100,21 @@ public void findFilterWithProjectionSchema( throw new IllegalArgumentException("Unexpected command name: " + commandName); } - assertProjectionSchema(validator, columns); + assertProjectionSchema(validator, projectionColumns); } @ParameterizedTest @MethodSource("findWithProjectionSchemaTests") public void findNoFilterWithProjectionSchema( - Command.CommandName commandName, Map projectionColumns) { + Command.CommandName commandName, ApiColumnDefContainer projectionColumns) { var validator = assertTableCommand(keyspaceName, TABLE_NAME) .templated() - .find(commandName, Map.of(), projectionColumns.keySet().stream().toList()) + .find( + commandName, + Map.of(), + projectionColumns.keySet().stream().map(CqlIdentifier::asInternal).toList()) .wasSuccessful() .hasProjectionSchema(); @@ -131,8 +144,8 @@ public void findManyProjectionMissingColumns() { } private DataApiResponseValidator assertProjectionSchema( - DataApiResponseValidator validator, Map columns) { - columns.forEach(validator::hasProjectionSchemaWith); + DataApiResponseValidator validator, ApiColumnDefContainer columns) { + columns.values().forEach(validator::hasProjectionSchemaWith); return validator; } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiCommandSenders.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiCommandSenders.java index 15bac61009..2bba51b1b7 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiCommandSenders.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiCommandSenders.java @@ -9,6 +9,4 @@ public static DataApiKeyspaceCommandSender assertNamespaceCommand(String keyspac public static DataApiTableCommandSender assertTableCommand(String keyspace, String tableName) { return new DataApiTableCommandSender(keyspace, tableName); } - - public static final TestDataScenarios TEST_DATA_SCENARIOS = new TestDataScenarios(); } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiResponseValidator.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiResponseValidator.java index 8082bc5ee7..f2a957d943 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiResponseValidator.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/DataApiResponseValidator.java @@ -10,6 +10,7 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandStatus; import io.stargate.sgv2.jsonapi.config.constants.ErrorObjectV2Constants; import io.stargate.sgv2.jsonapi.exception.*; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiColumnDef; import io.stargate.sgv2.jsonapi.service.schema.tables.ApiDataType; import java.util.Map; import org.hamcrest.Matcher; @@ -232,6 +233,10 @@ public DataApiResponseValidator hasSingleDocument() { return body("data.document", is(notNullValue())); } + public DataApiResponseValidator hasSingleDocument(String documentJSON) { + return body("data.document", jsonEquals(documentJSON)); + } + public DataApiResponseValidator hasEmptyDataDocuments() { return body("data.documents", is(empty())); } @@ -245,6 +250,10 @@ public DataApiResponseValidator hasProjectionSchema() { return hasField("status." + CommandStatus.PROJECTION_SCHEMA); } + public DataApiResponseValidator hasProjectionSchemaWith(ApiColumnDef columnDef) { + return hasProjectionSchemaWith(columnDef.name().asInternal(), columnDef.type()); + } + public DataApiResponseValidator hasProjectionSchemaWith(String columnName, ApiDataType type) { // expected format /** diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/KeyspaceTemplates.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/KeyspaceTemplates.java index fbe82152ca..1a04f06671 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/KeyspaceTemplates.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/KeyspaceTemplates.java @@ -1,5 +1,7 @@ package io.stargate.sgv2.jsonapi.api.v1.util; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiColumnDef; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiColumnDefContainer; import java.util.Map; public class KeyspaceTemplates extends TemplateRunner { @@ -14,6 +16,25 @@ public KeyspaceTemplates(DataApiKeyspaceCommandSender sender) { // DDL - TABLES // =================================================================================================================== + public DataApiResponseValidator createTable( + String tableName, ApiColumnDefContainer columns, ApiColumnDef primaryKeyDef) { + var json = + """ + { + "name": "%s", + "definition": { + "columns": %s, + "primaryKey": %s + } + } + """ + .formatted( + tableName, + asJSON(columns.toColumnsDesc()), + asJSON(primaryKeyDef.name().asInternal())); + return sender.postCreateTable(json); + } + public DataApiResponseValidator createTable( String tableName, Map columns, Object primaryKeyDef) { var json = diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TableTemplates.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TableTemplates.java index 77e96c61b4..c8045a7f03 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TableTemplates.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TableTemplates.java @@ -1,16 +1,13 @@ package io.stargate.sgv2.jsonapi.api.v1.util; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; import io.stargate.sgv2.jsonapi.api.model.command.Command; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.stream.Collectors; public class TableTemplates extends TemplateRunner { - private static final ObjectMapper MAPPER = new ObjectMapper(); - private DataApiTableCommandSender sender; public TableTemplates(DataApiTableCommandSender sender) { @@ -21,36 +18,87 @@ public TableTemplates(DataApiTableCommandSender sender) { // DML - FIND // ================================================================================================================== - private String findClause(Map filter, List columns) { - var projection = columns.stream().collect(Collectors.toMap(col -> col, col -> 1)); - - var clause = - Map.of( - "filter", filter, - "projection", projection); + private String findClause( + Map filter, + List projection, + Map sort, + Map options) { - try { - return MAPPER.writeValueAsString(clause); - } catch (JsonProcessingException e) { - throw new RuntimeException(e); + var clause = new LinkedHashMap<>(); + if (filter != null) { + clause.put("filter", filter); + } + if (projection != null) { + // this is for the positive projection of selecting columns + clause.put("projection", projection.stream().collect(Collectors.toMap(col -> col, col -> 1))); + } + if (sort != null) { + clause.put("sort", sort); } + if (sort != null) { + clause.put("options", options); + } + return asJSON(clause); } public DataApiResponseValidator find( Command.CommandName commandName, Map filter, List columns) { + return find(commandName, filter, columns, null); + } + + public DataApiResponseValidator find( + Command.CommandName commandName, + Map filter, + List columns, + Map sort) { + return find(commandName, filter, columns, sort, null); + } + + public DataApiResponseValidator find( + Command.CommandName commandName, + Map filter, + List columns, + Map sort, + Map options) { return switch (commandName) { - case FIND_ONE -> findOne(filter, columns); - case FIND -> find(filter, columns); + case FIND_ONE -> findOne(filter, columns, sort, options); + case FIND -> find(filter, columns, sort, options); default -> throw new IllegalArgumentException("Unexpected command for find: " + commandName); }; } public DataApiResponseValidator findOne(Map filter, List columns) { - return sender.postFindOne(findClause(filter, columns)); + return findOne(filter, columns, null, null); + } + + public DataApiResponseValidator findOne( + Map filter, List columns, Map sort) { + return findOne(filter, columns, sort, null); + } + + public DataApiResponseValidator findOne( + Map filter, + List columns, + Map sort, + Map options) { + return sender.postFindOne(findClause(filter, columns, sort, options)); } - public DataApiResponseValidator find(Map filter, List columns) { - return sender.postFind(findClause(filter, columns)); + public DataApiResponseValidator find(Map filter, List projection) { + return find(filter, projection, null); + } + + public DataApiResponseValidator find( + Map filter, List projection, Map sort) { + return find(filter, projection, sort, null); + } + + public DataApiResponseValidator find( + Map filter, + List projection, + Map sort, + Map options) { + return sender.postFind(findClause(filter, projection, sort, options)); } public DataApiResponseValidator find(String filter) { @@ -122,6 +170,10 @@ public DataApiResponseValidator insertOne(String document) { return sender.postInsertOne(json); } + public DataApiResponseValidator insertManyMap(List> documents) { + return insertMany(documents.stream().map(TemplateRunner::asJSON).collect(Collectors.toList())); + } + public DataApiResponseValidator insertMany(String... documents) { return insertMany(List.of(documents)); } @@ -153,6 +205,18 @@ public DataApiResponseValidator createIndex(String indexName, String column) { return sender.postCreateIndex(json); } + public DataApiResponseValidator createVectorIndex(String indexName, String column) { + var json = + """ + { + "name": "%s", + "definition": {"column": "%s"} + } + """ + .formatted(indexName, column); + return sender.postCreateVectorIndex(json); + } + public DataApiResponseValidator alterTable(String alterOperation, Object columns) { var json = """ diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TemplateRunner.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TemplateRunner.java index 4fc350489c..ac05416691 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TemplateRunner.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TemplateRunner.java @@ -9,7 +9,7 @@ public abstract class TemplateRunner { protected TemplateRunner() {} - protected static String asJSON(Object ob) { + public static String asJSON(Object ob) { try { return MAPPER.writeValueAsString(ob); } catch (IOException e) { // should never happen diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TestDataScenarios.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TestDataScenarios.java deleted file mode 100644 index cd78159274..0000000000 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/TestDataScenarios.java +++ /dev/null @@ -1,116 +0,0 @@ -package io.stargate.sgv2.jsonapi.api.v1.util; - -import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.assertNamespaceCommand; -import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.assertTableCommand; - -import com.fasterxml.jackson.databind.ObjectMapper; -import io.stargate.sgv2.jsonapi.fixtures.data.DefaultData; -import io.stargate.sgv2.jsonapi.fixtures.types.ApiDataTypesForTesting; -import io.stargate.sgv2.jsonapi.service.schema.tables.ApiDataTypeDefs; -import io.stargate.sgv2.jsonapi.service.schema.tables.PrimitiveApiDataTypeDef; -import java.util.ArrayList; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Re-usable scenarios of tables and sample data for testing. - * - *

Will create the table, insert rows, and verify - */ -public class TestDataScenarios { - - private static final Logger LOGGER = LoggerFactory.getLogger(TestDataScenarios.class); - private static final ObjectMapper MAPPER = new ObjectMapper(); - - public static final String ID_COL = "id"; - public static final String COL_NAME_PREFIX = "col_"; - - private static final DefaultData dataSource = new DefaultData(); - - /** - * Create a table with a col for all {@link - * io.stargate.sgv2.jsonapi.fixtures.types.ApiDataTypesForTesting#ALL_SCALAR_TYPES_FOR_CREATE} and - * insert a rows, one with all set, one for each col with a null value. - * - * @param keyspaceName - * @param tableName - */ - public void allScalarTypesRowsWithNulls(String keyspaceName, String tableName) { - - createTableWithTypes( - keyspaceName, tableName, ApiDataTypesForTesting.ALL_SCALAR_TYPES_FOR_CREATE); - - var rows = new ArrayList<>(); - for (int i = -1; i < ApiDataTypesForTesting.ALL_SCALAR_TYPES_FOR_CREATE.size(); i++) { - - // linked to keep order - var row = new LinkedHashMap(); - row.put(ID_COL, "row" + i); - addRowValues(row, ApiDataTypesForTesting.ALL_SCALAR_TYPES_FOR_CREATE, i); - rows.add(row); - } - var rowsJson = - rows.stream() - .map( - row -> { - try { - return MAPPER.writeValueAsString(row); - } catch (Exception e) { - throw new RuntimeException(e); - } - }) - .toList(); - LOGGER.warn("Inserting rows {}", rowsJson); - - assertTableCommand(keyspaceName, tableName).templated().insertMany(rowsJson).wasSuccessful(); - } - - private void createTableWithTypes( - String keyspaceName, String tableName, List types) { - - // linked to keep order - var columns = new LinkedHashMap(); - columns.put(ID_COL, columnDef(ApiDataTypeDefs.TEXT)); - buildColumns(columns, types); - - LOGGER.warn("Creating table {} with columns {}", tableName, columns); - assertNamespaceCommand(keyspaceName) - .templated() - .createTable(tableName, columns, ID_COL) - .wasSuccessful(); - } - - private LinkedHashMap buildColumns( - LinkedHashMap columns, List types) { - - types.forEach( - type -> { - columns.put(columnName(type), columnDef(type)); - }); - return columns; - } - - private static Map columnDef(PrimitiveApiDataTypeDef type) { - return Map.of("type", type.typeName().apiName()); - } - - public static String columnName(PrimitiveApiDataTypeDef type) { - return COL_NAME_PREFIX + type.typeName().apiName(); - } - - public Object columnValue(PrimitiveApiDataTypeDef type) { - return dataSource.fromJSON(type.cqlType()).value(); - } - - private LinkedHashMap addRowValues( - LinkedHashMap row, List types, int nullIndex) { - for (int i = 0; i < types.size(); i++) { - var type = types.get(i); - row.put(columnName(type), i == nullIndex ? null : columnValue(type)); - } - return row; - } -} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/AllScalarTypesTableScenario.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/AllScalarTypesTableScenario.java new file mode 100644 index 0000000000..880b0a7c43 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/AllScalarTypesTableScenario.java @@ -0,0 +1,55 @@ +package io.stargate.sgv2.jsonapi.api.v1.util.scenarios; + +import com.datastax.oss.driver.api.core.CqlIdentifier; +import io.stargate.sgv2.jsonapi.fixtures.data.DefaultData; +import io.stargate.sgv2.jsonapi.fixtures.types.ApiDataTypesForTesting; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiColumnDef; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiColumnDefContainer; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiDataType; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Creates a table with: - PK of the single field column {@link TestDataScenario#ID_COL} - a column + * for each scalar type in {@link ApiDataTypesForTesting#ALL_SCALAR_TYPES_FOR_CREATE} - the non PK + * columns are named with the prefix {@link #COL_NAME_PREFIX} then the data type name - gets data + * from the {@link DefaultData} class - inserts row with the PK prefix "row" - the row "row-1" (-1) + * has a value for every column - the row "row-0" and beyond have a null for a single column in each + * row, one column at a time - call {@link #columnForDatatype(ApiDataType)} to get the column for a + * specific data type + */ +public class AllScalarTypesTableScenario extends TestDataScenario { + + public static final String COL_NAME_PREFIX = "col_"; + + public AllScalarTypesTableScenario(String keyspaceName, String tableName) { + super(keyspaceName, tableName, ID_COL, createColumns(), new DefaultData()); + } + + public ApiColumnDef columnForDatatype(ApiDataType type) { + return nonPkColumns.get(columnNameForType(type)); + } + + public static CqlIdentifier columnNameForType(ApiDataType type) { + return CqlIdentifier.fromInternal(COL_NAME_PREFIX + type.typeName().apiName()); + } + + private static ApiColumnDefContainer createColumns() { + var columns = new ApiColumnDefContainer(); + columns.put(ID_COL); + addColumnsForTypes( + columns, + ApiDataTypesForTesting.ALL_SCALAR_TYPES_FOR_CREATE, + AllScalarTypesTableScenario::columnNameForType); + return columns; + } + + @Override + protected void insertRows() { + AtomicInteger pkCounter = new AtomicInteger(-1); + + var rows = + createRowForEachNonPkColumnWithNull(apiColumnDef -> "row" + pkCounter.getAndIncrement()); + + insertManyRows(rows); + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/TestDataScenario.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/TestDataScenario.java new file mode 100644 index 0000000000..5750b5fdb7 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/TestDataScenario.java @@ -0,0 +1,154 @@ +package io.stargate.sgv2.jsonapi.api.v1.util.scenarios; + +import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.assertNamespaceCommand; +import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.assertTableCommand; + +import com.datastax.oss.driver.api.core.CqlIdentifier; +import com.fasterxml.jackson.databind.ObjectMapper; +import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.JsonLiteral; +import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.JsonType; +import io.stargate.sgv2.jsonapi.fixtures.data.FixtureData; +import io.stargate.sgv2.jsonapi.service.schema.tables.*; +import java.util.*; +import java.util.function.Function; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Base for classes that build a test data scenario */ +public abstract class TestDataScenario { + private static final Logger LOGGER = LoggerFactory.getLogger(TestDataScenario.class); + private static final ObjectMapper MAPPER = new ObjectMapper(); + + public static final ApiColumnDef ID_COL = + new ApiColumnDef(CqlIdentifier.fromInternal("id"), ApiDataTypeDefs.TEXT); + + protected final String keyspaceName; + protected final String tableName; + + public final FixtureData dataSource; + public final ApiColumnDef primaryKey; + public final ApiColumnDefContainer allColumns; + public final ApiColumnDefContainer nonPkColumns; + + protected TestDataScenario( + String keyspaceName, + String tableName, + ApiColumnDef primaryKey, + ApiColumnDefContainer allColumns, + FixtureData dataSource) { + this.keyspaceName = keyspaceName; + this.tableName = tableName; + this.dataSource = dataSource; + + this.primaryKey = primaryKey; + this.allColumns = allColumns.toUnmodifiable(); + var workingNonPkColumns = new ApiColumnDefContainer(allColumns); + workingNonPkColumns.remove(primaryKey.name()); + this.nonPkColumns = workingNonPkColumns.toUnmodifiable(); + } + + public void create() { + createTable(); + createIndexes(); + insertRows(); + } + + public void drop() { + assertNamespaceCommand(keyspaceName).templated().dropTable(tableName, false).wasSuccessful(); + } + + public static String fieldName(ApiColumnDef apiColumnDef) { + return apiColumnDef.name().asInternal(); + } + + public Object columnValue(ApiColumnDef apiColumnDef) { + var jsonLiteral = dataSource.fromJSON(apiColumnDef); + if (jsonLiteral.type() == JsonType.ARRAY) { + List> literals = (List>) jsonLiteral.value(); + return literals.stream().map(JsonLiteral::value).toArray(); + } + return jsonLiteral.value(); + } + + protected void createTable() { + createTableWithTypes(keyspaceName, tableName, primaryKey, allColumns); + } + + protected void createIndexes() {} + ; + + protected void insertRows() {} + ; + + // ========= Helper methods ========= + + protected void createTableWithTypes( + String keyspaceName, + String tableName, + ApiColumnDef primaryKey, + ApiColumnDefContainer columns) { + + LOGGER.warn("Creating table {}.{} with columns {}", keyspaceName, tableName, columns); + assertNamespaceCommand(keyspaceName) + .templated() + .createTable(tableName, columns, primaryKey) + .wasSuccessful(); + } + + protected void insertManyRows(List> rows) { + if (LOGGER.isWarnEnabled()) { + for (var row : rows) { + LOGGER.warn("Inserting row {}", row); + } + } + assertTableCommand(keyspaceName, tableName).templated().insertManyMap(rows).wasSuccessful(); + } + + protected static void addColumnsForTypes( + ApiColumnDefContainer columns, List types, String columnPrefix) { + addColumnsForTypes( + columns, + types, + type -> CqlIdentifier.fromInternal(columnPrefix + type.typeName().apiName())); + } + + protected static void addColumnsForTypes( + ApiColumnDefContainer columns, + List types, + Function nameMapper) { + types.forEach( + type -> { + columns.put(new ApiColumnDef(nameMapper.apply(type), type)); + }); + } + + protected void addNonPKColValuesWithNull(Map row, int nullIndex) { + + var i = 0; + for (var apiColumnDef : nonPkColumns.values()) { + row.put(fieldName(apiColumnDef), i == nullIndex ? null : columnValue(apiColumnDef)); + i++; + } + } + + protected List> createRowForEachNonPkColumnWithNull( + Function pkValueSupplier) { + var rows = new ArrayList>(); + + // and a row with all the columns set + var row = new HashMap(); + row.put(fieldName(primaryKey), pkValueSupplier.apply(primaryKey)); + addNonPKColValuesWithNull(row, -1); + rows.add(row); + + var i = 0; + for (var apiColumnDef : nonPkColumns.values()) { + row = new HashMap(); + row.put(fieldName(primaryKey), pkValueSupplier.apply(primaryKey)); + addNonPKColValuesWithNull(row, i++); + rows.add(row); + } + + return rows; + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/VectorDimension5TableScenario.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/VectorDimension5TableScenario.java new file mode 100644 index 0000000000..aae4db7782 --- /dev/null +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/util/scenarios/VectorDimension5TableScenario.java @@ -0,0 +1,94 @@ +package io.stargate.sgv2.jsonapi.api.v1.util.scenarios; + +import static io.stargate.sgv2.jsonapi.api.v1.util.DataApiCommandSenders.assertTableCommand; +import static io.stargate.sgv2.jsonapi.api.v1.util.TemplateRunner.asJSON; + +import com.datastax.oss.driver.api.core.CqlIdentifier; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.JsonNodeFactory; +import io.stargate.sgv2.jsonapi.fixtures.data.DefaultData; +import io.stargate.sgv2.jsonapi.fixtures.types.ApiDataTypesForTesting; +import io.stargate.sgv2.jsonapi.service.schema.tables.*; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.Map; + +/** + * Creates a table with: - PK of the single field column {@link TestDataScenario#ID_COL} - a column + * for each scalar type in {@link ApiDataTypesForTesting#ALL_SCALAR_TYPES_FOR_CREATE} - the non PK + * columns are named with the prefix {@link #COL_NAME_PREFIX} then the data type name - gets data + * from the {@link DefaultData} class - inserts row with the PK prefix "row" - the row "row-1" (-1) + * has a value for every column - the row "row-0" and beyond have a null for a single column in each + * row, one column at a time - call {@link #columnForDatatype(ApiDataType)} to get the column for a + * specific data type + */ +public class VectorDimension5TableScenario extends TestDataScenario { + + public static final ApiColumnDef CONTENT_COL = + new ApiColumnDef(CqlIdentifier.fromCql("content"), ApiDataTypeDefs.TEXT); + public static final ApiColumnDef INDEXED_VECTOR_COL = + new ApiColumnDef(CqlIdentifier.fromCql("indexed_vector"), ApiVectorType.from(5)); + public static final ApiColumnDef UNINDEXED_VECTOR_COL = + new ApiColumnDef(CqlIdentifier.fromCql("unindexed_vector"), ApiVectorType.from(5)); + + public static final ArrayNode KNOWN_VECTOR = JsonNodeFactory.instance.arrayNode(5); + public static final Map KNOWN_VECTOR_ROW = new LinkedHashMap<>(); + public static final String KNOWN_VECTOR_ROW_JSON; + + static { + for (int i = 0; i < 5; i++) { + KNOWN_VECTOR.add(1.0f); + } + + KNOWN_VECTOR_ROW.put(fieldName(ID_COL), "row-1"); + KNOWN_VECTOR_ROW.put( + fieldName(CONTENT_COL), "This is the content for row-1 - all 1.0 vectors "); + KNOWN_VECTOR_ROW.put(fieldName(INDEXED_VECTOR_COL), KNOWN_VECTOR); + KNOWN_VECTOR_ROW.put(fieldName(UNINDEXED_VECTOR_COL), KNOWN_VECTOR); + + KNOWN_VECTOR_ROW_JSON = asJSON(KNOWN_VECTOR_ROW); + } + + public VectorDimension5TableScenario(String keyspaceName, String tableName) { + super(keyspaceName, tableName, ID_COL, createColumns(), new DefaultData()); + } + + private static ApiColumnDefContainer createColumns() { + + var columns = new ApiColumnDefContainer(); + columns.put(ID_COL); + columns.put(CONTENT_COL); + columns.put(INDEXED_VECTOR_COL); + columns.put(UNINDEXED_VECTOR_COL); + return columns; + } + + @Override + protected void createIndexes() { + assertTableCommand(keyspaceName, tableName) + .templated() + .createVectorIndex( + "idx_" + tableName + "_" + fieldName(INDEXED_VECTOR_COL), fieldName(INDEXED_VECTOR_COL)) + .wasSuccessful(); + } + + @Override + protected void insertRows() { + + var rows = new ArrayList>(); + for (int i = 0; i < 20; i++) { + var row = new HashMap(); + row.put(fieldName(ID_COL), "row" + i); + row.put(fieldName(CONTENT_COL), "This is the content for row " + i); + var vector = columnValue(INDEXED_VECTOR_COL); + row.put(fieldName(INDEXED_VECTOR_COL), vector); + row.put(fieldName(UNINDEXED_VECTOR_COL), vector); + rows.add(row); + } + + rows.add(KNOWN_VECTOR_ROW); + + insertManyRows(rows); + } +} diff --git a/src/test/java/io/stargate/sgv2/jsonapi/fixtures/data/DefaultData.java b/src/test/java/io/stargate/sgv2/jsonapi/fixtures/data/DefaultData.java index 5325505cbc..990f6a3553 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/fixtures/data/DefaultData.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/fixtures/data/DefaultData.java @@ -2,13 +2,17 @@ import com.datastax.oss.driver.api.core.type.DataType; import com.datastax.oss.driver.api.core.type.DataTypes; +import com.datastax.oss.driver.api.core.type.VectorType; +import com.datastax.oss.driver.internal.core.type.DefaultVectorType; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.JsonNodeFactory; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.EJSONWrapper; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.JsonLiteral; import io.stargate.sgv2.jsonapi.fixtures.CqlFixture; +import io.stargate.sgv2.jsonapi.service.cqldriver.override.ExtendedVectorType; import io.stargate.sgv2.jsonapi.service.shredding.tables.RowShredder; import java.math.BigDecimal; +import java.util.Random; /** * Implementations fo the {@link FixtureData} interface that provide test data for use by the {@link @@ -22,6 +26,8 @@ */ public class DefaultData implements FixtureData { + private static final Random RANDOM = new Random(); + @Override public JsonLiteral fromJSON(DataType type) { // We want to get the value type that the RowShredder will generate, so just call it directly @@ -63,15 +69,30 @@ protected JsonNode getJsonNode(DataType type) { } else if (DataTypes.TIME.equals(type)) { return JsonNodeFactory.instance.textNode("13:13:04.010"); } else if (DataTypes.TIMESTAMP.equals(type)) { - return JsonNodeFactory.instance.textNode("1975-04-03T13:13:04Z"); + return JsonNodeFactory.instance.textNode("1970-02-01T13:13:04Z"); } else if (DataTypes.TINYINT.equals(type)) { return JsonNodeFactory.instance.numberNode(Byte.valueOf("1")); } else if (DataTypes.UUID.equals(type)) { return JsonNodeFactory.instance.textNode("550e8400-e29b-41d4-a716-446655440000"); } else if (DataTypes.VARINT.equals(type)) { return JsonNodeFactory.instance.numberNode(BigDecimal.valueOf(1L)); + } else if (type.getClass().getName().startsWith(DefaultVectorType.VECTOR_CLASS_NAME)) { + // See Driver class DataTypes#custom() for the above check + return vectorNode((VectorType) type); + } else if (type.getClass().getName().startsWith(ExtendedVectorType.class.getName())) { + // See Driver class DataTypes#custom() for the above check + return vectorNode((VectorType) type); + } + throw new UnsupportedOperationException( + "Unknown type: %s className %s".formatted(type, type.getClass().getName())); + } + + private JsonNode vectorNode(VectorType vt) { + var arrayNode = JsonNodeFactory.instance.arrayNode(vt.getDimensions()); + for (int i = 0; i < vt.getDimensions(); i++) { + arrayNode.add(RANDOM.nextFloat()); } - throw new UnsupportedOperationException("Unknown type: " + type); + return arrayNode; } /** Override so the test description gets a simple name */ diff --git a/src/test/java/io/stargate/sgv2/jsonapi/fixtures/data/FixtureData.java b/src/test/java/io/stargate/sgv2/jsonapi/fixtures/data/FixtureData.java index 3df2502cce..c07011529e 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/fixtures/data/FixtureData.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/fixtures/data/FixtureData.java @@ -3,6 +3,8 @@ import com.datastax.oss.driver.api.core.type.DataType; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.JsonLiteral; import io.stargate.sgv2.jsonapi.fixtures.CqlFixture; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiColumnDef; +import io.stargate.sgv2.jsonapi.service.schema.tables.ApiDataType; import java.util.List; /** @@ -29,6 +31,14 @@ public interface FixtureData { /** Data for types that are not supported by the API. */ List UNSUPPORTED_TYPES = List.of(new UnsupportedTypesData()); + default JsonLiteral fromJSON(ApiColumnDef columnDef) { + return fromJSON(columnDef.type()); + } + + default JsonLiteral fromJSON(ApiDataType apiDataType) { + return fromJSON(apiDataType.cqlType()); + } + /** * Returns a Java value that we want we would have read from Jackson. * From 5f5d809e5fddbbb78c9346621853a5e7f18de702 Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Fri, 1 Nov 2024 10:43:42 +1300 Subject: [PATCH 15/19] fmt fix --- .../jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java index 1f97b78284..dbbab78f04 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/tables/AnnSortTableIntegrationTest.java @@ -60,7 +60,8 @@ public void findUnindexedVector(Command.CommandName commandName) { SortException.Code.CANNOT_VECTOR_SORT_NON_INDEXED_VECTOR_COLUMNS, SortException.class, "And has indexes on the vector columns: %s.\nThe command attempted to sort vector columns: %s" - .formatted(SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), + .formatted( + SCENARIO.fieldName(VectorDimension5TableScenario.INDEXED_VECTOR_COL), SCENARIO.fieldName(VectorDimension5TableScenario.UNINDEXED_VECTOR_COL))); } From 7a5e30dfb9640183cebc1f777a8688a3c4a0b41a Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Sat, 2 Nov 2024 10:31:30 +1300 Subject: [PATCH 16/19] tweak from review --- .../service/operation/tables/TableANNOrderByCQlClause.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableANNOrderByCQlClause.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableANNOrderByCQlClause.java index d21b278be7..68a6f2b656 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableANNOrderByCQlClause.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/tables/TableANNOrderByCQlClause.java @@ -21,7 +21,6 @@ public class TableANNOrderByCQlClause implements OrderByCqlClause { public TableANNOrderByCQlClause(ApiColumnDef apiColumnDef, CqlVector vector) { this.apiColumnDef = Objects.requireNonNull(apiColumnDef, "apiColumnDef must not be null"); this.vector = Objects.requireNonNull(vector, "vector must not be null"); - ; // sanity check if (apiColumnDef.type().typeName() != ApiTypeName.VECTOR) { From 6a8e98b4939eef64b90add3359e50db949f9e81a Mon Sep 17 00:00:00 2001 From: Yuqi Du Date: Fri, 1 Nov 2024 15:05:43 -0700 Subject: [PATCH 17/19] init --- .../sgv2/jsonapi/exception/SortException.java | 6 ++++-- .../resolver/DeleteOneCommandResolver.java | 9 +++++++++ .../resolver/UpdateOneCommandResolver.java | 9 +++++++++ src/main/resources/errors.yaml | 19 +++++++++++++++++++ 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java index e63d38e634..e5ff36a2d6 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/SortException.java @@ -14,10 +14,12 @@ public SortException(ErrorInstance errorInstance) { } public enum Code implements ErrorCode { + CANNOT_MIX_VECTOR_AND_NON_VECTOR_SORT, + CANNOT_SORT_TABLE_DELETE_COMMAND, + CANNOT_SORT_TABLE_UPDATE_COMMAND, CANNOT_SORT_UNKNOWN_COLUMNS, - CANNOT_VECTOR_SORT_NON_VECTOR_COLUMNS, CANNOT_VECTOR_SORT_NON_INDEXED_VECTOR_COLUMNS, - CANNOT_MIX_VECTOR_AND_NON_VECTOR_SORT, + CANNOT_VECTOR_SORT_NON_VECTOR_COLUMNS, MORE_THAN_ONE_VECTOR_SORT, ; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/DeleteOneCommandResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/DeleteOneCommandResolver.java index 56d4e50047..a09dc8fac0 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/DeleteOneCommandResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/DeleteOneCommandResolver.java @@ -1,5 +1,7 @@ package io.stargate.sgv2.jsonapi.service.resolver; +import static io.stargate.sgv2.jsonapi.exception.ErrorFormatters.errVars; + import com.fasterxml.jackson.databind.ObjectMapper; import io.micrometer.core.instrument.MeterRegistry; import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; @@ -9,6 +11,7 @@ import io.stargate.sgv2.jsonapi.api.v1.metrics.JsonApiMetricsConfig; import io.stargate.sgv2.jsonapi.config.DebugModeConfig; import io.stargate.sgv2.jsonapi.config.OperationsConfig; +import io.stargate.sgv2.jsonapi.exception.SortException; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.TableSchemaObject; import io.stargate.sgv2.jsonapi.service.operation.*; import io.stargate.sgv2.jsonapi.service.operation.collections.CollectionReadType; @@ -66,6 +69,12 @@ public DeleteOneCommandResolver( public Operation resolveTableCommand( CommandContext ctx, DeleteOneCommand command) { + // Sort clause is not supported for table deleteOne command. + if (command.sortClause() != null && !command.sortClause().isEmpty()) { + throw SortException.Code.CANNOT_SORT_TABLE_DELETE_COMMAND.get( + errVars(ctx.schemaObject(), map -> {})); + } + var builder = new DeleteAttemptBuilder<>(ctx.schemaObject(), true); var where = diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/UpdateOneCommandResolver.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/UpdateOneCommandResolver.java index 37c62fbc2c..b4a0ffd7c8 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/UpdateOneCommandResolver.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/UpdateOneCommandResolver.java @@ -1,5 +1,7 @@ package io.stargate.sgv2.jsonapi.service.resolver; +import static io.stargate.sgv2.jsonapi.exception.ErrorFormatters.errVars; + import com.fasterxml.jackson.databind.ObjectMapper; import io.micrometer.core.instrument.MeterRegistry; import io.stargate.sgv2.jsonapi.api.model.command.CommandContext; @@ -9,6 +11,7 @@ import io.stargate.sgv2.jsonapi.api.v1.metrics.JsonApiMetricsConfig; import io.stargate.sgv2.jsonapi.config.DebugModeConfig; import io.stargate.sgv2.jsonapi.config.OperationsConfig; +import io.stargate.sgv2.jsonapi.exception.SortException; import io.stargate.sgv2.jsonapi.service.cqldriver.executor.TableSchemaObject; import io.stargate.sgv2.jsonapi.service.embedding.DataVectorizerService; import io.stargate.sgv2.jsonapi.service.operation.*; @@ -80,6 +83,12 @@ public Class getCommandClass() { public Operation resolveTableCommand( CommandContext ctx, UpdateOneCommand command) { + // Sort clause is not supported for table updateOne command. + if (command.sortClause() != null && !command.sortClause().isEmpty()) { + throw SortException.Code.CANNOT_SORT_TABLE_UPDATE_COMMAND.get( + errVars(ctx.schemaObject(), map -> {})); + } + var builder = new UpdateAttemptBuilder<>(ctx.schemaObject()); var where = diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml index a1118e4eeb..264ed10a19 100644 --- a/src/main/resources/errors.yaml +++ b/src/main/resources/errors.yaml @@ -733,7 +733,26 @@ request-errors: The command attempted to sort the non-vector columns: ${sortNonVectorColumns}. Resend the command using only vector columns that have been indexed. + + - scope: SORT + code: CANNOT_SORT_TABLE_DELETE_COMMAND + title: Sort supplier for non sortable delete Table command + body: |- + The command attempted to sort a delete command running against a table. + + Deleting rows in a table does not support sorting, rows can only be deleted by specifying the partition key(s) and optionally the clustering key(s) for the row(s) to be deleted. + + Resend the command without the sort clause. + - scope: SORT + code: CANNOT_SORT_TABLE_UPDATE_COMMAND + title: Sort supplier for non sortable UPDATE Table command + body: |- + The command attempted to sort a update command running against a table. + + Updating row in a table does not support sorting, row can only be updated by specifying full primary key(s). + + Resend the command without the sort clause. # ================================================================================================================ # ================================================================================================================ # Server Errors From 65da8b728487641599cc4cfa81cebc7f51e4b1fe Mon Sep 17 00:00:00 2001 From: Yuqi Du Date: Fri, 1 Nov 2024 15:09:54 -0700 Subject: [PATCH 18/19] merged from main --- src/main/resources/errors.yaml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml index a1118e4eeb..dbeca5a9d7 100644 --- a/src/main/resources/errors.yaml +++ b/src/main/resources/errors.yaml @@ -734,6 +734,26 @@ request-errors: Resend the command using only vector columns that have been indexed. + - scope: SORT + code: CANNOT_SORT_TABLE_DELETE_COMMAND + title: Sort supplier for non sortable delete Table command + body: |- + The command attempted to sort a delete command running against a table. + + Deleting rows in a table does not support sorting, rows can only be deleted by specifying the partition key(s) and optionally the clustering key(s) for the row(s) to be deleted. + + Resend the command without the sort clause. + + - scope: SORT + code: CANNOT_SORT_TABLE_UPDATE_COMMAND + title: Sort supplier for non sortable UPDATE Table command + body: |- + The command attempted to sort a update command running against a table. + + Updating row in a table does not support sorting, row can only be updated by specifying full primary key(s). + + Resend the command without the sort clause. + # ================================================================================================================ # ================================================================================================================ # Server Errors From a8cd346fbf0d78f78218d6235f96bffc46a52d48 Mon Sep 17 00:00:00 2001 From: Aaron Morton Date: Tue, 5 Nov 2024 13:03:22 +1300 Subject: [PATCH 19/19] error msg tweak --- src/main/resources/errors.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml index 4916b765a3..b00dffce9e 100644 --- a/src/main/resources/errors.yaml +++ b/src/main/resources/errors.yaml @@ -780,7 +780,7 @@ request-errors: - scope: SORT code: CANNOT_SORT_TABLE_DELETE_COMMAND - title: Sort supplier for non sortable delete Table command + title: Sort supplied for non sortable delete Table command body: |- The command attempted to sort a delete command running against a table. @@ -790,11 +790,11 @@ request-errors: - scope: SORT code: CANNOT_SORT_TABLE_UPDATE_COMMAND - title: Sort supplier for non sortable UPDATE Table command + title: Sort supplied for non sortable update Table command body: |- The command attempted to sort a update command running against a table. - Updating row in a table does not support sorting, row can only be updated by specifying full primary key(s). + Updating row in a table does not support sorting, a row can only be updated by specifying full primary key(s). Resend the command without the sort clause.