From d332fdf9b83d655a2d79ded7420314659355c71b Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 31 Jan 2017 13:30:30 -0800 Subject: [PATCH 1/3] OptionsSnapshot always created per scope --- .../OptionsSnapshot.cs | 25 +--- .../FakeChangeToken.cs | 31 ++++ .../OptionsMonitorTest.cs | 74 +--------- .../OptionsSnapshotTest.cs | 136 ++++++++++++++++++ 4 files changed, 174 insertions(+), 92 deletions(-) create mode 100644 test/Microsoft.Extensions.Options.Test/FakeChangeToken.cs create mode 100644 test/Microsoft.Extensions.Options.Test/OptionsSnapshotTest.cs diff --git a/src/Microsoft.Extensions.Options/OptionsSnapshot.cs b/src/Microsoft.Extensions.Options/OptionsSnapshot.cs index dc56683..225b7f8 100644 --- a/src/Microsoft.Extensions.Options/OptionsSnapshot.cs +++ b/src/Microsoft.Extensions.Options/OptionsSnapshot.cs @@ -1,34 +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. +using System.Collections.Generic; + namespace Microsoft.Extensions.Options { /// /// Implementation of IOptionsSnapshot. /// /// - public class OptionsSnapshot : IOptionsSnapshot where TOptions : class, new() + public class OptionsSnapshot : OptionsManager, IOptionsSnapshot where TOptions : class, new() { - private TOptions _options; - /// /// Initializes a new instance. /// - /// The monitor to fetch the options value from. - public OptionsSnapshot(IOptionsMonitor monitor) - { - _options = monitor.CurrentValue; - } - - /// - /// The configured options instance. - /// - public virtual TOptions Value - { - get - { - return _options; - } - } + /// The configuration actions to run. + public OptionsSnapshot(IEnumerable> setups) : base(setups) + { } } } \ No newline at end of file diff --git a/test/Microsoft.Extensions.Options.Test/FakeChangeToken.cs b/test/Microsoft.Extensions.Options.Test/FakeChangeToken.cs new file mode 100644 index 0000000..45688a6 --- /dev/null +++ b/test/Microsoft.Extensions.Options.Test/FakeChangeToken.cs @@ -0,0 +1,31 @@ +// 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 Microsoft.Extensions.Primitives; + +namespace Microsoft.Extensions.Options.Tests +{ + public class FakeChangeToken : IChangeToken, IDisposable + { + public bool ActiveChangeCallbacks { get; set; } + public bool HasChanged { get; set; } + public IDisposable RegisterChangeCallback(Action callback, object state) + { + _callback = () => callback(state); + return this; + } + + public void InvokeChangeCallback() + { + _callback?.Invoke(); + } + + public void Dispose() + { + _callback = null; + } + + private Action _callback; + } +} diff --git a/test/Microsoft.Extensions.Options.Test/OptionsMonitorTest.cs b/test/Microsoft.Extensions.Options.Test/OptionsMonitorTest.cs index 18dcfad..1318df1 100644 --- a/test/Microsoft.Extensions.Options.Test/OptionsMonitorTest.cs +++ b/test/Microsoft.Extensions.Options.Test/OptionsMonitorTest.cs @@ -13,33 +13,7 @@ public class OptionsMonitorTest { public int SetupInvokeCount { get; set; } - public class FakeChangeToken : IChangeToken, IDisposable - { - public bool ActiveChangeCallbacks { get; set; } - public bool HasChanged { get; set; } - public IDisposable RegisterChangeCallback(Action callback, object state) - { - _callback = () => callback(state); - return this; - } - - public void InvokeChangeCallback() - { - if (_callback != null) - { - _callback(); - } - } - - public void Dispose() - { - _callback = null; - } - - private Action _callback; - } - - public class CountIncrement : IConfigureOptions + private class CountIncrement : IConfigureOptions { private OptionsMonitorTest _test; @@ -55,7 +29,6 @@ public void Configure(FakeOptions options) } } - public class FakeSource : IOptionsChangeTokenSource { public FakeSource(FakeChangeToken token) @@ -248,18 +221,6 @@ public void Dispose() public string Message => _options?.Message; } - public class ControllerWithSnapshot - { - FakeOptions _options; - - public ControllerWithSnapshot(IOptionsSnapshot snap) - { - _options = snap.Value; - } - - public string Message => _options?.Message; - } - [Fact] public void ControllerCanWatchOptionsThatTrackConfigChanges() { @@ -281,38 +242,5 @@ public void ControllerCanWatchOptionsThatTrackConfigChanges() config.Reload(); Assert.Equal("2", controller.Message); } - - [Fact] - public void SnapshotOptionsDoNotChangeEvenWhenMonitorChanges() - { - var config = new ConfigurationBuilder().AddInMemoryCollection().Build(); - - var services = new ServiceCollection().AddOptions(); - services.AddSingleton>(new CountIncrement(this)); - services.Configure(config); - - var sp = services.BuildServiceProvider(); - - var monitor = sp.GetRequiredService>(); - var snapshot = sp.GetRequiredService>(); - - var options = monitor.CurrentValue; - Assert.Equal("1", options.Message); - Assert.Equal(options, snapshot.Value); - - var token = config.GetReloadToken(); - - config.Reload(); - - Assert.NotEqual(monitor.CurrentValue, snapshot.Value); - Assert.Equal("2", monitor.CurrentValue.Message); - Assert.Equal("1", snapshot.Value.Message); - - config.Reload(); - - Assert.NotEqual(monitor.CurrentValue, snapshot.Value); - Assert.Equal("3", monitor.CurrentValue.Message); - Assert.Equal("1", snapshot.Value.Message); - } } } \ No newline at end of file diff --git a/test/Microsoft.Extensions.Options.Test/OptionsSnapshotTest.cs b/test/Microsoft.Extensions.Options.Test/OptionsSnapshotTest.cs new file mode 100644 index 0000000..79ad7c8 --- /dev/null +++ b/test/Microsoft.Extensions.Options.Test/OptionsSnapshotTest.cs @@ -0,0 +1,136 @@ +// 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 Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Primitives; +using Xunit; + +namespace Microsoft.Extensions.Options.Tests +{ + public class OptionsSnapshotTest + { + public int SetupInvokeCount { get; set; } + + private class CountIncrement : IConfigureOptions + { + private OptionsSnapshotTest _test; + + public CountIncrement(OptionsSnapshotTest test) + { + _test = test; + } + + public void Configure(FakeOptions options) + { + _test.SetupInvokeCount++; + options.Message += _test.SetupInvokeCount; + } + } + + + public class FakeSource : IOptionsChangeTokenSource + { + public FakeSource(FakeChangeToken token) + { + Token = token; + } + + public FakeChangeToken Token { get; set; } + + public IChangeToken GetChangeToken() + { + return Token; + } + + public void Changed() + { + Token.HasChanged = true; + Token.InvokeChangeCallback(); + } + } + + public class ControllerWithSnapshot + { + FakeOptions _options; + + public ControllerWithSnapshot(IOptionsSnapshot snap) + { + _options = snap.Value; + } + + public string Message => _options?.Message; + } + + [Fact] + public void SnapshotOptionsDoNotChangeEvenWhenMonitorChanges() + { + var config = new ConfigurationBuilder().AddInMemoryCollection().Build(); + + var services = new ServiceCollection().AddOptions(); + services.AddSingleton>(new CountIncrement(this)); + services.Configure(config); + + var sp = services.BuildServiceProvider(); + + var monitor = sp.GetRequiredService>(); + var snapshot = sp.GetRequiredService>(); + + var options = monitor.CurrentValue; + Assert.Equal("1", options.Message); + Assert.Equal("2", snapshot.Value.Message); + Assert.NotEqual(options, snapshot.Value); + + var token = config.GetReloadToken(); + + config.Reload(); + + Assert.NotEqual(monitor.CurrentValue, snapshot.Value); + Assert.Equal("3", monitor.CurrentValue.Message); + Assert.Equal("2", snapshot.Value.Message); + + config.Reload(); + + Assert.NotEqual(monitor.CurrentValue, snapshot.Value); + Assert.Equal("4", monitor.CurrentValue.Message); + Assert.Equal("2", snapshot.Value.Message); + } + + private class TestConfigure : IConfigureOptions + { + public static int ConfigureCount; + + public void Configure(FakeOptions options) + { + ConfigureCount++; + } + } + + + [Fact] + public void SnapshotOptionsAreRecreatedPerScope() + { + var services = new ServiceCollection() + .AddOptions() + .AddSingleton, TestConfigure>() + .BuildServiceProvider(); + + var factory = services.GetRequiredService(); + FakeOptions options = null; + using (var scope = factory.CreateScope()) + { + options = scope.ServiceProvider.GetRequiredService>().Value; + Assert.Equal(options, scope.ServiceProvider.GetRequiredService>().Value); + } + Assert.Equal(1, TestConfigure.ConfigureCount); + using (var scope = factory.CreateScope()) + { + var options2 = scope.ServiceProvider.GetRequiredService>().Value; + Assert.Equal(options2, scope.ServiceProvider.GetRequiredService>().Value); + Assert.NotEqual(options, options2); + } + Assert.Equal(2, TestConfigure.ConfigureCount); + } + } +} From f0be8e9c055b3fc891dd331d2a3ae5fe4b6e00af Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 31 Jan 2017 13:34:52 -0800 Subject: [PATCH 2/3] Tweak test to scoped IConfigureOptions --- .../OptionsSnapshotTest.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/Microsoft.Extensions.Options.Test/OptionsSnapshotTest.cs b/test/Microsoft.Extensions.Options.Test/OptionsSnapshotTest.cs index 79ad7c8..4c88dec 100644 --- a/test/Microsoft.Extensions.Options.Test/OptionsSnapshotTest.cs +++ b/test/Microsoft.Extensions.Options.Test/OptionsSnapshotTest.cs @@ -101,10 +101,14 @@ private class TestConfigure : IConfigureOptions { public static int ConfigureCount; - public void Configure(FakeOptions options) + public TestConfigure() { ConfigureCount++; } + + public void Configure(FakeOptions options) + { + } } @@ -113,7 +117,7 @@ public void SnapshotOptionsAreRecreatedPerScope() { var services = new ServiceCollection() .AddOptions() - .AddSingleton, TestConfigure>() + .AddScoped, TestConfigure>() .BuildServiceProvider(); var factory = services.GetRequiredService(); From 55f7c89ecac3744233dd2908bef7a1e8311c4daf Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Fri, 10 Feb 2017 15:00:44 -0800 Subject: [PATCH 3/3] OptionsManager implements Snapshot as well --- .../OptionsManager.cs | 2 +- .../OptionsServiceCollectionExtensions.cs | 2 +- .../OptionsSnapshot.cs | 21 ------------------- 3 files changed, 2 insertions(+), 23 deletions(-) delete mode 100644 src/Microsoft.Extensions.Options/OptionsSnapshot.cs diff --git a/src/Microsoft.Extensions.Options/OptionsManager.cs b/src/Microsoft.Extensions.Options/OptionsManager.cs index b651b74..a0ee9fb 100644 --- a/src/Microsoft.Extensions.Options/OptionsManager.cs +++ b/src/Microsoft.Extensions.Options/OptionsManager.cs @@ -9,7 +9,7 @@ namespace Microsoft.Extensions.Options /// Implementation of IOptions. /// /// - public class OptionsManager : IOptions where TOptions : class, new() + public class OptionsManager : IOptions, IOptionsSnapshot where TOptions : class, new() { private OptionsCache _optionsCache; diff --git a/src/Microsoft.Extensions.Options/OptionsServiceCollectionExtensions.cs b/src/Microsoft.Extensions.Options/OptionsServiceCollectionExtensions.cs index 2422253..aa7343a 100644 --- a/src/Microsoft.Extensions.Options/OptionsServiceCollectionExtensions.cs +++ b/src/Microsoft.Extensions.Options/OptionsServiceCollectionExtensions.cs @@ -25,8 +25,8 @@ public static IServiceCollection AddOptions(this IServiceCollection services) } services.TryAdd(ServiceDescriptor.Singleton(typeof(IOptions<>), typeof(OptionsManager<>))); + services.TryAdd(ServiceDescriptor.Scoped(typeof(IOptionsSnapshot<>), typeof(OptionsManager<>))); services.TryAdd(ServiceDescriptor.Singleton(typeof(IOptionsMonitor<>), typeof(OptionsMonitor<>))); - services.TryAdd(ServiceDescriptor.Scoped(typeof(IOptionsSnapshot<>), typeof(OptionsSnapshot<>))); return services; } diff --git a/src/Microsoft.Extensions.Options/OptionsSnapshot.cs b/src/Microsoft.Extensions.Options/OptionsSnapshot.cs deleted file mode 100644 index 225b7f8..0000000 --- a/src/Microsoft.Extensions.Options/OptionsSnapshot.cs +++ /dev/null @@ -1,21 +0,0 @@ -// 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 -{ - /// - /// Implementation of IOptionsSnapshot. - /// - /// - public class OptionsSnapshot : OptionsManager, IOptionsSnapshot where TOptions : class, new() - { - /// - /// Initializes a new instance. - /// - /// The configuration actions to run. - public OptionsSnapshot(IEnumerable> setups) : base(setups) - { } - } -} \ No newline at end of file