dustin10 / VichUploaderBundle

A simple Symfony bundle to ease file uploads with ORM entities and ODM documents.
MIT License
1.85k stars 519 forks source link

Feature `DownloadHandler` doesn't work #1465

Open NikDevPHP opened 2 months ago

NikDevPHP commented 2 months ago

Bug Report

Q A
BC Break yes
Bundle version 2.4.0
Symfony version 6.4.10
PHP version 8.3

We are encountering an issue where VichUploaderBundle's DownloadHandler is unable to retrieve the absolute file path when using Flysystem as the storage driver. This results in the inability to determine the file's mime-type.

The issue is triggered by the use of Flysystem with inject_on_load: true. The DownloadHandler is expecting an absolute path but is instead receiving a non-absolute path, leading to an error when attempting to fetch the mime-type.

https://github.com/dustin10/VichUploaderBundle/blob/master/src/Handler/DownloadHandler.php#L43

vich_uploader:
    db_driver: orm
    storage: flysystem
    form: false
    metadata:
        type: attribute

    use_flysystem_to_resolve_uri: true

    mappings:
        media:
            uri_prefix: /media
            upload_destination: default.storage
            namer: Vich\UploaderBundle\Naming\UniqidNamer
            directory_namer: Vich\UploaderBundle\Naming\SubdirDirectoryNamer
            delete_on_remove: true
            delete_on_update: true
            inject_on_load: true

when@test:
    vich_uploader:
        mappings:
            media:
                directory_namer: App\Vich\Naming\ParallelThreadsDirectoryNamer

    #[Groups(['read_media'])]
    public ?string $contentUrl = null;

    #[Vich\UploadableField(mapping: 'media', fileNameProperty: 'filePath', originalName: 'originalName')]
    public ?File $file = null;

    #[ORM\Column(type: 'string', length: self::FILE_PATH_MAX_LENGTH, nullable: true)]
    public ?string $filePath = null;

    #[ORM\Column(type: 'string', length: self::ORIGINAL_NAME_MAX_LENGTH)]
    #[Groups(['read_media'])]
    public ?string $originalName = '';

     public function __construct(?UploadedFile $file, string $context)
    {
        $this->file = $file;
        $this->context = $context;

        // For validation, we need to do it in constructor
        if ($file !== null) {
            $this->originalName = $file->getClientOriginalName();
        }
    }

    /**
     * If manually uploading a file (i.e. not using Symfony Form) ensure an instance
     * of 'UploadedFile' is injected into this setter to trigger the update. If this
     * bundle's configuration parameter 'inject_on_load' is set to 'true' this setter
     * must be able to accept an instance of 'File' as the bundle will inject one here
     * during Doctrine hydration.
     */
    public function setFile(?File $file = null): void
    {
        $this->file = $file;

        if (null !== $file) {
            // It is required that at least one field changes if you are using doctrine
            // otherwise the event listeners won't be called and the file is lost
            $this->updatedAt = new \DateTimeImmutable();
        }
    }

final readonly class DownloadMedia
{
    public function __invoke(Media $data, DownloadHandler $handler): Response
    {
        return $handler->downloadObject($data, 'file');
    }
}

image

garak commented 1 month ago

I can't help with FlySytem, I've never used it myself. We recently had many issues with that, where I just trusted other devs about possible solutions. If you'd like to propose a fix, you can open a PR, but I'll need a review from someone else.

Otherwise, you can simply decorate the FlysystemStorage service and adapt it to your needs.