From 926a94594559e0963b313be18d39e6c7bcf0c859 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Tue, 30 Apr 2019 11:32:53 +0200 Subject: [PATCH 1/5] test: expose bug --- .../Data/AppDbContext.cs | 6 ++- .../Models/Passport.cs | 11 ++++ .../JsonApiDotNetCoreExample/Models/Person.cs | 4 ++ .../Spec/UpdatingRelationshipsTests.cs | 50 +++++++++++++++++++ 4 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 src/Examples/JsonApiDotNetCoreExample/Models/Passport.cs diff --git a/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs b/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs index b6609eb0dc..86bb4deb30 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs @@ -50,6 +50,10 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) .WithOne(t => t.ParentTodoItem) .HasForeignKey(t => t.ParentTodoItemId); + modelBuilder.Entity() + .HasOne(p => p.Passport) + .WithOne(p => p.Person) + .HasForeignKey(p => p.PassportId); } @@ -61,7 +65,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) public DbSet Authors { get; set; } public DbSet NonJsonApiResources { get; set; } public DbSet Users { get; set; } - public DbSet Courses { get; set; } public DbSet Departments { get; set; } public DbSet Registrations { get; set; } @@ -69,5 +72,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) public DbSet PersonRoles { get; set; } public DbSet ArticleTags { get; set; } public DbSet Tags { get; set; } + public DbSet Passports { get; set; } } } diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/Passport.cs b/src/Examples/JsonApiDotNetCoreExample/Models/Passport.cs new file mode 100644 index 0000000000..7586b67674 --- /dev/null +++ b/src/Examples/JsonApiDotNetCoreExample/Models/Passport.cs @@ -0,0 +1,11 @@ +using JsonApiDotNetCore.Models; + +namespace JsonApiDotNetCoreExample.Models +{ + public class Passport : Identifiable + { + public virtual int? SocialSecurityNumber { get; set; } + [HasOne("person")] + public virtual Person Person { get; set; } + } +} \ No newline at end of file diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/Person.cs b/src/Examples/JsonApiDotNetCoreExample/Models/Person.cs index eb032cc5fd..6b7f6f757c 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Models/Person.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Models/Person.cs @@ -38,6 +38,10 @@ public class Person : Identifiable, IHasMeta [HasOne("unincludeable-item", documentLinks: Link.All, canInclude: false)] public virtual TodoItem UnIncludeableItem { get; set; } + public int? PassportId { get; set; } + [HasOne("passport")] + public virtual Passport Passport { get; set; } + public Dictionary GetMeta(IJsonApiContext context) { return new Dictionary { diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs index f126dbee9d..8e05ac9914 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs @@ -621,5 +621,55 @@ public async Task Can_Delete_Relationship_By_Patching_Relationship() Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.Null(todoItemResult.Owner); } + + [Fact] + public async Task Updating_ToOne_Relationship_With_Implicit_Remove() + { + + + // Arrange + var context = _fixture.GetService(); + var passport = new Passport(); + var person1 = _personFaker.Generate(); + person1.Passport = passport; + var person2 = _personFaker.Generate(); + context.People.AddRange(new List() { person1, person2 }); + await context.SaveChangesAsync(); + + + + var content = new + { + data = new + { + type = "people", + id = person2.Id, + relationships = new Dictionary + { + { "passport", new + { + data = new { type = "passports", id = $"{person1.PassportId}" } + } + } + } + } + }; + + var httpMethod = new HttpMethod("PATCH"); + var route = $"/api/v1/people/{person2.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 _fixture.Client.SendAsync(request); + + // Assert + var body = await response.Content.ReadAsStringAsync(); + Assert.True(HttpStatusCode.OK == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}"); + + } } } From 4a3016cd93cc727c72c79ae421058839d8c5b40e Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Tue, 30 Apr 2019 11:34:40 +0200 Subject: [PATCH 2/5] chore: link with #502 --- .../Acceptance/Spec/UpdatingRelationshipsTests.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs index 8e05ac9914..ac464ed68e 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs @@ -625,8 +625,6 @@ public async Task Can_Delete_Relationship_By_Patching_Relationship() [Fact] public async Task Updating_ToOne_Relationship_With_Implicit_Remove() { - - // Arrange var context = _fixture.GetService(); var passport = new Passport(); @@ -636,8 +634,6 @@ public async Task Updating_ToOne_Relationship_With_Implicit_Remove() context.People.AddRange(new List() { person1, person2 }); await context.SaveChangesAsync(); - - var content = new { data = new From 91580bfee920ba11204fac94301ac7f93cc7ae7d Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Tue, 30 Apr 2019 13:19:29 +0200 Subject: [PATCH 3/5] fix:#502 --- .../Data/DefaultEntityRepository.cs | 11 ++++ .../Extensions/DbContextExtensions.cs | 24 ++++++++ .../Spec/UpdatingRelationshipsTests.cs | 60 ++++++++++++++++++- 3 files changed, 94 insertions(+), 1 deletion(-) diff --git a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs index 9922c7c84f..9e135ec915 100644 --- a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs +++ b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs @@ -2,6 +2,7 @@ using System.Collections; using System.Collections.Generic; using System.Linq; +using System.Reflection; using System.Threading.Tasks; using JsonApiDotNetCore.Extensions; using JsonApiDotNetCore.Internal; @@ -10,6 +11,7 @@ using JsonApiDotNetCore.Models; using JsonApiDotNetCore.Services; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Metadata; using Microsoft.Extensions.Logging; namespace JsonApiDotNetCore.Data @@ -338,6 +340,15 @@ public virtual async Task UpdateAsync(TId id, TEntity entity) { if ((relationship.Key.TypeId as Type).IsAssignableFrom(typeof(HasOneAttribute))) { + var attr = relationship.Key; + if (_jsonApiContext.HasOneRelationshipPointers.Get().TryGetValue(attr, out var pointer) ) + { + /// we need to attach inverse relations to make sure + /// we're not violating any foreign key constraints + /// when implicitely removing pre-existing relations. + /// See #502 for more info. + _context.LoadInverseNavigation(attr, pointer); + } relationship.Key.SetValue(oldEntity, relationship.Value); } if ((relationship.Key.TypeId as Type).IsAssignableFrom(typeof(HasManyAttribute))) diff --git a/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs b/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs index 42f998891c..40784bdd40 100644 --- a/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs @@ -14,6 +14,30 @@ public static class DbContextExtensions public static DbSet GetDbSet(this DbContext context) where T : class => context.Set(); + public static void LoadInverseNavigation( + this DbContext context, + RelationshipAttribute parentToChildAttribute, + object childEntity) where TParent : class, IIdentifiable + { + var navigationMeta = context.Model + .FindEntityType(typeof(TParent)) + .FindNavigation(parentToChildAttribute.InternalRelationshipName); + var inverseNavigationMeta = navigationMeta.FindInverse(); + if (inverseNavigationMeta != null) + { + var inversePropertyType = inverseNavigationMeta.PropertyInfo.PropertyType; + var inversePropertyName = inverseNavigationMeta.Name; + var entityEntry = context.Entry(childEntity); + if (inversePropertyType.IsGenericType ) + { // if generic, means we're dealing with a list + entityEntry.Collection(inversePropertyName).Load(); + } else + { + entityEntry.Navigation(inversePropertyName).Load(); + } + } + } + /// /// Get the DbSet when the model type is unknown until runtime /// diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs index ac464ed68e..4e4f174462 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs @@ -634,6 +634,60 @@ public async Task Updating_ToOne_Relationship_With_Implicit_Remove() context.People.AddRange(new List() { person1, person2 }); await context.SaveChangesAsync(); + var passportId = person1.PassportId; + + var content = new + { + data = new + { + type = "people", + id = person2.Id, + relationships = new Dictionary + { + { "passport", new + { + data = new { type = "passports", id = $"{passportId}" } + } + } + } + } + }; + + var httpMethod = new HttpMethod("PATCH"); + var route = $"/api/v1/people/{person2.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 _fixture.Client.SendAsync(request); + + // Assert + var body = await response.Content.ReadAsStringAsync(); + Assert.True(HttpStatusCode.OK == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}"); + + var _person1 = context.People.AsNoTracking().Include(ppl => ppl.Passport).Single(ppl => ppl.Id == person1.Id); + var _person2 = context.People.AsNoTracking().Include(ppl => ppl.Passport).Single(ppl => ppl.Id == person2.Id); + Assert.Null(_person1.Passport); + Assert.Equal(passportId, _person2.PassportId); + } + + [Fact] + public async Task Updating_ToMany_Relationship_With_Implicit_Remove() + { + // Arrange + var context = _fixture.GetService(); + var passport = new Passport(); + var person1 = _personFaker.Generate(); + person1.Passport = passport; + var person2 = _personFaker.Generate(); + context.People.AddRange(new List() { person1, person2 }); + await context.SaveChangesAsync(); + + var passportId = person1.PassportId; + var content = new { data = new @@ -644,7 +698,7 @@ public async Task Updating_ToOne_Relationship_With_Implicit_Remove() { { "passport", new { - data = new { type = "passports", id = $"{person1.PassportId}" } + data = new { type = "passports", id = $"{passportId}" } } } } @@ -666,6 +720,10 @@ public async Task Updating_ToOne_Relationship_With_Implicit_Remove() var body = await response.Content.ReadAsStringAsync(); Assert.True(HttpStatusCode.OK == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}"); + var _person1 = context.People.AsNoTracking().Include(ppl => ppl.Passport).Single(ppl => ppl.Id == person1.Id); + var _person2 = context.People.AsNoTracking().Include(ppl => ppl.Passport).Single(ppl => ppl.Id == person2.Id); + Assert.Null(_person1.Passport); + Assert.Equal(passportId, _person2.PassportId); } } } From 265d51730f73a52d0b6a8aff3b209f2d5756ac36 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Tue, 30 Apr 2019 13:39:34 +0200 Subject: [PATCH 4/5] chore: removed duplicate test, fix typo in comment --- .../Data/DefaultEntityRepository.cs | 2 +- .../Spec/UpdatingRelationshipsTests.cs | 52 ------------------- 2 files changed, 1 insertion(+), 53 deletions(-) diff --git a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs index 9e135ec915..167c73b7ab 100644 --- a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs +++ b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs @@ -345,7 +345,7 @@ public virtual async Task UpdateAsync(TId id, TEntity entity) { /// we need to attach inverse relations to make sure /// we're not violating any foreign key constraints - /// when implicitely removing pre-existing relations. + /// when implicitly removing pre-existing relations. /// See #502 for more info. _context.LoadInverseNavigation(attr, pointer); } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs index 4e4f174462..f375c6ecc2 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs @@ -673,57 +673,5 @@ public async Task Updating_ToOne_Relationship_With_Implicit_Remove() Assert.Null(_person1.Passport); Assert.Equal(passportId, _person2.PassportId); } - - [Fact] - public async Task Updating_ToMany_Relationship_With_Implicit_Remove() - { - // Arrange - var context = _fixture.GetService(); - var passport = new Passport(); - var person1 = _personFaker.Generate(); - person1.Passport = passport; - var person2 = _personFaker.Generate(); - context.People.AddRange(new List() { person1, person2 }); - await context.SaveChangesAsync(); - - var passportId = person1.PassportId; - - var content = new - { - data = new - { - type = "people", - id = person2.Id, - relationships = new Dictionary - { - { "passport", new - { - data = new { type = "passports", id = $"{passportId}" } - } - } - } - } - }; - - var httpMethod = new HttpMethod("PATCH"); - var route = $"/api/v1/people/{person2.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 _fixture.Client.SendAsync(request); - - // Assert - var body = await response.Content.ReadAsStringAsync(); - Assert.True(HttpStatusCode.OK == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}"); - - var _person1 = context.People.AsNoTracking().Include(ppl => ppl.Passport).Single(ppl => ppl.Id == person1.Id); - var _person2 = context.People.AsNoTracking().Include(ppl => ppl.Passport).Single(ppl => ppl.Id == person2.Id); - Assert.Null(_person1.Passport); - Assert.Equal(passportId, _person2.PassportId); - } } } From c159c6fb7fefd53c204350c120e06fc5f1fb8237 Mon Sep 17 00:00:00 2001 From: Maurits Moeys Date: Tue, 30 Apr 2019 13:47:40 +0200 Subject: [PATCH 5/5] chore: add comment --- src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs b/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs index 40784bdd40..963e925c66 100644 --- a/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs @@ -14,6 +14,15 @@ public static class DbContextExtensions public static DbSet GetDbSet(this DbContext context) where T : class => context.Set(); + /// + /// Given a child entity and a relationship attribute between a parent + /// entity to that child entity, attaches the entities on the inverse navigation + /// property to the dbContext. + /// + /// Context. + /// Parent to child relationship attribute. + /// Child entity. + /// The 1st type parameter. public static void LoadInverseNavigation( this DbContext context, RelationshipAttribute parentToChildAttribute,