Search code examples
phpsymfonyvichuploaderbundledoctrine-extensions

Symfony vich uploader and doctrine loggable extension problem?


I am using this two libraries to create an entity that has a picture using vich/uploader-bundle and I am logging entity changes history using the loggable doctrine extension provided from stof/doctrine-extensions-bundle which provides the extension from atlantic18/doctrineextensions.

So here is the problem: I have an entity that has a Vich uploadable picture field and it is using doctrine's Gedmo loggable extension with annotations.

/**
 * @var VersionedFile
 *
 * @ORM\Embedded(class="App\Entity\Embedded\VersionedFile")
 *
 * @Gedmo\Versioned()
 */
private $picture;

/**
 * @var File
 *
 * @Vich\UploadableField(
 *     mapping="user_picture",
 *     fileNameProperty="picture.name",
 *     size="picture.size",
 *     mimeType="picture.mimeType",
 *     originalName="picture.originalName",
 *     dimensions="picture.dimensions
 * )
 */
private $pictureFile;

/**
 * @var DateTimeInterface
 *
 * @ORM\Column(type="datetime", nullable=true)
 *
 * @Gedmo\Versioned()
 */
private $pictureUpdatedAt;

The embedded entity class App\Entity\Embedded\VersionedFile has all the needed annotations in order to version properly using the loggable doctrine extension.

// Not the whole code but just to get the idea for property versioning

/**
 * @ORM\Column(name="name", nullable=true)
 *
 * @Gedmo\Versioned()
 */
protected $name;

And now the problem. When I upload the file and persist the entity the following thing happens. The entity manager persist the entity and the onFlush method of the Gedmo loggable listener (Gedmo\Loggable\LoggableListener) is called. This listeners checks the changes and schedules log entries to be inserted.

The problem is that the VichUploaders upload listener (Vich\UploaderBundle\EventListener\Doctrine\UploadListener) is called after the loggable listener and then the file is uploaded which changes the properties name, size, etc. The computed changes about name, size, etc. are not available in theLoggableListener` becaues it is called first and so it doesn't know that they should be inserted.

Am I missing some configuration or am I doing something wrong. The idea is to log changes made to the picture. For now in the database the log entries consist only of the $pictureUpdatedAt field.

I debugged the problem and all I can see is the order and that in LoggableListener the method getObjectChangeSetData is returning only the $pictureUpdatedAt field that has changed. I don't think this has something in common with the Embedded entity because I think the calling order of the listeners is the problem. The first idea I had was to change the listeners priority but even if I do that the order of the calling is not changed mainly because when onFlush is called it is triggering the preUpdate method which triggers the UploadListener of the uploader bundle.


Solution

  • You are correct, the root of the problem is the UploadListener listens to prePersist and preUpdate while the LoggableListener listens to onFlush. Since onFlush is triggered before preUpdate, file changes are never logged. This can be fixed in a few steps.

    1. Create New UploadListener

    First, you can write your own UploadListener to listen to onFlush instead.

    // src/EventListener/VichUploadListener.php using Flex
    // src/AppBundle/EventListener/VichUploadListener.php otherwise
    namespace App\EventListener;
    
    use Doctrine\ORM\Event\LifecycleEventArgs;
    use Doctrine\ORM\Event\OnFlushEventArgs;
    use Doctrine\ORM\Events;
    use Vich\UploaderBundle\EventListener\Doctrine\UploadListener;
    
    class VichUploadListener extends UploadListener
    {
        public function onFlush(OnFlushEventArgs $args): void
        {
            $em = $args->getEntityManager();
            $uow = $em->getUnitOfWork();
    
            foreach ($uow->getScheduledEntityUpdates() as $entity) {
                $this->preUpdate(new LifecycleEventArgs($entity, $em));
            }
    
            // Required if using property namer on sluggable field. Otherwise, you
            // can also subscribe to "prePersist" and remove this foreach.
            foreach ($uow->getScheduledEntityInsertions() as $entity) {
                // We use "preUpdate" here so the changeset is recomputed.
                $this->preUpdate(new LifecycleEventArgs($entity, $em));
            }
        }
    
        public function getSubscribedEvents(): array
        {
            return [Events::onFlush];
        }
    }
    

    In this example, I reuse the original UploadListener to make things easier. Since we are listening to onFlush, it is important we recompute the entity changeset after the file is uploaded which is why I used the "preUpdate" method for both scheduled updates and inserts.

    You do have to be careful when changing events like this. If you have another listener that expects the value of one of your file fields to be set (or unset), this may change the expected behavior. This is especially true if you use the second foreach to handle new uploads. prePersist is triggered before onFlush, so this would make new uploads get set later than before.

    2. Create New CleanListener

    Next, we now have to create a new CleanListener. This listener deletes old files when we update the file field if delete_on_update is set to true. Since it listens to preUpdate, we have to change it to onFlush so old files are properly deleted.

    // src/EventListener/VichCleanListener.php on Flex
    // src/AppBundle/EventListener/VichCleanListener.php otherwise
    namespace App\EventListener;
    
    use Doctrine\ORM\Event\LifecycleEventArgs;
    use Doctrine\ORM\Event\OnFlushEventArgs;
    use Doctrine\ORM\Events;
    use Vich\UploaderBundle\EventListener\Doctrine\CleanListener;
    
    class VichCleanListener extends CleanListener
    {
        public function onFlush(OnFlushEventArgs $args): void
        {
            $em = $args->getEntityManager();
            $uow = $em->getUnitOfWork();
    
            foreach ($uow->getScheduledEntityUpdates() as $entity) {
                $this->preUpdate(new LifecycleEventArgs($entity, $em));
            }
        }
    
        public function getSubscribedEvents(): array
        {
            return [Events::onFlush];
        }
    }
    

    3. Configure New Listeners

    Now, we need to override the default listeners in our config with the ones we just wrote.

    # config/services.yaml on Flex
    # app/config/services.yml otherwise
    services:
        # ...
    
        vich_uploader.listener.upload.orm:
            class: 'App\EventListener\VichUploadListener'
            parent: 'vich_uploader.listener.doctrine.base'
            autowire: false
            autoconfigure: false
            public: false
        vich_uploader.listener.clean.orm:
            class: 'App\EventListener\VichCleanListener'
            parent: 'vich_uploader.listener.doctrine.base'
            autowire: false
            autoconfigure: false
            public: false
    

    4. Change Gedmo Extension Priorities

    If all that wasn't enough, now comes the other problem you brought up: listener priority. At a minimum, we need to make sure LoggableListener is triggered after our upload/clean listeners. If you are using any of the other Gedmo extensions, you need to make sure they are loaded in the order you need them. The defaults set by VichUploaderExtension set the CleanListener to 50 and the UploadListener to 0. You can see the Gedmo Listener defaults in StofDoctrineExtensionsExtension.

    For me, I have a property namer that depends on a sluggable field, so I want to make sure SluggableListener is called before the UploadListener. I also use softdeleteable and want soft deletes logged as "remove", so I want to make sure LoggableListener is registered before SoftDeleteableListener. You can change these priorities by overriding the services in your config.

    # config/services.yaml on Flex
    # app/config/services.yml otherwise
    services:
        # ...
    
        stof_doctrine_extensions.listener.sluggable:
            class: '%stof_doctrine_extensions.listener.sluggable.class%'
            autowire: false
            autoconfigure: false
            public: false
            calls:
                - { method: 'setAnnotationReader', arguments: ['@annotation_reader'] }
            tags:
                - { name: 'doctrine.event_subscriber', connection: 'default', priority: 5 }
    
        stof_doctrine_extensions.listener.loggable:
            class: '%stof_doctrine_extensions.listener.loggable.class%'
            autowire: false
            autoconfigure: false
            public: false
            calls:
                - { method: 'setAnnotationReader', arguments: ['@annotation_reader'] }
            tags:
                - { name: 'doctrine.event_subscriber', connection: 'default', priority: -1 }
    
        stof_doctrine_extensions.listener.softdeleteable:
            class: '%stof_doctrine_extensions.listener.softdeleteable.class%'
            autowire: false
            autoconfigure: false
            public: false
            calls:
                - { method: 'setAnnotationReader', arguments: ['@annotation_reader'] }
            tags:
                - { name: 'doctrine.event_subscriber', connection: 'default', priority: -2 }
    

    Alternatively, you could create a compiler pass to just change the priorities of the doctrine.event_subscriber tags for these services.

    // src/DependencyInjection/Compiler/DoctrineExtensionsCompilerPass.php on Flex
    // src/AppBundle/DependencyInjection/Compiler/DoctrineExtensionsCompilerPass.php otherwise
    namespace App\DependencyInjection\Compiler;
    
    use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
    use Symfony\Component\DependencyInjection\ContainerBuilder;
    
    class DoctrineExtensionsCompilerPass implements CompilerPassInterface
    {
        public function process(ContainerBuilder $container)
        {
            $listenerPriorities = [
                'sluggable' => 5,
                'loggable' => -1,
                'softdeleteable' => -2,
            ];
    
            foreach ($listenerPriorities as $ext => $priority) {
                $id = sprintf('stof_doctrine_extensions.listener.%s', $ext);
    
                if (!$container->hasDefinition($id)) {
                    continue;
                }
    
                $definition = $container->getDefinition($id);
                $tags = $definition->getTag('doctrine.event_subscriber');
                $definition->clearTag('doctrine.event_subscriber');
    
                foreach ($tags as $tag) {
                    $tag['priority'] = $priority;
                    $definition->addTag('doctrine.event_subscriber', $tag);
                }
            }
        }
    }
    

    If you go this route, make sure to register the compiler pass with a higher priority (higher than 0) to ensure it is ran before RegisterEventListenersAndSubscribersPass.

    // src/Kernel.php on Flex
    // src/AppBundle/AppBundle.php otherwsie
    
    // ...
    
    use App\DependencyInjection\Compiler\DoctrineExtensionsCompilerPass;
    use Symfony\Component\DependencyInjection\Compiler\PassConfig;
    use Symfony\Component\DependencyInjection\ContainerBuilder;
    
    // ...
    
    protected function build(ContainerBuilder $container)
    {
        $container->addCompilerPass(new DoctrineExtensionsCompilerPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 5);
    }
    

    Now, just ensure your cache is cleared.