Uploaded image for project: 'Doctrine 2 - ORM'
  1. Doctrine 2 - ORM
  2. DDC-1734

Uninitialized proxies cannot be serialized properly

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.4
    • Component/s: ORM
    • Security Level: All
    • Labels:
      None

      Description

      I have the following entity:

      class Currency
      {
          /** @Id @Column(type="string") */
          protected $code;
              
          /** @Column(type="string") */    
          protected $name;
      }
      

      When I serialize an object graph which references a Currency proxy, but this proxy is initialized, then this works as expected:

      O:65:"Application\Domain\Proxy\__CG__\Application\Domain\Model\Currency":3:{s:17:"__isInitialized__";b:1;s:7:"*code";s:3:"USD";s:7:"*name";s:20:"United States Dollar";}
      

      However, if the proxy is not initialized, then all properties are serialized as null, and the identity is lost:

      O:65:"Application\Domain\Proxy\__CG__\Application\Domain\Model\Currency":3:{s:17:"__isInitialized__";b:0;s:7:"*code";N;s:7:"*name";N;}
      

      That makes a merge() impossible once stored in a session, as the identity of the entity is now unknown.

      I think the solution might be:

      • either to have the proxy's $_identity field serialized as well
      • or to have the $code (identity field) populated when the proxy is created.

        Issue Links

          Activity

          benjamin Benjamin Morel created issue -
          benjamin Benjamin Morel made changes -
          Field Original Value New Value
          Description I have the following entity:

          {code}
          class Currency
          {
              /** @Id @Column(type="string") */
              protected $code;
                  
              /** @Column(type="string") */
              protected $name;
          }
          {code}

          When I serialize an object graph which references a Currency proxy, but this proxy is initialized, then this works as expected:

          {code}
          O:65:"Application\Domain\Proxy\__CG__\Application\Domain\Model\Currency":3:{s:17:"__isInitialized__";b:1;s:7:"*code";s:3:"USD";s:7:"*name";s:20:"United States Dollar";}
          {code}

          However, if the proxy is not initialized, then all properties are serialized as null, and the identity is lost:

          {code}
          O:65:"Application\Domain\Proxy\__CG__\Application\Domain\Model\Currency":3:{s:17:"__isInitialized__";b:0;s:7:"*code";N;s:7:"*name";N;}
          {code}

          That makes a merge() impossible once stored in a session, as the identity of the entity is now unknown.

          I think the solution might be:
          - either to have the proxy's "_identity" field serialized as well
          - or to have the "code" (identity field) populated when the proxy is created.
          I have the following entity:

          {code}
          class Currency
          {
              /** @Id @Column(type="string") */
              protected $code;
                  
              /** @Column(type="string") */
              protected $name;
          }
          {code}

          When I serialize an object graph which references a Currency proxy, but this proxy is initialized, then this works as expected:

          {code}
          O:65:"Application\Domain\Proxy\__CG__\Application\Domain\Model\Currency":3:{s:17:"__isInitialized__";b:1;s:7:"*code";s:3:"USD";s:7:"*name";s:20:"United States Dollar";}
          {code}

          However, if the proxy is not initialized, then all properties are serialized as null, and the identity is lost:

          {code}
          O:65:"Application\Domain\Proxy\__CG__\Application\Domain\Model\Currency":3:{s:17:"__isInitialized__";b:0;s:7:"*code";N;s:7:"*name";N;}
          {code}

          That makes a merge() impossible once stored in a session, as the identity of the entity is now unknown.

          I think the solution might be:
          - either to have the proxy's $_identity field serialized as well
          - or to have the $code (identity field) populated when the proxy is created.
          Hide
          ocramius Marco Pivetta added a comment - - edited

          I am personally for dropping the `$_identifier` field and using the entity fields themselves. Will check this tomorrow. (also: not sure if it is possible, just checking)

          Show
          ocramius Marco Pivetta added a comment - - edited I am personally for dropping the `$_identifier` field and using the entity fields themselves. Will check this tomorrow. (also: not sure if it is possible, just checking)
          Hide
          beberlei Benjamin Eberlei added a comment -

          This is not fixable. The Proxy requires access to the Entity Persister when unserialized, but that is not available anymore. Proxies are broken if unserialized and then accessed, i believe an exception is thrown in this case.

          Show
          beberlei Benjamin Eberlei added a comment - This is not fixable. The Proxy requires access to the Entity Persister when unserialized, but that is not available anymore. Proxies are broken if unserialized and then accessed, i believe an exception is thrown in this case.
          beberlei Benjamin Eberlei made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Can't Fix [ 7 ]
          Hide
          benjamin Benjamin Morel added a comment -

          Of course they cannot be initialized once unserialized, but should'nt be merge()able to a new EntityManager anyway?

          What I would imagine is:

          Show
          benjamin Benjamin Morel added a comment - Of course they cannot be initialized once unserialized, but should'nt be merge()able to a new EntityManager anyway? What I would imagine is: Serialize the identity as part of the proxy If used when unserialized, throw an exception (see http://www.doctrine-project.org/jira/browse/DDC-1732 ) If found during a merge(), replace the proxy with a fresh proxy/entity from the current EntityManager
          beberlei Benjamin Eberlei made changes -
          Workflow jira [ 13570 ] jira-feedback [ 15292 ]
          beberlei Benjamin Eberlei made changes -
          Workflow jira-feedback [ 15292 ] jira-feedback2 [ 17156 ]
          beberlei Benjamin Eberlei made changes -
          Workflow jira-feedback2 [ 17156 ] jira-feedback3 [ 19410 ]
          Hide
          benjamin Benjamin Morel added a comment -

          @Marco Pivetta This is a great idea. Have you checked if it is possible?
          I'm really surprised that a proxied entity's identity is lost forever upon serialization.

          Show
          benjamin Benjamin Morel added a comment - @Marco Pivetta This is a great idea. Have you checked if it is possible? I'm really surprised that a proxied entity's identity is lost forever upon serialization.
          Hide
          mnapoli Matthieu Napoli added a comment -

          Can this issue be reopened please?

          As Benjamin Morel said, it is expected that the proxy is broken once unserialized, but it should be possible to merge to the EntityManager and then use it.

          For now, objects graphs with proxies can't be serialized! It could be if the merge would resolve the situation.

          I'm giving it a try by serializing the $_identifier field in the Proxy

          Thanks

          Show
          mnapoli Matthieu Napoli added a comment - Can this issue be reopened please? As Benjamin Morel said, it is expected that the proxy is broken once unserialized, but it should be possible to merge to the EntityManager and then use it. For now, objects graphs with proxies can't be serialized! It could be if the merge would resolve the situation. I'm giving it a try by serializing the $_identifier field in the Proxy Thanks
          Show
          ocramius Marco Pivetta added a comment - Benjamin Morel you can check my implementation for DCOM-96 at https://github.com/doctrine/common/pull/168 and https://github.com/doctrine/doctrine2/pull/406
          Show
          ocramius Marco Pivetta added a comment - Matthieu Napoli can you try adding $identifier at https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/Proxy/ProxyFactory.php#L319-L323 ?
          Hide
          mnapoli Matthieu Napoli added a comment -

          Marco Pivetta Yes that's what I'm planning to do. I've written a testcase that reproduces the bug right now.

          Show
          mnapoli Matthieu Napoli added a comment - Marco Pivetta Yes that's what I'm planning to do. I've written a testcase that reproduces the bug right now.
          Hide
          ocramius Marco Pivetta added a comment -

          Matthieu Napoli awesome. If you can attach the test here, it would be possible to add it to the proxy refactoring.

          Show
          ocramius Marco Pivetta added a comment - Matthieu Napoli awesome. If you can attach the test here, it would be possible to add it to the proxy refactoring.
          Hide
          mnapoli Matthieu Napoli added a comment -

          @ocramius I will make a pull request on Github and post the link here.

          However I'm sort of stuck right now: I've added '_identifier' to the fields serialized, but it's not enough. To merge, the UoW needs to initialize the proxy to get the identifiers (because $_identifier is private in the proxy). Problem: $_entityPersister (used to initialize the proxy) is also private.

          So I can neither get the identifier of the proxy (even for creating a new proxy for example), nor feed it a new entityPersister to load it... The solution would be to change the proxy a bit (like adding a "__setEntityPersister" method for example) but I guess that changing the Proxy interface in "Doctrine\Common" is a large change and that will not be accepted as a pull request...

          Any ideas ?

          Show
          mnapoli Matthieu Napoli added a comment - @ocramius I will make a pull request on Github and post the link here. However I'm sort of stuck right now: I've added '_identifier' to the fields serialized, but it's not enough. To merge, the UoW needs to initialize the proxy to get the identifiers (because $_identifier is private in the proxy). Problem: $_entityPersister (used to initialize the proxy) is also private. So I can neither get the identifier of the proxy (even for creating a new proxy for example), nor feed it a new entityPersister to load it... The solution would be to change the proxy a bit (like adding a "__setEntityPersister" method for example) but I guess that changing the Proxy interface in "Doctrine\Common" is a large change and that will not be accepted as a pull request... Any ideas ?
          Hide
          ocramius Marco Pivetta added a comment -

          Matthieu Napoli I wouldn't start hacking on that right now... The new impl allows injecting an initializer. A hack may be ok-ish, but the API is already at its EOL.

          Show
          ocramius Marco Pivetta added a comment - Matthieu Napoli I wouldn't start hacking on that right now... The new impl allows injecting an initializer. A hack may be ok-ish, but the API is already at its EOL.
          Hide
          ocramius Marco Pivetta added a comment -

          Matthieu Napoli I'd think of marking this as to be fixed in 2.4.x

          Show
          ocramius Marco Pivetta added a comment - Matthieu Napoli I'd think of marking this as to be fixed in 2.4.x
          Hide
          mnapoli Matthieu Napoli added a comment - - edited

          Marco Pivetta I'm working on the master branch, it's 2.4 right? I don't see anything different for the proxies in this branch (I don't see any way to inject an initializer).

          Or is it your PR (https://github.com/doctrine/common/pull/168) maybe?

          Show
          mnapoli Matthieu Napoli added a comment - - edited Marco Pivetta I'm working on the master branch, it's 2.4 right? I don't see anything different for the proxies in this branch (I don't see any way to inject an initializer). Or is it your PR ( https://github.com/doctrine/common/pull/168 ) maybe?
          Hide
          benjamin Benjamin Morel added a comment -

          I'm happy that this bug is taken care of, I think that's a major problem that's been dismissed a bit quickly.
          Can the bug be reopened, with fix version 2.4, to avoid forgetting it once more?

          Show
          benjamin Benjamin Morel added a comment - I'm happy that this bug is taken care of, I think that's a major problem that's been dismissed a bit quickly. Can the bug be reopened, with fix version 2.4, to avoid forgetting it once more?
          Hide
          mnapoli Matthieu Napoli added a comment -
          Show
          mnapoli Matthieu Napoli added a comment - Benjamin Morel +1
          Hide
          ocramius Marco Pivetta added a comment -

          Matthieu Napoli yes, that's the PR. Please let me know for the tests so that I can add them to doctrine/common.
          I'm re-opening the issue

          Show
          ocramius Marco Pivetta added a comment - Matthieu Napoli yes, that's the PR. Please let me know for the tests so that I can add them to doctrine/common. I'm re-opening the issue
          Hide
          ocramius Marco Pivetta added a comment -

          Re-opening issue - need test to verify the behavior before proceeding.

          Show
          ocramius Marco Pivetta added a comment - Re-opening issue - need test to verify the behavior before proceeding.
          ocramius Marco Pivetta made changes -
          Resolution Can't Fix [ 7 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Assignee Benjamin Eberlei [ beberlei ] Marco Pivetta [ ocramius ]
          ocramius Marco Pivetta made changes -
          Status Reopened [ 4 ] Awaiting Feedback [ 10000 ]
          Hide
          mnapoli Matthieu Napoli added a comment - - edited

          Marco Pivetta Sorry the test was on my computer at work, here it is now: https://gist.github.com/4193721

          Just to be clear: I created 2 test methods, the first one works (there is no serialization step), the second doesn't. This is to make it clear that the serialization step is the problem. I hope this is alright.

          Show
          mnapoli Matthieu Napoli added a comment - - edited Marco Pivetta Sorry the test was on my computer at work, here it is now: https://gist.github.com/4193721 Just to be clear: I created 2 test methods, the first one works (there is no serialization step), the second doesn't. This is to make it clear that the serialization step is the problem. I hope this is alright.
          Hide
          ocramius Marco Pivetta added a comment - - edited

          Matthieu Napoli thank you for the test. But please fix the tests so that both fail (if these are actually two verified cases).

          Show
          ocramius Marco Pivetta added a comment - - edited Matthieu Napoli thank you for the test. But please fix the tests so that both fail (if these are actually two verified cases).
          Hide
          mnapoli Matthieu Napoli added a comment -

          Marco Pivetta No those aren't 2 different cases, they are the same, it's just that one fails because we use serialization (this is the problem described in this ticket), and the other one works because we don't use serialization (and it only exists to check that everything works if we don't use serialization). Actually I think the "working test" has no reason to exist really, I just made it to "prove" that the problem was indeed the serialization, so it can be removed.

          Show
          mnapoli Matthieu Napoli added a comment - Marco Pivetta No those aren't 2 different cases, they are the same, it's just that one fails because we use serialization (this is the problem described in this ticket), and the other one works because we don't use serialization (and it only exists to check that everything works if we don't use serialization). Actually I think the "working test" has no reason to exist really, I just made it to "prove" that the problem was indeed the serialization, so it can be removed.
          Hide
          ocramius Marco Pivetta added a comment -

          Matthieu Napoli ok, so I can basically strip the working one

          Show
          ocramius Marco Pivetta added a comment - Matthieu Napoli ok, so I can basically strip the working one
          Show
          ocramius Marco Pivetta added a comment - Matthieu Napoli I deployed a fix in DCOM-96 ( https://github.com/doctrine/doctrine2/pull/406 https://github.com/Ocramius/doctrine2/commit/1424cc781277a4f8183229c5e8d838724e1ca879 )
          ocramius Marco Pivetta made changes -
          Status Awaiting Feedback [ 10000 ] In Progress [ 3 ]
          Hide
          mnapoli Matthieu Napoli added a comment -

          Marco Pivetta fantastic!

          Show
          mnapoli Matthieu Napoli added a comment - Marco Pivetta fantastic!
          Hide
          beberlei Benjamin Eberlei added a comment -

          A related Github Pull-Request [GH-247] was opened
          https://github.com/doctrine/common/pull/247

          Show
          beberlei Benjamin Eberlei added a comment - A related Github Pull-Request [GH-247] was opened https://github.com/doctrine/common/pull/247
          Hide
          beberlei Benjamin Eberlei added a comment -

          A related Github Pull-Request [GH-247] was closed
          https://github.com/doctrine/common/pull/247

          Show
          beberlei Benjamin Eberlei added a comment - A related Github Pull-Request [GH-247] was closed https://github.com/doctrine/common/pull/247
          Hide
          ocramius Marco Pivetta added a comment -

          Resolved in DCOM-96

          Show
          ocramius Marco Pivetta added a comment - Resolved in DCOM-96
          ocramius Marco Pivetta made changes -
          Status In Progress [ 3 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          beberlei Benjamin Eberlei made changes -
          Fix Version/s 2.4 [ 10321 ]
          ocramius Marco Pivetta made changes -
          Link This issue is referenced by DDC-3368 [ DDC-3368 ]

          This list may be incomplete, as errors occurred whilst retrieving source from linked applications:

          • Request to http://www.doctrine-project.org/fisheye/ failed: Error in remote call to 'FishEye 0 (http://www.doctrine-project.org/fisheye/)' (http://www.doctrine-project.org/fisheye) [AbstractRestCommand{path='/rest-service-fe/search-v1/crossRepositoryQuery', params={expand=changesets[0:20].revisions[0:29],reviews, query=DDC-1734}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)

            People

            • Assignee:
              ocramius Marco Pivetta
              Reporter:
              benjamin Benjamin Morel
            • Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: