Skip to content

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

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
beikov wants to merge 1 commit into
base: main
Choose a base branch
from
+169 −12
Open

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

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
26 changes: 16 additions & 10 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 @@ -764,19 +764,25 @@ private JavaType<?> specialJavaType(
// and implement toString/fromString as well as copying based on FormatMapper operations
switch ( jdbcTypeCode ) {
case SqlTypes.JSON:
final var 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 var 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
47 changes: 45 additions & 2 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 @@ -21,6 +21,7 @@
import java.util.function.Consumer;
import java.util.function.Supplier;

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


Expand All @@ -38,6 +39,7 @@

private final TypeConfiguration typeConfiguration;
private final ConcurrentHashMap<String, JavaType<?>> descriptorsByTypeName = new ConcurrentHashMap<>();
private final ConcurrentHashMap<Integer, ConcurrentHashMap<String, JavaType<?>>> typeCodeSpecificDescriptorsByTypeName = new ConcurrentHashMap<>();

public JavaTypeRegistry(TypeConfiguration typeConfiguration) {
this.typeConfiguration = typeConfiguration;
Expand Down Expand Up @@ -75,9 +77,22 @@

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

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

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

private void addDescriptor(ConcurrentHashMap<String, JavaType<?>> descriptorsByTypeName, JavaType<?> descriptor) {
final var old = descriptorsByTypeName.put( descriptor.getJavaType().getTypeName(), descriptor );
if ( old != null ) {
LOG.debugf(
Expand All @@ -99,15 +114,26 @@
return (JavaType<T>) resolveDescriptor( javaType );
}

public JavaType<?> findDescriptor(Type javaType) {
public @Nullable JavaType<?> findDescriptor(Type javaType) {
return descriptorsByTypeName.get( javaType.getTypeName() );
}

public <J> JavaType<J> findDescriptor(Class<J> javaClass) {
public @Nullable JavaType<?> findDescriptor(int sqlTypeCode, Type javaType) {
final ConcurrentHashMap<String, JavaType<?>> descriptorsByTypeName =
typeCodeSpecificDescriptorsByTypeName.get( sqlTypeCode );
return descriptorsByTypeName.get( javaType.getTypeName() );
}

public <J> @Nullable JavaType<J> findDescriptor(Class<J> javaClass) {

Check notice

Code scanning / CodeQL

Confusing overloading of methods Note

Method JavaTypeRegistry.findDescriptor(..) could be confused with overloaded method
findDescriptor
Loading
, since dispatch depends on static types.
final var cached = descriptorsByTypeName.get( javaClass.getTypeName() );
return cached == null ? null : checkCached( javaClass, cached );
}

public <J> @Nullable JavaType<J> findDescriptor(int sqlTypeCode, Class<J> javaType) {
//noinspection unchecked
return (JavaType<J>) findDescriptor( sqlTypeCode, (Type) javaType );
}

public <J> JavaType<J> resolveDescriptor(Class<? extends J> javaType, Supplier<JavaType<J>> creator) {
final String javaTypeName = javaType.getTypeName();
final var cached = descriptorsByTypeName.get( javaTypeName );
Expand Down Expand Up @@ -135,8 +161,25 @@
return cachedClass == canonicalize( givenClass );
}

public <J> JavaType<J> resolveDescriptor(int sqlTypeCode, Class<? extends J> javaType, Supplier<JavaType<J>> creator) {
final ConcurrentHashMap<String, JavaType<?>> descriptorsByTypeName =
typeCodeSpecificDescriptorsByTypeName.computeIfAbsent( sqlTypeCode, k -> new ConcurrentHashMap<>() );
//noinspection unchecked
return (JavaType<J>) resolveDescriptor( descriptorsByTypeName, javaType.getTypeName(), creator );
}

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

@Deprecated(since = "7.2", forRemoval = true) // Can be private
private JavaType<?> resolveDescriptor(String javaTypeName, Supplier<? extends JavaType<?>> creator) {
return resolveDescriptor( descriptorsByTypeName, javaTypeName, creator );
}

private JavaType<?> resolveDescriptor(ConcurrentHashMap<String, JavaType<?>> descriptorsByTypeName, String javaTypeName, Supplier<? extends JavaType<?>> creator) {
final var cached = descriptorsByTypeName.get( javaTypeName );
if ( cached != null ) {
return cached;
Expand Down
108 changes: 108 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,108 @@
/*
* 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.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(), isA( ArrayJdbcType.class ) );
assertThat( listIntegerArrayAttribute.getJdbcMapping().getJdbcType(), isA( 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() {
}
}
}