From b1358274c93c0795528b87682c85f8c62aad63be Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Sun, 5 May 2024 21:15:43 +0200 Subject: [PATCH 1/2] Seperate listeners and checks --- src/Exercise/AbstractExercise.php | 14 ++++++---- src/Exercise/ExerciseInterface.php | 15 ++++++---- src/ExerciseDispatcher.php | 8 +++--- test/Asset/CgiExerciseImpl.php | 9 +++++- test/Asset/CliExerciseImpl.php | 9 +++++- test/Asset/CliExerciseMissingInterface.php | 7 +++++ test/Asset/ComposerExercise.php | 9 ++++-- test/Asset/ExerciseWithInitialCode.php | 14 +++++++--- test/Asset/FileComparisonExercise.php | 14 +++++++--- test/Asset/FunctionRequirementsExercise.php | 17 +++++++---- test/Asset/PatchableExercise.php | 10 +++++-- test/Asset/ProvidesSolutionExercise.php | 16 +++++++---- test/Check/DatabaseCheckTest.php | 31 ++++++--------------- test/Exercise/AbstractExerciseTest.php | 7 +++-- 14 files changed, 115 insertions(+), 65 deletions(-) diff --git a/src/Exercise/AbstractExercise.php b/src/Exercise/AbstractExercise.php index ff790419..87a0b241 100644 --- a/src/Exercise/AbstractExercise.php +++ b/src/Exercise/AbstractExercise.php @@ -4,6 +4,8 @@ namespace PhpSchool\PhpWorkshop\Exercise; +use PhpSchool\PhpWorkshop\Check\FileComparisonCheck; +use PhpSchool\PhpWorkshop\Event\EventDispatcher; use PhpSchool\PhpWorkshop\ExerciseDispatcher; use PhpSchool\PhpWorkshop\Solution\SingleFileSolution; use PhpSchool\PhpWorkshop\Solution\SolutionInterface; @@ -78,12 +80,14 @@ public static function normaliseName(string $name): string } /** - * This method is implemented as empty by default, if you want to add additional checks or listen - * to events, you should override this method. - * - * @param ExerciseDispatcher $dispatcher + * @return list */ - public function configure(ExerciseDispatcher $dispatcher): void + public function getRequiredChecks(): array + { + return []; + } + + public function defineListeners(EventDispatcher $dispatcher): void { } } diff --git a/src/Exercise/ExerciseInterface.php b/src/Exercise/ExerciseInterface.php index 2a969359..10443374 100644 --- a/src/Exercise/ExerciseInterface.php +++ b/src/Exercise/ExerciseInterface.php @@ -4,6 +4,7 @@ namespace PhpSchool\PhpWorkshop\Exercise; +use PhpSchool\PhpWorkshop\Event\EventDispatcher; use PhpSchool\PhpWorkshop\ExerciseDispatcher; /** @@ -34,14 +35,16 @@ public function getType(): ExerciseType; public function getProblem(): string; /** - * This is where the exercise specifies the extra checks it may require. It is also - * possible to grab the event dispatcher from the exercise dispatcher and listen to any - * events. This method is automatically invoked just before verifying/running an student's solution - * to an exercise. + * Subscribe to events triggered throughout the verification process + */ + public function defineListeners(EventDispatcher $dispatcher): void; + + /** + * This is where the exercise specifies the extra checks it may require. * - * @param ExerciseDispatcher $dispatcher + * @return array */ - public function configure(ExerciseDispatcher $dispatcher): void; + public function getRequiredChecks(): array; /** * A short description of the exercise. diff --git a/src/ExerciseDispatcher.php b/src/ExerciseDispatcher.php index 6d9348a6..112fd68b 100644 --- a/src/ExerciseDispatcher.php +++ b/src/ExerciseDispatcher.php @@ -129,11 +129,11 @@ public function requireCheck(string $requiredCheck): void */ public function verify(ExerciseInterface $exercise, Input $input): ResultAggregator { - $exercise->configure($this); - $runner = $this->runnerManager->getRunner($exercise); - foreach ($runner->getRequiredChecks() as $requiredCheck) { + $exercise->defineListeners($this->eventDispatcher); + + foreach ([...$runner->getRequiredChecks(), ...$exercise->getRequiredChecks()] as $requiredCheck) { $this->requireCheck($requiredCheck); } @@ -181,7 +181,7 @@ public function verify(ExerciseInterface $exercise, Input $input): ResultAggrega */ public function run(ExerciseInterface $exercise, Input $input, OutputInterface $output): bool { - $exercise->configure($this); + $exercise->defineListeners($this->eventDispatcher); /** @var PhpLintCheck $lint */ $lint = $this->checkRepository->getByClass(PhpLintCheck::class); diff --git a/test/Asset/CgiExerciseImpl.php b/test/Asset/CgiExerciseImpl.php index 45c78005..d8c23c96 100644 --- a/test/Asset/CgiExerciseImpl.php +++ b/test/Asset/CgiExerciseImpl.php @@ -2,6 +2,8 @@ namespace PhpSchool\PhpWorkshopTest\Asset; +use PhpSchool\PhpWorkshop\Check\FileComparisonCheck; +use PhpSchool\PhpWorkshop\Event\EventDispatcher; use PhpSchool\PhpWorkshop\Exercise\CgiExercise; use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; use PhpSchool\PhpWorkshop\Exercise\ExerciseType; @@ -62,7 +64,12 @@ public function getType(): ExerciseType return ExerciseType::CGI(); } - public function configure(ExerciseDispatcher $dispatcher): void + public function getRequiredChecks(): array + { + return []; + } + + public function defineListeners(EventDispatcher $dispatcher): void { } } diff --git a/test/Asset/CliExerciseImpl.php b/test/Asset/CliExerciseImpl.php index df157189..0fcc9e32 100644 --- a/test/Asset/CliExerciseImpl.php +++ b/test/Asset/CliExerciseImpl.php @@ -2,6 +2,8 @@ namespace PhpSchool\PhpWorkshopTest\Asset; +use PhpSchool\PhpWorkshop\Check\FileComparisonCheck; +use PhpSchool\PhpWorkshop\Event\EventDispatcher; use PhpSchool\PhpWorkshop\Exercise\CliExercise; use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; use PhpSchool\PhpWorkshop\Exercise\ExerciseType; @@ -66,7 +68,12 @@ public function getType(): ExerciseType return ExerciseType::CLI(); } - public function configure(ExerciseDispatcher $dispatcher): void + public function getRequiredChecks(): array + { + return []; + } + + public function defineListeners(EventDispatcher $dispatcher): void { } } diff --git a/test/Asset/CliExerciseMissingInterface.php b/test/Asset/CliExerciseMissingInterface.php index 5edd2200..21c8d482 100644 --- a/test/Asset/CliExerciseMissingInterface.php +++ b/test/Asset/CliExerciseMissingInterface.php @@ -2,6 +2,8 @@ namespace PhpSchool\PhpWorkshopTest\Asset; +use PhpSchool\PhpWorkshop\Check\FileComparisonCheck; +use PhpSchool\PhpWorkshop\Event\EventDispatcher; use PhpSchool\PhpWorkshop\Exercise\AbstractExercise; use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; use PhpSchool\PhpWorkshop\Exercise\ExerciseType; @@ -31,4 +33,9 @@ public function getType(): ExerciseType { return ExerciseType::CLI(); } + + public function getRequiredChecks(): array + { + return []; + } } diff --git a/test/Asset/ComposerExercise.php b/test/Asset/ComposerExercise.php index 622a7de8..e7365893 100644 --- a/test/Asset/ComposerExercise.php +++ b/test/Asset/ComposerExercise.php @@ -3,6 +3,7 @@ namespace PhpSchool\PhpWorkshopTest\Asset; use PhpSchool\PhpWorkshop\Check\ComposerCheck; +use PhpSchool\PhpWorkshop\Event\EventDispatcher; use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; use PhpSchool\PhpWorkshop\Exercise\ExerciseType; use PhpSchool\PhpWorkshop\ExerciseCheck\ComposerExerciseCheck; @@ -53,8 +54,12 @@ public function getType(): ExerciseType return ExerciseType::CLI(); } - public function configure(ExerciseDispatcher $dispatcher): void + public function getRequiredChecks(): array + { + return [ComposerCheck::class]; + } + + public function defineListeners(EventDispatcher $dispatcher): void { - $dispatcher->requireCheck(ComposerCheck::class); } } diff --git a/test/Asset/ExerciseWithInitialCode.php b/test/Asset/ExerciseWithInitialCode.php index 398b3785..49282605 100644 --- a/test/Asset/ExerciseWithInitialCode.php +++ b/test/Asset/ExerciseWithInitialCode.php @@ -2,6 +2,8 @@ namespace PhpSchool\PhpWorkshopTest\Asset; +use PhpSchool\PhpWorkshop\Check\FileComparisonCheck; +use PhpSchool\PhpWorkshop\Event\EventDispatcher; use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; use PhpSchool\PhpWorkshop\Exercise\ExerciseType; use PhpSchool\PhpWorkshop\Exercise\ProvidesInitialCode; @@ -41,13 +43,17 @@ public function getType(): ExerciseType // TODO: Implement getType() method. } - public function configure(ExerciseDispatcher $dispatcher): void + public function getInitialCode(): SolutionInterface { - // TODO: Implement configure() method. + return SingleFileSolution::fromFile(__DIR__ . '/initial-code/init-solution.php'); } - public function getInitialCode(): SolutionInterface + public function getRequiredChecks(): array + { + return []; + } + + public function defineListeners(EventDispatcher $dispatcher): void { - return SingleFileSolution::fromFile(__DIR__ . '/initial-code/init-solution.php'); } } diff --git a/test/Asset/FileComparisonExercise.php b/test/Asset/FileComparisonExercise.php index b4ecd9c1..8627a37c 100644 --- a/test/Asset/FileComparisonExercise.php +++ b/test/Asset/FileComparisonExercise.php @@ -3,6 +3,8 @@ namespace PhpSchool\PhpWorkshopTest\Asset; use PhpSchool\PhpWorkshop\Check\ComposerCheck; +use PhpSchool\PhpWorkshop\Check\FileComparisonCheck; +use PhpSchool\PhpWorkshop\Event\EventDispatcher; use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; use PhpSchool\PhpWorkshop\Exercise\ExerciseType; use PhpSchool\PhpWorkshop\ExerciseCheck\FileComparisonExerciseCheck; @@ -66,13 +68,17 @@ public function getType(): ExerciseType return ExerciseType::CLI(); } - public function configure(ExerciseDispatcher $dispatcher): void + public function getFilesToCompare(): array { - $dispatcher->requireCheck(ComposerCheck::class); + return $this->files; } - public function getFilesToCompare(): array + public function getRequiredChecks(): array + { + return [FileComparisonCheck::class]; + } + + public function defineListeners(EventDispatcher $dispatcher): void { - return $this->files; } } diff --git a/test/Asset/FunctionRequirementsExercise.php b/test/Asset/FunctionRequirementsExercise.php index 927d6f56..4e243a3f 100644 --- a/test/Asset/FunctionRequirementsExercise.php +++ b/test/Asset/FunctionRequirementsExercise.php @@ -3,6 +3,9 @@ namespace PhpSchool\PhpWorkshopTest\Asset; use PhpSchool\PhpWorkshop\Check\ComposerCheck; +use PhpSchool\PhpWorkshop\Check\FileComparisonCheck; +use PhpSchool\PhpWorkshop\Check\FunctionRequirementsCheck; +use PhpSchool\PhpWorkshop\Event\EventDispatcher; use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; use PhpSchool\PhpWorkshop\Exercise\ExerciseType; use PhpSchool\PhpWorkshop\ExerciseCheck\FunctionRequirementsExerciseCheck; @@ -45,11 +48,6 @@ public function getType(): ExerciseType return ExerciseType::CLI(); } - public function configure(ExerciseDispatcher $dispatcher): void - { - $dispatcher->requireCheck(ComposerCheck::class); - } - /** * @return string[] */ @@ -65,4 +63,13 @@ public function getBannedFunctions(): array { return ['file']; } + + public function getRequiredChecks(): array + { + return [FunctionRequirementsCheck::class]; + } + + public function defineListeners(EventDispatcher $dispatcher): void + { + } } diff --git a/test/Asset/PatchableExercise.php b/test/Asset/PatchableExercise.php index 51a9d601..c232113e 100644 --- a/test/Asset/PatchableExercise.php +++ b/test/Asset/PatchableExercise.php @@ -2,6 +2,8 @@ namespace PhpSchool\PhpWorkshopTest\Asset; +use PhpSchool\PhpWorkshop\Check\FileComparisonCheck; +use PhpSchool\PhpWorkshop\Event\EventDispatcher; use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; use PhpSchool\PhpWorkshop\Exercise\ExerciseType; use PhpSchool\PhpWorkshop\Exercise\SubmissionPatchable; @@ -45,8 +47,12 @@ public function getType(): ExerciseType // TODO: Implement getType() method. } - public function configure(ExerciseDispatcher $dispatcher): void + public function getRequiredChecks(): array + { + return []; + } + + public function defineListeners(EventDispatcher $dispatcher): void { - // TODO: Implement configure() method. } } diff --git a/test/Asset/ProvidesSolutionExercise.php b/test/Asset/ProvidesSolutionExercise.php index b86f8dd9..503e1bf8 100644 --- a/test/Asset/ProvidesSolutionExercise.php +++ b/test/Asset/ProvidesSolutionExercise.php @@ -4,6 +4,8 @@ namespace PhpSchool\PhpWorkshopTest\Asset; +use PhpSchool\PhpWorkshop\Check\FileComparisonCheck; +use PhpSchool\PhpWorkshop\Event\EventDispatcher; use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; use PhpSchool\PhpWorkshop\Exercise\ExerciseType; use PhpSchool\PhpWorkshop\Exercise\ProvidesSolution; @@ -28,11 +30,6 @@ public function getProblem(): string // TODO: Implement getProblem() method. } - public function configure(ExerciseDispatcher $dispatcher): void - { - // TODO: Implement configure() method. - } - public function getDescription(): string { // TODO: Implement getDescription() method. @@ -47,4 +44,13 @@ public function getSolution(): SolutionInterface { return SingleFileSolution::fromFile(__DIR__ . '/provided-solution/solution.php'); } + + public function getRequiredChecks(): array + { + return []; + } + + public function defineListeners(EventDispatcher $dispatcher): void + { + } } diff --git a/test/Check/DatabaseCheckTest.php b/test/Check/DatabaseCheckTest.php index 14629e90..da96cd7f 100644 --- a/test/Check/DatabaseCheckTest.php +++ b/test/Check/DatabaseCheckTest.php @@ -131,10 +131,8 @@ public function testIfPDOThrowsExceptionItCleansUp(): void $this->exercise ->expects($this->once()) - ->method('configure') - ->willReturnCallback(function (ExerciseDispatcher $dispatcher) { - $dispatcher->requireCheck(DatabaseCheck::class); - }); + ->method('getRequiredChecks') + ->willReturn([DatabaseCheck::class]); $this->exercise ->expects($this->once()) @@ -172,10 +170,8 @@ public function testSuccessIsReturnedIfDatabaseVerificationPassed(): void $this->exercise ->expects($this->once()) - ->method('configure') - ->willReturnCallback(function (ExerciseDispatcher $dispatcher) { - $dispatcher->requireCheck(DatabaseCheck::class); - }); + ->method('getRequiredChecks') + ->willReturn([DatabaseCheck::class]); $this->exercise ->expects($this->once()) @@ -207,13 +203,6 @@ public function testRunExercise(): void ->method('getArgs') ->willReturn([]); - $this->exercise - ->expects($this->once()) - ->method('configure') - ->willReturnCallback(function (ExerciseDispatcher $dispatcher) { - $dispatcher->requireCheck(DatabaseCheck::class); - }); - $this->checkRepository->registerCheck($this->check); $results = new ResultAggregator(); @@ -248,10 +237,8 @@ public function testFailureIsReturnedIfDatabaseVerificationFails(): void $this->exercise ->expects($this->once()) - ->method('configure') - ->willReturnCallback(function (ExerciseDispatcher $dispatcher) { - $dispatcher->requireCheck(DatabaseCheck::class); - }); + ->method('getRequiredChecks') + ->willReturn([DatabaseCheck::class]); $this->exercise ->expects($this->once()) @@ -296,10 +283,8 @@ public function testAlteringDatabaseInSolutionDoesNotEffectDatabaseInUserSolutio $this->exercise ->expects($this->once()) - ->method('configure') - ->willReturnCallback(function (ExerciseDispatcher $dispatcher) { - $dispatcher->requireCheck(DatabaseCheck::class); - }); + ->method('getRequiredChecks') + ->willReturn([DatabaseCheck::class]); $this->exercise ->expects($this->once()) diff --git a/test/Exercise/AbstractExerciseTest.php b/test/Exercise/AbstractExerciseTest.php index 6b2d722c..c500b0f7 100644 --- a/test/Exercise/AbstractExerciseTest.php +++ b/test/Exercise/AbstractExerciseTest.php @@ -2,6 +2,7 @@ namespace PhpSchool\PhpWorkshopTest\Exercise; +use PhpSchool\PhpWorkshop\Event\EventDispatcher; use PhpSchool\PhpWorkshop\ExerciseDispatcher; use PhpSchool\PhpWorkshop\Solution\SolutionFile; use PhpSchool\PhpWorkshop\Solution\SolutionInterface; @@ -65,11 +66,11 @@ public function problemProvider(): array ]; } - public function testConfigureDoesNothing(): void + public function testDefineListenersDoesNothing(): void { - $dispatcher = $this->createMock(ExerciseDispatcher::class); + $dispatcher = $this->createMock(EventDispatcher::class); $exercise = new AbstractExerciseImpl('Array We Go'); - $this->assertNull($exercise->configure($dispatcher)); + $this->assertNull($exercise->defineListeners($dispatcher)); } } From 9cdd0ec69fea50c2015931aa5e5e6e2595e6917a Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Sun, 5 May 2024 21:56:09 +0200 Subject: [PATCH 2/2] Use new process factory --- app/config.php | 10 +- src/ExerciseRunner/CgiRunner.php | 91 +++++------- src/ExerciseRunner/CliRunner.php | 131 ++++++------------ .../Factory/CgiRunnerFactory.php | 16 +-- .../Factory/CliRunnerFactory.php | 16 +-- src/ExerciseRunner/RunnerManager.php | 2 +- test/Check/DatabaseCheckTest.php | 5 +- test/ExerciseRunner/CgiRunnerTest.php | 16 +-- test/ExerciseRunner/CliRunnerTest.php | 18 +-- .../Factory/CgiRunnerFactoryTest.php | 14 +- .../Factory/CliRunnerFactoryTest.php | 14 +- 11 files changed, 116 insertions(+), 217 deletions(-) diff --git a/app/config.php b/app/config.php index ab0aa126..39cea60d 100644 --- a/app/config.php +++ b/app/config.php @@ -71,6 +71,8 @@ use PhpSchool\PhpWorkshop\Output\OutputInterface; use PhpSchool\PhpWorkshop\Output\StdOutput; use PhpSchool\PhpWorkshop\Patch; +use PhpSchool\PhpWorkshop\Process\HostProcessFactory; +use PhpSchool\PhpWorkshop\Process\ProcessFactory; use PhpSchool\PhpWorkshop\Result\Cgi\CgiResult; use PhpSchool\PhpWorkshop\Result\Cgi\GenericFailure as CgiGenericFailure; use PhpSchool\PhpWorkshop\Result\Cgi\RequestFailure as CgiRequestFailure; @@ -187,12 +189,16 @@ //Exercise Runners RunnerManager::class => function (ContainerInterface $c) { $manager = new RunnerManager(); - $manager->addFactory(new CliRunnerFactory($c->get(EventDispatcher::class))); - $manager->addFactory(new CgiRunnerFactory($c->get(EventDispatcher::class))); + $manager->addFactory(new CliRunnerFactory($c->get(EventDispatcher::class), $c->get(ProcessFactory::class))); + $manager->addFactory(new CgiRunnerFactory($c->get(EventDispatcher::class), $c->get(ProcessFactory::class))); $manager->addFactory(new CustomVerifyingRunnerFactory()); return $manager; }, + ProcessFactory::class => function (ContainerInterface $c) { + return new HostProcessFactory(); + }, + //commands MenuCommand::class => function (ContainerInterface $c) { return new MenuCommand($c->get('menu')); diff --git a/src/ExerciseRunner/CgiRunner.php b/src/ExerciseRunner/CgiRunner.php index e29e3348..692e7891 100644 --- a/src/ExerciseRunner/CgiRunner.php +++ b/src/ExerciseRunner/CgiRunner.php @@ -19,6 +19,8 @@ use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; use PhpSchool\PhpWorkshop\Input\Input; use PhpSchool\PhpWorkshop\Output\OutputInterface; +use PhpSchool\PhpWorkshop\Process\ProcessFactory; +use PhpSchool\PhpWorkshop\Process\ProcessInput; use PhpSchool\PhpWorkshop\Result\Cgi\CgiResult; use PhpSchool\PhpWorkshop\Result\Cgi\RequestFailure; use PhpSchool\PhpWorkshop\Result\Cgi\GenericFailure; @@ -32,31 +34,18 @@ use Symfony\Component\Process\ExecutableFinder; use Symfony\Component\Process\Process; +use function PHPStan\dumpType; + /** * The `CGI` runner. This runner executes solutions as if they were behind a web-server. They populate the `$_SERVER`, * `$_GET` & `$_POST` super globals with information based of the request objects returned from the exercise. */ class CgiRunner implements ExerciseRunnerInterface { - /** - * @var CgiExercise&ExerciseInterface - */ - private $exercise; - - /** - * @var EventDispatcher - */ - private $eventDispatcher; - - /** - * @var string - */ - private $phpLocation; - /** * @var array */ - private static $requiredChecks = [ + private static array $requiredChecks = [ FileExistsCheck::class, CodeExistsCheck::class, PhpLintCheck::class, @@ -68,26 +57,13 @@ class CgiRunner implements ExerciseRunnerInterface * be available. It will check for it's existence in the system's $PATH variable or the same * folder that the CLI php binary lives in. * - * @param CgiExercise $exercise The exercise to be invoked. - * @param EventDispatcher $eventDispatcher The event dispatcher. + * @param CgiExercise&ExerciseInterface $exercise The exercise to be invoked. */ public function __construct( - CgiExercise $exercise, - EventDispatcher $eventDispatcher + private CgiExercise $exercise, + private EventDispatcher $eventDispatcher, + private ProcessFactory $processFactory ) { - $php = (new ExecutableFinder())->find('php-cgi'); - - if (null === $php) { - throw new RuntimeException( - 'Could not load php-cgi binary. Please install php using your package manager.' - ); - } - - $this->phpLocation = $php; - - /** @var CgiExercise&ExerciseInterface $exercise */ - $this->eventDispatcher = $eventDispatcher; - $this->exercise = $exercise; } /** @@ -172,7 +148,7 @@ private function getHeaders(ResponseInterface $response): array */ private function executePhpFile(string $fileName, RequestInterface $request, string $type): ResponseInterface { - $process = $this->getProcess($fileName, $request); + $process = $this->getPhpProcess(dirname($fileName), basename($fileName), $request); $process->start(); $this->eventDispatcher->dispatch(new CgiExecuteEvent(sprintf('cgi.verify.%s.executing', $type), $request)); @@ -196,47 +172,38 @@ private function executePhpFile(string $fileName, RequestInterface $request, str * @param RequestInterface $request * @return Process */ - private function getProcess(string $fileName, RequestInterface $request): Process + private function getPhpProcess(string $workingDirectory, string $fileName, RequestInterface $request): Process { - $env = $this->getDefaultEnv(); - $env += [ + $env = [ 'REQUEST_METHOD' => $request->getMethod(), 'SCRIPT_FILENAME' => $fileName, - 'REDIRECT_STATUS' => 302, + 'REDIRECT_STATUS' => '302', 'QUERY_STRING' => $request->getUri()->getQuery(), 'REQUEST_URI' => $request->getUri()->getPath(), 'XDEBUG_MODE' => 'off', ]; - $cgiBinary = sprintf( - '%s -dalways_populate_raw_post_data=-1 -dhtml_errors=0 -dexpose_php=0', - $this->phpLocation - ); - $content = $request->getBody()->__toString(); - $cmd = sprintf('echo %s | %s', escapeshellarg($content), $cgiBinary); - $env['CONTENT_LENGTH'] = $request->getBody()->getSize(); + $env['CONTENT_LENGTH'] = (string) $request->getBody()->getSize(); $env['CONTENT_TYPE'] = $request->getHeaderLine('Content-Type'); foreach ($request->getHeaders() as $name => $values) { $env[sprintf('HTTP_%s', strtoupper($name))] = implode(", ", $values); } - return Process::fromShellCommandline($cmd, null, $env, null, 10); - } - - /** - * We need to reset env entirely, because Symfony inherits it. We do that by setting all - * the current env vars to false - * - * @return array - */ - private function getDefaultEnv(): array - { - $env = array_map(fn () => false, $_ENV); - $env + array_map(fn () => false, $_SERVER); + $processInput = new ProcessInput( + 'php-cgi', + [ + '-dalways_populate_raw_post_data=-1', + '-dhtml_errors=0', + '-dexpose_php=0', + ], + $workingDirectory, + $env, + $content + ); - return $env; + return $this->processFactory->create($processInput); } /** @@ -297,7 +264,11 @@ public function run(Input $input, OutputInterface $output): bool $event = $this->eventDispatcher->dispatch( new CgiExecuteEvent('cgi.run.student-execute.pre', $request) ); - $process = $this->getProcess($input->getRequiredArgument('program'), $event->getRequest()); + $process = $this->getPhpProcess( + dirname($input->getRequiredArgument('program')), + $input->getRequiredArgument('program'), + $event->getRequest() + ); $process->start(); $this->eventDispatcher->dispatch( diff --git a/src/ExerciseRunner/CliRunner.php b/src/ExerciseRunner/CliRunner.php index 4750405e..b56ff1d2 100644 --- a/src/ExerciseRunner/CliRunner.php +++ b/src/ExerciseRunner/CliRunner.php @@ -18,6 +18,8 @@ use PhpSchool\PhpWorkshop\Exercise\ExerciseInterface; use PhpSchool\PhpWorkshop\Input\Input; use PhpSchool\PhpWorkshop\Output\OutputInterface; +use PhpSchool\PhpWorkshop\Process\ProcessFactory; +use PhpSchool\PhpWorkshop\Process\ProcessInput; use PhpSchool\PhpWorkshop\Result\Cli\RequestFailure; use PhpSchool\PhpWorkshop\Result\Cli\CliResult; use PhpSchool\PhpWorkshop\Result\Cli\GenericFailure; @@ -25,6 +27,7 @@ use PhpSchool\PhpWorkshop\Result\Cli\ResultInterface as CliResultInterface; use PhpSchool\PhpWorkshop\Result\ResultInterface; use PhpSchool\PhpWorkshop\Utils\ArrayObject; +use PhpSchool\PhpWorkshop\Utils\Collection; use RuntimeException; use Symfony\Component\Process\ExecutableFinder; use Symfony\Component\Process\Process; @@ -39,25 +42,10 @@ */ class CliRunner implements ExerciseRunnerInterface { - /** - * @var CliExercise&ExerciseInterface - */ - private $exercise; - - /** - * @var EventDispatcher - */ - private $eventDispatcher; - - /** - * @var string - */ - private $phpLocation; - /** * @var array */ - private static $requiredChecks = [ + private static array $requiredChecks = [ FileExistsCheck::class, CodeExistsCheck::class, PhpLintCheck::class, @@ -67,24 +55,13 @@ class CliRunner implements ExerciseRunnerInterface /** * Requires the exercise instance and an event dispatcher. * - * @param CliExercise $exercise The exercise to be invoked. - * @param EventDispatcher $eventDispatcher The event dispatcher. + * @param CliExercise&ExerciseInterface $exercise The exercise to be invoked. */ - public function __construct(CliExercise $exercise, EventDispatcher $eventDispatcher) - { - $php = (new ExecutableFinder())->find('php'); - - if (null === $php) { - throw new RuntimeException( - 'Could not load php binary. Please install php using your package manager.' - ); - } - - $this->phpLocation = $php; - - /** @var CliExercise&ExerciseInterface $exercise */ - $this->eventDispatcher = $eventDispatcher; - $this->exercise = $exercise; + public function __construct( + private CliExercise $exercise, + private EventDispatcher $eventDispatcher, + private ProcessFactory $processFactory + ) { } /** @@ -105,59 +82,6 @@ public function getRequiredChecks(): array return self::$requiredChecks; } - /** - * @param string $fileName - * @param ArrayObject $args - * @param string $type - * @return string - */ - private function executePhpFile(string $fileName, ArrayObject $args, string $type): string - { - $process = $this->getPhpProcess($fileName, $args); - - $process->start(); - $this->eventDispatcher->dispatch(new CliExecuteEvent(sprintf('cli.verify.%s.executing', $type), $args)); - $process->wait(); - - if (!$process->isSuccessful()) { - throw CodeExecutionException::fromProcess($process); - } - - return $process->getOutput(); - } - - /** - * @param string $fileName - * @param ArrayObject $args - * - * @return Process - */ - private function getPhpProcess(string $fileName, ArrayObject $args): Process - { - return new Process( - $args->prepend($fileName)->prepend($this->phpLocation)->getArrayCopy(), - dirname($fileName), - $this->getDefaultEnv() + ['XDEBUG_MODE' => 'off'], - null, - 10 - ); - } - - /** - * We need to reset env entirely, because Symfony inherits it. We do that by setting all - * the current env vars to false - * - * @return array - */ - private function getDefaultEnv(): array - { - $env = array_map(fn () => false, $_ENV); - $env + array_map(fn () => false, $_SERVER); - - return $env; - } - - /** * Verifies a solution by invoking PHP from the CLI passing the arguments gathered from the exercise * as command line arguments to PHP. @@ -272,7 +196,12 @@ public function run(Input $input, OutputInterface $output): bool $args = $event->getArgs(); - $process = $this->getPhpProcess($input->getRequiredArgument('program'), $args); + $process = $this->getPhpProcess( + dirname($input->getRequiredArgument('program')), + $input->getRequiredArgument('program'), + $args + ); + $process->start(); $this->eventDispatcher->dispatch( new CliExecuteEvent('cli.run.student.executing', $args, ['output' => $output]) @@ -296,4 +225,32 @@ public function run(Input $input, OutputInterface $output): bool $this->eventDispatcher->dispatch(new ExerciseRunnerEvent('cli.run.finish', $this->exercise, $input)); return $success; } + + /** + * @param ArrayObject $args + */ + private function executePhpFile(string $fileName, ArrayObject $args, string $type): string + { + $process = $this->getPhpProcess(dirname($fileName), $fileName, $args); + + $process->start(); + $this->eventDispatcher->dispatch(new CliExecuteEvent(sprintf('cli.verify.%s.executing', $type), $args)); + $process->wait(); + + if (!$process->isSuccessful()) { + throw CodeExecutionException::fromProcess($process); + } + + return $process->getOutput(); + } + + /** + * @param ArrayObject $args + */ + private function getPhpProcess(string $workingDirectory, string $fileName, ArrayObject $args): Process + { + return $this->processFactory->create( + new ProcessInput('php', [$fileName, ...$args->getArrayCopy()], $workingDirectory, []) + ); + } } diff --git a/src/ExerciseRunner/Factory/CgiRunnerFactory.php b/src/ExerciseRunner/Factory/CgiRunnerFactory.php index a7917039..3b0deed3 100644 --- a/src/ExerciseRunner/Factory/CgiRunnerFactory.php +++ b/src/ExerciseRunner/Factory/CgiRunnerFactory.php @@ -12,6 +12,7 @@ use PhpSchool\PhpWorkshop\Exercise\ExerciseType; use PhpSchool\PhpWorkshop\ExerciseRunner\CgiRunner; use PhpSchool\PhpWorkshop\ExerciseRunner\ExerciseRunnerInterface; +use PhpSchool\PhpWorkshop\Process\ProcessFactory; use PhpSchool\PhpWorkshop\Utils\RequestRenderer; /** @@ -22,19 +23,10 @@ class CgiRunnerFactory implements ExerciseRunnerFactoryInterface /** * @var string */ - private static $type = ExerciseType::CGI; + private static string $type = ExerciseType::CGI; - /** - * @var EventDispatcher - */ - private $eventDispatcher; - - /** - * @param EventDispatcher $eventDispatcher - */ - public function __construct(EventDispatcher $eventDispatcher) + public function __construct(private EventDispatcher $eventDispatcher, private ProcessFactory $processFactory) { - $this->eventDispatcher = $eventDispatcher; } /** @@ -66,6 +58,6 @@ public function configureInput(CommandDefinition $commandDefinition): void */ public function create(ExerciseInterface $exercise): ExerciseRunnerInterface { - return new CgiRunner($exercise, $this->eventDispatcher); + return new CgiRunner($exercise, $this->eventDispatcher, $this->processFactory); } } diff --git a/src/ExerciseRunner/Factory/CliRunnerFactory.php b/src/ExerciseRunner/Factory/CliRunnerFactory.php index 9694d642..9b6eec2e 100644 --- a/src/ExerciseRunner/Factory/CliRunnerFactory.php +++ b/src/ExerciseRunner/Factory/CliRunnerFactory.php @@ -12,6 +12,7 @@ use PhpSchool\PhpWorkshop\Exercise\ExerciseType; use PhpSchool\PhpWorkshop\ExerciseRunner\CliRunner; use PhpSchool\PhpWorkshop\ExerciseRunner\ExerciseRunnerInterface; +use PhpSchool\PhpWorkshop\Process\ProcessFactory; /** * Factory class for `CliRunner` @@ -21,19 +22,10 @@ class CliRunnerFactory implements ExerciseRunnerFactoryInterface /** * @var string */ - private static $type = ExerciseType::CLI; + private static string $type = ExerciseType::CLI; - /** - * @var EventDispatcher - */ - private $eventDispatcher; - - /** - * @param EventDispatcher $eventDispatcher - */ - public function __construct(EventDispatcher $eventDispatcher) + public function __construct(private EventDispatcher $eventDispatcher, private ProcessFactory $processFactory) { - $this->eventDispatcher = $eventDispatcher; } /** @@ -65,6 +57,6 @@ public function configureInput(CommandDefinition $commandDefinition): void */ public function create(ExerciseInterface $exercise): ExerciseRunnerInterface { - return new CliRunner($exercise, $this->eventDispatcher); + return new CliRunner($exercise, $this->eventDispatcher, $this->processFactory); } } diff --git a/src/ExerciseRunner/RunnerManager.php b/src/ExerciseRunner/RunnerManager.php index 149022e0..d1b96f9a 100644 --- a/src/ExerciseRunner/RunnerManager.php +++ b/src/ExerciseRunner/RunnerManager.php @@ -17,7 +17,7 @@ class RunnerManager /** * @var array */ - private $factories = []; + private array $factories = []; /** * @param ExerciseRunnerFactoryInterface $factory diff --git a/test/Check/DatabaseCheckTest.php b/test/Check/DatabaseCheckTest.php index da96cd7f..70a40256 100644 --- a/test/Check/DatabaseCheckTest.php +++ b/test/Check/DatabaseCheckTest.php @@ -15,6 +15,7 @@ use PhpSchool\PhpWorkshop\ExerciseRunner\RunnerManager; use PhpSchool\PhpWorkshop\Input\Input; use PhpSchool\PhpWorkshop\Output\OutputInterface; +use PhpSchool\PhpWorkshop\Process\HostProcessFactory; use PhpSchool\PhpWorkshop\ResultAggregator; use PhpSchool\PhpWorkshop\Solution\SingleFileSolution; use PhpSchool\PhpWorkshopTest\Asset\DatabaseExerciseInterface; @@ -68,8 +69,8 @@ public function setUp(): void private function getRunnerManager(ExerciseInterface $exercise, EventDispatcher $eventDispatcher): MockObject { $runner = $this->getMockBuilder(CliRunner::class) - ->setConstructorArgs([$exercise, $eventDispatcher]) - ->setMethods(['configure', 'getRequiredChecks']) + ->setConstructorArgs([$exercise, $eventDispatcher, new HostProcessFactory()]) + ->onlyMethods(['getRequiredChecks']) ->getMock(); $runner diff --git a/test/ExerciseRunner/CgiRunnerTest.php b/test/ExerciseRunner/CgiRunnerTest.php index df8304a3..aade9085 100644 --- a/test/ExerciseRunner/CgiRunnerTest.php +++ b/test/ExerciseRunner/CgiRunnerTest.php @@ -6,6 +6,7 @@ use GuzzleHttp\Psr7\Request; use PhpSchool\PhpWorkshop\Check\CodeExistsCheck; use PhpSchool\PhpWorkshop\Listener\OutputRunInfoListener; +use PhpSchool\PhpWorkshop\Process\HostProcessFactory; use PhpSchool\Terminal\Terminal; use PhpSchool\PhpWorkshop\Check\CodeParseCheck; use PhpSchool\PhpWorkshop\Check\FileExistsCheck; @@ -23,6 +24,7 @@ use PhpSchool\PhpWorkshop\Solution\SingleFileSolution; use PhpSchool\PhpWorkshop\Utils\RequestRenderer; use PhpSchool\PhpWorkshopTest\Asset\CgiExerciseInterface; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Yoast\PHPUnitPolyfills\Polyfills\AssertionRenames; @@ -30,24 +32,18 @@ class CgiRunnerTest extends TestCase { use AssertionRenames; - /** @var CgiRunner */ - private $runner; - + private CgiRunner $runner; /** - * @var CgiExerciseInterface + * @var CgiExerciseInterface&MockObject */ private $exercise; - - /** - * @var EventDispatcher - */ - private $eventDispatcher; + private EventDispatcher $eventDispatcher; public function setUp(): void { $this->exercise = $this->createMock(CgiExerciseInterface::class); $this->eventDispatcher = new EventDispatcher(new ResultAggregator()); - $this->runner = new CgiRunner($this->exercise, $this->eventDispatcher); + $this->runner = new CgiRunner($this->exercise, $this->eventDispatcher, new HostProcessFactory()); $this->exercise ->method('getType') diff --git a/test/ExerciseRunner/CliRunnerTest.php b/test/ExerciseRunner/CliRunnerTest.php index 78bad5e0..ecbf0bc6 100644 --- a/test/ExerciseRunner/CliRunnerTest.php +++ b/test/ExerciseRunner/CliRunnerTest.php @@ -5,6 +5,7 @@ use Colors\Color; use PhpSchool\PhpWorkshop\Check\CodeExistsCheck; use PhpSchool\PhpWorkshop\Listener\OutputRunInfoListener; +use PhpSchool\PhpWorkshop\Process\HostProcessFactory; use PhpSchool\PhpWorkshop\Utils\RequestRenderer; use PhpSchool\Terminal\Terminal; use PhpSchool\PhpWorkshop\Check\CodeParseCheck; @@ -23,6 +24,7 @@ use PhpSchool\PhpWorkshop\ResultAggregator; use PhpSchool\PhpWorkshop\Solution\SingleFileSolution; use PhpSchool\PhpWorkshopTest\Asset\CliExerciseInterface; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Yoast\PHPUnitPolyfills\Polyfills\AssertionRenames; @@ -30,24 +32,18 @@ class CliRunnerTest extends TestCase { use AssertionRenames; - /** @var CliRunner */ - private $runner; - - /** - * @var CliExerciseInterface|\PHPUnit_Framework_MockObject_MockObject - */ - private $exercise; - + private CliRunner $runner; /** - * @var EventDispatcher + * @var CliExerciseInterface&MockObject */ - private $eventDispatcher; + private CliExerciseInterface $exercise; + private EventDispatcher $eventDispatcher; public function setUp(): void { $this->exercise = $this->createMock(CliExerciseInterface::class); $this->eventDispatcher = new EventDispatcher(new ResultAggregator()); - $this->runner = new CliRunner($this->exercise, $this->eventDispatcher); + $this->runner = new CliRunner($this->exercise, $this->eventDispatcher, new HostProcessFactory()); $this->exercise ->method('getType') diff --git a/test/ExerciseRunner/Factory/CgiRunnerFactoryTest.php b/test/ExerciseRunner/Factory/CgiRunnerFactoryTest.php index 03daf67e..9b334eae 100644 --- a/test/ExerciseRunner/Factory/CgiRunnerFactoryTest.php +++ b/test/ExerciseRunner/Factory/CgiRunnerFactoryTest.php @@ -8,26 +8,20 @@ use PhpSchool\PhpWorkshop\Exercise\ExerciseType; use PhpSchool\PhpWorkshop\ExerciseRunner\CgiRunner; use PhpSchool\PhpWorkshop\ExerciseRunner\Factory\CgiRunnerFactory; +use PhpSchool\PhpWorkshop\Process\HostProcessFactory; use PhpSchool\PhpWorkshop\Utils\RequestRenderer; use PhpSchool\PhpWorkshopTest\Asset\CgiExerciseImpl; use PHPUnit\Framework\TestCase; class CgiRunnerFactoryTest extends TestCase { - /** - * @var EventDispatcher - */ - private $eventDispatcher; - - /** - * @var CgiRunnerFactory - */ - private $factory; + private EventDispatcher $eventDispatcher; + private CgiRunnerFactory $factory; public function setUp(): void { $this->eventDispatcher = $this->createMock(EventDispatcher::class); - $this->factory = new CgiRunnerFactory($this->eventDispatcher); + $this->factory = new CgiRunnerFactory($this->eventDispatcher, new HostProcessFactory()); } public function testSupports(): void diff --git a/test/ExerciseRunner/Factory/CliRunnerFactoryTest.php b/test/ExerciseRunner/Factory/CliRunnerFactoryTest.php index 6678104d..8fc5eb58 100644 --- a/test/ExerciseRunner/Factory/CliRunnerFactoryTest.php +++ b/test/ExerciseRunner/Factory/CliRunnerFactoryTest.php @@ -8,25 +8,19 @@ use PhpSchool\PhpWorkshop\Exercise\ExerciseType; use PhpSchool\PhpWorkshop\ExerciseRunner\CliRunner; use PhpSchool\PhpWorkshop\ExerciseRunner\Factory\CliRunnerFactory; +use PhpSchool\PhpWorkshop\Process\HostProcessFactory; use PhpSchool\PhpWorkshopTest\Asset\CliExerciseImpl; use PHPUnit\Framework\TestCase; class CliRunnerFactoryTest extends TestCase { - /** - * @var EventDispatcher - */ - private $eventDispatcher; - - /** - * @var CliRunnerFactory - */ - private $factory; + private EventDispatcher $eventDispatcher; + private CliRunnerFactory $factory; public function setUp(): void { $this->eventDispatcher = $this->createMock(EventDispatcher::class); - $this->factory = new CliRunnerFactory($this->eventDispatcher); + $this->factory = new CliRunnerFactory($this->eventDispatcher, new HostProcessFactory()); } public function testSupports(): void