ChilliCream / graphql-platform

Welcome to the home of the Hot Chocolate GraphQL server for .NET, the Strawberry Shake GraphQL client for .NET and Banana Cake Pop the awesome Monaco based GraphQL IDE.
https://chillicream.com
MIT License
5.23k stars 744 forks source link

Cleaner options for ResolveWith and static classes/methods #5445

Open glen-84 opened 2 years ago

glen-84 commented 2 years ago

Is your feature request related to a problem?

I started with something like this (a nested resolver class):

public sealed class ArticleRelatedContentType : ObjectType<ArticleRelatedContent>
{
    protected override void Configure(IObjectTypeDescriptor<ArticleRelatedContent> descriptor)
    {
        descriptor.BindFieldsExplicitly();

        descriptor
            .Field("players")
            .ResolveWith<Resolvers>(_ => Resolvers.Players(default!, default!));
    }

    private class Resolvers
    {
        [UsePaging]
        [UseProjection]
        [UseFiltering]
        [UseSorting]
        public static IQueryable<Player> Players(
            [Service] PlayerService playerService,
            [Parent] ArticleRelatedContent relatedContent)
        {
            return playerService.GetPlayersByArticleId(relatedContent.Article.Id);
        }
    }
}

But the default! stuff bothers me. It seems like there should be some way to reference a method without a lambda. Maybe a Delegate?

I was also unable to make the class static, since the generic type argument cannot be static.

I'm now thinking of switching to something like this instead (just a method on the type class):

public sealed class ArticleRelatedContentType : ObjectType<ArticleRelatedContent>
{
    protected override void Configure(IObjectTypeDescriptor<ArticleRelatedContent> descriptor)
    {
        descriptor.BindFieldsExplicitly();

        descriptor
            .Field("players")
            .ResolveWith<ArticleRelatedContentType>(_ => Players(default!, default!));
    }

    [UsePaging]
    [UseProjection]
    [UseFiltering]
    [UseSorting]
    private static IQueryable<Player> Players(
        [Service] PlayerService playerService,
        [Parent] ArticleRelatedContent relatedContent)
    {
        return playerService.GetPlayersByArticleId(relatedContent.Article.Id);
    }
}

Again we have the lambda, and it also feels unnecessary to specify the current type in the generic.

The solution you'd like

Something simpler/cleaner, like:

   descriptor
        .Field("players")
        .ResolveWith(Players);

If ResolveWith took a Delegate, could that work?

(I know that Resolve can also be used, but it can get messy to put all of the code into the Configure method.)

Product

Hot Chocolate

michaelstaib commented 2 years ago

Why do you not use the Resolve lambda?

michaelstaib commented 2 years ago

OR do you mean minimal API like?

michaelstaib commented 2 years ago

For static you could just use resolve btw

michaelstaib commented 2 years ago

How would think about this?

public sealed class ArticleRelatedContentType : ObjectType<ArticleRelatedContent>
{
    protected override void Configure(IObjectTypeDescriptor<ArticleRelatedContent> descriptor)
    {
        descriptor.BindFieldsExplicitly();

        descriptor
            .Field("players")
            .Resolve(
                [UsePaging]
                [UseProjection]
                [UseFiltering]
                [UseSorting]
                ([Service] PlayerService playerService,
                [Parent] ArticleRelatedContent relatedContent) 
                => playerService.GetPlayersByArticleId(relatedContent.Article.Id));
    }
}

you could in this case even do a static delegate.

michaelstaib commented 2 years ago

CC @PascalSenn

glen-84 commented 2 years ago

Why do you not use the Resolve lambda?

  1. As mentioned, if there are multiple/many resolvers that are not simple one-liners, it'll get messy having all the code in one class method.
  2. I don't think it supports DI ([Service]), the [Parent] attribute, etc., so that all has to be accessed via the context.

Trying to switch it now to something like:

    descriptor
        .Field("players")
        .Resolve((IResolverContext context) =>
        {
            var playerService = context.Service<PlayerService>();
            var relatedContent = context.Parent<ArticleRelatedContent>();

            return playerService.GetPlayersByArticleId(relatedContent.Article.Id);
        })
        .UsePaging()
        .UseProjection()
        .UseFiltering()
        .UseSorting();

And getting this for some reason:

Unable to infer the element type from the current resolver.

Maybe I'm doing something stupid.

How would think about this?

I feel like it looks much easier to read as a separate method. Resolve is okay for simple cases. With all these attributes it looks a bit crazy. 😄

I assume by your answer that it's not technically possible to take a Delegate. 😞

michaelstaib commented 2 years ago

Can you specify some code ... like how you would want to write it?

glen-84 commented 2 years ago

I did, under The solution you'd like ...

   descriptor
        .Field("players")
        .ResolveWith(Players);

Where ResolveWith takes a Delegate ... can the engine invoke a delegate and figure out necessary parameters etc. with theDelegate.Method.GetParameters(), etc.?

I haven't written a ton of C#, so forgive me if this is a silly question.

michaelstaib commented 2 years ago

its probably what I suggested ... the above, which also the minimal API uses compiles delegates...

I will have a look at it ... It only works since .NET 6. But I do not know... if it can infer if from the method.

dcby commented 6 months ago

@glen-84, problem solves very easy with simple extension methods.

internal static class DescriptorExtensions
{
  public static IObjectFieldDescriptor Field(this IObjectTypeDescriptor descriptor, Delegate @delegate)
    => descriptor.Field(@delegate.Method);

  public static IObjectFieldDescriptor Field<TRuntimeType>(this IObjectTypeDescriptor<TRuntimeType> descriptor, Delegate @delegate)
    => descriptor.Field(@delegate.Method);

  public static IObjectFieldDescriptor ResolveWith(this IObjectFieldDescriptor descriptor, Delegate @delegate)
    => descriptor.ResolveWith(@delegate.Method);
}

Then

descriptor
  .Field(Resolvers.Players);

// -- or --

descriptor
  .Field("myname")
  .ResolveWith(Resolvers.Players);

@michaelstaib, does it matter in terms of performance if we provide MethodInfo instead of Expression<Func<TIn, TOut>>? How does it compile internally?