-
-
Notifications
You must be signed in to change notification settings - Fork 32
feat: Support Multiple session stores #277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
feat: Support Multiple session stores #277
Conversation
Codecov ReportAttention: Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
f8e50cd
to
3c6dbd6
Compare
@m4tx @seqre this PR is not fully ready, but I'd love some initial feedback if this design is heading in the right direction or makes any sense to you at all. Example usagesNo session storage specified(using the admin example)when the session_store is not provided, it defaults to the struct AdminProject;
impl Project for AdminProject {
...
fn config(&self, _config_name: &str) -> cot::Result<ProjectConfig> {
Ok(ProjectConfig::builder()
.debug(true)
.database(
DatabaseConfig::builder()
.url("sqlite://db.sqlite3?mode=rwc")
.build(),
)
.auth_backend(AuthBackendConfig::Database)
.middlewares(
MiddlewareConfig::builder()
.session(SessionMiddlewareConfig::builder()
.secure(false)
.build())
.build(),
)
.build())
}
...
} Session storage provided using MemoryStore provided by Cotstruct AdminProject;
impl Project for AdminProject {
...
fn config(&self, _config_name: &str) -> cot::Result<ProjectConfig> {
Ok(ProjectConfig::builder()
.debug(true)
.database(
DatabaseConfig::builder()
.url("sqlite://db.sqlite3?mode=rwc")
.build(),
)
.auth_backend(AuthBackendConfig::Database)
.middlewares(
MiddlewareConfig::builder()
.session(SessionMiddlewareConfig::builder()
.secure(false)
.session_store(Arc::new(MemoryStore::default()))
.build())
.build(),
)
.build())
}
...
} (Also haven't played around custom SessionStores yet) |
@ElijahAhianyo I think we should do this similarly to the Auth backend, i.e. the Lines 335 to 351 in 70995f0
Lines 1240 to 1241 in 70995f0
It's not ideal (mainly because it inflates the number of methods in the |
Based on your suggestion, I did some research and now have a clearer picture. My plan is to start by sketching out what the TOML file might look like, then work my way down into implementation details. I looked at how other frameworks handle this and identified two main patterns we canexplore: 1 a. Self-contained store configYou declare the store type and provide any necessary connection details (or file paths, in the case of file storage): secret_key = "{{ dev_secret_key }}"
[database]
url = "sqlite://db.sqlite3?mode=rwc"
[auth_backend]
type = "database"
[middlewares]
live_reload.enabled = true
[middlewares.session]
secure = false
[middlewares.session.store]
type = "redis"
connection = "redis://" 1 b Per-store subsectionThe approach in 1.a could get quite noisy(or maybe not) if different store types have some configs specific to them. secret_key = "{{ dev_secret_key }}"
[database]
url = "sqlite://db.sqlite3?mode=rwc"
[auth_backend]
type = "database"
[middlewares]
live_reload.enabled = true
[middlewares.session]
secure = false
[middlewares.session.store]
type = "file"
[middlewares.session.store.file]
dir = "/tmp/"
2. Named connectionsThe first pattern forces you to re-declare connection details even if you’ve already defined them under [databases] or [caches](currently not implemented). It also assumes a single DB/cache backend. We may need to structurally change the API to support this: secret_key = "{{ dev_secret_key }}"
[databases]
default = "postgres://…"
[caches]
redis_main = "redis://…"
[middlewares.session]
store = "redis"
connection = "cache.redis_main" # or, alternatively, use a separate `connection_source` field instead of dotted notation Option 1 should be easy to implement given our current design. However, with option 2, do we have any plans to support multiple DBs or caches? @m4tx let me also know if you've got other opinions on what the TOML should look like. |
Ah, that's a good question. Definitely yes, but certainly not in the near future. After some consideration, I think it would make sense to have some sort of hybrid between 1a and 2. I can imagine the following typical use cases for the session backends:
If we want to store sessions externally, we probably should already have an API for that. This doesn't apply for in-memory or file stores (because they're mainly development-focused anyway), but we already have a way to define a DB connection in the config (for use with the Cot's ORM) and the framework user shouldn't need to provide the same credentials twice, but the session store should rather use the Cot's ORM directly. The same should be true for Redis/Mongo/whatever cache – we don't have any Cache API at the moment, but I think we should at some point. And when we have the Cache API, we should just be able to use it to implement session stores. So I think we could try to translate these use cases into Config files:
[middlewares.session.store]
type = "memory" [middlewares.session.store]
type = "file"
path = "sessions.db"
[middlewares.session.store]
type = "database" # will just use Cot's ORM
For now, something like so should be good enough: [middlewares.session.store]
type = "redis"
url = "redis://localhost:6379" But, when Cot gets a dedicated Cache API with its own connection settings, the above can be just changed to: [middlewares.session.store]
type = "cache" # similarly to "database", this just means we'll use Cot's Cache API
name = "cache1" # optional; only needed if more than one cache is defined Do you think the above makes sense? Obviously, implementing a Cache API is out of scope for this PR, but if you'd like to do it as well, you're more than welcome to. |
@m4tx Yes, that makes sense. I can look into having a Cache API(we can create an issue to track this), in a separate PR—almost async with this one. Would you still want that designed with a singular backend in mind for now, and support multi later? |
@ElijahAhianyo actually, I think cases 1 and 3 from my answer can be merged together – there's nothing stopping us from implementing in-memory and file-backed caching. This would leave us with only two session backends that we would need to implement - I've created an issue to track the Cache API: #308.
I think having just one backend is 100% fine for now - we can work on this to add support for multiple ones later, as this doesn't sound like an everyday use case anyways. |
@m4tx I see, just so we're on the same page on what the TOML file should look like in the context of this PR, If we have two(
[middlewares.session.store]
type = "cache"
cache_type = "redis" # very verbose
url = "redis://localhost:6379"
[middlewares.session.store]
type = "cache"
url = "redis://localhost:6379" # will be infered as redis store from the uri
|
@ElijahAhianyo I think option number 2 should be good enough, as it doesn't duplicate the data in the config, and it will be consistent with the ORM behavior. When creating the Cache API, we just need to make sure it's possible to register new cache backends (for instance, the cache backend may need to provide all the URL schemas it supports, and our code would just try to match the URL to all registered backends). By the way, just to be sure, this is fine for now: [middlewares.session.store]
type = "cache"
url = "redis://localhost:6379" But eventually we'll want to have something closer to this (when we have the Cache API): [cache]
URL = "redis://localhost:6379"
[middlewares.session.store]
type = "cache" |
1c7763d
to
fe3713b
Compare
}, | ||
#[cfg(feature = "db")] | ||
Self::Database => { | ||
unimplemented!(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ended up moving the DB implementation into another PR since there are some nuances to it; The DB option requires a migration model and generated migration, which I haven't gotten to the bottom of
This Pr lays the foundation for supporting multiple session stores. Currently, there are 4 supported store types:
Memory, Cache, File and DB(implementation to be added in a follow up PR).
Memory
This is the default store which stores the session data in a thread-safe hashmap. It is suitable for development environments
Toml Example
Config Example
File Store
The file store persists sessions in a specified directory as files on a file system.
Toml Example
Config Example
Cache Store
The cache store uses a configured cache backed to persist data. This PR ships with support for a Redis backend. Support for other cache backends will be added in follow up PRs.
The cache store is gated behind the
cache
feature, while the redis implementation is gated behind theredis
feature. To use the redis cache store, you will have to enable both.Toml Example
Config Example
DB store
The DB store uses Cot's ORM to persist session data, Its implementation details will be added in a follow up PR.