cloudflare / chanfana

OpenAPI 3 and 3.1 schema generator and validator for Hono, itty-router and more!
https://chanfana.pages.dev
MIT License
288 stars 38 forks source link

non-default base doesn't play nicely with Swagger #18

Closed Manouchehri closed 1 year ago

Manouchehri commented 1 year ago

When using a non-root/non-default base value, the Swagger UI seems to ignore what we set.

const router = OpenAPIRouter({
    base: '/plans'
})

i.e. in the example above, Swagger's web UI attempts to make requests to / instead of /plans.

Related: #17

G4brym commented 1 year ago

Hey @Manouchehri I've just pushed a new version (v0.0.12) that fix's this Thanks for reporting this issue

Manouchehri commented 1 year ago

Hmm, do we actually want to prepend the base like 3c411a0970a928bd3542bcc1731385d052a547f7 does?

I did it differently in 13eb443f4b49341bb548b76db5d7e31fad804d0f, so that you don't see the prepended base.

I'm not sure which method is actually more "correct" though.

G4brym commented 1 year ago

In my opinion, if you are defining the base property in the router, then it should be prepended in every endpoint, just like itty-router originally did. Otherwise, if the project you are doing is in a sub path (ex: mydomain.com/api/*) then i think you should manually define the servers object for your project

Manouchehri commented 1 year ago

Otherwise, if the project you are doing is in a sub path (ex: mydomain.com/api/*) then i think you should manually define the servers object for your project

I like doing that, but currently /openapi.json, /docs and /redocs do not respect the server URL. So in the example I gave in #17, accessing anything except for the endpoint becomes impossible.

Manouchehri commented 1 year ago

In this example, Swagger now tries to use /plans/plans/plans/plans.json.

import { OpenAPIRouter, OpenAPIRoute, Query, Int } from '@cloudflare/itty-router-openapi'

const router = OpenAPIRouter({
    base: '/plans'
})

export default {
    fetch: router.handle
}

export class Plans extends OpenAPIRoute {
    static schema = { 
        tags: ['pricing', 'prices', 'plan', 'plans'],
        summary: "Get the list of products.",
        parameters: {
            product: Query(Int, {
                description: 'Product number',
                default: 1,
                required: false,
              }),
        },
        servers: [
            {
                url: 'plans/'
            }
        ]
    }

    async handle(request, env, ctx, data) {
        const { product }  = data;
        console.debug('------');

        return new Response("Hello world", {
            status: 200
        })
    }
}

router.get('/plans.json', Plans);
router.all('*', () => new Response('Not Found.', { status: 404 }))

Ironically this way works in v0.0.11 but broke with v0.0.12. =P

G4brym commented 1 year ago

Theservers object is supposed to go inside the router schema and not the endpoint schema this works fine:

import { OpenAPIRouter, OpenAPIRoute, Query, Int } from '@cloudflare/itty-router-openapi'

const router = OpenAPIRouter({
    base: '/plans'
})

export default {
    fetch: router.handle
}

export class Plans extends OpenAPIRoute {
    static schema = {
        tags: ['pricing', 'prices', 'plan', 'plans'],
        summary: "Get the list of products.",
        parameters: {
            product: Query(Int, {
                description: 'Product number',
                default: 1,
                required: false,
              }),
        },
    }

    async handle(request, env, ctx, data) {
        const { product }  = data;
        console.debug('------');

        return new Response("Hello world", {
            status: 200
        })
    }
}

router.get('/plans.json', Plans);
router.all('*', () => new Response('Not Found.', { status: 404 }))

I have indeed deployed this script to a sub path of a domain with this route: example.com/plans/* and both /plans/docs and trying out the endpoint works as expected

What i was saying above is that you either define the base property or the servers object, not both at the same time