Support lists in Apollo Federation reference resolvers
Product
Hot Chocolate
Version
14.2.0
Link to minimal reproduction
https://github.com/ChilliCream/graphql-platform/commit/e96c3b71afc9e9a4d0621e9cf65f207699abcf2a#diff-9ffdbfd9cc96a0b754318fd0f72577c4f85bbc8721d5ade34204c92a0aa07c25R371
Steps to reproduce
- Create a GraphQL type with a reference resolver using HotChocolate.ApolloFederation
- Add a field that is a list, and mark it as
[External] - Add the list field as an argument to the reference resolver Example:
public class ListFieldType
{
public ListFieldType(string id, List<double> listField)
{
Id = id;
ListField = listField;
}
[Key]
[External]
public string Id { get; }
[External]
public List<double> ListField { get; }
public string InternalField { get; set; } = "InternalValue";
[ReferenceResolver]
public static ListFieldType GetByExternal(string id, List<double> listField, IResolverContext context)
{
return new(id, listField);
}
}
- Query the reference resolver, passing in a value for the external field
What is expected?
At this point, I expect to see a value passed into the reference resolver's arguments.
What is actually happening?
The value passed into the reference resolver is always null.
Relevant log output
Additional context
I have created a branch that adds this case to the existing tests dealing with External fields. The root cause seems to be that the ArgumentParser class does not currently handle list types - I have also added the beginnings of an implementation in the branch to confirm that adding that missing logic does fix the issue.
While I don't see any explicit confirmation in the Apollo documentation that external list fields are supported, it does seem like something that should work.
I'm happy to create a PR based on the branch linked above if we agree that this functionality is something worth supporting - it'll need a bit of tidying up as there are some edge cases I haven't fully implemented handling for yet, and the way reflection is being used is a bit messy.
Create a PR and we will see what needs to be done.
Additionally on this, I'm pretty sure this shouldn't be true currently for objects, lists etc. and leads to unhandled exceptions later on:
https://github.com/ChilliCream/graphql-platform/blob/170994cf181ef1a2764b36f02e721b5aecb62f54/src/HotChocolate/ApolloFederation/src/ApolloFederation/Resolvers/ReferenceResolverArgumentExpressionBuilder.cs#L51