diff --git a/lib/Doctrine/ODM/PHPCR/DocumentRepository.php b/lib/Doctrine/ODM/PHPCR/DocumentRepository.php index a2ea908ef..c7289d588 100644 --- a/lib/Doctrine/ODM/PHPCR/DocumentRepository.php +++ b/lib/Doctrine/ODM/PHPCR/DocumentRepository.php @@ -146,23 +146,7 @@ public function findBy(array $criteria, array $orderBy = null, $limit = null, $o $orderByNode = $qb->orderBy(); if ($orderBy) { - $classMeta = $this->getClassMetadata(); foreach ($orderBy as $field => $order) { - if ($field === $classMeta->nodename) { - throw new InvalidArgumentException(sprintf( - 'It is not possible to order by a nodename property "%s->%s"', - $classMeta->name, - $field - )); - } - if ($classMeta->hasAssociation($field)) { - throw new InvalidArgumentException(sprintf( - 'It is not possible to order by association field "%s->%s"', - $classMeta->name, - $field - )); - } - $order = strtolower($order); if (!in_array($order, array('asc', 'desc'))) { throw new InvalidArgumentException(sprintf( @@ -202,16 +186,7 @@ public function findBy(array $criteria, array $orderBy = null, $limit = null, $o */ protected function constraintField(ConstraintFactory $where, $field, $value, $alias) { - $classMeta = $this->getClassMetadata(); - if ($classMeta->hasAssociation($field)) { - throw new InvalidArgumentException(sprintf( - 'It is not possible to filter on association fields %s->%s', - $classMeta->name, - $field - )); - } - - if ($field === $this->getClassMetadata()->nodename) { + if ($field === $this->class->nodename) { $where->eq()->name($alias)->literal($value); } else { $where->eq()->field($alias.'.'.$field)->literal($value); diff --git a/lib/Doctrine/ODM/PHPCR/Query/Builder/BuilderConverterPhpcr.php b/lib/Doctrine/ODM/PHPCR/Query/Builder/BuilderConverterPhpcr.php index 8ba5eceab..6133f0980 100644 --- a/lib/Doctrine/ODM/PHPCR/Query/Builder/BuilderConverterPhpcr.php +++ b/lib/Doctrine/ODM/PHPCR/Query/Builder/BuilderConverterPhpcr.php @@ -132,7 +132,6 @@ protected function getPhpcrProperty($originalAlias, $odmField) $this->validateAlias($originalAlias); $meta = $this->aliasMetadata[$originalAlias];; - if ($meta->hasField($odmField)) { $fieldMeta = $meta->getField($odmField); } elseif ($meta->hasAssociation($odmField)) { @@ -637,9 +636,30 @@ protected function walkConstraintNot(ConstraintNot $node) protected function walkOperandDynamicField(OperandDynamicField $node) { + $alias = $node->getAlias(); + $field = $node->getField(); + + $classMeta = $this->aliasMetadata[$alias]; + + if ($field === $classMeta->nodename) { + throw new InvalidArgumentException(sprintf( + 'Cannot use nodename property "%s->%s" in a field operand; use "localname()" instead.', + $classMeta->name, + $field + )); + } + + if ($classMeta->hasAssociation($field)) { + throw new InvalidArgumentException(sprintf( + 'Cannot use association property "%s->%s" in a field operand.', + $classMeta->name, + $field + )); + } + list($alias, $phpcrProperty) = $this->getPhpcrProperty( - $node->getAlias(), - $node->getField() + $alias, + $field ); $op = $this->qomf->propertyValue( @@ -751,6 +771,29 @@ protected function walkOrderBy(OrderBy $node) QBConstants::NT_OPERAND_DYNAMIC ); + if ($dynOp instanceof OperandDynamicField) { + $alias = $dynOp->getAlias(); + $field = $dynOp->getField(); + + $classMeta = $this->aliasMetadata[$alias]; + + if ($field === $classMeta->nodename) { + throw new InvalidArgumentException(sprintf( + 'It is not possible to order by a nodename property "%s->%s"', + $classMeta->name, + $field + )); + } + + if ($classMeta->hasAssociation($field)) { + throw new InvalidArgumentException(sprintf( + 'It is not possible to order by an association field "%s->%s"', + $classMeta->name, + $field + )); + } + } + $phpcrDynOp = $this->dispatch($dynOp); if ($ordering->getOrder() == QOMConstants::JCR_ORDER_ASCENDING) { diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Functional/DocumentRepositoryTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Functional/DocumentRepositoryTest.php index c12945e22..f37ec0ee1 100644 --- a/tests/Doctrine/Tests/ODM/PHPCR/Functional/DocumentRepositoryTest.php +++ b/tests/Doctrine/Tests/ODM/PHPCR/Functional/DocumentRepositoryTest.php @@ -153,17 +153,25 @@ public function testFindByOnNodename() $this->dm->persist($user); $this->dm->flush(); - $users = $this->dm->getRepository('Doctrine\Tests\Models\CMS\CmsTeamUser')->findBy(array('nodename' =>'beberlei')); + $users = $this->dm->getRepository('Doctrine\Tests\Models\CMS\CmsTeamUser')->findBy(array('nodename' => 'beberlei')); $this->assertCount(1, $users); $this->assertEquals($user->username, $users['/functional/lsmith/beberlei']->username); } + /** + * @expectedException \Doctrine\ODM\PHPCR\Exception\InvalidArgumentException + */ + public function testFindByOrderNonExistentDirectionString() + { + $this->dm->getRepository('Doctrine\Tests\Models\CMS\CmsTeamUser')->findBy(array('nodename' =>'beberlei'), array('username' =>'nowhere')); + } + /** * @expectedException \Doctrine\ODM\PHPCR\Exception\InvalidArgumentException */ public function testFindByOrderNodename() { - $this->dm->getRepository('Doctrine\Tests\Models\CMS\CmsTeamUser')->findBy(array('nodename' =>'beberlei'), array('nodename')); + $this->dm->getRepository('Doctrine\Tests\Models\CMS\CmsTeamUser')->findBy(array('nodename' =>'beberlei'), array('nodename' =>'asc')); } /** @@ -179,7 +187,7 @@ public function testFindByOnAssociation() */ public function testFindByOrderAssociation() { - $this->dm->getRepository('Doctrine\Tests\Models\CMS\CmsTeamUser')->findBy(array('nodename' =>'beberlei'), array('parent')); + $this->dm->getRepository('Doctrine\Tests\Models\CMS\CmsTeamUser')->findBy(array('username' =>'beberlei'), array('parent' => 'asc')); } public function testFindOneBy() diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Functional/QueryBuilderTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Functional/QueryBuilderTest.php index 5baf86bba..60daae82c 100644 --- a/tests/Doctrine/Tests/ODM/PHPCR/Functional/QueryBuilderTest.php +++ b/tests/Doctrine/Tests/ODM/PHPCR/Functional/QueryBuilderTest.php @@ -246,6 +246,19 @@ public function testOrderBy() $this->assertEquals('js', $res->first()->username); } + /** + * @depends testFrom + * @expectedException \Doctrine\ODM\PHPCR\Exception\InvalidArgumentException + */ + public function testOrderByNonSimpleField() + { + $qb = $this->createQb(); + $qb->from('a')->document('Doctrine\Tests\Models\Blog\User', 'a'); + $qb->orderBy()->asc()->localname('a.username')->end(); + + $qb->getQuery()->execute(); + } + /** * Removes jcr:primaryType from row values, * Jackrabbit does not return this, but doctrinedbal does. diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Query/Builder/BuilderConverterPhpcrTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Query/Builder/BuilderConverterPhpcrTest.php index 7d1e2c439..828e40227 100644 --- a/tests/Doctrine/Tests/ODM/PHPCR/Query/Builder/BuilderConverterPhpcrTest.php +++ b/tests/Doctrine/Tests/ODM/PHPCR/Query/Builder/BuilderConverterPhpcrTest.php @@ -63,6 +63,19 @@ public function setUp() ->method('getName') ->will($me->returnValue($documentFqn)); + $meta->expects($me->any()) + ->method('hasAssociation') + ->will($me->returnCallback(function ($field) { + if ($field === 'associationfield') { + return true; + } + + return false; + })); + + $meta->nodename = 'nodenameProperty'; + $meta->name = 'MyClassName'; + return $meta; })); @@ -480,7 +493,6 @@ public function provideTestDispatchOperands() 'phpcr_class' => 'LiteralInterface', )), ); - } /** @@ -547,6 +559,37 @@ public function testOrderBy() $this->assertCount(2, $res); } + public function provideOrderByDynamicField() + { + return array( + array('alias_1.ok_field', null), + array('alias_1.nodenameProperty', 'It is not possible to order by a nodename property "MyClassName->nodenameProperty"'), + array('alias_1.associationfield', 'It is not possible to order by an association field "MyClassName->associationfield"'), + ); + } + + /** + * @dataProvider provideOrderByDynamicField + */ + public function testOrderByDynamicField($field, $exception) + { + $this->primeBuilder(); + $order1 = $this->createNode('Ordering', array(QOMConstants::JCR_ORDER_ASCENDING)); + + $orderBy = $this->createNode('OrderBy', array()); + $orderBy->addChild($order1); + + $op = $this->createNode('OperandDynamicField', array($field)); + $order1->addChild($op); + + if (null !== $exception) { + $this->setExpectedException('Doctrine\ODM\PHPCR\Exception\InvalidArgumentException', $exception); + } + + $res = $this->converter->dispatch($orderBy); + $this->assertCount(1, $res); + } + public function testGetQuery() { $me = $this; @@ -622,7 +665,7 @@ public function testGetQuery() } /** - * @expectedException Doctrine\ODM\PHPCR\Exception\InvalidArgumentException + * @expectedException \Doctrine\ODM\PHPCR\Exception\InvalidArgumentException * @expectedExceptionMessage You must specify a primary alias */ public function testGetQueryMoreThanOneSourceNoPrimaryAlias()