Use StoredValues.PLUGIN_CONFIG_FACTORY
* Construct a Config object for each request to read latest
configuration, instead of using static variables and functions.
* Config parameters are retrieved from injected pluginConfigFactory
or StoredValues.PLUGIN_CONFIG_FACTORY.
* Use Config.PLUGIN_NAME as fixed plugin name; no more injected name.
* Use config.getDefaultOwnersFileName to find configured file name.
* Cache, OwnersDb, and Checker store and pass a PluginConfigFactory.
* One Cache per GitRepositoryManager.
* Use repositoryManager instead of branch name for dbCache key.
* Coding style improvements:
* Use private final for members only assigned in constructors.
* Remove unnecessary setup code in PredicagteModule
* Move overloaded findApproval functions to be next to each other.
* Use simpler functions owner2Paths.size().
Change-Id: I68bfb879a5a6ab4d89562b8b45b0a73b713dd587
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Action.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Action.java
index 8edb479..6da211c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Action.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Action.java
@@ -16,10 +16,8 @@
import static java.util.stream.Collectors.toList;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Streams;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -54,12 +52,14 @@
class Action implements RestReadView<RevisionResource>, UiAction<RevisionResource> {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private AccountCache accountCache;
- private Emails emails;
- private ChangeData.Factory changeDataFactory;
- private GitRepositoryManager repoManager;
- private Provider<CurrentUser> userProvider;
- private ProjectCache projectCache;
+ private final AccountCache accountCache;
+ private final Emails emails;
+ private final ChangeData.Factory changeDataFactory;
+ private final GitRepositoryManager repoManager;
+ private final PluginConfigFactory configFactory;
+ private final Provider<CurrentUser> userProvider;
+ private final ProjectCache projectCache;
+ private final Config config;
static class Parameters {
Boolean debug; // REST API "debug" parameter, or null
@@ -69,7 +69,6 @@
@Inject
Action(
- @PluginName String pluginName,
PluginConfigFactory configFactory,
Provider<CurrentUser> userProvider,
ChangeData.Factory changeDataFactory,
@@ -83,13 +82,10 @@
this.emails = emails;
this.repoManager = repoManager;
this.projectCache = projectCache;
- Config.setVariables(pluginName, configFactory);
- Cache.getInstance(); // Create a single Cache.
+ this.configFactory = configFactory;
+ this.config = new Config(configFactory);
}
- @VisibleForTesting
- Action() {}
-
private String getUserName() {
if (userProvider != null && userProvider.get().getUserName().isPresent()) {
return userProvider.get().getUserName().get();
@@ -166,14 +162,14 @@
int patchset = getValidPatchsetNum(changeData, params.patchset);
ProjectState projectState = projectCache.get(changeData.project());
Boolean useCache = params.nocache == null || !params.nocache;
- OwnersDb db = Cache.getInstance().get(
- useCache, projectState, accountCache, emails, repoManager, changeData, patchset);
+ OwnersDb db = Cache.getInstance(configFactory, repoManager).get(
+ useCache, projectState, accountCache, emails, repoManager, configFactory, changeData, patchset);
Collection<String> changedFiles = changeData.currentFilePaths();
Map<String, Set<String>> file2Owners = db.findOwners(changedFiles);
- Boolean addDebugMsg = (params.debug != null) ? params.debug : Config.getAddDebugMsg();
+ Boolean addDebugMsg = (params.debug != null) ? params.debug : config.getAddDebugMsg();
RestResult obj =
- new RestResult(Config.getMinOwnerVoteLevel(projectState, changeData), addDebugMsg);
+ new RestResult(config.getMinOwnerVoteLevel(projectState, changeData), addDebugMsg);
obj.change = changeData.getId().get();
obj.patchset = patchset;
obj.ownerRevision = db.revision;
@@ -214,16 +210,17 @@
&& status != Status.ABANDONED
&& status != Status.MERGED;
// If alwaysShowButton is true, skip expensive owner lookup.
- if (needFindOwners && !Config.getAlwaysShowButton()) {
+ if (needFindOwners && !config.getAlwaysShowButton()) {
needFindOwners = false; // Show button only if some owner is found.
OwnersDb db =
- Cache.getInstance()
+ Cache.getInstance(configFactory, repoManager)
.get(
true, // use cached OwnersDb
projectCache.get(resource.getProject()),
accountCache,
emails,
repoManager,
+ configFactory,
changeData);
logger.atFiner().log("getDescription db key = %s", db.key);
needFindOwners = db.getNumOwners() > 0;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Cache.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Cache.java
index df56fc7..072c087 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Cache.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Cache.java
@@ -20,12 +20,16 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.Emails;
+import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import java.io.IOException;
import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.WeakHashMap;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
@@ -48,18 +52,17 @@
// It would not be enough to keep the cache in a Prolog engine environment
// or a StoredValues.
- private static Cache instance = null; // a singleton
+ // Since a Java runtime can host multiple Gerrit sites, each site should have
+ // its own cache. We assume that each site has its own GitRepositoryManager.
+ // We use a WeakHashMap to avoid leaking objects in the map.
+ private static final Map<GitRepositoryManager, Cache> cacheMap =
+ Collections.synchronizedMap(new WeakHashMap<GitRepositoryManager, Cache>());
- // When branch is "refs/heads/xyz" use only "xyz",
- // to share cached OwnersDb between these two branch names.
- private static final String REFS_HEADS = "refs/heads/";
-
- // Until we have a way to inject Java singleton into Prolog PRED_* constructors,
- // we keep the cache in a static object for all HTTP requests.
+ // dbCache key is generated by makeKey.
private com.google.common.cache.Cache<String, OwnersDb> dbCache;
- private Cache() {
- init(Config.getMaxCacheAge(), Config.getMaxCacheSize());
+ private Cache(int maxSeconds, int maxSize) {
+ init(maxSeconds, maxSize);
}
long size() {
@@ -93,6 +96,7 @@
AccountCache accountCache,
Emails emails,
GitRepositoryManager repoManager,
+ PluginConfigFactory configFactory,
ChangeData changeData)
throws OrmException, IOException {
return get(
@@ -101,6 +105,7 @@
accountCache,
emails,
repoManager,
+ configFactory,
changeData,
changeData.currentPatchSet().getId().get());
}
@@ -112,11 +117,12 @@
AccountCache accountCache,
Emails emails,
GitRepositoryManager repoManager,
+ PluginConfigFactory configFactory,
ChangeData changeData,
int patchset)
throws OrmException, IOException {
String branch = changeData.change().getDest().get();
- String dbKey = Cache.makeKey(changeData.getId().get(), patchset, branch);
+ String dbKey = Cache.makeKey(changeData.getId().get(), patchset, repoManager);
// TODO: get changed files of the given patchset?
return get(
useCache,
@@ -125,6 +131,7 @@
emails,
dbKey,
repoManager,
+ configFactory,
changeData,
branch,
changeData.currentFilePaths());
@@ -138,13 +145,22 @@
Emails emails,
String key,
GitRepositoryManager repoManager,
+ PluginConfigFactory configFactory,
ChangeData changeData,
String branch,
Collection<String> files) {
if (dbCache == null || !useCache) { // Do not cache OwnersDb
logger.atFiner().log("Create new OwnersDb, key=%s", key);
return new OwnersDb(
- projectState, accountCache, emails, key, repoManager, changeData, branch, files);
+ projectState,
+ accountCache,
+ emails,
+ key,
+ repoManager,
+ configFactory,
+ changeData,
+ branch,
+ files);
}
try {
logger.atFiner().log(
@@ -156,28 +172,45 @@
public OwnersDb call() {
logger.atFiner().log("Create new OwnersDb, key=%s", key);
return new OwnersDb(
- projectState, accountCache, emails, key, repoManager, changeData, branch, files);
+ projectState,
+ accountCache,
+ emails,
+ key,
+ repoManager,
+ configFactory,
+ changeData,
+ branch,
+ files);
}
});
} catch (ExecutionException e) {
logger.atSevere().withCause(e).log(
"Cache.get has exception for %s", Config.getChangeId(changeData));
return new OwnersDb(
- projectState, accountCache, emails, key, repoManager, changeData, branch, files);
+ projectState,
+ accountCache,
+ emails,
+ key,
+ repoManager,
+ configFactory,
+ changeData,
+ branch,
+ files);
}
}
- public static String makeKey(int change, int patch, String branch) {
- if (branch.indexOf(REFS_HEADS) == 0) {
- branch = branch.substring(REFS_HEADS.length());
- }
- return change + ":" + patch + ":" + branch;
+ public static String makeKey(int change, int patch, GitRepositoryManager repoManager) {
+ return String.format("%d:%d:%H", change, patch, repoManager);
}
- public static Cache getInstance() {
- if (instance == null) {
- instance = new Cache();
+ public static Cache getInstance(
+ PluginConfigFactory configFactory, GitRepositoryManager repoManager) {
+ Cache cache = cacheMap.get(repoManager);
+ if (cache == null) {
+ Config config = new Config(configFactory);
+ cache = new Cache(config.getMaxCacheAge(), config.getMaxCacheSize());
+ cacheMap.put(repoManager, cache);
}
- return instance;
+ return cache;
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java
index 187e87b..7b7540b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java
@@ -19,6 +19,7 @@
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.Emails;
+import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
@@ -40,12 +41,19 @@
private static final String EXEMPT_MESSAGE2 = "Exempted-From-Owner-Approval:";
private final GitRepositoryManager repoManager;
+ private final PluginConfigFactory configFactory;
+ private final Config config;
+ private final ProjectState projectState; // could be null when used by FindOwnersIT
private final ChangeData changeData;
private int minVoteLevel;
- Checker(GitRepositoryManager repoManager, ChangeData changeData, int v) {
+ Checker(GitRepositoryManager repoManager, PluginConfigFactory configFactory,
+ ProjectState projectState, ChangeData changeData, int v) {
this.repoManager = repoManager;
+ this.configFactory = configFactory;
+ this.projectState = projectState;
this.changeData = changeData;
+ this.config = new Config(configFactory);
minVoteLevel = v;
}
@@ -97,7 +105,7 @@
/** Returns 1 if owner approval is found, -1 if missing, 0 if unneeded. */
int findApproval(AccountCache accountCache, OwnersDb db) throws OrmException, IOException {
Map<String, Set<String>> file2Owners = db.findOwners(changeData.currentFilePaths());
- if (file2Owners.size() == 0) { // do not need owner approval
+ if (file2Owners.isEmpty()) { // do not need owner approval
return 0;
}
Map<String, Integer> votes = getVotes(accountCache, changeData);
@@ -114,18 +122,41 @@
ChangeData changeData = null;
try {
changeData = StoredValues.CHANGE_DATA.get(engine);
- ProjectState projectState = StoredValues.PROJECT_STATE.get(engine);
- GitRepositoryManager repoManager = StoredValues.REPO_MANAGER.get(engine);
- AccountCache accountCache = StoredValues.ACCOUNT_CACHE.get(engine);
- Emails emails = StoredValues.EMAILS.get(engine);
- return new Checker(repoManager, changeData, minVoteLevel)
- .findApproval(projectState, accountCache, emails);
+ Checker checker = new Checker(
+ StoredValues.REPO_MANAGER.get(engine),
+ StoredValues.PLUGIN_CONFIG_FACTORY.get(engine),
+ StoredValues.PROJECT_STATE.get(engine),
+ changeData, minVoteLevel);
+ return checker.findApproval(
+ StoredValues.ACCOUNT_CACHE.get(engine),
+ StoredValues.EMAILS.get(engine));
} catch (OrmException | IOException e) {
logger.atSevere().withCause(e).log("Exception for %s ", Config.getChangeId(changeData));
return 0; // owner approval may or may not be required.
}
}
+ /** Returns 1 if owner approval is found, -1 if missing, 0 if unneeded. */
+ int findApproval(AccountCache accountCache, Emails emails)
+ throws OrmException, IOException {
+ if (isExemptFromOwnerApproval(changeData)) {
+ return 0;
+ }
+ // One update to a Gerrit change can call submit_rule or submit_filter
+ // many times. So this function should use cached values.
+ OwnersDb db =
+ Cache.getInstance(configFactory, repoManager)
+ .get(true, projectState, accountCache, emails, repoManager, configFactory, changeData);
+ if (db.getNumOwners() <= 0) {
+ return 0;
+ }
+ if (minVoteLevel <= 0) {
+ minVoteLevel = config.getMinOwnerVoteLevel(projectState, changeData);
+ }
+ logger.atFiner().log("findApproval db key = %s", db.key);
+ return findApproval(accountCache, db);
+ }
+
/** Returns true if exempt from owner approval. */
static boolean isExemptFromOwnerApproval(ChangeData changeData) throws OrmException {
try {
@@ -142,23 +173,4 @@
Status status = changeData.change().getStatus();
return (status == Status.ABANDONED || status == Status.MERGED);
}
-
- int findApproval(ProjectState projectState, AccountCache accountCache, Emails emails)
- throws OrmException, IOException {
- if (isExemptFromOwnerApproval(changeData)) {
- return 0;
- }
- // One update to a Gerrit change can call submit_rule or submit_filter
- // many times. So this function should use cached values.
- OwnersDb db =
- Cache.getInstance().get(true, projectState, accountCache, emails, repoManager, changeData);
- if (db.getNumOwners() <= 0) {
- return 0;
- }
- if (minVoteLevel <= 0) {
- minVoteLevel = Config.getMinOwnerVoteLevel(projectState, changeData);
- }
- logger.atFiner().log("findApproval db key = %s", db.key);
- return findApproval(accountCache, db);
- }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java
index 20b5d11..0653f3f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java
@@ -38,23 +38,25 @@
static final String PLUGIN_NAME = "find-owners";
static final String PROLOG_NAMESPACE = "find_owners";
+ private final PluginConfigFactory configFactory;
+
// Global/plugin config parameters.
- private static PluginConfigFactory config = null;
- private static boolean addDebugMsg = false;
- private static int minOwnerVoteLevel = 1;
- private static int maxCacheAge = 0;
- private static int maxCacheSize = 1000;
- private static boolean reportSyntaxError = false;
- private static boolean alwaysShowButton = false;
+ private boolean addDebugMsg = false;
+ private int minOwnerVoteLevel = 1;
+ private int maxCacheAge = 0;
+ private int maxCacheSize = 1000;
+ private boolean reportSyntaxError = false;
+ private boolean alwaysShowButton = false;
+ private String ownersFileName = OWNERS;
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- static void setVariables(String pluginName, PluginConfigFactory conf) {
- if (conf == null) { // When called from integration tests.
+ Config(PluginConfigFactory configFactory) {
+ this.configFactory = configFactory;
+ if (configFactory == null) { // When called from integration tests.
return;
}
- config = conf;
- PluginConfig gc = config.getFromGerritConfig(pluginName, true);
+ PluginConfig gc = configFactory.getFromGerritConfig(PLUGIN_NAME);
// Get config variables from the plugin section of gerrit.config
addDebugMsg = gc.getBoolean(ADD_DEBUG_MSG, false);
reportSyntaxError = gc.getBoolean(REPORT_SYNTAX_ERROR, false);
@@ -62,25 +64,26 @@
minOwnerVoteLevel = gc.getInt(MIN_OWNER_VOTE_LEVEL, 1);
maxCacheAge = gc.getInt(MAX_CACHE_AGE, 0);
maxCacheSize = gc.getInt(MAX_CACHE_SIZE, 1000);
+ ownersFileName = gc.getString(OWNERS_FILE_NAME, OWNERS);
}
- static boolean getAddDebugMsg() {
+ boolean getAddDebugMsg() {
return addDebugMsg; // defined globally, not per-project
}
- static int getMaxCacheAge() {
+ int getMaxCacheAge() {
return maxCacheAge;
}
- static int getMaxCacheSize() {
+ int getMaxCacheSize() {
return maxCacheSize;
}
- static boolean getReportSyntaxError() {
+ boolean getReportSyntaxError() {
return reportSyntaxError;
}
- static boolean getAlwaysShowButton() {
+ boolean getAlwaysShowButton() {
return alwaysShowButton;
}
@@ -88,43 +91,39 @@
return data == null ? "(unknown change)" : ("change c/" + data.getId().get());
}
- static String getDefaultOwnersFileName() {
- return OWNERS;
+ String getDefaultOwnersFileName() {
+ return ownersFileName; // could be defined in gerrit.config
}
- static String getOwnersFileName(ProjectState projectState, ChangeData c) {
+ private PluginConfig getPluginConfig(ProjectState state) {
+ return configFactory.getFromProjectConfigWithInheritance(state, PLUGIN_NAME);
+ }
+
+ String getOwnersFileName(ProjectState projectState, ChangeData c) {
+ String defaultName = getDefaultOwnersFileName();
if (projectState == null) {
logger.atSevere().log("Null projectState for change %s", getChangeId(c));
- } else if (config != null) {
- String name =
- config
- .getFromProjectConfigWithInheritance(projectState, PLUGIN_NAME)
- .getString(OWNERS_FILE_NAME, OWNERS);
- if (name.trim().equals("")) {
- logger.atSevere().log("Project %s has wrong %s: \"%s\" for %s",
- projectState.getProject(), OWNERS_FILE_NAME, name, getChangeId(c));
- return OWNERS;
- }
- return name;
+ return defaultName;
}
- return OWNERS;
+ String name = getPluginConfig(projectState).getString(OWNERS_FILE_NAME, defaultName);
+ if (name.trim().isEmpty()) {
+ logger.atSevere().log("Project %s has wrong %s: \"%s\" for %s",
+ projectState.getProject(), OWNERS_FILE_NAME, name, getChangeId(c));
+ return defaultName;
+ }
+ return name;
}
@VisibleForTesting
- static void setReportSyntaxError(boolean value) {
+ void setReportSyntaxError(boolean value) {
reportSyntaxError = value;
}
- static int getMinOwnerVoteLevel(ProjectState projectState, ChangeData c) {
+ int getMinOwnerVoteLevel(ProjectState projectState, ChangeData c) {
if (projectState == null) {
logger.atSevere().log("Null projectState for change %s", getChangeId(c));
return minOwnerVoteLevel;
- } else if (config == null) {
- return minOwnerVoteLevel;
- } else {
- return config
- .getFromProjectConfigWithInheritance(projectState, PLUGIN_NAME)
- .getInt(MIN_OWNER_VOTE_LEVEL, minOwnerVoteLevel);
}
+ return getPluginConfig(projectState).getInt(MIN_OWNER_VOTE_LEVEL, minOwnerVoteLevel);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/GetOwners.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/GetOwners.java
index f1b2303..f08056d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/GetOwners.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/GetOwners.java
@@ -15,7 +15,6 @@
package com.googlesource.gerrit.plugins.findowners;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
@@ -53,7 +52,6 @@
@Inject
GetOwners(
- @PluginName String pluginName,
PluginConfigFactory configFactory,
Provider<CurrentUser> userProvider,
ChangeData.Factory dataFactory,
@@ -63,7 +61,6 @@
ProjectCache projectCache) {
this.action =
new Action(
- pluginName,
configFactory,
userProvider,
dataFactory,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java
index 23cc28a..41b97cd 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java
@@ -22,6 +22,7 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.Emails;
+import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
@@ -50,9 +51,10 @@
class OwnersDb {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private AccountCache accountCache;
- private GitRepositoryManager repoManager;
- private Emails emails;
+ private final AccountCache accountCache;
+ private final GitRepositoryManager repoManager;
+ private final Emails emails;
+ private final Config config;
private int numOwners = -1; // # of owners of all given files.
String key = ""; // key to find this OwnersDb in a cache.
@@ -66,14 +68,13 @@
List<String> errors = new ArrayList<>(); // error messages
List<String> logs = new ArrayList<>(); // trace/debug messages
- OwnersDb() {}
-
OwnersDb(
ProjectState projectState,
AccountCache accountCache,
Emails emails,
String key,
GitRepositoryManager repoManager,
+ PluginConfigFactory configFactory,
ChangeData changeData,
String branch,
Collection<String> files) {
@@ -81,6 +82,7 @@
this.repoManager = repoManager;
this.emails = emails;
this.key = key;
+ this.config = new Config(configFactory);
try {
InetAddress inetAddress = InetAddress.getLocalHost();
logs.add("HostName:" + inetAddress.getHostName());
@@ -91,7 +93,7 @@
preferredEmails.put("*", "*");
String projectName = projectState.getName();
logs.add("project:" + projectName);
- String ownersFileName = Config.getOwnersFileName(projectState, changeData);
+ String ownersFileName = config.getOwnersFileName(projectState, changeData);
logs.add("ownersFileName:" + ownersFileName);
try (Repository repo = repoManager.openRepository(projectState.getNameKey())) {
// Some hacked CL could have a target branch that is not created yet.
@@ -151,7 +153,7 @@
}
int getNumOwners() {
- return (numOwners >= 0) ? numOwners : owner2Paths.keySet().size();
+ return (numOwners >= 0) ? numOwners : owner2Paths.size();
}
private void countNumOwners(Collection<String> files) {
@@ -162,7 +164,7 @@
file2Owners.values().forEach(emails::addAll);
numOwners = emails.size();
} else {
- numOwners = owner2Paths.keySet().size();
+ numOwners = owner2Paths.size();
}
}
@@ -227,7 +229,7 @@
addOwnerPathPair(email, path);
}
}
- if (Config.getReportSyntaxError()) {
+ if (config.getReportSyntaxError()) {
result.warnings.forEach(w -> logger.atWarning().log(w));
result.errors.forEach(w -> logger.atSevere().log(w));
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/PredicateModule.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/PredicateModule.java
index d0fb040..6a2e28d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/PredicateModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/PredicateModule.java
@@ -15,24 +15,15 @@
package com.googlesource.gerrit.plugins.findowners;
import com.google.common.collect.ImmutableSet;
-import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.rules.PredicateProvider;
import com.google.inject.AbstractModule;
-import com.google.inject.Inject;
/** provides the Prolog predicate, even in a batch mode */
public class PredicateModule extends AbstractModule {
/** Prolog Predicate Provider. */
static class FindOwnersProvider implements PredicateProvider {
- @Inject
- public FindOwnersProvider(@PluginName String pluginName, PluginConfigFactory configFactory) {
- Config.setVariables(pluginName, configFactory);
- Cache.getInstance(); // Create a single Cache.
- }
-
@Override
public ImmutableSet<String> getPackages() {
return ImmutableSet.of(Config.PROLOG_NAMESPACE);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
index 0ef3b3e..7559da9 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
@@ -39,7 +39,6 @@
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.change.ChangeResource;
-import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.inject.Inject;
import java.util.Collection;
import java.util.Optional;
@@ -50,6 +49,7 @@
import org.eclipse.jgit.transport.RefSpec;
import org.eclipse.jgit.treewalk.TreeWalk;
import org.eclipse.jgit.treewalk.filter.PathFilterGroup;
+import org.junit.Before;
import org.junit.Test;
/** Test find-owners plugin API. */
@@ -57,8 +57,13 @@
public class FindOwnersIT extends LightweightPluginDaemonTest {
@Inject private Emails emails;
- @Inject private PluginConfigFactory configFactory;
@Inject private ProjectOperations projectOperations;
+ private Config config;
+
+ @Before
+ public void setConfig() {
+ config = new Config(pluginConfig);
+ }
@Test
public void getOwnersTest() throws Exception {
@@ -564,7 +569,7 @@
// Default owners file name is "OWNERS".
assertThat(Config.OWNERS).isEqualTo("OWNERS");
- assertThat(Config.getDefaultOwnersFileName()).isEqualTo("OWNERS");
+ assertThat(config.getDefaultOwnersFileName()).isEqualTo("OWNERS");
assertThat(projectOwnersFileName(pA)).isEqualTo("OWNERS");
assertThat(projectOwnersFileName(pB)).isEqualTo("OWNERS");
@@ -697,8 +702,7 @@
Action.Parameters param = new Action.Parameters();
Action action =
new Action(
- Config.PLUGIN_NAME,
- configFactory,
+ pluginConfig,
null,
changeDataFactory,
accountCache,
@@ -828,8 +832,8 @@
Project.NameKey project = r.getChange().project();
Cache cache = getCache().init(0, 0);
OwnersDb db = cache.get(true, projectCache.get(project), accountCache, emails,
- repoManager, r.getChange(), 1);
- Checker c = new Checker(repoManager, r.getChange(), 1);
+ repoManager, pluginConfig, r.getChange(), 1);
+ Checker c = new Checker(repoManager, pluginConfig, null, r.getChange(), 1);
return c.findApproval(accountCache, db);
}
@@ -889,10 +893,10 @@
}
private String projectOwnersFileName(Project.NameKey name) {
- return Config.getOwnersFileName(projectCache.get(name), null);
+ return config.getOwnersFileName(projectCache.get(name), null);
}
private Cache getCache() {
- return Cache.getInstance();
+ return Cache.getInstance(pluginConfig, repoManager);
}
}
