Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify Plugin API for constructing modules #12952

Merged
merged 6 commits into from
Aug 18, 2015
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
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
92 changes: 3 additions & 89 deletions core/src/main/java/org/elasticsearch/plugins/AbstractPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,103 +19,17 @@

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();
}

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

/**
* 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();
}

/**
* 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();
}

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

/**
* 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
}

@Override
public Settings additionalSettings() {
return Settings.Builder.EMPTY_SETTINGS;
}
public abstract class AbstractPlugin extends Plugin {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really need this class?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops, meant to remove that.


}
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(settings);
}

@Override
Expand Down
65 changes: 28 additions & 37 deletions core/src/main/java/org/elasticsearch/plugins/Plugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,82 +25,73 @@

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

/**
* 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 {
public abstract class Plugin {

/**
* The name of the plugin.
*/
String name();
public abstract String name();

/**
* The description of the plugin.
*/
String description();
public abstract 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);
public Collection<Module> nodeModules() {
return Collections.emptyList();
}

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

/**
* Per index modules.
*/
Collection<Class<? extends Module>> indexModules();
public Collection<Class<? extends LifecycleComponent>> nodeServices() {
return Collections.emptyList();
}

/**
* Per index modules.
*/
Collection<? extends Module> indexModules(Settings settings);
public Collection<Module> indexModules(Settings indexSettings) {
return Collections.emptyList();
}

/**
* Per index services that will be automatically closed.
*/
Collection<Class<? extends Closeable>> indexServices();
public Collection<Class<? extends Closeable>> indexServices() {
return Collections.emptyList();
}

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

/**
* Per index shard module.
*/
Collection<? extends Module> shardModules(Settings settings);
public Collection<Module> shardModules(Settings indexSettings) {
return Collections.emptyList();
}

/**
* 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);
public Collection<Class<? extends Closeable>> shardServices() {
return Collections.emptyList();
}

/**
* 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.
*/
Settings additionalSettings();
public Settings additionalSettings() {
return Settings.Builder.EMPTY_SETTINGS;
}
}
14 changes: 1 addition & 13 deletions core/src/main/java/org/elasticsearch/plugins/PluginsModule.java
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