diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 24e3536e0..1fe80e939 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -56,6 +56,8 @@ class Postgres extends SQL implements Feature\Spatial, Feature\Timeouts public const int MAX_IDENTIFIER_NAME = 63; + protected const int MAX_LABEL_LENGTH = self::MAX_IDENTIFIER_NAME; + protected const string MIN_DATETIME = '-4713-01-01 00:00:00'; private const string QUOTED_IDENTIFIER = '/["\x{AB}\x{BB}\x{201C}\x{201D}\x{201E}\x{300C}\x{300D}][\s\x{A0}\x{202F}]*([^"\x{AB}\x{BB}\x{201C}\x{201D}\x{201E}\x{300C}\x{300D}]+?)[\s\x{A0}\x{202F}]*["\x{AB}\x{BB}\x{201C}\x{201D}\x{201E}\x{300C}\x{300D}]/u'; diff --git a/src/Database/Adapter/SQL.php b/src/Database/Adapter/SQL.php index 7fedaab1c..75f946e59 100644 --- a/src/Database/Adapter/SQL.php +++ b/src/Database/Adapter/SQL.php @@ -89,6 +89,26 @@ abstract class SQL extends Adapter implements Feature\Connection, Feature\RawQue private const string FOJ_ROWS_ALIAS = 'foj_rows'; + /** + * A joined column whose `alias.column` label is longer than the engine keeps is labelled with + * this prefix and a digest of the label instead, and remapRow() reads it back. A join alias + * cannot hold `$`. + */ + private const string JOIN_LABEL_PREFIX = '$join_'; + + /** + * The `alias.column` label each digest label stands for. A digest names one label for good, so + * the map only grows with the joined columns a process reads. + * + * @var array + */ + private static array $joinLabels = []; + + /** + * MariaDB and MySQL keep a column label of up to 256 characters. + */ + protected const int MAX_LABEL_LENGTH = 256; + protected const string MIN_DATETIME = '1000-01-01 00:00:00'; /** @@ -2109,7 +2129,7 @@ private function rowsColumn(string $attribute, array $queries, bool $grouped): s return $this->qualifiedGroupNames($values)[$index] ?? $column; } - return $dot === false ? $column : \substr($attribute, 0, $dot).'.'.$column; + return $dot === false ? $column : $this->joinLabel(\substr($attribute, 0, $dot), $column); } throw new QueryException('Invalid query: sum() over '.($grouped ? 'grouped' : 'distinct').' rows adds up one of their '.($grouped ? 'groupBy' : 'selected').' attributes, and '.$attribute.' is not one of them'); @@ -4194,7 +4214,6 @@ protected function mapSelectionsToColumns(array $selections, bool $includeIntern } $aliasSet = \array_fill_keys($joinAliases, true); - $quote = $this->getIdentifierQuote(); $columns = []; foreach ($selections as $selection) { $dot = \strpos($selection, '.'); @@ -4203,9 +4222,7 @@ protected function mapSelectionsToColumns(array $selections, bool $includeIntern if (isset($aliasSet[$prefix])) { $name = \substr($selection, $dot + 1); $internal = $this->filter($this->getInternalKeyForAttribute($name)); - $qualified = $quote.$this->filter($prefix).$quote.'.'.$quote.$internal.$quote; - $output = $prefix.'.'.$internal; - $columns[] = $qualified.' AS '.$quote.$output.$quote; + $columns[] = $this->quote($prefix).'.'.$this->quote($internal).' AS '.$this->quote($this->joinLabel($prefix, $internal)); continue; } @@ -4216,6 +4233,23 @@ protected function mapSelectionsToColumns(array $selections, bool $includeIntern return $columns; } + /** + * The label a joined column is read under: `alias.column`, or a digest of it when that is longer + * than the engine keeps a label, so the engine cannot cut it short. + */ + private function joinLabel(string $alias, string $column): string + { + $label = $alias.'.'.$column; + if (\strlen($label) <= static::MAX_LABEL_LENGTH) { + return $label; + } + + $digest = self::JOIN_LABEL_PREFIX.\md5($label); + self::$joinLabels[$digest] = $label; + + return $digest; + } + /** * The projection of a join without a select or with `*`: every column of the main table, and under each * join alias the joined collection's `$id` and the attributes the Database layer handed over for it. A @@ -6401,7 +6435,7 @@ private function qualifyOrderAttribute(string $attribute, array $joinAliases = [ } /** - * @param array $row + * @param array $row */ private function remapRow(array &$row): void { @@ -6410,6 +6444,11 @@ private function remapRow(array &$row): void unset($row[$key]); continue; } + if (isset(self::$joinLabels[$key])) { + $row[self::$joinLabels[$key]] = $row[$key]; + unset($row[$key]); + $key = self::$joinLabels[$key]; + } if (\str_starts_with($key, self::FOJ_ORDER_ALIAS_PREFIX)) { unset($row[$key]); diff --git a/src/Database/Adapter/SQL/Hook/Permission/Filter.php b/src/Database/Adapter/SQL/Hook/Permission/Filter.php index 24d7bf96b..8e8bbaff1 100644 --- a/src/Database/Adapter/SQL/Hook/Permission/Filter.php +++ b/src/Database/Adapter/SQL/Hook/Permission/Filter.php @@ -52,7 +52,11 @@ public function __construct( protected string $quoteCharacter = '`', protected bool $semiJoin = true, ) { - foreach ([$documentColumn, $permissionDocumentColumn, $permissionRoleColumn, $permissionTypeColumn, $scopeColumn] as $column) { + if (! \preg_match(self::QUOTED_IDENTIFIER_PATTERN, $documentColumn)) { + throw new InvalidArgumentException('Invalid column name: '.$documentColumn); + } + + foreach ([$permissionDocumentColumn, $permissionRoleColumn, $permissionTypeColumn, $scopeColumn] as $column) { if (! \preg_match(self::IDENTIFIER_PATTERN, $column)) { throw new InvalidArgumentException('Invalid column name: '.$column); } diff --git a/src/Database/Validator/Query/Join.php b/src/Database/Validator/Query/Join.php index 681d3f49e..b3b229e92 100644 --- a/src/Database/Validator/Query/Join.php +++ b/src/Database/Validator/Query/Join.php @@ -15,7 +15,15 @@ */ class Join extends Base { - public const string ALIAS_PATTERN = '/^[A-Za-z_][A-Za-z0-9_]*$/'; + /** + * The characters of an Appwrite ID but `.`, which separates an alias from its column. + */ + public const string ALIAS_PATTERN = '/^[A-Za-z0-9_][A-Za-z0-9_-]*$/'; + + /** + * PostgreSQL folds an identifier past 63 bytes, so two longer aliases could read as one. + */ + public const int MAX_ALIAS_LENGTH = 63; private const array OPERATORS = ['=', '!=', '<', '>', '<=', '>=', '<>']; @@ -133,8 +141,14 @@ public static function describeInvalidAlias(string $alias, bool $defaulted = fal if (\preg_match(self::ALIAS_PATTERN, $alias) !== 1) { 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'; + ? self::describeUnusableTable($alias, 'an alias must start with a letter, a digit or an underscore and contain only letters, digits, underscores and hyphens') + : 'Join alias must start with a letter, a digit or an underscore and contain only letters, digits, underscores and hyphens'; + } + + if (\strlen($alias) > self::MAX_ALIAS_LENGTH) { + return $defaulted + ? self::describeUnusableTable($alias, 'an alias must be at most '.self::MAX_ALIAS_LENGTH.' characters') + : 'Join alias must be at most '.self::MAX_ALIAS_LENGTH.' characters'; } if (\strcasecmp($alias, Query::DEFAULT_ALIAS) === 0) { diff --git a/tests/e2e/Adapter/Base.php b/tests/e2e/Adapter/Base.php index 06a0e15fa..7f7a1080f 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\JoinAliasIdTests; use Tests\E2E\Adapter\Scopes\JoinAliasTests; use Tests\E2E\Adapter\Scopes\JoinComboTests; use Tests\E2E\Adapter\Scopes\JoinReadRuleTests; @@ -59,6 +60,7 @@ abstract class Base extends TestCase use GeneralTests; use IndexTests; use IntegerRoundingTests; + use JoinAliasIdTests; use JoinAliasTests; use JoinComboTests; use JoinReadRuleTests; diff --git a/tests/e2e/Adapter/Scopes/JoinAliasIdTests.php b/tests/e2e/Adapter/Scopes/JoinAliasIdTests.php new file mode 100644 index 000000000..9e00a29eb --- /dev/null +++ b/tests/e2e/Adapter/Scopes/JoinAliasIdTests.php @@ -0,0 +1,370 @@ +getAdapter()->supports(Capability::Joins)) { + $this->expectNotToPerformAssertions(); + + return; + } + + $this->createAliasIdFixtures($database); + + try { + foreach ([self::ID_UNIQUE, self::ID_HYPHEN] as $table) { + $this->assertSame( + [[10, 'first'], [20, 'second']], + $this->readAliasIdRows($database, Query::join($table, '', [Query::on('customerId', 'customerId')]), $table), + $table.': the visible items under the table ID', + ); + + foreach ($this->aliasIdJoins() as $method => $join) { + $this->assertSame( + $this->readAliasIdShapes($database, $join($table, self::BASELINE), self::BASELINE), + $this->readAliasIdShapes($database, $join($table, ''), $table), + "{$table} {$method} without an alias reads as under a plain alias", + ); + } + } + } finally { + $this->cleanupAggCollections($database, $this->aliasIdCollections()); + } + } + + public function testJoinAliasShapedLikeAnAppwriteIdIsRead(): void + { + $database = static::getDatabase(); + if (! $database->getAdapter()->supports(Capability::Joins)) { + $this->expectNotToPerformAssertions(); + + return; + } + + $this->createAliasIdFixtures($database); + + try { + foreach (['6ad1e9ed691819ab52ef', 'order-lines', '_6ad1', '0'] as $alias) { + $this->assertSame( + [[10, 'first'], [20, 'second']], + $this->readAliasIdRows($database, Query::join(self::ID_ITEMS, $alias, [Query::on('customerId', 'customerId')]), $alias), + $alias.': the visible items under the alias', + ); + + foreach ($this->aliasIdJoins() as $method => $join) { + $this->assertSame( + $this->readAliasIdShapes($database, $join(self::ID_ITEMS, self::BASELINE), self::BASELINE), + $this->readAliasIdShapes($database, $join(self::ID_ITEMS, $alias), $alias), + "{$method} aliased {$alias} reads as under a plain alias", + ); + } + } + + $rows = $database->find(self::ID_ORDERS, [ + Query::join(self::ID_UNIQUE, '', [Query::on('customerId', 'customerId')]), + Query::join(self::ID_ITEMS, 'order-lines', [Query::on('customerId', 'customerId')]), + Query::select(['total', self::ID_UNIQUE.'.label', 'order-lines.label']), + Query::equal('order-lines.label', ['first']), + ]); + $this->assertSame([[10, 'first', 'first']], \array_map(static fn (Document $row): array => [$row->getAttribute('total'), $row->getAttribute(self::ID_UNIQUE.'.label'), $row->getAttribute('order-lines.label')], $rows), 'two such aliases in one read'); + } finally { + $this->cleanupAggCollections($database, $this->aliasIdCollections()); + } + } + + /** + * Postgres folds identifiers past 63 bytes, so a long alias and a long attribute must still come + * back under their full `alias.attribute` key. + */ + public function testJoinWithoutAliasReadsALongTableIdAndAttribute(): void + { + $database = static::getDatabase(); + if (! $database->getAdapter()->supports(Capability::Joins)) { + $this->expectNotToPerformAssertions(); + + return; + } + + $this->createAliasIdFixtures($database); + + try { + $key = self::ID_LONG.'.'.self::KEY_LONG; + $rows = $database->find(self::ID_ORDERS, [ + Query::join(self::ID_LONG, '', [Query::on('customerId', 'customerId')]), + Query::select(['total', $key]), + Query::orderAsc('total'), + Query::orderDesc($key), + ]); + + $this->assertSame([[10, 'first'], [20, 'second']], \array_map(static fn (Document $row): array => [$row->getAttribute('total'), $row->getAttribute($key)], $rows)); + + $rows = $database->find(self::ID_ORDERS, [ + Query::fullOuterJoin(self::ID_LONG, '', [Query::on('customerId', 'customerId')]), + Query::select(['total', $key]), + Query::orderAsc('total'), + ]); + $this->assertSame([[10, 'first'], [20, 'second'], [30, null]], \array_map(static fn (Document $row): array => [$row->getAttribute('total'), $row->getAttribute($key)], $rows), 'a full outer join'); + + $document = $database->getDocument(self::ID_ORDERS, 'o2', [ + Query::join(self::ID_LONG, '', [Query::on('customerId', 'customerId')]), + Query::select(['total', $key]), + ]); + $this->assertSame('second', $document->getAttribute($key), 'getDocument()'); + + $quantity = self::ID_LONG.'.'.self::KEY_LONG_NUMBER; + $this->assertSame(5, $database->sum(self::ID_ORDERS, $quantity, [ + Query::join(self::ID_LONG, '', [Query::on('customerId', 'customerId')]), + Query::distinct(), + Query::select([$quantity]), + ]), 'a distinct sum of a joined column'); + + $sixty = $database->find(self::ID_ORDERS, [ + Query::join(self::ID_ITEMS, 'i', [Query::on('customerId', 'customerId')]), + Query::select(['total', 'i.'.self::KEY_SIXTY]), + Query::orderAsc('total'), + ]); + $this->assertSame(['first', 'second'], \array_map(static fn (Document $row): mixed => $row->getAttribute('i.'.self::KEY_SIXTY), $sixty), 'a column whose own name nearly fills the label'); + + $groups = $database->aggregate(self::ID_ORDERS, [ + Query::join(self::ID_LONG, '', [Query::on('customerId', 'customerId')]), + Query::count('*', 'cnt'), + Query::groupBy([$key]), + ]); + $labels = \array_map(static fn (array $row): mixed => $row[$key] ?? null, $groups); + \sort($labels); + $this->assertSame(['first', 'second'], $labels, 'a group of the joined column'); + $this->assertSame(1, $database->count(self::ID_ORDERS, [ + Query::join(self::ID_LONG, '', [Query::on('customerId', 'customerId')]), + Query::equal($key, ['second']), + ])); + } finally { + $this->cleanupAggCollections($database, $this->aliasIdCollections()); + } + } + + public function testJoinAliasesOutsideTheIdCharsetAreStillRefused(): void + { + $database = static::getDatabase(); + if (! $database->getAdapter()->supports(Capability::Joins)) { + $this->expectNotToPerformAssertions(); + + return; + } + + $this->createAliasIdFixtures($database); + $invalid = 'Join alias must start with a letter, a digit or an underscore and contain only letters, digits, underscores and hyphens'; + + try { + foreach (['-items', 'order.items', 'order items', 'x`y', 'x"y'] as $alias) { + $this->assertAliasIdRefused($invalid, fn (): mixed => $database->find(self::ID_ORDERS, [Query::join(self::ID_ITEMS, $alias, [Query::on('customerId', 'customerId')])]), $alias); + } + + $this->assertAliasIdRefused( + 'Join alias must be at most 63 characters', + fn (): mixed => $database->find(self::ID_ORDERS, [Query::join(self::ID_ITEMS, \str_repeat('a', 64), [Query::on('customerId', 'customerId')])]), + 'a 64 character alias', + ); + $this->assertAliasIdRefused( + 'Join alias "'.\strtoupper(Query::DEFAULT_ALIAS).'" is reserved for the main collection', + fn (): mixed => $database->find(self::ID_ORDERS, [Query::join(self::ID_ITEMS, \strtoupper(Query::DEFAULT_ALIAS), [Query::on('customerId', 'customerId')])]), + 'the reserved alias', + ); + $this->assertAliasIdRefused( + 'Join alias "ORDER-ITEMS" is declared more than once', + fn (): mixed => $database->find(self::ID_ORDERS, [ + Query::join(self::ID_HYPHEN, '', [Query::on('customerId', 'customerId')]), + Query::join(self::ID_ITEMS, 'ORDER-ITEMS', [Query::on('customerId', 'customerId')]), + ]), + 'a hyphenated alias repeated in another case', + ); + } finally { + $this->cleanupAggCollections($database, $this->aliasIdCollections()); + } + } + + /** + * @return array + */ + private function aliasIdJoins(): array + { + $on = static fn (): array => [Query::on('customerId', 'customerId')]; + + return [ + 'join' => static fn (string $table, string $alias): Query => Query::join($table, $alias, $on()), + 'leftJoin' => static fn (string $table, string $alias): Query => Query::leftJoin($table, $alias, $on()), + 'rightJoin' => static fn (string $table, string $alias): Query => Query::rightJoin($table, $alias, $on()), + 'fullOuterJoin' => static fn (string $table, string $alias): Query => Query::fullOuterJoin($table, $alias, $on()), + 'crossJoin' => static fn (string $table, string $alias): Query => Query::crossJoin($table, $alias), + ]; + } + + /** + * @return list + */ + private function readAliasIdRows(Database $database, Query $join, string $alias): array + { + $rows = $database->find(self::ID_ORDERS, [ + $join, + Query::select(['total', $alias.'.label']), + Query::orderAsc('total'), + Query::orderAsc($alias.'.label'), + ]); + + foreach ($rows as $row) { + $this->assertArrayHasKey($alias.'.label', $row->getArrayCopy(), 'rows are keyed alias.column'); + } + + return \array_values(\array_map(static fn (Document $row): array => [$row->getAttribute('total'), $row->getAttribute($alias.'.label')], $rows)); + } + + /** + * Every read a join alias reaches, each as plain data with the alias written as `@`, or the refusal + * it gives. + * + * @return array + */ + private function readAliasIdShapes(Database $database, Query $join, string $alias): array + { + $label = $alias.'.label'; + $pair = static fn (Document $row): array => [$row->getAttribute('total'), $row->getAttribute($label)]; + + $reads = [ + 'find' => fn (): array => $this->readAliasIdRows($database, $join, $alias), + 'filter' => fn (): array => \array_map($pair, $database->find(self::ID_ORDERS, [$join, Query::select(['total', $label]), Query::equal($label, ['second']), Query::orderAsc('total')])), + 'filter by an ON condition' => fn (): array => \array_map($pair, $database->find(self::ID_ORDERS, [$join, Query::select(['total', $label]), Query::notEqual($label, 'zzz'), Query::orderDesc($label), Query::orderAsc('total')])), + 'order' => fn (): array => \array_map($pair, $database->find(self::ID_ORDERS, [$join, Query::select(['total', $label]), Query::orderDesc($label), Query::orderAsc('total')])), + 'wildcard' => fn (): array => \array_map(static fn (Document $row): mixed => $row->getAttribute($label), $database->find(self::ID_ORDERS, [$join, Query::select(['total', $alias.'.*']), Query::orderAsc('total'), Query::orderAsc($label)])), + 'count' => fn (): int => $database->count(self::ID_ORDERS, [$join]), + 'count filtered' => fn (): int => $database->count(self::ID_ORDERS, [$join, Query::isNotNull($label)]), + 'sum' => fn (): mixed => $database->sum(self::ID_ORDERS, 'total', [$join]), + 'page' => function () use ($database, $join, $alias, $label, $pair): array { + $queries = [$join, Query::select(['total', $label, $alias.'.$id']), Query::orderAsc('total'), Query::orderAsc($label)]; + $first = $database->find(self::ID_ORDERS, [...$queries, Query::limit(1)]); + + return \array_map($pair, $first === [] ? [] : $database->find(self::ID_ORDERS, [...$queries, Query::cursorAfter($first[0]), Query::limit(2)])); + }, + 'group' => function () use ($database, $join, $label): array { + $groups = \array_map( + static fn (array $row): array => [$row[$label] ?? null, $row['cnt'] ?? null, $row['spent'] ?? null], + $database->aggregate(self::ID_ORDERS, [$join, Query::count('*', 'cnt'), Query::sum('total', 'spent'), Query::groupBy([$label]), Query::having([Query::greaterThan('cnt', 0)])]), + ); + \sort($groups); + + return $groups; + }, + 'aggregate a joined column' => fn (): array => \array_map( + static fn (array $row): array => [$row['labels'] ?? null, $row['rows'] ?? null], + $database->aggregate(self::ID_ORDERS, [$join, Query::countDistinct($label, 'labels'), Query::count('*', 'rows')]), + ), + ]; + + $shapes = []; + foreach ($reads as $name => $read) { + try { + $shapes[$name] = $read(); + } catch (QueryException $error) { + $shapes[$name] = 'refused: '.\str_replace($alias, '@', $error->getMessage()); + } catch (Throwable $error) { + $this->fail("{$name} under {$alias} failed: ".$error::class.': '.$error->getMessage()); + } + } + + return $shapes; + } + + private function assertAliasIdRefused(string $message, Closure $read, string $case): void + { + try { + $read(); + $this->fail($case.': the join ran instead of being refused with "'.$message.'"'); + } catch (QueryException $error) { + $this->assertStringContainsString($message, $error->getMessage(), $case); + } + } + + /** + * @return list + */ + private function aliasIdCollections(): array + { + return [self::ID_ORDERS, self::ID_ITEMS, self::ID_UNIQUE, self::ID_HYPHEN, self::ID_LONG]; + } + + private function createAliasIdFixtures(Database $database): void + { + $this->cleanupAggCollections($database, $this->aliasIdCollections()); + + $database->createCollection(Collection::create( + id: self::ID_ORDERS, + attributes: [Attribute::string(key: 'customerId', size: 32), Attribute::integer(key: 'total')], + permissions: [Permission::create(Role::any()), Permission::read(Role::any())], + documentSecurity: false, + )); + foreach ([['o1', 'ann', 10], ['o2', 'bob', 20], ['o3', 'nobody', 30]] as [$id, $customer, $total]) { + $database->createDocument(self::ID_ORDERS, new Document(['$id' => $id, 'customerId' => $customer, 'total' => $total])); + } + + foreach ([self::ID_ITEMS => 'label', self::ID_UNIQUE => 'label', self::ID_HYPHEN => 'label', self::ID_LONG => self::KEY_LONG] as $table => $key) { + $database->createCollection(Collection::create( + id: $table, + attributes: [ + Attribute::string(key: 'customerId', size: 32), + Attribute::string(key: $key, size: 32), + Attribute::integer(key: self::KEY_LONG_NUMBER, required: false), + Attribute::string(key: self::KEY_SIXTY, size: 32, required: false), + ], + permissions: [Permission::create(Role::any())], + documentSecurity: true, + )); + foreach ([['i1', 'ann', 'first', Role::any()], ['i2', 'bob', 'second', Role::any()], ['i3', 'ann', 'hidden', Role::user('someone-else')]] as [$id, $customer, $value, $reader]) { + $database->createDocument($table, new Document([ + '$id' => $id, + 'customerId' => $customer, + $key => $value, + self::KEY_LONG_NUMBER => $value === 'second' ? 3 : 2, + self::KEY_SIXTY => $value, + '$permissions' => [Permission::read($reader)], + ])); + } + } + } +} diff --git a/tests/e2e/Adapter/Scopes/JoinAliasTests.php b/tests/e2e/Adapter/Scopes/JoinAliasTests.php index a60f62ad5..28415defc 100644 --- a/tests/e2e/Adapter/Scopes/JoinAliasTests.php +++ b/tests/e2e/Adapter/Scopes/JoinAliasTests.php @@ -180,14 +180,14 @@ public function testJoinWithoutAliasRefusesATableIdThatIsNotAnAlias(): void $this->createAliasFixtures($database); try { - foreach ([self::ALIAS_HYPHEN, self::ALIAS_DOT] as $table) { + foreach ([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", + "Table ID \"{$table}\" cannot be used as the join alias: an alias must start with a letter, a digit or an underscore and contain only letters, digits, underscores and hyphens. 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", + "Table ID \"{$table}\" cannot be used as the join alias: an alias must start with a letter, a digit or an underscore and contain only letters, digits, underscores and hyphens. Pass an alias for the join", fn (): mixed => $database->find(self::ALIAS_ORDERS, [Query::crossJoin($table, '')]), $table.' cross join', ); @@ -199,9 +199,15 @@ public function testJoinWithoutAliasRefusesATableIdThatIsNotAnAlias(): void $this->assertSame(['first'], \array_map(static fn (Document $row): mixed => $row->getAttribute('item.label'), $rows), $table.' with an alias'); } + $rows = $database->find(self::ALIAS_ORDERS, [ + Query::join(self::ALIAS_HYPHEN, '', [Query::on('customerId', 'customerId')]), + Query::select(['total', self::ALIAS_HYPHEN.'.label']), + ]); + $this->assertSame(['first'], \array_map(static fn (Document $row): mixed => $row->getAttribute(self::ALIAS_HYPHEN.'.label'), $rows), 'a hyphenated table ID is 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')])]), + 'Join alias must start with a letter, a digit or an underscore and contain only letters, digits, underscores and hyphens', + 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 { @@ -325,7 +331,7 @@ public function testJoinWithoutAliasKeepsTheTableIdAHookRewrites(): void } $this->createAliasFixtures($database); - $hook = new JoinTables(['customers' => self::ALIAS_CUSTOMERS, 'public-customers' => self::ALIAS_CUSTOMERS]); + $hook = new JoinTables(['customers' => self::ALIAS_CUSTOMERS, 'public.customers' => self::ALIAS_CUSTOMERS]); $database->addHook($hook); try { @@ -339,8 +345,8 @@ public function testJoinWithoutAliasKeepsTheTableIdAHookRewrites(): void $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')])]), + 'Table ID "public.customers" cannot be used as the join alias: an alias must start with a letter, a digit or an underscore and contain only letters, digits, underscores and hyphens. 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); diff --git a/tests/unit/Hook/ColumnNameTest.php b/tests/unit/Hook/ColumnNameTest.php index d475beb30..83effb93e 100644 --- a/tests/unit/Hook/ColumnNameTest.php +++ b/tests/unit/Hook/ColumnNameTest.php @@ -130,6 +130,31 @@ public function testPermissionFilterWithColumnsMatchesThemOrCollectionWidePermis $this->assertSame(['any', 'users', 'read', 'title', 'body'], $condition->bindings); } + /** + * The document column is quoted, so it may be a join alias shaped like an Appwrite ID. + */ + public function testPermissionFilterQuotesADocumentColumnUnderAnAppwriteIdAlias(): void + { + $permissionsTable = static fn (string $table): string => $table.'_perms'; + + foreach (['6ad1e9ed691819ab52ee' => '`6ad1e9ed691819ab52ee`.`_uid`', 'order-items' => '`order-items`.`_uid`'] as $alias => $quoted) { + $condition = (new Filter(['any'], $permissionsTable, documentColumn: $alias.'._uid'))->filter('posts'); + + $this->assertStringStartsWith($quoted.' IN (SELECT ', $condition->expression, $alias); + } + + $postgres = (new Filter(['any'], $permissionsTable, documentColumn: '6ad1e9ed691819ab52ee._uid', quoteCharacter: '"'))->filter('posts'); + $this->assertStringStartsWith('"6ad1e9ed691819ab52ee"."_uid" IN (SELECT ', $postgres->expression); + } + + public function testPermissionFilterKeepsUnquotedColumnsToPlainIdentifiers(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid column name: 1document'); + + new Filter(['any'], static fn (string $table): string => $table.'_perms', permissionDocumentColumn: '1document'); + } + public function testWrapAcceptsADigitOrHyphenLeadingColumnAndQuotesIt(): void { $this->assertSame('(x = 1 OR `1db`.`_uid` IS NULL)', AllowNull::wrap(new Condition('x = 1'), '1db._uid')->expression); diff --git a/tests/unit/JoinAliasTest.php b/tests/unit/JoinAliasTest.php index a984d9e32..41698a89c 100644 --- a/tests/unit/JoinAliasTest.php +++ b/tests/unit/JoinAliasTest.php @@ -166,7 +166,10 @@ public static function collidingAliases(): iterable Query::crossJoin('c', 'x'), ]]; yield 'an alias that is not an identifier' => [[ - Query::join('b', 'my-alias', [Query::on('$id', 'mainId')]), + Query::join('b', '-alias', [Query::on('$id', 'mainId')]), + ]]; + yield 'an alias with a dot' => [[ + Query::join('b', 'my.alias', [Query::on('$id', 'mainId')]), ]]; yield 'a collection joined twice without aliases' => [[ Query::join('b', '', [Query::on('$id', 'mainId')]), diff --git a/tests/unit/Validator/Query/JoinAliasTest.php b/tests/unit/Validator/Query/JoinAliasTest.php index 63bebb75a..a34848567 100644 --- a/tests/unit/Validator/Query/JoinAliasTest.php +++ b/tests/unit/Validator/Query/JoinAliasTest.php @@ -33,6 +33,13 @@ public static function acceptedJoins(): iterable 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 unique ID that starts with a digit' => [Query::join('6ad1e9ed691819ab52ee', '', [Query::on('$id', '6ad1e9ed691819ab52ee.customerId')])]; + yield 'no alias on an ID with a hyphen' => [Query::leftJoin('order-items', '', [Query::on('$id', 'order-items.customerId')])]; + yield 'no alias on a 36 character ID' => [Query::crossJoin('a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8', '')]; + yield 'an alias that starts with a digit' => [Query::join('orders', '6ad1e9ed691819ab52ef', [Query::on('$id', '6ad1e9ed691819ab52ef.customerId')])]; + yield 'an alias with a hyphen' => [Query::join('orders', 'order-lines', [Query::on('$id', 'customerId')])]; + yield 'a digit alias' => [Query::crossJoin('orders', '0')]; + yield 'an alias of 63 characters' => [Query::join('orders', \str_repeat('a', 63), [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')])]; @@ -55,14 +62,17 @@ public static function rejectedJoins(): iterable { $main = Query::DEFAULT_ALIAS; $upper = \strtoupper(Query::DEFAULT_ALIAS); - $invalid = 'Join alias must start with a letter or an underscore and contain only letters, digits and underscores'; + $invalid = 'Join alias must start with a letter, a digit or an underscore and contain only letters, digits, underscores and hyphens'; 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"]; yield 'the main collection alias on a nested join' => [Query::fullOuterJoin('orders', $main, [Query::on('$id', 'customerId')]), "Join alias \"{$main}\" is reserved for the main collection"]; - yield 'a hyphen' => [Query::join('orders', 'my-alias', [Query::on('$id', 'customerId')]), $invalid]; - yield 'a leading digit' => [Query::leftJoin('orders', '1st', [Query::on('$id', 'customerId')]), $invalid]; + yield 'a leading hyphen' => [Query::join('orders', '-alias', [Query::on('$id', 'customerId')]), $invalid]; + yield 'a space' => [Query::leftJoin('orders', 'my alias', [Query::on('$id', 'customerId')]), $invalid]; + yield 'a double quote' => [Query::leftJoin('orders', 'x"y', [Query::on('$id', 'customerId')]), $invalid]; + yield 'a dollar' => [Query::leftJoin('orders', '$id', [Query::on('$id', 'customerId')]), $invalid]; + yield 'past 63 characters' => [Query::join('orders', \str_repeat('a', 64), [Query::on('$id', 'customerId')]), 'Join alias must be at most 63 characters']; yield 'a dot' => [Query::join('orders', 'a.b', [Query::on('$id', 'customerId')]), $invalid]; yield 'a quote' => [Query::crossJoin('orders', 'x`y'), $invalid]; } @@ -84,15 +94,15 @@ 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"; + $pattern = static fn (string $table): string => "Table ID \"{$table}\" cannot be used as the join alias: an alias must start with a letter, a digit or an underscore and contain only letters, digits, underscores and hyphens. 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 'a leading hyphen' => [Query::rightJoin('-orders', '', [Query::on('$id', 'customerId')]), $pattern('-orders')]; + yield 'a dot on a cross join' => [Query::crossJoin('orders.v2', ''), $pattern('orders.v2')]; + yield 'a dot on a join built without an alias' => [new Query(Method::FullOuterJoin, 'orders.v2', [Query::on('$id', 'customerId')]), $pattern('orders.v2')]; + yield 'a dot on a parsed join' => [Query::parseQuery(['method' => 'join', 'attribute' => 'orders.v2', 'values' => ['$id', '=', 'customerId']]), $pattern('orders.v2')]; + yield 'past 63 characters' => [Query::join(\str_repeat('a', 64), '', [Query::on('$id', 'customerId')]), 'Table ID "'.\str_repeat('a', 64).'" cannot be used as the join alias: an alias must be at most 63 characters. Pass an alias for the join']; 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)]; }