fix: #492

Author: maurei

src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs

@@ -163,7 +163,7 @@ public void DetachRelationshipPointers(TEntity entity)
         {
             foreach (var hasOneRelationship in _jsonApiContext.HasOneRelationshipPointers.Get())
             {
-                var hasOne = (HasOneAttribute) hasOneRelationship.Key;
+                var hasOne = (HasOneAttribute)hasOneRelationship.Key;
                 if (hasOne.EntityPropertyName != null)
                 {
                     var relatedEntity = entity.GetType().GetProperty(hasOne.EntityPropertyName)?.GetValue(entity);
@@ -178,7 +178,7 @@ public void DetachRelationshipPointers(TEntity entity)
 
             foreach (var hasManyRelationship in _jsonApiContext.HasManyRelationshipPointers.Get())
             {
-                var hasMany = (HasManyAttribute) hasManyRelationship.Key;
+                var hasMany = (HasManyAttribute)hasManyRelationship.Key;
                 if (hasMany.EntityPropertyName != null)
                 {
                     var relatedList = (IList)entity.GetType().GetProperty(hasMany.EntityPropertyName)?.GetValue(entity);
@@ -194,7 +194,7 @@ public void DetachRelationshipPointers(TEntity entity)
                         _context.Entry(pointer).State = EntityState.Detached;
                     }
                 }
-                
+
                 // HACK: detaching has many relationships doesn't appear to be sufficient
                 // the navigation property actually needs to be nulled out, otherwise
                 // EF adds duplicate instances to the collection
@@ -234,7 +234,7 @@ private void AttachHasMany(TEntity entity, HasManyAttribute relationship, IList
                 {
                     _context.Entry(pointer).State = EntityState.Unchanged;
                 }
-            }  
+            }
         }
 
         private void AttachHasManyThrough(TEntity entity, HasManyThroughAttribute hasManyThrough, IList pointers)
@@ -270,7 +270,7 @@ private void AttachHasOnePointers(TEntity entity)
                 if (relationship.Key.GetType() != typeof(HasOneAttribute))
                     continue;
 
-                var hasOne = (HasOneAttribute) relationship.Key;
+                var hasOne = (HasOneAttribute)relationship.Key;
                 if (hasOne.EntityPropertyName != null)
                 {
                     var relatedEntity = entity.GetType().GetProperty(hasOne.EntityPropertyName)?.GetValue(entity);
@@ -296,13 +296,19 @@ public virtual async Task<TEntity> UpdateAsync(TId id, TEntity entity)
             foreach (var attr in _jsonApiContext.AttributesToUpdate)
                 attr.Key.SetValue(oldEntity, attr.Value);
 
-            foreach (var relationship in _jsonApiContext.RelationshipsToUpdate)
-                relationship.Key.SetValue(oldEntity, relationship.Value);
-
-            AttachRelationships(oldEntity);
-
+            if (_jsonApiContext.RelationshipsToUpdate.Any())
+            {
+                AttachRelationships(oldEntity);
+                foreach (var relationship in _jsonApiContext.RelationshipsToUpdate)
+                {
+                    /// If we are updating to-many relations from PATCH, we need to include the relation first,
+                    /// else it will not peform a complete replacement, as required by the specs.
+                    if (relationship.Key.IsHasMany)
+                        await _context.Entry(oldEntity).Collection(relationship.Key.InternalRelationshipName).LoadAsync();
+                    relationship.Key.SetValue(oldEntity, relationship.Value); // article.tags = nieuwe lijst    
+                }
+            }
             await _context.SaveChangesAsync();
-
             return oldEntity;
         }
 
@@ -366,7 +372,7 @@ public virtual IQueryable<TEntity> Include(IQueryable<TEntity> entities, string
                     ? relationship.RelationshipPath
                     : $"{internalRelationshipPath}.{relationship.RelationshipPath}";
 
-                if(i < relationshipChain.Length)
+                if (i < relationshipChain.Length)
                     entity = _jsonApiContext.ResourceGraph.GetContextEntity(relationship.Type);
             }
 

src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs

@@ -186,7 +186,7 @@ private object SetRelationships(
             {
                 entity = attr.IsHasOne
                     ? SetHasOneRelationship(entity, entityProperties, (HasOneAttribute)attr, contextEntity, relationships, included)
-                    : SetHasManyRelationship(entity, entityProperties, attr, contextEntity, relationships, included);
+                    : SetHasManyRelationship(entity, entityProperties, (HasManyAttribute)attr, contextEntity, relationships, included);
             }
 
             return entity;
@@ -230,6 +230,15 @@ private object SetHasOneRelationship(object entity,
             return entity;
         }
 
+        /// @TODO investigate the following: when running the Can_Patch_Entity_And_HasOne_Relationship, the updating of the to-one relation happens
+        /// by assigning a new value to the foreignkey on the TodoItem:  todoItem.ownerId = new value.
+        /// However, this happens in two places: here in this method, but also in the repository, here
+        /// https://github.com/json-api-dotnet/JsonApiDotNetCore/blob/8f685a6a23515acf8f440c70d20444a0cec1c502/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs#L300
+        /// 
+        /// Is there a reason this happens twice? Should we need to get rid of one? It should probably happen in the repository only, not here,
+        /// because in the case of one-to-one, updating a foreign key on the main entity (TodoItem in this case) is sufficient and can indeed be done from the deserializer,
+        /// but when updating one-to-many or many-to-many, we will need to make extra queries, which is a repository thing.
+        /// 
         private void SetHasOneForeignKeyValue(object entity, HasOneAttribute hasOneAttr, PropertyInfo foreignKeyProperty, ResourceIdentifierObject rio)
         {
             var foreignKeyPropertyValue = rio?.Id ?? null;
@@ -274,7 +283,7 @@ private void SetHasOneNavigationPropertyValue(object entity, HasOneAttribute has
 
         private object SetHasManyRelationship(object entity,
             PropertyInfo[] entityProperties,
-            RelationshipAttribute attr,
+            HasManyAttribute attr,
             ContextEntity contextEntity,
             Dictionary<string, RelationshipData> relationships,
             List<ResourceObject> included = null)
@@ -295,7 +304,7 @@ private object SetHasManyRelationship(object entity,
                 var convertedCollection = TypeHelper.ConvertCollection(relatedResources, attr.Type);
 
                 attr.SetValue(entity, convertedCollection);
-
+                _jsonApiContext.RelationshipsToUpdate[attr] = convertedCollection;
                 _jsonApiContext.HasManyRelationshipPointers.Add(attr, convertedCollection);
             }
 

test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs

@@ -46,6 +46,11 @@ public UpdatingRelationshipsTests(TestFixture<TestStartup> fixture)
         public async Task Can_Update_ToMany_Relationship_By_Patching_Resource()
         {
             // arrange
+            _context.TodoItemCollections.RemoveRange(_context.TodoItemCollections);
+            _context.People.RemoveRange(_context.People);
+            _context.TodoItems.RemoveRange(_context.TodoItems);
+            _context.SaveChanges();
+
             var todoCollection = new TodoItemCollection();
             todoCollection.TodoItems = new List<TodoItem>();
             var person = _personFaker.Generate();
@@ -60,8 +65,6 @@ public async Task Can_Update_ToMany_Relationship_By_Patching_Resource()
             _context.AddRange(new TodoItem[] { newTodoItem1, newTodoItem2 });
             _context.SaveChanges();
 
-
-
             var builder = new WebHostBuilder()
                 .UseStartup<Startup>();
 
@@ -102,13 +105,15 @@ public async Task Can_Update_ToMany_Relationship_By_Patching_Resource()
             // Act
             var response = await client.SendAsync(request);
             _context = _fixture.GetService<AppDbContext>();
-            var updatedTodoItems = _context.TodoItemCollections
-                                            .Where(tic => tic.Id == todoCollection.Id)
-                                            .Include(tdc => tdc.TodoItems).SingleOrDefault().TodoItems;
+            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(3, updatedTodoItems.Count);
+            /// we are expecting two, not three, because the request does 
+            /// a "complete replace".
+            Assert.Equal(2, updatedTodoItems.Count);
         }
 
         [Fact]