diff --git a/Documentation/cmd-query.txt b/Documentation/cmd-query.txt index 0ff59d4d3c..090781b1bc 100644 --- a/Documentation/cmd-query.txt +++ b/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. diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index d890fa4488..056b4ed22f 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -236,6 +236,11 @@ public abstract class AbstractDaemonTest { return push.to(git, ref); } + protected PushOneCommit.Result amendChangeAsDraft(String changeId) + throws GitAPIException, IOException { + return amendChange(changeId, "refs/drafts/master"); + } + protected ChangeInfo getChange(String changeId, ListChangesOption... options) throws IOException { return getChange(adminSession, changeId, options); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/QueryIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/QueryIT.java index 066d3628d7..0e381c192e 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/QueryIT.java +++ b/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 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 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 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 getChanges(String rawResponse) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java index e4e94a12f8..c565ab2327 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java @@ -17,6 +17,8 @@ package com.google.gerrit.server.query.change; import static com.google.gerrit.server.ApprovalsUtil.sortApprovals; 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.ListMultimap; import com.google.common.collect.Lists; @@ -544,6 +546,22 @@ public class ChangeData { return patches; } + /** + * @return patches for the change visible to the current user. + * @throws OrmException an error occurred reading the database. + */ + public Collection visiblePatches() throws OrmException { + return FluentIterable.from(patches()).filter(new Predicate() { + @Override + public boolean apply(PatchSet input) { + try { + return changeControl().isPatchVisible(input, db); + } catch (OrmException e) { + return false; + } + }}).toList(); + } + /** * @return patch with the given ID, or null if it does not exist. * @throws OrmException an error occurred reading the database. diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java index d594f8acdf..301273564c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java @@ -209,11 +209,11 @@ public class OutputStreamQuery { if (includePatchSets) { if (includeFiles) { - eventFactory.addPatchSets(c, d.patches(), + eventFactory.addPatchSets(c, d.visiblePatches(), includeApprovals ? d.approvals().asMap() : null, includeFiles, d.change(), labelTypes); } else { - eventFactory.addPatchSets(c, d.patches(), + eventFactory.addPatchSets(c, d.visiblePatches(), includeApprovals ? d.approvals().asMap() : null, labelTypes); } @@ -221,7 +221,7 @@ public class OutputStreamQuery { if (includeCurrentPatchSet) { PatchSet current = d.currentPatchSet(); - if (current != null) { + if (current != null && cc.isPatchVisible(current, d.db())) { c.currentPatchSet = eventFactory.asPatchSetAttribute(current); eventFactory.addApprovals(c.currentPatchSet, d.currentApprovals(), labelTypes); @@ -240,7 +240,7 @@ public class OutputStreamQuery { if (includeComments) { eventFactory.addComments(c, d.messages()); if (includePatchSets) { - eventFactory.addPatchSets(c, d.patches(), + eventFactory.addPatchSets(c, d.visiblePatches(), includeApprovals ? d.approvals().asMap() : null, includeFiles, d.change(), labelTypes); for (PatchSetAttribute attribute : c.patchSets) {