You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bval.apache.org by rm...@apache.org on 2019/02/07 08:45:21 UTC

bval git commit: BVAL-170 adding back some caching for resource bundle and reflection

Repository: bval
Updated Branches:
  refs/heads/master 83f28d84c -> b3afb92fa


BVAL-170 adding back some caching for resource bundle and reflection


Project: http://git-wip-us.apache.org/repos/asf/bval/repo
Commit: http://git-wip-us.apache.org/repos/asf/bval/commit/b3afb92f
Tree: http://git-wip-us.apache.org/repos/asf/bval/tree/b3afb92f
Diff: http://git-wip-us.apache.org/repos/asf/bval/diff/b3afb92f

Branch: refs/heads/master
Commit: b3afb92fa21e05aaf99717ee65983c1b6222f681
Parents: 83f28d8
Author: Romain Manni-Bucau <rm...@gmail.com>
Authored: Thu Feb 7 09:45:10 2019 +0100
Committer: Romain Manni-Bucau <rm...@gmail.com>
Committed: Thu Feb 7 09:45:10 2019 +0100

----------------------------------------------------------------------
 .../apache/bval/jsr/ApacheFactoryContext.java   | 19 +++++
 .../org/apache/bval/jsr/ConstraintCached.java   |  9 +++
 .../bval/jsr/DefaultMessageInterpolator.java    | 77 ++++++++++++++++++--
 .../org/apache/bval/jsr/job/ValidationJob.java  | 50 +++++++------
 pom.xml                                         |  3 +
 5 files changed, 127 insertions(+), 31 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/bval/blob/b3afb92f/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheFactoryContext.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheFactoryContext.java b/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheFactoryContext.java
index a48e379..1ea3aed 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheFactoryContext.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/ApacheFactoryContext.java
@@ -18,7 +18,11 @@
  */
 package org.apache.bval.jsr;
 
+import java.util.concurrent.ConcurrentMap;
+import java.util.function.Supplier;
+
 import javax.validation.ClockProvider;
+import javax.validation.ConstraintValidator;
 import javax.validation.ConstraintValidatorFactory;
 import javax.validation.MessageInterpolator;
 import javax.validation.ParameterNameProvider;
@@ -27,6 +31,7 @@ import javax.validation.Validator;
 import javax.validation.ValidatorContext;
 import javax.validation.valueextraction.ValueExtractor;
 
+import org.apache.bval.jsr.descriptor.ConstraintD;
 import org.apache.bval.jsr.descriptor.DescriptorManager;
 import org.apache.bval.jsr.groups.GroupsComputer;
 import org.apache.bval.jsr.valueextraction.ValueExtractors;
@@ -185,4 +190,18 @@ public class ApacheFactoryContext implements ValidatorContext {
     public ApacheValidatorFactory getFactory() {
         return factory;
     }
+
+    public ConstraintValidator getOrComputeConstraintValidator(final ConstraintD<?> constraint, final Supplier<ConstraintValidator> computer) {
+        final ConcurrentMap<ConstraintD<?>, ConstraintValidator<?, ?>> constraintsCache = factory.getConstraintsCache().getValidators();
+        final ConstraintValidator<?, ?> validator = constraintsCache.get(constraint); // constraints are cached so we can query per instance
+        if (validator != null) {
+            return validator;
+        }
+        ConstraintValidator<?, ?> instance = computer.get();
+        final ConstraintValidator<?, ?> existing = constraintsCache.putIfAbsent(constraint, instance);
+        if (existing != null) {
+            instance = existing;
+        }
+        return instance;
+    }
 }

http://git-wip-us.apache.org/repos/asf/bval/blob/b3afb92f/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintCached.java
----------------------------------------------------------------------
diff --git a/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintCached.java b/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintCached.java
index 5dbdef0..9622b9e 100644
--- a/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintCached.java
+++ b/bval-jsr/src/main/java/org/apache/bval/jsr/ConstraintCached.java
@@ -28,6 +28,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 import java.util.stream.Collectors;
 
 import javax.validation.ConstraintDefinitionException;
@@ -35,6 +37,7 @@ import javax.validation.ConstraintValidator;
 import javax.validation.constraintvalidation.SupportedValidationTarget;
 import javax.validation.constraintvalidation.ValidationTarget;
 
+import org.apache.bval.jsr.descriptor.ConstraintD;
 import org.apache.bval.jsr.metadata.AnnotationDeclaredValidatorMappingProvider;
 import org.apache.bval.jsr.metadata.CompositeValidatorMappingProvider;
 import org.apache.bval.jsr.metadata.DualValidationMappingProvider;
@@ -98,11 +101,17 @@ public class ConstraintCached {
 
     private final Map<Class<? extends Annotation>, Set<ConstraintValidatorInfo<?>>> constraintValidatorInfo =
         new HashMap<>();
+    private final ConcurrentMap<ConstraintD<?>, ConstraintValidator<?, ?>> validators =
+        new ConcurrentHashMap<>();
 
     private final List<ValidatorMappingProvider> customValidatorMappingProviders = new ArrayList<>();
     private final Lazy<ValidatorMappingProvider> validatorMappingProvider =
         new Lazy<>(this::createValidatorMappingProvider);
 
+    public ConcurrentMap<ConstraintD<?>, ConstraintValidator<?, ?>> getValidators() {
+        return validators;
+    }
+
     public void add(ValidatorMappingProvider validatorMappingProvider) {
         customValidatorMappingProviders.add(validatorMappingProvider);
         this.validatorMappingProvider.reset(this::createValidatorMappingProvider);

http://git-wip-us.apache.org/repos/asf/bval/blob/b3afb92f/bval-jsr/src/main/java/org/apache/bval/jsr/DefaultMessageInterpolator.java
----------------------------------------------------------------------
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 15e7add..a3eb78a 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
@@ -16,8 +16,13 @@
  */
 package org.apache.bval.jsr;
 
+import static java.util.Collections.emptyEnumeration;
+import static java.util.Optional.empty;
+import static java.util.Optional.of;
+
 import java.lang.reflect.InvocationTargetException;
 import java.util.Arrays;
+import java.util.Enumeration;
 import java.util.Locale;
 import java.util.Map;
 import java.util.MissingResourceException;
@@ -25,6 +30,7 @@ import java.util.Objects;
 import java.util.Optional;
 import java.util.ResourceBundle;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
@@ -56,6 +62,18 @@ public class DefaultMessageInterpolator implements MessageInterpolator {
     private static final LookBehindRegexHolder MESSAGE_PARAMETER = new LookBehindRegexHolder(
         "(?<!(?:^|[^\\\\])(?:\\\\\\\\){0,%1$d}\\\\)\\{((?:[\\w\\.]|\\\\[\\{\\$\\}\\\\])+)\\}", n -> (n - 4) / 2);
 
+    private static final ResourceBundle EMPTY_BUNDLE = new ResourceBundle() {
+        @Override
+        protected Object handleGetObject(final String key) {
+            return null;
+        }
+
+        @Override
+        public Enumeration<String> getKeys() {
+            return emptyEnumeration();
+        }
+    };
+
     /** The default locale for the current user. */
     private Locale defaultLocale;
 
@@ -65,6 +83,8 @@ public class DefaultMessageInterpolator implements MessageInterpolator {
     /** Builtin resource bundles hashed against their locale. */
     private final Map<Locale, ResourceBundle> defaultBundlesMap = new ConcurrentHashMap<>();
 
+    private final ConcurrentMap<MessageKey, String> cachedMessages = new ConcurrentHashMap<>();
+
     private final MessageEvaluator evaluator;
 
     /**
@@ -208,7 +228,7 @@ public class DefaultMessageInterpolator implements MessageInterpolator {
                 log.log(Level.FINEST, String.format("%s found", USER_VALIDATION_MESSAGES));
             }
         }
-        return rb;
+        return rb == null ? EMPTY_BUNDLE : rb;
     }
 
     private ResourceBundle loadBundle(ClassLoader classLoader, Locale locale, String message) {
@@ -221,6 +241,17 @@ public class DefaultMessageInterpolator implements MessageInterpolator {
     }
 
     private String replaceVariables(String message, ResourceBundle bundle, Locale locale, boolean recurse) {
+        final MessageKey key = new MessageKey(locale, message, bundle);
+        final String cached = cachedMessages.get(key);
+        if (cached != null) {
+            return cached;
+        }
+        final String value = doReplaceVariables(message, bundle, locale, recurse);
+        cachedMessages.putIfAbsent(key, value);
+        return value;
+    }
+
+    private String doReplaceVariables(String message, ResourceBundle bundle, Locale locale, boolean recurse) {
         final Matcher matcher = MESSAGE_PARAMETER.matcher(message);
         final StringBuilder sb = new StringBuilder(64);
         int prev = 0;
@@ -275,13 +306,12 @@ public class DefaultMessageInterpolator implements MessageInterpolator {
 
     private Optional<String> resolveParameter(String parameterName, ResourceBundle bundle, Locale locale,
         boolean recurse) {
-        return Optional.ofNullable(bundle).map(b -> {
-            try {
-                return b.getString(parameterName);
-            } catch (final MissingResourceException e) {
-                return null;
-            }
-        }).map(v -> recurse ? replaceVariables(v, bundle, locale, recurse) : v);
+        try {
+            final String string = bundle.getString(parameterName);
+            return of(recurse ? replaceVariables(string, bundle, locale, recurse) : string);
+        } catch (final MissingResourceException e) {
+            return empty();
+        }
     }
 
     private ResourceBundle findDefaultResourceBundle(Locale locale) {
@@ -300,4 +330,35 @@ public class DefaultMessageInterpolator implements MessageInterpolator {
     public void setLocale(Locale locale) {
         defaultLocale = locale;
     }
+
+    private static class MessageKey {
+        private final Locale locale;
+        private final String key;
+        private final ResourceBundle bundle;
+        private final int hash;
+
+        private MessageKey(final Locale locale, final String key, final ResourceBundle bundle) {
+            this.locale = locale;
+            this.key = key;
+            this.bundle = bundle;
+            this.hash = Objects.hash(locale, key, bundle);
+        }
+
+        @Override
+        public boolean equals(final Object o) {
+            if (this == o) {
+                return true;
+            }
+            if (o == null || getClass() != o.getClass()) {
+                return false;
+            }
+            final MessageKey that = MessageKey.class.cast(o);
+            return locale.equals(that.locale) && key.equals(that.key) && bundle.equals(that.bundle);
+        }
+
+        @Override
+        public int hashCode() {
+            return hash;
+        }
+    }
 }

http://git-wip-us.apache.org/repos/asf/bval/blob/b3afb92f/bval-jsr/src/main/java/org/apache/bval/jsr/job/ValidationJob.java
----------------------------------------------------------------------
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 b86a79d..8d63c09 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
@@ -38,6 +38,7 @@ import java.util.stream.IntStream;
 import java.util.stream.Stream;
 
 import javax.validation.ConstraintValidator;
+import javax.validation.ConstraintValidatorContext;
 import javax.validation.ConstraintViolation;
 import javax.validation.ElementKind;
 import javax.validation.MessageInterpolator;
@@ -79,6 +80,7 @@ import org.apache.bval.util.Validate;
 import org.apache.bval.util.reflection.TypeUtils;
 
 public abstract class ValidationJob<T> {
+    private static final ConstraintValidator NOOP_VALIDATOR = (o, ctx) -> true;
 
     public abstract class Frame<D extends ElementD<?, ?>> {
         protected final Frame<?> parent;
@@ -195,30 +197,32 @@ public abstract class ValidationJob<T> {
 
         @SuppressWarnings({ "rawtypes" })
         private ConstraintValidator getConstraintValidator(ConstraintD<?> constraint) {
-            final Class<? extends ConstraintValidator> constraintValidatorClass =
-                new ComputeConstraintValidatorClass<>(validatorContext.getConstraintsCache(), constraint,
-                    getValidationTarget(), computeValidatedType(constraint)).get();
-
-            if (constraintValidatorClass == null) {
-                if (constraint.getComposingConstraints().isEmpty()) {
-                    Exceptions.raise(UnexpectedTypeException::new, "No %s type located for non-composed constraint %s",
-                        ConstraintValidator.class.getSimpleName(), constraint);
+            return validatorContext.getOrComputeConstraintValidator(constraint, () -> {
+                final Class<? extends ConstraintValidator> constraintValidatorClass =
+                        new ComputeConstraintValidatorClass<>(validatorContext.getConstraintsCache(), constraint,
+                                getValidationTarget(), computeValidatedType(constraint)).get();
+
+                if (constraintValidatorClass == null) {
+                    if (constraint.getComposingConstraints().isEmpty()) {
+                        Exceptions.raise(UnexpectedTypeException::new, "No %s type located for non-composed constraint %s",
+                                ConstraintValidator.class.getSimpleName(), constraint);
+                    }
+                    return NOOP_VALIDATOR;
                 }
-                return null;
-            }
-            ConstraintValidator constraintValidator = null;
-            Exception cause = null;
-            try {
-                constraintValidator =
-                    validatorContext.getConstraintValidatorFactory().getInstance(constraintValidatorClass);
-            } catch (Exception e) {
-                cause = e;
-            }
-            if (constraintValidator == null) {
-                Exceptions.raise(ValidationException::new, cause, "Unable to get %s instance from %s",
-                    constraintValidatorClass.getName(), validatorContext.getConstraintValidatorFactory());
-            }
-            return constraintValidator;
+                ConstraintValidator constraintValidator = null;
+                Exception cause = null;
+                try {
+                    constraintValidator =
+                            validatorContext.getConstraintValidatorFactory().getInstance(constraintValidatorClass);
+                } catch (Exception e) {
+                    cause = e;
+                }
+                if (constraintValidator == null) {
+                    Exceptions.raise(ValidationException::new, cause, "Unable to get %s instance from %s",
+                            constraintValidatorClass.getName(), validatorContext.getConstraintValidatorFactory());
+                }
+                return constraintValidator;
+            });
         }
 
         private Class<?> computeValidatedType(ConstraintD<?> constraint) {

http://git-wip-us.apache.org/repos/asf/bval/blob/b3afb92f/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 333c3bc..41a9696 100644
--- a/pom.xml
+++ b/pom.xml
@@ -573,6 +573,9 @@
                     <groupId>org.apache.maven.plugins</groupId>
                     <artifactId>maven-surefire-plugin</artifactId>
                     <version>2.20.1</version>
+                    <configuration>
+                        <trimStackTrace>false</trimStackTrace>
+                    </configuration>
                 </plugin>
                 <plugin>
                     <groupId>org.apache.maven.plugins</groupId>