huma icon indicating copy to clipboard operation
huma copied to clipboard

Suggestion: groups and sub-routing

Open cardinalby opened this issue 1 year ago • 2 comments

Hi! Thanks for the great library, I believe it's the best approach to documenting API in Go ecosystem so far.

Although, I found a batch of issues created by different people here regarding:

  • groups (how to define several routing groups with different set of middlewares?)
  • sub-routing (how to define sub-route group that will prefix all operations and keep auto-generated operation ids and summaries relevant)

Also, some of the features (Transformers) are available only globally for the API instance and can't be set per operation.

OpenAPI endpoints are registered directly with Adapter and I can't apply middlewares to them (how to add auth for openapi.yml endpoint?)

I believe these issues should be addressed by Huma instead of by tuning underlying routers:

  • being aware of that things Huma can create proper operation ids and summaries
  • it will be independent of router capabilities
  • it would provide consistent user experience and nothing except std http.ServeMux would be needed for a user
  • it would make easier to migrate from other routers

I want to use Huma in my company's project, we need all these features to migrate from Gin due to the project structure and heavy usage of gin groups.

I found an elegant way to provide all these features with Huma solely and want you to take a look at it. I believe it can be integrated into Huma with full backward compatibility resolving all the issues you have on Github.

Check out the library and contact me if you would be interested in bringing these features into Huma. I believe it would be even more elegant being a part of Huma rather than a separate library.

Some examples:

🔻 "Base path + tags + middlewares" group

v1gr := api.            // all operations registered with v1gr will have:
	AddBasePath("/v1").                            // - "/v1" base path
	AddOpHandler(op_handler.AddTags("some_tag")).  // - "some_tag" tag
	AddMiddlewares(m1, m2)                         // - m1, m2 middlewares
	
hureg.Get(v1gr, "/cat", ...) // "/v1/cat" with "some_tag" tag and m1, m2 middlewares
hureg.Get(v1gr, "/dog", ...) // "/v1/dog" with "some_tag" tag and m1, m2 middlewares

🔻 Multiple base paths

Sometimes we need to register the same endpoint with multiple base paths (e.g. /v1 and /v2).

multiPathGr := api.AddMultiBasePaths(nil, "/v1", "/v2")

hureg.Get(multiPathGr, "/sparrow", ...) // "/v1/sparrow"
                                        // "/v2/sparrow"

🔻 Transformers per group

trGr := api.AddTransformers(...) // transformers will be applied only to the operations 
                                 // registered in this group

hureg.Get(trGr, "/crocodile", ...)

cardinalby avatar Jun 25 '24 21:06 cardinalby

Check out the library and contact me if you would be interested in bringing these features into Huma.

@cardinalby, I'm not sure how to contact you other than here. FWIW, I think the features your library pair well with ❤️ Huma ❤️. My use case is an API protected by an authentication middleware BUT there are some endpoints which do not need or cannot require authentication. It feels weird to break them out in to a separate API OR have to remember to add the authentication middleware for every API endpoint except the few exceptions (which seems error prone).

Consider a contrived example:

# no need for authentication
POST /api/authentication
POST /api/reset_password
POST /api/create_account
GET  /api/plans

# requires authentication
GET  /api/authentication
... everything else

braindev avatar Oct 07 '24 23:10 braindev

@braindev Hi and thanks :) You are right, it's one of the use-cases I created it for. Same as base paths for groups, middlewares are must-have for routing.

P.S. It's the issue I created hoping to contact the Huma author if he is interested in the features from my lib. If you have questions regarding Hureg usage you can create an issue at Hureg's issues page.

cardinalby avatar Oct 08 '24 08:10 cardinalby

FWIW, I had a similar issue and solved it via moving the authentication middleware to the router-level, so I could skip the auth middleware on public routes:

func Authenticate(next http.Handler) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
      // Ignore public endpoints
      publicRoutes := []string{"/api/authentication", "/api/reset_password", "/api/create_account" }
      for _, route := range publicRoutes {
        if r.URL.Path == route {
          next.ServeHTTP(w, r)
          return
        }
      }
     
    // Continue authentication middleware
     ...

There isn't a huma-first solution AFAIK, but now every route is authenticated unless specified in the middleware. The hureg library looks great though

Jtcruthers avatar Jan 08 '25 07:01 Jtcruthers

@Jtcruthers that's feasible, but it takes out some of the Huma advantages: declarative routes description. Aside from necessity to duplicate route paths (strictly speaking you should also check HTTP method here), to have nice OpenAPI you need to additionally assign "security" entries for these routes while registering them with Huma. For example, if they are correctly set it's possible to make requests through OpenAPI web explorer (filling the token field) and it's clear for OpenAPI consumer which security protocol particular endpoints expect.

That's the use-case for "operation handlers" in Huma. You can create an operation handler that will add an auth middleware and a corresponding security entry to an operation at once.

You don't even need Hureg in this case (it just makes it more convenient to make it for groups of routes)

cardinalby avatar Jan 08 '25 16:01 cardinalby

These issues are also blocking my team from migrating from gin. Hureg is really nice, but it would be even better if it was already part of huma.

Luks17 avatar Jan 17 '25 19:01 Luks17

This is also blocking our adoption of huma, while the hureg by @cardinalby looks really nice indeed, we'd prefer for this to be inside huma itself.

nutgood avatar Feb 03 '25 19:02 nutgood

This is also blocking our adoption of huma, while the hureg by @cardinalby looks really nice indeed, we'd prefer for this to be inside huma itself.

me too, but for the while it has to be a separate project since unfortunately it seems that @danielgtaylor doesn't have capacity to look at it closer

cardinalby avatar Feb 03 '25 22:02 cardinalby

This is also blocking our adoption of huma, while the hureg by @cardinalby looks really nice indeed, we'd prefer for this to be inside huma itself.

me too, but for the while it has to be a separate project since unfortunately it seems that @danielgtaylor doesn't have capacity to look at it closer

I'm sure he has good personal reasons for not looking at it yet. This is an amazing project, but it must take a lot of effort an time to maintain.

Luks17 avatar Feb 03 '25 22:02 Luks17

Don't blame him at all, just addressing comments saying that it should be a part of Huma. As an author of a separate lib I'm ready to deprecate it and to help to integrate it to Huma since I believe it will provide more consistent and better experience. I made my lib just for one reason, because I believe Huma goes the right way and it's the most promising approach among other ways to define our endpoints

cardinalby avatar Feb 03 '25 22:02 cardinalby

Sorry folks I'm trying my best here, and thank you @cardinalby this library is great. It seems there is a big desire for this functionality to be in the core, so if we can do it without breaking the huma.API interface I would be happy to see this land in the core if someone wants to try and get a PR up.

I played around a bit today and came up with this which might work if someone wants to run with it (or see why it wouldn't work or wouldn't work well):

type groupAdapter struct {
	huma.Adapter
	prefixes []string
	// TODO: add operation modifiers here?
}

func (a *groupAdapter) Handle(op *huma.Operation, handler func(huma.Context)) {
	for _, prefix := range a.prefixes {
		modified := *op
		modified.Path = prefix + modified.Path
		a.Adapter.Handle(&modified, handler)
	}
}

type Group struct {
	huma.API
	adapter     huma.Adapter
	middlewares huma.Middlewares
	// TODO: add group transformers?
}

func NewGroup(api huma.API, prefixes ...string) *Group {
	if len(prefixes) == 0 {
		prefixes = append(prefixes, "")
	}
	return &Group{API: api, adapter: &groupAdapter{Adapter: api.Adapter(), prefixes: prefixes}}
}

func (g *Group) Adapter() huma.Adapter {
	return g.adapter
}

func (g *Group) UseMiddleware(middlewares ...func(ctx huma.Context, next func(huma.Context))) {
	g.middlewares = append(g.middlewares, middlewares...)
}

func (g *Group) Middlewares() huma.Middlewares {
	m := append(huma.Middlewares{}, g.API.Middlewares()...)
	return append(m, g.middlewares...)
}

Usage example:

grp := NewGroup(api, "/v1", "/v2")
grp.UseMiddleware(func(ctx huma.Context, next func(huma.Context)) {
	fmt.Println("I'm a middleware")
	next(ctx)
})

huma.Get(grp, "/test", func(ctx context.Context, i *struct{}) (*struct{}, error) {
	return nil, nil
})

Then you can fetch /v1/test or /v2/test and it'll print out from the middleware, and it continues using the existing huma.Register/huma.Get functions (and requires no modification of huma.API. What do you think?

danielgtaylor avatar Feb 10 '25 22:02 danielgtaylor

Sorry folks I'm trying my best here, and thank you @cardinalby this library is great. It seems there is a big desire for this functionality to be in the core, so if we can do it without breaking the huma.API interface I would be happy to see this land in the core if someone wants to try and get a PR up.

I played around a bit today and came up with this which might work if someone wants to run with it (or see why it wouldn't work or wouldn't work well):

type groupAdapter struct { huma.Adapter prefixes []string // TODO: add operation modifiers here? }

func (a *groupAdapter) Handle(op *huma.Operation, handler func(huma.Context)) { for _, prefix := range a.prefixes { modified := *op modified.Path = prefix + modified.Path a.Adapter.Handle(&modified, handler) } }

type Group struct { huma.API adapter huma.Adapter middlewares huma.Middlewares // TODO: add group transformers? }

func NewGroup(api huma.API, prefixes ...string) *Group { if len(prefixes) == 0 { prefixes = append(prefixes, "") } return &Group{API: api, adapter: &groupAdapter{Adapter: api.Adapter(), prefixes: prefixes}} }

func (g *Group) Adapter() huma.Adapter { return g.adapter }

func (g *Group) UseMiddleware(middlewares ...func(ctx huma.Context, next func(huma.Context))) { g.middlewares = append(g.middlewares, middlewares...) }

func (g *Group) Middlewares() huma.Middlewares { m := append(huma.Middlewares{}, g.API.Middlewares()...) return append(m, g.middlewares...) } Usage example:

grp := NewGroup(api, "/v1", "/v2") grp.UseMiddleware(func(ctx huma.Context, next func(huma.Context)) { fmt.Println("I'm a middleware") next(ctx) })

huma.Get(grp, "/test", func(ctx context.Context, i *struct{}) (*struct{}, error) { return nil, nil }) Then you can fetch /v1/test or /v2/test and it'll print out from the middleware, and it continues using the existing huma.Register/huma.Get functions (and requires no modification of huma.API. What do you think?

That worked pretty well, but I was wondering how to keep the docs generated correctly with this change.

newton-peixoto avatar Feb 13 '25 21:02 newton-peixoto

@newton-peixoto I knew I rushed that and would miss something! I don't have a solution for updating the docs when multiple prefixes are desired 🤔 For a single prefix it would be easy to modify the operation struct at registration time...

danielgtaylor avatar Feb 15 '25 01:02 danielgtaylor

@newton-peixoto I knew I rushed that and would miss something! I don't have a solution for updating the docs when multiple prefixes are desired 🤔 For a single prefix it would be easy to modify the operation struct at registration time...

For our useCase we needed only one prefix per group so we did it by modifying the operation as you said and it worked just fine.

newton-peixoto avatar Feb 15 '25 01:02 newton-peixoto

@cardinalby @newton-peixoto @Luks17 @nutgood @Jtcruthers @braindev FYI I have created #728 to try and address this. Please have a look, try it out & break it if possible, and leave a review. Thanks!

danielgtaylor avatar Feb 16 '25 01:02 danielgtaylor

@danielgtaylor, you're a wizard and this completely addresses our concerns.

nutgood avatar Feb 18 '25 19:02 nutgood

Amazing work @danielgtaylor , thank you!

Luks17 avatar Feb 20 '25 15:02 Luks17