From a816e5b9944ad0ad4bda4a77985f3262894770e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Wed, 11 Sep 2019 17:36:40 +0200 Subject: [PATCH] Refactoring retrieveManyToOneRelationshipsStorage signature This will allow us in the future to build a oneToMany handler for smart eager loading. --- src/AbstractTDBMObject.php | 17 ++++- .../SmartEagerLoad/OneToManyDataLoader.php | 69 +++++++++++++++++++ .../Query/ManyToOnePartialQuery.php | 3 +- src/Utils/BeanDescriptor.php | 22 +----- .../DirectForeignKeyMethodDescriptor.php | 30 +++----- src/Utils/Psr2Utils.php | 52 ++++++++++++++ tests/TDBMDaoGeneratorTest.php | 22 +++++- 7 files changed, 166 insertions(+), 49 deletions(-) create mode 100644 src/QueryFactory/SmartEagerLoad/OneToManyDataLoader.php create mode 100644 src/Utils/Psr2Utils.php diff --git a/src/AbstractTDBMObject.php b/src/AbstractTDBMObject.php index 36179c9a..8518338e 100644 --- a/src/AbstractTDBMObject.php +++ b/src/AbstractTDBMObject.php @@ -26,6 +26,7 @@ use TheCodingMachine\TDBM\QueryFactory\SmartEagerLoad\StorageNode; use TheCodingMachine\TDBM\Schema\ForeignKeys; use TheCodingMachine\TDBM\Utils\ManyToManyRelationshipPathDescriptor; +use function array_combine; /** * Instances of this class represent a "bean". Usually, a bean is mapped to a row of one table. @@ -531,12 +532,15 @@ private function removeManyToOneRelationship(string $tableName, string $foreignK * * @param string $tableName * @param string $foreignKeyName - * @param mixed[] $searchFilter - * @param string $orderString The ORDER BY part of the query. All columns must be prefixed by the table name (in the form: table.column). WARNING : This parameter is not kept when there is an additionnal or removal object ! + * @param array $localColumns + * @param array $foreignColumns + * @param string $foreignTableName + * @param string $orderString The ORDER BY part of the query. All columns must be prefixed by the table name (in the form: table.column). WARNING : This parameter is not kept when there is an additional or removal object ! * * @return AlterableResultIterator + * @throws TDBMException */ - protected function retrieveManyToOneRelationshipsStorage(string $tableName, string $foreignKeyName, array $searchFilter, string $orderString = null) : AlterableResultIterator + protected function retrieveManyToOneRelationshipsStorage(string $tableName, string $foreignKeyName, array $localColumns, array $foreignColumns, string $foreignTableName, string $orderString = null) : AlterableResultIterator { $key = $tableName.'___'.$foreignKeyName; $alterableResultIterator = $this->getManyToOneAlterableResultIterator($tableName, $foreignKeyName); @@ -544,6 +548,13 @@ protected function retrieveManyToOneRelationshipsStorage(string $tableName, stri return $alterableResultIterator; } + $ids = []; + foreach ($foreignColumns as $foreignColumn) { + $ids[] = $this->get($foreignColumn, $foreignTableName); + } + + $searchFilter = array_combine($localColumns, $ids); + $unalteredResultIterator = $this->tdbmService->findObjects($tableName, $searchFilter, [], $orderString); $alterableResultIterator->setResultIterator($unalteredResultIterator->getIterator()); diff --git a/src/QueryFactory/SmartEagerLoad/OneToManyDataLoader.php b/src/QueryFactory/SmartEagerLoad/OneToManyDataLoader.php new file mode 100644 index 00000000..c4d0bfa6 --- /dev/null +++ b/src/QueryFactory/SmartEagerLoad/OneToManyDataLoader.php @@ -0,0 +1,69 @@ +>> Array of rows, indexed by foreign key. + */ + private $data; + + public function __construct(Connection $connection, string $sql, string $fkColumn) + { + $this->connection = $connection; + $this->sql = $sql; + $this->fkColumn = $fkColumn; + } + + /** + * @return array> Rows, indexed by ID. + */ + private function load(): array + { + $results = $this->connection->fetchAll($this->sql); + + $data = []; + foreach ($results as $row) { + $data[$row[$this->fkColumn]][] = $row; + } + + return $data; + } + + /** + * Returns the DB row with the given ID. + * Loads all rows if necessary. + * Throws an exception if nothing found. + * + * @param string $id + * @return array + */ + public function get(string $id): array + { + if ($this->data === null) { + $this->data = $this->load(); + } + + if (!isset($this->data[$id])) { + return []; + } + return $this->data[$id]; + } +} diff --git a/src/QueryFactory/SmartEagerLoad/Query/ManyToOnePartialQuery.php b/src/QueryFactory/SmartEagerLoad/Query/ManyToOnePartialQuery.php index 8291a595..073d1ac4 100644 --- a/src/QueryFactory/SmartEagerLoad/Query/ManyToOnePartialQuery.php +++ b/src/QueryFactory/SmartEagerLoad/Query/ManyToOnePartialQuery.php @@ -38,10 +38,9 @@ class ManyToOnePartialQuery implements PartialQuery public function __construct(PartialQuery $partialQuery, string $originTableName, string $tableName, string $pk, string $columnName) { - // TODO: move this in a separate function. The constructor is called for every bean. $this->partialQuery = $partialQuery; $this->mainTable = $tableName; - $this->key = $partialQuery->getKey().'__'.$columnName; + $this->key = $partialQuery->getKey().'__mto__'.$columnName; $this->pk = $pk; $this->originTableName = $originTableName; $this->columnName = $columnName; diff --git a/src/Utils/BeanDescriptor.php b/src/Utils/BeanDescriptor.php index e908259c..2200b3ae 100644 --- a/src/Utils/BeanDescriptor.php +++ b/src/Utils/BeanDescriptor.php @@ -1596,7 +1596,7 @@ private function generateGetForeignKeys(array $fks): MethodGenerator } return parent::getForeignKeys(\$tableName); EOF; - $code = sprintf($code, var_export($this->getTable()->getName(), true), $this->psr2VarExport($fkArray, ' ')); + $code = sprintf($code, var_export($this->getTable()->getName(), true), Psr2Utils::psr2VarExport($fkArray, ' ')); $method = new MethodGenerator('getForeignKeys'); $method->setVisibility(AbstractMemberGenerator::VISIBILITY_PROTECTED); @@ -1613,24 +1613,4 @@ private function generateGetForeignKeys(array $fks): MethodGenerator return $method; } - - /** - * @param mixed $var - * @param string $indent - * @return string - */ - private function psr2VarExport($var, string $indent=''): string - { - if (is_array($var)) { - $indexed = array_keys($var) === range(0, count($var) - 1); - $r = []; - foreach ($var as $key => $value) { - $r[] = "$indent " - . ($indexed ? '' : $this->psr2VarExport($key) . ' => ') - . $this->psr2VarExport($value, "$indent "); - } - return "[\n" . implode(",\n", $r) . "\n" . $indent . ']'; - } - return var_export($var, true); - } } diff --git a/src/Utils/DirectForeignKeyMethodDescriptor.php b/src/Utils/DirectForeignKeyMethodDescriptor.php index 97c88778..c2816a37 100644 --- a/src/Utils/DirectForeignKeyMethodDescriptor.php +++ b/src/Utils/DirectForeignKeyMethodDescriptor.php @@ -14,6 +14,7 @@ use Zend\Code\Generator\AbstractMemberGenerator; use Zend\Code\Generator\DocBlock\Tag\ReturnTag; use Zend\Code\Generator\MethodGenerator; +use function var_export; /** * Represents a method to get a list of beans from a direct foreign key pointing to our bean. @@ -133,10 +134,12 @@ public function getCode() : array $getter->setReturnType('?' . $classType); $code = sprintf( - 'return $this->retrieveManyToOneRelationshipsStorage(%s, %s, %s)->first();', + 'return $this->retrieveManyToOneRelationshipsStorage(%s, %s, %s, %s, %s)->first();', var_export($this->foreignKey->getLocalTableName(), true), var_export($tdbmFk->getCacheKey(), true), - $this->getFilters($this->foreignKey) + Psr2Utils::psr2InlineVarExport($this->foreignKey->getUnquotedLocalColumns()), + Psr2Utils::psr2InlineVarExport($this->foreignKey->getUnquotedForeignColumns()), + var_export($this->foreignKey->getForeignTableName(), true) ); } else { $getter->setDocBlock(sprintf('Returns the list of %s pointing to this bean via the %s column.', $beanClass, implode(', ', $this->foreignKey->getUnquotedLocalColumns()))); @@ -147,10 +150,12 @@ public function getCode() : array $getter->setReturnType(AlterableResultIterator::class); $code = sprintf( - 'return $this->retrieveManyToOneRelationshipsStorage(%s, %s, %s);', + 'return $this->retrieveManyToOneRelationshipsStorage(%s, %s, %s, %s, %s);', var_export($this->foreignKey->getLocalTableName(), true), var_export($tdbmFk->getCacheKey(), true), - $this->getFilters($this->foreignKey) + Psr2Utils::psr2InlineVarExport($this->foreignKey->getUnquotedLocalColumns()), + Psr2Utils::psr2InlineVarExport($this->foreignKey->getUnquotedForeignColumns()), + var_export($this->foreignKey->getForeignTableName(), true) ); } @@ -163,23 +168,6 @@ public function getCode() : array return [ $getter ]; } - private function getFilters(ForeignKeyConstraint $fk) : string - { - $counter = 0; - $parameters = []; - - $fkForeignColumns = $fk->getUnquotedForeignColumns(); - - foreach ($fk->getUnquotedLocalColumns() as $columnName) { - $fkColumn = $fkForeignColumns[$counter]; - $parameters[] = sprintf('%s => $this->get(%s, %s)', var_export($fk->getLocalTableName().'.'.$columnName, true), var_export($fkColumn, true), var_export($this->foreignKey->getForeignTableName(), true)); - ++$counter; - } - $parametersCode = '['.implode(', ', $parameters).']'; - - return $parametersCode; - } - private $hasLocalUniqueIndex; /** * Check if the ForeignKey have an unique index diff --git a/src/Utils/Psr2Utils.php b/src/Utils/Psr2Utils.php new file mode 100644 index 00000000..4e4766d2 --- /dev/null +++ b/src/Utils/Psr2Utils.php @@ -0,0 +1,52 @@ + $value) { + $r[] = "$indent " + . ($indexed ? '' : self::psr2VarExport($key) . ' => ') + . self::psr2VarExport($value, "$indent "); + } + return "[\n" . implode(",\n", $r) . "\n" . $indent . ']'; + } + return var_export($var, true); + } + + /** + * @param mixed $var + * @return string + */ + public static function psr2InlineVarExport($var): string + { + if (is_array($var)) { + $indexed = array_keys($var) === range(0, count($var) - 1); + $r = []; + foreach ($var as $key => $value) { + $r[] = ($indexed ? '' : self::psr2InlineVarExport($key) . ' => ') + . self::psr2InlineVarExport($value); + } + return '[' . implode(',', $r) . ']'; + } + return var_export($var, true); + } +} diff --git a/tests/TDBMDaoGeneratorTest.php b/tests/TDBMDaoGeneratorTest.php index 6bae87b5..6cde5a8b 100644 --- a/tests/TDBMDaoGeneratorTest.php +++ b/tests/TDBMDaoGeneratorTest.php @@ -21,6 +21,7 @@ namespace TheCodingMachine\TDBM; +use Author; use Doctrine\Common\Cache\ArrayCache; use Doctrine\DBAL\Exception\ForeignKeyConstraintViolationException; use Doctrine\DBAL\Exception\UniqueConstraintViolationException; @@ -40,6 +41,7 @@ use TheCodingMachine\TDBM\Test\Dao\AlbumDao; use TheCodingMachine\TDBM\Test\Dao\AllNullableDao; use TheCodingMachine\TDBM\Test\Dao\AnimalDao; +use TheCodingMachine\TDBM\Test\Dao\ArticleDao; use TheCodingMachine\TDBM\Test\Dao\ArtistDao; use TheCodingMachine\TDBM\Test\Dao\BaseObjectDao; use TheCodingMachine\TDBM\Test\Dao\Bean\AccountBean; @@ -2229,7 +2231,7 @@ public function testManyToOneEagerLoading(): void $countryIds[] = $user->getCountry()->getId(); } - $this->assertFalse($users->getIterator()->hasManyToOneDataLoader('__country_id')); + $this->assertFalse($users->getIterator()->hasManyToOneDataLoader('__mto__country_id')); $this->assertSame([2, 1, 3, 2, 2, 4], $countryIds); $countryNames = []; @@ -2237,10 +2239,26 @@ public function testManyToOneEagerLoading(): void $countryNames[] = $user->getCountry()->getLabel(); } - $this->assertTrue($users->getIterator()->hasManyToOneDataLoader('__country_id')); + $this->assertTrue($users->getIterator()->hasManyToOneDataLoader('__mto__country_id')); $this->assertSame(['UK', 'France', 'Jamaica', 'UK', 'UK', 'Mexico'], $countryNames); } + /** + * @depends testDaoGeneration + */ + public function testManyToOneEagerLoadingOnTableWithInheritance(): void + { + $articleDao = new ArticleDao($this->tdbmService); + /** @var ArticleBean[] $articles */ + $articles = $articleDao->findAll()->withOrder('id asc'); + $names = []; + foreach ($articles as $article) { + $names[] = $article->getAuthor()->getName(); + } + $this->assertCount(1, $names); + $this->assertSame('Bill Shakespeare', $names[0]); + } + /** * @depends testDaoGeneration */