Handle commit validation errors when publishing change edit

When a commit validator throws a CommitValidationException during
publish of a new patch set from the inline editor, it is translated
to an InvalidChangeOperationException which results in an internal
server error message being displayed to the user.

Instead, catch the InvalidChangeOperationException and throw a
ResourceConflictException, which results in the actual error message
being displayed to the user.

The corresponding change in the cookbook plugin includes an example
of commit validation to reproduce this case.

Bug: Issue 3442
Change-Id: I95b88dc6f2bc76df088ebef50a52974b33cab5ad
This commit is contained in:
David Pursehouse 2015-06-22 16:25:37 +09:00
parent 5f239bb3b9
commit bcee7ff75a
3 changed files with 11 additions and 10 deletions

View File

@ -29,7 +29,6 @@ import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.server.edit.ChangeEdit; import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditUtil; import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
@ -85,7 +84,7 @@ public class PublishChangeEdit implements
@Override @Override
public Response<?> apply(ChangeResource rsrc, Publish.Input in) public Response<?> apply(ChangeResource rsrc, Publish.Input in)
throws AuthException, ResourceConflictException, NoSuchChangeException, throws AuthException, ResourceConflictException, NoSuchChangeException,
IOException, InvalidChangeOperationException, OrmException { IOException, OrmException {
Capable r = Capable r =
rsrc.getControl().getProjectControl().canPushToAtLeastOneRef(); rsrc.getControl().getProjectControl().canPushToAtLeastOneRef();
if (r != Capable.OK) { if (r != Capable.OK) {

View File

@ -134,13 +134,11 @@ public class ChangeEditUtil {
* @param edit change edit to publish * @param edit change edit to publish
* @throws NoSuchChangeException * @throws NoSuchChangeException
* @throws IOException * @throws IOException
* @throws InvalidChangeOperationException
* @throws OrmException * @throws OrmException
* @throws ResourceConflictException * @throws ResourceConflictException
*/ */
public void publish(ChangeEdit edit) throws NoSuchChangeException, public void publish(ChangeEdit edit) throws NoSuchChangeException,
IOException, InvalidChangeOperationException, IOException, OrmException, ResourceConflictException {
OrmException, ResourceConflictException {
Change change = edit.getChange(); Change change = edit.getChange();
try (Repository repo = gitManager.openRepository(change.getProject()); try (Repository repo = gitManager.openRepository(change.getProject());
RevWalk rw = new RevWalk(repo); RevWalk rw = new RevWalk(repo);
@ -151,10 +149,14 @@ public class ChangeEditUtil {
"only edit for current patch set can be published"); "only edit for current patch set can be published");
} }
insertPatchSet(edit, change, repo, rw, basePatchSet, try {
squashEdit(rw, inserter, edit.getEditCommit(), basePatchSet)); insertPatchSet(edit, change, repo, rw, basePatchSet,
// TODO(davido): This should happen in the same BatchRefUpdate. squashEdit(rw, inserter, edit.getEditCommit(), basePatchSet));
deleteRef(repo, edit); // TODO(davido): This should happen in the same BatchRefUpdate.
deleteRef(repo, edit);
} catch (InvalidChangeOperationException e) {
throw new ResourceConflictException(e.getMessage());
}
} }
} }

@ -1 +1 @@
Subproject commit 955332734bd554527064f8af32907ee1c1561d2c Subproject commit abba4940141178bbc7c6ad38690262fd93533ded