Fork me on GitHub
#reitit
<
2019-08-30
>
onetom10:08:09

if i have a router like this:

(reitit.ring/router
    ["" {:middleware
         [reitit.ring.middleware.muuntaja/format-middleware
          ring.middleware.cookies/wrap-cookies
          [ring.middleware.session/wrap-session
           {:store (ring.middleware.session.memory/memory-store sessions)}]]}
     ["/test" {:get test-route}]]
    {:reitit.middleware/transform print-request-diffs})
my output structure looks like this:
--- :request---
--- :request---
--- :request---
--- :response---
--- :response---
--- :response---
but it doesn't mention the middleware names, like on the screenshot in the docs: https://github.com/metosin/reitit/blob/master/doc/ring/transforming_middleware_chain.md

onetom10:08:25

i guess those middlewares must be defined as data with a name attribute and should be put into the middleware registry? but i was expecting to see at least reitit.ring.middleware.muuntaja/format-middleware appearing in the diffs

onetom11:08:58

Minimal example:

(-> ["" {:middleware [reitit.ring.middleware.muuntaja/format-middleware]}
     ["/test" {:get (constantly {:status 200 :body "ok"})}]]
    (reitit.ring/router
      {:reitit.middleware/transform print-request-diffs})
    (reitit.ring/ring-handler
      (reitit.ring/create-default-handler))
    (apply [{:request-method :get
             :uri "/test"}]))
it outputs only this:
--- :request---

  {:path-params {}, :request-method :get, :uri "/test"}

--- :response---

  {:body "ok", :status 200}

=> {:status 200, :body "ok"}
and doesn't mention the format-middleware

onetom14:08:46

I was just missing the :muuntaja muuntaja.core/instance from the router config:

(defn router []
  (reitit.ring/router
    ["" {:muuntaja muuntaja.core/instance
         :middleware
         [reitit.ring.middleware.muuntaja/format-middleware
...
now I can see the muuntaja/format-middleware being traced both during the request and the response processing phases:
--- :request---

  {:body #<java.io.ByteArrayInputStream@4ce95882>,
   :headers {"content-length" "11",
             "cookie" "ring-session=673143dd-c7dc-4230-a7ac-f761c0da9ffa",
             "host" "localhost:8080"},
   :path-params {},
   :query-string nil,
   :remote-addr "127.0.0.1",
   :request-method :post,
   :scheme :http,
   :server-name "localhost",
   :server-port 8080,
   :uri "/test",
   :aleph/keep-alive? true,
   :aleph/request-arrived 868329216079431}

--- :request :reitit.ring.middleware.muuntaja/format ---

  {:body #<java.io.ByteArrayInputStream@4ce95882>,
   :headers {"content-length" "11",
             "cookie" "ring-session=673143dd-c7dc-4230-a7ac-f761c0da9ffa",
             "host" "localhost:8080"},
   :path-params {},
   :query-string nil,
   :remote-addr "127.0.0.1",
   :request-method :post,
   :scheme :http,
   :server-name "localhost",
   :server-port 8080,
   :uri "/test",
   :aleph/keep-alive? true,
   :aleph/request-arrived 868329216079431,
   +:muuntaja/request nil,
   +:muuntaja/response #muuntaja.core.FormatAndCharset
   {:charset "utf-8", :format "application/json", :raw-format nil}}

--- :request---
...
  {:body {:response data}, :headers {}, :status 200}

--- :response :reitit.ring.middleware.muuntaja/format ---

  {:body {:response data}, :headers {}, :status 200}

--- :response---

  {:body -{:response data} +#<java.io.ByteArrayInputStream@a057914>,
   :headers {+"Content-Type" "application/json; charset=utf-8"},
   :status 200}

onetom14:08:45

I find the response traces a bit confusing though. None of the responses before or after the --- :response :reitit.ring.middleware.muuntaja/format --- divider are produced by this middleware, but instead the last - unnamed - response is the result of this transformation.

ikitommi14:08:42

the original :request is the thing entering the chain. when request enters the format-middleware, it adds the content negotiation results into the request.

ikitommi14:08:46

when your handler responds, in the :response phase the middleware uses the content negotiation results and changes the `:body into a stream and adds the negotiation headers into the response.

onetom14:08:52

which middleware does the last, unnamed, --- :response--- line represent? based on the output, it seem like the :reitit.ring.middleware.muuntaja/format is not directly doing anything to the response, just delegates the transformation to something else, which is not named

ikitommi14:08:36

I think the printing of respones is off-by-one. It now reports the response before the middleware is applied.

ikitommi14:08:18

PR welcome to fix that, it’s not very intuitive like that

onetom14:08:09

ok, thanks for the confirmation!

onetom14:08:17

i've also figured out how can i attach names to the regular ring middlewares:

{:muuntaja   muuntaja.core/instance
         :middleware [reitit.ring.middleware.muuntaja/format-middleware
                      {:name :ring.middleware.cookies/wrap-cookies
                       :wrap ring.middleware.cookies/wrap-cookies}
                      {:name :ring.middleware.session/wrap-session
                       :wrap #(ring.middleware.session/wrap-session
                                % {:store (ring.middleware.session.memory/memory-store sessions)})}]}
it's a really amazing solution! thank you and whoever else worked on it!

onetom14:08:16

shouldn't the name of these middlewares follow their function name though?

ikitommi14:08:17

Good idea to derive the names from functions!

ikitommi14:08:31

(clojure.lang.Compiler/demunge (str identical?))
=> "clojure.core/identical?@609674d"

onetom14:08:03

like https://github.com/metosin/reitit/blob/master/modules/reitit-middleware/src/reitit/ring/middleware/muuntaja.clj#L27

:name ::format
could be
:name ::format-middleware
just to be consistent. I see there is a repetition of the word middleware in the namespaced keyword, but maybe that's more of an indication that the function shouldn't contain -middleware

ikitommi14:08:09

helper on top of that? could strip the end too

onetom14:08:45

yeah, that demunged str of the middleware function should be the default. i wouldn't even bother cleaning it further

onetom14:08:13

it could actually be useful to see if a different instance of a middleware function has been used in different traces, so it's better to leave the address of the function in the name

ikitommi14:08:52

need to think the naming of default middleware -middleware or not. But the default mapping from functions could do the basic demunging.

ikitommi15:08:02

would you like to do a PR of that?

onetom15:08:36

ok, i will have a look. it's probably simpler than the off-by-one issue

onetom15:08:21

have a great weekend and thanks for the quick feedback!

ikitommi15:08:50

👍 have a great weekend too!

ikitommi14:08:10

based on the output you pasted.

ikitommi14:08:57

@kenny the CORS interceptor looks good and the model of pushing the CORS data into route data is good. ring-cors is not optimized for perf, but I think it’s a good idea to add that now and maybe make it faster later. So, PR most welcome!

4
kenny14:08:43

Will do that today 🙂