Allow to update accounts and general preferences atomically

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

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

PreferencesConfig encapsules all details about parsing and storing
general preferences and also provides means to read and update the
default general preferences.

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

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

@@ -136,18 +136,12 @@ public class AccountCacheImpl implements AccountCache {
   static class ByIdLoader extends CacheLoader<Account.Id, Optional<AccountState>> {
     private final AllUsersName allUsersName;
     private final Accounts accounts;
-    private final GeneralPreferencesLoader loader;
     private final ExternalIds externalIds;
 
     @Inject
-    ByIdLoader(
+    ByIdLoader(AllUsersName allUsersName, Accounts accounts, ExternalIds externalIds) {
-        AllUsersName allUsersName,
-        Accounts accounts,
-        GeneralPreferencesLoader loader,
-        ExternalIds externalIds) {
       this.allUsersName = allUsersName;
       this.accounts = accounts;
-      this.loader = loader;
       this.externalIds = externalIds;
     }
 
@@ -159,7 +153,7 @@ public class AccountCacheImpl implements AccountCache {
       }
 
       try {
-        account.setGeneralPreferences(loader.load(who));
+        account.setGeneralPreferences(accounts.getGeneralPreferences(who));
       } catch (IOException | ConfigInvalidException e) {
         log.warn("Cannot load GeneralPreferences for " + who + " (using default)", e);
         account.setGeneralPreferences(GeneralPreferencesInfo.defaults());

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

@@ -20,6 +20,7 @@ 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.extensions.client.GeneralPreferencesInfo;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.account.WatchConfig.NotifyType;
@@ -39,6 +40,7 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.CommitBuilder;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevSort;
 
@@ -78,17 +80,20 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
   public static final String KEY_STATUS = "status";
 
   private final Account.Id accountId;
+  private final Repository repo;
   private final String ref;
 
   private Optional<Account> loadedAccount;
   private WatchConfig watchConfig;
+  private PreferencesConfig prefConfig;
   private Optional<InternalAccountUpdate> accountUpdate = Optional.empty();
   private Timestamp registeredOn;
   private boolean eagerParsing;
   private List<ValidationError> validationErrors;
 
-  public AccountConfig(Account.Id accountId) {
+  public AccountConfig(Account.Id accountId, Repository allUsersRepo) {
-    this.accountId = checkNotNull(accountId);
+    this.accountId = checkNotNull(accountId, "accountId");
+    this.repo = checkNotNull(allUsersRepo, "allUsersRepo");
     this.ref = RefNames.refsUsers(accountId);
   }
 
@@ -112,6 +117,11 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
     return ref;
   }
 
+  public AccountConfig load() throws IOException, ConfigInvalidException {
+    load(repo);
+    return this;
+  }
+
   /**
    * Get the loaded account.
    *
@@ -134,6 +144,16 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
     return watchConfig.getProjectWatches();
   }
 
+  /**
+   * Get the general preferences of the loaded account.
+   *
+   * @return the general preferences of the loaded account
+   */
+  public GeneralPreferencesInfo getGeneralPreferences() {
+    checkLoaded();
+    return prefConfig.getGeneralPreferences();
+  }
+
   /**
    * Sets the account. This means the loaded account will be overwritten with the given account.
    *
@@ -142,7 +162,7 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
    * @param account account that should be set
    * @throws IllegalStateException if the account was not loaded yet
    */
-  public void setAccount(Account account) {
+  public AccountConfig setAccount(Account account) {
     checkLoaded();
     this.loadedAccount = Optional.of(account);
     this.accountUpdate =
@@ -154,6 +174,7 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
                 .setStatus(account.getStatus())
                 .build());
     this.registeredOn = account.getRegisteredOn();
+    return this;
   }
 
   /**
@@ -182,8 +203,9 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
     return loadedAccount.get();
   }
 
-  public void setAccountUpdate(InternalAccountUpdate accountUpdate) {
+  public AccountConfig setAccountUpdate(InternalAccountUpdate accountUpdate) {
     this.accountUpdate = Optional.of(accountUpdate);
+    return this;
   }
 
   @Override
@@ -198,8 +220,17 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
       loadedAccount = Optional.of(parse(accountConfig, revision.name()));
 
       watchConfig = new WatchConfig(accountId, readConfig(WatchConfig.WATCH_CONFIG), this);
+
+      prefConfig =
+          new PreferencesConfig(
+              accountId,
+              readConfig(PreferencesConfig.PREFERENCES_CONFIG),
+              PreferencesConfig.readDefaultConfig(repo),
+              this);
+
       if (eagerParsing) {
         watchConfig.parse();
+        prefConfig.parse();
       }
     } else {
       loadedAccount = Optional.empty();
@@ -249,6 +280,7 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
 
     Config accountConfig = saveAccount();
     saveProjectWatches();
+    saveGeneralPreferences();
 
     // metaId is set in the commit(MetaDataUpdate) method after the commit is created
     loadedAccount = Optional.of(parse(accountConfig, null));
@@ -290,6 +322,14 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
     }
   }
 
+  private void saveGeneralPreferences() throws IOException, ConfigInvalidException {
+    if (accountUpdate.isPresent() && accountUpdate.get().getGeneralPreferences().isPresent()) {
+      saveConfig(
+          PreferencesConfig.PREFERENCES_CONFIG,
+          prefConfig.saveGeneralPreferences(accountUpdate.get().getGeneralPreferences().get()));
+    }
+  }
+
   /**
    * Sets/Unsets {@code account.active} in the given config.
    *

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

@@ -19,6 +19,7 @@ import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
 
 import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.account.WatchConfig.NotifyType;
@@ -130,9 +131,14 @@ public class Accounts {
   public Map<ProjectWatchKey, Set<NotifyType>> getProjectWatches(Account.Id accountId)
       throws IOException, ConfigInvalidException {
     try (Repository repo = repoManager.openRepository(allUsersName)) {
-      AccountConfig accountConfig = new AccountConfig(accountId);
+      return new AccountConfig(accountId, repo).load().getProjectWatches();
-      accountConfig.load(repo);
+    }
-      return accountConfig.getProjectWatches();
+  }
+
+  public GeneralPreferencesInfo getGeneralPreferences(Account.Id accountId)
+      throws IOException, ConfigInvalidException {
+    try (Repository repo = repoManager.openRepository(allUsersName)) {
+      return new AccountConfig(accountId, repo).load().getGeneralPreferences();
     }
   }
 
@@ -144,9 +150,7 @@ public class Accounts {
 
   private Optional<Account> read(Repository allUsersRepository, Account.Id accountId)
       throws IOException, ConfigInvalidException {
-    AccountConfig accountConfig = new AccountConfig(accountId);
+    return new AccountConfig(accountId, allUsersRepository).load().getLoadedAccount();
-    accountConfig.load(allUsersRepository);
-    return accountConfig.getLoadedAccount();
   }
 
   public static Stream<Account.Id> readUserRefs(Repository repo) throws IOException {

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

@@ -424,11 +424,8 @@ public class AccountsUpdate {
 
   private AccountConfig read(Repository allUsersRepo, Account.Id accountId)
       throws IOException, ConfigInvalidException {
-    AccountConfig accountConfig = new AccountConfig(accountId);
+    AccountConfig accountConfig = new AccountConfig(accountId, allUsersRepo).load();
-    accountConfig.load(allUsersRepo);
-
     afterReadRevision.run();
-
     return accountConfig;
   }
 

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

@@ -1,198 +0,0 @@
-// Copyright (C) 2015 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.account;
-
-import static com.google.gerrit.server.config.ConfigUtil.loadSection;
-import static com.google.gerrit.server.config.ConfigUtil.skipField;
-import static com.google.gerrit.server.git.UserConfigSections.CHANGE_TABLE;
-import static com.google.gerrit.server.git.UserConfigSections.CHANGE_TABLE_COLUMN;
-import static com.google.gerrit.server.git.UserConfigSections.KEY_ID;
-import static com.google.gerrit.server.git.UserConfigSections.KEY_MATCH;
-import static com.google.gerrit.server.git.UserConfigSections.KEY_TARGET;
-import static com.google.gerrit.server.git.UserConfigSections.KEY_TOKEN;
-import static com.google.gerrit.server.git.UserConfigSections.KEY_URL;
-import static com.google.gerrit.server.git.UserConfigSections.URL_ALIAS;
-
-import com.google.common.base.Strings;
-import com.google.common.collect.Lists;
-import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
-import com.google.gerrit.extensions.client.MenuItem;
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.UserConfigSections;
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-import java.io.IOException;
-import java.lang.reflect.Field;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.lib.Repository;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-@Singleton
-public class GeneralPreferencesLoader {
-  private static final Logger log = LoggerFactory.getLogger(GeneralPreferencesLoader.class);
-
-  private final GitRepositoryManager gitMgr;
-  private final AllUsersName allUsersName;
-
-  @Inject
-  public GeneralPreferencesLoader(GitRepositoryManager gitMgr, AllUsersName allUsersName) {
-    this.gitMgr = gitMgr;
-    this.allUsersName = allUsersName;
-  }
-
-  public GeneralPreferencesInfo load(Account.Id id)
-      throws IOException, ConfigInvalidException, RepositoryNotFoundException {
-    return read(id, null);
-  }
-
-  public GeneralPreferencesInfo merge(Account.Id id, GeneralPreferencesInfo in)
-      throws IOException, ConfigInvalidException, RepositoryNotFoundException {
-    return read(id, in);
-  }
-
-  private GeneralPreferencesInfo read(Account.Id id, GeneralPreferencesInfo in)
-      throws IOException, ConfigInvalidException, RepositoryNotFoundException {
-    try (Repository allUsers = gitMgr.openRepository(allUsersName)) {
-      // Load all users default prefs
-      VersionedAccountPreferences dp = VersionedAccountPreferences.forDefault();
-      dp.load(allUsers);
-
-      // Load user prefs
-      VersionedAccountPreferences p = VersionedAccountPreferences.forUser(id);
-      p.load(allUsers);
-      GeneralPreferencesInfo r =
-          loadSection(
-              p.getConfig(),
-              UserConfigSections.GENERAL,
-              null,
-              new GeneralPreferencesInfo(),
-              readDefaultsFromGit(dp.getConfig(), in),
-              in);
-      loadChangeTableColumns(r, p, dp);
-      return loadMyMenusAndUrlAliases(r, p, dp);
-    }
-  }
-
-  public GeneralPreferencesInfo readDefaultsFromGit(Repository git, GeneralPreferencesInfo in)
-      throws ConfigInvalidException, IOException {
-    VersionedAccountPreferences dp = VersionedAccountPreferences.forDefault();
-    dp.load(git);
-    return readDefaultsFromGit(dp.getConfig(), in);
-  }
-
-  private GeneralPreferencesInfo readDefaultsFromGit(Config config, GeneralPreferencesInfo in)
-      throws ConfigInvalidException {
-    GeneralPreferencesInfo allUserPrefs = new GeneralPreferencesInfo();
-    loadSection(
-        config,
-        UserConfigSections.GENERAL,
-        null,
-        allUserPrefs,
-        GeneralPreferencesInfo.defaults(),
-        in);
-    return updateDefaults(allUserPrefs);
-  }
-
-  private GeneralPreferencesInfo updateDefaults(GeneralPreferencesInfo input) {
-    GeneralPreferencesInfo result = GeneralPreferencesInfo.defaults();
-    try {
-      for (Field field : input.getClass().getDeclaredFields()) {
-        if (skipField(field)) {
-          continue;
-        }
-        Object newVal = field.get(input);
-        if (newVal != null) {
-          field.set(result, newVal);
-        }
-      }
-    } catch (IllegalAccessException e) {
-      log.error("Cannot get default general preferences from " + allUsersName.get(), e);
-      return GeneralPreferencesInfo.defaults();
-    }
-    return result;
-  }
-
-  public GeneralPreferencesInfo loadMyMenusAndUrlAliases(
-      GeneralPreferencesInfo r, VersionedAccountPreferences v, VersionedAccountPreferences d) {
-    r.my = my(v);
-    if (r.my.isEmpty() && !v.isDefaults()) {
-      r.my = my(d);
-    }
-    if (r.my.isEmpty()) {
-      r.my.add(new MenuItem("Changes", "#/dashboard/self", null));
-      r.my.add(new MenuItem("Draft Comments", "#/q/has:draft", null));
-      r.my.add(new MenuItem("Edits", "#/q/has:edit", null));
-      r.my.add(new MenuItem("Watched Changes", "#/q/is:watched+is:open", null));
-      r.my.add(new MenuItem("Starred Changes", "#/q/is:starred", null));
-      r.my.add(new MenuItem("Groups", "#/groups/self", null));
-    }
-
-    r.urlAliases = urlAliases(v);
-    if (r.urlAliases == null && !v.isDefaults()) {
-      r.urlAliases = urlAliases(d);
-    }
-    return r;
-  }
-
-  private static List<MenuItem> my(VersionedAccountPreferences v) {
-    List<MenuItem> my = new ArrayList<>();
-    Config cfg = v.getConfig();
-    for (String subsection : cfg.getSubsections(UserConfigSections.MY)) {
-      String url = my(cfg, subsection, KEY_URL, "#/");
-      String target = my(cfg, subsection, KEY_TARGET, url.startsWith("#") ? null : "_blank");
-      my.add(new MenuItem(subsection, url, target, my(cfg, subsection, KEY_ID, null)));
-    }
-    return my;
-  }
-
-  private static String my(Config cfg, String subsection, String key, String defaultValue) {
-    String val = cfg.getString(UserConfigSections.MY, subsection, key);
-    return !Strings.isNullOrEmpty(val) ? val : defaultValue;
-  }
-
-  public GeneralPreferencesInfo loadChangeTableColumns(
-      GeneralPreferencesInfo r, VersionedAccountPreferences v, VersionedAccountPreferences d) {
-    r.changeTable = changeTable(v);
-
-    if (r.changeTable.isEmpty() && !v.isDefaults()) {
-      r.changeTable = changeTable(d);
-    }
-    return r;
-  }
-
-  private static List<String> changeTable(VersionedAccountPreferences v) {
-    return Lists.newArrayList(v.getConfig().getStringList(CHANGE_TABLE, null, CHANGE_TABLE_COLUMN));
-  }
-
-  private static Map<String, String> urlAliases(VersionedAccountPreferences v) {
-    HashMap<String, String> urlAliases = new HashMap<>();
-    Config cfg = v.getConfig();
-    for (String subsection : cfg.getSubsections(URL_ALIAS)) {
-      urlAliases.put(
-          cfg.getString(URL_ALIAS, subsection, KEY_MATCH),
-          cfg.getString(URL_ALIAS, subsection, KEY_TOKEN));
-    }
-    return !urlAliases.isEmpty() ? urlAliases : null;
-  }
-}

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

@@ -18,6 +18,7 @@ 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.extensions.client.GeneralPreferencesInfo;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.account.WatchConfig.NotifyType;
 import com.google.gerrit.server.account.WatchConfig.ProjectWatchKey;
@@ -111,6 +112,16 @@ public abstract class InternalAccountUpdate {
    */
   public abstract ImmutableSet<ProjectWatchKey> getDeletedProjectWatches();
 
+  /**
+   * Returns the new value for the general preferences.
+   *
+   * <p>Only preferences that are non-null in the returned GeneralPreferencesInfo should be updated.
+   *
+   * @return the new value for the general preferences, {@code Optional#empty()} if the general
+   *     preferences are not being updated, the wrapped value is never {@code null}
+   */
+  public abstract Optional<GeneralPreferencesInfo> getGeneralPreferences();
+
   /**
    * Class to build an account update.
    *
@@ -363,6 +374,16 @@ public abstract class InternalAccountUpdate {
       return this;
     }
 
+    /**
+     * Sets the general preferences for the account.
+     *
+     * <p>Updates any preference that is non-null in the provided GeneralPreferencesInfo.
+     *
+     * @param generalPreferences the general preferences that should be set
+     * @return the builder
+     */
+    public abstract Builder setGeneralPreferences(GeneralPreferencesInfo generalPreferences);
+
     /**
      * Builds the account update.
      *
@@ -493,6 +514,12 @@ public abstract class InternalAccountUpdate {
         delegate.deleteProjectWatches(projectWatches);
         return this;
       }
+
+      @Override
+      public Builder setGeneralPreferences(GeneralPreferencesInfo generalPreferences) {
+        delegate.setGeneralPreferences(generalPreferences);
+        return this;
+      }
     }
   }
 }

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

@@ -0,0 +1,368 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.account;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+import static com.google.gerrit.server.config.ConfigUtil.loadSection;
+import static com.google.gerrit.server.config.ConfigUtil.skipField;
+import static com.google.gerrit.server.config.ConfigUtil.storeSection;
+import static com.google.gerrit.server.git.UserConfigSections.CHANGE_TABLE;
+import static com.google.gerrit.server.git.UserConfigSections.CHANGE_TABLE_COLUMN;
+import static com.google.gerrit.server.git.UserConfigSections.KEY_ID;
+import static com.google.gerrit.server.git.UserConfigSections.KEY_MATCH;
+import static com.google.gerrit.server.git.UserConfigSections.KEY_TARGET;
+import static com.google.gerrit.server.git.UserConfigSections.KEY_TOKEN;
+import static com.google.gerrit.server.git.UserConfigSections.KEY_URL;
+import static com.google.gerrit.server.git.UserConfigSections.URL_ALIAS;
+
+import com.google.common.base.Strings;
+import com.google.common.collect.Lists;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
+import com.google.gerrit.extensions.client.MenuItem;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.gerrit.server.git.UserConfigSections;
+import com.google.gerrit.server.git.ValidationError;
+import com.google.gerrit.server.git.VersionedMetaData;
+import java.io.IOException;
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Repository;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class PreferencesConfig {
+  private static final Logger log = LoggerFactory.getLogger(PreferencesConfig.class);
+
+  public static final String PREFERENCES_CONFIG = "preferences.config";
+
+  private final Account.Id accountId;
+  private final Config cfg;
+  private final Config defaultCfg;
+  private final ValidationError.Sink validationErrorSink;
+
+  private GeneralPreferencesInfo generalPreferences;
+
+  public PreferencesConfig(
+      Account.Id accountId,
+      Config cfg,
+      Config defaultCfg,
+      ValidationError.Sink validationErrorSink) {
+    this.accountId = checkNotNull(accountId, "accountId");
+    this.cfg = checkNotNull(cfg, "cfg");
+    this.defaultCfg = checkNotNull(defaultCfg, "defaultCfg");
+    this.validationErrorSink = checkNotNull(validationErrorSink, "validationErrorSink");
+  }
+
+  public GeneralPreferencesInfo getGeneralPreferences() {
+    if (generalPreferences == null) {
+      parse();
+    }
+    return generalPreferences;
+  }
+
+  public void parse() {
+    generalPreferences = parse(null);
+  }
+
+  public Config saveGeneralPreferences(GeneralPreferencesInfo input) throws ConfigInvalidException {
+    // merge configs
+    input = parse(input);
+
+    storeSection(
+        cfg, UserConfigSections.GENERAL, null, input, parseDefaultPreferences(defaultCfg, null));
+    setChangeTable(cfg, input.changeTable);
+    setMy(cfg, input.my);
+    setUrlAliases(cfg, input.urlAliases);
+
+    // evict the cached general preferences
+    this.generalPreferences = null;
+
+    return cfg;
+  }
+
+  private GeneralPreferencesInfo parse(@Nullable GeneralPreferencesInfo input) {
+    try {
+      return parse(cfg, defaultCfg, input);
+    } catch (ConfigInvalidException e) {
+      validationErrorSink.error(
+          new ValidationError(
+              PREFERENCES_CONFIG,
+              String.format(
+                  "Invalid general preferences for account %d: %s",
+                  accountId.get(), e.getMessage())));
+      return new GeneralPreferencesInfo();
+    }
+  }
+
+  private static GeneralPreferencesInfo parse(
+      Config cfg, @Nullable Config defaultCfg, @Nullable GeneralPreferencesInfo input)
+      throws ConfigInvalidException {
+    GeneralPreferencesInfo r =
+        loadSection(
+            cfg,
+            UserConfigSections.GENERAL,
+            null,
+            new GeneralPreferencesInfo(),
+            defaultCfg != null
+                ? parseDefaultPreferences(defaultCfg, input)
+                : GeneralPreferencesInfo.defaults(),
+            input);
+    if (input != null) {
+      r.changeTable = input.changeTable;
+      r.my = input.my;
+      r.urlAliases = input.urlAliases;
+    } else {
+      r.changeTable = parseChangeTableColumns(cfg, defaultCfg);
+      r.my = parseMyMenus(cfg, defaultCfg);
+      r.urlAliases = parseUrlAliases(cfg, defaultCfg);
+    }
+    return r;
+  }
+
+  private static GeneralPreferencesInfo parseDefaultPreferences(
+      Config defaultCfg, GeneralPreferencesInfo input) throws ConfigInvalidException {
+    GeneralPreferencesInfo allUserPrefs = new GeneralPreferencesInfo();
+    loadSection(
+        defaultCfg,
+        UserConfigSections.GENERAL,
+        null,
+        allUserPrefs,
+        GeneralPreferencesInfo.defaults(),
+        input);
+    return updateDefaults(allUserPrefs);
+  }
+
+  private static GeneralPreferencesInfo updateDefaults(GeneralPreferencesInfo input) {
+    GeneralPreferencesInfo result = GeneralPreferencesInfo.defaults();
+    try {
+      for (Field field : input.getClass().getDeclaredFields()) {
+        if (skipField(field)) {
+          continue;
+        }
+        Object newVal = field.get(input);
+        if (newVal != null) {
+          field.set(result, newVal);
+        }
+      }
+    } catch (IllegalAccessException e) {
+      log.error("Failed to apply default general preferences", e);
+      return GeneralPreferencesInfo.defaults();
+    }
+    return result;
+  }
+
+  private static List<String> parseChangeTableColumns(Config cfg, @Nullable Config defaultCfg) {
+    List<String> changeTable = changeTable(cfg);
+    if (changeTable == null && defaultCfg != null) {
+      changeTable = changeTable(defaultCfg);
+    }
+    return changeTable;
+  }
+
+  private static List<MenuItem> parseMyMenus(Config cfg, @Nullable Config defaultCfg) {
+    List<MenuItem> my = my(cfg);
+    if (my.isEmpty() && defaultCfg != null) {
+      my = my(defaultCfg);
+    }
+    if (my.isEmpty()) {
+      my.add(new MenuItem("Changes", "#/dashboard/self", null));
+      my.add(new MenuItem("Draft Comments", "#/q/has:draft", null));
+      my.add(new MenuItem("Edits", "#/q/has:edit", null));
+      my.add(new MenuItem("Watched Changes", "#/q/is:watched+is:open", null));
+      my.add(new MenuItem("Starred Changes", "#/q/is:starred", null));
+      my.add(new MenuItem("Groups", "#/groups/self", null));
+    }
+    return my;
+  }
+
+  private static Map<String, String> parseUrlAliases(Config cfg, @Nullable Config defaultCfg) {
+    Map<String, String> urlAliases = urlAliases(cfg);
+    if (urlAliases == null && defaultCfg != null) {
+      urlAliases = urlAliases(defaultCfg);
+    }
+    return urlAliases;
+  }
+
+  public static GeneralPreferencesInfo readDefaultPreferences(Repository allUsersRepo)
+      throws IOException, ConfigInvalidException {
+    return parse(readDefaultConfig(allUsersRepo), null, null);
+  }
+
+  static Config readDefaultConfig(Repository allUsersRepo)
+      throws IOException, ConfigInvalidException {
+    VersionedDefaultPreferences defaultPrefs = new VersionedDefaultPreferences();
+    defaultPrefs.load(allUsersRepo);
+    return defaultPrefs.getConfig();
+  }
+
+  public static GeneralPreferencesInfo updateDefaultPreferences(
+      MetaDataUpdate md, GeneralPreferencesInfo input) throws IOException, ConfigInvalidException {
+    VersionedDefaultPreferences defaultPrefs = new VersionedDefaultPreferences();
+    defaultPrefs.load(md);
+    storeSection(
+        defaultPrefs.getConfig(),
+        UserConfigSections.GENERAL,
+        null,
+        input,
+        GeneralPreferencesInfo.defaults());
+    setMy(defaultPrefs.getConfig(), input.my);
+    setChangeTable(defaultPrefs.getConfig(), input.changeTable);
+    setUrlAliases(defaultPrefs.getConfig(), input.urlAliases);
+    defaultPrefs.commit(md);
+
+    return parse(defaultPrefs.getConfig(), null, null);
+  }
+
+  private static List<String> changeTable(Config cfg) {
+    return Lists.newArrayList(cfg.getStringList(CHANGE_TABLE, null, CHANGE_TABLE_COLUMN));
+  }
+
+  private static void setChangeTable(Config cfg, List<String> changeTable) {
+    if (changeTable != null) {
+      unsetSection(cfg, UserConfigSections.CHANGE_TABLE);
+      cfg.setStringList(UserConfigSections.CHANGE_TABLE, null, CHANGE_TABLE_COLUMN, changeTable);
+    }
+  }
+
+  private static List<MenuItem> my(Config cfg) {
+    List<MenuItem> my = new ArrayList<>();
+    for (String subsection : cfg.getSubsections(UserConfigSections.MY)) {
+      String url = my(cfg, subsection, KEY_URL, "#/");
+      String target = my(cfg, subsection, KEY_TARGET, url.startsWith("#") ? null : "_blank");
+      my.add(new MenuItem(subsection, url, target, my(cfg, subsection, KEY_ID, null)));
+    }
+    return my;
+  }
+
+  private static String my(Config cfg, String subsection, String key, String defaultValue) {
+    String val = cfg.getString(UserConfigSections.MY, subsection, key);
+    return !Strings.isNullOrEmpty(val) ? val : defaultValue;
+  }
+
+  private static void setMy(Config cfg, List<MenuItem> my) {
+    if (my != null) {
+      unsetSection(cfg, UserConfigSections.MY);
+      for (MenuItem item : my) {
+        checkState(!isNullOrEmpty(item.name), "MenuItem.name must not be null or empty");
+        checkState(!isNullOrEmpty(item.url), "MenuItem.url must not be null or empty");
+
+        setMy(cfg, item.name, KEY_URL, item.url);
+        setMy(cfg, item.name, KEY_TARGET, item.target);
+        setMy(cfg, item.name, KEY_ID, item.id);
+      }
+    }
+  }
+
+  public static void validateMy(List<MenuItem> my) throws BadRequestException {
+    if (my == null) {
+      return;
+    }
+    for (MenuItem item : my) {
+      checkRequiredMenuItemField(item.name, "name");
+      checkRequiredMenuItemField(item.url, "URL");
+    }
+  }
+
+  private static void checkRequiredMenuItemField(String value, String name)
+      throws BadRequestException {
+    if (isNullOrEmpty(value)) {
+      throw new BadRequestException(name + " for menu item is required");
+    }
+  }
+
+  private static boolean isNullOrEmpty(String value) {
+    return value == null || value.trim().isEmpty();
+  }
+
+  private static void setMy(Config cfg, String section, String key, @Nullable String val) {
+    if (val == null || val.trim().isEmpty()) {
+      cfg.unset(UserConfigSections.MY, section.trim(), key);
+    } else {
+      cfg.setString(UserConfigSections.MY, section.trim(), key, val.trim());
+    }
+  }
+
+  private static Map<String, String> urlAliases(Config cfg) {
+    HashMap<String, String> urlAliases = new HashMap<>();
+    for (String subsection : cfg.getSubsections(URL_ALIAS)) {
+      urlAliases.put(
+          cfg.getString(URL_ALIAS, subsection, KEY_MATCH),
+          cfg.getString(URL_ALIAS, subsection, KEY_TOKEN));
+    }
+    return !urlAliases.isEmpty() ? urlAliases : null;
+  }
+
+  private static void setUrlAliases(Config cfg, Map<String, String> urlAliases) {
+    if (urlAliases != null) {
+      for (String subsection : cfg.getSubsections(URL_ALIAS)) {
+        cfg.unsetSection(URL_ALIAS, subsection);
+      }
+
+      int i = 1;
+      for (Entry<String, String> e : urlAliases.entrySet()) {
+        cfg.setString(URL_ALIAS, URL_ALIAS + i, KEY_MATCH, e.getKey());
+        cfg.setString(URL_ALIAS, URL_ALIAS + i, KEY_TOKEN, e.getValue());
+        i++;
+      }
+    }
+  }
+
+  private static void unsetSection(Config cfg, String section) {
+    cfg.unsetSection(section, null);
+    for (String subsection : cfg.getSubsections(section)) {
+      cfg.unsetSection(section, subsection);
+    }
+  }
+
+  private static class VersionedDefaultPreferences extends VersionedMetaData {
+    private Config cfg;
+
+    @Override
+    protected String getRefName() {
+      return RefNames.REFS_USERS_DEFAULT;
+    }
+
+    private Config getConfig() {
+      checkState(cfg != null, "Default preferences not loaded yet.");
+      return cfg;
+    }
+
+    @Override
+    protected void onLoad() throws IOException, ConfigInvalidException {
+      cfg = readConfig(PREFERENCES_CONFIG);
+    }
+
+    @Override
+    protected boolean onSave(CommitBuilder commit) throws IOException, ConfigInvalidException {
+      if (Strings.isNullOrEmpty(commit.getMessage())) {
+        commit.setMessage("Update default preferences\n");
+      }
+      saveConfig(PREFERENCES_CONFIG, cfg);
+      return true;
+    }
+  }
+}

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

@@ -31,6 +31,7 @@ import java.util.List;
 import java.util.Optional;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevWalk;
 
 public class AccountValidator {
@@ -45,12 +46,12 @@ public class AccountValidator {
   }
 
   public List<String> validate(
-      Account.Id accountId, RevWalk rw, @Nullable ObjectId oldId, ObjectId newId)
+      Account.Id accountId, Repository repo, RevWalk rw, @Nullable ObjectId oldId, ObjectId newId)
       throws IOException {
     Optional<Account> oldAccount = Optional.empty();
     if (oldId != null && !ObjectId.zeroId().equals(oldId)) {
       try {
-        oldAccount = loadAccount(accountId, rw, oldId, null);
+        oldAccount = loadAccount(accountId, repo, rw, oldId, null);
       } catch (ConfigInvalidException e) {
         // ignore, maybe the new commit is repairing it now
       }
@@ -59,7 +60,7 @@ public class AccountValidator {
     List<String> messages = new ArrayList<>();
     Optional<Account> newAccount;
     try {
-      newAccount = loadAccount(accountId, rw, newId, messages);
+      newAccount = loadAccount(accountId, repo, rw, newId, messages);
     } catch (ConfigInvalidException e) {
       return ImmutableList.of(
           String.format(
@@ -91,10 +92,14 @@ public class AccountValidator {
   }
 
   private Optional<Account> loadAccount(
-      Account.Id accountId, RevWalk rw, ObjectId commit, @Nullable List<String> messages)
+      Account.Id accountId,
+      Repository repo,
+      RevWalk rw,
+      ObjectId commit,
+      @Nullable List<String> messages)
       throws IOException, ConfigInvalidException {
     rw.reset();
-    AccountConfig accountConfig = new AccountConfig(accountId);
+    AccountConfig accountConfig = new AccountConfig(accountId, repo);
     accountConfig.setEagerParsing(true).load(rw, commit);
     if (messages != null) {
       messages.addAll(

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

@@ -41,6 +41,7 @@ import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.events.CommitReceivedEvent;
 import com.google.gerrit.server.git.BanCommit;
+import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.git.ValidationError;
 import com.google.gerrit.server.permissions.PermissionBackend;
@@ -84,6 +85,7 @@ public class CommitValidators {
     private final PersonIdent gerritIdent;
     private final String canonicalWebUrl;
     private final DynamicSet<CommitValidationListener> pluginValidators;
+    private final GitRepositoryManager repoManager;
     private final AllUsersName allUsers;
     private final AllProjectsName allProjects;
     private final ExternalIdsConsistencyChecker externalIdsConsistencyChecker;
@@ -97,6 +99,7 @@ public class CommitValidators {
         @CanonicalWebUrl @Nullable String canonicalWebUrl,
         @GerritServerConfig Config cfg,
         DynamicSet<CommitValidationListener> pluginValidators,
+        GitRepositoryManager repoManager,
         AllUsersName allUsers,
         AllProjectsName allProjects,
         ExternalIdsConsistencyChecker externalIdsConsistencyChecker,
@@ -105,6 +108,7 @@ public class CommitValidators {
       this.gerritIdent = gerritIdent;
       this.canonicalWebUrl = canonicalWebUrl;
       this.pluginValidators = pluginValidators;
+      this.repoManager = repoManager;
       this.allUsers = allUsers;
       this.allProjects = allProjects;
       this.externalIdsConsistencyChecker = externalIdsConsistencyChecker;
@@ -137,7 +141,7 @@ public class CommitValidators {
               new BannedCommitsValidator(rejectCommits),
               new PluginCommitValidationListener(pluginValidators),
               new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
-              new AccountCommitValidator(allUsers, accountValidator),
+              new AccountCommitValidator(repoManager, allUsers, accountValidator),
               new GroupCommitValidator(allUsers)));
     }
 
@@ -163,7 +167,7 @@ public class CommitValidators {
               new ConfigValidator(branch, user, rw, allUsers, allProjects),
               new PluginCommitValidationListener(pluginValidators),
               new ExternalIdUpdateListener(allUsers, externalIdsConsistencyChecker),
-              new AccountCommitValidator(allUsers, accountValidator),
+              new AccountCommitValidator(repoManager, allUsers, accountValidator),
               new GroupCommitValidator(allUsers)));
     }
 
@@ -700,10 +704,15 @@ public class CommitValidators {
   }
 
   public static class AccountCommitValidator implements CommitValidationListener {
+    private final GitRepositoryManager repoManager;
     private final AllUsersName allUsers;
     private final AccountValidator accountValidator;
 
-    public AccountCommitValidator(AllUsersName allUsers, AccountValidator accountValidator) {
+    public AccountCommitValidator(
+        GitRepositoryManager repoManager,
+        AllUsersName allUsers,
+        AccountValidator accountValidator) {
+      this.repoManager = repoManager;
       this.allUsers = allUsers;
       this.accountValidator = accountValidator;
     }
@@ -726,10 +735,11 @@ public class CommitValidators {
         return Collections.emptyList();
       }
 
-      try {
+      try (Repository repo = repoManager.openRepository(allUsers)) {
         List<String> errorMessages =
             accountValidator.validate(
                 accountId,
+                repo,
                 receiveEvent.revWalk,
                 receiveEvent.command.getOldId(),
                 receiveEvent.commit);

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

@@ -285,7 +285,7 @@ public class MergeValidators {
       }
 
       try (RevWalk rw = new RevWalk(repo)) {
-        List<String> errorMessages = accountValidator.validate(accountId, rw, null, commit);
+        List<String> errorMessages = accountValidator.validate(accountId, repo, rw, null, commit);
         if (!errorMessages.isEmpty()) {
           throw new MergeValidationException(
               "invalid account configuration: " + Joiner.on("; ").join(errorMessages));

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

@@ -14,7 +14,6 @@
 
 package com.google.gerrit.server.restapi.account;
 
-import static com.google.gerrit.server.config.ConfigUtil.storeSection;
 import static com.google.gerrit.server.git.UserConfigSections.CHANGE_TABLE_COLUMN;
 import static com.google.gerrit.server.git.UserConfigSections.KEY_ID;
 import static com.google.gerrit.server.git.UserConfigSections.KEY_MATCH;
@@ -36,14 +35,14 @@ import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.AccountResource;
-import com.google.gerrit.server.account.GeneralPreferencesLoader;
+import com.google.gerrit.server.account.AccountsUpdate;
+import com.google.gerrit.server.account.PreferencesConfig;
 import com.google.gerrit.server.account.VersionedAccountPreferences;
-import com.google.gerrit.server.config.AllUsersName;
-import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gerrit.server.git.UserConfigSections;
 import com.google.gerrit.server.permissions.GlobalPermission;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
@@ -52,7 +51,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.lib.Config;
 
 @Singleton
@@ -60,9 +58,7 @@ public class SetPreferences implements RestModifyView<AccountResource, GeneralPr
   private final Provider<CurrentUser> self;
   private final AccountCache cache;
   private final PermissionBackend permissionBackend;
-  private final GeneralPreferencesLoader loader;
+  private final AccountsUpdate.User accountsUpdate;
-  private final Provider<MetaDataUpdate.User> metaDataUpdateFactory;
-  private final AllUsersName allUsersName;
   private final DynamicMap<DownloadScheme> downloadSchemes;
 
   @Inject
@@ -70,62 +66,33 @@ public class SetPreferences implements RestModifyView<AccountResource, GeneralPr
       Provider<CurrentUser> self,
       AccountCache cache,
       PermissionBackend permissionBackend,
-      GeneralPreferencesLoader loader,
+      AccountsUpdate.User accountsUpdate,
-      Provider<MetaDataUpdate.User> metaDataUpdateFactory,
-      AllUsersName allUsersName,
       DynamicMap<DownloadScheme> downloadSchemes) {
     this.self = self;
-    this.loader = loader;
     this.cache = cache;
     this.permissionBackend = permissionBackend;
-    this.metaDataUpdateFactory = metaDataUpdateFactory;
+    this.accountsUpdate = accountsUpdate;
-    this.allUsersName = allUsersName;
     this.downloadSchemes = downloadSchemes;
   }
 
   @Override
-  public GeneralPreferencesInfo apply(AccountResource rsrc, GeneralPreferencesInfo i)
+  public GeneralPreferencesInfo apply(AccountResource rsrc, GeneralPreferencesInfo input)
       throws AuthException, BadRequestException, IOException, ConfigInvalidException,
-          PermissionBackendException {
+          PermissionBackendException, OrmException {
     if (self.get() != rsrc.getUser()) {
       permissionBackend.user(self).check(GlobalPermission.MODIFY_ACCOUNT);
     }
 
-    checkDownloadScheme(i.downloadScheme);
+    checkDownloadScheme(input.downloadScheme);
+    PreferencesConfig.validateMy(input.my);
     Account.Id id = rsrc.getUser().getAccountId();
-    GeneralPreferencesInfo n = loader.merge(id, i);
-
-    n.changeTable = i.changeTable;
-    n.my = i.my;
-    n.urlAliases = i.urlAliases;
-
-    writeToGit(id, n);
 
+    accountsUpdate
+        .create()
+        .update("Set Preferences via API", id, u -> u.setGeneralPreferences(input));
     return cache.get(id).getAccount().getGeneralPreferencesInfo();
   }
 
-  private void writeToGit(Account.Id id, GeneralPreferencesInfo i)
-      throws RepositoryNotFoundException, IOException, ConfigInvalidException, BadRequestException {
-    VersionedAccountPreferences prefs;
-    try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) {
-      prefs = VersionedAccountPreferences.forUser(id);
-      prefs.load(md);
-
-      storeSection(
-          prefs.getConfig(),
-          UserConfigSections.GENERAL,
-          null,
-          i,
-          loader.readDefaultsFromGit(md.getRepository(), null));
-
-      storeMyChangeTableColumns(prefs, i.changeTable);
-      storeMyMenus(prefs, i.my);
-      storeUrlAliases(prefs, i.urlAliases);
-      prefs.commit(md);
-      cache.evict(id);
-    }
-  }
-
   public static void storeMyMenus(VersionedAccountPreferences prefs, List<MenuItem> my)
       throws BadRequestException {
     Config cfg = prefs.getConfig();

java/com/google/gerrit/server/restapi/config/GetPreferences.java

@@ -14,33 +14,25 @@
 
 package com.google.gerrit.server.restapi.config;
 
-import static com.google.gerrit.server.config.ConfigUtil.loadSection;
-
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
 import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.server.account.GeneralPreferencesLoader;
+import com.google.gerrit.server.account.PreferencesConfig;
-import com.google.gerrit.server.account.VersionedAccountPreferences;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.config.ConfigResource;
 import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.UserConfigSections;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import java.io.IOException;
 import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.lib.Repository;
 
 @Singleton
 public class GetPreferences implements RestReadView<ConfigResource> {
-  private final GeneralPreferencesLoader loader;
   private final GitRepositoryManager gitMgr;
   private final AllUsersName allUsersName;
 
   @Inject
-  public GetPreferences(
+  public GetPreferences(GitRepositoryManager gitMgr, AllUsersName allUsersName) {
-      GeneralPreferencesLoader loader, GitRepositoryManager gitMgr, AllUsersName allUsersName) {
-    this.loader = loader;
     this.gitMgr = gitMgr;
     this.allUsersName = allUsersName;
   }
@@ -48,30 +40,8 @@ public class GetPreferences implements RestReadView<ConfigResource> {
   @Override
   public GeneralPreferencesInfo apply(ConfigResource rsrc)
       throws IOException, ConfigInvalidException {
-    return readFromGit(gitMgr, loader, allUsersName, null);
-  }
-
-  static GeneralPreferencesInfo readFromGit(
-      GitRepositoryManager gitMgr,
-      GeneralPreferencesLoader loader,
-      AllUsersName allUsersName,
-      GeneralPreferencesInfo in)
-      throws IOException, ConfigInvalidException, RepositoryNotFoundException {
     try (Repository git = gitMgr.openRepository(allUsersName)) {
-      VersionedAccountPreferences p = VersionedAccountPreferences.forDefault();
+      return PreferencesConfig.readDefaultPreferences(git);
-      p.load(git);
-
-      GeneralPreferencesInfo r =
-          loadSection(
-              p.getConfig(),
-              UserConfigSections.GENERAL,
-              null,
-              new GeneralPreferencesInfo(),
-              GeneralPreferencesInfo.defaults(),
-              in);
-
-      // TODO(davido): Maintain cache of default values in AllUsers repository
-      return loader.loadMyMenusAndUrlAliases(r, p, null);
     }
   }
 }

java/com/google/gerrit/server/restapi/config/SetPreferences.java

@@ -14,10 +14,7 @@
 
 package com.google.gerrit.server.restapi.config;
 
-import static com.google.gerrit.server.config.ConfigUtil.loadSection;
 import static com.google.gerrit.server.config.ConfigUtil.skipField;
-import static com.google.gerrit.server.config.ConfigUtil.storeSection;
-import static com.google.gerrit.server.restapi.config.GetPreferences.readFromGit;
 
 import com.google.gerrit.common.data.GlobalCapability;
 import com.google.gerrit.extensions.annotations.RequiresCapability;
@@ -25,20 +22,16 @@ import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.server.account.AccountCache;
-import com.google.gerrit.server.account.GeneralPreferencesLoader;
+import com.google.gerrit.server.account.PreferencesConfig;
-import com.google.gerrit.server.account.VersionedAccountPreferences;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.config.ConfigResource;
-import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.MetaDataUpdate;
-import com.google.gerrit.server.git.UserConfigSections;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
 import java.io.IOException;
 import java.lang.reflect.Field;
 import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -47,57 +40,31 @@ import org.slf4j.LoggerFactory;
 public class SetPreferences implements RestModifyView<ConfigResource, GeneralPreferencesInfo> {
   private static final Logger log = LoggerFactory.getLogger(SetPreferences.class);
 
-  private final GeneralPreferencesLoader loader;
-  private final GitRepositoryManager gitManager;
   private final Provider<MetaDataUpdate.User> metaDataUpdateFactory;
   private final AllUsersName allUsersName;
   private final AccountCache accountCache;
 
   @Inject
   SetPreferences(
-      GeneralPreferencesLoader loader,
-      GitRepositoryManager gitManager,
       Provider<MetaDataUpdate.User> metaDataUpdateFactory,
       AllUsersName allUsersName,
       AccountCache accountCache) {
-    this.loader = loader;
-    this.gitManager = gitManager;
     this.metaDataUpdateFactory = metaDataUpdateFactory;
     this.allUsersName = allUsersName;
     this.accountCache = accountCache;
   }
 
   @Override
-  public GeneralPreferencesInfo apply(ConfigResource rsrc, GeneralPreferencesInfo i)
+  public GeneralPreferencesInfo apply(ConfigResource rsrc, GeneralPreferencesInfo input)
       throws BadRequestException, IOException, ConfigInvalidException {
-    if (!hasSetFields(i)) {
+    if (!hasSetFields(input)) {
       throw new BadRequestException("unsupported option");
     }
-    return writeToGit(readFromGit(gitManager, loader, allUsersName, i));
+    PreferencesConfig.validateMy(input.my);
-  }
-
-  private GeneralPreferencesInfo writeToGit(GeneralPreferencesInfo i)
-      throws RepositoryNotFoundException, IOException, ConfigInvalidException, BadRequestException {
     try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) {
-      VersionedAccountPreferences p = VersionedAccountPreferences.forDefault();
+      GeneralPreferencesInfo updatedPrefs = PreferencesConfig.updateDefaultPreferences(md, input);
-      p.load(md);
-      storeSection(
-          p.getConfig(), UserConfigSections.GENERAL, null, i, GeneralPreferencesInfo.defaults());
-      com.google.gerrit.server.restapi.account.SetPreferences.storeMyMenus(p, i.my);
-      com.google.gerrit.server.restapi.account.SetPreferences.storeUrlAliases(p, i.urlAliases);
-      p.commit(md);
-
       accountCache.evictAllNoReindex();
-
+      return updatedPrefs;
-      GeneralPreferencesInfo r =
-          loadSection(
-              p.getConfig(),
-              UserConfigSections.GENERAL,
-              null,
-              new GeneralPreferencesInfo(),
-              GeneralPreferencesInfo.defaults(),
-              null);
-      return loader.loadMyMenusAndUrlAliases(r, p, null);
     }
   }
 

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

@@ -1,16 +1,16 @@
-//Copyright (C) 2016 The Android Open Source Project
+// Copyright (C) 2016 The Android Open Source Project
 //
-//Licensed under the Apache License, Version 2.0 (the "License");
+// Licensed under the Apache License, Version 2.0 (the "License");
-//you may not use this file except in compliance with the License.
+// you may not use this file except in compliance with the License.
-//You may obtain a copy of the License at
+// You may obtain a copy of the License at
 //
-//http://www.apache.org/licenses/LICENSE-2.0
+// http://www.apache.org/licenses/LICENSE-2.0
 //
-//Unless required by applicable law or agreed to in writing, software
+// Unless required by applicable law or agreed to in writing, software
-//distributed under the License is distributed on an "AS IS" BASIS,
+// distributed under the License is distributed on an "AS IS" BASIS,
-//WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-//See the License for the specific language governing permissions and
+// See the License for the specific language governing permissions and
-//limitations under the License.
+// limitations under the License.
 
 package com.google.gerrit.server.schema;
 
@@ -148,7 +148,7 @@ public class Schema_139 extends SchemaVersion {
           md.getCommitBuilder().setCommitter(serverUser);
           md.setMessage(MSG);
 
-          AccountConfig accountConfig = new AccountConfig(e.getKey());
+          AccountConfig accountConfig = new AccountConfig(e.getKey(), git);
           accountConfig.load(md);
           accountConfig.setAccountUpdate(
               InternalAccountUpdate.builder()

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

@@ -139,10 +139,7 @@ public class Schema_154 extends SchemaVersion {
     PersonIdent ident = serverIdent.get();
     md.getCommitBuilder().setAuthor(ident);
     md.getCommitBuilder().setCommitter(ident);
-    AccountConfig accountConfig = new AccountConfig(account.getId());
+    new AccountConfig(account.getId(), allUsersRepo).load().setAccount(account).commit(md);
-    accountConfig.load(allUsersRepo);
-    accountConfig.setAccount(account);
-    accountConfig.commit(md);
   }
 
   @FunctionalInterface

javatests/com/google/gerrit/acceptance/api/config/GeneralPreferencesIT.java

@@ -27,7 +27,7 @@ public class GeneralPreferencesIT extends AbstractDaemonTest {
   @Test
   public void getGeneralPreferences() throws Exception {
     GeneralPreferencesInfo result = gApi.config().server().getDefaultPreferences();
-    assertPrefs(result, GeneralPreferencesInfo.defaults(), "my");
+    assertPrefs(result, GeneralPreferencesInfo.defaults(), "changeTable", "my");
   }
 
   @Test
@@ -41,6 +41,6 @@ public class GeneralPreferencesIT extends AbstractDaemonTest {
     result = gApi.config().server().getDefaultPreferences();
     GeneralPreferencesInfo expected = GeneralPreferencesInfo.defaults();
     expected.signedOffBy = newSignedOffBy;
-    assertPrefs(result, expected, "my");
+    assertPrefs(result, expected, "changeTable", "my");
   }
 }

javatests/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java

@@ -416,10 +416,10 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
       PersonIdent ident = serverIdent.get();
       md.getCommitBuilder().setAuthor(ident);
       md.getCommitBuilder().setCommitter(ident);
-      AccountConfig accountConfig = new AccountConfig(accountId);
+      new AccountConfig(accountId, repo)
-      accountConfig.load(repo);
+          .load()
-      accountConfig.setAccountUpdate(InternalAccountUpdate.builder().setFullName(newName).build());
+          .setAccountUpdate(InternalAccountUpdate.builder().setFullName(newName).build())
-      accountConfig.commit(md);
+          .commit(md);
     }
 
     assertQuery("name:" + quote(user1.name), user1);

javatests/com/google/gerrit/server/schema/Schema_159_to_160_Test.java

@@ -106,7 +106,7 @@ public class Schema_159_to_160_Test {
     assertThat(myMenusFromApi(accountId).keySet()).containsExactlyElementsIn(newNames).inOrder();
   }
 
-  // Raw config values, bypassing the defaults set by GeneralPreferencesLoader.
+  // Raw config values, bypassing the defaults set by PreferencesConfig.
   private ImmutableMap<String, String> myMenusFromNoteDb(Account.Id id) throws Exception {
     try (Repository repo = repoManager.openRepository(allUsersName)) {
       VersionedAccountPreferences prefs = VersionedAccountPreferences.forUser(id);