seriousme / fastify-openapi-glue

A plugin for the Fastify webserver to autogenerate a Fastify configuration based on a OpenApi(v2/v3) specification.
MIT License
203 stars 33 forks source link

feat: allow responses with empty body schema #574

Closed ivan-tymoshenko closed 5 months ago

seriousme commented 5 months ago

Hi,

if the body is empty then:

"/emptyBodySchema": {
            "get": {
                "operationId": "emptyBodySchema",
                "summary": "Empty body schema",
                "responses": {
                    "204": {
                        "description": "Empty"
                    },
                    "302": {
                        "description": "Empty"
                    },
                    "400": {
                        "description": "Empty"
                    },
                }
            }
        }

will return a fastify config that does not contain schema for the 204 and the 304, which makes sense in my view as there is no schema present for those responses, and under normal circumstances this will generate an error like:

{
  "statusCode": 400,
  "error": "Bad Request",
  "message": "body should have required property 'name'"
}

Forcing an empty schema will change fastify's behaviour and block this error during serialization. Ideally I would like to come up with a solution that works without a specific flag for platformatic.

e.g.

{
  "302": {
    "$ref": "#/components/emptyBody",
    "description": "Empty"
  }
}

or :

{
  "204": {
    "x-fastify-empty-body": true,
    "description": "Empty"
  }
}

Kind regards, Hans

ivan-tymoshenko commented 5 months ago

Thanks for your feedback. I don't think the flag is needed for platformatic specifically. It's just another way to treat a route schema that has some status object, but doesn't have a content schema.

About your concern that it will rewrite the default 400 error handler. If I understand corectlly you are worried that setting an empty schema for status code 400 will eliminate all the fields form the default fastify validator error response (if it's not please explaine one more time). This is true. But I don't think this is a problem, because if you set a fastify route with an empty 400 schema it will also return you an empty object instead of the proper error. So it can be considered as an expected behaviour.

'use strict'

const fastify = require('fastify')

const app = fastify()

app.post('/example', {
  schema: {
    body: {
      type: 'object',
      properties: {
        foo: { type: 'string' }
      },
      required: ['foo']
    },
    response: {
      200: {
        type: 'object',
        properties: {
          foo: { type: 'string' }
        }
      },
      // 400: {} // uncomment this line to see the error
    }
  }
}, async (request, reply) => {
  return request.body
})

app.inject({
  method: 'POST',
  url: '/example',
  body: {
    wrong: 'fff'
  }
}, (err, res) => {
  console.log(res.body)
})
seriousme commented 5 months ago

Hi,

I understand that your change will achieve the same as setting 400: {} as that is what the plugin will generate with your fix.

Anyways: the way I understand it you need a placeholder so you can roundtrip back to a specification based on the items found in the fastify config. That is ok by me.

The only thing I'd like to change is the name: allowEmptyBody sounds like its not allowed to have an empty body without the flag. So I propose to change the name to: addEmptySchema so its clear that we purposely add an emptySchema even if its not a hard requirement in Fastify

Kind regards, Hans

seriousme commented 5 months ago

Thx !

seriousme commented 5 months ago

Released as 4.6.0 on NPM