Trying to understand why setting a entity to detached can alter the loaded classes

[JonPSmith]

Context on why I am asking this questions

When writing unit tests that use EF Core there is the problem that the setup of the database can hide problems in the EF Core code you are testing, especially missing .Includes. One way is to have one DbContext instance to set up the database and another DbContext instance to run the test. This works, but you have to have other variables to hold the key(s) of the setup data you want to use in the actual test.

I thought one way to use one DbContext in a unit test would be to clear the tracking snapshots by detaching all the tracked entities. i.e.

foreach (var entry in context.ChangeTracker.Entries())
    entry.State = EntityState.Detached;

I used this with when working on a client's system and I hit a problem. The rest of this issue describes the problem I encountered.

Problem

If a non-required relationship is created and written to the database and I set the State of the relationship to Detached, then the foreign key is set to null.

NOTE: I my client code I have also seen the navigational set to null, but I couldn't reproduce that in a simple example.

Steps to reproduce

1. simplified unit test

This unit test fails on the last line.

[Fact]
public void TestLinkEntitiesByRelationshipThenDetachSimplifiedOk()
{
    //SETUP
    var sqlOptions = SqliteInMemory.CreateOptions<RelDbContext>();
    using (var context = new RelDbContext(sqlOptions))
    {
        context.Database.EnsureCreated();

        var company = new Company();
        var myJob = new Job { MyCompany = company };
        context.Add(myJob);
        context.SaveChanges();

        //ATTEMPT
        context.Entry(company).State = EntityState.Detached;

        //VERIFY
        myJob.MyCompany.ShouldNotBeNull();
        myJob.CompanyId.ShouldNotBeNull();
    }
}

2. Unit test checking each part

Here is a more detailed unit test with the the lines that fail marked with //NEXT LINE FAILS

public void TestLinkEntitiesByRelationshipThenDetachOk()
{
    //SETUP
    var sqlOptions = SqliteInMemory.CreateOptions<RelDbContext>();
    using (var context = new RelDbContext(sqlOptions))
    {
        context.Database.EnsureCreated();
        var company = new Company();
        var myJob = new Job { MyCompany = company };
        context.Add(myJob);
        context.SaveChanges();
        //These pass
        var dbMyJob1 = context.Jobs.Include(c => c.MyCompany).First();
        dbMyJob1.MyCompany.ShouldNotBeNull();
        dbMyJob1.CompanyId.ShouldNotBeNull();

        //ATTEMPT
        context.Entry(company).State = EntityState.Detached;

        //VERIFY
        using (var context1 = new RelDbContext(sqlOptions))
        {
            //These pass
            var dbMyJob2 = context1.Jobs.Include(c => c.MyCompany).First();
            dbMyJob2.MyCompany.ShouldNotBeNull();
            dbMyJob2.CompanyId.ShouldNotBeNull();
        }
        var dbMyJob3 = context.Jobs.Include(c => c.MyCompany).First();
        dbMyJob3.MyCompany.ShouldNotBeNull(); 
        //NEXT LINE FAILS
        dbMyJob3.CompanyId.ShouldNotBeNull();
        myJob.MyCompany.ShouldNotBeNull();
        //NEXT LINE FAILS
        myJob.CompanyId.ShouldNotBeNull();
    }
}

As you can see from this some pretty odd things going on in that code! Different data returned depending on the DbContext.

Note: I tried it with SQL Server and it failed too.

Entity classes and the DbContext

public class Job
{
    public int Id { get; set; }

    public int? CompanyId { get; set; }
    public Company MyCompany { get; set; }
}

public class Company
{
    public int Id { get; set; }
}

public class RelDbContext : DbContext
{
    public DbSet<Job> Jobs { get; set; }

    public RelDbContext(DbContextOptions<RelDbContext> options)
        : base(options) { }
}

Further technical details

EF Core version: 3.1.1
Database provider: (e.g. Microsoft.EntityFrameworkCore.SqlServer and Sqlite 3.1.1)
Target framework: .NET Core 3.1
Operating system: Windows
IDE: (e.g. Visual Studio 2019 16.4.2)

[JonPSmith]

PS. I tried this on EF Core 2.1 and everything worked, i.e. it passed all the asserts.

I did have to add a .ToList() to the ...ChangeTracker.Entries() otherwise I got a "Collection was modified" exception.

[ajcvickers]

I thought one way to use one DbContext in a unit test would be to clear the tracking snapshots by detaching all the tracked entities.

This is not something that I would recommend, although I have come to accept that people are going to do it. :-) In my opinion its better to have tests that do setup in one context instance and then test in another context instance. This matches the one context per unit-of-work guideline much more closely that attempting to reset state. This is what I always show when posting examples.

But, as I said, people do what to do this. Issue #15577 would make this easier and safer. We are also looking into ways to make it easier to get a short-lived context instance from D.I. (#18575) but that's probably not relevant to your tests.

As well as being less clean than creating a new context instance, detaching also has some other issues:

• It's slow. Much slower than creating a new context instance. The context is required to iterate through its data structures to free various references to the entity.
• The context tries to maintain data sanity, which means there may be side-effects. Some of this is what you are running into, except see below.

All this being said, I think you are running into bugs here. In particular, take a look at #19911 which is a PR I intend to merge today (or at the weekend, based on how today is going!) and will hopefully make it into the March patch. The issues linked from this PR seem to cover cases like yours. If you could try your code with the daily build of 5.0 and verify that it no longer fails, then that would be great! You may also want to look into changing the default cascade and delete orphans timing as a workaround, as described in those issues.

Hope this helps! :-)

[JonPSmith]

Thanks @ajcvickers for taking the time to explain that. To elaborate why I was looking for a simpler way to separate the data setup to the actual test is because using two DbContexts often (always?) requires passing a key(s) in the outer level for the second part to use. For instance:

[Fact]
public void TestLinkEntitiesOk()
{
    //SETUP
    int pk;
    var options = //... setup database options. in-memory 
    using (var context = new MyContext(options))
    {
        context.Database.EnsureCreated();
        pk = ...setup data and return its primary key; //note: could be a few of these
    }
    using (var context = new MyContext(options))
    {
        //ATTEMPT
        //... run test that uses pk to pick up setup data

        //VERIFY
        //etc....
    }
}

This takes more time so I (and other) only do it when we think its needed (I am a working contractor and being fast is quite important to my client). Recently I missed some bugs because I didn't use a two-context approach, which was bad.

What I would looking for is a method which I would at ALL my unit tests involving the database. Then I would a) never miss a missing .Include etc. and b) its really quick/easy to add. For that reason I think issue #15577 could be a good fit.

Also, as you said, there is clearly a bug there. I have subscribed to #19911 and will track that and test.

[ajcvickers]

@JonPSmith We talked about this, but we really don't see how creating two context instances is significantly more work when writing tests than using a single context instance. With copy/paste and refactoring tools it should literally take only a few seconds more.

Also, when I'm writing tests I tend to avoid using the PK value unless I need to for the specific test I am writing. Often the data has some other unique value (e.g. username) that is not dynamically generated and therefore can be hard-coded into the test. Alternatively, when using the in-memory database, sometimes the key value can be explicitly set for the test even if it is generated when the app is running normally.

[JonPSmith]

@ajcvickers . OK, you and I have different approaches, but that's fine. I will, of course, look at #15577 when it comes out to see if its useful.

BTW. I haven't tried my code, which is one case of #19911, which against an nightly build, but I can at the weekend. How do I get the EF Core nightly build?

[ajcvickers]

@JonPSmith Ensure that the 3.1 SDK is installed then configure a NuGet feed: https://github.com/dotnet/aspnetcore/blob/master/docs/DailyBuilds.md

[JonPSmith]

OK, I tried that and all of my tests passed, apart from one that had another issue. I think this says that your update fixes my problem.

I don't think you are interested in the exception I had, but here it is. It applies to the unit test called 1. simplified unit test in my original comment.

System.TypeLoadException : Method 'ProcessModelFinalized' in type 'Microsoft.EntityFrameworkCore.Metadata.Conventions.SqlServerValueGenerationStrategyConvention' from assembly 'Microsoft.EntityFrameworkCore.SqlServer, Version=3.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60' does not have an implementation.
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.SqlServerConventionSetBuilder.CreateConventionSet()
   at Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal.RuntimeConventionSetBuilder.CreateConventionSet()
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.CreateModel(DbContext context, IConventionSetBuilder conventionSetBuilder)
   at Microsoft.EntityFrameworkCore.Infrastructure.ModelSource.GetModel(DbContext context, IConventionSetBuilder conventionSetBuilder)
   at Microsoft.EntityFrameworkCore.Internal.DbContextServices.CreateModel()
   at Microsoft.EntityFrameworkCore.Internal.DbContextServices.get_Model()
   at Microsoft.EntityFrameworkCore.Infrastructure.EntityFrameworkServicesBuilder.<>c.<TryAddCoreServices>b__7_3(IServiceProvider p)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitFactory(FactoryCallSite factoryCallSite, RuntimeResolverContext context)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSiteMain(ServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitCache(ServiceCallSite callSite, RuntimeResolverContext context, ServiceProviderEngineScope serviceProviderEngine, RuntimeResolverLock lockType)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitScopeCache(ServiceCallSite singletonCallSite, RuntimeResolverContext context)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(ServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitConstructor(ConstructorCallSite constructorCallSite, RuntimeResolverContext context)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSiteMain(ServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitCache(ServiceCallSite callSite, RuntimeResolverContext context, ServiceProviderEngineScope serviceProviderEngine, RuntimeResolverLock lockType)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitScopeCache(ServiceCallSite singletonCallSite, RuntimeResolverContext context)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(ServiceCallSite callSite, TArgument argument)
   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.Resolve(ServiceCallSite callSite, ServiceProviderEngineScope scope)
   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.Internal.InfrastructureExtensions.GetService[TService](IInfrastructure`1 accessor)
   at Microsoft.EntityFrameworkCore.Infrastructure.AccessorExtensions.GetService[TService](IInfrastructure`1 accessor)
   at Microsoft.EntityFrameworkCore.Infrastructure.DatabaseFacade.get_Dependencies()
   at Microsoft.EntityFrameworkCore.Infrastructure.DatabaseFacade.EnsureCreated()
   at Test.UnitTests.TestRelationshipDataLayer.TestRelationships.TestLinkEntitiesByRelationshipThenDetachSimplifiedSqlDatabaseOk() in C:\Users\Jon\Source\Repos\GetBestOutOfEfCore\Test\UnitTests\TestRelationshipDataLayer\TestRelationships.cs:line 77

The nightly build was 5.0.0-preview.2.20119.2

[ajcvickers]

@JonPSmith You're still referencing the 3.0 package for SQL Server: Microsoft.EntityFrameworkCore.SqlServer, Version=3.0.0.0.

[JonPSmith]

Yep. That was it. I hadn't added Microsoft.EntityFrameworkCore.SqlServer so it was using the Microsoft.EntityFrameworkCore.SqlServer, Version=3.0.0.0 from my EfCore.TestSupport library. Loading the nightly version of SQL Server fixed that.

I consider this issue closed and have set that. I hope that matches your view too.

[ajcvickers]

@JonPSmith Thanks for testing and thanks for the feedback as always. :-)