Add account predicate that checks if user can see a certain change

This predicate is useful if account queries are used to suggest users
in the context of a change, because then users that can't see the
change should be filtered out. E.g. this predicate should be used when
suggesting assignees for a change.

Change-Id: I2c43d04c65718e1e6419cf94c4bbfb21aaf7d9f8
Signed-off-by: Edwin Kempin <ekempin@google.com>

Documentation/user-search-accounts.txt

@@ -23,6 +23,11 @@ are added to the same query string, they further restrict the
 returned results. Search can also be performed by typing only a
 text with no operator, which will match against a variety of fields.
 
+[[cansee]]
+cansee:'CHANGE'::
++
+Matches accounts that can see the change 'CHANGE'.
+
 [[email]]
 email:'EMAIL'::
 +

gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticQueryBuilder.java

@@ -21,6 +21,7 @@ import com.google.gerrit.index.query.IndexPredicate;
 import com.google.gerrit.index.query.IntegerRangePredicate;
 import com.google.gerrit.index.query.NotPredicate;
 import com.google.gerrit.index.query.OrPredicate;
+import com.google.gerrit.index.query.PostFilterPredicate;
 import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.index.query.QueryParseException;
 import com.google.gerrit.index.query.RegexPredicate;
@@ -43,6 +44,8 @@ public class ElasticQueryBuilder {
       return not(p);
     } else if (p instanceof IndexPredicate) {
       return fieldQuery((IndexPredicate<T>) p);
+    } else if (p instanceof PostFilterPredicate) {
+      return QueryBuilders.matchAllQuery();
     } else {
       throw new QueryParseException("cannot create query for index: " + p);
     }

gerrit-index/src/main/java/com/google/gerrit/index/query/PostFilterPredicate.java

@@ -0,0 +1,21 @@
+// Copyright (C) 2016 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.index.query;
+
+/**
+ * Matches all documents in the index, with additional filtering done in the subclass's {@code
+ * match} method.
+ */
+public abstract class PostFilterPredicate<T> extends Predicate<T> implements Matchable<T> {}

gerrit-lucene/src/main/java/com/google/gerrit/lucene/QueryBuilder.java

@@ -28,6 +28,7 @@ import com.google.gerrit.index.query.IndexPredicate;
 import com.google.gerrit.index.query.IntegerRangePredicate;
 import com.google.gerrit.index.query.NotPredicate;
 import com.google.gerrit.index.query.OrPredicate;
+import com.google.gerrit.index.query.PostFilterPredicate;
 import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.index.query.QueryParseException;
 import com.google.gerrit.index.query.RegexPredicate;
@@ -76,6 +77,8 @@ public class QueryBuilder<V> {
       return not(p);
     } else if (p instanceof IndexPredicate) {
       return fieldQuery((IndexPredicate<V>) p);
+    } else if (p instanceof PostFilterPredicate) {
+      return new MatchAllDocsQuery();
     } else {
       throw new QueryParseException("cannot create query for index: " + p);
     }

gerrit-server/src/main/java/com/google/gerrit/server/ChangeFinder.java

@@ -76,6 +76,14 @@ public class ChangeFinder {
     this.changeNotesFactory = changeNotesFactory;
   }
 
+  public ChangeNotes findOne(String id) throws OrmException {
+    List<ChangeNotes> ctls = find(id);
+    if (ctls.size() != 1) {
+      return null;
+    }
+    return ctls.get(0);
+  }
+
   /**
    * Find changes matching the given identifier.
    *

gerrit-server/src/main/java/com/google/gerrit/server/index/account/IndexedAccountQuery.java

@@ -14,21 +14,41 @@
 
 package com.google.gerrit.server.index.account;
 
+import static com.google.common.base.Preconditions.checkState;
+
 import com.google.gerrit.index.Index;
 import com.google.gerrit.index.IndexedQuery;
 import com.google.gerrit.index.QueryOptions;
 import com.google.gerrit.index.query.DataSource;
+import com.google.gerrit.index.query.Matchable;
 import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.index.query.QueryParseException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.account.AccountState;
+import com.google.gwtorm.server.OrmException;
 
 public class IndexedAccountQuery extends IndexedQuery<Account.Id, AccountState>
-    implements DataSource<AccountState> {
+    implements DataSource<AccountState>, Matchable<AccountState> {
 
   public IndexedAccountQuery(
       Index<Account.Id, AccountState> index, Predicate<AccountState> pred, QueryOptions opts)
       throws QueryParseException {
     super(index, pred, opts.convertForBackend());
   }
+
+  @Override
+  public boolean match(AccountState accountState) throws OrmException {
+    Predicate<AccountState> pred = getChild(0);
+    checkState(
+        pred.isMatchable(),
+        "match invoked, but child predicate %s doesn't implement %s",
+        pred,
+        Matchable.class.getName());
+    return pred.asMatchable().match(accountState);
+  }
+
+  @Override
+  public int getCost() {
+    return 1;
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/query/account/AccountPredicates.java

@@ -18,12 +18,15 @@ import com.google.common.collect.Lists;
 import com.google.common.primitives.Ints;
 import com.google.gerrit.index.FieldDef;
 import com.google.gerrit.index.query.IndexPredicate;
+import com.google.gerrit.index.query.Matchable;
 import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.index.query.QueryBuilder;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.account.AccountState;
 import com.google.gerrit.server.index.account.AccountField;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gwtorm.server.OrmException;
 import java.util.List;
 
 public class AccountPredicates {
@@ -101,7 +104,14 @@ public class AccountPredicates {
     return new AccountPredicate(AccountField.WATCHED_PROJECT, project.get());
   }
 
-  static class AccountPredicate extends IndexPredicate<AccountState> {
+  public static Predicate<AccountState> cansee(
+      AccountQueryBuilder.Arguments args, ChangeNotes changeNotes) {
+    return new CanSeeChangePredicate(
+        args.db, args.permissionBackend, args.userFactory, changeNotes);
+  }
+
+  static class AccountPredicate extends IndexPredicate<AccountState>
+      implements Matchable<AccountState> {
     AccountPredicate(FieldDef<AccountState, ?> def, String value) {
       super(def, value);
     }
@@ -109,6 +119,16 @@ public class AccountPredicates {
     AccountPredicate(FieldDef<AccountState, ?> def, String name, String value) {
       super(def, name, value);
     }
+
+    @Override
+    public boolean match(AccountState object) throws OrmException {
+      return true;
+    }
+
+    @Override
+    public int getCost() {
+      return 1;
+    }
   }
 
   private AccountPredicates() {}

gerrit-server/src/main/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java

@@ -23,9 +23,16 @@ import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.index.query.QueryBuilder;
 import com.google.gerrit.index.query.QueryParseException;
 import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.ChangeFinder;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.ProvisionException;
@@ -45,11 +52,25 @@ public class AccountQueryBuilder extends QueryBuilder<AccountState> {
       new QueryBuilder.Definition<>(AccountQueryBuilder.class);
 
   public static class Arguments {
+    final Provider<ReviewDb> db;
+    final ChangeFinder changeFinder;
+    final IdentifiedUser.GenericFactory userFactory;
+    final PermissionBackend permissionBackend;
+
     private final Provider<CurrentUser> self;
 
     @Inject
-    public Arguments(Provider<CurrentUser> self) {
+    public Arguments(
+        Provider<CurrentUser> self,
+        Provider<ReviewDb> db,
+        ChangeFinder changeFinder,
+        IdentifiedUser.GenericFactory userFactory,
+        PermissionBackend permissionBackend) {
       this.self = self;
+      this.db = db;
+      this.changeFinder = changeFinder;
+      this.userFactory = userFactory;
+      this.permissionBackend = permissionBackend;
     }
 
     IdentifiedUser getIdentifiedUser() throws QueryParseException {
@@ -81,6 +102,22 @@ public class AccountQueryBuilder extends QueryBuilder<AccountState> {
     this.args = args;
   }
 
+  @Operator
+  public Predicate<AccountState> cansee(String change)
+      throws QueryParseException, OrmException, PermissionBackendException {
+    ChangeNotes changeNotes = args.changeFinder.findOne(change);
+    if (changeNotes == null
+        || !args.permissionBackend
+            .user(args.getUser())
+            .database(args.db)
+            .change(changeNotes)
+            .test(ChangePermission.READ)) {
+      throw error(String.format("change %s not found", change));
+    }
+
+    return AccountPredicates.cansee(args, changeNotes);
+  }
+
   @Operator
   public Predicate<AccountState> email(String email) {
     return AccountPredicates.email(email);

gerrit-server/src/main/java/com/google/gerrit/server/query/account/CanSeeChangePredicate.java

@@ -0,0 +1,71 @@
+package com.google.gerrit.server.query.account;
+
+import com.google.gerrit.index.query.PostFilterPredicate;
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Provider;
+import java.util.Collection;
+import java.util.Objects;
+
+public class CanSeeChangePredicate extends PostFilterPredicate<AccountState> {
+  private final Provider<ReviewDb> db;
+  private final PermissionBackend permissionBackend;
+  private final IdentifiedUser.GenericFactory userFactory;
+  private final ChangeNotes changeNotes;
+
+  CanSeeChangePredicate(
+      Provider<ReviewDb> db,
+      PermissionBackend permissionBackend,
+      IdentifiedUser.GenericFactory userFactory,
+      ChangeNotes changeNotes) {
+    this.db = db;
+    this.permissionBackend = permissionBackend;
+    this.userFactory = userFactory;
+    this.changeNotes = changeNotes;
+  }
+
+  @Override
+  public boolean match(AccountState accountState) throws OrmException {
+    try {
+      return permissionBackend
+          .user(userFactory.create(accountState.getAccount().getId()))
+          .database(db)
+          .change(changeNotes)
+          .test(ChangePermission.READ);
+    } catch (PermissionBackendException e) {
+      throw new OrmException("Failed to check if account can see change", e);
+    }
+  }
+
+  @Override
+  public int getCost() {
+    return 1;
+  }
+
+  @Override
+  public Predicate<AccountState> copy(Collection<? extends Predicate<AccountState>> children) {
+    return new CanSeeChangePredicate(db, permissionBackend, userFactory, changeNotes);
+  }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(changeNotes.getChange().getChangeId());
+  }
+
+  @Override
+  public boolean equals(Object other) {
+    if (other == null) {
+      return false;
+    }
+    return getClass() == other.getClass()
+        && changeNotes.getChange().getChangeId()
+            == ((CanSeeChangePredicate) other).changeNotes.getChange().getChangeId();
+  }
+}

gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java

@@ -18,12 +18,22 @@ import static com.google.common.truth.Truth.assertThat;
 import static java.util.stream.Collectors.toList;
 import static org.junit.Assert.fail;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.api.access.AccessSectionInfo;
+import com.google.gerrit.extensions.api.access.PermissionInfo;
+import com.google.gerrit.extensions.api.access.PermissionRuleInfo;
+import com.google.gerrit.extensions.api.access.ProjectAccessInput;
 import com.google.gerrit.extensions.api.accounts.Accounts.QueryRequest;
+import com.google.gerrit.extensions.api.groups.GroupInput;
+import com.google.gerrit.extensions.api.projects.ProjectInput;
 import com.google.gerrit.extensions.client.ListAccountsOption;
 import com.google.gerrit.extensions.client.ProjectWatchInfo;
 import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.ChangeInput;
+import com.google.gerrit.extensions.common.GroupInfo;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.lifecycle.LifecycleManager;
@@ -60,6 +70,7 @@ import com.google.inject.Provider;
 import com.google.inject.util.Providers;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import org.eclipse.jgit.lib.Config;
@@ -291,6 +302,22 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
     assertQuery("name:" + quote(user2.name), user2);
   }
 
+  @Test
+  public void byCansee() throws Exception {
+    String domain = name("test.com");
+    AccountInfo user1 = newAccountWithEmail("account1", "account1@" + domain);
+    AccountInfo user2 = newAccountWithEmail("account2", "account2@" + domain);
+    AccountInfo user3 = newAccountWithEmail("account3", "account3@" + domain);
+
+    Project.NameKey p = createProject(name("p"));
+    ChangeInfo c = createChange(p);
+    assertQuery("name:" + domain + " cansee:" + c.changeId, user1, user2, user3);
+
+    GroupInfo group = createGroup(name("group"), user1, user2);
+    blockRead(p, group);
+    assertQuery("name:" + domain + " cansee:" + c.changeId, user3);
+  }
+
   @Test
   public void byWatchedProject() throws Exception {
     Project.NameKey p = createProject(name("p"));
@@ -456,10 +483,43 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
   }
 
   protected Project.NameKey createProject(String name) throws RestApiException {
-    gApi.projects().create(name);
+    ProjectInput in = new ProjectInput();
+    in.name = name;
+    in.createEmptyCommit = true;
+    gApi.projects().create(in);
     return new Project.NameKey(name);
   }
 
+  protected void blockRead(Project.NameKey project, GroupInfo group) throws RestApiException {
+    ProjectAccessInput in = new ProjectAccessInput();
+    in.add = new HashMap<>();
+
+    AccessSectionInfo a = new AccessSectionInfo();
+    PermissionInfo p = new PermissionInfo(null, null);
+    p.rules =
+        ImmutableMap.of(group.id, new PermissionRuleInfo(PermissionRuleInfo.Action.BLOCK, false));
+    a.permissions = ImmutableMap.of("read", p);
+    in.add = ImmutableMap.of("refs/*", a);
+
+    gApi.projects().name(project.get()).access(in);
+  }
+
+  protected ChangeInfo createChange(Project.NameKey project) throws RestApiException {
+    ChangeInput in = new ChangeInput();
+    in.subject = "A change";
+    in.project = project.get();
+    in.branch = "master";
+    return gApi.changes().create(in).get();
+  }
+
+  protected GroupInfo createGroup(String name, AccountInfo... members) throws RestApiException {
+    GroupInput in = new GroupInput();
+    in.name = name;
+    in.members =
+        Arrays.asList(members).stream().map(a -> String.valueOf(a._accountId)).collect(toList());
+    return gApi.groups().create(in).get();
+  }
+
   protected void watch(AccountInfo account, Project.NameKey project, String filter)
       throws RestApiException {
     List<ProjectWatchInfo> projectsToWatch = new ArrayList<>();