Continued Dependency Injection Refactor

Author: JLLeitschuh

build.gradle

@@ -31,6 +31,7 @@ allprojects {
 
 
     dependencies {
+        testCompile group: 'net.jodah', name: 'concurrentunit', version: '0.4.2'
         testCompile group: 'org.hamcrest', name: 'hamcrest-all', version: '1.3'
         testCompile group: 'junit', name: 'junit', version: '4.12'
     }
@@ -76,6 +77,7 @@ def os = osdetector.classifier.replace("osx", "macosx").replace("windows-x86_32"
 project(":core") {
     apply plugin: 'java'
     apply plugin: 'idea'
+    apply plugin: 'jacoco'
     apply plugin: "com.github.johnrengelman.shadow"
 
     configurations {
@@ -173,6 +175,7 @@ project(":core") {
 project(":ui") {
     apply plugin: 'java'
     apply plugin: 'idea'
+    apply plugin: 'jacoco'
     apply plugin: 'application'
     apply from: 'http://dl.bintray.com/shemnon/javafx-gradle/8.1.1/javafx.plugin'
 

core/src/main/java/edu/wpi/grip/core/Connection.java

@@ -5,10 +5,7 @@
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 import com.thoughtworks.xstream.annotations.XStreamAlias;
-import edu.wpi.grip.core.events.ConnectionRemovedEvent;
-import edu.wpi.grip.core.events.SocketChangedEvent;
-import edu.wpi.grip.core.events.SourceRemovedEvent;
-import edu.wpi.grip.core.events.StepRemovedEvent;
+import edu.wpi.grip.core.events.*;
 
 import static com.google.common.base.Preconditions.checkArgument;
 
@@ -38,7 +35,6 @@ public interface Factory <T> {
         this.outputSocket = outputSocket;
         this.inputSocket = inputSocket;
         checkArgument(pipeline.canConnect(outputSocket, inputSocket), "Cannot connect sockets");
-        inputSocket.setValueOptional(outputSocket.getValue());
     }
 
     public OutputSocket<? extends T> getOutputSocket() {
@@ -49,10 +45,21 @@ public InputSocket<T> getInputSocket() {
         return this.inputSocket;
     }
 
+    private void runConnection() {
+        inputSocket.setValueOptional(outputSocket.getValue());
+    }
+
+    @Subscribe
+    public void onConnectionAdded(ConnectionAddedEvent event) {
+        if (event.getConnection().equals(this)) {
+            runConnection();
+        }
+    }
+
     @Subscribe
     public void onOutputChanged(SocketChangedEvent e) {
         if (e.getSocket() == outputSocket) {
-            inputSocket.setValueOptional(outputSocket.getValue());
+            runConnection();
         }
     }
 

core/src/main/java/edu/wpi/grip/core/GRIPCoreModule.java

@@ -40,11 +40,6 @@ public <I> void hear(TypeLiteral<I> type, TypeEncounter<I> encounter) {
 
         bind(EventBus.class).toInstance(eventBus);
 
-        install(new FactoryModuleBuilder().build(Step.Factory.class));
-
-
-        install(new FactoryModuleBuilder().build(new TypeLiteral<Connection.Factory<Number>>(){}));
-        install(new FactoryModuleBuilder().build(new TypeLiteral<Connection.Factory<Double>>(){}));
         install(new FactoryModuleBuilder().build(new TypeLiteral<Connection.Factory<Object>>(){}));
 
 

core/src/main/java/edu/wpi/grip/core/Main.java

@@ -19,15 +19,18 @@
  */
 public class Main {
 
-    @Inject private Project project;
-    @Inject private EventBus eventBus;
+    @Inject
+    private Project project;
+    @Inject
+    private EventBus eventBus;
 
+    @SuppressWarnings("PMD.SignatureDeclareThrowsException")
     public static void main(String[] args) throws Exception {
         final Injector injector = Guice.createInjector(new GRIPCoreModule());
         injector.getInstance(Main.class).start(args);
     }
 
-    public void start(String[] args) throws Exception {
+    public void start(String[] args) throws IOException, InterruptedException {
         if (args.length != 1) {
             System.err.println("Usage: GRIP.jar project.grip");
             return;

core/src/main/java/edu/wpi/grip/core/Palette.java

@@ -17,7 +17,12 @@
 @Singleton
 public class Palette {
 
-    @Inject private EventBus eventBus;
+    private final EventBus eventBus;
+
+    @Inject
+    Palette(EventBus eventBus) {
+        this.eventBus = eventBus;
+    }
 
     private final Map<String, Operation> operations = new LinkedHashMap<>();
 

core/src/main/java/edu/wpi/grip/core/Pipeline.java

@@ -11,6 +11,9 @@
 import java.util.*;
 import java.util.stream.Collectors;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /**
  * Pipeline has the list of steps in a computer vision algorithm, as well as the set of connections between the inputs
  * and outputs of different steps.
@@ -135,48 +138,50 @@ public void onSourceRemoved(SourceRemovedEvent event) {
         }
     }
 
-    @Subscribe
-    public synchronized void onStepAdded(StepAddedEvent event) {
-        final Step step = event.getStep();
-
-        this.steps.add(event.getIndex().or(this.steps.size()), step);
-        this.eventBus.register(event.getStep());
+    public synchronized void addStep(Step step, int index) {
+        checkNotNull(step, "The step can not be null");
+        this.steps.add(index, step);
+        this.eventBus.register(step);
+        this.eventBus.post(new StepAddedEvent(step, index));
     }
 
-    @Subscribe
-    public synchronized void onStepRemoved(StepRemovedEvent event) {
-        this.steps.remove(event.getStep());
-        this.eventBus.unregister(event.getStep());
+    public synchronized  void addStep(Step step) {
+        addStep(step, this.steps.size());
+    }
 
+    public synchronized void removeStep(Step step) {
+        checkNotNull(step, "The step can not be null");
+        this.steps.remove(step);
         // Sockets of deleted steps should not be previewed
-        for (OutputSocket<?> socket : event.getStep().getOutputSockets()) {
+        for (OutputSocket<?> socket : step.getOutputSockets()) {
             socket.setPreviewed(false);
         }
+        this.eventBus.post(new StepRemovedEvent(step));
+        this.eventBus.unregister(step);
     }
 
-    @Subscribe
-    public synchronized void onStepMoved(StepMovedEvent event) {
-        final Step step = event.getStep();
+    public synchronized void moveStep(Step step, int delta) {
+        checkNotNull(step, "The step can not be null");
+        checkArgument(this.steps.contains(step), "The step must exist in the pipeline to be moved");
 
         final int oldIndex = this.steps.indexOf(step);
         this.steps.remove(oldIndex);
 
         // Compute the new index of the step, clamping to the beginning or end of pipeline if it goes past either end
-        final int newIndex = Math.min(Math.max(oldIndex + event.getDistance(), 0), this.steps.size());
+        final int newIndex = Math.min(Math.max(oldIndex + delta, 0), this.steps.size());
         this.steps.add(newIndex, step);
+        eventBus.post(new StepMovedEvent(step, delta));
     }
 
     @Subscribe
     public void onConnectionAdded(ConnectionAddedEvent event) {
-        this.connections.add(event.getConnection());
-        this.eventBus.register(event.getConnection());
+        final Connection connection = event.getConnection();
+        this.connections.add(connection);
     }
 
     @Subscribe
     public void onConnectionRemoved(ConnectionRemovedEvent event) {
         this.connections.remove(event.getConnection());
         this.eventBus.unregister(event.getConnection());
     }
-
-
 }

core/src/main/java/edu/wpi/grip/core/Step.java

@@ -3,7 +3,7 @@
 import com.google.common.eventbus.EventBus;
 import com.google.common.eventbus.Subscribe;
 import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
+import com.google.inject.Singleton;
 import com.thoughtworks.xstream.annotations.XStreamAlias;
 import edu.wpi.grip.core.events.SocketChangedEvent;
 
@@ -20,43 +20,62 @@
  */
 @XStreamAlias(value = "grip:Step")
 public class Step {
-    private final Logger logger =  Logger.getLogger(Step.class.getName());
-    private Operation operation;
-    private InputSocket<?>[] inputSockets;
-    private OutputSocket<?>[] outputSockets;
-    private Optional<?> data;
 
-    public interface Factory {
-        Step create(Operation operation);
-    }
+    private final Logger logger =  Logger.getLogger(Step.class.getName());
 
-    /**
-     * @param eventBus  The Guava {@link EventBus} used by the application.
-     * @param operation The operation that is performed at this step.
-     */
-    @Inject
-    Step(EventBus eventBus, @Assisted Operation operation) {
-        this.operation = operation;
+    private final Operation operation;
+    private final InputSocket<?>[] inputSockets;
+    private final OutputSocket<?>[] outputSockets;
+    private final Optional<?> data;
 
-        checkNotNull(eventBus);
-        checkNotNull(operation);
+    @Singleton
+    public static class Factory {
+        private final EventBus eventBus;
 
-        // Create the list of input and output sockets, and mark this step as their owner.
-        inputSockets = operation.createInputSockets(eventBus);
-        for (Socket<?> socket : inputSockets) {
-            socket.setStep(Optional.of(this));
-            eventBus.register(socket);
+        @Inject
+        public Factory(EventBus eventBus) {
+            this.eventBus = eventBus;
         }
 
-        outputSockets = operation.createOutputSockets(eventBus);
-        for (Socket<?> socket : outputSockets) {
-            socket.setStep(Optional.of(this));
-            eventBus.register(socket);
-        }
+        public Step create(Operation operation) {
+            checkNotNull(operation, "The operation can not be null");
+            // Create the list of input and output sockets, and mark this step as their owner.
+            final InputSocket<?>[] inputSockets = operation.createInputSockets(eventBus);
 
-        data = operation.createData();
+            for (Socket<?> socket : inputSockets) {
+                eventBus.register(socket);
+            }
+
+            final OutputSocket<?>[] outputSockets = operation.createOutputSockets(eventBus);
+            for (Socket<?> socket : outputSockets) {
+                eventBus.register(socket);
+            }
 
-        runPerformIfPossible();
+            final Step step = new Step(operation, inputSockets, outputSockets, operation.createData());
+            eventBus.register(step);
+            for (Socket<?> socket : inputSockets) {
+                socket.setStep(Optional.of(step));
+            }
+            for (Socket<?> socket : outputSockets) {
+                socket.setStep(Optional.of(step));
+            }
+
+            step.runPerformIfPossible();
+            return step;
+        }
+    }
+
+    /**
+     * @param operation     The operation that is performed at this step.
+     * @param inputSockets  The input sockets from the operation.
+     * @param outputSockets The output sockets provided by the operation.
+     * @param data          The data provided by the operation.
+     */
+    Step(Operation operation, InputSocket<?>[] inputSockets, OutputSocket<?>[] outputSockets, Optional<?> data) {
+        this.operation = operation;
+        this.inputSockets = inputSockets;
+        this.outputSockets = outputSockets;
+        this.data = data;
     }
 
     /**

core/src/main/java/edu/wpi/grip/core/serialization/StepConverter.java

@@ -8,7 +8,6 @@
 import com.thoughtworks.xstream.io.HierarchicalStreamReader;
 import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
 import edu.wpi.grip.core.*;
-import edu.wpi.grip.core.events.StepAddedEvent;
 
 import javax.inject.Inject;
 import java.util.Optional;
@@ -25,6 +24,7 @@ public class StepConverter implements Converter {
 
     @Inject private EventBus eventBus;
     @Inject private Palette palette;
+    @Inject private Pipeline pipeline;
     @Inject private Step.Factory stepFactory;
 
     @Override
@@ -54,7 +54,7 @@ public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext co
 
         // Instead of simply returning the step and having XStream insert it into the pipeline using reflection, send a
         // StepAddedEvent.  This allows other interested classes (such as PipelineView) to also know when steps are added.
-        this.eventBus.post(new StepAddedEvent(stepFactory.create(operation.get())));
+        pipeline.addStep(stepFactory.create(operation.get()));
 
         while (reader.hasMoreChildren()) {
             context.convertAnother(this, Socket.class);

core/src/main/java/edu/wpi/grip/core/sources/CameraSource.java

@@ -57,6 +57,7 @@ public interface Factory {
 
     public interface FrameGrabberFactory {
         FrameGrabber create(int deviceNumber);
+
         FrameGrabber create(String addressProperty) throws MalformedURLException;
     }
 
@@ -192,7 +193,7 @@ public void start() throws IOException, IllegalStateException {
                         eventBus.post(new UnexpectedThrowableEvent(exception, "Camera Frame Grabber Thread crashed with uncaught exception"));
                         try {
                             stop();
-                        } catch (TimeoutException e) {
+                        } catch (TimeoutException | IOException e) {
                             // TODO: This should use the ExceptionWitness once that has a UI component added for it
                             eventBus.post(new UnexpectedThrowableEvent(e, "Camera Frame Grabber could not be stopped!"));
                         }
@@ -211,10 +212,11 @@ public void start() throws IOException, IllegalStateException {
      * This will stop the source publishing new socket values after this method returns.
      *
      * @return The source that was stopped
-     * @throws TimeoutException if the thread running the source fails to stop.
-     * @throws IOException      If there is a problem stopping the Source
+     * @throws TimeoutException      If the thread running the source fails to stop.
+     * @throws IOException           If there is a problem stopping the Source
+     * @throws IllegalStateException If the camera is already stopped.
      */
-    public final void stop() throws TimeoutException, IllegalStateException {
+    public final void stop() throws TimeoutException, IOException {
         synchronized (this) {
             if (frameThread.isPresent()) {
                 final Thread ex = frameThread.get();
@@ -235,7 +237,7 @@ public final void stop() throws TimeoutException, IllegalStateException {
                         // Calling this multiple times will have no effect
                         grabber.stop();
                     } catch (FrameGrabber.Exception e) {
-                        throw new IllegalStateException("A problem occurred trying to stop the frame grabber", e);
+                        throw new IOException("A problem occurred trying to stop the frame grabber", e);
                     }
                 }
             } else {
@@ -252,7 +254,7 @@ public synchronized boolean isStarted() {
     }
 
     @Subscribe
-    public void onSourceRemovedEvent(SourceRemovedEvent event) throws TimeoutException {
+    public void onSourceRemovedEvent(SourceRemovedEvent event) throws TimeoutException, IOException {
         if (event.getSource() == this) {
             try {
                 if (this.isStarted()) this.stop();
@@ -270,7 +272,7 @@ private static Properties createProperties(String address) {
 
     private static Properties createProperties(int deviceNumber) {
         final Properties properties = new Properties();
-        properties.setProperty(DEVICE_NUMBER_PROPERTY, "" + deviceNumber);
+        properties.setProperty(DEVICE_NUMBER_PROPERTY, Integer.toString(deviceNumber));
         return properties;
     }