diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java index 2b3fe11ee3..af0ee4cf30 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java @@ -24,6 +24,7 @@ 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.reviewdb.client.AccountGroupMember; +import com.google.gerrit.reviewdb.client.AccountProjectWatch; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.cache.CacheModule; import com.google.gerrit.server.index.account.AccountIndexer; @@ -132,8 +133,9 @@ public class AccountCacheImpl implements AccountCache { Account account = new Account(accountId, TimeUtil.nowTs()); account.setActive(false); Collection ids = Collections.emptySet(); + Collection projectWatches = Collections.emptySet(); Set anon = ImmutableSet.of(); - return new AccountState(account, anon, ids); + return new AccountState(account, anon, ids, projectWatches); } static class ByIdLoader extends CacheLoader { @@ -168,16 +170,15 @@ public class AccountCacheImpl implements AccountCache { private AccountState load(final ReviewDb db, final Account.Id who) throws OrmException { - final Account account = db.accounts().get(who); + Account account = db.accounts().get(who); if (account == null) { // Account no longer exists? They are anonymous. - // return missing(who); } - final Collection externalIds = - Collections.unmodifiableCollection(db.accountExternalIds().byAccount( - who).toList()); + Collection externalIds = + Collections.unmodifiableCollection( + db.accountExternalIds().byAccount(who).toList()); Set internalGroups = new HashSet<>(); for (AccountGroupMember g : db.accountGroupMembers().byAccount(who)) { @@ -197,7 +198,12 @@ public class AccountCacheImpl implements AccountCache { account.setGeneralPreferences(GeneralPreferencesInfo.defaults()); } - return new AccountState(account, internalGroups, externalIds); + Collection projectWatches = + Collections.unmodifiableCollection( + db.accountProjectWatches().byAccount(who).toList()); + + return new AccountState(account, internalGroups, externalIds, + projectWatches); } } 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 a18a6b2d27..145f34b761 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 @@ -23,6 +23,7 @@ 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.reviewdb.client.AccountProjectWatch; import com.google.gerrit.server.CurrentUser.PropertyKey; import com.google.gerrit.server.IdentifiedUser; @@ -34,14 +35,17 @@ public class AccountState { private final Account account; private final Set internalGroups; private final Collection externalIds; + private final Collection projectWatches; private Cache, Object> properties; - public AccountState(final Account account, - final Set actualGroups, - final Collection externalIds) { + public AccountState(Account account, + Set actualGroups, + Collection externalIds, + Collection projectWatches) { this.account = account; this.internalGroups = actualGroups; this.externalIds = externalIds; + this.projectWatches = projectWatches; this.account.setUserName(getUserName(externalIds)); } @@ -76,6 +80,11 @@ public class AccountState { return externalIds; } + /** The project watches of the account. */ + public Collection getProjectWatches() { + return projectWatches; + } + /** The set of groups maintained directly within the Gerrit database. */ public Set getInternalGroups() { return internalGroups; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteWatchedProjects.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteWatchedProjects.java index 4a447fb9d7..43e984011e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteWatchedProjects.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteWatchedProjects.java @@ -19,6 +19,7 @@ import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.restapi.UnprocessableEntityException; +import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountProjectWatch; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; @@ -29,6 +30,7 @@ import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; +import java.io.IOException; import java.util.HashMap; import java.util.LinkedList; import java.util.List; @@ -39,26 +41,29 @@ public class DeleteWatchedProjects private final Provider dbProvider; private final Provider self; + private final AccountCache accountCache; @Inject DeleteWatchedProjects(Provider dbProvider, - Provider self) { + Provider self, + AccountCache accountCache) { this.dbProvider = dbProvider; this.self = self; + this.accountCache = accountCache; } @Override - public Response apply( - AccountResource rsrc, List input) - throws UnprocessableEntityException, OrmException, AuthException { - if (self.get() != rsrc.getUser() + public Response apply(AccountResource rsrc, List input) + throws AuthException, UnprocessableEntityException, OrmException, + IOException { + if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canAdministrateServer()) { throw new AuthException("It is not allowed to edit project watches " + "of other users"); } + Account.Id accountId = rsrc.getUser().getAccountId(); ResultSet watchedProjects = - dbProvider.get().accountProjectWatches() - .byAccount(rsrc.getUser().getAccountId()); + dbProvider.get().accountProjectWatches().byAccount(accountId); HashMap watchedProjectsMap = new HashMap<>(); for (AccountProjectWatch watchedProject : watchedProjects) { @@ -68,10 +73,8 @@ public class DeleteWatchedProjects if (input != null) { List watchesToDelete = new LinkedList<>(); for (ProjectWatchInfo projectInfo : input) { - AccountProjectWatch.Key key = new AccountProjectWatch.Key( - rsrc.getUser().getAccountId(), - new Project.NameKey(projectInfo.project), - projectInfo.filter); + AccountProjectWatch.Key key = new AccountProjectWatch.Key(accountId, + new Project.NameKey(projectInfo.project), projectInfo.filter); if (!watchedProjectsMap.containsKey(key)) { throw new UnprocessableEntityException(projectInfo.project + " is not currently watched by this user."); @@ -79,6 +82,7 @@ public class DeleteWatchedProjects watchesToDelete.add(watchedProjectsMap.get(key)); } dbProvider.get().accountProjectWatches().delete(watchesToDelete); + accountCache.evict(accountId); } return Response.none(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PostWatchedProjects.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PostWatchedProjects.java index 29ded3bfbd..fcac2b4fde 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PostWatchedProjects.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PostWatchedProjects.java @@ -38,20 +38,23 @@ import java.util.List; @Singleton public class PostWatchedProjects implements RestModifyView> { + private final Provider dbProvider; private final Provider self; private final GetWatchedProjects getWatchedProjects; - private final Provider dbProvider; private final ProjectsCollection projectsCollection; + private final AccountCache accountCache; @Inject - public PostWatchedProjects(GetWatchedProjects getWatchedProjects, - Provider dbProvider, + public PostWatchedProjects(Provider dbProvider, + Provider self, + GetWatchedProjects getWatchedProjects, ProjectsCollection projectsCollection, - Provider self) { - this.getWatchedProjects = getWatchedProjects; + AccountCache accountCache) { this.dbProvider = dbProvider; - this.projectsCollection = projectsCollection; this.self = self; + this.getWatchedProjects = getWatchedProjects; + this.projectsCollection = projectsCollection; + this.accountCache = accountCache; } @Override @@ -62,9 +65,11 @@ public class PostWatchedProjects && !self.get().getCapabilities().canAdministrateServer()) { throw new AuthException("not allowed to edit project watches"); } + Account.Id accountId = rsrc.getUser().getAccountId(); List accountProjectWatchList = - getAccountProjectWatchList(input, rsrc.getUser().getAccountId()); + getAccountProjectWatchList(input, accountId); dbProvider.get().accountProjectWatches().upsert(accountProjectWatchList); + accountCache.evict(accountId); return getWatchedProjects.apply(rsrc); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java index 2038276b6d..c678a2b310 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java @@ -241,7 +241,7 @@ public class AccountApiImpl implements AccountApi { throws RestApiException { try { deleteWatchedProjects.apply(account, in); - } catch (OrmException e) { + } catch (OrmException | IOException e) { throw new RestApiException("Cannot delete watched projects", e); } } 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 fb0ec51bee..1e0bfbc41a 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 @@ -38,6 +38,7 @@ import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.PatchLineCommentsUtil; import com.google.gerrit.server.StarredChangesUtil; +import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.account.CapabilityControl; import com.google.gerrit.server.account.GroupBackend; @@ -184,6 +185,7 @@ public class ChangeQueryBuilder extends QueryBuilder { final IndexConfig indexConfig; final Provider listMembers; final StarredChangesUtil starredChangesUtil; + final AccountCache accountCache; final boolean allowsDrafts; private final Provider self; @@ -217,6 +219,7 @@ public class ChangeQueryBuilder extends QueryBuilder { IndexConfig indexConfig, Provider listMembers, StarredChangesUtil starredChangesUtil, + AccountCache accountCache, @GerritServerConfig Config cfg) { this(db, queryProvider, rewriter, opFactories, userFactory, self, capabilityControlFactory, changeControlGenericFactory, notesFactory, @@ -224,7 +227,7 @@ public class ChangeQueryBuilder extends QueryBuilder { allProjectsName, allUsersName, patchListCache, repoManager, projectCache, listChildProjects, submitDryRun, conflictsCache, trackingFooters, indexes != null ? indexes.getSearchIndex() : null, - indexConfig, listMembers, starredChangesUtil, + indexConfig, listMembers, starredChangesUtil, accountCache, cfg == null ? true : cfg.getBoolean("change", "allowDrafts", true)); } @@ -256,6 +259,7 @@ public class ChangeQueryBuilder extends QueryBuilder { IndexConfig indexConfig, Provider listMembers, StarredChangesUtil starredChangesUtil, + AccountCache accountCache, boolean allowsDrafts) { this.db = db; this.queryProvider = queryProvider; @@ -284,6 +288,7 @@ public class ChangeQueryBuilder extends QueryBuilder { this.indexConfig = indexConfig; this.listMembers = listMembers; this.starredChangesUtil = starredChangesUtil; + this.accountCache = accountCache; this.allowsDrafts = allowsDrafts; } @@ -295,7 +300,7 @@ public class ChangeQueryBuilder extends QueryBuilder { allProjectsName, allUsersName, patchListCache, repoManager, projectCache, listChildProjects, submitDryRun, conflictsCache, trackingFooters, index, indexConfig, listMembers, - starredChangesUtil, allowsDrafts); + starredChangesUtil, accountCache, allowsDrafts); } Arguments asUser(Account.Id otherId) { 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 c8512bfef7..0090cf462d 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,7 +14,6 @@ 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; @@ -22,22 +21,13 @@ import com.google.gerrit.server.query.AndPredicate; 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.Collection; import java.util.Collections; import java.util.List; class IsWatchedByPredicate extends AndPredicate { - private static final Logger log = - LoggerFactory.getLogger(IsWatchedByPredicate.class); - - private static final CurrentUser.PropertyKey> PROJECT_WATCHES = - CurrentUser.PropertyKey.create(); - private static String describe(CurrentUser user) { if (user.isIdentifiedUser()) { return user.getAccountId().toString(); @@ -101,22 +91,14 @@ class IsWatchedByPredicate extends AndPredicate { } } - private static List getWatches( + private static Collection getWatches( ChangeQueryBuilder.Arguments args) throws QueryParseException { CurrentUser user = args.getUser(); - List 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); - } + if (user.isIdentifiedUser()) { + return args.accountCache.get(args.getUser().getAccountId()) + .getProjectWatches(); } - return MoreObjects.firstNonNull( - watches, - Collections. emptyList()); + return Collections. emptySet(); } private static List> none() { 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 0b5ed32eff..545fd08446 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 @@ -29,7 +29,8 @@ public class FakeQueryBuilder extends ChangeQueryBuilder { 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, null, indexes, null, null, null, null, null, null, null, + null)); } @Operator diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/mail/FromAddressGeneratorProviderTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/mail/FromAddressGeneratorProviderTest.java index b96b780917..fab21abffd 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/mail/FromAddressGeneratorProviderTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/FromAddressGeneratorProviderTest.java @@ -25,6 +25,7 @@ import com.google.gerrit.common.TimeUtil; 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.reviewdb.client.AccountProjectWatch; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountState; @@ -298,6 +299,7 @@ public class FromAddressGeneratorProviderTest { account.setFullName(name); account.setPreferredEmail(email); return new AccountState(account, Collections. emptySet(), - Collections. emptySet()); + Collections. emptySet(), + Collections. emptySet()); } } diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/FakeAccountCache.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/FakeAccountCache.java index d9841a38de..ca322a7a16 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/FakeAccountCache.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/FakeAccountCache.java @@ -19,6 +19,7 @@ import com.google.gerrit.common.TimeUtil; 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.reviewdb.client.AccountProjectWatch; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountState; @@ -73,8 +74,8 @@ public class FakeAccountCache implements AccountCache { } private static AccountState newState(Account account) { - return new AccountState( - account, ImmutableSet. of(), - ImmutableSet. of()); + return new AccountState(account, ImmutableSet. of(), + ImmutableSet. of(), + ImmutableSet. of()); } }