Moved commands that are closures to "closure" option, so we don't hav…

…e to hackishly figure out if string is closure data or not. Deprecated passing SerializableClosures in directly, not sure why this is needed and it breaks stuff.
jobbyphp · Dec 8, 2015 · d75fe89 · d75fe89
1 parent af29a03
commit d75fe89
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 41 deletions.
diff --git a/src/BackgroundJob.php b/src/BackgroundJob.php
@@ -3,10 +3,11 @@
 namespace Jobby;
 
 use Cron\CronExpression;
-use SuperClosure\SerializableClosure;
 
 class BackgroundJob
 {
+    use SerializerTrait;
+
     /**
      * @var Helper
      */
@@ -83,7 +84,7 @@ public function run()
             $this->helper->acquireLock($lockFile);
             $lockAcquired = true;
 
-            if ($this->isFunction()) {
+            if (isset($this->config['closure'])) {
                 $this->runFunction();
             } else {
                 $this->runFile();
@@ -227,24 +228,9 @@ protected function log($message)
         }
     }
 
-    /**
-     * @return bool
-     */
-    protected function isFunction()
-    {
-        $cmd = @unserialize($this->config['command']);
-
-        if ($cmd === false) {
-            return false;
-        }
-
-        return is_object($cmd) && $cmd instanceof SerializableClosure;
-    }
-
     protected function runFunction()
     {
-        /** @var SerializableClosure $command */
-        $command = unserialize($this->config['command']);
+        $command = $this->getSerializer()->unserialize($this->config['closure']);
 
         ob_start();
         $retval = $command();

diff --git a/src/Jobby.php b/src/Jobby.php
@@ -2,11 +2,14 @@
 
 namespace Jobby;
 
+use Closure;
 use SuperClosure\SerializableClosure;
 use Symfony\Component\Process\PhpExecutableFinder;
 
 class Jobby
 {
+    use SerializerTrait;
+
     /**
      * @var array
      */
@@ -104,9 +107,25 @@ public function getConfig()
      */
     public function add($job, array $config)
     {
-        foreach (['command', 'schedule'] as $field) {
-            if (empty($config[$field])) {
-                throw new Exception("'$field' is required for '$job' job");
+        if (empty($config['schedule'])) {
+            throw new Exception("'schedule' is required for '$job' job");
+        }
+
+        if (!(isset($config['command']) xor isset($config['closure']))) {
+            throw new Exception("Either 'command' or 'closure' is required for '$job' job");
+        }
+
+        if (isset($config['command']) &&
+            (
+                $config['command'] instanceof Closure ||
+                $config['command'] instanceof SerializableClosure
+            )
+        ) {
+            $config['closure'] = $config['command'];
+            unset($config['command']);
+
+            if ($config['closure'] instanceof SerializableClosure) {
+                $config['closure'] = $config['closure']->getClosure();
             }
         }
 
@@ -170,17 +189,9 @@ protected function runWindows($job, array $config)
      */
     protected function getExecutableCommand($job, array $config)
     {
-        if ($config['command'] instanceof SerializableClosure) {
-            $config['command'] = serialize($config['command']);
-
-        } else if ($config['command'] instanceof \Closure) {
-            // Convert closures to its source code as a string so that we
-            // can send it on the command line.
-            $config['command'] = $this->getHelper()
-                ->closureToString($config['command'])
-            ;
+        if (isset($config['closure'])) {
+            $config['closure'] = $this->getSerializer()->serialize($config['closure']);
         }
-
         return sprintf('"%s" "%s" "%s"', $this->script, $job, http_build_query($config));
     }
 

diff --git a/tests/BackgroundJobTest.php b/tests/BackgroundJobTest.php
@@ -4,13 +4,16 @@
 
 use Jobby\BackgroundJob;
 use Jobby\Helper;
+use Jobby\SerializerTrait;
 use Symfony\Component\Filesystem\Filesystem;
 
 /**
  * @coversDefaultClass Jobby\BackgroundJob
  */
 class BackgroundJobTest extends \PHPUnit_Framework_TestCase
 {
+    use SerializerTrait;
+
     const JOB_NAME = 'name';
 
     /**
@@ -51,15 +54,15 @@ public function runProvider()
 
             return true;
         };
-        $job = ['command' => $echo];
+        $job = ['closure' => $echo];
 
         return [
             'diabled, not run'       => [$job + ['enabled' => false], ''],
             'cron schedule, not run' => [$job + ['schedule' => '0 0 1 1 *'], ''],
             'date time, not run'     => [$job + ['schedule' => date('Y-m-d H:i:s', strtotime('tomorrow'))], ''],
             'date time, run'         => [$job + ['schedule' => date('Y-m-d H:i:s')], 'test'],
             'wrong host, not run'    => [$job + ['runOnHost' => 'something that does not match'], ''],
-            'current user, run,'     => [['command' => $uid], getmyuid()],
+            'current user, run,'     => [['closure' => $uid], getmyuid()],
         ];
     }
 
@@ -100,7 +103,7 @@ public function testClosureNotReturnTrue()
     {
         $this->runJob(
             [
-                'command' => function () {
+                'closure' => function () {
                     return false;
                 },
             ]
@@ -120,7 +123,7 @@ public function testHideStdOutByDefault()
         ob_start();
         $this->runJob(
             [
-                'command' => function () {
+                'closure' => function () {
                     echo 'foo bar';
                 },
                 'output'  => null,
@@ -140,7 +143,7 @@ public function testShouldCreateLogFolder()
         $logfile = dirname($this->logFile) . '/foo/bar.log';
         $this->runJob(
             [
-                'command' => function () {
+                'closure' => function () {
                     echo 'foo bar';
                 },
                 'output'  => $logfile,
@@ -169,7 +172,7 @@ public function testNotSendMailOnMissingRecipients()
 
         $this->runJob(
             [
-                'command'    => function () {
+                'closure'    => function () {
                     return false;
                 },
                 'recipients' => '',
@@ -190,7 +193,7 @@ public function testMailShouldTriggerHelper()
 
         $this->runJob(
             [
-                'command'    => function () {
+                'closure'    => function () {
                     return false;
                 },
                 'recipients' => 'test@example.com',
@@ -267,7 +270,7 @@ public function testHaltDir($createFile, $jobRuns)
         $this->runJob(
             [
                 'haltDir' => $dir,
-                'command' => function () {
+                'closure' => function () {
                     echo 'test';
 
                     return true;
@@ -312,8 +315,8 @@ private function getJobConfig(array $config)
     {
         $helper = new Helper();
 
-        if ($config['command'] instanceof \Closure) {
-            $config['command'] = $helper->closureToString($config['command']);
+        if (isset($config['closure'])) {
+            $config['closure'] = $this->getSerializer()->serialize($config['closure']);
         }
 
         return array_merge(