diff --git a/java/com/google/gerrit/server/account/AccountState.java b/java/com/google/gerrit/server/account/AccountState.java index 6eb6ca101d..dd81c93ca9 100644 --- a/java/com/google/gerrit/server/account/AccountState.java +++ b/java/com/google/gerrit/server/account/AccountState.java @@ -131,9 +131,9 @@ public class AccountState { 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 @@ public class 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 class AccountState { /** 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/Preferences.java b/java/com/google/gerrit/server/account/Preferences.java new file mode 100644 index 0000000000..63ff9e3e37 --- /dev/null +++ b/java/com/google/gerrit/server/account/Preferences.java @@ -0,0 +1,463 @@ +// 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 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.client.Theme; +import java.util.Optional; + +@AutoValue +public abstract class Preferences { + @AutoValue + public abstract static class General { + public abstract Optional changesPerPage(); + + public abstract Optional downloadScheme(); + + public abstract Optional downloadCommand(); + + public abstract Optional dateFormat(); + + public abstract Optional timeFormat(); + + public abstract Optional expandInlineDiffs(); + + public abstract Optional highlightAssigneeInChangeTable(); + + public abstract Optional relativeDateInChangeTable(); + + public abstract Optional diffView(); + + public abstract Optional sizeBarInChangeTable(); + + public abstract Optional legacycidInChangeTable(); + + public abstract Optional muteCommonPathPrefixes(); + + public abstract Optional signedOffBy(); + + public abstract Optional emailStrategy(); + + public abstract Optional emailFormat(); + + public abstract Optional defaultBaseForMerges(); + + public abstract Optional publishCommentsOnPush(); + + public abstract Optional workInProgressByDefault(); + + public abstract Optional> my(); + + public abstract Optional> changeTable(); + + public abstract Optional> urlAliases(); + + @AutoValue.Builder + public abstract static class Builder { + abstract Builder changesPerPage(@Nullable Integer val); + + abstract Builder downloadScheme(@Nullable String val); + + abstract Builder downloadCommand(@Nullable DownloadCommand val); + + abstract Builder dateFormat(@Nullable DateFormat val); + + abstract Builder timeFormat(@Nullable TimeFormat val); + + abstract Builder expandInlineDiffs(@Nullable Boolean val); + + abstract Builder highlightAssigneeInChangeTable(@Nullable Boolean val); + + abstract Builder relativeDateInChangeTable(@Nullable Boolean val); + + abstract Builder diffView(@Nullable DiffView val); + + abstract Builder sizeBarInChangeTable(@Nullable Boolean val); + + abstract Builder legacycidInChangeTable(@Nullable Boolean val); + + abstract Builder muteCommonPathPrefixes(@Nullable Boolean val); + + abstract Builder signedOffBy(@Nullable Boolean val); + + abstract Builder emailStrategy(@Nullable EmailStrategy val); + + abstract Builder emailFormat(@Nullable EmailFormat val); + + abstract Builder defaultBaseForMerges(@Nullable DefaultBase val); + + abstract Builder publishCommentsOnPush(@Nullable Boolean val); + + abstract Builder workInProgressByDefault(@Nullable Boolean val); + + abstract Builder my(@Nullable ImmutableList val); + + abstract Builder changeTable(@Nullable ImmutableList val); + + abstract Builder urlAliases(@Nullable ImmutableMap val); + + abstract General build(); + } + + 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(); + } + + 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; + } + } + + @AutoValue + public abstract static class Edit { + public abstract Optional tabSize(); + + 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(); + + 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(); + } + + 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(); + } + + 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; + } + } + + @AutoValue + public abstract static class Diff { + public abstract Optional context(); + + 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); + + abstract Diff build(); + } + + 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(); + } + + 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/javatests/com/google/gerrit/acceptance/rest/change/WorkInProgressByDefaultIT.java b/javatests/com/google/gerrit/acceptance/rest/change/WorkInProgressByDefaultIT.java index ccf1c0d0dd..51d524b439 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.AbstractDaemonTest; 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 @@ import org.junit.Test; public class WorkInProgressByDefaultIT extends AbstractDaemonTest { @Inject private ProjectOperations projectOperations; + @Inject private RequestScopeOperations requestScopeOperations; @Test public void createChangeWithWorkInProgressByDefaultForProjectDisabled() throws Exception { @@ -175,6 +180,14 @@ public class WorkInProgressByDefaultIT extends AbstractDaemonTest { 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 @@ public class WorkInProgressByDefaultIT extends AbstractDaemonTest { } 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 1bb22e41ea..4383431b74 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 new file mode 100644 index 0000000000..9866481de8 --- /dev/null +++ b/javatests/com/google/gerrit/server/account/PreferencesTest.java @@ -0,0 +1,50 @@ +// 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 com.google.gerrit.extensions.client.DiffPreferencesInfo; +import com.google.gerrit.extensions.client.EditPreferencesInfo; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo; +import com.google.gerrit.json.OutputFormat; +import com.google.gson.Gson; +import org.junit.Test; + +public class PreferencesTest { + + private static final Gson GSON = OutputFormat.JSON_COMPACT.newGson(); + + @Test + public void generalPreferencesRoundTrip() { + GeneralPreferencesInfo original = GeneralPreferencesInfo.defaults(); + assertThat(GSON.toJson(original)) + .isEqualTo(GSON.toJson(Preferences.General.fromInfo(original).toInfo())); + } + + @Test + public void diffPreferencesRoundTrip() { + DiffPreferencesInfo original = DiffPreferencesInfo.defaults(); + assertThat(GSON.toJson(original)) + .isEqualTo(GSON.toJson(Preferences.Diff.fromInfo(original).toInfo())); + } + + @Test + public void editPreferencesRoundTrip() { + EditPreferencesInfo original = EditPreferencesInfo.defaults(); + assertThat(GSON.toJson(original)) + .isEqualTo(GSON.toJson(Preferences.Edit.fromInfo(original).toInfo())); + } +}