diff --git a/phpBB/phpbb/storage/adapter/adapter_interface.php b/phpBB/phpbb/storage/adapter/adapter_interface.php index 36642962ba..5bd6525a73 100644 --- a/phpBB/phpbb/storage/adapter/adapter_interface.php +++ b/phpBB/phpbb/storage/adapter/adapter_interface.php @@ -20,86 +20,81 @@ interface adapter_interface /** * Set adapter parameters * - * @param array options Storage-specific options. + * @param array options Storage-specific options. */ - public function configure($options); + public function configure(array $options): void; /** * Dumps content into a file * - * @param string path The file to be written to. - * @param string content The data to write into the file. - * - * @throws exception When the file cannot be written + * @param string $path + * @param string $content + * @throws exception When the file cannot be written */ - public function put_contents($path, $content); + public function put_contents(string $path, string $content): void; /** * Read the contents of a file * - * @param string $path The file to read - * - * @throws exception When cannot read file contents - * - * @return string Returns file contents + * @param string $path The file to read * + * @return string Returns file contents + * @throws exception When cannot read file contents */ - public function get_contents($path); + public function get_contents(string $path): string; /** * Checks the existence of files or directories * - * @param string $path file/directory to check + * @param string $path file/directory to check * - * @return bool Returns true if the file/directory exist, false otherwise. + * @return bool Returns true if the file/directory exist, false otherwise. */ - public function exists($path); + public function exists(string $path): bool; /** * Removes files or directories * - * @param string $path file/directory to remove + * @param string $path file/directory to remove * - * @throws exception When removal fails. + * @throws exception When removal fails. */ - public function delete($path); + public function delete(string $path): void; /** * Rename a file or a directory * - * @param string $path_orig The original file/direcotry - * @param string $path_dest The target file/directory + * @param string $path_orig The original file/direcotry + * @param string $path_dest The target file/directory * - * @throws exception When file/directory cannot be renamed + * @throws exception When file/directory cannot be renamed */ - public function rename($path_orig, $path_dest); + public function rename(string $path_orig, string $path_dest): void; /** * Copies a file * - * @param string $path_orig The original filename - * @param string $path_dest The target filename + * @param string $path_orig The original filename + * @param string $path_dest The target filename * - * @throws exception When the file cannot be copied + * @throws exception When the file cannot be copied */ - public function copy($path_orig, $path_dest); + public function copy(string $path_orig, string $path_dest): void; /** * Get direct link * - * @param string $path The file - * - * @return string Returns link. + * @param string $path The file * + * @return string Returns link. */ - public function get_link($path); + public function get_link(string $path): string; /** * Get space available in bytes * - * @throws exception When unable to retrieve available storage space - * - * @return float Returns available space + * @return float Returns available space + * @throws exception When unable to retrieve available storage space */ - public function free_space(); + public function free_space(): float; } diff --git a/phpBB/phpbb/storage/adapter/local.php b/phpBB/phpbb/storage/adapter/local.php index 1516794276..fa2635d953 100644 --- a/phpBB/phpbb/storage/adapter/local.php +++ b/phpBB/phpbb/storage/adapter/local.php @@ -104,7 +104,7 @@ class local implements adapter_interface, stream_interface /** * {@inheritdoc} */ - public function configure($options) + public function configure(array $options): void { if (substr($options['path'], -1, 1) !== DIRECTORY_SEPARATOR) { @@ -119,7 +119,7 @@ class local implements adapter_interface, stream_interface /** * {@inheritdoc} */ - public function put_contents($path, $content) + public function put_contents(string $path, string $content): void { $this->ensure_directory_exists($path); @@ -136,7 +136,7 @@ class local implements adapter_interface, stream_interface /** * {@inheritdoc} */ - public function get_contents($path) + public function get_contents(string $path): string { $content = @file_get_contents($this->root_path . $this->get_path($path) . $this->get_filename($path)); @@ -151,7 +151,7 @@ class local implements adapter_interface, stream_interface /** * {@inheritdoc} */ - public function exists($path) + public function exists(string $path): bool { return $this->filesystem->exists($this->root_path . $this->get_path($path) . $this->get_filename($path)); } @@ -159,7 +159,7 @@ class local implements adapter_interface, stream_interface /** * {@inheritdoc} */ - public function delete($path) + public function delete(string $path): void { try { @@ -176,7 +176,7 @@ class local implements adapter_interface, stream_interface /** * {@inheritdoc} */ - public function rename($path_orig, $path_dest) + public function rename(string $path_orig, string $path_dest): void { $this->ensure_directory_exists($path_dest); @@ -195,7 +195,7 @@ class local implements adapter_interface, stream_interface /** * {@inheritdoc} */ - public function copy($path_orig, $path_dest) + public function copy(string $path_orig, string $path_dest): void { $this->ensure_directory_exists($path_dest); @@ -310,7 +310,7 @@ class local implements adapter_interface, stream_interface /** * {@inheritdoc} */ - public function read_stream($path) + public function read_stream(string $path) { $stream = @fopen($this->root_path . $this->get_path($path) . $this->get_filename($path), 'rb'); @@ -325,7 +325,7 @@ class local implements adapter_interface, stream_interface /** * {@inheritdoc} */ - public function write_stream($path, $resource) + public function write_stream(string $path, $resource): void { $this->ensure_directory_exists($path); @@ -420,7 +420,7 @@ class local implements adapter_interface, stream_interface * * @return array Properties */ - public function file_image_height($path) + public function file_image_height($path): array { return $this->image_dimensions($path); } @@ -428,7 +428,7 @@ class local implements adapter_interface, stream_interface /** * {@inheritdoc} */ - public function get_link($path) + public function get_link(string $path): string { return generate_board_url() . '/' . $this->path . $path; } @@ -436,7 +436,7 @@ class local implements adapter_interface, stream_interface /** * {@inheritdoc} */ - public function free_space() + public function free_space(): float { if (function_exists('disk_free_space')) { diff --git a/phpBB/phpbb/storage/stream_interface.php b/phpBB/phpbb/storage/stream_interface.php index 6d0af71550..3ecc1cef4c 100644 --- a/phpBB/phpbb/storage/stream_interface.php +++ b/phpBB/phpbb/storage/stream_interface.php @@ -13,27 +13,29 @@ namespace phpbb\storage; +use phpbb\storage\exception\exception; + interface stream_interface { /** * Reads a file as a stream * - * @param string $path File to read + * @param string $path File to read * - * @throws \phpbb\storage\exception\exception When unable to open file - * - * @return resource Returns a file pointer + * @return resource Returns a file pointer + * @throws exception\exception When unable to open file */ - public function read_stream($path); + public function read_stream(string $path); /** * Writes a new file using a stream * - * @param string $path The target file - * @param resource $resource The resource + * @param string $path The target file + * @param resource $resource The resource * - * @throws \phpbb\storage\exception\exception When target file exists - * When target file cannot be created + * @return void + * @throws exception When target file exists + * When target file cannot be created */ - public function write_stream($path, $resource); + public function write_stream(string $path, $resource): void; } diff --git a/tests/storage/adapter/local_subfolders_test.php b/tests/storage/adapter/local_subfolders_test.php index 3ff4de7f42..f6f9b23f1f 100644 --- a/tests/storage/adapter/local_subfolders_test.php +++ b/tests/storage/adapter/local_subfolders_test.php @@ -11,95 +11,121 @@ * */ - class phpbb_storage_adapter_local_subfolders_test extends phpbb_test_case - { - protected $adapter; - - protected $path; - - protected $filesystem; +require_once __DIR__ . '/local_test_case.php'; +class local_subfolders_test extends phpbb_local_test_case +{ protected function setUp(): void { parent::setUp(); - $this->filesystem = new \phpbb\filesystem\filesystem(); - $phpbb_root_path = getcwd() . DIRECTORY_SEPARATOR; - - $this->adapter = new \phpbb\storage\adapter\local($this->filesystem, new \FastImageSize\FastImageSize(), new \phpbb\mimetype\guesser(array(new \phpbb\mimetype\extension_guesser)), $phpbb_root_path); $this->adapter->configure(['path' => 'test_path', 'subfolders' => true]); - - $this->path = $phpbb_root_path . 'test_path/'; - mkdir($this->path); } - protected function tearDown(): void - { - $this->adapter = null; - rmdir($this->path); - } - - public function test_put_contents() + public function test_put_contents(): void { + // When $this->adapter->put_contents('file.txt', 'abc'); - $this->assertTrue(file_exists($this->path . '3d/8e/file.txt')); - $this->assertEquals(file_get_contents($this->path . '3d/8e/file.txt'), 'abc'); + + // Then + $this->assertFileExists($this->path . '3d/8e/file.txt'); + $this->assertFileContains($this->path . '3d/8e/file.txt', 'abc'); + + // Clean test unlink($this->path . '3d/8e/file.txt'); rmdir($this->path . '3d/8e'); rmdir($this->path . '3d'); } - public function test_get_contents() + public function test_get_contents(): void { + // Given mkdir($this->path . '3d/8e', 0777, true); file_put_contents($this->path . '3d/8e/file.txt', 'abc'); - $this->assertEquals($this->adapter->get_contents('file.txt'), 'abc'); + + // When + $content = $this->adapter->get_contents('file.txt'); + + // Then + $this->assertEquals('abc', $content); + + // Clean test unlink($this->path . '3d/8e/file.txt'); rmdir($this->path . '3d/8e'); rmdir($this->path . '3d'); } - public function test_exists() + public function test_exists(): void { + // Given mkdir($this->path . '3d/8e', 0777, true); touch($this->path . '3d/8e/file.txt'); - $this->assertTrue($this->adapter->exists('file.txt')); - $this->assertFalse($this->adapter->exists('3d/8e/file.txt')); + + // When + $existent_file = $this->adapter->exists('file.txt'); + $non_existent_file = $this->adapter->exists('noexist.txt'); + $non_existent_file2 = $this->adapter->exists('3d/8e/file.txt'); + + // Then + $this->assertTrue($existent_file); + $this->assertFalse($non_existent_file); + $this->assertFalse($non_existent_file2); + + // Clean test unlink($this->path . '3d/8e/file.txt'); rmdir($this->path . '3d/8e'); rmdir($this->path . '3d'); } - public function test_delete_file() + public function test_delete_file(): void { + // Given mkdir($this->path . '3d/8e', 0777, true); touch($this->path . '3d/8e/file.txt'); - $this->assertTrue(file_exists($this->path . '3d/8e/file.txt')); + $this->assertFileExists($this->path . '3d/8e/file.txt'); + + // When $this->adapter->delete('file.txt'); - $this->assertFalse(file_exists($this->path . '3d/8e/file.txt')); - $this->assertFalse(file_exists($this->path . '3d')); + + // Then + $this->assertFileNotExists($this->path . '3d/8e/file.txt'); + $this->assertFileNotExists($this->path . '3d'); } - public function test_rename() + public function test_rename(): void { + // Given mkdir($this->path . '3d/8e', 0777, true); touch($this->path . '3d/8e/file.txt'); + + // When $this->adapter->rename('file.txt', 'file2.txt'); - $this->assertFalse(file_exists($this->path . '3d/8e/file.txt')); - $this->assertTrue(file_exists($this->path . '27/36/file2.txt')); - $this->assertFalse(file_exists($this->path . '3d')); + + // Then + $this->assertFileNotExists($this->path . '3d/8e/file.txt'); + $this->assertFileExists($this->path . '27/36/file2.txt'); + $this->assertFileNotExists($this->path . '3d'); + + // Clean test unlink($this->path . '27/36/file2.txt'); rmdir($this->path . '27/36'); rmdir($this->path . '27'); } - public function test_copy() + public function test_copy(): void { + // Given mkdir($this->path . '3d/8e', 0777, true); file_put_contents($this->path . '3d/8e/file.txt', 'abc'); + + // When $this->adapter->copy('file.txt', 'file2.txt'); - $this->assertEquals(file_get_contents($this->path . '3d/8e/file.txt'), 'abc'); - $this->assertEquals(file_get_contents($this->path . '27/36/file2.txt'), 'abc'); + + // Then + $this->assertFileContains($this->path . '3d/8e/file.txt', 'abc'); + $this->assertFileContains($this->path . '27/36/file2.txt', 'abc'); + + // Clean test unlink($this->path . '3d/8e/file.txt'); rmdir($this->path . '3d/8e'); rmdir($this->path . '3d'); @@ -108,29 +134,42 @@ rmdir($this->path . '27'); } - public function test_read_stream() + public function test_read_stream(): void { + // Given mkdir($this->path . '3d/8e', 0777, true); touch($this->path . '3d/8e/file.txt'); + + // When $stream = $this->adapter->read_stream('file.txt'); - $this->assertTrue(is_resource($stream)); + + // Then + $this->assertIsResource($stream); + + // Clean test fclose($stream); unlink($this->path . '3d/8e/file.txt'); rmdir($this->path . '3d/8e'); rmdir($this->path . '3d'); } - public function test_write_stream() + public function test_write_stream(): void { + // Given file_put_contents($this->path . 'file.txt', 'abc'); $stream = fopen($this->path . 'file.txt', 'rb'); + + // When $this->adapter->write_stream('file2.txt', $stream); fclose($stream); - $this->assertEquals(file_get_contents($this->path . '27/36/file2.txt'), 'abc'); + + // Then + $this->assertFileContains($this->path . '27/36/file2.txt', 'abc'); + + // Clean test unlink($this->path . 'file.txt'); unlink($this->path . '27/36/file2.txt'); rmdir($this->path . '27/36'); rmdir($this->path . '27'); } - - } +} diff --git a/tests/storage/adapter/local_test.php b/tests/storage/adapter/local_test.php index 7b2a2af117..ee86dd6ca1 100644 --- a/tests/storage/adapter/local_test.php +++ b/tests/storage/adapter/local_test.php @@ -11,103 +11,141 @@ * */ - class phpbb_storage_adapter_local_test extends phpbb_test_case - { - protected $adapter; - - protected $path; - - protected $filesystem; +require_once __DIR__ . '/local_test_case.php'; +class phpbb_storage_adapter_local_test extends phpbb_local_test_case +{ protected function setUp(): void { parent::setUp(); - $this->filesystem = new \phpbb\filesystem\filesystem(); - $phpbb_root_path = getcwd() . DIRECTORY_SEPARATOR; - - $this->adapter = new \phpbb\storage\adapter\local($this->filesystem, new \FastImageSize\FastImageSize(), new \phpbb\mimetype\guesser(array(new \phpbb\mimetype\extension_guesser)), $phpbb_root_path); $this->adapter->configure(['path' => 'test_path', 'subfolders' => false]); - - $this->path = $phpbb_root_path . 'test_path/'; - mkdir($this->path); } - protected function tearDown(): void - { - $this->adapter = null; - rmdir($this->path); - } - - public function test_put_contents() + public function test_put_contents(): void { + // When $this->adapter->put_contents('file.txt', 'abc'); - $this->assertTrue(file_exists($this->path . 'file.txt')); - $this->assertEquals(file_get_contents($this->path . 'file.txt'), 'abc'); + + // Then + $this->assertFileExists($this->path . 'file.txt'); + $this->assertFileContains($this->path . 'file.txt', 'abc'); + + // Clean test unlink($this->path . 'file.txt'); } - public function test_get_contents() + public function test_get_contents(): void { + // Given file_put_contents($this->path . 'file.txt', 'abc'); - $this->assertEquals($this->adapter->get_contents('file.txt'), 'abc'); + + // When + $content = $this->adapter->get_contents('file.txt'); + + // Then + $this->assertEquals('abc', $content); + + // Clean test unlink($this->path . 'file.txt'); } - public function test_exists() + public function test_exists(): void { + // Given touch($this->path . 'file.txt'); - $this->assertTrue($this->adapter->exists('file.txt')); - $this->assertFalse($this->adapter->exists('noexist.txt')); + + // When + $existent_file = $this->adapter->exists('file.txt'); + $non_existent_file = $this->adapter->exists('noexist.txt'); + + // Then + $this->assertTrue($existent_file); + $this->assertFalse($non_existent_file); + + // Clean test unlink($this->path . 'file.txt'); } - public function test_delete_file() + public function test_delete_file(): void { + // Given touch($this->path . 'file.txt'); - $this->assertTrue(file_exists($this->path . 'file.txt')); + $this->assertFileExists($this->path . 'file.txt'); + + // When $this->adapter->delete('file.txt'); - $this->assertFalse(file_exists($this->path . 'file.txt')); + + // Then + $this->assertFileNotExists($this->path . 'file.txt'); } - public function test_rename() + public function test_rename(): void { + // Given touch($this->path . 'file.txt'); + $this->assertFileExists($this->path . 'file.txt'); + $this->assertFileNotExists($this->path . 'file2.txt'); + + // When $this->adapter->rename('file.txt', 'file2.txt'); - $this->assertFalse(file_exists($this->path . 'file.txt')); - $this->assertTrue(file_exists($this->path . 'file2.txt')); - $this->assertFalse(file_exists($this->path . 'file.txt')); + + // Then + $this->assertFileNotExists($this->path . 'file.txt'); + $this->assertFileExists($this->path . 'file2.txt'); + + // Clean test unlink($this->path . 'file2.txt'); } - public function test_copy() + public function test_copy(): void { + // Given file_put_contents($this->path . 'file.txt', 'abc'); + + // When $this->adapter->copy('file.txt', 'file2.txt'); - $this->assertEquals(file_get_contents($this->path . 'file.txt'), 'abc'); - $this->assertEquals(file_get_contents($this->path . 'file2.txt'), 'abc'); + + // Then + $this->assertFileContains($this->path . 'file.txt', 'abc'); + $this->assertFileContains($this->path . 'file2.txt', 'abc'); + + // Clean test unlink($this->path . 'file.txt'); unlink($this->path . 'file2.txt'); } public function test_read_stream() { + // Given touch($this->path . 'file.txt'); + + // When $stream = $this->adapter->read_stream('file.txt'); - $this->assertTrue(is_resource($stream)); + + // Then + $this->assertIsResource($stream); + + // Clean test fclose($stream); unlink($this->path . 'file.txt'); } public function test_write_stream() { + // Given file_put_contents($this->path . 'file.txt', 'abc'); $stream = fopen($this->path . 'file.txt', 'rb'); + + // When $this->adapter->write_stream('file2.txt', $stream); fclose($stream); - $this->assertEquals(file_get_contents($this->path . 'file2.txt'), 'abc'); + + // Then + $this->assertFileContains($this->path . 'file2.txt', 'abc'); + + // Clean test unlink($this->path . 'file.txt'); unlink($this->path . 'file2.txt'); } - - } +} diff --git a/tests/storage/adapter/local_test_case.php b/tests/storage/adapter/local_test_case.php new file mode 100644 index 0000000000..2c008120e7 --- /dev/null +++ b/tests/storage/adapter/local_test_case.php @@ -0,0 +1,63 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +use FastImageSize\FastImageSize; +use phpbb\mimetype\extension_guesser; +use phpbb\mimetype\guesser; +use phpbb\storage\adapter\local; + +class phpbb_local_test_case extends phpbb_test_case +{ + protected $adapter; + + protected $path; + + protected $filesystem; + + protected function setUp(): void + { + parent::setUp(); + + $this->filesystem = new \phpbb\filesystem\filesystem(); + $phpbb_root_path = getcwd() . DIRECTORY_SEPARATOR; + + $this->adapter = new local( + $this->filesystem, + new FastImageSize(), + new guesser(array(new extension_guesser)), + $phpbb_root_path + ); + + $this->path = $phpbb_root_path . 'test_path/'; + mkdir($this->path); + } + + protected function tearDown(): void + { + parent::tearDown(); + + $this->adapter = null; + rmdir($this->path); + } + + /** + * Check if a file contains a string + * + * @param string $file + * @param string $content + */ + protected function assertFileContains(string $file, string $content): void + { + $this->assertEquals($content, file_get_contents($file)); + } +}