DE-106539 Make max delay in seconds configurable (#98)

* DE-106539 Make max delay in seconds configurable * DE-106539 Add tests directory to ecs configuration * DE-106539 Simplify code by defaulting argument to MAX_DELAY_IN_SECONDS * DE-106539 Add delayInSeconds and maxDelayInSeconds to log context
BrandEmbassy · Jul 18, 2024 · 8f8e878 · 8f8e878
1 parent 5f07776
commit 8f8e878
Show file tree

Hide file tree

Showing 14 changed files with 131 additions and 53 deletions.
diff --git a/ecs.php b/ecs.php
@@ -1,5 +1,6 @@
 <?php declare(strict_types = 1);
 
+use PHP_CodeSniffer\Standards\Squiz\Sniffs\PHP\CommentedOutCodeSniff;
 use Symplify\EasyCodingStandard\Config\ECSConfig;
 
 $defaultEcsConfigurationSetup = require 'vendor/brandembassy/coding-standard/default-ecs.php';
@@ -9,10 +10,15 @@
 
     $ecsConfig->paths([
         'src',
+        'tests',
         'ecs.php',
     ]);
 
-    $skipList = [];
+    $skipList = [
+        CommentedOutCodeSniff::class . '.Found' => [
+            'tests/Queue/AWSSQS/SqsClientFactoryTest.php',
+        ],
+    ];
 
     $ecsConfig->skip(array_merge($defaultSkipList, $skipList));
 };
diff --git a/src/Queue/AWSSQS/SqsQueueManager.php b/src/Queue/AWSSQS/SqsQueueManager.php
@@ -45,7 +45,7 @@ class SqsQueueManager implements QueueManagerInterface
     private const DELAY_SECONDS = 'DelaySeconds';
 
     // SQS allows maximum message delay of 15 minutes
-    private const MAX_DELAY_SECONDS = 15 * 60;
+    private const MAX_DELAY_IN_SECONDS = 15 * 60;
 
     private string $s3BucketName;
 
@@ -231,11 +231,22 @@ public function pushDelayedWithMilliseconds(JobInterface $job, int $delayInMilli
     }
 
 
-    public function pushDelayed(JobInterface $job, int $delayInSeconds): void
+    /**
+     * @param int $maxDelayInSeconds This parameter can be used to override the default maximum delay before using
+     *                               delayed job scheduler (if one is configured). This can be useful for
+     *                               implementation of automated tests & synthetic monitoring of delayed job
+     *                               scheduler on live environments while maintaining quick feedback loop.
+     */
+    public function pushDelayed(JobInterface $job, int $delayInSeconds, int $maxDelayInSeconds = self::MAX_DELAY_IN_SECONDS): void
     {
+        assert(
+            $maxDelayInSeconds > 0,
+            'Argument $maxDelayInSeconds must be greater than 0',
+        );
+
         $prefixedQueueName = $this->getPrefixedQueueName($job->getJobDefinition()->getQueueName());
 
-        if ($delayInSeconds > self::MAX_DELAY_SECONDS) {
+        if ($delayInSeconds > $maxDelayInSeconds) {
             $executionPlannedAt = $this->dateTimeImmutableFactory->getNow()->modify(
                 sprintf('+ %d seconds', $delayInSeconds),
             );
@@ -252,6 +263,8 @@ public function pushDelayed(JobInterface $job, int $delayInSeconds): void
                     [
                         'executionPlannedAt' => DateTimeFormatter::format($executionPlannedAt),
                         'scheduledEventId' => $scheduledEventId,
+                        'delayInSeconds' => $delayInSeconds,
+                        'maxDelayInSeconds' => $maxDelayInSeconds,
                         LoggerContextField::JOB_QUEUE_NAME => $prefixedQueueName,
                         LoggerContextField::JOB_UUID => $job->getUuid(),
                     ],
@@ -264,12 +277,14 @@ public function pushDelayed(JobInterface $job, int $delayInSeconds): void
                 'Requested delay is greater than SQS limit. Job execution has been planned and will be requeued until then.',
                 [
                     'executionPlannedAt' => DateTimeFormatter::format($executionPlannedAt),
+                    'delayInSeconds' => $delayInSeconds,
+                    'maxDelayInSeconds' => $maxDelayInSeconds,
                     LoggerContextField::JOB_QUEUE_NAME => $prefixedQueueName,
                     LoggerContextField::JOB_UUID => $job->getUuid(),
                 ],
             );
 
-            $delayInSeconds = self::MAX_DELAY_SECONDS;
+            $delayInSeconds = self::MAX_DELAY_IN_SECONDS;
         }
 
         $parameters = [self::DELAY_SECONDS => $delayInSeconds];
@@ -309,7 +324,7 @@ private function publishMessage(
 
         $delaySeconds = (int)($properties[self::DELAY_SECONDS] ?? 0);
 
-        if ($delaySeconds < 0 || $delaySeconds > self::MAX_DELAY_SECONDS) {
+        if ($delaySeconds < 0 || $delaySeconds > self::MAX_DELAY_IN_SECONDS) {
             throw SqsClientException::createFromInvalidDelaySeconds($delaySeconds);
         }
 

diff --git a/tests/Jobs/ExampleJob.php b/tests/Jobs/ExampleJob.php
@@ -8,6 +8,8 @@
 use BrandEmbassy\DateTime\DateTimeFromString;
 use Doctrine\Common\Collections\ArrayCollection;
 use Tests\BE\QueueManagement\Jobs\JobDefinitions\ExampleJobDefinition;
+use function assert;
+use function is_string;
 use function str_repeat;
 
 /**

diff --git a/tests/Jobs/ExampleJobTest.php b/tests/Jobs/ExampleJobTest.php
@@ -37,7 +37,9 @@ public function testInitializedData(): void
             JobParameters::JOB_NAME => ExampleJob::JOB_NAME,
             JobParameters::ATTEMPTS => 1,
             JobParameters::CREATED_AT => self::JOB_CREATED_AT,
-            JobParameters::PARAMETERS => ['foo' => 'bar'],
+            JobParameters::PARAMETERS => [
+                'foo' => 'bar',
+            ],
             JobParameters::EXECUTION_PLANNED_AT => null,
         ];
 

diff --git a/tests/Jobs/Execution/ExampleExceptionWithCustomLogLevel.php b/tests/Jobs/Execution/ExampleExceptionWithCustomLogLevel.php
@@ -17,6 +17,7 @@ public static function create(JobInterface $job): self
         return new self($job, 'I will be logged as info');
     }
 
+
     public function getLogLevelForJob(JobInterface $job): string
     {
         return LogLevel::INFO;

diff --git a/tests/Jobs/Execution/JobLoaderTest.php b/tests/Jobs/Execution/JobLoaderTest.php
@@ -18,6 +18,7 @@
 use Mockery\MockInterface;
 use Nette\Utils\Json;
 use PHPUnit\Framework\Assert;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 use Tests\BE\QueueManagement\Jobs\ExampleJob;
 use Tests\BE\QueueManagement\Jobs\JobDefinitions\ExampleJobDefinition;
@@ -50,7 +51,7 @@ protected function setUp(): void
     }
 
 
-    #[\PHPUnit\Framework\Attributes\DataProvider('executionPlannedAtDataProvider')]
+    #[DataProvider('executionPlannedAtDataProvider')]
     public function testLoadSimpleJob(?DateTimeImmutable $executionPlannedAt): void
     {
         $jobLoader = $this->createJobLoader();
@@ -73,7 +74,9 @@ public function testLoadSimpleJob(?DateTimeImmutable $executionPlannedAt): void
             JobParameters::ATTEMPTS => ExampleJob::ATTEMPTS,
             JobParameters::JOB_NAME => ExampleJob::JOB_NAME,
             JobParameters::CREATED_AT => ExampleJob::CREATED_AT,
-            JobParameters::PARAMETERS => [ExampleJob::PARAMETER_FOO => 'bar'],
+            JobParameters::PARAMETERS => [
+                ExampleJob::PARAMETER_FOO => 'bar',
+            ],
         ];
 
         if ($executionPlannedAt !== null) {
@@ -131,8 +134,12 @@ private function createJobLoader(): JobLoader
     public static function executionPlannedAtDataProvider(): array
     {
         return [
-            'Null executionPlannedAt' => ['executionPlannedAt' => null],
-            'Not Null executionPlannedAt' => ['executionPlannedAt' => DateTimeFromString::create(self::EXECUTION_PLANNED_AT)],
+            'Null executionPlannedAt' => [
+                'executionPlannedAt' => null,
+            ],
+            'Not Null executionPlannedAt' => [
+                'executionPlannedAt' => DateTimeFromString::create(self::EXECUTION_PLANNED_AT),
+            ],
         ];
     }
 }
diff --git a/tests/Jobs/FailResolving/DefinedDelayRuleTest.php b/tests/Jobs/FailResolving/DefinedDelayRuleTest.php
@@ -14,6 +14,7 @@
 use Mockery;
 use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
 use PHPUnit\Framework\Assert;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 use Ramsey\Uuid\Uuid;
 use Throwable;
@@ -35,13 +36,16 @@ class DefinedDelayRuleTest extends TestCase
 
     private const MAXIMUM_DELAY = 300;
 
-    private const LINEAR_DELAY_DEFINITION = [4 => 30, 0 => 5];
+    private const LINEAR_DELAY_DEFINITION = [
+        4 => 30,
+        0 => 5,
+    ];
 
 
     /**
      * @throw Throwable
      */
-    #[\PHPUnit\Framework\Attributes\DataProvider('attemptsDataProvider')]
+    #[DataProvider('attemptsDataProvider')]
     public function testCorrectDelayIsReturned(
         int $expectedDelay,
         int $attempts
@@ -103,11 +107,10 @@ public static function attemptsDataProvider(): array
 
 
     /**
-     *
      * @param class-string<Throwable> $expectedException
      * @param int[] $linearDelayDefinition
      */
-    #[\PHPUnit\Framework\Attributes\DataProvider('badDataProvider')]
+    #[DataProvider('badDataProvider')]
     public function testExceptionIsThrownWithBadDelayDefinition(
         string $expectedException,
         string $expectedExceptionMessage,
@@ -132,12 +135,17 @@ public static function badDataProvider(): array
             'Missing definition for 0 attempts' => [
                 'expectedException' => DelayRuleException::class,
                 'expectedExceptionMessage' => 'Missing definition for 0 attempts',
-                'linearDelayDefinition' => [1 => 5],
+                'linearDelayDefinition' => [
+                    1 => 5,
+                ],
             ],
             'Incorrect definition order' => [
                 'expectedException' => DelayRuleException::class,
                 'expectedExceptionMessage' => 'Delays definition keys must be sorted descending',
-                'linearDelayDefinition' => [0 => 5, 2 => 5],
+                'linearDelayDefinition' => [
+                    0 => 5,
+                    2 => 5,
+                ],
             ],
         ];
     }

diff --git a/tests/Jobs/FailResolving/ExponentialDelayRuleTest.php b/tests/Jobs/FailResolving/ExponentialDelayRuleTest.php
@@ -14,6 +14,7 @@
 use Mockery;
 use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
 use PHPUnit\Framework\Assert;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 use Ramsey\Uuid\Uuid;
 
@@ -32,7 +33,7 @@ class ExponentialDelayRuleTest extends TestCase
     /**
      * @throw Throwable
      */
-    #[\PHPUnit\Framework\Attributes\DataProvider('attemptNumberDataProvider')]
+    #[DataProvider('attemptNumberDataProvider')]
     public function testCorrectDelayIsReturned(
         int $expectedDelayInMilliseconds,
         int $expectedDelayInSeconds,

diff --git a/tests/Jobs/JobDefinitions/JobDefinitionsContainerTest.php b/tests/Jobs/JobDefinitions/JobDefinitionsContainerTest.php
@@ -13,6 +13,7 @@
 use BE\QueueManagement\Jobs\SimpleJob;
 use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
 use PHPUnit\Framework\Assert;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 use Tests\BE\QueueManagement\Jobs\Execution\ExampleJobProcessor;
 
@@ -27,7 +28,7 @@ class JobDefinitionsContainerTest extends TestCase
     private const SIMPLE_JOB_NAME = 'simpleJob';
 
 
-    #[\PHPUnit\Framework\Attributes\DataProvider('queueDefinitionDataProvider')]
+    #[DataProvider('queueDefinitionDataProvider')]
     public function testGetJobDefinition(string $expectedQueueName, string $queueNamePrefix): void
     {
         $exampleJobProcessor = new ExampleJobProcessor();

diff --git a/tests/Queue/AWSSQS/SqsClientFactoryTest.php b/tests/Queue/AWSSQS/SqsClientFactoryTest.php
@@ -15,7 +15,7 @@ public function testUnableToEstablishConnectionThrowsException(): void
     {
         $sqsClientFactory = new SqsClientFactory(
             [
-                'region'  => 'eu-central-1',
+                'region' => 'eu-central-1',
                 'version' => '2015-10-07',  // will throw Aws\Exception\UnresolvedApiException: 'The sqs service does not have version: 2015-10-07'
                 'http' => [
                     'verify' => false,

diff --git a/tests/Queue/AWSSQS/SqsConsumerTest.php b/tests/Queue/AWSSQS/SqsConsumerTest.php
@@ -173,7 +173,8 @@ public function testRejectBlacklistedJob(): void
 
     public function testRemoveJobWithUnresolvableExceptionFailure(): void
     {
-        $unresolvableProcessFailException = new class() extends Exception implements UnresolvableProcessFailExceptionInterface{};
+        $unresolvableProcessFailException = new class() extends Exception implements UnresolvableProcessFailExceptionInterface {
+        };
 
         $this->jobLoaderMock->expects('loadJob')
             ->with('{"foo":"bar"}')
@@ -233,7 +234,7 @@ public function testRequeueDelayableProcessFail(): void
 
 
     #[DataProvider('possibleLogLevelAlteringExceptionsThrownDataProvider')]
-    public function testRequeueDelayableProcessFailWithLogLevelControl(Exception $thrownException, JobInterface $job, callable $loggerExpectationCallable): void
+    public function testRequeueDelayableProcessFailWithLogLevelControl(Throwable $thrownException, JobInterface $job, callable $loggerExpectationCallable): void
     {
         $this->jobLoaderMock->expects('loadJob')
             ->with('{"foo":"bar"}')
@@ -267,40 +268,40 @@ public function testRequeueDelayableProcessFailWithLogLevelControl(Exception $th
     public static function possibleLogLevelAlteringExceptionsThrownDataProvider(): array
     {
         return [
-            'exception is warning only' => (static function(): array {
+            'exception is warning only' => (static function (): array {
                 $exampleJob = new ExampleJob();
 
                 return [
                     'thrownException' => ExampleWarningOnlyException::create($exampleJob),
                     'job' => $exampleJob,
-                    'loggerExpectationCallable' => static fn(TestLogger $logger) => $logger->hasWarning('Job execution failed [attempts: 1], reason: I will be logged as a warning')
+                    'loggerExpectationCallable' => static fn(TestLogger $logger) => $logger->hasWarning('Job execution failed [attempts: 1], reason: I will be logged as a warning'),
                 ];
             })(),
-            'previous of the exception is warning only' => (static function(): array {
+            'previous of the exception is warning only' => (static function (): array {
                 $exampleJob = new ExampleJob();
 
                 return [
                     'thrownException' => ExampleExceptionWithPreviousWarningOnlyException::create($exampleJob),
                     'job' => $exampleJob,
-                    'loggerExpectationCallable' => static fn(TestLogger $logger) => $logger->hasWarning('Job execution failed [attempts: 1], reason: I will be logged as a warning')
+                    'loggerExpectationCallable' => static fn(TestLogger $logger) => $logger->hasWarning('Job execution failed [attempts: 1], reason: I will be logged as a warning'),
                 ];
             })(),
-            'exception is custom log level' => (static function(): array {
+            'exception is custom log level' => (static function (): array {
                 $exampleJob = new ExampleJob();
 
                 return [
                     'thrownException' => ExampleExceptionWithCustomLogLevel::create($exampleJob),
                     'job' => $exampleJob,
-                    'loggerExpectationCallable' => static fn(TestLogger $logger) => $logger->hasInfo('Job execution failed [attempts: 1], reason: I will be logged as a info')
+                    'loggerExpectationCallable' => static fn(TestLogger $logger) => $logger->hasInfo('Job execution failed [attempts: 1], reason: I will be logged as a info'),
                 ];
             })(),
-            'previous of the exception is custom log level' => (static function(): array {
+            'previous of the exception is custom log level' => (static function (): array {
                 $exampleJob = new ExampleJob();
 
                 return [
                     'thrownException' => ExampleExceptionWithPreviousCustomLogLevelException::create($exampleJob),
                     'job' => $exampleJob,
-                    'loggerExpectationCallable' => static fn(TestLogger $logger) => $logger->hasInfo('Job execution failed [attempts: 1], reason: I will be logged as a info')
+                    'loggerExpectationCallable' => static fn(TestLogger $logger) => $logger->hasInfo('Job execution failed [attempts: 1], reason: I will be logged as a info'),
                 ];
             })(),
         ];
@@ -397,7 +398,6 @@ public static function executionPlannedAtDataProvider(): array
             'Same dateTime' => [
                 'executionPlannedAt' => new DateTimeImmutable('2016-08-15T15:00:00+00:0'),
             ],
-
         ];
     }
 

diff --git a/tests/Queue/AWSSQS/SqsMessageTest.php b/tests/Queue/AWSSQS/SqsMessageTest.php
@@ -4,8 +4,10 @@
 
 use BE\QueueManagement\Queue\AWSSQS\SqsMessage;
 use PHPUnit\Framework\Assert;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 use function str_repeat;
+use function strlen;
 
 /**
  * @final
@@ -15,7 +17,7 @@ class SqsMessageTest extends TestCase
     /**
      * @param array<string, array<string, string>> $messageAttributes
      */
-    #[\PHPUnit\Framework\Attributes\DataProvider('messageProvider')]
+    #[DataProvider('messageProvider')]
     public function testIsTooBig(bool $expectedIsTooBig, string $messageBody, array $messageAttributes): void
     {
         Assert::assertSame($expectedIsTooBig, SqsMessage::isTooBig($messageBody, $messageAttributes));
-Original file line number
+Diff line change
@@ Expand Up / @@ -17,6 +17,7 @@ public static function create(JobInterface $job): self @@
             return new self($job, 'I will be logged as info');
         }
         public function getLogLevelForJob(JobInterface $job): string
         {
             return LogLevel::INFO;
@@ Expand Down @@