Bugfix/has and belongs to many using wrong table to–check relations

lib/Model.php

@@ -342,7 +342,7 @@ class Model
      * Constructs a model.
      *
      * When a user instantiates a new object (e.g.: it was not ActiveRecord that instantiated via a find)
-     * then @var $attributes will be mapped according to the schema's defaults. Otherwise, the given
+     * then @var will $attributes be mapped according to the schema's defaults. Otherwise, the given
      * $attributes will be mapped via set_attributes_via_mass_assignment.
      *
      * ```

lib/Relationship/HasAndBelongsToMany.php

@@ -47,9 +47,11 @@ public function load(Model $model): mixed
          */
         $rel = new Relation($this->class_name, [], []);
         $rel->from($this->attribute_name);
-        $other_table = Table::load(get_class($model))->table;
-        $rel->where($other_table . '. ' . $this->options['foreign_key'] . ' = ?', $model->{$model->get_primary_key()});
-        $rel->joins([$other_table]);
+        $other_table = Table::load(get_class($model));
+        $other_table_name = $other_table->table;
+        $other_table_primary_key = $other_table->pk[0];
+        $rel->where($other_table_name . '.' . $other_table_primary_key . ' = ?', $model->{$model->get_primary_key()});
+        $rel->joins([$other_table_name]);
 
         return $rel->to_a();
     }
@@ -70,8 +72,12 @@ public function construct_inner_join_sql(Table $from_table, bool $using_through
         $foreign_key = $this->options['foreign_key'];
         $join_primary_key = $this->options['association_foreign_key'];
         $linkingTableName = $this->options['join_table'];
-        $res = 'INNER JOIN ' . $linkingTableName . " ON ($from_table_name.$foreign_key = " . $linkingTableName . ".$foreign_key) "
-            . 'INNER JOIN ' . $associated_table_name . ' ON ' . $associated_table_name . '.' . $join_primary_key . ' = ' . $linkingTableName . '.' . $join_primary_key;
+
+        $from_table_primary_key = $from_table->pk[0];
+        $associated_table_primary_key = $other_table->pk[0];
+
+        $res = 'INNER JOIN ' . $linkingTableName . " ON ($from_table_name.$from_table_primary_key = " . $linkingTableName . ".$foreign_key) "
+            . 'INNER JOIN ' . $associated_table_name . ' ON ' . $associated_table_name . '.' . $associated_table_primary_key . ' = ' . $linkingTableName . '.' . $join_primary_key;
 
         return $res;
     }

lib/Table.php

@@ -569,7 +569,7 @@ private function set_associations(): void
 
                     case 'has_and_belongs_to_many':
                         $definition['join_table'] ??= HasAndBelongsToMany::inferJoiningTableName($this->table, $attribute);
-                        $definition['foreign_key'] ??= $this->pk[0];
+                        $definition['foreign_key'] ??= Inflector::keyify(Utils::singularize($this->table));
                         $relationship = new HasAndBelongsToMany($attribute, $definition);
                         break;
                 }

test/RelationshipTest.php

@@ -20,7 +20,9 @@
 use test\models\Position;
 use test\models\Property;
 use test\models\Student;
+use test\models\Task;
 use test\models\Venue;
+use test\models\Worker;
 
 class NotModel
 {
@@ -308,6 +310,42 @@ public function testHasAndBelongsToManyDoesNotEagerLoad()
         $hasAndBelongsToMany->load_eagerly([], [], [], Table::load(Book::class));
     }
 
+    public function testHasAndBelongsToManyPrimaryKeyIsSameAsForeignKey()
+    {
+        $student = Student::find(1);
+        $courses = $student->courses;
+
+        $this->assertEquals(2, count($courses));
+        $this->assert_sql_includes('INNER JOIN courses_students ON (courses.course_id = courses_students.course_id) INNER JOIN students ON students.student_id = courses_students.student_id', Table::load(Course::class)->last_sql);
+    }
+
+    public function testHasAndBelongsToManyPrimaryKeyIsSameAsForeignKeyReverse()
+    {
+        $course = Course::find(1);
+        $students = $course->students;
+
+        $this->assertEquals(2, count($students));
+        $this->assert_sql_includes('INNER JOIN courses_students ON (students.student_id = courses_students.student_id) INNER JOIN courses ON courses.course_id = courses_students.course_id', Table::load(Student::class)->last_sql);
+    }
+
+    public function testHasAndBelongsToManyPrimaryKeyIsDifferentThanForeignKey()
+    {
+        $worker = Worker::find(1);
+        $tasks = $worker->tasks;
+
+        $this->assertEquals(1, count($tasks));
+        $this->assert_sql_includes('INNER JOIN tasks_workers ON (tasks.id = tasks_workers.task_id) INNER JOIN workers ON workers.id = tasks_workers.worker_id', Table::load(Task::class)->last_sql);
+    }
+
+    public function testHasAndBelongsToManyPrimaryKeyIsDifferentThanForeignKeyReverse()
+    {
+        $task = Task::find(1);
+        $workers = $task->workers;
+
+        $this->assertEquals(1, count($workers));
+        $this->assert_sql_includes('INNER JOIN tasks_workers ON (workers.id = tasks_workers.worker_id) INNER JOIN tasks ON tasks.id = tasks_workers.task_id', Table::load(Worker::class)->last_sql);
+    }
+
     public function testBelongsToCreateAssociation()
     {
         $event = Event::find(5);

test/fixtures/tasks.csv

@@ -0,0 +1,2 @@
+id
+1

test/fixtures/tasks_workers.csv

@@ -0,0 +1,2 @@
+id,worker_id,task_id
+1,1,1

test/fixtures/workers.csv

@@ -0,0 +1,2 @@
+id
+1

test/models/Task.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace test\models;
+
+use ActiveRecord\Model;
+
+class Task extends Model
+{
+    public static array $has_and_belongs_to_many = [
+        'workers' => []
+    ];
+}

test/models/Worker.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace test\models;
+
+use ActiveRecord\Model;
+
+class Worker extends Model
+{
+    public static array $has_and_belongs_to_many = [
+        'tasks' => []
+    ];
+}

test/sql/mysql.sql

@@ -132,3 +132,17 @@ CREATE TABLE courses_students(
   `student_id` int(11) NOT NULL DEFAULT '0',
   PRIMARY KEY(course_id, student_id)
 );
+
+CREATE TABLE tasks (
+  id INT NOT NULL AUTO_INCREMENT PRIMARY KEY
+);
+
+CREATE TABLE workers (
+  id INT NOT NULL AUTO_INCREMENT PRIMARY KEY
+);
+
+CREATE TABLE tasks_workers (
+  id INT NOT NULL AUTO_INCREMENT PRIMARY KEY,
+  worker_id INT NOT NULL,
+  task_id INT NOT NULL
+);

test/sql/pgsql-after-fixtures.sql

@@ -15,3 +15,6 @@ SELECT setval('users_id_seq', max(id)) FROM users;
 SELECT setval('newsletters_id_seq', max(id)) FROM newsletters;
 SELECT setval('user_newsletters_id_seq', max(id)) FROM user_newsletters;
 SELECT setval('valuestore_id_seq', max(id)) FROM valuestore;
+SELECT setval('tasks_id_seq', max(id)) FROM tasks;
+SELECT setval('workers_id_seq', max(id)) FROM workers;
+SELECT setval('tasks_workers_id_seq', max(id)) FROM tasks_workers;

test/sql/pgsql.sql

@@ -132,5 +132,19 @@ CREATE TABLE courses_students(
   PRIMARY KEY(course_id, student_id)
 );
 
+CREATE TABLE tasks (
+	id serial primary key
+);
+
+CREATE TABLE workers (
+	id serial primary key
+);
+
+CREATE TABLE tasks_workers (
+	id serial primary key,
+	worker_id int not null,
+	task_id int not null
+);
+
 -- reproduces issue GH-96 for testing
 CREATE INDEX user_newsletters_id_and_user_id_idx ON user_newsletters USING btree(id, user_id);

test/sql/sqlite.sql

@@ -131,3 +131,17 @@ CREATE TABLE courses_students(
      student_id int not null,
      PRIMARY KEY(course_id, student_id)
 );
+
+CREATE TABLE tasks (
+  id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT
+);
+
+CREATE TABLE workers (
+  id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT
+);
+
+CREATE TABLE tasks_workers (
+  id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT,
+  worker_id INTEGER NOT NULL,
+  task_id INTEGER NOT NULL
+);