mongodb-odm icon indicating copy to clipboard operation
mongodb-odm copied to clipboard

@MappedSuperclass vs. abstract class

Open Jan0707 opened this issue 9 years ago • 6 comments

Given we have an abstract class which is declared a MappedSuperclass like this:

/**
 * @MongoDB\MappedSuperclass(repositoryClass="EventBundle\Repository\MongoDB\ReportRepository")
 * @MongoDB\DiscriminatorField(fieldName="type")
 * @MongoDB\DiscriminatorMap({
 *      "royalty_batch" = "TheaterBundle\Document\Event\Report\RoyaltyBatchReport",
 *      "royalty"       = "TheaterBundle\Document\Event\Report\RoyaltyReport",
 *      "tour"          = "TheaterBundle\Document\Event\Report\TourReport"
 * })
 * @Serializer\ExclusionPolicy("all")
 */
abstract class AbstractReport implements ReportInterface, ContextableInterface, TimestampableInterface

And that we have several child classes like:

/**
 * @MongoDB\Document(collection="reports")
 * @Serializer\ExclusionPolicy("all")
 */
class TourReport extends AbstractReport

In order to work with the child documents we get the repository (as a service) via the abstract class (because we should not have to declare a custom repository for each child class):

    event.repository.report:
        class: Doctrine\Common\Persistence\ObjectRepository
        factory_service: doctrine_mongodb
        factory_method: getRepository
        arguments:
            - 'EventBundle\Document\Report\AbstractReport'

In which case the $repository->findAll() yields no result.

The only fix I could come up with is to NOT declare the MappedSuperclass an abstract class and to declare the repository via the Document annotation :

/**
 * @MongoDB\Document(collection="reports", repositoryClass="EventBundle\Repository\MongoDB\ReportRepository")
 * @MongoDB\MappedSuperclass()
 * @MongoDB\DiscriminatorField(fieldName="type")
 * @MongoDB\DiscriminatorMap({
 *      "royalty_batch" = "TheaterBundle\Document\Event\Report\RoyaltyBatchReport",
 *      "royalty"       = "TheaterBundle\Document\Event\Report\RoyaltyReport",
 *      "tour"          = "TheaterBundle\Document\Event\Report\TourReport"
 * })
 * @Serializer\ExclusionPolicy("all")
 */
class AbstractReport implements ReportInterface, ContextableInterface, TimestampableInterface

I am slightly confused. Why can a MappedSuperclas NOT be an abstract class ? Or can it ?

Jan0707 avatar Mar 18 '15 16:03 Jan0707

@Jan0707 if you add @ODM\InheritanceType("SINGLE_COLLECTION") to your abstract class then everything works correctly, here's passing test case I've pulled together for you:

<?php

namespace Doctrine\ODM\MongoDB\Tests\Functional\Ticket;

use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;

class GH1050Test extends \Doctrine\ODM\MongoDB\Tests\BaseTest
{
    public function testRepositoryClassIsCorrect()
    {
        $r = $this->dm->getRepository(__NAMESPACE__ . '\\GH1050AbstractDocument');
        $this->assertInstanceOf(__NAMESPACE__ . '\\GH1050Repository', $r);
    }

    public function testRepositoryFindAll()
    {
        $d1 = new GH1050Document1();
        $d1->title = "a";
        $this->dm->persist($d1);
        $d2 = new GH1050Document2();
        $d2->title = "z";
        $this->dm->persist($d2);
        $this->dm->flush();
        $this->dm->clear();
        $r = $this->dm->getRepository(__NAMESPACE__ . '\\GH1050AbstractDocument');
        $results = $r->findAll();
        $this->assertCount(2, $results);
    }
}

/**
 * @ODM\MappedSuperclass(repositoryClass="GH1050Repository")
 * @ODM\InheritanceType("SINGLE_COLLECTION")
 * @ODM\DiscriminatorField(fieldName="type")
 * @ODM\DiscriminatorMap({
 *      "d1" = "GH1050Document1",
 *      "d2" = "GH1050Document2"
 * })
 */
abstract class GH1050AbstractDocument
{
    /** @ODM\Id */
    public $id;
}

/** @ODM\Document(collection="d1") */
class GH1050Document1 extends GH1050AbstractDocument
{
    /** @ODM\String */
    public $title;
}

/** @ODM\Document(collection="d1") */
class GH1050Document2 extends GH1050AbstractDocument
{
    /** @ODM\String */
    public $title;
}

class GH1050Repository extends \Doctrine\ODM\MongoDB\DocumentRepository
{

}

malarzm avatar Mar 19 '15 12:03 malarzm

After making the changes you proposed I end up with the following situation:

The repository can find all documents I create, but stores them in a collection that is named after the class of the AbstractDocument. This seems logical as the MappedSuperclass annotation does not include a collection property, so there is no apparent way of specifying a collection.

However there is a collection specified in the child classes as this is supported by the document annotation.

Which leads to the following dilemma:

When the repository tries to findAll() it comes up with a list of documents, however if I try to lead a specific document the operation fails, when trying:

at UnitOfWork ->tryGetById ('550bde4ee8e75f6d0c8b4567', object(ClassMetadata))

and throws an exception:

Class "EventBundle\Document\Report\AbstractReport" does not have an identifier 500 Internal Server Error - InvalidArgumentException

I assume this is because of the difference in collection naming between the abstract class and the child class ?

Jan0707 avatar Mar 20 '15 09:03 Jan0707

Sorry for long response time, I had rough week. I'll try to inspect the issue soon. For now I've found following pattern in my project:

/**
 * @MongoDB\MappedSuperclass
 */
abstract class ContentBase 
{  
    /**
     * @MongoDB\Id
     */
    private $id;

    // some other common stuff for all documents
}

/**
 * @MongoDB\Document(collection="images")
 * @MongoDB\InheritanceType("SINGLE_COLLECTION")
 * @MongoDB\DiscriminatorField(fieldName="image_type")
 * @MongoDB\DiscriminatorMap(...)
 */
class Image extends ContentBase 
{  
    // again some common stuff for classes extending this one
}

/**
 * @MongoDB\Document
 */
class ImageWithDescription extends Image {  }

As far as I remember I never use Image class directly, only when querying for all possible Images, everything is stored correctly in images collection and generally I don't have any problems. Maybe this will help you :)

malarzm avatar Mar 26 '15 10:03 malarzm

@malarzm We've had a similar issue in our software. We have multiple content types and wanted to have all functionality they share in a mapped superclass. Since it's also possible to query for all content, we wanted to have a generic ContentRepository instead of having more specific repositories that only query for a given content type. Designing classes, you always end up with the same structure:

  • An abstract Document\Content which defines all common properties. This should be a mapped superclass, but able to define inheritance, collection, common indexes, and so on. It also should be abstract since it's of no use by itself.
  • A number of Document\Content\Foo documents that extend the mapped superclass and are mapped as Document but should not need to define a bunch of settings, only add stuff (e.g. indexes)
  • A generic Repository\Content which is defined in the mapped superclass and is returned whenever fetching a document repository for Document\Content or Document\Content\Foo

It is possible to work around this by doing exactly what @Jan0707 suggested (Using the Document annotation and keeping the class abstract) but this kind of makes MapperSuperclass useless.

At the moment, this is not possible. I'd suggest extending the MappedSuperclass annotation to support all fields from the Document annotation and present them as defaults to inheriting classes, but this would change it significantly from the corresponding annotation in the ORM.

alcaeus avatar Apr 06 '15 06:04 alcaeus

Given that we're facing the same problem, I strongly support @alcaeus 's proposed solution

Jan0707 avatar Apr 06 '15 10:04 Jan0707

Any update?

dennisoehme avatar May 05 '15 11:05 dennisoehme