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

OwnsMany throws InvalidOperationException on SaveChanges with multiple items in list #15600

Closed
dave-hillier opened this issue May 3, 2019 · 8 comments · Fixed by #16500
Closed
Labels
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

@dave-hillier
Copy link

dave-hillier commented May 3, 2019

In the examples for "Owned Entities" there is no example of OwnsMany's usage.

I've put together what I thought was a sensible way of using it, but it doesnt not appear to work as I would expect. See below for the code example.

Exception message:
Stack trace: System.InvalidOperationException : The instance of entity type 'StreetAddress' cannot be tracked because another instance with the same key value for {'DistributorId', 'Id'} is already being tracked. When replacing owned entities modify the properties without changing the instance or detach the previous owned entity entry first. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the conflicting key values.
   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.NullableKeyIdentityMap`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)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.InternalEntityEntry.SetEntityState(EntityState entityState, Boolean acceptChanges, Nullable`1 forceStateWhenUnknownKey)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.PaintAction(EntityEntryGraphNode node, Boolean force)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraph[TState](EntityEntryGraphNode node, TState state, Func`3 handleNode)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityEntryGraphIterator.TraverseGraph[TState](EntityEntryGraphNode node, TState state, Func`3 handleNode)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.EntityGraphAttacher.AttachGraph(InternalEntityEntry rootEntry, EntityState entityState, Boolean forceStateWhenUnknownKey)
   at Microsoft.EntityFrameworkCore.DbContext.SetEntityState(InternalEntityEntry entry, EntityState entityState)
   at Microsoft.EntityFrameworkCore.DbContext.SetEntityState[TEntity](TEntity entity, EntityState entityState)
   at Microsoft.EntityFrameworkCore.DbContext.Add[TEntity](TEntity entity)
   at Microsoft.EntityFrameworkCore.Internal.InternalDbSet`1.Add(TEntity entity)

Steps to reproduce

[Owned]
        public class StreetAddress 
        {       
            public string Street { get; set; }
            public string City { get; set; }
        }
        
        public class Distributor
        {
            public int Id { get; set; }
            public ICollection<StreetAddress> ShippingCenters { get; set; }
        }

        public class TestDbContext : DbContext
        {
            public DbSet<Distributor> Distributors { get; set; }
            
            protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
            {
                optionsBuilder.UseInMemoryDatabase("Test");
            }

            protected override void OnModelCreating(ModelBuilder modelBuilder)
            {
                modelBuilder.Entity<Distributor>().OwnsMany(rt => rt.ShippingCenters, image =>
                {
                    image.HasForeignKey("DistributorId");
                    image.Property<int>("Id");
                    image.HasKey("DistributorId", "Id");
                });
            }
        }
     
... snip

        [Fact]
        public void TestDb()
        {
            using (var context = new TestDbContext())
            {
                var model = new Distributor
                {
                    ShippingCenters = new List<StreetAddress>
                    {
                        new StreetAddress {Street = "1", City="City"},
                        new StreetAddress {Street = "2", City="City"} // When this item is removed the exception is no longer thrown
                    }
                };
                    
                context.Distributors.Add(model);
                context.SaveChanges(); // Throws System.InvalidOperationException
            }
        }

Further technical details

EF Core version: 2.2.0
InMemoryDatabase

@ajcvickers
Copy link
Member

@dave-hillier Question from triage: with regard to this code:

image.HasForeignKey("DistributorId");
image.Property<int>("Id");
image.HasKey("DistributorId", "Id");

Is this something that you are doing because you want and value this kind of composite key and relationship in the database? Or is it just that this was what was shown in the blog\docs?

@dave-hillier
Copy link
Author

dave-hillier commented May 3, 2019

I put this example together from the EF Core docs

The docs state:

It is common to use a complex key for these type of entities incorporating the foreign key to the owner and an additional unique property that can also be in shadow state

The way it's written, it looks like guidance. I was struggling to get the same owned class to work from different places. I get a duplicate key exception but I assume I am doing something wrong. My intention was to get the documented example working first then go back to my code.

@ajcvickers
Copy link
Member

@dave-hillier And is that statement from the docs something you would have expected and wanted in this case?

@ajcvickers
Copy link
Member

@dave-hillier To be fully transparent, I think it's unnecessary and overly complex, but I'm the only one on the team who thinks that. Others assert that this is the normal pattern for this case and something that people will expect and want.

@dave-hillier
Copy link
Author

I have removed it from my code, but I still have a different problem. This other problem I'm unable to make a simple but it seems related (why I mention it here).

When I have two classes using the same owned class I either get cast exceptions or duplicate keys.

As a workaround, I've now got a copy of the owned entity e.g. StreetAddress2. I've tried adding a ToTable that didnt help.

@ajcvickers
Copy link
Member

@dave-hillier Thanks for the additional info. We will discuss again and get back to you.

@dave-hillier
Copy link
Author

dave-hillier commented May 3, 2019

More info. The parent classes keys differ. This seems to be a cause. I can make them the same and this exception goes away, though the duplicate keys one still persists.

System.InvalidCastException : Unable to cast object of type 'System.String' to type 'System.Int32'.
   at Microsoft.EntityFrameworkCore.ChangeTracking.ValueComparer`1.Snapshot(Object instance)
   at Microsoft.EntityFrameworkCore.InMemory.Query.Internal.InMemoryQueryModelVisitor.SnapshotValueBuffer(IEntityType entityType, MaterializationContext c)
   at Microsoft.EntityFrameworkCore.InMemory.Query.Internal.InMemoryQueryModelVisitor.<>c__DisplayClass2_1`1.<EntityQuery>b__2(MaterializationContext c)
   at Microsoft.EntityFrameworkCore.Query.EntityLoadInfo.Materialize()
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryBuffer.GetEntity(IKey key, EntityLoadInfo entityLoadInfo, Boolean queryStateManager, Boolean throwOnNullKey)
   at Microsoft.EntityFrameworkCore.InMemory.Query.Internal.InMemoryQueryModelVisitor.<>c__DisplayClass2_1`1.<EntityQuery>b__1(Object[] vs)
   at System.Linq.Enumerable.SelectListIterator`2.MoveNext()
   at System.Linq.Enumerable.SelectManySingleSelectorIterator`2.MoveNext()
   at System.Linq.Enumerable.JoinIterator[TOuter,TInner,TKey,TResult](IEnumerable`1 outer, IEnumerable`1 inner, Func`2 outerKeySelector, Func`2 innerKeySelector, Func`3 resultSelector, IEqualityComparer`1 comparer)+MoveNext()
   at System.Collections.Generic.EnumerableHelpers.ToArray[T](IEnumerable`1 source, Int32& length)
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.OrderedEnumerable`1.GetEnumerator()+MoveNext()
   at System.Linq.Enumerable.SelectIPartitionIterator`2.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryBuffer.IncludeCollection[TEntity,TRelated,TElement](Int32 includeId, INavigation navigation, INavigation inverseNavigation, IEntityType targetEntityType, IClrCollectionAccessor clrCollectionAccessor, IClrPropertySetter inverseClrPropertySetter, Boolean tracking, TEntity entity, Func`1 relatedEntitiesFactory, Func`3 joinPredicate)
   at lambda_method(Closure , QueryContext , RoomType , Object[] )
   at Microsoft.EntityFrameworkCore.Query.Internal.IncludeCompiler._Include[TEntity](QueryContext queryContext, TEntity entity, Object[] included, Action`3 fixup)
   at lambda_method(Closure , TransparentIdentifier`2 )
   at System.Linq.Enumerable.SelectIPartitionIterator`2.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryBuffer.IncludeCollection[TEntity,TRelated,TElement](Int32 includeId, INavigation navigation, INavigation inverseNavigation, IEntityType targetEntityType, IClrCollectionAccessor clrCollectionAccessor, IClrPropertySetter inverseClrPropertySetter, Boolean tracking, TEntity entity, Func`1 relatedEntitiesFactory, Func`3 joinPredicate)
   at lambda_method(Closure , QueryContext , PropertyThing , Object[] )
   at Microsoft.EntityFrameworkCore.Query.Internal.IncludeCompiler._Include[TEntity](QueryContext queryContext, TEntity entity, Object[] included, Action`3 fixup)
   at lambda_method(Closure , TransparentIdentifier`2 )
   at System.Linq.Enumerable.SelectIPartitionIterator`2.MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider._TrackEntities[TOut,TIn](IEnumerable`1 results, QueryContext queryContext, IList`1 entityTrackingInfos, IList`1 entityAccessors)+MoveNext()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ExceptionInterceptor`1.EnumeratorExceptionInterceptor.MoveNext()
   at System.Linq.AsyncEnumerable.AsyncEnumerableAdapter`1.MoveNextCore(CancellationToken cancellationToken) in D:\a\1\s\Ix.NET\Source\System.Interactive.Async\ToAsyncEnumerable.cs:line 131
   at System.Linq.AsyncEnumerable.AsyncIterator`1.MoveNext(CancellationToken cancellationToken) in D:\a\1\s\Ix.NET\Source\System.Interactive.Async\AsyncIterator.cs:line 98
   at System.Linq.AsyncEnumerable.Aggregate_[TSource,TAccumulate,TResult](IAsyncEnumerable`1 source, TAccumulate seed, Func`3 accumulator, Func`2 resultSelector, CancellationToken cancellationToken) in D:\a\1\s\Ix.NET\Source\System.Interactive.Async\Aggregate.cs:line 120
   at Properties.Controllers.PropertiesController.GetLatestVersionProperties(String tenant) in /Users/daveh/source/repos/task/Properties/Controllers/PropertiesController.cs:line 74
   at lambda_method(Closure , Object )
   at Microsoft.Extensions.Internal.ObjectMethodExecutorAwaitable.Awaiter.GetResult()
   at Microsoft.AspNetCore.Mvc.Internal.ActionMethodExecutor.AwaitableObjectResultExecutor.Execute(IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
   at System.Threading.Tasks.ValueTask`1.get_Result()
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.InvokeActionMethodAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.InvokeNextActionFilterAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.Rethrow(ActionExecutedContext context)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.InvokeInnerFilterAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeNextResourceFilter()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Rethrow(ResourceExecutedContext context)
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeFilterPipelineAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeAsync()
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Routing.EndpointRoutingMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Builder.Extensions.MapWhenMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.TestHost.HttpContextBuilder.<>c__DisplayClass10_0.<<SendAsync>b__0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.AspNetCore.TestHost.ClientHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.FinishSendAsyncBuffered(Task`1 sendTask, HttpRequestMessage request, CancellationTokenSource cts, Boolean disposeCts)

@dave-hillier dave-hillier changed the title OwnsMany throws InvalidOperationException with multiple items in list OwnsMany throws InvalidOperationException on SaveChanges with multiple items in list May 4, 2019
@ajcvickers ajcvickers added this to the 3.0.0 milestone May 6, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0, 3.0.0-preview7 Jul 2, 2019
@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 Jul 7, 2019
@ajcvickers ajcvickers assigned ajcvickers and unassigned AndriySvyryd Jul 7, 2019
@ajcvickers
Copy link
Member

Poaching...

ajcvickers added a commit that referenced this issue Jul 7, 2019
Fixes (or verifies as fixed) several fix-up/owned types issues:

* Fixes #16161
* Fixes #15760
* Fixes #15600
* Fixes #15484
* Fixes #14675
ajcvickers added a commit that referenced this issue Jul 8, 2019
Fixes (or verifies as fixed) several fix-up/owned types issues:

* Fixes #16161
* Fixes #15760
* Fixes #15600
* Fixes #15484
* Fixes #14675
@ajcvickers ajcvickers modified the milestones: 3.0.0, 3.0.0-preview8 Jul 29, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0-preview8, 3.0.0 Nov 11, 2019
@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
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