Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: OneOf Input Objects #825

Open
wants to merge 36 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
c385058
Renumber list items
benjie Feb 19, 2021
f6bd659
@oneOf input objects
benjie Feb 19, 2021
39e593c
@oneOf fields
benjie Feb 19, 2021
b6741c3
Fix typos (thanks @eapache!)
benjie Feb 26, 2021
d17d5ec
Much stricter validation for oneof literals (with examples)
benjie Mar 6, 2021
dca3826
Add missing coercion rule
benjie Mar 6, 2021
7e02f5a
Clearer wording of oneof coercion rule
benjie Mar 6, 2021
4111476
Add more examples for clarity
benjie Mar 6, 2021
6754e0a
Rename introspection fields to oneOf
benjie Mar 6, 2021
7c4c1a2
Oneof's now require exactly one field/argument, and non-nullable vari…
benjie Mar 6, 2021
bb225f7
Remove extraneous newline
benjie Mar 6, 2021
05fde06
graphgl -> graphql
benjie Apr 8, 2021
e8f6145
Apply suggestions from @eapache's review
benjie Apr 8, 2021
08abf49
Apply suggestions from code review
benjie Dec 23, 2021
59cb12d
Update spec/Section 3 -- Type System.md
benjie Jan 4, 2022
c470afb
Merge branch 'main' into oneof-v2
benjie Mar 22, 2022
99aa5d9
Remove Oneof Fields from spec
benjie Mar 22, 2022
691087d
Oneof -> OneOf
benjie Mar 22, 2022
7109dbc
Spellings
benjie Mar 22, 2022
05ab541
Remove out of date example
benjie May 6, 2022
6a6be52
Rename __Type.oneOf to __Type.isOneOf
benjie May 25, 2022
de87d2f
Add a:null example
benjie May 25, 2022
57e2388
Rewrite to avoid ambiguity of language
benjie May 25, 2022
5a966f2
Forbid 'extend input' from introducing the @oneOf directive
benjie May 26, 2022
e78d2b5
Merge branch 'main' into oneof-v2
benjie Nov 13, 2023
c6cd857
Merge branch 'main' into oneof-v2
benjie Mar 27, 2024
d106233
Add yet more examples to the example coercion table
benjie Mar 27, 2024
87d0b22
Indicate `@oneOf` is a built-in directive
benjie Jun 4, 2024
d88d62a
Update spec/Section 3 -- Type System.md
benjie Jun 5, 2024
a810aef
Merge branch 'main' into oneof-v2
benjie Jul 19, 2024
a1563a9
remove OneOf-specific rule in favor of update to VariablesInAllowedPo…
yaacovCR Sep 21, 2024
b45c0e4
Clarify IsNonNullPosition algorithm
benjie Oct 17, 2024
0c9830e
Clarify OneOf examples
benjie Oct 17, 2024
c4d0b50
Add more examples
benjie Oct 17, 2024
340594e
Remove new validation rule in favour of updates to existing rules
benjie Oct 17, 2024
dbccf84
Null literal is separate
benjie Oct 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 152 additions & 8 deletions spec/Section 3 -- Type System.md
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,9 @@ of rules must be adhered to by every Object type in a GraphQL schema.
characters {"__"} (two underscores).
2. The argument must accept a type where {IsInputType(argumentType)}
returns {true}.
3. If the field is a Oneof Field:
1. The argument must be nullable.
2. The argument must not have a default value.
3. An object type may declare that it implements one or more unique interfaces.
4. An object type must be a super-set of all interfaces it implements:
1. Let this object type be {objectType}.
Expand All @@ -832,19 +835,21 @@ IsValidImplementation(type, implementedType):
defined in {implementedType}.
1. Let {field} be that named field on {type}.
2. Let {implementedField} be that named field on {implementedType}.
1. {field} must include an argument of the same name for every argument
3. {field} must include an argument of the same name for every argument
defined in {implementedField}.
1. That named argument on {field} must accept the same type
(invariant) as that named argument on {implementedField}.
2. {field} may include additional arguments not defined in
4. {field} may include additional arguments not defined in
{implementedField}, but any additional argument must not be required,
e.g. must not be of a non-nullable type.
3. {field} must return a type which is equal to or a sub-type of
5. {field} must return a type which is equal to or a sub-type of
(covariant) the return type of {implementedField} field's return type:
1. Let {fieldType} be the return type of {field}.
2. Let {implementedFieldType} be the return type of {implementedField}.
3. {IsValidImplementationFieldType(fieldType, implementedFieldType)}
must be {true}.
6. {field} must be a Oneof Field if and only if {implementedField} is a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels kinda awkward, like we're skirting around the idea of having fundamentally different types of fields without really committing to it?

Oneof Field.

IsValidImplementationFieldType(fieldType, implementedFieldType):
1. If {fieldType} is a Non-Null type:
Expand Down Expand Up @@ -917,6 +922,30 @@ May yield the result:
The type of an object field argument must be an input type (any type except an
Object, Interface, or Union type).

**Oneof Fields**

Oneof Fields are a special variant of Object Type fields where the type system
asserts that exactly one of the field's arguments must be set and non-null, all
others being omitted. This is useful for representing situations where an input
may be one of many different options.
benjie marked this conversation as resolved.
Show resolved Hide resolved
benjie marked this conversation as resolved.
Show resolved Hide resolved

When using the type system definition language, the `@oneOf` directive is used
to indicate that a Field is a Oneof Field (and thus requires exactly one of its
arguments be provided):

```graphql
type Query {
findUser(
byID: ID
byUsername: String
byEmail: String
byRegistrationNumber: Int
): User @oneOf
}
```

In schema introspection, the `__Field.oneOf` field will return {true} for Oneof
Fields, and {false} for all other Fields.

### Field Deprecation

Expand Down Expand Up @@ -1160,6 +1189,9 @@ Interface types have the potential to be invalid if incorrectly defined.
characters {"__"} (two underscores).
2. The argument must accept a type where {IsInputType(argumentType)}
returns {true}.
3. If the field is a Oneof Field:
1. The argument must be nullable.
2. The argument must not have a default value.
3. An interface type may declare that it implements one or more unique
interfaces, but may not implement itself.
4. An interface type must be a super-set of all interfaces it implements:
Expand Down Expand Up @@ -1448,6 +1480,28 @@ define arguments or contain references to interfaces and unions, neither of
which is appropriate for use as an input argument. For this reason, input
objects have a separate type in the system.

**Oneof Input Objects**

Oneof Input Objects are a special variant of Input Objects where the type
system asserts that exactly one of the fields must be set and non-null, all
others being omitted. This is useful for representing situations where an input
may be one of many different options.

When using the type system definition language, the `@oneOf` directive is used
to indicate that an Input Object is a Oneof Input Object (and thus requires
exactly one of its field be provided):

```graphql
input UserUniqueCondition @oneOf {
id: ID
username: String
organizationAndEmail: OrganizationAndEmailInput
}
```

In schema introspection, the `__Type.oneOf` field will return {true} for
Oneof Input Objects, and {false} for all other Input Objects.
benjie marked this conversation as resolved.
Show resolved Hide resolved

**Circular References**

Input Objects are allowed to reference other Input Objects as field types. A
Expand Down Expand Up @@ -1541,6 +1595,21 @@ is constructed with the following rules:
definition does not provide a default value, the input object field
definition's default value should be used.

Further, if the input object is a Oneof Input Object, the following additional
rules apply:

* If the input object literal or unordered map does not contain exactly one
entry, an error must be thrown.

* If the single entry in the input object literal or unordered map is {null},
an error must be thrown.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are already enforced by validation, so I do not know if we need them in coercion. However, if we do not enforce them here then the coercion table below would have different outputs (e.g. {a: $a, b: $b} with { "a": "abc" } would be allowed by coercion, but not by validation).

An alternative approach is to keep the above and add a different non-null coerced value rule so we can drop the two rules below (since they'll be implicitly true).

@leebyron what are your thoughts on this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are already enforced by validation, so I do not know if we need them in coercion. However, if we do not enforce them here then the coercion table below would have different outputs (e.g. {a: $a, b: $b} with { "a": "abc" } would be allowed by coercion, but not by validation).

An alternative approach is to keep the above and add a different non-null coerced value rule so we can drop the two rules below (since they'll be implicitly true).

@leebyron what are your thoughts on this?

@benjie @erickessler1

It looks like the implementing PR for oneof tests allows {a: $a, b: $b} with { "a": "abc" } within coercion, only disallowing in validation. The spec changes as understood here by me seem to disallow this within coercion. I am not sure what the desired behavior was decided to be/should be, but just double checking that everything is aligned. :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bumping this


* If the coerced unordered map does not contain exactly one entry, an error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 why do we have basically the same rules twice, once for "unordered map" and once for "coerced unordered map"? Can this be simplified, or are we being explicit about something subtle?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the note above regarding validation. These new bullets are coercion (rather than validation) constraints; it's the ones above that we might want to remove.

must be thrown.

* If the value of the single entry in the coerced unordered map is {null}, an
error must be thrown.

Following are examples of input coercion for an input object type with a
`String` field `a` and a required (non-null) `Int!` field `b`:

Expand Down Expand Up @@ -1570,6 +1639,38 @@ Literal Value | Variables | Coerced Value
`{ b: $var }` | `{ var: null }` | Error: {b} must be non-null.
`{ b: 123, c: "xyz" }` | `{}` | Error: Unexpected field {c}


Following are examples of input coercion for a Oneof Input Object with a
`String` member field `a` and an `Int` member field `b`:

```graphql example
input ExampleInputTagged @oneOf {
a: String
b: Int
}
```

Literal Value | Variables | Coerced Value
------------------------ | ----------------------- | ---------------------------
`{ a: "abc", b: 123 }` | `{}` | Error: Exactly one key must be specified
`{ a: null, b: 123 }` | `{}` | Error: Exactly one key must be specified
`{ b: 123 }` | `{}` | `{ b: 123 }`
`{ a: $var, b: 123 }` | `{ var: null }` | Error: Exactly one key must be specified
benjie marked this conversation as resolved.
Show resolved Hide resolved
`{ a: $var, b: 123 }` | `{}` | Error: Exactly one key must be specified
`{ b: $var }` | `{ var: 123 }` | `{ b: 123 }`
`$var` | `{ var: { b: 123 } }` | `{ b: 123 }`
`"abc123"` | `{}` | Error: Incorrect value
`$var` | `{ var: "abc123" } }` | Error: Incorrect value
`{ a: "abc", b: "123" }` | `{}` | Error: Exactly one key must be specified
`{ b: "123" }` | `{}` | Error: Incorrect value for member field {b}
`{ a: "abc" }` | `{}` | `{ a: "abc" }`
`{ b: $var }` | `{}` | Error: Exactly one key must be specified
benjie marked this conversation as resolved.
Show resolved Hide resolved
`$var` | `{ var: { a: "abc" } }` | `{ a: "abc" }`
`{ a: "abc", b: null }` | `{}` | Error: Exactly one key must be specified
`{ b: $var }` | `{ var: null }` | Error: Value for member field {b} must be non-null
`{ b: 123, c: "xyz" }` | `{}` | Error: Exactly one key must be specified
Copy link
Member

@eapache eapache Mar 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing { a: $varA, b: $varB } with various combinations of values for varA and varB.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My in meeting proposal was that this case could just be invalid at start.

This L1441 in Validation file in this PR sounds like it would do just that:
https://github.com/graphql/graphql-spec/pull/825/files#diff-607ee7e6b71821eecadde7d92451b978e8a75e23d596150950799dc5f8afa43eR1441

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are exactly the same as for input objects (which also don't specify what happens if you have multiple variables); but I'll add some for clarity.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leebyron Good catch; that was not my intent. I have updated the PR with better validation and more examples.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've since revisited my thoughts on this and for the sake of defining types of variables on the client I've adopted the suggestion: #825 (comment)

benjie marked this conversation as resolved.
Show resolved Hide resolved
benjie marked this conversation as resolved.
Show resolved Hide resolved
`{ a: $a, b: $b }` | `{ a: "abc" }` | Error: Exactly one key must be specified

benjie marked this conversation as resolved.
Show resolved Hide resolved
**Type Validation**

1. An Input Object type must define one or more input fields.
michaelstaib marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -1580,6 +1681,9 @@ Literal Value | Variables | Coerced Value
characters {"__"} (two underscores).
3. The input field must accept a type where {IsInputType(inputFieldType)}
returns {true}.
4. If the Input Object is a Oneof Input Object then:
1. The type of the input field must be nullable.
2. The input field must not have a default value.
3. If an Input Object references itself either directly or through referenced
Input Objects, at least one of the fields in the chain of references must be
either a nullable or a List type.
Expand All @@ -1600,11 +1704,15 @@ be used by a GraphQL service which is itself an extension of another GraphQL ser
Input object type extensions have the potential to be invalid if incorrectly defined.

1. The named type must already be defined and must be a Input Object type.
3. All fields of an Input Object type extension must have unique names.
4. All fields of an Input Object type extension must not already be a field of
2. All fields of an Input Object type extension must have unique names.
3. All fields of an Input Object type extension must not already be a field of
the original Input Object.
5. Any non-repeatable directives provided must not already apply to the
4. Any non-repeatable directives provided must not already apply to the
original Input Object type.
5. If the original Input Object is a Oneof Input Object then:
1. All fields of the Input Object type extension must be nullable.
2. All fields of the Input Object type extension must not have default
values.


## List
Expand Down Expand Up @@ -1815,8 +1923,13 @@ by a validator, executor, or client tool such as a code generator.
GraphQL implementations should provide the `@skip` and `@include` directives.

GraphQL implementations that support the type system definition language must
provide the `@deprecated` directive if representing deprecated portions of
the schema.
provide:

- the `@deprecated` directive if representing deprecated portions of the
schema;
- the `@oneOf` directive if representing types that require exactly one field
benjie marked this conversation as resolved.
Show resolved Hide resolved
(i.e. Oneof Input Objects) or fields that require exactly one argument (i.e.
Oneof Fields).
benjie marked this conversation as resolved.
Show resolved Hide resolved

**Custom Directives**

Expand Down Expand Up @@ -1980,3 +2093,34 @@ type ExampleType {
oldField: String @deprecated(reason: "Use `newField`.")
}
```

### @oneOf

```graphql
directive @oneOf on INPUT_OBJECT | FIELD_DEFINITION
```

The `@oneOf` directive is used within the type system definition language
to indicate:

- an Input Object is a Oneof Input Object, or
- an Object Type's Field is a Oneof Field.

```graphql example
input UserUniqueCondition @oneOf {
id: ID
username: String
organizationAndEmail: OrganizationAndEmailInput
}
```

```graphql example
type Query {
benjie marked this conversation as resolved.
Show resolved Hide resolved
findUser(
byID: ID
byUsername: String
byEmail: String
byRegistrationNumber: Int
): User @oneOf
}
```
7 changes: 7 additions & 0 deletions spec/Section 4 -- Introspection.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@ type __Type {

# should be non-null for NON_NULL and LIST only, must be null for the others
ofType: __Type

# should be non-null for INPUT_OBJECT only
benjie marked this conversation as resolved.
Show resolved Hide resolved
oneOf: Boolean
}

type __Field {
Expand All @@ -156,6 +159,7 @@ type __Field {
type: __Type!
isDeprecated: Boolean!
deprecationReason: String
oneOf: Boolean!
}

type __InputValue {
Expand Down Expand Up @@ -336,6 +340,8 @@ Fields
* `name` must return a String.
* `description` may return a String or {null}.
* `inputFields`: a list of `InputValue`.
* `oneOf` must return {true} for Oneof Input Objects, {false} for all other
Input Objects.
* All other fields must return {null}.


Expand Down Expand Up @@ -380,6 +386,7 @@ Fields
* `isDeprecated` returns {true} if this field should no longer be used,
otherwise {false}.
* `deprecationReason` optionally provides a reason why this field is deprecated.
* `oneOf` must return {true} for Oneof Fields, {false} for all other Fields.


### The __InputValue Type
Expand Down
Loading