pass session data to storage provider when deleting

this helps with keeping session index up to date

Author: fa-sharp

src/fairing.rs

@@ -35,20 +35,21 @@ struct MySession {
 #[rocket::launch]
 fn rocket() -> _ {
     // Use default settings with in-memory storage
-    let session_fairing = RocketFlexSession::<MySession>::default();
+    let default_fairing = RocketFlexSession::<MySession>::default();
 
     // Or customize settings with the builder
-    let custom_session = RocketFlexSession::<MySession>::builder()
-        .storage(CookieStorage::default()) // or a custom storage provider
+    let my_fairing = RocketFlexSession::<MySession>::builder()
+        .storage(CookieStorage::default())
         .with_options(|opt| {
             opt.cookie_name = "my_cookie".to_string();
             opt.path = "/app".to_string();
-            opt.max_age = 7 * 24 * 60 * 60; // 7 days
+            opt.max_age = 7 * 24 * 60 * 60;
         })
         .build();
 
     rocket::build()
-        .attach(session_fairing)
+        .attach(default_fairing)
+        .attach(my_fairing)
         // ... other configuration ...
 }
 ```
@@ -131,19 +132,19 @@ where
         let (updated, deleted) = session_inner.lock().unwrap().take_for_storage();
 
         // Handle deleted session
-        if let Some(deleted_id) = deleted {
-            rocket::debug!("Found deleted session. Deleting session '{deleted_id}'...");
-            if let Err(e) = self.storage.delete(&deleted_id, req.cookies()).await {
-                rocket::warn!("Error while deleting session '{deleted_id}': {e}");
+        if let Some((id, data)) = deleted {
+            rocket::debug!("Found deleted session. Deleting session '{id}'...");
+            if let Err(e) = self.storage.delete(&id, data).await {
+                rocket::warn!("Error while deleting session '{id}': {e}");
             } else {
-                rocket::debug!("Deleted session '{deleted_id}' successfully");
+                rocket::debug!("Deleted session '{id}' successfully");
             }
         }
 
         // Handle updated session
-        if let Some((id, pending_data, ttl)) = updated {
+        if let Some((id, data, ttl)) = updated {
             rocket::debug!("Found updated session. Saving session '{id}'...");
-            if let Err(e) = self.storage.save(&id, pending_data, ttl).await {
+            if let Err(e) = self.storage.save(&id, data, ttl).await {
                 rocket::error!("Error while saving session '{id}': {e}");
             } else {
                 rocket::debug!("Saved session '{id}' successfully");

src/session_inner.rs

@@ -7,16 +7,16 @@ use crate::SessionIdentifier;
 pub(crate) struct SessionInner<T> {
     /// The current, active session
     current: Option<ActiveSession<T>>,
-    /// The ID of the original session if deleted during the request
-    deleted: Option<String>,
+    /// The original session if deleted during the request
+    deleted: Option<ActiveSession<T>>,
 }
 impl<T> Default for SessionInner<T> {
     fn default() -> Self {
         Self::new_empty()
     }
 }
 
-/// Represents a current, active session
+/// Represents an active session
 #[derive(Debug)]
 struct ActiveSession<T> {
     /// Session ID (20-character alphanumeric string)
@@ -156,28 +156,28 @@ impl<T> SessionInner<T> {
         }
     }
 
-    /// Mark the current session ID as deleted, and clear all data. Can safely be called
+    /// Mark the current session as deleted, and clear all data. Can safely be called
     /// multiple times in a request if needed - the original session will still be deleted.
     pub(crate) fn delete(&mut self) {
         if let Some(current) = self.current.take() {
-            self.deleted.get_or_insert(current.id);
+            self.deleted.get_or_insert(current);
         }
     }
 
     pub(crate) fn get_deleted_id(&self) -> Option<&str> {
-        self.deleted.as_deref()
+        self.deleted.as_ref().map(|s| s.id.as_str())
     }
 
     /// Get all data for storage if the session needs to be saved/updated. Returns a tuple of Options
-    /// representing an updated session along with the id of a deleted session. This should only be
-    /// called once at the end of the request, as it takes ownership of the session data.
-    pub(crate) fn take_for_storage(&mut self) -> (Option<(String, T, u32)>, Option<String>) {
+    /// representing an updated session along with a deleted session. This should only be
+    /// called once at the end of the request, as it takes ownership of all data.
+    pub(crate) fn take_for_storage(&mut self) -> (Option<(String, T, u32)>, Option<(String, T)>) {
         let updated_session = self
             .current
             .take()
             .filter(|c| should_save_session(&c.status))
             .map(|c| (c.id, c.data, c.ttl));
-        (updated_session, self.deleted.take())
+        (updated_session, self.deleted.take().map(|s| (s.id, s.data)))
     }
 }
 

src/storage/cookie.rs

@@ -172,7 +172,7 @@ where
         Ok(()) // no-op (cookie session should already be saved by `save_cookie`)
     }
 
-    async fn delete(&self, _id: &str, _cookie_jar: &CookieJar) -> SessionResult<()> {
+    async fn delete(&self, _id: &str, _data: T) -> SessionResult<()> {
         Ok(()) // no-op (cookie session should already be deleted by `save_cookie`)
     }
 }

src/storage/interface.rs

@@ -25,7 +25,7 @@ where
     async fn save(&self, id: &str, data: T, ttl: u32) -> SessionResult<()>;
 
     /// Delete a session in storage. This will be performed at the end of the request lifecycle.
-    async fn delete(&self, id: &str, cookie_jar: &CookieJar) -> SessionResult<()>;
+    async fn delete(&self, id: &str, data: T) -> SessionResult<()>;
 
     /// Optional callback when there's a pending change to the session data. A `data` value
     /// of `None` indicates a deleted session. This callback can be used by cookie-based

src/storage/memory.rs

@@ -73,7 +73,7 @@ where
         Ok(())
     }
 
-    async fn delete(&self, id: &str, _cookie_jar: &CookieJar) -> SessionResult<()> {
+    async fn delete(&self, id: &str, _data: T) -> SessionResult<()> {
         self.cache.remove(&id.to_owned()).await;
         Ok(())
     }
@@ -211,14 +211,9 @@ where
         self.base_storage.save(id, data, ttl).await
     }
 
-    async fn delete(&self, id: &str, cookie_jar: &CookieJar) -> SessionResult<()> {
-        // Get the data first so we can update the index
-        if let Ok((data, _)) = self.base_storage.load(id, None, cookie_jar).await {
-            self.remove_from_identifier_index(id, &data);
-        }
-
-        // Delete using base storage
-        self.base_storage.delete(id, cookie_jar).await
+    async fn delete(&self, id: &str, data: T) -> SessionResult<()> {
+        self.remove_from_identifier_index(id, &data);
+        self.base_storage.delete(id, data).await
     }
 
     async fn setup(&self) -> SessionResult<()> {

src/storage/redis/storage.rs

@@ -33,7 +33,7 @@ where
         Ok(())
     }
 
-    async fn delete(&self, id: &str, _cookie_jar: &CookieJar) -> SessionResult<()> {
+    async fn delete(&self, id: &str, _data: T) -> SessionResult<()> {
         let _: () = self.pool.del(self.session_key(id)).await?;
         Ok(())
     }

src/storage/redis/storage_indexed.rs

@@ -99,10 +99,7 @@ where
         self.base_storage.save_session(id, value, ttl).await
     }
 
-    async fn delete(&self, id: &str, _cookie_jar: &CookieJar) -> SessionResult<()> {
-        let (value, _) = self.base_storage.fetch_session(id, None).await?;
-        let data = T::from_value(value)?;
-
+    async fn delete(&self, id: &str, data: T) -> SessionResult<()> {
         let pipeline = self.base_storage.pool.next().pipeline();
         let _: () = pipeline.del(self.base_storage.session_key(id)).await?;
         if let Some(identifier) = data.identifier() {

src/storage/sqlx/postgres.rs

@@ -170,7 +170,7 @@ where
         Ok(())
     }
 
-    async fn delete(&self, id: &str, _cookie_jar: &CookieJar) -> SessionResult<()> {
+    async fn delete(&self, id: &str, _data: T) -> SessionResult<()> {
         let sql = format!("DELETE FROM {} WHERE {ID_COLUMN} = $1", &self.table_name);
         sqlx::query(&sql).bind(id).execute(&self.pool).await?;
 

tests/storages_indexed.rs

@@ -2,7 +2,7 @@ mod common;
 
 use std::{collections::HashMap, future::Future, pin::Pin};
 
-use rocket::{futures::FutureExt, local::asynchronous::Client};
+use rocket::futures::FutureExt;
 use rocket_flex_session::{
     storage::{
         memory::MemoryStorageIndexed,
@@ -308,7 +308,6 @@ async fn invalidate_all_but_one_by_identifier(storage_case: &str) {
 #[test_case("redis"; "Redis Fred")]
 #[rocket::async_test]
 async fn delete_single_session(storage_case: &str) {
-    let client = Client::tracked(rocket::build()).await.unwrap();
     let (storage, cleanup_task) = create_storage(storage_case).await;
     storage.setup().await.unwrap();
 
@@ -336,7 +335,7 @@ async fn delete_single_session(storage_case: &str) {
     );
 
     // Delete one session
-    storage.delete("sid1", &client.cookies()).await.unwrap();
+    storage.delete("sid1", session1.clone()).await.unwrap();
 
     // Verify only one session remains
     let remaining_sessions = storage