Bug fix: Allow loading of metadata for Proxies #211

Closed
wants to merge 2 commits into from

4 participants

@sandulungu

Covers a few use cases, mainly when working with merged documents.
A similar problem (with possibly similar fix) has been reported in Doctrine ORM: http://www.doctrine-project.org/jira/browse/DDC-1441

Sandu Lungu Bug fix: Allow loading metadata for Proxies
Covers a few use cases after merging documents.
A similar problem (with possibly similar fix) has been reposted in Doctrine ORM: http://www.doctrine-project.org/jira/browse/DDC-1441
a8cc38c
@stof stof and 1 other commented on an outdated diff Dec 17, 2011
...Doctrine/ODM/MongoDB/Mapping/ClassMetadataFactory.php
@@ -219,6 +219,15 @@ private function loadMetadata($className)
continue;
}
+ // Proxy classes should use parent's metadata
+ if (strpos($className, 'Proxy')) {
@stof
Doctrine member
stof added a note Dec 17, 2011

why assuming that a proxy class contain Proxy in its name ? the good way to determine it is checking for the interface

Well, I dug in the source code and the "Proxy" is suffixed always by the standard ProxyFactory.
But this check can be removed, as later, reflection is used to check if the Proxy Interface is implemented.

@stof
Doctrine member
stof added a note Jan 27, 2012

if the code is updated to use the new code related to proxies in Doctrine Common, this will not be true anymore so you should remove this check on the class name

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jwage
Doctrine member

We can do this by checking the interface with instanceof right and get rid of the reflection all together?

@sandulungu

I tried, but it seems like "instanceof" does not work properly when the left parameter is a string classname.
Tested on PHP Version 5.3.5-1ubuntu7.4.
There is a discussion on this @ http://php.net/manual/en/language.operators.type.php

@jwage
Doctrine member

I'm still not sure about this change. We don't have anything like this in the ORM so I am not sure. Can we write a test for it so I can better understand what is happening?

@jwage
Doctrine member

Here is the similar change that was made to the ORM doctrine/doctrine2#332

I am open to this change if we can update it to match the ORM.

@jmikola
Doctrine member

FYI: doctrine/doctrine2#315 is the change to port over.

@malarzm
Doctrine member

This seems to be fixed in doctrine/common now so we don't have to care :)

@malarzm malarzm closed this Jan 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment