This implements a SessionStore that that uses Moka, a high-quality in-process concurrent memory cache. The store takes another SessionStore as an argument when created, which it acts as a wrapper for. It will act as a write-trough cache: when loading a session it will read from the cache and only reach out to the wrapped SessionStore if there is a cache miss. When saving it will put the value both into the cache and push out to the wrapped SessionStore. So essentially it saves a database roundtrip when you load a session, since typically the value will be in the cache. It also intentionally caches negative lookups, to avoid having to hit the database if the same non-existant session id is checked multiple times.

I also made it possible to use it as a pure memory store, since it only took a couple of extra lines of code to do so.

I encountered an issue in 0.2.0 where removing a value from the session would not be considered a change and therefore not saved back into the store.

This problem only shows when the removed value was the very last one in the session's data, leaving it empty.

Here is a minimal example of the problem:

use tower_sessions::Session;

fn main() {
    let session = Session::default();
    assert!(!session.modified());

    session.insert("foo", 42);
    assert!(session.modified());
    //      ^^^^^^^^^^^^^^^^^^^ returns `true`, as expected

    session.remove_value("foo");
    assert!(session.modified());
    //      ^^^^^^^^^^^^^^^^^^^ suddenly returns `false`, despite the internal `modified` flag being set
}

Other methods like session.clear() and session.flush() can trigger this issue.

My use case was that I was removing a user_id field from the session to implement a logout endpoint.
This issue caused the logout button to effectively do nothing, since the removal wasn't persisted anymore.

This implements CachingSessionStore, which holds two SessionStores. One acts as a frontend cache, while the other asks as the backend persistence store.

Anything that implements SessionStore may be used as either the cache or the store.

We also refactor our Moka implementation as it can now be used for layered caching directly in via CachingSessionStore.

I am using https://github.com/maxcountryman/tower-sessions/blob/main/examples/moka-postgres-store.rs

Without the moka store, inserting then fetching the value is no problem, everything is updated as it should, however the design of insert must invalidate the cache, right now I tried to use the example and session.insert and later in another route fetching the data gets you the stale cached value.

I don't know exactly whether this is a good title for the issue but I'm wondering how you manage to run tests for all databases without the hassle of setting them all up separately.

When trying to use a RedisStore inside a CachingSessionStore, it fails, because RedisStore doesn't implement debug.

Code

let config = RedisConfig::from_url(&CONFIG.redis_url)?;
let perf = PerformanceConfig::default();
let policy = ReconnectPolicy::default();
let client = RedisClient::new(config, Some(perf), Some(policy));

let redis_conn = client.connect();
client.wait_for_connect().await?;

let moka_store = MokaStore::new(Some(2000));
let redis_store = RedisStore::new(client);
let caching_store = CachingSessionStore::new(moka_store, redis_store);

let session_layer = ServiceBuilder::new()
	.layer(HandleErrorLayer::new(|_: BoxError| async {
		StatusCode::BAD_REQUEST
	}))
	.layer(
		SessionManagerLayer::new(caching_store)
			.with_secure(true)
			.with_max_age(time::Duration::seconds(10)),
	);

Error

error[E0277]: `RedisStore` doesn't implement `std::fmt::Debug`
   --> apps/appealsy-api/src/lib.rs:49:38
    |
49  |             SessionManagerLayer::new(caching_store)
    |             ------------------------ ^^^^^^^^^^^^^ `RedisStore` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
    |             |
    |             required by a bound introduced by this call
    |
    = help: the trait `std::fmt::Debug` is not implemented for `RedisStore`
    = help: the trait `SessionStore` is implemented for `CachingSessionStore<Cache, Store>`
    = note: required for `CachingSessionStore<MokaStore, RedisStore>` to implement `SessionStore`
note: required by a bound in `SessionManagerLayer::<Store>::new`
   --> /home/carter/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tower-sessions-0.2.1/src/service.rs:255:13
    |
255 | impl<Store: SessionStore> SessionManagerLayer<Store> {
    |             ^^^^^^^^^^^^ required by this bound in `SessionManagerLayer::<Store>::new`
...
267 |     pub fn new(session_store: Store) -> Self {
    |            --- required by a bound in this associated function

error[E0599]: the method `with_secure` exists for struct `SessionManagerLayer<CachingSessionStore<MokaStore, RedisStore>>`, but its trait bounds were not satisfied
  --> apps/appealsy-api/src/lib.rs:50:18
   |
49 | /             SessionManagerLayer::new(caching_store)
50 | |                 .with_secure(true)
   | |                 -^^^^^^^^^^^ method cannot be called due to unsatisfied trait bounds
   | |_________________|
   | 
   |
  ::: /home/carter/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tower-sessions-0.2.1/src/session_store.rs:59:1
   |
59 |   pub struct CachingSessionStore<Cache: SessionStore, Store: SessionStore> {
   |   ------------------------------------------------------------------------ doesn't satisfy `_: SessionStore`
   |
   = note: the following trait bounds were not satisfied:
           `CachingSessionStore<MokaStore, RedisStore>: SessionStore`

error[E0277]: `RedisStore` doesn't implement `std::fmt::Debug`
   --> apps/appealsy-api/src/lib.rs:49:13
    |
49  |             SessionManagerLayer::new(caching_store)
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `RedisStore` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug`
    |
    = help: the trait `std::fmt::Debug` is not implemented for `RedisStore`
    = help: the trait `SessionStore` is implemented for `CachingSessionStore<Cache, Store>`
    = note: required for `CachingSessionStore<MokaStore, RedisStore>` to implement `SessionStore`
note: required by a bound in `SessionManagerLayer`
   --> /home/carter/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tower-sessions-0.2.1/src/service.rs:154:39
    |
154 | pub struct SessionManagerLayer<Store: SessionStore> {
    |                                       ^^^^^^^^^^^^ required by this bound in `SessionManagerLayer`

Bumps fred from 6.3.1 to 6.3.2.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

This moves to a pattern of tracking loaded sessions via a hash set. By doing so, we can share sessions concurrently. In other words, we no longer need a lock to protect the session for the duration of the request and instead are able to share sessions between concurrent tasks.

Why do we need this at all? A data race can otherwise occur when a session has been loaded but its modifications have not been stored. For instance, another task may load the session from the store in the meantime and this load will overwrite pending modifications as it will effectively replace the session in memory.

One solution to this problem is to lock the request, as we've done in a stopgap capacity. This unscalable at present and even when refined, i.e. to lock per session as opposed for all sessions, limits concurrency over the session itself. In practice, sessions can only be operated on serially and are not available concurrently.

A more flexible approach is implemented here, allowing for concurrent memory access by limiting loads from the store to situations where no session exists in the loaded set. Put another way, sessions are tracked such that they can be used instead of loading from the store thus ensuring that pending modifications are not lost by a load.

See #36 for more details about the stopgap locking behavior; this replaces that approach altogether.

This is a temporary fix for a data race that occurs when a session is saved and loaded at the same time. The result of this race is that stale data can be loaded into memory, resulting in potential data loss where multiple requests using the same session are issued simultaneously.

Here we address this by holding a lock for the duration of the request. This solution is only temporary, as the underlying session implementation can instead be modified to ensure safe access between the save and load await points.

This supercedes #35.

This fixes a bug where session data could be lost if a session is loaded from the store while it's also being saved back to the store. Because sessions are loaded and stored in memory at the beginning of a request, it was previously possible for a session's inner value to be correctly altered but for that modification to be lost between the moment the session is saved to the store and when it's loaded. In other words, the data loaded would not reflect the in-progress update and instead be stale.

To address this we could hold a lock for the duration of the request. However, this imposes a performance penalty we would like to avoid. Alternatively we can manage a registry of sessions, a la tower-cookie's management of CookieJar: with this, we load a session first from the service's cache of sessions and fallback to the store only when the session is not present.

Currently sessions are managed like this via DashMap. However, alternatively caching facilities, such as Moka, might prove to be a better fit. Further, the current implementation does not bound the total number of cached sessions--this should be addressed in future updates. At present, this means memory will continue to grow, albeit typically at a very small rate; this is a memory leak!

Because sessions are safely written to and read from this cache, we prevent stale reads and thus a data race which could lead to data loss in highly concurrent environments, especially where multiple requests are made using the same session cookie simultaneously.

Note that replace_if_equal is still required to prevent data races in application code.

Updates the requirements on fred to permit the latest version.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Right now the schema_name for the SQLX stores is hardcoded to be "tower_sessions", ignoring the database specified in the database URL.

My current Docker setup creates a user with access to only one database. It would be useful to to be able to have it use that specific database.

Tower-sessions offers various sessions storages backed by relational databases. It only supports sqlx based connections for these stores. It would be great to allow other database connection types like that one provided by diesel/diesel-async as well, as users might prefer these for various reasons.