graphql-platform icon indicating copy to clipboard operation
graphql-platform copied to clipboard

Support lists in Apollo Federation reference resolvers

Open luke-pp opened this issue 11 months ago • 2 comments

Product

Hot Chocolate

Version

14.2.0

Link to minimal reproduction

https://github.com/ChilliCream/graphql-platform/commit/e96c3b71afc9e9a4d0621e9cf65f207699abcf2a#diff-9ffdbfd9cc96a0b754318fd0f72577c4f85bbc8721d5ade34204c92a0aa07c25R371

Steps to reproduce

  1. Create a GraphQL type with a reference resolver using HotChocolate.ApolloFederation
  2. Add a field that is a list, and mark it as [External]
  3. 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);
        }
    }
  1. 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.

luke-pp avatar Feb 04 '25 20:02 luke-pp

Create a PR and we will see what needs to be done.

michaelstaib avatar Feb 05 '25 09:02 michaelstaib

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

1rre avatar Apr 03 '25 07:04 1rre