Eager loading extender (#2724)

* Eager loading extender
* Add tests for the eager loading extender

framework/core/src/Api/Controller/AbstractSerializeController.php

@@ -11,6 +11,8 @@ namespace Flarum\Api\Controller;
 
 use Flarum\Api\JsonApiResponse;
 use Illuminate\Contracts\Container\Container;
+use Illuminate\Database\Eloquent\Collection;
+use Illuminate\Support\Str;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\RequestHandlerInterface;
@@ -84,6 +86,11 @@ abstract class AbstractSerializeController implements RequestHandlerInterface
      */
     protected static $beforeSerializationCallbacks = [];
 
+    /**
+     * @var array
+     */
+    protected static $loadRelations = [];
+
     /**
      * {@inheritdoc}
      */
@@ -139,6 +146,47 @@ abstract class AbstractSerializeController implements RequestHandlerInterface
      */
     abstract protected function createElement($data, SerializerInterface $serializer);
 
+    /**
+     * Eager loads the required relationships.
+     *
+     * @param Collection $models
+     * @param array $relations
+     * @return void
+     */
+    protected function loadRelations(Collection $models, array $relations): void
+    {
+        $addedRelations = [];
+
+        foreach (array_reverse(array_merge([static::class], class_parents($this))) as $class) {
+            if (isset(static::$loadRelations[$class])) {
+                $addedRelations = array_merge($addedRelations, static::$loadRelations[$class]);
+            }
+        }
+
+        if (! empty($addedRelations)) {
+            usort($addedRelations, function ($a, $b) {
+                return substr_count($a, '.') - substr_count($b, '.');
+            });
+
+            foreach ($addedRelations as $relation) {
+                if (strpos($relation, '.') !== false) {
+                    $parentRelation = Str::beforeLast($relation, '.');
+
+                    if (! in_array($parentRelation, $relations, true)) {
+                        continue;
+                    }
+                }
+
+                $relations[] = $relation;
+            }
+        }
+
+        if (! empty($relations)) {
+            $relations = array_unique($relations);
+            $models->loadMissing($relations);
+        }
+    }
+
     /**
      * @param ServerRequestInterface $request
      * @return array
@@ -348,4 +396,13 @@ abstract class AbstractSerializeController implements RequestHandlerInterface
 
         static::$beforeSerializationCallbacks[$controllerClass][] = $callback;
     }
+
+    public static function setLoadRelations(string $controllerClass, array $relations)
+    {
+        if (! isset(static::$loadRelations[$controllerClass])) {
+            static::$loadRelations[$controllerClass] = [];
+        }
+
+        static::$loadRelations[$controllerClass] = array_merge(static::$loadRelations[$controllerClass], $relations);
+    }
 }

framework/core/src/Api/Controller/ListDiscussionsController.php

@@ -121,7 +121,9 @@ class ListDiscussionsController extends AbstractListController
             }
         }
 
-        $results = $results->getResults()->load($include);
+        $results = $results->getResults();
+
+        $this->loadRelations($results, $include);
 
         if ($relations = array_intersect($include, ['firstPost', 'lastPost', 'mostRelevantPost'])) {
             foreach ($results as $discussion) {

framework/core/src/Api/Controller/ListGroupsController.php

@@ -28,6 +28,10 @@ class ListGroupsController extends AbstractListController
     {
         $actor = $request->getAttribute('actor');
 
-        return Group::whereVisibleTo($actor)->get();
+        $results = Group::whereVisibleTo($actor)->get();
+
+        $this->loadRelations($results, []);
+
+        return $results;
     }
 }

framework/core/src/Api/Controller/ListNotificationsController.php

@@ -76,9 +76,11 @@ class ListNotificationsController extends AbstractListController
             $include[] = 'subject';
         }
 
-        $notifications = $this->notifications->findByUser($actor, $limit + 1, $offset)
+        $notifications = $this->notifications->findByUser($actor, $limit + 1, $offset);
-            ->load(array_diff($include, ['subject.discussion']))
+
-            ->all();
+        $this->loadRelations($notifications, array_diff($include, ['subject.discussion']));
+
+        $notifications = $notifications->all();
 
         $areMoreResults = false;
 

framework/core/src/Api/Controller/ListPostsController.php

@@ -104,7 +104,11 @@ class ListPostsController extends AbstractListController
             $include[] = 'user.groups';
         }
 
-        return $results->getResults()->load($include);
+        $results = $results->getResults();
+
+        $this->loadRelations($results, $include);
+
+        return $results;
     }
 
     /**

framework/core/src/Api/Controller/ListUsersController.php

@@ -105,6 +105,10 @@ class ListUsersController extends AbstractListController
             $results->areMoreResults() ? null : 0
         );
 
-        return $results->getResults()->load($include);
+        $results = $results->getResults();
+
+        $this->loadRelations($results, $include);
+
+        return $results;
     }
 }

framework/core/src/Extend/ApiController.php

@@ -29,6 +29,7 @@ class ApiController implements ExtenderInterface
     private $addSortFields = [];
     private $removeSortFields = [];
     private $sort;
+    private $load = [];
 
     /**
      * @param string $controllerClass The ::class attribute of the controller you are modifying.
@@ -216,6 +217,27 @@ class ApiController implements ExtenderInterface
         return $this;
     }
 
+    /**
+     * Eager loads relationships needed for serializer logic.
+     *
+     * First level relationships will be loaded regardless of whether they are included in the response.
+     * Sublevel relationships will only be loaded if the upper level was included or manually loaded.
+     *
+     * @example If a relationship such as: 'relation.subRelation' is specified,
+     * it will only be loaded if 'relation' is or has been loaded.
+     * To force load the relationship, both levels have to be specified,
+     * example: ['relation', 'relation.subRelation'].
+     *
+     * @param string|array
+     * @return self
+     */
+    public function load($relations)
+    {
+        $this->load = array_merge($this->load, (array) $relations);
+
+        return $this;
+    }
+
     public function extend(Container $container, Extension $extension = null)
     {
         $this->beforeDataCallbacks[] = function (AbstractSerializeController $controller) use ($container) {
@@ -281,6 +303,8 @@ class ApiController implements ExtenderInterface
             $beforeSerializationCallback = ContainerUtil::wrapCallback($beforeSerializationCallback, $container);
             AbstractSerializeController::addSerializationPreparationCallback($this->controllerClass, $beforeSerializationCallback);
         }
+
+        AbstractSerializeController::setLoadRelations($this->controllerClass, $this->load);
     }
 
     /**

framework/core/tests/integration/extenders/ApiControllerTest.php

@@ -12,6 +12,7 @@ namespace Flarum\Tests\integration\extenders;
 use Carbon\Carbon;
 use Flarum\Api\Controller\AbstractShowController;
 use Flarum\Api\Controller\ListDiscussionsController;
+use Flarum\Api\Controller\ListUsersController;
 use Flarum\Api\Controller\ShowDiscussionController;
 use Flarum\Api\Controller\ShowForumController;
 use Flarum\Api\Controller\ShowPostController;
@@ -22,6 +23,7 @@ use Flarum\Api\Serializer\PostSerializer;
 use Flarum\Api\Serializer\UserSerializer;
 use Flarum\Discussion\Discussion;
 use Flarum\Extend;
+use Flarum\Post\Post;
 use Flarum\Testing\integration\RetrievesAuthorizedUsers;
 use Flarum\Testing\integration\TestCase;
 use Flarum\User\User;
@@ -47,6 +49,11 @@ class ApiControllerTest extends TestCase
                 ['id' => 2, 'title' => 'Custom Discussion Title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 3, 'first_post_id' => 0, 'comment_count' => 1, 'is_private' => 0],
                 ['id' => 3, 'title' => 'Custom Discussion Title', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'first_post_id' => 0, 'comment_count' => 1, 'is_private' => 0],
             ],
+            'posts' => [
+                ['id' => 1, 'discussion_id' => 3, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'discussionRenamed', 'content' => '<t><p>can i haz relationz?</p></t>'],
+                ['id' => 2, 'discussion_id' => 2, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'discussionRenamed', 'content' => '<t><p>can i haz relationz?</p></t>'],
+                ['id' => 3, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'discussionRenamed', 'content' => '<t><p>can i haz relationz?</p></t>'],
+            ],
         ]);
     }
 
@@ -652,6 +659,150 @@ class ApiControllerTest extends TestCase
         $this->assertEquals(200, $response->getStatusCode());
         $this->assertEquals([2, 1, 3], Arr::pluck($payload['data'], 'id'));
     }
+
+    /**
+     * @test
+     */
+    public function custom_first_level_relation_is_not_loaded_by_default()
+    {
+        $users = null;
+
+        $this->extend(
+            (new Extend\Model(User::class))
+                ->hasOne('firstLevelRelation', Post::class, 'user_id'),
+            (new Extend\ApiController(ListUsersController::class))
+                ->prepareDataForSerialization(function ($controller, $data) use (&$users) {
+                    $users = $data;
+
+                    return [];
+                })
+        );
+
+        $this->send(
+            $this->request('GET', '/api/users', [
+                'authenticatedAs' => 1,
+            ])
+        );
+
+        $this->assertTrue($users->filter->relationLoaded('firstLevelRelation')->isEmpty());
+    }
+
+    /**
+     * @test
+     */
+    public function custom_first_level_relation_is_loaded_if_added()
+    {
+        $users = null;
+
+        $this->extend(
+            (new Extend\Model(User::class))
+                ->hasOne('firstLevelRelation', Post::class, 'user_id'),
+            (new Extend\ApiController(ListUsersController::class))
+                ->load('firstLevelRelation')
+                ->prepareDataForSerialization(function ($controller, $data) use (&$users) {
+                    $users = $data;
+
+                    return [];
+                })
+        );
+
+        $this->send(
+            $this->request('GET', '/api/users', [
+                'authenticatedAs' => 1,
+            ])
+        );
+
+        $this->assertFalse($users->filter->relationLoaded('firstLevelRelation')->isEmpty());
+    }
+
+    /**
+     * @test
+     */
+    public function custom_second_level_relation_is_not_loaded_by_default()
+    {
+        $users = null;
+
+        $this->extend(
+            (new Extend\Model(User::class))
+                ->hasOne('firstLevelRelation', Post::class, 'user_id'),
+            (new Extend\Model(Post::class))
+                ->belongsTo('secondLevelRelation', Discussion::class),
+            (new Extend\ApiController(ListUsersController::class))
+                ->prepareDataForSerialization(function ($controller, $data) use (&$users) {
+                    $users = $data;
+
+                    return [];
+                })
+        );
+
+        $this->send(
+            $this->request('GET', '/api/users', [
+                'authenticatedAs' => 1,
+            ])
+        );
+
+        $this->assertTrue($users->pluck('firstLevelRelation')->filter->relationLoaded('secondLevelRelation')->isEmpty());
+    }
+
+    /**
+     * @test
+     */
+    public function custom_second_level_relation_is_loaded_if_added()
+    {
+        $users = null;
+
+        $this->extend(
+            (new Extend\Model(User::class))
+                ->hasOne('firstLevelRelation', Post::class, 'user_id'),
+            (new Extend\Model(Post::class))
+                ->belongsTo('secondLevelRelation', Discussion::class),
+            (new Extend\ApiController(ListUsersController::class))
+                ->load(['firstLevelRelation', 'firstLevelRelation.secondLevelRelation'])
+                ->prepareDataForSerialization(function ($controller, $data) use (&$users) {
+                    $users = $data;
+
+                    return [];
+                })
+        );
+
+        $this->send(
+            $this->request('GET', '/api/users', [
+                'authenticatedAs' => 1,
+            ])
+        );
+
+        $this->assertFalse($users->pluck('firstLevelRelation')->filter->relationLoaded('secondLevelRelation')->isEmpty());
+    }
+
+    /**
+     * @test
+     */
+    public function custom_second_level_relation_is_not_loaded_when_first_level_is_not()
+    {
+        $users = null;
+
+        $this->extend(
+            (new Extend\Model(User::class))
+                ->hasOne('firstLevelRelation', Post::class, 'user_id'),
+            (new Extend\Model(Post::class))
+                ->belongsTo('secondLevelRelation', Discussion::class),
+            (new Extend\ApiController(ListUsersController::class))
+                ->load(['firstLevelRelation.secondLevelRelation'])
+                ->prepareDataForSerialization(function ($controller, $data) use (&$users) {
+                    $users = $data;
+
+                    return [];
+                })
+        );
+
+        $this->send(
+            $this->request('GET', '/api/users', [
+                'authenticatedAs' => 1,
+            ])
+        );
+
+        $this->assertTrue($users->pluck('firstLevelRelation')->filter->relationLoaded('secondLevelRelation')->isEmpty());
+    }
 }
 
 class CustomDiscussionSerializer extends DiscussionSerializer