various session api improvements (#170)

actix-redis/examples/authentication.rs

@@ -53,7 +53,7 @@ async fn login(
     let credentials = credentials.into_inner();
 
     match User::authenticate(credentials) {
-        Ok(user) => session.set("user_id", user.id).unwrap(),
+        Ok(user) => session.insert("user_id", user.id).unwrap(),
         Err(_) => return Err(HttpResponse::Unauthorized().json("Unauthorized")),
     };
 

actix-redis/examples/basic.rs

@@ -9,9 +9,9 @@ async fn index(req: HttpRequest, session: Session) -> Result<impl Responder, Err
     // session
     if let Some(count) = session.get::<i32>("counter")? {
         println!("SESSION value: {}", count);
-        session.set("counter", count + 1)?;
+        session.insert("counter", count + 1)?;
     } else {
-        session.set("counter", 1)?;
+        session.insert("counter", 1)?;
     }
 
     Ok("Welcome!")

actix-redis/src/session.rs

@@ -157,8 +157,9 @@ where
 
         Box::pin(async move {
             let state = inner.load(&req).await?;
+
             let value = if let Some((state, value)) = state {
-                Session::set_session(state, &mut req);
+                Session::set_session(&mut req, state);
                 Some(value)
             } else {
                 None
@@ -167,8 +168,7 @@ where
             let mut res = srv.call(req).await?;
 
             match Session::get_changes(&mut res) {
-                (SessionStatus::Unchanged, None) => Ok(res),
+                (SessionStatus::Unchanged, state) => {
-                (SessionStatus::Unchanged, Some(state)) => {
                     if value.is_none() {
                         // implies the session is new
                         inner.update(res, state, value).await
@@ -176,10 +176,10 @@ where
                         Ok(res)
                     }
                 }
-                (SessionStatus::Changed, Some(state)) => {
+
-                    inner.update(res, state, value).await
+                (SessionStatus::Changed, state) => inner.update(res, state, value).await,
-                }
+
-                (SessionStatus::Purged, Some(_)) => {
+                (SessionStatus::Purged, _) => {
                     if let Some(val) = value {
                         inner.clear_cache(val).await?;
                         match inner.remove_cookie(&mut res) {
@@ -190,7 +190,8 @@ where
                         Err(error::ErrorInternalServerError("unexpected"))
                     }
                 }
-                (SessionStatus::Renewed, Some(state)) => {
+
+                (SessionStatus::Renewed, state) => {
                     if let Some(val) = value {
                         inner.clear_cache(val).await?;
                         inner.update(res, state, None).await
@@ -198,7 +199,6 @@ where
                         inner.update(res, state, None).await
                     }
                 }
-                (_, None) => unreachable!(),
             }
         })
     }
@@ -343,7 +343,7 @@ impl Inner {
         Ok(res)
     }
 
-    /// removes cache entry
+    /// Removes cache entry.
     async fn clear_cache(&self, key: String) -> Result<(), Error> {
         let cache_key = (self.cache_keygen)(&key);
 
@@ -362,7 +362,7 @@ impl Inner {
         }
     }
 
-    /// invalidates session cookie
+    /// Invalidates session cookie.
     fn remove_cookie<B>(&self, res: &mut ServiceResponse<B>) -> Result<(), Error> {
         let mut cookie = Cookie::named(self.name.clone());
         cookie.set_value("");
@@ -411,7 +411,7 @@ mod test {
             .get::<i32>("counter")
             .unwrap_or(Some(0))
             .map_or(1, |inner| inner + 1);
-        session.set("counter", counter)?;
+        session.insert("counter", &counter)?;
 
         Ok(HttpResponse::Ok().json(&IndexResponse { user_id, counter }))
     }
@@ -426,7 +426,7 @@ mod test {
         session: Session,
     ) -> Result<HttpResponse> {
         let id = user_id.into_inner().user_id;
-        session.set("user_id", &id)?;
+        session.insert("user_id", &id)?;
         session.renew();
 
         let counter: i32 = session

actix-session/CHANGES.md

@@ -1,9 +1,17 @@
 # Changes
 
 ## Unreleased - 2020-xx-xx
+* Add `Session::entries`. [#170]
+* Rename `Session::{set => insert}` to match standard hash map naming. [#170]
+* Return values from `Session::remove`. [#170]
+* Add `Session::remove_as` deserializing variation. [#170]
+* Simplify `Session::get_changes` now always returning iterator even when empty. [#170]
+* Swap order of arguments on `Session::set_session`. [#170]
 * Update `actix-web` dependency to 4.0.0 beta.
 * Minimum supported Rust version (MSRV) is now 1.46.0.
 
+[#170]: https://github.com/actix/actix-extras/pull/170
+
 
 ## 0.4.1 - 2020-03-21
 * `Session::set_session` takes a `IntoIterator` instead of `Iterator`. [#105]

actix-session/Cargo.toml

@@ -22,8 +22,10 @@ cookie-session = ["actix-web/secure-cookies"]
 [dependencies]
 actix-web = { version = "4.0.0-beta.4", default_features = false, features = ["cookies"] }
 actix-service = "2.0.0-beta.5"
-derive_more = "0.99.2"
+
+derive_more = "0.99.5"
 futures-util = { version = "0.3.7", default-features = false }
+log = "0.4"
 serde = "1.0"
 serde_json = "1.0"
 time = "0.2.23"

actix-session/src/cookie.rs

@@ -8,7 +8,7 @@ use actix_web::dev::{ServiceRequest, ServiceResponse};
 use actix_web::http::{header::SET_COOKIE, HeaderValue};
 use actix_web::{Error, HttpMessage, ResponseError};
 use derive_more::Display;
-use futures_util::future::{ok, FutureExt, LocalBoxFuture, Ready};
+use futures_util::future::{ok, LocalBoxFuture, Ready};
 use serde_json::error::Error as JsonError;
 use time::{Duration, OffsetDateTime};
 
@@ -77,6 +77,7 @@ impl CookieSessionInner {
 
         let value =
             serde_json::to_string(&state).map_err(CookieSessionError::Serialize)?;
+
         if value.len() > 4064 {
             return Err(CookieSessionError::Overflow.into());
         }
@@ -144,6 +145,7 @@ impl CookieSessionInner {
                             jar.private(&self.key).get(&self.name)
                         }
                     };
+
                     if let Some(cookie) = cookie_opt {
                         if let Ok(val) = serde_json::from_str(cookie.value()) {
                             return (false, val);
@@ -152,6 +154,7 @@ impl CookieSessionInner {
                 }
             }
         }
+
         (true, HashMap::new())
     }
 }
@@ -309,7 +312,7 @@ where
     }
 }
 
-/// Cookie session middleware
+/// Cookie based session middleware.
 pub struct CookieSessionMiddleware<S> {
     service: S,
     inner: Rc<CookieSessionInner>,
@@ -336,42 +339,41 @@ where
         let inner = self.inner.clone();
         let (is_new, state) = self.inner.load(&req);
         let prolong_expiration = self.inner.expires_in.is_some();
-        Session::set_session(state, &mut req);
+        Session::set_session(&mut req, state);
 
         let fut = self.service.call(req);
 
-        async move {
+        Box::pin(async move {
-            fut.await.map(|mut res| {
+            let mut res = fut.await?;
-                match Session::get_changes(&mut res) {
+
-                    (SessionStatus::Changed, Some(state))
+            let res = match Session::get_changes(&mut res) {
-                    | (SessionStatus::Renewed, Some(state)) => {
+                (SessionStatus::Changed, state) | (SessionStatus::Renewed, state) => {
                     res.checked_expr(|res| inner.set_cookie(res, state))
                 }
-                    (SessionStatus::Unchanged, Some(state)) if prolong_expiration => {
+
+                (SessionStatus::Unchanged, state) if prolong_expiration => {
                     res.checked_expr(|res| inner.set_cookie(res, state))
                 }
-                    (SessionStatus::Unchanged, _) =>
+
                 // set a new session cookie upon first request (new client)
-                    {
+                (SessionStatus::Unchanged, _) => {
                     if is_new {
                         let state: HashMap<String, String> = HashMap::new();
-                            res.checked_expr(|res| {
+                        res.checked_expr(|res| inner.set_cookie(res, state.into_iter()))
-                                inner.set_cookie(res, state.into_iter())
-                            })
                     } else {
                         res
                     }
                 }
+
                 (SessionStatus::Purged, _) => {
                     let _ = inner.remove_cookie(&mut res);
                     res
                 }
-                    _ => res,
+            };
-                }
+
+            Ok(res)
         })
     }
-        .boxed_local()
-    }
 }
 
 #[cfg(test)]
@@ -386,7 +388,7 @@ mod tests {
             App::new()
                 .wrap(CookieSession::signed(&[0; 32]).secure(false))
                 .service(web::resource("/").to(|ses: Session| async move {
-                    let _ = ses.set("counter", 100);
+                    let _ = ses.insert("counter", 100);
                     "test"
                 })),
         )
@@ -406,7 +408,7 @@ mod tests {
             App::new()
                 .wrap(CookieSession::private(&[0; 32]).secure(false))
                 .service(web::resource("/").to(|ses: Session| async move {
-                    let _ = ses.set("counter", 100);
+                    let _ = ses.insert("counter", 100);
                     "test"
                 })),
         )
@@ -426,7 +428,7 @@ mod tests {
             App::new()
                 .wrap(CookieSession::signed(&[0; 32]).secure(false).lazy(true))
                 .service(web::resource("/count").to(|ses: Session| async move {
-                    let _ = ses.set("counter", 100);
+                    let _ = ses.insert("counter", 100);
                     "counting"
                 }))
                 .service(web::resource("/").to(|_ses: Session| async move { "test" })),
@@ -452,7 +454,7 @@ mod tests {
             App::new()
                 .wrap(CookieSession::signed(&[0; 32]).secure(false))
                 .service(web::resource("/").to(|ses: Session| async move {
-                    let _ = ses.set("counter", 100);
+                    let _ = ses.insert("counter", 100);
                     "test"
                 })),
         )
@@ -480,7 +482,7 @@ mod tests {
                         .max_age(100),
                 )
                 .service(web::resource("/").to(|ses: Session| async move {
-                    let _ = ses.set("counter", 100);
+                    let _ = ses.insert("counter", 100);
                     "test"
                 }))
                 .service(web::resource("/test/").to(|ses: Session| async move {
@@ -513,7 +515,7 @@ mod tests {
             App::new()
                 .wrap(CookieSession::signed(&[0; 32]).secure(false).expires_in(60))
                 .service(web::resource("/").to(|ses: Session| async move {
-                    let _ = ses.set("counter", 100);
+                    let _ = ses.insert("counter", 100);
                     "test"
                 }))
                 .service(

actix-session/src/lib.rs

@@ -18,9 +18,9 @@
 //!     // access session data
 //!     if let Some(count) = session.get::<i32>("counter")? {
 //!         println!("SESSION value: {}", count);
-//!         session.set("counter", count + 1)?;
+//!         session.insert("counter", count + 1)?;
 //!     } else {
-//!         session.set("counter", 1)?;
+//!         session.insert("counter", 1)?;
 //!     }
 //!
 //!     Ok("Welcome!")
@@ -39,21 +39,27 @@
 //! }
 //! ```
 
-#![deny(rust_2018_idioms)]
+#![deny(rust_2018_idioms, nonstandard_style)]
+#![warn(missing_docs)]
 
-use std::{cell::RefCell, collections::HashMap, rc::Rc};
+use std::{
-
+    cell::{Ref, RefCell},
-use actix_web::dev::{
+    collections::HashMap,
-    Extensions, Payload, RequestHead, ServiceRequest, ServiceResponse,
+    mem,
+    rc::Rc,
+};
+
+use actix_web::{
+    dev::{Extensions, Payload, RequestHead, ServiceRequest, ServiceResponse},
+    Error, FromRequest, HttpMessage, HttpRequest,
 };
-use actix_web::{Error, FromRequest, HttpMessage, HttpRequest};
 use futures_util::future::{ok, Ready};
 use serde::{de::DeserializeOwned, Serialize};
 
 #[cfg(feature = "cookie-session")]
 mod cookie;
 #[cfg(feature = "cookie-session")]
-pub use crate::cookie::CookieSession;
+pub use self::cookie::CookieSession;
 
 /// The high-level interface you use to modify session data.
 ///
@@ -67,9 +73,9 @@ pub use crate::cookie::CookieSession;
 /// async fn index(session: Session) -> Result<&'static str> {
 ///     // access session data
 ///     if let Some(count) = session.get::<i32>("counter")? {
-///         session.set("counter", count + 1)?;
+///         session.insert("counter", count + 1)?;
 ///     } else {
-///         session.set("counter", 1)?;
+///         session.insert("counter", 1)?;
 ///     }
 ///
 ///     Ok("Welcome!")
@@ -79,6 +85,7 @@ pub struct Session(Rc<RefCell<SessionInner>>);
 
 /// Extraction of a [`Session`] object.
 pub trait UserSession {
+    /// Extract the [`Session`] object
     fn get_session(&self) -> Session;
 }
 
@@ -100,13 +107,31 @@ impl UserSession for RequestHead {
     }
 }
 
+/// Status of a [`Session`].
 #[derive(PartialEq, Clone, Debug)]
 pub enum SessionStatus {
+    /// Session has been updated and requires a new persist operation.
     Changed,
+
+    /// Session is flagged for deletion and should be removed from client and server.
+    ///
+    /// Most operations on the session after purge flag is set should have no effect.
     Purged,
+
+    /// Session is flagged for refresh.
+    ///
+    /// For example, when using a backend that has a TTL (time-to-live) expiry on the session entry,
+    /// the session will be refreshed even if no data inside it has changed. The client may also
+    /// be notified of the refresh.
     Renewed,
+
+    /// Session is unchanged from when last seen (if exists).
+    ///
+    /// This state also captures new (previously unissued) sessions such as a user's first
+    /// site visit.
     Unchanged,
 }
+
 impl Default for SessionStatus {
     fn default() -> SessionStatus {
         SessionStatus::Unchanged
@@ -116,7 +141,7 @@ impl Default for SessionStatus {
 #[derive(Default)]
 struct SessionInner {
     state: HashMap<String, String>,
-    pub status: SessionStatus,
+    status: SessionStatus,
 }
 
 impl Session {
@@ -129,37 +154,80 @@ impl Session {
         }
     }
 
-    /// Set a `value` from the session.
+    /// Get all raw key-value data from the session.
-    pub fn set<T: Serialize>(&self, key: &str, value: T) -> Result<(), Error> {
+    ///
+    /// Note that values are JSON encoded.
+    pub fn entries(&self) -> Ref<'_, HashMap<String, String>> {
+        Ref::map(self.0.borrow(), |inner| &inner.state)
+    }
+
+    /// Inserts a key-value pair into the session.
+    ///
+    /// Any serializable value can be used and will be encoded as JSON in session data, hence why
+    /// only a reference to the value is taken.
+    pub fn insert(
+        &self,
+        key: impl Into<String>,
+        value: impl Serialize,
+    ) -> Result<(), Error> {
         let mut inner = self.0.borrow_mut();
+
         if inner.status != SessionStatus::Purged {
             inner.status = SessionStatus::Changed;
-            inner
+            let val = serde_json::to_string(&value)?;
-                .state
+            inner.state.insert(key.into(), val);
-                .insert(key.to_owned(), serde_json::to_string(&value)?);
         }
+
         Ok(())
     }
 
     /// Remove value from the session.
-    pub fn remove(&self, key: &str) {
+    ///
+    /// If present, the JSON encoded value is returned.
+    pub fn remove(&self, key: &str) -> Option<String> {
         let mut inner = self.0.borrow_mut();
+
         if inner.status != SessionStatus::Purged {
             inner.status = SessionStatus::Changed;
-            inner.state.remove(key);
+            return inner.state.remove(key);
         }
+
+        None
+    }
+
+    /// Remove value from the session and deserialize.
+    ///
+    /// Returns None if key was not present in session. Returns T if deserialization succeeds,
+    /// otherwise returns un-deserialized JSON string.
+    pub fn remove_as<T: DeserializeOwned>(
+        &self,
+        key: &str,
+    ) -> Option<Result<T, String>> {
+        self.remove(key)
+            .map(|val_str| match serde_json::from_str(&val_str) {
+                Ok(val) => Ok(val),
+                Err(_err) => {
+                    log::debug!(
+                        "removed value (key: {}) could not be deserialized as {}",
+                        key,
+                        std::any::type_name::<T>()
+                    );
+                    Err(val_str)
+                }
+            })
     }
 
     /// Clear the session.
     pub fn clear(&self) {
         let mut inner = self.0.borrow_mut();
+
         if inner.status != SessionStatus::Purged {
             inner.status = SessionStatus::Changed;
             inner.state.clear()
         }
     }
 
-    /// Removes session, both client and server side.
+    /// Removes session both client and server side.
     pub fn purge(&self) {
         let mut inner = self.0.borrow_mut();
         inner.status = SessionStatus::Purged;
@@ -169,6 +237,7 @@ impl Session {
     /// Renews the session key, assigning existing session state to new key.
     pub fn renew(&self) {
         let mut inner = self.0.borrow_mut();
+
         if inner.status != SessionStatus::Purged {
             inner.status = SessionStatus::Renewed;
         }
@@ -186,35 +255,32 @@ impl Session {
     /// let mut req = test::TestRequest::default().to_srv_request();
     ///
     /// Session::set_session(
-    ///     vec![("counter".to_string(), serde_json::to_string(&0).unwrap())],
     ///     &mut req,
+    ///     vec![("counter".to_string(), serde_json::to_string(&0).unwrap())],
     /// );
     /// ```
     pub fn set_session(
-        data: impl IntoIterator<Item = (String, String)>,
         req: &mut ServiceRequest,
+        data: impl IntoIterator<Item = (String, String)>,
     ) {
         let session = Session::get_session(&mut *req.extensions_mut());
         let mut inner = session.0.borrow_mut();
         inner.state.extend(data);
     }
 
+    /// Returns session status and iterator of key-value pairs of changes.
     pub fn get_changes<B>(
         res: &mut ServiceResponse<B>,
-    ) -> (
+    ) -> (SessionStatus, impl Iterator<Item = (String, String)>) {
-        SessionStatus,
-        Option<impl Iterator<Item = (String, String)>>,
-    ) {
         if let Some(s_impl) = res
             .request()
             .extensions()
             .get::<Rc<RefCell<SessionInner>>>()
         {
-            let state =
+            let state = mem::take(&mut s_impl.borrow_mut().state);
-                std::mem::replace(&mut s_impl.borrow_mut().state, HashMap::new());
+            (s_impl.borrow().status.clone(), state.into_iter())
-            (s_impl.borrow().status.clone(), Some(state.into_iter()))
         } else {
-            (SessionStatus::Unchanged, None)
+            (SessionStatus::Unchanged, HashMap::new().into_iter())
         }
     }
 
@@ -230,21 +296,23 @@ impl Session {
 
 /// Extractor implementation for Session type.
 ///
-/// ```rust
+/// # Examples
+/// ```
 /// # use actix_web::*;
 /// use actix_session::Session;
 ///
-/// fn index(session: Session) -> Result<&'static str> {
+/// #[get("/")]
+/// async fn index(session: Session) -> Result<impl Responder> {
 ///     // access session data
 ///     if let Some(count) = session.get::<i32>("counter")? {
-///         session.set("counter", count + 1)?;
+///         session.insert("counter", count + 1)?;
 ///     } else {
-///         session.set("counter", 1)?;
+///         session.insert("counter", 1)?;
 ///     }
 ///
-///     Ok("Welcome!")
+///     let count = session.get::<i32>("counter")?.unwrap();
+///     Ok(format!("Counter: {}", count))
 /// }
-/// # fn main() {}
 /// ```
 impl FromRequest for Session {
     type Error = Error;
@@ -268,19 +336,19 @@ mod tests {
         let mut req = test::TestRequest::default().to_srv_request();
 
         Session::set_session(
-            vec![("key".to_string(), serde_json::to_string("value").unwrap())],
             &mut req,
+            vec![("key".to_string(), serde_json::to_string("value").unwrap())],
         );
         let session = Session::get_session(&mut *req.extensions_mut());
         let res = session.get::<String>("key").unwrap();
         assert_eq!(res, Some("value".to_string()));
 
-        session.set("key2", "value2".to_string()).unwrap();
+        session.insert("key2", "value2").unwrap();
         session.remove("key");
 
         let mut res = req.into_response(HttpResponse::Ok().finish());
         let (_status, state) = Session::get_changes(&mut res);
-        let changes: Vec<_> = state.unwrap().collect();
+        let changes: Vec<_> = state.collect();
         assert_eq!(changes, [("key2".to_string(), "\"value2\"".to_string())]);
     }
 
@@ -289,8 +357,8 @@ mod tests {
         let mut req = test::TestRequest::default().to_srv_request();
 
         Session::set_session(
-            vec![("key".to_string(), serde_json::to_string(&true).unwrap())],
             &mut req,
+            vec![("key".to_string(), serde_json::to_string(&true).unwrap())],
         );
 
         let session = req.get_session();
@@ -303,8 +371,8 @@ mod tests {
         let mut req = test::TestRequest::default().to_srv_request();
 
         Session::set_session(
-            vec![("key".to_string(), serde_json::to_string(&10).unwrap())],
             &mut req,
+            vec![("key".to_string(), serde_json::to_string(&10).unwrap())],
         );
 
         let session = req.head_mut().get_session();
@@ -329,4 +397,15 @@ mod tests {
         session.renew();
         assert_eq!(session.0.borrow().status, SessionStatus::Renewed);
     }
+
+    #[test]
+    fn session_entries() {
+        let session = Session(Rc::new(RefCell::new(SessionInner::default())));
+        session.insert("test_str", "val").unwrap();
+        session.insert("test_num", 1).unwrap();
+
+        let map = session.entries();
+        map.contains_key("test_str");
+        map.contains_key("test_num");
+    }
 }