Skip to content

Sorting by an interface member from a lambda expression fails #1161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ngeien opened this issue Jun 22, 2022 · 1 comment · Fixed by #1162
Closed

Sorting by an interface member from a lambda expression fails #1161

ngeien opened this issue Jun 22, 2022 · 1 comment · Fixed by #1162
Labels

Comments

@ngeien
Copy link

ngeien commented Jun 22, 2022

DESCRIPTION

Sorting by an interface member from a lambda expression fails in v5.0.1. It was working before in v4.2.0.
It is beneficial to implement common sorting logic in a generic resource definition.

STEPS TO REPRODUCE

The following example code is based on your GettingStarted example. A generic resource definition that supports sorting for all models that implement an ISortable interface.

public interface ISortable
{
    [Attr]
    int SortOrder { get; set; }
}

[Resource]
public sealed class Book : Identifiable<int>, ISortable
{
    [Attr]
    public string Title { get; set; } = null!;

    [Attr]
    public int PublishYear { get; set; }

    [HasOne]
    public Person Author { get; set; } = null!;

    [Attr]
    public int SortOrder { get; set; }

}
public class SortableResourceDefinition<TResource> : JsonApiResourceDefinition<TResource, int>
    where TResource : class, IIdentifiable<int>, ISortable
{
    public SortableResourceDefinition(IResourceGraph resourceGraph)
        : base(resourceGraph)
    { }

    public override SortExpression? OnApplySort(SortExpression? existingSort)
    {
        return existingSort == null
            ? CreateSortExpressionFromLambda(new PropertySortOrder { (resource => resource.SortOrder, ListSortDirection.Ascending) })
            : existingSort;
    }
}

public class BookDefinition : SortableResourceDefinition<Book>
{
    public BookDefinition(IResourceGraph resourceGraph)
        : base(resourceGraph)
    { }
}
  1. Add the ISortable interface and update the model Book
  2. Add the generic resource definition and its implementation for the book resource.
  3. Add the resource definition service at startup.

EXPECTED BEHAVIOR

It should return an ordered collection by the property SortOrder.

ACTUAL BEHAVIOR

It raises an exception in SortExpressionLambdaConverter.cs:

{
    "id": "5a2eb44b-35d7-4e5c-9299-25e5ad1cf1ef",
    "status": "500",
    "title": "Invalid lambda expression for sorting from resource definition. It should select a property that is exposed as an attribute, or a to-many relationship followed by Count(). The property can be preceded by a path of to-one relationships. Examples: 'blog => blog.Title', 'blog => blog.Posts.Count', 'blog => blog.Author.Name.LastName'.",
    "detail": "The lambda expression 'resource => Convert(resource.SortOrder, Object)' is invalid. Type 'GettingStarted.Models.ISortable' does not exist in the resource graph."
}

Possible solution

internal sealed class SortExpressionLambdaConverter
{
   private Expression? ReadAttribute(Expression expression)
   {
       ...
-      ResourceType resourceType = memberExpression.Member.Name == nameof(Identifiable<object>.Id) && memberExpression.Expression != null
-                ? _resourceGraph.GetResourceType(memberExpression.Expression.Type)
-                : _resourceGraph.GetResourceType(memberExpression.Member.DeclaringType!);
+      ResourceType resourceType = memberExpression.Expression != null
+                ? _resourceGraph.GetResourceType(memberExpression.Expression.Type)
+                : _resourceGraph.GetResourceType(memberExpression.Member.DeclaringType!);
       ...
   }
}

VERSIONS USED

  • JsonApiDotNetCore version: 5.0.1
  • ASP.NET Core version: 6.0.6
  • Entity Framework Core version: 6.0.6
  • Database provider: Sqlite
@bkoelman
Copy link
Member

Thanks for reporting this, including great repro steps, and even diving into the sources to isolate the bug!

I've opened a PR to solve this. Once the cibuild has completed, can you verify this fixes your issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

2 participants