Merge "Also return inactive accounts for internal account queries"

* submodules:
* Update plugins/singleusergroup from branch 'master'
  - Adapt to change in core that includes inactive accounts in query results
    
    By default account queries now return also inactive accounts. To keep
    the old behaviour of suggesting only active accounts we must now add a
    predicate for "is:active" explicitly.
    
    Change-Id: I021294187d17ca94f08267c56c579bdd6ab4ac59
    Signed-off-by: Edwin Kempin <ekempin@google.com>

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java

@@ -89,6 +89,7 @@ import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.notedb.rebuild.ChangeRebuilderImpl;
 import com.google.gerrit.server.project.RefPattern;
+import com.google.gerrit.server.query.account.InternalAccountQuery;
 import com.google.gerrit.server.util.MagicBranch;
 import com.google.gerrit.testutil.ConfigSuite;
 import com.google.gerrit.testutil.FakeEmailSender.Message;
@@ -151,6 +152,8 @@ public class AccountIT extends AbstractDaemonTest {
 
   @Inject private Sequences seq;
 
+  @Inject private InternalAccountQuery accountQuery;
+
   private AccountIndexedCounter accountIndexedCounter;
   private RegistrationHandle accountIndexEventCounterHandle;
   private ExternalIdsUpdate externalIdsUpdate;
@@ -1228,6 +1231,21 @@ public class AccountIT extends AbstractDaemonTest {
     assertThat(checkInfo.checkAccountsResult.problems).containsExactlyElementsIn(expectedProblems);
   }
 
+  @Test
+  public void internalQueryFindActiveAndInactiveAccounts() throws Exception {
+    String name = name("foo");
+    assertThat(accountQuery.byDefault(name)).isEmpty();
+
+    TestAccount foo1 = accountCreator.create(name + "-1");
+    assertThat(gApi.accounts().id(foo1.username).getActive()).isTrue();
+
+    TestAccount foo2 = accountCreator.create(name + "-2");
+    gApi.accounts().id(foo2.username).setActive(false);
+    assertThat(gApi.accounts().id(foo2.username).getActive()).isFalse();
+
+    assertThat(accountQuery.byDefault(name)).hasSize(2);
+  }
+
   private void assertSequenceNumbers(List<SshKeyInfo> sshKeys) {
     int seq = 1;
     for (SshKeyInfo key : sshKeys) {

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

@@ -402,6 +402,24 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
         .inOrder();
   }
 
+  @Test
+  public void suggestNoInactiveAccounts() throws Exception {
+    String name = name("foo");
+    TestAccount foo1 = accountCreator.create(name + "-1");
+    assertThat(gApi.accounts().id(foo1.username).getActive()).isTrue();
+
+    TestAccount foo2 = accountCreator.create(name + "-2");
+    assertThat(gApi.accounts().id(foo2.username).getActive()).isTrue();
+
+    String changeId = createChange().getChangeId();
+    assertReviewers(
+        suggestReviewers(changeId, name), ImmutableList.of(foo1, foo2), ImmutableList.of());
+
+    gApi.accounts().id(foo2.username).setActive(false);
+    assertThat(gApi.accounts().id(foo2.username).getActive()).isFalse();
+    assertReviewers(suggestReviewers(changeId, name), ImmutableList.of(foo1), ImmutableList.of());
+  }
+
   private List<SuggestedReviewerInfo> suggestReviewers(String changeId, String query)
       throws Exception {
     return gApi.changes().id(changeId).suggestReviewers(query).get();

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

@@ -43,6 +43,7 @@ import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.query.QueryParseException;
 import com.google.gerrit.server.query.QueryResult;
+import com.google.gerrit.server.query.account.AccountPredicates;
 import com.google.gerrit.server.query.account.AccountQueryBuilder;
 import com.google.gerrit.server.query.account.AccountQueryProcessor;
 import com.google.gwtorm.server.OrmException;
@@ -200,7 +201,9 @@ public class ReviewersUtil {
         QueryResult<AccountState> result =
             accountQueryProcessor
                 .setLimit(suggestReviewers.getLimit() * CANDIDATE_LIST_MULTIPLIER)
-                .query(accountQueryBuilder.defaultQuery(suggestReviewers.getQuery()));
+                .query(
+                    AccountPredicates.andActive(
+                        accountQueryBuilder.defaultQuery(suggestReviewers.getQuery())));
         return result.entities().stream().map(a -> a.getAccount().getId()).collect(toList());
       } catch (QueryParseException e) {
         return ImmutableList.of();

gerrit-server/src/main/java/com/google/gerrit/server/account/QueryAccounts.java

@@ -29,6 +29,7 @@ import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.query.Predicate;
 import com.google.gerrit.server.query.QueryParseException;
 import com.google.gerrit.server.query.QueryResult;
+import com.google.gerrit.server.query.account.AccountPredicates;
 import com.google.gerrit.server.query.account.AccountQueryBuilder;
 import com.google.gerrit.server.query.account.AccountQueryProcessor;
 import com.google.gwtorm.server.OrmException;
@@ -180,6 +181,11 @@ public class QueryAccounts implements RestReadView<TopLevelResource> {
       } else {
         queryPred = queryBuilder.parse(query);
       }
+      if (!AccountPredicates.hasActive(queryPred)) {
+        // if neither 'is:active' nor 'is:inactive' appears in the query only
+        // active accounts should be queried
+        queryPred = AccountPredicates.andActive(queryPred);
+      }
       QueryResult<AccountState> result = queryProcessor.query(queryPred);
       for (AccountState accountState : result.entities()) {
         Account.Id id = accountState.getAccount().getId();

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

@@ -21,7 +21,6 @@ import com.google.gerrit.server.index.IndexRewriter;
 import com.google.gerrit.server.index.QueryOptions;
 import com.google.gerrit.server.query.Predicate;
 import com.google.gerrit.server.query.QueryParseException;
-import com.google.gerrit.server.query.account.AccountPredicates;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 
@@ -38,9 +37,6 @@ public class AccountIndexRewriter implements IndexRewriter<AccountState> {
   @Override
   public Predicate<AccountState> rewrite(Predicate<AccountState> in, QueryOptions opts)
       throws QueryParseException {
-    if (!AccountPredicates.hasActive(in)) {
-      in = Predicate.and(in, AccountPredicates.isActive());
-    }
     AccountIndex index = indexes.getSearchIndex();
     checkNotNull(index, "no active search index configured for accounts");
     return new IndexedAccountQuery(index, in, opts);

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

@@ -31,6 +31,10 @@ public class AccountPredicates {
     return QueryBuilder.find(p, AccountPredicate.class, AccountField.ACTIVE.getName()) != null;
   }
 
+  public static Predicate<AccountState> andActive(Predicate<AccountState> p) {
+    return Predicate.and(p, isActive());
+  }
+
   public static Predicate<AccountState> defaultPredicate(String query) {
     // Adapt the capacity of this list when adding more default predicates.
     List<Predicate<AccountState>> preds = Lists.newArrayListWithCapacity(3);