Consolidate watcher interfaces via JUnitWatcher marker (#63)

Resolves #61
This set of revisions consolidates the declaration of JUnit Foundation service providers into a single configuration file. Support for the previous file-per-interface approach is retained to provide backward compatibility.
One significant benefit to switching from ServiceLoaders to pre-initialized unmodifiable lists is that I no longer need to synchronize my iterators. The code is therefore smaller and less complicated.
Another benefit is that we no longer end up with multiple instances of each service provider that implements multiple interfaces. Previously, a separate instance would be created for each implemented interface.

Author: sbabcoc

README.md

@@ -229,17 +229,21 @@ To enable notifications in the native test runner of IDEs like Eclipse or IDEA,
 
 To provide reliable, consistent behavior regardless of execution environment, **JUnit Foundation** notification subscribers are registered through the standard Java **ServiceLoader** mechanism. To attach **JUnit Foundation** watchers and standard JUnit run listeners to your tests, declare them in **ServiceLoader** [provider configuration files](https://docs.oracle.com/javase/tutorial/ext/basics/spi.html#register-service-providers) in a **_META-INF/services/_** folder of your project resources:
 
-###### com.nordstrom.automation.junit.MethodWatcher
+###### com.nordstrom.automation.junit.JUnitWatcher
 ```
-com.mycompany.example.MyWatcher
+# implements com.nordstrom.automation.junit.MethodWatcher
+com.mycompany.example.MyMethodWatcher
+# implements com.nordstrom.automation.junit.ShutdownListener
+com.mycompany.example.MyShutdownListener
 ```
 
 ###### org.junit.runner.notification.RunListener
 ```
-com.mycompany.example.MyListener
+# implements org.junit.runner.notification.RunListener
+com.mycompany.example.MyRunListener
 ```
 
-The preceding **ServiceLoader** provider configuration files declare a **JUnit Foundation** [MethodWatcher](https://github.com/Nordstrom/JUnit-Foundation/blob/master/src/main/java/com/nordstrom/automation/junit/MethodWatcher.java) and a standard JUnit [RunListener](https://github.com/junit-team/junit4/blob/41d44734f41aba0cf6ba5a11ff5d32ffed155027/src/main/java/org/junit/runner/notification/RunListener.java).
+The preceding **ServiceLoader** provider configuration files declare a **JUnit Foundation** [MethodWatcher](https://github.com/Nordstrom/JUnit-Foundation/blob/master/src/main/java/com/nordstrom/automation/junit/MethodWatcher.java), a **JUnit Foundation** [ShutdownListener](https://github.com/Nordstrom/JUnit-Foundation/blob/master/src/main/java/com/nordstrom/automation/junit/ShutdownListener.java), and a standard **JUnit** [RunListener](https://github.com/junit-team/junit4/blob/41d44734f41aba0cf6ba5a11ff5d32ffed155027/src/main/java/org/junit/runner/notification/RunListener.java). Note that all **JUnit Foundation** watcher/listener service providers are declared in a single common configuration file to eliminate the need to declare classes implementing multiple interfaces in several different configuration files.
 
 ### Defined Service Provider Interfaces
 
@@ -443,7 +447,7 @@ Failed attempts of tests that are selected for retry are tallied as ignored test
 **JUnit** provides a run listener feature, but this operates most readily on a per-class basis. The method for attaching these run listeners also imposes structural and operational constraints on **JUnit** projects, and the configuration required to register for end-of-suite notifications necessitates hard-coding the composition of the suite. All of these factors make run listeners unattractive or ineffectual for final cleanup operations.
 
 **JUnit Foundation** enables you to declare shutdown listeners in a service loader configuration file.  
-**_META-INF/services/com.nordstrom.automation.junit.ShutdownListener_** is the service loader shutdown listener configuration file. By default, this file is absent. To add managed listeners, create this file and add the fully-qualified names of their classes, one line per item. When it loads, the **JUnit Foundation** Java agent uses the service loader to instantiate your shutdown listeners and attaches them to the active JVM.
+As shown previously under [ServiceLoader Configuration Files](#serviceloader-configuration-files), **_META-INF/services/com.nordstrom.automation.junit.JUnitWatcher_** is the configuration file where shutdown listeners are declared. By default, this file is absent. To add managed listeners, create this file and add the fully-qualified names of their classes, one line per item. When it loads, the **JUnit Foundation** Java agent uses the service loader to instantiate your shutdown listeners and attaches them to the active JVM.
 
 ## Artifact Capture
 

src/main/java/com/nordstrom/automation/junit/CreateTest.java

@@ -1,13 +1,11 @@
 package com.nordstrom.automation.junit;
 
 import java.util.Map;
-import java.util.ServiceLoader;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentHashMap;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.base.Optional;
 import net.bytebuddy.implementation.bind.annotation.RuntimeType;
 import net.bytebuddy.implementation.bind.annotation.SuperCall;
 import net.bytebuddy.implementation.bind.annotation.This;
@@ -19,14 +17,12 @@
 @SuppressWarnings("squid:S1118")
 public class CreateTest {
 
-    private static final ServiceLoader<TestObjectWatcher> objectWatcherLoader;
     private static final Map<Object, Object> TARGET_TO_RUNNER = new ConcurrentHashMap<>();
     private static final Map<Object, Object> RUNNER_TO_TARGET = new ConcurrentHashMap<>();
     private static final ThreadLocal<DepthGauge> DEPTH;
     private static final Logger LOGGER = LoggerFactory.getLogger(CreateTest.class);
 
     static {
-        objectWatcherLoader = ServiceLoader.load(TestObjectWatcher.class);
         DEPTH = new ThreadLocal<DepthGauge>() {
             @Override
             protected DepthGauge initialValue() {
@@ -60,10 +56,8 @@ public static Object intercept(@This final Object runner,
 
         if (DEPTH.get().atGroundLevel()) {
             LOGGER.debug("testObjectCreated: {}", target);
-            synchronized(objectWatcherLoader) {
-                for (TestObjectWatcher watcher : objectWatcherLoader) {
-                    watcher.testObjectCreated(target, runner);
-                }
+            for (TestObjectWatcher watcher : LifecycleHooks.getObjectWatchers()) {
+                watcher.testObjectCreated(target, runner);
             }
         }
 
@@ -91,26 +85,4 @@ static Object getRunnerForTarget(Object target) {
     static Object getTargetForRunner(Object runner) {
         return RUNNER_TO_TARGET.get(runner);
     }
-    
-    /**
-     * Get reference to an instance of the specified watcher type.
-     * 
-     * @param <T> watcher type
-     * @param watcherType watcher type
-     * @return optional watcher instance
-     */
-    @SuppressWarnings("unchecked")
-    static <T extends JUnitWatcher> Optional<T> getAttachedWatcher(Class<T> watcherType) {
-        if (TestObjectWatcher.class.isAssignableFrom(watcherType)) {
-            synchronized(objectWatcherLoader) {
-                for (TestObjectWatcher watcher : objectWatcherLoader) {
-                    if (watcher.getClass() == watcherType) {
-                        return Optional.of((T) watcher);
-                    }
-                }
-            }
-        }
-        return Optional.absent();
-    }
-    
 }

src/main/java/com/nordstrom/automation/junit/LifecycleHooks.java

@@ -6,7 +6,10 @@
 import java.lang.instrument.Instrumentation;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
+import java.util.AbstractList;
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.List;
 import java.util.ServiceLoader;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentMap;
@@ -36,6 +39,11 @@
 public class LifecycleHooks {
 
     private static JUnitConfig config;
+    private static final List<JUnitWatcher> watchers;
+    private static final List<RunWatcher<?>> runWatchers;
+    private static final List<RunnerWatcher> runnerWatchers;
+    private static final List<TestObjectWatcher> objectWatchers;
+    private static final List<MethodWatcher<?>> methodWatchers;
 
     private LifecycleHooks() {
         throw new AssertionError("LifecycleHooks is a static utility class that cannot be instantiated");
@@ -46,8 +54,68 @@ private LifecycleHooks() {
      * and BlockJUnit4ClassRunner classes to enable the core functionality of JUnit Foundation.
      */
     static {
-        for (ShutdownListener listener : ServiceLoader.load(ShutdownListener.class)) {
-            Runtime.getRuntime().addShutdownHook(getShutdownHook(listener));
+        WatcherClassifier classifier = new WatcherClassifier();
+        
+        for (JUnitWatcher watcher : ServiceLoader.load(JUnitWatcher.class)) {
+            classifier.add(watcher);
+        }
+        
+        for (ShutdownListener watcher : ServiceLoader.load(ShutdownListener.class)) {
+            classifier.add(watcher);
+        }
+        
+        for (RunWatcher<?> watcher : ServiceLoader.load(RunWatcher.class)) {
+            classifier.add(watcher);
+        }
+        
+        for (RunnerWatcher watcher : ServiceLoader.load(RunnerWatcher.class)) {
+            classifier.add(watcher);
+        }
+        
+        for (TestObjectWatcher watcher : ServiceLoader.load(TestObjectWatcher.class)) {
+            classifier.add(watcher);
+        }
+        
+        for (MethodWatcher<?> watcher : ServiceLoader.load(MethodWatcher.class)) {
+            classifier.add(watcher);
+        }
+        
+        watchers = classifier.watchers;
+        
+        runWatchers = new WatcherList<>(classifier.runWatcherIndexes);
+        runnerWatchers = new WatcherList<>(classifier.runnerWatcherIndexes);
+        objectWatchers = new WatcherList<>(classifier.objectWatcherIndexes);
+        methodWatchers = new WatcherList<>(classifier.methodWatcherIndexes);
+    }
+    
+    private static class WatcherClassifier {
+        int i = 0;
+        
+        List<JUnitWatcher> watchers = new ArrayList<>();
+        List<Class<? extends JUnitWatcher>> watcherClasses = new ArrayList<>();
+        
+        List<Integer> runWatcherIndexes = new ArrayList<>();
+        List<Integer> runnerWatcherIndexes = new ArrayList<>();
+        List<Integer> objectWatcherIndexes = new ArrayList<>();
+        List<Integer> methodWatcherIndexes = new ArrayList<>();
+        
+        boolean add(JUnitWatcher watcher) {
+            if ( ! watcherClasses.contains(watcher.getClass())) {
+                watchers.add(watcher);
+                watcherClasses.add(watcher.getClass());
+                
+                if (watcher instanceof ShutdownListener) {
+                    Runtime.getRuntime().addShutdownHook(getShutdownHook((ShutdownListener) watcher));
+                }
+                
+                if (watcher instanceof RunWatcher) runWatcherIndexes.add(i);
+                if (watcher instanceof RunnerWatcher) runnerWatcherIndexes.add(i);
+                if (watcher instanceof TestObjectWatcher) objectWatcherIndexes.add(i);
+                if (watcher instanceof MethodWatcher) methodWatcherIndexes.add(i);
+                
+                i++;
+            }
+            return false;
         }
     }
 
@@ -334,17 +402,14 @@ static Object callProxy(final Callable<?> proxy) throws Exception {
      * @param watcherType watcher type
      * @return optional watcher instance
      */
+    @SuppressWarnings("unchecked")
     public static <T extends JUnitWatcher> Optional<T> getAttachedWatcher(Class<T> watcherType) {
-        Optional<T> watcher = CreateTest.getAttachedWatcher(watcherType);
-        if (watcher.isPresent()) return watcher;
-        
-        watcher = Run.getAttachedWatcher(watcherType);
-        if (watcher.isPresent()) return watcher;
-        
-        watcher = RunAnnouncer.getAttachedWatcher(watcherType);
-        if (watcher.isPresent()) return watcher;
-        
-        return RunReflectiveCall.getAttachedWatcher(watcherType);
+        for (JUnitWatcher watcher : watchers) {
+            if (watcher.getClass() == watcherType) {
+                return Optional.of((T) watcher);
+            }
+        }
+        return Optional.absent();
     }
 
     /**
@@ -378,4 +443,84 @@ static <K, T> T computeIfAbsent(ConcurrentMap<K, T> map, K key, Function<K, T> f
         }
         return val;
     }
+    
+    /**
+     * Get the list of attached {@link RunWatcher} objects.
+     * 
+     * @return run watcher list
+     */
+    static List<RunWatcher<?>> getRunWatchers() {
+        return runWatchers;
+    }
+    
+    /**
+     * Get the list of attached {@link RunnerWatcher} objects.
+     * 
+     * @return runner watcher list
+     */
+    static List<RunnerWatcher> getRunnerWatchers() {
+        return runnerWatchers;
+    }
+    
+    /**
+     * Get the list of attached {@link TestObjectWatcher} objects.
+     * 
+     * @return test object watcher list
+     */
+    static List<TestObjectWatcher> getObjectWatchers() {
+        return objectWatchers;
+    }
+    
+    /**
+     * Get the list of attached {@link MethodWatcher} objects.
+     * 
+     * @return method watcher list
+     */
+    static List<MethodWatcher<?>> getMethodWatchers() {
+        return methodWatchers;
+    }
+    
+    /**
+     * This class encapsulates the process of retrieving watcher objects of the target type from the collection of all
+     * attached watcher objects. This is a private nested class that directly accesses the main collection. It is also
+     * unmodifiable. Any attempts to alter the collection will trigger an {@link UnsupportedOperationException}.
+     * 
+     * @param <T> subclass of {@link JUnitWatcher} object supplied by this instance
+     */
+    private static class WatcherList<T extends JUnitWatcher> extends AbstractList<T> {
+        
+        private int[] indexes;
+        
+        /**
+         * Constructor for a list of watcher objects of the target type retrieved from the collection of all attached
+         * {@link JUnitWatcher} objects.
+         * 
+         * @param indexes indexes of watchers of the requisite type in the main collection
+         */
+        private WatcherList(List<Integer> indexes) {
+            int i = 0;
+            this.indexes = new int[indexes.size()];
+            for (int index : indexes) {
+                this.indexes[i++] = index;
+            }
+        }
+
+        /**
+         * {@inheritDoc}
+         */
+        @Override
+        @SuppressWarnings("unchecked")
+        public T get(int index) {
+            return (T) watchers.get(indexes[index]);
+        }
+
+        /**
+         * {@inheritDoc}
+         */
+        @Override
+        public int size() {
+            return indexes.length;
+        }
+        
+    }
 }

src/main/java/com/nordstrom/automation/junit/Run.java

@@ -31,7 +31,6 @@ public class Run {
     private static final Set<String> startNotified = new CopyOnWriteArraySet<>();
     private static final Set<String> finishNotified = new CopyOnWriteArraySet<>();
     private static final ServiceLoader<RunListener> runListenerLoader;
-    private static final ServiceLoader<RunnerWatcher> runnerWatcherLoader;
     private static final Map<Object, Object> CHILD_TO_PARENT = new ConcurrentHashMap<>();
     private static final Set<RunNotifier> NOTIFIERS = new CopyOnWriteArraySet<>();
     private static final Logger LOGGER = LoggerFactory.getLogger(Run.class);
@@ -44,7 +43,6 @@ protected Deque<Object> initialValue() {
             }
         };
         runListenerLoader = ServiceLoader.load(RunListener.class);
-        runnerWatcherLoader = ServiceLoader.load(RunnerWatcher.class);
     }
 
     /**
@@ -143,10 +141,8 @@ static boolean fireRunStarted(Object runner) {
                 CHILD_TO_PARENT.put(grandchild, runner);
             }
             LOGGER.debug("runStarted: {}", runner);
-            synchronized(runnerWatcherLoader) {
-                for (RunnerWatcher watcher : runnerWatcherLoader) {
-                    watcher.runStarted(runner);
-                }
+            for (RunnerWatcher watcher : LifecycleHooks.getRunnerWatchers()) {
+                watcher.runStarted(runner);
             }
             return true;
         }
@@ -163,37 +159,14 @@ static boolean fireRunStarted(Object runner) {
     static boolean fireRunFinished(Object runner) {
         if (finishNotified.add(runner.toString())) {
             LOGGER.debug("runFinished: {}", runner);
-            synchronized(runnerWatcherLoader) {
-                for (RunnerWatcher watcher : runnerWatcherLoader) {
-                    watcher.runFinished(runner);
-                }
+            for (RunnerWatcher watcher : LifecycleHooks.getRunnerWatchers()) {
+                watcher.runFinished(runner);
             }
             return true;
         }
         return false;
     }
 
-    /**
-     * Get reference to an instance of the specified watcher type.
-     * 
-     * @param <T> watcher type
-     * @param watcherType watcher type
-     * @return optional watcher instance
-     */
-    @SuppressWarnings("unchecked")
-    static <T extends JUnitWatcher> Optional<T> getAttachedWatcher(Class<T> watcherType) {
-        if (RunnerWatcher.class.isAssignableFrom(watcherType)) {
-            synchronized(runnerWatcherLoader) {
-                for (RunnerWatcher watcher : runnerWatcherLoader) {
-                    if (watcher.getClass() == watcherType) {
-                        return Optional.of((T) watcher);
-                    }
-                }
-            }
-        }
-        return Optional.absent();
-    }
-    
     /**
      * Get reference to an instance of the specified listener type.
      *