HHH-7888 - Bind @OrderBy with empty value

hibernate-core/src/main/java/org/hibernate/cfg/annotations/CollectionBinder.java

@@ -1065,7 +1065,7 @@ protected void bindManyToManySecondPass(
 			TableBinder associationTableBinder,
 			XProperty property,
 			PropertyHolder parentPropertyHolder,
-			String hqlOrderBy,
+			final String hqlOrderBy,
 			Mappings mappings) throws MappingException {
 
 		PersistentClass collectionEntity = (PersistentClass) persistentClasses.get( collType.getName() );
@@ -1286,7 +1286,6 @@ else if ( owner.getIdentifierMapper() != null && owner.getIdentifierMapper().get
 				collValue.setElement( component );
 
 				if ( StringHelper.isNotEmpty( hqlOrderBy ) ) {
-					String path = collValue.getOwnerEntityName() + "." + joinColumns[0].getPropertyName();
 					String orderBy = adjustUserSuppliedValueCollectionOrderingFragment( hqlOrderBy );
 					if ( orderBy != null ) {
 						collValue.setOrderBy( orderBy );

hibernate-core/src/main/java/org/hibernate/internal/util/collections/CollectionHelper.java

@@ -156,4 +156,20 @@ public static boolean isNotEmpty(Object[] objs){
 	public static <T> List<T> createEmptyList(Class<T> elementClass) {
 		return Collections.emptyList();
 	}
+
+	public static boolean isCollectionOrArray(Class clazz) {
+		if ( clazz == null ) {
+			return false;
+		}
+		if ( Collection.class.isAssignableFrom( clazz ) ) {
+			return true;
+		}
+		if ( Map.class.isAssignableFrom( clazz ) ) {
+			return true;
+		}
+//		if ( clazz.isArray() ) {
+//			return true;
+//		}
+		return false;
+	}
 }

hibernate-core/src/main/java/org/hibernate/metamodel/internal/Binder.java

@@ -1980,23 +1980,7 @@ private void bindSortingAndOrdering(
 		if ( Orderable.class.isInstance( attributeSource ) ) {
 			final Orderable orderable = (Orderable) attributeSource;
 			if ( orderable.isOrdered() ) {
-				String orderBy = orderable.getOrder();
+				attributeBinding.setOrderBy( orderable.getOrder() );
-				if ( orderBy.equals( "" ) ) {
-					PrimaryKey pk = attributeBinding.getPluralAttributeKeyBinding()
-							.getReferencedAttributeBinding()
-							.getContainer()
-							.seekEntityBinding()
-							.getPrimaryTable()
-							.getPrimaryKey();
-					List<Column> pkColumns = pk.getColumns();
-					StringBuffer sb = new StringBuffer();
-					for ( final Column column : pkColumns ) {
-						sb.append( column.getColumnName().getText() );
-						sb.append( " asc ," );
-					}
-					orderBy = sb.substring( 0, sb.length() - 2 );
-				}
-				attributeBinding.setOrderBy( orderBy );
 
 			}
 		}
@@ -2250,47 +2234,52 @@ private void bindCollectionTableForeignKey(
 		}
 	}
 
+	/**
+	 * TODO : It is really confusing that we have so many different <tt>natures</tt>
+	 */
 	private void bindCollectionTablePrimaryKey(
 			final AbstractPluralAttributeBinding attributeBinding,
 			final PluralAttributeSource attributeSource) {
-		PluralAttributeSource.Nature pluralAttributeNature = attributeSource.getNature();
+		final PluralAttributeSource.Nature pluralAttributeSourceNature = attributeSource.getNature();
-		if ( attributeSource.getElementSource().getNature() == PluralAttributeElementSource.Nature.ONE_TO_MANY
+		final PluralAttributeElementSource.Nature pluralElementSourceNature = attributeSource.getElementSource().getNature();
-				|| pluralAttributeNature == PluralAttributeSource.Nature.BAG ) {
+		final PluralAttributeElementBinding.Nature pluralElementBindingNature = attributeBinding.getPluralAttributeElementBinding().getNature();
+
+		//TODO what is this case? it would be really good to add a comment
+		if ( pluralElementSourceNature == PluralAttributeElementSource.Nature.ONE_TO_MANY
+				|| pluralAttributeSourceNature == PluralAttributeSource.Nature.BAG ) {
 			return;
 		}
-		if ( attributeBinding.getPluralAttributeElementBinding()
+		if ( pluralElementBindingNature == PluralAttributeElementBinding.Nature.BASIC ) {
-				.getNature() == PluralAttributeElementBinding.Nature.BASIC ) {
+			switch ( pluralAttributeSourceNature ) {
-			if ( pluralAttributeNature == PluralAttributeSource.Nature.SET ) {
+				case SET:
-				bindBasicSetCollectionTablePrimaryKey( (SetBinding) attributeBinding );
+					bindBasicSetCollectionTablePrimaryKey( (SetBinding) attributeBinding );
-			}
+					break;
-			else if (
+				case LIST:
-					pluralAttributeNature == PluralAttributeSource.Nature.LIST
+				case MAP:
-							|| pluralAttributeNature == PluralAttributeSource.Nature.MAP
+				case ARRAY:
-							|| pluralAttributeNature == PluralAttributeSource.Nature.ARRAY ) {
+					bindIndexedCollectionTablePrimaryKey( (IndexedPluralAttributeBinding) attributeBinding );
-				bindIndexedCollectionTablePrimaryKey( (IndexedPluralAttributeBinding) attributeBinding );
+					break;
-			}
+				default:
-			else {
+					throw new NotYetImplementedException(
-				throw new NotYetImplementedException(
+							String.format( "%s of basic elements is not supported yet.", pluralAttributeSourceNature )
-						String.format( "%s of basic elements is not supported yet.", pluralAttributeNature )
+					);
-				);
 			}
 		}
-		else if ( attributeBinding.getPluralAttributeElementBinding()
+		else if ( pluralElementBindingNature == PluralAttributeElementBinding.Nature.MANY_TO_MANY ) {
-				.getNature() == PluralAttributeElementBinding.Nature.MANY_TO_MANY ) {
 			if ( !attributeBinding.getPluralAttributeKeyBinding().isInverse() ) {
-				if ( pluralAttributeNature == PluralAttributeSource.Nature.SET ) {
+				switch ( pluralAttributeSourceNature ) {
-					bindSetCollectionTablePrimaryKey( (SetBinding) attributeBinding );
+					case SET:
-				}
+						bindSetCollectionTablePrimaryKey( (SetBinding) attributeBinding );
-				else if (
+						break;
-						pluralAttributeNature == PluralAttributeSource.Nature.LIST
+					case LIST:
-								|| pluralAttributeNature == PluralAttributeSource.Nature.MAP
+					case MAP:
-								|| pluralAttributeNature == PluralAttributeSource.Nature.ARRAY ) {
+					case ARRAY:
-					bindIndexedCollectionTablePrimaryKey( (IndexedPluralAttributeBinding) attributeBinding );
+						bindIndexedCollectionTablePrimaryKey( (IndexedPluralAttributeBinding) attributeBinding );
-				}
+						break;
-				else {
+					default:
-					throw new NotYetImplementedException(
+						throw new NotYetImplementedException(
-							String.format( "Many-to-many %s is not supported yet.", pluralAttributeNature )
+								String.format( "Many-to-many %s is not supported yet.", pluralAttributeSourceNature )
-					);
+						);
 				}
 			}
 		}

hibernate-core/src/main/java/org/hibernate/metamodel/internal/source/annotations/ComponentAttributeSourceImpl.java

@@ -144,8 +144,8 @@ public List<AttributeSource> initialize() {
 					}
 					for ( AssociationAttribute associationAttribute : embeddableClass.getAssociationAttributes() ) {
 						associationAttribute.setNaturalIdMutability( embeddableClass.getNaturalIdMutability() );
-						attributeList.add( new ToOneAttributeSourceImpl( associationAttribute ) );
 					}
+					SourceHelper.resolveAssociationAttributes( embeddableClass, attributeList );
 					return Collections.unmodifiableList( attributeList );
 				}
 			}

hibernate-core/src/main/java/org/hibernate/metamodel/internal/source/annotations/CompositePluralAttributeElementSourceImpl.java

@@ -31,8 +31,8 @@
 import org.hibernate.metamodel.internal.source.annotations.attribute.AssociationAttribute;
 import org.hibernate.metamodel.internal.source.annotations.attribute.AttributeOverride;
 import org.hibernate.metamodel.internal.source.annotations.attribute.BasicAttribute;
+import org.hibernate.metamodel.internal.source.annotations.entity.ConfiguredClass;
 import org.hibernate.metamodel.internal.source.annotations.entity.EmbeddableClass;
-import org.hibernate.metamodel.internal.source.annotations.entity.EntityClass;
 import org.hibernate.metamodel.spi.binding.CascadeType;
 import org.hibernate.metamodel.spi.source.AttributeSource;
 import org.hibernate.metamodel.spi.source.CompositePluralAttributeElementSource;
@@ -47,7 +47,7 @@ public class CompositePluralAttributeElementSourceImpl implements CompositePlura
 	private static final String PATH_SEPARATOR = ".";
 	
 	private final AssociationAttribute associationAttribute;
-	private final EntityClass entityClass;
+	private final ConfiguredClass entityClass;
 	
 	private List<AttributeSource> attributeSources
 			= new ArrayList<AttributeSource>();
@@ -56,7 +56,7 @@ public class CompositePluralAttributeElementSourceImpl implements CompositePlura
 	
 	public CompositePluralAttributeElementSourceImpl(
 			AssociationAttribute associationAttribute,
-			EntityClass rootEntityClass ) {
+			ConfiguredClass rootEntityClass ) {
 		this.associationAttribute = associationAttribute;
 		this.entityClass = rootEntityClass;
 		
@@ -152,8 +152,8 @@ private void buildAttributeSources() {
 		}
 		for ( AssociationAttribute associationAttribute : embeddableClass.getAssociationAttributes() ) {
 			associationAttribute.setNaturalIdMutability( embeddableClass.getNaturalIdMutability() );
-			attributeSources.add( new ToOneAttributeSourceImpl( associationAttribute ) );
 		}
+		SourceHelper.resolveAssociationAttributes( embeddableClass, attributeSources );
 	}
 
 	private Map<String, AttributeOverride> createAggregatedOverrideMap(

hibernate-core/src/main/java/org/hibernate/metamodel/internal/source/annotations/EntitySourceImpl.java

@@ -32,16 +32,12 @@
 
 import org.hibernate.AnnotationException;
 import org.hibernate.MappingException;
-import org.hibernate.cfg.NotYetImplementedException;
 import org.hibernate.internal.util.StringHelper;
 import org.hibernate.jaxb.spi.Origin;
-import org.hibernate.metamodel.internal.source.annotations.attribute.AssociationAttribute;
 import org.hibernate.metamodel.internal.source.annotations.attribute.AttributeOverride;
 import org.hibernate.metamodel.internal.source.annotations.attribute.BasicAttribute;
-import org.hibernate.metamodel.internal.source.annotations.attribute.PluralAssociationAttribute;
 import org.hibernate.metamodel.internal.source.annotations.entity.EmbeddableClass;
 import org.hibernate.metamodel.internal.source.annotations.entity.EntityClass;
-import org.hibernate.metamodel.internal.source.annotations.entity.RootEntityClass;
 import org.hibernate.metamodel.internal.source.annotations.util.HibernateDotNames;
 import org.hibernate.metamodel.internal.source.annotations.util.JPADotNames;
 import org.hibernate.metamodel.internal.source.annotations.util.JandexHelper;
@@ -53,7 +49,6 @@
 import org.hibernate.metamodel.spi.source.JpaCallbackSource;
 import org.hibernate.metamodel.spi.source.LocalBindingContext;
 import org.hibernate.metamodel.spi.source.MetaAttributeSource;
-import org.hibernate.metamodel.spi.source.PluralAttributeSource;
 import org.hibernate.metamodel.spi.source.PrimaryKeyJoinColumnSource;
 import org.hibernate.metamodel.spi.source.SecondaryTableSource;
 import org.hibernate.metamodel.spi.source.SubclassEntitySource;
@@ -229,32 +224,7 @@ public List<AttributeSource> attributeSources() {
 					)
 			);
 		}
-
+		SourceHelper.resolveAssociationAttributes( entityClass, attributeList );
-		for ( AssociationAttribute associationAttribute : entityClass.getAssociationAttributes() ) {
-			switch ( associationAttribute.getNature() ) {
-				case ONE_TO_ONE:
-				case MANY_TO_ONE: {
-					attributeList.add( new ToOneAttributeSourceImpl( associationAttribute ) );
-					break;
-				}
-				case MANY_TO_MANY:
-				case ONE_TO_MANY:
-				case ELEMENT_COLLECTION_BASIC:
-				case ELEMENT_COLLECTION_EMBEDDABLE: {
-					PluralAssociationAttribute pluralAssociationAttribute = (PluralAssociationAttribute) associationAttribute;
-					PluralAttributeSource.Nature pluralAttributeNature = pluralAssociationAttribute.getPluralAttributeNature();
-
-					AttributeSource source = pluralAssociationAttribute.isIndexed() ?
-							new IndexedPluralAttributeSourceImpl( pluralAssociationAttribute, entityClass )
-							: new PluralAttributeSourceImpl( pluralAssociationAttribute, entityClass );
-					attributeList.add( source );
-					break;
-				}
-				default: {
-					throw new NotYetImplementedException();
-				}
-			}
-		}
 		return attributeList;
 	}
 

hibernate-core/src/main/java/org/hibernate/metamodel/internal/source/annotations/IndexedPluralAttributeSourceImpl.java

@@ -4,6 +4,7 @@
 
 import org.hibernate.metamodel.internal.source.annotations.attribute.MappedAttribute;
 import org.hibernate.metamodel.internal.source.annotations.attribute.PluralAssociationAttribute;
+import org.hibernate.metamodel.internal.source.annotations.entity.ConfiguredClass;
 import org.hibernate.metamodel.internal.source.annotations.entity.EntityClass;
 import org.hibernate.metamodel.spi.source.IndexedPluralAttributeSource;
 import org.hibernate.metamodel.spi.source.MappingException;
@@ -15,16 +16,16 @@
 public class IndexedPluralAttributeSourceImpl extends PluralAttributeSourceImpl
 		implements IndexedPluralAttributeSource {
 	private final PluralAttributeIndexSource indexSource;
-	private final static EnumSet<MappedAttribute.Nature> validNatures = EnumSet.of(
+	private final static EnumSet<MappedAttribute.Nature> VALID_NATURES = EnumSet.of(
 			MappedAttribute.Nature.MANY_TO_MANY,
 			MappedAttribute.Nature.ONE_TO_MANY,
 			MappedAttribute.Nature.ELEMENT_COLLECTION_BASIC,
 			MappedAttribute.Nature.ELEMENT_COLLECTION_EMBEDDABLE);
 
 	public IndexedPluralAttributeSourceImpl(PluralAssociationAttribute attribute,
-			EntityClass entityClass ) {
+			ConfiguredClass entityClass ) {
 		super( attribute, entityClass );
-		if ( !validNatures.contains( attribute.getNature() ) ) {
+		if ( !VALID_NATURES.contains( attribute.getNature() ) ) {
 			throw new MappingException(
 					"Indexed column could be only mapped on the MANY side",
 					attribute.getContext().getOrigin()

hibernate-core/src/main/java/org/hibernate/metamodel/internal/source/annotations/PluralAttributeSourceImpl.java

@@ -24,9 +24,6 @@
 package org.hibernate.metamodel.internal.source.annotations;
 
 import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
 
 import org.jboss.jandex.AnnotationInstance;
 
@@ -35,8 +32,8 @@
 import org.hibernate.internal.util.StringHelper;
 import org.hibernate.internal.util.ValueHolder;
 import org.hibernate.metamodel.internal.source.annotations.attribute.PluralAssociationAttribute;
+import org.hibernate.metamodel.internal.source.annotations.entity.ConfiguredClass;
 import org.hibernate.metamodel.internal.source.annotations.entity.EntityClass;
-import org.hibernate.metamodel.internal.source.annotations.entity.RootEntityClass;
 import org.hibernate.metamodel.spi.binding.Caching;
 import org.hibernate.metamodel.spi.binding.CustomSQL;
 import org.hibernate.metamodel.spi.source.ExplicitHibernateTypeSource;
@@ -61,7 +58,7 @@ public class PluralAttributeSourceImpl implements PluralAttributeSource, Orderab
 
 	public PluralAttributeSourceImpl(
 			final PluralAssociationAttribute associationAttribute,
-			final EntityClass entityClass ) {
+			final ConfiguredClass entityClass ) {
 		this.associationAttribute = associationAttribute;
 		this.keySource = new PluralAttributeKeySourceImpl( associationAttribute );
 		this.typeSource = new ExplicitHibernateTypeSourceImpl( associationAttribute );
@@ -91,7 +88,7 @@ public ValueHolder<Class<?>> getElementClassReference() {
 		}
 	}
 
-	private static PluralAttributeElementSource determineElementSource(PluralAssociationAttribute associationAttribute, EntityClass entityClass) {
+	private static PluralAttributeElementSource determineElementSource(PluralAssociationAttribute associationAttribute, ConfiguredClass entityClass) {
 		switch ( associationAttribute.getNature() ) {
 			case MANY_TO_MANY:
 				return new ManyToManyPluralAttributeElementSourceImpl( associationAttribute );
@@ -222,7 +219,7 @@ public String getOrder() {
 
 	@Override
 	public boolean isOrdered() {
-		return getOrder() != null;
+		return StringHelper.isNotEmpty( getOrder() );
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/metamodel/internal/source/annotations/SourceHelper.java

@@ -0,0 +1,69 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2011, 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.metamodel.internal.source.annotations;
+
+import java.util.List;
+
+import org.hibernate.cfg.NotYetImplementedException;
+import org.hibernate.metamodel.internal.source.annotations.attribute.AssociationAttribute;
+import org.hibernate.metamodel.internal.source.annotations.attribute.PluralAssociationAttribute;
+import org.hibernate.metamodel.internal.source.annotations.entity.ConfiguredClass;
+import org.hibernate.metamodel.spi.source.AttributeSource;
+
+/**
+ * @author Strong Liu <stliu@hibernate.org>
+ */
+public class SourceHelper {
+	/**
+	 * Bind association attributes within {@param configuredClass} to the proper source impl based on its nature.
+	 *
+	 * @param configuredClass The holder of association attributes.
+	 * @param attributeList Attribute source container, can't be <code>null</code>.
+	 */
+	public static void resolveAssociationAttributes(ConfiguredClass configuredClass, List<AttributeSource> attributeList) {
+		for ( AssociationAttribute associationAttribute : configuredClass.getAssociationAttributes() ) {
+			switch ( associationAttribute.getNature() ) {
+				case ONE_TO_ONE:
+				case MANY_TO_ONE: {
+					attributeList.add( new ToOneAttributeSourceImpl( associationAttribute ) );
+					break;
+				}
+				case MANY_TO_MANY:
+				case ONE_TO_MANY:
+				case ELEMENT_COLLECTION_BASIC:
+				case ELEMENT_COLLECTION_EMBEDDABLE: {
+					PluralAssociationAttribute pluralAssociationAttribute = (PluralAssociationAttribute) associationAttribute;
+					AttributeSource source = pluralAssociationAttribute.isIndexed() ?
+							new IndexedPluralAttributeSourceImpl( pluralAssociationAttribute, configuredClass )
+							: new PluralAttributeSourceImpl( pluralAssociationAttribute, configuredClass );
+					attributeList.add( source );
+					break;
+				}
+				default: {
+					throw new NotYetImplementedException();
+				}
+			}
+		}
+	}
+}

hibernate-core/src/main/java/org/hibernate/metamodel/internal/source/annotations/ToOneAttributeSourceImpl.java

@@ -57,6 +57,7 @@ public class ToOneAttributeSourceImpl extends SingularAttributeSourceImpl implem
 	private final Set<CascadeStyle> cascadeStyles;
 
 	public ToOneAttributeSourceImpl(AssociationAttribute associationAttribute) {
+
 		super( associationAttribute );
 		this.associationAttribute = associationAttribute;
 		this.cascadeStyles = EnumConversionHelper.cascadeTypeToCascadeStyleSet(
@@ -75,9 +76,8 @@ else if ( MappedAttribute.Nature.ONE_TO_ONE.equals( associationAttribute.getNatu
 				throw new NotYetImplementedException( "One-to-one using annotations is not supported yet." );
 		}
 		else {
-			throw new AssertionError(
+			throw new AssertionError(String.format( "Wrong attribute nature[%s] for toOne attribute: %s",
-					"Wrong attribute nature for toOne attribute: " + associationAttribute.getNature()
+					associationAttribute.getNature(), associationAttribute.getRole() ));
-			);
 		}
 	}
 

hibernate-core/src/main/java/org/hibernate/metamodel/internal/source/annotations/attribute/AssociationAttribute.java

@@ -49,12 +49,16 @@
 import org.hibernate.metamodel.internal.source.annotations.util.HibernateDotNames;
 import org.hibernate.metamodel.internal.source.annotations.util.JPADotNames;
 import org.hibernate.metamodel.internal.source.annotations.util.JandexHelper;
+import org.hibernate.metamodel.internal.source.annotations.xml.mocker.MockHelper;
 import org.hibernate.metamodel.spi.source.MappingException;
 import org.jboss.jandex.AnnotationInstance;
+import org.jboss.jandex.AnnotationTarget;
 import org.jboss.jandex.AnnotationValue;
 import org.jboss.jandex.ClassInfo;
 import org.jboss.jandex.DotName;
+import org.jboss.jandex.FieldInfo;
 import org.jboss.jandex.Index;
+import org.jboss.jandex.Type;
 import org.jboss.logging.Logger;
 
 /**
@@ -87,6 +91,7 @@ public class AssociationAttribute extends MappedAttribute {
 	private AttributeTypeResolver resolver;
 
 	public static AssociationAttribute createAssociationAttribute(
+			ClassInfo classInfo,
 			String name,
 			Class<?> attributeType,
 			Nature attributeNature,
@@ -94,6 +99,7 @@ public static AssociationAttribute createAssociationAttribute(
 			Map<DotName, List<AnnotationInstance>> annotations,
 			EntityBindingContext context) {
 		return new AssociationAttribute(
+				classInfo,
 				name,
 				attributeType,
 				attributeType,
@@ -105,6 +111,7 @@ public static AssociationAttribute createAssociationAttribute(
 	}
 
 	AssociationAttribute(
+			ClassInfo classInfo,
 			String name,
 			Class<?> attributeType,
 			Class<?> referencedAttributeType,
@@ -119,6 +126,23 @@ public static AssociationAttribute createAssociationAttribute(
 				annotations,
 				attributeNature.getAnnotationDotName()
 		);
+		if ( associationAnnotation == null &&
+				( attributeNature == Nature.ELEMENT_COLLECTION_BASIC || attributeNature == Nature.ELEMENT_COLLECTION_EMBEDDABLE ) ) {
+
+			AnnotationTarget target = MockHelper.getTarget(
+					context.getServiceRegistry(),
+					classInfo,
+					name,
+					MockHelper.TargetType.valueOf( accessType.toUpperCase() )
+			);
+
+
+			associationAnnotation = AnnotationInstance.create(
+					attributeNature.getAnnotationDotName(),
+					target,
+					MockHelper.EMPTY_ANNOTATION_VALUE_ARRAY
+			);
+		}
 
 		// using jandex we don't really care which exact type of annotation we are dealing with
 		this.referencedEntityType = determineReferencedEntityType( associationAnnotation, referencedAttributeType );

hibernate-core/src/main/java/org/hibernate/metamodel/internal/source/annotations/attribute/PluralAssociationAttribute.java

@@ -23,9 +23,13 @@
  */
 package org.hibernate.metamodel.internal.source.annotations.attribute;
 
+import java.util.Collection;
+import java.util.EnumSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.SortedMap;
+import java.util.SortedSet;
 
 import javax.persistence.FetchType;
 
@@ -77,17 +81,22 @@ public class PluralAssociationAttribute extends AssociationAttribute {
 	private final String explicitForeignKeyName;
 
 	private final PluralAttributeSource.Nature pluralAttributeNature;
+	private static final EnumSet<PluralAttributeSource.Nature> SHOULD_NOT_HAS_COLLECTION_ID = EnumSet.of( PluralAttributeSource.Nature.SET,
+			PluralAttributeSource.Nature.MAP, PluralAttributeSource.Nature.LIST, PluralAttributeSource.Nature.ARRAY );
 
 	private LazyCollectionOption lazyOption;
+	private final boolean isCollectionIdPresent;
 
-	public static PluralAssociationAttribute createPluralAssociationAttribute(ClassInfo entityClassInfo,
+
-																			  String name,
+	public static PluralAssociationAttribute createPluralAssociationAttribute(
-																			  Class<?> attributeType,
+			ClassInfo entityClassInfo,
-																			  Class<?> referencedAttributeType,
+			String name,
-																			  Nature attributeNature,
+			Class<?> attributeType,
-																			  String accessType,
+			Class<?> referencedAttributeType,
-																			  Map<DotName, List<AnnotationInstance>> annotations,
+			Nature attributeNature,
-																			  EntityBindingContext context) {
+			String accessType,
+			Map<DotName, List<AnnotationInstance>> annotations,
+			EntityBindingContext context) {
 		return new PluralAssociationAttribute(
 				entityClassInfo,
 				name,
@@ -171,15 +180,16 @@ public boolean isIndexed() {
 		return isIndexed;
 	}
 
-	private PluralAssociationAttribute(ClassInfo entityClassInfo,
+	private PluralAssociationAttribute(
-									   String name,
+			final ClassInfo entityClassInfo,
-									   Class<?> attributeType,
+			final String name,
-									   Class<?> referencedAttributeType,
+			final Class<?> attributeType,
-									   Nature associationType,
+			final Class<?> referencedAttributeType,
-									   String accessType,
+			final Nature associationType,
-									   Map<DotName, List<AnnotationInstance>> annotations,
+			final String accessType,
-									   EntityBindingContext context) {
+			final Map<DotName, List<AnnotationInstance>> annotations,
-		super( name, attributeType, referencedAttributeType, associationType, accessType, annotations, context );
+			final EntityBindingContext context) {
+		super( entityClassInfo, name, attributeType, referencedAttributeType, associationType, accessType, annotations, context );
 		this.entityClassInfo = entityClassInfo;
 		this.whereClause = determineWereClause();
 		this.orderBy = determineOrderBy();
@@ -215,7 +225,10 @@ HibernateDotNames.SQL_DELETE, annotations()
 				HibernateDotNames.SQL_DELETE_ALL, annotations()
 		);
 		this.onDeleteAction = determineOnDeleteAction();
-
+		this.isCollectionIdPresent = JandexHelper.getSingleAnnotation(
+				annotations,
+				HibernateDotNames.COLLECTION_ID
+		) != null;
 		final AnnotationInstance sortAnnotation =  JandexHelper.getSingleAnnotation( annotations, HibernateDotNames.SORT );
 		if ( sortAnnotation == null ) {
 			this.sorted = false;
@@ -249,15 +262,58 @@ HibernateDotNames.SQL_DELETE_ALL, annotations()
 		}
 		this.isIndexed = orderColumnAnnotation != null || indexColumnAnnotation != null;
 		this.pluralAttributeNature = resolvePluralAttributeNature();
+
+		validateMapping();
 	}
 
+	private void validateMapping() {
+		checkSortedTypeIsSortable();
+		checkIfCollectionIdIsWronglyPlaced();
+	}
+
+	private void checkIfCollectionIdIsWronglyPlaced() {
+		if ( isCollectionIdPresent && SHOULD_NOT_HAS_COLLECTION_ID.contains( pluralAttributeNature ) ) {
+			throw new MappingException(
+					"The Collection type doesn't support @CollectionId annotation: " + getRole(),
+					getContext().getOrigin()
+			);
+		}
+	}
+
+	private void checkSortedTypeIsSortable() {
+		//shortcut, a little performance improvement of avoiding the class type check
+		if ( pluralAttributeNature == PluralAttributeSource.Nature.MAP
+				|| pluralAttributeNature == PluralAttributeSource.Nature.SET ) {
+			if ( SortedMap.class.isAssignableFrom( getAttributeType() )
+					|| SortedSet.class.isAssignableFrom( getAttributeType() ) ) {
+				if ( !isSorted() ) {
+					throw new MappingException(
+							"A sorted collection has to define @Sort: " + getRole(),
+							getContext().getOrigin()
+					);
+				}
+			}
+		}
+
+	}
+
+
+	//TODO org.hibernate.cfg.annotations.CollectionBinder#hasToBeSorted
 	private PluralAttributeSource.Nature resolvePluralAttributeNature() {
 
 		if ( Map.class.isAssignableFrom( getAttributeType() ) ) {
 			return PluralAttributeSource.Nature.MAP;
 		}
 		else if ( List.class.isAssignableFrom( getAttributeType() ) ) {
-			return isIndexed() ? PluralAttributeSource.Nature.LIST : PluralAttributeSource.Nature.BAG;
+			if ( isIndexed() ) {
+				return PluralAttributeSource.Nature.LIST;
+			}
+			else if ( isCollectionIdPresent ) {
+				return PluralAttributeSource.Nature.ID_BAG;
+			}
+			else {
+				return PluralAttributeSource.Nature.BAG;
+			}
 		}
 		else if ( Set.class.isAssignableFrom( getAttributeType() ) ) {
 			return PluralAttributeSource.Nature.SET;
@@ -265,22 +321,24 @@ else if ( Set.class.isAssignableFrom( getAttributeType() ) ) {
 		else if ( getAttributeType().isArray() ) {
 			return PluralAttributeSource.Nature.ARRAY;
 		}
+		else if ( Collection.class.isAssignableFrom( getAttributeType() ) ) {
+			return isCollectionIdPresent ? PluralAttributeSource.Nature.ID_BAG : PluralAttributeSource.Nature.BAG;
+		}
 		else {
 			return PluralAttributeSource.Nature.BAG;
 		}
 	}
 
-	private OnDeleteAction determineOnDeleteAction() {
 
-		OnDeleteAction action = null;
+	private OnDeleteAction determineOnDeleteAction() {
 		final AnnotationInstance onDeleteAnnotation = JandexHelper.getSingleAnnotation(
 				annotations(),
 				HibernateDotNames.ON_DELETE
 		);
 		if ( onDeleteAnnotation != null ) {
-			action = JandexHelper.getEnumValue( onDeleteAnnotation, "action", OnDeleteAction.class );
+			return JandexHelper.getEnumValue( onDeleteAnnotation, "action", OnDeleteAction.class );
 		}
-		return action;
+		return null;
 	}
 
 	@Override
@@ -369,9 +427,10 @@ private String determineOrderBy() {
 		);
 
 		if ( jpaWhereAnnotation != null && hibernateWhereAnnotation != null ) {
-			throw new AnnotationException(
+			throw new MappingException(
 					"Cannot use sql order by clause (@org.hibernate.annotations.OrderBy) " +
-							"in conjunction with JPA order by clause (@java.persistence.OrderBy) on  " + getRole()
+							"in conjunction with JPA order by clause (@java.persistence.OrderBy) on  " + getRole(),
+					getContext().getOrigin()
 			);
 		}
 
@@ -385,12 +444,21 @@ private String determineOrderBy() {
 			// associated entity is assumed
 			// The binder will need to take this into account and generate the right property names
 			orderBy = JandexHelper.getValue( jpaWhereAnnotation, "value", String.class );
-			orderBy = orderBy == null ?  "" : orderBy;
+			if ( orderBy == null ) {
+				orderBy = isBasicCollection() ?  "$element$ asc" :"id asc" ;
+			}
+			if ( orderBy.equalsIgnoreCase( "desc" ) ) {
+				orderBy = isBasicCollection() ? "$element$ desc" :"id desc";
+			}
 		}
 
 		return orderBy;
 	}
 
+	private boolean isBasicCollection(){
+		return getNature() == Nature.ELEMENT_COLLECTION_BASIC || getNature() == Nature.ELEMENT_COLLECTION_EMBEDDABLE;
+	}
+
 	private Caching determineCachingSettings() {
 		Caching caching = null;
 		final AnnotationInstance hibernateCacheAnnotation = JandexHelper.getSingleAnnotation(
@@ -418,6 +486,8 @@ private Caching determineCachingSettings() {
 		}
 		return caching;
 	}
+
+
 }
 
 

hibernate-core/src/main/java/org/hibernate/metamodel/internal/source/annotations/entity/ConfiguredClass.java

@@ -30,6 +30,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumMap;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -46,6 +47,7 @@
 import org.hibernate.internal.CoreMessageLogger;
 import org.hibernate.internal.util.ReflectHelper;
 import org.hibernate.internal.util.StringHelper;
+import org.hibernate.internal.util.collections.CollectionHelper;
 import org.hibernate.metamodel.internal.source.annotations.AnnotationBindingContext;
 import org.hibernate.metamodel.internal.source.annotations.attribute.AssociationAttribute;
 import org.hibernate.metamodel.internal.source.annotations.attribute.AttributeOverride;
@@ -513,6 +515,7 @@ else if ( attribute.isVersioned() ) {
 			case ONE_TO_ONE:
 			case MANY_TO_ONE: {
 				final AssociationAttribute attribute = AssociationAttribute.createAssociationAttribute(
+						classInfo,
 						attributeName,
 						resolvedMember.getType().getErasedType(),
 						attributeNature,
@@ -599,39 +602,36 @@ private MappedAttribute.Nature determineAttributeNature( Map<DotName,
 			List<AnnotationInstance>> annotations,
 			Class<?> attributeType,
 			Class<?> referencedCollectionType ) {
-		EnumMap<MappedAttribute.Nature, AnnotationInstance> discoveredAttributeTypes =
+		EnumSet<MappedAttribute.Nature>  discoveredAttributeTypes = EnumSet.noneOf( MappedAttribute.Nature.class );
-				new EnumMap<MappedAttribute.Nature, AnnotationInstance>( MappedAttribute.Nature.class );
-
 		AnnotationInstance oneToOne = JandexHelper.getSingleAnnotation( annotations, JPADotNames.ONE_TO_ONE );
 		if ( oneToOne != null ) {
-			discoveredAttributeTypes.put( MappedAttribute.Nature.ONE_TO_ONE, oneToOne );
+			discoveredAttributeTypes.add( MappedAttribute.Nature.ONE_TO_ONE );
 		}
 
 		AnnotationInstance oneToMany = JandexHelper.getSingleAnnotation( annotations, JPADotNames.ONE_TO_MANY );
 		if ( oneToMany != null ) {
-			discoveredAttributeTypes.put( MappedAttribute.Nature.ONE_TO_MANY, oneToMany );
+			discoveredAttributeTypes.add( MappedAttribute.Nature.ONE_TO_MANY );
 		}
 
 		AnnotationInstance manyToOne = JandexHelper.getSingleAnnotation( annotations, JPADotNames.MANY_TO_ONE );
 		if ( manyToOne != null ) {
-			discoveredAttributeTypes.put( MappedAttribute.Nature.MANY_TO_ONE, manyToOne );
+			discoveredAttributeTypes.add( MappedAttribute.Nature.MANY_TO_ONE );
 		}
 
 		AnnotationInstance manyToMany = JandexHelper.getSingleAnnotation( annotations, JPADotNames.MANY_TO_MANY );
 		if ( manyToMany != null ) {
-			discoveredAttributeTypes.put( MappedAttribute.Nature.MANY_TO_MANY, manyToMany );
+			discoveredAttributeTypes.add( MappedAttribute.Nature.MANY_TO_MANY );
 		}
 
 		AnnotationInstance embeddedId = JandexHelper.getSingleAnnotation( annotations, JPADotNames.EMBEDDED_ID );
 		if ( embeddedId != null ) {
-			discoveredAttributeTypes.put( MappedAttribute.Nature.EMBEDDED_ID, embeddedId );
+			discoveredAttributeTypes.add( MappedAttribute.Nature.EMBEDDED_ID );
 		}
 
 		AnnotationInstance embedded = JandexHelper.getSingleAnnotation( 
 				annotations, JPADotNames.EMBEDDED );
 		if ( embedded != null ) {
-			discoveredAttributeTypes.put( MappedAttribute.Nature.EMBEDDED, 
+			discoveredAttributeTypes.add( MappedAttribute.Nature.EMBEDDED );
-					embedded );
 		} else if ( embeddedId == null ) {
 			// For backward compatibility, we're allowing attributes of an
 			// @Embeddable type to leave off @Embedded.  Check the type's
@@ -644,8 +644,7 @@ private MappedAttribute.Nature determineAttributeNature( Map<DotName,
 					&& JandexHelper.getSingleAnnotation( 
 							typeClassInfo.annotations(),
 							JPADotNames.EMBEDDABLE ) != null ) {
-				discoveredAttributeTypes.put( MappedAttribute.Nature.EMBEDDED,
+				discoveredAttributeTypes.add( MappedAttribute.Nature.EMBEDDED );
-						null );
 			}
 		}
 
@@ -653,19 +652,9 @@ private MappedAttribute.Nature determineAttributeNature( Map<DotName,
 				annotations,
 				JPADotNames.ELEMENT_COLLECTION
 		);
-		if ( elementCollection != null ) {
+		if ( elementCollection != null || ( discoveredAttributeTypes.isEmpty() && CollectionHelper.isCollectionOrArray( attributeType ) )) {
-			// class info can be null for types like string, etc where there are no annotations
+			boolean isEmbeddable = isEmbeddableType( referencedCollectionType );
-			ClassInfo classInfo = getLocalBindingContext().getIndex().getClassByName(
+			discoveredAttributeTypes.add( isEmbeddable? MappedAttribute.Nature.ELEMENT_COLLECTION_EMBEDDABLE : MappedAttribute.Nature.ELEMENT_COLLECTION_BASIC );
-					DotName.createSimple(
-							referencedCollectionType.getName()
-					)
-			);
-			if ( classInfo != null && classInfo.annotations().get( JPADotNames.EMBEDDABLE ) != null ) {
-				discoveredAttributeTypes.put( MappedAttribute.Nature.ELEMENT_COLLECTION_EMBEDDABLE, elementCollection );
-			}
-			else {
-				discoveredAttributeTypes.put( MappedAttribute.Nature.ELEMENT_COLLECTION_BASIC, elementCollection );
-			}
 		}
 
 		int size = discoveredAttributeTypes.size();
@@ -673,12 +662,22 @@ private MappedAttribute.Nature determineAttributeNature( Map<DotName,
 			case 0:
 				return MappedAttribute.Nature.BASIC;
 			case 1:
-				return discoveredAttributeTypes.keySet().iterator().next();
+				return discoveredAttributeTypes.iterator().next();
 			default:
 				throw new AnnotationException( "More than one association type configured for property  " + getName() + " of class " + getName() );
 		}
 	}
 
+	private boolean isEmbeddableType(Class<?> referencedCollectionType) {
+		// class info can be null for types like string, etc where there are no annotations
+		ClassInfo classInfo = getLocalBindingContext().getIndex().getClassByName(
+				DotName.createSimple(
+						referencedCollectionType.getName()
+				)
+		);
+		return classInfo != null && classInfo.annotations().get( JPADotNames.EMBEDDABLE ) != null;
+	}
+
 	private ResolvedMember findResolvedMember(String name, ResolvedMember[] resolvedMembers) {
 		for ( ResolvedMember resolvedMember : resolvedMembers ) {
 			if ( resolvedMember.getName().equals( name ) ) {

hibernate-core/src/main/java/org/hibernate/metamodel/internal/source/annotations/xml/mocker/MockHelper.java

@@ -53,7 +53,7 @@
  */
 public class MockHelper {
 
-	static final AnnotationValue[] EMPTY_ANNOTATION_VALUE_ARRAY = new AnnotationValue[0];
+	public static final AnnotationValue[] EMPTY_ANNOTATION_VALUE_ARRAY = new AnnotationValue[0];
 	static final Type[] EMPTY_TYPE_ARRAY = new Type[0];
 
 	/**
@@ -305,9 +305,9 @@ private static MethodInfo getMethodInfo(ClassInfo classInfo, Method method) {
 		);
 	}
 
-	enum TargetType {METHOD, FIELD, PROPERTY}
+	public enum TargetType {METHOD, FIELD, PROPERTY}
 
-	static AnnotationTarget getTarget(ServiceRegistry serviceRegistry, ClassInfo classInfo, String name, TargetType type) {
+	public static AnnotationTarget getTarget(ServiceRegistry serviceRegistry, ClassInfo classInfo, String name, TargetType type) {
 		Class clazz = serviceRegistry.getService( ClassLoaderService.class ).classForName( classInfo.toString() );
 		switch ( type ) {
 			case FIELD:
@@ -440,7 +440,7 @@ private static Type getType(Class clazz) {
 		return Type.create( DotName.createSimple( clazz.getName() ), getTypeKind( clazz ) );
 	}
 
-	private static Type.Kind getTypeKind(Class clazz) {
+	public static Type.Kind getTypeKind(Class clazz) {
 		Type.Kind kind;
 		if ( clazz == Void.TYPE ) {
 			kind = Type.Kind.VOID;

hibernate-core/src/main/java/org/hibernate/metamodel/internal/source/hbm/BagAttributeSourceImpl.java

@@ -51,7 +51,7 @@ public JaxbBagElement getPluralAttributeElement() {
 
 	@Override
 	public boolean isOrdered() {
-		return getOrder() != null;
+		return StringHelper.isNotEmpty( getOrder() );
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/metamodel/internal/source/hbm/SetAttributeSourceImpl.java

@@ -64,7 +64,7 @@ public String getComparatorName() {
 
 	@Override
 	public boolean isOrdered() {
-		return getOrder() != null;
+		return StringHelper.isNotEmpty( getOrder() );
 	}
 
 	@Override

hibernate-core/src/test/java/org/hibernate/test/annotations/collectionelement/indexedCollection/IndexedCollectionOfElementsTest.java

@@ -26,6 +26,7 @@
 import org.junit.Test;
 
 import org.hibernate.Session;
+import org.hibernate.testing.FailureExpectedWithNewMetamodel;
 import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
 
 import static org.junit.Assert.assertEquals;
@@ -33,6 +34,7 @@
 /**
  * @author Emmanuel Bernard
  */
+@FailureExpectedWithNewMetamodel
 public class IndexedCollectionOfElementsTest extends BaseCoreFunctionalTestCase {
 	@Test
 	public void testIndexedCollectionOfElements() throws Exception {