Skip to content

Commit

Permalink
Fix user search duplicate bug
Browse files Browse the repository at this point in the history
  • Loading branch information
amirRamirfatahi committed Dec 20, 2024
1 parent 23b1dbb commit 16eeb04
Show file tree
Hide file tree
Showing 4 changed files with 239 additions and 0 deletions.
69 changes: 69 additions & 0 deletions src/db/kv/index/hashmap.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
use redis::AsyncCommands;

use crate::{db::connectors::redis::get_redis_conn, types::DynError};

/// puts multiple values into a hashmap in Redis
///
/// # Arguments
///
/// * `prefix` - A string slice that represents the prefix for the Redis key.
/// * `key` - A string slice that represents the key under which the value is stored.
/// * `values` - A vector of tuples containing the field and value to be stored.
///
/// # Errors
///
/// Returns an error if the operation fails.
pub async fn put_index_hashmap(
prefix: &str,
key: &str,
values: &[(&str, &str)],
) -> Result<(), DynError> {
let mut conn = get_redis_conn().await?;
let index_key = format!("{}:{}", prefix, key);
let _: () = conn.hset_multiple(index_key, values).await?;
Ok(())
}

/// gets a value from a hashmap in Redis
///
/// # Arguments
///
/// * `prefix` - A string slice that represents the prefix for the Redis key.
/// * `key` - A string slice that represents the key under which the value is stored.
/// * `field` - A string slice that represents the field under which the value is stored.
///
/// # Errors
///
/// Returns an error if the operation fails.
///
/// # Returns
///
/// Returns an Option containing the value if it exists, or None if it does not.
pub async fn get_index_hashmap(
prefix: &str,
key: &str,
field: &str,
) -> Result<Option<String>, DynError> {
let mut conn = get_redis_conn().await?;
let index_key = format!("{}:{}", prefix, key);
let value: Option<String> = conn.hget(index_key, field).await?;
Ok(value)
}

/// deletes a value from a hashmap in Redis
///
/// # Arguments
///
/// * `prefix` - A string slice that represents the prefix for the Redis key.
/// * `key` - A string slice that represents the key under which the value is stored.
/// * `field` - A string slice that represents the field under which the value is stored.
///
/// # Errors
///
/// Returns an error if the operation fails.
pub async fn del_index_hashmap(prefix: &str, key: &str, field: &str) -> Result<(), DynError> {
let mut conn = get_redis_conn().await?;
let index_key = format!("{}:{}", prefix, key);
let _: () = conn.hdel(index_key, field).await?;
Ok(())
}
1 change: 1 addition & 0 deletions src/db/kv/index/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/// Module for redis Indexing operations split into modules by Redis types
pub mod hashmap;
pub mod json;
pub mod lists;
pub mod sets;
Expand Down
66 changes: 66 additions & 0 deletions src/db/kv/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -679,4 +679,70 @@ pub trait RedisOps: Serialize + DeserializeOwned + Send + Sync {
let key = key_parts.join(":");
sorted_sets::get_lex_range("Sorted", &key, min, max, skip, limit).await
}

/// Puts multiple values into a hashmap in Redis.
///
/// # Arguments
///
/// * `key_parts` - A slice of string slices used to form the key under which the value is stored.
/// * `values` - A vector of tuples containing the field and value to be stored.
///
/// # Errors
///
/// Returns an error if the operation fails.
///
/// # Returns
///
/// Returns a `Result` indicating success or failure.
async fn put_index_hashmap(
key_parts: &[&str],
values: &[(&str, &str)],
) -> Result<(), DynError> {
let prefix = Self::prefix().await;
let key = key_parts.join(":");
hashmap::put_index_hashmap(&prefix, &key, values).await
}

/// Gets a value from a hashmap in Redis.
///
/// # Arguments
///
/// * `key_parts` - A slice of string slices used to form the key under which the value is stored.
/// * `field` - A string slice representing the field to retrieve from the hashmap.
///
/// # Returns
///
/// Returns an `Option<String>` containing the value if it exists, or `None` if the field does not exist.
///
/// # Errors
///
/// Returns an error if the operation fails.
async fn try_from_index_hashmap(
key_parts: &[&str],
field: &str,
) -> Result<Option<String>, DynError> {
let prefix = Self::prefix().await;
let key = key_parts.join(":");
hashmap::get_index_hashmap(&prefix, &key, field).await
}

/// Deletes a field from a hashmap in Redis.
///
/// # Arguments
///
/// * `key_parts` - A slice of string slices used to form the key under which the value is stored.
/// * `field` - A string slice representing the field to delete from the hashmap.
///
/// # Returns
///
/// Returns a `Result` indicating success or failure.
///
/// # Errors
///
/// Returns an error if the operation fails.
async fn remove_from_index_hashmap(key_parts: &[&str], field: &str) -> Result<(), DynError> {
let prefix = Self::prefix().await;
let key = key_parts.join(":");
hashmap::del_index_hashmap(&prefix, &key, field).await
}
}
103 changes: 103 additions & 0 deletions src/models/user/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use serde::{Deserialize, Serialize};
use utoipa::ToSchema;

pub const USER_NAME_KEY_PARTS: [&str; 2] = ["Users", "Name"];
pub const USER_NAME_HASHMAP_KEY_PARTS: [&str; 3] = ["Hashmap", "Users", "Name"];

#[derive(Serialize, Deserialize, ToSchema, Default)]
pub struct UserSearch(pub Vec<String>);
Expand Down Expand Up @@ -57,8 +58,19 @@ impl UserSearch {
///
/// This method takes a list of `UserDetails` and adds them all to the sorted set at once.
pub async fn put_to_index(details_list: &[&UserDetails]) -> Result<(), DynError> {
// 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?;

// Collect all the `username:user_id` pairs and their corresponding scores
let mut items: Vec<(f64, String)> = Vec::with_capacity(details_list.len());
let mut hashmap_items: Vec<(String, String)> = Vec::with_capacity(details_list.len());

for details in details_list {
// Convert the username to lowercase before storing
Expand All @@ -72,9 +84,19 @@ 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()));
items.push((score, member));
}

// put the items in hashmap for unique index
Self::put_index_hashmap(
&USER_NAME_HASHMAP_KEY_PARTS,
&hashmap_items
.iter()
.map(|(user_id, username)| (user_id.as_str(), username.as_str()))
.collect::<Vec<(&str, &str)>>(),
)
.await?;
// Perform a single Redis ZADD operation with all the items
Self::put_index_sorted_set(
&USER_NAME_KEY_PARTS,
Expand All @@ -87,4 +109,85 @@ impl UserSearch {
)
.await
}

async fn delete_existing_records(user_ids: &[&str]) -> Result<(), DynError> {
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?;
if let Some(existing_record) = existing_record {
let search_key = format!("{}:{}", existing_record, user_id);
records_to_delete.push(search_key);
}
}

Self::remove_from_index_sorted_set(
&USER_NAME_KEY_PARTS,
records_to_delete
.iter()
.map(|item| item.as_str())
.collect::<Vec<&str>>()
.as_slice(),
)
.await?;
Ok(())
}
}

#[cfg(test)]
mod tests {
use chrono::Utc;

use crate::{
models::user::{UserDetails, UserSearch},
types::{DynError, PubkyId},
};

#[tokio_shared_rt::test(shared)]
async fn test_put_to_index_no_duplicates() -> Result<(), DynError> {
// Test that the `put_to_index` method does not add duplicate records to the index
// when called with the same `UserDetails` multiple times.

// Create a `UserDetails` object
let user_id = "user_id";
let user_name = "Test User Duplicate";
let user_details = UserDetails {
id: PubkyId(user_id.to_string()),
name: user_name.to_string(),
bio: None,
status: None,
links: None,
image: None,
indexed_at: Utc::now().timestamp_millis(),
};

// Call `put_to_index` with the same `UserDetails` object multiple times
UserSearch::put_to_index(&[&user_details]).await?;

// Check that the index contains only one record for the user
let search_result = UserSearch::get_by_name(&user_name, None, None).await?;
assert_eq!(search_result.unwrap().0, vec![user_id.to_string()]);

let new_user_name = "Test User Duplicate 2";
let new_user_details = UserDetails {
id: PubkyId(user_id.to_string()),
name: new_user_name.to_string(),
bio: None,
status: None,
links: None,
image: None,
indexed_at: Utc::now().timestamp_millis(),
};

// Call `put_to_index` with new user details
UserSearch::put_to_index(&[&new_user_details]).await?;

// Check the previous record is deleted
// Check that the index contains only one record for the user
let search_result = UserSearch::get_by_name(&user_name, None, None).await?;
let empty_result: Vec<String> = vec![];
assert_eq!(search_result.unwrap().0, empty_result);

Ok(())
}
}

0 comments on commit 16eeb04

Please sign in to comment.