diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 153752d838..03849937f9 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -1066,6 +1066,268 @@ message is contained in the response body. blocked by Verified ---- +[[submitted_together]] +=== Changes submitted together +-- +'GET /changes/link:#change-id[\{change-id\}]/submitted_together' +-- + +Returns a list of all changes which are submitted when +link:#submit-change[\{submit\}] is called for this change. + +.Request +---- + GET /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/submitted_together HTTP/1.0 + Content-Type: application/json; charset=UTF-8 +---- + +The return value is a list of changes in the same format as in +link:#list-changes[\{listing changes\}] with the options +link:#labels[\{LABELS\}], link:#detailed-labels[\{DETAILED_LABELS\}], +link:#current-revision[\{CURRENT_REVISION\}], +link:#current-commit[\{CURRENT_COMMIT\}] set. +The list consists of: + +* The given change. +* If link:config-gerrit.html#change.submitWholeTopic[`change.submitWholeTopic`] + is enabled, include all open changes with the same topic. +* For each change whose submit type is not CHERRY_PICK, include unmerged + ancestors targeting the same branch. + +.Response +---- + HTTP/1.1 200 OK + Content-Disposition: attachment + Content-Type: application/json; charset=UTF-8 + +)]}' +[ + { + "id": "gerrit~master~I1ffe09a505e25f15ce1521bcfb222e51e62c2a14", + "project": "gerrit", + "branch": "master", + "hashtags": [], + "change_id": "I1ffe09a505e25f15ce1521bcfb222e51e62c2a14", + "subject": "ChangeMergeQueue: Rewrite such that it works on set of changes", + "status": "NEW", + "created": "2015-05-01 15:39:57.979000000", + "updated": "2015-05-20 19:25:21.592000000", + "mergeable": true, + "insertions": 303, + "deletions": 210, + "_number": 1779, + "owner": { + "_account_id": 1000000 + }, + "labels": { + "Code-Review": { + "approved": { + "_account_id": 1000000 + }, + "all": [ + { + "value": 2, + "date": "2015-05-20 19:25:21.592000000", + "_account_id": 1000000 + } + ], + "values": { + "-2": "This shall not be merged", + "-1": "I would prefer this is not merged as is", + " 0": "No score", + "+1": "Looks good to me, but someone else must approve", + "+2": "Looks good to me, approved" + }, + "default_value": 0 + }, + "Verified": { + "approved": { + "_account_id": 1000000 + }, + "all": [ + { + "value": 1, + "date": "2015-05-20 19:25:21.592000000", + "_account_id": 1000000 + } + ], + "values": { + "-1": "Fails", + " 0": "No score", + "+1": "Verified" + }, + "default_value": 0 + } + }, + "permitted_labels": { + "Code-Review": [ + "-2", + "-1", + " 0", + "+1", + "+2" + ], + "Verified": [ + "-1", + " 0", + "+1" + ] + }, + "removable_reviewers": [ + { + "_account_id": 1000000 + } + ], + "current_revision": "9adb9f4c7b40eeee0646e235de818d09164d7379", + "revisions": { + "9adb9f4c7b40eeee0646e235de818d09164d7379": { + "_number": 1, + "created": "2015-05-01 15:39:57.979000000", + "uploader": { + "_account_id": 1000000 + }, + "ref": "refs/changes/79/1779/1", + "fetch": {}, + "commit": { + "parents": [ + { + "commit": "2d3176497a2747faed075f163707e57d9f961a1c", + "subject": "Merge changes from topic \u0027submodule-subscription-tests-and-fixes-3\u0027" + } + ], + "author": { + "name": "Stefan Beller", + "email": "sbeller@google.com", + "date": "2015-04-29 21:36:52.000000000", + "tz": -420 + }, + "committer": { + "name": "Stefan Beller", + "email": "sbeller@google.com", + "date": "2015-05-01 00:11:16.000000000", + "tz": -420 + }, + "subject": "ChangeMergeQueue: Rewrite such that it works on set of changes", + "message": "ChangeMergeQueue: Rewrite such that it works on set of changes\n\nChangeMergeQueue used to work on branches rather than sets of changes.\nThis change is a first step to merge sets of changes (e.g. grouped by a\ntopic and `changes.submitWholeTopic` enabled) in an atomic fashion.\nThis change doesn\u0027t aim to implement these changes, but only as a step\ntowards it.\n\nMergeOp keeps its functionality and behavior as is. A new class\nMergeOpMapper is introduced which will map the set of changes to\nthe set of branches. Additionally the MergeOpMapper is also\nresponsible for the threading done right now, which was part of\nthe ChangeMergeQueue before.\n\nChange-Id: I1ffe09a505e25f15ce1521bcfb222e51e62c2a14\n" + } + } + } + }, + { + "id": "gerrit~master~I7fe807e63792b3d26776fd1422e5e790a5697e22", + "project": "gerrit", + "branch": "master", + "hashtags": [], + "change_id": "I7fe807e63792b3d26776fd1422e5e790a5697e22", + "subject": "AbstractSubmoduleSubscription: Split up createSubscription", + "status": "NEW", + "created": "2015-05-01 15:39:57.979000000", + "updated": "2015-05-20 19:25:21.546000000", + "mergeable": true, + "insertions": 15, + "deletions": 6, + "_number": 1780, + "owner": { + "_account_id": 1000000 + }, + "labels": { + "Code-Review": { + "approved": { + "_account_id": 1000000 + }, + "all": [ + { + "value": 2, + "date": "2015-05-20 19:25:21.546000000", + "_account_id": 1000000 + } + ], + "values": { + "-2": "This shall not be merged", + "-1": "I would prefer this is not merged as is", + " 0": "No score", + "+1": "Looks good to me, but someone else must approve", + "+2": "Looks good to me, approved" + }, + "default_value": 0 + }, + "Verified": { + "approved": { + "_account_id": 1000000 + }, + "all": [ + { + "value": 1, + "date": "2015-05-20 19:25:21.546000000", + "_account_id": 1000000 + } + ], + "values": { + "-1": "Fails", + " 0": "No score", + "+1": "Verified" + }, + "default_value": 0 + } + }, + "permitted_labels": { + "Code-Review": [ + "-2", + "-1", + " 0", + "+1", + "+2" + ], + "Verified": [ + "-1", + " 0", + "+1" + ] + }, + "removable_reviewers": [ + { + "_account_id": 1000000 + } + ], + "current_revision": "1bd7c12a38854a2c6de426feec28800623f492c4", + "revisions": { + "1bd7c12a38854a2c6de426feec28800623f492c4": { + "_number": 1, + "created": "2015-05-01 15:39:57.979000000", + "uploader": { + "_account_id": 1000000 + }, + "ref": "refs/changes/80/1780/1", + "fetch": {}, + "commit": { + "parents": [ + { + "commit": "9adb9f4c7b40eeee0646e235de818d09164d7379", + "subject": "ChangeMergeQueue: Rewrite such that it works on set of changes" + } + ], + "author": { + "name": "Stefan Beller", + "email": "sbeller@google.com", + "date": "2015-04-25 00:11:59.000000000", + "tz": -420 + }, + "committer": { + "name": "Stefan Beller", + "email": "sbeller@google.com", + "date": "2015-05-01 00:11:16.000000000", + "tz": -420 + }, + "subject": "AbstractSubmoduleSubscription: Split up createSubscription", + "message": "AbstractSubmoduleSubscription: Split up createSubscription\n\nLater we want to have subscriptions to more submodules, so we need to\nfind a way to add more submodule entries into the file. By splitting up\nthe createSubscription() method, that is very easy by using the\naddSubmoduleSubscription method multiple times.\n\nChange-Id: I7fe807e63792b3d26776fd1422e5e790a5697e22\n" + } + } + } + } +] +---- + + [[publish-draft-change]] === Publish Draft Change -- diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java new file mode 100644 index 0000000000..c15c1085b0 --- /dev/null +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java @@ -0,0 +1,134 @@ +// 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.acceptance.server.change; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.acceptance.GitUtil.pushHead; + +import com.google.common.base.Function; +import com.google.common.collect.Iterables; +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.GitUtil; +import com.google.gerrit.extensions.client.SubmitType; +import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.server.git.ProjectConfig; + +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; +import org.junit.Test; + +import java.io.IOException; +import java.util.Arrays; +import java.util.List; + +public class SubmittedTogetherIT extends AbstractDaemonTest { + + @Test + public void returnsAncestors() throws Exception { + // Create two commits and push. + RevCommit c1_1 = commitBuilder() + .add("a.txt", "1") + .message("subject: 1") + .create(); + String id1 = getChangeId(c1_1); + RevCommit c2_1 = commitBuilder() + .add("b.txt", "2") + .message("subject: 2") + .create(); + String id2 = getChangeId(c2_1); + pushHead(testRepo, "refs/for/master", false); + + assertSubmittedTogether(id1, id1); + assertSubmittedTogether(id2, id2, id1); + } + + @Test + public void respectsWholeTopicAndAncestors() throws Exception { + RevCommit initialHead = getRemoteHead(); + // Create two independant commits and push. + RevCommit c1_1 = commitBuilder() + .add("a.txt", "1") + .message("subject: 1") + .create(); + String id1 = getChangeId(c1_1); + pushHead(testRepo, "refs/for/master/" + name("connectingTopic"), false); + + testRepo.reset(initialHead); + RevCommit c2_1 = commitBuilder() + .add("b.txt", "2") + .message("subject: 2") + .create(); + String id2 = getChangeId(c2_1); + pushHead(testRepo, "refs/for/master/" + name("connectingTopic"), false); + + if (isSubmitWholeTopicEnabled()) { + assertSubmittedTogether(id1, id2, id1); + assertSubmittedTogether(id2, id2, id1); + } else { + assertSubmittedTogether(id1, id1); + assertSubmittedTogether(id2, id2); + } + } + + @Test + public void testCherryPickWithoutAncestors() throws Exception { + ProjectConfig cfg = projectCache.checkedGet(project).getConfig(); + cfg.getProject().setSubmitType(SubmitType.CHERRY_PICK); + saveProjectConfig(project, cfg); + + // Create two commits and push. + RevCommit c1_1 = commitBuilder() + .add("a.txt", "1") + .message("subject: 1") + .create(); + String id1 = getChangeId(c1_1); + RevCommit c2_1 = commitBuilder() + .add("b.txt", "2") + .message("subject: 2") + .create(); + String id2 = getChangeId(c2_1); + pushHead(testRepo, "refs/for/master", false); + + assertSubmittedTogether(id1, id1); + assertSubmittedTogether(id2, id2); + } + + private void assertSubmittedTogether(String chId, String... expected) + throws Exception { + List actual = gApi.changes().id(chId).submittedTogether(); + assertThat(actual).hasSize(expected.length); + assertThat(Arrays.asList(expected)) + .containsExactlyElementsIn( + Iterables.transform(actual, new Function() { + @Override + public String apply(ChangeInfo input) { + return input.changeId; + } + })).inOrder(); + } + + private RevCommit getRemoteHead() throws IOException { + try (Repository repo = repoManager.openRepository(project); + RevWalk rw = new RevWalk(repo)) { + return rw.parseCommit(repo.getRef("refs/heads/master").getObjectId()); + } + } + + private String getChangeId(RevCommit c) throws Exception { + return GitUtil.getChangeId(testRepo, c).get(); + } +} \ No newline at end of file diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java index 6781af275a..ce070984bb 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java @@ -78,6 +78,8 @@ public interface ChangeApi { */ ChangeApi revert(RevertInput in) throws RestApiException; + List submittedTogether() throws RestApiException; + String topic() throws RestApiException; void topic(String topic) throws RestApiException; @@ -288,5 +290,10 @@ public interface ChangeApi { public ChangeInfo check(FixInput fix) throws RestApiException { throw new NotImplementedException(); } + + @Override + public List submittedTogether() throws RestApiException { + throw new NotImplementedException(); + } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java index 78b3f10c00..85cae58e60 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java @@ -49,6 +49,7 @@ import com.google.gerrit.server.change.PutTopic; import com.google.gerrit.server.change.Restore; import com.google.gerrit.server.change.Revert; import com.google.gerrit.server.change.Revisions; +import com.google.gerrit.server.change.SubmittedTogether; import com.google.gerrit.server.change.SuggestReviewers; import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gwtorm.server.OrmException; @@ -76,6 +77,7 @@ class ChangeApiImpl implements ChangeApi { private final Abandon abandon; private final Revert revert; private final Restore restore; + private final SubmittedTogether submittedTogether; private final GetTopic getTopic; private final PutTopic putTopic; private final PostReviewers postReviewers; @@ -96,6 +98,7 @@ class ChangeApiImpl implements ChangeApi { Abandon abandon, Revert revert, Restore restore, + SubmittedTogether submittedTogether, GetTopic getTopic, PutTopic putTopic, PostReviewers postReviewers, @@ -115,6 +118,7 @@ class ChangeApiImpl implements ChangeApi { this.suggestReviewers = suggestReviewers; this.abandon = abandon; this.restore = restore; + this.submittedTogether = submittedTogether; this.getTopic = getTopic; this.putTopic = putTopic; this.postReviewers = postReviewers; @@ -195,6 +199,15 @@ class ChangeApiImpl implements ChangeApi { } } + @Override + public List submittedTogether() throws RestApiException { + try { + return submittedTogether.apply(change); + } catch (Exception e) { + throw new RestApiException("Cannot query submittedTogether", e); + } + } + @Override public String topic() throws RestApiException { return getTopic.apply(change); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java index 77e44fa423..f15276905c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java @@ -223,17 +223,37 @@ public class ChangeJson { return format(changeDataFactory.create(db.get(), c)); } - public ChangeInfo format(ChangeData cd) throws OrmException { - return format(cd, Optional. absent()); + public List format(Collection ids) throws OrmException { + List changes = new ArrayList<>(ids.size()); + List ret = new ArrayList<>(ids.size()); + ReviewDb reviewDb = db.get(); + for (Change.Id id : ids) { + changes.add(changeDataFactory.create(reviewDb, id)); + } + accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS)); + for (ChangeData cd : changes) { + ret.add(format(cd, Optional. absent(), false)); + } + accountLoader.fill(); + return ret; } - private ChangeInfo format(ChangeData cd, Optional limitToPsId) + public ChangeInfo format(ChangeData cd) throws OrmException { + return format(cd, Optional. absent(), true); + } + + private ChangeInfo format(ChangeData cd, Optional limitToPsId, + boolean fillAccountLoader) throws OrmException { try { - accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS)); - ChangeInfo res = toChangeInfo(cd, limitToPsId); - accountLoader.fill(); - return res; + if (fillAccountLoader) { + accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS)); + ChangeInfo res = toChangeInfo(cd, limitToPsId); + accountLoader.fill(); + return res; + } else { + return toChangeInfo(cd, limitToPsId); + } } catch (PatchListNotAvailableException | OrmException | IOException | RuntimeException e) { if (!has(CHECK)) { @@ -246,7 +266,7 @@ public class ChangeJson { public ChangeInfo format(RevisionResource rsrc) throws OrmException { ChangeData cd = changeDataFactory.create(db.get(), rsrc.getControl()); - return format(cd, Optional.of(rsrc.getPatchSet().getId())); + return format(cd, Optional.of(rsrc.getPatchSet().getId()), true); } public List> formatQueryResults(List in) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java index b73af9318b..f92b13d9a7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java @@ -65,6 +65,7 @@ public class Module extends RestApiModule { post(CHANGE_KIND, "restore").to(Restore.class); post(CHANGE_KIND, "revert").to(Revert.class); post(CHANGE_KIND, "submit").to(Submit.CurrentRevision.class); + get(CHANGE_KIND, "submitted_together").to(SubmittedTogether.class); post(CHANGE_KIND, "rebase").to(Rebase.CurrentRevision.class); post(CHANGE_KIND, "index").to(Index.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java new file mode 100644 index 0000000000..c909239c9a --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java @@ -0,0 +1,75 @@ +// 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.change; + +import com.google.gerrit.extensions.client.ListChangesOption; +import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.BadRequestException; +import com.google.gerrit.extensions.restapi.ResourceConflictException; +import com.google.gerrit.extensions.restapi.RestReadView; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.git.ChangeSet; +import com.google.gerrit.server.git.MergeSuperSet; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.Provider; +import com.google.inject.Singleton; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.util.EnumSet; +import java.util.List; + +@Singleton +public class SubmittedTogether implements RestReadView { + private static final Logger log = LoggerFactory.getLogger( + SubmittedTogether.class); + + private final ChangeJson json; + + private final Provider dbProvider; + private final MergeSuperSet mergeSuperSet; + + @Inject + SubmittedTogether(ChangeJson json, + Provider dbProvider, + MergeSuperSet mergeSuperSet) { + this.json = json; + this.dbProvider = dbProvider; + this.mergeSuperSet = mergeSuperSet; + } + + @Override + public List apply(ChangeResource resource) + throws AuthException, BadRequestException, + ResourceConflictException, Exception { + try { + ChangeSet cs = mergeSuperSet.completeChangeSet(dbProvider.get(), + ChangeSet.create(resource.getChange())); + json.addOptions(EnumSet.of( + ListChangesOption.CURRENT_REVISION, + ListChangesOption.CURRENT_COMMIT, + ListChangesOption.DETAILED_LABELS, + ListChangesOption.LABELS)); + return json.format(cs.ids()); + } catch (OrmException | IOException e) { + log.error("Error on getting a ChangeSet", e); + throw e; + } + } +}