From 3814d5ea805a116ed3d9ada631f92519a7f48784 Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Tue, 25 Sep 2018 21:46:34 -0700 Subject: [PATCH 01/18] port @jasolko 's changes --- .gitignore | 2 + .../Data/AppDbContext.cs | 3 +- .../Models/Article.cs | 7 +++ .../Models/ArticleTag.cs | 13 ++++ .../JsonApiDotNetCoreExample/Models/Tag.cs | 9 +++ .../Builders/ContextGraphBuilder.cs | 18 ++++++ .../Extensions/DbContextExtensions.cs | 10 ++++ .../Internal/Generics/GenericProcessor.cs | 34 ++++++++++- .../Models/HasManyThroughAttribute.cs | 60 +++++++++++++++++++ 9 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 src/Examples/JsonApiDotNetCoreExample/Models/ArticleTag.cs create mode 100644 src/Examples/JsonApiDotNetCoreExample/Models/Tag.cs create mode 100644 src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs diff --git a/.gitignore b/.gitignore index 0ca27f04e1..b6767ff903 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ +.DS_Store + ## Ignore Visual Studio temporary files, build results, and ## files generated by popular Visual Studio add-ons. diff --git a/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs b/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs index cee66678ab..922c23f9b1 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs @@ -53,7 +53,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) public DbSet Departments { get; set; } public DbSet Registrations { get; set; } public DbSet Students { get; set; } - public DbSet PersonRoles { get; set; } + public DbSet ArticleTags { get; set; } + public DbSet Tags { get; set; } } } diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/Article.cs b/src/Examples/JsonApiDotNetCoreExample/Models/Article.cs index c633d58bdd..9bbfc15fe1 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Models/Article.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Models/Article.cs @@ -1,3 +1,5 @@ +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations.Schema; using JsonApiDotNetCore.Models; namespace JsonApiDotNetCoreExample.Models @@ -10,5 +12,10 @@ public class Article : Identifiable [HasOne("author")] public Author Author { get; set; } public int AuthorId { get; set; } + + [NotMapped] + [HasManyThrough("tags", nameof(ArticleTags))] + public List Tags { get; set; } + public List ArticleTags { get; set; } } } diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/ArticleTag.cs b/src/Examples/JsonApiDotNetCoreExample/Models/ArticleTag.cs new file mode 100644 index 0000000000..2e45ea84ae --- /dev/null +++ b/src/Examples/JsonApiDotNetCoreExample/Models/ArticleTag.cs @@ -0,0 +1,13 @@ +using JsonApiDotNetCore.Models; + +namespace JsonApiDotNetCoreExample.Models +{ + public class ArticleTag : Identifiable + { + public int ArticleId { get; set; } + public Article Article { get; set; } + + public int TagId { get; set; } + public Tag Tag { get; set; } + } +} \ No newline at end of file diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/Tag.cs b/src/Examples/JsonApiDotNetCoreExample/Models/Tag.cs new file mode 100644 index 0000000000..ebd3bd5a1c --- /dev/null +++ b/src/Examples/JsonApiDotNetCoreExample/Models/Tag.cs @@ -0,0 +1,9 @@ +using JsonApiDotNetCore.Models; + +namespace JsonApiDotNetCoreExample.Models +{ + public class Tag : Identifiable + { + public string Name { get; set; } + } +} \ No newline at end of file diff --git a/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs b/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs index b343243463..c1a449b891 100644 --- a/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs +++ b/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs @@ -166,7 +166,25 @@ protected virtual List GetRelationships(Type entityType) attribute.InternalRelationshipName = prop.Name; attribute.Type = GetRelationshipType(attribute, prop); attributes.Add(attribute); + + if(attribute is HasManyThroughAttribute hasManyThroughAttribute) { + var throughProperty = properties.SingleOrDefault(p => p.Name == hasManyThroughAttribute.InternalThroughName); + if(throughProperty == null) + throw new JsonApiSetupException($"Invalid '{nameof(HasManyThroughAttribute)}' on type '{entityType}'. Type does not contain a property named '{hasManyThroughAttribute.InternalThroughName}'."); + + // assumption: the property should be a generic collection, e.g. List + if(prop.PropertyType.IsGenericType == false) + throw new JsonApiSetupException($"Invalid '{nameof(HasManyThroughAttribute)}' on type '{entityType}'. Expected through entity to be a generic type, such as List<{prop.PropertyType}>."); + hasManyThroughAttribute.ThroughType = prop.PropertyType.GetGenericArguments()[0]; + + var throughProperties = hasManyThroughAttribute.ThroughType.GetProperties(); + // Article → ArticleTag.Article + hasManyThroughAttribute.LeftProperty = throughProperties.Single(x => x.PropertyType == entityType); + // Article → ArticleTag.Tag + hasManyThroughAttribute.RightProperty = throughProperties.Single(x => x.PropertyType == hasManyThroughAttribute.Type); + } } + return attributes; } diff --git a/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs b/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs index 9176474548..6df8db4c39 100644 --- a/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs @@ -11,6 +11,16 @@ public static class DbContextExtensions public static DbSet GetDbSet(this DbContext context) where T : class => context.Set(); + /// + /// Get the DbSet when the model type is unknown until runtime + /// + public static IQueryable Set(this DbContext context, Type t) + => (IQueryable)context + .GetType() + .GetMethod("Set") + .MakeGenericMethod(t) // TODO: will caching help runtime performance? + .Invoke(context, null); + /// /// Determines whether or not EF is already tracking an entity of the same Type and Id /// diff --git a/src/JsonApiDotNetCore/Internal/Generics/GenericProcessor.cs b/src/JsonApiDotNetCore/Internal/Generics/GenericProcessor.cs index 0a56fdfd4a..c9576367c5 100644 --- a/src/JsonApiDotNetCore/Internal/Generics/GenericProcessor.cs +++ b/src/JsonApiDotNetCore/Internal/Generics/GenericProcessor.cs @@ -1,7 +1,10 @@ +using System; using System.Collections.Generic; using System.Linq; +using System.Linq.Expressions; using System.Threading.Tasks; using JsonApiDotNetCore.Data; +using JsonApiDotNetCore.Extensions; using JsonApiDotNetCore.Models; using Microsoft.EntityFrameworkCore; @@ -21,16 +24,41 @@ public GenericProcessor(IDbContextResolver contextResolver) _context = contextResolver.GetContext(); } - public async Task UpdateRelationshipsAsync(object parent, RelationshipAttribute relationship, IEnumerable relationshipIds) + public virtual async Task UpdateRelationshipsAsync(object parent, RelationshipAttribute relationship, IEnumerable relationshipIds) { SetRelationships(parent, relationship, relationshipIds); await _context.SaveChangesAsync(); } - public void SetRelationships(object parent, RelationshipAttribute relationship, IEnumerable relationshipIds) + public virtual void SetRelationships(object parent, RelationshipAttribute relationship, IEnumerable relationshipIds) { - if (relationship.IsHasMany) + if (relationship is HasManyThroughAttribute hasManyThrough) + { + var parentId = ((IIdentifiable)parent).StringId; + ParameterExpression parameter = Expression.Parameter(hasManyThrough.Type); + Expression property = Expression.Property(parameter, hasManyThrough.LeftProperty); + Expression target = Expression.Constant(parentId); + Expression toString = Expression.Call(property, "ToString", null, null); + Expression equals = Expression.Call(toString, "Equals", null, target); + Expression> lambda = Expression.Lambda>(equals, parameter); + + var oldLinks = _context + .Set(hasManyThrough.ThroughType) + .Where(lambda.Compile()) + .ToList(); + + _context.Remove(oldLinks); + + var newLinks = relationshipIds.Select(x => { + var link = Activator.CreateInstance(hasManyThrough.ThroughType); + hasManyThrough.LeftProperty.SetValue(link, TypeHelper.ConvertType(parent, hasManyThrough.LeftProperty.PropertyType)); + hasManyThrough.RightProperty.SetValue(link, TypeHelper.ConvertType(x, hasManyThrough.RightProperty.PropertyType)); + return link; + }); + _context.AddRange(newLinks); + } + else if (relationship.IsHasMany) { var entities = _context.Set().Where(x => relationshipIds.Contains(x.StringId)).ToList(); relationship.SetValue(parent, entities); diff --git a/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs b/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs new file mode 100644 index 0000000000..1a633da3f0 --- /dev/null +++ b/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs @@ -0,0 +1,60 @@ +using System; +using System.Reflection; + +namespace JsonApiDotNetCore.Models +{ + /// + /// + /// + /// + /// + /// + /// + public class HasManyThroughAttribute : HasManyAttribute + { + /// + /// + /// + /// + /// + /// + public HasManyThroughAttribute(string publicName, string internalThroughName, Link documentLinks = Link.All, bool canInclude = true) + : base(publicName, documentLinks, canInclude) + { + InternalThroughName = internalThroughName; + } + + public string InternalThroughName { get; private set; } + public Type ThroughType { get; internal set; } + + /// + /// The navigation property back to the parent resource. + /// + /// + /// + /// In the `[HasManyThrough("tags", nameof(ArticleTags))]` example + /// this would point to the `Article.ArticleTags.Article` property + /// + /// + /// public Article Article { get; set; } + /// + /// + /// + public PropertyInfo LeftProperty { get; internal set; } + + /// + /// The navigation property to the related resource. + /// + /// + /// + /// In the `[HasManyThrough("tags", nameof(ArticleTags))]` example + /// this would point to the `Article.ArticleTags.Tag` property + /// + /// + /// public Tag Tag { get; set; } + /// + /// + /// + public PropertyInfo RightProperty { get; internal set; } + } +} \ No newline at end of file From 8d887faf14abca55bbf5693ce0ff7bcc49e1df81 Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Tue, 25 Sep 2018 22:04:47 -0700 Subject: [PATCH 02/18] add convention based ctor for HasManyThroughAttribute --- .../Models/HasManyThroughAttribute.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs b/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs index 1a633da3f0..80cf16571b 100644 --- a/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs +++ b/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs @@ -15,6 +15,30 @@ public class HasManyThroughAttribute : HasManyAttribute /// /// /// + /// + /// The relationship name as exposed by the API + /// The name of the navigation property that will be used to get the HasMany relationship + /// Which links are available. Defaults to + /// Whether or not this relationship can be included using the ?include=public-name query string + /// + /// + /// + /// + public HasManyThroughAttribute(string internalThroughName, Link documentLinks = Link.All, bool canInclude = true) + : base(null, documentLinks, canInclude) + { + InternalThroughName = internalThroughName; + } + + /// + /// + /// + /// + /// The relationship name as exposed by the API + /// The name of the navigation property that will be used to get the HasMany relationship + /// Which links are available. Defaults to + /// Whether or not this relationship can be included using the ?include=public-name query string + /// /// /// /// From 6fc162fb0c2d3f8f598e094bfb5bc577e759d419 Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Wed, 26 Sep 2018 22:54:24 -0700 Subject: [PATCH 03/18] add test for querying HasManyThrough --- .../Controllers/ArticlesController.cs | 15 +++++ .../Data/AppDbContext.cs | 6 +- .../Models/ArticleTag.cs | 4 +- .../Builders/ContextGraphBuilder.cs | 13 +++-- .../Builders/DocumentBuilder.cs | 17 +++--- .../Data/DefaultEntityRepository.cs | 4 +- .../Internal/ContextGraph.cs | 40 ++++++++++++- .../Models/HasManyThroughAttribute.cs | 6 ++ .../Models/RelationshipAttribute.cs | 10 +++- .../Acceptance/ManyToManyTests.cs | 56 +++++++++++++++++++ 10 files changed, 149 insertions(+), 22 deletions(-) create mode 100644 src/Examples/JsonApiDotNetCoreExample/Controllers/ArticlesController.cs create mode 100644 test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs diff --git a/src/Examples/JsonApiDotNetCoreExample/Controllers/ArticlesController.cs b/src/Examples/JsonApiDotNetCoreExample/Controllers/ArticlesController.cs new file mode 100644 index 0000000000..95aa7d69f9 --- /dev/null +++ b/src/Examples/JsonApiDotNetCoreExample/Controllers/ArticlesController.cs @@ -0,0 +1,15 @@ +using JsonApiDotNetCore.Controllers; +using JsonApiDotNetCore.Services; +using JsonApiDotNetCoreExample.Models; + +namespace JsonApiDotNetCoreExample.Controllers +{ + public class ArticlesController : JsonApiController
+ { + public ArticlesController( + IJsonApiContext jsonApiContext, + IResourceService
resourceService) + : base(jsonApiContext, resourceService) + { } + } +} \ No newline at end of file diff --git a/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs b/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs index 922c23f9b1..5486eedb6f 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs @@ -31,13 +31,15 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) modelBuilder.Entity() .HasOne(r => r.Course) .WithMany(c => c.Students) - .HasForeignKey(r => r.CourseId) - ; + .HasForeignKey(r => r.CourseId); modelBuilder.Entity() .HasOne(r => r.Student) .WithMany(s => s.Courses) .HasForeignKey(r => r.StudentId); + + modelBuilder.Entity() + .HasKey(bc => new { bc.ArticleId, bc.TagId }); } public DbSet TodoItems { get; set; } diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/ArticleTag.cs b/src/Examples/JsonApiDotNetCoreExample/Models/ArticleTag.cs index 2e45ea84ae..992e688c51 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Models/ArticleTag.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Models/ArticleTag.cs @@ -1,8 +1,6 @@ -using JsonApiDotNetCore.Models; - namespace JsonApiDotNetCoreExample.Models { - public class ArticleTag : Identifiable + public class ArticleTag { public int ArticleId { get; set; } public Article Article { get; set; } diff --git a/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs b/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs index c1a449b891..41378de939 100644 --- a/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs +++ b/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs @@ -173,15 +173,20 @@ protected virtual List GetRelationships(Type entityType) throw new JsonApiSetupException($"Invalid '{nameof(HasManyThroughAttribute)}' on type '{entityType}'. Type does not contain a property named '{hasManyThroughAttribute.InternalThroughName}'."); // assumption: the property should be a generic collection, e.g. List - if(prop.PropertyType.IsGenericType == false) + if(throughProperty.PropertyType.IsGenericType == false) throw new JsonApiSetupException($"Invalid '{nameof(HasManyThroughAttribute)}' on type '{entityType}'. Expected through entity to be a generic type, such as List<{prop.PropertyType}>."); - hasManyThroughAttribute.ThroughType = prop.PropertyType.GetGenericArguments()[0]; + + hasManyThroughAttribute.ThroughType = throughProperty.PropertyType.GetGenericArguments()[0]; var throughProperties = hasManyThroughAttribute.ThroughType.GetProperties(); + // Article → ArticleTag.Article - hasManyThroughAttribute.LeftProperty = throughProperties.Single(x => x.PropertyType == entityType); + hasManyThroughAttribute.LeftProperty = throughProperties.SingleOrDefault(x => x.PropertyType == entityType) + ?? throw new JsonApiSetupException($"{hasManyThroughAttribute.ThroughType} does not contain a navigation property to type {entityType}"); + // Article → ArticleTag.Tag - hasManyThroughAttribute.RightProperty = throughProperties.Single(x => x.PropertyType == hasManyThroughAttribute.Type); + hasManyThroughAttribute.RightProperty = throughProperties.SingleOrDefault(x => x.PropertyType == hasManyThroughAttribute.Type) + ?? throw new JsonApiSetupException($"{hasManyThroughAttribute.ThroughType} does not contain a navigation property to type {hasManyThroughAttribute.Type}"); } } diff --git a/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs b/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs index 6afa13e029..e56b279d49 100644 --- a/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs +++ b/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs @@ -177,8 +177,9 @@ private RelationshipData GetRelationshipData(RelationshipAttribute attr, Context relationshipData.Links.Related = linkBuilder.GetRelatedRelationLink(contextEntity.EntityName, entity.StringId, attr.PublicRelationshipName); } - // this only includes the navigation property, we need to actually check the navigation property Id - var navigationEntity = _jsonApiContext.ContextGraph.GetRelationship(entity, attr.InternalRelationshipName); + // this only includes the navigation property, we need to actually check the navigation property Id + var navigationEntity = _jsonApiContext.ContextGraph.GetRelationshipValue(entity, attr); + if (navigationEntity == null) relationshipData.SingleData = attr.IsHasOne ? GetIndependentRelationshipIdentifier((HasOneAttribute)attr, entity) @@ -213,7 +214,7 @@ private List IncludeRelationshipChain( { var requestedRelationship = relationshipChain[relationshipChainIndex]; var relationship = parentEntity.Relationships.FirstOrDefault(r => r.PublicRelationshipName == requestedRelationship); - var navigationEntity = _jsonApiContext.ContextGraph.GetRelationship(parentResource, relationship.InternalRelationshipName); + var navigationEntity = _jsonApiContext.ContextGraph.GetRelationship(parentResource, relationship.InternalRelationshipName); if (navigationEntity is IEnumerable hasManyNavigationEntity) { foreach (IIdentifiable includedEntity in hasManyNavigationEntity) @@ -226,7 +227,7 @@ private List IncludeRelationshipChain( { included = AddIncludedEntity(included, (IIdentifiable)navigationEntity); included = IncludeSingleResourceRelationships(included, (IIdentifiable)navigationEntity, relationship, relationshipChain, relationshipChainIndex); - } + } return included; } @@ -284,16 +285,14 @@ private ResourceObject GetIncludedEntity(IIdentifiable entity) private List GetRelationships(IEnumerable entities) { - var objType = entities.GetElementType(); - - var typeName = _jsonApiContext.ContextGraph.GetContextEntity(objType); - + string typeName = null; var relationships = new List(); foreach (var entity in entities) { + typeName = _jsonApiContext.ContextGraph.GetContextEntity(entity.GetType()).EntityName; relationships.Add(new ResourceIdentifierObject { - Type = typeName.EntityName, + Type = typeName, Id = ((IIdentifiable)entity).StringId }); } diff --git a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs index 808c1929a4..b7ae1ef8d4 100644 --- a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs +++ b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs @@ -275,8 +275,8 @@ public virtual IQueryable Include(IQueryable entities, string } internalRelationshipPath = (internalRelationshipPath == null) - ? relationship.InternalRelationshipName - : $"{internalRelationshipPath}.{relationship.InternalRelationshipName}"; + ? relationship.RelationshipPath + : $"{internalRelationshipPath}.{relationship.RelationshipPath}"; if(i < relationshipChain.Length) entity = _jsonApiContext.ContextGraph.GetContextEntity(relationship.Type); diff --git a/src/JsonApiDotNetCore/Internal/ContextGraph.cs b/src/JsonApiDotNetCore/Internal/ContextGraph.cs index 4b6a310527..d0733db5b1 100644 --- a/src/JsonApiDotNetCore/Internal/ContextGraph.cs +++ b/src/JsonApiDotNetCore/Internal/ContextGraph.cs @@ -1,6 +1,8 @@ using System; +using System.Collections; using System.Collections.Generic; using System.Linq; +using JsonApiDotNetCore.Models; namespace JsonApiDotNetCore.Internal { @@ -19,6 +21,19 @@ public interface IContextGraph /// object GetRelationship(TParent resource, string propertyName); + /// + /// Gets the value of the navigation property, defined by the relationshipName, + /// on the provided instance. + /// + /// The resource instance + /// The attribute used to define the relationship. + /// + /// + /// _graph.GetRelationshipValue(todoItem, nameof(TodoItem.Owner)); + /// + /// + object GetRelationshipValue(TParent resource, RelationshipAttribute relationship) where TParent : IIdentifiable; + /// /// Get the internal navigation property name for the specified public /// relationship name. @@ -107,6 +122,29 @@ public object GetRelationship(TParent entity, string relationshipName) return navigationProperty.GetValue(entity); } + public object GetRelationshipValue(TParent resource, RelationshipAttribute relationship) where TParent : IIdentifiable + { + if(relationship is HasManyThroughAttribute hasManyThroughRelationship) + { + return GetHasManyThrough(resource, hasManyThroughRelationship); + } + + return GetRelationship(resource, relationship.InternalRelationshipName); + } + + private IEnumerable GetHasManyThrough(IIdentifiable parent, HasManyThroughAttribute hasManyThrough) + { + var throughProperty = GetRelationship(parent, hasManyThrough.InternalThroughName); + if (throughProperty is IEnumerable hasManyNavigationEntity) + { + foreach (var includedEntity in hasManyNavigationEntity) + { + var targetValue = hasManyThrough.RightProperty.GetValue(includedEntity) as IIdentifiable; + yield return targetValue; + } + } + } + /// public string GetRelationshipName(string relationshipName) { @@ -125,5 +163,5 @@ public string GetPublicAttributeName(string internalAttributeName) .SingleOrDefault(a => a.InternalAttributeName == internalAttributeName)? .PublicAttributeName; } - } + } } diff --git a/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs b/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs index 80cf16571b..7d03ec55b1 100644 --- a/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs +++ b/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs @@ -80,5 +80,11 @@ public HasManyThroughAttribute(string publicName, string internalThroughName, Li /// /// public PropertyInfo RightProperty { get; internal set; } + + /// + /// + /// "ArticleTags.Tag" + /// + public override string RelationshipPath => $"{InternalThroughName}.{RightProperty.Name}"; } } \ No newline at end of file diff --git a/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs b/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs index b479d3bb12..b1cbb80e6e 100644 --- a/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs +++ b/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs @@ -25,7 +25,7 @@ protected RelationshipAttribute(string publicName, Link documentLinks, bool canI /// /// public Type Type { get; internal set; } - public bool IsHasMany => GetType() == typeof(HasManyAttribute); + public bool IsHasMany => GetType() == typeof(HasManyAttribute) || typeof(HasManyAttribute).IsAssignableFrom(GetType()); public bool IsHasOne => GetType() == typeof(HasOneAttribute); public Link DocumentLinks { get; } = Link.All; public bool CanInclude { get; } @@ -78,5 +78,13 @@ public override bool Equals(object obj) /// public virtual bool Is(string publicRelationshipName) => string.Equals(publicRelationshipName, PublicRelationshipName, StringComparison.OrdinalIgnoreCase); + + /// + /// The internal navigation property path to the related entity. + /// + /// + /// In all cases except the HasManyThrough relationships, this will just be the . + /// + public virtual string RelationshipPath => InternalRelationshipName; } } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs new file mode 100644 index 0000000000..887c4c23a4 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs @@ -0,0 +1,56 @@ +using System.Net; +using System.Threading.Tasks; +using Bogus; +using JsonApiDotNetCore.Serialization; +using JsonApiDotNetCoreExample.Data; +using JsonApiDotNetCoreExample.Models; +using Xunit; + +namespace JsonApiDotNetCoreExampleTests.Acceptance +{ + [Collection("WebHostCollection")] + public class ManyToManyTests + { + private static readonly Faker
_articleFaker = new Faker
() + .RuleFor(a => a.Name, f => f.Random.AlphaNumeric(10)) + .RuleFor(a => a.Author, f => new Author()); + private static readonly Faker _tagFaker = new Faker().RuleFor(a => a.Name, f => f.Random.AlphaNumeric(10)); + + private TestFixture _fixture; + public ManyToManyTests(TestFixture fixture) + { + _fixture = fixture; + } + + [Fact] + public async Task Can_Fetch_Many_To_Many_Through() + { + // arrange + var context = _fixture.GetService(); + var article = _articleFaker.Generate(); + var tag = _tagFaker.Generate(); + var articleTag = new ArticleTag { + Article = article, + Tag = tag + }; + context.ArticleTags.Add(articleTag); + await context.SaveChangesAsync(); + + var route = $"/api/v1/articles/{article.Id}?include=tags"; + + // act + var response = await _fixture.Client.GetAsync(route); + + // assert + var body = await response.Content.ReadAsStringAsync(); + Assert.True(HttpStatusCode.OK == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}"); + + var articleResponse = _fixture.GetService().Deserialize
(body); + Assert.NotNull(articleResponse); + Assert.Equal(article.Id, articleResponse.Id); + + var tagResponse = Assert.Single(articleResponse.Tags); + Assert.Equal(tag.Id, tagResponse.Id); + } + } +} \ No newline at end of file From 8a69eff26d79bb82ba0586215b2a68cdd1f3f239 Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Wed, 26 Sep 2018 23:06:53 -0700 Subject: [PATCH 04/18] refactor(DocumentBuilder): dont lookup resource on graph multiple times --- .../Builders/DocumentBuilder.cs | 32 +++++++++++-------- .../Internal/ContextGraph.cs | 4 +++ 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs b/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs index e56b279d49..730dd773c4 100644 --- a/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs +++ b/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs @@ -18,8 +18,8 @@ public class DocumentBuilder : IDocumentBuilder private readonly IScopedServiceProvider _scopedServiceProvider; public DocumentBuilder( - IJsonApiContext jsonApiContext, - IRequestMeta requestMeta = null, + IJsonApiContext jsonApiContext, + IRequestMeta requestMeta = null, IDocumentBuilderOptionsProvider documentBuilderOptionsProvider = null, IScopedServiceProvider scopedServiceProvider = null) { @@ -112,7 +112,8 @@ public ResourceObject GetData(ContextEntity contextEntity, IIdentifiable entity) public ResourceObject GetData(ContextEntity contextEntity, IIdentifiable entity, IResourceDefinition resourceDefinition = null) { - var data = new ResourceObject { + var data = new ResourceObject + { Type = contextEntity.EntityName, Id = entity.StringId }; @@ -177,9 +178,8 @@ private RelationshipData GetRelationshipData(RelationshipAttribute attr, Context relationshipData.Links.Related = linkBuilder.GetRelatedRelationLink(contextEntity.EntityName, entity.StringId, attr.PublicRelationshipName); } - // this only includes the navigation property, we need to actually check the navigation property Id + // this only includes the navigation property, we need to actually check the navigation property Id var navigationEntity = _jsonApiContext.ContextGraph.GetRelationshipValue(entity, attr); - if (navigationEntity == null) relationshipData.SingleData = attr.IsHasOne ? GetIndependentRelationshipIdentifier((HasOneAttribute)attr, entity) @@ -196,14 +196,14 @@ private List GetIncludedEntities(List included, { if (_jsonApiContext.IncludedRelationships != null) { - foreach(var relationshipName in _jsonApiContext.IncludedRelationships) + foreach (var relationshipName in _jsonApiContext.IncludedRelationships) { var relationshipChain = relationshipName.Split('.'); var contextEntity = rootContextEntity; var entity = rootResource; included = IncludeRelationshipChain(included, rootContextEntity, rootResource, relationshipChain, 0); - } + } } return included; @@ -214,7 +214,7 @@ private List IncludeRelationshipChain( { var requestedRelationship = relationshipChain[relationshipChainIndex]; var relationship = parentEntity.Relationships.FirstOrDefault(r => r.PublicRelationshipName == requestedRelationship); - var navigationEntity = _jsonApiContext.ContextGraph.GetRelationship(parentResource, relationship.InternalRelationshipName); + var navigationEntity = _jsonApiContext.ContextGraph.GetRelationship(parentResource, relationship.InternalRelationshipName); if (navigationEntity is IEnumerable hasManyNavigationEntity) { foreach (IIdentifiable includedEntity in hasManyNavigationEntity) @@ -227,7 +227,7 @@ private List IncludeRelationshipChain( { included = AddIncludedEntity(included, (IIdentifiable)navigationEntity); included = IncludeSingleResourceRelationships(included, (IIdentifiable)navigationEntity, relationship, relationshipChain, relationshipChainIndex); - } + } return included; } @@ -235,15 +235,15 @@ private List IncludeRelationshipChain( private List IncludeSingleResourceRelationships( List included, IIdentifiable navigationEntity, RelationshipAttribute relationship, string[] relationshipChain, int relationshipChainIndex) { - if (relationshipChainIndex < relationshipChain.Length) + if (relationshipChainIndex < relationshipChain.Length) { var nextContextEntity = _jsonApiContext.ContextGraph.GetContextEntity(relationship.Type); var resource = (IIdentifiable)navigationEntity; // recursive call - if(relationshipChainIndex < relationshipChain.Length - 1) + if (relationshipChainIndex < relationshipChain.Length - 1) included = IncludeRelationshipChain(included, nextContextEntity, resource, relationshipChain, relationshipChainIndex + 1); } - + return included; } @@ -289,7 +289,11 @@ private List GetRelationships(IEnumerable enti var relationships = new List(); foreach (var entity in entities) { - typeName = _jsonApiContext.ContextGraph.GetContextEntity(entity.GetType()).EntityName; + // this method makes the assumption that entities is a homogenous collection + // so, we just lookup the type of the first entity on the graph + // this is better than trying to get it from the generic parameter since it could + // be less specific than what is registered on the graph (e.g. IEnumerable) + typeName = typeName ?? _jsonApiContext.ContextGraph.GetContextEntity(entity.GetType()).EntityName; relationships.Add(new ResourceIdentifierObject { Type = typeName, @@ -304,7 +308,7 @@ private ResourceIdentifierObject GetRelationship(object entity) var objType = entity.GetType(); var contextEntity = _jsonApiContext.ContextGraph.GetContextEntity(objType); - if(entity is IIdentifiable identifiableEntity) + if (entity is IIdentifiable identifiableEntity) return new ResourceIdentifierObject { Type = contextEntity.EntityName, diff --git a/src/JsonApiDotNetCore/Internal/ContextGraph.cs b/src/JsonApiDotNetCore/Internal/ContextGraph.cs index d0733db5b1..b526ee44ac 100644 --- a/src/JsonApiDotNetCore/Internal/ContextGraph.cs +++ b/src/JsonApiDotNetCore/Internal/ContextGraph.cs @@ -18,6 +18,10 @@ public interface IContextGraph /// /// _graph.GetRelationship(todoItem, nameof(TodoItem.Owner)); /// + /// + /// This method will not work with HasManyThrough relationships. + /// You should instead use the method instead. + /// /// object GetRelationship(TParent resource, string propertyName); From 23dbad082f1671905ab4a9a3d6a1a4e075505f50 Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Wed, 26 Sep 2018 23:29:00 -0700 Subject: [PATCH 05/18] update documentation --- .../Models/HasManyThroughAttribute.cs | 46 +++++++++++++++---- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs b/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs index 7d03ec55b1..cf791114b2 100644 --- a/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs +++ b/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs @@ -4,25 +4,36 @@ namespace JsonApiDotNetCore.Models { /// - /// + /// Create a HasMany relationship through a many-to-many join relationship. /// /// /// - /// + /// In the following example, we expose a relationship named "tags" + /// through the navigation property `ArticleTags`. + /// The `Tags` property is decorated as `NotMapped` so that EF does not try + /// to map this to a database relationship. + /// + /// [NotMapped] + /// [HasManyThrough("tags", nameof(ArticleTags))] + /// public List<Tag> Tags { get; set; } + /// public List<ArticleTag> ArticleTags { get; set; } + /// /// public class HasManyThroughAttribute : HasManyAttribute { /// - /// + /// Create a HasMany relationship through a many-to-many join relationship. + /// The public name exposed through the API will be based on the configured convention. /// /// - /// The relationship name as exposed by the API /// The name of the navigation property that will be used to get the HasMany relationship /// Which links are available. Defaults to /// Whether or not this relationship can be included using the ?include=public-name query string /// /// - /// + /// + /// [HasManyThrough(nameof(ArticleTags), documentLinks: Link.All, canInclude: true)] + /// /// public HasManyThroughAttribute(string internalThroughName, Link documentLinks = Link.All, bool canInclude = true) : base(null, documentLinks, canInclude) @@ -31,7 +42,7 @@ public HasManyThroughAttribute(string internalThroughName, Link documentLinks = } /// - /// + /// Create a HasMany relationship through a many-to-many join relationship. /// /// /// The relationship name as exposed by the API @@ -40,7 +51,9 @@ public HasManyThroughAttribute(string internalThroughName, Link documentLinks = /// Whether or not this relationship can be included using the ?include=public-name query string /// /// - /// + /// + /// [HasManyThrough("tags", nameof(ArticleTags), documentLinks: Link.All, canInclude: true)] + /// /// public HasManyThroughAttribute(string publicName, string internalThroughName, Link documentLinks = Link.All, bool canInclude = true) : base(publicName, documentLinks, canInclude) @@ -48,11 +61,26 @@ public HasManyThroughAttribute(string publicName, string internalThroughName, Li InternalThroughName = internalThroughName; } + /// + /// The name of the join property on the parent resource. + /// + /// + /// In the `[HasManyThrough("tags", nameof(ArticleTags))]` example + /// this would be "ArticleTags". + /// public string InternalThroughName { get; private set; } + + /// + /// The join type. + /// + /// + /// In the `[HasManyThrough("tags", nameof(ArticleTags))]` example + /// this would be `ArticleTag`. + /// public Type ThroughType { get; internal set; } /// - /// The navigation property back to the parent resource. + /// The navigation property back to the parent resource from the join type. /// /// /// @@ -67,7 +95,7 @@ public HasManyThroughAttribute(string publicName, string internalThroughName, Li public PropertyInfo LeftProperty { get; internal set; } /// - /// The navigation property to the related resource. + /// The navigation property to the related resource from the join type. /// /// /// From 14d175badae678109af15318f6cbd574e2eca7ef Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Wed, 26 Sep 2018 23:30:50 -0700 Subject: [PATCH 06/18] use convention based attr --- src/Examples/JsonApiDotNetCoreExample/Models/Article.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/Article.cs b/src/Examples/JsonApiDotNetCoreExample/Models/Article.cs index 9bbfc15fe1..8d4d310f70 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Models/Article.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Models/Article.cs @@ -14,7 +14,7 @@ public class Article : Identifiable public int AuthorId { get; set; } [NotMapped] - [HasManyThrough("tags", nameof(ArticleTags))] + [HasManyThrough(nameof(ArticleTags))] public List Tags { get; set; } public List ArticleTags { get; set; } } From 11b0cc2e3a900d79e60e4fd23eb0d01c5344630c Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Wed, 26 Sep 2018 23:43:09 -0700 Subject: [PATCH 07/18] add test for creating many-to-many (failing) --- .../Acceptance/ManyToManyTests.cs | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs index 887c4c23a4..fc065f9172 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs @@ -1,10 +1,17 @@ +using System.Collections.Generic; +using System.Linq; using System.Net; +using System.Net.Http; +using System.Net.Http.Headers; using System.Threading.Tasks; using Bogus; using JsonApiDotNetCore.Serialization; using JsonApiDotNetCoreExample.Data; using JsonApiDotNetCoreExample.Models; +using Microsoft.EntityFrameworkCore; +using Newtonsoft.Json; using Xunit; +using Person = JsonApiDotNetCoreExample.Models.Person; namespace JsonApiDotNetCoreExampleTests.Acceptance { @@ -52,5 +59,68 @@ public async Task Can_Fetch_Many_To_Many_Through() var tagResponse = Assert.Single(articleResponse.Tags); Assert.Equal(tag.Id, tagResponse.Id); } + + [Fact] + public async Task Can_Create_Many_To_Many() + { + // arrange + var context = _fixture.GetService(); + var tag = _tagFaker.Generate(); + var author = new Person(); + context.Tags.Add(tag); + context.People.Add(author); + await context.SaveChangesAsync(); + + var article = _articleFaker.Generate(); + + var route = "/api/v1/articles"; + var request = new HttpRequestMessage(new HttpMethod("POST"), route); + var content = new + { + data = new + { + type = "articles", + relationships = new Dictionary + { + { "author", new { + data = new + { + type = "people", + id = author.StringId + } + } }, + { "tags", new { + data = new dynamic[] + { + new { + type = "tags", + id = tag.StringId + } + } + } } + } + } + }; + + request.Content = new StringContent(JsonConvert.SerializeObject(content)); + 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.Created == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}"); + + var articleResponse = _fixture.GetService().Deserialize
(body); + Assert.NotNull(articleResponse); + + var persistedArticle = await _fixture.Context.Articles + .Include(a => a.ArticleTags) + .SingleAsync(a => a.Id == articleResponse.Id); + + var persistedArticleTag = Assert.Single(persistedArticle.ArticleTags); + Assert.Equal(tag.Id, persistedArticleTag.TagId); + } } } \ No newline at end of file From 985342f32394eb5781390e48230d76a74ad8c6ef Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Thu, 27 Sep 2018 08:16:25 -0700 Subject: [PATCH 08/18] improve documentation --- src/JsonApiDotNetCore/Internal/ContextGraph.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/JsonApiDotNetCore/Internal/ContextGraph.cs b/src/JsonApiDotNetCore/Internal/ContextGraph.cs index b526ee44ac..3ea6758365 100644 --- a/src/JsonApiDotNetCore/Internal/ContextGraph.cs +++ b/src/JsonApiDotNetCore/Internal/ContextGraph.cs @@ -18,16 +18,19 @@ public interface IContextGraph /// /// _graph.GetRelationship(todoItem, nameof(TodoItem.Owner)); /// + /// /// - /// This method will not work with HasManyThrough relationships. - /// You should instead use the method instead. + /// In the case of a `HasManyThrough` relationship, it will not traverse the relationship + /// and will instead return the value of the shadow property (e.g. Articles.Tags). + /// If you want to traverse the relationship, you should use . /// - /// object GetRelationship(TParent resource, string propertyName); /// - /// Gets the value of the navigation property, defined by the relationshipName, + /// Gets the value of the navigation property (defined by the ) /// on the provided instance. + /// In the case of `HasManyThrough` relationships, it will traverse the through entity and return the + /// value of the relationship on the other side of a join entity (e.g. Articles.ArticleTags.Tag). /// /// The resource instance /// The attribute used to define the relationship. From 542b0214d7249e5714fd58e443582ebd3bf05f77 Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Thu, 27 Sep 2018 21:06:53 -0700 Subject: [PATCH 09/18] implement creation of HasManyThrough relationships --- .../Builders/ContextGraphBuilder.cs | 8 ++++ .../Data/DefaultEntityRepository.cs | 44 +++++++++++++++---- .../Extensions/TypeExtensions.cs | 12 +++++ .../Models/HasManyThroughAttribute.cs | 16 +++++++ .../Models/RelationshipAttribute.cs | 1 + .../Extensions/TypeExtensions_Tests.cs | 27 ++++++++++++ 6 files changed, 100 insertions(+), 8 deletions(-) diff --git a/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs b/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs index 41378de939..3cd5954075 100644 --- a/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs +++ b/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs @@ -1,4 +1,5 @@ using System; +using System.Collections; using System.Collections.Generic; using System.Linq; using System.Reflection; @@ -172,10 +173,17 @@ protected virtual List GetRelationships(Type entityType) if(throughProperty == null) throw new JsonApiSetupException($"Invalid '{nameof(HasManyThroughAttribute)}' on type '{entityType}'. Type does not contain a property named '{hasManyThroughAttribute.InternalThroughName}'."); + if(throughProperty.PropertyType.Implements() == false) + throw new JsonApiSetupException($"Invalid '{nameof(HasManyThroughAttribute)}' on type '{entityType}.{throughProperty.Name}'. Property type does not implement IList."); + // assumption: the property should be a generic collection, e.g. List if(throughProperty.PropertyType.IsGenericType == false) throw new JsonApiSetupException($"Invalid '{nameof(HasManyThroughAttribute)}' on type '{entityType}'. Expected through entity to be a generic type, such as List<{prop.PropertyType}>."); + // Article → List + hasManyThroughAttribute.ThroughProperty = throughProperty; + + // ArticleTag hasManyThroughAttribute.ThroughType = throughProperty.PropertyType.GetGenericArguments()[0]; var throughProperties = hasManyThroughAttribute.ThroughType.GetProperties(); diff --git a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs index b7ae1ef8d4..b3cc79c420 100644 --- a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs +++ b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs @@ -1,4 +1,5 @@ using System; +using System.Collections; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; @@ -143,7 +144,7 @@ public virtual async Task GetAndIncludeAsync(TId id, string relationshi /// public virtual async Task CreateAsync(TEntity entity) { - AttachRelationships(); + AttachRelationships(entity); _dbSet.Add(entity); await _context.SaveChangesAsync(); @@ -151,9 +152,9 @@ public virtual async Task CreateAsync(TEntity entity) return entity; } - protected virtual void AttachRelationships() + protected virtual void AttachRelationships(TEntity entity = null) { - AttachHasManyPointers(); + AttachHasManyPointers(entity); AttachHasOnePointers(); } @@ -183,15 +184,42 @@ public void DetachRelationshipPointers(TEntity entity) /// This is used to allow creation of HasMany relationships when the /// dependent side of the relationship already exists. /// - private void AttachHasManyPointers() + private void AttachHasManyPointers(TEntity entity) { var relationships = _jsonApiContext.HasManyRelationshipPointers.Get(); foreach (var relationship in relationships) { - foreach (var pointer in relationship.Value) - { - _context.Entry(pointer).State = EntityState.Unchanged; - } + if(relationship.Key is HasManyThroughAttribute hasManyThrough) + AttachHasManyThrough(entity, hasManyThrough, relationship.Value); + else + AttachHasMany(relationship.Key as HasManyAttribute, relationship.Value); + } + } + + private void AttachHasMany(HasManyAttribute relationship, IList pointers) + { + foreach (var pointer in pointers) + { + _context.Entry(pointer).State = EntityState.Unchanged; + } + } + + 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) + { + _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); } } diff --git a/src/JsonApiDotNetCore/Extensions/TypeExtensions.cs b/src/JsonApiDotNetCore/Extensions/TypeExtensions.cs index 74c390e8d5..29bd93adfd 100644 --- a/src/JsonApiDotNetCore/Extensions/TypeExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/TypeExtensions.cs @@ -67,5 +67,17 @@ private static object CreateNewInstance(Type type) throw new JsonApiException(500, $"Type '{type}' cannot be instantiated using the default constructor.", e); } } + + /// + /// Whether or not a type implements an interface. + /// + public static bool Implements(this Type concreteType) + => Implements(concreteType, typeof(T)); + + /// + /// Whether or not a type implements an interface. + /// + public static bool Implements(this Type concreteType, Type interfaceType) + => interfaceType?.IsAssignableFrom(concreteType) == true; } } diff --git a/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs b/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs index cf791114b2..ff865e63dc 100644 --- a/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs +++ b/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs @@ -5,6 +5,7 @@ namespace JsonApiDotNetCore.Models { /// /// Create a HasMany relationship through a many-to-many join relationship. + /// This type can only be applied on types that implement IList. /// /// /// @@ -109,6 +110,21 @@ public HasManyThroughAttribute(string publicName, string internalThroughName, Li /// public PropertyInfo RightProperty { get; internal set; } + /// + /// The join entity property on the parent resource. + /// + /// + /// + /// In the `[HasManyThrough("tags", nameof(ArticleTags))]` example + /// this would point to the `Article.ArticleTags` property + /// + /// + /// public List<ArticleTags> ArticleTags { get; set; } + /// + /// + /// + public PropertyInfo ThroughProperty { get; internal set; } + /// /// /// "ArticleTags.Tag" diff --git a/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs b/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs index b1cbb80e6e..7a5defa4a3 100644 --- a/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs +++ b/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs @@ -1,4 +1,5 @@ using System; +using System.Reflection; namespace JsonApiDotNetCore.Models { diff --git a/test/UnitTests/Extensions/TypeExtensions_Tests.cs b/test/UnitTests/Extensions/TypeExtensions_Tests.cs index f59fa37be0..f565019f26 100644 --- a/test/UnitTests/Extensions/TypeExtensions_Tests.cs +++ b/test/UnitTests/Extensions/TypeExtensions_Tests.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using JsonApiDotNetCore.Extensions; using JsonApiDotNetCore.Models; @@ -36,6 +37,32 @@ public void New_Creates_An_Instance_If_T_Implements_Interface() Assert.IsType(instance); } + [Fact] + public void Implements_Returns_True_If_Type_Implements_Interface() + { + // arrange + var type = typeof(Model); + + // act + var result = type.Implements(); + + // assert + Assert.True(result); + } + + [Fact] + public void Implements_Returns_False_If_Type_DoesNot_Implement_Interface() + { + // arrange + var type = typeof(String); + + // act + var result = type.Implements(); + + // assert + Assert.False(result); + } + private class Model : IIdentifiable { public string StringId { get; set; } From 1fa4605fe584b157690b96adb75e68cd93f79824 Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Thu, 27 Sep 2018 22:04:38 -0700 Subject: [PATCH 10/18] Can_Update_Many_To_Many test (failing) --- .../Data/DefaultEntityRepository.cs | 5 ++ .../Serialization/JsonApiDeSerializer.cs | 4 ++ .../Acceptance/ManyToManyTests.cs | 54 +++++++++++++++++++ .../Acceptance/TestFixture.cs | 6 +++ 4 files changed, 69 insertions(+) diff --git a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs index b3cc79c420..1665620d22 100644 --- a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs +++ b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs @@ -212,11 +212,14 @@ private void AttachHasManyThrough(TEntity entity, HasManyThroughAttribute hasMan // 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) { _context.Entry(pointer).State = EntityState.Unchanged; var throughInstance = Activator.CreateInstance(hasManyThrough.ThroughType); + _context.Entry(throughInstance).State = EntityState.Added; + hasManyThrough.LeftProperty.SetValue(throughInstance, entity); hasManyThrough.RightProperty.SetValue(throughInstance, pointer); throughRelationshipCollection.Add(throughInstance); @@ -249,6 +252,8 @@ public virtual async Task UpdateAsync(TId id, TEntity entity) foreach (var relationship in _jsonApiContext.RelationshipsToUpdate) relationship.Key.SetValue(oldEntity, relationship.Value); + AttachRelationships(entity); + await _context.SaveChangesAsync(); return oldEntity; diff --git a/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs b/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs index 28fc9c1ae2..20ae99194f 100644 --- a/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs +++ b/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs @@ -283,6 +283,10 @@ private object SetHasManyRelationship(object entity, if (relationships.TryGetValue(relationshipName, out RelationshipData relationshipData)) { + if(relationshipData.IsHasMany == false) { + throw new JsonApiException(400, $"Cannot set HasMany relationship '{attr.PublicRelationshipName}'. Value must be a JSON array of Resource Identifier Objects."); + } + var data = (List)relationshipData.ExposedData; if (data == null) return entity; diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs index fc065f9172..daa2d221a5 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs @@ -122,5 +122,59 @@ public async Task Can_Create_Many_To_Many() var persistedArticleTag = Assert.Single(persistedArticle.ArticleTags); Assert.Equal(tag.Id, persistedArticleTag.TagId); } + + [Fact] + public async Task Can_Update_Many_To_Many() + { + // arrange + var context = _fixture.GetService(); + var tag = _tagFaker.Generate(); + var article = _articleFaker.Generate(); + context.Tags.Add(tag); + context.Articles.Add(article); + await context.SaveChangesAsync(); + + var route = $"/api/v1/articles/{article.Id}"; + var request = new HttpRequestMessage(new HttpMethod("PATCH"), route); + var content = new + { + data = new + { + type = "articles", + id = article.StringId, + relationships = new Dictionary + { + { "tags", new { + data = new [] { new + { + type = "tags", + id = tag.StringId + } } + } } + } + } + }; + + request.Content = new StringContent(JsonConvert.SerializeObject(content)); + 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 articleResponse = _fixture.GetService().Deserialize
(body); + Assert.NotNull(articleResponse); + + _fixture.ReloadDbContext(); + var persistedArticle = await _fixture.Context.Articles + .Include(a => a.ArticleTags) + .SingleAsync(a => a.Id == articleResponse.Id); + + var persistedArticleTag = Assert.Single(persistedArticle.ArticleTags); + Assert.Equal(tag.Id, persistedArticleTag.TagId); + } } } \ No newline at end of file diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/TestFixture.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/TestFixture.cs index ce70fced84..6d6de345a0 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/TestFixture.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/TestFixture.cs @@ -6,6 +6,7 @@ using Microsoft.AspNetCore.TestHost; using JsonApiDotNetCore.Services; using JsonApiDotNetCore.Data; +using Microsoft.EntityFrameworkCore; namespace JsonApiDotNetCoreExampleTests.Acceptance { @@ -33,6 +34,11 @@ public TestFixture() public IJsonApiDeSerializer DeSerializer { get; private set; } public IJsonApiContext JsonApiContext { get; private set; } public T GetService() => (T)_services.GetService(typeof(T)); + + public void ReloadDbContext() + { + Context = new AppDbContext(GetService>()); + } private bool disposedValue = false; protected virtual void Dispose(bool disposing) From dc8453e47a2c3ff61a142548ab963abc148fd0dc Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Thu, 27 Sep 2018 22:45:14 -0700 Subject: [PATCH 11/18] fix tests...lots of ugly code in here. need to refactor --- .../Builders/ContextGraphBuilder.cs | 10 +++++ .../Data/DefaultEntityRepository.cs | 42 ++++++++++++++++--- .../Models/HasManyThroughAttribute.cs | 41 ++++++++++++++++++ 3 files changed, 88 insertions(+), 5 deletions(-) diff --git a/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs b/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs index 3cd5954075..f52ea7edd3 100644 --- a/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs +++ b/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs @@ -192,9 +192,19 @@ protected virtual List GetRelationships(Type entityType) hasManyThroughAttribute.LeftProperty = throughProperties.SingleOrDefault(x => x.PropertyType == entityType) ?? throw new JsonApiSetupException($"{hasManyThroughAttribute.ThroughType} does not contain a navigation property to type {entityType}"); + // ArticleTag.ArticleId + var leftIdPropertyName = hasManyThroughAttribute.GetIdentityPropertyName(hasManyThroughAttribute.LeftProperty); + hasManyThroughAttribute.LeftIdProperty = throughProperties.SingleOrDefault(p => p.Name == leftIdPropertyName) + ?? throw new JsonApiSetupException($"{hasManyThroughAttribute.ThroughType} does not contain an identity property {leftIdPropertyName}"); + // Article → ArticleTag.Tag hasManyThroughAttribute.RightProperty = throughProperties.SingleOrDefault(x => x.PropertyType == hasManyThroughAttribute.Type) ?? throw new JsonApiSetupException($"{hasManyThroughAttribute.ThroughType} does not contain a navigation property to type {hasManyThroughAttribute.Type}"); + + // ArticleTag.TagId + var rightIdPropertyName = hasManyThroughAttribute.GetIdentityPropertyName(hasManyThroughAttribute.RightProperty); + hasManyThroughAttribute.RightIdProperty = throughProperties.SingleOrDefault(p => p.Name == rightIdPropertyName) + ?? throw new JsonApiSetupException($"{hasManyThroughAttribute.ThroughType} does not contain an identity property {rightIdPropertyName}"); } } diff --git a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs index 1665620d22..7000659f73 100644 --- a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs +++ b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs @@ -199,9 +199,7 @@ private void AttachHasManyPointers(TEntity entity) private void AttachHasMany(HasManyAttribute relationship, IList pointers) { foreach (var pointer in pointers) - { _context.Entry(pointer).State = EntityState.Unchanged; - } } private void AttachHasManyThrough(TEntity entity, HasManyThroughAttribute hasManyThrough, IList pointers) @@ -218,14 +216,48 @@ private void AttachHasManyThrough(TEntity entity, HasManyThroughAttribute hasMan _context.Entry(pointer).State = EntityState.Unchanged; var throughInstance = Activator.CreateInstance(hasManyThrough.ThroughType); - _context.Entry(throughInstance).State = EntityState.Added; - hasManyThrough.LeftProperty.SetValue(throughInstance, entity); hasManyThrough.RightProperty.SetValue(throughInstance, pointer); + throughRelationshipCollection.Add(throughInstance); } } + private void UpdateHasManyThrough(TEntity entity) + { + var relationships = _jsonApiContext.HasManyRelationshipPointers.Get(); + foreach (var relationship in relationships) + { + if(relationship.Key is HasManyThroughAttribute hasManyThrough) + { + // 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 relationship.Value) + { + _context.Entry(pointer).State = EntityState.Unchanged; + + var throughInstance = Activator.CreateInstance(hasManyThrough.ThroughType); + _context.Entry(throughInstance).State = EntityState.Added; + + hasManyThrough.LeftIdProperty.SetValue(throughInstance, entity.Id); + hasManyThrough.LeftProperty.SetValue(throughInstance, entity); + hasManyThrough.RightProperty.SetValue(throughInstance, pointer); + + var pointerId = (pointer as Identifiable) // known limitation, just need to come up with a solution... + ?? throw new JsonApiException(500, $"Cannot update the HasManyThrough relationship '{hasManyThrough.PublicRelationshipName}'. Id type must match the parent resource id type."); + + hasManyThrough.RightIdProperty.SetValue(throughInstance, pointerId.Id); + + throughRelationshipCollection.Add(throughInstance); + } + } + } + } + /// /// This is used to allow creation of HasOne relationships when the /// independent side of the relationship already exists. @@ -252,7 +284,7 @@ public virtual async Task UpdateAsync(TId id, TEntity entity) foreach (var relationship in _jsonApiContext.RelationshipsToUpdate) relationship.Key.SetValue(oldEntity, relationship.Value); - AttachRelationships(entity); + UpdateHasManyThrough(entity); await _context.SaveChangesAsync(); diff --git a/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs b/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs index ff865e63dc..f7e238664b 100644 --- a/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs +++ b/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs @@ -95,6 +95,21 @@ public HasManyThroughAttribute(string publicName, string internalThroughName, Li /// public PropertyInfo LeftProperty { get; internal set; } + /// + /// The identity property back to the parent resource. + /// + /// + /// + /// In the `[HasManyThrough("tags", nameof(ArticleTags))]` example + /// this would point to the `Article.ArticleTags.ArticleId` property + /// + /// + /// public int ArticleId { get; set; } + /// + /// + /// + public PropertyInfo LeftIdProperty { get; internal set; } + /// /// The navigation property to the related resource from the join type. /// @@ -110,6 +125,32 @@ public HasManyThroughAttribute(string publicName, string internalThroughName, Li /// public PropertyInfo RightProperty { get; internal set; } + /// + /// The identity property back to the related resource from the join type. + /// + /// + /// + /// In the `[HasManyThrough("tags", nameof(ArticleTags))]` example + /// this would point to the `Article.ArticleTags.TagId` property + /// + /// + /// public int TagId { get; set; } + /// + /// + /// + public PropertyInfo RightIdProperty { get; internal set; } + + /// + /// Get the name of the identity property from the navigation property. + /// + /// + /// + /// In the `[HasManyThrough("tags", nameof(ArticleTags))]` example + /// this would translate to the `Article.ArticleTags.Tag` property + /// to TagId. This is then used to set the Left and RightIdProperties. + /// + public virtual string GetIdentityPropertyName(PropertyInfo property) => property.Name + "Id"; + /// /// The join entity property on the parent resource. /// From 9c52b774f7ffe02f11607c611a1b82da5ee710b5 Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Thu, 27 Sep 2018 23:00:06 -0700 Subject: [PATCH 12/18] cleanup repository --- .../Data/DefaultEntityRepository.cs | 39 +------------------ 1 file changed, 2 insertions(+), 37 deletions(-) diff --git a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs index 7000659f73..87ea4519fc 100644 --- a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs +++ b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs @@ -214,8 +214,8 @@ private void AttachHasManyThrough(TEntity entity, HasManyThroughAttribute hasMan foreach (var pointer in pointers) { _context.Entry(pointer).State = EntityState.Unchanged; - var throughInstance = Activator.CreateInstance(hasManyThrough.ThroughType); + hasManyThrough.LeftProperty.SetValue(throughInstance, entity); hasManyThrough.RightProperty.SetValue(throughInstance, pointer); @@ -223,41 +223,6 @@ private void AttachHasManyThrough(TEntity entity, HasManyThroughAttribute hasMan } } - private void UpdateHasManyThrough(TEntity entity) - { - var relationships = _jsonApiContext.HasManyRelationshipPointers.Get(); - foreach (var relationship in relationships) - { - if(relationship.Key is HasManyThroughAttribute hasManyThrough) - { - // 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 relationship.Value) - { - _context.Entry(pointer).State = EntityState.Unchanged; - - var throughInstance = Activator.CreateInstance(hasManyThrough.ThroughType); - _context.Entry(throughInstance).State = EntityState.Added; - - hasManyThrough.LeftIdProperty.SetValue(throughInstance, entity.Id); - hasManyThrough.LeftProperty.SetValue(throughInstance, entity); - hasManyThrough.RightProperty.SetValue(throughInstance, pointer); - - var pointerId = (pointer as Identifiable) // known limitation, just need to come up with a solution... - ?? throw new JsonApiException(500, $"Cannot update the HasManyThrough relationship '{hasManyThrough.PublicRelationshipName}'. Id type must match the parent resource id type."); - - hasManyThrough.RightIdProperty.SetValue(throughInstance, pointerId.Id); - - throughRelationshipCollection.Add(throughInstance); - } - } - } - } - /// /// This is used to allow creation of HasOne relationships when the /// independent side of the relationship already exists. @@ -284,7 +249,7 @@ public virtual async Task UpdateAsync(TId id, TEntity entity) foreach (var relationship in _jsonApiContext.RelationshipsToUpdate) relationship.Key.SetValue(oldEntity, relationship.Value); - UpdateHasManyThrough(entity); + AttachRelationships(oldEntity); await _context.SaveChangesAsync(); From 412ab89544e76b3a849ba5f7fdbffe1188e48a28 Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Thu, 27 Sep 2018 23:01:55 -0700 Subject: [PATCH 13/18] remove unnecessary code --- .../Builders/ContextGraphBuilder.cs | 10 ----- .../Models/HasManyThroughAttribute.cs | 41 ------------------- 2 files changed, 51 deletions(-) diff --git a/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs b/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs index f52ea7edd3..7ff8e299f2 100644 --- a/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs +++ b/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs @@ -191,20 +191,10 @@ protected virtual List GetRelationships(Type entityType) // Article → ArticleTag.Article hasManyThroughAttribute.LeftProperty = throughProperties.SingleOrDefault(x => x.PropertyType == entityType) ?? throw new JsonApiSetupException($"{hasManyThroughAttribute.ThroughType} does not contain a navigation property to type {entityType}"); - - // ArticleTag.ArticleId - var leftIdPropertyName = hasManyThroughAttribute.GetIdentityPropertyName(hasManyThroughAttribute.LeftProperty); - hasManyThroughAttribute.LeftIdProperty = throughProperties.SingleOrDefault(p => p.Name == leftIdPropertyName) - ?? throw new JsonApiSetupException($"{hasManyThroughAttribute.ThroughType} does not contain an identity property {leftIdPropertyName}"); // Article → ArticleTag.Tag hasManyThroughAttribute.RightProperty = throughProperties.SingleOrDefault(x => x.PropertyType == hasManyThroughAttribute.Type) ?? throw new JsonApiSetupException($"{hasManyThroughAttribute.ThroughType} does not contain a navigation property to type {hasManyThroughAttribute.Type}"); - - // ArticleTag.TagId - var rightIdPropertyName = hasManyThroughAttribute.GetIdentityPropertyName(hasManyThroughAttribute.RightProperty); - hasManyThroughAttribute.RightIdProperty = throughProperties.SingleOrDefault(p => p.Name == rightIdPropertyName) - ?? throw new JsonApiSetupException($"{hasManyThroughAttribute.ThroughType} does not contain an identity property {rightIdPropertyName}"); } } diff --git a/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs b/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs index f7e238664b..ff865e63dc 100644 --- a/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs +++ b/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs @@ -95,21 +95,6 @@ public HasManyThroughAttribute(string publicName, string internalThroughName, Li /// public PropertyInfo LeftProperty { get; internal set; } - /// - /// The identity property back to the parent resource. - /// - /// - /// - /// In the `[HasManyThrough("tags", nameof(ArticleTags))]` example - /// this would point to the `Article.ArticleTags.ArticleId` property - /// - /// - /// public int ArticleId { get; set; } - /// - /// - /// - public PropertyInfo LeftIdProperty { get; internal set; } - /// /// The navigation property to the related resource from the join type. /// @@ -125,32 +110,6 @@ public HasManyThroughAttribute(string publicName, string internalThroughName, Li /// public PropertyInfo RightProperty { get; internal set; } - /// - /// The identity property back to the related resource from the join type. - /// - /// - /// - /// In the `[HasManyThrough("tags", nameof(ArticleTags))]` example - /// this would point to the `Article.ArticleTags.TagId` property - /// - /// - /// public int TagId { get; set; } - /// - /// - /// - public PropertyInfo RightIdProperty { get; internal set; } - - /// - /// Get the name of the identity property from the navigation property. - /// - /// - /// - /// In the `[HasManyThrough("tags", nameof(ArticleTags))]` example - /// this would translate to the `Article.ArticleTags.Tag` property - /// to TagId. This is then used to set the Left and RightIdProperties. - /// - public virtual string GetIdentityPropertyName(PropertyInfo property) => property.Name + "Id"; - /// /// The join entity property on the parent resource. /// From edbbd4253a048cbafb7d414cf0f4b31ac1213428 Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Thu, 27 Sep 2018 23:06:18 -0700 Subject: [PATCH 14/18] add test Can_Update_Many_To_Many_Through_Relationship_Link (failing) --- .../Acceptance/ManyToManyTests.cs | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs index daa2d221a5..2a61f9e8cf 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs @@ -176,5 +176,50 @@ public async Task Can_Update_Many_To_Many() var persistedArticleTag = Assert.Single(persistedArticle.ArticleTags); Assert.Equal(tag.Id, persistedArticleTag.TagId); } + + [Fact] + public async Task Can_Update_Many_To_Many_Through_Relationship_Link() + { + // arrange + var context = _fixture.GetService(); + var tag = _tagFaker.Generate(); + var article = _articleFaker.Generate(); + context.Tags.Add(tag); + context.Articles.Add(article); + await context.SaveChangesAsync(); + + var route = $"/api/v1/articles/{article.Id}/relationships/tags"; + var request = new HttpRequestMessage(new HttpMethod("PATCH"), route); + var content = new + { + data = new [] { + new { + type = "tags", + id = tag.StringId + } + } + }; + + request.Content = new StringContent(JsonConvert.SerializeObject(content)); + 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 articleResponse = _fixture.GetService().Deserialize
(body); + Assert.NotNull(articleResponse); + + _fixture.ReloadDbContext(); + var persistedArticle = await _fixture.Context.Articles + .Include(a => a.ArticleTags) + .SingleAsync(a => a.Id == articleResponse.Id); + + var persistedArticleTag = Assert.Single(persistedArticle.ArticleTags); + Assert.Equal(tag.Id, persistedArticleTag.TagId); + } } } \ No newline at end of file From 1da949c8d792caf4823483c828ffd59ded07c826 Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Tue, 2 Oct 2018 09:07:43 -0700 Subject: [PATCH 15/18] fix test --- .../Builders/ContextGraphBuilder.cs | 12 +++++- .../Configuration/JsonApiOptions.cs | 5 +++ .../Data/DefaultEntityRepository.cs | 10 ++++- .../Extensions/TypeExtensions.cs | 12 ++++++ .../Graph/ResourceIdMapper.cs | 27 +++++++++++++ .../Internal/Generics/GenericProcessor.cs | 40 ++++++++++++------- .../Models/HasManyThroughAttribute.cs | 30 ++++++++++++++ .../Models/HasOneAttribute.cs | 3 +- .../Models/RelationshipAttribute.cs | 5 ++- .../Acceptance/ManyToManyTests.cs | 5 +-- 10 files changed, 126 insertions(+), 23 deletions(-) create mode 100644 src/JsonApiDotNetCore/Graph/ResourceIdMapper.cs diff --git a/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs b/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs index 7ff8e299f2..88670821fe 100644 --- a/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs +++ b/src/JsonApiDotNetCore/Builders/ContextGraphBuilder.cs @@ -188,13 +188,23 @@ protected virtual List GetRelationships(Type entityType) var throughProperties = hasManyThroughAttribute.ThroughType.GetProperties(); - // Article → ArticleTag.Article + // ArticleTag.Article hasManyThroughAttribute.LeftProperty = throughProperties.SingleOrDefault(x => x.PropertyType == entityType) ?? throw new JsonApiSetupException($"{hasManyThroughAttribute.ThroughType} does not contain a navigation property to type {entityType}"); + // ArticleTag.ArticleId + var leftIdPropertyName = JsonApiOptions.RelatedIdMapper.GetRelatedIdPropertyName(hasManyThroughAttribute.LeftProperty.Name); + hasManyThroughAttribute.LeftIdProperty = throughProperties.SingleOrDefault(x => x.Name == leftIdPropertyName) + ?? throw new JsonApiSetupException($"{hasManyThroughAttribute.ThroughType} does not contain a relationship id property to type {entityType} with name {leftIdPropertyName}"); + // Article → ArticleTag.Tag hasManyThroughAttribute.RightProperty = throughProperties.SingleOrDefault(x => x.PropertyType == hasManyThroughAttribute.Type) ?? throw new JsonApiSetupException($"{hasManyThroughAttribute.ThroughType} does not contain a navigation property to type {hasManyThroughAttribute.Type}"); + + // ArticleTag.TagId + var rightIdPropertyName = JsonApiOptions.RelatedIdMapper.GetRelatedIdPropertyName(hasManyThroughAttribute.RightProperty.Name); + hasManyThroughAttribute.RightIdProperty = throughProperties.SingleOrDefault(x => x.Name == rightIdPropertyName) + ?? throw new JsonApiSetupException($"{hasManyThroughAttribute.ThroughType} does not contain a relationship id property to type {hasManyThroughAttribute.Type} with name {rightIdPropertyName}"); } } diff --git a/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs b/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs index 66386aac19..e4d61e3864 100644 --- a/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs +++ b/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs @@ -22,6 +22,11 @@ public class JsonApiOptions ///
public static IResourceNameFormatter ResourceNameFormatter { get; set; } = new DefaultResourceNameFormatter(); + /// + /// Provides an interface for formatting relationship id properties given the navigation property name + /// + public static IRelatedIdMapper RelatedIdMapper { get; set; } = new DefaultRelatedIdMapper(); + /// /// Whether or not stack traces should be serialized in Error objects /// diff --git a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs index 87ea4519fc..c0c7c93192 100644 --- a/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs +++ b/src/JsonApiDotNetCore/Data/DefaultEntityRepository.cs @@ -259,7 +259,15 @@ public virtual async Task UpdateAsync(TId id, TEntity entity) /// public async Task UpdateRelationshipsAsync(object parent, RelationshipAttribute relationship, IEnumerable relationshipIds) { - var genericProcessor = _genericProcessorFactory.GetProcessor(typeof(GenericProcessor<>), relationship.Type); + // TODO: it would be better to let this be determined within the relationship attribute... + // need to think about the right way to do that since HasMany doesn't need to think about this + // and setting the HasManyThrough.Type to the join type (ArticleTag instead of Tag) for this changes the semantics + // of the property... + var typeToUpdate = (relationship is HasManyThroughAttribute hasManyThrough) + ? hasManyThrough.ThroughType + : relationship.Type; + + var genericProcessor = _genericProcessorFactory.GetProcessor(typeof(GenericProcessor<>), typeToUpdate); await genericProcessor.UpdateRelationshipsAsync(parent, relationship, relationshipIds); } diff --git a/src/JsonApiDotNetCore/Extensions/TypeExtensions.cs b/src/JsonApiDotNetCore/Extensions/TypeExtensions.cs index 29bd93adfd..03874d9b76 100644 --- a/src/JsonApiDotNetCore/Extensions/TypeExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/TypeExtensions.cs @@ -79,5 +79,17 @@ public static bool Implements(this Type concreteType) /// public static bool Implements(this Type concreteType, Type interfaceType) => interfaceType?.IsAssignableFrom(concreteType) == true; + + /// + /// Whether or not a type inherits a base type. + /// + public static bool Inherits(this Type concreteType) + => Inherits(concreteType, typeof(T)); + + /// + /// Whether or not a type inherits a base type. + /// + public static bool Inherits(this Type concreteType, Type interfaceType) + => interfaceType?.IsAssignableFrom(concreteType) == true; } } diff --git a/src/JsonApiDotNetCore/Graph/ResourceIdMapper.cs b/src/JsonApiDotNetCore/Graph/ResourceIdMapper.cs new file mode 100644 index 0000000000..7252d5c710 --- /dev/null +++ b/src/JsonApiDotNetCore/Graph/ResourceIdMapper.cs @@ -0,0 +1,27 @@ +namespace JsonApiDotNetCore.Graph +{ + /// + /// Provides an interface for formatting relationship identifiers from the navigation property name + /// + public interface IRelatedIdMapper + { + /// + /// Get the internal property name for the database mapped identifier property + /// + /// + /// + /// + /// DefaultResourceNameFormatter.FormatId("Article"); + /// // "ArticleId" + /// + /// + string GetRelatedIdPropertyName(string propertyName); + } + + /// + public class DefaultRelatedIdMapper : IRelatedIdMapper + { + /// + public string GetRelatedIdPropertyName(string propertyName) => propertyName + "Id"; + } +} \ No newline at end of file diff --git a/src/JsonApiDotNetCore/Internal/Generics/GenericProcessor.cs b/src/JsonApiDotNetCore/Internal/Generics/GenericProcessor.cs index c9576367c5..bfca40e80d 100644 --- a/src/JsonApiDotNetCore/Internal/Generics/GenericProcessor.cs +++ b/src/JsonApiDotNetCore/Internal/Generics/GenericProcessor.cs @@ -16,7 +16,7 @@ public interface IGenericProcessor void SetRelationships(object parent, RelationshipAttribute relationship, IEnumerable relationshipIds); } - public class GenericProcessor : IGenericProcessor where T : class, IIdentifiable + public class GenericProcessor : IGenericProcessor where T : class { private readonly DbContext _context; public GenericProcessor(IDbContextResolver contextResolver) @@ -33,39 +33,51 @@ public virtual async Task UpdateRelationshipsAsync(object parent, RelationshipAt public virtual void SetRelationships(object parent, RelationshipAttribute relationship, IEnumerable relationshipIds) { - if (relationship is HasManyThroughAttribute hasManyThrough) + if (relationship is HasManyThroughAttribute hasManyThrough && parent is IIdentifiable identifiableParent) { - var parentId = ((IIdentifiable)parent).StringId; - ParameterExpression parameter = Expression.Parameter(hasManyThrough.Type); - Expression property = Expression.Property(parameter, hasManyThrough.LeftProperty); + // ArticleTag + ParameterExpression parameter = Expression.Parameter(hasManyThrough.ThroughType); + + // ArticleTag.ArticleId + Expression property = Expression.Property(parameter, hasManyThrough.LeftIdProperty); + + // article.Id + var parentId = TypeHelper.ConvertType(identifiableParent.StringId, hasManyThrough.LeftIdProperty.PropertyType); Expression target = Expression.Constant(parentId); - Expression toString = Expression.Call(property, "ToString", null, null); - Expression equals = Expression.Call(toString, "Equals", null, target); - Expression> lambda = Expression.Lambda>(equals, parameter); + + // ArticleTag.ArticleId.Equals(article.Id) + Expression equals = Expression.Call(property, "Equals", null, target); + + var lambda = Expression.Lambda>(equals, parameter); var oldLinks = _context - .Set(hasManyThrough.ThroughType) + .Set() .Where(lambda.Compile()) .ToList(); - _context.Remove(oldLinks); + // TODO: we shouldn't need to do this and it especially shouldn't happen outside a transaction + // instead we should try updating the existing? + _context.RemoveRange(oldLinks); var newLinks = relationshipIds.Select(x => { var link = Activator.CreateInstance(hasManyThrough.ThroughType); - hasManyThrough.LeftProperty.SetValue(link, TypeHelper.ConvertType(parent, hasManyThrough.LeftProperty.PropertyType)); - hasManyThrough.RightProperty.SetValue(link, TypeHelper.ConvertType(x, hasManyThrough.RightProperty.PropertyType)); + hasManyThrough.LeftIdProperty.SetValue(link, TypeHelper.ConvertType(parentId, hasManyThrough.LeftIdProperty.PropertyType)); + hasManyThrough.RightIdProperty.SetValue(link, TypeHelper.ConvertType(x, hasManyThrough.RightIdProperty.PropertyType)); return link; }); + _context.AddRange(newLinks); } else if (relationship.IsHasMany) { - var entities = _context.Set().Where(x => relationshipIds.Contains(x.StringId)).ToList(); + // TODO: need to handle the failure mode when the relationship does not implement IIdentifiable + var entities = _context.Set().Where(x => relationshipIds.Contains(((IIdentifiable)x).StringId)).ToList(); relationship.SetValue(parent, entities); } else { - var entity = _context.Set().SingleOrDefault(x => relationshipIds.First() == x.StringId); + // TODO: need to handle the failure mode when the relationship does not implement IIdentifiable + var entity = _context.Set().SingleOrDefault(x => relationshipIds.First() == ((IIdentifiable)x).StringId); relationship.SetValue(parent, entity); } } diff --git a/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs b/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs index ff865e63dc..49e18a43b3 100644 --- a/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs +++ b/src/JsonApiDotNetCore/Models/HasManyThroughAttribute.cs @@ -95,6 +95,21 @@ public HasManyThroughAttribute(string publicName, string internalThroughName, Li /// public PropertyInfo LeftProperty { get; internal set; } + /// + /// The id property back to the parent resource from the join type. + /// + /// + /// + /// In the `[HasManyThrough("tags", nameof(ArticleTags))]` example + /// this would point to the `Article.ArticleTags.ArticleId` property + /// + /// + /// public int ArticleId { get; set; } + /// + /// + /// + public PropertyInfo LeftIdProperty { get; internal set; } + /// /// The navigation property to the related resource from the join type. /// @@ -110,6 +125,21 @@ public HasManyThroughAttribute(string publicName, string internalThroughName, Li /// public PropertyInfo RightProperty { get; internal set; } + /// + /// The id property to the related resource from the join type. + /// + /// + /// + /// In the `[HasManyThrough("tags", nameof(ArticleTags))]` example + /// this would point to the `Article.ArticleTags.TagId` property + /// + /// + /// public int TagId { get; set; } + /// + /// + /// + public PropertyInfo RightIdProperty { get; internal set; } + /// /// The join entity property on the parent resource. /// diff --git a/src/JsonApiDotNetCore/Models/HasOneAttribute.cs b/src/JsonApiDotNetCore/Models/HasOneAttribute.cs index 2d83c3dd69..4c145727eb 100644 --- a/src/JsonApiDotNetCore/Models/HasOneAttribute.cs +++ b/src/JsonApiDotNetCore/Models/HasOneAttribute.cs @@ -1,4 +1,5 @@ using System; +using JsonApiDotNetCore.Configuration; namespace JsonApiDotNetCore.Models { @@ -38,7 +39,7 @@ public HasOneAttribute(string publicName = null, Link documentLinks = Link.All, /// The independent resource identifier. /// public string IdentifiablePropertyName => string.IsNullOrWhiteSpace(_explicitIdentifiablePropertyName) - ? $"{InternalRelationshipName}Id" + ? JsonApiOptions.RelatedIdMapper.GetRelatedIdPropertyName(InternalRelationshipName) : _explicitIdentifiablePropertyName; /// diff --git a/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs b/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs index 7a5defa4a3..a93bcbc868 100644 --- a/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs +++ b/src/JsonApiDotNetCore/Models/RelationshipAttribute.cs @@ -1,5 +1,6 @@ using System; using System.Reflection; +using JsonApiDotNetCore.Extensions; namespace JsonApiDotNetCore.Models { @@ -22,11 +23,11 @@ protected RelationshipAttribute(string publicName, Link documentLinks, bool canI /// /// /// - /// public List<Articles> Articles { get; set; } // Type => Article + /// public List<Tag> Tags { get; set; } // Type => Tag /// /// public Type Type { get; internal set; } - public bool IsHasMany => GetType() == typeof(HasManyAttribute) || typeof(HasManyAttribute).IsAssignableFrom(GetType()); + public bool IsHasMany => GetType() == typeof(HasManyAttribute) || GetType().Inherits(typeof(HasManyAttribute)); public bool IsHasOne => GetType() == typeof(HasOneAttribute); public Link DocumentLinks { get; } = Link.All; public bool CanInclude { get; } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs index 2a61f9e8cf..e483aa327c 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/ManyToManyTests.cs @@ -210,13 +210,10 @@ public async Task Can_Update_Many_To_Many_Through_Relationship_Link() var body = await response.Content.ReadAsStringAsync(); Assert.True(HttpStatusCode.OK == response.StatusCode, $"{route} returned {response.StatusCode} status code with payload: {body}"); - var articleResponse = _fixture.GetService().Deserialize
(body); - Assert.NotNull(articleResponse); - _fixture.ReloadDbContext(); var persistedArticle = await _fixture.Context.Articles .Include(a => a.ArticleTags) - .SingleAsync(a => a.Id == articleResponse.Id); + .SingleAsync(a => a.Id == article.Id); var persistedArticleTag = Assert.Single(persistedArticle.ArticleTags); Assert.Equal(tag.Id, persistedArticleTag.TagId); From 3c20861f3c2f7a20b1ad7172df1f9f76cb09bf70 Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Tue, 2 Oct 2018 21:06:32 -0700 Subject: [PATCH 16/18] wrap updates in transaction --- .../Extensions/DbContextExtensions.cs | 63 +++++++++++++++++++ .../Internal/Generics/GenericProcessor.cs | 42 ++++++++++--- 2 files changed, 96 insertions(+), 9 deletions(-) diff --git a/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs b/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs index 6df8db4c39..c4b059f403 100644 --- a/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs +++ b/src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs @@ -1,7 +1,10 @@ using System; using System.Linq; +using System.Threading.Tasks; using JsonApiDotNetCore.Models; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Storage; namespace JsonApiDotNetCore.Extensions { @@ -38,5 +41,65 @@ public static bool EntityIsTracked(this DbContext context, IIdentifiable entity) return trackedEntries != null; } + + /// + /// Gets the current transaction or creates a new one. + /// If a transaction already exists, commit, rollback and dispose + /// will not be called. It is assumed the creator of the original + /// transaction should be responsible for disposal. + /// + /// + /// + /// + /// using(var transaction = _context.GetCurrentOrCreateTransaction()) + /// { + /// // perform multiple operations on the context and then save... + /// _context.SaveChanges(); + /// } + /// + /// + public static async Task GetCurrentOrCreateTransactionAsync(this DbContext context) + => await SafeTransactionProxy.GetOrCreateAsync(context.Database); + } + + /// + /// Gets the current transaction or creates a new one. + /// If a transaction already exists, commit, rollback and dispose + /// will not be called. It is assumed the creator of the original + /// transaction should be responsible for disposal. + /// + internal struct SafeTransactionProxy : IDbContextTransaction + { + private readonly bool _shouldExecute; + private readonly IDbContextTransaction _transaction; + + private SafeTransactionProxy(IDbContextTransaction transaction, bool shouldExecute) + { + _transaction = transaction; + _shouldExecute = shouldExecute; + } + + public static async Task GetOrCreateAsync(DatabaseFacade databaseFacade) + => (databaseFacade.CurrentTransaction != null) + ? new SafeTransactionProxy(databaseFacade.CurrentTransaction, shouldExecute: false) + : new SafeTransactionProxy(await databaseFacade.BeginTransactionAsync(), shouldExecute: true); + + /// + public Guid TransactionId => _transaction.TransactionId; + + /// + public void Commit() => Proxy(t => t.Commit()); + + /// + public void Rollback() => Proxy(t => t.Rollback()); + + /// + public void Dispose() => Proxy(t => t.Dispose()); + + private void Proxy(Action func) + { + if(_shouldExecute) + func(_transaction); + } } } diff --git a/src/JsonApiDotNetCore/Internal/Generics/GenericProcessor.cs b/src/JsonApiDotNetCore/Internal/Generics/GenericProcessor.cs index bfca40e80d..8a98745465 100644 --- a/src/JsonApiDotNetCore/Internal/Generics/GenericProcessor.cs +++ b/src/JsonApiDotNetCore/Internal/Generics/GenericProcessor.cs @@ -7,15 +7,22 @@ using JsonApiDotNetCore.Extensions; using JsonApiDotNetCore.Models; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Storage; namespace JsonApiDotNetCore.Internal.Generics { + // TODO: consider renaming to PatchRelationshipService (or something) public interface IGenericProcessor { Task UpdateRelationshipsAsync(object parent, RelationshipAttribute relationship, IEnumerable relationshipIds); - void SetRelationships(object parent, RelationshipAttribute relationship, IEnumerable relationshipIds); } + /// + /// A special processor that gets instantiated for a generic type (<T>) + /// when the actual type is not known until runtime. Specifically, this is used for updating + /// relationships. + /// public class GenericProcessor : IGenericProcessor where T : class { private readonly DbContext _context; @@ -26,14 +33,21 @@ public GenericProcessor(IDbContextResolver contextResolver) public virtual async Task UpdateRelationshipsAsync(object parent, RelationshipAttribute relationship, IEnumerable relationshipIds) { - SetRelationships(parent, relationship, relationshipIds); - - await _context.SaveChangesAsync(); + if (relationship is HasManyThroughAttribute hasManyThrough && parent is IIdentifiable identifiableParent) + { + await SetHasManyThroughRelationshipAsync(identifiableParent, hasManyThrough, relationshipIds); + } + else + { + await SetRelationshipsAsync(parent, relationship, relationshipIds); + } } - public virtual void SetRelationships(object parent, RelationshipAttribute relationship, IEnumerable relationshipIds) + private async Task SetHasManyThroughRelationshipAsync(IIdentifiable identifiableParent, HasManyThroughAttribute hasManyThrough, IEnumerable relationshipIds) { - if (relationship is HasManyThroughAttribute hasManyThrough && parent is IIdentifiable identifiableParent) + // we need to create a transaction for the HasManyThrough case so we can get and remove any existing + // join entities and only commit if all operations are successful + using(var transaction = await _context.GetCurrentOrCreateTransactionAsync()) { // ArticleTag ParameterExpression parameter = Expression.Parameter(hasManyThrough.ThroughType); @@ -50,13 +64,14 @@ public virtual void SetRelationships(object parent, RelationshipAttribute relati var lambda = Expression.Lambda>(equals, parameter); + // TODO: we shouldn't need to do this instead we should try updating the existing? + // the challenge here is if a composite key is used, then we will fail to + // create due to a unique key violation var oldLinks = _context .Set() .Where(lambda.Compile()) .ToList(); - // TODO: we shouldn't need to do this and it especially shouldn't happen outside a transaction - // instead we should try updating the existing? _context.RemoveRange(oldLinks); var newLinks = relationshipIds.Select(x => { @@ -67,8 +82,15 @@ public virtual void SetRelationships(object parent, RelationshipAttribute relati }); _context.AddRange(newLinks); + await _context.SaveChangesAsync(); + + transaction.Commit(); } - else if (relationship.IsHasMany) + } + + private async Task SetRelationshipsAsync(object parent, RelationshipAttribute relationship, IEnumerable relationshipIds) + { + if (relationship.IsHasMany) { // TODO: need to handle the failure mode when the relationship does not implement IIdentifiable var entities = _context.Set().Where(x => relationshipIds.Contains(((IIdentifiable)x).StringId)).ToList(); @@ -80,6 +102,8 @@ public virtual void SetRelationships(object parent, RelationshipAttribute relati var entity = _context.Set().SingleOrDefault(x => relationshipIds.First() == ((IIdentifiable)x).StringId); relationship.SetValue(parent, entity); } + + await _context.SaveChangesAsync(); } } } From 78b0ee7abf6119a9cb083a00a481444adb1e9f47 Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Tue, 2 Oct 2018 21:25:34 -0700 Subject: [PATCH 17/18] fix unit tests --- .../Data/DefaultEntityRepository_Tests.cs | 174 +++++++++--------- 1 file changed, 92 insertions(+), 82 deletions(-) diff --git a/test/UnitTests/Data/DefaultEntityRepository_Tests.cs b/test/UnitTests/Data/DefaultEntityRepository_Tests.cs index 5b50fa4cbc..f5a77789e2 100644 --- a/test/UnitTests/Data/DefaultEntityRepository_Tests.cs +++ b/test/UnitTests/Data/DefaultEntityRepository_Tests.cs @@ -11,8 +11,10 @@ using Microsoft.Extensions.Logging; using JsonApiDotNetCore.Services; using System.Threading.Tasks; -using System.Linq; - +using System.Linq; +using System.Collections; +using JsonApiDotNetCore.Request; + namespace UnitTests.Data { public class DefaultEntityRepository_Tests : JsonApiControllerMixin @@ -49,10 +51,10 @@ public async Task UpdateAsync_Updates_Attributes_In_AttributesToUpdate() { Id = _todoItem.Id, Description = Guid.NewGuid().ToString() - }; - + }; + var descAttr = new AttrAttribute("description", "Description"); - descAttr.PropertyInfo = typeof(TodoItem).GetProperty(nameof(TodoItem.Description)); + descAttr.PropertyInfo = typeof(TodoItem).GetProperty(nameof(TodoItem.Description)); _attrsToUpdate = new Dictionary { @@ -91,87 +93,95 @@ private DefaultEntityRepository GetRepository() .Setup(m => m.RelationshipsToUpdate) .Returns(_relationshipsToUpdate); + _jsonApiContextMock + .Setup(m => m.HasManyRelationshipPointers) + .Returns(new HasManyRelationshipPointers()); + + _jsonApiContextMock + .Setup(m => m.HasOneRelationshipPointers) + .Returns(new HasOneRelationshipPointers()); + return new DefaultEntityRepository( _loggFactoryMock.Object, _jsonApiContextMock.Object, _contextResolverMock.Object); - } - - [Theory] - [InlineData(0)] - [InlineData(-1)] - [InlineData(-10)] - public async Task Page_When_PageSize_Is_NonPositive_Does_Nothing(int pageSize) - { - var todoItems = DbSetMock.Create(TodoItems(2, 3, 1)).Object; - var repository = GetRepository(); - - var result = await repository.PageAsync(todoItems, pageSize, 3); - - Assert.Equal(TodoItems(2, 3, 1), result, new IdComparer()); - } - - [Fact] - public async Task Page_When_PageNumber_Is_Zero_Pretends_PageNumber_Is_One() - { - var todoItems = DbSetMock.Create(TodoItems(2, 3, 1)).Object; - var repository = GetRepository(); - - var result = await repository.PageAsync(todoItems, 1, 0); - - Assert.Equal(TodoItems(2), result, new IdComparer()); - } - - [Fact] - public async Task Page_When_PageNumber_Of_PageSize_Does_Not_Exist_Return_Empty_Queryable() - { - var todoItems = DbSetMock.Create(TodoItems(2, 3, 1)).Object; - var repository = GetRepository(); - - var result = await repository.PageAsync(todoItems, 2, 3); - - Assert.Empty(result); - } - - [Theory] - [InlineData(3, 2, new[] { 4, 5, 6 })] - [InlineData(8, 2, new[] { 9 })] - [InlineData(20, 1, new[] { 1, 2, 3, 4, 5, 6, 7, 8, 9 })] - public async Task Page_When_PageNumber_Is_Positive_Returns_PageNumberTh_Page_Of_Size_PageSize(int pageSize, int pageNumber, int[] expectedResult) - { - var todoItems = DbSetMock.Create(TodoItems(1, 2, 3, 4, 5, 6, 7, 8, 9)).Object; - var repository = GetRepository(); - - var result = await repository.PageAsync(todoItems, pageSize, pageNumber); - - Assert.Equal(TodoItems(expectedResult), result, new IdComparer()); - } - - [Theory] - [InlineData(6, -1, new[] { 4, 5, 6, 7, 8, 9 })] - [InlineData(6, -2, new[] { 1, 2, 3 })] - [InlineData(20, -1, new[] { 1, 2, 3, 4, 5, 6, 7, 8, 9 })] - public async Task Page_When_PageNumber_Is_Negative_Returns_PageNumberTh_Page_From_End(int pageSize, int pageNumber, int[] expectedIds) - { - var todoItems = DbSetMock.Create(TodoItems(1, 2, 3, 4, 5, 6, 7, 8, 9)).Object; - var repository = GetRepository(); - - var result = await repository.PageAsync(todoItems, pageSize, pageNumber); - - Assert.Equal(TodoItems(expectedIds), result, new IdComparer()); - } - - private static TodoItem[] TodoItems(params int[] ids) - { - return ids.Select(id => new TodoItem { Id = id }).ToArray(); - } - - private class IdComparer : IEqualityComparer - where T : IIdentifiable - { - public bool Equals(T x, T y) => x?.StringId == y?.StringId; - - public int GetHashCode(T obj) => obj?.StringId?.GetHashCode() ?? 0; + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + [InlineData(-10)] + public async Task Page_When_PageSize_Is_NonPositive_Does_Nothing(int pageSize) + { + var todoItems = DbSetMock.Create(TodoItems(2, 3, 1)).Object; + var repository = GetRepository(); + + var result = await repository.PageAsync(todoItems, pageSize, 3); + + Assert.Equal(TodoItems(2, 3, 1), result, new IdComparer()); + } + + [Fact] + public async Task Page_When_PageNumber_Is_Zero_Pretends_PageNumber_Is_One() + { + var todoItems = DbSetMock.Create(TodoItems(2, 3, 1)).Object; + var repository = GetRepository(); + + var result = await repository.PageAsync(todoItems, 1, 0); + + Assert.Equal(TodoItems(2), result, new IdComparer()); + } + + [Fact] + public async Task Page_When_PageNumber_Of_PageSize_Does_Not_Exist_Return_Empty_Queryable() + { + var todoItems = DbSetMock.Create(TodoItems(2, 3, 1)).Object; + var repository = GetRepository(); + + var result = await repository.PageAsync(todoItems, 2, 3); + + Assert.Empty(result); + } + + [Theory] + [InlineData(3, 2, new[] { 4, 5, 6 })] + [InlineData(8, 2, new[] { 9 })] + [InlineData(20, 1, new[] { 1, 2, 3, 4, 5, 6, 7, 8, 9 })] + public async Task Page_When_PageNumber_Is_Positive_Returns_PageNumberTh_Page_Of_Size_PageSize(int pageSize, int pageNumber, int[] expectedResult) + { + var todoItems = DbSetMock.Create(TodoItems(1, 2, 3, 4, 5, 6, 7, 8, 9)).Object; + var repository = GetRepository(); + + var result = await repository.PageAsync(todoItems, pageSize, pageNumber); + + Assert.Equal(TodoItems(expectedResult), result, new IdComparer()); + } + + [Theory] + [InlineData(6, -1, new[] { 4, 5, 6, 7, 8, 9 })] + [InlineData(6, -2, new[] { 1, 2, 3 })] + [InlineData(20, -1, new[] { 1, 2, 3, 4, 5, 6, 7, 8, 9 })] + public async Task Page_When_PageNumber_Is_Negative_Returns_PageNumberTh_Page_From_End(int pageSize, int pageNumber, int[] expectedIds) + { + var todoItems = DbSetMock.Create(TodoItems(1, 2, 3, 4, 5, 6, 7, 8, 9)).Object; + var repository = GetRepository(); + + var result = await repository.PageAsync(todoItems, pageSize, pageNumber); + + Assert.Equal(TodoItems(expectedIds), result, new IdComparer()); + } + + private static TodoItem[] TodoItems(params int[] ids) + { + return ids.Select(id => new TodoItem { Id = id }).ToArray(); + } + + private class IdComparer : IEqualityComparer + where T : IIdentifiable + { + public bool Equals(T x, T y) => x?.StringId == y?.StringId; + + public int GetHashCode(T obj) => obj?.StringId?.GetHashCode() ?? 0; } } } From d779b0a4ecbef83faecfb599bb04eb48db39e9c2 Mon Sep 17 00:00:00 2001 From: Jared Nance Date: Tue, 2 Oct 2018 21:38:08 -0700 Subject: [PATCH 18/18] fix deserializer tests --- .../Serialization/JsonApiDeSerializer.cs | 9 ++------- .../Acceptance/TodoItemsControllerTests.cs | 2 +- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs b/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs index 20ae99194f..33a3f2e119 100644 --- a/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs +++ b/src/JsonApiDotNetCore/Serialization/JsonApiDeSerializer.cs @@ -283,13 +283,8 @@ private object SetHasManyRelationship(object entity, if (relationships.TryGetValue(relationshipName, out RelationshipData relationshipData)) { - if(relationshipData.IsHasMany == false) { - throw new JsonApiException(400, $"Cannot set HasMany relationship '{attr.PublicRelationshipName}'. Value must be a JSON array of Resource Identifier Objects."); - } - - var data = (List)relationshipData.ExposedData; - - if (data == null) return entity; + if(relationshipData.IsHasMany == false || relationshipData.ManyData == null) + return entity; var relatedResources = relationshipData.ManyData.Select(r => { diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs index ccc9a1e870..c53e81738c 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/TodoItemsControllerTests.cs @@ -301,10 +301,10 @@ public async Task Can_Get_TodoItem_WithOwner() // Act var response = await _fixture.Client.SendAsync(request); var body = await response.Content.ReadAsStringAsync(); - var deserializedBody = (TodoItem)_fixture.GetService().Deserialize(body); // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var deserializedBody = (TodoItem)_fixture.GetService().Deserialize(body); Assert.Equal(person.Id, deserializedBody.OwnerId); Assert.Equal(todoItem.Id, deserializedBody.Id); Assert.Equal(todoItem.Description, deserializedBody.Description);