Make SearchOrder validating more strict

UPGRADE-2.0.md

@@ -1,3 +1,11 @@
+UPGRADE FROM 2.0-BETA13 to 2.0-BETA14
+=====================================
+
+### Core
+
+ * The `SearchOrder` now expects an associative array of field-names and direction.
+   Passing a `ValuesGroup` is deprecated, and will be removed in v3.0.
+
 UPGRADE FROM 2.0-BETA10 to 2.0-BETA13
 =====================================
 

lib/Core/Input/JsonInput.php

@@ -19,11 +19,9 @@
 use Rollerworks\Component\Search\Exception\InvalidSearchConditionException;
 use Rollerworks\Component\Search\Exception\UnexpectedTypeException;
 use Rollerworks\Component\Search\Exception\UnknownFieldException;
-use Rollerworks\Component\Search\Field\FieldConfig;
 use Rollerworks\Component\Search\Field\OrderField;
 use Rollerworks\Component\Search\FieldSet;
 use Rollerworks\Component\Search\SearchCondition;
-use Rollerworks\Component\Search\SearchOrder;
 use Rollerworks\Component\Search\StructureBuilder;
 use Rollerworks\Component\Search\Value\ValuesGroup;
 
@@ -77,7 +75,7 @@
 final class JsonInput extends AbstractInput
 {
     private ?StructureBuilder $structureBuilder = null;
-    private ?StructureBuilder $orderStructureBuilder = null;
+    private ?OrderStructureBuilder $orderStructureBuilder = null;
 
     public function process(ProcessorConfig $config, $input): SearchCondition
     {
@@ -219,22 +217,21 @@ private function processOrder(SearchCondition $condition, array $array, FieldSet
         $order = $array['order'] ?? [];
 
         if ($order === []) {
-            /** @var FieldConfig $field */
             foreach ($fieldSet->all() as $name => $field) {
+                if (! OrderField::isOrder($name)) {
+                    continue;
+                }
+
                 $direction = $field->getOption('default');
 
-                if (OrderField::isOrder($name) && $direction !== null) {
+                if ($direction !== null) {
                     $this->orderStructureBuilder->field($name, '[order][%s]');
                     $this->orderStructureBuilder->simpleValue($direction, '');
                     $this->orderStructureBuilder->endValues();
                 }
             }
 
-            $orderValuesGroup = $this->orderStructureBuilder->getRootGroup();
-
-            if ($orderValuesGroup->countValues() > 0) {
-                $condition->setOrder(new SearchOrder($orderValuesGroup));
-            }
+            $condition->setOrder($this->orderStructureBuilder->getOrder());
 
             return;
         }
@@ -245,8 +242,7 @@ private function processOrder(SearchCondition $condition, array $array, FieldSet
             $this->orderStructureBuilder->endValues();
         }
 
-        $orderCondition = new SearchOrder($this->orderStructureBuilder->getRootGroup());
-        $condition->setOrder($orderCondition);
+        $condition->setOrder($this->orderStructureBuilder->getOrder());
     }
 
     private function assertValueArrayHasKeys($array, array $requiredKeys, string $path): void

lib/Core/Input/OrderStructureBuilder.php

@@ -20,6 +20,7 @@
 use Rollerworks\Component\Search\Exception\TransformationFailedException;
 use Rollerworks\Component\Search\Field\FieldConfig;
 use Rollerworks\Component\Search\FieldSet;
+use Rollerworks\Component\Search\SearchOrder;
 use Rollerworks\Component\Search\StructureBuilder;
 use Rollerworks\Component\Search\Value\ValuesBag;
 use Rollerworks\Component\Search\Value\ValuesGroup;
@@ -35,6 +36,9 @@ final class OrderStructureBuilder implements StructureBuilder
     private ?ValuesBag $valuesBag = null;
     private ?DataTransformer $inputTransformer;
 
+    /** @var array<string, mixed> */
+    private array $order = [];
+
     public function __construct(
         ProcessorConfig $config,
         private readonly Validator $validator,
@@ -91,17 +95,21 @@ public function simpleValue(mixed $value, string $path): void
             throw new \LogicException('Cannot add value to unknown bag.');
         }
 
+        $name = $this->fieldConfig->getName();
+
         if ($this->valuesBag->count()) {
-            throw OrderStructureException::invalidValue($this->fieldConfig->getName());
+            throw OrderStructureException::invalidValue($name);
         }
 
-        $path = str_replace('{pos}', $this->fieldConfig->getName(), $path);
+        $path = str_replace('{pos}', $name, $path);
+        $modelVal = $this->inputToNorm($value, $path);
 
-        if (($modelVal = $this->inputToNorm($value, $path)) !== null) {
+        if ($modelVal !== null) {
             $this->validator->validate($modelVal, 'simple', $value, $path);
         }
 
         $this->valuesBag->addSimpleValue($modelVal);
+        $this->order[$name] = $modelVal;
     }
 
     public function excludedSimpleValue(mixed $value, string $path): void
@@ -135,6 +143,11 @@ public function endValues(): void
         $this->valuesBag = null;
     }
 
+    public function getOrder(): ?SearchOrder
+    {
+        return $this->order === [] ? null : new SearchOrder($this->order);
+    }
+
     private function addError(ConditionErrorMessage $error): void
     {
         $this->errorList[] = $error;

lib/Core/Input/StringInput.php

@@ -24,7 +24,6 @@
 use Rollerworks\Component\Search\Field\OrderField;
 use Rollerworks\Component\Search\FieldSet;
 use Rollerworks\Component\Search\SearchCondition;
-use Rollerworks\Component\Search\SearchOrder;
 use Rollerworks\Component\Search\StructureBuilder;
 use Rollerworks\Component\Search\Value\ValuesGroup;
 
@@ -137,7 +136,7 @@
 abstract class StringInput extends AbstractInput
 {
     protected ?StructureBuilder $structureBuilder;
-    protected ?StructureBuilder $orderStructureBuilder;
+    protected ?OrderStructureBuilder $orderStructureBuilder;
 
     /** @var array<string, string> */
     protected array $fields = [];
@@ -178,12 +177,8 @@ public function process(ProcessorConfig $config, $input): SearchCondition
         try {
             $this->parse($config, $input, $fieldSet);
             $condition = new SearchCondition($fieldSet, $this->structureBuilder->getRootGroup());
+            $condition->setOrder($this->orderStructureBuilder->getOrder());
 
-            $orderValuesGroup = $this->orderStructureBuilder->getRootGroup();
-
-            if ($orderValuesGroup->countValues() > 0) {
-                $condition->setOrder(new SearchOrder($orderValuesGroup));
-            }
             $this->assertLevel0();
         } catch (InputProcessorException $e) {
             $this->errors[] = $e->toErrorMessageObj();

lib/Core/SearchConditionBuilder.php

@@ -16,13 +16,14 @@
 use Rollerworks\Component\Search\Exception\BadMethodCallException;
 use Rollerworks\Component\Search\Exception\InvalidArgumentException;
 use Rollerworks\Component\Search\Field\OrderField;
-use Rollerworks\Component\Search\Value\ValuesBag;
 use Rollerworks\Component\Search\Value\ValuesGroup;
 
 final class SearchConditionBuilder
 {
+    /** @var array<string, 'ASC'|'DESC'> */
+    private ?array $order = [];
+
     private ValuesGroup $valuesGroup;
-    private ?ValuesGroup $order = null;
     private ?self $primaryCondition = null;
 
     /**
@@ -80,12 +81,12 @@ public function setGroupLogical(string $logical): self
      * ->end() // Returns to the main condition
      * ```
      *
-     * @param string                    $name      The field-name (must be a valid known ordering field like "@id")
-     * @param 'asc'|'desc'|'ASC'|'DESC' $direction
+     * @param string                         $name      The field-name (must be a valid known ordering field like "@id")
+     * @param 'asc'|'desc'|'ASC'|'DESC'|null $direction Use null to remove the ordering for the field
      *
      * @return $this
      */
-    public function order(string $name, string $direction = 'ASC'): self
+    public function order(string $name, ?string $direction = 'ASC'): self
     {
         if ($this->parent && $this->parent->primaryCondition !== $this) {
             throw new BadMethodCallException('Cannot add ordering at nested levels.');
@@ -96,19 +97,20 @@ public function order(string $name, string $direction = 'ASC'): self
         }
 
         $this->fieldSet->get($name);
+
+        if ($direction === null) {
+            unset($this->order[$name]);
+
+            return $this;
+        }
+
         $direction = mb_strtoupper($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, $name));
         }
 
-        if ($this->order === null) {
-            $this->order = new ValuesGroup();
-        }
-
-        $values = new ValuesBag();
-        $values->addSimpleValue($direction);
-        $this->order->addField($name, $values);
+        $this->order[$name] = $direction;
 
         return $this;
     }
@@ -120,7 +122,7 @@ public function order(string $name, string $direction = 'ASC'): self
      */
     public function clearOrder(): self
     {
-        $this->order = null;
+        $this->order = [];
 
         return $this;
     }

lib/Core/SearchOrder.php

@@ -14,19 +14,71 @@
 namespace Rollerworks\Component\Search;
 
 use Rollerworks\Component\Search\Exception\InvalidArgumentException;
+use Rollerworks\Component\Search\Field\OrderField;
+use Rollerworks\Component\Search\Value\ValuesBag;
 use Rollerworks\Component\Search\Value\ValuesGroup;
 
 /**
  * @author Dalibor Karlović <dalibor@flexolabs.io>
+ * @author Sebastiaan Stok <s.stok@rollerscapes.net>
  */
 final class SearchOrder
 {
+    /** @var array <string, 'desc'|'asc'> */
+    private readonly array $fields;
+
+    private readonly ValuesGroup $valuesGroup;
+
+    /**
+     * @param ValuesGroup|array<string, 'desc'|'asc'|'DESC'|'ASC'> $values
+     */
     public function __construct(
-        private ValuesGroup $valuesGroup,
+        ValuesGroup | array $values,
     ) {
-        if ($valuesGroup->hasGroups()) {
-            throw new InvalidArgumentException('A SearchOrder must have a single-level structure. Only fields with single values are accepted.');
+        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__);
+
+            if ($values->hasGroups()) {
+                throw new InvalidArgumentException('A SearchOrder must have a single-level structure. Only fields with single values are accepted.');
+            }
+
+            $fields = [];
+
+            foreach ($values->getFields() as $fieldName => $valuesBag) {
+                if ($valuesBag->count() !== 1 || ! $valuesBag->hasSimpleValues()) {
+                    throw new InvalidArgumentException(\sprintf('Field "%s" must have a single value only.', $fieldName));
+                }
+
+                $fields[$fieldName] = current($valuesBag->getSimpleValues());
+            }
+
+            $values = $fields;
         }
+
+        $valuesGroup = new ValuesGroup();
+        $fields = [];
+
+        foreach ($values as $fieldName => $direction) {
+            if (! OrderField::isOrder($fieldName)) {
+                throw new InvalidArgumentException(\sprintf('Field "%s" is not a valid ordering field. Expected either "@%1$s".', $fieldName));
+            }
+
+            if (! \is_string($direction)) {
+                throw new InvalidArgumentException(\sprintf('Field "%s" direction must be a string.', $fieldName));
+            }
+
+            $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));
+            }
+
+            $valuesGroup->addField($fieldName, (new ValuesBag())->addSimpleValue($direction));
+            $fields[$fieldName] = $direction;
+        }
+
+        $this->fields = $fields;
+        $this->valuesGroup = $valuesGroup;
     }
 
     public function getValuesGroup(): ValuesGroup
@@ -39,15 +91,6 @@ public function getValuesGroup(): ValuesGroup
      */
     public function getFields(): array
     {
-        $fields = [];
-
-        foreach ($this->valuesGroup->getFields() as $fieldName => $valuesBag) {
-            $direction = mb_strtolower((string) current($valuesBag->getSimpleValues()));
-            \assert($direction === 'desc' || $direction === 'asc');
-
-            $fields[$fieldName] = $direction;
-        }
-
-        return $fields;
+        return $this->fields;
     }
 }

lib/Core/Tests/SearchConditionTest.php

@@ -128,7 +128,7 @@ public function it_gives_whether_condition_is_empty(): void
         // Ordering
         self::assertFalse(
             (new SearchCondition($fieldSet, new ValuesGroup()))
-                ->setOrder(new SearchOrder((new ValuesGroup())->addField('id', (new ValuesBag())->addSimpleValue('desc'))))
+                ->setOrder(new SearchOrder(['@id' => 'desc']))
                 ->isEmpty(),
         );
     }