Fork me on GitHub
#yada
<
2017-04-13
>
borkdude10:04:01

@malcolmsparks I looked into extending Yada with those namespaced keywords on the resource. It would mean adding an extra arg or args to to-body and render-map + render-seq. E.g.:

(defprotocol MessageBody
  (to-body [resource representation & [opts]] "Construct the reponse body for the given resource, given the negotiated representation (metadata)")
  ...)

(defmulti render-map (fn [resource representation & [opts]] (-> representation :media-type :name)))

(defmethod render-map "application/json"
  [m representation & [opts]
  (let [escape-non-ascii (get opts :yada.cheshire/escape-non-ascii)
         pretty (get-in representation [:media-type :parameters "pretty"])]
    (str (json/encode m {:pretty pretty :escape-non-ascii escape-non-ascii}) \newline)))

(defn create-response
  [ctx]
  (let [method (:method ctx)
        produces (get-in ctx [:response :produces])
        body (body/to-body (get-in ctx [:response :body]) produces (select-keys (:resource ctx) [:yada.cheshire/escape-non-ascii])
...
How do you feel about this solution?

borkdude10:04:58

We could also pass all namespaced keys to the opts in create-response to be less specific

borkdude10:04:32

And possibly doing this with other defmultis as well, but we can refactor this on a by need basis

dominicm10:04:45

I don't understand. Why can't you get the namespaced keyword from the resource / first param?

borkdude10:04:04

@dominicm Because that isn’t the Yada resource, but some object like a String or map

borkdude10:04:24

if I understand correctly

dominicm10:04:32

I was thinking more about the to-body MessageBody protocol.

borkdude10:04:42

That’s what I mean too

dominicm10:04:03

What's this representation being passed around too?

borkdude10:04:37

Something with e.g. “application/json” in it and some other stuff

dominicm11:04:14

oh, so that pretty parameter is supplied by the api consumer, not onto the resource or anything like that.

dominicm11:04:17

Worth noting that yada doesn't need to be configurable here. copy/paste programming is OK, and you can override the method with your own version, which knows the opts ahead of time.

dominicm11:04:11

dynamic vars are intended for ad-hoc options (it's why elisp defaults to dynamic scoping) so maybe that's preferred? Just thinking out loud here.

borkdude11:04:58

I don’t like dynamic vars that much, don’t know how they behave with Manifold etc. But yeah, overriding the methods is definitely an option.

dominicm11:04:30

Maybe to be more extensible, yada could separate implementations of protocols/multimethods into their own functions, and let them have their own rules of what they support (pass through chesire options & so on). That would make overriding it be as simple as:

(defmethod render-map "application/json"  [m representation]
  (yada.impl.render-map/cheshire m representation {:cheshire-opts {:ascii-thing true}})

borkdude11:04:59

I now have this, which is a pretty thin copy/paste operation:

(defmethod render-map "application/json"
  [m representation]
  (let [pretty (get-in representation [:media-type :parameters "pretty"])]
    (str (json/encode m {:pretty pretty
                         :escape-non-ascii true})
         \newline)))

borkdude11:04:33

but wouldn’t this just be moving around the problem to another location?

dominicm11:04:28

how do you mean?

borkdude11:04:17

Well, I guess the impl signature could change as well, but it’s slightly better

dominicm11:04:04

You can have contracts that make sense with each impl. If it makes sense to have 5 params, you can.

borkdude11:04:34

Hmm, extending the protocol with one extra option also brings some nastiness with it:

(defprotocol Boo
    (boo [a] [a b]))

  (extend-protocol Boo
    String
    (boo [a] {:a a})
    (boo [a b] (boo a)))

  (boo "foo") ;;=> doesn't work

dominicm11:04:55

I thought it may not.

borkdude11:04:58

varargs also doesn’t work for protocols

borkdude11:04:17

and you can’t provide a default implementation for the other arity

borkdude14:04:08

Does Yada support Swagger tags like Compojure-api does, so you can group endpoints?

borkdude14:04:24

I only see ‘Default’ as of now, Edge doesn’t document this

borkdude14:04:11

Ah namespaced keyword swagger/tags does it. I’ll add this to the manual.