InMemory: Improve in-memory key generation

[ardalis]

The Issue

For testing purposes, you should be able to delete, recreate, and reseed InMemory databases and the result should be the same for each test. Currently identity columns do not reset, so IDs increment with each test iteration.

Repro Steps

This test fails. It should pass.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.DependencyInjection;
using Xunit;

namespace EfCoreInMemory
{
    public class Item
    {
        public int Id { get; set; }
        public string Name { get; set; }
    }
    public class AppDbContext : DbContext
    {
        public DbSet<Item> Items { get; set; }

        public AppDbContext(DbContextOptions<AppDbContext> options ):base(options)
        {
        }
    }
    public class Tests
    {
        private static DbContextOptions<AppDbContext> CreateNewContextOptions()
        {
            // Create a fresh service provider, and therefore a fresh 
            // InMemory database instance.
            var serviceProvider = new ServiceCollection()
                .AddEntityFrameworkInMemoryDatabase()
                .BuildServiceProvider();

            // Create a new options instance telling the context to use an
            // InMemory database and the new service provider.
            var builder = new DbContextOptionsBuilder<AppDbContext>();
            builder.UseInMemoryDatabase()
                   .UseInternalServiceProvider(serviceProvider);

            return builder.Options;
        }

        [Fact]
        public void Test1()
        {
            // create a brand new dbContext
            var dbContext = new AppDbContext(CreateNewContextOptions());

            // add one item
            var item = new Item();
            dbContext.Items.Add(item);
            dbContext.SaveChanges();

            // ID should be 1
            Assert.Equal(1, item.Id);

            dbContext.Database.EnsureDeleted();

            Assert.False(dbContext.Items.Any());

            var item2 = new Item();
            dbContext.Items.Add(item2);
            dbContext.SaveChanges();

            // ID should STILL be 1
            Assert.Equal(1, item2.Id);

        }
    }
}

Further technical details

Project.json:

{
  "version": "1.0.0-*",
  "testRunner": "xunit",
  "dependencies": {
    "NETStandard.Library": "1.6.0",
    "xunit": "2.2.0-beta2-build3300",
    "dotnet-test-xunit": "2.2.0-preview2-build1029",
    "Microsoft.AspNetCore.Server.Kestrel": "1.0.0",
    "Microsoft.AspNetCore.TestHost": "1.0.0",
    "Microsoft.EntityFrameworkCore": "1.0.0",
    "Microsoft.EntityFrameworkCore.InMemory": "1.0.0"
  },
  "frameworks": {
    "netcoreapp1.0": {
      "imports": [
        "dotnet5.6"
      ],
      "dependencies": {
        "Microsoft.NETCore.App": {
          "type": "platform",
          "version": "1.0.0"
        }
      }
    }
  }
}

VS2015

[robertmclaws]

I don't know if I agree with this exact behavior, mostly because I'm having a hard time envisioning a unit test or operational scenario where you'd want to delete an existing item, then add a new item with the same ID.

HOWEVER, I do see the need for a command like dbContext.ResetIdTracking() that frees up any IDs not presently in the change tracker. You could then in this command at the end of each test function to reset the context for the next test.

[ardalis]

The repel shows one test. The problem exhibits itself when you have many tests. Every test will end up with different IDs - there is now way to really reset the in memory db.

Steve Smith

On Oct 26, 2016, at 15:26, Robert McLaws (Microsoft MVP) [email protected] wrote:

I don't know if I agree with this exact behavior, mostly because I'm having a hard time envisioning a unit test or operational scenario where you'd want to delete an existing item, then add a new item with the same ID.

HOWEVER, I do see the need for a command like dbContext.ResetIdTracking() that frees up any IDs not presently in the change tracker. You could then in this command at the end of each test function to reset the context for the next test.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[ardalis]

repro not repel

[ajcvickers]

Note for triage: dupe of #4096.

[rowanmiller]

The InMemory provider doesn't the Identity pattern you would get in a relational database. When a column is configured as ValueGeneratedOnAdd it uses values from a single generator for all tables. This would be similar to having all tables setup to use a single sequence in a relational database. This raises one of the important design principles of our InMemory provider, which is that we are not trying to emulate a relational database. This becomes more important as we start to light up non-relational providers.

If you want to have a database that acts like a relational database, but doesn't have the overhead of I/O, then we'd recommend using an In-Memory SQLite database - http://www.sqlite.org/inmemorydb.html.

We're going to update our testing docs to make the SQLite option more prominent.

[ardalis]

Thanks, @rowanmiller

2 things I'd like to see that I think would be useful:

• A way to 100% reset the in-memory database, to use in testing. I don't care whether it behaves like a relational database or not - for test purposes I want a way to reset its state completely.
• A testing sample that shows not just how to test EF directly, but how to test an app (like an ASP.NET Core app) that is using EF, resetting the database after each test. The current guidance on how to do this is not trivial to achieve using ASP.NET Core's standard Startup.ConfigureServices code used to configure EF.

Thanks!

[rowanmiller]

@ardalis fair point on point 1, I'm re-opening this issue for us to re-discuss.

Regarding point 2, our docs show an example that tests a BlogService, which would be the same as testing an MVC controller etc. - https://docs.efproject.net/en/latest/miscellaneous/testing.html. For testing, you generally wouldn't be using Startup.cs to setup all your services, you'd be creating your test doubles of everything the controller depends on and passing them in. The example I linked to shows how to do that for EF, but you'd have to do it for the other dependencies too.

[robertmclaws]

I do want to point out that I made a suggestion on how to handle what @ardalis is asking for in point 1 with a command like dbContext.ResetIdTracking(), or something to that effect.

[ardalis]

@rowanmiller You're right, unit testing an individual controller action works just like your BlogService sample. To clarify, I'm talking about integration testing using WebHostBuilder, as shown here: https://docs.asp.net/en/latest/testing/integration-testing.html. In this case, there's no direct access to the controller action. The web host is constructed for each test and the request is made such that it goes through the full MVC stack (routing, filters, etc). This is an awesome new feature of ASP.NET Core but getting the data reset for each test has proven much more difficult than I would like. To be fair, I haven't yet resorted to using SqlLite for this purpose, but my hope has been to avoid the need for this since it adds complexity to the process. Thanks.

[ardalis]

...and regarding that link, you'll see it doesn't include any EF examples. That's because I couldn't figure a great way to include them such that they were reset between each test.

[ardalis]

Here's a complete example showing how to (try to) configure an ASP.NET Core app using WebHostBuilder and InMemory for integration tests: https://github.com/ardalis/CleanArchitecture/blob/master/src/CleanArchitecture.Web/Startup.cs#L73-L104

The sample shown works, but starts to fall down as you add more entities and tests to it, as test data starts to step on other test data. One workaround I was able to use was hardcoding the IDs of test data, as suggested in another issue. However, any guidance or assistance with how to reset data for each integration test like this would be appreciated.

[rowanmiller]

@ardalis what if you used a throw away instance of the InMemory database for each test? There is an overload of UseInMemoryDatabase that takes a string (the named instance to connect to).

[ardalis]

I tried that but the numbers still incremented every time. I ended up working around it by hard-coding the initial ID. I had tried this:

        services.AddDbContext<AppDbContext>(options =>
            options.UseInMemoryDatabase(Guid.NewGuid().ToString()));

But even with that in place, and with disposing of the TestServer after every test, the ID numbers would continue incrementing between tests (which made it hard for me to construct API calls like /item/{id} when I couldn't count on a valid test ID to use).

Again this was fixed by specifying the ID in my test data method. A working version of what I have currently can be found here:
https://github.com/ardalis/ddd-guestbook/tree/Lab5Start

[rowanmiller]

Note for triage: One idea we had in triage was replacing some services to make the value generator resettable, but it involves copying a lot of code because the value generator is cached - and can't be replaced. You have to re-implement InMemoryValueGeneratorSelector to keep a handle of the created generators (so that you can reset them) and then re-implement InMemoryIntegerValueGeneratorFactory and InMemoryIntegerValueGenerator to make them resettable. You have to copy the code, as the public surface doesn't give you the hooks you need.

[ajcvickers]

@rowanmiller Does this work:

modelBuilder
    .Entity<Blog>()
    .Property(e => e.Id)
    .HasValueGenerator<InMemoryIntegerValueGenerator<int>>();

[ajcvickers]

@ardalis @robertmclaws Here is some code that might work for you:

public static class DbContextExtensions
{
    public static void ResetValueGenerators(this DbContext context)
    {
        var cache = context.GetService<IValueGeneratorCache>();

        foreach (var keyProperty in context.Model.GetEntityTypes()
            .Select(e => e.FindPrimaryKey().Properties[0])
            .Where(p => p.ClrType == typeof(int)
                        && p.ValueGenerated == ValueGenerated.OnAdd))
        {
            var generator = (ResettableValueGenerator)cache.GetOrAdd(
                keyProperty,
                keyProperty.DeclaringEntityType,
                (p, e) => new ResettableValueGenerator());

            generator.Reset();
        }
    }
}

public class ResettableValueGenerator : ValueGenerator<int>
{
    private int _current;

    public override bool GeneratesTemporaryValues => false;

    public override int Next(EntityEntry entry)
        => Interlocked.Increment(ref _current);

    public void Reset() => _current = 0;
}

To use, call context.ResetValueGenerators(); before the context is used for the first time and any time that EnsureDeleted is called. For example:

using (var context = new BlogContext())
{
    context.ResetValueGenerators();
    context.Database.EnsureDeleted();

    context.Posts.Add(new Post {Title = "Open source FTW", Blog = new Blog {Title = "One Unicorn"}});
    context.SaveChanges();
}

No matter how many times I call this code, Blog.Id and Post.Id will always get the value 1.

The code works by finding every int primary key in the model and setting the cached value generator to a ResettableValueGenerator, or resetting that value generator if it has already been created. It can be easily adapted for other key types.

[divega]

Moving to backlog to consider:

1. Automatically resetting value generators when in-memory database is dropped (value generators would need to be associated with the database instead of the model)
2. Add identity behavior on the in-memory database (and stop emulating it with client-side value generation)
3. Add an explicit API to reset value generation in the product.

[brockallen]

FYI I ran into this today where the state of the in-mem database is preserved across each run of my unit tests (and each test creates a whole new DI system per test with a new ServiceCollection, etc.). I can't imagine I'd ever want to preserve the old data across each test, since unit tests don't run in a consistent order, and they might run in parallel.

This behavior seems like a strange choice by default if this is being positioned for unit testing. Why isn't the in-mem database just a singleton when added to DI?

As a workaround I'm using Guid.NewGuid() to set the database name on each run, but it feels uncomfortable to have to do so.

[ErikEJ]

@brockallen The InMemory provider is not a relational database provider, current recommendation is to use SQLite (or maybe SQL Compact?) instead: https://docs.microsoft.com/en-us/ef/core/miscellaneous/testing/

[brockallen]

@ErikEJ I'm confused how that relates. I don't care what DB is used -- IMO, between tests it should start from scratch/empty.

[RichiCoder1]

bump
Ran into this ourselves. I'm also baffled that this is the default behavior. What is the intended use case of InMemory if not testing?

[spottedmahn]

@rowanmiller

We're going to update our testing docs to make the SQLite option more prominent.

I started with the SQLite user guide but quickly stopped once I learned schemas are not supported.

[TAGC]

I ran into this issue and was led down a rabbit hole that culminated in this thread. I agree with everyone else here - unit tests are supposed to be completely independent of each other and therefore able to run in any order without any change in test results. There absolutely should be a way to completely reset all EF Core state between unit tests for this reason, independent of what sort of database provider you're trying to simulate.

In my case, I'm trying to verify my application service logic by performing deep equality comparisons between entities. The persistence of primary key auto-generation state between unit tests is screwing up these comparisons when comparing the entity IDs:

 DeepEqual.Syntax.DeepEqualException : Comparison Failed: The following 3 differences were found.
	Actual[0].Id != Expected[0].Id (4 != 1)
	Actual[1].Id != Expected[1].Id (5 != 2)
	Actual[2].Id != Expected[2].Id (6 != 3)
Stack Trace:

[JRBonnema]

I tried to test this issue using 3.0.0 preview5 but I ran into the issue described in #14906 (Could not load type 'System.Collections.Generic.IAsyncEnumerable'1' from assembly 'System.Interactive.Async, Version=4.0.0.0), so in my case testing was not possible.

[denmitchell]

@DanNSam, I like your workaround. I encountered a similar issue and ended up creating a MaxPlusOneValueGenerator that works with Entities that implement IHasIntegerId (a public int Id property).

@ajcvickers, is Microsoft still looking at possible a long-term solution for this?

[ajcvickers]

@denmitchell As indicated by the Closed state and the closed-fixed label, this issue has been closed as fixed. As indicated by the milestone 3.0.0-preview4, it was first included in the EF Core 3.0 preview 4 release.

[eskensberg]

#6872 (comment) is great (thanks @ajcvickers), but... turns out that it does not work with tests running in parallel.

I've ended up fixing it by providing a custom IServiceProvider to UseInternalServiceProvider that declares IValueGeneratorCache as instance per test run.

There's an example building the custom IServiceProvider using Autofac. Autofac supports named scopes making it reasonably easy to set up:

public class Test : IDisposable
{
    static readonly Lazy<IContainer> container = new Lazy<IContainer>(() =>
    {
        var collection = new ServiceCollection()
            .AddEntityFrameworkInMemoryDatabase();

        // replacing IValueGeneratorCache lifetime from singleton to instance per test run
        var valueGeneratorCacheDescriptor = collection.First(x => x.ServiceType == typeof(IValueGeneratorCache));

        collection.Remove(valueGeneratorCacheDescriptor);

        var builder = new ContainerBuilder();
        builder.Populate(collection);

        builder
            .RegisterType(valueGeneratorCacheDescriptor.ImplementationType)
            .InstancePerMatchingLifetimeScope("testrun")
            .As(valueGeneratorCacheDescriptor.ServiceType);

        return builder.Build();
    });

    readonly ILifetimeScope scope;
    readonly DbContext dbContext;

    public Test()
    {
        scope = container.Value.BeginLifetimeScope("testrun");

        var serviceProvider = scope.Resolve<IServiceProvider>();

        var optionsBuilder = new DbContextOptionsBuilder<CoreDbContext>();
        optionsBuilder.UseInMemoryDatabase(Guid.NewGuid().ToString("D"));
        optionsBuilder.UseInternalServiceProvider(serviceProvider);

        dbContext = new DbContext(optionsBuilder.Options);
    }

    public void TestRun()
    {
        // ...
    }

    public void Dispose()
    {
        dbContext?.Dispose();
        scope?.Dispose();
    }
}

what would I go around using this snippet?
Do I have to wrap each test using it and run my logic inside the test?

[ajcvickers]

@eskensberg Can you file a new issue describing the problem when running tests in parallel?

[wmcainsh]

@DanNSam, I like your workaround. I encountered a similar issue and ended up creating a MaxPlusOneValueGenerator that works with Entities that implement IHasIntegerId (a public int Id property).

For anyone experiencing the auto-increment issue who doesn't use a column called Id on every table, I slightly modified the solution provided by @denmitchell

    public override int Next(EntityEntry entry)
    {
        var context = entry.Context;
        var qry = generic.Invoke(context, null) as DbSet<TEntity>;

        var key1Name = entry.Metadata
                            .FindPrimaryKey()
                            .Properties
                            .First()
                            .Name;

        var currentMax = qry.Max(e =>
            (int)e.GetType()
                  .GetProperty(key1Name)
                  .GetValue(e));

        //return max plus one
        return currentMax + 1;
    }

[ardalis]

This still is not working 100% for me when I try to reproduce the original behavior at the top of this issue. That is, trying to add an item, deleting the database, and then trying to add a separate new instance of the same entity. I get an InvalidOperationException saying the item is already being tracked:

The good news is that the new entity's ID is 1 as it should be. In my test I'm using the same dbContext before and after calling EnsureDeleted - maybe that's the unsupported scenario because the identitymap in the dbcontext still has a reference to the now-deleted entity from before I called EnsureDeleted.

Repro is here: https://github.com/ardalis/EFCoreFeatureTests/blob/ced23657c3c9257758d6734104fb3fc0f0562c25/EFCoreFeatureTests/UnitTest1.cs

[smitpatel]

@ardalis - Filed #18103 because cause of that issue is different from value generation.

[aherrick]

Is this broken in EF Core 3? Not sure I'm seeing this working and having trouble resetting my in memory DB. I'm wondering if similar to here:

#18187

[RobK410]

I see usage of TryAddSingleton in the bowels of the InMemory code. I think the problem as I can understand it, is that internally the in memory database functionality uses singletons. Are they not thread safe? Therefore, for parallel tests, or tests that execute multi-threaded, run risk of non-thread safe issues. Is this correct?

I found this to be the case when I wrote my own inmemory db abstraction for .NET Framework EF6

[AndriySvyryd]

@vasont That's likely a duplicate of #17672

[saliksaly]

@ardalis @robertmclaws Here is some code that might work for you:

public static class DbContextExtensions
{
    public static void ResetValueGenerators(this DbContext context)
    {
        var cache = context.GetService<IValueGeneratorCache>();

        foreach (var keyProperty in context.Model.GetEntityTypes()
            .Select(e => e.FindPrimaryKey().Properties[0])
            .Where(p => p.ClrType == typeof(int)
                        && p.ValueGenerated == ValueGenerated.OnAdd))
        {
            var generator = (ResettableValueGenerator)cache.GetOrAdd(
                keyProperty,
                keyProperty.DeclaringEntityType,
                (p, e) => new ResettableValueGenerator());

            generator.Reset();
        }
    }
}

public class ResettableValueGenerator : ValueGenerator<int>
{
    private int _current;

    public override bool GeneratesTemporaryValues => false;

    public override int Next(EntityEntry entry)
        => Interlocked.Increment(ref _current);

    public void Reset() => _current = 0;
}

To use, call context.ResetValueGenerators(); before the context is used for the first time and any time that EnsureDeleted is called. For example:

using (var context = new BlogContext())
{
    context.ResetValueGenerators();
    context.Database.EnsureDeleted();

    context.Posts.Add(new Post {Title = "Open source FTW", Blog = new Blog {Title = "One Unicorn"}});
    context.SaveChanges();
}

No matter how many times I call this code, Blog.Id and Post.Id will always get the value 1.

The code works by finding every int primary key in the model and setting the cached value generator to a ResettableValueGenerator, or resetting that value generator if it has already been created. It can be easily adapted for other key types.

@ajcvickers, please - I have been using your solution to set id value generators for my in-emory tests with seed data. Now, in EF Core 3.1, it stoped working. It seems that ResettableValueGenerator.Next() is newer called.
Again, I get error: System.InvalidOperationException: 'The instance of entity type X cannot be tracked because another instance with the key value '{Id: 1}' is already being tracked.

Would not you know solution for EF Core 3.1?

Thanks

[ajcvickers]

@saliksaly It should no longer be necessary to use a workaround here since the underlying issues were fixed in 3.0. If you're running into issues with in-memory keys, then please open a new issue and include a small, runnable project or complete code listing so that we can investigate.

[saliksaly]

@saliksaly It should no longer be necessary to use a workaround here since the underlying issues were fixed in 3.0. If you're running into issues with in-memory keys, then please open a new issue and include a small, runnable project or complete code listing so that we can investigate.

Thanks, here is the issue: #19854