search

noir-clojure / lib-noir

A set of libraries for ring apps, including stateful sessions.
Eclipse Public License 1.0
483 stars 47 forks source link

Order of middleware in noir.util.middleware/app-handler #81

Closed keeds closed 10 years ago

keeds commented 10 years ago

Was wondering why the (wrap-middleware ...) function call was where it was in the chain of middleware calls in app-handler? I have a middleware function the requires request parameters to be in a map which is done by wrap-keyword-params which is called in the api middleware function. The chain is called bottom up, so I would have thought the best place for the (wrap-middleware ...) would be the first call after (apply routes ...)?

yogthos commented 10 years ago

The best way to add middleware is by supplying it to the app-handler using the :middleware key. As can be seen in the API docs.

The app-handler will stick the middleware in the appropriate place.

keeds commented 10 years ago

Dmitri, Thanks for the reply. I already do that, it just that my middleware gets called in the chain before the (api) call and thus the http request parameters have not been converted into a map and made available in the request map.

My question was would it be more appropriate for any user supplied middleware to be the last middleware to be called?

Thanks, Andrew

On Saturday, August 3, 2013, Dmitri Sotnikov wrote:

The best way to add middleware is by supplying it to the app-handlerusing the :middleware key. As can be seen in the API docshttp://yogthos.github.io/lib-noir/noir.util.middleware.html#var-app-handler .

The app-handler will stick the middleware in the appropriate placehttps://github.com/noir-clojure/lib-noir/blob/master/src/noir/util/middleware.clj#L171 .

— Reply to this email directly or view it on GitHubhttps://github.com/noir-clojure/lib-noir/issues/81#issuecomment-22042017 .

yogthos commented 10 years ago

Ah I see the problem, sorry I misread your original message. I had the user middleware resolved last originally but found that caused some issues. I don't recall what the actual problem was though as it's been a little while since I touched it.

I could take a look at retesting this to see what the issue was and if there's a workaround. It sounds like currently you'd have to basically make your own version of app-handler which isn't ideal.

yogthos commented 10 years ago

@keeds I got around to switching the order of middleware loading around so that custom middleware will be called last. It should address the issue. The new version 0.6.8 is up on Clojars, let me know if that addresses the issue.

keeds commented 10 years ago

I can confirm that this update fixes this issue. Many thanks, Andrew.