Fail with 409 if rebase of change edit fails due to conflicts

With this the user gets a proper error message displayed, rather then
getting the generic 'Server Error' message.

Change-Id: Icd60ebaff7756906899293ba689dc3e77be63431
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin 2015-02-12 14:35:33 +01:00 committed by David Pursehouse
parent 4512bb4582
commit f1d9cab348
2 changed files with 25 additions and 3 deletions

View File

@ -21,6 +21,7 @@ import static com.google.gerrit.acceptance.GitUtil.createProject;
import static java.nio.charset.StandardCharsets.UTF_8; import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.SECONDS; import static java.util.concurrent.TimeUnit.SECONDS;
import static org.apache.http.HttpStatus.SC_CONFLICT;
import static org.apache.http.HttpStatus.SC_NOT_FOUND; import static org.apache.http.HttpStatus.SC_NOT_FOUND;
import static org.apache.http.HttpStatus.SC_NO_CONTENT; import static org.apache.http.HttpStatus.SC_NO_CONTENT;
import static org.apache.http.HttpStatus.SC_OK; import static org.apache.http.HttpStatus.SC_OK;
@ -100,6 +101,7 @@ public class ChangeEditIT extends AbstractDaemonTest {
private Change change; private Change change;
private String changeId; private String changeId;
private Change change2; private Change change2;
private String changeId2;
private PatchSet ps; private PatchSet ps;
private PatchSet ps2; private PatchSet ps2;
@ -111,7 +113,7 @@ public class ChangeEditIT extends AbstractDaemonTest {
amendChange(git, admin.getIdent(), changeId); amendChange(git, admin.getIdent(), changeId);
change = getChange(changeId); change = getChange(changeId);
assertThat(ps).isNotNull(); assertThat(ps).isNotNull();
String changeId2 = newChange2(git, admin.getIdent()); changeId2 = newChange2(git, admin.getIdent());
change2 = getChange(changeId2); change2 = getChange(changeId2);
assertThat(change2).isNotNull(); assertThat(change2).isNotNull();
ps2 = getCurrentPatchSet(changeId2); ps2 = getCurrentPatchSet(changeId2);
@ -231,6 +233,24 @@ public class ChangeEditIT extends AbstractDaemonTest {
assertThat(afterRebase).isNotEqualTo(beforeRebase); assertThat(afterRebase).isNotEqualTo(beforeRebase);
} }
@Test
public void rebaseEditWithConflictsRest_Conflict() throws Exception {
PatchSet current = getCurrentPatchSet(changeId2);
assertThat(modifier.createEdit(change2, current)).isEqualTo(RefUpdate.Result.NEW);
assertThat(
modifier.modifyFile(editUtil.byChange(change2).get(), FILE_NAME,
RestSession.newRawInput(CONTENT_NEW))).isEqualTo(RefUpdate.Result.FORCED);
ChangeEdit edit = editUtil.byChange(change2).get();
assertThat(edit.getBasePatchSet().getPatchSetId()).isEqualTo(
current.getPatchSetId());
PushOneCommit push =
pushFactory.create(db, admin.getIdent(), PushOneCommit.SUBJECT, FILE_NAME,
new String(CONTENT_NEW2), changeId2);
push.to(git, "refs/for/master").assertOkStatus();
RestResponse r = adminSession.post(urlRebase());
assertThat(r.getStatusCode()).isEqualTo(SC_CONFLICT);
}
@Test @Test
public void updateExistingFile() throws Exception { public void updateExistingFile() throws Exception {
assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW); assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW);

View File

@ -144,11 +144,13 @@ public class ChangeEditModifier {
* @param edit change edit that contains edit to rebase * @param edit change edit that contains edit to rebase
* @param current patch set to rebase the edit on * @param current patch set to rebase the edit on
* @throws AuthException * @throws AuthException
* @throws ResourceConflictException thrown if rebase fails due to merge conflicts
* @throws InvalidChangeOperationException * @throws InvalidChangeOperationException
* @throws IOException * @throws IOException
*/ */
public void rebaseEdit(ChangeEdit edit, PatchSet current) public void rebaseEdit(ChangeEdit edit, PatchSet current)
throws AuthException, InvalidChangeOperationException, IOException { throws AuthException, ResourceConflictException,
InvalidChangeOperationException, IOException {
if (!currentUser.get().isIdentifiedUser()) { if (!currentUser.get().isIdentifiedUser()) {
throw new AuthException("Authentication required"); throw new AuthException("Authentication required");
} }
@ -202,7 +204,7 @@ public class ChangeEditModifier {
} }
} else { } else {
// TODO(davido): Allow to resolve conflicts inline // TODO(davido): Allow to resolve conflicts inline
throw new InvalidChangeOperationException("merge conflict"); throw new ResourceConflictException("merge conflict");
} }
} finally { } finally {
rw.release(); rw.release();