efcore
efcore copied to clipboard
dotnet
DbSet.Add causes failure if called after lazy loading related entity
When creating and adding a new entity using lazy loading proxies, the following code is used:
var newProduct = Context.Products.CreateProxy();
Context.Products.Add(newProduct);
Context.SaveChanges();
You probably also need to set name, price, description, section, etc. properties of the entity before saving changes.
It appears that - at least with lazy loading proxies in place - the order of calling the DbSet.Add method matters. The following example illustrates this:
On line 37 (in AddOrderLine1), the Add method is called right after creating the proxy, before setting properties (especially: before triggering lazy loading when querying the products catalog price). No issue here.
However, the same code (AddOrderLine2) but with the Add method call delayed to just before the SaveChanges (line 52) causes the program to fail (see exception details below):
using Microsoft.Data.Sqlite;
using Microsoft.EntityFrameworkCore;
using System.ComponentModel.DataAnnotations;
using System.ComponentModel.DataAnnotations.Schema;
public static class Program
{
static OrderContext Context = new OrderContext();
static int prod1Id;
public static void Main(string[] args)
{
Context.Database.EnsureDeleted();
Context.Database.EnsureCreated();
CreateProduct();
AddOrderLine1();
AddOrderLine2();
}
public static void CreateProduct()
{
// Create and store a product with Id=1:
var prod = Context.Products.CreateProxy();
prod.Name = "FooBar";
prod.CatalogPrice = 12.99m;
Context.Products.Add(prod);
Context.SaveChanges();
prod1Id = prod.Id;
Console.WriteLine(prod1Id);
}
public static void AddOrderLine1()
{
// Create an OrderLine for the product:
var line = Context.OrderLines.CreateProxy();
// And add the line immediately to the context:
/*37*/ Context.OrderLines.Add(line);
line.ProductId = prod1Id;
line.OrderPrice = line.Product.CatalogPrice;
Context.SaveChanges();
Console.WriteLine(line.OrderPrice);
}
public static void AddOrderLine2()
{
// Create an OrderLine for the product:
var line = Context.OrderLines.CreateProxy();
line.ProductId = prod1Id;
line.OrderPrice = line.Product.CatalogPrice;
// Add the line after lazy loading the product:
/*52*/ Context.OrderLines.Add(line); // <-- Fails here
Context.SaveChanges();
Console.WriteLine(line.OrderPrice);
}
}
public class OrderContext : DbContext
{
protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
{
var connection = new SqliteConnection("Filename=:memory:");
connection.Open();
optionsBuilder.UseSqlite(connection);
optionsBuilder.UseChangeTrackingProxies();
optionsBuilder.UseLazyLoadingProxies();
}
public virtual DbSet<OrderLine> OrderLines { get; set; }
public virtual DbSet<Product> Products { get; set; }
}
public class OrderLine
{
[Key]
public virtual int Id { get; set; }
public virtual int ProductId { get; set; }
[ForeignKey(nameof(ProductId))]
public virtual required Product Product { get; set; }
public virtual decimal OrderPrice { get; set; }
}
public class Product
{
[Key]
public virtual int Id { get; set; }
public virtual required string Name { get; set; }
public virtual decimal CatalogPrice { get; set; }
}
Target framework: .NET 8.0 Included package references:
<ItemGroup>
<PackageReference Include="Microsoft.EntityFrameworkCore" Version="8.0.1" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Proxies" Version="8.0.1" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="8.0.1" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Tools" Version="8.0.1">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
</ItemGroup>
Exception thrown:
System.InvalidOperationException
HResult=0x80131509
Message=The instance of entity type 'Product' cannot be tracked because another instance with the same key value for {'Id'} is already being tracked. When attaching existing entities, ensure that only one entity instance with a given key value is attached. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the conflicting key values.
Source=Microsoft.EntityFrameworkCore
StackTrace:
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IdentityMap`1.ThrowIdentityConflict(InternalEntityEntry entry)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IdentityMap`1.Add(TKey key, InternalEntityEntry entry, Boolean updateDuplicate)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IdentityMap`1.Add(TKey key, InternalEntityEntry entry)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.IdentityMap`1.Add(InternalEntityEntry entry)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.StartTracking(InternalEntityEntry entry)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState oldState, EntityState newState, Boolean acceptChanges, Boolean modifyProperties)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState entityState, Boolean acceptChanges, Boolean modifyProperties, Nullable`1 forceStateWhenUnknownKey, Nullable`1 fallbackState)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.PaintAction(EntityEntryGraphNode`1 node)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraph[TState](EntityEntryGraphNode`1 node, Func`2 handleNode)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraph[TState](EntityEntryGraphNode`1 node, Func`2 handleNode)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.AttachGraph(InternalEntityEntry rootEntry, EntityState targetState, EntityState storeGeneratedWithKeySetTargetState, Boolean forceStateWhenUnknownKey)
at Microsoft.EntityFrameworkCore.Internal.InternalDbSet`1.SetEntityState(InternalEntityEntry entry, EntityState entityState)
at Microsoft.EntityFrameworkCore.Internal.InternalDbSet`1.Add(TEntity entity)
at Program.AddOrderLine2() in C:\Stuff\EfCoreLazyAddOrder\EfCoreLazyAddOrder\Program.cs:line 52
at Program.Main(String[] args) in C:\Stuff\EfCoreLazyAddOrder\EfCoreLazyAddOrder\Program.cs:line 17
This behavious was also consistent using SQL Server as database.
The DbSet.Add method adds the object graph in added state to the context. Lazy loading however already attached (in unmodified state) the Product (as it loaded it through the context). IMHO, the DbSet.Add method should detect the Product entity already has an entry and should not create a new entry for this entity.
@codetuner , I would like to fix this. It seems the issue is that when we assign value to any property of the entity, the EF core is creating the actual entity.
I understand that in DbSet.Add method, put a check of being lazyLoading being enabled or not. If so, delete the existing virtual entity and add the newly passed entity.
However, I couldn't find DbSet.Add method. Any idea from where to start?
@RupenAnjaria By the DbSet.Add method I mean the following method: https://learn.microsoft.com/en-us/dotnet/api/microsoft.entityframeworkcore.dbset-1.add?view=efcore-8.0 It's base implementation on line 173 of https://github.com/RupenAnjaria/efcore/blob/main/src/EFCore/DbSet.cs is not implemented, so it is apparantly implement separately per provider. Yet I had the same issue with the SQLite provider as with the SQLServer provider.
That being said, IMHO the problem lays not in the DbSet Add method. Yes, the place where you call the method matters, as I showed here. But the original cause is in my opinion different (I insist on "my opinion" because I don't know the code at all, it's just based on logical deduction based on my limited understanding of EFCore): when doing "line.OrderPrice = line.Product.CatalogPrice;" (lines 39 and 50 resp.) the Product associated with the line is lazy loaded - hence instantiated.
In AddOrderLine2 this is done before the line is added/attached to the context. One would expect that lazy loading on an entity that is not attached to a context is not working: what context is being used ? what's the database connection and transaction the loazy loading occures in ?? It appears that when the line entity was created using CreateProxy, it must somehow already have been "connected" to a context (the context used to create the proxy I guess). This seems to me a design issue: either the line object is attached to a context, or not. If it is, then there should be no need to call the Add method. If it is not, then lazy loading should not work yet.
(First solution) This is also one way to solve the problem described above: do not allow lazy loading on an entity that is not attached to a context. It makes sense and would make AddOrderLine2 fail on line 50, which would be correct.
There is also another way to solve this. Therefore we need a more in depth understanding of the problem. Again: according to my understanding. When lazy loading of the "line.Product" property occures, an entity of Product is materialized and related to the line (via de Product property). It therefore needs a context and unless AsNoTracking was used, it is attached to that context. When the Add method is called, the line instance is attached to the context in "Added" state, and it does so for the whole object graph, so also for all object related to the line. Except that the product entity is originating from the database and should not be attached (as it alread is attached) in Added state. (Second solution) A possible solution would be that the Add method would make sure the whole object graph is attached, but would only set the state to Added when the concerned object was not yet attached to the context.
But maybe - and there you have my limited knowledge of how EFCore works internally - when doing "line.OrderPrice = line.Product.CatalogPrice;", although a context was used to load the product, that product was not actually attached to the context. In that case, my previous solution of not putting in Added state entities that are already attached would not be realizable. In that case my first solution would probably be better (most consistent). Or there should be a way to figure out that an entity of the graph does not need to be added when the Add method is called because it already exists in the database...
I believe this is not a "simple bug" but a design issue requiring a new design decision.
Kindest regards
@codetuner Proxies are associated with the context instance that created them unless they are tracked by a different context instance, in which case the proxy instance becomes associated with that context instance. The association is separate from whether or not an instance is tracked by the context, which is about keeping track of changes for when SaveChanges is called.
@ajcvickers Thank you for these insights.
In what I described the "second solution" the system would have to keep track of status (new or existing) of associated entities. Which either requires to do change tracking, or have a parallell/shadow change tracking status for merely associated (not attached) entities. Does not sound great. So maybe the first solution would be better: it's a restriction that throws exceptions (if you trigger lazy loading on an associated but not attached entity) but that ensures more consistency.
BTW I justed posted issue 33055 on the same codebase, but it's a quite different issue to my understanding.
@codetuner I think it's very unlikely that we will change these behaviors; this is all very much by design. See Change Tracking in the docs for more details.
@ajcvickers Correct. And the docs state that the "second solution" is the right one. My mistake. This is what the doc of DbSet.Add says: "Begins tracking the given entity, and any other reachable entities that are not already being tracked, in the Added state..."
While in this case, as the following version of AddOrderLine shows, the Product entity is already change tracked, and so according to the documentation it should not be added (and there should not be an exception saying that adding it fails):
public static void AddOrderLine3()
{
// Create an OrderLine for the product:
var line = Context.OrderLines.CreateProxy();
line.ProductId = prod1Id;
line.OrderPrice = line.Product.CatalogPrice;
// Shows that the Product entity is now change tracked:
foreach (var e in Context.ChangeTracker.Entries())
{
Console.WriteLine(e);
}
// Add the line after lazy loading the product:
Context.OrderLines.Add(line); // <-- Fails here
Context.SaveChanges();
Console.WriteLine(line.OrderPrice);
}
So it is a "simple bug" in the DbSet.Add method after all. My mistake.
I too have limited knowledge and if it's simple bug then I would like to contribute. Are we in agreement that "(Second solution) A possible solution would be that the Add method would make sure the whole object graph is attached, but would only set the state to Added when the concerned object was not yet attached to the context." is the right direction?
@RupenAnjaria For me yes: that's what the documentation of DbSet.Add says...
if Add method is provider specific then we have to update implementation for each provider?
Well, that's where you need to do some investigation...
I added the following code to see the exact type and hierarchy of DbSet:
var bc = Context.OrderLines.GetType();
while(bc != null)
{
Console.WriteLine(bc);
bc = bc.BaseType;
}
This is the output of the above code:
Microsoft.EntityFrameworkCore.Internal.InternalDbSet`1[OrderLine]
Microsoft.EntityFrameworkCore.DbSet`1[OrderLine]
System.Object
So it turns out it's not a "per provider" class, but an internal class. The implementation is all on the InternalDbSet<T> class, and that is on line 188 of https://github.com/RupenAnjaria/efcore/blob/main/src/EFCore/Internal/InternalDbSet.cs
Now, it's not an abvious error here. You will probably have to change code somewhere deeper. So you'll have to use my code to reproduce the case and with it debug into the EF code. BTW, the InternalDbSet.Add code was last changed by @ajcvickers, so he may be able to help you further better than I can.
Good luck!
@codetuner The problem with AddOrderLine3() is that the same context instance is being re-used, and it is already tracking the previous product instance. Consider using a new DbContext instance for each unit-of-work.
@ajcvickers Thanks. Correct.
But there's no difference between AddOrderLine3 and AddOrderLine2, apart from listing the tracked entities on the console.
The errormessage of both (as they are the same) says: "The instance of entity type 'Product' cannot be tracked because another instance with the same key value for {'Id'} is already being tracked."
Now, I couldn't create a context per unit of work (method in this case) with Sqlite in-memory db (the DB got lost). So I migrated the code to SQLServer. And created a context per method. Now I get the following error message on the SaveChanges() (line 54 in the original code): an EF DbUpdateException with inner exception "SqlException: Cannot insert explicit value for identity column in table 'Products' when IDENTITY_INSERT is set to OFF.". This is essantially the same error message as the previous one, but now served by the database: both error messages mean "I'm trying to create a new Product, but the Id is already taken".
What to think about the following solution: when the product is lazy loaded by line 50, there's a context used to lazy load it, and we didn't mention "AsNoTracking" (I don't think we even could), so should the entity, since it's been loaded by the context, not be tracked by the context ? If it would be tracked by the context, if would have the state Unchanged because it is not added but loaded and not modified so far. Then, when we call the DbSet.Add method to add the orderline, in accordance to the documentation, the product would not be added because it is already change-tracked.
That would solve it!
In that case it is not DbSet.Add that's the issue, but the lazy loading of entities: the loaded entities should always be change tracked by their context, whether the entity holding the lazy-loaded property is attached or merely associated to the context. How does that sound ?
The following example code demonstrates the same issue without using proxies, proving the issue is not specific to proxies.
This code uses SQLServer as backend. And every operation is in a separate context.
AddOrderline4 loads the Product with changetracking. This works fine.
AddOrderline5 does the same, but the Product is not tracked. This results in an error when calling SaveChanges (line 74).
AddOrderline6 uses explicit loading to load the Product. Again the product is not tracked. This results in an error when calling SaveChanges (line 87).
AddOrderline7 applies one solution: add the line to the context right after creating it (before associating it with the Product). This works.
AddOrderline8 applies another solution: the line is added to the context at the end, but just before that, the Product instance is Attached to the context. This also works.
IMO the best option is AddOrderline4: it's the most straight-forward code that correctly supports the object initializer syntax. It works because the Product is change tracked by default when using Find (as it is when using Single, SingleOrDefault, First, FirstOrDefault, Where,...). Having the entity attached is the default (you need to opt-out explicitely by using AsNoTracking if you don't want it).
The problem is that nor Explicit Loading nor Lazy Loading attaches the entities to the context. And these "loaded but unattached" entities cause the problem.
It is noteworthy that the Find method attaches the entity even if it was loaded but not attached yet. Have an entity loaded (without attaching it, i.e. by using AsNoTracking) and then Find that same entity and EFCore will roundtrip again to the database. (I guess because there is no list of untracked entities the Find method could search in).
using Microsoft.Data.Sqlite;
using Microsoft.EntityFrameworkCore;
using System.ComponentModel.DataAnnotations;
using System.ComponentModel.DataAnnotations.Schema;
public static class Program
{
public static void Main(string[] args)
{
int prod1Id;
using (var context = new OrderContext())
{
context.Database.EnsureDeleted();
context.Database.EnsureCreated();
prod1Id = CreateProduct(context);
}
using (var context = new OrderContext())
{
AddOrderline4(context, prod1Id);
}
using (var context = new OrderContext())
{
AddOrderline5(context, prod1Id);
}
using (var context = new OrderContext())
{
AddOrderline6(context, prod1Id);
}
using (var context = new OrderContext())
{
AddOrderline7(context, prod1Id);
}
using (var context = new OrderContext())
{
AddOrderline8(context, prod1Id);
}
}
public static int CreateProduct(OrderContext context)
{
// Create and store a product with Id=1:
var prod = new Product() { Name = "FooBar", CatalogPrice = 12.99m };
context.Products.Add(prod);
context.SaveChanges();
Console.WriteLine($"Product {prod.Id} created");
return prod.Id;
}
public static void AddOrderline4(OrderContext context, int prod1Id)
{
// Get a product:
var prod = context.Products.Find(prod1Id)!;
// Create orderline:
var line = new OrderLine() { Product = prod };
var cp = line.Product.CatalogPrice;
line.OrderPrice = cp;
context.OrderLines.Add(line);
context.SaveChanges(); // Line 60: no failure
Console.WriteLine($"4: Orderline {line.Id} created with price {line.OrderPrice:#,##0.00}");
}
public static void AddOrderline5(OrderContext context, int prod1Id)
{
// Get a product:
var prod = context.Products.AsNoTracking().Single(p => p.Id == prod1Id);
// Create orderline:
var line = new OrderLine() { Product = prod };
line.OrderPrice = line.Product.CatalogPrice;
context.OrderLines.Add(line);
context.SaveChanges(); // Line 74: fails here
Console.WriteLine($"5: Orderline {line.Id} created with price {line.OrderPrice:#,##0.00}");
}
public static void AddOrderline6(OrderContext context, int prod1Id)
{
// Create orderline:
var line = new OrderLine() { Product = null! };
line.ProductId = prod1Id;
context.Entry(line).Reference(line => line.Product).Load();
line.OrderPrice = line.Product.CatalogPrice;
context.OrderLines.Add(line);
context.SaveChanges(); // Line 87: fails here
Console.WriteLine($"6: Orderline {line.Id} created with price {line.OrderPrice:#,##0.00}");
}
public static void AddOrderline7(OrderContext context, int prod1Id)
{
// Create orderline:
var line = new OrderLine() { Product = null! };
context.OrderLines.Add(line);
line.ProductId = prod1Id;
context.Entry(line).Reference(line => line.Product).Load();
line.OrderPrice = line.Product.CatalogPrice;
context.SaveChanges(); // Line 100: no failure
Console.WriteLine($"7: Orderline {line.Id} created with price {line.OrderPrice:#,##0.00}");
}
public static void AddOrderline8(OrderContext context, int prod1Id)
{
// Create orderline:
var line = new OrderLine() { Product = null! };
line.ProductId = prod1Id;
context.Entry(line).Reference(line => line.Product).Load();
line.OrderPrice = line.Product.CatalogPrice;
context.Attach(line.Product);
context.OrderLines.Add(line);
context.SaveChanges(); // Line 114: no failure
Console.WriteLine($"8: Orderline {line.Id} created with price {line.OrderPrice:#,##0.00}");
}
}
public class OrderContext : DbContext
{
protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
{
optionsBuilder.UseSqlServer("Data Source=(local);Initial Catalog=EfCoreLazyAddOrder;Integrated Security=True;MultipleActiveResultSets=True;TrustServerCertificate=True");
//optionsBuilder.LogTo(Console.WriteLine, Microsoft.Extensions.Logging.LogLevel.Information);
}
public virtual DbSet<OrderLine> OrderLines { get; set; }
public virtual DbSet<Product> Products { get; set; }
}
public class OrderLine
{
[Key]
public virtual int Id { get; set; }
public virtual int ProductId { get; set; }
[ForeignKey(nameof(ProductId))]
public virtual required Product Product { get; set; }
public virtual decimal OrderPrice { get; set; }
}
public class Product
{
[Key]
public virtual int Id { get; set; }
public virtual required string Name { get; set; }
public virtual decimal CatalogPrice { get; set; }
}
In both cases where an exception is thrown, it is the following:
Unhandled exception. Microsoft.EntityFrameworkCore.DbUpdateException: An error occurred while saving the entity changes. See the inner exception for details.
---> Microsoft.Data.SqlClient.SqlException (0x80131904): Cannot insert explicit value for identity column in table 'Products' when IDENTITY_INSERT is set to OFF.
For AddOrderline5, the call to context.OrderLines.Add(line); will mark all entities in the graph as Added. This is because none of the entities are already tracked, and Add always marked untracked entities as Added regardless of key value. AddOrderline6 is the same, it just takes a different path to get to the graph of untracked entities.
As far as I can tell, everything here is by-design.
this "design-by" problem introduced in ef core 8. it was not an issue in ef core 7.
solved it by moving DbSet.Add to right after creating the proxy. @codetuner thanks.
