Explore injecting the event dispatcher in the database service

mondrake → changed the visibility of the branch 3492391-explore-injecting-the to hidden.

Why not module_handler?

You can make it a hook, they are now event listeners and give a lot of the same benefits of event subscribers, they are now classes and are autowired.

I also think there's less boilerplate needed among other benefits.

@nicxvan re #5 this is about the event dispatcher itself, not about the subscribers.

I think we should add a CR.

Added some docs inline.

Let’s see if we can make one more step and make the factory a decorator of the event dispatcher.

Added 2 CR stubs, one [#3494043] for the generic availability of the event dispatcher before container bootstrap, and one [#3494044] for its usage in Database::Connection. Will complete at later stage, doing at this stage has an high risk of getting stale information in it while reviews and tuning still needs to happen.

📌 Allow parsing and writing PHP constants and enums in YAML files Needs work would help, we could pass an enum parameter in the service definition file, which we cannot presently.

Maybe we should do 📌 Allow parsing and writing PHP constants and enums in YAML files Needs work first?

Thanks for review @daffie!

#22 would be nice, but I do not see it as a blocker.

Addressed the comments.

The CI pipeline is not happy. It fails for Drupal\KernelTests\Core\Database\DatabaseEventTest

Restored bot happiness. Back to NR.

The MR looks good to me.
The IS and the CRs need to be updated.

If you are trying to make this unified post bootstrap then why not have a utility class which you can call instead of the event dispatcher and does a fixed call before bootstrap and dispatches an event once the container is available?

@daffie thanks, if you do not mind I'd leave this to NR a bit longer to see if more reviews/input come. I will definitely do the CR and IS updates when we have settled on the final solution.

@nicxvan I am not sure I understand; feel free to open another issue branch and MR if you want to propose an alternative!

What I mean is you're jumping through all these hoops to get the invent dispatcher in place. But you're only calling a fixed output for that.

You should just replace this with a utility class that before bootstrap calls the static helper you're creating preBootstrapSubscribers to handle.
So the helper would call the static preBootstrapSubscribers if you are prebootstrap and then call the event dispatcher as it does now after.

You get the benefits you're looking for with way less code.

Please take a look at MR 10598

nicxvan → changed the visibility of the branch 3492391-early-fixed-event-dispatcher to hidden.

@nicxvan thanks, I like where MR 10607 is heading, especially in the part where the early subscribers to be registered with the early dispatcher are moved to the database API.

However, this is creating an (early) event dispatcher that is attached to the Connection class instead of being unique across the request. Not a big deal for now, right, but what if in the future more code would need to dispatch events early? Cache for instance may want to dispatch an event to be responded by a database listener (again, not the case know, but to prepare for possible future). I'd rather keep a singleton somewhere outside of the Connection class, so that we do not end up with vertical dispatchers not talking to each other. That's an architecture topic, tagging so we can have a review and guidance.

Then, I think we should use a closure also to register the early subscribers in place of the const array - so to allow passing instantiated objects that may require parameters (atm in both MRs the assumption is that the subscribers have no parameters in the construction).