Change serialization of inheritance fields #1556
Conversation
alcaeus
force-pushed
the
serialize-inheritance-fields-embedded-docs
branch
from
508b3ea
to
20da3ee
Compare
alcaeus
changed the title
alcaeus
force-pushed
the
serialize-inheritance-fields-embedded-docs
branch
from
20da3ee
to
7ad4804
Compare
|
Is there any reason not to merge this? |
| @@ -123,7 +123,7 @@ public function __sleep() | |||
| $serialized[] = 'customRepositoryClassName'; | |||
| } | |||
|
|
|||
| if ($this->inheritanceType != self::INHERITANCE_TYPE_NONE) { | |||
| if ($this->inheritanceType != self::INHERITANCE_TYPE_NONE || $this->discriminatorField !== null) { | |||
There was a problem hiding this comment.
wouldn't it be better to check for being embedded document's metadata? Otherwise the issue still persists when not using discriminatorField which I think can be null to default to _doctrine_class_name later down the line? Please take this with a grain of salt though, I'm always confused by class metadata's defaults
There was a problem hiding this comment.
Ah, didn't think about _doctrine_class_name - especially since that doesn't involve a map either. Will have to investigate.
There was a problem hiding this comment.
Actually I think that map can be combined with default discriminator field since it only provides string <=> FQCN mapping ;)
There was a problem hiding this comment.
After looking at the code, this looks safe: _doctrine_class_name is not used at the class level, which this PR is trying to fix. Instead it's used when an association contains no targetDocument and no discriminatorField has been set - in this case the discriminatorField is simply set to _doctrine_class_name as if it were used in the mapping. I think we should be safe merging this.
Fixes #1271.
The previous solution would always require specifying an inheritance type, which makes sense for documents but seems weird for embedded documents. With this PR, this changes to serialize the inheritance information (inheritance type and discriminator data) if an inheritance type was specified or if the discriminator field was given. This should fix hard to find cases where data can't be stored or loaded once the metadata was loaded from cache.