From bfdd7f648b3258902025414cddd6ccae9874176e Mon Sep 17 00:00:00 2001 From: Gavin Date: Sat, 7 Jan 2023 02:53:26 +0100 Subject: [PATCH] HHH-10557 fix @Loader applied to a collection the issue here is we have no @CollectionResult for annotation-based result set mappings --- .../userguide/sql/CollectionLoaderTest.java | 84 ++++++++----------- .../hibernate/engine/spi/CollectionEntry.java | 4 +- .../internal/CollectionLoaderNamedQuery.java | 29 ++++++- .../query/results/ResultSetMappingImpl.java | 2 +- 4 files changed, 65 insertions(+), 54 deletions(-) diff --git a/documentation/src/test/java/org/hibernate/userguide/sql/CollectionLoaderTest.java b/documentation/src/test/java/org/hibernate/userguide/sql/CollectionLoaderTest.java index 88688db168..781e71b058 100644 --- a/documentation/src/test/java/org/hibernate/userguide/sql/CollectionLoaderTest.java +++ b/documentation/src/test/java/org/hibernate/userguide/sql/CollectionLoaderTest.java @@ -16,11 +16,9 @@ import jakarta.persistence.ElementCollection; import jakarta.persistence.Entity; import jakarta.persistence.GeneratedValue; import jakarta.persistence.Id; -import jakarta.persistence.NamedNativeQueries; import jakarta.persistence.NamedNativeQuery; import org.hibernate.annotations.Loader; -import org.hibernate.annotations.ResultCheckStyle; import org.hibernate.annotations.SQLDelete; import org.hibernate.annotations.SQLDeleteAll; import org.hibernate.annotations.SQLInsert; @@ -37,6 +35,7 @@ import org.hibernate.testing.TestForIssue; import org.junit.Before; import org.junit.Test; +import static org.hibernate.annotations.ResultCheckStyle.COUNT; import static org.hibernate.cfg.AvailableSettings.DEFAULT_LIST_SEMANTICS; import static org.hibernate.testing.transaction.TransactionUtil.doInJPA; import static org.junit.Assert.assertEquals; @@ -71,9 +70,11 @@ public class CollectionLoaderTest extends BaseEntityManagerFunctionalTestCase { session.doWork(connection -> { try(Statement statement = connection.createStatement();) { statement.executeUpdate(String.format( "ALTER TABLE person %s valid %s", - getDialect().getAddColumnString(), ddlTypeRegistry.getTypeName( Types.BOOLEAN, getDialect()))); + getDialect().getAddColumnString(), + ddlTypeRegistry.getTypeName( Types.BOOLEAN, getDialect()))); statement.executeUpdate(String.format( "ALTER TABLE Person_phones %s valid %s", - getDialect().getAddColumnString(), ddlTypeRegistry.getTypeName( Types.BOOLEAN, getDialect()))); + getDialect().getAddColumnString(), + ddlTypeRegistry.getTypeName( Types.BOOLEAN, getDialect()))); } }); }); @@ -91,55 +92,42 @@ public class CollectionLoaderTest extends BaseEntityManagerFunctionalTestCase { return person; }); - try { - - doInJPA(this::entityManagerFactory, entityManager -> { - Long postId = _person.getId(); - Person person = entityManager.find(Person.class, postId); - assertEquals(2, person.getPhones().size()); - person.getPhones().remove(0); - person.setName("Mr. John Doe"); - }); + doInJPA(this::entityManagerFactory, entityManager -> { + Long postId = _person.getId(); + Person person = entityManager.find(Person.class, postId); + assertEquals(2, person.getPhones().size()); + person.getPhones().remove(0); + person.setName("Mr. John Doe"); + }); - doInJPA(this::entityManagerFactory, entityManager -> { - Long postId = _person.getId(); - Person person = entityManager.find(Person.class, postId); - assertEquals(1, person.getPhones().size()); - }); - } - catch (Exception e) { - log.error("Throws NullPointerException because the bag is not initialized by the @Loader"); - } + doInJPA(this::entityManagerFactory, entityManager -> { + Long postId = _person.getId(); + Person person = entityManager.find(Person.class, postId); + assertEquals(1, person.getPhones().size()); + }); } //tag::sql-custom-crud-example[] @Entity(name = "Person") - @SQLInsert( - sql = "INSERT INTO person (name, id, valid) VALUES (?, ?, true) ", - check = ResultCheckStyle.COUNT - ) - @SQLUpdate( - sql = "UPDATE person SET name = ? where id = ? ") - @SQLDelete( - sql = "UPDATE person SET valid = false WHERE id = ? ") + @SQLInsert(sql = "INSERT INTO person (name, id, valid) VALUES (?, ?, true) ", check = COUNT) + @SQLUpdate(sql = "UPDATE person SET name = ? where id = ? ") + @SQLDelete(sql = "UPDATE person SET valid = false WHERE id = ? ") @Loader(namedQuery = "find_valid_person") - @NamedNativeQueries({ - @NamedNativeQuery( - name = "find_valid_person", - query = "SELECT id, name " + - "FROM person " + - "WHERE id = ? and valid = true", - resultClass = Person.class - ), - @NamedNativeQuery( - name = "find_valid_phones", - query = "SELECT person_id, phones " + - "FROM Person_phones " + - "WHERE person_id = ? and valid = true " - ) - }) + @NamedNativeQuery( + name = "find_valid_person", + query = "SELECT id, name " + + "FROM person " + + "WHERE id = ? and valid = true", + resultClass = Person.class + ) + @NamedNativeQuery( + name = "find_valid_phones", + query = "SELECT phones " + + "FROM Person_phones " + + "WHERE person_id = ? and valid = true " + ) public static class Person { @Id @@ -149,10 +137,8 @@ public class CollectionLoaderTest extends BaseEntityManagerFunctionalTestCase { private String name; @ElementCollection - @SQLInsert( - sql = "INSERT INTO person_phones (person_id, phones, valid) VALUES (?, ?, true) ") - @SQLDeleteAll( - sql = "UPDATE person_phones SET valid = false WHERE person_id = ?") + @SQLInsert(sql = "INSERT INTO person_phones (person_id, phones, valid) VALUES (?, ?, true) ") + @SQLDeleteAll(sql = "UPDATE person_phones SET valid = false WHERE person_id = ?") @Loader(namedQuery = "find_valid_phones") private List phones = new ArrayList<>(); diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/CollectionEntry.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/CollectionEntry.java index fc9e2f69ef..30c81010a2 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/CollectionEntry.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/CollectionEntry.java @@ -231,8 +231,8 @@ public final class CollectionEntry implements Serializable { loadedKey = getCurrentKey(); setLoadedPersister( getCurrentPersister() ); - boolean resnapshot = collection.wasInitialized() && - ( isDoremove() || isDorecreate() || isDoupdate() ); + boolean resnapshot = collection.wasInitialized() + && ( isDoremove() || isDorecreate() || isDoupdate() ); if ( resnapshot ) { snapshot = loadedPersister == null || !loadedPersister.isMutable() ? null : diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/CollectionLoaderNamedQuery.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/CollectionLoaderNamedQuery.java index 178cd38045..003af4b5c9 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/CollectionLoaderNamedQuery.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/CollectionLoaderNamedQuery.java @@ -8,15 +8,19 @@ package org.hibernate.loader.ast.internal; import org.hibernate.FlushMode; import org.hibernate.collection.spi.PersistentCollection; +import org.hibernate.engine.spi.CollectionKey; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.loader.ast.spi.CollectionLoader; import org.hibernate.metamodel.mapping.PluralAttributeMapping; import org.hibernate.persister.collection.CollectionPersister; +import org.hibernate.query.QueryTypeMismatchException; import org.hibernate.query.named.NamedQueryMemento; import org.hibernate.query.spi.QueryImplementor; import jakarta.persistence.Parameter; +import java.util.List; + /** * @author Steve Ebersole */ @@ -36,10 +40,31 @@ public class CollectionLoaderNamedQuery implements CollectionLoader { @Override public PersistentCollection load(Object key, SharedSessionContractImplementor session) { - final QueryImplementor> query = namedQueryMemento.toQuery( session ); + final QueryImplementor query = namedQueryMemento.toQuery( session ); //noinspection unchecked query.setParameter( (Parameter) query.getParameters().iterator().next(), key ); query.setHibernateFlushMode( FlushMode.MANUAL ); - return query.getResultList().get( 0 ); + final List resultList = query.getResultList(); + // TODO: we need a good way to inspect the query itself to see what it returns + if ( !resultList.isEmpty() && resultList.get(0) instanceof PersistentCollection ) { + // in hbm.xml files we have the element + return (PersistentCollection) resultList.get(0); + } + else { + // using annotations we have no way to specify a @CollectionResult + final CollectionKey collectionKey = new CollectionKey( persister, key ); + final PersistentCollection collection = session.getPersistenceContextInternal().getCollection( collectionKey ); + for ( Object element : resultList ) { + if ( element != null && !persister.getElementType().getReturnedClass().isInstance( element ) ) { + throw new QueryTypeMismatchException( "Collection loader for '" + persister.getRole() + + "' returned an instance of '" + element.getClass().getName() + "'" ); + } + } + collection.beforeInitialize( persister, resultList.size() ); + collection.injectLoadedState( getLoadable(), resultList ); + collection.afterInitialize(); + session.getPersistenceContextInternal().getCollectionEntry( collection ).postInitialize( collection ); + return collection; + } } } diff --git a/hibernate-core/src/main/java/org/hibernate/query/results/ResultSetMappingImpl.java b/hibernate-core/src/main/java/org/hibernate/query/results/ResultSetMappingImpl.java index 5616e765e2..a852bcdf19 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/results/ResultSetMappingImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/results/ResultSetMappingImpl.java @@ -360,7 +360,7 @@ public class ResultSetMappingImpl implements ResultSetMapping { && Objects.equals( legacyFetchBuilders, that.legacyFetchBuilders ); } else { - return !that.isDynamic && mappingIdentifier.equals( that.mappingIdentifier ); + return !that.isDynamic && mappingIdentifier != null && mappingIdentifier.equals( that.mappingIdentifier ); } } }