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
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 6c1fb39..48aa0bb 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -152,7 +152,7 @@
@Inject private Sequences seq;
- @Inject private InternalAccountQuery accountQuery;
+ @Inject private Provider<InternalAccountQuery> accountQueryProvider;
@Inject protected Emails emails;
@@ -1033,7 +1033,7 @@
}
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 @@
@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 @@
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) {
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java
index 365789c..4f7a5ba 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java
@@ -38,6 +38,7 @@
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 @@
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 @@
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 @@
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 @@
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
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ReviewerRecommender.java b/gerrit-server/src/main/java/com/google/gerrit/server/ReviewerRecommender.java
index b9bd658..7374833 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ReviewerRecommender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ReviewerRecommender.java
@@ -80,7 +80,7 @@
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 @@
ReviewerRecommender(
ChangeQueryBuilder changeQueryBuilder,
DynamicMap<ReviewerSuggestion> reviewerSuggestionPluginMap,
- InternalChangeQuery internalChangeQuery,
+ Provider<InternalChangeQuery> queryProvider,
WorkQueue workQueue,
Provider<ReviewDb> dbProvider,
ApprovalsUtil approvalsUtil,
@@ -98,7 +98,7 @@
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 @@
// 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 @@
}
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();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ReviewersUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ReviewersUtil.java
index b328e37..7ba75c5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ReviewersUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ReviewersUtil.java
@@ -109,7 +109,7 @@
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 @@
ReviewersUtil(
AccountLoader.Factory accountLoaderFactory,
AccountQueryBuilder accountQueryBuilder,
- AccountQueryProcessor accountQueryProcessor,
+ Provider<AccountQueryProcessor> queryProvider,
GroupBackend groupBackend,
GroupMembers.Factory groupMembersFactory,
Provider<CurrentUser> currentUser,
@@ -130,7 +130,7 @@
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 @@
try (Timer0.Context ctx = metrics.queryAccountsLatency.start()) {
try {
QueryResult<AccountState> result =
- accountQueryProcessor
+ queryProvider
+ .get()
.setLimit(suggestReviewers.getLimit() * CANDIDATE_LIST_MULTIPLIER)
.query(
AccountPredicates.andActive(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/Emails.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/Emails.java
index 15f4509..db707a8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/Emails.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/Emails.java
@@ -24,6 +24,7 @@
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 @@
@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 @@
* @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 @@
.entries()
.stream()
.forEach(e -> builder.put(e.getKey(), e.getValue().accountId()));
- accountQuery
+ queryProvider
+ .get()
.byPreferredEmail(emails)
.entries()
.stream()
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/AbandonUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/AbandonUtil.java
index 80f25af..58a908d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/AbandonUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/AbandonUtil.java
@@ -27,6 +27,7 @@
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 @@
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 @@
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 @@
}
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 @@
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 {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
index d3ddbd9..8823e6d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
@@ -228,7 +228,7 @@
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 @@
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 @@
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 @@
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);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseSorter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseSorter.java
index d596987..6fc5eaa 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseSorter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/RebaseSorter.java
@@ -21,6 +21,7 @@
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 @@
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 @@
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 @@
}
// 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)) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java
index 9892b6e..2a22c1c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java
@@ -58,6 +58,7 @@
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 @@
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 @@
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 @@
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 @@
this.mergeSorter = new MergeSorter(rw, alreadyAccepted, canMergeFlag, incoming);
this.rebaseSorter =
new RebaseSorter(
- rw,
- mergeTip.getInitialTip(),
- alreadyAccepted,
- canMergeFlag,
- internalChangeQuery,
- incoming);
+ rw, mergeTip.getInitialTip(), alreadyAccepted, canMergeFlag, queryProvider, incoming);
this.mergeUtil = mergeUtilFactory.create(project);
this.onSubmitValidatorsFactory = onSubmitValidatorsFactory;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/InternalQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/InternalQuery.java
index 64029ab..30f7328 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/InternalQuery.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/InternalQuery.java
@@ -33,6 +33,9 @@
* <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;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/QueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/QueryProcessor.java
index 65f9721..af814a6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/QueryProcessor.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/QueryProcessor.java
@@ -51,8 +51,15 @@
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 @@
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 @@
this.indexes = indexes;
this.rewriter = rewriter;
this.limitField = limitField;
+ this.used = new AtomicBoolean(false);
}
public QueryProcessor<T> setStart(int n) {
@@ -180,6 +192,7 @@
@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();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/account/AccountQueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/account/AccountQueryProcessor.java
index dd092c7..67f62a4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/account/AccountQueryProcessor.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/account/AccountQueryProcessor.java
@@ -32,6 +32,12 @@
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;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/account/InternalAccountQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/account/InternalAccountQuery.java
index 46e7d2a..153f115 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/account/InternalAccountQuery.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/account/InternalAccountQuery.java
@@ -34,6 +34,12 @@
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);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
index d3ec68f..3759a67 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
@@ -40,6 +40,12 @@
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 {
/**
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
index 3c3b223..1ddfd5a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
@@ -46,6 +46,12 @@
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());
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 1522fcf..91d7dbc 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
@@ -59,7 +59,12 @@
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);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/group/GroupQueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/group/GroupQueryProcessor.java
index d5534e2..4abb7aa 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/group/GroupQueryProcessor.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/group/GroupQueryProcessor.java
@@ -32,6 +32,12 @@
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;
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
index 8323051..c5eea0f 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
@@ -104,7 +104,7 @@
@Inject protected OneOffRequestContext oneOffRequestContext;
- @Inject protected InternalAccountQuery internalAccountQuery;
+ @Inject protected Provider<InternalAccountQuery> queryProvider;
@Inject protected AllProjectsName allProjects;
@@ -294,19 +294,19 @@
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(internalAccountQuery.byWatchedProject(p2), user3);
- assertAccounts(internalAccountQuery.byWatchedProject(allProjects), user3);
+ assertAccounts(queryProvider.get().byWatchedProject(p), user1, user2);
+ assertAccounts(queryProvider.get().byWatchedProject(p2), user3);
+ assertAccounts(queryProvider.get().byWatchedProject(allProjects), user3);
}
@Test
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 3a1e1233..9ab4db0 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -167,13 +167,13 @@
@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 @@
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 @@
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 @@
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()))
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
index 96f573f..db89eda 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
@@ -40,7 +40,6 @@
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 @@
@Inject protected OneOffRequestContext oneOffRequestContext;
- @Inject protected InternalAccountQuery internalAccountQuery;
-
@Inject protected AllProjectsName allProjects;
@Inject protected GroupCache groupCache;
diff --git a/plugins/singleusergroup b/plugins/singleusergroup
index 1f28eff..89e92bc 160000
--- a/plugins/singleusergroup
+++ b/plugins/singleusergroup
@@ -1 +1 @@
-Subproject commit 1f28effbe7878e51776f63af4e2541183a445286
+Subproject commit 89e92bc36f04184a709cf16fc9773eebba8d9c0f
