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

Include shared schemas in response #172

Closed
PatrickHeneise opened this issue Apr 24, 2019 · 20 comments
Closed

Include shared schemas in response #172

PatrickHeneise opened this issue Apr 24, 2019 · 20 comments
Assignees

Comments

@PatrickHeneise
Copy link

I use a shared schema:

{
  "$id": "https://foo/common.json",
  "type": "object",
  "definitions": {
    "foo": {
      "$id": "#bar",
      "type": "object",
      "properties": {
        "id": { "type": "string" }
      }
    }
  }
}

And trying to use that schema in the response validation:

It works perfectly fine in a list/index route:

200: {
  type: 'array',
  items: {
    $ref: 'https://foo/common.json#bar'
  }
}

but not for get /:id

response: {
  200: 'https://foo/common.json#bar'
}

I'm trying to track down the issue but would appreciate a hint to where to start. Everything works fine if I add the schema "bar" individually before:

  fastify.addSchema({
    $id: 'bar',
    type: 'object',
    properties: {
      id: { type: 'number' }
    }
  })

...

works: 200: 'bar#'
@mcollina
Copy link
Member

What does it mean “it works”? @Eomm is the master of shared schemas, he might take a look here as well.

@PatrickHeneise
Copy link
Author

Using "bar#" (which is not a common/shared schema) works. Shared schema doesn't.

@Eomm
Copy link
Member

Eomm commented Apr 24, 2019

This should work (with last fast-json-stringify installed):

response: {
  200: {$ref: 'https://foo/common.json#bar'}
}

When you write someId# (hash at the end) you are using the "replace way" of the shared schema feature, but if you write the $ref as you did, you are using the $ref-way so you need to use the standard schema definition.

@PatrickHeneise
Copy link
Author

I thought I tested that case and it didn't work, but I'll try again in the morning.

@PatrickHeneise
Copy link
Author

response: {
  200: { $ref: 'https://foo/common.js#bar' }
}

UnhandledPromiseRejectionWarning: TypeError: Cannot read property '$ref' of undefined

@Eomm
Copy link
Member

Eomm commented Apr 25, 2019

Looking here:

const schema = transform

Maybe, the shared schemas are not provided to swagger.

I can't go deeper now, could you check it?

@PatrickHeneise
Copy link
Author

when using 'bar#', the schema is transformed to an object, with the ref it's not.

I've decided to move forward with multiple schema files instead of common.json.

@PatrickHeneise
Copy link
Author

Going back and forth experimenting with the schemas. I found some inconsistencies:

works:

body: { $ref: '/foo.json#/definitions/bar' }

fails:

response: {
  200: { $ref: '/foo.json#/definitions/bar' }
}

TypeError: Cannot read property '$ref' of undefined in fast-json-stringify/index.js:485:15

Then I tried

response: {
  200: '/foo.json#/definitions/bar'
}

Error: schema is invalid: data should be object,boolean in fast-json-stringify/index.js:30:17

@Eomm
Copy link
Member

Eomm commented Apr 26, 2019

Yeah, that doesn't surprise me:

  • body/querystring etc => use ajv as parser
  • response => use fast-json-stringy as serializer

For the trouble:

  • 200: { $ref: '/foo.json#/definitions/bar' } => I think this should work, could you share your npm ls?
  • 200: '/foo.json#/definitions/bar' => it is correct that it throw error since it is not valid

@PatrickHeneise
Copy link
Author

It's a long list ... I selected the path that might be the one you're looking for:

├─┬ [email protected]
│ ├── [email protected] deduped
│ ├── UNMET DEPENDENCY ajv-merge-patch@^4.1.0
│ ├─┬ [email protected]
│ │ ├── [email protected] deduped

started the project from scratch, so the dependencies should all be up-to-date.

If you point me into the direction, I'm happy to help fix this, as I need it in my current project.

@Eomm
Copy link
Member

Eomm commented May 7, 2019

Sorry @PatrickHeneise , I missed your comment. Did you solve meanwhile?

Otherwise, I will check this problem in next days

@Eomm Eomm self-assigned this May 7, 2019
@PatrickHeneise
Copy link
Author

No, I worked around the issue by placing the schema I need in /index.js and reference from there, but it's not pretty.

@vladlen-volkov
Copy link

vladlen-volkov commented Sep 20, 2019

I've almost the same problem. The swagger doesn't recognize some shared schemas.
Here is a repo with reproducing of error.

There are 4 routes:
GET /api/auth -> works fine, 'cause I wrap my shared schema in additional property "userSource"
GET /api/chats/ -> is ok with shared schema

GET /api/auth-broken -> fails in UI with Could not resolve reference: undefined undefined
PUT ​/api​/chats​/{chatId}​/messages -> fails in UI like a previous [despite of wrapping]

Are there any ways to avoid this? Or should we waiting for fixing?

@dannysofftie
Copy link

dannysofftie commented Oct 25, 2020

Came across the same issue today.

This works, when the response is of type array

response: {
 200: {
    description: 'Successful response',
    type: 'array',
      items: {
        $ref: '#Journal',
       },
   },
},

But this doesn't work when response is an object. I don't know if this is the intended behaviour.

response: {
 200: {
    description: 'Successful response',
    type: 'object',
    $ref: '#Journal',
   },
},

As a workaround, I used fastify instance to pull the already compiled schema and it works as expected:
Option 1.

response: {
 200: {
    description: 'Successful response',
    type: 'object',
   ...(fastify.getSchema('#Journal') as object), // silence typescript
   },
},

Option 2.

response: {
 200: fastify.getSchema('#Journal')
},

@pelepelin
Copy link

Does anybody here have a full example of referencing the models from dynamic routes the Swagger way?

    "responses": {
      "500": {
        "schema": {
          "$ref": "#/definitions/ErrorModel"
        }
      }
    }

I've tried to put "definitions" into their place in the plugin options, but that did not work. Should it be a separate feature request?

@pelepelin
Copy link

pelepelin commented Oct 29, 2020

So when I did

addSchema({
  $id: 'shared',
  definitions
});

then this response definition does not blow up fastify

500: {
      $ref: 'shared#/definitions/ErrorResponse'
    }

However, in this case the schema is nested and the path is transformed in output: "500":{"schema":{"$ref":"#/definitions/def-0/definitions/ErrorResponse"} which violates Swagger spec as definitions object must not be nested https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#definitionsObject

@melroy89

This comment was marked as off-topic.

@melroy89
Copy link
Contributor

melroy89 commented Mar 20, 2024

Using:

fastify.addSchema({
    $id: 'shared',
    type: 'object',
    definitions: {
      internalServerErrorResponse: {
        type: 'object',
        description: '500 Internal Server Error',
        properties: {
          message: {
            type: 'string',
            description: 'Error message',
            example: 'Some internal error message'
          }
        }
      }
    }
  })

And then use the shared definition:

    500: {
      $ref: 'shared#/definitions/internalServerErrorResponse'
    }

I will get:

image

@melroy89
Copy link
Contributor

melroy89 commented Mar 20, 2024

OpenAPI v3 component responses also doesn't work in Fastify Swagger, so I created a ticket for that: #793

@melroy89
Copy link
Contributor

Now I'm using addSchema without definitions, but only a top level ID:

fastify.addSchema({
  $id: 'internalServerErrorResponse',
  type: 'object',       
  description: '500 Internal Server Error',
  properties: {
    message: {
      type: 'string',
      description: 'Error message',
      example: 'Some internal error message'
    }
  }
})

Then use it in your schema like this:

schema: {
  description: 'example',
  response: {
    500: {
      $ref: 'internalServerErrorResponse#'
    }
  }
}

This ALMOST works.. Except the description message still says "Default Response" in Swagger UI:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants