8000 Always load Doctrine listeners lazily · Issue #27661 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Always load Doctrine listeners lazily #27661

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.

Sign up for GitHub

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

Jump to bottom
Closed
weaverryan opened this issue Jun 20, 2018 · 2 comments
Closed

Always load Doctrine listeners lazily #27661

weaverryan opened this issue Jun 20, 2018 · 2 comments
Labels
DoctrineBridge Feature

Comments

@weaverryan
Copy link
Member

Description
Currently, Doctrine event listeners are instantiated in order to create the Doctrine event manager. You can fix this by adding a lazy=true option to your service (this feature was added 6+ years ago). Could we just always make this lazy? This would solve issues during cache building, where all of the listeners are instantiated.

Relevant code:

symfony/src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php

Lines 97 to 99 in 8123bb1

if ($lazy = !empty($tag['lazy'])) {
$taggedListenerDef->setPublic(true);
}

Especially if we refactored the ContainerAwareEventManager to accept a service locator (instead of the entire container), this would be an easy win: we wouldn't even need to make the original definitions public anymore (which is the only downside).
@cryptiklemur cryptiklemur mentioned this issue Jun 20, 2018
Closed
@dmaicher dmaicher mentioned this issue Jun 21, 2018
Merged
3 tasks
@dmaicher
Copy link
Contributor

Makes sense 👍 See #27675

symfony-splitter pushed a commit to symfony/doctrine-bridge that referenced this issue Jun 28, 2018
@fabpot
feature #27675 [DoctrineBridge] always load event listeners lazy via …
d742c6f 
…ServiceLocator (dmaicher)

This PR was squashed before being merged into the 4.2-dev branch (closes #27675).

Discussion
----------

[DoctrineBridge] always load event listeners lazy via ServiceLocator

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes/no
| Tests pass?   | yes
| Fixed tickets | symfony/symfony#27661
| License       | MIT
| Doc PR        | symfony/symfony-docs#9973

As described in symfony/symfony#27661 this PR suggests to always load doctrine event listeners lazily from a service locator instead of the full service container.

If we agree to move forward I could tackle the remaining todos:

- [x] update UPGRADE.md
- [x] documentation PR
- [x] tested on real app

Commits
-------

130ec0525d [DoctrineBridge] always load event listeners lazy via ServiceLocator
fabpot added a commit that referenced this issue Jun 28, 2018
@fabpot
feature #27675 [DoctrineBridge] always load event listeners lazy via …
83232f8 
…ServiceLocator (dmaicher)

This PR was squashed before being merged into the 4.2-dev branch (closes #27675).

Discussion
----------

[DoctrineBridge] always load event listeners lazy via ServiceLocator

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes/no
| Tests pass?   | yes
| Fixed tickets | #27661
| License       | MIT
| Doc PR        | symfony/symfony-docs#9973

As described in #27661 this PR suggests to always load doctrine event listeners lazily from a service locator instead of the full service container.

If we agree to move forward I could tackle the remaining todos:

- [x] update UPGRADE.md
- [x] documentation PR
- [x] tested on real app

Commits
-------

130ec05 [DoctrineBridge] always load event listeners lazy via ServiceLocator
@fabpot fabpot closed this as completed Jun 28, 2018
@HKandulla
Copy link
HKandulla commented Jan 14, 2019 •
edited
Loading

Hi,

we just ran into some major problems upgrading to DoctrineBridge 4.2. due to this change.

Removing EventListeners by using $this->em->getEventManager()->removeEventListener([$event], $listener); and passing the listener-object as the second argument will not remove the listener anymore (and not throw an exception/error).
After upgrading to DoctrineBridge 4.2. you need to pass the FQCN of the listener.
Maybe this should be mentioned?

Best, Hannes

@malarzm malarzm mentioned this issue Apr 10, 2019
Closed
@malarzm malarzm mentioned this issue May 6, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
DoctrineBridge Feature
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@fabpot @weaverryan @HKandulla @dmaicher @xabbuh
0