Skip to content

Commit

Permalink
Revert "Change IOptionsSnapshot to reuse options instances across sco…
Browse files Browse the repository at this point in the history
…pes (#56271)" (#57570)

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

This reverts commit 8f5f9d0.

* Add a test to ensure ASP.NET's scenario isn't broken again
  • Loading branch information
eerhardt authored Aug 17, 2021
1 parent 04b6370 commit f21b2e3
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 86 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public static IServiceCollection AddOptions(this IServiceCollection services)
}

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<>)));
Expand Down
66 changes: 0 additions & 66 deletions src/libraries/Microsoft.Extensions.Options/src/OptionsSnapshot.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Linq;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Microsoft.Extensions.Primitives;
using Xunit;

Expand Down Expand Up @@ -131,14 +132,12 @@ public void Configure(string name, FakeOptions options)
[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;
Expand All @@ -150,30 +149,18 @@ public void SnapshotOptionsAreCachedPerScope()
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]
Expand Down Expand Up @@ -213,5 +200,47 @@ private void CheckLifetime(IServiceCollection services, Type serviceType, Servic
{
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) { }
}
}
}

0 comments on commit f21b2e3

Please sign in to comment.