Skip to content

Fix/#492 #493

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Apr 13, 2019
Merged

Fix/#492 #493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8ca7934
test: exposing bug as described in #492
maurei Apr 11, 2019
46a3c02
fix: #492
maurei Apr 11, 2019
b251482
fix: previous tests not passing
maurei Apr 11, 2019
02fc513
fix: one more failing test
maurei Apr 11, 2019
733867f
chore: remove comment that no longer applies
maurei Apr 11, 2019
ff4de51
chore: put back commented-out test
maurei Apr 11, 2019
e7ee2c2
Update DefaultEntityRepository.cs
wisepotato Apr 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion JsonApiDotnetCore.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
31 changes: 19 additions & 12 deletions src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}

Expand Down Expand Up @@ -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);
}

Expand Down
6 changes: 3 additions & 3 deletions src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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);
}

Expand Down
1 change: 1 addition & 0 deletions test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Included.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
72 changes: 72 additions & 0 deletions test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 2 additions & 0 deletions test/UnitTests/Serialization/JsonApiDeSerializerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down