diff --git a/ROADMAP.md b/ROADMAP.md index fc5345a7..077d513e 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1,7 +1,7 @@ # BowPHP Framework Roadmap > Living document based on source code analysis (5.x branch) and the project manifesto. -> Last updated: May 2026 +> Last updated: June 2026 --- @@ -68,6 +68,11 @@ Highlights from the latest iterations — already merged into `5.x`. Full detail ### Barry ORM +* **Eager loading** via `Builder::eager(string|array $relations)` — batches related models in a single `WHERE IN` query (solves N+1), supports `hasOne` / `hasMany` / `belongsTo` / `belongsToMany`; the eager list is reset after each `get()` so it can't leak into the next query on the shared builder. +* Relations now **lazy-load once per model instance** and keep the result in memory (`Model::setRelation` + private `$relations`), replacing the previous file-based cache in `BelongsTo` / `HasOne` (fixes stale and cross-parent cache bugs). +* Fixed `belongsTo` resolution inside loops — each parent now resolves its own related model instead of always returning the first. +* Fixed `HasMany::addConstraints()` to filter on the foreign-key column (was incorrectly using the parent primary key). +* Deep casting now applied when `toArray()` / `toJson()` / `__toString()` are called. * `SoftDelete` trait (`delete` → `deleted_at`, `restore`, `forceDelete`, `withTrashed` / `onlyTrashed` / `withoutTrashed`, events `model.restoring/restored/forceDeleting/forceDeleted`). * Fixed `array` cast: no longer returns `stdClass`. * Removed dead `$soft_delete` property (replaced by the trait). @@ -112,7 +117,7 @@ Highlights from the latest iterations — already merged into `5.x`. Full detail | Separate unit tests from integration tests | ⏳ Planned | High | DB/FTP/S3 tests require external services | | Add PHPUnit `@group` annotations | ⏳ Planned | High | `@group unit`, `@group integration`, `@group database` | | Configure GitHub Actions with Docker services | ⏳ Planned | High | MySQL, PostgreSQL, Redis for CI | -| Increase unit test coverage | 🔄 Ongoing | Medium | 1,600+ tests, 0 logical failures. Recent additions: SoftDelete, AttributeRouteRegistrar, new validation rules, Pagination | +| Increase unit test coverage | 🔄 Ongoing | Medium | 1,600+ tests, 0 logical failures. Recent additions: eager loading (`EagerLoadingQueryTest`), SoftDelete, AttributeRouteRegistrar, new validation rules, Pagination | | Integrate PHPStan level 5+ | ⏳ Planned | Medium | Current constraint: `phpstan/phpstan: ^0.12.87` — upgrade to ^1.x before targeting higher levels | ### Code Fixes diff --git a/src/Database/Barry/Builder.php b/src/Database/Barry/Builder.php index 1c75b2b7..08fad8f1 100644 --- a/src/Database/Barry/Builder.php +++ b/src/Database/Barry/Builder.php @@ -18,6 +18,26 @@ class Builder extends QueryBuilder */ protected ?string $model = null; + /** + * The relationships to eager load. + * + * @var array + */ + protected array $eager_loads = []; + + /** + * Register relationships to eager load on the query result. + * + * @param string|array $relations + * @return Builder + */ + public function eager(string|array $relations): Builder + { + $this->eager_loads = array_merge($this->eager_loads, (array)$relations); + + return $this; + } + /** * Get information * @@ -28,6 +48,11 @@ public function get(array $columns = []): Model|Collection|null { $data = parent::get($columns); + // Read and reset the eager loads now: query() memoizes a shared Builder + // instance, so the list must not leak into the next query on this model. + $eager_loads = $this->eager_loads; + $this->eager_loads = []; + if (is_null($data)) { return null; } @@ -41,9 +66,36 @@ public function get(array $columns = []): Model|Collection|null $data[$key] = new $this->model((array)$value); } + if (count($eager_loads) > 0) { + $this->eagerLoadRelations($data, $eager_loads); + } + return new Collection($data); } + /** + * Eager load the given relationships onto a set of parent models. + * + * @param Model[] $models + * @param array $relations + * @return void + */ + protected function eagerLoadRelations(array $models, array $relations): void + { + if (count($models) === 0) { + return; + } + + foreach ($relations as $name) { + // Build the relation without the single parent constraint so it can + // be batched across every parent with one whereIn query. + $relation = Relation::noConstraints(fn () => $models[0]->$name()); + + $relation->addEagerConstraints($models); + $relation->match($models, $relation->getEager(), $name); + } + } + /** * Check if rows exists * diff --git a/src/Database/Barry/Model.php b/src/Database/Barry/Model.php index 9ad2c72f..8361d7f5 100644 --- a/src/Database/Barry/Model.php +++ b/src/Database/Barry/Model.php @@ -191,6 +191,13 @@ abstract class Model implements ArrayAccess, JsonSerializable */ private array $original = []; + /** + * The loaded relationships, resolved lazily once per model instance. + * + * @var array + */ + private array $relations = []; + /** * Model constructor. * @@ -880,6 +887,20 @@ public function getAttribute(string $key): mixed return $this->attributes[$key] ?? null; } + /** + * Set a loaded relationship on the model's in-memory store. + * + * Used by eager loading to pre-populate a relation so a later access + * resolves from memory instead of issuing a query. + * + * @param string $name + * @param mixed $value + */ + public function setRelation(string $name, mixed $value): void + { + $this->relations[$name] = $value; + } + /** * Returns the data * @@ -919,8 +940,20 @@ public function __get(string $name): mixed $attribute_exists = isset($this->attributes[$name]); if (!$attribute_exists && method_exists($this, $name)) { + // Lazy-load once: a relation is resolved a single time per model + // instance and its result kept in memory. Repeated access returns + // the same loaded object instead of re-querying or re-hydrating. + if (array_key_exists($name, $this->relations)) { + return $this->relations[$name]; + } + $result = $this->$name(); - return $result instanceof Relation ? $result->getResults() : $result; + + if ($result instanceof Relation) { + return $this->relations[$name] = $result->getResults(); + } + + return $result; } if (!$attribute_exists) { diff --git a/src/Database/Barry/Relation.php b/src/Database/Barry/Relation.php index 2a7e97b8..bc9ab610 100644 --- a/src/Database/Barry/Relation.php +++ b/src/Database/Barry/Relation.php @@ -4,6 +4,8 @@ namespace Bow\Database\Barry; +use Closure; +use Bow\Database\Collection; use Bow\Database\QueryBuilder; abstract class Relation @@ -139,4 +141,103 @@ public function create(array $attributes): Model * @return mixed */ abstract public function getResults(): mixed; + + /** + * The parent attribute whose value is matched against the related models. + * + * @return string + */ + abstract protected function eagerParentKey(): string; + + /** + * The related column queried when eager loading the relation. + * + * @return string + */ + abstract protected function eagerRelatedKey(): string; + + /** + * Whether the relation resolves to many related models. + * + * @return bool + */ + abstract protected function eagerIsMany(): bool; + + /** + * Run the given callback with relation constraints disabled. + * + * Lets the eager loader build a relation without the single parent WHERE + * clause so it can be replaced by a batched whereIn over every parent. + * + * @param Closure $callback + * @return mixed + */ + public static function noConstraints(Closure $callback): mixed + { + $previous = static::$has_constraints; + static::$has_constraints = false; + + try { + return $callback(); + } finally { + static::$has_constraints = $previous; + } + } + + /** + * Constrain the relation query to every parent key in a single whereIn. + * + * @param Model[] $parents + * @return void + */ + public function addEagerConstraints(array $parents): void + { + $keys = array_values(array_unique(array_filter( + array_map(fn (Model $parent) => $parent->getAttribute($this->eagerParentKey()), $parents), + fn ($value) => !is_null($value) + ))); + + // Fall back to an impossible match when no parent exposes a key so the + // query stays well-formed and returns nothing. + $this->query->whereIn($this->eagerRelatedKey(), count($keys) > 0 ? $keys : [0]); + } + + /** + * Execute the eager query and return the related models. + * + * @return Collection + */ + public function getEager(): Collection + { + $results = $this->query->get(); + + return $results instanceof Collection ? $results : new Collection([]); + } + + /** + * Match the eager loaded related models back onto their parents. + * + * @param Model[] $parents + * @param Collection $results + * @param string $name + * @return void + */ + public function match(array $parents, Collection $results, string $name): void + { + $dictionary = []; + + foreach ($results as $related) { + $dictionary[$related->getAttribute($this->eagerRelatedKey())][] = $related; + } + + foreach ($parents as $parent) { + $key = $parent->getAttribute($this->eagerParentKey()); + $matched = $dictionary[$key] ?? []; + + $parent->setRelation( + $name, + $this->eagerIsMany() ? new Collection($matched) : ($matched[0] ?? null) + ); + } + } } diff --git a/src/Database/Barry/Relations/BelongsTo.php b/src/Database/Barry/Relations/BelongsTo.php index 3bf8b83e..06f18f08 100644 --- a/src/Database/Barry/Relations/BelongsTo.php +++ b/src/Database/Barry/Relations/BelongsTo.php @@ -4,7 +4,6 @@ namespace Bow\Database\Barry\Relations; -use Bow\Cache\Cache; use Bow\Database\Barry\Model; use Bow\Database\Barry\Relation; use Bow\Database\Exception\QueryBuilderException; @@ -38,28 +37,9 @@ public function __construct( */ public function getResults(): mixed { - // 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); - - if (!is_null($cache)) { - $related = new $this->related(); - $related->setAttributes($cache); - return $related; - } - - $result = $this->query->first(); - - if (!is_null($result)) { - Cache::store('file')->set($key, $result->toArray(), 500); - } - - return $result; + // The result is lazy-loaded once per parent model instance and kept in + // memory by the model itself, so a plain query is enough here. + return $this->query->first(); } /** @@ -80,4 +60,28 @@ public function addConstraints(): void $foreign_key_value = $this->parent->getAttribute($this->foreign_key); $this->query->where($this->local_key, '=', $foreign_key_value); } + + /** + * @inheritDoc + */ + protected function eagerParentKey(): string + { + return $this->foreign_key; + } + + /** + * @inheritDoc + */ + protected function eagerRelatedKey(): string + { + return $this->local_key; + } + + /** + * @inheritDoc + */ + protected function eagerIsMany(): bool + { + return false; + } } diff --git a/src/Database/Barry/Relations/BelongsToMany.php b/src/Database/Barry/Relations/BelongsToMany.php index 7e4bf0b8..4b0ad159 100644 --- a/src/Database/Barry/Relations/BelongsToMany.php +++ b/src/Database/Barry/Relations/BelongsToMany.php @@ -55,4 +55,28 @@ public function addConstraints(): void $foreign_key_value = $this->parent->getAttribute($this->foreign_key); $this->query->where($this->local_key, '=', $foreign_key_value); } + + /** + * @inheritDoc + */ + protected function eagerParentKey(): string + { + return $this->foreign_key; + } + + /** + * @inheritDoc + */ + protected function eagerRelatedKey(): string + { + return $this->local_key; + } + + /** + * @inheritDoc + */ + protected function eagerIsMany(): bool + { + return true; + } } diff --git a/src/Database/Barry/Relations/HasMany.php b/src/Database/Barry/Relations/HasMany.php index 1ce65b77..357b7d6f 100644 --- a/src/Database/Barry/Relations/HasMany.php +++ b/src/Database/Barry/Relations/HasMany.php @@ -43,6 +43,33 @@ public function getResults(): Collection */ public function addConstraints(): void { - $this->query = $this->query->where($this->foreign_key, $this->parent->getKeyValue()); + // Match the related foreign key column against the parent's primary key. + // local_key holds the foreign key column name; foreign_key holds the + // parent primary key name, so filtering must use local_key here. + $this->query = $this->query->where($this->local_key, $this->parent->getKeyValue()); + } + + /** + * @inheritDoc + */ + protected function eagerParentKey(): string + { + return $this->parent->getKey(); + } + + /** + * @inheritDoc + */ + protected function eagerRelatedKey(): string + { + return $this->local_key; + } + + /** + * @inheritDoc + */ + protected function eagerIsMany(): bool + { + return true; } } diff --git a/src/Database/Barry/Relations/HasOne.php b/src/Database/Barry/Relations/HasOne.php index 850b481e..60725b87 100644 --- a/src/Database/Barry/Relations/HasOne.php +++ b/src/Database/Barry/Relations/HasOne.php @@ -4,7 +4,6 @@ namespace Bow\Database\Barry\Relations; -use Bow\Cache\Cache; use Bow\Database\Barry\Model; use Bow\Database\Barry\Relation; @@ -33,28 +32,9 @@ public function __construct(Model $related, Model $parent, string $foreign_key, */ public function getResults(): ?Model { - // 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); - - if (!is_null($cache)) { - $related = new $this->related(); - $related->setAttributes($cache); - return $related; - } - - $result = $this->query->first(); - - if (!is_null($result)) { - Cache::store('file')->set($key, $result->toArray(), 60); - } - - return $result; + // The result is lazy-loaded once per parent model instance and kept in + // memory by the model itself, so a plain query is enough here. + return $this->query->first(); } /** @@ -70,4 +50,28 @@ public function addConstraints(): void $this->query = $this->query->where($this->foreign_key, $this->parent->getAttribute($this->local_key)); } + + /** + * @inheritDoc + */ + protected function eagerParentKey(): string + { + return $this->local_key; + } + + /** + * @inheritDoc + */ + protected function eagerRelatedKey(): string + { + return $this->foreign_key; + } + + /** + * @inheritDoc + */ + protected function eagerIsMany(): bool + { + return false; + } } diff --git a/tests/Database/Relation/BelongsToRelationQueryTest.php b/tests/Database/Relation/BelongsToRelationQueryTest.php index 7da176a9..ca6c3cec 100644 --- a/tests/Database/Relation/BelongsToRelationQueryTest.php +++ b/tests/Database/Relation/BelongsToRelationQueryTest.php @@ -172,6 +172,10 @@ public function test_multiple_relationship_accesses(string $name) $this->assertInstanceOf(PetMasterModelStub::class, $master2); $this->assertEquals($master1->id, $master2->id); $this->assertEquals($master1->name, $master2->name); + + // Lazy-load once: repeated access on the same instance must resolve the + // relation a single time and return the very same loaded object. + $this->assertSame($master1, $master2); } /** diff --git a/tests/Database/Relation/EagerLoadingQueryTest.php b/tests/Database/Relation/EagerLoadingQueryTest.php new file mode 100644 index 00000000..3cbd5abb --- /dev/null +++ b/tests/Database/Relation/EagerLoadingQueryTest.php @@ -0,0 +1,299 @@ +connection($name)->dropIfExists("pets", false); + $migration->connection($name)->dropIfExists("pet_masters", false); + } catch (\Exception $e) { + // Ignore errors during cleanup + } + } + } + + private function executeMigration(string $name): void + { + $migration = new MigrationExtendedStub(); + $migration->connection($name)->dropIfExists("pets", false); + $migration->connection($name)->dropIfExists("pet_masters", false); + + $migration->connection($name)->create("pet_masters", function (Table $table) { + $table->addIncrement("id"); + $table->addString("name"); + }, false); + + $migration->connection($name)->create("pets", function (Table $table) { + $table->addIncrement("id"); + $table->addString("name"); + $table->addInteger("master_id"); + $table->addForeign("master_id", [ + "table" => "pet_masters", + "references" => "id", + "on" => "delete cascade" + ]); + }, false); + } + + private function seedTestData(string $name): void + { + Database::connection($name)->statement("INSERT INTO pet_masters VALUES (1, 'didi'), (2, 'john'), (3, 'jane')"); + Database::connection($name)->statement("INSERT INTO pets VALUES (1, 'fluffy', 1), (2, 'dolly', 1), (3, 'rex', 2), (4, 'max', 2), (5, 'bella', 3)"); + } + + // ===== belongsTo ===== + + /** + * @dataProvider connectionNames + */ + public function test_eager_belongs_to_matches_each_parent(string $name) + { + $this->executeMigration($name); + $this->seedTestData($name); + + $expected = [1 => 'didi', 2 => 'didi', 3 => 'john', 4 => 'john', 5 => 'jane']; + + $pets = PetModelStub::connection($name)->eager('master')->get(); + + $this->assertInstanceOf(Collection::class, $pets); + + foreach ($pets as $pet) { + $master = $pet->master; + $this->assertInstanceOf(PetMasterModelStub::class, $master); + $this->assertEquals($expected[$pet->id], $master->name); + $this->assertEquals($pet->master_id, $master->id); + } + } + + /** + * Eager loading must pre-populate the relation so accessing it issues no + * further query. We prove this by removing the related rows after the eager + * load: a lazy fetch would now find nothing, an eager one still has the data. + * + * @dataProvider connectionNames + */ + public function test_eager_belongs_to_avoids_followup_query(string $name) + { + $this->executeMigration($name); + $this->seedTestData($name); + + $pets = PetModelStub::connection($name)->eager('master')->get(); + + // Wipe the related table; an eager-loaded relation is unaffected. + Database::connection($name)->statement("DELETE FROM pets"); + Database::connection($name)->statement("DELETE FROM pet_masters"); + + foreach ($pets as $pet) { + $master = $pet->master; + $this->assertInstanceOf(PetMasterModelStub::class, $master); + $this->assertEquals($pet->master_id, $master->id); + } + } + + /** + * @dataProvider connectionNames + */ + public function test_eager_returns_same_instance_on_repeat_access(string $name) + { + $this->executeMigration($name); + $this->seedTestData($name); + + $pets = PetModelStub::connection($name)->eager('master')->get(); + $pet = $pets->first(); + + $this->assertSame($pet->master, $pet->master); + } + + // ===== hasMany ===== + + /** + * @dataProvider connectionNames + */ + public function test_eager_has_many_matches_each_parent(string $name) + { + $this->executeMigration($name); + $this->seedTestData($name); + + $expected = [1 => ['fluffy', 'dolly'], 2 => ['rex', 'max'], 3 => ['bella']]; + + $masters = PetMasterModelStub::connection($name)->eager('pets')->get(); + + Database::connection($name)->statement("DELETE FROM pets"); + + foreach ($masters as $master) { + $pets = $master->pets; + $this->assertInstanceOf(Collection::class, $pets); + $names = array_map(fn ($pet) => $pet->name, $pets->all()); + sort($names); + $want = $expected[$master->id]; + sort($want); + $this->assertEquals($want, $names); + } + } + + // ===== hasOne ===== + + /** + * @dataProvider connectionNames + */ + public function test_eager_has_one_matches_each_parent(string $name) + { + $this->executeMigration($name); + $this->seedTestData($name); + + $masters = PetMasterModelStub::connection($name)->eager('firstPet')->get(); + + Database::connection($name)->statement("DELETE FROM pets"); + + foreach ($masters as $master) { + $pet = $master->firstPet; + $this->assertInstanceOf(PetModelStub::class, $pet); + $this->assertEquals($master->id, $pet->master_id); + } + } + + // ===== belongsToMany ===== + + /** + * @dataProvider connectionNames + */ + public function test_eager_belongs_to_many_matches_each_parent(string $name) + { + $this->executeMigration($name); + $this->seedTestData($name); + + $expected = [1 => 2, 2 => 2, 3 => 1]; + + $masters = PetMasterModelStub::connection($name)->eager('manyPets')->get(); + + Database::connection($name)->statement("DELETE FROM pets"); + + foreach ($masters as $master) { + $pets = $master->manyPets; + $this->assertInstanceOf(Collection::class, $pets); + $this->assertCount($expected[$master->id], $pets->all()); + } + } + + // ===== multiple names ===== + + /** + * @dataProvider connectionNames + */ + public function test_eager_multiple_relations_in_one_call(string $name) + { + $this->executeMigration($name); + $this->seedTestData($name); + + $masters = PetMasterModelStub::connection($name)->eager(['pets', 'firstPet'])->get(); + + Database::connection($name)->statement("DELETE FROM pets"); + + foreach ($masters as $master) { + $this->assertInstanceOf(Collection::class, $master->pets); + $this->assertInstanceOf(PetModelStub::class, $master->firstPet); + } + } + + // ===== edge cases ===== + + /** + * @dataProvider connectionNames + */ + public function test_eager_with_no_related_rows(string $name) + { + $this->executeMigration($name); + Database::connection($name)->statement("INSERT INTO pet_masters VALUES (1, 'didi')"); + + $masters = PetMasterModelStub::connection($name)->eager('pets')->get(); + + foreach ($masters as $master) { + $pets = $master->pets; + $this->assertInstanceOf(Collection::class, $pets); + $this->assertCount(0, $pets->all()); + } + } + + /** + * Guards the HasMany::addConstraints() fix: lazy access must filter on the + * foreign key column (master_id), not the parent primary key. + * + * @dataProvider connectionNames + */ + public function test_lazy_has_many_filters_on_foreign_key(string $name) + { + $this->executeMigration($name); + $this->seedTestData($name); + + $master = PetMasterModelStub::connection($name)->retrieve(2); + $pets = $master->pets; + + $names = array_map(fn ($pet) => $pet->name, $pets->all()); + sort($names); + + $this->assertEquals(['max', 'rex'], $names); + } + + /** + * The eager list must not leak onto a subsequent query reusing the shared + * builder instance. + * + * @dataProvider connectionNames + */ + public function test_eager_does_not_leak_into_next_query(string $name) + { + $this->executeMigration($name); + $this->seedTestData($name); + + PetModelStub::connection($name)->eager('master')->get(); + + // A plain fetch afterwards must not attempt to eager load anything. + $pets = PetModelStub::connection($name)->all(); + + $this->assertInstanceOf(Collection::class, $pets); + $this->assertCount(5, $pets->all()); + } +} diff --git a/tests/Database/Stubs/PetMasterModelStub.php b/tests/Database/Stubs/PetMasterModelStub.php index 7ce52aec..fad409bd 100644 --- a/tests/Database/Stubs/PetMasterModelStub.php +++ b/tests/Database/Stubs/PetMasterModelStub.php @@ -2,7 +2,9 @@ namespace Bow\Tests\Database\Stubs; +use Bow\Database\Barry\Relations\BelongsToMany; use Bow\Database\Barry\Relations\HasMany; +use Bow\Database\Barry\Relations\HasOne; class PetMasterModelStub extends \Bow\Database\Barry\Model { @@ -28,6 +30,26 @@ class PetMasterModelStub extends \Bow\Database\Barry\Model */ public function pets(): HasMany { - return $this->hasMany(PetModelStub::class); + return $this->hasMany(PetModelStub::class, 'id', 'master_id'); + } + + /** + * Get a single owned pet + * + * @return HasOne + */ + public function firstPet(): HasOne + { + return $this->hasOne(PetModelStub::class, 'master_id', 'id'); + } + + /** + * Get the list of pets through a belongs to many relation + * + * @return BelongsToMany + */ + public function manyPets(): BelongsToMany + { + return $this->belongsToMany(PetModelStub::class, 'id', 'master_id'); } }