From 6416fbd5d39b3372eb0e23562f4c36c522b3f3e6 Mon Sep 17 00:00:00 2001
From: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com>
Date: Fri, 24 Apr 2020 15:17:31 -0400
Subject: [PATCH] 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.
---
 framework/core/src/Database/AbstractModel.php | 23 +++--
 .../tests/integration/extenders/ModelTest.php | 98 +++++++++++++++++--
 2 files changed, 107 insertions(+), 14 deletions(-)

diff --git a/framework/core/src/Database/AbstractModel.php b/framework/core/src/Database/AbstractModel.php
index 5a74e0fae..d6f7b4ce6 100644
--- a/framework/core/src/Database/AbstractModel.php
+++ b/framework/core/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
diff --git a/framework/core/tests/integration/extenders/ModelTest.php b/framework/core/tests/integration/extenders/ModelTest.php
index f06a0ad34..dc0629332 100644
--- a/framework/core/tests/integration/extenders/ModelTest.php
+++ b/framework/core/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';
+}