OpenAPI Collector not traversing child nodes, or parsing their mountpoints

[shairozan]

I'm using this framework in some projects at work and really love it, but the rough edge I've run up could be me missing something.

In the go-chi world, it's common to have different routers mounted to parent routers. This is a way of separating out / differentiating the middlewares as part of the radix tree. Thus your middlewares can be applied closer to your domain objects in a way that makes sense.

With this library, I've struggled with this, specifically because of the documentation side. If I create a default router:

baseRouter := web.DefaultService()

But then try to add a sub router to it (for endpoints that require authentication via JWT for example)

baseRouter := web.DefaultService()
authenticated := web.DefaultService()

baseRouter.Mount("/authenticated",authenticated)

Will work but without documentation. If I force the sub router to have the same collector as the parent during construction the docs are present but missing the /authenticated prefix from the mount.

I wrote Usecase as a workaround by basically allowing this kind of setup as the UseCase implementation layer as well to allow for HTTP level globals and the further down the stack to a more implementation specific one.

Is there something I'm just missing about how the collector is supposed to be traversing nodes to generate the docs?

[vearutop]

Hi, thank you for raising this concern. Mount indeed works counter intuitively.

I've experimented a bit with introspection and wrapping of handlers in #86, but I'm not sure if this is a good change.

The problem is that mounted handler/router has made impact on docs collection and handlers setup. Merging the product into another docs/router seems like a less reliable process compared to linear flow when a handler is added into its final place initially.

(For example this prototype implementation would apply security documentation to POST /sum2 and POST /mul2 if they were added with service.Post(...) because security middleware of sub declares openapi annotation for these routes.)

Maybe this implementation will be good enough for the edge case, given it only opens a can of worms for Mount users and would not affect other cases.

As a workaround I can suggest using Route instead:

    // Endpoints with admin access.
    s.Route("/admin", func(r chi.Router) {
        r.Group(func(r chi.Router) {
            r.Use(nethttp.AnnotateOpenAPI(s.OpenAPICollector, func(op *openapi3.Operation) error {
                op.Tags = []string{"Admin Mode"}

                return nil
            }))
            r.Use(adminAuth, nethttp.HTTPBasicSecurityMiddleware(s.OpenAPICollector, "Admin", "Admin access"))
            r.Method(http.MethodPut, "/tasks/{id}", nethttp.NewHandler(usecase.UpdateTask(locator), ff))
        })
    })

    // Endpoints with user access.
    s.Route("/user", func(r chi.Router) {
        r.Group(func(r chi.Router) {
            r.Use(userAuth, nethttp.HTTPBasicSecurityMiddleware(s.OpenAPICollector, "User", "User access"))
            r.Method(http.MethodPost, "/tasks", nethttp.NewHandler(usecase.CreateTask(locator),
                nethttp.SuccessStatus(http.StatusCreated)))
        })
    })

https://github.com/swaggest/rest/blob/v0.2.29/_examples/task-api/internal/infra/nethttp/router.go#L54-L74

[shairozan]

I think mount may be an ugly route to go. The Route workaround works for me where the Method wrapper takes a handler. It's easy enough to provide a Handler method to the Usecase on top of Interactor so that nothing changes about how the core business logic is written. I'd say I'm good with the workaround with Route since the docs are working. Wouldn't worry about messing the code base up too much :)

Thanks much for the help!