Move SQL queries from SimpleWorker to model, improve naming

[MAILPOET-2538]

lib/Cron/Workers/Scheduler.php

@@ -248,7 +248,7 @@ class Scheduler {
   }
 
   private function reScheduleBounceTask() {
-    $bounce_tasks = Bounce::getScheduledTasks($future = true);
+    $bounce_tasks = ScheduledTask::findFutureScheduledByType(Bounce::TASK_TYPE);
     if (count($bounce_tasks)) {
       $bounce_task = reset($bounce_tasks);
       if (Carbon::createFromTimestamp(current_time('timestamp'))->addHour(42)->lessThan($bounce_task->scheduled_at)) {

lib/Cron/Workers/SimpleWorker.php

@@ -47,7 +47,7 @@ abstract class SimpleWorker {
 
     $this->init();
 
-    $scheduled_tasks = self::getScheduledTasks();
+    $scheduled_tasks = self::getDueTasks();
     $running_tasks = self::getRunningTasks();
 
     if (!$scheduled_tasks && !$running_tasks) {
@@ -192,42 +192,15 @@ abstract class SimpleWorker {
     return $date;
   }
 
-  /**
-   * @param bool $future
-   * @return ScheduledTask[]
-   */
-  static function getScheduledTasks($future = false) {
-    $dateWhere = ($future) ? 'whereGt' : 'whereLte';
-    $wp = new WPFunctions();
-    return ScheduledTask::where('type', static::TASK_TYPE)
-      ->$dateWhere('scheduled_at', Carbon::createFromTimestamp($wp->currentTime('timestamp')))
-      ->whereNull('deleted_at')
-      ->where('status', ScheduledTask::STATUS_SCHEDULED)
-      ->limit(self::TASK_BATCH_SIZE)
-      ->findMany();
+  static function getDueTasks() {
+    return ScheduledTask::findDueByType(static::TASK_TYPE, self::TASK_BATCH_SIZE);
   }
 
   static function getRunningTasks() {
-    $wp = new WPFunctions();
-    return ScheduledTask::where('type', static::TASK_TYPE)
-      ->whereLte('scheduled_at', Carbon::createFromTimestamp($wp->currentTime('timestamp')))
-      ->whereNull('deleted_at')
-      ->whereNull('status')
-      ->limit(self::TASK_BATCH_SIZE)
-      ->findMany();
-  }
-
-  static function getDueTasks() {
-    $scheduled_tasks = self::getScheduledTasks();
-    $running_tasks = self::getRunningTasks();
-    return array_merge((array)$scheduled_tasks, (array)$running_tasks);
+    return ScheduledTask::findRunningByType(static::TASK_TYPE, self::TASK_BATCH_SIZE);
   }
 
   static function getCompletedTasks() {
-    return ScheduledTask::where('type', static::TASK_TYPE)
-      ->whereNull('deleted_at')
-      ->where('status', ScheduledTask::STATUS_COMPLETED)
-      ->limit(self::TASK_BATCH_SIZE)
-      ->findMany();
+    return ScheduledTask::findCompletedByType(static::TASK_TYPE, self::TASK_BATCH_SIZE);
   }
 }

lib/Models/ScheduledTask.php

@@ -160,4 +160,44 @@ class ScheduledTask extends Model {
       ->whereNull('tasks.deleted_at')
       ->findOne() ?: null;
   }
+
+  static function findDueByType($type, $limit = null) {
+    return self::findByTypeAndStatus($type, ScheduledTask::STATUS_SCHEDULED, $limit);
+  }
+
+  static function findRunningByType($type, $limit = null) {
+    return self::findByTypeAndStatus($type, null, $limit);
+  }
+
+  static function findFutureScheduledByType($type, $limit = null) {
+    return self::findByTypeAndStatus($type, ScheduledTask::STATUS_SCHEDULED, $limit, true);
+  }
+
+  static function findCompletedByType($type, $limit = null) {
+    return self::findByTypeAndStatus($type, ScheduledTask::STATUS_COMPLETED, $limit);
+  }
+
+  private static function findByTypeAndStatus($type, $status, $limit = null, $future = false) {
+    $query = ScheduledTask::where('type', $type)
+      ->whereNull('deleted_at');
+
+    $now = Carbon::createFromTimestamp(WPFunctions::get()->currentTime('timestamp'));
+    if ($future) {
+      $query->whereGt('scheduled_at', $now);
+    } else {
+      $query->whereLte('scheduled_at', $now);
+    }
+
+    if ($status === null) {
+      $query->whereNull('status');
+    } else {
+      $query->where('status', $status);
+    }
+
+    if ($limit !== null) {
+      $query->limit($limit);
+    }
+
+    return $query->findMany();
+  }
 }

tests/integration/Cron/Workers/SimpleWorkerTest.php

@@ -65,16 +65,16 @@ class SimpleWorkerTest extends \MailPoetTest {
   }
 
   function testItCanGetScheduledTasks() {
-    expect(MockSimpleWorker::getScheduledTasks())->isEmpty();
+    expect(MockSimpleWorker::getDueTasks())->isEmpty();
     $this->createScheduledTask();
-    expect(MockSimpleWorker::getScheduledTasks())->notEmpty();
+    expect(MockSimpleWorker::getDueTasks())->notEmpty();
   }
 
   function testItCanGetABatchOfScheduledTasks() {
     for ($i = 0; $i < MockSimpleWorker::TASK_BATCH_SIZE + 5; $i += 1) {
       $this->createScheduledTask();
     }
-    expect(count(MockSimpleWorker::getScheduledTasks()))->equals(MockSimpleWorker::TASK_BATCH_SIZE);
+    expect(count(MockSimpleWorker::getDueTasks()))->equals(MockSimpleWorker::TASK_BATCH_SIZE);
   }
 
   function testItCanGetRunningTasks() {
@@ -97,28 +97,6 @@ class SimpleWorkerTest extends \MailPoetTest {
     expect(count(MockSimpleWorker::getCompletedTasks()))->equals(MockSimpleWorker::TASK_BATCH_SIZE);
   }
 
-  function testItCanGetDueTasks() {
-    expect(MockSimpleWorker::getDueTasks())->isEmpty();
-
-    // scheduled for now
-    $this->createScheduledTask();
-
-    // running
-    $this->createRunningTask();
-
-    // scheduled in the future (should not be retrieved)
-    $task = $this->createScheduledTask();
-    $task->scheduled_at = Carbon::createFromTimestamp(current_time('timestamp'))->addDays(7);
-    $task->save();
-
-    // completed (should not be retrieved)
-    $task = $this->createRunningTask();
-    $task->status = ScheduledTask::STATUS_COMPLETED;
-    $task->save();
-
-    expect(count(MockSimpleWorker::getDueTasks()))->equals(2);
-  }
-
   function testItFailsToProcessWithoutTasks() {
     expect($this->worker->process())->false();
   }