Split concerns of Configuration and Destinations
Preparation work to start breaking down the whole logic of controlling
the replication destinations into two separate concerns:
1. ReplicationConfig
Parse and load of the replication.config
2. ReplicationDestinations
Activate and control the scheduling of the replication destinations
Both interfaces are implemented by the same Config classes; However,
in the follow-up of this change, each interface will have a focussed
implementation.
Having different interfaces will allow also to have other possible
implementations of the replication logic to a destination.
Feature: Issue 11425
Change-Id: I2e5a27f14e20b18d12215071219097d19355ac05
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
index 21c3960..fe6808c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
@@ -36,7 +36,7 @@
import org.eclipse.jgit.transport.URIish;
@Singleton
-public class AutoReloadConfigDecorator implements ReplicationConfig {
+public class AutoReloadConfigDecorator implements ReplicationConfig, ReplicationDestinations {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private static final long RELOAD_DELAY = 120;
private static final long RELOAD_INTERVAL = 60;
@@ -85,8 +85,8 @@
}
@Override
- public synchronized List<Destination> getDestinations(FilterType filterType) {
- return currentConfig.getDestinations(filterType);
+ public synchronized List<Destination> getAll(FilterType filterType) {
+ return currentConfig.getAll(filterType);
}
@Override
@@ -121,7 +121,7 @@
lastFailedConfigTs = 0;
logger.atInfo().log(
"Configuration reloaded: %d destinations",
- currentConfig.getDestinations(FilterType.ALL).size());
+ currentConfig.getAll(FilterType.ALL).size());
}
} catch (Exception e) {
logger.atSevere().withCause(e).log(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/CreateProjectTask.java b/src/main/java/com/googlesource/gerrit/plugins/replication/CreateProjectTask.java
index a8dede3..804d116 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/CreateProjectTask.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/CreateProjectTask.java
@@ -31,7 +31,7 @@
}
private final RemoteConfig config;
- private final ReplicationConfig replicationConfig;
+ private final ReplicationDestinations replicationDestinations;
private final DynamicItem<AdminApiFactory> adminApiFactory;
private final Project.NameKey project;
private final String head;
@@ -39,19 +39,19 @@
@Inject
CreateProjectTask(
RemoteConfig config,
- ReplicationConfig replicationConfig,
+ ReplicationDestinations replicationDestinations,
DynamicItem<AdminApiFactory> adminApiFactory,
@Assisted Project.NameKey project,
@Assisted String head) {
this.config = config;
- this.replicationConfig = replicationConfig;
+ this.replicationDestinations = replicationDestinations;
this.adminApiFactory = adminApiFactory;
this.project = project;
this.head = head;
}
public boolean create() {
- return replicationConfig
+ return replicationDestinations
.getURIs(Optional.of(config.getName()), project, FilterType.PROJECT_CREATION).values()
.stream()
.map(u -> createProject(u, project, head))
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ListCommand.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ListCommand.java
index fa17dce..d337883 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ListCommand.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ListCommand.java
@@ -40,11 +40,11 @@
@Option(name = "--json", usage = "output in json format")
private boolean json;
- @Inject private ReplicationConfig config;
+ @Inject private ReplicationDestinations destinations;
@Override
protected void run() {
- for (Destination d : config.getDestinations(FilterType.ALL)) {
+ for (Destination d : destinations.getAll(FilterType.ALL)) {
if (matches(d.getRemoteConfigName())) {
printRemote(d);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfig.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfig.java
index 929c538..12b85db 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfig.java
@@ -11,43 +11,54 @@
// 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 com.google.common.collect.Multimap;
-import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.server.git.WorkQueue;
import java.nio.file.Path;
-import java.util.List;
-import java.util.Optional;
-import org.eclipse.jgit.transport.URIish;
+/** Configuration of all the replication end points. */
public interface ReplicationConfig {
+ /** Filter for accessing replication projects. */
enum FilterType {
PROJECT_CREATION,
PROJECT_DELETION,
ALL
}
- List<Destination> getDestinations(FilterType filterType);
-
- Multimap<Destination, URIish> getURIs(
- Optional<String> remoteName, Project.NameKey projectName, FilterType filterType);
-
+ /**
+ * Returns current replication configuration of whether to replicate or not all the projects when
+ * the plugin starts.
+ *
+ * @return true if replication at plugin start, false otherwise.
+ */
boolean isReplicateAllOnPluginStart();
+ /**
+ * Returns the default behaviour of the replication plugin when pushing to remote replication
+ * ends. Even though the property name has the 'update' suffix, it actually refers to Git push
+ * operation and not to a Git update.
+ *
+ * @return true if forced push is the default, false otherwise.
+ */
boolean isDefaultForceUpdate();
+ /**
+ * Returns the maximum number of ref-specs to log into the replication_log whenever a push
+ * operation is completed against a replication end.
+ *
+ * @return maximum number of refs to log, zero if unlimited.
+ */
int getMaxRefsToLog();
- boolean isEmpty();
-
+ /**
+ * Configured location where the replication events are stored on the filesystem for being resumed
+ * and kept across restarts.
+ *
+ * @return path to store persisted events.
+ */
Path getEventsDirectory();
- int shutdown();
-
- void startup(WorkQueue workQueue);
-
int getSshConnectionTimeout();
int getSshCommandTimeout();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationDestinations.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationDestinations.java
new file mode 100644
index 0000000..1a6b77f
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationDestinations.java
@@ -0,0 +1,63 @@
+// Copyright (C) 2019 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 com.google.common.collect.Multimap;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.git.WorkQueue;
+import com.googlesource.gerrit.plugins.replication.ReplicationConfig.FilterType;
+import java.util.List;
+import java.util.Optional;
+import org.eclipse.jgit.transport.URIish;
+
+/** Git destinations currently active for replication. */
+public interface ReplicationDestinations {
+
+ /**
+ * Return all the URIs associated to a project and a filter criteria.
+ *
+ * @param remoteName name of the replication end or empty if selecting all ends.
+ * @param projectName name of the project
+ * @param filterType type of filter criteria for selecting projects
+ * @return the multi-map of destinations and the associated replication URIs
+ */
+ Multimap<Destination, URIish> getURIs(
+ Optional<String> remoteName, Project.NameKey projectName, FilterType filterType);
+
+ /**
+ * List of currently active replication destinations.
+ *
+ * @param filterType type project filtering
+ * @return the list of active destinations
+ */
+ List<Destination> getAll(FilterType filterType);
+
+ /** @return true if there are no destinations, false otherwise. */
+ boolean isEmpty();
+
+ /**
+ * Start replicating to all destinations.
+ *
+ * @param workQueue execution queue for scheduling the replication events.
+ */
+ void startup(WorkQueue workQueue);
+
+ /**
+ * Stop the replication to all destinations.
+ *
+ * @return number of events cancelled during shutdown.
+ */
+ int shutdown();
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java
index a075f12..b363a6a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfig.java
@@ -53,7 +53,7 @@
import org.eclipse.jgit.util.FS;
@Singleton
-public class ReplicationFileBasedConfig implements ReplicationConfig {
+public class ReplicationFileBasedConfig implements ReplicationConfig, ReplicationDestinations {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private static final int DEFAULT_SSH_CONNECTION_TIMEOUT_MS = 2 * 60 * 1000; // 2 minutes
@@ -86,7 +86,7 @@
* (com.googlesource.gerrit.plugins.replication.ReplicationConfig.FilterType)
*/
@Override
- public List<Destination> getDestinations(FilterType filterType) {
+ public List<Destination> getAll(FilterType filterType) {
Predicate<? super Destination> filter;
switch (filterType) {
case PROJECT_CREATION:
@@ -183,12 +183,12 @@
@Override
public Multimap<Destination, URIish> getURIs(
Optional<String> remoteName, Project.NameKey projectName, FilterType filterType) {
- if (getDestinations(filterType).isEmpty()) {
+ if (getAll(filterType).isEmpty()) {
return ImmutableMultimap.of();
}
SetMultimap<Destination, URIish> uris = HashMultimap.create();
- for (Destination config : getDestinations(filterType)) {
+ for (Destination config : getAll(filterType)) {
if (!config.wouldPushProject(projectName)) {
continue;
}
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 835d068..fcd58b8 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationModule.java
@@ -58,6 +58,7 @@
install(new FactoryModuleBuilder().build(PushAll.Factory.class));
bind(ReplicationConfig.class).to(AutoReloadConfigDecorator.class);
+ bind(ReplicationDestinations.class).to(AutoReloadConfigDecorator.class);
DynamicSet.setOf(binder(), ReplicationStateListener.class);
DynamicSet.bind(binder(), ReplicationStateListener.class).to(ReplicationStateLogger.class);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
index 0afcffe..9f8f8fb 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
@@ -47,6 +47,7 @@
private final WorkQueue workQueue;
private final DynamicItem<EventDispatcher> dispatcher;
+ private final ReplicationDestinations destinations;
private final ReplicationConfig config;
private final ReplicationTasksStorage replicationTasksStorage;
private volatile boolean running;
@@ -55,12 +56,14 @@
@Inject
ReplicationQueue(
WorkQueue wq,
+ ReplicationDestinations rd,
ReplicationConfig rc,
DynamicItem<EventDispatcher> dis,
ReplicationStateListeners sl,
ReplicationTasksStorage rts) {
workQueue = wq;
dispatcher = dis;
+ destinations = rd;
config = rc;
stateLog = sl;
replicationTasksStorage = rts;
@@ -69,7 +72,7 @@
@Override
public void start() {
if (!running) {
- config.startup(workQueue);
+ destinations.startup(workQueue);
running = true;
firePendingEvents();
}
@@ -78,7 +81,7 @@
@Override
public void stop() {
running = false;
- int discarded = config.shutdown();
+ int discarded = destinations.shutdown();
if (discarded > 0) {
repLog.warn("Canceled {} replication events during shutdown", discarded);
}
@@ -104,7 +107,7 @@
return;
}
- for (Destination cfg : config.getDestinations(FilterType.ALL)) {
+ for (Destination cfg : destinations.getAll(FilterType.ALL)) {
if (cfg.wouldPushProject(project)) {
for (URIish uri : cfg.getURIs(project, urlMatch)) {
cfg.schedule(project, PushOne.ALL_REFS, uri, state, now);
@@ -129,7 +132,7 @@
}
Project.NameKey project = Project.nameKey(projectName);
- for (Destination cfg : config.getDestinations(FilterType.ALL)) {
+ for (Destination cfg : destinations.getAll(FilterType.ALL)) {
if (cfg.wouldPushProject(project) && cfg.wouldPushRef(refName)) {
for (URIish uri : cfg.getURIs(project, null)) {
replicationTasksStorage.persist(
@@ -162,14 +165,14 @@
@Override
public void onProjectDeleted(ProjectDeletedListener.Event event) {
Project.NameKey p = Project.nameKey(event.getProjectName());
- config.getURIs(Optional.empty(), p, FilterType.PROJECT_DELETION).entries().stream()
+ destinations.getURIs(Optional.empty(), p, FilterType.PROJECT_DELETION).entries().stream()
.forEach(e -> e.getKey().scheduleDeleteProject(e.getValue(), p));
}
@Override
public void onHeadUpdated(HeadUpdatedListener.Event event) {
Project.NameKey p = Project.nameKey(event.getProjectName());
- config.getURIs(Optional.empty(), p, FilterType.ALL).entries().stream()
+ destinations.getURIs(Optional.empty(), p, FilterType.ALL).entries().stream()
.forEach(e -> e.getKey().scheduleUpdateHead(e.getValue(), p, event.getNewHeadName()));
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecoratorTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecoratorTest.java
index 211cafa..8c1699a 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecoratorTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecoratorTest.java
@@ -176,7 +176,7 @@
"replication",
workQueueMock);
- List<Destination> destinations = autoReloadConfig.getDestinations(FilterType.ALL);
+ List<Destination> destinations = autoReloadConfig.getAll(FilterType.ALL);
assertThat(destinations).hasSize(1);
assertThatIsDestination(destinations.get(0), remoteName, remoteUrl);
}
@@ -200,7 +200,7 @@
workQueueMock);
autoReloadConfig.startup(workQueueMock);
- List<Destination> destinations = autoReloadConfig.getDestinations(FilterType.ALL);
+ List<Destination> destinations = autoReloadConfig.getAll(FilterType.ALL);
assertThat(destinations).hasSize(1);
assertThatIsDestination(destinations.get(0), remoteName1, remoteUrl1);
@@ -212,7 +212,7 @@
replicationConfig.save();
executorService.refreshCommand.run();
- destinations = autoReloadConfig.getDestinations(FilterType.ALL);
+ destinations = autoReloadConfig.getAll(FilterType.ALL);
assertThat(destinations).hasSize(2);
assertThatContainsDestination(destinations, remoteName1, remoteUrl1);
assertThatContainsDestination(destinations, remoteName2, remoteUrl2);
@@ -237,7 +237,7 @@
workQueueMock);
autoReloadConfig.startup(workQueueMock);
- List<Destination> destinations = autoReloadConfig.getDestinations(FilterType.ALL);
+ List<Destination> destinations = autoReloadConfig.getAll(FilterType.ALL);
assertThat(destinations).hasSize(1);
assertThatIsDestination(destinations.get(0), remoteName1, remoteUrl1);
@@ -247,6 +247,6 @@
replicationConfig.save();
executorService.refreshCommand.run();
- assertThat(autoReloadConfig.getDestinations(FilterType.ALL)).isEqualTo(destinations);
+ assertThat(autoReloadConfig.getAll(FilterType.ALL)).isEqualTo(destinations);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfigTest.java
index 36cc209..3bfcf40 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfigTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationFileBasedConfigTest.java
@@ -38,7 +38,7 @@
config.save();
ReplicationFileBasedConfig replicationConfig = newReplicationFileBasedConfig();
- List<Destination> destinations = replicationConfig.getDestinations(FilterType.ALL);
+ List<Destination> destinations = replicationConfig.getAll(FilterType.ALL);
assertThat(destinations).hasSize(1);
assertThatIsDestination(destinations.get(0), remoteName, remoteUrl);
@@ -56,7 +56,7 @@
config.save();
ReplicationFileBasedConfig replicationConfig = newReplicationFileBasedConfig();
- List<Destination> destinations = replicationConfig.getDestinations(FilterType.ALL);
+ List<Destination> destinations = replicationConfig.getAll(FilterType.ALL);
assertThat(destinations).hasSize(2);
assertThatIsDestination(destinations.get(0), remoteName1, remoteUrl1);
