mirror/php-flarum
1
0
Fork 0
mirror of https://github.com/flarum/core.git synced 2025-08-17 22:01:44 +02:00
Code Issues Releases Wiki Activity

Improve search performance (#1339)

Browse Source 
* Improve fulltext gambit

* Only search in visible posts

This change relies on the `visibility-scoping` branch to be merged.

* Change posts table to use InnoDB engine

Doing a JOIN between an InnoDB table (discussions) and a MyISAM table
(posts) is very very (very) bad for performance. FULLTEXT indexes are
fully supported in InnoDB now, and it is a superior engine in every
other way, so there is no longer any reason to be using MyISAM.

* Use ::class

* Only search for comment posts

* Add fulltext index to discussions.title

* Fix migration not working if there is a table prefix

* Update frontend appearance

* Apply fixes from StyleCI

[ci skip] [skip ci]

* Show search result excerpts on mobile
This commit is contained in:
Toby Zerner
2018-02-08 06:38:08 +10:30
committed by GitHub
parent 80ec3b5e17
commit 322a84f516
18 changed files with 278 additions and 299 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
src/Api/Controller/ListDiscussionsController.php
View File

@@ -31,9 +31,8 @@ class ListDiscussionsController extends AbstractListController
public $include = [
'startUser',
'lastUser',
'relevantPosts',
'relevantPosts.discussion',
'relevantPosts.user'
'mostRelevantPost',
'mostRelevantPost.user'
];
/**
@@ -84,7 +83,7 @@ class ListDiscussionsController extends AbstractListController
$offset = $this->extractOffset($request);
$load = array_merge($this->extractInclude($request), ['state']);
$results = $this->searcher->search($criteria, $limit, $offset, $load);
$results = $this->searcher->search($criteria, $limit, $offset);
$document->addPaginationLinks(
$this->url->to('api')->route('discussions.index'),
@@ -94,7 +93,7 @@ class ListDiscussionsController extends AbstractListController
$results->areMoreResults() ? null : 0
);
$results = $results->getResults();
$results = $results->getResults()->load($load);
if ($relations = array_intersect($load, ['startPost', 'lastPost'])) {
foreach ($results as $discussion) {

4
src/Api/Serializer/BasicDiscussionSerializer.php
View File

@@ -84,9 +84,9 @@ class BasicDiscussionSerializer extends AbstractSerializer
/**
* @return \Tobscure\JsonApi\Relationship
*/
protected function relevantPosts($discussion)
protected function mostRelevantPost($discussion)
{
return $this->hasMany($discussion, BasicPostSerializer::class);
return $this->hasOne($discussion, PostSerializer::class);
}
/**

10
src/Discussion/Discussion.php
View File

@@ -382,6 +382,16 @@ class Discussion extends AbstractModel
return $this->belongsTo(User::class, 'last_user_id');
}
/**
* Define the relationship with the discussion's most relevant post.
*
* @return \Illuminate\Database\Eloquent\Relations\BelongsTo
*/
public function mostRelevantPost()
{
return $this->belongsTo(Post::class, 'most_relevant_post_id');
}
/**
* Define the relationship with the discussion's readers.
*

72
src/Discussion/Search/DiscussionSearcher.php
View File

@@ -11,26 +11,20 @@
namespace Flarum\Discussion\Search;
use Flarum\Discussion\Discussion;
use Flarum\Discussion\DiscussionRepository;
use Flarum\Discussion\Event\Searching;
use Flarum\Post\PostRepository;
use Flarum\Search\ApplySearchParametersTrait;
use Flarum\Search\GambitManager;
use Flarum\Search\SearchCriteria;
use Flarum\Search\SearchResults;
use Illuminate\Database\Eloquent\Collection;
use Illuminate\Contracts\Events\Dispatcher;
/**
* Takes a DiscussionSearchCriteria object, performs a search using gambits,
* and spits out a DiscussionSearchResults object.
*/
class DiscussionSearcher
{
use ApplySearchParametersTrait;
/**
* @var \Flarum\Search\GambitManager
* @var GambitManager
*/
protected $gambits;
@@ -40,37 +34,34 @@ class DiscussionSearcher
protected $discussions;
/**
* @var PostRepository
* @var Dispatcher
*/
protected $posts;
protected $events;
/**
* @param \Flarum\Search\GambitManager $gambits
* @param GambitManager $gambits
* @param DiscussionRepository $discussions
* @param PostRepository $posts
* @param Dispatcher $events
*/
public function __construct(
GambitManager $gambits,
DiscussionRepository $discussions,
PostRepository $posts
) {
public function __construct(GambitManager $gambits, DiscussionRepository $discussions, Dispatcher $events)
{
$this->gambits = $gambits;
$this->discussions = $discussions;
$this->posts = $posts;
$this->events = $events;
}
/**
* @param SearchCriteria $criteria
* @param int|null $limit
* @param int $offset
* @param array $load An array of relationships to load on the results.
*
* @return SearchResults
*/
public function search(SearchCriteria $criteria, $limit = null, $offset = 0, array $load = [])
public function search(SearchCriteria $criteria, $limit = null, $offset = 0)
{
$actor = $criteria->actor;
$query = $this->discussions->query()->whereVisibleTo($actor);
$query = $this->discussions->query()->select('discussions.*')->whereVisibleTo($actor);
// Construct an object which represents this search for discussions.
// Apply gambits to it, sort, and paging criteria. Also give extensions
@@ -82,8 +73,7 @@ class DiscussionSearcher
$this->applyOffset($search, $offset);
$this->applyLimit($search, $limit + 1);
// TODO: inject dispatcher
event(new Searching($search, $criteria));
$this->events->dispatch(new Searching($search, $criteria));
// Execute the search query and retrieve the results. We get one more
// results than the user asked for, so that we can say if there are more
@@ -96,42 +86,6 @@ class DiscussionSearcher
$discussions->pop();
}
// The relevant posts relationship isn't a typical Eloquent
// relationship; rather, we need to extract that information from our
// search object. We will delegate that task and prevent Eloquent
// from trying to load it.
if (in_array('relevantPosts', $load)) {
$this->loadRelevantPosts($discussions, $search);
$load = array_diff($load, ['relevantPosts', 'relevantPosts.discussion', 'relevantPosts.user']);
}
Discussion::setStateUser($actor);
$discussions->load($load);
return new SearchResults($discussions, $areMoreResults);
}
/**
* Load relevant posts onto each discussion using information from the
* search.
*
* @param Collection $discussions
* @param DiscussionSearch $search
*/
protected function loadRelevantPosts(Collection $discussions, DiscussionSearch $search)
{
$postIds = [];
foreach ($search->getRelevantPostIds() as $relevantPostIds) {
$postIds = array_merge($postIds, array_slice($relevantPostIds, 0, 2));
}
$posts = $postIds ? $this->posts->findByIds($postIds, $search->getActor())->load('user')->all() : [];
foreach ($discussions as $discussion) {
$discussion->relevantPosts = array_filter($posts, function ($post) use ($discussion) {
return $post->discussion_id == $discussion->id;
});
}
}
}

24
src/Discussion/Search/Fulltext/DriverInterface.php
View File

@@ -1,24 +0,0 @@
<?php
/*
* This file is part of Flarum.
*
* (c) Toby Zerner <toby.zerner@gmail.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Flarum\Discussion\Search\Fulltext;
interface DriverInterface
{
/**
* Return an array of arrays of post IDs, grouped by discussion ID, which
* match the given string.
*
* @param string $string
* @return array
*/
public function match($string);
}

36
src/Discussion/Search/Fulltext/MySqlFulltextDriver.php
View File

@@ -1,36 +0,0 @@
<?php
/*
* This file is part of Flarum.
*
* (c) Toby Zerner <toby.zerner@gmail.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Flarum\Discussion\Search\Fulltext;
use Flarum\Post\Post;
class MySqlFulltextDriver implements DriverInterface
{
/**
* {@inheritdoc}
*/
public function match($string)
{
$discussionIds = Post::where('type', 'comment')
->whereRaw('MATCH (`content`) AGAINST (? IN BOOLEAN MODE)', [$string])
->orderByRaw('MATCH (`content`) AGAINST (?) DESC', [$string])
->pluck('discussion_id', 'id');
$relevantPostIds = [];
foreach ($discussionIds as $postId => $discussionId) {
$relevantPostIds[$discussionId][] = $postId;
}
return $relevantPostIds;
}
}

40
src/Discussion/Search/Gambit/FulltextGambit.php
View File

@@ -12,26 +12,14 @@
namespace Flarum\Discussion\Search\Gambit;
use Flarum\Discussion\Search\DiscussionSearch;
use Flarum\Discussion\Search\Fulltext\DriverInterface;
use Flarum\Event\ScopeModelVisibility;
use Flarum\Post\Post;
use Flarum\Search\AbstractSearch;
use Flarum\Search\GambitInterface;
use LogicException;
class FulltextGambit implements GambitInterface
{
/**
* @var \Flarum\Discussion\Search\Fulltext\DriverInterface
*/
protected $fulltext;
/**
* @param \Flarum\Discussion\Search\Fulltext\DriverInterface $fulltext
*/
public function __construct(DriverInterface $fulltext)
{
$this->fulltext = $fulltext;
}
/**
* {@inheritdoc}
*/
@@ -41,14 +29,22 @@ class FulltextGambit implements GambitInterface
throw new LogicException('This gambit can only be applied on a DiscussionSearch');
}
$relevantPostIds = $this->fulltext->match($bit);
$search->getQuery()
->selectRaw('SUBSTRING_INDEX(GROUP_CONCAT(posts.id ORDER BY MATCH(posts.content) AGAINST (?) DESC), \',\', 1) as most_relevant_post_id', [$bit])
->leftJoin('posts', 'posts.discussion_id', '=', 'discussions.id')
->where('posts.type', 'comment')
->where(function ($query) use ($search) {
event(new ScopeModelVisibility(Post::query()->setQuery($query), $search->getActor(), 'view'));
})
->where(function ($query) use ($bit) {
$query->whereRaw('MATCH(discussions.title) AGAINST (? IN BOOLEAN MODE)', [$bit])
->orWhereRaw('MATCH(posts.content) AGAINST (? IN BOOLEAN MODE)', [$bit]);
})
->groupBy('posts.discussion_id');
$discussionIds = array_keys($relevantPostIds);
$search->setRelevantPostIds($relevantPostIds);
$search->getQuery()->whereIn('id', $discussionIds);
$search->setDefaultSort(['id' => $discussionIds]);
$search->setDefaultSort(function ($query) use ($bit) {
$query->orderByRaw('MATCH(discussions.title) AGAINST (?) desc', [$bit]);
$query->orderByRaw('MATCH(posts.content) AGAINST (?) desc', [$bit]);
});
}
}

4
src/Search/AbstractSearch.php
View File

@@ -85,12 +85,12 @@ abstract class AbstractSearch
* Set the default sort order for the search. This will only be applied if
* a sort order has not been specified in the search criteria.
*
* @param array $defaultSort An array of sort-order pairs, where the column
* @param mixed $defaultSort An array of sort-order pairs, where the column
* is the key, and the order is the value. The order may be 'asc',
* 'desc', or an array of IDs to order by.
* @return mixed
*/
public function setDefaultSort(array $defaultSort)
public function setDefaultSort($defaultSort)
{
$this->defaultSort = $defaultSort;
}

16
src/Search/ApplySearchParametersTrait.php
View File

@@ -23,13 +23,17 @@ trait ApplySearchParametersTrait
{
$sort = $sort ?: $search->getDefaultSort();
foreach ($sort as $field => $order) {
if (is_array($order)) {
foreach ($order as $value) {
$search->getQuery()->orderByRaw(snake_case($field).' != ?', [$value]);
if (is_callable($sort)) {
$sort($search->getQuery());
} else {
foreach ($sort as $field => $order) {
if (is_array($order)) {
foreach ($order as $value) {
$search->getQuery()->orderByRaw(snake_case($field).' != ?', [$value]);
}
} else {
$search->getQuery()->orderBy(snake_case($field), $order);
}
} else {
$search->getQuery()->orderBy(snake_case($field), $order);
}
}
}