dustin10 / VichUploaderBundle

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

Vich\Uploadable not working correctly with OneToOne relation #1409

Open davidrojo opened 12 months ago

davidrojo commented 12 months ago

Bug Report

Q A
BC Break no
Bundle version 2.2.0
Symfony version 5.4.21
PHP version 8.2.5

Summary

When having an entity with a relation OneToOne with other entity with a VichUploadable file, on changing the relation and flushing the related entity is not properly updated in php, although in database is properly updated. A refresh of the entity is required to get the updated value.

Current behavior

I have an entity Item with a relation OneToOne with ItemImage that is a Vich\Uploadable entity. When updating this entity, after flushing it to the database, the previous entity is loaded in the relation without an Id instead the new entity that has been stored in the database.

$item = $this->entityManager->getRepository(Item::class)->findOneById(1);
// (1) $item->getImage() returns a Doctrine proxy ItemImage with an Id
$item->setImage(new ItemImage($file));
// (2) $item->getImage() returns an ItemImage without an id, as it has not been saved into the database
$this->entityManager->flush();
// (3) $item->getImage() returns the ItemImage object (not the proxy) of step (1) with id=null, instead of the new ItemImage with the id

How to reproduce

This is the source code:

Item.php

#[ORM\Entity]
class Item
{
    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\Column(type: 'integer')]
    protected ?int $id = null;

    #[ORM\OneToOne(inversedBy: 'target', targetEntity: ItemImage::class, cascade: ['all'], orphanRemoval: true)]
    protected ?ItemImage $image = null;

    ....getters and setters
}

ItemImage.php

#[ORM\Entity]
#[Vich\Uploadable]
class ItemImage
{
    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\Column(type: 'integer')]
    protected ?int $id;

    #[ORM\OneToOne(mappedBy: 'image', targetEntity: Item::class)]
    protected ?Item $target = null;

    #[Vich\UploadableField(
        mapping: 'profile',
        fileNameProperty: 'name'
    )]
    protected ?File $file = null;

    #[ORM\Column(type: 'string', length: 255)]
    #[Groups(['GeneralRead'])]
    protected ?string $name = '';

    #[ORM\Column(type: 'datetime_immutable', nullable: true)]
    protected ?DateTimeImmutable $uploadedAt;

    public function __construct(?File $file)
    {
        $this->setFile($file);
    }

    public function setFile(?File $file): self
    {
        $this->file = $file;
        if ($file instanceof UploadedFile) {
            $this->uploadedAt = new DateTimeImmutable();
        }
        return $this;
    }

    ... getters and setters
}

vich_uploader.yaml

vich_uploader:
    db_driver: orm
    metadata:
        type: attribute
    mappings:
        profile:
            uri_prefix: '%download.path.profile%'
            upload_destination: '%upload.path.profile%'
            namer: Vich\UploaderBundle\Naming\UniqidNamer
            inject_on_load: true
            directory_namer:
                service: Vich\UploaderBundle\Naming\CurrentDateTimeDirectoryNamer
                options:
                    date_time_property: uploadedAt

Execute the following code:

$file = new UploadedFile(
    $path,
    'sample-image.jpg',
    mime_content_type($path),
    false,
    true
);

$item = $this->entityManager->getRepository(Item::class)->findOneById(1);
dump($item->getImage());

imagen

$item->setImage(new ItemImage($file));
dump($item->getImage());

imagen

$this->entityManager->flush();
dump($item->getImage());

imagen

$this->entityManager->refresh($item);
dump($item->getImage());

imagen

Expected behavior

The third dump, shold be the same as the fourth dump. Shoud be a ItemImage entity with id=11 and filename ...584.jpg, instead the id is null and the filename is ...440.jpg (the original filename before being replaced).

If before setting the new image, you fully load the entity (doing a $item->getImage()->getFile() for example) then the first dump is not a Proxy, and it works fine. So there must be an issue with the use of Proxies.

Removing #[Vich\Uploadable] from ItemImage, works fine (although the image will not be handled) so it seems that is not an issue of doctrine.

garak commented 12 months ago

My suggestion is avoiding to use entities with autoincrement ids or in general entities which depend on their database status, which defeats the main purpose of using a data mapper (independence between entities and the persistence layer)

davidrojo commented 12 months ago

@garak the problem is not the id, it is the path of file returned. The id is just another indicator that something is not working properly. In the third dump it returns the file ...440.jpg (which is the original file before replacing the relation), after calling refresh() it returns the file ...584.jpg which is the correct one.

jorismak commented 8 months ago

Using 2.2.0, the old problem of not working on Doctrine proxy objects is there again.

I think this might be related / the same thing?

I have an entity Quote, which has a (1 to 1 I think, so same thing by accident?) relation to an entity Country. On Country I have an upload for an image. The mapped property for Vich is stored in the database, and is filled. But requesting the File object from the entity returns null.

If I request any property from the Country entity beforehand, now Vich returns a proper File object. So, if the entity is actually a proxy object that isn't yet initialized, Vich will not load the object (any more) and simply return null.

There are old issues about the same thing that had fixes: https://github.com/dustin10/VichUploaderBundle/issues/28

I encounter this with an old Symfony 3.4 project that has been upgraded to Symfony 6.