From 5a07e55a2bb21c6b6f19f642ec91b937cb52d4da Mon Sep 17 00:00:00 2001 From: Nathan Wise Date: Fri, 15 Jun 2018 15:36:15 -0400 Subject: [PATCH 1/5] Fixing issue where total-records was 0 on POST and PATCH calls. --- .../Services/JsonApiContext.cs | 2 + .../Acceptance/Spec/DocumentTests/Meta.cs | 114 ++++++++++++++++++ 2 files changed, 116 insertions(+) diff --git a/src/JsonApiDotNetCore/Services/JsonApiContext.cs b/src/JsonApiDotNetCore/Services/JsonApiContext.cs index 0643d494d6..df71d3ab02 100644 --- a/src/JsonApiDotNetCore/Services/JsonApiContext.cs +++ b/src/JsonApiDotNetCore/Services/JsonApiContext.cs @@ -119,11 +119,13 @@ private PageManager GetPageManager() return new PageManager(); var query = QuerySet?.PageQuery ?? new PageQuery(); + var requestMethod = _httpContextAccessor.HttpContext.Request.Method; return new PageManager { DefaultPageSize = Options.DefaultPageSize, CurrentPage = query.PageOffset, + TotalRecords = (requestMethod == "POST" || requestMethod == "PATCH") ? 1 : 0, PageSize = query.PageSize > 0 ? query.PageSize : Options.DefaultPageSize }; } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Meta.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Meta.cs index bb055e9935..013cd85bcb 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Meta.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Meta.cs @@ -1,6 +1,7 @@ using System.Collections; using System.Net; using System.Net.Http; +using System.Net.Http.Headers; using System.Threading.Tasks; using JsonApiDotNetCore.Models; using JsonApiDotNetCoreExample; @@ -54,6 +55,119 @@ public async Task Total_Record_Count_Included() Assert.Equal((long)expectedCount, (long)documents.Meta["total-records"]); } + [Fact] + public async Task Total_Record_Count_Not_Included_When_None() + { + // arrange + _context.TodoItems.RemoveRange(_context.TodoItems); + _context.SaveChanges(); + var builder = new WebHostBuilder() + .UseStartup(); + + var httpMethod = new HttpMethod("GET"); + var route = $"/api/v1/todo-items"; + + var server = new TestServer(builder); + var client = server.CreateClient(); + var request = new HttpRequestMessage(httpMethod, route); + + // act + var response = await client.SendAsync(request); + var responseBody = await response.Content.ReadAsStringAsync(); + var documents = JsonConvert.DeserializeObject(responseBody); + + // assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Null(documents.Meta); + } + + [Fact] + public async Task Total_Record_Count_Included_POST() + { + // arrange + _context.TodoItems.RemoveRange(_context.TodoItems); + _context.SaveChanges(); + var expectedCount = 1; + var builder = new WebHostBuilder() + .UseStartup(); + + var httpMethod = new HttpMethod("POST"); + var route = $"/api/v1/todo-items"; + + var server = new TestServer(builder); + var client = server.CreateClient(); + var request = new HttpRequestMessage(httpMethod, route); + var content = new + { + data = new + { + type = "todo-items", + attributes = new + { + description = "New Description", + } + } + }; + + request.Content = new StringContent(JsonConvert.SerializeObject(content)); + request.Content.Headers.ContentType = new MediaTypeHeaderValue("application/vnd.api+json"); + + // act + var response = await client.SendAsync(request); + var responseBody = await response.Content.ReadAsStringAsync(); + var documents = JsonConvert.DeserializeObject(responseBody); + + // assert + Assert.Equal(HttpStatusCode.Created, response.StatusCode); + Assert.NotNull(documents.Meta); + Assert.Equal((long)expectedCount, (long)documents.Meta["total-records"]); + } + + [Fact] + public async Task Total_Record_Count_Included_PATCH() + { + // arrange + _context.TodoItems.RemoveRange(_context.TodoItems); + TodoItem todoItem = new TodoItem(); + _context.TodoItems.Add(todoItem); + _context.SaveChanges(); + var expectedCount = 1; + var builder = new WebHostBuilder() + .UseStartup(); + + var httpMethod = new HttpMethod("PATCH"); + var route = $"/api/v1/todo-items/{todoItem.Id}"; + + var server = new TestServer(builder); + var client = server.CreateClient(); + var request = new HttpRequestMessage(httpMethod, route); + var content = new + { + data = new + { + type = "todo-items", + id = todoItem.Id, + attributes = new + { + description = "New Description", + } + } + }; + + request.Content = new StringContent(JsonConvert.SerializeObject(content)); + request.Content.Headers.ContentType = new MediaTypeHeaderValue("application/vnd.api+json"); + + // act + var response = await client.SendAsync(request); + var responseBody = await response.Content.ReadAsStringAsync(); + var documents = JsonConvert.DeserializeObject(responseBody); + + // assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(documents.Meta); + Assert.Equal((long)expectedCount, (long)documents.Meta["total-records"]); + } + [Fact] public async Task EntityThatImplements_IHasMeta_Contains_MetaData() { From edead435f9936507f629b8d3d9270dc225e30aec Mon Sep 17 00:00:00 2001 From: jaredcnance Date: Wed, 20 Jun 2018 18:37:54 -0500 Subject: [PATCH 2/5] PR feedback --- .../Builders/DocumentBuilder.cs | 2 +- src/JsonApiDotNetCore/Internal/PageManager.cs | 4 +-- .../JsonApiDotNetCore.csproj | 2 +- .../Services/JsonApiContext.cs | 12 +++---- .../Acceptance/Spec/DocumentTests/Meta.cs | 35 +++++++++---------- 5 files changed, 26 insertions(+), 29 deletions(-) diff --git a/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs b/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs index 0beb0516c1..1a7a692101 100644 --- a/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs +++ b/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs @@ -75,7 +75,7 @@ private Dictionary GetMeta(IIdentifiable entity) if (entity is IHasMeta metaEntity) builder.Add(metaEntity.GetMeta(_jsonApiContext)); - if (_jsonApiContext.Options.IncludeTotalRecordCount) + if (_jsonApiContext.Options.IncludeTotalRecordCount && _jsonApiContext.PageManager.TotalRecords != null) builder.Add("total-records", _jsonApiContext.PageManager.TotalRecords); if (_requestMeta != null) diff --git a/src/JsonApiDotNetCore/Internal/PageManager.cs b/src/JsonApiDotNetCore/Internal/PageManager.cs index 17da00333a..1e35b71b4e 100644 --- a/src/JsonApiDotNetCore/Internal/PageManager.cs +++ b/src/JsonApiDotNetCore/Internal/PageManager.cs @@ -6,12 +6,12 @@ namespace JsonApiDotNetCore.Internal { public class PageManager { - public int TotalRecords { get; set; } + public int? TotalRecords { get; set; } public int PageSize { get; set; } public int DefaultPageSize { get; set; } public int CurrentPage { get; set; } public bool IsPaginated => PageSize > 0; - public int TotalPages => (TotalRecords == 0) ? -1 : (int)Math.Ceiling(decimal.Divide(TotalRecords, PageSize)); + public int TotalPages => (TotalRecords == 0 || TotalRecords == null) ? -1 : (int)Math.Ceiling(decimal.Divide(TotalRecords.Value, PageSize)); public RootLinks GetPageLinks(LinkBuilder linkBuilder) { diff --git a/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj b/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj index e9d5eea415..7019a8e6cc 100755 --- a/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj +++ b/src/JsonApiDotNetCore/JsonApiDotNetCore.csproj @@ -1,6 +1,6 @@  - 2.3.0 + 2.3.1 $(NetStandardVersion) JsonApiDotNetCore JsonApiDotNetCore diff --git a/src/JsonApiDotNetCore/Services/JsonApiContext.cs b/src/JsonApiDotNetCore/Services/JsonApiContext.cs index df71d3ab02..21c4b56736 100644 --- a/src/JsonApiDotNetCore/Services/JsonApiContext.cs +++ b/src/JsonApiDotNetCore/Services/JsonApiContext.cs @@ -86,20 +86,20 @@ internal static bool PathIsRelationship(string requestPath) const char pathSegmentDelimiter = '/'; var span = requestPath.AsSpan(); - + // we need to iterate over the string, from the end, // checking whether or not the 2nd to last path segment // is "relationships" // -2 is chosen in case the path ends with '/' - for(var i = requestPath.Length - 2; i >= 0; i--) + for (var i = requestPath.Length - 2; i >= 0; i--) { // if there are not enough characters left in the path to // contain "relationships" - if(i < relationships.Length) + if (i < relationships.Length) return false; // we have found the first instance of '/' - if(span[i] == pathSegmentDelimiter) + if (span[i] == pathSegmentDelimiter) { // in the case of a "relationships" route, the next // path segment will be "relationships" @@ -112,20 +112,18 @@ internal static bool PathIsRelationship(string requestPath) return false; } - + private PageManager GetPageManager() { if (Options.DefaultPageSize == 0 && (QuerySet == null || QuerySet.PageQuery.PageSize == 0)) return new PageManager(); var query = QuerySet?.PageQuery ?? new PageQuery(); - var requestMethod = _httpContextAccessor.HttpContext.Request.Method; return new PageManager { DefaultPageSize = Options.DefaultPageSize, CurrentPage = query.PageOffset, - TotalRecords = (requestMethod == "POST" || requestMethod == "PATCH") ? 1 : 0, PageSize = query.PageSize > 0 ? query.PageSize : Options.DefaultPageSize }; } diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Meta.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Meta.cs index 013cd85bcb..5c74c2c715 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Meta.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Meta.cs @@ -48,7 +48,7 @@ public async Task Total_Record_Count_Included() var response = await client.SendAsync(request); var responseBody = await response.Content.ReadAsStringAsync(); var documents = JsonConvert.DeserializeObject(responseBody); - + // assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(documents.Meta); @@ -56,7 +56,7 @@ public async Task Total_Record_Count_Included() } [Fact] - public async Task Total_Record_Count_Not_Included_When_None() + public async Task Total_Record_Count_Included_When_None() { // arrange _context.TodoItems.RemoveRange(_context.TodoItems); @@ -75,14 +75,15 @@ public async Task Total_Record_Count_Not_Included_When_None() var response = await client.SendAsync(request); var responseBody = await response.Content.ReadAsStringAsync(); var documents = JsonConvert.DeserializeObject(responseBody); - + // assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); - Assert.Null(documents.Meta); + Assert.NotNull(documents.Meta); + Assert.Equal(0, (long)documents.Meta["total-records"]); } [Fact] - public async Task Total_Record_Count_Included_POST() + public async Task Total_Record_Count_Not_Included_In_POST_Response() { // arrange _context.TodoItems.RemoveRange(_context.TodoItems); @@ -116,15 +117,14 @@ public async Task Total_Record_Count_Included_POST() var response = await client.SendAsync(request); var responseBody = await response.Content.ReadAsStringAsync(); var documents = JsonConvert.DeserializeObject(responseBody); - + // assert Assert.Equal(HttpStatusCode.Created, response.StatusCode); - Assert.NotNull(documents.Meta); - Assert.Equal((long)expectedCount, (long)documents.Meta["total-records"]); + Assert.Null(documents.Meta); } [Fact] - public async Task Total_Record_Count_Included_PATCH() + public async Task Total_Record_Count_Not_Included_In_PATCH_Response() { // arrange _context.TodoItems.RemoveRange(_context.TodoItems); @@ -161,11 +161,10 @@ public async Task Total_Record_Count_Included_PATCH() var response = await client.SendAsync(request); var responseBody = await response.Content.ReadAsStringAsync(); var documents = JsonConvert.DeserializeObject(responseBody); - + // assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); - Assert.NotNull(documents.Meta); - Assert.Equal((long)expectedCount, (long)documents.Meta["total-records"]); + Assert.Null(documents.Meta); } [Fact] @@ -187,26 +186,26 @@ public async Task EntityThatImplements_IHasMeta_Contains_MetaData() // act var response = await client.SendAsync(request); var documents = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); - + // assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(documents.Meta); Assert.NotNull(expectedMeta); Assert.NotEmpty(expectedMeta); - - foreach(var hash in expectedMeta) + + foreach (var hash in expectedMeta) { - if(hash.Value is IList) + if (hash.Value is IList) { var listValue = (IList)hash.Value; - for(var i=0; i < listValue.Count; i++) + for (var i = 0; i < listValue.Count; i++) Assert.Equal(listValue[i].ToString(), ((IList)documents.Meta[hash.Key])[i].ToString()); } else { Assert.Equal(hash.Value, documents.Meta[hash.Key]); } - } + } } } } From faa00b4a8ef8c8f0c90a535ee782624cb6d19d39 Mon Sep 17 00:00:00 2001 From: jaredcnance Date: Wed, 20 Jun 2018 18:44:27 -0500 Subject: [PATCH 3/5] allow TotalRecords to be 0 --- src/JsonApiDotNetCore/Internal/PageManager.cs | 2 +- .../Acceptance/Spec/DocumentTests/Meta.cs | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/JsonApiDotNetCore/Internal/PageManager.cs b/src/JsonApiDotNetCore/Internal/PageManager.cs index 1e35b71b4e..d27fc158fd 100644 --- a/src/JsonApiDotNetCore/Internal/PageManager.cs +++ b/src/JsonApiDotNetCore/Internal/PageManager.cs @@ -11,7 +11,7 @@ public class PageManager public int DefaultPageSize { get; set; } public int CurrentPage { get; set; } public bool IsPaginated => PageSize > 0; - public int TotalPages => (TotalRecords == 0 || TotalRecords == null) ? -1 : (int)Math.Ceiling(decimal.Divide(TotalRecords.Value, PageSize)); + public int TotalPages => (TotalRecords == null) ? -1 : (int)Math.Ceiling(decimal.Divide(TotalRecords.Value, PageSize)); public RootLinks GetPageLinks(LinkBuilder linkBuilder) { diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Meta.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Meta.cs index 5c74c2c715..2b6b1e251b 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Meta.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/DocumentTests/Meta.cs @@ -88,7 +88,6 @@ public async Task Total_Record_Count_Not_Included_In_POST_Response() // arrange _context.TodoItems.RemoveRange(_context.TodoItems); _context.SaveChanges(); - var expectedCount = 1; var builder = new WebHostBuilder() .UseStartup(); @@ -120,7 +119,7 @@ public async Task Total_Record_Count_Not_Included_In_POST_Response() // assert Assert.Equal(HttpStatusCode.Created, response.StatusCode); - Assert.Null(documents.Meta); + Assert.False(documents.Meta.ContainsKey("total-records")); } [Fact] @@ -131,7 +130,6 @@ public async Task Total_Record_Count_Not_Included_In_PATCH_Response() TodoItem todoItem = new TodoItem(); _context.TodoItems.Add(todoItem); _context.SaveChanges(); - var expectedCount = 1; var builder = new WebHostBuilder() .UseStartup(); @@ -164,7 +162,7 @@ public async Task Total_Record_Count_Not_Included_In_PATCH_Response() // assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); - Assert.Null(documents.Meta); + Assert.False(documents.Meta.ContainsKey("total-records")); } [Fact] From 46fbf741efe8df9c47cfcbdd278ac5b62b0f9918 Mon Sep 17 00:00:00 2001 From: jaredcnance Date: Wed, 20 Jun 2018 19:07:40 -0500 Subject: [PATCH 4/5] allow meta to be included even if there is no returned resource --- src/JsonApiDotNetCore/Builders/DocumentBuilder.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs b/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs index 1a7a692101..c09cbefef3 100644 --- a/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs +++ b/src/JsonApiDotNetCore/Builders/DocumentBuilder.cs @@ -68,21 +68,20 @@ public Documents Build(IEnumerable entities) private Dictionary GetMeta(IIdentifiable entity) { - if (entity == null) return null; - var builder = _jsonApiContext.MetaBuilder; - - if (entity is IHasMeta metaEntity) - builder.Add(metaEntity.GetMeta(_jsonApiContext)); - if (_jsonApiContext.Options.IncludeTotalRecordCount && _jsonApiContext.PageManager.TotalRecords != null) builder.Add("total-records", _jsonApiContext.PageManager.TotalRecords); if (_requestMeta != null) builder.Add(_requestMeta.GetMeta()); + if (entity != null && entity is IHasMeta metaEntity) + builder.Add(metaEntity.GetMeta(_jsonApiContext)); + var meta = builder.Build(); - if (meta.Count > 0) return meta; + if (meta.Count > 0) + return meta; + return null; } From 0a7faedf7aaaa9ba7925bb1de7d1125fe11a58ef Mon Sep 17 00:00:00 2001 From: jaredcnance Date: Wed, 20 Jun 2018 19:30:55 -0500 Subject: [PATCH 5/5] expect total-records to be included if configured --- .../Acceptance/Spec/FetchingDataTests.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/FetchingDataTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/FetchingDataTests.cs index 8573d0b560..9c8d5f8214 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/FetchingDataTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/FetchingDataTests.cs @@ -53,8 +53,10 @@ public async Task Request_ForEmptyCollection_Returns_EmptyDataCollection() var server = new TestServer(builder); var client = server.CreateClient(); var request = new HttpRequestMessage(httpMethod, route); - var expectedBody = JsonConvert.SerializeObject(new { - data = new List() + var expectedBody = JsonConvert.SerializeObject(new + { + data = new List(), + meta = new Dictionary { { "total-records", 0 } } }); // act