From 25f5c83594a3d0ff6713d579f6ac6c100a691a0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 8 Aug 2024 08:38:15 +0000 Subject: [PATCH 1/6] Fix 4XX execution status --- app/controllers/api/functions.php | 2 +- app/controllers/general.php | 2 +- src/Appwrite/Platform/Workers/Functions.php | 2 +- .../Functions/FunctionsCustomServerTest.php | 67 +++++++++++++++++++ tests/resources/functions/php-4xx/index.php | 5 ++ 5 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 tests/resources/functions/php-4xx/index.php diff --git a/app/controllers/api/functions.php b/app/controllers/api/functions.php index f155d7427c..e08c69b8fb 100644 --- a/app/controllers/api/functions.php +++ b/app/controllers/api/functions.php @@ -1878,7 +1878,7 @@ App::post('/v1/functions/:functionId/executions') } /** Update execution status */ - $status = $executionResponse['statusCode'] >= 400 ? 'failed' : 'completed'; + $status = $executionResponse['statusCode'] >= 500 ? 'failed' : 'completed'; $execution->setAttribute('status', $status); $execution->setAttribute('responseStatusCode', $executionResponse['statusCode']); $execution->setAttribute('responseHeaders', $headersFiltered); diff --git a/app/controllers/general.php b/app/controllers/general.php index d532110df3..95e6dc18df 100644 --- a/app/controllers/general.php +++ b/app/controllers/general.php @@ -302,7 +302,7 @@ function router(App $utopia, Database $dbForConsole, callable $getProjectDB, Swo } /** Update execution status */ - $status = $executionResponse['statusCode'] >= 400 ? 'failed' : 'completed'; + $status = $executionResponse['statusCode'] >= 500 ? 'failed' : 'completed'; $execution->setAttribute('status', $status); $execution->setAttribute('responseStatusCode', $executionResponse['statusCode']); $execution->setAttribute('responseHeaders', $headersFiltered); diff --git a/src/Appwrite/Platform/Workers/Functions.php b/src/Appwrite/Platform/Workers/Functions.php index 78671bfeb0..8ade08dc2d 100644 --- a/src/Appwrite/Platform/Workers/Functions.php +++ b/src/Appwrite/Platform/Workers/Functions.php @@ -494,7 +494,7 @@ class Functions extends Action logging: $function->getAttribute('logging', true), ); - $status = $executionResponse['statusCode'] >= 400 ? 'failed' : 'completed'; + $status = $executionResponse['statusCode'] >= 500 ? 'failed' : 'completed'; $headersFiltered = []; foreach ($executionResponse['headers'] as $key => $value) { diff --git a/tests/e2e/Services/Functions/FunctionsCustomServerTest.php b/tests/e2e/Services/Functions/FunctionsCustomServerTest.php index e3148752c8..30f7c54df7 100644 --- a/tests/e2e/Services/Functions/FunctionsCustomServerTest.php +++ b/tests/e2e/Services/Functions/FunctionsCustomServerTest.php @@ -1659,6 +1659,73 @@ class FunctionsCustomServerTest extends Scope $this->assertEquals(204, $response['headers']['status-code']); } + public function text4xxExecution() + { + $timeout = 15; + $code = realpath(__DIR__ . '/../../../resources/functions') . "/php-4xx/code.tar.gz"; + $this->packageCode('php-4xx'); + + $function = $this->client->call(Client::METHOD_POST, '/functions', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'functionId' => ID::unique(), + 'name' => 'Test PHP 4XX executions', + 'runtime' => 'php-8.0', + 'entrypoint' => 'index.php', + 'timeout' => $timeout, + ]); + + $functionId = $function['body']['$id'] ?? ''; + + $this->assertEquals(201, $function['headers']['status-code']); + + $deployment = $this->client->call(Client::METHOD_POST, '/functions/' . $functionId . '/deployments', array_merge([ + 'content-type' => 'multipart/form-data', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'entrypoint' => 'index.php', + 'code' => new CURLFile($code, 'application/x-gzip', basename($code)), + 'activate' => true + ]); + + $deploymentId = $deployment['body']['$id'] ?? ''; + $this->assertEquals(202, $deployment['headers']['status-code']); + + $this->awaitDeploymentIsBuilt($function['body']['$id'], $deploymentId, checkForSuccess: false); + + $deployment = $this->client->call(Client::METHOD_PATCH, '/functions/' . $functionId . '/deployments/' . $deploymentId, array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), []); + + $this->assertEquals(200, $deployment['headers']['status-code']); + + // Wait a little for activation to finish + sleep(5); + + $execution = $this->client->call(Client::METHOD_POST, '/functions/' . $functionId . '/executions', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'async' => false + ]); + + $this->assertEquals(201, $execution['headers']['status-code']); + $this->assertEquals('completed', $execution['body']['status']); + $this->assertEquals(400, $execution['body']['responseStatusCode']); + $this->assertEquals('Invalid input.', $execution['body']['responseBody']); + + // Cleanup : Delete function + $response = $this->client->call(Client::METHOD_DELETE, '/functions/' . $functionId, [ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'], + ], []); + + $this->assertEquals(204, $response['headers']['status-code']); + } + public function testFunctionsDomain() { $timeout = 15; diff --git a/tests/resources/functions/php-4xx/index.php b/tests/resources/functions/php-4xx/index.php new file mode 100644 index 0000000000..92f0bf28fe --- /dev/null +++ b/tests/resources/functions/php-4xx/index.php @@ -0,0 +1,5 @@ +res->text('Invalid input.', 400); +}; From f8fb98d377663aad2333f115de7b894c2763a9a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 8 Aug 2024 08:42:38 +0000 Subject: [PATCH 2/6] Update response filter --- src/Appwrite/Utopia/Response/Filters/V18.php | 4 ++ .../unit/Utopia/Response/Filters/V18Test.php | 40 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/src/Appwrite/Utopia/Response/Filters/V18.php b/src/Appwrite/Utopia/Response/Filters/V18.php index d0aa680e3b..62a19e3985 100644 --- a/src/Appwrite/Utopia/Response/Filters/V18.php +++ b/src/Appwrite/Utopia/Response/Filters/V18.php @@ -24,6 +24,10 @@ class V18 extends Filter protected function parseExecution(array $content) { + if($content['status'] === 'completed' && $content['statusCode'] >= 400 && $content['statusCode'] < 500) { + $content['status'] === 'failed'; + } + unset($content['scheduledAt']); return $content; } diff --git a/tests/unit/Utopia/Response/Filters/V18Test.php b/tests/unit/Utopia/Response/Filters/V18Test.php index c4011c08a1..2110e518a9 100644 --- a/tests/unit/Utopia/Response/Filters/V18Test.php +++ b/tests/unit/Utopia/Response/Filters/V18Test.php @@ -34,6 +34,46 @@ class V18Test extends TestCase ], [ ] + ], + 'update 404 status' => [ + [ + 'statusCode' => '404', + 'status' => 'completed' + ], + [ + 'statusCode' => '404', + 'status' => 'failed' + ] + ], + 'update 400 status' => [ + [ + 'statusCode' => '400', + 'status' => 'completed' + ], + [ + 'statusCode' => '400', + 'status' => 'failed' + ] + ], + 'dont update 200 status' => [ + [ + 'statusCode' => '200', + 'status' => 'completed' + ], + [ + 'statusCode' => '200', + 'status' => 'completed' + ] + ], + 'dont update 500 status' => [ + [ + 'statusCode' => '500', + 'status' => 'failed' + ], + [ + 'statusCode' => '500', + 'status' => 'failed' + ] ] ]; } From bc4a2508138424f3e981ab9ceaf7dd615c872643 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 8 Aug 2024 08:51:38 +0000 Subject: [PATCH 3/6] Fix bug in response filter --- src/Appwrite/Utopia/Response/Filters/V18.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Appwrite/Utopia/Response/Filters/V18.php b/src/Appwrite/Utopia/Response/Filters/V18.php index 62a19e3985..2b78d60b37 100644 --- a/src/Appwrite/Utopia/Response/Filters/V18.php +++ b/src/Appwrite/Utopia/Response/Filters/V18.php @@ -25,7 +25,7 @@ class V18 extends Filter protected function parseExecution(array $content) { if($content['status'] === 'completed' && $content['statusCode'] >= 400 && $content['statusCode'] < 500) { - $content['status'] === 'failed'; + $content['status'] = 'failed'; } unset($content['scheduledAt']); From 90b9d176ba9cb29d1292855aa4bd07d6e88c28fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 8 Aug 2024 09:02:49 +0000 Subject: [PATCH 4/6] Fix unit test placement --- .../unit/Utopia/Response/Filters/V18Test.php | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/tests/unit/Utopia/Response/Filters/V18Test.php b/tests/unit/Utopia/Response/Filters/V18Test.php index 2110e518a9..5396779b77 100644 --- a/tests/unit/Utopia/Response/Filters/V18Test.php +++ b/tests/unit/Utopia/Response/Filters/V18Test.php @@ -35,6 +35,32 @@ class V18Test extends TestCase [ ] ], + ]; + } + + /** + * @dataProvider functionProvider + */ + public function testFunction(array $content, array $expected): void + { + $model = Response::MODEL_FUNCTION; + + $result = $this->filter->parse($content, $model); + + $this->assertEquals($expected, $result); + } + + + public function executionProvider(): array + { + return [ + 'remove scheduledAt' => [ + [ + 'scheduledAt' => '2024-07-13T09:00:00.000Z', + ], + [ + ] + ], 'update 404 status' => [ [ 'statusCode' => '404', @@ -78,32 +104,6 @@ class V18Test extends TestCase ]; } - /** - * @dataProvider functionProvider - */ - public function testFunction(array $content, array $expected): void - { - $model = Response::MODEL_FUNCTION; - - $result = $this->filter->parse($content, $model); - - $this->assertEquals($expected, $result); - } - - - public function executionProvider(): array - { - return [ - 'remove scheduledAt' => [ - [ - 'scheduledAt' => '2024-07-13T09:00:00.000Z', - ], - [ - ] - ] - ]; - } - /** * @dataProvider executionProvider */ From 47316a250b375b04ffee29243d906820e8fef177 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Thu, 8 Aug 2024 09:15:53 +0000 Subject: [PATCH 5/6] Edge case for test case --- src/Appwrite/Utopia/Response/Filters/V18.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Appwrite/Utopia/Response/Filters/V18.php b/src/Appwrite/Utopia/Response/Filters/V18.php index 2b78d60b37..6485b6f0ba 100644 --- a/src/Appwrite/Utopia/Response/Filters/V18.php +++ b/src/Appwrite/Utopia/Response/Filters/V18.php @@ -24,8 +24,10 @@ class V18 extends Filter protected function parseExecution(array $content) { - if($content['status'] === 'completed' && $content['statusCode'] >= 400 && $content['statusCode'] < 500) { - $content['status'] = 'failed'; + if(!empty($content['status']) && !empty($content['statusCode'])) { + if($content['status'] === 'completed' && $content['statusCode'] >= 400 && $content['statusCode'] < 500) { + $content['status'] = 'failed'; + } } unset($content['scheduledAt']); From 1334cf116bd3d50fe75e27e551c474a152889a15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Ba=C4=8Do?= Date: Mon, 12 Aug 2024 07:26:20 +0000 Subject: [PATCH 6/6] Speed up new test --- .../Functions/FunctionsCustomServerTest.php | 84 ++++--------------- tests/resources/functions/php-4xx/index.php | 5 -- tests/resources/functions/php/index.php | 4 +- 3 files changed, 20 insertions(+), 73 deletions(-) delete mode 100644 tests/resources/functions/php-4xx/index.php diff --git a/tests/e2e/Services/Functions/FunctionsCustomServerTest.php b/tests/e2e/Services/Functions/FunctionsCustomServerTest.php index 30f7c54df7..0b6bf985c1 100644 --- a/tests/e2e/Services/Functions/FunctionsCustomServerTest.php +++ b/tests/e2e/Services/Functions/FunctionsCustomServerTest.php @@ -802,6 +802,23 @@ class FunctionsCustomServerTest extends Scope $this->assertEquals('', $execution['body']['logs']); $this->assertLessThan(10, $execution['body']['duration']); + $execution = $this->client->call(Client::METHOD_POST, '/functions/' . $data['functionId'] . '/executions', array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), [ + 'async' => false, + 'path' => '/?code=400' + ]); + $this->assertEquals(201, $execution['headers']['status-code']); + $this->assertEquals('completed', $execution['body']['status']); + $this->assertEquals(400, $execution['body']['responseStatusCode']); + + $execution = $this->client->call(Client::METHOD_DELETE, '/functions/' . $data['functionId'] . '/executions/' . $execution['body']['$id'], array_merge([ + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], $this->getHeaders()), []); + $this->assertEquals(204, $execution['headers']['status-code']); + return array_merge($data, ['executionId' => $executionId]); } @@ -1659,73 +1676,6 @@ class FunctionsCustomServerTest extends Scope $this->assertEquals(204, $response['headers']['status-code']); } - public function text4xxExecution() - { - $timeout = 15; - $code = realpath(__DIR__ . '/../../../resources/functions') . "/php-4xx/code.tar.gz"; - $this->packageCode('php-4xx'); - - $function = $this->client->call(Client::METHOD_POST, '/functions', array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), [ - 'functionId' => ID::unique(), - 'name' => 'Test PHP 4XX executions', - 'runtime' => 'php-8.0', - 'entrypoint' => 'index.php', - 'timeout' => $timeout, - ]); - - $functionId = $function['body']['$id'] ?? ''; - - $this->assertEquals(201, $function['headers']['status-code']); - - $deployment = $this->client->call(Client::METHOD_POST, '/functions/' . $functionId . '/deployments', array_merge([ - 'content-type' => 'multipart/form-data', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), [ - 'entrypoint' => 'index.php', - 'code' => new CURLFile($code, 'application/x-gzip', basename($code)), - 'activate' => true - ]); - - $deploymentId = $deployment['body']['$id'] ?? ''; - $this->assertEquals(202, $deployment['headers']['status-code']); - - $this->awaitDeploymentIsBuilt($function['body']['$id'], $deploymentId, checkForSuccess: false); - - $deployment = $this->client->call(Client::METHOD_PATCH, '/functions/' . $functionId . '/deployments/' . $deploymentId, array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), []); - - $this->assertEquals(200, $deployment['headers']['status-code']); - - // Wait a little for activation to finish - sleep(5); - - $execution = $this->client->call(Client::METHOD_POST, '/functions/' . $functionId . '/executions', array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - ], $this->getHeaders()), [ - 'async' => false - ]); - - $this->assertEquals(201, $execution['headers']['status-code']); - $this->assertEquals('completed', $execution['body']['status']); - $this->assertEquals(400, $execution['body']['responseStatusCode']); - $this->assertEquals('Invalid input.', $execution['body']['responseBody']); - - // Cleanup : Delete function - $response = $this->client->call(Client::METHOD_DELETE, '/functions/' . $functionId, [ - 'content-type' => 'application/json', - 'x-appwrite-project' => $this->getProject()['$id'], - 'x-appwrite-key' => $this->getProject()['apiKey'], - ], []); - - $this->assertEquals(204, $response['headers']['status-code']); - } - public function testFunctionsDomain() { $timeout = 15; diff --git a/tests/resources/functions/php-4xx/index.php b/tests/resources/functions/php-4xx/index.php deleted file mode 100644 index 92f0bf28fe..0000000000 --- a/tests/resources/functions/php-4xx/index.php +++ /dev/null @@ -1,5 +0,0 @@ -res->text('Invalid input.', 400); -}; diff --git a/tests/resources/functions/php/index.php b/tests/resources/functions/php/index.php index d5328c40e1..e8e5eb67d5 100644 --- a/tests/resources/functions/php/index.php +++ b/tests/resources/functions/php/index.php @@ -1,6 +1,8 @@ req->query['code'] ?? '200'; + return $context->res->json([ 'APPWRITE_FUNCTION_ID' => \getenv('APPWRITE_FUNCTION_ID') ?: '', 'APPWRITE_FUNCTION_NAME' => \getenv('APPWRITE_FUNCTION_NAME') ?: '', @@ -11,5 +13,5 @@ return function ($context) { 'APPWRITE_REGION' => \getenv('APPWRITE_REGION') ?: '', 'UNICODE_TEST' => "êä", 'GLOBAL_VARIABLE' => \getenv('GLOBAL_VARIABLE') ?: '' - ]); + ], \intval($statusCode)); };