Internal: Simplify custom repository type setup

Custom repository types are registered through the RepositoriesModule.
Later, when a specific repository type is used, the RespositoryModule
is installed, which in turn would spawn the module that was
registered for that repository type. However, a module is not needed
here. Each repository type has two associated classes, a Repository and
an IndexShardRepository.

This change makes the registration method for custom repository
types take both of these classes, instead of a module.

See #12783.

Backport of #12948.

core/src/main/java/org/elasticsearch/common/util/ExtensionPoint.java

@@ -131,13 +131,16 @@ public SelectedType(String name, Class<T> extensionClass) {
          * the settings object.
          *
          * @param binder       the binder to use
-         * @param settings     the settings to look up the key to find the implemetation to bind
+         * @param settings     the settings to look up the key to find the implementation to bind
          * @param settingsKey  the key to use with the settings
-         * @param defaultValue the default value if they settings doesn't contain the key
+         * @param defaultValue the default value if the settings do not contain the key, or null if there is no default
          * @return the actual bound type key
          */
         public String bindType(Binder binder, Settings settings, String settingsKey, String defaultValue) {
             final String type = settings.get(settingsKey, defaultValue);
+            if (type == null) {
+                throw new IllegalArgumentException("Missing setting [" + settingsKey + "]");
+            }
             final Class<? extends T> instance = getExtension(type);
             if (instance == null) {
                 throw new IllegalArgumentException("Unknown [" + this.name + "] type [" + type + "]");

core/src/main/java/org/elasticsearch/repositories/RepositoriesModule.java

@@ -19,44 +19,33 @@
 
 package org.elasticsearch.repositories;
 
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Maps;
 import org.elasticsearch.action.admin.cluster.snapshots.status.TransportNodesSnapshotsStatus;
 import org.elasticsearch.common.inject.AbstractModule;
-import org.elasticsearch.common.inject.Module;
+import org.elasticsearch.index.snapshots.IndexShardRepository;
+import org.elasticsearch.index.snapshots.blobstore.BlobStoreIndexShardRepository;
 import org.elasticsearch.repositories.fs.FsRepository;
-import org.elasticsearch.repositories.fs.FsRepositoryModule;
 import org.elasticsearch.repositories.uri.URLRepository;
-import org.elasticsearch.repositories.uri.URLRepositoryModule;
 import org.elasticsearch.snapshots.RestoreService;
-import org.elasticsearch.snapshots.SnapshotsService;
 import org.elasticsearch.snapshots.SnapshotShardsService;
-
-import java.util.Map;
+import org.elasticsearch.snapshots.SnapshotsService;
 
 /**
- * Module responsible for registering other repositories.
- * <p/>
- * Repositories implemented as plugins should implement {@code onModule(RepositoriesModule module)} method, in which
- * they should register repository using {@link #registerRepository(String, Class)} method.
+ * Sets up classes for Snapshot/Restore.
+ *
+ * Plugins can add custom repository types by calling {@link #registerRepository(String, Class, Class)}.
  */
 public class RepositoriesModule extends AbstractModule {
 
-    private Map<String, Class<? extends Module>> repositoryTypes = Maps.newHashMap();
+    private final RepositoryTypesRegistry repositoryTypes = new RepositoryTypesRegistry();
 
     public RepositoriesModule() {
-        registerRepository(FsRepository.TYPE, FsRepositoryModule.class);
-        registerRepository(URLRepository.TYPE, URLRepositoryModule.class);
+        registerRepository(FsRepository.TYPE, FsRepository.class, BlobStoreIndexShardRepository.class);
+        registerRepository(URLRepository.TYPE, URLRepository.class, BlobStoreIndexShardRepository.class);
     }
 
-    /**
-     * Registers a custom repository type name against a module.
-     *
-     * @param type   The type
-     * @param module The module
-     */
-    public void registerRepository(String type, Class<? extends Module> module) {
-        repositoryTypes.put(type, module);
+    /** Registers a custom repository type to the given {@link Repository} and {@link IndexShardRepository}. */
+    public void registerRepository(String type, Class<? extends Repository> repositoryType, Class<? extends IndexShardRepository> shardRepositoryType) {
+        repositoryTypes.registerRepository(type, repositoryType, shardRepositoryType);
     }
 
     @Override
@@ -66,6 +55,6 @@ protected void configure() {
         bind(SnapshotShardsService.class).asEagerSingleton();
         bind(TransportNodesSnapshotsStatus.class).asEagerSingleton();
         bind(RestoreService.class).asEagerSingleton();
-        bind(RepositoryTypesRegistry.class).toInstance(new RepositoryTypesRegistry(ImmutableMap.copyOf(repositoryTypes)));
+        bind(RepositoryTypesRegistry.class).toInstance(repositoryTypes);
     }
 }

core/src/main/java/org/elasticsearch/repositories/RepositoryModule.java

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.repositories;
 
-import com.google.common.collect.ImmutableList;
 import org.elasticsearch.common.inject.AbstractModule;
 import org.elasticsearch.common.inject.Module;
 import org.elasticsearch.common.inject.Modules;
@@ -29,12 +28,10 @@
 import java.util.Arrays;
 import java.util.Collections;
 
-import static org.elasticsearch.common.Strings.toCamelCase;
-
 /**
- * This module spawns specific repository module
+ * Binds repository classes for the specific repository type.
  */
-public class RepositoryModule extends AbstractModule implements SpawnModules {
+public class RepositoryModule extends AbstractModule {
 
     private RepositoryName repositoryName;
 
@@ -59,28 +56,12 @@ public RepositoryModule(RepositoryName repositoryName, Settings settings, Settin
         this.typesRegistry = typesRegistry;
     }
 
-    /**
-     * Returns repository module.
-     * <p/>
-     * First repository type is looked up in typesRegistry and if it's not found there, this module tries to
-     * load repository by it's class name.
-     *
-     * @return repository module
-     */
-    @Override
-    public Iterable<? extends Module> spawnModules() {
-        Class<? extends Module> repoModuleClass = typesRegistry.type(repositoryName.type());
-        if (repoModuleClass == null) {
-            throw new IllegalArgumentException("Could not find repository type [" + repositoryName.getType() + "] for repository [" + repositoryName.getName() + "]");
-        }
-        return Collections.unmodifiableList(Arrays.asList(Modules.createModule(repoModuleClass, globalSettings)));
-    }
-
     /**
      * {@inheritDoc}
      */
     @Override
     protected void configure() {
+        typesRegistry.bindType(binder(), repositoryName.type());
         bind(RepositorySettings.class).toInstance(new RepositorySettings(globalSettings, settings));
     }
 }

core/src/main/java/org/elasticsearch/repositories/RepositoryTypesRegistry.java

@@ -19,31 +19,29 @@
 
 package org.elasticsearch.repositories;
 
-import com.google.common.collect.ImmutableMap;
-import org.elasticsearch.common.inject.Module;
+import org.elasticsearch.common.inject.Binder;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.util.ExtensionPoint;
+import org.elasticsearch.index.snapshots.IndexShardRepository;
 
 /**
- * Map of registered repository types and associated with these types modules
+ * A mapping from type name to implementations of {@link Repository} and {@link IndexShardRepository}.
  */
 public class RepositoryTypesRegistry {
-    private final ImmutableMap<String, Class<? extends Module>> repositoryTypes;
+    // invariant: repositories and shardRepositories have the same keyset
+    private final ExtensionPoint.TypeExtensionPoint<Repository> repositoryTypes =
+        new ExtensionPoint.TypeExtensionPoint<>("repository", Repository.class);
+    private final ExtensionPoint.TypeExtensionPoint<IndexShardRepository> shardRepositoryTypes =
+        new ExtensionPoint.TypeExtensionPoint<>("index_repository", IndexShardRepository.class);
 
-    /**
-     * Creates new repository with given map of types
-     *
-     * @param repositoryTypes
-     */
-    public RepositoryTypesRegistry(ImmutableMap<String, Class<? extends Module>> repositoryTypes) {
-        this.repositoryTypes = repositoryTypes;
+    public void registerRepository(String name, Class<? extends Repository> repositoryType, Class<? extends IndexShardRepository> shardRepositoryType) {
+        repositoryTypes.registerExtension(name, repositoryType);
+        shardRepositoryTypes.registerExtension(name, shardRepositoryType);
     }
 
-    /**
-     * Returns repository module class for the given type
-     *
-     * @param type repository type
-     * @return repository module class or null if type is not found
-     */
-    public Class<? extends Module> type(String type) {
-        return repositoryTypes.get(type);
+    public void bindType(Binder binder, String type) {
+        Settings settings = Settings.builder().put("type", type).build();
+        repositoryTypes.bindType(binder, settings, "type", null);
+        shardRepositoryTypes.bindType(binder, settings, "type", null);
     }
 }

core/src/test/java/org/elasticsearch/snapshots/AbstractSnapshotIntegTestCase.java

@@ -35,7 +35,6 @@
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.repositories.RepositoriesService;
 import org.elasticsearch.snapshots.mockstore.MockRepository;
-import org.elasticsearch.snapshots.mockstore.MockRepositoryPlugin;
 import org.elasticsearch.test.ESIntegTestCase;
 
 import java.io.IOException;
@@ -58,10 +57,10 @@ public abstract class AbstractSnapshotIntegTestCase extends ESIntegTestCase {
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {
         return settingsBuilder().put(super.nodeSettings(nodeOrdinal))
-                // Rebalancing is causing some checks after restore to randomly fail
-                // due to https://github.com/elastic/elasticsearch/issues/9421
-                .put(EnableAllocationDecider.CLUSTER_ROUTING_REBALANCE_ENABLE, EnableAllocationDecider.Rebalance.NONE)
-                .extendArray("plugin.types", MockRepositoryPlugin.class.getName()).build();
+            // Rebalancing is causing some checks after restore to randomly fail
+            // due to https://github.com/elastic/elasticsearch/issues/9421
+            .put(EnableAllocationDecider.CLUSTER_ROUTING_REBALANCE_ENABLE, EnableAllocationDecider.Rebalance.NONE)
+            .extendArray("plugin.types", MockRepository.Plugin.class.getName()).build();
     }
 
     public static long getFailureCount(String repository) {

core/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java

@@ -23,7 +23,6 @@
 import com.carrotsearch.hppc.IntSet;
 import com.google.common.collect.ImmutableList;
 import com.google.common.util.concurrent.ListenableFuture;
-
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.action.ListenableActionFuture;
 import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryResponse;
@@ -40,8 +39,8 @@
 import org.elasticsearch.cluster.ClusterService;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.ProcessedClusterStateUpdateTask;
-import org.elasticsearch.cluster.metadata.MetaData.Custom;
 import org.elasticsearch.cluster.metadata.MetaData;
+import org.elasticsearch.cluster.metadata.MetaData.Custom;
 import org.elasticsearch.cluster.metadata.MetaDataIndexStateService;
 import org.elasticsearch.cluster.routing.allocation.decider.EnableAllocationDecider;
 import org.elasticsearch.common.Nullable;
@@ -63,11 +62,9 @@
 import org.elasticsearch.rest.RestResponse;
 import org.elasticsearch.rest.action.admin.cluster.repositories.get.RestGetRepositoriesAction;
 import org.elasticsearch.rest.action.admin.cluster.state.RestClusterStateAction;
-import org.elasticsearch.snapshots.mockstore.MockRepositoryModule;
-import org.elasticsearch.snapshots.mockstore.MockRepositoryPlugin;
+import org.elasticsearch.snapshots.mockstore.MockRepository;
 import org.elasticsearch.test.InternalTestCluster;
 import org.elasticsearch.test.rest.FakeRestRequest;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import java.io.IOException;
@@ -87,7 +84,15 @@
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertBlocked;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertThrows;
-import static org.hamcrest.Matchers.*;
+import static org.hamcrest.Matchers.allOf;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.lessThan;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
 
 /**
  */
@@ -609,7 +614,7 @@ public boolean clearData(String nodeName) {
     @Test
     public void registrationFailureTest() {
         logger.info("--> start first node");
-        internalCluster().startNode(settingsBuilder().put("plugin.types", MockRepositoryPlugin.class.getName()));
+        internalCluster().startNode(settingsBuilder().put("plugin.types", MockRepository.Plugin.class.getName()));
         logger.info("--> start second node");
         // Make sure the first node is elected as master
         internalCluster().startNode(settingsBuilder().put("node.master", false));
@@ -628,7 +633,7 @@ public void registrationFailureTest() {
 
     @Test
     public void testThatSensitiveRepositorySettingsAreNotExposed() throws Exception {
-        Settings nodeSettings = settingsBuilder().put("plugin.types", MockRepositoryPlugin.class.getName()).build();
+        Settings nodeSettings = settingsBuilder().put("plugin.types", MockRepository.Plugin.class.getName()).build();
         logger.info("--> start two nodes");
         internalCluster().startNodesAsync(2, nodeSettings).get();
         // Register mock repositories

core/src/test/java/org/elasticsearch/snapshots/RepositoriesIT.java

@@ -32,15 +32,12 @@
 import org.elasticsearch.common.unit.ByteSizeUnit;
 import org.elasticsearch.repositories.RepositoryException;
 import org.elasticsearch.repositories.RepositoryVerificationException;
-import org.elasticsearch.snapshots.mockstore.MockRepositoryModule;
-import org.elasticsearch.snapshots.mockstore.MockRepositoryPlugin;
 import org.elasticsearch.test.ESIntegTestCase;
 import org.junit.Test;
 
 import java.nio.file.Path;
 import java.util.List;
 
-import static org.elasticsearch.common.settings.Settings.settingsBuilder;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertThrows;
 import static org.hamcrest.Matchers.containsString;