From a180caecd68dbe992af2f7c3c65916ffb6f4d23a Mon Sep 17 00:00:00 2001 From: Ulrich Bestfleisch Date: Thu, 21 Jun 2018 14:45:44 +0200 Subject: [PATCH] HHH-12718 - Entity changes in @PreUpdate callback are not persisted when lazy loading is active for more than one field --- .../DefaultFlushEntityEventListener.java | 17 +- .../PreUpdateBytecodeEnhancementTest.java | 127 ++++++++++++ ...dateCustomEntityDirtinessStrategyTest.java | 180 ++++++++++++++++++ ...PreUpdateDirtyCheckingInterceptorTest.java | 155 +++++++++++++++ 4 files changed, 473 insertions(+), 6 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/jpa/test/callbacks/PreUpdateBytecodeEnhancementTest.java create mode 100644 hibernate-core/src/test/java/org/hibernate/jpa/test/callbacks/PreUpdateCustomEntityDirtinessStrategyTest.java create mode 100644 hibernate-core/src/test/java/org/hibernate/jpa/test/callbacks/PreUpdateDirtyCheckingInterceptorTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultFlushEntityEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultFlushEntityEventListener.java index 93694ebae0..8e26a83c91 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultFlushEntityEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultFlushEntityEventListener.java @@ -332,15 +332,20 @@ public class DefaultFlushEntityEventListener implements FlushEntityEventListener final boolean intercepted = invokeInterceptor( session, entity, entry, values, persister ); //now we might need to recalculate the dirtyProperties array - if ( intercepted && event.isDirtyCheckPossible() && !event.isDirtyCheckHandledByInterceptor() ) { - int[] dirtyProperties; - if ( event.hasDatabaseSnapshot() ) { - dirtyProperties = persister.findModified( event.getDatabaseSnapshot(), values, entity, session ); + if ( intercepted && event.isDirtyCheckPossible() ) { + if ( !event.isDirtyCheckHandledByInterceptor() ) { + int[] dirtyProperties; + if ( event.hasDatabaseSnapshot() ) { + dirtyProperties = persister.findModified( event.getDatabaseSnapshot(), values, entity, session ); + } + else { + dirtyProperties = persister.findDirty( values, entry.getLoadedState(), entity, session ); + } + event.setDirtyProperties( dirtyProperties ); } else { - dirtyProperties = persister.findDirty( values, entry.getLoadedState(), entity, session ); + dirtyCheck( event ); } - event.setDirtyProperties( dirtyProperties ); } return intercepted; diff --git a/hibernate-core/src/test/java/org/hibernate/jpa/test/callbacks/PreUpdateBytecodeEnhancementTest.java b/hibernate-core/src/test/java/org/hibernate/jpa/test/callbacks/PreUpdateBytecodeEnhancementTest.java new file mode 100644 index 0000000000..0e9ab1b321 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/jpa/test/callbacks/PreUpdateBytecodeEnhancementTest.java @@ -0,0 +1,127 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.jpa.test.callbacks; + +import java.nio.ByteBuffer; +import java.time.Instant; +import java.util.List; +import java.util.Map; +import javax.persistence.Basic; +import javax.persistence.ElementCollection; +import javax.persistence.Entity; +import javax.persistence.FetchType; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.Lob; +import javax.persistence.PrePersist; +import javax.persistence.PreUpdate; + +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.bytecode.enhancement.BytecodeEnhancerRunner; +import org.junit.Test; +import org.junit.runner.RunWith; + +import static org.hibernate.testing.transaction.TransactionUtil.doInJPA; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +@TestForIssue(jiraKey = "HHH-12718") +@RunWith(BytecodeEnhancerRunner.class) +public class PreUpdateBytecodeEnhancementTest extends BaseEntityManagerFunctionalTestCase { + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { Person.class }; + } + + @Override + protected void addConfigOptions(Map options) { + options.put( AvailableSettings.CLASSLOADERS, getClass().getClassLoader() ); + options.put( AvailableSettings.ENHANCER_ENABLE_LAZY_INITIALIZATION, "true" ); + options.put( AvailableSettings.ENHANCER_ENABLE_DIRTY_TRACKING, "true" ); + } + + @Test + public void testPreUpdateModifications() { + Person person = new Person(); + + doInJPA( this::entityManagerFactory, entityManager -> { + entityManager.persist( person ); + } ); + + doInJPA( this::entityManagerFactory, entityManager -> { + Person p = entityManager.find( Person.class, person.id ); + assertNotNull( p ); + assertNotNull( p.createdAt ); + assertNull( p.lastUpdatedAt ); + + p.setName( "Changed Name" ); + } ); + + doInJPA( this::entityManagerFactory, entityManager -> { + Person p = entityManager.find( Person.class, person.id ); + assertNotNull( p.lastUpdatedAt ); + } ); + } + + @Entity(name = "Person") + private static class Person { + @Id + @GeneratedValue + private int id; + + private String name; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + private Instant createdAt; + + public Instant getCreatedAt() { + return createdAt; + } + + public void setCreatedAt(Instant createdAt) { + this.createdAt = createdAt; + } + + private Instant lastUpdatedAt; + + public Instant getLastUpdatedAt() { + return lastUpdatedAt; + } + + public void setLastUpdatedAt(Instant lastUpdatedAt) { + this.lastUpdatedAt = lastUpdatedAt; + } + + @ElementCollection + private List tags; + + @Lob + @Basic(fetch = FetchType.LAZY) + private ByteBuffer image; + + @PrePersist + void beforeCreate() { + this.setCreatedAt( Instant.now() ); + } + + @PreUpdate + void beforeUpdate() { + this.setLastUpdatedAt( Instant.now() ); + } + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/jpa/test/callbacks/PreUpdateCustomEntityDirtinessStrategyTest.java b/hibernate-core/src/test/java/org/hibernate/jpa/test/callbacks/PreUpdateCustomEntityDirtinessStrategyTest.java new file mode 100644 index 0000000000..bb45944c81 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/jpa/test/callbacks/PreUpdateCustomEntityDirtinessStrategyTest.java @@ -0,0 +1,180 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.jpa.test.callbacks; + +import java.nio.ByteBuffer; +import java.time.Instant; +import java.util.List; +import java.util.Map; +import javax.persistence.Basic; +import javax.persistence.ElementCollection; +import javax.persistence.Entity; +import javax.persistence.FetchType; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.Lob; +import javax.persistence.PrePersist; +import javax.persistence.PreUpdate; + +import org.hibernate.CustomEntityDirtinessStrategy; +import org.hibernate.Session; +import org.hibernate.annotations.DynamicUpdate; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.persister.entity.EntityPersister; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase; +import org.junit.Test; + +import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +@TestForIssue(jiraKey = "HHH-12718") +public class PreUpdateCustomEntityDirtinessStrategyTest + extends BaseNonConfigCoreFunctionalTestCase { + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { Person.class }; + } + + @Test + public void testPreUpdateModifications() { + Person person = new Person(); + + doInHibernate( this::sessionFactory, session -> { + session.persist( person ); + } ); + + doInHibernate( this::sessionFactory, session -> { + Person p = session.find( Person.class, person.id ); + assertNotNull( p ); + assertNotNull( p.createdAt ); + assertNull( p.lastUpdatedAt ); + + p.setName( "Changed Name" ); + } ); + + doInHibernate( this::sessionFactory, session -> { + Person p = session.find( Person.class, person.id ); + assertNotNull( p.lastUpdatedAt ); + } ); + + assertTrue( DefaultCustomEntityDirtinessStrategy.INSTANCE.isPersonNameChanged() ); + assertTrue( DefaultCustomEntityDirtinessStrategy.INSTANCE.isPersonLastUpdatedAtChanged() ); + } + + @Override + protected void addSettings(Map settings) { + settings.put( AvailableSettings.CUSTOM_ENTITY_DIRTINESS_STRATEGY, DefaultCustomEntityDirtinessStrategy.INSTANCE ); + } + + public static class DefaultCustomEntityDirtinessStrategy + implements CustomEntityDirtinessStrategy { + private static final DefaultCustomEntityDirtinessStrategy INSTANCE = + new DefaultCustomEntityDirtinessStrategy(); + + private boolean personNameChanged = false; + private boolean personLastUpdatedAtChanged = false; + + @Override + public boolean canDirtyCheck(Object entity, EntityPersister persister, Session session) { + return true; + } + + @Override + public boolean isDirty(Object entity, EntityPersister persister, Session session) { + Person person = (Person) entity; + if ( !personNameChanged ) { + personNameChanged = person.getName() != null; + return personNameChanged; + } + if ( !personLastUpdatedAtChanged ) { + personLastUpdatedAtChanged = person.getLastUpdatedAt() != null; + return personLastUpdatedAtChanged; + } + return false; + } + + @Override + public void resetDirty(Object entity, EntityPersister persister, Session session) { + } + + @Override + public void findDirty( + Object entity, + EntityPersister persister, + Session session, + DirtyCheckContext dirtyCheckContext) { + } + + public boolean isPersonNameChanged() { + return personNameChanged; + } + + public boolean isPersonLastUpdatedAtChanged() { + return personLastUpdatedAtChanged; + } + } + + @Entity(name = "Person") + @DynamicUpdate + private static class Person { + @Id + @GeneratedValue + private int id; + + private String name; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + private Instant createdAt; + + public Instant getCreatedAt() { + return createdAt; + } + + public void setCreatedAt(Instant createdAt) { + this.createdAt = createdAt; + } + + private Instant lastUpdatedAt; + + public Instant getLastUpdatedAt() { + return lastUpdatedAt; + } + + public void setLastUpdatedAt(Instant lastUpdatedAt) { + this.lastUpdatedAt = lastUpdatedAt; + } + + @ElementCollection + private List tags; + + @Lob + @Basic(fetch = FetchType.LAZY) + private ByteBuffer image; + + @PrePersist + void beforeCreate() { + this.setCreatedAt( Instant.now() ); + } + + @PreUpdate + void beforeUpdate() { + this.setLastUpdatedAt( Instant.now() ); + } + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/jpa/test/callbacks/PreUpdateDirtyCheckingInterceptorTest.java b/hibernate-core/src/test/java/org/hibernate/jpa/test/callbacks/PreUpdateDirtyCheckingInterceptorTest.java new file mode 100644 index 0000000000..93a3f80697 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/jpa/test/callbacks/PreUpdateDirtyCheckingInterceptorTest.java @@ -0,0 +1,155 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.jpa.test.callbacks; + +import java.io.Serializable; +import java.nio.ByteBuffer; +import java.time.Instant; +import java.util.List; +import java.util.Objects; +import javax.persistence.Basic; +import javax.persistence.ElementCollection; +import javax.persistence.Entity; +import javax.persistence.FetchType; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.Lob; +import javax.persistence.PrePersist; +import javax.persistence.PreUpdate; + +import org.hibernate.EmptyInterceptor; +import org.hibernate.SessionBuilder; +import org.hibernate.annotations.DynamicUpdate; +import org.hibernate.type.Type; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase; +import org.junit.Test; + +import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate; +import static org.hibernate.testing.transaction.TransactionUtil.doInHibernateSessionBuilder; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +@TestForIssue(jiraKey = "HHH-12718") +public class PreUpdateDirtyCheckingInterceptorTest + extends BaseNonConfigCoreFunctionalTestCase { + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { Person.class }; + } + + @Test + public void testPreUpdateModifications() { + Person person = new Person(); + + doInHibernate( this::sessionFactory, session -> { + session.persist( person ); + } ); + + doInHibernateSessionBuilder( this::sessionWithInterceptor, session -> { + Person p = session.find( Person.class, person.id ); + assertNotNull( p ); + assertNotNull( p.createdAt ); + assertNull( p.lastUpdatedAt ); + + p.setName( "Changed Name" ); + } ); + + doInHibernate( this::sessionFactory, session -> { + Person p = session.find( Person.class, person.id ); + assertNotNull( p.lastUpdatedAt ); + } ); + } + + public static class OnFlushDirtyInterceptor extends EmptyInterceptor { + + private static OnFlushDirtyInterceptor INSTANCE = new OnFlushDirtyInterceptor(); + + @Override + public int[] findDirty( + Object entity, + Serializable id, + Object[] currentState, + Object[] previousState, + String[] propertyNames, + Type[] types) { + int[] result = new int[propertyNames.length]; + int span = 0; + + for ( int i = 0; i < previousState.length; i++ ) { + if( !Objects.deepEquals(previousState[i], currentState[i])) { + result[span++] = i; + } + } + + return result; + } + } + + private SessionBuilder sessionWithInterceptor() { + return sessionFactory() + .withOptions() + .interceptor( OnFlushDirtyInterceptor.INSTANCE ); + } + + @Entity(name = "Person") + @DynamicUpdate + private static class Person { + @Id + @GeneratedValue + private int id; + + private String name; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + private Instant createdAt; + + public Instant getCreatedAt() { + return createdAt; + } + + public void setCreatedAt(Instant createdAt) { + this.createdAt = createdAt; + } + + private Instant lastUpdatedAt; + + public Instant getLastUpdatedAt() { + return lastUpdatedAt; + } + + public void setLastUpdatedAt(Instant lastUpdatedAt) { + this.lastUpdatedAt = lastUpdatedAt; + } + + @ElementCollection + private List tags; + + @Lob + @Basic(fetch = FetchType.LAZY) + private ByteBuffer image; + + @PrePersist + void beforeCreate() { + this.setCreatedAt( Instant.now() ); + } + + @PreUpdate + void beforeUpdate() { + this.setLastUpdatedAt( Instant.now() ); + } + } +}