CurrentUser.PropertyKey<T>: allow plugins to cache data on users
This new map inside of both IdentifiedUser and AccountState allows
plugins to store their own information within a single user instance,
or the cached AccountState.
A property cache is primarily useful for an expensive AvatarProvider
that may need to lookup data based on AccountExternalIds on the
AccountState or the IdentifiedUser and reuse this for multiple
getUrl() invocations of varying image sizes.
Plugins can use it like this:
private static final CurrentUser.PropertyKey<String> imageUrl =
CurrentUser.PropertyKey.create();
public String getUrl(IdentifiedUser user, int size) {
String url = user.get(imageUrl);
if (url == null) {
url = "http://image.host/" + userId + "?size=";
user.set(imageUrl, url);
}
return url + size;
}
Begin using this for the notification filters, which are only used
inside of IsWatchedByPredicate, a non-common predicate. This gets
the database code out of IdentifiedUser and isolates it down to the
sole user in the query system.
Change-Id: Ica7409411eaf60af8d554c33b873c930b9abea6f
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 ff09df5..e916aff 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.AccountProjectWatch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupMembership;
@@ -22,7 +21,6 @@
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.inject.Inject;
-import java.util.Collection;
import java.util.Collections;
import java.util.Set;
@@ -44,11 +42,6 @@
}
@Override
- public Collection<AccountProjectWatch> getNotificationFilters() {
- 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 f1242b9..16e868f 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
@@ -14,14 +14,13 @@
package com.google.gerrit.server;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.AccountProjectWatch;
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.Collection;
import java.util.Set;
/**
@@ -33,6 +32,16 @@
* @see IdentifiedUser
*/
public abstract class CurrentUser {
+ /** Unique key for plugin/extension specific data on a CurrentUser. */
+ public static final class PropertyKey<T> {
+ public static <T> PropertyKey<T> create() {
+ return new PropertyKey<>();
+ }
+
+ private PropertyKey() {
+ }
+ }
+
private final CapabilityControl.Factory capabilityControlFactory;
private AccessPath accessPath = AccessPath.UNKNOWN;
@@ -82,9 +91,6 @@
@Deprecated
public abstract Set<Change.Id> getStarredChanges();
- /** Filters selecting changes the user wants to monitor. */
- public abstract Collection<AccountProjectWatch> getNotificationFilters();
-
/** Unique name of the user on this server, if one has been assigned. */
public String getUserName() {
return null;
@@ -119,4 +125,24 @@
public boolean isInternalUser() {
return false;
}
+
+ /**
+ * Lookup a previously stored property.
+ *
+ * @param key unique property key.
+ * @return previously stored value, or {@code null}.
+ */
+ @Nullable
+ public <T> T get(PropertyKey<T> key) {
+ return null;
+ }
+
+ /**
+ * Store a property for later retrieval.
+ *
+ * @param key unique property key.
+ * @param value value to store; or {@code null} to clear the value.
+ */
+ public <T> void put(PropertyKey<T> key, @Nullable T value) {
+ }
}
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 f6fae8c..0493df3 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
@@ -19,7 +19,6 @@
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.AccountProjectWatch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountCache;
@@ -34,28 +33,23 @@
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.OrmException;
import com.google.gwtorm.server.ResultSet;
import com.google.inject.Inject;
-import com.google.inject.OutOfScopeException;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import com.google.inject.util.Providers;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.util.SystemReader;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.MalformedURLException;
import java.net.SocketAddress;
import java.net.URL;
-import java.util.Collection;
-import java.util.Collections;
import java.util.Date;
-import java.util.List;
+import java.util.HashMap;
+import java.util.Map;
import java.util.Set;
import java.util.TimeZone;
@@ -184,9 +178,6 @@
}
}
- private static final Logger log =
- LoggerFactory.getLogger(IdentifiedUser.class);
-
private static final GroupMembership registeredGroups =
new ListGroupMembership(ImmutableSet.of(
SystemGroupBackend.ANONYMOUS_USERS,
@@ -219,8 +210,8 @@
private GroupMembership effectiveGroups;
private Set<Change.Id> starredChanges;
private ResultSet<Change.Id> starredQuery;
- private Collection<AccountProjectWatch> notificationFilters;
private CurrentUser realUser;
+ private Map<PropertyKey<Object>, Object> properties;
private IdentifiedUser(
CapabilityControl.Factory capabilityControlFactory,
@@ -256,7 +247,6 @@
return realUser;
}
- // TODO(cranger): maybe get the state through the accountCache instead.
public AccountState state() {
if (state == null) {
state = accountCache.get(getAccountId());
@@ -374,29 +364,6 @@
}
}
- private void checkRequestScope() {
- if (dbProvider == null) {
- throw new OutOfScopeException("Not in request scoped user");
- }
- }
-
- @Override
- public Collection<AccountProjectWatch> getNotificationFilters() {
- if (notificationFilters == null) {
- checkRequestScope();
- List<AccountProjectWatch> r;
- try {
- r = dbProvider.get().accountProjectWatches() //
- .byAccount(getAccountId()).toList();
- } catch (OrmException e) {
- log.warn("Cannot query notification filters of a user", e);
- r = Collections.emptyList();
- }
- notificationFilters = Collections.unmodifiableList(r);
- }
- return notificationFilters;
- }
-
public PersonIdent newRefLogIdent() {
return newRefLogIdent(new Date(), TimeZone.getDefault());
}
@@ -487,6 +454,41 @@
return true;
}
+ @Override
+ @Nullable
+ public synchronized <T> T get(PropertyKey<T> key) {
+ if (properties != null) {
+ @SuppressWarnings("unchecked")
+ T value = (T) properties.get(key);
+ return value;
+ }
+ return null;
+ }
+
+ /**
+ * Store a property for later retrieval.
+ *
+ * @param key unique property key.
+ * @param value value to store; or {@code null} to clear the value.
+ */
+ @Override
+ public synchronized <T> void put(PropertyKey<T> key, @Nullable T value) {
+ if (properties == null) {
+ if (value == null) {
+ return;
+ }
+ properties = new HashMap<>();
+ }
+
+ @SuppressWarnings("unchecked")
+ PropertyKey<Object> k = (PropertyKey<Object>) key;
+ if (value != null) {
+ properties.put(k, value);
+ } else {
+ properties.remove(k);
+ }
+ }
+
private String getHost(final InetAddress in) {
if (Boolean.FALSE.equals(disableReverseDnsLookup)) {
return in.getCanonicalHostName();
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 d0c2dc0..3c63bf8 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,13 +15,11 @@
package com.google.gerrit.server;
import com.google.common.annotations.VisibleForTesting;
-import com.google.gerrit.reviewdb.client.AccountProjectWatch;
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.Collection;
import java.util.Collections;
import java.util.Set;
@@ -57,11 +55,6 @@
}
@Override
- public Collection<AccountProjectWatch> getNotificationFilters() {
- 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 4d26f02..ea3080d 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,7 +14,6 @@
package com.google.gerrit.server;
-import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupMembership;
@@ -22,7 +21,6 @@
import com.google.inject.assistedinject.Assisted;
import java.net.SocketAddress;
-import java.util.Collection;
import java.util.Collections;
import java.util.Set;
@@ -54,11 +52,6 @@
return Collections.emptySet();
}
- @Override
- public Collection<AccountProjectWatch> getNotificationFilters() {
- return Collections.emptySet();
- }
-
public SocketAddress getRemoteAddress() {
return peer;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountState.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountState.java
index 815b519..37e36cd 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountState.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountState.java
@@ -16,9 +16,14 @@
import static com.google.gerrit.reviewdb.client.AccountExternalId.SCHEME_USERNAME;
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.server.CurrentUser.PropertyKey;
+import com.google.gerrit.server.IdentifiedUser;
import java.util.Collection;
import java.util.Set;
@@ -27,6 +32,7 @@
private final Account account;
private final Set<AccountGroup.UUID> internalGroups;
private final Collection<AccountExternalId> externalIds;
+ private Cache<IdentifiedUser.PropertyKey<Object>, Object> properties;
public AccountState(final Account account,
final Set<AccountGroup.UUID> actualGroups,
@@ -81,4 +87,59 @@
}
return null;
}
+
+ /**
+ * Lookup a previously stored property.
+ * <p>
+ * All properties are automatically cleared when the account cache invalidates
+ * the {@code AccountState}. This method is thread-safe.
+ *
+ * @param key unique property key.
+ * @return previously stored value, or {@code null}.
+ */
+ @Nullable
+ public <T> T get(PropertyKey<T> key) {
+ Cache<PropertyKey<Object>, Object> p = properties(false);
+ if (p != null) {
+ @SuppressWarnings("unchecked")
+ T value = (T) p.getIfPresent(key);
+ return value;
+ }
+ return null;
+ }
+
+ /**
+ * Store a property for later retrieval.
+ * <p>
+ * This method is thread-safe.
+ *
+ * @param key unique property key.
+ * @param value value to store; or {@code null} to clear the value.
+ */
+ public <T> void put(PropertyKey<T> key, @Nullable T value) {
+ Cache<PropertyKey<Object>, Object> p = properties(value != null);
+ if (p != null || value != null) {
+ @SuppressWarnings("unchecked")
+ PropertyKey<Object> k = (PropertyKey<Object>) key;
+ if (value != null) {
+ p.put(k, value);
+ } else {
+ p.invalidate(k);
+ }
+ }
+ }
+
+ private synchronized Cache<PropertyKey<Object>, Object> properties(
+ boolean allocate) {
+ if (properties == null && allocate) {
+ properties = CacheBuilder.newBuilder()
+ .concurrencyLevel(1)
+ .initialCapacity(16)
+ // Use weakKeys to ensure plugins that garbage collect will also
+ // eventually release data held in any still live AccountState.
+ .weakKeys()
+ .build();
+ }
+ return properties;
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsWatchedByPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsWatchedByPredicate.java
index bb5f8ff..4a8e71b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsWatchedByPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsWatchedByPredicate.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.query.change;
+import com.google.common.base.MoreObjects;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.server.CurrentUser;
@@ -21,11 +22,22 @@
import com.google.gerrit.server.query.Predicate;
import com.google.gerrit.server.query.QueryBuilder;
import com.google.gerrit.server.query.QueryParseException;
+import com.google.gwtorm.server.OrmException;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.List;
class IsWatchedByPredicate extends AndPredicate<ChangeData> {
+ private static final Logger log =
+ LoggerFactory.getLogger(IsWatchedByPredicate.class);
+
+ private static final CurrentUser.PropertyKey<List<AccountProjectWatch>> PROJECT_WATCHES =
+ CurrentUser.PropertyKey.create();
+
private static String describe(CurrentUser user) {
if (user.isIdentifiedUser()) {
return user.getAccountId().toString();
@@ -44,10 +56,9 @@
private static List<Predicate<ChangeData>> filters(
ChangeQueryBuilder.Arguments args,
boolean checkIsVisible) throws QueryParseException {
- CurrentUser user = args.getUser();
List<Predicate<ChangeData>> r = new ArrayList<>();
ChangeQueryBuilder builder = new ChangeQueryBuilder(args);
- for (AccountProjectWatch w : user.getNotificationFilters()) {
+ for (AccountProjectWatch w : getWatches(args)) {
Predicate<ChangeData> f = null;
if (w.getFilter() != null) {
try {
@@ -90,6 +101,24 @@
}
}
+ private static List<AccountProjectWatch> getWatches(
+ ChangeQueryBuilder.Arguments args) throws QueryParseException {
+ CurrentUser user = args.getUser();
+ List<AccountProjectWatch> watches = user.get(PROJECT_WATCHES);
+ if (watches == null && user.isIdentifiedUser()) {
+ try {
+ watches = args.db.get().accountProjectWatches()
+ .byAccount(user.asIdentifiedUser().getAccountId()).toList();
+ user.put(PROJECT_WATCHES, watches);
+ } catch (OrmException e) {
+ log.warn("Cannot load accountProjectWatches", e);
+ }
+ }
+ return MoreObjects.firstNonNull(
+ watches,
+ Collections.<AccountProjectWatch> emptyList());
+ }
+
private static List<Predicate<ChangeData>> none() {
Predicate<ChangeData> any = any();
return ImmutableList.of(not(any));
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 4b71719..ca7c990 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,14 +15,12 @@
package com.google.gerrit.server.query.change;
import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.AccountProjectWatch;
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;
import com.google.gerrit.server.account.ListGroupMembership;
-import java.util.Collection;
import java.util.Collections;
import java.util.Set;
@@ -49,9 +47,4 @@
public Set<Change.Id> getStarredChanges() {
return Collections.emptySet();
}
-
- @Override
- public Collection<AccountProjectWatch> getNotificationFilters() {
- return Collections.emptySet();
- }
}
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 e8c62e93..6f24a4b 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.AccountProjectWatch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -76,7 +75,6 @@
import java.io.IOException;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@@ -914,10 +912,5 @@
public Set<Change.Id> getStarredChanges() {
return Collections.emptySet();
}
-
- @Override
- public Collection<AccountProjectWatch> getNotificationFilters() {
- return Collections.emptySet();
- }
}
}
diff --git a/plugins/replication b/plugins/replication
index 4883981..945c842 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 4883981228e871f9b58c01197c01d93ded6250d2
+Subproject commit 945c842f9c884469ec0fb2d883d6cda552e97747
