Fix user search duplicate bug #260
Conversation
There was a problem hiding this comment.
The proposed solution to this bug works.
However, the solution is overly complex. Adding a new Hashmap to find Usernames by pubky_id does not seem necessary. This lookup table is not needed as we already have methods to retrieve a username of any user by its pubky_id. We already have User:Details:{user_id} keys that we can access with UserDetails::get_by_id(pubky_id).username . This is enough to figure what username:pubky_id (if any) we should remove from the sorted set.
| let mut records_to_delete: Vec<String> = Vec::with_capacity(user_ids.len()); | ||
| for user_id in user_ids { | ||
| let existing_record = | ||
| Self::try_from_index_hashmap(&USER_NAME_HASHMAP_KEY_PARTS, user_id).await?; |
There was a problem hiding this comment.
Why not simply do UserDetails::get_bt_id(user_id) to figure what this user's current username (if any) is?
| // ensure existing records are deleted | ||
| Self::delete_existing_records( | ||
| details_list | ||
| .iter() | ||
| .map(|details| details.id.0.as_str()) | ||
| .collect::<Vec<&str>>() | ||
| .as_slice(), | ||
| ) | ||
| .await?; |
| @@ -72,9 +84,21 @@ impl UserSearch { | |||
| // The value in the sorted set will be `username:user_id` | |||
| let member = format!("{}:{}", username, user_id.0); | |||
|
|
|||
| hashmap_items.push((user_id.0.clone(), username.clone())); | |||
There was a problem hiding this comment.
Not really needed, unless we can proof some sort of performance gain by maintaining this hashmap vs retrieving the usernames from the existing Redis JSON per user. Even if there is some performance improvement: switching username by the watcher will be rare and not performance sensitive (no problem if it's slow as the watcher runs in the background).
Fixes #259
Pre-submission Checklist
cargo test.cargo bench