From 8080c3d45e01752eeb4881716ab3f534aa4d8d5b Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Mon, 19 Sep 2016 19:15:04 -0700 Subject: [PATCH] submitted_together: accept standard formatting options Some applications may want to obtain additional formatting data like WEB_LINKS from /submitted_together. Accept the standard option list in addition to the custom SubmittedTogetherOption. Change-Id: I3be2fe5a372df0ed4852eb5739322990bd11f13e --- Documentation/rest-api-changes.txt | 4 ++ .../server/change/SubmittedTogetherIT.java | 60 +++++++++++++++++++ .../extensions/api/changes/ChangeApi.java | 10 ++++ .../api/changes/SubmittedTogetherOption.java | 12 +--- .../server/api/changes/ChangeApiImpl.java | 27 ++++++--- .../server/change/SubmittedTogether.java | 49 +++++++++++---- 6 files changed, 131 insertions(+), 31 deletions(-) diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index e28332c017..cce00ef71e 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -1275,6 +1275,10 @@ link:#labels[`LABELS`], link:#detailed-labels[`DETAILED_LABELS`], link:#current-revision[`CURRENT_REVISION`], and link:#current-commit[`CURRENT_COMMIT`] options set. +Standard link:#query-options[formatting options] can be specified +with the `o` parameter, as well as the `submitted_together` specific +option `NON_VISIBLE_CHANGES`. + .Response ---- HTTP/1.1 200 OK 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 index 06170d005e..b843721237 100644 --- 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 @@ -23,8 +23,11 @@ import com.google.gerrit.acceptance.GitUtil; import com.google.gerrit.acceptance.TestProjectInput; import com.google.gerrit.extensions.api.changes.SubmittedTogetherInfo; import com.google.gerrit.extensions.client.ChangeStatus; +import com.google.gerrit.extensions.client.ListChangesOption; import com.google.gerrit.extensions.client.SubmitType; import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.common.FileInfo; +import com.google.gerrit.extensions.common.RevisionInfo; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.testutil.ConfigSuite; @@ -43,6 +46,63 @@ public class SubmittedTogetherIT extends AbstractDaemonTest { return submitWholeTopicEnabledConfig(); } + @Test + public void doesNotIncludeCurrentFiles() throws Exception { + RevCommit c1_1 = commitBuilder() + .add("a.txt", "1") + .message("subject: 1") + .create(); + RevCommit c2_1 = commitBuilder() + .add("b.txt", "2") + .message("subject: 2") + .create(); + String id2 = getChangeId(c2_1); + pushHead(testRepo, "refs/for/master", false); + + SubmittedTogetherInfo info = + gApi.changes() + .id(id2) + .submittedTogether(EnumSet.of(NON_VISIBLE_CHANGES)); + assertThat(info.changes).hasSize(2); + assertThat(info.changes.get(0).currentRevision).isEqualTo(c2_1.name()); + assertThat(info.changes.get(1).currentRevision).isEqualTo(c1_1.name()); + + assertThat(info.changes.get(0).currentRevision).isEqualTo(c2_1.name()); + RevisionInfo rev = info.changes.get(0).revisions.get(c2_1.name()); + assertThat(rev.files).isNull(); + } + + @Test + public void returnsCurrentFilesIfOptionRequested() throws Exception { + RevCommit c1_1 = commitBuilder() + .add("a.txt", "1") + .message("subject: 1") + .create(); + RevCommit c2_1 = commitBuilder() + .add("b.txt", "2") + .message("subject: 2") + .create(); + String id2 = getChangeId(c2_1); + pushHead(testRepo, "refs/for/master", false); + + SubmittedTogetherInfo info = + gApi.changes() + .id(id2) + .submittedTogether( + EnumSet.of(ListChangesOption.CURRENT_FILES), + EnumSet.of(NON_VISIBLE_CHANGES)); + assertThat(info.changes).hasSize(2); + assertThat(info.changes.get(0).currentRevision).isEqualTo(c2_1.name()); + assertThat(info.changes.get(1).currentRevision).isEqualTo(c1_1.name()); + + assertThat(info.changes.get(0).currentRevision).isEqualTo(c2_1.name()); + RevisionInfo rev = info.changes.get(0).revisions.get(c2_1.name()); + assertThat(rev).isNotNull(); + FileInfo file = rev.files.get("b.txt"); + assertThat(file).isNotNull(); + assertThat(file.status).isEqualTo('A'); + } + @Test public void returnsAncestors() throws Exception { // Create two commits and push. 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 f656c2d33b..f8605524d5 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 @@ -97,6 +97,9 @@ public interface ChangeApi { List submittedTogether() throws RestApiException; SubmittedTogetherInfo submittedTogether( EnumSet options) throws RestApiException; + SubmittedTogetherInfo submittedTogether( + EnumSet listOptions, + EnumSet submitOptions) throws RestApiException; /** * Publishes a draft change. @@ -360,5 +363,12 @@ public interface ChangeApi { EnumSet options) throws RestApiException { throw new NotImplementedException(); } + + @Override + public SubmittedTogetherInfo submittedTogether( + EnumSet a, + EnumSet b) throws RestApiException { + throw new NotImplementedException(); + } } } diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/SubmittedTogetherOption.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/SubmittedTogetherOption.java index 8649e91f94..e2cab4dba0 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/SubmittedTogetherOption.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/SubmittedTogetherOption.java @@ -16,15 +16,5 @@ package com.google.gerrit.extensions.api.changes; /** Output options available for submitted_together requests. */ public enum SubmittedTogetherOption { - NON_VISIBLE_CHANGES(0); - - private final int value; - - SubmittedTogetherOption(int v) { - value = v; - } - - public int getValue() { - return value; - } + NON_VISIBLE_CHANGES; } 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 9bfb342497..9c123481ed 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 @@ -61,6 +61,7 @@ import com.google.gerrit.server.git.UpdateException; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; +import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; import java.io.IOException; @@ -84,7 +85,7 @@ class ChangeApiImpl implements ChangeApi { private final Abandon abandon; private final Revert revert; private final Restore restore; - private final SubmittedTogether submittedTogether; + private final Provider submittedTogether; private final PublishDraftPatchSet.CurrentRevision publishDraftChange; private final DeleteDraftChange deleteDraftChange; @@ -111,7 +112,7 @@ class ChangeApiImpl implements ChangeApi { Abandon abandon, Revert revert, Restore restore, - SubmittedTogether submittedTogether, + Provider submittedTogether, PublishDraftPatchSet.CurrentRevision publishDraftChange, DeleteDraftChange deleteDraftChange, GetTopic getTopic, @@ -248,21 +249,29 @@ class ChangeApiImpl implements ChangeApi { } } - @SuppressWarnings("unchecked") @Override public List submittedTogether() throws RestApiException { - try { - return (List) submittedTogether.apply(change); - } catch (IOException | OrmException e) { - throw new RestApiException("Cannot query submittedTogether", e); - } + SubmittedTogetherInfo info = submittedTogether( + EnumSet.noneOf(ListChangesOption.class), + EnumSet.noneOf(SubmittedTogetherOption.class)); + return info.changes; } @Override public SubmittedTogetherInfo submittedTogether( EnumSet options) throws RestApiException { + return submittedTogether(EnumSet.noneOf(ListChangesOption.class), options); + } + + @Override + public SubmittedTogetherInfo submittedTogether( + EnumSet listOptions, + EnumSet submitOptions) throws RestApiException { try { - return submittedTogether.apply(change, options); + return submittedTogether.get() + .addListChangesOption(listOptions) + .addSubmittedTogetherOption(submitOptions) + .applyInfo(change); } catch (IOException | OrmException e) { throw new RestApiException("Cannot query submittedTogether", e); } 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 index 351ebf974d..908929a68d 100644 --- 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 @@ -14,6 +14,8 @@ package com.google.gerrit.server.change; +import static com.google.gerrit.extensions.api.changes.SubmittedTogetherOption.NON_VISIBLE_CHANGES; + import com.google.gerrit.extensions.api.changes.SubmittedTogetherInfo; import com.google.gerrit.extensions.api.changes.SubmittedTogetherOption; import com.google.gerrit.extensions.client.ChangeStatus; @@ -32,7 +34,6 @@ import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; -import com.google.inject.Singleton; import org.kohsuke.args4j.Option; import org.slf4j.Logger; @@ -44,13 +45,17 @@ import java.util.Collections; 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 EnumSet options = EnumSet.noneOf(SubmittedTogetherOption.class); + + private final EnumSet jsonOpt = EnumSet.of( + ListChangesOption.CURRENT_REVISION, + ListChangesOption.CURRENT_COMMIT); + private final ChangeJson.Factory json; private final Provider dbProvider; private final Provider queryProvider; @@ -58,8 +63,22 @@ public class SubmittedTogether implements RestReadView { private final Provider sorter; @Option(name = "-o", usage = "Output options") - void addOption(SubmittedTogetherOption o) { - options.add(o); + void addOption(String option) { + for (ListChangesOption o : ListChangesOption.values()) { + if (o.name().equalsIgnoreCase(option)) { + jsonOpt.add(o); + return; + } + } + + for (SubmittedTogetherOption o : SubmittedTogetherOption.values()) { + if (o.name().equalsIgnoreCase(option)) { + options.add(o); + return; + } + } + + throw new IllegalArgumentException("option not recognized: " + option); } @Inject @@ -75,19 +94,29 @@ public class SubmittedTogether implements RestReadView { this.sorter = sorter; } + public SubmittedTogether addListChangesOption(EnumSet o) { + jsonOpt.addAll(o); + return this; + } + + public SubmittedTogether addSubmittedTogetherOption( + EnumSet o) { + options.addAll(o); + return this; + } + @Override public Object apply(ChangeResource resource) throws AuthException, BadRequestException, ResourceConflictException, IOException, OrmException { - SubmittedTogetherInfo info = apply(resource, options); + SubmittedTogetherInfo info = applyInfo(resource); if (options.isEmpty()) { return info.changes; } return info; } - public SubmittedTogetherInfo apply(ChangeResource resource, - EnumSet options) + public SubmittedTogetherInfo applyInfo(ChangeResource resource) throws AuthException, IOException, OrmException { Change c = resource.getChange(); try { @@ -109,7 +138,7 @@ public class SubmittedTogether implements RestReadView { } if (hidden != 0 - && !options.contains(SubmittedTogetherOption.NON_VISIBLE_CHANGES)) { + && !options.contains(NON_VISIBLE_CHANGES)) { throw new AuthException( "change would be submitted with a change that you cannot see"); } @@ -123,9 +152,7 @@ public class SubmittedTogether implements RestReadView { } SubmittedTogetherInfo info = new SubmittedTogetherInfo(); - info.changes = json.create(EnumSet.of( - ListChangesOption.CURRENT_REVISION, - ListChangesOption.CURRENT_COMMIT)) + info.changes = json.create(jsonOpt) .includeSubmittable(true) .formatChangeDatas(cds); info.nonVisibleChanges = hidden;