Draft for Asp.Net controller-style graph support with first-class support of dependency injection

[Shane32]

This DRAFT pull request lets you write graphs in the same style of Asp.Net Core controllers. An instance of each graph "controller" is created when required. Assuming they are registered as scoped "controllers", they can rely on any scoped services within the program. Public methods are added as fieldtypes, and method parameters are added as arguments. Methods and parameters can specify exact graphtypes via attributes, as well as names, descriptions, deprecation messages, and metadata. When not specified, field and argument types are inferred via the GraphTypeTypeRegistry; class types are assumed to be optional unless tagged with the [Required] attribute, and value types are assumed to be required unless they are a Nullable<> type.

To link this up, a graphql graphtype class is required for each of these "controllers", which uses reflection to read all of the methods and properties on the "controller". This graphtype class is a singleton, so the reflection code is performed only once when the schema is created. Resolver functions are COMPILED ahead of time, so neither reflection nor dynamic linking is necessary during the execution of a query. Obviously there will be a performance hit during the creation of the schema.

The code also has support for defining synchronous or asynchronous behavior on a per-field basis.
By default, all methods will run synchronously within the scope that was provided to the DocumentExecuter. However, any static methods that return a Task<> will run asynchronously. Also, fields can be marked with a Concurrent attribute, which will force them to run asynchronously, optionally creating a scope for them to run within. This is useful when unrelated fields on the main Query graphtype need to retrieve information from a scoped service provider.

To create a dedicated scope for a method that requests it (via the Concurrent(true) attribute), the code relies on IServiceScopeFactory and IServiceScope being provided by the service provider. A user can register a custom instance of IServiceScopeFactory if they are using a nonstandard service provider. To eliminate the nuget dependency, the few lines of code in CreateScopedResolver to create the service scope could be replaced with throw new NotSupportedException(), letting users override the class to implement their own code to create a new service scope. However, this is the critical piece of code to allow multithreading when the resolvers need a service scope to execute.

In order to track the service provider used for creating instances, the AsyncServiceProvider class assists in passing the IServiceProvider through the document executor into the resolve function that is compiled by the graphtype. AsyncServiceProvider relies on AsyncLocal, just as Asp.Net Core uses it to return the HttpContext via IHttpContextAccessor. Performance would be improved if the IServiceProvider was passed through the ResolveFieldContext to the resolve function.

Also, the document executor requires a reliance on the service provider to execute. This is handled by simply registering the document executor (DIDocumentExecutor) with the service provider as a scoped service. The document executor will then receive the service provider (and any other dependencies, such as the document validator) via dependency injection.

Finally, all this code does not help the fact that when using a DataLoader, the document executor is restricted to synchronously execute (assuming the dataloader relies on a scoped dependency that is not designed for multithreaded operation). To resolve this a "DelayLoader" class was created to replace the DataLoader. The DocumentExecuter watches for IDelayLoadedResult<> results and queues them to be processed once all other nodes are resolved. DelayLoader "resolve" functions are executed synchronously, as they all exist within the same scope. The provided IDelayLoader / IDelayLoadedResult<> implementation supports optional caching and functions similar to the BatchDataLoader class. The DocumentExecuter is also backwards compatible with the DataLoader classes, albeit with the same functional restrictions.

To use the DelayLoader class, just inherit from it, adding in any scoped dependencies to the constructor as would be typical for dependency injection. Override the abstract LoadValuesAsync function to load the data. Register the DelayLoader class with the service provider as a scoped service. Then reference the DelayLoader class in the graph "controller" constructor, which should also be registered as a scoped service.

I modified the StarWarsQuery class and DroidType class to demonstrate how the code can be used. It is far from a good demonstration of the capabilities, and does not test the DelayLoader at all. Since the GraphTypeTypeRegistry was not used in the sample, it's a little more verbose than would typically be required. Also since it does not have a typical service provider, it is unable to demonstrate the scoped dependency support of this code.

There's no special code for returning connection objects. I'm thinking it should notice ResolveConnectionContext as a parameter type, but I've not implemented it yet.

The code is all designed to run as an independent library from the main GraphQL library, without relying on any changes to the base GraphQL library. However, in this sample it is all integrated directly into the project. My class naming is questionable, and I have not added xml comments or tests to any of the code. There is extensive commenting on the code throughout, however.

In general, I think this would be an excellent way to support graph types that rely on scoped services. It's a significant change from the typical way to write graphs, but would be much more intuitive for people converting Asp.Net Core REST APIs to a GraphQL API (with native back-end, not simply a translation layer). It also provides a much more intuitive method for accessing scoped services within a graph. Finally, it moves the schema from code into attributes, which would seem to be a bit more logical place for them.

Please comment if you feel this would be a welcome addition into the base dotnet-graphql library.

[sungam3r]

Relates to #648

[sungam3r]

Leaky dependency.

[sungam3r]

Have you watched the conventions project? It is closer to your proposal than graphql-dotnet itself. As you already know, there are a number of considerations for which I avoid getting into this project specific things related to dependency injection. And I must say that the Proposed changes are quite invasive and affect the current design. I came across a similar approach at work. I can’t say that I like it. A lot of implicit logic, very error prone. This code is hard to maintain.

This library is a complementary layer on top that allows you to automatically wrap your .NET classes into GraphQL schema definitions using existing property getters and methods as field resolvers.

As conventions project says, I also think that the proposed changes are just an add-on built on top of the base graphql-dotnet but not its part. Maybe you look if is it better to integrate your proposals there?

[Shane32]

I had not seen that project. It is interesting. The biggest difference is probably that my code can run in conjunction with existing code, rather than replacing it. (For better or worse.)

[sungam3r]

I had not seen that project.

It is better to see it in order to understand the goals and design and not solve the same problem a second time, but in a slightly different way. At the very least, it may be possible to extend the current behavior by adding a new convention mechanism.

[Shane32]

Even better: The underlying graph type is constructed when the schema is built, and all the resolvers are compiled and set at that time. So when the queries run, there is no need to use reflection to examine methods or use caches to do so. It merely executes the precompiled field resolver, which directly calls your code. When compared on an apple-to-apples basis -- for example, using a static method with no injected services -- It should run equally as fast as the usual way of defining field resolvers.

[avin-kavish]

Hmm... I can't seem to wrap my head around it. I did notice now that all the reflection info is static. But when this class is scoped instantiated (for DI purposes) everytime a resolver is invoked in parallel, the constructor would run and invoke the initialization code. Could you clarify that for me?

[Shane32]

There are two constructors for each graph type. One is the underlying graphtype registered as a singleton and created when the schema is created. Its constructor runs when the schema is created, and it examines the other class for methods and etc.
It allows full control over how the auto-generated graphtype is constructed, allowing you to add or change fields during its execution. See below:

    public class DroidTypeDIGraph : DIObjectGraphType<DroidTypeDI, Droid>
    {
        // singleton class; gets created when the schema gets created
        // the base constructor does all the reflection stuff
        public DroidTypeDIGraph()
        {
            //add some fields using declarative graph code
            Field(d => d.Id).Description("The id of the droid.");
            Field(d => d.Name, nullable: true).Description("The name of the droid.");

            //interfaces are not yet supported as an attribute on the class below, but as
            //  the code is backwards compatible, we can still use this feature
            Interface<CharacterInterface>();
        }

        // all of the reflection guts can individually be extended or overridden by overriding methods here
    }

    [Name("Droid")]
    [Description("A mechanical creature in the Star Wars universe.")]
    public class DroidTypeDI : DIObjectGraphBase<Droid>
    {
        // scoped class; gets created upon each use of the class, unless it is calling a static member

        private StarWarsData _data; //a scoped service

        public DroidTypeDI(StarWarsData data)
        {
            _data = data;
        }

        public IEnumerable<StarWarsCharacter> Friends([FromSource] Droid droid)
            => _data.GetFriends(droid);

        [Description("A list of a character's friends.")]
        public Connection<StarWarsCharacter> FriendsConnection(
            IResolveFieldContext context,
            [FromSource] Droid droid,
            int? first, int? last, string after, string before)
        {
            var connectionContext = new ResolveConnectionContext<Droid>(context, false, 100);
            return connectionContext.GetPagedResults<Droid, StarWarsCharacter>(_data, droid.Friends);
        }

        // because the bottom two methods are static, the class is not created by the service provider
        //   for these methods, since they do not need access to _data
        [Description("Which movie they appear in.")]
        public static int[] AppearsIn([FromSource] Droid droid)
            => droid.AppearsIn;

        [Description("The primary function of the droid.")]
        public static string PrimaryFunction([FromSource] Droid droid)
            => droid.PrimaryFunction;
    }

    // however, you don't need to create the wrapper class
    [Name("Query")]
    public class StarWarsQueryDI : DIObjectGraphBase
    {
        private StarWarsData _data; //a scoped service

        public StarWarsQueryDI(StarWarsData data)
            => _data = data;

        public async Task<Droid> Droid([Required] [Description("id of the droid")] string id)
            => await _data.GetDroidByIdAsync(id);
    }

    public class Startup
    {
        public void ConfigureServices(IServiceCollection services)
        {
            // if you are using both classes as shown above, you'd register both services like this:
            services.AddScoped<DroidTypeDI>();
            services.AddSingleton<DroidTypeDIGraph>();

            // but this would be more typical, without the wrapper class:
            services.AddScoped<StarWarsQueryDI>();
            services.AddSingleton<DIObjectGraphType<StarWarsQueryDI>>();
        }
    }

The above sample assumes you register your data types with the GraphTypeTypeRegistry, not shown here.

[sungam3r]

@Shane32 It looks very complicated and duplicates a lot of code. I am absolutely sure that this will not be possible to maintain. In addition, over the past year we have already resolved some of the issues with dependency injection design. I like the current design of working with dependencies and I cannot say that it lacks functionality. Do you agree?

[Shane32]

I maintain this code at https://github.com/Shane32/GraphQL.DI

It is posted to nuget and I use it in production. No/little tests or documentation unfortunately.

I really think this is the way to go, and I think it should be part of the base library and the typical way to create graphs rather than the alternate. Much more like HotChocolate I think. Makes creating graphs a breeze compared to now with very small learning curve. It could be a duplicate of GraphQL.Conventions - I don’t know. But it builds on standard graph types so you can use both interchangeably, and is built to work like ASP.Net Core controllers.

Also it has much greater control of threading and scopes with much less work. And completely eliminates the “bad DI pattern” present now.

We can close this PR if you like.

[Shane32]

It is very complicated. There is no duplication of code, however. Nothing it does has any similarities anywhere in the base GraphQL library.

[sungam3r]

There is no duplication of code

ContextWrapper
DIExecutionStrategy
MetadataAttribute
ResolveFieldContextAdapter
ResolveFieldContextExtensions
DIScopedFieldResolver
TaskExtensions

😄

If you've managed to separate the functionality into a separate project, then that's good. At least for the purposes of independent evolution.

[Shane32]

DIExecutionStrategy works notably differently than the current execution strategies to support some of the feature set. It only overrides ExecuteNodeTreeAsync. As for the other classes, v4 has not been released yet, and the code in v4 came from here! It previously also had many dataloader classes, but once that was merged in, the "duplicate code" was removed. Once v4 is released, I will remove the remaining "duplicate code". But that's not really what the point of this library/PR is. It's about writing graphs intuitively with classes and methods and native .NET types rather than having to configure fields and define arguments and specify resolvers.

It's all about writing this:

public class Query : DIGraphType
{
    private MyDbContext _db;

    public Query(MyDbContext db)
    {
        _db = db;
    }

    public async Task<int> TotalStarWarsCharacters(int epsiode)
    {
        return await _db.People.Where(x => x.Episodes.Contains(episode)).CountAsync();
    }
}

Rather than this:

public class QueryType : ObjectGraphType
{
    public QueryType()
    {
        Field<IntGraphType, int>("TotalStarWarsCharacters")
            .Argument<IntGraphType>("episode")
            .ResolveAsync(async context =>
            {
                var episode = context.GetArgument<int>("episode");
                var db = context.RequestServices.GetRequiredService<MyDbContext>();
                return await _db.People.Where(x => x.Episodes.Contains(episode)).CountAsync();
            });
    }
}

It also is not designed to be a mapping helper, like AutoRegisteringObjectGraphType. It's designed to allow programmatic control of each field of a graph, so it can be mapped, converted or processed as the user desires.

Each field is compiled differently for its best performance. So async methods are compiled differently than sync methods, and static methods differently yet. DI services can be requested in the method arguments, and .NET attributes can be applied to any class or method to change naming and so on -- just like ASP.Net.

Finally, it also is designed to interplay nicely with all existing GraphQL.NET types and components, so intermixing of code styles is available to best suit the needs of the graph.

If GraphQL.NET is going to live on, it will need this, and it will need this as part of the main library/documentation. See how HotChocolate has this as their code-first sample: https://chillicream.com/docs/hotchocolate/v10/#code-first-approach Their "hello world" query is public string Hello() => "world";!!

[sungam3r]

OK. To be honest, I deliberately decided not to watch anything done in HotChocolate. At least for now. Just to go my own way without copying someone else's decisions.

[Shane32]

OK. To be honest, I deliberately decided not to watch anything done in HotChocolate. At least for now. Just to go my own way without copying someone else's decisions.

I am not following there either. All the code you see in this PR was designed before I ever knew of HotChocolate

[Shane32]

This feature is conceptually implemented in PRs listed within this issue:

• Auto-registering graph type todo / wish list #2893

[Shane32]

The difference between this PR and the updated functionality within the auto-registering graph type classes is that the updates to the auto-registering graph type do not allow for DI-injection of services in the constructor, but only as method arguments via [FromServices]. It is possible to allow DI-injection of services in the constructor with some customizations in a derived class, demonstrated within GraphQL.Tests.Bugs.Issue2932_DemoDIGraphType. Below is a sample copied from there that demonstrates how such a class could be written, so services are injected by DI:

        // ==== demo graph using new DIGraph<T> base class ====
        // attributes still work
        [Name("Sample")]
        public class SampleGraph : DIGraph<SampleSource>
        {
            // standard DI injection of services
            private readonly Service1 _service1;
            public SampleGraph(Service1 service1)
            {
                _service1 = service1;
            }

            // methods work with attributes and can use source property
            [Id]
            public int Id() => Source.Id;

            // can write as static methods so that an instance of SampleGraph is not constructed at runtime
            // static methods can pull source from an attribute or similar
            public static string Name([FromSource] SampleSource source) => source.Name; // do not create instance of SampleGraph

            // sample of using an asynchronous method with a cancellation token provided through the class instead of within the method
            // also demonstrates auto inference of return type and removal of "Async" from asynchronous methods
            public Task<IEnumerable<string>> ChildrenAsync() => _service1.GetChildrenAsync(Source.Id, RequestAborted);

            // sample of a static method, pulling in a specific service
            public static int Service2Test([FromServices] Service2 service2) => service2.Value;

            // sample of returning data from a scoped service
            public int Counter() => _service1.Count;

            // sample of the field resolver creating a scope for this method, and returning data from a scoped service within that scope
            [Scoped]
            public int ScopedCounter() => _service1.Count;
        }

[Shane32]

The code to implement such functionality is copied here, and quite simple:

public class DIGraphType<TDIObject, TSourceType> : AutoRegisteringObjectGraphType<TSourceType>
    where TDIObject : DIGraph<TSourceType>
{
    protected override void ConfigureGraph()
    {
        // do not configure attributes set on TSourceType
        // base.ConfigureGraph();

        // configure attributes set on TDIObject instead
        var attributes = typeof(TDIObject).GetCustomAttributes<GraphQLAttribute>();
        foreach (var attr in attributes)
        {
            attr.Modify(this);
        }
    }

    // only process methods declared directly on TDIObject -- not anything declared on TSourceType
    protected override IEnumerable<MemberInfo> GetRegisteredMembers()
    {
        return typeof(TDIObject)
            .GetMethods(BindingFlags.Instance | BindingFlags.Static | BindingFlags.Public | BindingFlags.DeclaredOnly);
    }

    // each field resolver will build a new instance of TDIObject
    protected override LambdaExpression BuildMemberInstanceExpression(MemberInfo memberInfo)
        => (Expression<Func<IResolveFieldContext, TDIObject>>)(context => MemberInstanceFunc(context));

    private TDIObject MemberInstanceFunc(IResolveFieldContext context)
    {
        // create a new instance of TDIObject, filling in any constructor arguments from DI
        var graph = ActivatorUtilities.CreateInstance<TDIObject>(context.RequestServices ?? throw new MissingRequestServicesException());
        // set the context
        graph.Context = context;
        // return the object
        return graph;
    }
}

public abstract class DIGraph<TSourceType>
{
    public IResolveFieldContext Context { get; internal set; } = null!;
    public TSourceType Source => (TSourceType)Context.Source!;
    public CancellationToken RequestAborted => Context.CancellationToken;
}

[Shane32]

Note that replacing the call to CreateInstance with context.RequestServices.GetRequiredService<TDIObject>() would improve speed but the classes would need to be registered as transients within the DI provider since the code to create the instance would then likely be precompiled. Still, a new instance is created for every field resolver, so using this code for all graph types isn't ideal. Rather, if the Context, Source and RequestAborted properties were eliminated from the above sample, then the classes derived from DIGraph could be registered as scoped, so it would only be created once per request. Just sorta depends on how it will be used and how much the tiny performance improvement matters.