Skip to content

HHH-15848 Session.isDirty() and more (rebase) #9476

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Dec 20, 2024
Merged

HHH-15848 Session.isDirty() and more (rebase) #9476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4c26596
extract a method
gavinking Dec 5, 2024
5bd3e6b
whitespace
gavinking Dec 5, 2024
54ed8db
HHH-15848 suppress some exceptions when calling Session.isDirty()
gavinking Dec 5, 2024
623d5dc
HHH-15848 completely new impl of DefaultDirtyCheckEventListener
gavinking Dec 19, 2024
a6203b8
javadoc fixups
gavinking Dec 5, 2024
77ae2cf
remove return value of Callback.performCallback, since it was always …
gavinking Dec 20, 2024
3501cfd
extract two methods
gavinking Dec 20, 2024
f1a5b5b
HHH-15848 clean up unnecessary logging and tx stuff in Session.isDirty()
gavinking Dec 20, 2024
7b2b52e
get rid of useless "throws HibernateException" in SessionImpl
gavinking Dec 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions hibernate-core/src/main/java/org/hibernate/Session.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,8 @@ public interface Session extends SharedSessionContract, EntityManager {
* the database? In other words, would any DML operations be executed if
* we flushed this session?
*
* @return {@code true} if the session contains pending changes; {@code false} otherwise.
* @throws HibernateException could not perform dirtying checking
* @return {@code true} if the session contains pending changes;
* {@code false} otherwise.
*/
boolean isDirty();

Expand Down
38 changes: 22 additions & 16 deletions hibernate-core/src/main/java/org/hibernate/boot/model/internal/EntityBinder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
import org.hibernate.models.internal.ClassTypeDetailsImpl;
import org.hibernate.models.spi.AnnotationTarget;
import org.hibernate.models.spi.ClassDetails;
import org.hibernate.models.spi.ClassDetailsRegistry;
import org.hibernate.models.spi.MemberDetails;
import org.hibernate.models.spi.SourceModelBuildingContext;
import org.hibernate.models.spi.TypeDetails;
Expand Down Expand Up @@ -480,22 +481,9 @@ private boolean mapAsIdClass(
// properties and create an identifier mapper containing the id properties of the main entity
final ClassDetails classWithIdClass = inheritanceState.getClassWithIdClass( false );
if ( classWithIdClass != null ) {
final IdClass idClassAnn = classWithIdClass.getDirectAnnotationUsage( IdClass.class );
final ClassDetails compositeClass;
if ( idClassAnn == null ) {
try {
compositeClass = getMetadataCollector().getSourceModelBuildingContext()
.getClassDetailsRegistry()
.resolveClassDetails( inheritanceState.getClassDetails().getClassName() + "_$Id" );
}
catch (RuntimeException e) {
return false;
}
}
else {
final Class<?> idClassValue = idClassAnn.value();
compositeClass = getMetadataCollector().getSourceModelBuildingContext()
.getClassDetailsRegistry().resolveClassDetails( idClassValue.getName() );
final ClassDetails compositeClass = idClassDetails( inheritanceState, classWithIdClass );
if ( compositeClass == null ) {
return false;
}
final TypeDetails compositeType = new ClassTypeDetailsImpl( compositeClass, TypeDetails.Kind.CLASS );
final TypeDetails classWithIdType = new ClassTypeDetailsImpl( classWithIdClass, TypeDetails.Kind.CLASS );
Expand Down Expand Up @@ -557,6 +545,24 @@ private boolean mapAsIdClass(
}
}

private ClassDetails idClassDetails(InheritanceState inheritanceState, ClassDetails classWithIdClass) {
final IdClass idClassAnn = classWithIdClass.getDirectAnnotationUsage( IdClass.class );
final ClassDetailsRegistry classDetailsRegistry = getSourceModelContext().getClassDetailsRegistry();
if ( idClassAnn == null ) {
try {
// look for an Id class generated by Hibernate Processor as an inner class of static metamodel
final String generatedIdClassName = inheritanceState.getClassDetails().getClassName() + "_$Id";
return classDetailsRegistry.resolveClassDetails( generatedIdClassName );
}
catch (RuntimeException e) {
return null;
}
}
else {
return classDetailsRegistry.resolveClassDetails( idClassAnn.value().getName() );
}
}

private Component createMapperProperty(
Map<ClassDetails, InheritanceState> inheritanceStates,
PersistentClass persistentClass,
Expand Down
77 changes: 41 additions & 36 deletions hibernate-core/src/main/java/org/hibernate/engine/internal/AbstractEntityEntry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,49 +357,54 @@ public boolean requiresDirtyCheck(Object entity) {
}

private boolean isUnequivocallyNonDirty(Object entity) {
if ( isSelfDirtinessTracker( entity ) ) {
final boolean uninitializedProxy;
if ( isPersistentAttributeInterceptable( entity ) ) {
final PersistentAttributeInterceptor interceptor =
asPersistentAttributeInterceptable( entity ).$$_hibernate_getInterceptor();
if ( interceptor instanceof EnhancementAsProxyLazinessInterceptor ) {
EnhancementAsProxyLazinessInterceptor enhancementAsProxyLazinessInterceptor =
(EnhancementAsProxyLazinessInterceptor) interceptor;
return !enhancementAsProxyLazinessInterceptor.hasWrittenFieldNames(); //EARLY EXIT!
}
else {
uninitializedProxy = false;
}
return isSelfDirtinessTracker( entity )
? isNonDirtyViaTracker( entity )
: isNonDirtyViaCustomStrategy( entity );
}

private boolean isNonDirtyViaCustomStrategy(Object entity) {
if ( isPersistentAttributeInterceptable( entity ) ) {
final PersistentAttributeInterceptor interceptor =
asPersistentAttributeInterceptable( entity ).$$_hibernate_getInterceptor();
if ( interceptor instanceof EnhancementAsProxyLazinessInterceptor ) {
// we never have to check an uninitialized proxy
// TODO: why do we not check !lazinessInterceptor.hasWrittenFieldNames()
// as we do below in isNonDirtyViaTracker() ?
Comment on lines +371 to +372
Copy link
Member Author

@gavinking gavinking Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dreab8 This is a question for you.

Copy link
Member

@dreab8 dreab8 Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gavinking with dirty checking setting a property will not trigger the initialization, while without dirty checking setting a property triggers the enhamced proxy initilaization and then the interceptor would not be of type EnhancementAsProxyLazinessInterceptor

Copy link
Member

@dreab8 dreab8 Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

basically writtenFieldNames is populated only when dirty checking is enabled

Copy link
Member Author

@gavinking gavinking Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dreab8 I guess I don't understand what is the difference between the two branches. They both do:

if ( interceptor instanceof EnhancementAsProxyLazinessInterceptor ) {

and then one of them does the additional check, and one doesn't.

Copy link
Member

@dreab8 dreab8 Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gavinking one branch deals wit the case dirty checking is not enabled, in this case it's enough interceptor instanceof EnhancementAsProxyLazinessInterceptor to be sure it's not dirty, the other branch deals with the case dirty checking is enabled and if interceptor instanceof EnhancementAsProxyLazinessInterceptor we need also to be sure it has not written field names to say that it's not dirty

return true;
}
else if ( isHibernateProxy( entity ) ) {
uninitializedProxy = extractLazyInitializer( entity ).isUninitialized();
}

final SessionImplementor session = getPersistenceContext().getSession().asSessionImplementor();
final CustomEntityDirtinessStrategy customEntityDirtinessStrategy =
session.getFactory().getCustomEntityDirtinessStrategy();
return customEntityDirtinessStrategy.canDirtyCheck( entity, getPersister(), session )
&& !customEntityDirtinessStrategy.isDirty( entity, getPersister(), session );
}

private boolean isNonDirtyViaTracker(Object entity) {
final boolean uninitializedProxy;
if ( isPersistentAttributeInterceptable( entity ) ) {
final PersistentAttributeInterceptor interceptor =
asPersistentAttributeInterceptable( entity ).$$_hibernate_getInterceptor();
if ( interceptor instanceof EnhancementAsProxyLazinessInterceptor lazinessInterceptor ) {
return !lazinessInterceptor.hasWrittenFieldNames();
}
else {
uninitializedProxy = false;
}
// we never have to check an uninitialized proxy
return uninitializedProxy
|| !persister.hasCollections()
&& !persister.hasMutableProperties()
&& !asSelfDirtinessTracker( entity ).$$_hibernate_hasDirtyAttributes()
&& asManagedEntity( entity ).$$_hibernate_useTracker();
}
else {
if ( isPersistentAttributeInterceptable( entity ) ) {
final PersistentAttributeInterceptor interceptor =
asPersistentAttributeInterceptable( entity ).$$_hibernate_getInterceptor();
if ( interceptor instanceof EnhancementAsProxyLazinessInterceptor ) {
// we never have to check an uninitialized proxy
return true; //EARLY EXIT!
}
}

final SessionImplementor session = getPersistenceContext().getSession().asSessionImplementor();
final CustomEntityDirtinessStrategy customEntityDirtinessStrategy =
session.getFactory().getCustomEntityDirtinessStrategy();
return customEntityDirtinessStrategy.canDirtyCheck( entity, getPersister(), session )
&& !customEntityDirtinessStrategy.isDirty( entity, getPersister(), session );
else if ( isHibernateProxy( entity ) ) {
uninitializedProxy = extractLazyInitializer( entity ).isUninitialized();
}
else {
uninitializedProxy = false;
}
// we never have to check an uninitialized proxy
return uninitializedProxy
|| !persister.hasCollections()
&& !persister.hasMutableProperties()
&& !asSelfDirtinessTracker( entity ).$$_hibernate_hasDirtyAttributes()
&& asManagedEntity( entity ).$$_hibernate_useTracker();
}

@Override
Expand Down
93 changes: 42 additions & 51 deletions hibernate-core/src/main/java/org/hibernate/engine/spi/CollectionEntry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import org.hibernate.collection.spi.PersistentCollection;
import org.hibernate.internal.CoreLogging;
import org.hibernate.internal.CoreMessageLogger;
import org.hibernate.internal.util.NullnessUtil;
import org.hibernate.persister.collection.CollectionPersister;

import java.io.IOException;
Expand All @@ -20,6 +19,7 @@

import org.checkerframework.checker.nullness.qual.Nullable;

import static org.hibernate.internal.util.NullnessUtil.castNonNull;
import static org.hibernate.pretty.MessageHelper.collectionInfoString;

/**
Expand Down Expand Up @@ -68,11 +68,11 @@
// during flush shouldn't be ignored
ignore = false;

collection.clearDirty(); //a newly wrapped collection is NOT dirty (or we get unnecessary version updates)
// a newly wrapped collection is NOT dirty
// (or we get unnecessary version updates)
collection.clearDirty();

snapshot = persister.isMutable() ?
collection.getSnapshot( persister ) :
null;
snapshot = persister.isMutable() ? collection.getSnapshot( persister ) : null;
collection.setSnapshot( loadedKey, role, snapshot );
}

Expand Down Expand Up @@ -123,7 +123,7 @@

loadedKey = collection.getKey();
role = collection.getRole();
loadedPersister = factory.getRuntimeMetamodels().getMappingMetamodel().getCollectionDescriptor( NullnessUtil.castNonNull( role ) );
loadedPersister = factory.getMappingMetamodel().getCollectionDescriptor( castNonNull( role ) );

snapshot = collection.getStoredSnapshot();
}
Expand Down Expand Up @@ -156,12 +156,11 @@
final CollectionPersister loadedPersister = getLoadedPersister();
final boolean forceDirty =
collection.wasInitialized()
&& !collection.isDirty() //optimization
&& loadedPersister != null
&& loadedPersister.isMutable() //optimization
&& ( collection.isDirectlyAccessible() || loadedPersister.getElementType().isMutable() ) //optimization
&& !collection.equalsSnapshot( loadedPersister );

&& !collection.isDirty() //optimization
&& loadedPersister != null
&& loadedPersister.isMutable() //optimization
&& ( collection.isDirectlyAccessible() || loadedPersister.getElementType().isMutable() ) //optimization

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation Note

Invoking
CollectionPersister.getElementType
Loading
should be avoided because it has been deprecated.
&& !collection.equalsSnapshot( loadedPersister );
if ( forceDirty ) {
collection.dirty();
}
Expand All @@ -174,23 +173,19 @@
}

final CollectionPersister loadedPersister = getLoadedPersister();
boolean nonMutableChange = collection.isDirty()
&& loadedPersister != null
&& !loadedPersister.isMutable();
final boolean nonMutableChange =
collection.isDirty()
&& loadedPersister != null
&& !loadedPersister.isMutable();
if ( nonMutableChange ) {
throw new HibernateException(
"changed an immutable collection instance: " +
collectionInfoString( NullnessUtil.castNonNull( loadedPersister ).getRole(), getLoadedKey() )
);
throw new HibernateException( "changed an immutable collection instance: " +
collectionInfoString( castNonNull( loadedPersister ).getRole(), getLoadedKey() ) );
}

dirty( collection );

if ( LOG.isDebugEnabled() && collection.isDirty() && loadedPersister != null ) {
LOG.debugf(
"Collection dirty: %s",
collectionInfoString( loadedPersister.getRole(), getLoadedKey() )
);
LOG.debug( "Collection dirty: " + collectionInfoString( loadedPersister.getRole(), getLoadedKey() ) );
}

setReached( false );
Expand All @@ -208,9 +203,9 @@
? collection.getSnapshot( loadedPersister )
: null;
collection.setSnapshot( loadedKey, role, snapshot );
if ( loadedPersister != null && session.getLoadQueryInfluencers().effectivelyBatchLoadable( loadedPersister ) ) {
session.getPersistenceContextInternal()
.getBatchFetchQueue()
if ( loadedPersister != null
&& session.getLoadQueryInfluencers().effectivelyBatchLoadable( loadedPersister ) ) {
session.getPersistenceContextInternal().getBatchFetchQueue()
.removeBatchLoadableCollection( this );
}
}
Expand All @@ -236,12 +231,12 @@
loadedKey = getCurrentKey();
setLoadedPersister( getCurrentPersister() );

boolean resnapshot = collection.wasInitialized()
final boolean resnapshot = collection.wasInitialized()
&& ( isDoremove() || isDorecreate() || isDoupdate() );
if ( resnapshot ) {
snapshot = loadedPersister == null || !loadedPersister.isMutable() ?
null :
collection.getSnapshot( NullnessUtil.castNonNull( loadedPersister ) ); //re-snapshot
snapshot = loadedPersister != null && loadedPersister.isMutable()
? collection.getSnapshot( castNonNull( loadedPersister ) )
: null; //re-snapshot
}

collection.postAction();
Expand Down Expand Up @@ -271,13 +266,11 @@
public void resetStoredSnapshot(PersistentCollection<?> collection, Serializable storedSnapshot) {
LOG.debugf("Reset storedSnapshot to %s for %s", storedSnapshot, this);

if ( fromMerge ) {
return; // EARLY EXIT!
if ( !fromMerge ) {
snapshot = storedSnapshot;
collection.setSnapshot( loadedKey, role, snapshot );
fromMerge = true;
}

snapshot = storedSnapshot;
collection.setSnapshot( loadedKey, role, snapshot );
fromMerge = true;
}

private void setLoadedPersister(@Nullable CollectionPersister persister) {
Expand All @@ -286,12 +279,9 @@
}

void afterDeserialize(@Nullable SessionFactoryImplementor factory) {
if ( factory == null ) {
loadedPersister = null;
}
else {
loadedPersister = factory.getRuntimeMetamodels().getMappingMetamodel().getCollectionDescriptor( NullnessUtil.castNonNull( role ) );
}
loadedPersister = factory == null ? null
: factory.getRuntimeMetamodels().getMappingMetamodel()
.getCollectionDescriptor( castNonNull( role ) );
}

public boolean wasDereferenced() {
Expand Down Expand Up @@ -379,13 +369,15 @@

@Override
public String toString() {
String result = "CollectionEntry" +
collectionInfoString( role, loadedKey );
if ( currentPersister != null ) {
result += "->" +
collectionInfoString( currentPersister.getRole(), currentKey );
final StringBuilder result =
new StringBuilder( "CollectionEntry" )
.append( collectionInfoString( role, loadedKey ) );
final CollectionPersister persister = currentPersister;
if ( persister != null ) {
result.append( "->" )
.append( collectionInfoString( persister.getRole(), currentKey ) );
}
return result;
return result.toString();
}

/**
Expand Down Expand Up @@ -432,9 +424,8 @@
*
* @return The deserialized CollectionEntry
*/
public static CollectionEntry deserialize(
ObjectInputStream ois,
SessionImplementor session) throws IOException, ClassNotFoundException {
public static CollectionEntry deserialize(ObjectInputStream ois, SessionImplementor session)
throws IOException, ClassNotFoundException {
return new CollectionEntry(
(String) ois.readObject(),
(Serializable) ois.readObject(),
Expand Down
5 changes: 3 additions & 2 deletions hibernate-core/src/main/java/org/hibernate/engine/spi/Status.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
/**
* Represents the status of an entity with respect to
* this session. These statuses are for internal
* book-keeping only and are not intended to represent
* any notion that is visible to the _application_.
* bookkeeping only and are not intended to represent
* any notion that is visible to the application
* program.
*/
public enum Status {
MANAGED,
Expand Down
Loading
Loading