diff --git a/composer.json b/composer.json index bedf0a71a..e35127263 100755 --- a/composer.json +++ b/composer.json @@ -54,7 +54,7 @@ "utopia-php/cache": "^4.0 || ^5.0", "utopia-php/pools": "^2.0", "utopia-php/mongo": "^1.5.4", - "utopia-php/query": "^0.7.1", + "utopia-php/query": "^0.7.2", "utopia-php/async": "^0.2" }, "require-dev": { diff --git a/composer.lock b/composer.lock index 2ecf01fdb..6893e9e93 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "45b5cf01aef6a3cd2ad9b296d965edfb", + "content-hash": "166a6243504fe68982cde7684c933e3d", "packages": [ { "name": "brick/math", @@ -2278,16 +2278,16 @@ }, { "name": "utopia-php/query", - "version": "0.7.1", + "version": "0.7.2", "source": { "type": "git", "url": "https://github.com/utopia-php/query.git", - "reference": "291ec903812f6b752991c02ee4ea50e835c23b9b" + "reference": "945ef1c9f1c863b0e6e8bbac4c81c1b667dbfc9c" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/query/zipball/291ec903812f6b752991c02ee4ea50e835c23b9b", - "reference": "291ec903812f6b752991c02ee4ea50e835c23b9b", + "url": "https://api.github.com/repos/utopia-php/query/zipball/945ef1c9f1c863b0e6e8bbac4c81c1b667dbfc9c", + "reference": "945ef1c9f1c863b0e6e8bbac4c81c1b667dbfc9c", "shasum": "" }, "require": { @@ -2316,9 +2316,9 @@ ], "support": { "issues": "https://github.com/utopia-php/query/issues", - "source": "https://github.com/utopia-php/query/tree/0.7.1" + "source": "https://github.com/utopia-php/query/tree/0.7.2" }, - "time": "2026-10-15T06:11:20+00:00" + "time": "2026-10-16T10:52:31+00:00" }, { "name": "utopia-php/telemetry", diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index b12a0ba53..7fedaab1c 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -4507,14 +4507,14 @@ private function assertJoinAliases(array $queries): void } $alias = $query->getAlias(); - $invalid = JoinValidator::describeInvalidAlias($alias); + $invalid = JoinValidator::describeInvalidAlias($alias, $query->isAliasDefaulted()); if ($invalid !== null) { throw new QueryException($invalid); } $key = \strtolower($alias); if (isset($declared[$key])) { - throw new QueryException("Join alias \"{$alias}\" is declared more than once"); + throw new QueryException(JoinValidator::describeDuplicateAlias($alias, $query->isAliasDefaulted())); } $declared[$key] = true; } diff --git a/src/Database/Query.php b/src/Database/Query.php index b3e6b3dac..31fbe763d 100644 --- a/src/Database/Query.php +++ b/src/Database/Query.php @@ -46,6 +46,8 @@ class Query extends BaseQuery public const int SHAPE_MAX_NODES = 10000; /** + * A join given no alias is read under the collection it joins. + * * @param array $values */ public function __construct(Method|string $method, string $attribute = '', array $values = [], string $alias = '') @@ -56,6 +58,10 @@ public function __construct(Method|string $method, string $attribute = '', array $attribute = Document::SEQUENCE; } + if ($alias === '' && $methodEnum->isJoin()) { + $alias = $attribute; + } + parent::__construct($methodEnum, $attribute, $values, $alias); } diff --git a/src/Database/Validator/Query/Join.php b/src/Database/Validator/Query/Join.php index e97197458..681d3f49e 100644 --- a/src/Database/Validator/Query/Join.php +++ b/src/Database/Validator/Query/Join.php @@ -122,24 +122,46 @@ public function resetJoinAliases(): void /** * Why a join alias cannot be used, or null when it can. The reserved alias is matched without * case: SQLite, and MySQL on case-insensitive file systems, treat `x` and `X` as one alias. + * A defaulted alias is the table ID the join names, so the reason says that ID cannot serve as + * the alias and asks for one. */ - public static function describeInvalidAlias(string $alias): ?string + public static function describeInvalidAlias(string $alias, bool $defaulted = false): ?string { if ($alias === '') { return 'Join alias is required'; } if (\preg_match(self::ALIAS_PATTERN, $alias) !== 1) { - return 'Join alias must start with a letter or an underscore and contain only letters, digits and underscores'; + return $defaulted + ? self::describeUnusableTable($alias, 'an alias must start with a letter or an underscore and contain only letters, digits and underscores') + : 'Join alias must start with a letter or an underscore and contain only letters, digits and underscores'; } if (\strcasecmp($alias, Query::DEFAULT_ALIAS) === 0) { - return "Join alias \"{$alias}\" is reserved for the main collection"; + return $defaulted + ? self::describeUnusableTable($alias, 'it is reserved for the main collection') + : "Join alias \"{$alias}\" is reserved for the main collection"; } return null; } + /** + * Why a join alias another join of the same query set already declared cannot be used. The + * adapters check it, as they read `x` and `X` as one alias. + */ + public static function describeDuplicateAlias(string $alias, bool $defaulted = false): string + { + return $defaulted + ? self::describeUnusableTable($alias, 'another join of the query already uses it as its alias') + : "Join alias \"{$alias}\" is declared more than once"; + } + + private static function describeUnusableTable(string $table, string $reason): string + { + return "Table ID \"{$table}\" cannot be used as the join alias: {$reason}. Pass an alias for the join"; + } + /** * Get the method type this validator handles. * @@ -187,7 +209,8 @@ protected function isValidQuery(Query $query): bool } $alias = $query->getAlias(); - $invalidAlias = self::describeInvalidAlias($alias); + $defaulted = $query->isAliasDefaulted(); + $invalidAlias = self::describeInvalidAlias($alias, $defaulted); if ($invalidAlias !== null) { $this->message = $invalidAlias; @@ -195,7 +218,9 @@ protected function isValidQuery(Query $query): bool } if (isset($this->relationships[$alias])) { - $this->message = "Join alias \"{$alias}\" is the key of the relationship attribute \"{$alias}\": give the join another alias"; + $this->message = $defaulted + ? self::describeUnusableTable($alias, "it is the key of the relationship attribute \"{$alias}\"") + : "Join alias \"{$alias}\" is the key of the relationship attribute \"{$alias}\": give the join another alias"; return false; } diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 22669f79b..06a0e15fa 100644 --- a/tests/e2e/Adapter/Base.php +++ b/tests/e2e/Adapter/Base.php @@ -15,6 +15,7 @@ use Tests\E2E\Adapter\Scopes\GeneralTests; use Tests\E2E\Adapter\Scopes\IndexTests; use Tests\E2E\Adapter\Scopes\IntegerRoundingTests; +use Tests\E2E\Adapter\Scopes\JoinAliasTests; use Tests\E2E\Adapter\Scopes\JoinComboTests; use Tests\E2E\Adapter\Scopes\JoinReadRuleTests; use Tests\E2E\Adapter\Scopes\JoinRelationshipTests; @@ -58,6 +59,7 @@ abstract class Base extends TestCase use GeneralTests; use IndexTests; use IntegerRoundingTests; + use JoinAliasTests; use JoinComboTests; use JoinReadRuleTests; use JoinRelationshipTests; diff --git a/tests/e2e/Adapter/Scopes/JoinAliasTests.php b/tests/e2e/Adapter/Scopes/JoinAliasTests.php new file mode 100644 index 000000000..a60f62ad5 --- /dev/null +++ b/tests/e2e/Adapter/Scopes/JoinAliasTests.php @@ -0,0 +1,414 @@ +getAdapter()->supports(Capability::Joins)) { + $this->expectNotToPerformAssertions(); + + return; + } + + $this->createAliasFixtures($database); + + try { + $rows = $database->find(self::ALIAS_ORDERS, [ + Query::join(self::ALIAS_CUSTOMERS, '', [Query::on('customerId', '$id')]), + Query::select(['total', self::ALIAS_CUSTOMERS.'.name']), + Query::orderAsc('total'), + ]); + + $this->assertSame( + [[10, 'Ann'], [20, 'Bob']], + \array_map(static fn (Document $row): array => [$row->getAttribute('total'), $row->getAttribute(self::ALIAS_CUSTOMERS.'.name')], $rows), + ); + $this->assertArrayHasKey(self::ALIAS_CUSTOMERS.'.name', $rows[0]->getArrayCopy()); + + $this->assertSame(2, $database->count(self::ALIAS_ORDERS, [ + Query::join(self::ALIAS_CUSTOMERS, '', [Query::on('customerId', '$id')]), + Query::equal(self::ALIAS_CUSTOMERS.'.name', ['Ann', 'Bob']), + ])); + + $parsed = Query::parse(Query::join(self::ALIAS_CUSTOMERS, '', [Query::on('customerId', '$id')])->toString()); + $rows = $database->find(self::ALIAS_ORDERS, [ + $parsed, + Query::select(['total', self::ALIAS_CUSTOMERS.'.name']), + Query::equal(self::ALIAS_CUSTOMERS.'.name', ['Bob']), + ]); + $this->assertCount(1, $rows); + $this->assertSame('Bob', $rows[0]->getAttribute(self::ALIAS_CUSTOMERS.'.name'), 'a parsed join without an alias reads the same'); + } finally { + $this->cleanupAggCollections($database, $this->aliasFixtureCollections()); + } + } + + public function testLeftJoinWithoutAliasIsReadUnderItsTableId(): void + { + $database = static::getDatabase(); + if (! $database->getAdapter()->supports(Capability::Joins)) { + $this->expectNotToPerformAssertions(); + + return; + } + + $this->createAliasFixtures($database); + + try { + $rows = $database->find(self::ALIAS_ORDERS, [ + Query::leftJoin(self::ALIAS_CUSTOMERS, '', [Query::on('customerId', '$id')]), + Query::select(['total', self::ALIAS_CUSTOMERS.'.name']), + Query::orderAsc('total'), + ]); + + $this->assertSame( + [[10, 'Ann'], [20, 'Bob'], [30, null]], + \array_map(static fn (Document $row): array => [$row->getAttribute('total'), $row->getAttribute(self::ALIAS_CUSTOMERS.'.name')], $rows), + ); + } finally { + $this->cleanupAggCollections($database, $this->aliasFixtureCollections()); + } + } + + public function testCrossJoinWithoutAliasIsReadUnderItsTableId(): void + { + $database = static::getDatabase(); + if (! $database->getAdapter()->supports(Capability::Joins)) { + $this->expectNotToPerformAssertions(); + + return; + } + + $this->createAliasFixtures($database); + + try { + $rows = $database->find(self::ALIAS_ORDERS, [ + Query::crossJoin(self::ALIAS_COLORS, ''), + Query::select(['total', self::ALIAS_COLORS.'.name']), + Query::lessThan('total', 30), + Query::orderAsc('total'), + Query::orderAsc(self::ALIAS_COLORS.'.name'), + ]); + + $this->assertSame( + [[10, 'blue'], [10, 'red'], [20, 'blue'], [20, 'red']], + \array_map(static fn (Document $row): array => [$row->getAttribute('total'), $row->getAttribute(self::ALIAS_COLORS.'.name')], $rows), + ); + } finally { + $this->cleanupAggCollections($database, $this->aliasFixtureCollections()); + } + } + + /** + * The main collection is never referenced by its own ID, so a self-join without an alias reads the + * joined copy under the table ID and the main copy bare. + */ + public function testSelfJoinWithoutAliasIsReadUnderItsTableId(): void + { + $database = static::getDatabase(); + if (! $database->getAdapter()->supports(Capability::Joins)) { + $this->expectNotToPerformAssertions(); + + return; + } + + $this->createAliasFixtures($database); + + try { + $rows = $database->find(self::ALIAS_STAFF, [ + Query::join(self::ALIAS_STAFF, '', [Query::on('managerId', '$id')]), + Query::select(['name', self::ALIAS_STAFF.'.name']), + Query::orderAsc('name'), + ]); + + $this->assertSame( + [['Cal', 'Ann'], ['Dee', 'Cal']], + \array_map(static fn (Document $row): array => [$row->getAttribute('name'), $row->getAttribute(self::ALIAS_STAFF.'.name')], $rows), + ); + + $this->assertRejectedJoin( + 'The left column of a join condition must belong to the main collection or to a join declared before it: '.self::ALIAS_STAFF.'.managerId', + fn (): mixed => $database->find(self::ALIAS_STAFF, [Query::join(self::ALIAS_STAFF, '', [Query::on(self::ALIAS_STAFF.'.managerId', '$id')])]), + ); + } finally { + $this->cleanupAggCollections($database, $this->aliasFixtureCollections()); + } + } + + public function testJoinWithoutAliasRefusesATableIdThatIsNotAnAlias(): void + { + $database = static::getDatabase(); + if (! $database->getAdapter()->supports(Capability::Joins)) { + $this->expectNotToPerformAssertions(); + + return; + } + + $this->createAliasFixtures($database); + + try { + foreach ([self::ALIAS_HYPHEN, self::ALIAS_DOT] as $table) { + $this->assertRejectedJoin( + "Table ID \"{$table}\" cannot be used as the join alias: an alias must start with a letter or an underscore and contain only letters, digits and underscores. Pass an alias for the join", + fn (): mixed => $database->find(self::ALIAS_ORDERS, [Query::join($table, '', [Query::on('customerId', '$id')])]), + $table, + ); + $this->assertRejectedJoin( + "Table ID \"{$table}\" cannot be used as the join alias: an alias must start with a letter or an underscore and contain only letters, digits and underscores. Pass an alias for the join", + fn (): mixed => $database->find(self::ALIAS_ORDERS, [Query::crossJoin($table, '')]), + $table.' cross join', + ); + + $rows = $database->find(self::ALIAS_ORDERS, [ + Query::join($table, 'item', [Query::on('customerId', 'customerId')]), + Query::select(['total', 'item.label']), + ]); + $this->assertSame(['first'], \array_map(static fn (Document $row): mixed => $row->getAttribute('item.label'), $rows), $table.' with an alias'); + } + + $this->assertRejectedJoin( + 'Join alias must start with a letter or an underscore and contain only letters, digits and underscores', + fn (): mixed => $database->find(self::ALIAS_ORDERS, [Query::join(self::ALIAS_CUSTOMERS, 'my-alias', [Query::on('customerId', '$id')])]), + 'an explicit alias keeps its message', + ); + } finally { + $this->cleanupAggCollections($database, $this->aliasFixtureCollections()); + } + } + + public function testJoinWithoutAliasRefusesTheReservedMainAlias(): void + { + $database = static::getDatabase(); + if (! $database->getAdapter()->supports(Capability::Joins)) { + $this->expectNotToPerformAssertions(); + + return; + } + + $this->createAliasFixtures($database); + $this->createAliasCollection($database, Query::DEFAULT_ALIAS, [Attribute::string(key: 'customerId', size: 32)]); + + try { + $this->assertRejectedJoin( + 'Table ID "'.Query::DEFAULT_ALIAS.'" cannot be used as the join alias: it is reserved for the main collection. Pass an alias for the join', + fn (): mixed => $database->find(self::ALIAS_ORDERS, [Query::join(Query::DEFAULT_ALIAS, '', [Query::on('customerId', 'customerId')])]), + ); + $this->assertRejectedJoin( + 'Join alias "'.Query::DEFAULT_ALIAS.'" is reserved for the main collection', + fn (): mixed => $database->find(self::ALIAS_ORDERS, [Query::join(self::ALIAS_CUSTOMERS, Query::DEFAULT_ALIAS, [Query::on('customerId', '$id')])]), + 'an explicit alias keeps its message', + ); + } finally { + $this->cleanupAggCollections($database, [...$this->aliasFixtureCollections(), Query::DEFAULT_ALIAS]); + } + } + + public function testJoinWithoutAliasRefusesARelationshipKey(): void + { + $database = static::getDatabase(); + if (! $database->getAdapter()->supports(Capability::Joins) || ! $database->getAdapter()->hasFeature(Feature\Relationships::class)) { + $this->expectNotToPerformAssertions(); + + return; + } + + $this->createAliasFixtures($database); + $database->createRelationship(self::ALIAS_ORDERS, Relationship::manyToOne(relatedCollection: self::ALIAS_CUSTOMERS, key: self::ALIAS_CUSTOMERS, twoWayKey: 'orders')); + + try { + $this->assertRejectedJoin( + 'Table ID "'.self::ALIAS_CUSTOMERS.'" cannot be used as the join alias: it is the key of the relationship attribute "'.self::ALIAS_CUSTOMERS.'". Pass an alias for the join', + fn (): mixed => $database->find(self::ALIAS_ORDERS, [Query::join(self::ALIAS_CUSTOMERS, '', [Query::on('customerId', '$id')])]), + ); + $this->assertRejectedJoin( + 'Join alias "'.self::ALIAS_CUSTOMERS.'" is the key of the relationship attribute "'.self::ALIAS_CUSTOMERS.'": give the join another alias', + fn (): mixed => $database->find(self::ALIAS_ORDERS, [Query::join(self::ALIAS_COLORS, self::ALIAS_CUSTOMERS, [Query::on('customerId', '$id')])]), + 'an explicit alias keeps its message', + ); + } finally { + $this->cleanupAggCollections($database, $this->aliasFixtureCollections()); + } + } + + public function testJoiningATableTwiceWithoutAliasesIsRefused(): void + { + $database = static::getDatabase(); + if (! $database->getAdapter()->supports(Capability::Joins)) { + $this->expectNotToPerformAssertions(); + + return; + } + + $this->createAliasFixtures($database); + + try { + $twice = [ + Query::join(self::ALIAS_CUSTOMERS, '', [Query::on('customerId', '$id')]), + Query::leftJoin(self::ALIAS_CUSTOMERS, '', [Query::on('customerId', '$id')]), + ]; + $message = 'Table ID "'.self::ALIAS_CUSTOMERS.'" cannot be used as the join alias: another join of the query already uses it as its alias. Pass an alias for the join'; + + $this->assertRejectedJoin($message, fn (): mixed => $database->find(self::ALIAS_ORDERS, $twice), 'find()'); + $this->assertRejectedJoin($message, fn (): mixed => $database->count(self::ALIAS_ORDERS, $twice), 'count()'); + $this->assertRejectedJoin($message, fn (): mixed => $database->find(self::ALIAS_ORDERS, [$twice[0], Query::crossJoin(self::ALIAS_CUSTOMERS, '')]), 'a cross join'); + + $this->assertRejectedJoin( + 'Join alias "c" is declared more than once', + fn (): mixed => $database->find(self::ALIAS_ORDERS, [ + Query::join(self::ALIAS_CUSTOMERS, 'c', [Query::on('customerId', '$id')]), + Query::join(self::ALIAS_COLORS, 'c', [Query::on('customerId', '$id')]), + ]), + 'an explicit alias keeps its message', + ); + + $rows = $database->find(self::ALIAS_ORDERS, [ + Query::join(self::ALIAS_CUSTOMERS, '', [Query::on('customerId', '$id')]), + Query::leftJoin(self::ALIAS_CUSTOMERS, 'again', [Query::on('customerId', '$id')]), + Query::select(['total', self::ALIAS_CUSTOMERS.'.name', 'again.name']), + Query::orderAsc('total'), + ]); + $this->assertSame( + [['Ann', 'Ann'], ['Bob', 'Bob']], + \array_map(static fn (Document $row): array => [$row->getAttribute(self::ALIAS_CUSTOMERS.'.name'), $row->getAttribute('again.name')], $rows), + 'aliasing the second join reads both', + ); + } finally { + $this->cleanupAggCollections($database, $this->aliasFixtureCollections()); + } + } + + /** + * A query hook may point a join at another table, as Appwrite points a public table ID at its + * internal collection. The alias is fixed when the join is built, so rows stay keyed by the ID + * the caller gave and a refusal names that ID, never the internal one. + */ + public function testJoinWithoutAliasKeepsTheTableIdAHookRewrites(): void + { + $database = static::getDatabase(); + if (! $database->getAdapter()->supports(Capability::Joins)) { + $this->expectNotToPerformAssertions(); + + return; + } + + $this->createAliasFixtures($database); + $hook = new JoinTables(['customers' => self::ALIAS_CUSTOMERS, 'public-customers' => self::ALIAS_CUSTOMERS]); + $database->addHook($hook); + + try { + $rows = $database->find(self::ALIAS_ORDERS, [ + Query::parseQuery(['method' => 'join', 'attribute' => 'customers', 'values' => [Query::on('customerId', '$id')->toArray()]]), + Query::select(['total', 'customers.name']), + Query::orderAsc('total'), + ]); + + $this->assertSame(['Ann', 'Bob'], \array_map(static fn (Document $row): mixed => $row->getAttribute('customers.name'), $rows)); + $this->assertArrayNotHasKey(self::ALIAS_CUSTOMERS.'.name', $rows[0]->getArrayCopy(), 'rows are not keyed by the internal table'); + + $this->assertRejectedJoin( + 'Table ID "public-customers" cannot be used as the join alias: an alias must start with a letter or an underscore and contain only letters, digits and underscores. Pass an alias for the join', + fn (): mixed => $database->find(self::ALIAS_ORDERS, [Query::join('public-customers', '', [Query::on('customerId', '$id')])]), + ); + } finally { + $database->removeHook($hook); + $this->cleanupAggCollections($database, $this->aliasFixtureCollections()); + } + } + + /** + * @param non-empty-string $message + */ + private function assertRejectedJoin(string $message, Closure $read, string $case = ''): void + { + try { + $read(); + $this->fail(($case === '' ? '' : $case.': ').'the join ran instead of being refused with "'.$message.'"'); + } catch (QueryException $error) { + $this->assertStringEndsWith($message, $error->getMessage(), $case); + } + } + + /** + * @return list + */ + private function aliasFixtureCollections(): array + { + return [self::ALIAS_ORDERS, self::ALIAS_CUSTOMERS, self::ALIAS_COLORS, self::ALIAS_STAFF, self::ALIAS_HYPHEN, self::ALIAS_DOT]; + } + + private function createAliasFixtures(Database $database): void + { + $this->cleanupAggCollections($database, [...$this->aliasFixtureCollections(), Query::DEFAULT_ALIAS]); + + $this->createAliasCollection($database, self::ALIAS_ORDERS, [Attribute::string(key: 'customerId', size: 32), Attribute::integer(key: 'total')]); + $this->createAliasCollection($database, self::ALIAS_CUSTOMERS, [Attribute::string(key: 'name', size: 32)]); + $this->createAliasCollection($database, self::ALIAS_COLORS, [Attribute::string(key: 'name', size: 32)]); + $this->createAliasCollection($database, self::ALIAS_STAFF, [Attribute::string(key: 'name', size: 32), Attribute::string(key: 'managerId', size: 32)]); + foreach ([self::ALIAS_HYPHEN, self::ALIAS_DOT] as $table) { + $this->createAliasCollection($database, $table, [Attribute::string(key: 'customerId', size: 32), Attribute::string(key: 'label', size: 32)]); + } + + foreach ([ + [self::ALIAS_ORDERS, 'o1', ['customerId' => 'ann', 'total' => 10]], + [self::ALIAS_ORDERS, 'o2', ['customerId' => 'bob', 'total' => 20]], + [self::ALIAS_ORDERS, 'o3', ['customerId' => 'nobody', 'total' => 30]], + [self::ALIAS_CUSTOMERS, 'ann', ['name' => 'Ann']], + [self::ALIAS_CUSTOMERS, 'bob', ['name' => 'Bob']], + [self::ALIAS_COLORS, 'red', ['name' => 'red']], + [self::ALIAS_COLORS, 'blue', ['name' => 'blue']], + [self::ALIAS_STAFF, 'ann', ['name' => 'Ann', 'managerId' => '']], + [self::ALIAS_STAFF, 'cal', ['name' => 'Cal', 'managerId' => 'ann']], + [self::ALIAS_STAFF, 'dee', ['name' => 'Dee', 'managerId' => 'cal']], + [self::ALIAS_HYPHEN, 'i1', ['customerId' => 'ann', 'label' => 'first']], + [self::ALIAS_DOT, 'i1', ['customerId' => 'ann', 'label' => 'first']], + ] as [$collection, $id, $attributes]) { + $database->createDocument($collection, new Document(['$id' => $id, '$permissions' => [Permission::read(Role::any())], ...$attributes])); + } + } + + /** + * @param list $attributes + */ + private function createAliasCollection(Database $database, string $id, array $attributes): void + { + $database->createCollection(Collection::create( + id: $id, + attributes: $attributes, + permissions: [Permission::create(Role::any()), Permission::read(Role::any())], + documentSecurity: false, + )); + } +} diff --git a/tests/e2e/Adapter/Support/JoinTables.php b/tests/e2e/Adapter/Support/JoinTables.php new file mode 100644 index 000000000..a0fc3f631 --- /dev/null +++ b/tests/e2e/Adapter/Support/JoinTables.php @@ -0,0 +1,34 @@ + $tables The table each joined collection is read from + */ + public function __construct(private array $tables) + { + } + + #[\Override] + public function rewrite(Event $event, Collection $collection, array $queries): array + { + foreach ($queries as $index => $query) { + $table = $this->tables[$query->getAttribute()] ?? null; + if ($table !== null && $query->getMethod()->isJoin()) { + $queries[$index] = (clone $query)->setAttribute($table); + } + } + + return $queries; + } +} diff --git a/tests/unit/Builder/PostgresDistanceTest.php b/tests/unit/Builder/PostgresDistanceTest.php index bdcb4d4fe..6c7aedf3b 100644 --- a/tests/unit/Builder/PostgresDistanceTest.php +++ b/tests/unit/Builder/PostgresDistanceTest.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\TestCase; use Utopia\Database\Builder\Postgres; use Utopia\Database\Query; +use Utopia\Query\Exception\ValidationException; use Utopia\Query\Schema\ColumnType; final class PostgresDistanceTest extends TestCase @@ -46,9 +47,6 @@ public static function exactOnly(): array 'a polygon value' => [self::typed(Query::distanceLessThan('location', [[[0, 0], [0, 1], [1, 1], [0, 0]]], 1000, true), ColumnType::Point)], 'a box reaching a pole' => [self::typed(Query::distanceLessThan('location', [10, 89.99], 5000, true), ColumnType::Point)], 'a box reaching the antimeridian' => [self::typed(Query::distanceLessThan('location', [179.99, 0], 5000, true), ColumnType::Point)], - 'a distance that is not a number' => [self::typed(Query::distanceLessThan('location', [10, 20], NAN, true), ColumnType::Point)], - 'an infinite distance' => [self::typed(Query::distanceLessThan('location', [10, 20], INF, true), ColumnType::Point)], - 'a negatively infinite distance' => [self::typed(Query::distanceLessThan('location', [10, 20], -INF, true), ColumnType::Point)], ]; } @@ -61,6 +59,27 @@ public function testDistanceLessThanInMetersKeepsTheExactCheckOnlyWhenNoBoxHolds $this->assertCount(2, $condition->bindings); } + /** + * @return array + */ + public static function nonFiniteDistances(): array + { + return [ + 'a distance that is not a number' => [self::typed(Query::distanceLessThan('location', [10, 20], NAN, true), ColumnType::Point)], + 'an infinite distance' => [self::typed(Query::distanceLessThan('location', [10, 20], INF, true), ColumnType::Point)], + 'a negatively infinite distance' => [self::typed(Query::distanceLessThan('location', [10, 20], -INF, true), ColumnType::Point)], + ]; + } + + #[DataProvider('nonFiniteDistances')] + public function testDistanceLessThanInMetersRefusesADistanceThatIsNotFinite(Query $query): void + { + $this->expectException(ValidationException::class); + $this->expectExceptionMessage('Invalid distance query: the distance must be a finite number'); + + (new Postgres())->compileFilters([$query]); + } + public function testDistanceGreaterThanIsUnchanged(): void { $condition = (new Postgres())->compileFilters([self::typed(Query::distanceGreaterThan('location', [10, 20], 5), ColumnType::Point)]); diff --git a/tests/unit/JoinAliasTest.php b/tests/unit/JoinAliasTest.php index 0a671780e..a984d9e32 100644 --- a/tests/unit/JoinAliasTest.php +++ b/tests/unit/JoinAliasTest.php @@ -54,16 +54,40 @@ public function testAnAliasSortingBeforeAnEarlierOneStillReadsItsOwnJoin(): void ])); } - public function testAJoinWithoutAnAliasIsRejectedWithoutQueryValidation(): void + public function testAJoinWithoutAnAliasIsReadUnderItsCollection(): void { $database = $this->database(); - $this->expectException(QueryException::class); - $this->expectExceptionMessage('Join alias is required'); + foreach ([ + 'factory' => Query::join('b', '', [Query::on('$id', 'mainId')]), + 'constructor' => new Query(Method::Join, 'b', [Query::on('$id', 'mainId')]), + 'parsed' => Query::parseQuery(['method' => 'join', 'attribute' => 'b', 'values' => ['$id', '=', 'mainId']]), + ] as $case => $join) { + $rows = $database->find('main', [$join, Query::select(['name', 'b.score'])]); + $this->assertCount(1, $rows, $case); + $this->assertSame(1, $rows[0]->getAttribute('b.score'), $case); + + $rows = $database->skipValidation(fn (): array => $database->find('main', [$join, Query::select(['name', 'b.score'])])); + $this->assertSame(1, $rows[0]->getAttribute('b.score'), $case.' without query validation'); + } + } - $database->skipValidation(fn (): array => $database->find('main', [ - new Query(Method::Join, 'b', [Query::on('$id', 'mainId')], ''), - ])); + public function testJoiningACollectionTwiceWithoutAliasesNamesTheCollection(): void + { + $database = $this->database(); + $joins = [ + Query::join('b', '', [Query::on('$id', 'mainId')]), + Query::leftJoin('b', '', [Query::on('$id', 'mainId')]), + ]; + + foreach (['with query validation' => fn (): array => $database->find('main', $joins), 'without query validation' => fn (): array => $database->skipValidation(fn (): array => $database->find('main', $joins))] as $case => $read) { + try { + $read(); + $this->fail($case.': the joins ran'); + } catch (QueryException $error) { + $this->assertSame('Table ID "b" cannot be used as the join alias: another join of the query already uses it as its alias. Pass an alias for the join', $error->getMessage(), $case); + } + } } /** @@ -144,6 +168,14 @@ public static function collidingAliases(): iterable yield 'an alias that is not an identifier' => [[ Query::join('b', 'my-alias', [Query::on('$id', 'mainId')]), ]]; + yield 'a collection joined twice without aliases' => [[ + Query::join('b', '', [Query::on('$id', 'mainId')]), + Query::crossJoin('b', ''), + ]]; + yield 'a join without an alias that repeats an earlier alias' => [[ + Query::join('c', 'b', [Query::on('$id', 'mainId')]), + Query::join('b', '', [Query::on('$id', 'mainId')]), + ]]; yield 'a nested join alias that repeats an earlier one' => [[ Query::join('b', 'x', [Query::on('$id', 'mainId')]), Query::leftJoin('c', 'x', [Query::on('$id', 'mainId')]), diff --git a/tests/unit/QuerySerializationTest.php b/tests/unit/QuerySerializationTest.php index 5de062d12..4fbb9aa49 100644 --- a/tests/unit/QuerySerializationTest.php +++ b/tests/unit/QuerySerializationTest.php @@ -130,12 +130,61 @@ public function testParsedColumnFormJoinBecomesAnOnCondition(): void ); } - public function testParsedJoinWithoutAnAliasIsAQueryException(): void + /** + * @return iterable, Method}> + */ + public static function joinsWithoutAlias(): iterable + { + $on = ['method' => 'on', 'values' => ['$id', '=', 'user']]; + + yield 'the nested ON form' => [['method' => 'join', 'attribute' => 'orders', 'values' => [$on]], Method::Join]; + yield 'an empty alias property' => [['method' => 'leftJoin', 'attribute' => 'orders', 'alias' => '', 'values' => [$on]], Method::LeftJoin]; + yield 'an empty legacy alias first' => [['method' => 'join', 'attribute' => 'orders', 'values' => ['', $on]], Method::Join]; + yield 'the column form' => [['method' => 'join', 'attribute' => 'orders', 'values' => ['$id', '=', 'user']], Method::Join]; + yield 'a cross join' => [['method' => 'crossJoin', 'attribute' => 'orders', 'values' => []], Method::CrossJoin]; + } + + /** + * @param array $array + */ + #[DataProvider('joinsWithoutAlias')] + public function testParsedJoinWithoutAnAliasIsReadUnderItsCollection(array $array, Method $method): void + { + $parsed = Query::parseQuery($array); + + $this->assertSame($method, $parsed->getMethod()); + $this->assertSame('orders', $parsed->getAlias()); + $this->assertTrue($parsed->isAliasDefaulted()); + + $serialized = $parsed->toArray(); + $this->assertSame('orders', $serialized['alias']); + $this->assertSame($serialized, Query::parseQuery($serialized)->toArray()); + $this->assertTrue(Query::parse($parsed->toString())->isAliasDefaulted()); + } + + public function testAJoinBuiltWithoutAnAliasIsReadUnderItsCollection(): void + { + $join = new Query(Method::Join, 'orders', [Query::on('$id', 'user')]); + + $this->assertSame('orders', $join->getAlias()); + $this->assertTrue($join->isAliasDefaulted()); + $this->assertFalse(Query::join('orders', 'ord', [Query::on('$id', 'ord.user')])->isAliasDefaulted()); + } + + public function testADefaultedAliasSurvivesRenamingTheJoinedCollection(): void + { + $renamed = (clone Query::join('orders', '', [Query::on('$id', 'user')]))->setAttribute('database_1_collection_2'); + + $this->assertSame('orders', $renamed->getAlias()); + $this->assertTrue($renamed->isAliasDefaulted()); + } + + public function testParsedJoinWithoutAnAliasOrCollectionIsAQueryException(): void { $this->expectException(QueryException::class); - $this->expectExceptionMessage('Join alias is required: join orders'); + $this->expectExceptionMessage('Join alias is required: join '); - Query::parseQuery(['method' => 'join', 'attribute' => 'orders', 'values' => [['method' => 'on', 'values' => ['$id', '=', 'user']]]]); + Query::parseQuery(['method' => 'join', 'values' => [['method' => 'on', 'values' => ['$id', '=', 'user']]]]); } /** diff --git a/tests/unit/Validator/Query/JoinAliasTest.php b/tests/unit/Validator/Query/JoinAliasTest.php index 6fe43cc79..63bebb75a 100644 --- a/tests/unit/Validator/Query/JoinAliasTest.php +++ b/tests/unit/Validator/Query/JoinAliasTest.php @@ -4,9 +4,13 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use Utopia\Database\Document; use Utopia\Database\Query; +use Utopia\Database\RelationshipType; use Utopia\Database\Validator\Query\Join; +use Utopia\Database\Validator\Query\Joined\Collection as JoinedCollection; use Utopia\Query\Method; +use Utopia\Query\Schema\ColumnType; final class JoinAliasTest extends TestCase { @@ -28,6 +32,11 @@ public static function acceptedJoins(): iterable yield 'an identifier with digits and underscores' => [Query::leftJoin('orders', '_order_2', [Query::on('$id', 'customerId')])]; yield 'a cross join alias' => [Query::crossJoin('orders', 'ord')]; yield 'a nested join alias' => [Query::rightJoin('orders', 'ord', [Query::on('$id', 'customerId')])]; + yield 'no alias' => [Query::join('orders', '', [Query::on('$id', 'customerId')])]; + yield 'no alias on a left join' => [Query::leftJoin('orders', '', [Query::on('$id', 'orders.customerId')])]; + yield 'no alias on a cross join' => [Query::crossJoin('orders', '')]; + yield 'no alias on a join built without one' => [new Query(Method::Join, 'orders', [Query::on('$id', 'customerId')])]; + yield 'no alias on a join of a table named like the main collection' => [Query::join('main', '', [Query::on('$id', 'main.customerId')])]; } #[DataProvider('rejectedJoins')] @@ -48,7 +57,6 @@ public static function rejectedJoins(): iterable $upper = \strtoupper(Query::DEFAULT_ALIAS); $invalid = 'Join alias must start with a letter or an underscore and contain only letters, digits and underscores'; - yield 'no alias' => [new Query(Method::Join, 'orders', [Query::on('$id', 'customerId')], ''), 'Join alias is required']; yield 'the main collection alias' => [Query::join('orders', $main, [Query::on('$id', 'customerId')]), "Join alias \"{$main}\" is reserved for the main collection"]; yield 'the main collection alias in upper case' => [Query::join('orders', $upper, [Query::on('$id', 'customerId')]), "Join alias \"{$upper}\" is reserved for the main collection"]; yield 'the main collection alias on a cross join' => [Query::crossJoin('orders', $main), "Join alias \"{$main}\" is reserved for the main collection"]; @@ -58,4 +66,86 @@ public static function rejectedJoins(): iterable yield 'a dot' => [Query::join('orders', 'a.b', [Query::on('$id', 'customerId')]), $invalid]; yield 'a quote' => [Query::crossJoin('orders', 'x`y'), $invalid]; } + + #[DataProvider('rejectedTableIds')] + public function testRejectsATableIdThatCannotBeTheAlias(Query $join, string $message): void + { + $validator = new Join(); + + $this->assertTrue($join->isAliasDefaulted()); + $this->assertFalse($validator->isValid($join)); + $this->assertSame($message, $validator->getDescription()); + } + + /** + * @return iterable + */ + public static function rejectedTableIds(): iterable + { + $main = Query::DEFAULT_ALIAS; + $upper = \strtoupper(Query::DEFAULT_ALIAS); + $pattern = static fn (string $table): string => "Table ID \"{$table}\" cannot be used as the join alias: an alias must start with a letter or an underscore and contain only letters, digits and underscores. Pass an alias for the join"; + $reserved = static fn (string $table): string => "Table ID \"{$table}\" cannot be used as the join alias: it is reserved for the main collection. Pass an alias for the join"; + + yield 'a hyphen' => [Query::join('my-orders', '', [Query::on('$id', 'customerId')]), $pattern('my-orders')]; + yield 'a dot' => [Query::leftJoin('orders.v2', '', [Query::on('$id', 'customerId')]), $pattern('orders.v2')]; + yield 'a leading digit' => [Query::rightJoin('2024orders', '', [Query::on('$id', 'customerId')]), $pattern('2024orders')]; + yield 'a hyphen on a cross join' => [Query::crossJoin('my-orders', ''), $pattern('my-orders')]; + yield 'a hyphen on a join built without an alias' => [new Query(Method::FullOuterJoin, 'my-orders', [Query::on('$id', 'customerId')]), $pattern('my-orders')]; + yield 'a hyphen on a parsed join' => [Query::parseQuery(['method' => 'join', 'attribute' => 'my-orders', 'values' => ['$id', '=', 'customerId']]), $pattern('my-orders')]; + yield 'the main collection alias' => [Query::join($main, '', [Query::on('$id', 'customerId')]), $reserved($main)]; + yield 'the main collection alias in upper case' => [Query::crossJoin($upper, ''), $reserved($upper)]; + } + + public function testRejectsATableIdThatIsARelationshipKey(): void + { + $validator = new Join([ + new Document(['$id' => 'customerId', 'key' => 'customerId', 'type' => ColumnType::String->value]), + new Document(['$id' => 'customers', 'key' => 'customers', 'type' => ColumnType::Relationship->value, 'options' => ['relationType' => RelationshipType::ManyToOne->value, 'side' => 'parent', 'relatedCollection' => 'customers']]), + ]); + + $this->assertFalse($validator->isValid(Query::join('customers', '', [Query::on('customerId', '$id')]))); + $this->assertSame('Table ID "customers" cannot be used as the join alias: it is the key of the relationship attribute "customers". Pass an alias for the join', $validator->getDescription()); + + $validator->resetJoinAliases(); + $this->assertFalse($validator->isValid(Query::join('people', 'customers', [Query::on('customerId', '$id')]))); + $this->assertSame('Join alias "customers" is the key of the relationship attribute "customers": give the join another alias', $validator->getDescription(), 'an explicit alias keeps its message'); + + $validator->resetJoinAliases(); + $this->assertTrue($validator->isValid(Query::join('customers', 'buyer', [Query::on('customerId', '$id')])), $validator->getDescription()); + } + + /** + * The main collection is read under the reserved alias, never under its own ID, so a self-join's + * defaulted alias names only the joined copy: its columns are qualified by it on the right, and a + * left column qualified by it is not the main collection's. + */ + public function testASelfJoinWithoutAnAliasIsReadUnderTheTableId(): void + { + $columns = [new Document(['$id' => 'managerId', 'key' => 'managerId', 'type' => ColumnType::String->value])]; + $validator = new Join($columns); + $validator->allowJoins([JoinedCollection::of('staff', new Document(['$id' => 'staff', 'attributes' => $columns]))]); + + $this->assertTrue($validator->isValid(Query::join('staff', '', [Query::on('managerId', '$id')])), $validator->getDescription()); + + $validator->resetJoinAliases(); + $validator->allowJoins([JoinedCollection::of('staff', new Document(['$id' => 'staff', 'attributes' => $columns]))]); + $this->assertTrue($validator->isValid(Query::join('staff', '', [Query::on('$id', 'staff.managerId')])), $validator->getDescription()); + + $validator->resetJoinAliases(); + $this->assertFalse($validator->isValid(Query::join('staff', '', [Query::on('staff.managerId', '$id')]))); + $this->assertSame('The left column of a join condition must belong to the main collection or to a join declared before it: staff.managerId', $validator->getDescription()); + } + + public function testAnEmptyAliasIsStillDescribedAsRequired(): void + { + $this->assertSame('Join alias is required', Join::describeInvalidAlias('')); + $this->assertSame('Join alias is required', Join::describeInvalidAlias('', true)); + } + + public function testDescribesADuplicateAliasByHowItWasGiven(): void + { + $this->assertSame('Join alias "c" is declared more than once', Join::describeDuplicateAlias('c')); + $this->assertSame('Table ID "customers" cannot be used as the join alias: another join of the query already uses it as its alias. Pass an alias for the join', Join::describeDuplicateAlias('customers', true)); + } }