Merge "Prevent ConcurrentModificationException for PushOne.refBatchesToPush"
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ConfigResource.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ConfigResource.java
index 7b41fe6..c0a5a7d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ConfigResource.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ConfigResource.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.replication;
+import java.io.IOException;
import org.eclipse.jgit.lib.Config;
/**
@@ -37,6 +38,16 @@
Config getConfig();
/**
+ * Update the configuration resource.
+ *
+ * <p>Allows to persist changes to the configuration resource.
+ *
+ * @param config updated configuration
+ * @throws IOException when configuration cannot be persisted
+ */
+ void update(Config config) throws IOException;
+
+ /**
* Current logical version string of the current configuration loaded in memory, depending on the
* actual implementation of the configuration on the persistent storage.
*
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
index 008e50b..98170ae 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
@@ -127,8 +127,8 @@
}
public static class QueueInfo {
- public final Map<URIish, PushOne> pending;
- public final Map<URIish, PushOne> inFlight;
+ public final ImmutableMap<URIish, PushOne> pending;
+ public final ImmutableMap<URIish, PushOne> inFlight;
public QueueInfo(Map<URIish, PushOne> pending, Map<URIish, PushOne> inFlight) {
this.pending = ImmutableMap.copyOf(pending);
@@ -640,7 +640,7 @@
}
// by default push all projects
- List<String> projects = config.getProjects();
+ ImmutableList<String> projects = config.getProjects();
if (projects.isEmpty()) {
return true;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java b/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java
index 08f3477..3854801 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java
@@ -52,7 +52,7 @@
private final Destination.Factory destinationFactory;
private final Provider<ReplicationQueue> replicationQueue;
- private volatile List<Destination> destinations;
+ private volatile ImmutableList<Destination> destinations;
private boolean shuttingDown;
public static class EventQueueNotEmptyException extends Exception {
@@ -258,7 +258,7 @@
}
}
- private List<Destination> allDestinations(
+ private ImmutableList<Destination> allDestinations(
Destination.Factory destinationFactory, List<RemoteConfiguration> remoteConfigurations) {
ImmutableList.Builder<Destination> dest = ImmutableList.builder();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResource.java b/src/main/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResource.java
index 4220ddb..0a2afa6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResource.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResource.java
@@ -23,9 +23,11 @@
import com.google.common.hash.Hashing;
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
+import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
+import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Stream;
@@ -61,6 +63,30 @@
}
}
+ @Override
+ public void update(Config updates) throws IOException {
+ Set<String> remotes = updates.getSubsections("remote");
+ for (String remote : remotes) {
+ File remoteFile = remoteConfigsDirPath.resolve(remote + ".config").toFile();
+ FileBasedConfig remoteConfig = new FileBasedConfig(remoteFile, FS.DETECTED);
+ try {
+ remoteConfig.load();
+ } catch (ConfigInvalidException e) {
+ throw new IOException(
+ String.format("Cannot parse configuration file: %s", remoteFile.getAbsoluteFile()), e);
+ }
+ Set<String> options = updates.getNames("remote", remote);
+ for (String option : options) {
+ List<String> values = List.of(updates.getStringList("remote", remote, option));
+ remoteConfig.setStringList("remote", remote, option, values);
+ }
+ remoteConfig.save();
+ }
+
+ removeRemotes(updates);
+ super.update(updates);
+ }
+
private static void removeRemotes(Config config) {
Set<String> remoteNames = config.getSubsections("remote");
if (remoteNames.size() > 0) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/FileConfigResource.java b/src/main/java/com/googlesource/gerrit/plugins/replication/FileConfigResource.java
index e5bd8d0..7e19d2c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/FileConfigResource.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/FileConfigResource.java
@@ -17,12 +17,14 @@
import static com.googlesource.gerrit.plugins.replication.ReplicationQueue.repLog;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.Lists;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.common.UsedAt.Project;
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
import java.io.IOException;
import java.nio.file.Path;
+import java.util.List;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.storage.file.FileBasedConfig;
@@ -53,6 +55,25 @@
}
@Override
+ public void update(Config updates) throws IOException {
+ for (String section : updates.getSections()) {
+ for (String subsection : updates.getSubsections(section)) {
+ for (String name : updates.getNames(section, subsection, true)) {
+ List<String> values =
+ Lists.newArrayList(updates.getStringList(section, subsection, name));
+ config.setStringList(section, subsection, name, values);
+ }
+ }
+
+ for (String name : updates.getNames(section, true)) {
+ List<String> values = Lists.newArrayList(updates.getStringList(section, null, name));
+ config.setStringList(section, null, name, values);
+ }
+ }
+ config.save();
+ }
+
+ @Override
public String getVersion() {
return Long.toString(config.getFile().lastModified());
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
index c25f375..a9f8154 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
@@ -355,7 +355,7 @@
}
ReplicationState[] getStatesByRef(String ref) {
- Collection<ReplicationState> states = stateMap.get(ref);
+ List<ReplicationState> states = stateMap.get(ref);
return states.toArray(new ReplicationState[states.size()]);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteConfiguration.java b/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteConfiguration.java
index 05b4066..726bcf5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteConfiguration.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/RemoteConfiguration.java
@@ -14,7 +14,6 @@
package com.googlesource.gerrit.plugins.replication;
import com.google.common.collect.ImmutableList;
-import java.util.List;
import org.eclipse.jgit.transport.RemoteConfig;
/** Remote configuration for a replication endpoint */
@@ -117,7 +116,7 @@
* @return true, when configuration is for a single project, false otherwise
*/
default boolean isSingleProjectMatch() {
- List<String> projects = getProjects();
+ ImmutableList<String> projects = getProjects();
boolean ret = (projects.size() == 1);
if (ret) {
String projectMatch = projects.get(0);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImpl.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImpl.java
index 76ee14b..4f2def8 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImpl.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImpl.java
@@ -13,9 +13,13 @@
// limitations under the License.
package com.googlesource.gerrit.plugins.replication;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.SECONDS;
+
import com.google.common.base.Strings;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.annotations.PluginData;
+import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
import java.nio.file.Path;
@@ -30,7 +34,7 @@
private int maxRefsToLog;
private final int maxRefsToShow;
private int sshCommandTimeout;
- private int sshConnectionTimeout = DEFAULT_SSH_CONNECTION_TIMEOUT_MS;
+ private int sshConnectionTimeout;
private final MergedConfigResource configResource;
private final Path pluginDataDir;
@@ -44,6 +48,17 @@
this.defaultForceUpdate = config.getBoolean("gerrit", "defaultForceUpdate", false);
this.maxRefsToLog = config.getInt("gerrit", "maxRefsToLog", 0);
this.maxRefsToShow = config.getInt("gerrit", "maxRefsToShow", 2);
+ this.sshCommandTimeout =
+ (int) ConfigUtil.getTimeUnit(config, "gerrit", null, "sshCommandTimeout", 0, SECONDS);
+ this.sshConnectionTimeout =
+ (int)
+ ConfigUtil.getTimeUnit(
+ config,
+ "gerrit",
+ null,
+ "sshConnectionTimeout",
+ DEFAULT_SSH_CONNECTION_TIMEOUT_MS,
+ MILLISECONDS);
this.pluginDataDir = pluginDataDir;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigModule.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigModule.java
index 93a2b85..114ad15 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigModule.java
@@ -23,11 +23,14 @@
import com.google.inject.Inject;
import com.google.inject.ProvisionException;
import com.google.inject.Scopes;
+import com.google.inject.assistedinject.FactoryModuleBuilder;
import com.google.inject.internal.UniqueAnnotations;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
+
+import com.googlesource.gerrit.plugins.replication.events.ProjectDeletionState;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.util.FS;
@@ -55,6 +58,14 @@
} else {
bind(ReplicationConfig.class).to(ReplicationConfigImpl.class).in(Scopes.SINGLETON);
}
+
+ bind(ReplicationQueue.class).in(Scopes.SINGLETON);
+ bind(ObservableQueue.class).to(ReplicationQueue.class);
+ bind(ReplicationDestinations.class).to(DestinationsCollection.class);
+ bind(ConfigParser.class).to(DestinationConfigParser.class).in(Scopes.SINGLETON);
+
+ install(new FactoryModuleBuilder().build(Destination.Factory.class));
+ install(new FactoryModuleBuilder().build(ProjectDeletionState.Factory.class));
}
public FileBasedConfig getReplicationConfig() {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationModule.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationModule.java
index ac27280..a357a20 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationModule.java
@@ -52,10 +52,7 @@
@Override
protected void configure() {
- install(new FactoryModuleBuilder().build(Destination.Factory.class));
install(configModule);
- bind(ReplicationQueue.class).in(Scopes.SINGLETON);
- bind(ObservableQueue.class).to(ReplicationQueue.class);
bind(LifecycleListener.class)
.annotatedWith(UniqueAnnotations.create())
.to(ReplicationQueue.class);
@@ -77,10 +74,8 @@
.to(StartReplicationCapability.class);
install(new FactoryModuleBuilder().build(PushAll.Factory.class));
- install(new FactoryModuleBuilder().build(ProjectDeletionState.Factory.class));
bind(EventBus.class).in(Scopes.SINGLETON);
- bind(ReplicationDestinations.class).to(DestinationsCollection.class);
bind(ConfigParser.class).to(DestinationConfigParser.class).in(Scopes.SINGLETON);
DynamicSet.setOf(binder(), ReplicationStateListener.class);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesUpdater.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesUpdater.java
new file mode 100644
index 0000000..8c27ff6
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesUpdater.java
@@ -0,0 +1,108 @@
+// Copyright (C) 2024 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.googlesource.gerrit.plugins.replication;
+
+import static com.google.gerrit.common.UsedAt.Project.PLUGIN_GITHUB;
+
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.UsedAt;
+import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.gerrit.server.securestore.SecureStore;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.util.List;
+import org.eclipse.jgit.lib.Config;
+
+/** Public API to update replication plugin remotes configurations programmatically. */
+@Singleton
+@UsedAt(PLUGIN_GITHUB)
+public class ReplicationRemotesUpdater {
+
+ private final SecureStore secureStore;
+ private final Provider<ConfigResource> baseConfigProvider;
+ private final DynamicItem<ReplicationConfigOverrides> configOverridesItem;
+
+ @Inject
+ ReplicationRemotesUpdater(
+ SecureStore secureStore,
+ Provider<ConfigResource> baseConfigProvider,
+ @Nullable DynamicItem<ReplicationConfigOverrides> configOverridesItem) {
+ this.secureStore = secureStore;
+ this.baseConfigProvider = baseConfigProvider;
+ this.configOverridesItem = configOverridesItem;
+ }
+
+ /**
+ * Adds or updates the remote configuration for the replication plugin.
+ *
+ * <p>Provided JGit {@link Config} object should contain at least one named <em>remote</em>
+ * section. All other configurations will be ignored.
+ *
+ * <p>NOTE: The {@code remote.$name.password} will be stored using {@link SecureStore}.
+ *
+ * @param remoteConfig remotes to add or update
+ * @throws IOException when persisting fails
+ */
+ public void update(Config remoteConfig) throws IOException {
+ if (remoteConfig.getSubsections("remote").isEmpty()) {
+ throw new IllegalArgumentException(
+ "configuration update must have at least one 'remote' section");
+ }
+
+ SeparatedRemoteConfigs configs = onlyRemoteSectionsWithSeparatedPasswords(remoteConfig);
+ persistRemotesPasswords(configs);
+
+ if (hasConfigOverrides()) {
+ configOverridesItem.get().update(configs.remotes);
+ } else {
+ baseConfigProvider.get().update(configs.remotes);
+ }
+ }
+
+ private SeparatedRemoteConfigs onlyRemoteSectionsWithSeparatedPasswords(Config configUpdates) {
+ SeparatedRemoteConfigs configs = new SeparatedRemoteConfigs();
+ for (String subSection : configUpdates.getSubsections("remote")) {
+ for (String name : configUpdates.getNames("remote", subSection)) {
+ List<String> values = List.of(configUpdates.getStringList("remote", subSection, name));
+ if ("password".equals(name)) {
+ configs.passwords.setStringList("remote", subSection, "password", values);
+ } else {
+ configs.remotes.setStringList("remote", subSection, name, values);
+ }
+ }
+ }
+
+ return configs;
+ }
+
+ private void persistRemotesPasswords(SeparatedRemoteConfigs configs) {
+ for (String subSection : configs.passwords.getSubsections("remote")) {
+ List<String> values =
+ List.of(configs.passwords.getStringList("remote", subSection, "password"));
+ secureStore.setList("remote", subSection, "password", values);
+ }
+ }
+
+ private boolean hasConfigOverrides() {
+ return configOverridesItem != null && configOverridesItem.get() != null;
+ }
+
+ private static class SeparatedRemoteConfigs {
+ private final Config remotes = new Config();
+ private final Config passwords = new Config();
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java
index 6782700..61f81df 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java
@@ -16,12 +16,15 @@
import static com.google.common.io.RecursiveDeleteOption.ALLOW_INSECURE;
import static com.google.common.truth.Truth.assertThat;
+import static com.googlesource.gerrit.plugins.replication.FanoutConfigResource.CONFIG_DIR;
import com.google.common.io.MoreFiles;
import com.googlesource.gerrit.plugins.replication.ReplicationConfig.FilterType;
+import java.io.File;
import java.io.IOException;
import java.util.List;
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;
import org.junit.Before;
@@ -39,13 +42,18 @@
String remoteUrl2 = "ssh://git@git.elsewhere.com/${name}";
@Before
- public void setupTests() {
+ public void setupTests() throws Exception {
FileBasedConfig config = newReplicationConfig();
try {
config.save();
} catch (IOException e) {
throw new RuntimeException(e);
}
+ File replicationConfig = sitePaths.etc_dir.resolve(CONFIG_DIR).toFile();
+ if (!replicationConfig.mkdir()) {
+ throw new IOException(
+ "Cannot create test replication config directory in: " + replicationConfig.toPath().toAbsolutePath());
+ }
}
@Test
@@ -277,9 +285,69 @@
assertThat(objectUnderTest.getVersion()).isEqualTo(replicationConfigVersion);
}
+ @Test
+ public void shouldAddConfigOptionToMainConfig() throws Exception {
+ FanoutConfigResource objectUnderTest = new FanoutConfigResource(sitePaths);
+ Config update = new Config();
+ update.setString("new", null, "value", "set");
+
+ objectUnderTest.update(update);
+ Config updatedConfig = objectUnderTest.getConfig();
+
+ assertThat(updatedConfig.getString("new", null, "value")).isEqualTo("set");
+ }
+
+ @Test
+ public void shouldUpdateConfigOptionInMainConfig() throws Exception {
+ FileBasedConfig config = newReplicationConfig();
+ config.setString("updatable", null, "value", "orig");
+ config.save();
+ FanoutConfigResource objectUnderTest = new FanoutConfigResource(sitePaths);
+ Config update = new Config();
+ update.setString("updatable", null, "value", "updated");
+
+ objectUnderTest.update(update);
+ Config updatedConfig = objectUnderTest.getConfig();
+
+ assertThat(updatedConfig.getString("updatable", null, "value")).isEqualTo("updated");
+ }
+
+ @Test
+ public void shouldAddNewRemoteFile() throws Exception {
+ FanoutConfigResource objectUnderTest = new FanoutConfigResource(sitePaths);
+ Config update = new Config();
+ update.setString("remote", remoteName1, "url", remoteUrl1);
+
+ objectUnderTest.update(update);
+
+ Config actual = loadRemoteConfig(remoteName1);
+ assertThat(actual.getString("remote", remoteName1, "url")).isEqualTo(remoteUrl1);
+ }
+
+ @Test
+ public void shouldUpdateExistingRemote() throws Exception {
+ FileBasedConfig rawRemoteConfig = newRemoteConfig(remoteName1);
+ rawRemoteConfig.setString("remote", remoteName1, "url", remoteUrl1);
+ rawRemoteConfig.save();
+ FanoutConfigResource objectUnderTest = new FanoutConfigResource(sitePaths);
+ Config update = new Config();
+ update.setString("remote", remoteName1, "url", remoteUrl2);
+
+ objectUnderTest.update(update);
+
+ Config actual = loadRemoteConfig(remoteName1);
+ assertThat(actual.getString("remote", remoteName1, "url")).isEqualTo(remoteUrl2);
+ }
+
protected FileBasedConfig newRemoteConfig(String configFileName) {
return new FileBasedConfig(
sitePaths.etc_dir.resolve("replication/" + configFileName + ".config").toFile(),
FS.DETECTED);
}
+
+ private Config loadRemoteConfig(String siteName) throws Exception {
+ FileBasedConfig config = newRemoteConfig(siteName);
+ config.load();
+ return config;
+ }
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/FileConfigResourceTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/FileConfigResourceTest.java
new file mode 100644
index 0000000..9e8d36c
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/FileConfigResourceTest.java
@@ -0,0 +1,120 @@
+// Copyright (C) 2024 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.googlesource.gerrit.plugins.replication;
+
+import static com.google.common.io.RecursiveDeleteOption.ALLOW_INSECURE;
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.io.MoreFiles;
+import com.google.gerrit.server.config.SitePaths;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.util.FS;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class FileConfigResourceTest {
+ private static final String VALUE_KEY = "value";
+ private static final String INITIAL_KEY = "initial";
+ private static final String UPDATABLE_KEY = "updatable";
+
+ private Path testDir;
+ private SitePaths sitePaths;
+ private ConfigResource configResource;
+
+ @Before
+ public void setUp() throws Exception {
+ testDir = Files.createTempDirectory("fileConfigResourceTest");
+ sitePaths = new SitePaths(testDir);
+ configResource = newFileConfigResource();
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ MoreFiles.deleteRecursively(testDir, ALLOW_INSECURE);
+ }
+
+ @Test
+ public void updateEmptyFile() throws Exception {
+ Config configUpdate = newConfigUpdate();
+
+ Config beforeUpdate = configResource.getConfig();
+ assertThat(beforeUpdate.getSections()).isEmpty();
+
+ configResource.update(configUpdate);
+ Config updatedConfig = newFileConfigResource().getConfig();
+
+ assertConfigUpdate(updatedConfig);
+ }
+
+ @Test
+ public void appendOptionToConfig() throws Exception {
+ FileBasedConfig rawConfig = getRawReplicationConfig();
+ rawConfig.setInt(INITIAL_KEY, null, VALUE_KEY, 10);
+ rawConfig.save();
+ configResource = newFileConfigResource();
+ Config configUpdate = newConfigUpdate();
+
+ configResource.update(configUpdate);
+ Config updatedConfig = configResource.getConfig();
+
+ assertConfigUpdate(updatedConfig);
+ assertThat(updatedConfig.getInt(INITIAL_KEY, null, VALUE_KEY, -1)).isEqualTo(10);
+ }
+
+ @Test
+ public void updateExistingOption() throws Exception {
+ int expectedValue = 20;
+ Config configUpdate = new Config();
+ configUpdate.setInt(UPDATABLE_KEY, null, VALUE_KEY, expectedValue);
+ FileBasedConfig rawConfig = getRawReplicationConfig(newConfigUpdate());
+ rawConfig.save();
+
+ configResource.update(configUpdate);
+ Config updatedConfig = configResource.getConfig();
+
+ assertConfigUpdate(updatedConfig, expectedValue);
+ }
+
+ private FileConfigResource newFileConfigResource() {
+ return new FileConfigResource(sitePaths);
+ }
+
+ private Config newConfigUpdate() {
+ Config configUpdate = new Config();
+ configUpdate.setInt(UPDATABLE_KEY, null, VALUE_KEY, 1);
+ return configUpdate;
+ }
+
+ private void assertConfigUpdate(Config config) {
+ assertConfigUpdate(config, 1);
+ }
+
+ private void assertConfigUpdate(Config config, int expectedValue) {
+ assertThat(config.getInt(UPDATABLE_KEY, null, VALUE_KEY, -1)).isEqualTo(expectedValue);
+ }
+
+ private FileBasedConfig getRawReplicationConfig() {
+ return getRawReplicationConfig(new Config());
+ }
+
+ private FileBasedConfig getRawReplicationConfig(Config base) {
+ Path configPath = sitePaths.etc_dir.resolve(FileConfigResource.CONFIG_NAME);
+ return new FileBasedConfig(base, configPath.toFile(), FS.DETECTED);
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/MergedConfigResourceTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/MergedConfigResourceTest.java
index 86da1c0..c9ce2c5 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/MergedConfigResourceTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/MergedConfigResourceTest.java
@@ -19,6 +19,7 @@
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.inject.AbstractModule;
import com.google.inject.Guice;
+import org.apache.commons.lang3.NotImplementedException;
import org.eclipse.jgit.lib.Config;
import org.junit.Test;
@@ -75,6 +76,11 @@
}
@Override
+ public void update(Config config) {
+ throw new NotImplementedException("not implemented");
+ }
+
+ @Override
public String getVersion() {
return "base";
}
@@ -90,6 +96,11 @@
}
@Override
+ public void update(Config config) {
+ throw new NotImplementedException("not implemented");
+ }
+
+ @Override
public String getVersion() {
return "override";
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesUpdaterTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesUpdaterTest.java
new file mode 100644
index 0000000..6e57ba7
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesUpdaterTest.java
@@ -0,0 +1,149 @@
+// Copyright (C) 2024 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.googlesource.gerrit.plugins.replication;
+
+import static com.google.common.io.RecursiveDeleteOption.ALLOW_INSECURE;
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import com.google.common.io.MoreFiles;
+import com.google.common.truth.StringSubject;
+import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.securestore.SecureStore;
+import com.google.inject.util.Providers;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+import org.eclipse.jgit.lib.Config;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ReplicationRemotesUpdaterTest {
+
+ private Path testSite;
+ private SecureStore secureStoreMock;
+ private FileConfigResource baseConfig;
+
+ @Before
+ public void setUp() throws Exception {
+ testSite = Files.createTempDirectory("replicationRemotesUpdateTest");
+ secureStoreMock = mock(SecureStore.class);
+ baseConfig = new FileConfigResource(new SitePaths(testSite));
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ MoreFiles.deleteRecursively(testSite, ALLOW_INSECURE);
+ }
+
+ @Test
+ public void shouldThrowWhenNoRemotesInTheUpdate() {
+ Config update = new Config();
+ ReplicationRemotesUpdater objectUnderTest = newReplicationConfigUpdater();
+
+ assertThrows(IllegalArgumentException.class, () -> objectUnderTest.update(update));
+
+ update.setString("non-remote", null, "value", "one");
+
+ assertThrows(IllegalArgumentException.class, () -> objectUnderTest.update(update));
+ }
+
+ @Test
+ public void addRemoteSectionToBaseConfigWhenNoOverrides() throws Exception {
+ String url = "fake_url";
+ Config update = new Config();
+ setRemoteSite(update, "url", url);
+ ReplicationRemotesUpdater objectUnderTest = newReplicationConfigUpdater();
+
+ objectUnderTest.update(update);
+
+ assertRemoteSite(baseConfig.getConfig(), "url").isEqualTo(url);
+ }
+
+ @Test
+ public void addRemoteSectionToBaseOverridesConfig() throws Exception {
+ TestReplicationConfigOverrides testOverrides = new TestReplicationConfigOverrides();
+ String url = "fake_url";
+ Config update = new Config();
+ setRemoteSite(update, "url", url);
+ ReplicationRemotesUpdater objectUnderTest = newReplicationConfigUpdater(testOverrides);
+
+ objectUnderTest.update(update);
+
+ assertRemoteSite(testOverrides.getConfig(), "url").isEqualTo(url);
+ assertRemoteSite(baseConfig.getConfig(), "url").isNull();
+ }
+
+ @Test
+ public void encryptPassword() throws Exception {
+ TestReplicationConfigOverrides testOverrides = new TestReplicationConfigOverrides();
+ Config update = new Config();
+ String password = "my_secret_password";
+ setRemoteSite(update, "password", password);
+ ReplicationRemotesUpdater objectUnderTest = newReplicationConfigUpdater(testOverrides);
+
+ objectUnderTest.update(update);
+
+ verify(secureStoreMock).setList("remote", "site", "password", List.of(password));
+ assertRemoteSite(baseConfig.getConfig(), "password").isNull();
+ assertRemoteSite(testOverrides.getConfig(), "password").isNull();
+ }
+
+ private ReplicationRemotesUpdater newReplicationConfigUpdater() {
+ return newReplicationConfigUpdater(null);
+ }
+
+ private void setRemoteSite(Config config, String name, String value) {
+ config.setString("remote", "site", name, value);
+ }
+
+ private StringSubject assertRemoteSite(Config config, String name) {
+ return assertThat(config.getString("remote", "site", name));
+ }
+
+ private ReplicationRemotesUpdater newReplicationConfigUpdater(
+ ReplicationConfigOverrides overrides) {
+ DynamicItem<ReplicationConfigOverrides> dynamicItemMock = mock(DynamicItem.class);
+ when(dynamicItemMock.get()).thenReturn(overrides);
+
+ return new ReplicationRemotesUpdater(
+ secureStoreMock, Providers.of(baseConfig), dynamicItemMock);
+ }
+
+ static class TestReplicationConfigOverrides implements ReplicationConfigOverrides {
+ private Config config = new Config();
+
+ @Override
+ public Config getConfig() {
+ return config;
+ }
+
+ @Override
+ public void update(Config update) throws IOException {
+ config = update;
+ }
+
+ @Override
+ public String getVersion() {
+ return "none";
+ }
+ }
+}
