Skip to content
This repository was archived by the owner on Mar 13, 2025. It is now read-only.

Reset embedded dirty checking #169

Merged
merged 2 commits into from
Jan 31, 2020
Merged

Reset embedded dirty checking #169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c2dc1ef
grails/grails-data-mapping#1272 Embedded object shouldn't be dirty af…
puneetbehl Jan 29, 2020
94d67a9
grails/grails-data-mapping#1272 Reset dirty in embedded objects
puneetbehl Jan 29, 2020
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
28 changes: 28 additions & 0 deletions ...nate5/src/main/groovy/org/grails/orm/hibernate/dirty/GrailsEntityDirtinessStrategy.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import org.hibernate.Session
import org.hibernate.engine.spi.SessionImplementor
import org.hibernate.engine.spi.Status
import org.hibernate.persister.entity.EntityPersister
import org.slf4j.Logger
import org.slf4j.LoggerFactory

/**
* A class to customize Hibernate dirtiness based on Grails {@link DirtyCheckable} interface
Expand All @@ -42,6 +44,8 @@ import org.hibernate.persister.entity.EntityPersister
@CompileStatic
class GrailsEntityDirtinessStrategy implements CustomEntityDirtinessStrategy {

protected static final Logger LOG = LoggerFactory.getLogger(GrailsEntityDirtinessStrategy.class)

@Override
boolean canDirtyCheck(Object entity, EntityPersister persister, Session session) {
return entity instanceof DirtyCheckable
Expand All @@ -56,6 +60,30 @@ class GrailsEntityDirtinessStrategy implements CustomEntityDirtinessStrategy {
void resetDirty(Object entity, EntityPersister persister, Session session) {
if (canDirtyCheck(entity, persister, session)) {
cast(entity).trackChanges()
try {
PersistentEntity persistentEntity = GormEnhancer.findEntity(Hibernate.getClass(entity))
if (persistentEntity != null) {
resetDirtyEmbeddedObjects(persistentEntity, entity, persister, session)
}
} catch (IllegalStateException e) {
if (LOG.isDebugEnabled()) {
LOG.debug(e.message, e)
Copy link
Member

@graemerocher graemerocher Jan 29, 2020

Choose a reason for hiding this comment

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

Why are we swallowing the exception here and logging at debug level?

Copy link
Contributor Author

@puneetbehl puneetbehl Jan 29, 2020

Choose a reason for hiding this comment

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

With this PR, we are recursively resetting the $changedProperties of embedded objects. In cases where the embedded object is not a PersistentEntity the recursion will break, and IllegalStateException is thrown as:

Either class [grails.gorm.tests.dirtychecking.Address] is not a domain class or GORM has not been initialized correctly or has already been shutdown. Ensure GORM is loaded and configured correctly before calling any methods on a GORM entity.

}
}
}
}

private void resetDirtyEmbeddedObjects(PersistentEntity persistentEntity,
Object entity,
EntityPersister persister,
Session session) {

if (DirtyCheckingSupport.areEmbeddedDirty(persistentEntity, entity)) {
final associations = persistentEntity.getEmbedded()
for (Embedded a in associations) {
final value = a.reader.read(entity)
resetDirty(value, persister, session)
Copy link
Member

@graemerocher graemerocher Jan 29, 2020

Choose a reason for hiding this comment

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

What if value is null?

Copy link
Contributor Author

@puneetbehl puneetbehl Jan 29, 2020

Choose a reason for hiding this comment

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

If the value is null then we do not need to reset $changedProperties on embedded. So, the check at line#81 DirtyCheckingSupport.areEmbeddedDirty(persistentEntity, entity) would return false.

}
}
}

Expand Down
7 changes: 6 additions & 1 deletion ...ernate5/src/test/groovy/grails/gorm/tests/dirtychecking/HibernateDirtyCheckingSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import grails.gorm.annotation.Entity
import grails.gorm.dirty.checking.DirtyCheck
import grails.gorm.transactions.Rollback
import org.grails.orm.hibernate.HibernateDatastore
import org.springframework.transaction.PlatformTransactionManager
import spock.lang.AutoCleanup
import spock.lang.Issue
import spock.lang.Shared
Expand Down Expand Up @@ -67,6 +66,12 @@ class HibernateDirtyCheckingSpec extends Specification {

when:
person.save(flush:true)

then:
!person.address.hasChanged()
person.address.listDirtyPropertyNames().isEmpty()

when:
hibernateDatastore.sessionFactory.currentSession.clear()
person = Person.first()

Expand Down