Merge "Rewrite my menu preferences to avoid ProjectState caching"
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetPreferences.java
index f991ba2..401846a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetPreferences.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetPreferences.java
@@ -28,22 +28,27 @@
import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.DownloadCommand;
import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.DownloadScheme;
import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.TimeFormat;
-import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.project.ProjectCache;
-import com.google.gerrit.server.project.ProjectState;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.git.GitRepositoryManager;
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.Config;
+import org.eclipse.jgit.lib.Repository;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
public class GetPreferences implements RestReadView<AccountResource> {
- public static final String PREFERENCES = "preferences.config";
+ private static final Logger log = LoggerFactory.getLogger(GetPreferences.class);
+
public static final String MY = "my";
public static final String KEY_URL = "url";
public static final String KEY_TARGET = "target";
@@ -51,19 +56,26 @@
private final Provider<CurrentUser> self;
private final Provider<ReviewDb> db;
- private final ProjectState allUsers;
+ private final AllUsersName allUsersName;
+ private final GitRepositoryManager gitMgr;
@Inject
GetPreferences(Provider<CurrentUser> self, Provider<ReviewDb> db,
- ProjectCache projectCache) {
+ AllUsersName allUsersName,
+ GitRepositoryManager gitMgr) {
this.self = self;
this.db = db;
- this.allUsers = projectCache.getAllUsers();
+ this.allUsersName = allUsersName;
+ this.gitMgr = gitMgr;
}
@Override
public PreferenceInfo apply(AccountResource rsrc)
- throws AuthException, ResourceNotFoundException, OrmException {
+ throws AuthException,
+ ResourceNotFoundException,
+ OrmException,
+ IOException,
+ ConfigInvalidException {
if (self.get() != rsrc.getUser()
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("restricted to administrator");
@@ -72,8 +84,16 @@
if (a == null) {
throw new ResourceNotFoundException();
}
- return new PreferenceInfo(a.getGeneralPreferences(),
- rsrc.getUser().getAccountId(), allUsers);
+
+ Repository git = gitMgr.openRepository(allUsersName);
+ try {
+ VersionedAccountPreferences p =
+ VersionedAccountPreferences.forUser(rsrc.getUser().getAccountId());
+ p.load(git);
+ return new PreferenceInfo(a.getGeneralPreferences(), p, git);
+ } finally {
+ git.close();
+ }
}
public static class PreferenceInfo {
@@ -96,13 +116,8 @@
ChangeScreen changeScreen;
List<TopMenu.MenuItem> my;
- public PreferenceInfo(AccountGeneralPreferences p, Account.Id accountId,
- ProjectState allUsers) {
- this(p, RefNames.refsUsers(accountId), allUsers);
- }
-
- public PreferenceInfo(AccountGeneralPreferences p, String ref,
- ProjectState allUsers) {
+ public PreferenceInfo(AccountGeneralPreferences p,
+ VersionedAccountPreferences v, Repository allUsers) {
if (p != null) {
changesPerPage = p.getMaximumPageSize();
showSiteHeader = p.isShowSiteHeader() ? true : null;
@@ -120,13 +135,20 @@
diffView = p.getDiffView();
changeScreen = p.getChangeScreen();
}
- my = my(ref, allUsers);
+ my = my(v, allUsers);
}
- private List<TopMenu.MenuItem> my(String ref, ProjectState allUsers) {
- List<TopMenu.MenuItem> my = my(allUsers, ref);
- if (my.isEmpty() && !ref.equals(RefNames.REFS_USER + "default")) {
- my = my(allUsers, RefNames.REFS_USER + "default");
+ private List<TopMenu.MenuItem> my(VersionedAccountPreferences v,
+ Repository allUsers) {
+ List<TopMenu.MenuItem> my = my(v);
+ if (my.isEmpty() && !v.isDefaults()) {
+ try {
+ VersionedAccountPreferences d = VersionedAccountPreferences.forDefault();
+ d.load(allUsers);
+ my = my(d);
+ } catch (ConfigInvalidException | IOException e) {
+ log.warn("cannot read default preferences", e);
+ }
}
if (my.isEmpty()) {
my.add(new TopMenu.MenuItem("Changes", "#/dashboard/self", null));
@@ -138,9 +160,9 @@
return my;
}
- private List<TopMenu.MenuItem> my(ProjectState allUsers, String ref) {
+ private List<TopMenu.MenuItem> my(VersionedAccountPreferences v) {
List<TopMenu.MenuItem> my = new ArrayList<>();
- Config cfg = allUsers.getConfig(PREFERENCES, ref).get();
+ Config cfg = v.getConfig();
for (String subsection : cfg.getSubsections(MY)) {
String url = my(cfg, subsection, KEY_URL, "#/");
String target = my(cfg, subsection, KEY_TARGET,
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java
index baf8eda..df985ec 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetPreferences.java
@@ -18,7 +18,6 @@
import static com.google.gerrit.server.account.GetPreferences.KEY_TARGET;
import static com.google.gerrit.server.account.GetPreferences.KEY_URL;
import static com.google.gerrit.server.account.GetPreferences.MY;
-import static com.google.gerrit.server.account.GetPreferences.PREFERENCES;
import com.google.common.base.Strings;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -34,18 +33,16 @@
import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.DownloadCommand;
import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.DownloadScheme;
import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.TimeFormat;
-import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.SetPreferences.Input;
+import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.MetaDataUpdate;
-import com.google.gerrit.server.git.ProjectLevelConfig;
-import com.google.gerrit.server.project.ProjectCache;
-import com.google.gerrit.server.project.ProjectState;
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.Config;
import java.io.IOException;
@@ -76,22 +73,22 @@
private final AccountCache cache;
private final ReviewDb db;
private final MetaDataUpdate.User metaDataUpdateFactory;
- private final ProjectState allUsers;
+ private final AllUsersName allUsersName;
@Inject
SetPreferences(Provider<CurrentUser> self, AccountCache cache, ReviewDb db,
- MetaDataUpdate.User metaDataUpdateFactory, ProjectCache projectCache) {
+ MetaDataUpdate.User metaDataUpdateFactory, AllUsersName allUsersName) {
this.self = self;
this.cache = cache;
this.db = db;
this.metaDataUpdateFactory = metaDataUpdateFactory;
- this.allUsers = projectCache.getAllUsers();
+ this.allUsersName = allUsersName;
}
@Override
public GetPreferences.PreferenceInfo apply(AccountResource rsrc, Input i)
throws AuthException, ResourceNotFoundException, OrmException,
- IOException {
+ IOException, ConfigInvalidException {
if (self.get() != rsrc.getUser()
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("restricted to administrator");
@@ -102,6 +99,8 @@
Account.Id accountId = rsrc.getUser().getAccountId();
AccountGeneralPreferences p;
+ VersionedAccountPreferences versionedPrefs;
+ MetaDataUpdate md = metaDataUpdateFactory.create(allUsersName);
db.accounts().beginTransaction(accountId);
try {
Account a = db.accounts().get(accountId);
@@ -109,6 +108,9 @@
throw new ResourceNotFoundException();
}
+ versionedPrefs = VersionedAccountPreferences.forUser(accountId);
+ versionedPrefs.load(md);
+
p = a.getGeneralPreferences();
if (p == null) {
p = new AccountGeneralPreferences();
@@ -163,24 +165,21 @@
db.accounts().update(Collections.singleton(a));
db.commit();
- storeMyMenus(accountId, i.my);
+ storeMyMenus(versionedPrefs, i.my);
+ versionedPrefs.commit(md);
cache.evict(accountId);
+ return new GetPreferences.PreferenceInfo(
+ p, versionedPrefs,
+ md.getRepository());
} finally {
+ md.close();
db.rollback();
}
- return new GetPreferences.PreferenceInfo(p, accountId, allUsers);
}
- private void storeMyMenus(Account.Id accountId, List<TopMenu.MenuItem> my)
- throws IOException {
- storeMyMenus(RefNames.refsUsers(accountId), my);
- }
-
- public void storeMyMenus(String ref, List<TopMenu.MenuItem> my)
- throws IOException {
- ProjectLevelConfig prefsCfg =
- allUsers.getConfig(PREFERENCES, ref);
- Config cfg = prefsCfg.get();
+ public static void storeMyMenus(VersionedAccountPreferences prefs,
+ List<TopMenu.MenuItem> my) {
+ Config cfg = prefs.getConfig();
if (my != null) {
unsetSection(cfg, MY);
for (TopMenu.MenuItem item : my) {
@@ -189,10 +188,6 @@
set(cfg, item.name, KEY_ID, item.id);
}
}
- MetaDataUpdate md =
- metaDataUpdateFactory.create(allUsers.getProject().getNameKey());
- md.setMessage("Updated preferences\n");
- prefsCfg.commit(md);
}
private static void set(Config cfg, String section, String key, String val) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAccountPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAccountPreferences.java
new file mode 100644
index 0000000..c4d4b06
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAccountPreferences.java
@@ -0,0 +1,75 @@
+// Copyright (C) 2014 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.common.base.Strings;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.git.VersionedMetaData;
+
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.Config;
+
+import java.io.IOException;
+
+/** Preferences for user accounts. */
+public class VersionedAccountPreferences extends VersionedMetaData {
+ private static final String REFS_USER_DEFAULT = RefNames.REFS_USER + "default";
+ private static final String PREFERENCES = "preferences.config";
+
+ public static VersionedAccountPreferences forUser(Account.Id id) {
+ return new VersionedAccountPreferences(RefNames.refsUsers(id));
+ }
+
+ public static VersionedAccountPreferences forDefault() {
+ return new VersionedAccountPreferences(REFS_USER_DEFAULT);
+ }
+
+ private final String ref;
+ private Config cfg;
+
+ private VersionedAccountPreferences(String ref) {
+ this.ref = ref;
+ }
+
+ public boolean isDefaults() {
+ return REFS_USER_DEFAULT.equals(getRefName());
+ }
+
+ @Override
+ protected String getRefName() {
+ return ref;
+ }
+
+ public Config getConfig() {
+ return cfg;
+ }
+
+ @Override
+ protected void onLoad() throws IOException, ConfigInvalidException {
+ cfg = readConfig(PREFERENCES);
+ }
+
+ @Override
+ protected boolean onSave(CommitBuilder commit) throws IOException,
+ ConfigInvalidException {
+ if (Strings.isNullOrEmpty(commit.getMessage())) {
+ commit.setMessage("Updated preferences\n");
+ }
+ saveConfig(PREFERENCES, cfg);
+ return true;
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GetPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GetPreferences.java
index 8cd7c19..f6c1d1d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GetPreferences.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GetPreferences.java
@@ -15,22 +15,38 @@
package com.google.gerrit.server.config;
import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.account.GetPreferences.PreferenceInfo;
-import com.google.gerrit.server.project.ProjectCache;
-import com.google.gerrit.server.project.ProjectState;
+import com.google.gerrit.server.account.VersionedAccountPreferences;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.Repository;
+
+import java.io.IOException;
+
public class GetPreferences implements RestReadView<ConfigResource> {
- private final ProjectState allUsers;
+ private final AllUsersName allUsersName;
+ private final GitRepositoryManager gitMgr;
@Inject
- public GetPreferences(ProjectCache projectCache) {
- this.allUsers = projectCache.getAllUsers();
+ public GetPreferences(AllUsersName allUsersName,
+ GitRepositoryManager gitMgr) {
+ this.allUsersName = allUsersName;
+ this.gitMgr = gitMgr;
}
@Override
- public PreferenceInfo apply(ConfigResource rsrc) {
- return new PreferenceInfo(null, RefNames.REFS_USER + "default", allUsers);
+ public PreferenceInfo apply(ConfigResource rsrc)
+ throws IOException, ConfigInvalidException {
+ Repository git = gitMgr.openRepository(allUsersName);
+ try {
+ VersionedAccountPreferences p =
+ VersionedAccountPreferences.forDefault();
+ p.load(git);
+ return new PreferenceInfo(null, p, git);
+ } finally {
+ git.close();
+ }
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java
index f93e908..e6f5253 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/SetPreferences.java
@@ -18,29 +18,31 @@
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.account.GetPreferences.PreferenceInfo;
import com.google.gerrit.server.account.SetPreferences.Input;
+import com.google.gerrit.server.account.VersionedAccountPreferences;
+import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.inject.Inject;
-import com.google.inject.Provider;
+
+import org.eclipse.jgit.errors.ConfigInvalidException;
import java.io.IOException;
@RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
public class SetPreferences implements RestModifyView<ConfigResource, Input> {
- private final Provider<com.google.gerrit.server.account.SetPreferences> setPreferences;
- private final Provider<GetPreferences> getDefaultPreferences;
+ private final MetaDataUpdate.User metaDataUpdateFactory;
+ private final AllUsersName allUsersName;
@Inject
- SetPreferences(
- Provider<com.google.gerrit.server.account.SetPreferences> setPreferences,
- Provider<GetPreferences> getDefaultPreferences) {
- this.setPreferences = setPreferences;
- this.getDefaultPreferences = getDefaultPreferences;
+ SetPreferences(MetaDataUpdate.User metaDataUpdateFactory,
+ AllUsersName allUsersName) {
+ this.metaDataUpdateFactory = metaDataUpdateFactory;
+ this.allUsersName = allUsersName;
}
@Override
public Object apply(ConfigResource rsrc, Input i) throws BadRequestException,
- IOException {
+ IOException, ConfigInvalidException {
if (i.changesPerPage != null || i.showSiteHeader != null
|| i.useFlashClipboard != null || i.downloadScheme != null
|| i.downloadCommand != null || i.copySelfOnEmail != null
@@ -53,9 +55,17 @@
|| i.changeScreen != null) {
throw new BadRequestException("unsupported option");
}
- if (i.my != null) {
- setPreferences.get().storeMyMenus(RefNames.REFS_USER + "default", i.my);
+
+ VersionedAccountPreferences p;
+ MetaDataUpdate md = metaDataUpdateFactory.create(allUsersName);
+ try {
+ p = VersionedAccountPreferences.forDefault();
+ p.load(md);
+ com.google.gerrit.server.account.SetPreferences.storeMyMenus(p, i.my);
+ p.commit(md);
+ return new PreferenceInfo(null, p, md.getRepository());
+ } finally {
+ md.close();
}
- return getDefaultPreferences.get().apply(rsrc);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java
index e8b4b6a..b48028e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MetaDataUpdate.java
@@ -137,7 +137,7 @@
return projectName;
}
- Repository getRepository() {
+ public Repository getRepository() {
return db;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectLevelConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectLevelConfig.java
index b141a46..b4f41a0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectLevelConfig.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectLevelConfig.java
@@ -29,23 +29,17 @@
/** Configuration file in the projects refs/meta/config branch. */
public class ProjectLevelConfig extends VersionedMetaData {
private final String fileName;
- private final String ref;
private final ProjectState project;
private Config cfg;
public ProjectLevelConfig(String fileName, ProjectState project) {
- this(fileName, RefNames.REFS_CONFIG, project);
- }
-
- public ProjectLevelConfig(String fileName, String ref, ProjectState project) {
this.fileName = fileName;
this.project = project;
- this.ref = ref;
}
@Override
protected String getRefName() {
- return ref;
+ return RefNames.REFS_CONFIG;
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
index 88f4d1f..69f76e3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
@@ -87,7 +87,7 @@
private final List<CommentLinkInfo> commentLinks;
private final ProjectConfig config;
- private final Map<ConfigKey, ProjectLevelConfig> configs;
+ private final Map<String, ProjectLevelConfig> configs;
private final Set<AccountGroup.UUID> localOwners;
/** Prolog rule state. */
@@ -223,16 +223,11 @@
}
public ProjectLevelConfig getConfig(String fileName) {
- return getConfig(fileName, RefNames.REFS_CONFIG);
- }
-
- public ProjectLevelConfig getConfig(String fileName, String ref) {
- ConfigKey cfgKey = new ConfigKey(fileName, ref);
- if (configs.containsKey(cfgKey)) {
- return configs.get(cfgKey);
+ if (configs.containsKey(fileName)) {
+ return configs.get(fileName);
}
- ProjectLevelConfig cfg = new ProjectLevelConfig(fileName, ref, this);
+ ProjectLevelConfig cfg = new ProjectLevelConfig(fileName, this);
try {
Repository git = gitMgr.openRepository(getProject().getNameKey());
try {
@@ -246,7 +241,7 @@
log.warn("Failed to load " + fileName + " for " + getProject().getName(), e);
}
- configs.put(cfgKey, cfg);
+ configs.put(fileName, cfg);
return cfg;
}
@@ -505,26 +500,4 @@
}
return false;
}
-
- private static class ConfigKey {
- final String file;
- final String ref;
-
- ConfigKey(String file, String ref) {
- this.file = file;
- this.ref = ref;
- }
-
- @Override
- public boolean equals(Object other) {
- return other instanceof ConfigKey
- && file.equals(((ConfigKey) other).file)
- && ref.equals(((ConfigKey) other).ref);
- }
-
- @Override
- public int hashCode() {
- return file.hashCode() * 31 + ref.hashCode();
- }
- }
}
