You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by GitBox <gi...@apache.org> on 2020/09/18 14:46:27 UTC

[GitHub] [cxf] rmannibucau opened a new pull request #695: Enable to pass a bval Validator to BeanValidationProvider (it is more…

rmannibucau opened a new pull request #695:
URL: https://github.com/apache/cxf/pull/695


   … than enough and faster than creating validators again and again) + enable to cache the fact no validation is needed


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta commented on a change in pull request #695: Enable to pass a bval Validator to BeanValidationProvider (it is more…

Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #695:
URL: https://github.com/apache/cxf/pull/695#discussion_r491763217



##########
File path: core/src/main/java/org/apache/cxf/validation/BeanValidationProvider.java
##########
@@ -111,46 +142,81 @@ private static void initFactoryConfig(Configuration<?> factoryCfg, ValidationCon
     }
 
     public< T > void validateParameters(final T instance, final Method method, final Object[] arguments) {
-
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set< ConstraintViolation< T > > violations = methodValidator.validateParameters(instance,
-            method, arguments);
-
-        if (!violations.isEmpty()) {
-            throw new ConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateParameters(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateParameters(instance,
+                    method, arguments);
+            if (!violations.isEmpty()) {
+                throw new ConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T instance, final Method method, final Object returnValue) {
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set<ConstraintViolation< T > > violations = methodValidator.validateReturnValue(instance,
-            method, returnValue);
-
-        if (!violations.isEmpty()) {
-            throw new ResponseConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateReturnedValue(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateReturnValue(instance,
+                    method, returnValue);
+            if (!violations.isEmpty()) {
+                throw new ResponseConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T bean) {
-        final Set<ConstraintViolation< T > > violations = doValidateBean(bean);
+        Validator validator = factory.get();
+        if (runtimeCache != null && bean != null
+                && !runtimeCache.shouldValidateBean(validator, bean.getClass())) {

Review comment:
       Shouldn't it be `!runtimeCache.shouldValidateReturnedValue`?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] rmannibucau commented on pull request #695: Enable to pass a bval Validator to BeanValidationProvider (it is more…

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #695:
URL: https://github.com/apache/cxf/pull/695#issuecomment-695909321


   @reta thanks for the review, fixed the last comment, think others are ok (already "protected" by the caller)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] rmannibucau commented on pull request #695: Enable to pass a bval Validator to BeanValidationProvider (it is more…

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #695:
URL: https://github.com/apache/cxf/pull/695#issuecomment-695909321


   @reta thanks for the review, fixed the last comment, think others are ok (already "protected" by the caller)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] rmannibucau commented on a change in pull request #695: Enable to pass a bval Validator to BeanValidationProvider (it is more…

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #695:
URL: https://github.com/apache/cxf/pull/695#discussion_r491800652



##########
File path: core/src/main/java/org/apache/cxf/validation/BeanValidationProvider.java
##########
@@ -111,46 +142,81 @@ private static void initFactoryConfig(Configuration<?> factoryCfg, ValidationCon
     }
 
     public< T > void validateParameters(final T instance, final Method method, final Object[] arguments) {
-
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set< ConstraintViolation< T > > violations = methodValidator.validateParameters(instance,
-            method, arguments);
-
-        if (!violations.isEmpty()) {
-            throw new ConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateParameters(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateParameters(instance,
+                    method, arguments);
+            if (!violations.isEmpty()) {
+                throw new ConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T instance, final Method method, final Object returnValue) {
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set<ConstraintViolation< T > > violations = methodValidator.validateReturnValue(instance,
-            method, returnValue);
-
-        if (!violations.isEmpty()) {
-            throw new ResponseConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateReturnedValue(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateReturnValue(instance,
+                    method, returnValue);
+            if (!violations.isEmpty()) {
+                throw new ResponseConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T bean) {
-        final Set<ConstraintViolation< T > > violations = doValidateBean(bean);
+        Validator validator = factory.get();
+        if (runtimeCache != null && bean != null
+                && !runtimeCache.shouldValidateBean(validator, bean.getClass())) {
+            return;
+        }
+        final Set<ConstraintViolation< T > > violations = doValidateBean(validator, bean);
         if (!violations.isEmpty()) {
             throw new ResponseConstraintViolationException(violations);
         }
     }
 
     public< T > void validateBean(final T bean) {
-        final Set<ConstraintViolation< T > > violations = doValidateBean(bean);
+        final Set<ConstraintViolation< T > > violations = doValidateBean(factory.get(), bean);
         if (!violations.isEmpty()) {
             throw new ConstraintViolationException(violations);
         }
     }
 
-    private< T > Set<ConstraintViolation< T > > doValidateBean(final T bean) {
-        return factory.getValidator().validate(bean);
+    private< T > Set<ConstraintViolation< T > > doValidateBean(final Validator validator, final T bean) {
+        if (validator.getConstraintsForClass(bean.getClass()).isBeanConstrained()) {

Review comment:
       think this call is always protected by a public call before which already have the exit condition - at least was the intent




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta merged pull request #695: Enable to pass a bval Validator to BeanValidationProvider (it is more…

Posted by GitBox <gi...@apache.org>.
reta merged pull request #695:
URL: https://github.com/apache/cxf/pull/695


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta commented on a change in pull request #695: Enable to pass a bval Validator to BeanValidationProvider (it is more…

Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #695:
URL: https://github.com/apache/cxf/pull/695#discussion_r491764413



##########
File path: core/src/main/java/org/apache/cxf/validation/BeanValidationProvider.java
##########
@@ -111,46 +142,81 @@ private static void initFactoryConfig(Configuration<?> factoryCfg, ValidationCon
     }
 
     public< T > void validateParameters(final T instance, final Method method, final Object[] arguments) {
-
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set< ConstraintViolation< T > > violations = methodValidator.validateParameters(instance,
-            method, arguments);
-
-        if (!violations.isEmpty()) {
-            throw new ConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateParameters(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateParameters(instance,
+                    method, arguments);
+            if (!violations.isEmpty()) {
+                throw new ConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T instance, final Method method, final Object returnValue) {
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set<ConstraintViolation< T > > violations = methodValidator.validateReturnValue(instance,
-            method, returnValue);
-
-        if (!violations.isEmpty()) {
-            throw new ResponseConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateReturnedValue(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateReturnValue(instance,
+                    method, returnValue);
+            if (!violations.isEmpty()) {
+                throw new ResponseConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T bean) {
-        final Set<ConstraintViolation< T > > violations = doValidateBean(bean);
+        Validator validator = factory.get();
+        if (runtimeCache != null && bean != null
+                && !runtimeCache.shouldValidateBean(validator, bean.getClass())) {
+            return;
+        }
+        final Set<ConstraintViolation< T > > violations = doValidateBean(validator, bean);
         if (!violations.isEmpty()) {
             throw new ResponseConstraintViolationException(violations);
         }
     }
 
     public< T > void validateBean(final T bean) {
-        final Set<ConstraintViolation< T > > violations = doValidateBean(bean);
+        final Set<ConstraintViolation< T > > violations = doValidateBean(factory.get(), bean);
         if (!violations.isEmpty()) {
             throw new ConstraintViolationException(violations);
         }
     }
 
-    private< T > Set<ConstraintViolation< T > > doValidateBean(final T bean) {
-        return factory.getValidator().validate(bean);
+    private< T > Set<ConstraintViolation< T > > doValidateBean(final Validator validator, final T bean) {
+        if (validator.getConstraintsForClass(bean.getClass()).isBeanConstrained()) {
+            return validator.validate(bean);
+        }
+        return emptySet();
+    }
+
+    @Override
+    public void close() {
+        close.run();
     }
 
-    private ExecutableValidator getExecutableValidator() {
+    // only created when there is a single validator/factory so it is safe to cache
+    // note: the validator is passed as param to avoid to create useless ones
+    private static class RuntimeCache {
+        private final ConcurrentMap<Class<?>, Boolean> types = new ConcurrentHashMap<>();
+        private final ConcurrentMap<Method, Boolean> params = new ConcurrentHashMap<>();
+        private final ConcurrentMap<Method, Boolean> returnedValues = new ConcurrentHashMap<>();
+
+        public boolean shouldValidateParameters(final Validator validator, final Method method) {
+            return params.computeIfAbsent(method, m -> validator.getConstraintsForClass(m.getDeclaringClass())
+                    .getConstraintsForMethod(m.getName(), m.getParameterTypes())
+                    .hasConstrainedParameters());
+        }
 
-        return factory.getValidator().forExecutables();
+        public boolean shouldValidateReturnedValue(final Validator validator, final Method method) {
+            return returnedValues.computeIfAbsent(method, m -> validator.getConstraintsForClass(m.getDeclaringClass())
+                    .getConstraintsForMethod(m.getName(), method.getParameterTypes())
+                    .hasConstrainedReturnValue());
+        }
+
+        public boolean shouldValidateBean(final Validator validator, final Class<?> clazz) {
+            return types.computeIfAbsent(clazz, it -> validator.getConstraintsForClass(it).hasConstraints());

Review comment:
       Should you also check for `isBeanConstrained` and cache that?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta commented on a change in pull request #695: Enable to pass a bval Validator to BeanValidationProvider (it is more…

Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #695:
URL: https://github.com/apache/cxf/pull/695#discussion_r491763387



##########
File path: core/src/main/java/org/apache/cxf/validation/BeanValidationProvider.java
##########
@@ -111,46 +142,81 @@ private static void initFactoryConfig(Configuration<?> factoryCfg, ValidationCon
     }
 
     public< T > void validateParameters(final T instance, final Method method, final Object[] arguments) {
-
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set< ConstraintViolation< T > > violations = methodValidator.validateParameters(instance,
-            method, arguments);
-
-        if (!violations.isEmpty()) {
-            throw new ConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateParameters(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateParameters(instance,
+                    method, arguments);
+            if (!violations.isEmpty()) {
+                throw new ConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T instance, final Method method, final Object returnValue) {
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set<ConstraintViolation< T > > violations = methodValidator.validateReturnValue(instance,
-            method, returnValue);
-
-        if (!violations.isEmpty()) {
-            throw new ResponseConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateReturnedValue(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateReturnValue(instance,
+                    method, returnValue);
+            if (!violations.isEmpty()) {
+                throw new ResponseConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T bean) {
-        final Set<ConstraintViolation< T > > violations = doValidateBean(bean);
+        Validator validator = factory.get();
+        if (runtimeCache != null && bean != null
+                && !runtimeCache.shouldValidateBean(validator, bean.getClass())) {
+            return;
+        }
+        final Set<ConstraintViolation< T > > violations = doValidateBean(validator, bean);
         if (!violations.isEmpty()) {
             throw new ResponseConstraintViolationException(violations);
         }
     }
 
     public< T > void validateBean(final T bean) {
-        final Set<ConstraintViolation< T > > violations = doValidateBean(bean);
+        final Set<ConstraintViolation< T > > violations = doValidateBean(factory.get(), bean);
         if (!violations.isEmpty()) {
             throw new ConstraintViolationException(violations);
         }
     }
 
-    private< T > Set<ConstraintViolation< T > > doValidateBean(final T bean) {
-        return factory.getValidator().validate(bean);
+    private< T > Set<ConstraintViolation< T > > doValidateBean(final Validator validator, final T bean) {
+        if (validator.getConstraintsForClass(bean.getClass()).isBeanConstrained()) {

Review comment:
       Probably call to `runtime.shouldValidateBean` is needed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta commented on pull request #695: Enable to pass a bval Validator to BeanValidationProvider (it is more…

Posted by GitBox <gi...@apache.org>.
reta commented on pull request #695:
URL: https://github.com/apache/cxf/pull/695#issuecomment-713192022


   @rmannibucau anything outstanding left for this pr? or good to merge?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta commented on a change in pull request #695: Enable to pass a bval Validator to BeanValidationProvider (it is more…

Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #695:
URL: https://github.com/apache/cxf/pull/695#discussion_r491763217



##########
File path: core/src/main/java/org/apache/cxf/validation/BeanValidationProvider.java
##########
@@ -111,46 +142,81 @@ private static void initFactoryConfig(Configuration<?> factoryCfg, ValidationCon
     }
 
     public< T > void validateParameters(final T instance, final Method method, final Object[] arguments) {
-
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set< ConstraintViolation< T > > violations = methodValidator.validateParameters(instance,
-            method, arguments);
-
-        if (!violations.isEmpty()) {
-            throw new ConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateParameters(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateParameters(instance,
+                    method, arguments);
+            if (!violations.isEmpty()) {
+                throw new ConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T instance, final Method method, final Object returnValue) {
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set<ConstraintViolation< T > > violations = methodValidator.validateReturnValue(instance,
-            method, returnValue);
-
-        if (!violations.isEmpty()) {
-            throw new ResponseConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateReturnedValue(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateReturnValue(instance,
+                    method, returnValue);
+            if (!violations.isEmpty()) {
+                throw new ResponseConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T bean) {
-        final Set<ConstraintViolation< T > > violations = doValidateBean(bean);
+        Validator validator = factory.get();
+        if (runtimeCache != null && bean != null
+                && !runtimeCache.shouldValidateBean(validator, bean.getClass())) {

Review comment:
       Shouldn't it be `!runtimeCache.shouldValidateReturnedValue`?

##########
File path: core/src/main/java/org/apache/cxf/validation/BeanValidationProvider.java
##########
@@ -111,46 +142,81 @@ private static void initFactoryConfig(Configuration<?> factoryCfg, ValidationCon
     }
 
     public< T > void validateParameters(final T instance, final Method method, final Object[] arguments) {
-
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set< ConstraintViolation< T > > violations = methodValidator.validateParameters(instance,
-            method, arguments);
-
-        if (!violations.isEmpty()) {
-            throw new ConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateParameters(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateParameters(instance,
+                    method, arguments);
+            if (!violations.isEmpty()) {
+                throw new ConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T instance, final Method method, final Object returnValue) {
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set<ConstraintViolation< T > > violations = methodValidator.validateReturnValue(instance,
-            method, returnValue);
-
-        if (!violations.isEmpty()) {
-            throw new ResponseConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateReturnedValue(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateReturnValue(instance,
+                    method, returnValue);
+            if (!violations.isEmpty()) {
+                throw new ResponseConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T bean) {
-        final Set<ConstraintViolation< T > > violations = doValidateBean(bean);
+        Validator validator = factory.get();
+        if (runtimeCache != null && bean != null
+                && !runtimeCache.shouldValidateBean(validator, bean.getClass())) {
+            return;
+        }
+        final Set<ConstraintViolation< T > > violations = doValidateBean(validator, bean);
         if (!violations.isEmpty()) {
             throw new ResponseConstraintViolationException(violations);
         }
     }
 
     public< T > void validateBean(final T bean) {
-        final Set<ConstraintViolation< T > > violations = doValidateBean(bean);
+        final Set<ConstraintViolation< T > > violations = doValidateBean(factory.get(), bean);
         if (!violations.isEmpty()) {
             throw new ConstraintViolationException(violations);
         }
     }
 
-    private< T > Set<ConstraintViolation< T > > doValidateBean(final T bean) {
-        return factory.getValidator().validate(bean);
+    private< T > Set<ConstraintViolation< T > > doValidateBean(final Validator validator, final T bean) {
+        if (validator.getConstraintsForClass(bean.getClass()).isBeanConstrained()) {

Review comment:
       Probably call to `runtime.shouldValidateBean` is needed

##########
File path: core/src/main/java/org/apache/cxf/validation/BeanValidationProvider.java
##########
@@ -111,46 +142,81 @@ private static void initFactoryConfig(Configuration<?> factoryCfg, ValidationCon
     }
 
     public< T > void validateParameters(final T instance, final Method method, final Object[] arguments) {
-
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set< ConstraintViolation< T > > violations = methodValidator.validateParameters(instance,
-            method, arguments);
-
-        if (!violations.isEmpty()) {
-            throw new ConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateParameters(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateParameters(instance,
+                    method, arguments);
+            if (!violations.isEmpty()) {
+                throw new ConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T instance, final Method method, final Object returnValue) {
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set<ConstraintViolation< T > > violations = methodValidator.validateReturnValue(instance,
-            method, returnValue);
-
-        if (!violations.isEmpty()) {
-            throw new ResponseConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateReturnedValue(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateReturnValue(instance,
+                    method, returnValue);
+            if (!violations.isEmpty()) {
+                throw new ResponseConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T bean) {
-        final Set<ConstraintViolation< T > > violations = doValidateBean(bean);
+        Validator validator = factory.get();
+        if (runtimeCache != null && bean != null
+                && !runtimeCache.shouldValidateBean(validator, bean.getClass())) {

Review comment:
       Oh nevermind, there is no method in this call

##########
File path: core/src/main/java/org/apache/cxf/validation/BeanValidationProvider.java
##########
@@ -111,46 +142,81 @@ private static void initFactoryConfig(Configuration<?> factoryCfg, ValidationCon
     }
 
     public< T > void validateParameters(final T instance, final Method method, final Object[] arguments) {
-
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set< ConstraintViolation< T > > violations = methodValidator.validateParameters(instance,
-            method, arguments);
-
-        if (!violations.isEmpty()) {
-            throw new ConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateParameters(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateParameters(instance,
+                    method, arguments);
+            if (!violations.isEmpty()) {
+                throw new ConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T instance, final Method method, final Object returnValue) {
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set<ConstraintViolation< T > > violations = methodValidator.validateReturnValue(instance,
-            method, returnValue);
-
-        if (!violations.isEmpty()) {
-            throw new ResponseConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateReturnedValue(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateReturnValue(instance,
+                    method, returnValue);
+            if (!violations.isEmpty()) {
+                throw new ResponseConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T bean) {
-        final Set<ConstraintViolation< T > > violations = doValidateBean(bean);
+        Validator validator = factory.get();
+        if (runtimeCache != null && bean != null
+                && !runtimeCache.shouldValidateBean(validator, bean.getClass())) {
+            return;
+        }
+        final Set<ConstraintViolation< T > > violations = doValidateBean(validator, bean);
         if (!violations.isEmpty()) {
             throw new ResponseConstraintViolationException(violations);
         }
     }
 
     public< T > void validateBean(final T bean) {
-        final Set<ConstraintViolation< T > > violations = doValidateBean(bean);
+        final Set<ConstraintViolation< T > > violations = doValidateBean(factory.get(), bean);
         if (!violations.isEmpty()) {
             throw new ConstraintViolationException(violations);
         }
     }
 
-    private< T > Set<ConstraintViolation< T > > doValidateBean(final T bean) {
-        return factory.getValidator().validate(bean);
+    private< T > Set<ConstraintViolation< T > > doValidateBean(final Validator validator, final T bean) {
+        if (validator.getConstraintsForClass(bean.getClass()).isBeanConstrained()) {
+            return validator.validate(bean);
+        }
+        return emptySet();
+    }
+
+    @Override
+    public void close() {
+        close.run();
     }
 
-    private ExecutableValidator getExecutableValidator() {
+    // only created when there is a single validator/factory so it is safe to cache
+    // note: the validator is passed as param to avoid to create useless ones
+    private static class RuntimeCache {
+        private final ConcurrentMap<Class<?>, Boolean> types = new ConcurrentHashMap<>();
+        private final ConcurrentMap<Method, Boolean> params = new ConcurrentHashMap<>();
+        private final ConcurrentMap<Method, Boolean> returnedValues = new ConcurrentHashMap<>();
+
+        public boolean shouldValidateParameters(final Validator validator, final Method method) {
+            return params.computeIfAbsent(method, m -> validator.getConstraintsForClass(m.getDeclaringClass())
+                    .getConstraintsForMethod(m.getName(), m.getParameterTypes())
+                    .hasConstrainedParameters());
+        }
 
-        return factory.getValidator().forExecutables();
+        public boolean shouldValidateReturnedValue(final Validator validator, final Method method) {
+            return returnedValues.computeIfAbsent(method, m -> validator.getConstraintsForClass(m.getDeclaringClass())
+                    .getConstraintsForMethod(m.getName(), method.getParameterTypes())
+                    .hasConstrainedReturnValue());
+        }
+
+        public boolean shouldValidateBean(final Validator validator, final Class<?> clazz) {
+            return types.computeIfAbsent(clazz, it -> validator.getConstraintsForClass(it).hasConstraints());

Review comment:
       Should you also check for `isBeanConstrained` and cache that?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] rmannibucau commented on a change in pull request #695: Enable to pass a bval Validator to BeanValidationProvider (it is more…

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on a change in pull request #695:
URL: https://github.com/apache/cxf/pull/695#discussion_r491800652



##########
File path: core/src/main/java/org/apache/cxf/validation/BeanValidationProvider.java
##########
@@ -111,46 +142,81 @@ private static void initFactoryConfig(Configuration<?> factoryCfg, ValidationCon
     }
 
     public< T > void validateParameters(final T instance, final Method method, final Object[] arguments) {
-
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set< ConstraintViolation< T > > violations = methodValidator.validateParameters(instance,
-            method, arguments);
-
-        if (!violations.isEmpty()) {
-            throw new ConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateParameters(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateParameters(instance,
+                    method, arguments);
+            if (!violations.isEmpty()) {
+                throw new ConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T instance, final Method method, final Object returnValue) {
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set<ConstraintViolation< T > > violations = methodValidator.validateReturnValue(instance,
-            method, returnValue);
-
-        if (!violations.isEmpty()) {
-            throw new ResponseConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateReturnedValue(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateReturnValue(instance,
+                    method, returnValue);
+            if (!violations.isEmpty()) {
+                throw new ResponseConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T bean) {
-        final Set<ConstraintViolation< T > > violations = doValidateBean(bean);
+        Validator validator = factory.get();
+        if (runtimeCache != null && bean != null
+                && !runtimeCache.shouldValidateBean(validator, bean.getClass())) {
+            return;
+        }
+        final Set<ConstraintViolation< T > > violations = doValidateBean(validator, bean);
         if (!violations.isEmpty()) {
             throw new ResponseConstraintViolationException(violations);
         }
     }
 
     public< T > void validateBean(final T bean) {
-        final Set<ConstraintViolation< T > > violations = doValidateBean(bean);
+        final Set<ConstraintViolation< T > > violations = doValidateBean(factory.get(), bean);
         if (!violations.isEmpty()) {
             throw new ConstraintViolationException(violations);
         }
     }
 
-    private< T > Set<ConstraintViolation< T > > doValidateBean(final T bean) {
-        return factory.getValidator().validate(bean);
+    private< T > Set<ConstraintViolation< T > > doValidateBean(final Validator validator, final T bean) {
+        if (validator.getConstraintsForClass(bean.getClass()).isBeanConstrained()) {

Review comment:
       think this call is always protected by a public call before which already have the exit condition - at least was the intent




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] rmannibucau commented on pull request #695: Enable to pass a bval Validator to BeanValidationProvider (it is more…

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #695:
URL: https://github.com/apache/cxf/pull/695#issuecomment-713317176


   @reta good to merge for me
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta commented on a change in pull request #695: Enable to pass a bval Validator to BeanValidationProvider (it is more…

Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #695:
URL: https://github.com/apache/cxf/pull/695#discussion_r491763895



##########
File path: core/src/main/java/org/apache/cxf/validation/BeanValidationProvider.java
##########
@@ -111,46 +142,81 @@ private static void initFactoryConfig(Configuration<?> factoryCfg, ValidationCon
     }
 
     public< T > void validateParameters(final T instance, final Method method, final Object[] arguments) {
-
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set< ConstraintViolation< T > > violations = methodValidator.validateParameters(instance,
-            method, arguments);
-
-        if (!violations.isEmpty()) {
-            throw new ConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateParameters(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateParameters(instance,
+                    method, arguments);
+            if (!violations.isEmpty()) {
+                throw new ConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T instance, final Method method, final Object returnValue) {
-        final ExecutableValidator methodValidator = getExecutableValidator();
-        final Set<ConstraintViolation< T > > violations = methodValidator.validateReturnValue(instance,
-            method, returnValue);
-
-        if (!violations.isEmpty()) {
-            throw new ResponseConstraintViolationException(violations);
+        final Validator validator = factory.get();
+        final ExecutableValidator methodValidator = validator.forExecutables();
+        if (runtimeCache == null || runtimeCache.shouldValidateReturnedValue(validator, method)) {
+            final Set<ConstraintViolation<T>> violations = methodValidator.validateReturnValue(instance,
+                    method, returnValue);
+            if (!violations.isEmpty()) {
+                throw new ResponseConstraintViolationException(violations);
+            }
         }
     }
 
     public< T > void validateReturnValue(final T bean) {
-        final Set<ConstraintViolation< T > > violations = doValidateBean(bean);
+        Validator validator = factory.get();
+        if (runtimeCache != null && bean != null
+                && !runtimeCache.shouldValidateBean(validator, bean.getClass())) {

Review comment:
       Oh nevermind, there is no method in this call




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org