-
Notifications
You must be signed in to change notification settings - Fork 516
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
feat: inject profile #2705
feat: inject profile #2705
Conversation
Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
Feedback on any unforeseen consequences of this are welcome; given that this pattern was kind of present in those profile init bind provider calls, I think it should be fine. |
Sorry, mixed up the assignee and reviewer fields... 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't comment on unforeseen consequences, but it looks good to me.
This PR adds the Profile to the injection context. This enables creating a
ClassProvider
for a class that takes a profile in its init method:It's a common pattern to create a class with a profile instance. This will make it easier to turn some of these components into pluggable components. For example, the
VcLdpManager
. At the moment, it's created directly where it's needed:With this change, I can create an (override-able) class provider for VcLdpManager, which changes the above into:
The only way this could be done previously was by binding providers in the Profile init method itself. This made it more difficult to make more components pluggable.
Side note: it's critical that the profile instance be injected as a weak reference or it will never be "finalized" (garbage collected) in multitenant contexts.