From cb49bd7b182469f9302778746de29166104bf3d1 Mon Sep 17 00:00:00 2001 From: Norman Huth Date: Sat, 7 Feb 2026 07:08:08 +0100 Subject: [PATCH] feat(job): add `take` parameter to `FetchRecipePageJob` for enhanced control over API pagination - Extend the `HelloFreshClient::getRecipes` method with a `take` parameter to allow dynamic batch sizes - Update `FetchRecipePageJob` to support custom `take` values or default to the `Country` model's `take` attribute - Increase `FetchRecipePageJob::tries` from 3 to 5 for improved retry handling - Introduce retry logic for server errors by progressively reducing the `take` value - Add new test cases to validate `take` functionality, error handling, and retry behavior --- .../Clients/HelloFresh/HelloFreshClient.php | 8 +- app/Jobs/Recipe/FetchRecipePageJob.php | 56 +++++++--- tests/Unit/Jobs/FetchRecipePageJobTest.php | 100 ++++++++++++++++-- 3 files changed, 138 insertions(+), 26 deletions(-) diff --git a/app/Http/Clients/HelloFresh/HelloFreshClient.php b/app/Http/Clients/HelloFresh/HelloFreshClient.php index e26ce6a0..a633ef51 100644 --- a/app/Http/Clients/HelloFresh/HelloFreshClient.php +++ b/app/Http/Clients/HelloFresh/HelloFreshClient.php @@ -166,17 +166,21 @@ protected function request(string $url): Response * @throws ConnectionException * @throws RequestException */ - public function getRecipes(Country $country, string $locale, int $skip = 0): RecipesResponse + public function getRecipes(Country $country, string $locale, int $skip = 0, ?int $take = null): RecipesResponse { $countryCode = Str::upper($country->code); + if ($take === null) { + $take = $country->take; + } + $url = sprintf( '%s/gw/api/recipes?country=%s&locale=%s-%s&take=%d&skip=%d', $country->domain, $countryCode, Str::lower($locale), $countryCode, - $country->take, + $take, $skip, ); diff --git a/app/Jobs/Recipe/FetchRecipePageJob.php b/app/Jobs/Recipe/FetchRecipePageJob.php index e3475c47..193b0fa4 100644 --- a/app/Jobs/Recipe/FetchRecipePageJob.php +++ b/app/Jobs/Recipe/FetchRecipePageJob.php @@ -5,7 +5,6 @@ use App\Contracts\LauncherJobInterface; use App\Enums\QueueEnum; use App\Http\Clients\HelloFresh\HelloFreshClient; -use App\Jobs\Concerns\HandlesApiFailuresTrait; use App\Models\Country; use Illuminate\Bus\Batchable; use Illuminate\Contracts\Queue\ShouldQueue; @@ -13,20 +12,20 @@ use Illuminate\Http\Client\ConnectionException; use Illuminate\Http\Client\RequestException; use Illuminate\Support\Facades\Context; +use Illuminate\Support\Facades\Log; /** - * @method static void dispatch(Country $country, string $locale, int $skip = 0, bool $paginates = true) + * @method static void dispatch(Country $country, string $locale, int $skip = 0, bool $paginates = true, ?int $take = null) */ class FetchRecipePageJob implements LauncherJobInterface, ShouldQueue { use Batchable; - use HandlesApiFailuresTrait; use Queueable; /** * The number of times the job may be attempted. */ - public int $tries = 3; + public int $tries = 5; /** * Create a new job instance. @@ -36,8 +35,13 @@ public function __construct( public string $locale, public int $skip = 0, public bool $paginates = true, + public ?int $take = null ) { $this->onQueue(QueueEnum::HelloFresh->value); + + if ($take === null) { + $this->take = $country->take; + } } /** @@ -67,18 +71,8 @@ public function handle(HelloFreshClient $client): void ]); try { - $response = $client->withOutThrow() - ->getRecipes($this->country, $this->locale, $this->skip); - } catch (ConnectionException $connectionException) { - $this->handleApiFailure($connectionException); - - return; - } - - if ($response->failed()) { - $exception = $response->toException(); - assert($exception !== null); - + $response = $client->getRecipes($this->country, $this->locale, $this->skip, $this->take); + } catch (ConnectionException|RequestException $exception) { $this->handleApiFailure($exception); return; @@ -91,7 +85,35 @@ public function handle(HelloFreshClient $client): void // Only add next page fetch to batch if pagination is enabled if ($this->paginates && $response->hasMorePages()) { - $this->batch()?->add([new self($this->country, $this->locale, $response->nextSkip(), paginates: true)]); + $this->batch()?->add([new self($this->country, $this->locale, $response->nextSkip(), paginates: true, take: $this->take)]); + } + } + + /** + * Handle a failed API request with logging and retry logic. + * + * @throws ConnectionException + * @throws RequestException + */ + protected function handleApiFailure(ConnectionException|RequestException $exception): void + { + $isLastAttempt = $this->attempts() >= $this->tries; + + if ($isLastAttempt) { + throw $exception; + } + + Log::warning(static::class . ' failed, retrying', [ + 'attempt' => $this->attempts(), + 'exception' => $exception->getMessage(), + ]); + + if ($this->take > 50 && $exception instanceof RequestException && $exception->response->serverError()) { + self::dispatch($this->country, $this->locale, $this->skip, $this->paginates, $this->take - 50); + + return; } + + $this->release(30); } } diff --git a/tests/Unit/Jobs/FetchRecipePageJobTest.php b/tests/Unit/Jobs/FetchRecipePageJobTest.php index 8066412d..bb486c62 100644 --- a/tests/Unit/Jobs/FetchRecipePageJobTest.php +++ b/tests/Unit/Jobs/FetchRecipePageJobTest.php @@ -11,6 +11,9 @@ use App\Models\Country; use GuzzleHttp\Psr7\Response as Psr7Response; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Http\Client\ConnectionException; +use Illuminate\Http\Client\RequestException; +use Illuminate\Http\Client\Response as HttpResponse; use Illuminate\Support\Facades\Bus; use Mockery; use PHPUnit\Framework\Attributes\Test; @@ -47,7 +50,7 @@ public function it_has_correct_tries(): void $job = new FetchRecipePageJob($country, 'en'); - $this->assertSame(3, $job->tries); + $this->assertSame(5, $job->tries); } #[Test] @@ -81,10 +84,30 @@ public function it_accepts_custom_skip_value(): void $this->assertSame(100, $job->skip); } + #[Test] + public function it_defaults_take_from_country(): void + { + $country = Country::factory()->create(['take' => 200]); + + $job = new FetchRecipePageJob($country, 'en'); + + $this->assertSame(200, $job->take); + } + + #[Test] + public function it_accepts_custom_take_value(): void + { + $country = Country::factory()->create(['take' => 200]); + + $job = new FetchRecipePageJob($country, 'en', take: 150); + + $this->assertSame(150, $job->take); + } + #[Test] public function handle_fetches_recipes(): void { - $country = Country::factory()->create(); + $country = Country::factory()->create(['take' => 50]); $response = $this->createRecipesResponse([ 'items' => [ @@ -97,10 +120,9 @@ public function handle_fetches_recipes(): void ]); $client = Mockery::mock(HelloFreshClient::class); - $client->shouldReceive('withOutThrow')->andReturnSelf(); $client->shouldReceive('getRecipes') ->once() - ->with($country, 'en', 0) + ->with($country, 'en', 0, 50) ->andReturn($response); Bus::fake(); @@ -114,7 +136,7 @@ public function handle_fetches_recipes(): void #[Test] public function handle_uses_skip_value(): void { - $country = Country::factory()->create(); + $country = Country::factory()->create(['take' => 50]); $response = $this->createRecipesResponse([ 'items' => [], @@ -125,10 +147,9 @@ public function handle_uses_skip_value(): void ]); $client = Mockery::mock(HelloFreshClient::class); - $client->shouldReceive('withOutThrow')->andReturnSelf(); $client->shouldReceive('getRecipes') ->once() - ->with($country, 'en', 100) + ->with($country, 'en', 100, 50) ->andReturn($response); Bus::fake(); @@ -139,6 +160,71 @@ public function handle_uses_skip_value(): void $this->assertTrue(true); } + #[Test] + public function handle_dispatches_new_job_with_reduced_take_on_server_error(): void + { + $country = Country::factory()->create(['take' => 200]); + + $httpResponse = new HttpResponse(new Psr7Response(500)); + $exception = new RequestException($httpResponse); + + $client = Mockery::mock(HelloFreshClient::class); + $client->shouldReceive('getRecipes') + ->once() + ->andThrow($exception); + + Bus::fake(); + + $job = new FetchRecipePageJob($country, 'en'); + $job->handle($client); + + Bus::assertDispatched(function (FetchRecipePageJob $dispatched) use ($country): bool { + return $dispatched->take === 150 + && $dispatched->country->is($country) + && $dispatched->locale === 'en' + && $dispatched->skip === 0; + }); + } + + #[Test] + public function handle_does_not_dispatch_reduced_take_on_connection_error(): void + { + $country = Country::factory()->create(['take' => 200]); + + $client = Mockery::mock(HelloFreshClient::class); + $client->shouldReceive('getRecipes') + ->once() + ->andThrow(new ConnectionException('Connection failed')); + + Bus::fake(); + + $job = new FetchRecipePageJob($country, 'en'); + $job->handle($client); + + Bus::assertNotDispatched(FetchRecipePageJob::class); + } + + #[Test] + public function handle_does_not_dispatch_reduced_take_when_take_is_fifty_or_less(): void + { + $country = Country::factory()->create(['take' => 50]); + + $httpResponse = new HttpResponse(new Psr7Response(500)); + $exception = new RequestException($httpResponse); + + $client = Mockery::mock(HelloFreshClient::class); + $client->shouldReceive('getRecipes') + ->once() + ->andThrow($exception); + + Bus::fake(); + + $job = new FetchRecipePageJob($country, 'en'); + $job->handle($client); + + Bus::assertNotDispatched(FetchRecipePageJob::class); + } + protected function createRecipesResponse(array $data): RecipesResponse { $psr7Response = new Psr7Response(200, [], json_encode($data));