Author: steve.ebersole(a)jboss.com Date: 2007-03-29 15:08:40 -0400 (Thu, 29 Mar 2007) New Revision: 11371 Modified: branches/Branch_3_2/Hibernate3/src/org/hibernate/hql/ast/tree/DotNode.java branches/Branch_3_2/Hibernate3/test/org/hibernate/test/hql/ASTParserLoadingTest.java branches/Branch_3_2/Hibernate3/test/org/hibernate/test/hql/HQLTest.java Log: HHH-2534 : better improper HQL collection-dereference error message Modified: branches/Branch_3_2/Hibernate3/src/org/hibernate/hql/ast/tree/DotNode.java =================================================================== --- branches/Branch_3_2/Hibernate3/src/org/hibernate/hql/ast/tree/DotNode.java 2007-03-29 17:39:59 UTC (rev 11370) +++ branches/Branch_3_2/Hibernate3/src/org/hibernate/hql/ast/tree/DotNode.java 2007-03-29 19:08:40 UTC (rev 11371) @@ -25,18 +25,28 @@ /** * Represents a reference to a property or alias expression. This should duplicate the relevant behaviors in * PathExpressionParser. - * <hr> - * User: josh<br> - * Date: Dec 16, 2003<br> - * Time: 8:03:09 AM + * + * @author Joshua Davis */ public class DotNode extends FromReferenceNode implements DisplayableNode, SelectExpression { /////////////////////////////////////////////////////////////////////////// // USED ONLY FOR REGRESSION TESTING!!!! + // + // todo : obviously get rid of all this junk ;) /////////////////////////////////////////////////////////////////////////// public static boolean useThetaStyleImplicitJoins = false; public static boolean REGRESSION_STYLE_JOIN_SUPPRESSION = false; + public static interface IllegalCollectionDereferenceExceptionBuilder { + public QueryException buildIllegalCollectionDereferenceException(String collectionPropertyName, FromReferenceNode lhs); + } + public static final IllegalCollectionDereferenceExceptionBuilder DEF_ILLEGAL_COLL_DEREF_EXCP_BUILDER = new IllegalCollectionDereferenceExceptionBuilder() { + public QueryException buildIllegalCollectionDereferenceException(String propertyName, FromReferenceNode lhs) { + String lhsPath = ASTUtil.getPathText( lhs ); + return new QueryException( "illegal attempt to dereference collection [" + lhsPath + "] with element property reference [" + propertyName + "]" ); + } + }; + public static IllegalCollectionDereferenceExceptionBuilder ILLEGAL_COLL_DEREF_EXCP_BUILDER = DEF_ILLEGAL_COLL_DEREF_EXCP_BUILDER; /////////////////////////////////////////////////////////////////////////// private static final Log log = LogFactory.getLog( DotNode.class ); @@ -188,24 +198,26 @@ return; } - // The property is a component... if ( propertyType.isComponentType() ) { + // The property is a component... checkLhsIsNotCollection(); dereferenceComponent( parent ); initText(); } - // The property is another class.. else if ( propertyType.isEntityType() ) { + // The property is another class.. checkLhsIsNotCollection(); dereferenceEntity( ( EntityType ) propertyType, implicitJoin, classAlias, generateJoin, parent ); initText(); } - // The property is a collection... else if ( propertyType.isCollectionType() ) { + // The property is a collection... checkLhsIsNotCollection(); dereferenceCollection( ( CollectionType ) propertyType, implicitJoin, false, classAlias, parent ); } - else { // Otherwise, this is a primitive type. + else { + // Otherwise, this is a primitive type. + checkLhsIsNotCollection(); dereferenceType = DEREF_PRIMITIVE; initText(); } @@ -495,11 +507,10 @@ private void checkLhsIsNotCollection() throws SemanticException { if ( getLhs().getDataType() != null && getLhs().getDataType().isCollectionType() ) { - // TODO : this exactly matches the output of the old parser, but we might want to be more explicit here - // that will however cause regression issues in HQLTest - throw new SemanticException( "illegal syntax near collection: " + propertyName ); + throw ILLEGAL_COLL_DEREF_EXCP_BUILDER.buildIllegalCollectionDereferenceException( propertyName, getLhs() ); } } + private void dereferenceComponent(AST parent) { dereferenceType = DEREF_COMPONENT; setPropertyNameAndPath( parent ); Modified: branches/Branch_3_2/Hibernate3/test/org/hibernate/test/hql/ASTParserLoadingTest.java =================================================================== --- branches/Branch_3_2/Hibernate3/test/org/hibernate/test/hql/ASTParserLoadingTest.java 2007-03-29 17:39:59 UTC (rev 11370) +++ branches/Branch_3_2/Hibernate3/test/org/hibernate/test/hql/ASTParserLoadingTest.java 2007-03-29 19:08:40 UTC (rev 11371) @@ -50,6 +50,9 @@ import org.hibernate.type.Type; import org.hibernate.util.StringHelper; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + /** * Tests the integration of the new AST parser into the loading of query results using * the Hibernate persisters and loaders. @@ -62,6 +65,8 @@ */ public class ASTParserLoadingTest extends FunctionalTestCase { + private static final Log log = LogFactory.getLog( ASTParserLoadingTest.class ); + private List createdAnimalIds = new ArrayList(); public ASTParserLoadingTest(String name) { @@ -96,6 +101,52 @@ return new FunctionalTestClassTestSuite( ASTParserLoadingTest.class ); } + public void testInvalidCollectionDereferencesFail() { + Session s = openSession(); + s.beginTransaction(); + + // control group... + s.createQuery( "from Animal a join a.offspring o where o.description = 'xyz'" ).list(); + s.createQuery( "from Animal a join a.offspring o where o.father.description = 'xyz'" ).list(); + s.createQuery( "from Animal a join a.offspring o order by o.description" ).list(); + s.createQuery( "from Animal a join a.offspring o order by o.father.description" ).list(); + + try { + s.createQuery( "from Animal a where a.offspring.description = 'xyz'" ).list(); + fail( "illegal collection dereference semantic did not cause failure" ); + } + catch( QueryException qe ) { + log.trace( "expected failure...", qe ); + } + + try { + s.createQuery( "from Animal a where a.offspring.father.description = 'xyz'" ).list(); + fail( "illegal collection dereference semantic did not cause failure" ); + } + catch( QueryException qe ) { + log.trace( "expected failure...", qe ); + } + + try { + s.createQuery( "from Animal a order by a.offspring.description" ).list(); + fail( "illegal collection dereference semantic did not cause failure" ); + } + catch( QueryException qe ) { + log.trace( "expected failure...", qe ); + } + + try { + s.createQuery( "from Animal a order by a.offspring.father.description" ).list(); + fail( "illegal collection dereference semantic did not cause failure" ); + } + catch( QueryException qe ) { + log.trace( "expected failure...", qe ); + } + + s.getTransaction().commit(); + s.close(); + } + /** * Copied from {@link HQLTest#testConcatenation} */ Modified: branches/Branch_3_2/Hibernate3/test/org/hibernate/test/hql/HQLTest.java =================================================================== --- branches/Branch_3_2/Hibernate3/test/org/hibernate/test/hql/HQLTest.java 2007-03-29 17:39:59 UTC (rev 11370) +++ branches/Branch_3_2/Hibernate3/test/org/hibernate/test/hql/HQLTest.java 2007-03-29 19:08:40 UTC (rev 11371) @@ -13,6 +13,7 @@ import junit.framework.Test; import org.hibernate.Hibernate; +import org.hibernate.QueryException; import org.hibernate.junit.functional.FunctionalTestClassTestSuite; import org.hibernate.dialect.DB2Dialect; import org.hibernate.dialect.HSQLDialect; @@ -34,6 +35,7 @@ import org.hibernate.hql.ast.tree.DotNode; import org.hibernate.hql.ast.tree.IndexNode; import org.hibernate.hql.ast.tree.SelectClause; +import org.hibernate.hql.ast.tree.FromReferenceNode; /** * Tests cases where the AST based query translator and the 'classic' query translator generate identical SQL. @@ -46,28 +48,44 @@ super( x ); } - protected boolean dropAfterFailure() { + public static Test suite() { + return new FunctionalTestClassTestSuite( HQLTest.class ); + } + + public boolean createSchema() { return false; } + public boolean recreateSchemaAfterFailure() { + return false; + } + protected void prepareTest() throws Exception { super.prepareTest(); SelectClause.VERSION2_SQL = true; DotNode.REGRESSION_STYLE_JOIN_SUPPRESSION = true; + DotNode.ILLEGAL_COLL_DEREF_EXCP_BUILDER = new DotNode.IllegalCollectionDereferenceExceptionBuilder() { + public QueryException buildIllegalCollectionDereferenceException(String propertyName, FromReferenceNode lhs) { + throw new QueryException( "illegal syntax near collection: " + propertyName ); + } + }; } - public static Test suite() { - return new FunctionalTestClassTestSuite( HQLTest.class ); - } - protected void cleanupTest() throws Exception { SelectClause.VERSION2_SQL = false; DotNode.REGRESSION_STYLE_JOIN_SUPPRESSION = false; + DotNode.ILLEGAL_COLL_DEREF_EXCP_BUILDER = DotNode.DEF_ILLEGAL_COLL_DEREF_EXCP_BUILDER; super.cleanupTest(); } //FAILING TESTS: + public void testInvalidCollectionDereferencesFail() { + // should fail with the same exceptions (because of the DotNode.ILLEGAL_COLL_DEREF_EXCP_BUILDER) + assertTranslation( "from Animal a where a.offspring.description = 'xyz'" ); + assertTranslation( "from Animal a where a.offspring.father.description = 'xyz'" ); + } + public void testSubComponentReferences() { assertTranslation( "select c.address.zip.code from ComponentContainer c" ); assertTranslation( "select c.address.zip from ComponentContainer c" );