Fix issue with auto reload of sources
Bind SourceConfigParser in the module to create collection of
SourceConfiguration instead of DesetinationConfiguration objects.
This binding solves the issue with removing source configuration
during the auto reload.
Feature: Issue 12720
Change-Id: I770baaa7bc7292a668a72b33d5a4103fcd779111
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationModule.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationModule.java
index 5541da6..90295c0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationModule.java
@@ -32,6 +32,7 @@
import com.google.inject.internal.UniqueAnnotations;
import com.googlesource.gerrit.plugins.replication.AutoReloadConfigDecorator;
import com.googlesource.gerrit.plugins.replication.AutoReloadSecureCredentialsFactoryDecorator;
+import com.googlesource.gerrit.plugins.replication.ConfigParser;
import com.googlesource.gerrit.plugins.replication.CredentialsFactory;
import com.googlesource.gerrit.plugins.replication.FanoutReplicationConfig;
import com.googlesource.gerrit.plugins.replication.MainReplicationConfig;
@@ -98,7 +99,7 @@
DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(ReplicationQueue.class);
- bind(ConfigParser.class).in(Scopes.SINGLETON);
+ bind(ConfigParser.class).to(SourceConfigParser.class).in(Scopes.SINGLETON);
if (getReplicationConfig().getBoolean("gerrit", "autoReload", false)) {
bind(ReplicationConfig.class)
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/ConfigParser.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParser.java
similarity index 92%
rename from src/main/java/com/googlesource/gerrit/plugins/replication/pull/ConfigParser.java
rename to src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParser.java
index 7440e0f..e7400ec 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/ConfigParser.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourceConfigParser.java
@@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.flogger.FluentLogger;
+import com.googlesource.gerrit.plugins.replication.ConfigParser;
import com.googlesource.gerrit.plugins.replication.RemoteConfiguration;
import java.net.URISyntaxException;
import java.util.Collections;
@@ -27,16 +28,12 @@
import org.eclipse.jgit.transport.RemoteConfig;
import org.eclipse.jgit.transport.URIish;
-public class ConfigParser {
+public class SourceConfigParser implements ConfigParser {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- /**
- * parse the new replication config
- *
- * @param config new configuration to parse
- * @return List of parsed {@link RemoteConfiguration}
- * @throws ConfigInvalidException if the new configuration is not valid.
+ /* (non-Javadoc)
+ * @see com.googlesource.gerrit.plugins.replication.ConfigParser#parseRemotes(org.eclipse.jgit.lib.Config)
*/
public List<RemoteConfiguration> parseRemotes(Config config) throws ConfigInvalidException {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourcesCollection.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourcesCollection.java
index acdac0d..81b77a3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourcesCollection.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/SourcesCollection.java
@@ -22,6 +22,7 @@
import com.google.gerrit.server.git.WorkQueue;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import com.googlesource.gerrit.plugins.replication.ConfigParser;
import com.googlesource.gerrit.plugins.replication.RemoteConfiguration;
import com.googlesource.gerrit.plugins.replication.ReplicationConfig;
import java.util.List;
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationFanoutConfigIT.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationFanoutConfigIT.java
index 1badb5c..fa99a62 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationFanoutConfigIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/PullReplicationFanoutConfigIT.java
@@ -29,7 +29,9 @@
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.replication.AutoReloadConfigDecorator;
import java.io.IOException;
+import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Duration;
import java.util.Arrays;
@@ -152,6 +154,62 @@
}
}
+ @Test
+ public void shouldAutoReloadConfiguration() throws Exception {
+ SourcesCollection sources = getInstance(SourcesCollection.class);
+ AutoReloadConfigDecorator autoReloadConfigDecorator =
+ getInstance(AutoReloadConfigDecorator.class);
+ assertThat(sources.getAll().get(0).getTimeout()).isEqualTo(600);
+ remoteConfig.setInt("remote", null, "timeout", 1000);
+ remoteConfig.save();
+ autoReloadConfigDecorator.reload();
+ waitUntil(() -> !sources.getAll().isEmpty() && sources.getAll().get(0).getTimeout() == 1000);
+ }
+
+ @Test
+ public void shouldAutoReloadConfigurationWhenRemoteConfigAdded() throws Exception {
+ FileBasedConfig newRemoteConfig =
+ new FileBasedConfig(
+ sitePaths.etc_dir.resolve("replication/new-remote-config.config").toFile(),
+ FS.DETECTED);
+ try {
+ SourcesCollection sources = getInstance(SourcesCollection.class);
+ AutoReloadConfigDecorator autoReloadConfigDecorator =
+ getInstance(AutoReloadConfigDecorator.class);
+ assertThat(sources.getAll().size()).isEqualTo(1);
+
+ setRemoteConfig(newRemoteConfig, TEST_REPLICATION_SUFFIX, ALL_PROJECTS);
+ autoReloadConfigDecorator.reload();
+ waitUntil(() -> sources.getAll().size() == 2);
+
+ } finally {
+ // clean up
+ Files.delete(newRemoteConfig.getFile().toPath());
+ }
+ }
+
+ @Test
+ public void shouldAutoReloadConfigurationWhenRemoteConfigDeleted() throws Exception {
+ SourcesCollection sources = getInstance(SourcesCollection.class);
+ AutoReloadConfigDecorator autoReloadConfigDecorator =
+ getInstance(AutoReloadConfigDecorator.class);
+ assertThat(sources.getAll().size()).isEqualTo(1);
+
+ FileBasedConfig newRemoteConfig =
+ new FileBasedConfig(
+ sitePaths.etc_dir.resolve("replication/new-remote-config.config").toFile(),
+ FS.DETECTED);
+ setRemoteConfig(newRemoteConfig, TEST_REPLICATION_SUFFIX, ALL_PROJECTS);
+ autoReloadConfigDecorator.reload();
+ waitUntil(() -> sources.getAll().size() == 2);
+
+ Files.delete(newRemoteConfig.getFile().toPath());
+
+ autoReloadConfigDecorator.reload();
+
+ waitUntil(() -> sources.getAll().size() == 1);
+ }
+
private Ref getRef(Repository repo, String branchName) throws IOException {
return repo.getRefDatabase().exactRef(branchName);
}
@@ -172,10 +230,17 @@
}
private void setRemoteConfig(String replicaSuffix, Optional<String> project) throws IOException {
- setRemoteConfig(Arrays.asList(replicaSuffix), project);
+ setRemoteConfig(remoteConfig, replicaSuffix, project);
}
- private void setRemoteConfig(List<String> replicaSuffixes, Optional<String> project)
+ private void setRemoteConfig(
+ FileBasedConfig remoteConfig, String replicaSuffix, Optional<String> project)
+ throws IOException {
+ setRemoteConfig(remoteConfig, Arrays.asList(replicaSuffix), project);
+ }
+
+ private void setRemoteConfig(
+ FileBasedConfig remoteConfig, List<String> replicaSuffixes, Optional<String> project)
throws IOException {
List<String> replicaUrls =
replicaSuffixes.stream()
