Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 2948f6c

Browse files
author
Bart Koelman
committedMar 17, 2022
Resource inheritance: derived includes
1 parent 00fdc88 commit 2948f6c

File tree

16 files changed

+1135
-137
lines changed

16 files changed

+1135
-137
lines changed
 

‎benchmarks/Serialization/ResourceSerializationBenchmarks.cs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,19 @@ protected override IEvaluatedIncludeCache CreateEvaluatedIncludeCache(IResourceG
127127
RelationshipAttribute multi4 = resourceAType.GetRelationshipByPropertyName(nameof(OutgoingResource.Multi4));
128128
RelationshipAttribute multi5 = resourceAType.GetRelationshipByPropertyName(nameof(OutgoingResource.Multi5));
129129

130-
ImmutableArray<ResourceFieldAttribute> chain = ImmutableArray.Create<ResourceFieldAttribute>(single2, single3, multi4, multi5);
131-
IEnumerable<ResourceFieldChainExpression> chains = new ResourceFieldChainExpression(chain).AsEnumerable();
132-
133-
var converter = new IncludeChainConverter();
134-
IncludeExpression include = converter.FromRelationshipChains(chains);
130+
var include = new IncludeExpression(new HashSet<IncludeElementExpression>
131+
{
132+
new(single2, new HashSet<IncludeElementExpression>
133+
{
134+
new(single3, new HashSet<IncludeElementExpression>
135+
{
136+
new(multi4, new HashSet<IncludeElementExpression>
137+
{
138+
new(multi5)
139+
}.ToImmutableHashSet())
140+
}.ToImmutableHashSet())
141+
}.ToImmutableHashSet())
142+
}.ToImmutableHashSet());
135143

136144
var cache = new EvaluatedIncludeCache();
137145
cache.Set(include);

‎src/JsonApiDotNetCore/Queries/Expressions/IncludeChainConverter.cs

Lines changed: 0 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -42,60 +42,6 @@ public IReadOnlyCollection<ResourceFieldChainExpression> GetRelationshipChains(I
4242
return converter.Chains;
4343
}
4444

45-
/// <summary>
46-
/// Converts a set of relationship chains into a tree of inclusions.
47-
/// </summary>
48-
/// <example>
49-
/// Input chains: <code><![CDATA[
50-
/// Article -> Blog,
51-
/// Article -> Revisions -> Author
52-
/// ]]></code> Output tree:
53-
/// <code><![CDATA[
54-
/// Article
55-
/// {
56-
/// Blog,
57-
/// Revisions
58-
/// {
59-
/// Author
60-
/// }
61-
/// }
62-
/// ]]></code>
63-
/// </example>
64-
public IncludeExpression FromRelationshipChains(IEnumerable<ResourceFieldChainExpression> chains)
65-
{
66-
ArgumentGuard.NotNull(chains, nameof(chains));
67-
68-
IImmutableSet<IncludeElementExpression> elements = ConvertChainsToElements(chains);
69-
return elements.Any() ? new IncludeExpression(elements) : IncludeExpression.Empty;
70-
}
71-
72-
private static IImmutableSet<IncludeElementExpression> ConvertChainsToElements(IEnumerable<ResourceFieldChainExpression> chains)
73-
{
74-
var rootNode = new MutableIncludeNode(null!);
75-
76-
foreach (ResourceFieldChainExpression chain in chains)
77-
{
78-
ConvertChainToElement(chain, rootNode);
79-
}
80-
81-
return rootNode.Children.Values.Select(child => child.ToExpression()).ToImmutableHashSet();
82-
}
83-
84-
private static void ConvertChainToElement(ResourceFieldChainExpression chain, MutableIncludeNode rootNode)
85-
{
86-
MutableIncludeNode currentNode = rootNode;
87-
88-
foreach (RelationshipAttribute relationship in chain.Fields.OfType<RelationshipAttribute>())
89-
{
90-
if (!currentNode.Children.ContainsKey(relationship))
91-
{
92-
currentNode.Children[relationship] = new MutableIncludeNode(relationship);
93-
}
94-
95-
currentNode = currentNode.Children[relationship];
96-
}
97-
}
98-
9945
private sealed class IncludeToChainsConverter : QueryExpressionVisitor<object?, object?>
10046
{
10147
private readonly Stack<RelationshipAttribute> _parentRelationshipStack = new();
@@ -144,22 +90,4 @@ private void FlushChain(IncludeElementExpression expression)
14490
Chains.Add(new ResourceFieldChainExpression(chainBuilder.ToImmutable()));
14591
}
14692
}
147-
148-
private sealed class MutableIncludeNode
149-
{
150-
private readonly RelationshipAttribute _relationship;
151-
152-
public IDictionary<RelationshipAttribute, MutableIncludeNode> Children { get; } = new Dictionary<RelationshipAttribute, MutableIncludeNode>();
153-
154-
public MutableIncludeNode(RelationshipAttribute relationship)
155-
{
156-
_relationship = relationship;
157-
}
158-
159-
public IncludeElementExpression ToExpression()
160-
{
161-
IImmutableSet<IncludeElementExpression> elementChildren = Children.Values.Select(child => child.ToExpression()).ToImmutableHashSet();
162-
return new IncludeElementExpression(_relationship, elementChildren);
163-
}
164-
}
16593
}

‎src/JsonApiDotNetCore/Queries/Internal/Parsing/IncludeParser.cs

Lines changed: 304 additions & 28 deletions
Large diffs are not rendered by default.

‎src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/IncludeClauseBuilder.cs

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,44 +37,54 @@ public Expression ApplyInclude(IncludeExpression include)
3737

3838
public override Expression VisitInclude(IncludeExpression expression, object? argument)
3939
{
40-
Expression source = ApplyEagerLoads(_source, _resourceType.EagerLoads, null);
40+
// De-duplicate chains coming from derived relationships.
41+
HashSet<string> propertyPaths = new();
42+
43+
ApplyEagerLoads(_resourceType.EagerLoads, null, propertyPaths);
4144

4245
foreach (ResourceFieldChainExpression chain in IncludeChainConverter.GetRelationshipChains(expression))
4346
{
44-
source = ProcessRelationshipChain(chain, source);
47+
ProcessRelationshipChain(chain, propertyPaths);
4548
}
4649

47-
return source;
50+
return ToExpression(propertyPaths);
4851
}
4952

50-
private Expression ProcessRelationshipChain(ResourceFieldChainExpression chain, Expression source)
53+
private void ProcessRelationshipChain(ResourceFieldChainExpression chain, ISet<string> outputPropertyPaths)
5154
{
5255
string? path = null;
53-
Expression result = source;
5456

5557
foreach (RelationshipAttribute relationship in chain.Fields.Cast<RelationshipAttribute>())
5658
{
5759
path = path == null ? relationship.Property.Name : $"{path}.{relationship.Property.Name}";
5860

59-
result = ApplyEagerLoads(result, relationship.RightType.EagerLoads, path);
61+
ApplyEagerLoads(relationship.RightType.EagerLoads, path, outputPropertyPaths);
6062
}
6163

62-
return IncludeExtensionMethodCall(result, path!);
64+
outputPropertyPaths.Add(path!);
6365
}
6466

65-
private Expression ApplyEagerLoads(Expression source, IEnumerable<EagerLoadAttribute> eagerLoads, string? pathPrefix)
67+
private void ApplyEagerLoads(IEnumerable<EagerLoadAttribute> eagerLoads, string? pathPrefix, ISet<string> outputPropertyPaths)
6668
{
67-
Expression result = source;
68-
6969
foreach (EagerLoadAttribute eagerLoad in eagerLoads)
7070
{
7171
string path = pathPrefix != null ? $"{pathPrefix}.{eagerLoad.Property.Name}" : eagerLoad.Property.Name;
72-
result = IncludeExtensionMethodCall(result, path);
72+
outputPropertyPaths.Add(path);
73+
74+
ApplyEagerLoads(eagerLoad.Children, path, outputPropertyPaths);
75+
}
76+
}
77+
78+
private Expression ToExpression(HashSet<string> propertyPaths)
79+
{
80+
Expression source = _source;
7381

74-
result = ApplyEagerLoads(result, eagerLoad.Children, path);
82+
foreach (string propertyPath in propertyPaths)
83+
{
84+
source = IncludeExtensionMethodCall(source, propertyPath);
7585
}
7686

77-
return result;
87+
return source;
7888
}
7989

8090
private Expression IncludeExtensionMethodCall(Expression source, string navigationPropertyPath)

‎src/JsonApiDotNetCore/QueryStrings/Internal/IncludeQueryStringParameterReader.cs

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
using JsonApiDotNetCore.Queries;
77
using JsonApiDotNetCore.Queries.Expressions;
88
using JsonApiDotNetCore.Queries.Internal.Parsing;
9-
using JsonApiDotNetCore.Resources.Annotations;
109
using Microsoft.Extensions.Primitives;
1110

1211
namespace JsonApiDotNetCore.QueryStrings.Internal;
@@ -18,7 +17,6 @@ public class IncludeQueryStringParameterReader : QueryStringParameterReader, IIn
1817
private readonly IncludeParser _includeParser;
1918

2019
private IncludeExpression? _includeExpression;
21-
private string? _lastParameterName;
2220

2321
public bool AllowEmptyValue => false;
2422

@@ -28,18 +26,7 @@ public IncludeQueryStringParameterReader(IJsonApiRequest request, IResourceGraph
2826
ArgumentGuard.NotNull(options, nameof(options));
2927

3028
_options = options;
31-
_includeParser = new IncludeParser(ValidateSingleRelationship);
32-
}
33-
34-
protected void ValidateSingleRelationship(RelationshipAttribute relationship, ResourceType resourceType, string path)
35-
{
36-
if (!relationship.CanInclude)
37-
{
38-
throw new InvalidQueryStringParameterException(_lastParameterName!, "Including the requested relationship is not allowed.",
39-
path == relationship.PublicName
40-
? $"Including the relationship '{relationship.PublicName}' on '{resourceType.PublicName}' is not allowed."
41-
: $"Including the relationship '{relationship.PublicName}' in '{path}' on '{resourceType.PublicName}' is not allowed.");
42-
}
29+
_includeParser = new IncludeParser();
4330
}
4431

4532
/// <inheritdoc />
@@ -59,8 +46,6 @@ public virtual bool CanRead(string parameterName)
5946
/// <inheritdoc />
6047
public virtual void Read(string parameterName, StringValues parameterValue)
6148
{
62-
_lastParameterName = parameterName;
63-
6449
try
6550
{
6651
_includeExpression = GetInclude(parameterValue);

‎src/JsonApiDotNetCore/Serialization/Response/ResponseModelAdapter.cs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -274,14 +274,25 @@ private void TraverseRelationships(IIdentifiable leftResource, ResourceObjectTre
274274
private void TraverseRelationship(RelationshipAttribute relationship, IIdentifiable leftResource, ResourceObjectTreeNode leftTreeNode,
275275
IncludeElementExpression includeElement, EndpointKind kind)
276276
{
277-
object? rightValue = relationship.GetValue(leftResource);
277+
if (!relationship.LeftType.ClrType.IsAssignableFrom(leftTreeNode.ResourceType.ClrType))
278+
{
279+
// Skipping over relationship that is declared on another derived type.
280+
return;
281+
}
282+
283+
// In case of resource inheritance, prefer the relationship on derived type over the one on base type.
284+
RelationshipAttribute effectiveRelationship = !leftTreeNode.ResourceType.Equals(relationship.LeftType)
285+
? leftTreeNode.ResourceType.GetRelationshipByPropertyName(relationship.Property.Name)
286+
: relationship;
287+
288+
object? rightValue = effectiveRelationship.GetValue(leftResource);
278289
ICollection<IIdentifiable> rightResources = CollectionConverter.ExtractResources(rightValue);
279290

280-
leftTreeNode.EnsureHasRelationship(relationship);
291+
leftTreeNode.EnsureHasRelationship(effectiveRelationship);
281292

282293
foreach (IIdentifiable rightResource in rightResources)
283294
{
284-
TraverseResource(rightResource, relationship.RightType, kind, includeElement.Children, leftTreeNode, relationship);
295+
TraverseResource(rightResource, effectiveRelationship.RightType, kind, includeElement.Children, leftTreeNode, effectiveRelationship);
285296
}
286297
}
287298

‎test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/BlogPost.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ public sealed class BlogPost : Identifiable<int>
2020
[HasOne]
2121
public WebAccount? Reviewer { get; set; }
2222

23+
[HasMany]
24+
public ISet<Human> Contributors { get; set; } = new HashSet<Human>();
25+
2326
[HasMany]
2427
public ISet<Label> Labels { get; set; } = new HashSet<Label>();
2528

‎test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Human.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
using JetBrains.Annotations;
2+
using JsonApiDotNetCore.Resources;
3+
using JsonApiDotNetCore.Resources.Annotations;
4+
5+
namespace JsonApiDotNetCoreTests.IntegrationTests.QueryStrings;
6+
7+
[UsedImplicitly(ImplicitUseTargetFlags.Members)]
8+
[Resource(ControllerNamespace = "JsonApiDotNetCoreTests.IntegrationTests.QueryStrings")]
9+
public abstract class Human : Identifiable<int>
10+
{
11+
[Attr]
12+
public string Name { get; set; } = null!;
13+
14+
[HasOne]
15+
public Man? Father { get; set; }
16+
17+
[HasOne]
18+
public Woman? Mother { get; set; }
19+
20+
[HasMany]
21+
public ISet<Human> Children { get; set; } = new HashSet<Human>();
22+
}

‎test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Includes/IncludeTests.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -893,6 +893,28 @@ public async Task Cannot_include_relationship_with_blocked_capability()
893893
error.Source.Parameter.Should().Be("include");
894894
}
895895

896+
[Fact]
897+
public async Task Cannot_include_relationship_with_nested_blocked_capability()
898+
{
899+
// Arrange
900+
const string route = "/blogs?include=posts.parent";
901+
902+
// Act
903+
(HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync<Document>(route);
904+
905+
// Assert
906+
httpResponse.Should().HaveStatusCode(HttpStatusCode.BadRequest);
907+
908+
responseDocument.Errors.ShouldHaveCount(1);
909+
910+
ErrorObject error = responseDocument.Errors[0];
911+
error.StatusCode.Should().Be(HttpStatusCode.BadRequest);
912+
error.Title.Should().Be("Including the requested relationship is not allowed.");
913+
error.Detail.Should().Be("Including the relationship 'parent' in 'posts.parent' on 'blogPosts' is not allowed.");
914+
error.Source.ShouldNotBeNull();
915+
error.Source.Parameter.Should().Be("include");
916+
}
917+
896918
[Fact]
897919
public async Task Ignores_null_parent_in_nested_include()
898920
{

‎test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Man.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
using JetBrains.Annotations;
2+
using JsonApiDotNetCore.Resources.Annotations;
3+
4+
namespace JsonApiDotNetCoreTests.IntegrationTests.QueryStrings;
5+
6+
[UsedImplicitly(ImplicitUseTargetFlags.Members)]
7+
[Resource(ControllerNamespace = "JsonApiDotNetCoreTests.IntegrationTests.QueryStrings")]
8+
public sealed class Man : Human
9+
{
10+
[Attr]
11+
public bool HasBeard { get; set; }
12+
13+
[HasOne]
14+
public Woman? Wife { get; set; }
15+
16+
[HasMany]
17+
public ISet<Human> Friends { get; set; } = new HashSet<Human>();
18+
19+
[HasMany]
20+
public ISet<Man> SameGenderFriends { get; set; } = new HashSet<Man>();
21+
}

‎test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/QueryStringDbContext.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ public sealed class QueryStringDbContext : DbContext
1313
public DbSet<Label> Labels => Set<Label>();
1414
public DbSet<Comment> Comments => Set<Comment>();
1515
public DbSet<WebAccount> Accounts => Set<WebAccount>();
16+
public DbSet<Human> Humans => Set<Human>();
17+
public DbSet<Man> Men => Set<Man>();
18+
public DbSet<Woman> Women => Set<Woman>();
1619
public DbSet<AccountPreferences> AccountPreferences => Set<AccountPreferences>();
1720
public DbSet<LoginAttempt> LoginAttempts => Set<LoginAttempt>();
1821
public DbSet<Calendar> Calendars => Set<Calendar>();
@@ -28,5 +31,10 @@ protected override void OnModelCreating(ModelBuilder builder)
2831
builder.Entity<WebAccount>()
2932
.HasMany(webAccount => webAccount.Posts)
3033
.WithOne(blogPost => blogPost.Author!);
34+
35+
builder.Entity<Man>()
36+
.HasOne(man => man.Wife)
37+
.WithOne(woman => woman.Husband)
38+
.HasForeignKey<Man>();
3139
}
3240
}

‎test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/WebAccount.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ public sealed class WebAccount : Identifiable<int>
2323
[Attr]
2424
public string EmailAddress { get; set; } = null!;
2525

26+
[HasOne]
27+
public Human? Person { get; set; }
28+
2629
[HasMany]
2730
public IList<BlogPost> Posts { get; set; } = new List<BlogPost>();
2831

‎test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Woman.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
using JetBrains.Annotations;
2+
using JsonApiDotNetCore.Resources.Annotations;
3+
4+
namespace JsonApiDotNetCoreTests.IntegrationTests.QueryStrings;
5+
6+
[UsedImplicitly(ImplicitUseTargetFlags.Members)]
7+
[Resource(ControllerNamespace = "JsonApiDotNetCoreTests.IntegrationTests.QueryStrings")]
8+
public sealed class Woman : Human
9+
{
10+
[Attr]
11+
public string MaidenName { get; set; } = null!;
12+
13+
[HasOne]
14+
public Man? Husband { get; set; }
15+
16+
[HasMany]
17+
public ISet<Human> Friends { get; set; } = new HashSet<Human>();
18+
19+
[HasMany]
20+
public ISet<Woman> SameGenderFriends { get; set; } = new HashSet<Woman>();
21+
}

‎test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ResourceInheritanceTests.cs

Lines changed: 665 additions & 0 deletions
Large diffs are not rendered by default.

‎test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/BaseParseTests.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ protected BaseParseTests()
2424
.Add<Label, int>()
2525
.Add<Comment, int>()
2626
.Add<WebAccount, int>()
27+
.Add<Human, int>()
28+
.Add<Man, int>()
29+
.Add<Woman, int>()
2730
.Add<AccountPreferences, int>()
2831
.Add<LoginAttempt, int>()
2932
.Build();

‎test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/IncludeParseTests.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ public void Reader_Is_Enabled(JsonApiQueryStringParameters parametersDisabled, b
5959
[InlineData("includes", "title", "Relationship 'title' does not exist on resource type 'blogs'.")]
6060
[InlineData("includes", "posts.comments.publishTime,",
6161
"Relationship 'publishTime' in 'posts.comments.publishTime' does not exist on resource type 'comments'.")]
62+
[InlineData("includes", "owner.person.children.unknown",
63+
"Relationship 'unknown' in 'owner.person.children.unknown' does not exist on resource type 'humans' or any of its derived types.")]
64+
[InlineData("includes", "owner.person.friends.unknown",
65+
"Relationship 'unknown' in 'owner.person.friends.unknown' does not exist on resource type 'humans' or any of its derived types.")]
66+
[InlineData("includes", "owner.person.sameGenderFriends.unknown",
67+
"Relationship 'unknown' in 'owner.person.sameGenderFriends.unknown' does not exist on any of the resource types 'men', 'women'.")]
6268
public void Reader_Read_Fails(string parameterName, string parameterValue, string errorMessage)
6369
{
6470
// Act
@@ -86,6 +92,12 @@ public void Reader_Read_Fails(string parameterName, string parameterValue, strin
8692
[InlineData("includes", "posts.comments", "posts.comments")]
8793
[InlineData("includes", "posts,posts.comments", "posts.comments")]
8894
[InlineData("includes", "posts,posts.labels,posts.comments", "posts.comments,posts.labels")]
95+
[InlineData("includes", "owner.person.children.husband", "owner.person.children.husband")]
96+
[InlineData("includes", "owner.person.wife,owner.person.husband", "owner.person.husband,owner.person.wife")]
97+
[InlineData("includes", "owner.person.father.children.wife", "owner.person.father.children.wife")]
98+
[InlineData("includes", "owner.person.friends", "owner.person.friends,owner.person.friends")]
99+
[InlineData("includes", "owner.person.friends.friends",
100+
"owner.person.friends.friends,owner.person.friends.friends,owner.person.friends.friends,owner.person.friends.friends")]
89101
public void Reader_Read_Succeeds(string parameterName, string parameterValue, string valueExpected)
90102
{
91103
// Act

0 commit comments

Comments
 (0)
Please sign in to comment.