Feature/automatic dependency injection 1
- Third attempt at adding automatic dependency injection.
- This one start from before
IStep. - Plugins now register
Autofac.IModule, which is configured before any step is run. Some plugin interface was removed as its no longer needed. -
IStepare now resolved via DI. So the plan is to migrate the steps in order of execution. This should eventually touch every component.
But why?
- Cleaner plugins. No need to add manual hooks. These hooks, factories and such can be done automatically. No need to add manual hooks for each use case.
- Eg: Replace
IVirtualMachinewithout creating an explicit hook, somewhere, or caring about dependency order. - Eg: This #6476, probably.
- Eg: Replace
- More flexible initialization.
- Eg: If i want to create a command that start VerifyTrie without starting anything else, it will automatically start only what is needed.
- Eg: If I want to start integration test for this particular part of code, I don't have to re-code how to wire them all up.
- Eg: If I want to re-start BlockTree for whatever reason, the lifecycle is much more clearly defined, so its a lot easier to achieve.
- Far less boilerplate (eventually).
- Automatically determine component dependencies, recursively. On missing deendency, it will throw.
- Automatically dispose components that implement IDispose. No longer miss dispose, probably.
Why AutoFac instead of .net core's Microsoft.Extensions.DependencyInjection?
- Mainly due to clearer separation lifetime. Core's DI, have transient, scoped and no-scoped, which is likely the only thing you need with a web server. Maybe its possible to have more nested lifetime, but I can't find doc for it or its hard to find doc for it. AutoFac have clear doc on how to create child lifetime recursively, which can be useful for things like world state lifetime, rpc module lifetime with inner world state lifetime, etc.
- AutoFac's IModule, which has a clearer intent IMO.
- Much clearer doc IMO. Most of Microsoft.Extensions.DependencyInjection is in context of ASP.Net.
- Have more feature. Probably a bad thing, probably not. Since I can dynamically resolve IConfig, its probably a good thing. who knows.
Changes
-
INethermindPluginnow resolved via dependency injection on a small pre-start container which have access to config. - Added
INethermindPlugin.Enabledwhich should return true if the plugin is enabled. Plugin is expected to use config to know if it is enabled or not. Disabled plugin will not have itsModuleloaded. - Added
INethermindPlugin.Modulewhich can return an AutofacIModuleto replace arbitrary component. Plugin that does not replace or declare anything yet, can just return null. Really... this is all a plugin usually need. - Remove
IConsensusPlugin.GetApi. Plugins now replaceINethermindApivia its module. - Remove
IInitializationPlugin. Plugins now declare that it should run step, by actually declaringIStepfrom its module. - Removed the previous special
IStepplugin discovery thing that depends on having the sameINethermindApiimplementation. -
IStepis now is resolved from the DI container. Previous handling to make sure overridingIStepfrom plugin take priority is kept. - Removed
InitializeNodeStatreplaced withNetworkModule. - Merged
FilterBootNodes,UpdateDiscoveryConfigintoSetupKeyStore. Most code inSetupKeyStoremoved toKeyStoreModule.
Types of changes
What types of changes does your code introduce?
- [X] Refactoring
Testing
Requires testing
- [X] Yes
- [ ] No
If yes, did you write tests?
- [X] Yes
- [ ] No
Notes on testing
- Seems to work.
- Running hive....
I thought container would replace INethermindApi alltogether?
This needs also a lot of testing (AuRa + Optimism too).
Eventually, yes.
@asdacap what do you think about something like this: A hybrid approach till we switch everything bit by bit to utilize DI https://github.com/NethermindEth/nethermind/blob/refactor%2Fintroduce_dependency_injection/src%2FNethermind%2FNethermind.Api%2FNethermindApiCustomResolver%20.cs Dont mind the implementation, it could probably be done in a better way, but the idea could stand?
@asdacap what do you think about something like this: A hybrid approach till we switch everything bit by bit to utilize DI https://github.com/NethermindEth/nethermind/blob/refactor%2Fintroduce_dependency_injection/src%2FNethermind%2FNethermind.Api%2FNethermindApiCustomResolver%20.cs Dont mind the implementation, it could probably be done in a better way, but the idea could stand?
You have a point. This would make sense if we are migrating something from the middle of the dependency stack where its dependency is not migrated yet, or new code that wants to immediately use dependency injection without waiting for its dependency to be migrated.
Hard to know exactly. I'm thinking of migrating things from bottom up, so there should not be a need for this except for new code. If guess if there is demand for it.
Can you layout the reason why autofac instead of Microsoft's Engine. I feel like Autofac is a bit more complicated. Is it justified?
Can you layout the reason why autofac instead of Microsoft's Engine. I feel like Autofac is a bit more complicated. Is it justified?
Added reasoning.