commit 3c18ab315e99949d04735598d6033e15e19714e9
author David Pursehouse <dpursehouse@collab.net> Tue Feb 06 12:19:09 2018 +0900
committer David Pursehouse <dpursehouse@collab.net> Tue Feb 06 13:10:45 2018 +0900
tree c1ceeb6141de605ccee51d82e089836bc1069ec0
parent c8c55e2131638509dcd8d5b8b31afea636de311a

CreateChange: Fix appending Signed-off-by line after Change-Id

The Signed-off-by line was appended onto the existing message
without a newline. If there was already a Change-Id line in the
commit message footer, this resulted in the Change-Id being
malformed.

Bug: Issue 8299
Change-Id: I0b8084bcf27737eec3291a79504f20e697a3b187

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java

diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
index 3692aa5..993f3f5 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
@@ -145,6 +145,26 @@
   }
 
   @Test
+  public void createNewChangeSignedOffByFooterWithChangeId() throws Exception {
+    setSignedOffByFooter(true);
+    try {
+      ChangeInput ci = newChangeInput(ChangeStatus.NEW);
+      String changeId = "I1234000000000000000000000000000000000000";
+      String changeIdLine = "Change-Id: " + changeId;
+      ci.subject = "Subject\n\n" + changeIdLine;
+      ChangeInfo info = assertCreateSucceeds(ci);
+      assertThat(info.changeId).isEqualTo(changeId);
+      String message = info.revisions.get(info.currentRevision).commit.message;
+      assertThat(message).contains(changeIdLine);
+      assertThat(message).contains(
+          String.format("%sAdministrator <%s>", SIGNED_OFF_BY_TAG,
+              admin.getIdent().getEmailAddress()));
+    } finally {
+      setSignedOffByFooter(false);
+    }
+  }
+
+  @Test
   public void createNewDraftChange() throws Exception {
     assume().that(isAllowDrafts()).isTrue();
     assertCreateSucceeds(newChangeInput(ChangeStatus.DRAFT));

gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
index 3079441..cf4be18 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
@@ -16,6 +16,7 @@
 
 import static org.eclipse.jgit.lib.Constants.SIGNED_OFF_BY_TAG;
 
+import com.google.common.base.Joiner;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Strings;
 import com.google.common.collect.Iterables;
@@ -242,9 +243,10 @@
       }
 
       if (Boolean.TRUE.equals(info.signedOffBy)) {
-        commitMessage += String.format("%s%s",
-            SIGNED_OFF_BY_TAG,
-            account.getAccount().getNameEmail(anonymousCowardName));
+        commitMessage =
+            Joiner.on("\n").join(commitMessage.trim(), String.format(
+                "%s%s", SIGNED_OFF_BY_TAG,
+                account.getAccount().getNameEmail(anonymousCowardName)));
       }
 
       RevCommit c;