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

jaredcnance · web-flow · commit 9059560ffdaa · 2019-04-13T10:48:21.000-07:00
Fix/#492
diff --git a/JsonApiDotnetCore.sln b/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
diff --git a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs b/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);
             }
 
diff --git a/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs b/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);
             }
 
diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Included.cs b/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();
diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs b/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]
diff --git a/test/UnitTests/Serialization/JsonApiDeSerializerTests.cs b/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();

Original file line number	Diff line number	Diff line change
`@@ -163,7 +163,7 @@ public void DetachRelationshipPointers(TEntity entity)`
`163`	`163`	`{`
`164`	`164`	`foreach (var hasOneRelationship in _jsonApiContext.HasOneRelationshipPointers.Get())`
`165`	`165`	`{`
`166`		`- var hasOne = (HasOneAttribute) hasOneRelationship.Key;`
	`166`	`+ var hasOne = (HasOneAttribute)hasOneRelationship.Key;`
`167`	`167`	`if (hasOne.EntityPropertyName != null)`
`168`	`168`	`{`
`169`	`169`	`var relatedEntity = entity.GetType().GetProperty(hasOne.EntityPropertyName)?.GetValue(entity);`
`@@ -178,7 +178,7 @@ public void DetachRelationshipPointers(TEntity entity)`
`178`	`178`
`179`	`179`	`foreach (var hasManyRelationship in _jsonApiContext.HasManyRelationshipPointers.Get())`
`180`	`180`	`{`
`181`		`- var hasMany = (HasManyAttribute) hasManyRelationship.Key;`
	`181`	`+ var hasMany = (HasManyAttribute)hasManyRelationship.Key;`
`182`	`182`	`if (hasMany.EntityPropertyName != null)`
`183`	`183`	`{`
`184`	`184`	`var relatedList = (IList)entity.GetType().GetProperty(hasMany.EntityPropertyName)?.GetValue(entity);`
`@@ -194,7 +194,7 @@ public void DetachRelationshipPointers(TEntity entity)`
`194`	`194`	`_context.Entry(pointer).State = EntityState.Detached;`
`195`	`195`	`}`
`196`	`196`	`}`
`197`		`-`
	`197`	`+`
`198`	`198`	`// HACK: detaching has many relationships doesn't appear to be sufficient`
`199`	`199`	`// the navigation property actually needs to be nulled out, otherwise`
`200`	`200`	`// EF adds duplicate instances to the collection`
`@@ -234,7 +234,7 @@ private void AttachHasMany(TEntity entity, HasManyAttribute relationship, IList`
`234`	`234`	`{`
`235`	`235`	`_context.Entry(pointer).State = EntityState.Unchanged;`
`236`	`236`	`}`
`237`		`- }`
	`237`	`+ }`
`238`	`238`	`}`
`239`	`239`
`240`	`240`	`private void AttachHasManyThrough(TEntity entity, HasManyThroughAttribute hasManyThrough, IList pointers)`
`@@ -270,7 +270,7 @@ private void AttachHasOnePointers(TEntity entity)`
`270`	`270`	`if (relationship.Key.GetType() != typeof(HasOneAttribute))`
`271`	`271`	`continue;`
`272`	`272`
`273`		`- var hasOne = (HasOneAttribute) relationship.Key;`
	`273`	`+ var hasOne = (HasOneAttribute)relationship.Key;`
`274`	`274`	`if (hasOne.EntityPropertyName != null)`
`275`	`275`	`{`
`276`	`276`	`var relatedEntity = entity.GetType().GetProperty(hasOne.EntityPropertyName)?.GetValue(entity);`
`@@ -296,13 +296,20 @@ public virtual async Task<TEntity> UpdateAsync(TId id, TEntity entity)`
`296`	`296`	`foreach (var attr in _jsonApiContext.AttributesToUpdate)`
`297`	`297`	`attr.Key.SetValue(oldEntity, attr.Value);`
`298`	`298`
`299`		`- foreach (var relationship in _jsonApiContext.RelationshipsToUpdate)`
`300`		`- relationship.Key.SetValue(oldEntity, relationship.Value);`
`301`		`-`
`302`		`- AttachRelationships(oldEntity);`
`303`		`-`
	`299`	`+ if (_jsonApiContext.RelationshipsToUpdate.Any())`
	`300`	`+ {`
	`301`	`+ AttachRelationships(oldEntity);`
	`302`	`+ foreach (var relationship in _jsonApiContext.RelationshipsToUpdate)`
	`303`	`+ {`
	`304`	`+ /// If we are updating to-many relations from PATCH, we need to include the relation first,`
	`305`	`+ /// else it will not peform a complete replacement, as required by the specs.`
	`306`	`+ /// Also, we currently do not support the same for many-to-many`
	`307`	`+ if (relationship.Key is HasManyAttribute && !(relationship.Key is HasManyThroughAttribute))`
	`308`	`+ await _context.Entry(oldEntity).Collection(relationship.Key.InternalRelationshipName).LoadAsync();`
	`309`	`+ relationship.Key.SetValue(oldEntity, relationship.Value); // article.tags = nieuwe lijst`
	`310`	`+ }`
	`311`	`+ }`
`304`	`312`	`await _context.SaveChangesAsync();`
`305`		`-`
`306`	`313`	`return oldEntity;`
`307`	`314`	`}`
`308`	`315`
`@@ -366,7 +373,7 @@ public virtual IQueryable<TEntity> Include(IQueryable<TEntity> entities, string`
`366`	`373`	`? relationship.RelationshipPath`
`367`	`374`	`: $"{internalRelationshipPath}.{relationship.RelationshipPath}";`
`368`	`375`
`369`		`- if(i < relationshipChain.Length)`
	`376`	`+ if (i < relationshipChain.Length)`
`370`	`377`	`entity = _jsonApiContext.ResourceGraph.GetContextEntity(relationship.Type);`
`371`	`378`	`}`
`372`	`379`
Original file line number	Diff line number	Diff line change
`@@ -181,6 +181,7 @@ public async Task GET_Included_DoesNot_Duplicate_Records_ForMultipleRelationship`
`181`	`181`	`public async Task GET_Included_DoesNot_Duplicate_Records_If_HasOne_Exists_Twice()`
`182`	`182`	`{`
`183`	`183`	`// arrange`
	`184`	`+ _context.TodoItemCollections.RemoveRange(_context.TodoItemCollections);`
`184`	`185`	`_context.People.RemoveRange(_context.People); // ensure all people have todo-items`
`185`	`186`	`_context.TodoItems.RemoveRange(_context.TodoItems);`
`186`	`187`	`var person = _personFaker.Generate();`