opendev/gerrit
Code Issues Proposed changes

PatchSetInserter: Improve error message when change is closed

Browse Source 
Say "Cannot create new patch set" because if we just say "Change is
closed" then it doesn't communicate why this is a problem and what the
server is trying to do.

Throw ResourceConflictException so the message is propagated through
BatchUpdate to the user.

Change-Id: I647e3038e1989537528f3a7c47e5eb93a9305a6b
This commit is contained in:
Dave Borowitz
2016-01-27 11:40:09 -05:00
parent 12d6212986
commit b40590166c
3 changed files with 8 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
View File

@@ -46,7 +46,6 @@ import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.ReviewerStateInternal; import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.RefControl; import com.google.gerrit.server.project.RefControl;
import com.google.gerrit.server.ssh.NoSshInfo; import com.google.gerrit.server.ssh.NoSshInfo;
import com.google.gerrit.server.ssh.SshInfo; import com.google.gerrit.server.ssh.SshInfo;
@@ -204,8 +203,8 @@ public class PatchSetInserter extends BatchUpdate.Op {
} }
@Override @Override
public boolean updateChange(ChangeContext ctx) throws OrmException, public boolean updateChange(ChangeContext ctx)
InvalidChangeOperationException, IOException { throws ResourceConflictException, OrmException, IOException {
ChangeControl ctl = ctx.getControl(); ChangeControl ctl = ctx.getControl();
change = ctx.getChange(); change = ctx.getChange();
@@ -213,8 +212,9 @@ public class PatchSetInserter extends BatchUpdate.Op {
update.setSubjectForCommit("Create patch set " + psId.get()); update.setSubjectForCommit("Create patch set " + psId.get());
if (!change.getStatus().isOpen() && !allowClosed) { if (!change.getStatus().isOpen() && !allowClosed) {
throw new InvalidChangeOperationException(String.format( throw new ResourceConflictException(String.format(
"Change %s is closed", change.getId())); "Cannot create new patch set of change %s because it is %s",
change.getId(), change.getStatus().name().toLowerCase()));
} }
Iterable<String> newGroups = groups; Iterable<String> newGroups = groups;

2
gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
View File

@@ -152,7 +152,7 @@ public class RebaseChangeOp extends BatchUpdate.Op {
@Override @Override
public boolean updateChange(ChangeContext ctx) public boolean updateChange(ChangeContext ctx)
throws OrmException, InvalidChangeOperationException, IOException { throws ResourceConflictException, OrmException, IOException {
boolean ret = patchSetInserter.updateChange(ctx); boolean ret = patchSetInserter.updateChange(ctx);
rebasedPatchSet = patchSetInserter.getPatchSet(); rebasedPatchSet = patchSetInserter.getPatchSet();
return ret; return ret;

3
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.server.git.strategy;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.gerrit.extensions.restapi.MergeConflictException; import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetApproval;
@@ -146,7 +147,7 @@ public class RebaseIfNecessary extends SubmitStrategy {
@Override @Override
public PatchSet updateChangeImpl(ChangeContext ctx) public PatchSet updateChangeImpl(ChangeContext ctx)
throws NoSuchChangeException, InvalidChangeOperationException, throws NoSuchChangeException, ResourceConflictException,
OrmException, IOException { OrmException, IOException {
if (rebaseOp == null) { if (rebaseOp == null) {
// Took the fast-forward option, nothing to do. // Took the fast-forward option, nothing to do.