diff --git a/docs/Architecture.md b/docs/Architecture.md index d4b6e6f..425bb5a 100644 --- a/docs/Architecture.md +++ b/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. diff --git a/docs/Path-Builders.md b/docs/Path-Builders.md index cfb2e1e..1c586ce 100644 --- a/docs/Path-Builders.md +++ b/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. diff --git a/docs/The-File-Object.md b/docs/The-File-Object.md index 05462b9..07e68f4 100644 --- a/docs/The-File-Object.md +++ b/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. diff --git a/src/Exception/MissingUuidException.php b/src/Exception/MissingUuidException.php new file mode 100644 index 0000000..e8ea1c3 --- /dev/null +++ b/src/Exception/MissingUuidException.php @@ -0,0 +1,31 @@ +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, diff --git a/src/PathBuilder/PathBuilder.php b/src/PathBuilder/PathBuilder.php index b2147e8..c5e8882 100644 --- a/src/PathBuilder/PathBuilder.php +++ b/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(), diff --git a/src/StorageService.php b/src/StorageService.php index 29f2740..508a027 100644 --- a/src/StorageService.php +++ b/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); } /** diff --git a/tests/TestCase/FileFactoryTest.php b/tests/TestCase/FileFactoryTest.php index c8b02bb..33d5bbf 100644 --- a/tests/TestCase/FileFactoryTest.php +++ b/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'); diff --git a/tests/TestCase/FileTest.php b/tests/TestCase/FileTest.php index 7705ddd..7203822 100644 --- a/tests/TestCase/FileTest.php +++ b/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()); + } } diff --git a/tests/TestCase/PathBuilder/PathBuilderTest.php b/tests/TestCase/PathBuilder/PathBuilderTest.php index 748dcd1..9c1c58d 100644 --- a/tests/TestCase/PathBuilder/PathBuilderTest.php +++ b/tests/TestCase/PathBuilder/PathBuilderTest.php @@ -15,9 +15,11 @@ namespace PhpCollective\Test\TestCase\PathBuilder; use DateTime; +use DateTimeInterface; use PhpCollective\Infrastructure\Storage\FileFactory; use PhpCollective\Infrastructure\Storage\PathBuilder\PathBuilder; use PhpCollective\Infrastructure\Storage\Processor\Image\ImageVariantCollection; +use PhpCollective\Infrastructure\Storage\Utility\NoopFilenameSanitizer; use PhpCollective\Test\TestCase\TestCase; /** @@ -30,19 +32,14 @@ class PathBuilderTest extends TestCase */ public function testDatePaths(): void { - /** @var \PhpCollective\Infrastructure\Storage\PathBuilder\PathBuilder|\PHPUnit\Framework\MockObject\MockObject $builder */ - $builder = $this->getMockBuilder(PathBuilder::class) - ->setConstructorArgs([ - [ - 'pathTemplate' => '{year}{ds}{month}{ds}{day}{ds}{hour}{ds}{minute}', - ], - ]) - ->onlyMethods(['getDateObject']) - ->getMock(); - - $builder->expects($this->any()) - ->method('getDateObject') - ->willReturn((new DateTime('2020-01-01T20:00:00'))); + $builder = new class ([ + 'pathTemplate' => '{year}{ds}{month}{ds}{day}{ds}{hour}{ds}{minute}', + ]) extends PathBuilder { + protected function getDateObject(): DateTimeInterface + { + return new DateTime('2020-01-01T20:00:00'); + } + }; $file = $this->getFixtureFile('titus.jpg'); $file = FileFactory::fromDisk($file, 'local') @@ -68,6 +65,71 @@ public function testPathWithEmptyPlaceHolders(): void $this->assertEquals($this->sanitizeSeparator('/fe/c3/b4/914e151291534253a81e7ee2edc1d973/titus.jpg'), $result); } + /** + * @return void + */ + public function testConfiguredFilenameSanitizerIsUsed(): void + { + $file = $this->getFixtureFile('titus.jpg'); + $file = FileFactory::fromDisk($file, 'local') + ->withUuid('914e1512-9153-4253-a81e-7ee2edc1d973') + ->withFilename('foo bar.jpg'); + + $builder = new PathBuilder([ + 'filenameSanitizer' => new NoopFilenameSanitizer(), + ]); + + $result = $builder->path($file); + + $this->assertStringContainsString( + $this->sanitizeSeparator('/foo bar.jpg'), + $result, + ); + } + + /** + * @return void + */ + public function testConfiguredRandomPathAndDirectorySeparatorAreUsed(): void + { + $file = $this->getFixtureFile('titus.jpg'); + $file = FileFactory::fromDisk($file, 'local') + ->withUuid('914e1512-9153-4253-a81e-7ee2edc1d973'); + + $builder = new PathBuilder([ + 'directorySeparator' => '-', + 'randomPath' => static fn (string $uuid, int $level): string => 'custom-path', + ]); + + $result = $builder->path($file); + + $this->assertEquals( + '-custom-path-914e151291534253a81e7ee2edc1d973-titus.jpg', + $result, + ); + } + + /** + * @return void + */ + public function testSha1RandomPathUsesConfiguredDirectorySeparator(): void + { + $file = $this->getFixtureFile('titus.jpg'); + $file = FileFactory::fromDisk($file, 'local') + ->withUuid('914e1512-9153-4253-a81e-7ee2edc1d973'); + + $builder = new PathBuilder([ + 'directorySeparator' => '-', + ]); + + $result = $builder->path($file); + + $this->assertSame( + '-fe-c3-b4-914e151291534253a81e7ee2edc1d973-titus.jpg', + $result, + ); + } + /** * @return void */ diff --git a/tests/TestCase/StorageServiceTest.php b/tests/TestCase/StorageServiceTest.php index 4b4b832..1c0dade 100644 --- a/tests/TestCase/StorageServiceTest.php +++ b/tests/TestCase/StorageServiceTest.php @@ -14,7 +14,9 @@ namespace PhpCollective\Test\TestCase; +use League\Flysystem\Config; use League\Flysystem\Local\LocalFilesystemAdapter; +use PhpCollective\Infrastructure\Storage\AdapterCollection; use PhpCollective\Infrastructure\Storage\StorageAdapterFactory; use PhpCollective\Infrastructure\Storage\StorageAdapterFactoryInterface; use PhpCollective\Infrastructure\Storage\StorageService; @@ -70,4 +72,35 @@ public function testStorage(): void $service->removeFile('local', '/horse/photo.jpg'); $this->assertFalse($service->fileExists('local', '/horse/photo.jpg')); } + + /** + * @return void + */ + public function testStoreFileUsesStreamWrite(): void + { + $adapter = $this->createMock(LocalFilesystemAdapter::class); + $adapter->expects($this->once()) + ->method('writeStream') + ->with( + '/horse/photo.jpg', + $this->callback(static fn ($resource): bool => is_resource($resource)), + $this->isInstanceOf(Config::class), + ); + $adapter->expects($this->never()) + ->method('write'); + + $collection = new AdapterCollection(); + $collection->add('local', $adapter); + + $service = new StorageService( + new StorageAdapterFactory(), + $collection, + ); + + $service->storeFile( + 'local', + '/horse/photo.jpg', + $this->getFixtureFile('titus.jpg'), + ); + } }