From fa604bdc36999303dd5ce3fa642705cc5a3588e8 Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Fri, 3 Aug 2018 01:25:34 -0700 Subject: [PATCH] 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 --- .../source/internal/hbm/ModelBinder.java | 42 +++++++++++-------- .../cfg/annotations/CollectionBinder.java | 35 ++++++---------- .../hibernate/internal/util/StringHelper.java | 34 +++++++++++++++ .../test/legacy/ParentChildTest.java | 12 ++++-- 4 files changed, 79 insertions(+), 44 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/ModelBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/ModelBinder.java index 91f134fbfe..7a2c4d98ee 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/ModelBinder.java +++ b/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( elementSource.getReferencedEntityName() ); - if ( StringHelper.isNotEmpty( referencedEntityBinding.getWhere() ) ) { - whereBuffer.append( referencedEntityBinding.getWhere() ); - } - if ( StringHelper.isNotEmpty( elementSource.getWhere() ) ) { - if ( whereBuffer.length() > 0 ) { - whereBuffer.append( " and " ); - } - whereBuffer.append( elementSource.getWhere() ); - } - getCollectionBinding().setManyToManyWhere( whereBuffer.toString() ); - + final PersistentClass referencedEntityBinding = mappingDocument.getMetadataCollector().getEntityBinding( + elementSource.getReferencedEntityName() + ); + getCollectionBinding().setWhere( getPluralAttributeSource().getWhere() ); + getCollectionBinding().setManyToManyWhere( + StringHelper.getNonEmptyOrConjunctionIfBothNonEmpty( + referencedEntityBinding.getWhere(), + elementSource.getWhere() + ) + ); getCollectionBinding().setManyToManyOrdering( elementSource.getOrder() ); if ( !CollectionHelper.isEmpty( elementSource.getFilterSources() ) diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/annotations/CollectionBinder.java b/hibernate-core/src/main/java/org/hibernate/cfg/annotations/CollectionBinder.java index 8c566518e9..7ccc8c4ebe 100644 --- a/hibernate-core/src/main/java/org/hibernate/cfg/annotations/CollectionBinder.java +++ b/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(); - if ( StringHelper.isNotEmpty( clause ) ) { - whereBuffer.append( clause ); - } + whereOnClassClause = whereOnClass.clause(); } } Where whereOnCollection = property.getAnnotation( Where.class ); + String whereOnCollectionClause = null; if ( whereOnCollection != null ) { - String clause = whereOnCollection.clause(); - if ( StringHelper.isNotEmpty( clause ) ) { - if ( whereBuffer.length() > 0 ) { - whereBuffer.append( ' ' ); - whereBuffer.append( Junction.Nature.AND.getOperator() ); - whereBuffer.append( ' ' ); - } - whereBuffer.append( clause ); - } + whereOnCollectionClause = whereOnCollection.clause(); } - if ( whereBuffer.length() > 0 ) { - String whereClause = whereBuffer.toString(); - if ( hasAssociationTable ) { - collection.setManyToManyWhere( whereClause ); - } - else { - collection.setWhere( whereClause ); - } + final String whereClause = StringHelper.getNonEmptyOrConjunctionIfBothNonEmpty( + whereOnClassClause, + whereOnCollectionClause + ); + if ( hasAssociationTable ) { + collection.setManyToManyWhere( whereClause ); + } + else { + collection.setWhere( whereClause ); } WhereJoinTable whereJoinTable = property.getAnnotation( WhereJoinTable.class ); diff --git a/hibernate-core/src/main/java/org/hibernate/internal/util/StringHelper.java b/hibernate-core/src/main/java/org/hibernate/internal/util/StringHelper.java index ac6db99f0f..acd425617d 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/util/StringHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/util/StringHelper.java @@ -852,4 +852,38 @@ public final class StringHelper { public interface Renderer { 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; + } + } } diff --git a/hibernate-core/src/test/java/org/hibernate/test/legacy/ParentChildTest.java b/hibernate-core/src/test/java/org/hibernate/test/legacy/ParentChildTest.java index 587f57ef50..0acff0581e 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/legacy/ParentChildTest.java +++ b/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, p2.getId() )); + s.delete( s.get( Part.class, p1.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();