Remove asynchronous loading of starred changes
When the starred changes were stored in the database for performance
reasons it was important to load them asynchronously while a change
query was executed. Now the starred changes are stored in the change
index so that this asynchronous loading is no longer needed.
This removes the starred changes functionality from IdentifiedUser
which was only used by IsStarredByLegacyPredicate.
IsStarredByLegacyPredicate is used to serve the "is:starred" query
when the current change index is old and doesn't contain any starred
changes. In this case we must load the starred changes from the git
backend. This is expensive since all starred changes refs must be
iterated, but implementing some asynchronous loading for this case
seems overkill since IsStarredByLegacyPredicate is only used until the
online reindexing to a newer index version that contains starred
changes is done.
The old implementation of IsStarredByLegacyPredicate was broken since
it tried to query the starred changes from the index which doesn't
contain starred changes when IsStarredByLegacyPredicate is used.
This also fixes the following warning from StarredChangesUtil when the
newest change index version is used from which the deprecated
STARREDBY is removed:
WARN com.google.gerrit.server.StarredChangesUtil : Cannot query
starred changes for account 1000000
java.lang.IllegalArgumentException: numHits must be > 0; please use
TotalHitCountCollector if you just need the total hit count
This warning occurred because the asynchronous loading of changes used
the STARREDBY field to load changes from the index, but the index
didn't contain this field anymore.
Since the starred changes functionality is removed from IdentifiedUser
the abstract CurrentUser.getStarredChanges() method is no longer
needed and hence it is removed. This means that all subclasses of
CurrentUser must be adapted. One subclass exists in the replication
plugin and hence this plugin must be updated.
Also, since IdentifiedUser no longer needs StarredChangesUtil, binding
a null provider for this class in tests is no longer needed.
Change-Id: I14a38e9b243b4b182230fe70b023203492a81904
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/AnonymousUser.java b/gerrit-server/src/main/java/com/google/gerrit/server/AnonymousUser.java
index e916aff..36888e3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/AnonymousUser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/AnonymousUser.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupMembership;
import com.google.gerrit.server.account.ListGroupMembership;
@@ -22,7 +21,6 @@
import com.google.inject.Inject;
import java.util.Collections;
-import java.util.Set;
/** An anonymous user who has not yet authenticated. */
public class AnonymousUser extends CurrentUser {
@@ -37,11 +35,6 @@
}
@Override
- public Set<Change.Id> getStarredChanges() {
- return Collections.emptySet();
- }
-
- @Override
public String toString() {
return "ANONYMOUS";
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/CurrentUser.java b/gerrit-server/src/main/java/com/google/gerrit/server/CurrentUser.java
index 16e868f..34a2d02 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/CurrentUser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/CurrentUser.java
@@ -16,13 +16,10 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupMembership;
import com.google.inject.servlet.RequestScoped;
-import java.util.Set;
-
/**
* Information about the currently logged in user.
* <p>
@@ -87,10 +84,6 @@
*/
public abstract GroupMembership getEffectiveGroups();
- /** Set of changes starred by this user. */
- @Deprecated
- public abstract Set<Change.Id> getStarredChanges();
-
/** Unique name of the user on this server, if one has been assigned. */
public String getUserName() {
return null;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java b/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java
index c7f4c4a..df818ac 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java
@@ -14,12 +14,10 @@
package com.google.gerrit.server;
-import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.CapabilityControl;
@@ -32,7 +30,6 @@
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.DisableReverseDnsLookup;
import com.google.gerrit.server.group.SystemGroupBackend;
-import com.google.gwtorm.server.ResultSet;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -58,7 +55,6 @@
@Singleton
public static class GenericFactory {
private final CapabilityControl.Factory capabilityControlFactory;
- private final StarredChangesUtil starredChangesUtil;
private final AuthConfig authConfig;
private final Realm realm;
private final String anonymousCowardName;
@@ -70,7 +66,6 @@
@Inject
public GenericFactory(
@Nullable CapabilityControl.Factory capabilityControlFactory,
- @Nullable StarredChangesUtil starredChangesUtil,
AuthConfig authConfig,
Realm realm,
@AnonymousCowardName String anonymousCowardName,
@@ -79,7 +74,6 @@
AccountCache accountCache,
GroupBackend groupBackend) {
this.capabilityControlFactory = capabilityControlFactory;
- this.starredChangesUtil = starredChangesUtil;
this.authConfig = authConfig;
this.realm = realm;
this.anonymousCowardName = anonymousCowardName;
@@ -99,10 +93,9 @@
public IdentifiedUser runAs(SocketAddress remotePeer, Account.Id id,
@Nullable CurrentUser caller) {
- return new IdentifiedUser(capabilityControlFactory, starredChangesUtil,
- authConfig, realm, anonymousCowardName, canonicalUrl, accountCache,
- groupBackend, disableReverseDnsLookup, Providers.of(remotePeer), id,
- caller);
+ return new IdentifiedUser(capabilityControlFactory, authConfig, realm,
+ anonymousCowardName, canonicalUrl, accountCache, groupBackend,
+ disableReverseDnsLookup, Providers.of(remotePeer), id, caller);
}
}
@@ -115,7 +108,6 @@
@Singleton
public static class RequestFactory {
private final CapabilityControl.Factory capabilityControlFactory;
- private final StarredChangesUtil starredChangesUtil;
private final AuthConfig authConfig;
private final Realm realm;
private final String anonymousCowardName;
@@ -128,7 +120,6 @@
@Inject
RequestFactory(
CapabilityControl.Factory capabilityControlFactory,
- @Nullable StarredChangesUtil starredChangesUtil,
AuthConfig authConfig,
Realm realm,
@AnonymousCowardName String anonymousCowardName,
@@ -138,7 +129,6 @@
@DisableReverseDnsLookup Boolean disableReverseDnsLookup,
@RemotePeer Provider<SocketAddress> remotePeerProvider) {
this.capabilityControlFactory = capabilityControlFactory;
- this.starredChangesUtil = starredChangesUtil;
this.authConfig = authConfig;
this.realm = realm;
this.anonymousCowardName = anonymousCowardName;
@@ -150,16 +140,15 @@
}
public IdentifiedUser create(Account.Id id) {
- return new IdentifiedUser(capabilityControlFactory, starredChangesUtil,
- authConfig, realm, anonymousCowardName, canonicalUrl, accountCache,
- groupBackend, disableReverseDnsLookup, remotePeerProvider, id, null);
+ return new IdentifiedUser(capabilityControlFactory, authConfig, realm,
+ anonymousCowardName, canonicalUrl, accountCache, groupBackend,
+ disableReverseDnsLookup, remotePeerProvider, id, null);
}
public IdentifiedUser runAs(Account.Id id, CurrentUser caller) {
- return new IdentifiedUser(capabilityControlFactory, starredChangesUtil,
- authConfig, realm, anonymousCowardName, canonicalUrl, accountCache,
- groupBackend, disableReverseDnsLookup, remotePeerProvider, id,
- caller);
+ return new IdentifiedUser(capabilityControlFactory, authConfig, realm,
+ anonymousCowardName, canonicalUrl, accountCache, groupBackend,
+ disableReverseDnsLookup, remotePeerProvider, id, caller);
}
}
@@ -168,9 +157,6 @@
SystemGroupBackend.ANONYMOUS_USERS,
SystemGroupBackend.REGISTERED_USERS));
- @Nullable
- private final StarredChangesUtil starredChangesUtil;
-
private final Provider<String> canonicalUrl;
private final AccountCache accountCache;
private final AuthConfig authConfig;
@@ -188,14 +174,11 @@
private boolean loadedAllEmails;
private Set<String> invalidEmails;
private GroupMembership effectiveGroups;
- private Set<Change.Id> starredChanges;
- private ResultSet<Change.Id> starredQuery;
private CurrentUser realUser;
private Map<PropertyKey<Object>, Object> properties;
private IdentifiedUser(
CapabilityControl.Factory capabilityControlFactory,
- @Nullable StarredChangesUtil starredChangesUtil,
final AuthConfig authConfig,
Realm realm,
final String anonymousCowardName,
@@ -207,7 +190,6 @@
final Account.Id id,
@Nullable CurrentUser realUser) {
super(capabilityControlFactory);
- this.starredChangesUtil = starredChangesUtil;
this.canonicalUrl = canonicalUrl;
this.accountCache = accountCache;
this.groupBackend = groupBackend;
@@ -295,53 +277,6 @@
return effectiveGroups;
}
- @SuppressWarnings("deprecation")
- @Override
- public Set<Change.Id> getStarredChanges() {
- if (starredChanges == null) {
- if (starredChangesUtil == null) {
- throw new IllegalStateException("StarredChangesUtil is missing");
- }
- try {
- starredChanges =
- FluentIterable.from(
- starredQuery != null
- ? starredQuery
- : starredChangesUtil.queryFromIndex(accountId))
- .toSet();
- } finally {
- starredQuery = null;
- }
- }
- return starredChanges;
- }
-
- @Deprecated
- public void clearStarredChanges() {
- // Async query may have started before an update that the caller expects
- // to see the results of, so we can't trust it.
- abortStarredChanges();
- starredChanges = null;
- }
-
- @Deprecated
- public void asyncStarredChanges() {
- if (starredChanges == null && starredChangesUtil != null) {
- starredQuery = starredChangesUtil.queryFromIndex(accountId);
- }
- }
-
- @Deprecated
- public void abortStarredChanges() {
- if (starredQuery != null) {
- try {
- starredQuery.close();
- } finally {
- starredQuery = null;
- }
- }
- }
-
public PersonIdent newRefLogIdent() {
return newRefLogIdent(new Date(), TimeZone.getDefault());
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/InternalUser.java b/gerrit-server/src/main/java/com/google/gerrit/server/InternalUser.java
index 3c63bf8..02d41f4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/InternalUser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/InternalUser.java
@@ -15,14 +15,10 @@
package com.google.gerrit.server;
import com.google.common.annotations.VisibleForTesting;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupMembership;
import com.google.inject.Inject;
-import java.util.Collections;
-import java.util.Set;
-
/**
* User identity for plugin code that needs an identity.
* <p>
@@ -50,11 +46,6 @@
}
@Override
- public Set<Change.Id> getStarredChanges() {
- return Collections.emptySet();
- }
-
- @Override
public boolean isInternalUser() {
return true;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/PeerDaemonUser.java b/gerrit-server/src/main/java/com/google/gerrit/server/PeerDaemonUser.java
index ea3080d..6616a66 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/PeerDaemonUser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/PeerDaemonUser.java
@@ -14,15 +14,12 @@
package com.google.gerrit.server;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupMembership;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.net.SocketAddress;
-import java.util.Collections;
-import java.util.Set;
/** Identity of a peer daemon process that isn't this JVM. */
public class PeerDaemonUser extends CurrentUser {
@@ -47,11 +44,6 @@
return GroupMembership.EMPTY;
}
- @Override
- public Set<Change.Id> getStarredChanges() {
- return Collections.emptySet();
- }
-
public SocketAddress getRemoteAddress() {
return peer;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
index 5e5e43c..5a89afa 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/StarredChangesUtil.java
@@ -40,9 +40,7 @@
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
-import com.google.gwtorm.server.ListResultSet;
import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.ResultSet;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -65,7 +63,6 @@
import org.slf4j.LoggerFactory;
import java.io.IOException;
-import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.SortedSet;
@@ -282,6 +279,47 @@
}
}
+ @Deprecated
+ // To be used only for IsStarredByLegacyPredicate.
+ public Set<Change.Id> byAccount(final Account.Id accountId,
+ final String label) throws OrmException {
+ try (final Repository repo = repoManager.openRepository(allUsers)) {
+ return FluentIterable
+ .from(getRefNames(repo, RefNames.REFS_STARRED_CHANGES))
+ .filter(new Predicate<String>() {
+ @Override
+ public boolean apply(String refPart) {
+ return refPart.endsWith("/" + accountId.get());
+ }
+ })
+ .transform(new Function<String, Change.Id>() {
+ @Override
+ public Change.Id apply(String refPart) {
+ return Change.Id.fromRefPart(refPart);
+ }
+ })
+ .filter(new Predicate<Change.Id>() {
+ @Override
+ public boolean apply(Change.Id changeId) {
+ try {
+ return readLabels(repo,
+ RefNames.refsStarredChanges(changeId, accountId))
+ .contains(label);
+ } catch (IOException e) {
+ log.error(String.format(
+ "Cannot query stars by account %d on change %d",
+ accountId.get(), changeId.get()), e);
+ return false;
+ }
+ }
+ }).toSet();
+ } catch (IOException e) {
+ throw new OrmException(
+ String.format("Get changes that were starred by %d failed",
+ accountId.get()), e);
+ }
+ }
+
public ImmutableMultimap<Account.Id, String> byChangeFromIndex(
Change.Id changeId) throws OrmException, NoSuchChangeException {
Set<String> fields = ImmutableSet.of(
@@ -295,29 +333,6 @@
return changeData.get(0).stars();
}
- @Deprecated
- public ResultSet<Change.Id> queryFromIndex(final Account.Id accountId) {
- try {
- Set<String> fields = ImmutableSet.of(
- ChangeField.ID.getName());
- List<ChangeData> changeData =
- queryProvider.get().setRequestedFields(fields).byIsStarred(accountId);
- return new ListResultSet<>(FluentIterable
- .from(changeData)
- .transform(new Function<ChangeData, Change.Id>() {
- @Override
- public Change.Id apply(ChangeData cd) {
- return cd.getId();
- }
- }).toList());
- } catch (OrmException | RuntimeException e) {
- log.warn(String.format("Cannot query starred changes for account %d",
- accountId.get()), e);
- List<Change.Id> empty = Collections.emptyList();
- return new ListResultSet<>(empty);
- }
- }
-
private static Set<String> getRefNames(Repository repo, String prefix)
throws IOException {
RefDatabase refDb = repo.getRefDatabase();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index f143bd6..fd205991 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -33,7 +33,6 @@
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.change.Abandon;
import com.google.gerrit.server.change.ChangeEdits;
import com.google.gerrit.server.change.ChangeJson;
@@ -73,7 +72,6 @@
ChangeApiImpl create(ChangeResource change);
}
- private final CurrentUser user;
private final Changes changeApi;
private final Reviewers reviewers;
private final Revisions revisions;
@@ -102,8 +100,7 @@
private final Move move;
@Inject
- ChangeApiImpl(CurrentUser user,
- Changes changeApi,
+ ChangeApiImpl(Changes changeApi,
Reviewers reviewers,
Revisions revisions,
ReviewerApiImpl.Factory reviewerApi,
@@ -128,7 +125,6 @@
ChangeEdits.Detail editDetail,
Move move,
@Assisted ChangeResource change) {
- this.user = user;
this.changeApi = changeApi;
this.revert = revert;
this.reviewers = reviewers;
@@ -336,14 +332,10 @@
}
}
- @SuppressWarnings("deprecation")
@Override
public ChangeInfo get(EnumSet<ListChangesOption> s)
throws RestApiException {
try {
- if (user.isIdentifiedUser()) {
- user.asIdentifiedUser().clearStarredChanges();
- }
return changeJson.create(s).format(change);
} catch (OrmException e) {
throw new RestApiException("Cannot retrieve change", e);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java
index f4c671f..bb2eea7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangesImpl.java
@@ -30,7 +30,6 @@
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.CreateChange;
import com.google.gerrit.server.git.UpdateException;
@@ -46,19 +45,16 @@
@Singleton
class ChangesImpl implements Changes {
- private final Provider<CurrentUser> user;
private final ChangesCollection changes;
private final ChangeApiImpl.Factory api;
private final CreateChange createChange;
private final Provider<QueryChanges> queryProvider;
@Inject
- ChangesImpl(Provider<CurrentUser> user,
- ChangesCollection changes,
+ ChangesImpl(ChangesCollection changes,
ChangeApiImpl.Factory api,
CreateChange createChange,
Provider<QueryChanges> queryProvider) {
- this.user = user;
this.changes = changes;
this.api = api;
this.createChange = createChange;
@@ -117,7 +113,6 @@
return query().withQuery(query);
}
- @SuppressWarnings("deprecation")
private List<ChangeInfo> get(final QueryRequest q) throws RestApiException {
QueryChanges qc = queryProvider.get();
if (q.getQuery() != null) {
@@ -130,10 +125,6 @@
}
try {
- CurrentUser u = user.get();
- if (u.isIdentifiedUser()) {
- u.asIdentifiedUser().clearStarredChanges();
- }
List<?> result = qc.apply(TopLevelResource.INSTANCE);
if (result.isEmpty()) {
return ImmutableList.of();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 590be32..38f0da9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -181,6 +181,7 @@
final ChangeIndex index;
final IndexConfig indexConfig;
final Provider<ListMembers> listMembers;
+ final StarredChangesUtil starredChangesUtil;
final boolean allowsDrafts;
private final Provider<CurrentUser> self;
@@ -213,6 +214,7 @@
TrackingFooters trackingFooters,
IndexConfig indexConfig,
Provider<ListMembers> listMembers,
+ StarredChangesUtil starredChangesUtil,
@GerritServerConfig Config cfg) {
this(db, queryProvider, rewriter, opFactories, userFactory, self,
capabilityControlFactory, changeControlGenericFactory, notesFactory,
@@ -220,7 +222,7 @@
allProjectsName, allUsersName, patchListCache, repoManager,
projectCache, listChildProjects, submitDryRun, conflictsCache,
trackingFooters, indexes != null ? indexes.getSearchIndex() : null,
- indexConfig, listMembers,
+ indexConfig, listMembers, starredChangesUtil,
cfg == null ? true : cfg.getBoolean("change", "allowDrafts", true));
}
@@ -251,6 +253,7 @@
ChangeIndex index,
IndexConfig indexConfig,
Provider<ListMembers> listMembers,
+ StarredChangesUtil starredChangesUtil,
boolean allowsDrafts) {
this.db = db;
this.queryProvider = queryProvider;
@@ -278,6 +281,7 @@
this.index = index;
this.indexConfig = indexConfig;
this.listMembers = listMembers;
+ this.starredChangesUtil = starredChangesUtil;
this.allowsDrafts = allowsDrafts;
}
@@ -289,7 +293,7 @@
allProjectsName, allUsersName, patchListCache, repoManager,
projectCache, listChildProjects, submitDryRun,
conflictsCache, trackingFooters, index, indexConfig, listMembers,
- allowsDrafts);
+ starredChangesUtil, allowsDrafts);
}
Arguments asUser(Account.Id otherId) {
@@ -678,9 +682,18 @@
return new StarPredicate(who, StarredChangesUtil.DEFAULT_LABEL);
}
- return args.getSchema().hasField(ChangeField.STARREDBY)
- ? new IsStarredByPredicate(who)
- : new IsStarredByLegacyPredicate(args.asUser(who));
+ if (args.getSchema().hasField(ChangeField.STARREDBY)) {
+ return new IsStarredByPredicate(who);
+ }
+
+ try {
+ // starred changes are not contained in the index, we must read them from
+ // git
+ return new IsStarredByLegacyPredicate(who, args.starredChangesUtil
+ .byAccount(who, StarredChangesUtil.DEFAULT_LABEL));
+ } catch (OrmException e) {
+ throw new QueryParseException("Failed to query starred changes.", e);
+ }
}
@Operator
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByLegacyPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByLegacyPredicate.java
index 718c3f6..19cbd23 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByLegacyPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByLegacyPredicate.java
@@ -15,25 +15,16 @@
package com.google.gerrit.server.query.change;
import com.google.common.collect.Lists;
+import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.query.OrPredicate;
import com.google.gerrit.server.query.Predicate;
-import com.google.gerrit.server.query.QueryParseException;
-import com.google.gerrit.server.query.change.ChangeQueryBuilder.Arguments;
import java.util.List;
import java.util.Set;
@Deprecated
class IsStarredByLegacyPredicate extends OrPredicate<ChangeData> {
- private static String describe(CurrentUser user) {
- if (user.isIdentifiedUser()) {
- return user.getAccountId().toString();
- }
- return user.toString();
- }
-
private static List<Predicate<ChangeData>> predicates(Set<Change.Id> ids) {
List<Predicate<ChangeData>> r = Lists.newArrayListWithCapacity(ids.size());
for (Change.Id id : ids) {
@@ -42,16 +33,19 @@
return r;
}
- private final CurrentUser user;
+ private final Account.Id accountId;
+ private final Set<Change.Id> starredChanges;
- IsStarredByLegacyPredicate(Arguments args) throws QueryParseException {
- super(predicates(args.getIdentifiedUser().getStarredChanges()));
- this.user = args.getIdentifiedUser();
+ IsStarredByLegacyPredicate(Account.Id accountId,
+ Set<Change.Id> starredChanges) {
+ super(predicates(starredChanges));
+ this.accountId = accountId;
+ this.starredChanges = starredChanges;
}
@Override
public boolean match(final ChangeData object) {
- return user.getStarredChanges().contains(object.getId());
+ return starredChanges.contains(object.getId());
}
@Override
@@ -61,11 +55,6 @@
@Override
public String toString() {
- String val = describe(user);
- if (val.indexOf(' ') < 0) {
- return ChangeQueryBuilder.FIELD_STARREDBY + ":" + val;
- } else {
- return ChangeQueryBuilder.FIELD_STARREDBY + ":\"" + val + "\"";
- }
+ return ChangeQueryBuilder.FIELD_STARREDBY + ":" + accountId.toString();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java
index 43fb0c5..e5ff46a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java
@@ -20,13 +20,10 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
-import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.query.QueryParseException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import org.kohsuke.args4j.Option;
@@ -41,7 +38,6 @@
private final ChangeJson.Factory json;
private final ChangeQueryBuilder qb;
private final QueryProcessor imp;
- private final Provider<CurrentUser> user;
private EnumSet<ListChangesOption> options;
@Option(name = "--query", aliases = {"-q"}, metaVar = "QUERY", usage = "Query string")
@@ -70,12 +66,10 @@
@Inject
QueryChanges(ChangeJson.Factory json,
ChangeQueryBuilder qb,
- QueryProcessor qp,
- Provider<CurrentUser> user) {
+ QueryProcessor qp) {
this.json = json;
this.qb = qb;
this.imp = qp;
- this.user = user;
options = EnumSet.noneOf(ListChangesOption.class);
}
@@ -111,7 +105,6 @@
return out.size() == 1 ? out.get(0) : out;
}
- @SuppressWarnings("deprecation")
private List<List<ChangeInfo>> query()
throws OrmException, QueryParseException {
if (imp.isDisabled()) {
@@ -125,22 +118,6 @@
throw new QueryParseException("limit of 10 queries");
}
- IdentifiedUser self = null;
- try {
- if (user.get().isIdentifiedUser()) {
- self = user.get().asIdentifiedUser();
- self.asyncStarredChanges();
- }
- return query0();
- } finally {
- if (self != null) {
- self.abortStarredChanges();
- }
- }
- }
-
- private List<List<ChangeInfo>> query0() throws OrmException,
- QueryParseException {
int cnt = queries.size();
List<QueryResult> results = imp.queryChanges(qb.parse(queries));
List<List<ChangeInfo>> res = json.create(options)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SingleGroupUser.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SingleGroupUser.java
index ca7c990..33b338c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SingleGroupUser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/SingleGroupUser.java
@@ -15,7 +15,6 @@
package com.google.gerrit.server.query.change;
import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupMembership;
@@ -42,9 +41,4 @@
public GroupMembership getEffectiveGroups() {
return groups;
}
-
- @Override
- public Set<Change.Id> getStarredChanges() {
- return Collections.emptySet();
- }
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/IdentifiedUserTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/IdentifiedUserTest.java
index 390a4c1..0d2de399 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/IdentifiedUserTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/IdentifiedUserTest.java
@@ -95,8 +95,6 @@
bind(CapabilityControl.Factory.class)
.toProvider(Providers.<CapabilityControl.Factory>of(null));
bind(Realm.class).toInstance(mockRealm);
- bind(StarredChangesUtil.class)
- .toProvider(Providers.<StarredChangesUtil> of(null));
}
};
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/index/change/FakeQueryBuilder.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/change/FakeQueryBuilder.java
index e88962e..5316bcb 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/index/change/FakeQueryBuilder.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/change/FakeQueryBuilder.java
@@ -30,7 +30,7 @@
FakeQueryBuilder.class),
new ChangeQueryBuilder.Arguments(null, null, null, null, null, null,
null, null, null, null, null, null, null, null, null, null, null,
- null, null, null, indexes, null, null, null, null, null, null));
+ null, null, null, indexes, null, null, null, null, null, null, null));
}
@Operator
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
index 5a11bfa..68c79a0 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
@@ -36,7 +36,6 @@
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.InternalUser;
-import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.FakeRealm;
@@ -169,8 +168,6 @@
.toInstance(serverIdent);
bind(GitReferenceUpdated.class)
.toInstance(GitReferenceUpdated.DISABLED);
- bind(StarredChangesUtil.class)
- .toProvider(Providers.<StarredChangesUtil> of(null));
bind(MetricMaker.class).to(DisabledMetricMaker.class);
bind(ReviewDb.class).toProvider(Providers.<ReviewDb> of(null));
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
index 6f24a4b..3ab2ff6 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
@@ -41,7 +41,6 @@
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.common.errors.InvalidNameException;
import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.rules.PrologEnvironment;
@@ -907,10 +906,5 @@
public String getUserName() {
return username;
}
-
- @Override
- public Set<Change.Id> getStarredChanges() {
- return Collections.emptySet();
- }
}
}
diff --git a/plugins/replication b/plugins/replication
index b3ab82d..5f91046 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit b3ab82de95bedd46a60152e2ecffdab1f762e00d
+Subproject commit 5f91046e32136bf4b58031c4b6d86e26389b8421
