Skip to content

Commit ba8a5fe

Browse files
abnegateabnegate
authored
Merge pull request #1023 from utopia-php/fix-order-columnless-relationship
fix(query): refuse ordering by a relationship that holds no column
2 parents 0555ec9 + 94f99ee commit ba8a5fe

2 files changed

Lines changed: 133 additions & 0 deletions

File tree

‎src/Database/Validator/Query/Order.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Utopia\Database\Document;
77
use Utopia\Database\Query;
88
use Utopia\Database\Validator\Query\Joined\Attributes;
9+
use Utopia\Database\Validator\Query\Joined\Collection;
910
use Utopia\Query\Method;
1011
use Utopia\Query\Query as BaseQuery;
1112

@@ -29,6 +30,14 @@ class Order extends Base
2930
*/
3031
protected array $aggregationAliases = [];
3132

33+
/**
34+
* The relationships whose related ids live on the other collection or in a junction table, so this collection
35+
* holds no column to order by.
36+
*
37+
* @var array<string, true>
38+
*/
39+
private array $columnless = [];
40+
3241
private bool $aggregates = false;
3342

3443
/**
@@ -48,6 +57,12 @@ public function __construct(
4857
$attribute = $attribute instanceof Attribute ? $attribute : Attribute::fromDocument($attribute);
4958
$this->schema[$attribute->key] = true;
5059
}
60+
61+
foreach (Collection::columns($attributes) as $key => $holdsColumn) {
62+
if (! $holdsColumn) {
63+
$this->columnless[$key] = true;
64+
}
65+
}
5166
}
5267

5368
protected function isValidAttribute(string $attribute): bool
@@ -109,6 +124,12 @@ public function isValid(mixed $value): bool
109124
$attribute = $value->getAttribute();
110125

111126
if ($method === Method::OrderAsc || $method === Method::OrderDesc) {
127+
if (isset($this->columnless[$attribute])) {
128+
$this->message = 'Cannot order by relationship attribute: '.$attribute;
129+
130+
return false;
131+
}
132+
112133
return $this->isValidAttribute($attribute) && $this->isGroupedOrder($attribute);
113134
}
114135

‎tests/unit/RelationshipOrderTest.php‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
<?php
2+
3+
namespace Tests\Unit;
4+
5+
use PDO;
6+
use PHPUnit\Framework\Attributes\DataProvider;
7+
use PHPUnit\Framework\TestCase;
8+
use Throwable;
9+
use Utopia\Cache\Adapter\None;
10+
use Utopia\Cache\Cache;
11+
use Utopia\Database\Adapter\SQLite;
12+
use Utopia\Database\Attribute;
13+
use Utopia\Database\Collection;
14+
use Utopia\Database\Database;
15+
use Utopia\Database\Document;
16+
use Utopia\Database\Exception\Query as QueryException;
17+
use Utopia\Database\Permission;
18+
use Utopia\Database\Query;
19+
use Utopia\Database\Relationship;
20+
use Utopia\Database\Role;
21+
22+
/**
23+
* A one-to-many parent, a many-to-one child and either side of a many-to-many keep their related ids on the other
24+
* collection or in a junction table, so their collection has no column to order by.
25+
*/
26+
final class RelationshipOrderTest extends TestCase
27+
{
28+
private Database $database;
29+
30+
#[\Override]
31+
protected function setUp(): void
32+
{
33+
$this->database = new Database(new SQLite(new PDO('sqlite::memory:')), new Cache(new None()));
34+
$this->database
35+
->setDatabase('relationship_order')
36+
->setNamespace('relationship_order_'.\uniqid());
37+
$this->database->getAuthorization()->addRole(Role::any()->toString());
38+
$this->database->create();
39+
40+
$permissions = [Permission::create(Role::any()), Permission::read(Role::any())];
41+
foreach (['authors', 'books', 'tags', 'covers', 'shelves'] as $collection) {
42+
$this->database->createCollection(Collection::create(
43+
id: $collection,
44+
attributes: [Attribute::string(key: 'name', size: 16)],
45+
permissions: $permissions,
46+
));
47+
}
48+
49+
$this->database->createRelationship('authors', Relationship::oneToMany(relatedCollection: 'books', key: 'books', twoWay: true, twoWayKey: 'author'));
50+
$this->database->createRelationship('books', Relationship::manyToMany(relatedCollection: 'tags', key: 'tags', twoWay: true, twoWayKey: 'books'));
51+
$this->database->createRelationship('books', Relationship::oneToOne(relatedCollection: 'covers', key: 'cover', twoWay: true, twoWayKey: 'book'));
52+
$this->database->createRelationship('books', Relationship::manyToOne(relatedCollection: 'shelves', key: 'shelf', twoWay: true, twoWayKey: 'books'));
53+
54+
$this->database->createDocument('authors', new Document(['$id' => 'ann', 'name' => 'Ann', '$permissions' => [Permission::read(Role::any())]]));
55+
$this->database->createDocument('books', new Document(['$id' => 'b1', 'name' => 'One', 'author' => 'ann', '$permissions' => [Permission::read(Role::any())]]));
56+
}
57+
58+
/**
59+
* @return iterable<string, array{string, string}>
60+
*/
61+
public static function columnless(): iterable
62+
{
63+
yield 'one-to-many parent' => ['authors', 'books'];
64+
yield 'many-to-many parent' => ['books', 'tags'];
65+
yield 'many-to-many child' => ['tags', 'books'];
66+
yield 'many-to-one child' => ['shelves', 'books'];
67+
}
68+
69+
#[DataProvider('columnless')]
70+
public function testOrderByARelationshipWithoutAColumnIsAnInvalidQuery(string $collection, string $attribute): void
71+
{
72+
foreach ([Query::orderAsc($attribute), Query::orderDesc($attribute)] as $order) {
73+
foreach ([
74+
'find()' => fn (): mixed => $this->database->find($collection, [$order]),
75+
'count()' => fn (): mixed => $this->database->count($collection, [$order]),
76+
] as $read => $call) {
77+
$error = null;
78+
try {
79+
$call();
80+
} catch (Throwable $caught) {
81+
$error = $caught;
82+
}
83+
84+
$this->assertInstanceOf(QueryException::class, $error, $read.' '.$order->getMethod()->value.': '.($error === null ? 'accepted' : $error::class.': '.$error->getMessage()));
85+
$this->assertSame('Invalid query: Cannot order by relationship attribute: '.$attribute, $error->getMessage(), $read);
86+
}
87+
}
88+
}
89+
90+
/**
91+
* @return iterable<string, array{string, string, list<string>}>
92+
*/
93+
public static function stored(): iterable
94+
{
95+
yield 'one-to-many child' => ['books', 'author', ['b1']];
96+
yield 'one-to-one parent' => ['books', 'cover', ['b1']];
97+
yield 'two-way one-to-one child' => ['covers', 'book', []];
98+
yield 'many-to-one parent' => ['books', 'shelf', ['b1']];
99+
}
100+
101+
/**
102+
* @param list<string> $ids
103+
*/
104+
#[DataProvider('stored')]
105+
public function testOrderByARelationshipThatHoldsAColumnIsRead(string $collection, string $attribute, array $ids): void
106+
{
107+
foreach ([Query::orderAsc($attribute), Query::orderDesc($attribute)] as $order) {
108+
$documents = $this->database->find($collection, [$order]);
109+
$this->assertSame($ids, \array_map(static fn (Document $document): string => $document->getId(), $documents));
110+
}
111+
}
112+
}

0 commit comments

Comments
 (0)