Restrict list types in GetGraphTypeFromType

[Shane32]

I keep running into this problem in my code: GetGraphTypeFromType takes classes that inherit or implement a list or dictionary type and assign it a graph type mapping of the KeyValuePair rather than the object as a whole.

For example:

private MyInfoClass : Dictionary<string, object>
{
    public string Name => this["Name"];
}

var type = GetGraphTypeFromType(typeof(MyInfoClass), true, TypeMappingMode.OutputType);
//expected:
type == typeof(GraphQLClrOutputGraphType<MyInfoClass>)
//actual:
type == typeof(ListGraphType<GraphQLClrOutputGraphType<KeyValuePair<string, object>>>);

My workaround is currently as follows:

1. Apply the patch in this PR to a custom version of the GetGraphTypeFromType method
2. Create a class derived from TypeInformation and override ConstructGraphType to use the new method
3. Create a class dervied from AutoRegisteringObjectGraphType and override GetTypeInformation to use the new class
4. Register the new class within DI to be used when AutoRegisteringObjectGraphType is requested.

My suggested fix is to only detect explicit enumerable types that we have already predefined in the TypeInformation class:

• IEnumerable
• IEnumerable<>
• IList<>
• List<>
• ICollection<>
• IReadOnlyCollection<>
• IReadOnlyList<>
• HashSet<>
• ISet<>

Any derived types from those types will not be detected as a list. This is a breaking change, and if someone is relying on the prior behavior, they will need to cast their custom return type to a detected list type. For example:

public class People : List<Person>
{
}

var type = GetGraphTypeFromType(typeof(People), true, TypeMappingMode.OutputType);
//previously:
type == typeof(ListGraphType<GraphQLClrOutputGraphType<Person>>)
//now:
type == typeof(GraphQLClrOutputGraphType<People>)

//breaking change affects uses like this:
Field(x => x.People); //where People is of type People not of type List<Person>

My workaround (described above) contains almost 200 lines of code to fix this problem (mostly copied from the main library). I suggest we fix it in the main library. However, it is a breaking change and there may be existing code that relies on the current behavior. As much as I would like to see it in 5.1 for my own needs, I suggest this be a change for 6.0. I can live with my existing workaround.

An alternative option would be to make this a global option. Even though I generally discourage global options, this really seems like a pain point (for me) that we can get rid of rather easily.

[Shane32]

Unfortunately my patch is required in multiple private libraries - ugh!

[sungam3r]

As much as I would like to see it in 5.1 for my own needs, I suggest this be a change for 6.0.

Let's keep it in 5.x.x timeline but with new global option that by default preserves old behavior. This option may be deprecated in v6.

[codecov-commenter]

Codecov Report

Merging #3099 (8f19a5c) into master (23361a7) will decrease coverage by 0.09%.
The diff coverage is 14.28%.

@@            Coverage Diff             @@
##           master    #3099      +/-   ##
==========================================
- Coverage   84.40%   84.30%   -0.10%     
==========================================
  Files         361      362       +1     
  Lines       15853    15884      +31     
  Branches     2578     2580       +2     
==========================================
+ Hits        13380    13391      +11     
- Misses       1859     1876      +17     
- Partials      614      617       +3     

Impacted Files	Coverage Δ
src/GraphQL/TypeExtensions.cs	79.56% <0.00%> (-6.82%)	⬇️
src/GraphQL/GlobalSwitches.cs	100.00% <100.00%> (ø)
src/GraphQL/Types/TypeInformation.cs	95.65% <100.00%> (ø)
src/GraphQL/Types/Schema.cs	78.99% <0.00%> (-0.92%)	⬇️
src/GraphQL/IConfigureAutoSchema.cs	100.00% <0.00%> (ø)
src/GraphQL/Types/AutoSchema.cs	100.00% <0.00%> (ø)
src/GraphQL/GraphQLBuilderExtensions.cs	96.00% <0.00%> (+0.06%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ccf984...8f19a5c. Read the comment docs.

[Shane32]

@sungam3r I added more details to the xml comment for the global switch if you want to review it.