Add changes/<id>/submitted_together REST API call

This REST API call will answer the set of changes
which will be affected by submitting the given
change.

The options set are the same as were used for the
the 'same topic' tab, such that this call can be used
to service that tab eventually.

Change-Id: Ie1a3a72db132b3fda63f2602213269641c8d943f
This commit is contained in:
Stefan Beller 2015-06-26 10:05:43 -07:00
parent 27a5a068b4
commit a7ad6614df
7 changed files with 520 additions and 8 deletions

View File

@ -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
--

View File

@ -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<ChangeInfo> actual = gApi.changes().id(chId).submittedTogether();
assertThat(actual).hasSize(expected.length);
assertThat(Arrays.asList(expected))
.containsExactlyElementsIn(
Iterables.transform(actual, new Function<ChangeInfo, String>() {
@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();
}
}

View File

@ -78,6 +78,8 @@ public interface ChangeApi {
*/
ChangeApi revert(RevertInput in) throws RestApiException;
List<ChangeInfo> 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<ChangeInfo> submittedTogether() throws RestApiException {
throw new NotImplementedException();
}
}
}

View File

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

View File

@ -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.<PatchSet.Id> absent());
public List<ChangeInfo> format(Collection<Change.Id> ids) throws OrmException {
List<ChangeData> changes = new ArrayList<>(ids.size());
List<ChangeInfo> 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.<PatchSet.Id> absent(), false));
}
accountLoader.fill();
return ret;
}
private ChangeInfo format(ChangeData cd, Optional<PatchSet.Id> limitToPsId)
public ChangeInfo format(ChangeData cd) throws OrmException {
return format(cd, Optional.<PatchSet.Id> absent(), true);
}
private ChangeInfo format(ChangeData cd, Optional<PatchSet.Id> 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<List<ChangeInfo>> formatQueryResults(List<QueryResult> in)

View File

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

View File

@ -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<ChangeResource> {
private static final Logger log = LoggerFactory.getLogger(
SubmittedTogether.class);
private final ChangeJson json;
private final Provider<ReviewDb> dbProvider;
private final MergeSuperSet mergeSuperSet;
@Inject
SubmittedTogether(ChangeJson json,
Provider<ReviewDb> dbProvider,
MergeSuperSet mergeSuperSet) {
this.json = json;
this.dbProvider = dbProvider;
this.mergeSuperSet = mergeSuperSet;
}
@Override
public List<ChangeInfo> 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;
}
}
}