From 77ee7f5134bc455700804951ef29b046d38e4c54 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Thu, 1 Jun 2023 12:45:58 +0200 Subject: [PATCH] HHH-16682 Test and fix dirty checking for @JdbcTypeCode(SqlTypes.JSON) maps --- .../java/spi/CollectionJavaType.java | 91 ++++++++++++++----- .../test/mapping/basic/JsonMappingTests.java | 18 ++++ .../embeddable/JsonEmbeddableTest.java | 15 +++ 3 files changed, 102 insertions(+), 22 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/spi/CollectionJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/spi/CollectionJavaType.java index c4a8d31dbe..35fdea6314 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/spi/CollectionJavaType.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/spi/CollectionJavaType.java @@ -6,16 +6,20 @@ */ package org.hibernate.type.descriptor.java.spi; +import java.io.Serializable; import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; +import java.util.Map; import java.util.Objects; +import org.hibernate.SharedSessionContract; import org.hibernate.collection.spi.CollectionSemantics; +import org.hibernate.collection.spi.MapSemantics; import org.hibernate.collection.spi.PersistentCollection; import org.hibernate.type.descriptor.WrapperOptions; import org.hibernate.type.descriptor.java.AbstractClassJavaType; import org.hibernate.type.descriptor.java.JavaType; import org.hibernate.type.descriptor.java.MutabilityPlan; -import org.hibernate.type.descriptor.java.MutableMutabilityPlan; import org.hibernate.type.descriptor.jdbc.JdbcType; import org.hibernate.type.descriptor.jdbc.JdbcTypeIndicators; import org.hibernate.type.spi.TypeConfiguration; @@ -52,6 +56,9 @@ public class CollectionJavaType extends AbstractClassJavaType { public JavaType createJavaType( ParameterizedType parameterizedType, TypeConfiguration typeConfiguration) { + final Type[] actualTypeArguments = parameterizedType.getActualTypeArguments(); + final JavaTypeRegistry javaTypeRegistry = typeConfiguration.getJavaTypeRegistry(); + final JavaType valueDescriptor = javaTypeRegistry.resolveDescriptor( actualTypeArguments[actualTypeArguments.length - 1] ); switch ( semantics.getCollectionClassification() ) { case ARRAY: case BAG: @@ -63,14 +70,21 @@ public class CollectionJavaType extends AbstractClassJavaType { //noinspection unchecked,rawtypes return new BasicCollectionJavaType( parameterizedType, - typeConfiguration.getJavaTypeRegistry() - .resolveDescriptor( parameterizedType.getActualTypeArguments()[0] ), + valueDescriptor, semantics ); + } // Construct a basic java type that knows its parametrization - //noinspection unchecked - return new UnknownBasicJavaType<>( parameterizedType, (MutabilityPlan) MutableMutabilityPlan.INSTANCE ); + //noinspection unchecked,rawtypes + return new UnknownBasicJavaType( + parameterizedType, + new MapMutabilityPlan<>( + (MapSemantics, Object, Object>) semantics, + javaTypeRegistry.resolveDescriptor( actualTypeArguments[0] ), + valueDescriptor + ) + ); } @Override @@ -90,30 +104,17 @@ public class CollectionJavaType extends AbstractClassJavaType { @Override public boolean areEqual(C one, C another) { -// return one == another || -// ( -// one instanceof PersistentCollection && -// ( (PersistentCollection) one ).wasInitialized() && -// ( (PersistentCollection) one ).isWrapper( another ) -// ) || -// ( -// another instanceof PersistentCollection && -// ( (PersistentCollection) another ).wasInitialized() && -// ( (PersistentCollection) another ).isWrapper( one ) -// ); - - if ( one == another ) { return true; } - if ( one instanceof PersistentCollection ) { - final PersistentCollection pc = (PersistentCollection) one; + if ( one instanceof PersistentCollection ) { + final PersistentCollection pc = (PersistentCollection) one; return pc.wasInitialized() && ( pc.isWrapper( another ) || pc.isDirectlyProvidedCollection( another ) ); } - if ( another instanceof PersistentCollection ) { - final PersistentCollection pc = (PersistentCollection) another; + if ( another instanceof PersistentCollection ) { + final PersistentCollection pc = (PersistentCollection) another; return pc.wasInitialized() && ( pc.isWrapper( one ) || pc.isDirectlyProvidedCollection( one ) ); } @@ -124,4 +125,50 @@ public class CollectionJavaType extends AbstractClassJavaType { public int extractHashCode(C x) { throw new UnsupportedOperationException(); } + + private static class MapMutabilityPlan, K, V> implements MutabilityPlan { + + private final MapSemantics semantics; + private final MutabilityPlan keyPlan; + private final MutabilityPlan valuePlan; + + public MapMutabilityPlan( + MapSemantics semantics, + JavaType keyType, + JavaType valueType) { + this.semantics = semantics; + this.keyPlan = keyType.getMutabilityPlan(); + this.valuePlan = valueType.getMutabilityPlan(); + } + + @Override + public boolean isMutable() { + return true; + } + + @Override + public C deepCopy(C value) { + if ( value == null ) { + return null; + } + final C copy = semantics.instantiateRaw( value.size(), null ); + + for ( Map.Entry entry : value.entrySet() ) { + copy.put( keyPlan.deepCopy( entry.getKey() ), valuePlan.deepCopy( entry.getValue() ) ); + } + return copy; + } + + @Override + public Serializable disassemble(C value, SharedSessionContract session) { + return (Serializable) deepCopy( value ); + } + + @Override + public C assemble(Serializable cached, SharedSessionContract session) { + //noinspection unchecked + return deepCopy( (C) cached ); + } + + } } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/basic/JsonMappingTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/basic/JsonMappingTests.java index 58ead2ce2e..8ec0a3b0b3 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/basic/JsonMappingTests.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/basic/JsonMappingTests.java @@ -26,6 +26,7 @@ import org.hibernate.type.descriptor.jdbc.JdbcType; import org.hibernate.type.descriptor.jdbc.spi.JdbcTypeRegistry; import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.JiraKey; import org.hibernate.testing.orm.junit.ServiceRegistry; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; @@ -142,6 +143,23 @@ public abstract class JsonMappingTests { ); } + @Test + @JiraKey( "HHH-16682" ) + public void verifyDirtyChecking(SessionFactoryScope scope) { + scope.inTransaction( + (session) -> { + EntityWithJson entityWithJson = session.find( EntityWithJson.class, 1 ); + entityWithJson.stringMap.clear(); + } + ); + scope.inTransaction( + (session) -> { + EntityWithJson entityWithJson = session.find( EntityWithJson.class, 1 ); + assertThat( entityWithJson.stringMap.isEmpty(), is( true ) ); + } + ); + } + @Test @SkipForDialect(dialectClass = DerbyDialect.class, reason = "Derby doesn't support comparing CLOBs with the = operator") @SkipForDialect(dialectClass = AbstractHANADialect.class, matchSubTypes = true, reason = "HANA doesn't support comparing LOBs with the = operator") diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/embeddable/JsonEmbeddableTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/embeddable/JsonEmbeddableTest.java index 3ab0bd2f14..1e59db7bf9 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/embeddable/JsonEmbeddableTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/embeddable/JsonEmbeddableTest.java @@ -27,6 +27,7 @@ import org.hibernate.testing.orm.domain.gambit.EntityOfBasics; import org.hibernate.testing.orm.domain.gambit.MutableValue; import org.hibernate.testing.orm.junit.BaseSessionFactoryFunctionalTest; import org.hibernate.testing.orm.junit.DialectFeatureChecks; +import org.hibernate.testing.orm.junit.JiraKey; import org.hibernate.testing.orm.junit.RequiresDialectFeature; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -81,6 +82,20 @@ public class JsonEmbeddableTest extends BaseSessionFactoryFunctionalTest { ); } + @Test + @JiraKey( "HHH-16682" ) + public void testDirtyChecking() { + sessionFactoryScope().inTransaction( + entityManager -> { + JsonHolder jsonHolder = entityManager.find( JsonHolder.class, 1L ); + jsonHolder.getAggregate().setTheString( "MyString" ); + entityManager.flush(); + entityManager.clear(); + assertEquals( "MyString", entityManager.find( JsonHolder.class, 1L ).getAggregate().getTheString() ); + } + ); + } + @Test public void testFetch() { sessionFactoryScope().inSession(