perf: speed up post creation time (#3808)

* chore: drop unused visibility checking in notif syncer

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* perf: eager load parsed mentions

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* perf: eager load some relations needed for visibility checking

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* perf: trigger mentions notifications in a queueable job

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

* Apply fixes from StyleCI

* fix: broken tag mentions

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>

---------

Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
Co-authored-by: StyleCI Bot <bot@styleci.io>

extensions/mentions/extend.php

@@ -40,6 +40,7 @@ return [
 
     (new Extend\Formatter)
         ->configure(ConfigureMentions::class)
+        ->parse(Formatter\EagerLoadMentionedModels::class)
         ->render(Formatter\FormatPostMentions::class)
         ->render(Formatter\FormatUserMentions::class)
         ->render(Formatter\FormatGroupMentions::class)

extensions/mentions/src/ConfigureMentions.php

@@ -11,13 +11,11 @@ namespace Flarum\Mentions;
 
 use Flarum\Extension\ExtensionManager;
 use Flarum\Group\Group;
-use Flarum\Group\GroupRepository;
 use Flarum\Http\UrlGenerator;
-use Flarum\Post\PostRepository;
 use Flarum\Settings\SettingsRepositoryInterface;
 use Flarum\Tags\Tag;
-use Flarum\Tags\TagRepository;
 use Flarum\User\User;
+use Illuminate\Database\Eloquent\Collection;
 use s9e\TextFormatter\Configurator;
 use s9e\TextFormatter\Parser\Tag as FormatterTag;
 
@@ -26,6 +24,12 @@ use s9e\TextFormatter\Parser\Tag as FormatterTag;
  */
 class ConfigureMentions
 {
+    public const USER_MENTION_WITH_DISPLAY_NAME_REGEX = '/\B@["“](?<displayname>((?!"#[a-z]{0,3}[0-9]+).)+)["”]#(?<id>[0-9]+)\b/';
+    public const USER_MENTION_WITH_USERNAME_REGEX = '/\B@(?<username>[a-z0-9_-]+)(?!#)/i';
+    public const POST_MENTION_WITH_DISPLAY_NAME_REGEX = '/\B@["“](?<displayname>((?!"#[a-z]{0,3}[0-9]+).)+)["”]#p(?<id>[0-9]+)\b/';
+    public const GROUP_MENTION_WITH_NAME_REGEX = '/\B@["“](?<groupname>((?!"#[a-z]{0,3}[0-9]+).)+)["|”]#g(?<id>[0-9]+)\b/';
+    public const TAG_MENTION_WITH_SLUG_REGEX = '/(?:[^“"]|^)\B#(?<slug>[-_\p{L}\p{N}\p{M}]+)\b/ui';
+
     /**
      * @var UrlGenerator
      */
@@ -74,27 +78,29 @@ class ConfigureMentions
             </xsl:choose>';
 
         $tag->filterChain->prepend([static::class, 'addUserId'])
-            ->setJS('function(tag) { return flarum.extensions["flarum-mentions"].filterUserMentions(tag); }');
+            ->setJS('function(tag) { return flarum.extensions["flarum-mentions"].filterUserMentions(tag); }')
+            ->addParameterByName('mentions');
 
         $tag->filterChain->append([static::class, 'dummyFilter'])
             ->setJs('function(tag) { return flarum.extensions["flarum-mentions"].postFilterUserMentions(tag); }');
 
-        $config->Preg->match('/\B@["“](?<displayname>((?!"#[a-z]{0,3}[0-9]+).)+)["”]#(?<id>[0-9]+)\b/', $tagName);
+        $config->Preg->match(self::USER_MENTION_WITH_DISPLAY_NAME_REGEX, $tagName);
-        $config->Preg->match('/\B@(?<username>[a-z0-9_-]+)(?!#)/i', $tagName);
+        $config->Preg->match(self::USER_MENTION_WITH_USERNAME_REGEX, $tagName);
     }
 
     /**
      * @param FormatterTag $tag
+     * @param array<string, Collection> $mentions
      * @return bool|void
      */
-    public static function addUserId($tag)
+    public static function addUserId($tag, array $mentions)
     {
         $allow_username_format = (bool) resolve(SettingsRepositoryInterface::class)->get('flarum-mentions.allow_username_format');
 
         if ($tag->hasAttribute('username') && $allow_username_format) {
-            $user = User::where('username', $tag->getAttribute('username'))->first();
+            $user = $mentions['users']->where('username', $tag->getAttribute('username'))->first();
         } elseif ($tag->hasAttribute('id')) {
-            $user = User::find($tag->getAttribute('id'));
+            $user = $mentions['users']->where('id', $tag->getAttribute('id'))->first();
         }
 
         if (isset($user)) {
@@ -133,23 +139,22 @@ class ConfigureMentions
         $tag->filterChain
             ->prepend([static::class, 'addPostId'])
             ->setJS('function(tag) { return flarum.extensions["flarum-mentions"].filterPostMentions(tag); }')
-            ->addParameterByName('actor');
+            ->addParameterByName('mentions');
 
         $tag->filterChain->append([static::class, 'dummyFilter'])
             ->setJs('function(tag) { return flarum.extensions["flarum-mentions"].postFilterPostMentions(tag); }');
 
-        $config->Preg->match('/\B@["“](?<displayname>((?!"#[a-z]{0,3}[0-9]+).)+)["”]#p(?<id>[0-9]+)\b/', $tagName);
+        $config->Preg->match(self::POST_MENTION_WITH_DISPLAY_NAME_REGEX, $tagName);
     }
 
     /**
      * @param FormatterTag $tag
+     * @param array<string, Collection> $mentions
      * @return bool|void
      */
-    public static function addPostId($tag, User $actor)
+    public static function addPostId($tag, array $mentions)
     {
-        $post = resolve(PostRepository::class)
+        $post = $mentions['posts']->where('id', $tag->getAttribute('id'))->first();
-            ->queryVisibleTo($actor)
-            ->find($tag->getAttribute('id'));
 
         if ($post) {
             $tag->setAttribute('discussionid', (string) $post->discussion_id);
@@ -205,18 +210,22 @@ class ConfigureMentions
 
         $tag->filterChain->prepend([static::class, 'addGroupId'])
             ->setJS('function(tag) { return flarum.extensions["flarum-mentions"].filterGroupMentions(tag); }')
-            ->addParameterByName('actor');
+            ->addParameterByName('actor')
+            ->addParameterByName('mentions');
 
         $tag->filterChain->append([static::class, 'dummyFilter'])
             ->setJS('function(tag) { return flarum.extensions["flarum-mentions"].postFilterGroupMentions(tag); }');
 
-        $config->Preg->match('/\B@["“](?<groupname>((?!"#[a-z]{0,3}[0-9]+).)+)["|”]#g(?<id>[0-9]+)\b/', $tagName);
+        $config->Preg->match(self::GROUP_MENTION_WITH_NAME_REGEX, $tagName);
     }
 
     /**
+     * @param FormatterTag $tag
+     * @param User $actor
+     * @param array<string, Collection> $mentions
      * @return bool|void
      */
-    public static function addGroupId(FormatterTag $tag, User $actor)
+    public static function addGroupId(FormatterTag $tag, User $actor, array $mentions)
     {
         $id = $tag->getAttribute('id');
 
@@ -226,9 +235,7 @@ class ConfigureMentions
             return false;
         }
 
-        $group = resolve(GroupRepository::class)
+        $group = $mentions['groups']->where('id', $id)->first();
-            ->queryVisibleTo($actor)
-            ->find($id);
 
         if ($group) {
             $tag->setAttribute('id', $group->id);
@@ -293,24 +300,24 @@ class ConfigureMentions
         $tag->filterChain
             ->prepend([static::class, 'addTagId'])
             ->setJS('function(tag) { return flarum.extensions["flarum-mentions"].filterTagMentions(tag); }')
-            ->addParameterByName('actor');
+            ->addParameterByName('mentions');
 
         $tag->filterChain
             ->append([static::class, 'dummyFilter'])
             ->setJS('function(tag) { return flarum.extensions["flarum-mentions"].postFilterTagMentions(tag); }');
 
-        $config->Preg->match('/(?:[^“"]|^)\B#(?<slug>[-_\p{L}\p{N}\p{M}]+)\b/ui', $tagName);
+        $config->Preg->match(self::TAG_MENTION_WITH_SLUG_REGEX, $tagName);
     }
 
     /**
+     * @param FormatterTag $tag
+     * @param array<string, Collection> $mentions
      * @return true|void
      */
-    public static function addTagId(FormatterTag $tag, User $actor)
+    public static function addTagId(FormatterTag $tag, array $mentions)
     {
         /** @var Tag|null $model */
-        $model = resolve(TagRepository::class)
+        $model = $mentions['tags']->where('slug', $tag->getAttribute('slug'))->first();
-            ->queryVisibleTo($actor)
-            ->firstWhere('slug', $tag->getAttribute('slug'));
 
         if ($model) {
             $tag->setAttribute('id', (string) $model->id);

extensions/mentions/src/Formatter/EagerLoadMentionedModels.php

@@ -0,0 +1,120 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Mentions\Formatter;
+
+use Flarum\Extension\ExtensionManager;
+use Flarum\Group\GroupRepository;
+use Flarum\Mentions\ConfigureMentions;
+use Flarum\Post\PostRepository;
+use Flarum\Tags\TagRepository;
+use Flarum\User\User;
+use Illuminate\Database\Eloquent\Collection;
+use s9e\TextFormatter\Parser;
+
+class EagerLoadMentionedModels
+{
+    /**
+     * @var ExtensionManager
+     */
+    protected $extensions;
+
+    /**
+     * @var PostRepository
+     */
+    protected $posts;
+
+    /**
+     * @var GroupRepository
+     */
+    protected $groups;
+
+    /**
+     * @var TagRepository
+     */
+    protected $tags;
+
+    public function __construct(ExtensionManager $extensions, PostRepository $posts, GroupRepository $groups, TagRepository $tags)
+    {
+        $this->extensions = $extensions;
+        $this->posts = $posts;
+        $this->groups = $groups;
+        $this->tags = $tags;
+    }
+
+    /**
+     * @param mixed|\Flarum\Post\CommentPost|null $context
+     */
+    public function __invoke(Parser $parser, $context, string $text, ?User $actor): string
+    {
+        $callables = $this->getEagerLoaders();
+
+        $parser->registeredVars['mentions'] = [];
+
+        foreach ($callables as $modelType => $callable) {
+            $parser->registeredVars['mentions'][$modelType] = $callable($text, $actor);
+        }
+
+        return $text;
+    }
+
+    protected function getEagerLoaders(): array
+    {
+        $callables = [
+            'users' => [$this, 'eagerLoadUserMentions'],
+            'posts' => [$this, 'eagerLoadPostMentions'],
+            'groups' => [$this, 'eagerLoadGroupMentions'],
+        ];
+
+        if ($this->extensions->isEnabled('flarum-tags')) {
+            $callables['tags'] = [$this, 'eagerLoadTagMentions'];
+        }
+
+        return $callables;
+    }
+
+    protected function eagerLoadUserMentions(string $text, ?User $actor): Collection
+    {
+        preg_match_all(ConfigureMentions::USER_MENTION_WITH_USERNAME_REGEX, $text, $usernameMatches);
+        preg_match_all(ConfigureMentions::USER_MENTION_WITH_DISPLAY_NAME_REGEX, $text, $idMatches);
+
+        return User::query()
+            ->whereIn('username', $usernameMatches['username'])
+            ->orWhereIn('id', $idMatches['id'])
+            ->get();
+    }
+
+    protected function eagerLoadPostMentions(string $text, ?User $actor): Collection
+    {
+        preg_match_all(ConfigureMentions::POST_MENTION_WITH_DISPLAY_NAME_REGEX, $text, $matches);
+
+        return $this->posts
+            ->queryVisibleTo($actor)
+            ->findMany($matches['id']);
+    }
+
+    protected function eagerLoadGroupMentions(string $text, ?User $actor): Collection
+    {
+        preg_match_all(ConfigureMentions::GROUP_MENTION_WITH_NAME_REGEX, $text, $matches);
+
+        return $this->groups
+            ->queryVisibleTo($actor)
+            ->findMany($matches['id']);
+    }
+
+    protected function eagerLoadTagMentions(string $text, ?User $actor): Collection
+    {
+        preg_match_all(ConfigureMentions::TAG_MENTION_WITH_SLUG_REGEX, $text, $matches);
+
+        return $this->tags
+            ->queryVisibleTo($actor)
+            ->whereIn('slug', $matches['slug'])
+            ->get();
+    }
+}

extensions/mentions/src/Job/SendMentionsNotificationsJob.php

@@ -0,0 +1,108 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Mentions\Job;
+
+use Flarum\Mentions\Notification\GroupMentionedBlueprint;
+use Flarum\Mentions\Notification\PostMentionedBlueprint;
+use Flarum\Mentions\Notification\UserMentionedBlueprint;
+use Flarum\Notification\NotificationSyncer;
+use Flarum\Post\CommentPost;
+use Flarum\Post\Post;
+use Flarum\Queue\AbstractJob;
+use Flarum\User\User;
+
+class SendMentionsNotificationsJob extends AbstractJob
+{
+    /**
+     * @var CommentPost
+     */
+    protected $post;
+
+    /**
+     * @var array
+     */
+    protected $userMentions;
+
+    /**
+     * @var array
+     */
+    protected $postMentions;
+
+    /**
+     * @var array
+     */
+    protected $groupMentions;
+
+    /**
+     * @var NotificationSyncer
+     */
+    private $notifications;
+
+    public function __construct(CommentPost $post, array $userMentions, array $postMentions, array $groupMentions)
+    {
+        $this->post = $post;
+        $this->userMentions = $userMentions;
+        $this->postMentions = $postMentions;
+        $this->groupMentions = $groupMentions;
+    }
+
+    public function handle(NotificationSyncer $notifications): void
+    {
+        $this->notifications = $notifications;
+
+        $this->notifyAboutUserMentions($this->post, $this->userMentions);
+        $this->notifyAboutPostMentions($this->post, $this->postMentions);
+        $this->notifyAboutGroupMentions($this->post, $this->groupMentions);
+    }
+
+    protected function notifyAboutUserMentions(Post $post, array $mentioned)
+    {
+        $users = User::whereIn('id', $mentioned)
+            ->with('groups')
+            ->get()
+            ->filter(function ($user) use ($post) {
+                return $post->isVisibleTo($user) && $user->id !== $post->user_id;
+            })
+            ->all();
+
+        $this->notifications->sync(new UserMentionedBlueprint($post), $users);
+    }
+
+    protected function notifyAboutPostMentions(Post $reply, array $mentioned)
+    {
+        $posts = Post::with('user')
+            ->whereIn('id', $mentioned)
+            ->with('user.groups')
+            ->get()
+            ->filter(function (Post $post) use ($reply) {
+                return $post->user && $post->user_id !== $reply->user_id && $reply->isVisibleTo($post->user);
+            })
+            ->all();
+
+        foreach ($posts as $post) {
+            $this->notifications->sync(new PostMentionedBlueprint($post, $reply), [$post->user]);
+        }
+    }
+
+    protected function notifyAboutGroupMentions(Post $post, array $mentioned)
+    {
+        $users = User::whereHas('groups', function ($query) use ($mentioned) {
+            $query->whereIn('groups.id', $mentioned);
+        })
+            ->with('groups')
+            ->get()
+            ->filter(function (User $user) use ($post) {
+                return $post->isVisibleTo($user) && $user->id !== $post->user_id;
+            })
+            ->all();
+
+        $this->notifications->sync(new GroupMentionedBlueprint($post), $users);
+    }
+}

extensions/mentions/src/Listener/UpdateMentionsMetadataWhenVisible.php

@@ -11,34 +11,31 @@ namespace Flarum\Mentions\Listener;
 
 use Flarum\Approval\Event\PostWasApproved;
 use Flarum\Extension\ExtensionManager;
-use Flarum\Mentions\Notification\GroupMentionedBlueprint;
+use Flarum\Mentions\Job\SendMentionsNotificationsJob;
-use Flarum\Mentions\Notification\PostMentionedBlueprint;
-use Flarum\Mentions\Notification\UserMentionedBlueprint;
-use Flarum\Notification\NotificationSyncer;
 use Flarum\Post\CommentPost;
 use Flarum\Post\Event\Posted;
 use Flarum\Post\Event\Restored;
 use Flarum\Post\Event\Revised;
 use Flarum\Post\Post;
-use Flarum\User\User;
+use Illuminate\Contracts\Queue\Queue;
 use s9e\TextFormatter\Utils;
 
 class UpdateMentionsMetadataWhenVisible
 {
-    /**
-     * @var NotificationSyncer
-     */
-    protected $notifications;
-
     /**
      * @var ExtensionManager
      */
     protected $extensions;
 
-    public function __construct(NotificationSyncer $notifications, ExtensionManager $extensions)
+    /**
+     * @var Queue
+     */
+    protected $queue;
+
+    public function __construct(ExtensionManager $extensions, Queue $queue)
     {
-        $this->notifications = $notifications;
         $this->extensions = $extensions;
+        $this->queue = $queue;
     }
 
     /**
@@ -54,17 +51,17 @@ class UpdateMentionsMetadataWhenVisible
 
         $this->syncUserMentions(
             $event->post,
-            Utils::getAttributeValues($content, 'USERMENTION', 'id')
+            $userMentions = Utils::getAttributeValues($content, 'USERMENTION', 'id')
         );
 
         $this->syncPostMentions(
             $event->post,
-            Utils::getAttributeValues($content, 'POSTMENTION', 'id')
+            $postMentions = Utils::getAttributeValues($content, 'POSTMENTION', 'id')
         );
 
         $this->syncGroupMentions(
             $event->post,
-            Utils::getAttributeValues($content, 'GROUPMENTION', 'id')
+            $groupMentions = Utils::getAttributeValues($content, 'GROUPMENTION', 'id')
         );
 
         if ($this->extensions->isEnabled('flarum-tags')) {
@@ -73,56 +70,26 @@ class UpdateMentionsMetadataWhenVisible
                 Utils::getAttributeValues($content, 'TAGMENTION', 'id')
             );
         }
+
+        $this->queue->push(new SendMentionsNotificationsJob($event->post, $userMentions, $postMentions, $groupMentions));
     }
 
     protected function syncUserMentions(Post $post, array $mentioned)
     {
         $post->mentionsUsers()->sync($mentioned);
         $post->unsetRelation('mentionsUsers');
-
-        $users = User::whereIn('id', $mentioned)
-            ->get()
-            ->filter(function ($user) use ($post) {
-                return $post->isVisibleTo($user) && $user->id !== $post->user_id;
-            })
-            ->all();
-
-        $this->notifications->sync(new UserMentionedBlueprint($post), $users);
     }
 
     protected function syncPostMentions(Post $reply, array $mentioned)
     {
         $reply->mentionsPosts()->sync($mentioned);
         $reply->unsetRelation('mentionsPosts');
-
-        $posts = Post::with('user')
-            ->whereIn('id', $mentioned)
-            ->get()
-            ->filter(function (Post $post) use ($reply) {
-                return $post->user && $post->user_id !== $reply->user_id && $reply->isVisibleTo($post->user);
-            })
-            ->all();
-
-        foreach ($posts as $post) {
-            $this->notifications->sync(new PostMentionedBlueprint($post, $reply), [$post->user]);
-        }
     }
 
     protected function syncGroupMentions(Post $post, array $mentioned)
     {
         $post->mentionsGroups()->sync($mentioned);
         $post->unsetRelation('mentionsGroups');
-
-        $users = User::whereHas('groups', function ($query) use ($mentioned) {
-            $query->whereIn('groups.id', $mentioned);
-        })
-            ->get()
-            ->filter(function (User $user) use ($post) {
-                return $post->isVisibleTo($user) && $user->id !== $post->user_id;
-            })
-            ->all();
-
-        $this->notifications->sync(new GroupMentionedBlueprint($post), $users);
     }
 
     protected function syncTagMentions(Post $post, array $mentioned)

framework/core/src/Formatter/Formatter.php

@@ -85,7 +85,7 @@ class Formatter
      * @param string $text
      * @param mixed $context
      * @param User|null $user
-     * @return string
+     * @return string the parsed XML
      */
     public function parse($text, $context = null, User $user = null)
     {

framework/core/src/Notification/NotificationSyncer.php

@@ -9,7 +9,6 @@
 
 namespace Flarum\Notification;
 
-use Flarum\Database\AbstractModel;
 use Flarum\Notification\Blueprint\BlueprintInterface;
 use Flarum\Notification\Driver\NotificationDriverInterface;
 use Flarum\User\User;
@@ -75,12 +74,6 @@ class NotificationSyncer
                 continue;
             }
 
-            // To add access checking on notification subjects, we first attempt
-            // to load visible subjects to this user.
-            if (! $this->userCanSeeSubject($user, $blueprint->getSubject())) {
-                continue;
-            }
-
             $existing = $toDelete->first(function ($notification) use ($user) {
                 return $notification->user_id === $user->id;
             });
@@ -168,18 +161,6 @@ class NotificationSyncer
         Notification::whereIn('id', $ids)->update(['is_deleted' => $isDeleted]);
     }
 
-    /**
-     * Check access to determine if the recipient is allowed to receive the notification.
-     */
-    protected function userCanSeeSubject(User $user, ?AbstractModel $subject): bool
-    {
-        if ($subject && method_exists($subject, 'registerVisibilityScoper')) {
-            return (bool) $subject->newQuery()->whereVisibleTo($user)->find($subject->id);
-        }
-
-        return true;
-    }
-
     /**
      * Adds a notification driver to the list.
      *

framework/core/tests/integration/notification/NotificationSyncerTest.php

@@ -10,15 +10,11 @@
 namespace Flarum\Tests\integration\notification;
 
 use Carbon\Carbon;
-use Flarum\Api\Serializer\BasicDiscussionSerializer;
-use Flarum\Api\Serializer\BasicPostSerializer;
 use Flarum\Database\AbstractModel;
-use Flarum\Discussion\Discussion;
 use Flarum\Extend;
 use Flarum\Notification\Blueprint\BlueprintInterface;
 use Flarum\Notification\Notification;
 use Flarum\Notification\NotificationSyncer;
-use Flarum\Post\Post;
 use Flarum\Testing\integration\RetrievesAuthorizedUsers;
 use Flarum\Testing\integration\TestCase;
 use Flarum\User\User;
@@ -50,35 +46,6 @@ class NotificationSyncerTest extends TestCase
         ]);
     }
 
-    /**
-     * @dataProvider visibleSubjectsProvider
-     * @param class-string<AbstractModel> $subjectClass
-     * @test
-     */
-    public function can_receive_notification_for_visible_subjects(string $subjectClass, int $subjectId, string $serializer)
-    {
-        $this->expect_notification_count_from_sending_notification_type_with_subject(
-            2,
-            $subjectClass,
-            $subjectId,
-            $serializer
-        );
-    }
-
-    /**
-     * @dataProvider invisibleSubjectsProvider
-     * @test
-     */
-    public function cannot_receive_notification_for_restricted_subjects(string $subjectClass, int $subjectId, string $serializer)
-    {
-        $this->expect_notification_count_from_sending_notification_type_with_subject(
-            0,
-            $subjectClass,
-            $subjectId,
-            $serializer
-        );
-    }
-
     /**
      * @param class-string<AbstractModel> $subjectClass
      */
@@ -112,23 +79,6 @@ class NotificationSyncerTest extends TestCase
                 ->count()
         );
     }
-
-    public function visibleSubjectsProvider()
-    {
-        return [
-            [Post::class, 1, BasicPostSerializer::class],
-            [Discussion::class, 1, BasicDiscussionSerializer::class],
-        ];
-    }
-
-    public function invisibleSubjectsProvider()
-    {
-        return [
-            [Post::class, 2, BasicPostSerializer::class],
-            [Discussion::class, 2, BasicDiscussionSerializer::class],
-            [Post::class, 3, BasicPostSerializer::class],
-        ];
-    }
 }
 
 class CustomNotificationType implements BlueprintInterface