[WIP] Type catalog (Making type scanning instance based)

[thecodejunkie]

Do not merge this yet

This is the first swing at making the type scanning instance based. We've introduced the new ITypeCatalog interface and stuck it on the bootstrapper in the form of a booststrapper so you can override it with your own implementation. The DefaultTypeCatalog implementation is just a proxy around the old AppDomainAssemblyTypeScanner with one change, ADATS now delay the scanning until the first time you try to pull types out of it.

I'd like to get a round of feedback on this before we pull it in. I know @damianh has had some issued with it not being instance based before, so just want to make sure his old pain point(s) is covered by this change.

Nancy

• NancyInternalConfiguration - This is the main consumer, the problem here is that is it being used from the static property Default. The class is used from the InternalConfiguration property on the NancyBoostrapperBase
• Registrations - It's used in a couple of helper methods. The Registrations instances are pulled from the container but the problem here is that we would have to enforce a ctor signature (something like protected Registrations(ITypeCatalog catalog)) and force all implementations to call it, so a major breaking change
• NancyConventions - Used to find the default conventions. Another issue is how we make use of the conventions. Currently we just create a new instance, of NancyConventions in NancyBoostrapperBase.ctor
• DiagnosticsModuleCatalog - Used to find all DiagnosticModule instances and shoving them into the container. We manually create an instance of this in the DiagnosticsHook.Enable method. This should not be a problem because the way we call this method is from inside DefaultDiagnostics which is an implementation of IDiagnostics and we pull that from the container already, so we just need to pass it down the chain.
• InfoModule - This is part of diagnostics and is a DiagnosticModule, so we can just add it to the ctor signature of the module, no worries
• SuperSimpleViewEngineRegistrations - Used to find all ISuperSimpleViewEngineMatcher and register them in the container. This should not be an issue because it is an implementation of IRegistrations and they are pulled from the container, so we should just be able to add the type catalog on the ctor
• ResourceAssemblyProvider - Is an implementation of IResourceAssemblyProvider that we stick into the container, from NancyInternalConfiguration so we can just add the catalog to the signature. HOWEVER It does not use ADATS for type discovery, it uses ADATS for ASSEMBLY DISCOVERY it grabs hold of all the loaded assemblies (the ones that ADATS loaded), that are not a Nancy-assembly. This breaks the signature we have for ITypeCatalog (new interface for this pull-request) because it currently only expose methods for grabbing types.
• StaticConfiguration - Used to figure out if we're running in debug mode or not. The biggest issue here is that, as the name suggest, this is all based on being super static! We've talked about redesigning this, and it's probably a requirement if we want to move forward with making type resolution instance based. Yak shaving galore!

Nancy.ViewEngines.Razor

• Used to load in assemblies that you have defined in the web.config
• Used to discover model types
• Used to get assembly names when building up a descriptive error name when it could not find a model type

Nancy.Testing

• ConfigurableBootstrapper - Used in the LoadReferencesForAssemblyUnderTest method that tires to make sure that all assemblies are loaded that is going to be needed to run the test (it has to do with the whole problem we sometimes have where .net tries to be smart with removing unused references)

Nancy.ViewEngines.Razor.Tests

• Used in a test to ensure sure that an assembly is loaded
• Used in a test that ensures that a model can be found in that assembly

Nancy.Demo.Hosting.Aspnet

• Used in a custom IResourceAssemblyProvider implementation. It's used to find all non dynamic assemblies. Again, this is another situation where ADATS is used to get assemblies instead of types

Nancy.Demo.Razor.Localiztion

• Used in a custom IResourceAssemblyProvider implementation. It's used to find all non dynamic assemblies. Again, this is another situation where ADATS is used to get assemblies instead of types

[thecodejunkie]

Right, just found the first pain point. While going back and changing direct uses of ADATS into ITypeCatalog usage, I came across the GetBoostrapperType method of the NancyBootstrapperLocator. It makes use of ADATS to figure out if there are any bootstrapper subclasses that should be used.

At this point, the bootstrapper is not created (and initialised) so there won't be an ITypeCatalog instance to use. A bit of a chicken & egg scenario here.

[grumpydev]

Merged.

[grumpydev]

😉 I think for the bootstrapper locator we should just put scanning logic directly in there, rather than using adats

[06b]

fyi, Some typos on the xml comments - determin -> determine

[thecodejunkie]

ping @NancyFx/most-valued-minions @NancyFx/nancy-core-contributors

I updated the description with information on all of the (remaining) places where ADATS is directly used. Some of them are simple to resolve, but there are a bunch of them that will require us to change things around, so feedback'a'hoy! =)

[khellang]

So IAssemblyCatalog as well then? Then either create a separate DefaultAssemblyCatalog or make the existing DefaultTypeCatalog implement the interface and change its name?

[thecodejunkie]

That was also my immediate reaction, but after thinking about it a bit more I do not think it is the path we want to follow. I reckon that most ITypeCatalog implementations would like to be based around consuming an IAssemblyCatalog to know which assemblies it should scan for types. Right there and then we run into a major road block, because we do not shove the ITypeCatalog into the container and yank it back out again i.e. it cannot have any dependencies satisfied by the container.

This is how we currently rig up the ITypeCatalog in the NancyBootstrapperBase

protected ITypeCatalog TypeCatalog
{
    get { return new DefaultTypeCatalog(); }
}

We then register it as an instance in the container, using the GetAdditionalInstances() method on the NancyBootstrapperBase. The reason we ended up doing it this way is that the NancyBootstrapperBase made internal use of ADATS to do certain things (a lot of things).

[khellang]

Moving this off 1.2 since it'll take a bit longer for it to be finished 😄

[thecodejunkie]

Bringing this back to life, coz' v2 :)

Pinging Calling all @NancyFx/most-valued-minions and @NancyFx/owners

Roll out!

[jchannon]

One thing to remember is that DNX does not have an AppDomain and reflection API is slightly different so we may need some #ifdef or we try doing it differently somehow

[khellang]

We can just base a DNX ITypeCatalog on ILibraryManager instead of relying on AppDomain 😄

[khellang]

For reference, this is how MVC does it: https://github.com/aspnet/Mvc/blob/d95e9956fe7233064a500ce90a3305c762cb2e37/src/Microsoft.AspNet.Mvc.Core/DefaultAssemblyProvider.cs

Remind you of something you've seen before? 😉

[jchannon]

C# 6 too - get them! ;)

On 15 July 2015 at 13:26, Kristian Hellang [email protected] wrote:

For reference, this is how MVC does it:
https://github.com/aspnet/Mvc/blob/d95e9956fe7233064a500ce90a3305c762cb2e37/src/Microsoft.AspNet.Mvc.Core/DefaultAssemblyProvider.cs

Remind you of something you've seen before? [image: 😉]

—
Reply to this email directly or view it on GitHub
#1846 (comment).

[grumpydev]

If I can't get to it before I will look at it when I'm on holiday start of August.. hoping to look before then though :)

[thecodejunkie]

For NancyConventions I did the following changes

1. Made the NancyConventions.BuildDefaultConventions() method public, instead of private and added an ITypeCatalog parameter
2. Updated NancyBoostrapperBase.Initialise method to call it, passing in the this.TypeCatalog instance, right before it does the convention stuff here

It works, but it is a bit ugly. The entire NancyConventions thing should be redesigned. Possibly using a new configuration mechanism that's been talked about

[thecodejunkie]

Made the following changes to allow NancyInternalConfiguration to use an ITypeCatalog instead of going straight to ADATS

Changed the signature of the InternalConfiguration property on the NancyBootstrapperBase

protected virtual Func<ITypeCatalog, NancyInternalConfiguration> InternalConfiguration
{
     get
     {
          return NancyInternalConfiguration.Default;
     }
}

The Default property and WithOverrides method, on NancyInternalConfiguration and returns Func<ITypeCatalog, NancyInternalConfiguration> instead of a plain instance.

The bootstrapper will then grab the actual instance, by passing in the ITypeCatalog into the func.

I've also changes, so that internally, the bootstrapper uses this method, instead of goings straight to the InternalConfiguration

private NancyInternalConfiguration GetInternalConfiguration()
{
   return this.internalConfiguration ?? (this.internalConfiguration = this.InternalConfiguration.Invoke(this.TypeCatalog));
}

which will make sure the instance is cached. Before this chance, InternalConfiguration would perform this caching, but if a user did override it then the caching would be lost.

[damianh]