HHH-12875 HHH-12882 : Class level where="..." clause hbm.xml mappings is not enforced on collections of that class; add parentheses when where clauses get combined in a conjunction (hbm and annotations)

HHH-12882 : correct assertions in ParentChildTest

hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/ModelBinder.java

@@ -1527,7 +1527,6 @@ public class ModelBinder {
 			binding.getSynchronizedTables().add( name );
 		}
 
-		binding.setWhere( source.getWhere() );
 		binding.setLoaderName( source.getCustomLoaderName() );
 		if ( source.getCustomSqlInsert() != null ) {
 			binding.setCustomSQLInsert(
@@ -3448,9 +3447,15 @@ public class ModelBinder {
 
 				final PersistentClass referencedEntityBinding = mappingDocument.getMetadataCollector()
 						.getEntityBinding( elementSource.getReferencedEntityName() );
+				collectionBinding.setWhere(
+					StringHelper.getNonEmptyOrConjunctionIfBothNonEmpty(
+							referencedEntityBinding.getWhere(),
+							getPluralAttributeSource().getWhere()
+					)
+				);
+
 				elementBinding.setReferencedEntityName( referencedEntityBinding.getEntityName() );
 				elementBinding.setAssociatedClass( referencedEntityBinding );
-
 				elementBinding.setIgnoreNotFound( elementSource.isIgnoreNotFound() );
 			}
 			else if ( getPluralAttributeSource().getElementSource() instanceof PluralAttributeElementSourceManyToMany ) {
@@ -3469,12 +3474,17 @@ public class ModelBinder {
 						new RelationalObjectBinder.ColumnNamingDelegate() {
 							@Override
 							public Identifier determineImplicitName(final LocalMetadataBuildingContext context) {
-								return context.getMetadataCollector().getDatabase().toIdentifier( Collection.DEFAULT_ELEMENT_COLUMN_NAME );
+								return context.getMetadataCollector()
+										.getDatabase()
+										.toIdentifier( Collection.DEFAULT_ELEMENT_COLUMN_NAME );
 							}
 						}
 				);
 
-				elementBinding.setLazy( elementSource.getFetchCharacteristics().getFetchTiming() != FetchTiming.IMMEDIATE );
+				elementBinding.setLazy(
+						elementSource.getFetchCharacteristics()
+								.getFetchTiming() != FetchTiming.IMMEDIATE
+				);
 				elementBinding.setFetchMode(
 						elementSource.getFetchCharacteristics().getFetchStyle() == FetchStyle.SELECT
 								? FetchMode.SELECT
@@ -3494,20 +3504,16 @@ public class ModelBinder {
 
 				getCollectionBinding().setElement( elementBinding );
 
-				final StringBuilder whereBuffer = new StringBuilder();
+				final PersistentClass referencedEntityBinding = mappingDocument.getMetadataCollector().getEntityBinding(
-				final PersistentClass referencedEntityBinding = mappingDocument.getMetadataCollector()
+						elementSource.getReferencedEntityName()
-						.getEntityBinding( elementSource.getReferencedEntityName() );
+				);
-				if ( StringHelper.isNotEmpty( referencedEntityBinding.getWhere() ) ) {
+				getCollectionBinding().setWhere( getPluralAttributeSource().getWhere() );
-					whereBuffer.append( referencedEntityBinding.getWhere() );
+				getCollectionBinding().setManyToManyWhere(
-				}
+						StringHelper.getNonEmptyOrConjunctionIfBothNonEmpty(
-				if ( StringHelper.isNotEmpty( elementSource.getWhere() ) ) {
+								referencedEntityBinding.getWhere(),
-					if ( whereBuffer.length() > 0 ) {
+								elementSource.getWhere()
-						whereBuffer.append( " and " );
+						)
-					}
+				);
-					whereBuffer.append( elementSource.getWhere() );
-				}
-				getCollectionBinding().setManyToManyWhere( whereBuffer.toString() );
-
 				getCollectionBinding().setManyToManyOrdering( elementSource.getOrder() );
 
 				if ( !CollectionHelper.isEmpty( elementSource.getFilterSources() )

hibernate-core/src/main/java/org/hibernate/cfg/annotations/CollectionBinder.java

@@ -953,36 +953,27 @@ public abstract class CollectionBinder {
 			}
 		}
 
-		StringBuilder whereBuffer = new StringBuilder();
+		String whereOnClassClause = null;
 		if ( property.getElementClass() != null ) {
 			Where whereOnClass = property.getElementClass().getAnnotation( Where.class );
 			if ( whereOnClass != null ) {
-				String clause = whereOnClass.clause();
+				whereOnClassClause = whereOnClass.clause();
-				if ( StringHelper.isNotEmpty( clause ) ) {
-					whereBuffer.append( clause );
-				}
 			}
 		}
 		Where whereOnCollection = property.getAnnotation( Where.class );
+		String whereOnCollectionClause = null;
 		if ( whereOnCollection != null ) {
-			String clause = whereOnCollection.clause();
+			whereOnCollectionClause = whereOnCollection.clause();
-			if ( StringHelper.isNotEmpty( clause ) ) {
-				if ( whereBuffer.length() > 0 ) {
-					whereBuffer.append( ' ' );
-					whereBuffer.append( Junction.Nature.AND.getOperator() );
-					whereBuffer.append( ' ' );
-				}
-				whereBuffer.append( clause );
-			}
 		}
-		if ( whereBuffer.length() > 0 ) {
+		final String whereClause = StringHelper.getNonEmptyOrConjunctionIfBothNonEmpty(
-			String whereClause = whereBuffer.toString();
+				whereOnClassClause,
-			if ( hasAssociationTable ) {
+				whereOnCollectionClause
-				collection.setManyToManyWhere( whereClause );
+		);
-			}
+		if ( hasAssociationTable ) {
-			else {
+			collection.setManyToManyWhere( whereClause );
-				collection.setWhere( whereClause );
+		}
-			}
+		else {
+			collection.setWhere( whereClause );
 		}
 
 		WhereJoinTable whereJoinTable = property.getAnnotation( WhereJoinTable.class );

hibernate-core/src/main/java/org/hibernate/internal/util/StringHelper.java

@@ -852,4 +852,38 @@ public final class StringHelper {
 	public interface Renderer<T> {
 		String render(T value);
 	}
+
+	/**
+	 * @param firstExpression the first expression
+	 * @param secondExpression the second expression
+	 * @return if {@code firstExpression} and {@code secondExpression} are both non-empty,
+	 * then "( " + {@code firstExpression} + " ) and ( " + {@code secondExpression} + " )" is returned;
+	 * if {@code firstExpression} is non-empty and {@code secondExpression} is empty,
+	 * then {@code firstExpression} is returned;
+	 * if {@code firstExpression} is empty and {@code secondExpression} is non-empty,
+	 * then {@code secondExpression} is returned;
+	 * if both {@code firstExpression} and {@code secondExpression} are empty, then null is returned.
+	 */
+	public static String getNonEmptyOrConjunctionIfBothNonEmpty( String firstExpression, String secondExpression ) {
+		final boolean isFirstExpressionNonEmpty = StringHelper.isNotEmpty( firstExpression );
+		final boolean isSecondExpressionNonEmpty = StringHelper.isNotEmpty( secondExpression );
+		if ( isFirstExpressionNonEmpty && isSecondExpressionNonEmpty ) {
+			final StringBuilder buffer = new StringBuilder();
+			buffer.append( "( " )
+					.append( firstExpression )
+					.append( " ) and ( ")
+					.append( secondExpression )
+					.append( " )" );
+			return buffer.toString();
+		}
+		else if ( isFirstExpressionNonEmpty ) {
+			return firstExpression;
+		}
+		else if ( isSecondExpressionNonEmpty ) {
+			return secondExpression;
+		}
+		else {
+			return null;
+		}
+	}
 }

hibernate-core/src/test/java/org/hibernate/test/legacy/ParentChildTest.java

@@ -482,12 +482,16 @@ public class ParentChildTest extends LegacyTestCase {
 		assertTrue( s.createCriteria(Part.class).list().size()==1 ); //there is a where condition on Part mapping
 		assertTrue( s.createCriteria(Part.class).add( Restrictions.eq( "id", p1.getId() ) ).list().size()==1 );
 		assertTrue( s.createQuery("from Part").list().size()==1 );
-		assertTrue( s.createQuery("from Baz baz join baz.parts").list().size()==2 );
+		// only Part entities that satisfy the where condition on Part mapping should be included in the collection
+		assertTrue( s.createQuery("from Baz baz join baz.parts").list().size()==1 );
 		baz = (Baz) s.createCriteria(Baz.class).uniqueResult();
-		assertTrue( s.createFilter( baz.getParts(), "" ).list().size()==2 );
+		// only Part entities that satisfy the where condition on Part mapping should be included in the collection
+		assertTrue( s.createFilter( baz.getParts(), "" ).list().size()==1 );
 		//assertTrue( baz.getParts().size()==1 );
-		s.delete( s.get( Part.class, p1.getId() ));
+		s.delete( s.get( Part.class, p1.getId() ) );
-		s.delete( s.get( Part.class, p2.getId() ));
+		// p2.description does not satisfy the condition on Part mapping, so it's not possible to retrieve it
+		// using Session#get; instead just delete using a native query
+		s.createNativeQuery( "delete from Part where id = :id" ).setParameter( "id", p2.getId() ).executeUpdate();
 		s.delete(baz);
 		t.commit();
 		s.close();