Cache all parsed config values to avoid reparsing them
The code-owners plugin has quite a lot of configuration parameters that
are accessed frequently. If reading config parameters is slow, this can
sum up to a significant amount of latency.
What we have done so far is that CodeOwnersPluginConfigSnapshot which
reads the project specific plugin configuration is cached for the time
of the request (see change If541bc329). This avoids loading the
code-owners.config file from this project and the parent projects again
and again. However whenever a config parameter is needed, we still
reparse it each time. Parsing a config parameter means inspecting the
Config that was read from the code-owners.config files and if necessary
fall-back to inspecting the Config that was read from gerrit.config.
With this change we do the parsing only once and cache the result in
CodeOwnersPluginConfigSnapshot.
In addition to project specific configuration parameters that are
handled by CodeOwnersPluginConfigSnapshot there are also a couple of
global configuration parameters that are handled in
CodeOwnersPluginConfiguration. For the global parameters we have the
same problem that they are reparsed on each access. E.g. the allowed
email domains are parsed each time a code owner email is resolved to an
account. This can be the reason why the step that resolves code owner
emails to accounts is rather slow. To address this we follow the example
of CodeOwnersPluginConfigSnapshot and add a
CodeOwnersPluginGlobalConfigSnapshot class that handles the reading of
the global config parameters. Same as CodeOwnersPluginConfigSnapshot it
is cached for the request time and internally it caches the parsed
config values so that they need to parsed only once.
To make it clearer that CodeOwnersPluginConfigSnapshot is about reading
project-specific configuration parameters, it is renamed to
CodeOwnersPluginProjectConfigSnapshot.
Change-Id: I6e4c608133717b598d356d1ce76246eb19b08c4b
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/BackendModule.java b/java/com/google/gerrit/plugins/codeowners/backend/BackendModule.java
index 0ba6d41..e137ba7 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/BackendModule.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/BackendModule.java
@@ -20,7 +20,8 @@
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfig;
-import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfigSnapshot;
+import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginGlobalConfigSnapshot;
+import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginProjectConfigSnapshot;
import com.google.gerrit.server.ExceptionHook;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ServerInitiated;
@@ -34,7 +35,8 @@
protected void configure() {
factory(CodeOwnersUpdate.Factory.class);
factory(CodeOwnerConfigScanner.Factory.class);
- factory(CodeOwnersPluginConfigSnapshot.Factory.class);
+ factory(CodeOwnersPluginGlobalConfigSnapshot.Factory.class);
+ factory(CodeOwnersPluginProjectConfigSnapshot.Factory.class);
factory(CodeOwnersPluginConfig.Factory.class);
DynamicMap.mapOf(binder(), CodeOwnerBackend.class);
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
index f98ecb8..792087d 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
@@ -33,8 +33,8 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.metrics.Timer0;
-import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfigSnapshot;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
+import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginProjectConfigSnapshot;
import com.google.gerrit.plugins.codeowners.backend.config.RequiredApproval;
import com.google.gerrit.plugins.codeowners.common.ChangedFile;
import com.google.gerrit.plugins.codeowners.common.CodeOwnerStatus;
@@ -260,7 +260,7 @@
"prepare stream to compute file statuses (project = %s, change = %d)",
changeNotes.getProjectName(), changeNotes.getChangeId().get());
- CodeOwnersPluginConfigSnapshot codeOwnersConfig =
+ CodeOwnersPluginProjectConfigSnapshot codeOwnersConfig =
codeOwnersPluginConfiguration.getProjectConfig(changeNotes.getProjectName());
Account.Id patchSetUploader = changeNotes.getCurrentPatchSet().uploader();
@@ -373,7 +373,7 @@
changeNotes.getChangeId().get(),
patchSet.id().get());
- CodeOwnersPluginConfigSnapshot codeOwnersConfig =
+ CodeOwnersPluginProjectConfigSnapshot codeOwnersConfig =
codeOwnersPluginConfiguration.getProjectConfig(changeNotes.getProjectName());
RequiredApproval requiredApproval = codeOwnersConfig.getRequiredApproval();
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
index a1f1c34..f78804f 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerResolver.java
@@ -557,7 +557,7 @@
requireNonNull(email, "email");
ImmutableSet<String> allowedEmailDomains =
- codeOwnersPluginConfiguration.getAllowedEmailDomains();
+ codeOwnersPluginConfiguration.getGlobalConfig().getAllowedEmailDomains();
if (allowedEmailDomains.isEmpty()) {
return OptionalResultWithMessages.create(true, "all domains are allowed");
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnAddReviewer.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnAddReviewer.java
index 403fe49..1aa9660 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnAddReviewer.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnAddReviewer.java
@@ -26,8 +26,8 @@
import com.google.gerrit.extensions.events.ReviewerAddedListener;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.metrics.Timer0;
-import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfigSnapshot;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
+import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginProjectConfigSnapshot;
import com.google.gerrit.plugins.codeowners.metrics.CodeOwnerMetrics;
import com.google.gerrit.plugins.codeowners.util.JgitPath;
import com.google.gerrit.server.ChangeMessagesUtil;
@@ -94,7 +94,7 @@
Change.Id changeId = Change.id(event.getChange()._number);
Project.NameKey projectName = Project.nameKey(event.getChange().project);
- CodeOwnersPluginConfigSnapshot codeOwnersConfig =
+ CodeOwnersPluginProjectConfigSnapshot codeOwnersConfig =
codeOwnersPluginConfiguration.getProjectConfig(projectName);
int maxPathsInChangeMessages = codeOwnersConfig.getMaxPathsInChangeMessages();
if (codeOwnersConfig.isDisabled(event.getChange().branch) || maxPathsInChangeMessages <= 0) {
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java
index 9803951..d6beb5f 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java
@@ -21,8 +21,8 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.metrics.Timer0;
-import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfigSnapshot;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
+import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginProjectConfigSnapshot;
import com.google.gerrit.plugins.codeowners.backend.config.RequiredApproval;
import com.google.gerrit.plugins.codeowners.metrics.CodeOwnerMetrics;
import com.google.gerrit.plugins.codeowners.util.JgitPath;
@@ -75,7 +75,7 @@
PatchSet patchSet,
Map<String, Short> oldApprovals,
Map<String, Short> approvals) {
- CodeOwnersPluginConfigSnapshot codeOwnersConfig =
+ CodeOwnersPluginProjectConfigSnapshot codeOwnersConfig =
codeOwnersPluginConfiguration.getProjectConfig(changeNotes.getProjectName());
int maxPathsInChangeMessage = codeOwnersConfig.getMaxPathsInChangeMessages();
if (codeOwnersConfig.isDisabled(changeNotes.getChange().getDest().branch())
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java
index 138623f..09e3624 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java
@@ -22,8 +22,8 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.metrics.Timer0;
-import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfigSnapshot;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
+import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginProjectConfigSnapshot;
import com.google.gerrit.plugins.codeowners.backend.config.RequiredApproval;
import com.google.gerrit.plugins.codeowners.metrics.CodeOwnerMetrics;
import com.google.gerrit.server.IdentifiedUser;
@@ -64,7 +64,7 @@
PatchSet patchSet,
Map<String, Short> oldApprovals,
Map<String, Short> approvals) {
- CodeOwnersPluginConfigSnapshot codeOwnersConfig =
+ CodeOwnersPluginProjectConfigSnapshot codeOwnersConfig =
codeOwnersPluginConfiguration.getProjectConfig(changeNotes.getProjectName());
if (codeOwnersConfig.isDisabled(changeNotes.getChange().getDest().branch())) {
return Optional.empty();
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/TransientCodeOwnerConfigCache.java b/java/com/google/gerrit/plugins/codeowners/backend/TransientCodeOwnerConfigCache.java
index 65ec308..e8a1ab7 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/TransientCodeOwnerConfigCache.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/TransientCodeOwnerConfigCache.java
@@ -55,7 +55,8 @@
CodeOwnerMetrics codeOwnerMetrics) {
this.repoManager = repoManager;
this.codeOwners = codeOwners;
- this.maxCacheSize = codeOwnersPluginConfiguration.getMaxCodeOwnerConfigCacheSize();
+ this.maxCacheSize =
+ codeOwnersPluginConfiguration.getGlobalConfig().getMaxCodeOwnerConfigCacheSize();
this.counters = new Counters(codeOwnerMetrics);
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfiguration.java b/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfiguration.java
index d070d81..a216eb1 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfiguration.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfiguration.java
@@ -16,17 +16,10 @@
import static java.util.Objects.requireNonNull;
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Project;
-import com.google.gerrit.extensions.annotations.PluginName;
-import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.server.cache.PerThreadCache;
-import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.inject.Inject;
import com.google.inject.Singleton;
-import java.util.Optional;
/**
* The configuration of the code-owners plugin.
@@ -42,112 +35,42 @@
*/
@Singleton
public class CodeOwnersPluginConfiguration {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ public static final String SECTION_CODE_OWNERS = "codeOwners";
- @VisibleForTesting public static final String SECTION_CODE_OWNERS = "codeOwners";
+ private static final String GLOBAL_CONFIG_IDENTIFIER = "GLOBAL_CONFIG";
- @VisibleForTesting
- static final String KEY_ENABLE_EXPERIMENTAL_REST_ENDPOINTS = "enableExperimentalRestEndpoints";
-
- @VisibleForTesting static final int DEFAULT_MAX_CODE_OWNER_CONFIG_CACHE_SIZE = 10000;
-
- private static final String KEY_MAX_CODE_OWNER_CONFIG_CACHE_SIZE = "maxCodeOwnerConfigCacheSize";
-
- private final CodeOwnersPluginConfigSnapshot.Factory codeOwnersPluginConfigSnapshotFactory;
- private final String pluginName;
- private final PluginConfigFactory pluginConfigFactory;
- private final GeneralConfig generalConfig;
+ private final CodeOwnersPluginGlobalConfigSnapshot.Factory
+ codeOwnersPluginGlobalConfigSnapshotFactory;
+ private final CodeOwnersPluginProjectConfigSnapshot.Factory
+ codeOwnersPluginProjectConfigSnapshotFactory;
@Inject
CodeOwnersPluginConfiguration(
- CodeOwnersPluginConfigSnapshot.Factory codeOwnersPluginConfigSnapshotFactory,
- @PluginName String pluginName,
- PluginConfigFactory pluginConfigFactory,
- GeneralConfig generalConfig) {
- this.codeOwnersPluginConfigSnapshotFactory = codeOwnersPluginConfigSnapshotFactory;
- this.pluginName = pluginName;
- this.pluginConfigFactory = pluginConfigFactory;
- this.generalConfig = generalConfig;
+ CodeOwnersPluginGlobalConfigSnapshot.Factory codeOwnersPluginGlobalConfigSnapshotFactory,
+ CodeOwnersPluginProjectConfigSnapshot.Factory codeOwnersPluginProjectConfigSnapshotFactory) {
+ this.codeOwnersPluginGlobalConfigSnapshotFactory = codeOwnersPluginGlobalConfigSnapshotFactory;
+ this.codeOwnersPluginProjectConfigSnapshotFactory =
+ codeOwnersPluginProjectConfigSnapshotFactory;
+ }
+
+ /** Returns the global code-owner plugin configuration. */
+ public CodeOwnersPluginGlobalConfigSnapshot getGlobalConfig() {
+ return PerThreadCache.getOrCompute(
+ PerThreadCache.Key.create(
+ CodeOwnersPluginGlobalConfigSnapshot.class, GLOBAL_CONFIG_IDENTIFIER),
+ () -> codeOwnersPluginGlobalConfigSnapshotFactory.create());
}
/**
- * Returns the code-owner plugin configuration for the given projects.
+ * Returns the code-owner plugin configuration for the given project.
*
* <p>Callers must ensure that the project of the specified branch exists. If the project doesn't
* exist the call fails with {@link IllegalStateException}.
*/
- public CodeOwnersPluginConfigSnapshot getProjectConfig(Project.NameKey projectName) {
+ public CodeOwnersPluginProjectConfigSnapshot getProjectConfig(Project.NameKey projectName) {
requireNonNull(projectName, "projectName");
return PerThreadCache.getOrCompute(
- PerThreadCache.Key.create(CodeOwnersPluginConfigSnapshot.class, projectName),
- () -> codeOwnersPluginConfigSnapshotFactory.create(projectName));
- }
-
- /**
- * Returns the email domains that are allowed to be used for code owners.
- *
- * @return the email domains that are allowed to be used for code owners, an empty set if all
- * email domains are allowed (if {@code plugin.code-owners.allowedEmailDomain} is not set or
- * set to an empty value)
- */
- public ImmutableSet<String> getAllowedEmailDomains() {
- return generalConfig.getAllowedEmailDomains();
- }
-
- /**
- * Checks whether experimental REST endpoints are enabled.
- *
- * @throws MethodNotAllowedException thrown if experimental REST endpoints are disabled
- */
- public void checkExperimentalRestEndpointsEnabled() throws MethodNotAllowedException {
- if (!areExperimentalRestEndpointsEnabled()) {
- throw new MethodNotAllowedException("experimental code owners REST endpoints are disabled");
- }
- }
-
- /** Whether experimental REST endpoints are enabled. */
- public boolean areExperimentalRestEndpointsEnabled() {
- try {
- return pluginConfigFactory
- .getFromGerritConfig(pluginName)
- .getBoolean(KEY_ENABLE_EXPERIMENTAL_REST_ENDPOINTS, /* defaultValue= */ false);
- } catch (IllegalArgumentException e) {
- logger.atWarning().withCause(e).log(
- "Value '%s' in gerrit.config (parameter plugin.%s.%s) is invalid.",
- pluginConfigFactory
- .getFromGerritConfig(pluginName)
- .getString(KEY_ENABLE_EXPERIMENTAL_REST_ENDPOINTS),
- pluginName,
- KEY_ENABLE_EXPERIMENTAL_REST_ENDPOINTS);
- return false;
- }
- }
-
- /**
- * Gets the maximum size for the {@link
- * com.google.gerrit.plugins.codeowners.backend.TransientCodeOwnerConfigCache}.
- *
- * @return the maximum cache size, {@link Optional#empty()} if the cache size is not limited
- */
- public Optional<Integer> getMaxCodeOwnerConfigCacheSize() {
- try {
- int maxCodeOwnerConfigCacheSize =
- pluginConfigFactory
- .getFromGerritConfig(pluginName)
- .getInt(
- KEY_MAX_CODE_OWNER_CONFIG_CACHE_SIZE, DEFAULT_MAX_CODE_OWNER_CONFIG_CACHE_SIZE);
- return maxCodeOwnerConfigCacheSize > 0
- ? Optional.of(maxCodeOwnerConfigCacheSize)
- : Optional.empty();
- } catch (IllegalArgumentException e) {
- logger.atWarning().withCause(e).log(
- "Value '%s' in gerrit.config (parameter plugin.%s.%s) is invalid.",
- pluginConfigFactory
- .getFromGerritConfig(pluginName)
- .getString(KEY_MAX_CODE_OWNER_CONFIG_CACHE_SIZE),
- pluginName,
- KEY_MAX_CODE_OWNER_CONFIG_CACHE_SIZE);
- return Optional.empty();
- }
+ PerThreadCache.Key.create(CodeOwnersPluginProjectConfigSnapshot.class, projectName),
+ () -> codeOwnersPluginProjectConfigSnapshotFactory.create(projectName));
}
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginGlobalConfigSnapshot.java b/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginGlobalConfigSnapshot.java
new file mode 100644
index 0000000..c466b8a
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginGlobalConfigSnapshot.java
@@ -0,0 +1,144 @@
+// Copyright (C) 2021 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.plugins.codeowners.backend.config;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
+import com.google.gerrit.server.config.PluginConfigFactory;
+import com.google.inject.Inject;
+import java.util.Optional;
+
+/** Snapshot of the global code-owners plugin configuration. */
+public class CodeOwnersPluginGlobalConfigSnapshot {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ @VisibleForTesting
+ static final String KEY_ENABLE_EXPERIMENTAL_REST_ENDPOINTS = "enableExperimentalRestEndpoints";
+
+ @VisibleForTesting static final int DEFAULT_MAX_CODE_OWNER_CONFIG_CACHE_SIZE = 10000;
+
+ private static final String KEY_MAX_CODE_OWNER_CONFIG_CACHE_SIZE = "maxCodeOwnerConfigCacheSize";
+
+ public interface Factory {
+ CodeOwnersPluginGlobalConfigSnapshot create();
+ }
+
+ private final String pluginName;
+ private final PluginConfigFactory pluginConfigFactory;
+ private final GeneralConfig generalConfig;
+
+ @Nullable private ImmutableSet<String> allowedEmailDomains;
+ @Nullable private Boolean enabledExperimentalRestEndpoints;
+ @Nullable private Optional<Integer> maxCodeOwnerConfigCacheSize;
+
+ @Inject
+ CodeOwnersPluginGlobalConfigSnapshot(
+ @PluginName String pluginName,
+ PluginConfigFactory pluginConfigFactory,
+ GeneralConfig generalConfig) {
+ this.pluginName = pluginName;
+ this.pluginConfigFactory = pluginConfigFactory;
+ this.generalConfig = generalConfig;
+ }
+
+ /**
+ * Returns the email domains that are allowed to be used for code owners.
+ *
+ * @return the email domains that are allowed to be used for code owners, an empty set if all
+ * email domains are allowed (if {@code plugin.code-owners.allowedEmailDomain} is not set or
+ * set to an empty value)
+ */
+ public ImmutableSet<String> getAllowedEmailDomains() {
+ if (allowedEmailDomains == null) {
+ allowedEmailDomains = generalConfig.getAllowedEmailDomains();
+ }
+ return allowedEmailDomains;
+ }
+
+ /**
+ * Checks whether experimental REST endpoints are enabled.
+ *
+ * @throws MethodNotAllowedException thrown if experimental REST endpoints are disabled
+ */
+ public void checkExperimentalRestEndpointsEnabled() throws MethodNotAllowedException {
+ if (!areExperimentalRestEndpointsEnabled()) {
+ throw new MethodNotAllowedException("experimental code owners REST endpoints are disabled");
+ }
+ }
+
+ /** Whether experimental REST endpoints are enabled. */
+ public boolean areExperimentalRestEndpointsEnabled() {
+ if (enabledExperimentalRestEndpoints == null) {
+ enabledExperimentalRestEndpoints = readEnabledExperimentalRestEndpoints();
+ }
+ return enabledExperimentalRestEndpoints;
+ }
+
+ private boolean readEnabledExperimentalRestEndpoints() {
+ try {
+ return pluginConfigFactory
+ .getFromGerritConfig(pluginName)
+ .getBoolean(KEY_ENABLE_EXPERIMENTAL_REST_ENDPOINTS, /* defaultValue= */ false);
+ } catch (IllegalArgumentException e) {
+ logger.atWarning().withCause(e).log(
+ "Value '%s' in gerrit.config (parameter plugin.%s.%s) is invalid.",
+ pluginConfigFactory
+ .getFromGerritConfig(pluginName)
+ .getString(KEY_ENABLE_EXPERIMENTAL_REST_ENDPOINTS),
+ pluginName,
+ KEY_ENABLE_EXPERIMENTAL_REST_ENDPOINTS);
+ return false;
+ }
+ }
+
+ /**
+ * Gets the maximum size for the {@link
+ * com.google.gerrit.plugins.codeowners.backend.TransientCodeOwnerConfigCache}.
+ *
+ * @return the maximum cache size, {@link Optional#empty()} if the cache size is not limited
+ */
+ public Optional<Integer> getMaxCodeOwnerConfigCacheSize() {
+ if (maxCodeOwnerConfigCacheSize == null) {
+ maxCodeOwnerConfigCacheSize = readMaxCodeOwnerConfigCacheSize();
+ }
+ return maxCodeOwnerConfigCacheSize;
+ }
+
+ private Optional<Integer> readMaxCodeOwnerConfigCacheSize() {
+ try {
+ int maxCodeOwnerConfigCacheSize =
+ pluginConfigFactory
+ .getFromGerritConfig(pluginName)
+ .getInt(
+ KEY_MAX_CODE_OWNER_CONFIG_CACHE_SIZE, DEFAULT_MAX_CODE_OWNER_CONFIG_CACHE_SIZE);
+ return maxCodeOwnerConfigCacheSize > 0
+ ? Optional.of(maxCodeOwnerConfigCacheSize)
+ : Optional.empty();
+ } catch (IllegalArgumentException e) {
+ logger.atWarning().withCause(e).log(
+ "Value '%s' in gerrit.config (parameter plugin.%s.%s) is invalid.",
+ pluginConfigFactory
+ .getFromGerritConfig(pluginName)
+ .getString(KEY_MAX_CODE_OWNER_CONFIG_CACHE_SIZE),
+ pluginName,
+ KEY_MAX_CODE_OWNER_CONFIG_CACHE_SIZE);
+ return Optional.empty();
+ }
+ }
+}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigSnapshot.java b/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginProjectConfigSnapshot.java
similarity index 75%
rename from java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigSnapshot.java
rename to java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginProjectConfigSnapshot.java
index df0d418..0d0b44d 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigSnapshot.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginProjectConfigSnapshot.java
@@ -48,11 +48,11 @@
import org.eclipse.jgit.lib.Config;
/** Snapshot of the code-owners plugin configuration for one project. */
-public class CodeOwnersPluginConfigSnapshot {
+public class CodeOwnersPluginProjectConfigSnapshot {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
- CodeOwnersPluginConfigSnapshot create(Project.NameKey projectName);
+ CodeOwnersPluginProjectConfigSnapshot create(Project.NameKey projectName);
}
private final ProjectCache projectCache;
@@ -65,10 +65,33 @@
private final Project.NameKey projectName;
private final Config pluginConfig;
+ @Nullable private Optional<String> fileExtension;
+ @Nullable private Boolean codeOwnerConfigsReadOnly;
+ @Nullable private Boolean exemptPureReverts;
+ @Nullable private Boolean rejectNonResolvableCodeOwners;
+ @Nullable private Boolean rejectNonResolvableImports;
+
+ @Nullable
+ private CodeOwnerConfigValidationPolicy codeOwnerConfigValidationPolicyForCommitReceived;
+
+ @Nullable private CodeOwnerConfigValidationPolicy codeOwnerConfigValidationPolicyForSubmit;
+ @Nullable private MergeCommitStrategy mergeCommitStrategy;
+ @Nullable private FallbackCodeOwners fallbackCodeOwners;
+ @Nullable private Integer maxPathsInChangeMessages;
+ @Nullable private ImmutableSet<CodeOwnerReference> globalCodeOwners;
@Nullable private ImmutableSet<Account.Id> exemptedAccounts;
+ @Nullable private Optional<String> overrideInfoUrl;
+ @Nullable private Optional<String> invalidCodeOwnerConfigInfoUrl;
+ private Map<String, Boolean> disabledByBranch = new HashMap<>();
+ @Nullable private Boolean isDisabled;
+ private Map<String, CodeOwnerBackend> backendByBranch = new HashMap<>();
+ @Nullable private CodeOwnerBackend backend;
+ @Nullable private Boolean implicitApprovalsEnabled;
+ @Nullable private RequiredApproval requiredApproval;
+ @Nullable private ImmutableSortedSet<RequiredApproval> overrideApprovals;
@Inject
- CodeOwnersPluginConfigSnapshot(
+ CodeOwnersPluginProjectConfigSnapshot(
CodeOwnersPluginConfig.Factory codeOwnersPluginConfigFactory,
ProjectCache projectCache,
Emails emails,
@@ -91,19 +114,28 @@
/** Gets the file extension of code owner config files, if any configured. */
public Optional<String> getFileExtension() {
- return generalConfig.getFileExtension(pluginConfig);
+ if (fileExtension == null) {
+ fileExtension = generalConfig.getFileExtension(pluginConfig);
+ }
+ return fileExtension;
}
/** Checks whether code owner configs are read-only. */
public boolean areCodeOwnerConfigsReadOnly() {
- return generalConfig.getReadOnly(projectName, pluginConfig);
+ if (codeOwnerConfigsReadOnly == null) {
+ codeOwnerConfigsReadOnly = generalConfig.getReadOnly(projectName, pluginConfig);
+ }
+ return codeOwnerConfigsReadOnly;
}
/**
* Checks whether pure revert changes are exempted from needing code owner approvals for submit.
*/
public boolean arePureRevertsExempted() {
- return generalConfig.getExemptPureReverts(projectName, pluginConfig);
+ if (exemptPureReverts == null) {
+ exemptPureReverts = generalConfig.getExemptPureReverts(projectName, pluginConfig);
+ }
+ return exemptPureReverts;
}
/**
@@ -114,6 +146,13 @@
* should be rejected
*/
public boolean rejectNonResolvableCodeOwners(String branchName) {
+ if (rejectNonResolvableCodeOwners == null) {
+ rejectNonResolvableCodeOwners = readRejectNonResolvableCodeOwners(branchName);
+ }
+ return rejectNonResolvableCodeOwners;
+ }
+
+ private boolean readRejectNonResolvableCodeOwners(String branchName) {
requireNonNull(branchName, "branchName");
Optional<Boolean> branchSpecificFlag =
@@ -134,6 +173,13 @@
* should be rejected
*/
public boolean rejectNonResolvableImports(String branchName) {
+ if (rejectNonResolvableImports == null) {
+ rejectNonResolvableImports = readRejectNonResolvableImports(branchName);
+ }
+ return rejectNonResolvableImports;
+ }
+
+ private boolean readRejectNonResolvableImports(String branchName) {
requireNonNull(branchName, "branchName");
Optional<Boolean> branchSpecificFlag =
@@ -154,6 +200,15 @@
*/
public CodeOwnerConfigValidationPolicy getCodeOwnerConfigValidationPolicyForCommitReceived(
String branchName) {
+ if (codeOwnerConfigValidationPolicyForCommitReceived == null) {
+ codeOwnerConfigValidationPolicyForCommitReceived =
+ readCodeOwnerConfigValidationPolicyForCommitReceived(branchName);
+ }
+ return codeOwnerConfigValidationPolicyForCommitReceived;
+ }
+
+ private CodeOwnerConfigValidationPolicy readCodeOwnerConfigValidationPolicyForCommitReceived(
+ String branchName) {
requireNonNull(branchName, "branchName");
Optional<CodeOwnerConfigValidationPolicy> branchSpecificPolicy =
@@ -175,6 +230,15 @@
*/
public CodeOwnerConfigValidationPolicy getCodeOwnerConfigValidationPolicyForSubmit(
String branchName) {
+ if (codeOwnerConfigValidationPolicyForSubmit == null) {
+ codeOwnerConfigValidationPolicyForSubmit =
+ readCodeOwnerConfigValidationPolicyForSubmit(branchName);
+ }
+ return codeOwnerConfigValidationPolicyForSubmit;
+ }
+
+ private CodeOwnerConfigValidationPolicy readCodeOwnerConfigValidationPolicyForSubmit(
+ String branchName) {
requireNonNull(branchName, "branchName");
Optional<CodeOwnerConfigValidationPolicy> branchSpecificPolicy =
@@ -189,22 +253,35 @@
/** Gets the merge commit strategy. */
public MergeCommitStrategy getMergeCommitStrategy() {
- return generalConfig.getMergeCommitStrategy(projectName, pluginConfig);
+ if (mergeCommitStrategy == null) {
+ mergeCommitStrategy = generalConfig.getMergeCommitStrategy(projectName, pluginConfig);
+ }
+ return mergeCommitStrategy;
}
/** Gets the fallback code owners. */
public FallbackCodeOwners getFallbackCodeOwners() {
- return generalConfig.getFallbackCodeOwners(projectName, pluginConfig);
+ if (fallbackCodeOwners == null) {
+ fallbackCodeOwners = generalConfig.getFallbackCodeOwners(projectName, pluginConfig);
+ }
+ return fallbackCodeOwners;
}
/** Gets the max paths in change messages. */
public int getMaxPathsInChangeMessages() {
- return generalConfig.getMaxPathsInChangeMessages(projectName, pluginConfig);
+ if (maxPathsInChangeMessages == null) {
+ maxPathsInChangeMessages =
+ generalConfig.getMaxPathsInChangeMessages(projectName, pluginConfig);
+ }
+ return maxPathsInChangeMessages;
}
/** Gets the global code owners. */
public ImmutableSet<CodeOwnerReference> getGlobalCodeOwners() {
- return generalConfig.getGlobalCodeOwners(pluginConfig);
+ if (globalCodeOwners == null) {
+ globalCodeOwners = generalConfig.getGlobalCodeOwners(pluginConfig);
+ }
+ return globalCodeOwners;
}
/** Gets the accounts that are exempted from requiring code owner approvals. */
@@ -241,12 +318,18 @@
/** Gets the override info URL that is configured. */
public Optional<String> getOverrideInfoUrl() {
- return generalConfig.getOverrideInfoUrl(pluginConfig);
+ if (overrideInfoUrl == null) {
+ overrideInfoUrl = generalConfig.getOverrideInfoUrl(pluginConfig);
+ }
+ return overrideInfoUrl;
}
/** Gets the invalid code owner config info URL that is configured. */
public Optional<String> getInvalidCodeOwnerConfigInfoUrl() {
- return generalConfig.getInvalidCodeOwnerConfigInfoUrl(pluginConfig);
+ if (invalidCodeOwnerConfigInfoUrl == null) {
+ invalidCodeOwnerConfigInfoUrl = generalConfig.getInvalidCodeOwnerConfigInfoUrl(pluginConfig);
+ }
+ return invalidCodeOwnerConfigInfoUrl;
}
/**
@@ -273,14 +356,16 @@
public boolean isDisabled(String branchName) {
requireNonNull(branchName, "branchName");
- boolean isDisabled =
- statusConfig.isDisabledForBranch(
- pluginConfig, BranchNameKey.create(projectName, branchName));
- if (isDisabled) {
- return true;
- }
-
- return isDisabled();
+ BranchNameKey branchNameKey = BranchNameKey.create(projectName, branchName);
+ return disabledByBranch.computeIfAbsent(
+ branchNameKey.branch(),
+ b -> {
+ boolean isDisabled = statusConfig.isDisabledForBranch(pluginConfig, branchNameKey);
+ if (isDisabled) {
+ return true;
+ }
+ return isDisabled();
+ });
}
/**
@@ -298,7 +383,10 @@
* @return {@code true} if the code owners functionality is disabled, otherwise {@code false}
*/
public boolean isDisabled() {
- return statusConfig.isDisabledForProject(pluginConfig, projectName);
+ if (isDisabled == null) {
+ isDisabled = statusConfig.isDisabledForProject(pluginConfig, projectName);
+ }
+ return isDisabled;
}
/**
@@ -319,15 +407,20 @@
* @return the {@link CodeOwnerBackend} that should be used for the branch
*/
public CodeOwnerBackend getBackend(String branchName) {
- // check if a branch specific backend is configured
- Optional<CodeOwnerBackend> codeOwnerBackend =
- backendConfig.getBackendForBranch(
- pluginConfig, BranchNameKey.create(projectName, branchName));
- if (codeOwnerBackend.isPresent()) {
- return codeOwnerBackend.get();
- }
+ requireNonNull(branchName, "branchName");
- return getBackend();
+ BranchNameKey branchNameKey = BranchNameKey.create(projectName, branchName);
+ return backendByBranch.computeIfAbsent(
+ branchNameKey.branch(),
+ b -> {
+ Optional<CodeOwnerBackend> codeOwnerBackend =
+ backendConfig.getBackendForBranch(
+ pluginConfig, BranchNameKey.create(projectName, branchName));
+ if (codeOwnerBackend.isPresent()) {
+ return codeOwnerBackend.get();
+ }
+ return getBackend();
+ });
}
/**
@@ -345,6 +438,13 @@
* @return the {@link CodeOwnerBackend} that should be used
*/
public CodeOwnerBackend getBackend() {
+ if (backend == null) {
+ backend = readBackend();
+ }
+ return backend;
+ }
+
+ private CodeOwnerBackend readBackend() {
// check if a project specific backend is configured
Optional<CodeOwnerBackend> codeOwnerBackend =
backendConfig.getBackendForProject(pluginConfig, projectName);
@@ -358,6 +458,13 @@
/** Checks whether an implicit code owner approval from the last uploader is assumed. */
public boolean areImplicitApprovalsEnabled() {
+ if (implicitApprovalsEnabled == null) {
+ implicitApprovalsEnabled = readImplicitApprovalsEnabled();
+ }
+ return implicitApprovalsEnabled;
+ }
+
+ private boolean readImplicitApprovalsEnabled() {
EnableImplicitApprovals enableImplicitApprovals =
generalConfig.getEnableImplicitApprovals(projectName, pluginConfig);
switch (enableImplicitApprovals) {
@@ -406,6 +513,13 @@
* @return the required code owner approval that should be used
*/
public RequiredApproval getRequiredApproval() {
+ if (requiredApproval == null) {
+ requiredApproval = readRequiredApproval();
+ }
+ return requiredApproval;
+ }
+
+ private RequiredApproval readRequiredApproval() {
ImmutableList<RequiredApproval> configuredRequiredApprovalConfig =
getConfiguredRequiredApproval(requiredApprovalConfig);
if (!configuredRequiredApprovalConfig.isEmpty()) {
@@ -442,6 +556,13 @@
* configured, in this case the override functionality is disabled
*/
public ImmutableSortedSet<RequiredApproval> getOverrideApprovals() {
+ if (overrideApprovals == null) {
+ overrideApprovals = readOverrideApprovals();
+ }
+ return overrideApprovals;
+ }
+
+ private ImmutableSortedSet<RequiredApproval> readOverrideApprovals() {
try {
return ImmutableSortedSet.copyOf(
filterOutDuplicateRequiredApprovals(
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
index 49053ec..7caf0a1 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
@@ -33,8 +33,8 @@
import com.google.gerrit.plugins.codeowners.api.GeneralInfo;
import com.google.gerrit.plugins.codeowners.api.RequiredApprovalInfo;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackendId;
-import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfigSnapshot;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
+import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginProjectConfigSnapshot;
import com.google.gerrit.plugins.codeowners.backend.config.RequiredApproval;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.BranchResource;
@@ -79,7 +79,7 @@
}
CodeOwnerBranchConfigInfo format(BranchResource branchResource) {
- CodeOwnersPluginConfigSnapshot codeOwnersConfig =
+ CodeOwnersPluginProjectConfigSnapshot codeOwnersConfig =
codeOwnersPluginConfiguration.getProjectConfig(branchResource.getNameKey());
CodeOwnerBranchConfigInfo info = new CodeOwnerBranchConfigInfo();
@@ -102,7 +102,7 @@
}
private GeneralInfo formatGeneralInfo(Project.NameKey projectName) {
- CodeOwnersPluginConfigSnapshot codeOwnersConfig =
+ CodeOwnersPluginProjectConfigSnapshot codeOwnersConfig =
codeOwnersPluginConfiguration.getProjectConfig(projectName);
GeneralInfo generalInfo = new GeneralInfo();
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerConfigForPathInBranch.java b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerConfigForPathInBranch.java
index 1347471..ac049d2 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerConfigForPathInBranch.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerConfigForPathInBranch.java
@@ -54,7 +54,7 @@
@Override
public Response<CodeOwnerConfigInfo> apply(PathResource rsrc)
throws MethodNotAllowedException, IOException {
- codeOwnersPluginConfiguration.checkExperimentalRestEndpointsEnabled();
+ codeOwnersPluginConfiguration.getGlobalConfig().checkExperimentalRestEndpointsEnabled();
Optional<CodeOwnerConfig> codeOwnerConfig =
codeOwners.get(rsrc.getCodeOwnerConfigKey(), rsrc.getRevision());
diff --git a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
index 8bb6fd7..7ec5b1f 100644
--- a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
+++ b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
@@ -39,8 +39,8 @@
import com.google.gerrit.plugins.codeowners.backend.CodeOwnersInternalServerErrorException;
import com.google.gerrit.plugins.codeowners.backend.InvalidCodeOwnerConfigException;
import com.google.gerrit.plugins.codeowners.backend.PathCodeOwners;
-import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfigSnapshot;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
+import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginProjectConfigSnapshot;
import com.google.gerrit.plugins.codeowners.backend.config.InvalidPluginConfigurationException;
import com.google.gerrit.plugins.codeowners.common.ChangedFile;
import com.google.gerrit.plugins.codeowners.common.CodeOwnerConfigValidationPolicy;
@@ -315,7 +315,7 @@
RevCommit revCommit,
IdentifiedUser user,
boolean force) {
- CodeOwnersPluginConfigSnapshot codeOwnersConfig =
+ CodeOwnersPluginProjectConfigSnapshot codeOwnersConfig =
codeOwnersPluginConfiguration.getProjectConfig(branchNameKey.project());
logger.atFine().log("force = %s", force);
if (!force && codeOwnersConfig.isDisabled(branchNameKey.branch())) {
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigurationTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigurationTest.java
index d9c3e14..28b15a6 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigurationTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigurationTest.java
@@ -16,11 +16,8 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
-import static com.google.gerrit.truth.OptionalSubject.assertThat;
-import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.entities.Project;
-import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
import org.junit.Before;
import org.junit.Test;
@@ -60,60 +57,4 @@
.isEqualTo(
"cannot get code-owners plugin config for non-existing project non-existing-project");
}
-
- @Test
- @GerritConfig(name = "plugin.code-owners.enableExperimentalRestEndpoints", value = "false")
- public void checkExperimentalRestEndpointsEnabledThrowsExceptionIfDisabled() throws Exception {
- MethodNotAllowedException exception =
- assertThrows(
- MethodNotAllowedException.class,
- () -> codeOwnersPluginConfiguration.checkExperimentalRestEndpointsEnabled());
- assertThat(exception)
- .hasMessageThat()
- .isEqualTo("experimental code owners REST endpoints are disabled");
- }
-
- @Test
- public void experimentalRestEndpointsNotEnabled() throws Exception {
- assertThat(codeOwnersPluginConfiguration.areExperimentalRestEndpointsEnabled()).isFalse();
- }
-
- @Test
- @GerritConfig(name = "plugin.code-owners.enableExperimentalRestEndpoints", value = "true")
- public void experimentalRestEndpointsEnabled() throws Exception {
- assertThat(codeOwnersPluginConfiguration.areExperimentalRestEndpointsEnabled()).isTrue();
- }
-
- @Test
- @GerritConfig(name = "plugin.code-owners.enableExperimentalRestEndpoints", value = "invalid")
- public void experimentalRestEndpointsNotEnabled_invalidConfig() throws Exception {
- assertThat(codeOwnersPluginConfiguration.areExperimentalRestEndpointsEnabled()).isFalse();
- }
-
- @Test
- public void codeOwnerConfigCacheSizeIsLimitedByDefault() throws Exception {
- assertThat(codeOwnersPluginConfiguration.getMaxCodeOwnerConfigCacheSize())
- .value()
- .isEqualTo(CodeOwnersPluginConfiguration.DEFAULT_MAX_CODE_OWNER_CONFIG_CACHE_SIZE);
- }
-
- @Test
- @GerritConfig(name = "plugin.code-owners.maxCodeOwnerConfigCacheSize", value = "0")
- public void codeOwnerConfigCacheSizeIsUnlimited() throws Exception {
- assertThat(codeOwnersPluginConfiguration.getMaxCodeOwnerConfigCacheSize()).isEmpty();
- }
-
- @Test
- @GerritConfig(name = "plugin.code-owners.maxCodeOwnerConfigCacheSize", value = "10")
- public void codeOwnerConfigCacheSizeIsLimited() throws Exception {
- assertThat(codeOwnersPluginConfiguration.getMaxCodeOwnerConfigCacheSize())
- .value()
- .isEqualTo(10);
- }
-
- @Test
- @GerritConfig(name = "plugin.code-owners.maxCodeOwnerConfigCacheSize", value = "invalid")
- public void maxCodeOwnerConfigCacheSize_invalidConfig() throws Exception {
- assertThat(codeOwnersPluginConfiguration.getMaxCodeOwnerConfigCacheSize()).isEmpty();
- }
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginGlobalConfigSnapshotTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginGlobalConfigSnapshotTest.java
new file mode 100644
index 0000000..e382566
--- /dev/null
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginGlobalConfigSnapshotTest.java
@@ -0,0 +1,94 @@
+// Copyright (C) 2021 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.plugins.codeowners.backend.config;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static com.google.gerrit.truth.OptionalSubject.assertThat;
+
+import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
+import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
+import org.junit.Before;
+import org.junit.Test;
+
+/** Tests for {@link CodeOwnersPluginGlobalConfigSnapshot}. */
+public class CodeOwnersPluginGlobalConfigSnapshotTest extends AbstractCodeOwnersTest {
+ private CodeOwnersPluginGlobalConfigSnapshot.Factory codeOwnersPluginGlobalConfigSnapshotFactory;
+
+ @Before
+ public void setUpCodeOwnersPlugin() throws Exception {
+ codeOwnersPluginGlobalConfigSnapshotFactory =
+ plugin.getSysInjector().getInstance(CodeOwnersPluginGlobalConfigSnapshot.Factory.class);
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.enableExperimentalRestEndpoints", value = "false")
+ public void checkExperimentalRestEndpointsEnabledThrowsExceptionIfDisabled() throws Exception {
+ MethodNotAllowedException exception =
+ assertThrows(
+ MethodNotAllowedException.class,
+ () -> cfgSnapshot().checkExperimentalRestEndpointsEnabled());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo("experimental code owners REST endpoints are disabled");
+ }
+
+ @Test
+ public void experimentalRestEndpointsNotEnabled() throws Exception {
+ assertThat(cfgSnapshot().areExperimentalRestEndpointsEnabled()).isFalse();
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.enableExperimentalRestEndpoints", value = "true")
+ public void experimentalRestEndpointsEnabled() throws Exception {
+ assertThat(cfgSnapshot().areExperimentalRestEndpointsEnabled()).isTrue();
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.enableExperimentalRestEndpoints", value = "invalid")
+ public void experimentalRestEndpointsNotEnabled_invalidConfig() throws Exception {
+ assertThat(cfgSnapshot().areExperimentalRestEndpointsEnabled()).isFalse();
+ }
+
+ @Test
+ public void codeOwnerConfigCacheSizeIsLimitedByDefault() throws Exception {
+ assertThat(cfgSnapshot().getMaxCodeOwnerConfigCacheSize())
+ .value()
+ .isEqualTo(CodeOwnersPluginGlobalConfigSnapshot.DEFAULT_MAX_CODE_OWNER_CONFIG_CACHE_SIZE);
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.maxCodeOwnerConfigCacheSize", value = "0")
+ public void codeOwnerConfigCacheSizeIsUnlimited() throws Exception {
+ assertThat(cfgSnapshot().getMaxCodeOwnerConfigCacheSize()).isEmpty();
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.maxCodeOwnerConfigCacheSize", value = "10")
+ public void codeOwnerConfigCacheSizeIsLimited() throws Exception {
+ assertThat(cfgSnapshot().getMaxCodeOwnerConfigCacheSize()).value().isEqualTo(10);
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.maxCodeOwnerConfigCacheSize", value = "invalid")
+ public void maxCodeOwnerConfigCacheSize_invalidConfig() throws Exception {
+ assertThat(cfgSnapshot().getMaxCodeOwnerConfigCacheSize()).isEmpty();
+ }
+
+ private CodeOwnersPluginGlobalConfigSnapshot cfgSnapshot() {
+ return codeOwnersPluginGlobalConfigSnapshotFactory.create();
+ }
+}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigSnapshotTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginProjectConfigSnapshotTest.java
similarity index 98%
rename from javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigSnapshotTest.java
rename to javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginProjectConfigSnapshotTest.java
index 4b65a20..f155572 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigSnapshotTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginProjectConfigSnapshotTest.java
@@ -56,17 +56,18 @@
import org.junit.Before;
import org.junit.Test;
-/** Tests for {@link CodeOwnersPluginConfigSnapshot}. */
-public class CodeOwnersPluginConfigSnapshotTest extends AbstractCodeOwnersTest {
+/** Tests for {@link CodeOwnersPluginProjectConfigSnapshot}. */
+public class CodeOwnersPluginProjectConfigSnapshotTest extends AbstractCodeOwnersTest {
@Inject private ProjectOperations projectOperations;
- private CodeOwnersPluginConfigSnapshot.Factory codeOwnersPluginConfigSnapshotFactory;
+ private CodeOwnersPluginProjectConfigSnapshot.Factory
+ codeOwnersPluginProjectConfigSnapshotFactory;
private DynamicMap<CodeOwnerBackend> codeOwnerBackends;
@Before
public void setUpCodeOwnersPlugin() throws Exception {
- codeOwnersPluginConfigSnapshotFactory =
- plugin.getSysInjector().getInstance(CodeOwnersPluginConfigSnapshot.Factory.class);
+ codeOwnersPluginProjectConfigSnapshotFactory =
+ plugin.getSysInjector().getInstance(CodeOwnersPluginProjectConfigSnapshot.Factory.class);
codeOwnerBackends =
plugin.getSysInjector().getInstance(new Key<DynamicMap<CodeOwnerBackend>>() {});
}
@@ -864,6 +865,14 @@
}
@Test
+ public void cannotGetBackendForNullBranch() throws Exception {
+ NullPointerException npe =
+ assertThrows(
+ NullPointerException.class, () -> cfgSnapshot().getBackend(/* branchName= */ null));
+ assertThat(npe).hasMessageThat().isEqualTo("branchName");
+ }
+
+ @Test
public void getBackendForNonExistingBranch() throws Exception {
assertThat(cfgSnapshot().getBackend("non-existing")).isInstanceOf(FindOwnersBackend.class);
}
@@ -1775,8 +1784,8 @@
assertThat(cfgSnapshot().rejectNonResolvableImports("master")).isFalse();
}
- private CodeOwnersPluginConfigSnapshot cfgSnapshot() {
- return codeOwnersPluginConfigSnapshotFactory.create(project);
+ private CodeOwnersPluginProjectConfigSnapshot cfgSnapshot() {
+ return codeOwnersPluginProjectConfigSnapshotFactory.create(project);
}
private void configureFileExtension(Project.NameKey project, String fileExtension)
diff --git a/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java b/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
index 3e9b614..9972afe 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
@@ -34,8 +34,8 @@
import com.google.gerrit.plugins.codeowners.api.RequiredApprovalInfo;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackendId;
import com.google.gerrit.plugins.codeowners.backend.FallbackCodeOwners;
-import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfigSnapshot;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
+import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginProjectConfigSnapshot;
import com.google.gerrit.plugins.codeowners.backend.config.RequiredApproval;
import com.google.gerrit.plugins.codeowners.backend.findowners.FindOwnersBackend;
import com.google.gerrit.plugins.codeowners.backend.proto.ProtoBackend;
@@ -64,7 +64,7 @@
@Rule public final MockitoRule mockito = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS);
@Mock private CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
- @Mock private CodeOwnersPluginConfigSnapshot codeOwnersPluginConfigSnapshot;
+ @Mock private CodeOwnersPluginProjectConfigSnapshot codeOwnersPluginConfigSnapshot;
@Inject private CurrentUser currentUser;
