Skip to content
Permalink
Browse files
rework use of groups, introducing GroupStrategy interface + implement…
…ations. Passes 1 additional TCK test AND allows validation of non-sequential groups without making multiple passes over the graph
  • Loading branch information
mbenson committed Oct 16, 2018
1 parent ecb4f74 commit 395f6df566c3376dff4aa83aea01e6ad57588024
Show file tree
Hide file tree
Showing 16 changed files with 649 additions and 227 deletions.
@@ -21,6 +21,7 @@
import java.lang.reflect.Type;
import java.lang.reflect.TypeVariable;
import java.util.Map;
import java.util.Objects;

import javax.validation.Path;
import javax.validation.ValidationException;
@@ -100,6 +101,23 @@ public GraphContext getParent() {
public String toString() {
return String.format("%s: %s at '%s'", getClass().getSimpleName(), value, path);
}

@Override
public boolean equals(Object obj) {
if (obj == this) {
return true;
}
if (obj == null || !obj.getClass().equals(getClass())) {
return false;
}
final GraphContext other = (GraphContext) obj;
return other.validatorContext == validatorContext && other.value == value && other.getPath().equals(path);
}

@Override
public int hashCode() {
return Objects.hash(validatorContext, value, path);
}

public ContainerElementKey runtimeKey(ContainerElementKey key) {
Validate.notNull(key);
@@ -20,7 +20,6 @@

import java.lang.reflect.Type;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
@@ -31,6 +30,7 @@
import javax.validation.metadata.MethodType;
import javax.validation.metadata.PropertyDescriptor;

import org.apache.bval.jsr.groups.GroupStrategy;
import org.apache.bval.jsr.metadata.Signature;
import org.apache.bval.jsr.util.ToUnmodifiable;
import org.apache.bval.util.Exceptions;
@@ -39,17 +39,17 @@
public class BeanD<T> extends ElementD<Class<T>, MetadataReader.ForBean<T>> implements BeanDescriptor {

private final Class<T> beanClass;
private final List<Class<?>> groupSequence;
private final Map<String, PropertyDescriptor> propertiesMap;
private final Set<PropertyDescriptor> properties;
private final Map<Signature, ConstructorD<T>> constructors;
private final Map<Signature, MethodD> methods;
private final GroupStrategy groupStrategy;

BeanD(MetadataReader.ForBean<T> reader) {
super(reader);
this.beanClass = reader.meta.getHost();

groupSequence = reader.getGroupSequence();
groupStrategy = reader.getGroupStrategy();
propertiesMap = reader.getProperties(this);
properties = propertiesMap.values().stream().filter(DescriptorManager::isConstrained).collect(ToUnmodifiable.set());
constructors = reader.getConstructors(this);
@@ -112,8 +112,8 @@ protected BeanD<T> getBean() {
}

@Override
public List<Class<?>> getGroupSequence() {
return groupSequence;
public GroupStrategy getGroupStrategy() {
return groupStrategy;
}

public final Type getGenericType() {
@@ -23,7 +23,6 @@
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Executable;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -38,7 +37,6 @@
import javax.validation.Payload;
import javax.validation.ReportAsSingleViolation;
import javax.validation.ValidationException;
import javax.validation.groups.Default;
import javax.validation.metadata.ConstraintDescriptor;
import javax.validation.metadata.Scope;
import javax.validation.metadata.ValidateUnwrappedValue;
@@ -91,7 +89,7 @@ public ConstraintD(A annotation, Scope scope, Meta<?> meta, ApacheValidatorFacto
this.meta = Validate.notNull(meta, "meta");

payload = computePayload();
groups = computeGroups();
groups = set(() -> read(ConstraintAnnotationAttributes.GROUPS, Optionality.REQUIRED));
reportAsSingle = annotation.annotationType().isAnnotationPresent(ReportAsSingleViolation.class);
valueUnwrapping = computeValidateUnwrappedValue();
attributes = AnnotationsManager.readAttributes(annotation);
@@ -218,14 +216,6 @@ private ValidateUnwrappedValue computeValidateUnwrappedValue() {
return skip ? ValidateUnwrappedValue.SKIP : ValidateUnwrappedValue.DEFAULT;
}

private Set<Class<?>> computeGroups() {
final Class<?>[] groups = read(ConstraintAnnotationAttributes.GROUPS, Optionality.REQUIRED);
if (groups.length == 0) {
return Collections.singleton(Default.class);
}
return set(() -> groups);
}

private Set<Class<? extends Payload>> computePayload() {
final Set<Class<? extends Payload>> result =
set(() -> read(ConstraintAnnotationAttributes.PAYLOAD, Optionality.REQUIRED));
@@ -43,6 +43,12 @@ public static <D extends ElementDescriptor & CascadableDescriptor & ContainerDes
|| !descriptor.getConstrainedContainerElementTypes().isEmpty());
}

public static <D extends ElementDescriptor & CascadableDescriptor & ContainerDescriptor> boolean isCascaded(
D descriptor) {
return descriptor != null && (descriptor.isCascaded()
|| descriptor.getConstrainedContainerElementTypes().stream().anyMatch(DescriptorManager::isCascaded));
}

public static <E extends ExecutableDescriptor> boolean isConstrained(E descriptor) {
return descriptor != null && (descriptor.hasConstrainedParameters() || descriptor.hasConstrainedReturnValue());
}
@@ -22,13 +22,13 @@
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Type;
import java.lang.reflect.TypeVariable;
import java.util.List;
import java.util.Map;
import java.util.Set;

import javax.validation.metadata.ConstraintDescriptor;
import javax.validation.metadata.ElementDescriptor;

import org.apache.bval.jsr.groups.GroupStrategy;
import org.apache.bval.jsr.groups.GroupsComputer;
import org.apache.bval.jsr.metadata.Meta;
import org.apache.bval.util.Validate;
@@ -67,8 +67,8 @@ final protected BeanD<?> getBean() {
}

@Override
public final List<Class<?>> getGroupSequence() {
return getBean().getGroupSequence();
public final GroupStrategy getGroupStrategy() {
return getBean().getGroupStrategy();
}
}

@@ -117,7 +117,7 @@ public final Class<?> getDeclaringClass() {

public abstract Type getGenericType();

public abstract List<Class<?>> getGroupSequence();
public abstract GroupStrategy getGroupStrategy();

@Override
public String toString() {
@@ -48,7 +48,8 @@

class Finder implements ConstraintFinder {
private static Stream<Group> allGroups(Groups groups) {
return Stream.concat(groups.getGroups().stream(), groups.getSequences().stream().flatMap(Collection::stream));
return Stream.concat(groups.getGroups().stream(),
groups.getSequences().stream().map(Group.Sequence::getGroups).flatMap(Collection::stream));
}

private volatile Predicate<ConstraintD<?>> groups = c -> true;
@@ -118,7 +119,9 @@ private ElementD<?,?> firstAtomicElementDescriptor() {

private Groups computeDefaultSequence() {
final ElementD<?, ?> element = firstAtomicElementDescriptor();
Collection<Class<?>> redef = element.getGroupSequence();
Collection<Class<?>> redef =
element.getGroupStrategy().getGroups().stream().map(Group::getGroup).collect(Collectors.toList());

if (redef == null) {
return GroupsComputer.DEFAULT_GROUPS;
}
@@ -26,15 +26,20 @@
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.Deque;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.TreeSet;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.stream.Stream;
@@ -51,20 +56,24 @@

import org.apache.bval.jsr.ApacheValidatorFactory;
import org.apache.bval.jsr.ConstraintAnnotationAttributes;
import org.apache.bval.jsr.groups.Group;
import org.apache.bval.jsr.groups.GroupConversion;
import org.apache.bval.jsr.groups.GroupStrategy;
import org.apache.bval.jsr.groups.GroupsComputer;
import org.apache.bval.jsr.metadata.ContainerElementKey;
import org.apache.bval.jsr.metadata.EmptyBuilder;
import org.apache.bval.jsr.metadata.Meta;
import org.apache.bval.jsr.metadata.MetadataBuilder;
import org.apache.bval.jsr.metadata.Signature;
import org.apache.bval.jsr.util.AnnotationsManager;
import org.apache.bval.jsr.util.Methods;
import org.apache.bval.jsr.util.ToUnmodifiable;
import org.apache.bval.jsr.xml.AnnotationProxyBuilder;
import org.apache.bval.util.Exceptions;
import org.apache.bval.util.ObjectUtils;
import org.apache.bval.util.Validate;
import org.apache.bval.util.reflection.Reflection;
import org.apache.bval.util.reflection.Reflection.Interfaces;

class MetadataReader {

@@ -79,16 +88,16 @@ class MetadataReader {
}

Set<ConstraintD<?>> getConstraints() {
return builder.getConstraintDeclarationMap(meta).entrySet().stream().flatMap(e -> {
final Meta<E> m = e.getKey();
final Class<?> declaredBy = m.getDeclaringClass();
final Scope scope = declaredBy.equals(beanClass) ? Scope.LOCAL_ELEMENT : Scope.HIERARCHY;
return Stream.of(e.getValue())
.peek(
return builder.getConstraintDeclarationMap(meta).entrySet().stream().filter(e -> e.getValue().length > 0)
.flatMap(e -> {
final Meta<E> m = e.getKey();
final Class<?> declaredBy = m.getDeclaringClass();
final Scope scope = declaredBy.equals(beanClass) ? Scope.LOCAL_ELEMENT : Scope.HIERARCHY;
return Stream.of(e.getValue()).peek(
c -> validatorFactory.getAnnotationsManager().validateConstraintDefinition(c.annotationType()))
.map(c -> rewriteConstraint(c, declaredBy))
.map(c -> new ConstraintD<>(c, scope, m, validatorFactory));
}).collect(ToUnmodifiable.set());
.map(c -> rewriteConstraint(c, declaredBy))
.map(c -> new ConstraintD<>(c, scope, m, validatorFactory));
}).collect(ToUnmodifiable.set());
}

ApacheValidatorFactory getValidatorFactory() {
@@ -133,8 +142,8 @@ Map<String, PropertyDescriptor> getProperties(BeanD<T> parent) {

beanBuilder.getFields(meta).forEach((f, builder) -> {
final Field fld = Reflection.find(meta.getHost(), t -> Reflection.getDeclaredField(t, f));
properties.computeIfAbsent(f, descriptorList).add(new PropertyD.ForField(
new MetadataReader.ForContainer<>(new Meta.ForField(fld), builder), parent));
properties.computeIfAbsent(f, descriptorList).add(
new PropertyD.ForField(new MetadataReader.ForContainer<>(new Meta.ForField(fld), builder), parent));
});
beanBuilder.getGetters(meta).forEach((g, builder) -> {
final Method getter = Methods.getter(meta.getHost(), g);
@@ -205,34 +214,68 @@ Map<Signature, ConstructorD<T>> getConstructors(BeanD<T> parent) {
return Collections.unmodifiableMap(result);
}

List<Class<?>> getGroupSequence() {
List<Class<?>> result = builder.getGroupSequence(meta);
GroupStrategy getGroupStrategy() {
final Class<T> host = meta.getHost();
if (result == null) {
// resolve group sequence/Default redefinition up class hierarchy:
final Class<?> superclass = host.getSuperclass();
if (superclass != null) {
// attempt to mock parent sequence intent by appending this type immediately after supertype:
result = ((ElementD<?, ?>) validatorFactory.getDescriptorManager().getBeanDescriptor(superclass))
.getGroupSequence();
if (result != null) {
result = new ArrayList<>(result);
result.add(result.indexOf(superclass) + 1, host);
if (host.isInterface()) {
return validatorFactory.getGroupsComputer().computeGroups(host).asStrategy();
}
final GroupStrategy parentStrategy = Optional.ofNullable(host.getSuperclass()).filter(JDK.negate())
.map(validatorFactory.getDescriptorManager()::getBeanDescriptor).map(BeanD.class::cast)
.map(BeanD::getGroupStrategy).orElse(null);

final List<Class<?>> groupSequence = builder.getGroupSequence(meta);

final Set<Group> parentGroups = parentStrategy == null ? null : parentStrategy.getGroups();

Group localGroup = Group.of(host);
if (groupSequence == null) {
final Set<Group> groups = new HashSet<>();
groups.add(localGroup);

for (Class<?> t : Reflection.hierarchy(host, Interfaces.INCLUDE)) {
if (JDK.test(t)) {
continue;
}
if (!t.isInterface()) {
continue;
}
if (AnnotationsManager.isAnnotationDirectlyPresent(t, GroupSequence.class)) {
continue;
}
final Group g = Group.of(t);
if (parentGroups != null && parentGroups.contains(g)) {
continue;
}
groups.add(g);
}
final GroupStrategy strategy = GroupStrategy.simple(groups);
return parentStrategy == null ? strategy : GroupStrategy.composite(strategy, parentStrategy);
}
if (result == null) {
return null;
if (groupSequence.contains(Default.class)) {
Exceptions.raise(GroupDefinitionException::new, "@%s for %s must not contain %s",
GroupSequence.class.getSimpleName(), host, Default.class.getName());
}
if (!result.contains(host)) {
if (!groupSequence.contains(host)) {
Exceptions.raise(GroupDefinitionException::new, "@%s for %s must contain %<s",
GroupSequence.class.getSimpleName(), host);
}
if (result.contains(Default.class)) {
Exceptions.raise(GroupDefinitionException::new, "@%s for %s must not contain %s",
GroupSequence.class.getSimpleName(), host, Default.class.getName());
final Group.Sequence result =
Group.sequence(groupSequence.stream().map(Group::of).collect(Collectors.toList()));

final Deque<Group> expanded = new ArrayDeque<>();
for (Class<?> t : Reflection.hierarchy(host, Interfaces.INCLUDE)) {
if (JDK.test(t)) {
continue;
}
if (t.isInterface() && AnnotationsManager.isAnnotationDirectlyPresent(t, GroupSequence.class)) {
continue;
}
expanded.push(Group.of(t));
}
return Collections.unmodifiableList(result);
if (expanded.size() == 1) {
return result;
}
return result.redefining(Collections.singletonMap(localGroup, GroupStrategy.simple(expanded)));
}
}

@@ -261,7 +304,8 @@ Set<GroupConversion> getGroupConversions() {
groupConversions.stream().map(GroupConversion::getFrom)
.forEach(from -> Exceptions.raiseIf(from.isAnnotationPresent(GroupSequence.class),
ConstraintDeclarationException::new,
"Invalid group conversion declared on %s from group sequence %s", f -> f.args(meta.describeHost(), from)));
"Invalid group conversion declared on %s from group sequence %s",
f -> f.args(meta.describeHost(), from)));
}
return groupConversions;
}
@@ -336,7 +380,8 @@ List<String> getParameterNames(ParameterNameProvider parameterNameProvider, Meth

class ForConstructor<T> extends ForExecutable<Constructor<? extends T>, ForConstructor<T>> {

ForConstructor(Meta<Constructor<? extends T>> meta, MetadataBuilder.ForExecutable<Constructor<? extends T>> builder) {
ForConstructor(Meta<Constructor<? extends T>> meta,
MetadataBuilder.ForExecutable<Constructor<? extends T>> builder) {
super(meta, builder);
}

@@ -346,6 +391,8 @@ List<String> getParameterNames(ParameterNameProvider parameterNameProvider, Cons
}
}

private static final Predicate<Class<?>> JDK = t -> t.getName().startsWith("java.");

private final ApacheValidatorFactory validatorFactory;
private final Class<?> beanClass;

0 comments on commit 395f6df

Please sign in to comment.