From 94bc6af6a65876e66abe89c0fc479d06e98b223d Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Mon, 28 Sep 2026 08:54:59 +0200 Subject: [PATCH 1/8] Speed up bean validation hot path Precompute per-bean and per-group validation plans instead of recomputing them on every validate() call, and remove allocations from the per-constraint path: - Cache the constraints matching a group strategy per element descriptor, the Default-group redefinition per bean descriptor, and the properties that need a frame for a group (constraints in scope, cascaded or with container element constraints). Properties with nothing to validate are no longer read. - Skip the traversable resolver and the path copies it needs when the default resolver runs without JPA. - Read property values through cached MethodHandles rather than Field#get/Method#invoke, which are slow when the reflective object is not a JIT constant. - Stop writing to a ConcurrentHashMap on every bean descriptor cache hit; resolve the root bean descriptor once per validation. - Precompute the deep cascaded flag of container descriptors. - Memoize the constraint message template and cache fully interpolated messages that need no EL evaluation, keying on the attribute map's identity. - Cache Groups#asStrategy() and group strategy hash codes. - Replace Lazy holders, streams and hash sets on the per-constraint path with plain fields and lists; back PathImpl with an ArrayList; compute violation and node hash codes without varargs arrays. --- .../bval/jsr/ConstraintViolationImpl.java | 13 +- .../bval/jsr/DefaultMessageInterpolator.java | 61 +++++++- .../org/apache/bval/jsr/descriptor/BeanD.java | 68 +++++++++ .../jsr/descriptor/CascadableContainerD.java | 12 ++ .../bval/jsr/descriptor/ConstraintD.java | 9 +- .../jsr/descriptor/DescriptorManager.java | 18 +-- .../apache/bval/jsr/descriptor/ElementD.java | 49 +++++++ .../apache/bval/jsr/descriptor/PropertyD.java | 84 ++++++++++- .../apache/bval/jsr/groups/GroupStrategy.java | 16 +- .../org/apache/bval/jsr/groups/Groups.java | 16 +- .../job/ConstraintValidatorContextImpl.java | 23 ++- .../org/apache/bval/jsr/job/ValidateBean.java | 15 +- .../apache/bval/jsr/job/ValidationJob.java | 137 +++++++++--------- .../resolver/DefaultTraversableResolver.java | 10 ++ .../org/apache/bval/jsr/util/NodeImpl.java | 6 +- .../org/apache/bval/jsr/util/PathImpl.java | 17 ++- 16 files changed, 435 insertions(+), 119 deletions(-) diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintViolationImpl.java b/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintViolationImpl.java index 75a99ece96..3515a393ae 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintViolationImpl.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintViolationImpl.java @@ -83,8 +83,17 @@ public ConstraintViolationImpl(String messageTemplate, String message, T rootBea this.elementType = elementType; this.returnValue = returnValue; this.parameters = parameters; - this.hashCode = Arrays.deepHashCode(new Object[] { messageTemplate, message, rootBean, rootBeanClass, leafBean, - value, propertyPath, elementType, constraintDescriptor, returnValue, parameters }); + int h = Objects.hashCode(messageTemplate); + h = 31 * h + Objects.hashCode(message); + h = 31 * h + Objects.hashCode(rootBean); + h = 31 * h + Objects.hashCode(rootBeanClass); + h = 31 * h + Objects.hashCode(leafBean); + h = 31 * h + Objects.hashCode(value); + h = 31 * h + Objects.hashCode(propertyPath); + h = 31 * h + Objects.hashCode(elementType); + h = 31 * h + Objects.hashCode(constraintDescriptor); + h = 31 * h + Objects.hashCode(returnValue); + this.hashCode = 31 * h + Arrays.deepHashCode(parameters); } /** diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/DefaultMessageInterpolator.java b/bval-jsr/src/main/java/org/apache/bval/jsr/DefaultMessageInterpolator.java index 568a6a71c2..801425cb15 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/DefaultMessageInterpolator.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/DefaultMessageInterpolator.java @@ -87,6 +87,8 @@ public Enumeration getKeys() { private final ConcurrentMap interpolatedMessages = new ConcurrentHashMap<>(); private final ConcurrentMap, Method> toStringMethods = new ConcurrentHashMap<>(); private final ConcurrentMap interpolations = new ConcurrentHashMap<>(); + /** Fully interpolated messages that involve no EL evaluation and so do not depend on the validated value. */ + private final ConcurrentMap finalMessages = new ConcurrentHashMap<>(); private final MessageEvaluator evaluator; @@ -134,6 +136,12 @@ public String interpolate(final String message, final Context context) { /** {@inheritDoc} */ @Override public String interpolate(final String message, final Context context, final Locale locale) { + final Map annotationParameters = context.getConstraintDescriptor().getAttributes(); + final FinalMessageKey finalKey = new FinalMessageKey(locale, message, annotationParameters); + final String cached = finalMessages.get(finalKey); + if (cached != null) { + return cached; + } if (!message.contains("{")) { return resolveEscapeSequences(message); } @@ -141,10 +149,14 @@ public String interpolate(final String message, final Context context, final Loc String resolvedMessage = getSharedInterpolatedMessage(message, locale); // resolve annotation attributes (step 4) - final Map annotationParameters = context.getConstraintDescriptor().getAttributes(); resolvedMessage = replaceAnnotationAttributes(resolvedMessage, annotationParameters); - // EL handling + // EL handling; the evaluator leaves messages without "${" untouched + if (!resolvedMessage.contains("${")) { + final String result = resolveEscapeSequences(resolvedMessage); + finalMessages.putIfAbsent(finalKey, result); + return result; + } if (evaluateExpressionLanguage(message, context)) { resolvedMessage = evaluator.interpolate(resolvedMessage, annotationParameters, context.getValidatedValue()); } @@ -323,6 +335,7 @@ private String replaceAnnotationAttributes(final String message, final Map annotationParameters; + private final int hash; + + private FinalMessageKey(final Locale locale, final String message, + final Map annotationParameters) { + this.locale = locale; + this.message = message; + this.annotationParameters = annotationParameters; + this.hash = 31 * (31 * locale.hashCode() + message.hashCode()) + System.identityHashCode(annotationParameters); + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + final FinalMessageKey that = (FinalMessageKey) o; + return annotationParameters == that.annotationParameters && message.equals(that.message) + && locale.equals(that.locale); + } + + @Override + public int hashCode() { + return hash; + } + } + + /** + * Keys the parameter map by identity: a constraint descriptor hands out the same attribute map instance on every + * call, so this avoids hashing and comparing the whole map for each interpolation. + */ private static class MessageWithParamsKey { private final String message; private final Map annotationParameters; @@ -400,7 +453,7 @@ private static class MessageWithParamsKey { private MessageWithParamsKey(final String message, final Map annotationParameters) { this.message = message; this.annotationParameters = annotationParameters; - this.hash = Objects.hash(message, annotationParameters); + this.hash = 31 * message.hashCode() + System.identityHashCode(annotationParameters); } @Override @@ -412,7 +465,7 @@ public boolean equals(final Object o) { return false; } final MessageWithParamsKey that = MessageWithParamsKey.class.cast(o); - return message.equals(that.message) && annotationParameters.equals(that.annotationParameters); + return annotationParameters == that.annotationParameters && message.equals(that.message); } @Override diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/BeanD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/BeanD.java index 91c24d2b21..7dc240386e 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/BeanD.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/BeanD.java @@ -19,11 +19,15 @@ package org.apache.bval.jsr.descriptor; import java.lang.reflect.Type; +import java.util.ArrayList; import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import jakarta.validation.metadata.BeanDescriptor; import jakarta.validation.metadata.ConstructorDescriptor; @@ -31,6 +35,7 @@ import jakarta.validation.metadata.MethodType; import jakarta.validation.metadata.PropertyDescriptor; +import org.apache.bval.jsr.groups.Group; import org.apache.bval.jsr.groups.GroupStrategy; import org.apache.bval.jsr.metadata.Signature; import org.apache.bval.jsr.util.ToUnmodifiable; @@ -52,6 +57,17 @@ public class BeanD extends ElementD, MetadataReader.ForBean> impl private final Set constrainedConstructors; private final Map, Set> methodCache = new HashMap<>(); + /** Leaf (non-composed) delegates of {@link #properties}. */ + private final PropertyD[] leafProperties; + /** + * Requested group strategy to the strategy with {@link Group#DEFAULT} redefined as this bean's default group + * sequence; {@link #UNCHANGED} when the redefinition is a no-op. + */ + private final ConcurrentMap localGroupStrategies = new ConcurrentHashMap<>(); + private final ConcurrentMap[]> propertiesByGroups = new ConcurrentHashMap<>(); + + private static final GroupStrategy UNCHANGED = GroupStrategy.simple(Collections.emptySet()); + BeanD(MetadataReader.ForBean reader) { super(reader); this.beanClass = reader.meta.getHost(); @@ -65,6 +81,11 @@ public class BeanD extends ElementD, MetadataReader.ForBean> impl cfpn.put(p.getPropertyName(), p); } constraintsForPropertyByName = Map.copyOf(cfpn); + final List> leaves = new ArrayList<>(); + for (PropertyDescriptor p : properties) { + ComposedD.forEachUnwrapped(p, PropertyD.class, leaves::add); + } + leafProperties = leaves.toArray(new PropertyD[0]); constructors = reader.getConstructors(this); methods = reader.getMethods(this); @@ -141,6 +162,53 @@ public GroupStrategy getGroupStrategy() { return groupStrategy; } + /** + * Get {@code groups} with {@link Group#DEFAULT} redefined as this bean's default group sequence. Returns + * {@code groups} itself (same instance) when no redefinition applies. + * + * @param groups + * @return {@link GroupStrategy} + */ + public GroupStrategy getLocalGroupStrategy(GroupStrategy groups) { + GroupStrategy result = localGroupStrategies.get(groups); + if (result == null) { + final GroupStrategy computed = + GroupStrategy.redefining(groups, Collections.singletonMap(Group.DEFAULT, groupStrategy)); + result = computed == groups ? UNCHANGED : computed; + localGroupStrategies.putIfAbsent(groups, result); + } + return result == UNCHANGED ? groups : result; + } + + /** + * Get the leaf constrained properties that need a validation frame for {@code groups}: those with constraints + * applying to any of its groups, with constrained container element types, or that are cascaded. The result is + * cached and must not be modified. + * + * @param groups + * @return {@link PropertyD} array, possibly empty + */ + public PropertyD[] getPropertiesFor(GroupStrategy groups) { + PropertyD[] result = propertiesByGroups.get(groups); + if (result == null) { + final Set targetGroups = groups.getGroups(); + final List> relevant = new ArrayList<>(leafProperties.length); + for (PropertyD p : leafProperties) { + if (p.isCascadedDeep() || !p.getConstrainedContainerElementTypes().isEmpty() + || p.getConstraintDescriptors().stream().map(ConstraintD.class::cast) + .anyMatch(c -> matchesGroups(c, targetGroups))) { + relevant.add(p); + } + } + result = relevant.toArray(new PropertyD[0]); + final PropertyD[] previous = propertiesByGroups.putIfAbsent(groups, result); + if (previous != null) { + result = previous; + } + } + return result; + } + public final Type getGenericType() { return getElementClass(); } diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/CascadableContainerD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/CascadableContainerD.java index ea506eb55b..30adbdc0f3 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/CascadableContainerD.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/CascadableContainerD.java @@ -36,6 +36,7 @@ public abstract class CascadableContainerD

, E extends A private final boolean cascaded; private final Set groupConversions; private final Set containerElementTypes; + private final boolean cascadedDeep; protected CascadableContainerD(MetadataReader.ForContainer reader, P parent) { super(reader, parent); @@ -45,6 +46,17 @@ protected CascadableContainerD(MetadataReader.ForContainer reader, P parent) .stream() .filter(DescriptorManager::isConstrained) .collect(ToUnmodifiable.set()); + cascadedDeep = cascaded || containerElementTypes.stream().anyMatch(DescriptorManager::isCascaded); + } + + /** + * Learn whether this element or any of its constrained container element types (recursively) is cascaded. + * + * @return {@code boolean} + * @see DescriptorManager#isCascaded + */ + public boolean isCascadedDeep() { + return cascadedDeep; } @Override diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ConstraintD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ConstraintD.java index 4e2dac5ff2..124bafdf11 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ConstraintD.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ConstraintD.java @@ -80,6 +80,8 @@ private static Set set(Supplier array) { private final Set> composingConstraints; private final List>> constraintValidatorClasses; + // racy single-check memoization: the annotation is immutable, so concurrent initializations agree + private String messageTemplate; private final Lazy toString = new Lazy<>(() -> String.format("%s: %s", ConstraintD.class.getSimpleName(), getAnnotation())); @@ -142,7 +144,12 @@ public boolean isReportAsSingleViolation() { @Override public String getMessageTemplate() { - return read(ConstraintAnnotationAttributes.MESSAGE, Optionality.REQUIRED); + String result = messageTemplate; + if (result == null) { + result = read(ConstraintAnnotationAttributes.MESSAGE, Optionality.REQUIRED); + messageTemplate = result; + } + return result; } @Override diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/DescriptorManager.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/DescriptorManager.java index 2d3081ea9e..5558785a54 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/DescriptorManager.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/DescriptorManager.java @@ -49,6 +49,9 @@ public static boolean isCascaded( D descriptor) { + if (descriptor instanceof CascadableContainerD) { + return ((CascadableContainerD) descriptor).isCascadedDeep(); + } return descriptor != null && (descriptor.isCascaded() || descriptor.getConstrainedContainerElementTypes().stream().anyMatch(DescriptorManager::isCascaded)); } @@ -59,8 +62,6 @@ public static boolean isConstrained(E descripto private final ApacheValidatorFactory validatorFactory; private final ConcurrentMap, BeanD> beanDescriptors = new ConcurrentHashMap<>(); - /** Same meaning as {@link BeanDescriptor#isBeanConstrained()} — avoids loading metadata on repeat {@code hasWork} checks. */ - private final ConcurrentMap, Boolean> beanConstrainedByType = new ConcurrentHashMap<>(); // synchronization unnecessary private final ReflectionBuilder reflectionBuilder; @@ -70,32 +71,21 @@ public DescriptorManager(ApacheValidatorFactory validatorFactory) { this.reflectionBuilder = new ReflectionBuilder(validatorFactory); } - /** - * @return cached result of {@link BeanDescriptor#isBeanConstrained()} for this class, or {@code null} if unknown - */ - public Boolean getCachedBeanConstrained(Class beanClass) { - return beanConstrainedByType.get(beanClass); - } - public BeanDescriptor getBeanDescriptor(Class beanClass) { Validate.notNull(beanClass, IllegalArgumentException::new, "beanClass"); // cannot use computeIfAbsent due to recursion being the usual case: final BeanD existing = beanDescriptors.get(beanClass); if (existing != null) { - beanConstrainedByType.put(beanClass, existing.isBeanConstrained()); return existing; } final BeanD value = new BeanD<>(new MetadataReader(validatorFactory, beanClass).forBean(builder(beanClass))); final BeanD previous = beanDescriptors.putIfAbsent(beanClass, value); - final BeanD result = previous == null ? value : previous; - beanConstrainedByType.put(beanClass, result.isBeanConstrained()); - return result; + return previous == null ? value : previous; } public void clear() { beanDescriptors.clear(); - beanConstrainedByType.clear(); } private MetadataBuilder.ForBean builder(Class beanClass) { diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ElementD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ElementD.java index 207f7ed844..a46c8f0ff8 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ElementD.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ElementD.java @@ -24,10 +24,14 @@ import java.lang.reflect.TypeVariable; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import jakarta.validation.groups.Default; import jakarta.validation.metadata.ConstraintDescriptor; import jakarta.validation.metadata.ElementDescriptor; +import org.apache.bval.jsr.groups.Group; import org.apache.bval.jsr.groups.GroupStrategy; import org.apache.bval.jsr.groups.GroupsComputer; import org.apache.bval.jsr.metadata.Meta; @@ -72,11 +76,33 @@ public final GroupStrategy getGroupStrategy() { } } + private static final ConstraintD[] NO_CONSTRAINTS = {}; + + /** + * Learn whether {@code constraint} applies to any of {@code targetGroups}. + * + * @param constraint + * @param targetGroups + * @return {@code boolean} + */ + public static boolean matchesGroups(ConstraintD constraint, Set targetGroups) { + final Set> constraintGroups = constraint.getGroups(); + final boolean impliesDefault = constraintGroups.contains(Default.class); + for (final Group target : targetGroups) { + final Class g = target.getGroup(); + if (constraintGroups.contains(g) || impliesDefault && constraint.getDeclaringClass().equals(g)) { + return true; + } + } + return false; + } + protected final Type genericType; final GroupsComputer groupsComputer; private final Meta meta; private final Set> constraints; + private final ConcurrentMap[]> constraintsByGroups; protected ElementD(R reader) { super(); @@ -84,6 +110,7 @@ protected ElementD(R reader) { this.meta = reader.meta; this.genericType = reader.meta.getType(); this.constraints = reader.getConstraints(); + this.constraintsByGroups = constraints.isEmpty() ? null : new ConcurrentHashMap<>(); this.groupsComputer = reader.getValidatorFactory().getGroupsComputer(); } @@ -98,6 +125,28 @@ public Set> getConstraintDescriptors() { return (Set) constraints; } + /** + * Get the constraints of this element that apply to any group of {@code groups}, in declaration order. The + * result is cached per {@link GroupStrategy} and must not be modified. + * + * @param groups + * @return {@link ConstraintD} array, possibly empty + */ + public ConstraintD[] getConstraintsFor(GroupStrategy groups) { + if (constraintsByGroups == null) { + return NO_CONSTRAINTS; + } + final ConstraintD[] cached = constraintsByGroups.get(groups); + if (cached != null) { + return cached; + } + final Set targetGroups = groups.getGroups(); + final ConstraintD[] result = + constraints.stream().filter(c -> matchesGroups(c, targetGroups)).toArray(ConstraintD[]::new); + final ConstraintD[] previous = constraintsByGroups.putIfAbsent(groups, result); + return previous == null ? result : previous; + } + @Override public final ConstraintFinder findConstraints() { return new Finder(groupsComputer, this); diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/PropertyD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/PropertyD.java index 3a89c6cc1c..0a1e39aacb 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/PropertyD.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/PropertyD.java @@ -16,10 +16,16 @@ */ package org.apache.bval.jsr.descriptor; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodHandles.Lookup; +import java.lang.invoke.MethodType; +import java.lang.reflect.AccessibleObject; import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.function.Consumer; import java.util.stream.Stream; import jakarta.validation.ValidationException; @@ -27,6 +33,7 @@ import org.apache.bval.jsr.GraphContext; import org.apache.bval.jsr.util.Methods; +import org.apache.bval.jsr.util.PathImpl; import org.apache.bval.util.reflection.Reflection; import org.apache.commons.weaver.privilizer.Privilizing; import org.apache.commons.weaver.privilizer.Privilizing.CallTo; @@ -48,6 +55,16 @@ public String getPropertyName() { @Override public Object getValue(Object parent) throws Exception { + final MethodHandle getter = getter(); + if (getter != null) { + try { + return (Object) getter.invokeExact(parent); + } catch (Exception | Error e) { + throw e; + } catch (Throwable t) { + throw new IllegalArgumentException(t); + } + } Reflection.makeAccessible(getTarget()); try { return getTarget().get(parent); @@ -55,6 +72,11 @@ public Object getValue(Object parent) throws Exception { throw new IllegalArgumentException(e); } } + + @Override + MethodHandle unreflect(Lookup lookup) throws IllegalAccessException { + return lookup.unreflectGetter(getTarget()); + } } static class ForMethod extends PropertyD { @@ -70,6 +92,15 @@ public String getPropertyName() { @Override public Object getValue(Object parent) throws Exception { + final MethodHandle getter = getter(); + if (getter != null) { + try { + return (Object) getter.invokeExact(parent); + } catch (Throwable t) { + // same wrapping as Method#invoke followed by the catch below + throw new IllegalArgumentException(new InvocationTargetException(t)); + } + } Reflection.makeAccessible(getTarget()); try { return getTarget().invoke(parent); @@ -77,23 +108,70 @@ public Object getValue(Object parent) throws Exception { throw new IllegalArgumentException(e); } } + + @Override + MethodHandle unreflect(Lookup lookup) throws IllegalAccessException { + return lookup.unreflect(getTarget()); + } } + private static final MethodType GETTER_TYPE = MethodType.methodType(Object.class, Object.class); + + private final Consumer addPropertyNode = p -> p.addProperty(getPropertyName()); + + /** + * Reading through a cached {@link MethodHandle} avoids the slow path that {@link Field#get(Object)} and + * {@link Method#invoke(Object, Object...)} take when the reflective object is not a JIT constant. Holds + * {@link #NO_GETTER} when no handle can be created, in which case reflection is used. + */ + private volatile MethodHandle getter; + + private static final MethodHandle NO_GETTER = MethodHandles.constant(Object.class, null); + protected PropertyD(MetadataReader.ForContainer reader, BeanD parent) { super(reader, parent); } public final Stream read(GraphContext context) { + final GraphContext child = readChild(context); + return child == null ? Stream.empty() : Stream.of(child); + } + + /** + * Read this property from the bean held by {@code context}. + * + * @param context + * @return child {@link GraphContext} holding the property value, or {@code null} if {@code context} holds no bean + */ + public final GraphContext readChild(GraphContext context) { if (context.getValue() == null) { - return Stream.empty(); + return null; } try { - final Object value = getValue(context.getValue()); - return Stream.of(context.child(p -> p.addProperty(getPropertyName()), value)); + return context.child(addPropertyNode, getValue(context.getValue())); } catch (Exception e) { throw e instanceof ValidationException ? (ValidationException) e : new ValidationException(e); } } public abstract Object getValue(Object parent) throws Exception; + + abstract MethodHandle unreflect(Lookup lookup) throws IllegalAccessException; + + /** + * Get a {@code (Object)Object} handle reading this property, or {@code null} if none could be created. + */ + final MethodHandle getter() { + MethodHandle result = getter; + if (result == null) { + try { + Reflection.makeAccessible((AccessibleObject) getTarget()); + result = unreflect(MethodHandles.lookup()).asType(GETTER_TYPE); + } catch (RuntimeException | IllegalAccessException e) { + result = NO_GETTER; + } + getter = result; + } + return result == NO_GETTER ? null : result; + } } diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/groups/GroupStrategy.java b/bval-jsr/src/main/java/org/apache/bval/jsr/groups/GroupStrategy.java index 343283c8fc..4f2cfc7399 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/groups/GroupStrategy.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/groups/GroupStrategy.java @@ -36,6 +36,7 @@ public interface GroupStrategy { public static class Simple implements GroupStrategy { private final Set groups; + private int hashCode; private Simple(Set groups) { this.groups = groups; @@ -57,7 +58,12 @@ public GroupStrategy redefining(Map redefinition @Override public int hashCode() { - return groups.hashCode(); + int result = hashCode; + if (result == 0) { + result = groups.hashCode(); + hashCode = result; + } + return result; } @Override @@ -75,6 +81,7 @@ public String toString() { public static class Composite implements GroupStrategy { private final Set components; protected final boolean ordered; + private int hashCode; public Composite(Collection components, boolean ordered) { this.components = new LinkedHashSet<>(components); @@ -117,7 +124,12 @@ public boolean applyTo(Predicate operation) { @Override public int hashCode() { - return Objects.hash(components, ordered); + int result = hashCode; + if (result == 0) { + result = Objects.hash(components, ordered); + hashCode = result; + } + return result; } @Override diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/groups/Groups.java b/bval-jsr/src/main/java/org/apache/bval/jsr/groups/Groups.java index 92eefb40b7..7df749cd8d 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/groups/Groups.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/groups/Groups.java @@ -38,6 +38,7 @@ public class Groups { private final Set groups = new LinkedHashSet<>(); private final Set sequences = new LinkedHashSet<>(); + private volatile GroupStrategy strategy; /** * Get the Groups. @@ -65,6 +66,7 @@ public Collection getSequences() { * @return success */ boolean insertGroup(Group group) { + strategy = null; return groups.add(group); } @@ -76,6 +78,7 @@ boolean insertGroup(Group group) { * @return success */ boolean insertSequence(Collection groups) { + strategy = null; return !(groups == null || groups.isEmpty()) && sequences.add(Group.sequence(groups)); } @@ -120,9 +123,14 @@ private void ensureExpandable(List groupList, List defaultGroupLis } public GroupStrategy asStrategy() { - final List components = new ArrayList<>(); - components.addAll(groups); - components.addAll(sequences); - return GroupStrategy.composite(components); + GroupStrategy result = strategy; + if (result == null) { + final List components = new ArrayList<>(); + components.addAll(groups); + components.addAll(sequences); + result = GroupStrategy.composite(components); + strategy = result; + } + return result; } } \ No newline at end of file diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ConstraintValidatorContextImpl.java b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ConstraintValidatorContextImpl.java index 2f88423f95..39bd290f2a 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ConstraintValidatorContextImpl.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ConstraintValidatorContextImpl.java @@ -18,8 +18,8 @@ */ package org.apache.bval.jsr.job; -import java.util.HashSet; -import java.util.Set; +import java.util.ArrayList; +import java.util.List; import jakarta.validation.ClockProvider; import jakarta.validation.ConstraintValidatorContext; @@ -40,7 +40,6 @@ import org.apache.bval.jsr.util.NodeImpl; import org.apache.bval.jsr.util.PathImpl; import org.apache.bval.util.Exceptions; -import org.apache.bval.util.Lazy; import org.apache.bval.util.Validate; public class ConstraintValidatorContextImpl implements ConstraintValidatorContext, ApacheMessageContext { @@ -136,7 +135,8 @@ private PathImpl extensiblePath() { private final ValidationJob.Frame frame; private final ConstraintD constraint; - private final Lazy>> violations = new Lazy<>(HashSet::new); + // typically holds a single violation; a list with a containment check keeps set semantics cheaply + private List> violations; private boolean defaultConstraintViolationDisabled; ConstraintValidatorContextImpl(ValidationJob.Frame frame, ConstraintD constraint) { @@ -178,14 +178,14 @@ ValidationJob.Frame getFrame() { return frame; } - Set> getRequiredViolations() { - if (!violations.optional().isPresent()) { + List> getRequiredViolations() { + if (violations == null) { if (defaultConstraintViolationDisabled) { Exceptions.raise(ValidationException::new, "Expected custom constraint violation(s)"); } addError(getDefaultConstraintMessageTemplate(), frame.context.getPath()); } - return violations.get(); + return violations; } @Override @@ -200,7 +200,14 @@ public Object getValidatedValue() { @SuppressWarnings({ "unchecked", "rawtypes" }) private void addError(String messageTemplate, PathImpl propertyPath) { - violations.get().add(((ValidationJob) frame.getJob()).createViolation(messageTemplate, this, propertyPath)); + if (violations == null) { + violations = new ArrayList<>(2); + } + final ConstraintViolation violation = + ((ValidationJob) frame.getJob()).createViolation(messageTemplate, this, propertyPath); + if (!violations.contains(violation)) { + violations.add(violation); + } } @Override diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateBean.java b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateBean.java index ee22b180fd..526c39981d 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateBean.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateBean.java @@ -23,7 +23,6 @@ import org.apache.bval.jsr.GraphContext; import org.apache.bval.jsr.descriptor.BeanD; import org.apache.bval.jsr.descriptor.ConstraintD; -import org.apache.bval.jsr.descriptor.DescriptorManager; import org.apache.bval.jsr.util.PathImpl; import org.apache.bval.jsr.util.Proxies; import org.apache.bval.util.Validate; @@ -33,6 +32,7 @@ public final class ValidateBean extends ValidationJob { private final T bean; + private BeanD descriptor; ValidateBean(ApacheFactoryContext validatorContext, T bean, Class[] groups) { super(validatorContext, groups); @@ -48,17 +48,16 @@ protected boolean hasWork() { unwrappedClass = Proxies.classFor(beanClass); classCache.putIfAbsent(beanClass, unwrappedClass); } - final DescriptorManager dm = validatorContext.getFactory().getDescriptorManager(); - final Boolean cached = dm.getCachedBeanConstrained(unwrappedClass); - if (cached != null) { - return cached; - } - return dm.getBeanDescriptor(unwrappedClass).isBeanConstrained(); + @SuppressWarnings("unchecked") + final BeanD d = + (BeanD) validatorContext.getFactory().getDescriptorManager().getBeanDescriptor(unwrappedClass); + descriptor = d; + return d.isBeanConstrained(); } @Override protected Frame> computeBaseFrame() { - return new BeanFrame(new GraphContext(validatorContext, PathImpl.create(), bean)); + return new BeanFrame(null, new GraphContext(validatorContext, PathImpl.create(), bean), descriptor); } @SuppressWarnings("unchecked") diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java index 3c33c555aa..85aabc1e1e 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java @@ -21,10 +21,9 @@ import java.lang.reflect.Array; import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -33,7 +32,6 @@ import java.util.function.Consumer; import java.util.function.Predicate; import java.util.stream.Collectors; -import java.util.stream.Stream; import jakarta.validation.ConstraintValidator; import jakarta.validation.ConstraintViolation; @@ -44,7 +42,6 @@ import jakarta.validation.UnexpectedTypeException; import jakarta.validation.ValidationException; import jakarta.validation.constraintvalidation.ValidationTarget; -import jakarta.validation.groups.Default; import jakarta.validation.metadata.CascadableDescriptor; import jakarta.validation.metadata.ContainerDescriptor; import jakarta.validation.metadata.ContainerElementTypeDescriptor; @@ -67,6 +64,7 @@ import org.apache.bval.jsr.groups.GroupStrategy; import org.apache.bval.jsr.groups.Groups; import org.apache.bval.jsr.metadata.ContainerElementKey; +import org.apache.bval.jsr.resolver.DefaultTraversableResolver; import org.apache.bval.jsr.util.NodeImpl; import org.apache.bval.jsr.util.PathImpl; import org.apache.bval.jsr.util.Proxies; @@ -117,7 +115,9 @@ void recurse(GroupStrategy groups, Consumer> sink) { abstract Object getBean(); void validateDescriptorConstraints(GroupStrategy groups, Consumer> sink) { - constraintsFor(descriptor, groups).forEach(c -> validateUnwrapped(c, sink)); + for (final ConstraintD c : descriptor.getConstraintsFor(groups)) { + validateUnwrapped(c, sink); + } } // Visit each (possibly unwrapped) frame for this constraint without allocating a Stream per constraint; @@ -254,30 +254,36 @@ public class BeanFrame extends Frame> { } BeanFrame(Frame parent, GraphContext context) { - super(parent, getBeanDescriptor(context.getValue()), - context.child(PathImpl::addBean, context.getValue())); + this(parent, context, getBeanDescriptor(context.getValue())); + } + + BeanFrame(Frame parent, GraphContext context, BeanD descriptor) { + super(parent, descriptor, context.child(PathImpl::addBean, context.getValue())); this.realContext = context; } @Override void process(GroupStrategy groups, Consumer> sink) { Validate.notNull(sink, "sink"); - final Lazy>> propertyFrames = new Lazy<>(this::propertyFrames); - - final GroupStrategy localGroupStrategy = GroupStrategy.redefining(groups, - Collections.singletonMap(Group.DEFAULT, descriptor.getGroupStrategy())); + final GroupStrategy localGroupStrategy = descriptor.getLocalGroupStrategy(groups); + final boolean redefined = localGroupStrategy != groups; + final List> propertyFrames = propertyFrames(descriptor.getPropertiesFor(localGroupStrategy)); - localGroupStrategy.applyTo(noViolations(gs -> { + localGroupStrategy.applyTo(gs -> { + final int originalCount = violationCount(); validateDescriptorConstraints(gs, sink); - propertyFrames.get().forEach(p -> { + for (final Frame p : propertyFrames) { p.validateDescriptorConstraints(gs, sink); - if (localGroupStrategy == groups) { + if (!redefined) { p.recurse(gs, sink); } - }); - })); - if (localGroupStrategy != groups) { - propertyFrames.get().forEach(p -> p.recurse(groups, sink)); + } + return violationCount() == originalCount; + }); + if (redefined) { + for (final Frame p : propertyFrames) { + p.recurse(groups, sink); + } } } @@ -290,30 +296,32 @@ Object getBean() { return context.getValue(); } - private Set> propertyFrames() { + private List> propertyFrames(PropertyD[] properties) { + if (properties.length == 0) { + return Collections.emptyList(); + } final TraversableResolver traversableResolver = validatorContext.getTraversableResolver(); - final Set> frames = new HashSet<>(); - for (final PropertyDescriptor pd : descriptor.getConstrainedProperties()) { - ComposedD.forEachUnwrapped(pd, PropertyD.class, d -> { + final boolean traverseAll = traversesAll(traversableResolver); + final List> frames = new ArrayList<>(properties.length); + for (final PropertyD d : properties) { + if (!traverseAll) { final PathImpl p = realContext.getPath(); p.addProperty(d.getPropertyName()); try { if (!traversableResolver.isReachable(context.getValue(), p.removeLeafNode(), getRootBeanClass(), p, d.getElementType())) { - return; + continue; } } catch (ValidationException ve) { throw ve; } catch (Exception e) { throw new ValidationException(e); } - for (final Iterator it = d.read(realContext).iterator(); it.hasNext();) { - final GraphContext child = it.next(); - if (!child.isRecursive()) { - frames.add(propertyFrame(d, child)); - } - } - }); + } + final GraphContext child = d.readChild(realContext); + if (child != null && !child.isRecursive()) { + frames.add(propertyFrame(d, child)); + } } return frames; } @@ -335,7 +343,7 @@ void validateDescriptorConstraints(GroupStrategy groups, Consumer { - if (!constraintsFor(d, groups).findFirst().isPresent() + if (d.getConstraintsFor(groups).length == 0 && d.getConstrainedContainerElementTypes().isEmpty()) { return; } @@ -357,8 +365,13 @@ void recurse(GroupStrategy groups, Consumer> sink) { if (context.getValue() == null || !DescriptorManager.isCascaded(descriptor)) { return; } + final Set groupConversions = descriptor.getGroupConversions(); + if (groupConversions.isEmpty()) { + groups.applyTo(noViolations(gs -> cascade(gs, sink))); + return; + } final Map conversions = new HashMap<>(); - for (final GroupConversionDescriptor gc : descriptor.getGroupConversions()) { + for (final GroupConversionDescriptor gc : groupConversions) { conversions.put(Group.of(gc.getFrom()), validatorContext.getGroupsComputer().computeGroups(gc.getTo()).asStrategy()); } @@ -384,8 +397,8 @@ private void cascade(GroupStrategy groups, Consumer> sink if (!descriptor.isCascaded()) { return; } - if (descriptor instanceof PropertyDescriptor) { - final TraversableResolver traversableResolver = validatorContext.getTraversableResolver(); + final TraversableResolver traversableResolver = validatorContext.getTraversableResolver(); + if (descriptor instanceof PropertyDescriptor && !traversesAll(traversableResolver)) { final Object traversableObject = Optional.ofNullable(context.getParent()).map(GraphContext::getValue).orElse(null); @@ -564,32 +577,22 @@ Object getBean() { protected static final TypeVariable MAP_VALUE = Map.class.getTypeParameters()[1]; protected static final TypeVariable ITERABLE_ELEMENT = Iterable.class.getTypeParameters()[0]; - private static Stream> constraintsFor(ElementD descriptor, GroupStrategy groups) { - // Resolve the target groups once per call rather than once per constraint: GroupStrategy.getGroups() - // may allocate (a singleton for a plain Group, a fully streamed-and-collected set for a Composite), - // and it is invariant across the constraints being filtered. - final Set targetGroups = groups.getGroups(); - return descriptor.getConstraintDescriptors().stream().> map(ConstraintD.class::cast) - .filter(c -> matchesGroups(c, targetGroups)); - } - - private static boolean matchesGroups(ConstraintD constraint, Set targetGroups) { - final Set> constraintGroups = constraint.getGroups(); - final boolean impliesDefault = constraintGroups.contains(Default.class); - for (final Group target : targetGroups) { - final Class g = target.getGroup(); - if (constraintGroups.contains(g) - || impliesDefault && constraint.getDeclaringClass().equals(g)) { - return true; - } - } - return false; + /** + * Learn whether {@code traversableResolver} is known to report every property as reachable and cascadable, so + * that consulting it (and building the paths it takes) can be skipped. + */ + private static boolean traversesAll(TraversableResolver traversableResolver) { + return traversableResolver.getClass() == DefaultTraversableResolver.class + && ((DefaultTraversableResolver) traversableResolver).isTraverseAll(); } protected final ApacheFactoryContext validatorContext; protected final Groups groups; - private final Lazy>> results = new Lazy<>(LinkedHashSet::new); + /** Collected violations; {@code null} until the first one is reported. */ + private Set> results; + /** What {@link #getResults()} returns once computed. */ + private Set> resultsView; ValidationJob(ApacheFactoryContext validatorContext, Class[] groups) { super(); @@ -598,21 +601,26 @@ private static boolean matchesGroups(ConstraintD constraint, Set targe } public final Set> getResults() { - if (results.optional().isPresent()) { - return results.get(); + if (resultsView != null) { + return resultsView; } if (hasWork()) { final Frame baseFrame = computeBaseFrame(); Validate.validState(baseFrame != null, "%s computed null baseFrame", getClass().getName()); - final Consumer> sink = results.consumer(Set::add); - - baseFrame.process(groups.asStrategy(), sink); - if (results.optional().isPresent()) { - return Collections.unmodifiableSet(results.get()); + baseFrame.process(groups.asStrategy(), this::addResult); + if (results != null) { + return resultsView = Collections.unmodifiableSet(results); } } - return results.reset(Collections::emptySet).get(); + return resultsView = Collections.emptySet(); + } + + private void addResult(ConstraintViolation violation) { + if (results == null) { + results = new LinkedHashSet<>(); + } + results.add(violation); } @SuppressWarnings("unchecked") @@ -658,8 +666,7 @@ protected Predicate noViolations(Consumer consumer) { } private int violationCount() { - final Optional>> maybeResults = results.optional(); - return maybeResults.isPresent() ? maybeResults.get().size() : 0; + return results == null ? 0 : results.size(); } private String interpolate(String messageTemplate, MessageInterpolator.Context context) { diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/resolver/DefaultTraversableResolver.java b/bval-jsr/src/main/java/org/apache/bval/jsr/resolver/DefaultTraversableResolver.java index af88caf8f3..ff514e27f9 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/resolver/DefaultTraversableResolver.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/resolver/DefaultTraversableResolver.java @@ -49,6 +49,16 @@ public DefaultTraversableResolver() { initJpa(); } + /** + * Learn whether this resolver reports every property as both reachable and cascadable, i.e. whether calls to it + * can be skipped altogether. + * + * @return {@code true} if no JPA-aware resolver is in use + */ + public boolean isTraverseAll() { + return jpaTR == null; + } + /** * {@inheritDoc} */ diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/util/NodeImpl.java b/bval-jsr/src/main/java/org/apache/bval/jsr/util/NodeImpl.java index 14eaab025e..754a30e92a 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/util/NodeImpl.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/util/NodeImpl.java @@ -383,7 +383,11 @@ public boolean equals(Object o) { */ @Override public int hashCode() { - return Objects.hash(name, Boolean.valueOf(inIterable), index, key, getKind()); + int h = Objects.hashCode(name); + h = 31 * h + Boolean.hashCode(inIterable); + h = 31 * h + Objects.hashCode(index); + h = 31 * h + Objects.hashCode(key); + return 31 * h + Objects.hashCode(getKind()); } public int getParameterIndex() { diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java b/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java index 5cd389df33..1cd8a0e208 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java @@ -19,9 +19,9 @@ package org.apache.bval.jsr.util; import java.io.Serializable; +import java.util.ArrayList; import java.util.Comparator; import java.util.Iterator; -import java.util.LinkedList; import java.util.List; import java.util.Objects; @@ -164,7 +164,7 @@ private static boolean isAwaitingPropertyName(NodeImpl n) { return n != null && n.getName() == null && (n.isInIterable() || n.getContainerClass() != null); } - private final LinkedList nodeList = new LinkedList<>(); + private final ArrayList nodeList; // Copy-on-write for the leaf node. copy() of another PathImpl shares node references instead of deep-copying // (nodes are effectively immutable once they are interior; only the current leaf is ever mutated in place). @@ -172,12 +172,14 @@ private static boolean isAwaitingPropertyName(NodeImpl n) { private boolean sharedLeaf; private PathImpl() { + nodeList = new ArrayList<>(4); } private PathImpl(Path path) { if (path instanceof PathImpl) { final PathImpl source = (PathImpl) path; // share node references; the leaf will be copied on first mutation by whichever path mutates it + nodeList = new ArrayList<>(source.nodeList.size() + 2); nodeList.addAll(source.nodeList); if (!nodeList.isEmpty()) { sharedLeaf = true; @@ -185,6 +187,7 @@ private PathImpl(Path path) { source.sharedLeaf = true; } } else { + nodeList = new ArrayList<>(); path.forEach(n -> nodeList.add(newNode(n))); } } @@ -229,7 +232,7 @@ public boolean isRootPath() { if (nodeList.size() != 1) { return false; } - final Path.Node first = nodeList.peekFirst(); + final Path.Node first = nodeList.get(0); return !first.isInIterable() && first.getName() == null; } @@ -243,7 +246,7 @@ public boolean isRootPath() { public PathImpl addNode(Node node) { final NodeImpl impl = node instanceof NodeImpl ? (NodeImpl) node : newNode(node); if (isRootPath()) { - nodeList.pop(); + nodeList.remove(0); } nodeList.add(impl); // the appended node is caller-owned/fresh, so the new leaf is exclusively owned @@ -298,7 +301,7 @@ public NodeImpl removeLeafNode() { Exceptions.raiseIf(isRootPath() || nodeList.isEmpty(), IllegalStateException::new, "No nodes in path!"); try { - return nodeList.removeLast(); + return nodeList.remove(nodeList.size() - 1); } finally { if (nodeList.isEmpty()) { nodeList.add(new NodeImpl.BeanNodeImpl()); @@ -336,7 +339,7 @@ public NodeImpl getNode(int index) { public NodeImpl getRootNode() { Exceptions.raiseIf(nodeList.isEmpty(), IndexOutOfBoundsException::new, "Path is empty"); - return nodeList.peekFirst(); + return nodeList.get(0); } /** @@ -346,7 +349,7 @@ public NodeImpl getRootNode() { public NodeImpl getLeafNode() { Exceptions.raiseIf(nodeList.isEmpty(), IndexOutOfBoundsException::new, "Path is empty"); - return nodeList.peekLast(); + return nodeList.get(nodeList.size() - 1); } /** From 484ace9a4f0553d1b9524ae9f7653139e3e75d52 Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Mon, 28 Sep 2026 09:13:25 +0200 Subject: [PATCH 2/8] Create property frames lazily and trim violation path copies - Build a bean's property frames on first use per group, so properties relevant only to groups that are never validated (such as later steps of a failing group sequence) are neither read nor resolved. - Front the per-group descriptor caches with a small identity-matched array, as validation keeps passing the same strategy instances. - Derive a violation's path directly from the parent context instead of materializing the context's own path and copying it again. --- .../org/apache/bval/jsr/GraphContext.java | 23 ++++-- .../org/apache/bval/jsr/descriptor/BeanD.java | 30 +++----- .../apache/bval/jsr/descriptor/ElementD.java | 19 ++--- .../bval/jsr/descriptor/GroupStrategyMap.java | 68 ++++++++++++++++ .../apache/bval/jsr/job/ValidationJob.java | 77 ++++++++++++++----- .../org/apache/bval/jsr/util/PathImpl.java | 7 +- 6 files changed, 162 insertions(+), 62 deletions(-) create mode 100644 bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/GroupStrategyMap.java diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/GraphContext.java b/bval-jsr/src/main/java/org/apache/bval/jsr/GraphContext.java index e65f3be4e2..ae4e0ad1cb 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/GraphContext.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/GraphContext.java @@ -65,6 +65,12 @@ public ApacheFactoryContext getValidatorContext() { } public PathImpl getPath() { + final PathImpl p = path; + if (p == null && pendingNode == null) { + // derive a private copy straight from the parent rather than materializing this context's path only to + // copy it again; the mutation appends fresh nodes, so nothing is shared with other copies + return derivePath(); + } return PathImpl.copy(pathReference()); } @@ -77,17 +83,22 @@ public PathImpl getPath() { public PathImpl pathReference() { PathImpl p = path; if (p == null) { - p = PathImpl.copy(parent.pathReference()); - if (pendingNode != null) { - p.addNode(pendingNode); - } else { - pendingMutation.accept(p); - } + p = derivePath(); path = p; } return p; } + private PathImpl derivePath() { + final PathImpl p = PathImpl.copy(parent.pathReference()); + if (pendingNode != null) { + p.addNode(pendingNode); + } else { + pendingMutation.accept(p); + } + return p; + } + public Object getValue() { return value; } diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/BeanD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/BeanD.java index 7dc240386e..9973f4c024 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/BeanD.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/BeanD.java @@ -26,8 +26,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import jakarta.validation.metadata.BeanDescriptor; import jakarta.validation.metadata.ConstructorDescriptor; @@ -63,8 +61,8 @@ public class BeanD extends ElementD, MetadataReader.ForBean> impl * Requested group strategy to the strategy with {@link Group#DEFAULT} redefined as this bean's default group * sequence; {@link #UNCHANGED} when the redefinition is a no-op. */ - private final ConcurrentMap localGroupStrategies = new ConcurrentHashMap<>(); - private final ConcurrentMap[]> propertiesByGroups = new ConcurrentHashMap<>(); + private final GroupStrategyMap localGroupStrategies = new GroupStrategyMap<>(); + private final GroupStrategyMap[]> propertiesByGroups = new GroupStrategyMap<>(); private static final GroupStrategy UNCHANGED = GroupStrategy.simple(Collections.emptySet()); @@ -170,13 +168,11 @@ public GroupStrategy getGroupStrategy() { * @return {@link GroupStrategy} */ public GroupStrategy getLocalGroupStrategy(GroupStrategy groups) { - GroupStrategy result = localGroupStrategies.get(groups); - if (result == null) { + final GroupStrategy result = localGroupStrategies.computeIfAbsent(groups, g -> { final GroupStrategy computed = - GroupStrategy.redefining(groups, Collections.singletonMap(Group.DEFAULT, groupStrategy)); - result = computed == groups ? UNCHANGED : computed; - localGroupStrategies.putIfAbsent(groups, result); - } + GroupStrategy.redefining(g, Collections.singletonMap(Group.DEFAULT, groupStrategy)); + return computed == g ? UNCHANGED : computed; + }); return result == UNCHANGED ? groups : result; } @@ -189,9 +185,8 @@ public GroupStrategy getLocalGroupStrategy(GroupStrategy groups) { * @return {@link PropertyD} array, possibly empty */ public PropertyD[] getPropertiesFor(GroupStrategy groups) { - PropertyD[] result = propertiesByGroups.get(groups); - if (result == null) { - final Set targetGroups = groups.getGroups(); + return propertiesByGroups.computeIfAbsent(groups, g -> { + final Set targetGroups = g.getGroups(); final List> relevant = new ArrayList<>(leafProperties.length); for (PropertyD p : leafProperties) { if (p.isCascadedDeep() || !p.getConstrainedContainerElementTypes().isEmpty() @@ -200,13 +195,8 @@ public PropertyD[] getPropertiesFor(GroupStrategy groups) { relevant.add(p); } } - result = relevant.toArray(new PropertyD[0]); - final PropertyD[] previous = propertiesByGroups.putIfAbsent(groups, result); - if (previous != null) { - result = previous; - } - } - return result; + return relevant.toArray(new PropertyD[0]); + }); } public final Type getGenericType() { diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ElementD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ElementD.java index a46c8f0ff8..f9bbe38c6a 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ElementD.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ElementD.java @@ -24,8 +24,6 @@ import java.lang.reflect.TypeVariable; import java.util.Map; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import jakarta.validation.groups.Default; import jakarta.validation.metadata.ConstraintDescriptor; @@ -102,7 +100,7 @@ public static boolean matchesGroups(ConstraintD constraint, Set target private final Meta meta; private final Set> constraints; - private final ConcurrentMap[]> constraintsByGroups; + private final GroupStrategyMap[]> constraintsByGroups; protected ElementD(R reader) { super(); @@ -110,7 +108,7 @@ protected ElementD(R reader) { this.meta = reader.meta; this.genericType = reader.meta.getType(); this.constraints = reader.getConstraints(); - this.constraintsByGroups = constraints.isEmpty() ? null : new ConcurrentHashMap<>(); + this.constraintsByGroups = constraints.isEmpty() ? null : new GroupStrategyMap<>(); this.groupsComputer = reader.getValidatorFactory().getGroupsComputer(); } @@ -136,15 +134,10 @@ public ConstraintD[] getConstraintsFor(GroupStrategy groups) { if (constraintsByGroups == null) { return NO_CONSTRAINTS; } - final ConstraintD[] cached = constraintsByGroups.get(groups); - if (cached != null) { - return cached; - } - final Set targetGroups = groups.getGroups(); - final ConstraintD[] result = - constraints.stream().filter(c -> matchesGroups(c, targetGroups)).toArray(ConstraintD[]::new); - final ConstraintD[] previous = constraintsByGroups.putIfAbsent(groups, result); - return previous == null ? result : previous; + return constraintsByGroups.computeIfAbsent(groups, g -> { + final Set targetGroups = g.getGroups(); + return constraints.stream().filter(c -> matchesGroups(c, targetGroups)).toArray(ConstraintD[]::new); + }); } @Override diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/GroupStrategyMap.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/GroupStrategyMap.java new file mode 100644 index 0000000000..5fe58f6163 --- /dev/null +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/GroupStrategyMap.java @@ -0,0 +1,68 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.bval.jsr.descriptor; + +import java.util.Arrays; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.function.Function; + +import org.apache.bval.jsr.groups.GroupStrategy; + +/** + * Thread-safe cache of values computed per {@link GroupStrategy}. Validation passes the same few strategy instances + * over and over, so lookups first scan a small array of recently seen instances by identity and only then fall back + * to an equality-keyed map. + * + * @param value type + */ +final class GroupStrategyMap { + private static final int MAX_IDENTITY_ENTRIES = 8; + private static final Object[] NO_ENTRIES = {}; + + private final ConcurrentMap byEquality = new ConcurrentHashMap<>(); + /** Alternating key, value pairs; replaced wholesale, so readers always see a consistent array. */ + private volatile Object[] byIdentity = NO_ENTRIES; + + @SuppressWarnings("unchecked") + V computeIfAbsent(GroupStrategy key, Function compute) { + final Object[] entries = byIdentity; + for (int i = 0; i < entries.length; i += 2) { + if (entries[i] == key) { + return (V) entries[i + 1]; + } + } + V value = byEquality.get(key); + if (value == null) { + value = compute.apply(key); + final V previous = byEquality.putIfAbsent(key, value); + if (previous != null) { + value = previous; + } + } + if (entries.length < 2 * MAX_IDENTITY_ENTRIES) { + // racing updates may drop an entry, which only costs a later equality lookup + final Object[] grown = Arrays.copyOf(entries, entries.length + 2); + grown[entries.length] = key; + grown[entries.length + 1] = value; + byIdentity = grown; + } + return value; + } +} diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java index 85aabc1e1e..f61c36efed 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java @@ -21,7 +21,6 @@ import java.lang.reflect.Array; import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; -import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashSet; @@ -267,22 +266,37 @@ void process(GroupStrategy groups, Consumer> sink) { Validate.notNull(sink, "sink"); final GroupStrategy localGroupStrategy = descriptor.getLocalGroupStrategy(groups); final boolean redefined = localGroupStrategy != groups; - final List> propertyFrames = propertyFrames(descriptor.getPropertiesFor(localGroupStrategy)); + final PropertyFrames propertyFrames = new PropertyFrames(descriptor.getPropertiesFor(localGroupStrategy)); + final PropertyD[] properties = propertyFrames.properties; localGroupStrategy.applyTo(gs -> { final int originalCount = violationCount(); validateDescriptorConstraints(gs, sink); - for (final Frame p : propertyFrames) { - p.validateDescriptorConstraints(gs, sink); - if (!redefined) { - p.recurse(gs, sink); + for (int i = 0; i < properties.length; i++) { + final PropertyD d = properties[i]; + final boolean recurse = !redefined && d.isCascadedDeep(); + if (!recurse && d.getConstraintsFor(gs).length == 0 + && d.getConstrainedContainerElementTypes().isEmpty()) { + continue; + } + final Frame p = propertyFrames.get(i); + if (p != null) { + p.validateDescriptorConstraints(gs, sink); + if (recurse) { + p.recurse(gs, sink); + } } } return violationCount() == originalCount; }); if (redefined) { - for (final Frame p : propertyFrames) { - p.recurse(groups, sink); + for (int i = 0; i < properties.length; i++) { + if (properties[i].isCascadedDeep()) { + final Frame p = propertyFrames.get(i); + if (p != null) { + p.recurse(groups, sink); + } + } } } } @@ -296,21 +310,43 @@ Object getBean() { return context.getValue(); } - private List> propertyFrames(PropertyD[] properties) { - if (properties.length == 0) { - return Collections.emptyList(); + /** + * Frames of the properties of this bean, created on first use so that properties relevant only to groups + * that are never validated (e.g. later steps of a failing group sequence) are not read. + */ + private class PropertyFrames { + final PropertyD[] properties; + private Object[] frames; + + PropertyFrames(PropertyD[] properties) { + this.properties = properties; } - final TraversableResolver traversableResolver = validatorContext.getTraversableResolver(); - final boolean traverseAll = traversesAll(traversableResolver); - final List> frames = new ArrayList<>(properties.length); - for (final PropertyD d : properties) { - if (!traverseAll) { + + /** + * @return the frame for {@code properties[i]}, or {@code null} if that property is not reachable or its + * value is already being validated further up the graph + */ + Frame get(int i) { + if (frames == null) { + frames = new Object[properties.length]; + } + Object frame = frames[i]; + if (frame == null) { + frame = create(properties[i]); + frames[i] = frame; + } + return frame == NO_FRAME ? null : (Frame) frame; + } + + private Object create(PropertyD d) { + final TraversableResolver traversableResolver = validatorContext.getTraversableResolver(); + if (!traversesAll(traversableResolver)) { final PathImpl p = realContext.getPath(); p.addProperty(d.getPropertyName()); try { if (!traversableResolver.isReachable(context.getValue(), p.removeLeafNode(), getRootBeanClass(), p, d.getElementType())) { - continue; + return NO_FRAME; } } catch (ValidationException ve) { throw ve; @@ -319,11 +355,8 @@ private List> propertyFrames(PropertyD[] properties) { } } final GraphContext child = d.readChild(realContext); - if (child != null && !child.isRecursive()) { - frames.add(propertyFrame(d, child)); - } + return child == null || child.isRecursive() ? NO_FRAME : propertyFrame(d, child); } - return frames; } } @@ -574,6 +607,8 @@ Object getBean() { } } + private static final Object NO_FRAME = new Object(); + protected static final TypeVariable MAP_VALUE = Map.class.getTypeParameters()[1]; protected static final TypeVariable ITERABLE_ELEMENT = Iterable.class.getTypeParameters()[0]; diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java b/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java index 1cd8a0e208..5e87ac85e2 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java @@ -179,8 +179,11 @@ private PathImpl(Path path) { if (path instanceof PathImpl) { final PathImpl source = (PathImpl) path; // share node references; the leaf will be copied on first mutation by whichever path mutates it - nodeList = new ArrayList<>(source.nodeList.size() + 2); - nodeList.addAll(source.nodeList); + final int size = source.nodeList.size(); + nodeList = new ArrayList<>(size + 2); + for (int i = 0; i < size; i++) { + nodeList.add(source.nodeList.get(i)); + } if (!nodeList.isEmpty()) { sharedLeaf = true; // the source now shares its leaf with this copy, so it must copy-on-write too From 8168739e6b613657db32ddad775ee110dd24eabc Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Mon, 28 Sep 2026 09:33:59 +0200 Subject: [PATCH 3/8] Resolve bean descriptors by runtime class in a single lookup Cache descriptors keyed by the runtime (possibly proxy) class of the validated object so that each validation needs one map lookup instead of unwrapping the class and then looking up its descriptor. --- .../jsr/descriptor/DescriptorManager.java | 22 +++++++++++++++++++ .../org/apache/bval/jsr/job/ValidateBean.java | 12 +--------- .../apache/bval/jsr/job/ValidationJob.java | 11 ++-------- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/DescriptorManager.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/DescriptorManager.java index 5558785a54..cce9952d2a 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/DescriptorManager.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/DescriptorManager.java @@ -38,6 +38,7 @@ import org.apache.bval.jsr.metadata.HierarchyBuilder; import org.apache.bval.jsr.metadata.MetadataBuilder; import org.apache.bval.jsr.metadata.ReflectionBuilder; +import org.apache.bval.jsr.util.Proxies; import org.apache.bval.util.Validate; public class DescriptorManager { @@ -62,6 +63,8 @@ public static boolean isConstrained(E descripto private final ApacheValidatorFactory validatorFactory; private final ConcurrentMap, BeanD> beanDescriptors = new ConcurrentHashMap<>(); + /** Keyed by the runtime class of validated objects, which may be a proxy of the described class. */ + private final ConcurrentMap, BeanD> beanDescriptorsByRuntimeClass = new ConcurrentHashMap<>(); // synchronization unnecessary private final ReflectionBuilder reflectionBuilder; @@ -84,8 +87,27 @@ public BeanDescriptor getBeanDescriptor(Class beanClass) { return previous == null ? value : previous; } + /** + * Get the descriptor for an object of runtime type {@code runtimeClass}, which is first unwrapped if it is a + * proxy class. + * + * @param runtimeClass + * @return {@link BeanD} + * @see Proxies#classFor(Class) + */ + public BeanD getBeanDescriptorForRuntimeClass(Class runtimeClass) { + final BeanD existing = beanDescriptorsByRuntimeClass.get(runtimeClass); + if (existing != null) { + return existing; + } + final BeanD result = (BeanD) getBeanDescriptor(Proxies.classFor(runtimeClass)); + beanDescriptorsByRuntimeClass.putIfAbsent(runtimeClass, result); + return result; + } + public void clear() { beanDescriptors.clear(); + beanDescriptorsByRuntimeClass.clear(); } private MetadataBuilder.ForBean builder(Class beanClass) { diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateBean.java b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateBean.java index 526c39981d..381604c4d6 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateBean.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateBean.java @@ -24,11 +24,8 @@ import org.apache.bval.jsr.descriptor.BeanD; import org.apache.bval.jsr.descriptor.ConstraintD; import org.apache.bval.jsr.util.PathImpl; -import org.apache.bval.jsr.util.Proxies; import org.apache.bval.util.Validate; -import java.util.Map; - public final class ValidateBean extends ValidationJob { private final T bean; @@ -41,16 +38,9 @@ public final class ValidateBean extends ValidationJob { @Override protected boolean hasWork() { - final Class beanClass = bean.getClass(); - final Map, Class> classCache = validatorContext.getFactory().getUnwrappedClassCache(); - Class unwrappedClass = classCache.get(beanClass); - if (unwrappedClass == null) { - unwrappedClass = Proxies.classFor(beanClass); - classCache.putIfAbsent(beanClass, unwrappedClass); - } @SuppressWarnings("unchecked") final BeanD d = - (BeanD) validatorContext.getFactory().getDescriptorManager().getBeanDescriptor(unwrappedClass); + (BeanD) validatorContext.getDescriptorManager().getBeanDescriptorForRuntimeClass(bean.getClass()); descriptor = d; return d.isBeanConstrained(); } diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java index f61c36efed..a418eda9c5 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java @@ -66,7 +66,6 @@ import org.apache.bval.jsr.resolver.DefaultTraversableResolver; import org.apache.bval.jsr.util.NodeImpl; import org.apache.bval.jsr.util.PathImpl; -import org.apache.bval.jsr.util.Proxies; import org.apache.bval.jsr.valueextraction.ExtractValues; import org.apache.bval.jsr.valueextraction.ValueExtractors; import org.apache.bval.util.Exceptions; @@ -660,14 +659,8 @@ private void addResult(ConstraintViolation violation) { @SuppressWarnings("unchecked") private BeanD getBeanDescriptor(Object bean) { - final Class beanClass = Validate.notNull(bean, "bean").getClass(); - final Map, Class> classCache = validatorContext.getFactory().getUnwrappedClassCache(); - Class unwrappedClass = classCache.get(beanClass); - if (unwrappedClass == null) { - unwrappedClass = Proxies.classFor(beanClass); - classCache.putIfAbsent(beanClass, unwrappedClass); - } - return (BeanD) validatorContext.getDescriptorManager().getBeanDescriptor(unwrappedClass); + return (BeanD) validatorContext.getDescriptorManager() + .getBeanDescriptorForRuntimeClass(Validate.notNull(bean, "bean").getClass()); } final ConstraintViolationImpl createViolation(String messageTemplate, ConstraintValidatorContextImpl context, From 14dde9a54e182379cc628825a94c38885e57ce2a Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Mon, 28 Sep 2026 09:58:00 +0200 Subject: [PATCH 4/8] Add scenario benchmarks comparing BVal and Hibernate Validator ScenarioBenchmark runs each scenario against both providers, selected by a JMH parameter, and checks the expected violation counts once per trial: mixed valid/invalid beans, explicit groups, validateProperty/Value, parameter and return value validation, cascading into large collections and cycles, container element constraints, deep graphs, beans with many built-in constraints (valid and fully invalid), composed and class-level constraints, and EL message expressions. Also run the JMH annotation processor explicitly, as JDK 23+ no longer does so implicitly, and register Tomcat's ExpressionFactory ahead of the bval-jsr test fixture that re-resolves its delegate on every call. --- bval-perf/pom.xml | 13 + .../apache/bval/jsr/ScenarioBenchmark.java | 653 ++++++++++++++++++ .../services/jakarta.el.ExpressionFactory | 19 + 3 files changed, 685 insertions(+) create mode 100644 bval-perf/src/test/java/org/apache/bval/jsr/ScenarioBenchmark.java create mode 100644 bval-perf/src/test/resources/META-INF/services/jakarta.el.ExpressionFactory diff --git a/bval-perf/pom.xml b/bval-perf/pom.xml index 23fa34c350..6891dac612 100644 --- a/bval-perf/pom.xml +++ b/bval-perf/pom.xml @@ -93,4 +93,17 @@ + + + + org.apache.maven.plugins + maven-compiler-plugin + + + full + + + + + diff --git a/bval-perf/src/test/java/org/apache/bval/jsr/ScenarioBenchmark.java b/bval-perf/src/test/java/org/apache/bval/jsr/ScenarioBenchmark.java new file mode 100644 index 0000000000..e58ac9df4e --- /dev/null +++ b/bval-perf/src/test/java/org/apache/bval/jsr/ScenarioBenchmark.java @@ -0,0 +1,653 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.bval.jsr; + +import static java.lang.annotation.ElementType.ANNOTATION_TYPE; +import static java.lang.annotation.ElementType.FIELD; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; +import java.lang.reflect.Method; +import java.math.BigDecimal; +import java.time.LocalDate; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Random; +import java.util.Set; +import java.util.concurrent.TimeUnit; + +import org.hibernate.validator.HibernateValidator; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; + +import jakarta.validation.Constraint; +import jakarta.validation.ConstraintValidator; +import jakarta.validation.ConstraintValidatorContext; +import jakarta.validation.ConstraintViolation; +import jakarta.validation.Payload; +import jakarta.validation.ReportAsSingleViolation; +import jakarta.validation.Valid; +import jakarta.validation.Validation; +import jakarta.validation.Validator; +import jakarta.validation.ValidatorFactory; +import jakarta.validation.constraints.AssertFalse; +import jakarta.validation.constraints.AssertTrue; +import jakarta.validation.constraints.DecimalMin; +import jakarta.validation.constraints.Digits; +import jakarta.validation.constraints.Email; +import jakarta.validation.constraints.Future; +import jakarta.validation.constraints.Max; +import jakarta.validation.constraints.Min; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.NotEmpty; +import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Null; +import jakarta.validation.constraints.Past; +import jakarta.validation.constraints.Pattern; +import jakarta.validation.constraints.Positive; +import jakarta.validation.constraints.PositiveOrZero; +import jakarta.validation.constraints.Size; +import jakarta.validation.executable.ExecutableValidator; + +/** + * Validation scenarios run against each provider under identical conditions, selected by {@link #provider}. The + * expected number of violations of every scenario is checked once per trial, so a provider cannot look fast by + * skipping work. + */ +@State(Scope.Benchmark) +@BenchmarkMode(Mode.Throughput) +@OutputTimeUnit(TimeUnit.MILLISECONDS) +public class ScenarioBenchmark { + private static final int DRIVERS = 128; // power of two, see Cursor + + @Param({ "bval", "hibernate" }) + public String provider; + + private ValidatorFactory factory; + private Validator validator; + private ExecutableValidator executableValidator; + + private Driver[] drivers; + private Object[][] driverArguments; + private final DriverService driverService = new DriverService(); + private Method createDriver; + private Method register; + + private Shop shop; + private Shop cyclicShop; + private Library library; + private Node deepGraph; + private Registration validRegistration; + private Registration invalidRegistration; + private Voucher validVoucher; + private Voucher invalidVoucher; + private Booking validBooking; + private Booking invalidBooking; + private Tag invalidTag; + + /** + * Round-robin position in the pregenerated drivers, per thread. + */ + @State(Scope.Thread) + public static class Cursor { + private int position; + + int next() { + position = (position + 1) & (DRIVERS - 1); + return position; + } + } + + @Setup(Level.Trial) + public void setUp() throws NoSuchMethodException { + switch (provider) { + case "bval": + factory = Validation.byProvider(ApacheValidationProvider.class).configure().buildValidatorFactory(); + break; + case "hibernate": + factory = Validation.byProvider(HibernateValidator.class).configure().buildValidatorFactory(); + break; + default: + throw new IllegalArgumentException(provider); + } + validator = factory.getValidator(); + executableValidator = validator.forExecutables(); + createDriver = DriverService.class.getMethod("createDriver", String.class, int.class, boolean.class); + register = DriverService.class.getMethod("register", Driver.class); + + final Random random = new Random(42); + drivers = new Driver[DRIVERS]; + driverArguments = new Object[DRIVERS][]; + for (int i = 0; i < DRIVERS; i++) { + drivers[i] = Driver.random(random); + driverArguments[i] = + new Object[] { drivers[i].name, Integer.valueOf(drivers[i].age), drivers[i].hasDrivingLicense }; + } + shop = Shop.withArticles(1000, false); + cyclicShop = Shop.withArticles(200, true); + library = Library.create(); + deepGraph = Node.chain(8); + validRegistration = Registration.valid(); + invalidRegistration = Registration.invalid(); + validVoucher = new Voucher("ABC-123"); + invalidVoucher = new Voucher(""); + validBooking = new Booking(LocalDate.of(2026, 1, 1), LocalDate.of(2026, 1, 8)); + invalidBooking = new Booking(LocalDate.of(2026, 1, 8), LocalDate.of(2026, 1, 1)); + invalidTag = new Tag("abcdef"); + + verifyExpectedViolations(); + } + + @TearDown(Level.Trial) + public void tearDown() { + factory.close(); + } + + private void verifyExpectedViolations() { + for (int i = 0; i < DRIVERS; i++) { + final Driver d = drivers[i]; + expect("validate(driver)", d.expectedDefault, validator.validate(d)); + expect("validate(driver, Strict)", d.expectedStrict, validator.validate(d, Strict.class)); + expect("validateProperty", d.age < 18 ? 1 : 0, validator.validateProperty(d, "age")); + expect("validateValue", d.name == null || d.name.length() < 2 ? 1 : 0, + validator.validateValue(Driver.class, "name", d.name)); + expect("validateParameters", d.expectedDefault, + executableValidator.validateParameters(driverService, createDriver, driverArguments[i])); + expect("validateReturnValue", d.expectedDefault, + executableValidator.validateReturnValue(driverService, register, d)); + } + expect("cascadedList", 0, validator.validate(shop)); + expect("cascadedCycles", 0, validator.validate(cyclicShop)); + expect("containerElements", 2, validator.validate(library)); + expect("deepGraph", 0, validator.validate(deepGraph)); + expect("wideBeanValid", 0, validator.validate(validRegistration)); + expect("wideBeanInvalid", 16, validator.validate(invalidRegistration)); + expect("composedValid", 0, validator.validate(validVoucher)); + expect("composedInvalid", 1, validator.validate(invalidVoucher)); + expect("classLevelValid", 0, validator.validate(validBooking)); + expect("classLevelInvalid", 1, validator.validate(invalidBooking)); + expect("elMessage", 1, validator.validate(invalidTag)); + final String message = validator.validate(invalidTag).iterator().next().getMessage(); + if (!"'abcdef' is longer than 3".equals(message)) { + throw new IllegalStateException(provider + " elMessage: unexpected message " + message); + } + } + + private void expect(String scenario, int expected, Set> violations) { + if (violations.size() != expected) { + throw new IllegalStateException( + String.format("%s %s: expected %d violations, got %s", provider, scenario, expected, violations)); + } + } + + // --- benchmarks --- + + /** Mix of valid and invalid simple beans, as a stream of requests would bring. */ + @Benchmark + public Set> mixedDrivers(Cursor cursor) { + return validator.validate(drivers[cursor.next()]); + } + + /** Validation of an explicitly requested, non-default group. */ + @Benchmark + public Set> explicitGroup(Cursor cursor) { + return validator.validate(drivers[cursor.next()], Strict.class); + } + + @Benchmark + public Set> validateProperty(Cursor cursor) { + return validator.validateProperty(drivers[cursor.next()], "age"); + } + + @Benchmark + public Set> validateValue(Cursor cursor) { + return validator.validateValue(Driver.class, "name", drivers[cursor.next()].name); + } + + @Benchmark + public Set> executableParameters(Cursor cursor) { + return executableValidator.validateParameters(driverService, createDriver, driverArguments[cursor.next()]); + } + + /** Return value validation cascading into the returned bean. */ + @Benchmark + public Set> executableReturnValue(Cursor cursor) { + return executableValidator.validateReturnValue(driverService, register, drivers[cursor.next()]); + } + + /** Cascading into a list of 1000 valid elements. */ + @Benchmark + public Set> cascadedList() { + return validator.validate(shop); + } + + /** Cascading into 200 elements that each refer back to their container. */ + @Benchmark + public Set> cascadedCycles() { + return validator.validate(cyclicShop); + } + + /** Constraints on map keys, list elements and an Optional, with cascading through them. */ + @Benchmark + public Set> containerElements() { + return validator.validate(library); + } + + /** A chain of eight cascaded beans. */ + @Benchmark + public Set> deepGraph() { + return validator.validate(deepGraph); + } + + /** A bean with many different built-in constraints, all satisfied. */ + @Benchmark + public Set> wideBeanValid() { + return validator.validate(validRegistration); + } + + /** The same bean with every constraint violated, so 16 messages are interpolated. */ + @Benchmark + public Set> wideBeanInvalid() { + return validator.validate(invalidRegistration); + } + + @Benchmark + public Set> composedValid() { + return validator.validate(validVoucher); + } + + /** A failing composed constraint reported as a single violation. */ + @Benchmark + public Set> composedInvalid() { + return validator.validate(invalidVoucher); + } + + @Benchmark + public Set> classLevelValid() { + return validator.validate(validBooking); + } + + /** A failing class-level constraint that reports a custom violation on a property node. */ + @Benchmark + public Set> classLevelInvalid() { + return validator.validate(invalidBooking); + } + + /** A failing constraint whose message uses an EL expression. */ + @Benchmark + public Set> elMessage() { + return validator.validate(invalidTag); + } + + // --- model --- + + public interface Strict { + } + + public static class Driver { + private static final String[] NAMES = { null, "Jacob", "Isabella", "Ethan", "Sophia", "Michael", "Emma", "J" }; + + @NotNull + @Size(min = 2, max = 30) + private final String name; + + @Min(18) + private final int age; + + @AssertTrue + private final boolean hasDrivingLicense; + + @NotBlank(groups = Strict.class) + @Email(groups = Strict.class) + private final String email; + + final int expectedDefault; + final int expectedStrict; + + Driver(String name, int age, boolean hasDrivingLicense, String email) { + this.name = name; + this.age = age; + this.hasDrivingLicense = hasDrivingLicense; + this.email = email; + this.expectedDefault = + (name == null || name.length() < 2 ? 1 : 0) + (age < 18 ? 1 : 0) + (hasDrivingLicense ? 0 : 1); + this.expectedStrict = email.contains("@") ? 0 : 1; + } + + static Driver random(Random random) { + return new Driver(NAMES[random.nextInt(NAMES.length)], random.nextInt(40), random.nextInt(4) != 0, + random.nextInt(4) == 0 ? "not-an-email" : "driver@example.org"); + } + } + + public static class DriverService { + public Driver createDriver(@NotNull @Size(min = 2, max = 30) String name, @Min(18) int age, + @AssertTrue boolean hasDrivingLicense) { + return new Driver(name, age, hasDrivingLicense, "driver@example.org"); + } + + @Valid + public Driver register(Driver driver) { + return driver; + } + } + + public static class Shop { + @NotNull + private final Integer id; + + @NotNull + private final List<@Valid @NotNull Article> articles = new ArrayList<>(); + + Shop(Integer id) { + this.id = id; + } + + static Shop withArticles(int count, boolean linkBack) { + final Shop shop = new Shop(1); + for (int i = 0; i < count; i++) { + final Article article = new Article(i, "article " + i); + shop.articles.add(article); + if (linkBack) { + article.shops.add(shop); + } + } + return shop; + } + } + + public static class Article { + @NotNull + private final Integer id; + + @Size(min = 1, max = 50) + private final String name; + + private final Set<@Valid Shop> shops = new HashSet<>(); + + Article(Integer id, String name) { + this.id = id; + this.name = name; + } + } + + public static class Library { + @NotNull + private final Map<@NotBlank String, @NotNull @Size(max = 50) List<@NotNull @Valid Book>> shelves; + + private final Optional<@Valid Author> curator; + + Library(Map> shelves, Optional curator) { + this.shelves = shelves; + this.curator = curator; + } + + /** 20 shelves of 10 books; one shelf has a blank name and one book has no pages. */ + static Library create() { + final Map> shelves = new LinkedHashMap<>(); + for (int s = 0; s < 20; s++) { + final List books = new ArrayList<>(); + for (int b = 0; b < 10; b++) { + books.add(new Book("book " + s + '/' + b, s == 3 && b == 7 ? 0 : 100 + b, + new Author("author " + b, "author" + b + "@example.org"))); + } + shelves.put(s == 5 ? " " : "shelf " + s, books); + } + return new Library(shelves, Optional.of(new Author("curator", "curator@example.org"))); + } + } + + public static class Book { + @NotBlank + private final String title; + + @Min(1) + private final int pages; + + @Valid + private final Author author; + + Book(String title, int pages, Author author) { + this.title = title; + this.pages = pages; + this.author = author; + } + } + + public static class Author { + @NotBlank + private final String name; + + @Email + private final String email; + + Author(String name, String email) { + this.name = name; + this.email = email; + } + } + + public static class Node { + @NotNull + private final String label; + + @Min(0) + private final int weight; + + @Valid + private final Node child; + + Node(String label, int weight, Node child) { + this.label = label; + this.weight = weight; + this.child = child; + } + + static Node chain(int depth) { + Node node = null; + for (int i = depth; i > 0; i--) { + node = new Node("node " + i, i, node); + } + return node; + } + } + + public static class Registration { + @NotBlank + @Size(max = 40) + private String firstName; + + @NotBlank + @Size(max = 40) + private String lastName; + + @NotNull + @Email + private String email; + + @Pattern(regexp = "\\+?[0-9 ]{6,20}") + private String phone; + + @Min(0) + @Max(150) + private int age; + + @Past + private LocalDate birthDate; + + @Future + private LocalDate membershipEnd; + + @Digits(integer = 6, fraction = 2) + private BigDecimal balance; + + @DecimalMin("0.00") + private BigDecimal credit; + + @Size(min = 1, max = 5) + private List tags; + + @NotEmpty + private String country; + + @Pattern(regexp = "[0-9]{5}") + private String zip; + + @Positive + private int loginCount; + + @PositiveOrZero + private long points; + + @AssertFalse + private boolean banned; + + @Null + private String internalNote; + + static Registration valid() { + final Registration r = new Registration(); + r.firstName = "Ada"; + r.lastName = "Lovelace"; + r.email = "ada@example.org"; + r.phone = "+44 20 7946 0000"; + r.age = 36; + r.birthDate = LocalDate.now().minusYears(36); + r.membershipEnd = LocalDate.now().plusYears(1); + r.balance = new BigDecimal("1234.50"); + r.credit = new BigDecimal("10.00"); + r.tags = Arrays.asList("math", "engines"); + r.country = "GB"; + r.zip = "12345"; + r.loginCount = 3; + r.points = 0; + return r; + } + + /** Violates each of the 16 annotated properties exactly once. */ + static Registration invalid() { + final Registration r = new Registration(); + r.firstName = ""; + r.lastName = "x".repeat(50); + r.email = "nope"; + r.phone = "abc"; + r.age = 200; + r.birthDate = LocalDate.now().plusYears(1); + r.membershipEnd = LocalDate.now().minusYears(1); + r.balance = new BigDecimal("1234567.123"); + r.credit = new BigDecimal("-1"); + r.tags = Collections.emptyList(); + r.country = ""; + r.zip = "12"; + r.loginCount = 0; + r.points = -1; + r.banned = true; + r.internalNote = "x"; + return r; + } + } + + @Target({ FIELD, ANNOTATION_TYPE }) + @Retention(RUNTIME) + @Constraint(validatedBy = {}) + @ReportAsSingleViolation + @NotNull + @NotBlank + @Size(min = 3, max = 16) + @Pattern(regexp = "[A-Z0-9-]+") + public @interface ValidCode { + String message() default "invalid code"; + + Class[] groups() default {}; + + Class[] payload() default {}; + } + + public static class Voucher { + @ValidCode + private final String code; + + Voucher(String code) { + this.code = code; + } + } + + @Target(TYPE) + @Retention(RUNTIME) + @Constraint(validatedBy = DateRangeValidator.class) + public @interface DateRange { + String message() default "end must not be before start"; + + Class[] groups() default {}; + + Class[] payload() default {}; + } + + public static class DateRangeValidator implements ConstraintValidator { + @Override + public boolean isValid(Booking booking, ConstraintValidatorContext context) { + if (booking == null || booking.from == null || booking.to == null || !booking.to.isBefore(booking.from)) { + return true; + } + context.disableDefaultConstraintViolation(); + context.buildConstraintViolationWithTemplate(context.getDefaultConstraintMessageTemplate()) + .addPropertyNode("to").addConstraintViolation(); + return false; + } + } + + @DateRange + public static class Booking { + @NotNull + private final LocalDate from; + + @NotNull + private final LocalDate to; + + Booking(LocalDate from, LocalDate to) { + this.from = from; + this.to = to; + } + } + + public static class Tag { + @Size(max = 3, message = "'${validatedValue}' is longer than {max}") + private final String value; + + Tag(String value) { + this.value = value; + } + } +} diff --git a/bval-perf/src/test/resources/META-INF/services/jakarta.el.ExpressionFactory b/bval-perf/src/test/resources/META-INF/services/jakarta.el.ExpressionFactory new file mode 100644 index 0000000000..da7fc2f9bb --- /dev/null +++ b/bval-perf/src/test/resources/META-INF/services/jakarta.el.ExpressionFactory @@ -0,0 +1,19 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. +# Takes precedence over the bval-jsr test fixture DelegateExpressionFactory (from the tests jar on this module's +# classpath), which looks up its delegate on every call and would dominate any benchmark that evaluates EL. +org.apache.el.ExpressionFactoryImpl From 7861911668484913e6b1e20d3344d70183219681 Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Mon, 28 Sep 2026 14:02:45 +0200 Subject: [PATCH 5/8] Neutralize deferred EL expressions without a look-behind regex ELFacade ran a look-behind regular expression with over a hundred alternations across every message containing "${" to disarm unescaped "#{" sequences. It dominated the cost of any violation whose message uses EL, including several built-in default messages. A direct scan gives the same result and returns immediately when there is no "#{". --- .../java/org/apache/bval/el/ELFacade.java | 47 +++++++++++-------- .../java/org/apache/bval/el/ELFacadeTest.java | 37 +++++++++++++++ 2 files changed, 64 insertions(+), 20 deletions(-) create mode 100644 bval-jsr/src/test/java/org/apache/bval/el/ELFacadeTest.java diff --git a/bval-jsr/src/main/java/org/apache/bval/el/ELFacade.java b/bval-jsr/src/main/java/org/apache/bval/el/ELFacade.java index 6e53509eac..f93951357a 100644 --- a/bval-jsr/src/main/java/org/apache/bval/el/ELFacade.java +++ b/bval-jsr/src/main/java/org/apache/bval/el/ELFacade.java @@ -34,27 +34,10 @@ import jakarta.el.ValueExpression; import jakarta.el.VariableMapper; -import org.apache.bval.jsr.util.LookBehindRegexHolder; import org.apache.bval.util.Lazy; // ELProcessor or JavaEE 7 would be perfect too but this impl can be used in javaee 6 public final class ELFacade implements MessageEvaluator { - private enum EvaluationType { - IMMEDIATE("\\$"), DEFERRED("#"); - - /** - * {@link LookBehindRegexHolder} to recognize a non-escaped EL - * expression of this evaluation type, hallmarked by a trigger - * character. - */ - private final LookBehindRegexHolder regex; - - private EvaluationType(String trigger) { - this.regex = new LookBehindRegexHolder( - String.format("(? (n - 3) / 2); - } - } - private static final ELResolver RESOLVER = initResolver(); // Lazily initialized: ExpressionFactory.newInstance() performs a relatively expensive @@ -78,9 +61,8 @@ public String interpolate(final String message, final Map annota expressionFactory.createValueExpression(validatedValue, Object.class)); // Java Bean Validation does not support EL expressions that look like JSP "deferred" expressions - return expressionFactory.createValueExpression(context, - EvaluationType.DEFERRED.regex.matcher(message).replaceAll("\\$0"), String.class).getValue(context) - .toString(); + return expressionFactory.createValueExpression(context, disarmDeferredExpressions(message), String.class) + .getValue(context).toString(); } catch (final Exception e) { // no-op } @@ -88,6 +70,31 @@ public String interpolate(final String message, final Map annota return message; } + /** + * Replace each {@code #{} that is not escaped by an odd number of preceding backslashes with {@code $0}, so that + * it is not evaluated as a deferred expression. + */ + static String disarmDeferredExpressions(String message) { + int trigger = message.indexOf("#{"); + if (trigger < 0) { + return message; + } + final StringBuilder result = new StringBuilder(message.length()); + int copied = 0; + do { + int backslashes = 0; + while (trigger - backslashes > 0 && message.charAt(trigger - backslashes - 1) == '\\') { + backslashes++; + } + if (backslashes % 2 == 0) { + result.append(message, copied, trigger).append("$0"); + copied = trigger + 2; + } + trigger = message.indexOf("#{", trigger + 2); + } while (trigger >= 0); + return result.append(message, copied, message.length()).toString(); + } + private static ELResolver initResolver() { final CompositeELResolver resolver = new CompositeELResolver(); resolver.add(new MapELResolver()); diff --git a/bval-jsr/src/test/java/org/apache/bval/el/ELFacadeTest.java b/bval-jsr/src/test/java/org/apache/bval/el/ELFacadeTest.java new file mode 100644 index 0000000000..1f3ed89a4f --- /dev/null +++ b/bval-jsr/src/test/java/org/apache/bval/el/ELFacadeTest.java @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.bval.el; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; + +public class ELFacadeTest { + + @Test + public void disarmsUnescapedDeferredExpressions() { + assertEquals("no deferred ${x}", ELFacade.disarmDeferredExpressions("no deferred ${x}")); + assertEquals("a $0x} b", ELFacade.disarmDeferredExpressions("a #{x} b")); + assertEquals("$0x}", ELFacade.disarmDeferredExpressions("#{x}")); + assertEquals("a \\#{x} b", ELFacade.disarmDeferredExpressions("a \\#{x} b")); + assertEquals("a \\\\$0x} b", ELFacade.disarmDeferredExpressions("a \\\\#{x} b")); + assertEquals("a \\\\\\#{x} b", ELFacade.disarmDeferredExpressions("a \\\\\\#{x} b")); + assertEquals("$0a} $0b}", ELFacade.disarmDeferredExpressions("#{a} #{b}")); + } +} From 485990d72817966444698810c9e1a25469c01235 Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Mon, 28 Sep 2026 14:02:45 +0200 Subject: [PATCH 6/8] Speed up @Email checks and explicit group resolution - Match the default e-mail address pattern with a hand-written matcher equivalent to DEFAULT_EMAIL_PATTERN. - Check the default ".*" regexp of @Email by looking for line terminators instead of running the regular expression. - Cache the groups computed for a single explicitly requested group, so that validating a group does not re-resolve it on every call and the per-strategy descriptor caches are hit by identity. --- .../constraints/AbstractPatternValidator.java | 28 ++++- .../bval/jsr/groups/GroupsComputer.java | 12 +++ .../bval/routines/EMailValidationUtils.java | 100 +++++++++++++++++- .../routines/EMailValidationUtilsTest.java | 53 ++++++++++ 4 files changed, 191 insertions(+), 2 deletions(-) create mode 100644 bval-jsr/src/test/java/org/apache/bval/routines/EMailValidationUtilsTest.java diff --git a/bval-jsr/src/main/java/org/apache/bval/constraints/AbstractPatternValidator.java b/bval-jsr/src/main/java/org/apache/bval/constraints/AbstractPatternValidator.java index 34b66e97b2..6b8fba7489 100644 --- a/bval-jsr/src/main/java/org/apache/bval/constraints/AbstractPatternValidator.java +++ b/bval-jsr/src/main/java/org/apache/bval/constraints/AbstractPatternValidator.java @@ -42,6 +42,12 @@ public interface PatternDescriptor { protected Pattern pattern; + /** + * Whether {@link #pattern} is {@code .*} without flags, the default of {@code @Email}, which matches exactly the + * values without a line terminator. + */ + private boolean matchesAnyLine; + protected AbstractPatternValidator(Function toDescriptor) { super(); this.toDescriptor = Validate.notNull(toDescriptor); @@ -60,6 +66,7 @@ public void initialize(A constraintAnnotation) { } try { pattern = Pattern.compile(pd.regexp(), intFlag); + matchesAnyLine = intFlag == 0 && ".*".equals(pd.regexp()); } catch (PatternSyntaxException e) { throw new IllegalArgumentException("Invalid regular expression.", e); } @@ -67,6 +74,25 @@ public void initialize(A constraintAnnotation) { @Override public boolean isValid(T value, ConstraintValidatorContext context) { - return value == null || pattern.matcher(value).matches(); + if (value == null) { + return true; + } + return matchesAnyLine ? !containsLineTerminator(value) : pattern.matcher(value).matches(); + } + + private static boolean containsLineTerminator(CharSequence value) { + for (int i = 0, n = value.length(); i < n; i++) { + switch (value.charAt(i)) { + case '\n': + case '\r': + case '\u0085': + case '\u2028': + case '\u2029': + return true; + default: + break; + } + } + return false; } } diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/groups/GroupsComputer.java b/bval-jsr/src/main/java/org/apache/bval/jsr/groups/GroupsComputer.java index 5a5e4e6a9d..aca11d7e2c 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/groups/GroupsComputer.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/groups/GroupsComputer.java @@ -61,6 +61,9 @@ public class GroupsComputer { /** caching resolved groups in a thread-safe map. */ private final Map, List> resolvedSequences = new ConcurrentHashMap<>(); + /** Results of {@link #computeGroups(Class...)} for a single group, the common case for explicit groups. */ + private final Map, Groups> singleGroups = new ConcurrentHashMap<>(); + /** * Compute groups from an array of group classes. * @@ -74,6 +77,15 @@ public final Groups computeGroups(Class... groups) { if (groups.length == 0 || (groups.length == 1 && groups[0] == Default.class)) { return DEFAULT_GROUPS; } + if (groups.length == 1 && groups[0] != null) { + final Groups cached = singleGroups.get(groups[0]); + if (cached != null) { + return cached; + } + final Groups result = computeGroups(Arrays.asList(groups)); + singleGroups.putIfAbsent(groups[0], result); + return result; + } return computeGroups(Arrays.asList(groups)); } diff --git a/bval-jsr/src/main/java/org/apache/bval/routines/EMailValidationUtils.java b/bval-jsr/src/main/java/org/apache/bval/routines/EMailValidationUtils.java index 0835baeab5..e654b612ed 100644 --- a/bval-jsr/src/main/java/org/apache/bval/routines/EMailValidationUtils.java +++ b/bval-jsr/src/main/java/org/apache/bval/routines/EMailValidationUtils.java @@ -44,7 +44,105 @@ public class EMailValidationUtils { * @return true if the validation passes */ public static boolean isValid(Object value) { - return isValid(value, DEFAULT_EMAIL_PATTERN); + if (value == null) { + return true; + } + if (!(value instanceof CharSequence)) { + return false; + } + final CharSequence seq = (CharSequence) value; + return seq.length() == 0 || matchesDefaultPattern(seq); + } + + /** + * Equivalent to {@code DEFAULT_EMAIL_PATTERN.matcher(seq).matches()} without the regular expression engine: one + * or more dot-separated runs of atom characters, {@code @}, then either such runs or a bracketed dotted quad. + */ + static boolean matchesDefaultPattern(CharSequence seq) { + final int length = seq.length(); + final int at = matchAtomRuns(seq, 0, length); + if (at < 0 || at >= length || seq.charAt(at) != '@') { + return false; + } + final int domain = at + 1; + return matchAtomRuns(seq, domain, length) == length || matchIpDomain(seq, domain, length); + } + + /** + * Match {@code ATOM+(\.ATOM+)*} starting at {@code start} as far as possible. + * + * @return the index after the match, or {@code -1} if there is none + */ + private static int matchAtomRuns(CharSequence seq, int start, int length) { + int i = start; + while (true) { + final int runStart = i; + while (i < length && isAtom(seq.charAt(i))) { + i++; + } + if (i == runStart) { + // an empty run: fail at the start, otherwise back off the dot that introduced it + return runStart == start ? -1 : runStart - 1; + } + if (i < length && seq.charAt(i) == '.') { + i++; + } else { + return i; + } + } + } + + private static boolean isAtom(char c) { + if (c <= 0x20) { + // control characters, and the space of \s; the other \s characters are control characters + return false; + } + switch (c) { + case '(': + case ')': + case '<': + case '>': + case '@': + case ',': + case ';': + case ':': + case '\\': + case '"': + case '.': + case '[': + case ']': + return false; + default: + return true; + } + } + + /** Match {@code \[d{1,3}\.d{1,3}\.d{1,3}\.d{1,3}\]} spanning exactly {@code [start, length)}. */ + private static boolean matchIpDomain(CharSequence seq, int start, int length) { + if (length - start < 9 || seq.charAt(start) != '[' || seq.charAt(length - 1) != ']') { + return false; + } + int i = start + 1; + for (int part = 0; part < 4; part++) { + if (part > 0) { + if (seq.charAt(i) != '.') { + return false; + } + i++; + } + final int digitsStart = i; + while (i < length - 1 && i - digitsStart < 3 && isDigit(seq.charAt(i))) { + i++; + } + if (i == digitsStart) { + return false; + } + } + return i == length - 1; + } + + private static boolean isDigit(char c) { + return c >= '0' && c <= '9'; } /** diff --git a/bval-jsr/src/test/java/org/apache/bval/routines/EMailValidationUtilsTest.java b/bval-jsr/src/test/java/org/apache/bval/routines/EMailValidationUtilsTest.java new file mode 100644 index 0000000000..b58f0397bd --- /dev/null +++ b/bval-jsr/src/test/java/org/apache/bval/routines/EMailValidationUtilsTest.java @@ -0,0 +1,53 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.bval.routines; + +import static org.junit.Assert.assertEquals; + +import java.util.Random; + +import org.junit.Test; + +public class EMailValidationUtilsTest { + private static final String[] TOKENS = + { "ab", "x", ".", "@", "[", "]", "1", "12", "123", "1234", " ", "é", "\\", "\"", ";", "\n", "\u007f" }; + + @Test + public void matchesLikeDefaultPattern() { + final String[] examples = { "a@b", "a.b@c.d", "a..b@c", "a.@b", ".a@b", "@b", "a@", "a@b.", "a@.b", + "a@[1.2.3.4]", "a@[1.2.3.1234]", "a@[1.2.3]", "a@[1.2.3.4]x", "a@b@c", "ä@ö.de", "a b@c", + "a@b c", "a\"b@c", "a@[123.45.6.789]" }; + for (String example : examples) { + assertMatchesLikePattern(example); + } + final Random random = new Random(42); + for (int i = 0; i < 100_000; i++) { + final StringBuilder candidate = new StringBuilder(); + for (int j = 1 + random.nextInt(12); j > 0; j--) { + candidate.append(TOKENS[random.nextInt(TOKENS.length)]); + } + assertMatchesLikePattern(candidate.toString()); + } + } + + private static void assertMatchesLikePattern(String candidate) { + assertEquals(candidate, EMailValidationUtils.DEFAULT_EMAIL_PATTERN.matcher(candidate).matches(), + EMailValidationUtils.matchesDefaultPattern(candidate)); + } +} From 0db1ace8556fed62275af63c7d6d2015f9671819 Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Mon, 28 Sep 2026 14:02:45 +0200 Subject: [PATCH 7/8] Reduce per-element and per-constraint validation overhead - Cache the runtime container key per container class on container element descriptors instead of resolving type arguments reflectively for every cascaded container. - Find the ancestor of a cascaded container element by comparing paths in place, and derive the element's context lazily from that ancestor. - Precompute container element descriptors as a flat array and drop the synchronized holder from value extraction. - Memoize each constraint's validator on its descriptor and look up cached unwrapping information before computing it. - Do not build violations of constraints composing one reported as a single violation; only whether they failed matters. - Cache executable descriptors per executable, resolve them once per validation, reuse the executable's base path and the bean's Default group redefinition, and create parameter frames without streams. - Look up per-group descriptor caches without allocating a lambda. --- .../org/apache/bval/jsr/descriptor/BeanD.java | 64 ++++++--- .../jsr/descriptor/CascadableContainerD.java | 18 +++ .../bval/jsr/descriptor/ConstraintD.java | 20 +++ .../jsr/descriptor/ContainerElementTypeD.java | 25 ++++ .../apache/bval/jsr/descriptor/ElementD.java | 25 +++- .../bval/jsr/descriptor/GroupStrategyMap.java | 33 +++-- .../job/ConstraintValidatorContextImpl.java | 17 +++ .../bval/jsr/job/ValidateExecutable.java | 27 +++- .../bval/jsr/job/ValidateParameters.java | 67 +++++----- .../bval/jsr/job/ValidateReturnValue.java | 15 +-- .../apache/bval/jsr/job/ValidationJob.java | 126 ++++++++++-------- .../org/apache/bval/jsr/util/PathImpl.java | 25 ++++ .../jsr/valueextraction/ExtractValues.java | 10 +- .../jsr/valueextraction/ValueExtractors.java | 4 + 14 files changed, 343 insertions(+), 133 deletions(-) diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/BeanD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/BeanD.java index 9973f4c024..cfc4196fa4 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/BeanD.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/BeanD.java @@ -18,6 +18,9 @@ */ package org.apache.bval.jsr.descriptor; +import java.lang.reflect.Constructor; +import java.lang.reflect.Executable; +import java.lang.reflect.Method; import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Collections; @@ -26,9 +29,12 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import jakarta.validation.metadata.BeanDescriptor; import jakarta.validation.metadata.ConstructorDescriptor; +import jakarta.validation.metadata.ExecutableDescriptor; import jakarta.validation.metadata.MethodDescriptor; import jakarta.validation.metadata.MethodType; import jakarta.validation.metadata.PropertyDescriptor; @@ -64,6 +70,11 @@ public class BeanD extends ElementD, MetadataReader.ForBean> impl private final GroupStrategyMap localGroupStrategies = new GroupStrategyMap<>(); private final GroupStrategyMap[]> propertiesByGroups = new GroupStrategyMap<>(); + /** Descriptors by executable, {@link #NOT_CONSTRAINED} for unconstrained ones. */ + private final ConcurrentMap executableDescriptors = new ConcurrentHashMap<>(); + + private static final Object NOT_CONSTRAINED = new Object(); + private static final GroupStrategy UNCHANGED = GroupStrategy.simple(Collections.emptySet()); BeanD(MetadataReader.ForBean reader) { @@ -133,6 +144,25 @@ public Set getConstrainedMethods(MethodType methodType, Method }); } + /** + * Like {@link #getConstraintsForMethod(String, Class...)} or {@link #getConstraintsForConstructor(Class...)} for + * {@code executable}, cached by executable. + * + * @param executable + * @return {@link ExecutableDescriptor} or {@code null} if {@code executable} is not constrained + */ + public ExecutableDescriptor getConstraintsForExecutable(Executable executable) { + Object result = executableDescriptors.get(executable); + if (result == null) { + final ExecutableDescriptor descriptor = executable instanceof Method + ? getConstraintsForMethod(executable.getName(), executable.getParameterTypes()) + : getConstraintsForConstructor(((Constructor) executable).getParameterTypes()); + result = descriptor == null ? NOT_CONSTRAINED : descriptor; + executableDescriptors.putIfAbsent(executable, result); + } + return result == NOT_CONSTRAINED ? null : (ExecutableDescriptor) result; + } + @Override public ConstructorDescriptor getConstraintsForConstructor(Class... parameterTypes) { return constructors.get(new Signature(beanClass.getName(), parameterTypes)); @@ -167,12 +197,14 @@ public GroupStrategy getGroupStrategy() { * @param groups * @return {@link GroupStrategy} */ + @Override public GroupStrategy getLocalGroupStrategy(GroupStrategy groups) { - final GroupStrategy result = localGroupStrategies.computeIfAbsent(groups, g -> { + GroupStrategy result = localGroupStrategies.get(groups); + if (result == null) { final GroupStrategy computed = - GroupStrategy.redefining(g, Collections.singletonMap(Group.DEFAULT, groupStrategy)); - return computed == g ? UNCHANGED : computed; - }); + GroupStrategy.redefining(groups, Collections.singletonMap(Group.DEFAULT, groupStrategy)); + result = localGroupStrategies.putIfAbsent(groups, computed == groups ? UNCHANGED : computed); + } return result == UNCHANGED ? groups : result; } @@ -185,18 +217,20 @@ public GroupStrategy getLocalGroupStrategy(GroupStrategy groups) { * @return {@link PropertyD} array, possibly empty */ public PropertyD[] getPropertiesFor(GroupStrategy groups) { - return propertiesByGroups.computeIfAbsent(groups, g -> { - final Set targetGroups = g.getGroups(); - final List> relevant = new ArrayList<>(leafProperties.length); - for (PropertyD p : leafProperties) { - if (p.isCascadedDeep() || !p.getConstrainedContainerElementTypes().isEmpty() - || p.getConstraintDescriptors().stream().map(ConstraintD.class::cast) - .anyMatch(c -> matchesGroups(c, targetGroups))) { - relevant.add(p); - } + final PropertyD[] cached = propertiesByGroups.get(groups); + if (cached != null) { + return cached; + } + final Set targetGroups = groups.getGroups(); + final List> relevant = new ArrayList<>(leafProperties.length); + for (PropertyD p : leafProperties) { + if (p.isCascadedDeep() || !p.getConstrainedContainerElementTypes().isEmpty() + || p.getConstraintDescriptors().stream().map(ConstraintD.class::cast) + .anyMatch(c -> matchesGroups(c, targetGroups))) { + relevant.add(p); } - return relevant.toArray(new PropertyD[0]); - }); + } + return propertiesByGroups.putIfAbsent(groups, relevant.toArray(new PropertyD[0])); } public final Type getGenericType() { diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/CascadableContainerD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/CascadableContainerD.java index 30adbdc0f3..208c76c5e5 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/CascadableContainerD.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/CascadableContainerD.java @@ -19,6 +19,8 @@ package org.apache.bval.jsr.descriptor; import java.lang.reflect.AnnotatedElement; +import java.util.ArrayList; +import java.util.List; import java.util.Set; import jakarta.validation.metadata.CascadableDescriptor; @@ -37,6 +39,7 @@ public abstract class CascadableContainerD

, E extends A private final Set groupConversions; private final Set containerElementTypes; private final boolean cascadedDeep; + private final ContainerElementTypeD[] leafContainerElementTypes; protected CascadableContainerD(MetadataReader.ForContainer reader, P parent) { super(reader, parent); @@ -47,6 +50,21 @@ protected CascadableContainerD(MetadataReader.ForContainer reader, P parent) .filter(DescriptorManager::isConstrained) .collect(ToUnmodifiable.set()); cascadedDeep = cascaded || containerElementTypes.stream().anyMatch(DescriptorManager::isCascaded); + final List leaves = new ArrayList<>(); + for (ContainerElementTypeDescriptor d : containerElementTypes) { + ComposedD.forEachUnwrapped(d, ContainerElementTypeD.class, leaves::add); + } + leafContainerElementTypes = leaves.toArray(new ContainerElementTypeD[0]); + } + + /** + * Get the leaf (non-composed) delegates of {@link #getConstrainedContainerElementTypes()}. The result must not + * be modified. + * + * @return {@link ContainerElementTypeD} array, possibly empty + */ + public ContainerElementTypeD[] getLeafContainerElementTypes() { + return leafContainerElementTypes; } /** diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ConstraintD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ConstraintD.java index 124bafdf11..43c19e8698 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ConstraintD.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ConstraintD.java @@ -82,6 +82,8 @@ private static Set set(Supplier array) { private final List>> constraintValidatorClasses; // racy single-check memoization: the annotation is immutable, so concurrent initializations agree private String messageTemplate; + /** Memo of this constraint's entry in the factory's validator cache. */ + private volatile ConstraintValidator validator; private final Lazy toString = new Lazy<>(() -> String.format("%s: %s", ConstraintD.class.getSimpleName(), getAnnotation())); @@ -171,6 +173,24 @@ public U unwrap(Class type) throws ValidationException { } } + /** + * Get the initialized {@link ConstraintValidator} previously recorded with {@link #setValidator}. + * + * @return {@link ConstraintValidator} or {@code null} + */ + public ConstraintValidator getValidator() { + return validator; + } + + /** + * Record the initialized {@link ConstraintValidator} the factory caches for this constraint. + * + * @param validator + */ + public void setValidator(ConstraintValidator validator) { + this.validator = validator; + } + public Scope getScope() { return scope; } diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ContainerElementTypeD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ContainerElementTypeD.java index ac5337a090..7c56ffb502 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ContainerElementTypeD.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ContainerElementTypeD.java @@ -19,9 +19,12 @@ package org.apache.bval.jsr.descriptor; import java.lang.reflect.AnnotatedType; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import jakarta.validation.metadata.ContainerElementTypeDescriptor; +import org.apache.bval.jsr.GraphContext; import org.apache.bval.jsr.metadata.ContainerElementKey; import org.apache.bval.util.Validate; @@ -29,6 +32,8 @@ public class ContainerElementTypeD extends CascadableContainerD, ContainerElementKey> runtimeKeys = new ConcurrentHashMap<>(); ContainerElementTypeD(ContainerElementKey key, MetadataReader.ForContainer reader, CascadableContainerD parent) { @@ -49,4 +54,24 @@ public Integer getTypeArgumentIndex() { public ContainerElementKey getKey() { return key; } + + /** + * Get {@link #getKey()} in terms of the runtime type of the container held by {@code context}. + * + * @param context + * @return {@link ContainerElementKey} + * @see GraphContext#runtimeKey(ContainerElementKey) + */ + public ContainerElementKey getRuntimeKey(GraphContext context) { + final Object value = context.getValue(); + if (value == null || value.getClass() == key.getContainerClass()) { + return key; + } + ContainerElementKey result = runtimeKeys.get(value.getClass()); + if (result == null) { + result = context.runtimeKey(key); + runtimeKeys.putIfAbsent(value.getClass(), result); + } + return result; + } } diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ElementD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ElementD.java index f9bbe38c6a..176cf77272 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ElementD.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ElementD.java @@ -72,6 +72,11 @@ final protected BeanD getBean() { public final GroupStrategy getGroupStrategy() { return getBean().getGroupStrategy(); } + + @Override + public final GroupStrategy getLocalGroupStrategy(GroupStrategy groups) { + return getBean().getLocalGroupStrategy(groups); + } } private static final ConstraintD[] NO_CONSTRAINTS = {}; @@ -134,10 +139,13 @@ public ConstraintD[] getConstraintsFor(GroupStrategy groups) { if (constraintsByGroups == null) { return NO_CONSTRAINTS; } - return constraintsByGroups.computeIfAbsent(groups, g -> { - final Set targetGroups = g.getGroups(); - return constraints.stream().filter(c -> matchesGroups(c, targetGroups)).toArray(ConstraintD[]::new); - }); + final ConstraintD[] cached = constraintsByGroups.get(groups); + if (cached != null) { + return cached; + } + final Set targetGroups = groups.getGroups(); + return constraintsByGroups.putIfAbsent(groups, + constraints.stream().filter(c -> matchesGroups(c, targetGroups)).toArray(ConstraintD[]::new)); } @Override @@ -161,6 +169,15 @@ public final Class getDeclaringClass() { public abstract GroupStrategy getGroupStrategy(); + /** + * Get {@code groups} with {@link Group#DEFAULT} redefined as {@link #getGroupStrategy()}. Returns + * {@code groups} itself (same instance) when no redefinition applies. + * + * @param groups + * @return {@link GroupStrategy} + */ + public abstract GroupStrategy getLocalGroupStrategy(GroupStrategy groups); + @Override public String toString() { return String.format("%s: %s", getClass().getSimpleName(), meta.describeHost()); diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/GroupStrategyMap.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/GroupStrategyMap.java index 5fe58f6163..2d741609cd 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/GroupStrategyMap.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/GroupStrategyMap.java @@ -21,7 +21,6 @@ import java.util.Arrays; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.function.Function; import org.apache.bval.jsr.groups.GroupStrategy; @@ -40,22 +39,37 @@ final class GroupStrategyMap { /** Alternating key, value pairs; replaced wholesale, so readers always see a consistent array. */ private volatile Object[] byIdentity = NO_ENTRIES; + /** + * @return the value for {@code key}, or {@code null} if none has been stored yet + */ @SuppressWarnings("unchecked") - V computeIfAbsent(GroupStrategy key, Function compute) { + V get(GroupStrategy key) { final Object[] entries = byIdentity; for (int i = 0; i < entries.length; i += 2) { if (entries[i] == key) { return (V) entries[i + 1]; } } - V value = byEquality.get(key); - if (value == null) { - value = compute.apply(key); - final V previous = byEquality.putIfAbsent(key, value); - if (previous != null) { - value = previous; - } + final V value = byEquality.get(key); + if (value != null) { + rememberIdentity(entries, key, value); } + return value; + } + + /** + * Store {@code value} for {@code key} unless a value is already present. + * + * @return the value now stored for {@code key} + */ + V putIfAbsent(GroupStrategy key, V value) { + final V previous = byEquality.putIfAbsent(key, value); + final V result = previous == null ? value : previous; + rememberIdentity(byIdentity, key, result); + return result; + } + + private void rememberIdentity(Object[] entries, GroupStrategy key, V value) { if (entries.length < 2 * MAX_IDENTITY_ENTRIES) { // racing updates may drop an entry, which only costs a later equality lookup final Object[] grown = Arrays.copyOf(entries, entries.length + 2); @@ -63,6 +77,5 @@ V computeIfAbsent(GroupStrategy key, Function constraint; // typically holds a single violation; a list with a containment check keeps set semantics cheaply private List> violations; + /** Whether violations are only counted, not built, because the caller discards them. */ + private final boolean discardViolations; private boolean defaultConstraintViolationDisabled; ConstraintValidatorContextImpl(ValidationJob.Frame frame, ConstraintD constraint) { + this(frame, constraint, false); + } + + ConstraintValidatorContextImpl(ValidationJob.Frame frame, ConstraintD constraint, + boolean discardViolations) { super(); this.frame = Validate.notNull(frame, "frame"); this.constraint = Validate.notNull(constraint, "constraint"); + this.discardViolations = discardViolations; } @Override @@ -183,6 +192,10 @@ List> getRequiredViolations() { if (defaultConstraintViolationDisabled) { Exceptions.raise(ValidationException::new, "Expected custom constraint violation(s)"); } + if (discardViolations) { + violations = Collections.emptyList(); + return violations; + } addError(getDefaultConstraintMessageTemplate(), frame.context.getPath()); } return violations; @@ -200,6 +213,10 @@ public Object getValidatedValue() { @SuppressWarnings({ "unchecked", "rawtypes" }) private void addError(String messageTemplate, PathImpl propertyPath) { + if (discardViolations) { + violations = Collections.emptyList(); + return; + } if (violations == null) { violations = new ArrayList<>(2); } diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateExecutable.java b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateExecutable.java index b222f3ebeb..6b4ca5b998 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateExecutable.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateExecutable.java @@ -25,6 +25,7 @@ import jakarta.validation.Path; import jakarta.validation.Path.Node; +import jakarta.validation.metadata.ExecutableDescriptor; import org.apache.bval.jsr.ApacheFactoryContext; import org.apache.bval.jsr.metadata.Meta; @@ -44,6 +45,9 @@ public abstract class ValidateExecutable extends Valida protected final E executable; private final Function executableNode; + private PathImpl basePath; + private ExecutableDescriptor executableDescriptor; + private boolean described; @SuppressWarnings("unchecked") public ValidateExecutable(ApacheFactoryContext validatorContext, Class[] groups, Meta meta) { @@ -64,8 +68,25 @@ public ValidateExecutable(ApacheFactoryContext validatorContext, Class[] grou } protected PathImpl createBasePath() { - final PathImpl path = PathImpl.create(); - path.addNode(executableNode.apply(executable)); - return path; + if (basePath == null) { + basePath = PathImpl.create(); + basePath.addNode(executableNode.apply(executable)); + } + return PathImpl.copy(basePath); } + + /** + * Get the descriptor of {@link #executable}, computed once per job. + * + * @return {@link ExecutableDescriptor} or {@code null} if the executable is not constrained + */ + protected final ExecutableDescriptor getExecutableDescriptor() { + if (!described) { + executableDescriptor = describe(); + described = true; + } + return executableDescriptor; + } + + protected abstract ExecutableDescriptor describe(); } \ No newline at end of file diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateParameters.java b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateParameters.java index 2f67de4171..517ae37001 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateParameters.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateParameters.java @@ -22,31 +22,27 @@ import java.lang.reflect.Executable; import java.lang.reflect.Method; import java.lang.reflect.Type; -import java.util.Collections; import java.util.List; -import java.util.Set; import java.util.function.Consumer; -import java.util.stream.Collectors; -import java.util.stream.IntStream; import jakarta.validation.ConstraintViolation; import jakarta.validation.ParameterNameProvider; import jakarta.validation.constraintvalidation.ValidationTarget; import jakarta.validation.metadata.ExecutableDescriptor; +import jakarta.validation.metadata.ParameterDescriptor; import org.apache.bval.jsr.ApacheFactoryContext; import org.apache.bval.jsr.ConstraintViolationImpl; import org.apache.bval.jsr.GraphContext; +import org.apache.bval.jsr.descriptor.BeanD; import org.apache.bval.jsr.descriptor.ConstraintD; import org.apache.bval.jsr.descriptor.CrossParameterD; import org.apache.bval.jsr.descriptor.ParameterD; -import org.apache.bval.jsr.groups.Group; import org.apache.bval.jsr.groups.GroupStrategy; import org.apache.bval.jsr.metadata.Meta; import org.apache.bval.jsr.util.NodeImpl; import org.apache.bval.jsr.util.PathImpl; import org.apache.bval.util.Exceptions; -import org.apache.bval.util.Lazy; import org.apache.bval.util.Validate; import org.apache.bval.util.reflection.TypeUtils; @@ -63,8 +59,8 @@ public static class ForMethod extends ValidateParameters { @Override protected ExecutableDescriptor describe() { - return validatorContext.getDescriptorManager().getBeanDescriptor(object.getClass()) - .getConstraintsForMethod(executable.getName(), executable.getParameterTypes()); + return ((BeanD) validatorContext.getDescriptorManager().getBeanDescriptor(object.getClass())) + .getConstraintsForExecutable(executable); } @SuppressWarnings("unchecked") @@ -94,8 +90,8 @@ public static class ForConstructor extends ValidateParameters) validatorContext.getDescriptorManager().getBeanDescriptor(executable.getDeclaringClass())) + .getConstraintsForExecutable(executable); } @SuppressWarnings("unchecked") @@ -132,33 +128,34 @@ protected ValidationTarget getValidationTarget() { @Override void process(GroupStrategy groups, Consumer> sink) { Validate.notNull(sink, "sink"); - final Lazy>> parameterFrames = new Lazy<>(this::parameterFrames); - - GroupStrategy.redefining(groups, Collections.singletonMap(Group.DEFAULT, descriptor.getGroupStrategy())) - .applyTo(noViolations(gs -> { - validateDescriptorConstraints(gs, sink); - parameterFrames.get().forEach(p -> p.validateDescriptorConstraints(gs, sink)); - })); - parameterFrames.get().forEach(p -> p.recurse(groups, sink)); + final List parameters = executableDescriptor.getParameterDescriptors(); + @SuppressWarnings("unchecked") + final Frame[] parameterFrames = new Frame[parameters.size()]; + for (int i = 0; i < parameterFrames.length; i++) { + final ParameterD pd = (ParameterD) parameters.get(i); + parameterFrames[i] = new SproutFrame>(this, pd, parameter(pd.getIndex())); + } + descriptor.getLocalGroupStrategy(groups).applyTo(noViolations(gs -> { + validateDescriptorConstraints(gs, sink); + for (final Frame p : parameterFrames) { + p.validateDescriptorConstraints(gs, sink); + } + })); + for (final Frame p : parameterFrames) { + p.recurse(groups, sink); + } } @Override Object getBean() { return object; } - - private Set> parameterFrames() { - return executableDescriptor.getParameterDescriptors().stream() - .map(pd -> new SproutFrame>(this, (ParameterD) pd, parameter(pd.getIndex()))) - .collect(Collectors.toSet()); - } } private static final String PARAMETERS_DO_NOT_MATCH = "Parameters do not match"; protected final T object; - protected final Lazy> parameterNames = - new Lazy<>(() -> getParameterNames(validatorContext.getParameterNameProvider())); + private List parameterNames; private final Object[] parameterValues; @@ -172,25 +169,24 @@ private Set> parameterFrames() { final Type[] genericParameterTypes = executable.getGenericParameterTypes(); Exceptions.raiseUnless(parameterValues.length == genericParameterTypes.length, IllegalArgumentException::new, PARAMETERS_DO_NOT_MATCH); - IntStream.range(0, genericParameterTypes.length) - .forEach(n -> Exceptions.raiseUnless(TypeUtils.isInstance(parameterValues[n], genericParameterTypes[n]), - IllegalArgumentException::new, PARAMETERS_DO_NOT_MATCH)); + for (int n = 0; n < genericParameterTypes.length; n++) { + Exceptions.raiseUnless(TypeUtils.isInstance(parameterValues[n], genericParameterTypes[n]), + IllegalArgumentException::new, PARAMETERS_DO_NOT_MATCH); + } } @Override protected Frame computeBaseFrame() { - return new ParametersFrame(describe(), new GraphContext(validatorContext, + return new ParametersFrame(getExecutableDescriptor(), new GraphContext(validatorContext, createBasePath().addNode(new NodeImpl.CrossParameterNodeImpl()), parameterValues)); } @Override protected boolean hasWork() { - final ExecutableDescriptor descriptor = describe(); + final ExecutableDescriptor descriptor = getExecutableDescriptor(); return descriptor != null && descriptor.hasConstrainedParameters(); } - protected abstract ExecutableDescriptor describe(); - protected abstract List getParameterNames(ParameterNameProvider parameterNameProvider); protected abstract T getRootBean(); @@ -205,7 +201,10 @@ ConstraintViolationImpl createViolation(String messageTemplate, String messag private GraphContext parameter(int i) { final PathImpl path = createBasePath(); - path.addNode(new NodeImpl.ParameterNodeImpl(parameterNames.get().get(i), i)); + if (parameterNames == null) { + parameterNames = getParameterNames(validatorContext.getParameterNameProvider()); + } + path.addNode(new NodeImpl.ParameterNodeImpl(parameterNames.get(i), i)); return new GraphContext(validatorContext, path, parameterValues[i]); } } diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateReturnValue.java b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateReturnValue.java index e22431ae3a..a2a6ba8dca 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateReturnValue.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidateReturnValue.java @@ -26,6 +26,7 @@ import org.apache.bval.jsr.ApacheFactoryContext; import org.apache.bval.jsr.ConstraintViolationImpl; import org.apache.bval.jsr.GraphContext; +import org.apache.bval.jsr.descriptor.BeanD; import org.apache.bval.jsr.descriptor.ConstraintD; import org.apache.bval.jsr.descriptor.ReturnValueD; import org.apache.bval.jsr.metadata.Meta; @@ -60,8 +61,8 @@ protected Class getRootBeanClass() { @Override protected ExecutableDescriptor describe() { - return validatorContext.getDescriptorManager().getBeanDescriptor(object.getClass()) - .getConstraintsForMethod(executable.getName(), executable.getParameterTypes()); + return ((BeanD) validatorContext.getDescriptorManager().getBeanDescriptor(object.getClass())) + .getConstraintsForExecutable(executable); } @Override @@ -97,8 +98,8 @@ protected Class getRootBeanClass() { @Override protected ExecutableDescriptor describe() { - return validatorContext.getDescriptorManager().getBeanDescriptor(executable.getDeclaringClass()) - .getConstraintsForConstructor(executable.getParameterTypes()); + return ((BeanD) validatorContext.getDescriptorManager().getBeanDescriptor(executable.getDeclaringClass())) + .getConstraintsForExecutable(executable); } @Override @@ -127,7 +128,7 @@ Object getBean() { @Override protected Frame computeBaseFrame() { - return createBaseFrame((ReturnValueD) describe().getReturnValueDescriptor(), new GraphContext( + return createBaseFrame((ReturnValueD) getExecutableDescriptor().getReturnValueDescriptor(), new GraphContext( validatorContext, createBasePath().addNode(new NodeImpl.ReturnValueNodeImpl()), returnValue)); } @@ -141,12 +142,10 @@ ConstraintViolationImpl createViolation(String messageTemplate, String messag @Override protected boolean hasWork() { - final ExecutableDescriptor descriptor = describe(); + final ExecutableDescriptor descriptor = getExecutableDescriptor(); return descriptor != null && descriptor.hasConstrainedReturnValue(); } - protected abstract ExecutableDescriptor describe(); - protected abstract T getRootBean(); protected abstract Frame createBaseFrame(ReturnValueD descriptor, GraphContext context); diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java index a418eda9c5..1b038dbf61 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java @@ -21,6 +21,7 @@ import java.lang.reflect.Array; import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashSet; @@ -30,7 +31,6 @@ import java.util.Set; import java.util.function.Consumer; import java.util.function.Predicate; -import java.util.stream.Collectors; import jakarta.validation.ConstraintValidator; import jakarta.validation.ConstraintViolation; @@ -42,6 +42,7 @@ import jakarta.validation.ValidationException; import jakarta.validation.constraintvalidation.ValidationTarget; import jakarta.validation.metadata.CascadableDescriptor; +import jakarta.validation.metadata.ConstraintDescriptor; import jakarta.validation.metadata.ContainerDescriptor; import jakarta.validation.metadata.ContainerElementTypeDescriptor; import jakarta.validation.metadata.GroupConversionDescriptor; @@ -53,6 +54,7 @@ import org.apache.bval.jsr.ConstraintViolationImpl; import org.apache.bval.jsr.GraphContext; import org.apache.bval.jsr.descriptor.BeanD; +import org.apache.bval.jsr.descriptor.CascadableContainerD; import org.apache.bval.jsr.descriptor.ComposedD; import org.apache.bval.jsr.descriptor.ConstraintD; import org.apache.bval.jsr.descriptor.ContainerElementTypeD; @@ -100,8 +102,7 @@ final ValidationJob getJob() { void process(GroupStrategy groups, Consumer> sink) { Validate.notNull(sink, "sink"); - GroupStrategy.redefining(groups, Collections.singletonMap(Group.DEFAULT, descriptor.getGroupStrategy())) - .applyTo(noViolations(gs -> validateDescriptorConstraints(gs, sink))); + descriptor.getLocalGroupStrategy(groups).applyTo(noViolations(gs -> validateDescriptorConstraints(gs, sink))); recurse(groups, sink); } @@ -148,7 +149,7 @@ private boolean validate(ConstraintD constraint, Consumer constraintValidatorContext = - new ConstraintValidatorContextImpl<>(this, constraint); + new ConstraintValidatorContextImpl<>(this, constraint, sink == DISCARD); final boolean valid; if (constraintValidator == null) { @@ -183,26 +184,33 @@ private boolean validateComposed(ConstraintD constraint, Consumer> effectiveSink = constraint.isReportAsSingleViolation() ? cv -> { - } : sink; - - // collect validation results to set of Boolean, ensuring all are evaluated: - final Set validationResults = constraint.getComposingConstraints().stream().map(ConstraintD.class::cast) - .map(c -> validate(c, effectiveSink)).collect(Collectors.toSet()); - - return Collections.singleton(Boolean.TRUE).equals(validationResults); + @SuppressWarnings("unchecked") + final Consumer> effectiveSink = + constraint.isReportAsSingleViolation() ? (Consumer>) DISCARD : sink; + + // evaluate all composing constraints, even after one has failed: + boolean allValid = true; + for (final ConstraintDescriptor c : constraint.getComposingConstraints()) { + allValid &= validate((ConstraintD) c, effectiveSink); + } + return allValid; } @SuppressWarnings({ "rawtypes" }) private ConstraintValidator getConstraintValidator(ConstraintD constraint) { // Fast path: the validator is cached after first use, so avoid building the (capturing) supplier // lambda and going through computeIfAbsent on the common cache-hit path. + final ConstraintValidator memo = constraint.getValidator(); + if (memo != null) { + return memo; + } final ConstraintValidator existing = validatorContext.getConstraintsCache().getValidators().get(constraint); if (existing != null) { + constraint.setValidator(existing); return existing; } - return validatorContext.getOrComputeConstraintValidator(constraint, () -> { + final ConstraintValidator computed = validatorContext.getOrComputeConstraintValidator(constraint, () -> { final Class constraintValidatorClass = new ComputeConstraintValidatorClass<>(validatorContext.getConstraintsCache(), constraint, getValidationTarget(), computeValidatedType(constraint)).get(); @@ -228,6 +236,8 @@ private ConstraintValidator getConstraintValidator(ConstraintD constraint) { } return constraintValidator; }); + constraint.setValidator(computed); + return computed; } private Class computeValidatedType(ConstraintD constraint) { @@ -373,21 +383,17 @@ public SproutFrame(Frame parent, D descriptor, GraphContext context) { void validateDescriptorConstraints(GroupStrategy groups, Consumer> sink) { super.validateDescriptorConstraints(groups, sink); if (context.getValue() != null) { - for (final ContainerElementTypeDescriptor ctd : descriptor.getConstrainedContainerElementTypes()) { - ComposedD.forEachUnwrapped(ctd, ContainerElementTypeD.class, d -> { - if (d.getConstraintsFor(groups).length == 0 - && d.getConstrainedContainerElementTypes().isEmpty()) { - return; - } - final ValueExtractor declaredTypeValueExtractor = - context.getValidatorContext().getValueExtractors().find(d.getKey()); - for (final GraphContext e : ExtractValues.extract(context, d.getKey(), declaredTypeValueExtractor)) { - if (!e.isRecursive()) { - new ContainerElementConstraintsFrame(this, d, e) - .validateDescriptorConstraints(groups, sink); - } + for (final ContainerElementTypeD d : containerElementTypes(descriptor)) { + if (d.getConstraintsFor(groups).length == 0 && d.getConstrainedContainerElementTypes().isEmpty()) { + continue; + } + final ValueExtractor declaredTypeValueExtractor = + context.getValidatorContext().getValueExtractors().find(d.getKey()); + for (final GraphContext e : ExtractValues.extract(context, d.getKey(), declaredTypeValueExtractor)) { + if (!e.isRecursive()) { + new ContainerElementConstraintsFrame(this, d, e).validateDescriptorConstraints(groups, sink); } - }); + } } } } @@ -412,19 +418,17 @@ void recurse(GroupStrategy groups, Consumer> sink) { } private void cascade(GroupStrategy groups, Consumer> sink) { - for (final ContainerElementTypeDescriptor ctd : descriptor.getConstrainedContainerElementTypes()) { - ComposedD.forEachUnwrapped(ctd, ContainerElementTypeD.class, d -> { - if (!d.isCascaded() && d.getConstrainedContainerElementTypes().isEmpty()) { - return; - } - final ValueExtractor runtimeTypeValueExtractor = - context.getValidatorContext().getValueExtractors().find(context.runtimeKey(d.getKey())); - for (final GraphContext e : ExtractValues.extract(context, d.getKey(), runtimeTypeValueExtractor)) { - if (!e.isRecursive()) { - new ContainerElementCascadeFrame(this, d, e).recurse(groups, sink); - } + for (final ContainerElementTypeD d : containerElementTypes(descriptor)) { + if (!d.isCascaded() && d.getConstrainedContainerElementTypes().isEmpty()) { + continue; + } + final ValueExtractor runtimeTypeValueExtractor = + context.getValidatorContext().getValueExtractors().find(d.getRuntimeKey(context)); + for (final GraphContext e : ExtractValues.extract(context, d.getKey(), runtimeTypeValueExtractor)) { + if (!e.isRecursive()) { + new ContainerElementCascadeFrame(this, d, e).recurse(groups, sink); } - }); + } } if (!descriptor.isCascaded()) { return; @@ -552,32 +556,26 @@ void validateDescriptorConstraints(GroupStrategy groups, Consumer DISCARD = cv -> { + }; + + private static ContainerElementTypeD[] containerElementTypes(ContainerDescriptor descriptor) { + if (descriptor instanceof CascadableContainerD) { + return ((CascadableContainerD) descriptor).getLeafContainerElementTypes(); + } + final List result = new ArrayList<>(); + for (final ContainerElementTypeDescriptor d : descriptor.getConstrainedContainerElementTypes()) { + ComposedD.forEachUnwrapped(d, ContainerElementTypeD.class, result::add); + } + return result.toArray(new ContainerElementTypeD[0]); + } + protected static final TypeVariable MAP_VALUE = Map.class.getTypeParameters()[1]; protected static final TypeVariable ITERABLE_ELEMENT = Iterable.class.getTypeParameters()[0]; diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java b/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java index 5e87ac85e2..dce0546143 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/util/PathImpl.java @@ -316,6 +316,31 @@ public NodeImpl removeLeafNode() { } } + /** + * Learn whether {@code other} equals this path with its leaf node removed as by {@link #removeLeafNode()}, + * without copying either path. + * + * @param other + * @return {@code boolean} + */ + public boolean isParentPath(PathImpl other) { + final int parentLength = nodeList.size() - 1; + if (parentLength < 1) { + final PathImpl parent = copy(this); + parent.removeLeafNode(); + return parent.equals(other); + } + if (other.nodeList.size() != parentLength) { + return false; + } + for (int i = 0; i < parentLength; i++) { + if (!nodeList.get(i).equals(other.nodeList.get(i))) { + return false; + } + } + return true; + } + /** * {@inheritDoc} */ diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/valueextraction/ExtractValues.java b/bval-jsr/src/main/java/org/apache/bval/jsr/valueextraction/ExtractValues.java index 137e68f8bc..fb077032bf 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/valueextraction/ExtractValues.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/valueextraction/ExtractValues.java @@ -29,7 +29,6 @@ import org.apache.bval.jsr.metadata.ContainerElementKey; import org.apache.bval.jsr.util.NodeImpl; import org.apache.bval.util.Exceptions; -import org.apache.bval.util.Lazy; import org.apache.bval.util.Validate; /** @@ -40,7 +39,7 @@ public final class ExtractValues { private static class Receiver implements ValueExtractor.ValueReceiver { private final GraphContext context; private final ContainerElementKey containerElementKey; - private final Lazy> result = new Lazy<>(ArrayList::new); + private List result; Receiver(GraphContext context, ContainerElementKey containerElementKey) { super(); @@ -77,7 +76,10 @@ public void keyedValue(String nodeName, Object key, Object object) { private void addChild(NodeImpl node, Object value) { // child(NodeImpl, ...) copies the path exactly once; building the path here and passing it to // child(Path, ...) would copy it a second time. - result.get().add(context.child( + if (result == null) { + result = new ArrayList<>(); + } + result.add(context.child( node.inContainer(containerElementKey.getContainerClass(), containerElementKey.getTypeArgumentIndex()), value)); } @@ -102,7 +104,7 @@ public static List extract(GraphContext context, ContainerElementK } catch (Exception e) { throw new ValidationException(e); } - return receiver.result.optional().orElse(Collections.emptyList()); + return receiver.result == null ? Collections. emptyList() : receiver.result; } return Collections.singletonList(context); } diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/valueextraction/ValueExtractors.java b/bval-jsr/src/main/java/org/apache/bval/jsr/valueextraction/ValueExtractors.java index d6522e4893..fe3a54b786 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/valueextraction/ValueExtractors.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/valueextraction/ValueExtractors.java @@ -321,6 +321,10 @@ public Optional findUnwrappingInfo(Class containerClass, valueUnwrapping == ValidateUnwrappedValue.UNWRAP ? unwrapForcedCache : unwrapDefaultCache; // computeIfAbsent does not cache a thrown exception, so the UNWRAP "not found" error below is // re-evaluated on each call for that (rare, misconfigured) case rather than being memoized. + final Optional cached = cache.get(containerClass); + if (cached != null) { + return cached; + } return cache.computeIfAbsent(containerClass, k -> computeUnwrappingInfo(k, valueUnwrapping)); } From 95caead05c579c0ed9b0154ab1ca974f426bb568 Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Mon, 28 Sep 2026 15:13:24 +0200 Subject: [PATCH 8/8] Trim cascading and executable validation overhead further - Resolve a bean's local group strategy and relevant properties with a single per-group plan lookup. - Validate a single-group strategy directly instead of through GroupStrategy#applyTo callbacks, and cascade into container elements without capturing lambdas or a property frame holder object. - Find the ancestor of a cascaded container element without materializing the element's path when it is its parent's plus one node. - Cache default parameter names per executable, and map wrapper types to primitives with a lookup rather than a scan. --- .../org/apache/bval/jsr/GraphContext.java | 24 +++ .../org/apache/bval/jsr/descriptor/BeanD.java | 45 +++++ .../apache/bval/jsr/job/ValidationJob.java | 175 +++++++++--------- .../DefaultParameterNameProvider.java | 15 +- .../bval/util/reflection/Reflection.java | 11 +- 5 files changed, 178 insertions(+), 92 deletions(-) diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/GraphContext.java b/bval-jsr/src/main/java/org/apache/bval/jsr/GraphContext.java index ae4e0ad1cb..0ed7756ba6 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/GraphContext.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/GraphContext.java @@ -99,6 +99,30 @@ private PathImpl derivePath() { return p; } + /** + * Get the leaf node of this context's path without materializing the path when it is only pending. + * + * @return {@link NodeImpl} + */ + public NodeImpl getLeafNode() { + return path == null && pendingNode != null ? pendingNode : pathReference().getLeafNode(); + } + + /** + * Learn whether {@code other}'s path equals this context's path with its leaf node removed, as by + * {@link PathImpl#removeLeafNode()}. + * + * @param other + * @return {@code boolean} + */ + public boolean hasParentPath(GraphContext other) { + if (path == null && pendingNode != null && !parent.pathReference().isRootPath()) { + // our path is the parent's with pendingNode appended, so removing the leaf yields the parent's path + return other == parent || other.pathReference().equals(parent.pathReference()); + } + return pathReference().isParentPath(other.pathReference()); + } + public Object getValue() { return value; } diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/BeanD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/BeanD.java index cfc4196fa4..a6c7e90d88 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/BeanD.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/BeanD.java @@ -69,6 +69,7 @@ public class BeanD extends ElementD, MetadataReader.ForBean> impl */ private final GroupStrategyMap localGroupStrategies = new GroupStrategyMap<>(); private final GroupStrategyMap[]> propertiesByGroups = new GroupStrategyMap<>(); + private final GroupStrategyMap plans = new GroupStrategyMap<>(); /** Descriptors by executable, {@link #NOT_CONSTRAINED} for unconstrained ones. */ private final ConcurrentMap executableDescriptors = new ConcurrentHashMap<>(); @@ -208,6 +209,50 @@ public GroupStrategy getLocalGroupStrategy(GroupStrategy groups) { return result == UNCHANGED ? groups : result; } + /** + * What validating this bean for a requested group strategy involves. + */ + public static final class GroupPlan { + private final GroupStrategy localGroupStrategy; + private final PropertyD[] properties; + + GroupPlan(GroupStrategy localGroupStrategy, PropertyD[] properties) { + this.localGroupStrategy = localGroupStrategy; + this.properties = properties; + } + + /** + * @param groups the requested strategy this plan was obtained for + * @return {@link BeanD#getLocalGroupStrategy(GroupStrategy)} of {@code groups} + */ + public GroupStrategy getLocalGroupStrategy(GroupStrategy groups) { + return localGroupStrategy == null ? groups : localGroupStrategy; + } + + /** + * @return {@link BeanD#getPropertiesFor(GroupStrategy)} of the local group strategy; must not be modified + */ + public PropertyD[] getProperties() { + return properties; + } + } + + /** + * Get the {@link GroupPlan} for validating this bean for {@code groups}, combining + * {@link #getLocalGroupStrategy(GroupStrategy)} and {@link #getPropertiesFor(GroupStrategy)} in one lookup. + * + * @param groups + * @return {@link GroupPlan} + */ + public GroupPlan getPlan(GroupStrategy groups) { + final GroupPlan cached = plans.get(groups); + if (cached != null) { + return cached; + } + final GroupStrategy local = getLocalGroupStrategy(groups); + return plans.putIfAbsent(groups, new GroupPlan(local == groups ? null : local, getPropertiesFor(local))); + } + /** * Get the leaf constrained properties that need a validation frame for {@code groups}: those with constraints * applying to any of its groups, with constrained container element types, or that are cascaded. The result is diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java index 1b038dbf61..14ad9140b6 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java @@ -273,35 +273,24 @@ public class BeanFrame extends Frame> { @Override void process(GroupStrategy groups, Consumer> sink) { Validate.notNull(sink, "sink"); - final GroupStrategy localGroupStrategy = descriptor.getLocalGroupStrategy(groups); + final BeanD.GroupPlan plan = descriptor.getPlan(groups); + final GroupStrategy localGroupStrategy = plan.getLocalGroupStrategy(groups); final boolean redefined = localGroupStrategy != groups; - final PropertyFrames propertyFrames = new PropertyFrames(descriptor.getPropertiesFor(localGroupStrategy)); - final PropertyD[] properties = propertyFrames.properties; - - localGroupStrategy.applyTo(gs -> { - final int originalCount = violationCount(); - validateDescriptorConstraints(gs, sink); - for (int i = 0; i < properties.length; i++) { - final PropertyD d = properties[i]; - final boolean recurse = !redefined && d.isCascadedDeep(); - if (!recurse && d.getConstraintsFor(gs).length == 0 - && d.getConstrainedContainerElementTypes().isEmpty()) { - continue; - } - final Frame p = propertyFrames.get(i); - if (p != null) { - p.validateDescriptorConstraints(gs, sink); - if (recurse) { - p.recurse(gs, sink); - } - } - } - return violationCount() == originalCount; - }); + final PropertyD[] properties = plan.getProperties(); + // frames are created on first use, so that properties relevant only to groups that are never validated + // (e.g. later steps of a failing group sequence) are not read + final Object[] propertyFrames = properties.length == 0 ? NO_FRAMES : new Object[properties.length]; + + if (localGroupStrategy instanceof Group) { + // a single group: applyTo would just call back once + validateGroup(localGroupStrategy, properties, propertyFrames, redefined, sink); + } else { + localGroupStrategy.applyTo(gs -> validateGroup(gs, properties, propertyFrames, redefined, sink)); + } if (redefined) { for (int i = 0; i < properties.length; i++) { if (properties[i].isCascadedDeep()) { - final Frame p = propertyFrames.get(i); + final Frame p = propertyFrame(properties, propertyFrames, i); if (p != null) { p.recurse(groups, sink); } @@ -310,6 +299,33 @@ void process(GroupStrategy groups, Consumer> sink) { } } + /** + * Validate this bean and its properties for one group of the local group strategy. + * + * @return whether no violation was found + */ + private boolean validateGroup(GroupStrategy gs, PropertyD[] properties, Object[] propertyFrames, + boolean redefined, Consumer> sink) { + final int originalCount = violationCount(); + validateDescriptorConstraints(gs, sink); + for (int i = 0; i < properties.length; i++) { + final PropertyD d = properties[i]; + final boolean recurse = !redefined && d.isCascadedDeep(); + if (!recurse && d.getConstraintsFor(gs).length == 0 + && d.getConstrainedContainerElementTypes().isEmpty()) { + continue; + } + final Frame p = propertyFrame(properties, propertyFrames, i); + if (p != null) { + p.validateDescriptorConstraints(gs, sink); + if (recurse) { + p.recurse(gs, sink); + } + } + } + return violationCount() == originalCount; + } + protected Frame propertyFrame(PropertyD d, GraphContext context) { return new SproutFrame<>(this, d, context); } @@ -320,52 +336,36 @@ Object getBean() { } /** - * Frames of the properties of this bean, created on first use so that properties relevant only to groups - * that are never validated (e.g. later steps of a failing group sequence) are not read. + * @return the frame for {@code properties[i]}, created on first use, or {@code null} if that property is not + * reachable or its value is already being validated further up the graph */ - private class PropertyFrames { - final PropertyD[] properties; - private Object[] frames; - - PropertyFrames(PropertyD[] properties) { - this.properties = properties; - } - - /** - * @return the frame for {@code properties[i]}, or {@code null} if that property is not reachable or its - * value is already being validated further up the graph - */ - Frame get(int i) { - if (frames == null) { - frames = new Object[properties.length]; - } - Object frame = frames[i]; - if (frame == null) { - frame = create(properties[i]); - frames[i] = frame; - } - return frame == NO_FRAME ? null : (Frame) frame; + private Frame propertyFrame(PropertyD[] properties, Object[] frames, int i) { + Object frame = frames[i]; + if (frame == null) { + frame = createPropertyFrame(properties[i]); + frames[i] = frame; } + return frame == NO_FRAME ? null : (Frame) frame; + } - private Object create(PropertyD d) { - final TraversableResolver traversableResolver = validatorContext.getTraversableResolver(); - if (!traversesAll(traversableResolver)) { - final PathImpl p = realContext.getPath(); - p.addProperty(d.getPropertyName()); - try { - if (!traversableResolver.isReachable(context.getValue(), p.removeLeafNode(), getRootBeanClass(), - p, d.getElementType())) { - return NO_FRAME; - } - } catch (ValidationException ve) { - throw ve; - } catch (Exception e) { - throw new ValidationException(e); + private Object createPropertyFrame(PropertyD d) { + final TraversableResolver traversableResolver = validatorContext.getTraversableResolver(); + if (!traversesAll(traversableResolver)) { + final PathImpl p = realContext.getPath(); + p.addProperty(d.getPropertyName()); + try { + if (!traversableResolver.isReachable(context.getValue(), p.removeLeafNode(), getRootBeanClass(), + p, d.getElementType())) { + return NO_FRAME; } + } catch (ValidationException ve) { + throw ve; + } catch (Exception e) { + throw new ValidationException(e); } - final GraphContext child = d.readChild(realContext); - return child == null || child.isRecursive() ? NO_FRAME : propertyFrame(d, child); } + final GraphContext child = d.readChild(realContext); + return child == null || child.isRecursive() ? NO_FRAME : propertyFrame(d, child); } } @@ -405,7 +405,12 @@ void recurse(GroupStrategy groups, Consumer> sink) { } final Set groupConversions = descriptor.getGroupConversions(); if (groupConversions.isEmpty()) { - groups.applyTo(noViolations(gs -> cascade(gs, sink))); + if (groups instanceof Group) { + // a single group: applyTo would just call back once, and the result is not needed + cascade(groups, sink); + } else { + groups.applyTo(noViolations(gs -> cascade(gs, sink))); + } return; } final Map conversions = new HashMap<>(); @@ -453,18 +458,20 @@ private void cascade(GroupStrategy groups, Consumer> sink throw new ValidationException(e); } } - multiplexEach(cx -> { - if (cx.getValue() != null && !cx.isRecursive()) { - new BeanFrame<>(this, cx).process(groups, sink); - } - }); + multiplexEach(groups, sink); } protected GraphContext getMultiplexContext() { return context; } - private void multiplexEach(Consumer consumer) { + private void cascadeInto(GraphContext cx, GroupStrategy groups, Consumer> sink) { + if (cx.getValue() != null && !cx.isRecursive()) { + new BeanFrame<>(this, cx).process(groups, sink); + } + } + + private void multiplexEach(GroupStrategy groups, Consumer> sink) { final GraphContext multiplexContext = getMultiplexContext(); final Object value = multiplexContext.getValue(); if (value == null) { @@ -474,37 +481,37 @@ private void multiplexEach(Consumer consumer) { // inconsistent: use Object[] here but specific type for Iterable? RI compatibility final Class arrayType = value instanceof Object[] ? Object[].class : value.getClass(); for (int i = 0, n = Array.getLength(value); i < n; i++) { - consumer.accept( - multiplexContext.child(NodeImpl.atIndex(i).inContainer(arrayType, null), Array.get(value, i))); + cascadeInto(multiplexContext.child(NodeImpl.atIndex(i).inContainer(arrayType, null), + Array.get(value, i)), groups, sink); } return; } if (Map.class.isInstance(value)) { for (final Map.Entry e : ((Map) value).entrySet()) { - consumer.accept(multiplexContext.child( + cascadeInto(multiplexContext.child( setContainerInformation(NodeImpl.atKey(e.getKey()), MAP_VALUE, descriptor.getElementClass()), - e.getValue())); + e.getValue()), groups, sink); } return; } if (List.class.isInstance(value)) { final List l = (List) value; for (int i = 0, n = l.size(); i < n; i++) { - consumer.accept(multiplexContext.child( + cascadeInto(multiplexContext.child( setContainerInformation(NodeImpl.atIndex(i), ITERABLE_ELEMENT, descriptor.getElementClass()), - l.get(i))); + l.get(i)), groups, sink); } return; } if (Iterable.class.isInstance(value)) { for (final Object o : (Iterable) value) { - consumer.accept(multiplexContext.child( + cascadeInto(multiplexContext.child( setContainerInformation(NodeImpl.atIndex(null), ITERABLE_ELEMENT, descriptor.getElementClass()), - o)); + o), groups, sink); } return; } - consumer.accept(multiplexContext); + cascadeInto(multiplexContext, groups, sink); } // RI apparently wants to use e.g. Set for Iterable containers, so use declared type + assigned type @@ -559,12 +566,11 @@ protected GraphContext getMultiplexContext() { GraphContext ancestor = context.getParent(); Validate.validState(ancestor != null, "Expected parent context"); - final PathImpl contextPath = context.pathReference(); - final NodeImpl leafNode = contextPath.getLeafNode(); + final NodeImpl leafNode = context.getLeafNode(); if (leafNode.getKind() == ElementKind.CONTAINER_ELEMENT) { // recurse using elided path: the ancestor whose path is ours minus the container element node - while (!contextPath.isParentPath(ancestor.pathReference())) { + while (!context.hasParentPath(ancestor)) { ancestor = ancestor.getParent(); Validate.validState(ancestor != null, "Expected parent context"); } @@ -605,6 +611,7 @@ Object getBean() { } private static final Object NO_FRAME = new Object(); + private static final Object[] NO_FRAMES = {}; /** * Sink for the violations of constraints composing one reported as a single violation. Recognized by identity so diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/parameter/DefaultParameterNameProvider.java b/bval-jsr/src/main/java/org/apache/bval/jsr/parameter/DefaultParameterNameProvider.java index 353c8b3f51..eedd243783 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/parameter/DefaultParameterNameProvider.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/parameter/DefaultParameterNameProvider.java @@ -22,7 +22,10 @@ import java.lang.reflect.Executable; import java.lang.reflect.Method; import java.lang.reflect.Parameter; +import java.util.Collections; import java.util.List; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -30,8 +33,16 @@ public class DefaultParameterNameProvider implements ParameterNameProvider { - private static List parameterNames(Executable exe) { - return Stream.of(exe.getParameters()).map(Parameter::getName).collect(Collectors.toList()); + private final ConcurrentMap> parameterNames = new ConcurrentHashMap<>(); + + private List parameterNames(Executable exe) { + List result = parameterNames.get(exe); + if (result == null) { + result = Collections.unmodifiableList( + Stream.of(exe.getParameters()).map(Parameter::getName).collect(Collectors.toList())); + parameterNames.putIfAbsent(exe, result); + } + return result; } @Override diff --git a/bval-jsr/src/main/java/org/apache/bval/util/reflection/Reflection.java b/bval-jsr/src/main/java/org/apache/bval/util/reflection/Reflection.java index 8c10c7d86d..7c84b8c0f8 100644 --- a/bval-jsr/src/main/java/org/apache/bval/util/reflection/Reflection.java +++ b/bval-jsr/src/main/java/org/apache/bval/util/reflection/Reflection.java @@ -149,6 +149,7 @@ public enum Interfaces { * Maps primitive {@code Class}es to their corresponding wrapper {@code Class}. */ private static final Map, Class> PRIMITIVE_WRAPPER_MAP; + private static final Map, Class> WRAPPER_PRIMITIVE_MAP; static { final Map, Class> m = new HashMap<>(); m.put(Boolean.TYPE, Boolean.class); @@ -161,6 +162,9 @@ public enum Interfaces { m.put(Float.TYPE, Float.class); m.put(Void.TYPE, Void.TYPE); PRIMITIVE_WRAPPER_MAP = Collections.unmodifiableMap(m); + final Map, Class> reverse = new HashMap<>(); + m.forEach((primitive, wrapper) -> reverse.put(wrapper, primitive)); + WRAPPER_PRIMITIVE_MAP = Collections.unmodifiableMap(reverse); } /** @@ -184,12 +188,7 @@ public static Class primitiveToWrapper(final Class cls) { } public static Class wrapperToPrimitive(final Class cls) { - for (Map.Entry, Class> primitiveEntry : PRIMITIVE_WRAPPER_MAP.entrySet()) { - if (primitiveEntry.getValue().equals(cls)) { - return primitiveEntry.getKey(); - } - } - return null; + return WRAPPER_PRIMITIVE_MAP.get(cls); } /**