diff --git a/src/Database/Validator/Query/Aggregate.php b/src/Database/Validator/Query/Aggregate.php index c64b5ea53..cf197460c 100644 --- a/src/Database/Validator/Query/Aggregate.php +++ b/src/Database/Validator/Query/Aggregate.php @@ -3,7 +3,9 @@ namespace Utopia\Database\Validator\Query; use Utopia\Database\Attribute; +use Utopia\Database\Database; use Utopia\Database\Document; +use Utopia\Database\Filter; use Utopia\Database\Query; use Utopia\Database\Storage; use Utopia\Database\Validator\Query\Joined\Attributes; @@ -69,7 +71,8 @@ class Aggregate extends Base protected array $numeric = []; /** - * The attributes whose values min() and max() cannot order: arrays and the unordered types. + * The attributes whose values min() and max() cannot order: arrays, the unordered types and the internal + * attributes stored as a JSON list, `$permissions`. * * @var array */ @@ -119,6 +122,12 @@ public function __construct(array $attributes = [], protected bool $supportForAt } } + foreach (Database::internalAttributesFor($sharedTables) as $attribute) { + if (\in_array(Filter::Json->value, $attribute->filters, true)) { + $this->unordered[$attribute->key] = true; + } + } + $this->schema += self::internalColumns($sharedTables); $this->columns = Collection::columns($attributes); } @@ -299,12 +308,14 @@ private function isValidExtremum(Query $query): bool $column = $dot === false ? $attribute : \substr($attribute, $dot + 1); $definition = $join?->schema[$column] ?? null; - if ($definition === null) { + if ($definition !== null) { + $type = $definition['type'] ?? null; + $ordered = self::isOrdered($type instanceof ColumnType ? $type : null, (bool) ($definition['array'] ?? false)); + } elseif ($join !== null && $this->isJoinedInternalAttribute($column)) { + $ordered = ! isset($this->unordered[$column]); + } else { return true; } - - $type = $definition['type'] ?? null; - $ordered = self::isOrdered($type instanceof ColumnType ? $type : null, (bool) ($definition['array'] ?? false)); } if (! $ordered) { diff --git a/tests/e2e/Adapter/Scopes/AggregationTests.php b/tests/e2e/Adapter/Scopes/AggregationTests.php index 404a26be0..a3ff2e0aa 100644 --- a/tests/e2e/Adapter/Scopes/AggregationTests.php +++ b/tests/e2e/Adapter/Scopes/AggregationTests.php @@ -1860,6 +1860,69 @@ public function testNumericAggregatesRejectAttributesThatAreNotNumbers(): void $database->deleteCollection($collection); } + /** + * `$permissions` is stored as a JSON list, so min() and max() of it compared the encoded text and answered `[]`, + * where they refuse an array attribute. Each aggregate treats it as the array attribute it is: min(), max() and the + * numeric aggregates are refused, count() and countDistinct() count it. + */ + public function testPermissionsAreAggregatedAsAnArrayAttribute(): void + { + $database = static::getDatabase(); + $collection = 'agg_permissions'.$this->getAggSuffix(); + $database->createCollection(Collection::create( + id: $collection, + attributes: [Attribute::string(key: 'tags', size: 20, array: true)], + permissions: [Permission::create(Role::any()), Permission::read(Role::any())], + documentSecurity: true, + )); + foreach ([[Role::any()], [Role::any()], [Role::any(), Role::users()]] as $roles) { + $database->createDocument($collection, new Document([ + 'tags' => ['a'], + '$permissions' => \array_map(static fn (Role $role): string => Permission::read($role), $roles), + ])); + } + + if (! $database->getAdapter()->supports(Capability::Aggregations)) { + foreach ([Query::min('$permissions', 'result'), Query::max('$permissions', 'result'), Query::count('$permissions', 'rows')] as $query) { + try { + $database->aggregate($collection, [$query]); + $this->fail('An adapter without aggregations refuses an aggregate'); + } catch (QueryException) { + $this->addToAssertionCount(1); + } + } + $database->deleteCollection($collection); + + return; + } + + foreach (['tags', '$permissions'] as $attribute) { + foreach (['min' => Query::min($attribute, 'result'), 'max' => Query::max($attribute, 'result')] as $method => $query) { + $this->assertRejectedAsQueryShape( + fn () => $database->aggregate($collection, [$query]), + 'Invalid query: Aggregate '.$method.' requires an attribute whose values are ordered, not an array, object, boolean, spatial or vector one: '.$attribute, + ); + } + foreach (['sum' => Query::sum($attribute, 'result'), 'avg' => Query::avg($attribute, 'result')] as $method => $query) { + $this->assertRejectedAsQueryShape( + fn () => $database->aggregate($collection, [$query]), + 'Invalid query: Aggregate '.$method.' requires a numeric attribute that is not an array: '.$attribute, + ); + } + } + + $joined = [Query::join($collection, 'other', [Query::on('$id', '$id')])]; + $this->assertRejectedAsQueryShape( + fn () => $database->aggregate($collection, [...$joined, Query::max('other.$permissions', 'result')]), + 'Invalid query: Aggregate max requires an attribute whose values are ordered, not an array, object, boolean, spatial or vector one: other.$permissions', + ); + + $counts = $database->aggregate($collection, [Query::count('$permissions', 'rows'), Query::countDistinct('$permissions', 'sets')]); + $this->assertSame([['rows' => 3, 'sets' => 2]], $counts); + + $database->deleteCollection($collection); + } + public function testEmptySetAggregatesAreZeroForCountsAndNullOtherwise(): void { $database = static::getDatabase(); diff --git a/tests/unit/Validator/QueryValidationTest.php b/tests/unit/Validator/QueryValidationTest.php index dfcea86ea..88f45e5f2 100644 --- a/tests/unit/Validator/QueryValidationTest.php +++ b/tests/unit/Validator/QueryValidationTest.php @@ -281,6 +281,9 @@ public static function extremaWithoutAnOrder(): iterable yield 'max of a joined boolean' => [[$join, Query::max('it.featured', 'most')], 'max', 'it.featured']; yield 'min of a joined array' => [[$join, Query::min('it.labels', 'least')], 'min', 'it.labels']; yield 'max of a bare joined boolean' => [[$join, Query::max('featured', 'most')], 'max', 'featured']; + yield 'max of the permissions list' => [[Query::max('$permissions', 'most')], 'max', '$permissions']; + yield 'min of the permissions list' => [[Query::min('$permissions', 'least')], 'min', '$permissions']; + yield 'max of a joined permissions list' => [[$join, Query::max('it.$permissions', 'most')], 'max', 'it.$permissions']; } /** @@ -337,12 +340,20 @@ public function testTheAggregateValidatorRefusesExtremaOfUnorderedTypes(): void $definition('born', ColumnType::Datetime), ]); - foreach (['meta', 'place', 'route', 'area', 'embedding', 'flag', 'codes'] as $attribute) { + foreach (['meta', 'place', 'route', 'area', 'embedding', 'flag', 'codes', '$permissions'] as $attribute) { foreach ([Query::min($attribute), Query::max($attribute)] as $query) { $this->assertFalse($validator->isValid($query), $attribute); $this->assertStringEndsWith(': '.$attribute, $validator->getDescription()); } $this->assertTrue($validator->isValid(Query::count($attribute)), $attribute.': count needs no order'); + $this->assertTrue($validator->isValid(Query::countDistinct($attribute)), $attribute.': countDistinct needs no order'); + } + + foreach (['codes', '$permissions'] as $attribute) { + foreach ([Query::sum($attribute), Query::avg($attribute), Query::bitOr($attribute)] as $query) { + $this->assertFalse($validator->isValid($query), $attribute); + $this->assertStringEndsWith('that is not an array: '.$attribute, $validator->getDescription()); + } } foreach (['name', 'size', 'ratio', 'born', '$id', '$createdAt'] as $attribute) {