Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON Schema Definitions not work #639

Open
2 tasks done
conioX opened this issue Jul 23, 2022 · 7 comments · May be fixed by #676
Open
2 tasks done

JSON Schema Definitions not work #639

conioX opened this issue Jul 23, 2022 · 7 comments · May be fixed by #676
Labels
bug Confirmed bug good first issue Good for newcomers

Comments

@conioX
Copy link

conioX commented Jul 23, 2022

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the issue has not already been raised

"@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
Copy link
Member

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

@mcollina mcollina added bug Confirmed bug good first issue Good for newcomers labels Jul 24, 2022
@Fdawgs
Copy link
Member

Fdawgs commented Aug 3, 2022

Looks to be related to #524

@mikaelkaron
Copy link

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
Copy link
Member

@mikaelkaron would you like to work on it?

@asc11cat
Copy link

asc11cat commented Oct 7, 2022

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 #524 (comment) 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
Copy link

@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
Copy link

asc11cat commented Oct 7, 2022

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants