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..63eea72cb0 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Models/Person.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Models/Person.cs @@ -45,5 +45,9 @@ public Dictionary GetMeta(IJsonApiContext context) { "authors", new string[] { "Jared Nance" } } }; } + public int? PassportId { get; set; } + [HasOne("passport")] + public virtual Passport Passport { get; set; } + } } diff --git a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs index 4cd32de946..c5fa47b6ab 100644 --- a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs +++ b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs @@ -11,7 +11,6 @@ using JsonApiDotNetCore.Services; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; - namespace JsonApiDotNetCore.Data { /// @@ -51,7 +50,6 @@ public class DefaultEntityRepository private readonly IJsonApiContext _jsonApiContext; private readonly IGenericProcessorFactory _genericProcessorFactory; private readonly ResourceDefinition _resourceDefinition; - public DefaultEntityRepository( IJsonApiContext jsonApiContext, IDbContextResolver contextResolver, @@ -82,6 +80,7 @@ public DefaultEntityRepository( public virtual IQueryable Get() => _dbSet; + /// public virtual IQueryable Select(IQueryable entities, List fields) { if (fields?.Count > 0) @@ -101,7 +100,6 @@ public virtual IQueryable Filter(IQueryable entities, FilterQu return defaultQueryFilter(entities, filterQuery); } } - return entities.Filter(_jsonApiContext, filterQuery); } @@ -123,7 +121,6 @@ public virtual IQueryable Sort(IQueryable entities, List GetAndIncludeAsync(TId id, string relationshi /// public virtual async Task CreateAsync(TEntity entity) { - AttachRelationships(entity); + foreach (var relationshipAttr in _jsonApiContext.RelationshipsToUpdate?.Keys) + { + var trackedRelationshipValue = GetTrackedRelationshipValue(relationshipAttr, entity, out bool wasAlreadyTracked); + LoadInverseRelationships(trackedRelationshipValue, relationshipAttr); + if (wasAlreadyTracked) + { + /// We only need to reassign the relationship value to the to-be-added + /// entity when we're using a different instance (because this different one + /// was already tracked) than the one assigned to the to-be-created entity. + AssignRelationshipValue(entity, trackedRelationshipValue, relationshipAttr); + } + else if (relationshipAttr is HasManyThroughAttribute throughAttr) + { + /// even if we don't have to reassign anything because of already tracked + /// entities, we still need to assign the "through" entities in the case of many-to-many. + AssignHasManyThrough(entity, throughAttr, (IList)trackedRelationshipValue); + } + } _dbSet.Add(entity); - await _context.SaveChangesAsync(); return entity; } - protected virtual void AttachRelationships(TEntity entity = null) - { - AttachHasManyPointers(entity); - AttachHasOnePointers(entity); - } - - /// - public void DetachRelationshipPointers(TEntity entity) + /// + /// Loads the inverse relationships to prevent foreign key constraints from being violated + /// to support implicit removes, see https://github.com/json-api-dotnet/JsonApiDotNetCore/issues/502. + /// + /// Consider the following example: + /// person.todoItems = [t1,t2] is updated to [t3, t4]. If t3, and/or t4 was + /// already related to a other person, and these persons are NOT loaded in to the + /// db context, then the query may cause a foreign key constraint. Loading + /// these "inverse relationships" into the DB context ensures EF core to take + /// this into account. + /// + /// + private void LoadInverseRelationships(object trackedRelationshipValue, RelationshipAttribute relationshipAttr) { - foreach (var hasOneRelationship in _jsonApiContext.HasOneRelationshipPointers.Get()) + if (relationshipAttr.InverseNavigation == null || trackedRelationshipValue == null) return; + if (relationshipAttr is HasOneAttribute hasOneAttr) { - var hasOne = (HasOneAttribute)hasOneRelationship.Key; - if (hasOne.EntityPropertyName != null) + var relationEntry = _context.Entry((IIdentifiable)trackedRelationshipValue); + if (IsHasOneRelationship(hasOneAttr.InverseNavigation, trackedRelationshipValue.GetType())) { - var relatedEntity = entity.GetType().GetProperty(hasOne.EntityPropertyName)?.GetValue(entity); - if (relatedEntity != null) - _context.Entry(relatedEntity).State = EntityState.Detached; + relationEntry.Reference(hasOneAttr.InverseNavigation).Load(); } else { - _context.Entry(hasOneRelationship.Value).State = EntityState.Detached; + relationEntry.Collection(hasOneAttr.InverseNavigation).Load(); } } - - foreach (var hasManyRelationship in _jsonApiContext.HasManyRelationshipPointers.Get()) + else if (relationshipAttr is HasManyAttribute hasManyAttr && !(relationshipAttr is HasManyThroughAttribute)) { - var hasMany = (HasManyAttribute)hasManyRelationship.Key; - if (hasMany.EntityPropertyName != null) + foreach (IIdentifiable relationshipValue in (IList)trackedRelationshipValue) { - var relatedList = (IList)entity.GetType().GetProperty(hasMany.EntityPropertyName)?.GetValue(entity); - foreach (var related in relatedList) - { - _context.Entry(related).State = EntityState.Detached; - } - } - else - { - foreach (var pointer in hasManyRelationship.Value) - { - _context.Entry(pointer).State = EntityState.Detached; - } + _context.Entry(relationshipValue).Reference(hasManyAttr.InverseNavigation).Load(); } - - // 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 - hasManyRelationship.Key.SetValue(entity, null); } } - /// - /// This is used to allow creation of HasMany relationships when the - /// dependent side of the relationship already exists. - /// - private void AttachHasManyPointers(TEntity entity) - { - var relationships = _jsonApiContext.HasManyRelationshipPointers.Get(); - foreach (var relationship in relationships) - { - if (relationship.Key is HasManyThroughAttribute hasManyThrough) - AttachHasManyThrough(entity, hasManyThrough, relationship.Value); - else - AttachHasMany(entity, relationship.Key as HasManyAttribute, relationship.Value); - } - } - private void AttachHasMany(TEntity entity, HasManyAttribute relationship, IList pointers) + private bool IsHasOneRelationship(string internalRelationshipName, Type type) { - if (relationship.EntityPropertyName != null) + var relationshipAttr = _jsonApiContext.ResourceGraph.GetContextEntity(type).Relationships.SingleOrDefault(r => r.InternalRelationshipName == internalRelationshipName); + if (relationshipAttr != null) { - var relatedList = (IList)entity.GetType().GetProperty(relationship.EntityPropertyName)?.GetValue(entity); - foreach (var related in relatedList) - { - if (_context.EntityIsTracked(related as IIdentifiable) == false) - _context.Entry(related).State = EntityState.Unchanged; - } + if (relationshipAttr is HasOneAttribute) return true; + return false; } else { - foreach (var pointer in pointers) - { - if (_context.EntityIsTracked(pointer as IIdentifiable) == false) - _context.Entry(pointer).State = EntityState.Unchanged; - } + // relationshipAttr is null when there is not put a [RelationshipAttribute] on the inverse navigation property. + // In this case we use relfection to figure out what kind of relationship is pointing back. + return !(type.GetProperty(internalRelationshipName).PropertyType.Inherits(typeof(IEnumerable))); } } - private void AttachHasManyThrough(TEntity entity, HasManyThroughAttribute hasManyThrough, IList pointers) - { - // create the collection (e.g. List) - // this type MUST implement IList so we can build the collection - // if this is problematic, we _could_ reflect on the type and find an Add method - // or we might be able to create a proxy type and implement the enumerator - var throughRelationshipCollection = Activator.CreateInstance(hasManyThrough.ThroughProperty.PropertyType) as IList; - hasManyThrough.ThroughProperty.SetValue(entity, throughRelationshipCollection); - - foreach (var pointer in pointers) - { - if (_context.EntityIsTracked(pointer as IIdentifiable) == false) - _context.Entry(pointer).State = EntityState.Unchanged; - var throughInstance = Activator.CreateInstance(hasManyThrough.ThroughType); - - hasManyThrough.LeftProperty.SetValue(throughInstance, entity); - hasManyThrough.RightProperty.SetValue(throughInstance, pointer); - - throughRelationshipCollection.Add(throughInstance); - } - } - /// - /// This is used to allow creation of HasOne relationships when the - /// independent side of the relationship already exists. - /// - private void AttachHasOnePointers(TEntity entity) + /// + public void DetachRelationshipPointers(TEntity entity) { - var relationships = _jsonApiContext.HasOneRelationshipPointers.Get(); - foreach (var relationship in relationships) - { - if (relationship.Key.GetType() != typeof(HasOneAttribute)) - continue; - var hasOne = (HasOneAttribute)relationship.Key; - if (hasOne.EntityPropertyName != null) + foreach (var relationshipAttr in _jsonApiContext.RelationshipsToUpdate.Keys) + { + if (relationshipAttr is HasOneAttribute hasOneAttr) { - var relatedEntity = entity.GetType().GetProperty(hasOne.EntityPropertyName)?.GetValue(entity); - if (relatedEntity != null && _context.Entry(relatedEntity).State == EntityState.Detached && _context.EntityIsTracked((IIdentifiable)relatedEntity) == false) - _context.Entry(relatedEntity).State = EntityState.Unchanged; + var relationshipValue = GetEntityResourceSeparationValue(entity, hasOneAttr) ?? (IIdentifiable)hasOneAttr.GetValue(entity); + if (relationshipValue == null) continue; + _context.Entry(relationshipValue).State = EntityState.Detached; + } else { - if (_context.Entry(relationship.Value).State == EntityState.Detached && _context.EntityIsTracked(relationship.Value) == false) - _context.Entry(relationship.Value).State = EntityState.Unchanged; + IEnumerable relationshipValueList = (IEnumerable)relationshipAttr.GetValue(entity); + /// This adds support for resource-entity separation in the case of one-to-many. + /// todo: currently there is no support for many to many relations. + if (relationshipAttr is HasManyAttribute hasMany) + relationshipValueList = GetEntityResourceSeparationValue(entity, hasMany) ?? relationshipValueList; + if (relationshipValueList == null) continue; + foreach (var pointer in relationshipValueList) + { + _context.Entry(pointer).State = EntityState.Detached; + } + /// detaching has many relationships is not sufficient to + /// trigger a full reload of relationships: the navigation + /// property actually needs to be nulled out, otherwise + /// EF will still add duplicate instances to the collection + relationshipAttr.SetValue(entity, null); } } } - /// - public virtual async Task UpdateAsync(TId id, TEntity entity) + [Obsolete("Use overload UpdateAsync(TEntity updatedEntity): providing parameter ID does no longer add anything relevant")] + public virtual async Task UpdateAsync(TId id, TEntity updatedEntity) { - var oldEntity = await GetAsync(id); + return await UpdateAsync(updatedEntity); + } - if (oldEntity == null) + /// + public virtual async Task UpdateAsync(TEntity updatedEntity) + { + var databaseEntity = await GetAsync(updatedEntity.Id); + if (databaseEntity == null) return null; - foreach (var attr in _jsonApiContext.AttributesToUpdate) - attr.Key.SetValue(oldEntity, attr.Value); + foreach (var attr in _jsonApiContext.AttributesToUpdate.Keys) + attr.SetValue(databaseEntity, attr.GetValue(updatedEntity)); - if (_jsonApiContext.RelationshipsToUpdate.Any()) + foreach (var relationshipAttr in _jsonApiContext.RelationshipsToUpdate?.Keys) { - /// For one-to-many and many-to-many, the PATCH must perform a - /// complete replace. When assigning new relationship values, - /// it will only be like this if the to-be-replaced entities are loaded - foreach (var relationship in _jsonApiContext.RelationshipsToUpdate) - { - if (relationship.Key is HasManyThroughAttribute throughAttribute) - { - await _context.Entry(oldEntity).Collection(throughAttribute.InternalThroughName).LoadAsync(); - } - } - - /// @HACK @TODO: It is inconsistent that for many-to-many, the new relationship value - /// is assigned in AttachRelationships() helper fn below, but not for - /// one-to-many and one-to-one (we need to do that manually as done below). - /// Simultaneously, for a proper working "complete replacement", in the case of many-to-many - /// we need to LoadAsync() BEFORE calling AttachRelationships(), but for one-to-many we - /// need to do it AFTER AttachRelationships or we we'll get entity tracking errors - /// This really needs a refactor. - AttachRelationships(oldEntity); - - foreach (var relationship in _jsonApiContext.RelationshipsToUpdate) - { - if ((relationship.Key.TypeId as Type).IsAssignableFrom(typeof(HasOneAttribute))) - { - relationship.Key.SetValue(oldEntity, relationship.Value); - } - if ((relationship.Key.TypeId as Type).IsAssignableFrom(typeof(HasManyAttribute))) - { - await _context.Entry(oldEntity).Collection(relationship.Key.InternalRelationshipName).LoadAsync(); - var value = PreventReattachment((IEnumerable)relationship.Value); - relationship.Key.SetValue(oldEntity, value); - } - } + /// loads databasePerson.todoItems + LoadCurrentRelationships(databaseEntity, relationshipAttr); + /// trackedRelationshipValue is either equal to updatedPerson.todoItems + /// or replaced with the same set of todoItems from the EF Core change tracker, + /// if they were already tracked + object trackedRelationshipValue = GetTrackedRelationshipValue(relationshipAttr, updatedEntity, out bool wasAlreadyTracked); + /// loads into the db context any persons currently related + /// to the todoItems in trackedRelationshipValue + LoadInverseRelationships(trackedRelationshipValue, relationshipAttr); + /// assigns the updated relationship to the database entity + AssignRelationshipValue(databaseEntity, trackedRelationshipValue, relationshipAttr); } + await _context.SaveChangesAsync(); - return oldEntity; + return databaseEntity; } + /// - /// We need to make sure we're not re-attaching entities when assigning - /// new relationship values. Entities may have been loaded in the change - /// tracker anywhere in the application beyond the control of - /// JsonApiDotNetCore. + /// Responsible for getting the relationship value for a given relationship + /// attribute of a given entity. It ensures that the relationship value + /// that it returns is attached to the database without reattaching duplicates instances + /// to the change tracker. It does so by checking if there already are + /// instances of the to-be-attached entities in the change tracker. /// - /// The interpolated related entity collection - /// Related entities. - object PreventReattachment(IEnumerable relatedEntities) + private object GetTrackedRelationshipValue(RelationshipAttribute relationshipAttr, TEntity entity, out bool wasAlreadyAttached) { - var relatedType = TypeHelper.GetTypeOfList(relatedEntities.GetType()); - var replaced = relatedEntities.Cast().Select(entity => _context.GetTrackedEntity(entity) ?? entity); - return TypeHelper.ConvertCollection(replaced, relatedType); + wasAlreadyAttached = false; + if (relationshipAttr is HasOneAttribute hasOneAttr) + { + /// This adds support for resource-entity separation in the case of one-to-one. + var relationshipValue = GetEntityResourceSeparationValue(entity, hasOneAttr) ?? (IIdentifiable)hasOneAttr.GetValue(entity); + if (relationshipValue == null) + return null; + return GetTrackedHasOneRelationshipValue(relationshipValue, hasOneAttr, ref wasAlreadyAttached); + } + else + { + IEnumerable relationshipValueList = (IEnumerable)relationshipAttr.GetValue(entity); + /// This adds support for resource-entity separation in the case of one-to-many. + /// todo: currently there is no support for many to many relations. + if (relationshipAttr is HasManyAttribute hasMany) + relationshipValueList = GetEntityResourceSeparationValue(entity, hasMany) ?? relationshipValueList; + if (relationshipValueList == null) return null; + return GetTrackedManyRelationshipValue(relationshipValueList, relationshipAttr, ref wasAlreadyAttached); + } + } + // helper method used in GetTrackedRelationshipValue. See comments there. + private IList GetTrackedManyRelationshipValue(IEnumerable relationshipValueList, RelationshipAttribute relationshipAttr, ref bool wasAlreadyAttached) + { + if (relationshipValueList == null) return null; + bool _wasAlreadyAttached = false; + Type entityType = null; + var trackedPointerCollection = relationshipValueList.Select(pointer => + { + /// todo: we can't just use relationshipAttr.Type because + /// this will point to the Resource type in the case of entity resource + /// separation. We should consider to store entity type on + /// the relationship attribute too. + entityType = entityType ?? pointer.GetType(); + var tracked = AttachOrGetTracked(pointer); + if (tracked != null) _wasAlreadyAttached = true; + return Convert.ChangeType(tracked ?? pointer, entityType); + }).ToList().Cast(entityType); + if (_wasAlreadyAttached) wasAlreadyAttached = true; + return (IList)trackedPointerCollection; } + // helper method used in GetTrackedRelationshipValue. See comments there. + private IIdentifiable GetTrackedHasOneRelationshipValue(IIdentifiable relationshipValue, HasOneAttribute hasOneAttr, ref bool wasAlreadyAttached) + { + var tracked = AttachOrGetTracked(relationshipValue); + if (tracked != null) wasAlreadyAttached = true; + return tracked ?? relationshipValue; + } /// public async Task UpdateRelationshipsAsync(object parent, RelationshipAttribute relationship, IEnumerable relationshipIds) @@ -374,18 +366,14 @@ public async Task UpdateRelationshipsAsync(object parent, RelationshipAttribute await genericProcessor.UpdateRelationshipsAsync(parent, relationship, relationshipIds); } + /// public virtual async Task DeleteAsync(TId id) { var entity = await GetAsync(id); - - if (entity == null) - return false; - + if (entity == null) return false; _dbSet.Remove(entity); - await _context.SaveChangesAsync(); - return true; } @@ -470,5 +458,123 @@ public async Task> ToListAsync(IQueryable entiti ? await entities.ToListAsync() : entities.ToList(); } + + + /// + /// Before assigning new relationship values (UpdateAsync), we need to + /// attach the current database values of the relationship to the dbcontext, else + /// it will not perform a complete-replace which is required for + /// one-to-many and many-to-many. + /// + /// For example: a person `p1` has 2 todoitems: `t1` and `t2`. + /// If we want to update this todoitem set to `t3` and `t4`, simply assigning + /// `p1.todoItems = [t3, t4]` will result in EF Core adding them to the set, + /// resulting in `[t1 ... t4]`. Instead, we should first include `[t1, t2]`, + /// after which the reassignment `p1.todoItems = [t3, t4]` will actually + /// make EF Core perform a complete replace. This method does the loading of `[t1, t2]`. + /// + protected void LoadCurrentRelationships(TEntity oldEntity, RelationshipAttribute relationshipAttribute) + { + if (relationshipAttribute is HasManyThroughAttribute throughAttribute) + { + _context.Entry(oldEntity).Collection(throughAttribute.InternalThroughName).Load(); + + } + else if (relationshipAttribute is HasManyAttribute hasManyAttribute) + { + _context.Entry(oldEntity).Collection(hasManyAttribute.InternalRelationshipName).Load(); + } + } + + /// + /// Assigns the to + /// + private void AssignRelationshipValue(TEntity targetEntity, object relationshipValue, RelationshipAttribute relationshipAttribute) + { + if (relationshipAttribute is HasManyThroughAttribute throughAttribute) + { + // todo: this logic should be put in the HasManyThrough attribute + AssignHasManyThrough(targetEntity, throughAttribute, (IList)relationshipValue); + } + else + { + relationshipAttribute.SetValue(targetEntity, relationshipValue); + } + } + + /// + /// The relationshipValue parameter contains the dependent side of the relationship (Tags). + /// We can't directly add them to the principal entity (Article): we need to + /// use the join table (ArticleTags). This methods assigns the relationship value to entity + /// by taking care of that + /// + private void AssignHasManyThrough(TEntity entity, HasManyThroughAttribute hasManyThrough, IList relationshipValue) + { + var pointers = relationshipValue.Cast(); + var throughRelationshipCollection = Activator.CreateInstance(hasManyThrough.ThroughProperty.PropertyType) as IList; + hasManyThrough.ThroughProperty.SetValue(entity, throughRelationshipCollection); + + foreach (var pointer in pointers) + { + var throughInstance = Activator.CreateInstance(hasManyThrough.ThroughType); + hasManyThrough.LeftProperty.SetValue(throughInstance, entity); + hasManyThrough.RightProperty.SetValue(throughInstance, pointer); + throughRelationshipCollection.Add(throughInstance); + } + } + + /// + /// A helper method that gets the relationship value in the case of + /// entity resource separation. + /// + private IIdentifiable GetEntityResourceSeparationValue(TEntity entity, HasOneAttribute attribute) + { + if (attribute.EntityPropertyName == null) + { + return null; + } + return (IIdentifiable)entity.GetType().GetProperty(attribute.EntityPropertyName)?.GetValue(entity); + } + + /// + /// A helper method that gets the relationship value in the case of + /// entity resource separation. + /// + private IEnumerable GetEntityResourceSeparationValue(TEntity entity, HasManyAttribute attribute) + { + if (attribute.EntityPropertyName == null) + { + return null; + } + return ((IEnumerable)(entity.GetType().GetProperty(attribute.EntityPropertyName)?.GetValue(entity))).Cast(); + } + + /// + /// Given a iidentifiable relationshipvalue, verify if an entity of the underlying + /// type with the same ID is already attached to the dbContext, and if so, return it. + /// If not, attach the relationship value to the dbContext. + /// + /// useful article: https://stackoverflow.com/questions/30987806/dbset-attachentity-vs-dbcontext-entryentity-state-entitystate-modified + /// + private IIdentifiable AttachOrGetTracked(IIdentifiable relationshipValue) + { + var trackedEntity = _context.GetTrackedEntity(relationshipValue); + + if (trackedEntity != null) + { + /// there already was an instance of this type and ID tracked + /// by EF Core. Reattaching will produce a conflict, so from now on we + /// will use the already attached instance instead. This entry might + /// contain updated fields as a result of business logic elsewhere in the application + return trackedEntity; + } + + /// the relationship pointer is new to EF Core, but we are sure + /// it exists in the database, so we attach it. In this case, as per + /// the json:api spec, we can also safely assume that no fields of + /// this entity were updated. + _context.Entry(relationshipValue).State = EntityState.Unchanged; + return null; + } } } diff --git a/src/JsonApiDotNetCore/Data/IEntityWriteRepository.cs b/src/JsonApiDotNetCore/Data/IEntityWriteRepository.cs index 37c60b8521..aa143f53e4 100644 --- a/src/JsonApiDotNetCore/Data/IEntityWriteRepository.cs +++ b/src/JsonApiDotNetCore/Data/IEntityWriteRepository.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Threading.Tasks; using JsonApiDotNetCore.Models; @@ -14,6 +15,9 @@ public interface IEntityWriteRepository { Task CreateAsync(TEntity entity); + Task UpdateAsync(TEntity entity); + + [Obsolete("Use overload UpdateAsync(TEntity updatedEntity): providing parameter ID does no longer add anything relevant")] Task UpdateAsync(TId id, TEntity entity); Task UpdateRelationshipsAsync(object parent, RelationshipAttribute relationship, IEnumerable relationshipIds); diff --git a/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs b/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs index 42f998891c..e90471083c 100644 --- a/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs @@ -1,8 +1,10 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using JsonApiDotNetCore.Models; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.ChangeTracking.Internal; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Storage; @@ -32,6 +34,7 @@ public static bool EntityIsTracked(this DbContext context, IIdentifiable entity) return GetTrackedEntity(context, entity) != null; } + /// /// Determines whether or not EF is already tracking an entity of the same Type and Id /// and returns that entity. diff --git a/src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs b/src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs index 20cee086d6..e66b51a7a0 100644 --- a/src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/IApplicationBuilderExtensions.cs @@ -4,6 +4,7 @@ using JsonApiDotNetCore.Middleware; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; namespace JsonApiDotNetCore.Extensions @@ -21,6 +22,12 @@ public static IApplicationBuilder UseJsonApi(this IApplicationBuilder app, bool if (useMvc) app.UseMvc(); + using (var scope = app.ApplicationServices.CreateScope()) + { + var inverseRelationshipResolver = scope.ServiceProvider.GetService(); + inverseRelationshipResolver?.Resolve(); + } + return app; } diff --git a/src/JsonApiDotNetCore/Extensions/IServiceCollectionExtensions.cs b/src/JsonApiDotNetCore/Extensions/IServiceCollectionExtensions.cs index d1c1de352c..a44020ae5b 100644 --- a/src/JsonApiDotNetCore/Extensions/IServiceCollectionExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/IServiceCollectionExtensions.cs @@ -155,6 +155,9 @@ public static void AddJsonApiInternals( services.AddScoped(); services.AddScoped(); + services.AddScoped(); + + // services.AddScoped(); } diff --git a/src/JsonApiDotNetCore/Extensions/TypeExtensions.cs b/src/JsonApiDotNetCore/Extensions/TypeExtensions.cs index 03874d9b76..9ebc942b03 100644 --- a/src/JsonApiDotNetCore/Extensions/TypeExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/TypeExtensions.cs @@ -8,6 +8,42 @@ namespace JsonApiDotNetCore.Extensions { internal static class TypeExtensions { + /// + /// Extension to use the LINQ AddRange method on an IList + /// + public static void AddRange(this IList list, IEnumerable items) + { + if (list == null) throw new ArgumentNullException(nameof(list)); + if (items == null) throw new ArgumentNullException(nameof(items)); + + if (list is List) + { + ((List)list).AddRange(items); + } + else + { + foreach (var item in items) + { + list.Add(item); + } + } + } + + + /// + /// Extension to use the LINQ cast method in a non-generic way: + /// + /// Type targetType = typeof(TEntity) + /// ((IList)myList).Cast(targetType). + /// + /// + public static IEnumerable Cast(this IEnumerable source, Type type) + { + if (source == null) throw new ArgumentNullException(nameof(source)); + if (type == null) throw new ArgumentNullException(nameof(type)); + return TypeHelper.ConvertCollection(source.Cast(), type); + } + public static Type GetElementType(this IEnumerable enumerable) { var enumerableTypes = enumerable.GetType() diff --git a/src/JsonApiDotNetCore/Formatters/JsonApiReader.cs b/src/JsonApiDotNetCore/Formatters/JsonApiReader.cs index b5c39ef4d5..b305bf9722 100644 --- a/src/JsonApiDotNetCore/Formatters/JsonApiReader.cs +++ b/src/JsonApiDotNetCore/Formatters/JsonApiReader.cs @@ -1,7 +1,9 @@ using System; +using System.Collections; using System.IO; using System.Threading.Tasks; using JsonApiDotNetCore.Internal; +using JsonApiDotNetCore.Models; using JsonApiDotNetCore.Serialization; using JsonApiDotNetCore.Services; using Microsoft.AspNetCore.Mvc.Formatters; @@ -37,7 +39,7 @@ public Task ReadAsync(InputFormatterContext context) { var body = GetRequestBody(context.HttpContext.Request.Body); - object model =null; + object model = null; if (_jsonApiContext.IsRelationshipPath) { @@ -48,10 +50,29 @@ public Task ReadAsync(InputFormatterContext context) model = _deSerializer.Deserialize(body); } + if (model == null) { _logger?.LogError("An error occurred while de-serializing the payload"); } + + if (context.HttpContext.Request.Method == "PATCH") + { + bool idMissing; + if (model is IList list) + { + idMissing = CheckForId(list); + } + else + { + idMissing = CheckForId(model); + } + if (idMissing) + { + _logger?.LogError("Payload must include id attribute"); + throw new JsonApiException(400, "Payload must include id attribute"); + } + } return InputFormatterResult.SuccessAsync(model); } catch (Exception ex) @@ -62,6 +83,40 @@ public Task ReadAsync(InputFormatterContext context) } } + /// Checks if the deserialized payload has an ID included Checks if the elements in the deserialized payload have an ID included + /// Responsible for populating the RelationshipAttribute InverseNavigation property. + /// + /// This service is instantiated in the configure phase of the application. + /// + /// When using a data access layer different from EF Core, and when using ResourceHooks + /// that depend on the inverse navigation property (BeforeImplicitUpdateRelationship), + /// you will need to override this service, or pass along the inverseNavigationProperty in + /// the RelationshipAttribute. + /// + public interface IInverseRelationships + { + /// + /// This method is called upon startup by JsonApiDotNetCore. It should + /// deal with resolving the inverse relationships. + /// + void Resolve(); + } + + /// + public class InverseRelationships : IInverseRelationships + { + private readonly ResourceGraph _graph; + private readonly IDbContextResolver _resolver; + + public InverseRelationships(IResourceGraph graph, IDbContextResolver resolver = null) + { + _graph = (ResourceGraph)graph; + _resolver = resolver; + } + + /// + public void Resolve() + { + if (EntityFrameworkCoreIsEnabled()) + { + DbContext context = _resolver.GetContext(); + + foreach (ContextEntity ce in _graph.Entities) + { + IEntityType meta = context.Model.FindEntityType(ce.EntityType); + if (meta == null) continue; + foreach (var attr in ce.Relationships) + { + if (attr is HasManyThroughAttribute) continue; + INavigation inverseNavigation = meta.FindNavigation(attr.InternalRelationshipName)?.FindInverse(); + attr.InverseNavigation = inverseNavigation?.Name; + } + } + } + } + + /// + /// If EF Core is not being used, we're expecting the resolver to not be registered. + /// + /// true, if entity framework core was enabled, false otherwise. + /// Resolver. + private bool EntityFrameworkCoreIsEnabled() + { + return _resolver != null; + } + } +} diff --git a/src/JsonApiDotNetCore/Models/HasManyAttribute.cs b/src/JsonApiDotNetCore/Models/HasManyAttribute.cs index 2d69b0e9c7..37fe42c9af 100644 --- a/src/JsonApiDotNetCore/Models/HasManyAttribute.cs +++ b/src/JsonApiDotNetCore/Models/HasManyAttribute.cs @@ -24,9 +24,11 @@ public class HasManyAttribute : RelationshipAttribute /// /// /// - public HasManyAttribute(string publicName = null, Link documentLinks = Link.All, bool canInclude = true, string mappedBy = null) + public HasManyAttribute(string publicName = null, Link documentLinks = Link.All, bool canInclude = true, string mappedBy = null, string inverseNavigationProperty = null) : base(publicName, documentLinks, canInclude, mappedBy) - { } + { + InverseNavigation = inverseNavigationProperty; + } /// /// Sets the value of the property identified by this attribute diff --git a/src/JsonApiDotNetCore/Models/HasOneAttribute.cs b/src/JsonApiDotNetCore/Models/HasOneAttribute.cs index 45b08590b3..d46d5adfdf 100644 --- a/src/JsonApiDotNetCore/Models/HasOneAttribute.cs +++ b/src/JsonApiDotNetCore/Models/HasOneAttribute.cs @@ -28,10 +28,11 @@ public class HasOneAttribute : RelationshipAttribute /// /// /// - public HasOneAttribute(string publicName = null, Link documentLinks = Link.All, bool canInclude = true, string withForeignKey = null, string mappedBy = null) + public HasOneAttribute(string publicName = null, Link documentLinks = Link.All, bool canInclude = true, string withForeignKey = null, string mappedBy = null, string inverseNavigationProperty = null) : base(publicName, documentLinks, canInclude, mappedBy) { _explicitIdentifiablePropertyName = withForeignKey; + InverseNavigation = inverseNavigationProperty; } private readonly string _explicitIdentifiablePropertyName; @@ -50,14 +51,13 @@ public HasOneAttribute(string publicName = null, Link documentLinks = Link.All, /// The new property value public override void SetValue(object resource, object newValue) { - var propertyName = (newValue?.GetType() == Type) - ? InternalRelationshipName - : IdentifiablePropertyName; - - var propertyInfo = resource - .GetType() - .GetProperty(propertyName); + string propertyName = InternalRelationshipName; + // if we're deleting the relationship (setting it to null), + // we set the foreignKey to null. We could also set the actual property to null, + // but then we would first need to load the current relationship, which requires an extra query. + if (newValue == null) propertyName = IdentifiablePropertyName; + var propertyInfo = resource.GetType().GetProperty(propertyName); propertyInfo.SetValue(resource, newValue); } diff --git a/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs b/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs index bc6982f062..1c484889c3 100644 --- a/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs +++ b/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs @@ -15,8 +15,9 @@ protected RelationshipAttribute(string publicName, Link documentLinks, bool canI } public string PublicRelationshipName { get; internal set; } - public string InternalRelationshipName { get; internal set; } - + public string InternalRelationshipName { get; internal set; } + public string InverseNavigation { get; internal set; } + /// /// The related entity type. This does not necessarily match the navigation property type. /// In the case of a HasMany relationship, this value will be the generic argument type. diff --git a/src/JsonApiDotNetCore/Request/HasManyRelationshipPointers.cs b/src/JsonApiDotNetCore/Request/HasManyRelationshipPointers.cs index 04056e4af8..3fda5dc44e 100644 --- a/src/JsonApiDotNetCore/Request/HasManyRelationshipPointers.cs +++ b/src/JsonApiDotNetCore/Request/HasManyRelationshipPointers.cs @@ -32,7 +32,7 @@ namespace JsonApiDotNetCore.Request /// public class HasManyRelationshipPointers { - private Dictionary _hasManyRelationships = new Dictionary(); + private readonly Dictionary _hasManyRelationships = new Dictionary(); /// /// Add the relationship to the list of relationships that should be diff --git a/src/JsonApiDotNetCore/Request/HasOneRelationshipPointers.cs b/src/JsonApiDotNetCore/Request/HasOneRelationshipPointers.cs index 753aa35e98..19046b9eaa 100644 --- a/src/JsonApiDotNetCore/Request/HasOneRelationshipPointers.cs +++ b/src/JsonApiDotNetCore/Request/HasOneRelationshipPointers.cs @@ -28,18 +28,18 @@ namespace JsonApiDotNetCore.Request /// public class HasOneRelationshipPointers { - private Dictionary _hasOneRelationships = new Dictionary(); + private readonly Dictionary _hasOneRelationships = new Dictionary(); /// /// Add the relationship to the list of relationships that should be /// set in the repository layer. /// - public void Add(RelationshipAttribute relationship, IIdentifiable entity) + public void Add(HasOneAttribute relationship, IIdentifiable entity) => _hasOneRelationships[relationship] = entity; /// /// Get all the models that should be associated /// - public Dictionary Get() => _hasOneRelationships; + public Dictionary Get() => _hasOneRelationships; } } diff --git a/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs b/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs index abcae6c9a6..5bd13a4aef 100644 --- a/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs +++ b/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs @@ -155,7 +155,12 @@ private object SetEntityAttributes( continue; var convertedValue = ConvertAttrValue(newValue, attr.PropertyInfo.PropertyType); attr.SetValue(entity, convertedValue); - _jsonApiContext.AttributesToUpdate[attr] = convertedValue; + /// todo: as a part of the process of decoupling JADNC (specifically + /// through the decoupling IJsonApiContext), we now no longer need to + /// store the updated relationship values in this property. For now + /// just assigning null as value, will remove this property later as a whole. + /// see #512 + _jsonApiContext.AttributesToUpdate[attr] = null; } } @@ -220,14 +225,14 @@ private object SetHasOneRelationship(object entity, SetHasOneNavigationPropertyValue(entity, attr, rio, included); // recursive call ... - if(included != null) + if (included != null) { var navigationPropertyValue = attr.GetValue(entity); var resourceGraphEntity = _jsonApiContext.ResourceGraph.GetContextEntity(attr.Type); - if(navigationPropertyValue != null && resourceGraphEntity != null) + if (navigationPropertyValue != null && resourceGraphEntity != null) { var includedResource = included.SingleOrDefault(r => r.Type == rio.Type && r.Id == rio.Id); - if(includedResource != null) + if (includedResource != null) SetRelationships(navigationPropertyValue, resourceGraphEntity, includedResource.Relationships, included); } } @@ -249,8 +254,12 @@ private void SetHasOneForeignKeyValue(object entity, HasOneAttribute hasOneAttr, throw new JsonApiException(400, $"Cannot set required relationship identifier '{hasOneAttr.IdentifiablePropertyName}' to null because it is a non-nullable type."); var convertedValue = TypeHelper.ConvertType(foreignKeyPropertyValue, foreignKeyProperty.PropertyType); - foreignKeyProperty.SetValue(entity, convertedValue); - _jsonApiContext.RelationshipsToUpdate[hasOneAttr] = convertedValue; + /// todo: as a part of the process of decoupling JADNC (specifically + /// through the decoupling IJsonApiContext), we now no longer need to + /// store the updated relationship values in this property. For now + /// just assigning null as value, will remove this property later as a whole. + /// see #512 + if (convertedValue == null) _jsonApiContext.HasOneRelationshipPointers.Add(hasOneAttr, null); } } @@ -270,10 +279,12 @@ private void SetHasOneNavigationPropertyValue(object entity, HasOneAttribute has if (includedRelationshipObject != null) hasOneAttr.SetValue(entity, includedRelationshipObject); - // we need to store the fact that this relationship was included in the payload - // for EF, the repository will use these pointers to make ensure we don't try to - // create resources if they already exist, we just need to create the relationship - _jsonApiContext.HasOneRelationshipPointers.Add(hasOneAttr, includedRelationshipObject); + /// todo: as a part of the process of decoupling JADNC (specifically + /// through the decoupling IJsonApiContext), we now no longer need to + /// store the updated relationship values in this property. For now + /// just assigning null as value, will remove this property later as a whole. + /// see #512 + _jsonApiContext.HasOneRelationshipPointers.Add(hasOneAttr, null); } } @@ -288,7 +299,7 @@ private object SetHasManyRelationship(object entity, if (relationships.TryGetValue(relationshipName, out RelationshipData relationshipData)) { - if(relationshipData.IsHasMany == false || relationshipData.ManyData == null) + if (relationshipData.IsHasMany == false || relationshipData.ManyData == null) return entity; var relatedResources = relationshipData.ManyData.Select(r => @@ -300,8 +311,12 @@ 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); + /// todo: as a part of the process of decoupling JADNC (specifically + /// through the decoupling IJsonApiContext), we now no longer need to + /// store the updated relationship values in this property. For now + /// just assigning null as value, will remove this property later as a whole. + /// see #512 + _jsonApiContext.HasManyRelationshipPointers.Add(attr, null); } return entity; diff --git a/src/JsonApiDotNetCore/Services/EntityResourceService.cs b/src/JsonApiDotNetCore/Services/EntityResourceService.cs index b392b3600d..b73866c4ed 100644 --- a/src/JsonApiDotNetCore/Services/EntityResourceService.cs +++ b/src/JsonApiDotNetCore/Services/EntityResourceService.cs @@ -154,7 +154,7 @@ public virtual async Task UpdateAsync(TId id, TResource resource) { var entity = MapIn(resource); - entity = await _entities.UpdateAsync(id, entity); + entity = await _entities.UpdateAsync(entity); return MapOut(entity); } diff --git a/src/JsonApiDotNetCore/Services/IJsonApiContext.cs b/src/JsonApiDotNetCore/Services/IJsonApiContext.cs index 2509d7124f..e0a14a4e02 100644 --- a/src/JsonApiDotNetCore/Services/IJsonApiContext.cs +++ b/src/JsonApiDotNetCore/Services/IJsonApiContext.cs @@ -35,7 +35,7 @@ public interface IUpdateRequest /// Any relationships that were included in a PATCH request. /// Only the relationships in this dictionary should be updated. /// - Dictionary RelationshipsToUpdate { get; set; } + Dictionary RelationshipsToUpdate { get; } } public interface IJsonApiRequest : IJsonApiApplication, IUpdateRequest, IQueryRequest diff --git a/src/JsonApiDotNetCore/Services/JsonApiContext.cs b/src/JsonApiDotNetCore/Services/JsonApiContext.cs index 2553cbe451..bec78e8237 100644 --- a/src/JsonApiDotNetCore/Services/JsonApiContext.cs +++ b/src/JsonApiDotNetCore/Services/JsonApiContext.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using JsonApiDotNetCore.Builders; using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Internal; @@ -52,7 +53,15 @@ public JsonApiContext( public bool IsBulkOperationRequest { get; set; } public Dictionary AttributesToUpdate { get; set; } = new Dictionary(); - public Dictionary RelationshipsToUpdate { get; set; } = new Dictionary(); + public Dictionary RelationshipsToUpdate { get => GetRelationshipsToUpdate(); } + + private Dictionary GetRelationshipsToUpdate() + { + var hasOneEntries = HasOneRelationshipPointers.Get().ToDictionary(kvp => (RelationshipAttribute)kvp.Key, kvp => (object)kvp.Value); + var hasManyEntries = HasManyRelationshipPointers.Get().ToDictionary(kvp => kvp.Key, kvp => (object)kvp.Value); + return hasOneEntries.Union(hasManyEntries).ToDictionary(kvp => kvp.Key, kvp => kvp.Value); + } + public HasManyRelationshipPointers HasManyRelationshipPointers { get; private set; } = new HasManyRelationshipPointers(); public HasOneRelationshipPointers HasOneRelationshipPointers { get; private set; } = new HasOneRelationshipPointers(); @@ -138,7 +147,6 @@ public void BeginOperation() { IncludedRelationships = new List(); AttributesToUpdate = new Dictionary(); - RelationshipsToUpdate = new Dictionary(); HasManyRelationshipPointers = new HasManyRelationshipPointers(); HasOneRelationshipPointers = new HasOneRelationshipPointers(); } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/CamelCasedModelsControllerTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/CamelCasedModelsControllerTests.cs index 533c59839a..b2059c10c9 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/CamelCasedModelsControllerTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/CamelCasedModelsControllerTests.cs @@ -143,6 +143,7 @@ public async Task Can_Patch_CamelCasedModels() data = new { type = "camelCasedModels", + id = model.Id, attributes = new Dictionary() { { "compoundAttr", newModel.CompoundAttr } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Extensibility/RepositoryOverrideTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Extensibility/RepositoryOverrideTests.cs index b161b14c87..d63575e263 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Extensibility/RepositoryOverrideTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Extensibility/RepositoryOverrideTests.cs @@ -59,7 +59,7 @@ public async Task Total_Record_Count_Included() // assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); foreach(var item in deserializedBody) - Assert.Equal(person.Id, item.OwnerId); + Assert.Equal(person.Id, item.Owner.Id); } [Fact] diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs index a34312ca2f..93137b4ee2 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/CreatingDataTests.cs @@ -17,6 +17,7 @@ using Microsoft.EntityFrameworkCore; using Newtonsoft.Json; using Xunit; +using Person = JsonApiDotNetCoreExample.Models.Person; namespace JsonApiDotNetCoreExampleTests.Acceptance.Spec { @@ -26,6 +27,7 @@ public class CreatingDataTests private TestFixture _fixture; private IJsonApiContext _jsonApiContext; private Faker _todoItemFaker; + private Faker _personFaker; public CreatingDataTests(TestFixture fixture) { @@ -35,6 +37,10 @@ public CreatingDataTests(TestFixture 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()); + _personFaker = new Faker() + .RuleFor(t => t.FirstName, f => f.Name.FirstName()) + .RuleFor(t => t.LastName, f => f.Name.LastName()); + } [Fact] @@ -586,5 +592,113 @@ public async Task Respond_409_ToIncorrectEntityType() // assert Assert.Equal(HttpStatusCode.Conflict, response.StatusCode); } + + [Fact] + public async Task Create_With_ToOne_Relationship_With_Implicit_Remove() + { + // Arrange + var context = _fixture.GetService(); + var passport = new Passport(); + var person1 = _personFaker.Generate(); + person1.Passport = passport; + context.People.AddRange(new List() { person1 }); + await context.SaveChangesAsync(); + var passportId = person1.PassportId; + var content = new + { + data = new + { + type = "people", + attributes = new Dictionary() { { "first-name", "Joe" } }, + relationships = new Dictionary + { + { "passport", new + { + data = new { type = "passports", id = $"{passportId}" } + } + } + } + } + }; + + var httpMethod = new HttpMethod("POST"); + var route = $"/api/v1/people"; + 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); + var body = await response.Content.ReadAsStringAsync(); + var personResult = _fixture.GetService().Deserialize(body); + + // Assert + + Assert.True(HttpStatusCode.Created == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}"); + var dbPerson = context.People.AsNoTracking().Where(p => p.Id == personResult.Id).Include("Passport").FirstOrDefault(); + Assert.Equal(passportId, dbPerson.Passport.Id); + } + + [Fact] + public async Task Create_With_ToMany_Relationship_With_Implicit_Remove() + { + // Arrange + var context = _fixture.GetService(); + var person1 = _personFaker.Generate(); + person1.TodoItems = _todoItemFaker.Generate(3).ToList(); + context.People.AddRange(new List() { person1 }); + await context.SaveChangesAsync(); + var todoItem1Id = person1.TodoItems[0].Id; + var todoItem2Id = person1.TodoItems[1].Id; + + var content = new + { + data = new + { + type = "people", + attributes = new Dictionary() { { "first-name", "Joe" } }, + relationships = new Dictionary + { + { "todo-items", new + { + data = new List + { + new { + type = "todo-items", + id = $"{todoItem1Id}" + }, + new { + type = "todo-items", + id = $"{todoItem2Id}" + } + } + } + } + } + } + }; + + var httpMethod = new HttpMethod("POST"); + var route = $"/api/v1/people"; + 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); + var body = await response.Content.ReadAsStringAsync(); + var personResult = _fixture.GetService().Deserialize(body); + + // Assert + Assert.True(HttpStatusCode.Created == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}"); + var dbPerson = context.People.AsNoTracking().Where(p => p.Id == personResult.Id).Include("TodoItems").FirstOrDefault(); + Assert.Equal(2, dbPerson.TodoItems.Count); + Assert.NotNull(dbPerson.TodoItems.SingleOrDefault(ti => ti.Id == todoItem1Id)); + Assert.NotNull(dbPerson.TodoItems.SingleOrDefault(ti => ti.Id == todoItem2Id)); + } } } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingDataTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingDataTests.cs index 5b58a05fe0..b1145f3654 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingDataTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingDataTests.cs @@ -55,8 +55,9 @@ public async Task Response400IfUpdatingNotSettableAttribute() var content = new { - datea = new + date = new { + id = todoItem.Id, type = "todo-items", attributes = new { @@ -90,6 +91,7 @@ public async Task Respond_404_If_EntityDoesNotExist() { data = new { + id = maxPersonId + 100, type = "todo-items", attributes = new { @@ -108,6 +110,41 @@ public async Task Respond_404_If_EntityDoesNotExist() Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); } + [Fact] + public async Task Respond_400_If_IdNotInAttributeList() + { + // Arrange + var maxPersonId = _context.TodoItems.LastOrDefault()?.Id ?? 0; + var todoItem = _todoItemFaker.Generate(); + var builder = new WebHostBuilder() + .UseStartup(); + + var server = new TestServer(builder); + var client = server.CreateClient(); + + var content = new + { + data = new + { + type = "todo-items", + attributes = new + { + description = todoItem.Description, + ordinal = todoItem.Ordinal, + createdDate = DateTime.Now + } + } + }; + var request = PrepareRequest("PATCH", $"/api/v1/todo-items/{maxPersonId}", content); + + // Act + var response = await client.SendAsync(request); + + // Assert + Assert.Equal(422, Convert.ToInt32(response.StatusCode)); + + } + [Fact] public async Task Can_Patch_Entity() @@ -129,6 +166,7 @@ public async Task Can_Patch_Entity() { data = new { + id = todoItem.Id, type = "todo-items", attributes = new { @@ -187,6 +225,8 @@ public async Task Patch_Entity_With_HasMany_Does_Not_Included_Relationships() data = new { type = "people", + id = person.Id, + attributes = new Dictionary { { "last-name", newPerson.LastName }, @@ -234,6 +274,7 @@ public async Task Can_Patch_Entity_And_HasOne_Relationships() data = new { type = "todo-items", + id = todoItem.Id, attributes = new { description = todoItem.Description, diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs index f126dbee9d..b1197447fd 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/UpdatingRelationshipsTests.cs @@ -549,6 +549,7 @@ public async Task Can_Delete_Relationship_By_Patching_Resource() { data = new { + id = todoItem.Id, type = "todo-items", relationships = new { @@ -621,5 +622,115 @@ 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 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); + var body = await response.Content.ReadAsStringAsync(); + + // Assert + + Assert.True(HttpStatusCode.OK == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}"); + var dbPerson = context.People.AsNoTracking().Where(p => p.Id == person2.Id).Include("Passport").FirstOrDefault(); + Assert.Equal(passportId, dbPerson.Passport.Id); + } + + [Fact] + public async Task Updating_ToMany_Relationship_With_Implicit_Remove() + { + // Arrange + var context = _fixture.GetService(); + var person1 = _personFaker.Generate(); + person1.TodoItems = _todoItemFaker.Generate(3).ToList(); + var person2 = _personFaker.Generate(); + person2.TodoItems = _todoItemFaker.Generate(2).ToList(); + context.People.AddRange(new List() { person1, person2 }); + await context.SaveChangesAsync(); + var todoItem1Id = person1.TodoItems[0].Id; + var todoItem2Id = person1.TodoItems[1].Id; + + var content = new + { + data = new + { + type = "people", + id = person2.Id, + relationships = new Dictionary + { + { "todo-items", new + { + data = new List + { + new { + type = "todo-items", + id = $"{todoItem1Id}" + }, + new { + type = "todo-items", + id = $"{todoItem2Id}" + } + } + } + } + } + } + }; + + 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); + var body = await response.Content.ReadAsStringAsync(); + + // Assert + + Assert.True(HttpStatusCode.OK == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}"); + var dbPerson = context.People.AsNoTracking().Where(p => p.Id == person2.Id).Include("TodoItems").FirstOrDefault(); + Assert.Equal(2, dbPerson.TodoItems.Count); + Assert.NotNull(dbPerson.TodoItems.SingleOrDefault(ti => ti.Id == todoItem1Id)); + Assert.NotNull(dbPerson.TodoItems.SingleOrDefault(ti => ti.Id == todoItem2Id)); + } } } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs index d28a6bd918..7ce1604683 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs @@ -114,7 +114,7 @@ public async Task Can_Filter_By_Relationship_Id() // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotEmpty(deserializedBody); - Assert.Contains(deserializedBody, (i) => i.OwnerId == person.Id); + Assert.Contains(deserializedBody, (i) => i.Owner.Id == person.Id); } [Fact] @@ -435,7 +435,8 @@ public async Task Can_Get_TodoItem_WithOwner() // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); var deserializedBody = (TodoItem)_fixture.GetService().Deserialize(body); - Assert.Equal(person.Id, deserializedBody.OwnerId); + + Assert.Equal(person.Id, deserializedBody.Owner.Id); Assert.Equal(todoItem.Id, deserializedBody.Id); Assert.Equal(todoItem.Description, deserializedBody.Description); Assert.Equal(todoItem.Ordinal, deserializedBody.Ordinal); @@ -587,6 +588,7 @@ public async Task Can_Patch_TodoItem() { data = new { + id = todoItem.Id, type = "todo-items", attributes = new Dictionary() { @@ -638,6 +640,7 @@ public async Task Can_Patch_TodoItemWithNullable() { data = new { + id = todoItem.Id, type = "todo-items", attributes = new Dictionary() { @@ -689,6 +692,7 @@ public async Task Can_Patch_TodoItemWithNullValue() { data = new { + id = todoItem.Id, type = "todo-items", attributes = new Dictionary() { diff --git a/test/ResourceEntitySeparationExampleTests/Acceptance/AddTests.cs b/test/ResourceEntitySeparationExampleTests/Acceptance/AddTests.cs index f50ad734b7..53105ee4b6 100644 --- a/test/ResourceEntitySeparationExampleTests/Acceptance/AddTests.cs +++ b/test/ResourceEntitySeparationExampleTests/Acceptance/AddTests.cs @@ -94,7 +94,7 @@ public async Task Can_Create_Course_With_Department_Id() Assert.Equal(course.Number, data.Number); Assert.Equal(course.Title, data.Title); Assert.Equal(course.Description, data.Description); - Assert.Equal(department.Id, data.DepartmentId); + Assert.Equal(department.Id, data.Department.Id); } [Fact] diff --git a/test/ResourceEntitySeparationExampleTests/Acceptance/UpdateTests.cs b/test/ResourceEntitySeparationExampleTests/Acceptance/UpdateTests.cs index c3a9cfdb78..eeb838c475 100644 --- a/test/ResourceEntitySeparationExampleTests/Acceptance/UpdateTests.cs +++ b/test/ResourceEntitySeparationExampleTests/Acceptance/UpdateTests.cs @@ -31,6 +31,7 @@ public async Task Can_Update_Course() { data = new { + id = course.Id, type = "courses", attributes = new Dictionary() { @@ -64,6 +65,7 @@ public async Task Can_Update_Department() { data = new { + id = dept.Id, type = "departments", attributes = new Dictionary() { @@ -97,6 +99,7 @@ public async Task Can_Update_Student() { data = new { + id = student.Id, type = "students", attributes = new Dictionary() { diff --git a/test/UnitTests/Data/DefaultEntityRepository_Tests.cs b/test/UnitTests/Data/DefaultEntityRepository_Tests.cs index f5a77789e2..958639575e 100644 --- a/test/UnitTests/Data/DefaultEntityRepository_Tests.cs +++ b/test/UnitTests/Data/DefaultEntityRepository_Tests.cs @@ -60,14 +60,14 @@ public async Task UpdateAsync_Updates_Attributes_In_AttributesToUpdate() { { descAttr, - todoItemUpdates.Description + null //todoItemUpdates.Description } }; var repository = GetRepository(); // act - var updatedItem = await repository.UpdateAsync(_todoItem.Id, todoItemUpdates); + var updatedItem = await repository.UpdateAsync(todoItemUpdates); // assert Assert.NotNull(updatedItem);