From c4c99089bd8a3b5f5e4c99cedba67918cabd9e31 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Fri, 25 Sep 2020 10:12:21 +0200 Subject: [PATCH 1/2] Adds support for best-effort paging links (in case the total number of resources is unknown). This strategy is also applied on all secondary resources, as it is quite complex to determine total resource count. Fixed: exception on pass-through query string parameter in secondary request Optimization: skip query execution for primary resource if total count is zero Fixed: self link was calculated twice (in different ways) when paging active Fixed: nested page size was lost when rendering paging links Fixed: invalid paging links on secondary endpoints Fixed: cases where self link was different from the actual URL being requested (we now leave page number/size in, even when they match the default) --- .../Queries/IPaginationContext.cs | 6 + .../Queries/PaginationContext.cs | 3 + ...ourceDefinitionQueryableParameterReader.cs | 8 +- .../Serialization/Building/LinkBuilder.cs | 120 +++++++--- .../Services/JsonApiResourceService.cs | 29 ++- .../Acceptance/Spec/PaginationLinkTests.cs | 12 +- ...ts.cs => PaginationWithTotalCountTests.cs} | 59 ++++- .../PaginationWithoutTotalCountTests.cs | 209 ++++++++++++++++++ ...nRangeTests.cs => RangeValidationTests.cs} | 4 +- ....cs => RangeValidationWithMaximumTests.cs} | 4 +- test/UnitTests/Builders/LinkBuilderTests.cs | 15 +- 11 files changed, 414 insertions(+), 55 deletions(-) rename test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/{PaginationTests.cs => PaginationWithTotalCountTests.cs} (82%) create mode 100644 test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationWithoutTotalCountTests.cs rename test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/{PaginationRangeTests.cs => RangeValidationTests.cs} (97%) rename test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/{PaginationRangeWithMaximumTests.cs => RangeValidationWithMaximumTests.cs} (97%) diff --git a/src/JsonApiDotNetCore/Queries/IPaginationContext.cs b/src/JsonApiDotNetCore/Queries/IPaginationContext.cs index 9db3a24c9f..e80199de7c 100644 --- a/src/JsonApiDotNetCore/Queries/IPaginationContext.cs +++ b/src/JsonApiDotNetCore/Queries/IPaginationContext.cs @@ -19,6 +19,12 @@ public interface IPaginationContext /// PageSize PageSize { get; set; } + /// + /// Indicates whether the number of resources on the current page equals the page size. + /// When true, a subsequent page might exist (assuming is unknown). + /// + bool IsPageFull { get; set; } + /// /// The total number of resources. /// null when is set to false. diff --git a/src/JsonApiDotNetCore/Queries/PaginationContext.cs b/src/JsonApiDotNetCore/Queries/PaginationContext.cs index fbdd8ad453..0ca1e25076 100644 --- a/src/JsonApiDotNetCore/Queries/PaginationContext.cs +++ b/src/JsonApiDotNetCore/Queries/PaginationContext.cs @@ -12,6 +12,9 @@ internal sealed class PaginationContext : IPaginationContext /// public PageSize PageSize { get; set; } + /// + public bool IsPageFull { get; set; } + /// public int? TotalResourceCount { get; set; } diff --git a/src/JsonApiDotNetCore/QueryStrings/Internal/ResourceDefinitionQueryableParameterReader.cs b/src/JsonApiDotNetCore/QueryStrings/Internal/ResourceDefinitionQueryableParameterReader.cs index 310f4cd87f..a9ef383e49 100644 --- a/src/JsonApiDotNetCore/QueryStrings/Internal/ResourceDefinitionQueryableParameterReader.cs +++ b/src/JsonApiDotNetCore/QueryStrings/Internal/ResourceDefinitionQueryableParameterReader.cs @@ -46,15 +46,17 @@ public virtual void Read(string parameterName, StringValues parameterValue) private object GetQueryableHandler(string parameterName) { - if (_request.Kind != EndpointKind.Primary) + var resourceType = _request.PrimaryResource.ResourceType; + var handler = _resourceDefinitionAccessor.GetQueryableHandlerForQueryStringParameter(resourceType, parameterName); + + if (handler != null && _request.Kind != EndpointKind.Primary) { throw new InvalidQueryStringParameterException(parameterName, "Custom query string parameters cannot be used on nested resource endpoints.", $"Query string parameter '{parameterName}' cannot be used on a nested resource endpoint."); } - var resourceType = _request.PrimaryResource.ResourceType; - return _resourceDefinitionAccessor.GetQueryableHandlerForQueryStringParameter(resourceType, parameterName); + return handler; } /// diff --git a/src/JsonApiDotNetCore/Serialization/Building/LinkBuilder.cs b/src/JsonApiDotNetCore/Serialization/Building/LinkBuilder.cs index 802bfe0fc1..4e0cb1011f 100644 --- a/src/JsonApiDotNetCore/Serialization/Building/LinkBuilder.cs +++ b/src/JsonApiDotNetCore/Serialization/Building/LinkBuilder.cs @@ -5,6 +5,8 @@ using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Middleware; using JsonApiDotNetCore.Queries; +using JsonApiDotNetCore.Queries.Expressions; +using JsonApiDotNetCore.Queries.Internal.Parsing; using JsonApiDotNetCore.QueryStrings; using JsonApiDotNetCore.Resources; using JsonApiDotNetCore.Resources.Annotations; @@ -15,6 +17,9 @@ namespace JsonApiDotNetCore.Serialization.Building { public class LinkBuilder : ILinkBuilder { + private const string _pageSizeParameterName = "page[size]"; + private const string _pageNumberParameterName = "page[number]"; + private readonly IResourceContextProvider _provider; private readonly IRequestQueryStringAccessor _queryStringAccessor; private readonly IJsonApiOptions _options; @@ -42,10 +47,10 @@ public TopLevelLinks GetTopLevelLinks() TopLevelLinks topLevelLinks = null; if (ShouldAddTopLevelLink(resourceContext, LinkTypes.Self)) { - topLevelLinks = new TopLevelLinks { Self = GetSelfTopLevelLink(resourceContext) }; + topLevelLinks = new TopLevelLinks {Self = GetSelfTopLevelLink(resourceContext, null)}; } - if (ShouldAddTopLevelLink(resourceContext, LinkTypes.Paging) && _paginationContext.PageSize != null) + if (ShouldAddTopLevelLink(resourceContext, LinkTypes.Paging) && _paginationContext.PageSize != null && _request.IsCollection) { SetPageLinks(resourceContext, topLevelLinks ??= new TopLevelLinks()); } @@ -70,36 +75,38 @@ private bool ShouldAddTopLevelLink(ResourceContext resourceContext, LinkTypes li private void SetPageLinks(ResourceContext resourceContext, TopLevelLinks links) { - if (_paginationContext.PageNumber.OneBasedValue > 1) + links.First = GetPageLink(resourceContext, 1, _paginationContext.PageSize); + + if (_paginationContext.TotalPageCount > 0) { - links.Prev = GetPageLink(resourceContext, _paginationContext.PageNumber.OneBasedValue - 1, _paginationContext.PageSize); + links.Last = GetPageLink(resourceContext, _paginationContext.TotalPageCount.Value, _paginationContext.PageSize); } - if (_paginationContext.PageNumber.OneBasedValue < _paginationContext.TotalPageCount) + if (_paginationContext.PageNumber.OneBasedValue > 1) { - links.Next = GetPageLink(resourceContext, _paginationContext.PageNumber.OneBasedValue + 1, _paginationContext.PageSize); + links.Prev = GetPageLink(resourceContext, _paginationContext.PageNumber.OneBasedValue - 1, _paginationContext.PageSize); } - if (_paginationContext.TotalPageCount > 0) + bool hasNextPage = _paginationContext.PageNumber.OneBasedValue < _paginationContext.TotalPageCount; + bool possiblyHasNextPage = _paginationContext.TotalPageCount == null && _paginationContext.IsPageFull; + + if (hasNextPage || possiblyHasNextPage) { - links.Self = GetPageLink(resourceContext, _paginationContext.PageNumber.OneBasedValue, _paginationContext.PageSize); - links.First = GetPageLink(resourceContext, 1, _paginationContext.PageSize); - links.Last = GetPageLink(resourceContext, _paginationContext.TotalPageCount.Value, _paginationContext.PageSize); + links.Next = GetPageLink(resourceContext, _paginationContext.PageNumber.OneBasedValue + 1, _paginationContext.PageSize); } } - private string GetSelfTopLevelLink(ResourceContext resourceContext) + private string GetSelfTopLevelLink(ResourceContext resourceContext, Action> queryStringUpdateAction) { var builder = new StringBuilder(); builder.Append(_request.BasePath); builder.Append("/"); builder.Append(resourceContext.PublicName); - string resourceId = _request.PrimaryId; - if (resourceId != null) + if (_request.PrimaryId != null) { builder.Append("/"); - builder.Append(resourceId); + builder.Append(_request.PrimaryId); } if (_request.Relationship != null) @@ -108,49 +115,102 @@ private string GetSelfTopLevelLink(ResourceContext resourceContext) builder.Append(_request.Relationship.PublicName); } - builder.Append(DecodeSpecialCharacters(_queryStringAccessor.QueryString.Value)); + string queryString = BuildQueryString(queryStringUpdateAction); + builder.Append(queryString); return builder.ToString(); } + private string BuildQueryString(Action> updateAction) + { + var parameters = _queryStringAccessor.Query.ToDictionary(pair => pair.Key, pair => pair.Value.ToString()); + updateAction?.Invoke(parameters); + string queryString = QueryString.Create(parameters).Value; + + return DecodeSpecialCharacters(queryString); + } + + private static string DecodeSpecialCharacters(string uri) + { + return uri.Replace("%5B", "[").Replace("%5D", "]").Replace("%27", "'").Replace("%3A", ":"); + } + private string GetPageLink(ResourceContext resourceContext, int pageOffset, PageSize pageSize) { - string queryString = BuildQueryString(parameters => + return GetSelfTopLevelLink(resourceContext, parameters => { - if (pageSize == null || pageSize.Equals(_options.DefaultPageSize)) + var existingPageSizeParameterValue = parameters.ContainsKey(_pageSizeParameterName) + ? parameters[_pageSizeParameterName] + : null; + + PageSize newTopPageSize = Equals(pageSize, _options.DefaultPageSize) ? null : pageSize; + + string newPageSizeParameterValue = ChangeTopPageSize(existingPageSizeParameterValue, newTopPageSize); + if (newPageSizeParameterValue == null) { - parameters.Remove("page[size]"); + parameters.Remove(_pageSizeParameterName); } else { - parameters["page[size]"] = pageSize.ToString(); + parameters[_pageSizeParameterName] = newPageSizeParameterValue; } if (pageOffset == 1) { - parameters.Remove("page[number]"); + parameters.Remove(_pageNumberParameterName); } else { - parameters["page[number]"] = pageOffset.ToString(); + parameters[_pageNumberParameterName] = pageOffset.ToString(); } }); - - return $"{_request.BasePath}/{resourceContext.PublicName}" + queryString; } - private string BuildQueryString(Action> updateAction) + private string ChangeTopPageSize(string pageSizeParameterValue, PageSize topPageSize) { - var parameters = _queryStringAccessor.Query.ToDictionary(pair => pair.Key, pair => pair.Value.ToString()); - updateAction(parameters); - string queryString = QueryString.Create(parameters).Value; + var elements = ParsePageSizeExpression(pageSizeParameterValue); + var elementInTopScopeIndex = elements.FindIndex(expression => expression.Scope == null); - return DecodeSpecialCharacters(queryString); + if (topPageSize != null) + { + var topPageSizeElement = new PaginationElementQueryStringValueExpression(null, topPageSize.Value); + + if (elementInTopScopeIndex != -1) + { + elements[elementInTopScopeIndex] = topPageSizeElement; + } + else + { + elements.Insert(0, topPageSizeElement); + } + } + else + { + if (elementInTopScopeIndex != -1) + { + elements.RemoveAt(elementInTopScopeIndex); + } + } + + var parameterValue = string.Join(',', + elements.Select(expression => expression.Scope == null ? expression.Value.ToString() : $"{expression.Scope}:{expression.Value}")); + + return parameterValue == string.Empty ? null : parameterValue; } - private static string DecodeSpecialCharacters(string uri) + private List ParsePageSizeExpression(string pageSizeParameterValue) { - return uri.Replace("%5B", "[").Replace("%5D", "]").Replace("%27", "'"); + if (pageSizeParameterValue == null) + { + return new List(); + } + + var requestResource = _request.SecondaryResource ?? _request.PrimaryResource; + + var parser = new PaginationParser(_provider); + var paginationExpression = parser.Parse(pageSizeParameterValue, requestResource); + + return new List(paginationExpression.Elements); } /// diff --git a/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs b/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs index e72377f877..0c273a8015 100644 --- a/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs +++ b/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs @@ -1,4 +1,5 @@ using System; +using System.Collections; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; @@ -119,6 +120,11 @@ public virtual async Task> GetAsync() { var topFilter = _queryLayerComposer.GetTopFilter(); _paginationContext.TotalResourceCount = await _repository.CountAsync(topFilter); + + if (_paginationContext.TotalResourceCount == 0) + { + return Array.Empty(); + } } var queryLayer = _queryLayerComposer.Compose(_request.PrimaryResource); @@ -130,6 +136,11 @@ public virtual async Task> GetAsync() return _hookExecutor.OnReturn(resources, ResourcePipeline.Get).ToArray(); } + if (queryLayer.Pagination?.PageSize != null && queryLayer.Pagination.PageSize.Value == resources.Count) + { + _paginationContext.IsPageFull = true; + } + return resources; } @@ -233,6 +244,14 @@ public virtual async Task GetSecondaryAsync(TId id, string relationshipN var secondaryLayer = _queryLayerComposer.Compose(_request.SecondaryResource); var primaryLayer = _queryLayerComposer.WrapLayerForSecondaryEndpoint(secondaryLayer, _request.PrimaryResource, id, _request.Relationship); + if (_request.IsCollection && _options.IncludeTotalResourceCount) + { + // TODO: Consider support for pagination links on secondary resource collection. This requires to call Count() on the inverse relationship (which may not exist). + // For /blogs/1/articles we need to execute Count(Articles.Where(article => article.Blog.Id == 1 && article.Blog.existingFilter))) to determine TotalResourceCount. + // This also means we need to invoke ResourceRepository
.CountAsync() from ResourceService. + // And we should call BlogResourceDefinition.OnApplyFilter to filter out soft-deleted blogs and translate from equals('IsDeleted','false') to equals('Blog.IsDeleted','false') + } + var primaryResources = await _repository.GetAsync(primaryLayer); var primaryResource = primaryResources.SingleOrDefault(); @@ -244,7 +263,15 @@ public virtual async Task GetSecondaryAsync(TId id, string relationshipN primaryResource = _hookExecutor.OnReturn(AsList(primaryResource), ResourcePipeline.GetRelationship).Single(); } - return _request.Relationship.GetValue(primaryResource); + var secondaryResource = _request.Relationship.GetValue(primaryResource); + + if (secondaryResource is ICollection secondaryResources && + secondaryLayer.Pagination?.PageSize != null && secondaryLayer.Pagination.PageSize.Value == secondaryResources.Count) + { + _paginationContext.IsPageFull = true; + } + + return secondaryResource; } /// diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/PaginationLinkTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/PaginationLinkTests.cs index 728fd699b0..c4b2e16495 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/PaginationLinkTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/PaginationLinkTests.cs @@ -27,12 +27,12 @@ public PaginationLinkTests(StandardApplicationFactory factory) : base(factory) } [Theory] - [InlineData(1, 1, 1, null, 2, 4)] - [InlineData(2, 2, 1, 1, 3, 4)] - [InlineData(3, 3, 1, 2, 4, 4)] - [InlineData(4, 4, 1, 3, null, 4)] + [InlineData(1, 1, null, 2, 4)] + [InlineData(2, 1, 1, 3, 4)] + [InlineData(3, 1, 2, 4, 4)] + [InlineData(4, 1, 3, null, 4)] public async Task When_page_number_is_specified_it_must_display_correct_top_level_links(int pageNumber, - int selfLink, int? firstLink, int? prevLink, int? nextLink, int? lastLink) + int? firstLink, int? prevLink, int? nextLink, int? lastLink) { // Arrange const int totalCount = 18; @@ -67,7 +67,7 @@ public async Task When_page_number_is_specified_it_must_display_correct_top_leve var body = await response.Content.ReadAsStringAsync(); var links = JsonConvert.DeserializeObject(body).Links; - Assert.EndsWith($"{routePrefix}{GetPageNumberInQueryString(selfLink)}", links.Self); + Assert.Equal("http://localhost" + route, links.Self); if (firstLink.HasValue) { diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationTests.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationWithTotalCountTests.cs similarity index 82% rename from test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationTests.cs rename to test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationWithTotalCountTests.cs index aecee57f26..80215e7138 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationWithTotalCountTests.cs @@ -14,15 +14,16 @@ namespace JsonApiDotNetCoreExampleTests.IntegrationTests.Pagination { - public sealed class PaginationTests : IClassFixture> + public sealed class PaginationWithTotalCountTests : IClassFixture> { private readonly IntegrationTestContext _testContext; - public PaginationTests(IntegrationTestContext testContext) + public PaginationWithTotalCountTests(IntegrationTestContext testContext) { _testContext = testContext; var options = (JsonApiOptions) testContext.Factory.Services.GetRequiredService(); + options.IncludeTotalResourceCount = true; options.DefaultPageSize = new PageSize(5); options.MaximumPageSize = null; options.MaximumPageNumber = null; @@ -65,6 +66,13 @@ await _testContext.RunOnDatabaseAsync(async dbContext => responseDocument.ManyData.Should().HaveCount(1); responseDocument.ManyData[0].Id.Should().Be(articles[1].StringId); + + responseDocument.Links.Should().NotBeNull(); + responseDocument.Links.Self.Should().Be("http://localhost" + route); + responseDocument.Links.First.Should().Be("http://localhost/api/v1/articles?page[size]=1"); + responseDocument.Links.Last.Should().Be(responseDocument.Links.Self); + responseDocument.Links.Prev.Should().Be(responseDocument.Links.First); + responseDocument.Links.Next.Should().BeNull(); } [Fact] @@ -134,6 +142,13 @@ await _testContext.RunOnDatabaseAsync(async dbContext => responseDocument.ManyData.Should().HaveCount(1); responseDocument.ManyData[0].Id.Should().Be(blog.Articles[1].StringId); + + responseDocument.Links.Should().NotBeNull(); + responseDocument.Links.Self.Should().Be("http://localhost" + route); + responseDocument.Links.First.Should().Be($"http://localhost/api/v1/blogs/{blog.StringId}/articles?page[size]=1"); + responseDocument.Links.Last.Should().BeNull(); + responseDocument.Links.Prev.Should().Be(responseDocument.Links.First); + responseDocument.Links.Next.Should().Be($"http://localhost/api/v1/blogs/{blog.StringId}/articles?page[number]=3&page[size]=1"); } [Fact] @@ -200,7 +215,8 @@ public async Task Can_paginate_in_scope_of_HasMany_relationship() Caption = "Second" } } - } + }, + new Blog() }; await _testContext.RunOnDatabaseAsync(async dbContext => @@ -211,7 +227,7 @@ await _testContext.RunOnDatabaseAsync(async dbContext => await dbContext.SaveChangesAsync(); }); - var route = "/api/v1/blogs?include=articles&page[number]=articles:2&page[size]=articles:1"; + var route = "/api/v1/blogs?include=articles&page[number]=articles:2&page[size]=2,articles:1"; // Act var (httpResponse, responseDocument) = await _testContext.ExecuteGetAsync(route); @@ -224,6 +240,13 @@ await _testContext.RunOnDatabaseAsync(async dbContext => responseDocument.Included[0].Id.Should().Be(blogs[0].Articles[1].StringId); responseDocument.Included[1].Id.Should().Be(blogs[1].Articles[1].StringId); + + responseDocument.Links.Should().NotBeNull(); + responseDocument.Links.Self.Should().Be("http://localhost" + route); + responseDocument.Links.First.Should().Be("http://localhost/api/v1/blogs?include=articles&page[size]=2,articles:1"); + responseDocument.Links.Last.Should().Be("http://localhost/api/v1/blogs?include=articles&page[number]=2&page[size]=2,articles:1"); + responseDocument.Links.Prev.Should().BeNull(); + responseDocument.Links.Next.Should().Be(responseDocument.Links.Last); } [Fact] @@ -267,6 +290,13 @@ await _testContext.RunOnDatabaseAsync(async dbContext => responseDocument.SingleData.Should().NotBeNull(); responseDocument.Included.Should().HaveCount(1); responseDocument.Included[0].Id.Should().Be(blog.Owner.Articles[1].StringId); + + responseDocument.Links.Should().NotBeNull(); + responseDocument.Links.Self.Should().Be("http://localhost" + route); + responseDocument.Links.First.Should().BeNull(); + responseDocument.Links.Last.Should().BeNull(); + responseDocument.Links.Prev.Should().BeNull(); + responseDocument.Links.Next.Should().BeNull(); } [Fact] @@ -345,6 +375,13 @@ await _testContext.RunOnDatabaseAsync(async dbContext => responseDocument.Included[0].Id.Should().Be(articles[0].ArticleTags.Skip(1).First().Tag.StringId); responseDocument.Included[1].Id.Should().Be(articles[1].ArticleTags.Skip(1).First().Tag.StringId); + + responseDocument.Links.Should().NotBeNull(); + responseDocument.Links.Self.Should().Be("http://localhost" + route); + responseDocument.Links.First.Should().Be("http://localhost/api/v1/articles?include=tags&page[size]=tags:1"); + responseDocument.Links.Last.Should().Be(responseDocument.Links.First); + responseDocument.Links.Prev.Should().BeNull(); + responseDocument.Links.Next.Should().BeNull(); } [Fact] @@ -414,6 +451,13 @@ await _testContext.RunOnDatabaseAsync(async dbContext => responseDocument.Included[0].Id.Should().Be(blogs[1].Owner.StringId); responseDocument.Included[1].Id.Should().Be(blogs[1].Owner.Articles[1].StringId); responseDocument.Included[2].Id.Should().Be(blogs[1].Owner.Articles[1].Revisions.Skip(1).First().StringId); + + responseDocument.Links.Should().NotBeNull(); + responseDocument.Links.Self.Should().Be("http://localhost" + route); + responseDocument.Links.First.Should().Be("http://localhost/api/v1/blogs?include=owner.articles.revisions&page[size]=1,owner.articles:1,owner.articles.revisions:1"); + responseDocument.Links.Last.Should().Be("http://localhost/api/v1/blogs?include=owner.articles.revisions&page[size]=1,owner.articles:1,owner.articles.revisions:1&page[number]=2"); + responseDocument.Links.Prev.Should().Be(responseDocument.Links.First); + responseDocument.Links.Next.Should().BeNull(); } [Fact] @@ -498,6 +542,13 @@ await _testContext.RunOnDatabaseAsync(async dbContext => responseDocument.ManyData.Should().HaveCount(2); responseDocument.ManyData[0].Id.Should().Be(blog.Articles[0].StringId); responseDocument.ManyData[1].Id.Should().Be(blog.Articles[1].StringId); + + responseDocument.Links.Should().NotBeNull(); + responseDocument.Links.Self.Should().Be("http://localhost" + route); + responseDocument.Links.First.Should().Be(responseDocument.Links.Self); + responseDocument.Links.Last.Should().BeNull(); + responseDocument.Links.Prev.Should().BeNull(); + responseDocument.Links.Next.Should().Be($"http://localhost/api/v1/blogs/{blog.StringId}/articles?page[number]=2"); } } } diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationWithoutTotalCountTests.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationWithoutTotalCountTests.cs new file mode 100644 index 0000000000..82c87d0429 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationWithoutTotalCountTests.cs @@ -0,0 +1,209 @@ +using System.Net; +using System.Threading.Tasks; +using Bogus; +using FluentAssertions; +using JsonApiDotNetCore.Configuration; +using JsonApiDotNetCore.Serialization.Objects; +using JsonApiDotNetCoreExample; +using JsonApiDotNetCoreExample.Data; +using JsonApiDotNetCoreExample.Models; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.Pagination +{ + public sealed class PaginationWithoutTotalCountTests : IClassFixture> + { + private const int _defaultPageSize = 5; + + private readonly IntegrationTestContext _testContext; + + private readonly Faker
_articleFaker = new Faker
(); + + public PaginationWithoutTotalCountTests(IntegrationTestContext testContext) + { + _testContext = testContext; + + var options = (JsonApiOptions) testContext.Factory.Services.GetRequiredService(); + + options.IncludeTotalResourceCount = false; + options.DefaultPageSize = new PageSize(_defaultPageSize); + options.AllowUnknownQueryStringParameters = true; + } + + [Fact] + public async Task When_page_size_is_unconstrained_it_should_not_render_pagination_links() + { + // Arrange + var options = (JsonApiOptions) _testContext.Factory.Services.GetRequiredService(); + options.DefaultPageSize = null; + + var route = "/api/v1/articles?foo=bar"; + + // Act + var (httpResponse, responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.Links.Should().NotBeNull(); + responseDocument.Links.Self.Should().Be("http://localhost/api/v1/articles?foo=bar"); + responseDocument.Links.First.Should().BeNull(); + responseDocument.Links.Last.Should().BeNull(); + responseDocument.Links.Prev.Should().BeNull(); + responseDocument.Links.Next.Should().BeNull(); + } + + [Fact] + public async Task When_page_size_is_specified_in_query_string_with_no_data_it_should_render_pagination_links() + { + // Arrange + var options = (JsonApiOptions) _testContext.Factory.Services.GetRequiredService(); + options.DefaultPageSize = null; + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + await dbContext.ClearTableAsync
(); + await dbContext.SaveChangesAsync(); + }); + + var route = "/api/v1/articles?page[size]=8&foo=bar"; + + // Act + var (httpResponse, responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.Links.Should().NotBeNull(); + responseDocument.Links.Self.Should().Be("http://localhost/api/v1/articles?page[size]=8&foo=bar"); + responseDocument.Links.First.Should().Be("http://localhost/api/v1/articles?page[size]=8&foo=bar"); + responseDocument.Links.Last.Should().BeNull(); + responseDocument.Links.Prev.Should().BeNull(); + responseDocument.Links.Next.Should().BeNull(); + } + + [Fact] + public async Task When_page_number_is_specified_in_query_string_with_no_data_it_should_render_pagination_links() + { + // Arrange + await _testContext.RunOnDatabaseAsync(async dbContext => + { + await dbContext.ClearTableAsync
(); + await dbContext.SaveChangesAsync(); + }); + + var route = "/api/v1/articles?page[number]=2&foo=bar"; + + // Act + var (httpResponse, responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.Links.Should().NotBeNull(); + responseDocument.Links.Self.Should().Be("http://localhost/api/v1/articles?page[number]=2&foo=bar"); + responseDocument.Links.First.Should().Be("http://localhost/api/v1/articles?foo=bar"); + responseDocument.Links.Last.Should().BeNull(); + responseDocument.Links.Prev.Should().Be("http://localhost/api/v1/articles?foo=bar"); + responseDocument.Links.Next.Should().BeNull(); + } + + [Fact] + public async Task When_page_number_is_specified_in_query_string_with_partially_filled_page_it_should_render_pagination_links() + { + // Arrange + var articles = _articleFaker.Generate(12); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + await dbContext.ClearTableAsync
(); + dbContext.Articles.AddRange(articles); + + await dbContext.SaveChangesAsync(); + }); + + var route = "/api/v1/articles?foo=bar&page[number]=3"; + + // Act + var (httpResponse, responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.ManyData.Count.Should().BeLessThan(_defaultPageSize); + + responseDocument.Links.Should().NotBeNull(); + responseDocument.Links.Self.Should().Be("http://localhost/api/v1/articles?foo=bar&page[number]=3"); + responseDocument.Links.First.Should().Be("http://localhost/api/v1/articles?foo=bar"); + responseDocument.Links.Last.Should().BeNull(); + responseDocument.Links.Prev.Should().Be("http://localhost/api/v1/articles?foo=bar&page[number]=2"); + responseDocument.Links.Next.Should().BeNull(); + } + + [Fact] + public async Task When_page_number_is_specified_in_query_string_with_full_page_it_should_render_pagination_links() + { + // Arrange + var articles = _articleFaker.Generate(_defaultPageSize * 3); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + await dbContext.ClearTableAsync
(); + dbContext.Articles.AddRange(articles); + + await dbContext.SaveChangesAsync(); + }); + + var route = "/api/v1/articles?page[number]=3&foo=bar"; + + // Act + var (httpResponse, responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.ManyData.Should().HaveCount(_defaultPageSize); + + responseDocument.Links.Should().NotBeNull(); + responseDocument.Links.Self.Should().Be("http://localhost/api/v1/articles?page[number]=3&foo=bar"); + responseDocument.Links.First.Should().Be("http://localhost/api/v1/articles?foo=bar"); + responseDocument.Links.Last.Should().BeNull(); + responseDocument.Links.Prev.Should().Be("http://localhost/api/v1/articles?page[number]=2&foo=bar"); + responseDocument.Links.Next.Should().Be("http://localhost/api/v1/articles?page[number]=4&foo=bar"); + } + + [Fact] + public async Task When_page_number_is_specified_in_query_string_with_full_page_on_secondary_endpoint_it_should_render_pagination_links() + { + // Arrange + var author = new Author + { + Articles = _articleFaker.Generate(_defaultPageSize * 3) + }; + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.AuthorDifferentDbContextName.Add(author); + await dbContext.SaveChangesAsync(); + }); + + var route = $"/api/v1/authors/{author.StringId}/articles?page[number]=3&foo=bar"; + + // Act + var (httpResponse, responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.ManyData.Should().HaveCount(_defaultPageSize); + + responseDocument.Links.Should().NotBeNull(); + responseDocument.Links.Self.Should().Be($"http://localhost/api/v1/authors/{author.StringId}/articles?page[number]=3&foo=bar"); + responseDocument.Links.First.Should().Be($"http://localhost/api/v1/authors/{author.StringId}/articles?foo=bar"); + responseDocument.Links.Last.Should().BeNull(); + responseDocument.Links.Prev.Should().Be($"http://localhost/api/v1/authors/{author.StringId}/articles?page[number]=2&foo=bar"); + responseDocument.Links.Next.Should().Be($"http://localhost/api/v1/authors/{author.StringId}/articles?page[number]=4&foo=bar"); + } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationRangeTests.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/RangeValidationTests.cs similarity index 97% rename from test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationRangeTests.cs rename to test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/RangeValidationTests.cs index 82075768d9..387eb8f99a 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationRangeTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/RangeValidationTests.cs @@ -12,14 +12,14 @@ namespace JsonApiDotNetCoreExampleTests.IntegrationTests.Pagination { - public sealed class PaginationRangeTests : IClassFixture> + public sealed class RangeValidationTests : IClassFixture> { private readonly IntegrationTestContext _testContext; private readonly Faker _todoItemFaker = new Faker(); private const int _defaultPageSize = 5; - public PaginationRangeTests(IntegrationTestContext testContext) + public RangeValidationTests(IntegrationTestContext testContext) { _testContext = testContext; diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationRangeWithMaximumTests.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/RangeValidationWithMaximumTests.cs similarity index 97% rename from test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationRangeWithMaximumTests.cs rename to test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/RangeValidationWithMaximumTests.cs index 6e6901a0e6..a8ee41db71 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationRangeWithMaximumTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/RangeValidationWithMaximumTests.cs @@ -10,14 +10,14 @@ namespace JsonApiDotNetCoreExampleTests.IntegrationTests.Pagination { - public sealed class PaginationRangeWithMaximumTests : IClassFixture> + public sealed class RangeValidationWithMaximumTests : IClassFixture> { private readonly IntegrationTestContext _testContext; private const int _maximumPageSize = 15; private const int _maximumPageNumber = 20; - public PaginationRangeWithMaximumTests(IntegrationTestContext testContext) + public RangeValidationWithMaximumTests(IntegrationTestContext testContext) { _testContext = testContext; diff --git a/test/UnitTests/Builders/LinkBuilderTests.cs b/test/UnitTests/Builders/LinkBuilderTests.cs index 5a5857066b..76fce6e770 100644 --- a/test/UnitTests/Builders/LinkBuilderTests.cs +++ b/test/UnitTests/Builders/LinkBuilderTests.cs @@ -16,13 +16,13 @@ public sealed class LinkBuilderTests { private readonly IPaginationContext _paginationContext = GetPaginationContext(); private readonly Mock _provider = new Mock(); - private readonly IRequestQueryStringAccessor _queryStringAccessor = new FakeRequestQueryStringAccessor("?foo=bar"); + private readonly IRequestQueryStringAccessor _queryStringAccessor = new FakeRequestQueryStringAccessor("?foo=bar&page[size]=10&page[number]=2"); private const string _host = "http://www.example.com"; private const int _primaryId = 123; private const string _relationshipName = "author"; - private const string _topSelf = "http://www.example.com/articles?foo=bar"; - private const string _topResourceSelf = "http://www.example.com/articles/123?foo=bar"; - private const string _topRelatedSelf = "http://www.example.com/articles/123/author?foo=bar"; + private const string _topSelf = "http://www.example.com/articles?foo=bar&page[size]=10&page[number]=2"; + private const string _topResourceSelf = "http://www.example.com/articles/123?foo=bar&page[size]=10&page[number]=2"; + private const string _topRelatedSelf = "http://www.example.com/articles/123/author?foo=bar&page[size]=10&page[number]=2"; private const string _resourceSelf = "http://www.example.com/articles/123"; private const string _relSelf = "http://www.example.com/articles/123/relationships/author"; private const string _relRelated = "http://www.example.com/articles/123/author"; @@ -143,7 +143,7 @@ public void BuildTopLevelLinks_GlobalAndResourceConfiguration_ExpectedLinks( var primaryResource = GetArticleResourceContext(topLevelLinks: resource); _provider.Setup(m => m.GetResourceContext
()).Returns(primaryResource); - bool usePrimaryId = expectedSelfLink != _topSelf; + bool usePrimaryId = expectedSelfLink != null && expectedSelfLink.Contains("123"); string relationshipName = expectedSelfLink == _topRelatedSelf ? _relationshipName : null; IJsonApiRequest request = GetRequestManager(primaryResource, usePrimaryId, relationshipName); @@ -159,9 +159,10 @@ public void BuildTopLevelLinks_GlobalAndResourceConfiguration_ExpectedLinks( } else { + Assert.Equal(links.Self, expectedSelfLink); + if (pages) { - Assert.Equal($"{_host}/articles?foo=bar&page[size]=10&page[number]=2", links.Self); Assert.Equal($"{_host}/articles?foo=bar&page[size]=10", links.First); Assert.Equal($"{_host}/articles?foo=bar&page[size]=10", links.Prev); Assert.Equal($"{_host}/articles?foo=bar&page[size]=10&page[number]=3", links.Next); @@ -169,7 +170,6 @@ public void BuildTopLevelLinks_GlobalAndResourceConfiguration_ExpectedLinks( } else { - Assert.Equal(links.Self , expectedSelfLink); Assert.Null(links.First); Assert.Null(links.Prev); Assert.Null(links.Next); @@ -185,6 +185,7 @@ private IJsonApiRequest GetRequestManager(ResourceContext resourceContext = null mock.Setup(m => m.PrimaryId).Returns(usePrimaryId ? _primaryId.ToString() : null); mock.Setup(m => m.Relationship).Returns(relationshipName != null ? new HasOneAttribute {PublicName = relationshipName} : null); mock.Setup(m => m.PrimaryResource).Returns(resourceContext); + mock.Setup(m => m.IsCollection).Returns(true); return mock.Object; } From 22d184321b77073993a2bfd8e476c55f36ff1437 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Fri, 25 Sep 2020 10:42:14 +0200 Subject: [PATCH 2/2] Migrated PaginationLinkTests --- .../Acceptance/Spec/PaginationLinkTests.cs | 118 ------------------ .../PaginationWithTotalCountTests.cs | 95 +++++++++++++- 2 files changed, 94 insertions(+), 119 deletions(-) delete mode 100644 test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/PaginationLinkTests.cs diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/PaginationLinkTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/PaginationLinkTests.cs deleted file mode 100644 index c4b2e16495..0000000000 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/PaginationLinkTests.cs +++ /dev/null @@ -1,118 +0,0 @@ -using System.Net; -using System.Threading.Tasks; -using Bogus; -using JsonApiDotNetCore.Configuration; -using JsonApiDotNetCore.Serialization.Objects; -using JsonApiDotNetCoreExample.Models; -using Newtonsoft.Json; -using Xunit; -using Person = JsonApiDotNetCoreExample.Models.Person; - -namespace JsonApiDotNetCoreExampleTests.Acceptance.Spec -{ - public class PaginationLinkTests : FunctionalTestCollection - { - private const int _defaultPageSize = 5; - - private readonly Faker _todoItemFaker = new Faker(); - - public PaginationLinkTests(StandardApplicationFactory factory) : base(factory) - { - var options = (JsonApiOptions) GetService(); - - options.DefaultPageSize = new PageSize(_defaultPageSize); - options.MaximumPageSize = null; - options.MaximumPageNumber = null; - options.AllowUnknownQueryStringParameters = true; - } - - [Theory] - [InlineData(1, 1, null, 2, 4)] - [InlineData(2, 1, 1, 3, 4)] - [InlineData(3, 1, 2, 4, 4)] - [InlineData(4, 1, 3, null, 4)] - public async Task When_page_number_is_specified_it_must_display_correct_top_level_links(int pageNumber, - int? firstLink, int? prevLink, int? nextLink, int? lastLink) - { - // Arrange - const int totalCount = 18; - - var person = new Person - { - LastName = "&Ampersand" - }; - - var todoItems = _todoItemFaker.Generate(totalCount); - foreach (var todoItem in todoItems) - { - todoItem.Owner = person; - } - - await _dbContext.ClearTableAsync(); - _dbContext.TodoItems.AddRange(todoItems); - await _dbContext.SaveChangesAsync(); - - string routePrefix = "/api/v1/todoItems?filter=equals(owner.lastName,'" + WebUtility.UrlEncode(person.LastName) + "')" + - "&fields[owner]=firstName&include=owner&sort=ordinal&foo=bar,baz"; - string route = pageNumber != 1 - ? routePrefix + $"&page[size]={_defaultPageSize}&page[number]={pageNumber}" - : routePrefix + $"&page[size]={_defaultPageSize}"; - - // Act - var response = await _client.GetAsync(route); - - // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - - var body = await response.Content.ReadAsStringAsync(); - var links = JsonConvert.DeserializeObject(body).Links; - - Assert.Equal("http://localhost" + route, links.Self); - - if (firstLink.HasValue) - { - var expected = $"{routePrefix}{GetPageNumberInQueryString(firstLink.Value)}"; - Assert.EndsWith(expected, links.First); - } - else - { - Assert.Null(links.First); - } - - if (prevLink.HasValue) - { - var expected = $"{routePrefix}{GetPageNumberInQueryString(prevLink.Value)}"; - Assert.EndsWith(expected, links.Prev); - } - else - { - Assert.Null(links.Prev); - } - - if (nextLink.HasValue) - { - var expected = $"{routePrefix}{GetPageNumberInQueryString(nextLink.Value)}"; - Assert.EndsWith(expected, links.Next); - } - else - { - Assert.Null(links.Next); - } - - if (lastLink.HasValue) - { - var expected = $"{routePrefix}{GetPageNumberInQueryString(lastLink.Value)}"; - Assert.EndsWith(expected, links.Last); - } - else - { - Assert.Null(links.Last); - } - } - - private static string GetPageNumberInQueryString(int offset) - { - return offset == 1 ? string.Empty : $"&page[number]={offset}"; - } - } -} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationWithTotalCountTests.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationWithTotalCountTests.cs index 80215e7138..cc695d45b9 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationWithTotalCountTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Pagination/PaginationWithTotalCountTests.cs @@ -2,6 +2,7 @@ using System.Linq; using System.Net; using System.Threading.Tasks; +using Bogus; using FluentAssertions; using FluentAssertions.Extensions; using JsonApiDotNetCore.Configuration; @@ -11,12 +12,16 @@ using JsonApiDotNetCoreExample.Models; using Microsoft.Extensions.DependencyInjection; using Xunit; +using Person = JsonApiDotNetCoreExample.Models.Person; namespace JsonApiDotNetCoreExampleTests.IntegrationTests.Pagination { public sealed class PaginationWithTotalCountTests : IClassFixture> { + private const int _defaultPageSize = 5; + private readonly IntegrationTestContext _testContext; + private readonly Faker _todoItemFaker = new Faker(); public PaginationWithTotalCountTests(IntegrationTestContext testContext) { @@ -24,9 +29,10 @@ public PaginationWithTotalCountTests(IntegrationTestContext(); options.IncludeTotalResourceCount = true; - options.DefaultPageSize = new PageSize(5); + options.DefaultPageSize = new PageSize(_defaultPageSize); options.MaximumPageSize = null; options.MaximumPageNumber = null; + options.AllowUnknownQueryStringParameters = true; options.DisableTopPagination = false; options.DisableChildrenPagination = false; @@ -550,5 +556,92 @@ await _testContext.RunOnDatabaseAsync(async dbContext => responseDocument.Links.Prev.Should().BeNull(); responseDocument.Links.Next.Should().Be($"http://localhost/api/v1/blogs/{blog.StringId}/articles?page[number]=2"); } + + [Theory] + [InlineData(1, 1, 4, null, 2)] + [InlineData(2, 1, 4, 1, 3)] + [InlineData(3, 1, 4, 2, 4)] + [InlineData(4, 1, 4, 3, null)] + public async Task Renders_correct_top_level_links_for_page_number(int pageNumber, int? firstLink, int? lastLink, int? prevLink, int? nextLink) + { + // Arrange + var person = new Person + { + LastName = "&Ampersand" + }; + + const int totalCount = 3 * _defaultPageSize + 3; + var todoItems = _todoItemFaker.Generate(totalCount); + + foreach (var todoItem in todoItems) + { + todoItem.Owner = person; + } + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + await dbContext.ClearTableAsync(); + dbContext.TodoItems.AddRange(todoItems); + + await dbContext.SaveChangesAsync(); + }); + + var routePrefix = "/api/v1/todoItems?filter=equals(owner.lastName,'" + WebUtility.UrlEncode(person.LastName) + "')" + + $"&fields[owner]=firstName&include=owner&sort=ordinal&foo=bar,baz"; + var route = routePrefix + $"&page[number]={pageNumber}"; + + // Act + var (httpResponse, responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + Assert.Equal("http://localhost" + route, responseDocument.Links.Self); + + if (firstLink != null) + { + var expected = "http://localhost" + SetPageNumberInUrl(routePrefix, firstLink.Value); + Assert.Equal(expected, responseDocument.Links.First); + } + else + { + Assert.Null(responseDocument.Links.First); + } + + if (prevLink != null) + { + var expected = "http://localhost" + SetPageNumberInUrl(routePrefix, prevLink.Value); + Assert.Equal(expected, responseDocument.Links.Prev); + } + else + { + Assert.Null(responseDocument.Links.Prev); + } + + if (nextLink != null) + { + var expected = "http://localhost" + SetPageNumberInUrl(routePrefix, nextLink.Value); + Assert.Equal(expected, responseDocument.Links.Next); + } + else + { + Assert.Null(responseDocument.Links.Next); + } + + if (lastLink != null) + { + var expected = "http://localhost" + SetPageNumberInUrl(routePrefix, lastLink.Value); + Assert.Equal(expected, responseDocument.Links.Last); + } + else + { + Assert.Null(responseDocument.Links.Last); + } + + static string SetPageNumberInUrl(string url, int pageNumber) + { + return pageNumber != 1 ? url + "&page[number]=" + pageNumber : url; + } + } } }