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

Resolver error with fastify v3 #30

Open
adrai opened this issue May 1, 2020 · 18 comments
Open

Resolver error with fastify v3 #30

adrai opened this issue May 1, 2020 · 18 comments

Comments

@adrai
Copy link
Contributor

adrai commented May 1, 2020

made a quick test....
changed fastify: '>=2.0.0 <2.12.0 || >=2.12.1 <4.0.0'

when opening swagger ui, and clicking on any route having a $ref in the schema:

image

example:
image

fastify changes: https://github.com/fastify/fastify/blob/master/docs/Migration-Guide-V3.md#changed-schema-substitution-2023

@adrai
Copy link
Contributor Author

adrai commented May 1, 2020

other downstream libs probably also need some changes: fastify/fastify-static#127

@adrai
Copy link
Contributor Author

adrai commented May 5, 2020

The /documentaiton/json in fastify v2 looked like this:
image

and with fastify v3 (+ $ref) it looks like this:
image

maybe @Eomm can help?

Seems like 'uuid#' in v2 was resolved before the onRoute hook
and now with v3 and { $ref: 'uuid#' } it's not resolved anymore

@SkeLLLa
Copy link
Owner

SkeLLLa commented May 5, 2020

@adrai It didn't resolved in on route hook, it resolved after, when app.ready() resolved. So the route object properties was replaced by resolved schema.

Here are some links where you can find more details:
fastify/fastify#2123 (this is an open issue)
fastify/fastify#2104 (this one has explanation how it works in details)

Maybe we just need a method in fastify that will allow to get schema object by id. Actually there's already https://github.com/SkeLLLa/fastify-oas/blob/master/lib/openapi/index.js#L5 that allows to get all schemes, but in fastify v2 it works only if schema was added via fastify.addSchema.

PS: also fixing this could be copy-pasted to fastify-swagger module, cause it uses absolutely the same approach.

@adrai
Copy link
Contributor Author

adrai commented May 5, 2020

It didn't resolved in on route hook, it resolved after, when app.ready() resolved

ok, nevertheless... this is not getting resolved anymore...

also fixing this could be copy-pasted to fastify-swagger module, cause it uses absolutely the same approach.

hehe so if fastify-oas or fastify-swagger get's fixed for v3 the other get's fixed too ;-)
so for fastify-swagger are you aware of this { $ref } "problem"? @SkeLLLa @mcollina
(have not verified this myself on fastify-swagger)

@Eomm
Copy link

Eomm commented May 5, 2020

I don't know exactly the problem here for oas.

In fastify-swagger all the schemas added with addSchema are not given to the swagger-ui, for this reason it is unable to resolve $refs and this feature doesn't work in v2 and v3.
This will be fixed for fastify-v3 for sure

@mcollina
Copy link

mcollina commented May 5, 2020

I don't understand this issue unfortunately :(.
Can you open an issue in the fastify repo with what is not possible in v3/what is missing?

This will be fixed for fastify-v3 for sure

I do not understand what the problem is and what the fix will be.

@adrai
Copy link
Contributor Author

adrai commented May 5, 2020

Added a test in my fastify fork:
2.x branch => adrai/fastify@e86e23d (works)
master branch =>adrai/fastify@0d535c4 (fails)

for v3 this changed:
this: vacation: 'sharedAddress#'
to: vacation: { $ref: 'sharedAddress#' }

I don't know if this is an issue or "by design"

@mcollina
Copy link

mcollina commented May 5, 2020

That's by design and part of some of the changes @Eomm made. I would recommend to open an issue on the fastify tracker to discuss.

I think fastify/fastify#2166 is supposed to fix that.

@Eomm
Copy link

Eomm commented May 5, 2020

I'll try to explain what is happening

  • v2:

this vacation: 'sharedAddress#', at runtime, become vacation: { a json }.
The string is replaced with the right object.
NB: v2 supports $ref too, and it would not work as per v3.

  • in v3:

this code { $ref: 'sharedAddress#' } remains this { $ref: 'sharedAddress#' }
fastify-swagger doesn't manage the $ref nor inject the shared schema to swagger-ui

"by design" we remove the non standard "replace schema feature", here all the details fastify/fastify#2023


solutions for @adrai :

  • slow: update fastify-swagger (fastify-oas should have the same issue). I know it and I have in plan to do so since I want/need to improve a lot this plugin
  • fast: use fastify.getSchema('sharedAddress') instead of $ref (read here how )

@adrai
Copy link
Contributor Author

adrai commented May 5, 2020

@Eomm thank you for the explanation.
I would prefer "slow" (I have no time pressure to update to fastify v3 currently, just did a first test)

@SkeLLLa
Copy link
Owner

SkeLLLa commented May 22, 2020

Some ideas. @adrai.

I had an idea to make behaviour that "addModels" do default, so if you use addSchema, then it will appear in OpenAPI spec. But when schema is added it's hard to determine in which place it should be stored (#/components/schemas/ or #/components/parameters/). So @Eomm PR give me an idea how to do that.

Instead of adding schemas when they are registered, we can add schemas when we find $ref and add it to proper components section https://swagger.io/docs/specification/components/.

  1. Scan route schema for $ref.
  2. Obtain schema from some "cache" or from "getSchemasRecursive".*
  3. General rule - get schema and put it into proper section with given name or maybe with instance prefix + name. In case if it's already stored there - just skip (or maybe compare it and error if they are different).
    3.1. $ref in "root" level means that we should reference the whole item (e.g. body, response, )
{
  body: { $ref: 'my-body' } // it will contain description and so on
}

3.1.1 If parsing body, then store in #/components/requestBodies/.
3.1.2 If parsing responses, then store in #/components/responses/.
3.1.3 If parsing path, query, header, cookie, then store in #/components/parameters/.
3.1.4 If parsing examples, then store in #/components/examples/.

3.2. $ref in non-root level (e.g. when it describes data models)

{
  body: { 
    type: object,
    description: 'my body'
    properties: { 
       foo: {
          $oneOf: [{ $ref: 'cat' }, { $ref: 'dog' }],
       }
    }
  }
}

3.2.1 Data models should be added to #/components/schemas/.

Also there's other option that could replace [2]. When each instance is registered, this plugin should add a route that will expose schemas that it adds. And use "Remote Reference". If we have some "service" that's registered via fastify.register and it has prefix /foo, then we need a route
/foo/schemas.json and put components there. And in main spec we could use reference like

{
  $ref: '/foo/__schemas__.json#/components/schema/Dog'
}

@intpp
Copy link

intpp commented Jun 30, 2020

any updates?

@BnayaZil
Copy link

BnayaZil commented Jul 23, 2020

@SkeLLLa after I update fastify-swagger to version 3.1.2 and modify the fastify: '>=2.0.0 <2.12.0 || >=2.12.1 <4.0.0' I manage to run my docs locally, maybe we can push this change in?

@SkeLLLa
Copy link
Owner

SkeLLLa commented Jul 23, 2020

In order to get this work with fastify v3, you'll need to setup fastify-oas@next.

@fyang1024
Copy link
Contributor

fyang1024 commented Aug 17, 2020

Hi @SkeLLLa This is probably a noob question. What do you mean setup fastify-oas@next? I am using fastify 3.2.0, and I get a blank page when accessing the open api doc generated by fastify-oas 3.0.1

The browser console shows an error message like this

Refused to execute inline script because it violates the following Content Security Policy directive: "script-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-Fu/hp0/DDYaCk5au7sfb4Q8mJ26rgJ10wlirmHP3isE='), or a nonce ('nonce-...') is required to enable inline execution.

at index.html:38:1

And index.html:38:1 points to the first line below

 <script>
    window.onload = function() {
      // Begin Swagger UI call region
      const ui = SwaggerUIBundle({
        url: resolveUrl('./json'),
    validatorUrl: null,
    oauth2RedirectUrl: resolveUrl('./oauth2-redirect.html'),
        dom_id: '#swagger-ui',
        deepLinking: true,
        presets: [
          SwaggerUIBundle.presets.apis,
          SwaggerUIStandalonePreset
        ],
        plugins: [
          SwaggerUIBundle.plugins.DownloadUrl
        ],
        layout: "StandaloneLayout"
      })
      // End Swagger UI call region

      window.ui = ui

      function resolveUrl (url) {
          const anchor = document.createElement('a')
          anchor.href = url
          return anchor.href
      }
    }
  </script>

@SkeLLLa
Copy link
Owner

SkeLLLa commented Aug 17, 2020

@fyang1024 it looks like swagger-ui + browser bug. swagger-api/swagger-ui#3370. You can disable CSP in your browser.

@fyang1024
Copy link
Contributor

fyang1024 commented Aug 17, 2020

@fyang1024 it looks like swagger-ui + browser bug. swagger-api/swagger-ui#3370. You can disable CSP in your browser.

Indeed, I disabled csp and it shows the doc now, thank you. But that does not really solve the issue. It seems Swagger UI team does not want to address this issue. I see Java provides such a solution swagger-api/swagger-ui#3370 (comment). I wonder if fastify-oas can do anything similar? I think it's a HTML header, right?

@jmilldotdev
Copy link

Bump, still a problem for me.

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

8 participants