Operations are dropped in the position they are released in pipeline

JLLeitschuh · JLLeitschuh · commit 69fd42634cec · 2016-04-08T12:07:43.000-04:00
Operations will now land in the location they were placed instead
of just being appended to the end of the pipeline.
diff --git a/core/src/main/java/edu/wpi/grip/core/Pipeline.java b/core/src/main/java/edu/wpi/grip/core/Pipeline.java
@@ -14,6 +14,7 @@
 import edu.wpi.grip.core.sockets.OutputSocket;
 import edu.wpi.grip.core.sockets.SocketHint;
 
+import javax.annotation.Nullable;
 import java.util.*;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReadWriteLock;
@@ -230,6 +231,37 @@ public void onSourceRemoved(SourceRemovedEvent event) {
         }
     }
 
+    /**
+     * Adds the step between two other steps.
+     * @param stepToAdd The step to add to the pipeline
+     * @param lower     The step to be added above
+     * @param higher    The step to be added below
+     */
+    public void addStepBetween(Step stepToAdd, @Nullable Step lower, @Nullable Step higher) {
+        checkNotNull(stepToAdd, "The step to add cannot be null");
+        int index = readStepsSafely(steps -> {
+            // If not in the list these can return -1
+            int lowerIndex = steps.indexOf(lower);
+            int upperIndex = steps.indexOf(higher);
+            if (lowerIndex != -1 && upperIndex != -1) {
+                assert lowerIndex <= upperIndex : "Upper step was before lower index";
+                int difference = Math.abs(upperIndex - lowerIndex); // Just in case
+                // Place the step halfway between these two steps
+                return lowerIndex + 1 + difference / 2;
+            } else if (lowerIndex != -1) {
+                // Place it above the lower one
+                return lowerIndex + 1;
+            } else if (upperIndex != -1) {
+                // Place it below the upper one
+                return upperIndex;
+            } else {
+                // Otherwise if they are both null place it at the end of the list
+                return steps.size();
+            }
+        });
+        addStep(index, stepToAdd);
+    }
+
     public void addStep(int index, Step step) {
         checkNotNull(step, "The step can not be null");
         checkArgument(!step.removed(), "The step must not have been disabled already");
diff --git a/core/src/test/java/edu/wpi/grip/core/PipelineTest.java b/core/src/test/java/edu/wpi/grip/core/PipelineTest.java
@@ -275,4 +275,64 @@ public void testCannotConnectIncompatibleTypes() {
 
         assertFalse("Should not be able to connect incompatible types", pipeline.canConnect((OutputSocket) b, (InputSocket) a));
     }
+
+    @Test
+    public void testAddBetweenSteps() {
+        final Step
+                stepToAdd = new MockStep(),
+                lowerStep = new MockStep(),
+                upperStep = new MockStep();
+        pipeline.addStep(lowerStep);
+        pipeline.addStep(upperStep);
+
+        pipeline.addStepBetween(stepToAdd, lowerStep, upperStep);
+        assertEquals("The step was not added to the middle of the pipeline",
+                Arrays.asList(lowerStep, stepToAdd, upperStep), pipeline.getSteps());
+    }
+
+    @Test
+    public void testAddBetweenNullAndStep() {
+        final Step
+                stepToAdd = new MockStep(),
+                lowerStep = new MockStep(),
+                upperStep = new MockStep();
+        pipeline.addStep(lowerStep);
+        pipeline.addStep(upperStep);
+        pipeline.addStepBetween(stepToAdd, null, lowerStep);
+        assertEquals("The step was not added to the begining of the pipeline",
+                Arrays.asList(stepToAdd, lowerStep, upperStep), pipeline.getSteps());
+    }
+
+    @Test
+    public void testAddBetweenStepAndNull() {
+        final Step
+                stepToAdd = new MockStep(),
+                lowerStep = new MockStep(),
+                upperStep = new MockStep();
+        pipeline.addStep(lowerStep);
+        pipeline.addStep(upperStep);
+        pipeline.addStepBetween(stepToAdd, upperStep, null);
+        assertEquals("The step was not added to the end of the pipeline",
+                Arrays.asList(lowerStep, upperStep, stepToAdd), pipeline.getSteps());
+    }
+
+    @Test
+    public void testAddBetweenTwoNulls() {
+        final Step stepToAdd = new MockStep();
+        pipeline.addStepBetween(stepToAdd, null, null);
+        assertEquals("The step should have been added to the pipeline",
+                Collections.singletonList(stepToAdd), pipeline.getSteps());
+    }
+
+    @Test(expected = AssertionError.class)
+    public void testAddBetweenStepsOutOfOrder() {
+        final Step
+                stepToAdd = new MockStep(),
+                lowerStep = new MockStep(),
+                upperStep = new MockStep();
+        pipeline.addStep(lowerStep);
+        pipeline.addStep(upperStep);
+
+        pipeline.addStepBetween(stepToAdd, upperStep, lowerStep);
+    }
 }
diff --git a/ui/src/main/java/edu/wpi/grip/ui/pipeline/PipelineController.java b/ui/src/main/java/edu/wpi/grip/ui/pipeline/PipelineController.java
@@ -33,6 +33,10 @@
 
 import javax.inject.Inject;
 import java.util.Collection;
+import java.util.Map;
+import java.util.NavigableMap;
+import java.util.TreeMap;
+import java.util.stream.Collectors;
 
 /**
  * A JavaFX controller for the pipeline.  This controller renders a list of steps.
@@ -97,15 +101,56 @@ public void initialize() throws Exception {
         });
 
         stepBox.setOnDragDropped(mouseEvent -> {
+            // If this is an operation being dropped
             operationDragService.getValue().ifPresent(operation -> {
+                // Then we need to figure out where to put it in the pipeline
+                // First create a map of every node in the steps list to its x position
+                final Map<Double, Node> positionMapping = stepsMapManager
+                        .entrySet()
+                        .stream()
+                        .collect(
+                                Collectors
+                                        .toMap(e -> calculateMiddleXPosOfNodeInParent(e.getValue()),
+                                                Map.Entry::getValue));
+
+                // A tree map is an easy way to sort the values
+                final NavigableMap<Double, Node> sortedPositionMapping
+                        = new TreeMap<>(positionMapping);
+
+                // Now we find the sockets that are to the immediate left and
+                // immediate right of the drop point
+
+                // These can be null
+                final Map.Entry<Double, Node>
+                        lowerEntry = sortedPositionMapping.floorEntry(mouseEvent.getX()),
+                        higherEntry = sortedPositionMapping.ceilingEntry(mouseEvent.getX());
+                // These can be null
+                final StepController
+                        lowerStepController =
+                        lowerEntry == null ?
+                                null : stepsMapManager.getWithNode(lowerEntry.getValue());
+                final StepController
+                        higherStepController =
+                        higherEntry == null ?
+                                null : stepsMapManager.getWithNode(higherEntry.getValue());
+                final Step
+                        lowerStep = lowerStepController == null ? null : lowerStepController.getStep(),
+                        higherStep = higherStepController == null ? null : higherStepController.getStep();
+
+
                 operationDragService.completeDrag();
-                pipeline.addStep(stepFactory.create(operation));
+                // Add the new step to the pipeline between these two steps
+                pipeline.addStepBetween(stepFactory.create(operation), lowerStep, higherStep);
             });
         });
 
         addSourcePane.getChildren().add(addSourceView);
     }
 
+    private double calculateMiddleXPosOfNodeInParent(Node node) {
+        return node.getLayoutX() + (node.getBoundsInParent().getWidth() / 2.);
+    }
+
     /**
      * @return An unmodifiable list of {@link SourceController} corresponding to all of the sources in the pipeline
      */
diff --git a/ui/src/main/java/edu/wpi/grip/ui/util/StyleClassNameUtility.java b/ui/src/main/java/edu/wpi/grip/ui/util/StyleClassNameUtility.java
@@ -31,7 +31,34 @@ public static String idNameFor(Operation operation) {
      * @return The CSS class for the step in the pipeline. To use as a css selector then prepend the string with a '.'
      */
     public static String classNameFor(Step step) {
-        return shortNameFor(step.getOperation()).append("-step").toString();
+        return classNameForStepHolding(step.getOperation());
+    }
+
+    /**
+     * Return the CSS Class name for a {@link Step} that holds this {@link Operation}
+     *
+     * @param operation The operation to get the step's class name for
+     * @return The CSS class for this step. To use as a css selector then prepend the string with a '.'
+     */
+    public static String classNameForStepHolding(Operation operation) {
+        return shortNameFor(operation).append("-step").toString();
+    }
+
+    public static String cssSelectorForOutputSocketHandleOnStepHolding(Operation operation) {
+        return ".pipeline ." + classNameForStepHolding(operation) + " .socket-handle.output";
+    }
+
+    public static String cssSelectorForInputSocketHandleOnStepHolding(Operation operation) {
+        return ".pipeline ." + classNameForStepHolding(operation) + " .socket-handle.input";
+    }
+
+
+    public static String cssSelectorForOutputSocketHandleOn(Step step) {
+        return cssSelectorForOutputSocketHandleOnStepHolding(step.getOperation());
+    }
+
+    public static String cssSelectorForInputSocketHandleOn(Step step) {
+        return cssSelectorForInputSocketHandleOnStepHolding(step.getOperation());
     }
 
     /**
diff --git a/ui/src/test/java/edu/wpi/grip/ui/MainWindowTest.java b/ui/src/test/java/edu/wpi/grip/ui/MainWindowTest.java
@@ -1,52 +1,145 @@
 package edu.wpi.grip.ui;
 
 
-import edu.wpi.grip.core.AdditionOperation;
-import edu.wpi.grip.core.PipelineRunner;
+import com.google.common.eventbus.EventBus;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.util.Modules;
+import edu.wpi.grip.core.*;
 import edu.wpi.grip.core.events.OperationAddedEvent;
+import edu.wpi.grip.ui.util.DPIUtility;
+import edu.wpi.grip.ui.util.StyleClassNameUtility;
+import edu.wpi.grip.util.GRIPCoreTestModule;
+import javafx.fxml.FXMLLoader;
+import javafx.scene.Parent;
+import javafx.scene.Scene;
 import javafx.stage.Stage;
 import org.junit.After;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.testfx.framework.junit.ApplicationTest;
 import org.testfx.matcher.base.NodeMatchers;
 import org.testfx.util.WaitForAsyncUtils;
 
+import static org.junit.Assert.assertEquals;
 import static org.testfx.api.FxAssert.verifyThat;
 
-
+@SuppressWarnings("PMD.JUnitTestsShouldIncludeAssert")
 public class MainWindowTest extends ApplicationTest {
-
+    private static final String STEP_NOT_ADDED_MSG = "Step was not added to pipeline";
+    private final GRIPCoreTestModule testModule = new GRIPCoreTestModule();
+    private Pipeline pipeline;
     private PipelineRunner pipelineRunner;
+    private Operation addOperation;
+    private AdditionOperation additionOperation;
 
     @Override
     @SuppressWarnings("PMD.SignatureDeclareThrowsException")
     public void start(Stage stage) throws Exception {
-        final Main main = new Main();
-        main.start(stage);
+        testModule.setUp();
+
+        Injector injector = Guice.createInjector(Modules.override(testModule).with(new GRIPUIModule()));
+
+        final Parent root =
+                FXMLLoader.load(Main.class.getResource("MainWindow.fxml"), null, null, injector::getInstance);
+        root.setStyle("-fx-font-size: " + DPIUtility.FONT_SIZE + "px");
+
+        pipeline = injector.getInstance(Pipeline.class);
+        pipelineRunner = injector.getInstance(PipelineRunner.class);
+        final EventBus eventBus = injector.getInstance(EventBus.class);
 
-        final OperationListController operationList = main.injector.getInstance(OperationListController.class);
-        pipelineRunner = main.injector.getInstance(PipelineRunner.class);
-        operationList.clearOperations();
-        operationList.onOperationAdded(new OperationAddedEvent(new AdditionOperation()));
+        addOperation = new AddOperation();
+        additionOperation = new AdditionOperation();
+
+        eventBus.post(new OperationAddedEvent(addOperation));
+        eventBus.post(new OperationAddedEvent(additionOperation));
+
+        stage.setScene(new Scene(root));
+        stage.show();
     }
 
     @After
     public void tearDown() {
+        testModule.tearDown();
         pipelineRunner.stopAsync().awaitTerminated();
     }
 
     @Test
-    @Ignore
-    @SuppressWarnings("PMD.JUnitTestsShouldIncludeAssert")
     public void testShouldCreateNewOperationInPipelineView() {
         // Given:
-        clickOn("#add-operation");
+        clickOn(addOperation.getName());
+
+        WaitForAsyncUtils.waitForFxEvents();
+
+        // Then:
+        final String cssSelector = "." + StyleClassNameUtility.classNameForStepHolding(addOperation);
+        verifyThat(cssSelector, NodeMatchers.isNotNull());
+        verifyThat(cssSelector, NodeMatchers.isVisible());
+
+        assertEquals(STEP_NOT_ADDED_MSG, 1, pipeline.getSteps().size());
+        assertEquals("Step added was not this addOperation", addOperation, pipeline.getSteps().get(0).getOperation());
+    }
+
+    @Test
+    public void testDragOperationFromPaletteToPipeline() {
+        // Given:
+        drag(addOperation.getName())
+                .dropTo(".steps");
 
         WaitForAsyncUtils.waitForFxEvents();
 
         // Then:
-        verifyThat(".pipeline", NodeMatchers.hasChild(".add-step"));
+        final String cssSelector = "." + StyleClassNameUtility.classNameForStepHolding(addOperation);
+        verifyThat(cssSelector, NodeMatchers.isNotNull());
+        verifyThat(cssSelector, NodeMatchers.isVisible());
+
+        assertEquals(STEP_NOT_ADDED_MSG, 1, pipeline.getSteps().size());
+        assertEquals("Step added was not this addOperation", addOperation, pipeline.getSteps().get(0).getOperation());
+    }
+
+    @Test
+    public void testDragOperationFromPaletteToLeftOfExistingStep() {
+        // Run the same test as before
+        testDragOperationFromPaletteToPipeline();
+
+        // Now add a second step before it
+        drag(additionOperation.getName())
+                // We drag to the input socket hint handle because this will always be on the left side of the
+                // step. This should cause the UI to put the new step on the left side
+                .dropTo(StyleClassNameUtility.cssSelectorForInputSocketHandleOnStepHolding(addOperation));
+
+        WaitForAsyncUtils.waitForFxEvents();
+
+        // Then:
+        final String cssSelector = "." + StyleClassNameUtility.classNameForStepHolding(additionOperation);
+        verifyThat(cssSelector, NodeMatchers.isNotNull());
+        verifyThat(cssSelector, NodeMatchers.isVisible());
+
+        assertEquals(STEP_NOT_ADDED_MSG, 2, pipeline.getSteps().size());
+        assertEquals("Step added was not added in the right place in the pipeline",
+                additionOperation, pipeline.getSteps().get(0).getOperation());
+    }
+
+    @Test
+    public void testDragOperationFromPaletteToRightOfExistingStep() {
+        // Run the same test as before
+        testDragOperationFromPaletteToPipeline();
+
+        // Now add a second step after it
+        drag(additionOperation.getName())
+                // We drag to the output socket hint handle because this will always be on the right side of the
+                // step. This should cause the UI to put the new step on the right side
+                .dropTo(StyleClassNameUtility.cssSelectorForOutputSocketHandleOnStepHolding(addOperation));
+
+        WaitForAsyncUtils.waitForFxEvents();
+
+        // Then:
+        final String cssSelector = "." + StyleClassNameUtility.classNameForStepHolding(additionOperation);
+        verifyThat(cssSelector, NodeMatchers.isNotNull());
+        verifyThat(cssSelector, NodeMatchers.isVisible());
+
+        assertEquals(STEP_NOT_ADDED_MSG, 2, pipeline.getSteps().size());
+        assertEquals("Step added was not added in the right place in the pipeline",
+                additionOperation, pipeline.getSteps().get(1).getOperation());
     }
 
 
diff --git a/ui/src/test/java/edu/wpi/grip/ui/pipeline/PipelineUITest.java b/ui/src/test/java/edu/wpi/grip/ui/pipeline/PipelineUITest.java
@@ -76,8 +76,8 @@ public void testConnectingTwoOperations() {
         Step subtractStep = addOperation(1, subtractionOperation);
 
         // When
-        drag(".pipeline ." + StyleClassNameUtility.classNameFor(addStep) + " .socket-handle.output", MouseButton.PRIMARY)
-                .dropTo(".pipeline ." + StyleClassNameUtility.classNameFor(subtractStep) + " .socket-handle.input");
+        drag(StyleClassNameUtility.cssSelectorForOutputSocketHandleOn(addStep), MouseButton.PRIMARY)
+                .dropTo(StyleClassNameUtility.cssSelectorForInputSocketHandleOn(subtractStep));
 
         // Then
         Connection connection = assertStepConnected("The add step did not connect to the subtract step", addStep, subtractStep);
diff --git a/ui/src/test/java/edu/wpi/grip/ui/util/DebugUtility.java b/ui/src/test/java/edu/wpi/grip/ui/util/DebugUtility.java
