-
Notifications
You must be signed in to change notification settings - Fork 25
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
EF Core 5 support #56
Comments
There's a series of API changes from 3.1.x -> 5.x that'll need to be figured out: // Removed
IAnnotationCodeGenerator.IsHandledByConvention
IAnnotationCodeGenerator.GenerateFluentApi
CoreAnnotationNames.TypeMapping
CoreAnnotationNames.ChangeTrackingStrategy
ChangeDetector.SkipDetectChangesAnnotation
Microsoft.EntityFrameworkCore.Internal.TypeExtensions.GetNamespaces
Type.DisplayName
Multigraph<T>
IEntityType.IsIgnoredByMigrations()
Type.GetNamespaces()
//Changed
IProperty.IsFixedLength() // bool to Nullable<bool>
ICSharpDbContextGenerator.WriteCode // changed signature
INavigation.IsCollection() // changed from method to property (also I'm getting a stackoverflow problem with paket) It shouldn't be a huge job. The code structure roughly follows the C# implementation so it should be a case of side-by-siding and figuring out how the API has changed. As there has never been a nuget release it would seem to make sense to shift to the latest packages. I'll have a look at it sometime this week. |
Hi, support is planned, and slowly in progress So far it's just getting everything targeting NET 5.0, I've handled the |
I think the 2 main pain points will be:
|
Because when I first started working on this back in the 2.x days, Trying to figure out the best way to handle |
That makes sense! I've built off your branch some more of the required code changes here. 2 of the unit tests are failing and I've side stepped the |
Perfect! Can you create a PR for that please? |
I've updated my branch to fix the tests. Most the work is done to migrate to There's a failing test still - the one related to snapshot enum generation that was commented out. I think there's a genuine issue here. The C# generation up to the point where // <auto-generated />
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Migrations.Design;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
namespace MyNamespace
{
[DbContext(typeof(CSharpMigrationsGeneratorTest.MyContext))]
partial class MySnapshot : ModelSnapshot
{
protected override void BuildModel(ModelBuilder modelBuilder)
{
#pragma warning disable 612, 618
modelBuilder.Entity("Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGeneratorTest+WithAnnotations", b =>
{
b.Property<int>("Id")
.ValueGeneratedOnAdd()
.HasColumnType("default_int_mapping");
b.Property<int>("EnumDiscriminator")
.HasColumnType("default_int_mapping");
b.HasKey("Id");
b.ToTable("WithAnnotations");
b.HasDiscriminator<int>("EnumDiscriminator") Whereas the F# version: namespace MyNamespace
open System
open EntityFrameworkCore.FSharp.Test.Migrations.Design
open Microsoft.EntityFrameworkCore
open Microsoft.EntityFrameworkCore.Infrastructure
open Microsoft.EntityFrameworkCore.Metadata
open Microsoft.EntityFrameworkCore.Migrations
open Microsoft.EntityFrameworkCore.Storage.ValueConversion
[<DbContext(typeof<MyContext>)>]
type MySnapshot() =
inherit ModelSnapshot()
override this.BuildModel(modelBuilder: ModelBuilder) =
modelBuilder.Entity("EntityFrameworkCore.FSharp.Test.Migrations.Design.Derived", (fun b ->
b.HasBaseType("EntityFrameworkCore.FSharp.Test.Migrations.Design.WithAnnotations")
b.HasDiscriminator() This was probably broken in the code in 3.1 as well. |
That's some really great work! Can you create a PR again for these changes please? @bricelam, whenever you get a chance to see this, can you add @DurdSoft as a collaborator to this repo? |
The issue was related to the missing |
I've put in another PR for a silly mistake I'd made, sorry! There are still a few outstanding issues in scaffolding:
If either argument is removed it is happy again.
|
I'm working my way through the Looks like the compiler currently thinks the expression is supposed to a tuple of |
D'oh, of course. I'm going to look into migrating more of the unit tests so we can get it 1 to 1 with C#. I'm surprised none of the existing ones have caught that missing |
Cool, I'm going to look at tidying up the implementations in Snapshot, DbContext and Migration generators so bring them more in line with the changes in EFCore for net5.0 |
Hi @DurdSoft |
@simon-reynolds I've added @DurdSoft and made you an admin. Copying/porting Multigraph into this project is the right thing to do. |
Hi @tomiaijo I'll see what I can do about these issues and hopefully we'll then be in a good enough state to be able to release a NuGet package |
Hi @tomiaijo, I couldn't see the issues you encountered though. I was able to create an example using basic MVC authentication at https://github.com/simon-reynolds/EFCore.FSharp.MvcAuth |
Is your feature request related to a problem? Please describe.
Is support for EF Core 5 planned? Seems like some of the interfaces have changed.
Output of running against EF Core 5.0.3 project:
The text was updated successfully, but these errors were encountered: