diff --git a/src/Database/Barry/Concerns/Relationship.php b/src/Database/Barry/Concerns/Relationship.php index 4ef649c4..8f76c054 100644 --- a/src/Database/Barry/Concerns/Relationship.php +++ b/src/Database/Barry/Concerns/Relationship.php @@ -17,19 +17,19 @@ trait Relationship * * @param string $related * @param string|null $foreign_key - * @param string|null $local_key + * @param string|null $primary_key * @return BelongsTo */ public function belongsTo( string $related, ?string $foreign_key = null, - ?string $local_key = null + ?string $primary_key = null ): BelongsTo { // Create the new instance of model from container $related_model = app()->make($related); - if (is_null($local_key)) { - $local_key = $this->getKey(); + if (is_null($primary_key)) { + $primary_key = $this->getKey(); } // We build here the foreign key name @@ -37,7 +37,7 @@ public function belongsTo( $foreign_key = rtrim($related_model->getTable(), 's') . '_id'; } - return new BelongsTo($related_model, $this, $foreign_key, $local_key); + return new BelongsTo($related_model, $this, $foreign_key, $primary_key); } /** @@ -71,15 +71,15 @@ public function belongsToMany( * The has many relative * * @param string $related - * @param string|null $primary_key * @param string|null $foreign_key + * @param string|null $primary_key * @return HasMany * @throws QueryBuilderException */ public function hasMany( string $related, - ?string $primary_key = null, - ?string $foreign_key = null + ?string $foreign_key = null, + ?string $primary_key = null ): HasMany { $related_model = app()->make($related); @@ -87,12 +87,14 @@ public function hasMany( $primary_key = $this->getKey(); } - // We build the foreign key name + // The foreign key lives on the related table but is named after the + // parent (e.g. User hasMany Post => posts.user_id), so derive it from + // $this, not from the related model. if (is_null($foreign_key)) { - $foreign_key = rtrim($related_model->getTable(), 's') . '_id'; + $foreign_key = rtrim($this->getTable(), 's') . '_id'; } - return new HasMany($related_model, $this, $primary_key, $foreign_key); + return new HasMany($related_model, $this, $foreign_key, $primary_key); } /** @@ -114,9 +116,11 @@ public function hasOne( $primary_key = $this->getKey(); } - // We build the foreign key name + // The foreign key lives on the related table but is named after the + // parent (e.g. User hasOne Profile => profiles.user_id), so derive it + // from $this, not from the related model. if (is_null($foreign_key)) { - $foreign_key = rtrim($related_model->getTable(), 's') . '_id'; + $foreign_key = rtrim($this->getTable(), 's') . '_id'; } return new HasOne($related_model, $this, $foreign_key, $primary_key); diff --git a/src/Database/Barry/Relation.php b/src/Database/Barry/Relation.php index 5a365f24..5035f577 100644 --- a/src/Database/Barry/Relation.php +++ b/src/Database/Barry/Relation.php @@ -36,7 +36,7 @@ abstract class Relation * * @var string */ - protected string $local_key; + protected string $primary_key; /** * The parent model instance diff --git a/src/Database/Barry/Relations/BelongsTo.php b/src/Database/Barry/Relations/BelongsTo.php index 06f18f08..43b2db17 100644 --- a/src/Database/Barry/Relations/BelongsTo.php +++ b/src/Database/Barry/Relations/BelongsTo.php @@ -16,15 +16,15 @@ class BelongsTo extends Relation * @param Model $related * @param Model $parent * @param string $foreign_key - * @param string $local_key + * @param string $primary_key */ public function __construct( Model $related, Model $parent, string $foreign_key, - string $local_key + string $primary_key ) { - $this->local_key = $local_key; + $this->primary_key = $primary_key; $this->foreign_key = $foreign_key; parent::__construct($related, $parent); @@ -58,7 +58,7 @@ public function addConstraints(): void // or has many relationships, we need to actually query on the primary key // of the related models matching on the foreign key that's on a parent. $foreign_key_value = $this->parent->getAttribute($this->foreign_key); - $this->query->where($this->local_key, '=', $foreign_key_value); + $this->query->where($this->primary_key, '=', $foreign_key_value); } /** @@ -74,7 +74,7 @@ protected function eagerParentKey(): string */ protected function eagerRelatedKey(): string { - return $this->local_key; + return $this->primary_key; } /** diff --git a/src/Database/Barry/Relations/BelongsToMany.php b/src/Database/Barry/Relations/BelongsToMany.php index 4b0ad159..3434489d 100644 --- a/src/Database/Barry/Relations/BelongsToMany.php +++ b/src/Database/Barry/Relations/BelongsToMany.php @@ -17,11 +17,11 @@ class BelongsToMany extends Relation * @param Model $related * @param Model $parent * @param string $foreign_key - * @param string $local_key + * @param string $primary_key */ - public function __construct(Model $related, Model $parent, string $foreign_key, string $local_key) + public function __construct(Model $related, Model $parent, string $foreign_key, string $primary_key) { - $this->local_key = $local_key; + $this->primary_key = $primary_key; $this->foreign_key = $foreign_key; parent::__construct($related, $parent); @@ -53,7 +53,7 @@ public function addConstraints(): void // or has many relationships, we need to actually query on the primary key // of the related models matching on the foreign key that's on a parent. $foreign_key_value = $this->parent->getAttribute($this->foreign_key); - $this->query->where($this->local_key, '=', $foreign_key_value); + $this->query->where($this->primary_key, '=', $foreign_key_value); } /** @@ -69,7 +69,7 @@ protected function eagerParentKey(): string */ protected function eagerRelatedKey(): string { - return $this->local_key; + return $this->primary_key; } /** diff --git a/src/Database/Barry/Relations/HasMany.php b/src/Database/Barry/Relations/HasMany.php index 357b7d6f..bf16b4c7 100644 --- a/src/Database/Barry/Relations/HasMany.php +++ b/src/Database/Barry/Relations/HasMany.php @@ -16,11 +16,11 @@ class HasMany extends Relation * @param Model $related * @param Model $parent * @param string $foreign_key - * @param string $local_key + * @param string $primary_key */ - public function __construct(Model $related, Model $parent, string $foreign_key, string $local_key) + public function __construct(Model $related, Model $parent, string $foreign_key, string $primary_key) { - $this->local_key = $local_key; + $this->primary_key = $primary_key; $this->foreign_key = $foreign_key; parent::__construct($related, $parent); @@ -43,10 +43,17 @@ public function getResults(): Collection */ public function addConstraints(): void { - // 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()); + if (!static::$has_constraints) { + return; + } + + // Match the related foreign key column against the parent's local key. + // foreign_key is the column on the related table; primary_key is the + // referenced column on the parent (its primary key by default). + $this->query = $this->query->where( + $this->foreign_key, + $this->parent->getAttribute($this->primary_key) + ); } /** @@ -54,7 +61,7 @@ public function addConstraints(): void */ protected function eagerParentKey(): string { - return $this->parent->getKey(); + return $this->primary_key; } /** @@ -62,7 +69,7 @@ protected function eagerParentKey(): string */ protected function eagerRelatedKey(): string { - return $this->local_key; + return $this->foreign_key; } /** diff --git a/src/Database/Barry/Relations/HasOne.php b/src/Database/Barry/Relations/HasOne.php index 60725b87..e569f1da 100644 --- a/src/Database/Barry/Relations/HasOne.php +++ b/src/Database/Barry/Relations/HasOne.php @@ -15,11 +15,11 @@ class HasOne extends Relation * @param Model $related * @param Model $parent * @param string $foreign_key - * @param string $local_key + * @param string $primary_key */ - public function __construct(Model $related, Model $parent, string $foreign_key, string $local_key) + public function __construct(Model $related, Model $parent, string $foreign_key, string $primary_key) { - $this->local_key = $local_key; + $this->primary_key = $primary_key; $this->foreign_key = $foreign_key; parent::__construct($related, $parent); @@ -48,7 +48,7 @@ public function addConstraints(): void return; } - $this->query = $this->query->where($this->foreign_key, $this->parent->getAttribute($this->local_key)); + $this->query = $this->query->where($this->foreign_key, $this->parent->getAttribute($this->primary_key)); } /** @@ -56,7 +56,7 @@ public function addConstraints(): void */ protected function eagerParentKey(): string { - return $this->local_key; + return $this->primary_key; } /** diff --git a/tests/Database/Relation/HasManyKeyOrderTest.php b/tests/Database/Relation/HasManyKeyOrderTest.php new file mode 100644 index 00000000..f6056867 --- /dev/null +++ b/tests/Database/Relation/HasManyKeyOrderTest.php @@ -0,0 +1,80 @@ + 7]); + + // (related, foreign_key, local_key) + $relation = $master->hasMany(PetModelStub::class, 'master_id', 'id'); + + $this->assertStringContainsString( + 'where master_id = ?', + strtolower($relation->toSql()) + ); + } + + public function test_hasmany_default_keys_filter_on_foreign_key_column() + { + $master = new PetMasterModelStub(['id' => 7]); + + // No keys: the foreign key defaults to the parent-derived column that + // lives on the related table (pet_masters -> pet_master_id), NOT the + // related table's own key (pet_id) nor the parent primary key (id). + $relation = $master->hasMany(PetModelStub::class); + + $sql = strtolower($relation->toSql()); + + $this->assertStringContainsString('where pet_master_id = ?', $sql); + $this->assertStringNotContainsString('where pet_id = ?', $sql); + $this->assertStringNotContainsString('where id = ?', $sql); + } + + public function test_hasone_and_hasmany_take_keys_in_the_same_order() + { + $master = new PetMasterModelStub(['id' => 7]); + + $one = $master->hasOne(PetModelStub::class, 'master_id', 'id'); + $many = $master->hasMany(PetModelStub::class, 'master_id', 'id'); + + $this->assertStringContainsString('where master_id = ?', strtolower($one->toSql())); + $this->assertStringContainsString('where master_id = ?', strtolower($many->toSql())); + } +} diff --git a/tests/Database/Stubs/PetMasterModelStub.php b/tests/Database/Stubs/PetMasterModelStub.php index fad409bd..d8c986c2 100644 --- a/tests/Database/Stubs/PetMasterModelStub.php +++ b/tests/Database/Stubs/PetMasterModelStub.php @@ -30,7 +30,7 @@ class PetMasterModelStub extends \Bow\Database\Barry\Model */ public function pets(): HasMany { - return $this->hasMany(PetModelStub::class, 'id', 'master_id'); + return $this->hasMany(PetModelStub::class, 'master_id', 'id'); } /**