From 39e70a9499c89dd24fa453212eced860afe50d50 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Thu, 20 Jul 2017 13:55:02 +0200 Subject: [PATCH] HV-1439 Have a specific method to check that cascading should be performed on container elements: there is no need to execute part of the code if only the annotated object is cascaded. --- .../internal/engine/ValidatorImpl.java | 14 +++++++++----- .../aggregated/CascadingMetaDataBuilder.java | 18 +++++++++++------- .../aggregated/ContainerCascadingMetaData.java | 16 ++++++++++------ 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorImpl.java b/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorImpl.java index a9d97024f0..3e8db6a14b 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorImpl.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorImpl.java @@ -583,10 +583,14 @@ private void validateCascadedConstraints(ValidationContext validationContext, validateCascadedAnnotatedObjectForCurrentGroup( value, validationContext, valueContext, cascadingMetaData ); } - if ( cascadingMetaData.isContainer() && cascadingMetaData.isMarkedForCascadingOnAnnotatedObjectOrContainerElements() ) { - // validate cascading on the container elements - validateCascadedContainerElementsForCurrentGroup( value, validationContext, valueContext, - cascadingMetaData.as( ContainerCascadingMetaData.class ).getContainerElementTypesCascadingMetaData() ); + if ( cascadingMetaData.isContainer() ) { + ContainerCascadingMetaData containerCascadingMetaData = cascadingMetaData.as( ContainerCascadingMetaData.class ); + + if ( containerCascadingMetaData.hasContainerElementsMarkedForCascading() ) { + // validate cascading on the container elements + validateCascadedContainerElementsForCurrentGroup( value, validationContext, valueContext, + containerCascadingMetaData.getContainerElementTypesCascadingMetaData() ); + } } } } @@ -701,7 +705,7 @@ private void doValidate(Object value, String nodeName) { } // Cascade validation to container elements if we are dealing with a container element - if ( cascadingMetaData.isMarkedForCascadingOnAnnotatedObjectOrContainerElements() ) { + if ( cascadingMetaData.hasContainerElementsMarkedForCascading() ) { ValueContext cascadedTypeArgumentValueContext = buildNewLocalExecutionContext( valueContext, value ); if ( cascadingMetaData.getTypeParameter() != null ) { cascadedValueContext.setTypeParameter( cascadingMetaData.getDeclaredContainerClass(), cascadingMetaData.getDeclaredTypeParameter() ); diff --git a/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/CascadingMetaDataBuilder.java b/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/CascadingMetaDataBuilder.java index c814de1cb3..a1b176361d 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/CascadingMetaDataBuilder.java +++ b/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/CascadingMetaDataBuilder.java @@ -83,9 +83,9 @@ public class CascadingMetaDataBuilder { private final Map, Class> groupConversions; /** - * Whether the constrained element is directly or indirectly (via type arguments) marked for cascaded validation. + * Whether any container element (it can be nested) is marked for cascaded validation. */ - private final boolean markedForCascadingOnAnnotatedObjectOrContainerElements; + private final boolean hasContainerElementsMarkedForCascading; /** * Whether the constrained element has directly or indirectly (via type arguments) group conversions defined. @@ -110,15 +110,15 @@ private CascadingMetaDataBuilder(Type enclosingType, TypeVariable typeParamet this.groupConversions = CollectionHelper.toImmutableMap( groupConversions ); this.containerElementTypesCascadingMetaData = CollectionHelper.toImmutableMap( containerElementTypesCascadingMetaData ); - boolean tmpMarkedForCascadingOnAnnotatedObjectOrContainerElements = cascading; + boolean tmpHasContainerElementsMarkedForCascading = false; boolean tmpHasGroupConversionsOnAnnotatedObjectOrContainerElements = !groupConversions.isEmpty(); for ( CascadingMetaDataBuilder nestedCascadingTypeParameter : containerElementTypesCascadingMetaData.values() ) { - tmpMarkedForCascadingOnAnnotatedObjectOrContainerElements = tmpMarkedForCascadingOnAnnotatedObjectOrContainerElements - || nestedCascadingTypeParameter.markedForCascadingOnAnnotatedObjectOrContainerElements; + tmpHasContainerElementsMarkedForCascading = tmpHasContainerElementsMarkedForCascading + || nestedCascadingTypeParameter.cascading || nestedCascadingTypeParameter.hasContainerElementsMarkedForCascading; tmpHasGroupConversionsOnAnnotatedObjectOrContainerElements = tmpHasGroupConversionsOnAnnotatedObjectOrContainerElements || nestedCascadingTypeParameter.hasGroupConversionsOnAnnotatedObjectOrContainerElements; } - markedForCascadingOnAnnotatedObjectOrContainerElements = tmpMarkedForCascadingOnAnnotatedObjectOrContainerElements; + hasContainerElementsMarkedForCascading = tmpHasContainerElementsMarkedForCascading; hasGroupConversionsOnAnnotatedObjectOrContainerElements = tmpHasGroupConversionsOnAnnotatedObjectOrContainerElements; } @@ -170,8 +170,12 @@ public Map, Class> getGroupConversions() { return groupConversions; } + public boolean hasContainerElementsMarkedForCascading() { + return hasContainerElementsMarkedForCascading; + } + public boolean isMarkedForCascadingOnAnnotatedObjectOrContainerElements() { - return markedForCascadingOnAnnotatedObjectOrContainerElements; + return cascading || hasContainerElementsMarkedForCascading; } public boolean hasGroupConversionsOnAnnotatedObjectOrContainerElements() { diff --git a/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/ContainerCascadingMetaData.java b/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/ContainerCascadingMetaData.java index b5eeede7fa..d8246630e7 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/ContainerCascadingMetaData.java +++ b/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/ContainerCascadingMetaData.java @@ -73,9 +73,9 @@ public class ContainerCascadingMetaData implements CascadingMetaData { private GroupConversionHelper groupConversionHelper; /** - * Whether the constrained element is directly or indirectly (via type arguments) marked for cascaded validation. + * Whether any container element (it can be nested) is marked for cascaded validation. */ - private final boolean markedForCascadingOnAnnotatedObjectOrContainerElements; + private final boolean hasContainerElementsMarkedForCascading; /** * The set of value extractors which are type compliant and container element compliant with the element where @@ -107,7 +107,7 @@ private ContainerCascadingMetaData(ValueExtractorManager valueExtractorManager, private ContainerCascadingMetaData(ValueExtractorManager valueExtractorManager, Type enclosingType, TypeVariable typeParameter, Class declaredContainerClass, TypeVariable declaredTypeParameter, List containerElementTypesCascadingMetaData, - boolean cascading, GroupConversionHelper groupConversionHelper, boolean markedForCascadingOnElementOrContainerElements) { + boolean cascading, GroupConversionHelper groupConversionHelper, boolean markedForCascadingOnContainerElements) { this.enclosingType = enclosingType; this.typeParameter = typeParameter; this.declaredContainerClass = declaredContainerClass; @@ -115,9 +115,9 @@ private ContainerCascadingMetaData(ValueExtractorManager valueExtractorManager, this.containerElementTypesCascadingMetaData = containerElementTypesCascadingMetaData; this.cascading = cascading; this.groupConversionHelper = groupConversionHelper; - this.markedForCascadingOnAnnotatedObjectOrContainerElements = markedForCascadingOnElementOrContainerElements; + this.hasContainerElementsMarkedForCascading = markedForCascadingOnContainerElements; - if ( TypeVariables.isAnnotatedObject( this.typeParameter ) || !markedForCascadingOnElementOrContainerElements ) { + if ( TypeVariables.isAnnotatedObject( this.typeParameter ) || !markedForCascadingOnContainerElements ) { this.valueExtractorCandidates = Collections.emptySet(); } else { @@ -158,9 +158,13 @@ public boolean isCascading() { return cascading; } + public boolean hasContainerElementsMarkedForCascading() { + return hasContainerElementsMarkedForCascading; + } + @Override public boolean isMarkedForCascadingOnAnnotatedObjectOrContainerElements() { - return markedForCascadingOnAnnotatedObjectOrContainerElements; + return cascading || hasContainerElementsMarkedForCascading; } public List getContainerElementTypesCascadingMetaData() {