Skip to content

Commit 552f877

Browse files
wytzevanderploegbeikov
wytzevanderploeg
authored and
beikov
committed
HHH-18724 Take constraint composition type into account when determining nullable state of a Column
1 parent 34bbe8e commit 552f877

File tree

10 files changed

+511
-40
lines changed

10 files changed

+511
-40
lines changed

‎hibernate-core/src/main/java/org/hibernate/boot/beanvalidation/TypeSafeActivator.java‎

Lines changed: 113 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55
package org.hibernate.boot.beanvalidation;
66

77
import java.lang.annotation.Annotation;
8+
import java.lang.reflect.InvocationTargetException;
9+
import java.lang.reflect.Method;
810
import java.lang.invoke.MethodHandles;
911
import java.util.Collection;
12+
import java.util.HashMap;
1013
import java.util.HashSet;
1114
import java.util.Locale;
1215
import java.util.Map;
@@ -187,12 +190,23 @@ public static void applyRelationalConstraints(
187190
final Class<?>[] groupsArray =
188191
buildGroupsForOperation( GroupsPerOperation.Operation.DDL, settings, classLoaderAccess );
189192
final Set<Class<?>> groups = new HashSet<>( asList( groupsArray ) );
193+
final Map<Class<? extends Annotation>, Boolean> constraintCompositionTypeCache = new HashMap<>();
194+
190195
for ( PersistentClass persistentClass : persistentClasses ) {
191196
final String className = persistentClass.getClassName();
192197
if ( isNotEmpty( className ) ) {
193198
final Class<?> clazz = entityClass( classLoaderAccess, className );
194199
try {
195-
applyDDL( "", persistentClass, clazz, factory, groups, true, dialect );
200+
applyDDL(
201+
"",
202+
persistentClass,
203+
clazz,
204+
factory,
205+
groups,
206+
true,
207+
dialect,
208+
constraintCompositionTypeCache
209+
);
196210
}
197211
catch (Exception e) {
198212
LOG.unableToApplyConstraints( className, e );
@@ -217,7 +231,9 @@ private static void applyDDL(
217231
ValidatorFactory factory,
218232
Set<Class<?>> groups,
219233
boolean activateNotNull,
220-
Dialect dialect) {
234+
Dialect dialect,
235+
Map<Class<? extends Annotation>, Boolean> constraintCompositionTypeCache
236+
) {
221237
final BeanDescriptor descriptor = factory.getValidator().getConstraintsForClass( clazz );
222238
//cno bean level constraints can be applied, go to the properties
223239
for ( PropertyDescriptor propertyDesc : descriptor.getConstrainedProperties() ) {
@@ -230,7 +246,9 @@ private static void applyDDL(
230246
propertyDesc,
231247
groups,
232248
activateNotNull,
233-
dialect
249+
false,
250+
dialect,
251+
constraintCompositionTypeCache
234252
);
235253
if ( property.isComposite() && propertyDesc.isCascaded() ) {
236254
final Component component = (Component) property.getValue();
@@ -244,7 +262,8 @@ private static void applyDDL(
244262
// activate not null and if the property is not null.
245263
// Otherwise, all sub columns should be left nullable
246264
activateNotNull && hasNotNull,
247-
dialect
265+
dialect,
266+
constraintCompositionTypeCache
248267
);
249268
}
250269
}
@@ -257,12 +276,18 @@ private static boolean applyConstraints(
257276
PropertyDescriptor propertyDesc,
258277
Set<Class<?>> groups,
259278
boolean canApplyNotNull,
260-
Dialect dialect) {
261-
boolean hasNotNull = false;
279+
boolean useOrLogicForComposedConstraint,
280+
Dialect dialect,
281+
Map<Class<? extends Annotation>, Boolean> constraintCompositionTypeCache) {
282+
283+
boolean firstItem = true;
284+
boolean composedResultHasNotNull = false;
262285
for ( ConstraintDescriptor<?> descriptor : constraintDescriptors ) {
286+
boolean hasNotNull = false;
287+
263288
if ( groups == null || !disjoint( descriptor.getGroups(), groups ) ) {
264289
if ( canApplyNotNull ) {
265-
hasNotNull = hasNotNull || applyNotNull( property, descriptor );
290+
hasNotNull = isNotNullDescriptor( descriptor );
266291
}
267292

268293
// apply bean validation specific constraints
@@ -276,19 +301,70 @@ private static boolean applyConstraints(
276301
// will be taken care later.
277302
applyLength( property, descriptor, propertyDesc );
278303

279-
// pass an empty set as composing constraints inherit the main constraint and thus are matching already
280-
final boolean hasNotNullFromComposingConstraints = applyConstraints(
281-
descriptor.getComposingConstraints(),
282-
property, propertyDesc, null,
283-
canApplyNotNull,
284-
dialect
285-
);
304+
// Composing constraints
305+
if ( !descriptor.getComposingConstraints().isEmpty() ) {
306+
// pass an empty set as composing constraints inherit the main constraint and thus are matching already
307+
final boolean hasNotNullFromComposingConstraints = applyConstraints(
308+
descriptor.getComposingConstraints(),
309+
property, propertyDesc, null,
310+
canApplyNotNull,
311+
isConstraintCompositionOfTypeOr( descriptor, constraintCompositionTypeCache ),
312+
dialect,
313+
constraintCompositionTypeCache
314+
);
315+
hasNotNull |= hasNotNullFromComposingConstraints;
316+
}
317+
}
286318

287-
hasNotNull = hasNotNull || hasNotNullFromComposingConstraints;
319+
if ( firstItem ) {
320+
composedResultHasNotNull = hasNotNull;
321+
firstItem = false;
288322
}
323+
else if ( !useOrLogicForComposedConstraint ) {
324+
// If the constraint composition is of type AND (default) then only ONE constraint needs to
325+
// be non-nullable for the property to be marked as 'not-null'.
326+
composedResultHasNotNull |= hasNotNull;
327+
}
328+
else {
329+
// If the constraint composition is of type OR then ALL constraints need to
330+
// be non-nullable for the property to be marked as 'not-null'.
331+
composedResultHasNotNull &= hasNotNull;
332+
}
333+
}
289334

335+
if ( composedResultHasNotNull ) {
336+
markNotNull( property );
290337
}
291-
return hasNotNull;
338+
339+
return composedResultHasNotNull;
340+
}
341+
342+
private static boolean isConstraintCompositionOfTypeOr(
343+
ConstraintDescriptor<?> descriptor,
344+
Map<Class<? extends Annotation>, Boolean> constraintCompositionTypeCache
345+
) {
346+
if ( descriptor.getComposingConstraints().size() < 2 ) {
347+
return false;
348+
}
349+
350+
final Class<? extends Annotation> composedAnnotation = descriptor.getAnnotation().annotationType();
351+
return constraintCompositionTypeCache.computeIfAbsent( composedAnnotation, value -> {
352+
for ( Annotation annotation : value.getAnnotations() ) {
353+
if ( "org.hibernate.validator.constraints.ConstraintComposition"
354+
.equals( annotation.annotationType().getName() ) ) {
355+
try {
356+
Method valueMethod = annotation.annotationType().getMethod( "value" );
357+
Object result = valueMethod.invoke( annotation );
358+
return result != null && "OR".equals( result.toString() );
359+
}
360+
catch ( NoSuchMethodException | IllegalAccessException | InvocationTargetException ex ) {
361+
LOG.debug( "ConstraintComposition type could not be determined. Assuming AND", ex );
362+
return false;
363+
}
364+
}
365+
}
366+
return false;
367+
});
292368
}
293369

294370
private static void applyMin(Property property, ConstraintDescriptor<?> descriptor, Dialect dialect) {
@@ -328,35 +404,33 @@ private static void applySQLCheck(Column column, String checkConstraint) {
328404
column.addCheckConstraint( new CheckConstraint( checkConstraint ) );
329405
}
330406

331-
private static boolean applyNotNull(Property property, ConstraintDescriptor<?> descriptor) {
332-
boolean hasNotNull = false;
333-
// NotNull, NotEmpty, and NotBlank annotation add not-null on column
407+
private static boolean isNotNullDescriptor(ConstraintDescriptor<?> descriptor) {
334408
final Class<? extends Annotation> annotationType = descriptor.getAnnotation().annotationType();
335-
if ( NotNull.class.equals(annotationType)
409+
return NotNull.class.equals(annotationType)
336410
|| NotEmpty.class.equals(annotationType)
337-
|| NotBlank.class.equals(annotationType)) {
338-
// single table inheritance should not be forced to null due to shared state
339-
if ( !( property.getPersistentClass() instanceof SingleTableSubclass ) ) {
340-
// composite should not add not-null on all columns
341-
if ( !property.isComposite() ) {
342-
for ( Selectable selectable : property.getSelectables() ) {
343-
if ( selectable instanceof Column column ) {
344-
column.setNullable( false );
345-
}
346-
else {
347-
LOG.debugf(
348-
"@NotNull was applied to attribute [%s] which is defined (at least partially) " +
349-
"by formula(s); formula portions will be skipped",
350-
property.getName()
351-
);
352-
}
411+
|| NotBlank.class.equals(annotationType);
412+
}
413+
414+
private static void markNotNull(Property property) {
415+
// single table inheritance should not be forced to null due to shared state
416+
if ( !( property.getPersistentClass() instanceof SingleTableSubclass ) ) {
417+
// composite should not add not-null on all columns
418+
if ( !property.isComposite() ) {
419+
for ( Selectable selectable : property.getSelectables() ) {
420+
if ( selectable instanceof Column column ) {
421+
column.setNullable( false );
422+
}
423+
else {
424+
LOG.debugf(
425+
"@NotNull was applied to attribute [%s] which is defined (at least partially) " +
426+
"by formula(s); formula portions will be skipped",
427+
property.getName()
428+
);
353429
}
354430
}
355431
}
356-
hasNotNull = true;
357432
}
358-
property.setOptional( !hasNotNull );
359-
return hasNotNull;
433+
property.setOptional( false );
360434
}
361435

362436
private static void applyDigits(Property property, ConstraintDescriptor<?> descriptor) {

‎hibernate-core/src/test/java/org/hibernate/orm/test/annotations/beanvalidation/Address.java‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,13 @@ public class Address {
1515

1616
private String line1;
1717
private String line2;
18+
private String line3;
19+
private String line4;
20+
private String line5;
21+
private String line6;
22+
private String line7;
23+
private String line8;
24+
private String line9;
1825
private String zip;
1926
private String state;
2027
@Size(max = 20)
@@ -52,6 +59,69 @@ public void setLine2(String line2) {
5259
this.line2 = line2;
5360
}
5461

62+
@CustomNullOrNotBlank
63+
public String getLine3() {
64+
return line3;
65+
}
66+
67+
public void setLine3(String line3) {
68+
this.line3 = line3;
69+
}
70+
71+
@CustomNotNullOrNotBlank
72+
public String getLine4() {
73+
return line4;
74+
}
75+
76+
public void setLine4(String line4) {
77+
this.line4 = line4;
78+
}
79+
80+
@CustomNullAndNotBlank
81+
public String getLine5() {
82+
return line5;
83+
}
84+
85+
public void setLine5(String line5) {
86+
this.line5 = line5;
87+
}
88+
89+
@CustomNotNullAndNotBlank
90+
public String getLine6() {
91+
return line6;
92+
}
93+
94+
public void setLine6(String line6) {
95+
this.line6 = line6;
96+
}
97+
98+
@CustomNullOrPattern
99+
public String getLine7() {
100+
return line7;
101+
}
102+
103+
public void setLine7(String line7) {
104+
this.line7 = line7;
105+
}
106+
107+
@CustomNotNull
108+
public String getLine8() {
109+
return line8;
110+
}
111+
112+
public void setLine8(String line8) {
113+
this.line8 = line8;
114+
}
115+
116+
@CustomNull
117+
public String getLine9() {
118+
return line9;
119+
}
120+
121+
public void setLine9(String line9) {
122+
this.line9 = line9;
123+
}
124+
55125
@Size(max = 3)
56126
@NotNull
57127
public String getState() {

‎hibernate-core/src/test/java/org/hibernate/orm/test/annotations/beanvalidation/CustomNotNull.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
* SPDX-License-Identifier: LGPL-2.1-or-later
3+
* Copyright Red Hat Inc. and Hibernate Authors
4+
*/
5+
package org.hibernate.orm.test.annotations.beanvalidation;
6+
7+
import jakarta.validation.Constraint;
8+
import jakarta.validation.Payload;
9+
import jakarta.validation.constraints.NotNull;
10+
11+
import java.lang.annotation.Documented;
12+
import java.lang.annotation.Retention;
13+
import java.lang.annotation.Target;
14+
15+
import static java.lang.annotation.ElementType.ANNOTATION_TYPE;
16+
import static java.lang.annotation.ElementType.CONSTRUCTOR;
17+
import static java.lang.annotation.ElementType.FIELD;
18+
import static java.lang.annotation.ElementType.METHOD;
19+
import static java.lang.annotation.ElementType.PARAMETER;
20+
import static java.lang.annotation.RetentionPolicy.RUNTIME;
21+
22+
/**
23+
* Used to test constraint composition.
24+
*/
25+
@NotNull
26+
@Target({METHOD, FIELD, ANNOTATION_TYPE, CONSTRUCTOR, PARAMETER})
27+
@Retention(RUNTIME)
28+
@Documented
29+
@Constraint(validatedBy = {})
30+
public @interface CustomNotNull {
31+
32+
String message() default "{org.hibernate.validator.constraints.CustomNotNull.message}";
33+
34+
Class<?>[] groups() default {};
35+
36+
Class<? extends Payload>[] payload() default {};
37+
38+
}

‎hibernate-core/src/test/java/org/hibernate/orm/test/annotations/beanvalidation/CustomNotNullAndNotBlank.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
* SPDX-License-Identifier: LGPL-2.1-or-later
3+
* Copyright Red Hat Inc. and Hibernate Authors
4+
*/
5+
package org.hibernate.orm.test.annotations.beanvalidation;
6+
7+
import jakarta.validation.Constraint;
8+
import jakarta.validation.Payload;
9+
import jakarta.validation.ReportAsSingleViolation;
10+
import jakarta.validation.constraints.NotBlank;
11+
import jakarta.validation.constraints.NotNull;
12+
import org.hibernate.validator.constraints.ConstraintComposition;
13+
14+
import java.lang.annotation.Documented;
15+
import java.lang.annotation.Retention;
16+
import java.lang.annotation.Target;
17+
18+
import static java.lang.annotation.ElementType.ANNOTATION_TYPE;
19+
import static java.lang.annotation.ElementType.CONSTRUCTOR;
20+
import static java.lang.annotation.ElementType.FIELD;
21+
import static java.lang.annotation.ElementType.METHOD;
22+
import static java.lang.annotation.ElementType.PARAMETER;
23+
import static java.lang.annotation.RetentionPolicy.RUNTIME;
24+
import static org.hibernate.validator.constraints.CompositionType.AND;
25+
26+
/**
27+
* Used to test constraint composition with AND for nullability of columns.
28+
*/
29+
@ConstraintComposition(AND)
30+
@NotNull
31+
@NotBlank
32+
@ReportAsSingleViolation
33+
@Target({METHOD, FIELD, ANNOTATION_TYPE, CONSTRUCTOR, PARAMETER})
34+
@Retention(RUNTIME)
35+
@Documented
36+
@Constraint(validatedBy = {})
37+
public @interface CustomNotNullAndNotBlank {
38+
39+
String message() default "{org.hibernate.validator.constraints.CustomNotNullAndNotBlank.message}";
40+
41+
Class<?>[] groups() default {};
42+
43+
Class<? extends Payload>[] payload() default {};
44+
45+
}

0 commit comments

Comments
 (0)