From 43dd13073d76e3ad1388f280aeff6f078f8db79f Mon Sep 17 00:00:00 2001 From: Emmanuel Bernard Date: Mon, 4 Jan 2010 18:50:45 +0000 Subject: [PATCH] HHH-4752 support no prefix on @AttributeOverride and @ElementCollection (legacy "element" prefix used if @CollectionOfElements is used) git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@18399 1b8cb986-b30d-0410-93ca-fae66ebed9b2 --- .../hibernate/cfg/AbstractPropertyHolder.java | 68 +++++++++++-------- .../org/hibernate/cfg/AnnotationBinder.java | 5 +- .../cfg/annotations/CollectionBinder.java | 68 +++++++++++++------ .../hibernate/cfg/annotations/MapBinder.java | 17 ++++- .../annotations/embedded/WealthyPerson.java | 2 +- .../override/AttributeOverrideTest.java | 14 ++++ .../annotations/override/PropertyRecord.java | 29 ++++++++ 7 files changed, 150 insertions(+), 53 deletions(-) diff --git a/annotations/src/main/java/org/hibernate/cfg/AbstractPropertyHolder.java b/annotations/src/main/java/org/hibernate/cfg/AbstractPropertyHolder.java index 9902064055..d6a15005c4 100644 --- a/annotations/src/main/java/org/hibernate/cfg/AbstractPropertyHolder.java +++ b/annotations/src/main/java/org/hibernate/cfg/AbstractPropertyHolder.java @@ -45,7 +45,7 @@ import org.hibernate.util.StringHelper; * @author Emmanuel Bernard */ public abstract class AbstractPropertyHolder implements PropertyHolder { - protected PropertyHolder parent; + protected AbstractPropertyHolder parent; private Map holderColumnOverride; private Map currentPropertyColumnOverride; private Map holderJoinColumnOverride; @@ -57,7 +57,7 @@ public abstract class AbstractPropertyHolder implements PropertyHolder { String path, PropertyHolder parent, XClass clazzToProcess, ExtendedMappings mappings ) { this.path = path; - this.parent = parent; + this.parent = (AbstractPropertyHolder) parent; this.mappings = mappings; buildHierarchyColumnOverride( clazzToProcess ); } @@ -98,38 +98,48 @@ public abstract class AbstractPropertyHolder implements PropertyHolder { /** * Get column overriding, property first, then parent, then holder - * replace - * - "index" by "key" if present - * - "element" by "value" if present + * replace the placeholder 'collection&&element' with nothing + * * These rules are here to support both JPA 2 and legacy overriding rules. * - * WARNING: this can conflict with user's expectations if: - * - the property uses some restricted values - * - the user has overridden the column - * But this is unlikely and avoid the need for a "legacy" flag */ public Column[] getOverriddenColumn(String propertyName) { Column[] result = getExactOverriddenColumn( propertyName ); if (result == null) { - if ( propertyName.contains( ".key." ) ) { + //the commented code can be useful if people use the new prefixes on old mappings and vice versa + // if we enable them: + // WARNING: this can conflict with user's expectations if: + // - the property uses some restricted values + // - the user has overridden the column + +// if ( propertyName.contains( ".key." ) ) { +// //support for legacy @AttributeOverride declarations +// //TODO cache the underlying regexp +// result = getExactOverriddenColumn( propertyName.replace( ".key.", ".index." ) ); +// } +// if ( result == null && propertyName.endsWith( ".key" ) ) { +// //support for legacy @AttributeOverride declarations +// //TODO cache the underlying regexp +// result = getExactOverriddenColumn( +// propertyName.substring( 0, propertyName.length() - ".key".length() ) + ".index" +// ); +// } +// if ( result == null && propertyName.contains( ".value." ) ) { +// //support for legacy @AttributeOverride declarations +// //TODO cache the underlying regexp +// result = getExactOverriddenColumn( propertyName.replace( ".value.", ".element." ) ); +// } +// if ( result == null && propertyName.endsWith( ".value" ) ) { +// //support for legacy @AttributeOverride declarations +// //TODO cache the underlying regexp +// result = getExactOverriddenColumn( +// propertyName.substring( 0, propertyName.length() - ".value".length() ) + ".element" +// ); +// } + if ( result == null && propertyName.contains( ".collection&&element." ) ) { + //support for non map collections where no prefix is needed //TODO cache the underlying regexp - result = getExactOverriddenColumn( propertyName.replace( ".key.", ".index." ) ); - } - if ( result == null && propertyName.endsWith( ".key" ) ) { - //TODO cache the underlying regexp - result = getExactOverriddenColumn( - propertyName.substring( 0, propertyName.length() - ".key".length() ) + ".index" - ); - } - if ( result == null && propertyName.contains( ".value." ) ) { - //TODO cache the underlying regexp - result = getExactOverriddenColumn( propertyName.replace( ".value.", ".element." ) ); - } - if ( result == null && propertyName.endsWith( ".value" ) ) { - //TODO cache the underlying regexp - result = getExactOverriddenColumn( - propertyName.substring( 0, propertyName.length() - ".value".length() ) + ".element" - ); + result = getExactOverriddenColumn( propertyName.replace( ".collection&&element.", "." ) ); } } return result; @@ -139,10 +149,10 @@ public abstract class AbstractPropertyHolder implements PropertyHolder { * Get column overriding, property first, then parent, then holder * find the overridden rules from the exact property name. */ - private Column[] getExactOverriddenColumn(String propertyName) { + public Column[] getExactOverriddenColumn(String propertyName) { Column[] override = null; if ( parent != null ) { - override = parent.getOverriddenColumn( propertyName ); + override = parent.getExactOverriddenColumn( propertyName ); } if ( override == null && currentPropertyColumnOverride != null ) { override = currentPropertyColumnOverride.get( propertyName ); diff --git a/annotations/src/main/java/org/hibernate/cfg/AnnotationBinder.java b/annotations/src/main/java/org/hibernate/cfg/AnnotationBinder.java index 1ce0aa1183..a9886a9d75 100644 --- a/annotations/src/main/java/org/hibernate/cfg/AnnotationBinder.java +++ b/annotations/src/main/java/org/hibernate/cfg/AnnotationBinder.java @@ -1589,7 +1589,10 @@ public final class AnnotationBinder { CollectionBinder collectionBinder = CollectionBinder.getCollectionBinder( propertyHolder.getEntityName(), property, - !indexColumn.isImplicit() + !indexColumn.isImplicit(), + property.isAnnotationPresent( CollectionOfElements.class ) + || property.isAnnotationPresent( org.hibernate.annotations.MapKey.class ) + // || property.isAnnotationPresent( ManyToAny.class ) ); collectionBinder.setIndexColumn( indexColumn ); MapKey mapKeyAnn = property.getAnnotation( MapKey.class ); diff --git a/annotations/src/main/java/org/hibernate/cfg/annotations/CollectionBinder.java b/annotations/src/main/java/org/hibernate/cfg/annotations/CollectionBinder.java index 7eef365c9a..610e3af6f2 100644 --- a/annotations/src/main/java/org/hibernate/cfg/annotations/CollectionBinder.java +++ b/annotations/src/main/java/org/hibernate/cfg/annotations/CollectionBinder.java @@ -159,6 +159,19 @@ public abstract class CollectionBinder { private XClass declaringClass; private boolean declaringClassSet; private AccessType accessType; + private boolean hibernateExtensionMapping; + + public boolean isMap() { + return false; + } + + public void setIsHibernateExtensionMapping(boolean hibernateExtensionMapping) { + this.hibernateExtensionMapping = hibernateExtensionMapping; + } + + protected boolean isHibernateExtensionMapping() { + return hibernateExtensionMapping; + } public void setUpdatable(boolean updatable) { this.updatable = updatable; @@ -224,14 +237,15 @@ public abstract class CollectionBinder { */ public static CollectionBinder getCollectionBinder( String entityName, XProperty property, - boolean isIndexed + boolean isIndexed, boolean isHibernateExtensionMapping ) { + CollectionBinder result; if ( property.isArray() ) { if ( property.getElementClass().isPrimitive() ) { - return new PrimitiveArrayBinder(); + result = new PrimitiveArrayBinder(); } else { - return new ArrayBinder(); + result = new ArrayBinder(); } } else if ( property.isCollection() ) { @@ -242,35 +256,35 @@ public abstract class CollectionBinder { throw new AnnotationException( "Set do not support @CollectionId: " + StringHelper.qualify( entityName, property.getName() ) ); } - return new SetBinder(); + result = new SetBinder(); } else if ( java.util.SortedSet.class.equals( returnedClass ) ) { if ( property.isAnnotationPresent( CollectionId.class ) ) { throw new AnnotationException( "Set do not support @CollectionId: " + StringHelper.qualify( entityName, property.getName() ) ); } - return new SetBinder( true ); + result = new SetBinder( true ); } else if ( java.util.Map.class.equals( returnedClass ) ) { if ( property.isAnnotationPresent( CollectionId.class ) ) { throw new AnnotationException( "Map do not support @CollectionId: " + StringHelper.qualify( entityName, property.getName() ) ); } - return new MapBinder(); + result = new MapBinder(); } else if ( java.util.SortedMap.class.equals( returnedClass ) ) { if ( property.isAnnotationPresent( CollectionId.class ) ) { throw new AnnotationException( "Map do not support @CollectionId: " + StringHelper.qualify( entityName, property.getName() ) ); } - return new MapBinder( true ); + result = new MapBinder( true ); } else if ( java.util.Collection.class.equals( returnedClass ) ) { if ( property.isAnnotationPresent( CollectionId.class ) ) { - return new IdBagBinder(); + result = new IdBagBinder(); } else { - return new BagBinder(); + result = new BagBinder(); } } else if ( java.util.List.class.equals( returnedClass ) ) { @@ -280,13 +294,13 @@ public abstract class CollectionBinder { "List do not support @CollectionId and @OrderColumn (or @IndexColumn) at the same time: " + StringHelper.qualify( entityName, property.getName() ) ); } - return new ListBinder(); + result = new ListBinder(); } else if ( property.isAnnotationPresent( CollectionId.class ) ) { - return new IdBagBinder(); + result = new IdBagBinder(); } else { - return new BagBinder(); + result = new BagBinder(); } } else { @@ -302,6 +316,8 @@ public abstract class CollectionBinder { + StringHelper.qualify( entityName, property.getName() ) ); } + result.setIsHibernateExtensionMapping( isHibernateExtensionMapping ); + return result; } protected CollectionBinder() { @@ -1251,10 +1267,7 @@ public abstract class CollectionBinder { else { XClass elementClass; AnnotatedClassType classType; -// Map columnOverrides = PropertyHolderBuilder.buildColumnOverride( -// property, StringHelper.qualify( collValue.getRole(), "element" ) -// ); - //FIXME the "element" is lost + PropertyHolder holder = null; if ( BinderHelper.PRIMITIVE_NAMES.contains( collType.getName() ) ) { classType = AnnotatedClassType.NONE; @@ -1266,7 +1279,7 @@ public abstract class CollectionBinder { holder = PropertyHolderBuilder.buildPropertyHolder( collValue, - collValue.getRole(), // + ".element", + collValue.getRole(), elementClass, property, parentPropertyHolder, mappings ); @@ -1295,8 +1308,25 @@ public abstract class CollectionBinder { throw new AssertionFailure( "Unable to guess collection property accessor name" ); } - //"value" is the JPA 2 prefix for map values (used to be "element") - PropertyData inferredData = new PropertyPreloadedData( AccessType.PROPERTY, "value", elementClass ); + PropertyData inferredData; + if ( isMap() ) { + //"value" is the JPA 2 prefix for map values (used to be "element") + if ( isHibernateExtensionMapping() ) { + inferredData = new PropertyPreloadedData( AccessType.PROPERTY, "element", elementClass ); + } + else { + inferredData = new PropertyPreloadedData( AccessType.PROPERTY, "value", elementClass ); + } + } + else { + if ( isHibernateExtensionMapping() ) { + inferredData = new PropertyPreloadedData( AccessType.PROPERTY, "element", elementClass ); + } + else { + //"collection&&element" is not a valid property name => placeholder + inferredData = new PropertyPreloadedData( AccessType.PROPERTY, "collection&&element", elementClass ); + } + } //TODO be smart with isNullable Component component = AnnotationBinder.fillComponent( holder, inferredData, isPropertyAnnotated ? AccessType.PROPERTY : AccessType.FIELD, true, diff --git a/annotations/src/main/java/org/hibernate/cfg/annotations/MapBinder.java b/annotations/src/main/java/org/hibernate/cfg/annotations/MapBinder.java index 92cd7c7914..884bdf13d0 100644 --- a/annotations/src/main/java/org/hibernate/cfg/annotations/MapBinder.java +++ b/annotations/src/main/java/org/hibernate/cfg/annotations/MapBinder.java @@ -83,6 +83,10 @@ public class MapBinder extends CollectionBinder { super(); } + public boolean isMap() { + return true; + } + protected Collection createCollection(PersistentClass persistentClass) { return new org.hibernate.mapping.Map( persistentClass ); } @@ -222,9 +226,16 @@ public class MapBinder extends CollectionBinder { throw new AssertionFailure( "Unable to guess collection property accessor name" ); } - //boolean propertyAccess = embeddable == null || AccessType.PROPERTY.equals( embeddable.access() ); - //"key" is the JPA 2 prefix for map keys - PropertyData inferredData = new PropertyPreloadedData( AccessType.PROPERTY, "key", elementClass ); + + PropertyData inferredData; + if ( isHibernateExtensionMapping() ) { + inferredData = new PropertyPreloadedData( AccessType.PROPERTY, "index", elementClass ); + } + else { + //"key" is the JPA 2 prefix for map keys + inferredData = new PropertyPreloadedData( AccessType.PROPERTY, "key", elementClass ); + } + //TODO be smart with isNullable Component component = AnnotationBinder.fillComponent( holder, inferredData, isPropertyAnnotated ? AccessType.PROPERTY : AccessType.FIELD, true, diff --git a/annotations/src/test/java/org/hibernate/test/annotations/embedded/WealthyPerson.java b/annotations/src/test/java/org/hibernate/test/annotations/embedded/WealthyPerson.java index 11b379703e..2095f3dbaa 100644 --- a/annotations/src/test/java/org/hibernate/test/annotations/embedded/WealthyPerson.java +++ b/annotations/src/test/java/org/hibernate/test/annotations/embedded/WealthyPerson.java @@ -27,5 +27,5 @@ public class WealthyPerson extends Person { @AttributeOverride(name="country", column=@Column(name="HOME_COUNTRY")) }) - protected Set
vacationHomes = new HashSet(); + protected Set
vacationHomes = new HashSet
(); } diff --git a/annotations/src/test/java/org/hibernate/test/annotations/override/AttributeOverrideTest.java b/annotations/src/test/java/org/hibernate/test/annotations/override/AttributeOverrideTest.java index f67146cf43..d86baf452a 100644 --- a/annotations/src/test/java/org/hibernate/test/annotations/override/AttributeOverrideTest.java +++ b/annotations/src/test/java/org/hibernate/test/annotations/override/AttributeOverrideTest.java @@ -17,6 +17,20 @@ public class AttributeOverrideTest extends TestCase { assertTrue( isColumnPresent( "PropertyRecord_parcels", "ASSESSMENT") ); assertTrue( isColumnPresent( "PropertyRecord_parcels", "SQUARE_FEET") ); assertTrue( isColumnPresent( "PropertyRecord_parcels", "STREET_NAME") ); + + //legacy mappings + assertTrue( isColumnPresent( "LegacyParcels", "ASSESSMENT") ); + assertTrue( isColumnPresent( "LegacyParcels", "SQUARE_FEET") ); + assertTrue( isColumnPresent( "LegacyParcels", "STREET_NAME") ); + } + + public void testElementCollection() throws Exception { + assertTrue( isColumnPresent( "PropertyRecord_unsortedParcels", "ASSESSMENT") ); + assertTrue( isColumnPresent( "PropertyRecord_unsortedParcels", "SQUARE_FEET") ); + + //legacy mappings + assertTrue( isColumnPresent( "PropertyRecord_legacyUnsortedParcels", "ASSESSMENT") ); + assertTrue( isColumnPresent( "PropertyRecord_legacyUnsortedParcels", "SQUARE_FEET") ); } public boolean isColumnPresent(String tableName, String columnName) { diff --git a/annotations/src/test/java/org/hibernate/test/annotations/override/PropertyRecord.java b/annotations/src/test/java/org/hibernate/test/annotations/override/PropertyRecord.java index 56e06b0cd1..5e37028f4e 100644 --- a/annotations/src/test/java/org/hibernate/test/annotations/override/PropertyRecord.java +++ b/annotations/src/test/java/org/hibernate/test/annotations/override/PropertyRecord.java @@ -1,6 +1,7 @@ package org.hibernate.test.annotations.override; import java.util.Map; +import java.util.Set; import javax.persistence.AttributeOverride; import javax.persistence.AttributeOverrides; import javax.persistence.Column; @@ -8,6 +9,10 @@ import javax.persistence.ElementCollection; import javax.persistence.Entity; import javax.persistence.GeneratedValue; import javax.persistence.Id; +import javax.persistence.CollectionTable; + +import org.hibernate.annotations.MapKey; +import org.hibernate.annotations.CollectionOfElements; /** * @author Emmanuel Bernard @@ -25,4 +30,28 @@ public class PropertyRecord { }) @ElementCollection public Map parcels; + + @AttributeOverrides({ + @AttributeOverride(name = "index.street", column = @Column(name = "STREET_NAME")), + @AttributeOverride(name = "element.size", column = @Column(name = "SQUARE_FEET")), + @AttributeOverride(name = "element.tax", column = @Column(name = "ASSESSMENT")) + }) + @CollectionOfElements + //@MapKey + @CollectionTable(name="LegacyParcels") + public Map legacyParcels; + + @AttributeOverrides({ + @AttributeOverride(name = "size", column = @Column(name = "SQUARE_FEET")), + @AttributeOverride(name = "tax", column = @Column(name = "ASSESSMENT")) + }) + @ElementCollection + public Set unsortedParcels; + + @AttributeOverrides({ + @AttributeOverride(name = "element.size", column = @Column(name = "SQUARE_FEET")), + @AttributeOverride(name = "element.tax", column = @Column(name = "ASSESSMENT")) + }) + @CollectionOfElements + public Set legacyUnsortedParcels; } \ No newline at end of file