Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix enum mapping when using NpgsqlDataSource #2628

Merged
merged 1 commit into from
Jan 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions src/EFCore.PG/Infrastructure/Internal/INpgsqlSingletonOptions.cs
Original file line number Diff line number Diff line change
@@ -1,32 +1,39 @@
namespace Npgsql.EntityFrameworkCore.PostgreSQL.Infrastructure.Internal;
using System.Data.Common;

namespace Npgsql.EntityFrameworkCore.PostgreSQL.Infrastructure.Internal;

/// <summary>
/// Represents options for Npgsql that can only be set at the <see cref="IServiceProvider"/> singleton level.
/// Represents options for Npgsql that can only be set at the <see cref="IServiceProvider"/> singleton level.
/// </summary>
public interface INpgsqlSingletonOptions : ISingletonOptions
{
/// <summary>
/// The backend version to target.
/// The backend version to target.
/// </summary>
Version PostgresVersion { get; }

/// <summary>
/// The backend version to target, but returns <see langword="null" /> unless the user explicitly specified a version.
/// The backend version to target, but returns <see langword="null" /> unless the user explicitly specified a version.
/// </summary>
Version? PostgresVersionWithoutDefault { get; }

/// <summary>
/// Whether to target Redshift.
/// Whether to target Redshift.
/// </summary>
bool UseRedshift { get; }

/// <summary>
/// True if reverse null ordering is enabled; otherwise, false.
/// Whether reverse null ordering is enabled.
/// </summary>
bool ReverseNullOrderingEnabled { get; }

/// <summary>
/// The collection of range mappings.
/// The data source being used, or <see langword="null" /> if a connection string or connection was provided directly.
/// </summary>
DbDataSource? DataSource { get; }

/// <summary>
/// The collection of range mappings.
/// </summary>
IReadOnlyList<UserRangeDefinition> UserRangeDefinitions { get; }
}
18 changes: 17 additions & 1 deletion src/EFCore.PG/Internal/NpgsqlSingletonOptions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Npgsql.EntityFrameworkCore.PostgreSQL.Infrastructure;
using System.Data.Common;
using Npgsql.EntityFrameworkCore.PostgreSQL.Infrastructure;
using Npgsql.EntityFrameworkCore.PostgreSQL.Infrastructure.Internal;

namespace Npgsql.EntityFrameworkCore.PostgreSQL.Internal;
Expand Down Expand Up @@ -46,6 +47,14 @@ public class NpgsqlSingletonOptions : INpgsqlSingletonOptions
/// </summary>
public virtual bool ReverseNullOrderingEnabled { get; private set; }

/// <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>
public virtual DbDataSource? DataSource { get; private set; }

/// <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
Expand All @@ -72,6 +81,7 @@ public virtual void Initialize(IDbContextOptions options)
PostgresVersion = npgsqlOptions.PostgresVersion ?? DefaultPostgresVersion;
UseRedshift = npgsqlOptions.UseRedshift;
ReverseNullOrderingEnabled = npgsqlOptions.ReverseNullOrdering;
DataSource = npgsqlOptions.DataSource;
UserRangeDefinitions = npgsqlOptions.UserRangeDefinitions;
}

Expand Down Expand Up @@ -104,6 +114,12 @@ public virtual void Validate(IDbContextOptions options)
nameof(DbContextOptionsBuilder.UseInternalServiceProvider)));
}

if (!ReferenceEquals(DataSource, npgsqlOptions.DataSource))
{
throw new InvalidOperationException(
NpgsqlStrings.TwoDataSourcesInSameServiceProvider(nameof(DbContextOptionsBuilder.UseInternalServiceProvider)));
}

if (UserRangeDefinitions.Count != npgsqlOptions.UserRangeDefinitions.Count
|| UserRangeDefinitions.Zip(npgsqlOptions.UserRangeDefinitions).Any(t => t.First != t.Second))
{
Expand Down
8 changes: 8 additions & 0 deletions src/EFCore.PG/Properties/NpgsqlStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/EFCore.PG/Properties/NpgsqlStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@
<resheader name="writer">
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
</resheader>
<data name="TwoDataSourcesInSameServiceProvider" xml:space="preserve">
<value>Using two distinct data sources within a service provider is not supported, and Entity Framework is not building its own internal service provider. Either allow Entity Framework to build the service provider by removing the call to '{useInternalServiceProvider}', or ensure that the same data source is used for all uses of a given service provider passed to '{useInternalServiceProvider}'.</value>
</data>
<data name="DuplicateColumnCompressionMethodMismatch" xml:space="preserve">
<value>'{entityType1}.{property1}' and '{entityType2}.{property2}' are both mapped to column '{columnName}' in '{table}' but are configured with different compression methods.</value>
</data>
Expand Down
39 changes: 24 additions & 15 deletions src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,6 @@ static NpgsqlTypeMappingSource()
/// </summary>
private readonly Dictionary<Type, List<NpgsqlMultirangeTypeMapping>> _multirangeTypeMappings;

private static MethodInfo? _adoUserTypeMappingsGetMethodInfo;

private readonly bool _supportsMultiranges;

#region Mappings
Expand Down Expand Up @@ -207,7 +205,8 @@ static NpgsqlTypeMappingSource()
/// 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>
public NpgsqlTypeMappingSource(TypeMappingSourceDependencies dependencies,
public NpgsqlTypeMappingSource(
TypeMappingSourceDependencies dependencies,
RelationalTypeMappingSourceDependencies relationalDependencies,
ISqlGenerationHelper sqlGenerationHelper,
INpgsqlSingletonOptions npgsqlSingletonOptions)
Expand Down Expand Up @@ -443,7 +442,7 @@ public NpgsqlTypeMappingSource(TypeMappingSourceDependencies dependencies,
StoreTypeMappings = new ConcurrentDictionary<string, RelationalTypeMapping[]>(storeTypeMappings, StringComparer.OrdinalIgnoreCase);
ClrTypeMappings = new ConcurrentDictionary<Type, RelationalTypeMapping>(clrTypeMappings);

LoadUserDefinedTypeMappings(sqlGenerationHelper);
LoadUserDefinedTypeMappings(sqlGenerationHelper, npgsqlSingletonOptions.DataSource as NpgsqlDataSource);

_userRangeDefinitions = npgsqlSingletonOptions?.UserRangeDefinitions ?? Array.Empty<UserRangeDefinition>();
}
Expand All @@ -452,28 +451,38 @@ public NpgsqlTypeMappingSource(TypeMappingSourceDependencies dependencies,
/// To be used in case user-defined mappings are added late, after this TypeMappingSource has already been initialized.
/// This is basically only for test usage.
/// </summary>
public virtual void LoadUserDefinedTypeMappings(ISqlGenerationHelper sqlGenerationHelper)
=> SetupEnumMappings(sqlGenerationHelper);
public virtual void LoadUserDefinedTypeMappings(
ISqlGenerationHelper sqlGenerationHelper,
NpgsqlDataSource? dataSource)
=> SetupEnumMappings(sqlGenerationHelper, dataSource);

/// <summary>
/// Gets all global enum mappings from the ADO.NET layer and creates mappings for them
/// </summary>
protected virtual void SetupEnumMappings(ISqlGenerationHelper sqlGenerationHelper)
protected virtual void SetupEnumMappings(ISqlGenerationHelper sqlGenerationHelper, NpgsqlDataSource? dataSource)
{
var adoEnumMappings = new List<IUserEnumTypeMapping>();

#pragma warning disable CS0618 // NpgsqlConnection.GlobalTypeMapper is obsolete
_adoUserTypeMappingsGetMethodInfo ??= NpgsqlConnection.GlobalTypeMapper.GetType().GetProperty("UserTypeMappings")?.GetMethod;
if (NpgsqlConnection.GlobalTypeMapper.GetType().GetProperty("UserTypeMappings")?.GetMethod is { } globalTypeMappingsMethodInfo
&& globalTypeMappingsMethodInfo.Invoke(NpgsqlConnection.GlobalTypeMapper, Array.Empty<object>()) is
IDictionary<string, IUserTypeMapping> globalUserMappings)
{
adoEnumMappings.AddRange(globalUserMappings.Values.OfType<IUserEnumTypeMapping>());
}
#pragma warning restore CS0618

if (_adoUserTypeMappingsGetMethodInfo is null)
// TODO: Think about what to do here. We could just require users to do the mapping at the EF level, and then EF would take care
// of the ADO mapping.
if (dataSource is not null
&& typeof(NpgsqlDataSource).GetField("_userTypeMappings", BindingFlags.NonPublic | BindingFlags.Instance) is
{ } dataSourceTypeMappingsFieldInfo
&& dataSourceTypeMappingsFieldInfo.GetValue(dataSource) is IDictionary<string, IUserTypeMapping> dataSourceUserMappings)
{
return;
adoEnumMappings.AddRange(dataSourceUserMappings.Values.OfType<IUserEnumTypeMapping>());
}

#pragma warning disable CS0618 // NpgsqlConnection.GlobalTypeMapper is obsolete
var adoUserTypeMappings = (IDictionary<string, IUserTypeMapping>)_adoUserTypeMappingsGetMethodInfo.Invoke(NpgsqlConnection.GlobalTypeMapper, Array.Empty<object>())!;
#pragma warning restore CS0618

foreach (var adoUserTypeMapping in adoUserTypeMappings.Values.OfType<IUserEnumTypeMapping>())
foreach (var adoUserTypeMapping in adoEnumMappings)
{
// TODO: update with schema per https://github.com/npgsql/npgsql/issues/2121
var components = adoUserTypeMapping.PgTypeName.Split('.');
Expand Down
4 changes: 3 additions & 1 deletion test/EFCore.PG.FunctionalTests/BuiltInDataTypesNpgsqlTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -977,10 +977,12 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
{
base.OnModelCreating(modelBuilder, context);

// TODO: Switch to using data source
#pragma warning disable CS0618 // NpgsqlConnection.GlobalTypeMapper is obsolete
NpgsqlConnection.GlobalTypeMapper.MapEnum<Mood>();
#pragma warning restore CS0618
((NpgsqlTypeMappingSource)context.GetService<ITypeMappingSource>()).LoadUserDefinedTypeMappings(context.GetService<ISqlGenerationHelper>());
((NpgsqlTypeMappingSource)context.GetService<ITypeMappingSource>()).LoadUserDefinedTypeMappings(
context.GetService<ISqlGenerationHelper>(), dataSource: null);

modelBuilder.HasPostgresEnum("mood", new[] { "happy", "sad" });

Expand Down