Fix #240: surround String-valued DocumentId in single quotes

src/main/java/io/stargate/sgv2/jsonapi/service/bridge/serializer/CustomValueSerializers.java

@@ -64,7 +64,7 @@ public static Map<QueryOuterClass.Value, QueryOuterClass.Value> getDoubleMapValu
   public static List<QueryOuterClass.Value> getDocumentIdValue(DocumentId documentId) {
     // Temporary implementation until we convert it to Tuple in DB
     List<QueryOuterClass.Value> tupleValues =
-        List.of(Values.of(documentId.typeId()), Values.of(documentId.toString()));
+        List.of(Values.of(documentId.typeId()), Values.of(documentId.asDBKey()));
     return tupleValues;
   }
 }

src/main/java/io/stargate/sgv2/jsonapi/service/shredding/model/DocumentId.java

@@ -27,6 +27,7 @@
 public interface DocumentId {
   int typeId();
 
+  /** Method called by JSON serializer to get value to include in JSON output. */
   @JsonValue
   Object value();
 
@@ -36,6 +37,14 @@ default JsonNode asJson(ObjectMapper mapper) {
 
   JsonNode asJson(JsonNodeFactory nodeFactory);
 
+  /**
+   * Accessor used to get canonical String representation of the id to be stored in database. Does
+   * NOT contain type prefix or suffic.
+   *
+   * @return Canonical String representation of the id
+   */
+  String asDBKey();
+
   static DocumentId fromJson(JsonNode node) {
     switch (node.getNodeType()) {
       case BOOLEAN -> {
@@ -127,9 +136,17 @@ public JsonNode asJson(JsonNodeFactory nodeFactory) {
       return nodeFactory.textNode(key);
     }
 
+    @Override
+    public String asDBKey() {
+      return key();
+    }
+
     @Override
     public String toString() {
-      return key;
+      // Enclose in single-quotes to indicate it is String value (not to overlap
+      // with Number, Boolean, null values), indicate start/end
+      // TODO: Consider escaping of quotes within value?
+      return "'" + key + "'";
     }
   }
 
@@ -149,6 +166,11 @@ public JsonNode asJson(JsonNodeFactory nodeFactory) {
       return nodeFactory.numberNode(key);
     }
 
+    @Override
+    public String asDBKey() {
+      return String.valueOf(key);
+    }
+
     @Override
     public String toString() {
       return String.valueOf(key);
@@ -178,6 +200,11 @@ public JsonNode asJson(JsonNodeFactory nodeFactory) {
       return nodeFactory.booleanNode(key);
     }
 
+    @Override
+    public String asDBKey() {
+      return String.valueOf(key);
+    }
+
     @Override
     public String toString() {
       return String.valueOf(key);
@@ -202,6 +229,11 @@ public JsonNode asJson(JsonNodeFactory nodeFactory) {
       return nodeFactory.nullNode();
     }
 
+    @Override
+    public String asDBKey() {
+      return "null";
+    }
+
     @Override
     public String toString() {
       return "null";

src/test/java/io/stargate/sgv2/jsonapi/api/v1/InsertIntegrationTest.java

@@ -206,7 +206,7 @@ public void insertDuplicateDocument() {
           .body(
               "errors[0].message",
               is(
-                  "Failed to insert document with _id duplicate: Document already exists with the given _id"))
+                  "Failed to insert document with _id 'duplicate': Document already exists with the given _id"))
           .body("errors[0].errorCode", is("DOCUMENT_ALREADY_EXISTS"));
 
       json =
@@ -373,7 +373,7 @@ public void orderedDuplicateIds() {
           .statusCode(200)
           .body("status.insertedIds", contains("doc4"))
           .body("data", is(nullValue()))
-          .body("errors[0].message", startsWith("Failed to insert document with _id doc4"))
+          .body("errors[0].message", startsWith("Failed to insert document with _id 'doc4'"))
           .body("errors[0].errorCode", is("DOCUMENT_ALREADY_EXISTS"));
 
       json =
@@ -433,7 +433,7 @@ public void orderedDuplicateDocumentNoNamespace() {
           .body(
               "errors[0].message",
               startsWith(
-                  "Failed to insert document with _id doc4: INVALID_ARGUMENT: keyspace something_else does not exist"))
+                  "Failed to insert document with _id 'doc4': INVALID_ARGUMENT: keyspace something_else does not exist"))
           .body("errors[0].exceptionClass", is("StatusRuntimeException"));
     }
 
@@ -525,7 +525,7 @@ public void unorderedDuplicateIds() {
           .statusCode(200)
           .body("status.insertedIds", containsInAnyOrder("doc4", "doc5"))
           .body("data", is(nullValue()))
-          .body("errors[0].message", startsWith("Failed to insert document with _id doc4"))
+          .body("errors[0].message", startsWith("Failed to insert document with _id 'doc4'"))
           .body("errors[0].errorCode", is("DOCUMENT_ALREADY_EXISTS"));
 
       json =

src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/DeleteOperationTest.java

@@ -456,7 +456,7 @@ public void deleteWithDynamicRetryFailure() {
                     .isEqualTo("CONCURRENCY_FAILURE");
                 assertThat(commandResult.errors().get(0).message())
                     .isEqualTo(
-                        "Failed to delete documents with _id [doc1]: Unable to complete transaction due to concurrent transactions");
+                        "Failed to delete documents with _id ['doc1']: Unable to complete transaction due to concurrent transactions");
               });
     }
 
@@ -1001,7 +1001,7 @@ public void errorPartial() {
                     .isEqualTo("CONCURRENCY_FAILURE");
                 assertThat(result.errors().get(0).message())
                     .isEqualTo(
-                        "Failed to delete documents with _id [doc1]: Unable to complete transaction due to concurrent transactions");
+                        "Failed to delete documents with _id ['doc1']: Unable to complete transaction due to concurrent transactions");
               });
     }
 
@@ -1174,7 +1174,7 @@ public void errorAll() {
                     .isEqualTo("CONCURRENCY_FAILURE");
                 assertThat(result.errors().get(0).message())
                     .isEqualTo(
-                        "Failed to delete documents with _id [doc1, doc2]: Unable to complete transaction due to concurrent transactions");
+                        "Failed to delete documents with _id ['doc1', 'doc2']: Unable to complete transaction due to concurrent transactions");
               });
     }
   }

src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/InsertOperationTest.java

@@ -182,7 +182,7 @@ public void insertDuplicate() throws Exception {
               error -> {
                 assertThat(error.message())
                     .isEqualTo(
-                        "Failed to insert document with _id doc1: Document already exists with the given _id");
+                        "Failed to insert document with _id 'doc1': Document already exists with the given _id");
                 assertThat(error.fields())
                     .containsEntry("exceptionClass", "JsonApiException")
                     .containsEntry("errorCode", "DOCUMENT_ALREADY_EXISTS");
@@ -507,7 +507,7 @@ public void failureOrdered() throws Exception {
           .satisfies(
               error -> {
                 assertThat(error.message())
-                    .isEqualTo("Failed to insert document with _id doc1: Ivan breaks the test.");
+                    .isEqualTo("Failed to insert document with _id 'doc1': Ivan breaks the test.");
                 assertThat(error.fields()).containsEntry("exceptionClass", "RuntimeException");
               });
     }
@@ -630,7 +630,7 @@ public void failureOrderedLastFails() throws Exception {
               error -> {
                 assertThat(error.message())
                     .isEqualTo(
-                        "Failed to insert document with _id doc2: Ivan really breaks the test.");
+                        "Failed to insert document with _id 'doc2': Ivan really breaks the test.");
                 assertThat(error.fields()).containsEntry("exceptionClass", "RuntimeException");
               });
     }
@@ -752,7 +752,7 @@ public void failureUnorderedPartial() throws Exception {
           .satisfies(
               error -> {
                 assertThat(error.message())
-                    .isEqualTo("Failed to insert document with _id doc1: Ivan breaks the test.");
+                    .isEqualTo("Failed to insert document with _id 'doc1': Ivan breaks the test.");
                 assertThat(error.fields()).containsEntry("exceptionClass", "RuntimeException");
               });
     }
@@ -871,8 +871,8 @@ public void failureUnorderedAll() throws Exception {
           .hasSize(2)
           .extracting(CommandResult.Error::message)
           .containsExactlyInAnyOrder(
-              "Failed to insert document with _id doc1: Ivan breaks the test.",
-              "Failed to insert document with _id doc2: Ivan really breaks the test.");
+              "Failed to insert document with _id 'doc1': Ivan breaks the test.",
+              "Failed to insert document with _id 'doc2': Ivan really breaks the test.");
     }
   }
 }

src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/ReadAndUpdateOperationRetryTest.java

@@ -446,7 +446,7 @@ public void findAndUpdateWithRetryFailure() throws Exception {
               assertThat(error.fields()).containsEntry("errorCode", "CONCURRENCY_FAILURE");
               assertThat(error.message())
                   .isEqualTo(
-                      "Failed to update documents with _id [doc1]: Unable to complete transaction due to concurrent transactions");
+                      "Failed to update documents with _id ['doc1']: Unable to complete transaction due to concurrent transactions");
             });
   }
 
@@ -651,7 +651,7 @@ public void findAndUpdateWithRetryFailureWithUpsert() throws Exception {
               assertThat(error.fields()).containsEntry("errorCode", "CONCURRENCY_FAILURE");
               assertThat(error.message())
                   .isEqualTo(
-                      "Failed to update documents with _id [doc1]: Unable to complete transaction due to concurrent transactions");
+                      "Failed to update documents with _id ['doc1']: Unable to complete transaction due to concurrent transactions");
             });
   }
 
@@ -914,7 +914,7 @@ public void findAndUpdateWithRetryPartialFailure() throws Exception {
               assertThat(error.fields()).containsEntry("errorCode", "CONCURRENCY_FAILURE");
               assertThat(error.message())
                   .isEqualTo(
-                      "Failed to update documents with _id [doc1]: Unable to complete transaction due to concurrent transactions");
+                      "Failed to update documents with _id ['doc1']: Unable to complete transaction due to concurrent transactions");
             });
   }
 
@@ -1238,7 +1238,7 @@ public void findOneAndUpdateWithRetryMultipleFailure() throws Exception {
                   .isEqualTo("CONCURRENCY_FAILURE");
               assertThat(commandResultSupplier.errors().get(0).message())
                   .isEqualTo(
-                      "Failed to update documents with _id [doc1, doc2]: Unable to complete transaction due to concurrent transactions");
+                      "Failed to update documents with _id ['doc1', 'doc2']: Unable to complete transaction due to concurrent transactions");
             });
   }
 }

src/test/java/io/stargate/sgv2/jsonapi/service/shredding/ShredderTest.java

@@ -112,7 +112,7 @@ public void shredSimpleWithoutId() throws Exception {
 
       assertThat(doc.id()).isInstanceOf(DocumentId.StringId.class);
       // should be auto-generated UUID:
-      assertThat(UUID.fromString(doc.id().toString())).isNotNull();
+      assertThat(UUID.fromString(doc.id().asDBKey())).isNotNull();
       List<JsonPath> expPaths =
           Arrays.asList(JsonPath.from("_id"), JsonPath.from("age"), JsonPath.from("name"));
 
@@ -128,7 +128,7 @@ public void shredSimpleWithoutId() throws Exception {
       JsonNode idNode = jsonFromShredded.get("_id");
       assertThat(idNode).isNotNull();
       String generatedId = idNode.textValue();
-      assertThat(generatedId).isEqualTo(doc.id().toString());
+      assertThat(generatedId).isEqualTo(doc.id().asDBKey());
       ((ObjectNode) inputDoc).put("_id", generatedId);
       assertThat(jsonFromShredded).isEqualTo(inputDoc);
 

src/test/java/io/stargate/sgv2/jsonapi/util/ExceptionUtilTest.java

@@ -41,7 +41,7 @@ public void getError() {
             err -> {
               assertThat(err.message())
                   .isEqualTo(
-                      "test error for ids [doc1, doc2]: Unable to complete transaction due to concurrent transactions");
+                      "test error for ids ['doc1', 'doc2']: Unable to complete transaction due to concurrent transactions");
               assertThat(err.fields()).containsEntry("exceptionClass", "JsonApiException");
               assertThat(err.fields()).containsEntry("errorCode", "CONCURRENCY_FAILURE");
             });
@@ -52,7 +52,8 @@ public void getError() {
     assertThat(error)
         .satisfies(
             err -> {
-              assertThat(err.message()).isEqualTo("test error for ids [doc1, doc2]: Some error");
+              assertThat(err.message())
+                  .isEqualTo("test error for ids ['doc1', 'doc2']: Some error");
               assertThat(err.fields()).containsEntry("exceptionClass", "RuntimeException");
             });
   }