opendev/gerrit
Code Issues Proposed changes

Merging Changes: Do not enter submitted state

Browse Source 
Recently the MergeQueue was removed, such that the submit process
will directly tell users the result of their submission (merge or reject).
For that changes were submitted and directly merged after. If problems
(such as a server going down, power loss etc.) occur between setting the
submitted state and the actual merge being performed, these changes would
be stuck in the submitted state, which is annoying UI wise, but the user
could just try to resubmit once the problems are solved.

This change moves changes directly from NEW to MERGED if possible or
rejects otherwise. We still need to keep lots of code in the Submit class
to record the Submit label. Eventually this can be moved into the MergeOp
as well.

The MergeOp doesn't rely on the submitted state as well any more but
just integrates the given changes. So changes which are submitted, but
not merged on the same branch are left untouched now. So the tests which
are testing exactly these corner cases of submitted changes on the same
branch are adapted.

Currently the test `submitMultipleChanges` is rather testing that the
other changes on the same target branch are left alone. Instead we
want to test if the submit strategy can submit changes both via
fast forward as well as merging if it's necessary. After applying
this change the name of `submitMultipleChanges` is correct again.

There is no access to the submitter when running a submit strategy
as it is not in the submitted state any more, so we need another way
of transporting the information of who is trying to integrate the change
to the submit strategy, so we do that via another argument in
SubmitStrategy$Argument.

The CherryPick strategy has a slight change in behavior as well.
The committer for a cherry-picked change will be the server instead
of the submitter as this aligns better with the other submit strategies,
creating new commits on behalf of the user.

Change-Id: I9a0c111c1cb3544774b2783c4f76318e2634faf2
This commit is contained in:
Stefan Beller
2015-07-10 13:30:21 -07:00
parent 8792bd67d7
commit 7063a4b8f0
25 changed files with 361 additions and 599 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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) {

4
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;
@@ -120,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());
}

36
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;
@@ -72,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;
@@ -146,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 {
@@ -203,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();
@@ -266,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");
@@ -294,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);

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

@@ -66,8 +66,7 @@ public class SubmitByCherryPickIT extends AbstractSubmit {
assertSubmitter(change2.getChangeId(), 1);
assertSubmitter(change2.getChangeId(), 2);
assertPersonEquals(admin.getIdent(), newHead.getAuthorIdent());
// TODO: Check the committer, too.
// assertPersonEquals(admin.getIdent(), newHead.getCommitterIdent());
assertPersonEquals(admin.getIdent(), newHead.getCommitterIdent());
}
@Test
@@ -109,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
@@ -149,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
@@ -165,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
@@ -234,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
@@ -241,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());
}
}

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

@@ -58,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());
}
}

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

@@ -46,25 +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.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.getShortMessage()).isEqualTo(
change2.getCommit().getShortMessage());
assertThat(tip.getParent(0).getId()).isEqualTo(initialHead.getId());
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());
assertThat(tip.getParent(0).getShortMessage()).isEqualTo(
change3.getCommit().getShortMessage());
assertPersonEquals(admin.getIdent(), tip.getAuthorIdent());
assertPersonEquals(serverIdent.get(), tip.getCommitterIdent());
assertNew(change2.getChangeId());
}
@Test
@@ -188,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());
@@ -195,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);
}
}

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

@@ -111,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);

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;

246
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;
@@ -120,17 +102,11 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
}
}
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 +117,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 +176,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);
@@ -374,203 +347,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() {

255
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() {
@@ -384,35 +393,7 @@ public class MergeOp {
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 +409,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 +428,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 +440,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 +474,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 +503,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 +522,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 +626,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 +654,9 @@ 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.SUBMITTED
&& chg.getStatus() != Change.Status.NEW) {
logDebug("Change {} is not new or submitted: {}", changeId, chg.getStatus());
continue;
}
if (chg.currentPatchSetId() == null) {
@@ -762,7 +740,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 +859,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 +870,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 +883,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 +1081,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.submit(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);
}

10
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);

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);

4
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;
}
@@ -178,7 +178,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
}
public void submit(Iterable<SubmitRecord> submitRecords) {
status = Change.Status.SUBMITTED;
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);

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

@@ -108,13 +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));
}

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

@@ -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"
@@ -180,7 +180,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: RULE_ERROR Problem with patch set: 1\n",
update.getRevision());
}