diff --git a/java/com/google/gerrit/server/account/AccountConfig.java b/java/com/google/gerrit/server/account/AccountConfig.java index 5263bad9efa..441391f5939 100644 --- a/java/com/google/gerrit/server/account/AccountConfig.java +++ b/java/com/google/gerrit/server/account/AccountConfig.java @@ -68,7 +68,7 @@ *
  • 'account.config': Contains the account properties. Parsing and writing it is delegated to * {@link AccountProperties}. *
  • 'preferences.config': Contains the preferences. Parsing and writing it is delegated to - * {@link Preferences}. + * {@link StoredPreferences}. *
  • 'account.config': Contains the project watches. Parsing and writing it is delegated to * {@link ProjectWatches}. * @@ -85,7 +85,7 @@ public class AccountConfig extends VersionedMetaData implements ValidationError. private Optional loadedAccountProperties; private Optional externalIdsRev; private ProjectWatches projectWatches; - private Preferences preferences; + private StoredPreferences preferences; private Optional accountUpdate = Optional.empty(); private List validationErrors; @@ -242,10 +242,10 @@ protected void onLoad() throws IOException, ConfigInvalidException { projectWatches = new ProjectWatches(accountId, readConfig(ProjectWatches.WATCH_CONFIG), this); preferences = - new Preferences( + new StoredPreferences( accountId, - readConfig(Preferences.PREFERENCES_CONFIG), - Preferences.readDefaultConfig(allUsersName, repo), + readConfig(StoredPreferences.PREFERENCES_CONFIG), + StoredPreferences.readDefaultConfig(allUsersName, repo), this); projectWatches.parse(); @@ -256,8 +256,11 @@ protected void onLoad() throws IOException, ConfigInvalidException { projectWatches = new ProjectWatches(accountId, new Config(), this); preferences = - new Preferences( - accountId, new Config(), Preferences.readDefaultConfig(allUsersName, repo), this); + new StoredPreferences( + accountId, + new Config(), + StoredPreferences.readDefaultConfig(allUsersName, repo), + this); } Ref externalIdsRef = repo.exactRef(RefNames.REFS_EXTERNAL_IDS); @@ -331,7 +334,7 @@ private void savePreferences() throws IOException, ConfigInvalidException { } saveConfig( - Preferences.PREFERENCES_CONFIG, + StoredPreferences.PREFERENCES_CONFIG, preferences.saveGeneralPreferences( accountUpdate.get().getGeneralPreferences(), accountUpdate.get().getDiffPreferences(), diff --git a/java/com/google/gerrit/server/account/AccountState.java b/java/com/google/gerrit/server/account/AccountState.java index 6eb6ca101de..dd81c93ca95 100644 --- a/java/com/google/gerrit/server/account/AccountState.java +++ b/java/com/google/gerrit/server/account/AccountState.java @@ -131,9 +131,9 @@ public static AccountState forAccount(Account account, Collection ex private final ImmutableSet externalIds; private final Optional userName; private final ImmutableMap> projectWatches; - private final GeneralPreferencesInfo generalPreferences; - private final DiffPreferencesInfo diffPreferences; - private final EditPreferencesInfo editPreferences; + private final Preferences.General generalPreferences; + private final Preferences.Diff diffPreferences; + private final Preferences.Edit editPreferences; private AccountState( Account account, @@ -146,9 +146,9 @@ private AccountState( this.externalIds = externalIds; this.userName = ExternalId.getUserName(externalIds); this.projectWatches = projectWatches; - this.generalPreferences = generalPreferences; - this.diffPreferences = diffPreferences; - this.editPreferences = editPreferences; + this.generalPreferences = Preferences.General.fromInfo(generalPreferences); + this.diffPreferences = Preferences.Diff.fromInfo(diffPreferences); + this.editPreferences = Preferences.Edit.fromInfo(editPreferences); } /** Get the cached account metadata. */ @@ -180,17 +180,17 @@ public ImmutableMap> getProjectWatches /** The general preferences of the account. */ public GeneralPreferencesInfo getGeneralPreferences() { - return generalPreferences; + return generalPreferences.toInfo(); } /** The diff preferences of the account. */ public DiffPreferencesInfo getDiffPreferences() { - return diffPreferences; + return diffPreferences.toInfo(); } /** The edit preferences of the account. */ public EditPreferencesInfo getEditPreferences() { - return editPreferences; + return editPreferences.toInfo(); } @Override diff --git a/java/com/google/gerrit/server/account/AccountsUpdate.java b/java/com/google/gerrit/server/account/AccountsUpdate.java index 2920cefac5b..db494674237 100644 --- a/java/com/google/gerrit/server/account/AccountsUpdate.java +++ b/java/com/google/gerrit/server/account/AccountsUpdate.java @@ -88,9 +88,9 @@ * The timestamp of the first commit on a user branch denotes the registration date. The initial * commit on the user branch may be empty (since having an 'account.config' is optional). See {@link * AccountConfig} for details of the 'account.config' file format. In addition the user branch can - * contain a 'preferences.config' config file to store preferences (see {@link Preferences}) and a - * 'watch.config' config file to store project watches (see {@link ProjectWatches}). External IDs - * are stored separately in the {@code refs/meta/external-ids} notes branch (see {@link + * contain a 'preferences.config' config file to store preferences (see {@link StoredPreferences}) + * and a 'watch.config' config file to store project watches (see {@link ProjectWatches}). External + * IDs are stored separately in the {@code refs/meta/external-ids} notes branch (see {@link * ExternalIdNotes}). * *

    On updating an account the account is evicted from the account cache and reindexed. The diff --git a/java/com/google/gerrit/server/account/Preferences.java b/java/com/google/gerrit/server/account/Preferences.java index bba5843e6db..63ff9e3e37a 100644 --- a/java/com/google/gerrit/server/account/Preferences.java +++ b/java/com/google/gerrit/server/account/Preferences.java @@ -1,4 +1,4 @@ -// Copyright (C) 2018 The Android Open Source Project +// Copyright (C) 2019 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. @@ -11,606 +11,453 @@ // 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.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 static java.util.Objects.requireNonNull; - -import com.google.common.base.Strings; -import com.google.common.collect.Lists; -import com.google.common.flogger.FluentLogger; +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.client.DiffPreferencesInfo; +import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace; import com.google.gerrit.extensions.client.EditPreferencesInfo; import com.google.gerrit.extensions.client.GeneralPreferencesInfo; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DateFormat; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DefaultBase; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DiffView; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo.DownloadCommand; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailFormat; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo.TimeFormat; +import com.google.gerrit.extensions.client.KeyMapType; 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.config.AllUsersName; -import com.google.gerrit.server.git.UserConfigSections; -import com.google.gerrit.server.git.ValidationError; -import com.google.gerrit.server.git.meta.MetaDataUpdate; -import com.google.gerrit.server.git.meta.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 com.google.gerrit.extensions.client.Theme; import java.util.Optional; -import org.eclipse.jgit.errors.ConfigInvalidException; -import org.eclipse.jgit.lib.CommitBuilder; -import org.eclipse.jgit.lib.Config; -import org.eclipse.jgit.lib.Repository; - -/** - * Parses/writes preferences from/to a {@link Config} file. - * - *

    This is a low-level API. Read/write of preferences in a user branch should be done through - * {@link AccountsUpdate} or {@link AccountConfig}. - * - *

    The config file has separate sections for general, diff and edit preferences: - * - *

    - *   [diff]
    - *     hideTopMenu = true
    - *   [edit]
    - *     lineLength = 80
    - * 
    - * - *

    The parameter names match the names that are used in the preferences REST API. - * - *

    If the preference is omitted in the config file, then the default value for the preference is - * used. - * - *

    Defaults for preferences that apply for all accounts can be configured in the {@code - * refs/users/default} branch in the {@code All-Users} repository. The config for the default - * preferences must be provided to this class so that it can read default values from it. - * - *

    The preferences are lazily parsed. - */ -public class Preferences { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - - 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; - private DiffPreferencesInfo diffPreferences; - private EditPreferencesInfo editPreferences; - - Preferences( - Account.Id accountId, - Config cfg, - Config defaultCfg, - ValidationError.Sink validationErrorSink) { - this.accountId = requireNonNull(accountId, "accountId"); - this.cfg = requireNonNull(cfg, "cfg"); - this.defaultCfg = requireNonNull(defaultCfg, "defaultCfg"); - this.validationErrorSink = requireNonNull(validationErrorSink, "validationErrorSink"); - } - public GeneralPreferencesInfo getGeneralPreferences() { - if (generalPreferences == null) { - parse(); - } - return generalPreferences; - } +@AutoValue +public abstract class Preferences { + @AutoValue + public abstract static class General { + public abstract Optional changesPerPage(); - public DiffPreferencesInfo getDiffPreferences() { - if (diffPreferences == null) { - parse(); - } - return diffPreferences; - } + public abstract Optional downloadScheme(); - public EditPreferencesInfo getEditPreferences() { - if (editPreferences == null) { - parse(); - } - return editPreferences; - } + public abstract Optional downloadCommand(); - public void parse() { - generalPreferences = parseGeneralPreferences(null); - diffPreferences = parseDiffPreferences(null); - editPreferences = parseEditPreferences(null); - } + public abstract Optional dateFormat(); - public Config saveGeneralPreferences( - Optional generalPreferencesInput, - Optional diffPreferencesInput, - Optional editPreferencesInput) - throws ConfigInvalidException { - if (generalPreferencesInput.isPresent()) { - GeneralPreferencesInfo mergedGeneralPreferencesInput = - parseGeneralPreferences(generalPreferencesInput.get()); - - 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; - } + public abstract Optional timeFormat(); - if (diffPreferencesInput.isPresent()) { - DiffPreferencesInfo mergedDiffPreferencesInput = - parseDiffPreferences(diffPreferencesInput.get()); + public abstract Optional expandInlineDiffs(); - storeSection( - cfg, - UserConfigSections.DIFF, - null, - mergedDiffPreferencesInput, - parseDefaultDiffPreferences(defaultCfg, null)); + public abstract Optional highlightAssigneeInChangeTable(); - // evict the cached diff preferences - this.diffPreferences = null; - } + public abstract Optional relativeDateInChangeTable(); - if (editPreferencesInput.isPresent()) { - EditPreferencesInfo mergedEditPreferencesInput = - parseEditPreferences(editPreferencesInput.get()); + public abstract Optional diffView(); - storeSection( - cfg, - UserConfigSections.EDIT, - null, - mergedEditPreferencesInput, - parseDefaultEditPreferences(defaultCfg, null)); + public abstract Optional sizeBarInChangeTable(); - // evict the cached edit preferences - this.editPreferences = null; - } + public abstract Optional legacycidInChangeTable(); - return cfg; - } + public abstract Optional muteCommonPathPrefixes(); - private GeneralPreferencesInfo parseGeneralPreferences(@Nullable GeneralPreferencesInfo input) { - try { - return parseGeneralPreferences(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(); - } - } + public abstract Optional signedOffBy(); - 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(); - } - } + public abstract Optional emailStrategy(); - private EditPreferencesInfo parseEditPreferences(@Nullable EditPreferencesInfo input) { - try { - return parseEditPreferences(cfg, defaultCfg, input); - } catch (ConfigInvalidException e) { - validationErrorSink.error( - new ValidationError( - PREFERENCES_CONFIG, - String.format( - "Invalid edit preferences for account %d: %s", accountId.get(), e.getMessage()))); - return new EditPreferencesInfo(); - } - } + public abstract Optional emailFormat(); - private static GeneralPreferencesInfo parseGeneralPreferences( - Config cfg, @Nullable Config defaultCfg, @Nullable GeneralPreferencesInfo input) - throws ConfigInvalidException { - GeneralPreferencesInfo r = - loadSection( - cfg, - UserConfigSections.GENERAL, - null, - new GeneralPreferencesInfo(), - defaultCfg != null - ? parseDefaultGeneralPreferences(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; - } + public abstract Optional defaultBaseForMerges(); - 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); - } + public abstract Optional publishCommentsOnPush(); - private static EditPreferencesInfo parseEditPreferences( - Config cfg, @Nullable Config defaultCfg, @Nullable EditPreferencesInfo input) - throws ConfigInvalidException { - return loadSection( - cfg, - UserConfigSections.EDIT, - null, - new EditPreferencesInfo(), - defaultCfg != null - ? parseDefaultEditPreferences(defaultCfg, input) - : EditPreferencesInfo.defaults(), - input); - } + public abstract Optional workInProgressByDefault(); - private static GeneralPreferencesInfo parseDefaultGeneralPreferences( - Config defaultCfg, GeneralPreferencesInfo input) throws ConfigInvalidException { - GeneralPreferencesInfo allUserPrefs = new GeneralPreferencesInfo(); - loadSection( - defaultCfg, - UserConfigSections.GENERAL, - null, - allUserPrefs, - GeneralPreferencesInfo.defaults(), - input); - return updateGeneralPreferencesDefaults(allUserPrefs); - } + public abstract Optional> my(); - 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); - } + public abstract Optional> changeTable(); - private static EditPreferencesInfo parseDefaultEditPreferences( - Config defaultCfg, EditPreferencesInfo input) throws ConfigInvalidException { - EditPreferencesInfo allUserPrefs = new EditPreferencesInfo(); - loadSection( - defaultCfg, - UserConfigSections.EDIT, - null, - allUserPrefs, - EditPreferencesInfo.defaults(), - input); - return updateEditPreferencesDefaults(allUserPrefs); - } + public abstract Optional> urlAliases(); - private static GeneralPreferencesInfo updateGeneralPreferencesDefaults( - 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) { - logger.atSevere().withCause(e).log("Failed to apply default general preferences"); - return GeneralPreferencesInfo.defaults(); - } - return result; - } + @AutoValue.Builder + public abstract static class Builder { + abstract Builder changesPerPage(@Nullable Integer val); - 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) { - logger.atSevere().withCause(e).log("Failed to apply default diff preferences"); - return DiffPreferencesInfo.defaults(); - } - return result; - } + abstract Builder downloadScheme(@Nullable String val); - private static EditPreferencesInfo updateEditPreferencesDefaults(EditPreferencesInfo input) { - EditPreferencesInfo result = EditPreferencesInfo.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) { - logger.atSevere().withCause(e).log("Failed to apply default edit preferences"); - return EditPreferencesInfo.defaults(); - } - return result; - } + abstract Builder downloadCommand(@Nullable DownloadCommand val); - private static List parseChangeTableColumns(Config cfg, @Nullable Config defaultCfg) { - List changeTable = changeTable(cfg); - if (changeTable == null && defaultCfg != null) { - changeTable = changeTable(defaultCfg); - } - return changeTable; - } + abstract Builder dateFormat(@Nullable DateFormat val); - private static List parseMyMenus(Config cfg, @Nullable Config defaultCfg) { - List 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", "#/settings/#Groups", null)); - } - return my; - } + abstract Builder timeFormat(@Nullable TimeFormat val); - private static Map parseUrlAliases(Config cfg, @Nullable Config defaultCfg) { - Map urlAliases = urlAliases(cfg); - if (urlAliases == null && defaultCfg != null) { - urlAliases = urlAliases(defaultCfg); - } - return urlAliases; - } + abstract Builder expandInlineDiffs(@Nullable Boolean val); - public static GeneralPreferencesInfo readDefaultGeneralPreferences( - AllUsersName allUsersName, Repository allUsersRepo) - throws IOException, ConfigInvalidException { - return parseGeneralPreferences(readDefaultConfig(allUsersName, allUsersRepo), null, null); - } + abstract Builder highlightAssigneeInChangeTable(@Nullable Boolean val); - public static DiffPreferencesInfo readDefaultDiffPreferences( - AllUsersName allUsersName, Repository allUsersRepo) - throws IOException, ConfigInvalidException { - return parseDiffPreferences(readDefaultConfig(allUsersName, allUsersRepo), null, null); - } + abstract Builder relativeDateInChangeTable(@Nullable Boolean val); - public static EditPreferencesInfo readDefaultEditPreferences( - AllUsersName allUsersName, Repository allUsersRepo) - throws IOException, ConfigInvalidException { - return parseEditPreferences(readDefaultConfig(allUsersName, allUsersRepo), null, null); - } + abstract Builder diffView(@Nullable DiffView val); - static Config readDefaultConfig(AllUsersName allUsersName, Repository allUsersRepo) - throws IOException, ConfigInvalidException { - VersionedDefaultPreferences defaultPrefs = new VersionedDefaultPreferences(); - defaultPrefs.load(allUsersName, allUsersRepo); - return defaultPrefs.getConfig(); - } + abstract Builder sizeBarInChangeTable(@Nullable Boolean val); - public static GeneralPreferencesInfo updateDefaultGeneralPreferences( - 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 parseGeneralPreferences(defaultPrefs.getConfig(), null, null); - } + abstract Builder legacycidInChangeTable(@Nullable Boolean val); - 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); - } + abstract Builder muteCommonPathPrefixes(@Nullable Boolean val); - public static EditPreferencesInfo updateDefaultEditPreferences( - MetaDataUpdate md, EditPreferencesInfo input) throws IOException, ConfigInvalidException { - VersionedDefaultPreferences defaultPrefs = new VersionedDefaultPreferences(); - defaultPrefs.load(md); - storeSection( - defaultPrefs.getConfig(), - UserConfigSections.EDIT, - null, - input, - EditPreferencesInfo.defaults()); - defaultPrefs.commit(md); - - return parseEditPreferences(defaultPrefs.getConfig(), null, null); - } + abstract Builder signedOffBy(@Nullable Boolean val); - private static List changeTable(Config cfg) { - return Lists.newArrayList(cfg.getStringList(CHANGE_TABLE, null, CHANGE_TABLE_COLUMN)); - } + abstract Builder emailStrategy(@Nullable EmailStrategy val); - private static void setChangeTable(Config cfg, List changeTable) { - if (changeTable != null) { - unsetSection(cfg, UserConfigSections.CHANGE_TABLE); - cfg.setStringList(UserConfigSections.CHANGE_TABLE, null, CHANGE_TABLE_COLUMN, changeTable); - } - } + abstract Builder emailFormat(@Nullable EmailFormat val); - private static List my(Config cfg) { - List 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; - } + abstract Builder defaultBaseForMerges(@Nullable DefaultBase val); - 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; - } + abstract Builder publishCommentsOnPush(@Nullable Boolean val); - private static void setMy(Config cfg, List 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); - } - } - } + abstract Builder workInProgressByDefault(@Nullable Boolean val); - public static void validateMy(List my) throws BadRequestException { - if (my == null) { - return; + abstract Builder my(@Nullable ImmutableList val); + + abstract Builder changeTable(@Nullable ImmutableList val); + + abstract Builder urlAliases(@Nullable ImmutableMap val); + + abstract General build(); } - for (MenuItem item : my) { - checkRequiredMenuItemField(item.name, "name"); - checkRequiredMenuItemField(item.url, "URL"); + + public static General fromInfo(GeneralPreferencesInfo info) { + return (new AutoValue_Preferences_General.Builder()) + .changesPerPage(info.changesPerPage) + .downloadScheme(info.downloadScheme) + .downloadCommand(info.downloadCommand) + .dateFormat(info.dateFormat) + .timeFormat(info.timeFormat) + .expandInlineDiffs(info.expandInlineDiffs) + .highlightAssigneeInChangeTable(info.highlightAssigneeInChangeTable) + .relativeDateInChangeTable(info.relativeDateInChangeTable) + .diffView(info.diffView) + .sizeBarInChangeTable(info.sizeBarInChangeTable) + .legacycidInChangeTable(info.legacycidInChangeTable) + .muteCommonPathPrefixes(info.muteCommonPathPrefixes) + .signedOffBy(info.signedOffBy) + .emailStrategy(info.emailStrategy) + .emailFormat(info.emailFormat) + .defaultBaseForMerges(info.defaultBaseForMerges) + .publishCommentsOnPush(info.publishCommentsOnPush) + .workInProgressByDefault(info.workInProgressByDefault) + .my(info.my == null ? null : ImmutableList.copyOf(info.my)) + .changeTable(info.changeTable == null ? null : ImmutableList.copyOf(info.changeTable)) + .urlAliases(info.urlAliases == null ? null : ImmutableMap.copyOf(info.urlAliases)) + .build(); } - } - private static void checkRequiredMenuItemField(String value, String name) - throws BadRequestException { - if (isNullOrEmpty(value)) { - throw new BadRequestException(name + " for menu item is required"); + public GeneralPreferencesInfo toInfo() { + GeneralPreferencesInfo info = new GeneralPreferencesInfo(); + info.changesPerPage = changesPerPage().orElse(null); + info.downloadScheme = downloadScheme().orElse(null); + info.downloadCommand = downloadCommand().orElse(null); + info.dateFormat = dateFormat().orElse(null); + info.timeFormat = timeFormat().orElse(null); + info.expandInlineDiffs = expandInlineDiffs().orElse(null); + info.highlightAssigneeInChangeTable = highlightAssigneeInChangeTable().orElse(null); + info.relativeDateInChangeTable = relativeDateInChangeTable().orElse(null); + info.diffView = diffView().orElse(null); + info.sizeBarInChangeTable = sizeBarInChangeTable().orElse(null); + info.legacycidInChangeTable = legacycidInChangeTable().orElse(null); + info.muteCommonPathPrefixes = muteCommonPathPrefixes().orElse(null); + info.signedOffBy = signedOffBy().orElse(null); + info.emailStrategy = emailStrategy().orElse(null); + info.emailFormat = emailFormat().orElse(null); + info.defaultBaseForMerges = defaultBaseForMerges().orElse(null); + info.publishCommentsOnPush = publishCommentsOnPush().orElse(null); + info.workInProgressByDefault = workInProgressByDefault().orElse(null); + info.my = my().orElse(null); + info.changeTable = changeTable().orElse(null); + info.urlAliases = urlAliases().orElse(null); + return info; } } - private static boolean isNullOrEmpty(String value) { - return value == null || value.trim().isEmpty(); - } + @AutoValue + public abstract static class Edit { + public abstract Optional tabSize(); - 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()); - } - } + public abstract Optional lineLength(); + + public abstract Optional indentUnit(); + + public abstract Optional cursorBlinkRate(); + + public abstract Optional hideTopMenu(); + + public abstract Optional showTabs(); + + public abstract Optional showWhitespaceErrors(); + + public abstract Optional syntaxHighlighting(); + + public abstract Optional hideLineNumbers(); + + public abstract Optional matchBrackets(); + + public abstract Optional lineWrapping(); + + public abstract Optional indentWithTabs(); - private static Map urlAliases(Config cfg) { - HashMap 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)); + public abstract Optional autoCloseBrackets(); + + public abstract Optional showBase(); + + public abstract Optional theme(); + + public abstract Optional keyMapType(); + + @AutoValue.Builder + public abstract static class Builder { + abstract Builder tabSize(@Nullable Integer val); + + abstract Builder lineLength(@Nullable Integer val); + + abstract Builder indentUnit(@Nullable Integer val); + + abstract Builder cursorBlinkRate(@Nullable Integer val); + + abstract Builder hideTopMenu(@Nullable Boolean val); + + abstract Builder showTabs(@Nullable Boolean val); + + abstract Builder showWhitespaceErrors(@Nullable Boolean val); + + abstract Builder syntaxHighlighting(@Nullable Boolean val); + + abstract Builder hideLineNumbers(@Nullable Boolean val); + + abstract Builder matchBrackets(@Nullable Boolean val); + + abstract Builder lineWrapping(@Nullable Boolean val); + + abstract Builder indentWithTabs(@Nullable Boolean val); + + abstract Builder autoCloseBrackets(@Nullable Boolean val); + + abstract Builder showBase(@Nullable Boolean val); + + abstract Builder theme(@Nullable Theme val); + + abstract Builder keyMapType(@Nullable KeyMapType val); + + abstract Edit build(); } - return !urlAliases.isEmpty() ? urlAliases : null; - } - private static void setUrlAliases(Config cfg, Map urlAliases) { - if (urlAliases != null) { - for (String subsection : cfg.getSubsections(URL_ALIAS)) { - cfg.unsetSection(URL_ALIAS, subsection); - } - - int i = 1; - for (Map.Entry 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++; - } + public static Edit fromInfo(EditPreferencesInfo info) { + return (new AutoValue_Preferences_Edit.Builder()) + .tabSize(info.tabSize) + .lineLength(info.lineLength) + .indentUnit(info.indentUnit) + .cursorBlinkRate(info.cursorBlinkRate) + .hideTopMenu(info.hideTopMenu) + .showTabs(info.showTabs) + .showWhitespaceErrors(info.showWhitespaceErrors) + .syntaxHighlighting(info.syntaxHighlighting) + .hideLineNumbers(info.hideLineNumbers) + .matchBrackets(info.matchBrackets) + .lineWrapping(info.lineWrapping) + .indentWithTabs(info.indentWithTabs) + .autoCloseBrackets(info.autoCloseBrackets) + .showBase(info.showBase) + .theme(info.theme) + .keyMapType(info.keyMapType) + .build(); } - } - private static void unsetSection(Config cfg, String section) { - cfg.unsetSection(section, null); - for (String subsection : cfg.getSubsections(section)) { - cfg.unsetSection(section, subsection); + public EditPreferencesInfo toInfo() { + EditPreferencesInfo info = new EditPreferencesInfo(); + info.tabSize = tabSize().orElse(null); + info.lineLength = lineLength().orElse(null); + info.indentUnit = indentUnit().orElse(null); + info.cursorBlinkRate = cursorBlinkRate().orElse(null); + info.hideTopMenu = hideTopMenu().orElse(null); + info.showTabs = showTabs().orElse(null); + info.showWhitespaceErrors = showWhitespaceErrors().orElse(null); + info.syntaxHighlighting = syntaxHighlighting().orElse(null); + info.hideLineNumbers = hideLineNumbers().orElse(null); + info.matchBrackets = matchBrackets().orElse(null); + info.lineWrapping = lineWrapping().orElse(null); + info.indentWithTabs = indentWithTabs().orElse(null); + info.autoCloseBrackets = autoCloseBrackets().orElse(null); + info.showBase = showBase().orElse(null); + info.theme = theme().orElse(null); + info.keyMapType = keyMapType().orElse(null); + return info; } } - private static class VersionedDefaultPreferences extends VersionedMetaData { - private Config cfg; + @AutoValue + public abstract static class Diff { + public abstract Optional context(); - @Override - protected String getRefName() { - return RefNames.REFS_USERS_DEFAULT; - } + public abstract Optional tabSize(); + + public abstract Optional fontSize(); + + public abstract Optional lineLength(); + + public abstract Optional cursorBlinkRate(); + + public abstract Optional expandAllComments(); + + public abstract Optional intralineDifference(); + + public abstract Optional manualReview(); + + public abstract Optional showLineEndings(); + + public abstract Optional showTabs(); + + public abstract Optional showWhitespaceErrors(); + + public abstract Optional syntaxHighlighting(); + + public abstract Optional hideTopMenu(); + + public abstract Optional autoHideDiffTableHeader(); + + public abstract Optional hideLineNumbers(); + + public abstract Optional renderEntireFile(); + + public abstract Optional hideEmptyPane(); + + public abstract Optional matchBrackets(); + + public abstract Optional lineWrapping(); + + public abstract Optional theme(); + + public abstract Optional ignoreWhitespace(); + + public abstract Optional retainHeader(); + + public abstract Optional skipDeleted(); + + public abstract Optional skipUnchanged(); + + public abstract Optional skipUncommented(); + + @AutoValue.Builder + public abstract static class Builder { + abstract Builder context(@Nullable Integer val); + + abstract Builder tabSize(@Nullable Integer val); + + abstract Builder fontSize(@Nullable Integer val); + + abstract Builder lineLength(@Nullable Integer val); + + abstract Builder cursorBlinkRate(@Nullable Integer val); + + abstract Builder expandAllComments(@Nullable Boolean val); + + abstract Builder intralineDifference(@Nullable Boolean val); + + abstract Builder manualReview(@Nullable Boolean val); + + abstract Builder showLineEndings(@Nullable Boolean val); + + abstract Builder showTabs(@Nullable Boolean val); + + abstract Builder showWhitespaceErrors(@Nullable Boolean val); + + abstract Builder syntaxHighlighting(@Nullable Boolean val); + + abstract Builder hideTopMenu(@Nullable Boolean val); + + abstract Builder autoHideDiffTableHeader(@Nullable Boolean val); + + abstract Builder hideLineNumbers(@Nullable Boolean val); + + abstract Builder renderEntireFile(@Nullable Boolean val); + + abstract Builder hideEmptyPane(@Nullable Boolean val); + + abstract Builder matchBrackets(@Nullable Boolean val); + + abstract Builder lineWrapping(@Nullable Boolean val); + + abstract Builder theme(@Nullable Theme val); + + abstract Builder ignoreWhitespace(@Nullable Whitespace val); + + abstract Builder retainHeader(@Nullable Boolean val); + + abstract Builder skipDeleted(@Nullable Boolean val); + + abstract Builder skipUnchanged(@Nullable Boolean val); + + abstract Builder skipUncommented(@Nullable Boolean val); - private Config getConfig() { - checkState(cfg != null, "Default preferences not loaded yet."); - return cfg; + abstract Diff build(); } - @Override - protected void onLoad() throws IOException, ConfigInvalidException { - cfg = readConfig(PREFERENCES_CONFIG); + public static Diff fromInfo(DiffPreferencesInfo info) { + return (new AutoValue_Preferences_Diff.Builder()) + .context(info.context) + .tabSize(info.tabSize) + .fontSize(info.fontSize) + .lineLength(info.lineLength) + .cursorBlinkRate(info.cursorBlinkRate) + .expandAllComments(info.expandAllComments) + .intralineDifference(info.intralineDifference) + .manualReview(info.manualReview) + .showLineEndings(info.showLineEndings) + .showTabs(info.showTabs) + .showWhitespaceErrors(info.showWhitespaceErrors) + .syntaxHighlighting(info.syntaxHighlighting) + .hideTopMenu(info.hideTopMenu) + .autoHideDiffTableHeader(info.autoHideDiffTableHeader) + .hideLineNumbers(info.hideLineNumbers) + .renderEntireFile(info.renderEntireFile) + .hideEmptyPane(info.hideEmptyPane) + .matchBrackets(info.matchBrackets) + .lineWrapping(info.lineWrapping) + .theme(info.theme) + .ignoreWhitespace(info.ignoreWhitespace) + .retainHeader(info.retainHeader) + .skipDeleted(info.skipDeleted) + .skipUnchanged(info.skipUnchanged) + .skipUncommented(info.skipUncommented) + .build(); } - @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; + public DiffPreferencesInfo toInfo() { + DiffPreferencesInfo info = new DiffPreferencesInfo(); + info.context = context().orElse(null); + info.tabSize = tabSize().orElse(null); + info.fontSize = fontSize().orElse(null); + info.lineLength = lineLength().orElse(null); + info.cursorBlinkRate = cursorBlinkRate().orElse(null); + info.expandAllComments = expandAllComments().orElse(null); + info.intralineDifference = intralineDifference().orElse(null); + info.manualReview = manualReview().orElse(null); + info.showLineEndings = showLineEndings().orElse(null); + info.showTabs = showTabs().orElse(null); + info.showWhitespaceErrors = showWhitespaceErrors().orElse(null); + info.syntaxHighlighting = syntaxHighlighting().orElse(null); + info.hideTopMenu = hideTopMenu().orElse(null); + info.autoHideDiffTableHeader = autoHideDiffTableHeader().orElse(null); + info.hideLineNumbers = hideLineNumbers().orElse(null); + info.renderEntireFile = renderEntireFile().orElse(null); + info.hideEmptyPane = hideEmptyPane().orElse(null); + info.matchBrackets = matchBrackets().orElse(null); + info.lineWrapping = lineWrapping().orElse(null); + info.theme = theme().orElse(null); + info.ignoreWhitespace = ignoreWhitespace().orElse(null); + info.retainHeader = retainHeader().orElse(null); + info.skipDeleted = skipDeleted().orElse(null); + info.skipUnchanged = skipUnchanged().orElse(null); + info.skipUncommented = skipUncommented().orElse(null); + return info; } } } diff --git a/java/com/google/gerrit/server/account/StoredPreferences.java b/java/com/google/gerrit/server/account/StoredPreferences.java new file mode 100644 index 00000000000..31705db265c --- /dev/null +++ b/java/com/google/gerrit/server/account/StoredPreferences.java @@ -0,0 +1,616 @@ +// 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.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 static java.util.Objects.requireNonNull; + +import com.google.common.base.Strings; +import com.google.common.collect.Lists; +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.common.Nullable; +import com.google.gerrit.extensions.client.DiffPreferencesInfo; +import com.google.gerrit.extensions.client.EditPreferencesInfo; +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.config.AllUsersName; +import com.google.gerrit.server.git.UserConfigSections; +import com.google.gerrit.server.git.ValidationError; +import com.google.gerrit.server.git.meta.MetaDataUpdate; +import com.google.gerrit.server.git.meta.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.Optional; +import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.lib.CommitBuilder; +import org.eclipse.jgit.lib.Config; +import org.eclipse.jgit.lib.Repository; + +/** + * Parses/writes preferences from/to a {@link Config} file. + * + *

    This is a low-level API. Read/write of preferences in a user branch should be done through + * {@link AccountsUpdate} or {@link AccountConfig}. + * + *

    The config file has separate sections for general, diff and edit preferences: + * + *

    + *   [diff]
    + *     hideTopMenu = true
    + *   [edit]
    + *     lineLength = 80
    + * 
    + * + *

    The parameter names match the names that are used in the preferences REST API. + * + *

    If the preference is omitted in the config file, then the default value for the preference is + * used. + * + *

    Defaults for preferences that apply for all accounts can be configured in the {@code + * refs/users/default} branch in the {@code All-Users} repository. The config for the default + * preferences must be provided to this class so that it can read default values from it. + * + *

    The preferences are lazily parsed. + */ +public class StoredPreferences { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + + 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; + private DiffPreferencesInfo diffPreferences; + private EditPreferencesInfo editPreferences; + + StoredPreferences( + Account.Id accountId, + Config cfg, + Config defaultCfg, + ValidationError.Sink validationErrorSink) { + this.accountId = requireNonNull(accountId, "accountId"); + this.cfg = requireNonNull(cfg, "cfg"); + this.defaultCfg = requireNonNull(defaultCfg, "defaultCfg"); + this.validationErrorSink = requireNonNull(validationErrorSink, "validationErrorSink"); + } + + public GeneralPreferencesInfo getGeneralPreferences() { + if (generalPreferences == null) { + parse(); + } + return generalPreferences; + } + + public DiffPreferencesInfo getDiffPreferences() { + if (diffPreferences == null) { + parse(); + } + return diffPreferences; + } + + public EditPreferencesInfo getEditPreferences() { + if (editPreferences == null) { + parse(); + } + return editPreferences; + } + + public void parse() { + generalPreferences = parseGeneralPreferences(null); + diffPreferences = parseDiffPreferences(null); + editPreferences = parseEditPreferences(null); + } + + public Config saveGeneralPreferences( + Optional generalPreferencesInput, + Optional diffPreferencesInput, + Optional editPreferencesInput) + throws ConfigInvalidException { + if (generalPreferencesInput.isPresent()) { + GeneralPreferencesInfo mergedGeneralPreferencesInput = + parseGeneralPreferences(generalPreferencesInput.get()); + + 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; + } + + if (editPreferencesInput.isPresent()) { + EditPreferencesInfo mergedEditPreferencesInput = + parseEditPreferences(editPreferencesInput.get()); + + storeSection( + cfg, + UserConfigSections.EDIT, + null, + mergedEditPreferencesInput, + parseDefaultEditPreferences(defaultCfg, null)); + + // evict the cached edit preferences + this.editPreferences = null; + } + + return cfg; + } + + private GeneralPreferencesInfo parseGeneralPreferences(@Nullable GeneralPreferencesInfo input) { + try { + return parseGeneralPreferences(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 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 EditPreferencesInfo parseEditPreferences(@Nullable EditPreferencesInfo input) { + try { + return parseEditPreferences(cfg, defaultCfg, input); + } catch (ConfigInvalidException e) { + validationErrorSink.error( + new ValidationError( + PREFERENCES_CONFIG, + String.format( + "Invalid edit preferences for account %d: %s", accountId.get(), e.getMessage()))); + return new EditPreferencesInfo(); + } + } + + private static GeneralPreferencesInfo parseGeneralPreferences( + Config cfg, @Nullable Config defaultCfg, @Nullable GeneralPreferencesInfo input) + throws ConfigInvalidException { + GeneralPreferencesInfo r = + loadSection( + cfg, + UserConfigSections.GENERAL, + null, + new GeneralPreferencesInfo(), + defaultCfg != null + ? parseDefaultGeneralPreferences(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 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 EditPreferencesInfo parseEditPreferences( + Config cfg, @Nullable Config defaultCfg, @Nullable EditPreferencesInfo input) + throws ConfigInvalidException { + return loadSection( + cfg, + UserConfigSections.EDIT, + null, + new EditPreferencesInfo(), + defaultCfg != null + ? parseDefaultEditPreferences(defaultCfg, input) + : EditPreferencesInfo.defaults(), + input); + } + + private static GeneralPreferencesInfo parseDefaultGeneralPreferences( + Config defaultCfg, GeneralPreferencesInfo input) throws ConfigInvalidException { + GeneralPreferencesInfo allUserPrefs = new GeneralPreferencesInfo(); + loadSection( + defaultCfg, + UserConfigSections.GENERAL, + null, + allUserPrefs, + GeneralPreferencesInfo.defaults(), + input); + return updateGeneralPreferencesDefaults(allUserPrefs); + } + + 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 EditPreferencesInfo parseDefaultEditPreferences( + Config defaultCfg, EditPreferencesInfo input) throws ConfigInvalidException { + EditPreferencesInfo allUserPrefs = new EditPreferencesInfo(); + loadSection( + defaultCfg, + UserConfigSections.EDIT, + null, + allUserPrefs, + EditPreferencesInfo.defaults(), + input); + return updateEditPreferencesDefaults(allUserPrefs); + } + + private static GeneralPreferencesInfo updateGeneralPreferencesDefaults( + 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) { + logger.atSevere().withCause(e).log("Failed to apply default general preferences"); + return GeneralPreferencesInfo.defaults(); + } + 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) { + logger.atSevere().withCause(e).log("Failed to apply default diff preferences"); + return DiffPreferencesInfo.defaults(); + } + return result; + } + + private static EditPreferencesInfo updateEditPreferencesDefaults(EditPreferencesInfo input) { + EditPreferencesInfo result = EditPreferencesInfo.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) { + logger.atSevere().withCause(e).log("Failed to apply default edit preferences"); + return EditPreferencesInfo.defaults(); + } + return result; + } + + private static List parseChangeTableColumns(Config cfg, @Nullable Config defaultCfg) { + List changeTable = changeTable(cfg); + if (changeTable == null && defaultCfg != null) { + changeTable = changeTable(defaultCfg); + } + return changeTable; + } + + private static List parseMyMenus(Config cfg, @Nullable Config defaultCfg) { + List 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", "#/settings/#Groups", null)); + } + return my; + } + + private static Map parseUrlAliases(Config cfg, @Nullable Config defaultCfg) { + Map urlAliases = urlAliases(cfg); + if (urlAliases == null && defaultCfg != null) { + urlAliases = urlAliases(defaultCfg); + } + return urlAliases; + } + + public static GeneralPreferencesInfo readDefaultGeneralPreferences( + AllUsersName allUsersName, Repository allUsersRepo) + throws IOException, ConfigInvalidException { + return parseGeneralPreferences(readDefaultConfig(allUsersName, allUsersRepo), null, null); + } + + public static DiffPreferencesInfo readDefaultDiffPreferences( + AllUsersName allUsersName, Repository allUsersRepo) + throws IOException, ConfigInvalidException { + return parseDiffPreferences(readDefaultConfig(allUsersName, allUsersRepo), null, null); + } + + public static EditPreferencesInfo readDefaultEditPreferences( + AllUsersName allUsersName, Repository allUsersRepo) + throws IOException, ConfigInvalidException { + return parseEditPreferences(readDefaultConfig(allUsersName, allUsersRepo), null, null); + } + + static Config readDefaultConfig(AllUsersName allUsersName, Repository allUsersRepo) + throws IOException, ConfigInvalidException { + VersionedDefaultPreferences defaultPrefs = new VersionedDefaultPreferences(); + defaultPrefs.load(allUsersName, allUsersRepo); + return defaultPrefs.getConfig(); + } + + public static GeneralPreferencesInfo updateDefaultGeneralPreferences( + 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 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); + } + + public static EditPreferencesInfo updateDefaultEditPreferences( + MetaDataUpdate md, EditPreferencesInfo input) throws IOException, ConfigInvalidException { + VersionedDefaultPreferences defaultPrefs = new VersionedDefaultPreferences(); + defaultPrefs.load(md); + storeSection( + defaultPrefs.getConfig(), + UserConfigSections.EDIT, + null, + input, + EditPreferencesInfo.defaults()); + defaultPrefs.commit(md); + + return parseEditPreferences(defaultPrefs.getConfig(), null, null); + } + + private static List changeTable(Config cfg) { + return Lists.newArrayList(cfg.getStringList(CHANGE_TABLE, null, CHANGE_TABLE_COLUMN)); + } + + private static void setChangeTable(Config cfg, List changeTable) { + if (changeTable != null) { + unsetSection(cfg, UserConfigSections.CHANGE_TABLE); + cfg.setStringList(UserConfigSections.CHANGE_TABLE, null, CHANGE_TABLE_COLUMN, changeTable); + } + } + + private static List my(Config cfg) { + List 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 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 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 urlAliases(Config cfg) { + HashMap 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 urlAliases) { + if (urlAliases != null) { + for (String subsection : cfg.getSubsections(URL_ALIAS)) { + cfg.unsetSection(URL_ALIAS, subsection); + } + + int i = 1; + for (Map.Entry 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; + } + } +} diff --git a/java/com/google/gerrit/server/restapi/account/SetPreferences.java b/java/com/google/gerrit/server/restapi/account/SetPreferences.java index 7967f2d2fb8..54f0d9b7d32 100644 --- a/java/com/google/gerrit/server/restapi/account/SetPreferences.java +++ b/java/com/google/gerrit/server/restapi/account/SetPreferences.java @@ -31,7 +31,7 @@ import com.google.gerrit.server.account.AccountResource; import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.AccountsUpdate; -import com.google.gerrit.server.account.Preferences; +import com.google.gerrit.server.account.StoredPreferences; import com.google.gerrit.server.permissions.GlobalPermission; import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackendException; @@ -68,7 +68,7 @@ public Response apply(AccountResource rsrc, GeneralPrefe } checkDownloadScheme(input.downloadScheme); - Preferences.validateMy(input.my); + StoredPreferences.validateMy(input.my); Account.Id id = rsrc.getUser().getAccountId(); return Response.ok( diff --git a/java/com/google/gerrit/server/restapi/config/GetDiffPreferences.java b/java/com/google/gerrit/server/restapi/config/GetDiffPreferences.java index 5cf93d8c0ec..44c71b374eb 100644 --- a/java/com/google/gerrit/server/restapi/config/GetDiffPreferences.java +++ b/java/com/google/gerrit/server/restapi/config/GetDiffPreferences.java @@ -19,7 +19,7 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestReadView; -import com.google.gerrit.server.account.Preferences; +import com.google.gerrit.server.account.StoredPreferences; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.ConfigResource; import com.google.gerrit.server.git.GitRepositoryManager; @@ -45,7 +45,7 @@ public class GetDiffPreferences implements RestReadView { public Response apply(ConfigResource configResource) throws BadRequestException, ResourceConflictException, IOException, ConfigInvalidException { try (Repository git = gitManager.openRepository(allUsersName)) { - return Response.ok(Preferences.readDefaultDiffPreferences(allUsersName, git)); + return Response.ok(StoredPreferences.readDefaultDiffPreferences(allUsersName, git)); } } } diff --git a/java/com/google/gerrit/server/restapi/config/GetEditPreferences.java b/java/com/google/gerrit/server/restapi/config/GetEditPreferences.java index d2e1031e207..a5ab967c36c 100644 --- a/java/com/google/gerrit/server/restapi/config/GetEditPreferences.java +++ b/java/com/google/gerrit/server/restapi/config/GetEditPreferences.java @@ -19,7 +19,7 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestReadView; -import com.google.gerrit.server.account.Preferences; +import com.google.gerrit.server.account.StoredPreferences; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.ConfigResource; import com.google.gerrit.server.git.GitRepositoryManager; @@ -44,7 +44,7 @@ public class GetEditPreferences implements RestReadView { public Response apply(ConfigResource configResource) throws BadRequestException, ResourceConflictException, IOException, ConfigInvalidException { try (Repository git = gitManager.openRepository(allUsersName)) { - return Response.ok(Preferences.readDefaultEditPreferences(allUsersName, git)); + return Response.ok(StoredPreferences.readDefaultEditPreferences(allUsersName, git)); } } } diff --git a/java/com/google/gerrit/server/restapi/config/GetPreferences.java b/java/com/google/gerrit/server/restapi/config/GetPreferences.java index bf0ad397a2c..8da9134a0d4 100644 --- a/java/com/google/gerrit/server/restapi/config/GetPreferences.java +++ b/java/com/google/gerrit/server/restapi/config/GetPreferences.java @@ -17,7 +17,7 @@ import com.google.gerrit.extensions.client.GeneralPreferencesInfo; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestReadView; -import com.google.gerrit.server.account.Preferences; +import com.google.gerrit.server.account.StoredPreferences; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.ConfigResource; import com.google.gerrit.server.git.GitRepositoryManager; @@ -42,7 +42,7 @@ public GetPreferences(GitRepositoryManager gitMgr, AllUsersName allUsersName) { public Response apply(ConfigResource rsrc) throws IOException, ConfigInvalidException { try (Repository git = gitMgr.openRepository(allUsersName)) { - return Response.ok(Preferences.readDefaultGeneralPreferences(allUsersName, git)); + return Response.ok(StoredPreferences.readDefaultGeneralPreferences(allUsersName, git)); } } } diff --git a/java/com/google/gerrit/server/restapi/config/SetDiffPreferences.java b/java/com/google/gerrit/server/restapi/config/SetDiffPreferences.java index fb816655058..96654a9c943 100644 --- a/java/com/google/gerrit/server/restapi/config/SetDiffPreferences.java +++ b/java/com/google/gerrit/server/restapi/config/SetDiffPreferences.java @@ -24,7 +24,7 @@ import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.server.account.AccountCache; -import com.google.gerrit.server.account.Preferences; +import com.google.gerrit.server.account.StoredPreferences; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.ConfigResource; import com.google.gerrit.server.git.meta.MetaDataUpdate; @@ -66,7 +66,7 @@ public Response apply( } try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) { - DiffPreferencesInfo updatedPrefs = Preferences.updateDefaultDiffPreferences(md, input); + DiffPreferencesInfo updatedPrefs = StoredPreferences.updateDefaultDiffPreferences(md, input); accountCache.evictAll(); return Response.ok(updatedPrefs); } diff --git a/java/com/google/gerrit/server/restapi/config/SetEditPreferences.java b/java/com/google/gerrit/server/restapi/config/SetEditPreferences.java index 178a4e101cd..4bb420b5709 100644 --- a/java/com/google/gerrit/server/restapi/config/SetEditPreferences.java +++ b/java/com/google/gerrit/server/restapi/config/SetEditPreferences.java @@ -24,7 +24,7 @@ import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.server.account.AccountCache; -import com.google.gerrit.server.account.Preferences; +import com.google.gerrit.server.account.StoredPreferences; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.ConfigResource; import com.google.gerrit.server.git.meta.MetaDataUpdate; @@ -66,7 +66,7 @@ public Response apply( } try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) { - EditPreferencesInfo updatedPrefs = Preferences.updateDefaultEditPreferences(md, input); + EditPreferencesInfo updatedPrefs = StoredPreferences.updateDefaultEditPreferences(md, input); accountCache.evictAll(); return Response.ok(updatedPrefs); } diff --git a/java/com/google/gerrit/server/restapi/config/SetPreferences.java b/java/com/google/gerrit/server/restapi/config/SetPreferences.java index 779f3e73581..c88c11197d7 100644 --- a/java/com/google/gerrit/server/restapi/config/SetPreferences.java +++ b/java/com/google/gerrit/server/restapi/config/SetPreferences.java @@ -24,7 +24,7 @@ import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.server.account.AccountCache; -import com.google.gerrit.server.account.Preferences; +import com.google.gerrit.server.account.StoredPreferences; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.ConfigResource; import com.google.gerrit.server.git.meta.MetaDataUpdate; @@ -60,9 +60,10 @@ public Response apply(ConfigResource rsrc, GeneralPrefer if (!hasSetFields(input)) { throw new BadRequestException("unsupported option"); } - Preferences.validateMy(input.my); + StoredPreferences.validateMy(input.my); try (MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName)) { - GeneralPreferencesInfo updatedPrefs = Preferences.updateDefaultGeneralPreferences(md, input); + GeneralPreferencesInfo updatedPrefs = + StoredPreferences.updateDefaultGeneralPreferences(md, input); accountCache.evictAll(); return Response.ok(updatedPrefs); } diff --git a/javatests/com/google/gerrit/acceptance/rest/change/WorkInProgressByDefaultIT.java b/javatests/com/google/gerrit/acceptance/rest/change/WorkInProgressByDefaultIT.java index ccf1c0d0ddf..51d524b4394 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/WorkInProgressByDefaultIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/WorkInProgressByDefaultIT.java @@ -22,12 +22,16 @@ import com.google.gerrit.acceptance.GitUtil; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.testsuite.project.ProjectOperations; +import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations; import com.google.gerrit.extensions.api.projects.ConfigInput; import com.google.gerrit.extensions.client.GeneralPreferencesInfo; import com.google.gerrit.extensions.client.InheritableBoolean; import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeInput; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.reviewdb.client.RefNames; import com.google.inject.Inject; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.junit.TestRepository; @@ -37,6 +41,7 @@ public class WorkInProgressByDefaultIT extends AbstractDaemonTest { @Inject private ProjectOperations projectOperations; + @Inject private RequestScopeOperations requestScopeOperations; @Test public void createChangeWithWorkInProgressByDefaultForProjectDisabled() throws Exception { @@ -175,6 +180,14 @@ public void pushNewPatchSetAndNewChangeAtOnceWithWorkInProgressByDefaultForUserE setWorkInProgressByDefaultForUser(); + // Clone the repo again. The test connection keeps an AccountState internally, so we need to + // create a new connection after changing account properties. + PatchSet.Id ps1OfChange1 = + PatchSet.id(Change.id(gApi.changes().id(changeId1).get()._number), 1); + testRepo = cloneProject(project); + testRepo.git().fetch().setRefSpecs(RefNames.patchSetRef(ps1OfChange1) + ":c1").call(); + testRepo.reset("c1"); + // Create a new patch set on the existing change and in the same push create a new successor // change. RevCommit commit1b = testRepo.amend(commit1a).create(); @@ -199,9 +212,12 @@ private void setWorkInProgressByDefaultForProject(Project.NameKey p) throws Exce } private void setWorkInProgressByDefaultForUser() throws Exception { - GeneralPreferencesInfo prefs = gApi.accounts().id(admin.id().get()).getPreferences(); + GeneralPreferencesInfo prefs = new GeneralPreferencesInfo(); prefs.workInProgressByDefault = true; gApi.accounts().id(admin.id().get()).setPreferences(prefs); + // Generate a new API scope. User preferences are stored in IdentifiedUser, so we need to flush + // that entity. + requestScopeOperations.resetCurrentApiUser(); } private PushOneCommit.Result createChange(Project.NameKey p) throws Exception { diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD index 1bb22e41ea0..4383431b74c 100644 --- a/javatests/com/google/gerrit/server/BUILD +++ b/javatests/com/google/gerrit/server/BUILD @@ -45,6 +45,7 @@ junit_tests( "//java/com/google/gerrit/index", "//java/com/google/gerrit/index:query_exception", "//java/com/google/gerrit/jgit", + "//java/com/google/gerrit/json", "//java/com/google/gerrit/lifecycle", "//java/com/google/gerrit/mail", "//java/com/google/gerrit/metrics", diff --git a/javatests/com/google/gerrit/server/account/PreferencesTest.java b/javatests/com/google/gerrit/server/account/PreferencesTest.java index b1d31bff32a..9866481de8d 100644 --- a/javatests/com/google/gerrit/server/account/PreferencesTest.java +++ b/javatests/com/google/gerrit/server/account/PreferencesTest.java @@ -15,52 +15,36 @@ package com.google.gerrit.server.account; import static com.google.common.truth.Truth.assertThat; -import static org.mockito.Mockito.verifyNoMoreInteractions; +import com.google.gerrit.extensions.client.DiffPreferencesInfo; +import com.google.gerrit.extensions.client.EditPreferencesInfo; import com.google.gerrit.extensions.client.GeneralPreferencesInfo; -import com.google.gerrit.reviewdb.client.Account; -import com.google.gerrit.server.git.ValidationError; -import org.eclipse.jgit.lib.Config; +import com.google.gerrit.json.OutputFormat; +import com.google.gson.Gson; import org.junit.Test; -import org.mockito.Mockito; -/** Tests for parsing user preferences from Git. */ public class PreferencesTest { - enum Unknown { - STATE - } + private static final Gson GSON = OutputFormat.JSON_COMPACT.newGson(); @Test - public void ignoreUnknownAccountPreferencesWhenParsing() { - ValidationError.Sink errorSink = Mockito.mock(ValidationError.Sink.class); - Preferences preferences = - new Preferences(Account.id(1), configWithUnknownEntries(), new Config(), errorSink); - GeneralPreferencesInfo parsedPreferences = preferences.getGeneralPreferences(); - - assertThat(parsedPreferences).isNotNull(); - assertThat(parsedPreferences.expandInlineDiffs).isTrue(); - verifyNoMoreInteractions(errorSink); + public void generalPreferencesRoundTrip() { + GeneralPreferencesInfo original = GeneralPreferencesInfo.defaults(); + assertThat(GSON.toJson(original)) + .isEqualTo(GSON.toJson(Preferences.General.fromInfo(original).toInfo())); } @Test - public void ignoreUnknownDefaultAccountPreferencesWhenParsing() { - ValidationError.Sink errorSink = Mockito.mock(ValidationError.Sink.class); - Preferences preferences = - new Preferences(Account.id(1), new Config(), configWithUnknownEntries(), errorSink); - GeneralPreferencesInfo parsedPreferences = preferences.getGeneralPreferences(); - - assertThat(parsedPreferences).isNotNull(); - assertThat(parsedPreferences.expandInlineDiffs).isTrue(); - verifyNoMoreInteractions(errorSink); + public void diffPreferencesRoundTrip() { + DiffPreferencesInfo original = DiffPreferencesInfo.defaults(); + assertThat(GSON.toJson(original)) + .isEqualTo(GSON.toJson(Preferences.Diff.fromInfo(original).toInfo())); } - private static Config configWithUnknownEntries() { - Config cfg = new Config(); - cfg.setBoolean("general", null, "expandInlineDiffs", true); - cfg.setBoolean("general", null, "unknown", true); - cfg.setEnum("general", null, "unknownenum", Unknown.STATE); - cfg.setString("general", null, "unknownstring", "bla"); - return cfg; + @Test + public void editPreferencesRoundTrip() { + EditPreferencesInfo original = EditPreferencesInfo.defaults(); + assertThat(GSON.toJson(original)) + .isEqualTo(GSON.toJson(Preferences.Edit.fromInfo(original).toInfo())); } } diff --git a/javatests/com/google/gerrit/server/account/StoredPreferencesTest.java b/javatests/com/google/gerrit/server/account/StoredPreferencesTest.java new file mode 100644 index 00000000000..2bb0deb00f2 --- /dev/null +++ b/javatests/com/google/gerrit/server/account/StoredPreferencesTest.java @@ -0,0 +1,66 @@ +// Copyright (C) 2019 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.truth.Truth.assertThat; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +import com.google.gerrit.extensions.client.GeneralPreferencesInfo; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.server.git.ValidationError; +import org.eclipse.jgit.lib.Config; +import org.junit.Test; +import org.mockito.Mockito; + +/** Tests for parsing user preferences from Git. */ +public class StoredPreferencesTest { + + enum Unknown { + STATE + } + + @Test + public void ignoreUnknownAccountPreferencesWhenParsing() { + ValidationError.Sink errorSink = Mockito.mock(ValidationError.Sink.class); + StoredPreferences preferences = + new StoredPreferences(Account.id(1), configWithUnknownEntries(), new Config(), errorSink); + GeneralPreferencesInfo parsedPreferences = preferences.getGeneralPreferences(); + + assertThat(parsedPreferences).isNotNull(); + assertThat(parsedPreferences.expandInlineDiffs).isTrue(); + verifyNoMoreInteractions(errorSink); + } + + @Test + public void ignoreUnknownDefaultAccountPreferencesWhenParsing() { + ValidationError.Sink errorSink = Mockito.mock(ValidationError.Sink.class); + StoredPreferences preferences = + new StoredPreferences(Account.id(1), new Config(), configWithUnknownEntries(), errorSink); + GeneralPreferencesInfo parsedPreferences = preferences.getGeneralPreferences(); + + assertThat(parsedPreferences).isNotNull(); + assertThat(parsedPreferences.expandInlineDiffs).isTrue(); + verifyNoMoreInteractions(errorSink); + } + + private static Config configWithUnknownEntries() { + Config cfg = new Config(); + cfg.setBoolean("general", null, "expandInlineDiffs", true); + cfg.setBoolean("general", null, "unknown", true); + cfg.setEnum("general", null, "unknownenum", Unknown.STATE); + cfg.setString("general", null, "unknownstring", "bla"); + return cfg; + } +}