Skip to content
This repository was archived by the owner on Nov 7, 2018. It is now read-only.

Options 2.0 - Add support for named options, validation, and caching #176

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions src/Microsoft.Extensions.Options/ConfigureNamedOptions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;

namespace Microsoft.Extensions.Options
{
/// <summary>
/// Implementation of IConfigureNamedOptions.
/// </summary>
/// <typeparam name="TOptions"></typeparam>
public class ConfigureNamedOptions<TOptions> : IConfigureNamedOptions<TOptions> where TOptions : class
{
/// <summary>
/// Constructor.
/// </summary>
/// <param name="name">The name of the options.</param>
/// <param name="action">The action to register.</param>
public ConfigureNamedOptions(string name, Action<TOptions> action)
{
Name = name;
Action = action;
}

/// <summary>
/// The options name.
/// </summary>
public string Name { get; }

/// <summary>
/// The configuration action.
/// </summary>
public Action<TOptions> Action { get; }

/// <summary>
/// Invokes the registered configure Action if the name matches.
/// </summary>
/// <param name="name"></param>
/// <param name="options"></param>
public virtual void Configure(string name, TOptions options)
{
if (options == null)
{
throw new ArgumentNullException(nameof(options));
}

// Null name is used to configure all named options.
if (Name == null || name == Name)
{
Action?.Invoke(options);
}
}
}
}
20 changes: 20 additions & 0 deletions src/Microsoft.Extensions.Options/IConfigureNamedOptions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.Extensions.Options
{

/// <summary>
/// Represents something that configures the TOptions type.
/// </summary>
/// <typeparam name="TOptions"></typeparam>
public interface IConfigureNamedOptions<in TOptions> where TOptions : class

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With name = null this will be the same like IConfigureOptions. Do we really need both of these interfaces?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to leave the original interfaces and functionality alone so existing usage of IOptions is completely unchanged. So basically you can think of this is as the IConfigureOptions2 interface. And OptionsFactory2 as IOptions2. So we need to decide how the new stuff interacts with the old (unnamed options). But for now the old stuff completely ignores any of the new apis (expect potentially with ConfigureAll which probably should result in configuring the 'default' option as well.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that if all old stuff will use all new new stuff with 'null' name - it will work as expected. Without any problems (except that validation might be applied). The one change is inject factory into OptionsManager and ask for unnamed options.

Am I missing something?
Or the original stuff is left just in case some unexpected issues.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default instances are only targeted with the old interface. The null name is used to signify special configures that are applied to every named instances.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The null Name is just saving me from having to creating a second ConfigureAllNamedOptions class that eliminates the Name check.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old API's will do (behave) the same, but only in a bit different way. End users won't notice the different, but they will know that named and unnamed options work alike.

Yes, I can write a helper method that will call Configure for each name that I need (in cycle call Configure for each name with the same action). But having something like: Configure(["name1", "name2"], o => DoStuff()) could help reduce the count of total IConfigureOptions instances (and because we iterate through all of them - potentially reduce the time).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its easy enough to add sugar that supports configuring multiple names at once in a single IConfigureNamedOptions instance.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not just sugar, but it requires to change the interface IConfigureNamedOptions and replace string Name with string[] Names (and change the realization for Configure method a bit).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its just sugar, the interface shouldn't actually have Name on it, that's a mistake I'll remedy, the name is supplied via Configure/Validate the property is something specific to the implementation, so it should be possible to have a IConfigureNamedOptions that handles as many different names as it likes, it is free to do whatever via its Validate since all of them will be called.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name is now removed on the interface

{
/// <summary>
/// Invoked to configure a TOptions instance.
/// </summary>
/// <param name="name">The name of the options instance being configured.</param>
/// <param name="options">The options instance to configure.</param>
void Configure(string name, TOptions options);
}
}
22 changes: 22 additions & 0 deletions src/Microsoft.Extensions.Options/IOptionsCache.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;

namespace Microsoft.Extensions.Options
{
/// <summary>
/// Used to cache TOptions instances.
/// </summary>
/// <typeparam name="TOptions">The type of options being requested.</typeparam>
public interface IOptionsCache<TOptions> where TOptions : class
{
TOptions GetOrAdd(string name, Func<TOptions> createOptions);

bool TryAdd(string name, TOptions options);

bool TryRemove(string name);

// Do we need a Clear all?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point it might be required.

I also was thinking about using some kind of IEvictionPolicy and TryAdd(TOptions, IEvictionPolicy<TOptions>). In that case we might add ClearEvicted method (perhaps we no need it at all (or no need right now), but just like a thing to consider).

}
}
17 changes: 17 additions & 0 deletions src/Microsoft.Extensions.Options/IOptionsFactory.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.Extensions.Options
{
/// <summary>
/// Used to create TOptions instances.
/// </summary>
/// <typeparam name="TOptions">The type of options being requested.</typeparam>
public interface IOptionsFactory<TOptions> where TOptions : class, new()
{
/// <summary>
/// Returns a configured TOptions instance with the given name.
/// </summary>
TOptions Create(string name);
}
}
21 changes: 21 additions & 0 deletions src/Microsoft.Extensions.Options/IOptionsService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.Extensions.Options
{
/// <summary>
/// Used to retreive configured and validated TOptions instances.
/// </summary>
/// <typeparam name="TOptions">The type of options being requested.</typeparam>
public interface IOptionsService<TOptions> where TOptions : class, new()
{
/// <summary>
/// Returns a configured and validated TOptions instance with the given name.
/// </summary>
TOptions Get(string name);

void Add(string name, TOptions options);

bool Remove(string name);
}
}
17 changes: 17 additions & 0 deletions src/Microsoft.Extensions.Options/IOptionsValidator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.Extensions.Options
{
/// <summary>
/// Used to validate TOptions instances.
/// </summary>
/// <typeparam name="TOptions">The type of options being requested.</typeparam>
public interface IOptionsValidator<TOptions> where TOptions : class, new()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now that's fine.

According my PR (#171) there are some changes that might be required to improve validation feature. But it definitely might be applied over this PR, so that's ok.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah so the basic idea is to make the signature of the most basic validation pieces flexible enough so more complex validation can be layered on, but keeping what's in core as simple as possible.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that core should be not only simple, but flexible as well in order to build complex validation based on it. Because if it will be only simple then it will become the point that needs to be refactored for enabling different validation scenarios.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but is there something that you have in mind that we can't accomplish with an Action<TOptions> as a starting point for all validation logic.

Stuff like aggregation of exceptions across multiple validations can be an implementation detail of the IOptionsValidator, but I think its reasonable for the validator to be responsible for generating a proper exception explaining why validation failed.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that having aggregated validation result is better. Because if you have a few validators and each will throw an exception, it will require you to run validation a few time to fix all errors (because information about new exceptions you will see only when you fixed previous error).

The same scenario with global validation of options at start up time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A validator implementation can choose to aggregate the results if it wants by wrapping each action in a try/catch, so its not an issue.

{
/// <summary>
/// Validates the options instance.
/// </summary>
void Validate(string name, TOptions options);
}
}
25 changes: 25 additions & 0 deletions src/Microsoft.Extensions.Options/IValidateNamedOptions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.


namespace Microsoft.Extensions.Options
{
/// <summary>
/// Represents something that validate the TOptions type.
/// </summary>
/// <typeparam name="TOptions">The type of options being requested.</typeparam>
public interface IValidateNamedOptions<in TOptions> where TOptions : class
{
/// <summary>
/// The name of the options instance to validate.
/// </summary>
string Name { get; }

/// <summary>
/// Invoked to validate a TOptions instance.
/// </summary>
/// <param name="name">The name of the options instance being validated.</param>
/// <param name="options">The options instance to validate.</param>
void Validate(string name, TOptions options);
}
}
49 changes: 49 additions & 0 deletions src/Microsoft.Extensions.Options/LegacyOptionsCache.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Threading;

namespace Microsoft.Extensions.Options
{
internal class LegacyOptionsCache<TOptions> where TOptions : class, new()
{
private readonly Func<TOptions> _createCache;
private object _cacheLock = new object();
private bool _cacheInitialized;
private TOptions _options;
private IEnumerable<IConfigureOptions<TOptions>> _setups;

public LegacyOptionsCache(IEnumerable<IConfigureOptions<TOptions>> setups)
{
_setups = setups;
_createCache = CreateOptions;
}

private TOptions CreateOptions()
{
var result = new TOptions();
if (_setups != null)
{
foreach (var setup in _setups)
{
setup.Configure(result);
}
}
return result;
}

public virtual TOptions Value
{
get
{
return LazyInitializer.EnsureInitialized(
ref _options,
ref _cacheInitialized,
ref _cacheLock,
_createCache);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

<PropertyGroup>
<Description>Provides a strongly typed way of specifying and accessing settings using dependency injection.</Description>
<TargetFramework>netstandard1.0</TargetFramework>
<TargetFramework>netstandard1.1</TargetFramework>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needed to bump to netstandard 1.1 to use ConcurrentDictionary

<NoWarn>$(NoWarn);CS1591</NoWarn>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<PackageTags>aspnetcore;options</PackageTags>
Expand All @@ -13,7 +13,6 @@
<ItemGroup>
<PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="$(AspNetCoreVersion)" />
<PackageReference Include="Microsoft.Extensions.Primitives" Version="$(AspNetCoreVersion)" />
<PackageReference Include="System.ComponentModel" Version="$(CoreFxVersion)" />
</ItemGroup>
<PackageReference Include="System.ComponentModel" Version="$(CoreFxVersion)" /> </ItemGroup>

</Project>
70 changes: 43 additions & 27 deletions src/Microsoft.Extensions.Options/OptionsCache.cs
Original file line number Diff line number Diff line change
@@ -1,49 +1,65 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Threading;
using System.Collections.Concurrent;

namespace Microsoft.Extensions.Options
{
internal class OptionsCache<TOptions> where TOptions : class, new()
/// <summary>
/// Used to cache TOptions instances.
/// </summary>
/// <typeparam name="TOptions">The type of options being requested.</typeparam>
public class OptionsCache<TOptions> : IOptionsCache<TOptions> where TOptions : class
{
private readonly Func<TOptions> _createCache;
private object _cacheLock = new object();
private bool _cacheInitialized;
private TOptions _options;
private IEnumerable<IConfigureOptions<TOptions>> _setups;
private readonly ConcurrentDictionary<string, Lazy<TOptions>> _cache = new ConcurrentDictionary<string, Lazy<TOptions>>(StringComparer.Ordinal);

public OptionsCache(IEnumerable<IConfigureOptions<TOptions>> setups)
public virtual TOptions GetOrAdd(string name, Func<TOptions> createOptions)
{
_setups = setups;
_createCache = CreateOptions;
if (name == null)
{
throw new ArgumentNullException(nameof(name));
}
if (createOptions == null)
{
throw new ArgumentNullException(nameof(createOptions));
}
return _cache.GetOrAdd(name, new Lazy<TOptions>(createOptions)).Value;
}

private TOptions CreateOptions()
/// <summary>
/// Tries to adds a new option to the cache, will return false if the name already exists.
/// </summary>
/// <param name="name">The name of the options instance.</param>
/// <param name="options">The options instance.</param>
/// <returns>Whether anything was added.</returns>
public virtual bool TryAdd(string name, TOptions options)
{
var result = new TOptions();
if (_setups != null)
if (name == null)
{
foreach (var setup in _setups)
{
setup.Configure(result);
}
throw new ArgumentNullException(nameof(name));
}
return result;
if (options == null)
{
throw new ArgumentNullException(nameof(options));
}
return _cache.TryAdd(name, new Lazy<TOptions>(() => options));
}

public virtual TOptions Value
/// <summary>
/// Try to remove an options instance.
/// </summary>
/// <param name="name">The name of the options instance.</param>
/// <returns>Whether anything was removed.</returns>
public virtual bool TryRemove(string name)
{
get
if (name == null)
{
return LazyInitializer.EnsureInitialized(
ref _options,
ref _cacheInitialized,
ref _cacheLock,
_createCache);
throw new ArgumentNullException(nameof(name));
}
return _cache.TryRemove(name, out var ignored);
}

// Do we need a Clear all?
}
}
35 changes: 35 additions & 0 deletions src/Microsoft.Extensions.Options/OptionsFactory.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Generic;

namespace Microsoft.Extensions.Options
{
/// <summary>
/// Implementation of IOptionsFactory.
/// </summary>
/// <typeparam name="TOptions">The type of options being requested.</typeparam>
public class OptionsFactory<TOptions> : IOptionsFactory<TOptions> where TOptions : class, new()
{
private readonly IEnumerable<IConfigureNamedOptions<TOptions>> _setups;

/// <summary>
/// Initializes a new instance with the specified options configurations.
/// </summary>
/// <param name="setups">The configuration actions to run.</param>
public OptionsFactory(IEnumerable<IConfigureNamedOptions<TOptions>> setups)
{
_setups = setups;
}

public TOptions Create(string name)
{
var options = new TOptions();
foreach (var setup in _setups)
{
setup.Configure(name, options);
}
return options;
}
}
}
Loading