Merge "PutMessage: automatically add Change-Id if requireChangeId is set"

3 files changed, 27 insertions, 17 deletions

diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 15f59c16a2..56376fa042 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -849,8 +849,10 @@ returned that describes the resulting change.
 Creates a new patch set with a new commit message.
 
 The new commit message must be provided in the request body inside a
-link:#commit-message-input[CommitMessageInput] entity and contain the change ID footer if
-link:project-configuration.html#require-change-id[Require Change-Id] was specified.
+link:#commit-message-input[CommitMessageInput] entity. If a Change-Id
+footer is specified, it must match the current Change-Id footer. If
+the Change-Id footer is absent, the current Change-Id is added to the
+message.
 
 .Request
 ----
diff --git a/java/com/google/gerrit/server/restapi/change/PutMessage.java b/java/com/google/gerrit/server/restapi/change/PutMessage.java
index db024188cc..f05641fed9 100644
--- a/java/com/google/gerrit/server/restapi/change/PutMessage.java
+++ b/java/com/google/gerrit/server/restapi/change/PutMessage.java
@@ -111,10 +111,13 @@ public class PutMessage
     String sanitizedCommitMessage = CommitMessageUtil.checkAndSanitizeCommitMessage(input.message);
 
     ensureCanEditCommitMessage(resource.getNotes());
-    ensureChangeIdIsCorrect(
-        projectCache.checkedGet(resource.getProject()).is(BooleanProjectConfig.REQUIRE_CHANGE_ID),
-        resource.getChange().getKey().get(),
-        sanitizedCommitMessage);
+    sanitizedCommitMessage =
+        ensureChangeIdIsCorrect(
+            projectCache
+                .checkedGet(resource.getProject())
+                .is(BooleanProjectConfig.REQUIRE_CHANGE_ID),
+            resource.getChange().getKey().get(),
+            sanitizedCommitMessage);
 
     try (Repository repository = repositoryManager.openRepository(resource.getProject());
         RevWalk revWalk = new RevWalk(repository);
@@ -193,7 +196,7 @@ public class PutMessage
     }
   }
 
-  private static void ensureChangeIdIsCorrect(
+  private static String ensureChangeIdIsCorrect(
       boolean requireChangeId, String currentChangeId, String newCommitMessage)
       throws ResourceConflictException, BadRequestException {
     RevCommit revCommit =
@@ -204,14 +207,21 @@ public class PutMessage
     CommitMessageUtil.checkAndSanitizeCommitMessage(revCommit.getShortMessage());
 
     List<String> changeIdFooters = revCommit.getFooterLines(FooterConstants.CHANGE_ID);
-    if (requireChangeId && changeIdFooters.isEmpty()) {
-      throw new ResourceConflictException("missing Change-Id footer");
-    }
     if (!changeIdFooters.isEmpty() && !changeIdFooters.get(0).equals(currentChangeId)) {
       throw new ResourceConflictException("wrong Change-Id footer");
     }
-    if (changeIdFooters.size() > 1) {
+
+    if (requireChangeId && revCommit.getFooterLines().isEmpty()) {
+      // sanitization always adds '\n' at the end.
+      newCommitMessage += "\n";
+    }
+
+    if (requireChangeId && changeIdFooters.isEmpty()) {
+      newCommitMessage += FooterConstants.CHANGE_ID.getName() + ": " + currentChangeId + "\n";
+    } else if (changeIdFooters.size() > 1) {
       throw new ResourceConflictException("multiple Change-Id footers");
     }
+
+    return newCommitMessage;
   }
 }
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 5c2f0330d6..7fc1134aea 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -3861,15 +3861,13 @@ public class ChangeIT extends AbstractDaemonTest {
   }
 
   @Test
-  public void changeCommitMessageWithNoChangeIdFails() throws Exception {
+  public void changeCommitMessageWithNoChangeIdRetainsChangeID() throws Exception {
     PushOneCommit.Result r = createChange();
     assertThat(getCommitMessage(r.getChangeId()))
         .isEqualTo("test commit\n\nChange-Id: " + r.getChangeId() + "\n");
-    ResourceConflictException thrown =
-        assertThrows(
-            ResourceConflictException.class,
-            () -> gApi.changes().id(r.getChangeId()).setMessage("modified commit\n"));
-    assertThat(thrown).hasMessageThat().contains("missing Change-Id footer");
+    gApi.changes().id(r.getChangeId()).setMessage("modified commit\n");
+    assertThat(getCommitMessage(r.getChangeId()))
+        .isEqualTo("modified commit\n\nChange-Id: " + r.getChangeId() + "\n");
   }
 
   @Test