microsoft / botbuilder-dotnet

Welcome to the Bot Framework SDK for .NET repository, which is the home for the libraries and packages that enable developers to build sophisticated bot applications using .NET.
https://github.com/Microsoft/botframework
MIT License
871 stars 478 forks source link

[Draft] bug: Adapters should work with skills [parent issue] #5606

Closed mdrichardson closed 2 years ago

mdrichardson commented 3 years ago

Draft

SkillHandler doesn't know how to hand off replies from the skill to the correct adapter when custom adapters (Slack, Facebook, etc) are used--it uses whichever BotAdapter is dependency-injected.

We could use some design planning about how to fix this.

Current idea is something along these lines, from @carlosscastro:

create an adapter that does the multiplexing internally, which means that the controller would pass the payload and route (which is the multiplexing key) to the multiplexing adapter, and the multiplexing adapter would then internally route to the right inner adapter. This then allows the skill handler to continue to receive just one adapter (though we'd have to tweak DI to make sure the one that goes there is the multiplexing adapter). The final step would be to update generators controller to use this new adapter instead of receiving multiple adapters. The constructor for this adapter would be somehting like MultiplexingAdapter(IConfiguration config, IEnumerable adapters). The mapping route -> adapter is in the config, so these 2 parameters would suffice for the purposes of this functionality.

Progress

Facebook

Slack

Twilio

Webex

carlosscastro commented 3 years ago

I think having the multiplexing adapter is a better design than having the skill handler and the generator controller both deal with the multi-adapter complexity. If we could centralize that responsibility into a single abstraction then that would compose nicely with the rest of the system. So I do prefer the multiplexing approach rather than the skill handler receiving multiple adapters approach.

mdrichardson commented 3 years ago

@carlosscastro I should've erased that (I have now). That was just my original half-baked idea that I kept in for posterity.