From dd7543620d97ddb561d46867a8cd93f45db5ec1b Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Thu, 25 Sep 2025 16:52:45 +0200 Subject: [PATCH] HHH-19805 Allow nested entity initializers to claim entity holder --- .../internal/StatefulPersistenceContext.java | 17 +- .../internal/EntityInitializerImpl.java | 14 +- .../set/PersistentSetRemoveTest.java | 174 ++++++++++++++++++ 3 files changed, 187 insertions(+), 18 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/collection/set/PersistentSetRemoveTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java index 81b8a13eb122..8e2554586ecb 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java @@ -392,29 +392,20 @@ public EntityHolder claimEntityHolderIfPossible( var holder = getOrInitializeNewHolder().withEntity( key, key.getPersister(), entity ); final var oldHolder = entityHolderMap.putIfAbsent( key, newEntityHolder ); if ( oldHolder != null ) { + // An initializer can't claim an entity holder if it's already initialized + if ( oldHolder.isInitialized() ) { + return oldHolder; + } if ( entity != null ) { assert oldHolder.entity == null || oldHolder.entity == entity; oldHolder.entity = entity; } - // Skip setting a new entity initializer if there already is one owner - // Also skip if an entity exists which is different from the effective optional object. - // The effective optional object is the current object to be refreshed, - // which always needs re-initialization, even if already initialized - if ( oldHolder.entityInitializer != null - || oldHolder.entity != null && oldHolder.state != EntityHolderState.ENHANCED_PROXY && ( - processingState.getProcessingOptions().getEffectiveOptionalObject() == null - || oldHolder.entity != processingState.getProcessingOptions().getEffectiveOptionalObject() ) - ) { - return oldHolder; - } holder = oldHolder; } else { newEntityHolder = null; } - assert holder.entityInitializer == null || holder.entityInitializer == initializer; holder.entityInitializer = initializer; - processingState.registerLoadingEntityHolder( holder ); return holder; } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityInitializerImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityInitializerImpl.java index a5c6e9f514f6..54187b9cd0af 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityInitializerImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityInitializerImpl.java @@ -510,9 +510,6 @@ public void resetResolvedEntityRegistrations(RowProcessingState rowProcessingSta rowProcessingState.getSession() .getPersistenceContextInternal() .removeEntityHolder( data.entityKey ); - rowProcessingState.getJdbcValuesSourceProcessingState() - .getLoadingEntityHolders() - .remove( data.entityHolder ); data.entityKey = null; data.entityHolder = null; data.entityInstanceForNotify = null; @@ -1091,7 +1088,9 @@ protected void resolveEntityInstance1(EntityInitializerData data) { else { data.setInstance( proxy ); if ( Hibernate.isInitialized( proxy ) ) { - data.setState( State.INITIALIZED ); + if ( data.entityHolder.isInitialized() ) { + data.setState( State.INITIALIZED ); + } data.entityInstanceForNotify = Hibernate.unproxy( proxy ); } else { @@ -1242,7 +1241,6 @@ protected Object resolveEntityInstance(EntityInitializerData data) { assert data.entityHolder.getEntityInitializer() == this; // If this initializer owns the entity, we have to remove the entity holder, // because the subsequent loading process will claim the entity - rowProcessingState.getJdbcValuesSourceProcessingState().getLoadingEntityHolders().remove( data.entityHolder ); session.getPersistenceContextInternal().removeEntityHolder( data.entityKey ); return session.internalLoad( data.concreteDescriptor.getEntityName(), @@ -1326,10 +1324,15 @@ protected void registerReloadedEntity(EntityInitializerData data) { @Override public void initializeInstance(EntityInitializerData data) { if ( data.getState() == State.RESOLVED ) { + // todo: think about what to do when one initializer fetches a lazy basic but not the other if ( !skipInitialization( data ) ) { assert consistentInstance( data ); initializeEntityInstance( data ); } + else if ( data.getRowProcessingState().needsResolveState() ) { + // We need to read result set values to correctly populate the query cache + resolveEntityState( data ); + } data.setState( State.INITIALIZED ); } } @@ -1352,6 +1355,7 @@ protected void initializeEntityInstance(EntityInitializerData data) { final Object entityIdentifier = entityKey.getIdentifier(); final Object[] resolvedEntityState = extractConcreteTypeStateValues( data ); + rowProcessingState.getJdbcValuesSourceProcessingState().registerLoadingEntityHolder( data.entityHolder ); preLoad( data, resolvedEntityState ); final Object entityInstanceForNotify = data.entityInstanceForNotify; diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/collection/set/PersistentSetRemoveTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/collection/set/PersistentSetRemoveTest.java new file mode 100644 index 000000000000..471ec183c256 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/collection/set/PersistentSetRemoveTest.java @@ -0,0 +1,174 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.collection.set; + +import jakarta.persistence.CascadeType; +import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.Id; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.OneToMany; +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.Test; + +import java.util.HashSet; +import java.util.Objects; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +@DomainModel(annotatedClasses = { + PersistentSetRemoveTest.Parent.class, + PersistentSetRemoveTest.Child.class +}) +@SessionFactory +@Jira("https://hibernate.atlassian.net/browse/HHH-19805") +public class PersistentSetRemoveTest { + + @Test + public void testRemove(SessionFactoryScope scope) { + Parent p1 = new Parent( 1L, "p1" ); + Child c1 = new Child( 1L, "c1" ); + Child c2 = new Child( 2L, "c2" ); + Child c3 = new Child( 3L, "c3" ); + p1.getChildren().add( c1 ); + c1.setParent( p1 ); + p1.getChildren().add( c2 ); + c2.setParent( p1 ); + p1.getChildren().add( c3 ); + c3.setParent( p1 ); + + scope.inTransaction( + session -> session.persist( p1 ) + ); + + scope.inTransaction( + session -> { + Child child = session.createQuery( "from Child c where c.name = :name", Child.class ) + .setParameter( "name", "c1" ) + .getSingleResult(); + + boolean removed = child.getParent().getChildren().remove( child ); + assertTrue( removed ); + } + ); + + scope.inTransaction( + session -> { + Parent parent = session.find( Parent.class, p1.getId() ); + Child childToRemove = parent.getChildren().stream() + .filter( book -> "c2".equals( book.getName() ) ) + .findFirst() + .orElseThrow(); + boolean removed = parent.getChildren().remove( childToRemove ); + assertTrue( removed ); + } + ); + } + + @Entity(name = "Parent") + public static class Parent { + @Id + private Long id; + private String name; + + @OneToMany(mappedBy = "parent", cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.EAGER) + private Set children; + + public Parent() { + this.children = new HashSet<>(); + } + + public Parent(Long id, String name) { + this.id = id; + this.name = name; + this.children = new HashSet<>(); + } + + 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 Set getChildren() { + return children; + } + + public void setChildren(Set children) { + this.children = children; + } + } + + + @Entity(name = "Child") + public static class Child { + @Id + private Long id; + private String name; + + @ManyToOne + private Parent parent; + + public Child() { + } + + public Child(Long id, String name) { + this.id = id; + this.name = name; + } + + 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 Parent getParent() { + return parent; + } + + public void setParent(Parent parent) { + this.parent = parent; + } + + @Override + public final boolean equals(Object o) { + if ( !(o instanceof Child child) ) { + return false; + } + + return Objects.equals( name, child.name ); + } + + @Override + public int hashCode() { + return Objects.hashCode( name ); + } + } +}