Revert "Change IOptionsSnapshot to reuse options instances across scopes (#56271...

* Revert "Change IOptionsSnapshot to reuse options instances across scopes (#56271)"

This reverts commit 8f5f9d049a6a98b138f88fa1d9d6a96c40c03aa7.

* Add a test to ensure ASP.NET's scenario isn't broken again

diff --git a/src/libraries/Microsoft.Extensions.Options/src/OptionsServiceCollectionExtensions.cs b/src/libraries/Microsoft.Extensions.Options/src/OptionsServiceCollectionExtensions.cs
index 3d89c86..a84cbe1 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Options/src/OptionsServiceCollectionExtensions.cs
+++ b/src/libraries/Microsoft.Extensions.Options/src/OptionsServiceCollectionExtensions.cs
@@ -27,7 +27,7 @@ namespace Microsoft.Extensions.DependencyInjection
             }
 
             services.TryAdd(ServiceDescriptor.Singleton(typeof(IOptions<>), typeof(UnnamedOptionsManager<>)));
-            services.TryAdd(ServiceDescriptor.Scoped(typeof(IOptionsSnapshot<>), typeof(OptionsSnapshot<>)));
+            services.TryAdd(ServiceDescriptor.Scoped(typeof(IOptionsSnapshot<>), typeof(OptionsManager<>)));
             services.TryAdd(ServiceDescriptor.Singleton(typeof(IOptionsMonitor<>), typeof(OptionsMonitor<>)));
             services.TryAdd(ServiceDescriptor.Transient(typeof(IOptionsFactory<>), typeof(OptionsFactory<>)));
             services.TryAdd(ServiceDescriptor.Singleton(typeof(IOptionsMonitorCache<>), typeof(OptionsCache<>)));

diff --git a/src/libraries/Microsoft.Extensions.Options/src/OptionsSnapshot.cs b/src/libraries/Microsoft.Extensions.Options/src/OptionsSnapshot.cs
deleted file mode 100644 (file)
index 197d97c..0000000
--- a/src/libraries/Microsoft.Extensions.Options/src/OptionsSnapshot.cs
+++ /dev/null
@@ -1,66 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-
-using System;
-using System.Collections.Concurrent;
-using System.Diagnostics.CodeAnalysis;
-using System.Threading;
-
-namespace Microsoft.Extensions.Options
-{
-    /// <summary>
-    /// Implementation of <see cref="IOptionsSnapshot{TOptions}"/>.
-    /// </summary>
-    /// <typeparam name="TOptions">Options type.</typeparam>
-    internal sealed class OptionsSnapshot<[DynamicallyAccessedMembers(Options.DynamicallyAccessedMembers)] TOptions> :
-        IOptionsSnapshot<TOptions>
-        where TOptions : class
-    {
-        private readonly IOptionsMonitor<TOptions> _optionsMonitor;
-
-        private volatile ConcurrentDictionary<string, TOptions> _cache;
-        private volatile TOptions _unnamedOptionsValue;
-
-        /// <summary>
-        /// Initializes a new instance with the specified options configurations.
-        /// </summary>
-        /// <param name="optionsMonitor">The options monitor to use to provide options.</param>
-        public OptionsSnapshot(IOptionsMonitor<TOptions> optionsMonitor)
-        {
-            _optionsMonitor = optionsMonitor;
-        }
-
-        /// <summary>
-        /// The default configured <typeparamref name="TOptions"/> instance, equivalent to Get(Options.DefaultName).
-        /// </summary>
-        public TOptions Value => Get(Options.DefaultName);
-
-        /// <summary>
-        /// Returns a configured <typeparamref name="TOptions"/> instance with the given <paramref name="name"/>.
-        /// </summary>
-        public TOptions Get(string name)
-        {
-            if (name == null || name == Options.DefaultName)
-            {
-                if (_unnamedOptionsValue is TOptions value)
-                {
-                    return value;
-                }
-
-                return _unnamedOptionsValue = _optionsMonitor.Get(Options.DefaultName);
-            }
-
-            var cache = _cache ?? Interlocked.CompareExchange(ref _cache, new(concurrencyLevel: 1, capacity: 5, StringComparer.Ordinal), null) ?? _cache;
-
-#if NETSTANDARD2_1
-            TOptions options = cache.GetOrAdd(name, static (name, optionsMonitor) => optionsMonitor.Get(name), _optionsMonitor);
-#else
-            if (!cache.TryGetValue(name, out TOptions options))
-            {
-                options = cache.GetOrAdd(name, _optionsMonitor.Get(name));
-            }
-#endif
-            return options;
-        }
-    }
-}

diff --git a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsSnapshotTest.cs b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsSnapshotTest.cs
index 2839069..534700e 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsSnapshotTest.cs
+++ b/src/libraries/Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsSnapshotTest.cs
@@ -5,6 +5,7 @@ using System;
 using System.Linq;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.DependencyInjection.Extensions;
 using Microsoft.Extensions.Primitives;
 using Xunit;
 
@@ -131,14 +132,12 @@ namespace Microsoft.Extensions.Options.Tests
         [Fact]
         public void SnapshotOptionsAreCachedPerScope()
         {
-            var config = new ConfigurationBuilder().AddInMemoryCollection().Build();
             var services = new ServiceCollection()
                 .AddOptions()
-                .AddSingleton<IConfigureOptions<FakeOptions>, TestConfigure>()
-                .AddSingleton<IOptionsChangeTokenSource<FakeOptions>>(new ConfigurationChangeTokenSource<FakeOptions>(Options.DefaultName, config))
-                .AddSingleton<IOptionsChangeTokenSource<FakeOptions>>(new ConfigurationChangeTokenSource<FakeOptions>("1", config))
+                .AddScoped<IConfigureOptions<FakeOptions>, TestConfigure>()
                 .BuildServiceProvider();
 
+            var cache = services.GetRequiredService<IOptionsMonitorCache<FakeOptions>>();
             var factory = services.GetRequiredService<IServiceScopeFactory>();
             FakeOptions options = null;
             FakeOptions namedOne = null;
@@ -150,30 +149,18 @@ namespace Microsoft.Extensions.Options.Tests
                 namedOne = scope.ServiceProvider.GetRequiredService<IOptionsSnapshot<FakeOptions>>().Get("1");
                 Assert.Equal(namedOne, scope.ServiceProvider.GetRequiredService<IOptionsSnapshot<FakeOptions>>().Get("1"));
                 Assert.Equal(2, TestConfigure.ConfigureCount);
-
-                // Reload triggers Configure for the two registered change tokens.
-                config.Reload();
-                Assert.Equal(4, TestConfigure.ConfigureCount);
-
-                // Reload should not affect current scope.
-                var options2 = scope.ServiceProvider.GetRequiredService<IOptionsSnapshot<FakeOptions>>().Value;
-                Assert.Equal(options, options2);
-                var namedOne2 = scope.ServiceProvider.GetRequiredService<IOptionsSnapshot<FakeOptions>>().Get("1");
-                Assert.Equal(namedOne, namedOne2);
             }
             Assert.Equal(1, TestConfigure.CtorCount);
-
-            // Reload should be reflected in a fresh scope.
             using (var scope = factory.CreateScope())
             {
                 var options2 = scope.ServiceProvider.GetRequiredService<IOptionsSnapshot<FakeOptions>>().Value;
                 Assert.NotEqual(options, options2);
-                Assert.Equal(4, TestConfigure.ConfigureCount);
+                Assert.Equal(3, TestConfigure.ConfigureCount);
                 var namedOne2 = scope.ServiceProvider.GetRequiredService<IOptionsSnapshot<FakeOptions>>().Get("1");
-                Assert.NotEqual(namedOne, namedOne2);
+                Assert.NotEqual(namedOne2, namedOne);
                 Assert.Equal(4, TestConfigure.ConfigureCount);
             }
-            Assert.Equal(1, TestConfigure.CtorCount);
+            Assert.Equal(2, TestConfigure.CtorCount);
         }
 
         [Fact]
@@ -213,5 +200,47 @@ namespace Microsoft.Extensions.Options.Tests
         {
             Assert.NotNull(services.Where(s => s.ServiceType == serviceType && s.Lifetime == lifetime).SingleOrDefault());
         }
+
+        /// <summary>
+        /// Duplicates an aspnetcore test to ensure when an IOptionsSnapshot is resolved both in
+        /// the root scope and a created scope, that dependent services are created both times.
+        /// </summary>
+        [Fact]
+        public void RecreateAspNetCore_AddOidc_CustomStateAndAccount_SetsUpConfiguration()
+        {
+            var services = new ServiceCollection().AddOptions();
+
+            int calls = 0;
+
+            services.TryAddEnumerable(ServiceDescriptor.Scoped<IPostConfigureOptions<RemoteAuthenticationOptions<OidcProviderOptions>>, DefaultOidcOptionsConfiguration>());
+            services.Replace(ServiceDescriptor.Scoped(typeof(NavigationManager), _ =>
+            {
+                calls++;
+                return new NavigationManager();
+            }));
+
+            using ServiceProvider provider = services.BuildServiceProvider();
+
+            using IServiceScope scope = provider.CreateScope();
+
+            // from the root scope.
+            var rootOptions = provider.GetRequiredService<IOptionsSnapshot<RemoteAuthenticationOptions<OidcProviderOptions>>>();
+
+            // from the created scope
+            var scopedOptions = scope.ServiceProvider.GetRequiredService<IOptionsSnapshot<RemoteAuthenticationOptions<OidcProviderOptions>>>();
+
+            // we should have 2 navigation managers. One in the root scope, and one in the created scope.
+            Assert.Equal(2, calls);
+        }
+
+        private class OidcProviderOptions { }
+        private class RemoteAuthenticationOptions<TRemoteAuthenticationProviderOptions> where TRemoteAuthenticationProviderOptions : new() { }
+        private class NavigationManager { }
+
+        private class DefaultOidcOptionsConfiguration : IPostConfigureOptions<RemoteAuthenticationOptions<OidcProviderOptions>>
+        {
+            public DefaultOidcOptionsConfiguration(NavigationManager navigationManager) { }
+            public void PostConfigure(string name, RemoteAuthenticationOptions<OidcProviderOptions> options) { }
+        }
     }
 }