middleware metadata

[yogthos]

The handler should have metadata containing the information about all the middleware functions. Each middleware function should be responsible for adding itself to the list when it loads.

This will allow middleware to load selectively based on what middleware is already loaded. Main use case for this is to facilitate libraries that provide their own middleware stacks. Such libraries could check what middleware is already loaded and choose what to add based on that.

[cjbarre]

If there is a central list of middleware metadata, then I am thinking each middleware handler should be required to supply a standard set of information to participate. Is that along the lines of what you are considering?

What form will the metadata take? Maybe a middleware protocol which has an info and handle function? Any thoughts on what kind of information middleware should suppy?

I don't know much about the project, at this point, I was just passing through and this issue reminded me of something I am working on!

[yogthos]

I'm thinking of using a map to describe each middleware function. The handler metadata would look something like this:

{:middleware
 {:wrap-params
   {:version  "1.0"
    :requires {}}
  :wrap-keyword-params
   {:version  "1.0"
    :requires {:wrap-params "1.0"}}}

A function could check the following things using the metadata:

• required middleware hasn't been loaded
• same middleware is already loaded
• conflicting middleware is loaded
• wrong version of required middleware is loaded

Middleware functions could also add their own arbitrary keys as needed, so this would be open to extension. The main goal is to have middleware provide good errors at load time.

[cjbarre]

Nice, I am dealing with nearly the same issue. The main difference is that you are dealing with middleware and I am dealing with tasks. We ended up deciding to try out a Task protocol which has two functions:

(defprotocol Task
  (info [this] "Provide metadata for this task."
  (execute [this r] "Execute this task on some request.")

It's been a pain deciding on how the tasks should be registered to the central list. Each task needs to have a dependency on the central list, or some other function needs to be able to find our tasks dynamically and register them.

I think most of our tasks metadata maps will only have required fields, but as you said, there is nothing stopping them from adding arbitrary keys inside during the info function call.

I am curious about your thoughts on the mechanism for getting middleware metadata to the list.

[yogthos]

Yeah, that sounds like a similar problem. My idea was to do something like the following:

(defn update-middleware-meta [handler middleware-meta]
  (with-meta
    handler
    {:macchiato/middleware
     (conj (-> handler meta :macchiato/middleware) middleware-meta)}))

(defn validate
  "middleware metadata can contain the following keys
  :name - name of the middleware function
  :required - a collection of the names of middleware functions it requires to be present
  :validator - custom validator, must return original metadata on success, throw on error"
  [handler-middleware
   {:keys [name required validator] :as middleware-meta}]
  (println "validator is a fn?" (type validator))
  (cond
    validator
    (validator handler-middleware middleware-meta)

    (not-empty (difference (set required) (set (map :name handler-middleware))))
    (throw (js/Error. (str name " is missing required middleware: " required)))

    :default middleware-meta))

(defn loaded? [middleware {:keys [name]}]
  (some #{name} (map :name middleware)))

(defn wrap-middleware [handler & middleware]
  (reduce
    (fn [handler middleware-fn]
      (let [handler-middleware (-> handler meta :macchiato/middleware)
            middleware-meta    (-> middleware-fn meta :macchiato/middleware)]
        (println "\n\nhandler meta:" handler-middleware
                 "\nmw meta:" middleware-meta)
        (if (loaded? handler-middleware middleware-meta)
          handler
          (->> (validate handler-middleware middleware-meta)
               (update-middleware-meta handler)
               middleware-fn))))
    (if (-> handler :macchiato/middleware)
      handler
      (with-meta handler {:macchiato/middleware []}))
    middleware))

The wrap-middleware function will go through the middleware and apply each one in turn. If the middleware has a :required key, then it checks that required middleware is present. If it has a :validator key, then it's used to validate the state instead.

[yogthos]

And looks like you can't put a function in the metadata, so the validator approach wouldn't work.

[yogthos]

So this is what I'm thinking of so far, and you can see how it would be used here.

[cjbarre]

I see that you are thinking about using the built in metadata facilities. I don't think I have used it before, but I didn't know you couldn't include a function :slightly_frowning_face:.

The wrap-middleware function will go through the middleware and apply each one in turn. If the middleware has a :required key, then it checks that required middleware is present. If it has a :validator key, then it's used to validate the state instead.

The validator idea is awesome, I plan to do the same with my task system in order to allow some default parameter validation, as well as give the task maker the option to supply a validation strategy.

What are your thoughts on using a protocol to formalize middleware a bit and moving (at least a little) away from the builtin metadata?

Here is a little example of what I was thinking:

(defprotocol Middleware
  "If you want to create middleware, look no further!"
  (metadata [_] "Get a map of metadata regarding this middleware.")
  (validate [_ registry] "Validate system state with respect to this middleware.")
  (handler [_] "Get the handler function associated with this middleware."))

(def wrap-hello-world
  (reify
    Middleware
    (metadata [_] {:id "ABCD-EFG-HIJKL-MNOP"
                   :name "wrap-hello-world"
                   :description "Appends hello world to all maps."
                   :required []})

    (validate [_ registry] (if (-> registry :middleware :wrap-goodbye-world) 
                             false 
                             true))

    (handler [_] (fn [handler]
                   (fn [request]
                     (handler (assoc request :hello "world")))))))

(metadata wrap-hello-world)
;; => {:id "ABCD-EFG-HIJKL-MNOP", :name "wrap-hello-world", :description "Appends hello world to all maps.", :required []}

(validate wrap-hello-world {:middleware {:wrap-goodbye-world {}}})
;; => false

(handler wrap-hello-world)
;; => #function[breezy-summer/reify--9952$fn--9953]

(let [wrap-hello-world-fn (handler wrap-hello-world)
      handler (-> identity 
                  wrap-hello-world-fn)]
  (handler {}))
;; => {:hello "world"}

It's certainly not a perfect solution, but maybe it'll give you some ideas regarding your middleware machinery. The protocol could be a nice hook into the framework for developers.

[ikitommi]

Making middleware pipeline less opaque is a good thing. Same for the automatic dependency injection. Not sure how the component versioning, haven't seen use for that in real projects.

About data, metadata & protocols. I did a journey from middleware with meta-data to data-driven interceptors with Kekkonen. Some thing I learned:

• meta-data is ok, but works only locally and makes inspection harder (e.g. doesn't show up when printing things)

• protocols (and records) are good for implementation, but there should be an easier way to define things on top. OM vs Reagent is good example of these two.

• data-driven syntax wins here (what @yogthos is proposing), Duct & Integrant are good examples of this => read & write from EDN, transfer over the wire as-is etc.

As a side-note - the data-driven Middleware start to look Interceptors. I recommend reading the Pedestal source code and checking utility libs like angel-interceptor.

I'll write a separate issue about the runtime input & output validation. Should be pluggable for handlers & middleware. We could spin off modules for both Schema & Spec (& Swagger).

[yogthos]

All great points, one of the things I'd like to do is to keep Ring compatibility. This is the main reason I went with the metadata approach. I agree that describing the relationships via EDN has some benefits as well.

I think the main goal is to provide an explicit view of the middleware pipeline. The main issue I've run into with middleware is that it can be difficult to tell what's loaded where and why. I think metadata should address that problem fairly cleanly.

This also allows for chaining groups of middleware. For example, this works:

(defn
  ^{:macchiato/middleware
    {:id       ::foo
     :required [::bar ::baz]}}
  foo [handler] handler)

(defn
  ^{:macchiato/middleware
    {:id ::bar}}
  bar [handler] (bar handler {}))

(defn
  ^{:macchiato/middleware
    {:id       ::baz
     :required [::bar]}}
  baz [handler] handler)

(= 
        (-> (fn [])
          (m/wrap-middleware
            #'baz
            #'bar)
          (m/wrap-middleware
            #'foo
            #'baz
            [#'bar {}]
            #'bar)
          (meta))
        {:macchiato/middleware
         [{:id :macchiato.test.middleware.middleware-meta/foo
           :required [:macchiato.test.middleware.middleware-meta/bar
                      :macchiato.test.middleware.middleware-meta/baz]}
          {:id :macchiato.test.middleware.middleware-meta/baz
           :required [:macchiato.test.middleware.middleware-meta/bar]}
          {:id :macchiato.test.middleware.middleware-meta/bar}]})

So, you could wrap core middleware with something like macchiato-defaults. This will return a handler with the metadata of what's already been wrapped, and it can be passed to a library that has its own middleware stack. The library can now wrap it intelligently without duplicating already existing middleware.

I'm also thinking that the :id key in the metadata should likely represent a type of middleware. For example, if two libraries handle RESTful params, then both of them could register as :wrap-restful-params or something.

[yogthos]

I've implemented the metadata proposal, so gonna close this with https://github.com/macchiato-framework/macchiato-core/commit/76c9cf0e9f1bc6877c867eebb6a70b3f660b9fc5

Could add new issues later regarding extending the metadata and adding a spec for it.