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

Undocumented doctrine.orm.enable_lazy_ghost_objects #1651

Closed
mvhirsch opened this issue May 3, 2023 · 13 comments · Fixed by #1733
Closed

Undocumented doctrine.orm.enable_lazy_ghost_objects #1651

mvhirsch opened this issue May 3, 2023 · 13 comments · Fixed by #1733

Comments

@mvhirsch
Copy link

mvhirsch commented May 3, 2023

I'm currently upgrading a big project's flex recipes and one of the dependencies I just upgraded was this one.

Now I'm facing doctrine.orm.enable_lazy_ghost_objects (see #1568), and wanted to research about possible implications on my project. Sadly, it's undocumented. If you're going to ask, yes on both #1605.

I'm pretty sure this is safe in my project, but I'm wondering: could we please documented what this flag is doing? I'm pretty sure what it's purpose is, but I might not be the only (experienced Symfony) developer ;-)

@SenseException
Copy link
Member

It's a relatively new feature depending on Symfony >=6.2 and ORM >=2.14 and the ORM docs are also missing this feature. Do you like to add the missing docs in a pull request?

@stof
Copy link
Member

stof commented May 4, 2023

This is an opt-in for the new way to manage proxies in the ORM (which requires symfony/var-exporter 6.2+).

As the code should be working fine now, I see no reason to set it to false when you are on Symfony 6.2+, especially in new projects. Set it to false as a workaround for issues you might face when upgrading (so for an upgrade, try setting it to true and if you don't have bugs, leave it like that, otherwise report the issue and switch it back to false until the issue is fixed).

@SerethiX
Copy link

Sadly it's not safe to keep it set to false when using readonly public properties in entities: #9432

@nicolas-grekas
Copy link
Member

Enabling this setting is the future. Not setting it to true is going to be deprecated soon.

@ostrolucky
Copy link
Member

Then it should have been documented in UPGRADE.md doc

@gnito-org
Copy link

gnito-org commented Jul 14, 2023

Enabling this setting is the future. Not setting it to true is going to be deprecated soon.

Whoa... not so fast. I constantly run into issues where a ManyToOne relation entity on the One side is not fully hydrated when this option is set to true. To be safe I have to always set it to false.

Short example:

You have an entity Foo that has a ManyToOne relation with entity Bar:

    #[ORM\ManyToOne(targetEntity: Bar::class)]
    #[ORM\JoinColumn(nullable: false)]
    private ?Bar $bar = null;

Entity Bar has a Doctrine listener that auto populates some non-persistent fields on PostLoad().

Entity Foo database record is auto read into a Controller change method (/change/{id}).

If you access the non-persistent field(s) in the Controller method with $foo->getBar()->getField(), they are not populated, which means that the Doctrine listener did not fire.

The only time it fires with the flag set to true is when you specify fetch: "EAGER" on the relation.

If you set doctrine.orm.enable_lazy_ghost_objects=false the listener fires and everything works as expected regardless of what relation fetch option you specify.

@stof
Copy link
Member

stof commented Aug 18, 2023

@gnito-org can you share a reproducer for that ?

@gnito-org
Copy link

Further investigation revealed that my issue is not related to the enable_lazy_ghost_objects setting.

The problem occurs when you access a virtual property of a LAZY or EXTRA_LAZY loaded entity and you do not also access a persisted property of the entity. In this case the doctrine postLoad listener does not fire. The listener does fire in this scenario if it is an EAGER loaded entity.

@gnito-org
Copy link

I published the replicator that I was using to debug the issue here:

https://github.com/gnito-org/issue-1651-replicator

@stof
Copy link
Member

stof commented Aug 18, 2023

Well, the listener fires when the persistent properties are loaded (as that exactly what postLoad is about).

With EAGER as strategy, Doctrine forces the loading of entities when they are registered in the unit of work, so the loading happen. With any lazy strategy, the loading is delayed.
The difference is that the way the loading is done:

  • the old proxy system was hooking into public methods: any call to a public method (other than a special case for getId()) would trigger the proxy initialization before calling the original implementation of the method
  • lazy ghost objects are hooking into access to the properties themselves. This has several benefits:
    • it supports using public properties in your entities
    • it avoids having to do special cases for getId() by detecting that it does not use other properties than the id (which is already known by the proxy object)
    • if some methods don't use the persistent properties, they won't need to initialize the proxy
    • if some methods are final, the proxy can still be generated in a working way
    • the implementation does not require overriding methods, which avoids incompatibility with tools looking at attributes on methods (which would not work well with proxies that override the method without copying all attributes)
    • the implementation does not require overriding methods, which avoids incompatibility with new PHP features related to method signatures (new kind of parameter types like what happened with the DNF types RFC, new kind of default values allowing expressions, etc...) making it easier to support new PHP versions

The drawback of the new approach is indeed that code that does not read any persistent property can indeed not rely anymore on the fact that the entity is loaded (and so that postLoad was fired)

@gnito-org
Copy link

Thank you for the detail explanation.

I will just use an eager fetch. Problem solved.

@craigh
Copy link

craigh commented Dec 1, 2023

So, still no documentation?

I'm currently on Symfony 5.4. I see a deprecation notice that I need to set enable_lazy_ghost_objects: true. I give it a try and everything breaks. I investigate and can't even figure out what this does.

@stof
Copy link
Member

stof commented Dec 4, 2023

@craigh please open a dedicated issue with the error message you get. This issue is about lacking the documentation, not about things being broken (and so it would be closed once the PR adding the documentation gets merged).

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.

8 participants