Mutation Without Input Bypasses NotEmpty Validation

[JarrydVanHoy]

Here is my basic validation:

public class AddUsersInputValidator : AbstractValidator<List<AddUserInput>>
{
	public AddUsersInputValidator()
	{
		RuleFor(r => r).NotEmpty().WithMessage("Must provide input");
	}
}

Issue 1: If I change from List<> to IEnumerable<> or T[], then this validation never seems to fire.

Issue 2: If I neglect to add the input as part of my request, then I'll be able to enter my endpoint which appears to bypass my validation.
The validation appears to work when I pass a mutation as follows:

mutation{
  addUser(input: []){
    firstName
  }
}

But fails to perform validation with the following mutations:

mutation{
  addUser{
    ...

or

mutation{
  addUser(){
    ...

or

mutation{
  addUser(input: null){
    ...

Is there something wrong with what I'm doing or is there an issue with FluentValidation, FairyBread, or even HotChocolate?

Versions:

• FairyBread: 7.0.0-rc.1
• HotChocolate: 11.3.1
• FluentValidation: 10.3.0

[benmccallum]

Hey!

FairyBread doesn't validate an argument that is null. It's sort of assumed that if the argument type supports null at the schema level that that is valid. I'm not even sure how I would do that validation. FV might not like ValidateAsync<T>(null), would need to check.

Firstly, to me, it seems a bit bizarre to allow the passing of a null here, so I'd first change it to:
addUsers(input: [AddUserInput!]!)

That way HotChocolate will actually validate it to the graphql spec and client tooling will show that null isn't allowed.

One scenario where null could be allowed but you sometimes want to validate it isn't null, would be a property within the input type where null is allowed conditionally with When. In this case though, the input type would be non-null.
e.g.

createBooking(input: CreateBookingInput!): CreateBookingPayload!

type CreateBookingInput {
  deliveryAddress: String!
  useDeliveryAddressForBillingAddress: Boolean!
  billingAddress: String
}

(with a validator that allows null for billing address if useDeliveryAddressForBillingAddress is true.

That brings me to how I see this. I think the design of your mutation field is could be improved. Keen to see what @michaelstaib or @PascalSenn think. To me you should always have a single, top-level arg called input on your mutation field that's always non-nullable, e.g. createBooking(input: CreateBookingInput!) and then inside that you can have optional properties as needed. Then your input is always guaranteed to be non-nullable and the validator will always be fired.

type CreateUsersInput {
  users: [CreateUsersUserInput!]!
}

Aside: This is actually sort of related to an open issue #56 about supporting auto-validation of arrays of T, if there's a validator for T, which I'm thinking about rejecting as it fails into a similar problem. Unique, non-null, top-level input type per mutation name is the way to go.

[JarrydVanHoy]

Awesome, thank you @benmccallum . That lead me to find the attribute to perform this code side: Non-null Attribute .

As for the mutation design, I'm always open to hearing different points of view. So again, thank you for you speedy response and input. If the other peeps have input too, I'm all ears.