diff --git a/docs/events/events.md b/docs/events/events.md index 3d7fbc1e..711195d1 100644 --- a/docs/events/events.md +++ b/docs/events/events.md @@ -5,14 +5,16 @@ the flux of other code, in this case into the flux of VichUploaderBundle. The following is a list of events you can listen to: -| Event name | Event constant | Trigger point| -|------------|----------------|--------------| -|`vich_uploader.pre_upload`|`Events::PRE_UPLOAD`|before a file upload is handled| -|`vich_uploader.post_upload`|`Events::POST_UPLOAD`|right after a file upload is handled| -|`vich_uploader.pre_inject`|`Events::PRE_INJECT`|before a file is injected into an entity| -|`vich_uploader.post_inject`|`Events::POST_INJECT`|after a file is injected into an entity| -|`vich_uploader.pre_remove`|`Events::PRE_REMOVE`|before a file is removed| -|`vich_uploader.post_remove`|`Events::POST_REMOVE`|after a file is removed| +| Event name | Event constant | Trigger point | +|------------------------------|------------------------|------------------------------------------| +| `vich_uploader.pre_upload` | `Events::PRE_UPLOAD` | before a file upload is handled | +| `vich_uploader.post_upload` | `Events::POST_UPLOAD` | right after a file upload is handled | +| `vich_uploader.pre_inject` | `Events::PRE_INJECT` | before a file is injected into an entity | +| `vich_uploader.post_inject` | `Events::POST_INJECT` | after a file is injected into an entity | +| `vich_uploader.pre_remove` | `Events::PRE_REMOVE` | before a file is removed | +| `vich_uploader.post_remove` | `Events::POST_REMOVE` | after a file is removed | +| `vich_uploader.upload_error` | `Events::UPLOAD_ERROR` | If file upload failed. | +| `vich_uploader.remove_error` | `Events::REMOVE_ERROR` | If file remove failed. | The `vich_uploader.pre_remove` event is cancelable, that means that the actual remove request will not take place, and you have to take action. @@ -51,4 +53,42 @@ services: - { name: kernel.event_listener, event: vich_uploader.pre_upload } ``` +## Error events + +The `UPLOAD_ERROR` event happens when writing a file fails, and before an exception is thrown. + +If an error occurs while removing a file, then the `REMOVE_ERROR` event gets fired. Failures in removing +files do not trigger any exceptions unless you choose to throw an error in the event handler for the `REMOVE_ERROR` +event. + +You can use the event to throw an error when the error occurs: + +```php + 'onUploadError', + ] + } + public function onUploadError(ErrorEvent $errorEvent) + { + $object = $event->getObject(); + $mapping = $event->getMapping(); + $exception = $errorEvent->getThrowable(); + + throw $exception; + } + +} +``` + [Return to the index](../index.md) diff --git a/src/Event/ErrorEvent.php b/src/Event/ErrorEvent.php new file mode 100644 index 00000000..a8473588 --- /dev/null +++ b/src/Event/ErrorEvent.php @@ -0,0 +1,21 @@ +throwable = $throwable; + } + + public function getThrowable(): \Throwable + { + return $this->throwable; + } +} diff --git a/src/Event/Events.php b/src/Event/Events.php index 4bba080b..09c82878 100644 --- a/src/Event/Events.php +++ b/src/Event/Events.php @@ -54,4 +54,14 @@ final class Events * @Event("Vich\UploaderBundle\Event\Event") */ public const POST_REMOVE = 'vich_uploader.post_remove'; + + /** + * Triggered if writing to storage fails. + */ + public const UPLOAD_ERROR = 'vich_uploader.upload_error'; + + /** + * Triggered if removing the file from storage fails. + */ + public const REMOVE_ERROR = 'vich_uploader.remove_error'; } diff --git a/src/Handler/UploadHandler.php b/src/Handler/UploadHandler.php index d0563f2b..01c47cea 100644 --- a/src/Handler/UploadHandler.php +++ b/src/Handler/UploadHandler.php @@ -4,6 +4,7 @@ use Symfony\Component\HttpFoundation\File\UploadedFile; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; +use Vich\UploaderBundle\Event\ErrorEvent; use Vich\UploaderBundle\Event\Event; use Vich\UploaderBundle\Event\Events; use Vich\UploaderBundle\FileAbstraction\ReplacingFile; @@ -46,8 +47,12 @@ public function upload(object $obj, string $fieldName): void } $this->dispatch(Events::PRE_UPLOAD, new Event($obj, $mapping)); - - $this->storage->upload($obj, $mapping); + try { + $this->storage->upload($obj, $mapping); + } catch (\Throwable $exception) { + $this->dispatch(Events::UPLOAD_ERROR, new ErrorEvent($obj, $mapping, $exception)); + throw $exception; + } $this->injector->injectFile($obj, $mapping); $this->dispatch(Events::POST_UPLOAD, new Event($obj, $mapping)); @@ -93,8 +98,11 @@ public function remove(object $obj, string $fieldName): void if ($preEvent->isCanceled()) { return; } - - $this->storage->remove($obj, $mapping); + try { + $this->storage->remove($obj, $mapping); + } catch (\Throwable $exception) { + $this->dispatch(Events::REMOVE_ERROR, new ErrorEvent($obj, $mapping, $exception)); + } $mapping->erase($obj); $this->dispatch(Events::POST_REMOVE, new Event($obj, $mapping)); diff --git a/src/Storage/FileSystemStorage.php b/src/Storage/FileSystemStorage.php index 228373b9..79d8d89f 100644 --- a/src/Storage/FileSystemStorage.php +++ b/src/Storage/FileSystemStorage.php @@ -41,11 +41,19 @@ protected function doRemove(PropertyMapping $mapping, ?string $dir, string $name { $file = $this->doResolvePath($mapping, $dir, $name); - return \file_exists($file) && \unlink($file); + if (!\file_exists($file) || !unlink($file)) { + throw new \Exception('Cannot remove file '.$file); + } + + return true; } - protected function doResolvePath(PropertyMapping $mapping, ?string $dir, string $name, ?bool $relative = false): string - { + protected function doResolvePath( + PropertyMapping $mapping, + ?string $dir, + string $name, + ?bool $relative = false + ): string { $path = !empty($dir) ? $dir.\DIRECTORY_SEPARATOR.$name : $name; if ($relative) { diff --git a/src/Storage/FlysystemStorage.php b/src/Storage/FlysystemStorage.php index 6316a3b9..3d894afc 100644 --- a/src/Storage/FlysystemStorage.php +++ b/src/Storage/FlysystemStorage.php @@ -56,13 +56,9 @@ protected function doRemove(PropertyMapping $mapping, ?string $dir, string $name $fs = $this->getFilesystem($mapping); $path = !empty($dir) ? $dir.'/'.$name : $name; - try { - $fs->delete($path); + $fs->delete($path); - return true; - } catch (FilesystemException) { - return false; - } + return true; } protected function doResolvePath(PropertyMapping $mapping, ?string $dir, string $name, ?bool $relative = false): string diff --git a/src/Storage/GaufretteStorage.php b/src/Storage/GaufretteStorage.php index b4868b98..849b1da6 100644 --- a/src/Storage/GaufretteStorage.php +++ b/src/Storage/GaufretteStorage.php @@ -3,7 +3,6 @@ namespace Vich\UploaderBundle\Storage; use Gaufrette\Adapter\MetadataSupporter; -use Gaufrette\Exception\FileNotFound; use Gaufrette\FilesystemInterface; use Gaufrette\FilesystemMapInterface; use Symfony\Component\HttpFoundation\File\File; @@ -46,11 +45,7 @@ protected function doRemove(PropertyMapping $mapping, ?string $dir, string $name $filesystem = $this->getFilesystem($mapping); $path = !empty($dir) ? $dir.'/'.$name : $name; - try { - return $filesystem->delete($path); - } catch (FileNotFound) { - return false; - } + return $filesystem->delete($path); } protected function doResolvePath(PropertyMapping $mapping, ?string $dir, string $name, ?bool $relative = false): string diff --git a/src/Storage/StorageInterface.php b/src/Storage/StorageInterface.php index 6489b31a..d605d961 100644 --- a/src/Storage/StorageInterface.php +++ b/src/Storage/StorageInterface.php @@ -23,6 +23,8 @@ public function upload(object $obj, PropertyMapping $mapping): void; * * @param object $obj The object * @param PropertyMapping $mapping The mapping representing the field to remove + * + * @throw \Exception Throws an exception */ public function remove(object $obj, PropertyMapping $mapping): ?bool; diff --git a/tests/Handler/UploadHandlerTest.php b/tests/Handler/UploadHandlerTest.php index c8ad7ee7..ca32b6a4 100644 --- a/tests/Handler/UploadHandlerTest.php +++ b/tests/Handler/UploadHandlerTest.php @@ -4,6 +4,7 @@ use PHPUnit\Framework\MockObject\MockObject; use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\HttpFoundation\File\Exception\CannotWriteFileException; use Vich\TestBundle\Entity\Article; use Vich\UploaderBundle\Event\Event; use Vich\UploaderBundle\Event\Events; @@ -191,6 +192,79 @@ public function testRemove(): void $this->handler->remove($this->object, self::FILE_FIELD); } + public function testRemoveFailsInStorageDriverEmitsEvent(): void + { + $this->expectEvents([Events::PRE_REMOVE, Events::REMOVE_ERROR, Events::POST_REMOVE]); + + $this->mapping + ->expects(self::once()) + ->method('getFileName') + ->with($this->object) + ->willReturn('something not null'); + + $this->mapping + ->expects(self::once()) + ->method('erase') + ->with($this->object); + + $this->storage + ->expects(self::once()) + ->method('remove') + ->with($this->object, $this->mapping) + ->willThrowException(new \RuntimeException('Test exception')) + ; + + $this->handler->remove($this->object, self::FILE_FIELD); + } + + public function testUploadFailsEmitsEventAndException(): void + { + $this->expectException(\RuntimeException::class); + + $this->expectEvents([Events::PRE_UPLOAD, Events::UPLOAD_ERROR]); + + $this->mapping + ->expects(self::once()) + ->method('getFile') + ->with($this->object) + ->willReturn($this->getUploadedFileMock()); + + $this->storage + ->expects(self::once()) + ->method('upload') + ->with($this->object, $this->mapping) + ->willThrowException(new \RuntimeException('This is a test')); + + $this->injector + ->expects(self::never()) + ->method('injectFile') + ->with($this->object, $this->mapping); + + $this->handler->upload($this->object, self::FILE_FIELD); + } + + public function testremoveFailsWithCannotWriteException(): void + { + $this->mapping + ->expects(self::once()) + ->method('getFileName') + ->with($this->object) + ->willReturn('something not null'); + + $this->mapping + ->expects(self::once()) + ->method('erase') + ->with($this->object); + + $this->storage + ->expects(self::once()) + ->method('remove') + ->with($this->object, $this->mapping) + ->willThrowException(new CannotWriteFileException('this is a test')); + + $this->handler->remove($this->object, self::FILE_FIELD); + } + public function testRemoveIfEventIsCanceled(): void { $this->expectEvents([Events::PRE_REMOVE]); diff --git a/tests/Storage/FileSystemStorageTest.php b/tests/Storage/FileSystemStorageTest.php index 9856c2ac..33465593 100644 --- a/tests/Storage/FileSystemStorageTest.php +++ b/tests/Storage/FileSystemStorageTest.php @@ -42,6 +42,8 @@ public function testRemoveSkipsEmptyFilenameProperties(?string $propertyValue): */ public function testRemoveSkipsNonExistingFile(): void { + $this->expectException(\Exception::class); + $this->mapping ->expects(self::once()) ->method('getUploadDir') diff --git a/tests/Storage/GaufretteStorageTest.php b/tests/Storage/GaufretteStorageTest.php index 8d8bfe21..3b34a19a 100644 --- a/tests/Storage/GaufretteStorageTest.php +++ b/tests/Storage/GaufretteStorageTest.php @@ -172,6 +172,8 @@ public function testThatRemoveMethodDoesDeleteFile(): void */ public function testRemoveNotFoundFile(): void { + // the exception is caught in the UploadHandler. + $this->expectException(FileNotFound::class); $this->mapping ->method('getUploadDestination') ->willReturn('filesystemKey');