ResolveWith<> Fails when an interface is specified in 13.1+ As a Non Abstract Class is Required

[williamdenton]

Is there an existing issue for this?

• [X] I have searched the existing issues

Product

Hot Chocolate

Describe the bug

The changes implemented in the PR linked below have broken ResolveWith<> when using an Interface to specify the resolver.

https://github.com/ChilliCream/graphql-platform/issues/6075 https://github.com/ChilliCream/graphql-platform/pull/6110

This pattern works in 12.x when using DI to resolve the correct implementation

serviceCollection.AddTransient<IFooResolvers, FooResolvers>();

public interface IFooResolvers {
    string GetFoo(string arg)
}

public class FooResolvers : IFooResolvers
{
    public string GetFoo(string arg)
    {
        // Omitted code for brevity
    }
}

public class QueryType : ObjectType
{
    protected override void Configure(IObjectTypeDescriptor descriptor)
    {
        descriptor
            .Field("foo")
            .Argument("arg", a => a.Type<NonNullType<StringType>>())
            .ResolveWith<IFooResolvers>(r => r.GetFoo(default));
    }
}

This is the exception that is thrown in my use case.

https://github.com/ChilliCream/graphql-platform/blob/d7b84630de9786bc0f73653970f5f6811fc181a8/src/HotChocolate/Core/src/Types/Types/Descriptors/ObjectFieldDescriptor.cs#L392-L403

Am I holding HotChocolate wrong by using DI/Interfaces to specify the resolver?

I note in the original issue this comment

ResolveWith needs a non abstract class at its a resolver type that will be instantiated. https://github.com/ChilliCream/graphql-platform/issues/6075#issuecomment-1521431178

Locally in debug I can set a break point on this line and drag it over to skip this condition execution and the code runs successfully. This suggests a possible fix for this is to make the change below.

-if (resolverType?.IsAbstract is true)
+if (resolverType?.IsAbstract is true &&  resolverType?.IsInterface is false)

As I mentioned above, if I'm holding hotChocolate wrong and i'm off the golden path I can update my codebase to use concrete resolvers (but this isn't a small undertaking for me)

Steps to reproduce

Relevant log output

System.ArgumentException: The resolver type namespace.IThingResolver cannot be used, a non-abstract type is required. (Parameter 'resolverType')
   at HotChocolate.Types.Descriptors.ObjectFieldDescriptor.ResolveWithInternal(MemberInfo propertyOrMethod, Type resolverType)
   at HotChocolate.Types.Descriptors.ObjectFieldDescriptor.ResolveWith[TResolver](Expression`1 propertyOrMethod)

Additional Context?

No response

Version

13.4

[michaelstaib]

I have to look at this again. In my mind I never expected people to hook interfaces in here.

[williamdenton]

Reflecting on this after updating my code base not to do this i think its a reasonably serious anti-pattern. There are some pretty serious pitfalls to doing this wrong as outlined in the documentation here https://chillicream.com/docs/hotchocolate/v13/server/dependency-injection#constructor-injection Could the library do more to be more opinionated? Maybe a rosyln analyzer to suggest better usage of the HC apis?

As I converted my codebase to best practice (not using interface in resolveWith, not using constructor injection) i came across some serious side effects of doing it wrong especially when it comes to injecting BatchDataLoader and CacheDataLoader.

I use the BannedApiAnalyzers nuget package in my solution. What I did to mitigate anyone else in my team from using ResolveWith incorrectly going forward was ban the usage of ResolveWith, then add an extension method that adds the generic type constraint of new() this enforces parameterless constructor (no injection) and no interfaces.

M:HotChocolate.Types.IObjectFieldDescriptor.ResolveWith``1(System.Linq.Expressions.Expression{System.Func{``0,System.Object}}); Use ResolveWithSafely to enforce parameter injection

public static class ResolverBestPracticeExtensions
{
    public static IObjectFieldDescriptor ResolveWithSafely<TResolver>(this IObjectFieldDescriptor x, Expression<Func<TResolver, object?>> propertyOrMethod) where TResolver: new()
#pragma warning disable RS0030
        => x.ResolveWith<TResolver>(propertyOrMethod);
#pragma warning restore RS0030
}

[marius-klimantavicius]

We are using interfaces with expectation that ResolveWith<T>(s=>s.Method(default, default, ...)) would be compiled to something like:

var service = context.Service<T>();
var arg1 = ...;
var arg2 = ...;
...
return service.Method(arg1, arg2, ...);

Would it be possible to provide a resolver method that does something like this if ResolveWith can only be used with non-abstract resolvers?

[PavelA85]

BUMP