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

EntityFrameworkExperimentalAttribute #26229

Closed
ajcvickers opened this issue Oct 1, 2021 · 14 comments
Closed

EntityFrameworkExperimentalAttribute #26229

ajcvickers opened this issue Oct 1, 2021 · 14 comments
Labels
closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed.

Comments

@ajcvickers
Copy link
Member

Introduce an EntityFrameworkExperimentalAttribute, which works like the EntityFrameworkInternalAttribute, except that it indicates a feature is experimental for the current release, rather than internal.

@jeffhandley
Copy link
Member

RequiresPreviewFeaturesAttribute was created in .NET 6 and is part of the shared framework, with an analyzer that reports builds errors if the caller hasn't opted in to using preview features. I'd be interested to know if that attribute could meet these needs instead of having another mechanism.

@ajcvickers
Copy link
Member Author

Thanks @jeffhandley. I'll look into this tomorrow. We may be able to use it in 6.0 for #26232.

@ajcvickers
Copy link
Member Author

@jeffhandley I can't seem to make RequiresPreviewFeaturesAttribute work. I don't get any warnings or errors when using it. What am I missing here?

I'm using a local build of EF Core. My code checks that the attribute is present:

using System;
using System.Linq;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
using Microsoft.Extensions.Logging;

public class Foo
{
    public int Id { get; set; }
    public string? Bar { get; set; }
}

public class SomeDbContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseInMemoryDatabase("Test")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder
            .Entity<Foo>()
            .Property(e => e.Bar)
            .HasConversion(new MyConverter());
    }
}

public class Program
{
    public static void Main()
    {
        var attributes = typeof(ValueConverter<,>)
            .GetConstructors()
            .Single(c => c.GetParameters().Any(p => p.Name == "convertsNulls"))
            .GetCustomAttributes(true)
            .ToList();

        foreach (var attribute in attributes)
        {
            Console.WriteLine($"Found: {attribute.GetType().Name}");
        }
        
        using (var context = new SomeDbContext())
        {
            context.Add(new Foo { Bar = "X" });
            context.SaveChanges();
        }
        
        using (var context = new SomeDbContext())
        {
            Console.WriteLine(context.Set<Foo>().Single().Bar);
        }
    }
}

public class MyConverter : ValueConverter<string?, string?>
{
    public MyConverter()
        : base(v => v, v => v, convertsNulls: true)
    {
    }
}

csproj:

<Project Sdk="Microsoft.NET.Sdk">

    <PropertyGroup>
        <OutputType>Exe</OutputType>
        <TargetFramework>net6.0</TargetFramework>
        <ImplicitUsings>enable</ImplicitUsings>
        <Nullable>enable</Nullable>
    </PropertyGroup>

    <ItemGroup>
      <PackageReference Include="Microsoft.EntityFrameworkCore.InMemory" Version="6.0.0-dev" />
    </ItemGroup>

</Project>

Build correctly warns for using EF internal code, but nothing for the preview attribute:

PS C:\local\code\TestPreviewFeature> dotnet build
Microsoft (R) Build Engine version 17.0.0-preview-21458-01+2c5510013 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  All projects are up-to-date for restore.
  You are using a preview version of .NET. See: https://aka.ms/dotnet-core-preview
C:\local\code\TestPreviewFeature\Program.cs(61,9): warning EF1001: Microsoft.EntityFrameworkCore.Storage.ValueConversion.ValueConverter<string?, string?> 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:\local\code\TestPreviewFeature\TestPreviewFeature.csproj]
  TestPreviewFeature -> C:\local\code\TestPreviewFeature\bin\Debug\net6.0\TestPreviewFeature.dll

Build succeeded.

C:\local\code\TestPreviewFeature\Program.cs(61,9): warning EF1001: Microsoft.EntityFrameworkCore.Storage.ValueConversion.ValueConverter<string?, string?> 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:\local\code\TestPreviewFeature\TestPreviewFeature.csproj]
    1 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.96

Running also results in no errors:

PS C:\local\code\TestPreviewFeature> dotnet run
Found: RequiresPreviewFeaturesAttribute
Found: EntityFrameworkInternalAttribute
warn: 10/6/2021 12:08:53.212 CoreEventId.SensitiveDataLoggingEnabledWarning[10400] (Microsoft.EntityFrameworkCore.Infrastructure)
      Sensitive data logging is enabled. Log entries and exception messages may include sensitive application data; this mode should only be enabled during development.
info: 10/6/2021 12:08:53.243 CoreEventId.ContextInitialized[10403] (Microsoft.EntityFrameworkCore.Infrastructure)
      Entity Framework Core 6.0.0-dev initialized 'SomeDbContext' using provider 'Microsoft.EntityFrameworkCore.InMemory:6.0.0-dev' with options: SensitiveDataLoggingEnabled StoreName=Test
info: 10/6/2021 12:08:53.307 InMemoryEventId.ChangesSaved[30100] (Microsoft.EntityFrameworkCore.Update)
      Saved 1 entities to in-memory store.
info: 10/6/2021 12:08:53.328 CoreEventId.ContextInitialized[10403] (Microsoft.EntityFrameworkCore.Infrastructure)
      Entity Framework Core 6.0.0-dev initialized 'SomeDbContext' using provider 'Microsoft.EntityFrameworkCore.InMemory:6.0.0-dev' with options: SensitiveDataLoggingEnabled StoreName=Test
X

@jeffhandley
Copy link
Member

I'm unsure from those details if you have 6.0 RC2+ SDK in place. The CA2252 (Preview Feature) analyzer was only recently set to report build errors when usage of preview features was detected--it was reporting as info level when it was first introduced. Otherwise, it does look to me like referencing the base constructor should report a diagnostic. If you are indeed using RC2+ SDK, let me know and we can pair up to figure out what's happening.

Thanks!

@ajcvickers
Copy link
Member Author

SDK is 6.0.100-rc.2.21505.57

@ajcvickers
Copy link
Member Author

@jeffhandley I tried creating a minimal class library and a console app referencing it (no EF), and I still can't get it to work. See attached.

TMin.zip

@roji
Copy link
Member

roji commented Oct 7, 2021

I wonder if this is related to using <ProjectReference> rather than <PackageReference> - I've seen many cases where this impacts things (especially since an analyzer is involved)...

@ajcvickers
Copy link
Member Author

@roji I tried it with a direct assembly reference, rather than a project reference, and there was no difference. Didn't try building it into a package.

@roji
Copy link
Member

roji commented Oct 7, 2021

Could be worth a try, we know that analyzers and assembly/project/package references are tricky..

@jeffhandley
Copy link
Member

Thanks for this. I meant to leave a comment earlier--I'm investigating this. It's not behaving as I expected it to, and I've repro'd this behavior with an even simpler scenario. I'll follow back up later.

@ajcvickers
Copy link
Member Author

Thanks @jeffhandley

@jeffhandley
Copy link
Member

@ajcvickers Thanks a bunch for trying this out and letting me know it wasn't working as expected. This uncovered that we had a 6.0 GA ship-blocker issue lurking here that had gone undetected. That fix is now in place and it will make the GA snap.

To work around the bug until you take in an SDK build with the fix though, you can select the CA2252 analyzer from your dependencies and set it to Error severity in your project's editorconfig.

@KennethHoff
Copy link

Should this be closed with the addition of [Experimental] Attribute in .Net 8?

@roji roji removed this from the Backlog milestone Jul 22, 2023
@roji
Copy link
Member

roji commented Jul 22, 2023

@KennethHoff yeah, probably - removing from the backlog to re-triage.

@ajcvickers ajcvickers added closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed. and removed propose-close type-enhancement area-global labels Jul 27, 2023
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-out-of-scope This is not something that will be fixed/implemented and the issue is closed.
Projects
None yet
Development

No branches or pull requests

4 participants