mirror of
https://github.com/doctrine/orm.git
synced 2026-04-25 15:38:10 +02:00
Problem with Proxies on PHP 7.4.2 with typed properties #6402
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @bizley on GitHub (Feb 19, 2020).
(I'm sorry if this is wrong place to report that issue, please transfer me to the proper one if this is the case)
We encountered this problem on the Behat testing phase with generated entity proxies. There is fatal error:
This happened when we upgraded PHP to 7.4.2 (everything was ok on 7.4.1). We used typed properties before the upgrade as well. After playing with the entity for a bit we discovered that now every typed object property must be initialized immediately (previously they were initialized in the constructor) like:
because this doesn't work anymore:
This is was already reported in Symfony https://github.com/symfony/symfony/issues/35574 but closed.
@ostrolucky commented on GitHub (Feb 19, 2020):
I don't see any DoctrineBundle issue here, moving to ORM
@beberlei commented on GitHub (Feb 19, 2020):
@bizley you are wrong though, it is still possible to set the property in
__constructor everywhere else, its not possible to access it if its not initialized. We probably need to add guards in __sleep.However why is sleep called anywys, please provide the whole stack trace for the error.
@bizley commented on GitHub (Feb 19, 2020):
I'm not saying it's not possible - I'm saying that it's triggering fatal error for proxy object, and yes, for the reason you mentioned. I just don't understand why it's suddenly happening in 7.4.2.
I will provide stack trace in few minutes, need to recreate the environment.
@bizley commented on GitHub (Feb 19, 2020):
Here is
__sleepfromProxies\__CG__\App\Entity\OrderTask:@beberlei commented on GitHub (Feb 19, 2020):
@dunglas looping you in as this seems to be an interaction between APIPlatfoorm, Symfony Serializer and Doctrine entities.
@EmanuelOster commented on GitHub (Feb 19, 2020):
I can confirm this. API Platform injects an
object_to_populateinto the$contextvariable during normalization for PATCH and PUT requests. This object is a doctrine entity and might have some doctrine proxy classes for relations.A possible solution would be to change
AbstractObjectNormalizersgetCacheKeyfunction and make it catch all Throwables instead of just Exceptions.However it is still unclear to me which property is supposedly uninitialized in that proxy class. I couldn't find any upon inspection with xdebug
@ZhukV commented on GitHub (Feb 20, 2020):
Same problem (non initialized object) exist in
UnitOfWork.If the entity contain typed property with nullable flag, after persist and flush we receive error:
It's can be fixed with next solution:
@ZhukV commented on GitHub (Feb 20, 2020):
In embedded properties (
\Doctrine\ORM\Mapping\ReflectionEmbeddedProperty) we've same errors.If property not initialized and we try to load entity we receive next error:
The property
$moneyis embedded.But in this case we've a critical bug, because this property can't contain null value by default (value set from constructor) and we can't assign null by default.
@bizley commented on GitHub (Feb 20, 2020):
Ok, now I understand why it's happening in PHP 7.4.2 - https://bugs.php.net/bug.php?id=79002
Turns out
serializewith__sleepwas behaving incorrectly before and allowing objects with uninitialized properties to be serialized (unserialize()was throwing TypeError then) and it was fixed in 7.4.2 (https://www.php.net/ChangeLog-7.php).Right now I see 3 possible solutions:
ErrorinAbstractObjectNormalizer::getCacheKey(like @EmanuelOster suggested) and returnfalsethere since object with uninitialized property can not be serialized anyway).ReflectionProperty::isInitialized).@beberlei commented on GitHub (Feb 20, 2020):
@ZhukV are you using the latest versions of doctrine/persistence (1.3.6) and doctrine/reflection (1.1)? Your bug should have been fixed with them.
@greedyivan commented on GitHub (Feb 21, 2020):
Do not use
privateproperties in entities: Projects/ORM/Documentation/Architecture#Serializing entities. Doctrine does not have full support for that.@Ocramius commented on GitHub (Feb 22, 2020):
Uhm, yes, entities+private properties is OK. __sleep/__wakeup are kinda
edge cases
On Fri, Feb 21, 2020, 21:51 Ivan Grigoriev notifications@github.com wrote:
@ZhukV commented on GitHub (Feb 22, 2020):
@beberlei , thank! After update to latest versions all correct work.
@beberlei commented on GitHub (Mar 1, 2020):
@bizley i would think option 2 is the correct one from my PoV, the other two are not really good and your initial bug report seems to be not true that it happens because of the constructor, but specificially from sleep.
@bizley commented on GitHub (Mar 1, 2020):
@beberlei thank you for looking at this case but I believe that you missed something here completely.
First of all, if you are referring to my list of possible solutions - number 2 is quite a joke that I hoped is obvious (apparently it is not). You can not initialize immediately every property in every possible case, and what is most important you can not possibly believe that you can force users to do that. Number 1 is a Symfony thing and I believe they should take care of it (as in "catch Error and not just Exception"). So I think number 3 is only viable solution (as in "fix sleep in proxies").
Second - I only described symptoms and the way to reproduce the problem in the bug description so yes - it is possible to live with current state of Doctrine and PHP 7.4.2. Still, the problem with sleep is there and should be fixed. I'm not sure what kind of a reporting policy is working here - should I open another ticket basically referring everything in this one?
I'm a bit disappointed how all this is handled right now.
@beberlei commented on GitHub (Mar 1, 2020):
@bizley I am a bit disappointed as well, that you provide a joke as an option and then feel offended when I pick it.
From the description I cannot see how this is a Doctrine bug yet. Maybe if you provide a reproducable test-case using
EntityManager#getReference(creates a proxy) and serializing it will show the error. I only provide my free time here, I can't be responsible to invest hours understanding every bug report, especially when it doesn't come with a reproducible test case.See https://github.com/doctrine/reflection/blob/master/phpunit.xml.dist#L12 how you can create a test in a special directory that only runs on a specific PHP version as this must be 7.4 only.
@bizley commented on GitHub (Mar 1, 2020):
My apologies then. I'll try to work on the test case. And I think this issue should be moved to doctrine/common since this is where ProxyGenerator is placed.
@chrBrd commented on GitHub (Mar 9, 2020):
@bizley
This example given in your first comment:
will delay the issue presenting itself, but as described in the comment by @EmanuelOster using
PUTandPATCHwill make it show up again.Consequently, even though it was tongue-in-cheek, this won't actually work:
@bizley commented on GitHub (Mar 9, 2020):
@chrBrd you are absolutely right. The problem is with proxy generator creating proxies with uninitialized typed properties in its __sleep and probably object normalizer not catching all errors.
@chrBrd commented on GitHub (Mar 9, 2020):
@bizley - looks like not entirely. While I agree with you this is an issue with the proxy generator, it turns out I hadn't quite initialized everything. I pasted your example above to save time, but the actual code snippet I'm looking into this with is:
I noticed I was still getting the error with
private $question = null;, so I had a look at the inverse:Changing that to
private ?Collection $answers = null;"fixes" it.Immediately initializing every typed property is definitely more of a workaround than a fix though, especially as it necessitates setting
nullvalues on properties that shouldn't be nullable.@beberlei, I completely understand why you want a reproducible test case for this, but I feel the issue may have been closed prematurely.
@bizley commented on GitHub (Mar 9, 2020):
I was wondering why doctrine/common's ProxyLogicTypedPropertiesTest passes when there is at least one uninitialized typed property in tested object and its proxy is serialized in the test.
Turns out that every uninitialized field is actually being initialized in the setup phase of that test which is why this issue I've reported goes undetected. You can easily verify it by adding additional
public string $test;inLazyLoadableObjectWithTypedPropertiesand runningDoctrine\Tests\Common\Proxy\ProxyLogicTypedPropertiesTest::testNotInitializedProxyUnserialization. The result is thenAnd please don't get me wrong, I know this test is to verify lazy loading and properties should be initialized but still I think it makes my point.
Will it be enough for reproducible test case @beberlei ? Once again sorry for sounding harsh before. You guys are doing great job, and I really appreciate it.
@someniatko commented on GitHub (Mar 23, 2020):
Hi there!
I get
with Embeddables when using
doctrine/reflection1.2.0. It's fine with1.1.0.tried
doctrine/orm2.7.1and2.7.2@dunglas commented on GitHub (Apr 3, 2020):
Sorry for the late reply @beberlei. Thanks to @alanpoulain it's gonna be fixed in Symfony: https://github.com/symfony/symfony/pull/36332
@berkut1 commented on GitHub (Jul 21, 2020):
@dunglas It looks like the problem still here.
I temporarily fixed by adding the Id field:
Also the first version works if remove the Item typed property, like it is in the php 7.3
Symfony 4.4.10 with last packages.
@dunglas commented on GitHub (Jul 21, 2020):
My PR hasn't been merged in Symfony. Has you can see in the comments, not everybody agree that it's the proper fix.