Remove usage of utils/restendpointhelper.go in favor of the new router

[gabek]

Share your bug report, feature request, or comment.

Once #3653 is in, we can refactor out all the use of the endpoint helper and use the native functionality built into the router to support dynamic path components as arguments.

[mahmed2000]

There's the easy way of just calling chi.URLParam to get the path parameter directly from the the request object.

Making a new middleware like pagination won't work with SendSystemMessageToConnectedClient. That already has an auth middleware that changes the function signature. It can't be chained cleanly with another one that does the same.

There's a better way to just use the Context. Store values there so it persists across an arbitrary number of middlewares. No need to introduce changed function signatures.

[gabek]

Sounds like the pagination is a different topic. I haven't looked into that. I'll file a new issue.

[mahmed2000]

Bit of a misunderstanding. To clarify, its more to do with design than anything being broken or non-functional.

---

Pagination works but the middleware directly queries the params from the request object instead of taking the arguments set in the handler, for example:

owncast/handler/handler.go

Line 78 in ae76abb

func (*ServerInterfaceImpl) GetFollowers(w http.ResponseWriter, r *http.Request, params generated.GetFollowersParams) {

The params argument is set by the new generated router and contains both the limit and offset args. It is ignored, which seems incorrect to me. Currently the pagination middleware just takes it from a standard r.Query().Get() call. The equivalent for the path params would just be the chi.URLParam() I mentioned. With this the userId and clientId params would go ignored:

owncast/handler/integrations.go

Line 22 in ae76abb

func (*ServerInterfaceImpl) SendSystemMessageToConnectedClient(w http.ResponseWriter, r *http.Request, clientId int) {

The generated router already surfaces this once, seems wasteful to do it again, but wouldn't exactly break anything.

---

The problem is more the incompatibility of chaining stuff like:

owncast/router/middleware/pagination.go

Line 9 in ae76abb

type PaginatedHandlerFunc func(int, int, http.ResponseWriter, *http.Request)

with this:

owncast/router/middleware/auth.go

Line 15 in ae76abb

type ExternalAccessTokenHandlerFunc func(user.ExternalAPIUser, http.ResponseWriter, *http.Request)

They work as their own ends of a chain, but if they need to be chained (like a hypothetical path parameter handler that is wrapped around the externalAPIUser auth middleware), it won't be clean. One would have to know about the other since the controller function would need to surface the other's params to be called.

They can't wrap each other since neither can accept a standard HandlerFunc with only the writer and request, but do return that.

They're more "end"-wares than middle, is my problem.

RequireExternalAPIAccessToken(scope, HandlePagination(dummy_func))(w, r)

won't work since the Access expects a ExternalAccessTokenHandlerFunc instead of a HandlerFunc

or

HandlePagination(ScopedRequireExternalAPIAccessToken(dummy_func))(w, r)

which also won't work since the pagination expects a PaginatedHandlerFunc instead of a HandlerFunc

---

We could ignore it for this case, but what happens if there's ever an endpoint that needs both pagination and and external API access?

The controller would look like:
func (int, int, user.ExternalAPIUser, responseWriter, Request)

Using contexts, we can simply use that to pass values around instead, and middlewares don't need to create these custom types to deal with specific arguments and can all just take and return just the writer and request objects.

[gabek]

I see what you're saying. We could remove the pagination middleware, as it is there primarily for convenience, and just export a similar version as a utility function that can be used in handlers that require pagination. The result would be the same, the logic for pulling out those values would just be done in a different place.