Skip to content

Commit c30db7c

Browse files
beikovbeikov
committed
HHH-19805 Allow nested entity initializers to claim entity holder
1 parent ab2512a commit c30db7c

File tree

3 files changed

+188
-22
lines changed

3 files changed

+188
-22
lines changed

‎hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -392,29 +392,20 @@ public EntityHolder claimEntityHolderIfPossible(
392392
var holder = getOrInitializeNewHolder().withEntity( key, key.getPersister(), entity );
393393
final var oldHolder = entityHolderMap.putIfAbsent( key, newEntityHolder );
394394
if ( oldHolder != null ) {
395+
// An initializer can't claim an entity holder if it's already initialized
396+
if ( oldHolder.isInitialized() ) {
397+
return oldHolder;
398+
}
395399
if ( entity != null ) {
396400
assert oldHolder.entity == null || oldHolder.entity == entity;
397401
oldHolder.entity = entity;
398402
}
399-
// Skip setting a new entity initializer if there already is one owner
400-
// Also skip if an entity exists which is different from the effective optional object.
401-
// The effective optional object is the current object to be refreshed,
402-
// which always needs re-initialization, even if already initialized
403-
if ( oldHolder.entityInitializer != null
404-
|| oldHolder.entity != null && oldHolder.state != EntityHolderState.ENHANCED_PROXY && (
405-
processingState.getProcessingOptions().getEffectiveOptionalObject() == null
406-
|| oldHolder.entity != processingState.getProcessingOptions().getEffectiveOptionalObject() )
407-
) {
408-
return oldHolder;
409-
}
410403
holder = oldHolder;
411404
}
412405
else {
413406
newEntityHolder = null;
414407
}
415-
assert holder.entityInitializer == null || holder.entityInitializer == initializer;
416408
holder.entityInitializer = initializer;
417-
processingState.registerLoadingEntityHolder( holder );
418409
return holder;
419410
}
420411

‎hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityInitializerImpl.java‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -505,9 +505,6 @@ public void resetResolvedEntityRegistrations(RowProcessingState rowProcessingSta
505505
rowProcessingState.getSession()
506506
.getPersistenceContextInternal()
507507
.removeEntityHolder( data.entityKey );
508-
rowProcessingState.getJdbcValuesSourceProcessingState()
509-
.getLoadingEntityHolders()
510-
.remove( data.entityHolder );
511508
data.entityKey = null;
512509
data.entityHolder = null;
513510
data.entityInstanceForNotify = null;
@@ -1094,7 +1091,9 @@ protected void resolveEntityInstance1(EntityInitializerData data) {
10941091
else {
10951092
data.setInstance( proxy );
10961093
if ( Hibernate.isInitialized( proxy ) ) {
1097-
data.setState( State.INITIALIZED );
1094+
if ( data.entityHolder.isInitialized() ) {
1095+
data.setState( State.INITIALIZED );
1096+
}
10981097
data.entityInstanceForNotify = Hibernate.unproxy( proxy );
10991098
}
11001099
else {
@@ -1254,11 +1253,7 @@ protected Object resolveEntityInstance(EntityInitializerData data) {
12541253
assert data.entityHolder.getEntityInitializer() == this;
12551254
// If this initializer owns the entity, we have to remove the entity holder,
12561255
// because the subsequent loading process will claim the entity
1257-
rowProcessingState.getJdbcValuesSourceProcessingState()
1258-
.getLoadingEntityHolders()
1259-
.remove( data.entityHolder );
1260-
session.getPersistenceContextInternal()
1261-
.removeEntityHolder( data.entityKey );
1256+
session.getPersistenceContextInternal().removeEntityHolder( data.entityKey );
12621257
return session.internalLoad(
12631258
data.concreteDescriptor.getEntityName(),
12641259
data.entityKey.getIdentifier(),
@@ -1340,10 +1335,15 @@ protected void registerReloadedEntity(EntityInitializerData data) {
13401335
@Override
13411336
public void initializeInstance(EntityInitializerData data) {
13421337
if ( data.getState() == State.RESOLVED ) {
1338+
// todo: think about what to do when one initializer fetches a lazy basic but not the other
13431339
if ( !skipInitialization( data ) ) {
13441340
assert consistentInstance( data );
13451341
initializeEntityInstance( data );
13461342
}
1343+
else if ( data.getRowProcessingState().needsResolveState() ) {
1344+
// We need to read result set values to correctly populate the query cache
1345+
resolveEntityState( data );
1346+
}
13471347
data.setState( State.INITIALIZED );
13481348
}
13491349
}
@@ -1367,6 +1367,7 @@ protected void initializeEntityInstance(EntityInitializerData data) {
13671367
final Object entityIdentifier = entityKey.getIdentifier();
13681368
final var resolvedEntityState = extractConcreteTypeStateValues( data );
13691369

1370+
rowProcessingState.getJdbcValuesSourceProcessingState().registerLoadingEntityHolder( data.entityHolder );
13701371
preLoad( data, resolvedEntityState );
13711372

13721373
final Object entityInstanceForNotify = data.entityInstanceForNotify;

‎hibernate-core/src/test/java/org/hibernate/orm/test/collection/set/PersistentSetRemoveTest.java‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
* Copyright Red Hat Inc. and Hibernate Authors
4+
*/
5+
package org.hibernate.orm.test.collection.set;
6+
7+
import jakarta.persistence.CascadeType;
8+
import jakarta.persistence.Entity;
9+
import jakarta.persistence.FetchType;
10+
import jakarta.persistence.Id;
11+
import jakarta.persistence.ManyToOne;
12+
import jakarta.persistence.OneToMany;
13+
import org.hibernate.testing.orm.junit.DomainModel;
14+
import org.hibernate.testing.orm.junit.Jira;
15+
import org.hibernate.testing.orm.junit.SessionFactory;
16+
import org.hibernate.testing.orm.junit.SessionFactoryScope;
17+
import org.junit.jupiter.api.Test;
18+
19+
import java.util.HashSet;
20+
import java.util.Objects;
21+
import java.util.Set;
22+
23+
import static org.junit.jupiter.api.Assertions.assertTrue;
24+
25+
@DomainModel(annotatedClasses = {
26+
PersistentSetRemoveTest.Parent.class,
27+
PersistentSetRemoveTest.Child.class
28+
})
29+
@SessionFactory
30+
@Jira("https://hibernate.atlassian.net/browse/HHH-19805")
31+
public class PersistentSetRemoveTest {
32+
33+
@Test
34+
public void testRemove(SessionFactoryScope scope) {
35+
Parent p1 = new Parent( 1L, "p1" );
36+
Child c1 = new Child( 1L, "c1" );
37+
Child c2 = new Child( 2L, "c2" );
38+
Child c3 = new Child( 3L, "c3" );
39+
p1.getChildren().add( c1 );
40+
c1.setParent( p1 );
41+
p1.getChildren().add( c2 );
42+
c2.setParent( p1 );
43+
p1.getChildren().add( c3 );
44+
c3.setParent( p1 );
45+
46+
scope.inTransaction(
47+
session -> session.persist( p1 )
48+
);
49+
50+
scope.inTransaction(
51+
session -> {
52+
Child child = session.createQuery( "from Child c where c.name = :name", Child.class )
53+
.setParameter( "name", "c1" )
54+
.getSingleResult();
55+
56+
boolean removed = child.getParent().getChildren().remove( child );
57+
assertTrue( removed );
58+
}
59+
);
60+
61+
scope.inTransaction(
62+
session -> {
63+
Parent parent = session.find( Parent.class, p1.getId() );
64+
Child childToRemove = parent.getChildren().stream()
65+
.filter( book -> "c2".equals( book.getName() ) )
66+
.findFirst()
67+
.orElseThrow();
68+
boolean removed = parent.getChildren().remove( childToRemove );
69+
assertTrue( removed );
70+
}
71+
);
72+
}
73+
74+
@Entity(name = "Parent")
75+
public static class Parent {
76+
@Id
77+
private Long id;
78+
private String name;
79+
80+
@OneToMany(mappedBy = "parent", cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.EAGER)
81+
private Set<Child> children;
82+
83+
public Parent() {
84+
this.children = new HashSet<>();
85+
}
86+
87+
public Parent(Long id, String name) {
88+
this.id = id;
89+
this.name = name;
90+
this.children = new HashSet<>();
91+
}
92+
93+
public Long getId() {
94+
return id;
95+
}
96+
97+
public void setId(Long id) {
98+
this.id = id;
99+
}
100+
101+
public String getName() {
102+
return name;
103+
}
104+
105+
public void setName(String name) {
106+
this.name = name;
107+
}
108+
109+
public Set<Child> getChildren() {
110+
return children;
111+
}
112+
113+
public void setChildren(Set<Child> children) {
114+
this.children = children;
115+
}
116+
}
117+
118+
119+
@Entity(name = "Child")
120+
public static class Child {
121+
@Id
122+
private Long id;
123+
private String name;
124+
125+
@ManyToOne
126+
private Parent parent;
127+
128+
public Child() {
129+
}
130+
131+
public Child(Long id, String name) {
132+
this.id = id;
133+
this.name = name;
134+
}
135+
136+
public Long getId() {
137+
return id;
138+
}
139+
140+
public void setId(Long id) {
141+
this.id = id;
142+
}
143+
144+
public String getName() {
145+
return name;
146+
}
147+
148+
public void setName(String name) {
149+
this.name = name;
150+
}
151+
152+
public Parent getParent() {
153+
return parent;
154+
}
155+
156+
public void setParent(Parent parent) {
157+
this.parent = parent;
158+
}
159+
160+
@Override
161+
public final boolean equals(Object o) {
162+
if ( !(o instanceof Child child) ) {
163+
return false;
164+
}
165+
166+
return Objects.equals( name, child.name );
167+
}
168+
169+
@Override
170+
public int hashCode() {
171+
return Objects.hashCode( name );
172+
}
173+
}
174+
}

0 commit comments

Comments
 (0)