Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-115

Two ClassMetadata instances for an entity in global namespace

    Details

    • Type: Bug Bug
    • Status: 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. dcc115.diff
        3 kB
        Benjamin Eberlei
      2. GlobalNamespaceModel.php
        1 kB
        Benjamin Eberlei

        Activity

        Hide
        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
        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
        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
        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
        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
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: