mercurius icon indicating copy to clipboard operation
mercurius copied to clipboard

User inputs validation and serialization

Open PacoDu opened this issue 6 years ago • 8 comments

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 ?

PacoDu avatar Jan 03 '19 10:01 PacoDu

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?

mcollina avatar Jan 03 '19 11:01 mcollina

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

PacoDu avatar Jan 03 '19 13:01 PacoDu

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?

mcollina avatar Jan 04 '19 13:01 mcollina

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

PacoDu avatar Jan 04 '19 20:01 PacoDu

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.

bhamon avatar Jan 13 '19 10:01 bhamon

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

mcollina avatar Jan 13 '19 19:01 mcollina

Long shot for this one:

  • I found a problem with JIT computation when the jit option equals to 1 (typically, your test case JIT doesn't trigger the JIT code). I corrected it with the #27 PR.
  • There is a bug in the fast-json-stringify module. The type alternative used in graphql-jit (['integer', 'null']) doesn't work. I created the fastify/fast-json-stringify#133 for this purpose.
  • I found another bug in the graphql-jit module: original errors thrown from resolvers are swallowed at runtime. I openned ruiaraujo/graphql-jit#2 for that 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.

bhamon avatar Jan 14 '19 08:01 bhamon

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

PacoDu avatar Feb 08 '19 14:02 PacoDu