Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

delete_on_update does not remove the original file #1410

Open
GlucNAc opened this issue Nov 7, 2023 · 2 comments
Open

delete_on_update does not remove the original file #1410

GlucNAc opened this issue Nov 7, 2023 · 2 comments
Labels

Comments

@GlucNAc
Copy link

GlucNAc commented Nov 7, 2023

Q A
Bundle version 2.2.0
Symfony version 6.2.10
PHP version 8.1.23

Support Question

Hi :)

I have the basic configuration so delete_on_update is set to true. But when I upload another image, the old one is not deleted. Using the interactive debugger, this is due to Vich\UploaderBundle\Handler\UploadHandler::hasUploadedFile() :

protected function hasUploadedFile(object $obj, PropertyMapping $mapping): bool
{
    $file = $mapping->getFile($obj);

    return $file instanceof UploadedFile || $file instanceof ReplacingFile;
}

The $file variable is an instance of Symfony\Component\HttpFoundation\File\File so this method always return false.

In my opinion, it is impossible to reach this stage as we will never deal with an UploadedFile here because the file has already been moved and so converted into its File instance (which is reported here but is normally fixed). I have overcome this problem using the custom listener below but I think I have missed somthing with the delete_on_update option.

use Symfony\Component\EventDispatcher\Attribute\AsEventListener;
use Vich\UploaderBundle\Event\Event;
use Vich\UploaderBundle\Event\Events;
use Vich\UploaderBundle\Handler\UploadHandler;

class VichCleanListener
{
    public function __construct(
        private readonly UploadHandler $handler,
    ) {
    }

    #[AsEventListener(Events::PRE_UPLOAD)]
    public function preUpload(Event $event): void
    {
        $this->handler->clean($event->getObject(), 'imageFile');
    }
}

Here is the relevant part of my entity Article :

#[Vich\UploadableField(mapping: 'xxx.article_image', fileNameProperty: 'imageName', mimeType: 'imageMimeType')]
#[Assert\File(maxSize: '5M', extensions: self::IMAGE_FILE_EXTENSIONS)]
private File|null $imageFile = null;

#[ORM\Column(type: Types::STRING, nullable: true)]
#[Gedmo\Versioned]
private string|null $imageName = null;

#[ORM\Column(type: Types::STRING, nullable: true)]
private string|null $imageMimeType = null;

And here is the yaml config :

vich_uploader:
    db_driver: 'orm'
    storage: 'flysystem'

    mappings:
        xxx.article_image:
            uri_prefix: '/uploads/articles/images'
            upload_destination: 'xxx.article_image.storage'
            namer: 'Vich\UploaderBundle\Naming\SmartUniqueNamer'
@garak garak added the Bug label Nov 11, 2023
@garak
Copy link
Collaborator

garak commented Nov 11, 2023

I tried on a project of mine and indeed the old file is not removed.
Sadly, I also failed to understand the reason, nor I can currently understand how your proposed listener can work. It's still calling the clean method and so the check is still against the same types, right?

@GlucNAc
Copy link
Author

GlucNAc commented Nov 23, 2023

Sorry for the late answer.

Actually, the listener I propose does work because when I upload a new image, it will be stored as an UploadedFile in the property $imageFile of my entity Article.

When executing handler->clean() method, it will check if the current $object (Article in my case) has an UploadedFile:

protected function hasUploadedFile(object $obj, PropertyMapping $mapping): bool
{
$file = $mapping->getFile($obj);
return $file instanceof UploadedFile || $file instanceof ReplacingFile;
}

Here, $mapping->getFile($obj); will read the property $imageFile and thus will return true.

Then, when executing the remove method, the script will read the value of $imageName (not $imageFile) because the remove method take the field name to be removed as an argument:

public function clean(object $obj, string $fieldName): void
{
$mapping = $this->getMapping($obj, $fieldName);
// nothing uploaded, do not remove anything
if (!$this->hasUploadedFile($obj, $mapping)) {
return;
}
$this->remove($obj, $fieldName);
}

And I call the clean method like this in my custom listener: $this->handler->clean($event->getObject(), 'imageFile');, so what will be removed is the "old" image.

Finally, the UploadedFile will be converted into a File instance, and $imageName will now have the new file name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants