Skip to content

Commit

Permalink
HHH-15733 Change convert logic to default to value for Map collection…
Browse files Browse the repository at this point in the history
…s of basic types
  • Loading branch information
mbladel authored and beikov committed Feb 6, 2023
1 parent 1bb6fcf commit f4e95d9
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2131,15 +2131,17 @@ private void handleElementCollection(XClass elementType, String hqlOrderBy) {
propertyHolder,
buildingContext
);
holder.prepare( property );

final Class<? extends CompositeUserType<?>> compositeUserType =
resolveCompositeUserType( property, elementClass, buildingContext );
if ( classType == EMBEDDABLE || compositeUserType != null ) {
boolean isComposite = classType == EMBEDDABLE || compositeUserType != null;
holder.prepare( property, isComposite );

if ( isComposite ) {
handleCompositeCollectionElement( hqlOrderBy, elementClass, holder, compositeUserType );
}
else {
handleCollectionElement( elementType, hqlOrderBy, elementClass, holder );
handleCollectionElement( elementType, hqlOrderBy, elementClass, holder );
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ public Collection getCollectionBinding() {

private void buildAttributeConversionInfoMaps(
XProperty collectionProperty,
boolean isComposite,
Map<String,AttributeConversionInfo> elementAttributeConversionInfoMap,
Map<String,AttributeConversionInfo> keyAttributeConversionInfoMap) {
if ( collectionProperty == null ) {
Expand All @@ -88,7 +89,13 @@ private void buildAttributeConversionInfoMaps(
{
final Convert convertAnnotation = collectionProperty.getAnnotation( Convert.class );
if ( convertAnnotation != null ) {
applyLocalConvert( convertAnnotation, collectionProperty, elementAttributeConversionInfoMap, keyAttributeConversionInfoMap );
applyLocalConvert(
convertAnnotation,
collectionProperty,
isComposite,
elementAttributeConversionInfoMap,
keyAttributeConversionInfoMap
);
}
}

Expand All @@ -99,6 +106,7 @@ private void buildAttributeConversionInfoMaps(
applyLocalConvert(
convertAnnotation,
collectionProperty,
isComposite,
elementAttributeConversionInfoMap,
keyAttributeConversionInfoMap
);
Expand All @@ -110,14 +118,15 @@ private void buildAttributeConversionInfoMaps(
private void applyLocalConvert(
Convert convertAnnotation,
XProperty collectionProperty,
boolean isComposite,
Map<String,AttributeConversionInfo> elementAttributeConversionInfoMap,
Map<String,AttributeConversionInfo> keyAttributeConversionInfoMap) {

// IMPL NOTE : the rules here are quite more lenient than what JPA says. For example, JPA says
// that @Convert on a Map always needs to specify attributeName of key/value (or prefixed with
// key./value. for embedded paths). However, we try to see if conversion of either is disabled
// for whatever reason. For example, if the Map is annotated with @Enumerated the elements cannot
// be converted so any @Convert likely meant the key, so we apply it to the key
// IMPL NOTE : the rules here are quite more lenient than what JPA says. For example, JPA says that @Convert
// on a Map of basic types should default to "value" but it should explicitly specify attributeName of "key"
// (or prefixed with "key." for embedded paths) to be applied on the key. However, we try to see if conversion
// of either is disabled for whatever reason. For example, if the Map is annotated with @Enumerated the
// elements cannot be converted so any @Convert likely meant the key, so we apply it to the key

final AttributeConversionInfo info = new AttributeConversionInfo( convertAnnotation, collectionProperty );
if ( collection.isMap() ) {
Expand All @@ -132,10 +141,15 @@ private void applyLocalConvert(
if ( isEmpty( info.getAttributeName() ) ) {
// the @Convert did not name an attribute...
if ( canElementBeConverted && canKeyBeConverted ) {
throw new IllegalStateException(
"@Convert placed on Map attribute [" + collection.getRole()
+ "] must define attributeName of 'key' or 'value'"
);
if ( !isComposite ) {
// if element is of basic type default to "value"
elementAttributeConversionInfoMap.put( "", info );
}
else {
throw new IllegalStateException(
"@Convert placed on Map attribute [" + collection.getRole()
+ "] of non-basic types must define attributeName of 'key' or 'value'" );
}
}
else if ( canKeyBeConverted ) {
keyAttributeConversionInfoMap.put( "", info );
Expand Down Expand Up @@ -325,7 +339,7 @@ public String toString() {

boolean prepared;

public void prepare(XProperty collectionProperty) {
public void prepare(XProperty collectionProperty, boolean isComposite) {
// fugly
if ( prepared ) {
return;
Expand Down Expand Up @@ -377,7 +391,12 @@ else if ( collectionProperty.isAnnotationPresent( CollectionType.class ) ) {
// Is it valid to reference a collection attribute in a @Convert attached to the owner (entity) by path?
// if so we should pass in 'clazzToProcess' also
if ( canKeyBeConverted || canElementBeConverted ) {
buildAttributeConversionInfoMaps( collectionProperty, elementAttributeConversionInfoMap, keyAttributeConversionInfoMap );
buildAttributeConversionInfoMaps(
collectionProperty,
isComposite,
elementAttributeConversionInfoMap,
keyAttributeConversionInfoMap
);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.hibernate.mapping.Table;
import org.hibernate.mapping.Value;
import org.hibernate.resource.beans.spi.ManagedBean;
import org.hibernate.type.BasicType;
import org.hibernate.usertype.CompositeUserType;
import org.hibernate.usertype.UserCollectionType;

Expand Down Expand Up @@ -278,7 +279,7 @@ private CollectionPropertyHolder buildCollectionPropertyHolder(
// 'holder' is the CollectionPropertyHolder.
// 'property' is the collection XProperty
propertyHolder.startingProperty( property );
holder.prepare(property);
holder.prepare( property, !( collection.getKey().getType() instanceof BasicType ) );
return holder;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,9 @@ public static class Customer {
private Integer id;

@ElementCollection(fetch = FetchType.EAGER)
@Convert(converter = MyStringConverter.class)
@Convert(converter = MyStringConverter.class) // note omitted attributeName
private final Map<String, String> colors = new HashMap<>();

public Customer() {
}

public Customer(Integer id) {
this.id = id;
}

public Map<String, String> getColors() {
return colors;
}
Expand Down

0 comments on commit f4e95d9

Please sign in to comment.