Inconsistent Chi(net/http) middleware func signature

[wangyoucao577]

Hi there,

Many thanks to @maclav3 , I'm able to use the chi middlewares per handler(PR #259) with the request validator chi-middleware (PR #278) to validate restful request via openapi spec. However, I found that the middleware func interface is inconsistent.

oapi-codegen generated middleware func singnature is type MiddlewareFunc func(http.HandlerFunc) http.HandlerFunc, but most of industry guys use func(http.Handler) http.Handler instead(e.g., https://www.alexedwards.net/blog/making-and-using-middleware). PR #278 satisfied func(http.Handler) http.Handler as well.

As above, I have to write some ugly codes like this to make them work:

	sampleSwagger, err := sample.GetSwagger()
	if err != nil {
		glog.Errorf("Error loading sample swagger spec\n: %s", err)
		return
	}
	sampleSwagger.Servers = nil
	sampleValidatorMiddleware := chimiddleware.OapiRequestValidator(sampleSwagger)

	sample.HandlerWithOptions(sampleService, sample.ChiServerOptions{
		BaseRouter: mux,

                // ugly adaptor code
		Middlewares: []sample.MiddlewareFunc{func(next http.HandlerFunc) http.HandlerFunc {
			return http.HandlerFunc(sampleValidatorMiddleware(next).ServeHTTP)
		}},
	})

So I'll suggest to change the MiddlewareFunc singature to type MiddlewareFunc func(http.Handler) http.Handler when generate code.

@maclav3 How's your opinion?

Thanks!

[maclav3]

Hey @wangyoucao577

That sounds like a perfectly reasonable idea; in fact after using the existing signature for a bit, I agree that it is a bit unfortunate 😅

Now the only problem that arises is that we'd be possibly breaking someone's code.
If we wanted to be strict, we should bump the major version.

But:

1. The innovation is recent, so the impact should be small
2. The change should be very easy to make
3. Bumping major impacts everybody, changing the signature only impacts those who have started using the middlewares option.

Let's ask for the opinion of the main maintainer @deepmap-marcinr ?

[wangyoucao577]

@maclav3 Excellent! Personally I agree with you that the impact should be small, maybe we don't need to be so that strict, just make the changes fastly.

[deepmap-marcinr]

I'm happy with this change, however, it breaks people's code when they re-generate after such an update.

I've been super busy with my main job and haven't had time to fix this properly, but my plan is to create a separate versioning dimension to allow us to fix such code generation issues, and allow people to pick the version of templates to use, eg, oapi-codegen --template-version=1.0.0. This decouples the version of the tool from the version of the generated code.

I'm still thinking about how to do it properly, because it can be a maintenance nightmare. I'd also like to make it easier to support more routers, like Gin

[robertscherbarth]

Hi, is this still on track?

Best Greetings