From 9dbd9d7998f685c0d5d81e9a9f2b18e2a8c20f51 Mon Sep 17 00:00:00 2001 From: Ethann Date: Mon, 15 Sep 2025 15:42:05 +0200 Subject: [PATCH 1/5] Fix relationship handling in HasAndBelongsToMany and improve test coverage for explicit class names --- lib/Relationship/HasAndBelongsToMany.php | 6 ++-- lib/Table.php | 38 +++++++++++------------- test/RelationshipTest.php | 14 +++++++++ 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/lib/Relationship/HasAndBelongsToMany.php b/lib/Relationship/HasAndBelongsToMany.php index 2df0dbf9..39826459 100644 --- a/lib/Relationship/HasAndBelongsToMany.php +++ b/lib/Relationship/HasAndBelongsToMany.php @@ -27,7 +27,9 @@ public function __construct(string $attribute, array $options = []) { parent::__construct($attribute, $options); - $this->set_class_name($this->inferred_class_name(Utils::singularize($attribute))); + if (!isset($this->class_name)) { + $this->set_class_name($this->inferred_class_name(Utils::singularize($attribute))); + } $this->options['association_foreign_key'] ??= Inflector::keyify($this->class_name); } @@ -46,7 +48,7 @@ public function load(Model $model): mixed * @var Relation */ $rel = new Relation($this->class_name, [], []); - $rel->from($this->attribute_name); + $rel->from($this->get_table()->table); $other_table = Table::load(get_class($model)); $other_table_name = $other_table->table; $other_table_primary_key = $other_table->pk[0]; diff --git a/lib/Table.php b/lib/Table.php index e80b0603..c35aaa2b 100644 --- a/lib/Table.php +++ b/lib/Table.php @@ -164,24 +164,20 @@ public function create_joins(array|string $joins): string $ret .= $space; if (false === stripos($value, 'JOIN ')) { - if (array_key_exists($value, $this->relationships)) { - $rel = $this->get_relationship($value); - - /** - * PHPStan seems to be getting confused about the usage of a class-string - * as an array string. - * - * @phpstan-ignore-next-line - */ - $alias = !empty($existing_tables[$rel->class_name]) ? $value : null; - /* @phpstan-ignore-next-line */ - $existing_tables[$rel->class_name] = true; - - /* @phpstan-ignore-next-line */ - $ret .= $rel->construct_inner_join_sql($this, false, $alias); - } else { - throw new RelationshipException("Relationship named $value has not been declared for class: {$this->class->getName()}"); - } + $rel = $this->get_relationship($value, true); + + /** + * PHPStan seems to be getting confused about the usage of a class-string + * as an array string. + * + * @phpstan-ignore-next-line + */ + $alias = !empty($existing_tables[$rel->class_name]) ? $value : null; + /* @phpstan-ignore-next-line */ + $existing_tables[$rel->class_name] = true; + + /* @phpstan-ignore-next-line */ + $ret .= $rel->construct_inner_join_sql($this, false, $alias); } else { $ret .= $value; } @@ -382,8 +378,10 @@ public function get_fully_qualified_table_name(): string */ public function get_relationship(string $name, bool $strict = false): ?AbstractRelationship { - if ($this->has_relationship($name)) { - return $this->relationships[$name]; + foreach ($this->relationships as $relationship) { + if ($relationship->attribute_name === $name || ($relationship instanceof HasAndBelongsToMany && $relationship->class_name === ucfirst(Utils::singularize($name)))) { + return $relationship; + } } if ($strict) { diff --git a/test/RelationshipTest.php b/test/RelationshipTest.php index dbe29188..d2eafa83 100644 --- a/test/RelationshipTest.php +++ b/test/RelationshipTest.php @@ -346,6 +346,20 @@ public function testHasAndBelongsToManyPrimaryKeyIsDifferentThanForeignKeyRevers $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 testHasAndBelongsToManyWithExplicitClassName() + { + Task::$has_and_belongs_to_many = [ + 'custom_workers' => [ + 'class_name' => 'Worker' + ] + ]; + + $task = Task::find(1); + $workers = $task->custom_workers; + $this->assertEquals(1, count($workers)); + $this->assertInstanceOf(Worker::class, $workers[0]); + } + public function testBelongsToCreateAssociation() { $event = Event::find(5); From 04ff908bff9adcdf751065e82f2df1f13e6f7844 Mon Sep 17 00:00:00 2001 From: Ethann Date: Tue, 16 Sep 2025 00:15:10 +0200 Subject: [PATCH 2/5] Add is_string_this_relationship method to AbstractRelationship and update usage in HasAndBelongsToMany and Table classes --- lib/Relationship/AbstractRelationship.php | 10 ++++++++++ lib/Relationship/HasAndBelongsToMany.php | 7 ++++++- lib/Table.php | 2 +- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/Relationship/AbstractRelationship.php b/lib/Relationship/AbstractRelationship.php index 7de81f99..12c6f575 100644 --- a/lib/Relationship/AbstractRelationship.php +++ b/lib/Relationship/AbstractRelationship.php @@ -212,6 +212,16 @@ protected function query_and_attach_related_models_eagerly(Table $table, array $ } } + /** + * Checks if the given string matches this relationship's attribute name. + * + * @param string $other the string to compare with this relationship's attribute name + */ + public function is_string_this_relationship(string $other): bool + { + return $this->attribute_name === $other; + } + /** * Creates a new instance of specified {@link Model} with the attributes pre-loaded. * diff --git a/lib/Relationship/HasAndBelongsToMany.php b/lib/Relationship/HasAndBelongsToMany.php index 39826459..93a09bea 100644 --- a/lib/Relationship/HasAndBelongsToMany.php +++ b/lib/Relationship/HasAndBelongsToMany.php @@ -53,7 +53,7 @@ public function load(Model $model): mixed $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]); + $rel->joins([get_class($model)]); return $rel->to_a(); } @@ -88,4 +88,9 @@ public function load_eagerly($models, $attributes, $includes, Table $table): voi { throw new \Exception('load_eagerly undefined for ' . __CLASS__); } + + public function is_string_this_relationship(string $other): bool + { + return parent::is_string_this_relationship($other) || $other === $this->class_name; + } } diff --git a/lib/Table.php b/lib/Table.php index c35aaa2b..e93612c3 100644 --- a/lib/Table.php +++ b/lib/Table.php @@ -379,7 +379,7 @@ public function get_fully_qualified_table_name(): string public function get_relationship(string $name, bool $strict = false): ?AbstractRelationship { foreach ($this->relationships as $relationship) { - if ($relationship->attribute_name === $name || ($relationship instanceof HasAndBelongsToMany && $relationship->class_name === ucfirst(Utils::singularize($name)))) { + if ($relationship->is_string_this_relationship($name)) { return $relationship; } } From 3035774484aca3c67984b2ffe3d403fe3b598f3f Mon Sep 17 00:00:00 2001 From: Ethann Date: Tue, 16 Sep 2025 00:15:20 +0200 Subject: [PATCH 3/5] Add test for HasAndBelongsToMany relationship with explicit class names on both sides --- test/RelationshipTest.php | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/RelationshipTest.php b/test/RelationshipTest.php index d2eafa83..e2a03414 100644 --- a/test/RelationshipTest.php +++ b/test/RelationshipTest.php @@ -360,6 +360,28 @@ public function testHasAndBelongsToManyWithExplicitClassName() $this->assertInstanceOf(Worker::class, $workers[0]); } + public function testHasAndBelongsToManyWithExplicitClassNameBothSide() + { + Task::$has_and_belongs_to_many = [ + 'custom_workers' => [ + 'class_name' => 'Worker', + 'join_table' => 'tasks_workers' + ] + ]; + + Worker::$has_and_belongs_to_many = [ + 'custom_tasks' => [ + 'class_name' => 'Task', + 'join_table' => 'tasks_workers' + ] + ]; + + $task = Task::find(1); + $workers = $task->custom_workers; + $this->assertEquals(1, count($workers)); + $this->assertInstanceOf(Worker::class, $workers[0]); + } + public function testBelongsToCreateAssociation() { $event = Event::find(5); From ce9e7889ff0849401cf875d173186c3f531e2464 Mon Sep 17 00:00:00 2001 From: Ethann Date: Tue, 16 Sep 2025 00:29:18 +0200 Subject: [PATCH 4/5] Added join_table since the autogenerated class_name is custom_workers_tasks --- test/RelationshipTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/RelationshipTest.php b/test/RelationshipTest.php index e2a03414..8d034475 100644 --- a/test/RelationshipTest.php +++ b/test/RelationshipTest.php @@ -350,7 +350,8 @@ public function testHasAndBelongsToManyWithExplicitClassName() { Task::$has_and_belongs_to_many = [ 'custom_workers' => [ - 'class_name' => 'Worker' + 'class_name' => 'Worker', + 'join_table' => 'tasks_workers' ] ]; From c3cbb576c1675daaaa512161bcd08c1cbfe9b901 Mon Sep 17 00:00:00 2001 From: Ethann Date: Tue, 16 Sep 2025 00:34:45 +0200 Subject: [PATCH 5/5] Fix HasAndBelongsToMany relationship to use join_table instead of class_name since join_table will alway be unique --- lib/Relationship/HasAndBelongsToMany.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Relationship/HasAndBelongsToMany.php b/lib/Relationship/HasAndBelongsToMany.php index 93a09bea..59efe563 100644 --- a/lib/Relationship/HasAndBelongsToMany.php +++ b/lib/Relationship/HasAndBelongsToMany.php @@ -53,7 +53,7 @@ public function load(Model $model): mixed $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([get_class($model)]); + $rel->joins([$this->options['join_table']]); return $rel->to_a(); } @@ -91,6 +91,6 @@ public function load_eagerly($models, $attributes, $includes, Table $table): voi public function is_string_this_relationship(string $other): bool { - return parent::is_string_this_relationship($other) || $other === $this->class_name; + return parent::is_string_this_relationship($other) || $other === $this->options['join_table']; } }