From 2416752362ccdf9e3d09e1aee62f7bff66434acb Mon Sep 17 00:00:00 2001 From: Lukas Kahwe Smith Date: Fri, 29 Aug 2014 11:36:52 +0200 Subject: [PATCH 1/5] fixed tests --- .../PHPCR/Functional/DocumentRepositoryTest.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) 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() From a56b8307f21b32c3900f3d6348283a0c5ec724ee Mon Sep 17 00:00:00 2001 From: Lukas Kahwe Smith Date: Fri, 29 Aug 2014 11:40:46 +0200 Subject: [PATCH 2/5] typo fix --- .../Tests/ODM/PHPCR/Query/Builder/BuilderConverterPhpcrTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Query/Builder/BuilderConverterPhpcrTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Query/Builder/BuilderConverterPhpcrTest.php index 7d1e2c439..8bad71d97 100644 --- a/tests/Doctrine/Tests/ODM/PHPCR/Query/Builder/BuilderConverterPhpcrTest.php +++ b/tests/Doctrine/Tests/ODM/PHPCR/Query/Builder/BuilderConverterPhpcrTest.php @@ -622,7 +622,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() From 5fa4e2cf0e1a7704c3b03a7c3dfc94407dce0665 Mon Sep 17 00:00:00 2001 From: Lukas Kahwe Smith Date: Fri, 29 Aug 2014 11:40:50 +0200 Subject: [PATCH 3/5] pushed field/orderby validation into the QB --- lib/Doctrine/ODM/PHPCR/DocumentRepository.php | 27 +------------ .../Query/Builder/BuilderConverterPhpcr.php | 38 +++++++++++++++++-- 2 files changed, 36 insertions(+), 29 deletions(-) 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..dafbe387d 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,21 @@ protected function walkConstraintNot(ConstraintNot $node) protected function walkOperandDynamicField(OperandDynamicField $node) { + $alias = $node->getAlias(); + $field = $node->getField(); + + $classMeta = $this->aliasMetadata[$alias]; + if ($classMeta->hasAssociation($field)) { + throw new InvalidArgumentException(sprintf( + 'It is not possible to filter on association fields %s->%s', + $classMeta->name, + $field + )); + } + list($alias, $phpcrProperty) = $this->getPhpcrProperty( - $node->getAlias(), - $node->getField() + $alias, + $field ); $op = $this->qomf->propertyValue( @@ -747,6 +758,27 @@ protected function walkOrderBy(OrderBy $node) /** @var $ordering Ordering */ foreach ($orderings as $ordering) { + $node = $ordering->getChild(); + $alias = $node->getAlias(); + $field = $node->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 association field "%s->%s"', + $classMeta->name, + $field + )); + } + $dynOp = $ordering->getChildOfType( QBConstants::NT_OPERAND_DYNAMIC ); From e0ad9f018c58c91bb8342684a732d763a3362699 Mon Sep 17 00:00:00 2001 From: dantleech Date: Sat, 30 Aug 2014 21:28:38 +0200 Subject: [PATCH 4/5] Fixed validation --- .../Query/Builder/BuilderConverterPhpcr.php | 29 +++++------- .../Builder/BuilderConverterPhpcrTest.php | 45 ++++++++++++++++++- 2 files changed, 54 insertions(+), 20 deletions(-) diff --git a/lib/Doctrine/ODM/PHPCR/Query/Builder/BuilderConverterPhpcr.php b/lib/Doctrine/ODM/PHPCR/Query/Builder/BuilderConverterPhpcr.php index dafbe387d..4313e1f71 100644 --- a/lib/Doctrine/ODM/PHPCR/Query/Builder/BuilderConverterPhpcr.php +++ b/lib/Doctrine/ODM/PHPCR/Query/Builder/BuilderConverterPhpcr.php @@ -640,9 +640,18 @@ protected function walkOperandDynamicField(OperandDynamicField $node) $field = $node->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 filter on association fields %s->%s', + 'It is not possible to order by association field "%s->%s"', $classMeta->name, $field )); @@ -760,24 +769,6 @@ protected function walkOrderBy(OrderBy $node) foreach ($orderings as $ordering) { $node = $ordering->getChild(); $alias = $node->getAlias(); - $field = $node->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 association field "%s->%s"', - $classMeta->name, - $field - )); - } $dynOp = $ordering->getChildOfType( QBConstants::NT_OPERAND_DYNAMIC diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Query/Builder/BuilderConverterPhpcrTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Query/Builder/BuilderConverterPhpcrTest.php index 8bad71d97..9575f995b 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 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; From 9bab892f98abe2a7fe6639fe75a6d32848b04263 Mon Sep 17 00:00:00 2001 From: Lukas Kahwe Smith Date: Mon, 1 Sep 2014 09:35:28 +0200 Subject: [PATCH 5/5] specifically handle order by error cases in the QB --- .../Query/Builder/BuilderConverterPhpcr.php | 27 +++++++++++++++++-- .../ODM/PHPCR/Functional/QueryBuilderTest.php | 13 +++++++++ .../Builder/BuilderConverterPhpcrTest.php | 2 +- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ODM/PHPCR/Query/Builder/BuilderConverterPhpcr.php b/lib/Doctrine/ODM/PHPCR/Query/Builder/BuilderConverterPhpcr.php index 036fb8608..6133f0980 100644 --- a/lib/Doctrine/ODM/PHPCR/Query/Builder/BuilderConverterPhpcr.php +++ b/lib/Doctrine/ODM/PHPCR/Query/Builder/BuilderConverterPhpcr.php @@ -643,7 +643,7 @@ protected function walkOperandDynamicField(OperandDynamicField $node) if ($field === $classMeta->nodename) { throw new InvalidArgumentException(sprintf( - 'It is not possible to order by a nodename property "%s->%s"', + 'Cannot use nodename property "%s->%s" in a field operand; use "localname()" instead.', $classMeta->name, $field )); @@ -651,7 +651,7 @@ protected function walkOperandDynamicField(OperandDynamicField $node) if ($classMeta->hasAssociation($field)) { throw new InvalidArgumentException(sprintf( - 'It is not possible to order by association field "%s->%s"', + 'Cannot use association property "%s->%s" in a field operand.', $classMeta->name, $field )); @@ -771,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/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 9575f995b..828e40227 100644 --- a/tests/Doctrine/Tests/ODM/PHPCR/Query/Builder/BuilderConverterPhpcrTest.php +++ b/tests/Doctrine/Tests/ODM/PHPCR/Query/Builder/BuilderConverterPhpcrTest.php @@ -564,7 +564,7 @@ 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 association field "MyClassName->associationfield"'), + array('alias_1.associationfield', 'It is not possible to order by an association field "MyClassName->associationfield"'), ); }