[WIP] Plain Context API (no request/response)

[martinklepsch]

This tries to address some of the requests (#15 & #28) for a more context focused API (i.e. without the request/response concepts).

execute-ctx still returns only the :response value but that's probably something we can figure out.

The main goal here is that we can move forward with a Context-based API while maintaining the previous request/response style for backwards compatibility.

cc @dspiteself @jjttjj @den1k

[den1k]

@martinklepsch looks good, thanks for doing this!

About execute-ctx returning :response. It looks like only await-result and deliver-result care about the :response. Could they maybe be passed a result-getter function instead that is identity for execute-ctx and :response for execute? It's less DRY but avoids leaking :response into the execute-ctx API.

[den1k]

Here it is sketched out (untested)

#?(:clj
   (defn- await-result [ctx]
     (if (a/async? ctx)
       (recur (a/await ctx))
       (if-let [error (:error ctx)]
         (throw error)
         ctx))))  ; <---

(defn- deliver-result [ctx get-result]  ; <---
  (if (a/async? ctx)
    (a/continue ctx deliver-result)
    (let [error    (:error ctx)
          result   (or error (get-result ctx))  ; <---
          callback (if error :on-error :on-complete)
          f        (get ctx callback identity)]
      (f result))))


(defn execute-ctx
  {:arglists '([interceptors initial-context]
               [interceptors initial-context on-complete on-error])}
  ([interceptors initial-context on-complete on-error]
   (execute-ctx interceptors initial-context on-complete on-error identity))  ; <---
  ([interceptors initial-context on-complete on-error get-result]
   (if-let [queue (q/into-queue interceptors)]
     (-> (context initial-context queue on-complete on-error)
         (enter)
         (leave)
         (deliver-result get-result))  ; <---
     ;; It is always necessary to call on-complete or the computation would not
     ;; keep going.
     (on-complete nil))
   nil)
  #?(:clj
     ([interceptors initial-context]
      (when-let [queue (q/into-queue interceptors)]
        (-> (context initial-context queue)
            (enter)
            (leave)
            (await-result))))))

(defn execute
  ([interceptors request on-complete on-error]
   (execute-ctx interceptors {:request request} on-complete on-error :response)) ; <---
  #?(:clj
     ([interceptors request]
      (some-> (execute-ctx interceptors {:request request})
              :response)))) ; <---

[martinklepsch]

I ended up just adding execute-ctx which duplicates most of the code of execute.

@den1k I've added you as a collaborator to my fork, please feel free to help with updating/expanding the test suite and testing in general. I'd love to get this into a place where it could be considered for inclusion in Sieppari itself but also only have limited time to hack on this...

[den1k]

@martinklepsch I'm in the same boat time-wise at least until the end of this week. However, it looks like you already made changes to src + tests. Is this ready for review?

[ikitommi]

Looking great! merge is kinda illegal for any performance critical code, instead, we could use two different Record impls, one for Request/Response things, another for the raw impl.

[ikitommi]

merge will sadly kill all performance.

[ikitommi]

Tests don't pass.

[martinklepsch]

closing in favor of #31