Error on entity clone for MappedSuperclass fields #5755

Closed
betd-claumond opened this Issue Apr 1, 2016 · 3 comments

Projects

None yet

2 participants

@betd-claumond

Hello,

We get an error on cloning entity proxy following an upgrade from Doctrine 2.3 to 2.4 (but maybe relevant for 2.5 too).

The error is that fields declared as private on a MappedSuperClass are not cloned when a clone is done on a proxy entity of child class, only the protected fields of the MappedSuperClass are cloned.

This is a breaking change for us because it is only partially cloning entity and could have cause loss of data.

The cause is the usage of ClassMetadataInfo->getReflectionClass()->getProperties() rather than ClassMetadataInfo->reflFields in the cloning process.

I didn't find any existing issue or documentation about that, is it a known issue ?

Is there a clean solution existing to this issue please ?

Below the context information, the code analyse and the workaround solution.

Thanks in advance for your feedback on this issue.

Best regards,
Christophe

Context

uname -a
Linux 3.2.0-4-amd64 #1 SMP Debian 3.2.73-2+deb7u2 x86_64 GNU/Linux

php --version
PHP 5.4.45-0+deb7u2 (cli) (built: Oct 17 2015 08:26:31)
Copyright (c) 1997-2014 The PHP Group
Zend Engine v2.4.0, Copyright (c) 1998-2014 Zend Technologies

changes done in composer.json:

  • before:
"doctrine/orm": "2.3.5",
"doctrine/doctrine-bundle": "1.2.0",
"doctrine/common": "2.3.0",
"doctrine/dbal": "2.3.4",
"stof/doctrine-extensions-bundle": "1.1.0",
"gedmo/doctrine-extensions": "2.3.9",
"jms/metadata": "1.5.0",
  • now:
"doctrine/orm": "2.4.*",
"doctrine/doctrine-bundle": "1.6.*",
"doctrine/common": "2.4.*",
"doctrine/dbal": "2.4.*",
"doctrine/lexer": "1.0.*",
"doctrine/inflector": "1.1.*",
"doctrine/collections": "1.3.*",
"doctrine/cache": "1.5.*",
"doctrine/annotations": "1.2.*",
"doctrine/doctrine-cache-bundle": "1.3.*",
"behat/transliterator": "1.1.*",
"stof/doctrine-extensions-bundle": "1.2.*",
"gedmo/doctrine-extensions": "2.4.*",
"jms/metadata": "1.5.*",

Code analysis

in Doctrine 2.3, the clone method of a Proxy was simple

    public function __clone()
    {
        if (!$this->__isInitialized__ && $this->_entityPersister) {
            $this->__isInitialized__ = true;
            $class = $this->_entityPersister->getClassMetadata();
            $original = $this->_entityPersister->load($this->_identifier);
            if ($original === null) {
                throw new \Doctrine\ORM\EntityNotFoundException();
            }
            foreach ($class->reflFields as $field => $reflProperty) {
                $reflProperty->setValue($this, $reflProperty->getValue($original));
            }
            unset($this->_entityPersister, $this->_identifier);
        }

    }

===> Cloning use the public property ClassMetadataInfo->reflFields in order to process the copy of fields

in Doctrine 2.4, the clone method delegate the process to a callback method which comes that we can see in ProxyFactory.createCloner method

    function (BaseProxy $proxy) use ($entityPersister, $classMetadata) {
        if ($proxy->__isInitialized()) {
            return;
        }

        $proxy->__setInitialized(true);
        $proxy->__setInitializer(null);
        $class = $entityPersister->getClassMetadata();
        $original = $entityPersister->load($classMetadata->getIdentifierValues($proxy));

        if (null === $original) {
            throw new EntityNotFoundException();
        }

        foreach ($class->getReflectionClass()->getProperties() as $reflectionProperty) {
            $propertyName = $reflectionProperty->getName();

            if ($class->hasField($propertyName) || $class->hasAssociation($propertyName)) {
                $reflectionProperty->setAccessible(true);
                $reflectionProperty->setValue($proxy, $reflectionProperty->getValue($original));
            }
        }
    };

====> Cloning use ClassMetadataInfo->getReflectionClass()->getProperties() in order to process the copy of fields

In our use case, ClassMetadataInfo->reflFields was containing all the fields of the entity and its superclass, private+protected, but ClassMetadataInfo->getReflectionClass()->getProperties() returns only protected fields of the superclass.

So it's only copying partially the entity.

Workaround

Declare all the fields of the MappedSuperClass as protected, rather than private

@Ocramius
Member
Ocramius commented Apr 2, 2016

Can you check if ClassMetadataInfo#getReflectionProperties() can be used instead?

@betd-claumond

Hello,

Thanks for the feedback.

Yes, it's working properly for my use case with the code

foreach ($class->getReflectionProperties() as $reflectionProperty) {

Regards,
Christophe

@edhgoose edhgoose added a commit to edhgoose/doctrine2 that referenced this issue Apr 11, 2016
@edhgoose edhgoose Fixes #5755, uses '->getReflectionProperties()' instead of '->getRefl…
…ectionClass()->getProperties()' to ensure all fields are copied, and adds test to confirm behaviour
807ccaa
@Ocramius Ocramius added the Bug label Sep 10, 2016
@Ocramius Ocramius self-assigned this Sep 10, 2016
@Ocramius Ocramius added this to the 2.5.5 milestone Sep 10, 2016
@Ocramius Ocramius added a commit that closed this issue Sep 10, 2016
@edhgoose @Ocramius edhgoose + Ocramius Fixes #5755, uses '->getReflectionProperties()' instead of '->getRefl…
…ectionClass()->getProperties()' to ensure all fields are copied, and adds test to confirm behaviour
67e205b
@Ocramius Ocramius closed this in 67e205b Sep 10, 2016
@Ocramius Ocramius added a commit that referenced this issue Sep 10, 2016
@Ocramius Ocramius Merge branch 'fix/#5768-#5755-clone-proxy-private-properties-in-multi…
…-level-inheritances'

Close #5768
Close #5755
cd11723
@Ocramius Ocramius added a commit that referenced this issue Sep 10, 2016
@edhgoose @Ocramius edhgoose + Ocramius Fixes #5755, uses '->getReflectionProperties()' instead of '->getRefl…
…ectionClass()->getProperties()' to ensure all fields are copied, and adds test to confirm behaviour
d7026c4
@Ocramius Ocramius added a commit that referenced this issue Sep 10, 2016
@Ocramius Ocramius Merge branch 'fix/#5768-#5755-clone-proxy-private-properties-in-multi…
…-level-inheritances-2.5' into 2.5

Close #5768
Close #5755
73e4be7
@Ocramius
Member

Handled in #5768

@yvoyer yvoyer added a commit to yvoyer/doctrine2 that referenced this issue Nov 21, 2016
@edhgoose @yvoyer edhgoose + yvoyer Fixes #5755, uses '->getReflectionProperties()' instead of '->getRefl…
…ectionClass()->getProperties()' to ensure all fields are copied, and adds test to confirm behaviour
e3181ee
@yvoyer yvoyer added a commit to yvoyer/doctrine2 that referenced this issue Nov 21, 2016
@Ocramius @yvoyer Ocramius + yvoyer #5768 #5755 cleaned up test scenario, using new mocking logic as per …
…PHPUnit 5.4+
3a1ab4d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment