Skip to content

Commit 0f7af6b

Browse files
fix: code quality and fixes (#116)
- Fix TypeHelpers.GetCollectionElementType treating Nullable<T> as a collection type - Consolidate 3 duplicate GetCollectionElementType implementations into TypeHelpers - Replace uncached reflection in InclusionMapper with QueryHelpers.GetPropertyByJsonName - Validate include depth during parsing instead of post-parse - Extract private methods from 209-line JsonApiQueryAsync - Split NestedPropertyNavigator (473 lines) into partial class files - Add opt-in StrictPagination option (returns 400 for invalid page params)
1 parent 381d1b1 commit 0f7af6b

10 files changed

Lines changed: 664 additions & 428 deletions

File tree

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
using JsonApiToolkit.Configuration;
2+
using JsonApiToolkit.Models.Errors;
3+
using JsonApiToolkit.Services;
4+
using Microsoft.AspNetCore.Http;
5+
using Microsoft.Extensions.Logging.Abstractions;
6+
using Microsoft.Extensions.Options;
7+
using Microsoft.Extensions.Primitives;
8+
9+
namespace JsonApiToolkit.Tests.Parsing;
10+
11+
public class StrictPaginationTests
12+
{
13+
private static JsonApiQueryParserService CreateService(JsonApiOptions? options = null)
14+
{
15+
options ??= new JsonApiOptions();
16+
return new JsonApiQueryParserService(
17+
NullLogger<JsonApiQueryParserService>.Instance,
18+
Options.Create(options)
19+
);
20+
}
21+
22+
private static HttpRequest CreateRequest(Dictionary<string, StringValues> query)
23+
{
24+
var httpContext = new DefaultHttpContext();
25+
httpContext.Request.Query = new QueryCollection(query);
26+
return httpContext.Request;
27+
}
28+
29+
// ─────────────────────────────────────────────────────────────────────────
30+
// Default behavior (StrictPagination = false)
31+
// ─────────────────────────────────────────────────────────────────────────
32+
33+
[Fact]
34+
public void Default_PageNumberZero_ClampedToOne()
35+
{
36+
var service = CreateService();
37+
var request = CreateRequest(
38+
new Dictionary<string, StringValues> { ["page[number]"] = "0" }
39+
);
40+
41+
var result = service.Parse(request);
42+
43+
Assert.NotNull(result.Pagination);
44+
Assert.Equal(1, result.Pagination.Number);
45+
}
46+
47+
[Fact]
48+
public void Default_PageSizeExceedsMax_ClampedToMax()
49+
{
50+
var service = CreateService(new JsonApiOptions { MaxPageSize = 50 });
51+
var request = CreateRequest(
52+
new Dictionary<string, StringValues> { ["page[size]"] = "200" }
53+
);
54+
55+
var result = service.Parse(request);
56+
57+
Assert.NotNull(result.Pagination);
58+
Assert.Equal(50, result.Pagination.Size);
59+
}
60+
61+
// ─────────────────────────────────────────────────────────────────────────
62+
// Strict mode (StrictPagination = true)
63+
// ─────────────────────────────────────────────────────────────────────────
64+
65+
[Fact]
66+
public void Strict_PageNumberZero_Throws400()
67+
{
68+
var service = CreateService(new JsonApiOptions { StrictPagination = true });
69+
var request = CreateRequest(
70+
new Dictionary<string, StringValues> { ["page[number]"] = "0" }
71+
);
72+
73+
var ex = Assert.Throws<JsonApiBadRequestException>(() => service.Parse(request));
74+
Assert.Contains("Invalid page number", ex.Message);
75+
}
76+
77+
[Fact]
78+
public void Strict_PageNumberNegative_Throws400()
79+
{
80+
var service = CreateService(new JsonApiOptions { StrictPagination = true });
81+
var request = CreateRequest(
82+
new Dictionary<string, StringValues> { ["page[number]"] = "-5" }
83+
);
84+
85+
var ex = Assert.Throws<JsonApiBadRequestException>(() => service.Parse(request));
86+
Assert.Contains("Invalid page number", ex.Message);
87+
}
88+
89+
[Fact]
90+
public void Strict_PageSizeZero_Throws400()
91+
{
92+
var service = CreateService(new JsonApiOptions { StrictPagination = true });
93+
var request = CreateRequest(new Dictionary<string, StringValues> { ["page[size]"] = "0" });
94+
95+
var ex = Assert.Throws<JsonApiBadRequestException>(() => service.Parse(request));
96+
Assert.Contains("Invalid page size", ex.Message);
97+
}
98+
99+
[Fact]
100+
public void Strict_PageSizeNegative_Throws400()
101+
{
102+
var service = CreateService(new JsonApiOptions { StrictPagination = true });
103+
var request = CreateRequest(
104+
new Dictionary<string, StringValues> { ["page[size]"] = "-10" }
105+
);
106+
107+
var ex = Assert.Throws<JsonApiBadRequestException>(() => service.Parse(request));
108+
Assert.Contains("Invalid page size", ex.Message);
109+
}
110+
111+
[Fact]
112+
public void Strict_PageSizeExceedsMax_Throws400()
113+
{
114+
var service = CreateService(
115+
new JsonApiOptions { StrictPagination = true, MaxPageSize = 50 }
116+
);
117+
var request = CreateRequest(
118+
new Dictionary<string, StringValues> { ["page[size]"] = "100" }
119+
);
120+
121+
var ex = Assert.Throws<JsonApiBadRequestException>(() => service.Parse(request));
122+
Assert.Contains("exceeds maximum", ex.Message);
123+
}
124+
125+
[Fact]
126+
public void Strict_ValidParameters_Works()
127+
{
128+
var service = CreateService(new JsonApiOptions { StrictPagination = true });
129+
var request = CreateRequest(
130+
new Dictionary<string, StringValues> { ["page[number]"] = "3", ["page[size]"] = "25" }
131+
);
132+
133+
var result = service.Parse(request);
134+
135+
Assert.NotNull(result.Pagination);
136+
Assert.Equal(3, result.Pagination.Number);
137+
Assert.Equal(25, result.Pagination.Size);
138+
}
139+
140+
[Fact]
141+
public void Strict_NonParseablePageNumber_DefaultsToOne()
142+
{
143+
var service = CreateService(new JsonApiOptions { StrictPagination = true });
144+
var request = CreateRequest(
145+
new Dictionary<string, StringValues> { ["page[number]"] = "abc" }
146+
);
147+
148+
// Non-parseable values are silently defaulted, not rejected
149+
var result = service.Parse(request);
150+
151+
Assert.NotNull(result.Pagination);
152+
Assert.Equal(1, result.Pagination.Number);
153+
}
154+
155+
[Fact]
156+
public void Strict_PageSizeAtMax_Works()
157+
{
158+
var service = CreateService(
159+
new JsonApiOptions { StrictPagination = true, MaxPageSize = 50 }
160+
);
161+
var request = CreateRequest(new Dictionary<string, StringValues> { ["page[size]"] = "50" });
162+
163+
var result = service.Parse(request);
164+
165+
Assert.NotNull(result.Pagination);
166+
Assert.Equal(50, result.Pagination.Size);
167+
}
168+
}

JsonApiToolkit.Tests/Services/JsonApiQueryParserServiceTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -239,9 +239,9 @@ public void Parse_WithMultipleLimitsExceeded_ThrowsOnFirstViolation()
239239
}
240240
);
241241

242-
// Filters are checked first
242+
// Include depth is validated during parsing (before post-parse filter count check)
243243
var exception = Assert.Throws<JsonApiBadRequestException>(() => service.Parse(request));
244-
Assert.Contains("filters", exception.Message);
244+
Assert.Contains("Include path", exception.Message);
245245
}
246246

247247
// ─────────────────────────────────────────────────────────────────────────

JsonApiToolkit/Configuration/JsonApiOptions.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,13 @@ public class JsonApiOptions
4747
/// </summary>
4848
public int DefaultPageSize { get; set; } = 10;
4949

50+
/// <summary>
51+
/// When true, returns 400 Bad Request for invalid pagination values instead of
52+
/// silently clamping. Invalid page numbers (less than 1) and page sizes (less than 1 or
53+
/// exceeding MaxPageSize) will return errors. Default: false (clamp for backwards compatibility).
54+
/// </summary>
55+
public bool StrictPagination { get; set; }
56+
5057
/// <summary>
5158
/// When true, applies database-level column filtering via EF Core Select() projection
5259
/// when fields[type] is specified in the request. Only fetches requested columns from

0 commit comments

Comments
 (0)