Fix #1232: unify/rename 2 different ErrorCodes for similar thing (#1233)

stargate · Jul 3, 2024 · c74a1e2 · c74a1e2
1 parent f44a74e
commit c74a1e2
Show file tree

Hide file tree

Showing 9 changed files with 43 additions and 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
@@ -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
@@ -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
@@ -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 <T extends Command> Uni<CommandResolver<T>> 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,19 +28,18 @@ 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");
               });
     }
 
     @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,15 +52,15 @@ 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");
               });
     }
 
     @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(