bioinformatics-ua / dicoogle

Dicoogle - Open Source PACS
http://www.dicoogle.com/
GNU General Public License v3.0
450 stars 133 forks source link

Plugin set development improvements #115

Open Enet4 opened 9 years ago

Enet4 commented 9 years ago

These are a few ideas that I had some months ago. They will not be too complicated to implement and will certainly make plugins look cleaner, safer and easier to develop. I am leaving the details below so that it can be considered for integration in the future.

The plugin set is meant to be an aggregation of the plugins that we which to inject to the system, and right now it feels occluded by a multitude of methods that need to be implemented before things work. We can give our developers the confidence that they are in charge of their plugins while keeping things simple.

Proposal: Make the development of a plugin set based on annotations rather than implementing an interface.

The key concept here is that instead of having plugin getters (one method per interface type), the plugins will be declared as non-static attributes of the plugin set class. New annotations can be used to retrieve these instances, as well as to identify the plugin set. Only the default constructor and the shutdown event method (renamed here to onShutdown) will be required. If we move to Java 8 later on, onShutdown may become a default method with an empty implementation.

@DicooglePluginSetName(String value) should extend jspf's @PluginImplementation and is also used to define the name of the plugin set (getName is discarded). Then we can have one annotation for each kind of interface (@QueryPlugin, @IndexerPlugin, @StoragePlugin, @ServletPlugin, @RestletPlugin, ...). The interface DicooglePluginSet will still extend jspf's Plugin and should still be used in our plugin sets, otherwise loading the plugins may not work. No collections are created, all plugin types are optional, and the PluginBase helper class becomes obsolete.

@DicooglePluginSetName("RSI")
public class RSIPluginSet implements DicooglePluginSet {
  @QueryPlugin
  final RSIQueryInterface queryInt;
  @IndexerPlugin
  final RSIIndexerInterface indexer;

  // other plugins and plugin-scoped resources here, no getters

  public RSIPluginSet() {
    // initialize plugins and resources here, as usual
    this.queryInt = new RSIQueryInterface("args...");
    this.indexer = new RSIIndexerInterface("...");
  }

  @Override
  public void onShutdown() {
    // free resources here
  }
}

These are the reasons why I think we should support these changes:

  1. It's simpler and more intuitive. Picturing the diverse range of plugin developers, who might not be programming experts, I count this as a plus. It will also help expert developers at understanding what should and should not be done.
  2. It's safer. We have faced issues before with an implementation of a plugin set that was not thread-safe. With plugin getters gone, that no longer poses an issue. We can also make sure that we would always have the same plugins in a set, regardless of when they are retrieved. We know this is only performed on start-up, but some plugin developers may think that a reassignment is possible when it really isn't.
  3. It's extensible. Whether it is appreciated or not, the requirements for next-generation PACS archives might change along with Dicoogle's. With this approach, compatibility with previous plugins is not broken in the event that new plugin interfaces are introduced.
tmgodinho commented 9 years ago

Sounds like a very cool idea. I think it will help a lot to develop the plugins, especially new developers. Can't wait to use :)

bastiao commented 9 years ago

Nice improvement to PluginBase: https://github.com/bioinformatics-ua/dicoogle/blob/dev/sdk/src/main/java/pt/ua/dicoogle/sdk/PluginBase.java

We will analyse and discuss it in a meeting after release 2.0.

Good job.

Enet4 commented 9 years ago

Yes, good thing you've reminded me of PluginBase. I've updated the proposal to make it clear that both PluginSet and PluginBase become obsolete.

Enet4 commented 9 years ago

I have revised the proposal again in order to fix another flaw. We cannot load plugins if they do not implement a Plugin interface.

Enet4 commented 9 years ago

I have updated the proposal. After some rethinking, I figured that plugin-scoped resources still make sense.

Enet4 commented 3 years ago

While these ideas are interesting, it is too late for something like this to enter v3. Maybe Dicoogle 4? :)