From ce98b043484e19940a6bc38845c209de8a0a7ae3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Fri, 28 Jul 2023 09:56:07 +0200 Subject: [PATCH] Address PR reviews --- app/config/collections.php | 30 ++++++- app/config/errors.php | 4 +- app/controllers/api/functions.php | 98 ++++++++++++--------- app/controllers/api/project.php | 8 +- app/controllers/api/proxy.php | 2 +- app/controllers/general.php | 13 +-- app/init.php | 15 ++++ app/workers/builds.php | 5 +- app/workers/functions.php | 8 +- src/Appwrite/Extend/Exception.php | 2 +- src/Appwrite/Utopia/Response/Model/Func.php | 12 +-- 11 files changed, 126 insertions(+), 71 deletions(-) diff --git a/app/config/collections.php b/app/config/collections.php index feddc2f4aa..42712cf375 100644 --- a/app/config/collections.php +++ b/app/config/collections.php @@ -732,6 +732,17 @@ $collections = [ 'array' => false, 'filters' => [], ], + [ + '$id' => ID::custom('variables'), + 'type' => Database::VAR_STRING, + 'format' => '', + 'size' => 16384, + 'signed' => true, + 'required' => false, + 'default' => null, + 'array' => false, + 'filters' => ['subQueryProjectVariables'], + ], ], 'indexes' => [ [ @@ -2412,7 +2423,17 @@ $collections = [ 'array' => false, 'filters' => [], ], - // TODO: Resource Internal ID? + [ + '$id' => ID::custom('resourceInternalId'), + 'type' => Database::VAR_STRING, + 'format' => '', + 'size' => Database::LENGTH_KEY, + 'signed' => true, + 'required' => false, + 'default' => null, + 'array' => false, + 'filters' => [], + ], [ '$id' => ID::custom('resourceType'), 'type' => Database::VAR_STRING, @@ -2479,6 +2500,13 @@ $collections = [ 'lengths' => [Database::LENGTH_KEY], 'orders' => [Database::ORDER_ASC], ], + [ + '$id' => '_key_resourceInternalId', + 'type' => Database::INDEX_KEY, + 'attributes' => ['resourceInternalId'], + 'lengths' => [Database::LENGTH_KEY], + 'orders' => [Database::ORDER_ASC], + ], [ '$id' => ID::custom('_key_resourceType'), 'type' => Database::INDEX_KEY, diff --git a/app/config/errors.php b/app/config/errors.php index f0045ca876..e4382aafe0 100644 --- a/app/config/errors.php +++ b/app/config/errors.php @@ -566,8 +566,8 @@ return [ 'description' => '_APP_DOMAIN_TARGET must be a public domain.', 'code' => 501, ], - Exception::RULE_RESOURCE_ID_NOT_FOUND => [ - 'name' => Exception::RULE_RESOURCE_ID_NOT_FOUND, + Exception::RULE_RESOURCE_NOT_FOUND => [ + 'name' => Exception::RULE_RESOURCE_NOT_FOUND, 'description' => 'Resource could not be found. Check resourceId and resourceType.', 'code' => 404, ], diff --git a/app/controllers/api/functions.php b/app/controllers/api/functions.php index 52e0c363d9..d54bee1c6e 100644 --- a/app/controllers/api/functions.php +++ b/app/controllers/api/functions.php @@ -155,32 +155,6 @@ App::post('/v1/functions') throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'When connecting to VCS you need to provide all VCS parameters.'); } - $vcsRepositoryDocId = ''; - $vcsRepositoryDocInternalId = ''; - - // Git connect logic - if (!empty($vcsRepositoryId)) { - $vcsRepoDoc = $dbForConsole->createDocument('vcsRepos', new Document([ - '$id' => ID::unique(), - '$permissions' => [ - Permission::read(Role::any()), - Permission::update(Role::any()), - Permission::delete(Role::any()), - ], - 'vcsInstallationId' => $installation->getId(), - 'vcsInstallationInternalId' => $installation->getInternalId(), - 'projectId' => $project->getId(), - 'projectInternalId' => $project->getInternalId(), - 'repositoryId' => $vcsRepositoryId, - 'resourceId' => $functionId, - 'resourceType' => 'function', - 'pullRequests' => [] - ])); - - $vcsRepositoryDocId = $vcsRepoDoc->getId(); - $vcsRepositoryDocInternalId = $vcsRepoDoc->getInternalId(); - } - $function = $dbForProject->createDocument('functions', new Document([ '$id' => $functionId, 'execute' => $execute, @@ -199,8 +173,8 @@ App::post('/v1/functions') 'vcsInstallationId' => $installation->getId(), 'vcsInstallationInternalId' => $installation->getInternalId(), 'vcsRepositoryId' => $vcsRepositoryId, - 'vcsRepositoryDocId' => $vcsRepositoryDocId, - 'vcsRepositoryDocInternalId' => $vcsRepositoryDocInternalId, + 'vcsRepositoryDocId' => '', + 'vcsRepositoryDocInternalId' => '', 'vcsBranch' => $vcsBranch, 'vcsRootDirectory' => $vcsRootDirectory, 'vcsSilentMode' => $vcsSilentMode, @@ -208,6 +182,38 @@ App::post('/v1/functions') 'version' => 'v3' ])); + $vcsRepositoryDocId = ''; + $vcsRepositoryDocInternalId = ''; + + // Git connect logic + if (!empty($vcsRepositoryId)) { + $vcsRepoDoc = $dbForConsole->createDocument('vcsRepos', new Document([ + '$id' => ID::unique(), + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'vcsInstallationId' => $installation->getId(), + 'vcsInstallationInternalId' => $installation->getInternalId(), + 'projectId' => $project->getId(), + 'projectInternalId' => $project->getInternalId(), + 'repositoryId' => $vcsRepositoryId, + 'resourceId' => $function->getId(), + 'resourceInternalId' => $function->getInternalId(), + 'resourceType' => 'function', + 'pullRequests' => [] + ])); + + $vcsRepositoryDocId = $vcsRepoDoc->getId(); + $vcsRepositoryDocInternalId = $vcsRepoDoc->getInternalId(); + + $function = $dbForProject->updateDocument('functions', $function->getId(), $function + ->setAttribute('vcsRepositoryDocId', $vcsRepositoryDocId) + ->setAttribute('vcsRepositoryDocInternalId', $vcsRepositoryDocInternalId)); + } + + $schedule = Authorization::skip( fn () => $dbForConsole->createDocument('schedules', new Document([ 'region' => App::getEnv('_APP_REGION', 'default'), // Todo replace with projects region @@ -680,7 +686,7 @@ App::put('/v1/functions/:functionId') if ($isConnected && empty($vcsRepositoryId)) { $repoDocs = $dbForConsole->find('vcsRepos', [ Query::equal('projectInternalId', [$project->getInternalId()]), - Query::equal('resourceId', [$functionId]), + Query::equal('resourceInternalId', [$function->getInternalId()]), Query::equal('resourceType', ['function']), Query::limit(100), ]); @@ -712,7 +718,8 @@ App::put('/v1/functions/:functionId') 'projectId' => $project->getId(), 'projectInternalId' => $project->getInternalId(), 'repositoryId' => $vcsRepositoryId, - 'resourceId' => $functionId, + 'resourceId' => $function->getId(), + 'resourceInternalId' => $function->getInternalId(), 'resourceType' => 'function', 'pullRequests' => [] ])); @@ -758,13 +765,12 @@ App::put('/v1/functions/:functionId') $redeployVcsLogic($request, $function, $project, $installation, $dbForProject, new Document()); } + // Inform scheduler if function is still active $schedule = $dbForConsole->getDocument('schedules', $function->getAttribute('scheduleId')); - $schedule ->setAttribute('resourceUpdatedAt', DateTime::now()) ->setAttribute('schedule', $function->getAttribute('schedule')) ->setAttribute('active', !empty($function->getAttribute('schedule')) && !empty($function->getAttribute('deployment'))); - Authorization::skip(fn () => $dbForConsole->updateDocument('schedules', $schedule->getId(), $schedule)); $eventsInstance->setParam('functionId', $function->getId()); @@ -820,6 +826,7 @@ App::patch('/v1/functions/:functionId/deployments/:deploymentId') 'deployment' => $deployment->getId() ]))); + // Inform scheduler if function is still active $schedule = $dbForConsole->getDocument('schedules', $function->getAttribute('scheduleId')); $schedule ->setAttribute('resourceUpdatedAt', DateTime::now()) @@ -866,12 +873,11 @@ App::delete('/v1/functions/:functionId') throw new Exception(Exception::GENERAL_SERVER_ERROR, 'Failed to remove function from DB'); } + // Inform scheduler to no longer run function $schedule = $dbForConsole->getDocument('schedules', $function->getAttribute('scheduleId')); - $schedule ->setAttribute('resourceUpdatedAt', DateTime::now()) ->setAttribute('active', false); - Authorization::skip(fn () => $dbForConsole->updateDocument('schedules', $schedule->getId(), $schedule)); $deletes @@ -1411,11 +1417,6 @@ App::post('/v1/functions/:functionId/executions') 'agent' => $agent ]); - if ($function->getAttribute('logging')) { - /** @var Document $execution */ - $execution = Authorization::skip(fn () => $dbForProject->createDocument('executions', $execution)); - } - $jwt = ''; // initialize if (!$user->isEmpty()) { // If userId exists, generate a JWT for function $sessions = $user->getAttribute('sessions', []); @@ -1464,6 +1465,11 @@ App::post('/v1/functions/:functionId/executions') ->setContext('function', $function); if ($async) { + if ($function->getAttribute('logging')) { + /** @var Document $execution */ + $execution = Authorization::skip(fn () => $dbForProject->createDocument('executions', $execution)); + } + $queueForFunctions ->setType('http') ->setExecution($execution) @@ -1487,10 +1493,8 @@ App::post('/v1/functions/:functionId/executions') $vars = []; // Shared vars - $vars = \array_merge($vars, \array_reduce($dbForProject->find('variables', [ - Query::equal('resourceType', ['project']), - Query::limit(APP_LIMIT_SUBQUERY) - ]), function (array $carry, Document $var) { + $varsShared = $project->getAttribute('variables', []); + $vars = \array_merge($vars, \array_reduce($varsShared, function (array $carry, Document $var) { $carry[$var->getAttribute('key')] = $var->getAttribute('value') ?? ''; return $carry; }, [])); @@ -1550,7 +1554,8 @@ App::post('/v1/functions/:functionId/executions') } if ($function->getAttribute('logging')) { - Authorization::skip(fn () => $dbForProject->updateDocument('executions', $executionId, $execution)); + /** @var Document $execution */ + $execution = Authorization::skip(fn () => $dbForProject->createDocument('executions', $execution)); } // TODO revise this later using route label @@ -1748,9 +1753,11 @@ App::post('/v1/functions/:functionId/variables') } catch (DuplicateException $th) { throw new Exception(Exception::VARIABLE_ALREADY_EXISTS); } + $dbForConsole->deleteCachedDocument('projects', $project->getId()); $dbForProject->updateDocument('functions', $function->getId(), $function->setAttribute('live', false)); + // Inform scheduler to pull the latest changes $schedule = $dbForConsole->getDocument('schedules', $function->getAttribute('scheduleId')); $schedule ->setAttribute('resourceUpdatedAt', DateTime::now()) @@ -1874,9 +1881,11 @@ App::put('/v1/functions/:functionId/variables/:variableId') } catch (DuplicateException $th) { throw new Exception(Exception::VARIABLE_ALREADY_EXISTS); } + $dbForConsole->deleteCachedDocument('projects', $project->getId()); $dbForProject->updateDocument('functions', $function->getId(), $function->setAttribute('live', false)); + // Inform scheduler to pull the latest changes $schedule = $dbForConsole->getDocument('schedules', $function->getAttribute('scheduleId')); $schedule ->setAttribute('resourceUpdatedAt', DateTime::now()) @@ -1927,6 +1936,7 @@ App::delete('/v1/functions/:functionId/variables/:variableId') $dbForProject->updateDocument('functions', $function->getId(), $function->setAttribute('live', false)); + // Inform scheduler to pull the latest changes $schedule = $dbForConsole->getDocument('schedules', $function->getAttribute('scheduleId')); $schedule ->setAttribute('resourceUpdatedAt', DateTime::now()) diff --git a/app/controllers/api/project.php b/app/controllers/api/project.php index bc6a2de01f..e1a969070b 100644 --- a/app/controllers/api/project.php +++ b/app/controllers/api/project.php @@ -139,7 +139,8 @@ App::post('/v1/project/variables') ->inject('project') ->inject('response') ->inject('dbForProject') - ->action(function (string $key, string $value, Document $project, Response $response, Database $dbForProject) { + ->inject('dbForConsole') + ->action(function (string $key, string $value, Document $project, Response $response, Database $dbForProject, Database $dbForConsole) { $variableId = ID::unique(); $variable = new Document([ @@ -162,6 +163,7 @@ App::post('/v1/project/variables') } catch (DuplicateException $th) { throw new Exception(Exception::VARIABLE_ALREADY_EXISTS); } + $dbForConsole->deleteCachedDocument('projects', $project->getId()); $functions = $dbForProject->find('functions', [ Query::limit(APP_LIMIT_SUBQUERY) @@ -244,7 +246,8 @@ App::put('/v1/project/variables/:variableId') ->inject('project') ->inject('response') ->inject('dbForProject') - ->action(function (string $variableId, string $key, ?string $value, Document $project, Response $response, Database $dbForProject) { + ->inject('dbForConsole') + ->action(function (string $variableId, string $key, ?string $value, Document $project, Response $response, Database $dbForProject, Database $dbForConsole) { $variable = $dbForProject->getDocument('variables', $variableId); if ($variable === false || $variable->isEmpty() || $variable->getAttribute('resourceType') !== 'project') { throw new Exception(Exception::VARIABLE_NOT_FOUND); @@ -260,6 +263,7 @@ App::put('/v1/project/variables/:variableId') } catch (DuplicateException $th) { throw new Exception(Exception::VARIABLE_ALREADY_EXISTS); } + $dbForConsole->deleteCachedDocument('projects', $project->getId()); $functions = $dbForProject->find('functions', [ Query::limit(APP_LIMIT_SUBQUERY) diff --git a/app/controllers/api/proxy.php b/app/controllers/api/proxy.php index be771e7f31..a77580c26a 100644 --- a/app/controllers/api/proxy.php +++ b/app/controllers/api/proxy.php @@ -73,7 +73,7 @@ App::post('/v1/proxy/rules') $function = $dbForProject->getDocument('functions', $resourceId); if ($function->isEmpty()) { - throw new Exception(Exception::RULE_RESOURCE_ID_NOT_FOUND); + throw new Exception(Exception::RULE_RESOURCE_NOT_FOUND); } $resourceInternalId = $function->getInternalId(); diff --git a/app/controllers/general.php b/app/controllers/general.php index 09fbf32d6b..af0c733470 100644 --- a/app/controllers/general.php +++ b/app/controllers/general.php @@ -43,11 +43,11 @@ Config::setParam('domainVerification', false); Config::setParam('cookieDomain', 'localhost'); Config::setParam('cookieSamesite', Response::COOKIE_SAMESITE_NONE); -function router(App $utopia, Database $dbForConsole, SwooleRequest $swooleRequest, Response $response) +function router(App $utopia, Database $dbForConsole, SwooleRequest $swooleRequest, Request $request, Response $response) { $utopia->getRoute()->label('error', __DIR__ . '/../views/general/error.phtml'); - $host = $swooleRequest->header['host'] ?? ''; + $host = $request->getHostname() ?? ''; $route = Authorization::skip( fn() => $dbForConsole->find('rules', [ @@ -169,11 +169,12 @@ App::init() /* * Appwrite Router */ - $host = $swooleRequest->header['host'] ?? ''; + + $host = $request->getHostname() ?? ''; $mainDomain = App::getEnv('_APP_DOMAIN', ''); // Only run Router when external domain if ($host !== $mainDomain && $host !== 'localhost') { - if (router($utopia, $dbForConsole, $swooleRequest, $response)) { + if (router($utopia, $dbForConsole, $swooleRequest, $request, $response)) { return; } } @@ -460,11 +461,11 @@ App::options() /* * Appwrite Router */ - $host = $swooleRequest->header['host'] ?? ''; + $host = $request->getHostname() ?? ''; $mainDomain = App::getEnv('_APP_DOMAIN', ''); // Only run Router when external domain if ($host !== $mainDomain && $host !== 'localhost') { - if (router($utopia, $dbForConsole, $swooleRequest, $response)) { + if (router($utopia, $dbForConsole, $swooleRequest, $request, $response)) { return; } } diff --git a/app/init.php b/app/init.php index 497e4ff05b..3482b196c2 100644 --- a/app/init.php +++ b/app/init.php @@ -436,6 +436,21 @@ Database::addFilter( } ); +// READ-ONLY! TO update, write directly to 'variables' collection. After update to vars, make sure to deleteCachedDocument() +Database::addFilter( + 'subQueryProjectVariables', + function (mixed $value) { + return null; + }, + function (mixed $value, Document $document, Database $database) { + return $database + ->find('variables', [ + Query::equal('resourceType', ['project']), + Query::limit(APP_LIMIT_SUBQUERY) + ]); + } +); + /** * DB Formats */ diff --git a/app/workers/builds.php b/app/workers/builds.php index 15ee66cc05..1551b7d608 100644 --- a/app/workers/builds.php +++ b/app/workers/builds.php @@ -427,13 +427,12 @@ class BuildsV1 extends Worker /** Update function schedule */ $dbForConsole = $this->getConsoleDB(); + // Inform scheduler if function is still active $schedule = $dbForConsole->getDocument('schedules', $function->getAttribute('scheduleId')); - $schedule->setAttribute('resourceUpdatedAt', DateTime::now()); - $schedule + ->setAttribute('resourceUpdatedAt', DateTime::now()) ->setAttribute('schedule', $function->getAttribute('schedule')) ->setAttribute('active', !empty($function->getAttribute('schedule')) && !empty($function->getAttribute('deployment'))); - Authorization::skip(fn () => $dbForConsole->updateDocument('schedules', $schedule->getId(), $schedule)); } catch (\Throwable $th) { $endTime = DateTime::now(); diff --git a/app/workers/functions.php b/app/workers/functions.php index 53fbf57ac4..a942a8888f 100644 --- a/app/workers/functions.php +++ b/app/workers/functions.php @@ -129,11 +129,9 @@ Server::setResource('execute', function () { $vars = []; - // global vars - $vars = \array_merge($vars, \array_reduce($dbForProject->find('variables', [ - Query::equal('resourceType', ['project']), - Query::limit(APP_LIMIT_SUBQUERY) - ]), function (array $carry, Document $var) { + // Shared vars + $varsShared = $project->getAttribute('variables', []); + $vars = \array_merge($vars, \array_reduce($varsShared, function (array $carry, Document $var) { $carry[$var->getAttribute('key')] = $var->getAttribute('value') ?? ''; return $carry; }, [])); diff --git a/src/Appwrite/Extend/Exception.php b/src/Appwrite/Extend/Exception.php index 0341b26a61..b000dd1dfe 100644 --- a/src/Appwrite/Extend/Exception.php +++ b/src/Appwrite/Extend/Exception.php @@ -178,7 +178,7 @@ class Exception extends \Exception /** Proxy */ public const RULE_CONFIGURATION_MISSING = 'rule_configuration_missing'; public const RULE_RESOURCE_ID_MISSING = 'rule_resource_id_missing'; - public const RULE_RESOURCE_ID_NOT_FOUND = 'rule_resource_id_not_found'; + public const RULE_RESOURCE_NOT_FOUND = 'rule_resource_not_found'; public const RULE_NOT_FOUND = 'rule_not_found'; public const RULE_ALREADY_EXISTS = 'rule_already_exists'; public const RULE_VERIFICATION_FAILED = 'rule_verification_failed'; diff --git a/src/Appwrite/Utopia/Response/Model/Func.php b/src/Appwrite/Utopia/Response/Model/Func.php index 535895d0c1..54a4fd002c 100644 --- a/src/Appwrite/Utopia/Response/Model/Func.php +++ b/src/Appwrite/Utopia/Response/Model/Func.php @@ -97,7 +97,7 @@ class Func extends Model 'type' => self::TYPE_INTEGER, 'description' => 'Function execution timeout in seconds.', 'default' => 15, - 'example' => 1592981237, + 'example' => 300, ]) ->addRule('entrypoint', [ 'type' => self::TYPE_STRING, @@ -113,9 +113,9 @@ class Func extends Model ]) ->addRule('vcsInstallationId', [ 'type' => self::TYPE_STRING, - 'description' => 'Function vcs installation id.', + 'description' => 'Function VCS (Version Control System) installation id.', 'default' => '', - 'example' => '644051bd6572792165cc', + 'example' => '6m40at4ejk5h2u9s1hboo', ]) ->addRule('vcsRepositoryId', [ 'type' => self::TYPE_STRING, @@ -125,19 +125,19 @@ class Func extends Model ]) ->addRule('vcsBranch', [ 'type' => self::TYPE_STRING, - 'description' => 'Git branch name', + 'description' => 'VCS (Version Control System) branch name', 'default' => '', 'example' => 'main', ]) ->addRule('vcsRootDirectory', [ 'type' => self::TYPE_STRING, - 'description' => 'Path to function in git repository', + 'description' => 'Path to function in VCS (Version Control System) repository', 'default' => '', 'example' => 'functions/helloWorld', ]) ->addRule('vcsSilentMode', [ 'type' => self::TYPE_BOOLEAN, - 'description' => 'Is VCS connection is in silent mode?', + 'description' => 'Is VCS (Version Control System) connection is in silent mode? When in silence mode, no comments will be posted on the repository pull or merge requests', 'default' => false, 'example' => false, ])