`:interceptors` does not flatten vectors like `:middleware` -> cannot succinctly compose chains of interceptors

[Ramblurr]

Given:

(require
   '[reitit.http :as http]
   '[reitit.http.coercion :as coercion]
   '[reitit.http.interceptors.parameters :as parameters])

(def api-chain [parameters/parameters-interceptor
                coercion/coerce-response-interceptor
                coercion/coerce-request-interceptor])

(def my-interceptor
  {:enter (fn [ctx] ctx)})

(http/router ["/" {:interceptors [api-chain my-interceptor]}])

Result:

   Wrong number of args (2) passed to:
   reitit.http.interceptors.parameters/parameters-interceptor
   {:reitit.exception/cause #error {
    :cause "Wrong number of args (2) passed to: reitit.http.interceptors.parameters/parameters-interceptor"
    :via
    [{:type clojure.lang.ArityException
      :message "Wrong number of args (2) passed to: reitit.http.interceptors.parameters/parameters-interceptor"
      :at [clojure.lang.AFn throwArity "AFn.java" 429]}]
    :trace
    [[clojure.lang.AFn throwArity "AFn.java" 429]
     [clojure.lang.AFn invoke "AFn.java" 36]
     [clojure.lang.AFn applyToHelper "AFn.java" 156]
     [clojure.lang.AFn applyTo "AFn.java" 144]
     [clojure.core$apply invokeStatic "core.clj" 667]
     [clojure.core$apply invoke "core.clj" 662]
     [reitit.interceptor$eval46785$fn__46787 invoke "NO_SOURCE_FILE" 58]
     [reitit.interceptor$eval33607$fn__33608$G__33596__33617 invoke "interceptor.cljc" 7]
     [reitit.interceptor$chain$fn__34004 invoke "interceptor.cljc" 113]
...

Expected result:

I expect the resulting interceptors chain for "/" to be:

[parameters/parameters-interceptor
  coercion/coerce-response-interceptor
  coercion/coerce-request-interceptor
  my-interceptor]

Extra

@ikitommi indicated that this should work:

(slack link)

However, looking at the implementation, it cannot work, because the :interceptors vector in route data is processed one element at a time:

reitit/modules/reitit-core/src/reitit/interceptor.cljc

Lines 103 to 116 in 0f94148

	(defn chain
	"Creates a Interceptor chain out of sequence of IntoInterceptor
	Optionally takes route data and (Router) opts."
	([interceptors]
	(chain interceptors nil nil))
	([interceptors data]
	(chain interceptors data nil))
	([interceptors data {::keys [transform] :or {transform identity} :as opts}]
	(let [transform (if (vector? transform) (apply comp (reverse transform)) transform)]
	(->> interceptors
	(keep #(into-interceptor % data opts))
	(transform)
	(keep #(into-interceptor % data opts))
	(into [])))))

So it is not possible for the into-interceptor implementation for clojure.lang.APersistentVector to flatten the vector, instead it assumes you're doing a delayed function call: [some-interceptor-ctor :a-param :another-param] -> (some-interceptor-ctor :a-param :another-param) (I'm curious what the usecase for that is btw?)

Workarounds

We would like this feature, as defining a stack, or chain, of interceptors that can be re-used and composed is very useful.

Unfortunately there aren't any easy workarounds.

1. :reitit.interceptor/transform i.e., (http/router ["/" {:interceptors [api-chain my-interceptor]}] {:reitit.interceptor/transform custom-transform)

   Not possible. Because into-interceptor is run on the interceptors vector once before the transformation (see (defn chain..) above).

2. :compile i.e., (http/router ["/" {:interceptors [api-chain my-interceptor]}] {:compile custom-compile-fn)
   This is technically possible, but requires redefining:

   • reitit.http/compile-result
   • reitit.interceptor/compile-result
   • reitit.interceptor/chain

   ...oof 😨

3. One simple workaround of course is to:

   ["/" {:interceptors (conj api-chain my-interceptor) }]

   Or for composing multiple chains (also very common):

   ["/" {:interceptors (concat api-chain auth-chain [my-interceptor]) }]

   For experienced clojure developers the above two are of course perfectly serviceable, but a far cry from the readability of the following. When teaching new clojure devs about web dev, one of the first files they see is the routes file and the low-level noise of concat is distracting from higher level concepts.

   Ideally we could write:

   ["/" {:interceptors [api-chain auth-chain my-interceptor])}]
   ;; where api-chain and auth-chain are vectors of IntoInterceptors, and my-interceptor is an IntoInterceptor

While I'm wishing, it would be even nicer if the api-chain (aka vectors of IntoInterceptors) could be put in the registry, allowing:

["/" {:interceptors [:my.proj/api-chain :my.auth/auth-chain my-interceptor])}]

And I am pretty sure this would work out-of-the-box, if one IntoInterceptor could expand into multiple IntoInterceptors. Because registry is already defined as a map of keyword => IntoInterceptor.

[Ramblurr]

A solution?

Here's another possible workaround (and potentially a route to fixing this here in reitit proper? 🤞 )

;; Change how vectors are handled so that:
;;     (1) the current fn behavior  is preserved in the case that the first element is a function
;;     (2) otherwise assume the vector contents are more IntoInterceptors
(extend-protocol reitit.interceptor/IntoInterceptor
  clojure.lang.APersistentVector
  (into-interceptor [form data opts]
    (if (fn? (first form))
      (reitit.interceptor/into-interceptor (apply (first form) (rest form)) data opts)
      (mapv #(reitit.interceptor/into-interceptor % data opts) form))))

;; However that is not sufficient, because the above impl returns nested vectors
;; We need to flatten them with a transform:

(defn flatten-into-interceptors [interceptors]
  (remove nil? (flatten interceptors)))

(reitit.core/compiled-routes
 (http/router [""
               ;; remember, api-chain and auth-chain are vectors of IntoInterceptor
               ["/" {:interceptors [api-chain auth-chain my-interceptor]
                     :handler      dummy-handler}]
               ;; and we still support the original implementation of into-interceptor for vector
               ["/old" {:interceptors [[(fn [arg1 arg2]
                                          {:args  [arg1 arg2]
                                           :enter (fn [ctx] ctx)}) :arg1 :arg2]]
                        :handler      dummy-handler}]]

              {:reitit.interceptor/registry  ...optional registry...
               ;; REQUIRED: transform nested interceptors into a flat list
               :reitit.interceptor/transform flatten-into-interceptors}))

👉 If this approach is suitable, I would be willing to write up an impl, tests, docs, and PR it.

Notes on the current implementation of into-interceptor for a vector

reitit/modules/reitit-core/src/reitit/interceptor.cljc

Lines 51 to 58 in 0f94148

	#?(:clj clojure.lang.APersistentVector
	:cljs cljs.core.PersistentVector)
	(into-interceptor [[f & args :as form] data opts]
	(when (and (seq args) (not (fn? f)))
	(exception/fail!
	(str "Invalid Interceptor form: " form "")
	{:form form}))
	(into-interceptor (apply f args) data opts))

I think this implementation is actually broken on the edge cases. Obviously the intention was to be able to pass a vector where the head is a fn, and the rest was arguments to that function:

(defn make-interceptor [v1 v2]
  {:enter (fn [ctx]
            (assoc ctx :my/args [v1 v2]))})

["/" {:interceptors [[make-interceptor 1 2]]}]

However the (when (and (seq args) (not (fn? f))) guard does not make sense, because f is always treated as a function, so if one does:

["/" {:interceptors [[:wut]]}]

You are greeted with the runtime exception Wrong number of args (0) passed to: :wut.

..and if you do the somewhat plausible action of passing a keyword as the fn with a map to yoink from:

["/" {:interceptors [[:wut {:wut {:enter (fn [ctx] .....)}}]]}]

Then you get the builtin fail! from that impl: Invalid Interceptor form: ...

In any case, IMHO, there is plenty of room here to introduce new semantics for the vector variant of IntoInterceptor while maintaining backwards compatibility with the expectation to support [fn args...]

[ikitommi]

Hi. This doesn't work with middleware either:

(defn mw [handler k v]
  (fn [req] (handler (assoc req k v))))

;; flat list
((middleware/chain [[mw :a 1] [mw :b 1]] identity) {})
; => {:a 1, :b 1}

;; one mw in nested vector
((middleware/chain [[mw :a 1] [[mw :b 1]]] identity) {})
; => {:a 1, :b 1}

;; many mw in nested vector 💣 
((middleware/chain [[mw :a 1] [[mw :b 1] [mw :c 1]]] identity) {})
; => Wrong number of args (2) passed ...

The vector-syntax for middleware is coming from Duct, see the docs. Now thinking about this, I would rather see the flattening of nested vectors (and stripping out nils) instead of that. Removing support for duct-style vectors would be a major breaking change.

Ideas welcome on how to support both - for both middleware & intercetors.