From 85684abdec9d70ab79fd62e8e1b43ea48092cea4 Mon Sep 17 00:00:00 2001 From: Ruben van der Linde Date: Fri, 16 Oct 2026 21:44:17 +0200 Subject: [PATCH] feat(search): filter on a field inside an object property `?dunning.currentStage=2` (and `['dunning.currentStage' => 2]` from PHP) answered an empty page: the request layer rebuilds the mangled key into `['dunning' => ['currentStage' => 2]]`, which applyObjectFilters read as a list of UUIDs, and a dotted key was an unknown property that added `1 = 0`. NestedPathFilter takes the filters on a path inside an object-typed property out of the filter map, in both spellings and at any depth, and writes their SQL against the JSON column (`#>>` on PostgreSQL, JSON_EXTRACT on MariaDB and MySQL): equality, a list, in, notIn, ne, isnull, like, and gt/gte/lt/lte, numeric when the operand is a number. The QueryBuilder path and the raw UNION path both use it. A segment that is not a plain name matches nothing; an encrypted object property is refused as its column filter is. A filter on the object property itself keeps its meaning. Assisted-by: ClaudeCode:claude-opus-5-5 --- lib/Db/MagicMapper/MagicSearchHandler.php | 77 ++++ lib/Db/MagicMapper/NestedPathFilter.php | 370 ++++++++++++++++++ .../nested-property-filters/proposal.md | 51 +++ .../specs/zoeken-filteren/spec.md | 34 ++ .../changes/nested-property-filters/tasks.md | 6 + ...MagicSearchHandlerNestedPathFilterTest.php | 306 +++++++++++++++ 6 files changed, 844 insertions(+) create mode 100644 lib/Db/MagicMapper/NestedPathFilter.php create mode 100644 openspec/changes/nested-property-filters/proposal.md create mode 100644 openspec/changes/nested-property-filters/specs/zoeken-filteren/spec.md create mode 100644 openspec/changes/nested-property-filters/tasks.md create mode 100644 tests/Unit/Db/MagicSearchHandlerNestedPathFilterTest.php diff --git a/lib/Db/MagicMapper/MagicSearchHandler.php b/lib/Db/MagicMapper/MagicSearchHandler.php index e29fee607b..147a4dbac9 100644 --- a/lib/Db/MagicMapper/MagicSearchHandler.php +++ b/lib/Db/MagicMapper/MagicSearchHandler.php @@ -204,6 +204,13 @@ class MagicSearchHandler { */ private SearchTermSqlCompiler $termCompiler; + /** + * Reads filters on a path inside an object property and writes their SQL. + * + * @var NestedPathFilter + */ + private NestedPathFilter $nestedPaths; + /** * Constructor for MagicSearchHandler * @@ -234,6 +241,7 @@ public function __construct( ) { $this->termParser = new SearchTermParser(); $this->termCompiler = new SearchTermSqlCompiler(); + $this->nestedPaths = new NestedPathFilter(); }//end __construct() /** @@ -1728,6 +1736,25 @@ private function buildObjectFilterConditionsSql( $reservedParams = $this->getReservedParams(); $properties = $schema->getProperties() ?? []; + [$query, $nestedFilters] = $this->nestedPaths->split(filters: $query, properties: $properties); + foreach ($nestedFilters as $filter) { + $expression = $this->nestedPaths->expression( + column: $this->quoteIdentifier(name: $this->sanitizeColumnName(name: $filter['root']), isPostgres: $isPostgres), + path: $filter['path'], + isPostgres: $isPostgres + ); + if ($expression === null) { + $conditions[] = '1=0'; + continue; + } + + $quote = static fn (string $value): string => (string)$connection->quote($value); + array_push( + $conditions, + ...$this->nestedPaths->conditions(expression: $expression, value: $filter['value'], bind: $quote, isPostgres: $isPostgres) + ); + } + foreach ($query as $key => $value) { // Skip reserved params, underscore-prefixed params, and @ metadata params. if (in_array($key, $reservedParams, true) === true @@ -2753,6 +2780,9 @@ private function applyObjectFilters(IQueryBuilder $qb, array $filters, Schema $s $isPostgres = $this->isPostgresPlatform(); + [$filters, $nestedFilters] = $this->nestedPaths->split(filters: $filters, properties: $properties ?? []); + $this->applyNestedPathFilters(qb: $qb, nestedFilters: $nestedFilters, properties: $properties ?? [], isPostgres: $isPostgres); + foreach ($filters as $field => $value) { // Check if this field exists as a column in the schema. if (($properties[$field] ?? null) === null) { @@ -2986,6 +3016,53 @@ private function applyJsonArrayFilter(IQueryBuilder $qb, string $columnName, mix $qb->andWhere($orConditions); }//end applyJsonArrayFilter() + /** + * Filter on fields inside object properties (`dunning.currentStage=2`), on the QueryBuilder path. + * + * A path the JSON column cannot be read at (a segment that is not a plain + * name) matches nothing and is reported as an ignored filter, the way an + * unknown property is. + * + * @param IQueryBuilder $qb Query builder to modify. + * @param list, value: mixed}> $nestedFilters The path filters from $this->nestedPaths->split(). + * @param array $properties The schema's properties. + * @param bool $isPostgres Whether the platform is PostgreSQL. + * + * @return void + * + * @throws EncryptedFieldFilterException When the object property is encrypted. + * + * @spec openspec/changes/nested-property-filters/specs/zoeken-filteren/spec.md#requirement-a-filter-can-name-a-field-inside-an-object-property + */ + private function applyNestedPathFilters(IQueryBuilder $qb, array $nestedFilters, array $properties, bool $isPostgres): void { + foreach ($nestedFilters as $filter) { + if (($properties[$filter['root']]['x-openregister-encrypted'] ?? false) === true) { + throw new EncryptedFieldFilterException(property: $filter['root']); + } + + $expression = $this->nestedPaths->expression( + column: 't.' . $this->sanitizeColumnName(name: $filter['root']), + path: $filter['path'], + isPostgres: $isPostgres + ); + if ($expression === null) { + $this->ignoredFilters[] = $filter['root'] . '.' . implode('.', $filter['path']); + $qb->andWhere('1 = 0'); + continue; + } + + $conditions = $this->nestedPaths->conditions( + expression: $expression, + value: $filter['value'], + bind: static fn (string $value): string => (string)$qb->createNamedParameter($value), + isPostgres: $isPostgres + ); + foreach ($conditions as $condition) { + $qb->andWhere($condition); + } + } + }//end applyNestedPathFilters() + /** * Apply filter for object columns (related objects) * diff --git a/lib/Db/MagicMapper/NestedPathFilter.php b/lib/Db/MagicMapper/NestedPathFilter.php new file mode 100644 index 0000000000..3b4e27a6f1 --- /dev/null +++ b/lib/Db/MagicMapper/NestedPathFilter.php @@ -0,0 +1,370 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://OpenRegister.app + */ + +declare(strict_types=1); + +namespace OCA\OpenRegister\Db\MagicMapper; + +use OCA\OpenRegister\Db\LikeOperator; + +/** + * Reads nested-path filters out of a filter map and writes their SQL. + * + * A filter reaches the magic table in one of two spellings: + * + * - a dotted key, `['dunning.currentStage' => 2]`, as a PHP caller passes it; + * - a nested map, `['dunning' => ['currentStage' => 2]]`, which is what the + * request layer rebuilds from `?dunning.currentStage=2` (PHP mangles the dot + * to `dunning_currentStage`, and SearchQueryHandler splits it back). + * + * Both name a root property of type `object` and a path inside its JSON + * column. The value is compared in its text form; a comparison operator with a + * numeric operand compares numerically. An object property's column holds JSON + * (plain `json` on PostgreSQL, JSON on MariaDB/MySQL), read with `#>>` and + * `JSON_EXTRACT` respectively. + * + * @spec openspec/changes/nested-property-filters/specs/zoeken-filteren/spec.md#requirement-a-filter-can-name-a-field-inside-an-object-property + */ +class NestedPathFilter { + + /** + * Operator keys of an operator bag. + */ + public const OPERATORS = ['gte', 'lte', 'gt', 'lt', 'in', 'notIn', 'ne', 'isnull', LikeOperator::KEY]; + + /** + * A path segment: a property name, nothing that could leave the JSON path. + */ + private const SEGMENT = '/^[A-Za-z0-9_\-]+$/'; + + /** + * Split a filter map into the filters on columns and the filters on a path inside an object property. + * + * @param array $filters The object filters. + * @param array $properties The schema's properties. + * + * @return array{0: array, 1: list, value: mixed}>} The column filters and the path filters. + * + * @spec openspec/changes/nested-property-filters/specs/zoeken-filteren/spec.md#requirement-a-filter-can-name-a-field-inside-an-object-property + */ + public function split(array $filters, array $properties): array { + $plain = []; + $nested = []; + foreach ($filters as $key => $value) { + $key = (string)$key; + $segments = explode('.', $key); + $root = array_shift($segments); + + if ($segments !== [] && self::isObjectProperty(properties: $properties, name: $root) === true) { + self::collect(root: $root, path: $segments, value: $value, into: $nested); + continue; + } + + if ($segments === [] + && self::isObjectProperty(properties: $properties, name: $root) === true + && self::isPathMap(value: $value) === true + ) { + foreach ($value as $segment => $inner) { + self::collect(root: $root, path: [(string)$segment], value: $inner, into: $nested); + } + + continue; + } + + $plain[$key] = $value; + }//end foreach + + return [$plain, $nested]; + }//end split() + + /** + * The SQL expression reading the text at a path inside an object column, or null when a segment is not a plain name. + * + * @param string $column The quoted, qualified column. + * @param list $path The segments below the column. + * @param bool $isPostgres Whether the platform is PostgreSQL. + * + * @return string|null The expression. + * + * @spec openspec/changes/nested-property-filters/specs/zoeken-filteren/spec.md#requirement-a-filter-can-name-a-field-inside-an-object-property + */ + public function expression(string $column, array $path, bool $isPostgres): ?string { + if ($path === []) { + return null; + } + + foreach ($path as $segment) { + if (preg_match(self::SEGMENT, $segment) !== 1) { + return null; + } + } + + if ($isPostgres === true) { + return "({$column} #>> '{" . implode(',', $path) . "}')"; + } + + return "JSON_UNQUOTE(JSON_EXTRACT({$column}, '$." . implode('.', $path) . "'))"; + }//end expression() + + /** + * The SQL conditions of one path filter. + * + * `$bind` turns a value into its SQL form: a named parameter on the + * QueryBuilder path, a quoted literal on the raw UNION path. + * + * @param string $expression The expression from expression(). + * @param mixed $value The filter value: a scalar, a list, or an operator bag. + * @param callable $bind Value to SQL: fn(string $value): string. + * @param bool $isPostgres Whether the platform is PostgreSQL. + * + * @return list The conditions, all of which must hold. + * + * @spec openspec/changes/nested-property-filters/specs/zoeken-filteren/spec.md#requirement-a-filter-can-name-a-field-inside-an-object-property + */ + public function conditions(string $expression, mixed $value, callable $bind, bool $isPostgres): array { + if ($value === 'IS NULL' || $value === null) { + return ["{$expression} IS NULL"]; + } + + if ($value === 'IS NOT NULL') { + return ["{$expression} IS NOT NULL"]; + } + + if (is_array($value) === false) { + return ["{$expression} = " . $bind(self::text(value: $value))]; + } + + if (self::isOperatorBag(value: $value) === false) { + return [self::inList(expression: $expression, values: $value, bind: $bind, negate: false)]; + } + + return [ + ...self::rangeConditions(expression: $expression, value: $value, bind: $bind, isPostgres: $isPostgres), + ...self::setConditions(expression: $expression, value: $value, bind: $bind), + ...self::presenceConditions(expression: $expression, value: $value, bind: $bind), + ]; + + }//end conditions() + + /** + * The comparisons of an operator bag: gt, gte, lt and lte. + * + * @param string $expression The expression. + * @param array $value The operator bag. + * @param callable $bind Value to SQL. + * @param bool $isPostgres Whether the platform is PostgreSQL. + * + * @return list The conditions. + */ + private static function rangeConditions(string $expression, array $value, callable $bind, bool $isPostgres): array { + $conditions = []; + foreach (['gte' => '>=', 'lte' => '<=', 'gt' => '>', 'lt' => '<'] as $operator => $sql) { + if (isset($value[$operator]) === true) { + $conditions[] = self::compare(expression: $expression, sql: $sql, operand: $value[$operator], bind: $bind, isPostgres: $isPostgres); + } + } + + return $conditions; + }//end rangeConditions() + + /** + * The set operators of an operator bag: ne, in and notIn. + * + * @param string $expression The expression. + * @param array $value The operator bag. + * @param callable $bind Value to SQL. + * + * @return list The conditions. + */ + private static function setConditions(string $expression, array $value, callable $bind): array { + $conditions = []; + if (isset($value['ne']) === true) { + $conditions[] = "{$expression} <> " . $bind(self::text(value: $value['ne'])); + } + + if (isset($value['in']) === true) { + $conditions[] = self::inList(expression: $expression, values: (array)$value['in'], bind: $bind, negate: false); + } + + $notIn = (array)($value['notIn'] ?? []); + if ($notIn !== []) { + $conditions[] = self::inList(expression: $expression, values: $notIn, bind: $bind, negate: true); + } + + return $conditions; + }//end setConditions() + + /** + * The presence and text operators of an operator bag: isnull and like. + * + * @param string $expression The expression. + * @param array $value The operator bag. + * @param callable $bind Value to SQL. + * + * @return list The conditions. + */ + private static function presenceConditions(string $expression, array $value, callable $bind): array { + $conditions = []; + if (array_key_exists('isnull', $value) === true) { + $predicate = 'IS NOT NULL'; + if (filter_var($value['isnull'], FILTER_VALIDATE_BOOLEAN) === true) { + $predicate = 'IS NULL'; + } + + $conditions[] = "{$expression} {$predicate}"; + } + + $term = self::text(value: ($value[LikeOperator::KEY] ?? '')); + if ($term !== '') { + $conditions[] = "LOWER({$expression}) LIKE " . $bind('%' . addcslashes(strtolower($term), '\\%_') . '%'); + } + + return $conditions; + }//end presenceConditions() + + /** + * Add one path filter, descending into a nested map that is not an operator bag. + * + * @param string $root The object property. + * @param list $path The segments so far. + * @param mixed $value The value at that path. + * @param list, value: mixed}> $into The collected path filters. + * + * @return void + */ + private static function collect(string $root, array $path, mixed $value, array &$into): void { + if (self::isPathMap(value: $value) === true) { + foreach ($value as $segment => $inner) { + self::collect(root: $root, path: [...$path, (string)$segment], value: $inner, into: $into); + } + + return; + } + + $into[] = ['root' => $root, 'path' => $path, 'value' => $value]; + }//end collect() + + /** + * Whether a property is declared with type object. + * + * @param array $properties The schema's properties. + * @param string $name The property. + * + * @return bool True for an object property. + */ + private static function isObjectProperty(array $properties, string $name): bool { + return is_array($properties[$name] ?? null) === true && ($properties[$name]['type'] ?? null) === 'object'; + }//end isObjectProperty() + + /** + * Whether a value is a map of field names: non-empty, string keys, and no operator key. + * + * @param mixed $value The value. + * + * @return bool True for a nested path map. + */ + private static function isPathMap(mixed $value): bool { + if (is_array($value) === false || $value === [] || array_is_list($value) === true) { + return false; + } + + return self::isOperatorBag(value: $value) === false; + }//end isPathMap() + + /** + * Whether a value is an operator bag. + * + * @param array $value The value. + * + * @return bool True when one of its keys is an operator. + */ + private static function isOperatorBag(array $value): bool { + return array_intersect(array_map('strval', array_keys($value)), self::OPERATORS) !== []; + }//end isOperatorBag() + + /** + * A comparison, numeric when the operand is a number. + * + * @param string $expression The expression. + * @param string $sql The SQL operator. + * @param mixed $operand The operand. + * @param callable $bind Value to SQL. + * @param bool $isPostgres Whether the platform is PostgreSQL. + * + * @return string The condition. + */ + private static function compare(string $expression, string $sql, mixed $operand, callable $bind, bool $isPostgres): string { + if (is_numeric($operand) === false) { + return "{$expression} {$sql} " . $bind(self::text(value: $operand)); + } + + $numeric = "CAST({$expression} AS DECIMAL(30,10))"; + if ($isPostgres === true) { + // A non-numeric text would abort the cast, so only numeric text is compared. + $numeric = "(CASE WHEN {$expression} ~ '^-?[0-9]+(\\.[0-9]+)?$' THEN CAST({$expression} AS NUMERIC) END)"; + } + + return "{$numeric} {$sql} " . $bind((string)$operand); + }//end compare() + + /** + * An IN or NOT IN list over text forms. + * + * @param string $expression The expression. + * @param array $values The values. + * @param callable $bind Value to SQL. + * @param bool $negate NOT IN when true. + * + * @return string The condition. + */ + private static function inList(string $expression, array $values, callable $bind, bool $negate): string { + if ($values === []) { + return '1=0'; + } + + $bound = array_map(static fn (mixed $item): string => $bind(self::text(value: $item)), array_values($values)); + if ($negate === true) { + return "{$expression} NOT IN (" . implode(', ', $bound) . ')'; + } + + return "{$expression} IN (" . implode(', ', $bound) . ')'; + }//end inList() + + /** + * A value's text form as JSON text extraction yields it: a boolean as true or false. + * + * @param mixed $value The value. + * + * @return string The text. + */ + private static function text(mixed $value): string { + if (is_bool($value) === true) { + return var_export($value, true); + } + + if (is_scalar($value) === false) { + return ''; + } + + return (string)$value; + }//end text() +}//end class diff --git a/openspec/changes/nested-property-filters/proposal.md b/openspec/changes/nested-property-filters/proposal.md new file mode 100644 index 0000000000..5b1eea5de3 --- /dev/null +++ b/openspec/changes/nested-property-filters/proposal.md @@ -0,0 +1,51 @@ +--- +kind: code +depends_on: [] +--- + +# Proposal: nested-property-filters + +## Why + +A filter cannot name a field inside an object property. shillinq keeps an +invoice's reminder state in an object property, `ARInvoice.dunning` +(`currentStage`, `nextDunningDate`, ...), and its receivables board counts the +invoices in a reminder round with `?dunning.currentStage[isnull]=false`. That +count is always 0. + +Measured on 2026-10-16 on the dev instance (PostgreSQL, openregister +`development` at b08b9fe188), over three ARInvoice objects whose `dunning` is +empty, as user pieter: + +| query | total | +|---|---| +| `_limit=1` | 1 | +| `dunning.currentStage[isnull]=true` | 0 | +| `dunning.currentStage[isnull]=false` | 0 | + +The first filter should match the invoice and the second should not. Neither +does. + +## Cause, in the code + +PHP turns `dunning.currentStage` into `dunning_currentStage`, and +`SearchQueryHandler::buildSearchQuery()` splits that back into +`['dunning' => ['currentStage' => ...]]`. `MagicSearchHandler::applyObjectFilters()` +then reads the value of an object property as a list of UUIDs +(`applyJsonObjectFilter`). A PHP caller passing `['dunning.currentStage' => 2]` +gets an unknown property instead, which adds `1 = 0`. The raw UNION path +(`buildObjectFilterConditionsSql`) behaves the same way. + +## What Changes + +- `NestedPathFilter` takes the filters on a path inside an object property out of the filter map, in both spellings: a dotted key, or the nested map the request layer rebuilds. A path can be any depth. +- The path is read from the JSON column: `#>>` on PostgreSQL, `JSON_UNQUOTE(JSON_EXTRACT())` on MariaDB and MySQL. +- The comparison uses the text form: equality, a list, `in`, `notIn`, `ne`, `isnull` and `like`. `gt`, `gte`, `lt` and `lte` compare as numbers when the operand is numeric. +- Both the QueryBuilder path and the raw UNION path use it. +- A path segment that is not a plain name matches nothing and is reported as an ignored filter. An encrypted object property is refused, the same as its column filter. +- A filter on the object property itself keeps its current meaning: a UUID, a list, or an operator bag. + +## Impact + +- Affected code: `lib/Db/MagicMapper/NestedPathFilter.php` (new), `lib/Db/MagicMapper/MagicSearchHandler.php`. +- No schema, migration or API shape changes. A query that answered an empty page now answers the matching objects. diff --git a/openspec/changes/nested-property-filters/specs/zoeken-filteren/spec.md b/openspec/changes/nested-property-filters/specs/zoeken-filteren/spec.md new file mode 100644 index 0000000000..c22d8496f5 --- /dev/null +++ b/openspec/changes/nested-property-filters/specs/zoeken-filteren/spec.md @@ -0,0 +1,34 @@ +# zoeken-filteren + +## ADDED Requirements + +### Requirement: A filter can name a field inside an object property + +A property filter SHALL accept a path into an object-typed property, written +as a dotted key (`dunning.currentStage=2`) or as the nested map the request +layer rebuilds from it (`['dunning' => ['currentStage' => 2]]`). It SHALL +compare the value at that path on PostgreSQL, MySQL and MariaDB, for the list +and the count, with equality, a list of values, `in`, `notIn`, `ne`, `isnull`, +`like` and the comparisons `gt`, `gte`, `lt` and `lte`. The comparisons SHALL +be numeric when the operand is a number. A boolean value SHALL compare in its +JSON spelling. A path segment that is not a plain property name SHALL match +nothing. A path into an encrypted property SHALL be refused, the same as a +filter on that property. A filter on the object property itself SHALL keep +its meaning. + +#### Scenario: invoices in a reminder round + +- **GIVEN** an ARInvoice schema whose object property `dunning` holds `currentStage`, two invoices at stage 1 and 2, and one without a stage +- **WHEN** a client lists `ARInvoice?dunning.currentStage[isnull]=false` +- **THEN** the two invoices with a stage are returned, and the total is 2 + +#### Scenario: the same filter from PHP + +- **GIVEN** the same invoices +- **WHEN** a PHP caller searches with `['dunning.currentStage' => 2]` +- **THEN** the invoice at stage 2 is returned + +#### Scenario: a path segment that is not a name + +- **WHEN** a client filters on `dunning.current'Stage=1` +- **THEN** nothing matches and the filter is reported as ignored diff --git a/openspec/changes/nested-property-filters/tasks.md b/openspec/changes/nested-property-filters/tasks.md new file mode 100644 index 0000000000..cd208d82ed --- /dev/null +++ b/openspec/changes/nested-property-filters/tasks.md @@ -0,0 +1,6 @@ +# Tasks: nested-property-filters + +## 1. Filter + +- [x] 1.1 `NestedPathFilter`: split the path filters out of a filter map in both spellings, read the path per platform, write the conditions per operator. Verify: PHPUnit `MagicSearchHandlerNestedPathFilterTest`. +- [x] 1.2 Apply them on the QueryBuilder path (`applyObjectFilters`) and the raw UNION path (`buildObjectFilterConditionsSql`), refusing an encrypted root. Verify: the same test, both paths; a live list on PostgreSQL with `dunning.currentStage[isnull]=true|false`. diff --git a/tests/Unit/Db/MagicSearchHandlerNestedPathFilterTest.php b/tests/Unit/Db/MagicSearchHandlerNestedPathFilterTest.php new file mode 100644 index 0000000000..1d9a24c27a --- /dev/null +++ b/tests/Unit/Db/MagicSearchHandlerNestedPathFilterTest.php @@ -0,0 +1,306 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://OpenRegister.app + * + * @spec openspec/changes/nested-property-filters/specs/zoeken-filteren/spec.md#requirement-a-filter-can-name-a-field-inside-an-object-property + */ + +declare(strict_types=1); + +namespace OCA\OpenRegister\Tests\Unit\Db; + +use Doctrine\DBAL\Platforms\PostgreSQL120Platform; +use OCA\OpenRegister\Db\MagicMapper\MagicOrganizationHandler; +use OCA\OpenRegister\Db\MagicMapper\MagicRbacHandler; +use OCA\OpenRegister\Db\MagicMapper\MagicSearchHandler; +use OCA\OpenRegister\Db\MagicMapper\NestedPathFilter; +use OCA\OpenRegister\Db\Schema; +use OCA\OpenRegister\Exception\EncryptedFieldFilterException; +use OCA\OpenRegister\Service\DateTimeNormalizer; +use OCA\OpenRegister\Service\Object\SchemaTypeConverter; +use OCA\OpenRegister\Service\Query\RelatedRowQueryApplier; +use OCP\DB\QueryBuilder\IExpressionBuilder; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; +use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; +use ReflectionMethod; + +/** + * Nested-path filters on the QueryBuilder path, the raw UNION path and in the helper. + */ +class MagicSearchHandlerNestedPathFilterTest extends TestCase { + + /** + * The ARInvoice shape the filters are written for. + * + * @var array + */ + private const PROPERTIES = [ + 'invoiceNumber' => ['type' => 'string'], + 'dunning' => [ + 'type' => 'object', + 'properties' => [ + 'currentStage' => ['type' => 'integer'], + 'paused' => ['type' => 'boolean'], + 'nextDunningDate' => ['type' => 'string'], + ], + ], + 'customer' => ['type' => 'object'], + ]; + + /** + * WHERE fragments captured from the QueryBuilder path. + * + * @var string[] + */ + private array $captured = []; + + /** + * The handler on a PostgreSQL connection double. + * + * @return MagicSearchHandler + */ + private function handler(): MagicSearchHandler { + $db = $this->createMock(IDBConnection::class); + $db->method('getDatabasePlatform')->willReturn($this->createMock(PostgreSQL120Platform::class)); + $logger = $this->createMock(LoggerInterface::class); + + return new MagicSearchHandler( + db: $db, + logger: $logger, + rbacHandler: $this->createMock(MagicRbacHandler::class), + organizationHandler: $this->createMock(MagicOrganizationHandler::class), + schemaTypeConverter: new SchemaTypeConverter(), + dateTimeNormalizer: new DateTimeNormalizer($logger), + relatedRows: $this->createMock(RelatedRowQueryApplier::class) + ); + }//end handler() + + /** + * A QueryBuilder double whose named parameters are the bare value and whose andWhere records the predicate. + * + * @return IQueryBuilder + */ + private function queryBuilder(): IQueryBuilder { + $expr = $this->createMock(IExpressionBuilder::class); + foreach (['eq', 'neq', 'lt', 'lte', 'gt', 'gte', 'in', 'notIn'] as $operator) { + $expr->method($operator)->willReturnCallback( + static function (string $column, $value) use ($operator): string { + if (is_array($value) === true) { + $value = implode('|', $value); + } + + return "{$operator}({$column},{$value})"; + } + ); + } + + $expr->method('isNull')->willReturnCallback(static fn (string $c): string => "isNull({$c})"); + $expr->method('isNotNull')->willReturnCallback(static fn (string $c): string => "isNotNull({$c})"); + + $qb = $this->createMock(IQueryBuilder::class); + $qb->method('expr')->willReturn($expr); + $qb->method('createNamedParameter')->willReturnCallback(static fn ($value) => is_array($value) ? $value : ":{$value}"); + $qb->method('andWhere')->willReturnCallback( + function (...$predicates) use ($qb) { + foreach ($predicates as $predicate) { + $this->captured[] = (string)$predicate; + } + + return $qb; + } + ); + + return $qb; + }//end queryBuilder() + + /** + * Run applyObjectFilters() and return what it added. + * + * @param array $filters Object filters. + * @param array $properties Schema properties. + * + * @return string[] + */ + private function applyObject(array $filters, array $properties = self::PROPERTIES): array { + $schema = $this->createMock(Schema::class); + $schema->method('getProperties')->willReturn($properties); + $this->captured = []; + + $method = new ReflectionMethod(MagicSearchHandler::class, 'applyObjectFilters'); + $method->invoke($this->handler(), $this->queryBuilder(), $filters, $schema); + + return $this->captured; + }//end applyObject() + + /** + * Run the raw-SQL object builder and return its conditions. + * + * @param array $query Search query. + * + * @return string[] + */ + private function rawObject(array $query): array { + $schema = $this->createMock(Schema::class); + $schema->method('getProperties')->willReturn(self::PROPERTIES); + $connection = $this->createMock(IDBConnection::class); + $connection->method('quote')->willReturnCallback(static fn ($v) => "'{$v}'"); + + $method = new ReflectionMethod(MagicSearchHandler::class, 'buildObjectFilterConditionsSql'); + return $method->invoke($this->handler(), $query, $schema, $connection, true); + }//end rawObject() + + /** + * The dotted key and the nested map the request layer rebuilds read the same path. + * + * @return void + */ + public function testTheDottedAndTheNestedSpellingReadTheSamePath(): void { + $dotted = $this->applyObject(['dunning.currentStage' => 2]); + $nested = $this->applyObject(['dunning' => ['currentStage' => 2]]); + + $this->assertSame(["(t.dunning #>> '{currentStage}') = :2"], $dotted); + $this->assertSame($dotted, $nested); + }//end testTheDottedAndTheNestedSpellingReadTheSamePath() + + /** + * Operators apply inside the path: a numeric bound compares as a number, isnull asks for presence. + * + * @return void + */ + public function testOperatorsApplyInsideThePath(): void { + $captured = $this->applyObject(['dunning' => ['currentStage' => ['gte' => '1', 'isnull' => 'false']]]); + + $this->assertCount(2, $captured); + $this->assertStringContainsString("CAST((t.dunning #>> '{currentStage}') AS NUMERIC)", $captured[0]); + $this->assertStringEndsWith('>= :1', $captured[0]); + $this->assertSame("(t.dunning #>> '{currentStage}') IS NOT NULL", $captured[1]); + }//end testOperatorsApplyInsideThePath() + + /** + * A boolean compares in its JSON spelling, and a list is an IN over the text forms. + * + * @return void + */ + public function testABooleanAndAListCompareAsJsonText(): void { + $this->assertSame(["(t.dunning #>> '{paused}') = :true"], $this->applyObject(['dunning.paused' => true])); + $this->assertSame( + ["(t.dunning #>> '{currentStage}') IN (:1, :2)"], + $this->applyObject(['dunning.currentStage' => ['1', '2']]) + ); + }//end testABooleanAndAListCompareAsJsonText() + + /** + * A filter on an object property that is not a path map keeps its old meaning. + * + * @return void + */ + public function testAnObjectPropertysOwnFilterIsUnchanged(): void { + $captured = $this->applyObject(['customer' => 'a1b2c3d4-0000-4000-8000-000000000001']); + $this->assertCount(1, $captured); + $this->assertStringContainsString('t.customer::text', $captured[0]); + }//end testAnObjectPropertysOwnFilterIsUnchanged() + + /** + * A dot on a property that is not an object, or a segment that is not a plain name, matches nothing. + * + * @return void + */ + public function testAPathThatCannotBeReadMatchesNothing(): void { + $this->assertSame(['1 = 0'], $this->applyObject(['invoiceNumber.x' => 'a'])); + $this->assertSame(['1 = 0'], $this->applyObject(["dunning.current'Stage" => 'a'])); + $this->assertNull((new NestedPathFilter())->expression(column: 't.dunning', path: ["a'}') OR 1=1 --"], isPostgres: true)); + }//end testAPathThatCannotBeReadMatchesNothing() + + /** + * An encrypted object property is refused as its column filter is. + * + * @return void + */ + public function testAPathIntoAnEncryptedPropertyIsRefused(): void { + $properties = self::PROPERTIES; + $properties['dunning']['x-openregister-encrypted'] = true; + + $this->expectException(EncryptedFieldFilterException::class); + $this->applyObject(['dunning.currentStage' => 2], $properties); + }//end testAPathIntoAnEncryptedPropertyIsRefused() + + /** + * The raw UNION path writes the same condition with a quoted literal. + * + * @return void + */ + public function testTheRawUnionPathAgrees(): void { + $this->assertSame(['("dunning" #>> \'{currentStage}\') = \'2\''], $this->rawObject(['dunning' => ['currentStage' => 2]])); + $this->assertSame(['("dunning" #>> \'{paused}\') = \'true\''], $this->rawObject(['dunning.paused' => 'true'])); + }//end testTheRawUnionPathAgrees() + + /** + * MariaDB and MySQL read the path with JSON_EXTRACT. + * + * @return void + */ + public function testMariaDbReadsThePathWithJsonExtract(): void { + $expression = (new NestedPathFilter())->expression(column: 't.dunning', path: ['currentStage'], isPostgres: false); + + $this->assertSame("JSON_UNQUOTE(JSON_EXTRACT(t.dunning, '$.currentStage'))", $expression); + $conditions = (new NestedPathFilter())->conditions( + expression: $expression, + value: ['lt' => 3, 'ne' => '1', 'like' => 'Her'], + bind: static fn (string $v): string => "'{$v}'", + isPostgres: false + ); + $this->assertSame( + [ + "CAST(JSON_UNQUOTE(JSON_EXTRACT(t.dunning, '$.currentStage')) AS DECIMAL(30,10)) < '3'", + "JSON_UNQUOTE(JSON_EXTRACT(t.dunning, '$.currentStage')) <> '1'", + "LOWER(JSON_UNQUOTE(JSON_EXTRACT(t.dunning, '$.currentStage'))) LIKE '%her%'", + ], + $conditions + ); + }//end testMariaDbReadsThePathWithJsonExtract() + + /** + * Deeper paths and several fields of one object split into one filter each. + * + * @return void + */ + public function testSplitCollectsEveryLeaf(): void { + [$plain, $nested] = (new NestedPathFilter())->split( + filters: [ + 'invoiceNumber' => 'VF-1', + 'dunning' => ['currentStage' => 2, 'paused' => 'false'], + 'customer.address.city' => 'Zuiddrecht', + ], + properties: self::PROPERTIES + ); + + $this->assertSame(['invoiceNumber' => 'VF-1'], $plain); + $this->assertSame( + [ + ['root' => 'dunning', 'path' => ['currentStage'], 'value' => 2], + ['root' => 'dunning', 'path' => ['paused'], 'value' => 'false'], + ['root' => 'customer', 'path' => ['address', 'city'], 'value' => 'Zuiddrecht'], + ], + $nested + ); + }//end testSplitCollectsEveryLeaf() +}//end class