oliyh / martian

The HTTP abstraction library for Clojure/script, supporting OpenAPI, Swagger, Schema, re-frame and more
MIT License
526 stars 45 forks source link

Consider not using `clojure.walk` to `keywordize-keys` #62

Closed andrewmcveigh closed 5 years ago

andrewmcveigh commented 5 years ago

We're trying to use a custom de/serializer that knows how to de/serialize Records and lists of Records. However, if I pass these Records in the params arg of response-for, the call to keywordize-keys recursively transforms any Record into a map, which the de/serializer doesn't recognize.

Arguably this issue is in clojure.walk/keywordize-keys, but perhaps walking and transforming the full nested data structure is unnecessary?

https://github.com/oliyh/martian/blob/master/core/src/martian/core.cljc#L59 https://github.com/oliyh/martian/blob/master/core/src/martian/core.cljc#L66 https://github.com/oliyh/martian/blob/master/core/src/martian/core.cljc#L79

andrewmcveigh commented 5 years ago

Hi. I'm moving my post over here because I'm still working on that branch.

Hi @oliyh. No probs, thanks for getting back to me.

I actually didn't mean to make this PR on your repo (at least not yet). I assumed I was making it against our fork.

Anyway...

I'm using the below interceptor chain (more-or-less), replacing martian's default encode-body and coerce-response interceptors with an encoder map that knows about these content-types that we want to en/decode.

(let [encoder (fn [content-type]
                {:encode (partial encode content-type)
                 :decode (partial decode content-type)
                 :as :stream})
      encoders (assoc (encoders/default-encoders)
                      "application/n-quads" (encoder "application/n-quads")
                      "application/n-triples" (encoder "application/n-triples"))
      interceptors (conj martian/default-interceptors
                         (interceptors/encode-body encoders)
                         (interceptors/coerce-response encoders)
                         martian-http/perform-request)]
  (martian-http/bootstrap-swagger "http://uri" {:interceptors interceptors}))

Sure, I could just encode to a binary stream before even calling martian/response-for (if that's what you mean?), but that doesn't seem like a very nice or flexible workaround.

I don't think putting these interceptors earlier in the stack would have any effect. The call to keywordize-keys on params happens before any of the interceptor stuff kicks in.

oliyh commented 5 years ago

Ah, I see what you mean, this happens before all the interceptors. Perhaps it could be done in a new first interceptor instead, which would then make it easier for you to replace with your own without the need for forking or redeffing or anything nasty?

andrewmcveigh commented 5 years ago

Yes, making it removable/replaceable would be preferable to the other options.

andrewmcveigh commented 5 years ago

Had a go at implementing this. Is that the kind of thing you had in mind? #65