Merge "Return ResourceConflict error when suggested fix can't be applied." into stable-3.11
diff --git a/java/com/google/gerrit/server/restapi/change/ApplyProvidedFix.java b/java/com/google/gerrit/server/restapi/change/ApplyProvidedFix.java
index 4e05420..be1bced 100644
--- a/java/com/google/gerrit/server/restapi/change/ApplyProvidedFix.java
+++ b/java/com/google/gerrit/server/restapi/change/ApplyProvidedFix.java
@@ -56,6 +56,7 @@
import org.eclipse.jgit.lib.ObjectLoader;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.patch.PatchApplier;
+import org.eclipse.jgit.patch.PatchApplier.Result.Error;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.treewalk.TreeWalk;
@@ -86,8 +87,13 @@
@Override
public Response<EditInfo> apply(
RevisionResource revisionResource, ApplyProvidedFixInput applyProvidedFixInput)
- throws AuthException, BadRequestException, ResourceConflictException, IOException,
- ResourceNotFoundException, PermissionBackendException, RestApiException {
+ throws AuthException,
+ BadRequestException,
+ ResourceConflictException,
+ IOException,
+ ResourceNotFoundException,
+ PermissionBackendException,
+ RestApiException {
if (applyProvidedFixInput == null) {
throw new BadRequestException("applyProvidedFixInput is required");
}
@@ -173,7 +179,8 @@
String targetCommitMessage = targetCommitMessageFile.modifiableContent();
if (!originCommitMessage.equals(targetCommitMessage)) {
throw new ResourceConflictException(
- "The fix attempts to modify commit message of an older patchset, but commit message has been updated in a newer patchset. The fix can't be applied.");
+ "The fix attempts to modify commit message of an older patchset, but commit message"
+ + " has been updated in a newer patchset. The fix can't be applied.");
}
resultBuilder.newCommitMessage(originCommitModification.newCommitMessage().get());
}
@@ -185,9 +192,20 @@
try (ObjectInserter oi = repository.newObjectInserter()) {
ApplyPatchInput inp = new ApplyPatchInput();
inp.patch = patch;
- inp.allowConflicts = false;
+ // Allow conflicts for showing more precise error message.
+ inp.allowConflicts = true;
result =
ApplyPatchUtil.applyPatch(repository, oi, inp, repository.parseCommit(targetCommit));
+ if (!result.getErrors().isEmpty()) {
+ String errorMessage =
+ (result.getErrors().stream().anyMatch(Error::isGitConflict)
+ ? "Merge conflict while applying a fix:\n"
+ : "Error while applying a fix:\n")
+ + result.getErrors().stream()
+ .map(Error::toString)
+ .collect(Collectors.joining("\n"));
+ throw new ResourceConflictException(errorMessage);
+ }
oi.flush();
for (String path : result.getPaths()) {
try (TreeWalk tw = TreeWalk.forPath(rw.getObjectReader(), path, result.getTreeId())) {
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java b/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java
index d66a0a5..effd96f 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java
@@ -378,8 +378,17 @@
.value()
.asString()
.isEqualTo(
- "New line at the start\nFirst line\nSecond line\nTModified contentrd line\nFourth line\nFifth line\n"
- + "Sixth line\nSeventh line\nEighth line\nNinth line\nTenth line\n");
+ "New line at the start\n"
+ + "First line\n"
+ + "Second line\n"
+ + "TModified contentrd line\n"
+ + "Fourth line\n"
+ + "Fifth line\n"
+ + "Sixth line\n"
+ + "Seventh line\n"
+ + "Eighth line\n"
+ + "Ninth line\n"
+ + "Tenth line\n");
applyProvidedFixInput = createApplyProvidedFixInput(FILE_NAME, "(1st)", 1, 5, 1, 5);
applyProvidedFixInput.originalPatchsetForFix = previousRevision;
@@ -390,8 +399,17 @@
.value()
.asString()
.isEqualTo(
- "New line at the start\nFirst(1st) line\nSecond line\nTModified contentrd line\nFourth line\nFifth line\n"
- + "Sixth line\nSeventh line\nEighth line\nNinth line\nTenth line\n");
+ "New line at the start\n"
+ + "First(1st) line\n"
+ + "Second line\n"
+ + "TModified contentrd line\n"
+ + "Fourth line\n"
+ + "Fifth line\n"
+ + "Sixth line\n"
+ + "Seventh line\n"
+ + "Eighth line\n"
+ + "Ninth line\n"
+ + "Tenth line\n");
}
@Test
@@ -440,8 +458,15 @@
.value()
.asString()
.isEqualTo(
- "Third line\nFourth line\nFifth line\n"
- + "Sixth line\nSeventh line\nEighth line\nNinth line\nFirst modification\nTenth line\n");
+ "Third line\n"
+ + "Fourth line\n"
+ + "Fifth line\n"
+ + "Sixth line\n"
+ + "Seventh line\n"
+ + "Eighth line\n"
+ + "Ninth line\n"
+ + "First modification\n"
+ + "Tenth line\n");
Optional<BinaryResult> file2 = gApi.changes().id(changeId).edit().getFile(FILE_NAME2);
BinaryResultSubject.assertThat(file2)
.value()
@@ -450,6 +475,44 @@
}
@Test
+ public void applyProvidedFixOnNewerPatchset_mergeConflictThrowsResourceConflictException()
+ throws Exception {
+ // Remember patch set and add another one.
+ int previousRevision = gApi.changes().id(changeId).get().currentRevisionNumber;
+ // Change first 2 lines;
+ String modifiedContent =
+ "abc\ndef\n"
+ + FILE_CONTENT.substring(
+ FILE_CONTENT.indexOf("\n", FILE_CONTENT.indexOf("\n") + 1) + 1);
+ amendChange(
+ changeId,
+ "refs/for/master",
+ admin,
+ testRepo,
+ PushOneCommit.SUBJECT,
+ FILE_NAME,
+ modifiedContent);
+ FixReplacementInfo fixReplacementInfo1 = new FixReplacementInfo();
+ fixReplacementInfo1.path = FILE_NAME;
+ fixReplacementInfo1.range = createRange(2, 0, 2, 0);
+ fixReplacementInfo1.replacement = "First modification\n";
+
+ ApplyProvidedFixInput applyProvidedFixInput = new ApplyProvidedFixInput();
+
+ applyProvidedFixInput.fixReplacementInfos = Arrays.asList(fixReplacementInfo1);
+ applyProvidedFixInput.originalPatchsetForFix = previousRevision;
+
+ ResourceConflictException thrown =
+ assertThrows(
+ ResourceConflictException.class,
+ () -> gApi.changes().id(changeId).current().applyFix(applyProvidedFixInput));
+
+ assertThat(thrown.getMessage()).contains("Merge conflict");
+ // Change edit must not be created
+ assertThat(gApi.changes().id(changeId).edit().get()).isEmpty();
+ }
+
+ @Test
public void applyProvidedFixOnCommitMessageCanBeAppliedToNewerPatchset() throws Exception {
// Set a dedicated commit message.
String footer = "\nChange-Id: " + changeId + "\n";
