Skip to content

Commit

Permalink
Scheduler Improvements (#16806)
Browse files Browse the repository at this point in the history
This PR adds improvements to the scheduler. Previously, when ->runInBackground() was used "after" hooks were not run, meaning output was not e-mailed to the developer (when using emailOutputTo.

This change provides a new, hidden schedule:finish command that is used to fire the after callbacks for a given command by its mutex name. This command will not show in the Artisan console command list since it uses the hidden command feature which is new in Symfony 3.2.
  • Loading branch information
taylorotwell authored Dec 14, 2016
1 parent b34a148 commit 42a851a
Show file tree
Hide file tree
Showing 6 changed files with 170 additions and 32 deletions.
9 changes: 9 additions & 0 deletions src/Illuminate/Console/Command.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ class Command extends SymfonyCommand
*/
protected $description;

/**
* Indicates whether the command should be shown in the Artisan command list.
*
* @var bool
*/
protected $hidden = false;

/**
* The default verbosity of output commands.
*
Expand Down Expand Up @@ -95,6 +102,8 @@ public function __construct()

$this->setDescription($this->description);

$this->setHidden($this->hidden);

if (! isset($this->signature)) {
$this->specifyParameters();
}
Expand Down
6 changes: 5 additions & 1 deletion src/Illuminate/Console/ScheduleServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ class ScheduleServiceProvider extends ServiceProvider
*/
public function register()
{
$this->commands('Illuminate\Console\Scheduling\ScheduleRunCommand');
$this->commands([
'Illuminate\Console\Scheduling\ScheduleRunCommand',
'Illuminate\Console\Scheduling\ScheduleFinishCommand',
]);
}

/**
Expand All @@ -32,6 +35,7 @@ public function provides()
{
return [
'Illuminate\Console\Scheduling\ScheduleRunCommand',
'Illuminate\Console\Scheduling\ScheduleFinishCommand',
];
}
}
69 changes: 69 additions & 0 deletions src/Illuminate/Console/Scheduling/CommandBuilder.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<?php

namespace Illuminate\Console\Scheduling;

use Illuminate\Console\Application;
use Symfony\Component\Process\ProcessUtils;

class CommandBuilder
{
/**
* Build the command for the given event.
*
* @param \Illuminate\Console\Scheduling\Event $event
* @return string
*/
public function buildCommand(Event $event)
{
if ($event->runInBackground) {
return $this->buildBackgroundCommand($event);
} else {
return $this->buildForegroundCommand($event);
}
}

/**
* Build the command for running the event in the foreground.
*
* @param \Illuminate\Console\Scheduling\Event $event
* @return string
*/
protected function buildForegroundCommand(Event $event)
{
$output = ProcessUtils::escapeArgument($event->output);

return $this->finalize($event, $event->command.($event->shouldAppendOutput ? ' >> ' : ' > ').$output.' 2>&1');
}

/**
* Build the command for running the event in the foreground.
*
* @param \Illuminate\Console\Scheduling\Event $event
* @return string
*/
protected function buildBackgroundCommand(Event $event)
{
$output = ProcessUtils::escapeArgument($event->output);

$redirect = $event->shouldAppendOutput ? ' >> ' : ' > ';

$finished = Application::formatCommandString('schedule:finish').' "'.$event->mutexName().'"';

return $this->finalize($event,
'('.$event->command.$redirect.$output.' 2>&1 '.(windows_os() ? '&' : ';').' '.$finished.') > '
.ProcessUtils::escapeArgument($event->getDefaultOutput()).' 2>&1 &'
);
}

/**
* Finalize the event's command syntax.
*
* @param \Illuminate\Console\Scheduling\Event $event
* @param string $command
* @return string
*/
protected function finalize(Event $event, $command)
{
return $event->user && ! windows_os() ? 'sudo -u '.$event->user.' -- sh -c \''.$command.'\'' : $command;
}
}
41 changes: 14 additions & 27 deletions src/Illuminate/Console/Scheduling/Event.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,9 @@
use Carbon\Carbon;
use LogicException;
use Cron\CronExpression;
use Illuminate\Console\Application;
use GuzzleHttp\Client as HttpClient;
use Illuminate\Contracts\Mail\Mailer;
use Symfony\Component\Process\Process;
use Symfony\Component\Process\ProcessUtils;
use Illuminate\Contracts\Container\Container;
use Illuminate\Contracts\Cache\Repository as Cache;

Expand Down Expand Up @@ -107,7 +105,7 @@ class Event
*
* @var bool
*/
protected $shouldAppendOutput = false;
public $shouldAppendOutput = false;

/**
* The array of callbacks to be run before the event is started.
Expand Down Expand Up @@ -149,7 +147,7 @@ public function __construct(Cache $cache, $command)
*
* @return string
*/
protected function getDefaultOutput()
public function getDefaultOutput()
{
return (DIRECTORY_SEPARATOR == '\\') ? 'NUL' : '/dev/null';
}
Expand All @@ -167,7 +165,7 @@ public function run(Container $container)
}

if ($this->runInBackground) {
$this->runCommandInBackground();
$this->runCommandInBackground($container);
} else {
$this->runCommandInForeground($container);
}
Expand All @@ -193,10 +191,13 @@ protected function runCommandInForeground(Container $container)
/**
* Run the command in the background.
*
* @param \Illuminate\Contracts\Container\Container $container
* @return void
*/
protected function runCommandInBackground()
protected function runCommandInBackground(Container $container)
{
$this->callBeforeCallbacks($container);

(new Process(
$this->buildCommand(), base_path(), null, null, null
))->run();
Expand All @@ -208,7 +209,7 @@ protected function runCommandInBackground()
* @param \Illuminate\Contracts\Container\Container $container
* @return void
*/
protected function callBeforeCallbacks(Container $container)
public function callBeforeCallbacks(Container $container)
{
foreach ($this->beforeCallbacks as $callback) {
$container->call($callback);
Expand All @@ -221,7 +222,7 @@ protected function callBeforeCallbacks(Container $container)
* @param \Illuminate\Contracts\Container\Container $container
* @return void
*/
protected function callAfterCallbacks(Container $container)
public function callAfterCallbacks(Container $container)
{
foreach ($this->afterCallbacks as $callback) {
$container->call($callback);
Expand All @@ -235,31 +236,15 @@ protected function callAfterCallbacks(Container $container)
*/
public function buildCommand()
{
$output = ProcessUtils::escapeArgument($this->output);

$redirect = $this->shouldAppendOutput ? ' >> ' : ' > ';

if ($this->withoutOverlapping) {
$forget = Application::formatCommandString('cache:forget');

if (windows_os()) {
$command = '('.$this->command.' & '.$forget.' "'.$this->mutexName().'")'.$redirect.$output.' 2>&1 &';
} else {
$command = '('.$this->command.'; '.$forget.' '.$this->mutexName().')'.$redirect.$output.' 2>&1 &';
}
} else {
$command = $this->command.$redirect.$output.' 2>&1 &';
}

return $this->user && ! windows_os() ? 'sudo -u '.$this->user.' -- sh -c \''.$command.'\'' : $command;
return (new CommandBuilder)->buildCommand($this);
}

/**
* Get the mutex name for the scheduled command.
*
* @return string
*/
protected function mutexName()
public function mutexName()
{
return 'framework'.DIRECTORY_SEPARATOR.'schedule-'.sha1($this->expression.$this->command);
}
Expand Down Expand Up @@ -399,7 +384,9 @@ public function withoutOverlapping()
{
$this->withoutOverlapping = true;

return $this->skip(function () {
return $this->then(function () {
$this->cache->forget($this->mutexName());
})->skip(function () {
return $this->cache->has($this->mutexName());
});
}
Expand Down
61 changes: 61 additions & 0 deletions src/Illuminate/Console/Scheduling/ScheduleFinishCommand.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

namespace Illuminate\Console\Scheduling;

use Illuminate\Console\Command;

class ScheduleFinishCommand extends Command
{
/**
* The console command name.
*
* @var string
*/
protected $signature = 'schedule:finish {id}';

/**
* The console command description.
*
* @var string
*/
protected $description = 'Handle the completion of a scheduled command';

/**
* Indicates whether the command should be shown in the Artisan command list.
*
* @var bool
*/
protected $hidden = true;

/**
* The schedule instance.
*
* @var \Illuminate\Console\Scheduling\Schedule
*/
protected $schedule;

/**
* Create a new command instance.
*
* @param \Illuminate\Console\Scheduling\Schedule $schedule
* @return void
*/
public function __construct(Schedule $schedule)
{
$this->schedule = $schedule;

parent::__construct();
}

/**
* Execute the console command.
*
* @return void
*/
public function fire()
{
collect($this->schedule->events())->filter(function ($value) {
return $value->mutexName() == $this->argument('id');
})->each->callAfterCallbacks($this->laravel);
}
}
16 changes: 12 additions & 4 deletions tests/Console/Scheduling/EventTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,15 @@ public function testBuildCommand()
$event = new Event(m::mock('Illuminate\Contracts\Cache\Repository'), 'php -i');

$defaultOutput = (DIRECTORY_SEPARATOR == '\\') ? 'NUL' : '/dev/null';
$this->assertSame("php -i > {$quote}{$defaultOutput}{$quote} 2>&1 &", $event->buildCommand());
$this->assertSame("php -i > {$quote}{$defaultOutput}{$quote} 2>&1", $event->buildCommand());

$quote = (DIRECTORY_SEPARATOR == '\\') ? '"' : "'";

$event = new Event(m::mock('Illuminate\Contracts\Cache\Repository'), 'php -i');
$event->runInBackground();

$defaultOutput = (DIRECTORY_SEPARATOR == '\\') ? 'NUL' : '/dev/null';
$this->assertSame("(php -i > {$quote}{$defaultOutput}{$quote} 2>&1 ; '".PHP_BINARY."' artisan schedule:finish \"framework/schedule-c65b1c374c37056e0c57fccb0c08d724ce6f5043\") > {$quote}{$defaultOutput}{$quote} 2>&1 &", $event->buildCommand());
}

public function testBuildCommandSendOutputTo()
Expand All @@ -27,12 +35,12 @@ public function testBuildCommandSendOutputTo()
$event = new Event(m::mock('Illuminate\Contracts\Cache\Repository'), 'php -i');

$event->sendOutputTo('/dev/null');
$this->assertSame("php -i > {$quote}/dev/null{$quote} 2>&1 &", $event->buildCommand());
$this->assertSame("php -i > {$quote}/dev/null{$quote} 2>&1", $event->buildCommand());

$event = new Event(m::mock('Illuminate\Contracts\Cache\Repository'), 'php -i');

$event->sendOutputTo('/my folder/foo.log');
$this->assertSame("php -i > {$quote}/my folder/foo.log{$quote} 2>&1 &", $event->buildCommand());
$this->assertSame("php -i > {$quote}/my folder/foo.log{$quote} 2>&1", $event->buildCommand());
}

public function testBuildCommandAppendOutput()
Expand All @@ -42,7 +50,7 @@ public function testBuildCommandAppendOutput()
$event = new Event(m::mock('Illuminate\Contracts\Cache\Repository'), 'php -i');

$event->appendOutputTo('/dev/null');
$this->assertSame("php -i >> {$quote}/dev/null{$quote} 2>&1 &", $event->buildCommand());
$this->assertSame("php -i >> {$quote}/dev/null{$quote} 2>&1", $event->buildCommand());
}

/**
Expand Down

0 comments on commit 42a851a

Please sign in to comment.