HHH-18724 Take constraint composition type into account when determining nullable state of a Column

Author: wytzevanderploeg

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

@@ -5,13 +5,18 @@
 package org.hibernate.boot.beanvalidation;
 
 import java.lang.annotation.Annotation;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.lang.invoke.MethodHandles;
 import java.util.Collection;
+import java.util.HashMap;
 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.Digits;
 import jakarta.validation.constraints.Max;
@@ -72,6 +77,9 @@ class TypeSafeActivator {
 
 	private static final CoreMessageLogger LOG = Logger.getMessageLogger( MethodHandles.lookup(), CoreMessageLogger.class, TypeSafeActivator.class.getName() );
 
+	private static final Map<Class<? extends Annotation>, Boolean>
+			CONSTRAINT_COMPOSITION_TYPE_CACHE = new HashMap<>();
+
 	/**
 	 * Used to validate a supplied ValidatorFactory instance as being castable to ValidatorFactory.
 	 *
@@ -230,6 +238,7 @@ private static void applyDDL(
 						propertyDesc,
 						groups,
 						activateNotNull,
+						false,
 						dialect
 				);
 				if ( property.isComposite() && propertyDesc.isCascaded() ) {
@@ -257,12 +266,20 @@ private static boolean applyConstraints(
 			PropertyDescriptor propertyDesc,
 			Set<Class<?>> groups,
 			boolean canApplyNotNull,
+			boolean useOrLogicForComposedConstraint,
 			Dialect dialect) {
 		boolean hasNotNull = false;
 		for ( ConstraintDescriptor<?> descriptor : constraintDescriptors ) {
 			if ( groups == null || !disjoint( descriptor.getGroups(), groups ) ) {
+				// 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 ( canApplyNotNull ) {
-					hasNotNull = hasNotNull || applyNotNull( property, descriptor );
+					hasNotNull = compositionLogic.apply( hasNotNull, isNotNullDescriptor( descriptor ) );
 				}
 
 				// apply bean validation specific constraints
@@ -281,16 +298,43 @@ 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(final ConstraintDescriptor<?> descriptor) {
+		if ( descriptor.getComposingConstraints() == null ||
+				descriptor.getComposingConstraints().size() < 2 ) {
+			return false;
+		}
+
+		return CONSTRAINT_COMPOSITION_TYPE_CACHE.computeIfAbsent( descriptor.getAnnotation().getClass(), unused -> {
+			// 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".equals( 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")
@@ -328,35 +372,31 @@ private static void applySQLCheck(Column column, String checkConstraint) {
 		column.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 ) {
-							column.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) {

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

@@ -15,6 +15,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)
@@ -52,6 +54,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() {

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

@@ -46,6 +46,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

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

@@ -0,0 +1,41 @@
+/*
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ * Copyright Red Hat Inc. and Hibernate Authors
+ */
+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 {};
+
+}

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

@@ -0,0 +1,41 @@
+/*
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ * Copyright Red Hat Inc. and Hibernate Authors
+ */
+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 {};
+
+}