diff --git a/CHANGELOG.md b/CHANGELOG.md index 43902c39..c22139eb 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## 3.4.2 - 2023-02-21 +### Changed +- Retry logic for queues + ## 3.4.1 - 2023-02-13 ### Changed - Queues priority decreased for sending jobs to 1024 and receiving to 2048 diff --git a/composer.json b/composer.json index 55eeb9a1..a79d8ae2 100755 --- a/composer.json +++ b/composer.json @@ -2,7 +2,7 @@ "name": "lilt/craft-lilt-plugin", "description": "The Lilt plugin makes it easy for you to send content to Lilt for translation right from within Craft CMS.", "type": "craft-plugin", - "version": "3.4.1", + "version": "3.4.2", "keywords": [ "craft", "cms", diff --git a/src/elements/Job.php b/src/elements/Job.php index f86db757..5bd32189 100644 --- a/src/elements/Job.php +++ b/src/elements/Job.php @@ -187,7 +187,11 @@ public static function hasStatuses(): bool public function afterDelete(): bool { JobRecord::deleteAll(['id' => $this->id]); + parent::afterDelete(); + + Craft::$app->elements->invalidateCachesForElementType(self::class); + return true; } diff --git a/src/modules/AbstractRetryJob.php b/src/modules/AbstractRetryJob.php new file mode 100644 index 00000000..4ecfcb3a --- /dev/null +++ b/src/modules/AbstractRetryJob.php @@ -0,0 +1,34 @@ +attempt < self::RETRY_COUNT; } - public function canRetry($attempt, $error): bool + public function getRetryJob(): BaseJob { - return $attempt < self::RETRY_COUNT; + return new self([ + 'jobId' => $this->jobId, + 'liltJobId' => $this->liltJobId, + 'attempt' => $this->attempt + 1 + ]); } public static function getDelay(): int diff --git a/src/modules/FetchTranslationFromConnector.php b/src/modules/FetchTranslationFromConnector.php index 917f6cc3..1e8fb5f4 100644 --- a/src/modules/FetchTranslationFromConnector.php +++ b/src/modules/FetchTranslationFromConnector.php @@ -21,9 +21,8 @@ use lilthq\craftliltplugin\parameters\CraftliltpluginParameters; use lilthq\craftliltplugin\records\TranslationRecord; use Throwable; -use yii\queue\RetryableJobInterface; -class FetchTranslationFromConnector extends BaseJob implements RetryableJobInterface +class FetchTranslationFromConnector extends AbstractRetryJob { public const DELAY_IN_SECONDS_INSTANT = 10; public const DELAY_IN_SECONDS_VERIFIED = 60 * 5; @@ -32,11 +31,6 @@ class FetchTranslationFromConnector extends BaseJob implements RetryableJobInter private const RETRY_COUNT = 3; - /** - * @var int $jobId - */ - public $jobId; - /** * @var int $liltJobId */ @@ -191,14 +185,19 @@ private function markAsDone($queue): void ); } - public function getTtr(): int + public function canRetry(): bool { - return self::TTR; + return $this->attempt < self::RETRY_COUNT; } - public function canRetry($attempt, $error): bool + public function getRetryJob(): BaseJob { - return $attempt < self::RETRY_COUNT; + return new self([ + 'jobId' => $this->jobId, + 'liltJobId' => $this->liltJobId, + 'translationId' => $this->translationId, + 'attempt' => $this->attempt + 1 + ]); } /** @@ -233,7 +232,7 @@ private function isTranslationFinished($job, TranslationResponse $translationFro ) === TranslationResponse::STATUS_EXPORT_COMPLETE); } - public static function getDelay(string $flow): int + public static function getDelay(string $flow = CraftliltpluginParameters::TRANSLATION_WORKFLOW_INSTANT): int { $envDelay = getenv('CRAFT_LILT_PLUGIN_QUEUE_DELAY_IN_SECONDS'); if (!empty($envDelay) || $envDelay === '0') { diff --git a/src/modules/SendJobToConnector.php b/src/modules/SendJobToConnector.php index b3c3f51f..cf752e53 100644 --- a/src/modules/SendJobToConnector.php +++ b/src/modules/SendJobToConnector.php @@ -17,9 +17,8 @@ use lilthq\craftliltplugin\elements\Job; use lilthq\craftliltplugin\records\JobRecord; use Throwable; -use yii\queue\RetryableJobInterface; -class SendJobToConnector extends BaseJob implements RetryableJobInterface +class SendJobToConnector extends AbstractRetryJob { public const DELAY_IN_SECONDS = 60; public const PRIORITY = 1024; @@ -27,11 +26,6 @@ class SendJobToConnector extends BaseJob implements RetryableJobInterface private const RETRY_COUNT = 3; - /** - * @var int $jobId - */ - public $jobId; - /** * @inheritdoc * @@ -106,23 +100,26 @@ private function markAsDone($queue): void ); } - public function getTtr(): int - { - return self::TTR; - } - - public function canRetry($attempt, $error): bool - { - return $attempt < self::RETRY_COUNT; - } - public static function getDelay(): int { $envDelay = getenv('CRAFT_LILT_PLUGIN_QUEUE_DELAY_IN_SECONDS'); if (!empty($envDelay) || $envDelay === '0') { - return (int) $envDelay; + return (int)$envDelay; } return self::DELAY_IN_SECONDS; } + + public function canRetry(): bool + { + return $this->attempt < self::RETRY_COUNT; + } + + public function getRetryJob(): BaseJob + { + return new self([ + 'jobId' => $this->jobId, + 'attempt' => $this->attempt + 1 + ]); + } } diff --git a/src/services/listeners/AfterErrorListener.php b/src/services/listeners/AfterErrorListener.php index 04e1fe7f..fc07ba95 100644 --- a/src/services/listeners/AfterErrorListener.php +++ b/src/services/listeners/AfterErrorListener.php @@ -53,27 +53,6 @@ private function isEventEligible(Event $event): bool return false; } - if ($event->retry) { - // we only wait for job which will be not retried anymore - - Craftliltplugin::getInstance()->jobLogsRepository->create( - $event->job->jobId, - Craft::$app->getUser()->getId(), - substr( - sprintf( - 'Job %s failed on %d attempt. Error message: %s', - get_class($event->job), - $event->attempt, - $event->error->getMessage() - ), - 0, - 255 - ) - ); - - return false; - } - $jobClass = get_class($event->job); return in_array($jobClass, self::SUPPORTED_JOBS); @@ -85,25 +64,30 @@ public function __invoke(Event $event): Event return $event; } - $jobRecord = JobRecord::findOne(['id' => $event->job->jobId]); + /** + * @var FetchTranslationFromConnector|FetchJobStatusFromConnector|SendJobToConnector $queueJob + */ + $queueJob = $event->job; + + $jobRecord = JobRecord::findOne(['id' => $queueJob->jobId]); + + Craftliltplugin::getInstance()->jobLogsRepository->create( + $jobRecord->id, + Craft::$app->getUser()->getId(), + substr( + sprintf( + 'Job failed after %d attempt(s). Error message: %s', + $queueJob->attempt, + $event->error->getMessage() + ), + 0, + 255 + ) + ); - if ($jobRecord !== null) { + if (!$queueJob->canRetry()) { $jobRecord->status = Job::STATUS_FAILED; - Craftliltplugin::getInstance()->jobLogsRepository->create( - $jobRecord->id, - Craft::$app->getUser()->getId(), - substr( - sprintf( - 'Job failed after %d attempt(s). Error message: %s', - $event->attempt, - $event->error->getMessage() - ), - 0, - 255 - ) - ); - $jobRecord->save(); TranslationRecord::updateAll( @@ -113,12 +97,26 @@ public function __invoke(Event $event): Event Craft::$app->elements->invalidateCachesForElementType(Translation::class); Craft::$app->elements->invalidateCachesForElementType(Job::class); + + Craft::$app->queue->release( + (string) $event->id + ); + + return $event; } Craft::$app->queue->release( (string) $event->id ); + ++$queueJob->attempt; + + \craft\helpers\Queue::push( + $queueJob, + $queueJob::PRIORITY, + $queueJob::getDelay() + ); + return $event; } } diff --git a/tests/integration/services/listeners/AfterErrorListenerCest.php b/tests/integration/services/listeners/AfterErrorListenerCest.php index de07dbea..a1521a70 100644 --- a/tests/integration/services/listeners/AfterErrorListenerCest.php +++ b/tests/integration/services/listeners/AfterErrorListenerCest.php @@ -11,10 +11,14 @@ use lilthq\craftliltplugin\Craftliltplugin; use lilthq\craftliltplugin\elements\Job; use lilthq\craftliltplugin\modules\FetchJobStatusFromConnector; +use lilthq\craftliltplugin\modules\FetchTranslationFromConnector; +use lilthq\craftliltplugin\modules\SendJobToConnector; +use lilthq\craftliltplugin\records\JobRecord; use lilthq\craftliltplugin\records\TranslationRecord; use lilthq\craftliltplugin\services\listeners\AfterErrorListener; use lilthq\craftliltplugintests\integration\AbstractIntegrationCest; use lilthq\tests\fixtures\EntriesFixture; +use PHPUnit\Framework\Assert; use yii\queue\ExecEvent; use IntegrationTester; @@ -32,7 +36,7 @@ public function _fixtures(): array /** * @throws ModuleException */ - public function testInvoke(IntegrationTester $I): void { + public function testInvokeNoRetry_FetchJobStatusFromConnector(IntegrationTester $I): void { $user = \Craft::$app->getUsers()->getUserById(1); $I->amLoggedInAs($user); @@ -62,6 +66,7 @@ public function testInvoke(IntegrationTester $I): void { $event->job = new FetchJobStatusFromConnector([ 'jobId' => $job->id, + 'attempt' => 10, ]); $afterErrorListener->__invoke($event); @@ -72,4 +77,268 @@ public function testInvoke(IntegrationTester $I): void { $I->assertTranslationStatus($translation->id, Job::STATUS_FAILED); } } + + /** + * @throws ModuleException + */ + public function testInvokeWithRetry_FetchJobStatusFromConnector(IntegrationTester $I): void { + $user = \Craft::$app->getUsers()->getUserById(1); + $I->amLoggedInAs($user); + + $element = Entry::findOne(['authorId' => 1]); + + /** + * @var Job $job + * @var TranslationRecord[] $translations + */ + [$job, $translations] = $I->createJobWithTranslations([ + 'title' => 'Awesome test job', + 'elementIds' => [$element->id], + 'targetSiteIds' => '*', + 'sourceSiteId' => Craftliltplugin::getInstance()->languageMapper->getSiteIdByLanguage('en-US'), + 'translationWorkflow' => SettingsResponse::LILT_TRANSLATION_WORKFLOW_INSTANT, + 'versions' => [], + 'authorId' => 1, + 'liltJobId' => 777, + ]); + + $jobRecord = JobRecord::findOne(['id' => $job->id]); + Assert::assertNotNull($jobRecord); + + $jobRecord->status = Job::STATUS_IN_PROGRESS; + $jobRecord->save(); + + $afterErrorListener = new AfterErrorListener(); + $event = new ExecEvent(); + + $event->id = 123; + $event->retry = false; + $event->error = new Exception("Exception example"); + + $event->job = new FetchJobStatusFromConnector([ + 'jobId' => $job->id, + 'attempt' => 2, + ]); + + $afterErrorListener->__invoke($event); + + $I->assertJobStatus($job->id, Job::STATUS_IN_PROGRESS); + + foreach ($translations as $translation) { + $I->assertTranslationStatus($translation->id, Job::STATUS_IN_PROGRESS); + } + + $I->assertJobInQueue( + new FetchJobStatusFromConnector([ + 'jobId' => $job->id, + 'attempt' => 3, + ]) + ); + } + + /** + * @throws ModuleException + */ + public function testInvokeNoRetry_FetchTranslationFromConnector(IntegrationTester $I): void { + $user = \Craft::$app->getUsers()->getUserById(1); + $I->amLoggedInAs($user); + + $element = Entry::findOne(['authorId' => 1]); + + /** + * @var Job $job + * @var TranslationRecord[] $translations + */ + [$job, $translations] = $I->createJobWithTranslations([ + 'title' => 'Awesome test job', + 'elementIds' => [$element->id], + 'targetSiteIds' => '*', + 'sourceSiteId' => Craftliltplugin::getInstance()->languageMapper->getSiteIdByLanguage('en-US'), + 'translationWorkflow' => SettingsResponse::LILT_TRANSLATION_WORKFLOW_INSTANT, + 'versions' => [], + 'authorId' => 1, + 'liltJobId' => 777, + ]); + + $afterErrorListener = new AfterErrorListener(); + $event = new ExecEvent(); + + $event->id = 123; + $event->retry = false; + $event->error = new Exception("Exception example"); + + $event->job = new FetchTranslationFromConnector([ + 'jobId' => $job->id, + 'attempt' => 10, + ]); + + $afterErrorListener->__invoke($event); + + $I->assertJobStatus($job->id, Job::STATUS_FAILED); + + foreach ($translations as $translation) { + $I->assertTranslationStatus($translation->id, Job::STATUS_FAILED); + } + } + + /** + * @throws ModuleException + */ + public function testInvokeWithRetry_FetchTranslationFromConnector(IntegrationTester $I): void { + $user = \Craft::$app->getUsers()->getUserById(1); + $I->amLoggedInAs($user); + + $element = Entry::findOne(['authorId' => 1]); + + /** + * @var Job $job + * @var TranslationRecord[] $translations + */ + [$job, $translations] = $I->createJobWithTranslations([ + 'title' => 'Awesome test job', + 'elementIds' => [$element->id], + 'targetSiteIds' => '*', + 'sourceSiteId' => Craftliltplugin::getInstance()->languageMapper->getSiteIdByLanguage('en-US'), + 'translationWorkflow' => SettingsResponse::LILT_TRANSLATION_WORKFLOW_INSTANT, + 'versions' => [], + 'authorId' => 1, + 'liltJobId' => 777, + ]); + + $jobRecord = JobRecord::findOne(['id' => $job->id]); + Assert::assertNotNull($jobRecord); + + $jobRecord->status = Job::STATUS_IN_PROGRESS; + $jobRecord->save(); + + $afterErrorListener = new AfterErrorListener(); + $event = new ExecEvent(); + + $event->id = 123; + $event->retry = false; + $event->error = new Exception("Exception example"); + + $event->job = new FetchTranslationFromConnector([ + 'jobId' => $job->id, + 'attempt' => 2, + ]); + + $afterErrorListener->__invoke($event); + + $I->assertJobStatus($job->id, Job::STATUS_IN_PROGRESS); + + foreach ($translations as $translation) { + $I->assertTranslationStatus($translation->id, Job::STATUS_IN_PROGRESS); + } + + $I->assertJobInQueue( + new FetchTranslationFromConnector([ + 'jobId' => $job->id, + 'attempt' => 3, + ]) + ); + } + + /** + * @throws ModuleException + */ + public function testInvokeNoRetry_SendJobToConnector(IntegrationTester $I): void { + $user = \Craft::$app->getUsers()->getUserById(1); + $I->amLoggedInAs($user); + + $element = Entry::findOne(['authorId' => 1]); + + /** + * @var Job $job + * @var TranslationRecord[] $translations + */ + [$job, $translations] = $I->createJobWithTranslations([ + 'title' => 'Awesome test job', + 'elementIds' => [$element->id], + 'targetSiteIds' => '*', + 'sourceSiteId' => Craftliltplugin::getInstance()->languageMapper->getSiteIdByLanguage('en-US'), + 'translationWorkflow' => SettingsResponse::LILT_TRANSLATION_WORKFLOW_INSTANT, + 'versions' => [], + 'authorId' => 1, + 'liltJobId' => 777, + ]); + + $afterErrorListener = new AfterErrorListener(); + $event = new ExecEvent(); + + $event->id = 123; + $event->retry = false; + $event->error = new Exception("Exception example"); + + $event->job = new SendJobToConnector([ + 'jobId' => $job->id, + 'attempt' => 10, + ]); + + $afterErrorListener->__invoke($event); + + $I->assertJobStatus($job->id, Job::STATUS_FAILED); + + foreach ($translations as $translation) { + $I->assertTranslationStatus($translation->id, Job::STATUS_FAILED); + } + } + + /** + * @throws ModuleException + */ + public function testInvokeWithRetry_SendJobToConnector(IntegrationTester $I): void { + $user = \Craft::$app->getUsers()->getUserById(1); + $I->amLoggedInAs($user); + + $element = Entry::findOne(['authorId' => 1]); + + /** + * @var Job $job + * @var TranslationRecord[] $translations + */ + [$job, $translations] = $I->createJobWithTranslations([ + 'title' => 'Awesome test job', + 'elementIds' => [$element->id], + 'targetSiteIds' => '*', + 'sourceSiteId' => Craftliltplugin::getInstance()->languageMapper->getSiteIdByLanguage('en-US'), + 'translationWorkflow' => SettingsResponse::LILT_TRANSLATION_WORKFLOW_INSTANT, + 'versions' => [], + 'authorId' => 1, + 'liltJobId' => 777, + ]); + + $jobRecord = JobRecord::findOne(['id' => $job->id]); + Assert::assertNotNull($jobRecord); + + $jobRecord->status = Job::STATUS_IN_PROGRESS; + $jobRecord->save(); + + $afterErrorListener = new AfterErrorListener(); + $event = new ExecEvent(); + + $event->id = 123; + $event->retry = false; + $event->error = new Exception("Exception example"); + + $event->job = new SendJobToConnector([ + 'jobId' => $job->id, + 'attempt' => 2, + ]); + + $afterErrorListener->__invoke($event); + + $I->assertJobStatus($job->id, Job::STATUS_IN_PROGRESS); + + foreach ($translations as $translation) { + $I->assertTranslationStatus($translation->id, Job::STATUS_IN_PROGRESS); + } + + $I->assertJobInQueue( + new SendJobToConnector([ + 'jobId' => $job->id, + 'attempt' => 3, + ]) + ); + } }