From 1d54c14c920188f09abfe8aad294824f9d9f0a66 Mon Sep 17 00:00:00 2001 From: Matt Benson Date: Wed, 17 Oct 2018 17:17:55 -0500 Subject: [PATCH] apply constraints cache size to constraint attributes cache used for annotation proxies --- .../bval/jsr/ApacheValidatorFactory.java | 8 +++- .../bval/jsr/descriptor/MetadataReader.java | 5 ++- .../bval/jsr/metadata/MetadataSource.java | 4 ++ .../apache/bval/jsr/metadata/XmlBuilder.java | 16 +++++--- .../jsr/{xml => util}/AnnotationProxy.java | 2 +- .../{xml => util}/AnnotationProxyBuilder.java | 40 +++++-------------- .../bval/jsr/util/AnnotationsManager.java | 24 ++++++++--- .../bval/jsr/xml/ValidationMappingParser.java | 17 +++++++- 8 files changed, 67 insertions(+), 49 deletions(-) rename bval-jsr/src/main/java/org/apache/bval/jsr/{xml => util}/AnnotationProxy.java (99%) rename bval-jsr/src/main/java/org/apache/bval/jsr/{xml => util}/AnnotationProxyBuilder.java (84%) diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheValidatorFactory.java b/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheValidatorFactory.java index 481b501171..89caf6385f 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheValidatorFactory.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheValidatorFactory.java @@ -24,6 +24,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import java.util.function.BiConsumer; import javax.validation.ClockProvider; @@ -368,6 +369,11 @@ private void loadAndVerifyUserCustomizations(ConfigurationState configuration) { getMetadataBuilders().registerCustomBuilder((Class) t, (MetadataBuilder.ForBean) b); }; participantFactory.loadServices(MetadataSource.class) - .forEach(ms -> ms.process(configuration, getConstraintsCache()::add, addBuilder)); + .forEach(ms -> { + Optional.of(ms).filter(MetadataSource.FactoryDependent.class::isInstance) + .map(MetadataSource.FactoryDependent.class::cast).ifPresent(fd -> fd.setFactory(this)); + + ms.process(configuration, getConstraintsCache()::add, addBuilder); + }); } } diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/MetadataReader.java b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/MetadataReader.java index 0828933656..6ef1a8695a 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/MetadataReader.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/descriptor/MetadataReader.java @@ -65,10 +65,10 @@ import org.apache.bval.jsr.metadata.Meta; import org.apache.bval.jsr.metadata.MetadataBuilder; import org.apache.bval.jsr.metadata.Signature; +import org.apache.bval.jsr.util.AnnotationProxyBuilder; import org.apache.bval.jsr.util.AnnotationsManager; import org.apache.bval.jsr.util.Methods; import org.apache.bval.jsr.util.ToUnmodifiable; -import org.apache.bval.jsr.xml.AnnotationProxyBuilder; import org.apache.bval.util.Exceptions; import org.apache.bval.util.ObjectUtils; import org.apache.bval.util.Validate; @@ -123,7 +123,8 @@ private A rewriteConstraint(A constraint, Class declar } } if (mustRewrite) { - final AnnotationProxyBuilder builder = new AnnotationProxyBuilder(constraint); + final AnnotationProxyBuilder builder = + validatorFactory.getAnnotationsManager().buildProxyFor(constraint); builder.setGroups(groups); return builder.createAnnotation(); } diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/MetadataSource.java b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/MetadataSource.java index 44cca43b6c..29cc32b970 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/MetadataSource.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/MetadataSource.java @@ -20,12 +20,16 @@ import java.util.function.Consumer; import javax.validation.ConstraintValidator; +import javax.validation.ValidatorFactory; import javax.validation.spi.ConfigurationState; /** * Service interface for user metadata customizations. */ public interface MetadataSource { + interface FactoryDependent extends MetadataSource { + void setFactory(ValidatorFactory validatorFactory); + } /** * Add {@link ConstraintValidator} mappings and/or metadata builders. diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/XmlBuilder.java b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/XmlBuilder.java index be23e7c224..42ce4399f2 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/XmlBuilder.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/metadata/XmlBuilder.java @@ -45,10 +45,11 @@ import javax.validation.groups.Default; import javax.xml.bind.JAXBElement; +import org.apache.bval.jsr.ApacheValidatorFactory; import org.apache.bval.jsr.ConstraintAnnotationAttributes; import org.apache.bval.jsr.groups.GroupConversion; +import org.apache.bval.jsr.util.AnnotationProxyBuilder; import org.apache.bval.jsr.util.ToUnmodifiable; -import org.apache.bval.jsr.xml.AnnotationProxyBuilder; import org.apache.bval.jsr.xml.AnnotationType; import org.apache.bval.jsr.xml.BeanType; import org.apache.bval.jsr.xml.ClassType; @@ -475,13 +476,14 @@ private static final T lazy(Lazy lazy, String name) { return lazy.get(); } + private final ApacheValidatorFactory validatorFactory; private final ConstraintMappingsType constraintMappings; private final Version version; - public XmlBuilder(ConstraintMappingsType constraintMappings) { + public XmlBuilder(ApacheValidatorFactory validatorFactory, ConstraintMappingsType constraintMappings) { super(); - this.constraintMappings = constraintMappings; - Validate.notNull(constraintMappings, "constraintMappings"); + this.validatorFactory = Validate.notNull(validatorFactory, "validatorFactory"); + this.constraintMappings = Validate.notNull(constraintMappings, "constraintMappings"); this.version = Version.of(constraintMappings); new MappingValidator(constraintMappings, this::resolveClass).validateMappings(); } @@ -544,7 +546,8 @@ private A createConstraint(final ConstraintType constr private A createConstraint(final ConstraintType constraint, ConstraintTarget target) { final Class annotationClass = this. loadClass(toQualifiedClassName(constraint.getAnnotation())); - final AnnotationProxyBuilder annoBuilder = new AnnotationProxyBuilder(annotationClass); + final AnnotationProxyBuilder annoBuilder = + validatorFactory.getAnnotationsManager().buildProxyFor(annotationClass); if (constraint.getMessage() != null) { annoBuilder.setMessage(constraint.getMessage()); @@ -676,7 +679,8 @@ private Object convertToResultType(Class returnType, String value) { } private Annotation createAnnotation(AnnotationType annotationType, Class returnType) { - final AnnotationProxyBuilder metaAnnotation = new AnnotationProxyBuilder<>(returnType); + final AnnotationProxyBuilder metaAnnotation = + validatorFactory.getAnnotationsManager().buildProxyFor(returnType); for (ElementType elementType : annotationType.getElement()) { final String name = elementType.getName(); metaAnnotation.setValue(name, getElementValue(elementType, getAnnotationParameterType(returnType, name))); diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/xml/AnnotationProxy.java b/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationProxy.java similarity index 99% rename from bval-jsr/src/main/java/org/apache/bval/jsr/xml/AnnotationProxy.java rename to bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationProxy.java index 44d67b8833..1d21189407 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/xml/AnnotationProxy.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationProxy.java @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.bval.jsr.xml; +package org.apache.bval.jsr.util; import java.io.Serializable; import java.lang.annotation.Annotation; diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/xml/AnnotationProxyBuilder.java b/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationProxyBuilder.java similarity index 84% rename from bval-jsr/src/main/java/org/apache/bval/jsr/xml/AnnotationProxyBuilder.java rename to bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationProxyBuilder.java index dd2756093a..ee9a139c23 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/xml/AnnotationProxyBuilder.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationProxyBuilder.java @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.bval.jsr.xml; +package org.apache.bval.jsr.util; import java.lang.annotation.Annotation; import java.lang.reflect.Constructor; @@ -24,8 +24,6 @@ import java.util.HashMap; import java.util.Map; import java.util.Objects; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import javax.enterprise.util.AnnotationLiteral; import javax.validation.ConstraintTarget; @@ -36,7 +34,7 @@ import org.apache.bval.cdi.EmptyAnnotationLiteral; import org.apache.bval.jsr.ConstraintAnnotationAttributes; -import org.apache.bval.jsr.util.AnnotationsManager; +import org.apache.bval.util.Validate; import org.apache.bval.util.reflection.Reflection; import org.apache.commons.weaver.privilizer.Privileged; import org.apache.commons.weaver.privilizer.Privilizing; @@ -48,17 +46,6 @@ */ @Privilizing(@CallTo(Reflection.class)) public final class AnnotationProxyBuilder { - private static final ConcurrentMap, Method[]> METHODS_CACHE = new ConcurrentHashMap<>(); - - public static Method[] findMethods(final Class annotationType) { - // cache only built-in constraints to avoid memory leaks: - // TODO use configurable cache size property? - if (annotationType.getName().startsWith("javax.validation.constraints.")) { - return METHODS_CACHE.computeIfAbsent(annotationType, Reflection::getDeclaredMethods); - } - return Reflection.getDeclaredMethods(annotationType); - } - private final Class type; private final Map elements = new HashMap<>(); private final Method[] methods; @@ -68,21 +55,11 @@ public static Method[] findMethods(final Class annotationType) { * Create a new AnnotationProxyBuilder instance. * * @param annotationType + * @param cache */ - public AnnotationProxyBuilder(final Class annotationType) { - this.type = annotationType; - this.methods = findMethods(annotationType); - } - - /** - * Create a new AnnotationProxyBuilder instance. - * - * @param annotationType - * @param elements - */ - public AnnotationProxyBuilder(Class annotationType, Map elements) { - this(annotationType); - elements.forEach(this.elements::put); + AnnotationProxyBuilder(final Class annotationType, Map, Method[]> cache) { + this.type = Validate.notNull(annotationType, "annotationType"); + this.methods = Validate.notNull(cache, "cache").computeIfAbsent(annotationType, Reflection::getDeclaredMethods); } /** @@ -91,10 +68,11 @@ public AnnotationProxyBuilder(Class annotationType, Map eleme * * @param annot * Annotation to be replicated. + * @param cache */ @SuppressWarnings("unchecked") - public AnnotationProxyBuilder(A annot) { - this((Class) annot.annotationType()); + AnnotationProxyBuilder(A annot, Map, Method[]> cache) { + this((Class) annot.annotationType(), cache); elements.putAll(AnnotationsManager.readAttributes(annot)); } diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationsManager.java b/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationsManager.java index ead0a3b938..dd9c28b7ba 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationsManager.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/util/AnnotationsManager.java @@ -56,7 +56,6 @@ import org.apache.bval.jsr.ConstraintAnnotationAttributes.Worker; import org.apache.bval.jsr.ConstraintCached.ConstraintValidatorInfo; import org.apache.bval.jsr.metadata.Meta; -import org.apache.bval.jsr.xml.AnnotationProxyBuilder; import org.apache.bval.util.Exceptions; import org.apache.bval.util.Lazy; import org.apache.bval.util.ObjectUtils; @@ -107,8 +106,8 @@ public int hashCode() { } } - private static class Composition { - static Optional> validWorker( + private class Composition { + Optional> validWorker( ConstraintAnnotationAttributes attr, Class type) { return Optional.of(type).map(attr::analyze).filter(Worker::isValid); } @@ -179,7 +178,7 @@ Annotation[] getComponents(Annotation source) { final int index = constraintCounts.computeIfAbsent(c.annotationType(), k -> new AtomicInteger()).getAndIncrement(); - final AnnotationProxyBuilder proxyBuilder = new AnnotationProxyBuilder<>(c); + final AnnotationProxyBuilder proxyBuilder = buildProxyFor(c); proxyBuilder.setGroups(groups); proxyBuilder.setPayload(payload); @@ -281,7 +280,6 @@ private static Annotation[] getDeclaredConstraints(AnnotatedElement e) { } return constraints.toArray(Annotation[]::new); } - private static Optional substitute(AnnotatedElement e) { if (e instanceof Parameter) { @@ -310,19 +308,23 @@ private static Optional substitute(AnnotatedElement e) { private final ApacheValidatorFactory validatorFactory; private final LRUCache, Composition> compositions; + private final LRUCache, Method[]> constraintAttributes; public AnnotationsManager(ApacheValidatorFactory validatorFactory) { super(); this.validatorFactory = Validate.notNull(validatorFactory); final String cacheSize = validatorFactory.getProperties().get(ConfigurationImpl.Properties.CONSTRAINTS_CACHE_SIZE); + final int sz; try { - compositions = new LRUCache<>(Integer.parseInt(cacheSize)); + sz = Integer.parseInt(cacheSize); } catch (NumberFormatException e) { throw Exceptions.create(IllegalStateException::new, e, "Cannot parse value %s for configuration property %s", cacheSize, ConfigurationImpl.Properties.CONSTRAINTS_CACHE_SIZE); } + compositions = new LRUCache<>(sz); + constraintAttributes = new LRUCache<>(sz); } public void validateConstraintDefinition(Class type) { @@ -414,6 +416,16 @@ public Set supportedTargets(Class co .collect(Collectors.toCollection(() -> EnumSet.noneOf(ValidationTarget.class))); } + @SuppressWarnings({ "unchecked", "rawtypes" }) + public AnnotationProxyBuilder buildProxyFor(Class type) { + return new AnnotationProxyBuilder<>(type, (Map) constraintAttributes); + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) + public AnnotationProxyBuilder buildProxyFor(A instance) { + return new AnnotationProxyBuilder<>(instance, (Map) constraintAttributes); + } + private Composition getComposition(Class annotationType) { return compositions.computeIfAbsent(annotationType, ct -> { final Set composedTargets = supportedTargets(annotationType); diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/xml/ValidationMappingParser.java b/bval-jsr/src/main/java/org/apache/bval/jsr/xml/ValidationMappingParser.java index 59cef76b7f..af80d0070b 100644 --- a/bval-jsr/src/main/java/org/apache/bval/jsr/xml/ValidationMappingParser.java +++ b/bval-jsr/src/main/java/org/apache/bval/jsr/xml/ValidationMappingParser.java @@ -30,8 +30,10 @@ import java.util.stream.Collectors; import javax.validation.ValidationException; +import javax.validation.ValidatorFactory; import javax.validation.spi.ConfigurationState; +import org.apache.bval.jsr.ApacheValidatorFactory; import org.apache.bval.jsr.metadata.MetadataBuilder; import org.apache.bval.jsr.metadata.MetadataBuilder.ForBean; import org.apache.bval.jsr.metadata.MetadataSource; @@ -39,6 +41,7 @@ import org.apache.bval.jsr.metadata.XmlBuilder; import org.apache.bval.jsr.metadata.XmlValidationMappingProvider; import org.apache.bval.util.Exceptions; +import org.apache.bval.util.Validate; import org.apache.bval.util.reflection.Reflection; import org.apache.commons.weaver.privilizer.Privilizing; import org.apache.commons.weaver.privilizer.Privilizing.CallTo; @@ -48,7 +51,7 @@ * Uses JAXB to parse constraints.xml based on the validation-mapping XML schema. */ @Privilizing(@CallTo(Reflection.class)) -public class ValidationMappingParser implements MetadataSource { +public class ValidationMappingParser implements MetadataSource.FactoryDependent { private static final SchemaManager SCHEMA_MANAGER = new SchemaManager.Builder() .add(XmlBuilder.Version.v10.getId(), "http://jboss.org/xml/ns/javax/validation/mapping", "META-INF/validation-mapping-1.0.xsd") @@ -58,9 +61,18 @@ public class ValidationMappingParser implements MetadataSource { "META-INF/validation-mapping-2.0.xsd") .build(); + private ApacheValidatorFactory validatorFactory; + + @Override + public void setFactory(ValidatorFactory validatorFactory) { + this.validatorFactory = Validate.notNull(validatorFactory).unwrap(ApacheValidatorFactory.class); + } + @Override public void process(ConfigurationState configurationState, Consumer addValidatorMappingProvider, BiConsumer, ForBean> addBuilder) { + Validate.validState(validatorFactory != null, "validatorFactory unknown"); + if (configurationState.isIgnoreXmlConfiguration()) { return; } @@ -70,7 +82,8 @@ public void process(ConfigurationState configurationState, Optional.of(mapping).map(this::toMappingProvider).ifPresent(addValidatorMappingProvider); - final Map, MetadataBuilder.ForBean> builders = new XmlBuilder(mapping).forBeans(); + final Map, MetadataBuilder.ForBean> builders = + new XmlBuilder(validatorFactory, mapping).forBeans(); if (Collections.disjoint(beanTypes, builders.keySet())) { builders.forEach(addBuilder::accept); beanTypes.addAll(builders.keySet());