diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java index 100e64232f..5b891107c1 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java @@ -47,8 +47,18 @@ import com.google.gerrit.extensions.common.MergeableInfo; import com.google.gerrit.extensions.common.RevisionInfo; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BinaryResult; +import com.google.gerrit.extensions.restapi.ETagView; +import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.reviewdb.client.Patch; +import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.server.ChangeUtil; +import com.google.gerrit.server.change.ChangeResource; +import com.google.gerrit.server.change.GetRevisionActions; +import com.google.gerrit.server.change.RevisionResource; +import com.google.gerrit.server.change.Revisions; +import com.google.gerrit.server.project.ChangeControl; +import com.google.inject.Inject; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.RefUpdate; @@ -69,6 +79,15 @@ import java.util.Map; @NoHttpd public class RevisionIT extends AbstractDaemonTest { + @Inject + private ChangeUtil changeUtil; + + @Inject + private GetRevisionActions getRevisionActions; + + @Inject + private Revisions revisions; + private TestAccount admin2; @Before @@ -618,6 +637,38 @@ public class RevisionIT extends AbstractDaemonTest { String.format(PATCH, r.getCommitId().name(), date, r.getChangeId())); } + @Test + public void actions() throws Exception { + PushOneCommit.Result r = createChange(); + assertThat(current(r).actions().keySet()) + .containsExactly("cherrypick", "rebase"); + + current(r).review(ReviewInput.approve()); + assertThat(current(r).actions().keySet()) + .containsExactly("submit", "cherrypick", "rebase"); + + current(r).submit(); + assertThat(current(r).actions().keySet()) + .containsExactly("cherrypick"); + } + + @Test + public void actionsETag() throws Exception { + PushOneCommit.Result r1 = createChange(); + PushOneCommit.Result r2 = createChange(); + + String oldETag = checkETag(getRevisionActions, r2, null); + current(r2).review(ReviewInput.approve()); + oldETag = checkETag(getRevisionActions, r2, oldETag); + + // Dependent change is included in ETag. + current(r1).review(ReviewInput.approve()); + oldETag = checkETag(getRevisionActions, r2, oldETag); + + current(r2).submit(); + oldETag = checkETag(getRevisionActions, r2, oldETag); + } + private void merge(PushOneCommit.Result r) throws Exception { revision(r).review(ReviewInput.approve()); revision(r).submit(); @@ -634,4 +685,26 @@ public class RevisionIT extends AbstractDaemonTest { PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo); return push.to("refs/drafts/master"); } + + private RevisionApi current(PushOneCommit.Result r) throws Exception { + return gApi.changes().id(r.getChangeId()).current(); + } + + private RevisionResource parseRevisionResource(PushOneCommit.Result r) + throws Exception { + PatchSet.Id psId = r.getPatchSetId(); + List ctls = changeUtil.findChanges( + Integer.toString(psId.getParentKey().get()), atrScope.get().getUser()); + assertThat(ctls).hasSize(1); + return revisions.parse( + new ChangeResource(ctls.get(0)), + IdString.fromDecoded(Integer.toString(psId.get()))); + } + + private String checkETag(ETagView view, + PushOneCommit.Result r, String oldETag) throws Exception { + String eTag = view.getETag(parseRevisionResource(r)); + assertThat(eTag).isNotEqualTo(oldETag); + return eTag; + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index f81c128bf1..c3c99d078e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -555,6 +555,8 @@ public class MergeOp implements AutoCloseable { logDebug("Beginning integration of {}", change); try { ChangeSet cs = mergeSuperSet.completeChangeSet(db, change); + checkState(cs.ids().contains(change.getId()), + "change %s missing from %s", change.getId(), cs); this.commits = new CommitStatus(cs); reloadChanges(cs); logDebug("Calculated to merge {}", cs); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java index 321b8fc649..b38304227f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java @@ -14,8 +14,6 @@ package com.google.gerrit.server.git; -import static com.google.common.base.Preconditions.checkState; - import com.google.common.base.Strings; import com.google.common.collect.Multimap; import com.google.gerrit.common.data.SubmitTypeRecord; @@ -86,15 +84,11 @@ public class MergeSuperSet { throws MissingObjectException, IncorrectObjectTypeException, IOException, OrmException { ChangeData cd = changeDataFactory.create(db, change.getId()); - ChangeSet result; if (Submit.wholeTopicEnabled(cfg)) { - result = completeChangeSetIncludingTopics(db, new ChangeSet(cd)); + return completeChangeSetIncludingTopics(db, new ChangeSet(cd)); } else { - result = completeChangeSetWithoutTopic(db, new ChangeSet(cd)); + return completeChangeSetWithoutTopic(db, new ChangeSet(cd)); } - checkState(result.ids().contains(change.getId()), - "change %s missing from result %s", change.getId(), result); - return result; } private ChangeSet completeChangeSetWithoutTopic(ReviewDb db, ChangeSet changes) @@ -139,7 +133,8 @@ public class MergeSuperSet { List hashes = new ArrayList<>(); // Always include the input, even if merged. This allows - // SubmitStrategyOp to correct the situation later. + // SubmitStrategyOp to correct the situation later, assuming it gets + // returned by byCommitsOnBranchNotMerged below. hashes.add(objIdStr); for (RevCommit c : rw) { if (!c.equals(commit)) { @@ -148,7 +143,6 @@ public class MergeSuperSet { } if (!hashes.isEmpty()) { - // Merged changes are ok to exclude Iterable destChanges = queryProvider.get() .byCommitsOnBranchNotMerged( repo, db, cd.change().getDest(), hashes);