HHH-10277 - AttributeConverter not applied to attributes of an embeddable used as collection element

This commit is contained in:
Steve Ebersole 2016-06-16 11:09:13 -05:00
parent 309b1b27b0
commit b7f17ce898
5 changed files with 98 additions and 44 deletions

View File

@ -7,6 +7,7 @@
package org.hibernate.cfg;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import javax.persistence.Convert;
import javax.persistence.Converts;
@ -144,15 +145,30 @@ public class CollectionPropertyHolder extends AbstractPropertyHolder {
}
else {
// the @Convert named an attribute...
final String keyPath = removePrefix( info.getAttributeName(), "key" );
final String elementPath = removePrefix( info.getAttributeName(), "value" );
if ( canElementBeConverted && canKeyBeConverted && keyPath == null && elementPath == null ) {
// specified attributeName needs to have 'key.' or 'value.' prefix
throw new IllegalStateException(
"@Convert placed on Map attribute [" + collection.getRole()
+ "] must define attributeName of 'key' or 'value'"
);
// we have different "resolution rules" based on whether element and key can be converted
final String keyPath;
final String elementPath;
if ( canElementBeConverted && canKeyBeConverted ) {
keyPath = removePrefix( info.getAttributeName(), "key" );
elementPath = removePrefix( info.getAttributeName(), "value" );
if ( keyPath == null && elementPath == null ) {
// specified attributeName needs to have 'key.' or 'value.' prefix
throw new IllegalStateException(
"@Convert placed on Map attribute [" + collection.getRole()
+ "] must define attributeName of 'key' or 'value'"
);
}
}
else if ( canKeyBeConverted ) {
keyPath = removePrefix( info.getAttributeName(), "key", info.getAttributeName() );
elementPath = null;
}
else {
keyPath = null;
elementPath = removePrefix( info.getAttributeName(), "value", info.getAttributeName() );
}
if ( keyPath != null ) {
@ -161,6 +177,17 @@ public class CollectionPropertyHolder extends AbstractPropertyHolder {
else if ( elementPath != null ) {
elementAttributeConversionInfoMap.put( elementPath, info );
}
else {
// specified attributeName needs to have 'key.' or 'value.' prefix
throw new IllegalStateException(
String.format(
Locale.ROOT,
"Could not determine how to apply @Convert(attributeName='%s') to collection [%s]",
info.getAttributeName(),
collection.getRole()
)
);
}
}
}
@ -172,6 +199,10 @@ public class CollectionPropertyHolder extends AbstractPropertyHolder {
* @return Path without prefix, or null, if path did not have the prefix.
*/
private String removePrefix(String path, String prefix) {
return removePrefix( path, prefix, null );
}
private String removePrefix(String path, String prefix, String defaultValue) {
if ( path.equals(prefix) ) {
return "";
}
@ -180,7 +211,7 @@ public class CollectionPropertyHolder extends AbstractPropertyHolder {
return path.substring( prefix.length() + 1 );
}
return null;
return defaultValue;
}
@Override

View File

@ -64,7 +64,7 @@ public class ComponentPropertyHolder extends AbstractPropertyHolder {
private Component component;
private boolean isOrWithinEmbeddedId;
private boolean virtual;
// private boolean virtual;
private String embeddedAttributeName;
private Map<String,AttributeConversionInfo> attributeConversionInfoMap;
@ -84,14 +84,20 @@ public class ComponentPropertyHolder extends AbstractPropertyHolder {
( embeddedXProperty.isAnnotationPresent( Id.class )
|| embeddedXProperty.isAnnotationPresent( EmbeddedId.class ) ) );
this.virtual = embeddedXProperty == null;
if ( !virtual ) {
if ( embeddedXProperty != null ) {
// this.virtual = false;
this.embeddedAttributeName = embeddedXProperty.getName();
this.attributeConversionInfoMap = processAttributeConversions( embeddedXProperty );
}
else {
embeddedAttributeName = "";
this.attributeConversionInfoMap = Collections.emptyMap();
// could be either:
// 1) virtual/dynamic component
// 2) collection element/key
// temp
// this.virtual = true;
this.embeddedAttributeName = "";
this.attributeConversionInfoMap = processAttributeConversions( inferredData.getClassOrElement() );
}
}
@ -119,30 +125,7 @@ public class ComponentPropertyHolder extends AbstractPropertyHolder {
// from the Embedded
// first apply conversions from the Embeddable...
{
// @Convert annotation on the Embeddable class level
final Convert convertAnnotation = embeddableXClass.getAnnotation( Convert.class );
if ( convertAnnotation != null ) {
final AttributeConversionInfo info = new AttributeConversionInfo( convertAnnotation, embeddableXClass );
if ( StringHelper.isEmpty( info.getAttributeName() ) ) {
throw new IllegalStateException( "@Convert placed on @Embeddable must define attributeName" );
}
infoMap.put( info.getAttributeName(), info );
}
}
{
// @Converts annotation on the Embeddable class level
final Converts convertsAnnotation = embeddableXClass.getAnnotation( Converts.class );
if ( convertsAnnotation != null ) {
for ( Convert convertAnnotation : convertsAnnotation.value() ) {
final AttributeConversionInfo info = new AttributeConversionInfo( convertAnnotation, embeddableXClass );
if ( StringHelper.isEmpty( info.getAttributeName() ) ) {
throw new IllegalStateException( "@Converts placed on @Embeddable must define attributeName" );
}
infoMap.put( info.getAttributeName(), info );
}
}
}
processAttributeConversions( embeddableXClass, infoMap );
// then we can overlay any conversions from the Embedded attribute
{
@ -173,6 +156,39 @@ public class ComponentPropertyHolder extends AbstractPropertyHolder {
return infoMap;
}
private void processAttributeConversions(XClass embeddableXClass, Map<String, AttributeConversionInfo> infoMap) {
{
// @Convert annotation on the Embeddable class level
final Convert convertAnnotation = embeddableXClass.getAnnotation( Convert.class );
if ( convertAnnotation != null ) {
final AttributeConversionInfo info = new AttributeConversionInfo( convertAnnotation, embeddableXClass );
if ( StringHelper.isEmpty( info.getAttributeName() ) ) {
throw new IllegalStateException( "@Convert placed on @Embeddable must define attributeName" );
}
infoMap.put( info.getAttributeName(), info );
}
}
{
// @Converts annotation on the Embeddable class level
final Converts convertsAnnotation = embeddableXClass.getAnnotation( Converts.class );
if ( convertsAnnotation != null ) {
for ( Convert convertAnnotation : convertsAnnotation.value() ) {
final AttributeConversionInfo info = new AttributeConversionInfo( convertAnnotation, embeddableXClass );
if ( StringHelper.isEmpty( info.getAttributeName() ) ) {
throw new IllegalStateException( "@Converts placed on @Embeddable must define attributeName" );
}
infoMap.put( info.getAttributeName(), info );
}
}
}
}
private Map<String,AttributeConversionInfo> processAttributeConversions(XClass embeddableXClass) {
final Map<String,AttributeConversionInfo> infoMap = new HashMap<String, AttributeConversionInfo>();
processAttributeConversions( embeddableXClass, infoMap );
return infoMap;
}
@Override
protected String normalizeCompositePath(String attributeName) {
return embeddedAttributeName + '.' + attributeName;
@ -189,9 +205,9 @@ public class ComponentPropertyHolder extends AbstractPropertyHolder {
return;
}
if ( virtual ) {
return;
}
// if ( virtual ) {
// return;
// }
// again : the property coming in here *should* be the property on the embeddable (Address#city in the example),
// so we just ignore it if there is already an existing conversion info for that path since they would have
@ -239,7 +255,7 @@ public class ComponentPropertyHolder extends AbstractPropertyHolder {
@Override
protected AttributeConversionInfo locateAttributeConversionInfo(String path) {
final String embeddedPath = embeddedAttributeName + '.' + path;
final String embeddedPath = StringHelper.qualifyConditionally( embeddedAttributeName, path );
AttributeConversionInfo fromParent = parent.locateAttributeConversionInfo( embeddedPath );
if ( fromParent != null ) {
return fromParent;

View File

@ -1437,6 +1437,8 @@ public abstract class CollectionBinder {
}
if ( AnnotatedClassType.EMBEDDABLE.equals( classType ) ) {
holder.prepare( property );
EntityBinder entityBinder = new EntityBinder();
PersistentClass owner = collValue.getOwner();
boolean isPropertyAnnotated;

View File

@ -496,6 +496,13 @@ public final class StringHelper {
return prefix + '.' + name;
}
public static String qualifyConditionally(String prefix, String name) {
if ( name == null ) {
throw new NullPointerException( "name was null attempting to build qualified name" );
}
return isEmpty( prefix ) ? name : prefix + '.' + name;
}
public static String[] qualify(String prefix, String[] names) {
if ( prefix == null ) {
return names;

View File

@ -32,7 +32,6 @@ import org.hibernate.mapping.PersistentClass;
import org.hibernate.mapping.Property;
import org.hibernate.mapping.SimpleValue;
import org.hibernate.testing.FailureExpected;
import org.hibernate.testing.TestForIssue;
import org.hibernate.testing.junit4.BaseUnitTestCase;
import org.junit.After;
@ -68,7 +67,6 @@ public class CollectionCompositeElementExplicitConversionTest extends BaseUnitTe
}
@Test
@FailureExpected( jiraKey = "HHH-10277" )
public void testCollectionOfEmbeddablesWithConvertedAttributes() throws Exception {
final MetadataImplementor metadata = (MetadataImplementor) new MetadataSources( ssr )
.addAnnotatedClass( Disguise.class )