opendev/gerrit
Code Issues Proposed changes

Add error messages for abandon and restore when in bad state

Browse Source 
The abandon and restore routines can fail if the change
is in the wrong state (abandoned already, or open still).
These failure lead to internal NPEs.  Instead, create a
new exception and give a reason for the failure to the user.

Change-Id: Id7861d75e535c439c12329f7e891797c5b1f6eca
This commit is contained in:
Martin Fick
2011-05-24 18:27:59 -06:00
parent 824d1e6c80
commit 066c71219d
5 changed files with 94 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

71
gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
View File

@@ -34,6 +34,7 @@ import com.google.gerrit.server.git.MergeQueue;
import com.google.gerrit.server.git.ReplicationQueue;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.mail.AbandonedSender;
import com.google.gerrit.server.mail.EmailException;
@@ -212,7 +213,7 @@ public class ChangeUtil {
final IdentifiedUser user, final String message, final ReviewDb db,
final AbandonedSender.Factory abandonedSenderFactory,
final ChangeHookRunner hooks) throws NoSuchChangeException,
EmailException, OrmException {
InvalidChangeOperationException, EmailException, OrmException {
final Change.Id changeId = patchSetId.getParentKey();
final PatchSet patch = db.patchSets().get(patchSetId);
if (patch == null) {
@@ -245,23 +246,26 @@ public class ChangeUtil {
}
});
if (updatedChange != null) {
db.changeMessages().insert(Collections.singleton(cmsg));
final List<PatchSetApproval> approvals =
db.patchSetApprovals().byChange(changeId).toList();
for (PatchSetApproval a : approvals) {
a.cache(updatedChange);
}
db.patchSetApprovals().update(approvals);
// Email the reviewers
final AbandonedSender cm = abandonedSenderFactory.create(updatedChange);
cm.setFrom(user.getAccountId());
cm.setChangeMessage(cmsg);
cm.send();
if (updatedChange == null) {
throw new InvalidChangeOperationException(
"Change is no longer open or patchset is not latest");
}
db.changeMessages().insert(Collections.singleton(cmsg));
final List<PatchSetApproval> approvals =
db.patchSetApprovals().byChange(changeId).toList();
for (PatchSetApproval a : approvals) {
a.cache(updatedChange);
}
db.patchSetApprovals().update(approvals);
// Email the reviewers
final AbandonedSender cm = abandonedSenderFactory.create(updatedChange);
cm.setFrom(user.getAccountId());
cm.setChangeMessage(cmsg);
cm.send();
hooks.doChangeAbandonedHook(updatedChange, user.getAccount(), message);
}
@@ -371,7 +375,7 @@ public class ChangeUtil {
final IdentifiedUser user, final String message, final ReviewDb db,
final AbandonedSender.Factory abandonedSenderFactory,
final ChangeHookRunner hooks) throws NoSuchChangeException,
EmailException, OrmException {
InvalidChangeOperationException, EmailException, OrmException {
final Change.Id changeId = patchSetId.getParentKey();
final PatchSet patch = db.patchSets().get(patchSetId);
if (patch == null) {
@@ -404,23 +408,26 @@ public class ChangeUtil {
}
});
if (updatedChange != null) {
db.changeMessages().insert(Collections.singleton(cmsg));
final List<PatchSetApproval> approvals =
db.patchSetApprovals().byChange(changeId).toList();
for (PatchSetApproval a : approvals) {
a.cache(updatedChange);
}
db.patchSetApprovals().update(approvals);
// Email the reviewers
final AbandonedSender cm = abandonedSenderFactory.create(updatedChange);
cm.setFrom(user.getAccountId());
cm.setChangeMessage(cmsg);
cm.send();
if (updatedChange == null) {
throw new InvalidChangeOperationException(
"Change is not abandoned or patchset is not latest");
}
db.changeMessages().insert(Collections.singleton(cmsg));
final List<PatchSetApproval> approvals =
db.patchSetApprovals().byChange(changeId).toList();
for (PatchSetApproval a : approvals) {
a.cache(updatedChange);
}
db.patchSetApprovals().update(approvals);
// Email the reviewers
final AbandonedSender cm = abandonedSenderFactory.create(updatedChange);
cm.setFrom(user.getAccountId());
cm.setChangeMessage(cmsg);
cm.send();
hooks.doChangeRestoreHook(updatedChange, user.getAccount(), message);
}