Merge pull request #268 from homersimpsons/fix/discard-changes-discar…

…ds-relations discardChanges: Discards relations and References
thecodingmachine · Dec 29, 2021 · 2847c66 · 2847c66
2 parents 10516d8 + 44c54f2
commit 2847c66
Show file tree

Hide file tree

Showing 9 changed files with 199 additions and 221 deletions.
diff --git a/composer.json b/composer.json
@@ -43,9 +43,9 @@
         "ext-intl": "*"
     },
     "require-dev" : {
-        "phpunit/phpunit": "^8.5.8",
+        "phpunit/phpunit": "^9.5",
         "php-coveralls/php-coveralls": "^2.1",
-        "wa72/simplelogger" : "^1.0",
+        "wa72/simplelogger": "^1.0",
         "friendsofphp/php-cs-fixer": "^2.16.4",
         "symfony/process": "^3 || ^4 || ^5",
         "thecodingmachine/tdbm-fluid-schema-builder": "^1.0.0",
@@ -71,12 +71,21 @@
     "scripts": {
         "phpstan": "php -d memory_limit=3G vendor/bin/phpstan analyse src -c phpstan.neon --no-progress -vvv",
         "require-checker": "composer-require-checker check --config-file=composer-require-checker.json",
-        "test": "phpunit",
+        "test": "XDEBUG_MODE=coverage phpunit",
         "csfix": "php-cs-fixer fix src/ && php-cs-fixer fix tests/",
         "cscheck": "php-cs-fixer fix src/ --dry-run --stop-on-violation && php-cs-fixer fix tests/ --dry-run --stop-on-violation ",
-        "ci": ["@cscheck", "@phpstan", "@test", "@require-checker"],
-        "post-install-cmd": ["@composer bin all install --ansi"],
-        "post-update-cmd": ["@composer bin all update --ansi"]
+        "ci": [
+            "@cscheck",
+            "@phpstan",
+            "@test",
+            "@require-checker"
+        ],
+        "post-install-cmd": [
+            "@composer bin all install --ansi"
+        ],
+        "post-update-cmd": [
+            "@composer bin all update --ansi"
+        ]
     },
     "extra": {
         "branch-alias": {

diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -2,7 +2,7 @@
 
 <phpunit
          xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-         xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/8.5/phpunit.xsd"
+         xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd"
          backupGlobals="false"
          backupStaticAttributes="false"
          colors="true"
@@ -21,25 +21,25 @@
 
     <php>
         <!-- "Real" test database -->
-        <var name="db_host" value="localhost" />
-        <var name="db_username" value="root" />
-        <var name="db_password" value="" />
-        <var name="db_name" value="tdbm_testcase" />
+        <var name="db_host" value="localhost"/>
+        <var name="db_username" value="root"/>
+        <var name="db_password" value=""/>
+        <var name="db_name" value="tdbm_testcase"/>
         <var name="db_port" value="3306"/>
         <var name="db_driver" value="pdo_mysql"/>
     </php>
 
-    <filter>
-  		<whitelist processUncoveredFilesFromWhitelist="true">
-    		<directory suffix=".php">src/</directory>
-    		<exclude>
-                <directory suffix=".php">src/Test</directory>
-                <file>src/Schema/LockFileSchemaManager.php</file>
-    		</exclude>
-  		</whitelist>
-	</filter>
-    <logging>
-        <log type="coverage-html" target="build/coverage"/>
-        <log type="coverage-clover" target="build/logs/clover.xml"/>
-    </logging>
+    <coverage processUncoveredFiles="true">
+        <include>
+            <directory suffix=".php">src/</directory>
+        </include>
+        <exclude>
+            <directory suffix=".php">src/Test</directory>
+            <file>src/Schema/LockFileSchemaManager.php</file>
+        </exclude>
+        <report>
+            <clover outputFile="build/logs/clover.xml"/>
+            <html outputDirectory="build/coverage"/>
+        </report>
+    </coverage>
 </phpunit>
diff --git a/src/AbstractTDBMObject.php b/src/AbstractTDBMObject.php
@@ -553,6 +553,7 @@ public function discardChanges(): void
         }
 
         $this->_setStatus(TDBMObjectStateEnum::STATE_NOT_LOADED);
+        $this->manyToOneRelationships = [];
     }
 
     /**

diff --git a/src/DbRow.php b/src/DbRow.php
@@ -172,6 +172,8 @@ public function _setStatus(string $state) : void
             // after saving we are back to a loaded state, hence unmodified.
             $this->modifiedColumns = [];
             $this->modifiedReferences = [];
+        } elseif ($state === TDBMObjectStateEnum::STATE_NOT_LOADED) {
+            $this->references = []; // We clear already set references to force re-fetch of those
         }
     }
 
@@ -295,10 +297,13 @@ public function getRef(string $foreignKeyName, string $className, string $result
 
             // If the foreign key points to the primary key, let's use findObjectByPk
             if ($this->tdbmService->getPrimaryKeyColumns($foreignTableName) === $foreignColumns) {
-                return $this->tdbmService->findObjectByPk($foreignTableName, $filter, [], true, $className, $resultIteratorClass);
+                $reference = $this->tdbmService->findObjectByPk($foreignTableName, $filter, [], true, $className, $resultIteratorClass);
             } else {
-                return $this->tdbmService->findObject($foreignTableName, $filter, [], [], $className, $resultIteratorClass);
+                $reference = $this->tdbmService->findObject($foreignTableName, $filter, [], [], $className, $resultIteratorClass);
             }
+
+            $this->references[$foreignKeyName] = $reference;
+            return $reference;
         }
     }
 

diff --git a/tests/TDBMAbstractServiceTest.php b/tests/TDBMAbstractServiceTest.php
@@ -797,22 +797,28 @@ private static function initSchema(Connection $connection): void
         self::insert($connection, 'all', ['analyze' => 1, 'and' => 1]);
     }
 
-    public static function insert(Connection $connection, string $tableName, array $data): void
+    private static function quoteKeys(Connection $connection, array $data): array
     {
         $quotedData = [];
         foreach ($data as $id => $value) {
             $quotedData[$connection->quoteIdentifier($id)] = $value;
         }
-        $connection->insert($connection->quoteIdentifier($tableName), $quotedData);
+        return $quotedData;
     }
 
-    protected static function delete(Connection $connection, string $tableName, array $data): void
+    public static function insert(Connection $connection, string $tableName, array $data): void
     {
-        $quotedData = [];
-        foreach ($data as $id => $value) {
-            $quotedData[$connection->quoteIdentifier($id)] = $value;
-        }
-        $connection->delete($connection->quoteIdentifier($tableName), $quotedData);
+        $connection->insert($connection->quoteIdentifier($tableName), self::quoteKeys($connection, $data));
+    }
+
+    public static function update(Connection $connection, string $tableName, array $data, array $criteria): void
+    {
+        $connection->update($connection->quoteIdentifier($tableName), self::quoteKeys($connection, $data), self::quoteKeys($connection, $criteria));
+    }
+
+    protected static function delete(Connection $connection, string $tableName, array $criteria): void
+    {
+        $connection->delete($connection->quoteIdentifier($tableName), self::quoteKeys($connection, $criteria));
     }
 
     protected static function isMariaDb(Connection $connection): bool

diff --git a/tests/TDBMDaoGeneratorTest.php b/tests/TDBMDaoGeneratorTest.php
@@ -149,7 +149,7 @@ public function testDaoGeneration(): void
 
         $this->tdbmDaoGenerator->generateAllDaosAndBeans();
 
-        $this->assertFileNotExists($dummyFile);
+        $this->assertFileDoesNotExist($dummyFile);
 
         //Check that the lock file was generated
         $this->assertFileExists($schemaFilePath);
@@ -1151,6 +1151,50 @@ public function testDiscardChanges(): void
         $this->assertEquals($oldName, $contactBean->getName());
     }
 
+    /**
+     * @depends testDaoGeneration
+     */
+    public function testDiscardChangesDiscardsRelations(): void
+    {
+        $countryDao = new CountryDao($this->tdbmService);
+        $countryBean = $countryDao->getById(1);
+
+        $oldCount = $countryBean->getBoatsByAnchorageCountry()->count();
+
+        self::insert($this->tdbmService->getConnection(), 'boats', [
+            'name' => 'RoseBud2',
+            'anchorage_country' => 1,
+            'current_country' => 1,
+            'length' => '13.5',
+        ]);
+
+        $countryBean->discardChanges();
+
+        $this->assertEquals($oldCount + 1, $countryBean->getBoatsByAnchorageCountry()->count());
+    }
+
+    /**
+     * @depends testDaoGeneration
+     */
+    public function testDiscardChangesDiscardsRowRef(): void
+    {
+        $newExpectedId = 3;
+
+        $userDao = new UserDao($this->tdbmService);
+        $countryDao = new CountryDao($this->tdbmService);
+        $userBean = $userDao->getById(4);
+
+        $oldId = $userBean->getCountry()->getId();
+        $this->assertNotEquals($newExpectedId, $oldId, 'The IDs are the same, the test won\'t have any effect');
+
+        $userBean->setCountry($countryDao->getById($oldId)); // This triggers the `DbRow::setRef` method which causes the issue
+        self::update($this->tdbmService->getConnection(), 'users', ['country_id' => $newExpectedId], ['id' => 4]);
+
+        $userBean->discardChanges();
+
+        $this->assertEquals($newExpectedId, $userBean->getCountry()->getId());
+    }
+
     /**
      * @depends testDaoGeneration
      */
@@ -1599,8 +1643,7 @@ public function testPSR2Compliance(): void
 
         // executes after the command finishes
         if (!$process->isSuccessful()) {
-            echo $process->getOutput();
-            $this->fail('Generated code is not PSR-2 compliant');
+            $this->fail('Generated code is not PSR-2 compliant' . PHP_EOL . $process->getErrorOutput());
         }
         $this->assertTrue($process->isSuccessful());
     }

diff --git a/tests/TDBMServiceTest.php b/tests/TDBMServiceTest.php
@@ -836,7 +836,7 @@ public function testBuildFilterFromFilterBagIterator(): void
         $tdbmService = new TDBMService(new Configuration('TheCodingMachine\\TDBM\\Test\\Dao\\Bean', 'TheCodingMachine\\TDBM\\Test\\Dao', self::getConnection(), $this->getNamingStrategy(), null, null, new NullLogger()));
 
         [$sql, $parameters, $counter] = $tdbmService->buildFilterFromFilterBag(new \ArrayIterator(['id' => 1]), self::getConnection()->getDatabasePlatform());
-        $this->assertRegExp('/\(.id. = :tdbmparam1\)/', $sql);
+        $this->assertMatchesRegularExpression('/\(.id. = :tdbmparam1\)/', $sql);
         $this->assertEquals($parameters['tdbmparam1'], 1);
     }