diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/NaturalIdXrefDelegate.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/NaturalIdXrefDelegate.java index 69da3afb98..baf75d1eda 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/NaturalIdXrefDelegate.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/NaturalIdXrefDelegate.java @@ -39,6 +39,7 @@ import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SessionImplementor; import org.hibernate.event.spi.EventSource; import org.hibernate.persister.entity.EntityPersister; +import org.hibernate.type.Type; /** * Maintains a {@link org.hibernate.engine.spi.PersistenceContext}-level 2-way cross-reference (xref) between the @@ -74,11 +75,12 @@ public class NaturalIdXrefDelegate { naturalIdResolutionCacheMap.put( persister, entityNaturalIdResolutionCache ); } - final CachedNaturalId cachedNaturalId = new CachedNaturalId( persister, naturalIdValues ); - entityNaturalIdResolutionCache.pkToNaturalIdMap.put( pk, cachedNaturalId ); - entityNaturalIdResolutionCache.naturalIdToPkMap.put( cachedNaturalId, pk ); + final boolean justAddedToLocalCache = entityNaturalIdResolutionCache.cache( pk, naturalIdValues ); - //If second-level caching is enabled cache the resolution there as well + // If second-level caching is enabled cache the resolution there as well + // NOTE : the checks using 'justAddedToLocalCache' below protect only the stat journaling, not actually + // putting into the shared cache. we still put into the shared cache because that might have locking + // semantics that we need to honor. if ( persister.hasNaturalIdCache() ) { final NaturalIdRegionAccessStrategy naturalIdCacheAccessStrategy = persister.getNaturalIdCacheAccessStrategy(); final NaturalIdCacheKey naturalIdCacheKey = new NaturalIdCacheKey( naturalIdValues, persister, session() ); @@ -93,8 +95,8 @@ public class NaturalIdXrefDelegate { session().getTimestamp(), null ); - - if ( put && factory.getStatistics().isStatisticsEnabled() ) { + + if ( put && justAddedToLocalCache && factory.getStatistics().isStatisticsEnabled() ) { factory.getStatisticsImplementor() .naturalIdCachePut( naturalIdCacheAccessStrategy.getRegion().getName() ); } @@ -110,7 +112,7 @@ public class NaturalIdXrefDelegate { public void doAfterTransactionCompletion(boolean success, SessionImplementor session) { final boolean put = naturalIdCacheAccessStrategy.afterInsert( naturalIdCacheKey, pk ); - if ( put && factory.getStatistics().isStatisticsEnabled() ) { + if ( put && justAddedToLocalCache && factory.getStatistics().isStatisticsEnabled() ) { factory.getStatisticsImplementor().naturalIdCachePut( naturalIdCacheAccessStrategy.getRegion().getName() ); } @@ -130,7 +132,7 @@ public class NaturalIdXrefDelegate { public void doAfterTransactionCompletion(boolean success, SessionImplementor session) { final boolean put = naturalIdCacheAccessStrategy.afterUpdate( naturalIdCacheKey, pk, lock ); - if ( put && factory.getStatistics().isStatisticsEnabled() ) { + if ( put && justAddedToLocalCache && factory.getStatistics().isStatisticsEnabled() ) { factory.getStatisticsImplementor().naturalIdCachePut( naturalIdCacheAccessStrategy.getRegion().getName() ); } @@ -310,16 +312,49 @@ public class NaturalIdXrefDelegate { private static class NaturalIdResolutionCache implements Serializable { private final EntityPersister persister; + private final Type[] naturalIdTypes; + + private Map pkToNaturalIdMap = new ConcurrentHashMap(); + private Map naturalIdToPkMap = new ConcurrentHashMap(); private NaturalIdResolutionCache(EntityPersister persister) { this.persister = persister; + + final int[] naturalIdPropertyIndexes = persister.getNaturalIdentifierProperties(); + naturalIdTypes = new Type[ naturalIdPropertyIndexes.length ]; + int i = 0; + for ( int naturalIdPropertyIndex : naturalIdPropertyIndexes ) { + naturalIdTypes[i++] = persister.getPropertyType( persister.getPropertyNames()[ naturalIdPropertyIndex ] ); + } } public EntityPersister getPersister() { return persister; } - private Map pkToNaturalIdMap = new ConcurrentHashMap(); - private Map naturalIdToPkMap = new ConcurrentHashMap(); + public boolean cache(Serializable pk, Object[] naturalIdValues) { + final CachedNaturalId initial = pkToNaturalIdMap.get( pk ); + if ( initial != null ) { + if ( areSame( naturalIdValues, initial.getValues() ) ) { + return false; + } + } + + final CachedNaturalId cachedNaturalId = new CachedNaturalId( persister, naturalIdValues ); + pkToNaturalIdMap.put( pk, cachedNaturalId ); + naturalIdToPkMap.put( cachedNaturalId, pk ); + + return true; + } + + private boolean areSame(Object[] naturalIdValues, Object[] values) { + // lengths have already been verified at this point + for ( int i = 0; i < naturalIdTypes.length; i++ ) { + if ( naturalIdTypes[i].compare( naturalIdValues[i], values[i] ) != 0 ) { + return false; + } + } + return true; + } } } diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLoadEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLoadEventListener.java index e66f904bf2..e6432fdf7a 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLoadEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLoadEventListener.java @@ -37,13 +37,13 @@ import org.hibernate.cache.spi.access.SoftLock; import org.hibernate.cache.spi.entry.CacheEntry; import org.hibernate.engine.internal.TwoPhaseLoad; import org.hibernate.engine.internal.Versioning; +import org.hibernate.engine.spi.CachedNaturalIdValueSource; import org.hibernate.engine.spi.EntityEntry; import org.hibernate.engine.spi.EntityKey; import org.hibernate.engine.spi.PersistenceContext; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SessionImplementor; import org.hibernate.engine.spi.Status; -import org.hibernate.engine.spi.PersistenceContext.CachedNaturalIdValueSource; import org.hibernate.event.service.spi.EventListenerRegistry; import org.hibernate.event.spi.EventSource; import org.hibernate.event.spi.EventType; diff --git a/hibernate-core/src/matrix/java/org/hibernate/test/annotations/naturalid/NaturalIdOnSingleManyToOneTest.java b/hibernate-core/src/matrix/java/org/hibernate/test/annotations/naturalid/NaturalIdOnSingleManyToOneTest.java index aedf22bcbe..f6b8143822 100644 --- a/hibernate-core/src/matrix/java/org/hibernate/test/annotations/naturalid/NaturalIdOnSingleManyToOneTest.java +++ b/hibernate-core/src/matrix/java/org/hibernate/test/annotations/naturalid/NaturalIdOnSingleManyToOneTest.java @@ -26,6 +26,8 @@ package org.hibernate.test.annotations.naturalid; import java.util.List; import org.jboss.logging.Logger; + +import org.junit.After; import org.junit.Test; import org.hibernate.Criteria; @@ -52,11 +54,16 @@ import static org.junit.Assert.assertTrue; public class NaturalIdOnSingleManyToOneTest extends BaseCoreFunctionalTestCase { private static final Logger log = Logger.getLogger( NaturalIdOnSingleManyToOneTest.class ); - @Override - protected void cleanupTest() throws Exception { - this.cleanupCache(); - - this.deleteAllData(); + @After + public void cleanupData() { + super.cleanupCache(); + Session s = sessionFactory().openSession(); + s.beginTransaction(); + s.createQuery( "delete NaturalIdOnManyToOne" ).executeUpdate(); + s.createQuery( "delete Citizen" ).executeUpdate(); + s.createQuery( "delete State" ).executeUpdate(); + s.getTransaction().commit(); + s.close(); } @Test diff --git a/hibernate-core/src/matrix/java/org/hibernate/test/annotations/naturalid/NaturalIdTest.java b/hibernate-core/src/matrix/java/org/hibernate/test/annotations/naturalid/NaturalIdTest.java index f4e79da47f..a29d8b7b92 100644 --- a/hibernate-core/src/matrix/java/org/hibernate/test/annotations/naturalid/NaturalIdTest.java +++ b/hibernate-core/src/matrix/java/org/hibernate/test/annotations/naturalid/NaturalIdTest.java @@ -38,6 +38,8 @@ import org.hibernate.criterion.Restrictions; import org.hibernate.metadata.ClassMetadata; import org.hibernate.stat.Statistics; import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; + +import org.junit.After; import org.junit.Test; /** @@ -48,13 +50,18 @@ import org.junit.Test; */ @SuppressWarnings("unchecked") public class NaturalIdTest extends BaseCoreFunctionalTestCase { - @Override - protected void cleanupTest() throws Exception { - this.cleanupCache(); - - this.deleteAllData(); + @After + public void cleanupData() { + super.cleanupCache(); + Session s = sessionFactory().openSession(); + s.beginTransaction(); + s.createQuery( "delete NaturalIdOnManyToOne" ).executeUpdate(); + s.createQuery( "delete Citizen" ).executeUpdate(); + s.createQuery( "delete State" ).executeUpdate(); + s.getTransaction().commit(); + s.close(); } - + @Test public void testMappingProperties() { ClassMetadata metaData = sessionFactory().getClassMetadata( diff --git a/hibernate-core/src/matrix/java/org/hibernate/test/naturalid/immutableentity/ImmutableEntityNaturalIdTest.java b/hibernate-core/src/matrix/java/org/hibernate/test/naturalid/immutableentity/ImmutableEntityNaturalIdTest.java index 5ba5128840..a85e22ece3 100644 --- a/hibernate-core/src/matrix/java/org/hibernate/test/naturalid/immutableentity/ImmutableEntityNaturalIdTest.java +++ b/hibernate-core/src/matrix/java/org/hibernate/test/naturalid/immutableentity/ImmutableEntityNaturalIdTest.java @@ -133,9 +133,9 @@ public class ImmutableEntityNaturalIdTest extends BaseCoreFunctionalTestCase { building = (Building) naturalIdLoader.load(); assertNull( building ); assertEquals( "Cache hits should be one after second query", 1, stats.getNaturalIdCacheHitCount() ); - assertEquals( "Cache misses should be one after second query", 1, stats.getNaturalIdCacheMissCount() ); + assertEquals( "Cache misses should be two after second query", 2, stats.getNaturalIdCacheMissCount() ); assertEquals( "Cache put should be one after second query", 1, stats.getNaturalIdCachePutCount() ); - assertEquals( "Query count should be one after second query", 1, stats.getNaturalIdQueryExecutionCount() ); + assertEquals( "Query count should be two after second query", 2, stats.getNaturalIdQueryExecutionCount() ); // cleanup tx.commit(); @@ -152,17 +152,13 @@ public class ImmutableEntityNaturalIdTest extends BaseCoreFunctionalTestCase { building = (Building) naturalIdLoader.load(); assertNull( building ); assertEquals( "Cache hits should be one after third query", 1, stats.getNaturalIdCacheHitCount() ); - assertEquals( "Cache misses should be one after third query", 2, stats.getNaturalIdCacheMissCount() ); + assertEquals( "Cache misses should be one after third query", 3, stats.getNaturalIdCacheMissCount() ); assertEquals( "Cache put should be one after third query", 1, stats.getNaturalIdCachePutCount() ); - assertEquals( "Query count should be one after third query", 2, stats.getNaturalIdQueryExecutionCount() ); + assertEquals( "Query count should be one after third query", 3, stats.getNaturalIdQueryExecutionCount() ); // cleanup tx.rollback(); s.close(); - - - - } @Override diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/junit4/BaseCoreFunctionalTestCase.java b/hibernate-testing/src/main/java/org/hibernate/testing/junit4/BaseCoreFunctionalTestCase.java index 93c6115e98..8dad3515d2 100644 --- a/hibernate-testing/src/main/java/org/hibernate/testing/junit4/BaseCoreFunctionalTestCase.java +++ b/hibernate-testing/src/main/java/org/hibernate/testing/junit4/BaseCoreFunctionalTestCase.java @@ -422,41 +422,6 @@ public abstract class BaseCoreFunctionalTestCase extends BaseUnitTestCase { assertAllDataRemoved(); } - - protected void deleteAllData() { - // Get all the entities the session factory knows about - final Map allClassMetadata = this.sessionFactory().getAllClassMetadata(); - Set entityTypes = new LinkedHashSet(allClassMetadata.values()); - - do { - final Set failedEntitieTypes = new HashSet(); - - for (final ClassMetadata entityType : entityTypes) { - final String entityClassName = entityType.getEntityName(); - - final Session s = openSession(); - final Transaction tx = s.beginTransaction(); - try { - final Criteria criteria = s.createCriteria( entityClassName ); - final List entities = criteria.list(); - for (final Object entity : entities) { - s.delete( entity); - } - - tx.commit(); - } - catch (ConstraintViolationException e) { - failedEntitieTypes.add(entityType); - tx.rollback(); - } - finally { - s.close(); - } - } - - entityTypes = failedEntitieTypes; - } while (!entityTypes.isEmpty()); - } protected void cleanupCache() { if ( sessionFactory != null ) {