Merge "Merge branch 'stable-2.16' into stable-3.0" into stable-3.0
diff --git a/java/com/google/gerrit/server/ChangeUtil.java b/java/com/google/gerrit/server/ChangeUtil.java
index b8a00f4..b39b544 100644
--- a/java/com/google/gerrit/server/ChangeUtil.java
+++ b/java/com/google/gerrit/server/ChangeUtil.java
@@ -19,18 +19,26 @@
import com.google.common.collect.Ordering;
import com.google.common.io.BaseEncoding;
+import com.google.gerrit.common.FooterConstants;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.server.util.CommitMessageUtil;
import com.google.inject.Singleton;
import java.io.IOException;
import java.security.SecureRandom;
import java.util.Collection;
+import java.util.List;
import java.util.Map;
import java.util.Random;
import java.util.Set;
import java.util.stream.Stream;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
@Singleton
public class ChangeUtil {
@@ -121,6 +129,37 @@
id);
}
+ /**
+ * Make sure that the change commit message has a correct footer.
+ *
+ * @param requireChangeId true if Change-Id is a mandatory footer for the project
+ * @param currentChangeId current Change-Id value before the commit message is updated
+ * @param newCommitMessage new commit message for the change
+ * @throws ResourceConflictException if the new commit message has a missing or invalid Change-Id
+ * @throws BadRequestException if the new commit message is null or empty
+ */
+ public static void ensureChangeIdIsCorrect(
+ boolean requireChangeId, String currentChangeId, String newCommitMessage)
+ throws ResourceConflictException, BadRequestException {
+ RevCommit revCommit =
+ RevCommit.parse(
+ Constants.encode("tree " + ObjectId.zeroId().name() + "\n\n" + newCommitMessage));
+
+ // Check that the commit message without footers is not empty
+ 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) {
+ throw new ResourceConflictException("multiple Change-Id footers");
+ }
+ }
+
public static String status(Change c) {
return c != null ? c.getStatus().name().toLowerCase() : "deleted";
}
diff --git a/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
index 246a53f..9620b49 100644
--- a/java/com/google/gerrit/server/edit/ChangeEditModifier.java
+++ b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
@@ -20,8 +20,10 @@
import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.RawInput;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
@@ -206,7 +208,8 @@
* @throws PermissionBackendException
* @throws BadRequestException if the commit message is malformed
*/
- public void modifyMessage(Repository repository, ChangeNotes notes, String newCommitMessage)
+ public void modifyMessage(
+ Repository repository, Project.NameKey project, ChangeNotes notes, String newCommitMessage)
throws AuthException, IOException, UnchangedCommitMessageException,
PermissionBackendException, BadRequestException, ResourceConflictException {
assertCanEdit(notes);
@@ -228,6 +231,11 @@
ObjectId newEditCommit =
createCommit(repository, basePatchSetCommit, baseTree, newCommitMessage, nowTimestamp);
+ ChangeUtil.ensureChangeIdIsCorrect(
+ projectCache.checkedGet(project).is(BooleanProjectConfig.REQUIRE_CHANGE_ID),
+ notes.getChange().getKey().get(),
+ newCommitMessage);
+
if (optionalChangeEdit.isPresent()) {
updateEdit(repository, optionalChangeEdit.get(), newEditCommit, nowTimestamp);
} else {
diff --git a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
index f0689a8..53e89b3 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
@@ -455,7 +455,7 @@
Project.NameKey project = rsrc.getProject();
try (Repository repository = repositoryManager.openRepository(project)) {
- editModifier.modifyMessage(repository, rsrc.getNotes(), input.message);
+ editModifier.modifyMessage(repository, project, rsrc.getNotes(), input.message);
} catch (UnchangedCommitMessageException e) {
throw new ResourceConflictException(e.getMessage());
}
diff --git a/java/com/google/gerrit/server/restapi/change/PutMessage.java b/java/com/google/gerrit/server/restapi/change/PutMessage.java
index c542164..3c7259d 100644
--- a/java/com/google/gerrit/server/restapi/change/PutMessage.java
+++ b/java/com/google/gerrit/server/restapi/change/PutMessage.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.restapi.change;
-import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.common.CommitMessageInput;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -48,11 +47,9 @@
import com.google.inject.Singleton;
import java.io.IOException;
import java.sql.Timestamp;
-import java.util.List;
import java.util.TimeZone;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
-import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
@@ -111,7 +108,7 @@
String sanitizedCommitMessage = CommitMessageUtil.checkAndSanitizeCommitMessage(input.message);
ensureCanEditCommitMessage(resource.getNotes());
- ensureChangeIdIsCorrect(
+ ChangeUtil.ensureChangeIdIsCorrect(
projectCache.checkedGet(resource.getProject()).is(BooleanProjectConfig.REQUIRE_CHANGE_ID),
resource.getChange().getKey().get(),
sanitizedCommitMessage);
@@ -192,26 +189,4 @@
throw new AuthException("modifying commit message not permitted", denied);
}
}
-
- private static void ensureChangeIdIsCorrect(
- boolean requireChangeId, String currentChangeId, String newCommitMessage)
- throws ResourceConflictException, BadRequestException {
- RevCommit revCommit =
- RevCommit.parse(
- Constants.encode("tree " + ObjectId.zeroId().name() + "\n\n" + newCommitMessage));
-
- // Check that the commit message without footers is not empty
- 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) {
- throw new ResourceConflictException("multiple Change-Id footers");
- }
- }
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
index c27d637..38195a9 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
@@ -426,7 +426,7 @@
gApi.changes().id(changeId).edit().modifyFile(filePath, RawInputUtil.create(fileContent));
gApi.changes().id(changeId).edit().publish();
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
- gApi.changes().id(changeId).edit().modifyCommitMessage("An unchanged patchset");
+ gApi.changes().id(changeId).edit().modifyCommitMessage(updatedCommitMessage());
gApi.changes().id(changeId).edit().publish();
DiffInfo diffInfo =
@@ -448,7 +448,7 @@
gApi.changes().id(changeId).edit().modifyFile(filePath, RawInputUtil.create(fileContent));
gApi.changes().id(changeId).edit().publish();
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
- gApi.changes().id(changeId).edit().modifyCommitMessage("An unchanged patchset");
+ gApi.changes().id(changeId).edit().modifyCommitMessage(updatedCommitMessage());
gApi.changes().id(changeId).edit().publish();
DiffInfo diffInfo =
@@ -466,7 +466,7 @@
gApi.changes().id(changeId).edit().modifyFile(filePath, RawInputUtil.create(fileContent));
gApi.changes().id(changeId).edit().publish();
String previousPatchSetId = gApi.changes().id(changeId).get().currentRevision;
- gApi.changes().id(changeId).edit().modifyCommitMessage("An unchanged patchset");
+ gApi.changes().id(changeId).edit().modifyCommitMessage(updatedCommitMessage());
gApi.changes().id(changeId).edit().publish();
DiffInfo diffInfo =
@@ -2717,6 +2717,10 @@
assertThat(diffInfo).content().element(0).numberOfSkippedLines().isGreaterThan(0);
}
+ private String updatedCommitMessage() {
+ return "An unchanged patchset\n\nChange-Id: " + changeId;
+ }
+
private static CommentInput createCommentInput(
int startLine, int startCharacter, int endLine, int endCharacter, String message) {
CommentInput comment = new CommentInput();
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
index ba228f6..d58210a 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
@@ -790,7 +790,8 @@
@Test
public void fixDoesNotModifyCommitMessageOfChangeEdit() throws Exception {
- String changeEditCommitMessage = "This is the commit message of the change edit.\n";
+ String changeEditCommitMessage =
+ "This is the commit message of the change edit.\n\nChange-Id: " + changeId + "\n";
gApi.changes().id(changeId).edit().modifyCommitMessage(changeEditCommitMessage);
fixReplacementInfo.path = FILE_NAME;
diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index 3a2a50d..0f2018d 100644
--- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -309,12 +309,14 @@
@Test
public void updateCommitMessageByEditingMagicCommitMsgFile() throws Exception {
createEmptyEditFor(changeId);
+ String updatedCommitMsg = "Foo Bar\n\nChange-Id: " + changeId + "\n";
gApi.changes()
.id(changeId)
.edit()
- .modifyFile(Patch.COMMIT_MSG, RawInputUtil.create("Foo Bar".getBytes(UTF_8)));
+ .modifyFile(Patch.COMMIT_MSG, RawInputUtil.create(updatedCommitMsg.getBytes(UTF_8)));
assertThat(getEdit(changeId)).isPresent();
- ensureSameBytes(getFileContentOfEdit(changeId, Patch.COMMIT_MSG), "Foo Bar\n".getBytes(UTF_8));
+ ensureSameBytes(
+ getFileContentOfEdit(changeId, Patch.COMMIT_MSG), updatedCommitMsg.getBytes(UTF_8));
}
@Test
@@ -335,6 +337,33 @@
}
@Test
+ public void updateMessageEditChangeIdShouldThrowResourceConflictException() throws Exception {
+ createEmptyEditFor(changeId);
+ String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
+
+ exception.expect(ResourceConflictException.class);
+ exception.expectMessage("wrong Change-Id footer");
+ gApi.changes()
+ .id(changeId)
+ .edit()
+ .modifyCommitMessage(commitMessage.replaceAll(changeId, changeId2));
+ }
+
+ @Test
+ public void updateMessageEditRemoveChangeIdShouldThrowResourceConflictException()
+ throws Exception {
+ createEmptyEditFor(changeId);
+ String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
+
+ exception.expect(ResourceConflictException.class);
+ exception.expectMessage("missing Change-Id footer");
+ gApi.changes()
+ .id(changeId)
+ .edit()
+ .modifyCommitMessage(commitMessage.replaceAll("(Change-Id:).*", ""));
+ }
+
+ @Test
public void updateMessageNoChange() throws Exception {
createEmptyEditFor(changeId);
String commitMessage = gApi.changes().id(changeId).edit().getCommitMessage();
