-
Notifications
You must be signed in to change notification settings - Fork 752
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
[ISSUE-2779] Add user in session #2986
Changes from all commits
5d4fdf3
0e36ffd
8e623d4
414cb1f
5eebf25
e0e5718
42d5888
99d9252
eeeb8cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ pub struct MutableStatus { | |
abort: AtomicBool, | ||
current_database: RwLock<String>, | ||
session_settings: RwLock<Settings>, | ||
current_user: RwLock<Option<String>>, | ||
#[ignore_malloc_size_of = "insignificant"] | ||
client_host: RwLock<Option<SocketAddr>>, | ||
#[ignore_malloc_size_of = "insignificant"] | ||
|
@@ -42,9 +43,10 @@ impl MutableStatus { | |
pub fn try_create() -> Result<Self> { | ||
Ok(MutableStatus { | ||
abort: Default::default(), | ||
current_user: Default::default(), | ||
client_host: Default::default(), | ||
current_database: RwLock::new("default".to_string()), | ||
session_settings: RwLock::new(Settings::try_create()?.as_ref().clone()), | ||
client_host: Default::default(), | ||
io_shutdown_tx: Default::default(), | ||
context_shared: Default::default(), | ||
}) | ||
|
@@ -72,6 +74,18 @@ impl MutableStatus { | |
*lock = db | ||
} | ||
|
||
// Set the current user after authentication | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice to add a unit test in test crate we have some MutableStatus tests there There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @BohuTANG thank you for remind, i've added a test case on the getter/setter for current user There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to test the behaviour of session's current user, i'd split another PR to add a current_user() function |
||
pub fn set_current_user(&self, user: String) { | ||
let mut lock = self.current_user.write(); | ||
*lock = Some(user); | ||
} | ||
|
||
// Get current user | ||
pub fn get_current_user(&self) -> Option<String> { | ||
let lock = self.current_user.read(); | ||
lock.clone() | ||
} | ||
|
||
pub fn get_settings(&self) -> Arc<Settings> { | ||
let lock = self.session_settings.read(); | ||
Arc::new(lock.clone()) | ||
|
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.
it will be the UserInfo in the future not only a user name?
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'm taking UserInfo at first but reverted it back to a simple user name, in the current code base UserInfo is a fat object:
the fields inside UserInfo might get mutated on the fly (like quota, privileges, even password), or mutated by another query instance. a conservative usage of UserInfo might be always fetching
UserInfo
from the metasrv, and make it transactional with the user-related operations.in the case about session privilege, we can construct a unique identifier for the privilege grants by user_name + client_address.