From bc003d0b36c00d8992a09c028bd027fc8dcfb48b Mon Sep 17 00:00:00 2001
From: Patrick Hiesel <hiesel@google.com>
Date: Wed, 23 Aug 2017 15:52:08 +0200
Subject: [PATCH] Initialize PatchList with minimal values for serialization

Change the way LargeObjectTombstone is initialized to enable correct
(de)serialization, and add a test for it.

Change-Id: Ib9290a9b95d78991ec683befaecd3aa38b3b94c2
---
 .../google/gerrit/server/patch/PatchList.java |  2 --
 .../server/patch/PatchListCacheImpl.java      | 16 +++++---------
 .../gerrit/server/patch/PatchListTest.java    | 22 +++++++++++++++++++
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchList.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchList.java
index b43dc16efb..16ede58c1d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchList.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchList.java
@@ -105,8 +105,6 @@ public class PatchList implements Serializable {
     this.patches = patches;
   }
 
-  protected PatchList() {}
-
   /** Old side tree or commit; null only if this is a combined diff. */
   @Nullable
   public ObjectId getOldId() {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
index be5a7aa9f2..b985723e6d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
@@ -19,7 +19,6 @@ import static com.google.gerrit.server.patch.DiffSummaryLoader.toDiffSummary;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.cache.Cache;
-import com.google.common.collect.ImmutableList;
 import com.google.common.util.concurrent.UncheckedExecutionException;
 import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
 import com.google.gerrit.reviewdb.client.Change;
@@ -31,7 +30,6 @@ import com.google.inject.Inject;
 import com.google.inject.Module;
 import com.google.inject.Singleton;
 import com.google.inject.name.Named;
-import java.util.List;
 import java.util.concurrent.ExecutionException;
 import org.eclipse.jgit.errors.LargeObjectException;
 import org.eclipse.jgit.lib.Config;
@@ -198,15 +196,11 @@ public class PatchListCacheImpl implements PatchListCache {
     private static final long serialVersionUID = 1L;
 
     @VisibleForTesting
-    public LargeObjectTombstone() {}
-
-    /**
-     * Return an empty list to prevent {@link NullPointerException}s inside of {@link
-     * PatchListWeigher}.
-     */
-    @Override
-    public List<PatchListEntry> getPatches() {
-      return ImmutableList.of();
+    public LargeObjectTombstone() {
+      // Initialize super class with valid values. We don't care about the inner state, but need to
+      // pass valid values that don't break (de)serialization.
+      super(
+          null, ObjectId.zeroId(), false, ComparisonType.againstAutoMerge(), new PatchListEntry[0]);
     }
   }
 }
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/patch/PatchListTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/patch/PatchListTest.java
index 19adf32f3f..0a7b97ccf9 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/patch/PatchListTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/patch/PatchListTest.java
@@ -17,6 +17,11 @@ package com.google.gerrit.server.patch;
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.gerrit.reviewdb.client.Patch;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.InputStream;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
 import java.util.Arrays;
 import java.util.Comparator;
 import org.junit.Test;
@@ -65,4 +70,21 @@ public class PatchListTest {
         });
     assertThat(names).isEqualTo(want);
   }
+
+  @Test
+  public void largeObjectTombstoneCanBeSerializedAndDeserialized() throws Exception {
+    // Serialize
+    byte[] serializedObject;
+    try (ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        ObjectOutputStream objectStream = new ObjectOutputStream(baos)) {
+      objectStream.writeObject(new PatchListCacheImpl.LargeObjectTombstone());
+      serializedObject = baos.toByteArray();
+      assertThat(serializedObject).isNotNull();
+    }
+    // Deserialize
+    try (InputStream is = new ByteArrayInputStream(serializedObject);
+        ObjectInputStream ois = new ObjectInputStream(is)) {
+      assertThat(ois.readObject()).isInstanceOf(PatchListCacheImpl.LargeObjectTombstone.class);
+    }
+  }
 }