From b284d23e562a50c02715712d8122a4c84fdbef92 Mon Sep 17 00:00:00 2001 From: Simon Hofer Date: Wed, 14 Sep 2016 10:26:29 +0200 Subject: [PATCH] bugfix: includes (eager-loading) must be applied to related not to primary resource! --- .../FetchingResourcesTests.cs | 17 ++ ...ted_to_many_include_external_response.json | 164 ++++++++++++++++++ ...sts.EntityFrameworkTestWebApp.Tests.csproj | 1 + ...ManyRelatedResourceDocumentMaterializer.cs | 36 ++-- 4 files changed, 196 insertions(+), 22 deletions(-) create mode 100644 JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests/Fixtures/FetchingResources/Get_related_to_many_include_external_response.json diff --git a/JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests/FetchingResourcesTests.cs b/JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests/FetchingResourcesTests.cs index 18e83042..39eb2938 100644 --- a/JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests/FetchingResourcesTests.cs +++ b/JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests/FetchingResourcesTests.cs @@ -116,6 +116,23 @@ public async Task Get_related_to_many_included() } } + + [TestMethod] + [DeploymentItem(@"Data\Comment.csv", @"Data")] + [DeploymentItem(@"Data\Post.csv", @"Data")] + [DeploymentItem(@"Data\PostTagLink.csv", @"Data")] + [DeploymentItem(@"Data\Tag.csv", @"Data")] + [DeploymentItem(@"Data\User.csv", @"Data")] + public async Task Get_related_to_many_included_external() + { + using (var effortConnection = GetEffortConnection()) + { + var response = await SubmitGet(effortConnection, "users/401/posts?include=tags"); + + await AssertResponseContent(response, @"Fixtures\FetchingResources\Get_related_to_many_include_external_response.json", HttpStatusCode.OK); + } + } + [TestMethod] [DeploymentItem(@"Data\Comment.csv", @"Data")] [DeploymentItem(@"Data\Post.csv", @"Data")] diff --git a/JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests/Fixtures/FetchingResources/Get_related_to_many_include_external_response.json b/JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests/Fixtures/FetchingResources/Get_related_to_many_include_external_response.json new file mode 100644 index 00000000..e8008645 --- /dev/null +++ b/JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests/Fixtures/FetchingResources/Get_related_to_many_include_external_response.json @@ -0,0 +1,164 @@ +{ + "data": [ + { + "type": "posts", + "id": "201", + "attributes": { + "content": "Post 1 content", + "created": "2015-01-31T14:00:00.0000000+00:00", + "title": "Post 1" + }, + "relationships": { + "author": { + "links": { + "self": "https://www.example.com/posts/201/relationships/author", + "related": "https://www.example.com/posts/201/author" + } + }, + "comments": { + "links": { + "self": "https://www.example.com/posts/201/relationships/comments", + "related": "https://www.example.com/posts/201/comments" + } + }, + "tags": { + "links": { + "self": "https://www.example.com/posts/201/relationships/tags", + "related": "https://www.example.com/posts/201/tags" + }, + "data": [ + { + "type": "tags", + "id": "301" + }, + { + "type": "tags", + "id": "302" + } + ] + } + } + }, + { + "type": "posts", + "id": "202", + "attributes": { + "content": "Post 2 content", + "created": "2015-02-05T08:10:00.0000000+00:00", + "title": "Post 2" + }, + "relationships": { + "author": { + "links": { + "self": "https://www.example.com/posts/202/relationships/author", + "related": "https://www.example.com/posts/202/author" + } + }, + "comments": { + "links": { + "self": "https://www.example.com/posts/202/relationships/comments", + "related": "https://www.example.com/posts/202/comments" + } + }, + "tags": { + "links": { + "self": "https://www.example.com/posts/202/relationships/tags", + "related": "https://www.example.com/posts/202/tags" + }, + "data": [ + { + "type": "tags", + "id": "302" + }, + { + "type": "tags", + "id": "303" + } + ] + } + } + }, + { + "type": "posts", + "id": "203", + "attributes": { + "content": "Post 3 content", + "created": "2015-02-07T11:11:00.0000000+00:00", + "title": "Post 3" + }, + "relationships": { + "author": { + "links": { + "self": "https://www.example.com/posts/203/relationships/author", + "related": "https://www.example.com/posts/203/author" + } + }, + "comments": { + "links": { + "self": "https://www.example.com/posts/203/relationships/comments", + "related": "https://www.example.com/posts/203/comments" + } + }, + "tags": { + "links": { + "self": "https://www.example.com/posts/203/relationships/tags", + "related": "https://www.example.com/posts/203/tags" + }, + "data": [ + { + "type": "tags", + "id": "303" + } + ] + } + } + } + ], + "included": [ + { + "type": "tags", + "id": "301", + "attributes": { + "name": "Tag A" + }, + "relationships": { + "posts": { + "links": { + "self": "https://www.example.com/tags/301/relationships/posts", + "related": "https://www.example.com/tags/301/posts" + } + } + } + }, + { + "type": "tags", + "id": "302", + "attributes": { + "name": "Tag B" + }, + "relationships": { + "posts": { + "links": { + "self": "https://www.example.com/tags/302/relationships/posts", + "related": "https://www.example.com/tags/302/posts" + } + } + } + }, + { + "type": "tags", + "id": "303", + "attributes": { + "name": "Tag C" + }, + "relationships": { + "posts": { + "links": { + "self": "https://www.example.com/tags/303/relationships/posts", + "related": "https://www.example.com/tags/303/posts" + } + } + } + } + ] +} \ No newline at end of file diff --git a/JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests/JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests.csproj b/JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests/JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests.csproj index e9cb8a2c..35d82430 100644 --- a/JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests/JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests.csproj +++ b/JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests/JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests.csproj @@ -280,6 +280,7 @@ + diff --git a/JSONAPI.EntityFramework/Http/EntityFrameworkToManyRelatedResourceDocumentMaterializer.cs b/JSONAPI.EntityFramework/Http/EntityFrameworkToManyRelatedResourceDocumentMaterializer.cs index 50004e16..142075a0 100644 --- a/JSONAPI.EntityFramework/Http/EntityFrameworkToManyRelatedResourceDocumentMaterializer.cs +++ b/JSONAPI.EntityFramework/Http/EntityFrameworkToManyRelatedResourceDocumentMaterializer.cs @@ -45,7 +45,8 @@ protected override async Task> GetRelatedQuery(string prima var param = Expression.Parameter(typeof (TPrimaryResource)); var accessorExpr = Expression.Property(param, _relationship.Property); var lambda = Expression.Lambda>>(accessorExpr, param); - var primaryEntityQuery = FilterById(primaryResourceId, _primaryTypeRegistration, GetNavigationPropertiesIncludes(Includes)); + + var primaryEntityQuery = FilterById(primaryResourceId, _primaryTypeRegistration); // We have to see if the resource even exists, so we can throw a 404 if it doesn't var relatedResource = await primaryEntityQuery.FirstOrDefaultAsync(cancellationToken); @@ -53,8 +54,12 @@ protected override async Task> GetRelatedQuery(string prima throw JsonApiException.CreateForNotFound(string.Format( "No resource of type `{0}` exists with id `{1}`.", _primaryTypeRegistration.ResourceTypeName, primaryResourceId)); + var includes = GetNavigationPropertiesIncludes(Includes); + var query = primaryEntityQuery.SelectMany(lambda); - return primaryEntityQuery.SelectMany(lambda); + if (includes != null && includes.Any()) + query = includes.Aggregate(query, (current, include) => current.Include(include)); + return query; } @@ -62,18 +67,17 @@ protected override async Task> GetRelatedQuery(string prima /// This method allows to include into query. /// This can reduce the number of queries (eager loading) /// - /// /// /// - protected virtual Expression>[] GetNavigationPropertiesIncludes(string[] includes) + protected virtual Expression>[] GetNavigationPropertiesIncludes(string[] includes) { - List>> list = new List>>(); + List>> list = new List>>(); foreach (var include in includes) { var incl = include.Pascalize(); - var param = Expression.Parameter(typeof(TResource)); + var param = Expression.Parameter(typeof(TRelated)); var lambda = - Expression.Lambda>( + Expression.Lambda>( Expression.PropertyOrField(param, incl), param); list.Add(lambda); } @@ -81,26 +85,14 @@ protected virtual Expression>[] GetNavigationPropertiesI } private IQueryable FilterById(string id, - IResourceTypeRegistration resourceTypeRegistration, - params Expression>[] includes) where TResource : class + IResourceTypeRegistration resourceTypeRegistration) where TResource : class { var param = Expression.Parameter(typeof (TResource)); var filterByIdExpression = resourceTypeRegistration.GetFilterByIdExpression(param, id); var predicate = Expression.Lambda>(filterByIdExpression, param); - return QueryIncludeNavigationProperties(predicate, includes); - } - - private IQueryable QueryIncludeNavigationProperties(Expression> predicate, - params Expression>[] includes) where TResource : class - { IQueryable query = _dbContext.Set(); - if (includes != null && includes.Any()) - query = includes.Aggregate(query, (current, include) => current.Include(include)); - - if (predicate != null) - query = query.Where(predicate); - - return query.AsQueryable(); + return query.Where(predicate); } + } }