Skip to content

Commit 7e80431

Browse files
marko-bekhtagsmet
marko-bekhta
authored and
gsmet
committed
HV-1599 Changed when the violation is created in ConstraintTree
- delayed creating of violation when validating constraint in ConstraintTree - added a benchmark to SimpleValidation with composing constraint validation
1 parent 8f08f09 commit 7e80431

File tree

5 files changed

+184
-86
lines changed

5 files changed

+184
-86
lines changed

‎engine/src/main/java/org/hibernate/validator/internal/engine/ValidationContext.java

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import java.util.Map;
1919
import java.util.Optional;
2020
import java.util.Set;
21-
import java.util.stream.Collectors;
2221

2322
import javax.validation.ClockProvider;
2423
import javax.validation.ConstraintValidatorFactory;
@@ -33,7 +32,6 @@
3332

3433
import org.hibernate.validator.constraintvalidation.HibernateConstraintValidatorInitializationContext;
3534
import org.hibernate.validator.internal.engine.ValidatorFactoryImpl.ValidatorFactoryScopedContext;
36-
import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorContextImpl;
3735
import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorManager;
3836
import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintViolationCreationContext;
3937
import org.hibernate.validator.internal.engine.path.PathImpl;
@@ -272,14 +270,6 @@ public HibernateConstraintValidatorInitializationContext getConstraintValidatorI
272270
return constraintValidatorInitializationContext;
273271
}
274272

275-
public Set<ConstraintViolation<T>> createConstraintViolations(ValueContext<?, ?> localContext,
276-
ConstraintValidatorContextImpl constraintValidatorContext) {
277-
278-
return constraintValidatorContext.getConstraintViolationCreationContexts().stream()
279-
.map( c -> createConstraintViolation( localContext, c, constraintValidatorContext.getConstraintDescriptor() ) )
280-
.collect( Collectors.toSet() );
281-
}
282-
283273
public ConstraintValidatorFactory getConstraintValidatorFactory() {
284274
return constraintValidatorFactory;
285275
}
@@ -308,16 +298,19 @@ public void markCurrentBeanAsProcessed(ValueContext<?, ?> valueContext) {
308298
markCurrentBeanAsProcessedForCurrentPath( valueContext.getCurrentBean(), valueContext.getPropertyPath() );
309299
}
310300

311-
public void addConstraintFailures(Set<ConstraintViolation<T>> failingConstraintViolations) {
312-
this.failingConstraintViolations.addAll( failingConstraintViolations );
301+
public void addConstraintFailure(ConstraintViolation<T> failingConstraintViolation) {
302+
this.failingConstraintViolations.add( failingConstraintViolation );
313303
}
314304

315305
public Set<ConstraintViolation<T>> getFailingConstraints() {
316306
return failingConstraintViolations;
317307
}
318308

319-
320-
public ConstraintViolation<T> createConstraintViolation(ValueContext<?, ?> localContext, ConstraintViolationCreationContext constraintViolationCreationContext, ConstraintDescriptor<?> descriptor) {
309+
public ConstraintViolation<T> createConstraintViolation(
310+
ValueContext<?, ?> localContext,
311+
ConstraintViolationCreationContext constraintViolationCreationContext,
312+
ConstraintDescriptor<?> descriptor
313+
) {
321314
String messageTemplate = constraintViolationCreationContext.getMessage();
322315
String interpolatedMessage = interpolate(
323316
messageTemplate,

‎engine/src/main/java/org/hibernate/validator/internal/engine/constraintvalidation/ComposingConstraintTree.java

Lines changed: 45 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,17 @@
99
import static org.hibernate.validator.constraints.CompositionType.ALL_FALSE;
1010
import static org.hibernate.validator.constraints.CompositionType.AND;
1111
import static org.hibernate.validator.constraints.CompositionType.OR;
12-
import static org.hibernate.validator.internal.util.CollectionHelper.newHashSet;
1312

1413
import java.lang.annotation.Annotation;
1514
import java.lang.invoke.MethodHandles;
1615
import java.lang.reflect.Type;
17-
import java.util.Collections;
16+
import java.util.ArrayList;
17+
import java.util.Collection;
1818
import java.util.List;
19-
import java.util.Set;
19+
import java.util.Optional;
2020
import java.util.stream.Collectors;
2121

2222
import javax.validation.ConstraintValidator;
23-
import javax.validation.ConstraintViolation;
2423

2524
import org.hibernate.validator.constraints.CompositionType;
2625
import org.hibernate.validator.internal.engine.ValidationContext;
@@ -67,15 +66,15 @@ private <U extends Annotation> ConstraintTree<U> createConstraintTree(Constraint
6766
@Override
6867
protected <T> void validateConstraints(ValidationContext<T> validationContext,
6968
ValueContext<?, ?> valueContext,
70-
Set<ConstraintViolation<T>> constraintViolations) {
69+
Collection<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts) {
7170
CompositionResult compositionResult = validateComposingConstraints(
72-
validationContext, valueContext, constraintViolations
71+
validationContext, valueContext, violatedConstraintValidatorContexts
7372
);
7473

75-
Set<ConstraintViolation<T>> localViolations;
74+
Optional<ConstraintValidatorContextImpl> violatedLocalConstraintValidatorContext;
7675

7776
// After all children are validated the actual ConstraintValidator of the constraint itself is executed
78-
if ( mainConstraintNeedsEvaluation( validationContext, constraintViolations ) ) {
77+
if ( mainConstraintNeedsEvaluation( validationContext, violatedConstraintValidatorContexts ) ) {
7978

8079
if ( LOG.isTraceEnabled() ) {
8180
LOG.tracef(
@@ -98,41 +97,40 @@ protected <T> void validateConstraints(ValidationContext<T> validationContext,
9897
);
9998

10099
// validate
101-
localViolations = validateSingleConstraint(
102-
validationContext,
100+
violatedLocalConstraintValidatorContext = validateSingleConstraint(
103101
valueContext,
104102
constraintValidatorContext,
105103
validator
106104
);
107105

108106
// We re-evaluate the boolean composition by taking into consideration also the violations
109107
// from the local constraintValidator
110-
if ( localViolations.isEmpty() ) {
108+
if ( !violatedLocalConstraintValidatorContext.isPresent() ) {
111109
compositionResult.setAtLeastOneTrue( true );
112110
}
113111
else {
114112
compositionResult.setAllTrue( false );
115113
}
116114
}
117115
else {
118-
localViolations = Collections.emptySet();
116+
violatedLocalConstraintValidatorContext = Optional.empty();
119117
}
120118

121-
if ( !passesCompositionTypeRequirement( constraintViolations, compositionResult ) ) {
119+
if ( !passesCompositionTypeRequirement( violatedConstraintValidatorContexts, compositionResult ) ) {
122120
prepareFinalConstraintViolations(
123-
validationContext, valueContext, constraintViolations, localViolations
121+
validationContext, valueContext, violatedConstraintValidatorContexts, violatedLocalConstraintValidatorContext
124122
);
125123
}
126124
}
127125

128-
private <T> boolean mainConstraintNeedsEvaluation(ValidationContext<T> executionContext,
129-
Set<ConstraintViolation<T>> constraintViolations) {
126+
private <T> boolean mainConstraintNeedsEvaluation(ValidationContext<T> validationContext,
127+
Collection<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts) {
130128
// we are dealing with a composing constraint with no validator for the main constraint
131129
if ( !descriptor.getComposingConstraints().isEmpty() && descriptor.getMatchingConstraintValidatorDescriptors().isEmpty() ) {
132130
return false;
133131
}
134132

135-
if ( constraintViolations.isEmpty() ) {
133+
if ( violatedConstraintValidatorContexts.isEmpty() ) {
136134
return true;
137135
}
138136

@@ -142,7 +140,7 @@ private <T> boolean mainConstraintNeedsEvaluation(ValidationContext<T> execution
142140
}
143141

144142
// explicit fail fast mode
145-
if ( executionContext.isFailFastModeEnabled() ) {
143+
if ( validationContext.isFailFastModeEnabled() ) {
146144
return false;
147145
}
148146

@@ -153,33 +151,32 @@ private <T> boolean mainConstraintNeedsEvaluation(ValidationContext<T> execution
153151
* Before the final constraint violations can be reported back we need to check whether we have a composing
154152
* constraint whose result should be reported as single violation.
155153
*
156-
* @param executionContext meta data about top level validation
154+
* @param validationContext meta data about top level validation
157155
* @param valueContext meta data for currently validated value
158-
* @param constraintViolations used to accumulate constraint violations
159-
* @param localViolations set of constraint violations of top level constraint
156+
* @param violatedConstraintValidatorContexts used to accumulate constraint validator contexts that cause constraint violations
157+
* @param localConstraintValidatorContext an optional of constraint violations of top level constraint
158+
*
160159
*/
161-
private <T> void prepareFinalConstraintViolations(ValidationContext<T> executionContext,
160+
private <T> void prepareFinalConstraintViolations(ValidationContext<T> validationContext,
162161
ValueContext<?, ?> valueContext,
163-
Set<ConstraintViolation<T>> constraintViolations,
164-
Set<ConstraintViolation<T>> localViolations) {
162+
Collection<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts,
163+
Optional<ConstraintValidatorContextImpl> localConstraintValidatorContext) {
165164
if ( reportAsSingleViolation() ) {
166165
// We clear the current violations list anyway
167-
constraintViolations.clear();
166+
violatedConstraintValidatorContexts.clear();
168167

169168
// But then we need to distinguish whether the local ConstraintValidator has reported
170169
// violations or not (or if there is no local ConstraintValidator at all).
171170
// If not we create a violation
172171
// using the error message in the annotation declaration at top level.
173-
if ( localViolations.isEmpty() ) {
174-
final String message = getDescriptor().getMessageTemplate();
175-
ConstraintViolationCreationContext constraintViolationCreationContext = new ConstraintViolationCreationContext(
176-
message,
177-
valueContext.getPropertyPath()
178-
);
179-
ConstraintViolation<T> violation = executionContext.createConstraintViolation(
180-
valueContext, constraintViolationCreationContext, descriptor
181-
);
182-
constraintViolations.add( violation );
172+
if ( !localConstraintValidatorContext.isPresent() ) {
173+
violatedConstraintValidatorContexts.add( new ConstraintValidatorContextImpl(
174+
validationContext.getParameterNames(),
175+
validationContext.getClockProvider(),
176+
valueContext.getPropertyPath(),
177+
descriptor,
178+
validationContext.getConstraintValidatorPayload()
179+
) );
183180
}
184181
}
185182

@@ -191,28 +188,30 @@ private <T> void prepareFinalConstraintViolations(ValidationContext<T> execution
191188
// as checked in test CustomErrorMessage.java
192189
// If no violations have been reported from the local ConstraintValidator, or no such validator exists,
193190
// then we just add an empty list.
194-
constraintViolations.addAll( localViolations );
191+
if ( localConstraintValidatorContext.isPresent() ) {
192+
violatedConstraintValidatorContexts.add( localConstraintValidatorContext.get() );
193+
}
195194
}
196195

197196
/**
198197
* Validates all composing constraints recursively.
199198
*
200-
* @param executionContext Meta data about top level validation
199+
* @param validationContext Meta data about top level validation
201200
* @param valueContext Meta data for currently validated value
202-
* @param constraintViolations Used to accumulate constraint violations
201+
* @param violatedConstraintValidatorContexts Used to accumulate constraint validator contexts that cause constraint violations
203202
*
204203
* @return Returns an instance of {@code CompositionResult} relevant for boolean composition of constraints
205204
*/
206-
private <T> CompositionResult validateComposingConstraints(ValidationContext<T> executionContext,
205+
private <T> CompositionResult validateComposingConstraints(ValidationContext<T> validationContext,
207206
ValueContext<?, ?> valueContext,
208-
Set<ConstraintViolation<T>> constraintViolations) {
207+
Collection<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts) {
209208
CompositionResult compositionResult = new CompositionResult( true, false );
210209
for ( ConstraintTree<?> tree : children ) {
211-
Set<ConstraintViolation<T>> tmpViolations = newHashSet( 5 );
212-
tree.validateConstraints( executionContext, valueContext, tmpViolations );
213-
constraintViolations.addAll( tmpViolations );
210+
List<ConstraintValidatorContextImpl> tmpConstraintValidatorContexts = new ArrayList<>( 5 );
211+
tree.validateConstraints( validationContext, valueContext, tmpConstraintValidatorContexts );
212+
violatedConstraintValidatorContexts.addAll( tmpConstraintValidatorContexts );
214213

215-
if ( tmpViolations.isEmpty() ) {
214+
if ( tmpConstraintValidatorContexts.isEmpty() ) {
216215
compositionResult.setAtLeastOneTrue( true );
217216
// no need to further validate constraints, because at least one validation passed
218217
if ( descriptor.getCompositionType() == OR ) {
@@ -222,15 +221,15 @@ private <T> CompositionResult validateComposingConstraints(ValidationContext<T>
222221
else {
223222
compositionResult.setAllTrue( false );
224223
if ( descriptor.getCompositionType() == AND
225-
&& ( executionContext.isFailFastModeEnabled() || descriptor.isReportAsSingleViolation() ) ) {
224+
&& ( validationContext.isFailFastModeEnabled() || descriptor.isReportAsSingleViolation() ) ) {
226225
break;
227226
}
228227
}
229228
}
230229
return compositionResult;
231230
}
232231

233-
private boolean passesCompositionTypeRequirement(Set<?> constraintViolations, CompositionResult compositionResult) {
232+
private boolean passesCompositionTypeRequirement(Collection<?> constraintViolations, CompositionResult compositionResult) {
234233
CompositionType compositionType = getDescriptor().getCompositionType();
235234
boolean passedValidation = false;
236235
switch ( compositionType ) {

‎engine/src/main/java/org/hibernate/validator/internal/engine/constraintvalidation/ConstraintTree.java

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,17 @@
77
package org.hibernate.validator.internal.engine.constraintvalidation;
88

99
import static org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorManager.DUMMY_CONSTRAINT_VALIDATOR;
10-
import static org.hibernate.validator.internal.util.CollectionHelper.newHashSet;
1110

1211
import java.lang.annotation.Annotation;
1312
import java.lang.invoke.MethodHandles;
1413
import java.lang.reflect.Type;
15-
import java.util.Collections;
16-
import java.util.Set;
14+
import java.util.ArrayList;
15+
import java.util.Collection;
16+
import java.util.List;
17+
import java.util.Optional;
1718

1819
import javax.validation.ConstraintDeclarationException;
1920
import javax.validation.ConstraintValidator;
20-
import javax.validation.ConstraintViolation;
2121
import javax.validation.ValidationException;
2222

2323
import org.hibernate.validator.internal.engine.ValidationContext;
@@ -68,17 +68,23 @@ public static <U extends Annotation> ConstraintTree<U> of(ConstraintDescriptorIm
6868
}
6969
}
7070

71-
public final <T> boolean validateConstraints(ValidationContext<T> executionContext, ValueContext<?, ?> valueContext) {
72-
Set<ConstraintViolation<T>> constraintViolations = newHashSet( 5 );
73-
validateConstraints( executionContext, valueContext, constraintViolations );
74-
if ( !constraintViolations.isEmpty() ) {
75-
executionContext.addConstraintFailures( constraintViolations );
71+
public final <T> boolean validateConstraints(ValidationContext<T> validationContext, ValueContext<?, ?> valueContext) {
72+
List<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts = new ArrayList( 5 );
73+
validateConstraints( validationContext, valueContext, violatedConstraintValidatorContexts );
74+
if ( !violatedConstraintValidatorContexts.isEmpty() ) {
75+
for ( ConstraintValidatorContextImpl constraintValidatorContext : violatedConstraintValidatorContexts ) {
76+
for ( ConstraintViolationCreationContext constraintViolationCreationContext : constraintValidatorContext.getConstraintViolationCreationContexts() ) {
77+
validationContext.addConstraintFailure(
78+
validationContext.createConstraintViolation( valueContext, constraintViolationCreationContext, ( (ConstraintValidatorContextImpl) constraintValidatorContext ).getConstraintDescriptor() )
79+
);
80+
}
81+
}
7682
return false;
7783
}
7884
return true;
7985
}
8086

81-
protected abstract <T> void validateConstraints(ValidationContext<T> executionContext, ValueContext<?, ?> valueContext, Set<ConstraintViolation<T>> constraintViolations);
87+
protected abstract <T> void validateConstraints(ValidationContext<T> executionContext, ValueContext<?, ?> valueContext, Collection<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts);
8288

8389
public final ConstraintDescriptorImpl<A> getDescriptor() {
8490
return descriptor;
@@ -160,7 +166,11 @@ private ValidationException getExceptionForNullValidator(Type validatedValueType
160166
}
161167
}
162168

163-
protected final <T, V> Set<ConstraintViolation<T>> validateSingleConstraint(ValidationContext<T> executionContext,
169+
/**
170+
* @return an {@link Optional#empty()} if there is no violation or a corresponding {@link ConstraintValidatorContextImpl}
171+
* otherwise.
172+
*/
173+
protected final <V> Optional<ConstraintValidatorContextImpl> validateSingleConstraint(
164174
ValueContext<?, ?> valueContext,
165175
ConstraintValidatorContextImpl constraintValidatorContext,
166176
ConstraintValidator<A, V> validator) {
@@ -179,11 +189,9 @@ protected final <T, V> Set<ConstraintViolation<T>> validateSingleConstraint(Vali
179189
if ( !isValid ) {
180190
//We do not add these violations yet, since we don't know how they are
181191
//going to influence the final boolean evaluation
182-
return executionContext.createConstraintViolations(
183-
valueContext, constraintValidatorContext
184-
);
192+
return Optional.of( constraintValidatorContext );
185193
}
186-
return Collections.emptySet();
194+
return Optional.empty();
187195
}
188196

189197
@Override

‎engine/src/main/java/org/hibernate/validator/internal/engine/constraintvalidation/SimpleConstraintTree.java

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,9 @@
99
import java.lang.annotation.Annotation;
1010
import java.lang.invoke.MethodHandles;
1111
import java.lang.reflect.Type;
12-
import java.util.Set;
12+
import java.util.Collection;
1313

1414
import javax.validation.ConstraintValidator;
15-
import javax.validation.ConstraintViolation;
1615

1716
import org.hibernate.validator.internal.engine.ValidationContext;
1817
import org.hibernate.validator.internal.engine.ValueContext;
@@ -41,7 +40,7 @@ public SimpleConstraintTree(ConstraintDescriptorImpl<B> descriptor, Type validat
4140
@Override
4241
protected <T> void validateConstraints(ValidationContext<T> validationContext,
4342
ValueContext<?, ?> valueContext,
44-
Set<ConstraintViolation<T>> constraintViolations) {
43+
Collection<ConstraintValidatorContextImpl> violatedConstraintValidatorContexts) {
4544

4645
if ( LOG.isTraceEnabled() ) {
4746
LOG.tracef(
@@ -64,13 +63,8 @@ protected <T> void validateConstraints(ValidationContext<T> validationContext,
6463
);
6564

6665
// validate
67-
constraintViolations.addAll(
68-
validateSingleConstraint(
69-
validationContext,
70-
valueContext,
71-
constraintValidatorContext,
72-
validator
73-
)
74-
);
66+
if ( validateSingleConstraint( valueContext, constraintValidatorContext, validator ).isPresent() ) {
67+
violatedConstraintValidatorContexts.add( constraintValidatorContext );
68+
}
7569
}
7670
}

0 commit comments

Comments
 (0)