Add project watches to AccountState

We need the project watches in AccountState so that we can add watched projects to the account index later. This change requires a manual flush of the account cache. Change-Id: I3bbeb623c20588a0cd44e5398a7a4079d9a79f32 Signed-off-by: Edwin Kempin <ekempin@google.com>
2016-07-04 16:33:38 +02:00
parent 22ca9734e7
commit ae27c46ea3
10 changed files with 75 additions and 60 deletions
--- 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<AccountExternalId> ids = Collections.emptySet();
+    Collection<AccountProjectWatch> projectWatches = Collections.emptySet();
    Set<AccountGroup.UUID> anon = ImmutableSet.of();
-    return new AccountState(account, anon, ids);
+    return new AccountState(account, anon, ids, projectWatches);
  }

  static class ByIdLoader extends CacheLoader<Account.Id, AccountState> {
@@ -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<AccountExternalId> externalIds =
-          Collections.unmodifiableCollection(db.accountExternalIds().byAccount(
-              who).toList());
+      Collection<AccountExternalId> externalIds =
+          Collections.unmodifiableCollection(
+              db.accountExternalIds().byAccount(who).toList());

      Set<AccountGroup.UUID> 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<AccountProjectWatch> projectWatches =
+          Collections.unmodifiableCollection(
+              db.accountProjectWatches().byAccount(who).toList());
+
+      return new AccountState(account, internalGroups, externalIds,
+          projectWatches);
    }
  }

--- 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<AccountGroup.UUID> internalGroups;
  private final Collection<AccountExternalId> externalIds;
+  private final Collection<AccountProjectWatch> projectWatches;
  private Cache<IdentifiedUser.PropertyKey<Object>, Object> properties;

-  public AccountState(final Account account,
-      final Set<AccountGroup.UUID> actualGroups,
-      final Collection<AccountExternalId> externalIds) {
+  public AccountState(Account account,
+      Set<AccountGroup.UUID> actualGroups,
+      Collection<AccountExternalId> externalIds,
+      Collection<AccountProjectWatch> 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<AccountProjectWatch> getProjectWatches() {
+    return projectWatches;
+  }
+
  /** The set of groups maintained directly within the Gerrit database. */
  public Set<AccountGroup.UUID> getInternalGroups() {
    return internalGroups;
--- 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<ReviewDb> dbProvider;
  private final Provider<IdentifiedUser> self;
+  private final AccountCache accountCache;

  @Inject
  DeleteWatchedProjects(Provider<ReviewDb> dbProvider,
-      Provider<IdentifiedUser> self) {
+      Provider<IdentifiedUser> self,
+      AccountCache accountCache) {
    this.dbProvider = dbProvider;
    this.self = self;
+    this.accountCache = accountCache;
  }

  @Override
-  public Response<?> apply(
-      AccountResource rsrc, List<ProjectWatchInfo> input)
-      throws UnprocessableEntityException, OrmException, AuthException {
+  public Response<?> apply(AccountResource rsrc, List<ProjectWatchInfo> 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<AccountProjectWatch> watchedProjects =
-        dbProvider.get().accountProjectWatches()
-            .byAccount(rsrc.getUser().getAccountId());
+        dbProvider.get().accountProjectWatches().byAccount(accountId);
    HashMap<AccountProjectWatch.Key, AccountProjectWatch>
        watchedProjectsMap = new HashMap<>();
    for (AccountProjectWatch watchedProject : watchedProjects) {
@@ -68,10 +73,8 @@ public class DeleteWatchedProjects
    if (input != null) {
      List<AccountProjectWatch> 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();
  }
--- 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<AccountResource, List<ProjectWatchInfo>> {
+  private final Provider<ReviewDb> dbProvider;
  private final Provider<IdentifiedUser> self;
  private final GetWatchedProjects getWatchedProjects;
-  private final Provider<ReviewDb> dbProvider;
  private final ProjectsCollection projectsCollection;
+  private final AccountCache accountCache;

  @Inject
-  public PostWatchedProjects(GetWatchedProjects getWatchedProjects,
-      Provider<ReviewDb> dbProvider,
+  public PostWatchedProjects(Provider<ReviewDb> dbProvider,
+      Provider<IdentifiedUser> self,
+      GetWatchedProjects getWatchedProjects,
      ProjectsCollection projectsCollection,
-      Provider<IdentifiedUser> 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<AccountProjectWatch> accountProjectWatchList =
-        getAccountProjectWatchList(input, rsrc.getUser().getAccountId());
+        getAccountProjectWatchList(input, accountId);
    dbProvider.get().accountProjectWatches().upsert(accountProjectWatchList);
+    accountCache.evict(accountId);
    return getWatchedProjects.apply(rsrc);
  }

--- 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);
    }
  }
--- 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<ChangeData> {
    final IndexConfig indexConfig;
    final Provider<ListMembers> listMembers;
    final StarredChangesUtil starredChangesUtil;
+    final AccountCache accountCache;
    final boolean allowsDrafts;

    private final Provider<CurrentUser> self;
@@ -217,6 +219,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
        IndexConfig indexConfig,
        Provider<ListMembers> 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<ChangeData> {
          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<ChangeData> {
        IndexConfig indexConfig,
        Provider<ListMembers> listMembers,
        StarredChangesUtil starredChangesUtil,
+        AccountCache accountCache,
        boolean allowsDrafts) {
     this.db = db;
     this.queryProvider = queryProvider;
@@ -284,6 +288,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
     this.indexConfig = indexConfig;
     this.listMembers = listMembers;
     this.starredChangesUtil = starredChangesUtil;
+     this.accountCache = accountCache;
     this.allowsDrafts = allowsDrafts;
    }

@@ -295,7 +300,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
          allProjectsName, allUsersName, patchListCache, repoManager,
          projectCache, listChildProjects, submitDryRun,
          conflictsCache, trackingFooters, index, indexConfig, listMembers,
-          starredChangesUtil, allowsDrafts);
+          starredChangesUtil, accountCache, allowsDrafts);
    }

    Arguments asUser(Account.Id otherId) {
--- 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<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();
@@ -101,22 +91,14 @@ class IsWatchedByPredicate extends AndPredicate<ChangeData> {
    }
  }

-  private static List<AccountProjectWatch> getWatches(
+  private static Collection<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);
+    if (user.isIdentifiedUser()) {
+      return args.accountCache.get(args.getUser().getAccountId())
+          .getProjectWatches();
    }
-    }
-    return MoreObjects.firstNonNull(
-        watches,
-        Collections.<AccountProjectWatch> emptyList());
+    return Collections.<AccountProjectWatch> emptySet();
  }

  private static List<Predicate<ChangeData>> none() {
--- 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
--- 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.<AccountGroup.UUID> emptySet(),
-          Collections.<AccountExternalId> emptySet());
+        Collections.<AccountExternalId> emptySet(),
+        Collections.<AccountProjectWatch> emptySet());
  }
 }
--- 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.<AccountGroup.UUID> of(),
-        ImmutableSet.<AccountExternalId> of());
+    return new AccountState(account, ImmutableSet.<AccountGroup.UUID> of(),
+        ImmutableSet.<AccountExternalId> of(),
+        ImmutableSet.<AccountProjectWatch> of());
  }
 }