Replace go-swagger with huma #33685
Comments
TheFox0x7
added
the
type/proposal
|
It seems to be more user-friendly for Chi routers. |
Gitea isn't really using Chi, it is using a heavily customized router system. Actually we could completely drop Chi by adding some more code since Gitea's "router framework" has been complex enough. |
|
All integrations are just adapters for it so it isn't a big problem... I hope.
Oh. Oh that explains why it's almost like chi but different enough to be more confusing. And why I have trouble wrapping my head around it at times. And why it takes |
I really hope it can be used independently by other projects. |
|
off topic Out of curiosity - why? I find it very confusing compared to basic chi and besides port of the Maybe there's a killer feature which I don't see - hence the question. I'm not even suggesting to swap it, as I know it won't be a good use of anyone's time and won't be accepted. I'm mostly curious what's the big benefit I'm overlooking. |
|
That's really a long history, it can't simply say it's right or wrong. Some key points are like this:
ps: in my mind, |
yes. it is off topic, just ignore my comment. 😉 |
At the moment, there is a blocker reason: many middlewares and handlers have different "ctx" arguments: for example: APIContext, PrivateContext, ArtifactContext, (Web)Context. (ps: some of these contexts were already abused due to various reasons, I left some comments in code when I found them) Golang's generic type system is quite weak, so the router method functions have to accept "any" at the moment (I added some runtime validation during "init" stage in some refactoring PRs to make sure there won't be low-level mistakes) |
|
I meant my comment was off topic for the proposal but your comment got me interested in the subject as I get really confused around that area anyway :)
I'm not a fan either especially since they don't often have a reason to be part of the struct except for less func arguments. I think that More on topic - writing an adapter for gitea router with APIContext is possiblefunc New(r *web.Router, config huma.Config) huma.API {
return huma.NewAPI(config, &teaAdapter{router: r})
}
type teaAdapter struct {
router *web.Router
}
type subApiContext struct {
*context.APIContext
op *huma.Operation
}
// AppendHeader implements huma.Context.
func (s *subApiContext) AppendHeader(name string, value string) {
s.RespHeader().Add(name, value)
}
// BodyReader implements huma.Context.
func (s *subApiContext) BodyReader() io.Reader {
return s.Req.Body
}
// BodyWriter implements huma.Context.
func (s *subApiContext) BodyWriter() io.Writer {
return s.Resp
}
// Context implements huma.Context.
func (s *subApiContext) Context() goctx.Context {
return s
}
// EachHeader implements huma.Context.
func (s *subApiContext) EachHeader(cb func(name string, value string)) {
for name, values := range s.Req.Header {
for _, value := range values {
cb(name, value)
}
}
}
// GetMultipartForm implements huma.Context.
func (s *subApiContext) GetMultipartForm() (*multipart.Form, error) {
err := s.Req.ParseMultipartForm(8 * 1024) //Temp because I know it's there somewhere but it's a draft
return s.Req.MultipartForm, err
}
// Header implements huma.Context.
func (s *subApiContext) Header(name string) string {
return s.Req.Header.Get(name)
}
// Host implements huma.Context.
func (s *subApiContext) Host() string {
return s.Req.Host
}
// Method implements huma.Context.
func (s *subApiContext) Method() string {
return s.Req.Method
}
// Operation implements huma.Context.
func (s *subApiContext) Operation() *huma.Operation {
return s.op
}
// Param implements huma.Context.
func (s *subApiContext) Param(name string) string {
return s.Req.PathValue(name)
}
// Query implements huma.Context.
func (s *subApiContext) Query(name string) string {
return queryparam.Get(s.Req.URL.RawQuery, name)
}
// SetHeader implements huma.Context.
func (s *subApiContext) SetHeader(name string, value string) {
s.Resp.Header().Set(name, value)
}
// SetReadDeadline implements huma.Context.
func (s *subApiContext) SetReadDeadline(deadline time.Time) error {
return huma.SetReadDeadline(s.Resp, deadline)
}
// SetStatus implements huma.Context.
func (s *subApiContext) SetStatus(code int) {
s.Resp.WriteHeader(code)
}
// Status implements huma.Context.
// Subtle: this method shadows the method (APIContext).Status of subApiContext.APIContext.
func (s *subApiContext) Status() int {
return s.WrittenStatus()
}
// TLS implements huma.Context.
func (s *subApiContext) TLS() *tls.ConnectionState {
return s.Req.TLS
}
// URL implements huma.Context.
func (s *subApiContext) URL() url.URL {
return *s.Req.URL
}
// Version implements huma.Context.
func (s *subApiContext) Version() huma.ProtoVersion {
return huma.ProtoVersion{
Proto: s.Req.Proto,
ProtoMajor: s.Req.ProtoMajor,
ProtoMinor: s.Req.ProtoMinor,
}
}
// Handle implements huma.Adapter.
func (t *teaAdapter) Handle(op *huma.Operation, handler func(ctx huma.Context)) {
t.router.Methods(op.Method, op.Path, func(apiCtx *context.APIContext) {
handler(&subApiContext{op: op, APIContext: apiCtx})
})
}
// ServeHTTP implements huma.Adapter.
func (t *teaAdapter) ServeHTTP(w http.ResponseWriter, r *http.Request) {
t.ServeHTTP(w, r)
}
func RoutesV2() *web.Router {
router := web.NewRouter()
router.Use(securityHeaders())
if setting.CORSConfig.Enabled {
router.Use(cors.Handler(cors.Options{
AllowedOrigins: setting.CORSConfig.AllowDomain,
AllowedMethods: setting.CORSConfig.Methods,
AllowCredentials: setting.CORSConfig.AllowCredentials,
AllowedHeaders: append([]string{"Authorization", "X-Gitea-OTP"}, setting.CORSConfig.Headers...),
MaxAge: int(setting.CORSConfig.MaxAge.Seconds()),
}))
}
router.Use(context.APIContexter())
router.Use(apiAuth(buildAuthGroup()))
api := New(router, huma.DefaultConfig("Gitea API", setting.AppVer))
api.OpenAPI().Servers = []*huma.Server{
&huma.Server{URL: setting.AppURL + "api/v1/"},
}
huma.Get(api, "/version", misc.Version)
huma.Get(api, "/signing-key.gpg", misc.SigningKey)
huma.Get(api, "/gitignore/templates", misc.ListGitignoresTemplates)
huma.Get(api, "/gitignore/templates/{name}", misc.GetGitignoreTemplateInfo)
huma.Get(api, "/licenses", misc.ListLicenseTemplates)
huma.Get(api, "/licenses/{name}", misc.GetLicenseTemplateInfo)
huma.Get(api, "/label/templates", misc.ListLabelTemplates)
huma.Get(api, "/label/templates/{name}", misc.GetLabelTemplate)
settingsAPI := huma.NewGroup(api, "/settings")
huma.Get(settingsAPI, "/ui", settings.GetGeneralUISettings)
huma.Get(settingsAPI, "/api", settings.GetGeneralAPISettings)
huma.Get(settingsAPI, "/attachment", settings.GetGeneralAttachmentSettings)
huma.Get(settingsAPI, "/repository", settings.GetGeneralRepoSettings)
return router
} |
I think we shouldn't allow returning an "error" directly. It would be abused a lot (using "err" as user error has been abused a lot in current code base). The reason is that some error contains internal sensitive information, for example: database IP, git filesystem path, internal URL, etc. Golang also has a weak error system, we are not able to use any error directly for end users. For safety, we need to only process the errors we could handle. And
The approach looks pretty good. I am not familiar with huma so I can't comment too much for that part. For the API part, for the Req/Context part:
|
|
Don't look too much into the implementation - it's mainly a test if it could be done fairly simply and it's based on humachi wrapper which had a comment that it can be replaced with that function in go 1.22+. I wasn't looking into it that much and I'm sure there are more issues with it but it works as a demo.
Good question, middlewares in huma have 2 layers, global router level ones (so web.Router middleware) and huma layer ones (with PR I'm running which adds groups and group layer for huma). So some middlewares might need adoption (for example the auth one) func Demo() {
api := New(router, huma.DefaultConfig("Gitea API", setting.AppVer))
huma.Get(api, "/version", misc.Version) // No middleware adding by default, unless with func (o *huma.Operation){} which does have access to middlewares - but I don't think it's a correct approach.
// Alternatively:
operation := Operation{
OperationID: GenerateOperationID(http.MethodGet, "/version" , VersionOutput), //Version output is a wrapper for struct with version - see later example
Summary: GenerateSummary(http.MethodGet, "/version" , VersionOutput),
Method: http.MethodGet,
Path: "/version" ,
Middlewares: Middlewares{func(ctx huma.Context, next func(huma.Context)) {
// Some middleware, which has access to [huma.Context](https://pkg.go.dev/github.com/danielgtaylor/huma/v2#Context)
next(ctx)
}},
}
huma.Register(api, operation, misc.Version)
}Discussion about route specific middlewares
I don't think that's needed. It did start and work as expected so I think it does properly, though rewrite of actual handlers will be required, which is the main pain point. I probably should give a small example of such rewrites so we're on the same page and have something to look at: With a really simple version endpoint import (
"context" //huma uses stdlib context
"code.gitea.io/gitea/modules/setting"
)
type VersionOutput struct {
Body struct {
Version string `json:"version" example:"1.23" doc:"Returns the version of the Gitea application"`
} // Response body contains a json, with a string "version". Example for that string is provided with docs
}
// Version shows the version of the Gitea server
func Version(ctx context.Context, _ *struct{}) (*VersionOutput, error) {
// empty struct because endpoint does not accept input
resp := &VersionOutput{}
resp.Body.Version = setting.AppVer
return resp, nil
}With a bit more complex LicenseTemplateInfo type LicenseTemplateInfoOutput struct {
Body api.LicenseTemplateInfo // struct is already defined so we can reuse it. I haven't added tags to it
}
// Returns information about a gitignore template
func GetLicenseTemplateInfo(ctx context.Context, input *struct {
Name string `path:"name", example:"MIT"`
}) (*LicenseTemplateInfoOutput, error) {
// note that input now has data about where the parameter is (in path) and an example payload (MIT)
name := input.Name // not really needed
text, err := options.License(name)
if err != nil {
locale := ctx.Value("locale").(translation.Locale) // HACK.
return nil, huma.Error404NotFound(locale.TrString("error.not_found"), err) // given what you mentioned about errors it might not be the best idea all the time. I haven't looked into error response that much
}
response := api.LicenseTemplateInfo{
Key: name,
[openapi.json](https://github.com/user-attachments/files/18929640/openapi.json)
Name: name,
URL: fmt.Sprintf("%sapi/v1/licenses/%s", setting.AppURL, url.PathEscape(name)),
Body: string(text),
// This is for combatibilty with the GitHub API. This Text is for some reason added to each License response.
Implementation: "Create a text file (typically named LICENSE or LICENSE.txt) in the root of your source code and copy the text of the license into the file",
}
return &LicenseTemplateInfoOutput{Body: response}, nil
}generated json api (with a few more routes): openapi.json And version endpoint (with a different UI but they are swappable): |
Thank you for the demo. IMO we'd better to start from the most complex (hardest) part, including middlewares. If the most complex (hardest) part could be resolved, then the simple APIs could also be resolved. Vice versa is not the case, we might get stuck when handling the complex APIs. And by the way & FYI, it seems that some APIs were not well-designed, for example this newly found one: #33683 (comment) , |
|
Yeah, some will be more difficult than the rest - I got stuck on middlewares as I don't really understand how they are handled right now. As for that particular endpoint, I guess it's a choice between a custom schema with oneOf in response, or splitting the endpoint into a filtered list and and ref query, second one being breaking but more obvious for end user what will they get in response. |
Feature Description
Disclaimer: I'm working on it currently to see if it's doable and so far I think it is. If this gets accepted I'd like to author it.
I'd like to suggest dropping go-swagger based generation in favor of automatically generated docs from code with huma
This would bring following benefits:
Downsides:
func (context.Context, *struct) (*struct, error), context being the std lib one.1Screenshots
No response
Footnotes
Semi related: I find current router system with multiple different functions and contexts being accepted really confusing so maybe it's an area that can be simplified a bit? ↩
The text was updated successfully, but these errors were encountered: