From 7d6306e7846a9ab7107a7af2c5cb0ce6cefed818 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 2 Jul 2024 16:16:42 -0700 Subject: [PATCH 1/6] Update READMEs wrt running things locally (#1225) Co-authored-by: Hazel --- README.md | 12 ++++++---- docker-compose/README.md | 49 ++++++++++++++++++++++++++++++++-------- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index c924ff4737..300b34332a 100644 --- a/README.md +++ b/README.md @@ -28,13 +28,15 @@ The quickest way to test out the Data API directly is to start a local copy of t ```shell cd docker-compose -./start_dse_next_dev_mode.sh +./start_hcd.sh +# or +./start_dse69.sh ``` -This starts an instance of the Data API along with a Stargate coordinator node in "developer mode" (with DataStax Enterprise 6.8 embedded). +This starts an instance of the Data API along with a backend database (HCD or DSE 6.9) > **Warning** -> Running this script with no options will use the latest `v1` tagged version of Data API and latest `v2` tagged version of the Stargate coordinator. Therefore, if you have these tags already present in your local Docker from other development/testing, those are the images that will be used. See our Docker compose [README](docker-compose/README.md) to see additional options. +> Running this script with no options will use the latest `v1` tagged version of Data API. Therefore, if you have these tags already present in your local Docker from other development/testing, those are the images that will be used. See our Docker compose [README](docker-compose/README.md) to see additional options. Once the services are up, you can access the Swagger endpoint at: http://localhost:8181/swagger-ui/ @@ -73,7 +75,9 @@ You can run your application in dev mode that enables live coding using: ```shell script docker run -d --rm -e CLUSTER_NAME=dse-cluster -e CLUSTER_VERSION=6.8 -e ENABLE_AUTH=true -e DEVELOPER_MODE=true -e DS_LICENSE=accept -e DSE=true -p 8081:8081 -p 8091:8091 -p 9042:9042 stargateio/coordinator-dse-next:v2 -./mvnw compile quarkus:dev +./mvnw compile quarkus:dev -Dstargate.data-store.ignore-bridge=true \ + -Dstargate.jsonapi.operations.vectorize-enabled=true \ + -Dstargate.jsonapi.operations.database-config.local-datacenter=dc1 ``` The command above will first spin the single Stargate DSE coordinator in dev that the API would communicate to. diff --git a/docker-compose/README.md b/docker-compose/README.md index fd69ada04b..8753ffc12d 100644 --- a/docker-compose/README.md +++ b/docker-compose/README.md @@ -1,6 +1,6 @@ -# Docker Compose scripts for Data API with DSE-6.9 +# Docker Compose scripts for Data API with HCD or DSE-6.9 -This directory provides two ways to start the Data API with DSE-6.9 or HCD using `docker compose`. +This directory provides two ways to start the Data API with HCD or DSE-6.9 using `docker compose`. ## Prerequisites @@ -19,21 +19,22 @@ You can control the platform using the `-Dquarkus.docker.buildx.platform=linux/a Follow instructions under the [Script options](#script-options) section to use the locally built image. -## Data API with DSE-6.9 cluster +## Data API with HCD or DSE-6.9 cluster -You can start a simple configuration with DSE 6.9 with the following command: +You can start a simple configuration with HCD with the following command: ``` -./start_dse69.sh +./start_hcd.sh ``` -You can start a simple configuration with HCD with the following command: +You can start a simple configuration with DSE 6.9 with the following command: ``` -./start_hcd.sh +./start_dse69.sh ``` -This convenience script verifies your Docker installation meets minimum requirements and brings up the configuration described in the `docker-compose.yml` file. The configuration includes health criteria for each container that is used to ensure the containers come up in the correct order. +This convenience script verifies your Docker installation meets minimum requirements and brings up the configuration described in the `docker-compose.yml` (DSE-6.9) or `docker-compose-hcd.yml` (HCD) file. +The configuration includes health criteria for each container that is used to ensure the containers come up in the correct order. The convenience script uses the `-d` and `--wait` options to track the startup progress, so that the compose command exits when all containers have started and reported healthy status within a specified timeout. @@ -42,6 +43,24 @@ We recommend doing a `docker compose pull` periodically to ensure you always hav Once done using the containers, you can stop them using the command `docker compose down`. +## HCD or DSE-6.9 cluster WITHOUT Data API + +To run local non-Docker instance of Data API, use `-d` with the script: it will prevent running of Data API container: + +``` +./start_hcd.sh -d +# OR +./start_dse69.sh -d +``` + +To run locally built Data API in Quarkus Development mode, you will use: + +``` +/mvnw compile quarkus:dev -Dstargate.data-store.ignore-bridge=true \ + -Dstargate.jsonapi.operations.vectorize-enabled=true \ + -Dstargate.jsonapi.operations.database-config.local-datacenter=dc1 +``` + ## Script options Both convenience scripts support the following options: @@ -54,10 +73,22 @@ Both convenience scripts support the following options: * You can enable reguest logging for the Data API using `-q`: if so, each request is logged under category `io.quarkus.http.access-log` -* You can start dse/hcd node only using `-d` option. This is useful when you want to start Data API locally for development. +* You can start DSE/HCD node only using `-d` option. This is useful when you want to start Data API locally for development. * You can specify the number of cassandra node required using `-n [NUMBER]`. Default is 1. +## CQLSH + +A convenient way to run `cqlsh` to connect locally is to use one of: + +``` +# HCD: +docker exec -it data-api-hcd-1 cqlsh -u cassandra -p cassandra + +# DSE 6.9: +docker exec -it data-api-dse-1 cqlsh -u cassandra -p cassandra +``` + ## Notes * The `.env` file defines variables for the docker compose project name (`COMPOSE_PROJECT_NAME`), From be58951df70115c36e9b18600eef7bcc3a5b4cb6 Mon Sep 17 00:00:00 2001 From: Mahesh Rajamani <99678631+maheshrajamani@users.noreply.github.com> Date: Tue, 2 Jul 2024 20:35:00 -0400 Subject: [PATCH 2/6] Fixed the index usage metrics for IDFilter and InFilter (#1227) --- .../operation/model/impl/DBFilterBase.java | 86 ++++++++++++++++--- 1 file changed, 73 insertions(+), 13 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DBFilterBase.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DBFilterBase.java index 5bd654aeb1..a795e2307b 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DBFilterBase.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DBFilterBase.java @@ -291,6 +291,7 @@ public IDFilter(IDFilter.Operator operator, List values) { super(DOC_ID); this.operator = operator; this.values = values; + updateIndexUsage(); } @Override @@ -301,6 +302,27 @@ public boolean equals(Object o) { return operator == idFilter.operator && Objects.equals(values, idFilter.values); } + private void updateIndexUsage() { + switch (operator) { + case EQ: + this.indexUsage.primaryKeyTag = true; + break; + case NE: + final DocumentId documentId = (DocumentId) values.get(0); + if (documentId.value() instanceof BigDecimal numberId) { + this.indexUsage.numberIndexTag = true; + } else if (documentId.value() instanceof String strId) { + this.indexUsage.textIndexTag = true; + } + break; + case IN: + values.forEach( + v -> { + this.indexUsage.primaryKeyTag = true; + }); + } + } + @Override public int hashCode() { return Objects.hash(operator, values); @@ -315,7 +337,6 @@ public BuiltCondition get() { public List getAll() { switch (operator) { case EQ: - this.indexUsage.primaryKeyTag = true; return List.of( BuiltCondition.of( BuiltCondition.LHS.column("key"), @@ -324,14 +345,12 @@ public List getAll() { case NE: final DocumentId documentId = (DocumentId) values.get(0); if (documentId.value() instanceof BigDecimal numberId) { - this.indexUsage.numberIndexTag = true; return List.of( BuiltCondition.of( BuiltCondition.LHS.mapAccess("query_dbl_values", DOC_ID), Predicate.NEQ, new JsonTerm(DOC_ID, numberId))); } else if (documentId.value() instanceof String strId) { - this.indexUsage.textIndexTag = true; return List.of( BuiltCondition.of( BuiltCondition.LHS.mapAccess("query_text_values", DOC_ID), @@ -347,7 +366,6 @@ public List getAll() { return values.stream() .map( v -> { - this.indexUsage.primaryKeyTag = true; return BuiltCondition.of( BuiltCondition.LHS.column("key"), Predicate.EQ, @@ -391,6 +409,55 @@ boolean canAddField() { return false; } + private void updateIndexUsage() { + List values = arrayValue; + switch (operator) { + case IN: + final ArrayList inResult = new ArrayList<>(); + for (Object value : values) { + if (value instanceof Map) { + // array element is sub_doc + this.indexUsage.textIndexTag = true; + } else if (value instanceof List) { + // array element is array + this.indexUsage.textIndexTag = true; + } else { + this.indexUsage.arrayContainsTag = true; + } + } + break; + case NIN: + if (!this.getPath().equals(DOC_ID)) { + final ArrayList ninResults = new ArrayList<>(); + for (Object value : values) { + if (value instanceof Map) { + // array element is sub_doc + this.indexUsage.textIndexTag = true; + } else if (value instanceof List) { + // array element is array + this.indexUsage.textIndexTag = true; + } else { + this.indexUsage.arrayContainsTag = true; + } + } + } else { + // can not use stream here, since lambda parameter casting is not allowed + List conditions = new ArrayList<>(); + for (Object value : values) { + if (value instanceof DocumentId) { + Object docIdValue = ((DocumentId) value).value(); + if (docIdValue instanceof BigDecimal numberId) { + this.indexUsage.numberIndexTag = true; + } else if (docIdValue instanceof String strId) { + this.indexUsage.textIndexTag = true; + } + } + } + break; + } + } + } + public enum Operator { IN, NIN, @@ -400,6 +467,7 @@ public InFilter(InFilter.Operator operator, String path, List arrayValue super(path); this.arrayValue = arrayValue; this.operator = operator; + updateIndexUsage(); } @Override @@ -429,7 +497,6 @@ public List getAll() { for (Object value : values) { if (value instanceof Map) { // array element is sub_doc - this.indexUsage.textIndexTag = true; inResult.add( BuiltCondition.of( BuiltCondition.LHS.mapAccess("query_text_values", this.getPath()), @@ -437,14 +504,12 @@ public List getAll() { new JsonTerm(this.getPath(), getHash(new DocValueHasher(), value)))); } else if (value instanceof List) { // array element is array - this.indexUsage.textIndexTag = true; inResult.add( BuiltCondition.of( BuiltCondition.LHS.mapAccess("query_text_values", this.getPath()), Predicate.EQ, new JsonTerm(this.getPath(), getHash(new DocValueHasher(), value)))); } else { - this.indexUsage.arrayContainsTag = true; inResult.add( BuiltCondition.of( BuiltCondition.LHS.column(DATA_CONTAINS), @@ -460,7 +525,6 @@ public List getAll() { for (Object value : values) { if (value instanceof Map) { // array element is sub_doc - this.indexUsage.textIndexTag = true; ninResults.add( BuiltCondition.of( BuiltCondition.LHS.mapAccess("query_text_values", this.getPath()), @@ -468,14 +532,12 @@ public List getAll() { new JsonTerm(this.getPath(), getHash(new DocValueHasher(), value)))); } else if (value instanceof List) { // array element is array - this.indexUsage.textIndexTag = true; ninResults.add( BuiltCondition.of( BuiltCondition.LHS.mapAccess("query_text_values", this.getPath()), Predicate.NEQ, new JsonTerm(this.getPath(), getHash(new DocValueHasher(), value)))); } else { - this.indexUsage.arrayContainsTag = true; ninResults.add( BuiltCondition.of( BuiltCondition.LHS.column(DATA_CONTAINS), @@ -491,7 +553,6 @@ public List getAll() { if (value instanceof DocumentId) { Object docIdValue = ((DocumentId) value).value(); if (docIdValue instanceof BigDecimal numberId) { - this.indexUsage.numberIndexTag = true; BuiltCondition condition = BuiltCondition.of( BuiltCondition.LHS.mapAccess("query_dbl_values", DOC_ID), @@ -499,7 +560,6 @@ public List getAll() { new JsonTerm(DOC_ID, numberId)); conditions.add(condition); } else if (docIdValue instanceof String strId) { - this.indexUsage.textIndexTag = true; BuiltCondition condition = BuiltCondition.of( BuiltCondition.LHS.mapAccess("query_text_values", DOC_ID), @@ -627,6 +687,7 @@ public AllFilter(String path, List arrayValue, boolean negation) { super(path); this.arrayValue = arrayValue; this.negation = negation; + this.indexUsage.arrayContainsTag = true; } public boolean isNegation() { @@ -646,7 +707,6 @@ boolean canAddField() { public List getAll() { final ArrayList result = new ArrayList<>(); for (Object value : arrayValue) { - this.indexUsage.arrayContainsTag = true; result.add( BuiltCondition.of( BuiltCondition.LHS.column(DATA_CONTAINS), From f44a74e7df2c431c4f502ccd428f8f4db082599e Mon Sep 17 00:00:00 2001 From: Stefano Lottini Date: Thu, 4 Jul 2024 00:43:50 +0200 Subject: [PATCH 3/6] Fix base64 encoding for 'cassandra' in docstring (#1235) --- src/main/java/io/stargate/sgv2/jsonapi/StargateJsonApi.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/StargateJsonApi.java b/src/main/java/io/stargate/sgv2/jsonapi/StargateJsonApi.java index a5f4e06f41..e24a694295 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/StargateJsonApi.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/StargateJsonApi.java @@ -44,7 +44,7 @@ `Cassandra:Base64(username):Base64(password)`. For example, assuming a username of `cassandra` and password of `cassandra` the `Token` header would be set to - `Cassandra:Y2Fzc2FuZHJhCg==:Y2Fzc2FuZHJhCg==`. + `Cassandra:Y2Fzc2FuZHJh:Y2Fzc2FuZHJh`. """), }, From c74a1e2f0d36a6715ea24b311234dcd293dd797c Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 3 Jul 2024 16:12:58 -0700 Subject: [PATCH 4/6] Fix #1232: unify/rename 2 different ErrorCodes for similar thing (#1233) --- .../CommandObjectMapperHandler.java | 7 +++---- .../sgv2/jsonapi/exception/ErrorCode.java | 14 ++++++++++---- .../resolver/CommandResolverService.java | 8 +++----- .../service/shredding/model/MD5Hasher.java | 4 ++-- .../ObjectMapperConfigurationTest.java | 12 ++++++++---- .../v1/CollectionResourceIntegrationTest.java | 5 +++-- .../api/v1/GeneralResourceIntegrationTest.java | 5 +++-- .../v1/NamespaceResourceIntegrationTest.java | 5 +++-- .../jsonapi/exception/JsonApiExceptionTest.java | 17 ++++++++--------- 9 files changed, 43 insertions(+), 34 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/configuration/CommandObjectMapperHandler.java b/src/main/java/io/stargate/sgv2/jsonapi/api/configuration/CommandObjectMapperHandler.java index f1e8a799c6..aee15e1065 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/configuration/CommandObjectMapperHandler.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/configuration/CommandObjectMapperHandler.java @@ -61,11 +61,10 @@ public JavaType handleUnknownTypeId( // CollectionCommand // interface io.stargate.sgv2.jsonapi.api.model.command.GeneralCommand -> // GeneralCommand - final String rawCommandClassString = baseType.getRawClass().toString(); + final String rawCommandClassString = baseType.getRawClass().getName(); final String baseCommand = rawCommandClassString.substring(rawCommandClassString.lastIndexOf('.') + 1); - throw new JsonApiException( - ErrorCode.NO_COMMAND_MATCHED, - String.format("No \"%s\" command found as \"%s\"", subTypeId, baseCommand)); + throw ErrorCode.COMMAND_UNKNOWN.toApiException( + "\"%s\" not one of \"%s\"s", subTypeId, baseCommand); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorCode.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorCode.java index cf31e1cf69..4f4a914b98 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorCode.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorCode.java @@ -4,9 +4,8 @@ public enum ErrorCode { /** Command error codes. */ COUNT_READ_FAILED("Unable to count documents"), - COMMAND_NOT_IMPLEMENTED("The provided command is not implemented."), + COMMAND_UNKNOWN("Provided command unknown"), INVALID_CREATE_COLLECTION_OPTIONS("The provided options are invalid"), - NO_COMMAND_MATCHED("Unable to find the provided command"), COMMAND_ACCEPTS_NO_OPTIONS("Command accepts no options"), /** @@ -69,8 +68,6 @@ public enum ErrorCode { SHRED_INTERNAL_NO_PATH("Internal: path being built does not point to a property or element"), - SHRED_NO_MD5("MD5 Hash algorithm not available"), - SHRED_UNRECOGNIZED_NODE_TYPE("Unrecognized JSON node type in input document"), SHRED_DOC_LIMIT_VIOLATION("Document size limitation violated"), @@ -172,6 +169,11 @@ public enum ErrorCode { SERVER_DRIVER_FAILURE("Driver failed"), /** Driver timeout failure. */ SERVER_DRIVER_TIMEOUT("Driver timeout"), + /** + * Error code used for "should never happen" style problems. Suffix part needs to include details + * of actual issue. + */ + SERVER_INTERNAL_ERROR("Server internal error"), SERVER_NO_NODE_AVAILABLE("No node was available to execute the query"), SERVER_QUERY_CONSISTENCY_FAILURE("Database query consistency failed"), SERVER_QUERY_EXECUTION_FAILURE("Database query execution failed"), @@ -196,6 +198,10 @@ public JsonApiException toApiException(String format, Object... args) { return new JsonApiException(this, message + ": " + String.format(format, args)); } + public JsonApiException toApiException(Throwable cause, String format, Object... args) { + return new JsonApiException(this, message + ": " + String.format(format, args), cause); + } + public JsonApiException toApiException() { return new JsonApiException(this, message); } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverService.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverService.java index 19075ec77f..9d0f7fdc7c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverService.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/CommandResolverService.java @@ -3,7 +3,6 @@ import io.smallrye.mutiny.Uni; import io.stargate.sgv2.jsonapi.api.model.command.Command; import io.stargate.sgv2.jsonapi.exception.ErrorCode; -import io.stargate.sgv2.jsonapi.exception.JsonApiException; import io.stargate.sgv2.jsonapi.service.resolver.model.CommandResolver; import jakarta.enterprise.context.ApplicationScoped; import jakarta.enterprise.inject.Instance; @@ -45,10 +44,9 @@ public Uni> resolverForCommand(T command) .ifNull() .failWith( () -> { - String msg = - "The command %s is not implemented." - .formatted(command.getClass().getSimpleName()); - return new JsonApiException(ErrorCode.COMMAND_NOT_IMPLEMENTED, msg); + // Should never happen: all Commands should have matching resolvers + return ErrorCode.SERVER_INTERNAL_ERROR.toApiException( + "No `CommandResolver` for Command \"%s\"", command.getClass().getName()); }); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/MD5Hasher.java b/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/MD5Hasher.java index a47839b7b2..0e3a626c73 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/MD5Hasher.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/MD5Hasher.java @@ -1,7 +1,6 @@ package io.stargate.sgv2.jsonapi.service.shredding.model; import io.stargate.sgv2.jsonapi.exception.ErrorCode; -import io.stargate.sgv2.jsonapi.exception.JsonApiException; import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; @@ -43,7 +42,8 @@ private String hashAndEncode(String value) { md = MessageDigest.getInstance("MD5"); } catch (NoSuchAlgorithmException e) { // should never happen but: - throw new JsonApiException(ErrorCode.SHRED_NO_MD5, e); + throw ErrorCode.SERVER_INTERNAL_ERROR.toApiException( + e, "MD5 Hash algorithm not available for Shredding"); } byte[] digest = md.digest(value.getBytes(StandardCharsets.UTF_8)); return BASE64_ENCODER.encodeToString(digest); diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/configuration/ObjectMapperConfigurationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/configuration/ObjectMapperConfigurationTest.java index e0086df9c3..46bada9f67 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/configuration/ObjectMapperConfigurationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/configuration/ObjectMapperConfigurationTest.java @@ -59,7 +59,8 @@ public void notExistedCommandMatchNamespaceCommand() throws Exception { Exception e = catchException(() -> objectMapper.readValue(json, NamespaceCommand.class)); assertThat(e) .isInstanceOf(JsonApiException.class) - .hasMessageStartingWith("No \"notExistedCommand\" command found as \"NamespaceCommand\""); + .hasMessageStartingWith( + "Provided command unknown: \"notExistedCommand\" not one of \"NamespaceCommand\"s"); } @Test @@ -74,7 +75,8 @@ public void collectionCommandNotMatchNamespaceCommand() throws Exception { Exception e = catchException(() -> objectMapper.readValue(json, NamespaceCommand.class)); assertThat(e) .isInstanceOf(JsonApiException.class) - .hasMessageStartingWith("No \"find\" command found as \"NamespaceCommand\""); + .hasMessageStartingWith( + "Provided command unknown: \"find\" not one of \"NamespaceCommand\"s"); } @Test @@ -89,7 +91,8 @@ public void collectionCommandNotMatchGeneralCommand() throws Exception { Exception e = catchException(() -> objectMapper.readValue(json, GeneralCommand.class)); assertThat(e) .isInstanceOf(JsonApiException.class) - .hasMessageStartingWith("No \"insertOne\" command found as \"GeneralCommand\""); + .hasMessageStartingWith( + "Provided command unknown: \"insertOne\" not one of \"GeneralCommand\"s"); } @Test @@ -104,7 +107,8 @@ public void generalCommandNotMatchCollectionCommand() throws Exception { Exception e = catchException(() -> objectMapper.readValue(json, CollectionCommand.class)); assertThat(e) .isInstanceOf(JsonApiException.class) - .hasMessageStartingWith("No \"createNamespace\" command found as \"CollectionCommand\""); + .hasMessageStartingWith( + "Provided command unknown: \"createNamespace\" not one of \"CollectionCommand\"s"); } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResourceIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResourceIntegrationTest.java index ae4993b3cb..a035f4e56a 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResourceIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResourceIntegrationTest.java @@ -75,11 +75,12 @@ public void unknownCommand() { .then() .statusCode(200) .body("errors", hasSize(1)) - .body("errors[0].errorCode", is("NO_COMMAND_MATCHED")) + .body("errors[0].errorCode", is("COMMAND_UNKNOWN")) .body("errors[0].exceptionClass", is("JsonApiException")) .body( "errors[0].message", - startsWith("No \"unknownCommand\" command found as \"CollectionCommand\"")); + startsWith( + "Provided command unknown: \"unknownCommand\" not one of \"CollectionCommand\"s")); } @Test diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResourceIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResourceIntegrationTest.java index 759103d6c6..5ced656629 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResourceIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResourceIntegrationTest.java @@ -75,8 +75,9 @@ public void unknownCommand() { .statusCode(200) .body( "errors[0].message", - startsWith("No \"unknownCommand\" command found as \"GeneralCommand\"")) - .body("errors[0].errorCode", is("NO_COMMAND_MATCHED")); + startsWith( + "Provided command unknown: \"unknownCommand\" not one of \"GeneralCommand\"s")) + .body("errors[0].errorCode", is("COMMAND_UNKNOWN")); } @Test diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResourceIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResourceIntegrationTest.java index b887c598e7..59b214f2ce 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResourceIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResourceIntegrationTest.java @@ -66,10 +66,11 @@ public void unknownCommand() { .post(NamespaceResource.BASE_PATH, namespaceName) .then() .statusCode(200) - .body("errors[0].errorCode", is("NO_COMMAND_MATCHED")) + .body("errors[0].errorCode", is("COMMAND_UNKNOWN")) .body( "errors[0].message", - startsWith("No \"unknownCommand\" command found as \"NamespaceCommand\"")); + startsWith( + "Provided command unknown: \"unknownCommand\" not one of \"NamespaceCommand\"s")); } @Test diff --git a/src/test/java/io/stargate/sgv2/jsonapi/exception/JsonApiExceptionTest.java b/src/test/java/io/stargate/sgv2/jsonapi/exception/JsonApiExceptionTest.java index 5509085195..c8e9ed541d 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/exception/JsonApiExceptionTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/exception/JsonApiExceptionTest.java @@ -18,7 +18,7 @@ class Get { @Test public void happyPath() { - JsonApiException ex = new JsonApiException(ErrorCode.COMMAND_NOT_IMPLEMENTED); + JsonApiException ex = new JsonApiException(ErrorCode.COMMAND_UNKNOWN); CommandResult result = ex.get(); @@ -28,10 +28,10 @@ public void happyPath() { .singleElement() .satisfies( error -> { - assertThat(error.message()).isEqualTo("The provided command is not implemented."); + assertThat(error.message()).isEqualTo("Provided command unknown"); assertThat(error.fields()) .hasSize(2) - .containsEntry("errorCode", "COMMAND_NOT_IMPLEMENTED") + .containsEntry("errorCode", "COMMAND_UNKNOWN") .containsEntry("exceptionClass", "JsonApiException"); }); } @@ -39,8 +39,7 @@ public void happyPath() { @Test public void withCustomMessage() { JsonApiException ex = - new JsonApiException( - ErrorCode.COMMAND_NOT_IMPLEMENTED, "Custom message is more important."); + new JsonApiException(ErrorCode.COMMAND_UNKNOWN, "Custom message is more important."); CommandResult result = ex.get(); @@ -53,7 +52,7 @@ public void withCustomMessage() { assertThat(error.message()).isEqualTo("Custom message is more important."); assertThat(error.fields()) .hasSize(2) - .containsEntry("errorCode", "COMMAND_NOT_IMPLEMENTED") + .containsEntry("errorCode", "COMMAND_UNKNOWN") .containsEntry("exceptionClass", "JsonApiException"); }); } @@ -61,7 +60,7 @@ public void withCustomMessage() { @Test public void withCause() { Exception cause = new IllegalArgumentException("Cause message is important"); - JsonApiException ex = new JsonApiException(ErrorCode.COMMAND_NOT_IMPLEMENTED, cause); + JsonApiException ex = new JsonApiException(ErrorCode.COMMAND_UNKNOWN, cause); CommandResult result = ex.get(); @@ -71,10 +70,10 @@ public void withCause() { .hasSize(2) .anySatisfy( error -> { - assertThat(error.message()).isEqualTo("The provided command is not implemented."); + assertThat(error.message()).isEqualTo("Provided command unknown"); assertThat(error.fields()) .hasSize(2) - .containsEntry("errorCode", "COMMAND_NOT_IMPLEMENTED") + .containsEntry("errorCode", "COMMAND_UNKNOWN") .containsEntry("exceptionClass", "JsonApiException"); }) .anySatisfy( From 44655865d054d1e29c3fdd42d2ee69d05a6ae812 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Wed, 3 Jul 2024 17:19:08 -0700 Subject: [PATCH 5/6] Fixes #1236: add valid Command names in fail message (#1237) --- .../jsonapi/api/configuration/CommandObjectMapperHandler.java | 3 ++- .../jsonapi/api/v1/CollectionResourceIntegrationTest.java | 2 +- .../sgv2/jsonapi/api/v1/GeneralResourceIntegrationTest.java | 4 ++-- .../sgv2/jsonapi/api/v1/NamespaceResourceIntegrationTest.java | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/configuration/CommandObjectMapperHandler.java b/src/main/java/io/stargate/sgv2/jsonapi/api/configuration/CommandObjectMapperHandler.java index aee15e1065..b5ed17c454 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/configuration/CommandObjectMapperHandler.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/configuration/CommandObjectMapperHandler.java @@ -65,6 +65,7 @@ public JavaType handleUnknownTypeId( final String baseCommand = rawCommandClassString.substring(rawCommandClassString.lastIndexOf('.') + 1); throw ErrorCode.COMMAND_UNKNOWN.toApiException( - "\"%s\" not one of \"%s\"s", subTypeId, baseCommand); + "\"%s\" not one of \"%s\"s: known commands are %s", + subTypeId, baseCommand, idResolver.getDescForKnownTypeIds()); } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResourceIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResourceIntegrationTest.java index a035f4e56a..158b8b765d 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResourceIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResourceIntegrationTest.java @@ -80,7 +80,7 @@ public void unknownCommand() { .body( "errors[0].message", startsWith( - "Provided command unknown: \"unknownCommand\" not one of \"CollectionCommand\"s")); + "Provided command unknown: \"unknownCommand\" not one of \"CollectionCommand\"s: known commands are [countDocuments,")); } @Test diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResourceIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResourceIntegrationTest.java index 5ced656629..3f572643d5 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResourceIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/GeneralResourceIntegrationTest.java @@ -73,11 +73,11 @@ public void unknownCommand() { .post(GeneralResource.BASE_PATH) .then() .statusCode(200) + .body("errors[0].errorCode", is("COMMAND_UNKNOWN")) .body( "errors[0].message", startsWith( - "Provided command unknown: \"unknownCommand\" not one of \"GeneralCommand\"s")) - .body("errors[0].errorCode", is("COMMAND_UNKNOWN")); + "Provided command unknown: \"unknownCommand\" not one of \"GeneralCommand\"s: known commands are [createNamespace,")); } @Test diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResourceIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResourceIntegrationTest.java index 59b214f2ce..2310ee7a85 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResourceIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/NamespaceResourceIntegrationTest.java @@ -70,7 +70,7 @@ public void unknownCommand() { .body( "errors[0].message", startsWith( - "Provided command unknown: \"unknownCommand\" not one of \"NamespaceCommand\"s")); + "Provided command unknown: \"unknownCommand\" not one of \"NamespaceCommand\"s: known commands are [createCollection")); } @Test From 96b283972d6b0eae989a7b1dcc3b685c2be3a381 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 8 Jul 2024 11:31:52 -0700 Subject: [PATCH 6/6] Fixes #1238: convert internal-only `ErrorCode`s into `ErrorCode.SERVER_INTERNAL_ERROR` (#1239) --- .../api/model/command/CommandResult.java | 6 +- .../api/model/command/NoOptionsCommand.java | 6 +- .../clause/filter/ComparisonExpression.java | 6 +- .../command/clause/update/UpdateOperator.java | 6 +- .../UpdateClauseDeserializer.java | 10 +- .../sgv2/jsonapi/exception/ErrorCode.java | 17 +--- .../ConstraintViolationExceptionMapper.java | 3 +- .../mappers/GenericExceptionMapper.java | 2 +- .../jsonapi/service/cql/ExpressionUtils.java | 7 +- .../service/cql/builder/QueryBuilder.java | 3 +- .../operation/model/ReadOperation.java | 13 +-- .../operation/model/impl/FindOperation.java | 5 +- .../service/projection/DocumentProjector.java | 92 ++++++------------- .../service/projection/ProjectionLayer.java | 11 +-- .../model/impl/matcher/FilterMatchRules.java | 4 +- .../jsonapi/service/shredding/JsonPath.java | 9 +- .../jsonapi/service/shredding/Shredder.java | 3 +- .../shredding/model/DocValueHasher.java | 9 +- .../ObjectMapperConfigurationTest.java | 4 +- .../api/v1/DeleteOneIntegrationTest.java | 4 +- .../jsonapi/api/v1/InsertIntegrationTest.java | 4 +- .../service/updater/DocumentUpdaterTest.java | 6 +- 22 files changed, 79 insertions(+), 151 deletions(-) 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 b546d192bc..f68947f409 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 @@ -194,11 +194,9 @@ public record Error( * @return */ public RestResponse map() { - if (null != this.errors() && !this.errors().isEmpty()) { + if (null != this.errors()) { final Optional first = - this.errors().stream() - .filter(error -> error.status().getStatusCode() != Response.Status.OK.getStatusCode()) - .findFirst(); + this.errors().stream().filter(error -> error.status() != Response.Status.OK).findFirst(); if (first.isPresent()) { return RestResponse.ResponseBuilder.create(first.get().status(), this).build(); } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/NoOptionsCommand.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/NoOptionsCommand.java index f3aee396b9..63691cd45a 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/NoOptionsCommand.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/NoOptionsCommand.java @@ -20,10 +20,6 @@ default void setOptions(JsonNode value) throws JsonApiException { if (value.isNull() || (value.isObject() && value.isEmpty())) { return; } - final String msg = - String.format( - "%s: %s", - ErrorCode.COMMAND_ACCEPTS_NO_OPTIONS.getMessage(), getClass().getSimpleName()); - throw new JsonApiException(ErrorCode.COMMAND_ACCEPTS_NO_OPTIONS, msg); + throw ErrorCode.COMMAND_ACCEPTS_NO_OPTIONS.toApiException("`%s`", getClass().getSimpleName()); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ComparisonExpression.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ComparisonExpression.java index 7ee9fad133..c8000a285d 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ComparisonExpression.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/filter/ComparisonExpression.java @@ -1,7 +1,6 @@ package io.stargate.sgv2.jsonapi.api.model.command.clause.filter; import io.stargate.sgv2.jsonapi.exception.ErrorCode; -import io.stargate.sgv2.jsonapi.exception.JsonApiException; import io.stargate.sgv2.jsonapi.service.operation.model.impl.DBFilterBase; import io.stargate.sgv2.jsonapi.service.shredding.model.DocumentId; import jakarta.validation.Valid; @@ -139,9 +138,8 @@ private static JsonLiteral getLiteral(Object value) { if (value instanceof UUID || value instanceof ObjectId) { return new JsonLiteral<>(value.toString(), JsonType.STRING); } - throw new JsonApiException( - ErrorCode.FILTER_UNRESOLVABLE, - String.format("Unsupported filter value type %s", value.getClass())); + throw ErrorCode.SERVER_INTERNAL_ERROR.toApiException( + "Unsupported filter value type `%s`", value.getClass().getName()); } public List> match( diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperator.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperator.java index 3c67ef0cc0..5d38f7a3c6 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperator.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/clause/update/UpdateOperator.java @@ -2,7 +2,6 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import io.stargate.sgv2.jsonapi.exception.ErrorCode; -import io.stargate.sgv2.jsonapi.exception.JsonApiException; import java.util.HashMap; import java.util.Map; @@ -106,9 +105,8 @@ public String operator() { } public UpdateOperation resolveOperation(ObjectNode arguments) { - throw new JsonApiException( - ErrorCode.UNSUPPORTED_UPDATE_OPERATION, - "Unsupported update operator '%s'".formatted(operator)); + throw ErrorCode.UNSUPPORTED_UPDATE_OPERATION.toApiException( + "Unsupported update operator '%s'", operator); } private static Map operatorMap = new HashMap<>(); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/UpdateClauseDeserializer.java b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/UpdateClauseDeserializer.java index a465fc0099..d6c8e949d2 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/UpdateClauseDeserializer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/api/model/command/deserializers/UpdateClauseDeserializer.java @@ -38,15 +38,13 @@ public UpdateClause deserialize( Map.Entry entry = fieldIter.next(); final String operName = entry.getKey(); if (!operName.startsWith("$")) { - throw new JsonApiException( - ErrorCode.UNSUPPORTED_UPDATE_OPERATION, - "Invalid update operator '%s' (must start with '$')".formatted(operName)); + throw ErrorCode.UNSUPPORTED_UPDATE_OPERATION.toApiException( + "Invalid update operator '%s' (must start with '$')", operName); } UpdateOperator oper = UpdateOperator.getUpdateOperator(operName); if (oper == null) { - throw new JsonApiException( - ErrorCode.UNSUPPORTED_UPDATE_OPERATION, - "Unrecognized update operator '%s'".formatted(operName)); + throw ErrorCode.UNSUPPORTED_UPDATE_OPERATION.toApiException( + "Unrecognized update operator '%s'", operName); } JsonNode operationArg = entry.getValue(); if (!operationArg.isObject()) { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorCode.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorCode.java index 4f4a914b98..0dbc1daaba 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorCode.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorCode.java @@ -32,8 +32,6 @@ public enum ErrorCode { EMBEDDING_PROVIDER_TIMEOUT("The Embedding Provider timed out"), EMBEDDING_PROVIDER_UNEXPECTED_RESPONSE("The Embedding Provider returned an unexpected response"), - FILTER_UNRESOLVABLE("Unable to resolve the filter"), - FILTER_MULTIPLE_ID_FILTER( "Cannot have more than one _id equals filter clause: use $in operator instead"), @@ -66,14 +64,6 @@ public enum ErrorCode { SHRED_BAD_DOCID_EMPTY_STRING("Bad value for '_id' property: empty String not allowed"), - SHRED_INTERNAL_NO_PATH("Internal: path being built does not point to a property or element"), - - SHRED_UNRECOGNIZED_NODE_TYPE("Unrecognized JSON node type in input document"), - - SHRED_DOC_LIMIT_VIOLATION("Document size limitation violated"), - - SHRED_DOC_KEY_NAME_VIOLATION("Document field name invalid"), - SHRED_BAD_EJSON_VALUE("Bad JSON Extension value"), SHRED_BAD_VECTOR_SIZE("$vector value can't be empty"), @@ -81,6 +71,9 @@ public enum ErrorCode { SHRED_BAD_VECTOR_VALUE("$vector value needs to be array of numbers"), SHRED_BAD_VECTORIZE_VALUE("$vectorize search clause needs to be non-blank text value"), + SHRED_DOC_KEY_NAME_VIOLATION("Document field name invalid"), + SHRED_DOC_LIMIT_VIOLATION("Document size limitation violated"), + EXISTING_COLLECTION_DIFFERENT_SETTINGS("Collection already exists"), INVALID_VECTORIZE_VALUE_TYPE("$vectorize value needs to be text value"), @@ -105,8 +98,6 @@ public enum ErrorCode { INVALID_USAGE_OF_VECTORIZE("`$vectorize` and `$vector` can't be used together"), - UNSUPPORTED_OPERATION("Unsupported operation class"), - UNSUPPORTED_PROJECTION_PARAM("Unsupported projection parameter"), UNSUPPORTED_UPDATE_DATA_TYPE("Unsupported update data type"), @@ -157,9 +148,7 @@ public enum ErrorCode { INVALID_ID_TYPE("Invalid Id type"), INVALID_QUERY("Invalid query"), NO_INDEX_ERROR("Faulty collection (missing indexes). Recommend re-creating the collection"), - UNSUPPORTED_CQL_QUERY_TYPE("Unsupported cql query type"), MISSING_VECTOR_VALUE("Missing the vector value when building cql"), - INVALID_LOGIC_OPERATOR("Invalid logical operator"), // Driver failure codes /** Error codes related to driver exceptions. */ 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 d649a9fa4c..63091f59e5 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 @@ -39,8 +39,7 @@ public RestResponse constraintViolationException( violations.stream().map(ConstraintViolationExceptionMapper::getError).distinct().toList(); // return result - CommandResult commandResult = new CommandResult(errors); - return RestResponse.ok(commandResult); + return new CommandResult(errors).map(); } private static CommandResult.Error getError(ConstraintViolation violation) { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/GenericExceptionMapper.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/GenericExceptionMapper.java index f6f47cff07..250e3de908 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/GenericExceptionMapper.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/GenericExceptionMapper.java @@ -15,6 +15,6 @@ public class GenericExceptionMapper { @ServerExceptionMapper({Exception.class, MismatchedInputException.class}) public RestResponse genericExceptionMapper(Throwable e) { CommandResult commandResult = new ThrowableCommandResultSupplier(e).get(); - return RestResponse.ok(commandResult); + return commandResult.map(); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/cql/ExpressionUtils.java b/src/main/java/io/stargate/sgv2/jsonapi/service/cql/ExpressionUtils.java index f811ba50c5..e19b720f8f 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/cql/ExpressionUtils.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/cql/ExpressionUtils.java @@ -15,11 +15,10 @@ */ package io.stargate.sgv2.jsonapi.service.cql; -import static io.stargate.sgv2.jsonapi.exception.ErrorCode.INVALID_LOGIC_OPERATOR; - import com.bpodgursky.jbool_expressions.And; import com.bpodgursky.jbool_expressions.Expression; import com.bpodgursky.jbool_expressions.Or; +import io.stargate.sgv2.jsonapi.exception.ErrorCode; import java.util.List; /** @@ -61,7 +60,9 @@ public static Expression buildExpression( case "$or" -> { return orOf(expressions); } - default -> throw INVALID_LOGIC_OPERATOR.toApiException(); + default -> + throw ErrorCode.SERVER_INTERNAL_ERROR.toApiException( + "Invalid logical operator '%s'", logicOperator); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/cql/builder/QueryBuilder.java b/src/main/java/io/stargate/sgv2/jsonapi/service/cql/builder/QueryBuilder.java index 93df2a28f6..a798e9a808 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/cql/builder/QueryBuilder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/cql/builder/QueryBuilder.java @@ -114,7 +114,8 @@ public Query build() { if (isSelect) { return selectQuery(); } - throw ErrorCode.UNSUPPORTED_CQL_QUERY_TYPE.toApiException(); + throw ErrorCode.SERVER_INTERNAL_ERROR.toApiException( + "Unsupported cql query type in QueryBuilder (isSelect=false)"); } private Query selectQuery() { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/ReadOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/ReadOperation.java index 72f2373262..9365e703a2 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/ReadOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/ReadOperation.java @@ -4,7 +4,7 @@ import com.datastax.oss.driver.api.core.cql.Row; import com.datastax.oss.driver.api.core.cql.SimpleStatement; import com.datastax.oss.driver.api.core.data.TupleValue; -import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.JacksonException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.JsonNodeFactory; @@ -129,7 +129,7 @@ default Uni findDocument( getDocumentId(row.getTupleValue(0)), // key row.getUuid(1), // tx_id root); - } catch (JsonProcessingException e) { + } catch (JacksonException e) { throw parsingExceptionToApiException(e); } documents.add(document); @@ -496,7 +496,7 @@ record DocJsonValue(ObjectMapper objectMapper, String docJsonValue) public JsonNode get() { try { return objectMapper.readTree(docJsonValue); - } catch (JsonProcessingException e) { + } catch (JacksonException e) { // These are data stored in the DB so the error should never happen throw parsingExceptionToApiException(e); } @@ -506,10 +506,7 @@ public JsonNode get() { /** * Helper method to handle details of exactly how much information to include in error message. */ - static JsonApiException parsingExceptionToApiException(JsonProcessingException e) { - return new JsonApiException( - ErrorCode.DOCUMENT_UNPARSEABLE, - String.format( - "%s: %s", ErrorCode.DOCUMENT_UNPARSEABLE.getMessage(), e.getOriginalMessage())); + static JsonApiException parsingExceptionToApiException(JacksonException e) { + return ErrorCode.DOCUMENT_UNPARSEABLE.toApiException("%s", e.getOriginalMessage()); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperation.java b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperation.java index e63c3c4217..eae4075039 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperation.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperation.java @@ -381,9 +381,8 @@ public Uni getDocuments( } default -> { JsonApiException failure = - new JsonApiException( - ErrorCode.UNSUPPORTED_OPERATION, - "Unsupported find operation read type " + readType); + ErrorCode.SERVER_INTERNAL_ERROR.toApiException( + "Unsupported find operation read type `%s`", readType); return Uni.createFrom().failure(failure); } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java index 4a7e4f1de8..b3baeff704 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/DocumentProjector.java @@ -5,7 +5,6 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import io.stargate.sgv2.jsonapi.config.constants.DocumentConstants; import io.stargate.sgv2.jsonapi.exception.ErrorCode; -import io.stargate.sgv2.jsonapi.exception.JsonApiException; import java.math.BigDecimal; import java.util.ArrayList; import java.util.List; @@ -84,11 +83,8 @@ public static DocumentProjector createFromDefinition( return defaultProjector(); } if (!projectionDefinition.isObject()) { - throw new JsonApiException( - ErrorCode.UNSUPPORTED_PROJECTION_PARAM, - ErrorCode.UNSUPPORTED_PROJECTION_PARAM.getMessage() - + ": definition must be OBJECT, was " - + projectionDefinition.getNodeType()); + throw ErrorCode.UNSUPPORTED_PROJECTION_PARAM.toApiException( + "definition must be OBJECT, was %s", projectionDefinition.getNodeType()); } // Special cases: "star-include/exclude" if (projectionDefinition.size() == 1) { @@ -113,13 +109,8 @@ private static boolean extractIncludeOrExclude(String path, JsonNode value) { return value.booleanValue(); } // Unknown JSON node type; error - throw new JsonApiException( - ErrorCode.UNSUPPORTED_PROJECTION_PARAM, - ErrorCode.UNSUPPORTED_PROJECTION_PARAM.getMessage() - + ": path ('" - + path - + "') value must be NUMBER or BOOLEAN, was " - + value.getNodeType()); + throw ErrorCode.UNSUPPORTED_PROJECTION_PARAM.toApiException( + "path ('%s') value must be NUMBER or BOOLEAN, was %s", path, value.getNodeType()); } public boolean isInclusion() { @@ -271,30 +262,23 @@ PathCollector collectFromObject(JsonNode ob, String parentPath) { String path = entry.getKey(); if (path.isEmpty()) { - throw new JsonApiException( - ErrorCode.UNSUPPORTED_PROJECTION_PARAM, - ErrorCode.UNSUPPORTED_PROJECTION_PARAM.getMessage() - + ": empty paths (and path segments) not allowed"); + throw ErrorCode.UNSUPPORTED_PROJECTION_PARAM.toApiException( + "empty paths (and path segments) not allowed"); } if (path.charAt(0) == '$' && !(path.equals(DocumentConstants.Fields.VECTOR_EMBEDDING_FIELD) || DocumentConstants.Fields.VECTOR_EMBEDDING_TEXT_FIELD.equals(path))) { // First: no operators allowed at root level if (parentPath == null) { - throw new JsonApiException( - ErrorCode.UNSUPPORTED_PROJECTION_PARAM, - ErrorCode.UNSUPPORTED_PROJECTION_PARAM.getMessage() - + ": '$vector'/'$vectorize' are the only allowed paths that can start with '$'"); + throw ErrorCode.UNSUPPORTED_PROJECTION_PARAM.toApiException( + "'$vector'/'$vectorize' are the only allowed paths that can start with '$'"); } // Second: we only support one operator for now if (!"$slice".equals(path)) { - throw new JsonApiException( - ErrorCode.UNSUPPORTED_PROJECTION_PARAM, - ErrorCode.UNSUPPORTED_PROJECTION_PARAM.getMessage() - + ": unrecognized/unsupported projection operator '" - + path - + "' (only '$slice' supported)"); + throw ErrorCode.UNSUPPORTED_PROJECTION_PARAM.toApiException( + "unrecognized/unsupported projection operator '%s' (only '$slice' supported)", + path); } addSlice(parentPath, entry.getValue()); @@ -303,10 +287,8 @@ PathCollector collectFromObject(JsonNode ob, String parentPath) { // Special rule for "*": only allowed as single root-level entry; if ("*".equals(path)) { - throw new JsonApiException( - ErrorCode.UNSUPPORTED_PROJECTION_PARAM, - ErrorCode.UNSUPPORTED_PROJECTION_PARAM.getMessage() - + ": wildcard ('*') only allowed as the only root-level path"); + throw ErrorCode.UNSUPPORTED_PROJECTION_PARAM.toApiException( + "wildcard ('*') only allowed as the only root-level path"); } if (parentPath != null) { path = parentPath + "." + path; @@ -330,13 +312,9 @@ PathCollector collectFromObject(JsonNode ob, String parentPath) { collectFromObject(value, path); } else { // Unknown JSON node type; error - throw new JsonApiException( - ErrorCode.UNSUPPORTED_PROJECTION_PARAM, - ErrorCode.UNSUPPORTED_PROJECTION_PARAM.getMessage() - + ": path ('" - + path - + "') value must be NUMBER, BOOLEAN or OBJECT, was " - + value.getNodeType()); + throw ErrorCode.UNSUPPORTED_PROJECTION_PARAM.toApiException( + "path ('%s') value must be NUMBER, BOOLEAN or OBJECT, was %s", + path, value.getNodeType()); } } return this; @@ -350,14 +328,9 @@ private void addSlice(String path, JsonNode sliceDef) { int skip = sliceDef.get(0).intValue(); int count = sliceDef.get(1).intValue(); if (count < 0) { // negative values not allowed - throw new JsonApiException( - ErrorCode.UNSUPPORTED_PROJECTION_PARAM, - ErrorCode.UNSUPPORTED_PROJECTION_PARAM.getMessage() - + ": path ('" - + path - + "') has unsupported parameter for '$slice' (" - + sliceDef.getNodeType() - + "): second NUMBER (entries to return) MUST be positive"); + throw ErrorCode.UNSUPPORTED_PROJECTION_PARAM.toApiException( + "path ('%s') has unsupported parameter for '$slice' (%s): second NUMBER (entries to return) MUST be positive", + path, sliceDef.getNodeType()); } slices.add( new ProjectionLayer.SliceDef(path, ProjectionLayer.constructSlicer(skip, count))); @@ -368,14 +341,9 @@ private void addSlice(String path, JsonNode sliceDef) { slices.add(new ProjectionLayer.SliceDef(path, ProjectionLayer.constructSlicer(count))); return; } - throw new JsonApiException( - ErrorCode.UNSUPPORTED_PROJECTION_PARAM, - ErrorCode.UNSUPPORTED_PROJECTION_PARAM.getMessage() - + ": path ('" - + path - + "') has unsupported parameter for '$slice' (" - + sliceDef.getNodeType() - + "): only NUMBER or ARRAY with 2 NUMBERs accepted"); + throw ErrorCode.UNSUPPORTED_PROJECTION_PARAM.toApiException( + "path ('%s') has unsupported parameter for '$slice' (%s): only NUMBER or ARRAY with 2 NUMBERs accepted", + path, sliceDef.getNodeType()); } private void addExclusion(String path) { @@ -388,12 +356,8 @@ private void addExclusion(String path) { } else { // Must not mix exclusions and inclusions if (inclusions > 0) { - throw new JsonApiException( - ErrorCode.UNSUPPORTED_PROJECTION_PARAM, - ErrorCode.UNSUPPORTED_PROJECTION_PARAM.getMessage() - + ": cannot exclude '" - + path - + "' on inclusion projection"); + throw ErrorCode.UNSUPPORTED_PROJECTION_PARAM.toApiException( + "cannot exclude '%s' on inclusion projection", path); } ++exclusions; paths.add(path); @@ -410,12 +374,8 @@ private void addInclusion(String path) { } else { // Must not mix exclusions and inclusions if (exclusions > 0) { - throw new JsonApiException( - ErrorCode.UNSUPPORTED_PROJECTION_PARAM, - ErrorCode.UNSUPPORTED_PROJECTION_PARAM.getMessage() - + ": cannot include '" - + path - + "' on exclusion projection"); + throw ErrorCode.UNSUPPORTED_PROJECTION_PARAM.toApiException( + "cannot include '%s' on exclusion projection", path); } ++inclusions; paths.add(path); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/ProjectionLayer.java b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/ProjectionLayer.java index fc62055d90..9fa97131ea 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/projection/ProjectionLayer.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/projection/ProjectionLayer.java @@ -5,7 +5,6 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import io.stargate.sgv2.jsonapi.config.constants.DocumentConstants; import io.stargate.sgv2.jsonapi.exception.ErrorCode; -import io.stargate.sgv2.jsonapi.exception.JsonApiException; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -308,14 +307,8 @@ public boolean applySlice(JsonNode subtree) { } void reportPathConflict(String fullPath1, String fullPath2) { - throw new JsonApiException( - ErrorCode.UNSUPPORTED_PROJECTION_PARAM, - ErrorCode.UNSUPPORTED_PROJECTION_PARAM.getMessage() - + ": projection path conflict between '" - + fullPath1 - + "' and '" - + fullPath2 - + "'"); + throw ErrorCode.UNSUPPORTED_PROJECTION_PARAM.toApiException( + "projection path conflict between '%s' and '%s'", fullPath1, fullPath2); } @Override diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterMatchRules.java b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterMatchRules.java index a4a0853c3c..9a2352381f 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterMatchRules.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/resolver/model/impl/matcher/FilterMatchRules.java @@ -6,7 +6,6 @@ import io.stargate.sgv2.jsonapi.api.model.command.Filterable; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.LogicalExpression; import io.stargate.sgv2.jsonapi.exception.ErrorCode; -import io.stargate.sgv2.jsonapi.exception.JsonApiException; import io.stargate.sgv2.jsonapi.service.operation.model.Operation; import io.stargate.sgv2.jsonapi.service.operation.model.impl.DBFilterBase; import java.util.ArrayList; @@ -69,8 +68,7 @@ public LogicalExpression apply(CommandContext commandContext, T command) { .findFirst() // unwraps the Optional from the resolver function. .orElseThrow( () -> - new JsonApiException( - ErrorCode.FILTER_UNRESOLVABLE, + ErrorCode.SERVER_INTERNAL_ERROR.toApiException( "Filter type not supported, unable to resolve to a filtering strategy")); } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/JsonPath.java b/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/JsonPath.java index a31a3c7ce3..2d2203582c 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/JsonPath.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/JsonPath.java @@ -2,7 +2,6 @@ import io.stargate.sgv2.jsonapi.config.constants.DocumentConstants; import io.stargate.sgv2.jsonapi.exception.ErrorCode; -import io.stargate.sgv2.jsonapi.exception.JsonApiException; import java.util.Objects; /** @@ -146,7 +145,9 @@ public Builder(String base) { public Builder nestedArrayBuilder() { // Must not be called unless we are pointing to a property or element: if (childPath == null) { - throw new JsonApiException(ErrorCode.SHRED_INTERNAL_NO_PATH); + throw ErrorCode.SERVER_INTERNAL_ERROR.toApiException( + "Shredder path being built does not point to a property or element (basePath: '%s')", + basePath); } return new Builder(childPath, true); } @@ -155,7 +156,9 @@ public Builder nestedArrayBuilder() { public Builder nestedObjectBuilder() { // Must not be called unless we are pointing to a property or element: if (childPath == null) { - throw new JsonApiException(ErrorCode.SHRED_INTERNAL_NO_PATH); + throw ErrorCode.SERVER_INTERNAL_ERROR.toApiException( + "Shredder path being built does not point to a property or element (basePath: '%s')", + basePath); } return new Builder(childPath, false); } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java b/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java index 5c5e970bee..f977c43f16 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java @@ -291,7 +291,8 @@ private void traverseValue(JsonNode value, ShredListener callback, JsonPath.Buil } else if (value.isNull()) { callback.shredNull(path); } else { - throw ErrorCode.SHRED_UNRECOGNIZED_NODE_TYPE.toApiException(value.getNodeType().toString()); + throw ErrorCode.SERVER_INTERNAL_ERROR.toApiException( + "Unsupported `JsonNodeType` in input document, `%s`", value.getNodeType()); } } } diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/DocValueHasher.java b/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/DocValueHasher.java index 96fcec6e0b..8d31216278 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/DocValueHasher.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/DocValueHasher.java @@ -45,12 +45,9 @@ public DocValueHash hash(JsonNode value) { case OBJECT -> objectHash((ObjectNode) value); case STRING -> stringValue(value.textValue()).hash(); - default -> // case BINARY, MISSING, POJO - throw new JsonApiException( - ErrorCode.SHRED_UNRECOGNIZED_NODE_TYPE, - String.format( - "%s: %s", - ErrorCode.SHRED_UNRECOGNIZED_NODE_TYPE.getMessage(), value.getNodeType())); + default -> // case BINARY, MISSING, POJO -- should these ever occur? + throw ErrorCode.SERVER_INTERNAL_ERROR.toApiException( + "Unsupported `JsonNodeType` in input document, `%s`", value.getNodeType()); }; } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/configuration/ObjectMapperConfigurationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/configuration/ObjectMapperConfigurationTest.java index 46bada9f67..fe8759a49c 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/configuration/ObjectMapperConfigurationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/configuration/ObjectMapperConfigurationTest.java @@ -278,7 +278,7 @@ public void failForNonEmptyOptions() throws Exception { assertThat(e) .isInstanceOf(JsonMappingException.class) .hasMessageStartingWith( - ErrorCode.COMMAND_ACCEPTS_NO_OPTIONS.getMessage() + ": InsertOneCommand"); + ErrorCode.COMMAND_ACCEPTS_NO_OPTIONS.getMessage() + ": `InsertOneCommand`"); } @Test @@ -367,7 +367,7 @@ public void failForNonEmptyOptions() throws Exception { assertThat(e) .isInstanceOf(JsonMappingException.class) .hasMessageStartingWith( - ErrorCode.COMMAND_ACCEPTS_NO_OPTIONS.getMessage() + ": DeleteOneCommand"); + ErrorCode.COMMAND_ACCEPTS_NO_OPTIONS.getMessage() + ": `DeleteOneCommand`"); } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/DeleteOneIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/DeleteOneIntegrationTest.java index 41766f0538..933d69ff5e 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/DeleteOneIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/DeleteOneIntegrationTest.java @@ -147,9 +147,9 @@ public void noOptionsAllowed() { .body("status", is(nullValue())) .body("errors", is(notNullValue())) .body("errors", hasSize(1)) - .body("errors[0].message", is("Command accepts no options: DeleteOneCommand")) .body("errors[0].errorCode", is("COMMAND_ACCEPTS_NO_OPTIONS")) - .body("errors[0].exceptionClass", is("JsonApiException")); + .body("errors[0].exceptionClass", is("JsonApiException")) + .body("errors[0].message", is("Command accepts no options: `DeleteOneCommand`")); } @Test diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java index 659eaaf406..6525841dcd 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java @@ -381,9 +381,9 @@ public void noOptionsAllowed() { .body("status", is(nullValue())) .body("errors", is(notNullValue())) .body("errors", hasSize(1)) - .body("errors[0].message", startsWith("Command accepts no options: InsertOneCommand")) .body("errors[0].errorCode", is("COMMAND_ACCEPTS_NO_OPTIONS")) - .body("errors[0].exceptionClass", is("JsonApiException")); + .body("errors[0].exceptionClass", is("JsonApiException")) + .body("errors[0].message", startsWith("Command accepts no options: `InsertOneCommand`")); } @Test diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdaterTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdaterTest.java index f5a8463456..2460bf69c3 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdaterTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/updater/DocumentUpdaterTest.java @@ -256,7 +256,8 @@ public void invalidUpdateOperator() throws Exception { .isNotNull() .isInstanceOf(JsonApiException.class) .hasFieldOrPropertyWithValue("errorCode", ErrorCode.UNSUPPORTED_UPDATE_OPERATION) - .hasMessageStartingWith("Invalid update operator 'location' (must start with '$')"); + .hasMessageStartingWith( + "Unsupported update operation: Invalid update operator 'location' (must start with '$')"); } @Test @@ -274,7 +275,8 @@ public void unsupportedUpdateOperator() throws Exception { .isNotNull() .isInstanceOf(JsonApiException.class) .hasFieldOrPropertyWithValue("errorCode", ErrorCode.UNSUPPORTED_UPDATE_OPERATION) - .hasMessageStartingWith("Unsupported update operator '$pullAll'"); + .hasMessageStartingWith( + "Unsupported update operation: Unsupported update operator '$pullAll'"); } /** Test for ensuring it is not legal to "$set" document id (_id) */