apply constraints cache size to constraint attributes cache used for …

…annotation proxies
apache · Oct 17, 2018 · 1d54c14 · 1d54c14
1 parent 3735154
commit 1d54c14
Show file tree

Hide file tree

Showing 8 changed files with 67 additions and 49 deletions.
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
@@ -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
@@ -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 extends Annotation> A rewriteConstraint(A constraint, Class<?> declar
                 }
             }
             if (mustRewrite) {
-                final AnnotationProxyBuilder<A> builder = new AnnotationProxyBuilder<A>(constraint);
+                final AnnotationProxyBuilder<A> 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
@@ -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
@@ -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> T lazy(Lazy<T> 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 extends Annotation, T> A createConstraint(final ConstraintType constr
 
     private <A extends Annotation, T> A createConstraint(final ConstraintType constraint, ConstraintTarget target) {
         final Class<A> annotationClass = this.<A> loadClass(toQualifiedClassName(constraint.getAnnotation()));
-        final AnnotationProxyBuilder<A> annoBuilder = new AnnotationProxyBuilder<A>(annotationClass);
+        final AnnotationProxyBuilder<A> 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 <A extends Annotation> Annotation createAnnotation(AnnotationType annotationType, Class<A> returnType) {
-        final AnnotationProxyBuilder<A> metaAnnotation = new AnnotationProxyBuilder<>(returnType);
+        final AnnotationProxyBuilder<A> 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/.../apache/bval/jsr/xml/AnnotationProxy.java → ...apache/bval/jsr/util/AnnotationProxy.java b/.../apache/bval/jsr/xml/AnnotationProxy.java → ...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/xml/AnnotationProxyBuilder.java → ...bval/jsr/util/AnnotationProxyBuilder.java b/.../bval/jsr/xml/AnnotationProxyBuilder.java → ...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<A extends Annotation> {
-    private static final ConcurrentMap<Class<?>, Method[]> METHODS_CACHE = new ConcurrentHashMap<>();
-
-    public static <A> Method[] findMethods(final Class<A> 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<A> type;
     private final Map<String, Object> elements = new HashMap<>();
     private final Method[] methods;
@@ -68,21 +55,11 @@ public static <A> Method[] findMethods(final Class<A> annotationType) {
      * Create a new AnnotationProxyBuilder instance.
      *
      * @param annotationType
+     * @param cache
      */
-    public AnnotationProxyBuilder(final Class<A> annotationType) {
-        this.type = annotationType;
-        this.methods = findMethods(annotationType);
-    }
-
-    /**
-     * Create a new AnnotationProxyBuilder instance.
-     *
-     * @param annotationType
-     * @param elements
-     */
-    public AnnotationProxyBuilder(Class<A> annotationType, Map<String, Object> elements) {
-        this(annotationType);
-        elements.forEach(this.elements::put);
+    AnnotationProxyBuilder(final Class<A> annotationType, Map<Class<?>, 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<A> annotationType, Map<String, Object> eleme
      * 
      * @param annot
      *            Annotation to be replicated.
+     * @param cache
      */
     @SuppressWarnings("unchecked")
-    public AnnotationProxyBuilder(A annot) {
-        this((Class<A>) annot.annotationType());
+    AnnotationProxyBuilder(A annot, Map<Class<?>, Method[]> cache) {
+        this((Class<A>) 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
@@ -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 <A extends Annotation> Optional<ConstraintAnnotationAttributes.Worker<A>> validWorker(
+    private class Composition {
+        <A extends Annotation> Optional<ConstraintAnnotationAttributes.Worker<A>> validWorker(
             ConstraintAnnotationAttributes attr, Class<A> 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<Annotation> proxyBuilder = new AnnotationProxyBuilder<>(c);
+                final AnnotationProxyBuilder<Annotation> 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<AnnotatedElement> substitute(AnnotatedElement e) {
         if (e instanceof Parameter) {
@@ -310,19 +308,23 @@ private static Optional<AnnotatedElement> substitute(AnnotatedElement e) {
 
     private final ApacheValidatorFactory validatorFactory;
     private final LRUCache<Class<? extends Annotation>, Composition> compositions;
+    private final LRUCache<Class<? extends Annotation>, 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<? extends Annotation> type) {
@@ -414,6 +416,16 @@ public <A extends Annotation> Set<ValidationTarget> supportedTargets(Class<A> co
             .collect(Collectors.toCollection(() -> EnumSet.noneOf(ValidationTarget.class)));
     }
 
+    @SuppressWarnings({ "unchecked", "rawtypes" })
+    public <A extends Annotation> AnnotationProxyBuilder<A> buildProxyFor(Class<A> type) {
+        return new AnnotationProxyBuilder<>(type, (Map) constraintAttributes);
+    }
+
+    @SuppressWarnings({ "unchecked", "rawtypes" })
+    public <A extends Annotation> AnnotationProxyBuilder<A> buildProxyFor(A instance) {
+        return new AnnotationProxyBuilder<>(instance, (Map) constraintAttributes);
+    }
+
     private Composition getComposition(Class<? extends Annotation> annotationType) {
         return compositions.computeIfAbsent(annotationType, ct -> {
             final Set<ValidationTarget> 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
@@ -30,15 +30,18 @@
 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;
 import org.apache.bval.jsr.metadata.ValidatorMappingProvider;
 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<ValidatorMappingProvider> addValidatorMappingProvider, BiConsumer<Class<?>, 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<Class<?>, MetadataBuilder.ForBean<?>> builders = new XmlBuilder(mapping).forBeans();
+            final Map<Class<?>, MetadataBuilder.ForBean<?>> builders =
+                new XmlBuilder(validatorFactory, mapping).forBeans();
             if (Collections.disjoint(beanTypes, builders.keySet())) {
                 builders.forEach(addBuilder::accept);
                 beanTypes.addAll(builders.keySet());