Skip to content

Commit

Permalink
Preserve extension registration order
Browse files Browse the repository at this point in the history 
Fixes #26071
  • Loading branch information
@AndriySvyryd
AndriySvyryd committed Sep 22, 2021
1 parent afaeff0 commit eb29ef3
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 19 deletions.
62 changes: 47 additions & 15 deletions src/EFCore/DbContextOptions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,27 +22,56 @@ namespace Microsoft.EntityFrameworkCore
/// </remarks>
public abstract class DbContextOptions : IDbContextOptions
{
private readonly ImmutableSortedDictionary<Type, (IDbContextOptionsExtension Extension, int Ordinal)> _extensionsMap;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
[EntityFrameworkInternal]
protected DbContextOptions()
{
_extensionsMap = ImmutableSortedDictionary.Create<Type, (IDbContextOptionsExtension, int)>(TypeFullNameComparer.Instance);
}

/// <summary>
/// Initializes a new instance of the <see cref="DbContextOptions" /> class. You normally override
/// <see cref="DbContext.OnConfiguring(DbContextOptionsBuilder)" /> or use a <see cref="DbContextOptionsBuilder" />
/// to create instances of this class and it is not designed to be directly constructed in your application code.
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
/// <param name="extensions"> The extensions that store the configured options. </param>
[EntityFrameworkInternal]
protected DbContextOptions(
IReadOnlyDictionary<Type, IDbContextOptionsExtension> extensions)
{
Check.NotNull(extensions, nameof(extensions));

_extensionsMap = extensions as ImmutableSortedDictionary<Type, IDbContextOptionsExtension>
?? ImmutableSortedDictionary.Create<Type, IDbContextOptionsExtension>(TypeFullNameComparer.Instance)
.AddRange(extensions);
_extensionsMap = ImmutableSortedDictionary.Create<Type, (IDbContextOptionsExtension, int)>(TypeFullNameComparer.Instance)
.AddRange(extensions.Select((p, i) => new KeyValuePair<Type, (IDbContextOptionsExtension, int)>(p.Key, (p.Value, i))));
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
[EntityFrameworkInternal]
protected DbContextOptions(
ImmutableSortedDictionary<Type, (IDbContextOptionsExtension Extension, int Ordinal)> extensions)
{
Check.NotNull(extensions, nameof(extensions));

_extensionsMap = extensions;
}

/// <summary>
/// Gets the extensions that store the configured options.
/// </summary>
public virtual IEnumerable<IDbContextOptionsExtension> Extensions
=> ExtensionsMap.Values;
=> _extensionsMap.Values.OrderBy(v => v.Ordinal).Select(v => v.Extension);

/// <summary>
/// Gets the extension of the specified type. Returns <see langword="null" /> if no extension of the specified type is configured.
Expand All @@ -51,7 +80,7 @@ public virtual IEnumerable<IDbContextOptionsExtension> Extensions
/// <returns> The extension, or <see langword="null" /> if none was found. </returns>
public virtual TExtension? FindExtension<TExtension>()
where TExtension : class, IDbContextOptionsExtension
=> ExtensionsMap.TryGetValue(typeof(TExtension), out var extension) ? (TExtension)extension : null;
=> _extensionsMap.TryGetValue(typeof(TExtension), out var value) ? (TExtension)value.Extension : null;

/// <summary>
/// Gets the extension of the specified type. Throws if no extension of the specified type is configured.
Expand Down Expand Up @@ -80,12 +109,14 @@ public virtual TExtension GetExtension<TExtension>()
public abstract DbContextOptions WithExtension<TExtension>(TExtension extension)
where TExtension : class, IDbContextOptionsExtension;

private readonly ImmutableSortedDictionary<Type, IDbContextOptionsExtension> _extensionsMap;

/// <summary>
/// Gets the extensions that store the configured options.
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
protected virtual IImmutableDictionary<Type, IDbContextOptionsExtension> ExtensionsMap
[EntityFrameworkInternal]
protected virtual ImmutableSortedDictionary<Type, (IDbContextOptionsExtension Extension, int Ordinal)> ExtensionsMap
=> _extensionsMap;

/// <summary>
Expand Down Expand Up @@ -121,7 +152,8 @@ public override bool Equals(object? obj)
protected virtual bool Equals(DbContextOptions other)
=> _extensionsMap.Count == other._extensionsMap.Count
&& _extensionsMap.Zip(other._extensionsMap)
.All(p => p.First.Value.Info.ShouldUseSameServiceProvider(p.Second.Value.Info));
.All(p => p.First.Value.Ordinal == p.Second.Value.Ordinal
&& p.First.Value.Extension.Info.ShouldUseSameServiceProvider(p.Second.Value.Extension.Info));

/// <inheritdoc />
public override int GetHashCode()
Expand All @@ -131,7 +163,7 @@ public override int GetHashCode()
foreach (var dbContextOptionsExtension in _extensionsMap)
{
hashCode.Add(dbContextOptionsExtension.Key);
hashCode.Add(dbContextOptionsExtension.Value.Info.GetServiceProviderHashCode());
hashCode.Add(dbContextOptionsExtension.Value.Extension.Info.GetServiceProviderHashCode());
}

return hashCode.ToHashCode();
Expand Down
17 changes: 15 additions & 2 deletions src/EFCore/DbContextOptions`.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public class DbContextOptions<TContext> : DbContextOptions
/// to create instances of this class and it is not designed to be directly constructed in your application code.
/// </summary>
public DbContextOptions()
: this(ImmutableSortedDictionary.Create<Type, IDbContextOptionsExtension>(TypeFullNameComparer.Instance))
: base()
{
}

Expand All @@ -44,12 +44,25 @@ public DbContextOptions(
{
}

private DbContextOptions(
ImmutableSortedDictionary<Type, (IDbContextOptionsExtension Extension, int Ordinal)> extensions)
: base(extensions)
{
}

/// <inheritdoc />
public override DbContextOptions WithExtension<TExtension>(TExtension extension)
{
Check.NotNull(extension, nameof(extension));

return new DbContextOptions<TContext>(ExtensionsMap.SetItem(extension.GetType(), extension));
var type = extension.GetType();
var ordinal = ExtensionsMap.Count;
if (ExtensionsMap.TryGetValue(type, out var existingValue))
{
ordinal = existingValue.Ordinal;
}

return new DbContextOptions<TContext>(ExtensionsMap.SetItem(type, (extension, ordinal)));
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions test/EFCore.Specification.Tests/LoggingTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ public void Logs_context_initialization_default_options()
public void Logs_context_initialization_no_tracking()
{
Assert.Equal(
ExpectedMessage(DefaultOptions + "NoTracking"),
ExpectedMessage("NoTracking " + DefaultOptions),
ActualMessage(s => CreateOptionsBuilder(s).UseQueryTrackingBehavior(QueryTrackingBehavior.NoTracking)));
}

[ConditionalFact]
public void Logs_context_initialization_sensitive_data_logging()
{
Assert.Equal(
ExpectedMessage(DefaultOptions + "SensitiveDataLoggingEnabled"),
ExpectedMessage("SensitiveDataLoggingEnabled " + DefaultOptions),
ActualMessage(s => CreateOptionsBuilder(s).EnableSensitiveDataLogging()));
}

Expand Down
57 changes: 57 additions & 0 deletions test/EFCore.Tests/ServiceProviderCacheTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,39 @@ public void Returns_different_provider_for_different_type_of_configured_extensio
loggerFactory.Log[1].Message);
}

[ConditionalFact]
public void Returns_different_provider_for_extensions_configured_in_different_order()
{
var loggerFactory = new ListLoggerFactory();

var config1Log = new List<string>();
var config1Builder = new DbContextOptionsBuilder();
((IDbContextOptionsBuilderInfrastructure)config1Builder)
.AddOrUpdateExtension(new FakeDbContextOptionsExtension1(config1Log));
((IDbContextOptionsBuilderInfrastructure)config1Builder)
.AddOrUpdateExtension(new FakeDbContextOptionsExtension2(config1Log));
config1Builder.UseLoggerFactory(loggerFactory);
config1Builder.UseInMemoryDatabase(Guid.NewGuid().ToString());

var config2Log = new List<string>();
var config2Builder = new DbContextOptionsBuilder();
((IDbContextOptionsBuilderInfrastructure)config2Builder)
.AddOrUpdateExtension(new FakeDbContextOptionsExtension2(config2Log));
((IDbContextOptionsBuilderInfrastructure)config2Builder)
.AddOrUpdateExtension(new FakeDbContextOptionsExtension1(config2Log));
config2Builder.UseLoggerFactory(loggerFactory);
config2Builder.UseInMemoryDatabase(Guid.NewGuid().ToString());

var cache = new ServiceProviderCache();

Assert.NotSame(cache.GetOrAdd(config1Builder.Options, true), cache.GetOrAdd(config2Builder.Options, true));

Assert.Equal(2, loggerFactory.Log.Count);

Assert.Equal(new[] { nameof(FakeDbContextOptionsExtension1), nameof(FakeDbContextOptionsExtension2) }, config1Log);
Assert.Equal(new[] { nameof(FakeDbContextOptionsExtension2), nameof(FakeDbContextOptionsExtension1) }, config2Log);
}

[ConditionalFact]
public void Returns_same_provider_for_same_type_of_configured_extensions_and_replaced_service_types()
{
Expand Down Expand Up @@ -226,14 +259,26 @@ private static DbContextOptions CreateOptions<TExtension>(ILoggerFactory loggerF
private class FakeDbContextOptionsExtension1 : IDbContextOptionsExtension
{
private DbContextOptionsExtensionInfo _info;
private readonly List<string> _log;

public string Something { get; set; }

public DbContextOptionsExtensionInfo Info
=> _info ??= new ExtensionInfo(this);

public FakeDbContextOptionsExtension1()
: this(new List<string>())
{
}

public FakeDbContextOptionsExtension1(List<string> log)
{
_log = log;
}

public virtual void ApplyServices(IServiceCollection services)
{
_log.Add(GetType().ShortDisplayName());
}

public virtual void Validate(IDbContextOptions options)
Expand Down Expand Up @@ -269,12 +314,24 @@ public override void PopulateDebugInfo(IDictionary<string, string> debugInfo)
private class FakeDbContextOptionsExtension2 : IDbContextOptionsExtension
{
private DbContextOptionsExtensionInfo _info;
private readonly List<string> _log;

public DbContextOptionsExtensionInfo Info
=> _info ??= new ExtensionInfo(this);

public FakeDbContextOptionsExtension2()
: this(new List<string>())
{
}

public FakeDbContextOptionsExtension2(List<string> log)
{
_log = log;
}

public virtual void ApplyServices(IServiceCollection services)
{
_log.Add(GetType().ShortDisplayName());
}

public virtual void Validate(IDbContextOptions options)
Expand Down

0 comments on commit eb29ef3

Please sign in to comment.