duct-framework / duct

Server-side application framework for Clojure
MIT License
1.13k stars 51 forks source link

Clarification wished for middleware (best practices) #31

Closed danielsz closed 8 years ago

danielsz commented 8 years ago

Hi James,

I haven't played yet with duct, but after reading watching the talk and reading the source, I am utterly seduced. Well done, @weavejester I have a remaining question regarding best practices: If a particular middleware makes only sense in a particular endpoint, should the user embed it in the endpoint, or should he pass it along with other middleware directly to the handler? What are the implications of both approaches?

Thanks! @danielsz

weavejester commented 8 years ago

Thanks for the praise!

I'll add a wiki page to clarify this, but in a nutshell the handler middleware is for the entire handler, and middleware only for the endpoint should be applied in the endpoint.

The reason the handler middleware is configurable is that it's often the case that handler middleware differs depending on the environment. For example, in development you might add middleware to display stacktraces, while in production you might have middleware that hides stacktraces and logs them.

I haven't come across any use-cases where it makes sense for the endpoint middleware to be configurable in the same way. At the very least I suspect it's a rare enough use-case to be handled manually, with some custom endpoint code.

danielsz commented 8 years ago

Thank you for clarifying this. This is very helpful.

yenda commented 8 years ago

The wiki page would be very welcome at the moment I have trouble figuring out how it works exactly compared to the simple functions that are used in a classic handler. I liked the spirit of duct the first time I tried it, I'm glad that it is integrated with system !

weavejester commented 8 years ago

I have trouble figuring out how it works exactly compared to the simple functions that are used in a classic handler.

Do you mean you have problems deciding when to use middleware on endpoints, or that you don't know how to use middleware on endpoints?

yenda commented 8 years ago

I put all my middleware in the handler for now, so my problem is mostly how to describe it in new-handler. Why does it change the way you define it in a vanilla ring app ?

I'm not sure I understand what this does :

(defn- middleware-fn [component middleware]
  (if (vector? middleware)
    (let [[f & keys] middleware
          arguments  (map #(get component %) keys)]
      #(apply f % arguments))
    middleware))

(defn- compose-middleware [{:keys [middleware] :as component}]
  (->> (reverse middleware)
       (map #(middleware-fn component %))
       (apply comp identity)))

I am wondering if the reverse in compose-middleware means that the last listed middleware is applied first.

Before adopting duct my middleware was declared like this :

(def app
  (-> app-routes
      (wrap-resource "public")
      auth
      (wrap-cors :access-control-allow-origin #".*"
                 :access-control-allow-methods [:get :put :post]
                 :access-control-allow-headers ["Content-Type"])
      wrap-restful-format
      (wrap-defaults my-defaults)))

I ended up writing my handler this way after following this page of the wiki https://github.com/weavejester/duct/wiki/Components .

(new-handler {:middleware [#(wrap-resource % "public")
                                        auth
                                        #(wrap-cors %
                                          :access-control-allow-origin #".*"
                                          :access-control-allow-methods [:get :put :post]
                                          :access-control-allow-headers ["Content-Type"])
                                        wrap-restful-format
                                        #(wrap-defaults % my-defaults)]})

With these 2 functions being common to both

(def my-defaults (-> site-defaults
                     (assoc-in [:static :resources] "/")
                     (assoc-in [:security :anti-forgery] false)))

(defn auth [app-routes]
  (friend/authenticate app-routes
                       {:credential-fn (partial creds/bcrypt-credential-fn users)
                        :workflows [(workflows/interactive-form)]}))

I was wondering about the necessity of this new 'syntax' to declare middleware because it gave me a hard time. Wouldn't it be simpler to just use a thread-first reader macro ? Or am I missing some of the benefits of the compose-middleware function ?

weavejester commented 8 years ago

I don't quite understand what you're having problems with. Maybe some more examples would make it clearer?

Normally you might apply middleware to a handler like so:

(-> handler
    (wrap-foo {:x 1})
    (wrap-bar {:y 2}))

The :middleware key works in a similar fashion:

(new-handler
 {:middleware
  [#(wrap-foo % {:x 1})
   #(wrap-bar % {:y 2})]})

Notice that we have a vector of middleware functions, and these are applied in the order they appear.

However, we can also pull the options out of the functions:

(new-handler
 {:middleware [[wrap-foo :foo]
               [wrap-bar :bar]]
  :foo {:x 1}
  :bar {:y 1}})

This means the same as the two previous examples. The keywords in the vectors are substituted for their corresponding values.

The advantage of this approach is that this allows the configuration to easily affect how the handler middleware behaves. For example, you might want to print stacktraces in development, but hide them in production.

Does that make things clearer? If so I'll update the wiki page with these examples.

yenda commented 8 years ago

I was confused by the use of the reverse function in compose-middleware regarding the order in which the middlewares are applied.

Your exemples could ease comprehension for newcomers it might indeed be a nice addition to the wiki page.

weavejester commented 8 years ago

The reverse is just because comp applies functions from last to first, whereas I want the middleware to be applied from first to last.

yenda commented 8 years ago

Thanks for the clarification I did not know about comp behavior

dgoodlad commented 8 years ago

I haven't come across any use-cases where it makes sense for the endpoint middleware to be configurable in the same way. At the very least I suspect it's a rare enough use-case to be handled manually, with some custom endpoint code.

I came across this issue while looking for a way to do exactly this: configure middleware on an endpoint. My use case is a webhook endpoint, which among other things needs to verify that a specific header value (an auth token) is present before processing the hook. That check, which needs to be configured with a valid-token value, seems to be a good use case for middleware. I don't want this check in place on other endpoints, and it doesn't make sense to have the middleware be aware of my application's routing in order to decide on its own when it should or shouldn't apply.

Can you think of a better approach that I haven't considered here?

:heart: for duct so far, though, it's exactly the lightweight set of defaults I was looking for!

weavejester commented 8 years ago

The way I'd solve this is to create a bare map component that acts as an additional set of configuration. So something like:

(defn new-system [config]
  (let [config (meta-merge base-config config)]
    (-> (component/system-map
         :app  (handler-component (:app config))
         :http (jetty-server (:http config))
         :foo  (endpoint-component foo-endpoint)
         :auth {:auth-token (:auth-token config)})
        (component/system-using
         {:http [:app]
          :app  [:foo]
          :foo  [:auth]}))))

Then in your endpoint, you get the configuration:

(defn foo-endpoint [{:keys [auth-token]} :auth}]
  (-> (routes ...)
      (wrap-auth {:token auth-token})))
dgoodlad commented 8 years ago

:star: that works very nicely, I hadn't realised that you could use a map as a component! Thanks @weavejester