Skip to content

Commit c792bee

Browse files
beikovbeikov
committed
HHH-19273 Don't trust state of read-only associations for initializtion
1 parent b7d2fd1 commit c792bee

File tree

29 files changed

+3738
-314
lines changed

29 files changed

+3738
-314
lines changed

‎hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/CodeTemplates.java‎

Lines changed: 38 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -172,16 +172,18 @@ static class CollectionAreCollectionFieldsDirty {
172172
@Advice.Return(readOnly = false) boolean returned,
173173
@FieldName String fieldName,
174174
@FieldValue Collection<?> collection,
175-
@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker) {
176-
if ( !returned && $$_hibernate_collectionTracker != null ) {
177-
final int size = $$_hibernate_collectionTracker.getSize( fieldName );
178-
if ( collection == null && size != -1 ) {
175+
@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker,
176+
@Advice.FieldValue(value = EnhancerConstants.INTERCEPTOR_FIELD_NAME) PersistentAttributeInterceptor $$_hibernate_attributeInterceptor) {
177+
// Only look at initialized attributes, since value sameness is tracked via InlineDirtyCheckingHandler
178+
if ( !returned && $$_hibernate_collectionTracker != null && ( $$_hibernate_attributeInterceptor == null
179+
|| $$_hibernate_attributeInterceptor.isAttributeLoaded( fieldName ) ) ) {
180+
if ( collection == null && $$_hibernate_collectionTracker.getSize( fieldName ) != -1 ) {
179181
returned = true;
180182
}
181183
else if ( collection != null ) {
182184
// We only check sizes of non-persistent or initialized persistent collections
183185
if ( ( !( collection instanceof PersistentCollection ) || ( (PersistentCollection<?>) collection ).wasInitialized() )
184-
&& size != collection.size() ) {
186+
&& $$_hibernate_collectionTracker.getSize( fieldName ) != collection.size() ) {
185187
returned = true;
186188
}
187189
}
@@ -195,16 +197,18 @@ static class MapAreCollectionFieldsDirty {
195197
@Advice.Return(readOnly = false) boolean returned,
196198
@FieldName String fieldName,
197199
@FieldValue Map<?, ?> map,
198-
@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker) {
199-
if ( !returned && $$_hibernate_collectionTracker != null ) {
200-
final int size = $$_hibernate_collectionTracker.getSize( fieldName );
201-
if ( map == null && size != -1 ) {
200+
@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker,
201+
@Advice.FieldValue(value = EnhancerConstants.INTERCEPTOR_FIELD_NAME) PersistentAttributeInterceptor $$_hibernate_attributeInterceptor) {
202+
// Only look at initialized attributes, since value sameness is tracked via InlineDirtyCheckingHandler
203+
if ( !returned && $$_hibernate_collectionTracker != null && ( $$_hibernate_attributeInterceptor == null
204+
|| $$_hibernate_attributeInterceptor.isAttributeLoaded( fieldName ) ) ) {
205+
if ( map == null && $$_hibernate_collectionTracker.getSize( fieldName ) != -1 ) {
202206
returned = true;
203207
}
204208
else if ( map != null ) {
205209
// We only check sizes of non-persistent or initialized persistent collections
206210
if ( ( !( map instanceof PersistentCollection ) || ( (PersistentCollection) map ).wasInitialized() )
207-
&& size != map.size() ) {
211+
&& $$_hibernate_collectionTracker.getSize( fieldName ) != map.size() ) {
208212
returned = true;
209213
}
210214
}
@@ -214,20 +218,22 @@ else if ( map != null ) {
214218

215219
static class CollectionGetCollectionFieldDirtyNames {
216220
@Advice.OnMethodExit
217-
static void $$_hibernate_areCollectionFieldsDirty(
221+
static void $$_hibernate_getCollectionFieldDirtyNames(
218222
@FieldName String fieldName,
219223
@FieldValue Collection<?> collection,
220224
@Advice.Argument(0) DirtyTracker tracker,
221-
@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker) {
222-
if ( $$_hibernate_collectionTracker != null ) {
223-
final int size = $$_hibernate_collectionTracker.getSize( fieldName );
224-
if ( collection == null && size != -1 ) {
225+
@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker,
226+
@Advice.FieldValue(value = EnhancerConstants.INTERCEPTOR_FIELD_NAME) PersistentAttributeInterceptor $$_hibernate_attributeInterceptor) {
227+
// Only look at initialized attributes, since value sameness is tracked via InlineDirtyCheckingHandler
228+
if ( $$_hibernate_collectionTracker != null && ( $$_hibernate_attributeInterceptor == null
229+
|| $$_hibernate_attributeInterceptor.isAttributeLoaded( fieldName ) ) ) {
230+
if ( collection == null && $$_hibernate_collectionTracker.getSize( fieldName ) != -1 ) {
225231
tracker.add( fieldName );
226232
}
227233
else if ( collection != null ) {
228234
// We only check sizes of non-persistent or initialized persistent collections
229235
if ( ( !( collection instanceof PersistentCollection ) || ( (PersistentCollection<?>) collection ).wasInitialized() )
230-
&& size != collection.size() ) {
236+
&& $$_hibernate_collectionTracker.getSize( fieldName ) != collection.size() ) {
231237
tracker.add( fieldName );
232238
}
233239
}
@@ -237,20 +243,22 @@ else if ( collection != null ) {
237243

238244
static class MapGetCollectionFieldDirtyNames {
239245
@Advice.OnMethodExit
240-
static void $$_hibernate_areCollectionFieldsDirty(
246+
static void $$_hibernate_getCollectionFieldDirtyNames(
241247
@FieldName String fieldName,
242248
@FieldValue Map<?, ?> map,
243249
@Advice.Argument(0) DirtyTracker tracker,
244-
@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker) {
245-
if ( $$_hibernate_collectionTracker != null ) {
246-
final int size = $$_hibernate_collectionTracker.getSize( fieldName );
247-
if ( map == null && size != -1 ) {
250+
@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker,
251+
@Advice.FieldValue(value = EnhancerConstants.INTERCEPTOR_FIELD_NAME) PersistentAttributeInterceptor $$_hibernate_attributeInterceptor) {
252+
// Only look at initialized attributes, since value sameness is tracked via InlineDirtyCheckingHandler
253+
if ( $$_hibernate_collectionTracker != null && ( $$_hibernate_attributeInterceptor == null
254+
|| $$_hibernate_attributeInterceptor.isAttributeLoaded( fieldName ) ) ) {
255+
if ( map == null && $$_hibernate_collectionTracker.getSize( fieldName ) != -1 ) {
248256
tracker.add( fieldName );
249257
}
250258
else if ( map != null ) {
251259
// We only check sizes of non-persistent or initialized persistent collections
252260
if ( ( !( map instanceof PersistentCollection ) || ( (PersistentCollection<?>) map ).wasInitialized() )
253-
&& size != map.size() ) {
261+
&& $$_hibernate_collectionTracker.getSize( fieldName ) != map.size() ) {
254262
tracker.add( fieldName );
255263
}
256264
}
@@ -260,16 +268,14 @@ else if ( map != null ) {
260268

261269
static class CollectionGetCollectionClearDirtyNames {
262270
@Advice.OnMethodExit
263-
static void $$_hibernate_clearDirtyCollectionNames(
271+
static void $$_hibernate_removeDirtyFields(
264272
@FieldName String fieldName,
265273
@FieldValue Collection<?> collection,
266274
@Advice.Argument(value = 0, readOnly = false) LazyAttributeLoadingInterceptor lazyInterceptor,
267275
@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker) {
268-
if ( lazyInterceptor == null || lazyInterceptor.isAttributeLoaded( fieldName ) ) {
269-
if ( collection == null || collection instanceof PersistentCollection && !( (PersistentCollection<?>) collection ).wasInitialized() ) {
270-
$$_hibernate_collectionTracker.add( fieldName, -1 );
271-
}
272-
else {
276+
// Only look at initialized attributes, since value sameness is tracked via InlineDirtyCheckingHandler
277+
if ( ( lazyInterceptor == null || lazyInterceptor.isAttributeLoaded( fieldName ) ) && collection != null ) {
278+
if ( !( collection instanceof PersistentCollection ) || ( (PersistentCollection<?>) collection ).wasInitialized() ) {
273279
$$_hibernate_collectionTracker.add( fieldName, collection.size() );
274280
}
275281
}
@@ -278,16 +284,14 @@ static class CollectionGetCollectionClearDirtyNames {
278284

279285
static class MapGetCollectionClearDirtyNames {
280286
@Advice.OnMethodExit
281-
static void $$_hibernate_clearDirtyCollectionNames(
287+
static void $$_hibernate_removeDirtyFields(
282288
@FieldName String fieldName,
283289
@FieldValue Map<?, ?> map,
284290
@Advice.Argument(value = 0, readOnly = false) LazyAttributeLoadingInterceptor lazyInterceptor,
285291
@Advice.FieldValue(EnhancerConstants.TRACKER_COLLECTION_NAME) CollectionTracker $$_hibernate_collectionTracker) {
286-
if ( lazyInterceptor == null || lazyInterceptor.isAttributeLoaded( fieldName ) ) {
287-
if ( map == null || map instanceof PersistentCollection && !( (PersistentCollection<?>) map ).wasInitialized() ) {
288-
$$_hibernate_collectionTracker.add( fieldName, -1 );
289-
}
290-
else {
292+
// Only look at initialized attributes, since value sameness is tracked via InlineDirtyCheckingHandler
293+
if ( ( lazyInterceptor == null || lazyInterceptor.isAttributeLoaded( fieldName ) ) && map != null ) {
294+
if ( !( map instanceof PersistentCollection ) || ( (PersistentCollection<?>) map ).wasInitialized() ) {
291295
$$_hibernate_collectionTracker.add( fieldName, map.size() );
292296
}
293297
}

‎hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/InlineDirtyCheckerEqualsHelper.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,19 @@ public static boolean areEquals(
2626
return Objects.deepEquals( a, b );
2727
}
2828

29+
public static boolean areSame(
30+
PersistentAttributeInterceptable persistentAttributeInterceptable,
31+
String fieldName,
32+
Object a,
33+
Object b) {
34+
final PersistentAttributeInterceptor persistentAttributeInterceptor = persistentAttributeInterceptable.$$_hibernate_getInterceptor();
35+
if ( persistentAttributeInterceptor != null
36+
&& !persistentAttributeInterceptor.isAttributeLoaded( fieldName ) ) {
37+
return false;
38+
}
39+
return a == b;
40+
}
41+
2942
public static boolean areEquals(
3043
PersistentAttributeInterceptable persistentAttributeInterceptable,
3144
String fieldName,

‎hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/InlineDirtyCheckingHandler.java‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
package org.hibernate.bytecode.enhance.internal.bytebuddy;
88

99
import java.util.Collection;
10+
import java.util.Map;
1011
import java.util.Objects;
1112

12-
import jakarta.persistence.Embedded;
1313
import jakarta.persistence.EmbeddedId;
1414
import jakarta.persistence.Id;
1515

@@ -43,16 +43,19 @@ final class InlineDirtyCheckingHandler implements Implementation, ByteCodeAppend
4343

4444
private final FieldDescription.InDefinedShape persistentField;
4545
private final boolean applyLazyCheck;
46+
private final boolean applySamenessCheck;
4647

4748
private InlineDirtyCheckingHandler(
4849
Implementation delegate,
4950
TypeDescription managedCtClass,
5051
FieldDescription.InDefinedShape persistentField,
51-
boolean applyLazyCheck) {
52+
boolean applyLazyCheck,
53+
boolean applySamenessCheck) {
5254
this.delegate = delegate;
5355
this.managedCtClass = managedCtClass;
5456
this.persistentField = persistentField;
5557
this.applyLazyCheck = applyLazyCheck;
58+
this.applySamenessCheck = applySamenessCheck;
5659
}
5760

5861
static Implementation wrap(
@@ -66,14 +69,16 @@ static Implementation wrap(
6669
implementation = Advice.to( CodeTemplates.CompositeDirtyCheckingHandler.class ).wrap( implementation );
6770
}
6871
else if ( !persistentField.hasAnnotation( Id.class )
69-
&& !persistentField.hasAnnotation( EmbeddedId.class )
70-
&& !( persistentField.getType().asErasure().isAssignableTo( Collection.class )
71-
&& enhancementContext.isMappedCollection( persistentField ) ) ) {
72+
&& !persistentField.hasAnnotation( EmbeddedId.class ) ) {
7273
implementation = new InlineDirtyCheckingHandler(
7374
implementation,
7475
managedCtClass,
7576
persistentField.asDefined(),
76-
enhancementContext.hasLazyLoadableAttributes( managedCtClass )
77+
enhancementContext.hasLazyLoadableAttributes( managedCtClass ),
78+
// Also track value changes (object identity) for persistent collection attributes
79+
( persistentField.getType().asErasure().isAssignableTo( Collection.class )
80+
|| persistentField.getType().asErasure().isAssignableTo( Map.class ) )
81+
&& enhancementContext.isMappedCollection( persistentField )
7782
);
7883
}
7984

@@ -161,7 +166,7 @@ public Size apply(
161166
methodVisitor.visitMethodInsn(
162167
Opcodes.INVOKESTATIC,
163168
HELPER_TYPE_NAME,
164-
"areEquals",
169+
applySamenessCheck ? "areSame" : "areEquals",
165170
Type.getMethodDescriptor(
166171
Type.BOOLEAN_TYPE,
167172
PE_INTERCEPTABLE_TYPE,

‎hibernate-core/src/main/java/org/hibernate/metamodel/mapping/CollectionPart.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,9 @@ default String getPartName() {
9797
default ModelPart getInclusionCheckPart() {
9898
return this;
9999
}
100+
101+
@Override
102+
default boolean isReadOnly() {
103+
return getCollectionAttribute().isReadOnly();
104+
}
100105
}

‎hibernate-core/src/main/java/org/hibernate/metamodel/mapping/PluralAttributeMapping.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,4 +222,10 @@ default boolean isPluralAttributeMapping() {
222222
return true;
223223
}
224224

225+
@Override
226+
default boolean isReadOnly() {
227+
return getCollectionDescriptor().getMappedByProperty() != null
228+
|| getKeyDescriptor().getKeyPart().isReadOnly();
229+
}
230+
225231
}

‎hibernate-core/src/main/java/org/hibernate/metamodel/mapping/SelectableMappings.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,15 @@ default int forEachSelectable(SelectableConsumer consumer) {
4242
return forEachSelectable( 0, consumer );
4343
}
4444

45+
default boolean isReadOnly() {
46+
final int jdbcTypeCount = getJdbcTypeCount();
47+
for ( int i = 0; i < jdbcTypeCount; i++ ) {
48+
final var selectableMapping = getSelectable( i );
49+
if ( selectableMapping.isInsertable() || selectableMapping.isUpdateable() ) {
50+
return false;
51+
}
52+
}
53+
return true;
54+
}
55+
4556
}

‎hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2694,7 +2694,7 @@ public int[] resolveDirtyAttributeIndexes(
26942694
}
26952695

26962696
if ( attributeNames.length != 0 ) {
2697-
final boolean[] propertyUpdateability = entityMetamodel.getPropertyUpdateability();
2697+
final boolean[] propertyCheckability = entityMetamodel.getPropertyCheckability();
26982698
if ( superMappingType == null ) {
26992699
/*
27002700
Sort attribute names so that we can traverse mappings efficiently
@@ -2721,7 +2721,7 @@ class ChildEntity extends SuperEntity {
27212721
final String attributeName = attributeMapping.getAttributeName();
27222722
if ( isPrefix( attributeMapping, attributeNames[index] ) ) {
27232723
final int position = attributeMapping.getStateArrayPosition();
2724-
if ( propertyUpdateability[position] && !fields.contains( position ) ) {
2724+
if ( propertyCheckability[position] && !fields.contains( position ) ) {
27252725
fields.add( position );
27262726
}
27272727
index++;
@@ -2745,7 +2745,7 @@ class ChildEntity extends SuperEntity {
27452745
else {
27462746
for ( String attributeName : attributeNames ) {
27472747
final Integer index = entityMetamodel.getPropertyIndexOrNull( attributeName );
2748-
if ( index != null && propertyUpdateability[index] && !fields.contains( index ) ) {
2748+
if ( index != null && propertyCheckability[index] && !fields.contains( index ) ) {
27492749
fields.add( index );
27502750
}
27512751
}

‎hibernate-core/src/main/java/org/hibernate/sql/results/graph/collection/internal/AbstractImmediateCollectionInitializer.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
public abstract class AbstractImmediateCollectionInitializer<Data extends AbstractImmediateCollectionInitializer.ImmediateCollectionInitializerData>
4343
extends AbstractCollectionInitializer<Data> implements BiConsumer<Data, List<Object>> {
4444

45+
private final boolean isReadOnly;
4546
/**
4647
* refers to the rows entry in the collection. null indicates that the collection is empty
4748
*/
@@ -86,6 +87,7 @@ public AbstractImmediateCollectionInitializer(
8687
this.collectionValueKeyResultAssembler = collectionKeyResult == collectionValueKeyResult
8788
? null
8889
: collectionValueKeyResult.createResultAssembler( this, creationState );
90+
this.isReadOnly = collectionAttributeMapping.isReadOnly();
8991
}
9092

9193
@Override
@@ -324,7 +326,13 @@ protected void setMissing(Data data) {
324326
public void resolveInstance(Object instance, Data data) {
325327
assert data.getState() == State.UNINITIALIZED || instance == data.getCollectionInstance();
326328
if ( instance == null ) {
327-
setMissing( data );
329+
if ( isReadOnly ) {
330+
// When the mapping is read-only, we can't trust the state of the persistence context
331+
resolveKey( data );
332+
}
333+
else {
334+
setMissing( data );
335+
}
328336
return;
329337
}
330338
final RowProcessingState rowProcessingState = data.getRowProcessingState();

0 commit comments

Comments
 (0)