HHH-8164 - Deprecate @Sort in favor of @SortNatural and @SortComparator

hibernate-core/src/main/java/org/hibernate/annotations/OrderBy.java

@@ -32,10 +32,15 @@ import static java.lang.annotation.RetentionPolicy.RUNTIME;
 /**
  * Order a collection using SQL ordering (not HQL ordering).
  *
+ * Different from {@link javax.persistence.OrderBy} in that this expects SQL fragment, JPA OrderBy expects a
+ * valid JPQL order-by fragment.
+ *
  * @author Emmanuel Bernard
+ * @author Steve Ebersole
  *
  * @see javax.persistence.OrderBy
- * @see Sort
+ * @see SortComparator
+ * @see SortNatural
  */
 @Target({METHOD, FIELD})
 @Retention(RUNTIME)

hibernate-core/src/main/java/org/hibernate/annotations/Sort.java

@@ -36,9 +36,12 @@ import static java.lang.annotation.RetentionPolicy.RUNTIME;
  * @author Emmanuel Bernard
  *
  * @see OrderBy
+ *
+ * @deprecated Use {@link SortComparator} or {@link SortNatural} instead depending on need.
  */
 @Target({METHOD, FIELD})
 @Retention(RUNTIME)
+@Deprecated
 public @interface Sort {
 	/**
 	 * The type of sorting to use.  The default is to not use sorting.

hibernate-core/src/main/java/org/hibernate/annotations/SortComparator.java

@@ -0,0 +1,46 @@
+/*
+ * 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.annotations;
+
+import java.util.Comparator;
+
+/**
+ * Specifies in-memory Set/Map sorting using a specified {@link Comparator} for sorting.
+ *
+ * NOTE : Sorting is different than ordering (see {@link OrderBy}) which is applied during the SQL SELECT.
+ *
+ * For sorting based on natural sort order, use {@link SortNatural} instead.  It is illegal to combine
+ * {@link SortComparator} and {@link SortNatural}.
+ *
+ * @see OrderBy
+ * @see SortComparator
+ *
+ * @author Steve Ebersole
+ */
+public @interface SortComparator {
+	/**
+	 * Specifies the comparator class to use.
+	 */
+	Class<? extends Comparator<?>> value();
+}

hibernate-core/src/main/java/org/hibernate/annotations/SortNatural.java

@@ -0,0 +1,40 @@
+/*
+ * 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.annotations;
+
+/**
+ * Specifies in-memory Set/Map sorting using natural sorting.
+ *
+ * NOTE : Sorting is different than ordering (see {@link OrderBy}) which is applied during the SQL SELECT.
+ *
+ * For sorting based on a comparator, use {@link SortComparator} instead.  It is illegal to combine
+ *{@link SortComparator} and SortNatural.
+ *
+ * @see OrderBy
+ * @see SortComparator
+ *
+ * @author Steve Ebersole
+ */
+public @interface SortNatural {
+}

hibernate-core/src/main/java/org/hibernate/annotations/SortType.java

@@ -27,7 +27,10 @@ package org.hibernate.annotations;
  * Possible collection sorting strategies.
  *
  * @author Emmanuel Bernard
+ *
+ * @deprecated Since {@link Sort} is deprecated.
  */
+@Deprecated
 public enum SortType {
 	/**
 	 * The collection is unsorted.

hibernate-core/src/main/java/org/hibernate/cfg/AnnotationBinder.java

@@ -126,6 +126,8 @@ import org.hibernate.annotations.Parameter;
 import org.hibernate.annotations.Parent;
 import org.hibernate.annotations.Proxy;
 import org.hibernate.annotations.Sort;
+import org.hibernate.annotations.SortComparator;
+import org.hibernate.annotations.SortNatural;
 import org.hibernate.annotations.Source;
 import org.hibernate.annotations.Tuplizer;
 import org.hibernate.annotations.Tuplizers;
@@ -1733,14 +1735,16 @@ public final class AnnotationBinder {
 				collectionBinder.setIndexColumn( indexColumn );
 				collectionBinder.setMapKey( property.getAnnotation( MapKey.class ) );
 				collectionBinder.setPropertyName( inferredData.getPropertyName() );
-				BatchSize batchAnn = property.getAnnotation( BatchSize.class );
+
-				collectionBinder.setBatchSize( batchAnn );
+				collectionBinder.setBatchSize( property.getAnnotation( BatchSize.class ) );
-				javax.persistence.OrderBy ejb3OrderByAnn = property.getAnnotation( javax.persistence.OrderBy.class );
+
-				OrderBy orderByAnn = property.getAnnotation( OrderBy.class );
+				collectionBinder.setJpaOrderBy( property.getAnnotation( javax.persistence.OrderBy.class ) );
-				collectionBinder.setEjb3OrderBy( ejb3OrderByAnn );
+				collectionBinder.setSqlOrderBy( property.getAnnotation( OrderBy.class ) );
-				collectionBinder.setSqlOrderBy( orderByAnn );
+
-				Sort sortAnn = property.getAnnotation( Sort.class );
+				collectionBinder.setSort( property.getAnnotation( Sort.class ) );
-				collectionBinder.setSort( sortAnn );
+				collectionBinder.setNaturalSort( property.getAnnotation( SortNatural.class ) );
+				collectionBinder.setComparatorSort( property.getAnnotation( SortComparator.class ) );
+
 				Cache cachAnn = property.getAnnotation( Cache.class );
 				collectionBinder.setCache( cachAnn );
 				collectionBinder.setPropertyHolder( propertyHolder );

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

@@ -31,8 +31,8 @@ import org.hibernate.mapping.PersistentClass;
  * @author Matthew Inger
  */
 public class BagBinder extends CollectionBinder {
-
 	public BagBinder() {
+		super( false );
 	}
 
 	protected Collection createCollection(PersistentClass persistentClass) {

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

@@ -72,6 +72,8 @@ import org.hibernate.annotations.SQLDeleteAll;
 import org.hibernate.annotations.SQLInsert;
 import org.hibernate.annotations.SQLUpdate;
 import org.hibernate.annotations.Sort;
+import org.hibernate.annotations.SortComparator;
+import org.hibernate.annotations.SortNatural;
 import org.hibernate.annotations.SortType;
 import org.hibernate.annotations.Where;
 import org.hibernate.annotations.WhereJoinTable;
@@ -138,11 +140,6 @@ public abstract class CollectionBinder {
 	String cacheRegionName;
 	private boolean oneToMany;
 	protected IndexColumn indexColumn;
-	private String orderBy;
-	protected String hqlOrderBy;
-	private boolean isSorted;
-	private Class comparator;
-	private boolean hasToBeSorted;
 	protected boolean cascadeDeleteEnabled;
 	protected String mapKeyPropertyName;
 	private boolean insertable = true;
@@ -163,6 +160,13 @@ public abstract class CollectionBinder {
 	private AccessType accessType;
 	private boolean hibernateExtensionMapping;
 
+	private boolean isSortedCollection;
+	private javax.persistence.OrderBy jpaOrderBy;
+	private OrderBy sqlOrderBy;
+	private Sort deprecatedSort;
+	private SortNatural naturalSort;
+	private SortComparator comparatorSort;
+
 	private String explicitType;
 	private Properties explicitTypeParameters = new Properties();
 
@@ -220,27 +224,24 @@ public abstract class CollectionBinder {
 		this.batchSize = batchSize == null ? -1 : batchSize.size();
 	}
 
-	public void setEjb3OrderBy(javax.persistence.OrderBy orderByAnn) {
+	public void setJpaOrderBy(javax.persistence.OrderBy jpaOrderBy) {
-		if ( orderByAnn != null ) {
+		this.jpaOrderBy = jpaOrderBy;
-			hqlOrderBy = orderByAnn.value();
-		}
 	}
 
-	public void setSqlOrderBy(OrderBy orderByAnn) {
+	public void setSqlOrderBy(OrderBy sqlOrderBy) {
-		if ( orderByAnn != null ) {
+		this.sqlOrderBy = sqlOrderBy;
-			if ( !BinderHelper.isEmptyAnnotationValue( orderByAnn.clause() ) ) {
-				orderBy = orderByAnn.clause();
-			}
-		}
 	}
 
-	public void setSort(Sort sortAnn) {
+	public void setSort(Sort deprecatedSort) {
-		if ( sortAnn != null ) {
+		this.deprecatedSort = deprecatedSort;
-			isSorted = !SortType.UNSORTED.equals( sortAnn.type() );
-			if ( isSorted && SortType.COMPARATOR.equals( sortAnn.type() ) ) {
-				comparator = sortAnn.comparator();
 	}
+
+	public void setNaturalSort(SortNatural naturalSort) {
+		this.naturalSort = naturalSort;
 	}
+
+	public void setComparatorSort(SortComparator comparatorSort) {
+		this.comparatorSort = comparatorSort;
 	}
 
 	/**
@@ -252,7 +253,7 @@ public abstract class CollectionBinder {
 			boolean isIndexed,
 			boolean isHibernateExtensionMapping,
 			Mappings mappings) {
-		CollectionBinder result;
+		final CollectionBinder result;
 		if ( property.isArray() ) {
 			if ( property.getElementClass().isPrimitive() ) {
 				result = new PrimitiveArrayBinder();
@@ -269,7 +270,7 @@ public abstract class CollectionBinder {
 					throw new AnnotationException( "Set do not support @CollectionId: "
 							+ StringHelper.qualify( entityName, property.getName() ) );
 				}
-				result = new SetBinder();
+				result = new SetBinder( false );
 			}
 			else if ( java.util.SortedSet.class.equals( returnedClass ) ) {
 				if ( property.isAnnotationPresent( CollectionId.class ) ) {
@@ -283,7 +284,7 @@ public abstract class CollectionBinder {
 					throw new AnnotationException( "Map do not support @CollectionId: "
 							+ StringHelper.qualify( entityName, property.getName() ) );
 				}
-				result = new MapBinder();
+				result = new MapBinder( false );
 			}
 			else if ( java.util.SortedMap.class.equals( returnedClass ) ) {
 				if ( property.isAnnotationPresent( CollectionId.class ) ) {
@@ -351,11 +352,8 @@ public abstract class CollectionBinder {
 		return result;
 	}
 
-	protected CollectionBinder() {
+	protected CollectionBinder(boolean isSortedCollection) {
-	}
+		this.isSortedCollection = isSortedCollection;
-
-	protected CollectionBinder(boolean sorted) {
-		this.hasToBeSorted = sorted;
 	}
 
 	public void setMappedBy(String mappedBy) {
@@ -427,11 +425,6 @@ public abstract class CollectionBinder {
 		//set laziness
 		defineFetchingStrategy();
 		collection.setBatchSize( batchSize );
-		if ( orderBy != null && hqlOrderBy != null ) {
-			throw new AnnotationException(
-					"Cannot use sql order by clause in conjunction of EJB3 order by clause: " + safeCollectionRole()
-			);
-		}
 
 		collection.setMutable( !property.isAnnotationPresent( Immutable.class ) );
 
@@ -449,36 +442,7 @@ public abstract class CollectionBinder {
 			collection.setCollectionPersisterClass( persisterAnn.impl() );
 		}
 
-		// set ordering
+		applySortingAndOrdering( collection );
-		if ( orderBy != null ) collection.setOrderBy( orderBy );
-		if ( isSorted ) {
-			collection.setSorted( true );
-			if ( comparator != null ) {
-				try {
-					collection.setComparator( (Comparator) comparator.newInstance() );
-				}
-				catch (ClassCastException e) {
-					throw new AnnotationException(
-							"Comparator not implementing java.util.Comparator class: "
-									+ comparator.getName() + "(" + safeCollectionRole() + ")"
-					);
-				}
-				catch (Exception e) {
-					throw new AnnotationException(
-							"Could not instantiate comparator class: "
-									+ comparator.getName() + "(" + safeCollectionRole() + ")"
-					);
-				}
-			}
-		}
-		else {
-			if ( hasToBeSorted ) {
-				throw new AnnotationException(
-						"A sorted collection has to define @Sort: "
-								+ safeCollectionRole()
-				);
-			}
-		}
 
 		//set cache
 		if ( StringHelper.isNotEmpty( cacheConcurrencyStrategy ) ) {
@@ -577,6 +541,99 @@ public abstract class CollectionBinder {
 		propertyHolder.addProperty( prop, declaringClass );
 	}
 
+	private void applySortingAndOrdering(Collection collection) {
+		boolean isSorted = isSortedCollection;
+
+		boolean hadOrderBy = false;
+		boolean hadExplicitSort = false;
+
+		Class<? extends Comparator> comparatorClass = null;
+
+		if ( jpaOrderBy == null && sqlOrderBy == null ) {
+			if ( deprecatedSort != null ) {
+				LOG.debug( "Encountered deprecated @Sort annotation; use @SortNatural or @SortComparator instead." );
+				if ( naturalSort != null || comparatorSort != null ) {
+					throw buildIllegalSortCombination();
+				}
+				hadExplicitSort = deprecatedSort.type() != SortType.UNSORTED;
+				if ( deprecatedSort.type() == SortType.NATURAL ) {
+					isSorted = true;
+				}
+				else if ( deprecatedSort.type() == SortType.COMPARATOR ) {
+					isSorted = true;
+					comparatorClass = deprecatedSort.comparator();
+				}
+			}
+			else if ( naturalSort != null ) {
+				if ( comparatorSort != null ) {
+					throw buildIllegalSortCombination();
+				}
+				hadExplicitSort = true;
+			}
+			else if ( comparatorSort != null ) {
+				hadExplicitSort = true;
+				comparatorClass = comparatorSort.value();
+			}
+		}
+		else {
+			if ( jpaOrderBy != null && sqlOrderBy != null ) {
+				throw new AnnotationException(
+						String.format(
+								"Illegal combination of @%s and @%s on %s",
+								javax.persistence.OrderBy.class.getName(),
+								OrderBy.class.getName(),
+								safeCollectionRole()
+						)
+				);
+			}
+
+			hadOrderBy = true;
+			hadExplicitSort = false;
+
+			// we can only apply the sql-based order by up front.  The jpa order by has to wait for second pass
+			if ( sqlOrderBy != null ) {
+				collection.setOrderBy( sqlOrderBy.clause() );
+			}
+		}
+
+		if ( isSortedCollection ) {
+			if ( ! hadExplicitSort && !hadOrderBy ) {
+				throw new AnnotationException(
+						"A sorted collection must define and ordering or sorting : " + safeCollectionRole()
+				);
+			}
+		}
+
+		collection.setSorted( isSortedCollection || hadExplicitSort );
+
+		if ( comparatorClass != null ) {
+			try {
+				collection.setComparator( comparatorClass.newInstance() );
+			}
+			catch (Exception e) {
+				throw new AnnotationException(
+						String.format(
+								"Could not instantiate comparator class [%s] for %s",
+								comparatorClass.getName(),
+								safeCollectionRole()
+						)
+				);
+			}
+		}
+	}
+
+	private AnnotationException buildIllegalSortCombination() {
+		return new AnnotationException(
+				String.format(
+						"Illegal combination of annotations on %s.  Only one of @%s, @%s and @%s can be used",
+						safeCollectionRole(),
+						Sort.class.getName(),
+						SortNatural.class.getName(),
+						SortComparator.class.getName()
+				)
+		);
+	}
+
 	private void defineFetchingStrategy() {
 		LazyCollection lazy = property.getAnnotation( LazyCollection.class );
 		Fetch fetch = property.getAnnotation( Fetch.class );
@@ -722,7 +779,7 @@ public abstract class CollectionBinder {
 					fkJoinColumns,
 					collType,
 					cascadeDeleteEnabled,
-					ignoreNotFound, hqlOrderBy,
+					ignoreNotFound,
 					mappings,
 					inheritanceStatePerClass
 			);
@@ -739,7 +796,10 @@ public abstract class CollectionBinder {
 					isEmbedded, collType,
 					ignoreNotFound, unique,
 					cascadeDeleteEnabled,
-					associationTableBinder, property, propertyHolder, hqlOrderBy, mappings
+					associationTableBinder,
+					property,
+					propertyHolder,
+					mappings
 			);
 			return false;
 		}
@@ -752,7 +812,6 @@ public abstract class CollectionBinder {
 			XClass collectionType,
 			boolean cascadeDeleteEnabled,
 			boolean ignoreNotFound,
-			String hqlOrderBy,
 			Mappings mappings,
 			Map<XClass, InheritanceState> inheritanceStatePerClass) {
 		if ( LOG.isDebugEnabled() ) {
@@ -765,8 +824,20 @@ public abstract class CollectionBinder {
 
 		String assocClass = oneToMany.getReferencedEntityName();
 		PersistentClass associatedClass = (PersistentClass) persistentClasses.get( assocClass );
-		String orderBy = buildOrderByClauseFromHql( hqlOrderBy, associatedClass, collection.getRole() );
+		if ( jpaOrderBy != null ) {
-		if ( orderBy != null ) collection.setOrderBy( orderBy );
+			final String jpaOrderByFragment = jpaOrderBy.value();
+			if ( StringHelper.isNotEmpty( jpaOrderByFragment ) ) {
+				final String orderByFragment = buildOrderByClauseFromHql(
+						jpaOrderBy.value(),
+						associatedClass,
+						collection.getRole()
+				);
+				if ( StringHelper.isNotEmpty( orderByFragment ) ) {
+					collection.setOrderBy( orderByFragment );
+				}
+			}
+		}
+
 		if ( mappings == null ) {
 			throw new AssertionFailure(
 					"CollectionSecondPass for oneToMany should not be called with null mappings"
@@ -1037,10 +1108,10 @@ public abstract class CollectionBinder {
 			TableBinder associationTableBinder,
 			XProperty property,
 			PropertyHolder parentPropertyHolder,
-			String hqlOrderBy,
 			Mappings mappings) throws MappingException {
-
 		PersistentClass collectionEntity = (PersistentClass) persistentClasses.get( collType.getName() );
+		final String hqlOrderBy = extractHqlOrderBy( jpaOrderBy );
+
 		boolean isCollectionOfEntities = collectionEntity != null;
 		ManyToAny anyAnn = property.getAnnotation( ManyToAny.class );
         if (LOG.isDebugEnabled()) {
@@ -1162,7 +1233,7 @@ public abstract class CollectionBinder {
 			element.setFetchMode( FetchMode.JOIN );
 			element.setLazy( false );
 			element.setIgnoreNotFound( ignoreNotFound );
-			// as per 11.1.38 of JPA-2 spec, default to primary key if no column is specified by @OrderBy.
+			// as per 11.1.38 of JPA 2.0 spec, default to primary key if no column is specified by @OrderBy.
 			if ( hqlOrderBy != null ) {
 				collValue.setManyToManyOrdering(
 						buildOrderByClauseFromHql( hqlOrderBy, collectionEntity, collValue.getRole() )
@@ -1308,6 +1379,16 @@ public abstract class CollectionBinder {
 
 	}
 
+	private String extractHqlOrderBy(javax.persistence.OrderBy jpaOrderBy) {
+		if ( jpaOrderBy != null ) {
+			final String jpaOrderByFragment = jpaOrderBy.value();
+			return StringHelper.isNotEmpty( jpaOrderByFragment )
+					? jpaOrderByFragment
+					: null;
+		}
+		return null;
+	}
+
 	private static void checkFilterConditions(Collection collValue) {
 		//for now it can't happen, but sometime soon...
 		if ( ( collValue.getFilters().size() != 0 || StringHelper.isNotEmpty( collValue.getWhere() ) ) &&

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

@@ -60,6 +60,7 @@ public class ListBinder extends CollectionBinder {
 	private static final CoreMessageLogger LOG = Logger.getMessageLogger( CoreMessageLogger.class, ListBinder.class.getName() );
 
 	public ListBinder() {
+		super( false );
 	}
 
 	@Override

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

@@ -77,10 +77,6 @@ public class MapBinder extends CollectionBinder {
 		super( sorted );
 	}
 
-	public MapBinder() {
-		super();
-	}
-
 	public boolean isMap() {
 		return true;
 	}

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

@@ -33,10 +33,6 @@ import org.hibernate.mapping.PersistentClass;
  * @author Matthew Inger
  */
 public class SetBinder extends CollectionBinder {
-
-	public SetBinder() {
-	}
-
 	public SetBinder(boolean sorted) {
 		super( sorted );
 	}