Explicitly set serialVersionUID and add serialization tests (#1187)

Author: pkoenig10

sls-versions/build.gradle

@@ -9,6 +9,7 @@ dependencies {
     implementation 'com.palantir.safe-logging:logger'
 
     annotationProcessor 'org.immutables:value'
+    compileOnly 'org.immutables:serial'
     compileOnly 'org.immutables:value::annotations'
 
     testImplementation 'com.fasterxml.jackson.core:jackson-databind'

sls-versions/src/main/java/com/palantir/sls/versions/NonOrderableSlsVersion.java

@@ -21,12 +21,16 @@
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.palantir.logsafe.UnsafeArg;
 import java.util.Optional;
+import org.immutables.serial.Serial;
 import org.immutables.value.Value;
 
 @Value.Immutable
 @ImmutablesStyle
+@Serial.Version(value = 8200475591920425177L)
 public abstract class NonOrderableSlsVersion extends SlsVersion {
 
+    private static final long serialVersionUID = -5283676181284606311L;
+
     @JsonCreator
     public static NonOrderableSlsVersion valueOf(String value) {
         Optional<NonOrderableSlsVersion> optional = safeValueOf(value);

sls-versions/src/main/java/com/palantir/sls/versions/OrderableSlsVersion.java

@@ -21,6 +21,7 @@
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.palantir.logsafe.UnsafeArg;
 import java.util.Optional;
+import org.immutables.serial.Serial;
 import org.immutables.value.Value;
 
 /**
@@ -29,8 +30,11 @@
  */
 @Value.Immutable
 @ImmutablesStyle
+@Serial.Version(value = 1396122010215857210L)
 public abstract class OrderableSlsVersion extends SlsVersion implements Comparable<OrderableSlsVersion> {
 
+    private static final long serialVersionUID = -8422496404637895757L;
+
     private static final SlsVersionType[] ORDERED_VERSION_TYPES = {
         SlsVersionType.RELEASE,
         SlsVersionType.RELEASE_CANDIDATE,

sls-versions/src/main/java/com/palantir/sls/versions/SlsVersion.java

@@ -27,6 +27,8 @@
 
 public abstract class SlsVersion implements Serializable {
 
+    private static final long serialVersionUID = -4387436969618893669L;
+
     @JsonCreator
     public static SlsVersion valueOf(String value) {
         Optional<OrderableSlsVersion> optionalOrderableVersion = OrderableSlsVersion.safeValueOf(value);

sls-versions/src/test/java/com/palantir/sls/versions/SlsVersionTests.java

@@ -22,10 +22,20 @@
 import com.fasterxml.jackson.databind.ObjectReader;
 import com.fasterxml.jackson.databind.ObjectWriter;
 import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 public class SlsVersionTests {
 
+    private static final Path SERIALIZABLE_CASES_PATH = Path.of("src/test/serializableCases");
+
     private static final ObjectMapper MAPPER = new ObjectMapper();
     private static final ObjectReader READER = MAPPER.readerFor(SlsVersion.class);
     private static final ObjectWriter WRITER = MAPPER.writerFor(SlsVersion.class);
@@ -76,4 +86,103 @@ public void testCheckWithNonOrderableVersion() {
     public void testCheckWithGarbage() {
         assertThat(SlsVersion.check("foo")).isFalse();
     }
+
+    @ParameterizedTest
+    @MethodSource("serializable_arguments")
+    public void serializable(int version, SlsVersion input, SlsVersion expected) throws Exception {
+        Path path =
+                SERIALIZABLE_CASES_PATH.resolve(String.format("v" + version)).resolve(input.getValue() + ".ser");
+
+        if (!Files.exists(path) && Boolean.getBoolean("recreate")) {
+            writeObject(path, input);
+        }
+
+        SlsVersion actual = readObject(path, SlsVersion.class);
+
+        assertThat(actual).isEqualTo(expected);
+    }
+
+    // The version number here is intended to represent a specific version of this library. The
+    // version number is used in the expected file path in order to be able to test the serialization behavior of the
+    // same value in different versions of this library.
+    //
+    // The version number and expected file for existing test cases should NEVER be changed. The expected value for
+    // existing test cases can be changed if the SlsVersion API changes (but the existing serialized values still need
+    // to deserialize "correctly").
+    //
+    // When changing to the serialization behehavior and adding new test cases, the new test cases should always use a
+    // new version number.
+    private static List<Arguments> serializable_arguments() {
+        return List.of(
+                Arguments.of(
+                        0,
+                        SlsVersion.valueOf("1.2.3-4-gabcdef"),
+                        new OrderableSlsVersion.Builder()
+                                .value("1.2.3-4-gabcdef")
+                                .majorVersionNumber(1)
+                                .minorVersionNumber(2)
+                                .patchVersionNumber(3)
+                                .firstSequenceVersionNumber(4)
+                                .type(SlsVersionType.RELEASE_SNAPSHOT)
+                                .build()),
+                Arguments.of(
+                        0,
+                        SlsVersion.valueOf("1.2.3"),
+                        new OrderableSlsVersion.Builder()
+                                .value("1.2.3")
+                                .majorVersionNumber(1)
+                                .minorVersionNumber(2)
+                                .patchVersionNumber(3)
+                                .type(SlsVersionType.RELEASE)
+                                .build()),
+                Arguments.of(
+                        0,
+                        SlsVersion.valueOf("1.2.3-rc4-5-gabcdef"),
+                        new OrderableSlsVersion.Builder()
+                                .value("1.2.3-rc4-5-gabcdef")
+                                .majorVersionNumber(1)
+                                .minorVersionNumber(2)
+                                .patchVersionNumber(3)
+                                .firstSequenceVersionNumber(4)
+                                .secondSequenceVersionNumber(5)
+                                .type(SlsVersionType.RELEASE_CANDIDATE_SNAPSHOT)
+                                .build()),
+                Arguments.of(
+                        0,
+                        SlsVersion.valueOf("1.2.3-rc4"),
+                        new OrderableSlsVersion.Builder()
+                                .value("1.2.3-rc1")
+                                .majorVersionNumber(1)
+                                .minorVersionNumber(2)
+                                .patchVersionNumber(3)
+                                .firstSequenceVersionNumber(4)
+                                .type(SlsVersionType.RELEASE_CANDIDATE)
+                                .build()),
+                Arguments.of(
+                        0,
+                        SlsVersion.valueOf("1.2.3-abcdef"),
+                        new NonOrderableSlsVersion.Builder()
+                                .value("1.2.3-abcdef")
+                                .majorVersionNumber(1)
+                                .minorVersionNumber(2)
+                                .patchVersionNumber(3)
+                                .type(SlsVersionType.NON_ORDERABLE)
+                                .build()));
+    }
+
+    private static void writeObject(Path path, Object object) throws Exception {
+        Files.createDirectories(path.getParent());
+
+        try (ObjectOutputStream objectOutputStream = new ObjectOutputStream(Files.newOutputStream(path))) {
+            objectOutputStream.writeObject(object);
+        }
+    }
+
+    private static <T> T readObject(Path path, Class<T> objectClass) throws Exception {
+        try (ObjectInputStream objectInputStream = new ObjectInputStream(Files.newInputStream(path))) {
+            Object object = objectInputStream.readObject();
+            assertThat(object).isInstanceOf(objectClass);
+            return objectClass.cast(object);
+        }
+    }
 }