Allow to update accounts and project watches atomically

AccountsUpdate now also supports to update project watches. This means
account properties and project watches can now be updated in the same
transaction.

WatchConfig.Accessor is removed so that project watch updates can only
be done through AccountsUpdate. For reading project watches a new
method is added to Accounts. In the future we would rather like to
change the Accounts#get(Account.Id) method to return AccountState
instead of Account so that this get method can be used to retrieve the
account as well as the project watches. Once this is done the
getProjectWatches method will be removed again.

Reading and writing project watches is now done via AccountConfig. On
load we always read the watch.config file, but parsing it is done
lazily when the project watches are accessed for the first time. Most
callers that load accounts are not interested in project watches and
this avoids unneeded parsing effort for them.

Change-Id: I8cd441eee3f65817a3653610c5d3bb62b0bfea5e
Signed-off-by: Edwin Kempin <ekempin@google.com>

java/com/google/gerrit/pgm/init/AccountsOnInit.java

@@ -70,7 +70,7 @@ public class AccountsOnInit {
                 new GerritPersonIdentProvider(flags.cfg).get(), account.getRegisteredOn());
 
         Config accountConfig = new Config();
-        AccountConfig.writeToConfig(
+        AccountConfig.writeToAccountConfig(
             InternalAccountUpdate.builder()
                 .setActive(account.isActive())
                 .setFullName(account.getFullName())

java/com/google/gerrit/server/account/AccountCacheImpl.java

@@ -137,7 +137,6 @@ public class AccountCacheImpl implements AccountCache {
     private final AllUsersName allUsersName;
     private final Accounts accounts;
     private final GeneralPreferencesLoader loader;
-    private final Provider<WatchConfig.Accessor> watchConfig;
     private final ExternalIds externalIds;
 
     @Inject
@@ -145,12 +144,10 @@ public class AccountCacheImpl implements AccountCache {
         AllUsersName allUsersName,
         Accounts accounts,
         GeneralPreferencesLoader loader,
-        Provider<WatchConfig.Accessor> watchConfig,
         ExternalIds externalIds) {
       this.allUsersName = allUsersName;
       this.accounts = accounts;
       this.loader = loader;
-      this.watchConfig = watchConfig;
       this.externalIds = externalIds;
     }
 
@@ -170,10 +167,7 @@ public class AccountCacheImpl implements AccountCache {
 
       return Optional.of(
           new AccountState(
-              allUsersName,
+              allUsersName, account, externalIds.byAccount(who), accounts.getProjectWatches(who)));
-              account,
-              externalIds.byAccount(who),
-              watchConfig.get().getProjectWatches(who)));
     }
   }
 }

java/com/google/gerrit/server/account/AccountConfig.java

@@ -18,15 +18,23 @@ import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 
 import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableList;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.account.WatchConfig.NotifyType;
+import com.google.gerrit.server.account.WatchConfig.ProjectWatchKey;
 import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.gerrit.server.git.ValidationError;
 import com.google.gerrit.server.git.VersionedMetaData;
 import com.google.gwtorm.server.OrmDuplicateKeyException;
 import java.io.IOException;
 import java.sql.Timestamp;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.CommitBuilder;
 import org.eclipse.jgit.lib.Config;
@@ -61,7 +69,7 @@ import org.eclipse.jgit.revwalk.RevSort;
  * account. The first commit may be an empty commit (if no properties were set and 'account.config'
  * doesn't exist).
  */
-public class AccountConfig extends VersionedMetaData {
+public class AccountConfig extends VersionedMetaData implements ValidationError.Sink {
   public static final String ACCOUNT_CONFIG = "account.config";
   public static final String ACCOUNT = "account";
   public static final String KEY_ACTIVE = "active";
@@ -73,14 +81,32 @@ public class AccountConfig extends VersionedMetaData {
   private final String ref;
 
   private Optional<Account> loadedAccount;
+  private WatchConfig watchConfig;
   private Optional<InternalAccountUpdate> accountUpdate = Optional.empty();
   private Timestamp registeredOn;
+  private boolean eagerParsing;
+  private List<ValidationError> validationErrors;
 
   public AccountConfig(Account.Id accountId) {
     this.accountId = checkNotNull(accountId);
     this.ref = RefNames.refsUsers(accountId);
   }
 
+  /**
+   * Sets whether all account data should be eagerly parsed.
+   *
+   * <p>Eager parsing should only be used if the caller is interested in validation errors for all
+   * account data (see {@link #getValidationErrors()}.
+   *
+   * @param eagerParsing whether all account data should be eagerly parsed
+   * @return this AccountConfig instance for chaining
+   */
+  public AccountConfig setEagerParsing(boolean eagerParsing) {
+    checkState(loadedAccount == null, "Account %s already loaded", accountId.get());
+    this.eagerParsing = eagerParsing;
+    return this;
+  }
+
   @Override
   protected String getRefName() {
     return ref;
@@ -98,6 +124,16 @@ public class AccountConfig extends VersionedMetaData {
     return loadedAccount;
   }
 
+  /**
+   * Get the project watches of the loaded account.
+   *
+   * @return the project watches of the loaded account
+   */
+  public Map<ProjectWatchKey, Set<NotifyType>> getProjectWatches() {
+    checkLoaded();
+    return watchConfig.getProjectWatches();
+  }
+
   /**
    * Sets the account. This means the loaded account will be overwritten with the given account.
    *
@@ -158,9 +194,13 @@ public class AccountConfig extends VersionedMetaData {
       rw.sort(RevSort.REVERSE);
       registeredOn = new Timestamp(rw.next().getCommitTime() * 1000L);
 
-      Config cfg = readConfig(ACCOUNT_CONFIG);
+      Config accountConfig = readConfig(ACCOUNT_CONFIG);
+      loadedAccount = Optional.of(parse(accountConfig, revision.name()));
 
-      loadedAccount = Optional.of(parse(cfg, revision.name()));
+      watchConfig = new WatchConfig(accountId, readConfig(WatchConfig.WATCH_CONFIG), this);
+      if (eagerParsing) {
+        watchConfig.parse();
+      }
     } else {
       loadedAccount = Optional.empty();
     }
@@ -207,21 +247,27 @@ public class AccountConfig extends VersionedMetaData {
       commit.setCommitter(new PersonIdent(commit.getCommitter(), registeredOn));
     }
 
-    Config cfg = readConfig(ACCOUNT_CONFIG);
+    Config accountConfig = saveAccount();
-    if (accountUpdate.isPresent()) {
+    saveProjectWatches();
-      writeToConfig(accountUpdate.get(), cfg);
-    }
-    saveConfig(ACCOUNT_CONFIG, cfg);
 
     // metaId is set in the commit(MetaDataUpdate) method after the commit is created
-    loadedAccount = Optional.of(parse(cfg, null));
+    loadedAccount = Optional.of(parse(accountConfig, null));
 
     accountUpdate = Optional.empty();
 
     return true;
   }
 
-  public static void writeToConfig(InternalAccountUpdate accountUpdate, Config cfg) {
+  private Config saveAccount() throws IOException, ConfigInvalidException {
+    Config accountConfig = readConfig(ACCOUNT_CONFIG);
+    if (accountUpdate.isPresent()) {
+      writeToAccountConfig(accountUpdate.get(), accountConfig);
+    }
+    saveConfig(ACCOUNT_CONFIG, accountConfig);
+    return accountConfig;
+  }
+
+  public static void writeToAccountConfig(InternalAccountUpdate accountUpdate, Config cfg) {
     accountUpdate.getActive().ifPresent(active -> setActive(cfg, active));
     accountUpdate.getFullName().ifPresent(fullName -> set(cfg, KEY_FULL_NAME, fullName));
     accountUpdate
@@ -230,6 +276,20 @@ public class AccountConfig extends VersionedMetaData {
     accountUpdate.getStatus().ifPresent(status -> set(cfg, KEY_STATUS, status));
   }
 
+  private void saveProjectWatches() throws IOException {
+    if (accountUpdate.isPresent()
+        && (!accountUpdate.get().getDeletedProjectWatches().isEmpty()
+            || !accountUpdate.get().getUpdatedProjectWatches().isEmpty())) {
+      Map<ProjectWatchKey, Set<NotifyType>> projectWatches = watchConfig.getProjectWatches();
+      accountUpdate.get().getDeletedProjectWatches().forEach(pw -> projectWatches.remove(pw));
+      accountUpdate
+          .get()
+          .getUpdatedProjectWatches()
+          .forEach((pw, nt) -> projectWatches.put(pw, nt));
+      saveConfig(WatchConfig.WATCH_CONFIG, watchConfig.save(projectWatches));
+    }
+  }
+
   /**
    * Sets/Unsets {@code account.active} in the given config.
    *
@@ -282,4 +342,27 @@ public class AccountConfig extends VersionedMetaData {
   private void checkLoaded() {
     checkState(loadedAccount != null, "Account %s not loaded yet", accountId.get());
   }
+
+  /**
+   * Get the validation errors, if any were discovered during parsing the account data.
+   *
+   * <p>To get validation errors for all account data request eager parsing before loading the
+   * account (see {@link #setEagerParsing(boolean)}).
+   *
+   * @return list of errors; empty list if there are no errors.
+   */
+  public List<ValidationError> getValidationErrors() {
+    if (validationErrors != null) {
+      return ImmutableList.copyOf(validationErrors);
+    }
+    return ImmutableList.of();
+  }
+
+  @Override
+  public void error(ValidationError error) {
+    if (validationErrors == null) {
+      validationErrors = new ArrayList<>(4);
+    }
+    validationErrors.add(error);
+  }
 }

java/com/google/gerrit/server/account/Accounts.java

@@ -21,6 +21,8 @@ import static java.util.stream.Collectors.toSet;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.account.WatchConfig.NotifyType;
+import com.google.gerrit.server.account.WatchConfig.ProjectWatchKey;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.inject.Inject;
@@ -29,6 +31,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -124,6 +127,15 @@ public class Accounts {
     return readUserRefs(repo).findAny().isPresent();
   }
 
+  public Map<ProjectWatchKey, Set<NotifyType>> getProjectWatches(Account.Id accountId)
+      throws IOException, ConfigInvalidException {
+    try (Repository repo = repoManager.openRepository(allUsersName)) {
+      AccountConfig accountConfig = new AccountConfig(accountId);
+      accountConfig.load(repo);
+      return accountConfig.getProjectWatches();
+    }
+  }
+
   private Stream<Account.Id> readUserRefs() throws IOException {
     try (Repository repo = repoManager.openRepository(allUsersName)) {
       return readUserRefs(repo);

java/com/google/gerrit/server/account/InternalAccountUpdate.java

@@ -16,12 +16,17 @@ package com.google.gerrit.server.account;
 
 import com.google.auto.value.AutoValue;
 import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.server.account.WatchConfig.NotifyType;
+import com.google.gerrit.server.account.WatchConfig.ProjectWatchKey;
 import com.google.gerrit.server.account.externalids.DuplicateExternalIdKeyException;
 import com.google.gerrit.server.account.externalids.ExternalId;
 import java.util.Collection;
+import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 
 /**
  * Class to prepare updates to an account.
@@ -92,6 +97,20 @@ public abstract class InternalAccountUpdate {
    */
   public abstract ImmutableSet<ExternalId> getDeletedExternalIds();
 
+  /**
+   * Returns external IDs that should be updated for the account.
+   *
+   * @return external IDs that should be updated for the account
+   */
+  public abstract ImmutableMap<ProjectWatchKey, Set<NotifyType>> getUpdatedProjectWatches();
+
+  /**
+   * Returns project watches that should be deleted for the account.
+   *
+   * @return project watches that should be deleted for the account
+   */
+  public abstract ImmutableSet<ProjectWatchKey> getDeletedProjectWatches();
+
   /**
    * Class to build an account update.
    *
@@ -239,7 +258,7 @@ public abstract class InternalAccountUpdate {
     }
 
     /**
-     * Delete external IDs for the account.
+     * Deletes external IDs for the account.
      *
      * <p>The account IDs of the external IDs must match the account ID of the account that is
      * updated.
@@ -277,6 +296,73 @@ public abstract class InternalAccountUpdate {
       return deleteExternalIds(extIdsToDelete).addExternalIds(extIdsToAdd);
     }
 
+    /**
+     * Returns a builder for the map of updated project watches.
+     *
+     * @return builder for the map of updated project watches.
+     */
+    abstract ImmutableMap.Builder<ProjectWatchKey, Set<NotifyType>> updatedProjectWatchesBuilder();
+
+    /**
+     * Updates a project watch for the account.
+     *
+     * <p>If no project watch with the key exists the project watch is created.
+     *
+     * @param projectWatchKey key of the project watch that should be updated
+     * @param notifyTypes the notify types that should be set for the project watch
+     * @return the builder
+     */
+    public Builder updateProjectWatch(
+        ProjectWatchKey projectWatchKey, Set<NotifyType> notifyTypes) {
+      return updateProjectWatches(ImmutableMap.of(projectWatchKey, notifyTypes));
+    }
+
+    /**
+     * Updates project watches for the account.
+     *
+     * <p>If any of the project watches already exists, it is overwritten. New project watches are
+     * inserted.
+     *
+     * @param projectWatches project watches that should be updated
+     * @return the builder
+     */
+    public Builder updateProjectWatches(Map<ProjectWatchKey, Set<NotifyType>> projectWatches) {
+      updatedProjectWatchesBuilder().putAll(projectWatches);
+      return this;
+    }
+
+    /**
+     * Returns a builder for the set of deleted project watches.
+     *
+     * @return builder for the set of deleted project watches.
+     */
+    abstract ImmutableSet.Builder<ProjectWatchKey> deletedProjectWatchesBuilder();
+
+    /**
+     * Deletes a project watch for the account.
+     *
+     * <p>If no project watch with the ID exists this is a no-op.
+     *
+     * @param projectWatch project watch that should be deleted
+     * @return the builder
+     */
+    public Builder deleteProjectWatch(ProjectWatchKey projectWatch) {
+      return deleteProjectWatches(ImmutableSet.of(projectWatch));
+    }
+
+    /**
+     * Deletes project watches for the account.
+     *
+     * <p>For non-existing project watches this is a no-op.
+     *
+     * @param projectWatches project watches that should be deleted
+     * @return the builder
+     */
+    public Builder deleteProjectWatches(Collection<ProjectWatchKey> projectWatches) {
+      deletedProjectWatchesBuilder().addAll(projectWatches);
+      return this;
+    }
+
     /**
      * Builds the account update.
      *
@@ -385,6 +471,28 @@ public abstract class InternalAccountUpdate {
         delegate.deleteExternalIds(extIds);
         return this;
       }
+
+      @Override
+      ImmutableMap.Builder<ProjectWatchKey, Set<NotifyType>> updatedProjectWatchesBuilder() {
+        return delegate.updatedProjectWatchesBuilder();
+      }
+
+      @Override
+      public Builder updateProjectWatches(Map<ProjectWatchKey, Set<NotifyType>> projectWatches) {
+        delegate.updateProjectWatches(projectWatches);
+        return this;
+      }
+
+      @Override
+      ImmutableSet.Builder<ProjectWatchKey> deletedProjectWatchesBuilder() {
+        return delegate.deletedProjectWatchesBuilder();
+      }
+
+      @Override
+      public Builder deleteProjectWatches(Collection<ProjectWatchKey> projectWatches) {
+        delegate.deleteProjectWatches(projectWatches);
+        return this;
+      }
     }
   }
 }

java/com/google/gerrit/server/account/WatchConfig.java

@@ -15,7 +15,7 @@
 package com.google.gerrit.server.account;
 
 import static com.google.common.base.MoreObjects.firstNonNull;
-import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.auto.value.AutoValue;
 import com.google.common.annotations.VisibleForTesting;
@@ -23,7 +23,6 @@ import com.google.common.base.Enums;
 import com.google.common.base.Joiner;
 import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.MultimapBuilder;
@@ -31,17 +30,7 @@ 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.Project;
-import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gerrit.server.git.ValidationError;
-import com.google.gerrit.server.git.VersionedMetaData;
-import com.google.inject.Inject;
-import com.google.inject.Provider;
-import com.google.inject.Singleton;
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.EnumSet;
@@ -49,10 +38,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.CommitBuilder;
 import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.lib.Repository;
 
 /**
  * ‘watch.config’ file in the user branch in the All-Users repository that contains the watch
@@ -85,91 +71,7 @@ import org.eclipse.jgit.lib.Repository;
  *
  * <p>Unknown notify types are ignored and removed on save.
  */
-public class WatchConfig extends VersionedMetaData implements ValidationError.Sink {
+public class WatchConfig {
-  @Singleton
-  public static class Accessor {
-    private final GitRepositoryManager repoManager;
-    private final AllUsersName allUsersName;
-    private final Provider<MetaDataUpdate.User> metaDataUpdateFactory;
-    private final IdentifiedUser.GenericFactory userFactory;
-
-    @Inject
-    Accessor(
-        GitRepositoryManager repoManager,
-        AllUsersName allUsersName,
-        Provider<MetaDataUpdate.User> metaDataUpdateFactory,
-        IdentifiedUser.GenericFactory userFactory) {
-      this.repoManager = repoManager;
-      this.allUsersName = allUsersName;
-      this.metaDataUpdateFactory = metaDataUpdateFactory;
-      this.userFactory = userFactory;
-    }
-
-    public Map<ProjectWatchKey, Set<NotifyType>> getProjectWatches(Account.Id accountId)
-        throws IOException, ConfigInvalidException {
-      try (Repository git = repoManager.openRepository(allUsersName)) {
-        WatchConfig watchConfig = new WatchConfig(accountId);
-        watchConfig.load(git);
-        return watchConfig.getProjectWatches();
-      }
-    }
-
-    public synchronized void upsertProjectWatches(
-        Account.Id accountId, Map<ProjectWatchKey, Set<NotifyType>> newProjectWatches)
-        throws IOException, ConfigInvalidException {
-      WatchConfig watchConfig = read(accountId);
-      Map<ProjectWatchKey, Set<NotifyType>> projectWatches = watchConfig.getProjectWatches();
-      projectWatches.putAll(newProjectWatches);
-      commit(watchConfig);
-    }
-
-    public synchronized void deleteProjectWatches(
-        Account.Id accountId, Collection<ProjectWatchKey> projectWatchKeys)
-        throws IOException, ConfigInvalidException {
-      WatchConfig watchConfig = read(accountId);
-      Map<ProjectWatchKey, Set<NotifyType>> projectWatches = watchConfig.getProjectWatches();
-      boolean commit = false;
-      for (ProjectWatchKey key : projectWatchKeys) {
-        if (projectWatches.remove(key) != null) {
-          commit = true;
-        }
-      }
-      if (commit) {
-        commit(watchConfig);
-      }
-    }
-
-    public synchronized void deleteAllProjectWatches(Account.Id accountId)
-        throws IOException, ConfigInvalidException {
-      WatchConfig watchConfig = read(accountId);
-      boolean commit = false;
-      if (!watchConfig.getProjectWatches().isEmpty()) {
-        watchConfig.getProjectWatches().clear();
-        commit = true;
-      }
-      if (commit) {
-        commit(watchConfig);
-      }
-    }
-
-    private WatchConfig read(Account.Id accountId) throws IOException, ConfigInvalidException {
-      try (Repository git = repoManager.openRepository(allUsersName)) {
-        WatchConfig watchConfig = new WatchConfig(accountId);
-        watchConfig.load(git);
-        return watchConfig;
-      }
-    }
-
-    private void commit(WatchConfig watchConfig) throws IOException {
-      try (MetaDataUpdate md =
-          metaDataUpdateFactory
-              .get()
-              .create(allUsersName, userFactory.create(watchConfig.accountId))) {
-        watchConfig.commit(md);
-      }
-    }
-  }
-
   @AutoValue
   public abstract static class ProjectWatchKey {
     public static ProjectWatchKey create(Project.NameKey project, @Nullable String filter) {
@@ -199,25 +101,26 @@ public class WatchConfig extends VersionedMetaData implements ValidationError.Si
   public static final String KEY_NOTIFY = "notify";
 
   private final Account.Id accountId;
-  private final String ref;
+  private final Config cfg;
+  private final ValidationError.Sink validationErrorSink;
 
   private Map<ProjectWatchKey, Set<NotifyType>> projectWatches;
-  private List<ValidationError> validationErrors;
 
-  public WatchConfig(Account.Id accountId) {
+  public WatchConfig(Account.Id accountId, Config cfg, ValidationError.Sink validationErrorSink) {
-    this.accountId = accountId;
+    this.accountId = checkNotNull(accountId, "accountId");
-    this.ref = RefNames.refsUsers(accountId);
+    this.cfg = checkNotNull(cfg, "cfg");
+    this.validationErrorSink = checkNotNull(validationErrorSink, "validationErrorSink");
   }
 
-  @Override
+  public Map<ProjectWatchKey, Set<NotifyType>> getProjectWatches() {
-  protected String getRefName() {
+    if (projectWatches == null) {
-    return ref;
+      parse();
+    }
+    return projectWatches;
   }
 
-  @Override
+  public void parse() {
-  protected void onLoad() throws IOException, ConfigInvalidException {
+    projectWatches = parse(accountId, cfg, validationErrorSink);
-    Config cfg = readConfig(WATCH_CONFIG);
-    projectWatches = parse(accountId, cfg, this);
   }
 
   @VisibleForTesting
@@ -248,24 +151,8 @@ public class WatchConfig extends VersionedMetaData implements ValidationError.Si
     return projectWatches;
   }
 
-  Map<ProjectWatchKey, Set<NotifyType>> getProjectWatches() {
+  public Config save(Map<ProjectWatchKey, Set<NotifyType>> projectWatches) {
-    checkLoaded();
-    return projectWatches;
-  }
-
-  public void setProjectWatches(Map<ProjectWatchKey, Set<NotifyType>> projectWatches) {
     this.projectWatches = projectWatches;
-  }
-
-  @Override
-  protected boolean onSave(CommitBuilder commit) throws IOException, ConfigInvalidException {
-    checkLoaded();
-
-    if (Strings.isNullOrEmpty(commit.getMessage())) {
-      commit.setMessage("Updated watch configuration\n");
-    }
-
-    Config cfg = readConfig(WATCH_CONFIG);
 
     for (String projectName : cfg.getSubsections(PROJECT)) {
       cfg.unsetSection(PROJECT, projectName);
@@ -282,32 +169,7 @@ public class WatchConfig extends VersionedMetaData implements ValidationError.Si
       cfg.setStringList(PROJECT, e.getKey(), KEY_NOTIFY, new ArrayList<>(e.getValue()));
     }
 
-    saveConfig(WATCH_CONFIG, cfg);
+    return cfg;
-    return true;
-  }
-
-  private void checkLoaded() {
-    checkState(projectWatches != null, "project watches not loaded yet");
-  }
-
-  @Override
-  public void error(ValidationError error) {
-    if (validationErrors == null) {
-      validationErrors = new ArrayList<>(4);
-    }
-    validationErrors.add(error);
-  }
-
-  /**
-   * Get the validation errors, if any were discovered during load.
-   *
-   * @return list of errors; empty list if there are no errors.
-   */
-  public List<ValidationError> getValidationErrors() {
-    if (validationErrors != null) {
-      return ImmutableList.copyOf(validationErrors);
-    }
-    return ImmutableList.of();
   }
 
   @AutoValue
@@ -356,7 +218,7 @@ public class WatchConfig extends VersionedMetaData implements ValidationError.Si
       return create(filter, notifyTypes);
     }
 
-    public static NotifyValue create(@Nullable String filter, Set<NotifyType> notifyTypes) {
+    public static NotifyValue create(@Nullable String filter, Collection<NotifyType> notifyTypes) {
       return new AutoValue_WatchConfig_NotifyValue(
           Strings.emptyToNull(filter), Sets.immutableEnumSet(notifyTypes));
     }

java/com/google/gerrit/server/git/validators/AccountValidator.java

@@ -14,11 +14,14 @@
 
 package com.google.gerrit.server.git.validators;
 
+import static java.util.stream.Collectors.toSet;
+
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountConfig;
+import com.google.gerrit.server.git.ValidationError;
 import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -47,15 +50,16 @@ public class AccountValidator {
     Optional<Account> oldAccount = Optional.empty();
     if (oldId != null && !ObjectId.zeroId().equals(oldId)) {
       try {
-        oldAccount = loadAccount(accountId, rw, oldId);
+        oldAccount = loadAccount(accountId, rw, oldId, null);
       } catch (ConfigInvalidException e) {
         // ignore, maybe the new commit is repairing it now
       }
     }
 
+    List<String> messages = new ArrayList<>();
     Optional<Account> newAccount;
     try {
-      newAccount = loadAccount(accountId, rw, newId);
+      newAccount = loadAccount(accountId, rw, newId, messages);
     } catch (ConfigInvalidException e) {
       return ImmutableList.of(
           String.format(
@@ -67,7 +71,6 @@ public class AccountValidator {
       return ImmutableList.of(String.format("account '%s' does not exist", accountId.get()));
     }
 
-    List<String> messages = new ArrayList<>();
     if (accountId.equals(self.get().getAccountId()) && !newAccount.get().isActive()) {
       messages.add("cannot deactivate own account");
     }
@@ -87,11 +90,20 @@ public class AccountValidator {
     return ImmutableList.copyOf(messages);
   }
 
-  private Optional<Account> loadAccount(Account.Id accountId, RevWalk rw, ObjectId commit)
+  private Optional<Account> loadAccount(
+      Account.Id accountId, RevWalk rw, ObjectId commit, @Nullable List<String> messages)
       throws IOException, ConfigInvalidException {
     rw.reset();
     AccountConfig accountConfig = new AccountConfig(accountId);
-    accountConfig.load(rw, commit);
+    accountConfig.setEagerParsing(true).load(rw, commit);
+    if (messages != null) {
+      messages.addAll(
+          accountConfig
+              .getValidationErrors()
+              .stream()
+              .map(ValidationError::getMessage)
+              .collect(toSet()));
+    }
     return accountConfig.getLoadedAccount();
   }
 }

java/com/google/gerrit/server/git/validators/CommitValidators.java

@@ -34,7 +34,6 @@ import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.WatchConfig;
 import com.google.gerrit.server.account.externalids.ExternalIdsConsistencyChecker;
 import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.config.AllUsersName;
@@ -418,34 +417,6 @@ public class CommitValidators {
         }
       }
 
-      if (allUsers.equals(branch.getParentKey()) && RefNames.isRefsUsers(branch.get())) {
-        List<CommitValidationMessage> messages = new ArrayList<>();
-        Account.Id accountId = Account.Id.fromRef(branch.get());
-        if (accountId != null) {
-          try {
-            WatchConfig wc = new WatchConfig(accountId);
-            wc.load(rw, receiveEvent.command.getNewId());
-            if (!wc.getValidationErrors().isEmpty()) {
-              addError("Invalid project configuration:", messages);
-              for (ValidationError err : wc.getValidationErrors()) {
-                addError("  " + err.getMessage(), messages);
-              }
-              throw new ConfigInvalidException("invalid watch configuration");
-            }
-          } catch (IOException | ConfigInvalidException e) {
-            log.error(
-                "User "
-                    + user.getUserName()
-                    + " tried to push an invalid watch configuration "
-                    + receiveEvent.command.getNewId().name()
-                    + " for account "
-                    + accountId.get(),
-                e);
-            throw new CommitValidationException("invalid watch configuration", messages);
-          }
-        }
-      }
-
       return Collections.emptyList();
     }
   }

java/com/google/gerrit/server/restapi/account/DeleteWatchedProjects.java

@@ -24,9 +24,8 @@ import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.AccountResource;
-import com.google.gerrit.server.account.WatchConfig;
+import com.google.gerrit.server.account.AccountsUpdate;
 import com.google.gerrit.server.account.WatchConfig.ProjectWatchKey;
 import com.google.gerrit.server.permissions.GlobalPermission;
 import com.google.gerrit.server.permissions.PermissionBackend;
@@ -45,19 +44,16 @@ public class DeleteWatchedProjects
     implements RestModifyView<AccountResource, List<ProjectWatchInfo>> {
   private final Provider<IdentifiedUser> self;
   private final PermissionBackend permissionBackend;
-  private final AccountCache accountCache;
+  private final AccountsUpdate.User accountsUpdate;
-  private final WatchConfig.Accessor watchConfig;
 
   @Inject
   DeleteWatchedProjects(
       Provider<IdentifiedUser> self,
       PermissionBackend permissionBackend,
-      AccountCache accountCache,
+      AccountsUpdate.User accountsUpdate) {
-      WatchConfig.Accessor watchConfig) {
     this.self = self;
     this.permissionBackend = permissionBackend;
-    this.accountCache = accountCache;
+    this.accountsUpdate = accountsUpdate;
-    this.watchConfig = watchConfig;
   }
 
   @Override
@@ -72,14 +68,18 @@ public class DeleteWatchedProjects
     }
 
     Account.Id accountId = rsrc.getUser().getAccountId();
-    watchConfig.deleteProjectWatches(
+    accountsUpdate
-        accountId,
+        .create()
-        input
+        .update(
-            .stream()
+            "Delete Project Watches via API",
-            .filter(Objects::nonNull)
+            accountId,
-            .map(w -> ProjectWatchKey.create(new Project.NameKey(w.project), w.filter))
+            u ->
-            .collect(toList()));
+                u.deleteProjectWatches(
-    accountCache.evict(accountId);
+                    input
+                        .stream()
+                        .filter(Objects::nonNull)
+                        .map(w -> ProjectWatchKey.create(new Project.NameKey(w.project), w.filter))
+                        .collect(toList())));
     return Response.none();
   }
 }

java/com/google/gerrit/server/restapi/account/GetWatchedProjects.java

@@ -22,7 +22,7 @@ import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountResource;
-import com.google.gerrit.server.account.WatchConfig;
+import com.google.gerrit.server.account.Accounts;
 import com.google.gerrit.server.account.WatchConfig.NotifyType;
 import com.google.gerrit.server.account.WatchConfig.ProjectWatchKey;
 import com.google.gerrit.server.permissions.GlobalPermission;
@@ -45,16 +45,14 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
 public class GetWatchedProjects implements RestReadView<AccountResource> {
   private final PermissionBackend permissionBackend;
   private final Provider<IdentifiedUser> self;
-  private final WatchConfig.Accessor watchConfig;
+  private final Accounts accounts;
 
   @Inject
   public GetWatchedProjects(
-      PermissionBackend permissionBackend,
+      PermissionBackend permissionBackend, Provider<IdentifiedUser> self, Accounts accounts) {
-      Provider<IdentifiedUser> self,
-      WatchConfig.Accessor watchConfig) {
     this.permissionBackend = permissionBackend;
     this.self = self;
-    this.watchConfig = watchConfig;
+    this.accounts = accounts;
   }
 
   @Override
@@ -68,7 +66,7 @@ public class GetWatchedProjects implements RestReadView<AccountResource> {
     Account.Id accountId = rsrc.getUser().getAccountId();
     List<ProjectWatchInfo> projectWatchInfos = new ArrayList<>();
     for (Map.Entry<ProjectWatchKey, Set<NotifyType>> e :
-        watchConfig.getProjectWatches(accountId).entrySet()) {
+        accounts.getProjectWatches(accountId).entrySet()) {
       ProjectWatchInfo pwi = new ProjectWatchInfo();
       pwi.filter = e.getKey().filter();
       pwi.project = e.getKey().project().get();

java/com/google/gerrit/server/restapi/account/PostWatchedProjects.java

@@ -19,10 +19,9 @@ import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.RestApiException;
 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.server.IdentifiedUser;
-import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.AccountResource;
+import com.google.gerrit.server.account.AccountsUpdate;
 import com.google.gerrit.server.account.WatchConfig;
 import com.google.gerrit.server.account.WatchConfig.NotifyType;
 import com.google.gerrit.server.account.WatchConfig.ProjectWatchKey;
@@ -49,8 +48,7 @@ public class PostWatchedProjects
   private final PermissionBackend permissionBackend;
   private final GetWatchedProjects getWatchedProjects;
   private final ProjectsCollection projectsCollection;
-  private final AccountCache accountCache;
+  private final AccountsUpdate.User accountsUpdate;
-  private final WatchConfig.Accessor watchConfig;
 
   @Inject
   public PostWatchedProjects(
@@ -58,14 +56,12 @@ public class PostWatchedProjects
       PermissionBackend permissionBackend,
       GetWatchedProjects getWatchedProjects,
       ProjectsCollection projectsCollection,
-      AccountCache accountCache,
+      AccountsUpdate.User accountsUpdate) {
-      WatchConfig.Accessor watchConfig) {
     this.self = self;
     this.permissionBackend = permissionBackend;
     this.getWatchedProjects = getWatchedProjects;
     this.projectsCollection = projectsCollection;
-    this.accountCache = accountCache;
+    this.accountsUpdate = accountsUpdate;
-    this.watchConfig = watchConfig;
   }
 
   @Override
@@ -76,9 +72,13 @@ public class PostWatchedProjects
       permissionBackend.user(self).check(GlobalPermission.ADMINISTRATE_SERVER);
     }
 
-    Account.Id accountId = rsrc.getUser().getAccountId();
+    Map<ProjectWatchKey, Set<NotifyType>> projectWatches = asMap(input);
-    watchConfig.upsertProjectWatches(accountId, asMap(input));
+    accountsUpdate
-    accountCache.evict(accountId);
+        .create()
+        .update(
+            "Update Project Watches via API",
+            rsrc.getUser().getAccountId(),
+            u -> u.updateProjectWatches(projectWatches));
     return getWatchedProjects.apply(rsrc);
   }
 

java/com/google/gerrit/server/schema/Schema_139.java

@@ -24,7 +24,8 @@ import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.GerritPersonIdent;
-import com.google.gerrit.server.account.WatchConfig;
+import com.google.gerrit.server.account.AccountConfig;
+import com.google.gerrit.server.account.InternalAccountUpdate;
 import com.google.gerrit.server.account.WatchConfig.NotifyType;
 import com.google.gerrit.server.account.WatchConfig.ProjectWatchKey;
 import com.google.gerrit.server.config.AllUsersName;
@@ -147,10 +148,14 @@ public class Schema_139 extends SchemaVersion {
           md.getCommitBuilder().setCommitter(serverUser);
           md.setMessage(MSG);
 
-          WatchConfig watchConfig = new WatchConfig(e.getKey());
+          AccountConfig accountConfig = new AccountConfig(e.getKey());
-          watchConfig.load(md);
+          accountConfig.load(md);
-          watchConfig.setProjectWatches(projectWatches);
+          accountConfig.setAccountUpdate(
-          watchConfig.commit(md);
+              InternalAccountUpdate.builder()
+                  .deleteProjectWatches(accountConfig.getProjectWatches().keySet())
+                  .updateProjectWatches(projectWatches)
+                  .build());
+          accountConfig.commit(md);
         }
       }
       bru.execute(rw, NullProgressMonitor.INSTANCE);

javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java

@@ -1335,7 +1335,7 @@ public class AccountIT extends AbstractDaemonTest {
             WatchConfig.WATCH_CONFIG,
             wc.toText());
     PushOneCommit.Result r = push.to(RefNames.REFS_USERS_SELF);
-    r.assertErrorStatus("invalid watch configuration");
+    r.assertErrorStatus("invalid account configuration");
     r.assertMessage(
         String.format(
             "%s: Invalid project watch of account %d for project %s: %s",

javatests/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java

@@ -26,30 +26,21 @@ import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.api.changes.StarsInput;
 import com.google.gerrit.extensions.common.GroupInfo;
-import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.server.account.WatchConfig;
 import com.google.gerrit.server.account.WatchConfig.NotifyType;
-import com.google.gerrit.server.account.WatchConfig.ProjectWatchKey;
 import com.google.gerrit.server.git.NotifyConfig;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.mail.Address;
 import com.google.gerrit.testing.FakeEmailSender.Message;
-import com.google.inject.Inject;
 import java.util.EnumSet;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
-import java.util.Set;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.TestRepository;
 import org.junit.Test;
 
 @NoHttpd
 public class ProjectWatchIT extends AbstractDaemonTest {
-  @Inject private WatchConfig.Accessor watchConfig;
-
   @Test
   public void newPatchSetsNotifyConfig() throws Exception {
     Address addr = new Address("Watcher", "watcher@example.com");
@@ -493,34 +484,6 @@ public class ProjectWatchIT extends AbstractDaemonTest {
     assertThat(sender.getMessages()).isEmpty();
   }
 
-  @Test
-  public void deleteAllProjectWatches() throws Exception {
-    Map<ProjectWatchKey, Set<NotifyType>> watches = new HashMap<>();
-    watches.put(ProjectWatchKey.create(project, "*"), ImmutableSet.of(NotifyType.ALL));
-    watchConfig.upsertProjectWatches(admin.getId(), watches);
-    assertThat(watchConfig.getProjectWatches(admin.getId())).isNotEmpty();
-
-    watchConfig.deleteAllProjectWatches(admin.getId());
-    assertThat(watchConfig.getProjectWatches(admin.getId())).isEmpty();
-  }
-
-  @Test
-  public void deleteAllProjectWatchesIfWatchConfigIsTheOnlyFileInUserBranch() throws Exception {
-    // Create account that has no files in its refs/users/ branch.
-    Account.Id id = accountCreator.create().id;
-
-    // Add a project watch so that a watch.config file in the refs/users/ branch is created.
-    Map<ProjectWatchKey, Set<NotifyType>> watches = new HashMap<>();
-    watches.put(ProjectWatchKey.create(project, "*"), ImmutableSet.of(NotifyType.ALL));
-    watchConfig.upsertProjectWatches(id, watches);
-    assertThat(watchConfig.getProjectWatches(id)).isNotEmpty();
-
-    // Delete all project watches so that the watch.config file in the refs/users/ branch is
-    // deleted.
-    watchConfig.deleteAllProjectWatches(id);
-    assertThat(watchConfig.getProjectWatches(id)).isEmpty();
-  }
-
   @Test
   public void watchProjectNoNotificationForPrivateChange() throws Exception {
     // watch project