diff --git a/lib/Core/Field/OrderFieldType.php b/lib/Core/Field/OrderFieldType.php index 8fe26a92..bc2335af 100644 --- a/lib/Core/Field/OrderFieldType.php +++ b/lib/Core/Field/OrderFieldType.php @@ -15,6 +15,7 @@ use Rollerworks\Component\Search\Extension\Core\DataTransformer\OrderToLocalizedTransformer; use Rollerworks\Component\Search\Extension\Core\DataTransformer\OrderTransformer; +use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; use Symfony\Component\OptionsResolver\Options; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -32,6 +33,7 @@ public function configureOptions(OptionsResolver $resolver): void { $resolver->setDefaults([ 'default' => null, + 'always' => null, 'case' => OrderTransformer::CASE_UPPERCASE, 'alias' => ['ASC' => 'ASC', 'DESC' => 'DESC'], 'view_label' => ['ASC' => 'asc', 'DESC' => 'desc'], @@ -44,9 +46,10 @@ public function configureOptions(OptionsResolver $resolver): void OrderTransformer::CASE_LOWERCASE, OrderTransformer::CASE_UPPERCASE, ]); + $resolver->setAllowedValues('default', ['asc', 'desc', 'ASC', 'DESC', null]); + $resolver->setAllowedValues('always', ['append', 'prepend', null]); $resolver->setAllowedTypes('alias', 'array'); $resolver->setAllowedTypes('view_label', ['array']); - $resolver->setAllowedTypes('default', ['null', 'string']); $resolver->setAllowedTypes('type', ['string', 'null']); $resolver->setAllowedTypes('type_options', ['array']); $resolver->setAllowedTypes('label', ['null', 'string']); @@ -85,6 +88,10 @@ public function configureOptions(OptionsResolver $resolver): void public function buildType(FieldConfig $config, array $options): void { + if ($options['always'] !== null && $options['default'] === null) { + throw new InvalidOptionsException('Setting "always" requires the "default" option is set with a direction.'); + } + $config->setNormTransformer(new OrderTransformer($options['alias'], $options['case'])); $config->setViewTransformer(new OrderToLocalizedTransformer($options['alias'], $options['view_label'], $options['case'])); } diff --git a/lib/Core/Input/AbstractInput.php b/lib/Core/Input/AbstractInput.php index 8dfc78d6..02889b61 100644 --- a/lib/Core/Input/AbstractInput.php +++ b/lib/Core/Input/AbstractInput.php @@ -15,6 +15,8 @@ use Rollerworks\Component\Search\ErrorList; use Rollerworks\Component\Search\InputProcessor; +use Rollerworks\Component\Search\SearchCondition; +use Rollerworks\Component\Search\SearchOrder; /** * AbstractInput provides the shared logic for the InputProcessors. @@ -35,6 +37,52 @@ public function __construct(?Validator $validator = null) $this->validator = $validator ?? new NullValidator(); } + /** + * Finalize the ordering of the fields. + * + * - Sets the default ordering if no ordering is set. + * - Ensures that the always-ordered fields are present. + */ + public static function finalizeOrdering(SearchCondition $condition): void + { + $ordering = $condition->getOrder()?->getFields() ?? []; + $hasOrder = $ordering !== []; + + $apply = $hasOrder; + $prepend = $append = []; + + foreach ($condition->getFieldSet()->all() as $field) { + $name = $field->getName(); + + if (! $condition->getFieldSet()->isOrder($name)) { + continue; + } + + $default = $field->getOption('default'); + + if ($default === null) { + continue; + } + + $always = $field->getOption('always'); + + if ($always === 'prepend') { + $prepend[$name] = $default; + $apply = true; + } elseif ($always === 'append') { + $append[$name] = $default; + $apply = true; + } elseif (! $hasOrder) { + $ordering[$name] = $default; + $apply = true; + } + } + + if ($apply) { + $condition->setOrder(new SearchOrder($ordering, $prepend, $append)); + } + } + /** * This method is called after processing and helps with finding bugs. */ diff --git a/lib/Core/Input/JsonInput.php b/lib/Core/Input/JsonInput.php index 2d177821..f776626f 100644 --- a/lib/Core/Input/JsonInput.php +++ b/lib/Core/Input/JsonInput.php @@ -134,6 +134,8 @@ public function process(ProcessorConfig $config, $input): SearchCondition throw new InvalidSearchConditionException($errors); } + self::finalizeOrdering($condition); + return $condition; } diff --git a/lib/Core/Input/StringInput.php b/lib/Core/Input/StringInput.php index f99ad7e5..91bf856d 100644 --- a/lib/Core/Input/StringInput.php +++ b/lib/Core/Input/StringInput.php @@ -192,6 +192,8 @@ public function process(ProcessorConfig $config, $input): SearchCondition throw new InvalidSearchConditionException($errors); } + self::finalizeOrdering($condition); + return $condition; } diff --git a/lib/Core/SearchConditionBuilder.php b/lib/Core/SearchConditionBuilder.php index beaf275d..c422facf 100644 --- a/lib/Core/SearchConditionBuilder.php +++ b/lib/Core/SearchConditionBuilder.php @@ -16,6 +16,7 @@ use Rollerworks\Component\Search\Exception\BadMethodCallException; use Rollerworks\Component\Search\Exception\InvalidArgumentException; use Rollerworks\Component\Search\Field\OrderField; +use Rollerworks\Component\Search\Input\AbstractInput; use Rollerworks\Component\Search\Value\ValuesGroup; final class SearchConditionBuilder @@ -273,6 +274,8 @@ public function getSearchCondition(): SearchCondition $searchCondition->setPrimaryCondition($this->getPrimaryCondition()); + AbstractInput::finalizeOrdering($searchCondition); + return $searchCondition; } diff --git a/lib/Core/SearchOrder.php b/lib/Core/SearchOrder.php index 48587eab..51b807d0 100644 --- a/lib/Core/SearchOrder.php +++ b/lib/Core/SearchOrder.php @@ -21,19 +21,42 @@ /** * @author Dalibor Karlović * @author Sebastiaan Stok + * + * @psalm-type Sorting = array */ final class SearchOrder { - /** @var array */ + /** @psalm-var Sorting */ private readonly array $fields; + /** @psalm-var Sorting */ + private readonly array $prepend; + + /** @psalm-var Sorting */ + private readonly array $append; + + /** @psalm-var Sorting */ + private array $finalSorting; + private readonly ValuesGroup $valuesGroup; /** + * Creates a new SearchOrder. + * + * The $prepend fields always appear first in the sorting order. + * The $append fields always appear last in the sorting order. + * + * Any fields added with $values will always overwrite the values provided + * in $prepend and $append, while keeping there original position. + * * @param ValuesGroup|array $values + * @param array $prepend + * @param array $append */ public function __construct( ValuesGroup | array $values, + array $prepend = [], + array $append = [], ) { if ($values instanceof ValuesGroup) { trigger_deprecation('rollerworks/search', '2.0-BETA14', 'Passing a "%s" to "%s()" is deprecated, pass an associative array fields and there directions instead.', ValuesGroup::class, __METHOD__); @@ -55,7 +78,71 @@ public function __construct( $values = $fields; } + $this->prepend = $this->processFields($prepend); + $this->fields = $this->processFields($values); + $this->append = $this->processFields($append); + $this->finalSorting = array_merge($this->prepend, $this->fields); + + // Only append fields that are not already in prepend or fields + $this->finalSorting += $this->append; + $valuesGroup = new ValuesGroup(); + + foreach ($this->finalSorting as $fieldName => $direction) { + $valuesGroup->addField($fieldName, (new ValuesBag())->addSimpleValue($direction)); + } + $this->valuesGroup = $valuesGroup; + } + + public function getValuesGroup(): ValuesGroup + { + return $this->valuesGroup; + } + + /** + * @psalm-return Sorting + */ + public function getFields(): array + { + return $this->fields; + } + + /** + * @psalm-return Sorting + */ + public function getAppend(): array + { + return $this->append; + } + + /** + * @psalm-return Sorting + */ + public function getPrepend(): array + { + return $this->prepend; + } + + /** + * Gets the final sorting order. + * + * This is the final sorting order, which is the combination fields that are + * always-sorted fields and fields provided by user-input. + * + * @psalm-return Sorting + */ + public function getSorting(): array + { + return $this->finalSorting; + } + + /** + * @param array $values + * + * @psalm-return Sorting + */ + private function processFields(array $values): array + { $fields = []; foreach ($values as $fieldName => $direction) { @@ -64,33 +151,24 @@ public function __construct( } if (! \is_string($direction)) { - throw new InvalidArgumentException(\sprintf('Field "%s" direction must be a string.', $fieldName)); + throw new InvalidArgumentException(\sprintf('Field "%s" direction must be a string, "%s" given.', $fieldName, get_debug_type($direction))); } $direction = mb_strtolower($direction); if (! \in_array($direction, ['desc', 'asc'], true)) { - throw new InvalidArgumentException(\sprintf('Invalid direction provided "%s" for field "%s", must be either "asc" or "desc" (case insensitive).', $direction, $fieldName)); + throw new InvalidArgumentException( + \sprintf( + 'Invalid direction provided "%s" for field "%s", must be either "asc" or "desc" (case insensitive).', + $direction, + $fieldName + ) + ); } - $valuesGroup->addField($fieldName, (new ValuesBag())->addSimpleValue($direction)); $fields[$fieldName] = $direction; } - $this->fields = $fields; - $this->valuesGroup = $valuesGroup; - } - - public function getValuesGroup(): ValuesGroup - { - return $this->valuesGroup; - } - - /** - * @return array - */ - public function getFields(): array - { - return $this->fields; + return $fields; } } diff --git a/lib/Core/Tests/Field/OrderFieldTypeTest.php b/lib/Core/Tests/Field/OrderFieldTypeTest.php index 4889c9d8..dea334e9 100644 --- a/lib/Core/Tests/Field/OrderFieldTypeTest.php +++ b/lib/Core/Tests/Field/OrderFieldTypeTest.php @@ -19,6 +19,7 @@ use Rollerworks\Component\Search\Field\OrderFieldType; use Rollerworks\Component\Search\Test\FieldTransformationAssertion; use Rollerworks\Component\Search\Test\SearchIntegrationTestCase; +use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; /** * @internal @@ -254,4 +255,17 @@ public function it_fails_to_transform_with_invalid_direction(): void ) ; } + + /** + * @test + */ + public function error_with_always_append_and_no_default_value(): void + { + $this->expectException(InvalidOptionsException::class); + $this->expectExceptionMessage('Setting "always" requires the "default" option is set with a direction.'); + + $this->getFactory()->createField('@id', OrderFieldType::class, [ + 'always' => 'append', + ]); + } } diff --git a/lib/Core/Tests/Input/InputProcessorTestCase.php b/lib/Core/Tests/Input/InputProcessorTestCase.php index 59254c2d..4b11d0fc 100644 --- a/lib/Core/Tests/Input/InputProcessorTestCase.php +++ b/lib/Core/Tests/Input/InputProcessorTestCase.php @@ -59,9 +59,10 @@ protected function getFieldSet(bool $build = true, bool $order = false) $fieldSet->add('date', DateType::class, ['pattern' => 'MM-dd-yyyy']); if ($order) { - $fieldSet->add('@date', OrderFieldType::class, ['case' => OrderTransformer::CASE_LOWERCASE, 'alias' => ['up' => 'ASC', 'down' => 'DESC'], 'default' => 'down']); + $fieldSet->add('@date', OrderFieldType::class, ['case' => OrderTransformer::CASE_LOWERCASE, 'alias' => ['up' => 'ASC', 'down' => 'DESC'], 'default' => 'desc']); $fieldSet->add('@id', OrderFieldType::class, ['default' => 'ASC']); } + $fieldSet->set( $this->getFactory()->createField('no-range-field', IntegerType::class) ->setValueTypeSupport(Range::class, false) diff --git a/lib/Core/Tests/SearchConditionBuilderTest.php b/lib/Core/Tests/SearchConditionBuilderTest.php index b8d3d22a..e21d4d33 100644 --- a/lib/Core/Tests/SearchConditionBuilderTest.php +++ b/lib/Core/Tests/SearchConditionBuilderTest.php @@ -473,4 +473,172 @@ static function (SearchConditionBuilder $builder): void { }, ]; } + + /** + * @test + */ + public function finalizes_ordering_fields_without_existing(): void + { + $searchFactory = Searches::createSearchFactory(); + + $fieldSetBuilder = $searchFactory->createFieldSetBuilder(); + $fieldSetBuilder + ->add('id', IntegerType::class) + ->add('@id', OrderFieldType::class) + ->add('@name', OrderFieldType::class, ['default' => 'ASC']) + ; + + $fieldSet = $fieldSetBuilder->getFieldSet('users'); + $condBuilder = SearchConditionBuilder::create($fieldSet) + ->field('id') + ->addSimpleValue(10) + ->addSimpleValue(30) + ->end() + ; + + $expected = new SearchCondition($fieldSet, (new ValuesGroup())->addField('id', (new ValuesBag())->addSimpleValue(10)->addSimpleValue(30))); + $expected->setOrder(new SearchOrder(['@name' => 'ASC'])); + + self::assertEquals($expected, $condBuilder->getSearchCondition()); + } + + /** + * @test + */ + public function finalizes_ordering_fields_with_existing(): void + { + $searchFactory = Searches::createSearchFactory(); + + $fieldSetBuilder = $searchFactory->createFieldSetBuilder(); + $fieldSetBuilder + ->add('id', IntegerType::class) + ->add('@id', OrderFieldType::class) + ->add('@name', OrderFieldType::class, ['default' => 'ASC']) + ; + + $fieldSet = $fieldSetBuilder->getFieldSet('users'); + $condBuilder = SearchConditionBuilder::create($fieldSet) + ->field('id') + ->addSimpleValue(10) + ->addSimpleValue(30) + ->end() + ->order('@id', 'DESC') + ; + + $expected = new SearchCondition($fieldSet, (new ValuesGroup())->addField('id', (new ValuesBag())->addSimpleValue(10)->addSimpleValue(30))); + $expected->setOrder(new SearchOrder(['@id' => 'DESC'])); + + self::assertEquals($expected, $condBuilder->getSearchCondition()); + } + + /** + * @test + */ + public function finalize_appended_fields_without_existing(): void + { + $fieldSetBuilder = Searches::createSearchFactory()->createFieldSetBuilder(); + $fieldSetBuilder + ->add('id', IntegerType::class) + ->add('@id', OrderFieldType::class) + ->add('@name', OrderFieldType::class, ['always' => 'append', 'default' => 'ASC']) + ; + + $fieldSet = $fieldSetBuilder->getFieldSet('users'); + $condBuilder = SearchConditionBuilder::create($fieldSet) + ->field('id') + ->addSimpleValue(10) + ->addSimpleValue(30) + ->end() + ; + + $expected = new SearchCondition($fieldSet, (new ValuesGroup())->addField('id', (new ValuesBag())->addSimpleValue(10)->addSimpleValue(30))); + $expected->setOrder(new SearchOrder([], append: ['@name' => 'ASC'])); + + self::assertEquals($expected, $condBuilder->getSearchCondition()); + } + + /** + * @test + */ + public function finalize_appended_fields_without_existing_default_set(): void + { + $fieldSetBuilder = Searches::createSearchFactory()->createFieldSetBuilder(); + $fieldSetBuilder + ->add('id', IntegerType::class) + ->add('@id', OrderFieldType::class, ['default' => 'desc']) + ->add('@name', OrderFieldType::class, ['always' => 'append', 'default' => 'ASC']) + ; + + $fieldSet = $fieldSetBuilder->getFieldSet('users'); + $condBuilder = SearchConditionBuilder::create($fieldSet) + ->field('id') + ->addSimpleValue(10) + ->addSimpleValue(30) + ->end() + ; + + $expected = new SearchCondition($fieldSet, (new ValuesGroup())->addField('id', (new ValuesBag())->addSimpleValue(10)->addSimpleValue(30))); + $expected->setOrder(new SearchOrder(['@id' => 'desc'], append: ['@name' => 'ASC'])); + + self::assertEquals($expected, $condBuilder->getSearchCondition()); + } + + /** + * @test + */ + public function finalize_prepend_fields_merge(): void + { + $searchFactory = Searches::createSearchFactory(); + + $fieldSetBuilder = $searchFactory->createFieldSetBuilder(); + $fieldSetBuilder + ->add('id', IntegerType::class) + ->add('@id', OrderFieldType::class) + ->add('@name', OrderFieldType::class, ['always' => 'prepend', 'default' => 'ASC']) + ; + + $fieldSet = $fieldSetBuilder->getFieldSet('users'); + $condBuilder = SearchConditionBuilder::create($fieldSet) + ->field('id') + ->addSimpleValue(10) + ->addSimpleValue(30) + ->end() + ->order('@id', 'DESC') + ; + + $expected = new SearchCondition($fieldSet, (new ValuesGroup())->addField('id', (new ValuesBag())->addSimpleValue(10)->addSimpleValue(30))); + $expected->setOrder(new SearchOrder(['@id' => 'DESC'], prepend: ['@name' => 'ASC'])); + + self::assertEquals($expected, $condBuilder->getSearchCondition()); + } + + /** + * @test + */ + public function finalize_prepend_and_appended_fields_merge(): void + { + $searchFactory = Searches::createSearchFactory(); + + $fieldSetBuilder = $searchFactory->createFieldSetBuilder(); + $fieldSetBuilder + ->add('id', IntegerType::class) + ->add('@id', OrderFieldType::class) + ->add('@name', OrderFieldType::class, ['always' => 'append', 'default' => 'ASC']) + ->add('@group', OrderFieldType::class, ['always' => 'prepend', 'default' => 'DESC']) + ; + + $fieldSet = $fieldSetBuilder->getFieldSet('users'); + $condBuilder = SearchConditionBuilder::create($fieldSet) + ->field('id') + ->addSimpleValue(10) + ->addSimpleValue(30) + ->end() + ->order('@id', 'DESC') + ; + + $expected = new SearchCondition($fieldSet, (new ValuesGroup())->addField('id', (new ValuesBag())->addSimpleValue(10)->addSimpleValue(30))); + $expected->setOrder(new SearchOrder(['@id' => 'DESC'], prepend: ['@group' => 'desc'], append: ['@name' => 'ASC'])); + + self::assertEquals($expected, $condBuilder->getSearchCondition()); + } } diff --git a/lib/Core/Tests/SearchOrderTest.php b/lib/Core/Tests/SearchOrderTest.php index ca39eb6e..e9da1675 100644 --- a/lib/Core/Tests/SearchOrderTest.php +++ b/lib/Core/Tests/SearchOrderTest.php @@ -22,6 +22,8 @@ /** * @internal + * + * @psalm-type Sorting = array */ final class SearchOrderTest extends TestCase { @@ -98,6 +100,67 @@ public function construct_with_uppercase_direction(): void ); } + /** + * @test + */ + public function construct_with_prepend(): void + { + // Prepend should be placed before any user sorting + self::assertSortingSame( + finalSorting: ['@date' => 'asc', '@id' => 'desc', '@name' => 'asc'], + prepend: ['@date' => 'asc'], + append: [], + fields: ['@id' => 'desc', '@name' => 'asc'], + actual: new SearchOrder(['@id' => 'DESC', '@name' => 'ASC'], prepend: ['@date' => 'ASC']), + ); + + // Prepend should keep the original position + self::assertSortingSame( + finalSorting: ['@name' => 'asc', '@id' => 'desc'], + prepend: ['@name' => 'desc'], + append: [], + fields: ['@id' => 'desc', '@name' => 'asc'], + actual: new SearchOrder(['@id' => 'DESC', '@name' => 'ASC'], prepend: ['@name' => 'DESC']), + ); + } + + /** + * @test + */ + public function construct_with_append(): void + { + // Append should be placed after the user sorting + self::assertSortingSame( + finalSorting: ['@id' => 'desc', '@name' => 'asc', '@date' => 'asc'], + prepend: [], + append: ['@date' => 'asc'], + fields: ['@id' => 'desc', '@name' => 'asc'], + actual: new SearchOrder(['@id' => 'DESC', '@name' => 'ASC'], append: ['@date' => 'ASC']), + ); + + // Append should ignore already existing sorting + self::assertSortingSame( + finalSorting: ['@id' => 'desc', '@name' => 'asc', '@date' => 'desc'], + prepend: [], + append: ['@id' => 'asc', '@date' => 'desc'], + fields: ['@id' => 'desc', '@name' => 'asc'], + actual: new SearchOrder(['@id' => 'DESC', '@name' => 'ASC'], append: ['@id' => 'ASC', '@date' => 'desc']), + ); + + // Prepend and append + self::assertSortingSame( + finalSorting: ['@name' => 'asc', '@group' => 'desc', '@id' => 'desc', '@date' => 'desc'], + prepend: ['@name' => 'desc', '@group' => 'desc'], + append: ['@id' => 'asc', '@date' => 'desc'], + fields: ['@id' => 'desc', '@name' => 'asc'], + actual: new SearchOrder( + values: ['@id' => 'DESC', '@name' => 'ASC'], + prepend: ['@name' => 'DESC', '@group' => 'DESC'], + append: ['@id' => 'ASC', '@date' => 'desc'] + ), + ); + } + /** * @test */ @@ -121,7 +184,7 @@ public function fail_with_invalid_value_type(): void $valuesGroup->addField('@id', (new ValuesBag())->addSimpleValue(['up'])); $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('Field "@id" direction must be a string.'); + $this->expectExceptionMessage('Field "@id" direction must be a string, "array" given.'); new SearchOrder($valuesGroup); } @@ -196,4 +259,27 @@ public function fail_with_invalid_type_value(): void new SearchOrder($valuesGroup); } + + /** + * @param Sorting $finalSorting + * @param Sorting $prepend + * @param Sorting $append + * @param Sorting $fields + */ + private static function assertSortingSame(array $finalSorting, array $prepend, array $append, array $fields, SearchOrder $actual): void + { + $valuesGroup = new ValuesGroup(); + + foreach ($finalSorting as $field => $direction) { + $valuesGroup->addField($field, (new ValuesBag())->addSimpleValue($direction)); + } + + self::assertEquals($valuesGroup, $actual->getValuesGroup(), 'The values group does not match the expected values group.'); + self::assertSame(array_keys($finalSorting), array_keys($actual->getValuesGroup()->getFields()), 'The fields do not match the expected fields order.'); + + self::assertSame($finalSorting, $actual->getSorting(), 'The final sorting does not match the expected sorting.'); + self::assertSame($prepend, $actual->getPrepend(), 'The prepend does not match.'); + self::assertSame($fields, $actual->getFields(), 'The fields do not match.'); + self::assertSame($append, $actual->getAppend(), 'The append does not match.'); + } } diff --git a/lib/Doctrine/Orm/DqlConditionGenerator.php b/lib/Doctrine/Orm/DqlConditionGenerator.php index 1fb4b939..16b96707 100644 --- a/lib/Doctrine/Orm/DqlConditionGenerator.php +++ b/lib/Doctrine/Orm/DqlConditionGenerator.php @@ -84,7 +84,7 @@ public static function applySortingTo(?SearchOrder $order, QueryBuilder $qb, Fie $fields = $configBuilder->getFields(); - foreach ($order->getFields() as $fieldName => $direction) { + foreach ($order->getSorting() as $fieldName => $direction) { if (! isset($fields[$fieldName])) { continue; } diff --git a/lib/Elasticsearch/QueryConditionGenerator.php b/lib/Elasticsearch/QueryConditionGenerator.php index a4808515..8ff265b0 100644 --- a/lib/Elasticsearch/QueryConditionGenerator.php +++ b/lib/Elasticsearch/QueryConditionGenerator.php @@ -345,7 +345,7 @@ private function processOrder(?SearchOrder $order, array &$clause, array &$condi $hints = new QueryPreparationHints(); $hints->context = QueryPreparationHints::CONTEXT_ORDER; - foreach ($order->getFields() as $fieldName => $direction) { + foreach ($order->getSorting() as $fieldName => $direction) { $mapping = $this->mappings[$fieldName]; // apply conditions from order fields