diff --git a/docs/jsonapi-spec.md b/docs/jsonapi-spec.md index 3fe2ff4893..6d35a0996f 100644 --- a/docs/jsonapi-spec.md +++ b/docs/jsonapi-spec.md @@ -532,11 +532,11 @@ See [Multi-Document Failure Considerations](#multi-document-failure-consideratio #### countDocuments Command Response -| Response Elements | Description | -| ----------------- | ------------------------------------------------------- | -| `data` | Not present | -| `status` | Preset with fields: `count: ` | -| `errors` | Present if errors occur. | +| Response Elements | Description | +| ----------------- |----------------------------------------------------------| +| `data` | Not present | +| `status` | Present with fields: `count: ` | +| `errors` | Present if errors occur. | If an error occurs the command will not return `status`. @@ -683,11 +683,11 @@ The maximum amount of documents that can be deleted in a single operation is 20. #### deleteMany Command Response -| Response Elements | Description | -| ----------------- | ------------------------------------------------------------------------------------ | -| `data` | Not present | -| `status` | Preset with fields: `deletedCount: `, `moreData` with value `true` if there are more documents that match the delete `filter`, but were not deleted since the limit of documents to delete in the single operation was hit. | -| `errors` | Present if errors occur. | +| Response Elements | Description | +| ----------------- |--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `data` | Not present | +| `status` | Present with fields: `deletedCount: `, `moreData` with value `true` if there are more documents that match the delete `filter`, but were not deleted since the limit of documents to delete in the single operation was hit. | +| `errors` | Present if errors occur. | If an error occurs the command will still return `status` as some documents may have been deleted. @@ -714,11 +714,11 @@ See [Multi-Document Failure Considerations](#multi-document-failure-consideratio #### deleteOne Command Response -| Response Elements | Description | -| ----------------- | --------------------------------------------------------- | -| `data` | Not present | -| `status` | Preset with fields: `deletedCount: ` | -| `errors` | Present if errors occur. | +| Response Elements | Description | +| ----------------- |------------------------------------------------------------| +| `data` | Not present | +| `status` | Present with fields: `deletedCount: ` | +| `errors` | Present if errors occur. | If an error occurs the command will not return `status`. @@ -905,7 +905,7 @@ If `` is present, the maximum amount of documents that could be sor | Response Elements | Description | |-------------------|----------------------------------------------------------------------------------------------------------------------------------------------------| | `data` | Present with fields : `document` only, see [findOneAndReplace Command Options](#findOneAndReplace-command-options) for controlling the projection. | -| `status` | Preset with fields: `upsertedId: `, if a document was upserted. | +| `status` | Present with fields: `upsertedId: `, if a document was upserted. | | `errors` | Present if errors occur. | If an error occurs the command will not return `data` or `status`. @@ -980,7 +980,7 @@ If `` is present, the maximum amount of documents that could be sor | Response Elements | Description | |-------------------|--------------------------------------------------------------------------------------------------------------------------------------------------| | `data` | Present with fields : `document` only, see [findOneAndUpdate Command Options](#findOneAndUpdate-command-options) for controlling the projection. | -| `status` | Preset with fields: `upsertedId: ` only if a document was upserted. | +| `status` | Present with fields: `upsertedId: ` only if a document was upserted. | | `errors` | Present if errors occur. | 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 5892c17bbd..718c06c02f 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 @@ -3,7 +3,6 @@ import io.stargate.sgv2.jsonapi.api.model.command.CommandResult; import jakarta.validation.ConstraintViolation; import jakarta.validation.ConstraintViolationException; -import jakarta.validation.Path; import jakarta.ws.rs.core.Response; import java.util.List; import java.util.Map; @@ -13,6 +12,7 @@ /** Simple exception mapper for the {@link ConstraintViolationException}. */ public class ConstraintViolationExceptionMapper { + private static final String PREFIX_POST_COMMAND = "postCommand."; // constant error fields public static final Map ERROR_FIELDS = @@ -36,8 +36,18 @@ public RestResponse constraintViolationException( private static CommandResult.Error getError(ConstraintViolation violation) { String message = violation.getMessage(); - Path propertyPath = violation.getPropertyPath(); - String msg = "Request invalid, the field %s not valid: %s.".formatted(propertyPath, message); + String propertyPath = violation.getPropertyPath().toString(); + // Let's remove useless "postCommand." prefix if seen + if (propertyPath.startsWith(PREFIX_POST_COMMAND)) { + propertyPath = propertyPath.substring(PREFIX_POST_COMMAND.length()); + } + + Object propertyValue = violation.getInvalidValue(); + String propertyValueDesc = + (propertyValue == null) ? "`null`" : String.format("\"%s\"", propertyValue); + String msg = + "Request invalid: field '%s' value %s not valid. Problem: %s." + .formatted(propertyPath, propertyValueDesc, message); return new CommandResult.Error(msg, ERROR_FIELDS, ERROR_FIELDS_METRICS_TAG, Response.Status.OK); } } 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 dfcc0c2bfe..7ee8466f4c 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 @@ -47,8 +47,8 @@ public void malformedBody() { .post(CollectionResource.BASE_PATH, namespaceName, collectionName) .then() .statusCode(200) - .body("errors[0].message", is(not(blankString()))) - .body("errors[0].exceptionClass", is("JsonParseException")); + .body("errors[0].exceptionClass", is("JsonParseException")) + .body("errors[0].message", is(not(blankString()))); } @Test @@ -69,10 +69,10 @@ public void unknownCommand() { .post(CollectionResource.BASE_PATH, namespaceName, collectionName) .then() .statusCode(200) + .body("errors[0].errorCode", is("NO_COMMAND_MATCHED")) .body( "errors[0].message", - startsWith("No \"unknownCommand\" command found as \"CollectionCommand\"")) - .body("errors[0].errorCode", is("NO_COMMAND_MATCHED")); + startsWith("No \"unknownCommand\" command found as \"CollectionCommand\"")); } @Test @@ -94,10 +94,11 @@ public void invalidNamespaceName() { .post(CollectionResource.BASE_PATH, "7_no_leading_number", collectionName) .then() .statusCode(200) + .body("errors[0].exceptionClass", is("ConstraintViolationException")) .body( "errors[0].message", - startsWith("Request invalid, the field postCommand.namespace not valid")) - .body("errors[0].exceptionClass", is("ConstraintViolationException")); + startsWith( + "Request invalid: field 'namespace' value \"7_no_leading_number\" not valid. Problem: must match ")); } @Test @@ -119,10 +120,11 @@ public void invalidCollectionName() { .post(CollectionResource.BASE_PATH, namespaceName, "7_no_leading_number") .then() .statusCode(200) + .body("errors[0].exceptionClass", is("ConstraintViolationException")) .body( "errors[0].message", - startsWith("Request invalid, the field postCommand.collection not valid")) - .body("errors[0].exceptionClass", is("ConstraintViolationException")); + startsWith( + "Request invalid: field 'collection' value \"7_no_leading_number\" not valid. Problem: must match ")); } @Test @@ -134,8 +136,8 @@ public void emptyBody() { .post(CollectionResource.BASE_PATH, namespaceName, collectionName) .then() .statusCode(200) - .body("errors[0].message", is(not(blankString()))) - .body("errors[0].exceptionClass", is("ConstraintViolationException")); + .body("errors[0].exceptionClass", is("ConstraintViolationException")) + .body("errors[0].message", is(not(blankString()))); } } } 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 852f3b4ff6..a107ef85e3 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 @@ -44,8 +44,8 @@ public void malformedBody() { .post(NamespaceResource.BASE_PATH, namespaceName) .then() .statusCode(200) - .body("errors[0].message", is(not(blankString()))) - .body("errors[0].exceptionClass", is("JsonParseException")); + .body("errors[0].exceptionClass", is("JsonParseException")) + .body("errors[0].message", is(not(blankString()))); } @Test @@ -66,10 +66,10 @@ public void unknownCommand() { .post(NamespaceResource.BASE_PATH, namespaceName) .then() .statusCode(200) + .body("errors[0].errorCode", is("NO_COMMAND_MATCHED")) .body( "errors[0].message", - startsWith("No \"unknownCommand\" command found as \"NamespaceCommand\"")) - .body("errors[0].errorCode", is("NO_COMMAND_MATCHED")); + startsWith("No \"unknownCommand\" command found as \"NamespaceCommand\"")); } @Test @@ -91,10 +91,11 @@ public void invalidNamespaceName() { .post(NamespaceResource.BASE_PATH, "7_no_leading_number") .then() .statusCode(200) + .body("errors[0].exceptionClass", is("ConstraintViolationException")) .body( "errors[0].message", - startsWith("Request invalid, the field postCommand.namespace not valid")) - .body("errors[0].exceptionClass", is("ConstraintViolationException")); + startsWith( + "Request invalid: field 'namespace' value \"7_no_leading_number\" not valid. Problem: must match ")); } @Test @@ -106,8 +107,8 @@ public void emptyBody() { .post(NamespaceResource.BASE_PATH, namespaceName) .then() .statusCode(200) - .body("errors[0].message", is(not(blankString()))) - .body("errors[0].exceptionClass", is("ConstraintViolationException")); + .body("errors[0].exceptionClass", is("ConstraintViolationException")) + .body("errors[0].message", is(not(blankString()))); } } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/UpdateManyIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/UpdateManyIntegrationTest.java index 3d06e5128d..d853abe817 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/UpdateManyIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/UpdateManyIntegrationTest.java @@ -674,7 +674,7 @@ public void invalidCommand() { .body( "errors[0].message", is( - "Request invalid, the field postCommand.command.updateClause not valid: must not be null.")); + "Request invalid: field 'command.updateClause' value `null` not valid. Problem: must not be null.")); } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/UpdateOneIntegrationTest.java b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/UpdateOneIntegrationTest.java index 2321420b0b..6590aa42ae 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/api/v1/UpdateOneIntegrationTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/api/v1/UpdateOneIntegrationTest.java @@ -2042,7 +2042,7 @@ public void invalidCommand() { .body( "errors[0].message", is( - "Request invalid, the field postCommand.command.updateClause not valid: must not be null.")); + "Request invalid: field 'command.updateClause' value `null` not valid. Problem: must not be null.")); } @Test