Validation rules for Client Controlled Nullability operators

[aprilrd]

Here is a prroposed list of validations with nullability operators. I am sure there are cases that I missed here so please comment below if you see any cases missed. The current changeset on graphql-js was made with this list in mind, but we need to audit and make sure they behave per this list.

Schema + Operation validation

All the tests under this section assumes this following schema

interface SomeBox {
  deepBox: SomeBox
  unrelatedField: String
}

type StringBox implements SomeBox {
  scalar: String
  deepBox: StringBox
  unrelatedField: String
  listStringBox: [StringBox]
  stringBox: StringBox
  intBox: IntBox
}

type IntBox implements SomeBox {
  scalar: Int
  deepBox: IntBox
  unrelatedField: String
  listStringBox: [StringBox]
  stringBox: StringBox
  intBox: IntBox
}

interface NonNullStringBox1 {
  scalar: String!
}

type NonNullStringBox1Impl implements SomeBox & NonNullStringBox1 {
  scalar: String!
  unrelatedField: String
  deepBox: SomeBox
}

interface NonNullStringBox2 {
  scalar: String!
}

type NonNullStringBox2Impl implements SomeBox & NonNullStringBox2 {
  scalar: String!
  unrelatedField: String
  deepBox: SomeBox
}

type Connection {
  edges: [Edge]
}

type Edge {
  node: Node
}

type Node {
  id: ID
  name: String
}

type Query {
  someBox: SomeBox
  connection: Connection
}

When types are not mutually exclusive (same parent / interface)

When a field is nullable

With aliases

• [ ] It's unclear to me what the behavior should be. Probably invalid?

Current Status

• There is a test

Current test case

"""
currently asserting validity
"""
{
  someBox {
    ...on SomeBox {
      nullable: unrelatedField
    }
    ...on IntBox {
      nonNullable: unrelatedField!
    }
  }
}

Without aliases

	!	?	not-marked
!	valid (case 1)	(duplicate)	(duplicate)
?	invalid (case 2)	valid (case 3)	(duplicate)
not-marked	invalid (case 4)	valid (case 5)	valid (case 6)

Current Status

• Case 1 is implemented
• Case 4 is implemented

Sample test cases

"""
case 1: valid
"""
{
  someBox {
    ...on SomeBox {
      unrelatedField!
    }
    ...on IntBox {
      unrelatedField!
    }
  }
}

"""
case 4: invalid
"""
{
  someBox {
    ...on SomeBox {
      unrelatedField
    }
    ...on IntBox {
      unrelatedField!
    }
  }
}

When a field is non-nullable

With aliases

• [ ] It's unclear to me what the behavior should be. Probably invalid?

Current Status

• There is a test

Current Test Case

"""
currently asserting validity
"""
{
  someBox {
    ...on NonNullStringBox1 {
      nonNullable: scalar!
    }
    ...on NonNullStringBox1Impl {
      nullable: scalar
    }
  }
}

Both with or without aliases

	!	?	not-marked
!	valid (case 1)	(duplicate)	(duplicate)
?	invalid (case 2)	valid (case 3)	(duplicate)
not-marked	valid (case 4)	valid (case 5)	valid (case 6)

Current Status

• Case 1 is implemented
• Case 4 is implemented

Sample test cases

"""
case 1: valid
"""
{
  someBox {
    ...on NonNullStringBox1 {
      scalar!
    }
    ...on NonNullStringBox1Impl {
      scalar!
    }
  }
}

"""
case 4: valid
"""
{
  someBox {
    ...on NonNullStringBox1 {
      scalar
    }
    ...on NonNullStringBox1Impl {
      scalar!
    }
  }
}

When types are exclusive

Here, with aliases, all combinations are valid.

• [ ] Do a double pass to ensure that marking a nullable field non-nullable stays backward compatible.

Fields are both nullable

	!	?	not-marked
!	valid (case 1)	(duplicate)	(duplicate)
?	invalid (case 2)	valid (case 3)	(duplicate)
not-marked	invalid (case 4)	valid (case 5)	valid (case 6)

Current Status

• Case 4 is implemented

Sample test cases

"""
case 1: valid
"""
{
  someBox {
    ... on IntBox {
      scalar!
    }
    ... on StringBox {
      scalar!
    }
  }
}

"""
case 4: invalid
"""
{
  someBox {
    ... on IntBox {
      scalar
    }
    ... on StringBox {
      scalar!
    }
  }
}

Fields are both non-nullable

	!	?	not-marked
!	valid	(duplicate)	(duplicate)
?	invalid	valid	(duplicate)
not-marked	valid	invalid	valid

Current Status

• Not implemented

Sample test cases The current schema does not have non-nullable fields across exclusive types. We need to add that first.

Field A is nullable and Field B is non-nullable

	! on B	? on B	Not-marked on B
! on A	valid (case 1)	invalid (case 2)	valid (case 3)
? on A	invalid (case 4)	valid (case 5)	invalid (case 6)
Not-marked A	invalid (case 7)	valid (case 8)	invalid (case 9)

Current Status

• Case 3 is implemented

Sample test cases

"""
case 1: valid
"""
{
  someBox {
    ... on StringBox {
      scalar!
    }
    ... on NonNullStringBox1 {
      scalar!
    }
  }
}

"""
case 3: valid
"""
{
  someBox {
    ... on StringBox {
      scalar!
    }
    ... on NonNullStringBox1 {
      scalar
    }
  }
}

"""
case 8: valid
"""
{
  someBox {
    ... on StringBox {
      scalar
    }
    ... on NonNullStringBox1 {
      scalar?
    }
  }
}

Operation only validation

Given fragment A and fragment B (one of them could just be a selection set)

	! on B	? on B	Not-marked on B
! on A	valid (case 1)	(duplicate)	(duplicate)
? on A	invalid (case 2)	valid (case 3)	(duplicate)
Not-marked A	valid (case 4)	valid (case 5)	valid (case 6)

Current Status

• [ ] We have a test for Case 4, but it asserts invalid. I believe it should be valid. Open for feedback

Sample test cases

"""
case 1: valid
"""
fragment conflictingNullability on Pet {
  ... on Dog {
    name!
  }
  ... on Cat {
    name!
  }
}

"""
case 2: invalid
"""
fragment conflictingNullability on Pet {
  ... on Dog {
    name?
  }
  ... on Cat {
    name!
  }
}

[magicmark]

🎉 thanks for this! this is super helpful in working out the corner cases :)

I'm having a little trouble understand the tables in context - maybe we could add example schema + queries / fragments and label the axes?

(I took a stab at it but then deleted my comments cos i realized i was being dumb :P)

[aprilrd]

Definitely, I will add more labels to the tables. I also liked your suggestion to add some examples to ground the cases in the tables so I will do that as well.

[aprilrd]

Updated the issue with examples and added the current status of tests we have. I marked open questions with the checkboxes.