Merge pull request #220 from open-runtimes/refactor-rector

refactor: rector

Author: loks0n

.github/workflows/ci.yml

@@ -36,6 +36,19 @@ jobs:
         docker run --rm -v $PWD:/app composer:2.8 sh -c \
         "composer install --profile --ignore-platform-reqs && composer analyze"
 
+  refactor:
+    name: Refactor
+    runs-on: ubuntu-latest
+
+    steps:
+    - name: Check out the repo
+      uses: actions/checkout@v6
+
+    - name: Run Refactor
+      run: |
+        docker run --rm -v $PWD:/app composer:2.8 sh -c \
+        "composer install --profile --ignore-platform-reqs && composer refactor:check"
+
   unit-tests:
     name: Unit Tests
     runs-on: ubuntu-latest

app/controllers.php

@@ -25,7 +25,7 @@
     ->param('timeout', '600', new Text(16), 'Maximum logs timeout.', true)
     ->inject('response')
     ->inject('runner')
-    ->action(function (string $runtimeId, string $timeoutStr, Response $response, Runner $runner) {
+    ->action(function (string $runtimeId, string $timeoutStr, Response $response, Runner $runner): void {
         $timeout = \intval($timeoutStr);
 
         $response->sendHeader('Content-Type', 'text/event-stream');
@@ -43,7 +43,7 @@
     ->param('timeout', 600, new Integer(), 'Commands execution time in seconds.', true)
     ->inject('response')
     ->inject('runner')
-    ->action(function (string $runtimeId, string $command, int $timeout, Response $response, Runner $runner) {
+    ->action(function (string $runtimeId, string $command, int $timeout, Response $response, Runner $runner): void {
         $output = $runner->executeCommand($runtimeId, $command, $timeout);
         $response->setStatusCode(Response::STATUS_CODE_OK)->json([ 'output' => $output ]);
     });
@@ -68,7 +68,7 @@
     ->param('restartPolicy', DockerAPI::RESTART_NO, new WhiteList([DockerAPI::RESTART_NO, DockerAPI::RESTART_ALWAYS, DockerAPI::RESTART_ON_FAILURE, DockerAPI::RESTART_UNLESS_STOPPED], true), 'Define restart policy for the runtime once an exit code is returned. Default value is "no". Possible values are "no", "always", "on-failure", "unless-stopped".', true)
     ->inject('response')
     ->inject('runner')
-    ->action(function (string $runtimeId, string $image, string $entrypoint, string $source, string $destination, string $outputDirectory, array $variables, string $runtimeEntrypoint, string $command, int $timeout, bool $remove, float $cpus, int $memory, string $version, string $restartPolicy, Response $response, Runner $runner) {
+    ->action(function (string $runtimeId, string $image, string $entrypoint, string $source, string $destination, string $outputDirectory, array $variables, string $runtimeEntrypoint, string $command, int $timeout, bool $remove, float $cpus, int $memory, string $version, string $restartPolicy, Response $response, Runner $runner): void {
         $secret = \bin2hex(\random_bytes(16));
 
         /**
@@ -90,7 +90,7 @@
             default => [],
         });
 
-        if (!empty($outputDirectory)) {
+        if ($outputDirectory !== '' && $outputDirectory !== '0') {
             $variables = \array_merge($variables, [
                 'OPEN_RUNTIMES_OUTPUT_DIRECTORY' => $outputDirectory
             ]);
@@ -100,7 +100,7 @@
             'CI' => 'true'
         ]);
 
-        $variables = array_map(fn ($v) => strval($v), $variables);
+        $variables = array_map(strval(...), $variables);
 
         $container = $runner->createRuntime($runtimeId, $secret, $image, $entrypoint, $source, $destination, $variables, $runtimeEntrypoint, $command, $timeout, $remove, $cpus, $memory, $version, $restartPolicy);
         $response->setStatusCode(Response::STATUS_CODE_CREATED)->json($container);
@@ -111,7 +111,7 @@
     ->desc("List currently active runtimes")
     ->inject('runner')
     ->inject('response')
-    ->action(function (Runner $runner, Response $response) {
+    ->action(function (Runner $runner, Response $response): void {
         $response->setStatusCode(Response::STATUS_CODE_OK)->json($runner->getRuntimes());
     });
 
@@ -121,7 +121,7 @@
     ->param('runtimeId', '', new Text(64), 'Runtime unique ID.')
     ->inject('runner')
     ->inject('response')
-    ->action(function (string $runtimeId, Runner $runner, Response $response) {
+    ->action(function (string $runtimeId, Runner $runner, Response $response): void {
         $runtimeName = System::getHostname() . '-' . $runtimeId;
         $response->setStatusCode(Response::STATUS_CODE_OK)->json($runner->getRuntime($runtimeName));
     });
@@ -132,7 +132,7 @@
     ->param('runtimeId', '', new Text(64), 'Runtime unique ID.')
     ->inject('response')
     ->inject('runner')
-    ->action(function (string $runtimeId, Response $response, Runner $runner) {
+    ->action(function (string $runtimeId, Response $response, Runner $runner): void {
         $runner->deleteRuntime($runtimeId);
         $response->setStatusCode(Response::STATUS_CODE_OK)->send();
     });
@@ -183,39 +183,18 @@ function (
             Response $response,
             Request $request,
             Runner $runner
-        ) {
-            // Extra parsers and validators to support both JSON and multipart
-            $intParams = ['timeout', 'memory'];
-            foreach ($intParams as $intParam) {
-                if (!empty($$intParam) && !is_numeric($$intParam)) {
-                    $$intParam = \intval($$intParam);
-                }
+        ): void {
+            // Parse JSON strings for assoc params when coming from multipart
+            if (\is_string($headers)) {
+                $headers = \json_decode($headers, true) ?? [];
             }
 
-            $floatParams = ['cpus'];
-            foreach ($floatParams as $floatPram) {
-                if (!empty($$floatPram) && !is_numeric($$floatPram)) {
-                    $$floatPram = \floatval($$floatPram);
-                }
+            if (\is_string($variables)) {
+                $variables = \json_decode($variables, true) ?? [];
             }
 
-            /**
-             * @var array<string, mixed> $headers
-             * @var array<string, mixed> $variables
-             */
-            $assocParams = ['headers', 'variables'];
-            foreach ($assocParams as $assocParam) {
-                if (!empty($$assocParam) && !is_array($$assocParam)) {
-                    $$assocParam = \json_decode($$assocParam, true);
-                }
-            }
-
-            $booleanParams = ['logging'];
-            foreach ($booleanParams as $booleamParam) {
-                if (!empty($$booleamParam) && !is_bool($$booleamParam)) {
-                    $$booleamParam = $$booleamParam === "true" ? true : false;
-                }
-            }
+            /** @var array<string, mixed> $headers */
+            /** @var array<string, mixed> $variables */
 
             // 'headers' validator
             $validator = new Assoc();
@@ -229,11 +208,11 @@ function (
                 throw new Exception(Exception::EXECUTION_BAD_REQUEST, $validator->getDescription());
             }
 
-            if (empty($payload)) {
+            if (in_array($payload, [null, '', '0'], true)) {
                 $payload = '';
             }
 
-            $variables = array_map(fn ($v) => strval($v), $variables);
+            $variables = array_map(strval(...), $variables);
 
             $execution = $runner->createExecution(
                 $runtimeId,
@@ -303,16 +282,16 @@ function (
 Http::get('/v1/health')
     ->desc("Get health status")
     ->inject('response')
-    ->action(function (Response $response) {
+    ->action(function (Response $response): void {
         $response->setStatusCode(Response::STATUS_CODE_OK)->text("OK");
     });
 
 Http::init()
     ->groups(['api'])
     ->inject('request')
-    ->action(function (Request $request) {
+    ->action(function (Request $request): void {
         $secretKey = \explode(' ', $request->getHeader('authorization', ''))[1] ?? '';
-        if (empty($secretKey) || $secretKey !== System::getEnv('OPR_EXECUTOR_SECRET', '')) {
+        if ($secretKey === '' || $secretKey === '0' || $secretKey !== System::getEnv('OPR_EXECUTOR_SECRET', '')) {
             throw new Exception(Exception::GENERAL_UNAUTHORIZED, 'Missing executor key');
         }
     });

app/error.php

@@ -8,7 +8,7 @@
 Http::error()
     ->inject('error')
     ->inject('response')
-    ->action(function (Throwable $error, Response $response) {
+    ->action(function (Throwable $error, Response $response): void {
         // Show all Executor\Exceptions, or everything if in development
         $public = $error instanceof Exception || Http::isDevelopment();
         $exception = $public ? $error : new Exception(Exception::GENERAL_UNKNOWN);

app/http.php

@@ -32,7 +32,7 @@
     ->inject('network')
     ->inject('imagePuller')
     ->inject('maintenance')
-    ->action(function (Orchestration $orchestration, Network $network, ImagePuller $imagePuller, Maintenance $maintenance) {
+    ->action(function (Orchestration $orchestration, Network $network, ImagePuller $imagePuller, Maintenance $maintenance): void {
         /* Fetch own container information */
         $hostname = gethostname() ?: throw new \RuntimeException('Could not determine hostname');
         $selfContainer = $orchestration->list(['name' => $hostname])[0] ?? throw new \RuntimeException('Own container not found');
@@ -58,11 +58,11 @@
 
 Http::onRequest()
     ->inject('response')
-    ->action(function (Response $response) {
+    ->action(function (Response $response): void {
         $response->addHeader('Server', 'Executor');
     });
 
-run(function () use ($settings) {
+run(function () use ($settings): void {
     $server = new Server('0.0.0.0', '80', $settings);
     $http = new Http($server, 'UTC');
     $http->start();

app/init.php

@@ -14,13 +14,13 @@
 use Utopia\Config\Config;
 
 const MAX_LOG_SIZE = 5 * 1024 * 1024;
-const MAX_BUILD_LOG_SIZE = 1 * 1000 * 1000;
+const MAX_BUILD_LOG_SIZE = 1000 * 1000;
 
 Config::load('errors', __DIR__ . '/config/errors.php');
 
 $registry = new Registry();
 
-$registry->set('runtimes', fn () => new Runtimes());
+$registry->set('runtimes', fn (): \OpenRuntimes\Executor\Runner\Repository\Runtimes => new Runtimes());
 
 Http::setResource('runtimes', fn () => $registry->get('runtimes'));
 

composer.json

@@ -13,9 +13,11 @@
   "scripts": {
     "format": "./vendor/bin/pint --config pint.json",
     "format:check": "./vendor/bin/pint --test --config pint.json",
-    "analyze": "./vendor/bin/phpstan analyse --level 8 --memory-limit=2G -c phpstan.neon app src tests",
+    "analyze": "./vendor/bin/phpstan analyse --memory-limit=1G -c phpstan.neon app src tests",
     "test:unit": "./vendor/bin/phpunit --configuration phpunit.xml --debug --testsuite=unit",
-    "test:e2e": "./vendor/bin/phpunit --configuration phpunit.xml --debug --testsuite=e2e"
+    "test:e2e": "./vendor/bin/phpunit --configuration phpunit.xml --debug --testsuite=e2e",
+    "refactor": "./vendor/bin/rector",
+    "refactor:check": "./vendor/bin/rector --dry-run"
   },
   "require": {
     "php": ">=8.3.0",
@@ -35,9 +37,10 @@
   },
   "require-dev": {
     "laravel/pint": "1.*",
-    "phpstan/phpstan": "1.*",
+    "phpstan/phpstan": "2.*",
     "phpunit/phpunit": "9.*",
-    "swoole/ide-helper": "5.1.2"
+    "swoole/ide-helper": "5.1.2",
+    "rector/rector": "2.3.1"
   },
   "config": {
     "platform": {