From d7337e9e1b48a27965623b7764a4df057978bd1e Mon Sep 17 00:00:00 2001
From: Dennis Eichhorn <spl1nes.com@googlemail.com>
Date: Sat, 4 Aug 2018 13:20:57 +0200
Subject: [PATCH] Fix schedule/cron implementation

---
 Utils/TaskSchedule/Cron.php                   | 97 ++++++++++++-------
 Utils/TaskSchedule/CronJob.php                |  5 +-
 Utils/TaskSchedule/NullCronJob.php            | 27 ++++++
 Utils/TaskSchedule/NullSchedule.php           | 27 ++++++
 Utils/TaskSchedule/Schedule.php               |  3 +-
 Utils/TaskSchedule/SchedulerAbstract.php      | 13 +++
 Utils/TaskSchedule/TaskAbstract.php           | 21 ++--
 Utils/TaskSchedule/TaskScheduler.php          | 10 +-
 tests/Utils/TaskSchedule/CronTest.php         | 31 +++---
 .../TaskSchedule/SchedulerAbstractTest.php    |  2 +-
 tests/Utils/TaskSchedule/TaskAbstractTest.php |  9 +-
 .../Utils/TaskSchedule/TaskSchedulerTest.php  | 34 ++++---
 12 files changed, 197 insertions(+), 82 deletions(-)
 create mode 100644 Utils/TaskSchedule/NullCronJob.php
 create mode 100644 Utils/TaskSchedule/NullSchedule.php

diff --git a/Utils/TaskSchedule/Cron.php b/Utils/TaskSchedule/Cron.php
index 424166596..7b6d2ca56 100644
--- a/Utils/TaskSchedule/Cron.php
+++ b/Utils/TaskSchedule/Cron.php
@@ -30,9 +30,10 @@ class Cron extends SchedulerAbstract
      */
     public function create(TaskAbstract $task) : void
     {
-        $this->run('-l > tmpcron');
-        \file_put_contents('tmpcron', "\n" . $task->__toString(), FILE_APPEND);
-        $this->run('tmpcron');
+        $this->run('-l > ' . __DIR__ . '/tmpcron.tmp');
+        \file_put_contents(__DIR__ . '/tmpcron.tmp', $task->__toString() . "\n", FILE_APPEND);
+        $this->run(__DIR__ . '/tmpcron.tmp');
+        unlink(__DIR__ . '/tmpcron.tmp');
     }
 
     /**
@@ -40,16 +41,16 @@ class Cron extends SchedulerAbstract
      */
     public function update(TaskAbstract $task) : void
     {
-        $this->run('-l > tmpcron');
+        $this->run('-l > ' . __DIR__ . '/tmpcron.tmp');
 
         $new = '';
-        $fp  = \fopen('tmpcron', 'r+');
+        $fp  = \fopen(__DIR__ . '/tmpcron.tmp', 'r+');
 
         if ($fp) {
             $line = \fgets($fp);
             while ($line !== false) {
-                if ($line[0] !== '#' && \stripos($line, '/tn = ' . $task->getId()) !== false) {
-                    $new .= $task->__toString();
+                if ($line[0] !== '#' && \stripos($line, 'name="' . $task->getId()) !== false) {
+                    $new .= $task->__toString() . "\n";
                 } else {
                     $new .= $line . "\n";
                 }
@@ -57,11 +58,12 @@ class Cron extends SchedulerAbstract
                 $line = \fgets($fp);
             }
 
-            \fwrite($fp, $new);
             \fclose($fp);
+            \file_put_contents(__DIR__ . '/tmpcron.tmp', $new);
         }
 
-        $this->run('tmpcron');
+        $this->run(__DIR__ . '/tmpcron.tmp');
+        unlink(__DIR__ . '/tmpcron.tmp');
     }
 
     /**
@@ -69,15 +71,23 @@ class Cron extends SchedulerAbstract
      */
     public function delete(TaskAbstract $task) : void
     {
-        $this->run('-l > tmpcron');
+        $this->deleteByName($task->getId());
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function deleteByName(string $name) : void
+    {
+        $this->run('-l > ' . __DIR__ . '/tmpcron.tmp');
 
         $new = '';
-        $fp  = \fopen('tmpcron', 'r+');
+        $fp  = \fopen(__DIR__ . '/tmpcron.tmp', 'r+');
 
         if ($fp) {
             $line = \fgets($fp);
             while ($line !== false) {
-                if ($line[0] !== '#' && \stripos($line, '/tn = ' . $task->getId()) !== false) {
+                if ($line[0] !== '#' && \stripos($line, 'name="' . $name) !== false) {
                     $line = \fgets($fp);
                     continue;
                 }
@@ -86,11 +96,12 @@ class Cron extends SchedulerAbstract
                 $line = \fgets($fp);
             }
 
-            \fwrite($fp, $new);
             \fclose($fp);
+            \file_put_contents(__DIR__ . '/tmpcron.tmp', $new);
         }
 
-        $this->run('tmpcron');
+        $this->run(__DIR__ . '/tmpcron.tmp');
+        unlink(__DIR__ . '/tmpcron.tmp');
     }
 
     /**
@@ -98,16 +109,31 @@ class Cron extends SchedulerAbstract
      */
     public function getAll() : array
     {
-        $lines = \explode("\n", $this->normalize($this->run('-l')));
-        unset($lines[0]);
+        $this->run('-l > ' . __DIR__ . '/tmpcron.tmp');
 
         $jobs = [];
-        foreach ($lines as $line) {
-            if ($line !== '' && \strrpos($line, '#', -\strlen($line)) === false) {
-                $jobs[] = CronJob::createWith(\str_getcsv($line, ' '));
+        $fp   = \fopen(__DIR__ . '/tmpcron.tmp', 'r+');
+
+        if ($fp) {
+            $line = \fgets($fp);
+            while ($line !== false) {
+                if ($line[0] !== '#') {
+                    $elements   = [];
+                    $namePos    = \stripos($line, 'name="');
+                    $elements[] = \substr($line, $namePos + 6, \stripos($line, '"', $namePos + 7) - 1);
+                    $elements  += \explode(' ', $line);
+                    $jobs[]     = CronJob::createWith($elements);
+                }
+
+                $line = \fgets($fp);
             }
+
+            \fclose($fp);
         }
 
+        $this->run(__DIR__ . '/tmpcron.tmp');
+        unlink(__DIR__ . '/tmpcron.tmp');
+
         return $jobs;
     }
 
@@ -116,29 +142,30 @@ class Cron extends SchedulerAbstract
      */
     public function getAllByName(string $name, bool $exact = true) : array
     {
-        $lines = \explode("\n", $this->normalize($this->run('-l')));
-        unset($lines[0]);
+        $this->run('-l > ' . __DIR__ . '/tmpcron.tmp');
 
-        if ($exact) {
-            $jobs = [];
-            foreach ($lines as $line) {
-                $csv = \str_getcsv($line, ' ');
+        $jobs = [];
+        $fp   = \fopen(__DIR__ . '/tmpcron.tmp', 'r+');
 
-                if ($line !== '' && \strrpos($line, '#', -\strlen($line)) === false && $csv[5] === $name) {
-                    $jobs[] = CronJob::createWith($csv);
+        if ($fp) {
+            $line = \fgets($fp);
+            while ($line !== false) {
+                if ($line[0] !== '#' && \stripos($line, '# name="' . $name) !== false) {
+                    $elements   = [];
+                    $elements[] = $name;
+                    $elements  += \explode(' ', $line);
+                    $jobs[]     = CronJob::createWith($elements);
                 }
-            }
-        } else {
-            $jobs = [];
-            foreach ($lines as $line) {
-                $csv = \str_getcsv($line, ' ');
 
-                if ($line !== '' && \strrpos($line, '#', -\strlen($line)) === false && \stripos($csv[5], $name) !== false) {
-                    $jobs[] = CronJob::createWith($csv);
-                }
+                $line = \fgets($fp);
             }
+
+            \fclose($fp);
         }
 
+        $this->run(__DIR__ . '/tmpcron.tmp');
+        unlink(__DIR__ . '/tmpcron.tmp');
+
         return $jobs;
     }
 }
diff --git a/Utils/TaskSchedule/CronJob.php b/Utils/TaskSchedule/CronJob.php
index 8a825b058..be874ef24 100644
--- a/Utils/TaskSchedule/CronJob.php
+++ b/Utils/TaskSchedule/CronJob.php
@@ -31,7 +31,7 @@ class CronJob extends TaskAbstract
      */
     public function __toString() : string
     {
-        return '';
+        return $this->interval . ' ' . $this->command . ' # name="' . $this->id . '" ' . $this->comment;
     }
     
     /**
@@ -39,7 +39,8 @@ class CronJob extends TaskAbstract
      */
     public static function createWith(array $jobData) : TaskAbstract
     {
-        $job = new self($jobData[1], '');
+        $interval = \array_splice($jobData, 1, 4);
+        $job      = new self($jobData[0], $jobData[1], \implode(' ', $interval));
 
         return $job;
     }
diff --git a/Utils/TaskSchedule/NullCronJob.php b/Utils/TaskSchedule/NullCronJob.php
new file mode 100644
index 000000000..03928ac29
--- /dev/null
+++ b/Utils/TaskSchedule/NullCronJob.php
@@ -0,0 +1,27 @@
+<?php
+/**
+ * Orange Management
+ *
+ * PHP Version 7.2
+ *
+ * @package    TBD
+ * @copyright  Dennis Eichhorn
+ * @license    OMS License 1.0
+ * @version    1.0.0
+ * @link       http://website.orange-management.de
+ */
+declare(strict_types=1);
+
+namespace phpOMS\Utils\TaskSchedule;
+
+/**
+ * CronJob class.
+ *
+ * @package    Framework
+ * @license    OMS License 1.0
+ * @link       http://website.orange-management.de
+ * @since      1.0.0
+ */
+class NullCronJob extends CronJob
+{
+}
diff --git a/Utils/TaskSchedule/NullSchedule.php b/Utils/TaskSchedule/NullSchedule.php
new file mode 100644
index 000000000..7ef06864c
--- /dev/null
+++ b/Utils/TaskSchedule/NullSchedule.php
@@ -0,0 +1,27 @@
+<?php
+/**
+ * Orange Management
+ *
+ * PHP Version 7.2
+ *
+ * @package    TBD
+ * @copyright  Dennis Eichhorn
+ * @license    OMS License 1.0
+ * @version    1.0.0
+ * @link       http://website.orange-management.de
+ */
+declare(strict_types=1);
+
+namespace phpOMS\Utils\TaskSchedule;
+
+/**
+ * CronJob class.
+ *
+ * @package    Framework
+ * @license    OMS License 1.0
+ * @link       http://website.orange-management.de
+ * @since      1.0.0
+ */
+class NullSchedule extends Schedule
+{
+}
diff --git a/Utils/TaskSchedule/Schedule.php b/Utils/TaskSchedule/Schedule.php
index c98469305..88f7d2618 100644
--- a/Utils/TaskSchedule/Schedule.php
+++ b/Utils/TaskSchedule/Schedule.php
@@ -39,9 +39,8 @@ class Schedule extends TaskAbstract
      */
     public static function createWith(array $jobData) : TaskAbstract
     {
-        $job = new self($jobData[1], '');
+        $job = new self($jobData[1], $jobData[8]);
 
-        $job->setRun($jobData[8]);
         $job->setStatus($jobData[3]);
 
         if (DateTime::isValid($jobData[2])) {
diff --git a/Utils/TaskSchedule/SchedulerAbstract.php b/Utils/TaskSchedule/SchedulerAbstract.php
index 4873fd18e..86bd2b094 100644
--- a/Utils/TaskSchedule/SchedulerAbstract.php
+++ b/Utils/TaskSchedule/SchedulerAbstract.php
@@ -83,6 +83,8 @@ abstract class SchedulerAbstract
             'e:/WINDOWS/system32/schtasks.exe',
             'f:/WINDOWS/system32/schtasks.exe',
             '/usr/bin/crontab',
+            '/usr/local/bin/crontab',
+            '/usr/local/sbin/crontab',
             '/usr/sbin/crontab',
             '/bin/crontab',
             '/sbin/crontab',
@@ -190,6 +192,17 @@ abstract class SchedulerAbstract
      */
     abstract public function update(TaskAbstract $task) : void;
 
+    /**
+     * Delete task by name
+     *
+     * @param string $name Task name
+     *
+     * @return void
+     *
+     * @since  1.0.0
+     */
+    abstract public function deleteByName(string $name) : void;
+
     /**
      * Delete task
      *
diff --git a/Utils/TaskSchedule/TaskAbstract.php b/Utils/TaskSchedule/TaskAbstract.php
index b809b4878..ca8607a93 100644
--- a/Utils/TaskSchedule/TaskAbstract.php
+++ b/Utils/TaskSchedule/TaskAbstract.php
@@ -41,12 +41,12 @@ abstract class TaskAbstract
     protected $command = '';
 
     /**
-     * Command/script to run.
+     * Run interval
      *
      * @var string
      * @since 1.0.0
      */
-    protected $run = '';
+    protected $interval = '';
 
     /**
      * Status of the task
@@ -91,10 +91,11 @@ abstract class TaskAbstract
      *
      * @since  1.0.0
      */
-    public function __construct(string $name, string $cmd = '')
+    public function __construct(string $name, string $cmd = '', string $interval = '')
     {
         $this->id          = $name;
         $this->command     = $cmd;
+        $this->interval    = $interval;
         $this->lastRunTime = new \DateTime('1900-01-01');
         $this->nextRunTime = new \DateTime('1900-01-01');
     }
@@ -147,29 +148,29 @@ abstract class TaskAbstract
     }
 
     /**
-     * Get command/script to run
+     * Get interval to create the task
      *
      * @return string
      *
      * @since  1.0.0
      */
-    public function getRun() : string
+    public function getInterval() : string
     {
-        return $this->run;
+        return $this->interval;
     }
 
     /**
-     * Set script to run
+     * Set interval to create the task
      *
-     * @param string $run Command/script to run
+     * @param string $interval Interval
      *
      * @return void
      *
      * @since  1.0.0
      */
-    public function setRun(string $run) : void
+    public function setInterval(string $interval) : void
     {
-        $this->run = $run;
+        $this->interval = $interval;
     }
 
     /**
diff --git a/Utils/TaskSchedule/TaskScheduler.php b/Utils/TaskSchedule/TaskScheduler.php
index 4248d738a..3f88a392e 100644
--- a/Utils/TaskSchedule/TaskScheduler.php
+++ b/Utils/TaskSchedule/TaskScheduler.php
@@ -41,12 +41,20 @@ class TaskScheduler extends SchedulerAbstract
         $this->run('/Change ' . $task->__toString());
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function deleteByName(string $name) : void
+    {
+        $this->run('/Delete /TN ' . $name);
+    }
+
     /**
      * {@inheritdoc}
      */
     public function delete(TaskAbstract $task) : void
     {
-        $this->run('/Delete /TN ' . $task->getId());
+        $this->deleteByName($task->getId());
     }
 
     /**
diff --git a/tests/Utils/TaskSchedule/CronTest.php b/tests/Utils/TaskSchedule/CronTest.php
index 7e2dc5c1c..8cf1b8078 100644
--- a/tests/Utils/TaskSchedule/CronTest.php
+++ b/tests/Utils/TaskSchedule/CronTest.php
@@ -26,24 +26,29 @@ class CronTest extends \PHPUnit\Framework\TestCase
     public function testCRUD()
     {
         if (\stristr(PHP_OS, 'LINUX')) {
+            Cron::guessBin();
             $cron = new Cron();
 
-            self::assertInstanceOf('\phpOMS\Utils\TaskSchedule\NullCronJob', $cron->getAllByName('testCronJob', false));
+            self::assertEquals([], $cron->getAllByName('testCronJob', false));
             
-            $cron->create(
-                new CronJob('testCronJob', 'testFile')
-            );
-            self::assertEquals('testFile', $cron->getRun());
+            $job = new CronJob('testCronJob', 'testFile', '0 0 1 1 *');
+            $cron->create($job);
+            
+            self::assertTrue(!empty($cron->getAllByName('testCronJob', false)));
+            if (!empty($cron->getAllByName('testCronJob', false))) {
+                self::assertEquals('testFile', $cron->getAllByName('testCronJob', false)[0]->getCommand());
+            }
 
-            $cron->update(
-                new CronJob('testCronJob', 'testFile2')
-            );
-            self::assertEquals('testFile2', $cron->getRun());
+            $job->setCommand('testFile2');
+            $cron->update($job);
 
-            $cron->delete(
-                new CronJob('testCronJob', 'testFile2')
-            );
-            self::assertInstanceOf('\phpOMS\Utils\TaskSchedule\NullCronJob', $cron->getAllByName('testCronJob', false));
+            self::assertTrue(!empty($cron->getAllByName('testCronJob', false)));
+            if (!empty($cron->getAllByName('testCronJob', false))) {
+                self::assertEquals('testFile2', $cron->getAllByName('testCronJob', false)[0]->getCommand());
+            }
+
+            $cron->delete($job);
+            self::assertEquals([], $cron->getAllByName('testCronJob', false));
         }
     }
 }
diff --git a/tests/Utils/TaskSchedule/SchedulerAbstractTest.php b/tests/Utils/TaskSchedule/SchedulerAbstractTest.php
index 0462f84cf..f3cacd58a 100644
--- a/tests/Utils/TaskSchedule/SchedulerAbstractTest.php
+++ b/tests/Utils/TaskSchedule/SchedulerAbstractTest.php
@@ -19,6 +19,6 @@ class SchedulerAbstractTest extends \PHPUnit\Framework\TestCase
 {
     public function testDefault()
     {
-        self::assertEquals('', SchedulerAbstract::getBin());
+        self::assertTrue(SchedulerAbstract::getBin() === '' || \file_exists(SchedulerAbstract::getBin() ));
     }
 }
diff --git a/tests/Utils/TaskSchedule/TaskAbstractTest.php b/tests/Utils/TaskSchedule/TaskAbstractTest.php
index 41f4ebcd2..12fae0da4 100644
--- a/tests/Utils/TaskSchedule/TaskAbstractTest.php
+++ b/tests/Utils/TaskSchedule/TaskAbstractTest.php
@@ -22,6 +22,11 @@ class TaskAbstractTest extends \PHPUnit\Framework\TestCase
     protected function setUp()
     {
         $this->class = new class('') extends TaskAbstract {
+            public function __toString() : string
+            {
+                return '';
+            }
+
             public static function createWith(array $jobData) : TaskAbstract
             {
 
@@ -33,7 +38,6 @@ class TaskAbstractTest extends \PHPUnit\Framework\TestCase
     {
         self::assertEquals('', $this->class->getId());
         self::assertEquals('', $this->class->getCommand());
-        self::assertEquals('', $this->class->getRun());
         self::assertEquals('', $this->class->getStatus());
         self::assertInstanceOf('\DateTime', $this->class->getNextRunTime());
         self::assertInstanceOf('\DateTime', $this->class->getLastRuntime());
@@ -45,9 +49,6 @@ class TaskAbstractTest extends \PHPUnit\Framework\TestCase
         $this->class->setCommand('Command');
         self::assertEquals('Command', $this->class->getCommand());
 
-        $this->class->setRun('Run');
-        self::assertEquals('Run', $this->class->getRun());
-
         $this->class->setStatus('Status');
         self::assertEquals('Status', $this->class->getStatus());
 
diff --git a/tests/Utils/TaskSchedule/TaskSchedulerTest.php b/tests/Utils/TaskSchedule/TaskSchedulerTest.php
index 6de0a9c16..b6bf0cb87 100644
--- a/tests/Utils/TaskSchedule/TaskSchedulerTest.php
+++ b/tests/Utils/TaskSchedule/TaskSchedulerTest.php
@@ -14,6 +14,7 @@
 namespace phpOMS\tests\Utils\TaskSchedule;
 
 use phpOMS\Utils\TaskSchedule\TaskScheduler;
+use phpOMS\Utils\TaskSchedule\Schedule;
 
 class TaskSchedulerTest extends \PHPUnit\Framework\TestCase
 {
@@ -25,24 +26,29 @@ class TaskSchedulerTest extends \PHPUnit\Framework\TestCase
     public function testCRUD()
     {
         if (\stristr(PHP_OS, 'WIN')) {
-            $cron = new Cron();
+            TaskScheduler::guessBin();
+            $cron = new TaskScheduler();
 
-            self::assertInstanceOf('\phpOMS\Utils\TaskSchedule\NullCronJob', $cron->getAllByName('testCronJob', false));
+            self::assertEquals([], $cron->getAllByName('testCronJob', false));
             
-            $cron->create(
-                new CronJob('testCronJob', 'testFile')
-            );
-            self::assertEquals('testFile', $cron->getRun());
+            $job = new Schedule('testCronJob', 'testFile', '0 0 1 1 *');
+            $cron->create($job);
+            
+            self::assertTrue(!empty($cron->getAllByName('testCronJob', false)));
+            if (!empty($cron->getAllByName('testCronJob', false))) {
+                self::assertEquals('testFile', $cron->getAllByName('testCronJob', false)[0]->getCommand());
+            }
 
-            $cron->update(
-                new CronJob('testCronJob', 'testFile2')
-            );
-            self::assertEquals('testFile2', $cron->getRun());
+            $job->setCommand('testFile2');
+            $cron->update($job);
 
-            $cron->delete(
-                new CronJob('testCronJob', 'testFile2')
-            );
-            self::assertInstanceOf('\phpOMS\Utils\TaskSchedule\NullCronJob', $cron->getAllByName('testCronJob', false));
+            self::assertTrue(!empty($cron->getAllByName('testCronJob', false)));
+            if (!empty($cron->getAllByName('testCronJob', false))) {
+                self::assertEquals('testFile2', $cron->getAllByName('testCronJob', false)[0]->getCommand());
+            }
+
+            $cron->delete($job);
+            self::assertEquals([], $cron->getAllByName('testCronJob', false));
         }
     }
 }