From ac002b84c89daa435e35c87f0ba8f595db8794c4 Mon Sep 17 00:00:00 2001 From: Dave Heineman Date: Sun, 19 May 2024 19:31:09 +0200 Subject: [PATCH] Optionally resolve the uri through flysystem (#1451) --- config/flysystem.xml | 1 + docs/storage/flysystem.md | 34 +++--------- src/DependencyInjection/Configuration.php | 1 + .../VichUploaderExtension.php | 1 + src/Storage/FlysystemStorage.php | 32 ++++++++++- .../AbstractFlysystemStorageTest.php | 53 ++++++++++++++++++- 6 files changed, 93 insertions(+), 29 deletions(-) diff --git a/config/flysystem.xml b/config/flysystem.xml index 6811fe1f..ba5c0771 100644 --- a/config/flysystem.xml +++ b/config/flysystem.xml @@ -8,6 +8,7 @@ + %vich_uploader.use_flysystem_to_resolve_uri% diff --git a/docs/storage/flysystem.md b/docs/storage/flysystem.md index aeafc19e..3e86e7f7 100644 --- a/docs/storage/flysystem.md +++ b/docs/storage/flysystem.md @@ -57,34 +57,14 @@ vich_uploader: ### Issues with adapters public url If you are using certain adapters like S3, Cloudflare, etc., the generated public URL for assets may be incorrect. -To resolve this, you can create your own -[custom storage](https://github.com/dustin10/VichUploaderBundle/blob/master/docs/storage/custom.md) -by duplicating the existing FlysystemStorage and adding this function: - -```php - public function resolveUri(object|array $obj, ?string $fieldName = null, ?string $className = null): ?string - { - $path = $this->resolvePath($obj, $fieldName, $className, true); - - if (empty($path)) { - return null; - } - - $mapping = null === $fieldName ? - $this->factory->fromFirstField($obj, $className) : - $this->factory->fromField($obj, $fieldName, $className); - $fs = $this->getFilesystem($mapping); - - try { - return $fs->publicUrl($path); - } catch (FilesystemException) { - return $mapping->getUriPrefix().'/'.$path; - } - } -``` +To resolve this, you can enable `use_flysystem_to_resolve_uri` in the configuration. + +This setting makes VichUploaderBundle retrieve the public URL from the configured Flysystem storage. +If Flysystem cannot generate a public URL, the bundle will fall back to default behaviour. + +**Note:**: -Be careful with that; if you have existing implementations, -it can break them. See [there](https://github.com/dustin10/VichUploaderBundle/pull/1441). +> Retrieving the public URL is only available when using flysystem version 3.6.0 and up. ## Integrating with [oneup/flysystem-bundle](https://github.com/1up-lab/OneupFlysystemBundle) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 3cfd35ce..ede4efef 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -57,6 +57,7 @@ private function addGeneralSection(ArrayNodeDefinition $node): void ->thenInvalid('The storage %s is not supported. Please choose one of '.\implode(', ', $this->supportedStorages).' or provide a service name prefixed with "@".') ->end() ->end() + ->booleanNode('use_flysystem_to_resolve_uri')->defaultFalse()->end() ->scalarNode('twig')->defaultTrue()->info('twig requires templating')->end() ->scalarNode('form')->defaultTrue()->end() ->end() diff --git a/src/DependencyInjection/VichUploaderExtension.php b/src/DependencyInjection/VichUploaderExtension.php index 89d1c963..6e7db8a3 100644 --- a/src/DependencyInjection/VichUploaderExtension.php +++ b/src/DependencyInjection/VichUploaderExtension.php @@ -39,6 +39,7 @@ public function load(array $configs, ContainerBuilder $container): void // define a few parameters $container->setParameter('vich_uploader.default_filename_attribute_suffix', $config['default_filename_attribute_suffix']); $container->setParameter('vich_uploader.mappings', $config['mappings']); + $container->setParameter('vich_uploader.use_flysystem_to_resolve_uri', $config['use_flysystem_to_resolve_uri']); if (\str_starts_with((string) $config['storage'], '@')) { $container->setAlias('vich_uploader.storage', \substr((string) $config['storage'], 1)); diff --git a/src/Storage/FlysystemStorage.php b/src/Storage/FlysystemStorage.php index 3d894afc..6175071a 100644 --- a/src/Storage/FlysystemStorage.php +++ b/src/Storage/FlysystemStorage.php @@ -22,10 +22,15 @@ final class FlysystemStorage extends AbstractStorage */ protected MountManager|ContainerInterface $registry; + /** + * @var bool use flysystem to resolve the uri + */ + protected bool $useFlysystemToResolveUri; + /** * @param MountManager|ContainerInterface|mixed $registry */ - public function __construct(PropertyMappingFactory $factory, $registry) + public function __construct(PropertyMappingFactory $factory, $registry, bool $useFlysystemToResolveUri = false) { parent::__construct($factory); @@ -34,6 +39,7 @@ public function __construct(PropertyMappingFactory $factory, $registry) } $this->registry = $registry; + $this->useFlysystemToResolveUri = $useFlysystemToResolveUri; } protected function doUpload(PropertyMapping $mapping, File $file, ?string $dir, string $name): void @@ -72,6 +78,30 @@ protected function doResolvePath(PropertyMapping $mapping, ?string $dir, string return $path; } + public function resolveUri(object|array $obj, ?string $fieldName = null, ?string $className = null): ?string + { + if (!$this->useFlysystemToResolveUri) { + return parent::resolveUri($obj, $fieldName, $className); + } + + $path = $this->resolvePath($obj, $fieldName, $className, true); + + if (empty($path)) { + return null; + } + + $mapping = null === $fieldName ? + $this->factory->fromFirstField($obj, $className) : + $this->factory->fromField($obj, $fieldName, $className); + $fs = $this->getFilesystem($mapping); + + try { + return $fs->publicUrl($path); + } catch (FilesystemException) { + return $mapping->getUriPrefix().'/'.$path; + } + } + public function resolveStream(object|array $obj, ?string $fieldName = null, ?string $className = null) { $path = $this->resolvePath($obj, $fieldName, $className, true); diff --git a/tests/Storage/Flysystem/AbstractFlysystemStorageTest.php b/tests/Storage/Flysystem/AbstractFlysystemStorageTest.php index 8aeff0ab..2c25313d 100644 --- a/tests/Storage/Flysystem/AbstractFlysystemStorageTest.php +++ b/tests/Storage/Flysystem/AbstractFlysystemStorageTest.php @@ -27,6 +27,8 @@ abstract class AbstractFlysystemStorageTest extends StorageTestCase protected FilesystemAdapter|MockObject $adapter; + protected bool $useFlysystemToResolveUri = false; + abstract protected function createRegistry(FilesystemOperator $filesystem): MountManager|ContainerInterface; /** @@ -38,7 +40,7 @@ public static function setUpBeforeClass(): void protected function getStorage(): StorageInterface { - return new FlysystemStorage($this->factory, $this->registry); + return new FlysystemStorage($this->factory, $this->registry, $this->useFlysystemToResolveUri); } protected function setUp(): void @@ -158,4 +160,53 @@ public static function pathProvider(): array ['foo', '/absolute/foo/file.txt', false], ]; } + + public function testResolveUri(): void + { + $this->mapping + ->expects(self::once()) + ->method('getUriPrefix') + ->willReturn('/uploads'); + + $this->mapping + ->expects(self::once()) + ->method('getFileName') + ->willReturn('file.txt'); + + $this->factory + ->expects(self::once()) + ->method('fromField') + ->with($this->object, 'file_field') + ->willReturn($this->mapping); + + $path = $this->getStorage()->resolveUri($this->object, 'file_field'); + + self::assertEquals('/uploads/file.txt', $path); + } + + public function testResolveUriThroughFlysystem(): void + { + $this->useFlysystemToResolveUri = true; + + $this->filesystem + ->expects(self::once()) + ->method('publicUrl') + ->with('file.txt') + ->willReturn('example.com/file.txt'); + + $this->mapping + ->expects(self::once()) + ->method('getFileName') + ->willReturn('file.txt'); + + $this->factory + ->expects(self::exactly(2)) + ->method('fromField') + ->with($this->object, 'file_field') + ->willReturn($this->mapping); + + $path = $this->getStorage()->resolveUri($this->object, 'file_field'); + + self::assertEquals('example.com/file.txt', $path); + } }