From c4f558af6948c585e52089d9877f936d55228b83 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Sat, 9 Aug 2014 09:29:14 +0200 Subject: [PATCH] validate findBy arguments to not be about association mappings. --- CHANGELOG.md | 2 + lib/Doctrine/ODM/PHPCR/DocumentRepository.php | 27 +++++++++--- .../ODM/PHPCR/DocumentRepositoryTest.php | 43 ------------------ .../Functional/DocumentRepositoryTest.php | 44 ++++++++++++++++++- 4 files changed, 67 insertions(+), 49 deletions(-) delete mode 100644 tests/Doctrine/Tests/ODM/PHPCR/DocumentRepositoryTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index d45644444..32b4574e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ Changelog * **2014-08-07**: properly handle fields mapped to nodename in DocumentRepository::findBy() + Report error when trying to find or order by association fields, instead of building + queries that won't work. 1.2.0-RC1 --------- diff --git a/lib/Doctrine/ODM/PHPCR/DocumentRepository.php b/lib/Doctrine/ODM/PHPCR/DocumentRepository.php index 6ec6facbd..a2ea908ef 100644 --- a/lib/Doctrine/ODM/PHPCR/DocumentRepository.php +++ b/lib/Doctrine/ODM/PHPCR/DocumentRepository.php @@ -28,7 +28,6 @@ use Doctrine\ODM\PHPCR\Query\Builder\ConstraintFactory; - /** * A DocumentRepository serves as a repository for documents with generic as well as * business specific methods for retrieving documents. @@ -147,10 +146,19 @@ 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 === $this->getClassMetadata()->nodename) { + 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 a nodename property "%s"', + 'It is not possible to order by association field "%s->%s"', + $classMeta->name, $field )); } @@ -186,14 +194,23 @@ public function findBy(array $criteria, array $orderBy = null, $limit = null, $o /** * Constraints a field for a given value - * + * * @param ConstraintFactory $where * @param string $field The field searched * @param mixed $value The value to search for - * @param string The alias used + * @param string $alias The alias used */ 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) { $where->eq()->name($alias)->literal($value); } else { diff --git a/tests/Doctrine/Tests/ODM/PHPCR/DocumentRepositoryTest.php b/tests/Doctrine/Tests/ODM/PHPCR/DocumentRepositoryTest.php deleted file mode 100644 index 644893d55..000000000 --- a/tests/Doctrine/Tests/ODM/PHPCR/DocumentRepositoryTest.php +++ /dev/null @@ -1,43 +0,0 @@ -dm = $this->createDocumentManager(); - $this->metadata = new ClassMetadata('Doctrine\Tests\Models\CMS\CmsUser'); - } - - public function testCreateQueryBuilder() - { - $rep = new DocumentRepository($this->dm, $this->metadata); - $qb = $rep->createQueryBuilder('a'); - - $from = $qb->getChildOfType(QBConstants::NT_FROM); - $this->assertInstanceOf('Doctrine\ODM\PHPCR\Query\Builder\From', $from); - $source = $from->getChildOfType(QBConstants::NT_SOURCE); - $this->assertInstanceOf('Doctrine\ODM\PHPCR\Query\Builder\SourceDocument', $source); - - $this->assertEquals('a', $source->getAlias()); - $this->assertEquals('a', $qb->getPrimaryAlias()); - - $this->assertEquals('Doctrine\Tests\Models\CMS\CmsUser', $source->getDocumentFqn()); - } - - public function testFindBy() - { - $rep = new DocumentRepository($this->dm, $this->metadata); - $res = $rep->findBy(array()); - $this->assertInstanceOf('Doctrine\Common\Collections\ArrayCollection', $res); - } -} diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Functional/DocumentRepositoryTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Functional/DocumentRepositoryTest.php index a2fa6000e..c12945e22 100644 --- a/tests/Doctrine/Tests/ODM/PHPCR/Functional/DocumentRepositoryTest.php +++ b/tests/Doctrine/Tests/ODM/PHPCR/Functional/DocumentRepositoryTest.php @@ -4,11 +4,13 @@ use Doctrine\Tests\Models\CMS\CmsTeamUser; use Doctrine\Tests\Models\CMS\CmsUser; +use Doctrine\Tests\ODM\PHPCR\PHPCRFunctionalTestCase; +use Doctrine\ODM\PHPCR\Query\Builder\AbstractNode as QBConstants; /** * @group functional */ -class DocumentRepositoryTest extends \Doctrine\Tests\ODM\PHPCR\PHPCRFunctionalTestCase +class DocumentRepositoryTest extends PHPCRFunctionalTestCase { /** * @var \Doctrine\ODM\PHPCR\DocumentManager @@ -40,6 +42,22 @@ public function setUp() $session->save(); } + public function testCreateQueryBuilder() + { + $rep = $this->dm->getRepository('Doctrine\Tests\Models\CMS\CmsUser'); + $qb = $rep->createQueryBuilder('a'); + + $from = $qb->getChildOfType(QBConstants::NT_FROM); + $this->assertInstanceOf('Doctrine\ODM\PHPCR\Query\Builder\From', $from); + $source = $from->getChildOfType(QBConstants::NT_SOURCE); + $this->assertInstanceOf('Doctrine\ODM\PHPCR\Query\Builder\SourceDocument', $source); + + $this->assertEquals('a', $source->getAlias()); + $this->assertEquals('a', $qb->getPrimaryAlias()); + + $this->assertEquals('Doctrine\Tests\Models\CMS\CmsUser', $source->getDocumentFqn()); + } + public function testLoadMany() { $user1 = new CmsUser(); @@ -140,6 +158,30 @@ public function testFindByOnNodename() $this->assertEquals($user->username, $users['/functional/lsmith/beberlei']->username); } + /** + * @expectedException \Doctrine\ODM\PHPCR\Exception\InvalidArgumentException + */ + public function testFindByOrderNodename() + { + $this->dm->getRepository('Doctrine\Tests\Models\CMS\CmsTeamUser')->findBy(array('nodename' =>'beberlei'), array('nodename')); + } + + /** + * @expectedException \Doctrine\ODM\PHPCR\Exception\InvalidArgumentException + */ + public function testFindByOnAssociation() + { + $this->dm->getRepository('Doctrine\Tests\Models\CMS\CmsTeamUser')->findBy(array('parent' =>'/foo')); + } + + /** + * @expectedException \Doctrine\ODM\PHPCR\Exception\InvalidArgumentException + */ + public function testFindByOrderAssociation() + { + $this->dm->getRepository('Doctrine\Tests\Models\CMS\CmsTeamUser')->findBy(array('nodename' =>'beberlei'), array('parent')); + } + public function testFindOneBy() { $user1 = new CmsUser();