[fix] Re-add SerializableError backcompat fields as write-only (palan…

…tir#115)
tpetracca · Aug 23, 2018 · ff297df · ff297df
1 parent b95a60c
commit ff297df
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 5 deletions.
diff --git a/errors/build.gradle b/errors/build.gradle
@@ -7,6 +7,7 @@ dependencies {
     compile "com.google.code.findbugs:jsr305"
     compile "com.palantir.safe-logging:safe-logging"
     compile "javax.ws.rs:javax.ws.rs-api"
+    implementation "com.palantir.safe-logging:preconditions"
 
     testCompile project(":extras:jackson-support")
     testCompile "org.assertj:assertj-core"

diff --git a/errors/src/main/java/com/palantir/conjure/java/api/errors/SerializableError.java b/errors/src/main/java/com/palantir/conjure/java/api/errors/SerializableError.java
@@ -21,9 +21,11 @@
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.fasterxml.jackson.databind.annotation.JsonSerialize;
 import com.palantir.logsafe.Arg;
+import com.palantir.logsafe.exceptions.SafeIllegalStateException;
 import java.io.Serializable;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import org.immutables.value.Value;
 
 /**
@@ -45,15 +47,23 @@ public abstract class SerializableError implements Serializable {
      * and/or name.
      */
     @JsonProperty("errorCode")
-    public abstract String errorCode();
+    @Value.Default
+    public String errorCode() {
+        return getExceptionClass().orElseThrow(() -> new SafeIllegalStateException(
+                "Expected either 'errorCode' or 'exceptionClass' to be set"));
+    }
 
     /**
      * A fixed name identifying the error. For errors generated from {@link ServiceException}, this corresponding to the
      * {@link ErrorType#name} and is part of the service's API surface. Clients are given access to the service-side
      * error name via {@link RemoteException#getError} and typically switch&dispatch on the error code and/or name.
      */
     @JsonProperty("errorName")
-    public abstract String errorName();
+    @Value.Default
+    public String errorName() {
+        return getMessage().orElseThrow(() -> new SafeIllegalStateException(
+                "Expected either 'errorName' or 'message' to be set"));
+    }
 
     /**
      * A unique identifier for this error instance, typically used to correlate errors displayed in user-facing
@@ -71,6 +81,24 @@ public String errorInstanceId() {
     /** A set of parameters that further explain the error. */
     public abstract Map<String, String> parameters();
 
+    /**
+     * @deprecated Used by the serialization-mechanism for back-compat only. Do not use.
+     */
+    @Deprecated
+    @JsonProperty(value = "exceptionClass", access = JsonProperty.Access.WRITE_ONLY)
+    @Value.Auxiliary
+    @SuppressWarnings("checkstyle:designforextension")
+    abstract Optional<String> getExceptionClass();
+
+    /**
+     * @deprecated Used by the serialization-mechanism for back-compat only. Do not use.
+     */
+    @Deprecated
+    @JsonProperty(value = "message", access = JsonProperty.Access.WRITE_ONLY)
+    @Value.Auxiliary
+    @SuppressWarnings("checkstyle:designforextension")
+    abstract Optional<String> getMessage();
+
     /**
      * Creates a {@link SerializableError} representation of this exception that derives from the error code and
      * message, as well as the {@link Arg#isSafeForLogging safe} and unsafe {@link ServiceException#args parameters}.

diff --git a/errors/src/test/java/com/palantir/conjure/java/api/errors/SerializableErrorTest.java b/errors/src/test/java/com/palantir/conjure/java/api/errors/SerializableErrorTest.java
@@ -105,7 +105,7 @@ public void testDeserializationFailsWhenNeitherErrorNameNorMessageIsSet() throws
         String serialized = "{\"errorCode\":\"code\"}";
         assertThatThrownBy(() -> deserialize(serialized))
                 .isInstanceOf(IllegalStateException.class)
-                .hasMessage("Cannot build SerializableError, some of required attributes are not set [errorName]");
+                .hasMessage("Expected either 'errorName' or 'message' to be set");
     }
 
     private static SerializableError deserialize(String serialized) throws IOException {

diff --git a/errors/versions.lock b/errors/versions.lock
@@ -18,9 +18,21 @@
         "com.google.code.findbugs:jsr305": {
             "locked": "3.0.1"
         },
-        "com.palantir.safe-logging:safe-logging": {
+        "com.google.errorprone:error_prone_annotations": {
+            "locked": "2.1.3",
+            "transitive": [
+                "com.palantir.safe-logging:preconditions"
+            ]
+        },
+        "com.palantir.safe-logging:preconditions": {
             "locked": "1.5.0"
         },
+        "com.palantir.safe-logging:safe-logging": {
+            "locked": "1.5.0",
+            "transitive": [
+                "com.palantir.safe-logging:preconditions"
+            ]
+        },
         "javax.ws.rs:javax.ws.rs-api": {
             "locked": "2.0.1"
         }

diff --git a/test-utils/versions.lock b/test-utils/versions.lock
@@ -87,15 +87,28 @@
                 "com.palantir.conjure.java.api:errors"
             ]
         },
+        "com.google.errorprone:error_prone_annotations": {
+            "locked": "2.1.3",
+            "transitive": [
+                "com.palantir.safe-logging:preconditions"
+            ]
+        },
         "com.palantir.conjure.java.api:errors": {
             "project": true
         },
-        "com.palantir.safe-logging:safe-logging": {
+        "com.palantir.safe-logging:preconditions": {
             "locked": "1.5.0",
             "transitive": [
                 "com.palantir.conjure.java.api:errors"
             ]
         },
+        "com.palantir.safe-logging:safe-logging": {
+            "locked": "1.5.0",
+            "transitive": [
+                "com.palantir.conjure.java.api:errors",
+                "com.palantir.safe-logging:preconditions"
+            ]
+        },
         "javax.ws.rs:javax.ws.rs-api": {
             "locked": "2.0.1",
             "transitive": [