Experiments to fix #1671

bkoelman · bkoelman · commit f6002994a978 · 2025-02-04T01:08:56.000+01:00
diff --git a/src/Examples/GettingStarted/Data/SampleDbContext.cs b/src/Examples/GettingStarted/Data/SampleDbContext.cs
@@ -9,4 +9,7 @@ public class SampleDbContext(DbContextOptions<SampleDbContext> options)
     : DbContext(options)
 {
     public DbSet<Book> Books => Set<Book>();
+    public DbSet<House> Houses => Set<House>();
+    public DbSet<TinyHouse> TinyHouses => Set<TinyHouse>();
+    public DbSet<BigHouse> BigHouses => Set<BigHouse>();
 }
diff --git a/src/Examples/GettingStarted/Definitions/PersonDefinition.cs b/src/Examples/GettingStarted/Definitions/PersonDefinition.cs
@@ -0,0 +1,25 @@
+using GettingStarted.Models;
+using JetBrains.Annotations;
+using JsonApiDotNetCore.Configuration;
+using JsonApiDotNetCore.Queries.Expressions;
+using JsonApiDotNetCore.Queries.Parsing;
+using JsonApiDotNetCore.Resources;
+
+namespace GettingStarted.Definitions;
+
+[UsedImplicitly(ImplicitUseKindFlags.InstantiatedNoFixedConstructorSignature)]
+internal sealed class PersonDefinition(IResourceGraph resourceGraph, IResourceFactory resourceFactory)
+    : JsonApiResourceDefinition<Person, int>(resourceGraph)
+{
+    private readonly IResourceFactory _resourceFactory = resourceFactory;
+
+    public override FilterExpression OnApplyFilter(FilterExpression? existingFilter)
+    {
+        var parser = new FilterParser(_resourceFactory);
+        FilterExpression isNotDeleted = parser.Parse("equals(isDeleted,'false')", ResourceType);
+        FilterExpression hasBooksWithName = parser.Parse("has(books,equals(author.name,'Mary Shelley'))", ResourceType);
+        FilterExpression isTypeWithCond = parser.Parse("isType(house,bigHouses,equals(floorCount,'3'))", ResourceType);
+
+        return LogicalExpression.Compose(LogicalOperator.And, isNotDeleted, hasBooksWithName, isTypeWithCond,existingFilter)!;
+    }
+}
diff --git a/src/Examples/GettingStarted/Models/Person.cs b/src/Examples/GettingStarted/Models/Person.cs
@@ -11,6 +11,25 @@ public sealed class Person : Identifiable<int>
     [Attr]
     public string Name { get; set; } = null!;
 
+    [Attr]
+    public bool IsDeleted { get; set; }
+
     [HasMany]
     public ICollection<Book> Books { get; set; } = new List<Book>();
+
+    [HasOne]
+    public House? House { get; set; }
+}
+
+[Resource]
+public abstract class House : Identifiable<int>;
+
+[Resource]
+public sealed class TinyHouse : House;
+
+[Resource]
+public sealed class BigHouse : House
+{
+    [Attr]
+    public int? FloorCount { get; set; }
 }
diff --git a/src/Examples/GettingStarted/Program.cs b/src/Examples/GettingStarted/Program.cs
@@ -1,5 +1,6 @@
 using System.Diagnostics;
 using GettingStarted.Data;
+using GettingStarted.Definitions;
 using GettingStarted.Models;
 using JsonApiDotNetCore.Configuration;
 using Microsoft.EntityFrameworkCore;
@@ -28,6 +29,8 @@
 #endif
 });
 
+builder.Services.AddResourceDefinition<PersonDefinition>();
+
 WebApplication app = builder.Build();
 
 // Configure the HTTP request pipeline.
@@ -69,23 +72,32 @@ static async Task CreateSampleDataAsync(SampleDbContext dbContext)
         PublishYear = 1818,
         Author = new Person
         {
-            Name = "Mary Shelley"
+            Name = "Mary Shelley",
+            House = new BigHouse
+            {
+                FloorCount = 3
+            }
         }
     }, new Book
     {
         Title = "Robinson Crusoe",
         PublishYear = 1719,
         Author = new Person
         {
-            Name = "Daniel Defoe"
+            Name = "Daniel Defoe",
+            House = new TinyHouse()
         }
     }, new Book
     {
         Title = "Gulliver's Travels",
         PublishYear = 1726,
         Author = new Person
         {
-            Name = "Jonathan Swift"
+            Name = "Jonathan Swift",
+            House = new BigHouse
+            {
+                FloorCount = 4
+            }
         }
     });
 
diff --git a/src/Examples/GettingStarted/Properties/launchSettings.json b/src/Examples/GettingStarted/Properties/launchSettings.json
@@ -11,15 +11,15 @@
     "IIS Express": {
       "commandName": "IISExpress",
       "launchBrowser": true,
-      "launchUrl": "api/people?include=books",
+      "launchUrl": "api/people/1/books",
       "environmentVariables": {
         "ASPNETCORE_ENVIRONMENT": "Development"
       }
     },
     "Kestrel": {
       "commandName": "Project",
       "launchBrowser": true,
-      "launchUrl": "api/people?include=books",
+      "launchUrl": "api/people/1/books",
       "applicationUrl": "http://localhost:14141",
       "environmentVariables": {
         "ASPNETCORE_ENVIRONMENT": "Development"
diff --git a/src/JsonApiDotNetCore/Queries/ChainInsertionFilterRewriter.cs b/src/JsonApiDotNetCore/Queries/ChainInsertionFilterRewriter.cs
@@ -0,0 +1,60 @@
+using System.Collections.Immutable;
+using JsonApiDotNetCore.Queries.Expressions;
+using JsonApiDotNetCore.Resources.Annotations;
+
+namespace JsonApiDotNetCore.Queries;
+
+internal sealed class ChainInsertionFilterRewriter : QueryExpressionRewriter<object?>
+{
+    private readonly ResourceFieldAttribute _fieldToInsert;
+    private bool _isInNestedScope;
+
+    public ChainInsertionFilterRewriter(ResourceFieldAttribute fieldToInsert)
+    {
+        ArgumentNullException.ThrowIfNull(fieldToInsert);
+
+        _fieldToInsert = fieldToInsert;
+    }
+
+    public override QueryExpression VisitResourceFieldChain(ResourceFieldChainExpression expression, object? argument)
+    {
+        if (_isInNestedScope)
+        {
+            return expression;
+        }
+
+        IImmutableList<ResourceFieldAttribute> newFields = expression.Fields.Insert(0, _fieldToInsert);
+        return new ResourceFieldChainExpression(newFields);
+    }
+
+    public override QueryExpression? VisitHas(HasExpression expression, object? argument)
+    {
+        if (Visit(expression.TargetCollection, argument) is ResourceFieldChainExpression newTargetCollection)
+        {
+            var backupIsInNestedScope = _isInNestedScope;
+            _isInNestedScope = true;
+            FilterExpression? newFilter = expression.Filter != null ? Visit(expression.Filter, argument) as FilterExpression : null;
+            _isInNestedScope = backupIsInNestedScope;
+
+            var newExpression = new HasExpression(newTargetCollection, newFilter);
+            return newExpression.Equals(expression) ? expression : newExpression;
+        }
+
+        return null;
+    }
+
+    public override QueryExpression VisitIsType(IsTypeExpression expression, object? argument)
+    {
+        ResourceFieldChainExpression? newTargetToOneRelationship = expression.TargetToOneRelationship != null
+            ? Visit(expression.TargetToOneRelationship, argument) as ResourceFieldChainExpression
+            : null;
+
+        var backupIsInNestedScope = _isInNestedScope;
+        _isInNestedScope = true;
+        FilterExpression? newChild = expression.Child != null ? Visit(expression.Child, argument) as FilterExpression : null;
+        _isInNestedScope = backupIsInNestedScope;
+
+        var newExpression = new IsTypeExpression(newTargetToOneRelationship, expression.DerivedType, newChild);
+        return newExpression.Equals(expression) ? expression : newExpression;
+    }
+}
diff --git a/src/JsonApiDotNetCore/Queries/QueryLayerComposer.cs b/src/JsonApiDotNetCore/Queries/QueryLayerComposer.cs
@@ -98,36 +98,64 @@ public QueryLayerComposer(IEnumerable<IQueryConstraintProvider> constraintProvid
         FilterExpression? primaryFilter = GetFilter(Array.Empty<QueryExpression>(), hasManyRelationship.LeftType);
         FilterExpression? secondaryFilter = GetFilter(filtersInSecondaryScope, hasManyRelationship.RightType);
 
-        FilterExpression inverseFilter = GetInverseRelationshipFilter(primaryId, hasManyRelationship, inverseRelationship);
+        if (primaryFilter != null)
+        {
+            // This would hide total resource count on secondary to-one endpoint, but at least not crash anymore.
+            // return null;
+        }
+
+        FilterExpression inverseFilter = GetInverseRelationshipFilter(primaryId, hasManyRelationship, inverseRelationship, primaryFilter);
 
-        return LogicalExpression.Compose(LogicalOperator.And, inverseFilter, primaryFilter, secondaryFilter);
+        return LogicalExpression.Compose(LogicalOperator.And, inverseFilter, secondaryFilter);
     }
 
     private static FilterExpression GetInverseRelationshipFilter<TId>([DisallowNull] TId primaryId, HasManyAttribute relationship,
-        RelationshipAttribute inverseRelationship)
+        RelationshipAttribute inverseRelationship, FilterExpression? primaryFilter)
     {
         return inverseRelationship is HasManyAttribute hasManyInverseRelationship
-            ? GetInverseHasManyRelationshipFilter(primaryId, relationship, hasManyInverseRelationship)
-            : GetInverseHasOneRelationshipFilter(primaryId, relationship, (HasOneAttribute)inverseRelationship);
+            ? GetInverseHasManyRelationshipFilter(primaryId, relationship, hasManyInverseRelationship, primaryFilter)
+            : GetInverseHasOneRelationshipFilter(primaryId, relationship, (HasOneAttribute)inverseRelationship, primaryFilter);
     }
 
-    private static ComparisonExpression GetInverseHasOneRelationshipFilter<TId>([DisallowNull] TId primaryId, HasManyAttribute relationship,
-        HasOneAttribute inverseRelationship)
+    private static FilterExpression GetInverseHasOneRelationshipFilter<TId>([DisallowNull] TId primaryId, HasManyAttribute relationship,
+        HasOneAttribute inverseRelationship, FilterExpression? primaryFilter)
     {
         AttrAttribute idAttribute = GetIdAttribute(relationship.LeftType);
         var idChain = new ResourceFieldChainExpression(ImmutableArray.Create<ResourceFieldAttribute>(inverseRelationship, idAttribute));
+        var idComparison = new ComparisonExpression(ComparisonOperator.Equals, idChain, new LiteralConstantExpression(primaryId));
+
+        FilterExpression? newPrimaryFilter = null;
 
-        return new ComparisonExpression(ComparisonOperator.Equals, idChain, new LiteralConstantExpression(primaryId));
+        if (primaryFilter != null)
+        {
+            // many-to-one. This is the hard part. We can special-case for built-in has() and isType() usage, however third-party filters can't participate.
+            // Because there is no way of indicating in an expression "this chain belongs to something related"; the parsers only know that.
+            // For example, see the third-party SumExpression.Selector with SumFilterParser in test project.
+
+            // For example:
+            // input:  and(equals(isDeleted,'false'),       has(books       ,equals(author.name,'Mary Shelley')),isType(       house,bigHouses,equals(floorCount,'3')))
+            // output: and(equals(author.isDeleted,'false'),has(author.books,equals(author.name,'Mary Shelley')),isType(author.house,bigHouses,equals(floorCount,'3')))
+            //                    ^                             ^                   ^!                                  ^                             ^!
+            // Note how some chains are updated, while others (expressions on related types) are intentionally not.
+
+            var rewriter = new ChainInsertionFilterRewriter(inverseRelationship);
+            newPrimaryFilter = (FilterExpression?)rewriter.Visit(primaryFilter, null);
+        }
+
+        return LogicalExpression.Compose(LogicalOperator.And, idComparison, newPrimaryFilter)!;
     }
 
     private static HasExpression GetInverseHasManyRelationshipFilter<TId>([DisallowNull] TId primaryId, HasManyAttribute relationship,
-        HasManyAttribute inverseRelationship)
+        HasManyAttribute inverseRelationship, FilterExpression? primaryFilter)
     {
+        // many-to-many. This one is easy, we can just push into the sub-condition of has().
+
         AttrAttribute idAttribute = GetIdAttribute(relationship.LeftType);
         var idChain = new ResourceFieldChainExpression(ImmutableArray.Create<ResourceFieldAttribute>(idAttribute));
         var idComparison = new ComparisonExpression(ComparisonOperator.Equals, idChain, new LiteralConstantExpression(primaryId));
 
-        return new HasExpression(new ResourceFieldChainExpression(inverseRelationship), idComparison);
+        FilterExpression filter = LogicalExpression.Compose(LogicalOperator.And, idComparison, primaryFilter)!;
+        return new HasExpression(new ResourceFieldChainExpression(inverseRelationship), filter);
     }
 
     /// <inheritdoc />

Original file line number	Diff line number	Diff line change
`@@ -9,4 +9,7 @@ public class SampleDbContext(DbContextOptions<SampleDbContext> options)`
`9`	`9`	`: DbContext(options)`
`10`	`10`	`{`
`11`	`11`	`public DbSet<Book> Books => Set<Book>();`
	`12`	`+ public DbSet<House> Houses => Set<House>();`
	`13`	`+ public DbSet<TinyHouse> TinyHouses => Set<TinyHouse>();`
	`14`	`+ public DbSet<BigHouse> BigHouses => Set<BigHouse>();`
`12`	`15`	`}`