Metadata is wrong when using markEntityToUpload with two entity classes

[JoolsMcFly]

Hi guys,

I found a bug and know how to fix it but I want to log it here first.

I inject \Stof\DoctrineExtensionsBundle\Uploadable\UploadableManager into my controller.
I then use it to upload two files to two entities.
Upload fails on the second one because lib/Gedmo/Uploadable/UploadableListener.php uses metadata of the first entity for both. getFilePathFieldValue will then complain because the entity does not match the metadata.

        // lib/Gedmo/Uploadable/UploadableListener.php:115
        $meta = $om->getClassMetadata(get_class($first['entity']));
        $config = $this->getConfiguration($om, $meta->name);

        foreach ($this->fileInfoObjects as $info) {
            $entity = $info['entity'];

            // If the entity is in the identity map, it means it will be updated. We need to force the
            // "dirty check" here by "modifying" the path. We are actually setting the same value, but
            // this will mark the entity as dirty, and the "onFlush" event will be fired, even if there's
            // no other change in the entity's fields apart from the file itself.
            if ($uow->isInIdentityMap($entity)) {
                if ($config['filePathField']) {
                    $path = $this->getFilePathFieldValue($meta, $config, $entity);
                    $uow->propertyChanged($entity, $config['filePathField'], $path, $path);
                } else {
                    $fileName = $this->getFileNameFieldValue($meta, $config, $entity);
                    $uow->propertyChanged($entity, $config['fileNameField'], $fileName, $fileName);
                }
                $uow->scheduleForUpdate($entity);
            }
        }

Easy fix would be to move these two lines inside the foreach loop but it would have a negative impact on performance.

        $meta = $om->getClassMetadata(get_class($first['entity']));
        $config = $this->getConfiguration($om, $meta->name);

How about a caching mechanism to avoid calling getClassMetadata and getConfiguration?

        $metaByClass = [];
        $configByClass = [];
        foreach ($this->fileInfoObjects as $info) {
            $entity = $info['entity'];

            /** changes start here **/
            $entityClass = get_class($info['entity'];

            if (!isset($metaByClass[$entityClass])) {
                $metaByClass[$entityClass] = $om->getClassMetadata($entityClass));
                $configByClass[$entityClass] = $om->getClassMetadata($entityClass));
            }
            $meta = $metaByClass[$entityClass];
            $config = $configByClass[$entityClass];
            /** changes end here **/

            // If the entity is in the identity map, it means it will be updated. We need to force the
            // "dirty check" here by "modifying" the path. We are actually setting the same value, but
            // this will mark the entity as dirty, and the "onFlush" event will be fired, even if there's
            // no other change in the entity's fields apart from the file itself.
            if ($uow->isInIdentityMap($entity)) {
                if ($config['filePathField']) {
                    $path = $this->getFilePathFieldValue($meta, $config, $entity);
                    $uow->propertyChanged($entity, $config['filePathField'], $path, $path);
                } else {
                    $fileName = $this->getFileNameFieldValue($meta, $config, $entity);
                    $uow->propertyChanged($entity, $config['fileNameField'], $fileName, $fileName);
                }
                $uow->scheduleForUpdate($entity);
            }
        }

What do you guys think? Good enough for a PR?

Thanks!

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[franmomu]

Hey! I know it's has been a while since the issue was open, but just came across this and it should be fine to just move those lines inside the loop since there are already cache mechanism in ObjectManager::getClassMetadata and MappedEventSubscriber::getConfiguration methods just in case someone wants to give it a try.

[JoolsMcFly]

Hi Fran.

Thanks for resurrecting this thread. I’ll see if I can find time to work on this.