Allow to update account and diff preferences atomically

AccountsUpdate now also support to update the diff preferences and
AccountState makes them accessible. This means diff preferences are now
also cached, but since the parsing of the diff preferences is done
lazily this doesn't make loading of accounts any more expensive (the
preferences.config file that contains the diff preferences is already
loaded because it also contains the general preferences).

Some methods in PreferencesConfig exist now twice, once for general
preferences and once for diff preferences. Future changes may make an
effort to remove code duplication, but for now we are happy that these
methods are in the same place now and that the handling of general
preferences and diff preferences is consistent now.

Change-Id: I5d116fbdf5593be4e701863b12fb7cf8f5eea9ad
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin 2018-01-15 17:32:19 +01:00
parent d40d81482d
commit 52f131ab68
11 changed files with 241 additions and 197 deletions

View File

@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.RefNames;
@ -175,6 +176,16 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
return prefConfig.getGeneralPreferences();
}
/**
* Get the diff preferences of the loaded account.
*
* @return the diff preferences of the loaded account
*/
public DiffPreferencesInfo getDiffPreferences() {
checkLoaded();
return prefConfig.getDiffPreferences();
}
/**
* Sets the account. This means the loaded account will be overwritten with the given account.
*
@ -310,7 +321,7 @@ public class AccountConfig extends VersionedMetaData implements ValidationError.
Config accountConfig = saveAccount();
saveProjectWatches();
saveGeneralPreferences();
savePreferences();
// metaId is set in the commit(MetaDataUpdate) method after the commit is created
loadedAccount = Optional.of(parse(accountConfig, null));
@ -353,12 +364,17 @@ 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()));
private void savePreferences() throws IOException, ConfigInvalidException {
if (!accountUpdate.isPresent()
|| (!accountUpdate.get().getGeneralPreferences().isPresent()
&& !accountUpdate.get().getDiffPreferences().isPresent())) {
return;
}
saveConfig(
PreferencesConfig.PREFERENCES_CONFIG,
prefConfig.saveGeneralPreferences(
accountUpdate.get().getGeneralPreferences(), accountUpdate.get().getDiffPreferences()));
}
/**

View File

@ -25,6 +25,7 @@ import com.google.common.cache.CacheBuilder;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.CurrentUser.PropertyKey;
@ -115,7 +116,8 @@ public class AccountState {
account,
extIds,
Suppliers.memoize(() -> accountConfig.getProjectWatches()),
Suppliers.memoize(() -> accountConfig.getGeneralPreferences())));
Suppliers.memoize(() -> accountConfig.getGeneralPreferences()),
Suppliers.memoize(() -> accountConfig.getDiffPreferences())));
}
/**
@ -145,7 +147,8 @@ public class AccountState {
account,
ImmutableSet.copyOf(extIds),
Suppliers.ofInstance(ImmutableMap.of()),
Suppliers.ofInstance(GeneralPreferencesInfo.defaults()));
Suppliers.ofInstance(GeneralPreferencesInfo.defaults()),
Suppliers.ofInstance(DiffPreferencesInfo.defaults()));
}
private final AllUsersName allUsersName;
@ -153,6 +156,7 @@ public class AccountState {
private final ImmutableSet<ExternalId> externalIds;
private final Supplier<ImmutableMap<ProjectWatchKey, ImmutableSet<NotifyType>>> projectWatches;
private final Supplier<GeneralPreferencesInfo> generalPreferences;
private final Supplier<DiffPreferencesInfo> diffPreferences;
private Cache<IdentifiedUser.PropertyKey<Object>, Object> properties;
private AccountState(
@ -160,12 +164,14 @@ public class AccountState {
Account account,
ImmutableSet<ExternalId> externalIds,
Supplier<ImmutableMap<ProjectWatchKey, ImmutableSet<NotifyType>>> projectWatches,
Supplier<GeneralPreferencesInfo> generalPreferences) {
Supplier<GeneralPreferencesInfo> generalPreferences,
Supplier<DiffPreferencesInfo> diffPreferences) {
this.allUsersName = allUsersName;
this.account = account;
this.externalIds = externalIds;
this.projectWatches = projectWatches;
this.generalPreferences = generalPreferences;
this.diffPreferences = diffPreferences;
this.account.setUserName(ExternalId.getUserName(externalIds).orElse(null));
}
@ -231,6 +237,11 @@ public class AccountState {
return generalPreferences.get();
}
/** The diff preferences of the account. */
public DiffPreferencesInfo getDiffPreferences() {
return diffPreferences.get();
}
/**
* Lookup a previously stored property.
*

View File

@ -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.DiffPreferencesInfo;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.account.WatchConfig.NotifyType;
@ -122,6 +123,16 @@ public abstract class InternalAccountUpdate {
*/
public abstract Optional<GeneralPreferencesInfo> getGeneralPreferences();
/**
* Returns the new value for the diff preferences.
*
* <p>Only preferences that are non-null in the returned DiffPreferencesInfo should be updated.
*
* @return the new value for the diff preferences, {@code Optional#empty()} if the diff
* preferences are not being updated, the wrapped value is never {@code null}
*/
public abstract Optional<DiffPreferencesInfo> getDiffPreferences();
/**
* Class to build an account update.
*
@ -384,6 +395,16 @@ public abstract class InternalAccountUpdate {
*/
public abstract Builder setGeneralPreferences(GeneralPreferencesInfo generalPreferences);
/**
* Sets the diff preferences for the account.
*
* <p>Updates any preference that is non-null in the provided DiffPreferencesInfo.
*
* @param diffPreferences the diff preferences that should be set
* @return the builder
*/
public abstract Builder setDiffPreferences(DiffPreferencesInfo diffPreferences);
/**
* Builds the account update.
*
@ -520,6 +541,12 @@ public abstract class InternalAccountUpdate {
delegate.setGeneralPreferences(generalPreferences);
return this;
}
@Override
public Builder setDiffPreferences(DiffPreferencesInfo diffPreferences) {
delegate.setDiffPreferences(diffPreferences);
return this;
}
}
}
}

View File

@ -31,6 +31,7 @@ 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.DiffPreferencesInfo;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.extensions.client.MenuItem;
import com.google.gerrit.extensions.restapi.BadRequestException;
@ -47,6 +48,7 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Optional;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
@ -65,6 +67,7 @@ public class PreferencesConfig {
private final ValidationError.Sink validationErrorSink;
private GeneralPreferencesInfo generalPreferences;
private DiffPreferencesInfo diffPreferences;
public PreferencesConfig(
Account.Id accountId,
@ -84,29 +87,61 @@ public class PreferencesConfig {
return generalPreferences;
}
public void parse() {
generalPreferences = parse(null);
public DiffPreferencesInfo getDiffPreferences() {
if (diffPreferences == null) {
parse();
}
return diffPreferences;
}
public Config saveGeneralPreferences(GeneralPreferencesInfo input) throws ConfigInvalidException {
// merge configs
input = parse(input);
public void parse() {
generalPreferences = parseGeneralPreferences(null);
diffPreferences = parseDiffPreferences(null);
}
storeSection(
cfg, UserConfigSections.GENERAL, null, input, parseDefaultPreferences(defaultCfg, null));
setChangeTable(cfg, input.changeTable);
setMy(cfg, input.my);
setUrlAliases(cfg, input.urlAliases);
public Config saveGeneralPreferences(
Optional<GeneralPreferencesInfo> generalPreferencesInput,
Optional<DiffPreferencesInfo> diffPreferencesInput)
throws ConfigInvalidException {
if (generalPreferencesInput.isPresent()) {
GeneralPreferencesInfo mergedGeneralPreferencesInput =
parseGeneralPreferences(generalPreferencesInput.get());
// evict the cached general preferences
this.generalPreferences = null;
storeSection(
cfg,
UserConfigSections.GENERAL,
null,
mergedGeneralPreferencesInput,
parseDefaultGeneralPreferences(defaultCfg, null));
setChangeTable(cfg, mergedGeneralPreferencesInput.changeTable);
setMy(cfg, mergedGeneralPreferencesInput.my);
setUrlAliases(cfg, mergedGeneralPreferencesInput.urlAliases);
// evict the cached general preferences
this.generalPreferences = null;
}
if (diffPreferencesInput.isPresent()) {
DiffPreferencesInfo mergedDiffPreferencesInput =
parseDiffPreferences(diffPreferencesInput.get());
storeSection(
cfg,
UserConfigSections.DIFF,
null,
mergedDiffPreferencesInput,
parseDefaultDiffPreferences(defaultCfg, null));
// evict the cached diff preferences
this.diffPreferences = null;
}
return cfg;
}
private GeneralPreferencesInfo parse(@Nullable GeneralPreferencesInfo input) {
private GeneralPreferencesInfo parseGeneralPreferences(@Nullable GeneralPreferencesInfo input) {
try {
return parse(cfg, defaultCfg, input);
return parseGeneralPreferences(cfg, defaultCfg, input);
} catch (ConfigInvalidException e) {
validationErrorSink.error(
new ValidationError(
@ -118,7 +153,20 @@ public class PreferencesConfig {
}
}
private static GeneralPreferencesInfo parse(
private DiffPreferencesInfo parseDiffPreferences(@Nullable DiffPreferencesInfo input) {
try {
return parseDiffPreferences(cfg, defaultCfg, input);
} catch (ConfigInvalidException e) {
validationErrorSink.error(
new ValidationError(
PREFERENCES_CONFIG,
String.format(
"Invalid diff preferences for account %d: %s", accountId.get(), e.getMessage())));
return new DiffPreferencesInfo();
}
}
private static GeneralPreferencesInfo parseGeneralPreferences(
Config cfg, @Nullable Config defaultCfg, @Nullable GeneralPreferencesInfo input)
throws ConfigInvalidException {
GeneralPreferencesInfo r =
@ -128,7 +176,7 @@ public class PreferencesConfig {
null,
new GeneralPreferencesInfo(),
defaultCfg != null
? parseDefaultPreferences(defaultCfg, input)
? parseDefaultGeneralPreferences(defaultCfg, input)
: GeneralPreferencesInfo.defaults(),
input);
if (input != null) {
@ -143,7 +191,21 @@ public class PreferencesConfig {
return r;
}
private static GeneralPreferencesInfo parseDefaultPreferences(
private static DiffPreferencesInfo parseDiffPreferences(
Config cfg, @Nullable Config defaultCfg, @Nullable DiffPreferencesInfo input)
throws ConfigInvalidException {
return loadSection(
cfg,
UserConfigSections.DIFF,
null,
new DiffPreferencesInfo(),
defaultCfg != null
? parseDefaultDiffPreferences(defaultCfg, input)
: DiffPreferencesInfo.defaults(),
input);
}
private static GeneralPreferencesInfo parseDefaultGeneralPreferences(
Config defaultCfg, GeneralPreferencesInfo input) throws ConfigInvalidException {
GeneralPreferencesInfo allUserPrefs = new GeneralPreferencesInfo();
loadSection(
@ -153,10 +215,24 @@ public class PreferencesConfig {
allUserPrefs,
GeneralPreferencesInfo.defaults(),
input);
return updateDefaults(allUserPrefs);
return updateGeneralPreferencesDefaults(allUserPrefs);
}
private static GeneralPreferencesInfo updateDefaults(GeneralPreferencesInfo input) {
private static DiffPreferencesInfo parseDefaultDiffPreferences(
Config defaultCfg, DiffPreferencesInfo input) throws ConfigInvalidException {
DiffPreferencesInfo allUserPrefs = new DiffPreferencesInfo();
loadSection(
defaultCfg,
UserConfigSections.DIFF,
null,
allUserPrefs,
DiffPreferencesInfo.defaults(),
input);
return updateDiffPreferencesDefaults(allUserPrefs);
}
private static GeneralPreferencesInfo updateGeneralPreferencesDefaults(
GeneralPreferencesInfo input) {
GeneralPreferencesInfo result = GeneralPreferencesInfo.defaults();
try {
for (Field field : input.getClass().getDeclaredFields()) {
@ -175,6 +251,25 @@ public class PreferencesConfig {
return result;
}
private static DiffPreferencesInfo updateDiffPreferencesDefaults(DiffPreferencesInfo input) {
DiffPreferencesInfo result = DiffPreferencesInfo.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 diff preferences", e);
return DiffPreferencesInfo.defaults();
}
return result;
}
private static List<String> parseChangeTableColumns(Config cfg, @Nullable Config defaultCfg) {
List<String> changeTable = changeTable(cfg);
if (changeTable == null && defaultCfg != null) {
@ -207,9 +302,14 @@ public class PreferencesConfig {
return urlAliases;
}
public static GeneralPreferencesInfo readDefaultPreferences(Repository allUsersRepo)
public static GeneralPreferencesInfo readDefaultGeneralPreferences(Repository allUsersRepo)
throws IOException, ConfigInvalidException {
return parse(readDefaultConfig(allUsersRepo), null, null);
return parseGeneralPreferences(readDefaultConfig(allUsersRepo), null, null);
}
public static DiffPreferencesInfo readDefaultDiffPreferences(Repository allUsersRepo)
throws IOException, ConfigInvalidException {
return parseDiffPreferences(readDefaultConfig(allUsersRepo), null, null);
}
static Config readDefaultConfig(Repository allUsersRepo)
@ -219,7 +319,7 @@ public class PreferencesConfig {
return defaultPrefs.getConfig();
}
public static GeneralPreferencesInfo updateDefaultPreferences(
public static GeneralPreferencesInfo updateDefaultGeneralPreferences(
MetaDataUpdate md, GeneralPreferencesInfo input) throws IOException, ConfigInvalidException {
VersionedDefaultPreferences defaultPrefs = new VersionedDefaultPreferences();
defaultPrefs.load(md);
@ -234,7 +334,22 @@ public class PreferencesConfig {
setUrlAliases(defaultPrefs.getConfig(), input.urlAliases);
defaultPrefs.commit(md);
return parse(defaultPrefs.getConfig(), null, null);
return parseGeneralPreferences(defaultPrefs.getConfig(), null, null);
}
public static DiffPreferencesInfo updateDefaultDiffPreferences(
MetaDataUpdate md, DiffPreferencesInfo input) throws IOException, ConfigInvalidException {
VersionedDefaultPreferences defaultPrefs = new VersionedDefaultPreferences();
defaultPrefs.load(md);
storeSection(
defaultPrefs.getConfig(),
UserConfigSections.DIFF,
null,
input,
DiffPreferencesInfo.defaults());
defaultPrefs.commit(md);
return parseDiffPreferences(defaultPrefs.getConfig(), null, null);
}
private static List<String> changeTable(Config cfg) {

View File

@ -14,19 +14,13 @@
package com.google.gerrit.server.restapi.account;
import static com.google.gerrit.server.config.ConfigUtil.loadSection;
import static com.google.gerrit.server.config.ConfigUtil.skipField;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestReadView;
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.VersionedAccountPreferences;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.GitRepositoryManager;
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;
@ -34,32 +28,20 @@ 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.eclipse.jgit.lib.Repository;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@Singleton
public class GetDiffPreferences implements RestReadView<AccountResource> {
private static final Logger log = LoggerFactory.getLogger(GetDiffPreferences.class);
private final Provider<CurrentUser> self;
private final Provider<AllUsersName> allUsersName;
private final PermissionBackend permissionBackend;
private final GitRepositoryManager gitMgr;
private final AccountCache accountCache;
@Inject
GetDiffPreferences(
Provider<CurrentUser> self,
Provider<AllUsersName> allUsersName,
PermissionBackend permissionBackend,
GitRepositoryManager gitMgr) {
Provider<CurrentUser> self, PermissionBackend permissionBackend, AccountCache accountCache) {
this.self = self;
this.allUsersName = allUsersName;
this.permissionBackend = permissionBackend;
this.gitMgr = gitMgr;
this.accountCache = accountCache;
}
@Override
@ -70,53 +52,6 @@ public class GetDiffPreferences implements RestReadView<AccountResource> {
}
Account.Id id = rsrc.getUser().getAccountId();
return readFromGit(id, gitMgr, allUsersName.get(), null);
}
static DiffPreferencesInfo readFromGit(
Account.Id id, GitRepositoryManager gitMgr, AllUsersName allUsersName, DiffPreferencesInfo in)
throws IOException, ConfigInvalidException, RepositoryNotFoundException {
try (Repository git = gitMgr.openRepository(allUsersName)) {
VersionedAccountPreferences p = VersionedAccountPreferences.forUser(id);
p.load(git);
DiffPreferencesInfo prefs = new DiffPreferencesInfo();
loadSection(
p.getConfig(), UserConfigSections.DIFF, null, prefs, readDefaultsFromGit(git, in), in);
return prefs;
}
}
static DiffPreferencesInfo readDefaultsFromGit(Repository git, DiffPreferencesInfo in)
throws ConfigInvalidException, IOException {
VersionedAccountPreferences dp = VersionedAccountPreferences.forDefault();
dp.load(git);
DiffPreferencesInfo allUserPrefs = new DiffPreferencesInfo();
loadSection(
dp.getConfig(),
UserConfigSections.DIFF,
null,
allUserPrefs,
DiffPreferencesInfo.defaults(),
in);
return updateDefaults(allUserPrefs);
}
private static DiffPreferencesInfo updateDefaults(DiffPreferencesInfo input) {
DiffPreferencesInfo result = DiffPreferencesInfo.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.warn("Cannot get default diff preferences from All-Users", e);
return DiffPreferencesInfo.defaults();
}
return result;
return accountCache.get(id).getDiffPreferences();
}
}

View File

@ -14,26 +14,19 @@
package com.google.gerrit.server.restapi.account;
import static com.google.gerrit.server.config.ConfigUtil.loadSection;
import static com.google.gerrit.server.config.ConfigUtil.storeSection;
import static com.google.gerrit.server.restapi.account.GetDiffPreferences.readDefaultsFromGit;
import static com.google.gerrit.server.restapi.account.GetDiffPreferences.readFromGit;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestModifyView;
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.VersionedAccountPreferences;
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.UserConfigSections;
import com.google.gerrit.server.account.AccountsUpdate;
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;
@ -44,52 +37,38 @@ import org.eclipse.jgit.errors.RepositoryNotFoundException;
@Singleton
public class SetDiffPreferences implements RestModifyView<AccountResource, DiffPreferencesInfo> {
private final Provider<CurrentUser> self;
private final Provider<MetaDataUpdate.User> metaDataUpdateFactory;
private final AllUsersName allUsersName;
private final PermissionBackend permissionBackend;
private final GitRepositoryManager gitMgr;
private final AccountCache accountCache;
private final AccountsUpdate.User accountsUpdate;
@Inject
SetDiffPreferences(
Provider<CurrentUser> self,
Provider<MetaDataUpdate.User> metaDataUpdateFactory,
AllUsersName allUsersName,
PermissionBackend permissionBackend,
GitRepositoryManager gitMgr) {
AccountCache accountCache,
AccountsUpdate.User accountsUpdate) {
this.self = self;
this.metaDataUpdateFactory = metaDataUpdateFactory;
this.allUsersName = allUsersName;
this.permissionBackend = permissionBackend;
this.gitMgr = gitMgr;
this.accountCache = accountCache;
this.accountsUpdate = accountsUpdate;
}
@Override
public DiffPreferencesInfo apply(AccountResource rsrc, DiffPreferencesInfo in)
public DiffPreferencesInfo apply(AccountResource rsrc, DiffPreferencesInfo input)
throws AuthException, BadRequestException, ConfigInvalidException,
RepositoryNotFoundException, IOException, PermissionBackendException {
RepositoryNotFoundException, IOException, PermissionBackendException, OrmException {
if (self.get() != rsrc.getUser()) {
permissionBackend.user(self).check(GlobalPermission.MODIFY_ACCOUNT);
}
if (in == null) {
if (input == null) {
throw new BadRequestException("input must be provided");
}
Account.Id id = rsrc.getUser().getAccountId();
return writeToGit(readFromGit(id, gitMgr, allUsersName, in), id);
}
private DiffPreferencesInfo writeToGit(DiffPreferencesInfo in, Account.Id userId)
throws RepositoryNotFoundException, IOException, ConfigInvalidException {
DiffPreferencesInfo out = new DiffPreferencesInfo();
try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) {
DiffPreferencesInfo allUserPrefs = readDefaultsFromGit(md.getRepository(), null);
VersionedAccountPreferences prefs = VersionedAccountPreferences.forUser(userId);
prefs.load(md);
storeSection(prefs.getConfig(), UserConfigSections.DIFF, null, in, allUserPrefs);
prefs.commit(md);
loadSection(prefs.getConfig(), UserConfigSections.DIFF, null, out, allUserPrefs, null);
}
return out;
accountsUpdate
.create()
.update("Set Diff Preferences via API", id, u -> u.setDiffPreferences(input));
return accountCache.get(id).getDiffPreferences();
}
}

View File

@ -73,7 +73,7 @@ public class SetPreferences implements RestModifyView<AccountResource, GeneralPr
accountsUpdate
.create()
.update("Set Preferences via API", id, u -> u.setGeneralPreferences(input));
.update("Set General Preferences via API", id, u -> u.setGeneralPreferences(input));
return cache.get(id).getGeneralPreferences();
}

View File

@ -14,22 +14,18 @@
package com.google.gerrit.server.restapi.config;
import static com.google.gerrit.server.config.ConfigUtil.loadSection;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.server.account.VersionedAccountPreferences;
import com.google.gerrit.server.account.PreferencesConfig;
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
@ -47,25 +43,8 @@ public class GetDiffPreferences implements RestReadView<ConfigResource> {
@Override
public DiffPreferencesInfo apply(ConfigResource configResource)
throws BadRequestException, ResourceConflictException, IOException, ConfigInvalidException {
return readFromGit(gitManager, allUsersName, null);
}
static DiffPreferencesInfo readFromGit(
GitRepositoryManager gitMgr, AllUsersName allUsersName, DiffPreferencesInfo in)
throws IOException, ConfigInvalidException, RepositoryNotFoundException {
try (Repository git = gitMgr.openRepository(allUsersName)) {
// Load all users prefs.
VersionedAccountPreferences dp = VersionedAccountPreferences.forDefault();
dp.load(git);
DiffPreferencesInfo allUserPrefs = new DiffPreferencesInfo();
loadSection(
dp.getConfig(),
UserConfigSections.DIFF,
null,
allUserPrefs,
DiffPreferencesInfo.defaults(),
in);
return allUserPrefs;
try (Repository git = gitManager.openRepository(allUsersName)) {
return PreferencesConfig.readDefaultDiffPreferences(git);
}
}
}

View File

@ -41,7 +41,7 @@ public class GetPreferences implements RestReadView<ConfigResource> {
public GeneralPreferencesInfo apply(ConfigResource rsrc)
throws IOException, ConfigInvalidException {
try (Repository git = gitMgr.openRepository(allUsersName)) {
return PreferencesConfig.readDefaultPreferences(git);
return PreferencesConfig.readDefaultGeneralPreferences(git);
}
}
}

View File

@ -14,28 +14,24 @@
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 com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.account.VersionedAccountPreferences;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.PreferencesConfig;
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;
@ -46,48 +42,33 @@ public class SetDiffPreferences implements RestModifyView<ConfigResource, DiffPr
private final Provider<MetaDataUpdate.User> metaDataUpdateFactory;
private final AllUsersName allUsersName;
private final GitRepositoryManager gitManager;
private final AccountCache accountCache;
@Inject
SetDiffPreferences(
GitRepositoryManager gitManager,
Provider<MetaDataUpdate.User> metaDataUpdateFactory,
AllUsersName allUsersName) {
this.gitManager = gitManager;
AllUsersName allUsersName,
AccountCache accountCache) {
this.metaDataUpdateFactory = metaDataUpdateFactory;
this.allUsersName = allUsersName;
this.accountCache = accountCache;
}
@Override
public DiffPreferencesInfo apply(ConfigResource configResource, DiffPreferencesInfo in)
public DiffPreferencesInfo apply(ConfigResource configResource, DiffPreferencesInfo input)
throws BadRequestException, IOException, ConfigInvalidException {
if (in == null) {
if (input == null) {
throw new BadRequestException("input must be provided");
}
if (!hasSetFields(in)) {
if (!hasSetFields(input)) {
throw new BadRequestException("unsupported option");
}
return writeToGit(GetDiffPreferences.readFromGit(gitManager, allUsersName, in));
}
private DiffPreferencesInfo writeToGit(DiffPreferencesInfo in)
throws RepositoryNotFoundException, IOException, ConfigInvalidException {
DiffPreferencesInfo out = new DiffPreferencesInfo();
try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) {
VersionedAccountPreferences prefs = VersionedAccountPreferences.forDefault();
prefs.load(md);
DiffPreferencesInfo defaults = DiffPreferencesInfo.defaults();
storeSection(prefs.getConfig(), UserConfigSections.DIFF, null, in, defaults);
prefs.commit(md);
loadSection(
prefs.getConfig(),
UserConfigSections.DIFF,
null,
out,
DiffPreferencesInfo.defaults(),
null);
DiffPreferencesInfo updatedPrefs = PreferencesConfig.updateDefaultDiffPreferences(md, input);
accountCache.evictAllNoReindex();
return updatedPrefs;
}
return out;
}
private static boolean hasSetFields(DiffPreferencesInfo in) {

View File

@ -62,7 +62,8 @@ public class SetPreferences implements RestModifyView<ConfigResource, GeneralPre
}
PreferencesConfig.validateMy(input.my);
try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) {
GeneralPreferencesInfo updatedPrefs = PreferencesConfig.updateDefaultPreferences(md, input);
GeneralPreferencesInfo updatedPrefs =
PreferencesConfig.updateDefaultGeneralPreferences(md, input);
accountCache.evictAllNoReindex();
return updatedPrefs;
}