feat : add openapi 3 support

[climba03003]

Feedback are welcome while construction.

This PR used to add minimal support for OpenAPI 3. I do not assume complicated situation.

Issue related to OpenAPI 3
Resolve: #87, Resolve: #236, Resolve #265, Resolve #330
PR related to OpenAPI 3
Closes: #237

TODO List

• refactor: reduce the number of files in root folder
• refactor: extract common function in dynamic.js
• feat: add openapi 3 support
• feat: add translation layer between swagger 2 and openapi 3 (maybe in next PR, too much work required)

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 the Developer's Certification of Origin
  and the Code of conduct

[mcollina]

huge +1 for the refactor.

[darkgl0w]

Really nice to see a refactor and an upgrade in features for this package :D
I will follow your work on this PR and will be happy to review it ^^

[climba03003]

Basic Functionality for OpenAPI 3 is added. In order to make thing simple and not dealing with the transition between swagger 2.0 and openapi 3.0. I decide to add a new option openapi. It may not be the ideal solution but it is the fastest way to get thing ready with semver-minor.

It should fix most of the issue with oneOf and anyOf. I will just add them all in this PR.

Maybe in the future, someone is willing to add a transition layer between swagger 2.0 and openapi 3.0.

[climba03003]

TypeScript type will not updated for now. As #328 dealing with it.

[mcollina]

@Eomm @SkeLLLa could you take a look?

[mcollina]

semver-major given the extent of the change?

[jsumners]

semver-major given the extent of the change?

I think that's a good idea.

[mcollina]

This is missing some documentation update.

[climba03003]

I will complete it today and also the typing update.

[mcollina]

I think we should throw if both swagger and openapi options are both set at the same time.

[climba03003]

I think we should throw if both swagger and openapi options are both set at the same time.

I think someone will left the swagger option there as backup and see the stability of the latest rollout of openapi support. Maybe I should add a warning instead of an error to notify the user swagger option will be ignored?

[mcollina]

lgtm

[Eomm]

Blocking because I need a couple of days to check the PR and I would like to review it 👍

[Eomm]

I think this feature is ok as the first step to OAS3 support

There is some issues with this usage:

Click to expand!

'use strict'

const Fastify = require('fastify')
const fastifySwagger = require('../../index')

const fastify = Fastify({ logger: true })
fastify.register(fastifySwagger, {
openapi: {
info: {
title: 'Test swagger',
description: 'testing the fastify swagger api',
version: '0.1.0'
},
servers: [{
url: 'http://localhost'
}],
components: {
securitySchemes: {
apiKey: {
type: 'apiKey',
name: 'apiKey',
in: 'header'
}
}
}
},
exposeRoute: true
})

fastify.addSchema({
$id: 'Order',
type: 'object',
properties: {
id: {
type: 'integer'
// format: 'int64'
}
}
})

fastify.post('/', {
schema: {
body: {
$ref: 'Order#'
},
response: {
200: {
$ref: 'Order#'
}
}
}
}, () => {})

fastify.listen(8080, err => {
console.log(err)
})


That lead to:

Because one big difference between oas2 and oas3 is the components over the old definitions keyword. (I use this schema as reference)

So I would land this PR as a major preview release to collect other edges cases before a major bump.

What do you think? @fastify/plugins

[climba03003]

I think this feature is ok as the first step to OAS3 support

There is some issues with this usage:
Click to expand!
'use strict' const Fastify = require('fastify') const fastifySwagger = require('../../index') const fastify = Fastify({ logger: true }) fastify.register(fastifySwagger, { openapi: { info: { title: 'Test swagger', description: 'testing the fastify swagger api', version: '0.1.0' }, servers: [{ url: 'http://localhost' }], components: { securitySchemes: { apiKey: { type: 'apiKey', name: 'apiKey', in: 'header' } } } }, exposeRoute: true }) fastify.addSchema({ $id: 'Order', type: 'object', properties: { id: { type: 'integer' // format: 'int64' } } }) fastify.post('/', { schema: { body: { $ref: 'Order#' }, response: { 200: { $ref: 'Order#' } } } }, () => {})

fastify.listen(8080, err => { console.log(err) })

That lead to:

Because one big difference between oas2 and oas3 is the components over the old definitions keyword. (I use this schema as reference)

So I would land this PR as a major preview release to collect other edges cases before a major bump.

What do you think? @fastify/plugins

I think it is the bug from addHook, it should be exist even for swagger. The main reason is that we never collect the schema from addSchema on root instance.

Test Case for your reproduction

test('collect addSchema on root instance', t => {
  t.plan(2)
  const fastify = Fastify()

  fastify.register(fastifySwagger, swaggerInfo)
  fastify.addSchema({ $id: 'Order', type: 'object', properties: { id: { type: 'integer' } } })
  fastify.post('/', { schema: { body: { $ref: 'Order#' }, response: { 200: { $ref: 'Order#' } } } }, () => {})

  fastify.ready(err => {
    t.error(err)

    const swaggerObject = fastify.swagger()
    t.ok(swaggerObject.definitions['def-0'])
  })
})

Reason behind

fastify.addHook('onRoute', (routeOptions) => {
  routes.push(routeOptions)
})

fastify.addHook('onRegister', async (instance) => {
  instance.addHook('onReady', (done) => {
    const allSchemas = instance.getSchemas()  // we only collected the encapsulate schema
    for (const schemaId of Object.keys(allSchemas)) {
      if (!sharedSchemasMap.has(schemaId)) {
        sharedSchemasMap.set(schemaId, allSchemas[schemaId])
      }
    }
    done()
  })
})

Another problem like you say, the definitions should change into components/schemas

[Eomm]

it should be exist even for swagger.

Nope, it is managed by the (ugly sigh) def-0 of the json-schema-resolver and it could be fixed there:
https://github.com/Eomm/json-schema-resolver/blob/39191491f3ef2c4b5fac5e19ad067459ccccdded/ref-resolver.js#L24
but your hotfix works 👍

[climba03003]

it should be exist even for swagger.

Nope, it is managed by the (ugly sigh) def-0 of the json-schema-resolver and it could be fixed there:
https://github.com/Eomm/json-schema-resolver/blob/39191491f3ef2c4b5fac5e19ad067459ccccdded/ref-resolver.js#L24
but your hotfix works 👍

I do not think it can be solved like you say as the #/definitions is hard coded in here:
https://github.com/Eomm/json-schema-resolver/blob/39191491f3ef2c4b5fac5e19ad067459ccccdded/ref-resolver.js#L113

What I said it exist for swagger is that, your example addSchema is on the root instance which we do not collect the schema currently and json-schema-resolver cannot resolve schema from an empty object. You can check my newly added test in here:

fastify-swagger/test/openapi.js

Lines 785 to 789 in 35a9be8

	fastify.register(function (instance, _, done) {
	instance.addSchema({ $id: 'Order', type: 'object', properties: { id: { type: 'integer' } } })
	instance.post('/', { schema: { body: { $ref: 'Order#' }, response: { 200: { $ref: 'Order#' } } } }, () => {})
	done()
	})

I need to wrap the addSchema in register in order to get the test work.
It should be a bug needed to file, so I will not fix it now.

[SkeLLLa]

For schemas there's slightly different approach that might be more elegant solution rather than resolve it. I've described it here: SkeLLLa/fastify-oas#30 (comment) but it require lot's of work, so it should be different PR.

[danez]

Because of this openapi-types needs to be a dependency and not dev-dependency.
See #340

[climba03003]

Actually, it is landed from here #328

[danez]

But this pr was landed after #328 (if i see that correctly here). So on master index.d.ts has a dependency on openapi-types, but it does not list it as dependency.

[climba03003]

This PR is merging the #328 by fast-forwarding commit. The actual PR that add the openapi-types for devDeps is #328
4.0.0 indeed is merging two PR. #328 and #333, you can check the commit history in here to see the detail.

[climba03003]

Let say in this way, even without this PR, #328 will cause the same problem. If the problem is adding openapi-types as devDeps. It is added in #328. Not arguing anything, I just want to figure out which is the root cause.