From 0a2bb3c811806d569790cc70f653736d56dccba8 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Thu, 10 Nov 2016 12:21:19 -0600 Subject: [PATCH] HHH-11155 - Lazy properties are not updated if not all lazy properties (e.g. collections) are initialized --- .../interceptor/LazyAttributesMetadata.java | 15 +- .../entity/AbstractEntityPersister.java | 29 +++- .../tuple/entity/EntityMetamodel.java | 5 +- .../bytecode/enhancement/EnhancerTest.java | 8 +- .../lazy/group/LazyGroupUpdateTestTask.java | 2 +- .../group/SimpleLazyGroupUpdateTestTask.java | 162 ++++++++++++++++++ 6 files changed, 203 insertions(+), 18 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/group/SimpleLazyGroupUpdateTestTask.java diff --git a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/interceptor/LazyAttributesMetadata.java b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/interceptor/LazyAttributesMetadata.java index 8480cd83f0..312bea07fd 100644 --- a/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/interceptor/LazyAttributesMetadata.java +++ b/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/spi/interceptor/LazyAttributesMetadata.java @@ -35,8 +35,8 @@ public class LazyAttributesMetadata implements Serializable { * @return The built LazyFetchGroupMetadata */ public static LazyAttributesMetadata from(PersistentClass mappedEntity) { - final Map lazyAttributeDescriptorMap = new LinkedHashMap(); - final Map> fetchGroupToAttributesMap = new HashMap>(); + final Map lazyAttributeDescriptorMap = new LinkedHashMap<>(); + final Map> fetchGroupToAttributesMap = new HashMap<>(); int i = -1; int x = 0; @@ -48,11 +48,10 @@ public class LazyAttributesMetadata implements Serializable { final LazyAttributeDescriptor lazyAttributeDescriptor = LazyAttributeDescriptor.from( property, i, x++ ); lazyAttributeDescriptorMap.put( lazyAttributeDescriptor.getName(), lazyAttributeDescriptor ); - Set attributeSet = fetchGroupToAttributesMap.get( lazyAttributeDescriptor.getFetchGroupName() ); - if ( attributeSet == null ) { - attributeSet = new LinkedHashSet(); - fetchGroupToAttributesMap.put( lazyAttributeDescriptor.getFetchGroupName(), attributeSet ); - } + final Set attributeSet = fetchGroupToAttributesMap.computeIfAbsent( + lazyAttributeDescriptor.getFetchGroupName(), + k -> new LinkedHashSet<>() + ); attributeSet.add( lazyAttributeDescriptor.getName() ); } } @@ -82,7 +81,7 @@ public class LazyAttributesMetadata implements Serializable { private final Map> fetchGroupToAttributeMap; public LazyAttributesMetadata(String entityName) { - this( entityName, Collections.emptyMap(), Collections.>emptyMap() ); + this( entityName, Collections.emptyMap(), Collections.emptyMap() ); } public LazyAttributesMetadata( diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java index b4c20b0de9..dc7b77a197 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java @@ -70,6 +70,7 @@ import org.hibernate.engine.spi.LoadQueryInfluencers; import org.hibernate.engine.spi.Mapping; import org.hibernate.engine.spi.PersistenceContext.NaturalIdHelper; import org.hibernate.engine.spi.PersistentAttributeInterceptable; +import org.hibernate.engine.spi.SelfDirtinessTracker; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.engine.spi.ValueInclusion; @@ -1060,26 +1061,40 @@ public abstract class AbstractEntityPersister final Object[] snapshot = entry.getLoadedState(); for ( LazyAttributeDescriptor fetchGroupAttributeDescriptor : fetchGroupAttributeDescriptors ) { final boolean previousInitialized = initializedLazyAttributeNames.contains( fetchGroupAttributeDescriptor.getName() ); - final Object loadedValue = fetchGroupAttributeDescriptor.getType().nullSafeGet( + + if ( previousInitialized ) { + // todo : one thing we should consider here is potentially un-marking an attribute as dirty based on the selected value + // we know the current value - getPropertyValue( entity, fetchGroupAttributeDescriptor.getAttributeIndex() ); + // we know the selected value (see selectedValue below) + // we can use the attribute Type to tell us if they are the same + // + // assuming entity is a SelfDirtinessTracker we can also know if the attribute is + // currently considered dirty, and if really not dirty we would do the un-marking + // + // of course that would mean a new method on SelfDirtinessTracker to allow un-marking + + // its already been initialized (e.g. by a write) so we don't want to overwrite + continue; + } + + + final Object selectedValue = fetchGroupAttributeDescriptor.getType().nullSafeGet( rs, lazyPropertyColumnAliases[fetchGroupAttributeDescriptor.getLazyIndex()], session, entity ); + final boolean set = initializeLazyProperty( fieldName, entity, session, snapshot, fetchGroupAttributeDescriptor.getLazyIndex(), - loadedValue + selectedValue ); - if ( previousInitialized ) { - // its already been initialized (e.g. by a write) so we don't want to overwrite - continue; - } if ( set ) { - result = loadedValue; + result = selectedValue; interceptor.attributeInitialized( fetchGroupAttributeDescriptor.getName() ); } diff --git a/hibernate-core/src/main/java/org/hibernate/tuple/entity/EntityMetamodel.java b/hibernate-core/src/main/java/org/hibernate/tuple/entity/EntityMetamodel.java index 878f575ccf..ca32f4aabd 100644 --- a/hibernate-core/src/main/java/org/hibernate/tuple/entity/EntityMetamodel.java +++ b/hibernate-core/src/main/java/org/hibernate/tuple/entity/EntityMetamodel.java @@ -334,8 +334,11 @@ public class EntityMetamodel implements Serializable { LOG.entityMappedAsNonAbstract(name); } } + selectBeforeUpdate = persistentClass.hasSelectBeforeUpdate(); - dynamicUpdate = persistentClass.useDynamicUpdate(); + + dynamicUpdate = persistentClass.useDynamicUpdate() + || ( getBytecodeEnhancementMetadata().isEnhancedForLazyLoading() && getBytecodeEnhancementMetadata().getLazyAttributesMetadata().getFetchGroupNames().size() > 1 ); dynamicInsert = persistentClass.useDynamicInsert(); polymorphic = persistentClass.isPolymorphic(); diff --git a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/EnhancerTest.java b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/EnhancerTest.java index b5cb4db6b4..1a9a0731ce 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/EnhancerTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/EnhancerTest.java @@ -10,6 +10,7 @@ import javassist.CtClass; import org.hibernate.test.bytecode.enhancement.association.InheritedAttributeAssociationTestTask; import org.hibernate.test.bytecode.enhancement.lazy.group.LazyGroupUpdateTestTask; +import org.hibernate.test.bytecode.enhancement.lazy.group.SimpleLazyGroupUpdateTestTask; import org.hibernate.test.bytecode.enhancement.otherentityentrycontext.OtherEntityEntryContextTestTask; import org.hibernate.testing.FailureExpected; import org.hibernate.testing.TestForIssue; @@ -137,11 +138,16 @@ public class EnhancerTest extends BaseUnitTestCase { @Test @TestForIssue( jiraKey = "HHH-11155" ) - @FailureExpected( jiraKey = "HHH-11155" ) public void testLazyGroupsUpdate() { EnhancerTestUtils.runEnhancerTestTask( LazyGroupUpdateTestTask.class ); } + @Test + @TestForIssue( jiraKey = "HHH-11155" ) + public void testLazyGroupsUpdateSimple() { + EnhancerTestUtils.runEnhancerTestTask( SimpleLazyGroupUpdateTestTask.class ); + } + @Test public void testLazyCollectionNoTransactionHandling() { EnhancerTestUtils.runEnhancerTestTask( LazyCollectionNoTransactionLoadingTestTask.class ); diff --git a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/group/LazyGroupUpdateTestTask.java b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/group/LazyGroupUpdateTestTask.java index 5eb1f6fcfe..4c1fd24473 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/group/LazyGroupUpdateTestTask.java +++ b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/group/LazyGroupUpdateTestTask.java @@ -80,7 +80,7 @@ public class LazyGroupUpdateTestTask extends AbstractEnhancerTestTask { // Now lets update nickName which ought to initialize nickName and parent, but not alternateParent c1.setNickName( "new nickName" ); assertLoaded( c1, "nickName" ); - assertLoaded( c1, "parent" ); + assertNotLoaded( c1, "parent" ); assertNotLoaded( c1, "alternateParent" ); assertEquals( "Hibernate", c1.getParent().getNombre() ); assertFalse( c1.getParent() instanceof HibernateProxy ); diff --git a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/group/SimpleLazyGroupUpdateTestTask.java b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/group/SimpleLazyGroupUpdateTestTask.java new file mode 100644 index 0000000000..576d1d229c --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/group/SimpleLazyGroupUpdateTestTask.java @@ -0,0 +1,162 @@ +/* + * 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 http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.test.bytecode.enhancement.lazy.group; + +import javax.persistence.Basic; +import javax.persistence.Entity; +import javax.persistence.FetchType; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; + +import org.hibernate.Session; +import org.hibernate.annotations.DynamicUpdate; +import org.hibernate.annotations.LazyGroup; +import org.hibernate.cfg.Configuration; +import org.hibernate.cfg.Environment; + +import org.hibernate.testing.bytecode.enhancement.EnhancerTestUtils; +import org.hibernate.test.bytecode.enhancement.AbstractEnhancerTestTask; + +import static junit.framework.Assert.assertNull; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +/** + * @author Steve Ebersole + */ +public class SimpleLazyGroupUpdateTestTask extends AbstractEnhancerTestTask { + public static final String REALLY_BIG_STRING = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"; + + @Override + public Class[] getAnnotatedClasses() { + return new Class[] { TestEntity.class }; + } + + @Override + public void prepare() { + Configuration cfg = new Configuration(); + cfg.setProperty( Environment.ENABLE_LAZY_LOAD_NO_TRANS, "true" ); + cfg.setProperty( Environment.USE_SECOND_LEVEL_CACHE, "false" ); + super.prepare( cfg ); + + Session s = getFactory().openSession(); + s.beginTransaction(); + + s.save( new TestEntity( 1, "entity 1", "blah", REALLY_BIG_STRING ) ); + + s.getTransaction().commit(); + s.close(); + } + + @Override + public void execute() { + Session s = getFactory().openSession(); + s.beginTransaction(); + + TestEntity entity = s.load( TestEntity.class, 1 ); + assertLoaded( entity, "name" ); + assertNotLoaded( entity, "lifeStory" ); + assertNotLoaded( entity, "reallyBigString" ); + + entity.setLifeStory( "blah blah blah" ); + assertLoaded( entity, "name" ); + assertLoaded( entity, "lifeStory" ); + assertNotLoaded( entity, "reallyBigString" ); + + s.getTransaction().commit(); + s.close(); + + + s = getFactory().openSession(); + s.beginTransaction(); + + entity = s.load( TestEntity.class, 1 ); + assertLoaded( entity, "name" ); + assertNotLoaded( entity, "lifeStory" ); + assertNotLoaded( entity, "reallyBigString" ); + assertEquals( "blah blah blah", entity.getLifeStory() ); + assertEquals( REALLY_BIG_STRING, entity.getReallyBigString() ); + + s.getTransaction().commit(); + s.close(); + } + + private void assertLoaded(Object owner, String name) { + // NOTE we assume null == not-loaded + Object fieldByReflection = EnhancerTestUtils.getFieldByReflection( owner, name ); + assertNotNull( "Expecting field '" + name + "' to be loaded, but it was not", fieldByReflection ); + } + + private void assertNotLoaded(Object owner, String name) { + // NOTE we assume null == not-loaded + Object fieldByReflection = EnhancerTestUtils.getFieldByReflection( owner, name ); + assertNull( "Expecting field '" + name + "' to be not loaded, but it was", fieldByReflection ); + } + + @Override + protected void cleanup() { + Session s = getFactory().openSession(); + s.beginTransaction(); + + s.createQuery( "delete TestEntity" ).executeUpdate(); + + s.getTransaction().commit(); + s.close(); + } + + @Entity( name = "TestEntity" ) + public static class TestEntity { + @Id + Integer id; + String name; + @Basic(fetch = FetchType.LAZY) + @LazyGroup( "grp1" ) + String lifeStory; + @Basic(fetch = FetchType.LAZY) + @LazyGroup( "grp2" ) + String reallyBigString; + + public TestEntity() { + } + + public TestEntity(Integer id, String name, String lifeStory, String reallyBigString) { + this.id = id; + this.name = name; + this.lifeStory = lifeStory; + this.reallyBigString = reallyBigString; + } + + public Integer getId() { + return id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getLifeStory() { + return lifeStory; + } + + public void setLifeStory(String lifeStory) { + this.lifeStory = lifeStory; + } + + public String getReallyBigString() { + return reallyBigString; + } + + public void setReallyBigString(String reallyBigString) { + this.reallyBigString = reallyBigString; + } + } +}