From e63cf2ba0c2f236df73c9f5129689f3b135d9449 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Thu, 23 May 2019 14:24:21 -0700 Subject: [PATCH] SystemReaderInstaller: Return a new instance from getSystemConfig This is the only way to respect the passed-in parent config, which may be different on each invocation. Additionally, returning a new instance matches the behavior of the default system reader, which downstream callers may be depending on. Change-Id: If95ccf4e97d4b969cdba7a6e85e613e9acc3ff0d --- .../server/git/SystemReaderInstaller.java | 4 +-- .../gerrit/server/git/JGitConfigTest.java | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/java/com/google/gerrit/server/git/SystemReaderInstaller.java b/java/com/google/gerrit/server/git/SystemReaderInstaller.java index 6597a54b41..104321074e 100644 --- a/java/com/google/gerrit/server/git/SystemReaderInstaller.java +++ b/java/com/google/gerrit/server/git/SystemReaderInstaller.java @@ -47,8 +47,6 @@ public class SystemReaderInstaller implements LifecycleListener { private SystemReader customReader() { SystemReader current = SystemReader.getInstance(); - FileBasedConfig jgitConfig = new FileBasedConfig(site.jgit_config.toFile(), FS.DETECTED); - return new SystemReader() { @Override public String getHostname() { @@ -72,7 +70,7 @@ public class SystemReaderInstaller implements LifecycleListener { @Override public FileBasedConfig openSystemConfig(Config parent, FS fs) { - return jgitConfig; + return new FileBasedConfig(parent, site.jgit_config.toFile(), FS.DETECTED); } @Override diff --git a/javatests/com/google/gerrit/server/git/JGitConfigTest.java b/javatests/com/google/gerrit/server/git/JGitConfigTest.java index 7cb5a98293..9f6b47e7ae 100644 --- a/javatests/com/google/gerrit/server/git/JGitConfigTest.java +++ b/javatests/com/google/gerrit/server/git/JGitConfigTest.java @@ -22,7 +22,11 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import org.eclipse.jgit.internal.storage.file.FileRepository; +import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.storage.file.FileBasedConfig; +import org.eclipse.jgit.util.FS; +import org.eclipse.jgit.util.SystemReader; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -52,4 +56,29 @@ public class JGitConfigTest { assertThat(repo.getConfig().getString("core", null, "trustFolderStat")).isEqualTo("false"); } } + + @Test + public void openSystemConfigRespectsParent() throws Exception { + Config parent = new Config(); + parent.setString("foo", null, "bar", "value"); + FileBasedConfig system = SystemReader.getInstance().openSystemConfig(parent, FS.DETECTED); + system.load(); + assertThat(system.getString("core", null, "trustFolderStat")).isEqualTo("false"); + assertThat(system.getString("foo", null, "bar")).isEqualTo("value"); + } + + @Test + public void openSystemConfigReturnsDifferentInstances() throws Exception { + FileBasedConfig system1 = SystemReader.getInstance().openSystemConfig(null, FS.DETECTED); + system1.load(); + assertThat(system1.getString("core", null, "trustFolderStat")).isEqualTo("false"); + + FileBasedConfig system2 = SystemReader.getInstance().openSystemConfig(null, FS.DETECTED); + system2.load(); + assertThat(system2.getString("core", null, "trustFolderStat")).isEqualTo("false"); + + system1.setString("core", null, "trustFolderStat", "true"); + assertThat(system1.getString("core", null, "trustFolderStat")).isEqualTo("true"); + assertThat(system2.getString("core", null, "trustFolderStat")).isEqualTo("false"); + } }