simplify UI preferences change logic

I copied the example of the password field by introducing a setter.
But I forgot: it was only that way because the password field has
the complexity of hashing/salting. For fields where setting is
idempotent, it can be directly exposed.
This commit is contained in:
Scott Lamb 2021-09-01 21:17:44 -07:00
parent c42314edb5
commit 070400095d
2 changed files with 14 additions and 25 deletions

View File

@ -90,7 +90,7 @@ impl User {
username: self.username.clone(), username: self.username.clone(),
flags: self.flags, flags: self.flags,
set_password_hash: None, set_password_hash: None,
set_preferences: None, preferences: self.preferences.clone(),
unix_uid: self.unix_uid, unix_uid: self.unix_uid,
permissions: self.permissions.clone(), permissions: self.permissions.clone(),
} }
@ -116,7 +116,7 @@ pub struct UserChange {
pub username: String, pub username: String,
pub flags: i32, pub flags: i32,
set_password_hash: Option<Option<String>>, set_password_hash: Option<Option<String>>,
set_preferences: Option<UserPreferences>, pub preferences: UserPreferences,
pub unix_uid: Option<i32>, pub unix_uid: Option<i32>,
pub permissions: Permissions, pub permissions: Permissions,
} }
@ -128,7 +128,7 @@ impl UserChange {
username, username,
flags: 0, flags: 0,
set_password_hash: None, set_password_hash: None,
set_preferences: None, preferences: UserPreferences::default(),
unix_uid: None, unix_uid: None,
permissions: Permissions::default(), permissions: Permissions::default(),
} }
@ -143,10 +143,6 @@ impl UserChange {
self.set_password_hash = Some(None); self.set_password_hash = Some(None);
} }
pub fn set_preferences(&mut self, preferences: UserPreferences) {
self.set_preferences = Some(preferences);
}
pub fn disable(&mut self) { pub fn disable(&mut self) {
self.flags |= UserFlag::Disabled as i32; self.flags |= UserFlag::Disabled as i32;
} }
@ -465,10 +461,6 @@ impl State {
::std::collections::btree_map::Entry::Vacant(_) => panic!("missing uid {}!", id), ::std::collections::btree_map::Entry::Vacant(_) => panic!("missing uid {}!", id),
::std::collections::btree_map::Entry::Occupied(e) => e, ::std::collections::btree_map::Entry::Occupied(e) => e,
}; };
let preferences = change.set_preferences.unwrap_or_else(|| {
let u = e.get();
u.preferences.clone()
});
{ {
let (phash, pid, pcount) = match change.set_password_hash.as_ref() { let (phash, pid, pcount) = match change.set_password_hash.as_ref() {
None => { None => {
@ -490,7 +482,7 @@ impl State {
":unix_uid": &change.unix_uid, ":unix_uid": &change.unix_uid,
":id": &id, ":id": &id,
":permissions": &permissions, ":permissions": &permissions,
":preferences": &preferences, ":preferences": &change.preferences,
})?; })?;
} }
let u = e.into_mut(); let u = e.into_mut();
@ -503,7 +495,7 @@ impl State {
u.flags = change.flags; u.flags = change.flags;
u.unix_uid = change.unix_uid; u.unix_uid = change.unix_uid;
u.permissions = change.permissions; u.permissions = change.permissions;
u.preferences = preferences; u.preferences = change.preferences;
Ok(u) Ok(u)
} }
@ -521,14 +513,13 @@ impl State {
.permissions .permissions
.write_to_bytes() .write_to_bytes()
.expect("proto3->vec is infallible"); .expect("proto3->vec is infallible");
let preferences = change.set_preferences.unwrap_or_default();
stmt.execute(named_params! { stmt.execute(named_params! {
":username": &change.username[..], ":username": &change.username[..],
":password_hash": &password_hash, ":password_hash": &password_hash,
":flags": &change.flags, ":flags": &change.flags,
":unix_uid": &change.unix_uid, ":unix_uid": &change.unix_uid,
":permissions": &permissions, ":permissions": &permissions,
":preferences": &preferences, ":preferences": &change.preferences,
})?; })?;
let id = conn.last_insert_rowid() as i32; let id = conn.last_insert_rowid() as i32;
self.users_by_name.insert(change.username.clone(), id); self.users_by_name.insert(change.username.clone(), id);
@ -547,7 +538,7 @@ impl State {
unix_uid: change.unix_uid, unix_uid: change.unix_uid,
dirty: false, dirty: false,
permissions: change.permissions, permissions: change.permissions,
preferences, preferences: change.preferences,
})) }))
} }
@ -1340,16 +1331,13 @@ mod tests {
db::init(&mut conn).unwrap(); db::init(&mut conn).unwrap();
let mut state = State::init(&conn).unwrap(); let mut state = State::init(&conn).unwrap();
let mut change = UserChange::add_user("slamb".to_owned()); let mut change = UserChange::add_user("slamb".to_owned());
let mut preferences = UserPreferences::default(); change.preferences.0.insert("foo".to_string(), 42.into());
preferences.0.insert("foo".to_string(), 42.into());
change.set_preferences(preferences.clone());
let u = state.apply(&conn, change).unwrap(); let u = state.apply(&conn, change).unwrap();
assert_eq!(preferences, u.preferences);
let mut change = u.change(); let mut change = u.change();
preferences.0.insert("bar".to_string(), 26.into()); change.preferences.0.insert("bar".to_string(), 26.into());
change.set_preferences(preferences.clone());
let u = state.apply(&conn, change).unwrap(); let u = state.apply(&conn, change).unwrap();
assert_eq!(preferences, u.preferences); assert_eq!(u.preferences.0.get("foo"), Some(&42.into()));
assert_eq!(u.preferences.0.get("bar"), Some(&26.into()));
let uid = u.id; let uid = u.id;
{ {
@ -1359,6 +1347,7 @@ mod tests {
} }
let state = State::init(&conn).unwrap(); let state = State::init(&conn).unwrap();
let u = state.users_by_id().get(&uid).unwrap(); let u = state.users_by_id().get(&uid).unwrap();
assert_eq!(preferences, u.preferences); assert_eq!(u.preferences.0.get("foo"), Some(&42.into()));
assert_eq!(u.preferences.0.get("bar"), Some(&26.into()));
} }
} }

View File

@ -1042,7 +1042,7 @@ impl Service {
if let Some(update) = r.update { if let Some(update) = r.update {
let mut change = user.change(); let mut change = user.change();
if let Some(preferences) = update.preferences { if let Some(preferences) = update.preferences {
change.set_preferences(preferences); change.preferences = preferences;
} }
db.apply_user_change(change).map_err(internal_server_err)?; db.apply_user_change(change).map_err(internal_server_err)?;
} }