Middlewares leak to next routes about compojure-api

[ikitommi]

ugh. we'll check this out.

[Deraen]

Ah yes. Middleware is executed before the handler checks if the route is correct.

1. Compojure executes GET* "/first" handler, it returns nil
2. Compojure executes GET* "/second" handler, it should return nil (but instead the middleware is executed first)
3. Should executes GET "/third" handler...

I merged the tests and I'll have a fix shortly.

[Deraen]

... And the fix is probably to remove :middlewares because it's impossible to implement this properly.

To apply middlewares after route is checked but before the handler is called would require changes in compojure.core generated code: https://github.com/weavejester/compojure/blob/1.1.8/src/compojure/core.clj#L99.

[phadej]

Couldn't middlewares be applied on the route body:

(GET* "/foo" []
  :middlewares [mw]
  (ok "foo"))

;;; =>
(GET "/foo" request
  (mw request))

or do I understand something wrong? (I though that middlewares is desugared into something like that)

[Deraen]

Middlewares don't take a request as param but a handler:

(defn mw [handler]
  (fn [req]
    (let [req (update-in req [:x] ...)
          res (handler req)]
      res)))

(GET* "/foo" req
  :body-params [x :- String]
  :middlewares [mw]
  (ok x)

=>
(GET "/foo" req
  ;; mock-handler
  ((mw
    (fn [_] (ok x)))
   req)   

And even while it's possible to create "a mock handler": (fn [_] ~@body) the edits to request won't be seen by coercers etc.

But if we forget the middleware semantics and create e.g. "routewares" we could implement what you are suggesting:

;; Can't edit the req
;; can call the route body conditionally
(defn rw [req next]
  (if (... req)
    (ok {:value "bar"})
    (next)))

(GET* "/foo" []
  :routewares [rw]
  (ok "foo"))

[phadej]

routewares are fine to me.

I found the bug while adding authentication middleware for just few selected routes (e.g. content modifying ones). Not sure what is the right way to do that kind of functionality.

EDIT though I want to do authentication check before parsing the request...

[Deraen]

I think I found a fix :)

[yogsototh]

I tried with the latest version it doesn't seem to work.

(defroute* my-routes
  (GET* "/route1" []
    :middle wares [admin-only]
    ... )

  (GET* "/route2" []
    :middle wares [user-only]
    ... ))

route2 isn't accessible for users :-(

[Deraen]

I haven't yet merged the fix to master branch.

[yogsototh]

Ah thanks! I'll try with the right version.

[yogsototh]

It works perfectly, thanks!