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

POPOs data is not being persisted #21

Closed
ghost opened this issue Jun 22, 2017 · 13 comments
Closed

POPOs data is not being persisted #21

ghost opened this issue Jun 22, 2017 · 13 comments
Labels

Comments

@ghost
Copy link

ghost commented Jun 22, 2017

Thanks for your nice bundle. I got the issue: if I change the json_document related plain php object property, it won't persisted by default.

...
$foo->misc[0]->setName('New Name');
$em->persist($foo);
$em->flush(); //nothing to update by uow
@Dragonqos
Copy link

Dragonqos commented Oct 16, 2017

I have resolved this issue with DoctrineEventSubscriber

<?php

use Doctrine\Common\Inflector\Inflector;
use Doctrine\ORM\Event\PreUpdateEventArgs;
use Doctrine\ORM\Events;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\Common\EventSubscriber;

class JsonDocumentSubscriber implements EventSubscriber
{
    /**
     * @return array
     */
    public function getSubscribedEvents(): array
    {
        return [
            Events::preUpdate
        ];
    }

    /**
     * @param PreUpdateEventArgs $event
     */
    public function preUpdate(PreUpdateEventArgs $event): void
    {
        $subject = $event->getObject();

        $em   = $event->getEntityManager();
        $uow  = $em->getUnitOfWork();
        $meta = $em->getClassMetadata(get_class($subject));

        $jsonObjects = $this->filterJsonObjects($meta);

        if($this->forceUpdateJsonDocument($jsonObjects, $subject)) {
            $uow->recomputeSingleEntityChangeSet($meta, $subject);
        }
    }

    /**
     * @param ClassMetadata $meta
     *
     * @return array
     */
    private function filterJsonObjects(ClassMetadata $meta): array 
    {
        $jsonObjects = [];
        $fieldMappings = $meta->fieldMappings;

        if(is_array($fieldMappings)) {
            foreach($fieldMappings as $filed => $options) {
                if($options['type'] === 'json_document') {
                    $jsonObjects[] = $filed;
                }
            }
        }

        return $jsonObjects;
    }

    /**
     * @param array $fields
     * @param       $object
     *
     * @return bool
     */
    private function forceUpdateJsonDocument(array $fields, $object): bool
    {
        $hasUpdates = false;

        if(!empty($fields)) {
            foreach($fields as $fieldName) {
                $methodGet = 'get'.Inflector::camelize($fieldName);
                $methodSet = 'set'.Inflector::camelize($fieldName);

                $newValue = null;
                $actualValue = $object->{$methodGet}();
                if(is_array($actualValue)) {
                    $newValue = [];
                    foreach ($actualValue as $k => $v) {
                        $newValue[$k] = clone $v;
                    }
                }

                if(is_object($actualValue)) {
                    $newValue =  clone $actualValue;
                }

                if(null !== $newValue) {
                    $object->{$methodSet}($newValue);
                    $hasUpdates = true;
                }
            }
        }

        return $hasUpdates;
    }
}

@thatside-zaraffa
Copy link

@Dragonqos How are your entities mapped? I can't get this to work - preUpdate event isn't fired. Or maybe you have other fields which are changed too?

@Dragonqos
Copy link

@thatside-zaraffa Yes, I am updating "updated_at" field manually when needed, to fire event.

@thatside-zaraffa
Copy link

@Dragonqos Seems it is the reason of your subscriber working :)
In my case I do not update any field except json_document one so preUpdate isn't called.
I'll write down the solution here when I'll find one.

@thatside-zaraffa
Copy link

Actually I've come to same solution - added updatedAt field and update it manually. Not the greatest way to work...

@thatside-zaraffa
Copy link

The problem is with the way UOW compares objects - it is simple "equals" comparison ===. I've researched this a little more and found out that it is enough to clone the embedded document to change it's link.
So you need to do this:

$data = clone $foo->misc;
$data[0]->setName('New Name');
$foo->misc = $data;
$em->persist($foo);
$em->flush(); 

My code is slightly different (simple object instead of array as misc field) but this should work.

@dunglas
Copy link
Owner

dunglas commented Nov 19, 2017

Would you be able to open a PR with a failing test?

@anonim1133
Copy link

So regarding to what @thatside-zaraffa said, there's no work for this bundle? Or is there any listener that could get all POPOs and clone them if changed?

@pierrejolly
Copy link

Any news about this issue ?

@Toflar
Copy link
Contributor

Toflar commented Apr 6, 2019

Maybe I can give some background here. So the issue is that Doctrine does not blindly execute UPDATE queries but instead it calculates which properties have changed. If you want to have a look at where it does this, checkout UnitOfWork::computeChangeSet().
Specifically what happens is this here: https://github.com/doctrine/orm/blob/master/lib/Doctrine/ORM/UnitOfWork.php#L607

So if your object stays the same (PHP compares the object reference when you do ===), Doctrine will not notice that a nested entity or property changed. So this is actually not really an issue related to this type but a general Doctrine issue or limitation.
An easy fix would be to clone the object before you set it. That way === would always return false.

Does this help?

@Toflar Toflar added the question label Apr 6, 2019
@pierrejolly
Copy link

Thank you @Toflar , I knew that Doctrine checked the status of the changes before saving but I did not think the object appear as the same even if a nested object was changed :).
I'll simply clone the object before set it.

@dimkasta
Copy link

Is there a way to make the embedded objects behave like associations so that doctrine checks them too (instead of just the parent object) with something like an implicit cascade="persist" ?

@Toflar
Copy link
Contributor

Toflar commented Jun 28, 2019

Imho there's not.
Folks, I'm closing this issue as there's nothing we can do about it and I would like to clean up the issues a little bit :)

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

7 participants