fix(database): fix hasMany and hosOne relations

src/Database/Barry/Concerns/Relationship.php

@@ -17,27 +17,27 @@ 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
         if (is_null($foreign_key)) {
             $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,28 +71,30 @@ 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);
 
         if (is_null($primary_key)) {
             $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);

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

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;
     }
 
     /**

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;
     }
 
     /**

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,26 +43,33 @@ 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)
+        );
     }
 
     /**
      * @inheritDoc
      */
     protected function eagerParentKey(): string
     {
-        return $this->parent->getKey();
+        return $this->primary_key;
     }
 
     /**
      * @inheritDoc
      */
     protected function eagerRelatedKey(): string
     {
-        return $this->local_key;
+        return $this->foreign_key;
     }
 
     /**

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,15 +48,15 @@ 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));
     }
 
     /**
      * @inheritDoc
      */
     protected function eagerParentKey(): string
     {
-        return $this->local_key;
+        return $this->primary_key;
     }
 
     /**

tests/Database/Relation/HasManyKeyOrderTest.php

@@ -0,0 +1,80 @@
+<?php
+
+namespace Bow\Tests\Database\Relation;
+
+use Bow\Cache\Cache;
+use Bow\Database\Database;
+use Bow\Tests\Config\TestingConfiguration;
+use Bow\Tests\Database\Stubs\PetMasterModelStub;
+use Bow\Tests\Database\Stubs\PetModelStub;
+
+/**
+ * Locks in the hasMany() signature `(related, foreign_key, local_key)`, now
+ * consistent with hasOne()/belongsTo(): the foreign key is the column on the
+ * related table, the local key is the referenced column on the parent.
+ */
+class HasManyKeyOrderTest extends \PHPUnit\Framework\TestCase
+{
+    private static bool $configured = false;
+
+    public static function setUpBeforeClass(): void
+    {
+        if (!static::$configured) {
+            $config = TestingConfiguration::getConfig();
+            Database::configure($config["database"]);
+            Cache::configure($config["cache"]);
+            static::$configured = true;
+        }
+    }
+
+    public function setUp(): void
+    {
+        ob_start();
+        Database::connection('sqlite');
+    }
+
+    public function tearDown(): void
+    {
+        ob_get_clean();
+    }
+
+    public function test_hasmany_filters_on_foreign_key_with_new_arg_order()
+    {
+        $master = new PetMasterModelStub(['id' => 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()));
+    }
+}

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');
     }
 
     /**