From c315240bce37e7ce5121f9152fe563ee994c2b46 Mon Sep 17 00:00:00 2001 From: fogelito Date: Wed, 1 Mar 2023 15:56:16 +0200 Subject: [PATCH] addressing comments --- app/config/errors.php | 8 +- app/controllers/api/databases.php | 35 ++--- app/init.php | 1 + .../databases/delete-slow-query-document.md | 1 + docs/references/databases/get-slow-queries.md | 1 + docs/references/databases/get-slow-query.md | 1 + src/Appwrite/Extend/Exception.php | 5 +- .../Database/Validator/IndexedQueries.php | 3 + .../e2e/Services/Databases/DatabasesBase.php | 118 ++++++---------- .../Databases/DatabasesConsoleClientTest.php | 132 +++++++++++++++++- 10 files changed, 195 insertions(+), 110 deletions(-) create mode 100644 docs/references/databases/delete-slow-query-document.md create mode 100644 docs/references/databases/get-slow-queries.md create mode 100644 docs/references/databases/get-slow-query.md diff --git a/app/config/errors.php b/app/config/errors.php index 5caab69edb..fdc88850f6 100644 --- a/app/config/errors.php +++ b/app/config/errors.php @@ -559,13 +559,13 @@ return [ 'description' => 'Too many queries.', 'code' => 400, ], - Exception::TIMEOUT_BLOCKED => [ - 'name' => Exception::TIMEOUT_BLOCKED, + Exception::QUERY_BLOCKED => [ + 'name' => Exception::QUERY_BLOCKED, 'description' => 'Api call has been blocked after exceeding maximum hits of slow query exception', 'code' => 403, ], - Exception::TIMEOUT => [ - 'name' => Exception::TIMEOUT, + Exception::QUERY_TIMEOUT => [ + 'name' => Exception::QUERY_TIMEOUT, 'description' => 'Query exceeded the maximum, the request will eventually be blocked if it keeps timing out, try adjusting the queries parameter and improve indexing', 'code' => 408, ], diff --git a/app/controllers/api/databases.php b/app/controllers/api/databases.php index 6cfafa572c..2a3acb2106 100644 --- a/app/controllers/api/databases.php +++ b/app/controllers/api/databases.php @@ -150,7 +150,6 @@ function createAttribute(string $databaseId, string $collectionId, Document $att return $attribute; } - App::init() ->groups(['timeout']) ->inject('request') @@ -164,7 +163,7 @@ App::init() /** @var Document $document */ $document = Authorization::skip(fn() => $dbForProject->getDocument('slowQueries', $key)); if ($document->getAttribute('blocked') === true) { - throw new Exception(Exception::TIMEOUT_BLOCKED); + throw new Exception(Exception::QUERY_BLOCKED); } }); @@ -223,10 +222,10 @@ App::error() } if ($document->getAttribute('blocked') === true) { - throw new Exception(Exception::TIMEOUT_BLOCKED); + throw new Exception(Exception::QUERY_BLOCKED); } - throw new Exception(Exception::TIMEOUT); + throw new Exception(Exception::QUERY_TIMEOUT); } throw $error; @@ -235,7 +234,6 @@ App::error() } }); - App::post('/v1/databases') ->desc('Create Database') ->groups(['api', 'database']) @@ -2888,6 +2886,7 @@ App::get('/v1/databases/:databaseId/slow-queries') ->label('sdk.auth', [APP_AUTH_TYPE_ADMIN]) ->label('sdk.namespace', 'databases') ->label('sdk.method', 'listSlowQueries') + ->label('sdk.description', '/docs/references/databases/list-slow-queries-documents.md') ->label('sdk.response.code', Response::STATUS_CODE_OK) ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_DOCUMENT_LIST) @@ -2926,15 +2925,13 @@ App::get('/v1/databases/slow-queries/:documentId') ->label('sdk.auth', [APP_AUTH_TYPE_ADMIN]) ->label('sdk.namespace', 'databases') ->label('sdk.method', 'getSlowQuery') - ->label('sdk.description', '/docs/references/databases/get-document.md') + ->label('sdk.description', '/docs/references/databases/get-slow-query.md') ->label('sdk.response.code', Response::STATUS_CODE_OK) ->label('sdk.response.type', Response::CONTENT_TYPE_JSON) ->label('sdk.response.model', Response::MODEL_DOCUMENT) ->param('documentId', '', new UID(), 'Document ID.') ->inject('response') ->inject('dbForProject') - ->inject('mode') - ->inject('project') ->action(function (string $documentId, Response $response, Database $dbForProject) { $document = $dbForProject->getDocument('slowQueries', $documentId); if ($document->isEmpty()) { @@ -2954,17 +2951,14 @@ App::delete('/v1/databases/slow-queries/:documentId') //->label('usage.params', ['databaseId:{request.databaseId}', 'collectionId:{request.collectionId}']) ->label('sdk.auth', [APP_AUTH_TYPE_ADMIN]) ->label('sdk.namespace', 'databases') - ->label('event', 'slowQueries.documents.[documentId].delete') - ->label('audits.event', 'document.delete') - ->label('audits.resource', 'database/{request.databaseId}/collection/{request.collectionId}/document/{request.documentId}') + ->label('sdk.method', 'deleteSlowQuery') + ->label('sdk.description', '/docs/references/databases/delete-slow-query-document.md') ->label('sdk.response.code', Response::STATUS_CODE_NOCONTENT) ->label('sdk.response.model', Response::MODEL_NONE) ->param('documentId', '', new UID(), 'Document ID.') ->inject('response') ->inject('dbForProject') - ->inject('events') - ->inject('deletes') - ->action(function (string $documentId, Response $response, Database $dbForProject, Event $events, Delete $deletes) { + ->action(function (string $documentId, Response $response, Database $dbForProject) { $document = $dbForProject->getDocument('slowQueries', $documentId); if ($document->isEmpty()) { throw new Exception(Exception::DOCUMENT_NOT_FOUND); @@ -2973,18 +2967,5 @@ App::delete('/v1/databases/slow-queries/:documentId') $dbForProject->deleteDocument('slowQueries', $documentId); $dbForProject->deleteCachedDocument('slowQueries', $documentId); - $deletes - ->setType(DELETE_TYPE_AUDIT) - ->setDocument($document) - ; - - // todo: check slow Queries events - - $events - ->setParam('collectionId', 'slowQueries') - ->setParam('documentId', $document->getId()) - ->setPayload($response->output($document, Response::MODEL_DOCUMENT)) - ; - $response->noContent(); }); diff --git a/app/init.php b/app/init.php index 16913ac667..b351f49c4e 100644 --- a/app/init.php +++ b/app/init.php @@ -936,6 +936,7 @@ App::setResource('dbForProject', function ($db, $cache, Document $project) { $database = new Database(new MariaDB($db), $cache); $database->setDefaultDatabase(App::getEnv('_APP_DB_SCHEMA', 'appwrite')); $database->setNamespace("_{$project->getInternalId()}"); + return $database; }, ['db', 'cache', 'project']); diff --git a/docs/references/databases/delete-slow-query-document.md b/docs/references/databases/delete-slow-query-document.md new file mode 100644 index 0000000000..36fbf6802d --- /dev/null +++ b/docs/references/databases/delete-slow-query-document.md @@ -0,0 +1 @@ +Delete a document by its unique ID. \ No newline at end of file diff --git a/docs/references/databases/get-slow-queries.md b/docs/references/databases/get-slow-queries.md new file mode 100644 index 0000000000..242f388bed --- /dev/null +++ b/docs/references/databases/get-slow-queries.md @@ -0,0 +1 @@ +Get a list of all the slow queries documents in a given database. You can use the query params to filter your results. \ No newline at end of file diff --git a/docs/references/databases/get-slow-query.md b/docs/references/databases/get-slow-query.md new file mode 100644 index 0000000000..732ac76a55 --- /dev/null +++ b/docs/references/databases/get-slow-query.md @@ -0,0 +1 @@ +Get a slow query by its unique ID. This endpoint response returns a JSON object with the document data. \ No newline at end of file diff --git a/src/Appwrite/Extend/Exception.php b/src/Appwrite/Extend/Exception.php index cbcbf62328..44625361a6 100644 --- a/src/Appwrite/Extend/Exception.php +++ b/src/Appwrite/Extend/Exception.php @@ -183,8 +183,8 @@ class Exception extends \Exception public const GRAPHQL_TOO_MANY_QUERIES = 'graphql_too_many_queries'; /** Timeout */ - public const TIMEOUT_BLOCKED = 'timeout_blocked'; - public const TIMEOUT = 'timeout'; + public const QUERY_BLOCKED = 'query_blocked'; + public const QUERY_TIMEOUT = 'query_timeout'; protected $type = ''; @@ -200,6 +200,7 @@ class Exception extends \Exception $this->message = $message ?? $this->message; $this->code = $code ?? $this->code; + parent::__construct($this->message, $this->code, $previous); } diff --git a/src/Appwrite/Utopia/Database/Validator/IndexedQueries.php b/src/Appwrite/Utopia/Database/Validator/IndexedQueries.php index 9220771563..1cc0429018 100644 --- a/src/Appwrite/Utopia/Database/Validator/IndexedQueries.php +++ b/src/Appwrite/Utopia/Database/Validator/IndexedQueries.php @@ -106,8 +106,10 @@ class IndexedQueries extends Queries if (!$query instanceof Query) { $query = Query::parse($query); } + $queries[] = $query; } + $grouped = Query::groupByType($queries); /** @var Query[] */ $filters = $grouped['filters']; /** @var string[] */ $orderAttributes = $grouped['orderAttributes']; @@ -120,6 +122,7 @@ class IndexedQueries extends Queries } $found = null; + foreach ($this->indexes as $index) { if ($this->arrayMatch($index->getAttribute('attributes'), array_keys($filtersByAttribute))) { $found = $index; diff --git a/tests/e2e/Services/Databases/DatabasesBase.php b/tests/e2e/Services/Databases/DatabasesBase.php index baccc859a7..54b0e02e06 100644 --- a/tests/e2e/Services/Databases/DatabasesBase.php +++ b/tests/e2e/Services/Databases/DatabasesBase.php @@ -183,17 +183,6 @@ trait DatabasesBase 'required' => false, ]); - $longtext = $this->client->call(Client::METHOD_POST, '/databases/' . $databaseId . '/collections/' . $data['moviesId'] . '/attributes/string', array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - 'x-appwrite-key' => $this->getProject()['apiKey'] - ]), [ - 'key' => 'longtext', - 'size' => 100000000, - 'required' => false, - 'default' => null, - ]); - $this->assertEquals(202, $title['headers']['status-code']); $this->assertEquals($title['body']['key'], 'title'); $this->assertEquals($title['body']['type'], 'string'); @@ -222,12 +211,6 @@ trait DatabasesBase $this->assertEquals($datetime['body']['type'], 'datetime'); $this->assertEquals($datetime['body']['required'], false); - $this->assertEquals($longtext['headers']['status-code'], 202); - $this->assertEquals($longtext['body']['key'], 'longtext'); - $this->assertEquals($longtext['body']['type'], 'string'); - $this->assertEquals($longtext['body']['required'], false); - - // wait for database worker to create attributes sleep(2); @@ -238,13 +221,12 @@ trait DatabasesBase ]), []); $this->assertIsArray($movies['body']['attributes']); - $this->assertCount(6, $movies['body']['attributes']); + $this->assertCount(5, $movies['body']['attributes']); $this->assertEquals($movies['body']['attributes'][0]['key'], $title['body']['key']); $this->assertEquals($movies['body']['attributes'][1]['key'], $releaseYear['body']['key']); $this->assertEquals($movies['body']['attributes'][2]['key'], $duration['body']['key']); $this->assertEquals($movies['body']['attributes'][3]['key'], $actors['body']['key']); $this->assertEquals($movies['body']['attributes'][4]['key'], $datetime['body']['key']); - $this->assertEquals($movies['body']['attributes'][5]['key'], $longtext['body']['key']); return $data; } @@ -1004,20 +986,58 @@ trait DatabasesBase /** - * @depends testCreateDocument + * @depends testCreateDatabase + */ + public function testTimeoutCollection(array $data): array + { + $collection = $this->client->call(Client::METHOD_POST, '/databases/' . $data['databaseId'] . '/collections', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'collectionId' => ID::unique(), + 'name' => 'Slow Queries', + 'documentSecurity' => true, + 'permissions' => [ + Permission::create(Role::user($this->getUser()['$id'])), + ], + ]); + + $this->assertEquals(201, $collection['headers']['status-code']); + + $data = [ + '$id' => $collection['body']['$id'], + 'databaseId' => $collection['body']['databaseId'] + ]; + + $longtext = $this->client->call(Client::METHOD_POST, '/databases/' . $data['databaseId'] . '/collections/' . $data['$id'] . '/attributes/string', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'key' => 'longtext', + 'size' => 100000000, + 'required' => false, + 'default' => null, + ]); + + $this->assertEquals($longtext['headers']['status-code'], 202); + + return $data; + } + + /** + * @depends testTimeoutCollection */ public function testTimeouts(array $data): void { - $documents = []; for ($i = 0; $i <= 1; $i++) { - $documents[] = $this->client->call(Client::METHOD_POST, '/databases/' . $data['databaseId'] . '/collections/' . $data['moviesId'] . '/documents', array_merge([ + $this->client->call(Client::METHOD_POST, '/databases/' . $data['databaseId'] . '/collections/' . $data['$id'] . '/documents', array_merge([ 'content-type' => 'application/json', 'x-appwrite-project' => $this->getProject()['$id'], ], $this->getHeaders()), [ 'documentId' => ID::unique(), 'data' => [ - 'title' => 'title', - 'releaseYear' => 2020, 'longtext' => file_get_contents(__DIR__ . '/longtext.txt'), ], 'permissions' => [ @@ -1030,7 +1050,7 @@ trait DatabasesBase $docs = []; for ($i = 0; $i <= 2; $i++) { - $docs[] = $this->client->call(Client::METHOD_GET, '/databases/' . $data['databaseId'] . '/collections/' . $data['moviesId'] . '/documents', array_merge([ + $docs[] = $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'], 'x-appwrite-timeout' => 1, @@ -1039,59 +1059,11 @@ trait DatabasesBase ]); } - for ($i = 0; $i < count($documents); $i++) { - $this->client->call(Client::METHOD_DELETE, '/databases/' . $data['databaseId'] . '/collections/' . $data['moviesId'] . '/documents/' . $documents[$i]['body']['$id'], array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders())); - } - $this->assertEquals(408, $docs[0]['headers']['status-code']); // insert $this->assertEquals(403, $docs[1]['headers']['status-code']); // update $this->assertEquals(403, $docs[2]['headers']['status-code']); // blocked } - /** - * @depends testCreateDocument - */ - public function testConsoleTimeouts(array $data): void - { - $documents = $this->client->call(Client::METHOD_GET, '/databases/' . $data['databaseId'] . '/slow-queries', array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), [ - 'queries' => [ - 'equal("blocked", true)', - 'equal("collectionId", "' . $data['moviesId'] . '")', - 'orderAsc("$updatedAt")' - ] - ]); - - $this->assertEquals(200, $documents['headers']['status-code']); - $this->assertEquals(1, $documents['body']['total']); - $this->assertEquals(true, $documents['body']['documents'][0]['blocked']); - $this->assertEquals(2, $documents['body']['documents'][0]['count']); - - $document = $this->client->call(Client::METHOD_GET, '/databases/slow-queries/' . $documents['body']['documents'][0]['$id'], array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), []); - $this->assertEquals(200, $documents['headers']['status-code']); - $this->assertEquals($document['body']['queries'][0], 'notEqual("longtext", "appwrite")'); - - $response = $this->client->call(Client::METHOD_DELETE, '/databases/slow-queries/' . $documents['body']['documents'][0]['$id'], array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), []); - $this->assertEquals(204, $response['headers']['status-code']); - - $document = $this->client->call(Client::METHOD_GET, '/databases/slow-queries/' . $documents['body']['documents'][0]['$id'], array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), []); - $this->assertEquals(404, $document['headers']['status-code']); - } - /** * @depends testCreateDocument */ diff --git a/tests/e2e/Services/Databases/DatabasesConsoleClientTest.php b/tests/e2e/Services/Databases/DatabasesConsoleClientTest.php index ad97a2bb3e..443eec3e23 100644 --- a/tests/e2e/Services/Databases/DatabasesConsoleClientTest.php +++ b/tests/e2e/Services/Databases/DatabasesConsoleClientTest.php @@ -15,7 +15,7 @@ class DatabasesConsoleClientTest extends Scope use ProjectCustom; use SideConsole; - public function testCreateCollection(): array + public function testCreateDatabase(): array { $database = $this->client->call(Client::METHOD_POST, '/databases', array_merge([ 'content-type' => 'application/json', @@ -54,7 +54,7 @@ class DatabasesConsoleClientTest extends Scope } /** - * @depends testCreateCollection + * @depends testCreateDatabase */ // public function testGetDatabaseUsage(array $data) // { @@ -100,7 +100,7 @@ class DatabasesConsoleClientTest extends Scope /** - * @depends testCreateCollection + * @depends testCreateDatabase */ public function testGetCollectionUsage(array $data) { @@ -148,7 +148,7 @@ class DatabasesConsoleClientTest extends Scope } /** - * @depends testCreateCollection + * @depends testCreateDatabase */ public function testGetCollectionLogs(array $data) { @@ -201,4 +201,128 @@ class DatabasesConsoleClientTest extends Scope $this->assertLessThanOrEqual(1, count($logs['body']['logs'])); $this->assertIsNumeric($logs['body']['total']); } + + + + + /** + * @depends testCreateDatabase + */ + public function testTimeoutCollection(array $data): array + { + $collection = $this->client->call(Client::METHOD_POST, '/databases/' . $data['databaseId'] . '/collections', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'collectionId' => ID::unique(), + 'name' => 'Slow Queries', + 'documentSecurity' => true, + 'permissions' => [ + Permission::create(Role::user($this->getUser()['$id'])), + ], + ]); + + $this->assertEquals(201, $collection['headers']['status-code']); + + $data = [ + '$id' => $collection['body']['$id'], + 'databaseId' => $collection['body']['databaseId'] + ]; + + $longtext = $this->client->call(Client::METHOD_POST, '/databases/' . $data['databaseId'] . '/collections/' . $data['$id'] . '/attributes/string', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'] + ]), [ + 'key' => 'longtext', + 'size' => 100000000, + 'required' => false, + 'default' => null, + ]); + + $this->assertEquals($longtext['headers']['status-code'], 202); + + return $data; + } + + + /** + * @depends testTimeoutCollection + */ + public function testTimeouts(array $data): void + { + for ($i = 0; $i <= 1; $i++) { + $this->client->call(Client::METHOD_POST, '/databases/' . $data['databaseId'] . '/collections/' . $data['$id'] . '/documents', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'documentId' => ID::unique(), + 'data' => [ + 'longtext' => file_get_contents(__DIR__ . '/longtext.txt'), + ], + 'permissions' => [ + Permission::read(Role::user($this->getUser()['$id'])), + Permission::update(Role::user($this->getUser()['$id'])), + Permission::delete(Role::user($this->getUser()['$id'])), + ] + ]); + } + + $docs = []; + for ($i = 0; $i <= 2; $i++) { + $docs[] = $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'], + 'x-appwrite-timeout' => 1, + ], $this->getHeaders()), [ + 'queries' => ['notEqual("longtext", "appwrite")'], + ]); + } + + $this->assertEquals(408, $docs[0]['headers']['status-code']); // insert + $this->assertEquals(403, $docs[1]['headers']['status-code']); // update + $this->assertEquals(403, $docs[2]['headers']['status-code']); // blocked + } + + /** + * @depends testTimeoutCollection + */ + public function testConsoleTimeouts($data): void + { + $documents = $this->client->call(Client::METHOD_GET, '/databases/' . $data['databaseId'] . '/slow-queries', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'queries' => [ + 'equal("collectionId", "' . $data['$id'] . '")', + 'equal("blocked", true)', + 'orderAsc("$updatedAt")' + ] + ]); + + $this->assertEquals(200, $documents['headers']['status-code']); + $this->assertEquals(1, $documents['body']['total']); + $this->assertEquals(true, $documents['body']['documents'][0]['blocked']); + $this->assertEquals(2, $documents['body']['documents'][0]['count']); + + $document = $this->client->call(Client::METHOD_GET, '/databases/slow-queries/' . $documents['body']['documents'][0]['$id'], array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), []); + $this->assertEquals(200, $documents['headers']['status-code']); + $this->assertEquals($document['body']['queries'][0], 'notEqual("longtext", "appwrite")'); + + $response = $this->client->call(Client::METHOD_DELETE, '/databases/slow-queries/' . $documents['body']['documents'][0]['$id'], array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), []); + $this->assertEquals(204, $response['headers']['status-code']); + + $document = $this->client->call(Client::METHOD_GET, '/databases/slow-queries/' . $documents['body']['documents'][0]['$id'], array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), []); + $this->assertEquals(404, $document['headers']['status-code']); + } }