From ea602953c74da31f24c32925269b24c9c9d86b3d Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Tue, 20 Aug 2024 14:24:08 +0200 Subject: [PATCH 1/6] Move unit tests to unit directory --- tests/{ => Unit}/DataMapper/Product/OptionsDataMapperTest.php | 2 +- .../DataMapper/Product/Provider/ProductPricesProviderTest.php | 0 .../SetonoSyliusMeilisearchExtensionTest.php | 2 +- tests/{ => Unit}/Document/Metadata/MetadataTest.php | 2 +- tests/{ => Unit}/Resolver/IndexName/IndexNameResolverTest.php | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) rename tests/{ => Unit}/DataMapper/Product/OptionsDataMapperTest.php (97%) rename tests/{ => Unit}/DataMapper/Product/Provider/ProductPricesProviderTest.php (100%) rename tests/{ => Unit}/DependencyInjection/SetonoSyliusMeilisearchExtensionTest.php (97%) rename tests/{ => Unit}/Document/Metadata/MetadataTest.php (90%) rename tests/{ => Unit}/Resolver/IndexName/IndexNameResolverTest.php (94%) diff --git a/tests/DataMapper/Product/OptionsDataMapperTest.php b/tests/Unit/DataMapper/Product/OptionsDataMapperTest.php similarity index 97% rename from tests/DataMapper/Product/OptionsDataMapperTest.php rename to tests/Unit/DataMapper/Product/OptionsDataMapperTest.php index 2571907..f87305b 100644 --- a/tests/DataMapper/Product/OptionsDataMapperTest.php +++ b/tests/Unit/DataMapper/Product/OptionsDataMapperTest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Setono\SyliusMeilisearchPlugin\Tests\DataMapper\Product; +namespace Setono\SyliusMeilisearchPlugin\Tests\Unit\DataMapper\Product; use Doctrine\Common\Collections\ArrayCollection; use PHPUnit\Framework\TestCase; diff --git a/tests/DataMapper/Product/Provider/ProductPricesProviderTest.php b/tests/Unit/DataMapper/Product/Provider/ProductPricesProviderTest.php similarity index 100% rename from tests/DataMapper/Product/Provider/ProductPricesProviderTest.php rename to tests/Unit/DataMapper/Product/Provider/ProductPricesProviderTest.php diff --git a/tests/DependencyInjection/SetonoSyliusMeilisearchExtensionTest.php b/tests/Unit/DependencyInjection/SetonoSyliusMeilisearchExtensionTest.php similarity index 97% rename from tests/DependencyInjection/SetonoSyliusMeilisearchExtensionTest.php rename to tests/Unit/DependencyInjection/SetonoSyliusMeilisearchExtensionTest.php index ccab261..f652961 100644 --- a/tests/DependencyInjection/SetonoSyliusMeilisearchExtensionTest.php +++ b/tests/Unit/DependencyInjection/SetonoSyliusMeilisearchExtensionTest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Setono\SyliusMeilisearchPlugin\Tests\DependencyInjection; +namespace Setono\SyliusMeilisearchPlugin\Tests\Unit\DependencyInjection; use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractExtensionTestCase; use Setono\SyliusMeilisearchPlugin\Controller\Action\SearchAction; diff --git a/tests/Document/Metadata/MetadataTest.php b/tests/Unit/Document/Metadata/MetadataTest.php similarity index 90% rename from tests/Document/Metadata/MetadataTest.php rename to tests/Unit/Document/Metadata/MetadataTest.php index e2c6b17..b304ee2 100644 --- a/tests/Document/Metadata/MetadataTest.php +++ b/tests/Unit/Document/Metadata/MetadataTest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Setono\SyliusMeilisearchPlugin\Tests\Document\Metadata; +namespace Setono\SyliusMeilisearchPlugin\Tests\Unit\Document\Metadata; use PHPUnit\Framework\TestCase; use Setono\SyliusMeilisearchPlugin\Document\Attribute\Facet; diff --git a/tests/Resolver/IndexName/IndexNameResolverTest.php b/tests/Unit/Resolver/IndexName/IndexNameResolverTest.php similarity index 94% rename from tests/Resolver/IndexName/IndexNameResolverTest.php rename to tests/Unit/Resolver/IndexName/IndexNameResolverTest.php index 3ed0d46..8ff06d7 100644 --- a/tests/Resolver/IndexName/IndexNameResolverTest.php +++ b/tests/Unit/Resolver/IndexName/IndexNameResolverTest.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Setono\SyliusMeilisearchPlugin\Tests\Resolver\IndexName; +namespace Setono\SyliusMeilisearchPlugin\Tests\Unit\Resolver\IndexName; use PHPUnit\Framework\TestCase; use Prophecy\PhpUnit\ProphecyTrait; From 7fd56ea5116a99275e1b3b810daa185d83dbf497 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Tue, 20 Aug 2024 15:44:43 +0200 Subject: [PATCH 2/6] First functional test for search engine --- composer.json | 3 ++- tests/Functional/SearchTest.php | 31 +++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 tests/Functional/SearchTest.php diff --git a/composer.json b/composer.json index 7bff322..851103e 100644 --- a/composer.json +++ b/composer.json @@ -67,6 +67,7 @@ "psalm/plugin-phpunit": "^0.18.4", "setono/code-quality-pack": "^2.8.2", "sylius/sylius": "~1.12.18", + "symfony/browser-kit": "^7.1", "symfony/debug-bundle": "^5.4 || ^6.4 || ^7.0", "symfony/dotenv": "^5.4 || ^6.4 || ^7.0", "symfony/http-client": "^5.4 || ^6.4 || ^7.0", @@ -107,7 +108,7 @@ "analyse": "psalm", "check-style": "ecs check", "fix-style": "ecs check --fix", - "phpunit": "phpunit", + "phpunit": "phpunit --exclude-group=functional", "rector": "rector" } } diff --git a/tests/Functional/SearchTest.php b/tests/Functional/SearchTest.php new file mode 100644 index 0000000..eff2cbf --- /dev/null +++ b/tests/Functional/SearchTest.php @@ -0,0 +1,31 @@ + 'test', 'debug' => true]); + } + + public function testItProvidesSearchResults(): void + { + $searchEngine = self::getContainer()->get(SearchEngine::class); + $result = $searchEngine->execute('jeans'); + + self::assertSame(8, $result->getHitsCount()); + } +} From 45ac62f207ec8f0ed35f7dd9974c514dcb9a29c2 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Tue, 20 Aug 2024 15:48:55 +0200 Subject: [PATCH 3/6] Extract search engine and add proper sorting handling --- src/Controller/Action/SearchAction.php | 26 ++--------- src/Engine/SearchEngine.php | 46 +++++++++++++++++++ src/Form/Builder/SearchFormBuilder.php | 6 ++- src/Meilisearch/Builder/FilterBuilder.php | 4 +- .../Builder/FilterBuilderInterface.php | 5 +- .../config/services/conditional/search.xml | 10 ++-- 6 files changed, 65 insertions(+), 32 deletions(-) create mode 100644 src/Engine/SearchEngine.php diff --git a/src/Controller/Action/SearchAction.php b/src/Controller/Action/SearchAction.php index 26d506b..8eb2faf 100644 --- a/src/Controller/Action/SearchAction.php +++ b/src/Controller/Action/SearchAction.php @@ -10,6 +10,7 @@ use Setono\SyliusMeilisearchPlugin\Config\Index; use Setono\SyliusMeilisearchPlugin\Document\Metadata\Facet; use Setono\SyliusMeilisearchPlugin\Document\Metadata\MetadataFactoryInterface; +use Setono\SyliusMeilisearchPlugin\Engine\SearchEngine; use Setono\SyliusMeilisearchPlugin\Form\Builder\SearchFormBuilderInterface; use Setono\SyliusMeilisearchPlugin\Meilisearch\Builder\FilterBuilderInterface; use Setono\SyliusMeilisearchPlugin\Model\IndexableInterface; @@ -26,40 +27,23 @@ final class SearchAction public function __construct( ManagerRegistry $managerRegistry, private readonly Environment $twig, - private readonly IndexNameResolverInterface $indexNameResolver, - private readonly Client $client, - private readonly MetadataFactoryInterface $metadataFactory, private readonly SearchFormBuilderInterface $searchFormBuilder, - private readonly FilterBuilderInterface $filterBuilder, - private readonly Index $index, - private readonly int $hitsPerPage, + private readonly SearchEngine $searchEngine, ) { $this->managerRegistry = $managerRegistry; } public function __invoke(Request $request): Response { - $q = $request->query->get('q'); - Assert::nullOrString($q); + $query = $request->query->get('q'); + Assert::nullOrString($query); - $page = (int) $request->query->get('p', 1); - $page = max(1, $page); - - $metadata = $this->metadataFactory->getMetadataFor($this->index->document); - - $searchResult = $this->client->index($this->indexNameResolver->resolve($this->index))->search($q, [ - 'facets' => array_map(static fn (Facet $facet) => $facet->name, $metadata->getFacets()), - 'filter' => $this->filterBuilder->build($request), - 'sort' => ['price:asc'], - 'hitsPerPage' => $this->hitsPerPage, - 'page' => $page, - ]); + $searchResult = $this->searchEngine->execute($query, $request->query->all()); $searchForm = $this->searchFormBuilder->build($searchResult); $searchForm->handleRequest($request); $items = []; - /** @var array{entityClass: class-string, entityId: mixed} $hit */ foreach ($searchResult->getHits() as $hit) { $items[] = $this->getManager($hit['entityClass'])->find($hit['entityClass'], $hit['entityId']); diff --git a/src/Engine/SearchEngine.php b/src/Engine/SearchEngine.php new file mode 100644 index 0000000..28c17cb --- /dev/null +++ b/src/Engine/SearchEngine.php @@ -0,0 +1,46 @@ +metadataFactory->getMetadataFor($this->index->document); + + $searchParams = [ + 'facets' => array_map(static fn (Facet $facet) => $facet->name, $metadata->getFacets()), + 'filter' => $this->filterBuilder->build($parameters), + 'hitsPerPage' => $this->hitsPerPage, + 'page' => $page, + ]; + if (null !== $sort && '' !== $sort) { + $searchParams['sort'] = [$sort]; + } + + return $this->client->index($this->indexNameResolver->resolve($this->index))->search($query, $searchParams); + } +} diff --git a/src/Form/Builder/SearchFormBuilder.php b/src/Form/Builder/SearchFormBuilder.php index 02cff31..53354e1 100644 --- a/src/Form/Builder/SearchFormBuilder.php +++ b/src/Form/Builder/SearchFormBuilder.php @@ -79,8 +79,10 @@ public function build(SearchResult $searchResult): FormInterface $searchFormBuilder->add('sort', ChoiceType::class, [ 'choices' => [ - 'Price: Low to High' => 'price:asc', - 'Price: High to Low' => 'price:desc', + 'Cheapest first' => 'price:asc', + 'Biggest discount' => 'discount:desc', + 'Newest first' => 'createdAt:desc', + 'Relevance' => '', ], 'required' => false, 'placeholder' => 'Sort by', diff --git a/src/Meilisearch/Builder/FilterBuilder.php b/src/Meilisearch/Builder/FilterBuilder.php index a029e4c..3e20162 100644 --- a/src/Meilisearch/Builder/FilterBuilder.php +++ b/src/Meilisearch/Builder/FilterBuilder.php @@ -9,11 +9,11 @@ // todo this should be refactored final class FilterBuilder implements FilterBuilderInterface { - public function build(Request $request): array + public function build(array $parameters): array { $filters = []; - $query = $request->query->has('facets') ? $request->query->all('facets') : $request->query->all(); + $query = $parameters['facets'] ?? $parameters; if (isset($query['onSale'])) { $filters[] = 'onSale = true'; diff --git a/src/Meilisearch/Builder/FilterBuilderInterface.php b/src/Meilisearch/Builder/FilterBuilderInterface.php index a5109ab..855bb1a 100644 --- a/src/Meilisearch/Builder/FilterBuilderInterface.php +++ b/src/Meilisearch/Builder/FilterBuilderInterface.php @@ -8,8 +8,5 @@ interface FilterBuilderInterface { - /** - * Takes a Symfony request and returns a filter ready for the Meilisearch client - */ - public function build(Request $request): array; + public function build(array $parameters): array; } diff --git a/src/Resources/config/services/conditional/search.xml b/src/Resources/config/services/conditional/search.xml index 26f49f3..fd5dc79 100644 --- a/src/Resources/config/services/conditional/search.xml +++ b/src/Resources/config/services/conditional/search.xml @@ -10,12 +10,16 @@ - - - + + + + + + + %setono_sylius_meilisearch.search.hits_per_page% From 221f7f66107310a7d3b71bfac1b3b9ef9edb9c41 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Wed, 21 Aug 2024 08:52:46 +0200 Subject: [PATCH 4/6] Some more search + sorting functional tests --- tests/Functional/SearchTest.php | 38 +++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/Functional/SearchTest.php b/tests/Functional/SearchTest.php index eff2cbf..29e3a0b 100644 --- a/tests/Functional/SearchTest.php +++ b/tests/Functional/SearchTest.php @@ -28,4 +28,42 @@ public function testItProvidesSearchResults(): void self::assertSame(8, $result->getHitsCount()); } + + public function testItSortsSearchResultsByLowestPrice(): void + { + $searchEngine = self::getContainer()->get(SearchEngine::class); + $result = $searchEngine->execute('jeans', ['sort' => 'price:asc']); + + self::assertSame(8, $result->getHitsCount()); + + $previousKey = null; + foreach ($result->getHits() as $key => $hit) { + if ($previousKey === null) { + $previousKey = $key; + continue; + } + + self::assertGreaterThanOrEqual($result->getHit($previousKey)['price'], $hit['price']); + $previousKey = $key; + } + } + + public function testItSortsSearchResultsByNewestDate(): void + { + $searchEngine = self::getContainer()->get(SearchEngine::class); + $result = $searchEngine->execute('jeans', ['sort' => 'createdAt:desc']); + + self::assertSame(8, $result->getHitsCount()); + + $previousKey = null; + foreach ($result->getHits() as $key => $hit) { + if ($previousKey === null) { + $previousKey = $key; + continue; + } + + self::assertLessThanOrEqual($result->getHit($previousKey)['createdAt'], $hit['createdAt']); + $previousKey = $key; + } + } } From a9f3d08cbe0d37ae9e5017cc9121ce11d7a471e9 Mon Sep 17 00:00:00 2001 From: Mateusz Zalewski Date: Wed, 21 Aug 2024 09:03:11 +0200 Subject: [PATCH 5/6] CS fixes --- composer.json | 2 +- src/Controller/Action/SearchAction.php | 6 ------ src/Engine/SearchEngine.php | 6 +++--- src/Meilisearch/Builder/FilterBuilder.php | 4 +--- src/Meilisearch/Builder/FilterBuilderInterface.php | 2 -- tests/Functional/SearchTest.php | 11 +++++++++-- 6 files changed, 14 insertions(+), 17 deletions(-) diff --git a/composer.json b/composer.json index 851103e..3138471 100644 --- a/composer.json +++ b/composer.json @@ -67,7 +67,7 @@ "psalm/plugin-phpunit": "^0.18.4", "setono/code-quality-pack": "^2.8.2", "sylius/sylius": "~1.12.18", - "symfony/browser-kit": "^7.1", + "symfony/browser-kit": "^5.4 || ^6.4 || ^7.0", "symfony/debug-bundle": "^5.4 || ^6.4 || ^7.0", "symfony/dotenv": "^5.4 || ^6.4 || ^7.0", "symfony/http-client": "^5.4 || ^6.4 || ^7.0", diff --git a/src/Controller/Action/SearchAction.php b/src/Controller/Action/SearchAction.php index 8eb2faf..ec7f460 100644 --- a/src/Controller/Action/SearchAction.php +++ b/src/Controller/Action/SearchAction.php @@ -5,16 +5,10 @@ namespace Setono\SyliusMeilisearchPlugin\Controller\Action; use Doctrine\Persistence\ManagerRegistry; -use Meilisearch\Client; use Setono\Doctrine\ORMTrait; -use Setono\SyliusMeilisearchPlugin\Config\Index; -use Setono\SyliusMeilisearchPlugin\Document\Metadata\Facet; -use Setono\SyliusMeilisearchPlugin\Document\Metadata\MetadataFactoryInterface; use Setono\SyliusMeilisearchPlugin\Engine\SearchEngine; use Setono\SyliusMeilisearchPlugin\Form\Builder\SearchFormBuilderInterface; -use Setono\SyliusMeilisearchPlugin\Meilisearch\Builder\FilterBuilderInterface; use Setono\SyliusMeilisearchPlugin\Model\IndexableInterface; -use Setono\SyliusMeilisearchPlugin\Resolver\IndexName\IndexNameResolverInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Twig\Environment; diff --git a/src/Engine/SearchEngine.php b/src/Engine/SearchEngine.php index 28c17cb..0769b61 100644 --- a/src/Engine/SearchEngine.php +++ b/src/Engine/SearchEngine.php @@ -24,10 +24,10 @@ public function __construct( ) { } - public function execute(string $query, array $parameters = []): SearchResult + public function execute(?string $query, array $parameters = []): SearchResult { $page = max(1, (int) ($parameters['p'] ?? 1)); - $sort = $parameters['sort'] ?? ''; + $sort = (string) ($parameters['sort'] ?? ''); $metadata = $this->metadataFactory->getMetadataFor($this->index->document); @@ -37,7 +37,7 @@ public function execute(string $query, array $parameters = []): SearchResult 'hitsPerPage' => $this->hitsPerPage, 'page' => $page, ]; - if (null !== $sort && '' !== $sort) { + if ('' !== $sort) { $searchParams['sort'] = [$sort]; } diff --git a/src/Meilisearch/Builder/FilterBuilder.php b/src/Meilisearch/Builder/FilterBuilder.php index 3e20162..cb35039 100644 --- a/src/Meilisearch/Builder/FilterBuilder.php +++ b/src/Meilisearch/Builder/FilterBuilder.php @@ -4,8 +4,6 @@ namespace Setono\SyliusMeilisearchPlugin\Meilisearch\Builder; -use Symfony\Component\HttpFoundation\Request; - // todo this should be refactored final class FilterBuilder implements FilterBuilderInterface { @@ -13,7 +11,7 @@ public function build(array $parameters): array { $filters = []; - $query = $parameters['facets'] ?? $parameters; + $query = (array) ($parameters['facets'] ?? $parameters); if (isset($query['onSale'])) { $filters[] = 'onSale = true'; diff --git a/src/Meilisearch/Builder/FilterBuilderInterface.php b/src/Meilisearch/Builder/FilterBuilderInterface.php index 855bb1a..989ac77 100644 --- a/src/Meilisearch/Builder/FilterBuilderInterface.php +++ b/src/Meilisearch/Builder/FilterBuilderInterface.php @@ -4,8 +4,6 @@ namespace Setono\SyliusMeilisearchPlugin\Meilisearch\Builder; -use Symfony\Component\HttpFoundation\Request; - interface FilterBuilderInterface { public function build(array $parameters): array; diff --git a/tests/Functional/SearchTest.php b/tests/Functional/SearchTest.php index 29e3a0b..6474bd8 100644 --- a/tests/Functional/SearchTest.php +++ b/tests/Functional/SearchTest.php @@ -23,6 +23,7 @@ protected function setUp(): void public function testItProvidesSearchResults(): void { + /** @var SearchEngine $searchEngine */ $searchEngine = self::getContainer()->get(SearchEngine::class); $result = $searchEngine->execute('jeans'); @@ -31,6 +32,7 @@ public function testItProvidesSearchResults(): void public function testItSortsSearchResultsByLowestPrice(): void { + /** @var SearchEngine $searchEngine */ $searchEngine = self::getContainer()->get(SearchEngine::class); $result = $searchEngine->execute('jeans', ['sort' => 'price:asc']); @@ -40,16 +42,19 @@ public function testItSortsSearchResultsByLowestPrice(): void foreach ($result->getHits() as $key => $hit) { if ($previousKey === null) { $previousKey = $key; + continue; } - self::assertGreaterThanOrEqual($result->getHit($previousKey)['price'], $hit['price']); + $previousHit = (array) $result->getHit($previousKey); + self::assertGreaterThanOrEqual($previousHit['price'], $hit['price']); $previousKey = $key; } } public function testItSortsSearchResultsByNewestDate(): void { + /** @var SearchEngine $searchEngine */ $searchEngine = self::getContainer()->get(SearchEngine::class); $result = $searchEngine->execute('jeans', ['sort' => 'createdAt:desc']); @@ -59,10 +64,12 @@ public function testItSortsSearchResultsByNewestDate(): void foreach ($result->getHits() as $key => $hit) { if ($previousKey === null) { $previousKey = $key; + continue; } - self::assertLessThanOrEqual($result->getHit($previousKey)['createdAt'], $hit['createdAt']); + $previousHit = (array) $result->getHit($previousKey); + self::assertLessThanOrEqual($previousHit['createdAt'], $hit['createdAt']); $previousKey = $key; } } From 74580c8d059b0f770b990d221a87591c25f01285 Mon Sep 17 00:00:00 2001 From: Jacob Tobiasz Date: Wed, 21 Aug 2024 16:12:07 +0200 Subject: [PATCH 6/6] Introduce the SearchEngineInterface --- src/Controller/Action/SearchAction.php | 4 ++-- src/Engine/SearchEngine.php | 2 +- src/Engine/SearchEngineInterface.php | 12 ++++++++++++ src/Resources/config/services/conditional/search.xml | 6 ++++-- 4 files changed, 19 insertions(+), 5 deletions(-) create mode 100644 src/Engine/SearchEngineInterface.php diff --git a/src/Controller/Action/SearchAction.php b/src/Controller/Action/SearchAction.php index ec7f460..d1437ee 100644 --- a/src/Controller/Action/SearchAction.php +++ b/src/Controller/Action/SearchAction.php @@ -6,7 +6,7 @@ use Doctrine\Persistence\ManagerRegistry; use Setono\Doctrine\ORMTrait; -use Setono\SyliusMeilisearchPlugin\Engine\SearchEngine; +use Setono\SyliusMeilisearchPlugin\Engine\SearchEngineInterface; use Setono\SyliusMeilisearchPlugin\Form\Builder\SearchFormBuilderInterface; use Setono\SyliusMeilisearchPlugin\Model\IndexableInterface; use Symfony\Component\HttpFoundation\Request; @@ -22,7 +22,7 @@ public function __construct( ManagerRegistry $managerRegistry, private readonly Environment $twig, private readonly SearchFormBuilderInterface $searchFormBuilder, - private readonly SearchEngine $searchEngine, + private readonly SearchEngineInterface $searchEngine, ) { $this->managerRegistry = $managerRegistry; } diff --git a/src/Engine/SearchEngine.php b/src/Engine/SearchEngine.php index 0769b61..e291060 100644 --- a/src/Engine/SearchEngine.php +++ b/src/Engine/SearchEngine.php @@ -12,7 +12,7 @@ use Setono\SyliusMeilisearchPlugin\Meilisearch\Builder\FilterBuilderInterface; use Setono\SyliusMeilisearchPlugin\Resolver\IndexName\IndexNameResolverInterface; -final class SearchEngine +final class SearchEngine implements SearchEngineInterface { public function __construct( private readonly MetadataFactoryInterface $metadataFactory, diff --git a/src/Engine/SearchEngineInterface.php b/src/Engine/SearchEngineInterface.php new file mode 100644 index 0000000..142fea4 --- /dev/null +++ b/src/Engine/SearchEngineInterface.php @@ -0,0 +1,12 @@ + - + - + @@ -22,5 +22,7 @@ %setono_sylius_meilisearch.search.hits_per_page% + +