search

actix / actix-extras

A collection of additional crates supporting the actix and actix-web frameworks.
https://actix.rs
Apache License 2.0
780 stars 200 forks source link

`generate_session_key()` could be public #434

Closed kakserpom closed 3 months ago

kakserpom commented 5 months ago

I've implemented my own session store and I had to copy-paste generate_session_key() instead of just calling it.

VaRusLAN commented 5 months ago

SessionState should also be public. It's just a type alias but I also had to copy-paste it.

robjtede commented 4 months ago

Please open a different issue for the SessionState discussion.

I think the choice to keep it private is that it might not be a good choice for all session implementations and that it just happens to work well for the built-in storage impls.

Seeing the desire to re-use it in 3rd party impls is a good indicator that it should be publicl; I'd accept a PR.

robjtede commented 4 months ago

If someone picks this up and creates a PR I think we should change the implementation to use rand::DistString::sample_string() to remove some unwraps: https://docs.rs/rand/0.8/rand/distributions/trait.DistString.html

sjoqvist commented 1 month ago

I really appreciate this change, but why does the function have to be hidden specifically behind the redis-session feature? I'm using the function as I'm implementing my own session store, i.e. I'm not using Redis. redis-session introduces a total of 12 new dependencies in my Cargo.lock.

robjtede commented 1 month ago

@sjoqvist removed the feature guard in v0.10.1