ensure sessions are deleted when empty (#22)

When a session is empty it should be removed entirely.

Previously this was not the case as the modified flag cannot be set when
the underlying session data is empty. This makes sense because an empty
session should not be updated but instead deleted.

However, this also meant that removing the last key of a previously
stored session would not reflect this update in the store--the session
would not be saved to the store or deleted.

To address this, we now look for an empty session after resolving the
request. When found, we delete the session from the store and return the
response.

Fixes #21.

src/service.rs

@@ -66,8 +66,10 @@ where
                 .cloned()
                 .expect("Something has gone wrong with tower-cookies.");
 
-            let session_cookie = cookies.get(&cookie_config.name).map(Cookie::into_owned);
-            let mut session = if let Some(session_cookie) = session_cookie {
+            let mut session_loaded_from_store = false;
+            let mut session = if let Some(session_cookie) =
+                cookies.get(&cookie_config.name).map(Cookie::into_owned)
+            {
                 // We do have a session cookie, so let's see if our store has the associated
                 // session.
                 //
@@ -78,6 +80,8 @@ where
                 // If the store does not know about this session, we should remove the cookie.
                 if session.is_none() {
                     cookies.remove(session_cookie);
+                } else {
+                    session_loaded_from_store = true;
                 }
 
                 session
@@ -118,6 +122,14 @@ where
                 }
             };
 
+            // In order to ensure removing the last value of a session updates the store, we
+            // check for an empty session. Empty sessions should be removed from the store.
+            if session_loaded_from_store && session.is_empty() {
+                session_store.delete(&session.id()).await?;
+                cookies.remove(cookie_config.build_cookie(&session));
+                return res;
+            }
+
             // For further consideration:
             //
             // We only persist the session in the store when the `modified` flag is set.

src/session.rs

@@ -495,7 +495,7 @@ impl Session {
     /// ```
     pub fn is_empty(&self) -> bool {
         let inner = self.inner.lock();
-        inner.expiration_time.is_none() && inner.data.is_empty()
+        inner.data.is_empty()
     }
 }
 

tests/common/mod.rs

@@ -21,12 +21,22 @@ fn routes() -> Router {
                 format!("{}", session.get::<usize>("foo").unwrap().unwrap())
             }),
         )
+        .route(
+            "/get_value",
+            get(|session: Session| async move { format!("{:?}", session.get_value("foo")) }),
+        )
         .route(
             "/remove",
             get(|session: Session| async move {
                 session.remove::<usize>("foo").unwrap();
             }),
         )
+        .route(
+            "/remove_value",
+            get(|session: Session| async move {
+                session.remove_value("foo");
+            }),
+        )
         .route(
             "/cycle_id",
             get(|session: Session| async move {
@@ -200,6 +210,47 @@ macro_rules! route_tests {
             assert_eq!(body_string(res.into_body()).await, "42");
         }
 
+        #[tokio::test]
+        async fn get_no_value() {
+            let app = $create_app(Some(Duration::hours(1))).await;
+
+            let req = Request::builder()
+                .uri("/get_value")
+                .body(Body::empty())
+                .unwrap();
+            let res = app.oneshot(req).await.unwrap();
+
+            assert_eq!(body_string(res.into_body()).await, "None");
+        }
+
+        #[tokio::test]
+        async fn remove_last_value() {
+            let app = $create_app(Some(Duration::hours(1))).await;
+
+            let req = Request::builder()
+                .uri("/insert")
+                .body(Body::empty())
+                .unwrap();
+            let res = app.clone().oneshot(req).await.unwrap();
+            let session_cookie = get_session_cookie(res.headers()).unwrap();
+
+            let req = Request::builder()
+                .uri("/remove_value")
+                .header(header::COOKIE, session_cookie.encoded().to_string())
+                .body(Body::empty())
+                .unwrap();
+            app.clone().oneshot(req).await.unwrap();
+
+            let req = Request::builder()
+                .uri("/get_value")
+                .header(header::COOKIE, session_cookie.encoded().to_string())
+                .body(Body::empty())
+                .unwrap();
+            let res = app.oneshot(req).await.unwrap();
+
+            assert_eq!(body_string(res.into_body()).await, "None");
+        }
+
         #[tokio::test]
         async fn cycle_session_id() {
             let app = $create_app(Some(Duration::hours(1))).await;