From d88c9ce9a9f37e7e47a361dcb847266b3670c0dd Mon Sep 17 00:00:00 2001 From: Sebastiaan Stok Date: Wed, 18 Mar 2026 09:33:34 +0100 Subject: [PATCH] [Core] Ensure proper JsonInput structure - Check for unexpected keys - Check for unexpected structure (expected [...] got [...]) - Add missing tests for validating missing keys --- lib/Core/Input/JsonInput.php | 95 +++++++++-- lib/Core/Tests/Input/JsonInputTest.php | 152 ++++++++++++++++++ .../Tests/Functional/ApiPlatformTest.php | 2 +- 3 files changed, 232 insertions(+), 17 deletions(-) diff --git a/lib/Core/Input/JsonInput.php b/lib/Core/Input/JsonInput.php index f776626f..28181482 100644 --- a/lib/Core/Input/JsonInput.php +++ b/lib/Core/Input/JsonInput.php @@ -33,12 +33,11 @@ * Each entry must contain an array with 'fields' and/or 'groups' structures. * Optionally the array can contain 'logical-case' => 'OR' to make it OR-cased. * - * The 'order' setting can only be applied at root level, and must NOT begin with the @-sign. - * + * The 'order' setting can only be applied at root level, and may begin with the @-sign. * The 'groups' array contains groups with the keys as described above ('fields' and/or 'groups'). * - * The fields array is an hash-map where each key is the field-name - * and the value as follow; All the types are optional, but at least one must exists. + * The 'fields' array is an hash-map where each key is the "field-name" + * and the value as follow; All the types are optional, but at least one must exists: * * ``` * { @@ -112,6 +111,8 @@ public function process(ProcessorConfig $config, $input): SearchCondition $this->orderStructureBuilder = new OrderStructureBuilder($this->config, $this->validator, $this->errors); try { + $this->assertArrayStructure($array, ['fields' => 'array', 'groups' => 'list', 'order' => 'array', 'logical-case' => ['OR', 'AND']]); + $valuesGroup = $this->structureBuilder->getRootGroup(); $valuesGroup->setGroupLogical($array['logical-case'] ?? ValuesGroup::GROUP_LOGICAL_AND); @@ -143,7 +144,9 @@ private function processGroup(array $group): void { $this->processFields($group['fields'] ?? []); - foreach ($group['groups'] ?? [] as $sub) { + foreach ($group['groups'] ?? [] as $idx => $sub) { + $this->assertArrayStructure($sub, ['fields' => 'array', 'groups' => 'list', 'order' => 'array', 'logical-case' => ['OR', 'AND']], "[groups][{$idx}]"); + $this->structureBuilder->enterGroup($sub['logical-case'] ?? ValuesGroup::GROUP_LOGICAL_AND, '[groups][%d]'); $this->processGroup($sub); $this->structureBuilder->leaveGroup(); @@ -159,16 +162,22 @@ private function processFields(array $values): void $this->structureBuilder->field($name, '[fields][%s]'); - foreach ($value['simple-values'] ?? [] as $index => $val) { + $this->assertArrayStructure( + $value, + ['simple-values' => 'list', 'excluded-simple-values' => 'list', 'ranges' => 'list', 'excluded-ranges' => 'list', 'comparisons' => 'list', 'pattern-matchers' => 'list'], + ); + + foreach ($value['simple-values'] ?? [] as $val) { $this->structureBuilder->simpleValue($val, '[simple-values][{idx}]'); } - foreach ($value['excluded-simple-values'] ?? [] as $index => $val) { + foreach ($value['excluded-simple-values'] ?? [] as $val) { $this->structureBuilder->excludedSimpleValue($val, '[excluded-simple-values][{idx}]'); } foreach ($value['ranges'] ?? [] as $index => $range) { $this->assertValueArrayHasKeys($range, ['lower', 'upper'], "[ranges][{$index}]"); + $this->assertArrayStructure($range, ['lower' => 'any', 'upper' => 'any', 'inclusive-lower' => 'bool', 'inclusive-upper' => 'bool'], "[ranges][{$index}]"); $this->structureBuilder->rangeValue( $range['lower'], @@ -181,6 +190,7 @@ private function processFields(array $values): void foreach ($value['excluded-ranges'] ?? [] as $index => $range) { $this->assertValueArrayHasKeys($range, ['lower', 'upper'], "[excluded-ranges][{$index}]"); + $this->assertArrayStructure($range, ['lower' => 'any', 'upper' => 'any', 'inclusive-lower' => 'bool', 'inclusive-upper' => 'bool'], "[excluded-ranges][{$index}]"); $this->structureBuilder->excludedRangeValue( $range['lower'], @@ -193,6 +203,8 @@ private function processFields(array $values): void foreach ($value['comparisons'] ?? [] as $index => $comparison) { $this->assertValueArrayHasKeys($comparison, ['value', 'operator'], "[comparisons][{$index}]"); + $this->assertArrayStructure($comparison, ['operator' => 'any', 'value' => 'any'], "[comparison][{$index}]"); + $this->structureBuilder->comparisonValue( $comparison['operator'], $comparison['value'], @@ -202,6 +214,8 @@ private function processFields(array $values): void foreach ($value['pattern-matchers'] ?? [] as $index => $matcher) { $this->assertValueArrayHasKeys($matcher, ['value', 'type'], "[pattern-matchers][{$index}]"); + $this->assertArrayStructure($matcher, ['type' => 'any', 'value' => 'string', 'case-insensitive' => 'bool'], "[pattern-matchers][{$index}]"); + $this->structureBuilder->patterMatchValue( $matcher['type'], $matcher['value'], @@ -227,8 +241,8 @@ private function processOrder(SearchCondition $condition, array $array, FieldSet $direction = $field->getOption('default'); if ($direction !== null) { - $this->orderStructureBuilder->field($name, '[order][%s]'); - $this->orderStructureBuilder->simpleValue($direction, ''); + $this->orderStructureBuilder->field($name, ''); + $this->orderStructureBuilder->simpleValue($direction, '[order][{pos}]'); $this->orderStructureBuilder->endValues(); } } @@ -239,15 +253,64 @@ private function processOrder(SearchCondition $condition, array $array, FieldSet } foreach ($order as $name => $direction) { - $this->orderStructureBuilder->field('@' . $name, '[order][%s]'); - $this->orderStructureBuilder->simpleValue($direction, ''); + $this->orderStructureBuilder->field('@' . mb_ltrim($name, '@'), ''); + $this->orderStructureBuilder->simpleValue($direction, '[order][{pos}]'); $this->orderStructureBuilder->endValues(); } $condition->setOrder($this->orderStructureBuilder->getOrder()); } - private function assertValueArrayHasKeys($array, array $requiredKeys, string $path): void + /** + * @param array $supported + */ + private function assertArrayStructure(mixed $array, array $supported, string $path = ''): void + { + if (! \is_array($array)) { + throw new InputProcessorException(implode('', $this->structureBuilder->getCurrentPath()) . $path, \sprintf('Expected structure to be an array, got "%s" instead.', \gettype($array))); + } + + foreach ($array as $key => $value) { + if (! \is_string($key)) { + throw new InputProcessorException(implode('', $this->structureBuilder->getCurrentPath()) . $path, 'Expected structure to be an array.'); + } + + if (! isset($supported[$key])) { + throw new InputProcessorException(implode('', $this->structureBuilder->getCurrentPath()) . $path, \sprintf('Unexpected key "%s" found in structure, expected only: "%s".', $key, implode('", "', array_keys($supported)))); + } + + $type = $supported[$key]; + + if ($type === 'any') { + continue; + } + + if ($type === 'list' && (! \is_array($value) || ! array_is_list($value))) { + throw new InputProcessorException(implode('', $this->structureBuilder->getCurrentPath()) . $path . "[{$key}]", \sprintf('Expected structure to be an array, got "%s" instead.', ! \is_array($value) ? \gettype($value) : 'object')); + } + + if ($type === 'array' && ! \is_array($value)) { + throw new InputProcessorException(implode('', $this->structureBuilder->getCurrentPath()) . $path . "[{$key}]", \sprintf('Expected structure to be an object, got "%s" instead.', \gettype($value))); + } + + if ($type === 'string' && ! \is_string($value)) { + throw new InputProcessorException(implode('', $this->structureBuilder->getCurrentPath()) . $path . "[{$key}]", \sprintf('Expected value to be a string, got "%s" instead.', \gettype($value))); + } + + if ($type === 'bool' && ! \is_bool($value)) { + throw new InputProcessorException(implode('', $this->structureBuilder->getCurrentPath()) . $path . "[{$key}]", \sprintf('Expected value to be a bool, got "%s" instead.', \gettype($value))); + } + + if (\is_array($type) && ! \in_array($value, $type, true)) { + throw new InputProcessorException(implode('', $this->structureBuilder->getCurrentPath()) . $path . "[{$key}]", \sprintf('Expected value to be one of: "%s". got "%s" instead.', implode('", "', $type), \gettype($value))); + } + } + } + + /** + * @param string[] $requiredKeys + */ + private function assertValueArrayHasKeys(mixed $array, array $requiredKeys, string $path): void { if (! \is_array($array)) { throw new InputProcessorException(implode('', $this->structureBuilder->getCurrentPath()) . $path, \sprintf('Expected value-structure to be an array, got "%s" instead.', \gettype($array))); @@ -265,10 +328,10 @@ private function assertValueArrayHasKeys($array, array $requiredKeys, string $pa throw new InputProcessorException( implode('', $this->structureBuilder->getCurrentPath()) . $path, \sprintf( - 'Expected value-structure to contain the following keys: %s. ' . - 'But the following keys are missing: %s.', - implode(', ', $requiredKeys), - implode(', ', $missingKeys) + 'Expected value-structure to contain the following keys: "%s". ' . + 'But the following keys are missing: "%s".', + implode('", "', $requiredKeys), + implode('", "', $missingKeys) ) ); } diff --git a/lib/Core/Tests/Input/JsonInputTest.php b/lib/Core/Tests/Input/JsonInputTest.php index 3e8aff70..6d75a9f9 100644 --- a/lib/Core/Tests/Input/JsonInputTest.php +++ b/lib/Core/Tests/Input/JsonInputTest.php @@ -14,6 +14,7 @@ namespace Rollerworks\Component\Search\Tests\Input; use Rollerworks\Component\Search\ConditionErrorMessage; +use Rollerworks\Component\Search\Exception\InvalidSearchConditionException; use Rollerworks\Component\Search\Input\JsonInput; use Rollerworks\Component\Search\Input\ProcessorConfig; use Rollerworks\Component\Search\InputProcessor; @@ -46,6 +47,138 @@ public function it_errors_on_invalid_json(): void $this->assertConditionContainsErrorsWithoutCause('{]', $config, [$error]); } + /** + * @test + * + * @dataProvider provide_invalid_structures + */ + public function it_validates_structure(array $structure, array $errors): void + { + $this->assertConditionContainsErrorsWithoutCause( + json_encode($structure, \JSON_THROW_ON_ERROR), + new ProcessorConfig($this->getFieldSet(order: true)), + $errors + ); + } + + public static function provide_invalid_structures(): iterable + { + yield 'Unknown key at root level' => [ + [ + 'date' => [], + ], + [new ConditionErrorMessage('', 'Unexpected key "date" found in structure, expected only: "fields", "groups", "order", "logical-case".')], + ]; + + yield 'Unknown key at field level' => [ + [ + 'fields' => [ + 'date' => [ + 'values' => ['1'], + ], + ], + ], + [new ConditionErrorMessage('[fields][date]', 'Unexpected key "values" found in structure, expected only: "simple-values", "excluded-simple-values", "ranges", "excluded-ranges", "comparisons", "pattern-matchers".')], + ]; + + yield 'Wrong key at field level' => [ + [ + 'fields' => [ + 'date' => [ + '1', + ], + ], + ], + [new ConditionErrorMessage('[fields][date]', 'Expected structure to be an array.')], + ]; + + yield 'Unknown key at nested level' => [ + [ + 'groups' => [ + 'date' => [], + ], + ], + [new ConditionErrorMessage('[groups]', 'Expected structure to be an array, got "object" instead.')], + ]; + + yield 'Unexpected value for single-values' => [ + [ + 'fields' => [ + 'date' => [ + 'simple-values' => ['first' => '2014-12-16'], + ], + ], + ], + [new ConditionErrorMessage('[fields][date][simple-values]', 'Expected structure to be an array, got "object" instead.')], + ]; + + yield 'Unexpected value for excluded-single-values' => [ + [ + 'fields' => [ + 'date' => [ + 'excluded-simple-values' => ['first' => '2014-12-16'], + ], + ], + ], + [new ConditionErrorMessage('[fields][date][excluded-simple-values]', 'Expected structure to be an array, got "object" instead.')], + ]; + + yield 'Unexpected value for ranges' => [ + [ + 'fields' => [ + 'date' => [ + 'ranges' => [['lower' => '2014-12-16', 'upper' => '2014-12-18', 'bounded' => 'first']], + ], + ], + ], + [new ConditionErrorMessage('[fields][date][ranges][0]', 'Unexpected key "bounded" found in structure, expected only: "lower", "upper", "inclusive-lower", "inclusive-upper".')], + ]; + + yield 'Unexpected bound-type for ranges' => [ + [ + 'fields' => [ + 'date' => [ + 'ranges' => [['lower' => '2014-12-16', 'upper' => '2014-12-18', 'inclusive-lower' => 'no']], + ], + ], + ], + [new ConditionErrorMessage('[fields][date][ranges][0][inclusive-lower]', 'Expected value to be a bool, got "string" instead.')], + ]; + + yield 'Missing lower-bound for ranges' => [ + [ + 'fields' => [ + 'date' => [ + 'ranges' => [['upper' => '2014-12-18']], + ], + ], + ], + [new ConditionErrorMessage('[fields][date][ranges][0]', 'Expected value-structure to contain the following keys: "lower", "upper". But the following keys are missing: "lower".')], + ]; + + yield 'Missing type for comparison' => [ + [ + 'fields' => [ + 'date' => [ + 'comparisons' => [['upper' => '2014-12-18']], + ], + ], + ], + [new ConditionErrorMessage('[fields][date][comparisons][0]', 'Expected value-structure to contain the following keys: "value", "operator". But the following keys are missing: "value", "operator".')], + ]; + + yield 'Missing type for pattern-matchers' => [ + [ + 'fields' => [ + 'name' => [ + 'pattern-matchers' => [['value' => '2014-12-18']], + ], + ], + ], + [new ConditionErrorMessage('[fields][name][pattern-matchers][0]', 'Expected value-structure to contain the following keys: "value", "type". But the following keys are missing: "type".')], + ]; + } + public static function provideEmptyInputTests(): iterable { return [ @@ -768,4 +901,23 @@ public static function provideNestedErrorsTests(): iterable ], ]; } + + protected function onNotSuccessfulTest(\Throwable $t): never + { + if ($t instanceof InvalidSearchConditionException) { + echo 'Error: ' . $t->getMessage() . \PHP_EOL; + + foreach ($t->getErrors() as $error) { + echo \sprintf('- %s: %s', $error->path, $error->message); + + if ($error->cause instanceof \Exception) { + echo ' Cause: ' . $error->cause->getMessage(); + } + + echo \PHP_EOL; + } + } + + parent::onNotSuccessfulTest($t); + } } diff --git a/lib/Symfony/SearchBundle/Tests/Functional/ApiPlatformTest.php b/lib/Symfony/SearchBundle/Tests/Functional/ApiPlatformTest.php index ab8e8c64..72896940 100644 --- a/lib/Symfony/SearchBundle/Tests/Functional/ApiPlatformTest.php +++ b/lib/Symfony/SearchBundle/Tests/Functional/ApiPlatformTest.php @@ -78,7 +78,7 @@ public function with_valid_condition_json(): void $client->request( 'GET', '/books.json', - ['search' => '{"fields":{"title":{"single-values":["Symfony;"]}}}'] + ['search' => '{"fields":{"title":{"simple-values":["Symfony;"]}}}'] ); self::assertFalse($client->getResponse()->isRedirection());