Skip to content
Browse files

Store *noir-session* managed keys in :noir map inside :session. Maint…

…ain an emulation of a functionally-pure :session so as not to break the contract with other middleware that expects the ring requests/responses to be pure.
  • Loading branch information...
1 parent eb36d99 commit f4dc605df99828e68ea47f95b592ab99031459f7 @kenrestivo kenrestivo committed
Showing with 70 additions and 2 deletions.
  1. +15 −2 src/noir/session.clj
  2. +55 −0 test/noir/test/core.clj
View
17 src/noir/session.clj
@@ -50,11 +50,24 @@
cur)))
(defn noir-session [handler]
+ "Store noir session keys in a :noir map, because other middleware that
+ expects pure functions may delete keys, and simply merging won't work.
+ Ring takes (not (contains? response :session) to mean: don't update session.
+ Ring takes (nil? (:session resonse) to mean: delete the session.
+ Because noir-session mutates :session, it needs to duplicate ring/wrap-session
+ functionality to handle these cases."
(fn [request]
- (binding [*noir-session* (atom (:session request))]
+ (binding [*noir-session* (atom (get-in request [:session :noir] {}))]
(remove! :_flash)
(when-let [resp (handler request)]
- (assoc resp :session (merge @*noir-session* (:session resp)))))))
+ (if (= (get-in request [:session :noir] {}) @*noir-session*)
@ithayer
ithayer added a note

It's worth noting that this change causes some unintuitive behavior compared to previous behavior -- this means that sessions aren't updated, and therefore session expiration times can't be changed by the ring handler, unless a response explicitly modifies the session. (we just noticed this in production after an upgrade)

our work around is to explicitly assoc the session each time using another middleware.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ resp
+ (if (contains? resp :session)
+ (if (nil? (:session resp))
+ resp
+ (assoc-in resp [:session :noir] @*noir-session*))
+ (assoc resp :session (assoc (:session request) :noir @*noir-session*))))))))
+
(defn assoc-if [m k v]
(if (not (nil? v))
View
55 test/noir/test/core.clj
@@ -297,3 +297,58 @@
(deftest different-header
(-> (send-request "/different/status")
(has-status 201)))
+
+
+(deftest session
+ (with-noir
+ (session/put! :noir3 "woo")
+ (is (= "woo" (session/get :noir3)))
+ (is (nil? (session/get :noir4)))
+ (is (= "noir" (session/get :noir4 "noir")))))
+
+;;; regresssion tests, assure noir-session works with middleware like friend
+;;; which expects ring requests/responses to be pure functions
+
+(deftest noir-session
+ (let [base-map {:uri "/foo" :request-method :get }]
+ ;; put session value in
+ (is (= "bar" (get-in ((session/noir-session
+ #(assoc-in % [:session :foo] "bar"))
+ base-map)
+ [:session :foo])))
+ (let [base-map (assoc base-map :session {:foo "bar"})]
+ ;; pass session value through
+ (is (= "bar" (get-in ((session/noir-session identity)
+ base-map)
+ [:session :foo])))
+ ;; change session value
+ (is (= "baz" (get-in ((session/noir-session
+ #(assoc-in % [:session :foo] "baz"))
+ base-map)
+ [:session :foo])))
+ ;; dissoc session value
+ (is (not (contains? (:session
+ ((session/noir-session
+ #(assoc % :session (dissoc (:session %) :foo)))
+ base-map))
+ :foo))))
+ ;; dissocing one key doesn't affect any others
+ (let [base-map (assoc base-map :session {:foo "bar" :quuz "auugh"})
+ part-dissoc (:session
+ ((session/noir-session
+ #(assoc % :session (dissoc (:session %) :foo)))
+ base-map))]
+ (is (not (contains? part-dissoc :foo)))
+ (is (= "auugh" (:quuz part-dissoc)))
+ ;; changing one key doesn't affect any others
+ (let [part-change (:session
+ ((session/noir-session
+ #(assoc-in % [:session :foo] "baz"))
+ base-map))]
+ (is (= "baz" (:foo part-change)))
+ (is (= "auugh" (:quuz part-change)))))))
+
+
+
+
+

0 comments on commit f4dc605

Please sign in to comment.
Something went wrong with that request. Please try again.