Skip to content

Commit dfef278

Browse files
spike83csantero
spike83
authored and
csantero
committed
bugfix: includes (eager-loading) must be applied to related not to primary resource! (#122)
1 parent 4374719 commit dfef278

4 files changed

Lines changed: 196 additions & 22 deletions

File tree

‎JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests/FetchingResourcesTests.cs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,23 @@ public async Task Get_related_to_many_included()
116116
}
117117
}
118118

119+
120+
[TestMethod]
121+
[DeploymentItem(@"Data\Comment.csv", @"Data")]
122+
[DeploymentItem(@"Data\Post.csv", @"Data")]
123+
[DeploymentItem(@"Data\PostTagLink.csv", @"Data")]
124+
[DeploymentItem(@"Data\Tag.csv", @"Data")]
125+
[DeploymentItem(@"Data\User.csv", @"Data")]
126+
public async Task Get_related_to_many_included_external()
127+
{
128+
using (var effortConnection = GetEffortConnection())
129+
{
130+
var response = await SubmitGet(effortConnection, "users/401/posts?include=tags");
131+
132+
await AssertResponseContent(response, @"Fixtures\FetchingResources\Get_related_to_many_include_external_response.json", HttpStatusCode.OK);
133+
}
134+
}
135+
119136
[TestMethod]
120137
[DeploymentItem(@"Data\Comment.csv", @"Data")]
121138
[DeploymentItem(@"Data\Post.csv", @"Data")]

‎JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests/Fixtures/FetchingResources/Get_related_to_many_include_external_response.json‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
{
2+
"data": [
3+
{
4+
"type": "posts",
5+
"id": "201",
6+
"attributes": {
7+
"content": "Post 1 content",
8+
"created": "2015-01-31T14:00:00.0000000+00:00",
9+
"title": "Post 1"
10+
},
11+
"relationships": {
12+
"author": {
13+
"links": {
14+
"self": "https://www.example.com/posts/201/relationships/author",
15+
"related": "https://www.example.com/posts/201/author"
16+
}
17+
},
18+
"comments": {
19+
"links": {
20+
"self": "https://www.example.com/posts/201/relationships/comments",
21+
"related": "https://www.example.com/posts/201/comments"
22+
}
23+
},
24+
"tags": {
25+
"links": {
26+
"self": "https://www.example.com/posts/201/relationships/tags",
27+
"related": "https://www.example.com/posts/201/tags"
28+
},
29+
"data": [
30+
{
31+
"type": "tags",
32+
"id": "301"
33+
},
34+
{
35+
"type": "tags",
36+
"id": "302"
37+
}
38+
]
39+
}
40+
}
41+
},
42+
{
43+
"type": "posts",
44+
"id": "202",
45+
"attributes": {
46+
"content": "Post 2 content",
47+
"created": "2015-02-05T08:10:00.0000000+00:00",
48+
"title": "Post 2"
49+
},
50+
"relationships": {
51+
"author": {
52+
"links": {
53+
"self": "https://www.example.com/posts/202/relationships/author",
54+
"related": "https://www.example.com/posts/202/author"
55+
}
56+
},
57+
"comments": {
58+
"links": {
59+
"self": "https://www.example.com/posts/202/relationships/comments",
60+
"related": "https://www.example.com/posts/202/comments"
61+
}
62+
},
63+
"tags": {
64+
"links": {
65+
"self": "https://www.example.com/posts/202/relationships/tags",
66+
"related": "https://www.example.com/posts/202/tags"
67+
},
68+
"data": [
69+
{
70+
"type": "tags",
71+
"id": "302"
72+
},
73+
{
74+
"type": "tags",
75+
"id": "303"
76+
}
77+
]
78+
}
79+
}
80+
},
81+
{
82+
"type": "posts",
83+
"id": "203",
84+
"attributes": {
85+
"content": "Post 3 content",
86+
"created": "2015-02-07T11:11:00.0000000+00:00",
87+
"title": "Post 3"
88+
},
89+
"relationships": {
90+
"author": {
91+
"links": {
92+
"self": "https://www.example.com/posts/203/relationships/author",
93+
"related": "https://www.example.com/posts/203/author"
94+
}
95+
},
96+
"comments": {
97+
"links": {
98+
"self": "https://www.example.com/posts/203/relationships/comments",
99+
"related": "https://www.example.com/posts/203/comments"
100+
}
101+
},
102+
"tags": {
103+
"links": {
104+
"self": "https://www.example.com/posts/203/relationships/tags",
105+
"related": "https://www.example.com/posts/203/tags"
106+
},
107+
"data": [
108+
{
109+
"type": "tags",
110+
"id": "303"
111+
}
112+
]
113+
}
114+
}
115+
}
116+
],
117+
"included": [
118+
{
119+
"type": "tags",
120+
"id": "301",
121+
"attributes": {
122+
"name": "Tag A"
123+
},
124+
"relationships": {
125+
"posts": {
126+
"links": {
127+
"self": "https://www.example.com/tags/301/relationships/posts",
128+
"related": "https://www.example.com/tags/301/posts"
129+
}
130+
}
131+
}
132+
},
133+
{
134+
"type": "tags",
135+
"id": "302",
136+
"attributes": {
137+
"name": "Tag B"
138+
},
139+
"relationships": {
140+
"posts": {
141+
"links": {
142+
"self": "https://www.example.com/tags/302/relationships/posts",
143+
"related": "https://www.example.com/tags/302/posts"
144+
}
145+
}
146+
}
147+
},
148+
{
149+
"type": "tags",
150+
"id": "303",
151+
"attributes": {
152+
"name": "Tag C"
153+
},
154+
"relationships": {
155+
"posts": {
156+
"links": {
157+
"self": "https://www.example.com/tags/303/relationships/posts",
158+
"related": "https://www.example.com/tags/303/posts"
159+
}
160+
}
161+
}
162+
}
163+
]
164+
}

‎JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests/JSONAPI.AcceptanceTests.EntityFrameworkTestWebApp.Tests.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@
280280
<EmbeddedResource Include="Fixtures\Pagination\GetFilterPaged-2-1.json" />
281281
<EmbeddedResource Include="Fixtures\Pagination\GetFilterPaged-1-2-sorted.json" />
282282
<EmbeddedResource Include="Fixtures\Pagination\GetFilterPaged-1-2-sorted-desc.json" />
283+
<EmbeddedResource Include="Fixtures\FetchingResources\Get_related_to_many_include_external_response.json" />
283284
<None Include="packages.config" />
284285
</ItemGroup>
285286
<ItemGroup />

‎JSONAPI.EntityFramework/Http/EntityFrameworkToManyRelatedResourceDocumentMaterializer.cs‎

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -45,62 +45,54 @@ protected override async Task<IQueryable<TRelated>> GetRelatedQuery(string prima
4545
var param = Expression.Parameter(typeof (TPrimaryResource));
4646
var accessorExpr = Expression.Property(param, _relationship.Property);
4747
var lambda = Expression.Lambda<Func<TPrimaryResource, IEnumerable<TRelated>>>(accessorExpr, param);
48-
var primaryEntityQuery = FilterById(primaryResourceId, _primaryTypeRegistration, GetNavigationPropertiesIncludes<TPrimaryResource>(Includes));
48+
49+
var primaryEntityQuery = FilterById<TPrimaryResource>(primaryResourceId, _primaryTypeRegistration);
4950

5051
// We have to see if the resource even exists, so we can throw a 404 if it doesn't
5152
var relatedResource = await primaryEntityQuery.FirstOrDefaultAsync(cancellationToken);
5253
if (relatedResource == null)
5354
throw JsonApiException.CreateForNotFound(string.Format(
5455
"No resource of type `{0}` exists with id `{1}`.",
5556
_primaryTypeRegistration.ResourceTypeName, primaryResourceId));
57+
var includes = GetNavigationPropertiesIncludes(Includes);
58+
var query = primaryEntityQuery.SelectMany(lambda);
5659

57-
return primaryEntityQuery.SelectMany(lambda);
60+
if (includes != null && includes.Any())
61+
query = includes.Aggregate(query, (current, include) => current.Include(include));
62+
return query;
5863
}
5964

6065

6166
/// <summary>
6267
/// This method allows to include <see cref="QueryableExtensions.Include{T}"/> into query.
6368
/// This can reduce the number of queries (eager loading)
6469
/// </summary>
65-
/// <typeparam name="TResource"></typeparam>
6670
/// <param name="includes"></param>
6771
/// <returns></returns>
68-
protected virtual Expression<Func<TResource, object>>[] GetNavigationPropertiesIncludes<TResource>(string[] includes)
72+
protected virtual Expression<Func<TRelated, object>>[] GetNavigationPropertiesIncludes(string[] includes)
6973
{
70-
List<Expression<Func<TResource, object>>> list = new List<Expression<Func<TResource, object>>>();
74+
List<Expression<Func<TRelated, object>>> list = new List<Expression<Func<TRelated, object>>>();
7175
foreach (var include in includes)
7276
{
7377
var incl = include.Pascalize();
74-
var param = Expression.Parameter(typeof(TResource));
78+
var param = Expression.Parameter(typeof(TRelated));
7579
var lambda =
76-
Expression.Lambda<Func<TResource, object>>(
80+
Expression.Lambda<Func<TRelated, object>>(
7781
Expression.PropertyOrField(param, incl), param);
7882
list.Add(lambda);
7983
}
8084
return list.ToArray();
8185
}
8286

8387
private IQueryable<TResource> FilterById<TResource>(string id,
84-
IResourceTypeRegistration resourceTypeRegistration,
85-
params Expression<Func<TResource, object>>[] includes) where TResource : class
88+
IResourceTypeRegistration resourceTypeRegistration) where TResource : class
8689
{
8790
var param = Expression.Parameter(typeof (TResource));
8891
var filterByIdExpression = resourceTypeRegistration.GetFilterByIdExpression(param, id);
8992
var predicate = Expression.Lambda<Func<TResource, bool>>(filterByIdExpression, param);
90-
return QueryIncludeNavigationProperties(predicate, includes);
91-
}
92-
93-
private IQueryable<TResource> QueryIncludeNavigationProperties<TResource>(Expression<Func<TResource, bool>> predicate,
94-
params Expression<Func<TResource, object>>[] includes) where TResource : class
95-
{
9693
IQueryable<TResource> query = _dbContext.Set<TResource>();
97-
if (includes != null && includes.Any())
98-
query = includes.Aggregate(query, (current, include) => current.Include(include));
99-
100-
if (predicate != null)
101-
query = query.Where(predicate);
102-
103-
return query.AsQueryable();
94+
return query.Where(predicate);
10495
}
96+
10597
}
10698
}

0 commit comments

Comments
 (0)