Remove the state SUBMITTED altogether

There is a new schema to move all submitted changes to new.

Change-Id: I9f3b015a59fc2446a98a03866330b0d4e22bbf3d
This commit is contained in:
Stefan Beller
2015-07-10 13:32:57 -07:00
parent 7063a4b8f0
commit 0d3cab0634
19 changed files with 64 additions and 108 deletions

View File

@@ -43,9 +43,6 @@ status:: Current state of this change.
DRAFT;; Change is a draft change that only consists of draft patchsets. DRAFT;; Change is a draft change that only consists of draft patchsets.
SUBMITTED;; Change has been submitted and is in the merge queue.
It may be waiting for one or more dependencies.
MERGED;; Change has been merged to its branch. MERGED;; Change has been merged to its branch.
ABANDONED;; Change was abandoned by its owner or administrator. ABANDONED;; Change was abandoned by its owner or administrator.

View File

@@ -3756,8 +3756,7 @@ The `refs/heads/` prefix is omitted.
|`subject` || |`subject` ||
The subject of the change (header line of the commit message). The subject of the change (header line of the commit message).
|`status` || |`status` ||
The status of the change (`NEW`, `SUBMITTED`, `MERGED`, `ABANDONED`, The status of the change (`NEW`, `MERGED`, `ABANDONED`, `DRAFT`).
`DRAFT`).
|`created` || |`created` ||
The link:rest-api.html#timestamp[timestamp] of when the change was The link:rest-api.html#timestamp[timestamp] of when the change was
created. created.
@@ -4322,7 +4321,7 @@ link:#commit-info[CommitInfo] entity.
|`_revision_number` |optional|The revision number. |`_revision_number` |optional|The revision number.
|`_current_revision_number`|optional|The current revision number. |`_current_revision_number`|optional|The current revision number.
|`status` |optional|The status of the change. The status of |`status` |optional|The status of the change. The status of
the change is one of (`NEW`, `SUBMITTED`, `MERGED`, `ABANDONED`, `DRAFT`). the change is one of (`NEW`, `MERGED`, `ABANDONED`, `DRAFT`).
|=========================== |===========================
[[related-changes-info]] [[related-changes-info]]
@@ -4530,8 +4529,8 @@ after submitting.
|========================== |==========================
|Field Name ||Description |Field Name ||Description
|`status` || |`status` ||
The status of the change after submitting, can be `MERGED` or The status of the change after submitting is `MERGED`.
`SUBMITTED`.+ +
As `wait_for_merge` in the link:#submit-input[SubmitInput] is deprecated and As `wait_for_merge` in the link:#submit-input[SubmitInput] is deprecated and
the request always waits for the merge to be completed, you can expect the request always waits for the merge to be completed, you can expect
`MERGED` to be returned here. `MERGED` to be returned here.

View File

@@ -56,7 +56,7 @@ public class CreateChangeIT extends AbstractDaemonTest {
@Test @Test
public void createEmptyChange_InvalidStatus() throws Exception { public void createEmptyChange_InvalidStatus() throws Exception {
ChangeInfo ci = newChangeInfo(ChangeStatus.SUBMITTED); ChangeInfo ci = newChangeInfo(ChangeStatus.MERGED);
assertCreateFails(ci, BadRequestException.class, assertCreateFails(ci, BadRequestException.class,
"unsupported change status"); "unsupported change status");
} }

View File

@@ -135,7 +135,6 @@ public class PageLinks {
case MERGED: case MERGED:
return "status:merged"; return "status:merged";
case NEW: case NEW:
case SUBMITTED:
default: default:
return "status:open"; return "status:open";
} }

View File

@@ -28,39 +28,12 @@ public enum ChangeStatus {
* <p> * <p>
* Changes in the NEW state can be moved to: * Changes in the NEW state can be moved to:
* <ul> * <ul>
* <li>{@link #SUBMITTED} - when the Submit Patch Set action is used; * <li>{@link #MERGED} - when the Submit Patch Set action is used;
* <li>{@link #ABANDONED} - when the Abandon action is used. * <li>{@link #ABANDONED} - when the Abandon action is used.
* </ul> * </ul>
*/ */
NEW, NEW,
/**
* Change is open, but has been submitted to the merge queue.
*
* <p>
* A change enters the SUBMITTED state when an authorized user presses the
* "submit" action through the web UI, requesting that Gerrit merge the
* change's current patch set into the destination branch.
*
* <p>
* Typically a change resides in the SUBMITTED for only a brief sub-second
* period while the merge queue fires and the destination branch is updated.
* However, if a dependency commit (directly or transitively) is not yet
* merged into the branch, the change will hang in the SUBMITTED state
* indefinitely.
*
* <p>
* Changes in the SUBMITTED state can be moved to:
* <ul>
* <li>{@link #NEW} - when a replacement patch set is supplied, OR when a
* merge conflict is detected;
* <li>{@link #MERGED} - when the change has been successfully merged into
* the destination branch;
* <li>{@link #ABANDONED} - when the Abandon action is used.
* </ul>
*/
SUBMITTED,
/** /**
* Change is a draft change that only consists of draft patchsets. * Change is a draft change that only consists of draft patchsets.
* *

View File

@@ -18,7 +18,6 @@ import com.google.gwt.i18n.client.Constants;
public interface ChangeConstants extends Constants { public interface ChangeConstants extends Constants {
String statusLongNew(); String statusLongNew();
String statusLongSubmitted();
String statusLongMerged(); String statusLongMerged();
String statusLongAbandoned(); String statusLongAbandoned();
String statusLongDraft(); String statusLongDraft();

View File

@@ -1,5 +1,4 @@
statusLongNew = Review in Progress statusLongNew = Review in Progress
statusLongSubmitted = Submitted, Merge Pending
statusLongMerged = Merged statusLongMerged = Merged
statusLongAbandoned = Abandoned statusLongAbandoned = Abandoned
statusLongDraft = Draft statusLongDraft = Draft

View File

@@ -34,8 +34,6 @@ public class Util {
return C.statusLongDraft(); return C.statusLongDraft();
case NEW: case NEW:
return C.statusLongNew(); return C.statusLongNew();
case SUBMITTED:
return C.statusLongSubmitted();
case MERGED: case MERGED:
return C.statusLongMerged(); return C.statusLongMerged();
case ABANDONED: case ABANDONED:

View File

@@ -251,8 +251,6 @@ public final class Change {
private static final char MIN_OPEN = 'a'; private static final char MIN_OPEN = 'a';
/** Database constant for {@link Status#NEW}. */ /** Database constant for {@link Status#NEW}. */
public static final char STATUS_NEW = 'n'; public static final char STATUS_NEW = 'n';
/** Database constant for {@link Status#SUBMITTED}. */
public static final char STATUS_SUBMITTED = 's';
/** Database constant for {@link Status#DRAFT}. */ /** Database constant for {@link Status#DRAFT}. */
public static final char STATUS_DRAFT = 'd'; public static final char STATUS_DRAFT = 'd';
/** Maximum database status constant for an open change. */ /** Maximum database status constant for an open change. */
@@ -285,39 +283,12 @@ public final class Change {
* <p> * <p>
* Changes in the NEW state can be moved to: * Changes in the NEW state can be moved to:
* <ul> * <ul>
* <li>{@link #SUBMITTED} - when the Submit Patch Set action is used; * <li>{@link #MERGED} - when the Submit Patch Set action is used;
* <li>{@link #ABANDONED} - when the Abandon action is used. * <li>{@link #ABANDONED} - when the Abandon action is used.
* </ul> * </ul>
*/ */
NEW(STATUS_NEW, ChangeStatus.NEW), NEW(STATUS_NEW, ChangeStatus.NEW),
/**
* Change is open, but has been submitted to the merge queue.
*
* <p>
* A change enters the SUBMITTED state when an authorized user presses the
* "submit" action through the web UI, requesting that Gerrit merge the
* change's current patch set into the destination branch.
*
* <p>
* Typically a change resides in the SUBMITTED for only a brief sub-second
* period while the merge queue fires and the destination branch is updated.
* However, if a dependency commit (a {@link PatchSetAncestor}, directly or
* transitively) is not yet merged into the branch, the change will hang in
* the SUBMITTED state indefinitely.
*
* <p>
* Changes in the SUBMITTED state can be moved to:
* <ul>
* <li>{@link #NEW} - when a replacement patch set is supplied, OR when a
* merge conflict is detected;
* <li>{@link #MERGED} - when the change has been successfully merged into
* the destination branch;
* <li>{@link #ABANDONED} - when the Abandon action is used.
* </ul>
*/
SUBMITTED(STATUS_SUBMITTED, ChangeStatus.SUBMITTED),
/** /**
* Change is a draft change that only consists of draft patchsets. * Change is a draft change that only consists of draft patchsets.
* *

View File

@@ -88,16 +88,10 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
private static final String CLICK_FAILURE_TOOLTIP = private static final String CLICK_FAILURE_TOOLTIP =
"Clicking the button would fail."; "Clicking the button would fail.";
public enum Status {
SUBMITTED, MERGED
}
public static class Output { public static class Output {
public Status status;
transient Change change; transient Change change;
private Output(Status s, Change c) { private Output(Change c) {
status = s;
change = c; change = c;
} }
} }
@@ -198,10 +192,8 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
throw new ResourceConflictException("change is deleted"); throw new ResourceConflictException("change is deleted");
} }
switch (change.getStatus()) { switch (change.getStatus()) {
case SUBMITTED:
return new Output(Status.SUBMITTED, change);
case MERGED: case MERGED:
return new Output(Status.MERGED, change); return new Output(change);
case NEW: case NEW:
ChangeMessage msg = getConflictMessage(rsrc); ChangeMessage msg = getConflictMessage(rsrc);
if (msg != null) { if (msg != null) {

View File

@@ -388,8 +388,7 @@ public class MergeOp {
throws ResourceConflictException, OrmException { throws ResourceConflictException, OrmException {
for (Change.Id id : cs.ids()) { for (Change.Id id : cs.ids()) {
ChangeData cd = changeDataFactory.create(db, id); ChangeData cd = changeDataFactory.create(db, id);
if (cd.change().getStatus() != Change.Status.NEW if (cd.change().getStatus() != Change.Status.NEW){
&& cd.change().getStatus() != Change.Status.SUBMITTED){
throw new OrmException("Change " + cd.change().getChangeId() throw new OrmException("Change " + cd.change().getChangeId()
+ " is in state " + cd.change().getStatus()); + " is in state " + cd.change().getStatus());
} else { } else {
@@ -654,9 +653,8 @@ public class MergeOp {
throw new MergeException("Failed to validate changes", e); throw new MergeException("Failed to validate changes", e);
} }
Change.Id changeId = cd.getId(); Change.Id changeId = cd.getId();
if (chg.getStatus() != Change.Status.SUBMITTED if (chg.getStatus() != Change.Status.NEW) {
&& chg.getStatus() != Change.Status.NEW) { logDebug("Change {} is not new: {}", changeId, chg.getStatus());
logDebug("Change {} is not new or submitted: {}", changeId, chg.getStatus());
continue; continue;
} }
if (chg.currentPatchSetId() == null) { if (chg.currentPatchSetId() == null) {
@@ -1093,7 +1091,7 @@ public class MergeOp {
ChangeUpdate update = updateFactory.create(control, timestamp); ChangeUpdate update = updateFactory.create(control, timestamp);
List<SubmitRecord> record = records.get(cd.change().getId()); List<SubmitRecord> record = records.get(cd.change().getId());
if (record != null) { if (record != null) {
update.submit(record); update.merge(record);
} }
db.changes().beginTransaction(cd.change().getId()); db.changes().beginTransaction(cd.change().getId());
try { try {

View File

@@ -1796,9 +1796,6 @@ public class ReceiveCommits {
for (Change c : changes) { for (Change c : changes) {
c = db.changes().get(c.getId()); c = db.changes().get(c.getId());
switch (c.getStatus()) { switch (c.getStatus()) {
case SUBMITTED:
addMessage("Change " + c.getChangeId() + " submitted.");
break;
case MERGED: case MERGED:
addMessage("Change " + c.getChangeId() + " merged."); addMessage("Change " + c.getChangeId() + " merged.");
break; break;

View File

@@ -177,7 +177,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
approvals.put(label, Optional.<Short> absent()); approvals.put(label, Optional.<Short> absent());
} }
public void submit(Iterable<SubmitRecord> submitRecords) { public void merge(Iterable<SubmitRecord> submitRecords) {
this.status = Change.Status.MERGED; this.status = Change.Status.MERGED;
this.submitRecords = ImmutableList.copyOf(submitRecords); this.submitRecords = ImmutableList.copyOf(submitRecords);
checkArgument(!this.submitRecords.isEmpty(), checkArgument(!this.submitRecords.isEmpty(),

View File

@@ -108,10 +108,6 @@ public class InternalChangeQuery {
return query(project(project)); return query(project(project));
} }
public List<ChangeData> allSubmitted() throws OrmException {
return query(status(Change.Status.SUBMITTED));
}
public List<ChangeData> byBranchOpen(Branch.NameKey branch) public List<ChangeData> byBranchOpen(Branch.NameKey branch)
throws OrmException { throws OrmException {
return query(and( return query(and(

View File

@@ -32,7 +32,7 @@ import java.util.List;
/** A version of the database schema. */ /** A version of the database schema. */
public abstract class SchemaVersion { public abstract class SchemaVersion {
/** The current schema version. */ /** The current schema version. */
public static final Class<Schema_108> C = Schema_108.class; public static final Class<Schema_109> C = Schema_109.class;
public static int getBinaryVersion() { public static int getBinaryVersion() {
return guessVersion(C); return guessVersion(C);

View File

@@ -0,0 +1,39 @@
// Copyright (C) 2015 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.schema;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.StatementExecutor;
import com.google.inject.Inject;
import com.google.inject.Provider;
public class Schema_109 extends SchemaVersion {
@Inject
Schema_109(Provider<Schema_108> prior) {
super(prior);
}
@Override
protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException {
String cmd = "UPDATE changes SET status = 'n' WHERE status = 's';";
ui.message("Running " + cmd);
try (StatementExecutor e = newExecutor(db)) {
e.execute(cmd);
}
ui.message("done");
}
}

View File

@@ -19,7 +19,6 @@ import static com.google.gerrit.reviewdb.client.Change.Status.ABANDONED;
import static com.google.gerrit.reviewdb.client.Change.Status.DRAFT; import static com.google.gerrit.reviewdb.client.Change.Status.DRAFT;
import static com.google.gerrit.reviewdb.client.Change.Status.MERGED; import static com.google.gerrit.reviewdb.client.Change.Status.MERGED;
import static com.google.gerrit.reviewdb.client.Change.Status.NEW; import static com.google.gerrit.reviewdb.client.Change.Status.NEW;
import static com.google.gerrit.reviewdb.client.Change.Status.SUBMITTED;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertSame; import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
@@ -183,17 +182,17 @@ public class IndexRewriteTest {
public void testGetPossibleStatus() throws Exception { public void testGetPossibleStatus() throws Exception {
assertEquals(EnumSet.allOf(Change.Status.class), status("file:a")); assertEquals(EnumSet.allOf(Change.Status.class), status("file:a"));
assertEquals(EnumSet.of(NEW), status("is:new")); assertEquals(EnumSet.of(NEW), status("is:new"));
assertEquals(EnumSet.of(SUBMITTED, DRAFT, MERGED, ABANDONED), assertEquals(EnumSet.of(DRAFT, MERGED, ABANDONED),
status("-is:new")); status("-is:new"));
assertEquals(EnumSet.of(NEW, MERGED), status("is:new OR is:merged")); assertEquals(EnumSet.of(NEW, MERGED), status("is:new OR is:merged"));
EnumSet<Change.Status> none = EnumSet.noneOf(Change.Status.class); EnumSet<Change.Status> none = EnumSet.noneOf(Change.Status.class);
assertEquals(none, status("is:new is:merged")); assertEquals(none, status("is:new is:merged"));
assertEquals(none, status("(is:new is:draft) (is:merged is:submitted)")); assertEquals(none, status("(is:new is:draft) (is:merged)"));
assertEquals(none, status("(is:new is:draft) (is:merged is:submitted)")); assertEquals(none, status("(is:new is:draft) (is:merged)"));
assertEquals(EnumSet.of(MERGED, SUBMITTED), assertEquals(EnumSet.of(MERGED),
status("(is:new is:draft) OR (is:merged OR is:submitted)")); status("(is:new is:draft) OR (is:merged)"));
} }
@Test @Test

View File

@@ -287,7 +287,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ChangeUpdate update = newUpdate(c, changeOwner); ChangeUpdate update = newUpdate(c, changeOwner);
update.setSubject("Submit patch set 1"); update.setSubject("Submit patch set 1");
update.submit(ImmutableList.of( update.merge(ImmutableList.of(
submitRecord("NOT_READY", null, submitRecord("NOT_READY", null,
submitLabel("Verified", "OK", changeOwner.getAccountId()), submitLabel("Verified", "OK", changeOwner.getAccountId()),
submitLabel("Code-Review", "NEED", null)), submitLabel("Code-Review", "NEED", null)),
@@ -314,7 +314,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
Change c = newChange(); Change c = newChange();
ChangeUpdate update = newUpdate(c, changeOwner); ChangeUpdate update = newUpdate(c, changeOwner);
update.setSubject("Submit patch set 1"); update.setSubject("Submit patch set 1");
update.submit(ImmutableList.of( update.merge(ImmutableList.of(
submitRecord("OK", null, submitRecord("OK", null,
submitLabel("Code-Review", "OK", otherUser.getAccountId())))); submitLabel("Code-Review", "OK", otherUser.getAccountId()))));
update.commit(); update.commit();
@@ -322,7 +322,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
incrementPatchSet(c); incrementPatchSet(c);
update = newUpdate(c, changeOwner); update = newUpdate(c, changeOwner);
update.setSubject("Submit patch set 2"); update.setSubject("Submit patch set 2");
update.submit(ImmutableList.of( update.merge(ImmutableList.of(
submitRecord("OK", null, submitRecord("OK", null,
submitLabel("Code-Review", "OK", changeOwner.getAccountId())))); submitLabel("Code-Review", "OK", changeOwner.getAccountId()))));
update.commit(); update.commit();

View File

@@ -108,7 +108,7 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest {
ChangeUpdate update = newUpdate(c, changeOwner); ChangeUpdate update = newUpdate(c, changeOwner);
update.setSubject("Submit patch set 1"); update.setSubject("Submit patch set 1");
update.submit(ImmutableList.of( update.merge(ImmutableList.of(
submitRecord("NOT_READY", null, submitRecord("NOT_READY", null,
submitLabel("Verified", "OK", changeOwner.getAccountId()), submitLabel("Verified", "OK", changeOwner.getAccountId()),
submitLabel("Code-Review", "NEED", null)), submitLabel("Code-Review", "NEED", null)),
@@ -173,7 +173,7 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest {
ChangeUpdate update = newUpdate(c, changeOwner); ChangeUpdate update = newUpdate(c, changeOwner);
update.setSubject("Submit patch set 1"); update.setSubject("Submit patch set 1");
update.submit(ImmutableList.of( update.merge(ImmutableList.of(
submitRecord("RULE_ERROR", "Problem with patch set:\n1"))); submitRecord("RULE_ERROR", "Problem with patch set:\n1")));
update.commit(); update.commit();