Override getPatches() in LargeObjectTombstone
Previously, adding a LargeObjectTombstone to the PatchListCache would fail because PatchListWeigher could not calculate the weight of the tombstone because patches was null. Overriding getPatches() fixes this problem. This commit also adds a test to check that LargeObjectTombstones are cached as expected. Change-Id: I843e8dbdfdc477b2f361305802837b4d10cb863d
This commit is contained in:
@@ -19,6 +19,7 @@ import static com.google.gerrit.acceptance.GitUtil.getChangeId;
|
|||||||
import static com.google.gerrit.acceptance.GitUtil.pushHead;
|
import static com.google.gerrit.acceptance.GitUtil.pushHead;
|
||||||
import static java.nio.charset.StandardCharsets.UTF_8;
|
import static java.nio.charset.StandardCharsets.UTF_8;
|
||||||
|
|
||||||
|
import com.google.common.cache.Cache;
|
||||||
import com.google.common.collect.ImmutableList;
|
import com.google.common.collect.ImmutableList;
|
||||||
import com.google.common.collect.Iterables;
|
import com.google.common.collect.Iterables;
|
||||||
import com.google.gerrit.acceptance.AbstractDaemonTest;
|
import com.google.gerrit.acceptance.AbstractDaemonTest;
|
||||||
@@ -29,11 +30,14 @@ import com.google.gerrit.reviewdb.client.Patch.ChangeType;
|
|||||||
import com.google.gerrit.server.patch.IntraLineDiff;
|
import com.google.gerrit.server.patch.IntraLineDiff;
|
||||||
import com.google.gerrit.server.patch.IntraLineDiffArgs;
|
import com.google.gerrit.server.patch.IntraLineDiffArgs;
|
||||||
import com.google.gerrit.server.patch.IntraLineDiffKey;
|
import com.google.gerrit.server.patch.IntraLineDiffKey;
|
||||||
|
import com.google.gerrit.server.patch.PatchList;
|
||||||
import com.google.gerrit.server.patch.PatchListCache;
|
import com.google.gerrit.server.patch.PatchListCache;
|
||||||
|
import com.google.gerrit.server.patch.PatchListCacheImpl;
|
||||||
import com.google.gerrit.server.patch.PatchListEntry;
|
import com.google.gerrit.server.patch.PatchListEntry;
|
||||||
import com.google.gerrit.server.patch.PatchListKey;
|
import com.google.gerrit.server.patch.PatchListKey;
|
||||||
import com.google.gerrit.server.patch.Text;
|
import com.google.gerrit.server.patch.Text;
|
||||||
import com.google.inject.Inject;
|
import com.google.inject.Inject;
|
||||||
|
import com.google.inject.name.Named;
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import org.eclipse.jgit.diff.Edit;
|
import org.eclipse.jgit.diff.Edit;
|
||||||
@@ -53,6 +57,10 @@ public class PatchListCacheIT extends AbstractDaemonTest {
|
|||||||
|
|
||||||
@Inject private PatchListCache patchListCache;
|
@Inject private PatchListCache patchListCache;
|
||||||
|
|
||||||
|
@Inject
|
||||||
|
@Named("diff")
|
||||||
|
private Cache<PatchListKey, PatchList> abstractPatchListCache;
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void listPatchesAgainstBase() throws Exception {
|
public void listPatchesAgainstBase() throws Exception {
|
||||||
commitBuilder().add(FILE_D, "4").message(SUBJECT_1).create();
|
commitBuilder().add(FILE_D, "4").message(SUBJECT_1).create();
|
||||||
@@ -204,6 +212,15 @@ public class PatchListCacheIT extends AbstractDaemonTest {
|
|||||||
assertThat(intraLineDiff.getEdits()).containsExactly(originalEdit);
|
assertThat(intraLineDiff.getEdits()).containsExactly(originalEdit);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void largeObjectTombstoneGetsCached() {
|
||||||
|
PatchListKey key = PatchListKey.againstDefaultBase(ObjectId.zeroId(), Whitespace.IGNORE_ALL);
|
||||||
|
PatchListCacheImpl.LargeObjectTombstone tombstone =
|
||||||
|
new PatchListCacheImpl.LargeObjectTombstone();
|
||||||
|
abstractPatchListCache.put(key, tombstone);
|
||||||
|
assertThat(abstractPatchListCache.getIfPresent(key)).isSameAs(tombstone);
|
||||||
|
}
|
||||||
|
|
||||||
private static void assertAdded(String expectedNewName, PatchListEntry e) {
|
private static void assertAdded(String expectedNewName, PatchListEntry e) {
|
||||||
assertName(expectedNewName, e);
|
assertName(expectedNewName, e);
|
||||||
assertThat(e.getChangeType()).isEqualTo(ChangeType.ADDED);
|
assertThat(e.getChangeType()).isEqualTo(ChangeType.ADDED);
|
||||||
|
|||||||
@@ -17,7 +17,9 @@ package com.google.gerrit.server.patch;
|
|||||||
|
|
||||||
import static com.google.gerrit.server.patch.DiffSummaryLoader.toDiffSummary;
|
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.cache.Cache;
|
||||||
|
import com.google.common.collect.ImmutableList;
|
||||||
import com.google.common.util.concurrent.UncheckedExecutionException;
|
import com.google.common.util.concurrent.UncheckedExecutionException;
|
||||||
import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
|
import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
|
||||||
import com.google.gerrit.reviewdb.client.Change;
|
import com.google.gerrit.reviewdb.client.Change;
|
||||||
@@ -29,6 +31,7 @@ import com.google.inject.Inject;
|
|||||||
import com.google.inject.Module;
|
import com.google.inject.Module;
|
||||||
import com.google.inject.Singleton;
|
import com.google.inject.Singleton;
|
||||||
import com.google.inject.name.Named;
|
import com.google.inject.name.Named;
|
||||||
|
import java.util.List;
|
||||||
import java.util.concurrent.ExecutionException;
|
import java.util.concurrent.ExecutionException;
|
||||||
import org.eclipse.jgit.errors.LargeObjectException;
|
import org.eclipse.jgit.errors.LargeObjectException;
|
||||||
import org.eclipse.jgit.lib.Config;
|
import org.eclipse.jgit.lib.Config;
|
||||||
@@ -190,9 +193,20 @@ public class PatchListCacheImpl implements PatchListCache {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/** Used to cache negative results in {@code fileCache}. */
|
/** Used to cache negative results in {@code fileCache}. */
|
||||||
private class LargeObjectTombstone extends PatchList {
|
@VisibleForTesting
|
||||||
|
public static class LargeObjectTombstone extends PatchList {
|
||||||
private static final long serialVersionUID = 1L;
|
private static final long serialVersionUID = 1L;
|
||||||
|
|
||||||
private LargeObjectTombstone() {}
|
@VisibleForTesting
|
||||||
|
public LargeObjectTombstone() {}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Return an empty list to prevent {@link NullPointerException}s inside of {@link
|
||||||
|
* PatchListWeigher}.
|
||||||
|
*/
|
||||||
|
@Override
|
||||||
|
public List<PatchListEntry> getPatches() {
|
||||||
|
return ImmutableList.of();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user