From a716661479ce0a446542fa528200a1997f731b7a Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 29 Jan 2026 11:22:05 +1100 Subject: [PATCH 1/5] Add simplified filter API for key-only filters Introduces a simplified Filters.Add(filter: ...) API for filters that only access primary key or foreign key properties, reducing boilerplate and improving clarity. Adds analyzer diagnostics (GQLEF004, GQLEF005, GQLEF006) to enforce and suggest correct usage, updates documentation and snippets, and provides comprehensive analyzer tests and integration tests for the new API. --- docs/filters.md | 149 +++++ docs/mdsource/filters.source.md | 90 +++ .../FilterIdentityProjectionAnalyzerTests.cs | 613 ++++++++++++++++++ .../AnalyzerReleases.Unshipped.md | 3 + .../DiagnosticDescriptors.cs | 30 + .../FilterIdentityProjectionAnalyzer.cs | 436 +++++++++++++ .../Filters/Filters.cs | 45 ++ src/Snippets/GlobalFilterSnippets.cs | 63 ++ ...d_filter_async_with_fk_access.verified.txt | 23 + ...ombined_with_existing_filters.verified.txt | 22 + ...er_connection_query_filtering.verified.txt | 40 ++ ...d_filter_list_query_filtering.verified.txt | 22 + ...ied_filter_multiple_fk_access.verified.txt | 23 + ...d_filter_nullable_fk_handling.verified.txt | 23 + ...ed_filter_sync_with_pk_access.verified.txt | 23 + .../IntegrationTests_simplified_filter_api.cs | 208 ++++++ 16 files changed, 1813 insertions(+) create mode 100644 src/GraphQL.EntityFramework.Analyzers.Tests/FilterIdentityProjectionAnalyzerTests.cs create mode 100644 src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs create mode 100644 src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_async_with_fk_access.verified.txt create mode 100644 src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_combined_with_existing_filters.verified.txt create mode 100644 src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_connection_query_filtering.verified.txt create mode 100644 src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_list_query_filtering.verified.txt create mode 100644 src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_multiple_fk_access.verified.txt create mode 100644 src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_nullable_fk_handling.verified.txt create mode 100644 src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_sync_with_pk_access.verified.txt create mode 100644 src/Tests/IntegrationTests/IntegrationTests_simplified_filter_api.cs diff --git a/docs/filters.md b/docs/filters.md index 52bb61c4..292b8b17 100644 --- a/docs/filters.md +++ b/docs/filters.md @@ -407,3 +407,152 @@ This is useful when: * User permissions are stored in the database * Filter logic requires async operations * Complex checks involve multiple data sources + + +## Simplified Filter API + +For filters that only need to access **primary key** or **foreign key** properties, a simplified API is available that eliminates the need to specify a projection: + + + +```cs +public class Accommodation +{ + public Guid Id { get; set; } + public Guid? LocationId { get; set; } + public string? City { get; set; } + public int Capacity { get; set; } +} +``` +snippet source | anchor + +```cs +var filters = new Filters(); + +// VALID: Simplified API with primary key access +filters.Add( + filter: (_, _, _, a) => a.Id != Guid.Empty); + +// VALID: Simplified API with foreign key access +var allowedLocationId = Guid.NewGuid(); +filters.Add( + filter: (_, _, _, a) => a.LocationId == allowedLocationId); + +// VALID: Simplified API with nullable foreign key check +filters.Add( + filter: (_, _, _, a) => a.LocationId != null); + +// INVALID: Simplified API accessing scalar property (will cause runtime error!) +// filters.Add( +// filter: (_, _, _, a) => a.City == "London"); // ERROR: City is not a key + +// INVALID: Simplified API accessing scalar property (will cause runtime error!) +// filters.Add( +// filter: (_, _, _, a) => a.Capacity > 10); // ERROR: Capacity is not a key + +// For non-key properties, use the full API with projection: +filters.For().Add( + projection: a => a.City, + filter: (_, _, _, city) => city == "London"); + +filters.For().Add( + projection: a => new { a.City, a.Capacity }, + filter: (_, _, _, x) => x.City == "London" && x.Capacity > 10); + +// COMPARISON: These are equivalent when filter only accesses keys +filters.Add( + filter: (_, _, _, a) => a.Id != Guid.Empty); +// Equivalent to: +filters.For().Add( + projection: _ => _, // Identity projection + filter: (_, _, _, a) => a.Id != Guid.Empty); + +EfGraphQLConventions.RegisterInContainer( + services, + resolveFilters: _ => filters); +``` +snippet source | anchor + + +### When to Use the Simplified API + +Use `filters.Add(filter: ...)` when the filter **only** accesses: + +* **Primary keys**: `Id`, `EntityId`, `CompanyId` (matching the entity type name) +* **Foreign keys**: Properties ending with `Id` like `ParentId`, `CategoryId`, `LocationId` + +The simplified API uses identity projection (`_ => _`) internally, which in EF projections only guarantees that key properties are loaded. + +### Key Property Detection Rules + +A property is considered a **primary key** if it is: + +* Named `Id` +* Named `{TypeName}Id` (e.g., `CompanyId` for `Company` entity) +* Named `{TypeName}Id` where TypeName has suffix removed: `Entity`, `Model`, `Dto` + * Example: `CompanyId` in `CompanyEntity` class + +A property is considered a **foreign key** if: + +* Name ends with `Id` (but is not solely `Id`) +* Not identified as a primary key +* Type is `int`, `long`, `short`, or `Guid` (nullable or non-nullable) + +### Restrictions + +**IMPORTANT**: Do not access scalar properties (like `Name`, `City`, `Capacity`) or navigation properties (like `Parent`, `Category`) with the simplified API. These properties are not loaded by identity projection and will cause runtime errors. + +For non-key properties, use the full API with explicit projection: + +```csharp +// INVALID - Will cause runtime error +filters.Add( + filter: (_, _, _, a) => a.City == "London"); // City is NOT a key + +// VALID - Explicit projection for scalar properties +filters.For().Add( + projection: a => a.City, + filter: (_, _, _, city) => city == "London"); +``` + +### Comparison with Full API + +The simplified API is syntactic sugar for the identity projection pattern: + +```csharp +// Simplified API +filters.Add( + filter: (_, _, _, a) => a.Id != Guid.Empty); + +// Equivalent full API +filters.For().Add( + projection: _ => _, // Identity projection + filter: (_, _, _, a) => a.Id != Guid.Empty); +``` + +### Analyzer Support + +Three analyzer diagnostics help ensure correct usage: + +* **GQLEF004** (Info): Suggests using the simplified API when identity projection only accesses keys +* **GQLEF005** (Error): Prevents accessing non-key properties with simplified API +* **GQLEF006** (Error): Prevents accessing non-key properties with identity projection + +### Migration Guide + +Existing code using identity projection with filters that only access keys can be migrated to the simplified API: + +**Before:** +```csharp +filters.For().Add( + projection: _ => _, + filter: (_, _, _, p) => p.CategoryId == allowedCategoryId); +``` + +**After:** +```csharp +filters.Add( + filter: (_, _, _, p) => p.CategoryId == allowedCategoryId); +``` + +The simplified API makes intent clearer and reduces boilerplate while maintaining the same runtime behavior diff --git a/docs/mdsource/filters.source.md b/docs/mdsource/filters.source.md index 44499257..d982647f 100644 --- a/docs/mdsource/filters.source.md +++ b/docs/mdsource/filters.source.md @@ -145,3 +145,93 @@ This is useful when: * User permissions are stored in the database * Filter logic requires async operations * Complex checks involve multiple data sources + + +## Simplified Filter API + +For filters that only need to access **primary key** or **foreign key** properties, a simplified API is available that eliminates the need to specify a projection: + +snippet: simplified-filter-api + +### When to Use the Simplified API + +Use `filters.Add(filter: ...)` when the filter **only** accesses: + +* **Primary keys**: `Id`, `EntityId`, `CompanyId` (matching the entity type name) +* **Foreign keys**: Properties ending with `Id` like `ParentId`, `CategoryId`, `LocationId` + +The simplified API uses identity projection (`_ => _`) internally, which in EF projections only guarantees that key properties are loaded. + +### Key Property Detection Rules + +A property is considered a **primary key** if it is: + +* Named `Id` +* Named `{TypeName}Id` (e.g., `CompanyId` for `Company` entity) +* Named `{TypeName}Id` where TypeName has suffix removed: `Entity`, `Model`, `Dto` + * Example: `CompanyId` in `CompanyEntity` class + +A property is considered a **foreign key** if: + +* Name ends with `Id` (but is not solely `Id`) +* Not identified as a primary key +* Type is `int`, `long`, `short`, or `Guid` (nullable or non-nullable) + +### Restrictions + +**IMPORTANT**: Do not access scalar properties (like `Name`, `City`, `Capacity`) or navigation properties (like `Parent`, `Category`) with the simplified API. These properties are not loaded by identity projection and will cause runtime errors. + +For non-key properties, use the full API with explicit projection: + +```csharp +// INVALID - Will cause runtime error +filters.Add( + filter: (_, _, _, a) => a.City == "London"); // City is NOT a key + +// VALID - Explicit projection for scalar properties +filters.For().Add( + projection: a => a.City, + filter: (_, _, _, city) => city == "London"); +``` + +### Comparison with Full API + +The simplified API is syntactic sugar for the identity projection pattern: + +```csharp +// Simplified API +filters.Add( + filter: (_, _, _, a) => a.Id != Guid.Empty); + +// Equivalent full API +filters.For().Add( + projection: _ => _, // Identity projection + filter: (_, _, _, a) => a.Id != Guid.Empty); +``` + +### Analyzer Support + +Three analyzer diagnostics help ensure correct usage: + +* **GQLEF004** (Info): Suggests using the simplified API when identity projection only accesses keys +* **GQLEF005** (Error): Prevents accessing non-key properties with simplified API +* **GQLEF006** (Error): Prevents accessing non-key properties with identity projection + +### Migration Guide + +Existing code using identity projection with filters that only access keys can be migrated to the simplified API: + +**Before:** +```csharp +filters.For().Add( + projection: _ => _, + filter: (_, _, _, p) => p.CategoryId == allowedCategoryId); +``` + +**After:** +```csharp +filters.Add( + filter: (_, _, _, p) => p.CategoryId == allowedCategoryId); +``` + +The simplified API makes intent clearer and reduces boilerplate while maintaining the same runtime behavior diff --git a/src/GraphQL.EntityFramework.Analyzers.Tests/FilterIdentityProjectionAnalyzerTests.cs b/src/GraphQL.EntityFramework.Analyzers.Tests/FilterIdentityProjectionAnalyzerTests.cs new file mode 100644 index 00000000..c43d2886 --- /dev/null +++ b/src/GraphQL.EntityFramework.Analyzers.Tests/FilterIdentityProjectionAnalyzerTests.cs @@ -0,0 +1,613 @@ +using GraphQL.EntityFramework; + +public class FilterIdentityProjectionAnalyzerTests +{ + // GQLEF004 Tests - Suggest simplified API + + [Fact] + public async Task GQLEF004_SuggestsSimplifiedAPI_ForIdentityProjectionWithPKAccess() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public class TestEntity + { + public Guid Id { get; set; } + public string Name { get; set; } = ""; + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters, Guid targetId) + { + filters.For().Add( + projection: _ => _, + filter: (_, _, _, e) => e.Id == targetId); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Single(diagnostics); + Assert.Equal("GQLEF004", diagnostics[0].Id); + Assert.Contains("TestEntity", diagnostics[0].GetMessage()); + } + + [Fact] + public async Task GQLEF004_SuggestsSimplifiedAPI_ForIdentityProjectionWithFKAccess() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public class TestEntity + { + public Guid Id { get; set; } + public Guid? ParentId { get; set; } + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters, Guid targetId) + { + filters.For().Add( + projection: _ => _, + filter: (_, _, _, e) => e.ParentId == targetId); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Single(diagnostics); + Assert.Equal("GQLEF004", diagnostics[0].Id); + } + + [Fact] + public async Task GQLEF004_SuggestsSimplifiedAPI_ForIdentityProjectionWithMultipleFKAccess() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public class TestEntity + { + public Guid Id { get; set; } + public Guid? ParentId { get; set; } + public int? CategoryId { get; set; } + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters, Guid parentId, int categoryId) + { + filters.For().Add( + projection: _ => _, + filter: (_, _, _, e) => e.ParentId == parentId || e.CategoryId == categoryId); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Single(diagnostics); + Assert.Equal("GQLEF004", diagnostics[0].Id); + } + + [Fact] + public async Task GQLEF004_NoWarning_ForNonIdentityProjection() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public class TestEntity + { + public Guid Id { get; set; } + public string Name { get; set; } = ""; + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters) + { + filters.For().Add( + projection: e => e.Name, + filter: (_, _, _, name) => name != ""); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Empty(diagnostics); + } + + // GQLEF005 Tests - Invalid simplified API usage + + [Fact] + public async Task GQLEF005_Error_ForSimplifiedAPIAccessingScalarProperty() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public class TestEntity + { + public Guid Id { get; set; } + public string Name { get; set; } = ""; + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters) + { + filters.Add( + filter: (_, _, _, e) => e.Name == "Test"); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Single(diagnostics); + Assert.Equal("GQLEF005", diagnostics[0].Id); + Assert.Contains("Name", diagnostics[0].GetMessage()); + } + + [Fact] + public async Task GQLEF005_Error_ForSimplifiedAPIAccessingNavigationProperty() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public class ParentEntity { public Guid Id { get; set; } } + public class TestEntity + { + public Guid Id { get; set; } + public Guid? ParentId { get; set; } + public ParentEntity? Parent { get; set; } + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters) + { + filters.Add( + filter: (_, _, _, e) => e.Parent != null); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Single(diagnostics); + Assert.Equal("GQLEF005", diagnostics[0].Id); + Assert.Contains("Parent", diagnostics[0].GetMessage()); + } + + [Fact] + public async Task GQLEF005_NoError_ForSimplifiedAPIAccessingPK() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public class TestEntity + { + public Guid Id { get; set; } + public string Name { get; set; } = ""; + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters, Guid targetId) + { + filters.Add( + filter: (_, _, _, e) => e.Id == targetId); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Empty(diagnostics); + } + + [Fact] + public async Task GQLEF005_NoError_ForSimplifiedAPIAccessingFK() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public class TestEntity + { + public Guid Id { get; set; } + public Guid? ParentId { get; set; } + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters, Guid parentId) + { + filters.Add( + filter: (_, _, _, e) => e.ParentId == parentId); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Empty(diagnostics); + } + + [Fact] + public async Task GQLEF005_NoError_ForSimplifiedAPIAccessingNullableFK() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public class TestEntity + { + public Guid Id { get; set; } + public int? CategoryId { get; set; } + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters) + { + filters.Add( + filter: (_, _, _, e) => e.CategoryId != null); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Empty(diagnostics); + } + + // GQLEF006 Tests - Invalid identity projection with existing API + + [Fact] + public async Task GQLEF006_Error_ForIdentityProjectionAccessingScalarProperty() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public class TestEntity + { + public Guid Id { get; set; } + public string City { get; set; } = ""; + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters) + { + filters.For().Add( + projection: _ => _, + filter: (_, _, _, e) => e.City == "London"); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Single(diagnostics); + Assert.Equal("GQLEF006", diagnostics[0].Id); + Assert.Contains("City", diagnostics[0].GetMessage()); + } + + [Fact] + public async Task GQLEF006_Error_ForIdentityProjectionAccessingNavigationProperty() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public class ParentEntity { public Guid Id { get; set; } } + public class TestEntity + { + public Guid Id { get; set; } + public ParentEntity? Parent { get; set; } + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters) + { + filters.For().Add( + projection: _ => _, + filter: (_, _, _, e) => e.Parent != null); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Single(diagnostics); + Assert.Equal("GQLEF006", diagnostics[0].Id); + Assert.Contains("Parent", diagnostics[0].GetMessage()); + } + + // Edge Cases + + [Fact] + public async Task EdgeCase_DiscardParameter() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public class TestEntity + { + public Guid Id { get; set; } + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters) + { + filters.Add( + filter: (_, _, _, _) => true); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Empty(diagnostics); + } + + [Fact] + public async Task EdgeCase_TypeNameWithEntitySuffix() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public class CompanyEntity + { + public Guid CompanyId { get; set; } + public string Name { get; set; } = ""; + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters, Guid companyId) + { + filters.Add( + filter: (_, _, _, e) => e.CompanyId == companyId); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Empty(diagnostics); + } + + [Fact] + public async Task EdgeCase_ComplexExpression() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public class TestEntity + { + public Guid Id { get; set; } + public Guid? ParentId { get; set; } + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public bool IsValidId(Guid? id) => id.HasValue; + + public void ConfigureFilters(Filters filters) + { + filters.Add( + filter: (_, _, _, e) => IsValidId(e.ParentId)); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Empty(diagnostics); + } + + [Fact] + public async Task EdgeCase_MultiplePropertyAccess() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + + public class TestEntity + { + public Guid Id { get; set; } + public Guid? ParentId { get; set; } + public int? CategoryId { get; set; } + public string Name { get; set; } = ""; + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters, Guid parentId) + { + filters.For().Add( + projection: _ => _, + filter: (_, _, _, e) => e.ParentId == parentId && e.Name == "Test"); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + // Should report error for Name access (first non-key property encountered) + Assert.Single(diagnostics); + Assert.Equal("GQLEF006", diagnostics[0].Id); + Assert.Contains("Name", diagnostics[0].GetMessage()); + } + + [Fact] + public async Task EdgeCase_AsyncFilter() + { + var source = """ + using GraphQL.EntityFramework; + using Microsoft.EntityFrameworkCore; + using System; + using System.Threading.Tasks; + + public class TestEntity + { + public Guid Id { get; set; } + public Guid? ParentId { get; set; } + } + + public class TestDbContext : DbContext { } + + public class TestClass + { + public void ConfigureFilters(Filters filters, Guid parentId) + { + filters.Add( + filter: async (_, _, _, e) => + { + await Task.Delay(1); + return e.ParentId == parentId; + }); + } + } + """; + + var diagnostics = await GetDiagnosticsAsync(source); + Assert.Empty(diagnostics); + } + + static async Task GetDiagnosticsAsync(string source) + { + var syntaxTree = CSharpSyntaxTree.ParseText(source); + + var references = new List + { + MetadataReference.CreateFromFile(typeof(object).Assembly.Location), + MetadataReference.CreateFromFile(typeof(DbContext).Assembly.Location) + }; + + // Add GraphQL.EntityFramework assembly + var efAssembly = typeof(Filters<>).Assembly; + references.Add(MetadataReference.CreateFromFile(efAssembly.Location)); + + // Add required assemblies for compilation + var requiredAssemblies = new[] + { + typeof(Guid).Assembly, + // System.Runtime + Assembly.Load("System.Runtime"), + // System.Linq.Expressions + typeof(IQueryable<>).Assembly, + // System.Security.Claims + Assembly.Load("System.Security.Claims"), + }; + + foreach (var assembly in requiredAssemblies) + { + if (!string.IsNullOrEmpty(assembly.Location)) + { + references.Add(MetadataReference.CreateFromFile(assembly.Location)); + } + } + + // Add all System.* and Microsoft.* assemblies except those that conflict + foreach (var assembly in AppDomain.CurrentDomain.GetAssemblies()) + { + if (!assembly.IsDynamic && + !string.IsNullOrEmpty(assembly.Location) && + references.All(_ => _.Display != assembly.Location)) + { + var name = assembly.GetName().Name ?? ""; + if ((!name.StartsWith("System.") && + !name.StartsWith("Microsoft.")) || + name.Contains("xunit", StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + try + { + references.Add(MetadataReference.CreateFromFile(assembly.Location)); + } + catch + { + // Ignore assemblies that can't be referenced + } + } + } + + var compilation = CSharpCompilation.Create( + "TestAssembly", + syntaxTrees: [syntaxTree], + references: references, + options: new( + OutputKind.DynamicallyLinkedLibrary, + nullableContextOptions: NullableContextOptions.Enable)); + + var analyzer = new FilterIdentityProjectionAnalyzer(); + var compilationWithAnalyzers = compilation.WithAnalyzers([analyzer]); + + var allDiagnostics = await compilationWithAnalyzers.GetAllDiagnosticsAsync(); + + // Check for compilation errors + var compilationErrors = allDiagnostics.Where(_ => _.Severity == DiagnosticSeverity.Error && !_.Id.StartsWith("GQLEF")).ToArray(); + if (compilationErrors.Length > 0) + { + var errorMessages = string.Join("\n", compilationErrors.Select(_ => $"{_.Id}: {_.GetMessage()}")); + throw new($"Compilation errors:\n{errorMessages}"); + } + + // Filter to only GQLEF004, GQLEF005, and GQLEF006 diagnostics + return allDiagnostics + .Where(_ => _.Id == "GQLEF004" || _.Id == "GQLEF005" || _.Id == "GQLEF006") + .ToArray(); + } +} diff --git a/src/GraphQL.EntityFramework.Analyzers/AnalyzerReleases.Unshipped.md b/src/GraphQL.EntityFramework.Analyzers/AnalyzerReleases.Unshipped.md index 08829c9b..5da040a4 100644 --- a/src/GraphQL.EntityFramework.Analyzers/AnalyzerReleases.Unshipped.md +++ b/src/GraphQL.EntityFramework.Analyzers/AnalyzerReleases.Unshipped.md @@ -7,3 +7,6 @@ Rule ID | Category | Severity | Notes --------|----------|----------|-------------------- GQLEF002 | Usage | Warning | Use projection-based Resolve extension methods when accessing navigation properties GQLEF003 | Usage | Error | Identity projection is not allowed in projection-based Resolve methods +GQLEF004 | Usage | Info | Suggests using simplified filter API when identity projection only accesses keys +GQLEF005 | Usage | Error | Prevents accessing non-key properties with simplified filter API +GQLEF006 | Usage | Error | Prevents accessing non-key properties with identity projection diff --git a/src/GraphQL.EntityFramework.Analyzers/DiagnosticDescriptors.cs b/src/GraphQL.EntityFramework.Analyzers/DiagnosticDescriptors.cs index e1ef786d..e87d5aef 100644 --- a/src/GraphQL.EntityFramework.Analyzers/DiagnosticDescriptors.cs +++ b/src/GraphQL.EntityFramework.Analyzers/DiagnosticDescriptors.cs @@ -19,4 +19,34 @@ static class DiagnosticDescriptors isEnabledByDefault: true, description: "Using '_ => _' as the projection parameter in projection-based Resolve extension methods is not allowed because it doesn't load any additional navigation properties. If you only need to access primary key or foreign key properties, use the regular Resolve() method instead. If you need to access navigation properties, specify them in the projection (e.g., 'x => x.Parent').", helpLinkUri: "https://github.com/SimonCropp/GraphQL.EntityFramework#projection-based-resolve"); + + public static readonly DiagnosticDescriptor GQLEF004 = new( + id: "GQLEF004", + title: "Use simplified filter API for identity projections", + messageFormat: "Identity projection '_ => _' detected. Consider using simplified API: filters.Add<{0}>(filter: ...).", + category: "Usage", + defaultSeverity: DiagnosticSeverity.Info, + isEnabledByDefault: true, + description: "When using identity projection '_ => _' with a filter that only accesses primary key or foreign key properties, use the simplified API filters.Add(filter: ...) instead.", + helpLinkUri: "https://github.com/SimonCropp/GraphQL.EntityFramework#simplified-filter-api"); + + public static readonly DiagnosticDescriptor GQLEF005 = new( + id: "GQLEF005", + title: "Filter with simplified API must only access primary key or foreign key properties", + messageFormat: "Filter accesses '{0}' which is not a primary key or foreign key. The simplified filter API uses identity projection, which only loads keys.", + category: "Usage", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "The simplified filter API filters.Add(filter: ...) uses identity projection internally, which only guarantees that primary keys and foreign keys are loaded. Use filters.For().Add(projection: ..., filter: ...) to project required properties.", + helpLinkUri: "https://github.com/SimonCropp/GraphQL.EntityFramework#simplified-filter-api"); + + public static readonly DiagnosticDescriptor GQLEF006 = new( + id: "GQLEF006", + title: "Identity projection with filter that accesses non-key properties is invalid", + messageFormat: "Filter accesses '{0}' which is not a primary key or foreign key, but projection is '_ => _'. Project the required properties instead.", + category: "Usage", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "When using identity projection '_ => _', only primary keys and foreign keys are guaranteed to be loaded. Either project specific properties needed or use the simplified API if you only need key properties.", + helpLinkUri: "https://github.com/SimonCropp/GraphQL.EntityFramework#identity-projection-filters"); } diff --git a/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs b/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs new file mode 100644 index 00000000..acaac647 --- /dev/null +++ b/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs @@ -0,0 +1,436 @@ +namespace GraphQL.EntityFramework.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class FilterIdentityProjectionAnalyzer : DiagnosticAnalyzer +{ + public override ImmutableArray SupportedDiagnostics { get; } = + [ + DiagnosticDescriptors.GQLEF004, + DiagnosticDescriptors.GQLEF005, + DiagnosticDescriptors.GQLEF006 + ]; + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterSyntaxNodeAction(AnalyzeInvocation, SyntaxKind.InvocationExpression); + } + + static void AnalyzeInvocation(SyntaxNodeAnalysisContext context) + { + var invocation = (InvocationExpressionSyntax)context.Node; + + // Detect three patterns: + // 1. filters.Add(filter: ...) - validate PK/FK only (GQLEF005) + // 2. filters.For().Add(projection: _ => _, filter: ...) - suggest or error (GQLEF004/GQLEF006) + + if (IsSimplifiedFilterAdd(invocation, context.SemanticModel, out var filterLambda1, out var entityType1)) + { + // Pattern 1: filters.Add(filter: ...) + // Validate that filter only accesses PK/FK properties + if (filterLambda1 != null) + { + var nonKeyProperty = FindNonKeyPropertyAccess(filterLambda1, context.SemanticModel); + if (nonKeyProperty != null) + { + var diagnostic = Diagnostic.Create( + DiagnosticDescriptors.GQLEF005, + invocation.GetLocation(), + nonKeyProperty); + context.ReportDiagnostic(diagnostic); + } + } + } + else if (IsFilterBuilderAdd(invocation, context.SemanticModel, out var projectionLambda, out var filterLambda2, out var entityType2)) + { + // Pattern 2: filters.For().Add(...) + if (IsIdentityProjection(projectionLambda)) + { + // Identity projection detected + if (filterLambda2 != null) + { + var nonKeyProperty = FindNonKeyPropertyAccess(filterLambda2, context.SemanticModel); + if (nonKeyProperty != null) + { + // Error: Identity projection with non-key access + var diagnostic = Diagnostic.Create( + DiagnosticDescriptors.GQLEF006, + invocation.GetLocation(), + nonKeyProperty); + context.ReportDiagnostic(diagnostic); + } + else if (!string.IsNullOrEmpty(entityType2)) + { + // Info: Suggest simplified API + var diagnostic = Diagnostic.Create( + DiagnosticDescriptors.GQLEF004, + invocation.GetLocation(), + entityType2); + context.ReportDiagnostic(diagnostic); + } + } + } + } + } + + static bool IsSimplifiedFilterAdd( + InvocationExpressionSyntax invocation, + SemanticModel semanticModel, + out LambdaExpressionSyntax? filterLambda, + out string? entityType) + { + filterLambda = null; + entityType = null; + + // Check method name is Add + if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess) + { + return false; + } + + if (memberAccess.Name.Identifier.Text != "Add") + { + return false; + } + + // Get the symbol info + var symbolInfo = semanticModel.GetSymbolInfo(invocation); + if (symbolInfo.Symbol is not IMethodSymbol methodSymbol) + { + return false; + } + + // Check if it's the simplified API: filters.Add(filter: ...) + // The containing type should be Filters + var containingType = methodSymbol.ContainingType; + if (containingType == null || + containingType.Name != "Filters" || + containingType.ContainingNamespace?.ToString() != "GraphQL.EntityFramework") + { + return false; + } + + // Check method signature: + // - Should have 1 type parameter (TEntity) + // - Should have 1 parameter named "filter" + if (methodSymbol.TypeArguments.Length != 1 || + methodSymbol.Parameters.Length != 1 || + methodSymbol.Parameters[0].Name != "filter") + { + return false; + } + + // Extract the entity type name + entityType = methodSymbol.TypeArguments[0].Name; + + // Extract filter lambda + if (invocation.ArgumentList.Arguments.Count > 0) + { + var firstArg = invocation.ArgumentList.Arguments[0].Expression; + if (firstArg is LambdaExpressionSyntax lambda) + { + filterLambda = lambda; + } + } + + return true; + } + + static bool IsFilterBuilderAdd( + InvocationExpressionSyntax invocation, + SemanticModel semanticModel, + out LambdaExpressionSyntax? projectionLambda, + out LambdaExpressionSyntax? filterLambda, + out string? entityType) + { + projectionLambda = null; + filterLambda = null; + entityType = null; + + // Check method name is Add + if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess) + { + return false; + } + + if (memberAccess.Name.Identifier.Text != "Add") + { + return false; + } + + // Get the symbol info + var symbolInfo = semanticModel.GetSymbolInfo(invocation); + if (symbolInfo.Symbol is not IMethodSymbol methodSymbol) + { + return false; + } + + // Check if it's the FilterBuilder API: filters.For().Add(projection: ..., filter: ...) + // The containing type should be FilterBuilder + var containingType = methodSymbol.ContainingType; + if (containingType == null || + containingType.Name != "FilterBuilder" || + containingType.ContainingNamespace?.ToString() != "GraphQL.EntityFramework") + { + return false; + } + + // Extract entity type from FilterBuilder + if (containingType.TypeArguments.Length >= 2) + { + entityType = containingType.TypeArguments[1].Name; + } + + // Find the projection and filter parameters + var projectionParameter = methodSymbol.Parameters.FirstOrDefault(p => p.Name == "projection"); + var filterParameter = methodSymbol.Parameters.FirstOrDefault(p => p.Name == "filter"); + + if (projectionParameter == null && filterParameter == null) + { + return false; + } + + // Extract arguments + foreach (var arg in invocation.ArgumentList.Arguments) + { + if (arg.NameColon?.Name.Identifier.Text == "projection" && arg.Expression is LambdaExpressionSyntax projLambda) + { + projectionLambda = projLambda; + } + else if (arg.NameColon?.Name.Identifier.Text == "filter" && arg.Expression is LambdaExpressionSyntax filtLambda) + { + filterLambda = filtLambda; + } + } + + // Try positional arguments if named not found + if (projectionLambda == null && projectionParameter != null) + { + var parameterIndex = Array.IndexOf(methodSymbol.Parameters.ToArray(), projectionParameter); + if (parameterIndex >= 0 && parameterIndex < invocation.ArgumentList.Arguments.Count) + { + var arg = invocation.ArgumentList.Arguments[parameterIndex]; + if (arg.Expression is LambdaExpressionSyntax lambda) + { + projectionLambda = lambda; + } + } + } + + if (filterLambda == null && filterParameter != null) + { + var parameterIndex = Array.IndexOf(methodSymbol.Parameters.ToArray(), filterParameter); + if (parameterIndex >= 0 && parameterIndex < invocation.ArgumentList.Arguments.Count) + { + var arg = invocation.ArgumentList.Arguments[parameterIndex]; + if (arg.Expression is LambdaExpressionSyntax lambda) + { + filterLambda = lambda; + } + } + } + + return projectionLambda != null || filterLambda != null; + } + + static bool IsIdentityProjection(LambdaExpressionSyntax? lambda) + { + if (lambda == null) + { + return false; + } + + // Check if it's an identity projection (x => x or _ => _) + // Lambda body should be a simple parameter reference that matches the lambda parameter + if (lambda.Body is not IdentifierNameSyntax identifier) + { + return false; + } + + // Get the parameter name (e.g., "x", "_", "item") + var parameterName = lambda is SimpleLambdaExpressionSyntax simpleLambda + ? simpleLambda.Parameter.Identifier.Text + : lambda is ParenthesizedLambdaExpressionSyntax { ParameterList.Parameters.Count: 1 } parenthesizedLambda + ? parenthesizedLambda.ParameterList.Parameters[0].Identifier.Text + : null; + + // Check if the body references the same parameter + return parameterName != null && identifier.Identifier.Text == parameterName; + } + + static string? FindNonKeyPropertyAccess(LambdaExpressionSyntax lambda, SemanticModel semanticModel) + { + var body = lambda.Body; + + // Get the filter lambda parameter name (last parameter in the filter signature) + string? filterParameterName = null; + if (lambda is SimpleLambdaExpressionSyntax simpleLambda) + { + filterParameterName = simpleLambda.Parameter.Identifier.Text; + } + else if (lambda is ParenthesizedLambdaExpressionSyntax parenthesizedLambda) + { + // Filter signature: (userContext, data, userPrincipal, entity) => ... + // The last parameter is the entity + var parameterCount = parenthesizedLambda.ParameterList.Parameters.Count; + if (parameterCount > 0) + { + filterParameterName = parenthesizedLambda.ParameterList.Parameters[parameterCount - 1].Identifier.Text; + } + } + + if (filterParameterName == null) + { + return null; + } + + // Find all member access expressions in the lambda + var memberAccesses = body.DescendantNodesAndSelf() + .OfType(); + + foreach (var memberAccess in memberAccesses) + { + // Check if this is accessing the filter parameter (e.g., e.Property, entity.Id) + if (!IsFilterParameterAccess(memberAccess, filterParameterName, out var propertyAccess)) + { + continue; + } + + if (propertyAccess == null) + { + continue; + } + + // Get the symbol for the property being accessed + var symbolInfo = semanticModel.GetSymbolInfo(propertyAccess); + if (symbolInfo.Symbol is not IPropertySymbol propertySymbol) + { + continue; + } + + // Check if this property is NOT a key property + if (!IsPrimaryKeyProperty(propertySymbol) && !IsForeignKeyProperty(propertySymbol)) + { + return propertySymbol.Name; + } + } + + return null; + } + + static bool IsFilterParameterAccess( + MemberAccessExpressionSyntax memberAccess, + string filterParameterName, + out MemberAccessExpressionSyntax? propertyAccess) + { + propertyAccess = null; + + // Check if the expression directly references the filter parameter + // e.g., e.Property or entity.Id + if (memberAccess.Expression is IdentifierNameSyntax identifier && + identifier.Identifier.Text == filterParameterName) + { + propertyAccess = memberAccess; + return true; + } + + // Check for nested access (e.g., e.Parent.Id) + // We'll detect the first level property access + if (memberAccess.Expression is MemberAccessExpressionSyntax nestedAccess && + nestedAccess.Expression is IdentifierNameSyntax nestedIdentifier && + nestedIdentifier.Identifier.Text == filterParameterName) + { + propertyAccess = nestedAccess; + return true; + } + + return false; + } + + static bool IsPrimaryKeyProperty(IPropertySymbol propertySymbol) + { + var name = propertySymbol.Name; + + // Simple "Id" is always a primary key + if (name == "Id") + { + return true; + } + + // EntityId, CompanyId (where Entity/Company is the class name) are primary keys + var containingType = propertySymbol.ContainingType; + if (containingType != null && name.EndsWith("Id") && name.Length > 2) + { + // Check if the property name matches the type name + "Id" + var typeName = containingType.Name; + + if (name == $"{typeName}Id") + { + return true; + } + + // Check if removing common suffixes from typeName + "Id" equals name + if (TryMatchWithoutSuffix(typeName, name, "Entity") || + TryMatchWithoutSuffix(typeName, name, "Model") || + TryMatchWithoutSuffix(typeName, name, "Dto")) + { + return true; + } + } + + return false; + } + + static bool TryMatchWithoutSuffix(string typeName, string propertyName, string suffix) + { + if (!typeName.EndsWith(suffix) || typeName.Length <= suffix.Length) + { + return false; + } + + var baseLength = typeName.Length - suffix.Length; + // Check if propertyName is baseTypeName + "Id" + return propertyName.Length == baseLength + 2 && + typeName.AsSpan(0, baseLength).SequenceEqual(propertyName.AsSpan(0, baseLength)) && + propertyName.EndsWith("Id"); + } + + static bool IsForeignKeyProperty(IPropertySymbol propertySymbol) + { + var name = propertySymbol.Name; + var type = propertySymbol.Type; + + // Foreign keys are typically nullable or non-nullable integer/Guid types ending with "Id" + if (!name.EndsWith("Id") || name == "Id") + { + return false; + } + + // If we already determined it's a primary key, it's not a foreign key + if (IsPrimaryKeyProperty(propertySymbol)) + { + return false; + } + + // Check if the type is a scalar type suitable for FK (int, long, Guid, etc.) + // Unwrap nullable + var underlyingType = type; + if (type is INamedTypeSymbol { OriginalDefinition.SpecialType: SpecialType.System_Nullable_T } namedType) + { + underlyingType = namedType.TypeArguments[0]; + } + + // Foreign keys are typically int, long, Guid + switch (underlyingType.SpecialType) + { + case SpecialType.System_Int32: + case SpecialType.System_Int64: + case SpecialType.System_Int16: + return true; + } + + var typeName = underlyingType.ToString(); + return typeName == "System.Guid"; + } +} diff --git a/src/GraphQL.EntityFramework/Filters/Filters.cs b/src/GraphQL.EntityFramework/Filters/Filters.cs index 968afae3..55174225 100644 --- a/src/GraphQL.EntityFramework/Filters/Filters.cs +++ b/src/GraphQL.EntityFramework/Filters/Filters.cs @@ -38,6 +38,51 @@ public FilterBuilder For() where TEntity : class => new(this); + /// + /// Add a synchronous filter that operates on the entity itself (identity projection). + /// Only primary key and foreign key properties should be accessed in the filter. + /// + /// The entity type to filter. + /// Synchronous filter function that receives the full entity. + /// + /// This simplified API is equivalent to: + /// + /// filters.For<TEntity>().Add( + /// projection: _ => _, + /// filter: (userContext, data, userPrincipal, entity) => /* logic */); + /// + /// IMPORTANT: Only access primary key (Id, EntityId) or foreign key (ParentId, etc.) + /// properties. Accessing scalar or navigation properties will cause runtime errors + /// because they won't be loaded by EF projections. + /// + public void Add(Filter filter) + where TEntity : class => + Add( + projection: _ => _, + filter: filter); + + /// + /// Add an asynchronous filter that operates on the entity itself (identity projection). + /// Only primary key and foreign key properties should be accessed in the filter. + /// + /// The entity type to filter. + /// Asynchronous filter function that receives the full entity. + /// + /// This simplified API is equivalent to: + /// + /// filters.For<TEntity>().Add( + /// projection: _ => _, + /// filter: async (userContext, data, userPrincipal, entity) => /* logic */); + /// + /// IMPORTANT: Only access primary key (Id, EntityId) or foreign key (ParentId, etc.) + /// properties. Accessing scalar or navigation properties will cause runtime errors. + /// + public void Add(AsyncFilter filter) + where TEntity : class => + Add( + projection: _ => _, + filter: filter); + internal void Add( Expression>? projection, AsyncFilter filter) diff --git a/src/Snippets/GlobalFilterSnippets.cs b/src/Snippets/GlobalFilterSnippets.cs index c41bca54..7abb61c8 100644 --- a/src/Snippets/GlobalFilterSnippets.cs +++ b/src/Snippets/GlobalFilterSnippets.cs @@ -304,4 +304,67 @@ public static void AddAsyncFilterWithoutProjection(ServiceCollection services) #endregion } + + #region simplified-filter-api + + public class Accommodation + { + public Guid Id { get; set; } + public Guid? LocationId { get; set; } + public string? City { get; set; } + public int Capacity { get; set; } + } + + #endregion + + public static void AddSimplifiedFilterApi(ServiceCollection services) + { + #region simplified-filter-api + + var filters = new Filters(); + + // VALID: Simplified API with primary key access + filters.Add( + filter: (_, _, _, a) => a.Id != Guid.Empty); + + // VALID: Simplified API with foreign key access + var allowedLocationId = Guid.NewGuid(); + filters.Add( + filter: (_, _, _, a) => a.LocationId == allowedLocationId); + + // VALID: Simplified API with nullable foreign key check + filters.Add( + filter: (_, _, _, a) => a.LocationId != null); + + // INVALID: Simplified API accessing scalar property (will cause runtime error!) + // filters.Add( + // filter: (_, _, _, a) => a.City == "London"); // ERROR: City is not a key + + // INVALID: Simplified API accessing scalar property (will cause runtime error!) + // filters.Add( + // filter: (_, _, _, a) => a.Capacity > 10); // ERROR: Capacity is not a key + + // For non-key properties, use the full API with projection: + filters.For().Add( + projection: a => a.City, + filter: (_, _, _, city) => city == "London"); + + filters.For().Add( + projection: a => new { a.City, a.Capacity }, + filter: (_, _, _, x) => x.City == "London" && x.Capacity > 10); + + // COMPARISON: These are equivalent when filter only accesses keys + filters.Add( + filter: (_, _, _, a) => a.Id != Guid.Empty); + // Equivalent to: + filters.For().Add( + projection: _ => _, // Identity projection + filter: (_, _, _, a) => a.Id != Guid.Empty); + + EfGraphQLConventions.RegisterInContainer( + services, + resolveFilters: _ => filters); + + #endregion + } } diff --git a/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_async_with_fk_access.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_async_with_fk_access.verified.txt new file mode 100644 index 00000000..638ad969 --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_async_with_fk_access.verified.txt @@ -0,0 +1,23 @@ +{ + target: +{ + "data": { + "childEntities": [ + { + "property": "Child1" + }, + { + "property": "Child3" + } + ] + } +}, + sql: { + Text: +select c.Id, + c.ParentId, + c.Property +from ChildEntities as c +order by c.Property + } +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_combined_with_existing_filters.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_combined_with_existing_filters.verified.txt new file mode 100644 index 00000000..223908d9 --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_combined_with_existing_filters.verified.txt @@ -0,0 +1,22 @@ +{ + target: +{ + "data": { + "parentEntitiesFiltered": [ + { + "property": "Entity1" + }, + { + "property": "Entity2" + } + ] + } +}, + sql: { + Text: +select f.Id, + f.Property +from FilterParentEntities as f +order by f.Property + } +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_connection_query_filtering.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_connection_query_filtering.verified.txt new file mode 100644 index 00000000..6b29268e --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_connection_query_filtering.verified.txt @@ -0,0 +1,40 @@ +{ + target: +{ + "data": { + "parentEntitiesConnectionFiltered": { + "edges": [ + { + "node": { + "property": "Entity1" + } + }, + { + "node": { + "property": "Entity3" + } + } + ] + } + } +}, + sql: [ + { + Text: +select COUNT(*) +from FilterParentEntities as f + }, + { + Text: +select f.Id, + f.Property +from FilterParentEntities as f +order by f.Property +offset @p rows fetch next @p1 rows only, + Parameters: { + @p: 0, + @p1: 10 + } + } + ] +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_list_query_filtering.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_list_query_filtering.verified.txt new file mode 100644 index 00000000..223908d9 --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_list_query_filtering.verified.txt @@ -0,0 +1,22 @@ +{ + target: +{ + "data": { + "parentEntitiesFiltered": [ + { + "property": "Entity1" + }, + { + "property": "Entity2" + } + ] + } +}, + sql: { + Text: +select f.Id, + f.Property +from FilterParentEntities as f +order by f.Property + } +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_multiple_fk_access.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_multiple_fk_access.verified.txt new file mode 100644 index 00000000..638ad969 --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_multiple_fk_access.verified.txt @@ -0,0 +1,23 @@ +{ + target: +{ + "data": { + "childEntities": [ + { + "property": "Child1" + }, + { + "property": "Child3" + } + ] + } +}, + sql: { + Text: +select c.Id, + c.ParentId, + c.Property +from ChildEntities as c +order by c.Property + } +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_nullable_fk_handling.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_nullable_fk_handling.verified.txt new file mode 100644 index 00000000..638ad969 --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_nullable_fk_handling.verified.txt @@ -0,0 +1,23 @@ +{ + target: +{ + "data": { + "childEntities": [ + { + "property": "Child1" + }, + { + "property": "Child3" + } + ] + } +}, + sql: { + Text: +select c.Id, + c.ParentId, + c.Property +from ChildEntities as c +order by c.Property + } +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_sync_with_pk_access.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_sync_with_pk_access.verified.txt new file mode 100644 index 00000000..0c4e334c --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Simplified_filter_sync_with_pk_access.verified.txt @@ -0,0 +1,23 @@ +{ + target: +{ + "data": { + "childEntities": [ + { + "property": "Entity1" + }, + { + "property": "Entity3" + } + ] + } +}, + sql: { + Text: +select c.Id, + c.ParentId, + c.Property +from ChildEntities as c +order by c.Property + } +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests_simplified_filter_api.cs b/src/Tests/IntegrationTests/IntegrationTests_simplified_filter_api.cs new file mode 100644 index 00000000..0e748925 --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests_simplified_filter_api.cs @@ -0,0 +1,208 @@ +public partial class IntegrationTests +{ + [Fact] + public async Task Simplified_filter_sync_with_pk_access() + { + var query = + """ + { + childEntities + { + property + } + } + """; + + var entity1 = new ChildEntity { Property = "Entity1" }; + var entity2 = new ChildEntity { Property = "Entity2" }; + var entity3 = new ChildEntity { Property = "Entity3" }; + + var filters = new Filters(); + // Simplified API - filter by primary key + filters.Add( + filter: (_, _, _, e) => e.Id == entity1.Id || e.Id == entity3.Id); + + await using var database = await sqlInstance.Build(); + await RunQuery(database, query, null, filters, false, [entity1, entity2, entity3]); + } + + [Fact] + public async Task Simplified_filter_async_with_fk_access() + { + var parent1 = new ParentEntity { Property = "Parent1" }; + var parent2 = new ParentEntity { Property = "Parent2" }; + + var query = + """ + { + childEntities + { + property + } + } + """; + + var entity1 = new ChildEntity { Property = "Child1", Parent = parent1 }; + var entity2 = new ChildEntity { Property = "Child2", Parent = parent2 }; + var entity3 = new ChildEntity { Property = "Child3", Parent = parent1 }; + + var filters = new Filters(); + // Simplified API - async filter by foreign key + filters.Add( + filter: async (_, _, _, e) => + { + await Task.Delay(1); // Simulate async validation + return e.ParentId == parent1.Id; + }); + + await using var database = await sqlInstance.Build(); + await RunQuery(database, query, null, filters, false, [parent1, parent2, entity1, entity2, entity3]); + } + + [Fact] + public async Task Simplified_filter_multiple_fk_access() + { + var parent1 = new ParentEntity { Property = "Parent1" }; + var parent2 = new ParentEntity { Property = "Parent2" }; + var parent3 = new ParentEntity { Property = "Parent3" }; + + var query = + """ + { + childEntities + { + property + } + } + """; + + var entity1 = new ChildEntity { Property = "Child1", Parent = parent1 }; + var entity2 = new ChildEntity { Property = "Child2", Parent = parent2 }; + var entity3 = new ChildEntity { Property = "Child3", Parent = parent3 }; + + var filters = new Filters(); + // Simplified API - filter with OR logic on foreign keys + filters.Add( + filter: (_, _, _, e) => e.ParentId == parent1.Id || e.ParentId == parent3.Id); + + await using var database = await sqlInstance.Build(); + await RunQuery(database, query, null, filters, false, [parent1, parent2, parent3, entity1, entity2, entity3]); + } + + [Fact] + public async Task Simplified_filter_nullable_fk_handling() + { + var parent1 = new ParentEntity { Property = "Parent1" }; + + var query = + """ + { + childEntities + { + property + } + } + """; + + var entity1 = new ChildEntity { Property = "Child1", Parent = parent1 }; + var entity2 = new ChildEntity { Property = "Child2", ParentId = null }; + var entity3 = new ChildEntity { Property = "Child3", Parent = parent1 }; + + var filters = new Filters(); + // Simplified API - filter with nullable FK check + filters.Add( + filter: (_, _, _, e) => e.ParentId != null); + + await using var database = await sqlInstance.Build(); + await RunQuery(database, query, null, filters, false, [parent1, entity1, entity2, entity3]); + } + + [Fact] + public async Task Simplified_filter_combined_with_existing_filters() + { + var query = + """ + { + parentEntitiesFiltered + { + property + } + } + """; + + var entity1 = new FilterParentEntity { Property = "Entity1" }; + var entity2 = new FilterParentEntity { Property = "Entity2" }; + var entity3 = new FilterParentEntity { Property = "Entity3" }; + + var filters = new Filters(); + // Simplified API filter + filters.Add( + filter: (_, _, _, e) => e.Id != entity2.Id); + + // Traditional API filter with projection + filters.For().Add( + projection: _ => _.Property, + filter: (_, _, _, prop) => prop != "Entity3"); + + await using var database = await sqlInstance.Build(); + await RunQuery(database, query, null, filters, false, [entity1, entity2, entity3]); + } + + [Fact] + public async Task Simplified_filter_list_query_filtering() + { + var query = + """ + { + parentEntitiesFiltered + { + property + } + } + """; + + var entity1 = new FilterParentEntity { Property = "Entity1" }; + var entity2 = new FilterParentEntity { Property = "Entity2" }; + var entity3 = new FilterParentEntity { Property = "Entity3" }; + + var filters = new Filters(); + // Simplified API on list query + filters.Add( + filter: (_, _, _, e) => e.Id == entity1.Id || e.Id == entity2.Id); + + await using var database = await sqlInstance.Build(); + await RunQuery(database, query, null, filters, false, [entity1, entity2, entity3]); + } + + [Fact] + public async Task Simplified_filter_connection_query_filtering() + { + var query = + """ + { + parentEntitiesConnectionFiltered + { + edges + { + node + { + property + } + } + } + } + """; + + var entity1 = new FilterParentEntity { Property = "Entity1" }; + var entity2 = new FilterParentEntity { Property = "Entity2" }; + var entity3 = new FilterParentEntity { Property = "Entity3" }; + + var filters = new Filters(); + // Simplified API on connection query + filters.Add( + filter: (_, _, _, e) => e.Id != entity2.Id); + + await using var database = await sqlInstance.Build(); + await RunQuery(database, query, null, filters, false, [entity1, entity2, entity3]); + } +} From 8343b21f9f1060ae9884cfde2bb1277733eee2a0 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 29 Jan 2026 11:22:37 +1100 Subject: [PATCH 2/5] Update FilterIdentityProjectionAnalyzer.cs --- .../FilterIdentityProjectionAnalyzer.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs b/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs index acaac647..84f77eb1 100644 --- a/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs +++ b/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs @@ -104,8 +104,7 @@ static bool IsSimplifiedFilterAdd( // Check if it's the simplified API: filters.Add(filter: ...) // The containing type should be Filters var containingType = methodSymbol.ContainingType; - if (containingType == null || - containingType.Name != "Filters" || + if (containingType is not { Name: "Filters" } || containingType.ContainingNamespace?.ToString() != "GraphQL.EntityFramework") { return false; @@ -169,8 +168,7 @@ static bool IsFilterBuilderAdd( // Check if it's the FilterBuilder API: filters.For().Add(projection: ..., filter: ...) // The containing type should be FilterBuilder var containingType = methodSymbol.ContainingType; - if (containingType == null || - containingType.Name != "FilterBuilder" || + if (containingType is not { Name: "FilterBuilder" } || containingType.ContainingNamespace?.ToString() != "GraphQL.EntityFramework") { return false; @@ -337,8 +335,7 @@ static bool IsFilterParameterAccess( // Check for nested access (e.g., e.Parent.Id) // We'll detect the first level property access - if (memberAccess.Expression is MemberAccessExpressionSyntax nestedAccess && - nestedAccess.Expression is IdentifierNameSyntax nestedIdentifier && + if (memberAccess.Expression is MemberAccessExpressionSyntax { Expression: IdentifierNameSyntax nestedIdentifier } nestedAccess && nestedIdentifier.Identifier.Text == filterParameterName) { propertyAccess = nestedAccess; From 90a25df4bc10c73d4e25b5b8261dff32b72cfc89 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 29 Jan 2026 11:24:06 +1100 Subject: [PATCH 3/5] Update FilterIdentityProjectionAnalyzer.cs --- .../FilterIdentityProjectionAnalyzer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs b/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs index 84f77eb1..ade3f4e0 100644 --- a/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs +++ b/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs @@ -25,7 +25,7 @@ static void AnalyzeInvocation(SyntaxNodeAnalysisContext context) // 1. filters.Add(filter: ...) - validate PK/FK only (GQLEF005) // 2. filters.For().Add(projection: _ => _, filter: ...) - suggest or error (GQLEF004/GQLEF006) - if (IsSimplifiedFilterAdd(invocation, context.SemanticModel, out var filterLambda1, out var entityType1)) + if (IsSimplifiedFilterAdd(invocation, context.SemanticModel, out var filterLambda1, out _)) { // Pattern 1: filters.Add(filter: ...) // Validate that filter only accesses PK/FK properties From 351200a8f4ad435e842909e1840b5d52a01407fb Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 29 Jan 2026 11:28:33 +1100 Subject: [PATCH 4/5] Update FilterIdentityProjectionAnalyzer.cs --- .../FilterIdentityProjectionAnalyzer.cs | 102 +++++++++--------- 1 file changed, 54 insertions(+), 48 deletions(-) diff --git a/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs b/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs index ade3f4e0..ffd7d950 100644 --- a/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs +++ b/src/GraphQL.EntityFramework.Analyzers/FilterIdentityProjectionAnalyzer.cs @@ -25,51 +25,63 @@ static void AnalyzeInvocation(SyntaxNodeAnalysisContext context) // 1. filters.Add(filter: ...) - validate PK/FK only (GQLEF005) // 2. filters.For().Add(projection: _ => _, filter: ...) - suggest or error (GQLEF004/GQLEF006) - if (IsSimplifiedFilterAdd(invocation, context.SemanticModel, out var filterLambda1, out _)) + if (IsSimplifiedFilterAdd(invocation, context.SemanticModel, out var filterLambda1)) { // Pattern 1: filters.Add(filter: ...) // Validate that filter only accesses PK/FK properties - if (filterLambda1 != null) + if (filterLambda1 == null) { - var nonKeyProperty = FindNonKeyPropertyAccess(filterLambda1, context.SemanticModel); - if (nonKeyProperty != null) - { - var diagnostic = Diagnostic.Create( - DiagnosticDescriptors.GQLEF005, - invocation.GetLocation(), - nonKeyProperty); - context.ReportDiagnostic(diagnostic); - } + return; } + + var nonKeyProperty = FindNonKeyPropertyAccess(filterLambda1, context.SemanticModel); + if (nonKeyProperty == null) + { + return; + } + + var diagnostic = Diagnostic.Create( + DiagnosticDescriptors.GQLEF005, + invocation.GetLocation(), + nonKeyProperty); + context.ReportDiagnostic(diagnostic); + return; } - else if (IsFilterBuilderAdd(invocation, context.SemanticModel, out var projectionLambda, out var filterLambda2, out var entityType2)) + + if (IsFilterBuilderAdd(invocation, context.SemanticModel, out var projectionLambda, out var filterLambda2, out var entityType2)) { // Pattern 2: filters.For().Add(...) - if (IsIdentityProjection(projectionLambda)) + if (!IsIdentityProjection(projectionLambda)) { - // Identity projection detected - if (filterLambda2 != null) - { - var nonKeyProperty = FindNonKeyPropertyAccess(filterLambda2, context.SemanticModel); - if (nonKeyProperty != null) - { - // Error: Identity projection with non-key access - var diagnostic = Diagnostic.Create( - DiagnosticDescriptors.GQLEF006, - invocation.GetLocation(), - nonKeyProperty); - context.ReportDiagnostic(diagnostic); - } - else if (!string.IsNullOrEmpty(entityType2)) - { - // Info: Suggest simplified API - var diagnostic = Diagnostic.Create( - DiagnosticDescriptors.GQLEF004, - invocation.GetLocation(), - entityType2); - context.ReportDiagnostic(diagnostic); - } - } + return; + } + + // Identity projection detected + if (filterLambda2 == null) + { + return; + } + + var nonKeyProperty = FindNonKeyPropertyAccess(filterLambda2, context.SemanticModel); + if (nonKeyProperty != null) + { + // Error: Identity projection with non-key access + var diagnostic = Diagnostic.Create( + DiagnosticDescriptors.GQLEF006, + invocation.GetLocation(), + nonKeyProperty); + context.ReportDiagnostic(diagnostic); + return; + } + + if (!string.IsNullOrEmpty(entityType2)) + { + // Info: Suggest simplified API + var diagnostic = Diagnostic.Create( + DiagnosticDescriptors.GQLEF004, + invocation.GetLocation(), + entityType2); + context.ReportDiagnostic(diagnostic); } } } @@ -77,11 +89,9 @@ static void AnalyzeInvocation(SyntaxNodeAnalysisContext context) static bool IsSimplifiedFilterAdd( InvocationExpressionSyntax invocation, SemanticModel semanticModel, - out LambdaExpressionSyntax? filterLambda, - out string? entityType) + out LambdaExpressionSyntax? filterLambda) { filterLambda = null; - entityType = null; // Check method name is Add if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess) @@ -121,7 +131,6 @@ static bool IsSimplifiedFilterAdd( } // Extract the entity type name - entityType = methodSymbol.TypeArguments[0].Name; // Extract filter lambda if (invocation.ArgumentList.Arguments.Count > 0) @@ -192,11 +201,13 @@ static bool IsFilterBuilderAdd( // Extract arguments foreach (var arg in invocation.ArgumentList.Arguments) { - if (arg.NameColon?.Name.Identifier.Text == "projection" && arg.Expression is LambdaExpressionSyntax projLambda) + if (arg.NameColon?.Name.Identifier.Text == "projection" && + arg.Expression is LambdaExpressionSyntax projLambda) { projectionLambda = projLambda; } - else if (arg.NameColon?.Name.Identifier.Text == "filter" && arg.Expression is LambdaExpressionSyntax filtLambda) + else if (arg.NameColon?.Name.Identifier.Text == "filter" && + arg.Expression is LambdaExpressionSyntax filtLambda) { filterLambda = filtLambda; } @@ -234,14 +245,9 @@ static bool IsFilterBuilderAdd( static bool IsIdentityProjection(LambdaExpressionSyntax? lambda) { - if (lambda == null) - { - return false; - } - // Check if it's an identity projection (x => x or _ => _) // Lambda body should be a simple parameter reference that matches the lambda parameter - if (lambda.Body is not IdentifierNameSyntax identifier) + if (lambda?.Body is not IdentifierNameSyntax identifier) { return false; } From f90622427818847c580e08dde6024a301fe0f027 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Thu, 29 Jan 2026 11:31:30 +1100 Subject: [PATCH 5/5] Update Directory.Build.props --- src/Directory.Build.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 6e2006cd..6f8c919b 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -2,7 +2,7 @@ CS1591;NU5104;CS1573;CS9107;NU1608;NU1109 - 34.0.1 + 34.1.0 preview 1.0.0 EntityFrameworkCore, EntityFramework, GraphQL