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

Remove multi-application registration #38

Closed
lcobucci opened this issue May 6, 2020 · 4 comments · Fixed by #43
Closed

Remove multi-application registration #38

lcobucci opened this issue May 6, 2020 · 4 comments · Fixed by #43
Labels
Improvement Improvement of existing feature
Milestone

Comments

@lcobucci
Copy link
Member

lcobucci commented May 6, 2020

We now support the configuration of different apps within a single DI container. That creates a lot of complexity for no good reason.

By removing this, we'd be able to define "static" services for routers and buses, which makes our lives much simpler.

It's quite important to mention that we should aim at doing this without breaking BC too much (registering aliases for things and still look for parameters using the app name as the prefix).

@rodrigorigotti
Copy link
Contributor

I'm studying a fix for this issue, and the idea I have in mind is as follows:

  • We'd still allow the name to be passed to RegisterApplication instead of relying on a default app name (eg, app) to keep BC
  • \Chimera\DependencyInjection\RegisterApplication::$name becomes a static property so that in case the application tried to register a second application it throws an exception (since we only allow single-applications registrations)
  • We apply the same behaviour on \Chimera\DependencyInjection\Routing\Expressive\RegisterServices and \Chimera\DependencyInjection\Routing\Expressive\Package which also rely on the application name (consequently simplifying the logic inside them, eg when extracting route list or extracting midldeware list)

What do you think?

@lcobucci
Copy link
Member Author

I don't believe we should rely on the user provided name to control that.

On the first run we should register the web application using Chimera\Routing\Application as an alias. If someone tries to register something again we check whether the alias exists or not.

For BC, we'd create an alias for the user provided name (which points to Chimera\Routing\Application).

All the compiler passes we have should use the class names as service ids (and map the interface as aliases), because they will only be registered once. This gives us an opportunity to clean up the dynamic service creation and pull things into the XML files.

Does that make sense to you?

@rodrigorigotti
Copy link
Contributor

Okay, so let me see if I understood correctly: when processing the services for an application we should add a Chimera\Routing\Application alias to it at the very end of this process. This alias would then be present in the container. Because of this, trying to register a second application would cause an exception because the alias already exists.

The second part makes sense since there would be a single instance of each compiler pass since there's only a single application registered. I only have to see what needs to be moved to the XML files but, like you said, we clean up a lot of PHP making the whole service definition a lot easier.

@WyriHaximus
Copy link

Have you considered splitting your app up in logistical chunks? For example app with your main app, metrics to expose metrics, and healtz to provide a health check. All of these would serve their domain isolated from the others so you can accidentally export metrics or your health check from your app.

@rdohms rdohms linked a pull request Feb 5, 2021 that will close this issue
@lcobucci lcobucci modified the milestones: 0.4.0, 0.5.0 Feb 21, 2021
@lcobucci lcobucci modified the milestones: 0.5.0, 1.0.0 Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Improvement of existing feature
Projects
None yet
3 participants