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

[5.8] Prevent event cache from firing multiple times the same event(s) #28904

Merged
merged 1 commit into from Jun 21, 2019

Conversation

@jf-m
Copy link
Contributor

jf-m commented Jun 20, 2019

This is a proposed solution for solving #28872 .

Changes explanations:

EventCacheCommand

  • The discoverEvents are cached only when shouldDiscoverEvents is true on the related providers, otherwise only the listens are cached.
  • I've changed the structure of events.php so the the events/Listeners are grouped by EventServiceProvider.
[
  [Arrayof EventServiceProviders] => [Arrayof Events] => [Arrayof Listeners] 
]

instead of the old structure :

[
   [Arrayof Events] => [Arrayof Listeners] 
]
  • I've changed array_merge to array_merge_recursive because it still is technically possible that a basic listen event is also inside a discovered event.

EventServiceProvider

  • The boot methods now calls a getEvents to get the events of both discover and listen
  • If the events are cached, the getEvents will return ONLY the events located under the name of the current EventServiceProvider in the events.php cache.
    Indeed, the $this->listens() are already cached and they are all located inside the cache file, so there is no need to merge the cache file with listens;
    There is also no need to check for shouldDiscoverEvents because it was already done inside the event:cache command.

I hope this helps, cheers !

@jf-m jf-m force-pushed the jf-m:fix/28872 branch from ee58a17 to ac3c74e Jun 21, 2019
@taylorotwell taylorotwell merged commit 29339ed into laravel:5.8 Jun 21, 2019
2 checks passed
2 checks passed
continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.