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

Already on GitHub? Sign in to your account

Wrong UnitOfWork::computeChangeSet() #617

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
7 participants

Sometimes some fields are Proxy when compute "changeSet". If it is Proxy, some listeners - example Gedmo sortable listener - belive the value has changed and this leads to chaos.

I check the $actualValue, if it is Proxy, the value didn't change.

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DDC-2354

Owner

Ocramius commented Mar 16, 2013

what is the case when this happens? Can you add a test for it?

I don't know exactly where the problem is.
This will happen when I run fixtures generate. I create ProjectStatuses, there are two fields:

  • project_status_category : @Gedmo\SortableGroup and @ORM\ManyToOne(targetEntity="ProjectStatusCategory", inversedBy="project_statuses")
  • position : @Gedmo\SortablePosition

Everything is all right, the values of position are good.

After, when I create the Projects:

$project = new Project();
$project
    ->...
    ->setProjectStatus($manager->merge($project_status));
$manager->persist($project);
//...
$manager->flush()

Then Gedmo SortableListener::processUpdate() function is running, and it is checking the changes. It belives the @Gedmo\SortableGroup field (project_status_category) changed, because it is in $changeSet var.

private function processUpdate($em, $config, $meta, $object)
{
    $uow = $em->getUnitOfWork();

    $changed = false;
    $changeSet = $uow->getEntityChangeSet($object);

    $groups = $this->getGroups($meta, $config, $object);
    foreach (array_keys($groups) as $group) {
        // $group = 'project_status_category'
        // array_key_exists is true!!!
        $changed = $changed || array_key_exists($group, $changeSet);
    }
    // ...
    // $changed is true, it doesn't stop!
    if (!$changed) return;

    // modify positions but new values are wrong

The getEntityChangeSet() returns the wrong values. Old and new value is Proxy, but different. The category didn't change.

Owner

Ocramius commented Mar 16, 2013

@fchris82 by "different proxy" do you mean a proxy with same identifier and different object hash? That should not even be possible.

I saw these in xdebug and in var_damp new and old values, AND var_dump the new and old "getId()" values. The old and new "Proxy" values are different, but the ids are equal! I don't know why. If it helps you, I can send the entity and the fixtures files.

I didn't want the debug, solve and understand this row:

->setProjectStatus($manager->merge($project_status));

This mades something, and than the $changeSet = $uow->getEntityChangeSet($object); row give wrong values.

Owner

Ocramius commented Mar 16, 2013

@fchris82 a more correct fix would be to check why these instances are swapped. That should not ever happen.

Contributor

hacfi commented Aug 7, 2013

I’m experiencing the same behavior and had similar problems with PHPCR ODM as discussed over at doctrine/phpcr-odm#297 and doctrine/phpcr-odm#305.

I’m loading entities from the database which have a mapped string field. It contains a serialized reference to a PHPCR ODM Document (for example a:1:{s:2:"id";s:43:"/ecommerce/product/properties/wuz";}) which gets converted to the references object or a proxy to load it in the postLoad event. When I flush without changing those entities it adds them to the entityChangeSets and entityUpdates property and overwrites the originalEntityData. In the preUpdate event I convert the proxy object back to the serialized reference string and run recomputeSingleEntityChangeSet again. It compares the string with the proxy object because it checks against originalEntityData and adds the string to the changeset although that value is already stored in the database. From my point of view this is not desired behavior. I currently misuse the preFlush event to check all the documents in the UnitOfWork if they need to be be serialized so they don’t get added to the changeset.

Member

stof commented Aug 7, 2013

@hacfi Are you putting the unserialized object in the same PHP property than the one used by Doctrine to store the serialized data ? If yes, it is totally expected that Doctrine see a change in the object as it contains something else. I suggest you to use a different property (not mapped by Doctrine) in your object to store the unserialized object

Contributor

hacfi commented Aug 7, 2013

@stof Yes, I do. That would solve the problem..true. Hoped there would be another solution but it’s ok.

Thanks for always helping out @stof 👍

@hacfi hacfi referenced this pull request in hacfi/ObjectBridgeBundle Oct 24, 2013

Open

Separate properties into serialized string and object #3

Owner

guilhermeblanco commented May 16, 2014

Only possibility I see here would be to not only check for Proxy, but also compare if identifiers of both old and actual values are the same in case both are proxies.
Still, I'm not sold that this issue is valid since something keeps needling my head that you guys are doing some funky stuff to reach this "issue" and it might be outside of Doctrine's core to fix it.

Contributor

flack commented May 16, 2014

@guilhermeblanco Not sure what happens in this particular case, but in my code, I see this type of proxy issues a lot. As a workaround, I wrote some custom change-tracking logic for associations in my own code and I manually set the unchanged ones to __isInitialized__ = false before passing them to persist. It's quite a horrible hack, but without it, I constantly run into exceptions during changeset calculations.

I guess you could say I'm doing "funky stuff" (my doctrine code is an emulation layer for a legacy active entity orm which doesn't use the identity map pattern, so I have to use the api somewhat creatively...), but it would definitely be good if Doctrine could handle these proxy issues a bit nicer. If you don't like the instanceof check, how about having the changeset calculation compare the associations' identifiers?

Owner

guilhermeblanco commented May 17, 2014

@flack the problem I see around that section is because you must have different values on old and actual when doing that comparison.
For objects (associations, collections and embedded), a possibility is that during your script execution you must be creating a new object representing same instance. This can easily happen:
1- You are unserialize()ing an object and it has same identifier
2- You're intentionally skipping identity map checks on UnitOfWork when creating references
3- Creating an object by hand and assigning its identifier (maybe you have either a setId() or your id generation is none)

So, all these situations could lead to this behavior you are experiencing... do you know if any of these apply?

Contributor

flack commented Jun 17, 2014

@guilhermeblanco: sorry for the late reply, I have been busy elsewhere...

None of the three scenarios you describe apply to my code, I don't do anything with unserialze(), I only create references with $em->getReference(), and I don't set IDs manually (actually, I have a code path that does that, but it is almost never used).

What I do a lot is detach()ing and merge()ing. I'm not sure if this has any effect on Proxies, but I think my problem is a little different from what is described here (see https://github.com/doctrine/doctrine2/pull/843/files#diff-437ce752cacf84ce55978270de0198a6R47 for what I think is causing proxy issues in my code)

Owner

Ocramius commented Nov 11, 2014

Closing as no failing test case was provided.

@Ocramius Ocramius closed this Nov 11, 2014

@Ocramius Ocramius self-assigned this Nov 11, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment