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

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:#commit-message-input[CommitMessageInput] entity. If a Change-Id
-link:project-configuration.html#require-change-id[Require Change-Id] was specified.
+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
 ----

java/com/google/gerrit/server/restapi/change/PutMessage.java

@@ -111,8 +111,11 @@ public class PutMessage
     String sanitizedCommitMessage = CommitMessageUtil.checkAndSanitizeCommitMessage(input.message);
 
     ensureCanEditCommitMessage(resource.getNotes());
+    sanitizedCommitMessage =
         ensureChangeIdIsCorrect(
-        projectCache.checkedGet(resource.getProject()).is(BooleanProjectConfig.REQUIRE_CHANGE_ID),
+            projectCache
+                .checkedGet(resource.getProject())
+                .is(BooleanProjectConfig.REQUIRE_CHANGE_ID),
             resource.getChange().getKey().get(),
             sanitizedCommitMessage);
 
@@ -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;
   }
 }

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 =
+    gApi.changes().id(r.getChangeId()).setMessage("modified commit\n");
-        assertThrows(
+    assertThat(getCommitMessage(r.getChangeId()))
-            ResourceConflictException.class,
+        .isEqualTo("modified commit\n\nChange-Id: " + r.getChangeId() + "\n");
-            () -> gApi.changes().id(r.getChangeId()).setMessage("modified commit\n"));
-    assertThat(thrown).hasMessageThat().contains("missing Change-Id footer");
   }
 
   @Test