Skip to content
Permalink
Browse files
BVAL-167: don't be fooled by inclusion of constrained interface at mu…
…ltiple hierarchy levels
  • Loading branch information
mbenson committed Feb 26, 2019
1 parent 43935b4 commit 3c5b3bf8ba263d5164f35f2100818c045f0dd293
Showing 2 changed files with 49 additions and 31 deletions.
@@ -52,13 +52,19 @@ private enum StrengtheningIssue implements Predicate<Map<Meta<?>, Set<Validation

@Override
public boolean test(Map<Meta<?>, Set<ValidationElement>> detectedValidationElements) {
boolean lowestFound = false;

for (Set<ValidationElement> validated : detectedValidationElements.values()) {
if (lowestFound) {
Class<?> declaringType = null;

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 @@ public boolean test(Map<Meta<?>, Set<ValidationElement>> detectedValidationEleme

@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 @@ static void validateContainerHierarchy(Collection<? extends ContainerDelegate<?>

// 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));

@@ -18,42 +18,36 @@
*/
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);
}
}

public interface Api {
String read(@NotNull String key);
}

public static abstract class Base {
public String read(final String key) {
return null;
}
public interface Api2 extends Api {
@Override
String read(String key);
}

public static class Impl extends Base implements Api {
public static abstract class Base implements Api {
@Override
public String read(final String key) {
return super.read(key);
return null;
}
}

public static class Impl extends Base implements Api2 {
}
}

0 comments on commit 3c5b3bf

Please sign in to comment.