Use fail-fast pattern in matchers

- Fixes bug where paths did not consider whether pathlinks matched
- Fixes bug where paths were parsed in reverse order

community/cypher/compatibility-suite/src/main/java/cypher/feature/parser/CypherMatchersCreator.java

@@ -255,8 +255,10 @@ public void exitPath( FeatureResultsParser.PathContext ctx )
         }
         else
         {
-            workload.push( new PathMatcher( new ArrayList<>( pathElements ) ) );
+            ArrayList<PathLinkMatcher> pathLinkMatchers = new ArrayList<>();
+            pathElements.descendingIterator().forEachRemaining( pathLinkMatchers::add );
             pathElements.clear();
+            workload.push( new PathMatcher( pathLinkMatchers ) );
         }
     }
 }

community/cypher/compatibility-suite/src/main/java/cypher/feature/parser/matchers/ListMatcher.java

@@ -36,22 +36,24 @@ public boolean matches( Object value )
         if ( value instanceof List )
         {
             List realList = (List) value;
-            boolean match = realList.size() == list.size();
-            if ( match )
+            if ( realList.size() == list.size() )
             {
                 for ( int i = 0; i < list.size(); ++i )
                 {
-                    match &= list.get( i ).matches( realList.get( i ) );
+                    if ( !list.get( i ).matches( realList.get( i ) ) )
+                    {
+                        return false;
+                    }
                 }
+                return true;
             }
-            return match;
         }
         return false;
     }
 
     @Override
     public String toString()
     {
-        return "ListMatcher for " + list.toString();
+        return "ListMatcher" + list.toString();
     }
 }

community/cypher/compatibility-suite/src/main/java/cypher/feature/parser/matchers/MapMatcher.java

@@ -39,18 +39,20 @@ public boolean matches( Object value )
     {
         if ( value instanceof Map )
         {
-            Map<String,Object> realMap = (Map<String,Object>) value;
+            Map realMap = (Map) value;
             Set<String> expectedKeys = map.keySet();
-            Set<String> actualKeys = realMap.keySet();
-            boolean match = expectedKeys.equals( actualKeys );
+            Set actualKeys = realMap.keySet();
 
-            if ( match )
+            if ( expectedKeys.equals( actualKeys ) )
             {
-                for ( String key : expectedKeys )
+                for ( Map.Entry<String,ValueMatcher> entry : map.entrySet() )
                 {
-                    match &= map.get( key ).matches( realMap.get( key ) );
+                    if ( !entry.getValue().matches( realMap.get( entry.getKey() ) ) )
+                    {
+                        return false;
+                    }
                 }
-                return match;
+                return true;
             }
         }
         return false;
@@ -59,6 +61,6 @@ public boolean matches( Object value )
     @Override
     public String toString()
     {
-        return "MapMatcher {" + map.toString();
+        return "MapMatcher" + map.toString();
     }
 }

community/cypher/compatibility-suite/src/main/java/cypher/feature/parser/matchers/Matcher.java

@@ -22,9 +22,4 @@
 public interface Matcher<T>
 {
     boolean matches( T value );
-
-    interface OrderedMatcher<T> extends Matcher<T>
-    {
-        boolean matchesOrdered( T value );
-    }
 }

community/cypher/compatibility-suite/src/main/java/cypher/feature/parser/matchers/NodeMatcher.java

@@ -41,26 +41,24 @@ public boolean matches( Object value )
         if ( value instanceof Node )
         {
             Node node = (Node) value;
-            Iterable<Label> labels = node.getLabels();
-            boolean match = true;
             int nbrOfLabels = 0;
-            for ( Label l : labels )
+            for ( Label l : node.getLabels() )
             {
-                match &= labelNames.contains( l.name() );
+                if ( !labelNames.contains( l.name() ) )
+                {
+                    return false;
+                }
                 ++nbrOfLabels;
             }
-            match &= nbrOfLabels == labelNames.size();
 
-            match &= propertyMatcher.matches( node.getAllProperties() );
-
-            return match;
+            return labelNames.size() == nbrOfLabels && propertyMatcher.matches( node.getAllProperties() );
         }
         return false;
     }
 
     @Override
     public String toString()
     {
-        return "NodeMatcher (" + labelNames + ") " + propertyMatcher;
+        return "NodeMatcher" + labelNames + propertyMatcher;
     }
 }

community/cypher/compatibility-suite/src/main/java/cypher/feature/parser/matchers/PathLinkMatcher.java

@@ -41,18 +41,23 @@ public boolean matches( Object value )
         if ( value instanceof Relationship )
         {
             Relationship real = (Relationship) value;
-            boolean matches = relMatcher.matches( real );
+            if ( !relMatcher.matches( real ) )
+            {
+                return false;
+            }
             if ( outgoing )
             {
-                matches &= leftNode.matches( real.getStartNode() );
-                matches &= rightNode.matches( real.getEndNode() );
+                if ( !leftNode.matches( real.getStartNode() ) || !rightNode.matches( real.getEndNode() ) )
+                {
+                    return false;
+                }
             }
-            else
+            // incoming
+            else if ( !leftNode.matches( real.getEndNode() ) || !rightNode.matches( real.getStartNode() ) )
             {
-                matches &= leftNode.matches( real.getEndNode() );
-                matches &= rightNode.matches( real.getStartNode() );
+                return false;
             }
-            return matches;
+            return true;
         }
         return false;
     }
@@ -65,9 +70,9 @@ public void setRightNode( NodeMatcher rightNode )
     @Override
     public String toString()
     {
-        return "PathLinkMatcher "
+        return "PathLinkMatcher <<"
                + leftNode + (outgoing ? "-" : "<-")
                + relMatcher + (outgoing ? "->" : "-")
-               + rightNode;
+               + rightNode + ">>";
     }
 }

community/cypher/compatibility-suite/src/main/java/cypher/feature/parser/matchers/PathMatcher.java

@@ -49,25 +49,28 @@ public boolean matches( Object value )
         if ( value instanceof Path )
         {
             Path path = (Path) value;
+            if ( path.length() != pathLinks.size() )
+            {
+                return false;
+            }
             if ( pathLinks.isEmpty() )
             {
-                boolean matches = path.length() == 0;
-                matches &= singleNodePath.matches( path.startNode() );
-                matches &= singleNodePath.matches( path.endNode() );
-                return matches;
+                return path.length() == 0
+                       && singleNodePath.matches( path.startNode() )
+                       && singleNodePath.matches( path.endNode() );
             }
             else
             {
-                boolean matches = path.length() == pathLinks.size();
-                if ( matches )
+                Iterator<Relationship> relationships = path.relationships().iterator();
+                for ( PathLinkMatcher pathLink : pathLinks )
                 {
-                    Iterator<Relationship> relationships = path.relationships().iterator();
-                    for ( PathLinkMatcher pathLink : pathLinks )
+                    Relationship next = relationships.next();
+                    if ( !pathLink.matches( next ) )
                     {
-                        pathLink.matches( relationships.next() );
+                        return false;
                     }
                 }
-                return matches;
+                return true;
             }
         }
         return false;
@@ -78,11 +81,11 @@ public String toString()
     {
         if ( pathLinks.isEmpty() )
         {
-            return "PathMatcher for " + singleNodePath;
+            return "PathMatcher" + singleNodePath;
         }
         else
         {
-            return "PathMatcher for " + pathLinks;
+            return "PathMatcher" + pathLinks;
         }
     }
 }

community/cypher/compatibility-suite/src/main/java/cypher/feature/parser/matchers/RelationshipMatcher.java

@@ -48,6 +48,6 @@ public boolean matches( Object value )
     @Override
     public String toString()
     {
-        return "RelationshipMatcher [" + relationshipTypeName + "] " + propertyMatcher;
+        return "RelationshipMatcher[" + relationshipTypeName + "]" + propertyMatcher;
     }
 }

community/cypher/compatibility-suite/src/main/java/cypher/feature/parser/matchers/ResultMatcher.java

@@ -25,7 +25,7 @@
 
 import org.neo4j.graphdb.Result;
 
-public class ResultMatcher implements Matcher<Result>, Matcher.OrderedMatcher<Result>
+public class ResultMatcher implements Matcher<Result>
 {
     private final List<RowMatcher> rowMatchers;
 
@@ -61,7 +61,6 @@ else if ( i == mutableCopy.size() - 1 )
         return nothingLeftInMatcher && nothingLeftInReal;
     }
 
-    @Override
     public boolean matchesOrdered( Result value )
     {
         boolean matches = true;

community/cypher/compatibility-suite/src/main/java/cypher/feature/parser/matchers/RowMatcher.java

@@ -35,20 +35,23 @@ public RowMatcher( Map<String,ValueMatcher> values )
     public boolean matches( Map<String,Object> value )
     {
         Set<String> keys = values.keySet();
-        boolean matches = keys.equals( value.keySet() );
-        if ( matches )
+        if ( keys.equals( value.keySet() ) )
         {
             for ( String key : keys )
             {
-                matches &= values.get( key ).matches( value.get( key ) );
+                if ( !values.get( key ).matches( value.get( key ) ) )
+                {
+                    return false;
+                }
             }
+            return true;
         }
-        return matches;
+        return false;
     }
 
     @Override
     public String toString()
     {
-        return "RowMatcher(" + values + ")";
+        return "RowMatcher" + values;
     }
 }

community/cypher/compatibility-suite/src/test/scala/cypher/feature/parser/ParsingTestSupport.scala

@@ -64,9 +64,10 @@ abstract class ParsingTestSupport extends FunSuite with Matchers with DecorateAs
   def path(relationships: Relationship*): Path = {
     val path = mock(classOf[Path])
     when(path.length()).thenReturn(relationships.length)
+    println(relationships.toIterable.asJava)
     when(path.relationships()).thenReturn(relationships.toIterable.asJava)
     // Mockito bug makes mocks unable to refer to other mocks in stubs; can't inline this
-    val pathString = s"<${relationships.mkString}>"
+    val pathString = s"<${relationships.mkString(",")}>"
     when(path.toString).thenReturn(pathString)
     path
   }

community/cypher/compatibility-suite/src/test/scala/cypher/feature/parser/matchers/ListMatcherTest.scala

@@ -44,4 +44,8 @@ class ListMatcherTest extends ParsingTestSupport {
     new ListMatcher(asList(new ListMatcher(asList(new IntegerMatcher(0), new BooleanMatcher(false))), new StringMatcher(""))) should accept(asList(asList(0L, false), ""))
   }
 
+  test("should not match different lists") {
+    new ListMatcher(asList(new IntegerMatcher(-1L))) shouldNot accept(asList(1L))
+  }
+
 }

community/cypher/compatibility-suite/src/test/scala/cypher/feature/parser/matchers/MapMatcherTest.scala

@@ -39,4 +39,8 @@ class MapMatcherTest extends ParsingTestSupport {
     new MapMatcher(Map("k" -> NULL_MATCHER, "k2" -> new BooleanMatcher(true)).asJava) shouldNot accept(Map("k" -> NULL_MATCHER))
   }
 
+  test("should not accept different maps") {
+    new MapMatcher(Map[String, ValueMatcher]("key" -> new StringMatcher("value1")).asJava) shouldNot accept(Map("key" -> "value2").asJava)
+  }
+
 }

community/cypher/compatibility-suite/src/test/scala/cypher/feature/parser/matchers/NodeMatcherTest.scala

@@ -53,4 +53,8 @@ class NodeMatcherTest extends ParsingTestSupport {
 
     matcher shouldNot accept(node(properties = Map("key" -> List("").asJava)))
   }
+
+  test("should not match a non-node") {
+    new NodeMatcher(Set.empty[String].asJava, MapMatcher.EMPTY) shouldNot accept("a string")
+  }
 }

community/cypher/compatibility-suite/src/test/scala/cypher/feature/parser/matchers/PathLinkMatcherTest.scala

@@ -45,4 +45,25 @@ class PathLinkMatcherTest extends ParsingTestSupport {
     matcher shouldNot accept(pathLink(node(Seq("L")), relationship("T"), node()))
   }
 
+  test("should not match a different relationship") {
+    val matcher = new PathLinkMatcher(new RelationshipMatcher("T1", EMPTY), new NodeMatcher(Set("L").asJava, EMPTY), true)
+    matcher.setRightNode(new NodeMatcher(Set.empty[String].asJava, EMPTY))
+
+    matcher shouldNot accept(pathLink(node(Seq("L")), relationship("T2"), node()))
+  }
+
+  test("should not match a different startNode") {
+    val matcher = new PathLinkMatcher(new RelationshipMatcher("T", EMPTY), new NodeMatcher(Set("L1").asJava, EMPTY), true)
+    matcher.setRightNode(new NodeMatcher(Set.empty[String].asJava, EMPTY))
+
+    matcher shouldNot accept(pathLink(node(Seq("L2")), relationship("T"), node()))
+  }
+
+  test("should not match a non-relationship") {
+    val matcher = new PathLinkMatcher(new RelationshipMatcher("T", EMPTY), new NodeMatcher(Set("L1").asJava, EMPTY), true)
+    matcher.setRightNode(new NodeMatcher(Set.empty[String].asJava, EMPTY))
+
+    matcher shouldNot accept("a string")
+  }
+
 }

community/cypher/compatibility-suite/src/test/scala/cypher/feature/parser/matchers/PathMatcherTest.scala

@@ -71,4 +71,17 @@ class PathMatcherTest extends ParsingTestSupport {
     pathMatcher shouldNot accept(expected)
   }
 
+  test("should not match a path with wrong link") {
+    val linkMatcher1 = new PathLinkMatcher(new RelationshipMatcher("T1", EMPTY),
+                                           new NodeMatcher(Set.empty[String].asJava, EMPTY), true)
+    linkMatcher1.setRightNode(new NodeMatcher(Set.empty[String].asJava, EMPTY))
+    val pathMatcher = new PathMatcher(List(linkMatcher1).asJava)
+
+    pathMatcher shouldNot accept(path(pathLink(node(), relationship("T2"), node())))
+  }
+
+  test("should not match a non-path") {
+    new PathMatcher(new NodeMatcher(Set("L").asJava, EMPTY)) shouldNot accept("a string")
+  }
+
 }