Merge "submitted_together: accept standard formatting options"

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

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.

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java

@@ -97,6 +97,9 @@ public interface ChangeApi {
   List<ChangeInfo> submittedTogether() throws RestApiException;
   SubmittedTogetherInfo submittedTogether(
       EnumSet<SubmittedTogetherOption> options) throws RestApiException;
+  SubmittedTogetherInfo submittedTogether(
+      EnumSet<ListChangesOption> listOptions,
+      EnumSet<SubmittedTogetherOption> submitOptions) throws RestApiException;
 
   /**
    * Publishes a draft change.
@@ -360,5 +363,12 @@ public interface ChangeApi {
         EnumSet<SubmittedTogetherOption> options) throws RestApiException {
       throw new NotImplementedException();
     }
+
+    @Override
+    public SubmittedTogetherInfo submittedTogether(
+        EnumSet<ListChangesOption> a,
+        EnumSet<SubmittedTogetherOption> b) throws RestApiException {
+      throw new NotImplementedException();
+    }
   }
 }

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);
+  NON_VISIBLE_CHANGES;
-
-  private final int value;
-
-  SubmittedTogetherOption(int v) {
-    value = v;
-  }
-
-  public int getValue() {
-    return value;
-  }
 }

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> 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> submittedTogether,
       PublishDraftPatchSet.CurrentRevision publishDraftChange,
       DeleteDraftChange deleteDraftChange,
       GetTopic getTopic,
@@ -248,21 +249,29 @@ class ChangeApiImpl implements ChangeApi {
     }
   }
 
-  @SuppressWarnings("unchecked")
   @Override
   public List<ChangeInfo> submittedTogether() throws RestApiException {
-    try {
+    SubmittedTogetherInfo info = submittedTogether(
-      return (List<ChangeInfo>) submittedTogether.apply(change);
+        EnumSet.noneOf(ListChangesOption.class),
-    } catch (IOException | OrmException e) {
+        EnumSet.noneOf(SubmittedTogetherOption.class));
-      throw new RestApiException("Cannot query submittedTogether", e);
+    return info.changes;
-    }
   }
 
   @Override
   public SubmittedTogetherInfo submittedTogether(
       EnumSet<SubmittedTogetherOption> options) throws RestApiException {
+    return submittedTogether(EnumSet.noneOf(ListChangesOption.class), options);
+  }
+
+  @Override
+  public SubmittedTogetherInfo submittedTogether(
+      EnumSet<ListChangesOption> listOptions,
+      EnumSet<SubmittedTogetherOption> 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);
     }

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<ChangeResource> {
   private static final Logger log = LoggerFactory.getLogger(
       SubmittedTogether.class);
 
   private final EnumSet<SubmittedTogetherOption> options =
       EnumSet.noneOf(SubmittedTogetherOption.class);
+
+  private final EnumSet<ListChangesOption> jsonOpt = EnumSet.of(
+      ListChangesOption.CURRENT_REVISION,
+      ListChangesOption.CURRENT_COMMIT);
+
   private final ChangeJson.Factory json;
   private final Provider<ReviewDb> dbProvider;
   private final Provider<InternalChangeQuery> queryProvider;
@@ -58,8 +63,22 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
   private final Provider<WalkSorter> sorter;
 
   @Option(name = "-o", usage = "Output options")
-  void addOption(SubmittedTogetherOption o) {
+  void addOption(String option) {
-    options.add(o);
+    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<ChangeResource> {
     this.sorter = sorter;
   }
 
+  public SubmittedTogether addListChangesOption(EnumSet<ListChangesOption> o) {
+    jsonOpt.addAll(o);
+    return this;
+  }
+
+  public SubmittedTogether addSubmittedTogetherOption(
+      EnumSet<SubmittedTogetherOption> 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,
+  public SubmittedTogetherInfo applyInfo(ChangeResource resource)
-      EnumSet<SubmittedTogetherOption> options)
       throws AuthException, IOException, OrmException {
     Change c = resource.getChange();
     try {
@@ -109,7 +138,7 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
       }
 
       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<ChangeResource> {
       }
 
       SubmittedTogetherInfo info = new SubmittedTogetherInfo();
-      info.changes = json.create(EnumSet.of(
+      info.changes = json.create(jsonOpt)
-          ListChangesOption.CURRENT_REVISION,
-          ListChangesOption.CURRENT_COMMIT))
         .includeSubmittable(true)
         .formatChangeDatas(cds);
       info.nonVisibleChanges = hidden;