From d3143860609b37aa628e6434af73f812cbeec188 Mon Sep 17 00:00:00 2001 From: Nadav Date: Fri, 26 Aug 2011 07:42:16 +0300 Subject: [PATCH 1/5] we can (now) transform it into IS NULL --- lib/Doctrine/ORM/EntityRepository.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/Doctrine/ORM/EntityRepository.php b/lib/Doctrine/ORM/EntityRepository.php index de2689db2..1aa2faef7 100644 --- a/lib/Doctrine/ORM/EntityRepository.php +++ b/lib/Doctrine/ORM/EntityRepository.php @@ -204,11 +204,6 @@ class EntityRepository implements ObjectRepository ); } - if ( !isset($arguments[0])) { - // we dont even want to allow null at this point, because we cannot (yet) transform it into IS NULL. - throw ORMException::findByRequiresParameter($method.$by); - } - $fieldName = lcfirst(\Doctrine\Common\Util\Inflector::classify($by)); if ($this->_class->hasField($fieldName) || $this->_class->hasAssociation($fieldName)) { From 5fc6277d3f2b469eb2e4260f7cb6aa0665287009 Mon Sep 17 00:00:00 2001 From: Nadav Date: Fri, 26 Aug 2011 07:51:29 +0300 Subject: [PATCH 2/5] Oops, shouldn't have removed the condition completely... checking a parameter is provided --- lib/Doctrine/ORM/EntityRepository.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/Doctrine/ORM/EntityRepository.php b/lib/Doctrine/ORM/EntityRepository.php index 1aa2faef7..3e1b83df6 100644 --- a/lib/Doctrine/ORM/EntityRepository.php +++ b/lib/Doctrine/ORM/EntityRepository.php @@ -203,6 +203,11 @@ class EntityRepository implements ObjectRepository "either findBy or findOneBy!" ); } + + + if (count($arguments) === 0) { + throw ORMException::findByRequiresParameter($method.$by); + } $fieldName = lcfirst(\Doctrine\Common\Util\Inflector::classify($by)); From 2e389e00d40dd68462ca170ac6ee3942297b412c Mon Sep 17 00:00:00 2001 From: Nadav Date: Fri, 26 Aug 2011 08:15:28 +0300 Subject: [PATCH 3/5] Removed blank line, used empty() instead of the count() check --- lib/Doctrine/ORM/EntityRepository.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/EntityRepository.php b/lib/Doctrine/ORM/EntityRepository.php index 3e1b83df6..28ba1f206 100644 --- a/lib/Doctrine/ORM/EntityRepository.php +++ b/lib/Doctrine/ORM/EntityRepository.php @@ -203,9 +203,8 @@ class EntityRepository implements ObjectRepository "either findBy or findOneBy!" ); } - - if (count($arguments) === 0) { + if (empty($arguments)) { throw ORMException::findByRequiresParameter($method.$by); } From 91bc9c0329aa7d8dfa76f07acb9c9038d2c80986 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 17 Oct 2011 18:54:20 +0200 Subject: [PATCH 4/5] Adjusted test to verify that findBy*(null) is now supported --- tests/Doctrine/Tests/Models/CMS/CmsUser.php | 2 +- .../ORM/Functional/EntityRepositoryTest.php | 32 ++++++++++++------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/tests/Doctrine/Tests/Models/CMS/CmsUser.php b/tests/Doctrine/Tests/Models/CMS/CmsUser.php index 0c7b007d1..3296fffc8 100644 --- a/tests/Doctrine/Tests/Models/CMS/CmsUser.php +++ b/tests/Doctrine/Tests/Models/CMS/CmsUser.php @@ -19,7 +19,7 @@ class CmsUser */ public $id; /** - * @Column(type="string", length=50) + * @Column(type="string", length=50, nullable=true) */ public $status; /** diff --git a/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php b/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php index c5216fb94..7cd37b906 100644 --- a/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php @@ -38,18 +38,25 @@ class EntityRepositoryTest extends \Doctrine\Tests\OrmFunctionalTestCase $user2->status = 'dev'; $this->_em->persist($user2); + $user3 = new CmsUser; + $user3->name = 'Benjamin'; + $user3->username = 'beberlei'; + $user3->status = null; + $this->_em->persist($user3); + $this->_em->flush(); - + $user1Id = $user->getId(); - + unset($user); unset($user2); - + unset($user3); + $this->_em->clear(); return $user1Id; } - + public function loadAssociatedFixture() { $address = new CmsAddress(); @@ -189,7 +196,7 @@ class EntityRepositoryTest extends \Doctrine\Tests\OrmFunctionalTestCase $repos = $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsUser'); $users = $repos->findAll(); - $this->assertEquals(2, count($users)); + $this->assertEquals(3, count($users)); } public function testFindByAlias() @@ -202,7 +209,7 @@ class EntityRepositoryTest extends \Doctrine\Tests\OrmFunctionalTestCase $repos = $this->_em->getRepository('CMS:CmsUser'); $users = $repos->findAll(); - $this->assertEquals(2, count($users)); + $this->assertEquals(3, count($users)); } /** @@ -284,10 +291,11 @@ class EntityRepositoryTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testFindMagicCallByNullValue() { $this->loadFixture(); + $repos = $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsUser'); - $this->setExpectedException('Doctrine\ORM\ORMException'); $users = $repos->findByStatus(null); + $this->assertEquals(1, count($users)); } /** @@ -411,7 +419,7 @@ class EntityRepositoryTest extends \Doctrine\Tests\OrmFunctionalTestCase $users1 = $repos->findBy(array(), null, 1, 0); $users2 = $repos->findBy(array(), null, 1, 1); - $this->assertEquals(2, count($repos->findBy(array()))); + $this->assertEquals(3, count($repos->findBy(array()))); $this->assertEquals(1, count($users1)); $this->assertEquals(1, count($users2)); $this->assertNotSame($users1[0], $users2[0]); @@ -428,10 +436,10 @@ class EntityRepositoryTest extends \Doctrine\Tests\OrmFunctionalTestCase $usersAsc = $repos->findBy(array(), array("username" => "ASC")); $usersDesc = $repos->findBy(array(), array("username" => "DESC")); - $this->assertEquals(2, count($usersAsc), "Pre-condition: only two users in fixture"); - $this->assertEquals(2, count($usersDesc), "Pre-condition: only two users in fixture"); - $this->assertSame($usersAsc[0], $usersDesc[1]); - $this->assertSame($usersAsc[1], $usersDesc[0]); + $this->assertEquals(3, count($usersAsc), "Pre-condition: only three users in fixture"); + $this->assertEquals(3, count($usersDesc), "Pre-condition: only three users in fixture"); + $this->assertSame($usersAsc[0], $usersDesc[2]); + $this->assertSame($usersAsc[2], $usersDesc[0]); } From b8af2415042dd5e3ab8518b8a3ab8775c8dab56a Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 17 Oct 2011 20:53:04 +0200 Subject: [PATCH 5/5] Added a testcase for findBy(.. => null) and renamed 'old' testcase --- .../Tests/ORM/Functional/EntityRepositoryTest.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php b/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php index 7cd37b906..4f8e11420 100644 --- a/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php @@ -397,7 +397,7 @@ class EntityRepositoryTest extends \Doctrine\Tests\OrmFunctionalTestCase /** * @group DDC-1087 */ - public function testIsNullCriteria() + public function testIsNullCriteriaDoesNotGenerateAParameter() { $repos = $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsUser'); $users = $repos->findBy(array('status' => null, 'username' => 'romanb')); @@ -407,6 +407,16 @@ class EntityRepositoryTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertEquals(array('romanb'), $params); } + public function testIsNullCriteria() + { + $this->loadFixture(); + + $repos = $this->_em->getRepository('Doctrine\Tests\Models\CMS\CmsUser'); + + $users = $repos->findBy(array('status' => null)); + $this->assertEquals(1, count($users)); + } + /** * @group DDC-1094 */