opendev/gerrit
Code Issues Proposed changes

Merge changes from topic 'kill-submitted'

Browse Source 
* changes:
  Remove the state SUBMITTED altogether
  Merging Changes: Do not enter submitted state
  Test author and committer for different submit strategies
This commit is contained in:
Dave Borowitz 2015-07-10 21:44:49 +00:00 committed by Gerrit Code Review
commit 04bddf22e9
40 changed files with 448 additions and 702 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
Documentation/json.txt
View File

@ -43,9 +43,6 @@ status:: Current state of this change.
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.
ABANDONED;; Change was abandoned by its owner or administrator.

9
Documentation/rest-api-changes.txt
View File

@ -3756,8 +3756,7 @@ The `refs/heads/` prefix is omitted.
|`subject` ||
The subject of the change (header line of the commit message).
|`status` ||
The status of the change (`NEW`, `SUBMITTED`, `MERGED`, `ABANDONED`,
`DRAFT`).
The status of the change (`NEW`, `MERGED`, `ABANDONED`, `DRAFT`).
|`created` ||
The link:rest-api.html#timestamp[timestamp] of when the change was
created.
@ -4322,7 +4321,7 @@ link:#commit-info[CommitInfo] entity.
|`_revision_number` |optional|The revision number.
|`_current_revision_number`|optional|The current revision number.
|`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]]
@ -4530,8 +4529,8 @@ after submitting.
|==========================
|Field Name ||Description
|`status` ||
The status of the change after submitting, can be `MERGED` or
`SUBMITTED`.+
The status of the change after submitting is `MERGED`.
+
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
`MERGED` to be returned here.

6
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
View File

@ -39,6 +39,7 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.OutputFormat;
import com.google.gerrit.server.account.GroupCache;
@ -67,6 +68,7 @@ import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.transport.Transport;
import org.junit.AfterClass;
@ -148,6 +150,10 @@ public abstract class AbstractDaemonTest {
@Inject
private Provider<AnonymousUser> anonymousUser;
@Inject
@GerritPersonIdent
protected Provider<PersonIdent> serverIdent;
protected TestRepository<InMemoryRepository> testRepo;
protected GerritServer server;
protected TestAccount admin;

2
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/TestAccount.java
View File

@ -79,7 +79,7 @@ public class TestAccount {
}
public PersonIdent getIdent() {
return new PersonIdent(username, email);
return new PersonIdent(fullName, email);
}
public String getHttpUrl(GerritServer server) {

10
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java
View File

@ -26,14 +26,12 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.git.CommitMergeStatus;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@ -53,10 +51,6 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
@Inject
private ChangeNotes.Factory changeNotesFactory;
@Inject
@GerritPersonIdent
private PersonIdent serverIdent;
@Test
public void submitOnPush() throws Exception {
grant(Permission.SUBMIT, project, "refs/for/refs/heads/master");
@ -124,7 +118,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
PushOneCommit.Result r =
push("refs/for/master%submit", "other change", "a.txt", "other content");
r.assertErrorStatus();
r.assertChange(Change.Status.NEW, null, admin);
r.assertChange(Change.Status.NEW, null);
r.assertMessage(CommitMergeStatus.PATH_CONFLICT.getMessage());
}
@ -257,7 +251,7 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
assertThat(c.getShortMessage()).isEqualTo("Merge \"" + subject + "\"");
assertThat(c.getAuthorIdent().getEmailAddress()).isEqualTo(admin.email);
assertThat(c.getCommitterIdent().getEmailAddress()).isEqualTo(
serverIdent.getEmailAddress());
serverIdent.get().getEmailAddress());
}
}

47
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
View File

@ -41,7 +41,6 @@ import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.LabelId;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
@ -61,6 +60,7 @@ import org.eclipse.jgit.diff.DiffFormatter;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@ -71,8 +71,6 @@ import org.junit.Test;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.Collections;
import java.util.List;
import java.util.Map;
@ -145,9 +143,11 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
change1.assertChange(Change.Status.MERGED, "test-topic", admin);
change2.assertChange(Change.Status.MERGED, "test-topic", admin);
change3.assertChange(Change.Status.MERGED, "test-topic", admin);
// Check for the exact change to have the correct submitter.
assertSubmitter(change3);
// Also check submitters for changes submitted via the topic relationship.
assertSubmitter(change1);
assertSubmitter(change2);
assertSubmitter(change3);
}
private void assertSubmitter(PushOneCommit.Result change) throws Exception {
@ -202,22 +202,6 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
submit(changeId, HttpStatus.SC_CONFLICT);
}
protected void submitStatusOnly(String changeId) throws Exception {
approve(changeId);
Change c = queryProvider.get().byKeyPrefix(changeId).get(0).change();
c.setStatus(Change.Status.SUBMITTED);
db.changes().update(Collections.singleton(c));
db.patchSetApprovals().insert(Collections.singleton(
new PatchSetApproval(
new PatchSetApproval.Key(
c.currentPatchSetId(),
admin.id,
LabelId.SUBMIT),
(short) 1,
new Timestamp(System.currentTimeMillis()))));
indexer.index(db, c);
}
private void submit(String changeId, int expectedStatus) throws Exception {
approve(changeId);
SubmitInput subm = new SubmitInput();
@ -265,6 +249,10 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
}
}
protected void assertNew(String changeId) throws Exception {
assertThat(get(changeId).status).isEqualTo(ChangeStatus.NEW);
}
protected void assertApproved(String changeId) throws Exception {
ChangeInfo c = get(changeId, DETAILED_LABELS);
LabelInfo cr = c.labels.get("Code-Review");
@ -273,6 +261,16 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
assertThat(new Account.Id(cr.all.get(0)._accountId)).isEqualTo(admin.getId());
}
protected void assertPersonEquals(PersonIdent expected,
PersonIdent actual) {
assertThat(actual.getEmailAddress())
.isEqualTo(expected.getEmailAddress());
assertThat(actual.getName())
.isEqualTo(expected.getName());
assertThat(actual.getTimeZone())
.isEqualTo(expected.getTimeZone());
}
protected void assertSubmitter(String changeId, int psId)
throws OrmException {
ChangeNotes cn = notesFactory.create(
@ -283,6 +281,15 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
assertThat(submitter.getAccountId()).isEqualTo(admin.getId());
}
protected void assertNoSubmitter(String changeId, int psId)
throws OrmException {
ChangeNotes cn = notesFactory.create(
getOnlyElement(queryProvider.get().byKeyPrefix(changeId)).change());
PatchSetApproval submitter = approvalsUtil.getSubmitter(
db, cn, new PatchSet.Id(cn.getChangeId(), psId));
assertThat(submitter).isNull();
}
protected void assertCherryPick(TestRepository<?> testRepo,
boolean contentMerge) throws IOException {
assertRebase(testRepo, contentMerge);

2
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
View File

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

81
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java
View File

@ -65,6 +65,8 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
assertCurrentRevision(change2.getChangeId(), 2, newHead);
assertSubmitter(change2.getChangeId(), 1);
assertSubmitter(change2.getChangeId(), 2);
assertPersonEquals(admin.getIdent(), newHead.getAuthorIdent());
assertPersonEquals(admin.getIdent(), newHead.getCommitterIdent());
}
@Test
@ -106,7 +108,7 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
submitWithConflict(change2.getChangeId());
assertThat(getRemoteHead()).isEqualTo(oldHead);
assertCurrentRevision(change2.getChangeId(), 1, change2.getCommitId());
assertSubmitter(change2.getChangeId(), 1);
assertNoSubmitter(change2.getChangeId(), 1);
}
@Test
@ -146,7 +148,7 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
submitWithConflict(change3.getChangeId());
assertThat(getRemoteHead()).isEqualTo(oldHead);
assertCurrentRevision(change3.getChangeId(), 1, change3.getCommitId());
assertSubmitter(change3.getChangeId(), 1);
assertNoSubmitter(change3.getChangeId(), 1);
}
@Test
@ -162,24 +164,17 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
testRepo.reset(initialHead);
PushOneCommit.Result change4 = createChange("Change 4", "d", "d");
submitStatusOnly(change2.getChangeId());
submitStatusOnly(change3.getChangeId());
approve(change2.getChangeId());
approve(change3.getChangeId());
submit(change4.getChangeId());
List<RevCommit> log = getRemoteLog();
assertThat(log.get(0).getShortMessage()).isEqualTo(
change4.getCommit().getShortMessage());
assertThat(log.get(0).getParent(0)).isEqualTo(log.get(1));
assertThat(log.get(1).getId()).isEqualTo(initialHead.getId());
assertThat(log.get(1).getShortMessage()).isEqualTo(
change3.getCommit().getShortMessage());
assertThat(log.get(1).getParent(0)).isEqualTo(log.get(2));
assertThat(log.get(2).getShortMessage()).isEqualTo(
change2.getCommit().getShortMessage());
assertThat(log.get(2).getParent(0)).isEqualTo(log.get(3));
assertThat(log.get(3).getId()).isEqualTo(initialHead.getId());
assertNew(change2.getChangeId());
assertNew(change3.getChangeId());
}
@Test
@ -231,6 +226,7 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
// Tip has not changed.
List<RevCommit> log = getRemoteLog();
assertThat(log.get(0)).isEqualTo(initialHead.getId());
assertNoSubmitter(change3.getChangeId(), 1);
}
@Test
@ -238,57 +234,16 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
RevCommit initialHead = getRemoteHead();
testRepo.reset(initialHead);
createChange("Change 2", "b", "b");
PushOneCommit.Result change2 = createChange("Change 2", "b", "b");
PushOneCommit.Result change3 = createChange("Change 3", "c", "c");
createChange("Change 4", "d", "d");
PushOneCommit.Result change5 = createChange("Change 5", "e", "e");
PushOneCommit.Result change4 = createChange("Change 5", "e", "e");
// Out of the above, only submit 3 and 5.
submitStatusOnly(change3.getChangeId());
submit(change5.getChangeId());
// Out of the above, only submit 4. 2,3 are not related to 4
// by topic or ancestor (due to cherrypicking!)
approve(change3.getChangeId());
submit(change4.getChangeId());
ChangeInfo info3 = get(change3.getChangeId());
assertThat(info3.status).isEqualTo(ChangeStatus.MERGED);
List<RevCommit> log = getRemoteLog();
assertThat(log.get(0).getShortMessage())
.isEqualTo(change5.getCommit().getShortMessage());
assertThat(log.get(1).getShortMessage())
.isEqualTo(change3.getCommit().getShortMessage());
assertThat(log.get(2).getShortMessage())
.isEqualTo(initialHead.getShortMessage());
}
@Test
public void submitChangeAfterParentFailsDueToConflict() throws Exception {
RevCommit initialHead = getRemoteHead();
testRepo.reset(initialHead);
PushOneCommit.Result change2 = createChange("Change 2", "b", "b1");
submit(change2.getChangeId());
testRepo.reset(initialHead);
PushOneCommit.Result change3 = createChange("Change 3", "b", "b2");
assertThat(change3.getCommit().getParent(0)).isEqualTo(initialHead);
PushOneCommit.Result change4 = createChange("Change 3", "c", "c3");
submitStatusOnly(change3.getChangeId());
submitStatusOnly(change4.getChangeId());
// Merge fails; change3 contains the delta "b1" -> "b2", which cannot be
// applied against tip.
// As change4 sits on top of change 3 we need to trigger submission there
// to include it into the mergeing
submitWithConflict(change4.getChangeId());
// change4 is a clean merge, so should succeed in the same run where change3
// failed.
ChangeInfo info4 = get(change4.getChangeId());
assertThat(info4.status).isEqualTo(ChangeStatus.MERGED);
List<RevCommit> log = getRemoteLog();
assertThat(log.get(0).getShortMessage())
.isEqualTo(change4.getCommit().getShortMessage());
assertThat(log.get(1).getShortMessage())
.isEqualTo(change2.getCommit().getShortMessage());
assertNew(change2.getChangeId());
assertNew(change3.getChangeId());
}
}

2
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java
View File

@ -56,6 +56,8 @@ public class SubmitByFastForwardIT extends AbstractSubmit {
assertThat(head.getParent(0).getId()).isEqualTo(change.getCommitId());
assertSubmitter(change.getChangeId(), 1);
assertSubmitter(change2.getChangeId(), 1);
assertPersonEquals(admin.getIdent(), head.getAuthorIdent());
assertPersonEquals(admin.getIdent(), head.getCommitterIdent());
}
@Test

23
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeAlwaysIT.java
View File

@ -41,6 +41,8 @@ public class SubmitByMergeAlwaysIT extends AbstractSubmitByMerge {
assertThat(head.getParent(0)).isEqualTo(oldHead);
assertThat(head.getParent(1)).isEqualTo(change.getCommitId());
assertSubmitter(change.getChangeId(), 1);
assertPersonEquals(admin.getIdent(), head.getAuthorIdent());
assertPersonEquals(serverIdent.get(), head.getCommitterIdent());
}
@Test
@ -56,23 +58,22 @@ public class SubmitByMergeAlwaysIT extends AbstractSubmitByMerge {
testRepo.reset(initialHead);
PushOneCommit.Result change4 = createChange("Change 4", "d", "d");
submitStatusOnly(change2.getChangeId());
submitStatusOnly(change3.getChangeId());
approve(change2.getChangeId());
approve(change3.getChangeId());
submit(change4.getChangeId());
List<RevCommit> log = getRemoteLog();
RevCommit tip = log.get(0);
assertThat(tip.getParent(1).getShortMessage()).isEqualTo(
change4.getCommit().getShortMessage());
tip = tip.getParent(0);
assertThat(tip.getParent(1).getShortMessage()).isEqualTo(
change3.getCommit().getShortMessage());
tip = tip.getParent(0);
assertThat(tip.getParent(1).getShortMessage()).isEqualTo(
change2.getCommit().getShortMessage());
assertThat(tip.getParent(0).getShortMessage()).isEqualTo(
initialHead.getShortMessage());
assertThat(tip.getParent(0).getId()).isEqualTo(initialHead.getId());
assertPersonEquals(admin.getIdent(), tip.getAuthorIdent());
assertPersonEquals(serverIdent.get(), tip.getCommitterIdent());
assertNew(change2.getChangeId());
assertNew(change3.getChangeId());
}
}

38
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByMergeIfNecessaryIT.java
View File

@ -29,6 +29,8 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
assertThat(head.getId()).isEqualTo(change.getCommitId());
assertThat(head.getParent(0)).isEqualTo(oldHead);
assertSubmitter(change.getChangeId(), 1);
assertPersonEquals(admin.getIdent(), head.getAuthorIdent());
assertPersonEquals(admin.getIdent(), head.getCommitterIdent());
}
@Test
@ -44,23 +46,32 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
testRepo.reset(initialHead);
PushOneCommit.Result change4 = createChange("Change 4", "d", "d");
submitStatusOnly(change2.getChangeId());
submitStatusOnly(change3.getChangeId());
submit(change4.getChangeId());
// Change 2 stays untouched.
approve(change2.getChangeId());
// Change 3 is a fast-forward, no need to merge.
submit(change3.getChangeId());
RevCommit tip = getRemoteLog().get(0);
assertThat(tip.getShortMessage()).isEqualTo(
change3.getCommit().getShortMessage());
assertThat(tip.getParent(0).getId()).isEqualTo(
initialHead.getId());
assertPersonEquals(admin.getIdent(), tip.getAuthorIdent());
assertPersonEquals(admin.getIdent(), tip.getCommitterIdent());
// We need to merge change 4.
submit(change4.getChangeId());
tip = getRemoteLog().get(0);
assertThat(tip.getParent(1).getShortMessage()).isEqualTo(
change4.getCommit().getShortMessage());
tip = tip.getParent(0);
assertThat(tip.getParent(1).getShortMessage()).isEqualTo(
assertThat(tip.getParent(0).getShortMessage()).isEqualTo(
change3.getCommit().getShortMessage());
tip = tip.getParent(0);
assertThat(tip.getShortMessage()).isEqualTo(
change2.getCommit().getShortMessage());
assertPersonEquals(admin.getIdent(), tip.getAuthorIdent());
assertPersonEquals(serverIdent.get(), tip.getCommitterIdent());
assertThat(tip.getParent(0).getId()).isEqualTo(initialHead.getId());
assertNew(change2.getChangeId());
}
@Test
@ -184,6 +195,10 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
initialHead2.getShortMessage());
assertThat(tip3.getShortMessage()).isEqualTo(
change3Conflict.getCommit().getShortMessage());
assertNoSubmitter(change1a.getChangeId(), 1);
assertNoSubmitter(change2a.getChangeId(), 1);
assertNoSubmitter(change2b.getChangeId(), 1);
assertNoSubmitter(change3.getChangeId(), 1);
} else {
assertThat(tip1.getShortMessage()).isEqualTo(
change1b.getCommit().getShortMessage());
@ -191,6 +206,9 @@ public class SubmitByMergeIfNecessaryIT extends AbstractSubmitByMerge {
initialHead2.getShortMessage());
assertThat(tip3.getShortMessage()).isEqualTo(
change3Conflict.getCommit().getShortMessage());
assertNoSubmitter(change2a.getChangeId(), 1);
assertNoSubmitter(change2b.getChangeId(), 1);
assertNoSubmitter(change3.getChangeId(), 1);
}
}

6
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseIfNecessaryIT.java
View File

@ -43,6 +43,8 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
assertApproved(change.getChangeId());
assertCurrentRevision(change.getChangeId(), 1, head);
assertSubmitter(change.getChangeId(), 1);
assertPersonEquals(admin.getIdent(), head.getAuthorIdent());
assertPersonEquals(admin.getIdent(), head.getCommitterIdent());
}
@Test
@ -65,6 +67,8 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
assertCurrentRevision(change2.getChangeId(), 2, head);
assertSubmitter(change2.getChangeId(), 1);
assertSubmitter(change2.getChangeId(), 2);
assertPersonEquals(admin.getIdent(), head.getAuthorIdent());
assertPersonEquals(serverIdent.get(), head.getCommitterIdent());
}
@Test
@ -107,6 +111,6 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
RevCommit head = getRemoteHead();
assertThat(head).isEqualTo(oldHead);
assertCurrentRevision(change2.getChangeId(), 1, change2.getCommitId());
assertSubmitter(change2.getChangeId(), 1);
assertNoSubmitter(change2.getChangeId(), 1);
}
}

4
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/GetCommitIT.java
View File

@ -102,9 +102,9 @@ public class GetCommitIT extends AbstractDaemonTest {
assertThat(info.subject).isEqualTo("test commit");
assertThat(info.message).isEqualTo(
"test commit\n\nChange-Id: " + r.getChangeId() + "\n");
assertThat(info.author.name).isEqualTo("admin");
assertThat(info.author.name).isEqualTo("Administrator");
assertThat(info.author.email).isEqualTo("admin@example.com");
assertThat(info.committer.name).isEqualTo("admin");
assertThat(info.committer.name).isEqualTo("Administrator");
assertThat(info.committer.email).isEqualTo("admin@example.com");
CommitInfo parent = Iterables.getOnlyElement(info.parents);

1
gerrit-common/src/main/java/com/google/gerrit/common/PageLinks.java
View File

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

29
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ChangeStatus.java
View File

@ -28,39 +28,12 @@ public enum ChangeStatus {
* <p>
* Changes in the NEW state can be moved to:
* <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.
* </ul>
*/
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.
*

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java
View File

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

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties
View File

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

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/Util.java
View File

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

31
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java
View File

@ -251,8 +251,6 @@ public final class Change {
private static final char MIN_OPEN = 'a';
/** Database constant for {@link Status#NEW}. */
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}. */
public static final char STATUS_DRAFT = 'd';
/** Maximum database status constant for an open change. */
@ -285,39 +283,12 @@ public final class Change {
* <p>
* Changes in the NEW state can be moved to:
* <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.
* </ul>
*/
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.
*

3
gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityCacheImpl.java
View File

@ -244,7 +244,8 @@ public class MergeabilityCacheImpl implements MergeabilityCache {
null /*inserter*/,
canMerge,
accepted,
key.load.dest).dryRun(tip, rev);
key.load.dest,
null).dryRun(tip, rev);
}
} finally {
key.load = null;

258
gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
View File

@ -18,17 +18,12 @@ import static com.google.gerrit.common.data.SubmitRecord.Status.OK;
import com.google.common.base.MoreObjects;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
import com.google.common.base.Strings;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
import com.google.common.collect.Table;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.ParameterizedString;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.extensions.api.changes.SubmitInput;
@ -38,34 +33,24 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.LabelId;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ProjectUtil;
import com.google.gerrit.server.account.AccountsCollection;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.ChangeSet;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LabelNormalizer;
import com.google.gerrit.server.git.MergeOp;
import com.google.gerrit.server.git.VersionedMetaData.BatchMetaDataUpdate;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.OrmRuntimeException;
import com.google.inject.Inject;
@ -73,15 +58,12 @@ import com.google.inject.Provider;
import com.google.inject.Singleton;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@ -106,31 +88,19 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
private static final String CLICK_FAILURE_TOOLTIP =
"Clicking the button would fail.";
public enum Status {
SUBMITTED, MERGED
}
public static class Output {
public Status status;
transient Change change;
private Output(Status s, Change c) {
status = s;
private Output(Change c) {
change = c;
}
}
private final PersonIdent serverIdent;
private final Provider<ReviewDb> dbProvider;
private final GitRepositoryManager repoManager;
private final IdentifiedUser.GenericFactory userFactory;
private final ChangeData.Factory changeDataFactory;
private final ChangeUpdate.Factory updateFactory;
private final ApprovalsUtil approvalsUtil;
private final ChangeMessagesUtil cmUtil;
private final Provider<MergeOp> mergeOpProvider;
private final ChangeIndexer indexer;
private final LabelNormalizer labelNormalizer;
private final AccountsCollection accounts;
private final ChangesCollection changes;
private final String label;
@ -141,34 +111,22 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
private final Provider<InternalChangeQuery> queryProvider;
@Inject
Submit(@GerritPersonIdent PersonIdent serverIdent,
Provider<ReviewDb> dbProvider,
Submit(Provider<ReviewDb> dbProvider,
GitRepositoryManager repoManager,
IdentifiedUser.GenericFactory userFactory,
ChangeData.Factory changeDataFactory,
ChangeUpdate.Factory updateFactory,
ApprovalsUtil approvalsUtil,
ChangeMessagesUtil cmUtil,
Provider<MergeOp> mergeOpProvider,
AccountsCollection accounts,
ChangesCollection changes,
ChangeIndexer indexer,
LabelNormalizer labelNormalizer,
@GerritServerConfig Config cfg,
Provider<InternalChangeQuery> queryProvider) {
this.serverIdent = serverIdent;
this.dbProvider = dbProvider;
this.repoManager = repoManager;
this.userFactory = userFactory;
this.changeDataFactory = changeDataFactory;
this.updateFactory = updateFactory;
this.approvalsUtil = approvalsUtil;
this.cmUtil = cmUtil;
this.mergeOpProvider = mergeOpProvider;
this.accounts = accounts;
this.changes = changes;
this.indexer = indexer;
this.labelNormalizer = labelNormalizer;
this.label = MoreObjects.firstNonNull(
Strings.emptyToNull(cfg.getString("change", null, "submitLabel")),
"Submit");
@ -212,7 +170,16 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
rsrc.getPatchSet().getRevision().get()));
}
ChangeSet submittedChanges = ChangeSet.create(submit(rsrc, caller, false));
List<Change> changes;
if (submitWholeTopic && !Strings.isNullOrEmpty(change.getTopic())) {
changes = new ArrayList<>();
for (ChangeData cd : getChangesByTopic(change.getTopic())) {
changes.add(cd.change());
}
} else {
changes = Arrays.asList(change);
}
ChangeSet submittedChanges = ChangeSet.create(changes);
try {
mergeOpProvider.get().merge(submittedChanges, caller, true);
@ -225,10 +192,8 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
throw new ResourceConflictException("change is deleted");
}
switch (change.getStatus()) {
case SUBMITTED:
return new Output(Status.SUBMITTED, change);
case MERGED:
return new Output(Status.MERGED, change);
return new Output(change);
case NEW:
ChangeMessage msg = getConflictMessage(rsrc);
if (msg != null) {
@ -374,203 +339,6 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
.orNull();
}
private Change submitToDatabase(final ReviewDb db, final Change.Id changeId,
final Timestamp timestamp) throws OrmException,
ResourceConflictException {
Change ret = db.changes().atomicUpdate(changeId,
new AtomicUpdate<Change>() {
@Override
public Change update(Change change) {
if (change.getStatus().isOpen()) {
change.setStatus(Change.Status.SUBMITTED);
change.setLastUpdatedOn(timestamp);
return change;
}
return null;
}
});
if (ret != null) {
return ret;
} else {
throw new ResourceConflictException("change " + changeId + " is "
+ status(db.changes().get(changeId)));
}
}
private Change submitThisChange(RevisionResource rsrc, IdentifiedUser caller,
boolean force) throws ResourceConflictException, OrmException,
IOException {
ReviewDb db = dbProvider.get();
ChangeData cd = changeDataFactory.create(db, rsrc.getControl());
List<SubmitRecord> submitRecords = checkSubmitRule(cd,
rsrc.getPatchSet(), force);
final Timestamp timestamp = TimeUtil.nowTs();
Change change = rsrc.getChange();
ChangeUpdate update = updateFactory.create(rsrc.getControl(), timestamp);
update.submit(submitRecords);
db.changes().beginTransaction(change.getId());
try {
BatchMetaDataUpdate batch = approve(rsrc.getPatchSet().getId(),
cd.changeControl(), update, caller, timestamp);
// Write update commit after all normalized label commits.
batch.write(update, new CommitBuilder());
change = submitToDatabase(db, change.getId(), timestamp);
db.commit();
} finally {
db.rollback();
}
indexer.index(db, change);
return change;
}
private List<Change> submitWholeTopic(RevisionResource rsrc, IdentifiedUser caller,
boolean force, String topic) throws ResourceConflictException, OrmException,
IOException {
Preconditions.checkNotNull(topic);
final Timestamp timestamp = TimeUtil.nowTs();
ReviewDb db = dbProvider.get();
ChangeData cd = changeDataFactory.create(db, rsrc.getControl());
List<ChangeData> changesByTopic = getChangesByTopic(topic);
String problems = problemsForSubmittingChanges(changesByTopic, caller);
if (problems != null) {
throw new ResourceConflictException(problems);
}
Change change = rsrc.getChange();
ChangeUpdate update = updateFactory.create(rsrc.getControl(), timestamp);
List<SubmitRecord> submitRecords = checkSubmitRule(cd,
rsrc.getPatchSet(), force);
update.submit(submitRecords);
db.changes().beginTransaction(change.getId());
try {
for (ChangeData c : changesByTopic) {
BatchMetaDataUpdate batch = approve(c.currentPatchSet().getId(),
c.changeControl(), update, caller, timestamp);
// Write update commit after all normalized label commits.
batch.write(update, new CommitBuilder());
submitToDatabase(db, c.getId(), timestamp);
}
db.commit();
} finally {
db.rollback();
}
List<Change.Id> ids = new ArrayList<>(changesByTopic.size());
List<Change> ret = new ArrayList<>(changesByTopic.size());
for (ChangeData c : changesByTopic) {
ids.add(c.getId());
ret.add(c.change());
}
indexer.indexAsync(ids).checkedGet();
return ret;
}
public List<Change> submit(RevisionResource rsrc, IdentifiedUser caller,
boolean force) throws ResourceConflictException, OrmException,
IOException {
String topic = rsrc.getChange().getTopic();
if (submitWholeTopic && !Strings.isNullOrEmpty(topic)) {
return submitWholeTopic(rsrc, caller, force, topic);
} else {
return Arrays.asList(submitThisChange(rsrc, caller, force));
}
}
private BatchMetaDataUpdate approve(PatchSet.Id psId, ChangeControl control,
ChangeUpdate update, IdentifiedUser caller, Timestamp timestamp)
throws OrmException {
Map<PatchSetApproval.Key, PatchSetApproval> byKey = Maps.newHashMap();
for (PatchSetApproval psa :
approvalsUtil.byPatchSet(dbProvider.get(), control, psId)) {
if (!byKey.containsKey(psa.getKey())) {
byKey.put(psa.getKey(), psa);
}
}
PatchSetApproval submit = ApprovalsUtil.getSubmitter(psId, byKey.values());
if (submit == null
|| !submit.getAccountId().equals(caller.getAccountId())) {
submit = new PatchSetApproval(
new PatchSetApproval.Key(
psId,
caller.getAccountId(),
LabelId.SUBMIT),
(short) 1, TimeUtil.nowTs());
byKey.put(submit.getKey(), submit);
}
submit.setValue((short) 1);
submit.setGranted(timestamp);
// Flatten out existing approvals for this patch set based upon the current
// permissions. Once the change is closed the approvals are not updated at
// presentation view time, except for zero votes used to indicate a reviewer
// was added. So we need to make sure votes are accurate now. This way if
// permissions get modified in the future, historical records stay accurate.
LabelNormalizer.Result normalized =
labelNormalizer.normalize(control, byKey.values());
// TODO(dborowitz): Don't use a label in notedb; just check when status
// change happened.
update.putApproval(submit.getLabel(), submit.getValue());
dbProvider.get().patchSetApprovals().upsert(normalized.getNormalized());
dbProvider.get().patchSetApprovals().delete(normalized.deleted());
try {
return saveToBatch(control, update, normalized, timestamp);
} catch (IOException e) {
throw new OrmException(e);
}
}
private BatchMetaDataUpdate saveToBatch(ChangeControl ctl,
ChangeUpdate callerUpdate, LabelNormalizer.Result normalized,
Timestamp timestamp) throws IOException {
Table<Account.Id, String, Optional<Short>> byUser = HashBasedTable.create();
for (PatchSetApproval psa : normalized.updated()) {
byUser.put(psa.getAccountId(), psa.getLabel(),
Optional.of(psa.getValue()));
}
for (PatchSetApproval psa : normalized.deleted()) {
byUser.put(psa.getAccountId(), psa.getLabel(), Optional.<Short> absent());
}
BatchMetaDataUpdate batch = callerUpdate.openUpdate();
for (Account.Id accountId : byUser.rowKeySet()) {
if (!accountId.equals(callerUpdate.getUser().getAccountId())) {
ChangeUpdate update = updateFactory.create(
ctl.forUser(userFactory.create(dbProvider, accountId)), timestamp);
update.setSubject("Finalize approvals at submit");
putApprovals(update, byUser.row(accountId));
CommitBuilder commit = new CommitBuilder();
commit.setCommitter(new PersonIdent(serverIdent, timestamp));
batch.write(update, commit);
}
}
putApprovals(callerUpdate,
byUser.row(callerUpdate.getUser().getAccountId()));
return batch;
}
private static void putApprovals(ChangeUpdate update,
Map<String, Optional<Short>> approvals) {
for (Map.Entry<String, Optional<Short>> e : approvals.entrySet()) {
if (e.getValue().isPresent()) {
update.putApproval(e.getKey(), e.getValue().get());
} else {
update.removeApproval(e.getKey());
}
}
}
private List<SubmitRecord> checkSubmitRule(ChangeData cd,
PatchSet patchSet, boolean force)
throws ResourceConflictException, OrmException {

14
gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeSet.java
View File

@ -33,18 +33,22 @@ public abstract class ChangeSet {
ImmutableSetMultimap.builder();
ImmutableSetMultimap.Builder<Project.NameKey, Change.Id> pcb =
ImmutableSetMultimap.builder();
ImmutableSetMultimap.Builder<Branch.NameKey, Change.Id> cbb =
ImmutableSetMultimap.builder();
for (Change c : changes) {
Project.NameKey project = c.getDest().getParentKey();
Branch.NameKey branch = c.getDest();
Project.NameKey project = branch.getParentKey();
pb.add(project);
bb.add(c.getDest());
bb.add(branch);
ib.add(c.getId());
pbb.put(project, c.getDest());
pbb.put(project, branch);
pcb.put(project, c.getId());
cbb.put(branch, c.getId());
}
return new AutoValue_ChangeSet(pb.build(), bb.build(),
ib.build(), pbb.build(), pcb.build());
ib.build(), pbb.build(), pcb.build(), cbb.build());
}
public static ChangeSet create(Change change) {
@ -58,6 +62,8 @@ public abstract class ChangeSet {
branchesByProject();
public abstract ImmutableSetMultimap<Project.NameKey, Change.Id>
changesByProject();
public abstract ImmutableSetMultimap<Branch.NameKey, Change.Id>
changesByBranch();
@Override
public int hashCode() {

257
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
View File

@ -20,11 +20,12 @@ import static org.eclipse.jgit.lib.RefDatabase.ALL;
import com.google.common.base.Optional;
import com.google.common.base.Predicate;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.common.collect.Maps;
import com.google.common.collect.Table;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.SubmitRecord;
@ -35,6 +36,7 @@ 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.ChangeMessage;
import com.google.gerrit.reviewdb.client.LabelId;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
@ -44,15 +46,14 @@ import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.RemotePeer;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.change.Submit;
import com.google.gerrit.server.config.GerritRequestModule;
import com.google.gerrit.server.config.RequestScopedReviewDbProvider;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.VersionedMetaData.BatchMetaDataUpdate;
import com.google.gerrit.server.git.strategy.SubmitStrategy;
import com.google.gerrit.server.git.strategy.SubmitStrategyFactory;
import com.google.gerrit.server.git.validators.MergeValidationException;
@ -89,6 +90,7 @@ import com.jcraft.jsch.HostKey;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
@ -105,6 +107,7 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.net.SocketAddress;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@ -144,6 +147,7 @@ public class MergeOp {
private final GitReferenceUpdated gitRefUpdated;
private final GitRepositoryManager repoManager;
private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final LabelNormalizer labelNormalizer;
private final MergedSender.Factory mergedSenderFactory;
private final MergeSuperSet mergeSuperSet;
private final MergeValidators.Factory mergeValidatorsFactory;
@ -151,12 +155,13 @@ public class MergeOp {
private final ProjectCache projectCache;
private final InternalChangeQuery internalChangeQuery;
private final SchemaFactory<ReviewDb> schemaFactory;
private final Submit submit;
private final PersonIdent serverIdent;
private final SubmitStrategyFactory submitStrategyFactory;
private final Provider<SubmoduleOp> subOpProvider;
private final TagCache tagCache;
private final WorkQueue workQueue;
private final Map<Change.Id, List<SubmitRecord>> records;
private final Map<Change.Id, CodeReviewCommit> commits;
private final PerThreadRequestScope.Scoper threadScoper;
private String logPrefix;
@ -185,6 +190,7 @@ public class MergeOp {
GitReferenceUpdated gitRefUpdated,
GitRepositoryManager repoManager,
IdentifiedUser.GenericFactory identifiedUserFactory,
LabelNormalizer labelNormalizer,
MergedSender.Factory mergedSenderFactory,
MergeSuperSet mergeSuperSet,
MergeValidators.Factory mergeValidatorsFactory,
@ -192,7 +198,7 @@ public class MergeOp {
ProjectCache projectCache,
InternalChangeQuery internalChangeQuery,
SchemaFactory<ReviewDb> schemaFactory,
Submit submit,
@GerritPersonIdent PersonIdent serverIdent,
SubmitStrategyFactory submitStrategyFactory,
Provider<SubmoduleOp> subOpProvider,
TagCache tagCache,
@ -208,6 +214,7 @@ public class MergeOp {
this.gitRefUpdated = gitRefUpdated;
this.repoManager = repoManager;
this.identifiedUserFactory = identifiedUserFactory;
this.labelNormalizer = labelNormalizer;
this.mergedSenderFactory = mergedSenderFactory;
this.mergeSuperSet = mergeSuperSet;
this.mergeValidatorsFactory = mergeValidatorsFactory;
@ -215,7 +222,7 @@ public class MergeOp {
this.projectCache = projectCache;
this.internalChangeQuery = internalChangeQuery;
this.schemaFactory = schemaFactory;
this.submit = submit;
this.serverIdent = serverIdent;
this.submitStrategyFactory = submitStrategyFactory;
this.subOpProvider = subOpProvider;
this.tagCache = tagCache;
@ -223,6 +230,8 @@ public class MergeOp {
commits = new HashMap<>();
pendingRefUpdates = new HashMap<>();
openBranches = new HashMap<>();
pendingRefUpdates = new HashMap<>();
records = new HashMap<>();
mergeTips = new HashMap<>();
Injector child = injector.createChildInjector(new AbstractModule() {
@ -379,40 +388,11 @@ public class MergeOp {
throws ResourceConflictException, OrmException {
for (Change.Id id : cs.ids()) {
ChangeData cd = changeDataFactory.create(db, id);
if (cd.change().getStatus() != Change.Status.NEW
&& cd.change().getStatus() != Change.Status.SUBMITTED){
if (cd.change().getStatus() != Change.Status.NEW){
throw new OrmException("Change " + cd.change().getChangeId()
+ " is in state " + cd.change().getStatus());
} else {
checkSubmitRule(cd);
}
}
}
// For historic reasons we will first go into the submitted state
// TODO(sbeller): remove this when we get rid of Change.Status.SUBMITTED
private void submitAllChanges(ChangeSet cs, IdentifiedUser caller,
boolean force) throws OrmException, ResourceConflictException,
IOException {
for (Change.Id id : cs.ids()) {
ChangeData cd = changeDataFactory.create(db, id);
switch (cd.change().getStatus()) {
case ABANDONED:
throw new ResourceConflictException("Change " + cd.getId() +
" was abandoned while processing this change set");
case DRAFT:
throw new ResourceConflictException("Cannot submit draft " + cd.getId());
case NEW:
RevisionResource rsrc =
new RevisionResource(new ChangeResource(cd.changeControl(), null),
cd.currentPatchSet());
logDebug("Submitting change id {}", cd.change().getId());
submit.submit(rsrc, caller, force);
break;
case MERGED:
// we're racing here, but having it already merged is fine.
case SUBMITTED:
// ok
records.put(cd.change().getId(), checkSubmitRule(cd));
}
}
}
@ -428,17 +408,11 @@ public class MergeOp {
ChangeSet cs = mergeSuperSet.completeChangeSet(db, changes);
logDebug("Calculated to merge {}", cs);
if (checkPermissions) {
logDebug("Submitting all calculated changes while "
+ "enforcing submit rules");
submitAllChanges(cs, caller, false);
logDebug("Checking permissions");
checkPermissions(cs);
} else {
logDebug("Submitting all calculated changes ignoring submit rules");
submitAllChanges(cs, caller, true);
}
try {
integrateIntoHistory(cs);
integrateIntoHistory(cs, caller);
} catch (MergeException e) {
logError("Merge Conflict", e);
throw new ResourceConflictException("Merge Conflict", e);
@ -453,10 +427,10 @@ public class MergeOp {
}
}
private void integrateIntoHistory(ChangeSet cs)
private void integrateIntoHistory(ChangeSet cs, IdentifiedUser caller)
throws MergeException, NoSuchChangeException, ResourceConflictException {
logDebug("Beginning merge attempt on {}", cs);
Map<Branch.NameKey, ListMultimap<SubmitType, Change>> toSubmit =
Map<Branch.NameKey, ListMultimap<SubmitType, ChangeData>> toSubmit =
new HashMap<>();
try {
openSchema();
@ -465,24 +439,25 @@ public class MergeOp {
openRepository(project);
for (Branch.NameKey branch : cs.branchesByProject().get(project)) {
setDestProject(branch);
ListMultimap<SubmitType, Change> submitting =
validateChangeList(internalChangeQuery.submitted(branch));
List<ChangeData> cds = new ArrayList<>();
for (Change.Id id : cs.changesByBranch().get(branch)) {
cds.add(changeDataFactory.create(db, id));
}
ListMultimap<SubmitType, ChangeData> submitting =
validateChangeList(cds);
toSubmit.put(branch, submitting);
Set<SubmitType> submitTypes = new HashSet<>(submitting.keySet());
for (SubmitType submitType : submitTypes) {
SubmitStrategy strategy = createStrategy(branch, submitType,
getBranchTip(branch));
getBranchTip(branch), caller);
MergeTip mergeTip = preMerge(strategy, submitting.get(submitType),
getBranchTip(branch));
mergeTips.put(branch, mergeTip);
if (submitType != SubmitType.CHERRY_PICK) {
// For cherry picking we have relaxed atomic guarantees
// as traditionally Gerrit kept going cherry picking if one
// failed. We want to keep it for now.
updateChangeStatus(submitting.get(submitType), branch, true);
}
updateChangeStatus(submitting.get(submitType), branch,
true, caller);
}
inserter.flush();
}
@ -498,9 +473,10 @@ public class MergeOp {
pendingRefUpdates.remove(branch);
setDestProject(branch);
ListMultimap<SubmitType, Change> submitting = toSubmit.get(branch);
ListMultimap<SubmitType, ChangeData> submitting = toSubmit.get(branch);
for (SubmitType submitType : submitting.keySet()) {
updateChangeStatus(submitting.get(submitType), branch, false);
updateChangeStatus(submitting.get(submitType), branch,
false, caller);
updateSubmoduleSubscriptions(subOp, branch, getBranchTip(branch));
}
if (update != null) {
@ -526,15 +502,15 @@ public class MergeOp {
}
private MergeTip preMerge(SubmitStrategy strategy,
List<Change> submitted, CodeReviewCommit branchTip)
throws MergeException {
List<ChangeData> submitted, CodeReviewCommit branchTip)
throws MergeException, OrmException {
logDebug("Running submit strategy {} for {} commits {}",
strategy.getClass().getSimpleName(), submitted.size(), submitted);
List<CodeReviewCommit> toMerge = new ArrayList<>(submitted.size());
for (Change c : submitted) {
CodeReviewCommit commit = commits.get(c.getId());
for (ChangeData cd : submitted) {
CodeReviewCommit commit = commits.get(cd.change().getId());
checkState(commit != null,
"commit for %s not found by validateChangeList", c.getId());
"commit for %s not found by validateChangeList", cd.change().getId());
toMerge.add(commit);
}
MergeTip mergeTip = strategy.run(branchTip, toMerge);
@ -545,10 +521,10 @@ public class MergeOp {
}
private SubmitStrategy createStrategy(Branch.NameKey destBranch,
SubmitType submitType, CodeReviewCommit branchTip)
SubmitType submitType, CodeReviewCommit branchTip, IdentifiedUser caller)
throws MergeException, NoSuchProjectException {
return submitStrategyFactory.create(submitType, db, repo, rw, inserter,
canMergeFlag, getAlreadyAccepted(branchTip), destBranch);
canMergeFlag, getAlreadyAccepted(branchTip), destBranch, caller);
}
private void openRepository(Project.NameKey name)
@ -649,10 +625,10 @@ public class MergeOp {
return alreadyAccepted;
}
private ListMultimap<SubmitType, Change> validateChangeList(
private ListMultimap<SubmitType, ChangeData> validateChangeList(
List<ChangeData> submitted) throws MergeException {
logDebug("Validating {} changes", submitted.size());
ListMultimap<SubmitType, Change> toSubmit = ArrayListMultimap.create();
ListMultimap<SubmitType, ChangeData> toSubmit = ArrayListMultimap.create();
Map<String, Ref> allRefs;
try {
@ -677,8 +653,8 @@ public class MergeOp {
throw new MergeException("Failed to validate changes", e);
}
Change.Id changeId = cd.getId();
if (chg.getStatus() != Change.Status.SUBMITTED) {
logDebug("Change {} is not submitted: {}", changeId, chg.getStatus());
if (chg.getStatus() != Change.Status.NEW) {
logDebug("Change {} is not new: {}", changeId, chg.getStatus());
continue;
}
if (chg.currentPatchSetId() == null) {
@ -762,7 +738,7 @@ public class MergeOp {
}
commit.add(canMergeFlag);
toSubmit.put(submitType, chg);
toSubmit.put(submitType, cd);
}
logDebug("Submitting on this run: {}", toSubmit);
return toSubmit;
@ -881,9 +857,10 @@ public class MergeOp {
return "";
}
private void updateChangeStatus(List<Change> submitted,
Branch.NameKey destBranch, boolean dryRun)
throws NoSuchChangeException, MergeException, ResourceConflictException {
private void updateChangeStatus(List<ChangeData> submitted,
Branch.NameKey destBranch, boolean dryRun, IdentifiedUser caller)
throws NoSuchChangeException, MergeException, ResourceConflictException,
OrmException {
if (!dryRun) {
logDebug("Updating change status for {} changes", submitted.size());
} else {
@ -891,7 +868,8 @@ public class MergeOp {
submitted.size());
}
MergeTip mergeTip = mergeTips.get(destBranch);
for (Change c : submitted) {
for (ChangeData cd : submitted) {
Change c = cd.change();
CodeReviewCommit commit = commits.get(c.getId());
CommitMergeStatus s = commit != null ? commit.getStatusCode() : null;
if (s == null) {
@ -903,6 +881,14 @@ public class MergeOp {
continue;
}
if (!dryRun) {
try {
setApproval(cd, caller);
} catch (IOException e) {
throw new OrmException(e);
}
}
String txt = s.getMessage();
logDebug("Status of change {} ({}) on {}: {}", c.getId(), commit.name(),
c.getDest(), s);
@ -1093,6 +1079,127 @@ public class MergeOp {
});
}
private void setApproval(ChangeData cd, IdentifiedUser user)
throws OrmException, IOException {
Timestamp timestamp = TimeUtil.nowTs();
ChangeControl control = cd.changeControl();
PatchSet.Id psId = cd.currentPatchSet().getId();
PatchSet.Id psIdNewRev = commits.get(cd.change().getId())
.change().currentPatchSetId();
logDebug("Add approval for " + cd + " from user " + user);
ChangeUpdate update = updateFactory.create(control, timestamp);
List<SubmitRecord> record = records.get(cd.change().getId());
if (record != null) {
update.merge(record);
}
db.changes().beginTransaction(cd.change().getId());
try {
BatchMetaDataUpdate batch = approve(control, psId, user,
update, timestamp);
batch.write(update, new CommitBuilder());
// If the submit strategy created a new revision (rebase, cherry-pick)
// approve that as well
if (!psIdNewRev.equals(psId)) {
batch = approve(control, psIdNewRev, user,
update, timestamp);
// Write update commit after all normalized label commits.
batch.write(update, new CommitBuilder());
}
db.commit();
} finally {
db.rollback();
}
indexer.index(db, cd.change());
}
private BatchMetaDataUpdate approve(ChangeControl control, PatchSet.Id psId,
IdentifiedUser user, ChangeUpdate update, Timestamp timestamp)
throws OrmException {
Map<PatchSetApproval.Key, PatchSetApproval> byKey = Maps.newHashMap();
for (PatchSetApproval psa :
approvalsUtil.byPatchSet(db, control, psId)) {
if (!byKey.containsKey(psa.getKey())) {
byKey.put(psa.getKey(), psa);
}
}
PatchSetApproval submit = new PatchSetApproval(
new PatchSetApproval.Key(
psId,
user.getAccountId(),
LabelId.SUBMIT),
(short) 1, TimeUtil.nowTs());
byKey.put(submit.getKey(), submit);
submit.setValue((short) 1);
submit.setGranted(timestamp);
// Flatten out existing approvals for this patch set based upon the current
// permissions. Once the change is closed the approvals are not updated at
// presentation view time, except for zero votes used to indicate a reviewer
// was added. So we need to make sure votes are accurate now. This way if
// permissions get modified in the future, historical records stay accurate.
LabelNormalizer.Result normalized =
labelNormalizer.normalize(control, byKey.values());
// TODO(dborowitz): Don't use a label in notedb; just check when status
// change happened.
update.putApproval(submit.getLabel(), submit.getValue());
logDebug("Adding submit label " + submit);
db.patchSetApprovals().upsert(normalized.getNormalized());
db.patchSetApprovals().delete(normalized.deleted());
try {
return saveToBatch(control, update, normalized, timestamp);
} catch (IOException e) {
throw new OrmException(e);
}
}
private BatchMetaDataUpdate saveToBatch(ChangeControl ctl,
ChangeUpdate callerUpdate, LabelNormalizer.Result normalized,
Timestamp timestamp) throws IOException {
Table<Account.Id, String, Optional<Short>> byUser = HashBasedTable.create();
for (PatchSetApproval psa : normalized.updated()) {
byUser.put(psa.getAccountId(), psa.getLabel(),
Optional.of(psa.getValue()));
}
for (PatchSetApproval psa : normalized.deleted()) {
byUser.put(psa.getAccountId(), psa.getLabel(), Optional.<Short> absent());
}
BatchMetaDataUpdate batch = callerUpdate.openUpdate();
for (Account.Id accountId : byUser.rowKeySet()) {
if (!accountId.equals(callerUpdate.getUser().getAccountId())) {
ChangeUpdate update = updateFactory.create(
ctl.forUser(identifiedUserFactory.create(accountId)), timestamp);
update.setSubject("Finalize approvals at submit");
putApprovals(update, byUser.row(accountId));
CommitBuilder commit = new CommitBuilder();
commit.setCommitter(new PersonIdent(serverIdent, timestamp));
batch.write(update, commit);
}
}
putApprovals(callerUpdate,
byUser.row(callerUpdate.getUser().getAccountId()));
return batch;
}
private static void putApprovals(ChangeUpdate update,
Map<String, Optional<Short>> approvals) {
for (Map.Entry<String, Optional<Short>> e : approvals.entrySet()) {
if (e.getValue().isPresent()) {
update.putApproval(e.getKey(), e.getValue().get());
} else {
update.removeApproval(e.getKey());
}
}
}
private void sendMergedEmail(final Change c, final PatchSetApproval from) {
workQueue.getDefaultQueue()
.submit(new Runnable() {

95
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
View File

@ -71,16 +71,13 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.io.InputStream;
import java.io.UnsupportedEncodingException;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.TimeZone;
public class MergeUtil {
private static final Logger log = LoggerFactory.getLogger(MergeUtil.class);
@ -167,10 +164,6 @@ public class MergeUtil {
return result;
}
public PatchSetApproval getSubmitter(CodeReviewCommit c) {
return approvalsUtil.getSubmitter(db.get(), c.notes(), c.getPatchsetId());
}
public RevCommit createCherryPickFromCommit(Repository repo,
ObjectInserter inserter, RevCommit mergeTip, RevCommit originalCommit,
PersonIdent cherryPickCommitterIdent, String commitMsg, RevWalk rw)
@ -347,52 +340,6 @@ public class MergeUtil {
return false;
}
public PersonIdent computeMergeCommitAuthor(final PersonIdent myIdent,
final RevWalk rw, final List<CodeReviewCommit> codeReviewCommits) {
PatchSetApproval submitter = null;
for (final CodeReviewCommit c : codeReviewCommits) {
PatchSetApproval s = getSubmitter(c);
if (submitter == null
|| (s != null && s.getGranted().compareTo(submitter.getGranted()) > 0)) {
submitter = s;
}
}
// Try to use the submitter's identity for the merge commit author.
// If all of the commits being merged are created by the submitter,
// prefer the identity line they used in the commits rather than the
// preferred identity stored in the user account. This way the Git
// commit records are more consistent internally.
//
PersonIdent authorIdent;
if (submitter != null) {
IdentifiedUser who =
identifiedUserFactory.create(submitter.getAccountId());
Set<String> emails = new HashSet<>();
for (RevCommit c : codeReviewCommits) {
try {
rw.parseBody(c);
} catch (IOException e) {
log.warn("Cannot parse commit " + c.name(), e);
continue;
}
emails.add(c.getAuthorIdent().getEmailAddress());
}
final Timestamp dt = submitter.getGranted();
final TimeZone tz = myIdent.getTimeZone();
if (emails.size() == 1 && who.hasEmailAddress(emails.iterator().next())) {
authorIdent =
new PersonIdent(codeReviewCommits.get(0).getAuthorIdent(), dt, tz);
} else {
authorIdent = who.newCommitterIdent(dt, tz);
}
} else {
authorIdent = myIdent;
}
return authorIdent;
}
public boolean canMerge(final MergeSorter mergeSorter,
final Repository repo, final CodeReviewCommit mergeTip,
final CodeReviewCommit toMerge)
@ -497,16 +444,15 @@ public class MergeUtil {
};
}
public CodeReviewCommit mergeOneCommit(final PersonIdent myIdent,
final Repository repo, final RevWalk rw, final ObjectInserter inserter,
final RevFlag canMergeFlag, final Branch.NameKey destBranch,
final CodeReviewCommit mergeTip, final CodeReviewCommit n)
throws MergeException {
public CodeReviewCommit mergeOneCommit(PersonIdent author,
PersonIdent committer, Repository repo, RevWalk rw,
ObjectInserter inserter, RevFlag canMergeFlag, Branch.NameKey destBranch,
CodeReviewCommit mergeTip, CodeReviewCommit n) throws MergeException {
final ThreeWayMerger m = newThreeWayMerger(repo, inserter);
try {
if (m.merge(new AnyObjectId[] {mergeTip, n})) {
return writeMergeCommit(myIdent, rw, inserter, canMergeFlag, destBranch,
mergeTip, m.getResultTreeId(), n);
return writeMergeCommit(author, committer, rw, inserter, canMergeFlag,
destBranch, mergeTip, m.getResultTreeId(), n);
} else {
failed(rw, canMergeFlag, mergeTip, n, CommitMergeStatus.PATH_CONFLICT);
}
@ -549,12 +495,12 @@ public class MergeUtil {
return failed;
}
public CodeReviewCommit writeMergeCommit(final PersonIdent myIdent,
final RevWalk rw, final ObjectInserter inserter,
final RevFlag canMergeFlag, final Branch.NameKey destBranch,
final CodeReviewCommit mergeTip, final ObjectId treeId,
final CodeReviewCommit n) throws IOException,
MissingObjectException, IncorrectObjectTypeException {
public CodeReviewCommit writeMergeCommit(PersonIdent author,
PersonIdent committer, RevWalk rw, ObjectInserter inserter,
RevFlag canMergeFlag, Branch.NameKey destBranch,
CodeReviewCommit mergeTip, ObjectId treeId, CodeReviewCommit n)
throws IOException, MissingObjectException,
IncorrectObjectTypeException {
final List<CodeReviewCommit> merged = new ArrayList<>();
rw.resetRetain(canMergeFlag);
rw.markStart(n);
@ -582,13 +528,11 @@ public class MergeUtil {
}
}
PersonIdent authorIdent = computeMergeCommitAuthor(myIdent, rw, merged);
final CommitBuilder mergeCommit = new CommitBuilder();
mergeCommit.setTreeId(treeId);
mergeCommit.setParentIds(mergeTip, n);
mergeCommit.setAuthor(authorIdent);
mergeCommit.setCommitter(myIdent);
mergeCommit.setAuthor(author);
mergeCommit.setCommitter(committer);
mergeCommit.setMessage(msgbuf.toString());
CodeReviewCommit mergeResult =
@ -691,7 +635,7 @@ public class MergeUtil {
return id;
}
public PatchSetApproval markCleanMerges(final RevWalk rw,
public void markCleanMerges(final RevWalk rw,
final RevFlag canMergeFlag, final CodeReviewCommit mergeTip,
final Set<RevCommit> alreadyAccepted) throws MergeException {
if (mergeTip == null) {
@ -699,12 +643,10 @@ public class MergeUtil {
// at the start of the merge process. We also elected to merge nothing,
// probably due to missing dependencies. Nothing was cleanly merged.
//
return null;
return;
}
try {
PatchSetApproval submitApproval = null;
rw.resetRetain(canMergeFlag);
rw.sort(RevSort.TOPO);
rw.sort(RevSort.REVERSE, true);
@ -717,13 +659,8 @@ public class MergeUtil {
while ((c = (CodeReviewCommit) rw.next()) != null) {
if (c.getPatchsetId() != null) {
c.setStatusCode(CommitMergeStatus.CLEAN_MERGE);
if (submitApproval == null) {
submitApproval = getSubmitter(c);
}
}
}
return submitApproval;
} catch (IOException e) {
throw new MergeException("Cannot mark clean merges", e);
}

13
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@ -1782,16 +1782,10 @@ public class ReceiveCommits {
}
private void submit(ChangeControl changeCtl, PatchSet ps)
throws OrmException, IOException, ResourceConflictException {
throws OrmException, ResourceConflictException {
Submit submit = submitProvider.get();
RevisionResource rsrc = new RevisionResource(changes.parse(changeCtl), ps);
List<Change> changes;
try {
// Force submit even if submit rule evaluation fails.
changes = submit.submit(rsrc, currentUser, true);
} catch (ResourceConflictException e) {
throw new IOException(e);
}
List<Change> changes = Lists.newArrayList(rsrc.getChange());
try {
mergeOpProvider.get().merge(ChangeSet.create(changes),
(IdentifiedUser) changeCtl.getCurrentUser(), false);
@ -1802,9 +1796,6 @@ public class ReceiveCommits {
for (Change c : changes) {
c = db.changes().get(c.getId());
switch (c.getStatus()) {
case SUBMITTED:
addMessage("Change " + c.getChangeId() + " submitted.");
break;
case MERGED:
addMessage("Change " + c.getChangeId() + " merged.");
break;

38
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
View File

@ -23,7 +23,6 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CommitMergeStatus;
@ -132,14 +131,15 @@ public class CherryPick extends SubmitStrategy {
if (args.rw.isMergedInto(mergeTip.getCurrentTip(), n)) {
mergeTip.moveTipTo(n, n);
} else {
CodeReviewCommit result = args.mergeUtil.mergeOneCommit(
args.serverIdent.get(), args.repo, args.rw, args.inserter,
PersonIdent myIdent = args.serverIdent.get();
CodeReviewCommit result = args.mergeUtil.mergeOneCommit(myIdent,
myIdent, args.repo, args.rw, args.inserter,
args.canMergeFlag, args.destBranch, mergeTip.getCurrentTip(), n);
mergeTip.moveTipTo(result, n);
}
PatchSetApproval submitApproval = args.mergeUtil.markCleanMerges(args.rw,
args.canMergeFlag, mergeTip.getCurrentTip(), args.alreadyAccepted);
setRefLogIdent(submitApproval);
args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag,
mergeTip.getCurrentTip(), args.alreadyAccepted);
setRefLogIdent();
} else {
// One or more dependencies were not met. The status was already marked on
// the commit so we have nothing further to perform at this time.
@ -153,33 +153,19 @@ public class CherryPick extends SubmitStrategy {
args.rw.parseBody(n);
PatchSetApproval submitAudit = args.mergeUtil.getSubmitter(n);
IdentifiedUser cherryPickUser;
PersonIdent serverNow = args.serverIdent.get();
PersonIdent cherryPickCommitterIdent;
if (submitAudit != null) {
cherryPickUser =
args.identifiedUserFactory.create(submitAudit.getAccountId());
cherryPickCommitterIdent = cherryPickUser.newCommitterIdent(
serverNow.getWhen(), serverNow.getTimeZone());
} else {
cherryPickUser = args.identifiedUserFactory.create(n.change().getOwner());
cherryPickCommitterIdent = serverNow;
}
String cherryPickCmtMsg = args.mergeUtil.createCherryPickCommitMessage(n);
PersonIdent committer = args.caller.newCommitterIdent(
TimeUtil.nowTs(), args.serverIdent.get().getTimeZone());
CodeReviewCommit newCommit =
(CodeReviewCommit) args.mergeUtil.createCherryPickFromCommit(args.repo,
args.inserter, mergeTip, n, cherryPickCommitterIdent,
cherryPickCmtMsg, args.rw);
args.inserter, mergeTip, n, committer, cherryPickCmtMsg, args.rw);
PatchSet.Id id =
ChangeUtil.nextPatchSetId(args.repo, n.change().currentPatchSetId());
PatchSet ps = new PatchSet(id);
ps.setCreatedOn(TimeUtil.nowTs());
ps.setUploader(cherryPickUser.getAccountId());
ps.setUploader(args.caller.getAccountId());
ps.setRevision(new RevId(newCommit.getId().getName()));
RefUpdate ru;
@ -220,9 +206,9 @@ public class CherryPick extends SubmitStrategy {
newCommit.copyFrom(n);
newCommit.setStatusCode(CommitMergeStatus.CLEAN_PICK);
newCommit.setControl(
args.changeControlFactory.controlFor(n.change(), cherryPickUser));
args.changeControlFactory.controlFor(n.change(), args.caller));
newCommits.put(newCommit.getPatchsetId().getParentKey(), newCommit);
setRefLogIdent(submitAudit);
setRefLogIdent();
return newCommit;
}

8
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/FastForwardOnly.java
View File

@ -14,7 +14,6 @@
package com.google.gerrit.server.git.strategy;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CommitMergeStatus;
import com.google.gerrit.server.git.MergeException;
@ -43,10 +42,9 @@ public class FastForwardOnly extends SubmitStrategy {
n.setStatusCode(CommitMergeStatus.NOT_FAST_FORWARD);
}
PatchSetApproval submitApproval =
args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, newMergeTipCommit,
args.alreadyAccepted);
setRefLogIdent(submitApproval);
args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag,
newMergeTipCommit, args.alreadyAccepted);
setRefLogIdent();
return mergeTip;
}

19
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeAlways.java
View File

@ -14,11 +14,12 @@
package com.google.gerrit.server.git.strategy;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.MergeException;
import com.google.gerrit.server.git.MergeTip;
import org.eclipse.jgit.lib.PersonIdent;
import java.util.Collection;
import java.util.List;
@ -42,17 +43,19 @@ public class MergeAlways extends SubmitStrategy {
}
while (!sorted.isEmpty()) {
CodeReviewCommit mergedFrom = sorted.remove(0);
PersonIdent serverIdent = args.serverIdent.get();
PersonIdent caller = args.caller.newCommitterIdent(
serverIdent.getWhen(), serverIdent.getTimeZone());
CodeReviewCommit newTip =
args.mergeUtil.mergeOneCommit(args.serverIdent.get(), args.repo, args.rw,
args.inserter, args.canMergeFlag, args.destBranch, mergeTip.getCurrentTip(),
mergedFrom);
args.mergeUtil.mergeOneCommit(caller, serverIdent,
args.repo, args.rw, args.inserter, args.canMergeFlag,
args.destBranch, mergeTip.getCurrentTip(), mergedFrom);
mergeTip.moveTipTo(newTip, mergedFrom);
}
final PatchSetApproval submitApproval =
args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, mergeTip.getCurrentTip(),
args.alreadyAccepted);
setRefLogIdent(submitApproval);
args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag,
mergeTip.getCurrentTip(), args.alreadyAccepted);
setRefLogIdent();
return mergeTip;
}

20
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/MergeIfNecessary.java
View File

@ -14,11 +14,12 @@
package com.google.gerrit.server.git.strategy;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.MergeException;
import com.google.gerrit.server.git.MergeTip;
import org.eclipse.jgit.lib.PersonIdent;
import java.util.Collection;
import java.util.List;
@ -48,18 +49,19 @@ public class MergeIfNecessary extends SubmitStrategy {
// For every other commit do a pair-wise merge.
while (!sorted.isEmpty()) {
CodeReviewCommit mergedFrom = sorted.remove(0);
PersonIdent serverIdent = args.serverIdent.get();
PersonIdent caller = args.caller.newCommitterIdent(
serverIdent.getWhen(), serverIdent.getTimeZone());
branchTip =
args.mergeUtil.mergeOneCommit(args.serverIdent.get(), args.repo,
args.rw, args.inserter, args.canMergeFlag, args.destBranch,
branchTip, mergedFrom);
args.mergeUtil.mergeOneCommit(caller, serverIdent,
args.repo, args.rw, args.inserter, args.canMergeFlag,
args.destBranch, branchTip, mergedFrom);
mergeTip.moveTipTo(branchTip, mergedFrom);
}
final PatchSetApproval submitApproval =
args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, branchTip,
args.alreadyAccepted);
setRefLogIdent(submitApproval);
args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, branchTip,
args.alreadyAccepted);
setRefLogIdent();
return mergeTip;
}

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

@ -18,7 +18,6 @@ import com.google.common.collect.Lists;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy;
import com.google.gerrit.server.change.RebaseChange;
import com.google.gerrit.server.git.CodeReviewCommit;
@ -33,6 +32,7 @@ import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import java.io.IOException;
import java.util.Collection;
@ -84,15 +84,11 @@ public class RebaseIfNecessary extends SubmitStrategy {
} else {
try {
IdentifiedUser uploader =
args.identifiedUserFactory.create(args.mergeUtil
.getSubmitter(n).getAccountId());
PatchSet newPatchSet =
rebaseChange.rebase(args.repo, args.rw, args.inserter,
n.change(), n.getPatchsetId(), uploader,
n.change(), n.getPatchsetId(), args.caller,
mergeTip.getCurrentTip(), args.mergeUtil,
args.serverIdent.get(), false, ValidatePolicy.NONE);
List<PatchSetApproval> approvals = Lists.newArrayList();
for (PatchSetApproval a : args.approvalsUtil.byPatchSet(args.db,
n.getControl(), n.getPatchsetId())) {
@ -109,13 +105,13 @@ public class RebaseIfNecessary extends SubmitStrategy {
newPatchSet.getId()));
mergeTip.getCurrentTip().copyFrom(n);
mergeTip.getCurrentTip().setControl(
args.changeControlFactory.controlFor(n.change(), uploader));
args.changeControlFactory.controlFor(n.change(), args.caller));
mergeTip.getCurrentTip().setPatchsetId(newPatchSet.getId());
mergeTip.getCurrentTip().setStatusCode(
CommitMergeStatus.CLEAN_REBASE);
newCommits.put(newPatchSet.getId().getParentKey(),
mergeTip.getCurrentTip());
setRefLogIdent(args.mergeUtil.getSubmitter(n));
setRefLogIdent();
} catch (MergeConflictException e) {
n.setStatusCode(CommitMergeStatus.REBASE_MERGE_CONFLICT);
} catch (NoSuchChangeException | OrmException | IOException
@ -135,15 +131,15 @@ public class RebaseIfNecessary extends SubmitStrategy {
if (args.rw.isMergedInto(mergeTip.getCurrentTip(), n)) {
mergeTip.moveTipTo(n, n);
} else {
PersonIdent myIdent = args.serverIdent.get();
mergeTip.moveTipTo(
args.mergeUtil.mergeOneCommit(args.serverIdent.get(),
args.mergeUtil.mergeOneCommit(myIdent, myIdent,
args.repo, args.rw, args.inserter, args.canMergeFlag,
args.destBranch, mergeTip.getCurrentTip(), n), n);
}
PatchSetApproval submitApproval =
args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag,
mergeTip.getCurrentTip(), args.alreadyAccepted);
setRefLogIdent(submitApproval);
args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag,
mergeTip.getCurrentTip(), args.alreadyAccepted);
setRefLogIdent();
} catch (IOException e) {
throw new MergeException("Cannot merge " + n.name(), e);
}

16
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java
View File

@ -14,10 +14,11 @@
package com.google.gerrit.server.git.strategy;
import static com.google.common.base.Preconditions.checkState;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.IdentifiedUser;
@ -66,6 +67,7 @@ public abstract class SubmitStrategy {
protected final MergeUtil mergeUtil;
protected final ChangeIndexer indexer;
protected final MergeSorter mergeSorter;
protected final IdentifiedUser caller;
Arguments(IdentifiedUser.GenericFactory identifiedUserFactory,
Provider<PersonIdent> serverIdent, ReviewDb db,
@ -73,7 +75,7 @@ public abstract class SubmitStrategy {
RevWalk rw, ObjectInserter inserter, RevFlag canMergeFlag,
Set<RevCommit> alreadyAccepted, Branch.NameKey destBranch,
ApprovalsUtil approvalsUtil, MergeUtil mergeUtil,
ChangeIndexer indexer) {
ChangeIndexer indexer, IdentifiedUser caller) {
this.identifiedUserFactory = identifiedUserFactory;
this.serverIdent = serverIdent;
this.db = db;
@ -89,6 +91,7 @@ public abstract class SubmitStrategy {
this.mergeUtil = mergeUtil;
this.indexer = indexer;
this.mergeSorter = new MergeSorter(rw, alreadyAccepted, canMergeFlag);
this.caller = caller;
}
}
@ -115,6 +118,7 @@ public abstract class SubmitStrategy {
public final MergeTip run(final CodeReviewCommit currentTip,
final Collection<CodeReviewCommit> toMerge) throws MergeException {
refLogIdent = null;
checkState(args.caller != null);
return _run(currentTip, toMerge);
}
@ -182,13 +186,11 @@ public abstract class SubmitStrategy {
/**
* Set the ref log identity if it wasn't set yet.
*
* @param submitApproval the approval that submitted the patch set
*/
protected final void setRefLogIdent(PatchSetApproval submitApproval) {
if (refLogIdent == null && submitApproval != null) {
protected final void setRefLogIdent() {
if (refLogIdent == null) {
refLogIdent = args.identifiedUserFactory.create(
submitApproval.getAccountId()) .newRefLogIdent();
args.caller.getAccountId()).newRefLogIdent();
}
}
}

4
gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyFactory.java
View File

@ -88,14 +88,14 @@ public class SubmitStrategyFactory {
public SubmitStrategy create(final SubmitType submitType, final ReviewDb db,
final Repository repo, final RevWalk rw, final ObjectInserter inserter,
final RevFlag canMergeFlag, final Set<RevCommit> alreadyAccepted,
final Branch.NameKey destBranch)
final Branch.NameKey destBranch, final IdentifiedUser caller)
throws MergeException, NoSuchProjectException {
ProjectState project = getProject(destBranch);
final SubmitStrategy.Arguments args =
new SubmitStrategy.Arguments(identifiedUserFactory, myIdent, db,
changeControlFactory, repo, rw, inserter, canMergeFlag,
alreadyAccepted, destBranch,approvalsUtil,
mergeUtilFactory.create(project), indexer);
mergeUtilFactory.create(project), indexer, caller);
switch (submitType) {
case CHERRY_PICK:
return new CherryPick(args, patchSetInfoFactory, gitRefUpdated);

6
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
View File

@ -164,7 +164,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
}
public void setStatus(Change.Status status) {
checkArgument(status != Change.Status.SUBMITTED,
checkArgument(status != Change.Status.MERGED,
"use submit(Iterable<PatchSetApproval>)");
this.status = status;
}
@ -177,8 +177,8 @@ public class ChangeUpdate extends AbstractChangeUpdate {
approvals.put(label, Optional.<Short> absent());
}
public void submit(Iterable<SubmitRecord> submitRecords) {
status = Change.Status.SUBMITTED;
public void merge(Iterable<SubmitRecord> submitRecords) {
this.status = Change.Status.MERGED;
this.submitRecords = ImmutableList.copyOf(submitRecords);
checkArgument(!this.submitRecords.isEmpty(),
"no submit records specified at submit time");

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

@ -117,7 +117,7 @@ class ConflictsPredicate extends OrPredicate<ChangeData> {
args.submitStrategyFactory.create(submitType,
db.get(), repo, rw, null, canMergeFlag,
getAlreadyAccepted(repo, rw, commit),
otherChange.getDest());
otherChange.getDest(), null);
CodeReviewCommit otherCommit =
(CodeReviewCommit) rw.parseCommit(other);
otherCommit.add(canMergeFlag);

11
gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
View File

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

2
gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
View File

@ -32,7 +32,7 @@ import java.util.List;
/** A version of the database schema. */
public abstract class SchemaVersion {
/** 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() {
return guessVersion(C);

39
gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_109.java Normal file
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");
}
}

11
gerrit-server/src/test/java/com/google/gerrit/server/index/IndexRewriteTest.java
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.MERGED;
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.assertSame;
import static org.junit.Assert.assertTrue;
@ -183,17 +182,17 @@ public class IndexRewriteTest {
public void testGetPossibleStatus() throws Exception {
assertEquals(EnumSet.allOf(Change.Status.class), status("file:a"));
assertEquals(EnumSet.of(NEW), status("is:new"));
assertEquals(EnumSet.of(SUBMITTED, DRAFT, MERGED, ABANDONED),
assertEquals(EnumSet.of(DRAFT, MERGED, ABANDONED),
status("-is:new"));
assertEquals(EnumSet.of(NEW, MERGED), status("is:new OR is:merged"));
EnumSet<Change.Status> none = EnumSet.noneOf(Change.Status.class);
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 is:submitted)"));
assertEquals(none, status("(is:new is:draft) (is:merged)"));
assertEquals(none, status("(is:new is:draft) (is:merged)"));
assertEquals(EnumSet.of(MERGED, SUBMITTED),
status("(is:new is:draft) OR (is:merged OR is:submitted)"));
assertEquals(EnumSet.of(MERGED),
status("(is:new is:draft) OR (is:merged)"));
}
@Test

6
gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
View File

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

8
gerrit-server/src/test/java/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
View File

@ -108,7 +108,7 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest {
ChangeUpdate update = newUpdate(c, changeOwner);
update.setSubject("Submit patch set 1");
update.submit(ImmutableList.of(
update.merge(ImmutableList.of(
submitRecord("NOT_READY", null,
submitLabel("Verified", "OK", changeOwner.getAccountId()),
submitLabel("Code-Review", "NEED", null)),
@ -121,7 +121,7 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest {
assertBodyEquals("Submit patch set 1\n"
+ "\n"
+ "Patch-set: 1\n"
+ "Status: submitted\n"
+ "Status: merged\n"
+ "Submitted-with: NOT_READY\n"
+ "Submitted-with: OK: Verified: Change Owner <1@gerrit>\n"
+ "Submitted-with: NEED: Code-Review\n"
@ -173,14 +173,14 @@ public class CommitMessageOutputTest extends AbstractChangeNotesTest {
ChangeUpdate update = newUpdate(c, changeOwner);
update.setSubject("Submit patch set 1");
update.submit(ImmutableList.of(
update.merge(ImmutableList.of(
submitRecord("RULE_ERROR", "Problem with patch set:\n1")));
update.commit();
assertBodyEquals("Submit patch set 1\n"
+ "\n"
+ "Patch-set: 1\n"
+ "Status: submitted\n"
+ "Status: merged\n"
+ "Submitted-with: RULE_ERROR Problem with patch set: 1\n",
update.getRevision());
}