Static properties fail to be accessed through RuntimeReflectionProperty

[melkamar]

Bug Report

Q	A
Version	3.3.3
Previous Version if the bug is a regression	2.5.7

Summary

After upgrading 2.5.7 → 3.3.3 as a dependency of the Doctrine ODM we are seeing crashes:

Cannot assign null to property RH\OrganisationBundle\Model\Organisation::$type of type string

The catch here is that the $type property is static. But it has been like this for years and years in our codebase and worked OK.

This happens when running the UnitOfWork->merge() method. I have tracked this down to the RuntimeReflectionProperty behaving differently from the native ReflectionProperty for static properties:

<?php

require 'vendor/autoload.php';

class Foo
{
    public static $staticProp = 'staticPropValue';
}

$fooInstance = new Foo();

$propNative = new ReflectionProperty(Foo::class, 'staticProp');
$propDoctrine = new \Doctrine\Persistence\Reflection\RuntimeReflectionProperty(Foo::class, 'staticProp');

var_dump(
    $propNative->getValue(),
    $propNative->getValue($fooInstance),
    $propDoctrine->getValue(),
    $propDoctrine->getValue($fooInstance)
);

/*
 * string(15) "staticPropValue"
 * string(15) "staticPropValue"
 * string(15) "staticPropValue"
 * NULL
 */

I believe this has been introduced in https://github.com/doctrine/persistence/commit/45c484fc8d70bf5aa4b4986a4f95210adec55878.

Current behavior

RuntimeReflectionProperty returns null for static properties.

Expected behavior

RuntimeReflectionProperty should return actual values for static properties.

How to reproduce

See linked PR.

[stof]

using a static property as a mapped property in a persisted entity does not make sense (as each hydrated entity would override the shared static property). What is the use case for that ?

[melkamar]

using a static property as a mapped property in a persisted entity does not make sense (as each hydrated entity would override the shared static property). What is the use case for that ?

Unfortunately all I can say is that our project git history commit says Fix issues with types. from a colleague who left years ago. The commit changed protected string $type; property to protected static string $type; and kept the MongoDB mapping definition.. 😅

I noticed this stopped working in production in an edge case of ours, didn't see any announced BC breaks in the doctrine libs so I went to search for the root cause.

I guess you are right though, having a static property mapped into the database doesn't really make sense. But if the mapping driver allows it, I suppose it should work? Especially since the RuntimeReflectionProperty extends the ReflectionProperty but changes the behaviour (albeit in an admittedly edge case there).

[stof]

But if the mapping driver allows it, I suppose it should work?

I don't think it ever work fine (load 2 documents from MongoDB and the override each other type in the PHP side, even if they have different values in MongoDB).

This is much more likely a non-sense mapping configuration that nobody among the Doctrine maintainers ever imagined, and so nobody added an explicit MappingException for that case.

I noticed this stopped working in production in an edge case of ours, didn't see any announced BC breaks in the doctrine libs so I went to search for the root cause.

Was it actually working in production before the upgrade of Doctrine ? Or was it silently corrupting data ?

Especially since the RuntimeReflectionProperty extends the ReflectionProperty but changes the behaviour (albeit in an admittedly edge case there).

This RuntimeReflectionProperty property is meant to solve some use cases of doctrine/persistence projects, not be a generic replacement. The fact that it extends ReflectionProperty is just to be able to add support for native lazy objects in PHP without doing major versions of doctrine/persistence, doctrine/orm and doctrine/mongodb-odm at the same time to change some deep API that 0.0001% of projects using Doctrine would use directly, by wrapping ReflectionProperty instead of using it. It is only meant to work for mapped persistence properties, and as said before, making such property static does not make any sense.

[melkamar]

I had a second look at the issue and I was wrong, I apologize for misleading:

There is no static mapped property.

The protected static string $type; property is not mapped. It simply exists in a class (that is mapped). The problem seems to be that

• the MongoDB ODM UnitOfWork::doMerge() seems to iterate over all properties (including static) of a class, regardless of whether they are mapped or not;
• in conjunction with the changes described here in the issue

We have fixed this by refactoring the static property into a static method.

---

I understand that the RuntimeReflectionProperty isn't meant to be a generic replacement, but currently it seems to be breaking an assumption that doctrine/mongodb-odm makes. Unless I misunderstood something else still. I don't know whether this should be addressed here or in the ODM, but probably the latter?

[stof]

I think this should be fixed in the ODM.