Merge pull request #264 from homersimpsons/fix/pivot-escaping

ManyToMany: Properly escape table / column names

.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"

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
+
+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)

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(),
-            $pathDescriptor->getPivotWhere(),
+            $pathDescriptor->getPivotFrom($platform),
+            $pathDescriptor->getPivotWhere($platform),
             $pathDescriptor->getPivotParams($this->getPrimaryKeyValues($bean)),
             null,
             null,

src/Utils/ManyToManyRelationshipPathDescriptor.php

@@ -2,20 +2,23 @@
 
 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;
-use function var_export;
 
+/**
+ * @internal
+ */
 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;
     /**
@@ -36,9 +39,6 @@ class ManyToManyRelationshipPathDescriptor
     private $resultIteratorClass;
 
     /**
-     * ManyToManyRelationshipPathDescriptor constructor.
-     * @param string $targetTable
-     * @param string $pivotTable
      * @param string[] $joinForeignKeys
      * @param string[] $joinLocalKeys
      * @param string[] $whereKeys
@@ -70,24 +70,40 @@ public function getTargetName(): string
         return $this->targetTable;
     }
 
-    public function getPivotFrom(): 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;
 
         $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',
+                $platform->quoteIdentifier($mainTable),
+                $platform->quoteIdentifier($column),
+                $platform->quoteIdentifier($pivotTable),
+                $platform->quoteIdentifier($this->joinLocalKeys[$key])
+            );
         }
 
-        return $mainTable . ' JOIN ' . $pivotTable . ' ON ' . implode(' AND ', $join);
+        return $platform->quoteIdentifier($mainTable) . ' JOIN ' . $platform->quoteIdentifier($pivotTable) . ' ON ' . implode(' AND ', $join);
     }
 
-    public function getPivotWhere(): 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', $this->pivotTable, $column, $key);
+            $paramList[] = sprintf('%s.%s = :param%s', $platform->quoteIdentifier($this->pivotTable), $platform->quoteIdentifier($column), $key);
         }
         return implode(' AND ', $paramList);
     }

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

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');