Skip to content

Commit

Permalink
Fix extension/service provider caching bug in regards to DbDataSource…
Browse files Browse the repository at this point in the history
… if set using the ApplicationServiceProvider.
  • Loading branch information
lauxjpn committed Dec 3, 2023
1 parent 8d1a27b commit f56c994
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 6 deletions.
5 changes: 5 additions & 0 deletions src/EFCore.MySql/Infrastructure/Internal/IMySqlOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ namespace Pomelo.EntityFrameworkCore.MySql.Infrastructure.Internal
public interface IMySqlOptions : ISingletonOptions
{
MySqlConnectionSettings ConnectionSettings { get; }

/// <remarks>
/// If null, there might still be a `DbDataSource` in the ApplicationServiceProvider.
/// </remarks>>
DbDataSource DataSource { get; }

ServerVersion ServerVersion { get; }
CharSet DefaultCharSet { get; }
CharSet NationalCharSet { get; }
Expand Down
21 changes: 18 additions & 3 deletions src/EFCore.MySql/Internal/MySqlOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,11 @@ public MySqlOptions()

public virtual void Initialize(IDbContextOptions options)
{
var coreOptions = options.FindExtension<CoreOptionsExtension>() ?? new CoreOptionsExtension();
var mySqlOptions = options.FindExtension<MySqlOptionsExtension>() ?? new MySqlOptionsExtension();
var mySqlJsonOptions = (MySqlJsonOptionsExtension)options.Extensions.LastOrDefault(e => e is MySqlJsonOptionsExtension);

ConnectionSettings = GetConnectionSettings(mySqlOptions);
DataSource = mySqlOptions.DataSource ?? coreOptions.ApplicationServiceProvider?.GetService<MySqlDataSource>();
DataSource = mySqlOptions.DataSource;
ServerVersion = mySqlOptions.ServerVersion ?? throw new InvalidOperationException($"The {nameof(ServerVersion)} has not been set.");
NoBackslashEscapes = mySqlOptions.NoBackslashEscapes;
ReplaceLineBreaksWithCharFunction = mySqlOptions.ReplaceLineBreaksWithCharFunction;
Expand All @@ -80,7 +79,18 @@ public virtual void Validate(IDbContextOptions options)
var mySqlJsonOptions = (MySqlJsonOptionsExtension)options.Extensions.LastOrDefault(e => e is MySqlJsonOptionsExtension);
var connectionSettings = GetConnectionSettings(mySqlOptions);

if (mySqlOptions.DataSource is not null &&
//
// CHECK: To we have to ensure that the ApplicationServiceProvider itself is not replaced, because we rely on it in our
// DbDataSource check, or is that not possible?
//

// Even though we only save a DbDataSource that has been explicitly set using the MySqlOptionsExtensions here in MySqlOptions,
// we will later also fall back to a DbDataSource that has been added as a service to the ApplicationServiceProvider, if no
// DbDataSource has been explicitly set here. We call that DbDataSource the "effective" DbDataSource and handle it in the same
// way we would handle a singleton option.
var effectiveDataSource = mySqlOptions.DataSource ??
options.FindExtension<CoreOptionsExtension>()?.ApplicationServiceProvider?.GetService<MySqlDataSource>();
if (effectiveDataSource is not null &&
!ReferenceEquals(DataSource, mySqlOptions.DataSource))
{
throw new InvalidOperationException(
Expand Down Expand Up @@ -305,7 +315,12 @@ public override int GetHashCode()
}

public virtual MySqlConnectionSettings ConnectionSettings { get; private set; }

/// <summary>
/// If null, there might still be a `DbDataSource` in the ApplicationServiceProvider.
/// </summary>
public virtual DbDataSource DataSource { get; private set; }

public virtual ServerVersion ServerVersion { get; private set; }
public virtual CharSet DefaultCharSet { get; private set; }
public virtual CharSet NationalCharSet { get; }
Expand Down
21 changes: 20 additions & 1 deletion src/EFCore.MySql/Storage/Internal/MySqlRelationalConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.Extensions.DependencyInjection;
using MySqlConnector;
using Pomelo.EntityFrameworkCore.MySql.Infrastructure;
using Pomelo.EntityFrameworkCore.MySql.Infrastructure.Internal;
Expand All @@ -28,7 +29,10 @@ public MySqlRelationalConnection(
RelationalConnectionDependencies dependencies,
IMySqlConnectionStringOptionsValidator mySqlConnectionStringOptionsValidator,
IMySqlOptions mySqlSingletonOptions)
: this(dependencies, mySqlConnectionStringOptionsValidator, mySqlSingletonOptions.DataSource)
: this(
dependencies,
mySqlConnectionStringOptionsValidator,
GetEffectiveDataSource(mySqlSingletonOptions, dependencies.ContextOptions))
{
}

Expand Down Expand Up @@ -69,6 +73,21 @@ public MySqlRelationalConnection(
}
}

/// <summary>
/// We allow users to either explicitly set a DbDataSource using our `MySqlOptionsExtensions` or by adding it as a service via DI
/// (`ApplicationServiceProvider`).
/// We don't set a DI injected service to the `MySqlOption.DbDataSource` property, because it might get cached by the service
/// collection cache, since no relevant property might have changed in the `MySqlOptionsExtension` instance. If we would create
/// a similar DbContext instance with a different service collection later, EF Core would provide us with the *same* `MySqlOptions`
/// instance (that was cached before) and we would use the old `DbDataSource` instance that we retrieved from the old
/// `ApplicationServiceProvider`.
/// Therefore, we check the `IMySqlOptions.DbDataSource` property and the current `ApplicationServiceProvider` at the time we
/// actually need the instance.
/// </summary>
protected static DbDataSource GetEffectiveDataSource(IMySqlOptions mySqlSingletonOptions, IDbContextOptions contextOptions)
=> mySqlSingletonOptions.DataSource ??
contextOptions.FindExtension<CoreOptionsExtension>()?.ApplicationServiceProvider?.GetService<MySqlDataSource>();

// TODO: Remove, because we don't use it anywhere.
private bool IsMasterConnection { get; set; }

Expand Down
4 changes: 2 additions & 2 deletions test/EFCore.MySql.Tests/MySqlRelationalConnectionTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public void Uses_correct_DbDataSource_from_ApplicationServiceProvider_with_cache
var context1 = serviceProvider1.GetRequiredService<FakeDbContext>();

var mySqlOptions1 = context1.GetService<IMySqlOptions>();
Assert.Same(dataSource1, mySqlOptions1.DataSource);
Assert.Null(mySqlOptions1.DataSource);

var relationalConnection1 = (MySqlRelationalConnection)context1.GetService<IRelationalConnection>()!;
Assert.Same(dataSource1, relationalConnection1.DbDataSource);
Expand All @@ -177,7 +177,7 @@ public void Uses_correct_DbDataSource_from_ApplicationServiceProvider_with_cache
var context2 = serviceProvider2.GetRequiredService<FakeDbContext>();

var mySqlOptions2 = context2.GetService<IMySqlOptions>();
Assert.Same(dataSource2, mySqlOptions2.DataSource);
Assert.Null(mySqlOptions2.DataSource);

var relationalConnection2 = (MySqlRelationalConnection)context2.GetService<IRelationalConnection>()!;
Assert.Same(dataSource2, relationalConnection2.DbDataSource);
Expand Down

0 comments on commit f56c994

Please sign in to comment.