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

Cleaner options for ResolveWith and static classes/methods

Open glen-84 opened this issue 2 years ago • 10 comments

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

glen-84 avatar Sep 29 '22 13:09 glen-84

Why do you not use the Resolve lambda?

michaelstaib avatar Sep 29 '22 15:09 michaelstaib

OR do you mean minimal API like?

michaelstaib avatar Sep 29 '22 15:09 michaelstaib

For static you could just use resolve btw

michaelstaib avatar Sep 29 '22 15:09 michaelstaib

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 avatar Sep 29 '22 15:09 michaelstaib

CC @PascalSenn

michaelstaib avatar Sep 29 '22 15:09 michaelstaib

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. 😞

glen-84 avatar Sep 29 '22 15:09 glen-84

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

michaelstaib avatar Sep 29 '22 15:09 michaelstaib

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.

glen-84 avatar Sep 29 '22 15:09 glen-84

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.

michaelstaib avatar Sep 29 '22 16:09 michaelstaib

@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?

dcby avatar Apr 24 '24 07:04 dcby