[feature/10.0] Move binding logic to descriptors #8126
Conversation
sbomer
changed the title
| @@ -73,6 +75,20 @@ public ICollectionRuleAction Create(IProcessInfo processInfo, BaseRecordOptions | |||
| } | |||
| } | |||
|
|
|||
| internal sealed class CallbackActionDescriptor : ICollectionRuleActionDescriptor | |||
There was a problem hiding this comment.
I assume that these cannot be handled by a single generic class because the configuration source generator wouldn't be able to intercept the Bind call? Because if it could, I think all of these could be collapsed into one class.
There was a problem hiding this comment.
That's right, the source generator can't intercept the call if it's made on a generic parameter.
| { | ||
| services.AddSingleton<TFactory>(); | ||
| services.AddSingleton<CollectionRuleActionFactoryProxy<TFactory, TOptions>>(); | ||
| services.AddSingleton<ICollectionRuleActionDescriptor, CollectionRuleActionDescriptor<TFactory, TOptions>>(sp => new CollectionRuleActionDescriptor<TFactory, TOptions>(actionName)); | ||
| services.AddSingleton<ICollectionRuleActionDescriptor, TDescriptor>(sp => new TDescriptor()); |
There was a problem hiding this comment.
Do the descriptors actually need to be explicitly constructed? Couldn't service registration just do this for us e.g. services.AddSingleton<ICollectionRuleActionDescriptor, TDescriptor>(). Same question for the other descriptor registrations. If not, you can also remove the new() constraint on TDescriptor.
There was a problem hiding this comment.
Fixed to let DI construct it for us, removed the new constraint.
- Don't construct descriptors explicitly
This is in preparation for using the configuration binding source generator, which needs the concrete type of the settings object to be available at the
Bindcallsite. We have that type available in the descriptors, so that seems like a natural place to do it. This change replaces the generic descriptor types with concrete ones that use the specific settings types.