Skip to content

Commit

Permalink
Plugins: Simplify Plugin API for constructing modules
Browse files Browse the repository at this point in the history 
The Plugin interface currently contains 6 different methods for
adding modules. Elasticsearch has 3 different levels of injectors,
and for each of those, there are two methods. The first takes no
arguments and returns a collection of class objects to construct. The
second takes a Settings object and returns a collection of module
objects already constructed. The settings argument is unecessary because
the plugin can already get the settings from its constructor. Removing
that, the only difference between the two versions is returning an
already constructed Module, or a module Class, and there is no reason
the plugin can't construct all their modules themselves.

This change reduces the plugin api down to just 3 methods for adding
modules. Each returns a Collection<Module>. It also removes the
processModule method, which was unnecessary since onModule
implementations fullfill the same requirement. And finally, it renames
the modules() method to nodeModules() so it is clear these are created
once for each node.
  • Loading branch information
@rjernst
rjernst committed Aug 18, 2015
1 parent 75ced05 commit 2bf8459
Show file tree
Hide file tree
Showing 27 changed files with 100 additions and 322 deletions.
2 changes: 1 addition & 1 deletion core/src/main/java/org/elasticsearch/client/transport/TransportClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ public void close() {
// ignore, might not be bounded
}

for (Class<? extends LifecycleComponent> plugin : injector.getInstance(PluginsService.class).services()) {
for (Class<? extends LifecycleComponent> plugin : injector.getInstance(PluginsService.class).nodeServices()) {
injector.getInstance(plugin).close();
}
try {
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/org/elasticsearch/node/Node.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ public Node start() {
// hack around dependency injection problem (for now...)
injector.getInstance(Discovery.class).setRoutingService(injector.getInstance(RoutingService.class));

for (Class<? extends LifecycleComponent> plugin : pluginsService.services()) {
for (Class<? extends LifecycleComponent> plugin : pluginsService.nodeServices()) {
injector.getInstance(plugin).start();
}

Expand Down Expand Up @@ -297,7 +297,7 @@ private Node stop() {
injector.getInstance(RestController.class).stop();
injector.getInstance(TransportService.class).stop();

for (Class<? extends LifecycleComponent> plugin : pluginsService.services()) {
for (Class<? extends LifecycleComponent> plugin : pluginsService.nodeServices()) {
injector.getInstance(plugin).stop();
}
// we should stop this last since it waits for resources to get released
Expand Down Expand Up @@ -364,7 +364,7 @@ public synchronized void close() {
stopWatch.stop().start("percolator_service");
injector.getInstance(PercolatorService.class).close();

for (Class<? extends LifecycleComponent> plugin : pluginsService.services()) {
for (Class<? extends LifecycleComponent> plugin : pluginsService.nodeServices()) {
stopWatch.stop().start("plugin(" + plugin.getName() + ")");
injector.getInstance(plugin).close();
}
Expand Down
57 changes: 12 additions & 45 deletions core/src/main/java/org/elasticsearch/plugins/AbstractPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,98 +19,65 @@

package org.elasticsearch.plugins;

import com.google.common.collect.ImmutableList;
import org.elasticsearch.common.component.LifecycleComponent;
import org.elasticsearch.common.inject.Module;
import org.elasticsearch.common.settings.Settings;

import java.io.Closeable;
import java.util.Collection;
import java.util.Collections;

/**
* A base class for a plugin.
* <p/>
* A plugin can be dynamically injected with {@link Module} by implementing <tt>onModule(AnyModule)</tt> method
* removing the need to override {@link #processModule(org.elasticsearch.common.inject.Module)} and check using
* instanceof.
* A base class for a plugin which returns no services or modules.
*/
public abstract class AbstractPlugin implements Plugin {

/**
* Defaults to return an empty list.
*/
@Override
public Collection<Class<? extends Module>> modules() {
return ImmutableList.of();
public Collection<Module> nodeModules() {
return Collections.emptyList();
}

/**
* Defaults to return an empty list.
*/
@Override
public Collection<Module> modules(Settings settings) {
return ImmutableList.of();
public Collection<Class<? extends LifecycleComponent>> nodeServices() {
return Collections.emptyList();
}

/**
* Defaults to return an empty list.
*/
@Override
public Collection<Class<? extends LifecycleComponent>> services() {
return ImmutableList.of();
}

/**
* Defaults to return an empty list.
*/
@Override
public Collection<Class<? extends Module>> indexModules() {
return ImmutableList.of();
}

/**
* Defaults to return an empty list.
*/
@Override
public Collection<Module> indexModules(Settings settings) {
return ImmutableList.of();
public Collection<Module> indexModules() {
return Collections.emptyList();
}

/**
* Defaults to return an empty list.
*/
@Override
public Collection<Class<? extends Closeable>> indexServices() {
return ImmutableList.of();
}

/**
* Defaults to return an empty list.
*/
@Override
public Collection<Class<? extends Module>> shardModules() {
return ImmutableList.of();
return Collections.emptyList();
}

/**
* Defaults to return an empty list.
*/
@Override
public Collection<Module> shardModules(Settings settings) {
return ImmutableList.of();
public Collection<Module> shardModules() {
return Collections.emptyList();
}

/**
* Defaults to return an empty list.
*/
@Override
public Collection<Class<? extends Closeable>> shardServices() {
return ImmutableList.of();
}

@Override
public void processModule(Module module) {
// nothing to do here
return Collections.emptyList();
}

@Override
Expand Down
14 changes: 1 addition & 13 deletions core/src/main/java/org/elasticsearch/plugins/IndexPluginsModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,12 @@

package org.elasticsearch.plugins;

import com.google.common.collect.Lists;
import org.elasticsearch.common.inject.AbstractModule;
import org.elasticsearch.common.inject.Module;
import org.elasticsearch.common.inject.PreProcessModule;
import org.elasticsearch.common.inject.SpawnModules;
import org.elasticsearch.common.settings.Settings;

import java.util.Collection;
import java.util.List;

import static org.elasticsearch.common.inject.Modules.createModule;

/**
*
*/
Expand All @@ -47,13 +41,7 @@ public IndexPluginsModule(Settings settings, PluginsService pluginsService) {

@Override
public Iterable<? extends Module> spawnModules() {
List<Module> modules = Lists.newArrayList();
Collection<Class<? extends Module>> modulesClasses = pluginsService.indexModules();
for (Class<? extends Module> moduleClass : modulesClasses) {
modules.add(createModule(moduleClass, settings));
}
modules.addAll(pluginsService.indexModules(settings));
return modules;
return pluginsService.indexModules();
}

@Override
Expand Down
38 changes: 7 additions & 31 deletions core/src/main/java/org/elasticsearch/plugins/Plugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@
/**
* An extension point allowing to plug in custom functionality.
* <p/>
* A plugin can be dynamically injected with {@link Module} by implementing <tt>onModule(AnyModule)</tt> method
* removing the need to override {@link #processModule(org.elasticsearch.common.inject.Module)} and check using
* instanceof.
* A plugin can be register custom extensions to builtin behavior by implementing <tt>onModule(AnyModule)</tt>,
* and registering the extension with the given module.
*/
public interface Plugin {

Expand All @@ -46,31 +45,19 @@ public interface Plugin {
String description();

/**
* Node level modules (classes, will automatically be created).
* Node level modules.
*/
Collection<Class<? extends Module>> modules();

/**
* Node level modules (instances)
*
* @param settings The node level settings.
*/
Collection<? extends Module> modules(Settings settings);
Collection<Module> nodeModules();

/**
* Node level services that will be automatically started/stopped/closed.
*/
Collection<Class<? extends LifecycleComponent>> services();

/**
* Per index modules.
*/
Collection<Class<? extends Module>> indexModules();
Collection<Class<? extends LifecycleComponent>> nodeServices();

/**
* Per index modules.
*/
Collection<? extends Module> indexModules(Settings settings);
Collection<Module> indexModules();

/**
* Per index services that will be automatically closed.
Expand All @@ -80,24 +67,13 @@ public interface Plugin {
/**
* Per index shard module.
*/
Collection<Class<? extends Module>> shardModules();

/**
* Per index shard module.
*/
Collection<? extends Module> shardModules(Settings settings);
Collection<Module> shardModules();

/**
* Per index shard service that will be automatically closed.
*/
Collection<Class<? extends Closeable>> shardServices();

/**
* Process a specific module. Note, its simpler to implement a custom <tt>onModule(AnyModule module)</tt>
* method, which will be automatically be called by the relevant type.
*/
void processModule(Module module);

/**
* Additional node settings loaded by the plugin. Note that settings that are explicit in the nodes settings can't be
* overwritten with the additional settings. These settings added if they don't exist.
Expand Down
14 changes: 1 addition & 13 deletions core/src/main/java/org/elasticsearch/plugins/PluginsModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,12 @@

package org.elasticsearch.plugins;

import com.google.common.collect.Lists;
import org.elasticsearch.common.inject.AbstractModule;
import org.elasticsearch.common.inject.Module;
import org.elasticsearch.common.inject.PreProcessModule;
import org.elasticsearch.common.inject.SpawnModules;
import org.elasticsearch.common.settings.Settings;

import java.util.Collection;
import java.util.List;

import static org.elasticsearch.common.inject.Modules.createModule;

/**
*
*/
Expand All @@ -47,13 +41,7 @@ public PluginsModule(Settings settings, PluginsService pluginsService) {

@Override
public Iterable<? extends Module> spawnModules() {
List<Module> modules = Lists.newArrayList();
Collection<Class<? extends Module>> modulesClasses = pluginsService.modules();
for (Class<? extends Module> moduleClass : modulesClasses) {
modules.add(createModule(moduleClass, settings));
}
modules.addAll(pluginsService.modules(settings));
return modules;
return pluginsService.nodeModules();
}

@Override
Expand Down
41 changes: 8 additions & 33 deletions core/src/main/java/org/elasticsearch/plugins/PluginsService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ public void processModules(Iterable<Module> modules) {

public void processModule(Module module) {
for (Tuple<PluginInfo, Plugin> plugin : plugins()) {
plugin.v2().processModule(module);
// see if there are onModule references
List<OnModuleReference> references = onModuleReferences.get(plugin.v2());
if (references != null) {
Expand All @@ -209,42 +208,26 @@ public Settings updatedSettings() {
return builder.put(this.settings).build();
}

public Collection<Class<? extends Module>> modules() {
List<Class<? extends Module>> modules = new ArrayList<>();
for (Tuple<PluginInfo, Plugin> plugin : plugins) {
modules.addAll(plugin.v2().modules());
}
return modules;
}

public Collection<Module> modules(Settings settings) {
public Collection<Module> nodeModules() {
List<Module> modules = new ArrayList<>();
for (Tuple<PluginInfo, Plugin> plugin : plugins) {
modules.addAll(plugin.v2().modules(settings));
modules.addAll(plugin.v2().nodeModules());
}
return modules;
}

public Collection<Class<? extends LifecycleComponent>> services() {
public Collection<Class<? extends LifecycleComponent>> nodeServices() {
List<Class<? extends LifecycleComponent>> services = new ArrayList<>();
for (Tuple<PluginInfo, Plugin> plugin : plugins) {
services.addAll(plugin.v2().services());
services.addAll(plugin.v2().nodeServices());
}
return services;
}

public Collection<Class<? extends Module>> indexModules() {
List<Class<? extends Module>> modules = new ArrayList<>();
for (Tuple<PluginInfo, Plugin> plugin : plugins) {
modules.addAll(plugin.v2().indexModules());
}
return modules;
}

public Collection<Module> indexModules(Settings settings) {
public Collection<Module> indexModules() {
List<Module> modules = new ArrayList<>();
for (Tuple<PluginInfo, Plugin> plugin : plugins) {
modules.addAll(plugin.v2().indexModules(settings));
modules.addAll(plugin.v2().indexModules());
}
return modules;
}
Expand All @@ -257,18 +240,10 @@ public Collection<Class<? extends Closeable>> indexServices() {
return services;
}

public Collection<Class<? extends Module>> shardModules() {
List<Class<? extends Module>> modules = new ArrayList<>();
for (Tuple<PluginInfo, Plugin> plugin : plugins) {
modules.addAll(plugin.v2().shardModules());
}
return modules;
}

public Collection<Module> shardModules(Settings settings) {
public Collection<Module> shardModules() {
List<Module> modules = new ArrayList<>();
for (Tuple<PluginInfo, Plugin> plugin : plugins) {
modules.addAll(plugin.v2().shardModules(settings));
modules.addAll(plugin.v2().shardModules());
}
return modules;
}
Expand Down

0 comments on commit 2bf8459

Please sign in to comment.