From 9f86babd880b785b965f7f1b55d58dc6a6943ada Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Wed, 21 Aug 2013 13:14:23 -0500 Subject: [PATCH] HHH-8111 - AttributeConverter doesn't override built-in type mappings --- .../cfg/AttributeConverterDefinition.java | 11 ++ .../cfg/annotations/SimpleValueBinder.java | 136 ++++++++++++--- .../org/hibernate/mapping/SimpleValue.java | 13 +- .../AttributeConverterTypeAdapter.java | 21 ++- .../convert/SimpleConvertAnnotationTest.java | 157 +++++++++++++++++ .../convert/SimpleConvertsAnnotationTest.java | 159 ++++++++++++++++++ 6 files changed, 466 insertions(+), 31 deletions(-) create mode 100644 hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/convert/SimpleConvertAnnotationTest.java create mode 100644 hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/convert/SimpleConvertsAnnotationTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/AttributeConverterDefinition.java b/hibernate-core/src/main/java/org/hibernate/cfg/AttributeConverterDefinition.java index 371f43a546..c43c3017c6 100644 --- a/hibernate-core/src/main/java/org/hibernate/cfg/AttributeConverterDefinition.java +++ b/hibernate-core/src/main/java/org/hibernate/cfg/AttributeConverterDefinition.java @@ -121,4 +121,15 @@ public class AttributeConverterDefinition { return (Class) boundTypes[0]; } + + @Override + public String toString() { + return String.format( + "%s[converterClass=%s, domainType=%s, jdbcType=%s]", + this.getClass().getName(), + attributeConverter.getClass().getName(), + entityAttributeType.getName(), + databaseColumnType.getName() + ); + } } diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/annotations/SimpleValueBinder.java b/hibernate-core/src/main/java/org/hibernate/cfg/annotations/SimpleValueBinder.java index 0ee10369cc..da236a84ab 100644 --- a/hibernate-core/src/main/java/org/hibernate/cfg/annotations/SimpleValueBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/cfg/annotations/SimpleValueBinder.java @@ -306,23 +306,49 @@ public class SimpleValueBinder { } private void applyAttributeConverter(XProperty property) { - final boolean canBeConverted = ! property.isAnnotationPresent( Id.class ) - && ! isVersion - && ! isAssociation() - && ! property.isAnnotationPresent( Temporal.class ) - && ! property.isAnnotationPresent( Enumerated.class ); + if ( attributeConverterDefinition != null ) { + return; + } - if ( canBeConverted ) { - // @Convert annotations take precedence - final Convert convertAnnotation = locateConvertAnnotation( property ); - if ( convertAnnotation != null ) { - if ( ! convertAnnotation.disableConversion() ) { - attributeConverterDefinition = mappings.locateAttributeConverter( convertAnnotation.converter() ); - } - } - else { - attributeConverterDefinition = locateAutoApplyAttributeConverter( property ); - } + LOG.debugf( "Starting applyAttributeConverter [%s:%s]", persistentClassName, property.getName() ); + + if ( property.isAnnotationPresent( Id.class ) ) { + LOG.debugf( "Skipping AttributeConverter checks for Id attribute [%s]", property.getName() ); + return; + } + + if ( isVersion ) { + LOG.debugf( "Skipping AttributeConverter checks for version attribute [%s]", property.getName() ); + return; + } + + if ( property.isAnnotationPresent( Temporal.class ) ) { + LOG.debugf( "Skipping AttributeConverter checks for Temporal attribute [%s]", property.getName() ); + return; + } + + if ( property.isAnnotationPresent( Enumerated.class ) ) { + LOG.debugf( "Skipping AttributeConverter checks for Enumerated attribute [%s]", property.getName() ); + return; + } + + if ( isAssociation() ) { + LOG.debugf( "Skipping AttributeConverter checks for association attribute [%s]", property.getName() ); + return; + } + + // @Convert annotations take precedence if present + final Convert convertAnnotation = locateConvertAnnotation( property ); + if ( convertAnnotation != null ) { + LOG.debugf( + "Applying located @Convert AttributeConverter [%s] to attribute [%]", + convertAnnotation.converter().getName(), + property.getName() + ); + attributeConverterDefinition = mappings.locateAttributeConverter( convertAnnotation.converter() ); + } + else { + attributeConverterDefinition = locateAutoApplyAttributeConverter( property ); } } @@ -347,10 +373,41 @@ public class SimpleValueBinder { @SuppressWarnings("unchecked") private Convert locateConvertAnnotation(XProperty property) { - // first look locally on the property for @Convert - Convert localConvertAnnotation = property.getAnnotation( Convert.class ); - if ( localConvertAnnotation != null ) { - return localConvertAnnotation; + LOG.debugf( + "Attempting to locate Convert annotation for property [%s:%s]", + persistentClassName, + property.getName() + ); + + // first look locally on the property for @Convert/@Converts + { + Convert localConvertAnnotation = property.getAnnotation( Convert.class ); + if ( localConvertAnnotation != null ) { + LOG.debugf( + "Found matching local @Convert annotation [disableConversion=%s]", + localConvertAnnotation.disableConversion() + ); + return localConvertAnnotation.disableConversion() + ? null + : localConvertAnnotation; + } + } + + { + Converts localConvertsAnnotation = property.getAnnotation( Converts.class ); + if ( localConvertsAnnotation != null ) { + for ( Convert localConvertAnnotation : localConvertsAnnotation.value() ) { + if ( isLocalMatch( localConvertAnnotation, property ) ) { + LOG.debugf( + "Found matching @Convert annotation as part local @Converts [disableConversion=%s]", + localConvertAnnotation.disableConversion() + ); + return localConvertAnnotation.disableConversion() + ? null + : localConvertAnnotation; + } + } + } } if ( persistentClassName == null ) { @@ -376,10 +433,20 @@ public class SimpleValueBinder { return null; } + LOG.debugf( + "Attempting to locate any AttributeConverter defined via @Convert/@Converts on type-hierarchy [%s] to apply to attribute [%s]", + owner.getName(), + property.getName() + ); + { Convert convertAnnotation = owner.getAnnotation( Convert.class ); if ( convertAnnotation != null && isMatch( convertAnnotation, property ) ) { - return convertAnnotation; + LOG.debugf( + "Found matching @Convert annotation [disableConversion=%s]", + convertAnnotation.disableConversion() + ); + return convertAnnotation.disableConversion() ? null : convertAnnotation; } } @@ -388,7 +455,11 @@ public class SimpleValueBinder { if ( convertsAnnotation != null ) { for ( Convert convertAnnotation : convertsAnnotation.value() ) { if ( isMatch( convertAnnotation, property ) ) { - return convertAnnotation; + LOG.debugf( + "Found matching @Convert annotation as part @Converts [disableConversion=%s]", + convertAnnotation.disableConversion() + ); + return convertAnnotation.disableConversion() ? null : convertAnnotation; } } } @@ -399,11 +470,22 @@ public class SimpleValueBinder { } @SuppressWarnings("unchecked") - private boolean isMatch(Convert convertAnnotation, XProperty property) { + private boolean isLocalMatch(Convert convertAnnotation, XProperty property) { + if ( StringHelper.isEmpty( convertAnnotation.attributeName() ) ) { + return isTypeMatch( convertAnnotation.converter(), property ); + } + return property.getName().equals( convertAnnotation.attributeName() ) && isTypeMatch( convertAnnotation.converter(), property ); } + @SuppressWarnings("unchecked") + private boolean isMatch(Convert convertAnnotation, XProperty property) { + return property.getName().equals( convertAnnotation.attributeName() ); +// return property.getName().equals( convertAnnotation.attributeName() ) +// && isTypeMatch( convertAnnotation.converter(), property ); + } + private boolean isTypeMatch(Class attributeConverterClass, XProperty property) { return areTypeMatch( extractEntityAttributeType( attributeConverterClass ), @@ -577,7 +659,7 @@ public class SimpleValueBinder { } public void fillSimpleValue() { - LOG.debugf( "Setting SimpleValue typeName for %s", propertyName ); + LOG.debugf( "Starting fillSimpleValue for %s", propertyName ); if ( attributeConverterDefinition != null ) { if ( ! BinderHelper.isEmptyAnnotationValue( explicitType ) ) { @@ -590,6 +672,12 @@ public class SimpleValueBinder { ) ); } + LOG.debugf( + "Applying JPA AttributeConverter [%s] to [%s:%s]", + attributeConverterDefinition, + persistentClassName, + propertyName + ); simpleValue.setJpaAttributeConverterDefinition( attributeConverterDefinition ); } else { diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java b/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java index 1adddba2ea..716cdb0704 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java +++ b/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java @@ -437,8 +437,17 @@ public class SimpleValue implements KeyValue { // todo : cache the AttributeConverterTypeAdapter in case that AttributeConverter is applied multiple times. - final String name = "BasicType adapter for AttributeConverter<" + entityAttributeJavaType + "," + databaseColumnJavaType + ">"; - return new AttributeConverterTypeAdapter( sqlTypeDescriptorAdapter, entityAttributeJavaTypeDescriptor, name ); + final String name = String.format( + "BasicType adapter for AttributeConverter<%s,%s>", + entityAttributeJavaType.getSimpleName(), + databaseColumnJavaType.getSimpleName() + ); + return new AttributeConverterTypeAdapter( + name, + jpaAttributeConverterDefinition.getAttributeConverter(), + sqlTypeDescriptorAdapter, + entityAttributeJavaTypeDescriptor + ); } public boolean isTypeSpecified() { diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/converter/AttributeConverterTypeAdapter.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/converter/AttributeConverterTypeAdapter.java index a322050194..d23d6dfe59 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/converter/AttributeConverterTypeAdapter.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/converter/AttributeConverterTypeAdapter.java @@ -23,6 +23,8 @@ */ package org.hibernate.type.descriptor.converter; +import javax.persistence.AttributeConverter; + import org.jboss.logging.Logger; import org.hibernate.type.AbstractSingleColumnStandardBasicType; @@ -30,19 +32,24 @@ import org.hibernate.type.descriptor.java.JavaTypeDescriptor; import org.hibernate.type.descriptor.sql.SqlTypeDescriptor; /** + * Adapts the Hibernate Type contract to incorporate JPA AttributeConverter calls. + * * @author Steve Ebersole */ -public class AttributeConverterTypeAdapter extends AbstractSingleColumnStandardBasicType { +public class AttributeConverterTypeAdapter extends AbstractSingleColumnStandardBasicType { private static final Logger log = Logger.getLogger( AttributeConverterTypeAdapter.class ); private final String name; + private final AttributeConverter attributeConverter; public AttributeConverterTypeAdapter( - SqlTypeDescriptor sqlTypeDescriptor, - JavaTypeDescriptor javaTypeDescriptor, - String name) { - super( sqlTypeDescriptor, javaTypeDescriptor ); + String name, + AttributeConverter attributeConverter, + SqlTypeDescriptor sqlTypeDescriptorAdapter, + JavaTypeDescriptor entityAttributeJavaTypeDescriptor) { + super( sqlTypeDescriptorAdapter, entityAttributeJavaTypeDescriptor ); this.name = name; + this.attributeConverter = attributeConverter; log.debug( "Created AttributeConverterTypeAdapter -> " + name ); } @@ -51,4 +58,8 @@ public class AttributeConverterTypeAdapter extends AbstractSingleColumnStandardB public String getName() { return name; } + + public AttributeConverter getAttributeConverter() { + return attributeConverter; + } } diff --git a/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/convert/SimpleConvertAnnotationTest.java b/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/convert/SimpleConvertAnnotationTest.java new file mode 100644 index 0000000000..f0397ab931 --- /dev/null +++ b/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/convert/SimpleConvertAnnotationTest.java @@ -0,0 +1,157 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2013, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.jpa.test.convert; + +import javax.persistence.AttributeConverter; +import javax.persistence.Convert; +import javax.persistence.Converter; +import javax.persistence.Entity; +import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; +import javax.persistence.Id; + +import java.net.MalformedURLException; +import java.net.URL; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.engine.spi.SessionFactoryImplementor; +import org.hibernate.jpa.boot.spi.Bootstrap; +import org.hibernate.jpa.test.PersistenceUnitDescriptorAdapter; +import org.hibernate.persister.entity.EntityPersister; +import org.hibernate.type.Type; +import org.hibernate.type.descriptor.converter.AttributeConverterTypeAdapter; + +import org.junit.Test; + +import org.hibernate.testing.junit4.BaseUnitTestCase; + +import static org.hibernate.testing.junit4.ExtraAssertions.assertTyping; +import static org.junit.Assert.assertEquals; + +/** + * @author Steve Ebersole + */ +public class SimpleConvertAnnotationTest extends BaseUnitTestCase { + + // test handling of an AttributeConverter explicitly named via a @Convert annotation ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + @Test + public void testSimpleConvertUsage() throws MalformedURLException { + final PersistenceUnitDescriptorAdapter pu = new PersistenceUnitDescriptorAdapter() { + @Override + public List getManagedClassNames() { + return Arrays.asList( Entity1.class.getName(), UrlConverter.class.getName(), AutoUrlConverter.class.getName() ); + } + }; + + final Map settings = new HashMap(); + settings.put( AvailableSettings.HBM2DDL_AUTO, "create-drop" ); + + EntityManagerFactory emf = Bootstrap.getEntityManagerFactoryBuilder( pu, settings ).build(); + final EntityPersister ep = emf.unwrap( SessionFactoryImplementor.class ).getEntityPersister( Entity1.class.getName() ); + final Type websitePropertyType = ep.getPropertyType( "website" ); + final AttributeConverterTypeAdapter type = assertTyping( AttributeConverterTypeAdapter.class, websitePropertyType ); + assertTyping( UrlConverter.class, type.getAttributeConverter() ); + + try { + EntityManager em = emf.createEntityManager(); + em.getTransaction().begin(); + em.persist( new Entity1( 1, "1", new URL( "http://hibernate.org" ) ) ); + em.getTransaction().commit(); + em.close(); + + assertEquals( 1, callsToConverter ); + + em = emf.createEntityManager(); + em.getTransaction().begin(); + em.createQuery( "delete Entity1" ).executeUpdate(); + em.getTransaction().commit(); + em.close(); + } + finally { + emf.close(); + } + } + + static int callsToConverter = 0; + + @Converter(autoApply = false) + public static class UrlConverter implements AttributeConverter { + @Override + public String convertToDatabaseColumn(URL attribute) { + callsToConverter++; + return attribute == null ? null : attribute.toExternalForm(); + } + + @Override + public URL convertToEntityAttribute(String dbData) { + callsToConverter++; + if ( dbData == null ) { + return null; + } + + try { + return new URL( dbData ); + } + catch (MalformedURLException e) { + throw new IllegalArgumentException( "Could not convert incoming value to URL : " + dbData ); + } + } + } + + @Converter( autoApply = true ) + public static class AutoUrlConverter implements AttributeConverter { + @Override + public String convertToDatabaseColumn(URL attribute) { + throw new IllegalStateException( "Should not be called" ); + } + + @Override + public URL convertToEntityAttribute(String dbData) { + throw new IllegalStateException( "Should not be called" ); + } + } + + @Entity( name = "Entity1" ) + public static class Entity1 { + @Id + private Integer id; + private String name; + @Convert( converter = UrlConverter.class ) + private URL website; + + public Entity1() { + } + + public Entity1(Integer id, String name, URL website) { + this.id = id; + this.name = name; + this.website = website; + } + } +} diff --git a/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/convert/SimpleConvertsAnnotationTest.java b/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/convert/SimpleConvertsAnnotationTest.java new file mode 100644 index 0000000000..4ea035de12 --- /dev/null +++ b/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/convert/SimpleConvertsAnnotationTest.java @@ -0,0 +1,159 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2013, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.jpa.test.convert; + +import javax.persistence.AttributeConverter; +import javax.persistence.Convert; +import javax.persistence.Converter; +import javax.persistence.Converts; +import javax.persistence.Entity; +import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; +import javax.persistence.Id; +import java.net.MalformedURLException; +import java.net.URL; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.engine.spi.SessionFactoryImplementor; +import org.hibernate.jpa.boot.spi.Bootstrap; +import org.hibernate.jpa.test.PersistenceUnitDescriptorAdapter; +import org.hibernate.persister.entity.EntityPersister; +import org.hibernate.type.Type; +import org.hibernate.type.descriptor.converter.AttributeConverterTypeAdapter; + +import org.junit.Test; + +import org.hibernate.testing.junit4.BaseUnitTestCase; + +import static org.hibernate.testing.junit4.ExtraAssertions.assertTyping; +import static org.junit.Assert.assertEquals; + +/** + * @author Steve Ebersole + */ +public class SimpleConvertsAnnotationTest extends BaseUnitTestCase { + + // test handling of an AttributeConverter explicitly named via a @Convert annotation ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + @Test + public void testSimpleConvertUsage() throws MalformedURLException { + final PersistenceUnitDescriptorAdapter pu = new PersistenceUnitDescriptorAdapter() { + @Override + public List getManagedClassNames() { + return Arrays.asList( Entity1.class.getName(), UrlConverter.class.getName(), AutoUrlConverter.class.getName() ); + } + }; + + final Map settings = new HashMap(); + settings.put( AvailableSettings.HBM2DDL_AUTO, "create-drop" ); + + EntityManagerFactory emf = Bootstrap.getEntityManagerFactoryBuilder( pu, settings ).build(); + final EntityPersister ep = emf.unwrap( SessionFactoryImplementor.class ).getEntityPersister( Entity1.class.getName() ); + final Type websitePropertyType = ep.getPropertyType( "website" ); + final AttributeConverterTypeAdapter type = assertTyping( AttributeConverterTypeAdapter.class, websitePropertyType ); + assertTyping( UrlConverter.class, type.getAttributeConverter() ); + + try { + EntityManager em = emf.createEntityManager(); + em.getTransaction().begin(); + em.persist( new Entity1( 1, "1", new URL( "http://hibernate.org" ) ) ); + em.getTransaction().commit(); + em.close(); + + assertEquals( 1, callsToConverter ); + + em = emf.createEntityManager(); + em.getTransaction().begin(); + em.createQuery( "delete Entity1" ).executeUpdate(); + em.getTransaction().commit(); + em.close(); + } + finally { + emf.close(); + } + } + + static int callsToConverter = 0; + + @Converter(autoApply = false) + public static class UrlConverter implements AttributeConverter { + @Override + public String convertToDatabaseColumn(URL attribute) { + callsToConverter++; + return attribute == null ? null : attribute.toExternalForm(); + } + + @Override + public URL convertToEntityAttribute(String dbData) { + callsToConverter++; + if ( dbData == null ) { + return null; + } + + try { + return new URL( dbData ); + } + catch (MalformedURLException e) { + throw new IllegalArgumentException( "Could not convert incoming value to URL : " + dbData ); + } + } + } + + @Converter( autoApply = true ) + public static class AutoUrlConverter implements AttributeConverter { + @Override + public String convertToDatabaseColumn(URL attribute) { + throw new IllegalStateException( "Should not be called" ); + } + + @Override + public URL convertToEntityAttribute(String dbData) { + throw new IllegalStateException( "Should not be called" ); + } + } + + @Entity( name = "Entity1" ) + @Converts( + @Convert( attributeName = "website", converter = UrlConverter.class ) + ) + public static class Entity1 { + @Id + private Integer id; + private String name; + private URL website; + + public Entity1() { + } + + public Entity1(Integer id, String name, URL website) { + this.id = id; + this.name = name; + this.website = website; + } + } +}