Use DiffFormatter instead of PatchListCache to count files

We expect this to be faster and fix current timeout issues.
Note that BatchUpdateTest covers the "no parent" case.

Change-Id: I9c5b8b7bf16f0a733aff7501c4194a0ef4f58e36

javatests/com/google/gerrit/acceptance/server/git/receive/ReceiveCommitsLimitsIT.java

@@ -14,54 +14,97 @@
 
 package com.google.gerrit.acceptance.server.git.receive;
 
-import static com.google.common.truth.Truth.assertThat;
-
-import com.google.common.cache.Cache;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.config.GerritConfig;
-import com.google.gerrit.server.patch.DiffSummary;
-import com.google.gerrit.server.patch.DiffSummaryKey;
-import com.google.inject.Inject;
-import com.google.inject.name.Named;
+import org.eclipse.jgit.revwalk.RevCommit;
 import org.junit.Test;
 
 /** Tests for applying limits to e.g. number of files per change. */
 public class ReceiveCommitsLimitsIT extends AbstractDaemonTest {
+  @Test
+  @GerritConfig(name = "change.maxFiles", value = "2")
+  public void limitFileCount() throws Exception {
+    // Create the parent.
+    RevCommit parent =
+        commitBuilder()
+            .add("foo.txt", "same old, same old")
+            .add("bar.txt", "bar")
+            .message("blah")
+            .create();
+    testRepo.reset(parent);
 
-  @Inject
-  private @Named("diff_summary") Cache<DiffSummaryKey, DiffSummary> diffSummaryCache;
+    // A commit with 2 files is OK.
+    pushFactory
+        .create(
+            admin.newIdent(),
+            testRepo,
+            "blah",
+            ImmutableMap.of(
+                "foo.txt", "same old, same old", "bar.txt", "changed file", "baz.txt", "new file"))
+        .setParent(parent)
+        .to("refs/for/master")
+        .assertOkStatus();
+
+    // A commit with 3 files is rejected.
+    pushFactory
+        .create(
+            admin.newIdent(),
+            testRepo,
+            "blah",
+            ImmutableMap.of(
+                "foo.txt",
+                "same old, same old",
+                "bar.txt",
+                "changed file",
+                "baz.txt",
+                "new file",
+                "boom.txt",
+                "boom!"))
+        .setParent(parent)
+        .to("refs/for/master")
+        .assertErrorStatus("Exceeding maximum number of files per change (3 > 2)");
+  }
 
   @Test
   @GerritConfig(name = "change.maxFiles", value = "1")
-  public void limitFileCount() throws Exception {
-    PushOneCommit.Result result =
-        pushFactory
-            .create(
-                admin.newIdent(),
-                testRepo,
-                "foo",
-                ImmutableMap.of("foo", "foo-1.0", "bar", "bar-1.0"))
-            .to("refs/for/master");
-    result.assertErrorStatus("Exceeding maximum number of files per change (2 > 1)");
-  }
+  public void limitFileCount_merge() throws Exception {
+    // Create the parents.
+    RevCommit commitFoo =
+        commitBuilder().add("foo.txt", "same old, same old").message("blah").create();
+    RevCommit commitBar =
+        testRepo
+            .branch("branch")
+            .commit()
+            .insertChangeId()
+            .add("bar.txt", "bar")
+            .message("blah")
+            .create();
+    testRepo.reset(commitFoo);
 
-  @Test
-  public void cacheKeyMatches() throws Exception {
-    int cacheSizeBefore = diffSummaryCache.asMap().size();
-    PushOneCommit.Result result =
-        pushFactory
-            .create(
-                admin.newIdent(),
-                testRepo,
-                "foo",
-                ImmutableMap.of("foo", "foo-1.0", "bar", "bar-1.0"))
-            .to("refs/for/master");
-    result.assertOkStatus();
+    // By convention we diff against the first parent.
 
-    // Assert that we only performed the diff computation once. This would e.g. catch
-    // bugs/deviations in the computation of the cache key.
-    assertThat(diffSummaryCache.asMap()).hasSize(cacheSizeBefore + 1);
+    // commitFoo is first -> 1 file changed -> OK
+    pushFactory
+        .create(
+            admin.newIdent(),
+            testRepo,
+            "blah",
+            ImmutableMap.of("foo.txt", "same old, same old", "bar.txt", "changed file"))
+        .setParents(ImmutableList.of(commitFoo, commitBar))
+        .to("refs/for/master")
+        .assertOkStatus();
+
+    // commitBar is first -> 2 files changed -> rejected
+    pushFactory
+        .create(
+            admin.newIdent(),
+            testRepo,
+            "blah",
+            ImmutableMap.of("foo.txt", "same old, same old", "bar.txt", "changed file"))
+        .setParents(ImmutableList.of(commitBar, commitFoo))
+        .to("refs/for/master")
+        .assertErrorStatus("Exceeding maximum number of files per change (2 > 1)");
   }
 }