From 9768d08458f94a8496d864734fe1b188329804ca Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 5 Feb 2011 11:42:10 +0100 Subject: [PATCH] [DDC-250] Add tests and fix some glitches and finalized index-by patch. --- doctrine-mapping.xsd | 2 + .../ORM/Mapping/ClassMetadataInfo.php | 2 +- .../ORM/Persisters/BasicEntityPersister.php | 4 +- lib/Doctrine/ORM/Query/Parser.php | 4 + lib/Doctrine/ORM/Query/SqlWalker.php | 18 +-- .../Tests/Models/StockExchange/Bond.php | 48 ++++++++ .../Tests/Models/StockExchange/Market.php | 56 ++++++++++ .../Tests/Models/StockExchange/Stock.php | 49 ++++++++ .../Tests/ORM/Functional/AllTests.php | 1 + .../ORM/Functional/IndexByAssociationTest.php | 105 ++++++++++++++++++ .../Doctrine/Tests/OrmFunctionalTestCase.php | 11 ++ 11 files changed, 289 insertions(+), 11 deletions(-) create mode 100644 tests/Doctrine/Tests/Models/StockExchange/Bond.php create mode 100644 tests/Doctrine/Tests/Models/StockExchange/Market.php create mode 100644 tests/Doctrine/Tests/Models/StockExchange/Stock.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/IndexByAssociationTest.php diff --git a/doctrine-mapping.xsd b/doctrine-mapping.xsd index f1f8db814..b8e4de1a7 100644 --- a/doctrine-mapping.xsd +++ b/doctrine-mapping.xsd @@ -257,6 +257,7 @@ + @@ -269,6 +270,7 @@ + diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 1ec61b666..5e96cc4c1 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -723,7 +723,7 @@ class ClassMetadataInfo $mapping['isOwningSide'] = true; // assume owning side until we hit mappedBy // unset optional indexBy attribute if its empty - if (isset($mapping['indexBy']) && !$mapping['indexBy']) { + if (!isset($mapping['indexBy']) || !$mapping['indexBy']) { unset($mapping['indexBy']); } diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 0cfd5e8cd..56e91e49a 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -763,7 +763,7 @@ class BasicEntityPersister private function loadArrayFromStatement($assoc, $stmt) { $entities = array(); - if ($assoc['indexBy']) { + if (isset($assoc['indexBy'])) { while ($result = $stmt->fetch(PDO::FETCH_ASSOC)) { $entity = $this->_createEntity($result); $entities[$this->_class->reflFields[$assoc['indexBy']]->getValue($entity)] = $entity; @@ -786,7 +786,7 @@ class BasicEntityPersister */ private function loadCollectionFromStatement($assoc, $stmt, $coll) { - if ($assoc['indexBy']) { + if (isset($assoc['indexBy'])) { while ($result = $stmt->fetch(PDO::FETCH_ASSOC)) { $entity = $this->_createEntity($result); $coll->hydrateSet($this->_class->reflFields[$assoc['indexBy']]->getValue($entity), $entity); diff --git a/lib/Doctrine/ORM/Query/Parser.php b/lib/Doctrine/ORM/Query/Parser.php index 0c22c9fbf..63351a89a 100644 --- a/lib/Doctrine/ORM/Query/Parser.php +++ b/lib/Doctrine/ORM/Query/Parser.php @@ -923,6 +923,10 @@ class Parser $token = $this->_lexer->lookahead; $identVariable = $this->IdentificationVariable(); + if (!isset($this->_queryComponents[$identVariable])) { + $this->semanticalError('Identification Variable ' . $identVariable .' used in join path expression but was not defined before.'); + } + $this->match(Lexer::T_DOT); $this->match(Lexer::T_IDENTIFIER); diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index ebbefbce8..202170c23 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -718,14 +718,6 @@ class SqlWalker implements TreeWalker $join = $joinVarDecl->join; $joinType = $join->joinType; - if ($joinVarDecl->indexBy) { - // For Many-To-One or One-To-One associations this obviously makes no sense, but is ignored silently. - $this->_rsm->addIndexBy( - $joinVarDecl->indexBy->simpleStateFieldPathExpression->identificationVariable, - $joinVarDecl->indexBy->simpleStateFieldPathExpression->field - ); - } - if ($joinType == AST\Join::JOIN_TYPE_LEFT || $joinType == AST\Join::JOIN_TYPE_LEFTOUTER) { $sql = ' LEFT JOIN '; } else { @@ -750,6 +742,16 @@ class SqlWalker implements TreeWalker } } + if ($joinVarDecl->indexBy) { + // For Many-To-One or One-To-One associations this obviously makes no sense, but is ignored silently. + $this->_rsm->addIndexBy( + $joinVarDecl->indexBy->simpleStateFieldPathExpression->identificationVariable, + $joinVarDecl->indexBy->simpleStateFieldPathExpression->field + ); + } else if (isset($relation['indexBy'])) { + $this->_rsm->addIndexBy($joinedDqlAlias, $relation['indexBy']); + } + // This condition is not checking ClassMetadata::MANY_TO_ONE, because by definition it cannot // be the owning side and previously we ensured that $assoc is always the owning side of the associations. // The owning side is necessary at this point because only it contains the JoinColumn information. diff --git a/tests/Doctrine/Tests/Models/StockExchange/Bond.php b/tests/Doctrine/Tests/Models/StockExchange/Bond.php new file mode 100644 index 000000000..c8d661782 --- /dev/null +++ b/tests/Doctrine/Tests/Models/StockExchange/Bond.php @@ -0,0 +1,48 @@ +name = $name; + } + + public function getId() + { + return $this->id; + } + + public function addStock(Stock $stock) + { + $this->stocks[$stock->getSymbol()] = $stock; + } +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/Models/StockExchange/Market.php b/tests/Doctrine/Tests/Models/StockExchange/Market.php new file mode 100644 index 000000000..87e12cab6 --- /dev/null +++ b/tests/Doctrine/Tests/Models/StockExchange/Market.php @@ -0,0 +1,56 @@ +name = $name; + $this->stocks = new ArrayCollection(); + } + + public function getId() + { + return $this->id; + } + + public function getName() + { + return $this->name; + } + + public function addStock(Stock $stock) + { + $this->stocks[$stock->getSymbol()] = $stock; + } + + public function getStock($symbol) + { + return $this->stocks[$symbol]; + } +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/Models/StockExchange/Stock.php b/tests/Doctrine/Tests/Models/StockExchange/Stock.php new file mode 100644 index 000000000..d65675be9 --- /dev/null +++ b/tests/Doctrine/Tests/Models/StockExchange/Stock.php @@ -0,0 +1,49 @@ +symbol = $symbol; + $this->price = $initialOfferingPrice; + $this->market = $market; + $market->addStock($this); + } + + public function getSymbol() + { + return $this->symbol; + } +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Functional/AllTests.php b/tests/Doctrine/Tests/ORM/Functional/AllTests.php index 0759bcf7f..319d5bb50 100644 --- a/tests/Doctrine/Tests/ORM/Functional/AllTests.php +++ b/tests/Doctrine/Tests/ORM/Functional/AllTests.php @@ -45,6 +45,7 @@ class AllTests $suite->addTestSuite('Doctrine\Tests\ORM\Functional\ManyToManySelfReferentialAssociationTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\OrderedCollectionTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\OrderedJoinedTableInheritanceCollectionTest'); + $suite->addTestSuite('Doctrine\Tests\ORM\Functional\IndexByAssociationTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\CompositePrimaryKeyTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\ReferenceProxyTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\LifecycleCallbackTest'); diff --git a/tests/Doctrine/Tests/ORM/Functional/IndexByAssociationTest.php b/tests/Doctrine/Tests/ORM/Functional/IndexByAssociationTest.php new file mode 100644 index 000000000..bca4ea5df --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/IndexByAssociationTest.php @@ -0,0 +1,105 @@ +useModelSet('stockexchange'); + parent::setUp(); + $this->loadFixture(); + } + + public function loadFixture() + { + $this->market = new Market("Some Exchange"); + $stock1 = new Stock("AAPL", 10, $this->market); + $stock2 = new Stock("GOOG", 20, $this->market); + + $this->bond = new Bond("MyBond"); + $this->bond->addStock($stock1); + $this->bond->addStock($stock2); + + $this->_em->persist($this->market); + $this->_em->persist($stock1); + $this->_em->persist($stock2); + $this->_em->persist($this->bond); + $this->_em->flush(); + $this->_em->clear(); + } + + public function testManyToOneFinder() + { + /* @var $market Doctrine\Tests\Models\StockExchange\Market */ + $market = $this->_em->find('Doctrine\Tests\Models\StockExchange\Market', $this->market->getId()); + + $this->assertEquals(2, count($market->stocks)); + $this->assertTrue(isset($market->stocks['AAPL']), "AAPL symbol has to be key in indexed assocation."); + $this->assertTrue(isset($market->stocks['GOOG']), "GOOG symbol has to be key in indexed assocation."); + $this->assertEquals("AAPL", $market->stocks['AAPL']->getSymbol()); + $this->assertEquals("GOOG", $market->stocks['GOOG']->getSymbol()); + } + + public function testManyToOneDQL() + { + $dql = "SELECT m, s FROM Doctrine\Tests\Models\StockExchange\Market m JOIN m.stocks s WHERE m.id = ?1"; + $market = $this->_em->createQuery($dql)->setParameter(1, $this->market->getId())->getSingleResult(); + + $this->assertEquals(2, count($market->stocks)); + $this->assertTrue(isset($market->stocks['AAPL']), "AAPL symbol has to be key in indexed assocation."); + $this->assertTrue(isset($market->stocks['GOOG']), "GOOG symbol has to be key in indexed assocation."); + $this->assertEquals("AAPL", $market->stocks['AAPL']->getSymbol()); + $this->assertEquals("GOOG", $market->stocks['GOOG']->getSymbol()); + } + + public function testManyToMany() + { + $bond = $this->_em->find('Doctrine\Tests\Models\StockExchange\Bond', $this->bond->getId()); + + $this->assertEquals(2, count($bond->stocks)); + $this->assertTrue(isset($bond->stocks['AAPL']), "AAPL symbol has to be key in indexed assocation."); + $this->assertTrue(isset($bond->stocks['GOOG']), "GOOG symbol has to be key in indexed assocation."); + $this->assertEquals("AAPL", $bond->stocks['AAPL']->getSymbol()); + $this->assertEquals("GOOG", $bond->stocks['GOOG']->getSymbol()); + } + + public function testManytoManyDQL() + { + $dql = "SELECT b, s FROM Doctrine\Tests\Models\StockExchange\Bond b JOIN b.stocks s WHERE b.id = ?1"; + $bond = $this->_em->createQuery($dql)->setParameter(1, $this->bond->getId())->getSingleResult(); + + $this->assertEquals(2, count($bond->stocks)); + $this->assertTrue(isset($bond->stocks['AAPL']), "AAPL symbol has to be key in indexed assocation."); + $this->assertTrue(isset($bond->stocks['GOOG']), "GOOG symbol has to be key in indexed assocation."); + $this->assertEquals("AAPL", $bond->stocks['AAPL']->getSymbol()); + $this->assertEquals("GOOG", $bond->stocks['GOOG']->getSymbol()); + } + + public function testDqlOverrideIndexBy() + { + $dql = "SELECT b, s FROM Doctrine\Tests\Models\StockExchange\Bond b JOIN b.stocks s INDEX BY s.id WHERE b.id = ?1"; + $bond = $this->_em->createQuery($dql)->setParameter(1, $this->bond->getId())->getSingleResult(); + + $this->assertEquals(2, count($bond->stocks)); + $this->assertFalse(isset($bond->stocks['AAPL']), "AAPL symbol not exists in re-indexed assocation."); + $this->assertFalse(isset($bond->stocks['GOOG']), "GOOG symbol not exists in re-indexed assocation."); + } +} + diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index d9dd9bc77..166cdfb6a 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -99,6 +99,11 @@ abstract class OrmFunctionalTestCase extends OrmTestCase 'Doctrine\Tests\Models\DDC117\DDC117ApproveChanges', 'Doctrine\Tests\Models\DDC117\DDC117Editor', ), + 'stockexchange' => array( + 'Doctrine\Tests\Models\StockExchange\Bond', + 'Doctrine\Tests\Models\StockExchange\Stock', + 'Doctrine\Tests\Models\StockExchange\Market', + ), ); protected function useModelSet($setName) @@ -191,6 +196,12 @@ abstract class OrmFunctionalTestCase extends OrmTestCase $conn->executeUpdate('DELETE FROM DDC117Translation'); $conn->executeUpdate('DELETE FROM DDC117Article'); } + if (isset($this->_usedModelSets['stockexchange'])) { + $conn->executeUpdate('DELETE FROM exchange_bonds_stocks'); + $conn->executeUpdate('DELETE FROM exchange_bonds'); + $conn->executeUpdate('DELETE FROM exchange_stocks'); + $conn->executeUpdate('DELETE FROM exchange_markets'); + } $this->_em->clear(); }