From 43bc16512836bbb06c97d8d814b762c58d25dec6 Mon Sep 17 00:00:00 2001 From: Franck DAKIA Date: Tue, 2 Jun 2026 14:09:02 +0000 Subject: [PATCH] fix(database): fix retrieve belongTo into loop execution --- src/Console/Command/MigrationCommand.php | 3 -- src/Database/Barry/Model.php | 4 +++ src/Database/Barry/Relation.php | 7 +++- src/Database/Barry/Relations/BelongsTo.php | 7 +++- src/Database/Barry/Relations/HasMany.php | 1 - src/Database/Barry/Relations/HasOne.php | 9 +++-- .../Relation/BelongsToRelationQueryTest.php | 34 +++++++++++++++++++ 7 files changed, 57 insertions(+), 8 deletions(-) diff --git a/src/Console/Command/MigrationCommand.php b/src/Console/Command/MigrationCommand.php index ccc1c362..01a60615 100644 --- a/src/Console/Command/MigrationCommand.php +++ b/src/Console/Command/MigrationCommand.php @@ -58,13 +58,10 @@ public function reset(): void */ private function factory(string $type): void { - $migrations = $this->collectMigrationFiles(); - $connection = $this->arg->getParameter("--connection", config("database.default")); - try { Database::connection($connection); } catch (Exception $exception) { diff --git a/src/Database/Barry/Model.php b/src/Database/Barry/Model.php index 6102128b..9ad2c72f 100644 --- a/src/Database/Barry/Model.php +++ b/src/Database/Barry/Model.php @@ -1020,6 +1020,10 @@ private function executeDataCasting(string $name): mixed $type = $this->casts[$name]; $value = $this->attributes[$name]; + if (is_null($value)) { + return $value; + } + if ($type === "date") { return new Carbon($value); } diff --git a/src/Database/Barry/Relation.php b/src/Database/Barry/Relation.php index 2b1bfa92..2a7e97b8 100644 --- a/src/Database/Barry/Relation.php +++ b/src/Database/Barry/Relation.php @@ -62,7 +62,12 @@ public function __construct(Model $related, Model $parent) $this->parent = $parent; $this->related = $related; - $this->query = $this->related::query(); + // Clone the model's shared static query builder so the constraints we + // apply below stay local to this relation. Without the clone, a relation + // that builds constraints but does not execute the query (e.g. a cache + // hit in BelongsTo/HasOne) would leave a pending WHERE clause on the + // shared builder and corrupt the next relation query on the same model. + $this->query = clone $this->related::query(); // Build the constraint effect if (static::$has_constraints) { diff --git a/src/Database/Barry/Relations/BelongsTo.php b/src/Database/Barry/Relations/BelongsTo.php index aa1cebd5..3bf8b83e 100644 --- a/src/Database/Barry/Relations/BelongsTo.php +++ b/src/Database/Barry/Relations/BelongsTo.php @@ -38,7 +38,12 @@ public function __construct( */ public function getResults(): mixed { - $key = $this->query->getTable() . ":" . $this->local_key . ":belongsto:" . $this->related->getTable() . ":" . $this->foreign_key; + // Include the parent's foreign key value in the cache key so each parent + // resolves to its own related model. Without it the key is identical for + // every parent and a loop would always return the first cached result. + $foreign_key_value = $this->parent->getAttribute($this->foreign_key); + $key = $this->query->getTable() . ":" . $this->local_key . ":belongsto:" + . $this->related->getTable() . ":" . $this->foreign_key . ":" . $foreign_key_value; $cache = Cache::store('file')->get($key); diff --git a/src/Database/Barry/Relations/HasMany.php b/src/Database/Barry/Relations/HasMany.php index 43ec1bda..1ce65b77 100644 --- a/src/Database/Barry/Relations/HasMany.php +++ b/src/Database/Barry/Relations/HasMany.php @@ -17,7 +17,6 @@ class HasMany extends Relation * @param Model $parent * @param string $foreign_key * @param string $local_key - * @param string $relation */ public function __construct(Model $related, Model $parent, string $foreign_key, string $local_key) { diff --git a/src/Database/Barry/Relations/HasOne.php b/src/Database/Barry/Relations/HasOne.php index cb47d921..850b481e 100644 --- a/src/Database/Barry/Relations/HasOne.php +++ b/src/Database/Barry/Relations/HasOne.php @@ -33,7 +33,12 @@ public function __construct(Model $related, Model $parent, string $foreign_key, */ public function getResults(): ?Model { - $key = $this->query->getTable() . ":" . $this->local_key . ":hasone:" . $this->related->getTable() . ":" . $this->foreign_key; + // Include the parent's local key value in the cache key so each parent + // resolves to its own related model. Without it the key is identical for + // every parent and a loop would always return the first cached result. + $local_key_value = $this->parent->getAttribute($this->local_key); + $key = $this->query->getTable() . ":" . $this->local_key . ":hasone:" + . $this->related->getTable() . ":" . $this->foreign_key . ":" . $local_key_value; $cache = Cache::store('file')->get($key); @@ -46,7 +51,7 @@ public function getResults(): ?Model $result = $this->query->first(); if (!is_null($result)) { - Cache::store('file')->add($key, $result->toArray(), 60); + Cache::store('file')->set($key, $result->toArray(), 60); } return $result; diff --git a/tests/Database/Relation/BelongsToRelationQueryTest.php b/tests/Database/Relation/BelongsToRelationQueryTest.php index ab1114d8..7da176a9 100644 --- a/tests/Database/Relation/BelongsToRelationQueryTest.php +++ b/tests/Database/Relation/BelongsToRelationQueryTest.php @@ -174,6 +174,40 @@ public function test_multiple_relationship_accesses(string $name) $this->assertEquals($master1->name, $master2->name); } + /** + * @dataProvider connectionNames + */ + public function test_relationship_in_loop_returns_correct_owner_per_pet(string $name) + { + $this->executeMigration($name); + $this->seedTestData($name); + + // Ensure no stale relation cache leaks between pets + Cache::store('file')->clear(); + + $expected = [ + 1 => 'didi', // fluffy -> master 1 + 2 => 'didi', // dolly -> master 1 + 3 => 'john', // rex -> master 2 + 4 => 'john', // max -> master 2 + 5 => 'jane', // bella -> master 3 + ]; + + $pets = PetModelStub::connection($name)->all(); + + foreach ($pets as $pet) { + $master = $pet->master; + + $this->assertInstanceOf(PetMasterModelStub::class, $master); + $this->assertEquals( + $expected[$pet->id], + $master->name, + "Pet #{$pet->id} should belong to master '{$expected[$pet->id]}'" + ); + $this->assertEquals($pet->master_id, $master->id); + } + } + // ===== Relationship Data Integrity Tests ===== /**