Fix: top-level links missing when paging

src/Examples/JsonApiDotNetCoreExample/Resources/LockableResource.cs

@@ -4,7 +4,6 @@
 using JsonApiDotNetCore.Internal;
 using JsonApiDotNetCore.Internal.Contracts;
 using JsonApiDotNetCore.Models;
-using JsonApiDotNetCore.Services;
 using JsonApiDotNetCoreExample.Models;
 
 namespace JsonApiDotNetCoreExample.Resources

src/JsonApiDotNetCore/Hooks/Execution/HookExecutorHelper.cs

@@ -74,15 +74,15 @@ public IResourceHookContainer<TResource> GetResourceHookContainer<TResource>(Res
             return (IResourceHookContainer<TResource>)GetResourceHookContainer(typeof(TResource), hook);
         }
 
-        public IEnumerable LoadDbValues(LeftType entityTypeForRepository, IEnumerable entities, ResourceHook hook, params RelationshipAttribute[] inclusionChain)
+        public IEnumerable LoadDbValues(LeftType entityTypeForRepository, IEnumerable entities, ResourceHook hook, params RelationshipAttribute[] relationshipsToNextLayer)
         {
             var idType = TypeHelper.GetIdentifierType(entityTypeForRepository);
             var parameterizedGetWhere = GetType()
                     .GetMethod(nameof(GetWhereAndInclude), BindingFlags.NonPublic | BindingFlags.Instance)
                     .MakeGenericMethod(entityTypeForRepository, idType);
             var casted = ((IEnumerable<object>)entities).Cast<IIdentifiable>();
             var ids = casted.Select(e => e.StringId).Cast(idType);
-            var values = (IEnumerable)parameterizedGetWhere.Invoke(this, new object[] { ids, inclusionChain });
+            var values = (IEnumerable)parameterizedGetWhere.Invoke(this, new object[] { ids, relationshipsToNextLayer });
             if (values == null) return null;
             return (IEnumerable)Activator.CreateInstance(typeof(HashSet<>).MakeGenericType(entityTypeForRepository), values.Cast(entityTypeForRepository));
         }
@@ -129,11 +129,15 @@ IHooksDiscovery GetHookDiscovery(Type entityType)
             return discovery;
         }
 
-        IEnumerable<TResource> GetWhereAndInclude<TResource, TId>(IEnumerable<TId> ids, RelationshipAttribute[] inclusionChain) where TResource : class, IIdentifiable<TId>
+        IEnumerable<TResource> GetWhereAndInclude<TResource, TId>(IEnumerable<TId> ids, RelationshipAttribute[] relationshipsToNextLayer) where TResource : class, IIdentifiable<TId>
         {
             var repo = GetRepository<TResource, TId>();
             var query = repo.Get().Where(e => ids.Contains(e.Id));
-            return repo.Include(query, inclusionChain).ToList();
+            foreach (var inclusionChainElement in relationshipsToNextLayer)
+            {
+                query = repo.Include(query, new RelationshipAttribute[] { inclusionChainElement });
+            }
+            return query.ToList();
         }
 
         IResourceReadRepository<TResource, TId> GetRepository<TResource, TId>() where TResource : class, IIdentifiable<TId>
@@ -183,12 +187,6 @@ public Dictionary<RelationshipAttribute, IEnumerable> LoadImplicitlyAffected(
             }
 
             return implicitlyAffected.ToDictionary(kvp => kvp.Key, kvp => TypeHelper.CreateHashSetFor(kvp.Key.RightType, kvp.Value));
-
-        }
-
-        private IEnumerable CreateHashSet(Type type, IList elements)
-        {
-            return (IEnumerable)Activator.CreateInstance(typeof(HashSet<>).MakeGenericType(type), new object[] { elements });
         }
 
         bool IsHasManyThrough(KeyValuePair<RelationshipAttribute, IEnumerable> kvp,
@@ -201,3 +199,4 @@ bool IsHasManyThrough(KeyValuePair<RelationshipAttribute, IEnumerable> kvp,
         }
     }
 }
+

src/JsonApiDotNetCore/QueryParameterServices/Contracts/IPageService.cs

@@ -14,16 +14,16 @@ public interface IPageService : IQueryParameterService
         /// </summary>
         int PageSize { get; set; }
         /// <summary>
-        /// What page are we currently on
+        /// The page requested. Note that the page number is one-based.
         /// </summary>
         int CurrentPage { get; set; }
         /// <summary>
         /// Total amount of pages for request
         /// </summary>
         int TotalPages { get; }
         /// <summary>
-        /// Checks if pagination is enabled
+        /// Denotes if pagination is possible for the current request
         /// </summary>
-        bool ShouldPaginate();
+        bool CanPaginate { get; }
     }
 }

src/JsonApiDotNetCore/QueryParameterServices/PageService.cs

@@ -10,52 +10,75 @@ namespace JsonApiDotNetCore.Query
     /// <inheritdoc/>
     public class PageService : QueryParameterService, IPageService
     {
-        private IJsonApiOptions _options;
+        private readonly IJsonApiOptions _options;
 
         public PageService(IJsonApiOptions options)
         {
             _options = options;
             PageSize = _options.DefaultPageSize;
         }
+
         /// <inheritdoc/>
         public int? TotalRecords { get; set; }
+
         /// <inheritdoc/>
         public int PageSize { get; set; }
+
         /// <inheritdoc/>
-        public int CurrentPage { get; set; }
+        public int CurrentPage { get; set; } = 1;
+
         /// <inheritdoc/>
         public int TotalPages => (TotalRecords == null || PageSize == 0) ? -1 : (int)Math.Ceiling(decimal.Divide(TotalRecords.Value, PageSize));
 
+        /// <inheritdoc/>
+        public bool CanPaginate { get { return TotalPages > 1; } }
+
         /// <inheritdoc/>
         public virtual void Parse(KeyValuePair<string, StringValues> queryParameter)
         {
-            // expected input = page[size]=10
-            //                  page[number]=1
+            // expected input = page[size]=<integer>
+            //                  page[number]=<integer > 0>
             var propertyName = queryParameter.Key.Split(QueryConstants.OPEN_BRACKET, QueryConstants.CLOSE_BRACKET)[1];
 
             const string SIZE = "size";
             const string NUMBER = "number";
 
             if (propertyName == SIZE)
             {
-                if (int.TryParse(queryParameter.Value, out var size))
+                if (!int.TryParse(queryParameter.Value, out var size))
+                {
+                    ThrowBadPagingRequest(queryParameter, "value could not be parsed as an integer");
+                }
+                else if (size < 1)
+                {
+                    ThrowBadPagingRequest(queryParameter, "value needs to be greater than zero");
+                }
+                else
+                {
                     PageSize = size;
-                else 
-                    throw new JsonApiException(400, $"Invalid page size '{queryParameter.Value}'");
+                }
             }
             else if (propertyName == NUMBER)
-            {
-                if (int.TryParse(queryParameter.Value, out var size))
-                    CurrentPage = size;
+            { 
+                if (!int.TryParse(queryParameter.Value, out var number))
+                {
+                    ThrowBadPagingRequest(queryParameter, "value could not be parsed as an integer");
+                }
+                else if (number == 0)
+                {
+                    ThrowBadPagingRequest(queryParameter, "page index is not zero-based");
+                }
                 else
-                    throw new JsonApiException(400, $"Invalid page number '{queryParameter.Value}'");
+                {
+                    CurrentPage = number;
+                }
             }
         }
 
-        /// <inheritdoc/>
-        public bool ShouldPaginate()
+        private void ThrowBadPagingRequest(KeyValuePair<string, StringValues> parameter, string message)
         {
-            return (PageSize > 0) || ((CurrentPage == 1 || CurrentPage == 0) && TotalPages <= 0);
+            throw new JsonApiException(400, $"Invalid page query parameter '{parameter.Key}={parameter.Value}': {message}");
         }
+
     }
 }

src/JsonApiDotNetCore/Serialization/Server/Builders/LinkBuilder.cs

@@ -31,10 +31,15 @@ public TopLevelLinks GetTopLevelLinks(ResourceContext primaryResource)
         {
             TopLevelLinks topLevelLinks = null;
             if (ShouldAddTopLevelLink(primaryResource, Link.Self))
+            {
                 topLevelLinks = new TopLevelLinks { Self = GetSelfTopLevelLink(primaryResource.ResourceName) };
+            }
 
-            if (ShouldAddTopLevelLink(primaryResource, Link.Paging))
-                SetPageLinks(primaryResource, ref topLevelLinks);
+            if (ShouldAddTopLevelLink(primaryResource, Link.Paging) && _pageService.CanPaginate)
+            {
+
+                SetPageLinks(primaryResource, topLevelLinks ??= new TopLevelLinks());
+            }
 
             return topLevelLinks;
         }
@@ -48,16 +53,16 @@ public TopLevelLinks GetTopLevelLinks(ResourceContext primaryResource)
         private bool ShouldAddTopLevelLink(ResourceContext primaryResource, Link link)
         {
             if (primaryResource.TopLevelLinks != Link.NotConfigured)
+            {
                 return primaryResource.TopLevelLinks.HasFlag(link);
+            }
+
             return _options.TopLevelLinks.HasFlag(link);
         }
 
-        private void SetPageLinks(ResourceContext primaryResource, ref TopLevelLinks links)
+        private void SetPageLinks(ResourceContext primaryResource, TopLevelLinks links)
         {
-            if (!_pageService.ShouldPaginate())
-                return;
-
-            links = links ?? new TopLevelLinks();
+            links.Self = GetPageLink(primaryResource, _pageService.CurrentPage, _pageService.PageSize);
 
             if (_pageService.CurrentPage > 1)
             {
@@ -66,11 +71,14 @@ private void SetPageLinks(ResourceContext primaryResource, ref TopLevelLinks lin
             }
 
             if (_pageService.CurrentPage < _pageService.TotalPages)
+            {
                 links.Next = GetPageLink(primaryResource, _pageService.CurrentPage + 1, _pageService.PageSize);
-
+            }
 
             if (_pageService.TotalPages > 0)
+            {
                 links.Last = GetPageLink(primaryResource, _pageService.TotalPages, _pageService.PageSize);
+            }
         }
 
         private string GetSelfTopLevelLink(string resourceName)
@@ -89,7 +97,9 @@ public ResourceLinks GetResourceLinks(string resourceName, string id)
         {
             var resourceContext = _provider.GetResourceContext(resourceName);
             if (ShouldAddResourceLink(resourceContext, Link.Self))
+            {
                 return new ResourceLinks { Self = GetSelfResourceLink(resourceName, id) };
+            }
 
             return null;
         }
@@ -101,7 +111,9 @@ public RelationshipLinks GetRelationshipLinks(RelationshipAttribute relationship
             var childNavigation = relationship.PublicRelationshipName;
             RelationshipLinks links = null;
             if (ShouldAddRelationshipLink(parentResourceContext, relationship, Link.Related))
+            {
                 links = new RelationshipLinks { Related = GetRelatedRelationshipLink(parentResourceContext.ResourceName, parent.StringId, childNavigation) };
+            }
 
             if (ShouldAddRelationshipLink(parentResourceContext, relationship, Link.Self))
             {
@@ -137,7 +149,9 @@ private string GetRelatedRelationshipLink(string parent, string parentId, string
         private bool ShouldAddResourceLink(ResourceContext resourceContext, Link link)
         {
             if (resourceContext.ResourceLinks != Link.NotConfigured)
+            {
                 return resourceContext.ResourceLinks.HasFlag(link);
+            }
             return _options.ResourceLinks.HasFlag(link);
         }
 
@@ -151,16 +165,24 @@ private bool ShouldAddResourceLink(ResourceContext resourceContext, Link link)
         private bool ShouldAddRelationshipLink(ResourceContext resourceContext, RelationshipAttribute relationship, Link link)
         {
             if (relationship.RelationshipLinks != Link.NotConfigured)
+            {
                 return relationship.RelationshipLinks.HasFlag(link);
+            }
             if (resourceContext.RelationshipLinks != Link.NotConfigured)
+            {
                 return resourceContext.RelationshipLinks.HasFlag(link);
+            }
+
             return _options.RelationshipLinks.HasFlag(link);
         }
 
         protected string GetBasePath()
         {
             if (_options.RelativeLinks)
+            {
                 return string.Empty;
+            }
+
             return _currentRequest.BasePath;
         }
     }

test/IntegrationTests/Data/EntityRepositoryTests.cs

@@ -9,22 +9,13 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using System.Reflection;
-using System.Text;
 using System.Threading.Tasks;
 using Xunit;
 
-
 namespace JADNC.IntegrationTests.Data
 {
     public class EntityRepositoryTests
     {
-
-
-        public EntityRepositoryTests()
-        {
-        }
-
         [Fact]
         public async Task UpdateAsync_AttributesUpdated_ShouldHaveSpecificallyThoseAttributesUpdated()
         {

test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/PagingTests.cs

@@ -7,6 +7,7 @@
 using JsonApiDotNetCoreExample;
 using JsonApiDotNetCoreExample.Models;
 using JsonApiDotNetCoreExampleTests.Helpers.Models;
+using Newtonsoft.Json;
 using Xunit;
 using Person = JsonApiDotNetCoreExample.Models.Person;
 
@@ -88,6 +89,69 @@ public async Task Can_Paginate_TodoItems_From_Start()
             Assert.Equal(expectedTodoItems, deserializedBody, new IdComparer<TodoItem>());
         }
 
+
+        [Fact]
+        public async Task Pagination_FirstPage_DisplaysCorrectLinks()
+        {
+            // Arrange
+            var totalCount = 20;
+            var person = new Person();
+            var todoItems = _todoItemFaker.Generate(totalCount).ToList();
+
+            foreach (var todoItem in todoItems)
+                todoItem.Owner = person;
+
+            Context.TodoItems.RemoveRange(Context.TodoItems);
+            Context.TodoItems.AddRange(todoItems);
+            Context.SaveChanges();
+
+            var route = $"/api/v1/todoItems";
+
+            // Act
+            var response = await Client.GetAsync(route);
+
+            // Assert
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            var body = await response.Content.ReadAsStringAsync();
+            var links = JsonConvert.DeserializeObject<Document>(body).Links;
+            Assert.EndsWith("/api/v1/todoItems?page[size]=5&page[number]=1", links.Self);
+            Assert.Null(links.First);
+            Assert.Null(links.Prev);
+            Assert.EndsWith("/api/v1/todoItems?page[size]=5&page[number]=2", links.Next);
+            Assert.EndsWith("/api/v1/todoItems?page[size]=5&page[number]=4", links.Last);
+        }
+
+        [Fact]
+        public async Task Pagination_SecondPage_DisplaysCorrectLinks()
+        {
+            // Arrange
+            var totalCount = 20;
+            var person = new Person();
+            var todoItems = _todoItemFaker.Generate(totalCount).ToList();
+
+            foreach (var todoItem in todoItems)
+                todoItem.Owner = person;
+
+            Context.TodoItems.RemoveRange(Context.TodoItems);
+            Context.TodoItems.AddRange(todoItems);
+            Context.SaveChanges();
+
+            var route = $"/api/v1/todoItems?page[size]=5&page[number]=2";
+
+            // Act
+            var response = await Client.GetAsync(route);
+
+            // Assert
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            var body = await response.Content.ReadAsStringAsync();
+            var links = JsonConvert.DeserializeObject<Document>(body).Links;
+            Assert.EndsWith("/api/v1/todoItems?page[size]=5&page[number]=2", links.Self);
+            Assert.EndsWith("/api/v1/todoItems?page[size]=5&page[number]=1", links.First);
+            Assert.EndsWith("/api/v1/todoItems?page[size]=5&page[number]=1", links.Prev);
+            Assert.EndsWith("/api/v1/todoItems?page[size]=5&page[number]=3", links.Next);
+            Assert.EndsWith("/api/v1/todoItems?page[size]=5&page[number]=4", links.Last);
+        }
+
         [Fact]
         public async Task Can_Paginate_TodoItems_From_End()
         {