feat: implement add console command as arguments+name rather than together

gsdevme · gsdevme · commit 3ee2cd6da721 · 2020-07-09T21:54:33.000+01:00
diff --git a/.travis.yml b/.travis.yml
@@ -25,7 +25,7 @@ install:
   - if [ "$deps" = "low" ]; then composer --prefer-lowest -n --prefer-stable --prefer-dist update; fi;
 
 
-script: bin/phpunit && bin/phpstan analyse -l 3 ./
+script: bin/phpunit && bin/phpstan analyse -l 5 ./
 
 notifications:
   email: "calum@usemarkup.com"
diff --git a/Command/AddCommandJobToQueueCommand.php b/Command/AddCommandJobToQueueCommand.php
@@ -14,6 +14,8 @@
  */
 class AddCommandJobToQueueCommand extends Command
 {
+    protected static $defaultName = 'markup:job_queue:add:command';
+
     /**
      * @var JobManager
      */
@@ -31,7 +33,6 @@ public function __construct(JobManager $jobby)
     protected function configure()
     {
         $this
-            ->setName('markup:job_queue:add:command')
             ->setDescription('Adds a single job that executes a command via the job queue')
             ->addArgument(
                 'cmd',
@@ -66,7 +67,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
         $timeout = intval($input->getOption('timeout'));
         $idleTimeout = intval($input->getOption('idle_timeout'));
 
-        $this->jobby->addCommandJob($command, $topic, $timeout, $idleTimeout);
+        $this->jobby->addConsoleCommandJob($command, [], $topic, $timeout, $idleTimeout);
 
         $output->writeln('<info>Added command to job queue</info>');
     }
diff --git a/Command/AddRecurringConsoleJobToQueueCommand.php b/Command/AddRecurringConsoleJobToQueueCommand.php
@@ -106,8 +106,9 @@ private function addRecurringJobs(OutputInterface $output)
                     continue;
                 }
             }
-            $this->jobManager->addCommandJob(
+            $this->jobManager->addConsoleCommandJob(
                 $configuration->getCommand(),
+                [],
                 $configuration->getTopic(),
                 $configuration->getTimeout(),
                 $configuration->getTimeout()
@@ -118,9 +119,8 @@ private function addRecurringJobs(OutputInterface $output)
                 $configuration->getCommand(),
                 $configuration->getTopic()
             );
-            if ($configuration->nextRun()) {
-                $message = sprintf('%s. Will next be added %s', $message, $configuration->nextRun()->format('r'));
-            }
+
+            $message = sprintf('%s. Will next be added %s', $message, $configuration->nextRun()->format('r'));
             $output->writeln(sprintf('<info>%s</info>', $message));
         }
 
diff --git a/Command/AddScheduleJobToQueueCommand.php b/Command/AddScheduleJobToQueueCommand.php
@@ -2,44 +2,80 @@
 
 namespace Markup\JobQueueBundle\Command;
 
-use Symfony\Bundle\FrameworkBundle\Command\ContainerAwareCommand;
+use Markup\JobQueueBundle\Service\JobManager;
+use Markup\JobQueueBundle\Service\ScheduledJobService;
+use Psr\Log\LoggerInterface;
+use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 
 /**
  * This command adds scheduled jobs to the job-queue
  *
  */
-class AddScheduleJobToQueueCommand extends ContainerAwareCommand
+class AddScheduleJobToQueueCommand extends Command
 {
+    protected static $defaultName = 'markup:scheduled_job:add';
+
+    /**
+     * @var JobManager
+     */
+    private $jobManager;
+
+    /**
+     * @var LoggerInterface|null
+     */
+    private $logger;
+
+    /**
+     * @var ScheduledJobService
+     */
+    private $scheduledJobService;
+
+    public function __construct(
+        JobManager $jobManager,
+        ScheduledJobService $scheduledJobService,
+        ?LoggerInterface $logger = null
+    ) {
+        $this->jobManager = $jobManager;
+        $this->logger = $logger;
+        $this->scheduledJobService = $scheduledJobService;
+
+        parent::__construct(null);
+    }
+
     /**
      * @see Command
      */
     protected function configure()
     {
         $this
-            ->setName('markup:scheduled_job:add')
             ->setDescription('Adds scheduled jobs to the job-queue');
     }
 
     protected function execute(InputInterface $input, OutputInterface $output)
     {
-        $scheduledJobService =  $this->getContainer()->get('markup_job_queue.scheduled');
-        $logger = $this->getContainer()->get('logger');
-
-        if ($jobs = $scheduledJobService->getUnqueuedJobs()) {
+        if ($jobs = $this->scheduledJobService->getUnqueuedJobs()) {
             foreach ($jobs as $job) {
                 try {
-                    $this->getContainer()->get('jobby')->addCommandJob(
+                    $this->jobManager->addConsoleCommandJob(
                         $job->getJob(),
+                        [],
                         $job->getTopic(),
                         3600,
                         3600
                     );
                     $job->setQueued(true);
-                    $scheduledJobService->save($job, $flush = true);
-                } catch(\Exception $e) {
-                    $logger->error(sprintf('There was an error adding the job "%s" to the job-queue, error: %s', $job->getJob(), $e->getMessage()));
+
+                    $this->scheduledJobService->save($job, $flush = true);
+                } catch (\Exception $e) {
+                    $this->logger->error(
+                        sprintf(
+                            'There was an error adding the job "%s" to the job-queue, error: %s',
+                            $job->getJob(),
+                            $e->getMessage()
+                        )
+                    );
                 }
             }
         }
diff --git a/Command/AddTestJobCommand.php b/Command/AddTestJobCommand.php
@@ -53,7 +53,7 @@ protected function configure()
                 'quantity',
                 InputArgument::OPTIONAL,
                 'The number of times to add the job',
-                1
+                '1'
             )
             ->addArgument(
                 'topic',
@@ -86,7 +86,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
                 break;
         }
 
-        $quantity = $input->getArgument('quantity');
+        $quantity = intval($input->getArgument('quantity'));
         for ($i = 0; $i < $quantity; $i++) {
             $this->jobby->addJob($job);
         }
diff --git a/Exception/JobFailedException.php b/Exception/JobFailedException.php
@@ -15,13 +15,13 @@ class JobFailedException extends \Exception
      * @param string         $message
      * @param null           $exitCode
      * @param int            $code
-     * @param \Exception|null $previous
+     * @param \Throwable|null $previous
      */
     public function __construct(
         $message = "",
         $exitCode = null,
         $code = 0,
-        \Exception $previous = null
+        \Throwable $previous = null
     ) {
         $this->exitCode = $exitCode;
         parent::__construct($message, $code, $previous);
diff --git a/Job/ConsoleCommandJob.php b/Job/ConsoleCommandJob.php
@@ -24,17 +24,11 @@ public function run(ParameterBagInterface $parameterBag): string
 
         $command[] = $this->getConsolePath($parameterBag->get('markup_job_queue.console_dir'));
 
-        /**
-         * This is less than ideal, but trying to support legacy and v3 and v4 symfony
-         */
-        // If the command has been provided like `do:something "hello"` allow it through
-        if (stripos($this->args['command'], '"') !== false) {
-            $command[] = $this->args['command'];
-        } else {
-            // If the command has been provided like `do:something hello` split so the escaping is correct
-            $command = array_merge($command, explode(' ', $this->args['command']));
-        }
+        $command[] = $this->args['command'];
 
+        foreach ($this->args['arguments'] as $argument) {
+            $command[] = $argument;
+        };
 
         $uuid = isset($this->args['uuid']) ? $this->args['uuid']: null;
         if($uuid) {
@@ -84,16 +78,9 @@ public function run(ParameterBagInterface $parameterBag): string
         } catch (JobFailedException $e) {
             throw $e;
         }  catch (RuntimeException $e) {
-            // if process has been signalled then use the termSignal as the exit code
-            try {
-                $code = sprintf('SIGNAL %s', $process->getTermSignal());
-            } catch(\Exception $e) {
-                // RuntimeException may have been thrown for some other reason
-                $code = 'UNKNOWN';
-            }
-            throw new JobFailedException($e->getMessage(), $code);
-        } catch (\Exception $e) {
-            throw new JobFailedException($e->getMessage());
+            throw new JobFailedException($e->getMessage(), $process->getExitCode(), $e->getCode(), $e);
+        } catch (\Throwable $e) {
+            throw new JobFailedException($e->getMessage(), $process->getExitCode(), $e->getCode(), $e);
         }
     }
 
diff --git a/Job/SleepJob.php b/Job/SleepJob.php
@@ -30,7 +30,7 @@ public function validate()
      */
     public function run(ParameterBagInterface $parameterBag): string
     {
-        $process = new Process(sprintf('sleep %s', $this->args['time']));
+        $process = new Process(['sleep', $this->args['time']]);
         $process->run();
 
         return '';
diff --git a/Model/RecurringConsoleCommandConfiguration.php b/Model/RecurringConsoleCommandConfiguration.php
@@ -131,7 +131,7 @@ public function previousRun($time = 'now')
 
         if ($cron->isDue($time)) {
             $now = new \DateTime();
-            $now->setTime($now->format('H'), $now->format('i'));
+            $now->setTime(intval($now->format('H')), intval($now->format('i')));
             return $now;
         }
         return $cron->getPreviousRunDate($time);
diff --git a/README.md b/README.md
@@ -81,8 +81,9 @@ Jobs can also be added directly. There is a utility method for adding 'command'
 
 ```php
 $container->get('jobby')
-	->addCommandJob(
-		'your:console:command --plus=any --options or arguments', #this needs to be a valid command
+	->addConsoleCommandJob(
+		'your:console:command',
+		['--test']
 		'a_valid_topic', # should be a valid topic name
 		600, # allowed timeout for command (see symfony process component documentation)
 		600, # allowed idle timeout for command (see symfony process component documentation)
diff --git a/Resources/config/commands.yml b/Resources/config/commands.yml
@@ -19,6 +19,10 @@ services:
 
     markup_job_queue.command.add_schedule_job_to_queue:
         class: Markup\JobQueueBundle\Command\AddScheduleJobToQueueCommand
+        arguments:
+            $jobManager: '@jobby'
+            $scheduledJobService: '@markup_job_queue.scheduled'
+            $logger: '@logger'
         tags:
             - { name: console.command, command: 'markup:scheduled_job:add' }
 
diff --git a/Service/JobManager.php b/Service/JobManager.php
@@ -45,15 +45,22 @@ public function addJob(Job $job, $dateTime = null)
 
     /**
      * Adds a named command to the job queue
-     * @param string  $command     A valid command for this application.
-     * @param string  $topic       The name of a valid topic.
-     * @param integer $timeout     The amount of time to allow the command to run.
+     *
+     * @param string $command A valid command for this application.
+     * @param array $arguments
+     * @param string $topic The name of a valid topic.
+     * @param integer $timeout The amount of time to allow the command to run.
      * @param integer $idleTimeout The amount of idle time to allow the command to run. Defaults to the same as timeout.
      */
-    public function addCommandJob($command, $topic = 'default', $timeout = 60, $idleTimeout = null)
+    public function addConsoleCommandJob(string $command, array $arguments = [], $topic = 'default', $timeout = 60, $idleTimeout = null)
     {
+        if (stripos($command, " ") !== false) {
+            throw new \InvalidArgumentException('Console command is not expected to have spaces within the name');
+        }
+
         $args = [];
         $args['command'] = $command;
+        $args['arguments'] = $arguments;
         $args['timeout'] = $timeout;
         $args['idleTimeout'] = $idleTimeout ?? $timeout;
         $job = new ConsoleCommandJob($args, $topic);
diff --git a/Tests/Service/JobManagerTest.php b/Tests/Service/JobManagerTest.php
@@ -59,14 +59,14 @@ public function testCanAddConsoleCommandJobWithDateTime(): void
 
     public function testCanAddCommandJob(): void
     {
-        $this->jobManager->addCommandJob('console:herp:derp', 'system', 60, 60);
+        $this->jobManager->addConsoleCommandJob('console:herp:derp', [], 'system', 60, 60);
         $this->assertCount(1, $this->jobPublisher->getJobs());
     }
 
     public function testIdleTimeoutDefaultsToTimeout(): void
     {
         $timeout = 720;
-        $this->jobManager->addCommandJob('command', 'topic', $timeout);
+        $this->jobManager->addConsoleCommandJob('command', [], 'topic', $timeout);
         /** @var Job $job */
         $job = $this->jobPublisher->getJobs()[0];
         $this->assertEquals($timeout, $job->getArgs()['idleTimeout']);
diff --git a/Util/LegacyFormHelper.php b/Util/LegacyFormHelper.php

Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ protected function configure()`
`53`	`53`	`'quantity',`
`54`	`54`	`InputArgument::OPTIONAL,`
`55`	`55`	`'The number of times to add the job',`
`56`		`- 1`
	`56`	`+ '1'`
`57`	`57`	`)`
`58`	`58`	`->addArgument(`
`59`	`59`	`'topic',`
`@@ -86,7 +86,7 @@ protected function execute(InputInterface $input, OutputInterface $output)`
`86`	`86`	`break;`
`87`	`87`	`}`
`88`	`88`
`89`		`- $quantity = $input->getArgument('quantity');`
	`89`	`+ $quantity = intval($input->getArgument('quantity'));`
`90`	`90`	`for ($i = 0; $i < $quantity; $i++) {`
`91`	`91`	`$this->jobby->addJob($job);`
`92`	`92`	`}`
Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,7 @@ public function validate()`
`30`	`30`	`*/`
`31`	`31`	`public function run(ParameterBagInterface $parameterBag): string`
`32`	`32`	`{`
`33`		`- $process = new Process(sprintf('sleep %s', $this->args['time']));`
	`33`	`+ $process = new Process(['sleep', $this->args['time']]);`
`34`	`34`	`$process->run();`
`35`	`35`
`36`	`36`	`return '';`
Original file line number	Diff line number	Diff line change
`@@ -131,7 +131,7 @@ public function previousRun($time = 'now')`
`131`	`131`
`132`	`132`	`if ($cron->isDue($time)) {`
`133`	`133`	`$now = new \DateTime();`
`134`		`- $now->setTime($now->format('H'), $now->format('i'));`
	`134`	`+ $now->setTime(intval($now->format('H')), intval($now->format('i')));`
`135`	`135`	`return $now;`
`136`	`136`	`}`
`137`	`137`	`return $cron->getPreviousRunDate($time);`