Avoid Path.toFile() method in PluginConfigFactory
The Path.toFile() methods works only for Path on default filesystem
and fails with other filesystems (like JimFS). Internally in google
we are going to use JimFS for storing sites data and provide config
from a non-file source.
Having Path.toFile() call in the PluginConfigFactory leads to exception
even if config files are not used at all.
This change moves config loading from a file into a separate dependecy,
so it can be replaced with a different source.
Change-Id: Id1dd46c397758524217fee500eaa510068b2fe47
diff --git a/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java b/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java
index de9a43d..e4b0eea 100644
--- a/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java
+++ b/java/com/google/gerrit/acceptance/InMemoryTestingDatabaseModule.java
@@ -21,7 +21,9 @@
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.server.config.FileBasedGlobalPluginConfigProvider;
import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.config.GlobalPluginConfigProvider;
import com.google.gerrit.server.config.SitePath;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.config.TrackingFooters;
@@ -54,6 +56,7 @@
@Override
protected void configure() {
bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(cfg);
+ bind(GlobalPluginConfigProvider.class).to(FileBasedGlobalPluginConfigProvider.class);
bind(Path.class).annotatedWith(SitePath.class).toInstance(sitePath);
if (repoManager != null) {
diff --git a/java/com/google/gerrit/server/config/FileBasedGlobalPluginConfigProvider.java b/java/com/google/gerrit/server/config/FileBasedGlobalPluginConfigProvider.java
new file mode 100644
index 0000000..098d2c2
--- /dev/null
+++ b/java/com/google/gerrit/server/config/FileBasedGlobalPluginConfigProvider.java
@@ -0,0 +1,57 @@
+// 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.server.config;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.nio.file.Path;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.util.FS;
+
+@Singleton
+public class FileBasedGlobalPluginConfigProvider implements GlobalPluginConfigProvider {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private final SitePaths site;
+
+ @Inject
+ FileBasedGlobalPluginConfigProvider(SitePaths site) {
+ this.site = site;
+ }
+
+ @Override
+ public Config get(String pluginName) {
+ Path pluginConfigFile = site.etc_dir.resolve(pluginName + ".config");
+ FileBasedConfig cfg = new FileBasedConfig(pluginConfigFile.toFile(), FS.DETECTED);
+ if (!cfg.getFile().exists()) {
+ logger.atInfo().log("No %s; assuming defaults", pluginConfigFile.toAbsolutePath());
+ return cfg;
+ }
+
+ try {
+ cfg.load();
+ } catch (ConfigInvalidException e) {
+ // This is an error in user input, don't spam logs with a stack trace.
+ logger.atWarning().log(
+ "Failed to load %s: %s", pluginConfigFile.toAbsolutePath(), e.getMessage());
+ } catch (IOException e) {
+ logger.atWarning().withCause(e).log("Failed to load %s", pluginConfigFile.toAbsolutePath());
+ }
+ return cfg;
+ }
+}
diff --git a/java/com/google/gerrit/server/config/GerritServerConfigModule.java b/java/com/google/gerrit/server/config/GerritServerConfigModule.java
index 27d1d58..da85834 100644
--- a/java/com/google/gerrit/server/config/GerritServerConfigModule.java
+++ b/java/com/google/gerrit/server/config/GerritServerConfigModule.java
@@ -66,6 +66,7 @@
bind(Config.class)
.annotatedWith(GerritServerConfig.class)
.toProvider(GerritServerConfigProvider.class);
+ bind(GlobalPluginConfigProvider.class).to(FileBasedGlobalPluginConfigProvider.class);
bind(SecureStore.class).toProvider(SecureStoreProvider.class).in(SINGLETON);
bind(Boolean.class)
.annotatedWith(GerritIsReplica.class)
diff --git a/java/com/google/gerrit/server/config/GlobalPluginConfigProvider.java b/java/com/google/gerrit/server/config/GlobalPluginConfigProvider.java
new file mode 100644
index 0000000..847708a
--- /dev/null
+++ b/java/com/google/gerrit/server/config/GlobalPluginConfigProvider.java
@@ -0,0 +1,21 @@
+// 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.server.config;
+
+import org.eclipse.jgit.lib.Config;
+
+public interface GlobalPluginConfigProvider {
+ Config get(String pluginName);
+}
diff --git a/java/com/google/gerrit/server/config/PluginConfigFactory.java b/java/com/google/gerrit/server/config/PluginConfigFactory.java
index c49e928..bd4b661 100644
--- a/java/com/google/gerrit/server/config/PluginConfigFactory.java
+++ b/java/com/google/gerrit/server/config/PluginConfigFactory.java
@@ -16,7 +16,6 @@
import static com.google.gerrit.server.project.ProjectCache.noSuchProject;
-import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Project;
import com.google.gerrit.server.plugins.Plugin;
import com.google.gerrit.server.plugins.ReloadPluginListener;
@@ -28,22 +27,15 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
-import java.io.IOException;
-import java.nio.file.Path;
import java.util.HashMap;
import java.util.Map;
-import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.storage.file.FileBasedConfig;
-import org.eclipse.jgit.util.FS;
@Singleton
public class PluginConfigFactory implements ReloadPluginListener {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
private static final String EXTENSION = ".config";
- private final SitePaths site;
+ private final GlobalPluginConfigProvider globalPluginConfigProvider;
private final Provider<Config> cfgProvider;
private final ProjectCache projectCache;
private final ProjectState.Factory projectStateFactory;
@@ -54,12 +46,12 @@
@Inject
PluginConfigFactory(
- SitePaths site,
@GerritServerConfig Provider<Config> cfgProvider,
+ GlobalPluginConfigProvider globalPluginConfigProvider,
ProjectCache projectCache,
ProjectState.Factory projectStateFactory,
SecureStore secureStore) {
- this.site = site;
+ this.globalPluginConfigProvider = globalPluginConfigProvider;
this.cfgProvider = cfgProvider;
this.projectCache = projectCache;
this.projectStateFactory = projectStateFactory;
@@ -211,25 +203,9 @@
return pluginConfigs.get(pluginName);
}
- Path pluginConfigFile = site.etc_dir.resolve(pluginName + ".config");
- FileBasedConfig cfg = new FileBasedConfig(pluginConfigFile.toFile(), FS.DETECTED);
+ Config cfg = globalPluginConfigProvider.get(pluginName);
GlobalPluginConfig pluginConfig = new GlobalPluginConfig(pluginName, cfg, secureStore);
pluginConfigs.put(pluginName, pluginConfig);
- if (!cfg.getFile().exists()) {
- logger.atInfo().log("No %s; assuming defaults", pluginConfigFile.toAbsolutePath());
- return pluginConfig;
- }
-
- try {
- cfg.load();
- } catch (ConfigInvalidException e) {
- // This is an error in user input, don't spam logs with a stack trace.
- logger.atWarning().log(
- "Failed to load %s: %s", pluginConfigFile.toAbsolutePath(), e.getMessage());
- } catch (IOException e) {
- logger.atWarning().withCause(e).log("Failed to load %s", pluginConfigFile.toAbsolutePath());
- }
-
return pluginConfig;
}
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index f28f60f..06d9453 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -54,6 +54,7 @@
import com.google.gerrit.server.config.CanonicalWebUrlModule;
import com.google.gerrit.server.config.CanonicalWebUrlProvider;
import com.google.gerrit.server.config.DefaultUrlFormatter;
+import com.google.gerrit.server.config.FileBasedGlobalPluginConfigProvider;
import com.google.gerrit.server.config.GerritGlobalModule;
import com.google.gerrit.server.config.GerritInstanceIdModule;
import com.google.gerrit.server.config.GerritInstanceNameModule;
@@ -62,6 +63,7 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GerritServerId;
import com.google.gerrit.server.config.GerritServerIdProvider;
+import com.google.gerrit.server.config.GlobalPluginConfigProvider;
import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.config.SitePath;
import com.google.gerrit.server.config.TrackingFooters;
@@ -195,6 +197,7 @@
bind(Path.class).annotatedWith(SitePath.class).toInstance(Paths.get("."));
bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(cfg);
bind(GerritOptions.class).toInstance(new GerritOptions(false, false));
+ bind(GlobalPluginConfigProvider.class).to(FileBasedGlobalPluginConfigProvider.class);
bind(GitRepositoryManager.class).to(InMemoryRepositoryManager.class);
bind(InMemoryRepositoryManager.class).in(SINGLETON);
