-
-
Notifications
You must be signed in to change notification settings - Fork 47
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
Named Profiles for option configuration #162
base: master
Are you sure you want to change the base?
Changes from all commits
480033f
439a43c
b1764f2
03e3faf
60c8137
7b1dfec
f4eba03
1a725bd
5c9ca9a
8851a6b
aa2770f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
using Smidge.Options; | ||
|
||
namespace Smidge | ||
{ | ||
|
||
/// <summary> | ||
/// An implementation of ISmidgeProfileStrategy that will always use the Default profile. | ||
/// </summary> | ||
/// <seealso cref="ISmidgeProfileStrategy" /> | ||
public class DefaultProfileStrategy : ISmidgeProfileStrategy | ||
{ | ||
public string GetCurrentProfileName() => SmidgeOptionsProfile.Default; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
using Microsoft.Extensions.Hosting; | ||
using Smidge.Options; | ||
|
||
namespace Smidge | ||
{ | ||
/// <summary> | ||
/// An implementation of ISmidgeProfileStrategy that will use the host environment to determine if the Debug profile should be used. | ||
/// </summary> | ||
/// <seealso cref="ISmidgeProfileStrategy" /> | ||
public class HostEnvironmentProfileStrategy : ISmidgeProfileStrategy | ||
{ | ||
private readonly IHostEnvironment _hostEnvironment; | ||
|
||
|
||
public HostEnvironmentProfileStrategy(IHostEnvironment hostEnvironment) | ||
{ | ||
_hostEnvironment = hostEnvironment; | ||
} | ||
|
||
|
||
private string _profileName; | ||
|
||
public string GetCurrentProfileName() => _profileName ??= GetProfileForEnvironment(_hostEnvironment); | ||
|
||
|
||
protected virtual string GetProfileForEnvironment(IHostEnvironment hostEnvironment) | ||
{ | ||
return hostEnvironment.IsDevelopment() | ||
? SmidgeOptionsProfile.Debug | ||
: SmidgeOptionsProfile.Default; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
|
||
namespace Smidge | ||
{ | ||
|
||
/// <summary> | ||
/// An interface that returns the name of an options profile to use for the current request. | ||
/// </summary> | ||
public interface ISmidgeProfileStrategy | ||
{ | ||
string GetCurrentProfileName(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,12 @@ | ||
using System; | ||
using Smidge.Cache; | ||
using System.Collections.Concurrent; | ||
using System.Collections.Generic; | ||
|
||
namespace Smidge.Options | ||
{ | ||
|
||
|
||
/// <summary> | ||
/// Defines the different bundle options for Debug vs Production | ||
/// Defines the different bundle options for various configuration profiles such as Debug or Production | ||
/// </summary> | ||
public sealed class BundleEnvironmentOptions | ||
{ | ||
|
@@ -20,14 +20,21 @@ public static BundleEnvironmentOptionsBuilder Create() | |
return new BundleEnvironmentOptionsBuilder(options); | ||
} | ||
|
||
|
||
|
||
private readonly IDictionary<string, BundleOptions> _profileOptions; | ||
|
||
|
||
/// <summary> | ||
/// Constructor, sets default options | ||
/// </summary> | ||
public BundleEnvironmentOptions() | ||
{ | ||
_profileOptions = new ConcurrentDictionary<string, BundleOptions>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
||
DebugOptions = new BundleOptions | ||
{ | ||
|
||
ProcessAsCompositeFile = false, | ||
CompressResult = false, | ||
CacheControlOptions = new CacheControlOptions | ||
|
@@ -36,17 +43,62 @@ public BundleEnvironmentOptions() | |
CacheControlMaxAge = 0 | ||
} | ||
}; | ||
ProductionOptions = new BundleOptions(); | ||
ProductionOptions = new BundleOptions(); | ||
} | ||
|
||
/// <summary> | ||
/// The options for the "debug" profile | ||
/// </summary> | ||
public BundleOptions DebugOptions | ||
{ | ||
get => this[SmidgeOptionsProfile.Debug]; | ||
set => this[SmidgeOptionsProfile.Debug] = value; | ||
} | ||
|
||
/// <summary> | ||
/// The options for debug mode | ||
/// The options for "production" profile | ||
/// </summary> | ||
public BundleOptions DebugOptions { get; set; } | ||
public BundleOptions ProductionOptions | ||
{ | ||
get => this[SmidgeOptionsProfile.Production]; | ||
set => this[SmidgeOptionsProfile.Production] = value; | ||
} | ||
|
||
|
||
|
||
/// <summary> | ||
/// The options for production mode | ||
/// Gets or sets the <see cref="BundleOptions"/> for the specified profile. | ||
/// If the profile has not been previously configured, returns a new <see cref="BundleOptions"/> instance. | ||
/// </summary> | ||
public BundleOptions ProductionOptions { get; set; } | ||
/// <param name="profileName">Name of the profile.</param> | ||
/// <returns></returns> | ||
public BundleOptions this[string profileName] | ||
{ | ||
get | ||
{ | ||
if (!TryGetProfileOptions(profileName, out BundleOptions options)) | ||
{ | ||
// Initialise a new BundleOptions for the requested profile | ||
options = new BundleOptions(); | ||
_profileOptions.Add(profileName, options); | ||
} | ||
|
||
return options; | ||
} | ||
set => _profileOptions[profileName] = value; | ||
} | ||
|
||
|
||
/// <summary> | ||
/// Gets the <see cref="BundleOptions"/> for the specified profile, if the profile has previously been configured. | ||
/// </summary> | ||
/// <param name="profileName">Name of the profile.</param> | ||
/// <param name="options">The profile options.</param> | ||
/// <returns></returns> | ||
public bool TryGetProfileOptions(string profileName, out BundleOptions options) | ||
{ | ||
return _profileOptions.TryGetValue(profileName, out options); | ||
} | ||
|
||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to check the profileName exists here with TryGetValue(profileName, out var options) instead of assuming the key exists?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.