Merge branch 'stable-2.11' into stable-2.12

* stable-2.11:
  OutputStreamQuery: Only return current patch set when visible to user
  Fix NPE in SubmitRuleEvaluator

Change-Id: Id40335752bbe67cc74552260445b29223da4eebd

Documentation/cmd-query.txt

@@ -54,15 +54,17 @@ command line parser in the server).
 
 --current-patch-set::
 	Include information about the current patch set in the results.
+	Note that the information will only be included when the current
+	patch set is visible to the caller.
 
 --patch-sets::
-	Include information about all patch sets.  If combined with
-	the --current-patch-set flag then the current patch set
-	information will be output twice, once in each field.
+	Include information about all patch sets visible to the caller.
+        If combined with the --current-patch-set flag then the current patch
+	set information will be output twice, once in each field.
 
 --all-approvals::
-	Include information about all patch sets along with the
-	approval information for each patch set.  If combined with
+	Include information about all patch sets visible to the caller along
+	with the approval information for each patch set.  If combined with
 	the --current-patch-set flag then the current patch set
 	information will be output twice, once in each field.
 
@@ -76,7 +78,7 @@ command line parser in the server).
 --comments::
 	Include comments for all changes. If combined with the
 	--patch-sets flag then all inline/file comments are included for
-	each patch set.
+	each patch set that is visible to the caller.
 
 --commit-message::
 	Include the full commit message in the change description.

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

@@ -440,6 +440,11 @@ public abstract class AbstractDaemonTest {
     return push.to(ref);
   }
 
+  protected PushOneCommit.Result amendChangeAsDraft(String changeId)
+      throws Exception {
+    return amendChange(changeId, "refs/drafts/master");
+  }
+
   protected ChangeInfo info(String id)
       throws RestApiException {
     return gApi.changes().id(id).info();

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/QueryIT.java

@@ -16,11 +16,13 @@ package com.google.gerrit.acceptance.ssh;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assert_;
+import static com.google.gerrit.acceptance.GitUtil.initSsh;
 
 import com.google.common.collect.Lists;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.SshSession;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.client.Side;
@@ -300,13 +302,39 @@ public class QueryIT extends AbstractDaemonTest {
     assertThat(changes.get(0).submitRecords.size()).isEqualTo(1);
   }
 
+  @Test
+  public void testQueryWithNonVisibleCurrentPatchSet() throws Exception {
+    String changeId = createChange().getChangeId();
+    amendChangeAsDraft(changeId);
+    String query = "--current-patch-set --patch-sets " + changeId;
+    List<ChangeAttribute> changes = executeSuccessfulQuery(query);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets).isNotNull();
+    assertThat(changes.get(0).patchSets).hasSize(2);
+    assertThat(changes.get(0).currentPatchSet).isNotNull();
+
+    SshSession userSession = new SshSession(server, user);
+    initSsh(user);
+    userSession.open();
+    changes = executeSuccessfulQuery(query, userSession);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets).hasSize(1);
+    assertThat(changes.get(0).currentPatchSet).isNull();
+    userSession.close();
+  }
+
+  private List<ChangeAttribute> executeSuccessfulQuery(String params,
+      SshSession session) throws Exception {
+    String rawResponse =
+        session.exec("gerrit query --format=JSON " + params);
+    assert_().withFailureMessage(session.getError())
+        .that(session.hasError()).isFalse();
+    return getChanges(rawResponse);
+  }
+
   private List<ChangeAttribute> executeSuccessfulQuery(String params)
       throws Exception {
-    String rawResponse =
-        sshSession.exec("gerrit query --format=JSON " + params);
-    assert_().withFailureMessage(sshSession.getError())
-        .that(sshSession.hasError()).isFalse();
-    return getChanges(rawResponse);
+    return executeSuccessfulQuery(params, sshSession);
   }
 
   private static List<ChangeAttribute> getChanges(String rawResponse) {

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

@@ -612,6 +612,9 @@ public class SubmitRuleEvaluator {
   private void initPatchSet() throws OrmException {
     if (patchSet == null) {
       patchSet = cd.currentPatchSet();
+      if (patchSet == null) {
+        throw new OrmException("No patch set found");
+      }
     }
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java

@@ -18,6 +18,8 @@ import static com.google.gerrit.server.ApprovalsUtil.sortApprovals;
 
 import com.google.auto.value.AutoValue;
 import com.google.common.base.MoreObjects;
+import com.google.common.base.Predicate;
+import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.ListMultimap;
@@ -675,13 +677,29 @@ public class ChangeData {
     return patchSets;
   }
 
+  /**
+   * @return patches for the change visible to the current user.
+   * @throws OrmException an error occurred reading the database.
+   */
+  public Collection<PatchSet> visiblePatchSets() throws OrmException {
+    return FluentIterable.from(patchSets()).filter(new Predicate<PatchSet>() {
+      @Override
+      public boolean apply(PatchSet input) {
+        try {
+          return changeControl().isPatchVisible(input, db);
+        } catch (OrmException e) {
+          return false;
+        }
+      }}).toList();
+  }
+
   public void setPatchSets(Collection<PatchSet> patchSets) {
     this.currentPatchSet = null;
     this.patchSets = patchSets;
   }
 
   /**
-   * @return patch set with the given ID, or null if it does not exist.
+   * @return patch with the given ID, or null if it does not exist.
    * @throws OrmException an error occurred reading the database.
    */
   public PatchSet patchSet(PatchSet.Id psId) throws OrmException {

gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java

@@ -276,14 +276,14 @@ public class OutputStreamQuery {
     }
 
     if (includePatchSets) {
-      eventFactory.addPatchSets(db.get(), rw, c, d.patchSets(),
+      eventFactory.addPatchSets(db.get(), rw, c, d.visiblePatchSets(),
           includeApprovals ? d.approvals().asMap() : null,
           includeFiles, d.change(), labelTypes);
     }
 
     if (includeCurrentPatchSet) {
       PatchSet current = d.currentPatchSet();
-      if (current != null) {
+      if (current != null && cc.isPatchVisible(current, d.db())) {
         c.currentPatchSet =
             eventFactory.asPatchSetAttribute(db.get(), rw, current);
         eventFactory.addApprovals(c.currentPatchSet,
@@ -303,7 +303,7 @@ public class OutputStreamQuery {
     if (includeComments) {
       eventFactory.addComments(c, d.messages());
       if (includePatchSets) {
-        eventFactory.addPatchSets(db.get(), rw, c, d.patchSets(),
+        eventFactory.addPatchSets(db.get(), rw, c, d.visiblePatchSets(),
             includeApprovals ? d.approvals().asMap() : null,
             includeFiles, d.change(), labelTypes);
         for (PatchSetAttribute attribute : c.patchSets) {