From 310b42209d0ca8aa88b4cf86778ed6566807ad37 Mon Sep 17 00:00:00 2001 From: Michael Woodward Date: Fri, 4 Jun 2021 20:40:27 +0100 Subject: [PATCH 1/2] Cleanup temp FS writes - prefix temp dir with php-school - fire application.tear-down on all app exits - listener on application.tear-down rm's temp/php-school --- app/config.php | 9 +++++++ src/Application.php | 3 +++ src/Exercise/ExerciseType.php | 3 +++ src/Listener/TearDownListener.php | 27 +++++++++++++++++++ src/Solution/InTempSolutionMapper.php | 2 +- src/Utils/System.php | 2 +- test/Check/DatabaseCheckTest.php | 2 +- test/Listener/TearDownListenerTest.php | 30 ++++++++++++++++++++++ test/Solution/InTempSolutionMapperTest.php | 6 ++--- test/Utils/SystemTest.php | 4 +-- 10 files changed, 80 insertions(+), 8 deletions(-) create mode 100644 src/Listener/TearDownListener.php create mode 100644 test/Listener/TearDownListenerTest.php diff --git a/app/config.php b/app/config.php index 5cf7fe65..d49bd9f0 100644 --- a/app/config.php +++ b/app/config.php @@ -4,6 +4,7 @@ use Colors\Color; use PhpSchool\PhpWorkshop\Listener\InitialCodeListener; +use PhpSchool\PhpWorkshop\Listener\TearDownListener; use PhpSchool\PhpWorkshop\Logger\ConsoleLogger; use PhpSchool\PhpWorkshop\Logger\Logger; use Psr\Log\LoggerInterface; @@ -242,6 +243,9 @@ ); }, RealPathListener::class => create(), + TearDownListener::class => function (ContainerInterface $c) { + return new TearDownListener($c->get(Filesystem::class)); + }, //checks FileExistsCheck::class => create(), @@ -414,6 +418,11 @@ function (CgiResult $result) use ($c) { 'exercise.selected' => [ containerListener(InitialCodeListener::class) ] + ], + 'cleanup-filesystem' => [ + 'application.tear-down' => [ + containerListener(TearDownListener::class, 'cleanupTempDir') + ] ] ], ]; diff --git a/src/Application.php b/src/Application.php index 71086e53..4e3c8a03 100644 --- a/src/Application.php +++ b/src/Application.php @@ -267,6 +267,9 @@ public function run(): int ); return 1; } + + $this->tearDown($container); + return $exitCode; } diff --git a/src/Exercise/ExerciseType.php b/src/Exercise/ExerciseType.php index 7baabc6f..94ea53e8 100644 --- a/src/Exercise/ExerciseType.php +++ b/src/Exercise/ExerciseType.php @@ -15,6 +15,9 @@ * $typeCustom = ExerciseType::CUSTOM(); * ``` * @extends Enum + * @method static self CLI() + * @method static self CGI() + * @method static self CUSTOM() */ class ExerciseType extends Enum { diff --git a/src/Listener/TearDownListener.php b/src/Listener/TearDownListener.php new file mode 100644 index 00000000..82dec2ea --- /dev/null +++ b/src/Listener/TearDownListener.php @@ -0,0 +1,27 @@ +filesystem = $filesystem; + } + + public function cleanupTempDir(): void + { + $this->filesystem->remove(System::tempDir()); + } +} diff --git a/src/Solution/InTempSolutionMapper.php b/src/Solution/InTempSolutionMapper.php index b63517fc..648bb1ab 100644 --- a/src/Solution/InTempSolutionMapper.php +++ b/src/Solution/InTempSolutionMapper.php @@ -52,6 +52,6 @@ public static function mapFile(string $file): string private static function getDeterministicTempDir(string $path): string { - return Path::join(System::tempDir(), 'php-school', md5($path)); + return Path::join(System::tempDir(), md5($path)); } } diff --git a/src/Utils/System.php b/src/Utils/System.php index ada58ec1..5dc80128 100644 --- a/src/Utils/System.php +++ b/src/Utils/System.php @@ -21,6 +21,6 @@ public static function realpath(string $path): string public static function tempDir(string $path = ''): string { - return Path::join(self::realpath(sys_get_temp_dir()), $path); + return Path::join(self::realpath(sys_get_temp_dir()), 'php-school', $path); } } diff --git a/test/Check/DatabaseCheckTest.php b/test/Check/DatabaseCheckTest.php index 4413912b..b1e6ca15 100644 --- a/test/Check/DatabaseCheckTest.php +++ b/test/Check/DatabaseCheckTest.php @@ -58,7 +58,7 @@ public function setUp(): void $this->exercise = $this->createMock(DatabaseExerciseInterface::class); $this->exercise->method('getType')->willReturn(ExerciseType::CLI()); $this->dbDir = sprintf( - '%s/PhpSchool_PhpWorkshop_Check_DatabaseCheck', + '%s/php-school/PhpSchool_PhpWorkshop_Check_DatabaseCheck', str_replace('\\', '/', realpath(sys_get_temp_dir())) ); diff --git a/test/Listener/TearDownListenerTest.php b/test/Listener/TearDownListenerTest.php new file mode 100644 index 00000000..45ea4d1f --- /dev/null +++ b/test/Listener/TearDownListenerTest.php @@ -0,0 +1,30 @@ +cleanupTempDir(); + + self::assertFileDoesNotExist($tempDir . '/some.file'); + self::assertFileDoesNotExist($tempDir . '/some/path/another.file'); + } +} diff --git a/test/Solution/InTempSolutionMapperTest.php b/test/Solution/InTempSolutionMapperTest.php index de6e3253..945ddd11 100644 --- a/test/Solution/InTempSolutionMapperTest.php +++ b/test/Solution/InTempSolutionMapperTest.php @@ -27,7 +27,7 @@ public function testFileMapping(): void self::assertFileExists($mappedFile); self::assertNotSame($filePath, $mappedFile); - self::assertStringContainsString(System::tempDir('php-school'), $mappedFile); + self::assertStringContainsString(System::tempDir(), $mappedFile); } public function testDirectoryMapping(): void @@ -42,7 +42,7 @@ public function testDirectoryMapping(): void self::assertFileExists(Path::join($mappedDir, 'test.file')); self::assertFileExists(Path::join($mappedDir, 'innerDir', 'test.file')); self::assertNotSame($this->getTemporaryDirectory(), $mappedDir); - self::assertStringContainsString(System::tempDir('php-school'), $mappedDir); + self::assertStringContainsString(System::tempDir(), $mappedDir); } public function testMappingIsDeterministicTempDir(): void @@ -58,7 +58,7 @@ public function testMappingIsDeterministicTempDir(): void self::assertSame( InTempSolutionMapper::mapFile($filePath), - Path::join(System::tempDir(), 'php-school', $fileHash, 'test.file') + Path::join(System::tempDir(), $fileHash, 'test.file') ); self::assertNotSame( diff --git a/test/Utils/SystemTest.php b/test/Utils/SystemTest.php index 9d4715fa..96759cd6 100644 --- a/test/Utils/SystemTest.php +++ b/test/Utils/SystemTest.php @@ -25,12 +25,12 @@ public function testRealpathReturnsFullPath(): void public function testTempDir(): void { - self::assertSame(realpath(sys_get_temp_dir()), System::tempDir()); + self::assertSame(realpath(sys_get_temp_dir()) . '/php-school', System::tempDir()); } public function testTempDirWithPath(): void { - $expect = sprintf('%s/%s', realpath(sys_get_temp_dir()), 'test'); + $expect = sprintf('%s/php-school/%s', realpath(sys_get_temp_dir()), 'test'); self::assertSame($expect, System::tempDir('test')); } } From 468954b2f370486dcd17c28254950963efb044f1 Mon Sep 17 00:00:00 2001 From: Aydin Hassan Date: Fri, 4 Jun 2021 23:06:13 +0100 Subject: [PATCH 2/2] Only delete or create stuff if exists or doesn't exist already --- src/Check/DatabaseCheck.php | 11 +++++++++-- test/BaseTest.php | 4 +++- test/Check/DatabaseCheckTest.php | 2 +- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Check/DatabaseCheck.php b/src/Check/DatabaseCheck.php index 2eba2878..118eca13 100644 --- a/src/Check/DatabaseCheck.php +++ b/src/Check/DatabaseCheck.php @@ -138,10 +138,17 @@ function (CliExecuteEvent $e) { ], function () use ($db) { unset($db); - @unlink($this->userDatabasePath); - @unlink($this->solutionDatabasePath); + $this->unlink($this->userDatabasePath); + $this->unlink($this->solutionDatabasePath); rmdir($this->databaseDirectory); } ); } + + private function unlink(string $file): void + { + if (file_exists($file)) { + unlink($file); + } + } } diff --git a/test/BaseTest.php b/test/BaseTest.php index f7fe6cc0..d25b9061 100644 --- a/test/BaseTest.php +++ b/test/BaseTest.php @@ -31,7 +31,9 @@ public function getTemporaryFile(string $filename, string $content = null): stri return $file; } - @mkdir(dirname($file), 0777, true); + if (!file_exists(dirname($file))) { + mkdir(dirname($file), 0777, true); + } $content !== null ? file_put_contents($file, $content) diff --git a/test/Check/DatabaseCheckTest.php b/test/Check/DatabaseCheckTest.php index b1e6ca15..3e3db2b1 100644 --- a/test/Check/DatabaseCheckTest.php +++ b/test/Check/DatabaseCheckTest.php @@ -89,7 +89,7 @@ private function getRunnerManager(ExerciseInterface $exercise, EventDispatcher $ public function testIfDatabaseFolderExistsExceptionIsThrown(): void { $eventDispatcher = new EventDispatcher(new ResultAggregator()); - @mkdir($this->dbDir); + mkdir($this->dbDir, 0777, true); try { $this->check->attach($eventDispatcher); $this->fail('Exception was not thrown');