Skip to content

Commit 78af822

Browse files
bkoelmanbkoelman
committed
Changes PaginationExpression.PageSize to be non-nullable. It doesn't make sense to expose a page number without a size.
Note this subtly breaks IResourceDefinition.OnApplyPagination, whose return value used to distinguish between "no pagination" and "give me the default page size from options". The latter is odd, because option defaults have already been applied when entering this method.
1 parent 221c33f commit 78af822

File tree

17 files changed

+74
-75
lines changed

17 files changed

+74
-75
lines changed

‎docs/usage/options.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ options.AllowClientGeneratedIds = true;
2222

2323
## Pagination
2424

25-
The default page size used for all resources can be overridden in options (10 by default). To disable paging, set it to `null`.
25+
The default page size used for all resources can be overridden in options (10 by default). To disable pagination, set it to `null`.
2626
The maximum page size and number allowed from client requests can be set too (unconstrained by default).
2727

2828
You can also include the total number of resources in each response.

‎src/JsonApiDotNetCore.Annotations/Resources/Annotations/LinkTypes.shared.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ public enum LinkTypes
55
{
66
Self = 1 << 0,
77
Related = 1 << 1,
8-
Paging = 1 << 2,
8+
Pagination = 1 << 2,
99
NotConfigured = 1 << 3,
1010
None = 1 << 4,
11-
All = Self | Related | Paging
11+
All = Self | Related | Pagination
1212
}

‎src/JsonApiDotNetCore/Configuration/IJsonApiOptions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ public interface IJsonApiOptions
9999
bool IncludeTotalResourceCount { get; }
100100

101101
/// <summary>
102-
/// The page size (10 by default) that is used when not specified in query string. Set to <c>null</c> to not use paging by default.
102+
/// The page size (10 by default) that is used when not specified in query string. Set to <c>null</c> to not use pagination by default.
103103
/// </summary>
104104
PageSize? DefaultPageSize { get; }
105105

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,12 @@ namespace JsonApiDotNetCore.Queries.Expressions;
1010
public class PaginationExpression : QueryExpression
1111
{
1212
public PageNumber PageNumber { get; }
13-
public PageSize? PageSize { get; }
13+
public PageSize PageSize { get; }
1414

15-
public PaginationExpression(PageNumber pageNumber, PageSize? pageSize)
15+
public PaginationExpression(PageNumber pageNumber, PageSize pageSize)
1616
{
1717
ArgumentGuard.NotNull(pageNumber);
18+
ArgumentGuard.NotNull(pageSize);
1819

1920
PageNumber = pageNumber;
2021
PageSize = pageSize;
@@ -27,7 +28,7 @@ public override TResult Accept<TArgument, TResult>(QueryExpressionVisitor<TArgum
2728

2829
public override string ToString()
2930
{
30-
return PageSize != null ? $"Page number: {PageNumber}, size: {PageSize}" : "(none)";
31+
return $"Page number: {PageNumber}, size: {PageSize}";
3132
}
3233

3334
public override string ToFullString()
@@ -49,7 +50,7 @@ public override bool Equals(object? obj)
4950

5051
var other = (PaginationExpression)obj;
5152

52-
return PageNumber.Equals(other.PageNumber) && Equals(PageSize, other.PageSize);
53+
return PageNumber.Equals(other.PageNumber) && PageSize.Equals(other.PageSize);
5354
}
5455

5556
public override int GetHashCode()

‎src/JsonApiDotNetCore/Queries/IPaginationContext.cs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,19 @@
33
namespace JsonApiDotNetCore.Queries;
44

55
/// <summary>
6-
/// Tracks values used for pagination, which is a combined effort from options, query string parsing and fetching the total number of rows.
6+
/// Tracks values used for pagination, which is a combined effort from options, query string parsing, resource definition callbacks and fetching the
7+
/// total number of rows.
78
/// </summary>
89
public interface IPaginationContext
910
{
1011
/// <summary>
11-
/// The value 1, unless specified from query string. Never null. Cannot be higher than options.MaximumPageNumber.
12+
/// The value 1, unless overridden from query string or resource definition. Should not be higher than <see cref="IJsonApiOptions.MaximumPageNumber" />.
1213
/// </summary>
1314
PageNumber PageNumber { get; set; }
1415

1516
/// <summary>
16-
/// The default page size from options, unless specified in query string. Can be <c>null</c>, which means no paging. Cannot be higher than
17-
/// options.MaximumPageSize.
17+
/// The default page size from options, unless overridden from query string or resource definition. Should not be higher than
18+
/// <see cref="IJsonApiOptions.MaximumPageSize" />. Can be <c>null</c>, which means pagination is disabled.
1819
/// </summary>
1920
PageSize? PageSize { get; set; }
2021

@@ -25,12 +26,12 @@ public interface IPaginationContext
2526
bool IsPageFull { get; set; }
2627

2728
/// <summary>
28-
/// The total number of resources. <c>null</c> when <see cref="IJsonApiOptions.IncludeTotalResourceCount" /> is set to <c>false</c>.
29+
/// The total number of resources, or <c>null</c> when <see cref="IJsonApiOptions.IncludeTotalResourceCount" /> is set to <c>false</c>.
2930
/// </summary>
3031
int? TotalResourceCount { get; set; }
3132

3233
/// <summary>
33-
/// The total number of resource pages. <c>null</c> when <see cref="IJsonApiOptions.IncludeTotalResourceCount" /> is set to <c>false</c> or
34+
/// The total number of resource pages, or <c>null</c> when <see cref="IJsonApiOptions.IncludeTotalResourceCount" /> is set to <c>false</c> or
3435
/// <see cref="PageSize" /> is <c>null</c>.
3536
/// </summary>
3637
int? TotalPageCount { get; }

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

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,13 @@ private QueryLayer ComposeTopLayer(IEnumerable<ExpressionInScope> constraints, R
158158
// @formatter:keep_existing_linebreaks restore
159159
// @formatter:wrap_chained_method_calls restore
160160

161-
PaginationExpression topPagination = GetPagination(expressionsInTopScope, resourceType);
162-
_paginationContext.PageSize = topPagination.PageSize;
163-
_paginationContext.PageNumber = topPagination.PageNumber;
161+
PaginationExpression? topPagination = GetPagination(expressionsInTopScope, resourceType);
162+
163+
if (topPagination != null)
164+
{
165+
_paginationContext.PageSize = topPagination.PageSize;
166+
_paginationContext.PageNumber = topPagination.PageNumber;
167+
}
164168

165169
return new QueryLayer(resourceType)
166170
{
@@ -511,18 +515,13 @@ protected virtual SortExpression GetSort(IReadOnlyCollection<QueryExpression> ex
511515
return sort;
512516
}
513517

514-
protected virtual PaginationExpression GetPagination(IReadOnlyCollection<QueryExpression> expressionsInScope, ResourceType resourceType)
518+
protected virtual PaginationExpression? GetPagination(IReadOnlyCollection<QueryExpression> expressionsInScope, ResourceType resourceType)
515519
{
516520
ArgumentGuard.NotNull(expressionsInScope);
517521
ArgumentGuard.NotNull(resourceType);
518522

519523
PaginationExpression? pagination = expressionsInScope.OfType<PaginationExpression>().FirstOrDefault();
520-
521-
pagination = _resourceDefinitionAccessor.OnApplyPagination(resourceType, pagination);
522-
523-
pagination ??= new PaginationExpression(PageNumber.ValueOne, _options.DefaultPageSize);
524-
525-
return pagination;
524+
return _resourceDefinitionAccessor.OnApplyPagination(resourceType, pagination);
526525
}
527526

528527
#pragma warning disable AV1130 // Return type in method signature should be an interface to an unchangeable collection

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

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,15 @@ public override Expression VisitPagination(PaginationExpression expression, obje
3535
{
3636
Expression skipTakeExpression = _source;
3737

38-
if (expression.PageSize != null)
39-
{
40-
int skipValue = (expression.PageNumber.OneBasedValue - 1) * expression.PageSize.Value;
41-
42-
if (skipValue > 0)
43-
{
44-
skipTakeExpression = ExtensionMethodCall(skipTakeExpression, "Skip", skipValue);
45-
}
38+
int skipValue = (expression.PageNumber.OneBasedValue - 1) * expression.PageSize.Value;
4639

47-
skipTakeExpression = ExtensionMethodCall(skipTakeExpression, "Take", expression.PageSize.Value);
40+
if (skipValue > 0)
41+
{
42+
skipTakeExpression = ExtensionMethodCall(skipTakeExpression, "Skip", skipValue);
4843
}
4944

45+
skipTakeExpression = ExtensionMethodCall(skipTakeExpression, "Take", expression.PageSize.Value);
46+
5047
return skipTakeExpression;
5148
}
5249

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public virtual void Read(string parameterName, StringValues parameterValue)
7373
}
7474
catch (QueryParseException exception)
7575
{
76-
throw new InvalidQueryStringParameterException(parameterName, "The specified paging is invalid.", exception.Message, exception);
76+
throw new InvalidQueryStringParameterException(parameterName, "The specified pagination is invalid.", exception.Message, exception);
7777
}
7878
}
7979

@@ -103,7 +103,6 @@ protected virtual void ValidatePageSize(PaginationQueryStringValueExpression con
103103
}
104104
}
105105

106-
[AssertionMethod]
107106
protected virtual void ValidatePageNumber(PaginationQueryStringValueExpression constraint)
108107
{
109108
if (_options.MaximumPageNumber != null && constraint.Elements.Any(element => element.Value > _options.MaximumPageNumber.OneBasedValue))
@@ -154,11 +153,7 @@ public MutablePaginationEntry ResolveEntryInScope(ResourceFieldChainExpression?
154153
return _globalScope;
155154
}
156155

157-
if (!_nestedScopes.ContainsKey(scope))
158-
{
159-
_nestedScopes.Add(scope, new MutablePaginationEntry());
160-
}
161-
156+
_nestedScopes.TryAdd(scope, new MutablePaginationEntry());
162157
return _nestedScopes[scope];
163158
}
164159

@@ -189,11 +184,17 @@ public IReadOnlyCollection<ExpressionInScope> GetExpressionsInScope()
189184

190185
private IEnumerable<ExpressionInScope> EnumerateExpressionsInScope()
191186
{
192-
yield return new ExpressionInScope(null, new PaginationExpression(_globalScope.PageNumber!, _globalScope.PageSize));
187+
if (_globalScope.PageSize != null)
188+
{
189+
yield return new ExpressionInScope(null, new PaginationExpression(_globalScope.PageNumber!, _globalScope.PageSize));
190+
}
193191

194192
foreach ((ResourceFieldChainExpression scope, MutablePaginationEntry entry) in _nestedScopes)
195193
{
196-
yield return new ExpressionInScope(scope, new PaginationExpression(entry.PageNumber!, entry.PageSize));
194+
if (entry.PageSize != null)
195+
{
196+
yield return new ExpressionInScope(scope, new PaginationExpression(entry.PageNumber!, entry.PageSize));
197+
}
197198
}
198199
}
199200
}

‎src/JsonApiDotNetCore/Resources/IResourceDefinition.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System.Collections.Immutable;
22
using JetBrains.Annotations;
3+
using JsonApiDotNetCore.Configuration;
34
using JsonApiDotNetCore.Middleware;
45
using JsonApiDotNetCore.Queries.Expressions;
56
using JsonApiDotNetCore.Resources.Annotations;
@@ -57,11 +58,10 @@ public interface IResourceDefinition<TResource, in TId>
5758
/// Enables to extend, replace or remove pagination that is being applied on a set of this resource type.
5859
/// </summary>
5960
/// <param name="existingPagination">
60-
/// An optional existing pagination, coming from query string. Can be <c>null</c>.
61+
/// An optional existing pagination, coming from query string and <see cref="IJsonApiOptions" /> defaults. Can be <c>null</c>.
6162
/// </param>
6263
/// <returns>
63-
/// The changed pagination, or <c>null</c> to use the first page with default size from options. To disable paging, set
64-
/// <see cref="PaginationExpression.PageSize" /> to <c>null</c>.
64+
/// The changed pagination, or <c>null</c> to disable pagination.
6565
/// </returns>
6666
PaginationExpression? OnApplyPagination(PaginationExpression? existingPagination);
6767

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ private static string NoAsyncSuffix(string actionName)
8585
links.Related = GetLinkForRelationshipRelated(_request.PrimaryId!, _request.Relationship);
8686
}
8787

88-
if (_request.IsCollection && _paginationContext.PageSize != null && ShouldIncludeTopLevelLink(LinkTypes.Paging, resourceType))
88+
if (_request.IsCollection && _paginationContext.PageSize != null && ShouldIncludeTopLevelLink(LinkTypes.Pagination, resourceType))
8989
{
9090
SetPaginationInTopLevelLinks(resourceType!, links);
9191
}

‎src/JsonApiDotNetCore/Services/JsonApiResourceService.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public virtual async Task<IReadOnlyCollection<TResource>> GetAsync(CancellationT
7676
QueryLayer queryLayer = _queryLayerComposer.ComposeFromConstraints(_request.PrimaryResourceType);
7777
IReadOnlyCollection<TResource> resources = await _repositoryAccessor.GetAsync<TResource>(queryLayer, cancellationToken);
7878

79-
if (queryLayer.Pagination?.PageSize?.Value == resources.Count)
79+
if (queryLayer.Pagination?.PageSize.Value == resources.Count)
8080
{
8181
_paginationContext.IsPageFull = true;
8282
}
@@ -128,7 +128,7 @@ public virtual async Task<TResource> GetAsync(TId id, CancellationToken cancella
128128

129129
object? rightValue = _request.Relationship.GetValue(primaryResource);
130130

131-
if (rightValue is ICollection rightResources && secondaryLayer.Pagination?.PageSize?.Value == rightResources.Count)
131+
if (rightValue is ICollection rightResources && secondaryLayer.Pagination?.PageSize.Value == rightResources.Count)
132132
{
133133
_paginationContext.IsPageFull = true;
134134
}
@@ -169,7 +169,7 @@ public virtual async Task<TResource> GetAsync(TId id, CancellationToken cancella
169169

170170
object? rightValue = _request.Relationship.GetValue(primaryResource);
171171

172-
if (rightValue is ICollection rightResources && secondaryLayer.Pagination?.PageSize?.Value == rightResources.Count)
172+
if (rightValue is ICollection rightResources && secondaryLayer.Pagination?.PageSize.Value == rightResources.Count)
173173
{
174174
_paginationContext.IsPageFull = true;
175175
}

‎test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Pagination/PaginationWithTotalCountTests.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
8888

8989
ErrorObject error = responseDocument.Errors[0];
9090
error.StatusCode.Should().Be(HttpStatusCode.BadRequest);
91-
error.Title.Should().Be("The specified paging is invalid.");
91+
error.Title.Should().Be("The specified pagination is invalid.");
9292
error.Detail.Should().Be("This query string parameter can only be used on a collection of resources (not on a single resource).");
9393
error.Source.ShouldNotBeNull();
9494
error.Source.Parameter.Should().Be("page[number]");
@@ -185,7 +185,7 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
185185

186186
ErrorObject error = responseDocument.Errors[0];
187187
error.StatusCode.Should().Be(HttpStatusCode.BadRequest);
188-
error.Title.Should().Be("The specified paging is invalid.");
188+
error.Title.Should().Be("The specified pagination is invalid.");
189189
error.Detail.Should().Be("This query string parameter can only be used on a collection of resources (not on a single resource).");
190190
error.Source.ShouldNotBeNull();
191191
error.Source.Parameter.Should().Be("page[size]");
@@ -463,7 +463,7 @@ public async Task Cannot_paginate_in_unknown_scope()
463463

464464
ErrorObject error = responseDocument.Errors[0];
465465
error.StatusCode.Should().Be(HttpStatusCode.BadRequest);
466-
error.Title.Should().Be("The specified paging is invalid.");
466+
error.Title.Should().Be("The specified pagination is invalid.");
467467
error.Detail.Should().Be($"Relationship '{Unknown.Relationship}' does not exist on resource type 'webAccounts'.");
468468
error.Source.ShouldNotBeNull();
469469
error.Source.Parameter.Should().Be("page[number]");
@@ -485,7 +485,7 @@ public async Task Cannot_paginate_in_unknown_nested_scope()
485485

486486
ErrorObject error = responseDocument.Errors[0];
487487
error.StatusCode.Should().Be(HttpStatusCode.BadRequest);
488-
error.Title.Should().Be("The specified paging is invalid.");
488+
error.Title.Should().Be("The specified pagination is invalid.");
489489
error.Detail.Should().Be($"Relationship '{Unknown.Relationship}' in 'posts.{Unknown.Relationship}' does not exist on resource type 'blogPosts'.");
490490
error.Source.ShouldNotBeNull();
491491
error.Source.Parameter.Should().Be("page[size]");
@@ -528,7 +528,7 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
528528
}
529529

530530
[Fact]
531-
public async Task Returns_all_resources_when_paging_is_disabled()
531+
public async Task Returns_all_resources_when_pagination_is_disabled()
532532
{
533533
// Arrange
534534
var options = (JsonApiOptions)_testContext.Factory.Services.GetRequiredService<IJsonApiOptions>();

‎test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Pagination/RangeValidationTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public async Task Cannot_use_negative_page_number()
4242

4343
ErrorObject error = responseDocument.Errors[0];
4444
error.StatusCode.Should().Be(HttpStatusCode.BadRequest);
45-
error.Title.Should().Be("The specified paging is invalid.");
45+
error.Title.Should().Be("The specified pagination is invalid.");
4646
error.Detail.Should().Be("Page number cannot be negative or zero.");
4747
error.Source.ShouldNotBeNull();
4848
error.Source.Parameter.Should().Be("page[number]");
@@ -64,7 +64,7 @@ public async Task Cannot_use_zero_page_number()
6464

6565
ErrorObject error = responseDocument.Errors[0];
6666
error.StatusCode.Should().Be(HttpStatusCode.BadRequest);
67-
error.Title.Should().Be("The specified paging is invalid.");
67+
error.Title.Should().Be("The specified pagination is invalid.");
6868
error.Detail.Should().Be("Page number cannot be negative or zero.");
6969
error.Source.ShouldNotBeNull();
7070
error.Source.Parameter.Should().Be("page[number]");
@@ -123,7 +123,7 @@ public async Task Cannot_use_negative_page_size()
123123

124124
ErrorObject error = responseDocument.Errors[0];
125125
error.StatusCode.Should().Be(HttpStatusCode.BadRequest);
126-
error.Title.Should().Be("The specified paging is invalid.");
126+
error.Title.Should().Be("The specified pagination is invalid.");
127127
error.Detail.Should().Be("Page size cannot be negative.");
128128
error.Source.ShouldNotBeNull();
129129
error.Source.Parameter.Should().Be("page[size]");

‎test/JsonApiDotNetCoreTests/IntegrationTests/QueryStrings/Pagination/RangeValidationWithMaximumTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public async Task Cannot_use_page_number_over_maximum()
7171

7272
ErrorObject error = responseDocument.Errors[0];
7373
error.StatusCode.Should().Be(HttpStatusCode.BadRequest);
74-
error.Title.Should().Be("The specified paging is invalid.");
74+
error.Title.Should().Be("The specified pagination is invalid.");
7575
error.Detail.Should().Be($"Page number cannot be higher than {MaximumPageNumber}.");
7676
error.Source.ShouldNotBeNull();
7777
error.Source.Parameter.Should().Be("page[number]");
@@ -93,7 +93,7 @@ public async Task Cannot_use_zero_page_size()
9393

9494
ErrorObject error = responseDocument.Errors[0];
9595
error.StatusCode.Should().Be(HttpStatusCode.BadRequest);
96-
error.Title.Should().Be("The specified paging is invalid.");
96+
error.Title.Should().Be("The specified pagination is invalid.");
9797
error.Detail.Should().Be("Page size cannot be unconstrained.");
9898
error.Source.ShouldNotBeNull();
9999
error.Source.Parameter.Should().Be("page[size]");
@@ -144,7 +144,7 @@ public async Task Cannot_use_page_size_over_maximum()
144144

145145
ErrorObject error = responseDocument.Errors[0];
146146
error.StatusCode.Should().Be(HttpStatusCode.BadRequest);
147-
error.Title.Should().Be("The specified paging is invalid.");
147+
error.Title.Should().Be("The specified pagination is invalid.");
148148
error.Detail.Should().Be($"Page size cannot be higher than {MaximumPageSize}.");
149149
error.Source.ShouldNotBeNull();
150150
error.Source.Parameter.Should().Be("page[size]");

‎test/JsonApiDotNetCoreTests/IntegrationTests/ResourceDefinitions/Reading/StarDefinition.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public override PaginationExpression OnApplyPagination(PaginationExpression? exi
4141

4242
if (existingPagination != null)
4343
{
44-
PageSize pageSize = existingPagination.PageSize?.Value <= maxPageSize.Value ? existingPagination.PageSize : maxPageSize;
44+
PageSize pageSize = existingPagination.PageSize.Value <= maxPageSize.Value ? existingPagination.PageSize : maxPageSize;
4545
return new PaginationExpression(existingPagination.PageNumber, pageSize);
4646
}
4747

0 commit comments

Comments
 (0)