Revert "submitted_together: Add a dummy change for not visible changes"

This reverts commit e73ebe2dc4.  It
broke anonymous access in projects with the CherryPick submit type.

Add a test to prevent the problem from happening again.

 Error in GET /changes/24150/revisions/f3d9f239bc26908994eebc8b4ba64e89c634f352/actions
 java.lang.IllegalStateException: user already specified: IdentifiedUser[account 5195]
	at com.google.gerrit.server.query.change.ChangeData.changeControl(ChangeData.java:691)
	at com.google.gerrit.server.git.ChangeSet.<init>(ChangeSet.java:66)
	at com.google.gerrit.server.git.MergeSuperSet.completeChangeSetWithoutTopic(MergeSuperSet.java:169)
	at com.google.gerrit.server.git.MergeSuperSet.completeChangeSet(MergeSuperSet.java:103)
	at com.google.gerrit.server.change.GetRevisionActions.getETag(GetRevisionActions.java:75)
	at com.google.gerrit.server.change.GetRevisionActions.getETag(GetRevisionActions.java:39)
	at com.google.gerrit.httpd.restapi.RestApiServlet.addResourceStateHeaders(RestApiServlet.java:496)
	at com.google.gerrit.httpd.restapi.RestApiServlet.configureCaching(RestApiServlet.java:476)
	at com.google.gerrit.httpd.restapi.RestApiServlet.service(RestApiServlet.java:347)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:717)

Change-Id: I143300e7b3a17046559d640bd589ba34aed662f0

Documentation/rest-api-changes.txt

@@ -1186,27 +1186,16 @@ message is contained in the response body.
 'GET /changes/link:#change-id[\{change-id\}]/submitted_together'
 --
 
-Returns a list of all visible changes which are submitted when
+Returns a list of all changes which are submitted when
 link:#submit-change[\{submit\}] is called for this change,
 including the current change itself.
 
-An empty list is returned if the list would contain
+An empty list is returned if this change will be submitted
-only the current change itself.
+by itself (no other changes).
-
-It is possible that some changes would be submitted along with this one,
-but those changes are not visible to the calling user, for example because
-they are on a branch that the user is not allowed to see. In that case,
-those changes are omitted from the results. To see whether any changes
-were omitted, use the ?o=DUMMY option.
-
-When the ?o=DUMMY option is used, a dummy change is added to the list,
-that is a new non-mergeable, unsubmittable change with change id 0.
-The subject is set to "Some changes are not visible", so this dummy change
-can be viewed in the UI as-is.
 
 .Request
 ----
-  GET /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/submitted_together?o=DUMMY HTTP/1.0
+  GET /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/submitted_together HTTP/1.0
   Content-Type: application/json; charset=UTF-8
 ----
 
@@ -1466,22 +1455,6 @@ The list consists of:
         }
       }
     }
-    {
-      "subject": "Some changes are not visible",
-      "status": "NEW",
-      "mergeable": false,
-      "submittable": false,
-      "_number": 0,
-      "current_revision": "0",
-      "revisions": {
-        "0": {
-          "_number": 0,
-          "commit": {
-            "subject": "Some changes are not visible"
-          }
-        }
-      }
-    }
   }
 ]
 ----

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -37,7 +37,6 @@ import com.google.gerrit.extensions.api.projects.BranchInput;
 import com.google.gerrit.extensions.api.projects.ProjectInput;
 import com.google.gerrit.extensions.client.InheritableBoolean;
 import com.google.gerrit.extensions.client.ListChangesOption;
-import com.google.gerrit.extensions.client.SubmittedTogetherOption;
 import com.google.gerrit.extensions.common.ActionInfo;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.common.EditInfo;
@@ -108,7 +107,6 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.EnumSet;
 import java.util.List;
 import java.util.Map;
 import java.util.regex.Pattern;
@@ -741,20 +739,13 @@ public abstract class AbstractDaemonTest {
 
   protected void assertSubmittedTogether(String chId, String... expected)
       throws Exception {
-    EnumSet<SubmittedTogetherOption> o = EnumSet.noneOf(
+    List<ChangeInfo> actual = gApi.changes().id(chId).submittedTogether();
-        SubmittedTogetherOption.class);
-    assertSubmittedTogether(chId, o, expected);
-  }
-
-  protected void assertSubmittedTogether(String chId,
-      EnumSet<SubmittedTogetherOption> o, String... expected) throws Exception {
-    List<ChangeInfo> actual = gApi.changes().id(chId).submittedTogether(o);
     assertThat(actual).hasSize(expected.length);
     assertThat(Iterables.transform(actual,
         new Function<ChangeInfo, String>() {
       @Override
       public String apply(ChangeInfo input) {
-        return input.changeId != null ? input.changeId : input.subject;
+        return input.changeId;
       }
     })).containsExactly((Object[])expected).inOrder();
   }

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ActionsIT.java

@@ -16,11 +16,16 @@ package com.google.gerrit.acceptance.rest.change;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.common.collect.ImmutableList;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.TestProjectInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.client.SubmitType;
 import com.google.gerrit.extensions.common.ActionInfo;
+import com.google.gerrit.server.change.GetRevisionActions;
 import com.google.gerrit.testutil.ConfigSuite;
+import com.google.inject.Inject;
 
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.TestRepository;
@@ -35,6 +40,9 @@ public class ActionsIT extends AbstractDaemonTest {
     return submitWholeTopicEnabledConfig();
   }
 
+  @Inject
+  private GetRevisionActions getRevisionActions;
+
   @Test
   public void revisionActionsOneChangePerTopicUnapproved() throws Exception {
     String changeId = createChangeWithTopic().getChangeId();
@@ -78,6 +86,85 @@ public class ActionsIT extends AbstractDaemonTest {
     }
   }
 
+  @Test
+  public void revisionActionsETag() throws Exception {
+    String parent = createChange().getChangeId();
+    String change = createChangeWithTopic().getChangeId();
+    approve(change);
+    String etag1 = getRevisionActions.getETag(parseCurrentRevisionResource(change));
+
+    approve(parent);
+    String etag2 = getRevisionActions.getETag(parseCurrentRevisionResource(change));
+
+    String changeWithSameTopic = createChangeWithTopic().getChangeId();
+    String etag3 = getRevisionActions.getETag(parseCurrentRevisionResource(change));
+
+    approve(changeWithSameTopic);
+    String etag4 = getRevisionActions.getETag(parseCurrentRevisionResource(change));
+
+    if (isSubmitWholeTopicEnabled()) {
+      assertThat(ImmutableList.of(etag1, etag2, etag3, etag4)).containsNoDuplicates();
+    } else {
+      assertThat(etag2).isNotEqualTo(etag1);
+      assertThat(etag3).isEqualTo(etag2);
+      assertThat(etag4).isEqualTo(etag2);
+    }
+  }
+
+  @Test
+  public void revisionActionsAnonymousETag() throws Exception {
+    String parent = createChange().getChangeId();
+    String change = createChangeWithTopic().getChangeId();
+    approve(change);
+
+    setApiUserAnonymous();
+    String etag1 = getRevisionActions.getETag(parseCurrentRevisionResource(change));
+
+    setApiUser(admin);
+    approve(parent);
+
+    setApiUserAnonymous();
+    String etag2 = getRevisionActions.getETag(parseCurrentRevisionResource(change));
+
+    setApiUser(admin);
+    String changeWithSameTopic = createChangeWithTopic().getChangeId();
+
+    setApiUserAnonymous();
+    String etag3 = getRevisionActions.getETag(parseCurrentRevisionResource(change));
+
+    setApiUser(admin);
+    approve(changeWithSameTopic);
+
+    setApiUserAnonymous();
+    String etag4 = getRevisionActions.getETag(parseCurrentRevisionResource(change));
+
+    if (isSubmitWholeTopicEnabled()) {
+      assertThat(ImmutableList.of(etag1, etag2, etag3, etag4)).containsNoDuplicates();
+    } else {
+      assertThat(etag2).isNotEqualTo(etag1);
+      assertThat(etag3).isEqualTo(etag2);
+      assertThat(etag4).isEqualTo(etag2);
+    }
+  }
+
+  @Test
+  @TestProjectInput(submitType = SubmitType.CHERRY_PICK)
+  public void revisionActionsAnonymousETagCherryPickStrategy() throws Exception {
+    String parent = createChange().getChangeId();
+    String change = createChange().getChangeId();
+    approve(change);
+
+    setApiUserAnonymous();
+    String etag1 = getRevisionActions.getETag(parseCurrentRevisionResource(change));
+
+    setApiUser(admin);
+    approve(parent);
+
+    setApiUserAnonymous();
+    String etag2 = getRevisionActions.getETag(parseCurrentRevisionResource(change));
+    assertThat(etag2).isEqualTo(etag1);
+  }
+
   @Test
   public void revisionActionsTwoChangesInTopic_conflicting() throws Exception {
     String changeId = createChangeWithTopic().getChangeId();

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/BUCK

@@ -13,6 +13,8 @@ acceptance_tests(
   srcs = OTHER_TESTS,
   deps = [
     ':submit_util',
+    '//gerrit-server:server',
+    '//lib/guice:guice',
     '//lib/joda:joda-time',
   ],
   labels = ['rest'],

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java

@@ -16,38 +16,22 @@ package com.google.gerrit.acceptance.server.change;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.gerrit.acceptance.GitUtil.pushHead;
-import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
-import static org.junit.Assert.fail;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.GitUtil;
 import com.google.gerrit.acceptance.TestProjectInput;
-import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.client.ChangeStatus;
 import com.google.gerrit.extensions.client.SubmitType;
-import com.google.gerrit.extensions.client.SubmittedTogetherOption;
-import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.server.git.ProjectConfig;
-import com.google.gerrit.server.project.Util;
-import com.google.gerrit.testutil.ConfigSuite;
 
 import org.eclipse.jgit.junit.TestRepository;
-import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.junit.Test;
 
-import java.util.EnumSet;
-
 public class SubmittedTogetherIT extends AbstractDaemonTest {
 
-  @ConfigSuite.Config
-  public static Config submitWholeTopicEnabled() {
-    return submitWholeTopicEnabledConfig();
-  }
-
   @Test
   public void returnsAncestors() throws Exception {
     // Create two commits and push.
@@ -238,115 +222,6 @@ public class SubmittedTogetherIT extends AbstractDaemonTest {
     assertSubmittedTogether(id2, id2, id1);
   }
 
-  @Test
-  public void testHiddenDraftChange() throws Exception {
-    setApiUser(admin);
-    RevCommit initialHead = getRemoteHead();
-    // Create two independent commits and push.
-    RevCommit c1_1 = commitBuilder()
-        .add("a.txt", "1")
-        .message("subject: 1")
-        .create();
-    String id1 = getChangeId(c1_1);
-    pushHead(testRepo, "refs/drafts/master/" + name("connectingTopic"), false);
-
-    testRepo.reset(initialHead);
-    setApiUser(user);
-    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);
-
-    String draftId = "Some changes are not visible";
-    EnumSet<SubmittedTogetherOption> o1 = EnumSet.noneOf(
-        SubmittedTogetherOption.class);
-    EnumSet<SubmittedTogetherOption> o2 = EnumSet.of(
-        SubmittedTogetherOption.DUMMY);
-    if (isSubmitWholeTopicEnabled()) {
-      setApiUser(admin);
-      assertSubmittedTogether(id1, o1, id2, id1);
-      assertSubmittedTogether(id2, o1, id2, id1);
-      assertSubmittedTogether(id1, o2, id2, id1);
-      assertSubmittedTogether(id2, o2, id2, id1);
-      setApiUser(user);
-      assertSubmittedTogether(id2, o1);
-      assertSubmittedTogether(id2, o2, id2, draftId);
-    } else {
-      setApiUser(admin);
-      assertSubmittedTogether(id1, o1);
-      assertSubmittedTogether(id2, o1);
-      assertSubmittedTogether(id1, o2);
-      assertSubmittedTogether(id2, o2);
-      setApiUser(user);
-      assertSubmittedTogether(id2, o1);
-      assertSubmittedTogether(id2, o2);
-    }
-  }
-
-  @Test
-  public void testHiddenByACLChange() throws Exception {
-    Project.NameKey p1 = createProject("a-new-project", null, false);
-    TestRepository<?> repo1 = cloneProject(p1);
-
-    RevCommit c1 = repo1.branch("HEAD").commit().insertChangeId()
-        .add("a.txt", "1")
-        .message("subject: 1")
-        .create();
-    String id1 = GitUtil.getChangeId(repo1, c1).get();
-    pushHead(repo1, "refs/for/master/" + name("connectingTopic"), false);
-
-    ProjectConfig cfg = projectCache.checkedGet(p1).getConfig();
-    Util.block(cfg, Permission.READ, REGISTERED_USERS, "refs/*");
-    saveProjectConfig(p1, cfg);
-
-    setApiUser(user);
-    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);
-
-    String invisibleId = "Some changes are not visible";
-    EnumSet<SubmittedTogetherOption> o1 = EnumSet.noneOf(
-        SubmittedTogetherOption.class);
-    EnumSet<SubmittedTogetherOption> o2 = EnumSet.of(
-        SubmittedTogetherOption.DUMMY);
-    if (isSubmitWholeTopicEnabled()) {
-      setApiUser(admin);
-      assertSubmittedTogetherBroken(id1, "Not found: " + id1);
-      assertSubmittedTogether(id2, o1);
-      assertSubmittedTogether(id2, o2, id2, invisibleId);
-      setApiUser(user);
-      assertSubmittedTogetherBroken(id1, "Not found: " + id1);
-      assertSubmittedTogether(id2, o1);
-      assertSubmittedTogether(id2, o2, id2, invisibleId);
-    } else {
-      setApiUser(admin);
-      assertSubmittedTogetherBroken(id1, "Not found: " + id1);
-      assertSubmittedTogether(id2, o1);
-      assertSubmittedTogether(id2, o2);
-      setApiUser(user);
-      assertSubmittedTogetherBroken(id1, "Not found: " + id1);
-      assertSubmittedTogether(id2, o1);
-      assertSubmittedTogether(id2, o2);
-    }
-  }
-
-  protected void assertSubmittedTogetherBroken(String chId,
-      String expectedMsg) throws Exception {
-    EnumSet<SubmittedTogetherOption> o = EnumSet.noneOf(
-        SubmittedTogetherOption.class);
-    try {
-      gApi.changes().id(chId).submittedTogether(o);
-      fail("Expected ResourceNotFoundException");
-    } catch (ResourceNotFoundException e) {
-      assertThat(e.getMessage()).isEqualTo(expectedMsg);
-    }
-  }
-
   private RevCommit getRemoteHead() throws Exception {
     try (Repository repo = repoManager.openRepository(project);
         RevWalk rw = new RevWalk(repo)) {

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

@@ -15,7 +15,6 @@
 package com.google.gerrit.extensions.api.changes;
 
 import com.google.gerrit.extensions.client.ListChangesOption;
-import com.google.gerrit.extensions.client.SubmittedTogetherOption;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.common.CommentInfo;
 import com.google.gerrit.extensions.common.EditInfo;
@@ -95,8 +94,7 @@ public interface ChangeApi {
    */
   ChangeApi revert(RevertInput in) throws RestApiException;
 
-  List<ChangeInfo> submittedTogether(EnumSet<SubmittedTogetherOption> o)
+  List<ChangeInfo> submittedTogether() throws RestApiException;
-      throws RestApiException;
 
   /**
    * Publishes a draft change.
@@ -351,8 +349,7 @@ public interface ChangeApi {
     }
 
     @Override
-    public List<ChangeInfo> submittedTogether(
+    public List<ChangeInfo> submittedTogether() throws RestApiException {
-        EnumSet<SubmittedTogetherOption> o) throws RestApiException {
       throw new NotImplementedException();
     }
   }

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/SubmittedTogetherOption.java

@@ -1,31 +0,0 @@
-// Copyright (C) 2012 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.extensions.client;
-
-/** Output options available for retrieval change details. */
-public enum SubmittedTogetherOption {
-  /** Dummy for changes not visible. */
-  DUMMY(0);
-
-  private final int value;
-
-  SubmittedTogetherOption(int v) {
-    this.value = v;
-  }
-
-  public int getValue() {
-    return value;
-  }
-}

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChanges.java

@@ -217,7 +217,6 @@ public class RelatedChanges extends TabPanel {
 
     // TODO(sbeller): show only on latest revision
     ChangeApi.change(info.legacyId().get()).view("submitted_together")
-        .addParameter("o", "DUMMY")
         .get(new TabChangeListCallback(Tab.SUBMITTED_TOGETHER,
             info.project(), revision));
 

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChangesTab.java

@@ -291,12 +291,10 @@ class RelatedChangesTab implements IsWidget {
           sb.setAttribute("onclick", OPEN);
         }
         sb.setAttribute("title", info.commit().subject());
-        if (showProjects && info.project() != null
+        if (showProjects) {
-            && !info.project().isEmpty()) {
           sb.append(info.project()).append(": ");
         }
-        if (showBranches && info.branch() != null
+        if (showBranches) {
-            && !info.branch().isEmpty()) {
           sb.append(info.branch()).append(": ");
         }
         sb.append(info.commit().subject());
@@ -333,10 +331,6 @@ class RelatedChangesTab implements IsWidget {
     }
 
     private String url() {
-      if (info.hasChangeNumber() && info._changeNumber() == 0) {
-        return null;
-      }
-
       if (info.hasChangeNumber() && info.hasRevisionNumber()) {
         return "#" + PageLinks.toChange(info.patchSetId());
       }

gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java

@@ -26,7 +26,6 @@ import com.google.gerrit.extensions.api.changes.RevertInput;
 import com.google.gerrit.extensions.api.changes.ReviewerApi;
 import com.google.gerrit.extensions.api.changes.RevisionApi;
 import com.google.gerrit.extensions.client.ListChangesOption;
-import com.google.gerrit.extensions.client.SubmittedTogetherOption;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.common.CommentInfo;
 import com.google.gerrit.extensions.common.EditInfo;
@@ -248,12 +247,8 @@ class ChangeApiImpl implements ChangeApi {
   }
 
   @Override
-  public List<ChangeInfo> submittedTogether(EnumSet<SubmittedTogetherOption> s)
+  public List<ChangeInfo> submittedTogether() throws RestApiException {
-      throws RestApiException {
     try {
-      for (SubmittedTogetherOption o : s) {
-        submittedTogether.addOption(o);
-      }
       return submittedTogether.apply(change);
     } catch (Exception e) {
       throw new RestApiException("Cannot query submittedTogether", e);

gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java

@@ -16,10 +16,7 @@ package com.google.gerrit.server.change;
 
 import com.google.gerrit.extensions.client.ChangeStatus;
 import com.google.gerrit.extensions.client.ListChangesOption;
-import com.google.gerrit.extensions.client.SubmittedTogetherOption;
 import com.google.gerrit.extensions.common.ChangeInfo;
-import com.google.gerrit.extensions.common.CommitInfo;
-import com.google.gerrit.extensions.common.RevisionInfo;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -35,8 +32,8 @@ 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;
 import org.slf4j.LoggerFactory;
 
@@ -44,10 +41,9 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.EnumSet;
-import java.util.LinkedHashMap;
 import java.util.List;
-import java.util.Map;
 
+@Singleton
 public class SubmittedTogether implements RestReadView<ChangeResource> {
   private static final Logger log = LoggerFactory.getLogger(
       SubmittedTogether.class);
@@ -58,14 +54,6 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
   private final MergeSuperSet mergeSuperSet;
   private final Provider<WalkSorter> sorter;
 
-  private final EnumSet<SubmittedTogetherOption> options =
-      EnumSet.noneOf(SubmittedTogetherOption.class);
-
-  @Option(name = "-o", usage = "Output options")
-  public void addOption(SubmittedTogetherOption o) {
-    options.add(o);
-  }
-
   @Inject
   SubmittedTogether(ChangeJson.Factory json,
       Provider<ReviewDb> dbProvider,
@@ -84,59 +72,38 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
       throws AuthException, BadRequestException,
       ResourceConflictException, Exception {
     try {
-      boolean addHiddenDummy = false;
       Change c = resource.getChange();
       List<ChangeData> cds;
       if (c.getStatus().isOpen()) {
-        ChangeSet cs = getForOpenChange(c, resource.getControl().getUser());
+        cds = getForOpenChange(c, resource.getControl().getUser());
-        cds = cs.changes().asList();
-        addHiddenDummy = !cs.isComplete();
       } else if (c.getStatus().asChangeStatus() == ChangeStatus.MERGED) {
         cds = getForMergedChange(c);
       } else {
         cds = getForAbandonedChange();
       }
-      addHiddenDummy &= options.contains(SubmittedTogetherOption.DUMMY);
+
-      if (cds.size() <= 1 && !addHiddenDummy) {
+      if (cds.size() <= 1) {
         cds = Collections.emptyList();
       } else {
         // Skip sorting for singleton lists, to avoid WalkSorter opening the
         // repo just to fill out the commit field in PatchSetData.
         cds = sort(cds);
       }
-      List<ChangeInfo> ret = json.create(EnumSet.of(
+
+      return json.create(EnumSet.of(
           ListChangesOption.CURRENT_REVISION,
           ListChangesOption.CURRENT_COMMIT))
         .formatChangeDatas(cds);
-      if (addHiddenDummy) {
-        ChangeInfo i = new ChangeInfo();
-        i.subject = "Some changes are not visible";
-        i.project = null;
-        i.branch = null;
-        i.submittable = false;
-        i.mergeable = false;
-        i.changeId = null;
-        i._number = 0;
-        i.currentRevision = "0";
-        i.status = ChangeStatus.NEW;
-        RevisionInfo ri = new RevisionInfo();
-        ri.commit = new CommitInfo();
-        ri.commit.subject = "Some changes are not visible";
-        Map<String, RevisionInfo> revs = new LinkedHashMap<>();
-        i.revisions = revs;
-        i.revisions.put("0", ri);
-        ret.add(i);
-      }
-      return ret;
     } catch (OrmException | IOException e) {
       log.error("Error on getting a ChangeSet", e);
       throw e;
     }
   }
 
-  private ChangeSet getForOpenChange(Change c, CurrentUser user)
+  private List<ChangeData> getForOpenChange(Change c, CurrentUser user)
       throws OrmException, IOException {
-    return mergeSuperSet.completeChangeSet(dbProvider.get(), c, user);
+    ChangeSet cs = mergeSuperSet.completeChangeSet(dbProvider.get(), c, user);
+    return cs.changes().asList();
   }
 
   private List<ChangeData> getForMergedChange(Change c) throws OrmException {

gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeSet.java

@@ -23,13 +23,10 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.SetMultimap;
-import com.google.gerrit.common.Nullable;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gwtorm.server.OrmException;
 
@@ -44,42 +41,20 @@ import java.util.Set;
  * This class is not thread safe.
  */
 public class ChangeSet {
-  private final boolean furtherHiddenChanges;
   private final ImmutableMap<Change.Id, ChangeData> changeData;
 
-  /**
+  public ChangeSet(Iterable<ChangeData> changes) {
-   * Construct the set of changes grouped together. The set is restricted to
-   * the changes that the given user can to see. Pass @code{null} for the
-   * complete set without visibility constraints.
-   *
-   * @param changes the initial set of changes to be completed
-   * @param db the review database
-   * @param user only pickup changes that this user can see.
-   * @throws OrmException
-   */
-  public ChangeSet(Iterable<ChangeData> changes, ReviewDb db,
-      @Nullable CurrentUser user) throws OrmException {
     Map<Change.Id, ChangeData> cds = new LinkedHashMap<>();
-    boolean hidden = false;
     for (ChangeData cd : changes) {
-      if (user != null) {
-        if (!cd.changeControl(user).isVisible(db, cd)) {
-          hidden = true;
-          continue;
-        }
-      }
-
       if (!cds.containsKey(cd.getId())) {
         cds.put(cd.getId(), cd);
       }
     }
-    furtherHiddenChanges = hidden;
     changeData = ImmutableMap.copyOf(cds);
   }
 
-  public ChangeSet(ChangeData change, ReviewDb db, @Nullable CurrentUser user)
+  public ChangeSet(ChangeData change) {
-      throws OrmException {
+    this(ImmutableList.of(change));
-    this(ImmutableList.of(change), db, user);
   }
 
   public ImmutableSet<Change.Id> ids() {
@@ -140,8 +115,4 @@ public class ChangeSet {
   public String toString() {
     return getClass().getSimpleName() + ids();
   }
-
-  public boolean isComplete() {
-    return !furtherHiddenChanges;
-  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/git/MergeSuperSet.java

@@ -98,9 +98,9 @@ public class MergeSuperSet {
         changeDataFactory.create(db, change.getProject(), change.getId());
     cd.changeControl(user);
     if (Submit.wholeTopicEnabled(cfg)) {
-      return completeChangeSetIncludingTopics(db, new ChangeSet(cd, db, null), user);
+      return completeChangeSetIncludingTopics(db, new ChangeSet(cd), user);
     }
-    return completeChangeSetWithoutTopic(db, new ChangeSet(cd, db, null), user);
+    return completeChangeSetWithoutTopic(db, new ChangeSet(cd), user);
   }
 
   private ChangeSet completeChangeSetWithoutTopic(ReviewDb db, ChangeSet changes,
@@ -114,6 +114,7 @@ public class MergeSuperSet {
            RevWalk rw = CodeReviewCommit.newRevWalk(repo)) {
         for (Change.Id cId : pc.get(project)) {
           ChangeData cd = changeDataFactory.create(db, project, cId);
+          cd.changeControl(user);
 
           SubmitTypeRecord str = cd.submitTypeRecord();
           if (!str.isOk()) {
@@ -166,7 +167,7 @@ public class MergeSuperSet {
       }
     }
 
-    return new ChangeSet(ret, db, user);
+    return new ChangeSet(ret);
   }
 
   private ChangeSet completeChangeSetIncludingTopics(
@@ -175,22 +176,23 @@ public class MergeSuperSet {
       OrmException {
     Set<String> topicsTraversed = new HashSet<>();
     boolean done = false;
+    ChangeSet newCs = completeChangeSetWithoutTopic(db, changes, user);
     while (!done) {
+      List<ChangeData> chgs = new ArrayList<>();
       done = true;
-      List<ChangeData> newChgs = new ArrayList<>();
+      for (ChangeData cd : newCs.changes()) {
-      for (ChangeData cd : changes.changes()) {
+        chgs.add(cd);
-        newChgs.add(cd);
         String topic = cd.change().getTopic();
         if (!Strings.isNullOrEmpty(topic) && !topicsTraversed.contains(topic)) {
-          newChgs.addAll(query().byTopicOpen(topic));
+          chgs.addAll(query().byTopicOpen(topic));
           done = false;
           topicsTraversed.add(topic);
         }
       }
-      changes = completeChangeSetWithoutTopic(db,
+      changes = new ChangeSet(chgs);
-          new ChangeSet(newChgs, db, null), null);
+      newCs = completeChangeSetWithoutTopic(db, changes, user);
     }
-    return completeChangeSetWithoutTopic(db, changes, user);
+    return newCs;
   }
 
   private InternalChangeQuery query() {
@@ -200,8 +202,7 @@ public class MergeSuperSet {
     // fields should clear them explicitly using reloadChanges().
     Set<String> fields = ImmutableSet.of(
         ChangeField.CHANGE.getName(),
-        ChangeField.PATCH_SET.getName(),
+        ChangeField.PATCH_SET.getName());
-        ChangeField.REVIEWER.getName());
     return queryProvider.get().setRequestedFields(fields);
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java

@@ -375,6 +375,22 @@ public class SubmitRuleEvaluator {
           + control.getChange().currentPatchSetId());
     }
 
+    try {
+      if (control.getChange().getStatus() == Change.Status.DRAFT
+          && !control.isDraftVisible(cd.db(), cd)) {
+        return SubmitTypeRecord.error(
+            "Patch set " + patchSet.getId() + " not found");
+      }
+      if (patchSet.isDraft() && !control.isDraftVisible(cd.db(), cd)) {
+        return SubmitTypeRecord.error(
+            "Patch set " + patchSet.getId() + " not found");
+      }
+    } catch (OrmException err) {
+      String msg = "Cannot read patch set " + patchSet.getId();
+      log.error(msg, err);
+      return SubmitTypeRecord.error(msg);
+    }
+
     List<Term> results;
     try {
       results = evaluateImpl("locate_submit_type", "get_submit_type",