Allow AdminApiFactory to be replaced dynamically
Since change Ie760bf3e1 the GerritSshApi class is no longer a singleton,
and is instead instantiated on demand by a new AdminApiFactory.
This breaks implementations that consume the replication plugin as a
library and extend the GerritSshApi, since the extended class no longer
gets instantiated in place of GerritSshApi.
Refactor it so that AdminApiFactory is an interface with a default
implementation that gets bound as a dynamic item, which can be replaced
by derived implementations.
Change-Id: Ia150d6802e11015fa00ee9144b3dfbfa696c7a0d
Signed-off-by: David Pursehouse <dpursehouse@collab.net>
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/AdminApiFactory.java b/src/main/java/com/googlesource/gerrit/plugins/replication/AdminApiFactory.java
index e5a1628..30e8245 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/AdminApiFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/AdminApiFactory.java
@@ -19,37 +19,49 @@
import java.util.Optional;
import org.eclipse.jgit.transport.URIish;
-@Singleton
-public class AdminApiFactory {
+/** Factory for creating an {@link AdminApi} instance for a remote URI. */
+public interface AdminApiFactory {
+ /**
+ * Create an {@link AdminApi} for the given remote URI.
+ *
+ * @param uri the remote URI.
+ * @return An API for the given remote URI, or {@code Optional.empty} if there is no appropriate
+ * API for the URI.
+ */
+ Optional<AdminApi> create(URIish uri);
- private final SshHelper sshHelper;
- private final GerritRestApi.Factory gerritRestApiFactory;
+ @Singleton
+ static class DefaultAdminApiFactory implements AdminApiFactory {
+ protected final SshHelper sshHelper;
+ private final GerritRestApi.Factory gerritRestApiFactory;
- @Inject
- AdminApiFactory(SshHelper sshHelper, GerritRestApi.Factory gerritRestApiFactory) {
- this.sshHelper = sshHelper;
- this.gerritRestApiFactory = gerritRestApiFactory;
- }
-
- public Optional<AdminApi> create(URIish uri) {
- if (isGerrit(uri)) {
- return Optional.of(new GerritSshApi(sshHelper, uri));
- } else if (!uri.isRemote()) {
- return Optional.of(new LocalFS(uri));
- } else if (isSSH(uri)) {
- return Optional.of(new RemoteSsh(sshHelper, uri));
- } else if (isGerritHttp(uri)) {
- return Optional.of(gerritRestApiFactory.create(uri));
+ @Inject
+ public DefaultAdminApiFactory(SshHelper sshHelper, GerritRestApi.Factory gerritRestApiFactory) {
+ this.sshHelper = sshHelper;
+ this.gerritRestApiFactory = gerritRestApiFactory;
}
- return Optional.empty();
+
+ @Override
+ public Optional<AdminApi> create(URIish uri) {
+ if (isGerrit(uri)) {
+ return Optional.of(new GerritSshApi(sshHelper, uri));
+ } else if (!uri.isRemote()) {
+ return Optional.of(new LocalFS(uri));
+ } else if (isSSH(uri)) {
+ return Optional.of(new RemoteSsh(sshHelper, uri));
+ } else if (isGerritHttp(uri)) {
+ return Optional.of(gerritRestApiFactory.create(uri));
+ }
+ return Optional.empty();
+ }
}
- public static boolean isGerrit(URIish uri) {
+ static boolean isGerrit(URIish uri) {
String scheme = uri.getScheme();
return scheme != null && scheme.toLowerCase().equals("gerrit+ssh");
}
- public static boolean isSSH(URIish uri) {
+ static boolean isSSH(URIish uri) {
if (!uri.isRemote()) {
return false;
}
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 e3d57de..a8dede3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/CreateProjectTask.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/CreateProjectTask.java
@@ -16,6 +16,7 @@
import static com.googlesource.gerrit.plugins.replication.ReplicationQueue.repLog;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.reviewdb.client.Project;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@@ -31,7 +32,7 @@
private final RemoteConfig config;
private final ReplicationConfig replicationConfig;
- private final AdminApiFactory adminApiFactory;
+ private final DynamicItem<AdminApiFactory> adminApiFactory;
private final Project.NameKey project;
private final String head;
@@ -39,7 +40,7 @@
CreateProjectTask(
RemoteConfig config,
ReplicationConfig replicationConfig,
- AdminApiFactory adminApiFactory,
+ DynamicItem<AdminApiFactory> adminApiFactory,
@Assisted Project.NameKey project,
@Assisted String head) {
this.config = config;
@@ -58,7 +59,7 @@
}
private boolean createProject(URIish replicateURI, Project.NameKey projectName, String head) {
- Optional<AdminApi> adminApi = adminApiFactory.create(replicateURI);
+ Optional<AdminApi> adminApi = adminApiFactory.get().create(replicateURI);
if (adminApi.isPresent() && adminApi.get().createProject(projectName, head)) {
return true;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/DeleteProjectTask.java b/src/main/java/com/googlesource/gerrit/plugins/replication/DeleteProjectTask.java
index 96d8c70..f9b2ad7 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/DeleteProjectTask.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/DeleteProjectTask.java
@@ -16,6 +16,7 @@
import static com.googlesource.gerrit.plugins.replication.ReplicationQueue.repLog;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.ioutil.HexFormat;
import com.google.gerrit.server.util.IdGenerator;
@@ -29,14 +30,14 @@
DeleteProjectTask create(URIish replicateURI, Project.NameKey project);
}
- private final AdminApiFactory adminApiFactory;
+ private final DynamicItem<AdminApiFactory> adminApiFactory;
private final int id;
private final URIish replicateURI;
private final Project.NameKey project;
@Inject
DeleteProjectTask(
- AdminApiFactory adminApiFactory,
+ DynamicItem<AdminApiFactory> adminApiFactory,
IdGenerator ig,
@Assisted URIish replicateURI,
@Assisted Project.NameKey project) {
@@ -48,7 +49,7 @@
@Override
public void run() {
- Optional<AdminApi> adminApi = adminApiFactory.create(replicateURI);
+ Optional<AdminApi> adminApi = adminApiFactory.get().create(replicateURI);
if (adminApi.isPresent()) {
adminApi.get().deleteProject(project);
return;
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 2677152..0a6d9d2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/Destination.java
@@ -173,7 +173,11 @@
install(new FactoryModuleBuilder().build(CreateProjectTask.Factory.class));
install(new FactoryModuleBuilder().build(DeleteProjectTask.Factory.class));
install(new FactoryModuleBuilder().build(UpdateHeadTask.Factory.class));
- bind(AdminApiFactory.class);
+
+ DynamicItem.itemOf(binder(), AdminApiFactory.class);
+ DynamicItem.bind(binder(), AdminApiFactory.class)
+ .to(AdminApiFactory.DefaultAdminApiFactory.class);
+
install(new FactoryModuleBuilder().build(GerritRestApi.Factory.class));
bind(CloseableHttpClient.class)
.toProvider(HttpClientProvider.class)
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/UpdateHeadTask.java b/src/main/java/com/googlesource/gerrit/plugins/replication/UpdateHeadTask.java
index db58f49..70452b4 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/replication/UpdateHeadTask.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/replication/UpdateHeadTask.java
@@ -16,6 +16,7 @@
import static com.googlesource.gerrit.plugins.replication.ReplicationQueue.repLog;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.ioutil.HexFormat;
import com.google.gerrit.server.util.IdGenerator;
@@ -25,7 +26,7 @@
import org.eclipse.jgit.transport.URIish;
public class UpdateHeadTask implements Runnable {
- private final AdminApiFactory adminApiFactory;
+ private final DynamicItem<AdminApiFactory> adminApiFactory;
private final int id;
private final URIish replicateURI;
private final Project.NameKey project;
@@ -37,7 +38,7 @@
@Inject
UpdateHeadTask(
- AdminApiFactory adminApiFactory,
+ DynamicItem<AdminApiFactory> adminApiFactory,
IdGenerator ig,
@Assisted URIish replicateURI,
@Assisted Project.NameKey project,
@@ -51,7 +52,7 @@
@Override
public void run() {
- Optional<AdminApi> adminApi = adminApiFactory.create(replicateURI);
+ Optional<AdminApi> adminApi = adminApiFactory.get().create(replicateURI);
if (adminApi.isPresent()) {
adminApi.get().updateHead(project, newHead);
return;
diff --git a/src/main/resources/Documentation/extension-point.md b/src/main/resources/Documentation/extension-point.md
index 345fd8f..076aded 100644
--- a/src/main/resources/Documentation/extension-point.md
+++ b/src/main/resources/Documentation/extension-point.md
@@ -1,7 +1,7 @@
@PLUGIN@ extension points
==============
-The replication plugin exposes an extension point to allow influencing the behaviour of the replication events from another plugin or a script.
+The replication plugin exposes an extension point to allow influencing its behaviour from another plugin or a script.
Extension points can be defined from the replication plugin only when it is loaded as [libModule](/config-gerrit.html#gerrit.installModule) and
implemented by another plugin by declaring a `provided` dependency from the replication plugin.
@@ -35,5 +35,19 @@
Example:
```
- DynamicItem.bind(binder(),ReplicationPushFilter.class).to(ReplicationPushFilterImpl.class);
+ DynamicItem.bind(binder(), ReplicationPushFilter.class).to(ReplicationPushFilterImpl.class);
+ ```
+
+* `com.googlesource.gerrit.plugins.replication.AdminApiFactory`
+
+ Create an instance of `AdminApi` for a given remote URL. The default implementation
+ provides API instances for local FS, remote SSH, and remote Gerrit.
+
+ Only one factory at a time is supported. The implementation needs to be bound as a
+ `DynamicItem`.
+
+ Example:
+
+ ```
+ DynamicItem.bind(binder(), AdminApiFactory.class).to(AdminApiFactoryImpl.class);
```
