From 4b0ef4598bd0eaae87d3b80dde7e81481ec1e0ac Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Wed, 11 Jan 2023 19:37:06 +1300 Subject: [PATCH] Fix deletes worker not deleting project database tables --- app/workers/deletes.php | 50 +++++++++++++++--- src/Appwrite/Resque/Worker.php | 92 ++++++++++++++++++++++------------ 2 files changed, 105 insertions(+), 37 deletions(-) diff --git a/app/workers/deletes.php b/app/workers/deletes.php index 5dc7e8d737..ca5299e8af 100644 --- a/app/workers/deletes.php +++ b/app/workers/deletes.php @@ -172,6 +172,7 @@ class DeletesV1 extends Worker /** * @param Document $document database document * @param string $projectId + * @throws Exception */ protected function deleteDatabase(Document $document, string $projectId): void { @@ -191,6 +192,7 @@ class DeletesV1 extends Worker /** * @param Document $document teams document * @param string $projectId + * @throws Exception */ protected function deleteCollection(Document $document, string $projectId): void { @@ -217,6 +219,7 @@ class DeletesV1 extends Worker /** * @param string $hourlyUsageRetentionDatetime + * @throws Exception */ protected function deleteUsageStats(string $hourlyUsageRetentionDatetime) { @@ -232,6 +235,7 @@ class DeletesV1 extends Worker /** * @param Document $document teams document * @param string $projectId + * @throws Exception */ protected function deleteMemberships(Document $document, string $projectId): void { @@ -245,13 +249,37 @@ class DeletesV1 extends Worker /** * @param Document $document project document + * @throws Exception */ protected function deleteProject(Document $document): void { - $projectId = $document->getId(); + // Delete project tables + $dbForProject = $this->getProjectDBFromDocument($document); - // Delete all DBs - $this->getProjectDB($projectId)->delete($projectId); + $limit = 50; + $offset = 0; + + while (true) { + $collections = $dbForProject->listCollections($limit, $offset); + + if (empty($collections)) { + break; + } + + foreach ($collections as $collection) { + $dbForProject->deleteCollection($collection->getId()); + } + + $offset += $limit; + } + + // Delete metadata tables + try { + $dbForProject->deleteCollection('_metadata'); + } catch (Exception) { + // Ignore: deleteCollection tries to delete a metadata entry after the collection is deleted, + // which will throw an exception here because the metadata collection is already deleted. + } // Delete all storage directories $uploads = $this->getFilesDevice($document->getId()); @@ -264,6 +292,7 @@ class DeletesV1 extends Worker /** * @param Document $document user document * @param string $projectId + * @throws Exception */ protected function deleteUser(Document $document, string $projectId): void { @@ -305,6 +334,7 @@ class DeletesV1 extends Worker /** * @param string $datetime + * @throws Exception */ protected function deleteExecutionLogs(string $datetime): void { @@ -337,6 +367,7 @@ class DeletesV1 extends Worker /** * @param string $datetime + * @throws Exception */ protected function deleteRealtimeUsage(string $datetime): void { @@ -393,6 +424,7 @@ class DeletesV1 extends Worker /** * @param string $resource * @param string $projectId + * @throws Exception */ protected function deleteAuditLogsByResource(string $resource, string $projectId): void { @@ -406,6 +438,7 @@ class DeletesV1 extends Worker /** * @param Document $document function document * @param string $projectId + * @throws Exception */ protected function deleteFunction(Document $document, string $projectId): void { @@ -479,6 +512,7 @@ class DeletesV1 extends Worker /** * @param Document $document deployment document * @param string $projectId + * @throws Exception */ protected function deleteDeployment(Document $document, string $projectId): void { @@ -528,9 +562,10 @@ class DeletesV1 extends Worker /** * @param Document $document to be deleted * @param Database $database to delete it from - * @param callable $callback to perform after document is deleted + * @param callable|null $callback to perform after document is deleted * * @return bool + * @throws \Utopia\Database\Exception\Authorization */ protected function deleteById(Document $document, Database $database, callable $callback = null): bool { @@ -550,6 +585,7 @@ class DeletesV1 extends Worker /** * @param callable $callback + * @throws Exception */ protected function deleteForProjectIds(callable $callback): void { @@ -584,9 +620,10 @@ class DeletesV1 extends Worker /** * @param string $collection collectionID - * @param Query[] $queries + * @param array $queries * @param Database $database - * @param callable $callback + * @param callable|null $callback + * @throws Exception */ protected function deleteByGroup(string $collection, array $queries, Database $database, callable $callback = null): void { @@ -620,6 +657,7 @@ class DeletesV1 extends Worker /** * @param Document $document certificates document + * @throws \Utopia\Database\Exception\Authorization */ protected function deleteCertificates(Document $document): void { diff --git a/src/Appwrite/Resque/Worker.php b/src/Appwrite/Resque/Worker.php index dd7cebd084..b01bace9ea 100644 --- a/src/Appwrite/Resque/Worker.php +++ b/src/Appwrite/Resque/Worker.php @@ -2,22 +2,23 @@ namespace Appwrite\Resque; +use Exception; use Utopia\App; -use Utopia\Cache\Cache; use Utopia\Cache\Adapter\Redis as RedisCache; +use Utopia\Cache\Cache; use Utopia\CLI\Console; -use Utopia\Database\Database; use Utopia\Database\Adapter\MariaDB; +use Utopia\Database\Database; +use Utopia\Database\Document; +use Utopia\Database\Validator\Authorization; use Utopia\Storage\Device; -use Utopia\Storage\Storage; -use Utopia\Storage\Device\Local; +use Utopia\Storage\Device\Backblaze; use Utopia\Storage\Device\DOSpaces; use Utopia\Storage\Device\Linode; -use Utopia\Storage\Device\Wasabi; -use Utopia\Storage\Device\Backblaze; +use Utopia\Storage\Device\Local; use Utopia\Storage\Device\S3; -use Exception; -use Utopia\Database\Validator\Authorization; +use Utopia\Storage\Device\Wasabi; +use Utopia\Storage\Storage; abstract class Worker { @@ -53,7 +54,7 @@ abstract class Worker * @return void * @throws \Exception|\Throwable */ - public function init() + public function init(): void { throw new Exception("Please implement init method in worker"); } @@ -65,7 +66,7 @@ abstract class Worker * @return void * @throws \Exception|\Throwable */ - public function run() + public function run(): void { throw new Exception("Please implement run method in worker"); } @@ -77,7 +78,7 @@ abstract class Worker * @return void * @throws \Exception|\Throwable */ - public function shutdown() + public function shutdown(): void { throw new Exception("Please implement shutdown method in worker"); } @@ -151,17 +152,18 @@ abstract class Worker /** * Register callback. Will be executed when error occurs. * @param callable $callback - * @param Throwable $error - * @return self + * @return void */ public static function error(callable $callback): void { - \array_push(self::$errorCallbacks, $callback); + self::$errorCallbacks[] = $callback; } + /** * Get internal project database * @param string $projectId * @return Database + * @throws Exception */ protected function getProjectDB(string $projectId): Database { @@ -177,9 +179,23 @@ abstract class Worker return $this->getDB(self::DATABASE_PROJECT, $projectId, $project->getInternalId()); } + /** + * Get internal project database given the project document + * + * Allows avoiding race conditions when modifying the projects collection + * @param Document $project + * @return Database + * @throws Exception + */ + protected function getProjectDBFromDocument(Document $project): Database + { + return $this->getDB(self::DATABASE_PROJECT, project: $project); + } + /** * Get console database * @return Database + * @throws Exception */ protected function getConsoleDB(): Database { @@ -187,24 +203,35 @@ abstract class Worker } /** - * Get console database - * @param string $type One of (internal, external, console) - * @param string $projectId of internal or external DB + * Get database + * @param string $type One of (project, console) + * @param string $projectId of project or console DB + * @param string $projectInternalId + * @param Document|null $project * @return Database + * @throws Exception */ - private function getDB(string $type, string $projectId = '', string $projectInternalId = ''): Database - { + private function getDB( + string $type, + string $projectId = '', + string $projectInternalId = '', + ?Document $project = null + ): Database { global $register; - $namespace = ''; $sleep = DATABASE_RECONNECT_SLEEP; // overwritten when necessary + if ($project !== null) { + $projectId = $project->getId(); + $projectInternalId = $project->getInternalId(); + } + switch ($type) { case self::DATABASE_PROJECT: if (!$projectId) { throw new \Exception('ProjectID not provided - cannot get database'); } - $namespace = "_{$projectInternalId}"; + $namespace = "_$projectInternalId"; break; case self::DATABASE_CONSOLE: $namespace = "_console"; @@ -212,12 +239,11 @@ abstract class Worker break; default: throw new \Exception('Unknown database type: ' . $type); - break; } $attempts = 0; - do { + while (true) { try { $attempts++; $cache = new Cache(new RedisCache($register->get('cache'))); @@ -225,8 +251,12 @@ abstract class Worker $database->setDefaultDatabase(App::getEnv('_APP_DB_SCHEMA', 'appwrite')); $database->setNamespace($namespace); // Main DB - if (!empty($projectId) && !$database->getDocument('projects', $projectId)->isEmpty()) { - throw new \Exception("Project does not exist: {$projectId}"); + if ( + $project === null + && !empty($projectId) + && !$database->getDocument('projects', $projectId)->isEmpty() + ) { + throw new \Exception("Project does not exist: $projectId"); } if ($type === self::DATABASE_CONSOLE && !$database->exists($database->getDefaultDatabase(), Database::METADATA)) { @@ -235,13 +265,13 @@ abstract class Worker break; // leave loop if successful } catch (\Exception $e) { - Console::warning("Database not ready. Retrying connection ({$attempts})..."); + Console::warning("Database not ready. Retrying connection ($attempts)..."); if ($attempts >= DATABASE_RECONNECT_MAX_ATTEMPTS) { throw new \Exception('Failed to connect to database: ' . $e->getMessage()); } sleep($sleep); } - } while ($attempts < DATABASE_RECONNECT_MAX_ATTEMPTS); + } return $database; } @@ -251,7 +281,7 @@ abstract class Worker * @param string $projectId of the project * @return Device */ - protected function getFunctionsDevice($projectId): Device + protected function getFunctionsDevice(string $projectId): Device { return $this->getDevice(APP_STORAGE_FUNCTIONS . '/app-' . $projectId); } @@ -261,7 +291,7 @@ abstract class Worker * @param string $projectId of the project * @return Device */ - protected function getFilesDevice($projectId): Device + protected function getFilesDevice(string $projectId): Device { return $this->getDevice(APP_STORAGE_UPLOADS . '/app-' . $projectId); } @@ -272,7 +302,7 @@ abstract class Worker * @param string $projectId of the project * @return Device */ - protected function getBuildsDevice($projectId): Device + protected function getBuildsDevice(string $projectId): Device { return $this->getDevice(APP_STORAGE_BUILDS . '/app-' . $projectId); } @@ -282,7 +312,7 @@ abstract class Worker * @param string $root path of the device * @return Device */ - public function getDevice($root): Device + public function getDevice(string $root): Device { switch (App::getEnv('_APP_STORAGE_DEVICE', Storage::DEVICE_LOCAL)) { case Storage::DEVICE_LOCAL: