From f0df316b4f1dec6813456e9fdcc338ca096a7eec Mon Sep 17 00:00:00 2001 From: Christy Jacob Date: Sat, 29 Apr 2023 12:41:17 +0530 Subject: [PATCH 1/6] fix: throw correct exception when project custom ID already exists --- app/config/errors.php | 5 +++ app/controllers/api/projects.php | 73 +++++++++++++++++-------------- composer.lock | 67 ++++++++++++++-------------- src/Appwrite/Extend/Exception.php | 1 + 4 files changed, 80 insertions(+), 66 deletions(-) diff --git a/app/config/errors.php b/app/config/errors.php index 2b21ff48e2..a27e77a58f 100644 --- a/app/config/errors.php +++ b/app/config/errors.php @@ -484,6 +484,11 @@ return [ 'description' => 'Project with the requested ID could not be found. Please check the value of the X-Appwrite-Project header to ensure the correct project ID is being used.', 'code' => 404, ], + Exception::PROJECT_ALREADY_EXISTS => [ + 'name' => Exception::PROJECT_ALREADY_EXISTS, + 'description' => 'Project with the requested ID already exists.', + 'code' => 403, + ], Exception::PROJECT_UNKNOWN => [ 'name' => Exception::PROJECT_UNKNOWN, 'description' => 'The project ID is either missing or not valid. Please check the value of the X-Appwrite-Project header to ensure the correct project ID is being used.', diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index c7a85f0489..cbce61c60b 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -29,6 +29,7 @@ use Appwrite\Extend\Exception; use Appwrite\Utopia\Database\Validator\Queries\Projects; use Utopia\Cache\Cache; use Utopia\Pools\Group; +use Utopia\Database\Exception\Duplicate; use Utopia\Validator\ArrayList; use Utopia\Validator\Boolean; use Utopia\Validator\Hostname; @@ -96,39 +97,45 @@ App::post('/v1/projects') throw new Exception(Exception::PROJECT_RESERVED_PROJECT, "'console' is a reserved project."); } - $project = $dbForConsole->createDocument('projects', new Document([ - '$id' => $projectId, - '$permissions' => [ - Permission::read(Role::team(ID::custom($teamId))), - Permission::update(Role::team(ID::custom($teamId), 'owner')), - Permission::update(Role::team(ID::custom($teamId), 'developer')), - Permission::delete(Role::team(ID::custom($teamId), 'owner')), - Permission::delete(Role::team(ID::custom($teamId), 'developer')), - ], - 'name' => $name, - 'teamInternalId' => $team->getInternalId(), - 'teamId' => $team->getId(), - 'region' => $region, - 'description' => $description, - 'logo' => $logo, - 'url' => $url, - 'version' => APP_VERSION_STABLE, - 'legalName' => $legalName, - 'legalCountry' => $legalCountry, - 'legalState' => $legalState, - 'legalCity' => $legalCity, - 'legalAddress' => $legalAddress, - 'legalTaxId' => ID::custom($legalTaxId), - 'services' => new stdClass(), - 'platforms' => null, - 'authProviders' => [], - 'webhooks' => null, - 'keys' => null, - 'domains' => null, - 'auths' => $auths, - 'search' => implode(' ', [$projectId, $name]), - 'database' => $database, - ])); + try { + $project = $dbForConsole->createDocument('projects', new Document([ + '$id' => $projectId, + '$permissions' => [ + Permission::read(Role::team(ID::custom($teamId))), + Permission::update(Role::team(ID::custom($teamId), 'owner')), + Permission::update(Role::team(ID::custom($teamId), 'developer')), + Permission::delete(Role::team(ID::custom($teamId), 'owner')), + Permission::delete(Role::team(ID::custom($teamId), 'developer')), + ], + 'name' => $name, + 'teamInternalId' => $team->getInternalId(), + 'teamId' => $team->getId(), + 'region' => $region, + 'description' => $description, + 'logo' => $logo, + 'url' => $url, + 'version' => APP_VERSION_STABLE, + 'legalName' => $legalName, + 'legalCountry' => $legalCountry, + 'legalState' => $legalState, + 'legalCity' => $legalCity, + 'legalAddress' => $legalAddress, + 'legalTaxId' => ID::custom($legalTaxId), + 'services' => new stdClass(), + 'platforms' => null, + 'authProviders' => [], + 'webhooks' => null, + 'keys' => null, + 'domains' => null, + 'auths' => $auths, + 'search' => implode(' ', [$projectId, $name]), + ])); + } catch (Duplicate $th) { + throw new Exception(Exception::PROJECT_ALREADY_EXISTS); + } + + /** @var array $collections */ + $collections = Config::getParam('collections', []); $dbForProject = new Database($pools->get($database)->pop()->getResource(), $cache); $dbForProject->setNamespace("_{$project->getInternalId()}"); diff --git a/composer.lock b/composer.lock index 5349ca36b1..596d6584c7 100644 --- a/composer.lock +++ b/composer.lock @@ -408,21 +408,21 @@ }, { "name": "guzzlehttp/guzzle", - "version": "7.6.1", + "version": "7.7.0", "source": { "type": "git", "url": "https://github.com/guzzle/guzzle.git", - "reference": "8444a2bacf1960bc6a2b62ed86b8e72e11eebe51" + "reference": "fb7566caccf22d74d1ab270de3551f72a58399f5" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/guzzle/guzzle/zipball/8444a2bacf1960bc6a2b62ed86b8e72e11eebe51", - "reference": "8444a2bacf1960bc6a2b62ed86b8e72e11eebe51", + "url": "https://api.github.com/repos/guzzle/guzzle/zipball/fb7566caccf22d74d1ab270de3551f72a58399f5", + "reference": "fb7566caccf22d74d1ab270de3551f72a58399f5", "shasum": "" }, "require": { "ext-json": "*", - "guzzlehttp/promises": "^1.5", + "guzzlehttp/promises": "^1.5.3 || ^2.0", "guzzlehttp/psr7": "^1.9.1 || ^2.4.5", "php": "^7.2.5 || ^8.0", "psr/http-client": "^1.0", @@ -434,7 +434,8 @@ "require-dev": { "bamarni/composer-bin-plugin": "^1.8.1", "ext-curl": "*", - "php-http/client-integration-tests": "^3.0", + "php-http/client-integration-tests": "dev-master#2c025848417c1135031fdf9c728ee53d0a7ceaee as 3.0.999", + "php-http/message-factory": "^1.1", "phpunit/phpunit": "^8.5.29 || ^9.5.23", "psr/log": "^1.1 || ^2.0 || ^3.0" }, @@ -513,7 +514,7 @@ ], "support": { "issues": "https://github.com/guzzle/guzzle/issues", - "source": "https://github.com/guzzle/guzzle/tree/7.6.1" + "source": "https://github.com/guzzle/guzzle/tree/7.7.0" }, "funding": [ { @@ -529,38 +530,37 @@ "type": "tidelift" } ], - "time": "2023-05-15T20:43:01+00:00" + "time": "2023-05-21T14:04:53+00:00" }, { "name": "guzzlehttp/promises", - "version": "1.5.2", + "version": "2.0.0", "source": { "type": "git", "url": "https://github.com/guzzle/promises.git", - "reference": "b94b2807d85443f9719887892882d0329d1e2598" + "reference": "3a494dc7dc1d7d12e511890177ae2d0e6c107da6" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/guzzle/promises/zipball/b94b2807d85443f9719887892882d0329d1e2598", - "reference": "b94b2807d85443f9719887892882d0329d1e2598", + "url": "https://api.github.com/repos/guzzle/promises/zipball/3a494dc7dc1d7d12e511890177ae2d0e6c107da6", + "reference": "3a494dc7dc1d7d12e511890177ae2d0e6c107da6", "shasum": "" }, "require": { - "php": ">=5.5" + "php": "^7.2.5 || ^8.0" }, "require-dev": { - "symfony/phpunit-bridge": "^4.4 || ^5.1" + "bamarni/composer-bin-plugin": "^1.8.1", + "phpunit/phpunit": "^8.5.29 || ^9.5.23" }, "type": "library", "extra": { - "branch-alias": { - "dev-master": "1.5-dev" + "bamarni-bin": { + "bin-links": true, + "forward-command": false } }, "autoload": { - "files": [ - "src/functions_include.php" - ], "psr-4": { "GuzzleHttp\\Promise\\": "src/" } @@ -597,7 +597,7 @@ ], "support": { "issues": "https://github.com/guzzle/promises/issues", - "source": "https://github.com/guzzle/promises/tree/1.5.2" + "source": "https://github.com/guzzle/promises/tree/2.0.0" }, "funding": [ { @@ -613,7 +613,7 @@ "type": "tidelift" } ], - "time": "2022-08-28T14:55:35+00:00" + "time": "2023-05-21T13:50:22+00:00" }, { "name": "guzzlehttp/psr7", @@ -3165,16 +3165,16 @@ }, { "name": "nikic/php-parser", - "version": "v4.15.4", + "version": "v4.15.5", "source": { "type": "git", "url": "https://github.com/nikic/PHP-Parser.git", - "reference": "6bb5176bc4af8bcb7d926f88718db9b96a2d4290" + "reference": "11e2663a5bc9db5d714eedb4277ee300403b4a9e" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/nikic/PHP-Parser/zipball/6bb5176bc4af8bcb7d926f88718db9b96a2d4290", - "reference": "6bb5176bc4af8bcb7d926f88718db9b96a2d4290", + "url": "https://api.github.com/repos/nikic/PHP-Parser/zipball/11e2663a5bc9db5d714eedb4277ee300403b4a9e", + "reference": "11e2663a5bc9db5d714eedb4277ee300403b4a9e", "shasum": "" }, "require": { @@ -3215,9 +3215,9 @@ ], "support": { "issues": "https://github.com/nikic/PHP-Parser/issues", - "source": "https://github.com/nikic/PHP-Parser/tree/v4.15.4" + "source": "https://github.com/nikic/PHP-Parser/tree/v4.15.5" }, - "time": "2023-03-05T19:49:14+00:00" + "time": "2023-05-19T20:20:00+00:00" }, { "name": "phar-io/manifest", @@ -3568,22 +3568,23 @@ }, { "name": "phpstan/phpdoc-parser", - "version": "1.20.4", + "version": "1.21.0", "source": { "type": "git", "url": "https://github.com/phpstan/phpdoc-parser.git", - "reference": "7d568c87a9df9c5f7e8b5f075fc469aa8cb0a4cd" + "reference": "6df62b08faef4f899772bc7c3bbabb93d2b7a21c" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpstan/phpdoc-parser/zipball/7d568c87a9df9c5f7e8b5f075fc469aa8cb0a4cd", - "reference": "7d568c87a9df9c5f7e8b5f075fc469aa8cb0a4cd", + "url": "https://api.github.com/repos/phpstan/phpdoc-parser/zipball/6df62b08faef4f899772bc7c3bbabb93d2b7a21c", + "reference": "6df62b08faef4f899772bc7c3bbabb93d2b7a21c", "shasum": "" }, "require": { "php": "^7.2 || ^8.0" }, "require-dev": { + "nikic/php-parser": "^4.15", "php-parallel-lint/php-parallel-lint": "^1.2", "phpstan/extension-installer": "^1.0", "phpstan/phpstan": "^1.5", @@ -3607,9 +3608,9 @@ "description": "PHPDoc parser with support for nullable, intersection and generic types", "support": { "issues": "https://github.com/phpstan/phpdoc-parser/issues", - "source": "https://github.com/phpstan/phpdoc-parser/tree/1.20.4" + "source": "https://github.com/phpstan/phpdoc-parser/tree/1.21.0" }, - "time": "2023-05-02T09:19:37+00:00" + "time": "2023-05-17T13:13:44+00:00" }, { "name": "phpunit/php-code-coverage", diff --git a/src/Appwrite/Extend/Exception.php b/src/Appwrite/Extend/Exception.php index ecf6e17c1f..71bb548823 100644 --- a/src/Appwrite/Extend/Exception.php +++ b/src/Appwrite/Extend/Exception.php @@ -156,6 +156,7 @@ class Exception extends \Exception public const PROJECT_UNKNOWN = 'project_unknown'; public const PROJECT_PROVIDER_DISABLED = 'project_provider_disabled'; public const PROJECT_PROVIDER_UNSUPPORTED = 'project_provider_unsupported'; + public const PROJECT_ALREADY_EXISTS = 'project_already_exists'; public const PROJECT_INVALID_SUCCESS_URL = 'project_invalid_success_url'; public const PROJECT_INVALID_FAILURE_URL = 'project_invalid_failure_url'; public const PROJECT_RESERVED_PROJECT = 'project_reserved_project'; From 8be36088d71f0dc2a42b292440db21ba3c29520f Mon Sep 17 00:00:00 2001 From: Christy Jacob Date: Sat, 29 Apr 2023 12:44:07 +0530 Subject: [PATCH 2/6] chore: linter --- app/controllers/api/projects.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index cbce61c60b..7978daa5d3 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -133,7 +133,7 @@ App::post('/v1/projects') } catch (Duplicate $th) { throw new Exception(Exception::PROJECT_ALREADY_EXISTS); } - + /** @var array $collections */ $collections = Config::getParam('collections', []); From 9a712686f2636e4cdc0bd6910724add60c01f0fd Mon Sep 17 00:00:00 2001 From: Christy Jacob Date: Sat, 29 Apr 2023 13:13:33 +0530 Subject: [PATCH 3/6] chore: add tests --- phpunit.xml | 4 +-- .../Projects/ProjectsConsoleClientTest.php | 33 ++++++++++++++++++- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/phpunit.xml b/phpunit.xml index 927e91567a..4074fe0f1c 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -6,8 +6,8 @@ convertNoticesToExceptions="true" convertWarningsToExceptions="true" processIsolation="false" - stopOnFailure="false" - > + stopOnFailure="true" +> diff --git a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php index 789020bf42..6380a81326 100644 --- a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php +++ b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php @@ -3,6 +3,7 @@ namespace Tests\E2E\Services\Projects; use Appwrite\Auth\Auth; +use Appwrite\Extend\Exception; use Tests\E2E\Scopes\Scope; use Tests\E2E\Scopes\ProjectConsole; use Tests\E2E\Scopes\SideClient; @@ -98,7 +99,37 @@ class ProjectsConsoleClientTest extends Scope $this->assertEquals(400, $response['headers']['status-code']); - return ['projectId' => $projectId]; + return [ + 'projectId' => $projectId, + 'teamId' => $team['body']['$id'] + ]; + } + + /** + * @depends testCreateProject + */ + public function testCreateDuplicateProject($data) + { + $teamId = $data['teamId'] ?? ''; + $projectId = $data['projectId'] ?? ''; + + /** + * Test for FAILURE + */ + $response = $this->client->call(Client::METHOD_POST, '/projects', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'projectId' => $projectId, + 'name' => 'Project Duplicate', + 'teamId' => $teamId, + 'region' => 'default' + ]); + + $this->assertEquals(403, $response['headers']['status-code']); + $this->assertEquals(403, $response['body']['code']); + $this->assertEquals(Exception::PROJECT_ALREADY_EXISTS, $response['body']['type']); + $this->assertEquals('Project with the requested ID already exists.', $response['body']['message']); } /** From 0839822a66418316dada33cc45acb69fbc091cfd Mon Sep 17 00:00:00 2001 From: Christy Jacob Date: Sun, 30 Apr 2023 01:54:45 +0530 Subject: [PATCH 4/6] Apply suggestions from code review --- app/config/errors.php | 2 +- phpunit.xml | 2 +- tests/e2e/Services/Projects/ProjectsConsoleClientTest.php | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/config/errors.php b/app/config/errors.php index a27e77a58f..66352e9bf4 100644 --- a/app/config/errors.php +++ b/app/config/errors.php @@ -487,7 +487,7 @@ return [ Exception::PROJECT_ALREADY_EXISTS => [ 'name' => Exception::PROJECT_ALREADY_EXISTS, 'description' => 'Project with the requested ID already exists.', - 'code' => 403, + 'code' => 409, ], Exception::PROJECT_UNKNOWN => [ 'name' => Exception::PROJECT_UNKNOWN, diff --git a/phpunit.xml b/phpunit.xml index 4074fe0f1c..4012c8c276 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -6,7 +6,7 @@ convertNoticesToExceptions="true" convertWarningsToExceptions="true" processIsolation="false" - stopOnFailure="true" + stopOnFailure="false" > diff --git a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php index 6380a81326..7254f88efe 100644 --- a/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php +++ b/tests/e2e/Services/Projects/ProjectsConsoleClientTest.php @@ -126,8 +126,8 @@ class ProjectsConsoleClientTest extends Scope 'region' => 'default' ]); - $this->assertEquals(403, $response['headers']['status-code']); - $this->assertEquals(403, $response['body']['code']); + $this->assertEquals(409, $response['headers']['status-code']); + $this->assertEquals(409, $response['body']['code']); $this->assertEquals(Exception::PROJECT_ALREADY_EXISTS, $response['body']['type']); $this->assertEquals('Project with the requested ID already exists.', $response['body']['message']); } From 48ca2530e6dfe1af372dc4c55032832570d61320 Mon Sep 17 00:00:00 2001 From: Christy Jacob Date: Tue, 23 May 2023 14:46:48 +0000 Subject: [PATCH 5/6] feat: add missing attribute --- app/controllers/api/projects.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index 7978daa5d3..b9aad22070 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -129,6 +129,7 @@ App::post('/v1/projects') 'domains' => null, 'auths' => $auths, 'search' => implode(' ', [$projectId, $name]), + 'database' => $database ])); } catch (Duplicate $th) { throw new Exception(Exception::PROJECT_ALREADY_EXISTS); From 54ad50bd4b12af984bce20edbe1b1fc606cb3bc2 Mon Sep 17 00:00:00 2001 From: Christy Jacob Date: Wed, 24 May 2023 20:07:31 +0000 Subject: [PATCH 6/6] feat: review comments --- app/controllers/api/projects.php | 3 --- composer.lock | 12 ++++++------ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/app/controllers/api/projects.php b/app/controllers/api/projects.php index b9aad22070..dec5520d85 100644 --- a/app/controllers/api/projects.php +++ b/app/controllers/api/projects.php @@ -135,9 +135,6 @@ App::post('/v1/projects') throw new Exception(Exception::PROJECT_ALREADY_EXISTS); } - /** @var array $collections */ - $collections = Config::getParam('collections', []); - $dbForProject = new Database($pools->get($database)->pop()->getResource(), $cache); $dbForProject->setNamespace("_{$project->getInternalId()}"); $dbForProject->create(); diff --git a/composer.lock b/composer.lock index 596d6584c7..7e949f8a05 100644 --- a/composer.lock +++ b/composer.lock @@ -2425,16 +2425,16 @@ }, { "name": "utopia-php/queue", - "version": "0.5.2", + "version": "0.5.3", "source": { "type": "git", "url": "https://github.com/utopia-php/queue.git", - "reference": "310271c5cd477541208d7fa74a4dea64df8e04a0" + "reference": "8e8b6cb27172713fe5d8b7b092ce68516caf129a" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/utopia-php/queue/zipball/310271c5cd477541208d7fa74a4dea64df8e04a0", - "reference": "310271c5cd477541208d7fa74a4dea64df8e04a0", + "url": "https://api.github.com/repos/utopia-php/queue/zipball/8e8b6cb27172713fe5d8b7b092ce68516caf129a", + "reference": "8e8b6cb27172713fe5d8b7b092ce68516caf129a", "shasum": "" }, "require": { @@ -2480,9 +2480,9 @@ ], "support": { "issues": "https://github.com/utopia-php/queue/issues", - "source": "https://github.com/utopia-php/queue/tree/0.5.2" + "source": "https://github.com/utopia-php/queue/tree/0.5.3" }, - "time": "2023-03-07T08:54:10+00:00" + "time": "2023-05-24T19:06:04+00:00" }, { "name": "utopia-php/registry",