Implement dependency injection for application instance, server and n… #3022
Conversation
|
|
wbtoms
force-pushed
the
IntroduceDependencyInjection
branch
from
6feb171 to
357cb10
Compare
wbtoms
force-pushed
the
IntroduceDependencyInjection
branch
from
357cb10 to
c326401
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3022 +/- ##
==========================================
+ Coverage 56.64% 56.65% +0.01%
==========================================
Files 356 359 +3
Lines 68435 68477 +42
Branches 14072 14080 +8
==========================================
+ Hits 38765 38797 +32
- Misses 25491 25493 +2
- Partials 4179 4187 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/azp run |
|
Pull request contains merge conflicts. |
wbtoms
force-pushed
the
IntroduceDependencyInjection
branch
from
c326401 to
7dfa024
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| ConfigSectionName = configSectionName, | ||
| CertificatePasswordProvider = PasswordProvider | ||
| }; | ||
| var passwordProvider = new CertificatePasswordProvider(Password); |
There was a problem hiding this comment.
previously LoadAsync created a new application instance, now it doesnt.
I need to check...
There was a problem hiding this comment.
Yes, now this object comes from the DI. This was the intension.
There was a problem hiding this comment.
Yeah but if you call load twice previously a new instance was created now the Same is reused. I need to Check If this could cause unwanted behaviour changes
There was a problem hiding this comment.
I see, for this shadow config use case, right? I think this works becauce LoadApplicationConfiguration sets the application configuration to null before relaoding it so it is always a complete new instance of the configuration.
But what is the shadow configuration use case for excatly? do you know?
wbtoms
force-pushed
the
IntroduceDependencyInjection
branch
from
6cce6c8 to
c4b74f7
Compare
Proposed changes
When integrating UA-.NETStandard, it is difficult to perform unit tests or even integration while you basically need to setup almost all objects from the application instance to the node managers with their actual implementations. No possibility to mock them or to create what is just necessary.
It is also diffiicult to replace an implementation of a given component by another custom implementation.
Therefore I propose this change which is, a first step, to introduce dependency injections for:
Related Issues
No related open issue found.
Types of changes
What types of changes does your code introduce?
Put an
xin the boxes that apply. You can also fill these out after creating the PR.Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
If this proposition is interresting and at some point is merged, it might be interresting also to continue with orther types of object especially everything around NodeState. And of course, the client library should be treated the same way. But before going further, let see if this make sense for the audience.