fix(#494): added another fix that also addresses an error in #492)

Author: maurei

src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs

@@ -225,13 +225,15 @@ private void AttachHasMany(TEntity entity, HasManyAttribute relationship, IList
                 var relatedList = (IList)entity.GetType().GetProperty(relationship.EntityPropertyName)?.GetValue(entity);
                 foreach (var related in relatedList)
                 {
-                    _context.Entry(related).State = EntityState.Unchanged;
+                    if (_context.EntityIsTracked(related as IIdentifiable) == false)
+                        _context.Entry(related).State = EntityState.Unchanged;
                 }
             }
             else
             {
                 foreach (var pointer in pointers)
                 {
+                    if (_context.EntityIsTracked(pointer as IIdentifiable) == false)
                     _context.Entry(pointer).State = EntityState.Unchanged;
                 }
             }
@@ -307,21 +309,27 @@ public virtual async Task<TEntity> UpdateAsync(TId id, TEntity entity)
                     {
                         await _context.Entry(oldEntity).Collection(throughAttribute.InternalThroughName).LoadAsync();
                     }
-                    else if (relationship.Key is HasManyAttribute)
-                    { 
-                        await _context.Entry(oldEntity).Collection(relationship.Key.InternalRelationshipName).LoadAsync();
-                    }
                 }
+
+                /// @HACK @TODO: It is inconsistent that for many-to-many, the new relationship value
+                /// is assigned in AttachRelationships() helper fn below, but not for 
+                /// one-to-many and one-to-one (we need to do that manually as done below).
+                /// Simultaneously, for a proper working "complete replacement", in the case of many-to-many
+                /// we need to LoadAsync() BEFORE calling AttachRelationships(), but for one-to-many we 
+                /// need to do it AFTER AttachRelationships or we we'll get entity tracking errors
+                /// This really needs a refactor.
                 AttachRelationships(oldEntity);
 
-                /// @TODO: It it not consistent that for many-to-many, the new relationship value
-                /// is assigned in AttachRelationships() helperfunction, whereas for
-                /// one-to-many and one-to-one, we need to do it manually as below.
-                /// As a result, we need to loop over RelationshipsToUpdate a second time.
                 foreach (var relationship in _jsonApiContext.RelationshipsToUpdate)
                 {
-                    if (!(relationship.Key is HasManyThroughAttribute))
-                    { 
+
+                    if ((relationship.Key.TypeId as Type).IsAssignableFrom(typeof(HasOneAttribute)))
+                    {
+                        relationship.Key.SetValue(oldEntity, relationship.Value);
+                    }
+                    if ((relationship.Key.TypeId as Type).IsAssignableFrom(typeof(HasManyAttribute)))
+                    {
+                        await _context.Entry(oldEntity).Collection(relationship.Key.InternalRelationshipName).LoadAsync();
                         relationship.Key.SetValue(oldEntity, relationship.Value);
                     }
                 }

test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs

@@ -313,6 +313,7 @@ public async Task Can_Update_Many_To_Many_With_Complete_Replacement()
             var tag = persistedArticle.ArticleTags.Select(at => at.Tag).Single();
             Assert.Equal(secondTag.Id, tag.Id);
         }
+
         [Fact]
         public async Task Can_Update_Many_To_Many_With_Complete_Replacement_With_Overlap()
         {

test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs

@@ -41,7 +41,6 @@ public UpdatingRelationshipsTests(TestFixture<TestStartup> fixture)
 
         }
 
-
         [Fact]
         public async Task Can_Update_ToMany_Relationship_By_Patching_Resource()
         {
@@ -66,7 +65,6 @@ public async Task Can_Update_ToMany_Relationship_By_Patching_Resource()
             var server = new TestServer(builder);
             var client = server.CreateClient();
 
-
             var content = new
             {
                 data = new
@@ -111,6 +109,72 @@ public async Task Can_Update_ToMany_Relationship_By_Patching_Resource()
             Assert.Equal(2, updatedTodoItems.Count);
         }
 
+        [Fact]
+        public async Task Can_Update_ToMany_Relationship_By_Patching_Resource_With_Overlap()
+        {
+            // arrange
+            var todoCollection = new TodoItemCollection();
+            todoCollection.TodoItems = new List<TodoItem>();
+            var person = _personFaker.Generate();
+            var todoItem1 = _todoItemFaker.Generate();
+            var todoItem2 = _todoItemFaker.Generate();
+            todoCollection.Owner = person;
+            todoCollection.TodoItems.Add(todoItem1);
+            todoCollection.TodoItems.Add(todoItem2);
+            _context.TodoItemCollections.Add(todoCollection);
+            _context.SaveChanges();
+
+            var builder = new WebHostBuilder()
+                .UseStartup<Startup>();
+
+            var server = new TestServer(builder);
+            var client = server.CreateClient();
+
+
+            var content = new
+            {
+                data = new
+                {
+                    type = "todo-collections",
+                    id = todoCollection.Id,
+                    relationships = new Dictionary<string, object>
+                    {
+                        { "todo-items", new
+                            {
+                                data = new object[]
+                                {
+                                    new { type = "todo-items", id = $"{todoItem1.Id}" },
+                                    new { type = "todo-items", id = $"{todoItem2.Id}" }
+                                }
+
+                            }
+                        },
+                    }
+                }
+            };
+
+            var httpMethod = new HttpMethod("PATCH");
+            var route = $"/api/v1/todo-collections/{todoCollection.Id}";
+            var request = new HttpRequestMessage(httpMethod, route);
+
+            string serializedContent = JsonConvert.SerializeObject(content);
+            request.Content = new StringContent(serializedContent);
+            request.Content.Headers.ContentType = new MediaTypeHeaderValue("application/vnd.api+json");
+
+            // Act
+            var response = await client.SendAsync(request);
+
+
+            _context = _fixture.GetService<AppDbContext>();
+            var updatedTodoItems = _context.TodoItemCollections.AsNoTracking()
+                .Where(tic => tic.Id == todoCollection.Id)
+                .Include(tdc => tdc.TodoItems).SingleOrDefault().TodoItems;
+
+
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            Assert.Equal(2, updatedTodoItems.Count);
+        }
+
         [Fact]
         public async Task Can_Update_ToMany_Relationship_ThroughLink()
         {