rework users cache to handle banned users

src/app/cache.rs

@@ -4,45 +4,57 @@ use tracing::trace;
 
 const EXPIRY_INTERVAL: u64 = 600;
 
+// Banned users are stored as None
 #[derive(Clone, Default)]
 pub struct UsersCache {
-    store: Arc<DashMap<String, (Instant, String)>>, // User id, login name
+    ids: Arc<DashMap<String, (Instant, Option<String>)>>,
+    logins: Arc<DashMap<String, (Instant, Option<String>)>>,
 }
 
 impl UsersCache {
     pub fn insert(&self, id: String, name: String) {
-        let inserted_at = Instant::now();
-        self.store.insert(id, (inserted_at, name));
+        self.insert_optional(Some(id), Some(name));
     }
 
-    pub fn get_login(&self, id: &str) -> Option<String> {
-        self.store.get(id).and_then(|entry| {
+    pub fn insert_optional(&self, id: Option<String>, name: Option<String>) {
+        let inserted_at = Instant::now();
+
+        if let Some(id) = id.clone() {
+            self.ids.insert(id, (inserted_at, name.clone()));
+        }
+
+        if let Some(name) = name {
+            self.logins.insert(name, (inserted_at, id));
+        }
+    }
+
+    pub fn get_login(&self, id: &str) -> Option<Option<String>> {
+        if let Some(entry) = self.ids.get(id) {
             if entry.value().0.elapsed().as_secs() > EXPIRY_INTERVAL {
                 drop(entry);
                 trace!("Removing {id} from cache");
-                self.store.remove(id);
+                self.ids.remove(id);
                 None
             } else {
                 trace!("Using cached value for id {id}");
                 Some(entry.value().1.clone())
             }
-        })
+        } else {
+            None
+        }
     }
 
-    pub fn get_id(&self, name: &str) -> Option<String> {
-        // Iter has to be a separate variable to that it can be explicitly dropped to avoid deadlock
-        let mut store_iter = self.store.iter();
-        if let Some(entry) = store_iter.find(|entry| entry.value().1 == name) {
+    pub fn get_id(&self, name: &str) -> Option<Option<String>> {
+        if let Some(entry) = self.logins.get(name) {
             if entry.value().0.elapsed().as_secs() > EXPIRY_INTERVAL {
                 let key = entry.key().clone();
                 drop(entry);
-                drop(store_iter);
                 trace!("Removing {name} from cache");
-                self.store.remove(&key);
+                self.logins.remove(&key);
                 None
             } else {
                 trace!("Using cached value for name {name}");
-                Some(entry.key().clone())
+                Some(entry.value().1.clone())
             }
         } else {
             None

src/app/mod.rs

@@ -2,7 +2,6 @@ pub mod cache;
 
 use self::cache::UsersCache;
 use crate::{config::Config, error::Error, Result};
-use anyhow::Context;
 use dashmap::DashSet;
 use std::{collections::HashMap, sync::Arc};
 use tracing::debug;
@@ -29,18 +28,22 @@ impl App {
         let mut names_to_request = Vec::new();
 
         for id in ids {
-            if let Some(login) = self.users.get_login(&id) {
-                users.insert(id, login);
-            } else {
-                ids_to_request.push(id.into())
+            match self.users.get_login(&id) {
+                Some(Some(login)) => {
+                    users.insert(id, login);
+                }
+                Some(None) => (),
+                None => ids_to_request.push(id.into()),
             }
         }
 
         for name in names {
-            if let Some(id) = self.users.get_id(&name) {
-                users.insert(id, name);
-            } else {
-                names_to_request.push(name.into());
+            match self.users.get_id(&name) {
+                Some(Some(id)) => {
+                    users.insert(id, name);
+                }
+                Some(None) => (),
+                None => names_to_request.push(name.into()),
             }
         }
 
@@ -72,25 +75,40 @@ impl App {
             users.insert(id, login);
         }
 
+        // Banned users which were not returned by the api
+        for id in ids_to_request {
+            if !users.contains_key(id.as_str()) {
+                self.users.insert_optional(Some(id.into_string()), None);
+            }
+        }
+        for name in names_to_request {
+            if !users.values().any(|login| login == name.as_str()) {
+                self.users.insert_optional(None, Some(name.into_string()));
+            }
+        }
+
         Ok(users)
     }
 
     pub async fn get_user_id_by_name(&self, name: &str) -> Result<String> {
-        if let Some(id) = self.users.get_id(name) {
-            Ok(id)
-        } else {
-            let request = GetUsersRequest::builder().login(vec![name.into()]).build();
-            let response = self.helix_client.req_get(request, &*self.token).await?;
-            let user = response
-                .data
-                .into_iter()
-                .next()
-                .context("Could not get user")?;
-            let user_id = user.id.into_string();
-
-            self.users.insert(user_id.clone(), user.login.into_string());
-
-            Ok(user_id)
+        match self.users.get_id(name) {
+            Some(Some(id)) => Ok(id),
+            Some(None) => Err(Error::NotFound),
+            None => {
+                let request = GetUsersRequest::builder().login(vec![name.into()]).build();
+                let response = self.helix_client.req_get(request, &*self.token).await?;
+                match response.data.into_iter().next() {
+                    Some(user) => {
+                        let user_id = user.id.into_string();
+                        self.users.insert(user_id.clone(), user.login.into_string());
+                        Ok(user_id)
+                    }
+                    None => {
+                        self.users.insert_optional(None, Some(name.to_owned()));
+                        Err(Error::NotFound)
+                    }
+                }
+            }
         }
     }