Merge pull request reliese#192 from coatesap/improved-relationship-names

Improved relationship names when using foreign_key naming strategy

config/models.php

@@ -333,14 +333,19 @@
                             generates Post::user() and User::posts()
         |
         | 'foreign_key' Use the foreign key as the relation name.
-        |                   (post.author --> user.id)
-        |                       generates Post::author() and User::posts_author()
-        |               Column id's are ignored.
+        |               This can help to provide more meaningful relationship names, and avoids naming conflicts
+        |               if you have more than one relationship between two tables.
         |                   (post.author_id --> user.id)
+        |                       generates Post::author() and User::posts_where_author()
+        |                   (post.editor_id --> user.id)
+        |                       generates Post::editor() and User::posts_where_editor()
+        |               ID suffixes can be omitted from foreign keys.
+        |                   (post.author --> user.id)
+        |                   (post.editor --> user.id)
         |                       generates the same as above.
-        |               When the foreign key is redundant, it is omited.
+        |               Where the foreign key matches the related table name, it behaves as per the 'related' strategy.
         |                   (post.user_id --> user.id)
-        |                       generates User::posts() and not User::posts_user()
+        |                       generates Post::user() and User::posts()
         */
 
         'relation_name_strategy' => 'related',

src/Coders/Model/Factory.php

@@ -283,7 +283,7 @@ private function imports($dependencies, Model $model)
         $imports = [];
         foreach ($dependencies as $dependencyClass) {
             // Skip when the same class
-            if ($dependencyClass == $model->getQualifiedUserClassName()) {
+            if (trim($dependencyClass, "\\") == trim($model->getQualifiedUserClassName(), "\\")) {
                 continue;
             }
 

src/Coders/Model/Relations/BelongsTo.php

@@ -51,7 +51,11 @@ public function name()
     {
         switch ($this->parent->getRelationNameStrategy()) {
             case 'foreign_key':
-                $relationName = preg_replace("/[^a-zA-Z0-9]?{$this->otherKey()}$/", '', $this->foreignKey());
+                $relationName = RelationHelper::stripSuffixFromForeignKey(
+                    $this->parent->usesSnakeAttributes(),
+                    $this->otherKey(),
+                    $this->foreignKey()
+                );
                 break;
             default:
             case 'related':

src/Coders/Model/Relations/HasMany.php

@@ -25,31 +25,22 @@ public function hint()
      */
     public function name()
     {
-        if ($this->parent->shouldPluralizeTableName()) {
-            $relationBaseName = Str::plural(Str::singular($this->related->getTable(true)));
-        } else {
-            $relationBaseName = $this->related->getTable(true);
-        }
-
-        if ($this->parent->shouldLowerCaseTableName()) {
-            $relationBaseName = strtolower($relationBaseName);
-        }
-
         switch ($this->parent->getRelationNameStrategy()) {
             case 'foreign_key':
-                $suffix = preg_replace("/[^a-zA-Z0-9]?{$this->localKey()}$/", '', $this->foreignKey());
-
-                $relationName = $relationBaseName;
-
-                // Don't make relations such as users_user, just leave it as 'users'.
-                if ($this->parent->getTable(true) !== $suffix) {
-                    $relationName .= "_{$suffix}";
+                $relationName = RelationHelper::stripSuffixFromForeignKey(
+                    $this->parent->usesSnakeAttributes(),
+                    $this->localKey(),
+                    $this->foreignKey()
+                );
+                if (Str::snake($relationName) === Str::snake($this->parent->getClassName())) {
+                    $relationName = Str::plural($this->related->getClassName());
+                } else {
+                    $relationName = Str::plural($this->related->getClassName()) . 'Where' . ucfirst(Str::singular($relationName));
                 }
-
                 break;
-            case 'related':
             default:
-                $relationName = $relationBaseName;
+            case 'related':
+                $relationName = Str::plural($this->related->getClassName());
                 break;
         }
 

src/Coders/Model/Relations/RelationHelper.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace Reliese\Coders\Model\Relations;
+
+use Illuminate\Support\Str;
+
+/**
+ * General utility functions for dealing with relationships
+ */
+class RelationHelper
+{
+    /**
+     * Turns a column name like 'manager_id' into 'manager'; or 'lineManagerId' into 'lineManager'.
+     *
+     * @param bool $usesSnakeAttributes
+     * @param string $primaryKey
+     * @param string $foreignKey
+     * @return string
+     */
+    public static function stripSuffixFromForeignKey($usesSnakeAttributes, $primaryKey, $foreignKey)
+    {
+        if ($usesSnakeAttributes) {
+            $lowerPrimaryKey = strtolower($primaryKey);
+            return preg_replace('/(_)(' . $primaryKey . '|' . $lowerPrimaryKey . ')$/', '', $foreignKey);
+        } else {
+            $studlyPrimaryKey = Str::studly($primaryKey);
+            return preg_replace('/(' . $primaryKey . '|' . $studlyPrimaryKey . ')$/', '', $foreignKey);
+        }
+    }
+}

tests/Coders/Model/Relations/BelongsToTest.php

@@ -0,0 +1,94 @@
+<?php
+
+use Illuminate\Support\Fluent;
+use PHPUnit\Framework\TestCase;
+use Reliese\Coders\Model\Model;
+use Reliese\Coders\Model\Relations\BelongsTo;
+
+class BelongsToTest extends TestCase
+{
+    public function provideForeignKeyStrategyPermutations()
+    {
+        // usesSnakeAttributes, primaryKey, foreignKey, expected
+        return [
+            // columns use camelCase
+            [false, 'id', 'lineManagerId', 'lineManager'],
+            [false, 'Id', 'lineManagerId', 'lineManager'],
+            [false, 'ID', 'lineManagerID', 'lineManager'],
+            // columns use snake_case
+            [true, 'id', 'line_manager_id', 'line_manager'],
+            [true, 'ID', 'line_manager_id', 'line_manager'],
+            // foreign keys without primary key suffix
+            [false, 'id', 'lineManager', 'lineManager'],
+            [true, 'id', 'line_manager', 'line_manager'],
+        ];
+    }
+
+    /**
+     * @dataProvider provideForeignKeyStrategyPermutations
+     *
+     * @param bool $usesSnakeAttributes
+     * @param string $primaryKey
+     * @param string $foreignKey
+     * @param string $expected
+     */
+    public function testNameUsingForeignKeyStrategy($usesSnakeAttributes, $primaryKey, $foreignKey, $expected)
+    {
+        $relation = Mockery::mock(Fluent::class)->makePartial();
+
+        $relatedModel = Mockery::mock(Model::class)->makePartial();
+
+        $subject = Mockery::mock(Model::class)->makePartial();
+        $subject->shouldReceive('getRelationNameStrategy')->andReturn('foreign_key');
+        $subject->shouldReceive('usesSnakeAttributes')->andReturn($usesSnakeAttributes);
+
+        /** @var BelongsTo|\Mockery\Mock $relationship */
+        $relationship = Mockery::mock(BelongsTo::class, [$relation, $subject, $relatedModel])->makePartial();
+        $relationship->shouldAllowMockingProtectedMethods();
+        $relationship->shouldReceive('otherKey')->andReturn($primaryKey);
+        $relationship->shouldReceive('foreignKey')->andReturn($foreignKey);
+
+        $this->assertEquals(
+            $expected,
+            $relationship->name(),
+            json_encode(compact('usesSnakeAttributes', 'primaryKey', 'foreignKey'))
+        );
+    }
+
+    public function provideRelatedStrategyPermutations()
+    {
+        // usesSnakeAttributes, relatedClassName, expected
+        return [
+            [false, 'LineManager', 'lineManager'],
+            [true, 'LineManager', 'line_manager'],
+        ];
+    }
+
+    /**
+     * @dataProvider provideRelatedStrategyPermutations
+     *
+     * @param bool $usesSnakeAttributes
+     * @param string $relatedClassName
+     * @param string $expected
+     */
+    public function testNameUsingRelatedStrategy($usesSnakeAttributes, $relatedClassName, $expected)
+    {
+        $relation = Mockery::mock(Fluent::class)->makePartial();
+
+        $relatedModel = Mockery::mock(Model::class)->makePartial();
+        $relatedModel->shouldReceive('getClassName')->andReturn($relatedClassName);
+
+        $subject = Mockery::mock(Model::class)->makePartial();
+        $subject->shouldReceive('getRelationNameStrategy')->andReturn('related');
+        $subject->shouldReceive('usesSnakeAttributes')->andReturn($usesSnakeAttributes);
+
+        /** @var BelongsTo|\Mockery\Mock $relationship */
+        $relationship = Mockery::mock(BelongsTo::class, [$relation, $subject, $relatedModel])->makePartial();
+
+        $this->assertEquals(
+            $expected,
+            $relationship->name(),
+            json_encode(compact('usesSnakeAttributes', 'relatedClassName'))
+        );
+    }
+}

tests/Coders/Model/Relations/HasManyTest.php

@@ -0,0 +1,111 @@
+<?php
+
+use Illuminate\Support\Fluent;
+use PHPUnit\Framework\TestCase;
+use Reliese\Coders\Model\Model;
+use Reliese\Coders\Model\Relations\BelongsTo;
+use Reliese\Coders\Model\Relations\HasMany;
+
+class HasManyTest extends TestCase
+{
+    public function provideForeignKeyStrategyPermutations()
+    {
+        // usesSnakeAttributes, subjectName, relationName, primaryKey, foreignKey, expected
+        return [
+            // camelCase
+            [false, 'StaffMember', 'BlogPost', 'id', 'authorId', 'blogPostsWhereAuthor'],
+            [false, 'StaffMember', 'BlogPost', 'ID', 'authorID', 'blogPostsWhereAuthor'],
+            [false, 'StaffMember', 'BlogPost', 'id', 'staffMemberId', 'blogPosts'],
+            [false, 'StaffMember', 'BlogPost', 'ID', 'staffMemberID', 'blogPosts'],
+            // snake_case
+            [true, 'StaffMember', 'BlogPost', 'id', 'author_id', 'blog_posts_where_author'],
+            [true, 'StaffMember', 'BlogPost', 'id', 'staff_member_id', 'blog_posts'],
+            [true, 'StaffMember', 'BlogPost', 'ID', 'author_id', 'blog_posts_where_author'],
+            [true, 'StaffMember', 'BlogPost', 'ID', 'staff_member_id', 'blog_posts'],
+            // no suffix
+            [false, 'StaffMember', 'BlogPost', 'id', 'author', 'blogPostsWhereAuthor'],
+            [false, 'StaffMember', 'BlogPost', 'id', 'staff_member', 'blogPosts'],
+            [true, 'StaffMember', 'BlogPost', 'id', 'author', 'blog_posts_where_author'],
+            [true, 'StaffMember', 'BlogPost', 'id', 'staff_member', 'blog_posts'],
+            // same table reference
+            [false, 'StaffMember', 'StaffMember', 'id', 'staffMemberId', 'staffMembers'],
+            [false, 'StaffMember', 'StaffMember', 'id', 'lineManagerId', 'staffMembersWhereLineManager'],
+        ];
+    }
+
+    /**
+     * @dataProvider provideForeignKeyStrategyPermutations
+     *
+     * @param bool $usesSnakeAttributes
+     * @param string $subjectName
+     * @param string $relationName
+     * @param string $primaryKey
+     * @param string $foreignKey
+     * @param string $expected
+     */
+    public function testNameUsingForeignKeyStrategy($usesSnakeAttributes, $subjectName, $relationName, $primaryKey, $foreignKey, $expected)
+    {
+        $relation = Mockery::mock(Fluent::class)->makePartial();
+
+        $relatedModel = Mockery::mock(Model::class)->makePartial();
+        $relatedModel->shouldReceive('getClassName')->andReturn($relationName);
+
+        $subject = Mockery::mock(Model::class)->makePartial();
+        $subject->shouldReceive('getRelationNameStrategy')->andReturn('foreign_key');
+        $subject->shouldReceive('usesSnakeAttributes')->andReturn($usesSnakeAttributes);
+        $subject->shouldReceive('getClassName')->andReturn($subjectName);
+
+        /** @var BelongsTo|\Mockery\Mock $relationship */
+        $relationship = Mockery::mock(HasMany::class, [$relation, $subject, $relatedModel])->makePartial();
+        $relationship->shouldAllowMockingProtectedMethods();
+        $relationship->shouldReceive('localKey')->andReturn($primaryKey);
+        $relationship->shouldReceive('foreignKey')->andReturn($foreignKey);
+
+        $this->assertEquals(
+            $expected,
+            $relationship->name(),
+            json_encode(compact('usesSnakeAttributes', 'subjectName', 'relationName', 'primaryKey', 'foreignKey'))
+        );
+    }
+
+    public function provideRelatedStrategyPermutations()
+    {
+        // usesSnakeAttributes, subjectName, relatedName, expected
+        return [
+            [false, 'StaffMember', 'BlogPost', 'blogPosts'],
+            [true, 'StaffMember', 'BlogPost', 'blog_posts'],
+            // Same table reference
+            [false, 'StaffMember', 'StaffMember', 'staffMembers']
+        ];
+    }
+
+    /**
+     * @dataProvider provideRelatedStrategyPermutations
+     *
+     * @param bool $usesSnakeAttributes
+     * @param string $subjectName
+     * @param string $relationName
+     * @param string $expected
+     */
+    public function testNameUsingRelatedStrategy($usesSnakeAttributes, $subjectName, $relationName, $expected)
+    {
+        $relation = Mockery::mock(Fluent::class)->makePartial();
+
+        $relatedModel = Mockery::mock(Model::class)->makePartial();
+        $relatedModel->shouldReceive('getClassName')->andReturn($relationName);
+
+        $subject = Mockery::mock(Model::class)->makePartial();
+        $subject->shouldReceive('getClassName')->andReturn($subjectName);
+        $subject->shouldReceive('getRelationNameStrategy')->andReturn('related');
+        $subject->shouldReceive('usesSnakeAttributes')->andReturn($usesSnakeAttributes);
+
+        /** @var BelongsTo|\Mockery\Mock $relationship */
+        $relationship = Mockery::mock(HasMany::class, [$relation, $subject, $relatedModel])->makePartial();
+
+        $this->assertEquals(
+            $expected,
+            $relationship->name(),
+            json_encode(compact('usesSnakeAttributes', 'subjectName', 'relationName'))
+        );
+    }
+}

tests/Coders/Model/Relations/RelationHelperTest.php

@@ -0,0 +1,43 @@
+<?php
+
+use PHPUnit\Framework\TestCase;
+use Reliese\Coders\Model\Relations\RelationHelper;
+
+class RelationHelperTest extends TestCase
+{
+    public function provideKeys()
+    {
+        // usesSnakeAttributes, primaryKey, foreignKey, expected
+        return [
+            // camelCase
+            [false, 'id', 'lineManagerId', 'lineManager'],
+            [false, 'ID', 'lineManagerID', 'lineManager'],
+            // snake_case
+            [true, 'id', 'line_manager_id', 'line_manager'],
+            [true, 'ID', 'line_manager_id', 'line_manager'],
+            // no suffix
+            [false, 'id', 'lineManager', 'lineManager'],
+            [true, 'id', 'line_manager', 'line_manager'],
+            // columns that contain the letters of the primary key as part of their name
+            [false, 'id', 'holiday', 'holiday'],
+            [true, 'id', 'something_identifier_id', 'something_identifier']
+        ];
+    }
+
+    /**
+     * @dataProvider provideKeys
+     *
+     * @param bool $usesSnakeAttributes
+     * @param string $primaryKey
+     * @param string $foreignKey
+     * @param string $expected
+     */
+    public function testNameUsingForeignKeyStrategy($usesSnakeAttributes, $primaryKey, $foreignKey, $expected)
+    {
+        $this->assertEquals(
+            $expected,
+            RelationHelper::stripSuffixFromForeignKey($usesSnakeAttributes, $primaryKey, $foreignKey),
+            json_encode(compact('usesSnakeAttributes', 'primaryKey', 'foreignKey'))
+        );
+    }
+}