Make SubmoduleOp#createSubmoduleCommitMsg() less wasteful

Change the way how strings are appended to the StringBuilder to prevent
repeated instantiation of throw-away strings. In addition, use Apache's
StringUtils to indent old commit messages to spare creating RegEx
patterns and matchers.

Add two new config parameters to limit the submodule's commit message.

Change-Id: If5dde99babde3361adf1afeff0fc7a2560546551

Documentation/config-gerrit.txt

@@ -4590,6 +4590,21 @@ This allows to enable the superproject subscription mechanism.
 +
 By default this is true.
 
+[[submodule.maxCombinedCommitMessageSize]]submodule.maxCombinedCommitMessageSize::
++
+This allows to limit the length of the commit message for a submodule.
++
+By default this is 262144 (256 KiB).
++
+Common unit suffixes of k, m, or g are supported.
+
+[[submodule.maxCommitMessages]]submodule.maxCommitMessages::
++
+This allows to limit the number of commit messages that should be combined when creating
+a commit message for a submodule.
++
+By default this is 1000.
+
 [[user]]
 === Section user
 

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.acceptance.git;
 
 import static com.google.common.truth.Truth.assertThat;
+import static java.util.stream.Collectors.toList;
 
 import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -25,7 +26,9 @@ import com.google.gerrit.extensions.client.SubmitType;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gerrit.server.git.ProjectConfig;
+import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.StreamSupport;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ObjectId;
@@ -141,6 +144,31 @@ public abstract class AbstractSubmoduleSubscription extends AbstractDaemonTest {
     return pushChangeTo(repo, "refs/heads/" + branch, "some change", "");
   }
 
+  protected ObjectId pushChangesTo(TestRepository<?> repo, String branch, int numChanges)
+      throws Exception {
+    for (int i = 0; i < numChanges; i++) {
+      repo.branch("HEAD")
+          .commit()
+          .insertChangeId()
+          .message("Message " + i)
+          .add(name("file"), "content" + i)
+          .create();
+    }
+    String remoteBranch = "refs/heads/" + branch;
+    Iterable<PushResult> res =
+        repo.git()
+            .push()
+            .setRemote("origin")
+            .setRefSpecs(new RefSpec("HEAD:" + remoteBranch))
+            .call();
+    List<Status> status =
+        StreamSupport.stream(res.spliterator(), false)
+            .map(r -> r.getRemoteUpdate(remoteBranch).getStatus())
+            .collect(toList());
+    assertThat(status).containsExactly(Status.OK);
+    return Iterables.getLast(res).getRemoteUpdate(remoteBranch).getNewObjectId();
+  }
+
   protected void allowSubmoduleSubscription(
       String submodule, String subBranch, String superproject, String superBranch, boolean match)
       throws Exception {

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.acceptance.git;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.TruthJUnit.assume;
 
 import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.GerritConfig;
@@ -499,4 +500,42 @@ public class SubmoduleSubscriptionsIT extends AbstractSubmoduleSubscription {
 
     expectToHaveSubmoduleState(superRepo, "master", "nested/subscribed-to-project", subHEAD);
   }
+
+  @Test
+  @GerritConfig(name = "submodule.verboseSuperprojectUpdate", value = "SUBJECT_ONLY")
+  @GerritConfig(name = "submodule.maxCommitMessages", value = "1")
+  public void submoduleSubjectCommitMessageCountLimit() throws Exception {
+    testSubmoduleSubjectCommitMessageAndExpectTruncation();
+  }
+
+  @Test
+  @GerritConfig(name = "submodule.verboseSuperprojectUpdate", value = "SUBJECT_ONLY")
+  @GerritConfig(name = "submodule.maxCombinedCommitMessageSize", value = "175")
+  public void submoduleSubjectCommitMessageSizeLimit() throws Exception {
+    assume().that(isSubmitWholeTopicEnabled()).isFalse();
+    testSubmoduleSubjectCommitMessageAndExpectTruncation();
+  }
+
+  private void testSubmoduleSubjectCommitMessageAndExpectTruncation() throws Exception {
+    TestRepository<?> superRepo = createProjectWithPush("super-project");
+    TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
+    allowMatchingSubmoduleSubscription(
+        "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+
+    pushChangeTo(subRepo, "master");
+    createSubmoduleSubscription(superRepo, "master", "subscribed-to-project", "master");
+    // The first update doesn't include the rev log, so we ignore it
+    pushChangeTo(subRepo, "master");
+
+    // Next, we push two commits at once. Since maxCommitMessages=1, we expect to see only the first
+    // message plus ellipsis to mark truncation.
+    ObjectId subHEAD = pushChangesTo(subRepo, "master", 2);
+    RevCommit subCommitMsg = subRepo.getRevWalk().parseCommit(subHEAD);
+    expectToHaveCommitMessage(
+        superRepo,
+        "master",
+        String.format(
+            "Update git submodules\n\n* Update %s from branch 'master'\n  - %s\n\n[...]",
+            name("subscribed-to-project"), subCommitMsg.getShortMessage()));
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/git/SubmoduleOp.java

@@ -45,10 +45,12 @@ import java.util.Collections;
 import java.util.Deque;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import org.apache.commons.lang.StringUtils;
 import org.eclipse.jgit.dircache.DirCache;
 import org.eclipse.jgit.dircache.DirCacheBuilder;
 import org.eclipse.jgit.dircache.DirCacheEditor;
@@ -135,6 +137,8 @@ public class SubmoduleOp {
   private final BatchUpdate.Factory batchUpdateFactory;
   private final VerboseSuperprojectUpdate verboseSuperProject;
   private final boolean enableSuperProjectSubscriptions;
+  private final long maxCombinedCommitMessageSize;
+  private final long maxCommitMessages;
   private final MergeOpRepoManager orm;
   private final Map<Branch.NameKey, GitModules> branchGitModules;
 
@@ -170,6 +174,9 @@ public class SubmoduleOp {
         cfg.getEnum("submodule", null, "verboseSuperprojectUpdate", VerboseSuperprojectUpdate.TRUE);
     this.enableSuperProjectSubscriptions =
         cfg.getBoolean("submodule", "enableSuperProjectSubscriptions", true);
+    this.maxCombinedCommitMessageSize =
+        cfg.getLong("submodule", "maxCombinedCommitMessageSize", 256 << 10);
+    this.maxCommitMessages = cfg.getLong("submodule", "maxCommitMessages", 1000);
     this.orm = orm;
     this.updatedBranches = updatedBranches;
     this.targets = MultimapBuilder.hashKeys().hashSetValues().build();
@@ -547,8 +554,11 @@ public class SubmoduleOp {
       RevCommit newCommit,
       RevCommit oldCommit)
       throws SubmoduleException {
-    msgbuf.append("* Update " + s.getPath());
-    msgbuf.append(" from branch '" + s.getSubmodule().getShortName() + "'");
+    msgbuf.append("* Update ");
+    msgbuf.append(s.getPath());
+    msgbuf.append(" from branch '");
+    msgbuf.append(s.getSubmodule().getShortName());
+    msgbuf.append("'");
 
     // newly created submodule gitlink, do not append whole history
     if (oldCommit == null) {
@@ -559,13 +569,27 @@ public class SubmoduleOp {
       subOr.rw.resetRetain(subOr.canMergeFlag);
       subOr.rw.markStart(newCommit);
       subOr.rw.markUninteresting(oldCommit);
-      for (RevCommit c : subOr.rw) {
+      int numMessages = 0;
+      for (Iterator<RevCommit> iter = subOr.rw.iterator(); iter.hasNext(); ) {
+        RevCommit c = iter.next();
         subOr.rw.parseBody(c);
-        if (verboseSuperProject == VerboseSuperprojectUpdate.SUBJECT_ONLY) {
-          msgbuf.append("\n  - " + c.getShortMessage());
-        } else if (verboseSuperProject == VerboseSuperprojectUpdate.TRUE) {
-          msgbuf.append("\n  - " + c.getFullMessage().replace("\n", "\n    "));
+
+        String message =
+            verboseSuperProject == VerboseSuperprojectUpdate.SUBJECT_ONLY
+                ? c.getShortMessage()
+                : StringUtils.replace(c.getFullMessage(), "\n", "\n    ");
+
+        String bullet = "\n  - ";
+        String ellipsis = "\n\n[...]";
+        int newSize = msgbuf.length() + bullet.length() + message.length();
+        if (++numMessages > maxCommitMessages
+            || newSize > maxCombinedCommitMessageSize
+            || iter.hasNext() && (newSize + ellipsis.length()) > maxCombinedCommitMessageSize) {
+          msgbuf.append(ellipsis);
+          break;
         }
+        msgbuf.append(bullet);
+        msgbuf.append(message);
       }
     } catch (IOException e) {
       throw new SubmoduleException(