From 834426e8f6d2c27d815164d2d565ca785864e13b Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Fri, 14 Jan 2022 15:25:05 -0600 Subject: [PATCH] Fix bug with creating a NaturalIdCacheKey via SimpleCacheKeysFactory --- .../internal/DefaultCacheKeysFactory.java | 2 +- .../cache/internal/NaturalIdCacheKey.java | 58 +++++------- .../internal/SimpleCacheKeysFactory.java | 2 +- .../NaturalIdCacheKeyCreationTests.java | 89 +++++++++++++++++++ .../caching/mocked/NaturalIdCacheKeyTest.java | 13 +++ 5 files changed, 126 insertions(+), 38 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/caching/NaturalIdCacheKeyCreationTests.java diff --git a/hibernate-core/src/main/java/org/hibernate/cache/internal/DefaultCacheKeysFactory.java b/hibernate-core/src/main/java/org/hibernate/cache/internal/DefaultCacheKeysFactory.java index 019c7d73dc..89597d1325 100644 --- a/hibernate-core/src/main/java/org/hibernate/cache/internal/DefaultCacheKeysFactory.java +++ b/hibernate-core/src/main/java/org/hibernate/cache/internal/DefaultCacheKeysFactory.java @@ -51,7 +51,7 @@ public class DefaultCacheKeysFactory implements CacheKeysFactory { } public static Object staticCreateNaturalIdKey(Object naturalIdValues, EntityPersister persister, SharedSessionContractImplementor session) { - return new NaturalIdCacheKey( naturalIdValues, persister.getPropertyTypes(), persister.getNaturalIdentifierProperties(), persister.getRootEntityName(), session ); + return new NaturalIdCacheKey( naturalIdValues, persister, session ); } public static Object staticGetEntityId(Object cacheKey) { diff --git a/hibernate-core/src/main/java/org/hibernate/cache/internal/NaturalIdCacheKey.java b/hibernate-core/src/main/java/org/hibernate/cache/internal/NaturalIdCacheKey.java index d69509616e..5ffec30599 100644 --- a/hibernate-core/src/main/java/org/hibernate/cache/internal/NaturalIdCacheKey.java +++ b/hibernate-core/src/main/java/org/hibernate/cache/internal/NaturalIdCacheKey.java @@ -13,9 +13,8 @@ import java.util.Objects; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.internal.util.ValueHolder; -import org.hibernate.metamodel.mapping.EntityMappingType; import org.hibernate.metamodel.mapping.NaturalIdMapping; -import org.hibernate.type.Type; +import org.hibernate.persister.entity.EntityPersister; /** * Defines a key for caching natural identifier resolutions into the second level cache. @@ -34,24 +33,15 @@ public class NaturalIdCacheKey implements Serializable { // "transient" is important here -- NaturalIdCacheKey needs to be Serializable private transient ValueHolder toString; - /** - * Construct a new key for a caching natural identifier resolutions into the second level cache. - * @param naturalIdValues The naturalIdValues associated with the cached data - * @param propertyTypes - * @param naturalIdPropertyIndexes - * @param session The originating session - */ - public NaturalIdCacheKey( - final Object naturalIdValues, - Type[] propertyTypes, - int[] naturalIdPropertyIndexes, - final String entityName, - final SharedSessionContractImplementor session) { + public NaturalIdCacheKey(Object naturalIdValues, EntityPersister persister, SharedSessionContractImplementor session) { + this( naturalIdValues, persister, persister.getRootEntityName(), session ); + } + + public NaturalIdCacheKey(Object naturalIdValues, EntityPersister persister, String entityName, SharedSessionContractImplementor session) { this.entityName = entityName; this.tenantId = session.getTenantIdentifier(); - final EntityMappingType entityMappingType = session.getFactory().getRuntimeMetamodels().getEntityMappingType( entityName ); - final NaturalIdMapping naturalIdMapping = entityMappingType.getNaturalIdMapping(); + final NaturalIdMapping naturalIdMapping = persister.getNaturalIdMapping(); this.naturalIdValues = naturalIdMapping.disassemble( naturalIdValues, session ); this.hashCode = naturalIdMapping.calculateHashCode( naturalIdValues, session ); @@ -61,28 +51,24 @@ public class NaturalIdCacheKey implements Serializable { private void initTransients() { this.toString = new ValueHolder<>( - new ValueHolder.DeferredInitializer() { - @Override - public String initialize() { - //Complex toString is needed as naturalIds for entities are not simply based on a single value like primary keys - //the only same way to differentiate the keys is to include the disassembled values in the string. - final StringBuilder toStringBuilder = new StringBuilder() - .append( entityName ).append( "##NaturalId[" ); - if ( naturalIdValues instanceof Object[] ) { - final Object[] values = (Object[]) naturalIdValues; - for ( int i = 0; i < values.length; i++ ) { - toStringBuilder.append( values[ i ] ); - if ( i + 1 < values.length ) { - toStringBuilder.append( ", " ); - } + () -> { + //Complex toString is needed as naturalIds for entities are not simply based on a single value like primary keys + //the only same way to differentiate the keys is to include the disassembled values in the string. + final StringBuilder toStringBuilder = new StringBuilder().append( entityName ).append( "##NaturalId[" ); + if ( naturalIdValues instanceof Object[] ) { + final Object[] values = (Object[]) naturalIdValues; + for ( int i = 0; i < values.length; i++ ) { + toStringBuilder.append( values[ i ] ); + if ( i + 1 < values.length ) { + toStringBuilder.append( ", " ); } } - else { - toStringBuilder.append( naturalIdValues ); - } - - return toStringBuilder.toString(); } + else { + toStringBuilder.append( naturalIdValues ); + } + + return toStringBuilder.toString(); } ); } diff --git a/hibernate-core/src/main/java/org/hibernate/cache/internal/SimpleCacheKeysFactory.java b/hibernate-core/src/main/java/org/hibernate/cache/internal/SimpleCacheKeysFactory.java index f865c935bb..f5e8345e03 100644 --- a/hibernate-core/src/main/java/org/hibernate/cache/internal/SimpleCacheKeysFactory.java +++ b/hibernate-core/src/main/java/org/hibernate/cache/internal/SimpleCacheKeysFactory.java @@ -34,7 +34,7 @@ public class SimpleCacheKeysFactory implements CacheKeysFactory { @Override public Object createNaturalIdKey(Object naturalIdValues, EntityPersister persister, SharedSessionContractImplementor session) { // natural ids always need to be wrapped - return new NaturalIdCacheKey(naturalIdValues, persister.getPropertyTypes(), persister.getNaturalIdentifierProperties(), null, session); + return new NaturalIdCacheKey(naturalIdValues, persister, null, session); } @Override diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/caching/NaturalIdCacheKeyCreationTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/caching/NaturalIdCacheKeyCreationTests.java new file mode 100644 index 0000000000..1dd7fa54eb --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/caching/NaturalIdCacheKeyCreationTests.java @@ -0,0 +1,89 @@ +/* + * 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.orm.test.caching; + +import org.hibernate.annotations.NaturalId; +import org.hibernate.cache.internal.DefaultCacheKeysFactory; +import org.hibernate.cache.internal.SimpleCacheKeysFactory; +import org.hibernate.persister.entity.EntityPersister; + +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.junit.jupiter.api.Test; + +import jakarta.persistence.Basic; +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.Table; + +/** + * @author Steve Ebersole + */ +@DomainModel( annotatedClasses = NaturalIdCacheKeyCreationTests.TheEntity.class ) +@SessionFactory +public class NaturalIdCacheKeyCreationTests { + + @Test + public void testSimpleKeyCreation(SessionFactoryScope scope) { + scope.inTransaction( (session) -> { + final EntityPersister entityDescriptor = (EntityPersister) session + .getFactory() + .getRuntimeMetamodels() + .getEntityMappingType( TheEntity.class ); + + SimpleCacheKeysFactory.INSTANCE.createEntityKey( 1, entityDescriptor, session.getSessionFactory(), null ); + SimpleCacheKeysFactory.INSTANCE.createNaturalIdKey( "Steve", entityDescriptor, session ); + + } ); + } + + @Test + public void testDefaultKeyCreation(SessionFactoryScope scope) { + scope.inTransaction( (session) -> { + final EntityPersister entityDescriptor = (EntityPersister) session + .getFactory() + .getRuntimeMetamodels() + .getEntityMappingType( TheEntity.class ); + + DefaultCacheKeysFactory.INSTANCE.createEntityKey( 1, entityDescriptor, session.getSessionFactory(), null ); + DefaultCacheKeysFactory.INSTANCE.createNaturalIdKey( "Steve", entityDescriptor, session ); + + } ); + } + + @Entity( name = "TheEntity" ) + @Table( name = "`entities`" ) + public static class TheEntity { + @Id + private Integer id; + @Basic + @NaturalId + private String name; + + private TheEntity() { + // for use by Hibernate + } + + public TheEntity(Integer id, String name) { + this.id = id; + this.name = name; + } + + public Integer getId() { + return id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/caching/mocked/NaturalIdCacheKeyTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/caching/mocked/NaturalIdCacheKeyTest.java index 7a7e34f5d4..0a1aa7857e 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/caching/mocked/NaturalIdCacheKeyTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/caching/mocked/NaturalIdCacheKeyTest.java @@ -11,6 +11,7 @@ import java.io.ByteArrayOutputStream; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; +import org.hibernate.annotations.NaturalId; import org.hibernate.cache.internal.DefaultCacheKeysFactory; import org.hibernate.cache.internal.NaturalIdCacheKey; import org.hibernate.engine.spi.SessionFactoryImplementor; @@ -21,6 +22,11 @@ import org.hibernate.persister.entity.EntityPersister; import org.junit.Test; +import jakarta.persistence.Basic; +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.Table; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertArrayEquals; import static org.mockito.ArgumentMatchers.any; @@ -30,6 +36,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; public class NaturalIdCacheKeyTest { + @Test public void testSerializationRoundTrip() throws Exception { final SessionFactoryImplementor sessionFactoryImplementor = mock( SessionFactoryImplementor.class ); @@ -65,4 +72,10 @@ public class NaturalIdCacheKeyTest { assertEquals(key.getTenantId(), keyClone.getTenantId()); } + + + @Test + public void testSimpleKeyCreation() { + + } }