dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.81k stars 3.2k forks source link

Is there a way to avoid EF1001 warnings without having to use every providers #26440

Open juchom opened 3 years ago

juchom commented 3 years ago

Ask a question

I continue my work on integrating Ulid in EF Core, the code is pretty small and should work for every SQL Database.

Ulid is stored to char(26) or binary(16) nothing fancy.

I'm trying to avoid creating a package for SQL Server, Postgre, MySQL...

Include your code

public static class RelationalUlidDbContextOptionsBuilderExtensions
{
    public static RelationalDbContextOptionsBuilder<TBuilder, TExtension> UseUlidToString<TBuilder, TExtension>(this RelationalDbContextOptionsBuilder<TBuilder, TExtension> optionsBuilder)
        where TBuilder : RelationalDbContextOptionsBuilder<TBuilder, TExtension> where TExtension : RelationalOptionsExtension, new()
        => optionsBuilder.UseUlid(UlidStorageMode.String);

    public static RelationalDbContextOptionsBuilder<TBuilder, TExtension> UseUlidToBytes<TBuilder, TExtension>(this RelationalDbContextOptionsBuilder<TBuilder, TExtension> optionsBuilder)
        where TBuilder : RelationalDbContextOptionsBuilder<TBuilder, TExtension> where TExtension : RelationalOptionsExtension, new()
        => optionsBuilder.UseUlid(UlidStorageMode.Bytes);

    internal static RelationalDbContextOptionsBuilder<TBuilder, TExtension> UseUlid<TBuilder, TExtension>(this RelationalDbContextOptionsBuilder<TBuilder, TExtension> optionsBuilder, UlidStorageMode storageMode)
        where TBuilder : RelationalDbContextOptionsBuilder<TBuilder, TExtension> where TExtension : RelationalOptionsExtension, new()
    {
        if (optionsBuilder is null)
        {
            throw new ArgumentNullException(nameof(optionsBuilder));
        }

        var coreOptionsBuilder = ((IRelationalDbContextOptionsBuilderInfrastructure)optionsBuilder).OptionsBuilder;

        var extension = coreOptionsBuilder.Options.FindExtension<RelationalUlidOptionsExtension>();

        if (extension is not null && extension.StorageMode != storageMode)
        {
            throw new InvalidOperationException("You are registering Ulid with two different storage mode");
        }

        if (extension is null)
        {
            extension = new RelationalUlidOptionsExtension(storageMode);
        }

        ((IDbContextOptionsBuilderInfrastructure)coreOptionsBuilder).AddOrUpdateExtension(extension);

        return optionsBuilder;
    }
}

If I do this in a Program.cs

// For SQL Server
optionsBuilder.UseSqlServer("connectionString", x => x.UseUlidToString());

// For Postgre
optionsBuilder.UseNpgsql("connectionString", x => x.UseUlidToString());

Warnings

C:\dev\ConsoleApplication1\Program.cs(11,18): warning EF1001: 
Npgsql.EntityFrameworkCore.PostgreSQL.Infrastructure.Internal.NpgsqlOptionsExtension 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. [C:\dev\ConsoleApplication1\ConsoleApplication1.csproj]

C:\dev\ConsoleApplication1\Program.cs(43,18): warning EF1001:
Microsoft.EntityFrameworkCore.SqlServer.Infrastructure.Internal.SqlServerOptionsExtension 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. [C:\dev\ConsoleApplication1\ConsoleApplication1.csproj]

Is there a way to refactor this extension methods to avoid duplicating code

Writing the code like this for Sql Server removes the EF1001 warning

public static class SqlServerUlidDbContextOptionsBuilderExtensions
{
    public static SqlServerDbContextOptionsBuilder UseUlidToString(this SqlServerDbContextOptionsBuilder optionsBuilder)
        => optionsBuilder.UseUlid(UlidStorageMode.String);

    public static SqlServerDbContextOptionsBuilder UseUlidToBytes(this SqlServerDbContextOptionsBuilder optionsBuilder)
        => optionsBuilder.UseUlid(UlidStorageMode.Bytes);

    internal static SqlServerDbContextOptionsBuilder UseUlid(this SqlServerDbContextOptionsBuilder optionsBuilder, UlidStorageMode storageMode)
    {
        if (optionsBuilder is null)
        {
            throw new ArgumentNullException(nameof(optionsBuilder));
        }

        var coreOptionsBuilder = ((IRelationalDbContextOptionsBuilderInfrastructure)optionsBuilder).OptionsBuilder;

        var extension = coreOptionsBuilder.Options.FindExtension<RelationalUlidOptionsExtension>();

        if (extension is not null && extension.StorageMode != storageMode)
        {
            throw new InvalidOperationException("You are registering Ulid with two different storage mode");
        }

        if (extension is null)
        {
            extension = new RelationalUlidOptionsExtension(storageMode);
        }

        ((IDbContextOptionsBuilderInfrastructure)coreOptionsBuilder).AddOrUpdateExtension(extension);

        return optionsBuilder;
    }
}

Writing the code like this for Postgre removes the EF1001 warning

public static class NpgsqlUlidDbContextOptionsBuilderExtensions
{
    public static NpgsqlDbContextOptionsBuilder UseUlidToString(this NpgsqlDbContextOptionsBuilder optionsBuilder)
        => optionsBuilder.UseUlid(UlidStorageMode.String);

    public static NpgsqlDbContextOptionsBuilder UseUlidToBytes(this NpgsqlDbContextOptionsBuilder optionsBuilder)
        => optionsBuilder.UseUlid(UlidStorageMode.Bytes);

    internal static NpgsqlDbContextOptionsBuilder UseUlid(this NpgsqlDbContextOptionsBuilder optionsBuilder, UlidStorageMode storageMode)
    {
        if (optionsBuilder is null)
        {
            throw new ArgumentNullException(nameof(optionsBuilder));
        }

        var coreOptionsBuilder = ((IRelationalDbContextOptionsBuilderInfrastructure)optionsBuilder).OptionsBuilder;

        var extension = coreOptionsBuilder.Options.FindExtension<RelationalUlidOptionsExtension>();

        if (extension is not null && extension.StorageMode != storageMode)
        {
            throw new InvalidOperationException("You are registering Ulid with two different storage mode");
        }

        if (extension is null)
        {
            extension = new RelationalUlidOptionsExtension(storageMode);
        }

        ((IDbContextOptionsBuilderInfrastructure)coreOptionsBuilder).AddOrUpdateExtension(extension);

        return optionsBuilder;
    }
}

As you can see, the only thing changing here is SqlServerDbContextOptionsBuilder to NpgsqlDbContextOptionsBuilder.

Include provider and version information

EF Core version: 6.0.0-rc2 Database provider: SqlServer & Npgsql Target framework: .NET 6.0.0-c2 Operating system: Windows 10 IDE: Visual Studio 2022

juchom commented 3 years ago

If I change the signature to use the IRelationalDbContextOptionsBuilderInfrastructure interface instead of RelationalDbContextOptionsBuilder<TBuilder, TExtension> abstract class

public static class RelationalUlidDbContextOptionsBuilderExtensions
{
    public static TBuilder UseUlidToString<TBuilder>(this TBuilder optionsBuilder)
        where TBuilder : IRelationalDbContextOptionsBuilderInfrastructure
        => optionsBuilder.UseUlid(UlidStorageMode.String);

    public static TBuilder UseUlidToBytes<TBuilder>(this TBuilder optionsBuilder)
        where TBuilder : IRelationalDbContextOptionsBuilderInfrastructure
        => optionsBuilder.UseUlid(UlidStorageMode.Bytes);

    internal static TBuilder UseUlid<TBuilder>(this TBuilder optionsBuilder, UlidStorageMode storageMode)
        where TBuilder : IRelationalDbContextOptionsBuilderInfrastructure
    {
        if (optionsBuilder is null)
        {
            throw new ArgumentNullException(nameof(optionsBuilder));
        }

        var coreOptionsBuilder = ((IRelationalDbContextOptionsBuilderInfrastructure)optionsBuilder).OptionsBuilder;

        var extension = coreOptionsBuilder.Options.FindExtension<RelationalUlidOptionsExtension>();

        if (extension is not null && extension.StorageMode != storageMode)
        {
            throw new InvalidOperationException("You are registering Ulid with two different storage mode");
        }

        if (extension is null)
        {
            extension = new RelationalUlidOptionsExtension(storageMode);
        }

        ((IDbContextOptionsBuilderInfrastructure)coreOptionsBuilder).AddOrUpdateExtension(extension);

        return optionsBuilder;
    }
}

Warnings are gone, but I'm not sure if this is a good solution...

ajcvickers commented 3 years ago

Note for triage: it seems to me that SqlOptionsExtension and similar should not be internal.

ajcvickers commented 3 years ago

Note from triage: we believe the analyzer should be changed to not mark this as use of internal code.