Skip to content

Commit

Permalink
improve efficiency of generated exceptions/messages
Browse files Browse the repository at this point in the history
  • Loading branch information
mbenson committed Oct 16, 2018
1 parent 725477d commit 6a98485
Show file tree
Hide file tree
Showing 26 changed files with 215 additions and 132 deletions.
Expand Up @@ -126,8 +126,9 @@ public <V> Object put(Map<? super String, ? super V> map, V value) {
public <V> V get(Map<? super String, ? super V> map) {
@SuppressWarnings("unchecked")
final V result = (V) map.get(getAttributeName());
Exceptions.raiseUnless(TypeUtils.isInstance(result, getType()), IllegalStateException::new,
"Invalid '%s' value: %s", getAttributeName(), result);
if (!TypeUtils.isInstance(result, getType())) {
Exceptions.raise(IllegalStateException::new, "Invalid '%s' value: %s", getAttributeName(), result);
}
return result;
}

Expand Down
Expand Up @@ -24,13 +24,11 @@
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import javax.validation.ConstraintDefinitionException;
import javax.validation.ConstraintValidator;
Expand All @@ -44,7 +42,6 @@
import org.apache.bval.jsr.util.ToUnmodifiable;
import org.apache.bval.util.Exceptions;
import org.apache.bval.util.Lazy;
import org.apache.bval.util.ObjectUtils;
import org.apache.bval.util.Validate;

/**
Expand Down Expand Up @@ -73,8 +70,10 @@ public static final class ConstraintValidatorInfo<T extends Annotation> {
supportedTargets = svt == null ? DEFAULT_VALIDATION_TARGETS
: Collections.unmodifiableSet(EnumSet.copyOf(Arrays.asList(svt.value())));

Exceptions.raiseIf(supportedTargets.isEmpty(), ConstraintDefinitionException::new,
"Illegally specified 0-length %s value on %s", SupportedValidationTarget.class.getSimpleName(), type);
if (supportedTargets.isEmpty()) {
Exceptions.raise(ConstraintDefinitionException::new, "Illegally specified 0-length %s value on %s",
SupportedValidationTarget.class.getSimpleName(), type);
}
}

public Class<? extends ConstraintValidator<T, ?>> getType() {
Expand Down
Expand Up @@ -172,7 +172,9 @@ public ConstraintDescriptor<?> getConstraintDescriptor() {

@Override
public <U> U unwrap(Class<U> type) {
Exceptions.raiseUnless(type.isInstance(this), ValidationException::new, "Type %s is not supported", type);
if (!type.isInstance(this)) {
Exceptions.raise(ValidationException::new, "Type %s is not supported", type);
}
return type.cast(this);
}

Expand Down
Expand Up @@ -64,7 +64,7 @@ private static class TypeWrapper {
this.arrayDepth = d;
}

Class<?> unwrap(Class<?> t) {
Class<?> unwrapArrayComponentType(Class<?> t) {
Exceptions.raiseUnless(t.isAssignableFrom(componentType), IllegalArgumentException::new,
"%s not assignable from %s", t, componentType);
if (arrayDepth == 0) {
Expand All @@ -80,8 +80,10 @@ Class<?> unwrap(Class<?> t) {
private static Class<?> getValidatedType(Class<? extends ConstraintValidator<?, ?>> validatorType) {
final Type result = TypeUtils.getTypeArguments(validatorType, ConstraintValidator.class)
.get(ConstraintValidator.class.getTypeParameters()[1]);
Exceptions.raiseUnless(isSupported(result), ConstraintDefinitionException::new,
"Validated type %s declared by %s %s is unsupported", result, CV, validatorType.getName());
if (!isSupported(result)) {
Exceptions.raise(ConstraintDefinitionException::new, "Validated type %s declared by %s %s is unsupported",
result, CV, validatorType.getName());
}
return TypeUtils.getRawType(result, null);
}

Expand Down Expand Up @@ -140,14 +142,15 @@ private static boolean isSupported(Type validatedType) {
@SuppressWarnings("unchecked")
final Class<A> constraintType = (Class<A>) constraint.annotationType();

Exceptions.raiseIf(set.size() > 1 || !composed && set.isEmpty(), ConstraintDefinitionException::new,
"%d cross-parameter %ss found for constraint type %s", set.size(), CV, constraintType);
final int size = set.size();
Exceptions.raiseIf(size > 1 || !composed && set.isEmpty(), ConstraintDefinitionException::new,
"%d cross-parameter %ss found for constraint type %s", size, CV, constraintType);

final Class<? extends ConstraintValidator<A, ?>> result = set.iterator().next().getType();
Exceptions.raiseUnless(TypeUtils.isAssignable(Object[].class, getValidatedType(result)),
ConstraintDefinitionException::new,
"Cross-parameter %s %s does not support the validation of an object array", CV, result.getName());

if (!TypeUtils.isAssignable(Object[].class, getValidatedType(result))) {
Exceptions.raise(ConstraintDefinitionException::new,
"Cross-parameter %s %s does not support the validation of an object array", CV, result.getName());
}
return result;
}

Expand Down Expand Up @@ -199,6 +202,6 @@ private Stream<Class<?>> walkHierarchy() {
final TypeWrapper w = new TypeWrapper(Reflection.primitiveToWrapper(validatedType));
Stream.Builder<Class<?>> hierarchy = Stream.builder();
Reflection.hierarchy(w.componentType, Interfaces.INCLUDE).forEach(hierarchy);
return hierarchy.build().map(w::unwrap);
return hierarchy.build().map(w::unwrapArrayComponentType);
}
}
Expand Up @@ -197,7 +197,8 @@ private <T> T read(ConstraintAnnotationAttributes attr, Optionality optionality)
Optional.of(constraintType).map(attr::analyze).filter(Worker::isValid).map(w -> w.<T> read(annotation));

Exceptions.raiseUnless(optionality.isOptional() || result.isPresent(), ConstraintDefinitionException::new,
"Required attribute %s missing from constraint type %s", attr.getAttributeName(), constraintType);
"Required attribute %s missing from constraint type %s",
f -> f.args(attr.getAttributeName(), constraintType));

return result.orElse(null);
}
Expand Down Expand Up @@ -237,18 +238,21 @@ private Set<Class<?>> computeGroups() {
private Set<Class<? extends Payload>> computePayload() {
final Set<Class<? extends Payload>> result =
set(() -> read(ConstraintAnnotationAttributes.PAYLOAD, Optionality.REQUIRED));
Exceptions.raiseIf(result.containsAll(Arrays.asList(Unwrapping.Unwrap.class, Unwrapping.Skip.class)),
ConstraintDeclarationException::new,
"Constraint %s declared at %s specifies conflicting value unwrapping hints", annotation, meta.getHost());
if (result.containsAll(Arrays.asList(Unwrapping.Unwrap.class, Unwrapping.Skip.class))) {
Exceptions.raise(ConstraintDeclarationException::new,
"Constraint %s declared at %s specifies conflicting value unwrapping hints", annotation,
meta.getHost());
}
return result;
}

private Class<?> computeValidatedType(ApacheValidatorFactory validatorFactory) {
final Class<?> rawType = TypeUtils.getRawType(meta.getType(), null);

Exceptions.raiseIf(rawType == null, UnexpectedTypeException::new, "Could not calculate validated type from %s",
meta.getType());

if (rawType == null) {
Exceptions.raise(UnexpectedTypeException::new, "Could not calculate validated type from %s",
meta.getType());
}
if (payload.contains(Unwrapping.Skip.class)) {
return rawType;
}
Expand All @@ -258,8 +262,10 @@ private Class<?> computeValidatedType(ApacheValidatorFactory validatorFactory) {
final boolean unwrap = payload.contains(Unwrapping.Unwrap.class);

if (valueExtractor == null) {
Exceptions.raiseIf(unwrap, ConstraintDeclarationException::new, "No compatible %s found for %s",
ValueExtractor.class.getSimpleName(), meta.getType());
if (unwrap) {
Exceptions.raise(ConstraintDeclarationException::new, "No compatible %s found for %s",
ValueExtractor.class.getSimpleName(), meta.getType());
}
} else {
@SuppressWarnings("unchecked")
final Class<? extends ValueExtractor<?>> extractorClass =
Expand Down
Expand Up @@ -59,9 +59,10 @@ public ContainerElementKey getKey() {
@Override
protected Stream<GraphContext> readImpl(GraphContext context) throws Exception {
final ValueExtractor<?> valueExtractor = context.getValidatorContext().getValueExtractors().find(key);
Exceptions.raiseIf(valueExtractor == null, ConstraintDeclarationException::new, "No %s found for %s",
ValueExtractor.class.getSimpleName(), key);

if (valueExtractor == null) {
Exceptions.raise(ConstraintDeclarationException::new, "No %s found for %s",
ValueExtractor.class.getSimpleName(), key);
}
return ExtractValues.extract(context, key, valueExtractor).stream();
}
}
Expand Up @@ -105,9 +105,9 @@ Map<String, PropertyDescriptor> getProperties(BeanD<T> parent) {
beanBuilder.getGetters(meta).forEach((g, builder) -> {
final Method getter = Methods.getter(meta.getHost(), g);

Exceptions.raiseIf(getter == null, IllegalStateException::new,
"Getter method for property %s not found", g);

if (getter == null) {
Exceptions.raise(IllegalStateException::new, "Getter method for property %s not found", g);
}
properties.computeIfAbsent(g, descriptorList).add(new PropertyD.ForMethod(
new MetadataReader.ForContainer<>(new Meta.ForMethod(getter), builder), parent));
});
Expand Down Expand Up @@ -173,27 +173,31 @@ Map<Signature, ConstructorD<T>> getConstructors(BeanD<T> parent) {

List<Class<?>> getGroupSequence() {
List<Class<?>> result = builder.getGroupSequence(meta);
final Class<T> host = meta.getHost();
if (result == null) {
// resolve group sequence/Default redefinition up class hierarchy:
final Class<?> superclass = meta.getHost().getSuperclass();
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, meta.getHost());
result.add(result.indexOf(superclass) + 1, host);
}
}
}
if (result == null) {
return null;
}
Exceptions.raiseUnless(result.contains(meta.getHost()), GroupDefinitionException::new,
"@%s for %s must contain %<s", GroupSequence.class.getSimpleName(), meta.getHost());
Exceptions.raiseIf(result.contains(Default.class), GroupDefinitionException::new,
"@%s for %s must not contain %s", GroupSequence.class.getSimpleName(), meta.getHost(),
Default.class.getName());
if (!result.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());
}
return Collections.unmodifiableList(result);
}
}
Expand All @@ -211,20 +215,19 @@ boolean isCascaded() {
Set<GroupConversion> getGroupConversions() {
final Set<GroupConversion> groupConversions = builder.getGroupConversions(meta);
if (!groupConversions.isEmpty()) {
Exceptions.raiseUnless(isCascaded(), ConstraintDeclarationException::new,
"@%s declared without @%s on %s", ConvertGroup.class.getSimpleName(), Valid.class.getSimpleName(),
meta.describeHost());

Exceptions.raiseIf(
groupConversions.stream().map(GroupConversion::getFrom).distinct().count() < groupConversions
.size(),
ConstraintDeclarationException::new, "%s has duplicate 'from' group conversions",
meta.describeHost());

if (!isCascaded()) {
Exceptions.raise(ConstraintDeclarationException::new, "@%s declared without @%s on %s",
ConvertGroup.class.getSimpleName(), Valid.class.getSimpleName(), meta.describeHost());
}
if (groupConversions.stream().map(GroupConversion::getFrom).distinct().count() < groupConversions
.size()) {
Exceptions.raise(ConstraintDeclarationException::new, "%s has duplicate 'from' group conversions",
meta.describeHost());
}
groupConversions.stream().map(GroupConversion::getFrom)
.forEach(f -> Exceptions.raiseIf(f.isAnnotationPresent(GroupSequence.class),
.forEach(from -> Exceptions.raiseIf(from.isAnnotationPresent(GroupSequence.class),
ConstraintDeclarationException::new,
"Invalid group conversion declared on %s from group sequence %s", meta.describeHost(), f));
"Invalid group conversion declared on %s from group sequence %s", f -> f.args(meta.describeHost(), from)));
}
return groupConversions;
}
Expand Down
Expand Up @@ -137,10 +137,9 @@ protected Groups computeGroups(Collection<Class<?>> groups) {
Exceptions.raiseIf(groups.stream().anyMatch(Objects::isNull), IllegalArgumentException::new,
"Null group specified");

for (final Class<?> clazz : groups) {
Exceptions.raiseUnless(clazz.isInterface(), ValidationException::new,
"A group must be an interface. %s is not.", clazz);
}
groups.forEach(g -> Exceptions.raiseUnless(g.isInterface(), ValidationException::new,
"A group must be an interface. %s is not.", g));

final Groups chain = new Groups();
for (Class<?> clazz : groups) {
final GroupSequence anno = clazz.getAnnotation(GroupSequence.class);
Expand Down
Expand Up @@ -82,7 +82,7 @@ public LeafNodeBuilderCustomizableContext addBeanNode() {
@Override
public NodeBuilderDefinedContext addParameterNode(int index) {
Exceptions.raiseUnless(frame.descriptor instanceof CrossParameterDescriptor, ValidationException::new,
"Cannot add parameter node for %s", frame.descriptor.getClass().getName());
"Cannot add parameter node for %s", f -> f.args(frame.descriptor.getClass().getName()));

final CrossParameterD<?, ?> crossParameter =
ComposedD.unwrap(frame.descriptor, CrossParameterD.class).findFirst().get();
Expand Down Expand Up @@ -175,9 +175,9 @@ ValidationJob<T>.Frame<?> getFrame() {

Set<ConstraintViolation<T>> getRequiredViolations() {
if (!violations.optional().isPresent()) {
Exceptions.raiseIf(defaultConstraintViolationDisabled, ValidationException::new,
"Expected custom constraint violation(s)");

if (defaultConstraintViolationDisabled) {
Exceptions.raise(ValidationException::new, "Expected custom constraint violation(s)");
}
addError(getDefaultConstraintMessageTemplate(), frame.context.getPath());
}
return violations.get();
Expand Down
Expand Up @@ -200,8 +200,10 @@ public void handleProperty(String name) {
bean = (BeanD<?>) validatorContext.getDescriptorManager().getBeanDescriptor(element.getElementClass());
}
final PropertyDescriptor property = bean.getProperty(name);
Exceptions.raiseIf(property == null, IllegalArgumentException::new, "Unknown property %s of %s", name,
bean.getElementClass());
if (property == null) {
Exceptions.raise(IllegalArgumentException::new, "Unknown property %s of %s", name,
bean.getElementClass());
}
current = new DescriptorWrapper(property);
}

Expand Down Expand Up @@ -259,11 +261,8 @@ private Step handleElementByType(Type type) {
Optional.ofNullable(TypeUtils.getTypeArguments(type, Iterable.class).get(ITERABLE_ELEMENT))
.orElse(ITERABLE_ELEMENT);
} else {
elementType = null;
throw Exceptions.create(IllegalArgumentException::new, "Unable to resolve element type of %s", type);
}
Exceptions.raiseIf(elementType == null, IllegalArgumentException::new,
"Unable to resolve element type of %s", type);

return new TypeWrapper(validatorContext, elementType);
}

Expand Down Expand Up @@ -380,7 +379,9 @@ private Object handleBasic(Object o, String indexOrKey) {
} else {
try {
final int index = Integer.parseInt(indexOrKey);
Exceptions.raiseIf(index < 0, IllegalArgumentException::new, "Invalid index %d", index);
if (index < 0) {
Exceptions.raise(IllegalArgumentException::new, "Invalid index %d", index);
}
if (o != null && TypeUtils.isArrayType(o.getClass())) {
if (Array.getLength(o) > index) {
return Array.get(o, index);
Expand Down Expand Up @@ -478,8 +479,9 @@ private ValidateProperty(Strategy<T> strategy, ApacheFactoryContext validatorCon
descriptor = (ElementD<?, ?>) validatorContext.getDescriptorManager().getBeanDescriptor(t);
} else {
final Class<?> propertyType = descriptor.getElementClass();
Exceptions.raiseUnless(TypeUtils.isInstance(value, propertyType), IllegalArgumentException::new,
"%s is not an instance of %s", value, propertyType);
if (!TypeUtils.isInstance(value, propertyType)) {
Exceptions.raise(IllegalArgumentException::new, "%s is not an instance of %s", value, propertyType);
}
}
}

Expand All @@ -489,8 +491,9 @@ private ValidateProperty(Strategy<T> strategy, ApacheFactoryContext validatorCon
this(new ForBeanProperty<>(validatorContext, bean), validatorContext,
(Class<T>) Validate.notNull(bean, IllegalArgumentException::new, "bean").getClass(), property, groups);

Exceptions.raiseIf(descriptor == null, IllegalArgumentException::new,
"Could not resolve property name/path: %s", property);
if (descriptor == null) {
Exceptions.raise(IllegalArgumentException::new, "Could not resolve property name/path: %s", property);
}
}

public ValidateProperty<T> cascade(boolean cascade) {
Expand Down
Expand Up @@ -103,9 +103,9 @@ protected Class<T> getRootBeanClass() {
super(validatorContext, groups, meta);

final Type type = Validate.notNull(meta, IllegalArgumentException::new, "meta").getType();
Exceptions.raiseUnless(TypeUtils.isInstance(returnValue, type), IllegalArgumentException::new,
"%s is not an instance of %s", returnValue, type);

if (!TypeUtils.isInstance(returnValue, type)) {
Exceptions.raise(IllegalArgumentException::new, "%s is not an instance of %s", returnValue, type);
}
this.returnValue = returnValue;
}

Expand Down

0 comments on commit 6a98485

Please sign in to comment.