CHANGELOG.md

+# v0.1.76
+
+* cron: The `dbp:relay:core:cron` command will no longer run all jobs the first
+  time it is called when the cache is empty.
+* cron: The `dbp:relay:core:cron` command gained `--force` option which forces
+  it to run all jobs, independent of their schedule.
+* cron: There is a new `dbp:relay:core:cron:list` command which lists all
+  registered cron jobs and related meta data.
+
 # v0.1.75
 
 * The logging context now includes the active symfony route name

src/Cron/CronCommand.php

@@ -4,14 +4,13 @@ declare(strict_types=1);
 
 namespace Dbp\Relay\CoreBundle\Cron;
 
-use Cron\CronExpression;
 use Dbp\Relay\CoreBundle\Cron\CronJobs\CachePrune;
-use Psr\Cache\CacheItemPoolInterface;
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerAwareTrait;
 use Psr\Log\NullLogger;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 
 final class CronCommand extends Command implements LoggerAwareInterface
@@ -21,127 +20,34 @@ final class CronCommand extends Command implements LoggerAwareInterface
     // dbp:cron only for backwards compat
     protected static $defaultName = 'dbp:relay:core:cron|dbp:cron';
 
-    /** @var CacheItemPoolInterface */
-    private $cachePool;
-
     /**
-     * @var CronJobInterface[]
+     * @var CronManager
      */
-    private $jobs;
+    private $manager;
 
-    public function __construct()
+    public function __construct(CronManager $manager)
     {
         parent::__construct();
-        $this->jobs = [];
         $this->logger = new NullLogger();
-    }
-
-    public function setCache(CacheItemPoolInterface $cachePool)
-    {
-        $this->cachePool = $cachePool;
-    }
-
-    public function addJob(CronJobInterface $job)
-    {
-        $this->jobs[] = $job;
+        $this->manager = $manager;
     }
 
     protected function configure()
     {
         $this->setDescription('Runs various tasks which need to be executed periodically');
-    }
-
-    /**
-     * Returns if a job should run or not. Note that there is no feedback channel, so if you skip
-     * this run you will only be notified the next time the cron job should run.
-     *
-     * @param string $cronExpression A cron expression
-     *
-     * @return bool If the job should run
-     */
-    public static function isDue(?\DateTimeInterface $previousRun, \DateTimeInterface $currentRun, string $cronExpression): bool
-    {
-        $cron = new CronExpression($cronExpression);
-        $previousExpectedRun = $cron->getPreviousRunDate($currentRun, 0, true);
-        $previousExpectedRun->setTimezone(new \DateTimeZone('UTC'));
-
-        $shouldRun = false;
-        // If we were scheduled to run between now and the previous run (or just before of no previous run exists)
-        // then we should run
-        if ($previousExpectedRun <= $currentRun && ($previousRun === null || $previousExpectedRun > $previousRun)) {
-            $shouldRun = true;
-        }
-
-        return $shouldRun;
-    }
-
-    public function getPreviousRun(\DateTimeInterface $currentTime): ?\DateTimeInterface
-    {
-        $cachePool = $this->cachePool;
-        // Store the previous run time in the cache and fetch from there
-        assert($cachePool instanceof CacheItemPoolInterface);
-        $item = $cachePool->getItem('cron-previous-run');
-        $value = $item->get();
-        $previousRun = null;
-        if ($value !== null) {
-            $previousRun = (new \DateTimeImmutable())->setTimezone(new \DateTimeZone('UTC'))->setTimestamp($value);
-            if ($previousRun > $currentTime) {
-                // Something is wrong, cap at the current time
-                $previousRun = $currentTime;
-            }
-        }
-        $item->set($currentTime->getTimestamp());
-        if ($cachePool->save($item) === false) {
-            throw new \RuntimeException('Saving cron timestamp failed');
-        }
-
-        return $previousRun;
-    }
-
-    /**
-     * @return CronJobInterface[]
-     */
-    protected function getDueJobs(): array
-    {
-        // Get all jobs that should have been run between the last time we were called and now
-        $currentTime = new \DateTimeImmutable('now', new \DateTimeZone('UTC'));
-        // round to full seconds, so we have the same resolution for both date times
-        $currentTime = $currentTime->setTimestamp($currentTime->getTimestamp());
-        $previousRunTime = $this->getPreviousRun($currentTime);
-
-        $toRun = [];
-        foreach ($this->jobs as $job) {
-            $interval = $job->getInterval();
-            $name = $job->getName();
-            $this->logger->info("cron: Checking '$name' ($interval)");
-            $isDue = self::isDue($previousRunTime, $currentTime, $interval);
-            if ($isDue) {
-                $toRun[] = $job;
-            }
-        }
-
-        return $toRun;
+        $this->addOption('force', null, InputOption::VALUE_NONE, 'Run the cron job even if it\'s not due');
     }
 
     protected function execute(InputInterface $input, OutputInterface $output): int
     {
         // We need to pass the prune command to CachePrune since I didn't find an alternative
         $app = $this->getApplication();
+        $force = $input->getOption('force');
         assert($app !== null);
         $command = $app->find('cache:pool:prune');
         CachePrune::setPruneCommand($command);
 
-        // Now run all jobs
-        $dueJobs = $this->getDueJobs();
-        foreach ($dueJobs as $job) {
-            $name = $job->getName();
-            $this->logger->info("cron: Running '$name'");
-            try {
-                $job->run(new CronOptions());
-            } catch (\Throwable $e) {
-                $this->logger->error("cron: '$name' failed", ['exception' => $e]);
-            }
-        }
+        $this->manager->runDueJobs($force);
 
         return 0;
     }

src/Cron/CronCompilerPass.php

@@ -20,10 +20,10 @@ class CronCompilerPass implements CompilerPassInterface
 
     public function process(ContainerBuilder $container)
     {
-        if (!$container->has(CronCommand::class)) {
+        if (!$container->has(CronManager::class)) {
             return;
         }
-        $definition = $container->findDefinition(CronCommand::class);
+        $definition = $container->findDefinition(CronManager::class);
         $taggedServices = $container->findTaggedServiceIds(self::TAG);
         foreach ($taggedServices as $id => $tags) {
             $definition->addMethodCall('addJob', [new Reference($id)]);

src/Cron/CronListCommand.php

+<?php
+
+declare(strict_types=1);
+
+namespace Dbp\Relay\CoreBundle\Cron;
+
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerAwareTrait;
+use Psr\Log\NullLogger;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+
+final class CronListCommand extends Command implements LoggerAwareInterface
+{
+    use LoggerAwareTrait;
+
+    // dbp:cron only for backwards compat
+    protected static $defaultName = 'dbp:relay:core:cron:list';
+
+    /**
+     * @var CronManager
+     */
+    private $manager;
+
+    public function __construct(CronManager $manager)
+    {
+        parent::__construct();
+        $this->logger = new NullLogger();
+        $this->manager = $manager;
+    }
+
+    protected function configure()
+    {
+        $this->setDescription('Lists all registered cron jobs');
+    }
+
+    protected function execute(InputInterface $input, OutputInterface $output): int
+    {
+        $currentTime = new \DateTimeImmutable('now', new \DateTimeZone('UTC'));
+        $jobs = $this->manager->getJobs();
+        foreach ($jobs as $job) {
+            $output->writeln('<fg=green;options=bold>['.get_class($job).']</>');
+            $output->writeln('<fg=blue;options=bold>Name:</> '.$job->getName());
+            $output->writeln('<fg=blue;options=bold>Cron:</> "'.$job->getInterval().'"');
+            $output->writeln('<fg=blue;options=bold>Now:</> '.$currentTime->format(\DateTime::ATOM));
+            $output->writeln('<fg=blue;options=bold>Next:</> '.$this->manager->getNextDate($job, $currentTime)->format(\DateTime::ATOM));
+        }
+
+        return 0;
+    }
+}

src/Cron/CronManager.php

+<?php
+
+declare(strict_types=1);
+
+namespace Dbp\Relay\CoreBundle\Cron;
+
+use Cron\CronExpression;
+use Psr\Cache\CacheItemPoolInterface;
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerAwareTrait;
+use Psr\Log\NullLogger;
+
+final class CronManager implements LoggerAwareInterface
+{
+    use LoggerAwareTrait;
+
+    /** @var CacheItemPoolInterface */
+    private $cachePool;
+
+    /**
+     * @var CronJobInterface[]
+     */
+    private $jobs;
+
+    public function __construct()
+    {
+        $this->jobs = [];
+        $this->logger = new NullLogger();
+    }
+
+    public function setCache(CacheItemPoolInterface $cachePool)
+    {
+        $this->cachePool = $cachePool;
+    }
+
+    public function addJob(CronJobInterface $job)
+    {
+        $this->jobs[] = $job;
+    }
+
+    /**
+     * Returns if a job should run or not. Note that there is no feedback channel, so if you skip
+     * this run you will only be notified the next time the cron job should run.
+     *
+     * @return bool If the job should run
+     */
+    public static function isDue(CronJobInterface $job, ?\DateTimeInterface $previousRun, \DateTimeInterface $currentRun): bool
+    {
+        $cron = new CronExpression($job->getInterval());
+        $previousExpectedRun = $cron->getPreviousRunDate($currentRun, 0, true);
+        $previousExpectedRun->setTimezone(new \DateTimeZone('UTC'));
+
+        $shouldRun = false;
+        if ($previousRun === null) {
+            // In case there is no previous run we just skip the cron job
+            // This can happen on re-deployments, and we don't want a cron-storm there, or jobs that run
+            // way off their schedule
+            $shouldRun = false;
+        } elseif ($previousExpectedRun->getTimestamp() > $previousRun->getTimestamp() && $previousExpectedRun->getTimestamp() <= $currentRun->getTimestamp()) {
+            // If we were scheduled to run between now and right the previous run then we should run
+            // XXX: We compare the timestamps, since that is what we use to serialize the last execution time (so we get the same rounding)
+            $shouldRun = true;
+        }
+
+        return $shouldRun;
+    }
+
+    /**
+     * Returns the date and time the job is scheduled to run the next time.
+     */
+    public static function getNextDate(CronJobInterface $job, \DateTimeInterface $currentTime): \DateTimeInterface
+    {
+        $cronExpression = $job->getInterval();
+        $cron = new CronExpression($cronExpression);
+        $nextDate = $cron->getNextRunDate($currentTime, 0, true);
+        $nextDate->setTimezone(new \DateTimeZone('UTC'));
+
+        return \DateTimeImmutable::createFromMutable($nextDate);
+    }
+
+    /**
+     * Returns the last time cron was executed.
+     */
+    public function getLastExecutionDate(): ?\DateTimeInterface
+    {
+        $cachePool = $this->cachePool;
+        assert($cachePool instanceof CacheItemPoolInterface);
+        $item = $cachePool->getItem('cron-previous-run');
+        $value = $item->get();
+        $previousRun = null;
+        if ($value !== null) {
+            $previousRun = (new \DateTimeImmutable())->setTimezone(new \DateTimeZone('UTC'))->setTimestamp($value);
+        }
+
+        return $previousRun;
+    }
+
+    /**
+     * Stores the given time as the new last cron execution time.
+     */
+    public function setLastExecutionDate(\DateTimeInterface $currentTime): void
+    {
+        $cachePool = $this->cachePool;
+        assert($cachePool instanceof CacheItemPoolInterface);
+        $item = $cachePool->getItem('cron-previous-run');
+        $item->set($currentTime->getTimestamp());
+        if ($cachePool->save($item) === false) {
+            throw new \RuntimeException('Saving cron timestamp failed');
+        }
+    }
+
+    /**
+     * @return CronJobInterface[]
+     */
+    public function getJobs(): array
+    {
+        return $this->jobs;
+    }
+
+    public function runDueJobs(bool $force = false, \DateTimeInterface $currentTime = null)
+    {
+        // Get all jobs that should have been run between the last time we were called and now
+        if ($currentTime === null) {
+            $currentTime = new \DateTimeImmutable('now', new \DateTimeZone('UTC'));
+        }
+        $lastDate = $this->getLastExecutionDate();
+        $this->setLastExecutionDate($currentTime);
+
+        if ($lastDate === null) {
+            $this->logger->info('cron: No last execution time available, will no run anything');
+        }
+
+        $toRun = [];
+        foreach ($this->jobs as $job) {
+            $interval = $job->getInterval();
+            $name = $job->getName();
+            $this->logger->info("cron: Checking '$name' ($interval)");
+            $isDue = self::isDue($job, $lastDate, $currentTime);
+            if ($isDue || $force) {
+                $toRun[] = $job;
+            }
+        }
+
+        if (count($toRun) === 0) {
+            $this->logger->info('cron: No jobs to run');
+        }
+
+        foreach ($toRun as $job) {
+            $name = $job->getName();
+            $this->logger->info("cron: Running '$name'");
+            try {
+                $job->run(new CronOptions());
+            } catch (\Throwable $e) {
+                $this->logger->error("cron: '$name' failed", ['exception' => $e]);
+            }
+        }
+    }
+}

src/DependencyInjection/DbpRelayCoreExtension.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace Dbp\Relay\CoreBundle\DependencyInjection;
 
 use Dbp\Relay\CoreBundle\Auth\ProxyAuthenticator;
+use Dbp\Relay\CoreBundle\Cron\CronManager;
 use Dbp\Relay\CoreBundle\DB\MigrateCommand;
 use Dbp\Relay\CoreBundle\Queue\TestMessage;
 use Dbp\Relay\CoreBundle\Queue\Utils as QueueUtils;
@@ -36,7 +37,7 @@ class DbpRelayCoreExtension extends ConfigurableExtension implements PrependExte
         $cronCacheDef->setArguments(['core-cron', 0, '%kernel.cache_dir%/dbp/relay/core-cron']);
         $cronCacheDef->addTag('cache.pool');
 
-        $definition = $container->getDefinition('Dbp\Relay\CoreBundle\Cron\CronCommand');
+        $definition = $container->getDefinition(CronManager::class);
         $definition->addMethodCall('setCache', [$cronCacheDef]);
 
         $definition = $container->getDefinition(MigrateCommand::class);

src/Resources/config/services.yaml

@@ -14,6 +14,14 @@ services:
     autowire: true
     autoconfigure: true
 
+  Dbp\Relay\CoreBundle\Cron\CronListCommand:
+    autowire: true
+    autoconfigure: true
+
+  Dbp\Relay\CoreBundle\Cron\CronManager:
+    autowire: true
+    autoconfigure: true
+
   Dbp\Relay\CoreBundle\Cron\CronJobs\:
     resource: '../../Cron/CronJobs'
     autowire: true

tests/Cron/CronJob.php

+<?php
+
+declare(strict_types=1);
+
+namespace Dbp\Relay\CoreBundle\Tests\Cron;
+
+use Dbp\Relay\CoreBundle\Cron\CronJobInterface;
+use Dbp\Relay\CoreBundle\Cron\CronOptions;
+
+class CronJob implements CronJobInterface
+{
+    /**
+     * @var string
+     */
+    private $name;
+
+    /**
+     * @var string
+     */
+    private $interval;
+
+    /**
+     * @var bool
+     */
+    public $ran;
+
+    public function __construct(string $interval, string $name = '')
+    {
+        $this->name = $name;
+        $this->interval = $interval;
+        $this->ran = false;
+    }
+
+    public function getName(): string
+    {
+        return $this->name;
+    }
+
+    public function getInterval(): string
+    {
+        return $this->interval;
+    }
+
+    public function run(CronOptions $options): void
+    {
+        $this->ran = true;
+    }
+}

tests/Cron/CronTest.php

@@ -5,19 +5,86 @@ declare(strict_types=1);
 namespace Dbp\Relay\CoreBundle\Tests\Cron;
 
 use Dbp\Relay\CoreBundle\Cron\CronCommand;
+use Dbp\Relay\CoreBundle\Cron\CronListCommand;
+use Dbp\Relay\CoreBundle\Cron\CronManager;
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\Cache\Adapter\ArrayAdapter;
+use Symfony\Component\Console\Input\ArrayInput;
+use Symfony\Component\Console\Output\BufferedOutput;
 
 class CronTest extends TestCase
 {
-    public function testCronisDue()
+    public function testCronIsDue()
     {
-        $isDue = CronCommand::isDue(new \DateTimeImmutable('2021-09-07T09:36:26Z'), new \DateTimeImmutable('2021-09-07T09:36:26Z'), '* * * * *');
+        $isDue = CronManager::isDue(new CronJob('* * * * *'), new \DateTimeImmutable('2021-09-07T09:36:26Z'), new \DateTimeImmutable('2021-09-07T09:36:26Z'));
         $this->assertFalse($isDue);
-        $isDue = CronCommand::isDue(new \DateTimeImmutable('2021-09-07T09:35:59Z'), new \DateTimeImmutable('2021-09-07T09:36:00Z'), '* * * * *');
-        $this->assertTrue($isDue);
-        $isDue = CronCommand::isDue(null, new \DateTimeImmutable('2021-09-07T09:36:00Z'), '0 0 1 1 *');
-        $this->assertTrue($isDue);
-        $isDue = CronCommand::isDue(null, new \DateTimeImmutable('2021-09-07T09:36:00Z'), '* * * * *');
+        $isDue = CronManager::isDue(new CronJob('* * * * *'), new \DateTimeImmutable('2021-09-07T09:35:59Z'), new \DateTimeImmutable('2021-09-07T09:36:00Z'));
         $this->assertTrue($isDue);
+        $isDue = CronManager::isDue(new CronJob('0 0 1 1 *'), null, new \DateTimeImmutable('2021-09-07T09:36:00Z'));
+        $this->assertFalse($isDue);
+        $isDue = CronManager::isDue(new CronJob('* * * * *'), null, new \DateTimeImmutable('2021-09-07T09:36:00Z'));
+        $this->assertFalse($isDue);
+    }
+
+    public function testExecutionDate()
+    {
+        $man = new CronManager();
+        $man->setCache(new ArrayAdapter());
+        $this->assertNull($man->getLastExecutionDate());
+        $date = (new \DateTimeImmutable())->setTimestamp(42);
+        $man->setLastExecutionDate($date);
+        $this->assertSame(42, $man->getLastExecutionDate()->getTimestamp());
+    }
+
+    public function testGetNextDate()
+    {
+        $current = (new \DateTimeImmutable())->setTimestamp(1676383110);
+        $next = CronManager::getNextDate(new CronJob('*/5 * * * *'), $current);
+        $this->assertSame(1676383200, $next->getTimestamp());
+    }
+
+    public function testManager()
+    {
+        $last = (new \DateTimeImmutable())->setTimestamp(1676383110);
+        $current = $last->setTimestamp($last->getTimestamp() + 30);
+        $man = new CronManager();
+        $man->setCache(new ArrayAdapter());
+        $man->setLastExecutionDate($last);
+        $job = new CronJob('* * * * *');
+
+        // add a job
+        $man->addJob($job);
+        $this->assertSame($job, $man->getJobs()[0]);
+
+        // run right away
+        $man->runDueJobs(false, $last);
+        $this->assertFalse($job->ran);
+
+        // now a bit later, the job gets run
+        $man->runDueJobs(false, $current);
+        $this->assertTrue($job->ran);
+
+        // now right away again, but force
+        $job->ran = false;
+        $man->runDueJobs(false, $current);
+        $this->assertFalse($job->ran);
+        $man->runDueJobs(true, $current);
+        $this->assertTrue($job->ran);
+    }
+
+    public function testCommands()
+    {
+        $man = new CronManager();
+        $job = new CronJob('* * * * *');
+        $man->addJob($job);
+
+        // cron command
+        new CronCommand($man);
+
+        // list command
+        $input = new ArrayInput([]);
+        $output = new BufferedOutput();
+        $cmd = new CronListCommand($man);
+        $this->assertSame(0, $cmd->run($input, $output));
     }
 }