Skip to content

Commit 6b4d659

Browse files
committed
Batch-hydrate search hits, drop stale ones and self-heal the index
SearchAction re-hydrated every Meilisearch hit one find() at a time (up to hits_per_page=60 extra queries per page), silently dropped hits whose entity was gone (so the count and rendered cards could disagree), and rendered products disabled after their last indexing. - Group hit ids by entity class and load each group with a single findBy(['id' => ]), then reorder to Meilisearch's ranking. - Drop hits whose entity is missing from the database or is disabled (ToggleableInterface), and dispatch RemoveEntity for them so the index self-heals (failure-safe: a dispatch error is logged, never breaks rendering). - The reorder/drop logic is extracted into the pure static reorderAndFilter() for straightforward unit testing. Closes #120
1 parent 786cb4f commit 6b4d659

3 files changed

Lines changed: 198 additions & 12 deletions

File tree

src/Controller/Action/SearchAction.php

Lines changed: 96 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
use Doctrine\Persistence\ManagerRegistry;
88
use Psr\EventDispatcher\EventDispatcherInterface;
9+
use Psr\Log\LoggerInterface;
10+
use Psr\Log\NullLogger;
911
use Setono\Doctrine\ORMTrait;
1012
use Setono\SyliusMeilisearchPlugin\Engine\SearchEngineInterface;
1113
use Setono\SyliusMeilisearchPlugin\Engine\SearchRequest;
@@ -14,9 +16,12 @@
1416
use Setono\SyliusMeilisearchPlugin\Event\Search\SearchResponseParametersCreated;
1517
use Setono\SyliusMeilisearchPlugin\Event\Search\SearchResultReceived;
1618
use Setono\SyliusMeilisearchPlugin\Form\Builder\SearchFormBuilderInterface;
19+
use Setono\SyliusMeilisearchPlugin\Message\Command\RemoveEntity;
1720
use Setono\SyliusMeilisearchPlugin\Model\IndexableInterface;
21+
use Sylius\Component\Resource\Model\ToggleableInterface;
1822
use Symfony\Component\HttpFoundation\Request;
1923
use Symfony\Component\HttpFoundation\Response;
24+
use Symfony\Component\Messenger\MessageBusInterface;
2025
use Twig\Environment;
2126

2227
final class SearchAction
@@ -29,6 +34,8 @@ public function __construct(
2934
private readonly SearchFormBuilderInterface $searchFormBuilder,
3035
private readonly SearchEngineInterface $searchEngine,
3136
private readonly EventDispatcherInterface $eventDispatcher,
37+
private readonly MessageBusInterface $commandBus,
38+
private readonly LoggerInterface $logger = new NullLogger(),
3239
) {
3340
$this->managerRegistry = $managerRegistry;
3441
}
@@ -48,18 +55,7 @@ public function __invoke(Request $request): Response
4855
// todo handle this scenario
4956
}
5057

51-
$items = [];
52-
/** @var array{entityClass: class-string<IndexableInterface>, entityId: mixed} $hit */
53-
foreach ($searchResult->hits as $hit) {
54-
$item = $this->getManager($hit['entityClass'])->find($hit['entityClass'], $hit['entityId']);
55-
56-
// todo if the item doesn't exist in the database, we can in fact end up with an empty $items list
57-
if (null === $item) {
58-
continue;
59-
}
60-
61-
$items[] = $item;
62-
}
58+
$items = $this->hydrateHits($searchResult->hits);
6359

6460
$searchResponseParametersCreatedEvent = new SearchResponseParametersCreated('@SetonoSyliusMeilisearchPlugin/search/index.html.twig', [
6561
'searchResult' => $searchResult,
@@ -80,4 +76,92 @@ public function __invoke(Request $request): Response
8076

8177
return $response;
8278
}
79+
80+
/**
81+
* Hydrates the Meilisearch hits into entities: one findBy() per entity class (instead of one
82+
* find() per hit), reordered to match Meilisearch's ranking. Hits whose entity is missing from
83+
* the database or disabled are dropped and a RemoveEntity is dispatched so the index self-heals.
84+
*
85+
* @param array<int, array> $hits
86+
*
87+
* @return list<IndexableInterface>
88+
*/
89+
private function hydrateHits(array $hits): array
90+
{
91+
// Group hit ids by entity class, preserving Meilisearch's order
92+
$idsByClass = [];
93+
foreach ($hits as $hit) {
94+
$class = $hit['entityClass'] ?? null;
95+
if (!is_string($class) || !is_a($class, IndexableInterface::class, true)) {
96+
continue;
97+
}
98+
99+
$idsByClass[$class][] = $hit['entityId'] ?? null;
100+
}
101+
102+
// Load each class in a single query, keyed by (string) id
103+
$loaded = [];
104+
foreach ($idsByClass as $class => $ids) {
105+
/** @var list<IndexableInterface> $entities */
106+
$entities = $this->getRepository($class)->findBy(['id' => $ids]);
107+
foreach ($entities as $entity) {
108+
$loaded[$class][(string) $entity->getId()] = $entity;
109+
}
110+
}
111+
112+
[$items, $stale] = self::reorderAndFilter($hits, $loaded);
113+
114+
foreach ($stale as $hit) {
115+
try {
116+
$this->commandBus->dispatch(new RemoveEntity($hit['entityClass'], $hit['entityId']));
117+
} catch (\Throwable $e) {
118+
// Self-healing must never break rendering the search page
119+
$this->logger->error('Failed to dispatch RemoveEntity for a stale search hit: {message}', [
120+
'message' => $e->getMessage(),
121+
'entity' => $hit['entityClass'],
122+
'exception' => $e,
123+
]);
124+
}
125+
}
126+
127+
return $items;
128+
}
129+
130+
/**
131+
* Reorders the loaded entities to match Meilisearch's hit order and separates out the stale
132+
* hits (missing from the database, or disabled) so the caller can self-heal the index.
133+
*
134+
* @param array<int, array<array-key, mixed>> $hits
135+
* @param array<string, array<array-key, IndexableInterface>> $loaded
136+
*
137+
* @return array{0: list<IndexableInterface>, 1: list<array{entityClass: class-string<IndexableInterface>, entityId: int|string}>}
138+
*/
139+
public static function reorderAndFilter(array $hits, array $loaded): array
140+
{
141+
$items = [];
142+
$stale = [];
143+
144+
foreach ($hits as $hit) {
145+
$class = $hit['entityClass'] ?? null;
146+
$id = $hit['entityId'] ?? null;
147+
148+
if (!is_string($class) || !is_a($class, IndexableInterface::class, true) || (!is_string($id) && !is_int($id))) {
149+
continue;
150+
}
151+
152+
$entity = $loaded[$class][(string) $id] ?? null;
153+
154+
// A hit whose entity was deleted, or that is now disabled, must not be rendered (the
155+
// index is stale); dispatch a removal so it heals on the next request instead.
156+
if (null === $entity || ($entity instanceof ToggleableInterface && !$entity->isEnabled())) {
157+
$stale[] = ['entityClass' => $class, 'entityId' => $id];
158+
159+
continue;
160+
}
161+
162+
$items[] = $entity;
163+
}
164+
165+
return [$items, $stale];
166+
}
83167
}

src/Resources/config/services/conditional/search.xml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@
1313
<argument type="service" id="Setono\SyliusMeilisearchPlugin\Form\Builder\SearchFormBuilderInterface"/>
1414
<argument type="service" id="Setono\SyliusMeilisearchPlugin\Engine\SearchEngineInterface"/>
1515
<argument type="service" id="event_dispatcher"/>
16+
<argument type="service" id="setono_sylius_meilisearch.command_bus"/>
17+
<argument type="service" id="logger"/>
18+
19+
<tag name="monolog.logger" channel="setono_sylius_meilisearch"/>
1620
</service>
1721

1822
<service id="Setono\SyliusMeilisearchPlugin\Engine\SearchEngine">
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Setono\SyliusMeilisearchPlugin\Tests\Unit\Controller\Action;
6+
7+
use PHPUnit\Framework\TestCase;
8+
use Prophecy\PhpUnit\ProphecyTrait;
9+
use Setono\SyliusMeilisearchPlugin\Controller\Action\SearchAction;
10+
use Setono\SyliusMeilisearchPlugin\Model\IndexableInterface;
11+
use Sylius\Component\Resource\Model\ToggleableInterface;
12+
13+
/**
14+
* @covers \Setono\SyliusMeilisearchPlugin\Controller\Action\SearchAction
15+
*/
16+
final class SearchActionTest extends TestCase
17+
{
18+
use ProphecyTrait;
19+
20+
private function indexable(): IndexableInterface
21+
{
22+
return $this->prophesize(IndexableInterface::class)->reveal();
23+
}
24+
25+
private function enabledToggleable(bool $enabled): IndexableInterface
26+
{
27+
$entity = $this->prophesize(IndexableInterface::class);
28+
$entity->willImplement(ToggleableInterface::class);
29+
$entity->isEnabled()->willReturn($enabled);
30+
31+
return $entity->reveal();
32+
}
33+
34+
/**
35+
* @param class-string<IndexableInterface> $class
36+
*
37+
* @return array{entityClass: class-string<IndexableInterface>, entityId: int|string}
38+
*/
39+
private function hit(string $class, int|string $id): array
40+
{
41+
return ['entityClass' => $class, 'entityId' => $id];
42+
}
43+
44+
/**
45+
* @test
46+
*/
47+
public function it_reorders_loaded_entities_to_match_the_hit_order(): void
48+
{
49+
$class = IndexableInterface::class;
50+
$a = $this->indexable();
51+
$b = $this->indexable();
52+
$c = $this->indexable();
53+
54+
// Loaded in a different order than the hits (as findBy would return them)
55+
$loaded = [$class => ['2' => $b, '3' => $c, '1' => $a]];
56+
$hits = [$this->hit($class, 1), $this->hit($class, 2), $this->hit($class, 3)];
57+
58+
[$items, $stale] = SearchAction::reorderAndFilter($hits, $loaded);
59+
60+
self::assertSame([$a, $b, $c], $items);
61+
self::assertSame([], $stale);
62+
}
63+
64+
/**
65+
* @test
66+
*/
67+
public function it_drops_and_reports_a_hit_whose_entity_is_missing(): void
68+
{
69+
$class = IndexableInterface::class;
70+
$a = $this->indexable();
71+
72+
$loaded = [$class => ['1' => $a]];
73+
$hits = [$this->hit($class, 1), $this->hit($class, 2)];
74+
75+
[$items, $stale] = SearchAction::reorderAndFilter($hits, $loaded);
76+
77+
self::assertSame([$a], $items);
78+
self::assertSame([$this->hit($class, 2)], $stale);
79+
}
80+
81+
/**
82+
* @test
83+
*/
84+
public function it_drops_and_reports_a_disabled_entity(): void
85+
{
86+
$class = IndexableInterface::class;
87+
$enabled = $this->enabledToggleable(true);
88+
$disabled = $this->enabledToggleable(false);
89+
90+
$loaded = [$class => ['1' => $enabled, '2' => $disabled]];
91+
$hits = [$this->hit($class, 1), $this->hit($class, 2)];
92+
93+
[$items, $stale] = SearchAction::reorderAndFilter($hits, $loaded);
94+
95+
self::assertSame([$enabled], $items);
96+
self::assertSame([$this->hit($class, 2)], $stale);
97+
}
98+
}

0 commit comments

Comments
 (0)