From bcedf056d00e59328b01bc616c199103685f70f4 Mon Sep 17 00:00:00 2001 From: Melvin van Twillert Date: Wed, 14 Feb 2024 11:05:33 +0100 Subject: [PATCH 1/2] Add SortOrder class to allow missing to be set for sorting --- src/Domain/Syntax/Sort.php | 14 ++++-- src/Domain/Syntax/SortOrder.php | 43 +++++++++++++++++++ .../Query/QueryProperties/SortingTest.php | 20 ++++++++- tests/Unit/Query/QueryTest.php | 10 +++++ tests/Unit/ScoutSearchCommandBuilderTest.php | 40 ++++++++++++++++- 5 files changed, 120 insertions(+), 7 deletions(-) create mode 100644 src/Domain/Syntax/SortOrder.php diff --git a/src/Domain/Syntax/Sort.php b/src/Domain/Syntax/Sort.php index 125b18d..16a1221 100644 --- a/src/Domain/Syntax/Sort.php +++ b/src/Domain/Syntax/Sort.php @@ -14,13 +14,19 @@ class Sort private string $field; - private string $order; + private string|array $order; - public function __construct(string $field, string $order = self::ASCENDING) + public function __construct(string $field, string|SortOrder $order = self::ASCENDING) { + $this->field = $field; - $this->order = $order; - Assert::inArray($order, [self::ASCENDING, self::DESCENDING]); + + if (is_string($order)){ + $this->order = $order; + Assert::inArray($order, [self::ASCENDING, self::DESCENDING]); + } else { + $this->order = $order->asArray(); + } } public function build(): array diff --git a/src/Domain/Syntax/SortOrder.php b/src/Domain/Syntax/SortOrder.php new file mode 100644 index 0000000..8a5c9d8 --- /dev/null +++ b/src/Domain/Syntax/SortOrder.php @@ -0,0 +1,43 @@ +order = $order; + $this->missing = $missing; + Assert::inArray($order, [self::ASCENDING, self::DESCENDING]); + Assert::inArray($missing, [self::MISSING_FIRST, self::MISSING_LAST]); + } + + public static function for(string $order = self::ASCENDING, string $missing = self::MISSING_LAST): self + { + return new self($order,$missing); + } + + public function asArray(): array + { + return [ + 'missing' => $this->missing, + 'order' => $this->order + ]; + } +} diff --git a/tests/Unit/Domain/Query/QueryProperties/SortingTest.php b/tests/Unit/Domain/Query/QueryProperties/SortingTest.php index 5f1dfaf..cb3bd89 100644 --- a/tests/Unit/Domain/Query/QueryProperties/SortingTest.php +++ b/tests/Unit/Domain/Query/QueryProperties/SortingTest.php @@ -6,6 +6,7 @@ use JeroenG\Explorer\Domain\Query\QueryProperties\Sorting; use JeroenG\Explorer\Domain\Syntax\Sort; +use JeroenG\Explorer\Domain\Syntax\SortOrder; use PHPUnit\Framework\TestCase; final class SortingTest extends TestCase @@ -18,6 +19,15 @@ public function test_it_builds_sorting(): void self::assertSame([ 'sort' => [ [ ':fld:' => 'desc' ]]],$sort->build()); } + + public function test_it_builds_sorting_from_sort_order(): void + { + $sort = Sorting::for( + new Sort(':fld:', SortOrder::for(SortOrder::DESCENDING, SortOrder::MISSING_FIRST)), + ); + + self::assertSame([ 'sort' => [ [ ':fld:' => ['missing' => '_first', 'order' => 'desc'] ]]],$sort->build()); + } public function test_it_combines(): void { @@ -31,10 +41,14 @@ public function test_it_combines(): void ); $c = Sorting::for( new Sort(':fld5:', Sort::DESCENDING), + new Sort(':fld6:', SortOrder::for(SortOrder::DESCENDING, SortOrder::MISSING_LAST)), + ); + $d = Sorting::for( + new Sort(':fld7:', SortOrder::for(SortOrder::DESCENDING, SortOrder::MISSING_FIRST)), ); - $d = Sorting::for(); + $e = Sorting::for(); - $result = $a->combine($b, $c, $d); + $result = $a->combine($b, $c, $d, $e); self::assertNotSame($a->build(), $result->build()); self::assertSame([ @@ -44,6 +58,8 @@ public function test_it_combines(): void [ ':fld3:' => 'desc' ], [ ':fld4:' => 'desc' ], [ ':fld5:' => 'desc' ], + [ ':fld6:' => ['missing' => '_last', 'order' => 'desc']], + [ ':fld7:' => ['missing' => '_first', 'order' => 'desc']] ], ], $result->build()); } diff --git a/tests/Unit/Query/QueryTest.php b/tests/Unit/Query/QueryTest.php index 408a0f1..0e34a40 100644 --- a/tests/Unit/Query/QueryTest.php +++ b/tests/Unit/Query/QueryTest.php @@ -11,6 +11,7 @@ use JeroenG\Explorer\Domain\Query\QueryProperties\TrackTotalHits; use JeroenG\Explorer\Domain\Syntax\MatchAll; use JeroenG\Explorer\Domain\Syntax\Sort; +use JeroenG\Explorer\Domain\Syntax\SortOrder; use JeroenG\Explorer\Domain\Syntax\Term; use PHPUnit\Framework\TestCase; use TypeError; @@ -44,6 +45,15 @@ public function test_it_builds_query_with_sort(): void $result = $this->query->build(); self::assertEquals([$sort->build()], $result['sort'] ?? null); } + + public function test_it_builds_query_with_sort_order(): void + { + $sort = new Sort('field', SortOrder::for(SortOrder::DESCENDING, SortOrder::MISSING_FIRST)); + $this->query->setSort([$sort]); + + $result = $this->query->build(); + self::assertEquals([$sort->build()], $result['sort'] ?? null); + } public function test_it_throws_on_invalid_sort_argument(): void { diff --git a/tests/Unit/ScoutSearchCommandBuilderTest.php b/tests/Unit/ScoutSearchCommandBuilderTest.php index 04b8d94..4e4f1a1 100644 --- a/tests/Unit/ScoutSearchCommandBuilderTest.php +++ b/tests/Unit/ScoutSearchCommandBuilderTest.php @@ -13,6 +13,7 @@ use JeroenG\Explorer\Domain\Syntax\Compound\QueryType; use JeroenG\Explorer\Domain\Syntax\MultiMatch; use JeroenG\Explorer\Domain\Syntax\Sort; +use JeroenG\Explorer\Domain\Syntax\SortOrder; use JeroenG\Explorer\Domain\Syntax\SyntaxInterface; use JeroenG\Explorer\Domain\Syntax\Term; use JeroenG\Explorer\Infrastructure\Scout\ScoutSearchCommandBuilder; @@ -139,7 +140,44 @@ public function test_it_can_set_the_sort_order(): void $command->setSort([new Sort('id', 'invalid')]); } - + + public function test_it_can_set_the_sort_order_as_array(): void + { + $command = new ScoutSearchCommandBuilder(); + + self::assertFalse($command->hasSort()); + + $command->setSort([new Sort('id')]); + + self::assertTrue($command->hasSort()); + self::assertSame([['id' => 'asc']], $command->getSort()); + + $command->setSort([]); + + self::assertFalse($command->hasSort()); + self::assertSame([], $command->getSort()); + + $command->setSort([new Sort('id', SortOrder::for('desc'))]); + + self::assertTrue($command->hasSort()); + self::assertSame([['id' => ['missing' => '_last', 'order' => 'desc']]], $command->getSort()); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Expected one of: "asc", "desc". Got: "invalid"'); + + $command->setSort([new Sort('id', SortOrder::for('invalid'))]); + } + + public function test_it_throws_exception_when_missing_is_invalid(): void + { + $command = new ScoutSearchCommandBuilder(); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Expected one of: "_first", "_last". Got: "invalid"'); + + $command->setSort([new Sort('id', SortOrder::for('desc', 'invalid'))]); + } + public function test_it_only_accepts_sort_classes(): void { $command = new ScoutSearchCommandBuilder(); From 7ad0364553cf48e301ed9a1252883b298d897bae Mon Sep 17 00:00:00 2001 From: Melvin van Twillert Date: Wed, 14 Feb 2024 13:37:57 +0100 Subject: [PATCH 2/2] Type order variable to SortOrder --- src/Domain/Syntax/Sort.php | 20 ++++--- src/Domain/Syntax/SortOrder.php | 19 +++++-- .../Query/QueryProperties/SortingTest.php | 14 ++--- tests/Unit/Domain/Syntax/SortOrderTest.php | 52 +++++++++++++++++++ 4 files changed, 82 insertions(+), 23 deletions(-) create mode 100644 tests/Unit/Domain/Syntax/SortOrderTest.php diff --git a/src/Domain/Syntax/Sort.php b/src/Domain/Syntax/Sort.php index 16a1221..6d47f5d 100644 --- a/src/Domain/Syntax/Sort.php +++ b/src/Domain/Syntax/Sort.php @@ -4,33 +4,31 @@ namespace JeroenG\Explorer\Domain\Syntax; -use Webmozart\Assert\Assert; - class Sort { + /** @deprecated Use SortOrder::ASCENDING instead */ public const ASCENDING = 'asc'; - + + /** @deprecated Use SortOrder::DESCENDING instead */ public const DESCENDING = 'desc'; private string $field; - private string|array $order; + private SortOrder $order; - public function __construct(string $field, string|SortOrder $order = self::ASCENDING) + public function __construct(string $field, string|SortOrder $order = SortOrder::ASCENDING) { - $this->field = $field; - if (is_string($order)){ - $this->order = $order; - Assert::inArray($order, [self::ASCENDING, self::DESCENDING]); + if (is_string($order)) { + $this->order = SortOrder::fromString($order); } else { - $this->order = $order->asArray(); + $this->order = $order; } } public function build(): array { - return [$this->field => $this->order]; + return [$this->field => $this->order->build()]; } } diff --git a/src/Domain/Syntax/SortOrder.php b/src/Domain/Syntax/SortOrder.php index 8a5c9d8..b5ec807 100644 --- a/src/Domain/Syntax/SortOrder.php +++ b/src/Domain/Syntax/SortOrder.php @@ -18,23 +18,32 @@ class SortOrder private string $order; - private string $missing; + private ?string $missing; - private function __construct(string $order, string $missing ) + private function __construct(string $order, ?string $missing) { $this->order = $order; $this->missing = $missing; Assert::inArray($order, [self::ASCENDING, self::DESCENDING]); - Assert::inArray($missing, [self::MISSING_FIRST, self::MISSING_LAST]); + Assert::nullOrInArray($missing, [self::MISSING_FIRST, self::MISSING_LAST]); + } + + public static function fromString(string $order): self + { + return new self($order, null); } public static function for(string $order = self::ASCENDING, string $missing = self::MISSING_LAST): self { - return new self($order,$missing); + return new self($order, $missing); } - public function asArray(): array + public function build(): array|string { + if (is_null($this->missing)) { + return $this->order; + } + return [ 'missing' => $this->missing, 'order' => $this->order diff --git a/tests/Unit/Domain/Query/QueryProperties/SortingTest.php b/tests/Unit/Domain/Query/QueryProperties/SortingTest.php index cb3bd89..d905545 100644 --- a/tests/Unit/Domain/Query/QueryProperties/SortingTest.php +++ b/tests/Unit/Domain/Query/QueryProperties/SortingTest.php @@ -14,7 +14,7 @@ final class SortingTest extends TestCase public function test_it_builds_sorting(): void { $sort = Sorting::for( - new Sort(':fld:', Sort::DESCENDING), + new Sort(':fld:', SortOrder::DESCENDING), ); self::assertSame([ 'sort' => [ [ ':fld:' => 'desc' ]]],$sort->build()); @@ -32,16 +32,16 @@ public function test_it_builds_sorting_from_sort_order(): void public function test_it_combines(): void { $a = Sorting::for( - new Sort(':fld1:', Sort::DESCENDING), - new Sort(':fld2:', Sort::DESCENDING), + new Sort(':fld1:', SortOrder::DESCENDING), + new Sort(':fld2:', SortOrder::DESCENDING), ); $b = Sorting::for( - new Sort(':fld3:', Sort::DESCENDING), - new Sort(':fld4:', Sort::DESCENDING), + new Sort(':fld3:', SortOrder::DESCENDING), + new Sort(':fld4:', SortOrder::DESCENDING), ); $c = Sorting::for( - new Sort(':fld5:', Sort::DESCENDING), - new Sort(':fld6:', SortOrder::for(SortOrder::DESCENDING, SortOrder::MISSING_LAST)), + new Sort(':fld5:', SortOrder::DESCENDING), + new Sort(':fld6:', SortOrder::for(SortOrder::DESCENDING)), ); $d = Sorting::for( new Sort(':fld7:', SortOrder::for(SortOrder::DESCENDING, SortOrder::MISSING_FIRST)), diff --git a/tests/Unit/Domain/Syntax/SortOrderTest.php b/tests/Unit/Domain/Syntax/SortOrderTest.php new file mode 100644 index 0000000..95ae4b5 --- /dev/null +++ b/tests/Unit/Domain/Syntax/SortOrderTest.php @@ -0,0 +1,52 @@ + SortOrder::MISSING_LAST, + 'order' => SortOrder::DESCENDING + ], $sort->build()); + } + + /** + * @dataProvider provideSortOrderStrings + */ + public function test_sort_order_can_be_created_from_sort_string(string $expectedResult, string $sortString): void + { + $subject = SortOrder::fromString($sortString); + Assert::assertSame($expectedResult, $subject->build()); + } + + /** + * @dataProvider provideMissingSortOrderStrings + */ + public function test_sort_order_can_be_created_from_sort_string_and_missing(array $expectedResult, string $sortString, string $missing): void + { + $subject = SortOrder::for($sortString, $missing); + Assert::assertSame($expectedResult, $subject->build()); + } + + public function provideSortOrderStrings(): iterable + { + yield 'asc' => ['asc', 'asc']; + yield 'desc' => ['desc', 'desc']; + } + + public function provideMissingSortOrderStrings(): iterable + { + yield 'asc order with _last missing' => [['missing' => '_last', 'order' => 'asc'], 'asc', '_last']; + yield 'desc order with _last missing' => [['missing' => '_last', 'order' => 'desc'], 'desc', '_last']; + yield 'asc order with _first missing' => [['missing' => '_first', 'order' => 'asc'], 'asc', '_first']; + yield 'desc order with _first missing' => [['missing' => '_first', 'order' => 'desc'], 'desc', '_first']; + } +}