mercurius-js / mercurius

Implement GraphQL servers and gateways with Fastify
https://mercurius.dev/
MIT License
2.33k stars 234 forks source link

User inputs validation and serialization #23

Open PacoDu opened 5 years ago

PacoDu commented 5 years ago

I'm looking for a way to take advantage of Fastify schema validation and serialization. GraphQL allows type validation but this is not enough for user inputs validation.

For now I'm using my own architecture with a model validation through AJV and json-schemas, but I'm wondering if I can benefit from fastify validation for inputs and serialization optimization for outputs (fast-json-stringify). Correct me if I'm wrong, but I don't think that fastify-gql response schema allows to break JSON recursive call for serialization optimization due to additionalProperties: true https://github.com/mcollina/fastify-gql/blob/d5d79d4a62a03a9ac5d742fe90354b1c6070aef6/routes.js#L7-L18

Merging graphql schemas and fastify json-schemas handling would be awesome. Maybe providing a json-schema that mirrors graphql types, queries and mutation and handle this inside the lib ?

Did you think about this subject yet ? Is this viable or useless for any reason that I'm missing ? Or fastify can't bring much more than my actual setup with AJV ?

mcollina commented 5 years ago

I think it would be fantastic to have a GraphQL to JSON Schema converter, and then use that inside this library with fast-json-stringify. I don't know if one exists already on NPM, but it would be a fun project to build and integrate. Would you like to work on this and send a PR?

Regarding validation, I'm not sure if something could be done. How are you integrating with ajv? could you make an example?

PacoDu commented 5 years ago

Well it's true that I've mixed two subjects here (serialization / validation) ^^

For the validation: my usage of AJV is pretty straightforward, this is a simple example (independent of fastify):

'use strict'

const ajv = require('ajv')

const schemaStuff = require('../../schemas/stuff')
const modelValidationError = require('../common/validationError')

const VALIDATOR_CREATE = new ajv()
  .addSchema(schemaStuff)
  .compile({
    type: 'object',
    additionalProperties: false,
    properties: {
      name: {$ref: 'stuff#/definitions/name'}
    }
  })

async function create(_context, _fields) {
  if (!VALIDATOR_CREATE(_fields)) {
    throw new modelValidationError('Invalid fields', {errors: VALIDATOR_CREATE.errors})
  }

  // call odm and return id
}

module.exports = create

Here a stuff service with the create method and others CRUD methods is then registered to fastify and used in the graphql resolvers

For the serialization after a quick lookup I've found some libraries on NPM for graphql to json-schema convertion.

https://github.com/jakubfiala/graphql-json-schema Which seems to be the first library that implement this conversion, but based on this PR https://github.com/jakubfiala/graphql-json-schema/pull/4 it doesn't generate valid json-schemas.

However the author of this PR has developed an other library that aims at generating valid json-schemas from an introspection query : https://github.com/wittydeveloper/graphql-to-json-schema

mcollina commented 5 years ago

I think https://github.com/wittydeveloper/graphql-to-json-schema could be a good starting point or as an inspiration.

I don't think the following is part of the JSON Schema spec:

                    return: {
                            $ref: '#/definitions/Todo'
                        }

But that is clearly what we would need to assemble a valid JSON schema for all the possible outputs of a graphql endpoint. Basically we should take that value for every query and mutation, and define a JSON schema that validates the results for:

{
  "data": {
    "todo": ... // this is a TODO object
  }
} 

Some care would need to be taken for nested resolvers.

Would you like to send a PR?

PacoDu commented 5 years ago

Yes of course, I will take a look and try to implement that, I'll send a PR when I have something interesting !

bhamon commented 5 years ago

The graphql-jit module can compute the graphQL -> JSON mapping to build a fast per-query response serializer if the compilerOptions.customJSONSerializer option is set to true. @mcollina Can you add an option property to your plugin to control those compiler options? Also, when using this option, the fastify response schema should be disabled to avoid double serialization.

Regarding input validation, I think we need the opposite of what graphql-to-json-schema do. The goal is to decorate all input types with custom JSON schemas to enforce extra conditions (like email, passwords, and so on...).

The ideal solution would be to write graphQL schemas as JSON schemas. The graphQL schemas would be infered from the JSON schemas and resolvers could be pre-decorated with input type validation. For simple schemas it shouldn't be too hard. But for interfaces, type resolvers, unions and custom scalars, it'll be a nightmare to implement.

The other solution may be to provide a way to construct schemas with decorating features to handle validation. It could take place in your defineResolvers function. A resolver could be a plain function (actual behavior) or an object with a validation schema and the resolver function itself. The drawback is the double definition of the input vars (one in the schema definition and the other in the JSON schema itself).

In the meantime I'm doing validation the same way as @PacoDu, even if it's not really satisfying.

mcollina commented 5 years ago

Would you lkke to send a PR? I’ll be under an intense travel schedule for the next month or so.

bhamon commented 5 years ago

Long shot for this one:

I checked the whole code of graphql-jit and their fork seems a bit outdated against the recent graphql-js updates. It's kind of scary for the future.

PacoDu commented 5 years ago

Sorry but I couldn't find some time to investigate this issue for now, maybe in some months.