HV-771, HV-772 Making sure the configuration from highest interface o…

…r superclass is used to determine execution type

engine/src/main/java/org/hibernate/validator/internal/cdi/ValidationExtension.java

@@ -22,6 +22,9 @@
 import java.lang.reflect.Method;
 import java.util.Collections;
 import java.util.EnumSet;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
 import java.util.Set;
 import javax.enterprise.event.Observes;
 import javax.enterprise.inject.Default;
@@ -202,12 +205,12 @@ private <T> Set<AnnotatedCallable<? super T>> determineConstrainedCallables(Anno
 	private <T> void determineConstrainedMethod(AnnotatedType<T> type,
 												BeanDescriptor beanDescriptor,
 												Set<AnnotatedCallable<? super T>> callables) {
-		Set<Method> interfaceMethods = ReflectionHelper.computeAllImplementedMethods( type.getJavaClass() );
+		List<Method> overriddenAndImplementedMethods = ReflectionHelper.computeAllOverridenAndImplementedMethods( type.getJavaClass() );
 		for ( AnnotatedMethod<? super T> annotatedMethod : type.getMethods() ) {
 			Method method = annotatedMethod.getJavaMember();
 
 			// if the method implements an interface we need to use the configuration of the interface
-			method = replaceWithInterfaceMethod( method, interfaceMethods );
+			method = replaceWithOverriddenOrInterfaceMethod( method, overriddenAndImplementedMethods );
 			boolean isGetter = ReflectionHelper.isGetterMethod( method );
 
 			EnumSet<ExecutableType> classLevelExecutableTypes = executableTypesDefinedOnType( method.getDeclaringClass() );
@@ -377,12 +380,16 @@ private EnumSet<ExecutableType> commonExecutableTypeChecks(ValidateOnExecution v
 		return executableTypes;
 	}
 
-	public Method replaceWithInterfaceMethod(Method method, Set<Method> interfaceMethods) {
-		for ( Method interfaceMethod : interfaceMethods ) {
+	public Method replaceWithOverriddenOrInterfaceMethod(Method method, List<Method> interfaceMethods) {
+		LinkedList<Method> list = new LinkedList<Method>( interfaceMethods );
+		Iterator<Method> iterator = list.descendingIterator();
+		while ( iterator.hasNext() ) {
+			Method interfaceMethod = iterator.next();
 			if ( ReflectionHelper.overrides( method, interfaceMethod ) ) {
 				return interfaceMethod;
 			}
 		}
+
 		return method;
 	}
 }

engine/src/main/java/org/hibernate/validator/internal/util/ReflectionHelper.java

@@ -32,7 +32,6 @@
 import java.security.PrivilegedAction;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -758,14 +757,13 @@ private static <T> void computeClassHierarchy(Class<? super T> clazz, List<Class
 	 *
 	 * @param clazz The class for which to find the interface methods
 	 *
-	 * @return Set of all methods {@code clazz} implements. The empty list is returned if {@code clazz} does not
-	 *         implement any interfaces or {@code clazz} is {@code null}
+	 * @return returns set of all methods {@code clazz} implements. The empty list is returned if {@code clazz} is {@code null}
 	 */
-	public static Set<Method> computeAllImplementedMethods(Class<?> clazz) {
-		Set<Method> methods = newHashSet();
+	public static <T> List<Method> computeAllOverridenAndImplementedMethods(Class<T> clazz) {
+		List<Method> methods = newArrayList();
 
-		Set<Class<?>> interfaces = computeAllImplementedInterfaces( clazz );
-		for ( Class<?> interfaceClass : interfaces ) {
+		List<Class<? super T>> interfacesAndSuperTypes = computeClassHierarchy( clazz, true );
+		for ( Class<?> interfaceClass : interfacesAndSuperTypes ) {
 			Collections.addAll( methods, interfaceClass.getMethods() );
 		}
 
@@ -778,10 +776,10 @@ public static Set<Method> computeAllImplementedMethods(Class<?> clazz) {
 	 * @param clazz The class for which to find the interfaces
 	 *
 	 * @return Set of all interfaces {@code clazz} implements. The empty list is returned if {@code clazz} does not
-	 *         implement any interfaces or {@code clazz} is {@code null}
+	 *         implement any interfaces or {@code clazz} is {@code null}.
 	 */
 	public static Set<Class<?>> computeAllImplementedInterfaces(Class<?> clazz) {
-		Set<Class<?>> classes = new HashSet<Class<?>>();
+		Set<Class<?>> classes = newHashSet();
 		computeAllImplementedInterfaces( clazz, classes );
 		return classes;
 	}

engine/src/test/java/org/hibernate/validator/test/internal/util/ReflectionHelperTest.java

@@ -25,7 +25,6 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.SortedMap;
 import java.util.TreeSet;
 import javax.validation.Payload;
@@ -180,13 +179,11 @@ public Class<? extends Annotation> annotationType() {
 
 	@Test
 	public void testComputeAllImplementedMethods() throws Exception {
-		assertTrue( ReflectionHelper.computeAllImplementedMethods( null ).isEmpty() );
-		assertTrue( ReflectionHelper.computeAllImplementedMethods( Foo.class ).isEmpty() );
+		assertTrue( ReflectionHelper.computeAllOverridenAndImplementedMethods( null ).isEmpty() );
 
-		Set<Method> interfaceMethods = ReflectionHelper.computeAllImplementedMethods( Fubar.class );
-		assertEquals( interfaceMethods.size(), 2, "There should be two implemented methods. One from each interface." );
-		assertTrue( interfaceMethods.contains( Snafu.class.getMethod( "snafu" ) ) );
-		assertTrue( interfaceMethods.contains( Susfu.class.getMethod( "susfu" ) ) );
+		List<Method> methods = ReflectionHelper.computeAllOverridenAndImplementedMethods( Fubar.class );
+		assertTrue( methods.contains( Snafu.class.getMethod( "snafu" ) ) );
+		assertTrue( methods.contains( Susfu.class.getMethod( "susfu" ) ) );
 	}
 
 	@Test

integration/weld/src/test/java/org/hibernate/validator/integration/cdi/methodvalidation/getter/DeliveryService.java

@@ -1,3 +1,19 @@
+/*
+* JBoss, Home of Professional Open Source
+* Copyright 2013, Red Hat, Inc. and/or its affiliates, and individual contributors
+* by the @authors tag. See the copyright.txt in the distribution for a
+* full listing of individual contributors.
+*
+* Licensed under the Apache License, Version 2.0 (the "License");
+* you may not use this file except in compliance with the License.
+* You may obtain a copy of the License at
+* http://www.apache.org/licenses/LICENSE-2.0
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
 package org.hibernate.validator.integration.cdi.methodvalidation.getter;
 
 import javax.validation.constraints.NotNull;

integration/weld/src/test/java/org/hibernate/validator/integration/cdi/methodvalidation/inheritance/ClassInheritanceMethodValidationTest.java

@@ -0,0 +1,68 @@
+/*
+* JBoss, Home of Professional Open Source
+* Copyright 2013, Red Hat, Inc. and/or its affiliates, and individual contributors
+* by the @authors tag. See the copyright.txt in the distribution for a
+* full listing of individual contributors.
+*
+* Licensed under the Apache License, Version 2.0 (the "License");
+* you may not use this file except in compliance with the License.
+* You may obtain a copy of the License at
+* http://www.apache.org/licenses/LICENSE-2.0
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
+package org.hibernate.validator.integration.cdi.methodvalidation.inheritance;
+
+import javax.inject.Inject;
+import javax.validation.ConstraintViolationException;
+import javax.validation.constraints.NotNull;
+
+import org.jboss.arquillian.container.test.api.Deployment;
+import org.jboss.arquillian.junit.Arquillian;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.asset.EmptyAsset;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import static junit.framework.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+/**
+ * @author Hardy Ferentschik
+ */
+@RunWith(Arquillian.class)
+public class ClassInheritanceMethodValidationTest {
+
+	@Deployment
+	public static JavaArchive createDeployment() {
+		return ShrinkWrap.create( JavaArchive.class )
+				.addClass( SecretService.class )
+				.addClass( SecretServiceBase.class )
+				.addAsManifestResource( EmptyAsset.INSTANCE, "beans.xml" );
+	}
+
+	@Inject
+	SecretService secretService;
+
+	@Test
+	public void testOverriddenExecutionTypeIsConsidered() {
+		try {
+			secretService.whisper( null );
+			fail( "Method invocation should have caused a ConstraintViolationException" );
+		}
+		catch ( ConstraintViolationException e ) {
+			assertEquals(
+					e.getConstraintViolations()
+							.iterator()
+							.next()
+							.getConstraintDescriptor()
+							.getAnnotation()
+							.annotationType(), NotNull.class
+			);
+		}
+	}
+}

integration/weld/src/test/java/org/hibernate/validator/integration/cdi/methodvalidation/inheritance/MultipleInterfaceInheritanceMethodValidationTest.java

@@ -0,0 +1,70 @@
+/*
+* JBoss, Home of Professional Open Source
+* Copyright 2013, Red Hat, Inc. and/or its affiliates, and individual contributors
+* by the @authors tag. See the copyright.txt in the distribution for a
+* full listing of individual contributors.
+*
+* Licensed under the Apache License, Version 2.0 (the "License");
+* you may not use this file except in compliance with the License.
+* You may obtain a copy of the License at
+* http://www.apache.org/licenses/LICENSE-2.0
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
+package org.hibernate.validator.integration.cdi.methodvalidation.inheritance;
+
+import javax.inject.Inject;
+import javax.validation.ConstraintViolationException;
+import javax.validation.constraints.NotNull;
+
+import org.jboss.arquillian.container.test.api.Deployment;
+import org.jboss.arquillian.junit.Arquillian;
+import org.jboss.shrinkwrap.api.ShrinkWrap;
+import org.jboss.shrinkwrap.api.asset.EmptyAsset;
+import org.jboss.shrinkwrap.api.spec.JavaArchive;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import static junit.framework.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+/**
+ * @author Hardy Ferentschik
+ */
+@RunWith(Arquillian.class)
+public class MultipleInterfaceInheritanceMethodValidationTest {
+
+	@Deployment
+	public static JavaArchive createDeployment() {
+		return ShrinkWrap.create( JavaArchive.class )
+				.addClass( ShipmentServiceFirstInHierarchy.class )
+				.addClass( ShipmentServiceSecondInHierarchy.class )
+				.addClass( Shipment.class )
+				.addClass( ShipmentServiceImpl.class )
+				.addAsManifestResource( EmptyAsset.INSTANCE, "beans.xml" );
+	}
+
+	@Inject
+	ShipmentServiceSecondInHierarchy shipmentService;
+
+	@Test
+	public void testExecutableValidationUsesSettingFromHighestMethodInHierarchy() throws Exception {
+		try {
+			shipmentService.getShipment();
+			fail( "Method invocation should have caused a ConstraintViolationException" );
+		}
+		catch ( ConstraintViolationException e ) {
+			assertEquals(
+					e.getConstraintViolations()
+							.iterator()
+							.next()
+							.getConstraintDescriptor()
+							.getAnnotation()
+							.annotationType(), NotNull.class
+			);
+		}
+	}
+}

integration/weld/src/test/java/org/hibernate/validator/integration/cdi/methodvalidation/inheritance/SecretService.java

@@ -0,0 +1,35 @@
+/*
+* JBoss, Home of Professional Open Source
+* Copyright 2013, Red Hat, Inc. and/or its affiliates, and individual contributors
+* by the @authors tag. See the copyright.txt in the distribution for a
+* full listing of individual contributors.
+*
+* Licensed under the Apache License, Version 2.0 (the "License");
+* you may not use this file except in compliance with the License.
+* You may obtain a copy of the License at
+* http://www.apache.org/licenses/LICENSE-2.0
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
+package org.hibernate.validator.integration.cdi.methodvalidation.inheritance;
+
+import javax.validation.constraints.NotNull;
+import javax.validation.executable.ExecutableType;
+import javax.validation.executable.ValidateOnExecution;
+
+/**
+ * @author Hardy Ferentschik
+ */
+public class SecretService extends SecretServiceBase {
+
+	// The @ValidateOnExecution annotations here are expected to be ignored
+	// since the methods override super-type methods
+
+	@ValidateOnExecution(type = ExecutableType.NONE)
+	@Override
+	public void whisper(@NotNull String secret) {
+	}
+}

integration/weld/src/test/java/org/hibernate/validator/integration/cdi/methodvalidation/inheritance/SecretServiceBase.java

@@ -0,0 +1,31 @@
+/*
+* JBoss, Home of Professional Open Source
+* Copyright 2013, Red Hat, Inc. and/or its affiliates, and individual contributors
+* by the @authors tag. See the copyright.txt in the distribution for a
+* full listing of individual contributors.
+*
+* Licensed under the Apache License, Version 2.0 (the "License");
+* you may not use this file except in compliance with the License.
+* You may obtain a copy of the License at
+* http://www.apache.org/licenses/LICENSE-2.0
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
+package org.hibernate.validator.integration.cdi.methodvalidation.inheritance;
+
+import javax.validation.constraints.NotNull;
+import javax.validation.executable.ExecutableType;
+import javax.validation.executable.ValidateOnExecution;
+
+/**
+ * @author Hardy Ferentschik
+ */
+@ValidateOnExecution(type = ExecutableType.ALL)
+public class SecretServiceBase {
+
+	public void whisper(@NotNull String id) {
+	}
+}

integration/weld/src/test/java/org/hibernate/validator/integration/cdi/methodvalidation/inheritance/Shipment.java

@@ -0,0 +1,32 @@
+/*
+* JBoss, Home of Professional Open Source
+* Copyright 2013, Red Hat, Inc. and/or its affiliates, and individual contributors
+* by the @authors tag. See the copyright.txt in the distribution for a
+* full listing of individual contributors.
+*
+* Licensed under the Apache License, Version 2.0 (the "License");
+* you may not use this file except in compliance with the License.
+* You may obtain a copy of the License at
+* http://www.apache.org/licenses/LICENSE-2.0
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
+package org.hibernate.validator.integration.cdi.methodvalidation.inheritance;
+
+import javax.validation.constraints.NotNull;
+
+/**
+ * @author Gunnar Morling
+ */
+public class Shipment {
+
+	@NotNull
+	private String name;
+
+	public String getName() {
+		return name;
+	}
+}

integration/weld/src/test/java/org/hibernate/validator/integration/cdi/methodvalidation/inheritance/ShipmentServiceFirstInHierarchy.java

@@ -0,0 +1,25 @@
+/*
+* JBoss, Home of Professional Open Source
+* Copyright 2013, Red Hat, Inc. and/or its affiliates, and individual contributors
+* by the @authors tag. See the copyright.txt in the distribution for a
+* full listing of individual contributors.
+*
+* Licensed under the Apache License, Version 2.0 (the "License");
+* you may not use this file except in compliance with the License.
+* You may obtain a copy of the License at
+* http://www.apache.org/licenses/LICENSE-2.0
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/
+package org.hibernate.validator.integration.cdi.methodvalidation.inheritance;
+
+/**
+ * @author Hardy Ferentschik
+ */
+public interface ShipmentServiceFirstInHierarchy extends ShipmentServiceSecondInHierarchy {
+	@Override
+	Shipment getShipment();
+}