From 3e0b14a16db657163b1d0f90632760d421f45298 Mon Sep 17 00:00:00 2001 From: Curtis Rueden Date: Fri, 1 Aug 2014 10:44:55 -0500 Subject: [PATCH 1/7] Gateway: remove get(String) method Obviously, this is a breaking API change. The reason this method is being removed is that we would like RichPlugin to implement BasicDetails, which has its own get(String) method with an incompatible return type. So the Gateway method ultimately has to go. Upon consideration, the Gateway#get(String) method was not very useful anyway: what would you do with the (weakly typed) Service once you got it back? You'd have to cast it to make any calls to its service API specifically, which would necessitate a compile-time dependency on the service's artifact, which would obviate the need to call Gateway#get(String) in the first place -- in that case, you could just call Gateway#get(Class) instead. I suppose there is a _rare_ case where you'd want to check for a service at runtime only, without an explicit compile-time dependency, and then invoke one or more of its methods via reflection. But this is certainly not an encouraged practice, so let's not have the Gateway make it easy. It is still easy enough to do anyway: Service s = myGateway.context().service("MyService"); --- pom.xml | 2 +- src/main/java/org/scijava/AbstractGateway.java | 5 ----- src/main/java/org/scijava/Gateway.java | 12 ------------ 3 files changed, 1 insertion(+), 18 deletions(-) diff --git a/pom.xml b/pom.xml index 90b37f502..d4540f538 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ scijava-common - 2.28.1-SNAPSHOT + 3.0.0-SNAPSHOT SciJava Common SciJava Common is a shared library for SciJava software. It provides a plugin framework, with an extensible mechanism for service discovery, backed by its own annotation processor, so that plugins can be loaded dynamically. It is used by both ImageJ and SCIFIO. diff --git a/src/main/java/org/scijava/AbstractGateway.java b/src/main/java/org/scijava/AbstractGateway.java index f08054a11..c620d70e3 100644 --- a/src/main/java/org/scijava/AbstractGateway.java +++ b/src/main/java/org/scijava/AbstractGateway.java @@ -93,11 +93,6 @@ public S get(final Class serviceClass) { return context().service(serviceClass); } - @Override - public Service get(final String serviceClassName) { - return context().service(serviceClassName); - } - // -- Gateway methods - services -- @Override diff --git a/src/main/java/org/scijava/Gateway.java b/src/main/java/org/scijava/Gateway.java index 199060a1c..7e8583400 100644 --- a/src/main/java/org/scijava/Gateway.java +++ b/src/main/java/org/scijava/Gateway.java @@ -130,18 +130,6 @@ public interface Gateway extends RichPlugin, Versioned { */ S get(Class serviceClass); - /** - * Returns an implementation of the {@link Service} with the given class name, - * if it exists in the underlying {@link Context}. - * - * @param serviceClassName name of the requested {@link Service} - * @return The singleton instance of the requested {@link Service} - * @throws NullContextException if the application context is not set. - * @throws NoSuchServiceException if there is no service matching - * {@code serviceClassName}. - */ - Service get(final String serviceClassName); - // -- Gateway methods - services -- AppEventService appEvent(); From 77c53e58b61e183d092e4f040c8402c74cb5c96f Mon Sep 17 00:00:00 2001 From: Curtis Rueden Date: Fri, 1 Aug 2014 10:17:54 -0500 Subject: [PATCH 2/7] Make RichPlugin implement more useful interfaces Now, all rich plugins are Identifiable, Locatable, Versioned, and know their own BasicDetails (which comes from their linked PluginInfo). This makes it easier to enable tracking of usage statistics relating to a particular kind of plugin. --- .../scijava/plugin/AbstractRichPlugin.java | 73 +++++++++++++++++++ .../java/org/scijava/plugin/RichPlugin.java | 8 +- 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/scijava/plugin/AbstractRichPlugin.java b/src/main/java/org/scijava/plugin/AbstractRichPlugin.java index fc3d36bb9..3b5a09757 100644 --- a/src/main/java/org/scijava/plugin/AbstractRichPlugin.java +++ b/src/main/java/org/scijava/plugin/AbstractRichPlugin.java @@ -31,10 +31,13 @@ package org.scijava.plugin; +import java.net.URL; + import org.scijava.AbstractContextual; import org.scijava.Prioritized; import org.scijava.Priority; import org.scijava.util.ClassUtils; +import org.scijava.util.Manifest; /** * Abstract base class for {@link RichPlugin} implementations. @@ -83,6 +86,76 @@ public void setInfo(final PluginInfo info) { this.info = info; } + // -- Identifiable methods -- + + @Override + public String getIdentifier() { + return "plugin:" + getClass().getName(); + } + + // -- Locatable methods -- + + @Override + public String getLocation() { + final URL location = ClassUtils.getLocation(getClass()); + return location == null ? null : location.toExternalForm(); + } + + // -- Versioned methods -- + + @Override + public String getVersion() { + final Manifest m = Manifest.getManifest(getClass()); + return m == null ? null : m.getImplementationVersion(); + } + + // -- BasicDetails methods -- + + @Override + public String getName() { + return getInfo().getName(); + } + + @Override + public String getLabel() { + return getInfo().getLabel(); + } + + @Override + public String getDescription() { + return getInfo().getDescription(); + } + + @Override + public boolean is(String key) { + return getInfo().is(key); + } + + @Override + public String get(String key) { + return getInfo().get(key); + } + + @Override + public void setName(String name) { + throw new UnsupportedOperationException(); + } + + @Override + public void setLabel(String label) { + throw new UnsupportedOperationException(); + } + + @Override + public void setDescription(String description) { + throw new UnsupportedOperationException(); + } + + @Override + public void set(String key, String value) { + throw new UnsupportedOperationException(); + } + // -- Comparable methods -- @Override diff --git a/src/main/java/org/scijava/plugin/RichPlugin.java b/src/main/java/org/scijava/plugin/RichPlugin.java index b20324121..81c37c9c2 100644 --- a/src/main/java/org/scijava/plugin/RichPlugin.java +++ b/src/main/java/org/scijava/plugin/RichPlugin.java @@ -31,8 +31,12 @@ package org.scijava.plugin; +import org.scijava.BasicDetails; import org.scijava.Contextual; +import org.scijava.Identifiable; +import org.scijava.Locatable; import org.scijava.Prioritized; +import org.scijava.Versioned; /** * Base interface for {@link Contextual}, {@link Prioritized} plugins that @@ -42,8 +46,8 @@ * * @author Curtis Rueden */ -public interface RichPlugin extends Contextual, Prioritized, HasPluginInfo, - SciJavaPlugin +public interface RichPlugin extends SciJavaPlugin, Contextual, Prioritized, + HasPluginInfo, Identifiable, Locatable, Versioned, BasicDetails { // NB: Marker interface. } From 76853c7bb2fbf12dfd21d81221d36cf275aa232a Mon Sep 17 00:00:00 2001 From: Curtis Rueden Date: Fri, 1 Aug 2014 10:35:11 -0500 Subject: [PATCH 3/7] AbstractPTService: add API to track plugin usage This method makes it easy for subclasses to register usage of their plugins in whatever way they see fit. --- .../java/org/scijava/plugin/AbstractPTService.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/main/java/org/scijava/plugin/AbstractPTService.java b/src/main/java/org/scijava/plugin/AbstractPTService.java index b94f713cd..3b26ddfb9 100644 --- a/src/main/java/org/scijava/plugin/AbstractPTService.java +++ b/src/main/java/org/scijava/plugin/AbstractPTService.java @@ -34,6 +34,7 @@ import java.util.List; import org.scijava.service.AbstractService; +import org.scijava.usage.UsageService; /** * Abstract base class for {@link PTService}s. @@ -48,6 +49,9 @@ public abstract class AbstractPTService extends @Parameter private PluginService pluginService; + @Parameter(required = false) + private UsageService usageService; + // -- PTService methods -- @Override @@ -60,4 +64,12 @@ public List> getPlugins() { return pluginService.getPluginsOfType(getPluginType()); } + // -- Internal methods -- + + /** Records the usage of a plugin. */ + protected void recordUsage(final Object plugin) { + if (usageService == null) return; + usageService.increment(plugin); + } + } From 27c226697c6c1ccb2dab068a43a7ce2d6ca64f5f Mon Sep 17 00:00:00 2001 From: Curtis Rueden Date: Fri, 1 Aug 2014 10:36:14 -0500 Subject: [PATCH 4/7] Record usage of all Handler and Wrapper plugins It is very straightforward when a Handler or Wrapper plugin is being "used": the service returns the appropriate and specifically requested Handler or Wrapper instance for the task at hand. So let's count it in our usage statistics when that happens! --- src/main/java/org/scijava/plugin/AbstractHandlerService.java | 5 ++++- src/main/java/org/scijava/plugin/AbstractWrapperService.java | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/scijava/plugin/AbstractHandlerService.java b/src/main/java/org/scijava/plugin/AbstractHandlerService.java index 92b58f947..6537654c6 100644 --- a/src/main/java/org/scijava/plugin/AbstractHandlerService.java +++ b/src/main/java/org/scijava/plugin/AbstractHandlerService.java @@ -47,7 +47,10 @@ public abstract class AbstractHandlerService> @Override public PT getHandler(final DT data) { for (final PT handler : getInstances()) { - if (handler.supports(data)) return handler; + if (handler.supports(data)) { + recordUsage(handler); + return handler; + } } return null; } diff --git a/src/main/java/org/scijava/plugin/AbstractWrapperService.java b/src/main/java/org/scijava/plugin/AbstractWrapperService.java index c9f1ea0cd..ec25eec77 100644 --- a/src/main/java/org/scijava/plugin/AbstractWrapperService.java +++ b/src/main/java/org/scijava/plugin/AbstractWrapperService.java @@ -59,6 +59,7 @@ public WrapperPlugin create(final D data) { @SuppressWarnings("unchecked") final WrapperPlugin typedInstance = (WrapperPlugin) instance; typedInstance.set(data); + recordUsage(typedInstance); return typedInstance; } From 7b45e70dfc616f5423b10d5238458b6a098b2988 Mon Sep 17 00:00:00 2001 From: Curtis Rueden Date: Fri, 1 Aug 2014 10:48:57 -0500 Subject: [PATCH 5/7] Gateway: remove superfluous Versioned inheritance All RichPlugins are now versioned. --- src/main/java/org/scijava/Gateway.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/main/java/org/scijava/Gateway.java b/src/main/java/org/scijava/Gateway.java index 7e8583400..ece250785 100644 --- a/src/main/java/org/scijava/Gateway.java +++ b/src/main/java/org/scijava/Gateway.java @@ -117,7 +117,7 @@ * @author Mark Hiner * @author Curtis Rueden */ -public interface Gateway extends RichPlugin, Versioned { +public interface Gateway extends RichPlugin { /** * Returns an implementation of the requested {@link Service}, if it exists in @@ -193,10 +193,4 @@ public interface Gateway extends RichPlugin, Versioned { /** @see org.scijava.app.App#getInfo(boolean) */ String getInfo(boolean mem); - // -- Versioned methods -- - - /** @see org.scijava.app.App#getVersion() */ - @Override - String getVersion(); - } From a0fba9a82f30f36ffea9e258910e8ddb2a1dd6b8 Mon Sep 17 00:00:00 2001 From: Curtis Rueden Date: Fri, 1 Aug 2014 10:49:50 -0500 Subject: [PATCH 6/7] Clean up method ordering and section headers See: http://imagej.net/Coding_style#Ordering_of_code_blocks --- src/main/java/org/scijava/AbstractGateway.java | 10 ++++++---- src/main/java/org/scijava/app/AbstractApp.java | 14 +++++++++----- src/main/java/org/scijava/script/ScriptInfo.java | 2 ++ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/scijava/AbstractGateway.java b/src/main/java/org/scijava/AbstractGateway.java index c620d70e3..e9fcece81 100644 --- a/src/main/java/org/scijava/AbstractGateway.java +++ b/src/main/java/org/scijava/AbstractGateway.java @@ -241,13 +241,15 @@ public String getTitle() { } @Override - public String getVersion() { - return getApp().getVersion(); + public String getInfo(final boolean mem) { + return getApp().getInfo(mem); } + // -- Versioned methods -- + @Override - public String getInfo(final boolean mem) { - return getApp().getInfo(mem); + public String getVersion() { + return getApp().getVersion(); } } diff --git a/src/main/java/org/scijava/app/AbstractApp.java b/src/main/java/org/scijava/app/AbstractApp.java index 5b6e22a3c..034452bf9 100644 --- a/src/main/java/org/scijava/app/AbstractApp.java +++ b/src/main/java/org/scijava/app/AbstractApp.java @@ -51,16 +51,13 @@ public abstract class AbstractApp extends AbstractRichPlugin implements App { /** JAR manifest with metadata about the application. */ private Manifest manifest; + // -- App methods -- + @Override public String getTitle() { return getInfo().getName(); } - @Override - public String getVersion() { - return getPOM() == null ? "Unknown" : getPOM().getVersion(); - } - @Override public POM getPOM() { if (pom == null) { @@ -108,4 +105,11 @@ public File getBaseDirectory() { return AppUtils.getBaseDirectory(getSystemProperty(), getClass(), null); } + // -- Versioned methods -- + + @Override + public String getVersion() { + return getPOM() == null ? "Unknown" : getPOM().getVersion(); + } + } diff --git a/src/main/java/org/scijava/script/ScriptInfo.java b/src/main/java/org/scijava/script/ScriptInfo.java index 733b7fe23..86f4ea620 100644 --- a/src/main/java/org/scijava/script/ScriptInfo.java +++ b/src/main/java/org/scijava/script/ScriptInfo.java @@ -292,6 +292,8 @@ public String getLocation() { return new File(path).toURI().normalize().toString(); } + // -- Versioned methods -- + @Override public String getVersion() { final File file = new File(path); From 54f24ed1a28db441a891114e3231512690b74f46 Mon Sep 17 00:00:00 2001 From: Curtis Rueden Date: Wed, 6 Aug 2014 21:02:53 -0500 Subject: [PATCH 7/7] AbstractRichPlugin: improve sorting behavior It now more closely resembles AbstractUIDetails. However, we may want to factor this sort of logic into a shared method somewhere, since it currently is not very DRY. --- .../java/org/scijava/plugin/AbstractRichPlugin.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/scijava/plugin/AbstractRichPlugin.java b/src/main/java/org/scijava/plugin/AbstractRichPlugin.java index 3b5a09757..0a5436d25 100644 --- a/src/main/java/org/scijava/plugin/AbstractRichPlugin.java +++ b/src/main/java/org/scijava/plugin/AbstractRichPlugin.java @@ -34,10 +34,12 @@ import java.net.URL; import org.scijava.AbstractContextual; +import org.scijava.BasicDetails; import org.scijava.Prioritized; import org.scijava.Priority; import org.scijava.util.ClassUtils; import org.scijava.util.Manifest; +import org.scijava.util.MiscUtils; /** * Abstract base class for {@link RichPlugin} implementations. @@ -167,7 +169,16 @@ public int compareTo(final Prioritized that) { if (priorityCompare != 0) return priorityCompare; // compare classes - return ClassUtils.compare(getClass(), that.getClass()); + final int classCompare = ClassUtils.compare(getClass(), that.getClass()); + if (classCompare != 0) return classCompare; + + if (!(that instanceof BasicDetails)) return 1; + final BasicDetails basicDetails = (BasicDetails) that; + + // compare names + final String thisName = getName(); + final String thatName = basicDetails.getName(); + return MiscUtils.compare(thisName, thatName); } }