Validation and serialization by Eomm · Pull Request #2023 · fastify/fastify

Eomm · 2020-01-04T15:53:41Z

This PR includes:

Changes

♻ the schemaCompiler has been renamed to validatorCompiler for clarity: this property is a function that must return a new function to validate the request parts
♻ the validatorCompiler function has a new signature: (method, url, httpPart, schema) to improve the tools that we can build around it (like store the generated functions without recompiling them on start)

Addition

🆕 a new serializerCompiler method has been added to open to customization of the default serializer (default: fast-json-stringify)
🆕 easier debug: the schema generation error say if it is a validation (ajv problem) or serialization (fast-json-stringify issue) problem

Breaking changes

🔴 removed the JSON shared schema replace-way) due it is not standard**
🔴removed the schemaResolver feature: it is not needed anymore without the shared schema "replace-way" since we are not resolving the $ref keyword anymore

** I have tested a lot of configurations to don't let the users suffer the upgrade, and if we wrote this:

Migration guide

removed the JSON shared schema [replace-way]

You wrote this:

schema: {
  body: 'schemaId#'
}

Now you can write this:

schema: {
  body: { $ref: 'schemaId#' }
}

removed the `schemaResolver` feature

You wrote this:

  const fastify = Fastify()
  const ajv = new AJV()
  ajv.addSchema(fastClone(schemaParent))
  ajv.addSchema(fastClone(schemaUsed))

  fastify.setSchemaCompiler(schema => ajv.compile(schema))
  fastify.setSchemaResolver((ref) => {
    return ajv.getSchema(ref).schema
  })

Now you can write this:

  const fastify = Fastify()
  const ajv = new AJV()
  ajv.addSchema(schemaA)
  ajv.addSchema(schemaBRefToA)

  fastify.setValidatorCompiler((method, url, httpPart, schema) => {
    return ajv.compile(schema)
  })

How it works (default)

When a route with a schema for validation (query, params, path, body) is found, an AJV instance is built. The instance will be the same for all the children plugins unless the user customizes the validatorCompiler per context or per route.
The same happens for the serialization.

Additional optimization:
When two different contexts need to build an AJV instance. If the ajv custom options and the shared schema (added with .addSchema) are the same, these two contexts will use the same AJV instance.

Checklist

run npm run test and npm run benchmark
tests and/or benchmarks are included
documentation is changed or added
commit message and code follows Code of conduct

mcollina

The reason this was implemented was because fast-json-stringify did not support external schemas. Are you sure it will do the job for us?

@jsumners This feature was also something you needed for Joi support. What do you think?

jsumners · 2020-01-05T21:20:32Z

I haven't looked through all of this to determine how it affects Joi support, but I can say that I no longer have need of that accursed library.

I think we definitely need to improve schema support in v3. We advertise having full support for JSON Schema, but as soon as you start trying to use references the whole thing falls apart in various ways. I fully support moving the schema container out of Fastify itself and merely accepting one being supplied at boot time.

Eomm · 2020-01-06T07:54:03Z

The reason this was implemented was because fast-json-stringify did not support external schemas.

From 1.15 it supports it at 3 levels of nesting

Are you sure it will do the job for us?

Yes, because those people are ourself 😆
I could invest more time on that module

This feature was also something you needed for Joi support.

I will test all the past use cases and will write a "how to migrate" guide to include it in the release message for v3

test/schema-feature.test.js

Eomm · 2020-01-22T21:45:10Z

Still 45 failing test before finish + docs

The backbone code is complete I think.
Any feedback or ideas for improvement of the code are appreciated

The logic right now is:

there is a validator item that generates the validation's functions
there is a serializator item that does the serialization's functions for the response
a fastify child instance use the validator and serializator of the parent if any

The default code flow is:

fastify start without any validator and serializator
the first route with a schema option will build the validator and serializator. This mean that an ajv instance and a fast-json-stringify instance will be created
the schemas are compiled by the items. The code in validation.js is now split between validation e serialization
the schemas.js file is now only a container of schemas and each fastify instance will have one of it for encapsulation. the cache is shared, instead and no more at route level

mcollina · 2020-01-23T08:11:44Z

This should significantly improve our cold start time on serverless, definitely +1.

Eomm · 2020-01-26T21:33:20Z

types/schema.d.ts

 * Compiler for FastifySchema Type
 */
-export type FastifySchemaCompiler = (schema: FastifySchema) => unknown
+export type FastifySchemaCompiler = (method: string, url: string, httpPart: string, schema: FastifySchema) => unknown


@Ethan-Arrowood I'm not sure, but the export interface FastifySchema could be wrong cause the user will receive as input a plain schema, not the complete schema configuration

fastify.post('/the/url', { body: bodyJsonSchema, querystring: queryStringJsonSchema, params: paramsJsonSchema, headers: headersJsonSchema }, handler)

The user will get 4 calls with bodyJsonSchema, queryStringJsonSchema, paramsJsonSchema, headersJsonSchema each one

Could you help me with the TS test? I need only some suggestions 😇

I'm confused -- i thought those properties fall under the schema property on the route options object. I noticed in your unit tests theres an reponse property now added to the schema so that should be added to the FastifySchema interface.

These schema properties are specified as type unknown because this allows the user to tell TypeScript what they are strictly. If we used something like { [k: string]: any } then we are making the type system non-strict by default and that does not lead to a great user experience. Check out the Schema section in my TS docs for more info.

i thought those properties fall under the schema property on the route options object

This is correct

in your unit tests theres an reponse property now added to the schema so that should be added to the FastifySchema interface.

This feature is already in place from master

My concern is that the FastifySchemaCompiler want a FastifySchema as input (schema parameter) but this parameter is a JSON Schema.
I think we need a different type here since FastifySchema works for route's configuration only.

I'll try to explain me by example:

// want a JSON schema input fastify.addSchema({ $id: 'test', type: 'object', properties: { id: { type: 'number' } } }) // want a `FastifySchema` type fastify.get('/:id', { handler: (req, reply) => { reply.send({ id: req.params.id * 2, ignore: 'it' }) }, schema: { params: { $ref: 'test#' }, response: { 200: { $ref: 'test#' } } } }) fastify.setValidatorCompiler((method, url, httpPart, schema) => { // looking the route above, the args are: - httpPart = "params" - schema = { $ref: 'test#' } })

this looks good to me now

mcollina

lgtm

mcollina · 2020-02-24T18:35:40Z

Can you please update the PR description to match the current status of this PR?
I'd also like to see a migration guide.

Eomm · 2020-02-24T20:39:01Z

Can you please update the PR description to match the current status of this PR?

Done

stale · 2020-03-11T02:32:19Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Eomm · 2020-03-11T13:25:37Z

Stalebot, I'm waiting for @Ethan-Arrowood 👍

Ethan-Arrowood

LGTM 😄 (sorry for missing this, im still trying to figure out github notifications lol)

Eomm · 2020-03-13T16:51:37Z

@fastify/core someone would like to check this PR?

Here all the changes:
#2023 (comment)

I would like to merge it in next before merging master since there are a couple of fixes in v2 that need to be adapted

jsumners · 2020-03-13T17:54:49Z

I really can't review it other than to say the code looks good. I'll need time to try it in an RC, and I don't even know when I'll have that time.

Eomm · 2020-03-13T18:11:25Z

No problem!
I hope someone else of the team could give it a check: for sure this is a request 🙏 , not an order 😀

delvedor

LGTM, great work!!

adrai · 2020-04-29T18:45:46Z

Has the binary format been removed in v3?

Eomm · 2020-04-29T19:14:08Z

Has the binary format been removed in v3?

I don't understand what you mean.
Could you add more info?

adrai · 2020-04-29T19:20:06Z

Sorry, I was not clear enough, (and did not even investigate)...
{ format: 'binary' } like here: fastify/fastify-swagger#47
I just tried the v3.0.0-rc.1 version and now it can't find the binary format anymore:
Failed building the validation schema for PUT: /file/organisations/:organisationId, due to error unknown format "binary" is used in schema at path "#"

adrai · 2020-04-29T19:23:19Z

Now I know why.... sorry my fault...
it's because of this change: schemaCompiler -> validatorCompiler
all green now

- remove error `FST_ERR_SCH_BUILD`. This error was removed in fastify#2023 , and doesn't exist in current master branch [error.js](https://github.com/fastify/fastify/blob/master/lib/errors.js). These lines were re-added to the file in #13f51c06ccc4f117fd8fa1892d8edefc554746b2.

github-actions · 2022-02-10T09:03:47Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

start design validation refactor

c7d397f

Eomm added semver-major Issue or PR that should land as semver major v3.x Issue or pr related to Fastify v3 labels Jan 4, 2020

Eomm changed the base branch from master to next January 4, 2020 15:54

mcollina reviewed Jan 5, 2020

View reviewed changes

delvedor added this to the v3.0.0 milestone Jan 7, 2020

Eomm added 5 commits January 19, 2020 11:37

Merge branch 'next' into valid-and-serial

cd8f230

working validation

fba82e6

add url to compile schema function

a301096

add working serialization

c97e9d0

writing tests

d3d7d1b

Eomm commented Jan 19, 2020

View reviewed changes

test/schema-feature.test.js Show resolved Hide resolved

Eomm added 5 commits January 19, 2020 19:11

fluent schema test

de09639

fix sibilings tests

7e0d22b

add validation complete test

b4cf7cd

refactor + tests

4a252fc

add getter

bf5d945

Eomm added 4 commits January 24, 2020 23:19

add ajv tests

2e5e658

complete test

ab284a5

improve coverage

e860437

add docs

ae6bf48

Eomm commented Jan 26, 2020

View reviewed changes

fix TODOs

7769ea1

Eomm marked this pull request as ready for review January 26, 2020 21:55

Eomm requested a review from a team January 26, 2020 21:58

Eomm mentioned this pull request Jan 26, 2020

Improving Start-Up Times #1598

Closed

mcollina approved these changes Feb 24, 2020

View reviewed changes

stale bot added the stale Issue or pr with more than 15 days of inactivity. label Mar 11, 2020

stale bot removed the stale Issue or pr with more than 15 days of inactivity. label Mar 11, 2020

Ethan-Arrowood approved these changes Mar 12, 2020

View reviewed changes

mcollina requested a review from delvedor March 13, 2020 18:25

delvedor approved these changes Mar 14, 2020

View reviewed changes

mcollina merged commit 6a6a562 into fastify:next Mar 14, 2020

thomheymann mentioned this pull request Mar 19, 2020

docs: Migration Guide v3 #2140

Merged

2 tasks

Eomm mentioned this pull request Apr 10, 2020

Allow replacing fast-json-stringify with another output serializer #1934

Closed

Eomm mentioned this pull request May 5, 2020

Resolver error with fastify v3 SkeLLLa/fastify-oas#30

Open

This was referenced Jun 7, 2020

Update: pr#2023 fastify/docs-chinese#181

Merged

文档持续更新List fastify/docs-chinese#18

Closed

fralonra mentioned this pull request Jun 14, 2020

Update Errors.md #2332

Merged

4 tasks

seriousme mentioned this pull request Jul 9, 2020

migrated to Fastify 3.0.0 seriousme/fastify-openapi-glue#92

Merged

lmeyerov mentioned this pull request Jul 22, 2020

Incomplete migration info for custom ajv $ref shared schema handling #2429

Closed

jsumners mentioned this pull request Jul 30, 2020

Schema absolute references failure #2446

Closed

github-actions bot locked as resolved and limited conversation to collaborators Feb 10, 2022

Uh oh!

Conversation

Eomm commented Jan 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Addition

Breaking changes

Migration guide

removed the JSON shared schema [replace-way]

removed the schemaResolver feature

How it works (default)

Checklist

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

jsumners commented Jan 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Eomm commented Jan 6, 2020

Uh oh!

Uh oh!

Eomm commented Jan 22, 2020

Uh oh!

mcollina commented Jan 23, 2020

Uh oh!

Eomm Jan 26, 2020

Choose a reason for hiding this comment

Uh oh!

Ethan-Arrowood Jan 27, 2020

Choose a reason for hiding this comment

Uh oh!

Eomm Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

Ethan-Arrowood Mar 12, 2020

Choose a reason for hiding this comment

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

mcollina commented Feb 24, 2020

Uh oh!

Eomm commented Feb 24, 2020

Uh oh!

stale bot commented Mar 11, 2020

Uh oh!

Eomm commented Mar 11, 2020

Uh oh!

Ethan-Arrowood left a comment

Choose a reason for hiding this comment

Uh oh!

Eomm commented Mar 13, 2020

Uh oh!

jsumners commented Mar 13, 2020

Uh oh!

Eomm commented Mar 13, 2020

Uh oh!

delvedor left a comment

Choose a reason for hiding this comment

Uh oh!

adrai commented Apr 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Eomm commented Apr 29, 2020

Uh oh!

adrai commented Apr 29, 2020

Uh oh!

adrai commented Apr 29, 2020

Uh oh!

github-actions bot commented Feb 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Eomm commented Jan 4, 2020 •

edited

Loading

removed the `schemaResolver` feature

jsumners commented Jan 5, 2020 •

edited

Loading

adrai commented Apr 29, 2020 •

edited

Loading