Fix serialization of AssumptionViolatedException (#1654)

Added serializable descriptions of values and matchers and use them in writeObject() serialization of AssumptionViolatedException.

Fixes #1192

Author: sirchia

src/main/java/org/junit/internal/AssumptionViolatedException.java

@@ -1,5 +1,8 @@
 package org.junit.internal;
 
+import java.io.IOException;
+import java.io.ObjectOutputStream;
+
 import org.hamcrest.Description;
 import org.hamcrest.Matcher;
 import org.hamcrest.SelfDescribing;
@@ -108,4 +111,29 @@ public void describeTo(Description description) {
             }
         }
     }
+
+    /**
+     * Override default Java object serialization to correctly deal with potentially unserializable matchers or values.
+     * By not implementing readObject, we assure ourselves of backwards compatibility and compatibility with the
+     * standard way of Java serialization.
+     *
+     * @param objectOutputStream The outputStream to write our representation to
+     * @throws IOException When serialization fails
+     */
+    private void writeObject(ObjectOutputStream objectOutputStream) throws IOException {
+        ObjectOutputStream.PutField putField = objectOutputStream.putFields();
+        putField.put("fAssumption", fAssumption);
+        putField.put("fValueMatcher", fValueMatcher);
+
+        // We have to wrap the matcher into a serializable form.
+        putField.put("fMatcher", SerializableMatcherDescription.asSerializableMatcher(fMatcher));
+
+        // We have to wrap the value inside a non-String class (instead of serializing the String value directly) as
+        // A Description will handle a String and non-String object differently (1st is surrounded by '"' while the
+        // latter will be surrounded by '<' '>'. Wrapping it makes sure that the description of a serialized and
+        // non-serialized instance produce the exact same description
+        putField.put("fValue", SerializableValueDescription.asSerializableValue(fValue));
+
+        objectOutputStream.writeFields();
+    }
 }

src/main/java/org/junit/internal/SerializableMatcherDescription.java

@@ -0,0 +1,47 @@
+package org.junit.internal;
+
+import java.io.Serializable;
+
+import org.hamcrest.BaseMatcher;
+import org.hamcrest.Description;
+import org.hamcrest.Matcher;
+import org.hamcrest.StringDescription;
+
+/**
+ * This class exists solely to provide a serializable description of a matcher to be serialized as a field in
+ * {@link AssumptionViolatedException}. Being a {@link Throwable}, it is required to be {@link Serializable}, but most
+ * implementations of {@link Matcher} are not. This class works around that limitation as
+ * {@link AssumptionViolatedException} only every uses the description of the {@link Matcher}, while still retaining
+ * backwards compatibility with classes compiled against its class signature before 4.14 and/or deserialization of
+ * previously serialized instances.
+ */
+class SerializableMatcherDescription<T> extends BaseMatcher<T> implements Serializable {
+
+    private final String matcherDescription;
+
+    private SerializableMatcherDescription(Matcher<T> matcher) {
+        matcherDescription = StringDescription.asString(matcher);
+    }
+
+    public boolean matches(Object o) {
+        throw new UnsupportedOperationException("This Matcher implementation only captures the description");
+    }
+
+    public void describeTo(Description description) {
+        description.appendText(matcherDescription);
+    }
+
+    /**
+     * Factory method that checks to see if the matcher is already serializable.
+     * @param matcher the matcher to make serializable
+     * @return The provided matcher if it is null or already serializable,
+     * the SerializableMatcherDescription representation of it if it is not.
+     */
+    static <T> Matcher<T> asSerializableMatcher(Matcher<T> matcher) {
+        if (matcher == null || matcher instanceof Serializable) {
+            return matcher;
+        } else {
+            return new SerializableMatcherDescription<T>(matcher);
+        }
+    }
+}

src/main/java/org/junit/internal/SerializableValueDescription.java

@@ -0,0 +1,38 @@
+package org.junit.internal;
+
+import java.io.Serializable;
+
+/**
+ * This class exists solely to provide a serializable description of a value to be serialized as a field in
+ * {@link AssumptionViolatedException}. Being a {@link Throwable}, it is required to be {@link Serializable}, but a
+ * value of type Object provides no guarantee to be serializable. This class works around that limitation as
+ * {@link AssumptionViolatedException} only every uses the string representation of the value, while still retaining
+ * backwards compatibility with classes compiled against its class signature before 4.14 and/or deserialization of
+ * previously serialized instances.
+ */
+class SerializableValueDescription implements Serializable {
+    private final String value;
+
+    private SerializableValueDescription(Object value) {
+        this.value = String.valueOf(value);
+    }
+
+    /**
+     * Factory method that checks to see if the value is already serializable.
+     * @param value the value to make serializable
+     * @return The provided value if it is null or already serializable,
+     * the SerializableValueDescription representation of it if it is not.
+     */
+    static Object asSerializableValue(Object value) {
+        if (value == null || value instanceof Serializable) {
+            return value;
+        } else {
+            return new SerializableValueDescription(value);
+        }
+    }
+
+    @Override
+    public String toString() {
+        return value;
+    }
+}

src/test/java/org/junit/AssumptionViolatedExceptionTest.java

@@ -4,12 +4,27 @@
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsNot.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assume.assumeThat;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.io.Serializable;
+
+import org.hamcrest.BaseMatcher;
+import org.hamcrest.Description;
 import org.hamcrest.Matcher;
 import org.hamcrest.StringDescription;
 import org.junit.experimental.theories.DataPoint;
 import org.junit.experimental.theories.Theories;
 import org.junit.experimental.theories.Theory;
+import org.junit.rules.TestName;
 import org.junit.runner.RunWith;
 
 @RunWith(Theories.class)
@@ -23,6 +38,14 @@ public class AssumptionViolatedExceptionTest {
     @DataPoint
     public static Matcher<Integer> NULL = null;
 
+    @Rule
+    public TestName name = new TestName();
+
+    private static final String MESSAGE = "Assumption message";
+    private static Matcher<Integer> SERIALIZABLE_IS_THREE = new SerializableIsThreeMatcher<Integer>();
+    private static final UnserializableClass UNSERIALIZABLE_VALUE = new UnserializableClass();
+    private static final Matcher<UnserializableClass> UNSERIALIZABLE_MATCHER = not(is(UNSERIALIZABLE_VALUE));
+
     @Theory
     public void toStringReportsMatcher(Integer actual, Matcher<Integer> matcher) {
         assumeThat(matcher, notNullValue());
@@ -92,4 +115,93 @@ public void canSetCauseWithInstanceCreatedWithExplicitThrowableConstructor() {
       AssumptionViolatedException e = new AssumptionViolatedException("invalid number", cause);
       assertThat(e.getCause(), is(cause));
     }
+
+    @Test
+    public void assumptionViolatedExceptionWithoutValueAndMatcherCanBeReserialized_v4_13()
+            throws IOException, ClassNotFoundException {
+        assertReserializable(new AssumptionViolatedException(MESSAGE));
+    }
+
+    @Test
+    public void assumptionViolatedExceptionWithValueAndMatcherCanBeReserialized_v4_13()
+            throws IOException, ClassNotFoundException {
+        assertReserializable(new AssumptionViolatedException(MESSAGE, TWO, SERIALIZABLE_IS_THREE));
+    }
+
+    @Test
+    public void unserializableValueAndMatcherCanBeSerialized() throws IOException, ClassNotFoundException {
+        AssumptionViolatedException exception = new AssumptionViolatedException(MESSAGE,
+                UNSERIALIZABLE_VALUE, UNSERIALIZABLE_MATCHER);
+
+        assertCanBeSerialized(exception);
+    }
+
+    @Test
+    public void nullValueAndMatcherCanBeSerialized() throws IOException, ClassNotFoundException {
+        AssumptionViolatedException exception = new AssumptionViolatedException(MESSAGE);
+
+        assertCanBeSerialized(exception);
+    }
+
+    @Test
+    public void serializableValueAndMatcherCanBeSerialized() throws IOException, ClassNotFoundException {
+        AssumptionViolatedException exception = new AssumptionViolatedException(MESSAGE,
+                TWO, SERIALIZABLE_IS_THREE);
+
+        assertCanBeSerialized(exception);
+    }
+
+    private void assertCanBeSerialized(AssumptionViolatedException exception)
+            throws IOException, ClassNotFoundException {
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        ObjectOutputStream oos = new ObjectOutputStream(baos);
+        oos.writeObject(exception);
+
+        ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());
+        ObjectInputStream ois = new ObjectInputStream(bais);
+        AssumptionViolatedException fromStream = (AssumptionViolatedException) ois.readObject();
+
+        assertSerializedCorrectly(exception, fromStream);
+    }
+
+    private void assertReserializable(AssumptionViolatedException expected)
+            throws IOException, ClassNotFoundException {
+        String resourceName = name.getMethodName();
+        InputStream resource = getClass().getResourceAsStream(resourceName);
+        assertNotNull("Could not read resource " + resourceName, resource);
+        ObjectInputStream objectInputStream = new ObjectInputStream(resource);
+        AssumptionViolatedException fromStream = (AssumptionViolatedException) objectInputStream.readObject();
+
+        assertSerializedCorrectly(expected, fromStream);
+    }
+
+    private void assertSerializedCorrectly(
+            AssumptionViolatedException expected, AssumptionViolatedException fromStream) {
+        assertNotNull(fromStream);
+
+        // Exceptions don't implement equals() so we need to compare field by field
+        assertEquals("message", expected.getMessage(), fromStream.getMessage());
+        assertEquals("description", StringDescription.asString(expected), StringDescription.asString(fromStream));
+        // We don't check the stackTrace as that will be influenced by how the test was started
+        // (e.g. by maven or directly from IDE)
+        // We also don't check the cause as that should already be serialized correctly by the superclass
+    }
+
+    private static class SerializableIsThreeMatcher<T> extends BaseMatcher<T> implements Serializable {
+
+        public boolean matches(Object item) {
+            return IS_THREE.matches(item);
+        }
+
+        public void describeTo(Description description) {
+            IS_THREE.describeTo(description);
+        }
+    }
+
+    private static class UnserializableClass {
+        @Override
+        public String toString() {
+            return "I'm not serializable";
+        }
+    }
 }