Make EntityManager::__construct() public #7004

Closed
opened 2026-01-22 15:42:56 +01:00 by admin · 7 comments
Owner

Originally created by @nicolas-grekas on GitHub (Jul 13, 2022).

Feature Request

Q A
New Feature yes
RFC yes
BC Break no

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:

  • create empty EM1
  • on demand: trigger EM1 initialization
  • initialization calls the factory, which returns a populated EM2 (which contains UOW -> EM2)
  • copy all properties of EM2 into EM1 to initialize EM1 as needed

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 readonly even if EM doesn't use it for now.)

Originally created by @nicolas-grekas on GitHub (Jul 13, 2022). ### Feature Request | Q | A |------------ | ------ | New Feature | yes | RFC | yes | BC Break | no #### 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: - create empty EM1 - on demand: trigger EM1 initialization - initialization calls the factory, which returns a populated EM2 (which contains UOW -> EM2) - copy all properties of EM2 into EM1 to initialize EM1 as needed 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 `readonly` even if EM doesn't use it for now.)
admin closed this issue 2026-01-22 15:42:56 +01:00
Author
Owner

@greg0ire commented on GitHub (Jul 13, 2022):

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 readonly even if EM doesn't use it for now.)

How about using reflection again, this time to make the constructor public?

@greg0ire commented on GitHub (Jul 13, 2022): > 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 readonly even if EM doesn't use it for now.) How about using reflection again, this time to make the constructor public?
Author
Owner

@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.

@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.
Author
Owner

@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

@greg0ire commented on GitHub (Jul 13, 2022): The commit message is not going to help here : 7d48c785f6e93e5c0189f42f1947ca59451ebd54 :sweat_smile: @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
Author
Owner

@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.

@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.
Author
Owner

@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 EntityManagerDecorator instead?

@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 `EntityManagerDecorator` instead?
Author
Owner

@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.

@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.
Author
Owner

@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.

@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.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#7004