efcore / EFCore.FSharp

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

Unable to access record fields via lambdas via EntityTypeBuilder #67

Closed zelenij closed 3 years ago

zelenij commented 3 years ago

Describe the bug

Unable to access record fields via lambdas via EntityTypeBuilder - fails at run time during migration.

To Reproduce Steps to reproduce the behavior:

Create a IEntityTypeConfiguration class for a record EF Core F# type. Inside Configure(builder: EntityTypeBuilder try to access a property via lambda: builder.Property(fun x -> x.FieldName) Register this new IEntityTypeConfiguration in DbContext Compile the projects (success) Try to add a new migration

Observe the following exception:

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.ArgumentException: The expression 'delegateArg0 => value(Omnicv.DataModule.CoreData+userIdColumn@45).Invoke(delegateArg0)' is not a valid member access expression. The expression should represent a simple property or field access: 't => t.MyProperty'. (Parameter 'memberAccessExpression')
at Microsoft.EntityFrameworkCore.Infrastructure.ExpressionExtensions.GetInternalMemberAccess[TMemberInfo](LambdaExpression memberAccessExpression)
at Microsoft.EntityFrameworkCore.Infrastructure.ExpressionExtensions.GetMemberAccess(LambdaExpression memberAccessExpression)
at Microsoft.EntityFrameworkCore.Metadata.Builders.EntityTypeBuilder1.Property[TProperty](Expression1 propertyExpression)

Expected behavior A new migration created successfully

zelenij commented 3 years ago

I did some further digging and it "seems" like converting 'a -> 'b function to Experession<Func<'a, 'b>> should be enough to achieve the goal, but doesn't seem to work. I have the following conversion code now:

let getNameFromBuilder (builder: EntityTypeBuilder<'a>) (extractor: 'a -> 'b) =
    let expr = <@ Func<'a, 'b>(extractor) @>
    let res = 
        QuotationToExpression expr
        |> unbox<Expression<Func<'a, 'b>>>
    builder.Property(res)

And it compiles fine, but the migration command dies with the same exception.

simon-reynolds commented 3 years ago

Hi @zelenij Is the record marked with a [<CliMutable>] attribute?

That will cause it to behave similarly to a C# class under the hood

zelenij commented 3 years ago

Yes, it is. Here is an example (getNameFromBuilder from the previous comment):

[<CLIMutable>]
type CoreEventInfo =
    {
        Id: int64
        User: User
        CreatedAt: DateTime
        LastUpdated: DateTime
    }

type CoreEventInfoConfiguration() =
    interface IEntityTypeConfiguration<CoreEventInfo> with
        member this.Configure(builder: EntityTypeBuilder<CoreEventInfo>) = 
            let userIdColumn = getNameFromBuilder builder (fun x -> x.User)
            let eventIdColumn = getNameFromBuilder builder (fun x -> x.Id)
            builder.HasIndex(userIdColumn, eventIdColumn)
            |> ignore
simon-reynolds commented 3 years ago

Thanks, I'll take a look and see what's going on Is there anything particular going on in the User type?

zelenij commented 3 years ago

Not really. For the sake of testing, feel free to drop it from the index and the record itself, it's not important.

I think it's something to do with the (fun o -> o.Prop) not being "simple enough" for the reflection part of the EF Core code.

simon-reynolds commented 3 years ago

I'm having difficulty getting the call to builder.HasIndex to compile

Never mind, working on it now

simon-reynolds commented 3 years ago

The IndexExperession is of type Expression<Func<T, object>> so you need to cast the result to obj

Can you try this and let me know if it works? (I replaced User with UserId of type int64 in your example)

type CoreEventInfoConfiguration() =
    interface IEntityTypeConfiguration<CoreEventInfo> with
        member this.Configure(builder: EntityTypeBuilder<CoreEventInfo>) = 
            builder.HasIndex(fun x -> (x.UserId, x.Id) :> obj) |> ignore
zelenij commented 3 years ago

Fair enough, this seems to be working, thanks!