Split Construction of Graph Types into individual modules

[EmmanuelPonnudurai]

Hello. The examples I have seen thus far are simplistic at best and don't provide a reference for how we need to structure things in a real-world application. This is my attempt to do so.

Especially when we use dependency injection and when there can be a lot of queries and mutations. (Note that I have not added a mutation yet).

We can build on this example and add things so that it's easy for increased adoption or at least serve as a point of reference.

Open to any suggestions and improvements. Thanks in advance.

[sungam3r]

Please use file-scoped namespaces. We will convert codebase to use filescoped namespaces some time.

[sungam3r]

I mean only for new files. Or just leave it as is, not a big deal.

[sungam3r]

Suggested change

	new Dog{ Breed = "Doberman" },
	new Dog{ Breed = "Pit Bull" },
	new Dog{ Breed = "German Shepard" }
	new Dog { Breed = "Doberman" },
	new Dog { Breed = "Pit Bull" },
	new Dog { Breed = "German Shepard" }

[sungam3r]

Suggested change

	new Cat{ Breed = "Abyssinian" },
	new Cat{ Breed = "American Bobtail" },
	new Cat{ Breed = "Burmese" }
	new Cat { Breed = "Abyssinian" },
	new Cat { Breed = "American Bobtail" },
	new Cat { Breed = "Burmese" }

[sungam3r]

Suggested change

	foreach(var field in fields)
	foreach (var field in fields)

[sungam3r]

Suggested change

	foreach(var catOperation in catOperations)
	foreach (var catOperation in catOperations)

[sungam3r]

Maybe it's better to use AutoSchema here.

[EmmanuelPonnudurai]

Can you give me some more details here? Do you have any doc handy for AutoSchema? I'm thinking it is some way to set things up automatically.

One reason I am doing the way I am right now, is to be non-opinionated on the setup. I give control to the implementor of IQuery to give me the fields as they see fit. They can subsequently use reflection or any other controlled mechanism to set things up and finally return the fields which I would then attach to the root query. This is especially useful when the application is large, and consumers need several ways to opt in/out of things.

Again, I might be talking about something different but will wait for information on AutoSchema.

[Shane32]

I'm not really sure how the use of AutoSchema fits into your example. AutoSchema is useful when creating a type-first schema, where the schema is automatically constructed from CLR types. Your sample does not use those features. Moreover, you are attempting to use methods from multiple classes to assemble a large root graph type. An auto-generated root query type would be contrary to that goal.

[Shane32]

You can see a demonstration of a type-first schema here:

https://github.com/graphql-dotnet/graphql-dotnet/tree/master/src/GraphQL.StarWars.TypeFirst

The entire schema can be constructed via:

services.AddGraphQL(b => b
    .AddSystemTextJson()
    .AddAutoSchema<StarWarsQuery>(c => c.WithMutation<StarWarsMutation>()));

You will notice there are no 'graph types' but rather simple CLR classes decorated with attributes where necessary

[Shane32]

In my own graphs, I use a slight variation of this to allow for DI injection of scoped services, which is not possible with the sample seen there. However, the code exists in the tests here:

https://github.com/graphql-dotnet/graphql-dotnet/blob/master/src/GraphQL.Tests/Bugs/Issue2932_DemoDIGraphType.cs

I also published it as a NuGet package here:

https://www.nuget.org/packages/Shane32.GraphQL.DI

[sungam3r]

@EmmanuelPonnudurai I'm fine with example as is without AutoSchema.

[sungam3r]

I would inline this method into ConfigureServices.

[sungam3r]

Field<StringGraphType>( appends field to graph type and returns reference to it.

[sungam3r]

See comments. I'm done here. Waiting for review from @Shane32 .

[EmmanuelPonnudurai]

@sungam3r @Shane32 Addressed comments. Pls take a look when you can.

[Shane32]

Waiting for review from @Shane32 .

I see this is attempting to use DI to build a query (or mutation) type dynamically. This makes sense, although I would not recommend building a set of fields manually (versus using the Field methods), at least not for a primary example, as shown in the GetFields() method. I might do this for an advanced use case, of course. I might suggest this instead:

interface IProvideFields
{
    void BuildQueryFields(ObjectGraphType graph) { }
    void BuildMutationFields(ObjectGraphType graph) { }
    void BuildSubscriptionFields(ObjectGraphType graph) { }
}

class DogHandler : IProvideFields
{
    // pull in singleton services here

    void IProvideFields.BuildQueryFields(ObjectGraphType graph)
    {
        graph.Field<Dog>("Dog")
            .Argument<int>("id")
            .Resolve(ctx => {
                // pull scoped services and retrieve dog by id
            });
        // add "Dogs" or other fields here
    }

    // optional: implement BuildMutationFields / BuildSubscriptionFields
}

class Query : ObjectGraphType
{
    public Query(IEnumerable<IProvdeFields> fieldProviders)
    {
        foreach (var fp in fieldProviders)
            fp.BuildQueryFields(this);
    }
}

services.AddSingleton<IProvideFields, DogHandler>();
services.AddSingleton<IProvideFields, CatHandler>();

In such a manner the same handler can be used for query, mutation, and subscription fields.

Note that I do not use such a design pattern, but I have no issue providing a demonstration of this or other design patterns. Some day I would like to provide an example of my own design pattern for large schemas.

On a side note, the title of the PR seems misleading to me; I typically think of "injecting dependencies" as meaning to assist injecting dependent services into graph types. However, the PR here does not really help as singleton and scoped services still cannot be injected in any other manner than is already available with typical graph construction. Rather, the PR aims to utilize DI to split the construction of the root graph types into individual modules based on the underlying data type it is serving.

[sungam3r]

Suggested change

[EmmanuelPonnudurai]

Waiting for review from @Shane32 .

I see this is attempting to use DI to build a query (or mutation) type dynamically. This makes sense, although I would not recommend building a set of fields manually (versus using the Field methods), at least not for a primary example, as shown in the GetFields() method. I might do this for an advanced use case, of course. I might suggest this instead:

interface IProvideFields
{
    void BuildQueryFields(ObjectGraphType graph) { }
    void BuildMutationFields(ObjectGraphType graph) { }
    void BuildSubscriptionFields(ObjectGraphType graph) { }
}

class DogHandler : IProvideFields
{
    // pull in singleton services here

    void IProvideFields.BuildQueryFields(ObjectGraphType graph)
    {
        graph.Field<Dog>("Dog")
            .Argument<int>("id")
            .Resolve(ctx => {
                // pull scoped services and retrieve dog by id
            });
        // add "Dogs" or other fields here
    }

    // optional: implement BuildMutationFields / BuildSubscriptionFields
}

class Query : ObjectGraphType
{
    public Query(IEnumerable<IProvdeFields> fieldProviders)
    {
        foreach (var fp in fieldProviders)
            fp.BuildQueryFields(this);
    }
}

services.AddSingleton<IProvideFields, DogHandler>();
services.AddSingleton<IProvideFields, CatHandler>();

In such a manner the same handler can be used for query, mutation, and subscription fields.

Note that I do not use such a design pattern, but I have no issue providing a demonstration of this or other design patterns. Some day I would like to provide an example of my own design pattern for large schemas.

On a side note, the title of the PR seems misleading to me; I typically think of "injecting dependencies" as meaning to assist injecting dependent services into graph types. However, the PR here does not really help as singleton and scoped services still cannot be injected in any other manner than is already available with typical graph construction. Rather, the PR aims to utilize DI to split the construction of the root graph types into individual modules based on the underlying data type it is serving.

@Shane32 Thank you for the feedback. This is completed pls check.

One thing I do feel is the lack of an easy way to create a FieldType in a functional manner. That is, without itself attaching to the current ObjectGraphType. I can open a change to have a Utility method somewhere called CreateField which would basically be similar the Field or Field<> or FieldAsync etc. But it would just be a side effect free abstraction to create the FieldType and return it.

I did search around for it but couldn't find such a method.

One wrinkle that I have though with the current approach is that it's not functional in nature. We pass this reference over to the other method which adds things and also does it like graphType.Field. I would love for the other method to not mutate the object I am passing and rather I do that from within the RootQuery.

[EmmanuelPonnudurai]

@sungam3r @Shane32 this is ready for review again.

[sungam3r]

One thing I do feel is the lack of an easy way to create a FieldType in a functional manner. That is, without itself attaching to the current ObjectGraphType.

FieldBuilder<TSourceType, TReturnType>.Create ?

[joemcbride]

@EmmanuelPonnudurai First - love your avatar, that is one of my favorite characters.

As far as this example goes, and as @Shane32 has kind of already mentioned, I think there are better approaches than what this example is demonstrating. I would highly suggest to avoid coupling dynamic creation of GraphTypes using "mutation" interfaces and "field providers" to "share" field resolution.

You can create a small framework to dynamically create GraphTypes, but this example is tightly coupling your domain to the GraphQL.NET framework. That means every time the GraphQL.NET project changes, you could have lots of work to do to get your project upgraded.

The Schema-First approach shows how you can almost completely de-couple your field resolvers from the main GraphQL.NET framework. So Schema-First and the Type-First example Shane linked should be preferred over this example.

I would highly suggest to avoid using GraphQL.NET types as much as possible in your domain code. Merging this example would seem like this type of approach would be a "suggested" or "sanctioned" usage of this framework, which I think would lead people to having less maintainable code and make it harder for their systems to evolve over time.

[EmmanuelPonnudurai]

@EmmanuelPonnudurai First - love your avatar, that is one of my favorite characters.

As far as this example goes, and as @Shane32 has kind of already mentioned, I think there are better approaches than what this example is demonstrating. I would highly suggest to avoid coupling dynamic creation of GraphTypes using "mutation" interfaces and "field providers" to "share" field resolution.

You can create a small framework to dynamically create GraphTypes, but this example is tightly coupling your domain to the GraphQL.NET framework. That means every time the GraphQL.NET project changes, you could have lots of work to do to get your project upgraded.

The Schema-First approach shows how you can almost completely de-couple your field resolvers from the main GraphQL.NET framework. So Schema-First and the Type-First example Shane linked should be preferred over this example.

I would highly suggest to avoid using GraphQL.NET types as much as possible in your domain code. Merging this example would seem like this type of approach would be a "suggested" or "sanctioned" usage of this framework, which I think would lead people to having less maintainable code and make it harder for their systems to evolve over time.

@joemcbride Thanks for the response. And glad to know what you are a Samurai X fan :)

When you have 100's of queries and mutations. Individually requiring 100's of domain scoped dependencies. We need to organize things without too much in lining or auto magic. This is my main motive.

Some may like the Auto stuff, but I prefer seeing the interaction clearly with the library. Also, if you go with Auto, im sure you will have to end up with some attributes for bailing out of things or fine-tuning stuff as needed. So, it comes down to a preference, I guess. Also, the only touchpoint for me with GraphQL.NET is the Field<> operation. That would not change, would it?

I am still using the same things that are required (adding fields), regardless of approach. Only thing is I'm organizing things in such a way that anyone else can add queries and mutations in a clear manner by implementing the necessary interfaces (which is again domain specific but asking you to add Fields)

This approach avoids hooking into the abstraction provided by GraphQL.NET by via of Auto Schema or auto things etc.

I do agree that this is a bit opinionated. However, I need to see a sample where we have at least 2 different queries and mutations, driven off domain dependencies which can be scoped or transient, and they are doing completely different things. Is there a sample available for this or is it mostly up to the consumer to decide which way they want to do it?

I will look at the samples which are shared by @Shane32

Any other pointers for documentation or suggestions are welcome. If I can do it better and still maintain isolation between each operation, that would suffice for me.

[Shane32]

Just my two cents here, but there certainly are different methods to build a large-scale application.

One of my medium-size applications has the data models (about 85 of them) and migrations in one library, with no references to GraphQL.NET of any kind. Then there's a service layer, which provides the business logic and other services required by the application. Then there's a GraphQL library, which contains code-first mappings from each data model into graphs, plus mutations with input models, and mappings from those input models back to the data layer. None of the data models are auto-mapped to graphs. Finally, there's the application layer which hosts the ASP.NET Core endpoint. So each of these layers is built on the previous layer.

I'm also sure that while my design works fine at its size, I may need another design if I were dealing with a larger application, say of 200-500 data models, as my Query.cs file contains about 1,500 lines of code already. I would likely seek some method of code-splitting the root query/mutation classes such as is presented in this PR.

In another of my applications, again the database models (55 of them) are in its own library, but the models use interfaces and attributes in a well-known manner, such that the GraphQL layer can auto-generate about 95% of the graphs from metadata already known to the database layer. Any graphs requiring special attention are 'overridden' with a custom graph type implementation. This does create a tighter relationship from GraphQL to the database engine, but has huge benefits in terms of simplicity. This is certainly not suitable for everyone, but suited my specific needs with minimal effort, and has almost no maintenance requirements.

In some of my smaller applications, I've integrated GraphQL annotations directly on the data types in a type-first approach, but the code can quickly get messy when the data models have intermixed database annotations and graphql annotations, such as @joemcbride was alluding to. However, this might work well if you were building an application small enough that all the code resided in a single project -- perhaps one with only 5-10 data models.

I suggest that when your sample application is finished, we be sure to write a few paragraphs explaining the pros and cons of the design approach taken. We might also not wish to overwrite the present sample, but rather to add a new sample to those that are already existing. Note that as this sample is not in the main repository, but rather in the examples repository, I feel it is suitable to provide samples that may not be the generally suggested approach, so long as we have adequate documentation explaining the purpose of the sample.

[Shane32]

Also, the only touchpoint for me with GraphQL.NET is the Field<> operation. That would not change, would it?

I guess I'm not sure what you're asking. You previously mentioned you were looking for a way to construct a FieldType using a builder without using the graphtype.Field method, as this would require a reference to the hosting graph type. @sungam3r indicated that this can be accomplished via FieldBuilder<>.Create. Is this what you were looking for?

[sungam3r]

I suggest that when your sample application is finished, we be sure to write a few paragraphs explaining the pros and cons of the design approach taken. We might also not wish to overwrite the present sample, but rather to add a new sample to those that are already existing. Note that as this sample is not in the main repository, but rather in the examples repository, I feel it is suitable to provide samples that may not be the generally suggested approach, so long as we have adequate documentation explaining the purpose of the sample.

Agree. I'm fine with either approach while it is documented with pros/cons (not even necessary in detail).

[EmmanuelPonnudurai]

So is the consensus that I create an entirely different solution for this example. Something like AspNetCoreOrganizationSample, add documentation and open another PR? @Shane32 @sungam3r @joemcbride?

[EmmanuelPonnudurai]

Also, the only touchpoint for me with GraphQL.NET is the Field<> operation. That would not change, would it?

I guess I'm not sure what you're asking. You previously mentioned you were looking for a way to construct a FieldType using a builder without using the graphtype.Field method, as this would require a reference to the hosting graph type. @sungam3r indicated that this can be accomplished via FieldBuilder<>.Create. Is this what you were looking for?

The FieldBuilder.Create is kind of asking me to create the filed completely. Which is what I had earlier by way of doing

var fieldType = new FieldType();
fieldType.Name = "someName";
fieldType.Type = typeof(NonNullGraphType<StringGraphType>);
fieldType.Resolver = resolverFunc;

return fieldType;

My point was that Field<>, AddField are all creating the field, adding it to the graph and returning an instance. I was looking for some static functionality which has a signature like what is there for Field<> but doesn't actually add it to the graph but just return the created field for the consumer to act on later as needed. So a sample would be

public FieldType CreateField<TGraphType>(string name, string? description = null, QueryArguments? arguments = null, Func<IResolveFieldContext<TSourceType>, object?>? resolve = null, string? deprecationReason = null) where TGraphType : IGraphType;

The only difference between existing Field<> and proposed CreateField is that the CreateField would NOT add it to the graph. Its just a utility to create and retugn the FIeldType.

Again this is a variant which i am looking for to facilitate the way I am structing things.

I can create local utility functions to do this kind of work so not the end of the world.

[Shane32]

The Field methods you are referring to have been marked as [Obsolete] and are not recommended for new code. They will be removed in a future version of GraphQL.NET.

See:

• https://github.com/graphql-dotnet/graphql-dotnet/blob/master/src/GraphQL/Types/Composite/ComplexGraphType.cs#L167

The suggested pattern for GraphQL.NET v7 is to use the field builders to define fields. Besides matching the current documentation and samples, this allows for extension methods to work properly, such as .AuthorizeWithPolicy() and the resolver builder methods available in the GraphQL.MicrosoftDI project or the dataloader overloads available in the GraphQL.Dataloader project.

We also do not suggest creating a FieldType 'manually' as this again does not allow use of those or other extension methods. I would suggest creating these extension methods (and possibly others if desired):

        // specify by graph type
        public static FieldBuilder<object, TReturnType> Field<TGraphType, TReturnType>(this ICollection<FieldType> fields, string name)
            where TGraphType : IGraphType
        {
            var builder = FieldBuilder<object, TReturnType>.Create(typeof(TGraphType)).Name(name);
            fields.Add(builder.FieldType);
            return builder;
        }

        // specify by CLR type
        public static FieldBuilder<object, TReturnType> Field<TReturnType>(this ICollection<FieldType> fields, string name, bool nullable = false)
        {
            // note: only output type supported here
            var type = typeof(TReturnType).GetGraphTypeFromType(nullable, TypeMappingMode.OutputType);
            var builder = FieldBuilder<object, TReturnType>.Create(type).Name(name);
            fields.Add(builder.FieldType);
            return builder;
        }

Then your classes could do something like this:

public DogQuery : IDogQuery
{
    public IEnumerable<FieldType> GetQueryFields()
    {
        var fields = new List<FieldType>();

        fields.Field<Dog>("dog")
            .Argument<IdGraphType>("id")
            .Resolve()
            .WithScope()
            .WithService<DogRepository>()
            .Resolve((ctx, dogRepository) => dogRepository.GetDog(ctx.GetArgument<int>("id")));

        return fields;
    }
}

I'd probably still pass in a collection, even if it is a new empty one, just so as to eliminate the redundant var fields = new List<FieldType>(); and return fields; in each of these methods.