Compare commits

...

11 Commits

Author SHA1 Message Date
Bart van Bragt c5643bed31 fix: default search sort to latest and keep dropdown label in sync
- Search with no explicit sort now defaults to latest (updated_at desc)
  on both backend and frontend, matching the forum UX expectation.
- "All discussions" link from the search box routes to /?q=...&sort=latest
  so the sort dropdown shows "Latest" immediately after submission.
- extendDiscussionState injects sort=-lastPostedAt into API params when
  no sort is present, keeping direct-URL navigation (?q=anan) consistent
  with the backend default.
- Backend guard changed from phpSortField===null to empty($sorts), which
  is the correct sentinel since $sorts is the authoritative ES sort list.
2026-04-15 17:21:10 +02:00
Bart van Bragt 6bcd2becd5 fix: report actual failed item error in bulk seeding exception 2026-04-15 14:08:04 +02:00
Bart van Bragt 2706ed6be0 fix: index discussion on rename 2026-04-15 14:07:55 +02:00
Bart van Bragt 557604e426 fix: retry fill ES queries on transient connection failure 2026-04-15 14:07:43 +02:00
Bart van Bragt 08353d2473 perf: sort by date by default, skip scoring on field sorts
- Default sort changed from relevance to updated_at desc. This lets
  has_child use score_mode:none, which skips child scoring entirely
  and lets ES short-circuit early on large corpora.
- When a field sort is requested, score_mode is also none for the same
  reason. score_mode:sum is only used when sorting by relevance.
- track_total_hits:false avoids a full-index count on every query,
  allowing ES to stop once it has collected enough results.
- Strip all gambit operators (tag:foo, author:bar, is:unread, etc.)
  from the ES query string, not just is:private. Leaving them in caused
  operator:and to require the gambit tokens to appear literally in post
  content, producing zero results when gambits were combined with text.
2026-04-14 22:40:43 +02:00
Bart van Bragt 80357e6a77 fix: include --staging in build conflict suggestions 2026-04-14 16:46:06 +02:00
Bart van Bragt d8eebb183a fix: prevent ES CPU saturation from has_child scoring all posts
Without minimum_should_match=1 on the inner post bool query, Elasticsearch
defaults MSM to 0 whenever a filter clause is present. This caused has_child
to score every non-hidden post on every search request, saturating CPU on
shared ES nodes with large corpora (3.7M+ posts).

- Add BoolQuery subclass with create() override and minimumShouldMatch()
  (spatie/elasticsearch-query-builder 1.x uses `new self()` in create(),
  so subclassing requires overriding it)
- Set minimumShouldMatch(1) on the inner post query after adding the
  is_hidden filter, so only genuinely matching posts are scored
- Remove the operator('or') clause from buildShouldClauses() — with
  Turkish min_ngram=2, 'or' generates 2-gram tokens that match nearly
  every post, causing near-total index scans
- Add ES client timeouts (connect: 2s, query: 10s) to prevent Apache
  mod_php worker saturation when ES is slow or unreachable
2026-04-14 16:07:50 +02:00
Bart van Bragt c54f37349f fix: use Support\Collection for cachedGlobalPermission — pluck() returns plain Collection, not Eloquent 2026-04-14 11:27:02 +02:00
Bart van Bragt ca8c248870 fix: use Eloquent Collection in job — loadMissing does not exist on Support\Collection 2026-04-14 09:59:02 +02:00
Bart van Bragt bf37e01eb0 fix: read forum attribute lazily inside extend callback 2026-04-14 09:33:17 +02:00
Bart van Bragt 4709b9d1db fix: is_hidden boolean coercion and forum settings access 2026-04-14 08:59:19 +02:00
13 changed files with 185 additions and 60 deletions

View File

@ -34,5 +34,6 @@ return [
(new Flarum\Settings()) (new Flarum\Settings())
->default('blomstra-search.search-discussion-subjects', true) ->default('blomstra-search.search-discussion-subjects', true)
->default('blomstra-search.search-post-bodies', true) ->default('blomstra-search.search-post-bodies', true)
->default('blomstra-search.min-search-length', Commands\BuildCommand::DEFAULT_MIN_SEARCH_LENGTH), ->default('blomstra-search.min-search-length', Commands\BuildCommand::DEFAULT_MIN_SEARCH_LENGTH)
->serializeToForum('blomstraSearchMinLength', 'blomstra-search.min-search-length', 'intval'),
]; ];

2
js/dist/forum.js vendored

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

View File

@ -17,6 +17,12 @@ export default function extendDiscussionState() {
...params.page, ...params.page,
}; };
// Default to latest sort when searching without an explicit sort parameter.
// Matches the backend default and keeps the dropdown label in sync.
if (!params.sort) {
params.sort = '-lastPostedAt';
}
if (Array.isArray(params.include)) { if (Array.isArray(params.include)) {
params.include = params.include.join(','); params.include = params.include.join(',');
} }

View File

@ -66,7 +66,7 @@ export default class DiscussionsSearchSource implements SearchSource {
return [ return [
<li className="Dropdown-header">{app.translator.trans('core.forum.search.discussions_heading')}</li>, <li className="Dropdown-header">{app.translator.trans('core.forum.search.discussions_heading')}</li>,
<li> <li>
<LinkButton icon="fas fa-search" href={app.route('index', { q: query })}> <LinkButton icon="fas fa-search" href={app.route('index', { q: query, sort: 'latest' })}>
{app.translator.trans('core.forum.search.all_discussions_button', { query })} {app.translator.trans('core.forum.search.all_discussions_button', { query })}
</LinkButton> </LinkButton>
</li>, </li>,

View File

@ -9,16 +9,14 @@ import DiscussionsSearchSource from './SearchSources/DiscussionsSearchSource';
import extendDiscussionState from './PaginatedListStates/extendDiscussionState'; import extendDiscussionState from './PaginatedListStates/extendDiscussionState';
app.initializers.add('blomstra-search', () => { app.initializers.add('blomstra-search', () => {
const minLength = parseInt(app.data.settings['blomstra-search.min-search-length'] || String(Search.MIN_SEARCH_LEN), 10); extend(Search.prototype, 'sourceItems', function (this: Search<SearchAttrs>, items: ItemList<SearchSource>) {
// app.forum is not available during initializers (it is set after they run),
// so read the setting lazily here, at first render time.
const minLength = (app.forum.attribute('blomstraSearchMinLength') as number) || Search.MIN_SEARCH_LEN;
if (minLength !== Search.MIN_SEARCH_LEN) { if (minLength !== Search.MIN_SEARCH_LEN) {
// Flarum provides no extension point for MIN_SEARCH_LEN, so we overwrite the
// static property directly. TypeScript `readonly` is compile-time only — at
// runtime this is a plain property assignment and is safe as long as no code
// reads MIN_SEARCH_LEN before this initializer runs.
(Search as any).MIN_SEARCH_LEN = minLength; (Search as any).MIN_SEARCH_LEN = minLength;
} }
extend(Search.prototype, 'sourceItems', function (this: Search<SearchAttrs>, items: ItemList<SearchSource>) {
items.replace('discussions', new DiscussionsSearchSource()); items.replace('discussions', new DiscussionsSearchSource());
}); });
}); });

View File

@ -37,7 +37,7 @@ use Illuminate\Support\Str;
use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ServerRequestInterface;
use Psr\Log\LoggerInterface; use Psr\Log\LoggerInterface;
use Spatie\ElasticsearchQueryBuilder\Builder; use Spatie\ElasticsearchQueryBuilder\Builder;
use Spatie\ElasticsearchQueryBuilder\Queries\BoolQuery; use Blomstra\Search\Elasticsearch\BoolQuery;
use Spatie\ElasticsearchQueryBuilder\Queries\TermQuery; use Spatie\ElasticsearchQueryBuilder\Queries\TermQuery;
use Spatie\ElasticsearchQueryBuilder\Sorts\Sort; use Spatie\ElasticsearchQueryBuilder\Sorts\Sort;
use Tobscure\JsonApi\Document; use Tobscure\JsonApi\Document;
@ -78,27 +78,13 @@ class SearchController extends ListDiscussionsController
$offset = $this->extractOffset($request); $offset = $this->extractOffset($request);
$include = array_merge($this->extractInclude($request), ['state']); $include = array_merge($this->extractInclude($request), ['state']);
$query = BoolQuery::create()
// Always restrict to discussion documents; posts are only searched via has_child.
->add(TermQuery::create('join_field', 'discussion'), 'filter');
if (!empty($search)) {
$query->add($this->buildTextQuery($search, $actor));
}
$this->addFilters($query, $actor, $filters);
$builder = (new Builder($this->elastic))
->index(resolve('blomstra.search.elastic_index'))
->size($limit + 1)
->from($offset)
->addQuery($query);
$knownSortFields = array_merge(array_values($this->translateSort), ['rawId']); $knownSortFields = array_merge(array_values($this->translateSort), ['rawId']);
$logger = resolve(LoggerInterface::class); $logger = resolve(LoggerInterface::class);
$phpSortField = null; $phpSortField = null;
$phpSortDir = 'desc'; $phpSortDir = 'desc';
$needsScoring = true;
$sorts = [];
foreach ($this->extractSort($request) as $field => $direction) { foreach ($this->extractSort($request) as $field => $direction) {
$translated = $this->translateSort[$field] ?? $field; $translated = $this->translateSort[$field] ?? $field;
@ -108,7 +94,8 @@ class SearchController extends ListDiscussionsController
continue; continue;
} }
$builder->addSort(new Sort($translated, $direction)); $sorts[] = new Sort($translated, $direction);
$needsScoring = false;
if ($phpSortField === null && $translated !== 'rawId') { if ($phpSortField === null && $translated !== 'rawId') {
$phpSortField = $translated; $phpSortField = $translated;
@ -116,7 +103,45 @@ class SearchController extends ListDiscussionsController
} }
} }
$response = $builder->search(); // Default to latest when no explicit sort is requested. This lets has_child use
// score_mode:none, which skips child scoring entirely and lets ES short-circuit
// early on large corpora. Relevance is still available via sort=relevant if added.
if (empty($sorts)) {
$needsScoring = false;
$phpSortField = 'updated_at';
$phpSortDir = 'desc';
$sorts[] = new Sort('updated_at', 'desc');
}
$query = BoolQuery::create()
// Always restrict to discussion documents; posts are only searched via has_child.
->add(TermQuery::create('join_field', 'discussion'), 'filter');
if (!empty($search)) {
$query->add($this->buildTextQuery($search, $actor, $needsScoring));
}
$this->addFilters($query, $actor, $filters);
$builder = (new Builder($this->elastic))
->index(resolve('blomstra.search.elastic_index'))
->addQuery($query);
foreach ($sorts as $sort) {
$builder->addSort($sort);
}
// track_total_hits: false lets ES stop counting once it has collected
// enough results in sort order, avoiding a full-index count on every query.
$payload = $builder->getPayload();
$payload['track_total_hits'] = false;
$response = $this->elastic->search([
'index' => resolve('blomstra.search.elastic_index'),
'size' => $limit + 1,
'from' => $offset,
'body' => $payload,
]);
Discussion::setStateUser($actor); Discussion::setStateUser($actor);
@ -196,12 +221,16 @@ class SearchController extends ListDiscussionsController
* Build the text-matching portion of the query. * Build the text-matching portion of the query.
* *
* Discussion titles are matched directly (filtered to join_field=discussion). * Discussion titles are matched directly (filtered to join_field=discussion).
* Post bodies are matched via has_child with score_mode=sum so discussions * Post bodies are matched via has_child. When $needsScoring is true (no explicit
* with many matching posts score higher than those with a single strong match. * sort, i.e. relevance ordering), score_mode=sum accumulates child scores onto the
* inner_hits returns the best-scoring post for use as mostRelevantPost. * parent so that discussions with many strongly-matching posts rank higher. When
* false (any explicit field sort), score_mode=none skips scoring entirely ES only
* checks whether a matching child exists, which is significantly cheaper on large
* corpora.
* inner_hits returns the best-matching post for use as mostRelevantPost.
* Hidden posts are only included in matching for users with post.hide permission. * Hidden posts are only included in matching for users with post.hide permission.
*/ */
protected function buildTextQuery(string $search, User $actor): BoolQuery protected function buildTextQuery(string $search, User $actor, bool $needsScoring = false): BoolQuery
{ {
$textQuery = BoolQuery::create(); $textQuery = BoolQuery::create();
@ -214,11 +243,15 @@ class SearchController extends ListDiscussionsController
// Guests and non-moderators may not see hidden posts; exclude them from child matching. // Guests and non-moderators may not see hidden posts; exclude them from child matching.
if ($actor->isGuest() || !$actor->hasPermission('post.hide')) { if ($actor->isGuest() || !$actor->hasPermission('post.hide')) {
$postQuery->add(TermQuery::create('is_hidden', false), 'filter'); $postQuery->add(TermQuery::create('is_hidden', 'false'), 'filter');
} }
// Without minimum_should_match, ES default MSM is 0 when a filter clause is present,
// causing has_child to score every non-hidden post instead of only matching ones.
$postQuery->minimumShouldMatch(1);
$textQuery->add( $textQuery->add(
HasChildQuery::create('post', $postQuery)->withInnerHits(), HasChildQuery::create('post', $postQuery, $needsScoring ? 'sum' : 'none')->withInnerHits(),
'should' 'should'
); );
} }
@ -235,7 +268,6 @@ class SearchController extends ListDiscussionsController
} }
if ($this->matchWords) { if ($this->matchWords) {
$should->add((new MatchQuery('content', $search))->operator('and')->boost(1.8 * $boost), 'should'); $should->add((new MatchQuery('content', $search))->operator('and')->boost(1.8 * $boost), 'should');
$should->add((new MatchQuery('content', $search))->operator('or')->boost(0.8 * $boost), 'should');
} }
return $should; return $should;
@ -301,8 +333,12 @@ class SearchController extends ListDiscussionsController
$search = Arr::get($filters, 'q'); $search = Arr::get($filters, 'q');
if ($search) { if ($search) {
// Strip Flarum gambit operators (tag:foo, author:bar, is:private, etc.)
// before passing to ES. These are structural filters handled separately;
// leaving them in causes operator:and to require the gambit tokens to
// appear literally in post content, producing zero results.
$q = collect(explode(' ', $search)) $q = collect(explode(' ', $search))
->filter(fn (string $part) => $part !== 'is:private') ->filter(fn (string $part) => !preg_match('/^\w+:/', $part))
->filter() ->filter()
->join(' '); ->join(' ');

View File

@ -16,6 +16,7 @@ use Blomstra\Search\Jobs\Job;
use Blomstra\Search\Jobs\UpdateSearchJob; use Blomstra\Search\Jobs\UpdateSearchJob;
use Blomstra\Search\Seeders\Seeder; use Blomstra\Search\Seeders\Seeder;
use Elasticsearch\Client; use Elasticsearch\Client;
use Elasticsearch\Common\Exceptions\ElasticsearchException;
use Flarum\Settings\SettingsRepositoryInterface; use Flarum\Settings\SettingsRepositoryInterface;
use Illuminate\Console\Command; use Illuminate\Console\Command;
use Illuminate\Contracts\Container\Container; use Illuminate\Contracts\Container\Container;
@ -155,7 +156,9 @@ HELP;
$this->line(''); $this->line('');
$this->line('Choose one of:'); $this->line('Choose one of:');
$this->line(' blomstra:search:index build --resume Continue from where it left off'); $this->line(' blomstra:search:index build --resume Continue from where it left off');
$this->line(' blomstra:search:index build --resume --staging Continue and keep in staging when done');
$this->line(' blomstra:search:index build --fresh Drop this build and start completely fresh'); $this->line(' blomstra:search:index build --fresh Drop this build and start completely fresh');
$this->line(' blomstra:search:index build --fresh --staging Start fresh and keep in staging when done');
return; return;
} }
@ -359,17 +362,7 @@ HELP;
$rangeTo = $continueAt; $rangeTo = $continueAt;
if ($seedMissing) { if ($seedMissing) {
$response = (new Builder($client)) $seeded = $this->queryIndexedIds($client, $targetIndex, $seeder->joinRelation(), $rangeFrom, $rangeTo);
->index($targetIndex)
->size(2500)
->addQuery(
(new BoolQuery())
->add((new RangeQuery('rawId'))->gte($rangeFrom)->lte($rangeTo))
->add(TermQuery::create('join_field', $seeder->joinRelation()))
)
->search();
$seeded = Arr::pluck(Arr::get($response, 'hits.hits'), '_source.rawId');
} }
/** @var Collection $collection */ /** @var Collection $collection */
@ -569,6 +562,48 @@ HELP;
]; ];
} }
/**
* Query ES for rawIds already indexed in $targetIndex for the given $joinRelation and ID range.
* Retries on transient ES failures (NoNodesAvailableException / other ElasticsearchException)
* by sleeping past the StaticNoPingConnectionPool dead-node timeout (default 60 s) before
* each retry, giving the pool a chance to resurface the node.
*/
protected function queryIndexedIds(
Client $client,
string $targetIndex,
string $joinRelation,
int $rangeFrom,
int $rangeTo,
int $maxRetries = 10
): array {
$attempt = 0;
while (true) {
try {
$response = (new Builder($client))
->index($targetIndex)
->size(2500)
->addQuery(
(new BoolQuery())
->add((new RangeQuery('rawId'))->gte($rangeFrom)->lte($rangeTo))
->add(TermQuery::create('join_field', $joinRelation))
)
->search();
return Arr::pluck(Arr::get($response, 'hits.hits'), '_source.rawId');
} catch (ElasticsearchException $e) {
$attempt++;
if ($attempt >= $maxRetries) {
throw $e;
}
$this->warn("ES error on range {$rangeFrom}{$rangeTo} (attempt {$attempt}/{$maxRetries}): {$e->getMessage()}. Waiting 65 s before retry…");
sleep(65); // outlast the StaticNoPingConnectionPool dead-node window (default 60 s)
}
}
}
protected function getContinueAt(SettingsRepositoryInterface $settings, string $type): ?int protected function getContinueAt(SettingsRepositoryInterface $settings, string $type): ?int
{ {
$raw = $settings->get("blomstra-search.continued-at.$type"); $raw = $settings->get("blomstra-search.continued-at.$type");

View File

@ -0,0 +1,41 @@
<?php
/*
* This file is part of blomstra/search.
*
* Copyright (c) 2022 Blomstra Ltd.
*
* For the full copyright and license information, please view the LICENSE.md
* file that was distributed with this source code.
*
*/
namespace Blomstra\Search\Elasticsearch;
class BoolQuery extends \Spatie\ElasticsearchQueryBuilder\Queries\BoolQuery
{
protected ?int $minimumShouldMatch = null;
public static function create(): static
{
return new self();
}
public function minimumShouldMatch(int $minimum): static
{
$this->minimumShouldMatch = $minimum;
return $this;
}
public function toArray(): array
{
$array = parent::toArray();
if ($this->minimumShouldMatch !== null) {
$array['bool']['minimum_should_match'] = $this->minimumShouldMatch;
}
return $array;
}
}

View File

@ -14,7 +14,7 @@ namespace Blomstra\Search\Jobs;
use Blomstra\Search\Seeders\Seeder; use Blomstra\Search\Seeders\Seeder;
use Flarum\Queue\AbstractJob; use Flarum\Queue\AbstractJob;
use Illuminate\Support\Collection; use Illuminate\Database\Eloquent\Collection;
abstract class Job extends AbstractJob abstract class Job extends AbstractJob
{ {

View File

@ -14,6 +14,7 @@ namespace Blomstra\Search\Jobs;
use Blomstra\Search\Exceptions\SeedingException; use Blomstra\Search\Exceptions\SeedingException;
use Elasticsearch\Client; use Elasticsearch\Client;
use Illuminate\Database\Eloquent\Collection as EloquentCollection;
use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Model;
use Illuminate\Support\Arr; use Illuminate\Support\Arr;
@ -50,10 +51,11 @@ class UpdateSearchJob extends Job
$items = Arr::get($response, 'items'); $items = Arr::get($response, 'items');
$error = Arr::get(Arr::first($items), 'index.error.reason'); $failed = array_filter($items, fn ($item) => isset($item['index']['error']));
$error = Arr::get(Arr::first($failed), 'index.error.reason', 'unknown error');
throw new SeedingException( throw new SeedingException(
"Failed to seed: $error", "Failed to seed: $error (" . count($failed) . '/' . count($items) . ' items failed)',
$items $items
); );
} }

View File

@ -26,7 +26,7 @@ use Flarum\Settings\SettingsRepositoryInterface;
use Illuminate\Contracts\Container\Container; use Illuminate\Contracts\Container\Container;
use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Contracts\Events\Dispatcher;
use Illuminate\Contracts\Queue\Queue; use Illuminate\Contracts\Queue\Queue;
use Illuminate\Support\Collection; use Illuminate\Database\Eloquent\Collection;
use Laminas\Stratigility\MiddlewarePipe; use Laminas\Stratigility\MiddlewarePipe;
use Psr\Log\LoggerInterface; use Psr\Log\LoggerInterface;
@ -47,7 +47,13 @@ class Provider extends AbstractServiceProvider
$this->container->singleton(Elastic::class, function (Container $container) use ($settings, $config) { $this->container->singleton(Elastic::class, function (Container $container) use ($settings, $config) {
$builder = ClientBuilder::create() $builder = ClientBuilder::create()
->setHosts([$settings->get('blomstra-search.elastic-endpoint')]); ->setHosts([$settings->get('blomstra-search.elastic-endpoint')])
->setConnectionParams([
'client' => [
'connect_timeout' => 2, // fail fast if ES is unreachable
'timeout' => 10, // allow time for complex queries
],
]);
if ($config->inDebugMode()) { if ($config->inDebugMode()) {
$builder->setLogger($container->make(LoggerInterface::class)); $builder->setLogger($container->make(LoggerInterface::class));

View File

@ -24,7 +24,7 @@ use FoF\Byobu\Events as Byobu;
use FoF\DiscussionViews\Events\DiscussionWasViewed; use FoF\DiscussionViews\Events\DiscussionWasViewed;
use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Contracts\Events\Dispatcher;
use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Collection; use Illuminate\Support\Collection;
use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Model;
class DiscussionSeeder extends Seeder class DiscussionSeeder extends Seeder
@ -70,7 +70,7 @@ class DiscussionSeeder extends Seeder
{ {
$events->listen([ $events->listen([
// flarum/core events // flarum/core events
Core\Started::class, Core\Restored::class, Core\Started::class, Core\Restored::class, Core\Renamed::class,
// fof/byobu discussion recipients events. // fof/byobu discussion recipients events.
Byobu\DiscussionMadePublic::class, Byobu\RemovedSelf::class, Byobu\RecipientsChanged::class, Byobu\DiscussionMadePublic::class, Byobu\RemovedSelf::class, Byobu\RecipientsChanged::class,
], function ($event) use ($callable) { ], function ($event) use ($callable) {