Hotchocolate: UseSelection combined with UsePaging throws an exception on query

[ghost]

I've found a potential bug within UseSelection feature, made in #1446.

Let's build following context:
csharp public class Query { [UseSelection] [UseFiltering] [UseSorting] public IQueryable<Post> GetPosts([Service] PostDbContext db) => db.Posts; }

When i create following GraphQL query:
graphql query { posts(order_by: {id: DESC} where: {id_gt: 25}) { id } }
i get nice optimized SQL query:
sql SELECT [p].[Id] FROM [Posts] AS [p] WHERE [p].[Id] > 25 ORDER BY [p].[Id] DESC
But when i add [UsePaging] attribute as well:
csharp public class Query { [UseSelection] [UsePaging] //Added [UseFiltering] [UseSorting] public IQueryable<Post> GetPosts([Service] PostDbContext db) => db.Posts; }
and perform the essentially same GraphQL query (with Id wrapped inside nodes field):
graphql query { posts(order_by: {id: DESC} where: {id_gt: 25}) { nodes { id } } }
i get exception:
Value cannot be null. (Parameter 'source') at System.Linq.Queryable.Select[TSource,TResult](IQueryable`1 source, Expression`1 selector)\r\n at HotChocolate.Types.SelectionMiddleware`1.InvokeAsync(IMiddlewareContext context) in ...\\Core\\Types.Selection\\SelectionMiddleware~1.cs:line 41\r\n at HotChocolate.Execution.ExecutionStrategyBase.ExecuteMiddlewareAsync(ResolverContext resolverContext, IErrorHandler errorHandler) in ...\\Core\\Core\\Execution\\ExecutionStrategyBase.Resolver.cs:line 40
and if, for instance, i perform GraphQL query requesting only totalCount:
graphql query { posts(order_by: {id: DESC} where: {id_gt: 25}) { totalCount } }
i get exception:
Type 'HotChocolate.Types.Relay.IConnection' does not have a default constructor (Parameter 'type') at System.Linq.Expressions.Expression.New(Type type)\r\n at HotChocolate.Types.Selections.SelectionClosure.CreateMemberInit() in ...\\Core\\Types.Selection\\SelectionClosure.cs:line 35\r\n at HotChocolate.Types.Selections.SelectionClosure.CreateMemberInitLambda() in ...\\Core\\Types.Selection\\SelectionClosure.cs:line 41\r\n at HotChocolate.Types.Selections.SelectionVisitor.Project[T]() in ...\\Core\\Types.Selection\\SelectionVisitor.cs:line 41\r\n at HotChocolate.Types.SelectionMiddleware`1.InvokeAsync(IMiddlewareContext context) in ...\\Core\\Types.Selection\\SelectionMiddleware~1.cs:line 41\r\n at HotChocolate.Execution.ExecutionStrategyBase.ExecuteMiddlewareAsync(ResolverContext resolverContext, IErrorHandler errorHandler) in ...\\Core\\Core\\Execution\\ExecutionStrategyBase.Resolver.cs:line 40

Additional context:
I'm using version _10.0.4-preview.17_ i've pulled and manually built all required assemblies i've later included in my project, so there may be some hidden issue within that, but i wanted to share this problem anyway.

[PascalSenn]

Thank you for reporting this. this is mots likely a bug.
We will investigate 👍

[michaelstaib]

I think this is not a bug but the order of middleware.

    [UseSelection]
    [UsePaging] //Added
    [UseFiltering]
    [UseSorting]

middleware order is important and the paging will execute the query hence selection cannot be applied.

This would work again I guess.

    [UsePaging] //Added
    [UseSelection]
    [UseFiltering]
    [UseSorting]

[michaelstaib]

Can you verify @PascalSenn?

[TheJayMann]

I believe I can confirm, in my testing, that this works, as my test project which I migrated from my implementation to the official implementation was ordered the correct way as mentioned and I did not have a problem.

However, I'm using the fluent syntax, not the attribute syntax, so I can't confirm that.

[michaelstaib]

The attributes have the fluent syntax inside :D

[michaelstaib]

I will have a look at the error ... maybe we could have a better error message around this that explains that it only works on IQueryable. But I have to look at the implementation.

[PascalSenn]

There is alread a test case for this.
This order should work:
UsePaging => UseFiltering => UseSorting => UseSelection

https://github.com/ChilliCream/hotchocolate/blob/version_10_0_0_master/src/Core/Types.Selection.Abstractions.Tests/SelectionTestsBase.cs#L648-L652

[ghost]

Thanks guys, i wasn't aware that ordering is important here. I'm not a pro, but i haven't come across any attribute design yet where ordering is important, but i'm not saying it doesn't make sense here.

This could be just a reminder to place special note about this in the docs and handle exception more gracefully 😄

[TheJayMann]

The documentation does explain the importance of the ordering of paging, filtering, and sorting. However, as new as selection is, it isn't in the documentation yet. Given that all of these are middleware, ordering is just as important as ASP.NET Core middleware is.

Also, from what I've seen, almost (if not) all middleware for HotChocolate immediately calls the next middleware, then attempts to do something with the results returned from the next middleware. This means that you are typically looking at adding the middleware in reverse ordering to how you would expect them to run.

Also, I can confirm that it works exactly the same in the order UsePaging, UseSelection, UseSorting, UseFiltering. This order makes most sense to me, as, when writing LINQ queries, one typically first filters the data, then sorts the data, then selects the data.

[ghost]

Ok, now i tested it and it works in following order:
[UsePaging]
[UseSelection]
[UseSorting]
[UseFiltering]

both with attributes and fluent syntax.
But i've encountered more errors. Here is the issue:
GraphQL query:
graphql query { posts(order_by: {id: DESC}) { nodes{ id } totalCount pageInfo { startCursor } } }
i get following result:
json { "data": { "posts": { "nodes": [], "totalCount": 0, "pageInfo": { "startCursor": null } } } }
and generated SQL is fine. But when i modify query and remove nodes:
graphql query { posts(order_by: {id: DESC}) { totalCount pageInfo { startCursor } } }
i get following exception: The property 'System.String StartCursor' has no 'set' accessor (Parameter 'member') .
And when i remove pageInfo and leave only totalCount:
graphql query { posts(order_by: {id: DESC}) { totalCount } }
i get following exception:
Type 'HotChocolate.Types.Relay.IConnection' does not have a default constructor (Parameter 'type')

[michaelstaib]

Interesting :) @PascalSenn we have to add more integration tests for these combinations. I think in this case you are not selecting but since you cannot figure out what. In these cases for version 10 always select everything or rather skip limiting the selection.

[TheJayMann]

I think the cause is in the TryUnwrapPaging method. The UnwrapPaging method assumes that if TryUnwrapPaging returns false, that the object type is not a paging connection object. However, it will also return false if the object type is a paging connection object but neither the edges field nor the nodes field is queried.

https://github.com/ChilliCream/hotchocolate/blob/8302ae71bd87fc195e652c99c880c359a67ca1bd/src/Core/Types.Selection/SelectionVisitorBase.cs#L130-L151

[PascalSenn]

@TheJayMann @rankdalibor Pushed a few fixes. Can you check with the following version? https://github.com/ChilliCream/hotchocolate/releases/tag/10.4.0-preview.19

[TheJayMann]

It appears this is working now.

[PascalSenn]

thanks for the feedback @TheJayMann :+1:
Waiting on @rankdalibor response to close the issue :)

[ghost]

I can confirm that issues i've noted in previous comment are now fixed 👍

@PascalSenn One more thing.. maybe doesn't belong to this issue. Is nested collection paging created in this moment? It isn't working for me. Filtering and sorting are working properly in my current test cases but paging doesn't.

[PascalSenn]

@rankdalibor cool :)
Nested paging should work. Can you close this issue in this case and open a new one for paging?
We can invetigate on that one then :)