BVAL-167: don't be fooled by inclusion of constrained interface at multiple hierarchy levels
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/Liskov.java b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/Liskov.java
index 18dfadc..d286898 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/Liskov.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/Liskov.java
@@ -52,13 +52,19 @@
@Override
public boolean test(Map<Meta<?>, Set<ValidationElement>> detectedValidationElements) {
- boolean lowestFound = false;
+ Class<?> declaringType = null;
- for (Set<ValidationElement> validated : detectedValidationElements.values()) {
- if (lowestFound) {
+ for (Map.Entry<Meta<?>, Set<ValidationElement>> e : detectedValidationElements.entrySet()){
+ final Class<?> t = e.getKey().getDeclaringClass();
+ if (declaringType != null) {
+ if (declaringType.isAssignableFrom(t)) {
+ continue;
+ }
return false;
}
- lowestFound = !validated.isEmpty();
+ if (!e.getValue().isEmpty()){
+ declaringType = t;
+ }
}
return true;
}
@@ -67,16 +73,34 @@
@Override
public boolean test(Map<Meta<?>, Set<ValidationElement>> detectedValidationElements) {
- final Set<Class<?>> interfaces = detectedValidationElements.keySet().stream().map(Meta::getDeclaringClass)
- .filter(Class::isInterface).collect(Collectors.toSet());
- if (interfaces.isEmpty()) {
+ if (detectedValidationElements.size() < 2) {
+ // no unrelated hierarchy possible
return true;
}
- final boolean allRelated =
- detectedValidationElements.keySet().stream().map(Meta::getDeclaringClass).allMatch(ifc -> interfaces
- .stream().filter(Predicate.isEqual(ifc).negate()).allMatch(ifc2 -> related(ifc, ifc2)));
-
- return allRelated;
+ final Map<Class<?>, Set<ValidationElement>> interfaceValidation = new LinkedHashMap<>();
+ detectedValidationElements.forEach((k,v)->{
+ final Class<?> t = k.getDeclaringClass();
+ if (t.isInterface()){
+ interfaceValidation.put(t, v);
+ }
+ });
+ if (interfaceValidation.isEmpty()) {
+ // if all are classes, there can be no unrelated types in the hierarchy:
+ return true;
+ }
+ // verify that all types can be assigned to the constrained interfaces:
+ for (Meta<?> meta : detectedValidationElements.keySet()) {
+ final Class<?> t = meta.getDeclaringClass();
+ for (Map.Entry<Class<?>, Set<ValidationElement>> e : interfaceValidation.entrySet()) {
+ if (t.equals(e.getKey()) || e.getValue().isEmpty()) {
+ continue;
+ }
+ if (!e.getKey().isAssignableFrom(t)) {
+ return false;
+ }
+ }
+ }
+ return true;
}
};
//@formatter:on
@@ -122,7 +146,7 @@
// pre-check return value overridden hierarchy:
Stream.of(StrengtheningIssue.values())
- .filter((Predicate<? super StrengtheningIssue>) si -> !(si == StrengtheningIssue.overriddenHierarchy
+ .filter(si -> !(si == StrengtheningIssue.overriddenHierarchy
&& detectedValidationElements.values().stream().filter(s -> !s.isEmpty()).count() < 2))
.forEach(si -> si.check(detectedValidationElements));
diff --git a/bval-jsr/src/test/java/org/apache/bval/jsr/LiskovTest.java b/bval-jsr/src/test/java/org/apache/bval/jsr/LiskovTest.java
index 9f40d0c..9f52f51 100644
--- a/bval-jsr/src/test/java/org/apache/bval/jsr/LiskovTest.java
+++ b/bval-jsr/src/test/java/org/apache/bval/jsr/LiskovTest.java
@@ -18,25 +18,17 @@
*/
package org.apache.bval.jsr;
-import java.lang.reflect.Method;
-
import javax.validation.Validation;
import javax.validation.ValidatorFactory;
import javax.validation.constraints.NotNull;
-import javax.validation.executable.ExecutableValidator;
-import org.junit.Ignore;
import org.junit.Test;
-@Ignore("requires some revisiting of Liskov impl - discuss in progress")
public class LiskovTest {
- @Test // this test was throwing a Liskov exception, here to ensure it is not the case
- public void validateParams() throws NoSuchMethodException {
- final Api service = new Impl();
+ @Test
+ public void testBVal167() {
try (final ValidatorFactory factory = Validation.buildDefaultValidatorFactory()) {
- final ExecutableValidator validator = factory.getValidator().forExecutables();
- final Method method = Api.class.getMethod("read", String.class);
- validator.validateParameters(service, method, new Object[]{""});
+ factory.getValidator().getConstraintsForClass(Impl.class);
}
}
@@ -44,16 +36,18 @@
String read(@NotNull String key);
}
- public static abstract class Base {
+ public interface Api2 extends Api {
+ @Override
+ String read(String key);
+ }
+
+ public static abstract class Base implements Api {
+ @Override
public String read(final String key) {
return null;
}
}
- public static class Impl extends Base implements Api {
- @Override
- public String read(final String key) {
- return super.read(key);
- }
+ public static class Impl extends Base implements Api2 {
}
}
