Skip to content

Commit ff4058e

Browse files
beikovbeikov
committed
HHH-17680 Avoid JavaType collisions with special JavaTypes for JSON/XML
1 parent 93c4950 commit ff4058e

File tree

3 files changed

+169
-12
lines changed

3 files changed

+169
-12
lines changed

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

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -783,19 +783,25 @@ private JavaType<?> specialJavaType(
783783
// and implement toString/fromString as well as copying based on FormatMapper operations
784784
switch ( jdbcTypeCode ) {
785785
case SqlTypes.JSON:
786-
final var jsonJavaType =
787-
new JsonJavaType<>( impliedJavaType,
786+
return javaTypeRegistry.resolveDescriptor(
787+
SqlTypes.JSON,
788+
impliedJavaType,
789+
() -> new JsonJavaType<>(
790+
impliedJavaType,
788791
mutabilityPlan( typeConfiguration, impliedJavaType ),
789-
typeConfiguration );
790-
javaTypeRegistry.addDescriptor( jsonJavaType );
791-
return jsonJavaType;
792+
typeConfiguration
793+
)
794+
);
792795
case SqlTypes.SQLXML:
793-
final var xmlJavaType =
794-
new XmlJavaType<>( impliedJavaType,
796+
return javaTypeRegistry.resolveDescriptor(
797+
SqlTypes.SQLXML,
798+
impliedJavaType,
799+
() -> new XmlJavaType<>(
800+
impliedJavaType,
795801
mutabilityPlan( typeConfiguration, impliedJavaType ),
796-
typeConfiguration );
797-
javaTypeRegistry.addDescriptor( xmlJavaType );
798-
return xmlJavaType;
802+
typeConfiguration
803+
)
804+
);
799805
}
800806
}
801807
return javaTypeRegistry.resolveDescriptor( impliedJavaType );

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

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.function.Consumer;
2222
import java.util.function.Supplier;
2323

24+
import org.checkerframework.checker.nullness.qual.Nullable;
2425
import static org.hibernate.internal.util.type.PrimitiveWrappers.canonicalize;
2526

2627

@@ -38,6 +39,7 @@ public class JavaTypeRegistry implements JavaTypeBaseline.BaselineTarget, Serial
3839

3940
private final TypeConfiguration typeConfiguration;
4041
private final ConcurrentHashMap<String, JavaType<?>> descriptorsByTypeName = new ConcurrentHashMap<>();
42+
private final ConcurrentHashMap<Integer, ConcurrentHashMap<String, JavaType<?>>> typeCodeSpecificDescriptorsByTypeName = new ConcurrentHashMap<>();
4143

4244
public JavaTypeRegistry(TypeConfiguration typeConfiguration) {
4345
this.typeConfiguration = typeConfiguration;
@@ -75,9 +77,22 @@ private void performInjections(JavaType<?> descriptor) {
7577

7678
public void forEachDescriptor(Consumer<JavaType<?>> consumer) {
7779
descriptorsByTypeName.values().forEach( consumer );
80+
typeCodeSpecificDescriptorsByTypeName.values().forEach( descriptorsByTypeName -> {
81+
descriptorsByTypeName.values().forEach( consumer );
82+
} );
7883
}
7984

8085
public void addDescriptor(JavaType<?> descriptor) {
86+
addDescriptor( descriptorsByTypeName, descriptor );
87+
}
88+
89+
public void addDescriptor(int sqlTypeCode, JavaType<?> descriptor) {
90+
final ConcurrentHashMap<String, JavaType<?>> descriptorsByTypeName =
91+
typeCodeSpecificDescriptorsByTypeName.computeIfAbsent( sqlTypeCode, k -> new ConcurrentHashMap<>() );
92+
addDescriptor( descriptorsByTypeName, descriptor );
93+
}
94+
95+
private void addDescriptor(ConcurrentHashMap<String, JavaType<?>> descriptorsByTypeName, JavaType<?> descriptor) {
8196
final var old = descriptorsByTypeName.put( descriptor.getJavaType().getTypeName(), descriptor );
8297
if ( old != null ) {
8398
LOG.debugf(
@@ -99,15 +114,26 @@ public <T> JavaType<T> getDescriptor(Type javaType) {
99114
return (JavaType<T>) resolveDescriptor( javaType );
100115
}
101116

102-
public JavaType<?> findDescriptor(Type javaType) {
117+
public @Nullable JavaType<?> findDescriptor(Type javaType) {
103118
return descriptorsByTypeName.get( javaType.getTypeName() );
104119
}
105120

106-
public <J> JavaType<J> findDescriptor(Class<J> javaClass) {
121+
public @Nullable JavaType<?> findDescriptor(int sqlTypeCode, Type javaType) {
122+
final ConcurrentHashMap<String, JavaType<?>> descriptorsByTypeName =
123+
typeCodeSpecificDescriptorsByTypeName.get( sqlTypeCode );
124+
return descriptorsByTypeName.get( javaType.getTypeName() );
125+
}
126+
127+
public <J> @Nullable JavaType<J> findDescriptor(Class<J> javaClass) {
107128
final var cached = descriptorsByTypeName.get( javaClass.getTypeName() );
108129
return cached == null ? null : checkCached( javaClass, cached );
109130
}
110131

132+
public <J> @Nullable JavaType<J> findDescriptor(int sqlTypeCode, Class<J> javaType) {
133+
//noinspection unchecked
134+
return (JavaType<J>) findDescriptor( sqlTypeCode, (Type) javaType );
135+
}
136+
111137
public <J> JavaType<J> resolveDescriptor(Class<? extends J> javaType, Supplier<JavaType<J>> creator) {
112138
final String javaTypeName = javaType.getTypeName();
113139
final var cached = descriptorsByTypeName.get( javaTypeName );
@@ -130,8 +156,25 @@ private static <J> JavaType<J> checkCached(Class<? extends J> javaClass, JavaTyp
130156
return resolvedType;
131157
}
132158

159+
public <J> JavaType<J> resolveDescriptor(int sqlTypeCode, Class<? extends J> javaType, Supplier<JavaType<J>> creator) {
160+
final ConcurrentHashMap<String, JavaType<?>> descriptorsByTypeName =
161+
typeCodeSpecificDescriptorsByTypeName.computeIfAbsent( sqlTypeCode, k -> new ConcurrentHashMap<>() );
162+
//noinspection unchecked
163+
return (JavaType<J>) resolveDescriptor( descriptorsByTypeName, javaType.getTypeName(), creator );
164+
}
165+
166+
public JavaType<?> resolveDescriptor(int sqlTypeCode, Type javaType, Supplier<JavaType<?>> creator) {
167+
final ConcurrentHashMap<String, JavaType<?>> descriptorsByTypeName =
168+
typeCodeSpecificDescriptorsByTypeName.computeIfAbsent( sqlTypeCode, k -> new ConcurrentHashMap<>() );
169+
return resolveDescriptor( descriptorsByTypeName, javaType.getTypeName(), creator );
170+
}
171+
133172
@Deprecated(since = "7.2", forRemoval = true) // Can be private
134173
private JavaType<?> resolveDescriptor(String javaTypeName, Supplier<? extends JavaType<?>> creator) {
174+
return resolveDescriptor( descriptorsByTypeName, javaTypeName, creator );
175+
}
176+
177+
private JavaType<?> resolveDescriptor(ConcurrentHashMap<String, JavaType<?>> descriptorsByTypeName, String javaTypeName, Supplier<? extends JavaType<?>> creator) {
135178
final var cached = descriptorsByTypeName.get( javaTypeName );
136179
if ( cached != null ) {
137180
return cached;

‎hibernate-core/src/test/java/org/hibernate/orm/test/mapping/basic/JsonAndArrayMappingTests.java‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
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.MatcherAssert.assertThat;
32+
import static org.hamcrest.Matchers.equalTo;
33+
import static org.hamcrest.Matchers.isA;
34+
35+
// It's vital that EntityWithJson is listed first to reproduce the bug,
36+
// the bug being, that JsonJavaType was registered in JavaTypeRegistry under e.g. List<Integer>,
37+
// which would then wrongly be used for EntityWithArray#listInteger as JavaType
38+
@DomainModel(annotatedClasses = { JsonAndArrayMappingTests.EntityWithJson.class, JsonAndArrayMappingTests.EntityWithArray.class})
39+
@SessionFactory
40+
@ServiceRegistry(settings = @Setting(name = AvailableSettings.JSON_FORMAT_MAPPER, value = "jackson"))
41+
@RequiresDialectFeature(feature = DialectFeatureChecks.SupportsTypedArrays.class)
42+
@Jira("https://hibernate.atlassian.net/browse/HHH-17680")
43+
public class JsonAndArrayMappingTests {
44+
45+
@Test
46+
public void verifyMappings(SessionFactoryScope scope) {
47+
final MappingMetamodelImplementor mappingMetamodel = scope.getSessionFactory()
48+
.getRuntimeMetamodels()
49+
.getMappingMetamodel();
50+
final JdbcTypeRegistry jdbcTypeRegistry = mappingMetamodel.getTypeConfiguration().getJdbcTypeRegistry();
51+
final JdbcType jsonType = jdbcTypeRegistry.getDescriptor( SqlTypes.JSON );
52+
final EntityPersister jsonEntity = mappingMetamodel.findEntityDescriptor( EntityWithJson.class );
53+
54+
final BasicAttributeMapping listStringJsonAttribute = (BasicAttributeMapping) jsonEntity.findAttributeMapping(
55+
"listString" );
56+
final BasicAttributeMapping listIntegerJsonAttribute = (BasicAttributeMapping) jsonEntity.findAttributeMapping(
57+
"listInteger" );
58+
59+
assertThat( listStringJsonAttribute.getJavaType().getJavaTypeClass(), equalTo( List.class ) );
60+
assertThat( listIntegerJsonAttribute.getJavaType().getJavaTypeClass(), equalTo( List.class ) );
61+
62+
assertThat( listStringJsonAttribute.getJdbcMapping().getJdbcType(), isA( (Class<JdbcType>) jsonType.getClass() ) );
63+
assertThat( listIntegerJsonAttribute.getJdbcMapping().getJdbcType(), isA( (Class<JdbcType>) jsonType.getClass() ) );
64+
65+
final EntityPersister arrayEntity = mappingMetamodel.findEntityDescriptor( EntityWithArray.class );
66+
67+
final BasicAttributeMapping listStringArrayAttribute = (BasicAttributeMapping) arrayEntity.findAttributeMapping(
68+
"listString" );
69+
final BasicAttributeMapping listIntegerArrayAttribute = (BasicAttributeMapping) arrayEntity.findAttributeMapping(
70+
"listInteger" );
71+
72+
assertThat( listStringArrayAttribute.getJavaType().getJavaTypeClass(), equalTo( List.class ) );
73+
assertThat( listIntegerArrayAttribute.getJavaType().getJavaTypeClass(), equalTo( List.class ) );
74+
75+
assertThat( listStringArrayAttribute.getJdbcMapping().getJdbcType(), isA( ArrayJdbcType.class ) );
76+
assertThat( listIntegerArrayAttribute.getJdbcMapping().getJdbcType(), isA( ArrayJdbcType.class ) );
77+
}
78+
79+
@Entity(name = "EntityWithJson")
80+
@Table(name = "EntityWithJson")
81+
public static class EntityWithJson {
82+
@Id
83+
private Integer id;
84+
85+
@JdbcTypeCode( SqlTypes.JSON )
86+
private List<String> listString;
87+
@JdbcTypeCode( SqlTypes.JSON )
88+
private List<Integer> listInteger;
89+
90+
public EntityWithJson() {
91+
}
92+
}
93+
94+
@Entity(name = "EntityWithArray")
95+
@Table(name = "EntityWithArray")
96+
public static class EntityWithArray {
97+
@Id
98+
private Integer id;
99+
100+
@JdbcTypeCode( SqlTypes.ARRAY )
101+
private List<String> listString;
102+
@JdbcTypeCode( SqlTypes.ARRAY )
103+
private List<Integer> listInteger;
104+
105+
public EntityWithArray() {
106+
}
107+
}
108+
}

0 commit comments

Comments
 (0)