Named Profiles for option configuration

[gplwhite]

This PR aims to add make it easier to configure different sets of options for different request scenarios.

I started out coming at this from the perspective that I don't like the current method of enabling "debugging" output (ie: unbundled file output). I really wanted to make it easier to have debugging output automatically enabled when developing locally, but disabled for production use. Prior to this PR, you have to explicitly pass a debug parameter to the various SmidgeHelper methods (and TagHelpers). This made it cumbersome to globally enable/disable debugging output and put a lot of onus on the user to determine when debugging should be enabled or not.

For example, the documentation suggests the following to achieve debugging output during development.

<environment names="Development">
    <script src="my-awesome-js-bundle" type="text/javascript" debug="true"></script>
</environment>
<environment names="Staging,Production">
    <script src="my-awesome-js-bundle" type="text/javascript"></script>
</environment>

But if you have multiple bundles in various places (think CSS in the page head, JS in the footer) etc then this becomes pretty verbose.

My Solution

I have added the concept of Named Profiles - where a profile is represented by the existing BundleOptions class. The existing BundleEnvironmentOptions is now responsible for managing the various profile configurations. The existing Debug and Production BundleOptions remain, but it is possible to add additional "named" options.
Secondly, I've added the ISmidgeProfileStrategy interface and a couple of default implementations. This interface is used by the SmidgeHelper to decide which profile (ie: pre-configured BundleOptions) to use for a given request.

The DefaultProfileStrategy is configured in the DI container. It just uses the Default (synonym for existing Production options) profile for all requests. I think this maintains the current out-of-the-box functionality.
There is also a HostEnvironmentProfileStrategy which if configured in the DI container will use the current IHostEnvironment to determine if the Debug or Default profile should be used.

Now its as simple as
services.AddSingleton<ISmidgeProfileStrategy, HostEnvironmentProfileStrategy>();
and you get debug output when developing locally and standard output for anything other than the Development environment.

As I said above, I was primarily wanting to make debug output easier to enable. But I think these changes offer even more extensibility. It is now possible to configure multiple sets of BundleOptions for different scenarios, and then either refer to the named profile when defining a Bundle or use a custom implementation of ISmidgeProfileStrategy that determines which BundleOptions to use at run time.

For example

 services.Configure<SmidgeOptions>(options =>
           {
               // Create some custom profiles with specific configurations
               var noCompressionProfile = options.DefaultBundleOptions["NoCompression"];
               noCompressionProfile.ProcessAsCompositeFile = true;
               noCompressionProfile.CompressResult = false;

               var cacheForeverProfile = options.DefaultBundleOptions["CacheForever"];
               cacheForeverProfile.ProcessAsCompositeFile = true;
               cacheForeverProfile.CacheControlOptions.CacheControlMaxAge = 99999;
           });

app.UseSmidge(bundles =>
            {
                bundles.CreateJs("bundle-1", "~/Js/1.js", "~/Js/2.js")
                    .UseProfile("NoCompression");
            });

or

    public class MyCustomProfileStrategy : ISmidgeProfileStrategy
    {
        private readonly HttpContext _httpContext;
        
        public MyCustomProfileStrategy(HttpContext httpContext) => _httpContext = httpContext;

        public string GetCurrentProfileName()
        {
            // Pick a Profile based on the request
            return _httpContext.Request.Host.Value.Contains("something")
                ? "NoCompression"
                : "CacheForever";
        }
    }

Backwards Compatibility

I've tried to maintain backwards compatibility, so I think this will be a fairly simple upgrade for most existing users. The debug parameter on the SmidgeHelper and the TagHelpers has been maintained although it has been changed to a Nullable. If the property is explicitly set it will override the profile configured during setup.
At some point in the future I think it would be good to remove the debug parameter as it will simplify the SmidgeHelper API surface area.

[Shazwazza]

So sorry I haven't seen this PR yet, somehow slipped through the cracks in my emails. I'll review and feedback asap

[Shazwazza]

Thanks so much for the effort here and again appologize for the delay! This is great stuff. I've left some comments in the PR.

[Shazwazza]

Do we want to check the profileName exists here with TryGetValue(profileName, out var options) instead of assuming the key exists?

[gplwhite]

This doesn't assume the profile exists. The BundleEnvironmentOptions collection guarantees that a BundleOptions instance will be returned when asking for a BundleOptions instance via the indexer property (a new instance will be created and stored in the collection if a profile with the requested name doesn't yet exist). (See BundleEnvironmentOptions indexer)

In the case you really want to know if a named profile exists (and without automatically creating one if it doesnt) you can call BundleEnvironmentOptions.TryGetProfileOptions();

So, in general it is safe to call bundleMgr.DefaultBundleOptions[profileName] or bundle.BundleOptions[profileName] without checking the return value for null.

[Shazwazza]

can we remove this commented out code?

[gplwhite]

Have removed the commented out code, and made a couple of other tidy ups.

[gplwhite]

Thanks so much for the effort here and again appologize for the delay! This is great stuff. I've left some comments in the PR.

No problem - it kinda slipped off my radar now too. I'm away from the office at the moment so won't get a chance to action the feedback for a week or two. Will push the updates as soon as a can.

Cheers.

[Shazwazza]

Have added a few questions

[Shazwazza]

Is there a reason for the ConcurrentDictionary if it isn't being exposed via the _profileOptions? This means that the concurrency methods of ConcurrentDictionary are not used, especially within the indexer method. I would assume we'd use GetOrAdd (etc...) instead of double operation of checking existence and then writing.

[Shazwazza]

Will the current way of adding debug: false still work when rendering?

[Shazwazza]

And if so, we should keep some of those tests here to ensure they all work.

[Shazwazza]

There's a few ctor changes on public classes which would be breaking changes requiring a major version rev. Can we avoid this with overloads? Else I think this would technically need a new major version