Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InMemory - bad key management for derived entity types #19854

Closed
saliksaly opened this issue Feb 10, 2020 · 9 comments · Fixed by #20205
Closed

InMemory - bad key management for derived entity types #19854

saliksaly opened this issue Feb 10, 2020 · 9 comments · Fixed by #20205
Labels
area-in-memory closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@saliksaly
Copy link

saliksaly commented Feb 10, 2020

Using in-memory database, ef core 3.1. (or 3.0)
Having abstract class Animal and two derived classes: Cat and Dog.
All are registered as entities in DbContext.
Seeding 2 Cat entities in DbContext.OnModelCreating.

1st problem:
When inserting a new Cat entity to DbSet, receiving error: "ArgumentException: An item with the same key has already been added. Key: 1".
The Id for the new entity shuld be 3 because there are already two.

2nd problem:
When inserting a new Dog entity to DbSet, it behaves inappropriately: replaces Cat entity with id 1 with the new Dog entity and adds new Dog entity with id 1!

Steps to reproduce

Here is asp net core demo app to demonstarte the problem described:
https://github.com/saliksaly/aspnetcoreapp-efcore-inherited-entity-id-problem

Further technical details

EF Core version: 3.1.0
Database provider: Microsoft.EntityFrameworkCore.InMemory
Target framework: .NET Core 3.1
Operating system: Win10 v. 1809
IDE: Visual Studio Community 2019 16.4.4

@ajcvickers
Copy link
Contributor

@saliksaly I was able to reproduce this and it does look like a bug.

Note for team: it looks like the code that peeks into the in-memory database to decide what key value to use is not working correctly when there is inheritance mapping.

@ajcvickers ajcvickers self-assigned this Feb 14, 2020
@ajcvickers ajcvickers added this to the 5.0.0 milestone Feb 14, 2020
@saliksaly
Copy link
Author

saliksaly commented Feb 17, 2020

Hi, would not be there any workaround until 5.0.0 version?
Maybe something similar to DbContextExtensions.ResetValueGenerators in #6872?

@ajcvickers
Copy link
Contributor

@saliksaly Just wanted to let you know that I have started investigating a workaround, but it may take me a while.

ajcvickers added a commit that referenced this issue Mar 6, 2020
Fixes #19854

Because in-memory isn't "TPH" and so derived types have their own "table" but the value generator is stored on the "table" that the property is defined in.
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Mar 6, 2020
@ajcvickers
Copy link
Contributor

ajcvickers commented Mar 6, 2020

@saliksaly I have a PR out to fix this for EF Core 5.0. Here's a workaround for 3.1:

private static void BumpGenerator(
    DbContext context, Type entityClrType, string propertyName, int newLowBound)
{
    var entityType = context.Model.FindEntityType(entityClrType);
    var property = entityType.FindProperty(propertyName);
    var options = context.GetService<IDbContextOptions>().FindExtension<InMemoryOptionsExtension>();
    var inMemoryStore = context.GetService<IInMemoryStoreCache>().GetStore(options.StoreName);
    var generator = inMemoryStore.GetIntegerValueGenerator<int>(property);
    
    var values = new object[entityType.GetDeclaredProperties().Count()];
    values[property.GetIndex()] = newLowBound;
    generator.Bump(values);
}

Use it like this:

BumpGenerator(context, typeof(AnimalBase), nameof(AnimalBase.Id), 2);

context.Add(
    new Cat
    {
        Name = "Tom",
    });

Passing 2 here means that the next value generated will be 3.

Note: this workaround uses EF internal code. This code will change in new releases of EF Core and the workaround will likely break. Use only with EF Core 3.1.

ajcvickers added a commit that referenced this issue Mar 16, 2020
Fixes #19854

Because in-memory isn't "TPH" and so derived types have their own "table" but the value generator is stored on the "table" that the property is defined in.
ajcvickers added a commit that referenced this issue Mar 19, 2020
Fixes #19854

Because in-memory isn't "TPH" and so derived types have their own "table" but the value generator is stored on the "table" that the property is defined in.
ajcvickers added a commit that referenced this issue Mar 19, 2020
Fixes #19854

Because in-memory isn't "TPH" and so derived types have their own "table" but the value generator is stored on the "table" that the property is defined in.
@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-preview3 Mar 31, 2020
@ShoSashko
Copy link

ShoSashko commented May 8, 2020

@ajcvickers
I just wanted to clarify. I have the same issue but with string as a key.
EF Core version: 3.1.0
Database provider: Microsoft.EntityFrameworkCore.InMemory
Target framework: .NET Core 3.1

{"An item with the same key has already been added. Key: [email protected]"}

Please let me know if there is any workaround?

@ajcvickers
Copy link
Contributor

@ShoSashko There is a workaround posted above.

@ShoSashko
Copy link

@ajcvickers Thanks for the quick response.

Not sure how I can apply this workaround for my specific case, having a string as a PK.
I've tried to change the workaround posted above from int to string

private static void BumpGenerator(
    DbContext context, Type entityClrType, string propertyName, string newLowBound)
        {
            var entityType = context.Model.FindEntityType(entityClrType);
            var property = entityType.FindProperty(propertyName);
            var options = context.GetService<IDbContextOptions>().FindExtension<InMemoryOptionsExtension>();
            var inMemoryStore = context.GetService<IInMemoryStoreCache>().GetStore(options.StoreName);
            var generator = inMemoryStore.GetIntegerValueGenerator<string>(property);

            var values = new object[entityType.GetDeclaredProperties().Count()];
            values[property.GetIndex()] = newLowBound;
            generator.Bump(values);
        }

But it throwing exception of the generator.Bump.

I have doubts that it will work anyway, there is no equal method for GetIntegerValueGenerator for the string.

Please let me know if I am even on the right track.

@ajcvickers
Copy link
Contributor

@ShoSashko No, this code doesn't make sense for strings, but then usually automatically generating string key values also doesn't make sense. You may be running into this breaking change.

saliksaly pushed a commit to saliksaly/aspnetcoreapp-efcore-inherited-entity-id-problem that referenced this issue Jun 11, 2020
@saliksaly
Copy link
Author

saliksaly commented Jun 11, 2020

I have applied the fix to the demo app:
https://github.com/saliksaly/aspnetcoreapp-efcore-inherited-entity-id-problem

I works now, no problems.

jiaguilera pushed a commit to jiaguilera/a-walk-in-graphql that referenced this issue Jul 22, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0-preview3, 5.0.0 Nov 7, 2020
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-in-memory closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants