New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error on entity clone for MappedSuperclass fields #5755

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

Comments

Projects
None yet
2 participants
@betd-claumond
Copy link

betd-claumond commented Apr 1, 2016

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

This comment has been minimized.

Copy link
Member

Ocramius commented Apr 2, 2016

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

@betd-claumond

This comment has been minimized.

Copy link
Author

betd-claumond commented Apr 4, 2016

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 added a commit to edhgoose/doctrine2 that referenced this issue Apr 11, 2016

Fixes doctrine#5755, uses '->getReflectionProperties()' instead of '-…
…>getReflectionClass()->getProperties()' to ensure all fields are copied, and adds test to confirm behaviour

@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 closed this in 67e205b Sep 10, 2016

Ocramius added a commit that referenced this issue Sep 10, 2016

Ocramius added a commit that referenced this issue Sep 10, 2016

Fixes #5755, uses '->getReflectionProperties()' instead of '->getRefl…
…ectionClass()->getProperties()' to ensure all fields are copied, and adds test to confirm behaviour

Ocramius added a commit that referenced this issue Sep 10, 2016

Merge branch 'fix/#5768-#5755-clone-proxy-private-properties-in-multi…
…-level-inheritances-2.5' into 2.5

Close #5768
Close #5755
@Ocramius

This comment has been minimized.

Copy link
Member

Ocramius commented Sep 10, 2016

Handled in #5768

yvoyer added a commit to yvoyer/doctrine2 that referenced this issue Nov 21, 2016

Fixes doctrine#5755, uses '->getReflectionProperties()' instead of '-…
…>getReflectionClass()->getProperties()' to ensure all fields are copied, and adds test to confirm behaviour

yvoyer added a commit to yvoyer/doctrine2 that referenced this issue Nov 21, 2016

alexgurrola added a commit to Sitetheory-Archive/doctrine2 that referenced this issue Apr 13, 2017

Fixes doctrine#5755, uses '->getReflectionProperties()' instead of '-…
…>getReflectionClass()->getProperties()' to ensure all fields are copied, and adds test to confirm behaviour
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment