From 7a1909916a54516ab516c796c38588ca77ae706d Mon Sep 17 00:00:00 2001 From: Guillaume Date: Mon, 23 Aug 2021 11:51:38 +0200 Subject: [PATCH 1/5] ManyToMany: Properly escape table / column names --- tests/TDBMAbstractServiceTest.php | 16 ++++++++++++++++ tests/TDBMDaoGeneratorTest.php | 17 ++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/tests/TDBMAbstractServiceTest.php b/tests/TDBMAbstractServiceTest.php index 86e5ff80..86fc84b3 100644 --- a/tests/TDBMAbstractServiceTest.php +++ b/tests/TDBMAbstractServiceTest.php @@ -431,6 +431,18 @@ private static function initSchema(Connection $connection): void ->column('UPPERCASE_B_ID')->references('UPPERCASE_B'); } + // Reserved keywords from https://dev.mysql.com/doc/refman/8.0/en/keywords.html + // and https://docs.oracle.com/cd/B19306_01/em.102/b40103/app_oracle_reserved_words.htm + $db->table('values') + ->column('key')->integer()->primaryKey()->autoIncrement(); + + $db->table('accessible') + ->column('add')->integer()->primaryKey()->autoIncrement(); + + $db->table('all') + ->column('analyze')->references('values') + ->column('and')->references('accessible'); + $sqlStmts = $toSchema->getMigrateFromSql($fromSchema, $connection->getDatabasePlatform()); foreach ($sqlStmts as $sqlStmt) { @@ -779,6 +791,10 @@ private static function initSchema(Connection $connection): void 'fk_1' => 1, 'fk_2' => 1 ]); + + self::insert($connection, 'values', ['key' => 1]); + self::insert($connection, 'accessible', ['add' => 1]); + self::insert($connection, 'all', ['analyze' => 1, 'and' => 1]); } public static function insert(Connection $connection, string $tableName, array $data): void diff --git a/tests/TDBMDaoGeneratorTest.php b/tests/TDBMDaoGeneratorTest.php index d041c718..20e8a98b 100644 --- a/tests/TDBMDaoGeneratorTest.php +++ b/tests/TDBMDaoGeneratorTest.php @@ -41,6 +41,7 @@ use TheCodingMachine\TDBM\Dao\TestUserDao; use TheCodingMachine\TDBM\Fixtures\Interfaces\TestUserDaoInterface; use TheCodingMachine\TDBM\Fixtures\Interfaces\TestUserInterface; +use TheCodingMachine\TDBM\Test\Dao\AccessibleDao; use TheCodingMachine\TDBM\Test\Dao\AlbumDao; use TheCodingMachine\TDBM\Test\Dao\AllNullableDao; use TheCodingMachine\TDBM\Test\Dao\AnimalDao; @@ -89,6 +90,7 @@ use TheCodingMachine\TDBM\Test\Dao\RoleDao; use TheCodingMachine\TDBM\Test\Dao\StateDao; use TheCodingMachine\TDBM\Test\Dao\UserDao; +use TheCodingMachine\TDBM\Test\Dao\ValueDao; use TheCodingMachine\TDBM\Utils\PathFinder\NoPathFoundException; use TheCodingMachine\TDBM\Utils\PathFinder\PathFinder; use TheCodingMachine\TDBM\Utils\TDBMDaoGenerator; @@ -2348,7 +2350,20 @@ public function testCanReadVirtualColumn(): void $this->assertSame('Sally', $player->getNamesVirtual()); } - private function skipOracle() + public function testPivotTableAreProperlyEscaped(): void + { + $valueDao = new ValueDao($this->tdbmService); + $accessibleDao = new AccessibleDao($this->tdbmService); + + $value = $valueDao->getById(1); + $accessible = $accessibleDao->getById(1); + $this->assertSame(1, $value->getKey()); + $this->assertSame(1, $accessible->getAdd()); + $this->assertCount(1, $value->getAccessible()); + $this->assertCount(1, $accessible->getValues()); + } + + private function skipOracle(): void { if (self::getConnection()->getDatabasePlatform() instanceof OraclePlatform) { $this->markTestSkipped('Not supported in Oracle'); From 6d2eefc7e769934d374a336a390e8089778e7896 Mon Sep 17 00:00:00 2001 From: Guillaume Date: Mon, 23 Aug 2021 12:15:24 +0200 Subject: [PATCH 2/5] ManyToMany: Properly escape table / column names --- src/TDBMService.php | 4 +-- .../ManyToManyRelationshipPathDescriptor.php | 25 +++++++++++-------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/TDBMService.php b/src/TDBMService.php index 8daf5c74..65a61b29 100644 --- a/src/TDBMService.php +++ b/src/TDBMService.php @@ -1476,8 +1476,8 @@ public function _getRelatedBeans(ManyToManyRelationshipPathDescriptor $pathDescr { return $this->findObjectsFromSql( $pathDescriptor->getTargetName(), - $pathDescriptor->getPivotFrom(), - $pathDescriptor->getPivotWhere(), + $pathDescriptor->getPivotFrom($this->connection), + $pathDescriptor->getPivotWhere($this->connection), $pathDescriptor->getPivotParams($this->getPrimaryKeyValues($bean)), null, null, diff --git a/src/Utils/ManyToManyRelationshipPathDescriptor.php b/src/Utils/ManyToManyRelationshipPathDescriptor.php index 2f035906..edc7f9a0 100644 --- a/src/Utils/ManyToManyRelationshipPathDescriptor.php +++ b/src/Utils/ManyToManyRelationshipPathDescriptor.php @@ -2,14 +2,16 @@ namespace TheCodingMachine\TDBM\Utils; +use Doctrine\DBAL\Connection; use Doctrine\DBAL\Schema\ForeignKeyConstraint; use TheCodingMachine\TDBM\ResultIterator; use TheCodingMachine\TDBM\TDBMInvalidArgumentException; -use function var_export; +/** + * @internal + */ class ManyToManyRelationshipPathDescriptor { - /** * @var string */ @@ -36,9 +38,6 @@ class ManyToManyRelationshipPathDescriptor private $resultIteratorClass; /** - * ManyToManyRelationshipPathDescriptor constructor. - * @param string $targetTable - * @param string $pivotTable * @param string[] $joinForeignKeys * @param string[] $joinLocalKeys * @param string[] $whereKeys @@ -70,24 +69,30 @@ public function getTargetName(): string return $this->targetTable; } - public function getPivotFrom(): string + public function getPivotFrom(Connection $connection): string { $mainTable = $this->targetTable; $pivotTable = $this->pivotTable; $join = []; foreach ($this->joinForeignKeys as $key => $column) { - $join[] = sprintf('%s.%s = %s.%s', $mainTable, $column, $pivotTable, $this->joinLocalKeys[$key]); + $join[] = sprintf( + '%s.%s = %s.%s', + $connection->quoteIdentifier($mainTable), + $connection->quoteIdentifier($column), + $connection->quoteIdentifier($pivotTable), + $connection->quoteIdentifier($this->joinLocalKeys[$key]) + ); } - return $mainTable . ' JOIN ' . $pivotTable . ' ON ' . implode(' AND ', $join); + return $connection->quoteIdentifier($mainTable) . ' JOIN ' . $connection->quoteIdentifier($pivotTable) . ' ON ' . implode(' AND ', $join); } - public function getPivotWhere(): string + public function getPivotWhere(Connection $connection): string { $paramList = []; foreach ($this->whereKeys as $key => $column) { - $paramList[] = sprintf('%s.%s = :param%s', $this->pivotTable, $column, $key); + $paramList[] = sprintf('%s.%s = :param%s', $connection->quoteIdentifier($this->pivotTable), $connection->quoteIdentifier($column), $key); } return implode(' AND ', $paramList); } From 990e7481050a82619acb421c4731451264ada81c Mon Sep 17 00:00:00 2001 From: Guillaume Date: Mon, 23 Aug 2021 16:23:39 +0200 Subject: [PATCH 3/5] ManyToMany: Escape with MySQL syntax --- README.md | 7 +++++ src/TDBMService.php | 7 +++-- .../ManyToManyRelationshipPathDescriptor.php | 31 +++++++++++++------ 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index a5239fec..05da88c7 100644 --- a/README.md +++ b/README.md @@ -12,3 +12,10 @@ TDBM (The DataBase Machine) Check out [the documentation at https://thecodingmachine.github.io/tdbm/](https://thecodingmachine.github.io/tdbm/). +## Run the test locally + +### Postgres + +Run an instance with `docker run -p 5432:5432 -e POSTGRES_HOST_AUTH_METHOD=trust postgres:12` + +Run the tests with `vendor/bin/phpunit -c phpunit.postgres.xml` diff --git a/src/TDBMService.php b/src/TDBMService.php index 65a61b29..b25246e8 100644 --- a/src/TDBMService.php +++ b/src/TDBMService.php @@ -1474,10 +1474,13 @@ private function fromCache(string $key, callable $closure) */ public function _getRelatedBeans(ManyToManyRelationshipPathDescriptor $pathDescriptor, AbstractTDBMObject $bean): ResultIterator { + // Magic Query expect MySQL syntax for quotes + $platform = new MySqlPlatform(); + return $this->findObjectsFromSql( $pathDescriptor->getTargetName(), - $pathDescriptor->getPivotFrom($this->connection), - $pathDescriptor->getPivotWhere($this->connection), + $pathDescriptor->getPivotFrom($platform), + $pathDescriptor->getPivotWhere($platform), $pathDescriptor->getPivotParams($this->getPrimaryKeyValues($bean)), null, null, diff --git a/src/Utils/ManyToManyRelationshipPathDescriptor.php b/src/Utils/ManyToManyRelationshipPathDescriptor.php index edc7f9a0..a5ab6420 100644 --- a/src/Utils/ManyToManyRelationshipPathDescriptor.php +++ b/src/Utils/ManyToManyRelationshipPathDescriptor.php @@ -3,6 +3,7 @@ namespace TheCodingMachine\TDBM\Utils; use Doctrine\DBAL\Connection; +use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Schema\ForeignKeyConstraint; use TheCodingMachine\TDBM\ResultIterator; use TheCodingMachine\TDBM\TDBMInvalidArgumentException; @@ -13,11 +14,11 @@ class ManyToManyRelationshipPathDescriptor { /** - * @var string + * @var string Unquoted identifier of the target table */ private $targetTable; /** - * @var string + * @var string Unquoted identifier of the pivot table */ private $pivotTable; /** @@ -69,7 +70,12 @@ public function getTargetName(): string return $this->targetTable; } - public function getPivotFrom(Connection $connection): string + /** + * Get the `FROM` clause of the query. + * + * This may have issue handling namespaced table (e.g. mydb.table) + */ + public function getPivotFrom(AbstractPlatform $platform): string { $mainTable = $this->targetTable; $pivotTable = $this->pivotTable; @@ -78,21 +84,26 @@ public function getPivotFrom(Connection $connection): string foreach ($this->joinForeignKeys as $key => $column) { $join[] = sprintf( '%s.%s = %s.%s', - $connection->quoteIdentifier($mainTable), - $connection->quoteIdentifier($column), - $connection->quoteIdentifier($pivotTable), - $connection->quoteIdentifier($this->joinLocalKeys[$key]) + $platform->quoteIdentifier($mainTable), + $platform->quoteIdentifier($column), + $platform->quoteIdentifier($pivotTable), + $platform->quoteIdentifier($this->joinLocalKeys[$key]) ); } - return $connection->quoteIdentifier($mainTable) . ' JOIN ' . $connection->quoteIdentifier($pivotTable) . ' ON ' . implode(' AND ', $join); + return $platform->quoteIdentifier($mainTable) . ' JOIN ' . $platform->quoteIdentifier($pivotTable) . ' ON ' . implode(' AND ', $join); } - public function getPivotWhere(Connection $connection): string + /** + * Get the `WHERE` clause of the query. + * + * This may have issue handling namespaced table (e.g. mydb.table) + */ + public function getPivotWhere(AbstractPlatform $platform): string { $paramList = []; foreach ($this->whereKeys as $key => $column) { - $paramList[] = sprintf('%s.%s = :param%s', $connection->quoteIdentifier($this->pivotTable), $connection->quoteIdentifier($column), $key); + $paramList[] = sprintf('%s.%s = :param%s', $platform->quoteIdentifier($this->pivotTable), $platform->quoteIdentifier($column), $key); } return implode(' AND ', $paramList); } From 7729c279bf75a88dda38c1da675b588cf1354577 Mon Sep 17 00:00:00 2001 From: Guillaume Date: Wed, 1 Sep 2021 15:59:28 +0200 Subject: [PATCH 4/5] Tests: Clarify usage in README.md --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 05da88c7..96c72d01 100644 --- a/README.md +++ b/README.md @@ -14,8 +14,8 @@ Check out [the documentation at https://thecodingmachine.github.io/tdbm/](https: ## Run the test locally -### Postgres - -Run an instance with `docker run -p 5432:5432 -e POSTGRES_HOST_AUTH_METHOD=trust postgres:12` - -Run the tests with `vendor/bin/phpunit -c phpunit.postgres.xml` +You can run the tests with different backend thanks to the following scripts: +- [./tests/phpunit-pgsql.sh](./tests/phpunit-pgsql.sh) +- [./tests/phpunit-mariadb.sh](./tests/phpunit-mariadb.sh) +- [./tests/phpunit-mysql8.sh](./tests/phpunit-mysql8.sh) +- [./tests/phpunit-oracle.sh](./tests/phpunit-oracle.sh) From 38b1060fbb0eb3055521084b88b3a163677a4813 Mon Sep 17 00:00:00 2001 From: Guillaume Date: Wed, 1 Sep 2021 17:27:58 +0200 Subject: [PATCH 5/5] Bench: fix CI run --- .github/workflows/continuous-integration.yml | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index 14a28632..f8142a34 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -373,11 +373,8 @@ jobs: - name: "Updating dependencies" run: "composer update" - - name: "Running PHPBENCH on master branch for comparison" - run: "./phpbench.dist.sh run --tag=master --store" - - - name: "Generating comparison" - run: "./phpbench.dist.sh report --uuid=tag:current_pr --uuid=tag:master --report='{extends: compare, compare: tag}'" + - name: "Running PHPBENCH on master, Generating Report (RESULTS ARE INVERTED AS MASTER IS RUN AFTER CURRENT)" + run: "./phpbench.dist.sh run --ref=current_pr --store --report=aggregate" phpunit-oci8: name: "PHPUnit on OCI8"