Doctrine 2 - ORM

Two ClassMetadata instances for an entity in global namespace

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 2.0-ALPHA2
  • Fix Version/s: None
  • Component/s: ORM
  • Security Level: All
  • Labels:
    None

Description

The global namespace creates problems for entities that are retrieved via getClassMetadata($className) when $className is not prepended by "\". This is only a problem in the global namespace, because in other instances a slash might be present and the metadata factory does not find the relevant previously generated ClassMetadata instance.

This problem specifically occours with Annotations Mapping, since ReflectionClass::getName() returns the class name unqualified.

php -r 'namespace Foo { $r = new \ReflectionClass("\stdClass"); echo $r->getName();} '

I attached a Test-Case that proves this behaviour. It requires the additional attached Global namespace model data.

  1. File
    dcc115.diff
    04/Nov/09 7:28 PM
    3 kB
    Benjamin Eberlei
  2. File
    GlobalNamespaceModel.php
    04/Nov/09 7:28 PM
    1 kB
    Benjamin Eberlei

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Benjamin Eberlei added a comment - 
[20:20:55] <beberlei> guilhermeblanco: the problem is that ReflectionClass->getName() returns the name without prepended namespace operator for global classes
[20:27:43] <guilhermeblanco> beberlei: so it's a bug in ReflectionClass
[20:27:51] <guilhermeblanco> can't you just do get_class($instance)?
[20:28:11] <beberlei> no its the annotations mapping
[20:28:22] <beberlei> its using ReflectionClass::getName()
[20:29:13] <guilhermeblanco> hm...
[20:29:22] <guilhermeblanco> give me a second... I wanna test something
[20:31:33] <beberlei> i all attach test and classes to a bug
[20:32:43] <guilhermeblanco> weird
[20:32:54] <guilhermeblanco> beberlei: let me show you something
[20:33:37] <beberlei> http://www.doctrine-project.org/jira/browse/DDC-115
[20:33:50] <beberlei> the problem is ReflectionClass->getName() has to be backwards compatible
[20:34:19] <guilhermeblanco> beberlei: http://pastie.org/683660
[20:34:54] <beberlei> oh
[20:34:57] <beberlei> they all strip it?
[20:35:04] <beberlei> we need a normalize method
[20:35:41] <guilhermeblanco> let me go further
[20:35:47] <guilhermeblanco> I wanna check if it's not a PHP bug
[20:37:53] <guilhermeblanco> beberlei: http://pastie.org/683669
[20:48:22] <beberlei> its not a bug yeah, but then we need to extend ClassMetadataFactory to prepend each $className which has none with the namespace separator
[20:48:34] <beberlei> hasMetadata getMetadata setMetadata
Show
Benjamin Eberlei added a comment - [20:20:55] <beberlei> guilhermeblanco: the problem is that ReflectionClass->getName() returns the name without prepended namespace operator for global classes [20:27:43] <guilhermeblanco> beberlei: so it's a bug in ReflectionClass [20:27:51] <guilhermeblanco> can't you just do get_class($instance)? [20:28:11] <beberlei> no its the annotations mapping [20:28:22] <beberlei> its using ReflectionClass::getName() [20:29:13] <guilhermeblanco> hm... [20:29:22] <guilhermeblanco> give me a second... I wanna test something [20:31:33] <beberlei> i all attach test and classes to a bug [20:32:43] <guilhermeblanco> weird [20:32:54] <guilhermeblanco> beberlei: let me show you something [20:33:37] <beberlei> http: //www.doctrine-project.org/jira/browse/DDC-115 [20:33:50] <beberlei> the problem is ReflectionClass->getName() has to be backwards compatible [20:34:19] <guilhermeblanco> beberlei: http: //pastie.org/683660 [20:34:54] <beberlei> oh [20:34:57] <beberlei> they all strip it? [20:35:04] <beberlei> we need a normalize method [20:35:41] <guilhermeblanco> let me go further [20:35:47] <guilhermeblanco> I wanna check if it's not a PHP bug [20:37:53] <guilhermeblanco> beberlei: http: //pastie.org/683669 [20:48:22] <beberlei> its not a bug yeah, but then we need to extend ClassMetadataFactory to prepend each $className which has none with the namespace separator [20:48:34] <beberlei> hasMetadata getMetadata setMetadata
Hide
Permalink
Benjamin Eberlei added a comment -

Committing a patch that adds the following snippet:

if($className[0] == "\\") {
    $className = substr($className, 1);
}

to each of the following methods of ClassMetadataFactory: 

hasMetdataFor($className)
getMetadataFor($className)
setMetadataFor($className, $class)
Show
Benjamin Eberlei added a comment - Committing a patch that adds the following snippet: if ($className[0] == "\\" ) { $className = substr($className, 1); } to each of the following methods of ClassMetadataFactory: hasMetdataFor($className) getMetadataFor($className) setMetadataFor($className, $class)
Hide
Permalink
Benjamin Eberlei added a comment -

Applied to trunk, Can someone verify this is ok?

Show
Benjamin Eberlei added a comment - Applied to trunk, Can someone verify this is ok?
Hide
Permalink
Roman S. Borschel added a comment -

Not sure I understand this. As I said in another issue class names in strings are always considered fully qualified by php. A leading backslash in class names in strings should never be used it is completely unnecessary.

The patch is problematic because the methods they're applied to are one of the most-called methods and they were already "slow enough" previously.

If there is a problem and the problem is only with models in the global namespace then this should not be fixed. User-defined classes dont belong in the global namespace, ever.

Show
Roman S. Borschel added a comment - Not sure I understand this. As I said in another issue class names in strings are always considered fully qualified by php. A leading backslash in class names in strings should never be used it is completely unnecessary. The patch is problematic because the methods they're applied to are one of the most-called methods and they were already "slow enough" previously. If there is a problem and the problem is only with models in the global namespace then this should not be fixed. User-defined classes dont belong in the global namespace, ever.
Hide
Permalink
Benjamin Eberlei added a comment -

We found the true issue.

Several methods in ClassMetadataInfo assumed when no slash was prepended that a shortcut is used and prepends its own namespace plus a NS. This caused the problem which is now fixed by additionally checking for strlen($this->namespace) on this positions.

Show
Benjamin Eberlei added a comment - We found the true issue. Several methods in ClassMetadataInfo assumed when no slash was prepended that a shortcut is used and prepends its own namespace plus a NS. This caused the problem which is now fixed by additionally checking for strlen($this->namespace) on this positions.

People

  • Assignee:
    Benjamin Eberlei
    Reporter:
    Benjamin Eberlei
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: