From 6998971ac9586502e14bfce9fc356d903016d343 Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Mon, 5 Oct 2026 09:47:59 +0200 Subject: [PATCH 1/4] Cache the constraints matching a group strategy per element Filter an element's constraints by the groups of a strategy once and cache the result on the descriptor instead of streaming over all constraints for every validation. The per-strategy caches first match recently seen strategy instances by identity, as validation keeps passing the same ones. --- .../apache/bval/jsr/descriptor/ElementD.java | 45 +++++++++++ .../bval/jsr/descriptor/GroupStrategyMap.java | 81 +++++++++++++++++++ .../apache/bval/jsr/job/ValidationJob.java | 29 +------ 3 files changed, 130 insertions(+), 25 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/descriptor/ElementD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/ElementD.java index 207f7ed844..a404878f1a 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 @@ -25,9 +25,11 @@ import java.util.Map; import java.util.Set; +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 +74,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} + */ + 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 GroupStrategyMap[]> constraintsByGroups; protected ElementD(R reader) { super(); @@ -84,6 +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 GroupStrategyMap<>(); this.groupsComputer = reader.getValidatorFactory().getGroupsComputer(); } @@ -98,6 +123,26 @@ 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(); + return constraintsByGroups.putIfAbsent(groups, + constraints.stream().filter(c -> matchesGroups(c, targetGroups)).toArray(ConstraintD[]::new)); + } + @Override public final ConstraintFinder findConstraints() { return new Finder(groupsComputer, this); 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..2d741609cd --- /dev/null +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/GroupStrategyMap.java @@ -0,0 +1,81 @@ +/* + * 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 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; + + /** + * @return the value for {@code key}, or {@code null} if none has been stored yet + */ + @SuppressWarnings("unchecked") + 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]; + } + } + 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); + grown[entries.length] = key; + grown[entries.length + 1] = value; + byIdentity = grown; + } + } +} 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 c58f02c2e0..88c582e453 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 @@ -43,7 +43,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.ConstraintDescriptor; import jakarta.validation.metadata.ContainerDescriptor; @@ -117,7 +116,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; @@ -349,7 +350,7 @@ void validateDescriptorConstraints(GroupStrategy groups, Consumer { - if (!constraintsFor(d, groups).findFirst().isPresent() + if (d.getConstraintsFor(groups).length == 0 && d.getConstrainedContainerElementTypes().isEmpty()) { return; } @@ -585,28 +586,6 @@ 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. From c291403434024c283f96d90ac742a2a26452dc52 Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Mon, 5 Oct 2026 09:50:09 +0200 Subject: [PATCH 2/4] Validate only the properties relevant to the requested groups - Cache the Default group redefinition per bean descriptor and requested group strategy instead of computing it for every bean and property. - Cache per bean and group strategy the properties that need a frame: those with constraints in scope, cascaded, or with constrained container elements. Properties with nothing to validate are no longer read. - Precompute whether a container descriptor is cascaded deeply. --- .../org/apache/bval/jsr/descriptor/BeanD.java | 62 ++++++++++++++++ .../jsr/descriptor/CascadableContainerD.java | 12 ++++ .../jsr/descriptor/DescriptorManager.java | 3 + .../apache/bval/jsr/descriptor/ElementD.java | 14 ++++ .../apache/bval/jsr/descriptor/PropertyD.java | 20 +++++- .../apache/bval/jsr/job/ValidationJob.java | 72 +++++++++---------- 6 files changed, 143 insertions(+), 40 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 91c24d2b21..7fe2d4b844 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,9 +19,11 @@ 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; @@ -31,6 +33,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 +55,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 GroupStrategyMap localGroupStrategies = new GroupStrategyMap<>(); + private final GroupStrategyMap[]> propertiesByGroups = new GroupStrategyMap<>(); + + private static final GroupStrategy UNCHANGED = GroupStrategy.simple(Collections.emptySet()); + BeanD(MetadataReader.ForBean reader) { super(reader); this.beanClass = reader.meta.getHost(); @@ -65,6 +79,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 +160,49 @@ 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} + */ + @Override + 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 = localGroupStrategies.putIfAbsent(groups, computed == groups ? UNCHANGED : computed); + } + 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) { + 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 propertiesByGroups.putIfAbsent(groups, relevant.toArray(new PropertyD[0])); + } + 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/DescriptorManager.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/DescriptorManager.java index fbf132d294..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 @@ -50,6 +50,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)); } 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 a404878f1a..9c25f8be17 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 = {}; @@ -164,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/PropertyD.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/PropertyD.java index 2fdd70f22f..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 @@ -25,6 +25,7 @@ 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; @@ -32,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; @@ -115,6 +117,8 @@ MethodHandle unreflect(Lookup lookup) throws IllegalAccessException { 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 @@ -129,12 +133,22 @@ protected PropertyD(MetadataReader.ForContainer reader, BeanD 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); } 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 88c582e453..c6c3f00d1f 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; @@ -32,7 +31,6 @@ import java.util.Set; import java.util.function.Consumer; import java.util.function.Predicate; -import java.util.stream.Stream; import jakarta.validation.ConstraintValidator; import jakarta.validation.ConstraintViolation; @@ -103,8 +101,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); } @@ -275,22 +272,23 @@ public class BeanFrame extends Frame> { @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 -> { 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)); + if (redefined) { + for (final Frame p : propertyFrames) { + p.recurse(groups, sink); + } } } @@ -303,32 +301,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 -> { - 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; - } - } catch (ValidationException ve) { - throw ve; - } catch (Exception e) { - throw new ValidationException(e); + 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())) { + 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; } From 7b0fc7c5b8f491e63252cbb0b9ad527f7d62b407 Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Mon, 5 Oct 2026 09:50:51 +0200 Subject: [PATCH 3/4] Create property frames lazily per group Build a bean's property frames on first use per group, and skip properties without constraints in the current group, so that properties relevant only to groups that are never validated (such as later steps of a failing group sequence) are neither read nor resolved. --- .../apache/bval/jsr/job/ValidationJob.java | 100 ++++++++++++------ 1 file changed, 66 insertions(+), 34 deletions(-) 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 c6c3f00d1f..7d51c3f612 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; @@ -274,22 +273,49 @@ 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 PropertyD[] properties = descriptor.getPropertiesFor(localGroupStrategy); + // 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]; - localGroupStrategy.applyTo(noViolations(gs -> { - validateDescriptorConstraints(gs, sink); - for (final Frame p : propertyFrames) { + 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 = propertyFrame(properties, propertyFrames, i); + if (p != null) { + p.recurse(groups, 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 (!redefined) { + if (recurse) { p.recurse(gs, sink); } } - })); - if (redefined) { - for (final Frame p : propertyFrames) { - p.recurse(groups, sink); - } } + return violationCount() == originalCount; } protected Frame propertyFrame(PropertyD d, GraphContext context) { @@ -301,34 +327,37 @@ Object getBean() { return context.getValue(); } - private List> propertyFrames(PropertyD[] properties) { - if (properties.length == 0) { - return Collections.emptyList(); + /** + * @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 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 createPropertyFrame(PropertyD d) { final TraversableResolver traversableResolver = validatorContext.getTraversableResolver(); - 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())) { - continue; - } - } catch (ValidationException ve) { - throw ve; - } catch (Exception e) { - throw new ValidationException(e); + 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; } - } - final GraphContext child = d.readChild(realContext); - if (child != null && !child.isRecursive()) { - frames.add(propertyFrame(d, child)); + } catch (ValidationException ve) { + throw ve; + } catch (Exception e) { + throw new ValidationException(e); } } - return frames; + final GraphContext child = d.readChild(realContext); + return child == null || child.isRecursive() ? NO_FRAME : propertyFrame(d, child); } } @@ -574,6 +603,9 @@ 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 * that such violations are not built at all. From 245457c782488c091632d3946d1b16c6151ceafb Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Mon, 5 Oct 2026 09:51:16 +0200 Subject: [PATCH 4/4] Resolve a bean's validation plan in one lookup and validate single groups directly - Combine the Default group redefinition and the relevant properties of a bean for a requested group strategy into one cached plan. - Validate a single-group strategy directly instead of through GroupStrategy#applyTo callbacks. --- .../org/apache/bval/jsr/descriptor/BeanD.java | 47 ++++++++++++++++++- .../apache/bval/jsr/job/ValidationJob.java | 12 +++-- 2 files changed, 55 insertions(+), 4 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 7fe2d4b844..7942c5f25d 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 @@ -63,6 +63,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<>(); private static final GroupStrategy UNCHANGED = GroupStrategy.simple(Collections.emptySet()); @@ -178,6 +179,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 the leaf properties that need a validation frame for 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 the properties that need a validation frame 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 @@ -186,7 +231,7 @@ public GroupStrategy getLocalGroupStrategy(GroupStrategy groups) { * @param groups * @return {@link PropertyD} array, possibly empty */ - public PropertyD[] getPropertiesFor(GroupStrategy groups) { + private PropertyD[] getPropertiesFor(GroupStrategy groups) { final PropertyD[] cached = propertiesByGroups.get(groups); if (cached != null) { return cached; 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 7d51c3f612..84bbcfe4e3 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 @@ -271,14 +271,20 @@ 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 PropertyD[] properties = descriptor.getPropertiesFor(localGroupStrategy); + 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]; - localGroupStrategy.applyTo(gs -> validateGroup(gs, properties, propertyFrames, redefined, sink)); + 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()) {