Release v1.4.0 #146
7 changed files with 252 additions and 0 deletions
|
|
@ -41,6 +41,7 @@ public function execute(RouteArticle $routeArticle): PublishOutcome
|
||||||
: PublishOutcome::failure('Could not recover the article content to publish');
|
: PublishOutcome::failure('Could not recover the article content to publish');
|
||||||
} catch (Exception $e) {
|
} catch (Exception $e) {
|
||||||
$routeArticle->update(['publish_status' => PublishStatusEnum::ERROR]);
|
$routeArticle->update(['publish_status' => PublishStatusEnum::ERROR]);
|
||||||
|
$routeArticle->recordPublishAttemptFailed();
|
||||||
|
|
||||||
ActionPerformed::dispatch('Failed to publish article', LogLevelEnum::ERROR, [
|
ActionPerformed::dispatch('Failed to publish article', LogLevelEnum::ERROR, [
|
||||||
'article_id' => $article->id,
|
'article_id' => $article->id,
|
||||||
|
|
@ -94,6 +95,7 @@ private function resolvePublishData(Article $article): array
|
||||||
private function recordPublished(RouteArticle $routeArticle): void
|
private function recordPublished(RouteArticle $routeArticle): void
|
||||||
{
|
{
|
||||||
$routeArticle->update(['publish_status' => PublishStatusEnum::PUBLISHED]);
|
$routeArticle->update(['publish_status' => PublishStatusEnum::PUBLISHED]);
|
||||||
|
$routeArticle->clearPublishAttempts();
|
||||||
|
|
||||||
ActionPerformed::dispatch('Published article', LogLevelEnum::INFO, [
|
ActionPerformed::dispatch('Published article', LogLevelEnum::INFO, [
|
||||||
'article_id' => $routeArticle->article->id,
|
'article_id' => $routeArticle->article->id,
|
||||||
|
|
@ -117,11 +119,13 @@ private function recordFailed(RouteArticle $routeArticle, PublishOutcome $outcom
|
||||||
$article = $routeArticle->article;
|
$article = $routeArticle->article;
|
||||||
|
|
||||||
$routeArticle->update(['publish_status' => PublishStatusEnum::ERROR]);
|
$routeArticle->update(['publish_status' => PublishStatusEnum::ERROR]);
|
||||||
|
$routeArticle->recordPublishAttemptFailed();
|
||||||
|
|
||||||
ActionPerformed::dispatch('No publication created for article', LogLevelEnum::WARNING, [
|
ActionPerformed::dispatch('No publication created for article', LogLevelEnum::WARNING, [
|
||||||
'article_id' => $article->id,
|
'article_id' => $article->id,
|
||||||
'title' => $article->title,
|
'title' => $article->title,
|
||||||
'reason' => $outcome->reason,
|
'reason' => $outcome->reason,
|
||||||
|
'attempt' => $routeArticle->publish_attempts,
|
||||||
]);
|
]);
|
||||||
|
|
||||||
$this->notificationService->send(
|
$this->notificationService->send(
|
||||||
|
|
|
||||||
|
|
@ -47,6 +47,11 @@ public function handle(PublishRouteArticleAction $publishRouteArticle): void
|
||||||
|
|
||||||
// Get the oldest approved route_article that hasn't been published to its channel yet
|
// Get the oldest approved route_article that hasn't been published to its channel yet
|
||||||
$routeArticle = RouteArticle::where('approval_status', ApprovalStatusEnum::APPROVED)
|
$routeArticle = RouteArticle::where('approval_status', ApprovalStatusEnum::APPROVED)
|
||||||
|
->where('publish_attempts', '<', RouteArticle::MAX_PUBLISH_ATTEMPTS)
|
||||||
|
->where(function ($query) {
|
||||||
|
$query->whereNull('next_attempt_at')
|
||||||
|
->orWhere('next_attempt_at', '<=', now());
|
||||||
|
})
|
||||||
->whereDoesntHave('article.articlePublications', function ($query) {
|
->whereDoesntHave('article.articlePublications', function ($query) {
|
||||||
$query->whereColumn('article_publications.platform_channel_id', 'route_articles.platform_channel_id');
|
$query->whereColumn('article_publications.platform_channel_id', 'route_articles.platform_channel_id');
|
||||||
})
|
})
|
||||||
|
|
|
||||||
|
|
@ -18,6 +18,8 @@
|
||||||
* @property int $article_id
|
* @property int $article_id
|
||||||
* @property ApprovalStatusEnum $approval_status
|
* @property ApprovalStatusEnum $approval_status
|
||||||
* @property PublishStatusEnum $publish_status
|
* @property PublishStatusEnum $publish_status
|
||||||
|
* @property int $publish_attempts
|
||||||
|
* @property Carbon|null $next_attempt_at
|
||||||
* @property Carbon|null $validated_at
|
* @property Carbon|null $validated_at
|
||||||
* @property Carbon $created_at
|
* @property Carbon $created_at
|
||||||
* @property Carbon $updated_at
|
* @property Carbon $updated_at
|
||||||
|
|
@ -33,12 +35,16 @@ class RouteArticle extends Model
|
||||||
'article_id',
|
'article_id',
|
||||||
'approval_status',
|
'approval_status',
|
||||||
'publish_status',
|
'publish_status',
|
||||||
|
'publish_attempts',
|
||||||
|
'next_attempt_at',
|
||||||
'validated_at',
|
'validated_at',
|
||||||
];
|
];
|
||||||
|
|
||||||
protected $casts = [
|
protected $casts = [
|
||||||
'approval_status' => ApprovalStatusEnum::class,
|
'approval_status' => ApprovalStatusEnum::class,
|
||||||
'publish_status' => PublishStatusEnum::class,
|
'publish_status' => PublishStatusEnum::class,
|
||||||
|
'publish_attempts' => 'integer',
|
||||||
|
'next_attempt_at' => 'datetime',
|
||||||
'validated_at' => 'datetime',
|
'validated_at' => 'datetime',
|
||||||
];
|
];
|
||||||
|
|
||||||
|
|
@ -105,4 +111,29 @@ public function reject(): void
|
||||||
{
|
{
|
||||||
$this->update(['approval_status' => ApprovalStatusEnum::REJECTED]);
|
$this->update(['approval_status' => ApprovalStatusEnum::REJECTED]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private const RETRY_BACKOFF_MINUTES = [5, 30, 120, 360];
|
||||||
|
|
||||||
|
public const MAX_PUBLISH_ATTEMPTS = 4;
|
||||||
|
|
||||||
|
public function recordPublishAttemptFailed(): void
|
||||||
|
{
|
||||||
|
$attempts = $this->publish_attempts + 1;
|
||||||
|
$backoff = self::RETRY_BACKOFF_MINUTES;
|
||||||
|
|
||||||
|
$this->update([
|
||||||
|
'publish_attempts' => $attempts,
|
||||||
|
'next_attempt_at' => now()->addMinutes($backoff[$attempts - 1] ?? end($backoff)),
|
||||||
|
]);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function clearPublishAttempts(): void
|
||||||
|
{
|
||||||
|
$this->update(['publish_attempts' => 0, 'next_attempt_at' => null]);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function hasExhaustedPublishAttempts(): bool
|
||||||
|
{
|
||||||
|
return $this->publish_attempts >= self::MAX_PUBLISH_ATTEMPTS;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
use Illuminate\Database\Migrations\Migration;
|
||||||
|
use Illuminate\Database\Schema\Blueprint;
|
||||||
|
use Illuminate\Support\Facades\Schema;
|
||||||
|
|
||||||
|
return new class extends Migration
|
||||||
|
{
|
||||||
|
public function up(): void
|
||||||
|
{
|
||||||
|
Schema::table('route_articles', function (Blueprint $table) {
|
||||||
|
$table->unsignedTinyInteger('publish_attempts')->default(0)->after('publish_status');
|
||||||
|
$table->timestamp('next_attempt_at')->nullable()->after('publish_attempts');
|
||||||
|
|
||||||
|
$table->index('next_attempt_at');
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
public function down(): void
|
||||||
|
{
|
||||||
|
Schema::table('route_articles', function (Blueprint $table) {
|
||||||
|
$table->dropIndex(['next_attempt_at']);
|
||||||
|
$table->dropColumn(['publish_attempts', 'next_attempt_at']);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
@ -177,6 +177,49 @@ public function test_publish_fails_when_fallback_fetch_returns_nothing(): void
|
||||||
]);
|
]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function test_repeated_failures_exhaust_the_retry_attempts(): void
|
||||||
|
{
|
||||||
|
$routeArticle = $this->createRouteArticle([], unvalidated: true);
|
||||||
|
|
||||||
|
$fetcher = Mockery::mock(ArticleFetcher::class);
|
||||||
|
$fetcher->shouldReceive('fetchArticleData')->andReturn([]);
|
||||||
|
|
||||||
|
$publishingService = Mockery::mock(ArticlePublishingService::class);
|
||||||
|
$action = new PublishRouteArticleAction($fetcher, $publishingService, new NotificationService);
|
||||||
|
|
||||||
|
for ($i = 0; $i < RouteArticle::MAX_PUBLISH_ATTEMPTS; $i++) {
|
||||||
|
$action->execute($routeArticle);
|
||||||
|
$routeArticle->refresh();
|
||||||
|
}
|
||||||
|
|
||||||
|
$this->assertSame(RouteArticle::MAX_PUBLISH_ATTEMPTS, $routeArticle->publish_attempts);
|
||||||
|
$this->assertTrue($routeArticle->hasExhaustedPublishAttempts());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function test_a_successful_publish_resets_earlier_failed_attempts(): void
|
||||||
|
{
|
||||||
|
$routeArticle = $this->createRouteArticle([
|
||||||
|
'description' => 'Stored description',
|
||||||
|
]);
|
||||||
|
$routeArticle->update(['publish_attempts' => 2, 'next_attempt_at' => now()->subMinute()]);
|
||||||
|
|
||||||
|
$fetcher = Mockery::mock(ArticleFetcher::class);
|
||||||
|
$fetcher->shouldNotReceive('fetchArticleData');
|
||||||
|
|
||||||
|
$publishingService = Mockery::mock(ArticlePublishingService::class);
|
||||||
|
$publishingService->shouldReceive('publishRouteArticle')
|
||||||
|
->once()
|
||||||
|
->andReturn(PublishOutcome::published($this->makePublication()));
|
||||||
|
|
||||||
|
(new PublishRouteArticleAction($fetcher, $publishingService, new NotificationService))
|
||||||
|
->execute($routeArticle);
|
||||||
|
|
||||||
|
$routeArticle->refresh();
|
||||||
|
|
||||||
|
$this->assertSame(0, $routeArticle->publish_attempts);
|
||||||
|
$this->assertNull($routeArticle->next_attempt_at);
|
||||||
|
}
|
||||||
|
|
||||||
public function test_publish_fails_when_fallback_recovers_only_a_title(): void
|
public function test_publish_fails_when_fallback_recovers_only_a_title(): void
|
||||||
{
|
{
|
||||||
$routeArticle = $this->createRouteArticle([], unvalidated: true);
|
$routeArticle = $this->createRouteArticle([], unvalidated: true);
|
||||||
|
|
|
||||||
|
|
@ -5,6 +5,7 @@
|
||||||
use App\Actions\PublishRouteArticleAction;
|
use App\Actions\PublishRouteArticleAction;
|
||||||
use App\Enums\NotificationSeverityEnum;
|
use App\Enums\NotificationSeverityEnum;
|
||||||
use App\Enums\NotificationTypeEnum;
|
use App\Enums\NotificationTypeEnum;
|
||||||
|
use App\Enums\PublishStatusEnum;
|
||||||
use App\Exceptions\PublishException;
|
use App\Exceptions\PublishException;
|
||||||
use App\Jobs\PublishNextArticleJob;
|
use App\Jobs\PublishNextArticleJob;
|
||||||
use App\Models\Article;
|
use App\Models\Article;
|
||||||
|
|
@ -398,6 +399,72 @@ public function test_handle_creates_notification_on_publish_exception(): void
|
||||||
$this->assertStringContainsString('Failing Article', $notification->title);
|
$this->assertStringContainsString('Failing Article', $notification->title);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function test_handle_skips_route_articles_that_are_not_due_for_retry(): void
|
||||||
|
{
|
||||||
|
$routeArticle = $this->createApprovedRouteArticle();
|
||||||
|
$routeArticle->update(['publish_attempts' => 1, 'next_attempt_at' => now()->addMinutes(5)]);
|
||||||
|
|
||||||
|
$articleFetcherMock = Mockery::mock(ArticleFetcher::class);
|
||||||
|
$articleFetcherMock->shouldNotReceive('fetchArticleData');
|
||||||
|
|
||||||
|
$publishingServiceMock = Mockery::mock(ArticlePublishingService::class);
|
||||||
|
$publishingServiceMock->shouldNotReceive('publishRouteArticle');
|
||||||
|
|
||||||
|
$job = new PublishNextArticleJob;
|
||||||
|
$job->handle(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, $this->notificationService));
|
||||||
|
|
||||||
|
$this->assertSame(PublishStatusEnum::UNPUBLISHED, $routeArticle->fresh()->publish_status);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function test_handle_skips_route_articles_that_exhausted_their_attempts(): void
|
||||||
|
{
|
||||||
|
$routeArticle = $this->createApprovedRouteArticle();
|
||||||
|
$routeArticle->update([
|
||||||
|
'publish_attempts' => RouteArticle::MAX_PUBLISH_ATTEMPTS,
|
||||||
|
'next_attempt_at' => now()->subDay(),
|
||||||
|
]);
|
||||||
|
|
||||||
|
$articleFetcherMock = Mockery::mock(ArticleFetcher::class);
|
||||||
|
$articleFetcherMock->shouldNotReceive('fetchArticleData');
|
||||||
|
|
||||||
|
$publishingServiceMock = Mockery::mock(ArticlePublishingService::class);
|
||||||
|
$publishingServiceMock->shouldNotReceive('publishRouteArticle');
|
||||||
|
|
||||||
|
$job = new PublishNextArticleJob;
|
||||||
|
$job->handle(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, $this->notificationService));
|
||||||
|
|
||||||
|
$this->assertSame(PublishStatusEnum::UNPUBLISHED, $routeArticle->fresh()->publish_status);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function test_handle_publishes_a_later_article_when_the_oldest_is_backing_off(): void
|
||||||
|
{
|
||||||
|
$blocked = $this->createApprovedRouteArticle(['title' => 'Blocked Article']);
|
||||||
|
$blocked->update([
|
||||||
|
'created_at' => now()->subDays(2),
|
||||||
|
'publish_attempts' => 1,
|
||||||
|
'next_attempt_at' => now()->addMinutes(5),
|
||||||
|
]);
|
||||||
|
|
||||||
|
$next = $this->createApprovedRouteArticle(['title' => 'Next Article']);
|
||||||
|
$next->update(['created_at' => now()->subDay()]);
|
||||||
|
|
||||||
|
$articleFetcherMock = Mockery::mock(ArticleFetcher::class);
|
||||||
|
$articleFetcherMock->shouldReceive('fetchArticleData')
|
||||||
|
->once()
|
||||||
|
->andReturn(['title' => 'Next Article', 'description' => 'Test description']);
|
||||||
|
|
||||||
|
$publishingServiceMock = Mockery::mock(ArticlePublishingService::class);
|
||||||
|
$publishingServiceMock->shouldReceive('publishRouteArticle')
|
||||||
|
->once()
|
||||||
|
->with(Mockery::on(fn ($ra) => $ra->id === $next->id), Mockery::any())
|
||||||
|
->andReturn(PublishOutcome::published($this->makePublication()));
|
||||||
|
|
||||||
|
$job = new PublishNextArticleJob;
|
||||||
|
$job->handle(new PublishRouteArticleAction($articleFetcherMock, $publishingServiceMock, $this->notificationService));
|
||||||
|
|
||||||
|
$this->assertSame(PublishStatusEnum::PUBLISHED, $next->fresh()->publish_status);
|
||||||
|
}
|
||||||
|
|
||||||
public function test_job_can_be_serialized(): void
|
public function test_job_can_be_serialized(): void
|
||||||
{
|
{
|
||||||
$job = new PublishNextArticleJob;
|
$job = new PublishNextArticleJob;
|
||||||
|
|
|
||||||
76
tests/Unit/Models/RouteArticleRetryTest.php
Normal file
76
tests/Unit/Models/RouteArticleRetryTest.php
Normal file
|
|
@ -0,0 +1,76 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
namespace Tests\Unit\Models;
|
||||||
|
|
||||||
|
use App\Models\RouteArticle;
|
||||||
|
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||||
|
use Tests\TestCase;
|
||||||
|
|
||||||
|
class RouteArticleRetryTest extends TestCase
|
||||||
|
{
|
||||||
|
use RefreshDatabase;
|
||||||
|
|
||||||
|
public function test_first_failure_schedules_the_shortest_backoff(): void
|
||||||
|
{
|
||||||
|
/** @var RouteArticle $routeArticle */
|
||||||
|
$routeArticle = RouteArticle::factory()->approved()->create();
|
||||||
|
|
||||||
|
$this->freezeTime(function () use ($routeArticle) {
|
||||||
|
$routeArticle->recordPublishAttemptFailed();
|
||||||
|
|
||||||
|
$this->assertSame(1, $routeArticle->publish_attempts);
|
||||||
|
$this->assertSame(
|
||||||
|
now()->addMinutes(5)->format('Y-m-d H:i:s'),
|
||||||
|
$routeArticle->next_attempt_at->format('Y-m-d H:i:s')
|
||||||
|
);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
public function test_backoff_grows_with_each_failure(): void
|
||||||
|
{
|
||||||
|
/** @var RouteArticle $routeArticle */
|
||||||
|
$routeArticle = RouteArticle::factory()->approved()->create();
|
||||||
|
|
||||||
|
$this->freezeTime(function () use ($routeArticle) {
|
||||||
|
foreach ([5, 30, 120, 360] as $expectedDelay) {
|
||||||
|
$routeArticle->recordPublishAttemptFailed();
|
||||||
|
|
||||||
|
$this->assertSame(
|
||||||
|
now()->addMinutes($expectedDelay)->format('Y-m-d H:i:s'),
|
||||||
|
$routeArticle->next_attempt_at->format('Y-m-d H:i:s'),
|
||||||
|
"Attempt {$routeArticle->publish_attempts} should wait {$expectedDelay} minutes"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
public function test_attempts_are_exhausted_after_the_configured_maximum(): void
|
||||||
|
{
|
||||||
|
/** @var RouteArticle $routeArticle */
|
||||||
|
$routeArticle = RouteArticle::factory()->approved()->create();
|
||||||
|
|
||||||
|
for ($i = 0; $i < RouteArticle::MAX_PUBLISH_ATTEMPTS - 1; $i++) {
|
||||||
|
$routeArticle->recordPublishAttemptFailed();
|
||||||
|
$this->assertFalse($routeArticle->hasExhaustedPublishAttempts());
|
||||||
|
}
|
||||||
|
|
||||||
|
$routeArticle->recordPublishAttemptFailed();
|
||||||
|
|
||||||
|
$this->assertTrue($routeArticle->hasExhaustedPublishAttempts());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function test_a_successful_publish_clears_previous_attempts(): void
|
||||||
|
{
|
||||||
|
/** @var RouteArticle $routeArticle */
|
||||||
|
$routeArticle = RouteArticle::factory()->approved()->create();
|
||||||
|
|
||||||
|
$routeArticle->recordPublishAttemptFailed();
|
||||||
|
$routeArticle->recordPublishAttemptFailed();
|
||||||
|
|
||||||
|
$routeArticle->clearPublishAttempts();
|
||||||
|
|
||||||
|
$this->assertSame(0, $routeArticle->publish_attempts);
|
||||||
|
$this->assertNull($routeArticle->next_attempt_at);
|
||||||
|
$this->assertFalse($routeArticle->hasExhaustedPublishAttempts());
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in a new issue