Skip to content

Commit 44e5dfa

Browse files
committed
HHH-17680 Avoid JavaType collisions with special JavaTypes for JSON/XML
1 parent 1ad46ec commit 44e5dfa

File tree

3 files changed

+163
-17
lines changed

3 files changed

+163
-17
lines changed

hibernate-core/src/main/java/org/hibernate/mapping/BasicValue.java

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -774,12 +774,12 @@ else if ( ownerName != null && propertyName != null ) {
774774
}
775775
}
776776

777-
private JavaType<Object> javaType(TypeConfiguration typeConfiguration, java.lang.reflect.Type impliedJavaType) {
778-
final JavaType<Object> javaType = typeConfiguration.getJavaTypeRegistry().findDescriptor( impliedJavaType );
777+
private JavaType<?> javaType(TypeConfiguration typeConfiguration, java.lang.reflect.Type impliedJavaType) {
778+
final JavaType<?> javaType = typeConfiguration.getJavaTypeRegistry().findDescriptor( impliedJavaType );
779779
return javaType == null ? specialJavaType( typeConfiguration, impliedJavaType ) : javaType;
780780
}
781781

782-
private JavaType<Object> specialJavaType(
782+
private JavaType<?> specialJavaType(
783783
TypeConfiguration typeConfiguration,
784784
java.lang.reflect.Type impliedJavaType) {
785785
final JavaTypeRegistry javaTypeRegistry = typeConfiguration.getJavaTypeRegistry();
@@ -788,19 +788,25 @@ private JavaType<Object> specialJavaType(
788788
// and implement toString/fromString as well as copying based on FormatMapper operations
789789
switch ( jdbcTypeCode ) {
790790
case SqlTypes.JSON:
791-
final JavaType<Object> jsonJavaType =
792-
new JsonJavaType<>( impliedJavaType,
791+
return javaTypeRegistry.resolveDescriptor(
792+
SqlTypes.JSON,
793+
impliedJavaType,
794+
() -> new JsonJavaType<>(
795+
impliedJavaType,
793796
mutabilityPlan( typeConfiguration, impliedJavaType ),
794-
typeConfiguration );
795-
javaTypeRegistry.addDescriptor( jsonJavaType );
796-
return jsonJavaType;
797+
typeConfiguration
798+
)
799+
);
797800
case SqlTypes.SQLXML:
798-
final JavaType<Object> xmlJavaType =
799-
new XmlJavaType<>( impliedJavaType,
801+
return javaTypeRegistry.resolveDescriptor(
802+
SqlTypes.SQLXML,
803+
impliedJavaType,
804+
() -> new XmlJavaType<>(
805+
impliedJavaType,
800806
mutabilityPlan( typeConfiguration, impliedJavaType ),
801-
typeConfiguration );
802-
javaTypeRegistry.addDescriptor( xmlJavaType );
803-
return xmlJavaType;
807+
typeConfiguration
808+
)
809+
);
804810
}
805811
}
806812
return javaTypeRegistry.resolveDescriptor( impliedJavaType );

hibernate-core/src/main/java/org/hibernate/type/descriptor/java/spi/JavaTypeRegistry.java

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import java.util.function.Consumer;
1515
import java.util.function.Supplier;
1616

17+
import org.checkerframework.checker.nullness.qual.Nullable;
1718
import org.hibernate.type.descriptor.java.ArrayJavaType;
1819
import org.hibernate.type.descriptor.java.JavaType;
1920
import org.hibernate.type.descriptor.java.MutabilityPlan;
@@ -37,6 +38,7 @@ public class JavaTypeRegistry implements JavaTypeBaseline.BaselineTarget, Serial
3738

3839
private final TypeConfiguration typeConfiguration;
3940
private final ConcurrentHashMap<Type, JavaType<?>> descriptorsByType = new ConcurrentHashMap<>();
41+
private final ConcurrentHashMap<Integer, ConcurrentHashMap<Type, JavaType<?>>> typeCodeSpecificDescriptorsByType = new ConcurrentHashMap<>();
4042

4143
public JavaTypeRegistry(TypeConfiguration typeConfiguration) {
4244
this.typeConfiguration = typeConfiguration;
@@ -74,13 +76,26 @@ private void performInjections(JavaType<?> descriptor) {
7476

7577
public void forEachDescriptor(Consumer<JavaType<?>> consumer) {
7678
descriptorsByType.values().forEach( consumer );
79+
typeCodeSpecificDescriptorsByType.values().forEach( descriptorsByTypeName -> {
80+
descriptorsByTypeName.values().forEach( consumer );
81+
} );
7782
}
7883

7984
public <T> JavaType<T> getDescriptor(Type javaType) {
8085
return resolveDescriptor( javaType );
8186
}
8287

8388
public void addDescriptor(JavaType<?> descriptor) {
89+
addDescriptor( descriptorsByType, descriptor );
90+
}
91+
92+
public void addDescriptor(int sqlTypeCode, JavaType<?> descriptor) {
93+
final ConcurrentHashMap<Type, JavaType<?>> descriptorsByTypeName =
94+
typeCodeSpecificDescriptorsByType.computeIfAbsent( sqlTypeCode, k -> new ConcurrentHashMap<>() );
95+
addDescriptor( descriptorsByTypeName, descriptor );
96+
}
97+
98+
private void addDescriptor(ConcurrentHashMap<Type, JavaType<?>> descriptorsByType, JavaType<?> descriptor) {
8499
JavaType<?> old = descriptorsByType.put( descriptor.getJavaType(), descriptor );
85100
if ( old != null ) {
86101
log.debugf(
@@ -93,19 +108,35 @@ public void addDescriptor(JavaType<?> descriptor) {
93108
performInjections( descriptor );
94109
}
95110

96-
public <J> JavaType<J> findDescriptor(Type javaType) {
111+
public <J> @Nullable JavaType<J> findDescriptor(Type javaType) {
97112
//noinspection unchecked
98113
return (JavaType<J>) descriptorsByType.get( javaType );
99114
}
100115

116+
public @Nullable JavaType<?> findDescriptor(int sqlTypeCode, Type javaType) {
117+
final ConcurrentHashMap<Type, JavaType<?>> descriptorsByType =
118+
typeCodeSpecificDescriptorsByType.get( sqlTypeCode );
119+
return descriptorsByType.get( javaType );
120+
}
121+
101122
public <J> JavaType<J> resolveDescriptor(Type javaType, Supplier<JavaType<J>> creator) {
123+
//noinspection unchecked
124+
return (JavaType<J>) resolveDescriptor( descriptorsByType, javaType, creator );
125+
}
126+
127+
public JavaType<?> resolveDescriptor(int sqlTypeCode, Type javaType, Supplier<JavaType<?>> creator) {
128+
final ConcurrentHashMap<Type, JavaType<?>> descriptorsByType =
129+
typeCodeSpecificDescriptorsByType.computeIfAbsent( sqlTypeCode, k -> new ConcurrentHashMap<>() );
130+
return resolveDescriptor( descriptorsByType, javaType, creator );
131+
}
132+
133+
private JavaType<?> resolveDescriptor(ConcurrentHashMap<Type, JavaType<?>> descriptorsByType, Type javaType, Supplier<? extends JavaType<?>> creator) {
102134
final JavaType<?> cached = descriptorsByType.get( javaType );
103135
if ( cached != null ) {
104-
//noinspection unchecked
105-
return (JavaType<J>) cached;
136+
return cached;
106137
}
107138

108-
final JavaType<J> created = creator.get();
139+
final JavaType<?> created = creator.get();
109140
descriptorsByType.put( javaType, created );
110141
return created;
111142
}
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
* Copyright Red Hat Inc. and Hibernate Authors
4+
*/
5+
package org.hibernate.orm.test.mapping.basic;
6+
7+
import jakarta.persistence.Entity;
8+
import jakarta.persistence.Id;
9+
import jakarta.persistence.Table;
10+
import org.hibernate.annotations.JdbcTypeCode;
11+
import org.hibernate.cfg.AvailableSettings;
12+
import org.hibernate.metamodel.mapping.internal.BasicAttributeMapping;
13+
import org.hibernate.metamodel.spi.MappingMetamodelImplementor;
14+
import org.hibernate.persister.entity.EntityPersister;
15+
import org.hibernate.testing.orm.junit.DialectFeatureChecks;
16+
import org.hibernate.testing.orm.junit.DomainModel;
17+
import org.hibernate.testing.orm.junit.Jira;
18+
import org.hibernate.testing.orm.junit.RequiresDialectFeature;
19+
import org.hibernate.testing.orm.junit.ServiceRegistry;
20+
import org.hibernate.testing.orm.junit.SessionFactory;
21+
import org.hibernate.testing.orm.junit.SessionFactoryScope;
22+
import org.hibernate.testing.orm.junit.Setting;
23+
import org.hibernate.type.SqlTypes;
24+
import org.hibernate.type.descriptor.jdbc.ArrayJdbcType;
25+
import org.hibernate.type.descriptor.jdbc.JdbcType;
26+
import org.hibernate.type.descriptor.jdbc.spi.JdbcTypeRegistry;
27+
import org.junit.jupiter.api.Test;
28+
29+
import java.util.List;
30+
31+
import static org.hamcrest.CoreMatchers.instanceOf;
32+
import static org.hamcrest.MatcherAssert.assertThat;
33+
import static org.hamcrest.Matchers.equalTo;
34+
import static org.hamcrest.Matchers.isA;
35+
36+
// It's vital that EntityWithJson is listed first to reproduce the bug,
37+
// the bug being, that JsonJavaType was registered in JavaTypeRegistry under e.g. List<Integer>,
38+
// which would then wrongly be used for EntityWithArray#listInteger as JavaType
39+
@DomainModel(annotatedClasses = { JsonAndArrayMappingTests.EntityWithJson.class, JsonAndArrayMappingTests.EntityWithArray.class})
40+
@SessionFactory
41+
@ServiceRegistry(settings = @Setting(name = AvailableSettings.JSON_FORMAT_MAPPER, value = "jackson"))
42+
@RequiresDialectFeature(feature = DialectFeatureChecks.SupportsTypedArrays.class)
43+
@Jira("https://hibernate.atlassian.net/browse/HHH-17680")
44+
public class JsonAndArrayMappingTests {
45+
46+
@Test
47+
public void verifyMappings(SessionFactoryScope scope) {
48+
final MappingMetamodelImplementor mappingMetamodel = scope.getSessionFactory()
49+
.getRuntimeMetamodels()
50+
.getMappingMetamodel();
51+
final JdbcTypeRegistry jdbcTypeRegistry = mappingMetamodel.getTypeConfiguration().getJdbcTypeRegistry();
52+
final JdbcType jsonType = jdbcTypeRegistry.getDescriptor( SqlTypes.JSON );
53+
final EntityPersister jsonEntity = mappingMetamodel.findEntityDescriptor( EntityWithJson.class );
54+
55+
final BasicAttributeMapping listStringJsonAttribute = (BasicAttributeMapping) jsonEntity.findAttributeMapping(
56+
"listString" );
57+
final BasicAttributeMapping listIntegerJsonAttribute = (BasicAttributeMapping) jsonEntity.findAttributeMapping(
58+
"listInteger" );
59+
60+
assertThat( listStringJsonAttribute.getJavaType().getJavaTypeClass(), equalTo( List.class ) );
61+
assertThat( listIntegerJsonAttribute.getJavaType().getJavaTypeClass(), equalTo( List.class ) );
62+
63+
assertThat( listStringJsonAttribute.getJdbcMapping().getJdbcType(), isA( (Class<JdbcType>) jsonType.getClass() ) );
64+
assertThat( listIntegerJsonAttribute.getJdbcMapping().getJdbcType(), isA( (Class<JdbcType>) jsonType.getClass() ) );
65+
66+
final EntityPersister arrayEntity = mappingMetamodel.findEntityDescriptor( EntityWithArray.class );
67+
68+
final BasicAttributeMapping listStringArrayAttribute = (BasicAttributeMapping) arrayEntity.findAttributeMapping(
69+
"listString" );
70+
final BasicAttributeMapping listIntegerArrayAttribute = (BasicAttributeMapping) arrayEntity.findAttributeMapping(
71+
"listInteger" );
72+
73+
assertThat( listStringArrayAttribute.getJavaType().getJavaTypeClass(), equalTo( List.class ) );
74+
assertThat( listIntegerArrayAttribute.getJavaType().getJavaTypeClass(), equalTo( List.class ) );
75+
76+
assertThat( listStringArrayAttribute.getJdbcMapping().getJdbcType(), instanceOf( ArrayJdbcType.class ) );
77+
assertThat( listIntegerArrayAttribute.getJdbcMapping().getJdbcType(), instanceOf( ArrayJdbcType.class ) );
78+
}
79+
80+
@Entity(name = "EntityWithJson")
81+
@Table(name = "EntityWithJson")
82+
public static class EntityWithJson {
83+
@Id
84+
private Integer id;
85+
86+
@JdbcTypeCode( SqlTypes.JSON )
87+
private List<String> listString;
88+
@JdbcTypeCode( SqlTypes.JSON )
89+
private List<Integer> listInteger;
90+
91+
public EntityWithJson() {
92+
}
93+
}
94+
95+
@Entity(name = "EntityWithArray")
96+
@Table(name = "EntityWithArray")
97+
public static class EntityWithArray {
98+
@Id
99+
private Integer id;
100+
101+
@JdbcTypeCode( SqlTypes.ARRAY )
102+
private List<String> listString;
103+
@JdbcTypeCode( SqlTypes.ARRAY )
104+
private List<Integer> listInteger;
105+
106+
public EntityWithArray() {
107+
}
108+
}
109+
}

0 commit comments

Comments
 (0)