Merge pull request #493 from wisepotato/fix/#492

Fix/#492

Author: jaredcnance

JsonApiDotnetCore.sln

@@ -190,7 +190,7 @@ Global
 		{6DFA30D7-1679-4333-9779-6FB678E48EF5}.Release|x64.ActiveCfg = Release|Any CPU
 		{6DFA30D7-1679-4333-9779-6FB678E48EF5}.Release|x64.Build.0 = Release|Any CPU
 		{6DFA30D7-1679-4333-9779-6FB678E48EF5}.Release|x86.ActiveCfg = Release|Any CPU
-		{6DFA30D7-1679-4333-9779-6FB678E48EF5}.Release|x86.Build.0 = Release|Any CPU\
+		{6DFA30D7-1679-4333-9779-6FB678E48EF5}.Release|x86.Build.0 = Release|Any CPU
 		{09C0C8D8-B721-4955-8889-55CB149C3B5C}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
 		{09C0C8D8-B721-4955-8889-55CB149C3B5C}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{09C0C8D8-B721-4955-8889-55CB149C3B5C}.Debug|x64.ActiveCfg = Debug|Any CPU
@@ -203,6 +203,7 @@ Global
 		{09C0C8D8-B721-4955-8889-55CB149C3B5C}.Release|x64.Build.0 = Release|Any CPU
 		{09C0C8D8-B721-4955-8889-55CB149C3B5C}.Release|x86.ActiveCfg = Release|Any CPU
 		{09C0C8D8-B721-4955-8889-55CB149C3B5C}.Release|x86.Build.0 = Release|Any CPU
+		{DF9BFD82-D937-4907-B0B4-64670417115F}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE

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,20 @@ 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.
+                    /// Also, we currently do not support the same for many-to-many
+                    if (relationship.Key is HasManyAttribute && !(relationship.Key is HasManyThroughAttribute))
+                        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 +373,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;
@@ -274,7 +274,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 +295,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/DocumentTests/Included.cs

@@ -181,6 +181,7 @@ public async Task GET_Included_DoesNot_Duplicate_Records_ForMultipleRelationship
         public async Task GET_Included_DoesNot_Duplicate_Records_If_HasOne_Exists_Twice()
         {
             // arrange
+            _context.TodoItemCollections.RemoveRange(_context.TodoItemCollections);
             _context.People.RemoveRange(_context.People); // ensure all people have todo-items
             _context.TodoItems.RemoveRange(_context.TodoItems);
             var person = _personFaker.Generate();

test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs

@@ -37,6 +37,78 @@ public UpdatingRelationshipsTests(TestFixture<TestStartup> fixture)
                 .RuleFor(t => t.Description, f => f.Lorem.Sentence())
                 .RuleFor(t => t.Ordinal, f => f.Random.Number())
                 .RuleFor(t => t.CreatedDate, f => f.Date.Past());
+
+
+        }
+
+
+        [Fact]
+        public async Task Can_Update_ToMany_Relationship_By_Patching_Resource()
+        {
+            // arrange
+            var todoCollection = new TodoItemCollection();
+            todoCollection.TodoItems = new List<TodoItem>();
+            var person = _personFaker.Generate();
+            var todoItem = _todoItemFaker.Generate();
+            todoCollection.Owner = person;
+            todoCollection.TodoItems.Add(todoItem);
+            _context.TodoItemCollections.Add(todoCollection);
+            _context.SaveChanges();
+
+            var newTodoItem1 = _todoItemFaker.Generate();
+            var newTodoItem2 = _todoItemFaker.Generate();
+            _context.AddRange(new TodoItem[] { newTodoItem1, newTodoItem2 });
+            _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 = $"{newTodoItem1.Id}" },
+                                    new { type = "todo-items", id = $"{newTodoItem2.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);
+            /// we are expecting two, not three, because the request does 
+            /// a "complete replace".
+            Assert.Equal(2, updatedTodoItems.Count);
         }
 
         [Fact]

test/UnitTests/Serialization/JsonApiDeSerializerTests.cs

@@ -361,6 +361,7 @@ public void Can_Deserialize_Object_With_HasManyRelationship()
             jsonApiContextMock.SetupAllProperties();
             jsonApiContextMock.Setup(m => m.ResourceGraph).Returns(resourceGraph);
             jsonApiContextMock.Setup(m => m.AttributesToUpdate).Returns(new Dictionary<AttrAttribute, object>());
+            jsonApiContextMock.Setup(m => m.RelationshipsToUpdate).Returns(new Dictionary<RelationshipAttribute, object>());
             jsonApiContextMock.Setup(m => m.HasManyRelationshipPointers).Returns(new HasManyRelationshipPointers());
 
             var jsonApiOptions = new JsonApiOptions();
@@ -414,6 +415,7 @@ public void Sets_Attribute_Values_On_Included_HasMany_Relationships()
             jsonApiContextMock.SetupAllProperties();
             jsonApiContextMock.Setup(m => m.ResourceGraph).Returns(resourceGraph);
             jsonApiContextMock.Setup(m => m.AttributesToUpdate).Returns(new Dictionary<AttrAttribute, object>());
+            jsonApiContextMock.Setup(m => m.RelationshipsToUpdate).Returns(new Dictionary<RelationshipAttribute, object>());
             jsonApiContextMock.Setup(m => m.HasManyRelationshipPointers).Returns(new HasManyRelationshipPointers());
 
             var jsonApiOptions = new JsonApiOptions();