improvements to Nullability and CascadingActions

includes a bugfix to nullability checking of composite collection elements

Author: gavinking

hibernate-core/src/main/java/org/hibernate/action/internal/AbstractEntityInsertAction.java

@@ -9,6 +9,7 @@
 import org.hibernate.engine.internal.ForeignKeys;
 import org.hibernate.engine.internal.NonNullableTransientDependencies;
 import org.hibernate.engine.internal.Nullability;
+import org.hibernate.engine.internal.Nullability.NullabilityCheckType;
 import org.hibernate.engine.spi.CachedNaturalIdValueSource;
 import org.hibernate.engine.spi.CollectionKey;
 import org.hibernate.engine.spi.EntityEntry;
@@ -121,7 +122,8 @@ protected final void nullifyTransientReferencesIfNotAlready() {
 		if ( !areTransientReferencesNullified ) {
 			new ForeignKeys.Nullifier( getInstance(), false, isEarlyInsert(), getSession(), getPersister() )
 					.nullifyTransientReferences( getState() );
-			new Nullability( getSession() ).checkNullability( getState(), getPersister(), false );
+			new Nullability( getSession(), NullabilityCheckType.CREATE )
+					.checkNullability( getState(), getPersister() );
 			areTransientReferencesNullified = true;
 		}
 	}

hibernate-core/src/main/java/org/hibernate/engine/internal/Nullability.java

@@ -19,6 +19,7 @@
 import org.hibernate.type.Type;
 
 import static org.hibernate.engine.spi.CascadingActions.getLoadedElementsIterator;
+import static org.hibernate.internal.util.StringHelper.qualify;
 
 /**
  * Implements the algorithm for validating property values for illegal null values
@@ -28,76 +29,83 @@
 public final class Nullability {
 	private final SharedSessionContractImplementor session;
 	private final boolean checkNullability;
+	private NullabilityCheckType checkType;
 
-	/**
-	 * Constructs a Nullability
-	 *
-	 * @param session The session
-	 */
+	public enum NullabilityCheckType {
+		CREATE,
+		UPDATE,
+		DELETE
+	}
+
+	public Nullability(SharedSessionContractImplementor session, NullabilityCheckType checkType) {
+		this.session = session;
+		this.checkNullability = session.getFactory().getSessionFactoryOptions().isCheckNullability();
+		this.checkType = checkType;
+	}
+
+	@Deprecated(forRemoval = true, since = "7")
 	public Nullability(SharedSessionContractImplementor session) {
 		this.session = session;
 		this.checkNullability = session.getFactory().getSessionFactoryOptions().isCheckNullability();
 	}
+
 	/**
-	 * Check nullability of the class persister properties
+	 * Check nullability of the entity properties
 	 *
 	 * @param values entity properties
 	 * @param persister class persister
 	 * @param isUpdate whether it is intended to be updated or saved
 	 *
 	 * @throws PropertyValueException Break the nullability of one property
 	 * @throws HibernateException error while getting Component values
+	 *
+	 * @deprecated Use {@link #checkNullability(Object[], EntityPersister)}
 	 */
+	@Deprecated(forRemoval = true, since = "7")
 	public void checkNullability(
 			final Object[] values,
 			final EntityPersister persister,
 			final boolean isUpdate) {
-		checkNullability( values, persister, isUpdate ? NullabilityCheckType.UPDATE : NullabilityCheckType.CREATE );
+		checkType =  isUpdate ? NullabilityCheckType.UPDATE : NullabilityCheckType.CREATE;
+		checkNullability( values, persister );
 	}
 
-	public enum NullabilityCheckType {
-		CREATE,
-		UPDATE,
-		DELETE
-	}
-
-	public void checkNullability(
-			final Object[] values,
-			final EntityPersister persister,
-			final NullabilityCheckType checkType) {
+	/**
+	 * Check nullability of the entity properties
+	 *
+	 * @param values entity properties
+	 * @param persister class persister
+	 *
+	 * @throws PropertyValueException Break the nullability of one property
+	 * @throws HibernateException error while getting Component values
+	 */
+	public void checkNullability(final Object[] values, final EntityPersister persister) {
 
-		/*
-		 * Typically when Bean Validation is on, we don't want to validate null values
-		 * at the Hibernate Core level. Hence the checkNullability setting.
-		 */
+		// Typically when Bean Validation is on, we don't want to validate null values
+		// at the Hibernate Core level. Hence, the checkNullability setting.
 		if ( checkNullability ) {
-			/*
-			 * Algorithm
-			 * Check for any level one nullability breaks
-			 * Look at non-null components to
-			 *   recursively check next level of nullability breaks
-			 * Look at Collections containing components to
-			 *   recursively check next level of nullability breaks
-			 *
-			 *
-			 * In the previous implementation, not-null stuffs where checked
-			 * filtering by level one only updatable
-			 * or insertable columns. So setting a subcomponent as update="false"
-			 * has no effect on not-null check if the main component had good checkability
-			 * In this implementation, we keep this feature.
-			 * However, I never see any documentation mentioning that, but it's for
-			 * sure a limitation.
-			 */
+			// Algorithm:
+			// Check for any level one nullability breaks
+			// Look at non-null components to
+			//   recursively check next level of nullability breaks
+			// Look at Collections containing components to
+			//   recursively check next level of nullability breaks
+			//
+			// In the previous implementation, not-null stuffs where checked
+			// filtering by level one only updatable
+			// or insertable columns. So setting a subcomponent as update="false"
+			// has no effect on not-null check if the main component had good checkability
+			// In this implementation, we keep this feature.
+			// However, I never see any documentation mentioning that, but it's for
+			// sure a limitation.
 
 			final boolean[] nullability = persister.getPropertyNullability();
 			final boolean[] checkability = checkType == NullabilityCheckType.CREATE
 					? persister.getPropertyInsertability()
 					: persister.getPropertyUpdateability();
 			final Type[] propertyTypes = persister.getPropertyTypes();
 			final Generator[] generators = persister.getEntityMetamodel().getGenerators();
-
 			for ( int i = 0; i < values.length; i++ ) {
-
 				if ( checkability[i]
 						&& values[i] != LazyPropertyInitializer.UNFETCHED_PROPERTY
 						&& !generated( generators[i] ) ) {
@@ -118,13 +126,12 @@ else if ( value != null ) {
 							throw new PropertyValueException(
 								"not-null property references a null or transient value",
 								persister.getEntityName(),
-								buildPropertyPath( persister.getPropertyNames()[i], breakProperties )
+								qualify( persister.getPropertyNames()[i], breakProperties )
 							);
 						}
 
 					}
 				}
-
 			}
 		}
 	}
@@ -134,98 +141,90 @@ private static boolean generated(Generator generator) {
 	}
 
 	/**
-	 * check sub elements-nullability. Returns property path that break
-	 * nullability or null if none
+	 * Check nullability of sub-elements.
+	 * Returns property path that break nullability, or null if none.
 	 *
 	 * @param propertyType type to check
 	 * @param value value to check
 	 *
 	 * @return property path
 	 * @throws HibernateException error while getting subcomponent values
 	 */
-	private String checkSubElementsNullability(Type propertyType, Object value) throws HibernateException {
-		if ( propertyType instanceof AnyType ) {
-			return checkComponentNullability( value, (AnyType) propertyType );
+	private String checkSubElementsNullability(Type propertyType, Object value) {
+		if ( propertyType instanceof AnyType anyType ) {
+			return checkComponentNullability( value, anyType );
 		}
-		if ( propertyType instanceof ComponentType ) {
-			return checkComponentNullability( value, (ComponentType) propertyType );
+		else if ( propertyType instanceof ComponentType componentType ) {
+			return checkComponentNullability( value, componentType );
 		}
-
-		if ( propertyType instanceof CollectionType collectionType ) {
+		else if ( propertyType instanceof CollectionType collectionType ) {
 			// persistent collections may have components
-			final Type collectionElementType = collectionType.getElementType( session.getFactory() );
-
-			if ( collectionElementType instanceof ComponentType || collectionElementType instanceof AnyType ) {
+			if ( collectionType.getElementType( session.getFactory() ) instanceof CompositeType componentType ) {
 				// check for all components values in the collection
-				final CompositeType componentType = (CompositeType) collectionElementType;
-				final Iterator<?> itr = getLoadedElementsIterator( session, collectionType, value );
-				while ( itr.hasNext() ) {
-					final Object compositeElement = itr.next();
+				final Iterator<?> iterator = getLoadedElementsIterator( collectionType, value );
+				while ( iterator.hasNext() ) {
+					final Object compositeElement = iterator.next();
 					if ( compositeElement != null ) {
-						return checkComponentNullability( compositeElement, componentType );
+						final String path = checkComponentNullability( compositeElement, componentType );
+						if ( path != null ) {
+							return path;
+						}
 					}
 				}
 			}
+			return null;
+		}
+		else {
+			return null;
 		}
-
-		return null;
 	}
 
 	/**
-	 * check component nullability. Returns property path that break
-	 * nullability or null if none
+	 * Check component nullability.
+	 * Returns property path that breaks nullability, or null if none.
 	 *
-	 * @param value component properties
+	 * @param composite component properties
 	 * @param compositeType component not-nullable type
 	 *
 	 * @return property path
 	 * @throws HibernateException error while getting subcomponent values
 	 */
-	private String checkComponentNullability(Object value, CompositeType compositeType) throws HibernateException {
+	private String checkComponentNullability(Object composite, CompositeType compositeType) {
 		// IMPL NOTE : we currently skip checking "any" and "many to any" mappings.
 		//
-		// This is not the best solution.  But atm there is a mismatch between AnyType#getPropertyNullability
+		// This is not the best solution. But atm there is a mismatch between AnyType#getPropertyNullability
 		// and the fact that cascaded-saves for "many to any" mappings are not performed until after this nullability
-		// check.  So the nullability check fails for transient entity elements with generated identifiers because
+		// check. So the nullability check fails for transient entity elements with generated identifiers because
 		// the identifier is not yet generated/assigned (is null)
 		//
 		// The more correct fix would be to cascade saves of the many-to-any elements before the Nullability checking
 
 		if ( compositeType instanceof AnyType ) {
 			return null;
 		}
-
-		final boolean[] nullability = compositeType.getPropertyNullability();
-		if ( nullability != null ) {
-			//do the test
-			final Object[] subValues = compositeType.getPropertyValues( value, session );
-			final Type[] propertyTypes = compositeType.getSubtypes();
-			for ( int i = 0; i < subValues.length; i++ ) {
-				final Object subValue = subValues[i];
-				if ( !nullability[i] && subValue==null ) {
-					return compositeType.getPropertyNames()[i];
-				}
-				else if ( subValue != null ) {
-					final String breakProperties = checkSubElementsNullability( propertyTypes[i], subValue );
-					if ( breakProperties != null ) {
-						return buildPropertyPath( compositeType.getPropertyNames()[i], breakProperties );
+		else {
+			final boolean[] nullability = compositeType.getPropertyNullability();
+			if ( nullability != null ) {
+				//do the test
+				final Object[] values = compositeType.getPropertyValues( composite, session );
+				final Type[] propertyTypes = compositeType.getSubtypes();
+				for ( int i = 0; i < values.length; i++ ) {
+					final Object value = values[i];
+					if ( value == null ) {
+						if ( !nullability[i] ) {
+							return compositeType.getPropertyNames()[i];
+						}
+					}
+					else {
+						final String breakProperties = checkSubElementsNullability( propertyTypes[i], value );
+						if ( breakProperties != null ) {
+							return qualify( compositeType.getPropertyNames()[i], breakProperties );
+						}
 					}
 				}
 			}
+			return null;
 		}
-		return null;
-	}
-
-	/**
-	 * Return a well formed property path. Basically, it will return parent.child
-	 *
-	 * @param parent parent in path
-	 * @param child child in path
-	 *
-	 * @return parent-child path
-	 */
-	private static String buildPropertyPath(String parent, String child) {
-		return parent + '.' + child;
 	}
 
 }