From b128be42dd5eca0dccdd4632b6f46e32c335a031 Mon Sep 17 00:00:00 2001 From: VincentBean Date: Wed, 28 Aug 2024 06:26:23 +0000 Subject: [PATCH 1/7] Update CHANGELOG --- CHANGELOG.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 115009a..7f817e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,14 @@ # Changelog -[Unreleased changes](https://github.com/justbetter/laravel-magento-async/compare/1.0.2...main) +[Unreleased changes](https://github.com/justbetter/laravel-magento-async/compare/1.0.3...main) +## [1.0.3](https://github.com/justbetter/laravel-magento-async/releases/tag/1.0.3) - 2024-08-28 + +### What's Changed +* Adjust bulk request cleanup by @VincentBean in https://github.com/justbetter/laravel-magento-async/pull/3 + + +**Full Changelog**: https://github.com/justbetter/laravel-magento-async/compare/1.0.2...1.0.3 + ## [1.0.2](https://github.com/justbetter/laravel-magento-async/releases/tag/1.0.2) - 2024-08-27 ### What's Changed From 7f2f283063ec239b74f7fdf115431a9b67f1e45a Mon Sep 17 00:00:00 2001 From: Vincent Boon Date: Wed, 18 Sep 2024 14:44:24 +0200 Subject: [PATCH 2/7] Implement retries --- ...000_magento_bulk_requests_method_field.php | 20 ++++ ...0_magento_bulk_requests_retry_of_field.php | 20 ++++ src/Actions/RetryBulkRequest.php | 67 +++++++++++ src/Client/MagentoAsync.php | 36 +++--- src/Commands/RetryBulkRequestCommand.php | 38 +++++++ src/Contracts/RetriesBulkRequest.php | 10 ++ src/Enums/OperationStatus.php | 9 ++ src/Models/BulkRequest.php | 17 +++ src/ServiceProvider.php | 4 + tests/Actions/CleanBulkRequestsTest.php | 2 + tests/Actions/RetryBulkRequestTest.php | 104 ++++++++++++++++++ tests/Actions/UpdateBulkStatusTest.php | 2 + tests/Actions/UpdateBulkStatusesTest.php | 3 + tests/Client/MagentoAsyncTest.php | 3 + .../Commands/RetryBulkRequestCommandTest.php | 77 +++++++++++++ .../Commands/UpdateBulkStatusCommandTest.php | 1 + tests/Jobs/UpdateBulkStatusJobTest.php | 1 + .../BulkOperationStatusListenerTest.php | 2 + tests/Models/BulkOperationTest.php | 1 + tests/Models/BulkRequestTest.php | 1 + 20 files changed, 401 insertions(+), 17 deletions(-) create mode 100644 database/migrations/2024_09_18_113000_magento_bulk_requests_method_field.php create mode 100644 database/migrations/2024_09_18_140000_magento_bulk_requests_retry_of_field.php create mode 100644 src/Actions/RetryBulkRequest.php create mode 100644 src/Commands/RetryBulkRequestCommand.php create mode 100644 src/Contracts/RetriesBulkRequest.php create mode 100644 tests/Actions/RetryBulkRequestTest.php create mode 100644 tests/Commands/RetryBulkRequestCommandTest.php diff --git a/database/migrations/2024_09_18_113000_magento_bulk_requests_method_field.php b/database/migrations/2024_09_18_113000_magento_bulk_requests_method_field.php new file mode 100644 index 0000000..b677230 --- /dev/null +++ b/database/migrations/2024_09_18_113000_magento_bulk_requests_method_field.php @@ -0,0 +1,20 @@ +string('method')->after('store_code'); + }); + } + + public function down(): void + { + Schema::dropColumns('magento_bulk_requests', ['method']); + } +}; diff --git a/database/migrations/2024_09_18_140000_magento_bulk_requests_retry_of_field.php b/database/migrations/2024_09_18_140000_magento_bulk_requests_retry_of_field.php new file mode 100644 index 0000000..458e6ef --- /dev/null +++ b/database/migrations/2024_09_18_140000_magento_bulk_requests_retry_of_field.php @@ -0,0 +1,20 @@ +unsignedBigInteger('retry_of')->after('id')->nullable(); + }); + } + + public function down(): void + { + Schema::dropColumns('magento_bulk_requests', ['retry_of']); + } +}; diff --git a/src/Actions/RetryBulkRequest.php b/src/Actions/RetryBulkRequest.php new file mode 100644 index 0000000..6bd47e1 --- /dev/null +++ b/src/Actions/RetryBulkRequest.php @@ -0,0 +1,67 @@ + $payload */ + $payload = []; + /** @var array $subjects */ + $subjects = []; + + $operations = $bulkRequest->operations; + + foreach ($bulkRequest->request as $index => $request) { + + /** @var BulkOperation $operation */ + $operation = $operations->where('operation_id', '=', $index)->firstOrFail(); + + if ($onlyFailed && ! in_array($operation->status, OperationStatus::failedStatuses()) || $operation->subject === null) { + continue; + } + + $payload[] = $request; + $subjects[] = $operation->subject; + } + + if ($payload === []) { + return null; + } + + $pendingRequest = $this->client + ->configure(fn (Magento $client): Magento => $client->store($bulkRequest->store_code)) + ->subjects($subjects); + + $retry = match ($bulkRequest->method) { + 'POST' => $pendingRequest->postBulk($bulkRequest->path, $payload), + 'PUT' => $pendingRequest->putBulk($bulkRequest->path, $payload), + 'DELETE' => $pendingRequest->deleteBulk($bulkRequest->path, $payload), + default => null, + }; + + if ($retry !== null) { + $retry->update([ + 'retry_of' => $bulkRequest->id, + ]); + } + + return $retry; + } + + public static function bind(): void + { + app()->singleton(RetriesBulkRequest::class, static::class); + } +} diff --git a/src/Client/MagentoAsync.php b/src/Client/MagentoAsync.php index d4ed710..30fe2bc 100644 --- a/src/Client/MagentoAsync.php +++ b/src/Client/MagentoAsync.php @@ -70,7 +70,7 @@ public function subject(Model $subject): static return $this; } - /** @param array $subjects */ + /** @param array $subjects */ public function subjects(array $subjects): static { $this->subjects = $subjects; @@ -78,60 +78,61 @@ public function subjects(array $subjects): static return $this; } - /** @param array $data */ + /** @param array $data */ public function post(string $path, array $data = []): BulkRequest { $response = $this->magento->postAsync($path, $data); - return $this->processResponse($response, $path, $data); + return $this->processResponse($response, 'POST', $path, $data); } - /** @param array $data */ + /** @param array $data */ public function postBulk(string $path, array $data = []): BulkRequest { $response = $this->magento->postBulk($path, $data); - return $this->processResponse($response, $path, $data, true); + return $this->processResponse($response, 'POST', $path, $data, true); } - /** @param array $data */ + /** @param array $data */ public function put(string $path, array $data = []): BulkRequest { $response = $this->magento->putAsync($path, $data); - return $this->processResponse($response, $path, $data); + return $this->processResponse($response, 'PUT', $path, $data); } - /** @param array $data */ + /** @param array $data */ public function putBulk(string $path, array $data = []): BulkRequest { $response = $this->magento->putBulk($path, $data); - return $this->processResponse($response, $path, $data, true); + return $this->processResponse($response, 'PUT', $path, $data, true); } - /** @param array $data */ + /** @param array $data */ public function delete(string $path, array $data = []): BulkRequest { $response = $this->magento->deleteAsync($path, $data); - return $this->processResponse($response, $path, $data); + return $this->processResponse($response, 'DELETE', $path, $data); } - /** @param array $data */ + /** @param array $data */ public function deleteBulk(string $path, array $data = []): BulkRequest { $response = $this->magento->deleteBulk($path, $data); - return $this->processResponse($response, $path, $data, true); + return $this->processResponse($response, 'DELETE', $path, $data, true); } - /** @param array $data */ + /** @param array $data */ public function processResponse( Response $response, + string $method, string $path, array $data = [], - bool $bulk = false + bool $bulk = false, ): BulkRequest { $response->throw(); @@ -141,16 +142,17 @@ public function processResponse( /** @var array> $requestItems */ $requestItems = $response->json('request_items', []); - $response = $response->json(null, []); + $responseData = $response->json(null, []); /** @var BulkRequest $bulkRequest */ $bulkRequest = BulkRequest::query()->create([ 'magento_connection' => $this->magento->connection, 'store_code' => $this->magento->storeCode ?? 'all', + 'method' => $method, 'path' => $path, 'bulk_uuid' => $bulkUuid, 'request' => $data, - 'response' => $response, + 'response' => $responseData, ]); foreach ($requestItems as $index => $requestItem) { diff --git a/src/Commands/RetryBulkRequestCommand.php b/src/Commands/RetryBulkRequestCommand.php new file mode 100644 index 0000000..56ee59c --- /dev/null +++ b/src/Commands/RetryBulkRequestCommand.php @@ -0,0 +1,38 @@ +argument('id'); + + /** @var bool $onlyFailed */ + $onlyFailed = $this->option('only-failed'); + + /** @var BulkRequest $request */ + $request = BulkRequest::query()->findOrFail($id); + + $bulkRequest = $contract->retry($request, $onlyFailed); + + if ($bulkRequest === null) { + $this->error('Failed to retry bulk request'); + + return static::FAILURE; + } + + $this->info('Retried with bulk uuid "'.$bulkRequest->bulk_uuid.'"'); + + return static::SUCCESS; + } +} diff --git a/src/Contracts/RetriesBulkRequest.php b/src/Contracts/RetriesBulkRequest.php new file mode 100644 index 0000000..290f375 --- /dev/null +++ b/src/Contracts/RetriesBulkRequest.php @@ -0,0 +1,10 @@ + */ + public static function failedStatuses(): array + { + return [ + OperationStatus::RetriablyFailed, + OperationStatus::NotRetriablyFailed, + ]; + } } diff --git a/src/Models/BulkRequest.php b/src/Models/BulkRequest.php index fd83b0c..a288796 100644 --- a/src/Models/BulkRequest.php +++ b/src/Models/BulkRequest.php @@ -4,13 +4,16 @@ use Illuminate\Database\Eloquent\Collection; use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\HasMany; use Illuminate\Support\Carbon; /** * @property int $id + * @property ?int $retry_of * @property string $magento_connection * @property string $store_code + * @property string $method * @property string $path * @property string $bulk_uuid * @property array $request @@ -19,6 +22,8 @@ * @property ?Carbon $created_at * @property ?Carbon $updated_at * @property Collection $operations + * @property Collection $retries + * @property ?BulkRequest $retryOf */ class BulkRequest extends Model { @@ -37,4 +42,16 @@ public function operations(): HasMany { return $this->hasMany(BulkOperation::class); } + + /** @return HasMany */ + public function retries(): HasMany + { + return $this->hasMany(BulkRequest::class, 'retry_of', 'id'); + } + + /** @return BelongsTo */ + public function retryOf(): BelongsTo + { + return $this->belongsTo(BulkRequest::class, 'retry_of', 'id'); + } } diff --git a/src/ServiceProvider.php b/src/ServiceProvider.php index ab5a01e..96668e2 100644 --- a/src/ServiceProvider.php +++ b/src/ServiceProvider.php @@ -4,9 +4,11 @@ use Illuminate\Support\ServiceProvider as BaseServiceProvider; use JustBetter\MagentoAsync\Actions\CleanBulkRequests; +use JustBetter\MagentoAsync\Actions\RetryBulkRequest; use JustBetter\MagentoAsync\Actions\UpdateBulkStatus; use JustBetter\MagentoAsync\Actions\UpdateBulkStatuses; use JustBetter\MagentoAsync\Commands\CleanBulkRequestsCommand; +use JustBetter\MagentoAsync\Commands\RetryBulkRequestCommand; use JustBetter\MagentoAsync\Commands\UpdateBulkStatusCommand; use JustBetter\MagentoAsync\Commands\UpdateBulkStatusesCommand; @@ -31,6 +33,7 @@ protected function registerActions(): static UpdateBulkStatus::bind(); UpdateBulkStatuses::bind(); CleanBulkRequests::bind(); + RetryBulkRequest::bind(); return $this; } @@ -66,6 +69,7 @@ protected function bootCommands(): static UpdateBulkStatusCommand::class, UpdateBulkStatusesCommand::class, CleanBulkRequestsCommand::class, + RetryBulkRequestCommand::class, ]); } diff --git a/tests/Actions/CleanBulkRequestsTest.php b/tests/Actions/CleanBulkRequestsTest.php index 8745ac4..b64d022 100644 --- a/tests/Actions/CleanBulkRequestsTest.php +++ b/tests/Actions/CleanBulkRequestsTest.php @@ -19,6 +19,7 @@ public function it_deletes_completed_operations(): void $request = BulkRequest::query()->create([ 'magento_connection' => '::magento-connection::', 'store_code' => '::store-code::', + 'method' => 'POST', 'path' => '::path::', 'bulk_uuid' => '::bulk-uuid-1::', 'request' => [], @@ -60,6 +61,7 @@ public function it_deletes_request(Carbon $requestCreatedAt, array $operations, $request = BulkRequest::query()->create([ 'magento_connection' => '::magento-connection::', 'store_code' => '::store-code::', + 'method' => 'POST', 'path' => '::path::', 'bulk_uuid' => '::bulk-uuid-1::', 'request' => [], diff --git a/tests/Actions/RetryBulkRequestTest.php b/tests/Actions/RetryBulkRequestTest.php new file mode 100644 index 0000000..75680c2 --- /dev/null +++ b/tests/Actions/RetryBulkRequestTest.php @@ -0,0 +1,104 @@ + Http::response([ + 'bulk_uuid' => 'new-uuid', + ]), + ])->preventStrayRequests(); + + foreach (['POST', 'PUT', 'DELETE'] as $method) { + /** @var BulkRequest $request */ + $request = BulkRequest::query()->create([ + 'magento_connection' => '::magento-connection::', + 'store_code' => '::store-code::', + 'method' => $method, + 'path' => '::path::', + 'bulk_uuid' => '::bulk-uuid-1::', + 'request' => [ + [ + 'call-1', + ], + [ + 'call-2', + ], + [ + 'call-3', + ], + ], + 'response' => [], + 'created_at' => now(), + ]); + + $request->operations()->create([ + 'operation_id' => 0, + 'status' => OperationStatus::Complete, + 'updated_at' => now()->subHours(2), + ]); + + $request->operations()->create([ + 'operation_id' => 1, + 'status' => OperationStatus::RetriablyFailed, + 'subject_id' => $request->id, + 'subject_type' => get_class($request), + ]); + + $request->operations()->create([ + 'operation_id' => 2, + 'status' => OperationStatus::Open, + ]); + + /** @var RetryBulkRequest $action */ + $action = app(RetryBulkRequest::class); + + $retry = $action->retry($request, true); + + $this->assertNotNull($retry); + $this->assertEquals('::path::', $retry->path); + $this->assertEquals($method, $retry->method); + $this->assertEquals([['call-2']], $retry->request); + $this->assertEquals($request->id, $retry->retry_of); + } + } + + #[Test] + public function it_does_nothing_without_payload(): void + { + Http::fake()->preventStrayRequests(); + + /** @var BulkRequest $request */ + $request = BulkRequest::query()->create([ + 'magento_connection' => '::magento-connection::', + 'store_code' => '::store-code::', + 'method' => 'POST', + 'path' => '::path::', + 'bulk_uuid' => '::bulk-uuid-1::', + 'request' => [], + 'response' => [], + 'created_at' => now(), + ]); + + /** @var RetryBulkRequest $action */ + $action = app(RetryBulkRequest::class); + + $action->retry($request, false); + + Http::assertNothingSent(); + } +} diff --git a/tests/Actions/UpdateBulkStatusTest.php b/tests/Actions/UpdateBulkStatusTest.php index e00ff30..b264cd7 100644 --- a/tests/Actions/UpdateBulkStatusTest.php +++ b/tests/Actions/UpdateBulkStatusTest.php @@ -42,6 +42,7 @@ public function it_can_update_bulk_statuses(): void $bulkRequest = BulkRequest::query()->create([ 'magento_connection' => 'default', 'store_code' => 'all', + 'method' => 'POST', 'path' => 'products', 'bulk_uuid' => '4c51f869-0a77-4238-be2a-290dd5081666', 'request' => [], @@ -96,6 +97,7 @@ public function it_can_skip_the_started_at(): void $bulkRequest = BulkRequest::query()->create([ 'magento_connection' => 'default', 'store_code' => 'all', + 'method' => 'POST', 'path' => 'products', 'bulk_uuid' => '4c51f869-0a77-4238-be2a-290dd5081666', 'request' => [], diff --git a/tests/Actions/UpdateBulkStatusesTest.php b/tests/Actions/UpdateBulkStatusesTest.php index 933f9ed..4631aba 100644 --- a/tests/Actions/UpdateBulkStatusesTest.php +++ b/tests/Actions/UpdateBulkStatusesTest.php @@ -21,6 +21,7 @@ public function it_can_update_bulk_statuses(): void $status1 = BulkRequest::query()->create([ 'magento_connection' => '::magento-connection::', 'store_code' => '::store-code::', + 'method' => 'POST', 'path' => '::path::', 'bulk_uuid' => '::bulk-uuid-1::', 'request' => [], @@ -31,6 +32,7 @@ public function it_can_update_bulk_statuses(): void $status2 = BulkRequest::query()->create([ 'magento_connection' => '::magento-connection::', 'store_code' => '::store-code::', + 'method' => 'POST', 'path' => '::path::', 'bulk_uuid' => '::bulk-uuid-2::', 'request' => [], @@ -41,6 +43,7 @@ public function it_can_update_bulk_statuses(): void $status3 = BulkRequest::query()->create([ 'magento_connection' => '::magento-connection::', 'store_code' => '::store-code::', + 'method' => 'POST', 'path' => '::path::', 'bulk_uuid' => '::bulk-uuid-3::', 'request' => [], diff --git a/tests/Client/MagentoAsyncTest.php b/tests/Client/MagentoAsyncTest.php index a5789c8..9570856 100644 --- a/tests/Client/MagentoAsyncTest.php +++ b/tests/Client/MagentoAsyncTest.php @@ -37,6 +37,7 @@ public function it_can_process_responses(): void $subject = BulkRequest::query()->create([ 'magento_connection' => '::magento-connection::', 'store_code' => '::store-code::', + 'method' => 'POST', 'path' => '::path::', 'bulk_uuid' => '::bulk-uuid::', 'request' => [], @@ -106,6 +107,7 @@ public function it_can_process_bulk_responses(): void BulkRequest::query()->create([ 'magento_connection' => '::magento-connection::', 'store_code' => '::store-code::', + 'method' => 'POST', 'path' => '::path::', 'bulk_uuid' => '::bulk-uuid-1::', 'request' => [], @@ -114,6 +116,7 @@ public function it_can_process_bulk_responses(): void BulkRequest::query()->create([ 'magento_connection' => '::magento-connection::', 'store_code' => '::store-code::', + 'method' => 'POST', 'path' => '::path::', 'bulk_uuid' => '::bulk-uuid-2::', 'request' => [], diff --git a/tests/Commands/RetryBulkRequestCommandTest.php b/tests/Commands/RetryBulkRequestCommandTest.php new file mode 100644 index 0000000..54c35c0 --- /dev/null +++ b/tests/Commands/RetryBulkRequestCommandTest.php @@ -0,0 +1,77 @@ +create([ + 'magento_connection' => '::magento-connection::', + 'store_code' => '::store-code::', + 'method' => 'POST', + 'path' => '::path::', + 'bulk_uuid' => '::bulk-uuid-1::', + 'request' => [], + 'response' => [], + 'created_at' => now(), + ]); + + $this->mock(RetriesBulkRequest::class, function (MockInterface $mock) use ($request): void { + $mock->shouldReceive('retry') + ->withArgs(function (BulkRequest $bulkRequest, bool $onlyFailed) use ($request) { + return $bulkRequest->id === $request->id && $onlyFailed; + })->once() + ->andReturn($request); + }); + + $result = $this->artisan(RetryBulkRequestCommand::class, [ + 'id' => $request->id, + '--only-failed' => true, + ]); + + $this->assertFalse(is_int($result)); + $result->assertSuccessful(); + } + + #[Test] + public function it_fails_without_retry(): void + { + /* @var BulkRequest $request */ + $request = BulkRequest::query()->create([ + 'magento_connection' => '::magento-connection::', + 'store_code' => '::store-code::', + 'method' => 'POST', + 'path' => '::path::', + 'bulk_uuid' => '::bulk-uuid-1::', + 'request' => [], + 'response' => [], + 'created_at' => now(), + ]); + + $this->mock(RetriesBulkRequest::class, function (MockInterface $mock) use ($request): void { + $mock->shouldReceive('retry') + ->withArgs(function (BulkRequest $bulkRequest, bool $onlyFailed) use ($request) { + return $bulkRequest->id === $request->id && $onlyFailed; + })->once() + ->andReturnNull(); + }); + + $result = $this->artisan(RetryBulkRequestCommand::class, [ + 'id' => $request->id, + '--only-failed' => true, + ]); + + $this->assertFalse(is_int($result)); + $result->assertFailed(); + } +} diff --git a/tests/Commands/UpdateBulkStatusCommandTest.php b/tests/Commands/UpdateBulkStatusCommandTest.php index 88e43d1..7c58dce 100644 --- a/tests/Commands/UpdateBulkStatusCommandTest.php +++ b/tests/Commands/UpdateBulkStatusCommandTest.php @@ -21,6 +21,7 @@ public function it_can_dispatch_jobs(): void $bulkRequest = BulkRequest::query()->create([ 'magento_connection' => '::magento-connection::', 'store_code' => '::store-code::', + 'method' => 'POST', 'path' => '::path::', 'bulk_uuid' => '::bulk-uuid::', 'request' => [], diff --git a/tests/Jobs/UpdateBulkStatusJobTest.php b/tests/Jobs/UpdateBulkStatusJobTest.php index 9c7a4ce..6ffe02c 100644 --- a/tests/Jobs/UpdateBulkStatusJobTest.php +++ b/tests/Jobs/UpdateBulkStatusJobTest.php @@ -25,6 +25,7 @@ public function it_can_update_bulk_statuses(): void $bulkRequest = BulkRequest::query()->create([ 'magento_connection' => '::magento-connection::', 'store_code' => '::store-code::', + 'method' => 'POST', 'path' => '::path::', 'bulk_uuid' => '::bulk-uuid::', 'request' => [], diff --git a/tests/Listeners/BulkOperationStatusListenerTest.php b/tests/Listeners/BulkOperationStatusListenerTest.php index 581f72b..f50ac52 100644 --- a/tests/Listeners/BulkOperationStatusListenerTest.php +++ b/tests/Listeners/BulkOperationStatusListenerTest.php @@ -19,6 +19,7 @@ public function it_can_handle_a_status_change(): void $request = BulkRequest::query()->create([ 'magento_connection' => '::magento-connection::', 'store_code' => '::store-code::', + 'method' => 'POST', 'path' => '::path::', 'bulk_uuid' => '::bulk-uuid::', 'request' => [], @@ -52,6 +53,7 @@ public function it_can_skip_executing(): void $request = BulkRequest::query()->create([ 'magento_connection' => '::magento-connection::', 'store_code' => '::store-code::', + 'method' => 'POST', 'path' => '::path::', 'bulk_uuid' => '::bulk-uuid::', 'request' => [], diff --git a/tests/Models/BulkOperationTest.php b/tests/Models/BulkOperationTest.php index 0dce7a7..b6ae9a8 100644 --- a/tests/Models/BulkOperationTest.php +++ b/tests/Models/BulkOperationTest.php @@ -16,6 +16,7 @@ public function it_is_linked_to_a_request_and_can_have_a_subject(): void $request = BulkRequest::query()->create([ 'magento_connection' => '::magento-connection::', 'store_code' => '::store-code::', + 'method' => 'POST', 'path' => '::path::', 'bulk_uuid' => '::bulk-uuid::', 'request' => [], diff --git a/tests/Models/BulkRequestTest.php b/tests/Models/BulkRequestTest.php index 84e8fb5..2828a6d 100644 --- a/tests/Models/BulkRequestTest.php +++ b/tests/Models/BulkRequestTest.php @@ -15,6 +15,7 @@ public function it_can_have_operations(): void $request = BulkRequest::query()->create([ 'magento_connection' => '::magento-connection::', 'store_code' => '::store-code::', + 'method' => 'POST', 'path' => '::path::', 'bulk_uuid' => '::bulk-uuid::', 'request' => [], From 970165024cf6cd6518164af1a4a2d0959ca2e62e Mon Sep 17 00:00:00 2001 From: Vincent Boon Date: Wed, 18 Sep 2024 15:29:46 +0200 Subject: [PATCH 3/7] Adjust coverage action --- .github/workflows/coverage.yml | 2 +- composer.json | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 0d9d3b0..0532e49 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -32,7 +32,7 @@ jobs: - name: Install dependencies run: | composer config allow-plugins.pestphp/pest-plugin true - composer require "laravel/framework:${{ matrix.laravel }}" "orchestra/testbench:${{ matrix.testbench }}" pestphp/pest --no-interaction --no-update + composer require "laravel/framework:${{ matrix.laravel }}" "orchestra/testbench:${{ matrix.testbench }}" --no-interaction --no-update composer update --${{ matrix.stability }} --prefer-dist --no-interaction - name: Execute tests run: XDEBUG_MODE=coverage php vendor/bin/pest --coverage --min=100 diff --git a/composer.json b/composer.json index 07feefd..ec9b6fd 100644 --- a/composer.json +++ b/composer.json @@ -11,11 +11,12 @@ "justbetter/laravel-magento-webhooks": "^2.1" }, "require-dev": { - "laravel/pint": "^1.16", "larastan/larastan": "^2.5", + "laravel/pint": "^1.16", + "orchestra/testbench": "^9.0", + "pestphp/pest": "^2.0", "phpstan/phpstan-mockery": "^1.1", - "phpunit/phpunit": "^10.0", - "orchestra/testbench": "^9.0" + "phpunit/phpunit": "^10.0" }, "authors": [ { @@ -51,7 +52,10 @@ "fix-style": "pint" }, "config": { - "sort-packages": true + "sort-packages": true, + "allow-plugins": { + "pestphp/pest-plugin": true + } }, "extra": { "laravel": { From 752159277476ca12b3117755c0c7e5d90256a8f8 Mon Sep 17 00:00:00 2001 From: Vincent Boon Date: Wed, 18 Sep 2024 15:36:23 +0200 Subject: [PATCH 4/7] Fix test --- src/Actions/RetryBulkRequest.php | 12 +++++++--- tests/Actions/RetryBulkRequestTest.php | 31 ++++++++++++++++++++++++++ tests/Models/BulkRequestTest.php | 30 +++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 3 deletions(-) diff --git a/src/Actions/RetryBulkRequest.php b/src/Actions/RetryBulkRequest.php index 6bd47e1..6e209d8 100644 --- a/src/Actions/RetryBulkRequest.php +++ b/src/Actions/RetryBulkRequest.php @@ -12,7 +12,9 @@ class RetryBulkRequest implements RetriesBulkRequest { - public function __construct(protected MagentoAsync $client) {} + public function __construct(protected MagentoAsync $client) + { + } public function retry(BulkRequest $bulkRequest, bool $onlyFailed): ?BulkRequest { @@ -28,12 +30,16 @@ public function retry(BulkRequest $bulkRequest, bool $onlyFailed): ?BulkRequest /** @var BulkOperation $operation */ $operation = $operations->where('operation_id', '=', $index)->firstOrFail(); - if ($onlyFailed && ! in_array($operation->status, OperationStatus::failedStatuses()) || $operation->subject === null) { + if ($onlyFailed && ! in_array($operation->status, OperationStatus::failedStatuses())) { continue; } $payload[] = $request; - $subjects[] = $operation->subject; + if ($operation->subject !== null) { + $subjects[] = $operation->subject; + } else { + $subjects[] = null; + } } if ($payload === []) { diff --git a/tests/Actions/RetryBulkRequestTest.php b/tests/Actions/RetryBulkRequestTest.php index 75680c2..9981b22 100644 --- a/tests/Actions/RetryBulkRequestTest.php +++ b/tests/Actions/RetryBulkRequestTest.php @@ -101,4 +101,35 @@ public function it_does_nothing_without_payload(): void Http::assertNothingSent(); } + + #[Test] + public function it_does_nothing_without_method(): void + { + Http::fake()->preventStrayRequests(); + + /** @var BulkRequest $request */ + $request = BulkRequest::query()->create([ + 'magento_connection' => '::magento-connection::', + 'store_code' => '::store-code::', + 'method' => '', + 'path' => '::path::', + 'bulk_uuid' => '::bulk-uuid-1::', + 'request' => [['call-1']], + 'response' => [], + 'created_at' => now(), + ]); + + $request->operations()->create([ + 'operation_id' => 0, + 'status' => OperationStatus::Complete, + 'updated_at' => now()->subHours(2), + ]); + + /** @var RetryBulkRequest $action */ + $action = app(RetryBulkRequest::class); + + $action->retry($request, false); + + Http::assertNothingSent(); + } } diff --git a/tests/Models/BulkRequestTest.php b/tests/Models/BulkRequestTest.php index 2828a6d..a8fdbad 100644 --- a/tests/Models/BulkRequestTest.php +++ b/tests/Models/BulkRequestTest.php @@ -36,4 +36,34 @@ public function it_can_have_operations(): void $this->assertCount(3, $request->operations); } + + #[Test] + public function it_has_retry_relationship(): void + { + /** @var BulkRequest $request */ + $request = BulkRequest::query()->create([ + 'magento_connection' => '::magento-connection::', + 'store_code' => '::store-code::', + 'method' => 'POST', + 'path' => '::path::', + 'bulk_uuid' => '::bulk-uuid::', + 'request' => [], + 'response' => [], + ]); + + /** @var BulkRequest $retry */ + $retry = BulkRequest::query()->create([ + 'retry_of' => $request->id, + 'magento_connection' => '::magento-connection::', + 'store_code' => '::store-code::', + 'method' => 'POST', + 'path' => '::path::', + 'bulk_uuid' => '::bulk-uuid::', + 'request' => [], + 'response' => [], + ]); + + $this->assertEquals($retry->id, $request->retries->first()?->id); + $this->assertEquals($request->id, $retry->retryOf?->id); + } } From 5e2daf3054afaff88b3aaac97f21cf13f9643ca1 Mon Sep 17 00:00:00 2001 From: Vincent Boon Date: Wed, 18 Sep 2024 15:38:17 +0200 Subject: [PATCH 5/7] SCA --- src/Actions/RetryBulkRequest.php | 2 +- src/Client/MagentoAsync.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Actions/RetryBulkRequest.php b/src/Actions/RetryBulkRequest.php index 6e209d8..a7f7df7 100644 --- a/src/Actions/RetryBulkRequest.php +++ b/src/Actions/RetryBulkRequest.php @@ -20,7 +20,7 @@ public function retry(BulkRequest $bulkRequest, bool $onlyFailed): ?BulkRequest { /** @var array $payload */ $payload = []; - /** @var array $subjects */ + /** @var array $subjects */ $subjects = []; $operations = $bulkRequest->operations; diff --git a/src/Client/MagentoAsync.php b/src/Client/MagentoAsync.php index 30fe2bc..7e34cde 100644 --- a/src/Client/MagentoAsync.php +++ b/src/Client/MagentoAsync.php @@ -21,7 +21,7 @@ class MagentoAsync protected ?Model $subject = null; - /** @var array */ + /** @var array */ protected array $subjects = []; public function __construct( @@ -70,7 +70,7 @@ public function subject(Model $subject): static return $this; } - /** @param array $subjects */ + /** @param array $subjects */ public function subjects(array $subjects): static { $this->subjects = $subjects; From 461e43694fc6045957733b1a442a40464ace9ed9 Mon Sep 17 00:00:00 2001 From: Vincent Boon Date: Thu, 19 Sep 2024 11:00:10 +0200 Subject: [PATCH 6/7] Feedback --- ...0_magento_bulk_requests_retry_of_field.php | 2 + src/Actions/RetryBulkRequest.php | 17 +-- src/Commands/RetryBulkRequestCommand.php | 2 +- src/Exceptions/InvalidMethodException.php | 7 + tests/Actions/RetryBulkRequestTest.php | 123 ++++++++++-------- .../Commands/RetryBulkRequestCommandTest.php | 5 +- tests/Models/BulkRequestTest.php | 2 +- 7 files changed, 86 insertions(+), 72 deletions(-) create mode 100644 src/Exceptions/InvalidMethodException.php diff --git a/database/migrations/2024_09_18_140000_magento_bulk_requests_retry_of_field.php b/database/migrations/2024_09_18_140000_magento_bulk_requests_retry_of_field.php index 458e6ef..5e5fef0 100644 --- a/database/migrations/2024_09_18_140000_magento_bulk_requests_retry_of_field.php +++ b/database/migrations/2024_09_18_140000_magento_bulk_requests_retry_of_field.php @@ -10,6 +10,8 @@ public function up(): void { Schema::table('magento_bulk_requests', function (Blueprint $table): void { $table->unsignedBigInteger('retry_of')->after('id')->nullable(); + + $table->foreign('retry_of')->references('id')->on('magento_bulk_requests')->onDelete('set null'); }); } diff --git a/src/Actions/RetryBulkRequest.php b/src/Actions/RetryBulkRequest.php index a7f7df7..119e153 100644 --- a/src/Actions/RetryBulkRequest.php +++ b/src/Actions/RetryBulkRequest.php @@ -6,15 +6,14 @@ use JustBetter\MagentoAsync\Client\MagentoAsync; use JustBetter\MagentoAsync\Contracts\RetriesBulkRequest; use JustBetter\MagentoAsync\Enums\OperationStatus; +use JustBetter\MagentoAsync\Exceptions\InvalidMethodException; use JustBetter\MagentoAsync\Models\BulkOperation; use JustBetter\MagentoAsync\Models\BulkRequest; use JustBetter\MagentoClient\Client\Magento; class RetryBulkRequest implements RetriesBulkRequest { - public function __construct(protected MagentoAsync $client) - { - } + public function __construct(protected MagentoAsync $client) {} public function retry(BulkRequest $bulkRequest, bool $onlyFailed): ?BulkRequest { @@ -35,11 +34,7 @@ public function retry(BulkRequest $bulkRequest, bool $onlyFailed): ?BulkRequest } $payload[] = $request; - if ($operation->subject !== null) { - $subjects[] = $operation->subject; - } else { - $subjects[] = null; - } + $subjects[] = $operation->subject; } if ($payload === []) { @@ -54,13 +49,11 @@ public function retry(BulkRequest $bulkRequest, bool $onlyFailed): ?BulkRequest 'POST' => $pendingRequest->postBulk($bulkRequest->path, $payload), 'PUT' => $pendingRequest->putBulk($bulkRequest->path, $payload), 'DELETE' => $pendingRequest->deleteBulk($bulkRequest->path, $payload), - default => null, + default => throw new InvalidMethodException('Unsupported method "'.$bulkRequest->method.'"'), }; if ($retry !== null) { - $retry->update([ - 'retry_of' => $bulkRequest->id, - ]); + $bulkRequest->retries()->save($retry); } return $retry; diff --git a/src/Commands/RetryBulkRequestCommand.php b/src/Commands/RetryBulkRequestCommand.php index 56ee59c..a5197bb 100644 --- a/src/Commands/RetryBulkRequestCommand.php +++ b/src/Commands/RetryBulkRequestCommand.php @@ -8,7 +8,7 @@ class RetryBulkRequestCommand extends Command { - protected $signature = 'magento:async:retry-bulk-request {id} {--only-failed}'; + protected $signature = 'magento:async:retry-bulk-request {id} {--only-failed=true}'; protected $description = 'Retry bulk request'; diff --git a/src/Exceptions/InvalidMethodException.php b/src/Exceptions/InvalidMethodException.php new file mode 100644 index 0000000..fb4725c --- /dev/null +++ b/src/Exceptions/InvalidMethodException.php @@ -0,0 +1,7 @@ +preventStrayRequests(); - foreach (['POST', 'PUT', 'DELETE'] as $method) { - /** @var BulkRequest $request */ - $request = BulkRequest::query()->create([ - 'magento_connection' => '::magento-connection::', - 'store_code' => '::store-code::', - 'method' => $method, - 'path' => '::path::', - 'bulk_uuid' => '::bulk-uuid-1::', - 'request' => [ - [ - 'call-1', - ], - [ - 'call-2', - ], - [ - 'call-3', - ], + /** @var BulkRequest $request */ + $request = BulkRequest::query()->create([ + 'magento_connection' => '::magento-connection::', + 'store_code' => '::store-code::', + 'method' => $method, + 'path' => '::path::', + 'bulk_uuid' => '::bulk-uuid-1::', + 'request' => [ + [ + 'call-1', + ], + [ + 'call-2', ], - 'response' => [], - 'created_at' => now(), - ]); - - $request->operations()->create([ - 'operation_id' => 0, - 'status' => OperationStatus::Complete, - 'updated_at' => now()->subHours(2), - ]); - - $request->operations()->create([ - 'operation_id' => 1, - 'status' => OperationStatus::RetriablyFailed, - 'subject_id' => $request->id, - 'subject_type' => get_class($request), - ]); - - $request->operations()->create([ - 'operation_id' => 2, - 'status' => OperationStatus::Open, - ]); - - /** @var RetryBulkRequest $action */ - $action = app(RetryBulkRequest::class); - - $retry = $action->retry($request, true); - - $this->assertNotNull($retry); - $this->assertEquals('::path::', $retry->path); - $this->assertEquals($method, $retry->method); - $this->assertEquals([['call-2']], $retry->request); - $this->assertEquals($request->id, $retry->retry_of); - } + [ + 'call-3', + ], + ], + 'response' => [], + 'created_at' => now(), + ]); + + $request->operations()->create([ + 'operation_id' => 0, + 'status' => OperationStatus::Complete, + 'updated_at' => now()->subHours(2), + ]); + + $request->operations()->create([ + 'operation_id' => 1, + 'status' => OperationStatus::RetriablyFailed, + 'subject_id' => $request->id, + 'subject_type' => get_class($request), + ]); + + $request->operations()->create([ + 'operation_id' => 2, + 'status' => OperationStatus::Open, + ]); + + /** @var RetryBulkRequest $action */ + $action = app(RetryBulkRequest::class); + + $retry = $action->retry($request, true); + + $this->assertNotNull($retry); + $this->assertEquals('::path::', $retry->path); + $this->assertEquals($method, $retry->method); + $this->assertEquals([['call-2']], $retry->request); + $this->assertEquals($request->id, $retry->retry_of); + } + + /** @return array> */ + public static function httpMethodProvider(): array + { + return [ + ['method' => 'POST'], + ['method' => 'PUT'], + ['method' => 'DELETE'], + ]; } #[Test] @@ -97,13 +108,14 @@ public function it_does_nothing_without_payload(): void /** @var RetryBulkRequest $action */ $action = app(RetryBulkRequest::class); - $action->retry($request, false); + $result = $action->retry($request, false); + $this->assertNull($result); Http::assertNothingSent(); } #[Test] - public function it_does_nothing_without_method(): void + public function it_throws_exception_without_method(): void { Http::fake()->preventStrayRequests(); @@ -128,8 +140,7 @@ public function it_does_nothing_without_method(): void /** @var RetryBulkRequest $action */ $action = app(RetryBulkRequest::class); + $this->expectException(InvalidMethodException::class); $action->retry($request, false); - - Http::assertNothingSent(); } } diff --git a/tests/Commands/RetryBulkRequestCommandTest.php b/tests/Commands/RetryBulkRequestCommandTest.php index 54c35c0..fc690bd 100644 --- a/tests/Commands/RetryBulkRequestCommandTest.php +++ b/tests/Commands/RetryBulkRequestCommandTest.php @@ -2,6 +2,7 @@ namespace JustBetter\MagentoAsync\Tests\Commands; +use Illuminate\Testing\PendingCommand; use JustBetter\MagentoAsync\Commands\RetryBulkRequestCommand; use JustBetter\MagentoAsync\Contracts\RetriesBulkRequest; use JustBetter\MagentoAsync\Models\BulkRequest; @@ -34,12 +35,12 @@ public function it_calls_action(): void ->andReturn($request); }); + /** @var PendingCommand $result */ $result = $this->artisan(RetryBulkRequestCommand::class, [ 'id' => $request->id, '--only-failed' => true, ]); - $this->assertFalse(is_int($result)); $result->assertSuccessful(); } @@ -66,12 +67,12 @@ public function it_fails_without_retry(): void ->andReturnNull(); }); + /** @var PendingCommand $result */ $result = $this->artisan(RetryBulkRequestCommand::class, [ 'id' => $request->id, '--only-failed' => true, ]); - $this->assertFalse(is_int($result)); $result->assertFailed(); } } diff --git a/tests/Models/BulkRequestTest.php b/tests/Models/BulkRequestTest.php index a8fdbad..a4325a5 100644 --- a/tests/Models/BulkRequestTest.php +++ b/tests/Models/BulkRequestTest.php @@ -51,7 +51,7 @@ public function it_has_retry_relationship(): void 'response' => [], ]); - /** @var BulkRequest $retry */ + /** @var BulkRequest $retry */ $retry = BulkRequest::query()->create([ 'retry_of' => $request->id, 'magento_connection' => '::magento-connection::', From 62c1f60692e8ef8601615bb2d34a7864df01d3d0 Mon Sep 17 00:00:00 2001 From: Vincent Boon Date: Thu, 19 Sep 2024 12:30:48 +0200 Subject: [PATCH 7/7] Restore default --- src/Commands/RetryBulkRequestCommand.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Commands/RetryBulkRequestCommand.php b/src/Commands/RetryBulkRequestCommand.php index a5197bb..56ee59c 100644 --- a/src/Commands/RetryBulkRequestCommand.php +++ b/src/Commands/RetryBulkRequestCommand.php @@ -8,7 +8,7 @@ class RetryBulkRequestCommand extends Command { - protected $signature = 'magento:async:retry-bulk-request {id} {--only-failed=true}'; + protected $signature = 'magento:async:retry-bulk-request {id} {--only-failed}'; protected $description = 'Retry bulk request';