Primary's OnApplyFilter being applied to relationship

[max-maurice]

DESCRIPTION

In my AuthorResourceDefinition the OnApplyFilter method is overridden and adds a filter expression. When requesting a relationship, the OnApplyFilter() of AuthorResourceDefinition gets called and adds the filter as expected. The ResourceFieldChainExpression however is applied on the secondary resource level. This fails, because the resource field name of the primary resource does not exist on the secondary one.

STEPS TO REPRODUCE

1. In AuthorResourceDefinition override OnApplyFilter() like shown at https://www.jsonapi.net/usage/extensibility/resource-definitions.html#change-filters:

   public override FilterExpression OnApplyFilter(FilterExpression existingFilter)
   {
       var isDeletedAttribute = ResourceType.Attributes.Single(author =>
           author.Property.Name == nameof(Author.IsDeleted));
   
       var isNotDeleted = new ComparisonExpression(ComparisonOperator.Equals,
           new ResourceFieldChainExpression(isDeletedAttribute),
           new LiteralConstantExpression(bool.FalseString));
   
       return existingFilter == null
           ? (FilterExpression) isNotDeleted
           : new LogicalExpression(LogicalOperator.And,
               new[] { isNotDeleted, existingFilter });
   }

2. Send GET request to /authors/1/relationships/books

EXPECTED BEHAVIOR

The request returns the books of author ID 1.

ACTUAL BEHAVIOR

The request returns an error:

{
    "errors": [
        {
            "detail": "Type 'Book' does not contain a property named 'IsDeleted'.",
            "id": "1",
            "status": "500",
            "title": "An unhandled error occurred while processing this request."
        }
    ],
    "links": {
        "related": "/authors/1/books",
        "self": "/authors/1/relationships/books"
    },
    "meta": {
        "build": "dev",
        "docs": "/swagger/index.html"
    }
}

VERSIONS USED

• JsonApiDotNetCore version: 5.6.0
• ASP.NET Core version: 7.0.15
• Entity Framework Core version: 7.0.15
• Database provider: MS SQL

[bkoelman]

Hi @max-maurice, thanks for reporting this. I can reproduce the problem, it's a bug indeed.

This happens only when the inverse relationship is used to determine the total resource count. Fetching the data itself doesn't suffer from this problem. As a temporary workaround, turn off IncludeTotalResourceCount in options. Alternatively, remove the inverse relationship from your model (so keep Person.Books but delete the Book.Person property), or provide custom IInverseNavigationResolver for full control)

In a nutshell, the problem is that the resource definition returns equals(isDeleted,'false'), which needs to be translated to equals(author.isDeleted,'false') because the query that fetches total resource count executes against Books. While this fixup may look trivial, it's far from it. I've done some experiments at master...investigate-issue-1671, in case you'd like to understand.

To fix this bug, the easiest solution would be to disable fetching total resource count if a resource definition adds a filter. Alternatively, come up with a way to make it work with third-party expressions. I'm open to ideas.

Oh, if you're trying to implement soft-deletion, the only reliable way is by using EF Core's HasQueryFilter method. See the sample at https://github.com/json-api-dotnet/JsonApiDotNetCore/tree/master/test/JsonApiDotNetCoreTests/IntegrationTests/SoftDeletion.

Edit: updated experiment can be found in #1712.

[max-maurice]

Hi @bkoelman

Thank you for looking into this issue!

Soft-deletion only serves as a poor example to demonstrate the issue.

Disabling IncludeTotalResourceCount or removing the inverse relationship are no options for me :-(

I've started implementing IInverseNavigationResolver and set up relationships just like I did in my entities using the InverseProperty and ForeignKey attributes. The relationships are set up on startup, but the error message stays the same.

Your ChainInsertionFilterRewriter seems to resolve some cases. Would you recommend replacing the QueryLayerComposer with something like the one from your experiments in my project?

[bkoelman]

The point of implementing IInverseNavigationResolver would be to make the InverseNavigationProperty return null, therefore preventing the crash from happening (exit early from GetSecondaryFilterFromConstraints).

In which cases doesn't ChainInsertionFilterRewriter solve the problem?

I don't recommend relying on the experimental code. I haven't found a good way to make it work with custom expressions. Unless that's solved, I don't think it's going to ship. Then the next best thing is to support total count only for many-to-many relationships. Would that work for you?

[max-maurice]

Oooh, my implementation of IInverseNavigationResolver was wrong. It set the InverseNavigationProperty of some of my entities to null but seems to have left out a few inverse properties. Instead, I now provide an empty implementation like this:

// Program.cs
services.AddScoped<IInverseNavigationResolver, CustomInverseNavigationResolver>();

// CustomInverseNavigationResolver.cs
public class CustomInverseNavigationResolver : IInverseNavigationResolver
{
    public void Resolve()
    {
        // no implementation, no inverse navigation properties
    }
}

With this, GET requests to /authors/1/relationships/books do work and return data. Are there any downsides to an empty implementation of IInverseNavigationResolver, except for the total count missing?

[bkoelman]

Yes, there are downsides. Without setting InverseNavigationProperty, the following properties are no longer populated correctly:

• HasManyAttribute.IsManyToMany: This results in the failure to update a many-to-many relationship with IDs in the request body that are already assigned.
• HasOneAttribute.IsOneToOne: This may fail with a unique constraint violation when updating a one-to-one relationship.

To reduce the impact, I recommend only clearing the InverseNavigationProperty for places where you're blocked by the crash while leaving the other ones intact. This can be done by instantiating and running the built-in InverseNavigationResolver from your CustomInverseNavigationResolver and clearing the needed ones afterward.

Alternatively, the experimental code can be used; just don't expect future versions to return the total resource count for many-to-one relationships.

[max-maurice]

Thanks, @bkoelman ! I've followed your recommendation and it works perfectly. For reference:

public class CustomInverseNavigationResolver : IInverseNavigationResolver
{
    readonly InverseNavigationResolver inverseNavigationResolver;
    readonly IResourceGraph resourceGraph;

    public CustomInverseNavigationResolver(InverseNavigationResolver inverseNavigationResolver,
        IResourceGraph resourceGraph)
    {
        this.inverseNavigationResolver = inverseNavigationResolver;
        this.resourceGraph = resourceGraph;
    }
    public void Resolve()
    {
        inverseNavigationResolver.Resolve();

        // https://github.com/json-api-dotnet/JsonApiDotNetCore/issues/1671#issuecomment-2641524344
        var authorType = resourceGraph.GetResourceType<Author>();
        var authorBooksRel = authorType.GetRelationshipByPropertyName(nameof(Author.Books));
        authorBooksRel.InverseNavigationProperty = null;
    }
}