Make duct/server.http.jetty dependency :excludable

[benalbrecht]

I swapped out jetty for immutant and tried to change my :dependencies entry to

[duct/module.web "0.2.0" :exclusions [duct/server.http.jetty]]

since I prefer to keep my dependency trees as clean as possible. 😀 This failed due to the require in duct.module.web The namespace containing the default (jetty) server seems to be necessary to pass the tests, so I changed it to be loaded only when no other :duct.server/http is defined.

I wonder why that is required, since duct seems to run load-namespaces once more after applying modules.

(defn prep
  "Prep a configuration, ready to be initiated. Key namespaces are loaded,
  and modules are applied."
  [config]
  (-> config
      (doto ig/load-namespaces)
      (apply-modules)
      (doto ig/load-namespaces)))

[weavejester]

Good catch! As you point out, require isn't necessary in this case, so we can just remove it without adding in an extra load-namespaces.

Also, can you change the commit message to:

Remove duct.server.http.jetty require

[benalbrecht]

Done.

Without the require, some values end up in :duct.server/http instead of :duct.server.http/jetty. I adapted the tests to represent that.

[benalbrecht]

this is weird:

~/git/module.web$ lein eftest

3/3   100% [==================================================]  ETA: 00:00

Ran 3 tests in 0.166 seconds
3 assertions, 0 failures, 0 errors.
~/git/module.web$ lein test

lein test duct.middleware.web-test

lein test duct.module.web-test

lein test :only duct.module.web-test/site-module-test

FAIL in (site-module-test) (web_test.clj:55)
expected: (= (core/prep site-config) (merge site-config {:duct.middleware.web/log-requests {:logger (ig/ref :duct/logger)}, :duct.core.web/handler {:endpoints [], :middleware [(ig/ref :duct.middleware.web/not-found) (ig/ref :duct.middleware.web/webjars) (ig/ref :duct.middleware.web/defaults) (ig/ref :duct.middleware.web/log-requests) (ig/ref :duct.middleware.web/log-errors) (ig/ref :duct.middleware.web/hide-errors)]}, :duct.middleware.web/defaults {:params {:urlencoded true, :multipart true, :nested true, :keywordize true}, :cookies true, :session {:flash true, :cookie-attrs {:http-only true}}, :security {:anti-forgery true, :xss-protection {:enable? true, :mode :block}, :frame-options :sameorigin, :content-type-options :nosniff}, :static {:resources ["duct/module/web/public" "foo/public"]}, :responses {:not-modified-responses true, :absolute-redirects true, :content-types true, :default-charset "utf-8"}}, :duct.server.http/jetty {:port 3000}, :duct.server/http {:handler (ig/ref :duct.core.web/handler), :logger (ig/ref :duct/logger)}, :duct.middleware.web/hide-errors {:response (io/resource "duct/module/web/errors/500.html")}, :duct.middleware.web/log-errors {:logger (ig/ref :duct/logger)}, :duct.middleware.web/webjars {}, :duct.middleware.web/not-found {:response (io/resource "duct/module/web/errors/404.html")}, :duct.middleware.web/stacktrace {}}))
  actual: (not (= {:duct.logger/fake {}, :duct.module.web/site {}, :duct.middleware.web/log-requests {:logger #integrant.core.Ref{:key :duct/logger}}, :duct.core.web/handler {:endpoints [], :middleware [#integrant.core.Ref{:key :duct.middleware.web/not-found} #integrant.core.Ref{:key :duct.middleware.web/webjars} #integrant.core.Ref{:key :duct.middleware.web/defaults} #integrant.core.Ref{:key :duct.middleware.web/log-requests} #integrant.core.Ref{:key :duct.middleware.web/log-errors} #integrant.core.Ref{:key :duct.middleware.web/hide-errors}]}, :duct.middleware.web/defaults {:params {:urlencoded true, :multipart true, :nested true, :keywordize true}, :cookies true, :session {:flash true, :cookie-attrs {:http-only true}}, :security {:anti-forgery true, :xss-protection {:enable? true, :mode :block}, :frame-options :sameorigin, :content-type-options :nosniff}, :static {:resources ["duct/module/web/public" "foo/public"]}, :responses {:not-modified-responses true, :absolute-redirects true, :content-types true, :default-charset "utf-8"}}, :duct.server.http/jetty {:port 3000, :handler #integrant.core.Ref{:key :duct.core.web/handler}, :logger #integrant.core.Ref{:key :duct/logger}}, :duct.middleware.web/hide-errors {:response #object[java.net.URL 0x518a7b8a "file:/home/ben/git/bim/module.web/resources/duct/module/web/errors/500.html"]}, :duct.middleware.web/log-errors {:logger #integrant.core.Ref{:key :duct/logger}}, :duct.middleware.web/webjars {}, :duct.middleware.web/not-found {:response #object[java.net.URL 0x5a24390b "file:/home/ben/git/bim/module.web/resources/duct/module/web/errors/404.html"]}, :duct.core/environment :production, :duct.middleware.web/stacktrace {}, :duct.core/project-ns foo} {:duct.logger/fake {}, :duct.module.web/site {}, :duct.middleware.web/log-requests {:logger #integrant.core.Ref{:key :duct/logger}}, :duct.core.web/handler {:endpoints [], :middleware [#integrant.core.Ref{:key :duct.middleware.web/not-found} #integrant.core.Ref{:key :duct.middleware.web/webjars} #integrant.core.Ref{:key :duct.middleware.web/defaults} #integrant.core.Ref{:key :duct.middleware.web/log-requests} #integrant.core.Ref{:key :duct.middleware.web/log-errors} #integrant.core.Ref{:key :duct.middleware.web/hide-errors}]}, :duct.middleware.web/defaults {:params {:urlencoded true, :multipart true, :nested true, :keywordize true}, :cookies true, :session {:flash true, :cookie-attrs {:http-only true}}, :security {:anti-forgery true, :xss-protection {:enable? true, :mode :block}, :frame-options :sameorigin, :content-type-options :nosniff}, :static {:resources ["duct/module/web/public" "foo/public"]}, :responses {:not-modified-responses true, :absolute-redirects true, :content-types true, :default-charset "utf-8"}}, :duct.server.http/jetty {:port 3000}, :duct.server/http {:handler #integrant.core.Ref{:key :duct.core.web/handler}, :logger #integrant.core.Ref{:key :duct/logger}}, :duct.middleware.web/hide-errors {:response #object[java.net.URL 0x7253c53 "file:/home/ben/git/bim/module.web/resources/duct/module/web/errors/500.html"]}, :duct.middleware.web/log-errors {:logger #integrant.core.Ref{:key :duct/logger}}, :duct.middleware.web/webjars {}, :duct.middleware.web/not-found {:response #object[java.net.URL 0x66d6e50b "file:/home/ben/git/bim/module.web/resources/duct/module/web/errors/404.html"]}, :duct.core/environment :production, :duct.middleware.web/stacktrace {}, :duct.core/project-ns foo}))

Ran 3 tests containing 3 assertions.
1 failures, 0 errors.
Tests failed.

In the first commit I pushed, i changed both tests, but the second one failed when using lein test. Maybe it passed eftest because eftest runs them in parallel?

[weavejester]

If the tests are failing, you shouldn't be changing the tests, but fixing the problem :)

As well as removing the additional require, we need to add a derive to tell the module that duct.server.http/jetty is derived from duct.server/http:

(derive :duct.server.http/jetty :duct.server/http)

Add that at the top of the file and put the tests back to normal and everything should work fine.

[benalbrecht]

ok, the missing derive was the problem, not the missing require. This is why i used load-namespaces in the first place - to make the tests pass. :)

The namespace containing the default (jetty) server seems to be necessary to pass the tests, so I changed it to be loaded only when no other :duct.server/http is defined.

[weavejester]

Thanks! I think this change has some unintuitive aspects to it, so perhaps we should add something to the commit message. Something like:

Remove duct.server.http.jetty require

Allows the module to be used with a different HTTP server key without
necessitating a Jetty dependency.

edit: Apologies for being difficult, but I can see myself forgetting why this require was removed in six months time!

[benalbrecht]

Done. No worries :)

[weavejester]

This change has been released in version 0.2.1.