Imrovements on file creation during write modes (#401)

azjezz · Mar 17, 2023 · 0b1eac3 · 0b1eac3
1 parent 37fef88
commit 0b1eac3
Show file tree

Hide file tree

Showing 13 changed files with 144 additions and 37 deletions.
diff --git a/docs/component/file.md b/docs/component/file.md
@@ -30,7 +30,7 @@
 - [Lock](./../../src/Psl/File/Lock.php#L9)
 - [ReadHandle](./../../src/Psl/File/ReadHandle.php#L10)
 - [ReadWriteHandle](./../../src/Psl/File/ReadWriteHandle.php#L11)
-- [WriteHandle](./../../src/Psl/File/WriteHandle.php#L10)
+- [WriteHandle](./../../src/Psl/File/WriteHandle.php#L11)
 
 #### `Enums`
 

diff --git a/docs/component/filesystem.md b/docs/component/filesystem.md
@@ -22,6 +22,7 @@
 - [change_permissions](./../../src/Psl/Filesystem/change_permissions.php#L21)
 - [copy](./../../src/Psl/Filesystem/copy.php#L22)
 - [create_directory](./../../src/Psl/Filesystem/create_directory.php#L19)
+- [create_directory_for_file](./../../src/Psl/Filesystem/create_directory_for_file.php#L16)
 - [create_file](./../../src/Psl/Filesystem/create_file.php#L24)
 - [create_hard_link](./../../src/Psl/Filesystem/create_hard_link.php#L23)
 - [create_symbolic_link](./../../src/Psl/Filesystem/create_symbolic_link.php#L22)

diff --git a/src/Psl/File/ReadWriteHandle.php b/src/Psl/File/ReadWriteHandle.php
@@ -32,18 +32,12 @@ public function __construct(string $file, WriteMode $write_mode = WriteMode::OPE
             throw Exception\NotFileException::for($file);
         }
 
-        $open_or_create = $write_mode === WriteMode::OPEN_OR_CREATE;
         $must_create = $write_mode === WriteMode::MUST_CREATE;
         if ($must_create && $is_file) {
             throw Exception\AlreadyCreatedException::for($file);
         }
 
-        $creating = $open_or_create || $must_create;
-        if (!$creating && !$is_file) {
-            throw Exception\NotFoundException::for($file);
-        }
-
-        if ((!$creating || ($open_or_create && $is_file))) {
+        if ($is_file) {
             if (!Filesystem\is_writable($file)) {
                 throw Exception\NotWritableException::for($file);
             }
@@ -53,15 +47,22 @@ public function __construct(string $file, WriteMode $write_mode = WriteMode::OPE
             }
         }
 
-        if ($creating && !$is_file) {
+        if (!$is_file) {
             try {
-                Filesystem\create_file($file);
+                $directory = Filesystem\create_directory_for_file($file);
+                if (!Filesystem\is_writable($directory)) {
+                    throw Exception\NotWritableException::for($file);
+                }
+
+                if (!Filesystem\is_readable($directory)) {
+                    throw Exception\NotReadableException::for($file);
+                }
             } catch (Filesystem\Exception\RuntimeException $previous) {
-                throw new Exception\RuntimeException(Str\format('Failed to create the file "%s".', $file), previous: $previous);
+                throw new Exception\RuntimeException(Str\format('Failed to create the directory for file "%s".', $file), previous: $previous);
             }
         }
 
-        $this->readWriteHandle = Internal\open($file, 'r' . ($write_mode->value) . '+', read: true, write: true);
+        $this->readWriteHandle = Internal\open($file, ($write_mode->value) . 'r+', read: true, write: true);
 
         parent::__construct($this->readWriteHandle);
     }

diff --git a/src/Psl/File/WriteHandle.php b/src/Psl/File/WriteHandle.php
@@ -6,6 +6,7 @@
 
 use Psl\Filesystem;
 use Psl\IO;
+use Psl\Str;
 
 final class WriteHandle extends Internal\AbstractHandleWrapper implements WriteHandleInterface
 {
@@ -19,7 +20,8 @@ final class WriteHandle extends Internal\AbstractHandleWrapper implements WriteH
      * @throws Exception\NotFileException If $file points to a non-file node on the filesystem.
      * @throws Exception\AlreadyCreatedException If $file is already created, and $write_mode is {@see WriteMode::MUST_CREATE}.
      * @throws Exception\NotFoundException If $file does not exist, and $write_mode is {@see WriteMode::TRUNCATE} or {@see WriteMode::APPEND}.
-     * @throws Exception\NotWritableException If $file exists, and is non-writable.
+     * @throws Exception\NotWritableException If $file is non-writable.
+     * @throws Exception\RuntimeException If unable to create the $file if it does not exist.
      */
     public function __construct(string $file, WriteMode $write_mode = WriteMode::OPEN_OR_CREATE)
     {
@@ -28,19 +30,24 @@ public function __construct(string $file, WriteMode $write_mode = WriteMode::OPE
             throw Exception\NotFileException::for($file);
         }
 
-        $open_or_create = $write_mode === WriteMode::OPEN_OR_CREATE || $write_mode === WriteMode::TRUNCATE;
         $must_create = $write_mode === WriteMode::MUST_CREATE;
         if ($must_create && $is_file) {
             throw Exception\AlreadyCreatedException::for($file);
         }
 
-        $creating = $open_or_create || $must_create;
-        if (!$creating && !$is_file) {
-            throw Exception\NotFoundException::for($file);
+        if ($is_file && !Filesystem\is_writable($file)) {
+            throw Exception\NotWritableException::for($file);
         }
 
-        if ((!$creating || ($open_or_create && $is_file)) && !Filesystem\is_writable($file)) {
-            throw Exception\NotWritableException::for($file);
+        if (!$is_file) {
+            try {
+                $directory = Filesystem\create_directory_for_file($file);
+                if (!Filesystem\is_writable($directory)) {
+                    throw Exception\NotWritableException::for($file);
+                }
+            } catch (Filesystem\Exception\RuntimeException $previous) {
+                throw new Exception\RuntimeException(Str\format('Failed to create the directory for file "%s".', $file), previous: $previous);
+            }
         }
 
         $this->writeHandle = Internal\open($file, $write_mode->value, read: false, write: true);

diff --git a/src/Psl/Filesystem/create_directory_for_file.php b/src/Psl/Filesystem/create_directory_for_file.php
@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Psl\Filesystem;
+
+/**
+ * Create the directory where the $filename is or will be stored.
+ *
+ * @param non-empty-string $filename
+ *
+ * @throws Exception\RuntimeException If unable to create the directory.
+ *
+ * @return non-empty-string
+ */
+function create_directory_for_file(string $filename, int $permissions = 0777): string
+{
+    $directory = namespace\get_directory($filename);
+    namespace\create_directory($directory, $permissions);
+
+    return $directory;
+}
diff --git a/src/Psl/Filesystem/create_file.php b/src/Psl/Filesystem/create_file.php
@@ -33,8 +33,7 @@ function create_file(string $filename, ?int $time = null, ?int $access_time = nu
         $fun = static fn(): bool => touch($filename, $time, Math\maxva($access_time, $time));
     }
 
-    $directory = namespace\get_directory($filename);
-    namespace\create_directory($directory);
+    namespace\create_directory_for_file($filename);
 
     [$result, $error_message] = Internal\box($fun);
     // @codeCoverageIgnoreStart

diff --git a/src/Psl/Filesystem/create_hard_link.php b/src/Psl/Filesystem/create_hard_link.php
@@ -42,9 +42,7 @@ function create_hard_link(string $source, string $destination): void
             namespace\delete_file($destination);
         }
     } else {
-        namespace\create_directory(
-            namespace\get_directory($destination)
-        );
+        namespace\create_directory_for_file($destination);
     }
 
     [$result, $error_message] = Internal\box(static fn() => link($source, $destination));

diff --git a/src/Psl/Filesystem/create_symbolic_link.php b/src/Psl/Filesystem/create_symbolic_link.php
@@ -25,8 +25,7 @@ function create_symbolic_link(string $source, string $destination): void
         throw Exception\NotFoundException::forNode($source);
     }
 
-    $destination_directory = namespace\get_directory($destination);
-    namespace\create_directory($destination_directory);
+    namespace\create_directory_for_file($destination);
 
     try {
         if (namespace\read_symbolic_link($destination) === $source) {

diff --git a/src/Psl/Internal/Loader.php b/src/Psl/Internal/Loader.php
@@ -414,6 +414,7 @@ final class Loader
         'Psl\\Filesystem\\change_permissions' => 'Psl/Filesystem/change_permissions.php',
         'Psl\\Filesystem\\copy' => 'Psl/Filesystem/copy.php',
         'Psl\\Filesystem\\create_directory' => 'Psl/Filesystem/create_directory.php',
+        'Psl\\Filesystem\\create_directory_for_file' => 'Psl/Filesystem/create_directory_for_file.php',
         'Psl\\Filesystem\\create_file' => 'Psl/Filesystem/create_file.php',
         'Psl\\Filesystem\\delete_directory' => 'Psl/Filesystem/delete_directory.php',
         'Psl\\Filesystem\\delete_file' => 'Psl/Filesystem/delete_file.php',

diff --git a/tests/unit/File/ReadHandleTest.php b/tests/unit/File/ReadHandleTest.php
@@ -26,4 +26,13 @@ public function testWriting(): void
 
         static::assertSame('hello, world!', $content);
     }
+
+    public function testNonExisting(): void
+    {
+        $temporary_file = Filesystem\create_temporary_file();
+        Filesystem\delete_file($temporary_file);
+
+        $this->expectException(File\Exception\NotFoundException::class);
+        File\open_read_only($temporary_file);
+    }
 }
diff --git a/tests/unit/File/ReadWriteHandleTest.php b/tests/unit/File/ReadWriteHandleTest.php
@@ -10,7 +10,6 @@
 use Psl\Filesystem;
 use Psl\IO;
 use Psl\OS;
-use Psl\SecureRandom;
 
 final class ReadWriteHandleTest extends TestCase
 {
@@ -90,10 +89,22 @@ public function testMustCreateExistingFile(): void
 
     public function testAppendToNonExistingFile(): void
     {
-        $this->expectException(File\Exception\NotFoundException::class);
-        $this->expectExceptionMessage('is not found.');
+        $temporary_file = Filesystem\create_temporary_file();
+        Filesystem\delete_file($temporary_file);
+
+        static::assertFalse(Filesystem\is_file($temporary_file));
 
-        new File\ReadWriteHandle(Env\temp_dir() . '/' . SecureRandom\string(20), File\WriteMode::APPEND);
+        $handle = File\open_read_write($temporary_file, File\WriteMode::APPEND);
+        $handle->writeAll('hello');
+        $handle->seek(0);
+
+        $content = $handle->readAll();
+
+        static::assertSame('hello', $content);
+
+        $handle->close();
+
+        static::assertTrue(Filesystem\is_file($temporary_file));
     }
 
     public function testAppendToANonWritableFile(): void
@@ -139,8 +150,8 @@ public function testThrowsWhenCreatingFile(): void
 
         $file = $temporary_file . Filesystem\SEPARATOR . 'foo';
 
-        $this->expectException(File\Exception\RuntimeException::class);
-        $this->expectExceptionMessage('Failed to create the file "' . $file . '".');
+        $this->expectException(File\Exception\NotWritableException::class);
+        $this->expectExceptionMessage('File "' . $file . '" is not writable.');
 
         new File\ReadWriteHandle($file, File\WriteMode::MUST_CREATE);
     }

diff --git a/tests/unit/File/ReadWriteTest.php b/tests/unit/File/ReadWriteTest.php
@@ -4,8 +4,10 @@
 
 namespace Psl\Tests\Unit\File;
 
+use Psl\Env;
 use Psl\File;
 use Psl\Filesystem;
+use Psl\OS;
 use Psl\Str;
 use Psl\Tests\Unit\Filesystem\AbstractFilesystemTest;
 
@@ -122,4 +124,21 @@ public function testRead(): void
 
         static::assertSame('PHP programmers.', $content);
     }
+
+    public function testThrowsWhenDirectoryCreationFails(): void
+    {
+        if (OS\is_windows()) {
+            static::markTestSkipped('Permissions are not reliable on windows.');
+        }
+
+        $target_directory = Env\temp_dir() . DIRECTORY_SEPARATOR . 'you-shall-not-pass';
+        Filesystem\create_directory($target_directory, 0000);
+
+        $target_file = $target_directory . DIRECTORY_SEPARATOR . 'fails-on-subdir-creation' . DIRECTORY_SEPARATOR . 'somefile.txt';
+
+        $this->expectException(File\Exception\RuntimeException::class);
+        $this->expectExceptionMessage('Failed to create the directory for file "' . $target_file . '".');
+
+        new File\ReadWriteHandle($target_file, File\WriteMode::MUST_CREATE);
+    }
 }
diff --git a/tests/unit/File/WriteHandleTest.php b/tests/unit/File/WriteHandleTest.php
@@ -8,7 +8,7 @@
 use Psl\Env;
 use Psl\File;
 use Psl\Filesystem;
-use Psl\SecureRandom;
+use Psl\OS;
 
 final class WriteHandleTest extends TestCase
 {
@@ -22,11 +22,15 @@ public function testMustCreateExistingFile(): void
 
     public function testAppendToNonExistingFile(): void
     {
-        $this->expectException(File\Exception\NotFoundException::class);
-        $this->expectExceptionMessage('is not found.');
+        $temporary_file = Filesystem\create_temporary_file();
+        Filesystem\delete_file($temporary_file);
+
+        static::assertFalse(Filesystem\is_file($temporary_file));
 
-        $f = new File\WriteHandle(Env\temp_dir() . '/' . SecureRandom\string(20), File\WriteMode::APPEND);
-        $f->write('g');
+        $handle = new File\WriteHandle($temporary_file, File\WriteMode::APPEND);
+        $handle->close();
+
+        static::assertTrue(Filesystem\is_file($temporary_file));
     }
 
     public function testAppendToANonWritableFile(): void
@@ -57,6 +61,25 @@ public function testWriting(): void
         static::assertSame('hello, world!', $content);
     }
 
+    public function testThrowsWhenCreatingFile(): void
+    {
+        if (OS\is_windows()) {
+            static::markTestSkipped('Permissions are not reliable on windows.');
+        }
+
+        $temporary_file = Filesystem\create_temporary_file();
+        Filesystem\delete_file($temporary_file);
+        Filesystem\create_directory($temporary_file);
+        Filesystem\change_permissions($temporary_file, 0555);
+
+        $file = $temporary_file . Filesystem\SEPARATOR . 'foo';
+
+        $this->expectException(File\Exception\NotWritableException::class);
+        $this->expectExceptionMessage('File "' . $file . '" is not writable.');
+
+        new File\WriteHandle($file, File\WriteMode::MUST_CREATE);
+    }
+
     public function testCreateNonExisting(): void
     {
         $temporary_file = Filesystem\create_temporary_file();
@@ -69,4 +92,21 @@ public function testCreateNonExisting(): void
 
         static::assertTrue(Filesystem\is_file($temporary_file));
     }
+
+    public function testThrowsWhenDirectoryCreationFails(): void
+    {
+        if (OS\is_windows()) {
+            static::markTestSkipped('Permissions are not reliable on windows.');
+        }
+
+        $target_directory = Env\temp_dir() . DIRECTORY_SEPARATOR . 'you-shall-not-pass';
+        Filesystem\create_directory($target_directory, 0000);
+
+        $target_file = $target_directory . DIRECTORY_SEPARATOR . 'fails-on-subdir-creation' . DIRECTORY_SEPARATOR . 'somefile.txt';
+
+        $this->expectException(File\Exception\RuntimeException::class);
+        $this->expectExceptionMessage('Failed to create the directory for file "' . $target_file . '".');
+
+        new File\WriteHandle($target_file, File\WriteMode::MUST_CREATE);
+    }
 }