Merge pull request #102 from moufmouf/multi_key_fix

Fixing multiple primary key with foreign key issue
thecodingmachine · Sep 12, 2018 · 9f0d39c · 9f0d39c
2 parents 6fae482 + 90f8310
commit 9f0d39c
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 6 deletions.
diff --git a/src/DbRow.php b/src/DbRow.php
@@ -436,6 +436,7 @@ public function _setPrimaryKeys(array $primaryKeys): void
     {
         $this->primaryKeys = $primaryKeys;
         foreach ($this->primaryKeys as $column => $value) {
+            // Warning: in case of multi-columns with one being a reference, the $dbRow will contain a reference column (which is not the case elsewhere in the application)
             $this->dbRow[$column] = $value;
         }
     }

diff --git a/src/QueryFactory/FindObjectsQueryFactory.php b/src/QueryFactory/FindObjectsQueryFactory.php
@@ -3,6 +3,7 @@
 
 namespace TheCodingMachine\TDBM\QueryFactory;
 
+use Doctrine\DBAL\Platforms\MySqlPlatform;
 use Doctrine\DBAL\Schema\Schema;
 use TheCodingMachine\TDBM\OrderByAnalyzer;
 use TheCodingMachine\TDBM\TDBMService;
@@ -35,7 +36,11 @@ protected function compute(): void
             return $this->tdbmService->getConnection()->quoteIdentifier($this->mainTable).'.'.$this->tdbmService->getConnection()->quoteIdentifier($pkColumn);
         }, $pkColumnNames);
 
-        $countSql = 'SELECT COUNT(DISTINCT '.implode(', ', $pkColumnNames).') FROM MAGICJOIN('.$this->mainTable.')';
+        if (count($pkColumnNames) === 1 || $this->tdbmService->getConnection()->getDatabasePlatform() instanceof MySqlPlatform) {
+            $countSql = 'SELECT COUNT(DISTINCT '.implode(', ', $pkColumnNames).') FROM MAGICJOIN('.$this->mainTable.')';
+        } else {
+            $countSql = 'SELECT COUNT(*) FROM (SELECT DISTINCT '.implode(', ', $pkColumnNames).' FROM MAGICJOIN('.$this->mainTable.')) tmp';
+        }
 
         if (!empty($this->filterString)) {
             $sql .= ' WHERE '.$this->filterString;

diff --git a/src/TDBMService.php b/src/TDBMService.php
@@ -886,10 +886,10 @@ public function getPrimaryKeysForObjectFromDbRow(DbRow $dbRow): array
 
         $primaryKeyColumns = $this->getPrimaryKeyColumns($table);
         $values = array();
+        $dbRowValues = $dbRow->_getDbRow();
         foreach ($primaryKeyColumns as $column) {
-            $value = $dbRow->get($column);
-            if ($value !== null) {
-                $values[$column] = $value;
+            if (isset($dbRowValues[$column])) {
+                $values[$column] = $dbRowValues[$column];
             }
         }
 

diff --git a/tests/TDBMAbstractServiceTest.php b/tests/TDBMAbstractServiceTest.php
@@ -344,6 +344,13 @@ private static function initSchema(Connection $connection): void
 
         $toSchema->getTable($connection->quoteIdentifier('ref_no_prim_key'))->addForeignKeyConstraint($connection->quoteIdentifier('ref_no_prim_key'), [$connection->quoteIdentifier('from')], [$connection->quoteIdentifier('to')]);
 
+        // A table with multiple primary keys.
+        $db->table('states')
+            ->column('country_id')->references('country')
+            ->column('code')->string(3)
+            ->column('name')->string(50)->then()
+            ->primaryKey(['country_id', 'code']);
+
         $sqlStmts = $toSchema->getMigrateFromSql($fromSchema, $connection->getDatabasePlatform());
 
         foreach ($sqlStmts as $sqlStmt) {

diff --git a/tests/TDBMDaoGeneratorTest.php b/tests/TDBMDaoGeneratorTest.php
@@ -46,6 +46,7 @@
 use TheCodingMachine\TDBM\Test\Dao\Bean\PersonBean;
 use TheCodingMachine\TDBM\Test\Dao\Bean\RefNoPrimKeyBean;
 use TheCodingMachine\TDBM\Test\Dao\Bean\RoleBean;
+use TheCodingMachine\TDBM\Test\Dao\Bean\StateBean;
 use TheCodingMachine\TDBM\Test\Dao\Bean\UserBean;
 use TheCodingMachine\TDBM\Test\Dao\BoatDao;
 use TheCodingMachine\TDBM\Test\Dao\CatDao;
@@ -57,6 +58,7 @@
 use TheCodingMachine\TDBM\Test\Dao\Generated\UserBaseDao;
 use TheCodingMachine\TDBM\Test\Dao\RefNoPrimKeyDao;
 use TheCodingMachine\TDBM\Test\Dao\RoleDao;
+use TheCodingMachine\TDBM\Test\Dao\StateDao;
 use TheCodingMachine\TDBM\Test\Dao\UserDao;
 use TheCodingMachine\TDBM\Utils\PathFinder\NoPathFoundException;
 use TheCodingMachine\TDBM\Utils\TDBMDaoGenerator;
@@ -1720,9 +1722,50 @@ public function testFilterBag()
         $this->assertSame('John Doe', $users[0]->getName());
     }
 
+    /**
+     * @depends testDaoGeneration
+     */
     public function testDecimalIsMappedToString()
     {
         $reflectionClass = new \ReflectionClass(BoatBaseBean::class);
         $this->assertSame('string', (string) $reflectionClass->getMethod('getLength')->getReturnType());
     }
+
+    /**
+     * @depends testDaoGeneration
+     */
+    public function testNoGetByIdOnMultiPrimaryKeys()
+    {
+        $reflectionClass = new \ReflectionClass(StateDao::class);
+        $this->assertFalse($reflectionClass->hasMethod('getById'));
+    }
+
+    /**
+     * @depends testDaoGeneration
+     */
+    public function testInsertMultiPrimaryKeysBean()
+    {
+        $countryDao = new CountryDao($this->tdbmService);
+
+        $country = $countryDao->getById(1);
+
+        $stateDao = new StateDao($this->tdbmService);
+        $state = new StateBean($country, 'IDF', 'Ile de France');
+        $stateDao->save($state);
+
+        $this->assertSame($state, $stateDao->findAll()[0]);
+    }
+
+    /**
+     * @depends testInsertMultiPrimaryKeysBean
+     */
+    public function testDeleteMultiPrimaryKeysBean()
+    {
+        $stateDao = new StateDao($this->tdbmService);
+
+        $state = $stateDao->findAll()[0];
+        $stateDao->delete($state);
+        $this->assertCount(0, $stateDao->findAll());
+
+    }
 }
diff --git a/tests/TDBMSchemaAnalyzerTest.php b/tests/TDBMSchemaAnalyzerTest.php
@@ -79,11 +79,12 @@ public function testGetIncomingForeignKeys3()
         $tdbmSchemaAnalyzer = new TDBMSchemaAnalyzer(self::getConnection(), $cache, $schemaAnalyzer);
 
         $fks = $tdbmSchemaAnalyzer->getIncomingForeignKeys('country');
-        $this->assertCount(4, $fks);
-        $tables = [$fks[0]->getLocalTableName(), $fks[1]->getLocalTableName(), $fks[2]->getLocalTableName(), $fks[3]->getLocalTableName()];
+        $this->assertCount(5, $fks);
+        $tables = [$fks[0]->getLocalTableName(), $fks[1]->getLocalTableName(), $fks[2]->getLocalTableName(), $fks[3]->getLocalTableName(), $fks[4]->getLocalTableName()];
         $this->assertContains('users', $tables);
         $this->assertContains('all_nullable', $tables);
         $this->assertContains('boats', $tables);
+        $this->assertContains('states', $tables);
     }
 
     public function testGetPivotTableLinkedToTable()