fastify / fastify-swagger

Swagger documentation generator for Fastify
MIT License
949 stars 209 forks source link

JSON Schema Definitions not work #639

Open conioX opened 2 years ago

conioX commented 2 years ago

Prerequisites

"@fastify/swagger": "7.4.1",

Issue

Hi Guys, I add new schema with addSchema in my fastify like this :

{
  $id: 'http://foo/common.json',
  type: 'object',
  definitions: {
    foo: {
      $id: '#address',
      type: 'object',
      properties: {
        city: { type: 'string' }
      }
    }
  }
}

and used in my route schema

 schema: {
      body: {
        $ref: 'http://foo/common.json#/definitions/foo'
      }
    },

its work in fastify no error found, but error in swagger

Could not resolve reference: Could not resolve pointer: /definitions/def-0/definitions/foo ddoes not exist in document

and when i check /documentation/json there is no my definitions inside -> definitions -> def-0

and models only {}

mcollina commented 2 years ago

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

Fdawgs commented 2 years ago

Looks to be related to https://github.com/fastify/fastify-swagger/issues/524

mikaelkaron commented 2 years ago

Any chance this could get a bump?

I ran into this (again) when trying the official examples from the docs for fuent-json-schema in fastify.

mcollina commented 2 years ago

@mikaelkaron would you like to work on it?

asc11cat commented 2 years ago

The 'definitions' keyword in the component(3.0)/definition(2.0) item schema itself is not allowed at all, it is allowed only in top-lvl as 'definition' if we are using the OpenAPI 2.0 version. The changes introduced in #472 (https://github.com/fastify/fastify-swagger/pull/472/files#diff-3faaf157cccc3cc534ec616affca5b4f5df9817c937b7ada4f110afd2770aae2R35) is deleting the 'definitions' key from the schema item object, therefore preventing any possible errors on the schema validation, but also preventing the correct use of json-schema with this keyword.

In the case of json-schema coming from fastify, it seems like we have two ways to solve this problem, first one is to mutate 'definitions' keyword to 'properties' as stated in https://github.com/fastify/fastify-swagger/issues/524#issuecomment-1203660976 workaround, but i have a suspicion that this can cause troubles when resolving some deep-nested schema, currently I'm in the process of checking it.

The second one is to move all nested definitions from 'definitions' obj to top-lvl of components/definitions, but that doesn't seem to follow the intended behaviour from original json-schema(?)

@mcollina can you please give any suggestions on optimal implementation in that case, so I can work on the fix?

mikaelkaron commented 2 years ago

@mikaelkaron would you like to work on it?

I’d love to help out, but not really in a place where I think I can dig into the code alone. However I see that @asc11cat is pointing out where to start so if there’s something I can do to help I’m in.

asc11cat commented 2 years ago

@mikaelkaron would you like to work on it?

I’d love to help out, but not really in a place where I think I can dig into the code alone. However I see that @asc11cat is pointing out where to start so if there’s something I can do to help I’m in.

That's great! Lets head to #675