Skip to content

Commit

Permalink
[framework] fixed bc-breaks of cron module runs (#2596)
Browse files Browse the repository at this point in the history
  • Loading branch information
TomasLudvik committed Apr 24, 2023
1 parent ee63104 commit 0cb9f2b
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 10 deletions.
14 changes: 13 additions & 1 deletion packages/framework/src/Command/CronCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Shopsys\FrameworkBundle\Component\Cron\Config\CronModuleConfig;
use Shopsys\FrameworkBundle\Component\Cron\CronFacade;
use Shopsys\FrameworkBundle\Component\Cron\MutexFactory;
use Shopsys\FrameworkBundle\Component\Deprecations\DeprecationHelper;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
Expand Down Expand Up @@ -188,11 +189,22 @@ private function runCron(InputInterface $input, CronFacade $cronFacade, MutexFac
}

/**
* @phpstan-ignore-next-line
* @param int $runEveryMin
* @return \DateTimeImmutable
*/
private function getCurrentRoundedTime(int $runEveryMin = CronModuleConfig::RUN_EVERY_MIN_DEFAULT)
private function getCurrentRoundedTime(/* int $runEveryMin */)
{
$runEveryMin = DeprecationHelper::triggerNewArgumentInMethod(
__METHOD__,
'$runEveryMin',
'int',
func_get_args(),
0,
CronModuleConfig::RUN_EVERY_MIN_DEFAULT,
true
);

$time = new DateTime('now', $this->getCronTimeZone());
$time->modify('-' . $time->format('s') . ' sec');
$time->modify('-' . ($time->format('i') % $runEveryMin) . ' min');
Expand Down
30 changes: 28 additions & 2 deletions packages/framework/src/Component/Cron/Config/CronConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Shopsys\FrameworkBundle\Component\Cron\Config\Exception\CronModuleConfigNotFoundException;
use Shopsys\FrameworkBundle\Component\Cron\CronTimeResolver;
use Shopsys\FrameworkBundle\Component\Cron\Exception\InvalidCronModuleException;
use Shopsys\FrameworkBundle\Component\Deprecations\DeprecationHelper;
use Shopsys\Plugin\Cron\IteratedCronModuleInterface;
use Shopsys\Plugin\Cron\SimpleCronModuleInterface;

Expand Down Expand Up @@ -37,7 +38,9 @@ public function __construct(CronTimeResolver $cronTimeResolver)
* @param string $timeMinutes
* @param string $instanceName
* @param string|null $readableName
* @phpstan-ignore-next-line
* @param int $runEveryMin
* @phpstan-ignore-next-line
* @param int $timeoutIteratedCronSec
*/
public function registerCronModuleInstance(
Expand All @@ -47,12 +50,35 @@ public function registerCronModuleInstance(
string $timeMinutes,
string $instanceName,
?string $readableName = null,
int $runEveryMin = CronModuleConfig::RUN_EVERY_MIN_DEFAULT,
int $timeoutIteratedCronSec = CronModuleConfig::TIMEOUT_ITERATED_CRON_SEC_DEFAULT
/*
int $runEveryMin,
int $timeoutIteratedCronSec,
*/
): void {
if (!$service instanceof SimpleCronModuleInterface && !$service instanceof IteratedCronModuleInterface) {
throw new InvalidCronModuleException($serviceId);
}

$runEveryMin = DeprecationHelper::triggerNewArgumentInMethod(
__METHOD__,
'$runEveryMin',
'int',
func_get_args(),
6,
CronModuleConfig::RUN_EVERY_MIN_DEFAULT,
true
);

$timeoutIteratedCronSec = DeprecationHelper::triggerNewArgumentInMethod(
__METHOD__,
'$timeoutIteratedCronSec',
'int',
func_get_args(),
7,
CronModuleConfig::TIMEOUT_ITERATED_CRON_SEC_DEFAULT,
true
);

$this->cronTimeResolver->validateTimeString($timeHours, 23, 1);
$this->cronTimeResolver->validateTimeString($timeMinutes, 55, 5);

Expand Down
19 changes: 16 additions & 3 deletions packages/framework/src/Component/Cron/CronModuleExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use DateTimeImmutable;
use Shopsys\FrameworkBundle\Component\Cron\Config\CronConfig;
use Shopsys\FrameworkBundle\Component\Cron\Config\CronModuleConfig;
use Shopsys\FrameworkBundle\Component\Deprecations\DeprecationHelper;
use Shopsys\FrameworkBundle\DependencyInjection\SetterInjectionTrait;
use Shopsys\Plugin\Cron\IteratedCronModuleInterface;
use Shopsys\Plugin\Cron\SimpleCronModuleInterface;
Expand Down Expand Up @@ -75,7 +76,7 @@ public function runModule($cronModuleService, $suspended)
$cronModuleService->wakeUp();
}
$inProgress = true;
while ($this->canRun($cronConfig) && $inProgress === true) {
while ($inProgress === true && $this->canRun($cronConfig)) {
$inProgress = $cronModuleService->iterate();
}
if ($inProgress === true) {
Expand All @@ -90,11 +91,23 @@ public function runModule($cronModuleService, $suspended)

/**
* @phpstan-impure
* @param \Shopsys\FrameworkBundle\Component\Cron\Config\CronModuleConfig|null $cronConfig
* @phpstan-ignore-next-line
* @param \Shopsys\FrameworkBundle\Component\Cron\Config\CronModuleConfig $cronConfig
* @return bool
*/
public function canRun(?CronModuleConfig $cronConfig = null): bool
public function canRun(/* CronModuleConfig $cronConfig */): bool
{
$triggerNewArgumentInMethod = DeprecationHelper::triggerNewArgumentInMethod(
__METHOD__,
'$cronConfig',
'CronModuleConfig',
func_get_args(),
0,
null,
true
);
$cronConfig = $triggerNewArgumentInMethod;

if ($cronConfig !== null) {
$canRunUntil = $this->startedAt->add(
DateInterval::createFromDateString($cronConfig->getTimeoutIteratedCronSec() . ' seconds')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,32 @@ public static function triggerAbstractMethod(string $methodName): void
$methodName
);
}

/**
* @param string $methodName
* @param string $argumentName
* @param string $argumentType
* @param array $functionArguments
* @param int $positionOfArgument
* @param mixed $defaultValue
* @param bool $required
* @return mixed
*/
public static function triggerNewArgumentInMethod(string $methodName, string $argumentName, string $argumentType, array $functionArguments, int $positionOfArgument, mixed $defaultValue, bool $required): mixed
{
if (count($functionArguments) < $positionOfArgument + 1) {
if ($required) {
self::trigger(
'Method "%s()" will have a new "%s()" argument of type "%s()" in next major version, not defining it is deprecated.',
$methodName,
$argumentName,
$argumentType,
);
}

return $defaultValue;
}

return $functionArguments[$positionOfArgument];
}
}
8 changes: 4 additions & 4 deletions upgrade/UPGRADE-v11.0.1-dev.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ There you can find links to upgrade notes for other versions too.
- method `getCurrentRoundedTime` changed its interface:
```diff
function getCurrentRoundedTime(
+ int $runEveryMin = CronModuleConfig::RUN_EVERY_MIN_DEFAULT
+ int $runEveryMin,
)
```
- `Shopsys\FrameworkBundle\Component\Cron\CronModuleExecutor` class:
Expand All @@ -96,7 +96,7 @@ There you can find links to upgrade notes for other versions too.
- method `canRun` changed its interface:
```diff
function canRun(
+ CronModuleConfig|null $cronConfig = null
+ CronModuleConfig $cronConfig,
): bool
```
- `Shopsys\FrameworkBundle\Command\CronCommand` class:
Expand All @@ -109,8 +109,8 @@ There you can find links to upgrade notes for other versions too.
string $timeMinutes,
string $instanceName,
?string $readableName = null,
+ int $runEveryMin = CronModuleConfig::RUN_EVERY_MIN_DEFAULT,
+ int $timeoutIteratedCronSec = CronModuleConfig::TIMEOUT_ITERATED_CRON_SEC_DEFAULT
+ int $runEveryMin,
+ int $timeoutIteratedCronSec,
): void {
```
- `Shopsys\FrameworkBundle\Component\Cron\Config\CronModuleConfig` class:
Expand Down

0 comments on commit 0cb9f2b

Please sign in to comment.