Generified Flow Search Algorithms

src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/AbstractFlowScanner.java

@@ -30,6 +30,7 @@
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
+import javax.annotation.concurrent.NotThreadSafe;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -38,7 +39,6 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.NoSuchElementException;
-import java.util.Set;
 
 /**
  * Core APIs and base logic for FlowScanners that extract information from a pipeline execution.
@@ -78,7 +78,7 @@
  *   <ul>
  *   <li>Implement a {@link FlowNodeVisitor} that collects metrics from each FlowNode visited, and call visitAll to extract the data.</li>
  *   <li>Find all flownodes of a given type (ex: stages), using {@link #filteredNodes(Collection, Collection, Predicate)}</li>
- *   <li>Find the first node with an Error before a specific node</li>
+ *   <li>Find the first node with an {@link org.jenkinsci.plugins.workflow.actions.ErrorAction} before a specific node</li>
  *   <li>Scan through all nodes *just* within a block
  *      <ul>
  *        <li>Use the {@link org.jenkinsci.plugins.workflow.graph.BlockEndNode} as the head</li>
@@ -88,6 +88,7 @@
  *
  * @author <samvanoort@gmail.com>Sam Van Oort</samvanoort@gmail.com>
  */
+@NotThreadSafe
 public abstract class AbstractFlowScanner implements Iterable <FlowNode>, Filterator<FlowNode> {
 
     protected FlowNode myCurrent;
@@ -235,15 +236,16 @@ public Filterator<FlowNode> filter(@Nonnull Predicate<FlowNode> filterCondition)
      * Find the first FlowNode within the iteration order matching a given condition
      * Includes null-checking on arguments to allow directly calling with unchecked inputs (simplifies use).
      * @param heads Head nodes to start walking from
-     * @param endNodes
+     * @param blackListNodes Nodes that are never visited, search stops here (bound is exclusive).
+     *                       If you want to create an inclusive bound, just use a node's parents.
      * @param matchCondition Predicate to match when we've successfully found a given node type
      * @return First matching node, or null if no matches found
      */
     @CheckForNull
     public FlowNode findFirstMatch(@CheckForNull Collection<FlowNode> heads,
-                                           @CheckForNull Collection<FlowNode> endNodes,
+                                           @CheckForNull Collection<FlowNode> blackListNodes,
                                            Predicate<FlowNode> matchCondition) {
-        if (!setup(heads, endNodes)) {
+        if (!setup(heads, blackListNodes)) {
             return null;
         }
 
@@ -283,7 +285,7 @@ public FlowNode findFirstMatch(@CheckForNull FlowExecution exec, @Nonnull Predic
      * Includes null-checking on arguments to allow directly calling with unchecked inputs (simplifies use).
      * @param heads Nodes to start iterating backward from by visiting their parents.
      * @param blackList Nodes we may not visit or walk beyond.
-     * @param matchCondition Predicate that must be met for nodes to be included in output.
+     * @param matchCondition Predicate that must be met for nodes to be included in output.  Input is always non-null.
      * @return List of flownodes matching the predicate.
      */
     @Nonnull

src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/DepthFirstScanner.java

@@ -28,6 +28,7 @@
 import org.jenkinsci.plugins.workflow.graph.FlowNode;
 
 import javax.annotation.Nonnull;
+import javax.annotation.concurrent.NotThreadSafe;
 import java.util.ArrayDeque;
 import java.util.Collection;
 import java.util.HashSet;
@@ -42,6 +43,7 @@
  * <p/> The behavior is analogous to {@link org.jenkinsci.plugins.workflow.graph.FlowGraphWalker} but faster.
  *  @author <samvanoort@gmail.com>Sam Van Oort</samvanoort@gmail.com>
  */
+@NotThreadSafe
 public class DepthFirstScanner extends AbstractFlowScanner {
 
     protected ArrayDeque<FlowNode> queue;

src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/FilteratorImpl.java

@@ -27,11 +27,13 @@
 import com.google.common.base.Predicate;
 
 import javax.annotation.Nonnull;
+import javax.annotation.concurrent.NotThreadSafe;
 import java.util.Iterator;
 
 /** Filters an iterator against a match predicate by wrapping an iterator
  * @author <samvanoort@gmail.com>Sam Van Oort</samvanoort@gmail.com>
  */
+@NotThreadSafe
 class FilteratorImpl<T> implements Filterator<T> {
     private boolean hasNext = false;
     private T nextVal = null;

src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/FlowScanningUtils.java

@@ -32,15 +32,10 @@
 import org.jenkinsci.plugins.workflow.actions.LogAction;
 import org.jenkinsci.plugins.workflow.actions.StageAction;
 import org.jenkinsci.plugins.workflow.actions.WorkspaceAction;
-import org.jenkinsci.plugins.workflow.graph.BlockEndNode;
 import org.jenkinsci.plugins.workflow.graph.BlockStartNode;
 import org.jenkinsci.plugins.workflow.graph.FlowNode;
 
 import javax.annotation.Nonnull;
-import java.util.ArrayDeque;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
 
 /**
  * Library of common functionality when analyzing/walking flow graphs
@@ -57,7 +52,7 @@ private FlowScanningUtils() {}
      * @return Predicate that will match when FlowNode has the action given
      */
     @Nonnull
-    public static  Predicate<FlowNode> nodeHasActionPredicate(@Nonnull final Class<? extends Action> actionClass) {
+    public static  Predicate<FlowNode> hasActionPredicate(@Nonnull final Class<? extends Action> actionClass) {
         return new Predicate<FlowNode>() {
             @Override
             public boolean apply(FlowNode input) {
@@ -67,11 +62,6 @@ public boolean apply(FlowNode input) {
     }
 
     // Default predicates, which may be used for common conditions
-    public static final Predicate<FlowNode> MATCH_HAS_LABEL = nodeHasActionPredicate(LabelAction.class);
-    public static final Predicate<FlowNode> MATCH_IS_STAGE = nodeHasActionPredicate(StageAction.class);
-    public static final Predicate<FlowNode> MATCH_HAS_WORKSPACE = nodeHasActionPredicate(WorkspaceAction.class);
-    public static final Predicate<FlowNode> MATCH_HAS_ERROR = nodeHasActionPredicate(ErrorAction.class);
-    public static final Predicate<FlowNode> MATCH_HAS_LOG = nodeHasActionPredicate(LogAction.class);
     public static final Predicate<FlowNode> MATCH_BLOCK_START = (Predicate)Predicates.instanceOf(BlockStartNode.class);
 
     /**

src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScanner.java

@@ -31,6 +31,7 @@
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
+import javax.annotation.concurrent.NotThreadSafe;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -61,6 +62,7 @@
  *
  * @author <samvanoort@gmail.com>Sam Van Oort</samvanoort@gmail.com>
  */
+@NotThreadSafe
 public class ForkScanner extends AbstractFlowScanner {
 
     // Last element in stack is end of myCurrent parallel start, first is myCurrent start
@@ -82,7 +84,8 @@ protected void reset() {
         myNext = null;
     }
 
-    /** If true, we are walking from the flow end node and have a complete view of the flow */
+    /** If true, we are walking from the flow end node and have a complete view of the flow
+     *  Needed because there are implications when not walking from a finished flow (blocks without a {@link BlockEndNode})*/
     public boolean isWalkingFromFinish() {
         return walkingFromFinish;
     }
@@ -264,7 +267,7 @@ ArrayDeque<ParallelBlockStart> leastCommonAncestor(@Nonnull Set<FlowNode> heads)
 
             while (itIterator.hasNext()) {
                 Filterator<FlowNode> blockStartIterator = itIterator.next();
-                FlowPiece myPiece = pieceIterator.next();
+                FlowPiece myPiece = pieceIterator.next(); //Safe because we always remove/add with both iterators at once
 
                 // Welp we hit the end of a branch
                 if (!blockStartIterator.hasNext()) {
@@ -316,7 +319,6 @@ ArrayDeque<ParallelBlockStart> leastCommonAncestor(@Nonnull Set<FlowNode> heads)
     @Override
     protected void setHeads(@Nonnull Collection<FlowNode> heads) {
         if (heads.size() > 1) {
-            //throw new IllegalArgumentException("ForkedFlowScanner can't handle multiple head nodes yet");
             parallelBlockStartStack = leastCommonAncestor(new LinkedHashSet<FlowNode>(heads));
             currentParallelStart = parallelBlockStartStack.pop();
             currentParallelStartNode = currentParallelStart.forkStart;
@@ -415,8 +417,8 @@ protected FlowNode next(@Nonnull FlowNode current, @Nonnull Collection<FlowNode>
 
         // First we look at the parents of the current node if present
         List<FlowNode> parents = current.getParents();
-        if (parents == null || parents.size() == 0) {
-            // welp done with this node, guess we consult the queue?
+        if (parents.isEmpty()) {
+            // welp, we're done with this node, guess we consult the queue?
         } else if (parents.size() == 1) {
             FlowNode p = parents.get(0);
             if (p == currentParallelStartNode) {
@@ -436,7 +438,7 @@ protected FlowNode next(@Nonnull FlowNode current, @Nonnull Collection<FlowNode>
                 return possibleOutput;
             }
         } else {
-            throw new IllegalStateException("Found a FlowNode with multiple parents that isn't the end of a block! "+ this.myCurrent.toString());
+            throw new IllegalStateException("Found a FlowNode with multiple parents that isn't the end of a block! "+ this.myCurrent);
         }
 
         if (currentParallelStart != null && currentParallelStart.unvisited.size() > 0) {

src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/LinearBlockHoppingScanner.java

@@ -30,6 +30,7 @@
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
+import javax.annotation.concurrent.NotThreadSafe;
 import java.util.Collection;
 import java.util.List;
 
@@ -49,11 +50,12 @@
  * <ul>
  *   <li>Finding out the executor workspace used to run a FlowNode</li>
  *   <li>Finding the start of the parallel block enclosing the current node</li>
- *   <li>Locating the label applying to a given FlowNode (if any)</li>
+ *   <li>Locating the label applying to a given FlowNode (if any) if using labelled blocks</li>
  * </ul>
  *
  * @author <samvanoort@gmail.com>Sam Van Oort</samvanoort@gmail.com>
  */
+@NotThreadSafe
 public class LinearBlockHoppingScanner extends LinearScanner {
 
     @Override

src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/LinearScanner.java

@@ -27,6 +27,7 @@
 import org.jenkinsci.plugins.workflow.graph.FlowNode;
 
 import javax.annotation.Nonnull;
+import javax.annotation.concurrent.NotThreadSafe;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
@@ -40,8 +41,10 @@
  * <p/>Use case: we don't care about parallel branches or know they don't exist, we just want to walk through the top-level blocks.
  *
  * <p/>This is the fastest & simplest way to walk a flow, because you only care about a single node at a time.
+ * Nuance: where there are multiple parent nodes (in a parallel block), and one is blacklisted, we'll find the first non-blacklisted one.
  * @author <samvanoort@gmail.com>Sam Van Oort</samvanoort@gmail.com>
  */
+@NotThreadSafe
 public class LinearScanner extends AbstractFlowScanner {
 
     @Override

src/test/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScannerTest.java

@@ -24,6 +24,8 @@
 
 package org.jenkinsci.plugins.workflow.graphanalysis;
 
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
 import org.jenkinsci.plugins.workflow.flow.FlowExecution;
 import org.jenkinsci.plugins.workflow.graph.BlockStartNode;
@@ -43,6 +45,7 @@
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Set;
 
 // Slightly dirty but it removes a ton of FlowTestUtils.* class qualifiers
@@ -131,6 +134,7 @@ public void setUp() throws Exception {
                         "}\n" +
                         "steps['2'] = {\n" +
                         "    echo '2a'\n" +
+                        "    echo '2b'\n" +
                         "    def nested = [:]\n" +
                         "    nested['2-1'] = {\n" +
                         "        echo 'do 2-1'\n" +
@@ -139,7 +143,6 @@ public void setUp() throws Exception {
                         "        sleep 1\n" +
                         "        echo '2 section 2'\n" +
                         "    }\n" +
-                        "    echo '2b'\n" +
                         "    parallel nested\n" +
                         "}\n" +
                         "parallel steps\n" +
@@ -280,6 +283,20 @@ public void testFlowSegmentSplit() throws Exception {
         Assert.assertEquals(sideBranch, nodeMap.get(exec.getNode("7")));
     }
 
+    @Test
+    public void testEmptyParallel() throws Exception {
+        WorkflowJob job = r.jenkins.createProject(WorkflowJob.class, "EmptyParallel");
+        job.setDefinition(new CpsFlowDefinition(
+                "parallel 'empty1': {}, 'empty2':{} \n" +
+                        "echo 'done' "
+        ));
+        WorkflowRun b = r.assertBuildStatusSuccess(job.scheduleBuild2(0));
+        ForkScanner scan = new ForkScanner();
+
+        List<FlowNode> outputs = scan.filteredNodes(b.getExecution().getCurrentHeads(), (Predicate) Predicates.alwaysTrue());
+        Assert.assertEquals(9, outputs.size());
+    }
+
     /** Reference the flow graphs in {@link #SIMPLE_PARALLEL_RUN} and {@link #NESTED_PARALLEL_RUN} */
     @Test
     public void testLeastCommonAncestor() throws Exception {