Nullable reference types not detected in mutation parameters

[breyed]

If a mutation uses separate parameters (vs. a parameters object), any of the parameters that are of a reference type and are marked as nullable still show up as non-nullable in the GraphQL.

For example, in a mutation:

void Update(InputType a, InputType? b) { ... }

Should output the GraphQL:

update(a: InputType!, b: InputType)

Currently, it outputs:

update(a: InputType!, b: InputType!)

EntityGraphQL should inspect the nullability:

var nullabilityContext = new NullabilityInfoContext();
var nullabilityInfo = nullabilityContext.Create(parameterInfo);
bool isNullable = nullabilityInfo.WriteState is NullabilityState.Nullable;

[bzbetty]

Pretty sure this is an issue in ArgType.FromParameter.

It's passing through prop.Member to MakeArgType which is the parent method instead of the parameter itself.

Probably need to switch from using MemberInfo to ICustomAttributeProvider to achieve this. I thought I did that in one of the validation branches, but maybe it never made it.

[lukemurray]

Thanks for the test. I'm doing a fix for this now. Also found an issue where autoAddInputTypes was not being used when not using MutationArgumentsAttribute.

[Eli-Black-Work]

@lukemurray Would this also fix an issue that I'm running into where nullable collections seem to not be recognized as nullable?

#nullable enable
public class User {
   public ICollection<Role>? Roles { get; set; } = null;
}

public static void ConfigureSchema(SchemaProvider<...> schema)
   schema.AddType<User>("user", null).AddAllFields();
}

query:

{"Query":" { users { roles { id } }"}

error:

{"errors":[{"message":"Field 'users' - Value cannot be null. (Parameter 'source')"}]}

It looks like Entity GraphQL is expecting the ICollection<Role>? Roles to be non-null, although it's marked as nullable.

If I should open a separate issue for this, just let me know, but I thought it might be related 🙂

[lukemurray]

This is actually hitting an error in the built expression. Likely users.Select(u => u.Roles.Select(r => new { id = r.Id })) where either users or one of there user .Roles is null.

[Eli-Black-Work]

@lukemurray Thanks 🙂 Agreed that it's hitting an error in the built expression. I think the expression in this instance is built by EntityGraphQL, though.

Here's a more full snippet:

#nullable enable
public class UserDbContext : DbContext {
   public DbSet<UserWrapper> UserWrappers { get; set; }
}

public class UserWrapper {
   public string Json { get; set; }
}

public class User {
    public ICollection<Role>? Roles { get; set; } = null;
}

services.AddGraphQLSchema<UserDbContext>(options =>
{
    options.ConfigureSchema = ConfigureSchema;
});

public static void ConfigureSchema(SchemaProvider<UserDbContext> demoSchema)
{
    demoSchema.AddType<User>("user", null).AddAllFields();

    // Add custom root fields
    demoSchema.UpdateQuery(queryType =>
    {
        queryType.ReplaceField(
            "users",
            new
            {
                ...
            },
            (db, args) => db.UserWrappers.Select(userWrapper => JsonConvert.DeserializeObject<User>(userWrapper.Json)).ToList(),
            "List of users"
        );
    });
}

[lukemurray]

Yes, I meant an expression that EntityGraphQL built.

Like any code users.Select(u => u.Roles.Select(r => new { id = r.Id })) will throw if something is null in your data.

If executing against EF we can get away with it as the expression is translated by EF to SQL so the null property is not actually hit.

If the data is not EF data context and there is a null EntityGraphQL tries to handle this by doing null checks. So you've likely hit an edge case where that null check is not working correctly.

E.g. EntityGraphQL builds something closer to this

users == null ? null : users.Select(u => u.Roles == null ? null : u.Roles.Select(r => new { id = r.Id }))

And typing that out I realise it is likely the inner check failing.

[lukemurray]

Can I ask what UserWrappers is? Is it a table via EF or something outside of the DB?

[Eli-Black-Work]

@lukemurray UserWrappers is defined at the top of the example in my previous post 🙂

(Sorry, I pretty heavily edited my previous post after I posted it, so if you got an email with my reply, it probably has the old example in it)

public class UserDbContext : DbContext {
   public DbSet<UserWrapper> UserWrappers { get; set; }
}

public class UserWrapper {
   public string Json { get; set; }
}

[Eli-Black-Work]

I'm off of work for today. Will check in again tomorrow, God willing 🙂

[lukemurray]

Whoops I did miss that. Thanks

[Eli-Black-Work]

@lukemurray It looks like this bug probably isn't related to this issue, so I've split it off into a separate bug report: #221

@breyed Sorry for the noise! 🙂