Fix save() leaving the model dirty after a quiet save

README.md

@@ -10,6 +10,7 @@ Doppar isn't one package pretending to be a framework. It's a core plus a set of
 |---|---|
 | [Ai](https://github.com/doppar/ai) | AI integration, wired into the framework's own conventions. |
 | [Queue](https://github.com/doppar/queue) | Database-backed background jobs with process-isolated timeouts — a stuck job gets killed, not left to hang the worker. |
+| [Ripple](https://github.com/doppar/ripple) | Build reactive interfaces in pure PHP. |
 | [Embeds](https://github.com/doppar/embeds) | Semantic search for your models. Locally-computed embeddings, no external API required. |
 | [Guard](https://github.com/doppar/guard) | Authorization — gates and policies without wiring your own. |
 | [Flarion](https://github.com/doppar/flarion) | API authentication. |
@@ -26,10 +27,12 @@ Every package here is versioned and released alongside the core. Add what your a
 
 ## Get started
 
-- **Documentation:** [doppar.com](https://doppar.com/versions/4.x/installation)
-- **What's new in 4.x:** [doppar.com/versions/4.x/releases](https://doppar.com/versions/4.x/releases)
-- **News and updates:** [blog.doppar.com](https://blog.doppar.com)
-- **Doppar tour:** [tour.doppar.com](https://tour.doppar.com)
+- **Documentation:** [Doppar](https://doppar.com/versions/4.x/installation)
+- **What's new in 4.x:** [Releases](https://doppar.com/versions/4.x/releases)
+- **AI:** [AI](https://ai.doppar.com)
+- **Ripple:** [Ripple](https://ripple.doppar.com)
+- **News and updates:** [Blog](https://blog.doppar.com)
+- **Doppar tour:** [Tour](https://tour.doppar.com)
 
 ## Contributing
 

src/Phaseolies/Database/Entity/Query/InteractsWithModelQueryProcessing.php

@@ -181,10 +181,16 @@ public function save(): bool
                     ->where($this->primaryKey, $this->attributes[$this->primaryKey])
                     ->update($dirtyAttributes);
 
-                if (! $this->hooksSuppressed && $response) {
+                if ($response) {
                     $this->pruneNonColumnDirtyAttributes($this->getDirtyAttributes());
-                    $this->fireAfterHooks('updated');
-                    $this->firePropertyWatches($dirtyAttributes);
+
+                    if (! $this->hooksSuppressed) {
+                        $this->fireAfterHooks('updated');
+                        $this->firePropertyWatches($dirtyAttributes);
+                    }
+
+                    // Sync even when hooks are suppressed, otherwise the model
+                    // stays dirty for a change that is already persisted.
                     $this->originalAttributes = $this->attributes;
                 }
 

tests/Model/EntityModelHookTest.php

@@ -410,4 +410,204 @@ public function testWithHooksAndWithoutHookTogetherSkipHooks(): void
         $this->assertSame('Quiet', MockHook::find(1)->name);
         $this->assertSame('Quiet', MockHook::find(3)->name);
     }
+
+    /**
+     * Register an after_updated spy on MockHook that records how many times it
+     * ran and what getOriginal('name') was while it ran.
+     *
+     * @return \stdClass
+     */
+    private function spyAfterUpdated(): \stdClass
+    {
+        $spy = new \stdClass();
+        $spy->calls = 0;
+        $spy->originals = [];
+
+        $this->tempHooks([
+            'after_updated' => function ($model) use ($spy) {
+                $spy->calls++;
+                $spy->originals[] = $model->getOriginal('name');
+            },
+        ]);
+
+        return $spy;
+    }
+
+    public function testQuietSaveLeavesTheModelClean(): void
+    {
+        $hook = MockHook::find(1);
+        $hook->name = 'Quiet';
+        $hook->withoutHook()->save();
+
+        $this->assertSame([], $hook->getDirtyAttributes());
+        $this->assertFalse($hook->isDirtyAttr('name'));
+        $this->assertSame('Quiet', $hook->getOriginal('name'));
+    }
+
+    public function testQuietSaveStillPersistsTheChange(): void
+    {
+        $hook = MockHook::find(1);
+        $hook->name = 'Quiet';
+
+        $this->assertTrue($hook->withoutHook()->save());
+        $this->assertSame('Quiet', MockHook::find(1)->name);
+    }
+
+    public function testQuietSaveDoesNotFireAfterUpdated(): void
+    {
+        $spy = $this->spyAfterUpdated();
+
+        $hook = MockHook::find(1);
+        $hook->name = 'Quiet';
+        $hook->withoutHook()->save();
+
+        $this->assertSame(0, $spy->calls);
+    }
+
+    public function testPlainSaveAfterQuietSaveDoesNotRefireTheHook(): void
+    {
+        $spy = $this->spyAfterUpdated();
+
+        $hook = MockHook::find(1);
+        $hook->name = 'Quiet';
+        $hook->withoutHook()->save();
+
+        $this->assertTrue($hook->save()); // nothing new changed
+
+        $this->assertSame(0, $spy->calls, 'a change that is already persisted must not fire after_updated later');
+    }
+
+    public function testPlainSaveAfterQuietSaveDoesNotRewriteTheRow(): void
+    {
+        $hook = MockHook::find(1);
+        $hook->name = 'Quiet';
+        $hook->withoutHook()->save();
+
+        // Change the row behind the model's back. A stale-dirty model would
+        // overwrite it again on the next save().
+        $this->pdo->exec("UPDATE hooks SET name = 'Changed elsewhere' WHERE id = 1");
+
+        $hook->save();
+
+        $this->assertSame('Changed elsewhere', MockHook::find(1)->name);
+    }
+
+    public function testChangeAfterQuietSaveIsTheOnlyDirtyAttribute(): void
+    {
+        $spy = $this->spyAfterUpdated();
+
+        $hook = MockHook::find(1);
+        $hook->name = 'Quiet';
+        $hook->withoutHook()->save();
+
+        $hook->name = 'Loud';
+        $this->assertSame(['name' => 'Loud'], $hook->getDirtyAttributes());
+
+        $hook->save();
+
+        $this->assertSame(1, $spy->calls);
+        $this->assertSame(['Quiet'], $spy->originals, 'the hook must see the quiet value as the original');
+        $this->assertSame('Loud', MockHook::find(1)->name);
+    }
+
+    public function testQuietSaveAppliesToOneWriteOnly(): void
+    {
+        $spy = $this->spyAfterUpdated();
+
+        $hook = MockHook::find(1);
+        $hook->name = 'Quiet';
+        $hook->withoutHook()->save();
+
+        $hook->name = 'Loud';
+        $hook->save();
+
+        $hook->name = 'Louder';
+        $hook->save();
+
+        $this->assertSame(2, $spy->calls);
+    }
+
+    public function testRepeatedQuietSavesStayClean(): void
+    {
+        $hook = MockHook::find(1);
+
+        foreach (['One', 'Two', 'Three'] as $name) {
+            $hook->name = $name;
+            $hook->withoutHook()->save();
+
+            $this->assertSame([], $hook->getDirtyAttributes());
+        }
+
+        $this->assertSame('Three', MockHook::find(1)->name);
+    }
+
+    public function testQuietSaveWithNothingDirtyDoesNotLeakSuppressionIntoTheNextSave(): void
+    {
+        $spy = $this->spyAfterUpdated();
+
+        $hook = MockHook::find(1);
+        $hook->withoutHook()->save(); // nothing dirty, returns early
+
+        $hook->name = 'Loud';
+        $hook->save();
+
+        $this->assertSame(1, $spy->calls);
+    }
+
+    public function testNormalSaveStillExposesTheOldValueInsideAfterUpdated(): void
+    {
+        $spy = $this->spyAfterUpdated();
+
+        $hook = MockHook::find(1);
+        $hook->name = 'Loud';
+        $hook->save();
+
+        $this->assertSame(1, $spy->calls);
+        $this->assertSame(['John Doe'], $spy->originals, 'originals must only be synced after the hook has run');
+        $this->assertSame([], $hook->getDirtyAttributes());
+        $this->assertSame('Loud', $hook->getOriginal('name'));
+    }
+
+    public function testNormalSaveFollowedByAnotherSaveDoesNotRefireTheHook(): void
+    {
+        $spy = $this->spyAfterUpdated();
+
+        $hook = MockHook::find(1);
+        $hook->name = 'Loud';
+        $hook->save();
+        $hook->save();
+
+        $this->assertSame(1, $spy->calls);
+    }
+
+    public function testQuietSaveOnOneInstanceDoesNotAffectAnotherInstanceOfTheSameRow(): void
+    {
+        $spy = $this->spyAfterUpdated();
+
+        $quiet = MockHook::find(1);
+        $other = MockHook::find(1);
+
+        $quiet->name = 'Quiet';
+        $quiet->withoutHook()->save();
+
+        $other->name = 'Loud';
+        $other->save();
+
+        $this->assertSame(1, $spy->calls);
+        $this->assertSame([], $quiet->getDirtyAttributes());
+        $this->assertSame([], $other->getDirtyAttributes());
+    }
+
+    public function testQuietUpdateAndQuietSaveBehaveTheSame(): void
+    {
+        $viaSave = MockHook::find(1);
+        $viaSave->name = 'Same';
+        $viaSave->withoutHook()->save();
+
+        $viaUpdate = MockHook::find(2);
+        $viaUpdate->withoutHook()->update(['name' => 'Same']);
+
+        $this->assertSame($viaUpdate->getDirtyAttributes(), $viaSave->getDirtyAttributes());
+        $this->assertSame($viaUpdate->getOriginal('name'), $viaSave->getOriginal('name'));
+    }
 }

tests/Model/WatcherTest.php

@@ -974,6 +974,22 @@ public function testWithoutHookDisablesWatchesOnSave(): void
         $this->assertFalse(SpyListener::$called, 'withoutHook() must suppress watches');
     }
 
+    /** A quiet save() must not leave the change pending, or the next save() fires the watch late */
+    public function testSaveAfterQuietSaveDoesNotFireTheWatchLater(): void
+    {
+        $id    = $this->insertOrder('pending', 100, 'test');
+        $order = MockWatchableOrder::find($id);
+
+        $order->status = 'cancelled';
+        $order->withoutHook()->save();
+        $this->assertFalse(SpyListener::$called);
+
+        $order->save(); // nothing new changed
+
+        $this->assertFalse(SpyListener::$called, 'an already-persisted change must not fire a watch on a later save');
+        $this->assertSame([], $order->getDirtyAttributes());
+    }
+
     /** withoutHook() prevents watches from firing on update() */
     public function testWithoutHookDisablesWatchesOnUpdate(): void
     {