Skip to content

Commit 68c7d48

Browse files
beikovbeikov
committed
HHH-19805 Allow nested entity initializers to claim entity holder
1 parent 9df82f7 commit 68c7d48

File tree

4 files changed

+192
-22
lines changed

4 files changed

+192
-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
@@ -404,29 +404,20 @@ public EntityHolder claimEntityHolderIfPossible(
404404
EntityHolderImpl holder = getOrInitializeNewHolder().withEntity( key, key.getPersister(), entity );
405405
final EntityHolderImpl oldHolder = entityHolderMap.putIfAbsent( key, newEntityHolder );
406406
if ( oldHolder != null ) {
407+
// An initializer can't claim an entity holder if it's already initialized
408+
if ( oldHolder.isInitialized() ) {
409+
return oldHolder;
410+
}
407411
if ( entity != null ) {
408412
assert oldHolder.entity == null || oldHolder.entity == entity;
409413
oldHolder.entity = entity;
410414
}
411-
// Skip setting a new entity initializer if there already is one owner
412-
// Also skip if an entity exists which is different from the effective optional object.
413-
// The effective optional object is the current object to be refreshed,
414-
// which always needs re-initialization, even if already initialized
415-
if ( oldHolder.entityInitializer != null
416-
|| oldHolder.entity != null && oldHolder.state != EntityHolderState.ENHANCED_PROXY && (
417-
processingState.getProcessingOptions().getEffectiveOptionalObject() == null
418-
|| oldHolder.entity != processingState.getProcessingOptions().getEffectiveOptionalObject() )
419-
) {
420-
return oldHolder;
421-
}
422415
holder = oldHolder;
423416
}
424417
else {
425418
newEntityHolder = null;
426419
}
427-
assert holder.entityInitializer == null || holder.entityInitializer == initializer;
428420
holder.entityInitializer = initializer;
429-
processingState.registerLoadingEntityHolder( holder );
430421
return holder;
431422
}
432423

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -510,9 +510,6 @@ public void resetResolvedEntityRegistrations(RowProcessingState rowProcessingSta
510510
rowProcessingState.getSession()
511511
.getPersistenceContextInternal()
512512
.removeEntityHolder( data.entityKey );
513-
rowProcessingState.getJdbcValuesSourceProcessingState()
514-
.getLoadingEntityHolders()
515-
.remove( data.entityHolder );
516513
data.entityKey = null;
517514
data.entityHolder = null;
518515
data.entityInstanceForNotify = null;
@@ -1151,7 +1148,9 @@ protected void resolveEntityInstance1(EntityInitializerData data) {
11511148
else {
11521149
data.setInstance( proxy );
11531150
if ( Hibernate.isInitialized( proxy ) ) {
1154-
data.setState( State.INITIALIZED );
1151+
if ( data.entityHolder.isInitialized() ) {
1152+
data.setState( State.INITIALIZED );
1153+
}
11551154
data.entityInstanceForNotify = Hibernate.unproxy( proxy );
11561155
}
11571156
else {
@@ -1302,7 +1301,7 @@ protected Object resolveEntityInstance(EntityInitializerData data) {
13021301
assert data.entityHolder.getEntityInitializer() == this;
13031302
// If this initializer owns the entity, we have to remove the entity holder,
13041303
// because the subsequent loading process will claim the entity
1305-
rowProcessingState.getJdbcValuesSourceProcessingState().getLoadingEntityHolders().remove( data.entityHolder );
1304+
13061305
session.getPersistenceContextInternal().removeEntityHolder( data.entityKey );
13071306
return session.internalLoad(
13081307
data.concreteDescriptor.getEntityName(),
@@ -1386,10 +1385,15 @@ protected void registerReloadedEntity(EntityInitializerData data) {
13861385
@Override
13871386
public void initializeInstance(EntityInitializerData data) {
13881387
if ( data.getState() == State.RESOLVED ) {
1388+
// todo: think about what to do when one initializer fetches a lazy basic but not the other
13891389
if ( !skipInitialization( data ) ) {
13901390
assert consistentInstance( data );
13911391
initializeEntityInstance( data );
13921392
}
1393+
else if ( data.getRowProcessingState().needsResolveState() ) {
1394+
// We need to read result set values to correctly populate the query cache
1395+
resolveEntityState( data );
1396+
}
13931397
data.setState( State.INITIALIZED );
13941398
}
13951399
}
@@ -1412,6 +1416,7 @@ protected void initializeEntityInstance(EntityInitializerData data) {
14121416
final Object entityIdentifier = entityKey.getIdentifier();
14131417
final Object[] resolvedEntityState = extractConcreteTypeStateValues( data );
14141418

1419+
rowProcessingState.getJdbcValuesSourceProcessingState().registerLoadingEntityHolder( data.entityHolder );
14151420
preLoad( data, resolvedEntityState );
14161421

14171422
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+
}

‎hibernate-core/src/test/java/org/hibernate/orm/test/locking/options/ScopeTests.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -279,8 +279,8 @@ void testEagerFindWithExtended(SessionFactoryScope factoryScope) {
279279
}
280280
else {
281281
assertThat( sqlCollector.getSqlQueries() ).hasSize( 3 );
282-
Helper.checkSql( sqlCollector.getSqlQueries().get( 1 ), session.getDialect(), REPORTS );
283-
Helper.checkSql( sqlCollector.getSqlQueries().get( 2 ), session.getDialect(), PERSONS );
282+
Helper.checkSql( sqlCollector.getSqlQueries().get( 1 ), session.getDialect(), PERSONS );
283+
Helper.checkSql( sqlCollector.getSqlQueries().get( 2 ), session.getDialect(), REPORTS );
284284
TransactionUtil.assertRowLock( factoryScope, REPORTS.getTableName(), "title", "id", report.getId(), true );
285285

286286
// these should happen but currently do not - follow-on locking is not locking element-collection tables...
@@ -315,8 +315,8 @@ void testEagerFindWithFetchScope(SessionFactoryScope factoryScope) {
315315
}
316316
else {
317317
assertThat( sqlCollector.getSqlQueries() ).hasSize( 3 );
318-
Helper.checkSql( sqlCollector.getSqlQueries().get( 1 ), session.getDialect(), REPORTS );
319-
Helper.checkSql( sqlCollector.getSqlQueries().get( 2 ), session.getDialect(), PERSONS );
318+
Helper.checkSql( sqlCollector.getSqlQueries().get( 1 ), session.getDialect(), PERSONS );
319+
Helper.checkSql( sqlCollector.getSqlQueries().get( 2 ), session.getDialect(), REPORTS );
320320
TransactionUtil.assertRowLock( factoryScope, REPORTS.getTableName(), "title", "id", report.getId(), true );
321321

322322
// these should happen but currently do not - follow-on locking is not locking element-collection tables...

0 commit comments

Comments
 (0)