Using oneOf in headers results in documentation not rendering properly

[chill-cod3r]

🐛 Bug Report

When using the default schema generation for fastify and fastify-swagger, using the oneOf directive in the schema.headers: {} field causes the documentation not to be rendered correctly. While it does validate the headers correctly, it would be nice if it rendered the headers in the generated documentation as well.

To Reproduce

Using any fastify route, paste the following schema definition into the route spec:
Paste your code here:

const schema = {
  headers: {
    oneOf: [
      {  
        type: 'object', 
        required: ['x-header-1', 'x-header-2'], 
        properties: {
          'x-header-1': { type: 'string' }, 
          'x-header-2': { type: 'string' } 
        }
    },
      {  
        type: 'object', 
        required: ['x-header-1', 'x-header-3'], 
        properties: {
          'x-header-1': { type: 'string' }, 
          'x-header-3': { type: 'string' } 
        }
    },
    ]
  },
}

Expected behavior

Schema validates the incoming request correctly AND renders documentation in a meaningful way. Currently the rendered documentation is a bit lacking :(

While it absolutely validates the incoming headers properly (which is awesome!!), it unfortunately leaves something to be desired on the actual rendered swagger-ui.

Your Environment

• node version: 12
• fastify version: >=1.0.0
• os: Mac
• swagger spec 2.0

[mcollina]

Thanks for reporting. Would you like to send a PR to fix this?

[chill-cod3r]

I absolutely would! I would like a more wholistic viewpoint of what the expected behavior should be though. I have my opinion which would be to basically render all of the headers anyways, and let the oneOf validation just be in the background, however I feel like some people might disagree with that. Not having a lot of experience with how swagger-ui works, with my initial research into this situation it looks like render definitions for "oneOf" are not supported in swagger 2.0 - swagger-api/swagger-ui#4819.

Since this is a mismatch at the spec level, would it be an acceptable assumption to just render all the headers and have the oneOf directive be implicit in 2.0 spec? To me it seems absolutely yes, because showing all the headers and then mentioning the oneOf piece in the description of the route seems to be a much better alternative than just showing oneOf and having to point people to a model definition in the description.

Thoughts?

[mcollina]

How would you implement that?

[chill-cod3r]

I don’t know yet, I was just speaking on what I would expect it to accomplish. 😁. Do you have specific thoughts or knowledge about how it may not be possible or if it’s a bad idea?

…

On Fri, Feb 21, 2020 at 12:36 PM Matteo Collina ***@***.***> wrote: How would you implement that? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#229?email_source=notifications&email_token=AFI3Y3OTTF2F5KJ63GCULG3REAGLPA5CNFSM4KZFCHYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMTPPJI#issuecomment-589756325>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFI3Y3OSXJS6TWDWB2PQNQLREAGLPANCNFSM4KZFCHYA> .

[mcollina]

I have no idea if what you are proposing is feasible.

[chill-cod3r]

Okay, I’ll take a look and circle back around to this thread ASAP

…

On Sat, Feb 22, 2020 at 10:26 AM Matteo Collina ***@***.***> wrote: I have no idea if what you are proposing is feasible. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#229?email_source=notifications&email_token=AFI3Y3MXRUK32Y4MV7J54DLREE7Z3A5CNFSM4KZFCHYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMVCY5I#issuecomment-589966453>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFI3Y3JPJVXSEZXLG5RLG23REE7Z3ANCNFSM4KZFCHYA> .

[chill-cod3r]

After taking a brief look, I'm not completely sure there's a good way to work this out. With a quick code duplication based on the code above it, I was able to at least get all of the headers to render instead of it just saying "oneOf", however the api is still unusable via the UI because of the mismatch of required parameters. (ie one param is required in one of the oneOf options, and another is required in the other, and when you mix them both together, swagger-ui "thinks" that you don't have all the required fields)

function getHeaderParams (parameters, headers) {
  if (headers.type && headers.properties) {
    // for the shorthand querystring declaration
    const headerProperties = Object.keys(headers.properties).reduce((acc, h) => {
      const required = (headers.required && headers.required.indexOf(h) >= 0) || false
      const newProps = Object.assign({}, headers.properties[h], { required })
      return Object.assign({}, acc, { [h]: newProps })
    }, {})

    return getHeaderParams(parameters, headerProperties)
  } else if (headers.oneOf) {
    const merged = headers.oneOf.map(headers => {
      return Object.keys(headers.properties).reduce((acc, h) => {
        const required = (headers.required && headers.required.indexOf(h) >= 0) || false
        const newProps = Object.assign({}, headers.properties[h], { required })
        return Object.assign({}, acc, { [h]: newProps })
      }, {});
    });

    let allMerge = {};
    merged.forEach(m => {
      allMerge = Object.assign({}, allMerge, m);
    });

    return getHeaderParams(parameters, allMerge);
  }

I don't see a path to getting it to render AND having it be useful, and may be better left to as is, with the suggested path being to upgrade to swagger spec 3.0 (ie fastify-oas) where the oneOf appears to be supported that it will eventually be supported in the upstream of swagger-ui - swagger-api/swagger-ui#3803.

[climba03003]

Closing as #333 add support for oneOf, anyOf and allOf