opendev/gerrit
Code Issues Proposed changes

Change: Add helper methods to check change state

Browse Source 
To check a change's state the caller currently has to call getState()
and then do a comparison on the enum value.

To make this easier, introduce new methods on Change:

- isNew
- isMerged
- isAbandoned
- isClosed

Note that we don't add "isOpen" since there are no longer multiple
states that could be considered "open". Previously there were "new"
and "draft", but the draft workflow was removed.

Update callers to use the new methods.

Change-Id: Ic047f74b114e2277e66c878dace73ea8e0babab6
This commit is contained in:
David Pursehouse 2019-02-14 13:15:41 +09:00
parent f23f78c99f
commit 4c58e1031f
48 changed files with 114 additions and 117 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
View File

@ -112,7 +112,7 @@ class ElasticChangeIndex extends AbstractElasticIndex<Change.Id, ChangeData>
String insertIndex;
try {
if (cd.change().getStatus().isOpen()) {
if (cd.change().isNew()) {
insertIndex = OPEN_CHANGES;
deleteIndex = CLOSED_CHANGES;
} else {

2
java/com/google/gerrit/lucene/LuceneChangeIndex.java
View File

@ -207,7 +207,7 @@ public class LuceneChangeIndex implements ChangeIndex {
// sub-index, so just pick one.
Document doc = openIndex.toDocument(cd);
try {
if (cd.change().getStatus().isOpen()) {
if (cd.change().isNew()) {
Futures.allAsList(closedIndex.delete(id), openIndex.replace(id, doc)).get();
} else {
Futures.allAsList(openIndex.delete(id), closedIndex.replace(id, doc)).get();

16
java/com/google/gerrit/reviewdb/client/Change.java
View File

@ -671,6 +671,22 @@ public final class Change {
status = newStatus.getCode();
}
public boolean isNew() {
return getStatus().equals(Status.NEW);
}
public boolean isMerged() {
return getStatus().equals(Status.MERGED);
}
public boolean isAbandoned() {
return getStatus().equals(Status.ABANDONED);
}
public boolean isClosed() {
return isAbandoned() || isMerged();
}
public String getTopic() {
return topic;
}

2
java/com/google/gerrit/server/PatchSetUtil.java
View File

@ -145,7 +145,7 @@ public class PatchSetUtil {
/** Is the current patch set locked against state changes? */
public boolean isPatchSetLocked(ChangeNotes notes) throws OrmException, IOException {
Change change = notes.getChange();
if (change.getStatus() == Change.Status.MERGED) {
if (change.isMerged()) {
return false;
}

2
java/com/google/gerrit/server/change/AbandonOp.java
View File

@ -83,7 +83,7 @@ public class AbandonOp implements BatchUpdateOp {
change = ctx.getChange();
PatchSet.Id psId = change.currentPatchSetId();
ChangeUpdate update = ctx.getUpdate(psId);
if (!change.getStatus().isOpen()) {
if (!change.isNew()) {
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
}
patchSet = psUtil.get(ctx.getNotes(), psId);

4
java/com/google/gerrit/server/change/ActionJson.java
View File

@ -29,7 +29,6 @@ import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.extensions.webui.PrivateInternals_UiActionDescription;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.extensions.webui.UiActions;
import com.google.gerrit.server.notedb.ChangeNotes;
@ -180,8 +179,7 @@ public class ActionJson {
// The followup action is a client-side only operation that does not
// have a server side handler. It must be manually registered into the
// resulting action map.
Status status = notes.getChange().getStatus();
if (status.isOpen() || status.equals(Status.MERGED)) {
if (!notes.getChange().isAbandoned()) {
UiAction.Description descr = new UiAction.Description();
PrivateInternals_UiActionDescription.setId(descr, "followup");
PrivateInternals_UiActionDescription.setMethod(descr, "POST");

6
java/com/google/gerrit/server/change/ChangeJson.java
View File

@ -510,7 +510,7 @@ public class ChangeJson {
out.assignee = in.getAssignee() != null ? accountLoader.get(in.getAssignee()) : null;
out.hashtags = cd.hashtags();
out.changeId = in.getKey().get();
if (in.getStatus().isOpen()) {
if (in.isNew()) {
SubmitTypeRecord str = cd.submitTypeRecord();
if (str.isOk()) {
out.submitType = str.type;
@ -547,7 +547,7 @@ public class ChangeJson {
}
}
if (in.getStatus().isOpen() && has(REVIEWED) && user.isIdentifiedUser()) {
if (in.isNew() && has(REVIEWED) && user.isIdentifiedUser()) {
out.reviewed = cd.isReviewedBy(user.getAccountId()) ? true : null;
}
@ -560,7 +560,7 @@ public class ChangeJson {
if (user.isIdentifiedUser()
&& (!limitToPsId.isPresent() || limitToPsId.get().equals(in.currentPatchSetId()))) {
out.permittedLabels =
cd.change().getStatus() != Change.Status.ABANDONED
!cd.change().isAbandoned()
? labelsJson.permittedLabels(user.getAccountId(), cd)
: ImmutableMap.of();
}

4
java/com/google/gerrit/server/change/ConsistencyChecker.java
View File

@ -368,12 +368,12 @@ public class ConsistencyChecker {
private void checkMergedBitMatchesStatus(PatchSet.Id psId, RevCommit commit, boolean merged) {
String refName = change().getDest().get();
if (merged && change().getStatus() != Change.Status.MERGED) {
if (merged && !change().isMerged()) {
ProblemInfo p = wrongChangeStatus(psId, commit);
if (fix != null) {
fixMerged(p);
}
} else if (!merged && change().getStatus() == Change.Status.MERGED) {
} else if (!merged && change().isMerged()) {
problem(
String.format(
"Patch set %d (%s) is not merged into"

10
java/com/google/gerrit/server/change/LabelsJson.java
View File

@ -41,7 +41,6 @@ import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.common.VotingRangeInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.account.AccountLoader;
@ -107,7 +106,7 @@ public class LabelsJson {
LabelTypes labelTypes = cd.getLabelTypes();
Map<String, LabelWithStatus> withStatus =
cd.change().getStatus() == Change.Status.MERGED
cd.change().isMerged()
? labelsForSubmittedChange(accountLoader, cd, labelTypes, standard, detailed)
: labelsForUnsubmittedChange(accountLoader, cd, labelTypes, standard, detailed);
return ImmutableMap.copyOf(Maps.transformValues(withStatus, LabelWithStatus::label));
@ -116,7 +115,7 @@ public class LabelsJson {
/** Returns all labels that the provided user has permission to vote on. */
Map<String, Collection<String>> permittedLabels(Account.Id filterApprovalsBy, ChangeData cd)
throws OrmException, PermissionBackendException {
boolean isMerged = cd.change().getStatus() == Change.Status.MERGED;
boolean isMerged = cd.change().isMerged();
LabelTypes labelTypes = cd.getLabelTypes();
Map<String, LabelType> toCheck = new HashMap<>();
for (SubmitRecord rec : submitRecords(cd)) {
@ -434,9 +433,10 @@ public class LabelsJson {
private void setAllApprovals(
AccountLoader accountLoader, ChangeData cd, Map<String, LabelWithStatus> labels)
throws OrmException, PermissionBackendException {
Change.Status status = cd.change().getStatus();
checkState(
status != Change.Status.MERGED, "should not call setAllApprovals on %s change", status);
!cd.change().isMerged(),
"should not call setAllApprovals on %s change",
cd.change().getStatus());
// Include a user in the output for this label if either:
// - They are an explicit reviewer.

2
java/com/google/gerrit/server/change/PatchSetInserter.java
View File

@ -194,7 +194,7 @@ public class PatchSetInserter implements BatchUpdateOp {
ChangeUpdate update = ctx.getUpdate(psId);
update.setSubjectForCommit("Create patch set " + psId.get());
if (!change.getStatus().isOpen() && !allowClosed) {
if (!change.isNew() && !allowClosed) {
throw new ResourceConflictException(
String.format(
"Cannot create new patch set of change %s because it is %s",

5
java/com/google/gerrit/server/change/RebaseChangeOp.java
View File

@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkState;
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.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.ChangeUtil;
@ -194,8 +193,8 @@ public class RebaseChangeOp implements BatchUpdateOp {
+ " was rebased");
}
if (base != null && base.notes().getChange().getStatus() != Change.Status.MERGED) {
if (base.notes().getChange().getStatus() != Change.Status.MERGED) {
if (base != null && !base.notes().getChange().isMerged()) {
if (!base.notes().getChange().isMerged()) {
// Add to end of relation chain for open base change.
patchSetInserter.setGroups(base.patchSet().getGroups());
} else {

5
java/com/google/gerrit/server/change/RebaseUtil.java
View File

@ -22,7 +22,6 @@ import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.PatchSetUtil;
@ -164,12 +163,12 @@ public class RebaseUtil {
continue;
}
Change depChange = cd.change();
if (depChange.getStatus() == Status.ABANDONED) {
if (depChange.isAbandoned()) {
throw new ResourceConflictException(
"Cannot rebase a change with an abandoned parent: " + depChange.getKey());
}
if (depChange.getStatus().isOpen()) {
if (depChange.isNew()) {
if (depPatchSet.getId().equals(depChange.currentPatchSetId())) {
throw new ResourceConflictException(
"Change is already based on the latest patch set of the dependent change.");

2
java/com/google/gerrit/server/edit/ChangeEditModifier.java
View File

@ -397,7 +397,7 @@ public class ChangeEditModifier {
}
Change c = notes.getChange();
if (!c.getStatus().isOpen()) {
if (!c.isNew()) {
throw new ResourceConflictException(
String.format(
"change %s is %s", c.getChangeId(), c.getStatus().toString().toLowerCase()));

2
java/com/google/gerrit/server/events/EventFactory.java
View File

@ -190,7 +190,7 @@ public class EventFactory {
*/
public void extend(ChangeAttribute a, Change change) {
a.lastUpdated = change.getLastUpdatedOn().getTime() / 1000L;
a.open = change.getStatus().isOpen();
a.open = change.isNew();
}
/**

3
java/com/google/gerrit/server/git/MergedByPushOp.java
View File

@ -125,8 +125,7 @@ public class MergedByPushOp implements BatchUpdateOp {
info = getPatchSetInfo(ctx);
ChangeUpdate update = ctx.getUpdate(psId);
Change.Status status = change.getStatus();
if (status == Change.Status.MERGED) {
if (change.isMerged()) {
return true;
}
change.setCurrentPatchSet(info);

4
java/com/google/gerrit/server/git/receive/ReceiveCommits.java
View File

@ -1975,7 +1975,7 @@ class ReceiveCommits {
*/
private boolean requestReplace(
ReceiveCommand cmd, boolean checkMergedInto, Change change, RevCommit newCommit) {
if (change.getStatus().isClosed()) {
if (change.isClosed()) {
reject(
cmd,
changeFormatter.changeClosed(
@ -2712,7 +2712,7 @@ class ReceiveCommits {
return false;
}
if (change.getStatus().isClosed()) {
if (change.isClosed()) {
reject(inputCommand, "change " + ontoChange + " closed");
return false;
} else if (revisions.containsKey(newCommit)) {

2
java/com/google/gerrit/server/git/receive/ReplaceOp.java
View File

@ -246,7 +246,7 @@ public class ReplaceOp implements BatchUpdateOp {
ConfigInvalidException {
notes = ctx.getNotes();
Change change = notes.getChange();
if (change == null || change.getStatus().isClosed()) {
if (change == null || change.isClosed()) {
rejectMessage = CHANGE_IS_CLOSED;
return false;
}

4
java/com/google/gerrit/server/permissions/ChangeControl.java
View File

@ -165,7 +165,7 @@ class ChangeControl {
/** Can this user edit the topic name? */
private boolean canEditTopicName() {
if (getChange().getStatus().isOpen()) {
if (getChange().isNew()) {
return isOwner() // owner (aka creator) of the change can edit topic
|| refControl.isOwner() // branch owner can edit topic
|| getProjectControl().isOwner() // project owner can edit topic
@ -178,7 +178,7 @@ class ChangeControl {
/** Can this user edit the description? */
private boolean canEditDescription() {
if (getChange().getStatus().isOpen()) {
if (getChange().isNew()) {
return isOwner() // owner (aka creator) of the change can edit desc
|| refControl.isOwner() // branch owner can edit desc
|| getProjectControl().isOwner() // project owner can edit desc

2
java/com/google/gerrit/server/project/RemoveReviewerControl.java
View File

@ -92,7 +92,7 @@ public class RemoveReviewerControl {
Account.Id reviewer,
int value)
throws PermissionBackendException {
if (change.getStatus().equals(Change.Status.MERGED)) {
if (change.isMerged()) {
return false;
}

2
java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
View File

@ -102,7 +102,7 @@ public class SubmitRuleEvaluator {
return ruleError("Error looking up change " + cd.getId(), e);
}
if (!opts.allowClosed() && change.getStatus().isClosed()) {
if (!opts.allowClosed() && change.isClosed()) {
SubmitRecord rec = new SubmitRecord();
rec.status = SubmitRecord.Status.CLOSED;
return Collections.singletonList(rec);

6
java/com/google/gerrit/server/query/change/ChangeData.java
View File

@ -173,7 +173,7 @@ public class ChangeData {
throws OrmException {
List<ChangeData> pending = new ArrayList<>();
for (ChangeData cd : changes) {
if (cd.reviewedBy == null && cd.change().getStatus().isOpen()) {
if (cd.reviewedBy == null && cd.change().isNew()) {
pending.add(cd);
}
}
@ -886,9 +886,9 @@ public class ChangeData {
if (c == null) {
return null;
}
if (c.getStatus() == Change.Status.MERGED) {
if (c.isMerged()) {
mergeable = true;
} else if (c.getStatus() == Change.Status.ABANDONED) {
} else if (c.isAbandoned()) {
return null;
} else if (c.isWorkInProgress()) {
return null;

2
java/com/google/gerrit/server/query/change/InternalChangeQuery.java
View File

@ -188,7 +188,7 @@ public class InternalChangeQuery extends InternalQuery<ChangeData, InternalChang
changeIds,
cn -> {
Change c = cn.getChange();
return c.getDest().equals(branch) && c.getStatus() != Change.Status.MERGED;
return c.getDest().equals(branch) && !c.isMerged();
});
return Lists.transform(notes, n -> changeDataFactory.create(n));
}

2
java/com/google/gerrit/server/restapi/change/Abandon.java
View File

@ -137,7 +137,7 @@ public class Abandon extends RetryingRestModifyView<ChangeResource, AbandonInput
.setVisible(false);
Change change = rsrc.getChange();
if (!change.getStatus().isOpen()) {
if (!change.isNew()) {
return description;
}

7
java/com/google/gerrit/server/restapi/change/CherryPickChange.java
View File

@ -31,7 +31,6 @@ import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.ApprovalsUtil;
@ -307,15 +306,15 @@ public class CherryPickChange {
}
Change change = changeDatas.get(0).change();
Change.Status status = change.getStatus();
if (status == Status.NEW || status == Status.MERGED) {
if (!change.isAbandoned()) {
// The base commit is a valid change revision.
return baseCommit;
}
throw new ResourceConflictException(
String.format(
"Change %s with commit %s is %s", change.getChangeId(), base, status.asChangeStatus()));
"Change %s with commit %s is %s",
change.getChangeId(), base, change.getStatus().asChangeStatus()));
}
private Change.Id insertPatchSet(

10
java/com/google/gerrit/server/restapi/change/DeleteChange.java
View File

@ -50,7 +50,7 @@ public class DeleteChange extends RetryingRestModifyView<ChangeResource, Input,
protected Response<?> applyImpl(
BatchUpdate.Factory updateFactory, ChangeResource rsrc, Input input)
throws RestApiException, UpdateException, PermissionBackendException {
if (!isChangeDeletable(rsrc.getChange().getStatus())) {
if (!isChangeDeletable(rsrc)) {
throw new MethodNotAllowedException("delete not permitted");
}
rsrc.permissions().check(ChangePermission.DELETE);
@ -66,16 +66,16 @@ public class DeleteChange extends RetryingRestModifyView<ChangeResource, Input,
@Override
public UiAction.Description getDescription(ChangeResource rsrc) {
Change.Status status = rsrc.getChange().getStatus();
PermissionBackend.ForChange perm = rsrc.permissions();
return new UiAction.Description()
.setLabel("Delete")
.setTitle("Delete change " + rsrc.getId())
.setVisible(and(isChangeDeletable(status), perm.testCond(ChangePermission.DELETE)));
.setVisible(and(isChangeDeletable(rsrc), perm.testCond(ChangePermission.DELETE)));
}
private static boolean isChangeDeletable(Change.Status status) {
if (status == Change.Status.MERGED) {
private static boolean isChangeDeletable(ChangeResource rsrc) {
Change change = rsrc.getChange();
if (change.isMerged()) {
// Merged changes should never be deleted.
return false;
}

3
java/com/google/gerrit/server/restapi/change/DeleteChangeOp.java
View File

@ -83,8 +83,7 @@ public class DeleteChangeOp implements BatchUpdateOp {
private void ensureDeletable(ChangeContext ctx, Change.Id id, Collection<PatchSet> patchSets)
throws ResourceConflictException, MethodNotAllowedException, IOException {
Change.Status status = ctx.getChange().getStatus();
if (status == Change.Status.MERGED) {
if (ctx.getChange().isMerged()) {
throw new MethodNotAllowedException("Deleting merged change " + id + " is not allowed");
}
for (PatchSet patchSet : patchSets) {

2
java/com/google/gerrit/server/restapi/change/Mergeable.java
View File

@ -96,7 +96,7 @@ public class Mergeable implements RestReadView<RevisionResource> {
PatchSet ps = resource.getPatchSet();
MergeableInfo result = new MergeableInfo();
if (!change.getStatus().isOpen()) {
if (!change.isNew()) {
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
} else if (!ps.getId().equals(change.currentPatchSetId())) {
// Only the current revision is mergeable. Others always fail.

7
java/com/google/gerrit/server/restapi/change/Move.java
View File

@ -33,7 +33,6 @@ import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.LabelId;
import com.google.gerrit.reviewdb.client.PatchSet;
@ -133,7 +132,7 @@ public class Move extends RetryingRestModifyView<ChangeResource, MoveInput, Chan
}
input.destinationBranch = RefNames.fullName(input.destinationBranch);
if (change.getStatus().isClosed()) {
if (!change.isNew()) {
throw new ResourceConflictException("Change is " + ChangeUtil.status(change));
}
@ -181,7 +180,7 @@ public class Move extends RetryingRestModifyView<ChangeResource, MoveInput, Chan
public boolean updateChange(ChangeContext ctx)
throws OrmException, ResourceConflictException, IOException {
change = ctx.getChange();
if (change.getStatus() != Status.NEW) {
if (!change.isNew()) {
throw new ResourceConflictException("Change is " + ChangeUtil.status(change));
}
@ -293,7 +292,7 @@ public class Move extends RetryingRestModifyView<ChangeResource, MoveInput, Chan
.setVisible(false);
Change change = rsrc.getChange();
if (!change.getStatus().isOpen()) {
if (!change.isNew()) {
return description;
}

2
java/com/google/gerrit/server/restapi/change/PostPrivate.java
View File

@ -98,7 +98,7 @@ public class PostPrivate
private BooleanCondition canSetPrivate(ChangeResource rsrc) {
PermissionBackend.WithUser user = permissionBackend.user(rsrc.getUser());
return or(
rsrc.isUserOwner() && rsrc.getChange().getStatus() != Change.Status.MERGED,
rsrc.isUserOwner() && !rsrc.getChange().isMerged(),
user.testCond(GlobalPermission.ADMINISTRATE_SERVER));
}
}

6
java/com/google/gerrit/server/restapi/change/PostReview.java
View File

@ -1122,7 +1122,7 @@ public class PostReview
// If no labels were modified and change is closed, abort early.
// This avoids trying to record a modified label caused by a user
// losing access to a label after the change was submitted.
if (inLabels.isEmpty() && ctx.getChange().getStatus().isClosed()) {
if (inLabels.isEmpty() && ctx.getChange().isClosed()) {
return false;
}
@ -1201,11 +1201,11 @@ public class PostReview
List<PatchSetApproval> ups,
List<PatchSetApproval> del)
throws ResourceConflictException {
if (ctx.getChange().getStatus().isOpen()) {
if (ctx.getChange().isNew()) {
return; // Not closed, nothing to validate.
} else if (del.isEmpty() && ups.isEmpty()) {
return; // No new votes.
} else if (ctx.getChange().getStatus() != Change.Status.MERGED) {
} else if (!ctx.getChange().isMerged()) {
throw new ResourceConflictException("change is closed");
}

2
java/com/google/gerrit/server/restapi/change/PreviewSubmit.java
View File

@ -99,7 +99,7 @@ public class PreviewSubmit implements RestReadView<RevisionResource> {
}
Change change = rsrc.getChange();
if (!change.getStatus().isOpen()) {
if (!change.isNew()) {
throw new PreconditionFailedException("change is " + ChangeUtil.status(change));
}
if (!rsrc.getUser().isIdentifiedUser()) {

7
java/com/google/gerrit/server/restapi/change/Rebase.java
View File

@ -27,7 +27,6 @@ import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.PatchSetUtil;
@ -115,7 +114,7 @@ public class Rebase extends RetryingRestModifyView<RevisionResource, RebaseInput
RevWalk rw = new RevWalk(reader);
BatchUpdate bu =
updateFactory.create(change.getProject(), rsrc.getUser(), TimeUtil.nowTs())) {
if (!change.getStatus().isOpen()) {
if (!change.isNew()) {
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
} else if (!hasOneParent(rw, rsrc.getPatchSet())) {
throw new ResourceConflictException(
@ -175,7 +174,7 @@ public class Rebase extends RetryingRestModifyView<RevisionResource, RebaseInput
} else if (!baseChange.getDest().equals(change.getDest())) {
throw new ResourceConflictException(
"base change is targeting wrong branch: " + baseChange.getDest());
} else if (baseChange.getStatus() == Status.ABANDONED) {
} else if (baseChange.isAbandoned()) {
throw new ResourceConflictException("base change is abandoned: " + baseChange.getKey());
} else if (isMergedInto(rw, rsrc.getPatchSet(), base.patchSet())) {
throw new ResourceConflictException(
@ -207,7 +206,7 @@ public class Rebase extends RetryingRestModifyView<RevisionResource, RebaseInput
.setVisible(false);
Change change = rsrc.getChange();
if (!(change.getStatus().isOpen() && rsrc.isCurrent())) {
if (!(change.isNew() && rsrc.isCurrent())) {
return description;
}

4
java/com/google/gerrit/server/restapi/change/Restore.java
View File

@ -113,7 +113,7 @@ public class Restore extends RetryingRestModifyView<ChangeResource, RestoreInput
@Override
public boolean updateChange(ChangeContext ctx) throws OrmException, ResourceConflictException {
change = ctx.getChange();
if (change == null || change.getStatus() != Status.ABANDONED) {
if (change == null || !change.isAbandoned()) {
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
}
PatchSet.Id psId = change.currentPatchSetId();
@ -162,7 +162,7 @@ public class Restore extends RetryingRestModifyView<ChangeResource, RestoreInput
.setVisible(false);
Change change = rsrc.getChange();
if (change.getStatus() != Status.ABANDONED) {
if (!change.isAbandoned()) {
return description;
}

4
java/com/google/gerrit/server/restapi/change/Revert.java
View File

@ -145,7 +145,7 @@ public class Revert extends RetryingRestModifyView<ChangeResource, RevertInput,
throws IOException, OrmException, RestApiException, UpdateException, NoSuchChangeException,
PermissionBackendException, NoSuchProjectException, ConfigInvalidException {
Change change = rsrc.getChange();
if (change.getStatus() != Change.Status.MERGED) {
if (!change.isMerged()) {
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
}
@ -265,7 +265,7 @@ public class Revert extends RetryingRestModifyView<ChangeResource, RevertInput,
.setTitle("Revert the change")
.setVisible(
and(
change.getStatus() == Change.Status.MERGED && projectStatePermitsWrite,
change.isMerged() && projectStatePermitsWrite,
permissionBackend
.user(rsrc.getUser())
.ref(change.getDest())

5
java/com/google/gerrit/server/restapi/change/SetReadyForReview.java
View File

@ -24,7 +24,6 @@ import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.change.ChangeResource;
@ -70,7 +69,7 @@ public class SetReadyForReview extends RetryingRestModifyView<ChangeResource, In
WorkInProgressOp.checkPermissions(permissionBackend, user.get(), rsrc.getChange());
Change change = rsrc.getChange();
if (change.getStatus() != Status.NEW) {
if (!change.isNew()) {
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
}
@ -94,7 +93,7 @@ public class SetReadyForReview extends RetryingRestModifyView<ChangeResource, In
.setTitle("Set Ready For Review")
.setVisible(
and(
rsrc.getChange().getStatus() == Status.NEW && rsrc.getChange().isWorkInProgress(),
rsrc.getChange().isNew() && rsrc.getChange().isWorkInProgress(),
or(
rsrc.isUserOwner(),
or(

5
java/com/google/gerrit/server/restapi/change/SetWorkInProgress.java
View File

@ -24,7 +24,6 @@ import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.change.ChangeResource;
@ -70,7 +69,7 @@ public class SetWorkInProgress extends RetryingRestModifyView<ChangeResource, In
WorkInProgressOp.checkPermissions(permissionBackend, user.get(), rsrc.getChange());
Change change = rsrc.getChange();
if (change.getStatus() != Status.NEW) {
if (!change.isNew()) {
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
}
@ -94,7 +93,7 @@ public class SetWorkInProgress extends RetryingRestModifyView<ChangeResource, In
.setTitle("Set Work In Progress")
.setVisible(
and(
rsrc.getChange().getStatus() == Status.NEW && !rsrc.getChange().isWorkInProgress(),
rsrc.getChange().isNew() && !rsrc.getChange().isWorkInProgress(),
or(
rsrc.isUserOwner(),
or(

19
java/com/google/gerrit/server/restapi/change/Submit.java
View File

@ -195,7 +195,7 @@ public class Submit
throws OrmException, RestApiException, IOException, UpdateException, ConfigInvalidException,
PermissionBackendException {
Change change = rsrc.getChange();
if (!change.getStatus().isOpen()) {
if (!change.isNew()) {
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
} else if (!ProjectUtil.branchExists(repoManager, change.getDest())) {
throw new ResourceConflictException(
@ -216,16 +216,13 @@ public class Submit
}
}
switch (change.getStatus()) {
case MERGED:
return change;
case NEW:
throw new RestApiException(
"change unexpectedly had status " + change.getStatus() + " after submit attempt");
case ABANDONED:
default:
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
if (change.isMerged()) {
return change;
}
if (change.isNew()) {
throw new RestApiException("change unexpectedly had status NEW after submit attempt");
}
throw new ResourceConflictException("change is " + ChangeUtil.status(change));
}
/**
@ -300,7 +297,7 @@ public class Submit
@Override
public UiAction.Description getDescription(RevisionResource resource) {
Change change = resource.getChange();
if (!change.getStatus().isOpen()
if (!change.isNew()
|| change.isWorkInProgress()
|| !resource.isCurrent()
|| !resource.permissions().testOrFalse(ChangePermission.SUBMIT)) {

5
java/com/google/gerrit/server/restapi/change/SubmittedTogether.java
View File

@ -21,7 +21,6 @@ import static java.util.stream.Collectors.toList;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.extensions.api.changes.SubmittedTogetherInfo;
import com.google.gerrit.extensions.api.changes.SubmittedTogetherOption;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
@ -124,11 +123,11 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
List<ChangeData> cds;
int hidden;
if (c.getStatus().isOpen()) {
if (c.isNew()) {
ChangeSet cs = mergeSuperSet.get().completeChangeSet(c, resource.getUser());
cds = ensureRequiredDataIsLoaded(cs.changes().asList());
hidden = cs.nonVisibleChanges().size();
} else if (c.getStatus().asChangeStatus() == ChangeStatus.MERGED) {
} else if (c.isMerged()) {
cds = queryProvider.get().bySubmissionId(c.getSubmissionId());
hidden = 0;
} else {

2
java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
View File

@ -159,7 +159,7 @@ public class PrologRuleEvaluator {
return ruleError("Error looking up change " + cd.getId(), e);
}
if (!opts.allowClosed() && change.getStatus().isClosed()) {
if (!opts.allowClosed() && change.isClosed()) {
SubmitRecord rec = new SubmitRecord();
rec.status = SubmitRecord.Status.CLOSED;
return Collections.singletonList(rec);

7
java/com/google/gerrit/server/submit/MergeOp.java
View File

@ -383,9 +383,8 @@ public class MergeOp implements AutoCloseable {
!cs.furtherHiddenChanges(), "checkSubmitRulesAndState called for topic with hidden change");
for (ChangeData cd : cs.changes()) {
try {
Change.Status status = cd.change().getStatus();
if (status != Change.Status.NEW) {
if (!(status == Change.Status.MERGED && allowMerged)) {
if (!cd.change().isNew()) {
if (!(cd.change().isMerged() && allowMerged)) {
commitStatus.problem(
cd.getId(),
"Change " + cd.getId() + " is " + cd.change().getStatus().toString().toLowerCase());
@ -906,7 +905,7 @@ public class MergeOp implements AutoCloseable {
@Override
public boolean updateChange(ChangeContext ctx) {
Change change = ctx.getChange();
if (!change.getStatus().isOpen()) {
if (!change.isNew()) {
return false;
}

4
java/com/google/gerrit/server/submit/RebaseSorter.java
View File

@ -16,7 +16,6 @@ package com.google.gerrit.server.submit;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
@ -126,8 +125,7 @@ public class RebaseSorter {
// check if the commit associated change is merged in the same branch
List<ChangeData> changes = queryProvider.get().byCommit(commit);
for (ChangeData change : changes) {
if (change.change().getStatus() == Status.MERGED
&& change.change().getDest().equals(dest)) {
if (change.change().isMerged() && change.change().getDest().equals(dest)) {
logger.atFine().log(
"Dependency %s associated with merged change %s.", commit.getName(), change.getId());
return true;

2
java/com/google/gerrit/server/submit/SubmitStrategyOp.java
View File

@ -215,7 +215,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
"%s#updateChange for change %s", getClass().getSimpleName(), toMerge.change().getId());
toMerge.setNotes(ctx.getNotes()); // Update change and notes from ctx.
if (ctx.getChange().getStatus() == Change.Status.MERGED) {
if (ctx.getChange().isMerged()) {
// Either another thread won a race, or we are retrying a whole topic submission after one
// repo failed with lock failure.
if (alreadyMergedCommit == null) {

6
javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
View File

@ -1783,7 +1783,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
// Added a new patch set and auto-closed the change.
cd = byChangeId(id);
assertThat(cd.change().getStatus()).isEqualTo(Change.Status.MERGED);
assertThat(cd.change().isMerged()).isTrue();
assertThat(getPatchSetRevisions(cd))
.containsExactlyEntriesIn(
ImmutableMap.of(1, ps1Rev, 2, testRepo.getRepository().resolve("HEAD").name()));
@ -1801,7 +1801,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
// Change not updated.
cd = byChangeId(id);
assertThat(cd.change().getStatus()).isEqualTo(Change.Status.NEW);
assertThat(cd.change().isNew()).isTrue();
assertThat(getPatchSetRevisions(cd)).containsExactlyEntriesIn(ImmutableMap.of(1, ps1Rev));
}
@ -1851,7 +1851,7 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
testRepo.reset(ps2Commit);
ChangeData cd = byCommit(ps1Commit);
assertThat(cd.change().getStatus()).isEqualTo(Change.Status.NEW);
assertThat(cd.change().isNew()).isTrue();
assertThat(getPatchSetRevisions(cd))
.containsExactlyEntriesIn(ImmutableMap.of(1, ps1Commit.name()));
return c.getId();

14
javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
View File

@ -160,7 +160,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
RevCommit c = r.getCommit();
PatchSet.Id psId = cd.currentPatchSet().getId();
assertThat(psId.get()).isEqualTo(1);
assertThat(cd.change().getStatus()).isEqualTo(Change.Status.MERGED);
assertThat(cd.change().isMerged()).isTrue();
assertSubmitApproval(psId);
assertThat(cd.patchSets()).hasSize(1);
@ -184,7 +184,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
assertCommit(project, master);
ChangeData cd =
Iterables.getOnlyElement(queryProvider.get().byKey(new Change.Key(r.getChangeId())));
assertThat(cd.change().getStatus()).isEqualTo(Change.Status.MERGED);
assertThat(cd.change().isMerged()).isTrue();
RemoteRefUpdate.Status status = pushCommitTo(commit, "refs/for/other");
assertThat(status).isEqualTo(RemoteRefUpdate.Status.OK);
@ -194,7 +194,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
for (ChangeData c : queryProvider.get().byKey(new Change.Key(r.getChangeId()))) {
if (c.change().getDest().get().equals(other)) {
assertThat(c.change().getStatus()).isEqualTo(Change.Status.MERGED);
assertThat(c.change().isMerged()).isTrue();
}
}
}
@ -230,7 +230,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
ChangeData cd = r.getChange();
RevCommit c2 = r.getCommit();
assertThat(cd.change().getStatus()).isEqualTo(Change.Status.MERGED);
assertThat(cd.change().isMerged()).isTrue();
PatchSet.Id psId2 = cd.change().currentPatchSetId();
assertThat(psId2.get()).isEqualTo(2);
assertCommit(project, "refs/heads/master");
@ -262,7 +262,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
cd = changeDataFactory.create(project, psId1.getParentKey());
Change c = cd.change();
assertThat(c.getStatus()).isEqualTo(Change.Status.MERGED);
assertThat(c.isMerged()).isTrue();
assertThat(c.currentPatchSetId()).isEqualTo(psId1);
assertThat(cd.patchSets().stream().map(PatchSet::getId).collect(toList()))
.containsExactly(psId1, psId2);
@ -301,14 +301,14 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
assertPushOk(pushHead(testRepo, "refs/heads/master", false), "refs/heads/master");
ChangeData cd2 = r2.getChange();
assertThat(cd2.change().getStatus()).isEqualTo(Change.Status.MERGED);
assertThat(cd2.change().isMerged()).isTrue();
PatchSet.Id psId2_2 = cd2.change().currentPatchSetId();
assertThat(psId2_2.get()).isEqualTo(2);
assertThat(cd2.patchSet(psId2_1).getRevision().get()).isEqualTo(c2_1.name());
assertThat(cd2.patchSet(psId2_2).getRevision().get()).isEqualTo(c2_2.name());
ChangeData cd1 = r1.getChange();
assertThat(cd1.change().getStatus()).isEqualTo(Change.Status.MERGED);
assertThat(cd1.change().isMerged()).isTrue();
PatchSet.Id psId1_2 = cd1.change().currentPatchSetId();
assertThat(psId1_2.get()).isEqualTo(2);
assertThat(cd1.patchSet(psId1_1).getRevision().get()).isEqualTo(c1_1.name());

3
javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
View File

@ -50,7 +50,6 @@ import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.Patch;
@ -336,7 +335,7 @@ public class ImpersonationIT extends AbstractDaemonTest {
gApi.changes().id(changeId).current().submit(in);
ChangeData cd = r.getChange();
assertThat(cd.change().getStatus()).isEqualTo(Change.Status.MERGED);
assertThat(cd.change().isMerged()).isTrue();
PatchSetApproval submitter =
approvalsUtil.getSubmitter(cd.notes(), cd.change().currentPatchSetId());
assertThat(submitter.getAccountId()).isEqualTo(admin2.id);

10
javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
View File

@ -379,7 +379,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
"Marked change as merged"));
notes = reload(notes);
assertThat(notes.getChange().getStatus()).isEqualTo(Change.Status.MERGED);
assertThat(notes.getChange().isMerged()).isTrue();
assertNoProblems(notes, null);
}
@ -422,7 +422,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
"Marked change as merged"));
notes = reload(notes);
assertThat(notes.getChange().getStatus()).isEqualTo(Change.Status.MERGED);
assertThat(notes.getChange().isMerged()).isTrue();
assertNoProblems(notes, null);
}
@ -572,7 +572,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
notes = reload(notes);
PatchSet.Id psId3 = new PatchSet.Id(notes.getChangeId(), 3);
assertThat(notes.getChange().currentPatchSetId()).isEqualTo(psId3);
assertThat(notes.getChange().getStatus()).isEqualTo(Change.Status.MERGED);
assertThat(notes.getChange().isMerged()).isTrue();
assertThat(psUtil.byChangeAsMap(notes).keySet()).containsExactly(ps2.getId(), psId3);
assertThat(psUtil.get(notes, psId3).getRevision().get()).isEqualTo(rev1);
}
@ -620,7 +620,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
notes = reload(notes);
PatchSet.Id psId4 = new PatchSet.Id(notes.getChangeId(), 4);
assertThat(notes.getChange().currentPatchSetId()).isEqualTo(psId4);
assertThat(notes.getChange().getStatus()).isEqualTo(Change.Status.MERGED);
assertThat(notes.getChange().isMerged()).isTrue();
assertThat(psUtil.byChangeAsMap(notes).keySet())
.containsExactly(ps1.getId(), ps3.getId(), psId4);
assertThat(psUtil.get(notes, psId4).getRevision().get()).isEqualTo(rev2);
@ -657,7 +657,7 @@ public class ConsistencyCheckerIT extends AbstractDaemonTest {
notes = reload(notes);
assertThat(notes.getChange().currentPatchSetId()).isEqualTo(psId2);
assertThat(notes.getChange().getStatus()).isEqualTo(Change.Status.MERGED);
assertThat(notes.getChange().isMerged()).isTrue();
assertThat(psUtil.byChangeAsMap(notes).keySet()).containsExactly(ps1.getId(), psId2);
assertThat(psUtil.get(notes, psId2).getRevision().get()).isEqualTo(rev2);
}

2
javatests/com/google/gerrit/reviewdb/converter/ChangeProtoConverterTest.java
View File

@ -253,7 +253,7 @@ public class ChangeProtoConverterTest {
assertThat(change.currentPatchSetId()).isNull();
// Default values for unset protobuf fields which can't be unset in the entity object.
assertThat(change.getRowVersion()).isEqualTo(0);
assertThat(change.getStatus()).isEqualTo(Change.Status.NEW);
assertThat(change.isNew()).isTrue();
assertThat(change.isPrivate()).isFalse();
assertThat(change.isWorkInProgress()).isFalse();
assertThat(change.hasReviewStarted()).isFalse();

2
plugins/reviewnotes

@ -1 +1 @@
Subproject commit a9456bfdb862dfa7197583decac3c22149ae8109
Subproject commit d7ba4997b5c0038c85f6393b73a9ca16ca4fb927