From e704e96290822db12744bda75b0a003bb2f9e5c3 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 16 Jun 2025 11:48:27 -0400 Subject: [PATCH] Revert "Feat: Lazy-load relationships" --- .github/workflows/tests.yml | 6 +- app/controllers/api/databases.php | 72 ++++------ app/controllers/general.php | 9 -- src/Appwrite/Utopia/Request/Filter.php | 41 ------ src/Appwrite/Utopia/Request/Filters/V19.php | 2 +- src/Appwrite/Utopia/Request/Filters/V20.php | 124 ------------------ src/Appwrite/Utopia/Response/Filters/V20.php | 14 -- .../e2e/Services/Databases/DatabasesBase.php | 37 +----- .../Databases/DatabasesCustomServerTest.php | 52 +------- 9 files changed, 38 insertions(+), 319 deletions(-) delete mode 100644 src/Appwrite/Utopia/Request/Filters/V20.php delete mode 100644 src/Appwrite/Utopia/Response/Filters/V20.php diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ff548f3447..97f3696e67 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -122,7 +122,7 @@ jobs: docker load --input /tmp/${{ env.IMAGE }}.tar docker compose up -d sleep 10 - + - name: Wait for Open Runtimes timeout-minutes: 3 run: | @@ -273,7 +273,7 @@ jobs: export _APP_DATABASE_SHARED_TABLES=database_db_main export _APP_DATABASE_SHARED_TABLES_V1= fi - + docker compose exec -T \ -e _APP_DATABASE_SHARED_TABLES \ -e _APP_DATABASE_SHARED_TABLES_V1 \ @@ -359,4 +359,4 @@ jobs: docker compose exec -T \ -e _APP_DATABASE_SHARED_TABLES \ -e _APP_DATABASE_SHARED_TABLES_V1 \ - appwrite test /usr/src/code/tests/e2e/Services/Projects --debug --group=devKeys \ No newline at end of file + appwrite test /usr/src/code/tests/e2e/Services/Projects --debug --group=devKeys diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index 31ab002df2..0e85171772 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -3649,20 +3649,8 @@ App::get('/v1/databases/:databaseId/collections/:collectionId/documents') $cursor->setValue($cursorDocument); } - - $selectQueries = []; - try { - $selectQueries = Query::groupByType($queries)['selections'] ?? []; - - if (! empty($selectQueries)) { - // has selects, allow relationship on documents! - $documents = $dbForProject->find('database_' . $database->getSequence() . '_collection_' . $collection->getSequence(), $queries); - } else { - // has no selects, disable relationship looping on documents! - $documents = $dbForProject->skipRelationships(fn () => $dbForProject->find('database_' . $database->getSequence() . '_collection_' . $collection->getSequence(), $queries)); - } - + $documents = $dbForProject->find('database_' . $database->getSequence() . '_collection_' . $collection->getSequence(), $queries); $total = $dbForProject->count('database_' . $database->getSequence() . '_collection_' . $collection->getSequence(), $queries, APP_LIMIT_COUNT); } catch (OrderException $e) { throw new Exception(Exception::DATABASE_QUERY_ORDER_NULL, "The order attribute '{$e->getAttribute()}' had a null value. Cursor pagination requires all documents order attribute values are non-null."); @@ -3735,41 +3723,29 @@ App::get('/v1/databases/:databaseId/collections/:collectionId/documents') ->addMetric(METRIC_DATABASES_OPERATIONS_READS, \max(1, $operations)) ->addMetric(str_replace('{databaseInternalId}', $database->getSequence(), METRIC_DATABASE_ID_OPERATIONS_READS), \max(1, $operations)); + $select = \array_reduce($queries, function ($result, $query) { + return $result || ($query->getMethod() === Query::TYPE_SELECT); + }, false); + // Check if the SELECT query includes $databaseId and $collectionId - $hasWildcard = false; $hasDatabaseId = false; $hasCollectionId = false; - $hasSelectQueries = !empty($selectQueries); + if ($select) { + $hasDatabaseId = \array_reduce($queries, function ($result, $query) { + return $result || ($query->getMethod() === Query::TYPE_SELECT && \in_array('$databaseId', $query->getValues())); + }, false); + $hasCollectionId = \array_reduce($queries, function ($result, $query) { + return $result || ($query->getMethod() === Query::TYPE_SELECT && \in_array('$collectionId', $query->getValues())); + }, false); + } - if ($hasSelectQueries) { - foreach ($selectQueries as $query) { - if ($query->getMethod() !== Query::TYPE_SELECT) { - continue; + if ($select) { + foreach ($documents as $document) { + if (!$hasDatabaseId) { + $document->removeAttribute('$databaseId'); } - - $values = $query->getValues(); - if (\in_array('*', $values, true)) { - $hasWildcard = true; - break; - } - - if (\in_array('$databaseId', $values, true)) { - $hasDatabaseId = true; - } - - if (\in_array('$collectionId', $values, true)) { - $hasCollectionId = true; - } - } - - if (!$hasWildcard) { - foreach ($documents as $document) { - if (!$hasDatabaseId) { - $document->removeAttribute('$databaseId'); - } - if (!$hasCollectionId) { - $document->removeAttribute('$collectionId'); - } + if (!$hasCollectionId) { + $document->removeAttribute('$collectionId'); } } } @@ -3827,14 +3803,10 @@ App::get('/v1/databases/:databaseId/collections/:collectionId/documents/:documen throw new Exception(Exception::GENERAL_QUERY_INVALID, $e->getMessage()); } - $selects = Query::groupByType($queries)['selections'] ?? []; - - if (! empty($selects)) { - // has selects, allow relationship on documents! + try { $document = $dbForProject->getDocument('database_' . $database->getSequence() . '_collection_' . $collection->getSequence(), $documentId, $queries); - } else { - // has no selects, disable relationship looping on documents! - $document = $dbForProject->skipRelationships(fn () => $dbForProject->getDocument('database_' . $database->getSequence() . '_collection_' . $collection->getSequence(), $documentId, $queries)); + } catch (QueryException $e) { + throw new Exception(Exception::GENERAL_QUERY_INVALID, $e->getMessage()); } if ($document->isEmpty()) { diff --git a/app/controllers/general.php b/app/controllers/general.php index c979f8f1b5..ba5b409a2b 100644 --- a/app/controllers/general.php +++ b/app/controllers/general.php @@ -22,13 +22,11 @@ use Appwrite\Utopia\Request\Filters\V16 as RequestV16; use Appwrite\Utopia\Request\Filters\V17 as RequestV17; use Appwrite\Utopia\Request\Filters\V18 as RequestV18; use Appwrite\Utopia\Request\Filters\V19 as RequestV19; -use Appwrite\Utopia\Request\Filters\V20 as RequestV20; use Appwrite\Utopia\Response; use Appwrite\Utopia\Response\Filters\V16 as ResponseV16; use Appwrite\Utopia\Response\Filters\V17 as ResponseV17; use Appwrite\Utopia\Response\Filters\V18 as ResponseV18; use Appwrite\Utopia\Response\Filters\V19 as ResponseV19; -use Appwrite\Utopia\Response\Filters\V20 as ResponseV20; use Appwrite\Utopia\View; use Executor\Executor; use MaxMind\Db\Reader; @@ -844,10 +842,6 @@ App::init() if (version_compare($requestFormat, '1.7.0', '<')) { $request->addFilter(new RequestV19()); } - if (version_compare($requestFormat, '1.8.0', '<')) { - $dbForProject = $getProjectDB($project); - $request->addFilter(new RequestV20($dbForProject, $route)); - } } $domain = $request->getHostname(); @@ -1017,9 +1011,6 @@ App::init() if (version_compare($responseFormat, '1.7.0', '<')) { $response->addFilter(new ResponseV19()); } - if (version_compare($responseFormat, '1.8.0', '<')) { - $response->addFilter(new ResponseV20()); - } if (version_compare($responseFormat, APP_VERSION_STABLE, '>')) { $response->addHeader('X-Appwrite-Warning', "The current SDK is built for Appwrite " . $responseFormat . ". However, the current Appwrite server version is " . APP_VERSION_STABLE . ". Please downgrade your SDK to match the Appwrite version: https://appwrite.io/docs/sdks"); } diff --git a/src/Appwrite/Utopia/Request/Filter.php b/src/Appwrite/Utopia/Request/Filter.php index 2a5fe4d394..59346c7e17 100644 --- a/src/Appwrite/Utopia/Request/Filter.php +++ b/src/Appwrite/Utopia/Request/Filter.php @@ -2,20 +2,8 @@ namespace Appwrite\Utopia\Request; -use Utopia\Database\Database; -use Utopia\Route; - abstract class Filter { - private ?Route $route; - private ?Database $dbForProject; - - public function __construct(Database $dbForProject = null, Route $route = null) - { - $this->route = $route; - $this->dbForProject = $dbForProject; - } - /** * Parse params to another format. * @@ -25,33 +13,4 @@ abstract class Filter * @return array */ abstract public function parse(array $content, string $model): array; - - /** - * Get the database for the current project. - * - * @return null|Database - */ - public function getDbForProject(): ?Database - { - return $this->dbForProject; - } - - /** - * Returns the value of the given route param key, or a default if not found or on error. - * - * @param string $key - * @param mixed $default - * - * @return mixed - */ - public function getParamValue(string $key, mixed $default = ''): mixed - { - try { - $value = $this->route?->getParamValue($key) ?? $default; - } catch (\Exception $e) { - $value = $default; - } - - return $value; - } } diff --git a/src/Appwrite/Utopia/Request/Filters/V19.php b/src/Appwrite/Utopia/Request/Filters/V19.php index e7789ac0f7..8680cd642c 100644 --- a/src/Appwrite/Utopia/Request/Filters/V19.php +++ b/src/Appwrite/Utopia/Request/Filters/V19.php @@ -39,7 +39,7 @@ class V19 extends Filter return $content; } - public function convertQueryAttribute(array $content, string $old, string $new): array + public function convertQueryAttribute(array $content, string $old, string $new) { if (isset($content['queries']) && is_array($content['queries'])) { foreach ($content['queries'] as $index => $query) { diff --git a/src/Appwrite/Utopia/Request/Filters/V20.php b/src/Appwrite/Utopia/Request/Filters/V20.php deleted file mode 100644 index 8719cf7325..0000000000 --- a/src/Appwrite/Utopia/Request/Filters/V20.php +++ /dev/null @@ -1,124 +0,0 @@ -manageSelectQueries($content, $model); - break; - } - return $content; - } - - /** - * From 1.8.x onward, related documents are no longer returned by default to improve performance. - * - * Use `Query::select(['related.*'])` for full documents or `Query::select(['related.key'])` for specific fields. - * - * This filter preserves 1.7.x behavior by including all related documents for backward compatibility with - * `listDocuments` and `getDocument` calls. - */ - protected function manageSelectQueries(array $content, string $model): array - { - $hasWildcard = false; - if (! isset($content['queries'])) { - $hasWildcard = true; - // only query, make it json encoded! - $content['queries'] = [Query::select(['*'])->toString()]; - } - - try { - $parsed = Query::parseQueries($content['queries']); - } catch (QueryException) { - // don't crash! - return $content; - } - - $selections = Query::groupByType($parsed)['selections'] ?? []; - - if (! $hasWildcard) { - // check if any select includes a wildcard as we added one above - foreach ($selections as $select) { - if (\in_array('*', $select->getValues(), true)) { - $hasWildcard = true; - break; - } - } - } - - if ($hasWildcard && $model === 'databases.listDocuments') { - $relatedKeys = $this->getRelatedCollectionKeys(); - - if (! empty($relatedKeys)) { - $selects = \array_values(\array_unique(\array_merge(['*'], $relatedKeys))); - - // remove previous select queries - $parsed = \array_filter( - $parsed, - fn ($query) => $query->getMethod() !== Query::TYPE_SELECT - ); - - // add wildcard + relationship(s) selects - $parsed[] = Query::select($selects); - } - } - - $resolvedQueries = []; - foreach ($parsed as $query) { - // make em json encoded! - $resolvedQueries[] = $query->toString(); - } - - $content['queries'] = $resolvedQueries; - - return $content; - } - - /** - * Returns all relationship attribute keys in `key.*` format for use with `Query::select`. - */ - private function getRelatedCollectionKeys(): array - { - $dbForProject = $this->getDbForProject(); - - if ($dbForProject === null) { - return []; - } - - $databaseId = $this->getParamValue('databaseId'); - $collectionId = $this->getParamValue('collectionId'); - - if (empty($databaseId) || empty($collectionId)) { - return []; - } - - $database = Authorization::skip(fn () => $dbForProject->getDocument('databases', $databaseId)); - - $collection = $dbForProject->getDocument( - 'database_' . $database->getSequence(), - $collectionId - ); - - $attributes = $collection->getAttribute('attributes', []); - - return \array_values(\array_map( - fn ($attr) => $attr['key'] . '.*', - \array_filter( - $attributes, - fn ($attr) => ($attr['type'] ?? null) === Database::VAR_RELATIONSHIP - ) - )); - } -} diff --git a/src/Appwrite/Utopia/Response/Filters/V20.php b/src/Appwrite/Utopia/Response/Filters/V20.php deleted file mode 100644 index c0ed8297cc..0000000000 --- a/src/Appwrite/Utopia/Response/Filters/V20.php +++ /dev/null @@ -1,14 +0,0 @@ -client->call(Client::METHOD_GET, '/databases/' . $databaseId . '/collections/' . $personCollection . '/documents/' . $person2['body']['$id'], array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), [ - 'queries' => [ - Query::select(['*', 'libraries.*'])->toString() - ] - ]); + ], $this->getHeaders())); $this->assertEquals(200, $response['headers']['status-code']); $this->assertArrayNotHasKey('$collection', $response['body']); @@ -4568,11 +4564,7 @@ trait DatabasesBase $response = $this->client->call(Client::METHOD_GET, '/databases/' . $databaseId . '/collections/' . $libraryCollection . '/documents/library11', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), [ - 'queries' => [ - Query::select(['person_one_to_many.$id'])->toString() - ] - ]); + ], $this->getHeaders())); $this->assertEquals(200, $response['headers']['status-code']); $this->assertArrayHasKey('person_one_to_many', $response['body']); @@ -4722,11 +4714,7 @@ trait DatabasesBase $album = $this->client->call(Client::METHOD_GET, '/databases/' . $databaseId . '/collections/' . $albums['body']['$id'] . '/documents/album1', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), [ - 'queries' => [ - Query::select(['*', 'artist.name', 'artist.$permissions'])->toString() - ] - ]); + ], $this->getHeaders())); $this->assertEquals(200, $album['headers']['status-code']); $this->assertEquals('album1', $album['body']['$id']); @@ -4738,11 +4726,7 @@ trait DatabasesBase $artist = $this->client->call(Client::METHOD_GET, '/databases/' . $databaseId . '/collections/' . $artists['body']['$id'] . '/documents/' . $album['body']['artist']['$id'], array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), [ - 'queries' => [ - Query::select(['*', 'albums.$id', 'albums.name', 'albums.$permissions'])->toString() - ] - ]); + ], $this->getHeaders())); $this->assertEquals(200, $artist['headers']['status-code']); $this->assertEquals('Artist 1', $artist['body']['name']); @@ -4883,11 +4867,7 @@ trait DatabasesBase $sport = $this->client->call(Client::METHOD_GET, '/databases/' . $databaseId . '/collections/' . $sports['body']['$id'] . '/documents/sport1', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), [ - 'queries' => [ - Query::select(['*', 'players.name', 'players.$permissions'])->toString() - ] - ]); + ], $this->getHeaders())); $this->assertEquals(200, $sport['headers']['status-code']); $this->assertEquals('sport1', $sport['body']['$id']); @@ -4901,11 +4881,7 @@ trait DatabasesBase $player = $this->client->call(Client::METHOD_GET, '/databases/' . $databaseId . '/collections/' . $players['body']['$id'] . '/documents/' . $sport['body']['players'][0]['$id'], array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), [ - 'queries' => [ - Query::select(['*', 'sports.$id', 'sports.name', 'sports.$permissions'])->toString() - ] - ]); + ], $this->getHeaders())); $this->assertEquals(200, $player['headers']['status-code']); $this->assertEquals('Player 1', $player['body']['name']); @@ -4933,7 +4909,6 @@ trait DatabasesBase ], $this->getHeaders()), [ 'queries' => [ Query::isNotNull('$id')->toString(), - Query::select(['*', 'libraries.*'])->toString(), Query::startsWith('fullName', 'Stevie')->toString(), Query::endsWith('fullName', 'Wonder')->toString(), Query::between('$createdAt', '1975-12-06', '2050-12-01')->toString(), diff --git a/tests/e2e/Services/Databases/DatabasesCustomServerTest.php b/tests/e2e/Services/Databases/DatabasesCustomServerTest.php index 752690cc10..e84c760c86 100644 --- a/tests/e2e/Services/Databases/DatabasesCustomServerTest.php +++ b/tests/e2e/Services/Databases/DatabasesCustomServerTest.php @@ -3779,11 +3779,7 @@ class DatabasesCustomServerTest extends Scope 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] - ]), [ - 'queries' => [ - Query::select(['new_level_2.*'])->toString() - ] - ]); + ])); $this->assertArrayHasKey('new_level_2', $newDocument['body']); $this->assertEquals(1, count($newDocument['body']['new_level_2'])); @@ -3893,11 +3889,7 @@ class DatabasesCustomServerTest extends Scope 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] - ]), [ - 'queries' => [ - Query::select(['new_level_2.*'])->toString() - ] - ]); + ])); $this->assertArrayHasKey('new_level_2', $newDocument['body']); $this->assertNotEmpty($newDocument['body']['new_level_2']); @@ -4007,11 +3999,7 @@ class DatabasesCustomServerTest extends Scope 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] - ]), [ - 'queries' => [ - Query::select(['new_level_2.*'])->toString() - ] - ]); + ])); $this->assertArrayHasKey('new_level_2', $newDocument['body']); $this->assertNotEmpty($newDocument['body']['new_level_2']); @@ -4022,11 +4010,7 @@ class DatabasesCustomServerTest extends Scope 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] - ]), [ - 'queries' => [ - Query::select(['*', 'level1.*'])->toString() - ] - ]); + ])); $this->assertArrayHasKey('level1', $level2Document['body']); $this->assertNotEmpty($level2Document['body']['level1']); @@ -4125,11 +4109,7 @@ class DatabasesCustomServerTest extends Scope 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] - ]), [ - 'queries' => [ - Query::select(['new_level_2.*'])->toString() - ] - ]); + ])); $this->assertArrayHasKey('new_level_2', $newDocument['body']); $this->assertNotEmpty($newDocument['body']['new_level_2']); @@ -4140,11 +4120,7 @@ class DatabasesCustomServerTest extends Scope 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], 'x-appwrite-key' => $this->getProject()['apiKey'] - ]), [ - 'queries' => [ - Query::select(['*', 'level1.*'])->toString() - ] - ]); + ])); $this->assertArrayHasKey('level1', $level2Document['body']); $this->assertNotEmpty($level2Document['body']['level1']); @@ -4489,14 +4465,6 @@ class DatabasesCustomServerTest extends Scope $createBulkDocuments(); - /** - * Wait for database to purge cache... - * - * This test specifically failed on 1.6.x response format, - * could be due to the slow or overworked machine, but being safe here! - */ - sleep(5); - // TEST: Update all documents $response = $this->client->call(Client::METHOD_PATCH, '/databases/' . $data['databaseId'] . '/collections/' . $data['$id'] . '/documents', array_merge([ 'content-type' => 'application/json', @@ -4515,14 +4483,6 @@ class DatabasesCustomServerTest extends Scope $this->assertEquals(200, $response['headers']['status-code']); $this->assertCount(10, $response['body']['documents']); - /** - * Wait for database to purge cache... - * - * This test specifically failed on 1.6.x response format, - * could be due to the slow or overworked machine, but being safe here! - */ - sleep(5); - $documents = $this->client->call(Client::METHOD_GET, '/databases/' . $data['databaseId'] . '/collections/' . $data['$id'] . '/documents', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'],