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()