Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot override the class metadata factory via configuration since 3.0 #11496

Open
alekitto opened this issue Jun 12, 2024 · 10 comments · May be fixed by #11497
Open

Cannot override the class metadata factory via configuration since 3.0 #11496

alekitto opened this issue Jun 12, 2024 · 10 comments · May be fixed by #11497

Comments

@alekitto
Copy link

Bug Report

Q A
BC Break no (bug introduced in major)
Version 3.0.0

Summary

Since version 3.0.0, the class metadata factory cannot be overridden.

Previous behavior

The entity manager has been instantiated taking the class name from the configuration on entity manager construction

Current behavior

Since ea97ea4 the override is broken as the entity manager is wired to the class Doctrine\ORM\Mapping\ClassMetadataFactory.
To allow overriding via configuration, it should be wired against Doctrine\Persistence\Mapping\ClassMetadataFactory interface.

How to reproduce

$conn = new Connection(...);

$config = new Configuration();
$config->setClassMetadataFactoryName(FakeMetadataFactory::class);

new EntityManager($conn, $config);

This is the same exact bug reported on mongodb-odm at doctrine/mongodb-odm#2551
The only difference is that on mongodb-odm this was considered a BC break, as the bug was introduced in a minor version.

@derrabus
Copy link
Member

Obviously not a bug. Please elaborate your use-case.

@alekitto
Copy link
Author

As explained also in the ODM issue:

It is mostly used for testing actually: I'm using a DocumentManager with a mocked connection and a custom metadata factory in order to precisely control what is returned from the factory itself

I used this feature in 2.x without any problem, but defining the concrete type into the EntityManager does not allow to use any other class implementing the common interface/extending the abstract class.

@derrabus
Copy link
Member

You did not answer the question.

@alekitto
Copy link
Author

You did not answer the question.

Yes, I did. My use-case is to create a fake/mock version of the class metadata factory implementing Doctrine\Persistence\Mapping\ClassMetadataFactory interface, and using it in testing to control what is returned from the factory itself and when the factory methods are called.

In the current state calling setClassMetadataFactoryName on configuration with such a class results in a fatal error on entity manager construction. But the same thing was possible in 2.x because the property type was not defined, so it was possible to implement the interface/extend the abstract class to have your custom implementation of the class metadata factory.

What else do you need to know precisely?

@derrabus
Copy link
Member

derrabus commented Jun 12, 2024

As far as I know, our factory is not final, so you may still mock it.

@alekitto
Copy link
Author

The class itself is not final, but some of the public methods are final in ClassMetadataFactory and in AbstractClassMetadataFactory and cannot be overridden (also what's the purpose of the abstract class if cannot be used in the entity manager?) .

I'm currently replacing the class completely, just implementing the interface.

@derrabus
Copy link
Member

I see. Well, we could introduce a new interface for our factory that you can mock. Please send a PR.

@alekitto
Copy link
Author

👍 Ok, thanks. I'll send it tomorrow.

@SenseException
Copy link
Member

I was wondering about the interface introduced in #11497. It seems that it contains the rest of the public methods that were added to the Doctrine\ORM\Mapping\ClassMetadataFactory as the "rest" that isn't provided by Doctrine Persistence. There is getOwningSide(), a setter for an EntityManager and a Cache setter for Psr\Cache\CacheItemPoolInterface instances. These seem to be different responsibilities for just putting then into one interface. Setting a Cache happens in Persistence while the EntityManager is in ORM. I wonder if those can be split up so that ORM doesn't get an interface where everything else is put into.

@alekitto
Copy link
Author

alekitto commented Aug 5, 2024

@SenseException I agree with your objections, but the same applies to the ClassMetadataFactory class.

The meaning of that interface is to create a minimum set of methods used by the ORM that needs to be implemented in order to override the metadata factory.

I don't know if this could be split easily; maybe the setters could be avoided passing them to the factory constructor, but in this case you can't just customise the factory class name in configuration, but you need to set into the configuration object at least a closure that creates the factory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants