Skip to content

Commit

Permalink
HHH-18724 Take constraint composition type into account when determin…
Browse files Browse the repository at this point in the history
…ing nullable state of a Column
  • Loading branch information
wytzevanderploeg committed Oct 15, 2024
1 parent d702496 commit bafc6a8
Show file tree
Hide file tree
Showing 5 changed files with 167 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,18 @@
*/
package org.hibernate.boot.beanvalidation;

import java.lang.annotation.Annotation;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.StringTokenizer;
import java.util.function.BiFunction;

import jakarta.validation.constraints.*;
import org.hibernate.AssertionFailure;
Expand Down Expand Up @@ -206,6 +209,7 @@ private static void applyDDL(
propertyDesc,
groups,
activateNotNull,
false,
dialect
);
if ( property.isComposite() && propertyDesc.isCascaded() ) {
Expand Down Expand Up @@ -236,15 +240,23 @@ private static boolean applyConstraints(
PropertyDescriptor propertyDesc,
Set<Class<?>> groups,
boolean canApplyNotNull,
boolean useOrLogicForComposedConstraint,
Dialect dialect) {
boolean hasNotNull = false;
for ( ConstraintDescriptor<?> descriptor : constraintDescriptors ) {
// If the composition logic is of type OR, then all the nested constraints need to be not-null in order
// for the property to be marked not-null.
// If the composition logic is of type AND (default), then only one needs to be not-null.
BiFunction<Boolean, Boolean, Boolean> compositionLogic = useOrLogicForComposedConstraint ?
Boolean::logicalAnd :
Boolean::logicalOr;

if ( groups != null && Collections.disjoint( descriptor.getGroups(), groups ) ) {
continue;
}

if ( canApplyNotNull ) {
hasNotNull = hasNotNull || applyNotNull( property, descriptor );
hasNotNull = compositionLogic.apply( hasNotNull, isNotNullDescriptor( descriptor ) );
}

// apply bean validation specific constraints
Expand All @@ -263,14 +275,40 @@ private static boolean applyConstraints(
descriptor.getComposingConstraints(),
property, propertyDesc, null,
canApplyNotNull,
isConstraintCompositionOfTypeOr(descriptor),
dialect
);

hasNotNull = hasNotNull || hasNotNullFromComposingConstraints;
hasNotNull = compositionLogic.apply( hasNotNull, hasNotNullFromComposingConstraints );
}
if ( hasNotNull ) {
markNotNull( property );
}
return hasNotNull;
}

private static boolean isConstraintCompositionOfTypeOr(ConstraintDescriptor<?> descriptor) {
if ( descriptor.getComposingConstraints() == null ||
descriptor.getComposingConstraints().isEmpty() ) {
return false;
}

// This check assumes that Hibernate Validator is being used providing an
// implementation that gives us the composition type that is being used.
try {
Method compositionTypeMethod = descriptor.getClass()
.getMethod( "getCompositionType" );
Object result = compositionTypeMethod.invoke( descriptor );
if ( result != null && "or".equalsIgnoreCase( result.toString() ) ) {
return true;
}
}
catch ( NoSuchMethodException | IllegalAccessException | InvocationTargetException ex ) {
LOG.debug( "ConstraintComposition type could not be determined. Assuming AND", ex );
}
return false;
}

private static void applyMin(Property property, ConstraintDescriptor<?> descriptor, Dialect dialect) {
if ( Min.class.equals( descriptor.getAnnotation().annotationType() ) ) {
@SuppressWarnings("unchecked")
Expand Down Expand Up @@ -314,35 +352,32 @@ private static void applySQLCheck(Column col, String checkConstraint) {
col.addCheckConstraint( new CheckConstraint( checkConstraint ) );
}

private static boolean applyNotNull(Property property, ConstraintDescriptor<?> descriptor) {
boolean hasNotNull = false;
// NotNull, NotEmpty, and NotBlank annotation add not-null on column
final Class<? extends Annotation> annotationType = descriptor.getAnnotation().annotationType();
if ( NotNull.class.equals(annotationType)
|| NotEmpty.class.equals(annotationType)
|| NotBlank.class.equals(annotationType)) {
// single table inheritance should not be forced to null due to shared state
if ( !( property.getPersistentClass() instanceof SingleTableSubclass ) ) {
//composite should not add not-null on all columns
if ( !property.isComposite() ) {
for ( Selectable selectable : property.getSelectables() ) {
if ( selectable instanceof Column ) {
((Column) selectable).setNullable( false );
}
else {
LOG.debugf(
"@NotNull was applied to attribute [%s] which is defined (at least partially) " +
"by formula(s); formula portions will be skipped",
property.getName()
);
}
private static boolean isNotNullDescriptor(ConstraintDescriptor<?> descriptor) {
return List.of( NotNull.class, NotEmpty.class, NotBlank.class )
.contains( descriptor.getAnnotation().annotationType() );
}

private static void markNotNull(Property property) {
// single table inheritance should not be forced to null due to shared state
if ( !( property.getPersistentClass() instanceof SingleTableSubclass ) ) {
//composite should not add not-null on all columns
if ( !property.isComposite() ) {
for ( Selectable selectable : property.getSelectables() ) {
if ( selectable instanceof Column ) {
((Column) selectable).setNullable( false );
}
else {
LOG.debugf(
"@NotNull was applied to attribute [%s] which is defined (at least partially) " +
"by formula(s); formula portions will be skipped",
property.getName()
);
}
}
}
hasNotNull = true;
}
property.setOptional( !hasNotNull );
return hasNotNull;

property.setOptional( false );
}

private static void applyDigits(Property property, ConstraintDescriptor<?> descriptor) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ public class Address {

private String line1;
private String line2;
private String line3;
private String line4;
private String zip;
private String state;
@Size(max = 20)
Expand Down Expand Up @@ -55,6 +57,24 @@ public void setLine2(String line2) {
this.line2 = line2;
}

@NullOrNotBlank
public String getLine3() {
return line3;
}

public void setLine3(String line3) {
this.line3 = line3;
}

@NullAndNotBlank
public String getLine4() {
return line4;
}

public void setLine4(String line4) {
this.line4 = line4;
}

@Size(max = 3)
@NotNull
public String getState() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@ public void testNotNullDDL() {

Column line2Column = classMapping.getProperty( "line2" ).getColumns().get(0);
assertFalse("Validator annotations are applied on line2 as it is @NotBlank", line2Column.isNullable());

Column line3Column = classMapping.getProperty("line3").getColumns().get(0);
assertTrue(
"Validator composition of type OR should result in line3 being nullable",
line3Column.isNullable());

Column line4Column = classMapping.getProperty("line4" ).getColumns().get(0);
assertFalse(
"Validator composition of type AND should result in line4 being not-null",
line4Column.isNullable());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package org.hibernate.orm.test.annotations.beanvalidation;

import jakarta.validation.Constraint;
import jakarta.validation.Payload;
import jakarta.validation.ReportAsSingleViolation;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.Null;
import org.hibernate.validator.constraints.ConstraintComposition;

import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;

import static java.lang.annotation.ElementType.*;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import static org.hibernate.validator.constraints.CompositionType.AND;

/**
* Used to test constraint composition with AND for nullability of columns.
*/
@ConstraintComposition(AND)
@Null
@NotBlank
@ReportAsSingleViolation
@Target({METHOD, FIELD, ANNOTATION_TYPE, CONSTRUCTOR, PARAMETER})
@Retention(RUNTIME)
@Documented
@Constraint(validatedBy = {})
public @interface NullAndNotBlank {

String message() default "{org.hibernate.validator.constraints.NullAndNotBlank.message}";

Class<?>[] groups() default {};

Class<? extends Payload>[] payload() default {};

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package org.hibernate.orm.test.annotations.beanvalidation;

import jakarta.validation.Constraint;
import jakarta.validation.Payload;
import jakarta.validation.ReportAsSingleViolation;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.Null;
import org.hibernate.validator.constraints.ConstraintComposition;

import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;

import static java.lang.annotation.ElementType.*;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import static org.hibernate.validator.constraints.CompositionType.OR;

/**
* Used to test constraint composition with OR for nullability of columns.
*/
@ConstraintComposition(OR)
@Null
@NotBlank
@ReportAsSingleViolation
@Target({METHOD, FIELD, ANNOTATION_TYPE, CONSTRUCTOR, PARAMETER})
@Retention(RUNTIME)
@Documented
@Constraint(validatedBy = {})
public @interface NullOrNotBlank {

String message() default "{org.hibernate.validator.constraints.NullOrNotBlank.message}";

Class<?>[] groups() default {};

Class<? extends Payload>[] payload() default {};

}

0 comments on commit bafc6a8

Please sign in to comment.