From 5fd74adcbf75a80bbb9de8a775b39395489a4353 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Thu, 15 Aug 2024 11:34:40 +0200 Subject: [PATCH] HHH-18493 Resolving already initialized collection elements leads to assertion error --- .../collection/internal/ArrayInitializer.java | 9 +- .../collection/internal/BagInitializer.java | 14 +- .../collection/internal/ListInitializer.java | 9 +- .../collection/internal/MapInitializer.java | 32 +- .../collection/internal/SetInitializer.java | 7 +- .../ReloadMultipleCollectionElementsTest.java | 307 ++++++++++++++++++ 6 files changed, 341 insertions(+), 37 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/join/ReloadMultipleCollectionElementsTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ArrayInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ArrayInitializer.java index 413221ae7c..1417600a7f 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ArrayInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ArrayInitializer.java @@ -6,6 +6,7 @@ */ package org.hibernate.sql.results.graph.collection.internal; +import java.lang.reflect.Array; import java.util.Iterator; import java.util.List; import java.util.function.BiConsumer; @@ -131,9 +132,11 @@ public class ArrayInitializer extends AbstractImmediateCollectionInitializer initializer = elementAssembler.getInitializer(); if ( initializer != null ) { final RowProcessingState rowProcessingState = data.getRowProcessingState(); - final Iterator iter = getCollectionInstance( data ).elements(); - while ( iter.hasNext() ) { - initializer.resolveInstance( iter.next(), rowProcessingState ); + final Integer index = listIndexAssembler.assemble( rowProcessingState ); + if ( index != null ) { + final PersistentArrayHolder arrayHolder = getCollectionInstance( data ); + assert arrayHolder != null; + initializer.resolveInstance( Array.get( arrayHolder.getArray(), index ), rowProcessingState ); } } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/BagInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/BagInitializer.java index 00e207c4d4..b73643cd47 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/BagInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/BagInitializer.java @@ -123,19 +123,7 @@ public class BagInitializer extends AbstractImmediateCollectionInitializer initializer = elementAssembler.getInitializer(); if ( initializer != null ) { - final RowProcessingState rowProcessingState = data.getRowProcessingState(); - final PersistentCollection persistentCollection = getCollectionInstance( data ); - assert persistentCollection != null; - if ( persistentCollection instanceof PersistentBag ) { - for ( Object element : ( (PersistentBag) persistentCollection ) ) { - initializer.resolveInstance( element, rowProcessingState ); - } - } - else { - for ( Object element : ( (PersistentIdentifierBag) persistentCollection ) ) { - initializer.resolveInstance( element, rowProcessingState ); - } - } + initializer.resolveKey( data.getRowProcessingState() ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ListInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ListInitializer.java index 0850a0927d..67e9c16a74 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ListInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/ListInitializer.java @@ -123,10 +123,11 @@ public class ListInitializer extends AbstractImmediateCollectionInitializer initializer = elementAssembler.getInitializer(); if ( initializer != null ) { final RowProcessingState rowProcessingState = data.getRowProcessingState(); - final PersistentList list = getCollectionInstance( data ); - assert list != null; - for ( Object element : list ) { - initializer.resolveInstance( element, rowProcessingState ); + final Integer index = listIndexAssembler.assemble( rowProcessingState ); + if ( index != null ) { + final PersistentList list = getCollectionInstance( data ); + assert list != null; + initializer.resolveInstance( list.get( index ), rowProcessingState ); } } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/MapInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/MapInitializer.java index 8c4f36185f..49c21cc965 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/MapInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/MapInitializer.java @@ -121,17 +121,27 @@ public class MapInitializer extends AbstractImmediateCollectionInitializer keyInitializer = mapKeyAssembler.getInitializer(); final Initializer valueInitializer = mapValueAssembler.getInitializer(); - if ( keyInitializer != null || valueInitializer != null ) { - final RowProcessingState rowProcessingState = data.getRowProcessingState(); - final PersistentMap map = getCollectionInstance( data ); - assert map != null; - for ( Map.Entry entry : map.entrySet() ) { - if ( keyInitializer != null ) { - keyInitializer.resolveInstance( entry.getKey(), rowProcessingState ); - } - if ( valueInitializer != null ) { - valueInitializer.resolveInstance( entry.getValue(), rowProcessingState ); - } + final RowProcessingState rowProcessingState = data.getRowProcessingState(); + if ( keyInitializer == null && valueInitializer != null ) { + // For now, we only support resolving the value initializer instance when keys have no initializer, + // though we could also support map keys with an initializer given that the initialized java type: + // * is an entity that uses only the primary key in equals/hashCode. + // If the primary key type is an embeddable, the next condition must hold for that + // * or is an embeddable that has no initializers for fields being used in the equals/hashCode + // which violate this same requirement (recursion) + final Object key = mapKeyAssembler.assemble( rowProcessingState ); + if ( key != null ) { + final PersistentMap map = getCollectionInstance( data ); + assert map != null; + valueInitializer.resolveInstance( map.get( key ), rowProcessingState ); + } + } + else { + if ( keyInitializer != null ) { + keyInitializer.resolveKey( rowProcessingState ); + } + if ( valueInitializer != null ) { + valueInitializer.resolveKey( rowProcessingState ); } } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/SetInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/SetInitializer.java index b2b86d88cc..30909c6ce0 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/SetInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/SetInitializer.java @@ -97,12 +97,7 @@ public class SetInitializer extends AbstractImmediateCollectionInitializer initializer = elementAssembler.getInitializer(); if ( initializer != null ) { - final RowProcessingState rowProcessingState = data.getRowProcessingState(); - final PersistentSet set = getCollectionInstance( data ); - assert set != null; - for ( Object element : set ) { - initializer.resolveInstance( element, rowProcessingState ); - } + initializer.resolveKey( data.getRowProcessingState() ); } } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/join/ReloadMultipleCollectionElementsTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/join/ReloadMultipleCollectionElementsTest.java new file mode 100644 index 0000000000..516bf16c9f --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/join/ReloadMultipleCollectionElementsTest.java @@ -0,0 +1,307 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.orm.test.inheritance.join; + +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.hibernate.Hibernate; + +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.Jira; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import jakarta.persistence.CascadeType; +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.ManyToMany; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.OneToMany; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@DomainModel( annotatedClasses = { + ReloadMultipleCollectionElementsTest.Flight.class, + ReloadMultipleCollectionElementsTest.Ticket.class, + ReloadMultipleCollectionElementsTest.Customer.class, + ReloadMultipleCollectionElementsTest.Company.class +} ) +@SessionFactory +@Jira("https://hibernate.atlassian.net/browse/HHH-18493") +public class ReloadMultipleCollectionElementsTest { + + @BeforeEach + public void setup(SessionFactoryScope scope) { + scope.inTransaction( s -> { + Flight f2 = new Flight(); + f2.setId(1L); + f2.setName("Flight two"); + + Company us = new Company(); + us.setName("Airline 2"); + f2.setCompany(us); + + Customer c1 = new Customer(); + c1.setId( 1L ); + c1.setName("Tom"); + + Customer c2 = new Customer(); + c2.setId( 2L ); + c2.setName("Pete"); + + Ticket t1 = new Ticket(); + t1.setId(1L); + t1.setCustomer(c2); + t1.setNumber( "123" ); + + Ticket t2 = new Ticket(); + t2.setId(2L); + t2.setCustomer(c2); + t2.setNumber( "456" ); + + f2.setCustomers(Set.of(c1, c2)); + + s.persist(c1); + s.persist(c2); + s.persist(f2); + s.persist(t1); + s.persist(t2); + } ); + } + + @AfterEach + public void cleanup(SessionFactoryScope scope) { + scope.inTransaction( s -> { + s.createMutationQuery( "delete from Ticket" ).executeUpdate(); + s.createMutationQuery( "delete from Flight" ).executeUpdate(); + s.createMutationQuery( "delete from Customer" ).executeUpdate(); + s.createMutationQuery( "delete from Company" ).executeUpdate(); + } ); + } + + @Test + public void testResolveElementOfInitializedCollection(SessionFactoryScope scope) { + scope.inTransaction( s -> { + // First load all customers with their flights collection and corresponding customers + List customers = s.createQuery( + "from Customer c join fetch c.flights f join fetch f.customers order by c.id", + Customer.class + ).getResultList(); + assertEquals( 2, customers.size() ); + assertFalse( Hibernate.isInitialized( customers.get( 0 ).getTickets() ) ); + assertFalse( Hibernate.isInitialized( customers.get( 1 ).getTickets() ) ); + + // Then load all flights with their customers collection, but in addition, also the customers tickets + // This will trigger resolveInstance(Object, Data) with the existing collection and will + // fetch tickets data into existing customers + s.createQuery( "from Flight f join fetch f.customers c left join fetch c.tickets", Flight.class ).getResultList(); + + assertTrue( Hibernate.isInitialized( customers.get( 0 ).getTickets() ) ); + assertTrue( Hibernate.isInitialized( customers.get( 1 ).getTickets() ) ); + assertEquals( 0, customers.get( 0 ).getTickets().size() ); + assertEquals( 2, customers.get( 1 ).getTickets().size() ); + } ); + } + + @Entity( name = "Flight" ) + public static class Flight { + private Long id; + private String name; + private Company company; + private Set customers; + + public Flight() { + } + + @Id + @Column(name = "flight_id") + public Long getId() { + return id; + } + + public void setId(Long long1) { + id = long1; + } + + @Column(updatable = false, name = "flight_name", nullable = false, length = 50) + public String getName() { + return name; + } + + public void setName(String string) { + name = string; + } + + + @ManyToOne(cascade = {CascadeType.ALL}) + @JoinColumn(name = "comp_id") + public Company getCompany() { + return company; + } + + public void setCompany(Company company) { + this.company = company; + } + + @ManyToMany(cascade = {CascadeType.PERSIST, CascadeType.MERGE}, fetch = FetchType.EAGER) + public Set getCustomers() { + return customers; + } + + public void setCustomers(Set customers) { + this.customers = customers; + } + } + + @Entity( name = "Ticket" ) + public static class Ticket { + + Long id; + String number; + Customer customer; + + public Ticket() { + } + + @Id + public Long getId() { + return id; + } + + public void setId(Long long1) { + id = long1; + } + + @Column(name = "nr") + public String getNumber() { + return number; + } + + public void setNumber(String string) { + number = string; + } + + @ManyToOne(cascade = CascadeType.ALL) + @JoinColumn(name = "CUSTOMER_ID") + public Customer getCustomer() { + return customer; + } + + public void setCustomer(Customer customer) { + this.customer = customer; + } + } + + @Entity( name = "Customer" ) + public static class Customer { + private Long id; + private String name; + private String address; + private Set tickets; + private Set flights; + + // Address address; + + public Customer() { + } + + @Id + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getAddress() { + return address; + } + + public void setAddress(String address) { + this.address = address; + } + + @OneToMany(cascade = CascadeType.ALL, mappedBy = "customer") + public Set getTickets() { + return tickets; + } + + public void setTickets(Set tickets) { + this.tickets = tickets; + } + + @ManyToMany(cascade = {CascadeType.PERSIST, CascadeType.MERGE}, mappedBy = "customers") + public Set getFlights() { + return flights; + } + + public void setFlights(Set flights) { + this.flights = flights; + } + } + + @Entity( name = "Company" ) + public static class Company { + + private Long id; + private String name; + private Set flights = new HashSet(); + + public Company() { + } + + @Id + @GeneratedValue(strategy = GenerationType.AUTO) + @Column(name = "comp_id") + public Long getId() { + return id; + } + + public String getName() { + return name; + } + + public void setId(Long newId) { + id = newId; + } + + public void setName(String string) { + name = string; + } + + @OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER, mappedBy = "company") + @Column(name = "flight_id") + public Set getFlights() { + return flights; + } + + public void setFlights(Set flights) { + this.flights = flights; + } + } +}