using of IDataLoaderResult<NonNull<List<X>>> leads to error

[syaylev-minbox]

Seems that "conventions" missing some additional unwrapper for that case. Please see unittest below.

I get an exception because ExecutionStrategy expects to see IEnumerable but it gets NonNull<List>. Seems that at some point graphql-dotnet decided to require IEnumerable in situations where NonNull is declared.

If you try to get NonNull<List> you will be succeeded. Also if you try to get IDataLoaderResult<NonNull> you will be succeeded too.

This is quite critical issue because this gives no ability to define type [X]! and use dataload at the same time.

Exception details:

Expected an IEnumerable list though did not find one. Found: NonNull`1
   at GraphQL.Execution.ExecutionStrategy.SetArrayItemNodes(ExecutionContext context, ArrayExecutionNode parent) in /_/src/GraphQL/Execution/ExecutionStrategy.cs:line 125
   at GraphQL.Execution.ExecutionStrategy.CompleteNode(ExecutionContext context, ExecutionNode node) in /_/src/GraphQL/Execution/ExecutionStrategy.cs:line 298

Unittest that fails (sorry for verbose):

    public class GqlNullableListTests : ConstructionTestBase
    {
        [Test]
        public async Task DataLoaderFail()
        {
            const string query = @"{ testClasses { x } }";

            var schema = Schema<TestRootSchema>();

            var result = await schema.ExecuteAsync((e) => e.Query = query);

            ResultHelpers.AssertNoErrorsInResult(result);
        }

        private class TestRootSchema {
            public TestQuery Query { get; }
        }

        public class TestQuery
        {
            private DataLoaderContext dataLoaderContext = new DataLoaderContext();

            private static List<TestClass> testClasses = new List<TestClass> {new TestClass {X = 3}};

            public NonNull<List<TestClass>> TestList { get; set; } = new NonNull<List<TestClass>>(testClasses);

            public IDataLoaderResult<NonNull<List<TestClass>>> TestClasses()
            {
                var loader = dataLoaderContext.GetOrAddBatchLoader<int, NonNull<List<TestClass>>>(
                    "Test",
                    async ids =>
                    {
                        var list = new NonNull<List<TestClass>>(testClasses);
                        // same list for all ids for simplicity
                        var dict = ids.ToDictionary(id => id, value => list);
                        return dict;
                    }
                );

                return loader.LoadAsync(1);
            }

            public class TestClass { public int X { get; set; } }
        }
    }

[syaylev-minbox]

For history. I have found one method to solve this that makes no pleasure at all. I have copy-pasted BatchDataLoader<TKey, T>, DataLoaderBase<TKey, T>, DataLoaderBase<TKey, T>.DataLoaderList and DataLoaderPair<TKey, T> classes and changed theirs signatures little bit.

Modified dataLoader must return IDataLoaderResult<NonNull<List<T>>>, but it also must fetch dictionary of List<T>. In this case ExecutionStrategy will be satisfied and gql-schema will treat TestClasses as [TestClass]!

Here is fragments of code just to illustrate an idea.

public IDataLoaderResult<**NonNull<List<TestClass>>**> TestClasses2()
{
    var loader = dataLoaderContext.GetOrAddNonNullBatchLoader<int, **List<TestClass>**>(
        "Test",
        async (ids, ct) =>
        {
            // same list for all ids for simplicity
            return ids.ToDictionary(id => id, value => testClasses);
        }
    );

    return loader.LoadAsync(1);
}

public sealed class NonNullDataLoaderPair<TKey, T> : IDataLoaderResult<**NonNull**<T>> where T : class
{
    public T Result => ...;

    public void SetResult(T value) => ...;

    public async Task<NonNull<T>> GetResultAsync(CancellationToken cancellationToken = default)
    {
        if (!IsResultSet)
            await Loader.DispatchAsync(cancellationToken).ConfigureAwait(false);
        return new NonNull<T>(Result);
    }

    async Task<object> IDataLoaderResult.GetResultAsync(CancellationToken cancellationToken)
    {
        if (!IsResultSet)
            await Loader.DispatchAsync(cancellationToken).ConfigureAwait(false);
        return Result;
    }
}

public static IDataLoader<TKey, NonNull<T>> GetOrAddNonNullBatchLoader<TKey, T>(this DataLoaderContext context, string loaderKey, Func<IEnumerable<TKey>, CancellationToken, Task<IDictionary<TKey, T>>> fetchFunc,
    ...) 
    where T : class
{
    ...
    return context.GetOrAdd(loaderKey, () => new NonNullBatchDataLoader<TKey, T>(fetchFunc, keyComparer, defaultValue, maxBatchSize));
}

public partial class NonNullBatchDataLoader<TKey, T> : IDataLoader, IDataLoader<TKey, NonNull<T>> 
    where T : class
{   
    private readonly Func<IEnumerable<TKey>, CancellationToken, Task<IDictionary<TKey, T>>> _loader;

    public NonNullBatchDataLoader(Func<IEnumerable<TKey>, CancellationToken, Task<IDictionary<TKey, T>>> fetchDelegate,
           ...) : this(keyComparer, maxBatchSize)
    {
        _loader = fetchDelegate ?? throw new ArgumentNullException(nameof(fetchDelegate));
        _defaultValue = defaultValue;
    }

    **public IDataLoaderResult<NonNull<T>> LoadAsync(TKey key)** => ...;

    **protected async Task FetchAsync(IEnumerable<NonNullDataLoaderPair<TKey, T>> list, CancellationToken cancellationToken)** => ...;

    private class DataLoaderList : Dictionary<TKey, NonNullDataLoaderPair<TKey, T>>, IDataLoader
    {
        ...
    }
}

[tlil]

Will bump the version in a bit. Thanks @Shane32 , and thanks for reporting @syaylev-minbox

[tlil]

New package is up btw :)