Ensure InternalQuery/QueryProcessor are one-time-use

These classes are non-threadsafe and have mutable fields for a
builder-like pattern; they should not be reused. Nonetheless, this was
happening in a few places, which seemed to not be causing problems, but
they may have just been hard to observe.

Ensure they are consistently used only once with a checkState in
QueryProcessor#query, and document this limitation.

Change-Id: I1c7d392b1094dcb562894108b652ee7750d98da4

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

@@ -152,7 +152,7 @@ public class AccountIT extends AbstractDaemonTest {
 
   @Inject private Sequences seq;
 
-  @Inject private InternalAccountQuery accountQuery;
+  @Inject private Provider<InternalAccountQuery> accountQueryProvider;
 
   @Inject protected Emails emails;
 
@@ -1033,7 +1033,7 @@ public class AccountIT extends AbstractDaemonTest {
     }
 
     assertThat(accountCache.getOrNull(admin.id)).isNull();
-    assertThat(accountQuery.byDefault(admin.id.toString())).isEmpty();
+    assertThat(accountQueryProvider.get().byDefault(admin.id.toString())).isEmpty();
   }
 
   @Test
@@ -1257,7 +1257,7 @@ public class AccountIT extends AbstractDaemonTest {
   @Test
   public void internalQueryFindActiveAndInactiveAccounts() throws Exception {
     String name = name("foo");
-    assertThat(accountQuery.byDefault(name)).isEmpty();
+    assertThat(accountQueryProvider.get().byDefault(name)).isEmpty();
 
     TestAccount foo1 = accountCreator.create(name + "-1");
     assertThat(gApi.accounts().id(foo1.username).getActive()).isTrue();
@@ -1266,7 +1266,7 @@ public class AccountIT extends AbstractDaemonTest {
     gApi.accounts().id(foo2.username).setActive(false);
     assertThat(gApi.accounts().id(foo2.username).getActive()).isFalse();
 
-    assertThat(accountQuery.byDefault(name)).hasSize(2);
+    assertThat(accountQueryProvider.get().byDefault(name)).hasSize(2);
   }
 
   private void assertSequenceNumbers(List<SshKeyInfo> sshKeys) {

gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java

@@ -38,6 +38,7 @@ import com.google.gwtexpui.server.CacheHeaders;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import com.google.inject.Singleton;
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -63,7 +64,7 @@ class BecomeAnyAccountLoginServlet extends HttpServlet {
   private final AccountCache accountCache;
   private final AccountManager accountManager;
   private final SiteHeaderFooter headers;
-  private final InternalAccountQuery accountQuery;
+  private final Provider<InternalAccountQuery> queryProvider;
 
   @Inject
   BecomeAnyAccountLoginServlet(
@@ -73,14 +74,14 @@ class BecomeAnyAccountLoginServlet extends HttpServlet {
       AccountCache ac,
       AccountManager am,
       SiteHeaderFooter shf,
-      InternalAccountQuery aq) {
+      Provider<InternalAccountQuery> qp) {
     webSession = ws;
     schema = sf;
     accounts = a;
     accountCache = ac;
     accountManager = am;
     headers = shf;
-    accountQuery = aq;
+    queryProvider = qp;
   }
 
   @Override
@@ -198,7 +199,8 @@ class BecomeAnyAccountLoginServlet extends HttpServlet {
 
   private AuthResult byUserName(String userName) {
     try {
-      List<AccountState> accountStates = accountQuery.byExternalId(SCHEME_USERNAME, userName);
+      List<AccountState> accountStates =
+          queryProvider.get().byExternalId(SCHEME_USERNAME, userName);
       if (accountStates.isEmpty()) {
         getServletContext().log("No accounts with username " + userName + " found");
         return null;
@@ -217,7 +219,8 @@ class BecomeAnyAccountLoginServlet extends HttpServlet {
   private AuthResult byPreferredEmail(String email) {
     try (ReviewDb db = schema.open()) {
       Optional<Account> match =
-          accountQuery
+          queryProvider
+              .get()
               .byPreferredEmail(email)
               .stream()
               // the index query also matches prefixes, filter those out

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

@@ -80,7 +80,7 @@ public class ReviewerRecommender {
   private final ChangeQueryBuilder changeQueryBuilder;
   private final Config config;
   private final DynamicMap<ReviewerSuggestion> reviewerSuggestionPluginMap;
-  private final InternalChangeQuery internalChangeQuery;
+  private final Provider<InternalChangeQuery> queryProvider;
   private final WorkQueue workQueue;
   private final Provider<ReviewDb> dbProvider;
   private final ApprovalsUtil approvalsUtil;
@@ -89,7 +89,7 @@ public class ReviewerRecommender {
   ReviewerRecommender(
       ChangeQueryBuilder changeQueryBuilder,
       DynamicMap<ReviewerSuggestion> reviewerSuggestionPluginMap,
-      InternalChangeQuery internalChangeQuery,
+      Provider<InternalChangeQuery> queryProvider,
       WorkQueue workQueue,
       Provider<ReviewDb> dbProvider,
       ApprovalsUtil approvalsUtil,
@@ -98,7 +98,7 @@ public class ReviewerRecommender {
     fillOptions.addAll(AccountLoader.DETAILED_OPTIONS);
     this.changeQueryBuilder = changeQueryBuilder;
     this.config = config;
-    this.internalChangeQuery = internalChangeQuery;
+    this.queryProvider = queryProvider;
     this.reviewerSuggestionPluginMap = reviewerSuggestionPluginMap;
     this.workQueue = workQueue;
     this.dbProvider = dbProvider;
@@ -202,7 +202,8 @@ public class ReviewerRecommender {
     // Get the user's last 25 changes, check approvals
     try {
       List<ChangeData> result =
-          internalChangeQuery
+          queryProvider
+              .get()
               .setLimit(25)
               .setRequestedFields(ImmutableSet.of(ChangeField.REVIEWER.getName()))
               .query(changeQueryBuilder.owner("self"));
@@ -268,7 +269,7 @@ public class ReviewerRecommender {
     }
 
     List<List<ChangeData>> result =
-        internalChangeQuery.setLimit(25).setRequestedFields(ImmutableSet.of()).query(predicates);
+        queryProvider.get().setLimit(25).setRequestedFields(ImmutableSet.of()).query(predicates);
 
     Iterator<List<ChangeData>> queryResultIterator = result.iterator();
     Iterator<Account.Id> reviewersIterator = reviewers.keySet().iterator();

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

@@ -109,7 +109,7 @@ public class ReviewersUtil {
 
   private final AccountLoader accountLoader;
   private final AccountQueryBuilder accountQueryBuilder;
-  private final AccountQueryProcessor accountQueryProcessor;
+  private final Provider<AccountQueryProcessor> queryProvider;
   private final GroupBackend groupBackend;
   private final GroupMembers.Factory groupMembersFactory;
   private final Provider<CurrentUser> currentUser;
@@ -120,7 +120,7 @@ public class ReviewersUtil {
   ReviewersUtil(
       AccountLoader.Factory accountLoaderFactory,
       AccountQueryBuilder accountQueryBuilder,
-      AccountQueryProcessor accountQueryProcessor,
+      Provider<AccountQueryProcessor> queryProvider,
       GroupBackend groupBackend,
       GroupMembers.Factory groupMembersFactory,
       Provider<CurrentUser> currentUser,
@@ -130,7 +130,7 @@ public class ReviewersUtil {
     fillOptions.addAll(AccountLoader.DETAILED_OPTIONS);
     this.accountLoader = accountLoaderFactory.create(fillOptions);
     this.accountQueryBuilder = accountQueryBuilder;
-    this.accountQueryProcessor = accountQueryProcessor;
+    this.queryProvider = queryProvider;
     this.currentUser = currentUser;
     this.groupBackend = groupBackend;
     this.groupMembersFactory = groupMembersFactory;
@@ -199,7 +199,8 @@ public class ReviewersUtil {
     try (Timer0.Context ctx = metrics.queryAccountsLatency.start()) {
       try {
         QueryResult<AccountState> result =
-            accountQueryProcessor
+            queryProvider
+                .get()
                 .setLimit(suggestReviewers.getLimit() * CANDIDATE_LIST_MULTIPLIER)
                 .query(
                     AccountPredicates.andActive(

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

@@ -24,6 +24,7 @@ import com.google.gerrit.server.account.externalids.ExternalIds;
 import com.google.gerrit.server.query.account.InternalAccountQuery;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import com.google.inject.Singleton;
 import java.io.IOException;
 import java.util.List;
@@ -32,12 +33,12 @@ import java.util.List;
 @Singleton
 public class Emails {
   private final ExternalIds externalIds;
-  private final InternalAccountQuery accountQuery;
+  private final Provider<InternalAccountQuery> queryProvider;
 
   @Inject
-  public Emails(ExternalIds externalIds, InternalAccountQuery accountQuery) {
+  public Emails(ExternalIds externalIds, Provider<InternalAccountQuery> queryProvider) {
     this.externalIds = externalIds;
-    this.accountQuery = accountQuery;
+    this.queryProvider = queryProvider;
   }
 
   /**
@@ -60,7 +61,7 @@ public class Emails {
    * @see #getAccountsFor(String...)
    */
   public ImmutableSet<Account.Id> getAccountFor(String email) throws IOException, OrmException {
-    List<AccountState> byPreferredEmail = accountQuery.byPreferredEmail(email);
+    List<AccountState> byPreferredEmail = queryProvider.get().byPreferredEmail(email);
     return Streams.concat(
             externalIds.byEmail(email).stream().map(e -> e.accountId()),
             byPreferredEmail
@@ -85,7 +86,8 @@ public class Emails {
         .entries()
         .stream()
         .forEach(e -> builder.put(e.getKey(), e.getValue().accountId()));
-    accountQuery
+    queryProvider
+        .get()
         .byPreferredEmail(emails)
         .entries()
         .stream()

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

@@ -27,6 +27,7 @@ import com.google.gerrit.server.query.change.ChangeQueryProcessor;
 import com.google.gerrit.server.update.BatchUpdate;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import com.google.inject.Singleton;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -40,7 +41,7 @@ public class AbandonUtil {
   private static final Logger log = LoggerFactory.getLogger(AbandonUtil.class);
 
   private final ChangeCleanupConfig cfg;
-  private final ChangeQueryProcessor queryProcessor;
+  private final Provider<ChangeQueryProcessor> queryProvider;
   private final ChangeQueryBuilder queryBuilder;
   private final Abandon abandon;
   private final InternalUser internalUser;
@@ -49,11 +50,11 @@ public class AbandonUtil {
   AbandonUtil(
       ChangeCleanupConfig cfg,
       InternalUser.Factory internalUserFactory,
-      ChangeQueryProcessor queryProcessor,
+      Provider<ChangeQueryProcessor> queryProvider,
       ChangeQueryBuilder queryBuilder,
       Abandon abandon) {
     this.cfg = cfg;
-    this.queryProcessor = queryProcessor;
+    this.queryProvider = queryProvider;
     this.queryBuilder = queryBuilder;
     this.abandon = abandon;
     internalUser = internalUserFactory.create();
@@ -72,7 +73,7 @@ public class AbandonUtil {
       }
 
       List<ChangeData> changesToAbandon =
-          queryProcessor.enforceVisibility(false).query(queryBuilder.parse(query)).entities();
+          queryProvider.get().enforceVisibility(false).query(queryBuilder.parse(query)).entities();
       ImmutableListMultimap.Builder<Project.NameKey, ChangeControl> builder =
           ImmutableListMultimap.builder();
       for (ChangeData cd : changesToAbandon) {
@@ -110,7 +111,11 @@ public class AbandonUtil {
     for (ChangeControl cc : changeControls) {
       String newQuery = query + " change:" + cc.getId();
       List<ChangeData> changesToAbandon =
-          queryProcessor.enforceVisibility(false).query(queryBuilder.parse(newQuery)).entities();
+          queryProvider
+              .get()
+              .enforceVisibility(false)
+              .query(queryBuilder.parse(newQuery))
+              .entities();
       if (!changesToAbandon.isEmpty()) {
         validChanges.add(cc);
       } else {

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

@@ -228,7 +228,7 @@ public class MergeOp implements AutoCloseable {
   private final InternalUser.Factory internalUserFactory;
   private final MergeSuperSet mergeSuperSet;
   private final MergeValidators.Factory mergeValidatorsFactory;
-  private final InternalChangeQuery internalChangeQuery;
+  private final Provider<InternalChangeQuery> queryProvider;
   private final SubmitStrategyFactory submitStrategyFactory;
   private final SubmoduleOp.Factory subOpFactory;
   private final Provider<MergeOpRepoManager> ormProvider;
@@ -255,7 +255,7 @@ public class MergeOp implements AutoCloseable {
       InternalUser.Factory internalUserFactory,
       MergeSuperSet mergeSuperSet,
       MergeValidators.Factory mergeValidatorsFactory,
-      InternalChangeQuery internalChangeQuery,
+      Provider<InternalChangeQuery> queryProvider,
       SubmitStrategyFactory submitStrategyFactory,
       SubmoduleOp.Factory subOpFactory,
       Provider<MergeOpRepoManager> ormProvider,
@@ -267,7 +267,7 @@ public class MergeOp implements AutoCloseable {
     this.internalUserFactory = internalUserFactory;
     this.mergeSuperSet = mergeSuperSet;
     this.mergeValidatorsFactory = mergeValidatorsFactory;
-    this.internalChangeQuery = internalChangeQuery;
+    this.queryProvider = queryProvider;
     this.submitStrategyFactory = submitStrategyFactory;
     this.subOpFactory = subOpFactory;
     this.ormProvider = ormProvider;
@@ -860,7 +860,7 @@ public class MergeOp implements AutoCloseable {
 
   private void abandonAllOpenChangeForDeletedProject(Project.NameKey destProject) {
     try {
-      for (ChangeData cd : internalChangeQuery.byProjectOpen(destProject)) {
+      for (ChangeData cd : queryProvider.get().byProjectOpen(destProject)) {
         try (BatchUpdate bu =
             batchUpdateFactory.create(db, destProject, internalUserFactory.create(), ts)) {
           bu.setRequestId(submissionId);

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

@@ -21,6 +21,7 @@ import com.google.gerrit.server.git.strategy.CommitMergeStatus;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gwtorm.server.OrmException;
+import com.google.inject.Provider;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -41,7 +42,7 @@ public class RebaseSorter {
   private final RevFlag canMergeFlag;
   private final RevCommit initialTip;
   private final Set<RevCommit> alreadyAccepted;
-  private final InternalChangeQuery internalChangeQuery;
+  private final Provider<InternalChangeQuery> queryProvider;
   private final Set<CodeReviewCommit> incoming;
 
   public RebaseSorter(
@@ -49,13 +50,13 @@ public class RebaseSorter {
       RevCommit initialTip,
       Set<RevCommit> alreadyAccepted,
       RevFlag canMergeFlag,
-      InternalChangeQuery internalChangeQuery,
+      Provider<InternalChangeQuery> queryProvider,
       Set<CodeReviewCommit> incoming) {
     this.rw = rw;
     this.canMergeFlag = canMergeFlag;
     this.initialTip = initialTip;
     this.alreadyAccepted = alreadyAccepted;
-    this.internalChangeQuery = internalChangeQuery;
+    this.queryProvider = queryProvider;
     this.incoming = incoming;
   }
 
@@ -116,7 +117,7 @@ public class RebaseSorter {
       }
 
       // check if the commit associated change is merged in the same branch
-      List<ChangeData> changes = internalChangeQuery.byCommit(commit);
+      List<ChangeData> changes = queryProvider.get().byCommit(commit);
       for (ChangeData change : changes) {
         if (change.change().getStatus() == Status.MERGED
             && change.change().getDest().equals(dest)) {

gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java

@@ -58,6 +58,7 @@ import com.google.gerrit.server.update.BatchUpdate;
 import com.google.gerrit.server.util.RequestId;
 import com.google.inject.Inject;
 import com.google.inject.Module;
+import com.google.inject.Provider;
 import com.google.inject.assistedinject.Assisted;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -119,7 +120,7 @@ public abstract class SubmitStrategy {
     final RebaseChangeOp.Factory rebaseFactory;
     final OnSubmitValidators.Factory onSubmitValidatorsFactory;
     final TagCache tagCache;
-    final InternalChangeQuery internalChangeQuery;
+    final Provider<InternalChangeQuery> queryProvider;
 
     final Branch.NameKey destBranch;
     final CodeReviewRevWalk rw;
@@ -159,7 +160,7 @@ public abstract class SubmitStrategy {
         RebaseChangeOp.Factory rebaseFactory,
         OnSubmitValidators.Factory onSubmitValidatorsFactory,
         TagCache tagCache,
-        InternalChangeQuery internalChangeQuery,
+        Provider<InternalChangeQuery> queryProvider,
         @Assisted Branch.NameKey destBranch,
         @Assisted CommitStatus commitStatus,
         @Assisted CodeReviewRevWalk rw,
@@ -188,7 +189,7 @@ public abstract class SubmitStrategy {
       this.projectCache = projectCache;
       this.rebaseFactory = rebaseFactory;
       this.tagCache = tagCache;
-      this.internalChangeQuery = internalChangeQuery;
+      this.queryProvider = queryProvider;
 
       this.serverIdent = serverIdent;
       this.destBranch = destBranch;
@@ -214,12 +215,7 @@ public abstract class SubmitStrategy {
       this.mergeSorter = new MergeSorter(rw, alreadyAccepted, canMergeFlag, incoming);
       this.rebaseSorter =
           new RebaseSorter(
-              rw,
+              rw, mergeTip.getInitialTip(), alreadyAccepted, canMergeFlag, queryProvider, incoming);
-              mergeTip.getInitialTip(),
-              alreadyAccepted,
-              canMergeFlag,
-              internalChangeQuery,
-              incoming);
       this.mergeUtil = mergeUtilFactory.create(project);
       this.onSubmitValidatorsFactory = onSubmitValidatorsFactory;
     }

gerrit-server/src/main/java/com/google/gerrit/server/query/InternalQuery.java

@@ -33,6 +33,9 @@ import java.util.Set;
  * <p>By default, visibility of returned entities is not enforced (unlike in {@link
  * QueryProcessor}). The methods in this class are not typically used by user-facing paths, but
  * rather by internal callers that need to process all matching results.
+ *
+ * <p>Instances are one-time-use. Other singleton classes should inject a Provider rather than
+ * holding on to a single instance.
  */
 public class InternalQuery<T> {
   private final QueryProcessor<T> queryProcessor;

gerrit-server/src/main/java/com/google/gerrit/server/query/QueryProcessor.java

@@ -51,8 +51,15 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.stream.IntStream;
 
+/**
+ * Lower-level implementation for executing a single query over a secondary index.
+ *
+ * <p>Instances are one-time-use. Other singleton classes should inject a Provider rather than
+ * holding on to a single instance.
+ */
 public abstract class QueryProcessor<T> {
   @Singleton
   protected static class Metrics {
@@ -81,6 +88,10 @@ public abstract class QueryProcessor<T> {
   private final IndexRewriter<T> rewriter;
   private final String limitField;
 
+  // This class is not generally thread-safe, but programmer error may result in it being shared
+  // across threads. At least ensure the bit for checking if it's been used is threadsafe.
+  private final AtomicBoolean used;
+
   protected int start;
 
   private boolean enforceVisibility = true;
@@ -104,6 +115,7 @@ public abstract class QueryProcessor<T> {
     this.indexes = indexes;
     this.rewriter = rewriter;
     this.limitField = limitField;
+    this.used = new AtomicBoolean(false);
   }
 
   public QueryProcessor<T> setStart(int n) {
@@ -180,6 +192,7 @@ public abstract class QueryProcessor<T> {
       @Nullable List<String> queryStrings, List<Predicate<T>> queries)
       throws OrmException, QueryParseException {
     long startNanos = System.nanoTime();
+    checkState(!used.getAndSet(true), "%s has already been used", getClass().getSimpleName());
     int cnt = queries.size();
     if (queryStrings != null) {
       int qs = queryStrings.size();

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

@@ -32,6 +32,12 @@ import com.google.gerrit.server.query.QueryProcessor;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
+/**
+ * Query processor for the account index.
+ *
+ * <p>Instances are one-time-use. Other singleton classes should inject a Provider rather than
+ * holding on to a single instance.
+ */
 public class AccountQueryProcessor extends QueryProcessor<AccountState> {
   private final AccountControl.Factory accountControlFactory;
 

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

@@ -34,6 +34,12 @@ import java.util.Set;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+/**
+ * Query wrapper for the account index.
+ *
+ * <p>Instances are one-time-use. Other singleton classes should inject a Provider rather than
+ * holding on to a single instance.
+ */
 public class InternalAccountQuery extends InternalQuery<AccountState> {
   private static final Logger log = LoggerFactory.getLogger(InternalAccountQuery.class);
 

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

@@ -40,6 +40,12 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 
+/**
+ * Query processor for the change index.
+ *
+ * <p>Instances are one-time-use. Other singleton classes should inject a Provider rather than
+ * holding on to a single instance.
+ */
 public class ChangeQueryProcessor extends QueryProcessor<ChangeData>
     implements PluginDefinedAttributesFactory {
   /**

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

@@ -46,6 +46,12 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 
+/**
+ * Query wrapper for the change index.
+ *
+ * <p>Instances are one-time-use. Other singleton classes should inject a Provider rather than
+ * holding on to a single instance.
+ */
 public class InternalChangeQuery extends InternalQuery<ChangeData> {
   private static Predicate<ChangeData> ref(Branch.NameKey branch) {
     return new RefPredicate(branch.get());

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

@@ -59,7 +59,12 @@ import org.joda.time.format.DateTimeFormatter;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-/** Change query implementation that outputs to a stream in the style of an SSH command. */
+/**
+ * Change query implementation that outputs to a stream in the style of an SSH command.
+ *
+ * <p>Instances are one-time-use. Other singleton classes should inject a Provider rather than
+ * holding on to a single instance.
+ */
 public class OutputStreamQuery {
   private static final Logger log = LoggerFactory.getLogger(OutputStreamQuery.class);
 

gerrit-server/src/main/java/com/google/gerrit/server/query/group/GroupQueryProcessor.java

@@ -32,6 +32,12 @@ import com.google.gerrit.server.query.QueryProcessor;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
+/**
+ * Query processor for the group index.
+ *
+ * <p>Instances are one-time-use. Other singleton classes should inject a Provider rather than
+ * holding on to a single instance.
+ */
 public class GroupQueryProcessor extends QueryProcessor<AccountGroup> {
   private final GroupControl.GenericFactory groupControlFactory;
 

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

@@ -104,7 +104,7 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
 
   @Inject protected OneOffRequestContext oneOffRequestContext;
 
-  @Inject protected InternalAccountQuery internalAccountQuery;
+  @Inject protected Provider<InternalAccountQuery> queryProvider;
 
   @Inject protected AllProjectsName allProjects;
 
@@ -294,19 +294,19 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
     AccountInfo user2 = newAccountWithFullName("jroe", "Jane Roe");
     AccountInfo user3 = newAccountWithFullName("user3", "Mr Selfish");
 
-    assertThat(internalAccountQuery.byWatchedProject(p)).isEmpty();
+    assertThat(queryProvider.get().byWatchedProject(p)).isEmpty();
 
     watch(user1, p, null);
-    assertAccounts(internalAccountQuery.byWatchedProject(p), user1);
+    assertAccounts(queryProvider.get().byWatchedProject(p), user1);
 
     watch(user2, p, "keyword");
-    assertAccounts(internalAccountQuery.byWatchedProject(p), user1, user2);
+    assertAccounts(queryProvider.get().byWatchedProject(p), user1, user2);
 
     watch(user3, p2, "keyword");
     watch(user3, allProjects, "keyword");
-    assertAccounts(internalAccountQuery.byWatchedProject(p), user1, user2);
+    assertAccounts(queryProvider.get().byWatchedProject(p), user1, user2);
-    assertAccounts(internalAccountQuery.byWatchedProject(p2), user3);
+    assertAccounts(queryProvider.get().byWatchedProject(p2), user3);
-    assertAccounts(internalAccountQuery.byWatchedProject(allProjects), user3);
+    assertAccounts(queryProvider.get().byWatchedProject(allProjects), user3);
   }
 
   @Test

gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java

@@ -167,13 +167,13 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
   @Inject protected ChangeIndexer indexer;
   @Inject protected IndexConfig indexConfig;
   @Inject protected InMemoryRepositoryManager repoManager;
-  @Inject protected InternalChangeQuery internalChangeQuery;
+  @Inject protected Provider<InternalChangeQuery> queryProvider;
   @Inject protected ChangeNotes.Factory notesFactory;
   @Inject protected OneOffRequestContext oneOffRequestContext;
   @Inject protected PatchSetInserter.Factory patchSetFactory;
   @Inject protected PatchSetUtil psUtil;
   @Inject protected ChangeControl.GenericFactory changeControlFactory;
-  @Inject protected ChangeQueryProcessor queryProcessor;
+  @Inject protected Provider<ChangeQueryProcessor> queryProcessorProvider;
   @Inject protected SchemaCreator schemaCreator;
   @Inject protected SchemaFactory<ReviewDb> schemaFactory;
   @Inject protected Sequences seq;
@@ -2011,7 +2011,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
 
     for (int i = 1; i <= 11; i++) {
       Iterable<ChangeData> cds =
-          internalChangeQuery.byCommitsOnBranchNotMerged(repo.getRepository(), db, dest, shas, i);
+          queryProvider.get().byCommitsOnBranchNotMerged(repo.getRepository(), db, dest, shas, i);
       Iterable<Integer> ids = FluentIterable.from(cds).transform(in -> in.getId().get());
       String name = "limit " + i;
       assertThat(ids).named(name).hasSize(n);
@@ -2029,7 +2029,10 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     requestContext.setContext(newRequestContext(userId));
     // Use QueryProcessor directly instead of API so we get ChangeDatas back.
     List<ChangeData> cds =
-        queryProcessor.query(queryBuilder.parse(change.getId().toString())).entities();
+        queryProcessorProvider
+            .get()
+            .query(queryBuilder.parse(change.getId().toString()))
+            .entities();
     assertThat(cds).hasSize(1);
 
     ChangeData cd = cds.get(0);
@@ -2059,7 +2062,8 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
     requestContext.setContext(newRequestContext(userId));
     // Use QueryProcessor directly instead of API so we get ChangeDatas back.
     List<ChangeData> cds =
-        queryProcessor
+        queryProcessorProvider
+            .get()
             .setRequestedFields(
                 ImmutableSet.of(ChangeField.PATCH_SET.getName(), ChangeField.CHANGE.getName()))
             .query(queryBuilder.parse(change.getId().toString()))

gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java

@@ -40,7 +40,6 @@ import com.google.gerrit.server.account.GroupCache;
 import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.group.GroupsUpdate;
 import com.google.gerrit.server.group.ServerInitiated;
-import com.google.gerrit.server.query.account.InternalAccountQuery;
 import com.google.gerrit.server.schema.SchemaCreator;
 import com.google.gerrit.server.util.ManualRequestContext;
 import com.google.gerrit.server.util.OneOffRequestContext;
@@ -94,8 +93,6 @@ public abstract class AbstractQueryGroupsTest extends GerritServerTests {
 
   @Inject protected OneOffRequestContext oneOffRequestContext;
 
-  @Inject protected InternalAccountQuery internalAccountQuery;
-
   @Inject protected AllProjectsName allProjects;
 
   @Inject protected GroupCache groupCache;