diff --git a/UPGRADE.md b/UPGRADE.md index 25a2b82..2a79fbb 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,9 +1,19 @@ UPGRADE ======= +- [1.2.0](#1.2.0) - [1.1.0](#1.1.0) - [0.4.0](#0.4.0) +### 1.2.0 + +In the database table `ta_task_executions` a new field was introduced. Run following +command to update the table. + +```bash +bin/console doctrine:schema:update +``` + ### 1.1.0 In the database table `ta_tasks` a new field was introduced. Run following diff --git a/src/Command/ExecuteCommand.php b/src/Command/ExecuteCommand.php index 554e104..e45fea3 100644 --- a/src/Command/ExecuteCommand.php +++ b/src/Command/ExecuteCommand.php @@ -16,6 +16,7 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\ConsoleOutputInterface; use Symfony\Component\Console\Output\OutputInterface; +use Task\Executor\FailedException; use Task\Handler\TaskHandlerFactoryInterface; use Task\Storage\TaskExecutionRepositoryInterface; @@ -63,15 +64,20 @@ protected function configure() */ protected function execute(InputInterface $input, OutputInterface $output) { - $errOutput = $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output; + $errorOutput = $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output; $execution = $this->executionRepository->findByUuid($input->getArgument('uuid')); $handler = $this->handlerFactory->create($execution->getHandlerClass()); try { $result = $handler->handle($execution->getWorkload()); - } catch (\Exception $e) { - $errOutput->writeln($e->__toString()); + } catch (\Exception $exception) { + if ($exception instanceof FailedException) { + $errorOutput->writeln(FailedException::class); + $exception = $exception->getPrevious(); + } + + $errorOutput->writeln($exception->__toString()); // Process exit-code: 0 = OK, >1 = FAIL return 1; diff --git a/src/Executor/ExecutionProcessFactory.php b/src/Executor/ExecutionProcessFactory.php new file mode 100644 index 0000000..0649381 --- /dev/null +++ b/src/Executor/ExecutionProcessFactory.php @@ -0,0 +1,46 @@ +consolePath = $consolePath; + $this->environment = $environment; + } + + /** + * Create process for given execution-uuid. + * + * @param string $uuid + * + * @return Process + */ + public function create($uuid) + { + return $process = ProcessBuilder::create( + [$this->consolePath, 'task:execute', $uuid, '-e ' . $this->environment] + )->getProcess(); + } +} diff --git a/src/Executor/SeparateProcessExecutor.php b/src/Executor/SeparateProcessExecutor.php index 610c38a..cd56c86 100644 --- a/src/Executor/SeparateProcessExecutor.php +++ b/src/Executor/SeparateProcessExecutor.php @@ -11,9 +11,12 @@ namespace Task\TaskBundle\Executor; -use Symfony\Component\Process\ProcessBuilder; use Task\Execution\TaskExecutionInterface; use Task\Executor\ExecutorInterface; +use Task\Executor\FailedException; +use Task\Executor\RetryTaskHandlerInterface; +use Task\Handler\TaskHandlerFactoryInterface; +use Task\Storage\TaskExecutionRepositoryInterface; /** * Uses a separate process to start the executions via console-command. @@ -21,23 +24,33 @@ class SeparateProcessExecutor implements ExecutorInterface { /** - * @var string + * @var TaskHandlerFactoryInterface */ - private $consolePath; + private $handlerFactory; /** - * @var string + * @var TaskExecutionRepositoryInterface */ - private $environment; + private $executionRepository; /** - * @param string $consolePath - * @param string $environment + * @var ExecutionProcessFactory */ - public function __construct($consolePath, $environment) - { - $this->consolePath = $consolePath; - $this->environment = $environment; + private $processFactory; + + /** + * @param TaskHandlerFactoryInterface $handlerFactory + * @param TaskExecutionRepositoryInterface $executionRepository + * @param ExecutionProcessFactory $processFactory + */ + public function __construct( + TaskHandlerFactoryInterface $handlerFactory, + TaskExecutionRepositoryInterface $executionRepository, + ExecutionProcessFactory $processFactory + ) { + $this->handlerFactory = $handlerFactory; + $this->executionRepository = $executionRepository; + $this->processFactory = $processFactory; } /** @@ -45,16 +58,85 @@ public function __construct($consolePath, $environment) */ public function execute(TaskExecutionInterface $execution) { - $process = ProcessBuilder::create( - [$this->consolePath, 'task:execute', $execution->getUuid(), '-e ' . $this->environment] - )->getProcess(); + $attempts = $this->getMaximumAttempts($execution->getHandlerClass()); + $lastException = null; + + for ($attempt = 0; $attempt < $attempts; ++$attempt) { + try { + return $this->handle($execution); + } catch (FailedException $exception) { + throw $exception; + } catch (SeparateProcessException $exception) { + if ($execution->getAttempts() < $attempts) { + $execution->incrementAttempts(); + $this->executionRepository->save($execution); + } + + $lastException = $exception; + } + } + + // maximum attempts to pass executions are reached + throw new FailedException($lastException); + } + + /** + * Returns maximum attempts for specified handler. + * + * @param string $handlerClass + * + * @return int + */ + private function getMaximumAttempts($handlerClass) + { + $handler = $this->handlerFactory->create($handlerClass); + if (!$handler instanceof RetryTaskHandlerInterface) { + return 1; + } + return $handler->getMaximumAttempts(); + } + + /** + * Handle execution by using console-command. + * + * @param TaskExecutionInterface $execution + * + * @return string + * + * @throws FailedException + * @throws SeparateProcessException + */ + private function handle(TaskExecutionInterface $execution) + { + $process = $this->processFactory->create($execution->getUuid()); $process->run(); if (!$process->isSuccessful()) { - throw new SeparateProcessException($process->getErrorOutput()); + throw $this->createException($process->getErrorOutput()); } return $process->getOutput(); } + + /** + * Create the correct exception. + * + * FailedException for failed executions. + * SeparateProcessExceptions for any exception during execution. + * + * @param string $errorOutput + * + * @return FailedException|SeparateProcessException + */ + private function createException($errorOutput) + { + if (strpos($errorOutput, FailedException::class) !== 0) { + return new SeparateProcessException($errorOutput); + } + + $errorOutput = trim(str_replace(FailedException::class, '', $errorOutput)); + + return new FailedException(new SeparateProcessException($errorOutput)); + } } diff --git a/src/Resources/config/doctrine/TaskExecution.orm.xml b/src/Resources/config/doctrine/TaskExecution.orm.xml index ac1aa8a..54ece00 100644 --- a/src/Resources/config/doctrine/TaskExecution.orm.xml +++ b/src/Resources/config/doctrine/TaskExecution.orm.xml @@ -22,7 +22,8 @@ - + + diff --git a/src/Resources/config/executor/separate.xml b/src/Resources/config/executor/separate.xml index 217f23f..040a447 100644 --- a/src/Resources/config/executor/separate.xml +++ b/src/Resources/config/executor/separate.xml @@ -4,6 +4,12 @@ xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> + + + + + + %task.executor.console_path% %kernel.environment%% diff --git a/src/Resources/config/task_event_listener.xml b/src/Resources/config/task_event_listener.xml index 184eb25..7dad9b1 100644 --- a/src/Resources/config/task_event_listener.xml +++ b/src/Resources/config/task_event_listener.xml @@ -11,6 +11,7 @@ Task\Event\Events::TASK_FINISHED Task\Event\Events::TASK_PASSED Task\Event\Events::TASK_FAILED + Task\Event\Events::TASK_RETRIED diff --git a/tests/Unit/Executor/SeparateProcessExecutorTest.php b/tests/Unit/Executor/SeparateProcessExecutorTest.php new file mode 100644 index 0000000..e7311c2 --- /dev/null +++ b/tests/Unit/Executor/SeparateProcessExecutorTest.php @@ -0,0 +1,205 @@ +handlerFactory = $this->prophesize(TaskHandlerFactoryInterface::class); + $this->executionRepository = $this->prophesize(TaskExecutionRepositoryInterface::class); + $this->processFactory = $this->prophesize(ExecutionProcessFactory::class); + + $this->executor = new SeparateProcessExecutor( + $this->handlerFactory->reveal(), $this->executionRepository->reveal(), $this->processFactory->reveal() + ); + + $this->handler = $this->prophesize(TaskHandlerInterface::class); + $this->retryHandler = $this->prophesize(TaskHandlerInterface::class); + $this->retryHandler->willImplement(RetryTaskHandlerInterface::class); + + $this->handlerFactory->create('TaskHandler')->willReturn($this->handler->reveal()); + $this->handlerFactory->create('RetryTaskHandler')->willReturn($this->retryHandler->reveal()); + + $this->execution = $this->prophesize(TaskExecutionInterface::class); + $this->execution->getUuid()->willReturn('123-123-123'); + $this->execution->getAttempts()->willReturn(1); + + $this->process = $this->prophesize(Process::class); + $this->processFactory->create('123-123-123')->willReturn($this->process->reveal()); + } + + public function testExecute() + { + $this->execution->getHandlerClass()->willReturn('TaskHandler'); + + $this->process->run()->shouldBeCalled(); + $this->process->isSuccessful()->willReturn(true); + + $this->process->getOutput()->willReturn('TEST'); + + $result = $this->executor->execute($this->execution->reveal()); + $this->assertEquals('TEST', $result); + } + + public function testExecuteException() + { + $this->execution->getHandlerClass()->willReturn('TaskHandler'); + + $this->process->run()->shouldBeCalled(); + $this->process->isSuccessful()->willReturn(false); + + $this->process->getErrorOutput()->willReturn('TEST'); + + try { + $this->executor->execute($this->execution->reveal()); + + $this->fail('No FailedException was thrown'); + } catch (\Exception $exception) { + $this->assertInstanceOf(FailedException::class, $exception); + $this->assertInstanceOf(SeparateProcessException::class, $exception->getPrevious()); + $this->assertEquals('TEST', $exception->getPrevious()->__toString()); + } + } + + public function testExecuteFailedException() + { + $this->execution->getHandlerClass()->willReturn('TaskHandler'); + + $this->process->run()->shouldBeCalled(); + $this->process->isSuccessful()->willReturn(false); + + $this->process->getErrorOutput()->willReturn(FailedException::class . PHP_EOL . 'TEST'); + + try { + $this->executor->execute($this->execution->reveal()); + + $this->fail('No FailedException was thrown'); + } catch (\Exception $exception) { + $this->assertInstanceOf(FailedException::class, $exception); + $this->assertInstanceOf(SeparateProcessException::class, $exception->getPrevious()); + $this->assertEquals('TEST', $exception->getPrevious()->__toString()); + } + } + + public function testExecuteRetryFailedException() + { + $this->execution->getHandlerClass()->willReturn('RetryTaskHandler'); + $this->retryHandler->getMaximumAttempts()->willReturn(3); + + $this->execution->incrementAttempts()->shouldNotBeCalled(); + + $this->process->run()->shouldBeCalledTimes(1); + $this->process->isSuccessful()->willReturn(false); + + $this->process->getErrorOutput()->willReturn(FailedException::class . PHP_EOL . 'TEST'); + + try { + $this->executor->execute($this->execution->reveal()); + + $this->fail('No FailedException was thrown'); + } catch (\Exception $exception) { + $this->assertInstanceOf(FailedException::class, $exception); + $this->assertInstanceOf(SeparateProcessException::class, $exception->getPrevious()); + $this->assertEquals('TEST', $exception->getPrevious()->__toString()); + } + } + + public function testExecuteRetryException() + { + $this->execution->getHandlerClass()->willReturn('RetryTaskHandler'); + $this->retryHandler->getMaximumAttempts()->willReturn(3); + + $attempts = 1; + $this->execution->incrementAttempts()->will( + function () use (&$attempts) { + ++$attempts; + + return $this; + } + ); + $this->execution->getAttempts()->will( + function () use (&$attempts) { + return $attempts; + } + ); + + $this->executionRepository->save($this->execution->reveal())->shouldBeCalled(2); + + $this->process->run()->shouldBeCalledTimes(3); + $this->process->isSuccessful()->willReturn(false); + + $this->process->getErrorOutput()->willReturn('TEST'); + + try { + $this->executor->execute($this->execution->reveal()); + + $this->fail('No FailedException was thrown'); + } catch (\Exception $exception) { + $this->assertInstanceOf(FailedException::class, $exception); + $this->assertInstanceOf(SeparateProcessException::class, $exception->getPrevious()); + $this->assertEquals('TEST', $exception->getPrevious()->__toString()); + $this->assertEquals(3, $attempts); + } + } +}