From 92354838ba359eae1fdc94255365930dd9a91a75 Mon Sep 17 00:00:00 2001 From: Timon de Groot Date: Mon, 29 Apr 2024 08:58:59 +0200 Subject: [PATCH 1/3] service/settings: Fix job polling mechanism Instead of using the unauthorized internal job URL, which is returned by the API, parse the job uuid from the URL and use that ID for polling on the proper endpoint. Ideally this is fixed in the API, but this is how it's been done in kamikaze so far as well, so let's first start here. --- src/Defaults.php | 9 +++ src/HypernodeClientFactory.php | 4 +- src/Resource/Logbook/Job.php | 58 ++++++++++---- src/Service/Settings.php | 9 ++- tests/unit/Resource/Logbook/JobTest.php | 101 +++++++++++++++++++++--- tests/unit/Service/SettingsTest.php | 6 +- 6 files changed, 156 insertions(+), 31 deletions(-) create mode 100644 src/Defaults.php diff --git a/src/Defaults.php b/src/Defaults.php new file mode 100644 index 0000000..4210f59 --- /dev/null +++ b/src/Defaults.php @@ -0,0 +1,9 @@ + 'application/json', 'Content-Type' => 'application/json', ]; - $httpClient = self::getHttpClient(self::HYPERNODE_API_URL, $httpHeaders, $httpClient); + $httpClient = self::getHttpClient(Defaults::HYPERNODE_API_URL, $httpHeaders, $httpClient); $apiClient = new HttpMethodsClient( $httpClient, Psr17FactoryDiscovery::findRequestFactory(), Psr17FactoryDiscovery::findStreamFactory() diff --git a/src/Resource/Logbook/Job.php b/src/Resource/Logbook/Job.php index 35d62a5..bc6466b 100644 --- a/src/Resource/Logbook/Job.php +++ b/src/Resource/Logbook/Job.php @@ -6,6 +6,7 @@ use Hypernode\Api\HypernodeClient; use Hypernode\Api\Resource\AbstractResource; +use Hypernode\Api\Service\App; /** * @property-read string $result @@ -15,17 +16,19 @@ */ class Job extends AbstractResource { - private string $url; + private string $id; + private string $appName; private bool $exists = false; private bool $running = false; + private bool $success = false; private bool $complete = false; - protected HypernodeClient $client; - public function __construct(HypernodeClient $client, string $urlOrId, array $data = []) + public function __construct(HypernodeClient $client, string $appName, string $id, array $data = []) { $this->client = $client; - $this->url = $urlOrId; + $this->appName = $appName; + $this->id = $id; $this->data = $data; } @@ -37,28 +40,41 @@ public function __construct(HypernodeClient $client, string $urlOrId, array $dat */ public function refresh() { - $response = $this->client->api->get($this->url); + $url = sprintf(App::V1_APP_FLOWS_URL, $this->appName) . '?' . http_build_query(['tracker_uuid' => $this->id]); + $response = $this->client->api->get($url); + $this->data = $this->client->getJsonFromResponse($response); - if ($response->getStatusCode() === 404) { + if ($response->getStatusCode() === 404 || $this->data['count'] === 0) { $this->data = []; $this->exists = false; $this->running = false; return; } - if ($response->getStatusCode() === 303) { - $this->data = []; - $this->exists = true; - $this->running = false; - $this->complete = true; - return; + $this->exists = true; + + $result = $this->data['results'][0]; + switch ($result['state']) { + case 'running': + $this->running = true; + break; + case 'success': + $this->success = true; + $this->running = false; + $this->complete = true; + break; + case 'reverted': + $this->running = false; + $this->complete = true; + break; } $this->client->maybeThrowApiExceptions($response); + } - $this->data = $this->client->getJsonFromResponse($response); - $this->exists = true; - $this->running = true; + public function id() + { + return $this->id; } public function exists(): bool @@ -66,8 +82,18 @@ public function exists(): bool return $this->exists; } + public function running(): bool + { + return $this->running; + } + public function completed(): bool { return $this->complete; } -} \ No newline at end of file + + public function success(): bool + { + return $this->success; + } +} diff --git a/src/Service/Settings.php b/src/Service/Settings.php index 9a4d962..4fe3e81 100644 --- a/src/Service/Settings.php +++ b/src/Service/Settings.php @@ -4,10 +4,13 @@ namespace Hypernode\Api\Service; +use Hypernode\Api\Defaults; use Hypernode\Api\Resource\Logbook\Job; class Settings extends AbstractService { + public const JOB_URL_REGEX = '#' . Defaults::HYPERNODE_API_URL. 'logbook/v1/jobs/(.*)/#'; + public function set(string $app, string $key, string $value): ?Job { return $this->setBatch($app, [$key => $value]); @@ -21,7 +24,11 @@ public function setBatch(string $app, array $settings): ?Job $this->client->maybeThrowApiExceptions($response); if ($response->getStatusCode() === 202) { - $job = new Job($this->client, $response->getHeaderLine('Location')); + $location = $response->getHeaderLine('Location'); + if (!preg_match(self::JOB_URL_REGEX, $location, $matches)) { + return null; + } + $job = new Job($this->client, $app, $matches[1]); return $job; } diff --git a/tests/unit/Resource/Logbook/JobTest.php b/tests/unit/Resource/Logbook/JobTest.php index 54be51c..f200396 100644 --- a/tests/unit/Resource/Logbook/JobTest.php +++ b/tests/unit/Resource/Logbook/JobTest.php @@ -16,8 +16,7 @@ class JobTest extends HypernodeClientTestCase protected function setUp(): void { parent::setUp(); - $this->jobUrl = "https://api.hypernode.com/logbook/v1/jobs/abcd/"; - $this->job = new Job($this->client, $this->jobUrl); + $this->job = new Job($this->client, 'johndoe', 'abcd'); } public function testIsInstanceOfAbstractResource() @@ -29,17 +28,100 @@ public function testRefresh() { $this->responses->append( new Response(404, [], json_encode([])), + new Response(200, [], json_encode(['count' => 0, 'results' => []])), new Response(200, [], json_encode([ - 'result' => 'pending', - 'flow_name' => 'update_node', - 'app_name' => 'johndoe' + 'count' => 1, + 'results' => [ + [ + 'uuid' => 'abcd', + 'state' => NULL, + 'name' => 'update_node' + ] + ] ])), new Response(200, [], json_encode([ - 'result' => 'running', - 'flow_name' => 'update_node', - 'app_name' => 'johndoe' + 'count' => 1, + 'results' => [ + [ + 'uuid' => 'abcd', + 'state' => 'running', + 'name' => 'update_node' + ] + ] + ])), + new Response(200, [], json_encode([ + 'count' => 1, + 'results' => [ + [ + 'uuid' => 'abcd', + 'state' => 'success', + 'name' => 'update_node' + ] + ] + ])), + ); + + $this->job->refresh(); + + $this->assertFalse($this->job->exists()); + $this->assertFalse($this->job->completed()); + + $this->job->refresh(); + + $this->assertFalse($this->job->exists()); + $this->assertFalse($this->job->completed()); + + $this->job->refresh(); + + $this->assertTrue($this->job->exists()); + $this->assertFalse($this->job->completed()); + + $this->job->refresh(); + + $this->assertTrue($this->job->exists()); + $this->assertFalse($this->job->completed()); + + $this->job->refresh(); + + $this->assertTrue($this->job->exists()); + $this->assertTrue($this->job->completed()); + $this->assertTrue($this->job->success()); + } + + public function testRefreshFailedJob() + { + $this->responses->append( + new Response(404, [], json_encode([])), + new Response(200, [], json_encode([ + 'count' => 1, + 'results' => [ + [ + 'uuid' => 'abcd', + 'state' => NULL, + 'name' => 'update_node' + ] + ] + ])), + new Response(200, [], json_encode([ + 'count' => 1, + 'results' => [ + [ + 'uuid' => 'abcd', + 'state' => 'running', + 'name' => 'update_node' + ] + ] + ])), + new Response(200, [], json_encode([ + 'count' => 1, + 'results' => [ + [ + 'uuid' => 'abcd', + 'state' => 'reverted', + 'name' => 'update_node' + ] + ] ])), - new Response(303, [], json_encode([])), ); $this->job->refresh(); @@ -61,6 +143,7 @@ public function testRefresh() $this->assertTrue($this->job->exists()); $this->assertTrue($this->job->completed()); + $this->assertFalse($this->job->success()); } public function testExistsReturnsFalseByDefault() diff --git a/tests/unit/Service/SettingsTest.php b/tests/unit/Service/SettingsTest.php index eb0ad0d..01b2bfb 100644 --- a/tests/unit/Service/SettingsTest.php +++ b/tests/unit/Service/SettingsTest.php @@ -33,7 +33,7 @@ public function testSetSingleSettingToDifferentValue() { $jobUrl = 'https://api.hypernode.com/logbook/v1/jobs/abcd/'; $this->responses->append( - new Response(202, ['Location: ' . $jobUrl], null), + new Response(202, ['Location' => $jobUrl], null), ); $job = $this->client->settings->set('johndoe', 'php_version', '8.1'); @@ -41,6 +41,7 @@ public function testSetSingleSettingToDifferentValue() $request = $this->responses->getLastRequest(); $this->assertNotNull($job); + $this->assertEquals('abcd', $job->id()); $this->assertEquals('PATCH', $request->getMethod()); $this->assertEquals('/v2/app/johndoe/', $request->getUri()); $this->assertJson((string)$request->getBody()); @@ -54,7 +55,7 @@ public function testSetMultipleSettings() { $jobUrl = 'https://api.hypernode.com/logbook/v1/jobs/abcd/'; $this->responses->append( - new Response(202, ['Location: ' . $jobUrl], null), + new Response(202, ['Location' => $jobUrl], null), ); $job = $this->client->settings->setBatch( @@ -68,6 +69,7 @@ public function testSetMultipleSettings() $request = $this->responses->getLastRequest(); $this->assertNotNull($job); + $this->assertEquals('abcd', $job->id()); $this->assertEquals('PATCH', $request->getMethod()); $this->assertEquals('/v2/app/johndoe/', $request->getUri()); $this->assertJson((string)$request->getBody()); From 0c3eb015da239afc5185e04636ad19f158a589d4 Mon Sep 17 00:00:00 2001 From: Eelco Kruders Date: Mon, 26 May 2025 15:58:05 +0200 Subject: [PATCH 2/3] Updated nesbot/carbon from v2 to v3 in composer.json --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index ff00b0a..dcf6291 100644 --- a/composer.json +++ b/composer.json @@ -22,7 +22,7 @@ "composer-runtime-api": "^2.0", "ext-curl": "*", "ext-json": "*", - "nesbot/carbon": "^2.0", + "nesbot/carbon": "^3.0", "psr/http-client-implementation": "^1.0", "php-http/client-common": "^2.5", "php-http/discovery": "^1.14", From 0f8d16663999fb966a63fe074b07578a40cb4ce6 Mon Sep 17 00:00:00 2001 From: Timon de Groot Date: Fri, 30 May 2025 16:19:35 +0200 Subject: [PATCH 3/3] ci: Test against supported PHP versions 8.1 - 8.4 --- .github/workflows/test.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 87c193c..fdec017 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -9,13 +9,13 @@ jobs: code_quality: strategy: matrix: - php_version: [7.4, 8.0, 8.1] + php_version: [8.1, 8.2, 8.3, 8.4] runs-on: ubuntu-latest steps: - name: Checkout hypernode-deploy uses: actions/checkout@v3 - name: Install PHP - uses: shivammathur/setup-php@2.21.1 + uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php_version }} tools: composer:v2