HHH-11400 - HHH90000016: Found use of deprecated 'collection property' issue for valid JPQL query

(cherry picked from commit 2dca5f2ceb)

hibernate-core/src/main/java/org/hibernate/hql/internal/ast/HqlSqlWalker.java

@@ -23,6 +23,7 @@ import org.hibernate.QueryException;
 import org.hibernate.engine.internal.JoinSequence;
 import org.hibernate.engine.internal.ParameterBinder;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
+import org.hibernate.hql.internal.CollectionProperties;
 import org.hibernate.hql.internal.antlr.HqlSqlBaseWalker;
 import org.hibernate.hql.internal.antlr.HqlSqlTokenTypes;
 import org.hibernate.hql.internal.antlr.HqlTokenTypes;
@@ -66,6 +67,7 @@ import org.hibernate.id.BulkInsertionCapableIdentifierGenerator;
 import org.hibernate.id.IdentifierGenerator;
 import org.hibernate.internal.CoreLogging;
 import org.hibernate.internal.CoreMessageLogger;
+import org.hibernate.internal.log.DeprecationLogger;
 import org.hibernate.internal.util.StringHelper;
 import org.hibernate.internal.util.collections.ArrayHelper;
 import org.hibernate.param.CollectionFilterKeyParameterSpecification;
@@ -73,6 +75,7 @@ import org.hibernate.param.NamedParameterSpecification;
 import org.hibernate.param.ParameterSpecification;
 import org.hibernate.param.PositionalParameterSpecification;
 import org.hibernate.param.VersionTypeSeedParameterSpecification;
+import org.hibernate.persister.collection.CollectionPropertyNames;
 import org.hibernate.persister.collection.QueryableCollection;
 import org.hibernate.persister.entity.Queryable;
 import org.hibernate.sql.JoinType;
@@ -578,9 +581,30 @@ public class HqlSqlWalker extends HqlSqlBaseWalker implements ErrorReporter, Par
 		DotNode dotNode = (DotNode) dot;
 		FromReferenceNode lhs = dotNode.getLhs();
 		AST rhs = lhs.getNextSibling();
-		switch ( rhs.getType() ) {
+
-			case SqlTokenTypes.ELEMENTS:
+		// this used to be a switch statement based on the rhs's node type
-			case SqlTokenTypes.INDICES:
+		//		expecting it to be SqlTokenTypes.ELEMENTS or
+		//		SqlTokenTypes.INDICES in the cases where the re-arranging is needed
+		//
+		// In such cases it additionally expects the RHS to be a CollectionFunction node.
+		//
+		// However, in my experience these assumptions sometimes did not works as sometimes the node
+		// 		types come in with the node type WEIRD_IDENT.  What this does now is to:
+		//			1) see if the LHS is a collection
+		//			2) see if the RHS is a reference to one of the "collection properties".
+		//		if both are true, we log a deprecation warning
+		if ( lhs.getDataType() != null
+				&& lhs.getDataType().isCollectionType() ) {
+			if ( CollectionProperties.isCollectionProperty( rhs.getText() ) ) {
+				DeprecationLogger.DEPRECATION_LOGGER.logDeprecationOfCollectionPropertiesInHql(
+						rhs.getText(),
+						lhs.getPath()
+				);
+			}
+
+			// perform the re-arrangement
+			if ( CollectionPropertyNames.COLLECTION_INDICES.equalsIgnoreCase( rhs.getText() )
+					|| CollectionPropertyNames.COLLECTION_ELEMENTS.equalsIgnoreCase( rhs.getText() ) ) {
 				if ( LOG.isDebugEnabled() ) {
 					LOG.debugf(
 							"lookupProperty() %s => %s(%s)",
@@ -589,7 +613,17 @@ public class HqlSqlWalker extends HqlSqlBaseWalker implements ErrorReporter, Par
 							lhs.getPath()
 					);
 				}
-				CollectionFunction f = (CollectionFunction) rhs;
+
+				final CollectionFunction f;
+				if ( rhs instanceof CollectionFunction ) {
+					f = (CollectionFunction) rhs;
+				}
+				else {
+					f = new CollectionFunction();
+					f.initialize( SqlTokenTypes.METHOD_CALL, rhs.getText() );
+					f.initialize( this );
+				}
+
 				// Re-arrange the tree so that the collection function is the root and the lhs is the path.
 				f.setFirstChild( lhs );
 				lhs.setNextSibling( null );
@@ -597,12 +631,13 @@ public class HqlSqlWalker extends HqlSqlBaseWalker implements ErrorReporter, Par
 				resolve( lhs );            // Don't forget to resolve the argument!
 				f.resolve( inSelect );    // Resolve the collection function now.
 				return f;
-			default:
+			}
-				// Resolve everything up to this dot, but don't resolve the placeholders yet.
+		}
+
+		// otherwise, resolve the path and return it
 		dotNode.resolveFirstChild();
 		return dotNode;
 	}
-	}
 
 	@Override
 	protected boolean isNonQualifiedPropertyRef(AST ident) {

hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/FromElementType.java

@@ -429,7 +429,7 @@ class FromElementType {
 			// this is hacky, but really this is difficult to handle given the current codebase.
 			if ( persister != propertyMapping ) {
 				// we want the subquery...
-				DeprecationLogger.DEPRECATION_LOGGER.logDeprecationOfCollectionPropertiesInHql( path, fromElement.getClassAlias() );
+//				DeprecationLogger.DEPRECATION_LOGGER.logDeprecationOfCollectionPropertiesInHql( path, fromElement.getClassAlias() );
 				return getCollectionPropertyReference( path ).toColumns( tableAlias );
 			}
 		}

hibernate-core/src/test/java/org/hibernate/test/hql/CollectionPropertyDeprecationsTest.java

@@ -0,0 +1,201 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.test.hql;
+
+import java.util.Collections;
+
+import org.hibernate.hql.internal.ast.ASTQueryTranslatorFactory;
+import org.hibernate.hql.internal.ast.QueryTranslatorImpl;
+import org.hibernate.hql.spi.QueryTranslatorFactory;
+import org.hibernate.internal.log.DeprecationLogger;
+
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.hibernate.testing.logger.LoggerInspectionRule;
+import org.hibernate.testing.logger.Triggerable;
+import org.junit.Rule;
+import org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Tests that the forms of referencing parts of and info about collections as a property
+ * gets logged as a deprecation warning.  E.g. {@code `h.family.elements`} is
+ * deprecated in preference for {@code `elements(h.family)`}
+ *
+ * @author Steve Ebersole
+ */
+public class CollectionPropertyDeprecationsTest extends BaseCoreFunctionalTestCase {
+	@Rule
+	public LoggerInspectionRule logInspection = new LoggerInspectionRule(
+			DeprecationLogger.DEPRECATION_LOGGER
+	);
+
+	@Override
+	public String[] getMappings() {
+		return new String[] {"hql/Animal.hbm.xml"};
+	}
+
+	@Override
+	public boolean createSchema() {
+		return false;
+	}
+
+	@Override
+	public boolean rebuildSessionFactoryOnError() {
+		return false;
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-11400" )
+	public void testReferencingBagElements() {
+		Triggerable triggerable = logInspection.watchForLogMessages( "HHH90000016" );
+
+		// first the accepted ways
+		compileQuery( "select elements(h.friends) from Human h" );
+		assertFalse( triggerable.wasTriggered() );
+		triggerable.reset();
+		compileQuery( "select h from Human h where h in elements(h.friends)" );
+		assertFalse( triggerable.wasTriggered() );
+		triggerable.reset();
+
+		// then the deprecated way
+		compileQuery( "select h.friends.elements from Human h" );
+		assertTrue( triggerable.wasTriggered() );
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-11400" )
+	public void testReferencingSetElements() {
+		Triggerable triggerable = logInspection.watchForLogMessages( "HHH90000016" );
+
+		// first the accepted ways
+		compileQuery( "select elements(h.nickNames) from Human h" );
+		assertFalse( triggerable.wasTriggered() );
+		triggerable.reset();
+		compileQuery( "select h from Human h where h.name.first in elements(h.nickNames)" );
+		assertFalse( triggerable.wasTriggered() );
+		triggerable.reset();
+
+		// then the deprecated way
+		compileQuery( "select h.nickNames.elements from Human h" );
+		assertTrue( triggerable.wasTriggered() );
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-11400" )
+	public void testReferencingListElements() {
+		Triggerable triggerable = logInspection.watchForLogMessages( "HHH90000016" );
+
+		// first the accepted ways
+		compileQuery( "select elements(u.permissions) from User u" );
+		assertFalse( triggerable.wasTriggered() );
+		triggerable.reset();
+		compileQuery( "select u from User u where u.userName in elements(u.permissions)" );
+		assertFalse( triggerable.wasTriggered() );
+		triggerable.reset();
+
+		// then the deprecated way
+		compileQuery( "select u.permissions.elements from User u" );
+		assertTrue( triggerable.wasTriggered() );
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-11400" )
+	public void testReferencingListIndices() {
+		Triggerable triggerable = logInspection.watchForLogMessages( "HHH90000016" );
+
+		// first the accepted ways
+		compileQuery( "select indices(u.permissions) from User u" );
+		assertFalse( triggerable.wasTriggered() );
+		triggerable.reset();
+		compileQuery( "select u from User u where u.userName in indices(u.permissions)" );
+		assertFalse( triggerable.wasTriggered() );
+		triggerable.reset();
+
+		// then the deprecated way
+		compileQuery( "select u.permissions.indices from User u" );
+		assertTrue( triggerable.wasTriggered() );
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-11400" )
+	public void testReferencingMapElements() {
+		// NOTE : JPA's VALUE ought to work fine as we never supported
+		// 		that in the legacy form...
+
+		Triggerable triggerable = logInspection.watchForLogMessages( "HHH90000016" );
+
+		// first the accepted ways
+		compileQuery( "select elements(h.family) from Human h" );
+		assertFalse( triggerable.wasTriggered() );
+		triggerable.reset();
+		compileQuery( "select h from Human h where h.name.first in elements(h.family)" );
+		assertFalse( triggerable.wasTriggered() );
+		triggerable.reset();
+
+		// then the deprecated way
+		compileQuery( "select h.family.elements from Human h" );
+		assertTrue( triggerable.wasTriggered() );
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-11400" )
+	public void testReferencingMapIndices() {
+		// NOTE : JPA's KEY ought to work fine as we never supported
+		// 		that in the legacy form...
+
+		Triggerable triggerable = logInspection.watchForLogMessages( "HHH90000016" );
+
+		// first the accepted ways
+		compileQuery( "select indices(h.family) from Human h" );
+		assertFalse( triggerable.wasTriggered() );
+		triggerable.reset();
+		compileQuery( "select h from Human h where h.name.first in indices(h.family)" );
+		assertFalse( triggerable.wasTriggered() );
+		triggerable.reset();
+
+		// then the deprecated way
+		compileQuery( "select h.family.indices from Human h" );
+		assertTrue( triggerable.wasTriggered() );
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-11400" )
+	public void testReferencingSize() {
+		Triggerable triggerable = logInspection.watchForLogMessages( "HHH90000016" );
+
+		// first the accepted ways
+		compileQuery( "select size(h.family) from Human h" );
+		assertFalse( triggerable.wasTriggered() );
+		triggerable.reset();
+		compileQuery( "select h from Human h where size(h.family) = 1" );
+		assertFalse( triggerable.wasTriggered() );
+		triggerable.reset();
+
+		// then the deprecated way
+		compileQuery( "select h.family.size from Human h" );
+		assertTrue( triggerable.wasTriggered() );
+	}
+
+
+
+
+	private QueryTranslatorImpl compileQuery(String hql) {
+		QueryTranslatorFactory ast = new ASTQueryTranslatorFactory();
+		QueryTranslatorImpl newQueryTranslator = (QueryTranslatorImpl) ast.createQueryTranslator(
+				hql,
+				hql,
+				Collections.EMPTY_MAP,
+				sessionFactory(),
+				null
+		);
+		newQueryTranslator.compile( Collections.emptyMap(), false );
+		return newQueryTranslator;
+	}
+}