FanoutConfigResourceTest: assert configuration updates being visible
When updating a remote configuration using the FanoutConfigResource,
the updated values should be also visible in-memory and not simply
stored on the underlying filesystem.
Fix an issue where the updates to the remotes performed using
$GERRIT_SITE/etc/replication/remote.config fan-out config scheme
was persisted on disk but left a stale in-memory copy.
Change-Id: I2360ba447ffbc3695faad72e5674ca883f75081a
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 0a2afa6..5d308a5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResource.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResource.java
@@ -30,6 +30,7 @@
import java.util.List;
import java.util.Optional;
import java.util.Set;
+import java.util.function.Predicate;
import java.util.stream.Stream;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.internal.storage.file.FileSnapshot;
@@ -65,6 +66,8 @@
@Override
public void update(Config updates) throws IOException {
+ super.update(filterOutRemotes(updates));
+
Set<String> remotes = updates.getSubsections("remote");
for (String remote : remotes) {
File remoteFile = remoteConfigsDirPath.resolve(remote + ".config").toFile();
@@ -79,12 +82,10 @@
for (String option : options) {
List<String> values = List.of(updates.getStringList("remote", remote, option));
remoteConfig.setStringList("remote", remote, option, values);
+ config.setStringList("remote", remote, option, values);
}
remoteConfig.save();
}
-
- removeRemotes(updates);
- super.update(updates);
}
private static void removeRemotes(Config config) {
@@ -111,6 +112,40 @@
}
}
+ private static Config filterOutRemotes(Config config) {
+ Config filteredConfig = new Config();
+ Set<String> sections = config.getSections();
+ sections.stream()
+ .filter(Predicate.not("remote"::equalsIgnoreCase))
+ .forEach(
+ section -> {
+ config
+ .getNames(section)
+ .forEach(
+ sectionName ->
+ filteredConfig.setStringList(
+ section,
+ null,
+ sectionName,
+ List.of(config.getStringList(section, null, sectionName))));
+ config
+ .getSubsections(section)
+ .forEach(
+ subsection ->
+ config
+ .getNames(section, subsection)
+ .forEach(
+ name ->
+ filteredConfig.setStringList(
+ section,
+ subsection,
+ name,
+ List.of(
+ config.getStringList(section, subsection, name)))));
+ });
+ return filteredConfig;
+ }
+
private static boolean isValid(Config cfg) {
if (cfg.getSections().size() != 1 || !cfg.getSections().contains("remote")) {
logger.atSevere().log(
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 a622170..c91ac14 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java
@@ -321,6 +321,8 @@
objectUnderTest.update(update);
+ assertThat(objectUnderTest.getConfig().getString("remote", remoteName1, "url"))
+ .isEqualTo(remoteUrl1);
Config actual = loadRemoteConfig(remoteName1);
assertThat(actual.getString("remote", remoteName1, "url")).isEqualTo(remoteUrl1);
}
@@ -336,6 +338,8 @@
objectUnderTest.update(update);
+ assertThat(objectUnderTest.getConfig().getString("remote", remoteName1, "url"))
+ .isEqualTo(remoteUrl2);
Config actual = loadRemoteConfig(remoteName1);
assertThat(actual.getString("remote", remoteName1, "url")).isEqualTo(remoteUrl2);
}
