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

Enable usage of Bot.Builder with ASP.Net Core #2346

Closed
wants to merge 25 commits into
base: master
from

Conversation

4 participants
@iedeny
Member

iedeny commented Feb 25, 2017

Purpose of changes

The goal of proposed change is to decouple Bot.Builder projects from specific version of Bot.Connector that will enable us to use Bot.Builder with ASP.Net Core.
Currently it is not possible due to following dependency chain: Bot.Builder -> Bot.Connecor -> “regular” ASP.Net 4.6 (see image below).

Dependency diagram before changes:
vs codemap original

Dependency diagram after proposed changes:
vs codemap proposed

Project level changes:

  • Create new portable Bot.Connector.Common.csproj based on Bot.Connector.Shared.shproj.
    New project will that target .NET Standard 1.4 which can be referenced from both .NET Framework and .NET Core (details)
  • Change Bot.Connector.AspNetCore to target .NET Standard 1.4 (currently targets 1.6).
    This will allow Bot.Connector.AspNetCore to be referenced by .NET Framework 4.6.1 and .NET Core 1.0 projects
    (currently there is no version of .NET Framework that implemented .NET Standard 1.6)
  • Create new Bot.Connector.AspNetCore.Mvc projects which contain TrustServiceUrlAttribute
    • Bot.Connector.AspNetCore.Mvc targets .NET Standard 1.6 (we cannot target 1.4 because Microsoft.AspNetCore.Mvc
      has a dependency on .NET Standard 1.6). As a result currently this project can only be used with .NET Core.
      This limitation should go away once .NET Standard 2.0 is released and supported by MVC.
    • Bot.Connector.AspNetCore.Mvc.NetFramework targets .NET Framework 4.6.1. This porject should be used if you are using ASP.Net Core with .NET Framework.
    • Once .NET Standard 2.0 is released and supported by MVC we can change first project to target it. As a result second project can be removed.
    • Both projects are distributed as a single package (depending on platform nuget will reference one assembly or another)
  • Remove Bot.Connector.NetCore project
  • Replace reference from Bot.Builder to Bot.Connector with reference to Bot.Connector.Common* (see diagram above)
  • Change all .NET Framework projects (non-portable) to target version 4.6.1 (currently it is a mix of 4.5 and 4.6)
    This is currently only platform that supports .NET Stanard 1.4
  • Migrate .xproj/project.json to new .csproj format.
    Visual Studio 2017 and .NET Core tools are deprecating .xproj/project.json in favor of new .csproj format.
    .NET Core tools for Visual Studio 2015 does not correctly handle references from .csproj to .xproj
    As a result we had to:
    • Convert Bot.Connector.AspNetCore to regular .csproj format
    • Convert Microsoft.Bot.Sample.AspNetCore.Echo to new .csproj format and
      remove it from original solution (.NET Core tools for VS 2015 are still in preview and do not support new .csproj yet).
    • Create new Microsoft.Bot.Builder.VS2017.sln solution file that contains all projects from original solution + .NET Core samples.
  • Consolidate and update some package dependencies (should not affect existing developers, nuget manager will take care of updating dependencies).

Removing conditional compilation

One of requirements of making Bot.Connector.Common portable is to remove conditionally compiled code (anything that is wrapped by "#if NET45" or "#if !NET45") under Bot.Connector.Shared.
Logging and Configuration are not implemented identically on both platforms.

For the Bot.Connector.Common to be able to use configuration and logging, instead of calling platform specific ConfigurationManager and Tracer classes, it now depends
on .NET extension packages like Microsoft.Extensions.Configuration.Abstractions and Microsoft.Extensions.Logging.Abstractions. This way the connector has access to
interfaces like ILogger and IConfigurationRoot and it is responsibility of the platform specific connector (either the ASP.Net or the ASP.Net Core) to provide an implementation.

The dependency injection process happens in three possible ways:

  1. In ASP.Net Core when the user register Bot.Connector on the Startup class by calling: services.UseBot.Connector(); on an instance of IServiceCollection
  2. In ASP.Net automatically, if any type of Microsoft.Bot.Connector is used.
  3. In either platform, if Bot.Builder is used, Autofac will load the platform specific dll from the AppDomain and perform the registration

Dependency injection in details

The Bot.Connector.Common assembly implements a singleton named ServiceProvider. It has handy methods like CreateLogger and properties like ConfigurationRoot.
Through it, the common code can access platform specific implementation through the interfaces mentioned before.

The ServiceProvider has the static method RegisterServiceProvider that is called by the platform specific connector to provide the implementation of the required interfaces.

The dependency injection process happens in three possible ways:

  1. In ASP.Net Core when the user register Bot.Connector on the Startup class by calling: services.UseBot.Connector(); on an instance of IServiceCollection
    • Internally, the UseBot.Connector will call ServiceProvider.RegisterServiceProvider passing the AspNetCore implementation of the required interfaces.
    • Given that Bot.Connector.AspNetCore is in Alpha stages, we felt this is an accetable breaking change.
  2. In ASP.Net automatically, if any type of Microsoft.Bot.Connector is used.
    • To prevent a breaking change of consumers using the ASP.NET connector without Bot.Builder, we had to provide a mechanism to load automatically register the platform specific interfaces.
      It works this way: for the classes in the ASP.Net connector, we implemented static constructors that will trigger the ServiceProvider registration automatically. This means that if any
      classes from the ASP.Net connector are used, then the registration happens automatically.
  3. In either platform, if Bot.Builder is used, Autofac will load the platform specific dll from the AppDomain and perform the registration
    • Bot.Builder uses AutoFac as a way to provide inversion of control. We leverage this to use Autofac to find the platform specific Connector assembly in the AppDomain and use it to
      register the required interfaces against the Connector.Common ServiceProvider.

Care was taken to make sure that the dependency injection process happens exactly once.

Nuget packaging changes

  • Make sure existing bots will not be broken after updating to new packages.
  • Keep existing Microsoft.Bot.Builder package as a “bundle/shim” package for backward compatibly
    (it will not contain any assemblies and will simple reference Connector and Builder.Framework packages below)
  • Create following new packages:
    • Microsoft.Bot.Connector.Common package - the connector code that is platform agnostic and can be shared across all platforms
    • Microsoft.Bot.Builder.Framework package - the Bot.Builder and Bot.Autofac libraries that can be used with any Connector implementation
    • Microsoft.Bot.Connector.AspNetCore package - the ASP.NET Core integration for the Bot Connector
    • Microsoft.Bot.Connector.AspNetCore.Mvc package - the ASP.NET Core MVC integration for the Bot Connector

package diagram

Testing

  • Uptake the changes in all existing samples/tests and make sure samples work and unit tests pass
    • 100% unit test passrate
    • Microsoft.Bot.Sample.AspNetCore.Echo
    • Microsoft.Bot.Sample.AspNetCore.Pizza - new sample that uses ASP.Net Core (under .NET Framework) with Bot.Builder.
    • Microsoft.Bot.Sample.SimpleEchoBot
    • Microsoft.Bot.Sample.SimpleSandwichBot
    • Microsoft.Bot.Sample.TemplateBot
    • Microsoft.Bot.Sample.AlarmBot - need .json file with LUIS model (quota for current model exceeded)
    • Microsoft.Bot.Sample.AnnotatedSandwichBot - need .json file with LUIS model (quota for current model exceeded). Tried importing model from report it fails.
    • Microsoft.Bot.Sample.SimpleAlarmBot - need .json file with LUIS model (quota for current model exceeded)
    • Microsoft.Bot.Sample.SimpleDispatchDialog - need .json file with LUIS model (quota for current model exceeded)
    • Microsoft.Bot.Sample.SimpleFacebookAuthBot - need instructions how to test (tried with localhost but facebook rejects it)
    • Microsoft.Bot.Sample.SimpleIVRBot - need instructions how to test Calling controller
    • Stock_Bot (3 bots are not part of solution) - follow up if those but are still relevant

iedeny and others added some commits Feb 11, 2017

Consolidate all nuget packages.
Update all packages except Moq, Autofac and Castle.Core
Convert AspNetCore Connector into portable library
 - Convert aspnet core connector to netstandard 1.4
 - Fix System.Net.Http binding
Remove xproj
Add VS2017 solution to support netcore projects
Enable ASP.NET CORE usage with BotBuilder
 - Fix simple sandwich bot and add TODO comment
 - Improve Service resolution for multi-platform support
 - Remove remaining #IF NET45 compiler conditionals
 - Remove #IF NET45 conditionals merge from upstream
Update System.Net.Http to 4.3.1
This is required to address issue dotnet/corefx#11100
Rename Connector.Abstractions folder back to Connector.Shared
 - Remove ApplicationInsights package from AspNetCore.Pizza sample
 - Consolidate packages to latest
 - Fix folder name in packaging script for Shared project
 - Fix remaining unit test failures
 - Consolidate assembly version
Add check for service provider registered on autofac composition
 - Add missing packages to samples
 - Fix authentication on AspNetCore project when secret is not provided
@azurecla

This comment has been minimized.

Show comment
Hide comment
@azurecla

azurecla Feb 25, 2017

Hi @iedeny, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!


It looks like you're working at Microsoft (iedeny). If you're full-time, we DON'T require a contribution license agreement.



If you are a vendor, DO please sign the electronic contribution license agreement. It will take 2 minutes and there's no faxing! https://cla.azure.com.

TTYL, AZPRBOT;

azurecla commented Feb 25, 2017

Hi @iedeny, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!


It looks like you're working at Microsoft (iedeny). If you're full-time, we DON'T require a contribution license agreement.



If you are a vendor, DO please sign the electronic contribution license agreement. It will take 2 minutes and there's no faxing! https://cla.azure.com.

TTYL, AZPRBOT;

@msft-shahins msft-shahins self-assigned this Mar 1, 2017

@msft-shahins

This comment has been minimized.

Show comment
Hide comment
@msft-shahins

msft-shahins Mar 21, 2017

Contributor

I merged this pull request to develop branch: 0123c41

Contributor

msft-shahins commented Mar 21, 2017

I merged this pull request to develop branch: 0123c41

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment