Harden file storage internals

docs/Architecture.md

@@ -36,6 +36,8 @@ The file object implements `withResource()` and `withFile()`. The second takes a
 
 When you store a file for the first time in your storage backend, you **must** add a resource to the file object. If you don't do that, you'll get an exception from the file storage service.
 
+The same applies to the UUID when you use the built-in `File` object together with serialization or the default `PathBuilder`: call `withUuid()` before persisting or generating paths.
+
 ## Dependencies
 
 We think there is a need to explain why we have picked the dependencies we have, because we try to keep dependencies low. However, there are some cases that make sense to use existing libraries.

docs/Path-Builders.md

@@ -14,7 +14,10 @@ $builder = new PathBuilder([
 
 ### Options:
 
+ * **directorySeparator**: `DIRECTORY_SEPARATOR`
  * **randomPath**: 'sha1'
+   * Can also be a callable with the signature `function (string $uuid, int $levels): string`
+ * **randomPathLevels**: 3
  * **sanitizeFilename**: true
  * **beautifyFilename**: false
  * **filenameSanitizer**: null|\PhpCollective\Infrastructure\Storage\Utility\FilenameSanitizerInterface
@@ -65,6 +68,8 @@ This path builder provides a `setFilenameSanitizer()` method that takes an objec
 
 This is an alternative way to provide a sanitizer besides passing it through the configuration array.
 
+The `filenameSanitizer` config option and `setFilenameSanitizer()` are equivalent. Use whichever fits your setup better.
+
 ## Conditional Path Builder
 
 Add callbacks and path builders to check on the file which of the builders should be used to build the path.

docs/The-File-Object.md

@@ -53,6 +53,8 @@ You'll also have to add the (uu)id to the file object if your intended path for
 
 The file object is serializable to json, and you can call `toArray()` on it to turn it into an array that you can either save in the structure you get or continue transforming it into whatever structure your persistence layer expects.
 
+If you want to serialize the built-in `File` object or use the default `PathBuilder`, make sure you set a UUID first using `withUuid()`. The UUID is treated as required state for persistence and default path generation.
+
 ## Restoring the file object
 
 You'll have to reconstruct the file object later from your persisted information when you want to come back to it later and work with new variants for example. Depending on your architecture, your domain model could also simply implement the `FileInterface` if this is more convenient for your.

src/Exception/MissingUuidException.php

@@ -0,0 +1,31 @@
+<?php declare(strict_types = 1);
+
+/**
+ * Copyright (c) Florian Krämer (https://florian-kraemer.net)
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright Copyright (c) Florian Krämer (https://florian-kraemer.net)
+ * @author Florian Krämer
+ * @link https://github.com/Phauthentic
+ * @license https://opensource.org/licenses/MIT MIT License
+ */
+
+namespace PhpCollective\Infrastructure\Storage\Exception;
+
+/**
+ * Missing UUID Exception
+ */
+class MissingUuidException extends StorageException
+{
+    /**
+     * @return self
+     */
+    public static function create(): self
+    {
+        return new self(
+            'UUID has not been set',
+        );
+    }
+}

src/File.php

@@ -15,6 +15,7 @@
 namespace PhpCollective\Infrastructure\Storage;
 
 use PhpCollective\Infrastructure\Storage\Exception\InvalidStreamResourceException;
+use PhpCollective\Infrastructure\Storage\Exception\MissingUuidException;
 use PhpCollective\Infrastructure\Storage\PathBuilder\PathBuilderInterface;
 use PhpCollective\Infrastructure\Storage\Processor\Exception\VariantDoesNotExistException;
 use PhpCollective\Infrastructure\Storage\UrlBuilder\UrlBuilderInterface;
@@ -395,10 +396,16 @@ public function filename(): string
     }
 
     /**
+     * @throws \PhpCollective\Infrastructure\Storage\Exception\MissingUuidException
+     *
      * @return string
      */
     public function uuid(): string
     {
+        if (!isset($this->uuid)) {
+            throw MissingUuidException::create();
+        }
+
         return $this->uuid;
     }
 
@@ -673,7 +680,7 @@ public function withUrl(string $url): FileInterface
     public function toArray(): array
     {
         return [
-            'uuid' => $this->uuid,
+            'uuid' => $this->uuid(),
             'filename' => $this->filename,
             'filesize' => $this->filesize,
             'mimeType' => $this->mimeType,

src/PathBuilder/PathBuilder.php

@@ -72,9 +72,9 @@ public function __construct(array $config = [])
     {
         $this->config = $config + $this->defaultConfig;
 
-        if (!$this->config['filenameSanitizer'] instanceof FilenameSanitizerInterface) {
-            $this->filenameSanitizer = new FilenameSanitizer();
-        }
+        $this->filenameSanitizer = $this->config['filenameSanitizer'] instanceof FilenameSanitizerInterface
+            ? $this->config['filenameSanitizer']
+            : new FilenameSanitizer();
     }
 
     /**
@@ -177,16 +177,21 @@ protected function filename(FileInterface $file, array $options = []): string
      *
      * @param string $string Input string
      * @param int $level Depth of the path to generate.
-     * @param string $method Hash method, crc32 or sha1.
+     * @param callable|string $method Hash method or callback.
+     * @param string $separator Directory separator to use.
      *
      * @throws \InvalidArgumentException
      *
      * @return string
      */
-    protected function randomPath($string, $level = 3, $method = 'sha1'): string
-    {
+    protected function randomPath(
+        string $string,
+        int $level = 3,
+        $method = 'sha1',
+        string $separator = DIRECTORY_SEPARATOR,
+    ): string {
         if ($method === 'sha1') {
-            return $this->randomPathSha1($string, $level);
+            return $this->randomPathSha1($string, $level, $separator);
         }
 
         if (is_callable($method)) {
@@ -206,17 +211,18 @@ protected function randomPath($string, $level = 3, $method = 'sha1'): string
      *
      * @param string $string Input string
      * @param int $level Depth of the path to generate.
+     * @param string $separator Directory separator to use.
      *
      * @return string
      */
-    protected function randomPathSha1(string $string, int $level): string
+    protected function randomPathSha1(string $string, int $level, string $separator): string
     {
         $result = sha1($string);
         $randomString = '';
         $counter = 0;
         for ($i = 1; $i <= $level; $i++) {
             $counter += 2;
-            $randomString .= substr($result, $counter, 2) . DIRECTORY_SEPARATOR;
+            $randomString .= substr($result, $counter, 2) . $separator;
         }
 
         return substr($randomString, 0, -1);
@@ -238,7 +244,7 @@ protected function getDateObject(): DateTimeInterface
     protected function buildPath(FileInterface $file, ?string $variant, array $options = []): string
     {
         $config = $options + $this->config;
-        $ds = $this->config['directorySeparator'];
+        $ds = $config['directorySeparator'];
         $filename = $this->filename($file, $options);
         $hashedVariant = substr(hash('sha1', (string)$variant), 0, 6);
         $template = $variant ? $config['variantPathTemplate'] : $config['pathTemplate'];
@@ -250,7 +256,12 @@ protected function buildPath(FileInterface $file, ?string $variant, array $optio
             '{model}' => $file->model(),
             '{collection}' => $file->collection(),
             '{id}' => $file->uuid(),
-            '{randomPath}' => $this->randomPath($file->uuid(), $randomPathLevels),
+            '{randomPath}' => $this->randomPath(
+                $file->uuid(),
+                $randomPathLevels,
+                $config['randomPath'],
+                $ds,
+            ),
             '{modelId}' => $file->modelId(),
             '{strippedId}' => str_replace('-', '', $file->uuid()),
             '{extension}' => $file->extension(),

src/StorageService.php

@@ -186,7 +186,9 @@ public function storeResource(string $adapter, string $path, $resource, ?Config
     public function storeFile(string $adapter, string $path, string $file, ?Config $config = null): void
     {
         $config = $this->makeConfigIfNeeded($config);
-        $this->adapter($adapter)->write($path, (string)file_get_contents($file), $config);
+        $resource = openFile($file, 'rb', false);
+        $this->adapter($adapter)->writeStream($path, $resource, $config);
+        fclose($resource);
     }
 
     /**

tests/TestCase/FileFactoryTest.php

@@ -30,12 +30,9 @@ class FileFactoryTest extends TestCase
      */
     public function testInvalidUpload(): void
     {
-        /** @var \Psr\Http\Message\UploadedFileInterface|\PHPUnit\Framework\MockObject\MockObject $uploadedFile */
-        $uploadedFile = $this->getMockBuilder(UploadedFileInterface::class)
-            ->getMock();
+        $uploadedFile = $this->createStub(UploadedFileInterface::class);
 
-        $uploadedFile->expects($this->any())
-            ->method('getError')
+        $uploadedFile->method('getError')
             ->willReturn(UPLOAD_ERR_NO_FILE);
 
         $this->expectException(RuntimeException::class);
@@ -47,44 +44,32 @@ public function testInvalidUpload(): void
      */
     public function testValidUpload(): void
     {
-        /** @var \Psr\Http\Message\StreamInterface|\PHPUnit\Framework\MockObject\MockObject $stream */
-        $stream = $this->getMockBuilder(StreamInterface::class)
-            ->getMock();
+        $stream = $this->createStub(StreamInterface::class);
 
-        $stream->expects($this->any())
-            ->method('isReadable')
+        $stream->method('isReadable')
             ->willReturn(true);
 
-        $stream->expects($this->any())
-            ->method('isWritable')
+        $stream->method('isWritable')
             ->willReturn(false);
 
-        $stream->expects($this->any())
-            ->method('detach')
+        $stream->method('detach')
             ->willReturn(fopen('composer.json', 'r'));
 
-        /** @var \Psr\Http\Message\UploadedFileInterface|\PHPUnit\Framework\MockObject\MockObject $uploadedFile */
-        $uploadedFile = $this->getMockBuilder(UploadedFileInterface::class)
-            ->getMock();
+        $uploadedFile = $this->createStub(UploadedFileInterface::class);
 
-        $uploadedFile->expects($this->any())
-            ->method('getError')
+        $uploadedFile->method('getError')
             ->willReturn(UPLOAD_ERR_OK);
 
-        $uploadedFile->expects($this->any())
-            ->method('getClientFilename')
+        $uploadedFile->method('getClientFilename')
             ->willReturn('titus.jpg');
 
-        $uploadedFile->expects($this->any())
-            ->method('getSize')
+        $uploadedFile->method('getSize')
             ->willReturn(12345);
 
-        $uploadedFile->expects($this->any())
-            ->method('getClientMediaType')
+        $uploadedFile->method('getClientMediaType')
             ->willReturn('image/image-jpg');
 
-        $uploadedFile->expects($this->any())
-            ->method('getStream')
+        $uploadedFile->method('getStream')
             ->willReturn($stream);
 
         $file = FileFactory::fromUploadedFile($uploadedFile, 'local');

tests/TestCase/FileTest.php

@@ -14,6 +14,7 @@
 
 namespace PhpCollective\Test\TestCase;
 
+use PhpCollective\Infrastructure\Storage\Exception\MissingUuidException;
 use PhpCollective\Infrastructure\Storage\File;
 use PhpCollective\Infrastructure\Storage\FileFactory;
 use PhpCollective\Infrastructure\Storage\FileInterface;
@@ -140,4 +141,34 @@ public function testPathException(): void
         $this->expectExceptionMessage('Path has not been set');
         $file->path();
     }
+
+    /**
+     * @return void
+     */
+    public function testMissingUuidExceptionOnSerialization(): void
+    {
+        $file = File::create(
+            'foobar.jpg',
+            123,
+            'image/jpeg',
+            'local',
+        );
+
+        $this->expectException(MissingUuidException::class);
+        $this->expectExceptionMessage('UUID has not been set');
+        $file->toArray();
+    }
+
+    /**
+     * @return void
+     */
+    public function testMissingUuidExceptionOnPathBuild(): void
+    {
+        $fileOnDisk = $this->getFixtureFile('titus.jpg');
+        $file = FileFactory::fromDisk($fileOnDisk, 'local');
+
+        $this->expectException(MissingUuidException::class);
+        $this->expectExceptionMessage('UUID has not been set');
+        $file->buildPath(new PathBuilder());
+    }
 }