Merge pull request #30 from Setono/refactor-search-engine

First refactor of the search execution
Setono · Oct 1, 2024 · 98665cb · 98665cb
2 parents 6b2b51e + 52851c5
commit 98665cb
Show file tree

Hide file tree

Showing 11 changed files with 154 additions and 45 deletions.
diff --git a/README.md b/README.md
@@ -159,6 +159,30 @@ class Product extends BaseProduct implements IndexableInterface, FilterableInter
 }
 ```
 
+## Testing
+
+To run tests in the plugin, here are the steps:
+
+1. Ensure you have Meilisearch running and that you've set the required environment variables in `.env.test.local`.
+
+2. Create the test database
+
+    ```shell
+    php bin/console doctrine:database:create --env=test
+    ```
+
+3. Update the test database schema
+
+    ```shell
+    php bin/console doctrine:schema:update --env=test --force
+    ```
+
+4. Load fixtures
+
+    ```shell
+    php bin/console sylius:fixtures:load -n --env=test
+    ```
+
 [ico-version]: https://poser.pugx.org/setono/sylius-meilisearch-plugin/v/stable
 [ico-license]: https://poser.pugx.org/setono/sylius-meilisearch-plugin/license
 [ico-github-actions]: https://github.com/Setono/sylius-meilisearch-plugin/workflows/build/badge.svg

diff --git a/src/Controller/Action/SearchAction.php b/src/Controller/Action/SearchAction.php
@@ -7,12 +7,12 @@
 use Doctrine\Persistence\ManagerRegistry;
 use Setono\Doctrine\ORMTrait;
 use Setono\SyliusMeilisearchPlugin\Engine\SearchEngineInterface;
+use Setono\SyliusMeilisearchPlugin\Engine\SearchRequest;
 use Setono\SyliusMeilisearchPlugin\Form\Builder\SearchFormBuilderInterface;
 use Setono\SyliusMeilisearchPlugin\Model\IndexableInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Twig\Environment;
-use Webmozart\Assert\Assert;
 
 final class SearchAction
 {
@@ -29,10 +29,7 @@ public function __construct(
 
     public function __invoke(Request $request): Response
     {
-        $query = $request->query->get('q');
-        Assert::nullOrString($query);
-
-        $searchResult = $this->searchEngine->execute($query, $request->query->all());
+        $searchResult = $this->searchEngine->execute(SearchRequest::fromRequest($request));
 
         $searchForm = $this->searchFormBuilder->build($searchResult);
         $searchForm->handleRequest($request);

diff --git a/src/DataMapper/Product/PopularityDataMapper.php b/src/DataMapper/Product/PopularityDataMapper.php
@@ -4,6 +4,7 @@
 
 namespace Setono\SyliusMeilisearchPlugin\DataMapper\Product;
 
+use Doctrine\ORM\NoResultException;
 use Doctrine\Persistence\ManagerRegistry;
 use Setono\Doctrine\ORMTrait;
 use Setono\SyliusMeilisearchPlugin\DataMapper\DataMapperInterface;
@@ -79,17 +80,20 @@ public function supports(
 
     private function getOrderIdLowerBound(): int
     {
-        return (int) $this->getManager($this->orderClass)
-            ->createQueryBuilder()
-            ->select('o.id')
-            ->from($this->orderClass, 'o')
-            ->andwhere('o.createdAt >= :date')
-            ->setMaxResults(1)
-            ->addOrderBy('o.id', 'ASC')
-            ->setParameter('date', new \DateTimeImmutable('-' . $this->popularityLookBackPeriod))
-            ->getQuery()
-            ->enableResultCache(3600) // todo should this be cached and should it be configurable?
-            ->getSingleScalarResult()
-        ;
+        try {
+            return (int) $this->getManager($this->orderClass)
+                ->createQueryBuilder()
+                ->select('o.id')
+                ->from($this->orderClass, 'o')
+                ->andwhere('o.createdAt >= :date')
+                ->setMaxResults(1)
+                ->addOrderBy('o.id', 'ASC')
+                ->setParameter('date', new \DateTimeImmutable('-' . $this->popularityLookBackPeriod))
+                ->getQuery()
+                ->enableResultCache(3600) // todo should this be cached and should it be configurable?
+                ->getSingleScalarResult();
+        } catch (NoResultException) {
+            return 0;
+        }
     }
 }
diff --git a/src/Engine/SearchEngine.php b/src/Engine/SearchEngine.php
@@ -27,31 +27,29 @@ public function __construct(
     ) {
     }
 
-    public function execute(?string $query, array $parameters = []): SearchResult
+    public function execute(SearchRequest $searchRequest): SearchResult
     {
         $indexName = $this->indexNameResolver->resolve($this->index);
         $metadata = $this->metadataFactory->getMetadataFor($this->index->document);
         $facetsNames = $metadata->getFacetableAttributeNames();
         $facets = $metadata->getFacetableAttributes();
 
-        /** @var array<string, mixed> $facetsFilter */
-        $facetsFilter = (array) ($parameters['facets'] ?? []);
         /** @var array<string, mixed> $filters */
-        $filters = $this->filterBuilder->build($facets, $facetsFilter);
+        $filters = $this->filterBuilder->build($facets, $searchRequest->filters);
 
         $mainQuery = $this->mainQueryBuilder->build(
             $indexName,
-            $query ?? '',
+            $searchRequest->query ?? '',
             $facetsNames,
             $filters,
-            max(1, (int) ($parameters['p'] ?? 1)),
-            (string) ($parameters['sort'] ?? ''),
+            $searchRequest->page,
+            $searchRequest->sort ?? '',
         );
 
         /** @var list<SearchQuery> $queries */
         $queries = array_merge(
             [$mainQuery],
-            $this->subQueriesBuilder->build($indexName, $query ?? '', $facets, $facetsFilter),
+            $this->subQueriesBuilder->build($indexName, $searchRequest->query ?? '', $facets, $searchRequest->filters),
         );
 
         /** @var array<SearchResult> $results */

diff --git a/src/Engine/SearchEngineInterface.php b/src/Engine/SearchEngineInterface.php
@@ -8,5 +8,5 @@
 
 interface SearchEngineInterface
 {
-    public function execute(?string $query, array $parameters = []): SearchResult;
+    public function execute(SearchRequest $searchRequest): SearchResult;
 }
diff --git a/src/Engine/SearchRequest.php b/src/Engine/SearchRequest.php
@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Setono\SyliusMeilisearchPlugin\Engine;
+
+use Symfony\Component\HttpFoundation\Request;
+
+final class SearchRequest
+{
+    public function __construct(
+        public readonly ?string $query,
+        /** @var array<string, mixed> $filters */
+        public readonly array $filters = [],
+        public readonly int $page = 1,
+        public readonly ?string $sort = null,
+    ) {
+    }
+
+    public static function fromRequest(Request $request): self
+    {
+        $q = $request->query->get('q');
+        if (!is_string($q)) {
+            $q = null;
+        }
+
+        $page = max(1, (int) $request->query->get('p', 1));
+
+        // todo rename sort to s?
+        $sort = $request->query->get('sort');
+        if (!is_string($sort)) {
+            $sort = null;
+        }
+
+        // todo rename facets to f or filters?
+        /** @var array<string, mixed> $filters */
+        $filters = $request->query->all('facets');
+
+        return new self($q, $filters, $page, $sort);
+    }
+}
diff --git a/tests/Functional/SearchFormBuilderTest.php b/tests/Functional/SearchFormBuilderTest.php
@@ -5,6 +5,7 @@
 namespace Setono\SyliusMeilisearchPlugin\Tests\Functional;
 
 use Setono\SyliusMeilisearchPlugin\Engine\SearchEngine;
+use Setono\SyliusMeilisearchPlugin\Engine\SearchRequest;
 use Setono\SyliusMeilisearchPlugin\Form\Builder\SearchFormBuilderInterface;
 use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
 
@@ -15,7 +16,7 @@ public function testItCreatesFormForSearchResultsWithProperlySortedFacetValues()
     {
         /** @var SearchEngine $searchEngine */
         $searchEngine = self::getContainer()->get(SearchEngine::class);
-        $result = $searchEngine->execute('jeans');
+        $result = $searchEngine->execute(new SearchRequest('jeans'));
 
         /** @var SearchFormBuilderInterface $searchFormBuilder */
         $searchFormBuilder = self::getContainer()->get(SearchFormBuilderInterface::class);

diff --git a/tests/Functional/SearchPaginationTest.php b/tests/Functional/SearchPaginationTest.php
@@ -5,6 +5,7 @@
 namespace Functional;
 
 use Setono\SyliusMeilisearchPlugin\Engine\SearchEngine;
+use Setono\SyliusMeilisearchPlugin\Engine\SearchRequest;
 use Setono\SyliusMeilisearchPlugin\Tests\Functional\FunctionalTestCase;
 
 /** @group functional */
@@ -14,19 +15,19 @@ public function testItPaginatesSearchResults(): void
     {
         /** @var SearchEngine $searchEngine */
         $searchEngine = self::getContainer()->get(SearchEngine::class);
-        $firstPageResult = $searchEngine->execute('jeans');
+        $firstPageResult = $searchEngine->execute(new SearchRequest('jeans'));
 
         self::assertSame(8, $firstPageResult->getHitsCount());
         self::assertSame(3, $firstPageResult->getHitsPerPage());
         self::assertSame(1, $firstPageResult->getPage());
         self::assertSame(3, $firstPageResult->getTotalPages());
 
-        $secondPageResult = $searchEngine->execute('jeans', ['p' => 2]);
+        $secondPageResult = $searchEngine->execute(new SearchRequest('jeans', page: 2));
         self::assertSame(2, $secondPageResult->getPage());
 
         self::assertNotSame($firstPageResult->getHits(), $secondPageResult->getHits());
 
-        $thirdPageResult = $searchEngine->execute('jeans', ['p' => 3]);
+        $thirdPageResult = $searchEngine->execute(new SearchRequest('jeans', page: 3));
         self::assertSame(3, $thirdPageResult->getPage());
         self::assertCount(2, $thirdPageResult->getHits());
     }

diff --git a/tests/Functional/SearchSortingTest.php b/tests/Functional/SearchSortingTest.php
@@ -4,9 +4,9 @@
 
 namespace Setono\SyliusMeilisearchPlugin\Tests\Functional;
 
-use function Amp\Promise\wait;
 use Doctrine\ORM\EntityManagerInterface;
 use Setono\SyliusMeilisearchPlugin\Engine\SearchEngine;
+use Setono\SyliusMeilisearchPlugin\Engine\SearchRequest;
 use Setono\SyliusMeilisearchPlugin\Message\Command\Index;
 use Sylius\Component\Core\Model\ChannelPricingInterface;
 use Sylius\Component\Core\Model\ProductInterface;
@@ -21,7 +21,7 @@ public function testItSortsSearchResultsByLowestPrice(): void
     {
         /** @var SearchEngine $searchEngine */
         $searchEngine = self::getContainer()->get(SearchEngine::class);
-        $result = $searchEngine->execute('jeans', ['sort' => 'price:asc']);
+        $result = $searchEngine->execute(new SearchRequest('jeans', sort: 'price:asc'));
 
         self::assertSame(8, $result->getHitsCount());
 
@@ -43,7 +43,7 @@ public function testItSortsSearchResultsByNewestDate(): void
     {
         /** @var SearchEngine $searchEngine */
         $searchEngine = self::getContainer()->get(SearchEngine::class);
-        $result = $searchEngine->execute('jeans', ['sort' => 'createdAt:desc']);
+        $result = $searchEngine->execute(new SearchRequest('jeans', sort: 'createdAt:desc'));
 
         self::assertSame(8, $result->getHitsCount());
 
@@ -67,7 +67,7 @@ public function testItSortsResultsByBiggestDiscount(): void
 
         /** @var SearchEngine $searchEngine */
         $searchEngine = self::getContainer()->get(SearchEngine::class);
-        $result = $searchEngine->execute('jeans', ['sort' => 'discount:desc']);
+        $result = $searchEngine->execute(new SearchRequest('jeans', sort: 'discount:desc'));
 
         /** @var array $firstHit */
         $firstHit = $result->getHit(0);

diff --git a/tests/Functional/SearchTest.php b/tests/Functional/SearchTest.php
@@ -5,6 +5,7 @@
 namespace Setono\SyliusMeilisearchPlugin\Tests\Functional;
 
 use Setono\SyliusMeilisearchPlugin\Engine\SearchEngine;
+use Setono\SyliusMeilisearchPlugin\Engine\SearchRequest;
 
 /** @group functional */
 final class SearchTest extends FunctionalTestCase
@@ -13,7 +14,7 @@ public function testItProvidesSearchResults(): void
     {
         /** @var SearchEngine $searchEngine */
         $searchEngine = self::getContainer()->get(SearchEngine::class);
-        $result = $searchEngine->execute('jeans');
+        $result = $searchEngine->execute(new SearchRequest('jeans'));
 
         self::assertSame(8, $result->getHitsCount());
     }
@@ -23,30 +24,28 @@ public function testItProvidesSearchResultByMultipleCriteria(): void
         /** @var SearchEngine $searchEngine */
         $searchEngine = self::getContainer()->get(SearchEngine::class);
         $result = $searchEngine->execute(
-            'jeans',
-            [
-                'facets' => [
-                    'brand' => ['Celsius small', 'You are breathtaking'],
-                    'price' => ['min' => '30', 'max' => '45'],
-                ],
-            ],
+            new SearchRequest('jeans', [
+                'brand' => ['Celsius small', 'You are breathtaking'],
+                'price' => ['min' => '30', 'max' => '45'],
+            ]),
         );
 
         /** @var array $hit */
         $hit = $result->getHit(0);
 
         self::assertLessThan(45, (int) $hit['price']);
         self::assertGreaterThan(30, (int) $hit['price']);
-        self::assertTrue(in_array(((array) $hit['brand'])[0], ['Celsius small', 'You are breathtaking'], true));
+        self::assertContains(((array) $hit['brand'])[0], ['Celsius small', 'You are breathtaking']);
     }
 
     public function testItAlwaysDisplaysFullFacetDistribution(): void
     {
         /** @var SearchEngine $searchEngine */
         $searchEngine = self::getContainer()->get(SearchEngine::class);
         $result = $searchEngine->execute(
-            'jeans',
-            ['facets' => ['brand' => ['Celsius small']]],
+            new SearchRequest('jeans', [
+                'brand' => ['Celsius small'],
+            ]),
         );
 
         $this->assertSame(1, $result->getHitsCount());

diff --git a/tests/Unit/Engine/SearchRequestTest.php b/tests/Unit/Engine/SearchRequestTest.php
@@ -0,0 +1,44 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Setono\SyliusMeilisearchPlugin\Tests\Unit\Engine;
+
+use PHPUnit\Framework\TestCase;
+use Setono\SyliusMeilisearchPlugin\Engine\SearchRequest;
+use Symfony\Component\HttpFoundation\Request;
+
+final class SearchRequestTest extends TestCase
+{
+    /**
+     * @test
+     */
+    public function it_creates_search_request(): void
+    {
+        $searchRequest = new SearchRequest(
+            'jeans',
+            ['brand' => ['Celsius small', 'You are breathtaking'], 'price' => ['min' => '30', 'max' => '45']],
+            2,
+            'price:asc',
+        );
+
+        self::assertSame('jeans', $searchRequest->query);
+        self::assertSame(['brand' => ['Celsius small', 'You are breathtaking'], 'price' => ['min' => '30', 'max' => '45']], $searchRequest->filters);
+        self::assertSame(2, $searchRequest->page);
+        self::assertSame('price:asc', $searchRequest->sort);
+    }
+
+    /**
+     * @test
+     */
+    public function it_creates_from_request(): void
+    {
+        $request = Request::create('/search', 'GET', ['q' => 'jeans', 'facets' => ['brand' => ['Celsius small', 'You are breathtaking'], 'price' => ['min' => '30', 'max' => '45']], 'p' => 2, 'sort' => 'price:asc']);
+        $searchRequest = SearchRequest::fromRequest($request);
+
+        self::assertSame('jeans', $searchRequest->query);
+        self::assertSame(['brand' => ['Celsius small', 'You are breathtaking'], 'price' => ['min' => '30', 'max' => '45']], $searchRequest->filters);
+        self::assertSame(2, $searchRequest->page);
+        self::assertSame('price:asc', $searchRequest->sort);
+    }
+}