Model extender: Fix inheritance (#2132)

This ensures that default values, date attributes and relationships are properly inherited, when we have deeper model class hierarchies.

This also adds test cases to ensure that inheritance order is honored for relationship and default attribute extender. As there's no way to remove date attributes, the order of evaluation there doesn't matter.

src/Database/AbstractModel.php

@@ -78,7 +78,11 @@ abstract class AbstractModel extends Eloquent
      */
     public function __construct(array $attributes = [])
     {
-        $this->attributes = Arr::get(static::$defaults, static::class, []);
+        $this->attributes = [];
+
+        foreach (array_merge(array_reverse(class_parents($this)), [static::class]) as $class) {
+            $this->attributes = array_merge($this->attributes, Arr::get(static::$defaults, $class, []));
+        }
 
         // Deprecated in beta 13, remove in beta 14.
         static::$dispatcher->dispatch(
@@ -103,7 +107,13 @@ abstract class AbstractModel extends Eloquent
             new ConfigureModelDates($this, $this->dates)
         );
 
-        return array_merge($this->dates, Arr::get(static::$dateAttributes, static::class, []));
+        $dates = $this->dates;
+
+        foreach (array_merge(array_reverse(class_parents($this)), [static::class]) as $class) {
+            $dates = array_merge($dates, Arr::get(static::$dateAttributes, $class, []));
+        }
+
+        return $dates;
     }
 
     /**
@@ -141,10 +151,11 @@ abstract class AbstractModel extends Eloquent
      */
     protected function getCustomRelation($name)
     {
-        $relation = Arr::get(static::$customRelations, static::class.".$name", null);
-
-        if (! is_null($relation)) {
-            return $relation($this);
+        foreach (array_merge([static::class], class_parents($this)) as $class) {
+            $relation = Arr::get(static::$customRelations, $class.".$name", null);
+            if (! is_null($relation)) {
+                return $relation($this);
+            }
         }
 
         // Deprecated, remove in beta 14

tests/integration/extenders/ModelTest.php

@@ -13,7 +13,9 @@ use Carbon\Carbon;
 use Flarum\Discussion\Discussion;
 use Flarum\Extend;
 use Flarum\Group\Group;
+use Flarum\Post\AbstractEventPost;
 use Flarum\Post\CommentPost;
+use Flarum\Post\DiscussionRenamedPost;
 use Flarum\Post\Post;
 use Flarum\Tests\integration\RetrievesAuthorizedUsers;
 use Flarum\Tests\integration\TestCase;
@@ -34,6 +36,21 @@ class ModelTest extends TestCase
         ]);
     }
 
+    protected function prepPostsHierarchy()
+    {
+        $this->prepareDatabase([
+            'users' => [
+                $this->normalUser(),
+            ],
+            'discussions' => [
+                ['id' => 1, 'title' => 'Discussion with post', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'first_post_id' => 1, 'comment_count' => 1, 'is_private' => 0],
+            ],
+            'posts' => [
+                ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'discussionRenamed', 'content' => '<t><p>can i haz relationz?</p></t>'],
+            ],
+        ]);
+    }
+
     /**
      * @test
      */
@@ -152,14 +169,7 @@ class ModelTest extends TestCase
                 ->belongsTo('ancestor', Discussion::class, 'discussion_id')
         );
 
-        $this->prepareDatabase([
-            'discussions' => [
-                ['id' => 1, 'title' => 'Discussion with post', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'first_post_id' => 1, 'comment_count' => 1, 'is_private' => 0],
-            ],
-            'posts' => [
-                ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>can i haz relationz?</p></t>'],
-            ],
-        ]);
+        $this->prepPostsHierarchy();
 
         $post = CommentPost::find(1);
 
@@ -167,6 +177,45 @@ class ModelTest extends TestCase
         $this->assertEquals(1, $post->ancestor->id);
     }
 
+    /**
+     * @test
+     */
+    public function custom_relationship_prioritizes_child_classes_within_2_parent_classes()
+    {
+        $this->extend(
+            (new Extend\Model(Post::class))
+                ->belongsTo('ancestor', User::class, 'user_id'),
+            (new Extend\Model(AbstractEventPost::class))
+                ->belongsTo('ancestor', Discussion::class, 'discussion_id')
+        );
+
+        $this->prepPostsHierarchy();
+
+        $post = DiscussionRenamedPost::find(1);
+
+        $this->assertInstanceOf(Discussion::class, $post->ancestor);
+        $this->assertEquals(1, $post->ancestor->id);
+    }
+
+    /**
+     * @test
+     */
+    public function custom_relationship_prioritizes_child_classes_within_child_class_and_immediate_parent()
+    {
+        $this->extend(
+            (new Extend\Model(AbstractEventPost::class))
+                ->belongsTo('ancestor', Discussion::class, 'discussion_id'),
+            (new Extend\Model(DiscussionRenamedPost::class))
+                ->belongsTo('ancestor', User::class, 'user_id')
+        );
+
+        $this->prepPostsHierarchy();
+        $post = DiscussionRenamedPost::find(1);
+
+        $this->assertInstanceOf(User::class, $post->ancestor);
+        $this->assertEquals(2, $post->ancestor->id);
+    }
+
     /**
      * @test
      */
@@ -261,6 +310,31 @@ class ModelTest extends TestCase
         $this->assertEquals(42, $post->answer);
     }
 
+    /**
+     * @test
+     */
+    public function custom_default_attribute_inheritance_prioritizes_child_class()
+    {
+        $this->extend(
+            (new Extend\Model(Post::class))
+                ->default('answer', 'dont do this'),
+            (new Extend\Model(AbstractEventPost::class))
+                ->default('answer', 42),
+            (new Extend\Model(DiscussionRenamedPost::class))
+                ->default('answer', 'ni!')
+        );
+
+        $this->app();
+
+        $post = new CustomPost;
+
+        $this->assertEquals(42, $post->answer);
+
+        $commentPost = new DiscussionRenamedPost;
+
+        $this->assertEquals('ni!', $commentPost->answer);
+    }
+
     /**
      * @test
      */
@@ -341,3 +415,11 @@ class ModelTest extends TestCase
         $this->assertNotContains('custom', $discussion->getDates());
     }
 }
+
+class CustomPost extends AbstractEventPost
+{
+    /**
+     * {@inheritdoc}
+     */
+    public static $type = 'customPost';
+}