diff --git a/src/Illuminate/Console/Scheduling/CallbackEvent.php b/src/Illuminate/Console/Scheduling/CallbackEvent.php index dde5d7dea549..e91600ac3a40 100644 --- a/src/Illuminate/Console/Scheduling/CallbackEvent.php +++ b/src/Illuminate/Console/Scheduling/CallbackEvent.php @@ -6,6 +6,7 @@ use Illuminate\Support\Reflector; use InvalidArgumentException; use LogicException; +use RuntimeException; use Throwable; class CallbackEvent extends Event @@ -24,11 +25,25 @@ class CallbackEvent extends Event */ protected $parameters; + /** + * The result of the callback's execution. + * + * @var mixed + */ + protected $result; + + /** + * The exception that was thrown when calling the callback, if any. + * + * @var \Throwable|null + */ + protected $exception; + /** * Create a new event instance. * * @param \Illuminate\Console\Scheduling\EventMutex $mutex - * @param string $callback + * @param string|callable $callback * @param array $parameters * @param \DateTimeZone|string|null $timezone * @return void @@ -50,58 +65,64 @@ public function __construct(EventMutex $mutex, $callback, array $parameters = [] } /** - * Run the given event. + * Run the callback event. * * @param \Illuminate\Contracts\Container\Container $container * @return mixed * - * @throws \Exception + * @throws \Throwable */ public function run(Container $container) { - if ($this->description && $this->withoutOverlapping && - ! $this->mutex->create($this)) { - return; - } - - $pid = getmypid(); - - register_shutdown_function(function () use ($pid) { - if ($pid === getmypid()) { - $this->removeMutex(); - } - }); - - parent::callBeforeCallbacks($container); - - try { - $response = is_object($this->callback) - ? $container->call([$this->callback, '__invoke'], $this->parameters) - : $container->call($this->callback, $this->parameters); - - $this->exitCode = $response === false ? 1 : 0; - } catch (Throwable $e) { - $this->exitCode = 1; + parent::run($container); - throw $e; - } finally { - $this->removeMutex(); - - parent::callAfterCallbacks($container); + if ($this->exception) { + throw $this->exception; } - return $response; + return $this->result; } /** - * Clear the mutex for the event. + * Determine if the event should skip because another process is overlapping. + * + * @return bool + */ + public function shouldSkipDueToOverlapping() + { + return $this->description && parent::shouldSkipDueToOverlapping(); + } + + /** + * Indicate that the callback should run in the background. * * @return void + * + * @throws \RuntimeException */ - protected function removeMutex() + public function runInBackground() + { + throw new RuntimeException('Scheduled closures can not be run in the background.'); + } + + /** + * Run the callback. + * + * @param \Illuminate\Contracts\Container\Container $container + * @return int + */ + protected function execute($container) { - if ($this->description && $this->withoutOverlapping) { - $this->mutex->forget($this); + try { + $this->result = is_object($this->callback) + ? $container->call([$this->callback, '__invoke'], $this->parameters) + : $container->call($this->callback, $this->parameters); + + return $this->result === false ? 1 : 0; + } catch (Throwable $e) { + $this->exception = $e; + + return 1; } } @@ -121,13 +142,7 @@ public function withoutOverlapping($expiresAt = 1440) ); } - $this->withoutOverlapping = true; - - $this->expiresAt = $expiresAt; - - return $this->skip(function () { - return $this->mutex->exists($this); - }); + return parent::withoutOverlapping($expiresAt); } /** @@ -145,9 +160,21 @@ public function onOneServer() ); } - $this->onOneServer = true; + return parent::onOneServer(); + } - return $this; + /** + * Get the summary of the event for display. + * + * @return string + */ + public function getSummaryForDisplay() + { + if (is_string($this->description)) { + return $this->description; + } + + return is_string($this->callback) ? $this->callback : 'Callback'; } /** @@ -161,16 +188,14 @@ public function mutexName() } /** - * Get the summary of the event for display. + * Clear the mutex for the event. * - * @return string + * @return void */ - public function getSummaryForDisplay() + protected function removeMutex() { - if (is_string($this->description)) { - return $this->description; + if ($this->description) { + parent::removeMutex(); } - - return is_string($this->callback) ? $this->callback : 'Callback'; } } diff --git a/src/Illuminate/Console/Scheduling/Event.php b/src/Illuminate/Console/Scheduling/Event.php index 4de88f163dbf..2df0417ae07d 100644 --- a/src/Illuminate/Console/Scheduling/Event.php +++ b/src/Illuminate/Console/Scheduling/Event.php @@ -188,66 +188,81 @@ public function getDefaultOutput() * * @param \Illuminate\Contracts\Container\Container $container * @return void + * + * @throws \Throwable */ public function run(Container $container) { - if ($this->withoutOverlapping && - ! $this->mutex->create($this)) { + if ($this->shouldSkipDueToOverlapping()) { return; } - $this->runInBackground - ? $this->runCommandInBackground($container) - : $this->runCommandInForeground($container); + $exitCode = $this->start($container); + + if (! $this->runInBackground) { + $this->finish($container, $exitCode); + } } /** - * Get the mutex name for the scheduled command. + * Determine if the event should skip because another process is overlapping. * - * @return string + * @return bool */ - public function mutexName() + public function shouldSkipDueToOverlapping() { - return 'framework'.DIRECTORY_SEPARATOR.'schedule-'.sha1($this->expression.$this->command); + return $this->withoutOverlapping && ! $this->mutex->create($this); } /** - * Run the command in the foreground. + * Run the command process. * * @param \Illuminate\Contracts\Container\Container $container - * @return void + * @return int + * + * @throws \Throwable */ - protected function runCommandInForeground(Container $container) + protected function start($container) { try { $this->callBeforeCallbacks($container); - $this->exitCode = Process::fromShellCommandline( - $this->buildCommand(), base_path(), null, null, null - )->run(); - - $this->callAfterCallbacks($container); - } finally { + return $this->execute($container); + } catch (Throwable $exception) { $this->removeMutex(); + + throw $exception; } } /** - * Run the command in the background. + * Run the command process. + * + * @param \Illuminate\Contracts\Container\Container $container + * @return int + */ + protected function execute($container) + { + return Process::fromShellCommandline( + $this->buildCommand(), base_path(), null, null, null + )->run(); + } + + /** + * Mark the command process as finished and run callbacks/cleanup. * * @param \Illuminate\Contracts\Container\Container $container + * @param int $exitCode * @return void */ - protected function runCommandInBackground(Container $container) + public function finish(Container $container, $exitCode) { - try { - $this->callBeforeCallbacks($container); + $this->exitCode = (int) $exitCode; - Process::fromShellCommandline($this->buildCommand(), base_path(), null, null, null)->run(); - } catch (Throwable $exception) { + try { + $this->callAfterCallbacks($container); + } finally { $this->removeMutex(); - - throw $exception; } } @@ -277,24 +292,6 @@ public function callAfterCallbacks(Container $container) } } - /** - * Call all of the "after" callbacks for the event. - * - * @param \Illuminate\Contracts\Container\Container $container - * @param int $exitCode - * @return void - */ - public function callAfterCallbacksWithExitCode(Container $container, $exitCode) - { - $this->exitCode = (int) $exitCode; - - try { - $this->callAfterCallbacks($container); - } finally { - $this->removeMutex(); - } - } - /** * Build the command string. * @@ -931,6 +928,16 @@ public function preventOverlapsUsing(EventMutex $mutex) return $this; } + /** + * Get the mutex name for the scheduled command. + * + * @return string + */ + public function mutexName() + { + return 'framework'.DIRECTORY_SEPARATOR.'schedule-'.sha1($this->expression.$this->command); + } + /** * Delete the mutex for the event. * diff --git a/src/Illuminate/Console/Scheduling/ScheduleFinishCommand.php b/src/Illuminate/Console/Scheduling/ScheduleFinishCommand.php index db52531b4a38..03a5020dd606 100644 --- a/src/Illuminate/Console/Scheduling/ScheduleFinishCommand.php +++ b/src/Illuminate/Console/Scheduling/ScheduleFinishCommand.php @@ -49,7 +49,7 @@ public function handle(Schedule $schedule) collect($schedule->events())->filter(function ($value) { return $value->mutexName() == $this->argument('id'); })->each(function ($event) { - $event->callafterCallbacksWithExitCode($this->laravel, $this->argument('code')); + $event->finish($this->laravel, $this->argument('code')); $this->laravel->make(Dispatcher::class)->dispatch(new ScheduledBackgroundTaskFinished($event)); }); diff --git a/tests/Integration/Console/CallbackSchedulingTest.php b/tests/Integration/Console/CallbackSchedulingTest.php index 9ad046b1931b..702ff8116553 100644 --- a/tests/Integration/Console/CallbackSchedulingTest.php +++ b/tests/Integration/Console/CallbackSchedulingTest.php @@ -52,10 +52,7 @@ protected function tearDown(): void parent::tearDown(); } - /** - * @dataProvider executionProvider - */ - public function testExecutionOrder($background) + public function testExecutionOrder() { $event = $this->app->make(Schedule::class) ->call($this->logger('call')) @@ -64,15 +61,20 @@ public function testExecutionOrder($background) ->after($this->logger('after 2')) ->before($this->logger('before 2')); - if ($background) { - $event->runInBackground(); - } - $this->artisan('schedule:run'); $this->assertLogged('before 1', 'before 2', 'call', 'after 1', 'after 2'); } + public function testCallbacksCannotRunInBackground() + { + $this->expectException(RuntimeException::class); + + $this->app->make(Schedule::class) + ->call($this->logger('call')) + ->runInBackground(); + } + public function testExceptionHandlingInCallback() { $event = $this->app->make(Schedule::class) @@ -118,14 +120,6 @@ public function testExceptionHandlingInCallback() $this->assertFalse($event->mutex->exists($event)); } - public function executionProvider() - { - return [ - 'Foreground' => [false], - 'Background' => [true], - ]; - } - protected function logger($message) { return function () use ($message) {