Migrating diagnostics to the unified configuration API

[thecodejunkie]

Migrated the configuration of the diagnostics system to the new unified configuration API, according to #1999 and #2000.

The following steps were performed

• Updated DiagnosticsConfiguration class to be immutable
• Added DiagnosticsConfiguration.Default static field that returns the default diagnostics configuration
• Added DiagnosticsDefaultConfigurationProvider that returns DiagnosticsConfiguration.Default
• Added Diagnostics extension method to INancyEnironment to enable configuration of diagnostics
• Removed NancyBootstrapperBase.DiagnosticsConfiguration method
• Updated tests to use the new Diagostics extension method instead of overriding ConfigurableBootstrapper.DiagnosticsConfiguration
• Updated sample projects to use the Diagostics extension method instead of overriding NancyBootstrapperBase.DiagnosticsConfiguration

I want this pull-request to server as a discussion point for how we perform the migrations. I will update #2000 according to review feedback

This looks like a massive pull-request, because I branched of the - yet unmerged - code in #2003 it is only the last 5 commits that are unique to this pull-request

The DiagnosticsConfiguration class defines a Default static field that returns the default diagnostics configuration

public static DiagnosticsConfiguration Default = new DiagnosticsConfiguration
{
    CookieName = "__ncd",
    CryptographyConfiguration = CryptographyConfiguration.Default,
    Password = null,
    Path = "/_Nancy",
    SlidingTimeout = 15
};

The public constructor checks if values are null and if they are it will pull out the values from Default

public DiagnosticsConfiguration(string password, string path, string cookieName, int slidingTimeout, CryptographyConfiguration cryptographyConfiguration)
{
    this.Password = password ?? Default.Password;
    this.Path = GetNormalizedPath(path ?? Default.Path);
    this.CookieName = cookieName ?? Default.CookieName;
    this.SlidingTimeout = slidingTimeout;
    this.CryptographyConfiguration = cryptographyConfiguration ?? Default.CryptographyConfiguration;
}

The Diagnostics extension on INancyEnvironment takes advantage of this

public static void Diagnostics(this INancyEnvironment environment, string password, string path = null, string cookieName = null, int slidingTimeout = 15, CryptographyConfiguration cryptographyConfiguration = null)
{
    environment.AddValue(new DiagnosticsConfiguration(
        password,
        path,
        cookieName,
        slidingTimeout,
        cryptographyConfiguration));
}

All values, except password have default values, which will force the DiagnosticsConfiguration ctor revert back to using the values defined in Default. The password is a mandatory value, hence no default fallback. The method was purposly designed this way to cater for the following API when consuming it

environment.Diagnostics(
    password: "Password",
    path: "some/path",
    cookieName: "__DiagName");

Of course the parameter names are optional, but they provide a much higher sense of readability and it is the style that I think that we should promote in our documentation for the guidelines for authoring configuration support into extensions.

Update 2015-11-07

This change also includes an INancyDefaultConfigurationProviders implementation for diagnostics (read more about them in #1999)

public class DiagnosticsDefaultConfigurationProvider : INancyDefaultConfigurationProvider
{
    public object GetDefaultConfiguration()
    {
        return DiagnosticsConfiguration.Default;
    }
}

⚠️ Question: Technically the DiagnosticsConfiguration class could implement INancyDefaultConfigurationProvider instread of having a completely different class that just returns DiagnosticsConfiguration.Default ? What should be the best-practice approach that we take for our configurations? Unique class or have the config object implement it? Our guidelines should recommend to always have an INancyDefaultConfigurationProvider implementation so you avoid in an unconfigured state.

Update 2015-11-09

Updated DiagnosticsDefaultConfigurationProvider to inherit from NancyDefaultConfigurationProvider and use the default key name (Type.FullName)

[thecodejunkie]

I will rebase this one #2003 has been merged

[thecodejunkie]

Rebased, but please do not merge. Read the initial description

I want this pull-request to server as a discussion point for how we perform the migrations

However this needs merging before we merge v2 back into master

[khellang]

LGTM 👍

I think we should just merge this and get to work on the rest of the configuration stuff. We'll see how it works out down the road.

[xt0rted]

Are these v2 builds listed on myget yet?

[khellang]

Some are, but we haven't gotten it building after the merge to master 😛