Fix: user search duplicate bug (#260)

* Fix user search

* fix file urls serde

src/db/kv/index/json.rs

@@ -358,22 +358,24 @@ pub async fn get_multiple<T: DeserializeOwned + Send + Sync>(
     let results: Vec<Option<T>> = if indexed_values.is_empty() {
         (0..keys.len()).map(|_| None).collect()
     } else {
-        deserialize_values(indexed_values)
+        deserialize_values(indexed_values)?
     };
 
     Ok(results)
 }
 
 // Helper function to deserialize JSON strings to Vec<Option<T>>
-fn deserialize_values<T: DeserializeOwned>(values: Vec<Option<String>>) -> Vec<Option<T>> {
+fn deserialize_values<T: DeserializeOwned>(
+    values: Vec<Option<String>>,
+) -> Result<Vec<Option<T>>, DynError> {
     values
         .into_iter()
-        .map(|opt| {
-            opt.and_then(|value_str| {
-                serde_json::from_str::<Vec<T>>(&value_str)
-                    .ok()
-                    .and_then(|vec| vec.into_iter().next())
-            })
+        .map(|value_str| match value_str {
+            Some(value) => {
+                let value: Vec<T> = serde_json::from_str(&value)?;
+                Ok(value.into_iter().next())
+            }
+            None => Ok(None),
         })
         .collect()
 }

src/models/file/details.rs

@@ -16,26 +16,24 @@ pub struct FileUrls {
 }
 
 mod json_string {
-    use serde::{self, Deserialize, Deserializer, Serialize, Serializer};
+    use serde::{self, Deserialize, Deserializer, Serializer};
 
-    // Deserialize function: convert the JSON string into a struct
-    pub fn deserialize<'de, D, T>(deserializer: D) -> Result<T, D::Error>
+    pub fn serialize<S, T>(value: &T, serializer: S) -> Result<S::Ok, S::Error>
     where
-        D: Deserializer<'de>,
-        T: Deserialize<'de>,
+        S: Serializer,
+        T: serde::Serialize,
     {
-        let json_str: &'de str = <&str>::deserialize(deserializer)?;
-        serde_json::from_str(json_str).map_err(serde::de::Error::custom)
+        let json_string = serde_json::to_string(value).map_err(serde::ser::Error::custom)?;
+        serializer.serialize_str(&json_string)
     }
 
-    // Serialize function: convert the struct back into a JSON string
-    pub fn serialize<S, T>(value: &T, serializer: S) -> Result<S::Ok, S::Error>
+    pub fn deserialize<'de, D, T>(deserializer: D) -> Result<T, D::Error>
     where
-        S: Serializer,
-        T: Serialize,
+        D: Deserializer<'de>,
+        T: serde::de::DeserializeOwned,
     {
-        let json_str = serde_json::to_string(value).map_err(serde::ser::Error::custom)?;
-        serializer.serialize_str(&json_str)
+        let json_string = String::deserialize(deserializer)?;
+        serde_json::from_str(&json_string).map_err(serde::de::Error::custom)
     }
 }
 

src/models/user/details.rs

@@ -71,7 +71,10 @@ where
                 .map_err(serde::de::Error::custom)?;
             Ok(Some(urls))
         }
-        _ => Err(serde::de::Error::custom("Expected a string or an array")),
+        serde_json::Value::Null => Ok(None),
+        _ => Err(serde::de::Error::custom(
+            "Expected either a string, an array or null",
+        )),
     }
 }
 

src/models/user/search.rs

@@ -1,6 +1,6 @@
 use super::UserDetails;
-use crate::types::DynError;
 use crate::RedisOps;
+use crate::{models::traits::Collection, types::DynError};
 use serde::{Deserialize, Serialize};
 use utoipa::ToSchema;
 
@@ -57,6 +57,16 @@ 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());
 
@@ -87,4 +97,110 @@ impl UserSearch {
         )
         .await
     }
+
+    async fn delete_existing_records(user_ids: &[&str]) -> Result<(), DynError> {
+        if user_ids.is_empty() {
+            return Ok(());
+        }
+        let mut records_to_delete: Vec<String> = Vec::with_capacity(user_ids.len());
+        let keys = user_ids
+            .iter()
+            .map(|&id| vec![id])
+            .collect::<Vec<Vec<&str>>>();
+        let users = UserDetails::get_from_index(keys.iter().map(|item| item.as_slice()).collect())
+            .await?
+            .into_iter()
+            .flatten()
+            .collect::<Vec<UserDetails>>();
+        for user_id in user_ids {
+            let existing_username = users
+                .iter()
+                .find(|user| user.id.0 == *user_id)
+                .map(|user| user.name.to_lowercase());
+            if let Some(existing_record) = existing_username {
+                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::{
+            traits::Collection,
+            user::{UserDetails, UserSearch},
+        },
+        setup,
+        types::{DynError, PubkyId},
+        Config, RedisOps,
+    };
+
+    #[tokio_shared_rt::test(shared)]
+    async fn test_put_to_index_no_duplicates() -> Result<(), DynError> {
+        let config = Config::from_env();
+        setup(&config).await;
+        // 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(),
+        };
+
+        user_details.put_to_graph().await?;
+        user_details
+            .put_index_json(vec![user_id].as_slice(), None)
+            .await?;
+
+        // Call `put_to_index` with the same `UserDetails` object
+        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 = "Some Other User Name";
+        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?;
+        assert_eq!(search_result.is_none(), true);
+
+        Ok(())
+    }
 }