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

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<String, Column[]> holderColumnOverride;
 	private Map<String, Column[]> currentPropertyColumnOverride;
 	private Map<String, JoinColumn[]> 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 );

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 );

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<String, javax.persistence.Column[]> 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,

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,

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<Address> vacationHomes = new HashSet();
+	protected Set<Address> vacationHomes = new HashSet<Address>();
 }

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) {

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<Address, PropertyInfo> 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<Address, PropertyInfo> legacyParcels;
+
+	@AttributeOverrides({
+			@AttributeOverride(name = "size", column = @Column(name = "SQUARE_FEET")),
+			@AttributeOverride(name = "tax", column = @Column(name = "ASSESSMENT"))
+					})
+	@ElementCollection
+	public Set<PropertyInfo> unsortedParcels;
+
+	@AttributeOverrides({
+			@AttributeOverride(name = "element.size", column = @Column(name = "SQUARE_FEET")),
+			@AttributeOverride(name = "element.tax", column = @Column(name = "ASSESSMENT"))
+					})
+	@CollectionOfElements
+	public Set<PropertyInfo> legacyUnsortedParcels;
 }