Efcore: model builder lambda APIs don't work with public fields on entities

[maumar]

repro:

        [Fact]
        public virtual void Field_key_test()
        {
            using (var ctx = new EntityWithFieldKeyContext())
            {
                ctx.Database.EnsureDeleted();
                ctx.Database.EnsureCreated();
            }
        }

        public class EntityWithFieldKey
        {
            public long Id;
        }

        public class EntityWithFieldKeyContext : DbContext
        {
            public DbSet<EntityWithFieldKey> MyEntities { get; set; }

            protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
            {
                optionsBuilder.UseSqlServer(@"Server=.;Database=Repro_field_key;Trusted_Connection=True;MultipleActiveResultSets=True");
            }

            protected override void OnModelCreating(ModelBuilder modelBuilder)
            {
                modelBuilder.Entity<EntityWithFieldKey>().Property(e => e.Id);
                modelBuilder.Entity<EntityWithFieldKey>().HasKey(e => e.Id);
            }
        }

throws:

System.ArgumentException : The expression 'e => e.Id' is not a valid property expression. The expression should represent a simple property access: 't => t.MyProperty'.
Parameter name: propertyAccessExpression
    at Microsoft.EntityFrameworkCore.Internal.ExpressionExtensions.GetPropertyAccess(LambdaExpression propertyAccessExpression)
    at Microsoft.EntityFrameworkCore.Metadata.Builders.EntityTypeBuilder`1.Property[TProperty](Expression`1 propertyExpression)
    at Microsoft.EntityFrameworkCore.Query.QueryBugsTest.EntityWithFieldKeyContext.OnModelCreating(ModelBuilder modelBuilder)
    at Microsoft.EntityFrameworkCore.Infrastructure.ModelCustomizer.Customize(ModelBuilder modelBuilder, DbContext context)
    at Microsoft.EntityFrameworkCore.Infrastructure.RelationalModelCustomizer.Customize(ModelBuilder modelBuilder, DbContext context)
    at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.CreateModel(DbContext context, IConventionSetBuilder conventionSetBuilder, IModelValidator validator)
    at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.<>c__DisplayClass5_0.<GetModel>b__1()
    at System.Lazy`1.CreateValue()
    at System.Lazy`1.LazyInitValue()
    at System.Lazy`1.get_Value()
    at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.GetModel(DbContext context, IConventionSetBuilder conventionSetBuilder, IModelValidator validator)
    at Microsoft.EntityFrameworkCore.Internal.DbContextServices.CreateModel()
    at Microsoft.EntityFrameworkCore.Internal.DbContextServices.get_Model()
    at Microsoft.EntityFrameworkCore.Infrastructure.EntityFrameworkServicesBuilder.<>c.<TryAddCoreServices>b__7_1(IServiceProvider p)
    at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitFactory(FactoryCallSite factoryCallSite, ServiceProviderEngineScope scope)
    at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(IServiceCallSite callSite, TArgument argument)
    at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitScoped(ScopedCallSite scopedCallSite, ServiceProviderEngineScope scope)
    at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(IServiceCallSite callSite, TArgument argument)
    at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitConstructor(ConstructorCallSite constructorCallSite, ServiceProviderEngineScope scope)
    at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(IServiceCallSite callSite, TArgument argument)
    at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitScoped(ScopedCallSite scopedCallSite, ServiceProviderEngineScope scope)
    at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(IServiceCallSite callSite, TArgument argument)
    at Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine.<>c__DisplayClass1_0.<RealizeService>b__0(ServiceProviderEngineScope scope)
    at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngine.GetService(Type serviceType, ServiceProviderEngineScope serviceProviderEngineScope)
    at Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngineScope.GetService(Type serviceType)
    at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService(IServiceProvider provider, Type serviceType)
    at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService[T](IServiceProvider provider)
    at Microsoft.EntityFrameworkCore.DbContext.get_DbContextDependencies()
    at Microsoft.EntityFrameworkCore.DbContext.get_InternalServiceProvider()
    at Microsoft.EntityFrameworkCore.DbContext.Microsoft.EntityFrameworkCore.Infrastructure.IInfrastructure<System.IServiceProvider>.get_Instance()
    at Microsoft.EntityFrameworkCore.Infrastructure.DatabaseFacade.Microsoft.EntityFrameworkCore.Infrastructure.IInfrastructure<System.IServiceProvider>.get_Instance()
    at Microsoft.EntityFrameworkCore.Internal.InternalAccessorExtensions.GetService[TService](IInfrastructure`1 accessor)
    at Microsoft.EntityFrameworkCore.Infrastructure.AccessorExtensions.GetService[TService](IInfrastructure`1 accessor)
    at Microsoft.EntityFrameworkCore.Infrastructure.DatabaseFacade.get_DatabaseCreator()
    at Microsoft.EntityFrameworkCore.Infrastructure.DatabaseFacade.EnsureDeleted()
    at Microsoft.EntityFrameworkCore.Query.QueryBugsTest.Field_key_test()

[smitpatel]

We match with PropertyInfo explicitly which in this case FieldInfo so no match causing to fail like that.
https://github.com/aspnet/EntityFrameworkCore/blob/80506bcc0cb89c8c66028b5be39a237c1d8149e5/src/EFCore/Extensions/Internal/ExpressionExtensions.cs#L173-L188

[ilmax]

I had a quick look at this, the change is straightforward but a lot of code is using PropertyInfo. If we allow expression with public fields, we should probably change a lot of code to work with MemberInfo instead, so this enhancement can be a quite big change. Any idea on this?

[divega]

@ilmax It is unlikely we would take this for 2.1 at this stage anyway. If it is for the next release, how big the change is shouldn’t be as concerning.

[ilmax]

@divega Thank for the info. I just wanted to point out that to enable this one it might require a fairly large change. I wish we had union type in C# for this one 😃

[ilmax]

@divega I would like to give this a try

[divega]

@ilmax sounds good.

[ajcvickers]

@ilmax It shouldn't be a huge change--not every place that references a PropertyInfo will need to be changed. A bit of background: any EF "property" (navigation or regular) can have:

• Just a PropertyInfo - indicates the EF "property" is mapped to a CLR property and no backing field is known
• Both a PropertyInfo and a FieldInfo - indicates the EF "property" is mapped to a CLR property with an associated backing field
• Just a FieldInfo - indicates that the EF "property" is mapped to a CLR field only--that is the field is not a backing field for a property
• Neither PropertyInfo or FieldInfo - only valid for shadow properties

All these cases can already be mapped and all the consuming code should handle all these cases.(Although there may be bugs, of course.) When specifying a field it must currently be specified as a string, rather than though a lambda expression. So this issue is only about allowing the fluent API to be used with fields as well as properties, which should not be a huge change.

[ilmax]

@ajcvickers Agree, in the end is not that big, I have made some changes and now it works, if you want, I can open a PR.
I still have some points to clarify, mostly what methods (on Entity/QueryTypeBuilder) should accept expression of either properties or fields, and what's the best place to put the tests, it took me some time to figure it out the best spot and I'm not sure the one I choose is the correct one.

[ralmsdeveloper]

@ajcvickers this should be closed?

[ajcvickers]

@ralmsdeveloper Why?

[ralmsdeveloper]

I believed that the PR of @ilmax referenced above had resolved.

[ilmax]

@ralmsdeveloper actually not because the PR was not merged due to lacking of test coverage (I still plan to re work this one for 3.0)

The pain point for adding new test is that visual Studio is fighting me, so debugging a single test is almost impossible (at least for me) using the built-in test runner.

Also I've tried recently to give this another go, but launching vs using the preview of dotnet core crashes my vs instance (15.8.latest) systematically every time.

To sum up I can say that sometimes I'm a bit discouraged to contribute to EF because it's not trivial to setup a proper development environment moreover (a small one here) running tests mess up localdb databases and the cleanup command is to eager and drops also my project DB

[ralmsdeveloper]

@ilmax I've been using http://www.testdriven.net/ and so far it's been great for my tests.

Shortly after @ajcvickers ask the question, I realized that the PR really was not merged.

[ilmax]

@ralmsdeveloper I'll give it a try! I still think there's a bit of room for improvement here

[smitpatel]

@ilmax - Sorry that you are having difficulties in getting dev env working to contribute. Given where master branch is, using preview .net core would be necessary. But I haven't seen any VS crash so far. Can you describe steps you have used in order to use preview SDK in VS?
Further there are multiple ways to run single test.

• VS Test explorer (It works for me in 15.9 Preview version at least)
• Test Driven (It is bit manual to set it up but it is the best experience you can get.
• Dotnet test with a variety of command line args can make single test run and perhaps debug in VS too.

Let me know any way we can help you out.

[ajcvickers]

@ilmax My workflow to get started again after doing other things for a few days is:

• Clear my NuGet caches. (Always good to do this periodically when working with pre-release bits, but we rarely see the issues we had in the past.)
• git clean -xdf on the repo clone
• build /t:Package to ensure the preview runtime is installed and get the build working
• Open the solution in my favorite IDE and go party

[ilmax]

@smitpatel, @ajcvickers Thank you for the tips, I will try to install VS preview to see if it works well, and will report here any issues (if any).

Thanks!

[ilmax]

@smitpatel I'm having some trouble opening the solution from the latest version of VS preview

Microsoft Visual Studio Professional 2017 Preview
Version 15.9.0 Preview 4.0
VisualStudio.15.Preview/15.9.0-pre.4.0+28219.56
Microsoft .NET Framework
Version 4.7.03190

Installed Version: Professional

C# Tools   2.10.0-beta2-63410-10+45b371170fd8f9f3e9c14ea2a71d17db9469f8b7

Essentially all the projects are almost empty, they only contains the shared code

And in the error list I got ~55 error like the following one:

Error Project "C:\Users\MassimilianoDonini\.dotnet\x64\sdk\2.2.100-preview2-009404\Sdks\Microsoft.NET.Sdk\Sdk\Sdk.props" was not imported by "c:\Code\Tests\MyEfCore\test\EFCore.Tests\EFCore.Tests.csproj" at (0,0), due to the file being invalid. EFCore.Tests C:\Users\MassimilianoDonini\.dotnet\x64\sdk\2.2.100-preview2-009404\Sdks\Microsoft.NET.Sdk\Sdk\Sdk.props 1

Note that build /t:Compile is compiling the project just fine, but VS is not able to correctly load the projects.

I've tried to execute a git clean -xid and also a dotnet nuget locals all --clear but still not able to properly load the project.
Using the current version of Visual Studio 15.8.8 with ReShaper installed it crashes after a couple (20) seconds, but the projects doesn't load properly.

I'm lunching VS from a command prompt this way:

build /t:Compile
set PATH=%USERPROFILE%\.dotnet\x64;%PATH%
"C:\Program Files (x86)\Microsoft Visual Studio\Preview\Professional\Common7\IDE\devenv.exe"

Any help is highly appreciated :)

[ajcvickers]

@ilmax Does a path change like that get picked up by VS? Try changing it Windows settings.

[ilmax]

@ajcvickers It was working for me time ago, I will try to set it at machine level (tomorrow) and will get back to you.

Thanks

[ajcvickers]

@ilmax Another thing to look for is any environment variables that might impact the way VS runs. It used to be necessary to set some of these things, but now it is not.

[smitpatel]

@ilmax - Those steps should work. Another step to take apart from above would be (based on error message), delete %userprofile%/.dotnet folder since it seems that downloaded preview SDK is in corrupt state.

[ilmax]

@smitpatel, @ajcvickers removing the %userprofile%/.dotnet did the trick, still unsure how the command line was able to build it succesfully, but this is another story.
Thank you for the tips

[lajones]

Note: thanks to @ilmax for the initial PR on this: #11610 . This PR was based on that one.