mirror of
https://github.com/doctrine/orm.git
synced 2026-04-29 09:23:20 +02:00
Make EntityManager::__construct() public #7004
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @nicolas-grekas on GitHub (Jul 13, 2022).
Feature Request
Summary
Right now, one is forced to call
EntityManager::create()in order to instantiate an entity manager.This makes the ORM hardly compatible with lazy ghost objects. The reason is that lazy ghost objects need to first be created with e.g.
ReflectionClass::newInstanceWithoutConstructor()and later on be populated on demand.Right now, the object graph contains e.g: EM -> UOW -> EM. This circular reference is defined in the constructor of EM.
Because of the factory, the current process when creating a lazy ghost object involves the following steps:
The resulting object graph is EM1 -> UOW -> EM2. It's broken of course.
If the constructor were made public, one could call the constructor in the initialization step and this concern would disappear.
The alternative right now would be to deep-search-n-replace EM2 by EM1 in EM1, but this would be super hacky code involving complex reflection/rebinding logic (and incompatible with
readonlyeven if EM doesn't use it for now.)@greg0ire commented on GitHub (Jul 13, 2022):
How about using reflection again, this time to make the constructor public?
@nicolas-grekas commented on GitHub (Jul 13, 2022):
The container doesn't know how to do such tricks yet the container needs access to the original instance. Adding a child class to DoctrineBundle would do it though and is likely what I'll suggest if this is rejected. But what would be the reasons to keep the constructor protected? The factory looks trivial logic that's barely needed to me, especially when something like a bundle is responsible for the wiring.
@greg0ire commented on GitHub (Jul 13, 2022):
The commit message is not going to help here :
7d48c785f6😅@beberlei maybe you remember why it is that way?
I also notice that there is a
/* final */comment that we need to uncomment on 3.0.x or challenge with @derrabus and @beberlei.Related: https://github.com/doctrine/orm/commit/acbafd081b13885bb65f7738682317cc27056c78
@beberlei commented on GitHub (Jul 14, 2022):
It makes sense we make the constructor public, the only thing the create method currently (still?) does is validate for the mapping driver. We could either put that into the getter of the configuration itself and make the return type non nullable, or move the check into the constructor.
@derrabus commented on GitHub (Jul 14, 2022):
Isn't the entity manager lazy by design already? Why would I want to create a ghost object from it? And even if I did, can't I create the ghost object from
EntityManagerDecoratorinstead?@nicolas-grekas commented on GitHub (Jul 14, 2022):
The entity manager is registered as a lazy service in DoctrineBundle so that it can be reset. In Symfony 6.2, we're going to use ghost objects instead of value holders. Ghost objects are superior in the sense that they provide a way to fix issues with referential identity, but only if initialized by constructor or when no circular refs are created inside the factory.
The decorator doesn't allow resetting the EM. Using it won't work.
@stof commented on GitHub (Aug 16, 2022):
@derrabus the alternative is to make the EntityManager resettable in doctrine/orm directly, without requiring to rely on a proxy-based hack.