HV-1993 reduces calls to isSubPathOf/rework isSubPathOf logic

engine/src/main/java/org/hibernate/validator/internal/engine/path/PathImpl.java

@@ -453,4 +453,40 @@ private static boolean isValidJavaIdentifier(String identifier) {
 		}
 		return true;
 	}
+
+	/**
+	 * checks if this PathImpl is a subpath of <code>other</code>.
+	 * @param other the path to compare with
+	 * @return true, if this path is a subpath
+	 */
+	public boolean isSubPathOf( PathImpl other ) {
+		if ( nodeList.size() > other.nodeList.size() ) {
+			// cannot be a subpath as it is already longer then the other path
+			return false;
+		}
+
+		for ( int i = 0; i < nodeList.size(); i++ ) {
+			if ( !nodeList.get( i ).equals( other.nodeList.get( i ) ) ) {
+				return false;
+			}
+		}
+		return true;
+	}
+
+	public boolean isSubPathOrContains(PathImpl other) {
+
+		// prefetch contant return values
+		int oSize = other.nodeList.size();
+		// calling Math.min will reduce speed significantly
+		int mySize = nodeList.size() < oSize
+						? nodeList.size()
+						: oSize;
+
+		for ( int i = 0; i < mySize; i++ ) {
+			if ( !nodeList.get( i ).equals( other.nodeList.get( i ) ) ) {
+				return false;
+			}
+		}
+		return true;
+	}
 }

engine/src/main/java/org/hibernate/validator/internal/engine/validationcontext/AbstractValidationContext.java

@@ -338,31 +338,23 @@ private boolean isAlreadyValidatedForPath(Object value, PathImpl path) {
 			return false;
 		}
 
+		if ( path.isRootPath() ) {
+			return true;
+		}
+
+		// Since this isAlreadyValidatedForPath(..) is only applicable for an object that is about to be cascaded into,
+		// it means that the new path we are testing cannot be a root path; also since we are cascading into inner
+		// objects, i.e. going further from the object tree root, it means that the new path cannot be shorter than
+		// the ones we've already encountered.
 		for ( PathImpl p : pathSet ) {
-			if ( path.isRootPath() || p.isRootPath() || isSubPathOf( path, p ) || isSubPathOf( p, path ) ) {
+			if ( p.isSubPathOrContains( path ) ) {
 				return true;
 			}
 		}
 
 		return false;
 	}
 
-	private boolean isSubPathOf(Path p1, Path p2) {
-		Iterator<Path.Node> p1Iter = p1.iterator();
-		Iterator<Path.Node> p2Iter = p2.iterator();
-		while ( p1Iter.hasNext() ) {
-			Path.Node p1Node = p1Iter.next();
-			if ( !p2Iter.hasNext() ) {
-				return false;
-			}
-			Path.Node p2Node = p2Iter.next();
-			if ( !p1Node.equals( p2Node ) ) {
-				return false;
-			}
-		}
-		return true;
-	}
-
 	private boolean isAlreadyValidatedForCurrentGroup(Object value, Class<?> group) {
 		return getInitializedProcessedGroupUnits().contains( new BeanGroupProcessedUnit( value, group ) );
 	}

engine/src/test/java/org/hibernate/validator/test/internal/engine/path/PathImplTest.java

@@ -166,6 +166,47 @@ public void testEmptyString() {
 		assertTrue( path.iterator().hasNext() );
 	}
 
+    @Test
+	public void testIsSubPathOf() {
+		PathImpl subPath = PathImpl.createPathFromString( "annotation" );
+		PathImpl middlePath = PathImpl.createPathFromString( "annotation.property" );
+		PathImpl middlePath2 = PathImpl.createPathFromString( "annotation.property[2]" );
+		PathImpl middlePath3 = PathImpl.createPathFromString( "annotation.property[3]" );
+		PathImpl fullPath3 = PathImpl.createPathFromString( "annotation.property[3].element" );
+		PathImpl fullPath4 = PathImpl.createPathFromString( "annotation.property[4].element" );
+
+		assertTrue( subPath.isSubPathOf( middlePath ), "bean is subpath of its properties" );
+		assertFalse( middlePath.isSubPathOf( subPath ), "a property is not a subPath of its bean" );
+		assertTrue( subPath.isSubPathOf( fullPath3 ), "bean is subpath of its tree" );
+		assertTrue( subPath.isSubPathOf( fullPath4 ), "bean is subpath of its tree, for every array index" );
+		assertFalse( middlePath.isSubPathOf( fullPath3 ), "property is not a subpath of an array" );
+		assertFalse( middlePath3.isSubPathOf( fullPath3 ), "array property is not a subpath of a property of a bean in an array" );
+		assertFalse( middlePath2.isSubPathOf( fullPath3 ), "array element is not a subpath of another element's children" );
+		assertFalse( fullPath3.isSubPathOf( fullPath4 ), "different array elements are not subpaths of the other" );
+	}
+
+	@Test
+	public void testIsSubPathOrContains() {
+		PathImpl rootPath = PathImpl.createPathFromString( "" );
+		PathImpl subPath = PathImpl.createPathFromString( "annotation" );
+		PathImpl middlePath = PathImpl.createPathFromString( "annotation.property" );
+		PathImpl middlePath2 = PathImpl.createPathFromString( "annotation.property[2]" );
+		PathImpl middlePath3 = PathImpl.createPathFromString( "annotation.property[3]" );
+		PathImpl fullPath3 = PathImpl.createPathFromString( "annotation.property[3].element" );
+		PathImpl fullPath4 = PathImpl.createPathFromString( "annotation.property[4].element" );
+
+		assertTrue( rootPath.isSubPathOrContains( middlePath ), "root path is in every path" );
+		assertTrue( middlePath.isSubPathOrContains( rootPath ), "every path contains the root path" );
+		assertTrue( subPath.isSubPathOrContains( middlePath ), "bean is subpath of its properties" );
+		assertTrue( middlePath.isSubPathOrContains( subPath ), "a property is an extension of its bean" );
+		assertTrue( subPath.isSubPathOrContains( fullPath3 ), "bean is subpath of its tree" );
+		assertTrue( subPath.isSubPathOrContains( fullPath4 ), "bean is subpath of its tree, for every array index" );
+		assertFalse( middlePath.isSubPathOrContains( fullPath3 ), "property is not a subpath of an array" );
+		assertFalse( middlePath3.isSubPathOrContains( fullPath3 ), "array property is not a subpath of a property of a bean in an array" );
+		assertFalse( middlePath2.isSubPathOrContains( fullPath3 ), "array element is not a subpath of another element's children" );
+		assertFalse( fullPath3.isSubPathOrContains( fullPath4 ), "different array elements are not subpaths of the other" );
+	}
+
 	@Test
 	public void testNonStringMapKey() {
 		Validator validator = ValidatorUtil.getValidator();