Skip to content

Commit f4db9ff

Browse files
Merge pull request #26 from intility/25-include-without-filtering-issues
2 parents 8d21509 + a0a51dd commit f4db9ff

52 files changed

Lines changed: 1187 additions & 1520 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CLAUDE.md

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ dotnet csharpier . --check
4848
### Documentation
4949
Documentation is built using DocFX and deployed to GitHub Pages. The documentation source is in `/docs/` and the built site goes to `/docs/_site/`.
5050

51+
### Debugging
52+
Enable detailed logging for query processing and troubleshooting:
53+
- Set `"JsonApiToolkit": "Debug"` in appsettings.json
54+
- See `/docs/docs/debugging.md` for comprehensive debugging guide
55+
5156
## Architecture
5257

5358
### Core Components
@@ -66,7 +71,12 @@ Documentation is built using DocFX and deployed to GitHub Pages. The documentati
6671
3. **Query Processing Pipeline** (`Extensions/Querying/`)
6772
- `JsonApiQueryParser`: Parses JSON:API query parameters
6873
- `FilterExpressionBuilder`: Builds LINQ expressions from filters
69-
- `QueryableExtensions`: Extension methods for applying filters, sorting, pagination
74+
- `FilterHandler`: Applies filter expressions to queryables
75+
- `SortingHandler`: Applies sorting to queryables
76+
- `PaginationHandler`: Applies pagination to queryables
77+
- `QueryHelpers`: Helper methods for property name mapping and type conversion
78+
- `IncludeFilterParser`: Separates filters targeting included resources from main entity filters
79+
- `FilteredIncludeBuilder`: Applies filtered includes using EF Core's filtered Include functionality
7080

7181
4. **Models** (`Models/`)
7282
- Document structures: `JsonApiDocument<T>`, `JsonApiCollectionDocument<T>`
@@ -79,19 +89,23 @@ Documentation is built using DocFX and deployed to GitHub Pages. The documentati
7989

8090
6. **Validation** (`Validation/`)
8191
- `IncludePatternValidator`: Validates include patterns with wildcard support
92+
- `IncludeValidator`: Validates include paths against entity relationships
93+
- `IncludePattern`: Model representing validated include patterns
8294

83-
7. **Include Filtering** (`Extensions/Querying/`)
84-
- `IncludeFilterParser`: Separates filters targeting included resources from main entity filters
85-
- `FilteredIncludeBuilder`: Applies filtered includes using EF Core's filtered Include functionality
86-
- Enables filtering on relationships (e.g., `filter[author.name]=John` with `include=author`)
95+
7. **Services** (`Services/`)
96+
- `IJsonApiQueryParser`: Interface for query parameter parsing
97+
- `JsonApiQueryParserService`: Service implementation for parsing JSON:API query strings
98+
99+
8. **Helpers** (`Helpers/`)
100+
- `EfIncludePathHelper`: Utilities for building EF Core Include expressions
87101

88102
### Key Patterns
89103

90104
- **Convention-based mapping**: Properties are automatically mapped from C# PascalCase to JSON camelCase
91105
- **Query parameter parsing**: Standard JSON:API query syntax (`filter[field]=value`, `sort=field,-field2`, `page[number]=1&page[size]=10`, `include=relationship`)
92106
- **Async-first**: Main controller method `JsonApiQueryAsync()` is async and works with `IQueryable<T>`
93107
- **Entity Framework integration**: Uses EF Core's `Include()` and query building capabilities
94-
- **Filter expressions**: Complex filtering with operators (eq, ne, gt, lt, contains, etc.), logical grouping, enum support, and filtering on included resources
108+
- **Filter expressions**: Complex filtering with operators (eq, ne, gt, lt, contains, etc.), logical grouping, enum support, and filtering on included resources via dot notation (e.g., `filter[author.name]=John` with `include=author`)
95109
- **JSON column detection**: Collections and complex objects without ID properties are automatically mapped as JSON attributes instead of relationships (useful for EF Core owned entities stored as JSON columns)
96110
- **Pagination safety**: Invalid page numbers are automatically clamped to valid ranges (page 1 for negative/zero, last page for overflow)
97111
- **Include whitelisting**: Use `AllowedIncludesAttribute` on controller actions to restrict which relationships can be included, preventing unauthorized data exposure
@@ -148,6 +162,7 @@ Tests are organized by component:
148162
- Entity types should have an `Id` property (auto-detected by `EntityMapper.GetIdProperty()`)
149163
- Use `QueryParameters queryParams = GetJsonApiQueryParameters()` to access parsed query parameters
150164
- For manual mapping, use `JsonApiMapper.ToDocument()` or `ToCollectionDocument()`
165+
- Enable debug logging with `"JsonApiToolkit": "Debug"` in appsettings.json for detailed query processing insights
151166

152167
## Package Publication
153168

JsonApiToolkit.Tests/Controllers/JsonApiControllerTests.cs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@
22
using JsonApiToolkit.Models.Documents;
33
using JsonApiToolkit.Models.Errors;
44
using JsonApiToolkit.Models.Resources;
5+
using JsonApiToolkit.Services;
56
using JsonApiToolkit.Tests.Models;
67
using Microsoft.AspNetCore.Http;
78
using Microsoft.AspNetCore.Mvc;
9+
using Microsoft.Extensions.DependencyInjection;
10+
using Microsoft.Extensions.Logging;
11+
using Moq;
812

913
namespace JsonApiToolkit.Tests.Controllers;
1014

@@ -42,9 +46,25 @@ public class JsonApiControllerTests
4246

4347
public JsonApiControllerTests()
4448
{
49+
var services = new ServiceCollection();
50+
services.AddLogging();
51+
services.AddScoped<IJsonApiQueryParser>(provider =>
52+
{
53+
var mock = new Mock<IJsonApiQueryParser>();
54+
mock.Setup(x => x.Parse(It.IsAny<Microsoft.AspNetCore.Http.HttpRequest>()))
55+
.Returns(
56+
new JsonApiToolkit.Models.Querying.QueryParameters
57+
{
58+
Include = new List<string>(),
59+
}
60+
);
61+
return mock.Object;
62+
});
63+
64+
var serviceProvider = services.BuildServiceProvider();
4565
_controller = new TestJsonApiController();
4666

47-
var httpContext = new DefaultHttpContext();
67+
var httpContext = new DefaultHttpContext { RequestServices = serviceProvider };
4868
httpContext.Request.Scheme = "https";
4969
httpContext.Request.Host = new HostString("api.example.com");
5070
httpContext.Request.Path = "/test-entities";
Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,258 @@
1+
using JsonApiToolkit.Extensions;
2+
using Microsoft.EntityFrameworkCore;
3+
using Xunit;
4+
5+
namespace JsonApiToolkit.Tests.Extensions;
6+
7+
/// <summary>
8+
/// Tests for EF Core Include behavior with pagination to catch split query issues.
9+
/// </summary>
10+
public class IncludesWithPaginationTests
11+
{
12+
private class TestEntity
13+
{
14+
public int Id { get; set; }
15+
public string Name { get; set; } = string.Empty;
16+
public List<RelatedEntity> Related { get; set; } = new();
17+
}
18+
19+
private class RelatedEntity
20+
{
21+
public int Id { get; set; }
22+
public string Value { get; set; } = string.Empty;
23+
public int TestEntityId { get; set; }
24+
public TestEntity? TestEntity { get; set; }
25+
}
26+
27+
private class TestDbContext : DbContext
28+
{
29+
public DbSet<TestEntity> TestEntities { get; set; } = null!;
30+
public DbSet<RelatedEntity> RelatedEntities { get; set; } = null!;
31+
32+
public TestDbContext(DbContextOptions<TestDbContext> options)
33+
: base(options) { }
34+
35+
protected override void OnModelCreating(ModelBuilder modelBuilder)
36+
{
37+
modelBuilder.Entity<TestEntity>().HasMany(e => e.Related).WithOne(r => r.TestEntity);
38+
}
39+
}
40+
41+
private static TestDbContext CreateInMemoryContext()
42+
{
43+
var options = new DbContextOptionsBuilder<TestDbContext>()
44+
.UseInMemoryDatabase(Guid.NewGuid().ToString())
45+
.Options;
46+
47+
var context = new TestDbContext(options);
48+
49+
// Create a large dataset similar to the user's scenario
50+
for (int i = 1; i <= 100; i++)
51+
{
52+
var entity = new TestEntity
53+
{
54+
Id = i,
55+
Name = $"Entity {i}",
56+
Related = new(),
57+
};
58+
59+
// Add 2-5 related entities per main entity
60+
var relatedCount = (i % 4) + 2;
61+
for (int j = 1; j <= relatedCount; j++)
62+
{
63+
entity.Related.Add(
64+
new RelatedEntity
65+
{
66+
Id = i * 100 + j,
67+
Value = $"Related {i}-{j}",
68+
TestEntityId = i,
69+
}
70+
);
71+
}
72+
73+
context.TestEntities.Add(entity);
74+
}
75+
76+
context.SaveChanges();
77+
return context;
78+
}
79+
80+
[Theory]
81+
[InlineData(1)]
82+
[InlineData(5)]
83+
[InlineData(9)]
84+
[InlineData(10)]
85+
[InlineData(20)]
86+
[InlineData(50)]
87+
public async Task ApplyIncludesSingleQuery_WithPagination_LoadsAllRelatedEntitiesAsync(
88+
int pageSize
89+
)
90+
{
91+
// Arrange
92+
using var context = CreateInMemoryContext();
93+
94+
// Act - Apply includes with AsSingleQuery and pagination
95+
var query = context
96+
.TestEntities.OrderBy(e => e.Id)
97+
.ApplyIncludesSingleQuery(new List<string> { "Related" })
98+
.Skip(0)
99+
.Take(pageSize);
100+
101+
var results = await query.ToListAsync();
102+
103+
// Assert
104+
Assert.NotEmpty(results);
105+
Assert.Equal(pageSize, results.Count);
106+
107+
// Verify that ALL entities have their related entities loaded
108+
foreach (var entity in results)
109+
{
110+
Assert.NotNull(entity.Related);
111+
Assert.NotEmpty(entity.Related);
112+
Assert.InRange(entity.Related.Count, 2, 5); // We created 2-5 related entities per entity
113+
}
114+
}
115+
116+
[Theory]
117+
[InlineData(1)]
118+
[InlineData(5)]
119+
[InlineData(9)]
120+
public async Task ApplyIncludes_WithPagination_MayFailOnSmallPageSizesAsync(int pageSize)
121+
{
122+
// Arrange
123+
using var context = CreateInMemoryContext();
124+
125+
// Act - Regular includes (may exhibit split query issues)
126+
var query = context
127+
.TestEntities.OrderBy(e => e.Id)
128+
.ApplyIncludes(new List<string> { "Related" })
129+
.Skip(0)
130+
.Take(pageSize);
131+
132+
var results = await query.ToListAsync();
133+
134+
// Assert - Just verify we got results, may or may not have includes loaded
135+
Assert.NotEmpty(results);
136+
Assert.Equal(pageSize, results.Count);
137+
138+
// Note: This test documents the issue - with regular ApplyIncludes,
139+
// related entities may not be loaded consistently across page sizes
140+
}
141+
142+
[Fact]
143+
public async Task ApplyIncludesSingleQuery_WithLargeDatasetAndPaginationAtFirstPage_LoadsCorrectEntitiesAsync()
144+
{
145+
// Arrange
146+
using var context = CreateInMemoryContext();
147+
148+
// Act - Get first page with page size 1 (the failing scenario from user's issue)
149+
var results = await context
150+
.TestEntities.OrderBy(e => e.Id)
151+
.ApplyIncludesSingleQuery(new List<string> { "Related" })
152+
.Skip(0)
153+
.Take(1)
154+
.ToListAsync();
155+
156+
// Assert
157+
var entity = Assert.Single(results);
158+
Assert.Equal(1, entity.Id);
159+
Assert.NotEmpty(entity.Related);
160+
Assert.All(entity.Related, r => Assert.Equal(1, r.TestEntityId));
161+
}
162+
163+
[Fact]
164+
public async Task ApplyIncludesSingleQuery_WithLargeDatasetAndPaginationAtMiddlePage_LoadsCorrectEntitiesAsync()
165+
{
166+
// Arrange
167+
using var context = CreateInMemoryContext();
168+
169+
// Act - Get middle page (page 50) with page size 1
170+
var results = await context
171+
.TestEntities.OrderBy(e => e.Id)
172+
.ApplyIncludesSingleQuery(new List<string> { "Related" })
173+
.Skip(49)
174+
.Take(1)
175+
.ToListAsync();
176+
177+
// Assert
178+
var entity = Assert.Single(results);
179+
Assert.Equal(50, entity.Id);
180+
Assert.NotEmpty(entity.Related);
181+
Assert.All(entity.Related, r => Assert.Equal(50, r.TestEntityId));
182+
}
183+
184+
[Fact]
185+
public async Task ApplyIncludesSingleQuery_WithMultiplePagesSmallPageSize_EachPageHasCorrectIncludesAsync()
186+
{
187+
// Arrange
188+
using var context = CreateInMemoryContext();
189+
const int pageSize = 3;
190+
const int totalPages = 5;
191+
192+
// Act & Assert - Verify each page has correct includes
193+
for (int page = 0; page < totalPages; page++)
194+
{
195+
var results = await context
196+
.TestEntities.OrderBy(e => e.Id)
197+
.ApplyIncludesSingleQuery(new List<string> { "Related" })
198+
.Skip(page * pageSize)
199+
.Take(pageSize)
200+
.ToListAsync();
201+
202+
Assert.Equal(pageSize, results.Count);
203+
204+
foreach (var entity in results)
205+
{
206+
Assert.NotEmpty(entity.Related);
207+
// Verify the related entities belong to this entity
208+
Assert.All(entity.Related, r => Assert.Equal(entity.Id, r.TestEntityId));
209+
}
210+
}
211+
}
212+
213+
[Fact]
214+
public async Task ApplyIncludesSingleQuery_WithNullIncludePaths_ReturnsQueryUnmodifiedAsync()
215+
{
216+
// Arrange
217+
using var context = CreateInMemoryContext();
218+
219+
// Act
220+
var query = context.TestEntities.ApplyIncludesSingleQuery(null);
221+
var results = await query.ToListAsync();
222+
223+
// Assert
224+
Assert.NotEmpty(results);
225+
}
226+
227+
[Fact]
228+
public async Task ApplyIncludesSingleQuery_WithEmptyIncludePaths_ReturnsQueryUnmodifiedAsync()
229+
{
230+
// Arrange
231+
using var context = CreateInMemoryContext();
232+
233+
// Act
234+
var query = context.TestEntities.ApplyIncludesSingleQuery(new List<string>());
235+
var results = await query.ToListAsync();
236+
237+
// Assert
238+
Assert.NotEmpty(results);
239+
}
240+
241+
[Fact]
242+
public async Task ApplyIncludesSingleQuery_WithoutPagination_WorksCorrectlyAsync()
243+
{
244+
// Arrange
245+
using var context = CreateInMemoryContext();
246+
247+
// Act
248+
var results = await context
249+
.TestEntities.OrderBy(e => e.Id)
250+
.ApplyIncludesSingleQuery(new List<string> { "Related" })
251+
.Take(10)
252+
.ToListAsync();
253+
254+
// Assert
255+
Assert.Equal(10, results.Count);
256+
Assert.All(results, e => Assert.NotEmpty(e.Related));
257+
}
258+
}

JsonApiToolkit.Tests/Extensions/QueryHelpersTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public void ConvertToPropertyType_WithInvalidInt_ThrowsFormatException()
7676
);
7777

7878
Assert.Contains(
79-
"Failed to convert 'not-a-number' to type 'System.Int32'",
79+
"Failed to convert filter value 'not-a-number' to type 'System.Int32'",
8080
exception.Message
8181
);
8282
}

JsonApiToolkit.Tests/Integration/AllowedIncludesIntegrationTests.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,15 @@
44
using JsonApiToolkit.Controllers;
55
using JsonApiToolkit.Extensions;
66
using JsonApiToolkit.Models.Errors;
7+
using JsonApiToolkit.Services;
78
using Microsoft.AspNetCore.Builder;
89
using Microsoft.AspNetCore.Hosting;
910
using Microsoft.AspNetCore.Mvc;
1011
using Microsoft.AspNetCore.TestHost;
1112
using Microsoft.EntityFrameworkCore;
1213
using Microsoft.Extensions.DependencyInjection;
1314
using Microsoft.Extensions.Hosting;
15+
using Microsoft.Extensions.Logging;
1416

1517
namespace JsonApiToolkit.Tests.Integration;
1618

0 commit comments

Comments
 (0)