wI2L / fizz

:lemon: Gin wrapper with OpenAPI 3 spec generation
https://pkg.go.dev/github.com/wI2L/fizz
MIT License
214 stars 52 forks source link

Support for Open API extensions? #54

Closed nikicc closed 3 years ago

nikicc commented 3 years ago

Hi!

I was wondering if Fizz currently supports a way to add Open API extensions to the autogenerated documentation?

Especially, the things I had in mind are the following:

I checked and I got the felling this is not supported. If this is true, how do we fell about adding this? Any ideas how the support could be added in a general way so all extensions would be supported?

If general supports turns out infeasible, how do we feel about adding support for this case-by-case? I check for the logos case, and extending the Info struct to add the logo information seems easy. Similarly for tag groups. But before I'd tackle the implementation, I wanted to check how do we even feel about adding support for extensions? Would this be welcome, or does it contradicts the idea of Fizz?

wI2L commented 3 years ago

Hello @nikicc

Basically, everything that is part of the official spec can be added in https://github.com/wI2L/fizz/blob/master/openapi/spec.go. The spec can be edited by accessing it with (f *Fizz) Generator() and (g *Generator) API(), https://github.com/wI2L/fizz/blob/master/openapi/generator.go#L108.

I might be reluctant when it comes to extend the existing package API with helper method that set whatever new field in the spec. I'd like to keep the package's surface API light and not overwhelm it with too many methods that most people won't use.

nikicc commented 3 years ago

To confirm I understood you correctly: you are OK with adding extra x-* fields to the spec (e.g. adding x-logo to Info struct, x-tagGroups to OpenAPI and x-codeSamples to Operation), but would resist adding helper methods to populate those, right? Is so, I'm happy to make the PR to add those fields to the corresponding structs.

I might be reluctant when it comes to extend the existing package API with helper method that set whatever new field in the spec. I'd like to keep the package's surface API light and not overwhelm it with too many methods that most people won't use.

I agree, let's set those through Generator().API() to not pollute the public API.

wI2L commented 3 years ago

To confirm I understood you correctly: you are OK with adding extra x-* fields to the spec (e.g. adding x-logo to Info struct, x-tagGroups to OpenAPI and x-codeSamples to Operation), but would resist adding helper methods to populate those, right?

That, exactly.

Is so, I'm happy to make the PR to add those fields to the corresponding structs.

Sure, go ahead. If you can share links to the official spec/reference for those extenions, that would be helpful for review.

nikicc commented 3 years ago

Sure, go ahead. If you can share links to the official spec/reference for those extenions, that would be helpful for review.

Sure thing. Will also add the links along with the structs.

Also, have a quick followup on your previous point:

Basically, everything that is part of the official spec can be added in https://github.com/wI2L/fizz/blob/master/openapi/spec.go. The spec can be edited by accessing it with (f Fizz) Generator() and (g Generator) API(), https://github.com/wI2L/fizz/blob/master/openapi/generator.go#L108.

(g *Generator) API() returns a copy of the API. Is that intentional? Asking as it makes the use of other methods (like OpenAPI) harder as the changes on the copy would not be considered. For example, you can't do something like this:

f := fizz.New()

// set extention fields 
f.Generator().API().XTagGroups = []*openapi.XTagGroup{
        {
            Name: "Group Name",
            Tags: []string{"Tag"},
        },
    }

// register endpoint
f.GET(
    "/openapi",
    nil,
    f.OpenAPI(*openapi.Info{...}, "yaml"),
)

It's not that big of an issue, as you can always write your own gin handler and return the right spec. Just wanted to get your thoughts.

wI2L commented 3 years ago

@nikicc It's intended that the (g *Generator) API() returns a copy of the spec, to avoid having to messa with the generator's internal. If a client want to modify/extend the spec after the first generation, one can indeed copy the spec object, modify it, and expose a custom handler that return the marshaled object. There's no downside to that, but it forces the declaration of the handler serving the spec after the declaration of the router's routes.