Not schedule job update if already queued

okvpn · Jan 9, 2023 · 134dde0 · 134dde0
1 parent 0f631cc
commit 134dde0
Show file tree

Hide file tree

Showing 5 changed files with 8 additions and 26 deletions.
diff --git a/config/packages/okvpn.yaml b/config/packages/okvpn.yaml
@@ -5,8 +5,7 @@ okvpn_cron:
             cron:    '0 0 * * *'
         -
             command: 'packagist:update'
-            cron:    '9,19,29,38,48,58 * * * *' # avoid running things at midnight etc
-#            cron: '*/1 * * * *'
+            cron:    '*/15 * * * *'
             arguments: {'--update-crawl-interval': '%env(int:PACKAGE_UPDATE_INTERVAL)%' }
 
     default_policy:

diff --git a/src/Command/UpdatePackagesCommand.php b/src/Command/UpdatePackagesCommand.php
@@ -100,7 +100,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             $idsGroup = array_splice($ids, 0, 100);
 
             foreach ($idsGroup as $id) {
-                $job = $this->scheduler->scheduleUpdate($id, $updateEqualRefs, $deleteBefore, $randomTimes ? new \DateTime('+'.rand(1, (int) ($interval/1.5)).'seconds') : null);
+                $job = $this->scheduler->scheduleUpdate($id, $updateEqualRefs, $deleteBefore, $randomTimes ? new \DateTime('+'.rand(1, (int) ($interval/1.5)).'seconds') : null, true);
                 if ($verbose) {
                     $output->writeln('Scheduled update job '.$job->getId().' for package '.$id);
                 }

diff --git a/src/Controller/PackageController.php b/src/Controller/PackageController.php
@@ -593,7 +593,7 @@ public function updatePackageAction(Request $req, $name)
 
         $update = $req->request->get('update', $req->query->get('update'));
         $autoUpdated = $req->request->get('autoUpdated', $req->query->get('autoUpdated'));
-        $updateEqualRefs = $req->request->get('updateAll', $req->query->get('updateAll'));
+        $updateEqualRefs = (bool)$req->request->get('updateAll', $req->query->get('updateAll'));
 
         $user = $this->getUser() ?: $doctrine
             ->getRepository(User::class)

diff --git a/src/Entity/Job.php b/src/Entity/Job.php
@@ -185,12 +185,12 @@ public function setExecuteAfter(DateTimeInterface $executeAfter)
         $this->executeAfter = $executeAfter;
     }
 
-    public function getExecuteAfter()
+    public function getExecuteAfter(): ?DateTimeInterface
     {
         return $this->executeAfter;
     }
 
-    public function getCompletedAt()
+    public function getCompletedAt(): ?DateTimeInterface
     {
         return $this->completedAt;
     }

diff --git a/src/Service/Scheduler.php b/src/Service/Scheduler.php
@@ -18,15 +18,7 @@ public function __construct(\Redis $redis, ManagerRegistry $doctrine)
         $this->redis = $redis;
     }
 
-    /**
-     * @param $packageOrId
-     * @param bool $updateEqualRefs
-     * @param bool $deleteBefore
-     * @param null $executeAfter
-     *
-     * @return Job|object
-     */
-    public function scheduleUpdate($packageOrId, $updateEqualRefs = false, $deleteBefore = false, $executeAfter = null): Job
+    public function scheduleUpdate($packageOrId, bool $updateEqualRefs = false, bool $deleteBefore = false, \DateTimeInterface $executeAfter = null): Job
     {
         if ($packageOrId instanceof Package) {
             $packageOrId = $packageOrId->getId();
@@ -36,18 +28,9 @@ public function scheduleUpdate($packageOrId, $updateEqualRefs = false, $deleteBe
 
         $pendingJobId = $this->getPendingUpdateJob($packageOrId, $updateEqualRefs, $deleteBefore);
         if ($pendingJobId) {
-            $pendingJob = $this->doctrine->getManager()->getRepository(Job::class)->findOneBy(['id' => $pendingJobId]);
-
-            // pending job will execute before the one we are trying to schedule so skip scheduling
-            if (
-                (!$pendingJob->getExecuteAfter() && $executeAfter)
-                || ($pendingJob->getExecuteAfter() && $executeAfter && $pendingJob->getExecuteAfter() < $executeAfter)
-            ) {
-                return $pendingJob;
-            }
+            $pendingJob = $this->doctrine->getRepository(Job::class)->findOneBy(['id' => $pendingJobId]);
 
-            // neither job has executeAfter, so the pending one is equivalent to the one we are trying to schedule and we can skip scheduling
-            if (!$pendingJob->getExecuteAfter() && !$executeAfter) {
+            if (!$pendingJob->getExecuteAfter() || $executeAfter) {
                 return $pendingJob;
             }