diff --git a/extensions/mentions/extend.php b/extensions/mentions/extend.php index 2e7b64784..52ee37717 100644 --- a/extensions/mentions/extend.php +++ b/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) diff --git a/extensions/mentions/src/ConfigureMentions.php b/extensions/mentions/src/ConfigureMentions.php index 18a156fe2..ddc700471 100644 --- a/extensions/mentions/src/ConfigureMentions.php +++ b/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@["“](?((?!"#[a-z]{0,3}[0-9]+).)+)["”]#(?[0-9]+)\b/'; + public const USER_MENTION_WITH_USERNAME_REGEX = '/\B@(?[a-z0-9_-]+)(?!#)/i'; + public const POST_MENTION_WITH_DISPLAY_NAME_REGEX = '/\B@["“](?((?!"#[a-z]{0,3}[0-9]+).)+)["”]#p(?[0-9]+)\b/'; + public const GROUP_MENTION_WITH_NAME_REGEX = '/\B@["“](?((?!"#[a-z]{0,3}[0-9]+).)+)["|”]#g(?[0-9]+)\b/'; + public const TAG_MENTION_WITH_SLUG_REGEX = '/(?:[^“"]|^)\B#(?[-_\p{L}\p{N}\p{M}]+)\b/ui'; + /** * @var UrlGenerator */ @@ -74,27 +78,29 @@ class ConfigureMentions '; $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@["“](?((?!"#[a-z]{0,3}[0-9]+).)+)["”]#(?[0-9]+)\b/', $tagName); - $config->Preg->match('/\B@(?[a-z0-9_-]+)(?!#)/i', $tagName); + $config->Preg->match(self::USER_MENTION_WITH_DISPLAY_NAME_REGEX, $tagName); + $config->Preg->match(self::USER_MENTION_WITH_USERNAME_REGEX, $tagName); } /** * @param FormatterTag $tag + * @param array $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@["“](?((?!"#[a-z]{0,3}[0-9]+).)+)["”]#p(?[0-9]+)\b/', $tagName); + $config->Preg->match(self::POST_MENTION_WITH_DISPLAY_NAME_REGEX, $tagName); } /** * @param FormatterTag $tag + * @param array $mentions * @return bool|void */ - public static function addPostId($tag, User $actor) + public static function addPostId($tag, array $mentions) { - $post = resolve(PostRepository::class) - ->queryVisibleTo($actor) - ->find($tag->getAttribute('id')); + $post = $mentions['posts']->where('id', $tag->getAttribute('id'))->first(); 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@["“](?((?!"#[a-z]{0,3}[0-9]+).)+)["|”]#g(?[0-9]+)\b/', $tagName); + $config->Preg->match(self::GROUP_MENTION_WITH_NAME_REGEX, $tagName); } /** + * @param FormatterTag $tag + * @param User $actor + * @param array $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) - ->queryVisibleTo($actor) - ->find($id); + $group = $mentions['groups']->where('id', $id)->first(); 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#(?[-_\p{L}\p{N}\p{M}]+)\b/ui', $tagName); + $config->Preg->match(self::TAG_MENTION_WITH_SLUG_REGEX, $tagName); } /** + * @param FormatterTag $tag + * @param array $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) - ->queryVisibleTo($actor) - ->firstWhere('slug', $tag->getAttribute('slug')); + $model = $mentions['tags']->where('slug', $tag->getAttribute('slug'))->first(); if ($model) { $tag->setAttribute('id', (string) $model->id); diff --git a/extensions/mentions/src/Formatter/EagerLoadMentionedModels.php b/extensions/mentions/src/Formatter/EagerLoadMentionedModels.php new file mode 100644 index 000000000..1033f45ac --- /dev/null +++ b/extensions/mentions/src/Formatter/EagerLoadMentionedModels.php @@ -0,0 +1,120 @@ +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(); + } +} diff --git a/extensions/mentions/src/Job/SendMentionsNotificationsJob.php b/extensions/mentions/src/Job/SendMentionsNotificationsJob.php new file mode 100644 index 000000000..f3ed2d7b0 --- /dev/null +++ b/extensions/mentions/src/Job/SendMentionsNotificationsJob.php @@ -0,0 +1,108 @@ +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); + } +} diff --git a/extensions/mentions/src/Listener/UpdateMentionsMetadataWhenVisible.php b/extensions/mentions/src/Listener/UpdateMentionsMetadataWhenVisible.php index df0a1af1f..51cc329fa 100755 --- a/extensions/mentions/src/Listener/UpdateMentionsMetadataWhenVisible.php +++ b/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\Notification\PostMentionedBlueprint; -use Flarum\Mentions\Notification\UserMentionedBlueprint; -use Flarum\Notification\NotificationSyncer; +use Flarum\Mentions\Job\SendMentionsNotificationsJob; 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) diff --git a/framework/core/src/Formatter/Formatter.php b/framework/core/src/Formatter/Formatter.php index 2176cfb69..f47daae69 100644 --- a/framework/core/src/Formatter/Formatter.php +++ b/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) { diff --git a/framework/core/src/Notification/NotificationSyncer.php b/framework/core/src/Notification/NotificationSyncer.php index 752538cab..18d845a8d 100644 --- a/framework/core/src/Notification/NotificationSyncer.php +++ b/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. * diff --git a/framework/core/tests/integration/notification/NotificationSyncerTest.php b/framework/core/tests/integration/notification/NotificationSyncerTest.php index cbe9287ea..fdcbdfcfa 100644 --- a/framework/core/tests/integration/notification/NotificationSyncerTest.php +++ b/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 $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 $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