graphql-platform icon indicating copy to clipboard operation
graphql-platform copied to clipboard

CanHandle of MongoDbCursorPagingProvider doesn't catch IExecutable<T> return type

Open tobias-tengler opened this issue 4 years ago • 5 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Describe the bug

When using the MongoDB pagination provider by itself it works, since the provider is taken as the default. If multiple providers are registered and the MongoDB provider is not the first pagination provider (default) it doesn't work since the CanHandle method returns false.

Steps to reproduce

Create a field, following the documentation

[UseMongoDbPaging]
public IExecutable<Person> GetPersons([Service] IMongoCollection<Person> collection)
{
    return collection.AsExecutable();
}

and CanHandle returns false.

Relevant log output

No response

Additional Context?

No response

Product

Hot Chocolate

Version

12.0.0-preview.34

tobias-tengler avatar Aug 28 '21 12:08 tobias-tengler

There are a whole bunch of issues or rather non intuitive behavior I'm noticing when trying to use the new pagination experience with multiple databases. For example when using EF Core by itself, I do not have to register anything as the Queryable provider is used as fallback. If I add the MongoDb paging providers to handle a mongodb field, the ef core one no longer works, since now the fallback queryable provider is no longer used, since a provider has been registered and is now used as the default.

I have to now register the queryable provider explicitly, BUT set the mongodb provider as the default or otherwise the "match-all" queryable provider takes precedence. This is all really confusing from a consumer standpoint and will only get worse, once we introduce the same experience for filtering, etc.

I think we should have a match all enumerable provider that's used as fallback and a specific queryable provider people have to register when using ef core/any queryable stuff. Once the above bug is addressed and providers can be reliably chosen based on the return type, the experience should be much more intuitive.

tobias-tengler avatar Aug 28 '21 13:08 tobias-tengler

@tobias-tengler would like to coordinate this with Neo4J as well as i am working on the offset paging provider?

arif-hanif avatar Aug 29 '21 15:08 arif-hanif

@arif-hanif sounds good to me. I'm not happy with how it's handled in Neo4J with the user instantiating a Neo4JExecutable himself anyways. Makes it harder to evolve the API. I think there should be an AsExecutable() extension method on the IAsyncSession as well and an IExecutable<T> method return type similar to the MongoDb integration.

But I'd like to hear @michaelstaib and @PascalSenn opinion on this as well.

I think Hot Chocolate should be able to determine the provider to use based on the return type alone. User's shouldnt have to (in the worst case) specify a ProviderName on multiple attributes on the same resolver. If this means that we have to introduce some marker interfaces like IMongoDbExecutable<T>, I'd still prefer it to having to specify a provider name. The provider name should only be necessary when trying to use a self-built provider alongside one of the Hot Chocolate providers for the same data source.

tobias-tengler avatar Aug 29 '21 15:08 tobias-tengler

  1. This is the reason why [UseMongoDbPaging] was introduced (now I can remember)
  2. With UsePaging you would only have to specify the provider name in a multi database scenario.
  3. AsExecutable() could indeed return a marker interface as these interfaces already exist, though the API doesn't feel nice. The point of IExecutable was to be transparent. I think the problem is not fixed when a IMongoDbExecutable is returned, then we could also just use UseMongoDbPaging. Maybe we need to rework this and resolve the Paging Provider on execution of the resolver and have a CanHandle on the result of the resolver

Also I agree with you @tobias-tengler, it should just work

PascalSenn avatar Aug 29 '21 18:08 PascalSenn

Someone on Slack just reported the problem I'm describing in my first comment when trying to use pagination with both MongoDb and EF Core data sources. I think we should take a look at that after v13 and make it just work.

tobias-tengler avatar Jan 07 '23 11:01 tobias-tengler