Skip to content

HHH-17680 Avoid JavaType collisions with special JavaTypes for JSON/XML #11366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: 6.6
Choose a base branch
from
+163 −17
Open

HHH-17680 Avoid JavaType collisions with special JavaTypes for JSON/XML #11366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 19 additions & 13 deletions hibernate-core/src/main/java/org/hibernate/mapping/BasicValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -774,12 +774,12 @@ else if ( ownerName != null && propertyName != null ) {
}
}

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

private JavaType<Object> specialJavaType(
private JavaType<?> specialJavaType(
TypeConfiguration typeConfiguration,
java.lang.reflect.Type impliedJavaType) {
final JavaTypeRegistry javaTypeRegistry = typeConfiguration.getJavaTypeRegistry();
Expand All @@ -788,19 +788,25 @@ private JavaType<Object> specialJavaType(
// and implement toString/fromString as well as copying based on FormatMapper operations
switch ( jdbcTypeCode ) {
case SqlTypes.JSON:
final JavaType<Object> jsonJavaType =
new JsonJavaType<>( impliedJavaType,
return javaTypeRegistry.resolveDescriptor(
SqlTypes.JSON,
impliedJavaType,
() -> new JsonJavaType<>(
impliedJavaType,
mutabilityPlan( typeConfiguration, impliedJavaType ),
typeConfiguration );
javaTypeRegistry.addDescriptor( jsonJavaType );
return jsonJavaType;
typeConfiguration
)
);
case SqlTypes.SQLXML:
final JavaType<Object> xmlJavaType =
new XmlJavaType<>( impliedJavaType,
return javaTypeRegistry.resolveDescriptor(
SqlTypes.SQLXML,
impliedJavaType,
() -> new XmlJavaType<>(
impliedJavaType,
mutabilityPlan( typeConfiguration, impliedJavaType ),
typeConfiguration );
javaTypeRegistry.addDescriptor( xmlJavaType );
return xmlJavaType;
typeConfiguration
)
);
}
}
return javaTypeRegistry.resolveDescriptor( impliedJavaType );
Expand Down
39 changes: 35 additions & 4 deletions hibernate-core/src/main/java/org/hibernate/type/descriptor/java/spi/JavaTypeRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.util.function.Consumer;
import java.util.function.Supplier;

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

private final TypeConfiguration typeConfiguration;
private final ConcurrentHashMap<Type, JavaType<?>> descriptorsByType = new ConcurrentHashMap<>();
private final ConcurrentHashMap<Integer, ConcurrentHashMap<Type, JavaType<?>>> typeCodeSpecificDescriptorsByType = new ConcurrentHashMap<>();

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

public void forEachDescriptor(Consumer<JavaType<?>> consumer) {
descriptorsByType.values().forEach( consumer );
typeCodeSpecificDescriptorsByType.values().forEach( descriptorsByTypeName -> {
descriptorsByTypeName.values().forEach( consumer );
} );
}

public <T> JavaType<T> getDescriptor(Type javaType) {
return resolveDescriptor( javaType );
}

public void addDescriptor(JavaType<?> descriptor) {
addDescriptor( descriptorsByType, descriptor );
}

public void addDescriptor(int sqlTypeCode, JavaType<?> descriptor) {
final ConcurrentHashMap<Type, JavaType<?>> descriptorsByTypeName =
typeCodeSpecificDescriptorsByType.computeIfAbsent( sqlTypeCode, k -> new ConcurrentHashMap<>() );
addDescriptor( descriptorsByTypeName, descriptor );
}

private void addDescriptor(ConcurrentHashMap<Type, JavaType<?>> descriptorsByType, JavaType<?> descriptor) {
JavaType<?> old = descriptorsByType.put( descriptor.getJavaType(), descriptor );
if ( old != null ) {
log.debugf(
Expand All @@ -93,19 +108,35 @@ public void addDescriptor(JavaType<?> descriptor) {
performInjections( descriptor );
}

public <J> JavaType<J> findDescriptor(Type javaType) {
public <J> @Nullable JavaType<J> findDescriptor(Type javaType) {
//noinspection unchecked
return (JavaType<J>) descriptorsByType.get( javaType );
}

public @Nullable JavaType<?> findDescriptor(int sqlTypeCode, Type javaType) {
final ConcurrentHashMap<Type, JavaType<?>> descriptorsByType =
typeCodeSpecificDescriptorsByType.get( sqlTypeCode );
return descriptorsByType.get( javaType );
}

public <J> JavaType<J> resolveDescriptor(Type javaType, Supplier<JavaType<J>> creator) {
//noinspection unchecked
return (JavaType<J>) resolveDescriptor( descriptorsByType, javaType, creator );
}

public JavaType<?> resolveDescriptor(int sqlTypeCode, Type javaType, Supplier<JavaType<?>> creator) {
final ConcurrentHashMap<Type, JavaType<?>> descriptorsByType =
typeCodeSpecificDescriptorsByType.computeIfAbsent( sqlTypeCode, k -> new ConcurrentHashMap<>() );
return resolveDescriptor( descriptorsByType, javaType, creator );
}

private JavaType<?> resolveDescriptor(ConcurrentHashMap<Type, JavaType<?>> descriptorsByType, Type javaType, Supplier<? extends JavaType<?>> creator) {
final JavaType<?> cached = descriptorsByType.get( javaType );
if ( cached != null ) {
//noinspection unchecked
return (JavaType<J>) cached;
return cached;
}

final JavaType<J> created = creator.get();
final JavaType<?> created = creator.get();
descriptorsByType.put( javaType, created );
return created;
}
Expand Down
109 changes: 109 additions & 0 deletions ...ate-core/src/test/java/org/hibernate/orm/test/mapping/basic/JsonAndArrayMappingTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/*
* SPDX-License-Identifier: Apache-2.0
* Copyright Red Hat Inc. and Hibernate Authors
*/
package org.hibernate.orm.test.mapping.basic;

import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import jakarta.persistence.Table;
import org.hibernate.annotations.JdbcTypeCode;
import org.hibernate.cfg.AvailableSettings;
import org.hibernate.metamodel.mapping.internal.BasicAttributeMapping;
import org.hibernate.metamodel.spi.MappingMetamodelImplementor;
import org.hibernate.persister.entity.EntityPersister;
import org.hibernate.testing.orm.junit.DialectFeatureChecks;
import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.Jira;
import org.hibernate.testing.orm.junit.RequiresDialectFeature;
import org.hibernate.testing.orm.junit.ServiceRegistry;
import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.SessionFactoryScope;
import org.hibernate.testing.orm.junit.Setting;
import org.hibernate.type.SqlTypes;
import org.hibernate.type.descriptor.jdbc.ArrayJdbcType;
import org.hibernate.type.descriptor.jdbc.JdbcType;
import org.hibernate.type.descriptor.jdbc.spi.JdbcTypeRegistry;
import org.junit.jupiter.api.Test;

import java.util.List;

import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.isA;

// It's vital that EntityWithJson is listed first to reproduce the bug,
// the bug being, that JsonJavaType was registered in JavaTypeRegistry under e.g. List<Integer>,
// which would then wrongly be used for EntityWithArray#listInteger as JavaType
@DomainModel(annotatedClasses = { JsonAndArrayMappingTests.EntityWithJson.class, JsonAndArrayMappingTests.EntityWithArray.class})
@SessionFactory
@ServiceRegistry(settings = @Setting(name = AvailableSettings.JSON_FORMAT_MAPPER, value = "jackson"))
@RequiresDialectFeature(feature = DialectFeatureChecks.SupportsTypedArrays.class)
@Jira("https://hibernate.atlassian.net/browse/HHH-17680")
public class JsonAndArrayMappingTests {

@Test
public void verifyMappings(SessionFactoryScope scope) {
final MappingMetamodelImplementor mappingMetamodel = scope.getSessionFactory()
.getRuntimeMetamodels()
.getMappingMetamodel();
final JdbcTypeRegistry jdbcTypeRegistry = mappingMetamodel.getTypeConfiguration().getJdbcTypeRegistry();
final JdbcType jsonType = jdbcTypeRegistry.getDescriptor( SqlTypes.JSON );
final EntityPersister jsonEntity = mappingMetamodel.findEntityDescriptor( EntityWithJson.class );

final BasicAttributeMapping listStringJsonAttribute = (BasicAttributeMapping) jsonEntity.findAttributeMapping(
"listString" );
final BasicAttributeMapping listIntegerJsonAttribute = (BasicAttributeMapping) jsonEntity.findAttributeMapping(
"listInteger" );

assertThat( listStringJsonAttribute.getJavaType().getJavaTypeClass(), equalTo( List.class ) );
assertThat( listIntegerJsonAttribute.getJavaType().getJavaTypeClass(), equalTo( List.class ) );

assertThat( listStringJsonAttribute.getJdbcMapping().getJdbcType(), isA( (Class<JdbcType>) jsonType.getClass() ) );
assertThat( listIntegerJsonAttribute.getJdbcMapping().getJdbcType(), isA( (Class<JdbcType>) jsonType.getClass() ) );

final EntityPersister arrayEntity = mappingMetamodel.findEntityDescriptor( EntityWithArray.class );

final BasicAttributeMapping listStringArrayAttribute = (BasicAttributeMapping) arrayEntity.findAttributeMapping(
"listString" );
final BasicAttributeMapping listIntegerArrayAttribute = (BasicAttributeMapping) arrayEntity.findAttributeMapping(
"listInteger" );

assertThat( listStringArrayAttribute.getJavaType().getJavaTypeClass(), equalTo( List.class ) );
assertThat( listIntegerArrayAttribute.getJavaType().getJavaTypeClass(), equalTo( List.class ) );

assertThat( listStringArrayAttribute.getJdbcMapping().getJdbcType(), instanceOf( ArrayJdbcType.class ) );
assertThat( listIntegerArrayAttribute.getJdbcMapping().getJdbcType(), instanceOf( ArrayJdbcType.class ) );
}

@Entity(name = "EntityWithJson")
@Table(name = "EntityWithJson")
public static class EntityWithJson {
@Id
private Integer id;

@JdbcTypeCode( SqlTypes.JSON )
private List<String> listString;
@JdbcTypeCode( SqlTypes.JSON )
private List<Integer> listInteger;

public EntityWithJson() {
}
}

@Entity(name = "EntityWithArray")
@Table(name = "EntityWithArray")
public static class EntityWithArray {
@Id
private Integer id;

@JdbcTypeCode( SqlTypes.ARRAY )
private List<String> listString;
@JdbcTypeCode( SqlTypes.ARRAY )
private List<Integer> listInteger;

public EntityWithArray() {
}
}
}
Loading