diff --git a/src/Command/ModelCommand.php b/src/Command/ModelCommand.php index 2a18fd73..131dbfa3 100644 --- a/src/Command/ModelCommand.php +++ b/src/Command/ModelCommand.php @@ -358,6 +358,15 @@ public function findBelongsTo(Table $model, array $associations, ?Arguments $arg ]; } else { $tmpModelName = $this->_modelNameFromKey($fieldName); + // A key that resolves to the table itself (e.g. a `system_id` + // column on `systems`) is only a real self-reference when it is + // actually constrained as a foreign key to this table. Otherwise + // (e.g. an external id that merely follows the `_id` + // naming) skip it to avoid generating an invalid self-association + // that collides with the table's own alias. + if ($tmpModelName === $model->getAlias() && !$this->findTableReferencedBy($schema, $fieldName)) { + continue; + } if (!$this->getTableLocator()->exists($tmpModelName)) { $this->getTableLocator()->get( $tmpModelName, @@ -520,7 +529,11 @@ public function findHasOne(Table $model, array $associations): array } $assoc = false; - if (!in_array($fieldName, $primaryKey) && $fieldName === $foreignKey) { + if ( + $otherTableName !== $tableName && + !in_array($fieldName, $primaryKey) && + $fieldName === $foreignKey + ) { $assoc = [ 'alias' => $otherModel->getAlias(), 'foreignKey' => $fieldName, @@ -564,6 +577,7 @@ public function findHasMany(Table $model, array $associations): array foreach ($otherSchema->columns() as $fieldName) { $assoc = false; if ( + $otherTableName !== $tableName && !in_array($fieldName, $primaryKey) && $fieldName === $foreignKey && !$this->hasUniqueConstraintFor($otherSchema, $fieldName) diff --git a/tests/TestCase/Command/ModelCommandTest.php b/tests/TestCase/Command/ModelCommandTest.php index 8870892b..dd34feb5 100644 --- a/tests/TestCase/Command/ModelCommandTest.php +++ b/tests/TestCase/Command/ModelCommandTest.php @@ -282,6 +282,82 @@ public function testApplyAssociationsConcreteClass(): void $this->assertEquals($original, $new); } + /** + * A non-foreign-key column matching the table name (e.g. `system_id` on + * `systems`) must not generate an invalid self-referencing association. + * + * @return void + */ + public function testGetAssociationsSkipsSelfReferencingKey(): void + { + $systems = $this->getTableLocator()->get('Systems'); + + $command = new ModelCommand(); + $command->connection = 'test'; + $args = new Arguments([], [], []); + $io = $this->createStub(ConsoleIo::class); + $result = $command->getAssociations($systems, $args, $io); + + $aliases = array_merge( + array_column($result['belongsTo'], 'alias'), + array_column($result['hasMany'], 'alias'), + ); + $this->assertNotContains('Systems', $aliases, 'Self-referencing association must not be generated.'); + + // Applying the associations must not throw. + $command->applyAssociations($systems, $result); + $this->assertSame([], $systems->associations()->keys()); + } + + /** + * A real self-referencing foreign key (constrained, not named `parent_id`) + * must still produce a belongsTo association. + * + * @return void + */ + public function testGetAssociationsKeepsConstrainedSelfReference(): void + { + $nodes = $this->getTableLocator()->get('Nodes'); + + $command = new ModelCommand(); + $command->connection = 'test'; + $args = new Arguments([], [], []); + $io = $this->createStub(ConsoleIo::class); + $result = $command->getAssociations($nodes, $args, $io); + + $this->assertSame(['Nodes'], array_column($result['belongsTo'], 'alias')); + + $command->applyAssociations($nodes, $result); + $this->assertSame(['Nodes'], $nodes->associations()->keys()); + } + + /** + * A unique non-foreign-key column matching the table name (e.g. a unique + * `gadget_id` on `gadgets`) must not generate a self-referencing hasOne. + * + * @return void + */ + public function testGetAssociationsSkipsUniqueSelfReferencingKey(): void + { + $gadgets = $this->getTableLocator()->get('Gadgets'); + + $command = new ModelCommand(); + $command->connection = 'test'; + $args = new Arguments([], [], []); + $io = $this->createStub(ConsoleIo::class); + $result = $command->getAssociations($gadgets, $args, $io); + + $aliases = array_merge( + array_column($result['belongsTo'], 'alias'), + array_column($result['hasOne'], 'alias'), + array_column($result['hasMany'], 'alias'), + ); + $this->assertNotContains('Gadgets', $aliases, 'Self-referencing association must not be generated.'); + + $command->applyAssociations($gadgets, $result); + $this->assertSame([], $gadgets->associations()->keys()); + } + /** * Test getAssociations * diff --git a/tests/schema.php b/tests/schema.php index 336c7d84..bc3c3d3c 100644 --- a/tests/schema.php +++ b/tests/schema.php @@ -587,6 +587,51 @@ 'unique_self_referencing_parent' => ['type' => 'unique', 'columns' => ['parent_id']], ], ], + // "systems" has a "system_id" column that is not a real foreign key. + // Bake must not generate a self-referencing Systems association for it. + [ + 'table' => 'systems', + 'columns' => [ + 'id' => ['type' => 'integer'], + 'system_id' => ['type' => 'string', 'length' => 255, 'null' => false], + 'name' => ['type' => 'string', 'length' => 255, 'null' => true], + ], + 'constraints' => ['primary' => ['type' => 'primary', 'columns' => ['id']]], + ], + // "nodes" has a real self-referencing foreign key (node_id -> nodes.id) + // that is not named parent_id. The belongsTo association must still be kept. + [ + 'table' => 'nodes', + 'columns' => [ + 'id' => ['type' => 'integer'], + 'node_id' => ['type' => 'integer', 'null' => true], + 'name' => ['type' => 'string', 'length' => 255, 'null' => true], + ], + 'constraints' => [ + 'primary' => ['type' => 'primary', 'columns' => ['id']], + 'node_id_fk' => [ + 'type' => 'foreign', + 'columns' => ['node_id'], + 'references' => ['nodes', 'id'], + 'update' => 'cascade', + 'delete' => 'cascade', + ], + ], + ], + // "gadgets" has a unique "gadget_id" column that is not a real foreign key. + // Bake must not generate a self-referencing hasOne Gadgets association. + [ + 'table' => 'gadgets', + 'columns' => [ + 'id' => ['type' => 'integer'], + 'gadget_id' => ['type' => 'string', 'length' => 255, 'null' => false], + 'name' => ['type' => 'string', 'length' => 255, 'null' => true], + ], + 'constraints' => [ + 'primary' => ['type' => 'primary', 'columns' => ['id']], + 'unique_gadget_id' => ['type' => 'unique', 'columns' => ['gadget_id']], + ], + ], // "news" is both singular and plural - tests variable collision fix [ 'table' => 'news',