From 2a9692c8b2019b75de7acea29cfe8482018a56fd Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Sat, 19 Sep 2015 13:41:07 +0200 Subject: [PATCH] Replace ACCOUNT_DIFF_PREFERENCES table with Git backend (part2) This is the second part in the series of migration of user preferences table to Git backend. Database migration is implemented, migrating the content of the diff account preferences table into Git backend, All-Users repository. Change-Id: I2245d0a3948fd3d65151b3774f7778842e95234a --- .../rest/account/DiffPreferencesIT.java | 9 - .../gerrit/client/diff/PreferencesBox.java | 13 +- .../gerrit/httpd/raw/HostPageServlet.java | 4 +- .../client/AccountDiffPreference.java | 338 ------------------ .../server/AccountDiffPreferenceAccess.java | 29 -- .../gerrit/reviewdb/server/ReviewDb.java | 3 +- .../server/account/GetDiffPreferences.java | 81 +---- .../server/account/SetDiffPreferences.java | 153 +------- .../gerrit/server/schema/SchemaVersion.java | 2 +- .../gerrit/server/schema/Schema_115.java | 178 +++++++++ .../gerrit/testutil/DisabledReviewDb.java | 6 - 11 files changed, 198 insertions(+), 618 deletions(-) delete mode 100644 gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountDiffPreference.java delete mode 100644 gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountDiffPreferenceAccess.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_115.java diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/DiffPreferencesIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/DiffPreferencesIT.java index 5550cb6598..0dea558d80 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/DiffPreferencesIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/DiffPreferencesIT.java @@ -22,20 +22,11 @@ import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.extensions.client.DiffPreferencesInfo; import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace; import com.google.gerrit.extensions.client.Theme; -import com.google.gerrit.testutil.ConfigSuite; import org.apache.http.HttpStatus; -import org.eclipse.jgit.lib.Config; import org.junit.Test; public class DiffPreferencesIT extends AbstractDaemonTest { - @ConfigSuite.Config - public static Config readFromGitConfig() { - Config cfg = new Config(); - cfg.setBoolean("user", null, "readPrefsFromGit", true); - return cfg; - } - @Test public void getDiffPreferencesOfNonExistingAccount_NotFound() throws Exception { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PreferencesBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PreferencesBox.java index 455a6437eb..36d6514b07 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PreferencesBox.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PreferencesBox.java @@ -14,12 +14,13 @@ package com.google.gerrit.client.diff; -import static com.google.gerrit.reviewdb.client.AccountDiffPreference.DEFAULT_CONTEXT; -import static com.google.gerrit.reviewdb.client.AccountDiffPreference.WHOLE_FILE_CONTEXT; -import static com.google.gerrit.reviewdb.client.AccountDiffPreference.Whitespace.IGNORE_ALL; -import static com.google.gerrit.reviewdb.client.AccountDiffPreference.Whitespace.IGNORE_LEADING_AND_TRAILING; -import static com.google.gerrit.reviewdb.client.AccountDiffPreference.Whitespace.IGNORE_NONE; -import static com.google.gerrit.reviewdb.client.AccountDiffPreference.Whitespace.IGNORE_TRAILING; +import static com.google.gerrit.extensions.client.DiffPreferencesInfo.DEFAULT_CONTEXT; +import static com.google.gerrit.extensions.client.DiffPreferencesInfo.WHOLE_FILE_CONTEXT; +import static com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace.IGNORE_ALL; +import static com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace.IGNORE_LEADING_AND_TRAILING; +import static com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace.IGNORE_NONE; +import static com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace.IGNORE_TRAILING; + import static com.google.gwt.event.dom.client.KeyCodes.KEY_ESCAPE; import com.google.gerrit.client.Gerrit; diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/HostPageServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/HostPageServlet.java index e21f973d79..ed498419a1 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/HostPageServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/HostPageServlet.java @@ -43,7 +43,6 @@ import com.google.gerrit.server.notedb.NotesMigration; import com.google.gwtexpui.server.CacheHeaders; import com.google.gwtjsonrpc.server.JsonServlet; import com.google.gwtjsonrpc.server.RPCServletUtils; -import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; @@ -250,8 +249,7 @@ public class HostPageServlet extends HttpServlet { private DiffPreferencesInfo getDiffPreferences(IdentifiedUser user) { try { return getDiff.apply(new AccountResource(user)); - } catch (AuthException | OrmException | ConfigInvalidException - | IOException e) { + } catch (AuthException | ConfigInvalidException | IOException e) { log.warn("Cannot query account diff preferences", e); } return DiffPreferencesInfo.defaults(); diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountDiffPreference.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountDiffPreference.java deleted file mode 100644 index cc0cbf03cd..0000000000 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountDiffPreference.java +++ /dev/null @@ -1,338 +0,0 @@ -// Copyright (C) 2010 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.reviewdb.client; - -import com.google.gerrit.extensions.client.Theme; -import com.google.gwtorm.client.Column; - -/** Diff formatting preferences of an account */ -public class AccountDiffPreference { - - /** Default number of lines of context. */ - public static final short DEFAULT_CONTEXT = 10; - - /** Context setting to display the entire file. */ - public static final short WHOLE_FILE_CONTEXT = -1; - - /** Typical valid choices for the default context setting. */ - public static final short[] CONTEXT_CHOICES = - {3, 10, 25, 50, 75, 100, WHOLE_FILE_CONTEXT}; - - public static enum Whitespace implements CodedEnum { - IGNORE_NONE('N'), // - IGNORE_TRAILING('E'), // - IGNORE_LEADING_AND_TRAILING('S'), // - IGNORE_ALL('A'); - - private final char code; - - private Whitespace(final char c) { - code = c; - } - - @Override - public char getCode() { - return code; - } - - public static Whitespace forCode(final char c) { - for (final Whitespace s : Whitespace.values()) { - if (s.code == c) { - return s; - } - } - return null; - } - } - - public static AccountDiffPreference createDefault(Account.Id accountId) { - AccountDiffPreference p = new AccountDiffPreference(accountId); - p.setIgnoreWhitespace(Whitespace.IGNORE_NONE); - p.setTheme(Theme.DEFAULT); - p.setTabSize(8); - p.setLineLength(100); - p.setSyntaxHighlighting(true); - p.setShowWhitespaceErrors(true); - p.setShowLineEndings(true); - p.setIntralineDifference(true); - p.setShowTabs(true); - p.setContext(DEFAULT_CONTEXT); - p.setManualReview(false); - p.setHideEmptyPane(false); - p.setAutoHideDiffTableHeader(true); - return p; - } - - @Column(id = 1, name = Column.NONE) - protected Account.Id accountId; - - @Column(id = 2) - protected char ignoreWhitespace; - - @Column(id = 3) - protected int tabSize; - - @Column(id = 4) - protected int lineLength; - - @Column(id = 5) - protected boolean syntaxHighlighting; - - @Column(id = 6) - protected boolean showWhitespaceErrors; - - @Column(id = 7) - protected boolean intralineDifference; - - @Column(id = 8) - protected boolean showTabs; - - /** Number of lines of context when viewing a patch. */ - @Column(id = 9) - protected short context; - - @Column(id = 10) - protected boolean skipDeleted; - - @Column(id = 11) - protected boolean skipUncommented; - - @Column(id = 12) - protected boolean expandAllComments; - - @Column(id = 13) - protected boolean retainHeader; - - @Column(id = 14) - protected boolean manualReview; - - @Column(id = 15) - protected boolean showLineEndings; - - @Column(id = 16) - protected boolean hideTopMenu; - - @Column(id = 17) - protected boolean hideLineNumbers; - - @Column(id = 18) - protected boolean renderEntireFile; - - @Column(id = 19, length = 20, notNull = false) - protected String theme; - - @Column(id = 20) - protected boolean hideEmptyPane; - - @Column(id = 21) - protected boolean autoHideDiffTableHeader; - - protected AccountDiffPreference() { - } - - public AccountDiffPreference(Account.Id accountId) { - this.accountId = accountId; - } - - public AccountDiffPreference(AccountDiffPreference p) { - this.accountId = p.accountId; - this.ignoreWhitespace = p.ignoreWhitespace; - this.tabSize = p.tabSize; - this.lineLength = p.lineLength; - this.syntaxHighlighting = p.syntaxHighlighting; - this.showWhitespaceErrors = p.showWhitespaceErrors; - this.showLineEndings = p.showLineEndings; - this.intralineDifference = p.intralineDifference; - this.showTabs = p.showTabs; - this.skipDeleted = p.skipDeleted; - this.skipUncommented = p.skipUncommented; - this.expandAllComments = p.expandAllComments; - this.context = p.context; - this.retainHeader = p.retainHeader; - this.manualReview = p.manualReview; - this.hideTopMenu = p.hideTopMenu; - this.hideLineNumbers = p.hideLineNumbers; - this.renderEntireFile = p.renderEntireFile; - this.hideEmptyPane = p.hideEmptyPane; - this.autoHideDiffTableHeader = p.autoHideDiffTableHeader; - } - - public Account.Id getAccountId() { - return accountId; - } - - public Whitespace getIgnoreWhitespace() { - return Whitespace.forCode(ignoreWhitespace); - } - - public void setIgnoreWhitespace(Whitespace ignoreWhitespace) { - this.ignoreWhitespace = ignoreWhitespace.getCode(); - } - - public int getTabSize() { - return tabSize; - } - - public void setTabSize(int tabSize) { - this.tabSize = tabSize; - } - - public int getLineLength() { - return lineLength; - } - - public void setLineLength(int lineLength) { - this.lineLength = lineLength; - } - - public boolean isSyntaxHighlighting() { - return syntaxHighlighting; - } - - public void setSyntaxHighlighting(boolean syntaxHighlighting) { - this.syntaxHighlighting = syntaxHighlighting; - } - - public boolean isShowWhitespaceErrors() { - return showWhitespaceErrors; - } - - public void setShowWhitespaceErrors(boolean showWhitespaceErrors) { - this.showWhitespaceErrors = showWhitespaceErrors; - } - - public boolean isShowLineEndings() { - return showLineEndings; - } - - public void setShowLineEndings(boolean showLineEndings) { - this.showLineEndings = showLineEndings; - } - - public boolean isIntralineDifference() { - return intralineDifference; - } - - public void setIntralineDifference(boolean intralineDifference) { - this.intralineDifference = intralineDifference; - } - - public boolean isShowTabs() { - return showTabs; - } - - public void setShowTabs(boolean showTabs) { - this.showTabs = showTabs; - } - - /** Get the number of lines of context when viewing a patch. */ - public short getContext() { - return context; - } - - /** Set the number of lines of context when viewing a patch. */ - public void setContext(final short context) { - assert 0 <= context || context == WHOLE_FILE_CONTEXT; - this.context = context; - } - - public boolean isSkipDeleted() { - return skipDeleted; - } - - public void setSkipDeleted(boolean skip) { - skipDeleted = skip; - } - - public boolean isSkipUncommented() { - return skipUncommented; - } - - public void setSkipUncommented(boolean skip) { - skipUncommented = skip; - } - - public boolean isExpandAllComments() { - return expandAllComments; - } - - public void setExpandAllComments(boolean expand) { - expandAllComments = expand; - } - - public boolean isRetainHeader() { - return retainHeader; - } - - public void setRetainHeader(boolean retain) { - retainHeader = retain; - } - - public boolean isManualReview() { - return manualReview; - } - - public void setManualReview(boolean manual) { - manualReview = manual; - } - - public boolean isHideTopMenu() { - return hideTopMenu; - } - - public void setHideTopMenu(boolean hide) { - hideTopMenu = hide; - } - - public boolean isHideLineNumbers() { - return hideLineNumbers; - } - - public void setHideLineNumbers(boolean hide) { - hideLineNumbers = hide; - } - - public boolean isRenderEntireFile() { - return renderEntireFile; - } - - public void setRenderEntireFile(boolean render) { - renderEntireFile = render; - } - - public Theme getTheme() { - return theme != null ? Theme.valueOf(theme) : null; - } - - public void setTheme(Theme theme) { - this.theme = theme != null ? theme.name() : null; - } - - public boolean isHideEmptyPane() { - return hideEmptyPane; - } - - public void setHideEmptyPane(boolean hideEmptyPane) { - this.hideEmptyPane = hideEmptyPane; - } - - public void setAutoHideDiffTableHeader(boolean hide) { - autoHideDiffTableHeader = hide; - } - - public boolean isAutoHideDiffTableHeader() { - return autoHideDiffTableHeader; - } -} diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountDiffPreferenceAccess.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountDiffPreferenceAccess.java deleted file mode 100644 index 499cb77936..0000000000 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountDiffPreferenceAccess.java +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (C) 2010 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.reviewdb.server; - -import com.google.gerrit.reviewdb.client.Account; -import com.google.gerrit.reviewdb.client.AccountDiffPreference; -import com.google.gwtorm.server.Access; -import com.google.gwtorm.server.OrmException; -import com.google.gwtorm.server.PrimaryKey; - -public interface AccountDiffPreferenceAccess extends Access { - - @Override - @PrimaryKey("accountId") - AccountDiffPreference get(Account.Id key) throws OrmException; - -} diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java index 37dcc59857..a63e63849c 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java @@ -68,8 +68,7 @@ public interface ReviewDb extends Schema { @Relation(id = 13) AccountGroupMemberAuditAccess accountGroupMembersAudit(); - @Relation(id = 17) - AccountDiffPreferenceAccess accountDiffPreferences(); + //Deleted @Relation(id = 17) @Relation(id = 18) StarredChangeAccess starredChanges(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetDiffPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetDiffPreferences.java index d2d6bf285f..be87ae73da 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetDiffPreferences.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetDiffPreferences.java @@ -20,22 +20,16 @@ import com.google.gerrit.extensions.client.DiffPreferencesInfo; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.reviewdb.client.Account; -import com.google.gerrit.reviewdb.client.AccountDiffPreference; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.config.AllUsersName; -import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.UserConfigSections; -import com.google.gerrit.server.patch.PatchListKey; -import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.RepositoryNotFoundException; -import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Repository; import java.io.IOException; @@ -43,36 +37,28 @@ import java.io.IOException; @Singleton public class GetDiffPreferences implements RestReadView { private final Provider self; - private final Provider db; private final Provider allUsersName; private final GitRepositoryManager gitMgr; - private final boolean readFromGit; @Inject GetDiffPreferences(Provider self, - Provider db, - @GerritServerConfig Config cfg, Provider allUsersName, GitRepositoryManager gitMgr) { this.self = self; - this.db = db; this.allUsersName = allUsersName; this.gitMgr = gitMgr; - readFromGit = cfg.getBoolean("user", null, "readPrefsFromGit", false); } @Override public DiffPreferencesInfo apply(AccountResource rsrc) - throws AuthException, OrmException, ConfigInvalidException, IOException { + throws AuthException, ConfigInvalidException, IOException { if (self.get() != rsrc.getUser() && !self.get().getCapabilities().canAdministrateServer()) { throw new AuthException("restricted to administrator"); } - Account.Id userId = rsrc.getUser().getAccountId(); - return readFromGit - ? readFromGit(userId, gitMgr, allUsersName.get(), null) - : readFromDb(userId); + Account.Id id = rsrc.getUser().getAccountId(); + return readFromGit(id, gitMgr, allUsersName.get(), null); } static DiffPreferencesInfo readFromGit(Account.Id id, @@ -89,65 +75,4 @@ public class GetDiffPreferences implements RestReadView { return prefs; } } - - private DiffPreferencesInfo readFromDb(Account.Id id) - throws OrmException { - AccountDiffPreference a = db.get().accountDiffPreferences().get(id); - return nullify(initFromDb(a)); - } - - static DiffPreferencesInfo initFromDb(AccountDiffPreference a) { - DiffPreferencesInfo prefs = DiffPreferencesInfo.defaults(); - if (a != null) { - prefs.context = (int)a.getContext(); - prefs.expandAllComments = a.isExpandAllComments(); - prefs.hideLineNumbers = a.isHideLineNumbers(); - prefs.hideTopMenu = a.isHideTopMenu(); - prefs.ignoreWhitespace = PatchListKey.WHITESPACE_TYPES.inverse().get( - a.getIgnoreWhitespace().getCode()); - prefs.intralineDifference = a.isIntralineDifference(); - prefs.lineLength = a.getLineLength(); - prefs.manualReview = a.isManualReview(); - prefs.renderEntireFile = a.isRenderEntireFile(); - prefs.retainHeader = a.isRetainHeader(); - prefs.showLineEndings = a.isShowLineEndings(); - prefs.showTabs = a.isShowTabs(); - prefs.showWhitespaceErrors = a.isShowWhitespaceErrors(); - prefs.skipDeleted = a.isSkipDeleted(); - prefs.skipUncommented = a.isSkipUncommented(); - prefs.syntaxHighlighting = a.isSyntaxHighlighting(); - prefs.tabSize = a.getTabSize(); - prefs.theme = a.getTheme(); - prefs.hideEmptyPane = a.isHideEmptyPane(); - prefs.autoHideDiffTableHeader = a.isAutoHideDiffTableHeader(); - } - - return prefs; - } - - private static DiffPreferencesInfo nullify(DiffPreferencesInfo prefs) { - prefs.expandAllComments = b(prefs.expandAllComments); - prefs.hideLineNumbers = b(prefs.hideLineNumbers); - prefs.hideTopMenu = b(prefs.hideTopMenu); - prefs.intralineDifference = b(prefs.intralineDifference); - prefs.manualReview = b(prefs.manualReview); - prefs.renderEntireFile = b(prefs.renderEntireFile); - prefs.retainHeader = b(prefs.retainHeader); - prefs.showLineEndings = b(prefs.showLineEndings); - prefs.showTabs = b(prefs.showTabs); - prefs.showWhitespaceErrors = b(prefs.showWhitespaceErrors); - prefs.skipDeleted = b(prefs.skipDeleted); - prefs.skipUncommented = b(prefs.skipUncommented); - prefs.syntaxHighlighting = b(prefs.syntaxHighlighting); - prefs.hideEmptyPane = b(prefs.hideEmptyPane); - prefs.autoHideDiffTableHeader = b(prefs.autoHideDiffTableHeader); - return prefs; - } - - private static Boolean b(Boolean b) { - if (b == null) { - return null; - } - return b ? Boolean.TRUE : null; - } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetDiffPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetDiffPreferences.java index 7b4d133e22..e14791c8af 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetDiffPreferences.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetDiffPreferences.java @@ -14,7 +14,6 @@ package com.google.gerrit.server.account; -import static com.google.gerrit.server.account.GetDiffPreferences.initFromDb; import static com.google.gerrit.server.account.GetDiffPreferences.readFromGit; import static com.google.gerrit.server.config.ConfigUtil.loadSection; import static com.google.gerrit.server.config.ConfigUtil.storeSection; @@ -24,16 +23,11 @@ import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.reviewdb.client.Account; -import com.google.gerrit.reviewdb.client.AccountDiffPreference; -import com.google.gerrit.reviewdb.client.AccountDiffPreference.Whitespace; -import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.config.AllUsersName; -import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.UserConfigSections; -import com.google.gerrit.server.patch.PatchListKey; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; @@ -41,34 +35,26 @@ import com.google.inject.Singleton; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.RepositoryNotFoundException; -import org.eclipse.jgit.lib.Config; import java.io.IOException; -import java.util.Collections; @Singleton public class SetDiffPreferences implements RestModifyView { private final Provider self; - private final Provider db; private final Provider metaDataUpdateFactory; private final AllUsersName allUsersName; private final GitRepositoryManager gitMgr; - private final boolean readFromGit; @Inject SetDiffPreferences(Provider self, - Provider db, - @GerritServerConfig Config cfg, Provider metaDataUpdateFactory, AllUsersName allUsersName, GitRepositoryManager gitMgr) { this.self = self; - this.db = db; this.metaDataUpdateFactory = metaDataUpdateFactory; this.allUsersName = allUsersName; this.gitMgr = gitMgr; - readFromGit = cfg.getBoolean("user", null, "readPrefsFromGit", false); } @Override @@ -84,40 +70,23 @@ public class SetDiffPreferences implements throw new BadRequestException("input must be provided"); } - Account.Id userId = rsrc.getUser().getAccountId(); - DiffPreferencesInfo n = readFromGit - ? readFromGit(userId, gitMgr, allUsersName, in) - : merge(initFromDb(db.get().accountDiffPreferences().get(userId)), in); - DiffPreferencesInfo out = writeToGit(n, userId); - writeToDb(n, userId); - return out; - } - - private void writeToDb(DiffPreferencesInfo in, Account.Id id) - throws OrmException { - db.get().accounts().beginTransaction(id); - try { - AccountDiffPreference p = db.get().accountDiffPreferences().get(id); - p = initAccountDiffPreferences(p, in, id); - db.get().accountDiffPreferences().upsert(Collections.singleton(p)); - db.get().commit(); - } finally { - db.get().rollback(); - } + Account.Id id = rsrc.getUser().getAccountId(); + return writeToGit(readFromGit(id, gitMgr, allUsersName, in), id); } private DiffPreferencesInfo writeToGit(DiffPreferencesInfo in, - Account.Id useId) throws RepositoryNotFoundException, IOException, + Account.Id userId) throws RepositoryNotFoundException, IOException, ConfigInvalidException { MetaDataUpdate md = metaDataUpdateFactory.get().create(allUsersName); - VersionedAccountPreferences prefs; DiffPreferencesInfo out = new DiffPreferencesInfo(); try { - prefs = VersionedAccountPreferences.forUser(useId); + VersionedAccountPreferences prefs = VersionedAccountPreferences.forUser( + userId); prefs.load(md); + DiffPreferencesInfo defaults = DiffPreferencesInfo.defaults(); storeSection(prefs.getConfig(), UserConfigSections.DIFF, null, in, - DiffPreferencesInfo.defaults()); + defaults); prefs.commit(md); loadSection(prefs.getConfig(), UserConfigSections.DIFF, null, out, DiffPreferencesInfo.defaults(), null); @@ -126,112 +95,4 @@ public class SetDiffPreferences implements } return out; } - - // TODO(davido): Remove manual merging in follow-up change - private DiffPreferencesInfo merge(DiffPreferencesInfo n, - DiffPreferencesInfo i) { - if (i.context != null) { - n.context = i.context; - } - if (i.expandAllComments != null) { - n.expandAllComments = i.expandAllComments; - } - if (i.hideLineNumbers != null) { - n.hideLineNumbers = i.hideLineNumbers; - } - if (i.hideTopMenu != null) { - n.hideTopMenu = i.hideTopMenu; - } - if (i.ignoreWhitespace != null) { - n.ignoreWhitespace = i.ignoreWhitespace; - } - if (i.intralineDifference != null) { - n.intralineDifference = i.intralineDifference; - } - if (i.lineLength != null) { - n.lineLength = i.lineLength; - } - if (i.manualReview != null) { - n.manualReview = i.manualReview; - } - if (i.renderEntireFile != null) { - n.renderEntireFile = i.renderEntireFile; - } - if (i.retainHeader != null) { - n.retainHeader = i.retainHeader; - } - if (i.showLineEndings != null) { - n.showLineEndings = i.showLineEndings; - } - if (i.showTabs != null) { - n.showTabs = i.showTabs; - } - if (i.showWhitespaceErrors != null) { - n.showWhitespaceErrors = i.showWhitespaceErrors; - } - if (i.skipDeleted != null) { - n.skipDeleted = i.skipDeleted; - } - if (i.skipUncommented != null) { - n.skipUncommented = i.skipUncommented; - } - if (i.syntaxHighlighting != null) { - n.syntaxHighlighting = i.syntaxHighlighting; - } - if (i.tabSize != null) { - n.tabSize = i.tabSize; - } - if (i.theme != null) { - n.theme = i.theme; - } - if (i.hideEmptyPane != null) { - n.hideEmptyPane = i.hideEmptyPane; - } - if (i.autoHideDiffTableHeader != null) { - n.autoHideDiffTableHeader = i.autoHideDiffTableHeader; - } - return n; - } - - private static AccountDiffPreference initAccountDiffPreferences( - AccountDiffPreference a, DiffPreferencesInfo i, Account.Id id) { - if (a == null) { - a = AccountDiffPreference.createDefault(id); - } - int context = i.context == null - ? DiffPreferencesInfo.DEFAULT_CONTEXT - : i.context; - a.setContext((short)context); - a.setExpandAllComments(b(i.expandAllComments)); - a.setHideLineNumbers(b(i.hideLineNumbers)); - a.setHideTopMenu(b(i.hideTopMenu)); - a.setIgnoreWhitespace(i.ignoreWhitespace == null - ? Whitespace.IGNORE_NONE - : Whitespace.forCode( - PatchListKey.WHITESPACE_TYPES.get(i.ignoreWhitespace))); - a.setIntralineDifference(b(i.intralineDifference)); - a.setLineLength(i.lineLength == null - ? DiffPreferencesInfo.DEFAULT_LINE_LENGTH - : i.lineLength); - a.setManualReview(b(i.manualReview)); - a.setRenderEntireFile(b(i.renderEntireFile)); - a.setRetainHeader(b(i.retainHeader)); - a.setShowLineEndings(b(i.showLineEndings)); - a.setShowTabs(b(i.showTabs)); - a.setShowWhitespaceErrors(b(i.showWhitespaceErrors)); - a.setSkipDeleted(b(i.skipDeleted)); - a.setSkipUncommented(b(i.skipUncommented)); - a.setSyntaxHighlighting(b(i.syntaxHighlighting)); - a.setTabSize(i.tabSize == null - ? DiffPreferencesInfo.DEFAULT_TAB_SIZE - : i.tabSize); - a.setTheme(i.theme); - a.setHideEmptyPane(b(i.hideEmptyPane)); - a.setAutoHideDiffTableHeader(b(i.autoHideDiffTableHeader)); - return a; - } - - private static boolean b(Boolean b) { - return b == null ? false : b; - } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java index 1c2998821a..27df9a5c9e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java @@ -32,7 +32,7 @@ import java.util.List; /** A version of the database schema. */ public abstract class SchemaVersion { /** The current schema version. */ - public static final Class C = Schema_114.class; + public static final Class C = Schema_115.class; public static int getBinaryVersion() { return guessVersion(C); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_115.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_115.java new file mode 100644 index 0000000000..920160b539 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_115.java @@ -0,0 +1,178 @@ +// Copyright (C) 2015 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.schema; + +import static com.google.gerrit.server.config.ConfigUtil.storeSection; + +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; +import com.google.gerrit.extensions.client.DiffPreferencesInfo; +import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace; +import com.google.gerrit.extensions.client.Theme; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.GerritPersonIdent; +import com.google.gerrit.server.account.VersionedAccountPreferences; +import com.google.gerrit.server.config.AllUsersName; +import com.google.gerrit.server.extensions.events.GitReferenceUpdated; +import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.git.MetaDataUpdate; +import com.google.gerrit.server.git.UserConfigSections; +import com.google.gerrit.server.patch.PatchListKey; +import com.google.gwtorm.jdbc.JdbcSchema; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.Provider; + +import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.lib.BatchRefUpdate; +import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevWalk; + +import java.io.IOException; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.HashMap; +import java.util.Map; + +public class Schema_115 extends SchemaVersion { + private final GitRepositoryManager mgr; + private final AllUsersName allUsersName; + private final PersonIdent serverUser; + + @Inject + Schema_115(Provider prior, + GitRepositoryManager mgr, + AllUsersName allUsersName, + @GerritPersonIdent PersonIdent serverUser) { + super(prior); + this.mgr = mgr; + this.allUsersName = allUsersName; + this.serverUser = serverUser; + } + + @Override + protected void migrateData(ReviewDb db, UpdateUI ui) + throws OrmException, SQLException { + Map imports = new HashMap<>(); + try (Statement stmt = ((JdbcSchema) db).getConnection().createStatement(); + ResultSet rs = stmt.executeQuery( + "SELECT " + + "id, " + + "context, " + + "expand_all_comments, " + + "hide_line_numbers, " + + "hide_top_menu, " + + "ignore_whitespace, " + + "intraline_difference, " + + "line_length, " + + "manual_review, " + + "render_entire_file, " + + "retain_header, " + + "show_line_endings, " + + "show_tabs, " + + "show_whitespace_errors, " + + "skip_deleted, " + + "skip_uncommented, " + + "syntax_highlighting, " + + "tab_size, " + + "theme, " + + "hide_empty_pane, " + + "auto_hide_diff_table_header " + + "FROM account_diff_preferences")) { + while (rs.next()) { + Account.Id accountId = new Account.Id(rs.getInt(1)); + DiffPreferencesInfo prefs = new DiffPreferencesInfo(); + prefs.context = (int)rs.getShort(2); + prefs.expandAllComments = toBoolean(rs.getString(3)); + prefs.hideLineNumbers = toBoolean(rs.getString(4)); + prefs.hideTopMenu = toBoolean(rs.getString(5)); + // Enum with char as value + prefs.ignoreWhitespace = toWhitespace(rs.getString(6)); + prefs.intralineDifference = toBoolean(rs.getString(7)); + prefs.lineLength = rs.getInt(8); + prefs.manualReview = toBoolean(rs.getString(9)); + prefs.renderEntireFile = toBoolean(rs.getString(10)); + prefs.retainHeader = toBoolean(rs.getString(11)); + prefs.showLineEndings = toBoolean(rs.getString(12)); + prefs.showTabs = toBoolean(rs.getString(13)); + prefs.showWhitespaceErrors = toBoolean(rs.getString(14)); + prefs.skipDeleted = toBoolean(rs.getString(15)); + prefs.skipUncommented = toBoolean(rs.getString(16)); + prefs.syntaxHighlighting = toBoolean(rs.getString(17)); + prefs.tabSize = rs.getInt(18); + // Enum with name as values; can be null + prefs.theme = toTheme(rs.getString(19)); + prefs.hideEmptyPane = toBoolean(rs.getString(20)); + prefs.autoHideDiffTableHeader = toBoolean(rs.getString(21)); + imports.put(accountId, prefs); + } + } + + if (imports.isEmpty()) { + return; + } + + try (Repository git = mgr.openRepository(allUsersName); + RevWalk rw = new RevWalk(git)) { + BatchRefUpdate bru = git.getRefDatabase().newBatchUpdate(); + MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, + allUsersName, git, bru); + md.getCommitBuilder().setAuthor(serverUser); + md.getCommitBuilder().setCommitter(serverUser); + + for (Map.Entry e : imports.entrySet()) { + VersionedAccountPreferences p = + VersionedAccountPreferences.forUser(e.getKey()); + p.load(md); + storeSection(p.getConfig(), UserConfigSections.DIFF, null, + e.getValue(), DiffPreferencesInfo.defaults()); + p.commit(md); + } + + bru.execute(rw, NullProgressMonitor.INSTANCE); + } catch (ConfigInvalidException | IOException ex) { + throw new OrmException(ex); + } + } + + private static Theme toTheme(String v) { + if (v == null) { + return Theme.DEFAULT; + } + return Theme.valueOf(v); + } + + private static Whitespace toWhitespace(String v) { + Preconditions.checkNotNull(v); + if (v.isEmpty()) { + return Whitespace.IGNORE_NONE; + } + Whitespace r = PatchListKey.WHITESPACE_TYPES.inverse().get(v.charAt(0)); + if (r == null) { + throw new IllegalArgumentException("Cannot find Whitespace type for: " + + v); + } + return r; + } + + private static boolean toBoolean(String v) { + Preconditions.checkState(!Strings.isNullOrEmpty(v)); + return v.equals("Y"); + } +} diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java index 9acae6d046..c526cea7d6 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java @@ -15,7 +15,6 @@ package com.google.gerrit.testutil; import com.google.gerrit.reviewdb.server.AccountAccess; -import com.google.gerrit.reviewdb.server.AccountDiffPreferenceAccess; import com.google.gerrit.reviewdb.server.AccountExternalIdAccess; import com.google.gerrit.reviewdb.server.AccountGroupAccess; import com.google.gerrit.reviewdb.server.AccountGroupByIdAccess; @@ -124,11 +123,6 @@ public class DisabledReviewDb implements ReviewDb { throw new Disabled(); } - @Override - public AccountDiffPreferenceAccess accountDiffPreferences() { - throw new Disabled(); - } - @Override public StarredChangeAccess starredChanges() { throw new Disabled();