Review suggestions and approaches mentioned to override Doctrine mapping configuration [RFC]

mpdude commented 1 year ago

I am faced with the question whether (and if so, how) it is possible to override the Doctrine ORM mapping configuration for entities provided in a bundle. In the most simple case, this means being able to re-configure e. g. the table or column names being used for an entity.

The documentation on this topic is sparse, slightly contradictory and recommendations possibly incomplete.

I would like to discuss our options here, get some guidance and be reminded of parts of the puzzle I probably miss. Once we get to conclusions, I'd be happy to prepare PR(s) with suggested updates.

Information, pointers and ideas from Symfony Documentation and related PRs/issues

The "Best Practices for Reusable Bundles" (emphasis mine) says:

If the bundle includes Doctrine ORM entities and/or ODM documents, it's recommended to define their mapping using XML files stored in Resources/config/doctrine/. This allows to override that mapping using the standard Symfony mechanism to override bundle parts. This is not possible when using annotations/attributes to define the mapping.

Source: https://symfony.com/doc/current/bundles/best_practices.html#doctrine-entities-documents

Obviously, annotations/attributes live directly within the code and cannot easily be replaced from outside a bundle.

XML mapping configuration takes some extra effort (IMHO), but that would make sense if we get extra configuration flexibility from it.

But then, "the standard Symfony mechanism to override bundle parts" links to this:

Overriding entity mapping is only possible if a bundle provides a mapped superclass (such as the User entity in the FOSUserBundle). It's possible to override attributes and associations in this way. Learn more about this feature and its limitations in the Doctrine documentation.

Source: https://symfony.com/doc/current/bundles/override.html#entities-entity-mapping

That particular text was written in #14152. Before, that section was phrased as:

If a bundle defines its entity mapping in configuration files instead of annotations, you can override them as any other regular bundle configuration file. The only caveat is that you must override all those mapping configuration files and not just the ones you actually want to override. If a bundle provides a mapped superclass [...] you can override ...

This previous version was written in #10053, based on the request in #7076.

It is not exactly correct, at least I don't see where and how DoctrineBundle would honor the override mechanism and prefer config files found through it. In other words, you cannot use a mechanism as there is for templates to drop in the replacement configuration.

The suggestion in #7076, however, was that it is indeed possible to overwrite the doctrine.orm.mappings configuration for a particular bundle in its entirety to point to a different set of mapping configuration files. The OP in #7076 provides an example.

Using Compiler passes to set up mapping configuration

The reason why #14152 dismissed the suggestion from #7076 was

[...] works only because auto_mapping is set to true [...]. From the POV of the bundle , it's IMHO not a good practice to rely on app auto_mapping configuration.

https://github.com/symfony/symfony-docs/pull/14152#issue-689079428

It then goes on suggesting that bundles should use a dedicated compiler pass on their own (example given: https://github.com/FriendsOfSymfony/FOSUserBundle/blob/cf7fe27b2f4e1f298ee6eadf537267f8c9f9b85c/FOSUserBundle.php#L50) to set up their ORM mappings. This would make them completely independent of doctrine.orm.mappings settings.

Regarding the compiler pass, I was surprised to learn about it, since I have never seen it mentioned anywhere before. In fact, the RegisterMappingsPass base class is part of the Symfony Doctrine Bridge since v2.3 (https://github.com/symfony/symfony/commit/099fd9f52cbaafc3faa28fa6814caf8c9460a708).

This compiler pass will hook up an extra XML, annotations or other Doctrine mapping driver in the MappingDriverChain, after the mapping drivers configured by the doctrine.orm.mappings configuration. Since the MappingDriverChain works per namespace, such compiler passes (and compiler passes in general) win over configuration settings. This is pointed out correctly in https://github.com/symfony/symfony-docs/pull/14152#issue-689079428 as well; mapping configuration made by such a compiler pass cannot be overwritten by configuration, only by another compiler pass that comes afterwards.

I agree that bundles should not rely on auto_mapping being set. I do not, however, share the assessment that #7076 only worked due to auto_mapping being used.

I think it is possible for a bundle to explicitly register its mapping _as if auto_mapping were turned on_, but without resorting to compiler passes. The solution is "prepending configuration", as outlined in the next section.

Alternative: Prepending Configuration

As an alternative to using the compiler pass, a bundle might also use the PrependExtensionInterface and the mechanism described in https://symfony.com/doc/current/bundles/prepend_extension.html#prepending-extension-in-the-bundle-class to provide configuration for DoctrineBundle.

To give an example, a bundle could have the following in its DependencyInjection/Extension class:

class ... implements PrependExtensionInterface
{ ...
    public function prepend(ContainerBuilder $container)
    {
        $container->prependExtensionConfig('doctrine', [
            'orm' => [
                'mappings' => [
                    'MyBundleShortName' => [
                        'dir' => 'Resources/config/doctrine',
                    ],
                ],
            ],
        ]);
    }
}

This explicitly provides the same configuration that would be created automatically if auto_mapping were set to true. Mapping configuration will be loaded from XML files in Resources/config/doctrine relative to the bundle class.

The application can, however, still overwrite this configuration:

# config.yml
doctrine:
    orm:
        # auto_mapping: false <-- irrelevant
        mappings:
            MyBundleShortName:
                is_bundle: false # <-- do not treat `dir` as relative to the bundle
                type: xml
                dir: "%kernel.project_dir%/src/Resources/MyBundle/config/doctrine" # whatever seems fit
                prefix: 'MyBundle\Entity' # Namespace Prefix

The bottom line is that application configuration completely overwrites the default configuration provided by the bundle. There are no duplicate definitions competing in the MappingDriverChain; the configuration is merged before the mapping driver is configured.

Alternative: Intercepting Doctrine ORM events

You can intercept "metadata load" events in Doctrine ORM and use that to re-write/modify mapping configuration just in time.

Definetly not something I would recommend in the official documentation, and this puts metadata configuration in really unexpected places (even subscribers/listeners).

Just mentioned for completeness, since this is what search engine results brought up. Seems to work for some people.

Alternative: Mapped Superclasses

The docs mention "mapped superclasses" in case a bundle wants to support overriding attributes and associations. There is a reference to the limitations listed in the Doctrine documentation, but apart from that, it does not further elaborate that path.

In fact, this is very limited, since overrides can only be applied in entity classes that inherit from mapped superclasses, to re-configure parts of the mapping configuration inherited. It cannot be used together with entity inheritance hierachies.

Mapped superclasses mean that a bundle cannot work "out of the box", since it does not provide any ready-to-use entities. Users must completely subclass all those superclasses to create their own entities from it.

Additionally, the bundle needs to provide extra means to make the actual classes that shall be used configurable. For example, when code in the bundle wants to create a new User entity, but that class is a mapped superclass, which class should be instantiated instead? How do we know about the appropriate constructor?

Also, when associations are defined between entities or superclasses, extra configuration is necessary so that Doctrine can know about the actual classes to be used at runtime.

Subclassing either entity or mapped superclasses for the sake of overriding mapping configuration through annotations or attributes requires fields to be protected instead of private. In that case, it leads to code duplication and/or inheritance-based designs only to create places where mapping configuration can be overwritten.

For these reasons, I think mapped superclasses are not a general solution to the problem. We should not suggest using them, at least not as the recommended way to make the mapping more configurable.

Side issue: Explicit naming configuration vs. Doctrine naming strategy

A side question I'd like to see answered is whether we should recommend bundle authors to avoid using name="..." configurations for tables and columns (e. g. in @ORM\Column and @ORM\Table annotations).

Explicit configuration makes the database table and column names predictable, but might work against the particular Doctrine Naming Strategy chosen by the project/application. This may lead to inconsistent naming rules being used.

mpdude commented 1 year ago

I'd like to invite to this discussion

... @dbu who authored the compiler passes in the first place
... @psrpinto who suggested overwriting configuration in #7076
... @l-vo who pointed out in #14152 that bundles should work independent from application configuration setting.

carsonbot commented 9 months ago

Thank you for this issue. There has not been a lot of activity here for a while. Has this been resolved?

mpdude commented 9 months ago

To my knowledge no, Sir

dbu commented 6 months ago

stumbled over this again and read through your research. good research job!

its been quite a while with the compiler pass i contributed. that was part of the symfony CMF effort. we had that issue of bundles providing entities (or documents for phpcr-odm) and wanting those to be registered (or a more complicated one in RoutingBundle).

afaik that also led to the prepend configuration mechanism that you mentioned.

its been quite a while, I don't remember exactly why things ended up exactly like this. i have vague recollections that i believed you could overwrite the mapping xml like templates but that can either be a misunderstanding or the functionality was removed at some point.

today, i am also less convinced that it would be a good idea to allow too much tightly coupled overriding for entities, as it will be rather fragile. in the CMF we provided storage-agnostic model classes that are then extended by entity or document classes that also provide the mapping for fields of the base model (see symfony-cmf/content-bundle for an example). to allow for customization, i would today build the bundle to base on interfaces, provide a default implementation but allow to plug your custom entity as long as it implements the interface. the approach with a base agnostic model class would still make sense in such a scenario

symfony / symfony-docs