Decouple email sending from file system operations

composer.json

@@ -28,7 +28,8 @@
   },
   "autoload": {
     "psr-4": {
-      "omnilight\\scheduling\\": "src"
+      "omnilight\\scheduling\\": "src",
+      "omnilight\\scheduling\\Tests\\": "tests"
     }
   },
   "extra": {

src/Event.php

@@ -7,7 +7,6 @@
 use Symfony\Component\Process\Process;
 use yii\base\Application;
 use yii\base\Component;
-use yii\base\InvalidCallException;
 use yii\base\InvalidConfigException;
 use yii\mail\MailerInterface;
 use yii\mutex\Mutex;
@@ -62,12 +61,6 @@ class Event extends Component
      * @var string
      */
     protected $_output = null;
-    /**
-     * The string for redirection.
-     *
-     * @var array
-     */
-    protected $_redirect = ' > ';
     /**
      * The array of callbacks to be run after the event is finished.
      *
@@ -83,7 +76,7 @@ class Event extends Component
     /**
      * The mutex implementation.
      *
-     * @var \yii\mutex\Mutex
+     * @var Mutex
      */
     protected $_mutex;
 
@@ -98,7 +91,6 @@ public function __construct(Mutex $mutex, $command, $config = [])
     {
         $this->command = $command;
         $this->_mutex = $mutex;
-        $this->_output = $this->getDefaultOutput();
         parent::__construct($config);
     }
 
@@ -110,7 +102,8 @@ public function run(Application $app)
     {
         $this->trigger(self::EVENT_BEFORE_RUN);
         if (count($this->_afterCallbacks) > 0) {
-            $this->runCommandInForeground($app);
+            $process = $this->runCommandInForeground($app);
+            $this->callAfterCallbacks($app, $process);
         } else {
             $this->runCommandInBackground($app);
         }
@@ -131,35 +124,41 @@ protected function mutexName()
      * Run the command in the foreground.
      *
      * @param Application $app
+     * @return Process
      */
     protected function runCommandInForeground(Application $app)
     {
-        (new Process(
-            trim($this->buildCommand(), '& '), dirname($app->request->getScriptFile()), null, null, null
-        ))->run();
-        $this->callAfterCallbacks($app);
+        $process = new Process($this->buildCommand(), dirname($app->request->getScriptFile()));
+        $logCallback = null;
+        if (isset($this->_output)) {
+            $logCallback = function ($type, $data) {
+                file_put_contents($this->_output, $data, FILE_APPEND);
+            };
+        }
+        $process->run($logCallback);
+        return $process;
     }
 
     /**
-     * Build the comand string.
+     * Build the command string.
      *
      * @return string
      */
     public function buildCommand()
     {
-        $command = $this->command . $this->_redirect . $this->_output . ' 2>&1 &';
-        return $this->_user ? 'sudo -u ' . $this->_user . ' ' . $command : $command;
+        return $this->_user ? 'sudo -u ' . $this->_user . ' ' . $this->command : $this->command;
     }
 
     /**
      * Call all of the "after" callbacks for the event.
      *
      * @param Application $app
+     * @param Process $process
      */
-    protected function callAfterCallbacks(Application $app)
+    protected function callAfterCallbacks(Application $app, Process $process)
     {
         foreach ($this->_afterCallbacks as $callback) {
-            call_user_func($callback, $app);
+            call_user_func($callback, $app, $process);
         }
     }
 
@@ -171,7 +170,7 @@ protected function callAfterCallbacks(Application $app)
     protected function runCommandInBackground(Application $app)
     {
         chdir(dirname($app->request->getScriptFile()));
-        exec($this->buildCommand());
+        exec($this->buildCommand() . ' 2>&1 &');
     }
 
     /**
@@ -565,8 +564,8 @@ public function skip(\Closure $callback)
      */
     public function sendOutputTo($location)
     {
-        $this->_redirect = ' > ';
         $this->_output = $location;
+        file_put_contents($this->_output, '');
         return $this;
     }
 
@@ -578,7 +577,6 @@ public function sendOutputTo($location)
      */
     public function appendOutputTo($location)
     {
-        $this->_redirect = ' >> ';
         $this->_output = $location;
         return $this;
     }
@@ -593,12 +591,9 @@ public function appendOutputTo($location)
      */
     public function emailOutputTo($addresses)
     {
-        if (is_null($this->_output) || $this->_output == $this->getDefaultOutput()) {
-            throw new InvalidCallException("Must direct output to a file in order to e-mail results.");
-        }
         $addresses = is_array($addresses) ? $addresses : func_get_args();
-        return $this->then(function (Application $app) use ($addresses) {
-            $this->emailOutput($app->mailer, $addresses);
+        return $this->then(function (Application $app, Process $process) use ($addresses) {
+            $this->emailOutput($app->mailer, $addresses, $process);
         });
     }
 
@@ -618,19 +613,17 @@ public function then(\Closure $callback)
      * E-mail the output of the event to the recipients.
      *
      * @param MailerInterface $mailer
-     * @param  array $addresses
+     * @param array $addresses
+     * @param Process $process
      */
-    protected function emailOutput(MailerInterface $mailer, $addresses)
+    protected function emailOutput(MailerInterface $mailer, array $addresses, Process $process)
     {
-        $textBody = file_get_contents($this->_output);
-
-        if (trim($textBody) != '' ) {
-            $mailer->compose()
-                ->setTextBody($textBody)
-                ->setSubject($this->getEmailSubject())
-                ->setTo($addresses)
-                ->send();
-        }
+        $mailer->compose()
+            ->setTextBody($process->getOutput() ?: '(empty)')
+            ->setSubject($this->getEmailSubject())
+            ->setTo($addresses)
+            ->send()
+        ;
     }
 
     /**
@@ -678,7 +671,9 @@ public function description($description)
      */
     public function getSummaryForDisplay()
     {
-        if (is_string($this->_description)) return $this->_description;
+        if (is_string($this->_description)) {
+            return $this->_description;
+        }
         return $this->buildCommand();
     }
 
@@ -691,13 +686,4 @@ public function getExpression()
     {
         return $this->_expression;
     }
-
-    public function getDefaultOutput()
-    {
-        if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') {
-            return 'NUL';
-        } else {
-            return '/dev/null';
-        }
-    }
 }

tests/EventTest.php

@@ -2,29 +2,101 @@
 
 namespace omnilight\scheduling\Tests;
 
-use omnilight\scheduling\Event;
-use yii\mutex\Mutex;
-
 class EventTest extends \PHPUnit_Framework_TestCase
 {
-    public function buildCommandData()
+    /**
+     * Call protected/private method of a class.
+     *
+     * @param object &$object    Instantiated object that we will run method on.
+     * @param string $methodName Method name to call
+     * @param array  $parameters Array of parameters to pass into method.
+     *
+     * @return mixed Method return.
+     */
+    protected function invokeMethod(&$object, $methodName, array $parameters = [])
+    {
+        $reflection = new \ReflectionClass(get_class($object));
+        $method = $reflection->getMethod($methodName);
+        $method->setAccessible(true);
+        return $method->invokeArgs($object, $parameters);
+    }
+
+    public function outputData()
     {
         return [
-            ['php -i', '/dev/null', "php -i > /dev/null 2>&1 &"],
-            ['php -i', '/my folder/foo.log', "php -i > /my folder/foo.log 2>&1 &"],
+            ['test', 'test'],
+            ['', '(empty)'],
         ];
     }
 
+    public function testEmailOutputCalled()
+    {
+        $process = $this->getMockBuilder('Symfony\Component\Process\Process')
+            ->disableOriginalConstructor()
+            ->getMock()
+        ;
+
+        $mailer = $this->getMock('yii\mail\BaseMailer');
+
+        /** @var \yii\console\Application|\PHPUnit_Framework_MockObject_MockObject $app */
+        $app = $this->getMockBuilder('yii\console\Application')
+            ->disableOriginalConstructor()
+            ->getMock()
+        ;
+        $app->expects($this->once())
+            ->method('__get')
+            ->willReturnCallback(function ($name) use ($mailer) {
+                if ($name === 'mailer') {
+                    return $mailer;
+                }
+                return null;
+            })
+        ;
+
+        /** @var \omnilight\scheduling\Event|\PHPUnit_Framework_MockObject_MockObject $event */
+        $event = $this->getMockBuilder('omnilight\scheduling\Event')
+            ->disableOriginalConstructor()
+            ->setMethods(['emailOutput', 'runCommandInForeground'])
+            ->getMock()
+        ;
+        $event->expects($this->once())
+            ->method('emailOutput')
+        ;
+        $event->expects($this->once())
+            ->method('runCommandInForeground')
+            ->willReturn($process)
+        ;
+
+        $event->emailOutputTo([]);
+        $event->run($app);
+    }
+
     /**
-     * @dataProvider buildCommandData
-     * @param $command
-     * @param $outputTo
-     * @param $result
+     * @dataProvider outputData
+     * @param string $commandOutput
+     * @param string $expectedBody
      */
-    public function testBuildCommandSendOutputTo($command, $outputTo, $result)
+    public function testEmailOutputGetsData($commandOutput, $expectedBody)
     {
-        $event = new Event($this->getMock(Mutex::className()), $command);
-        $event->sendOutputTo($outputTo);
-        $this->assertSame($result, $event->buildCommand());
+        $message = $this->getMock('yii\mail\BaseMessage');
+        $message->expects($this->once())->method('setTextBody')->with($this->equalTo($expectedBody))->willReturnSelf();
+        $message->expects($this->once())->method('setSubject')->willReturnSelf();
+        $message->expects($this->once())->method('setTo')->willReturnSelf();
+
+        $mailer = $this->getMock('yii\mail\BaseMailer');
+        $mailer->expects($this->once())->method('compose')->willReturn($message);
+
+        $process = $this->getMockBuilder('Symfony\Component\Process\Process')
+            ->disableOriginalConstructor()
+            ->getMock()
+        ;
+        $process->expects($this->once())->method('getOutput')->willReturn($commandOutput);
+
+        $event = $this->getMockBuilder('omnilight\scheduling\Event')
+            ->disableOriginalConstructor()
+            ->getMock()
+        ;
+
+        $this->invokeMethod($event, 'emailOutput', [$mailer, [], $process]);
     }
 }