Add default behavior for service accessors

ctrueden · ctrueden · commit 2cb16a62fbdd · 2016-08-26T03:20:03.000-05:00
It is easy and reasonable to fall back to asking the context directly for the service. However, there is a downside: services which do not override these methods to return their injected field values may have subtle bugs relating to service dependencies and initialization order. For example, InputService has a method EventService, which is intended to never return null. In other words: it is intended that InputService implementations (at least by default) _depend_ on the EventService. If you write: Context ctx = new Context(InputService.class); When DefaultInputService is on the classpath, it will be instantiated, its @parameter EventService field will be noted, and the highest-priority concrete EventService implementation (i.e., DefaultEventService) will then be recursively instantiated and initialized, and so forth. So you will end up with a Context containing a DefaultInputService _and_ all of its service dependencies. But if DefaultInputService neglects to declare an @parameter EventService, the way the service loader is coded right now, the EventService dependency will not be noticed, and the Context will contain only a DefaultInputService, whose eventService() method will return null, resulting in NPE when calling other methods which use it. There is one vital reason to define these default method behaviors anyway, though: to avoid breaking SPI compatibility with downstream components. For example, the net.imagej:imagej-legacy component implements a LegacyConsoleService (ConsoleService is a SingletonService), which was forced to implement the new objectService() method when upgrading its version of scijava-common. Unfortunately, the ImageJ Updater (net.imagej:imagej-updater) has a serious design flaw whereby it updates itself and its dependencies (including scijava-common) first, without updating any downstream libraries (e.g., imagej-legacy). So we end up with a situation where scijava-common is at 2.58.2, after the addition of the newly required objectService() method, while imagej-legacy remains at its old version which does not implement that method. And the Context fails to spin up. Adding these default method behaviors is a simple way around the issue. We should consider whether to remove these default implementations in SJC3. And we should definitely consider how best to fix SciJava Common so that downstream JAR skew cannot taint the Context startup so easily. But for now, it is safest to provide these methods, in case of JAR skew.
diff --git a/src/main/java/org/scijava/command/CommandService.java b/src/main/java/org/scijava/command/CommandService.java
@@ -64,9 +64,13 @@
  */
 public interface CommandService extends PTService<Command>, SciJavaService {
 
-	EventService eventService();
+	default EventService eventService() {
+		return context().getService(EventService.class);
+	}
 
-	ModuleService moduleService();
+	default ModuleService moduleService() {
+		return context().getService(ModuleService.class);
+	}
 
 	/** Gets the list of all available {@link Command}s). */
 	List<CommandInfo> getCommands();
diff --git a/src/main/java/org/scijava/display/DisplayService.java b/src/main/java/org/scijava/display/DisplayService.java
@@ -50,11 +50,17 @@
  */
 public interface DisplayService extends SciJavaService {
 
-	EventService eventService();
+	default EventService eventService() {
+		return context().getService(EventService.class);
+	}
 
-	ObjectService objectService();
+	default ObjectService objectService() {
+		return context().getService(ObjectService.class);
+	}
 
-	PluginService pluginService();
+	default PluginService pluginService() {
+		return context().getService(PluginService.class);
+	}
 
 	/** Gets the currently active display (of any Display type). */
 	Display<?> getActiveDisplay();
diff --git a/src/main/java/org/scijava/input/InputService.java b/src/main/java/org/scijava/input/InputService.java
@@ -45,7 +45,9 @@
  */
 public interface InputService extends SciJavaService {
 
-	EventService eventService();
+	default EventService eventService() {
+		return context().getService(EventService.class);
+	}
 
 	InputModifiers getModifiers();
 
diff --git a/src/main/java/org/scijava/object/ObjectService.java b/src/main/java/org/scijava/object/ObjectService.java
@@ -43,7 +43,9 @@
  */
 public interface ObjectService extends SciJavaService {
 
-	EventService eventService();
+	default EventService eventService() {
+		return context().getService(EventService.class);
+	}
 
 	/** Gets the index of available objects. */
 	ObjectIndex<Object> getIndex();
diff --git a/src/main/java/org/scijava/platform/PlatformService.java b/src/main/java/org/scijava/platform/PlatformService.java
@@ -52,9 +52,13 @@ public interface PlatformService extends SingletonService<Platform>,
 	SciJavaService
 {
 
-	EventService eventService();
+	default EventService eventService() {
+		return context().getService(EventService.class);
+	}
 
-	CommandService commandService();
+	default CommandService commandService() {
+		return context().getService(CommandService.class);
+	}
 
 	/** Gets the platform handlers applicable to this platform. */
 	List<Platform> getTargetPlatforms();
diff --git a/src/main/java/org/scijava/plugin/PTService.java b/src/main/java/org/scijava/plugin/PTService.java
@@ -87,7 +87,9 @@ public interface PTService<PT extends SciJavaPlugin> extends Service {
 	 * Gets the service responsible for discovering and managing this service's
 	 * plugins.
 	 */
-	PluginService pluginService();
+	default PluginService pluginService() {
+		return context().getService(PluginService.class);
+	}
 
 	/** Gets the type of plugins managed by this service. */
 	Class<PT> getPluginType();
diff --git a/src/main/java/org/scijava/plugin/SingletonService.java b/src/main/java/org/scijava/plugin/SingletonService.java
@@ -57,7 +57,9 @@ public interface SingletonService<PT extends SingletonPlugin> extends
 {
 
 	/** Gets the {@link ObjectService} upon which this service depends. */
-	ObjectService objectService();
+	default ObjectService objectService() {
+		return context().getService(ObjectService.class);
+	}
 
 	/**
 	 * Gets the list of plugin instances. There will be one singleton instance for

Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,9 @@ public interface SingletonService<PT extends SingletonPlugin> extends`
`57`	`57`	`{`
`58`	`58`
`59`	`59`	`/** Gets the {@link ObjectService} upon which this service depends. */`
`60`		`- ObjectService objectService();`
	`60`	`+ default ObjectService objectService() {`
	`61`	`+ return context().getService(ObjectService.class);`
	`62`	`+ }`
`61`	`63`
`62`	`64`	`/**`
`63`	`65`	`* Gets the list of plugin instances. There will be one singleton instance for`