efcore / EFCore.FSharp

Adds F# design-time support to EF Core
MIT License
234 stars 27 forks source link

.NET 6 - EnsureCreated creates all fields nullable #120

Open Ciantic opened 2 years ago

Ciantic commented 2 years ago

Describe the bug I'm using .NET 6, so this might be something new. With C# I get all fields as NOT NULL, but with F# all fields are nullable. Code is practically same, here is F# code:

E.g. F# code:

open System
open Microsoft.AspNetCore.Builder
open Microsoft.Extensions.Hosting
open Microsoft.Extensions.DependencyInjection
open Microsoft.AspNetCore.Hosting
open Microsoft.EntityFrameworkCore 
open Microsoft.EntityFrameworkCore.Sqlite
open EntityFrameworkCore.FSharp.Extensions

[<CLIMutable>]
type Person =
    { Id : Guid
      FirstName : string
      LastName : string
      Address : string
      City : string}

type AppDbContext(opts) =
    inherit DbContext(opts)

    // member this.Persons : DbSet<Person> = System.Linq.Set<Person>()

    [<DefaultValue>]
    val mutable persons : DbSet<Person> 

    override _.OnModelCreating builder =
        builder.RegisterOptionTypes() // enables option values for all entities

    member public this.Persons with get() = this.persons
                               and set p = this.persons <- p

[<EntryPoint>]
let main args =

    let builder = WebApplication.CreateBuilder(args)

    builder.Services
            .AddDbContext<AppDbContext>(fun opts -> opts.UseSqlite("Data Source=foo.sqlite") |> ignore)

    let app = builder.Build()

    app.UseDeveloperExceptionPage() |> ignore
    app.UseRouting() |> ignore
    app.MapGet("/", Func<string>(fun () -> "Hello World!")) |> ignore

    let createDb =
        use scope = app.Services.CreateScope()
        let db = scope.ServiceProvider.GetRequiredService<AppDbContext>()
        db.Database.EnsureDeleted() |> Console.WriteLine
        db.Database.EnsureCreated() |> Console.WriteLine

    createDb

    app.Run()

    0 // Exit code

I get this in the command line as I start:

CREATE TABLE "Persons" (
          "Id" TEXT NOT NULL CONSTRAINT "PK_Persons" PRIMARY KEY,
          "FirstName" TEXT NULL,
          "LastName" TEXT NULL,
          "Address" TEXT NULL,
          "City" TEXT NULL
      );

Expected behavior

CREATE TABLE "Persons" (
          "Id" TEXT NOT NULL CONSTRAINT "PK_Persons" PRIMARY KEY,
          "FirstName" TEXT NOT NULL,
          "LastName" TEXT NOT NULL,
          "Address" TEXT NOT NULL,
          "City" TEXT NOT NULL
      );

It's notable that both projects (C# csproj and F# fsproj) project has these settings:

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

I suspect the C# somehow configs everything non-nullable because of that, but I can't replicate this in F# side.

simon-reynolds commented 2 years ago

Hi @Ciantic Thank you for the feedback, I agree that the TEXT columns should be non-nullable, the intention here is that any record property that isn't a nullable or option type should be non-null by default.

Ciantic commented 2 years ago

@simon-reynolds I was just wondering how these can even differ between C# and F#? The C# must be pre-configuring the EF somehow, I'd like to configure F# similarily so that all fields are NOT NULL.

I suspect the reason EF does this now, is because it detects <Nullable>enable</Nullable> in the csproj and enables non-nullability.

I want same behavior in F#, do you know how?

Thanks.

simon-reynolds commented 2 years ago

You can get the same behaviour for now by using Attributes on the Person record like so

[<CLIMutable>]
type Person =
    { Id : Guid
      [<Required>]
      FirstName : string
      [<Required>]
      LastName : string
      [<Required>]
      Address : string
      [<Required>]
      City : string}

I'm looking now at how to define this behaviour by default and hope to have a fix for this in the coming days

LiteracyFanatic commented 2 years ago

I recently refactored a project of mine to use the following snippet in OnModelCreating instead of manually configuring each string property.

for entity in modelBuilder.Model.GetEntityTypes() do
    for property in entity.ClrType.GetProperties() do
        if property.GetType() = typeof<string> then
            modelBuilder.Entity(property.DeclaringType)
                .Property(property.PropertyType, property.Name)
                .IsRequired()
                |> ignore

Could something like this be included by default when calling modelBuilder.RegisterOptionTypes()?

jing8956 commented 2 years ago

Hi @simon-reynolds Version 6.0.6 seems to have implemented this feature, but a new problem arises: how to make the existing string instead of the Option<string> column can be null?

This problem arises from the fact that: The entity model defines by C# code that cannot be modified by myself, such as Microsoft.AspNetCore.Identity.IdentityUser.

I see that all string type properties generate not null type columns. Then when registering a user, the database reports an error because PhoneNumber is null.

I've tried using IsRequired(false), but no effect.