Doctrine MongoDB ODM
  1. Doctrine MongoDB ODM
  2. MODM-20

SINGLE_COLLECTION inheritance does not work

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0.0ALPHA2
    • Fix Version/s: 1.0.0BETA2
    • Component/s: Hydration
    • Labels:
      None
    • Environment:
      PHP 5.3.1, Doctrine ODM MongoDB 2010-06-22 16:07:10

      Description

      It seems that SINGLE_COLLECTION inheritance is not implemented?

      Method UnitOfWork::getOrCreateDocument calls Mapping\Metadata::newInstance without taking possible discriminator map into account.

      The example in the official documentation (http://www.doctrine-project.org/projects/mongodb_odm/1.0/docs/reference/inheritance-mapping/en) is not a real test: the object is first created as a child-class instance before being queried through the parent class, so the object is returned without hydration, from the UnitOfWork identityMap cache.

      The corresponding code from Doctrine\ORM is in Doctrine\ORM\Internal\Hydration\ObjectHydrator::_getEntity (find class name through discriminator map, if any).

        Activity

        Hide
        Jonathan H. Wage added a comment -

        It is just not implemented/finished yet. I will fix the code you mention soon. Thanks, Jon

        Show
        Jonathan H. Wage added a comment - It is just not implemented/finished yet. I will fix the code you mention soon. Thanks, Jon
        Hide
        Okapi added a comment -

        Hi Jon,

        Indeed, hydration seems aware of the discriminator map now (I only use Single Collection inheritance).

        But the discriminator map seems ignored when a document is fetched through its ID (eg. DM::find($class, $id) ).

        Because of that, I just can't use this inheritance model. I can't see any possible workaround, since the discriminator field is no longer allowed to be declared in the class.

        Other point which is not a bug but a suggestion : instead of using a static discrminator map, couldn't we register an inflector class? Indeed, in most use cases, the document class name is the mere concatenation of the base class with a backslash followed by the camel-cased discriminator value.

        Thanks,

        Okapi

        Show
        Okapi added a comment - Hi Jon, Indeed, hydration seems aware of the discriminator map now (I only use Single Collection inheritance). But the discriminator map seems ignored when a document is fetched through its ID (eg. DM::find($class, $id) ). Because of that, I just can't use this inheritance model. I can't see any possible workaround, since the discriminator field is no longer allowed to be declared in the class. Other point which is not a bug but a suggestion : instead of using a static discrminator map, couldn't we register an inflector class? Indeed, in most use cases, the document class name is the mere concatenation of the base class with a backslash followed by the camel-cased discriminator value. Thanks, Okapi
        Hide
        Jonathan H. Wage added a comment -

        Hi, the discriminator map should be used in both DocumentManager::find() and findOne()

        http://github.com/doctrine/mongodb-odm/blob/master/lib/Doctrine/ODM/MongoDB/MongoCollection.php#L285
        http://github.com/doctrine/mongodb-odm/blob/master/lib/Doctrine/ODM/MongoDB/MongoCollection.php#L315

        As far as the inflector class, we have to specify the full discriminator map on the parent class because otherwise we would have to load all the mapping information for all the subclasses to know the information.

        Can you give some kind of test code or a test case or something for your reason to re-open?

        Show
        Jonathan H. Wage added a comment - Hi, the discriminator map should be used in both DocumentManager::find() and findOne() http://github.com/doctrine/mongodb-odm/blob/master/lib/Doctrine/ODM/MongoDB/MongoCollection.php#L285 http://github.com/doctrine/mongodb-odm/blob/master/lib/Doctrine/ODM/MongoDB/MongoCollection.php#L315 As far as the inflector class, we have to specify the full discriminator map on the parent class because otherwise we would have to load all the mapping information for all the subclasses to know the information. Can you give some kind of test code or a test case or something for your reason to re-open?
        Hide
        Okapi added a comment - - edited

        Hi, indeed. I've had difficulties in running the PHPUnit tests, but have struggled on the issue within my app long enough to have a clear scenario.

        Consider the following unit test:
        Doctrine\ODM\MongoDB\Tests\Functional\InheritanceTest::testSingleCollectionInhertiance()

        Data has been inserted according to the code before $this->dm->clear();

        Now, in a new PHP instance (web req or CLI proc) (to avoid taking any risk related to the reliability of DocumentManager::clear() and make sure objects are freshly hydrated ),

        The follow assertion should be OK (I would expect it to be in the test, as it represents the purpose of the discriminator map):
        $document = $this->dm->findOne('Documents\Project', array('name' => 'Sub Project'));
        $this->assertInstanceOf('Documents\SubProject', $document);

        Now, suppose $id is the identifier value of $document as returned by the former code.
        The following assertion should be also true:
        $document = $this->dm->find('Documents\Project', $id);
        $this->assertInstanceOf('Documents\SubProject', $document);

        This former test would fail in my case (doc class is Documents\Project as queried).

        To me, the purpose of single inheritance is when we don't know by advance the class of the object that hydration will yield. The query is made on a common document class, and the class of each returned document will depend on the discriminator field.
        In a typical application, these classes would implement a common interface. Having the possibility to isolate the implementation (concrete class) from the query is a key feature to benefit from the schema-less character of a NoSQL DB.

        Am I right, or missing something?

        Show
        Okapi added a comment - - edited Hi, indeed. I've had difficulties in running the PHPUnit tests, but have struggled on the issue within my app long enough to have a clear scenario. Consider the following unit test: Doctrine\ODM\MongoDB\Tests\Functional\InheritanceTest::testSingleCollectionInhertiance() Data has been inserted according to the code before $this->dm->clear(); Now, in a new PHP instance (web req or CLI proc) (to avoid taking any risk related to the reliability of DocumentManager::clear() and make sure objects are freshly hydrated ), The follow assertion should be OK (I would expect it to be in the test, as it represents the purpose of the discriminator map): $document = $this->dm->findOne('Documents\Project', array('name' => 'Sub Project')); $this->assertInstanceOf('Documents\SubProject', $document); Now, suppose $id is the identifier value of $document as returned by the former code. The following assertion should be also true: $document = $this->dm->find('Documents\Project', $id); $this->assertInstanceOf('Documents\SubProject', $document); This former test would fail in my case (doc class is Documents\Project as queried). To me, the purpose of single inheritance is when we don't know by advance the class of the object that hydration will yield. The query is made on a common document class, and the class of each returned document will depend on the discriminator field. In a typical application, these classes would implement a common interface. Having the possibility to isolate the implementation (concrete class) from the query is a key feature to benefit from the schema-less character of a NoSQL DB. Am I right, or missing something?
        Hide
        Jonathan H. Wage added a comment -

        That is correct, this is how it is intended to work and as far as I've tested it is working that way. I even added your code to the test and it passes: http://github.com/doctrine/mongodb-odm/commit/54f68d9a4623ec7daa18bf5671ebba778bbc0df8

        Show
        Jonathan H. Wage added a comment - That is correct, this is how it is intended to work and as far as I've tested it is working that way. I even added your code to the test and it passes: http://github.com/doctrine/mongodb-odm/commit/54f68d9a4623ec7daa18bf5671ebba778bbc0df8
        Hide
        Okapi added a comment -

        Hello, after hours of seeking the cause why Doctrine tests pass and mines don't although very similar to each other!!

        Answer is: in my project, the base document class which declares the single inheritance behaviour does not include itself in the discriminator map.

        Indeed, in the unit tests, Documents\Project is used for both a collection (to query from) and an implementation (of documents with type "project").

        In my project, the "collection document class" (setting up the inheritance) is abstract. For now, to please Doctrine ODM, I have included it in the discriminator map under the dumb key "abstract". Doctrine seems fine with it, but there is a logical problem, in the sense that it may try innocently to instanciate my abstract class if it fetches from MongoDB a document with the corresponding type.

        What do you think?

        Thinking of the case where a fetched document does not match an item in the discriminator map, does the hydrator fall back to the main document class?
        Should we have an annotation like @DiscriminatorFallbackValue or @DiscriminatorFallbackDocument ?

        If I have suggestions / thinking about how inheritance can be better used, where do you want me to write them? Here or on the forum?

        Show
        Okapi added a comment - Hello, after hours of seeking the cause why Doctrine tests pass and mines don't although very similar to each other!! Answer is: in my project, the base document class which declares the single inheritance behaviour does not include itself in the discriminator map. Indeed, in the unit tests, Documents\Project is used for both a collection (to query from) and an implementation (of documents with type "project"). In my project, the "collection document class" (setting up the inheritance) is abstract. For now, to please Doctrine ODM, I have included it in the discriminator map under the dumb key "abstract". Doctrine seems fine with it, but there is a logical problem, in the sense that it may try innocently to instanciate my abstract class if it fetches from MongoDB a document with the corresponding type. What do you think? Thinking of the case where a fetched document does not match an item in the discriminator map, does the hydrator fall back to the main document class? Should we have an annotation like @DiscriminatorFallbackValue or @DiscriminatorFallbackDocument ? If I have suggestions / thinking about how inheritance can be better used, where do you want me to write them? Here or on the forum?
        Hide
        Jonathan H. Wage added a comment -

        Can you make a test then with how your entities are setup so I can work on a patch?

        Show
        Jonathan H. Wage added a comment - Can you make a test then with how your entities are setup so I can work on a patch?
        Hide
        Okapi added a comment -

        Sorry, I don't have a Github account, and am leaving for a week-long trip, so I don't have th etime yet to make a proper contribution.

        However, I have modified tests' Documents\Project, added Documents\OtherSubProject and updated Doctrine\ODM\MongoDB\Tests\Functional\InheritanceTest. Here is a tar.gz of these 3 files.

        The instanceOf assertions fails, as the hydrator always returns Documents\Project objects, whatever the "type" is.
        Note that adding "project"="Documents\Project" to the discriminator map makes the test pass again.

        Show
        Okapi added a comment - Sorry, I don't have a Github account, and am leaving for a week-long trip, so I don't have th etime yet to make a proper contribution. However, I have modified tests' Documents\Project, added Documents\OtherSubProject and updated Doctrine\ODM\MongoDB\Tests\Functional\InheritanceTest. Here is a tar.gz of these 3 files. The instanceOf assertions fails, as the hydrator always returns Documents\Project objects, whatever the "type" is. Note that adding "project"="Documents\Project" to the discriminator map makes the test pass again.
        Hide
        Jonathan H. Wage added a comment -

        I'm pretty certain this is fixed now. I ran your test and it is passing now. I merged the changes and committed it.

        Show
        Jonathan H. Wage added a comment - I'm pretty certain this is fixed now. I ran your test and it is passing now. I merged the changes and committed it.

          People

          • Assignee:
            Jonathan H. Wage
            Reporter:
            Okapi
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: