From a1a094e303b170b65bdec8056a402edb6ce0a4df Mon Sep 17 00:00:00 2001
From: Jacob Rothstein <hi@jbr.me>
Date: Thu, 16 Feb 2023 17:32:39 -0800
Subject: [PATCH 1/5] overhaul async-session

* Remove interior mutability inside of Session
* store values in memory as serde_json::Values instead of strings
* add Session::take_value
* use dashmap in MemoryStore
* add an Error associated type instead of using anyhow
* remove reexported but unused libraries
* make memory-store and cookie-store cargo features (currently enabled by default)
* updates base64
* adds Session::from_parts and Session::data
---
 Cargo.toml           |  27 ++++---
 src/cookie_store.rs  |  58 ++++++++++-----
 src/lib.rs           |  22 ++----
 src/memory_store.rs  | 142 ++++++++++++++++--------------------
 src/session.rs       | 169 ++++++++++++++++++++++++++-----------------
 src/session_store.rs |  13 ++--
 6 files changed, 235 insertions(+), 196 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 7e9442a..e3b9a4c 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -14,25 +14,28 @@ authors = [
   "Jacob Rothstein <hi@jbr.me>"
 ]
 
+[features]
+default = ["memory-store", "cookie-store"]
+memory-store = ["dashmap", "thiserror", "log"]
+cookie-store = ["bincode-json", "thiserror"]
+
 [dependencies]
-async-trait = "0.1.59"
+async-trait = "0.1.64"
 rand = "0.8.5"
-base64 = "0.20.0"
-sha2 = "0.10.6"
-hmac = "0.12.1"
-serde_json = "1.0.89"
-bincode = "1.3.3"
-anyhow = "1.0.66"
+base64 = "0.21.0"
+serde_json = "1.0.93"
 blake3 = "1.3.3"
-async-lock = "2.6.0"
-log = "0.4.17"
+log = { version = "0.4.17", optional = true }
+dashmap = { version = "5.4.0", optional = true }
+bincode-json = { version = "0.1.5", features = ["json"], optional = true }
+thiserror = { version = "1.0.38", optional = true }
 
 [dependencies.serde]
-version = "1.0.150"
-features = ["rc", "derive"]
+version = "1.0.152"
+features = ["derive"]
 
 [dependencies.time]
-version = "0.3.17"
+version = "0.3.18"
 features = ["serde"]
 
 [dev-dependencies.async-std]
diff --git a/src/cookie_store.rs b/src/cookie_store.rs
index ff6eaa5..32ee96d 100644
--- a/src/cookie_store.rs
+++ b/src/cookie_store.rs
@@ -1,13 +1,14 @@
-use crate::{async_trait, Result, Session, SessionStore};
+use crate::{async_trait, Session, SessionStore};
+use base64::{engine::general_purpose::STANDARD as BASE64, Engine};
 
 /// A session store that serializes the entire session into a Cookie.
 ///
 /// # ***This is not recommended for most production deployments.***
 ///
-/// This implementation uses [`bincode`](::bincode) to serialize the
-/// Session to decrease the size of the cookie. Note: There is a
-/// maximum of 4093 cookie bytes allowed _per domain_, so the cookie
-/// store is limited in capacity.
+/// This implementation uses [`bincode_json`](::bincode_json) to
+/// serialize the Session to decrease the size of the cookie. Note:
+/// There is a maximum of 4093 cookie bytes allowed _per domain_, so
+/// the cookie store is limited in capacity.
 ///
 /// **Note:** Currently, the data in the cookie is only signed, but *not
 /// encrypted*. If the contained session data is sensitive and
@@ -29,24 +30,43 @@ impl CookieStore {
     }
 }
 
+#[derive(thiserror::Error, Debug)]
+#[non_exhaustive]
+/// All errors that can occur in the [`CookieStore`]
+pub enum CookieStoreError {
+    /// A bincode_json error
+    #[error(transparent)]
+    Bincode(#[from] bincode_json::Error),
+
+    /// A base64 error
+    #[error(transparent)]
+    Base64(#[from] base64::DecodeError),
+
+    /// A json error
+    #[error(transparent)]
+    Json(#[from] serde_json::Error),
+}
+
 #[async_trait]
 impl SessionStore for CookieStore {
-    async fn load_session(&self, cookie_value: String) -> Result<Option<Session>> {
-        let serialized = base64::decode(cookie_value)?;
-        let session: Session = bincode::deserialize(&serialized)?;
+    type Error = CookieStoreError;
+
+    async fn load_session(&self, cookie_value: String) -> Result<Option<Session>, Self::Error> {
+        let serialized = BASE64.decode(cookie_value)?;
+        let session: Session = bincode_json::from_slice(&serialized)?;
         Ok(session.validate())
     }
 
-    async fn store_session(&self, session: Session) -> Result<Option<String>> {
-        let serialized = bincode::serialize(&session)?;
-        Ok(Some(base64::encode(serialized)))
+    async fn store_session(&self, session: Session) -> Result<Option<String>, Self::Error> {
+        let serialized = bincode_json::to_vec(&session)?;
+        Ok(Some(BASE64.encode(serialized)))
     }
 
-    async fn destroy_session(&self, _session: Session) -> Result {
+    async fn destroy_session(&self, _session: Session) -> Result<(), Self::Error> {
         Ok(())
     }
 
-    async fn clear_store(&self) -> Result {
+    async fn clear_store(&self) -> Result<(), Self::Error> {
         Ok(())
     }
 }
@@ -57,7 +77,7 @@ mod tests {
     use async_std::task;
     use std::time::Duration;
     #[async_std::test]
-    async fn creating_a_new_session_with_no_expiry() -> Result {
+    async fn creating_a_new_session_with_no_expiry() -> Result<(), CookieStoreError> {
         let store = CookieStore::new();
         let mut session = Session::new();
         session.insert("key", "Hello")?;
@@ -72,7 +92,7 @@ mod tests {
     }
 
     #[async_std::test]
-    async fn updating_a_session() -> Result {
+    async fn updating_a_session() -> Result<(), CookieStoreError> {
         let store = CookieStore::new();
         let mut session = Session::new();
 
@@ -90,18 +110,18 @@ mod tests {
     }
 
     #[async_std::test]
-    async fn updating_a_session_extending_expiry() -> Result {
+    async fn updating_a_session_extending_expiry() -> Result<(), CookieStoreError> {
         let store = CookieStore::new();
         let mut session = Session::new();
         session.expire_in(Duration::from_secs(1));
-        let original_expires = session.expiry().unwrap().clone();
+        let original_expires = *session.expiry().unwrap();
         let cookie_value = store.store_session(session).await?.unwrap();
 
         let mut session = store.load_session(cookie_value.clone()).await?.unwrap();
 
         assert_eq!(session.expiry().unwrap(), &original_expires);
         session.expire_in(Duration::from_secs(3));
-        let new_expires = session.expiry().unwrap().clone();
+        let new_expires = *session.expiry().unwrap();
         let cookie_value = store.store_session(session).await?.unwrap();
 
         let session = store.load_session(cookie_value.clone()).await?.unwrap();
@@ -114,7 +134,7 @@ mod tests {
     }
 
     #[async_std::test]
-    async fn creating_a_new_session_with_expiry() -> Result {
+    async fn creating_a_new_session_with_expiry() -> Result<(), CookieStoreError> {
         let store = CookieStore::new();
         let mut session = Session::new();
         session.expire_in(Duration::from_secs(3));
diff --git a/src/lib.rs b/src/lib.rs
index 4c1d222..6f70519 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -10,7 +10,7 @@
 //! ```
 //! use async_session::{Session, SessionStore, MemoryStore};
 //!
-//! # fn main() -> async_session::Result {
+//! # fn main() -> Result<(), Box<dyn std::error::Error>> {
 //! # async_std::task::block_on(async {
 //! #
 //! // Init a new session store we can persist sessions to.
@@ -46,26 +46,18 @@
     unused_qualifications
 )]
 
-pub use anyhow::Error;
-/// An anyhow::Result with default return type of ()
-pub type Result<T = ()> = std::result::Result<T, Error>;
-
+#[cfg(feature = "cookie-store")]
 mod cookie_store;
+#[cfg(feature = "memory-store")]
 mod memory_store;
 mod session;
 mod session_store;
 
-pub use cookie_store::CookieStore;
-pub use memory_store::MemoryStore;
+#[cfg(feature = "cookie-store")]
+pub use cookie_store::{CookieStore, CookieStoreError};
+#[cfg(feature = "memory-store")]
+pub use memory_store::{MemoryStore, MemoryStoreError};
 pub use session::Session;
 pub use session_store::SessionStore;
 
 pub use async_trait::async_trait;
-pub use base64;
-pub use blake3;
-pub use hmac;
-pub use log;
-pub use serde;
-pub use serde_json;
-pub use sha2;
-pub use time;
diff --git a/src/memory_store.rs b/src/memory_store.rs
index ce5fd10..f39c589 100644
--- a/src/memory_store.rs
+++ b/src/memory_store.rs
@@ -1,70 +1,75 @@
-use crate::{async_trait, log, Result, Session, SessionStore};
-use async_lock::RwLock;
-use std::{collections::HashMap, sync::Arc};
-
-/// # in-memory session store
-/// Because there is no external
-/// persistance, this session store is ephemeral and will be cleared
-/// on server restart.
+use crate::{async_trait, Session, SessionStore};
+use dashmap::{mapref::entry::Entry::Occupied, DashMap};
+use std::sync::Arc;
+
+/// # In-memory session store
+///
+/// Because there is no external persistance, this session store is
+/// ephemeral and will be cleared on server restart.
 ///
-/// # ***READ THIS BEFORE USING IN A PRODUCTION DEPLOYMENT***
+/// ## ***READ THIS BEFORE USING IN A PRODUCTION DEPLOYMENT***
 ///
 /// Storing sessions only in memory brings the following problems:
 ///
-/// 1. All sessions must fit in available memory (important for high load services)
+/// 1. All sessions must fit in available memory.
 /// 2. Sessions stored in memory are cleared only if a client calls [MemoryStore::destroy_session] or [MemoryStore::clear_store].
-///    If sessions are not cleaned up properly it might result in OOM
-/// 3. All sessions will be lost on shutdown
+///    If sessions are not cleaned up properly it might result in OOM.
+/// 3. All sessions will be lost on shutdown.
 /// 4. If the service is clustered particular session will be stored only on a single instance.
-///    This might be solved by using load balancers with sticky sessions.
-///    Unfortunately, this solution brings additional complexity especially if the connection is
-///    using secure transport since the load balancer has to perform SSL termination to understand
-///    where should it forward packets to
 ///
-/// Example crates providing alternative implementations:
-/// - [async-sqlx-session](https://crates.io/crates/async-sqlx-session) postgres & sqlite
-/// - [async-redis-session](https://crates.io/crates/async-redis-session)
-/// - [async-mongodb-session](https://crates.io/crates/async-mongodb-session)
+/// See the crate readme for preferable session stores.
 ///
 #[derive(Default, Debug, Clone)]
-pub struct MemoryStore {
-    inner: Arc<RwLock<HashMap<String, Session>>>,
+pub struct MemoryStore(Arc<DashMap<String, Session>>);
+
+#[derive(thiserror::Error, Debug)]
+#[non_exhaustive]
+/// All errors that can occur in [`MemoryStore`]
+pub enum MemoryStoreError {
+    /// A base64 error
+    #[error(transparent)]
+    Base64(#[from] base64::DecodeError),
+
+    /// A json error
+    #[error(transparent)]
+    Json(#[from] serde_json::Error),
 }
 
 #[async_trait]
 impl SessionStore for MemoryStore {
-    async fn load_session(&self, cookie_value: String) -> Result<Option<Session>> {
+    type Error = MemoryStoreError;
+
+    async fn load_session(&self, cookie_value: String) -> Result<Option<Session>, Self::Error> {
         let id = Session::id_from_cookie_value(&cookie_value)?;
         log::trace!("loading session by id `{}`", id);
-        Ok(self
-            .inner
-            .read()
-            .await
-            .get(&id)
-            .cloned()
-            .and_then(Session::validate))
+        let Occupied(entry) = self.0.entry(id) else {
+            return Ok(None);
+        };
+
+        if entry.get().is_expired() {
+            entry.remove();
+            Ok(None)
+        } else {
+            Ok(Some(entry.get().clone()))
+        }
     }
 
-    async fn store_session(&self, session: Session) -> Result<Option<String>> {
+    async fn store_session(&self, mut session: Session) -> Result<Option<String>, Self::Error> {
         log::trace!("storing session by id `{}`", session.id());
-        self.inner
-            .write()
-            .await
-            .insert(session.id().to_string(), session.clone());
-
         session.reset_data_changed();
+        self.0.insert(session.id().to_string(), session.clone());
         Ok(session.into_cookie_value())
     }
 
-    async fn destroy_session(&self, session: Session) -> Result {
+    async fn destroy_session(&self, session: Session) -> Result<(), Self::Error> {
         log::trace!("destroying session by id `{}`", session.id());
-        self.inner.write().await.remove(session.id());
+        self.0.remove(session.id());
         Ok(())
     }
 
-    async fn clear_store(&self) -> Result {
+    async fn clear_store(&self) -> Result<(), Self::Error> {
         log::trace!("clearing memory store");
-        self.inner.write().await.clear();
+        self.0.clear();
         Ok(())
     }
 }
@@ -78,43 +83,24 @@ impl MemoryStore {
     /// Performs session cleanup. This should be run on an
     /// intermittent basis if this store is run for long enough that
     /// memory accumulation is a concern
-    pub async fn cleanup(&self) -> Result {
+    pub fn cleanup(&self) {
         log::trace!("cleaning up memory store...");
-        let ids_to_delete: Vec<_> = self
-            .inner
-            .read()
-            .await
-            .values()
-            .filter_map(|session| {
-                if session.is_expired() {
-                    Some(session.id().to_owned())
-                } else {
-                    None
-                }
-            })
-            .collect();
-
-        log::trace!("found {} expired sessions", ids_to_delete.len());
-        for id in ids_to_delete {
-            self.inner.write().await.remove(&id);
-        }
-        Ok(())
+        self.0.retain(|_, session| !session.is_expired());
     }
 
     /// returns the number of elements in the memory store
     /// # Example
     /// ```rust
     /// # use async_session::{MemoryStore, Session, SessionStore};
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let mut store = MemoryStore::new();
-    /// assert_eq!(store.count().await, 0);
+    /// assert_eq!(store.count(), 0);
     /// store.store_session(Session::new()).await?;
-    /// assert_eq!(store.count().await, 1);
+    /// assert_eq!(store.count(), 1);
     /// # Ok(()) }) }
     /// ```
-    pub async fn count(&self) -> usize {
-        let data = self.inner.read().await;
-        data.len()
+    pub fn count(&self) -> usize {
+        self.0.len()
     }
 }
 
@@ -124,7 +110,7 @@ mod tests {
     use async_std::task;
     use std::time::Duration;
     #[async_std::test]
-    async fn creating_a_new_session_with_no_expiry() -> Result {
+    async fn creating_a_new_session_with_no_expiry() -> Result<(), MemoryStoreError> {
         let store = MemoryStore::new();
         let mut session = Session::new();
         session.insert("key", "Hello")?;
@@ -139,7 +125,7 @@ mod tests {
     }
 
     #[async_std::test]
-    async fn updating_a_session() -> Result {
+    async fn updating_a_session() -> Result<(), MemoryStoreError> {
         let store = MemoryStore::new();
         let mut session = Session::new();
 
@@ -157,18 +143,18 @@ mod tests {
     }
 
     #[async_std::test]
-    async fn updating_a_session_extending_expiry() -> Result {
+    async fn updating_a_session_extending_expiry() -> Result<(), MemoryStoreError> {
         let store = MemoryStore::new();
         let mut session = Session::new();
         session.expire_in(Duration::from_secs(1));
-        let original_expires = session.expiry().unwrap().clone();
+        let original_expires = *session.expiry().unwrap();
         let cookie_value = store.store_session(session).await?.unwrap();
 
         let mut session = store.load_session(cookie_value.clone()).await?.unwrap();
 
         assert_eq!(session.expiry().unwrap(), &original_expires);
         session.expire_in(Duration::from_secs(3));
-        let new_expires = session.expiry().unwrap().clone();
+        let new_expires = *session.expiry().unwrap();
         assert_eq!(None, store.store_session(session).await?);
 
         let session = store.load_session(cookie_value.clone()).await?.unwrap();
@@ -181,7 +167,7 @@ mod tests {
     }
 
     #[async_std::test]
-    async fn creating_a_new_session_with_expiry() -> Result {
+    async fn creating_a_new_session_with_expiry() -> Result<(), MemoryStoreError> {
         let store = MemoryStore::new();
         let mut session = Session::new();
         session.expire_in(Duration::from_secs(3));
@@ -203,18 +189,18 @@ mod tests {
     }
 
     #[async_std::test]
-    async fn destroying_a_single_session() -> Result {
+    async fn destroying_a_single_session() -> Result<(), MemoryStoreError> {
         let store = MemoryStore::new();
         for _ in 0..3i8 {
             store.store_session(Session::new()).await?;
         }
 
         let cookie = store.store_session(Session::new()).await?.unwrap();
-        assert_eq!(4, store.count().await);
+        assert_eq!(4, store.count());
         let session = store.load_session(cookie.clone()).await?.unwrap();
         store.destroy_session(session.clone()).await?;
         assert_eq!(None, store.load_session(cookie).await?);
-        assert_eq!(3, store.count().await);
+        assert_eq!(3, store.count());
 
         // attempting to destroy the session again is not an error
         assert!(store.destroy_session(session).await.is_ok());
@@ -222,15 +208,15 @@ mod tests {
     }
 
     #[async_std::test]
-    async fn clearing_the_whole_store() -> Result {
+    async fn clearing_the_whole_store() -> Result<(), MemoryStoreError> {
         let store = MemoryStore::new();
         for _ in 0..3i8 {
             store.store_session(Session::new()).await?;
         }
 
-        assert_eq!(3, store.count().await);
+        assert_eq!(3, store.count());
         store.clear_store().await.unwrap();
-        assert_eq!(0, store.count().await);
+        assert_eq!(0, store.count());
 
         Ok(())
     }
diff --git a/src/session.rs b/src/session.rs
index df4ba3d..9289ad5 100644
--- a/src/session.rs
+++ b/src/session.rs
@@ -1,23 +1,18 @@
+use base64::{engine::general_purpose::STANDARD as BASE64, Engine};
 use rand::RngCore;
 use serde::{Deserialize, Serialize};
-use std::{
-    collections::HashMap,
-    convert::TryFrom,
-    sync::{
-        atomic::{AtomicBool, Ordering},
-        Arc, RwLock,
-    },
-};
+use serde_json::Value;
+use std::{collections::HashMap, convert::TryFrom};
 use time::OffsetDateTime as DateTime;
 
 /// # The main session type.
 ///
 /// ## Cloning and Serialization
 ///
-/// The `cookie_value` field is not cloned or serialized, and it can
-/// only be read through `into_cookie_value`. The intent of this field
-/// is that it is set either by initialization or by a session store,
-/// and read exactly once in order to set the cookie value.
+/// The `cookie_value` field is not serialized, and it can only be
+/// read through `into_cookie_value`. The intent of this field is that
+/// it is set either by initialization or by a session store, and read
+/// exactly once in order to set the cookie value.
 ///
 /// ## Change tracking session tracks whether any of its inner data
 /// was changed since it was last serialized. Any session store that
@@ -28,7 +23,7 @@ use time::OffsetDateTime as DateTime;
 /// ### Change tracking example
 /// ```rust
 /// # use async_session::Session;
-/// # fn main() -> async_session::Result { async_std::task::block_on(async {
+/// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
 /// let mut session = Session::new();
 /// assert!(!session.data_changed());
 ///
@@ -58,14 +53,14 @@ use time::OffsetDateTime as DateTime;
 pub struct Session {
     id: String,
     expiry: Option<DateTime>,
-    data: Arc<RwLock<HashMap<String, String>>>,
+    data: HashMap<String, Value>,
 
     #[serde(skip)]
     cookie_value: Option<String>,
     #[serde(skip)]
-    data_changed: Arc<AtomicBool>,
+    data_changed: bool,
     #[serde(skip)]
-    destroy: Arc<AtomicBool>,
+    destroy: bool,
 }
 
 impl Clone for Session {
@@ -75,8 +70,8 @@ impl Clone for Session {
             id: self.id.clone(),
             data: self.data.clone(),
             expiry: self.expiry,
-            destroy: self.destroy.clone(),
-            data_changed: self.data_changed.clone(),
+            destroy: self.destroy,
+            data_changed: self.data_changed,
         }
     }
 }
@@ -91,7 +86,7 @@ impl Default for Session {
 fn generate_cookie(len: usize) -> String {
     let mut key = vec![0u8; len];
     rand::thread_rng().fill_bytes(&mut key);
-    base64::encode(key)
+    BASE64.encode(key)
 }
 
 impl Session {
@@ -102,7 +97,7 @@ impl Session {
     ///
     /// ```rust
     /// # use async_session::Session;
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let session = Session::new();
     /// assert_eq!(None, session.expiry());
     /// assert!(session.into_cookie_value().is_some());
@@ -112,15 +107,35 @@ impl Session {
         let id = Session::id_from_cookie_value(&cookie_value).unwrap();
 
         Self {
-            data_changed: Arc::new(AtomicBool::new(false)),
+            data_changed: false,
             expiry: None,
-            data: Arc::new(RwLock::new(HashMap::default())),
+            data: HashMap::default(),
             cookie_value: Some(cookie_value),
             id,
-            destroy: Arc::new(AtomicBool::new(false)),
+            destroy: false,
         }
     }
 
+    /// Create a session from id, data, and expiry. This is intended
+    /// to be used by session store implementers to rehydrate sessions
+    /// from persistence.
+    pub fn from_parts(id: String, data: HashMap<String, Value>, expiry: Option<DateTime>) -> Self {
+        Self {
+            data,
+            expiry,
+            id,
+            data_changed: false,
+            destroy: false,
+            cookie_value: None,
+        }
+    }
+
+    /// Borrow the data hashmap. This is intended to be used by
+    /// session store implementers.
+    pub fn data(&self) -> &HashMap<String, Value> {
+        &self.data
+    }
+
     /// applies a cryptographic hash function on a cookie value
     /// returned by [`Session::into_cookie_value`] to obtain the
     /// session id for that cookie. Returns an error if the cookie
@@ -130,7 +145,7 @@ impl Session {
     ///
     /// ```rust
     /// # use async_session::Session;
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let session = Session::new();
     /// let id = session.id().to_string();
     /// let cookie_value = session.into_cookie_value().unwrap();
@@ -138,9 +153,9 @@ impl Session {
     /// # Ok(()) }) }
     /// ```
     pub fn id_from_cookie_value(string: &str) -> Result<String, base64::DecodeError> {
-        let decoded = base64::decode(string)?;
+        let decoded = BASE64.decode(string)?;
         let hash = blake3::hash(&decoded);
-        Ok(base64::encode(hash.as_bytes()))
+        Ok(BASE64.encode(hash.as_bytes()))
     }
 
     /// mark this session for destruction. the actual session record
@@ -150,14 +165,14 @@ impl Session {
     ///
     /// ```rust
     /// # use async_session::Session;
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let mut session = Session::new();
     /// assert!(!session.is_destroyed());
     /// session.destroy();
     /// assert!(session.is_destroyed());
     /// # Ok(()) }) }
     pub fn destroy(&mut self) {
-        self.destroy.store(true, Ordering::SeqCst);
+        self.destroy = true;
     }
 
     /// returns true if this session is marked for destruction
@@ -166,7 +181,7 @@ impl Session {
     ///
     /// ```rust
     /// # use async_session::Session;
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let mut session = Session::new();
     /// assert!(!session.is_destroyed());
     /// session.destroy();
@@ -174,7 +189,7 @@ impl Session {
     /// # Ok(()) }) }
 
     pub fn is_destroyed(&self) -> bool {
-        self.destroy.load(Ordering::SeqCst)
+        self.destroy
     }
 
     /// Gets the session id
@@ -183,7 +198,7 @@ impl Session {
     ///
     /// ```rust
     /// # use async_session::Session;
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let session = Session::new();
     /// let id = session.id().to_owned();
     /// let cookie_value = session.into_cookie_value().unwrap();
@@ -208,10 +223,10 @@ impl Session {
     /// }
     /// let mut session = Session::new();
     /// session.insert("user", User { name: "chashu".into(), legs: 4 }).expect("serializable");
-    /// assert_eq!(r#"{"name":"chashu","legs":4}"#, session.get_raw("user").unwrap());
+    /// assert_eq!(r#"{"legs":4,"name":"chashu"}"#, session.get_value("user").unwrap().to_string());
     /// ```
     pub fn insert(&mut self, key: &str, value: impl Serialize) -> Result<(), serde_json::Error> {
-        self.insert_raw(key, serde_json::to_string(&value)?);
+        self.insert_value(key, serde_json::to_value(&value)?);
         Ok(())
     }
 
@@ -222,15 +237,14 @@ impl Session {
     /// ```rust
     /// # use async_session::Session;
     /// let mut session = Session::new();
-    /// session.insert_raw("ten", "10".to_string());
+    /// session.insert_value("ten", serde_json::json!(10));
     /// let ten: usize = session.get("ten").unwrap();
     /// assert_eq!(ten, 10);
     /// ```
-    pub fn insert_raw(&mut self, key: &str, value: String) {
-        let mut data = self.data.write().unwrap();
-        if data.get(key) != Some(&value) {
-            data.insert(key.to_string(), value);
-            self.data_changed.store(true, Ordering::Release);
+    pub fn insert_value(&mut self, key: &str, value: Value) {
+        if self.data.get(key) != Some(&value) {
+            self.data.insert(key.to_string(), value);
+            self.data_changed = true;
         }
     }
 
@@ -246,12 +260,14 @@ impl Session {
     /// assert_eq!(vec![1, 2, 3], numbers);
     /// ```
     pub fn get<T: serde::de::DeserializeOwned>(&self, key: &str) -> Option<T> {
-        let data = self.data.read().unwrap();
-        let string = data.get(key)?;
-        serde_json::from_str(string).ok()
+        self.get_value(key)
+            .map(serde_json::from_value)
+            .transpose()
+            .ok()
+            .flatten()
     }
 
-    /// returns the String value contained in the session hashmap
+    /// returns the [`serde_json::Value`] contained in the session hashmap
     ///
     /// # Example
     ///
@@ -259,11 +275,10 @@ impl Session {
     /// # use async_session::Session;
     /// let mut session = Session::new();
     /// session.insert("key", vec![1, 2, 3]);
-    /// assert_eq!("[1,2,3]", session.get_raw("key").unwrap());
+    /// assert_eq!("[1,2,3]", session.get_value("key").unwrap().to_string());
     /// ```
-    pub fn get_raw(&self, key: &str) -> Option<String> {
-        let data = self.data.read().unwrap();
-        data.get(key).cloned()
+    pub fn get_value(&self, key: &str) -> Option<Value> {
+        self.data.get(key).cloned()
     }
 
     /// removes an entry from the session hashmap
@@ -275,14 +290,34 @@ impl Session {
     /// let mut session = Session::new();
     /// session.insert("key", "value");
     /// session.remove("key");
-    /// assert!(session.get_raw("key").is_none());
+    /// assert!(session.get_value("key").is_none());
     /// assert_eq!(session.len(), 0);
     /// ```
     pub fn remove(&mut self, key: &str) {
-        let mut data = self.data.write().unwrap();
-        if data.remove(key).is_some() {
-            self.data_changed.store(true, Ordering::Release);
+        if self.data.remove(key).is_some() {
+            self.data_changed = true;
+        }
+    }
+
+    /// Takes an entry from the session hashmap
+    ///
+    /// # Example
+    ///
+    /// ```rust
+    /// # use async_session::Session;
+    /// let mut session = Session::new();
+    /// session.insert("key", "value");
+    /// let took = session.take_value("key").unwrap();
+    /// assert_eq!(took.to_string(), "\"value\"");
+    /// assert!(session.get_value("key").is_none());
+    /// assert_eq!(session.len(), 0);
+    /// ```
+    pub fn take_value(&mut self, key: &str) -> Option<Value> {
+        let took = self.data.remove(key);
+        if took.is_some() {
+            self.data_changed = true;
         }
+        took
     }
 
     /// returns the number of elements in the session hashmap
@@ -297,7 +332,7 @@ impl Session {
     /// assert_eq!(session.len(), 1);
     /// ```
     pub fn len(&self) -> usize {
-        self.data.read().unwrap().len()
+        self.data.len()
     }
 
     /// returns a boolean indicating whether there are zero elements in the session hashmap
@@ -311,7 +346,7 @@ impl Session {
     /// session.insert("key", 0);
     /// assert!(!session.is_empty());
     pub fn is_empty(&self) -> bool {
-        return self.data.read().unwrap().is_empty();
+        self.data.is_empty()
     }
 
     /// Generates a new id and cookie for this session
@@ -320,7 +355,7 @@ impl Session {
     ///
     /// ```rust
     /// # use async_session::Session;
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let mut session = Session::new();
     /// let old_id = session.id().to_string();
     /// session.regenerate();
@@ -345,7 +380,7 @@ impl Session {
     ///
     /// ```rust
     /// # use async_session::Session;
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let mut session = Session::new();
     /// session.set_cookie_value("hello".to_owned());
     /// let cookie_value = session.into_cookie_value().unwrap();
@@ -362,7 +397,7 @@ impl Session {
     ///
     /// ```rust
     /// # use async_session::Session;
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let mut session = Session::new();
     /// assert_eq!(None, session.expiry());
     /// session.expire_in(std::time::Duration::from_secs(1));
@@ -379,7 +414,7 @@ impl Session {
     ///
     /// ```rust
     /// # use async_session::Session;
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let mut session = Session::new();
     /// assert_eq!(None, session.expiry());
     /// session.set_expiry(time::OffsetDateTime::now_utc());
@@ -396,7 +431,7 @@ impl Session {
     ///
     /// ```rust
     /// # use async_session::Session;
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let mut session = Session::new();
     /// assert_eq!(None, session.expiry());
     /// session.expire_in(std::time::Duration::from_secs(1));
@@ -417,7 +452,7 @@ impl Session {
     /// # use async_session::Session;
     /// # use std::time::Duration;
     /// # use async_std::task;
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let mut session = Session::new();
     /// assert_eq!(None, session.expiry());
     /// assert!(!session.is_expired());
@@ -442,7 +477,7 @@ impl Session {
     /// # use async_session::Session;
     /// # use std::time::Duration;
     /// # use async_std::task;
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let session = Session::new();
     /// let mut session = session.validate().unwrap();
     /// session.expire_in(Duration::from_secs(1));
@@ -466,7 +501,7 @@ impl Session {
     ///
     /// ```rust
     /// # use async_session::Session;
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let mut session = Session::new();
     /// assert!(!session.data_changed(), "new session is not changed");
     /// session.insert("key", 1);
@@ -479,7 +514,7 @@ impl Session {
     /// # Ok(()) }) }
     /// ```
     pub fn data_changed(&self) -> bool {
-        self.data_changed.load(Ordering::Acquire)
+        self.data_changed
     }
 
     /// Resets `data_changed` dirty tracking. This is unnecessary for
@@ -490,7 +525,7 @@ impl Session {
     ///
     /// ```rust
     /// # use async_session::Session;
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let mut session = Session::new();
     /// assert!(!session.data_changed(), "new session is not changed");
     /// session.insert("key", 1);
@@ -502,8 +537,8 @@ impl Session {
     /// assert!(session.data_changed());
     /// # Ok(()) }) }
     /// ```
-    pub fn reset_data_changed(&self) {
-        self.data_changed.store(false, Ordering::SeqCst);
+    pub fn reset_data_changed(&mut self) {
+        self.data_changed = false;
     }
 
     /// Ensures that this session is not expired. Returns None if it is expired
@@ -514,7 +549,7 @@ impl Session {
     /// # use async_session::Session;
     /// # use std::time::Duration;
     /// # use async_std::task;
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let mut session = Session::new();
     /// session.expire_in(Duration::from_secs(123));
     /// let expires_in = session.expires_in().unwrap();
@@ -538,7 +573,7 @@ impl Session {
     ///
     /// ```rust
     /// # use async_session::Session;
-    /// # fn main() -> async_session::Result { async_std::task::block_on(async {
+    /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let mut session = Session::new();
     /// session.set_cookie_value("hello".to_owned());
     /// let cookie_value = session.into_cookie_value().unwrap();
diff --git a/src/session_store.rs b/src/session_store.rs
index 5049dea..116b6a7 100644
--- a/src/session_store.rs
+++ b/src/session_store.rs
@@ -1,24 +1,27 @@
-use crate::{async_trait, Result, Session};
+use crate::{async_trait, Session};
 
 /// An async session backend.
 #[async_trait]
 pub trait SessionStore {
+    /// The [`std::error::Error`] type that this store returns
+    type Error: std::error::Error;
+
     /// Get a session from the storage backend.
     ///
     /// The input is expected to be the value of an identifying
     /// cookie. This will then be parsed by the session middleware
     /// into a session if possible
-    async fn load_session(&self, cookie_value: String) -> Result<Option<Session>>;
+    async fn load_session(&self, cookie_value: String) -> Result<Option<Session>, Self::Error>;
 
     /// Store a session on the storage backend.
     ///
     /// The return value is the value of the cookie to store for the
     /// user that represents this session
-    async fn store_session(&self, session: Session) -> Result<Option<String>>;
+    async fn store_session(&self, session: Session) -> Result<Option<String>, Self::Error>;
 
     /// Remove a session from the session store
-    async fn destroy_session(&self, session: Session) -> Result;
+    async fn destroy_session(&self, session: Session) -> Result<(), Self::Error>;
 
     /// Empties the entire store, destroying all sessions
-    async fn clear_store(&self) -> Result;
+    async fn clear_store(&self) -> Result<(), Self::Error>;
 }

From 60aaa4f6043bce420995e80f25e00572a2c68174 Mon Sep 17 00:00:00 2001
From: Jacob Rothstein <hi@jbr.me>
Date: Wed, 29 Mar 2023 18:59:51 -0700
Subject: [PATCH 2/5] redefine SessionStore to take &mut Session and &str

---
 src/cookie_store.rs  | 38 ++++++++++++++---------------
 src/lib.rs           |  6 ++---
 src/memory_store.rs  | 57 ++++++++++++++++++++++----------------------
 src/session.rs       |  6 +++++
 src/session_store.rs |  6 ++---
 5 files changed, 60 insertions(+), 53 deletions(-)

diff --git a/src/cookie_store.rs b/src/cookie_store.rs
index 32ee96d..a170c7e 100644
--- a/src/cookie_store.rs
+++ b/src/cookie_store.rs
@@ -51,18 +51,18 @@ pub enum CookieStoreError {
 impl SessionStore for CookieStore {
     type Error = CookieStoreError;
 
-    async fn load_session(&self, cookie_value: String) -> Result<Option<Session>, Self::Error> {
+    async fn load_session(&self, cookie_value: &str) -> Result<Option<Session>, Self::Error> {
         let serialized = BASE64.decode(cookie_value)?;
         let session: Session = bincode_json::from_slice(&serialized)?;
         Ok(session.validate())
     }
 
-    async fn store_session(&self, session: Session) -> Result<Option<String>, Self::Error> {
-        let serialized = bincode_json::to_vec(&session)?;
+    async fn store_session(&self, session: &mut Session) -> Result<Option<String>, Self::Error> {
+        let serialized = bincode_json::to_vec(session)?;
         Ok(Some(BASE64.encode(serialized)))
     }
 
-    async fn destroy_session(&self, _session: Session) -> Result<(), Self::Error> {
+    async fn destroy_session(&self, _session: &mut Session) -> Result<(), Self::Error> {
         Ok(())
     }
 
@@ -82,8 +82,8 @@ mod tests {
         let mut session = Session::new();
         session.insert("key", "Hello")?;
         let cloned = session.clone();
-        let cookie_value = store.store_session(session).await?.unwrap();
-        let loaded_session = store.load_session(cookie_value).await?.unwrap();
+        let cookie_value = store.store_session(&mut session).await?.unwrap();
+        let loaded_session = store.load_session(&cookie_value).await?.unwrap();
         assert_eq!(cloned.id(), loaded_session.id());
         assert_eq!("Hello", &loaded_session.get::<String>("key").unwrap());
         assert!(!loaded_session.is_expired());
@@ -97,14 +97,14 @@ mod tests {
         let mut session = Session::new();
 
         session.insert("key", "value")?;
-        let cookie_value = store.store_session(session).await?.unwrap();
+        let cookie_value = store.store_session(&mut session).await?.unwrap();
 
-        let mut session = store.load_session(cookie_value.clone()).await?.unwrap();
+        let mut session = store.load_session(&cookie_value.clone()).await?.unwrap();
         session.insert("key", "other value")?;
 
-        let new_cookie_value = store.store_session(session).await?.unwrap();
-        let session = store.load_session(new_cookie_value).await?.unwrap();
-        assert_eq!(&session.get::<String>("key").unwrap(), "other value");
+        let new_cookie_value = store.store_session(&mut session).await?.unwrap();
+        let session = store.load_session(&new_cookie_value).await?.unwrap();
+        assert_eq!(&mut session.get::<String>("key").unwrap(), "other value");
 
         Ok(())
     }
@@ -115,20 +115,20 @@ mod tests {
         let mut session = Session::new();
         session.expire_in(Duration::from_secs(1));
         let original_expires = *session.expiry().unwrap();
-        let cookie_value = store.store_session(session).await?.unwrap();
+        let cookie_value = store.store_session(&mut session).await?.unwrap();
 
-        let mut session = store.load_session(cookie_value.clone()).await?.unwrap();
+        let mut session = store.load_session(&cookie_value.clone()).await?.unwrap();
 
         assert_eq!(session.expiry().unwrap(), &original_expires);
         session.expire_in(Duration::from_secs(3));
         let new_expires = *session.expiry().unwrap();
-        let cookie_value = store.store_session(session).await?.unwrap();
+        let cookie_value = store.store_session(&mut session).await?.unwrap();
 
-        let session = store.load_session(cookie_value.clone()).await?.unwrap();
+        let session = store.load_session(&cookie_value.clone()).await?.unwrap();
         assert_eq!(session.expiry().unwrap(), &new_expires);
 
         task::sleep(Duration::from_secs(3)).await;
-        assert_eq!(None, store.load_session(cookie_value).await?);
+        assert_eq!(None, store.load_session(&cookie_value).await?);
 
         Ok(())
     }
@@ -141,16 +141,16 @@ mod tests {
         session.insert("key", "value")?;
         let cloned = session.clone();
 
-        let cookie_value = store.store_session(session).await?.unwrap();
+        let cookie_value = store.store_session(&mut session).await?.unwrap();
 
-        let loaded_session = store.load_session(cookie_value.clone()).await?.unwrap();
+        let loaded_session = store.load_session(&cookie_value.clone()).await?.unwrap();
         assert_eq!(cloned.id(), loaded_session.id());
         assert_eq!("value", &*loaded_session.get::<String>("key").unwrap());
 
         assert!(!loaded_session.is_expired());
 
         task::sleep(Duration::from_secs(3)).await;
-        assert_eq!(None, store.load_session(cookie_value).await?);
+        assert_eq!(None, store.load_session(&cookie_value).await?);
 
         Ok(())
     }
diff --git a/src/lib.rs b/src/lib.rs
index 6f70519..f2f3ccf 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -22,10 +22,10 @@
 //! assert!(session.data_changed());
 //!
 //! // retrieve the cookie value to store in a session cookie
-//! let cookie_value = store.store_session(session).await?.unwrap();
+//! let cookie_value = store.store_session(&mut session).await?.unwrap();
 //!
 //! // Retrieve the session using the cookie.
-//! let session = store.load_session(cookie_value).await?.unwrap();
+//! let session = store.load_session(&cookie_value).await?.unwrap();
 //! assert_eq!(session.get::<usize>("user_id").unwrap(), 1);
 //! assert!(!session.data_changed());
 //! #
@@ -58,6 +58,6 @@ pub use cookie_store::{CookieStore, CookieStoreError};
 #[cfg(feature = "memory-store")]
 pub use memory_store::{MemoryStore, MemoryStoreError};
 pub use session::Session;
-pub use session_store::SessionStore;
+pub use session_store::{SessionStore};
 
 pub use async_trait::async_trait;
diff --git a/src/memory_store.rs b/src/memory_store.rs
index f39c589..f8cb4f8 100644
--- a/src/memory_store.rs
+++ b/src/memory_store.rs
@@ -39,8 +39,8 @@ pub enum MemoryStoreError {
 impl SessionStore for MemoryStore {
     type Error = MemoryStoreError;
 
-    async fn load_session(&self, cookie_value: String) -> Result<Option<Session>, Self::Error> {
-        let id = Session::id_from_cookie_value(&cookie_value)?;
+    async fn load_session(&self, cookie_value: &str) -> Result<Option<Session>, Self::Error> {
+        let id = Session::id_from_cookie_value(cookie_value)?;
         log::trace!("loading session by id `{}`", id);
         let Occupied(entry) = self.0.entry(id) else {
             return Ok(None);
@@ -54,14 +54,15 @@ impl SessionStore for MemoryStore {
         }
     }
 
-    async fn store_session(&self, mut session: Session) -> Result<Option<String>, Self::Error> {
+    async fn store_session(&self, session: &mut Session) -> Result<Option<String>, Self::Error> {
         log::trace!("storing session by id `{}`", session.id());
         session.reset_data_changed();
+        let cookie_value = session.take_cookie_value();
         self.0.insert(session.id().to_string(), session.clone());
-        Ok(session.into_cookie_value())
+        Ok(cookie_value)
     }
 
-    async fn destroy_session(&self, session: Session) -> Result<(), Self::Error> {
+    async fn destroy_session(&self, session: &mut Session) -> Result<(), Self::Error> {
         log::trace!("destroying session by id `{}`", session.id());
         self.0.remove(session.id());
         Ok(())
@@ -95,7 +96,7 @@ impl MemoryStore {
     /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let mut store = MemoryStore::new();
     /// assert_eq!(store.count(), 0);
-    /// store.store_session(Session::new()).await?;
+    /// store.store_session(&mut Session::new()).await?;
     /// assert_eq!(store.count(), 1);
     /// # Ok(()) }) }
     /// ```
@@ -115,8 +116,8 @@ mod tests {
         let mut session = Session::new();
         session.insert("key", "Hello")?;
         let cloned = session.clone();
-        let cookie_value = store.store_session(session).await?.unwrap();
-        let loaded_session = store.load_session(cookie_value).await?.unwrap();
+        let cookie_value = store.store_session(&mut session).await?.unwrap();
+        let loaded_session = store.load_session(&cookie_value).await?.unwrap();
         assert_eq!(cloned.id(), loaded_session.id());
         assert_eq!("Hello", &loaded_session.get::<String>("key").unwrap());
         assert!(!loaded_session.is_expired());
@@ -130,14 +131,14 @@ mod tests {
         let mut session = Session::new();
 
         session.insert("key", "value")?;
-        let cookie_value = store.store_session(session).await?.unwrap();
+        let cookie_value = store.store_session(&mut session).await?.unwrap();
 
-        let mut session = store.load_session(cookie_value.clone()).await?.unwrap();
+        let mut session = store.load_session(&cookie_value).await?.unwrap();
         session.insert("key", "other value")?;
 
-        assert_eq!(store.store_session(session).await?, None);
-        let session = store.load_session(cookie_value).await?.unwrap();
-        assert_eq!(&session.get::<String>("key").unwrap(), "other value");
+        assert_eq!(store.store_session(&mut session).await?, None);
+        let session = store.load_session(&cookie_value).await?.unwrap();
+        assert_eq!(&mut session.get::<String>("key").unwrap(), "other value");
 
         Ok(())
     }
@@ -148,20 +149,20 @@ mod tests {
         let mut session = Session::new();
         session.expire_in(Duration::from_secs(1));
         let original_expires = *session.expiry().unwrap();
-        let cookie_value = store.store_session(session).await?.unwrap();
+        let cookie_value = store.store_session(&mut session).await?.unwrap();
 
-        let mut session = store.load_session(cookie_value.clone()).await?.unwrap();
+        let mut session = store.load_session(&cookie_value).await?.unwrap();
 
         assert_eq!(session.expiry().unwrap(), &original_expires);
         session.expire_in(Duration::from_secs(3));
         let new_expires = *session.expiry().unwrap();
-        assert_eq!(None, store.store_session(session).await?);
+        assert_eq!(None, store.store_session(&mut session).await?);
 
-        let session = store.load_session(cookie_value.clone()).await?.unwrap();
+        let session = store.load_session(&cookie_value).await?.unwrap();
         assert_eq!(session.expiry().unwrap(), &new_expires);
 
         task::sleep(Duration::from_secs(3)).await;
-        assert_eq!(None, store.load_session(cookie_value).await?);
+        assert_eq!(None, store.load_session(&cookie_value).await?);
 
         Ok(())
     }
@@ -174,16 +175,16 @@ mod tests {
         session.insert("key", "value")?;
         let cloned = session.clone();
 
-        let cookie_value = store.store_session(session).await?.unwrap();
+        let cookie_value = store.store_session(&mut session).await?.unwrap();
 
-        let loaded_session = store.load_session(cookie_value.clone()).await?.unwrap();
+        let loaded_session = store.load_session(&cookie_value).await?.unwrap();
         assert_eq!(cloned.id(), loaded_session.id());
         assert_eq!("value", &*loaded_session.get::<String>("key").unwrap());
 
         assert!(!loaded_session.is_expired());
 
         task::sleep(Duration::from_secs(3)).await;
-        assert_eq!(None, store.load_session(cookie_value).await?);
+        assert_eq!(None, store.load_session(&cookie_value).await?);
 
         Ok(())
     }
@@ -192,18 +193,18 @@ mod tests {
     async fn destroying_a_single_session() -> Result<(), MemoryStoreError> {
         let store = MemoryStore::new();
         for _ in 0..3i8 {
-            store.store_session(Session::new()).await?;
+            store.store_session(&mut Session::new()).await?;
         }
 
-        let cookie = store.store_session(Session::new()).await?.unwrap();
+        let cookie = store.store_session(&mut Session::new()).await?.unwrap();
         assert_eq!(4, store.count());
-        let session = store.load_session(cookie.clone()).await?.unwrap();
-        store.destroy_session(session.clone()).await?;
-        assert_eq!(None, store.load_session(cookie).await?);
+        let mut session = store.load_session(&cookie).await?.unwrap();
+        store.destroy_session(&mut session).await?;
+        assert_eq!(None, store.load_session(&cookie).await?);
         assert_eq!(3, store.count());
 
         // attempting to destroy the session again is not an error
-        assert!(store.destroy_session(session).await.is_ok());
+        assert!(store.destroy_session(&mut session).await.is_ok());
         Ok(())
     }
 
@@ -211,7 +212,7 @@ mod tests {
     async fn clearing_the_whole_store() -> Result<(), MemoryStoreError> {
         let store = MemoryStore::new();
         for _ in 0..3i8 {
-            store.store_session(Session::new()).await?;
+            store.store_session(&mut Session::new()).await?;
         }
 
         assert_eq!(3, store.count());
diff --git a/src/session.rs b/src/session.rs
index 9289ad5..5d3b69d 100644
--- a/src/session.rs
+++ b/src/session.rs
@@ -581,6 +581,12 @@ impl Session {
     /// # Ok(()) }) }
     /// ```
     pub fn into_cookie_value(mut self) -> Option<String> {
+        self.take_cookie_value()
+    }
+
+    /// take the cookie value. this is generally only performed by a
+    /// session store.
+    pub fn take_cookie_value(&mut self) -> Option<String> {
         self.cookie_value.take()
     }
 }
diff --git a/src/session_store.rs b/src/session_store.rs
index 116b6a7..a778f87 100644
--- a/src/session_store.rs
+++ b/src/session_store.rs
@@ -11,16 +11,16 @@ pub trait SessionStore {
     /// The input is expected to be the value of an identifying
     /// cookie. This will then be parsed by the session middleware
     /// into a session if possible
-    async fn load_session(&self, cookie_value: String) -> Result<Option<Session>, Self::Error>;
+    async fn load_session(&self, cookie_value: &str) -> Result<Option<Session>, Self::Error>;
 
     /// Store a session on the storage backend.
     ///
     /// The return value is the value of the cookie to store for the
     /// user that represents this session
-    async fn store_session(&self, session: Session) -> Result<Option<String>, Self::Error>;
+    async fn store_session(&self, session: &mut Session) -> Result<Option<String>, Self::Error>;
 
     /// Remove a session from the session store
-    async fn destroy_session(&self, session: Session) -> Result<(), Self::Error>;
+    async fn destroy_session(&self, session: &mut Session) -> Result<(), Self::Error>;
 
     /// Empties the entire store, destroying all sessions
     async fn clear_store(&self) -> Result<(), Self::Error>;

From 493fa044ab0e0b66e82617f7071dcaadc220f009 Mon Sep 17 00:00:00 2001
From: Jacob Rothstein <hi@jbr.me>
Date: Wed, 29 Mar 2023 19:07:31 -0700
Subject: [PATCH 3/5] allow Session cookie value to be cloned

---
 src/session.rs | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/src/session.rs b/src/session.rs
index 5d3b69d..a7daaee 100644
--- a/src/session.rs
+++ b/src/session.rs
@@ -49,7 +49,7 @@ use time::OffsetDateTime as DateTime;
 /// assert!(session.data_changed());
 /// # Ok(()) }) }
 /// ```
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct Session {
     id: String,
     expiry: Option<DateTime>,
@@ -63,19 +63,6 @@ pub struct Session {
     destroy: bool,
 }
 
-impl Clone for Session {
-    fn clone(&self) -> Self {
-        Self {
-            cookie_value: None,
-            id: self.id.clone(),
-            data: self.data.clone(),
-            expiry: self.expiry,
-            destroy: self.destroy,
-            data_changed: self.data_changed,
-        }
-    }
-}
-
 impl Default for Session {
     fn default() -> Self {
         Self::new()

From c862bde3f1ae8c2252e2c7c741933db605a734f8 Mon Sep 17 00:00:00 2001
From: Jacob Rothstein <hi@jbr.me>
Date: Wed, 29 Mar 2023 19:40:27 -0700
Subject: [PATCH 4/5] fix format

---
 src/lib.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/lib.rs b/src/lib.rs
index f2f3ccf..e326cee 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -58,6 +58,6 @@ pub use cookie_store::{CookieStore, CookieStoreError};
 #[cfg(feature = "memory-store")]
 pub use memory_store::{MemoryStore, MemoryStoreError};
 pub use session::Session;
-pub use session_store::{SessionStore};
+pub use session_store::SessionStore;
 
 pub use async_trait::async_trait;

From 35cb0998f91b81b133c3314414adae4019a62741 Mon Sep 17 00:00:00 2001
From: Jacob Rothstein <hi@jbr.me>
Date: Wed, 29 Mar 2023 20:59:58 -0700
Subject: [PATCH 5/5] split cookie store and memory store into workspace crates

---
 Cargo.toml                                    | 46 ++-----------------
 async-session/Cargo.toml                      | 37 +++++++++++++++
 {src => async-session/src}/lib.rs             | 11 +----
 {src => async-session/src}/session.rs         |  0
 {src => async-session/src}/session_store.rs   |  0
 {tests => async-session/tests}/test.rs        |  0
 cookie-store/Cargo.toml                       | 14 ++++++
 .../src/lib.rs                                |  2 +-
 memory-store/Cargo.toml                       | 17 +++++++
 .../src/lib.rs                                |  5 +-
 10 files changed, 77 insertions(+), 55 deletions(-)
 create mode 100644 async-session/Cargo.toml
 rename {src => async-session/src}/lib.rs (82%)
 rename {src => async-session/src}/session.rs (100%)
 rename {src => async-session/src}/session_store.rs (100%)
 rename {tests => async-session/tests}/test.rs (100%)
 create mode 100644 cookie-store/Cargo.toml
 rename src/cookie_store.rs => cookie-store/src/lib.rs (98%)
 create mode 100644 memory-store/Cargo.toml
 rename src/memory_store.rs => memory-store/src/lib.rs (97%)

diff --git a/Cargo.toml b/Cargo.toml
index e3b9a4c..8d0a3ac 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,43 +1,3 @@
-[package]
-name = "async-session"
-version = "3.0.0"
-license = "MIT OR Apache-2.0"
-repository = "https://github.com/http-rs/async-session"
-documentation = "https://docs.rs/async-session"
-description = "Async session support with pluggable stores"
-readme = "README.md"
-edition = "2021"
-keywords = []
-categories = []
-authors = [
-  "Yoshua Wuyts <yoshuawuyts@gmail.com>",
-  "Jacob Rothstein <hi@jbr.me>"
-]
-
-[features]
-default = ["memory-store", "cookie-store"]
-memory-store = ["dashmap", "thiserror", "log"]
-cookie-store = ["bincode-json", "thiserror"]
-
-[dependencies]
-async-trait = "0.1.64"
-rand = "0.8.5"
-base64 = "0.21.0"
-serde_json = "1.0.93"
-blake3 = "1.3.3"
-log = { version = "0.4.17", optional = true }
-dashmap = { version = "5.4.0", optional = true }
-bincode-json = { version = "0.1.5", features = ["json"], optional = true }
-thiserror = { version = "1.0.38", optional = true }
-
-[dependencies.serde]
-version = "1.0.152"
-features = ["derive"]
-
-[dependencies.time]
-version = "0.3.18"
-features = ["serde"]
-
-[dev-dependencies.async-std]
-version = "1.12.0"
-features = ["attributes"]
+[workspace]
+resolver = "2"
+members = ["async-session", "memory-store", "cookie-store"]
diff --git a/async-session/Cargo.toml b/async-session/Cargo.toml
new file mode 100644
index 0000000..8f56feb
--- /dev/null
+++ b/async-session/Cargo.toml
@@ -0,0 +1,37 @@
+[package]
+name = "async-session"
+version = "3.0.0"
+license = "MIT OR Apache-2.0"
+repository = "https://github.com/http-rs/async-session"
+documentation = "https://docs.rs/async-session"
+description = "Async session support with pluggable stores"
+readme = "README.md"
+edition = "2021"
+keywords = []
+categories = []
+authors = [
+  "Yoshua Wuyts <yoshuawuyts@gmail.com>",
+  "Jacob Rothstein <hi@jbr.me>"
+]
+
+[dependencies]
+async-trait = "0.1.64"
+rand = "0.8.5"
+base64 = "0.21.0"
+serde_json = "1.0.93"
+blake3 = "1.3.3"
+
+[dependencies.serde]
+version = "1.0.152"
+features = ["derive"]
+
+[dependencies.time]
+version = "0.3.18"
+features = ["serde"]
+
+[dev-dependencies.async-std]
+version = "1.12.0"
+features = ["attributes"]
+
+[dev-dependencies]
+async-session-memory-store = {path = "../memory-store"}
diff --git a/src/lib.rs b/async-session/src/lib.rs
similarity index 82%
rename from src/lib.rs
rename to async-session/src/lib.rs
index e326cee..192eaf2 100644
--- a/src/lib.rs
+++ b/async-session/src/lib.rs
@@ -8,7 +8,8 @@
 //! # Example
 //!
 //! ```
-//! use async_session::{Session, SessionStore, MemoryStore};
+//! use async_session::{Session, SessionStore};
+//! use async_session_memory_store::MemoryStore;
 //!
 //! # fn main() -> Result<(), Box<dyn std::error::Error>> {
 //! # async_std::task::block_on(async {
@@ -46,17 +47,9 @@
     unused_qualifications
 )]
 
-#[cfg(feature = "cookie-store")]
-mod cookie_store;
-#[cfg(feature = "memory-store")]
-mod memory_store;
 mod session;
 mod session_store;
 
-#[cfg(feature = "cookie-store")]
-pub use cookie_store::{CookieStore, CookieStoreError};
-#[cfg(feature = "memory-store")]
-pub use memory_store::{MemoryStore, MemoryStoreError};
 pub use session::Session;
 pub use session_store::SessionStore;
 
diff --git a/src/session.rs b/async-session/src/session.rs
similarity index 100%
rename from src/session.rs
rename to async-session/src/session.rs
diff --git a/src/session_store.rs b/async-session/src/session_store.rs
similarity index 100%
rename from src/session_store.rs
rename to async-session/src/session_store.rs
diff --git a/tests/test.rs b/async-session/tests/test.rs
similarity index 100%
rename from tests/test.rs
rename to async-session/tests/test.rs
diff --git a/cookie-store/Cargo.toml b/cookie-store/Cargo.toml
new file mode 100644
index 0000000..eb63901
--- /dev/null
+++ b/cookie-store/Cargo.toml
@@ -0,0 +1,14 @@
+[package]
+name = "async-session-cookie-store"
+version = "0.0.0"
+edition = "2021"
+
+[dependencies]
+async-session = { path = "../async-session", version = "3.0.0" }
+base64 = "0.21.0"
+bincode-json = { version = "0.1.5", features = ["json"] }
+serde_json = "1.0.95"
+thiserror = "1.0.38"
+
+[dev-dependencies]
+async-std = { version = "1.12.0", features = ["attributes"] }
diff --git a/src/cookie_store.rs b/cookie-store/src/lib.rs
similarity index 98%
rename from src/cookie_store.rs
rename to cookie-store/src/lib.rs
index a170c7e..fcf9c97 100644
--- a/src/cookie_store.rs
+++ b/cookie-store/src/lib.rs
@@ -1,4 +1,4 @@
-use crate::{async_trait, Session, SessionStore};
+use async_session::{async_trait, Session, SessionStore};
 use base64::{engine::general_purpose::STANDARD as BASE64, Engine};
 
 /// A session store that serializes the entire session into a Cookie.
diff --git a/memory-store/Cargo.toml b/memory-store/Cargo.toml
new file mode 100644
index 0000000..340c1b8
--- /dev/null
+++ b/memory-store/Cargo.toml
@@ -0,0 +1,17 @@
+[package]
+name = "async-session-memory-store"
+version = "0.0.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+async-session = { path = "../async-session", version = "3.0.0" }
+log = "0.4.17"
+dashmap = "5.4.0"
+thiserror = "1.0.40"
+base64 = "0.21.0"
+serde_json = "1.0.95"
+
+[dev-dependencies]
+async-std = { version = "1.12.0", features = ["attributes"] }
diff --git a/src/memory_store.rs b/memory-store/src/lib.rs
similarity index 97%
rename from src/memory_store.rs
rename to memory-store/src/lib.rs
index f8cb4f8..acd20db 100644
--- a/src/memory_store.rs
+++ b/memory-store/src/lib.rs
@@ -1,4 +1,4 @@
-use crate::{async_trait, Session, SessionStore};
+use async_session::{async_trait, Session, SessionStore};
 use dashmap::{mapref::entry::Entry::Occupied, DashMap};
 use std::sync::Arc;
 
@@ -92,7 +92,8 @@ impl MemoryStore {
     /// returns the number of elements in the memory store
     /// # Example
     /// ```rust
-    /// # use async_session::{MemoryStore, Session, SessionStore};
+    /// # use async_session::{Session, SessionStore};
+    /// # use async_session_memory_store::MemoryStore;
     /// # fn main() -> Result<(), Box<dyn std::error::Error>> { async_std::task::block_on(async {
     /// let mut store = MemoryStore::new();
     /// assert_eq!(store.count(), 0);