Skip to content

Commit cce393f

Browse files
author
Bart Koelman
committed
Retrieve total resource count on secondary/relationship endpoints using inverse relationship
Bugfix: links.next was not set on full page at relationship endpoint
1 parent aab2fad commit cce393f

24 files changed

+400
-79
lines changed

‎benchmarks/Serialization/ResourceSerializationBenchmarks.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ protected override IEvaluatedIncludeCache CreateEvaluatedIncludeCache(IResourceG
129129
RelationshipAttribute multi4 = resourceAType.GetRelationshipByPropertyName(nameof(OutgoingResource.Multi4));
130130
RelationshipAttribute multi5 = resourceAType.GetRelationshipByPropertyName(nameof(OutgoingResource.Multi5));
131131

132-
ImmutableArray<ResourceFieldAttribute> chain = ArrayFactory.Create<ResourceFieldAttribute>(single2, single3, multi4, multi5).ToImmutableArray();
132+
ImmutableArray<ResourceFieldAttribute> chain = ImmutableArray.Create<ResourceFieldAttribute>(single2, single3, multi4, multi5);
133133
IEnumerable<ResourceFieldChainExpression> chains = new ResourceFieldChainExpression(chain).AsEnumerable();
134134

135135
var converter = new IncludeChainConverter();

‎src/JsonApiDotNetCore/CollectionExtensions.cs

+7
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,13 @@ public static IEnumerable<T> EmptyIfNull<T>(this IEnumerable<T>? source)
7676
return source ?? Enumerable.Empty<T>();
7777
}
7878

79+
public static IEnumerable<T> WhereNotNull<T>(this IEnumerable<T?> source)
80+
{
81+
#pragma warning disable AV1250 // Evaluate LINQ query before returning it
82+
return source.Where(element => element is not null)!;
83+
#pragma warning restore AV1250 // Evaluate LINQ query before returning it
84+
}
85+
7986
public static void AddRange<T>(this ICollection<T> source, IEnumerable<T> itemsToAdd)
8087
{
8188
ArgumentGuard.NotNull(source, nameof(source));

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

+9
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,15 @@ public LogicalExpression(LogicalOperator @operator, IImmutableList<FilterExpress
3434
Terms = terms;
3535
}
3636

37+
public static FilterExpression? Compose(LogicalOperator @operator, params FilterExpression?[] filters)
38+
{
39+
ArgumentGuard.NotNull(filters, nameof(filters));
40+
41+
ImmutableArray<FilterExpression> terms = filters.WhereNotNull().ToImmutableArray();
42+
43+
return terms.Length > 1 ? new LogicalExpression(@operator, terms) : terms.FirstOrDefault();
44+
}
45+
3746
public override TResult Accept<TArgument, TResult>(QueryExpressionVisitor<TArgument, TResult> visitor, TArgument argument)
3847
{
3948
return visitor.VisitLogical(this, argument);

‎src/JsonApiDotNetCore/Queries/IQueryLayerComposer.cs

+7-2
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,14 @@ namespace JsonApiDotNetCore.Queries
1212
public interface IQueryLayerComposer
1313
{
1414
/// <summary>
15-
/// Builds a top-level filter from constraints, used to determine total resource count.
15+
/// Builds a filter from constraints, used to determine total resource count on a primary collection endpoint.
1616
/// </summary>
17-
FilterExpression? GetTopFilterFromConstraints(ResourceType primaryResourceType);
17+
FilterExpression? GetPrimaryFilterFromConstraints(ResourceType primaryResourceType);
18+
19+
/// <summary>
20+
/// Builds a filter from constraints, used to determine total resource count on a secondary collection endpoint.
21+
/// </summary>
22+
FilterExpression? GetSecondaryFilterFromConstraints<TId>(TId primaryId, HasManyAttribute hasManyRelationship);
1823

1924
/// <summary>
2025
/// Collects constraints and builds a <see cref="QueryLayer" /> out of them, used to retrieve the actual resources.

‎src/JsonApiDotNetCore/Queries/Internal/QueryLayerComposer.cs

+73-4
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public QueryLayerComposer(IEnumerable<IQueryConstraintProvider> constraintProvid
4646
}
4747

4848
/// <inheritdoc />
49-
public FilterExpression? GetTopFilterFromConstraints(ResourceType primaryResourceType)
49+
public FilterExpression? GetPrimaryFilterFromConstraints(ResourceType primaryResourceType)
5050
{
5151
ExpressionInScope[] constraints = _constraintProviders.SelectMany(provider => provider.GetConstraints()).ToArray();
5252

@@ -65,6 +65,75 @@ public QueryLayerComposer(IEnumerable<IQueryConstraintProvider> constraintProvid
6565
return GetFilter(filtersInTopScope, primaryResourceType);
6666
}
6767

68+
/// <inheritdoc />
69+
public FilterExpression? GetSecondaryFilterFromConstraints<TId>(TId primaryId, HasManyAttribute hasManyRelationship)
70+
{
71+
ArgumentGuard.NotNull(hasManyRelationship, nameof(hasManyRelationship));
72+
73+
if (hasManyRelationship.InverseNavigationProperty == null)
74+
{
75+
return null;
76+
}
77+
78+
RelationshipAttribute? inverseRelationship =
79+
hasManyRelationship.RightType.FindRelationshipByPropertyName(hasManyRelationship.InverseNavigationProperty.Name);
80+
81+
if (inverseRelationship == null)
82+
{
83+
return null;
84+
}
85+
86+
ExpressionInScope[] constraints = _constraintProviders.SelectMany(provider => provider.GetConstraints()).ToArray();
87+
88+
var secondaryScope = new ResourceFieldChainExpression(hasManyRelationship);
89+
90+
// @formatter:wrap_chained_method_calls chop_always
91+
// @formatter:keep_existing_linebreaks true
92+
93+
FilterExpression[] filtersInSecondaryScope = constraints
94+
.Where(constraint => secondaryScope.Equals(constraint.Scope))
95+
.Select(constraint => constraint.Expression)
96+
.OfType<FilterExpression>()
97+
.ToArray();
98+
99+
// @formatter:keep_existing_linebreaks restore
100+
// @formatter:wrap_chained_method_calls restore
101+
102+
FilterExpression? primaryFilter = GetFilter(Array.Empty<QueryExpression>(), hasManyRelationship.LeftType);
103+
FilterExpression? secondaryFilter = GetFilter(filtersInSecondaryScope, hasManyRelationship.RightType);
104+
105+
FilterExpression inverseFilter = GetInverseRelationshipFilter(primaryId, hasManyRelationship, inverseRelationship);
106+
107+
return LogicalExpression.Compose(LogicalOperator.And, inverseFilter, primaryFilter, secondaryFilter);
108+
}
109+
110+
private static FilterExpression GetInverseRelationshipFilter<TId>(TId primaryId, HasManyAttribute relationship,
111+
RelationshipAttribute inverseRelationship)
112+
{
113+
return inverseRelationship is HasManyAttribute hasManyInverseRelationship
114+
? GetInverseHasManyRelationshipFilter(primaryId, relationship, hasManyInverseRelationship)
115+
: GetInverseHasOneRelationshipFilter(primaryId, relationship, (HasOneAttribute)inverseRelationship);
116+
}
117+
118+
private static FilterExpression GetInverseHasOneRelationshipFilter<TId>(TId primaryId, HasManyAttribute relationship,
119+
HasOneAttribute inverseRelationship)
120+
{
121+
AttrAttribute idAttribute = GetIdAttribute(relationship.LeftType);
122+
var idChain = new ResourceFieldChainExpression(ImmutableArray.Create<ResourceFieldAttribute>(inverseRelationship, idAttribute));
123+
124+
return new ComparisonExpression(ComparisonOperator.Equals, idChain, new LiteralConstantExpression(primaryId!.ToString()!));
125+
}
126+
127+
private static FilterExpression GetInverseHasManyRelationshipFilter<TId>(TId primaryId, HasManyAttribute relationship,
128+
HasManyAttribute inverseRelationship)
129+
{
130+
AttrAttribute idAttribute = GetIdAttribute(relationship.LeftType);
131+
var idChain = new ResourceFieldChainExpression(ImmutableArray.Create<ResourceFieldAttribute>(idAttribute));
132+
var idComparison = new ComparisonExpression(ComparisonOperator.Equals, idChain, new LiteralConstantExpression(primaryId!.ToString()!));
133+
134+
return new HasExpression(new ResourceFieldChainExpression(inverseRelationship), idComparison);
135+
}
136+
68137
/// <inheritdoc />
69138
public QueryLayer ComposeFromConstraints(ResourceType requestResourceType)
70139
{
@@ -309,7 +378,7 @@ private IncludeExpression RewriteIncludeForSecondaryEndpoint(IncludeExpression?
309378
filter = new AnyExpression(idChain, constants);
310379
}
311380

312-
return filter == null ? existingFilter : existingFilter == null ? filter : new LogicalExpression(LogicalOperator.And, filter, existingFilter);
381+
return LogicalExpression.Compose(LogicalOperator.And, filter, existingFilter);
313382
}
314383

315384
/// <inheritdoc />
@@ -419,8 +488,8 @@ protected virtual IImmutableSet<IncludeElementExpression> GetIncludeElements(IIm
419488
ArgumentGuard.NotNull(expressionsInScope, nameof(expressionsInScope));
420489
ArgumentGuard.NotNull(resourceType, nameof(resourceType));
421490

422-
ImmutableArray<FilterExpression> filters = expressionsInScope.OfType<FilterExpression>().ToImmutableArray();
423-
FilterExpression? filter = filters.Length > 1 ? new LogicalExpression(LogicalOperator.And, filters) : filters.FirstOrDefault();
491+
FilterExpression[] filters = expressionsInScope.OfType<FilterExpression>().ToArray();
492+
FilterExpression? filter = LogicalExpression.Compose(LogicalOperator.And, filters);
424493

425494
return _resourceDefinitionAccessor.OnApplyFilter(resourceType, filter);
426495
}

‎src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,11 @@ public virtual async Task<IReadOnlyCollection<TResource>> GetAsync(QueryLayer qu
8585
}
8686

8787
/// <inheritdoc />
88-
public virtual async Task<int> CountAsync(FilterExpression? topFilter, CancellationToken cancellationToken)
88+
public virtual async Task<int> CountAsync(FilterExpression? filter, CancellationToken cancellationToken)
8989
{
9090
_traceWriter.LogMethodStart(new
9191
{
92-
topFilter
92+
filter
9393
});
9494

9595
using (CodeTimingSessionManager.Current.Measure("Repository - Count resources"))
@@ -98,7 +98,7 @@ public virtual async Task<int> CountAsync(FilterExpression? topFilter, Cancellat
9898

9999
var layer = new QueryLayer(resourceType)
100100
{
101-
Filter = topFilter
101+
Filter = filter
102102
};
103103

104104
IQueryable<TResource> query = ApplyQueryLayer(layer);

‎src/JsonApiDotNetCore/Repositories/IResourceReadRepository.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ public interface IResourceReadRepository<TResource, in TId>
2727
Task<IReadOnlyCollection<TResource>> GetAsync(QueryLayer queryLayer, CancellationToken cancellationToken);
2828

2929
/// <summary>
30-
/// Executes a read query using the specified top-level filter and returns the top-level count of matching resources.
30+
/// Executes a read query using the specified filter and returns the count of matching resources.
3131
/// </summary>
32-
Task<int> CountAsync(FilterExpression? topFilter, CancellationToken cancellationToken);
32+
Task<int> CountAsync(FilterExpression? filter, CancellationToken cancellationToken);
3333
}
3434
}

‎src/JsonApiDotNetCore/Repositories/IResourceRepositoryAccessor.cs

+1-2
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@ Task<IReadOnlyCollection<TResource>> GetAsync<TResource>(QueryLayer queryLayer,
2727
/// <summary>
2828
/// Invokes <see cref="IResourceReadRepository{TResource,TId}.CountAsync" /> for the specified resource type.
2929
/// </summary>
30-
Task<int> CountAsync<TResource>(FilterExpression? topFilter, CancellationToken cancellationToken)
31-
where TResource : class, IIdentifiable;
30+
Task<int> CountAsync(ResourceType resourceType, FilterExpression? filter, CancellationToken cancellationToken);
3231

3332
/// <summary>
3433
/// Invokes <see cref="IResourceWriteRepository{TResource,TId}.GetForCreateAsync" />.

‎src/JsonApiDotNetCore/Repositories/ResourceRepositoryAccessor.cs

+3-4
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,10 @@ public async Task<IReadOnlyCollection<IIdentifiable>> GetAsync(ResourceType reso
5050
}
5151

5252
/// <inheritdoc />
53-
public async Task<int> CountAsync<TResource>(FilterExpression? topFilter, CancellationToken cancellationToken)
54-
where TResource : class, IIdentifiable
53+
public async Task<int> CountAsync(ResourceType resourceType, FilterExpression? filter, CancellationToken cancellationToken)
5554
{
56-
dynamic repository = ResolveReadRepository(typeof(TResource));
57-
return (int)await repository.CountAsync(topFilter, cancellationToken);
55+
dynamic repository = ResolveReadRepository(resourceType);
56+
return (int)await repository.CountAsync(filter, cancellationToken);
5857
}
5958

6059
/// <inheritdoc />

‎src/JsonApiDotNetCore/Services/JsonApiResourceService.cs

+37-4
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ public virtual async Task<IReadOnlyCollection<TResource>> GetAsync(CancellationT
6969

7070
if (_options.IncludeTotalResourceCount)
7171
{
72-
FilterExpression? topFilter = _queryLayerComposer.GetTopFilterFromConstraints(_request.PrimaryResourceType);
73-
_paginationContext.TotalResourceCount = await _repositoryAccessor.CountAsync<TResource>(topFilter, cancellationToken);
72+
FilterExpression? topFilter = _queryLayerComposer.GetPrimaryFilterFromConstraints(_request.PrimaryResourceType);
73+
_paginationContext.TotalResourceCount = await _repositoryAccessor.CountAsync(_request.PrimaryResourceType, topFilter, cancellationToken);
7474

7575
if (_paginationContext.TotalResourceCount == 0)
7676
{
@@ -81,7 +81,7 @@ public virtual async Task<IReadOnlyCollection<TResource>> GetAsync(CancellationT
8181
QueryLayer queryLayer = _queryLayerComposer.ComposeFromConstraints(_request.PrimaryResourceType);
8282
IReadOnlyCollection<TResource> resources = await _repositoryAccessor.GetAsync<TResource>(queryLayer, cancellationToken);
8383

84-
if (queryLayer.Pagination?.PageSize != null && queryLayer.Pagination.PageSize.Value == resources.Count)
84+
if (queryLayer.Pagination?.PageSize?.Value == resources.Count)
8585
{
8686
_paginationContext.IsPageFull = true;
8787
}
@@ -116,6 +116,14 @@ public virtual async Task<TResource> GetAsync(TId id, CancellationToken cancella
116116
AssertPrimaryResourceTypeInJsonApiRequestIsNotNull(_request.PrimaryResourceType);
117117
AssertHasRelationship(_request.Relationship, relationshipName);
118118

119+
if (_options.IncludeTotalResourceCount && _request.IsCollection)
120+
{
121+
await RetrieveResourceCountForNonPrimaryEndpointAsync(id, (HasManyAttribute)_request.Relationship, cancellationToken);
122+
123+
// We cannot return early when _paginationContext.TotalResourceCount == 0, because we don't know whether
124+
// the parent resource exists. In case the parent does not exist, an error is produced below.
125+
}
126+
119127
QueryLayer secondaryLayer = _queryLayerComposer.ComposeFromConstraints(_request.SecondaryResourceType!);
120128

121129
QueryLayer primaryLayer =
@@ -152,6 +160,14 @@ public virtual async Task<TResource> GetAsync(TId id, CancellationToken cancella
152160
AssertPrimaryResourceTypeInJsonApiRequestIsNotNull(_request.PrimaryResourceType);
153161
AssertHasRelationship(_request.Relationship, relationshipName);
154162

163+
if (_options.IncludeTotalResourceCount && _request.IsCollection)
164+
{
165+
await RetrieveResourceCountForNonPrimaryEndpointAsync(id, (HasManyAttribute)_request.Relationship, cancellationToken);
166+
167+
// We cannot return early when _paginationContext.TotalResourceCount == 0, because we don't know whether
168+
// the parent resource exists. In case the parent does not exist, an error is produced below.
169+
}
170+
155171
QueryLayer secondaryLayer = _queryLayerComposer.ComposeSecondaryLayerForRelationship(_request.SecondaryResourceType!);
156172

157173
QueryLayer primaryLayer =
@@ -162,7 +178,24 @@ public virtual async Task<TResource> GetAsync(TId id, CancellationToken cancella
162178
TResource? primaryResource = primaryResources.SingleOrDefault();
163179
AssertPrimaryResourceExists(primaryResource);
164180

165-
return _request.Relationship.GetValue(primaryResource);
181+
object? rightValue = _request.Relationship.GetValue(primaryResource);
182+
183+
if (rightValue is ICollection rightResources && secondaryLayer.Pagination?.PageSize?.Value == rightResources.Count)
184+
{
185+
_paginationContext.IsPageFull = true;
186+
}
187+
188+
return rightValue;
189+
}
190+
191+
private async Task RetrieveResourceCountForNonPrimaryEndpointAsync(TId id, HasManyAttribute relationship, CancellationToken cancellationToken)
192+
{
193+
FilterExpression? secondaryFilter = _queryLayerComposer.GetSecondaryFilterFromConstraints(id, relationship);
194+
195+
if (secondaryFilter != null)
196+
{
197+
_paginationContext.TotalResourceCount = await _repositoryAccessor.CountAsync(relationship.RightType, secondaryFilter, cancellationToken);
198+
}
166199
}
167200

168201
/// <inheritdoc />

‎test/JsonApiDotNetCoreTests/IntegrationTests/Archiving/TelevisionBroadcastDefinition.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public TelevisionBroadcastDefinition(IResourceGraph resourceGraph, TelevisionDbC
4848

4949
FilterExpression isUnarchived = new ComparisonExpression(ComparisonOperator.Equals, archivedAtChain, NullConstantExpression.Instance);
5050

51-
return existingFilter == null ? isUnarchived : new LogicalExpression(LogicalOperator.And, existingFilter, isUnarchived);
51+
return LogicalExpression.Compose(LogicalOperator.And, existingFilter, isUnarchived);
5252
}
5353
}
5454

‎test/JsonApiDotNetCoreTests/IntegrationTests/AtomicOperations/Transactions/PerformerRepository.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public Task<IReadOnlyCollection<Performer>> GetAsync(QueryLayer queryLayer, Canc
1818
throw new NotImplementedException();
1919
}
2020

21-
public Task<int> CountAsync(FilterExpression? topFilter, CancellationToken cancellationToken)
21+
public Task<int> CountAsync(FilterExpression? filter, CancellationToken cancellationToken)
2222
{
2323
throw new NotImplementedException();
2424
}

‎test/JsonApiDotNetCoreTests/IntegrationTests/HostingInIIS/HostingTests.cs

+4-4
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
4949
responseDocument.Links.ShouldNotBeNull();
5050
responseDocument.Links.Self.Should().Be($"{HostPrefix}{route}");
5151
responseDocument.Links.Related.Should().BeNull();
52-
responseDocument.Links.First.Should().Be($"{HostPrefix}{route}");
53-
responseDocument.Links.Last.Should().Be($"{HostPrefix}{route}");
52+
responseDocument.Links.First.Should().Be(responseDocument.Links.Self);
53+
responseDocument.Links.Last.Should().Be(responseDocument.Links.Self);
5454
responseDocument.Links.Prev.Should().BeNull();
5555
responseDocument.Links.Next.Should().BeNull();
5656

@@ -117,8 +117,8 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
117117
responseDocument.Links.ShouldNotBeNull();
118118
responseDocument.Links.Self.Should().Be($"{HostPrefix}{route}");
119119
responseDocument.Links.Related.Should().BeNull();
120-
responseDocument.Links.First.Should().Be($"{HostPrefix}{route}");
121-
responseDocument.Links.Last.Should().Be($"{HostPrefix}{route}");
120+
responseDocument.Links.First.Should().Be(responseDocument.Links.Self);
121+
responseDocument.Links.Last.Should().Be(responseDocument.Links.Self);
122122
responseDocument.Links.Prev.Should().BeNull();
123123
responseDocument.Links.Next.Should().BeNull();
124124

0 commit comments

Comments
 (0)