From 35ae45cd25cd198456143e75dd7447342722718e Mon Sep 17 00:00:00 2001 From: Samuel Levy Date: Fri, 20 Sep 2024 11:18:20 +1000 Subject: [PATCH] [11.x] Prevent recursion when touching chaperoned models --- .../Eloquent/Concerns/HasRelationships.php | 27 +++--- src/Illuminate/Support/helpers.php | 6 +- .../DatabaseEloquentIntegrationTest.php | 83 +++++++++++++++++++ .../Support/WithoutRecursionHelperTest.php | 3 +- tests/Support/SupportRecursableTest.php | 9 +- 5 files changed, 109 insertions(+), 19 deletions(-) diff --git a/src/Illuminate/Database/Eloquent/Concerns/HasRelationships.php b/src/Illuminate/Database/Eloquent/Concerns/HasRelationships.php index 634a19542203..2891113d2d03 100644 --- a/src/Illuminate/Database/Eloquent/Concerns/HasRelationships.php +++ b/src/Illuminate/Database/Eloquent/Concerns/HasRelationships.php @@ -792,17 +792,22 @@ public function touches($relation) */ public function touchOwners() { - foreach ($this->getTouchedRelations() as $relation) { - $this->$relation()->touch(); - - if ($this->$relation instanceof self) { - $this->$relation->fireModelEvent('saved', false); - - $this->$relation->touchOwners(); - } elseif ($this->$relation instanceof Collection) { - $this->$relation->each->touchOwners(); - } - } + without_recursion( + function () { + foreach ($this->getTouchedRelations() as $relation) { + $this->$relation()->touch(); + + if ($this->$relation instanceof self) { + $this->$relation->fireModelEvent('saved', false); + + $this->$relation->touchOwners(); + } elseif ($this->$relation instanceof Collection) { + $this->$relation->each->touchOwners(); + } + } + }, + null, + ); } /** diff --git a/src/Illuminate/Support/helpers.php b/src/Illuminate/Support/helpers.php index 33bbabdce2ef..3ef9a81b458d 100644 --- a/src/Illuminate/Support/helpers.php +++ b/src/Illuminate/Support/helpers.php @@ -528,9 +528,9 @@ function without_recursion(callable $callback, mixed $onRecursion, ?string $as = { return Recurser::instance() ->withoutRecursion( - $as - ? Recursable::fromSignature($as, $callback, $onRecursion, $for) - : Recursable::fromTrace(debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT, 2), $callback, $onRecursion, $for) + $as + ? Recursable::fromSignature($as, $callback, $onRecursion, $for) + : Recursable::fromTrace(debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT, 2), $callback, $onRecursion, $for) ); } } diff --git a/tests/Database/DatabaseEloquentIntegrationTest.php b/tests/Database/DatabaseEloquentIntegrationTest.php index 3dbb553891c9..26541e631c3b 100644 --- a/tests/Database/DatabaseEloquentIntegrationTest.php +++ b/tests/Database/DatabaseEloquentIntegrationTest.php @@ -152,6 +152,13 @@ protected function createSchema() $table->morphs('taggable'); $table->string('taxonomy')->nullable(); }); + + $this->schema($connection)->create('categories', function ($table) { + $table->increments('id'); + $table->string('name'); + $table->integer('parent_id')->nullable(); + $table->timestamps(); + }); } $this->schema($connection)->create('non_incrementing_users', function ($table) { @@ -2182,6 +2189,61 @@ public function testMorphPivotsCanBeRefreshed() $this->assertSame('primary', $pivot->taxonomy); } + public function testTouchingChaperonedChildModelUpdatesParentTimestamps() + { + $before = Carbon::now(); + + $one = EloquentTouchingCategory::create(['id' => 1, 'name' => 'One']); + $two = $one->children()->create(['id' => 2, 'name' => 'Two']); + + $this->assertTrue($before->isSameDay($one->updated_at)); + $this->assertTrue($before->isSameDay($two->updated_at)); + + Carbon::setTestNow($future = $before->copy()->addDays(3)); + + $two->touch(); + + $this->assertTrue($future->isSameDay($two->fresh()->updated_at), 'It is not touching model own timestamps.'); + $this->assertTrue($future->isSameDay($one->fresh()->updated_at), 'It is not touching chaperoned models related timestamps.'); + } + + public function testTouchingBiDirectionalChaperonedModelUpdatesAllRelatedTimestamps() + { + $before = Carbon::now(); + + EloquentTouchingCategory::insert([ + ['id' => 1, 'name' => 'One', 'parent_id' => null, 'created_at' => $before, 'updated_at' => $before], + ['id' => 2, 'name' => 'Two', 'parent_id' => 1, 'created_at' => $before, 'updated_at' => $before], + ['id' => 3, 'name' => 'Three', 'parent_id' => 1, 'created_at' => $before, 'updated_at' => $before], + ['id' => 4, 'name' => 'Four', 'parent_id' => 2, 'created_at' => $before, 'updated_at' => $before], + ]); + + $one = EloquentTouchingCategory::find(1); + [$two, $three] = $one->children; + [$four] = $two->children; + + $this->assertTrue($before->isSameDay($one->updated_at)); + $this->assertTrue($before->isSameDay($two->updated_at)); + $this->assertTrue($before->isSameDay($three->updated_at)); + $this->assertTrue($before->isSameDay($four->updated_at)); + + Carbon::setTestNow($future = $before->copy()->addDays(3)); + + // Touch a random model and check that all of the others have been updated + $models = tap([$one, $two, $three, $four], shuffle(...)); + $target = array_shift($models); + $target->touch(); + + $this->assertTrue($future->isSameDay($target->fresh()->updated_at), 'It is not touching model own timestamps.'); + + while ($next = array_shift($models)) { + $this->assertTrue( + $future->isSameDay($next->fresh()->updated_at), + 'It is not touching related models timestamps.' + ); + } + } + /** * Helpers... */ @@ -2486,3 +2548,24 @@ public function post() return $this->belongsTo(EloquentTouchingPost::class, 'post_id'); } } + +class EloquentTouchingCategory extends Eloquent +{ + protected $table = 'categories'; + protected $guarded = []; + + protected $touches = [ + 'parent', + 'children', + ]; + + public function parent() + { + return $this->belongsTo(EloquentTouchingCategory::class, 'parent_id'); + } + + public function children() + { + return $this->hasMany(EloquentTouchingCategory::class, 'parent_id')->chaperone(); + } +} diff --git a/tests/Integration/Support/WithoutRecursionHelperTest.php b/tests/Integration/Support/WithoutRecursionHelperTest.php index a914b560d9ae..894e8c706f74 100644 --- a/tests/Integration/Support/WithoutRecursionHelperTest.php +++ b/tests/Integration/Support/WithoutRecursionHelperTest.php @@ -125,7 +125,8 @@ public function testCallbacksAreCalledOnceOnRecursiveInstances() $this->assertSame(['upline' => 1, 'upline_callback' => 1], $tail->pullCallCount()); } - public function testRecursionCallbacksAreCalledOnRecursiveInstances() { + public function testRecursionCallbacksAreCalledOnRecursiveInstances() + { $head = DoublyLinkedRecursiveList::make(children: 2); $body = $head->getNext(); $tail = $body->getNext(); diff --git a/tests/Support/SupportRecursableTest.php b/tests/Support/SupportRecursableTest.php index a2291f8a0454..8a3e1ccb6258 100644 --- a/tests/Support/SupportRecursableTest.php +++ b/tests/Support/SupportRecursableTest.php @@ -12,8 +12,8 @@ class SupportRecursableTest extends TestCase { public function testForSetsObjectIfBlank() { - $one = (object)[]; - $two = (object)[]; + $one = (object) []; + $two = (object) []; $recursableOne = new Recursable(fn () => 'foo', 'bar', null); $recursableTwo = new Recursable(fn () => 'foo', 'bar', $one); @@ -133,7 +133,8 @@ public function testHashFromSignature(string $signature) } #[DataProvider('backtraceProvider')] - public function testObjectFromTrace(array $trace, array $target, string $signature) { + public function testObjectFromTrace(array $trace, array $target, string $signature) + { $this->assertSame($target['object'], RecursableStub::expose_objectFromTrace($trace)); } @@ -214,7 +215,7 @@ public function testFromSignatureCreatesRecursable(string $signature) public static function backtraceProvider(): array { $empty = ['file' => '', 'class' => '', 'function' => '', 'line' => 0, 'object' => null]; - $object = (object)[]; + $object = (object) []; return [ 'no frames' => [[], $empty, ':0'],