Update to AspNet HttpRequestHandler to use Fully Qualified Type Name (include assembly name)

[jfis]

GetType was returning null when configurationBootstrapperType.Name is not in the executing assembly. Assembly, which is a required attribute in the nancyFx config section, was not actually being used. More detail here: https://groups.google.com/forum/#!topic/nancy-web-framework/nxz9Pl4EUxE

[thecodejunkie]

We should use configurationBootstrapperType.AssemblyQualifiedName as that's the same thing, but without the string concatenation. Could you update it please?

[thecodejunkie]

Sorry, your code it correct, but could you change it to use string.Concat instead?

[jfis]

OK. But before I do, perhaps it's better to remove the assembly attribute and force the fully qualified type to be specified in the type attribute if needed?

ex.

<nancyFx>
  <bootstrapper type="Name.Space.Bootstrapper" assembly="Assembly.Name" />
</nancyFx>

would be

<nancyFx>
  <bootstrapper type="Name.Space.Bootstrapper, Assembly.Name" />
</nancyFx>

[thecodejunkie]

If we did that, then a) we would need to validate the string and throw a detailed exception if it was not in the right format and b) label it as a breaking change. It is a bit more explicit right now, because there are two fields

[jfis]

Because assembly is not actually being used in the current code, it is not a breaking change. Type could be specified same as it is now (ex. type="Name.Space.Bootstrapper") or fully qualified (ex. type="Name.Space.Bootstrapper, Assembly.Name"). The change to the line in question would be discarded. This is how I am currently working around this issue, but with a dummy assembly attribute.
Ex.

<nancyFx>
  <bootstrapper type="Name.Space.Bootstrapper, Assembly.Name" assembly="thisIsNotUsed" />
</nancyFx>

[jfis]

Regardless, I modified the original change to use string.Concat.

[horsdal]

@jfis The second commit broke the build.You can see the build server log if you click the red cross by the commit id.

Looking through the log the brake seems to be because of this:

Errors:
/home/travis/build/NancyFx/Nancy/src/Nancy.sln (default targets) ->
(Build target) ->
/home/travis/build/NancyFx/Nancy/src/Nancy.Tests/Nancy.Tests.csproj (default targets) ->
/usr/lib/mono/4.0/Microsoft.CSharp.targets (CoreCompile target) ->  Unit/Routing/DefaultRouteResolverFixture.cs(195,37): error CS1928: Type `string' does not contain a member `ShouldContain' and the best extension method overload `Nancy.Testing.AssertExtensions.ShouldContain(this Nancy.Testing.NodeWrapper, string, System.StringComparison)' has some invalid arguments
Unit/Routing/DefaultRouteResolverFixture.cs(195,37): error CS1929: Extension method instance type `string'   cannot be converted to `Nancy.Testing.NodeWrapper'
Unit/Routing/DefaultRouteResolverFixture.cs(196,37): error CS1928: Type `string' does not contain a member `ShouldContain' and the best extension method overload `Nancy.Testing.AssertExtensions.ShouldContain(this Nancy.Testing.NodeWrapper, string, System.StringComparison)' has some invalid arguments
Unit/Routing/DefaultRouteResolverFixture.cs(196,37): error CS1929: Extension method instance type `string' cannot be converted to `Nancy.Testing.NodeWrapper'

[horsdal]

A test that fails before the fix but not after would be good.

[jfis]

I see that it failed but I don't see how it relates to the change that was made (which was 1 line inside Hosting.AspNet). Was this test performed with any other changes to the code base? In other words, does it try to build with other changes that were applied externally to this pull request that may have happened since the original pull request, which passed? (I'm new to pull-requesting.)

[horsdal]

Do those tests compile locally? Have you rebased to make sure you are up to date?

[thecodejunkie]

Restarted Travis to see if it's was a Mono glitch

[jfis]

Is it because I didn't update my master before pushing the 2nd commit to the branch? I didn't think I had to do that since even if that is done, there's still a chance that it will be out of sync by the time the new commit is pushed. So at best it only minimizes chances (however small). That seems odd to me and would be a big problem on repos with many people pushing to master. Sorry for wasting your time. I'm new. I need to look into what rebasing does and also what happens when pull requests are tested. I'll scrap this and try again.

[horsdal]

Dont scrap it @thecodejunkie's restart worked - it's green now.
Still think a test for the fix would be nice, though :)

[thecodejunkie]

Yep, sometimes the Mono build on Travis doesn't play nice for what ever reason. I agree, if we could add a failing test then that would be awesome. I guess this is one of the "close to the metal" scenarios though and something like a config setting is hard to verify in a test