Skip to content

Add `GlobalIDType` scalar

Alex Kalderimis requested to merge 215848-id-type-graphql-pd into master

What does this MR do?

As suggested in !29897 (comment 330791982) this adds a new scalar type that represents GlobalID instances. This class only permits valid IDs for the current app, and we provide a mechanism to annotate the type with the expected type of object that each ID represents, so that we can verify the input.

This means that instead of:

argument :id, GraphQL::ID_TYPE, required: true, description: 'The ID of the thing'

You can have:

argument, :id, ::Types::GlobalIDType[::Thing], required: true, description: 'The ID of the thing'

And you get the following for free:

  • the value is parsed into a GlobalID instance, not given to you as a string
  • invalid values raise argument errors, before any application code (resolvers, ready methods) are called
  • if a type is provided, that is checked against the input, so that git://gitlab/Project/1 is not a valid input for ThingID. This means repetitive validation code can be removed.

The following potentially adverse impacts will be seen over the codebase:

  • Clients will need to update queries/mutations to change ID types to GlobalID if untyped, or the correct nominal type if type-restricted (such as ProjectID for example). For some types, these names can get long and ugly.
  • Developers will need to consider what the least-upper-bound for their types is. If this is incorrect there may be problems:
    • too high (too general): bugs could be introduced by admitting illegal IDs
    • too low (too specific): valid inputs could be excluded.

Although raising a bound is strictly speaking backwards compatible, the use of strict nominal typing in the GraphQL language and the lack of input interfaces mean that *any input type change requires all clients to update queries and mutations. We should want to avoid this.

Example queries

When used, this will require inputs to name the specific kind of ID they are using:

query ($userId: UserID) {
  user(id: $userId) { name }
}

This type is backwards compatible when used as output, since it serializes to a string

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

See: #215848

Edited by Michael Kozono

Merge request reports