Refresh entity original values after saving in RepositoryCore

MIGRATE.md

@@ -1,5 +1,11 @@
 # v11 release
 
+## Unreleased
+
+### Bug fixes
+
+* `Repository\RepositoryCore::save()` now refreshes the original values of an entity after a successful insert or update, so a field that is changed back to the value it was loaded with is persisted instead of silently dropped.
+
 ## v11.7
 
 ### New features

src/Repository/RepositoryCore.php

@@ -277,10 +277,20 @@ public function save(Entity $entity): void
                 $property->setValue($entity, $fieldValue);
             }
 
+            // The values the entity holds are the values that were just sent to
+            // the database, plus the additional id fields that were read back, so
+            // they become the new baseline for change tracking. A trigger or a
+            // column coercion can still make the stored row differ, but that is not
+            // something this entity knows about, and rewriting our own values over
+            // it on the next save would only make it worse.
+            //
+            $this->refreshOriginalValues($entity, $this->getEntityFields($entity, false));
+
             return;
         }
 
-        $data = $this->getChangedFields($entity);
+        $currentFields = $this->getEntityFields($entity, false);
+        $data = $this->diffWithOriginalValues($entity, $currentFields);
 
         if (count($data) === 0)
         {
@@ -302,6 +312,12 @@ public function save(Entity $entity): void
 
         $class = static::$entityClass;
         $this->database->query($query, $params, "Failed to update object ({$class})");
+
+        // The update succeeded, so the current values are the new baseline. Without
+        // this the entity would keep comparing against the values it was loaded with,
+        // and changing a field back to its loaded value would not be persisted.
+        //
+        $this->refreshOriginalValues($entity, $currentFields);
     }
 
     /**
@@ -436,10 +452,36 @@ public function getEntityFields(Entity $entity, bool $includeId = true): array
      */
     public function getChangedFields(Entity $entity): array
     {
-        $currentData = $this->getEntityFields($entity, false);
-        $originalData = $entity->getOriginalValues();
+        return $this->diffWithOriginalValues($entity, $this->getEntityFields($entity, false));
+    }
+
+    /**
+     * Compare already retrieved entity fields against the original values of an entity.
+     *
+     * @param T                    $entity        The entity to check for changes
+     * @param array<string, mixed> $currentFields The current fields of that entity
+     *
+     * @return array<string, mixed> The changed fields
+     */
+    private function diffWithOriginalValues(Entity $entity, array $currentFields): array
+    {
+        return array_diff_assoc($currentFields, $entity->getOriginalValues());
+    }
 
-        return array_diff_assoc($currentData, $originalData);
+    /**
+     * Make the given fields the new baseline for change tracking of an entity.
+     *
+     * The id key is kept, because instantiateEntityFromData() records it as well.
+     *
+     * @param T                    $entity        The entity to update the baseline for
+     * @param array<string, mixed> $currentFields The fields that are now stored in the database
+     */
+    private function refreshOriginalValues(Entity $entity, array $currentFields): void
+    {
+        $entity->setOriginalValues(array_merge(
+            ['id' => $entity->getId()],
+            $currentFields,
+        ));
     }
 
     /**

tests/Unit/RepositoryCoreTest.php

@@ -4,7 +4,10 @@
 
 use Codeception\Test\Unit;
 use Psr\Container\ContainerInterface as Container;
+use Tests\Support\TestEntity;
+use Tests\Support\TestRepository;
 use WebFramework\Database\Database;
+use WebFramework\Database\DatabaseResultWrapper;
 use WebFramework\Repository\Column;
 use WebFramework\Repository\UserRepository;
 
@@ -600,4 +603,193 @@ public function testInstantiateEntityFromDataWithoutPrefix()
             ])
         ;
     }
+
+    public function testSaveRevertedValueAfterSaveIsPersisted()
+    {
+        $queries = [];
+
+        $instance = $this->construct(
+            TestRepository::class,
+            [
+                $this->makeEmpty(Container::class),
+                $this->makeEmptyDatabase($queries),
+            ]
+        );
+
+        $entity = $instance->instantiateEntityFromData([
+            'id' => 42,
+            'name' => 'original',
+            'email' => 'tester@example.com',
+            'age' => 30,
+            'active' => true,
+            'secret_field' => 'secret',
+            'created_at' => 1000,
+        ]);
+
+        $entity->setName('changed');
+        $instance->save($entity);
+
+        $entity->setName('original');
+        $instance->save($entity);
+
+        verify(count($queries))
+            ->equals(2)
+        ;
+
+        verify($this->normalizeQuery($queries[0]['query']))
+            ->equals('UPDATE test_entities SET `name` = ? WHERE id = ?')
+        ;
+
+        verify($queries[0]['params'])
+            ->equals(['changed', 42])
+        ;
+
+        verify($this->normalizeQuery($queries[1]['query']))
+            ->equals('UPDATE test_entities SET `name` = ? WHERE id = ?')
+        ;
+
+        verify($queries[1]['params'])
+            ->equals(['original', 42])
+        ;
+    }
+
+    public function testSaveUnchangedEntityAfterUpdateDoesNotQueryAgain()
+    {
+        $queries = [];
+
+        $instance = $this->construct(
+            TestRepository::class,
+            [
+                $this->makeEmpty(Container::class),
+                $this->makeEmptyDatabase($queries),
+            ]
+        );
+
+        $entity = $instance->instantiateEntityFromData([
+            'id' => 42,
+            'name' => 'original',
+            'email' => 'tester@example.com',
+            'age' => 30,
+            'active' => true,
+            'secret_field' => 'secret',
+            'created_at' => 1000,
+        ]);
+
+        $entity->setName('changed');
+        $instance->save($entity);
+        $instance->save($entity);
+
+        verify(count($queries))
+            ->equals(1)
+        ;
+    }
+
+    public function testSaveAfterCreateDoesNotUpdateWhenUnchanged()
+    {
+        $queries = [];
+
+        $createdEntity = new TestEntity();
+        $createdEntity->setObjectId(42);
+
+        $instance = $this->construct(
+            TestRepository::class,
+            [
+                $this->makeEmpty(Container::class),
+                $this->makeEmptyDatabase($queries),
+            ],
+            [
+                'create' => fn (array $data) => $createdEntity,
+            ]
+        );
+
+        $entity = new TestEntity();
+        $entity->setName('tester');
+        $entity->setEmail('tester@example.com');
+        $entity->setAge(30);
+        $entity->setActive(true);
+        $entity->setSecretField('secret');
+        $entity->setCreatedAt(1000);
+
+        $instance->save($entity);
+
+        verify($entity->getId())
+            ->equals(42)
+        ;
+
+        $instance->save($entity);
+
+        verify($queries)
+            ->equals([])
+        ;
+    }
+
+    public function testSaveAfterCreateOnlyUpdatesChangedField()
+    {
+        $queries = [];
+
+        $createdEntity = new TestEntity();
+        $createdEntity->setObjectId(42);
+
+        $instance = $this->construct(
+            TestRepository::class,
+            [
+                $this->makeEmpty(Container::class),
+                $this->makeEmptyDatabase($queries),
+            ],
+            [
+                'create' => fn (array $data) => $createdEntity,
+            ]
+        );
+
+        $entity = new TestEntity();
+        $entity->setName('tester');
+        $entity->setEmail('tester@example.com');
+        $entity->setAge(30);
+        $entity->setActive(true);
+        $entity->setSecretField('secret');
+        $entity->setCreatedAt(1000);
+
+        $instance->save($entity);
+
+        $entity->setName('renamed');
+        $instance->save($entity);
+
+        verify(count($queries))
+            ->equals(1)
+        ;
+
+        verify($this->normalizeQuery($queries[0]['query']))
+            ->equals('UPDATE test_entities SET `name` = ? WHERE id = ?')
+        ;
+
+        verify($queries[0]['params'])
+            ->equals(['renamed', 42])
+        ;
+    }
+
+    /**
+     * Create a Database stub that records every query it receives.
+     *
+     * @param array<int, array{query: string, params: array<mixed>}> $queries
+     */
+    private function makeEmptyDatabase(array &$queries): Database
+    {
+        $result = $this->makeEmpty(DatabaseResultWrapper::class);
+
+        return $this->makeEmpty(Database::class, [
+            'query' => function (string $query, array $params) use (&$queries, $result) {
+                $queries[] = [
+                    'query' => $query,
+                    'params' => $params,
+                ];
+
+                return $result;
+            },
+        ]);
+    }
+
+    private function normalizeQuery(string $query): string
+    {
+        return preg_replace('/\s+/', ' ', trim($query));
+    }
 }