CreateChange: Only insert Change-Id if there isn't already one

When creating a new change, the Change-Id is inserted using JGit's
ChangeIdUtil. This does not detect an existing Change-Id in the
subject line, resulting in the commit message having two Change-Id
lines. The latter Change-Id, automatically added, is the one that
is actually used.

Add an extra check to detect the Change-Id in the subject line and
not add another one.

Bug: Issue 8284
Change-Id: Ic3c5953a86e8287c1b2228f406fdc54c2b4d914a

2 files changed, 33 insertions, 7 deletions

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 ed0dde04b4..3692aa50d0 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
@@ -105,8 +105,29 @@ public class CreateChangeIT extends AbstractDaemonTest {
   }
 
   @Test
+  public void createEmptyChange_InvalidSubject() throws Exception {
+   ChangeInput ci = newChangeInput(ChangeStatus.NEW);
+   ci.subject = "Change-Id: I1234000000000000000000000000000000000000";
+   assertCreateFails(ci, ResourceConflictException.class,
+       "missing subject; Change-Id must be in commit message footer");
+  }
+
+  @Test
   public void createNewChange() throws Exception {
-    assertCreateSucceeds(newChangeInput(ChangeStatus.NEW));
+    ChangeInfo info = assertCreateSucceeds(newChangeInput(ChangeStatus.NEW));
+    assertThat(info.revisions.get(info.currentRevision).commit.message)
+        .contains("Change-Id: " + info.changeId);
+  }
+
+  @Test
+  public void createNewChangeWithChangeId() throws Exception {
+    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);
+    assertThat(info.revisions.get(info.currentRevision).commit.message).contains(changeIdLine);
   }
 
   @Test
@@ -266,7 +287,7 @@ public class CreateChangeIT extends AbstractDaemonTest {
   private ChangeInfo assertCreateSucceeds(ChangeInput in) throws Exception {
     ChangeInfo out = gApi.changes().create(in).get();
     assertThat(out.branch).isEqualTo(in.branch);
-    assertThat(out.subject).isEqualTo(in.subject);
+    assertThat(out.subject).isEqualTo(in.subject.split("\n")[0]);
     assertThat(out.topic).isEqualTo(in.topic);
     assertThat(out.status).isEqualTo(in.status);
     assertThat(out.revisions).hasSize(1);
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 f5ddfe5f31..3079441539 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
@@ -231,11 +231,16 @@ public class CreateChange implements
       GeneralPreferencesInfo info =
           account.getAccount().getGeneralPreferencesInfo();
 
-      ObjectId treeId =
-          mergeTip == null ? emptyTreeId(oi) : mergeTip.getTree();
-      ObjectId id = ChangeIdUtil.computeChangeId(treeId,
-          mergeTip, author, author, input.subject);
-      String commitMessage = ChangeIdUtil.insertId(input.subject, id);
+      // Add a Change-Id line if there isn't already one
+      String commitMessage = input.subject;
+      if (ChangeIdUtil.indexOfChangeId(commitMessage, "\n") == -1) {
+        ObjectId treeId =
+            mergeTip == null ? emptyTreeId(oi) : mergeTip.getTree();
+        ObjectId id = ChangeIdUtil.computeChangeId(treeId,
+            mergeTip, author, author, commitMessage);
+        commitMessage = ChangeIdUtil.insertId(commitMessage, id);
+      }
+
       if (Boolean.TRUE.equals(info.signedOffBy)) {
         commitMessage += String.format("%s%s",
             SIGNED_OFF_BY_TAG,