karma: leaderboard + rank, drop code-noise targets, atomic store writes

.gitignore

@@ -4,3 +4,4 @@ rustbot.service
 rustbot.conf
 weather.*.json
 karma.*.json
+*.json.tmp

README.md

@@ -68,7 +68,7 @@ lists them at runtime and `help <command>` describes one.
 |---|---|
 | `ping` / `echo <text>` / `hello` | basics |
 | `roll [NdM]` | roll dice (default `1d6`) |
-| `karma <thing>` (bump with `thing++` / `thing--` in chat) | show/track karma |
+| `karma [thing]` / `karmabottom` (bump with `thing++` / `thing--`) | karma + rank, or the top/bottom list |
 | `md5` / `sha1` / `sha256` / `hash <text>` | hex digests |
 | `add <location>` then `w [location]` | weather (wttr.in), per-user location |
 | `crypto <sym>` | coin price in USD (Bitstamp) |

src/bot/modules/karma.rs

@@ -5,11 +5,21 @@ use std::path::PathBuf;
 use crate::bot::module::{Action, Chat, Command, CommandSpec, Module};
 use crate::json::{self, Json};
 
-const COMMANDS: &[CommandSpec] = &[CommandSpec {
+const COMMANDS: &[CommandSpec] = &[
+    CommandSpec {
         name: "karma",
-    usage: "karma <thing>",
-    about: "show karma; bump it in chat with thing++ / thing--",
-}];
+        usage: "karma [thing]",
+        about: "karma of <thing> with rank, or the top list; bump with thing++ / thing--",
+    },
+    CommandSpec {
+        name: "karmabottom",
+        usage: "karmabottom",
+        about: "the lowest-karma things",
+    },
+];
+
+const MAX_THING_LEN: usize = 32;
+const TOP_N: usize = 5;
 
 pub struct Karma {
     store: Store,
@@ -34,6 +44,30 @@ impl Karma {
     pub fn get(&self, thing: &str) -> i64 {
         self.store.get(thing)
     }
+
+    fn report(&self, thing: &str) -> String {
+        if !self.store.has(thing) {
+            return format!("no karma for '{thing}' yet");
+        }
+        let (rank, total) = self.store.rank(thing);
+        format!(
+            "{thing} has {} karma (#{rank} of {total})",
+            self.store.get(thing)
+        )
+    }
+
+    fn leaderboard(&self, bottom: bool) -> String {
+        let entries = self.store.ranked(TOP_N, bottom);
+        if entries.is_empty() {
+            return "no karma yet".to_string();
+        }
+        let items: Vec<String> = entries.iter().map(|(k, v)| format!("{k} ({v})")).collect();
+        format!(
+            "karma {}: {}",
+            if bottom { "bottom" } else { "top" },
+            items.join(", ")
+        )
+    }
 }
 
 impl Module for Karma {
@@ -46,13 +80,15 @@ impl Module for Karma {
     }
 
     fn on_command(&mut self, cmd: &Command) -> Vec<Action> {
-        let Some(thing) = cmd.args.first() else {
-            return vec![Action::reply(format!("usage: {}karma <thing>", cmd.prefix))];
+        let reply = match cmd.name {
+            "karma" => match cmd.args.first() {
+                Some(thing) => self.report(thing),
+                None => self.leaderboard(false),
+            },
+            "karmabottom" => self.leaderboard(true),
+            _ => return vec![],
         };
-        vec![Action::reply(format!(
-            "{thing} has {} karma",
-            self.store.get(thing)
-        ))]
+        vec![Action::reply(reply)]
     }
 
     fn on_message(&mut self, chat: &Chat) -> Vec<Action> {
@@ -86,7 +122,7 @@ pub fn parse_changes(text: &str, sender: &str) -> Vec<(String, i64)> {
             continue;
         };
         let thing = thing.trim_start_matches('(').trim_end_matches(')');
-        if thing.is_empty() {
+        if !is_valid_target(thing) {
             continue;
         }
         let key = thing.to_lowercase();
@@ -100,6 +136,11 @@ pub fn parse_changes(text: &str, sender: &str) -> Vec<(String, i64)> {
     out
 }
 
+fn is_valid_target(thing: &str) -> bool {
+    let len = thing.chars().count();
+    (2..=MAX_THING_LEN).contains(&len) && !thing.chars().all(|c| c.is_ascii_digit())
+}
+
 struct Store {
     map: HashMap<String, i64>,
     path: PathBuf,
@@ -131,7 +172,12 @@ impl Store {
             .iter()
             .map(|(k, v)| format!("\"{}\":{}", json::escape(k), v))
             .collect();
-        fs::write(&self.path, format!("{{{}}}\n", items.join(",")))
+        let body = format!("{{{}}}\n", items.join(","));
+        let mut tmp = self.path.clone().into_os_string();
+        tmp.push(".tmp");
+        let tmp = PathBuf::from(tmp);
+        fs::write(&tmp, body)?;
+        fs::rename(&tmp, &self.path)
     }
 
     fn bump(&mut self, thing: &str, delta: i64) {
@@ -141,4 +187,30 @@ impl Store {
     fn get(&self, thing: &str) -> i64 {
         *self.map.get(&thing.to_lowercase()).unwrap_or(&0)
     }
+
+    fn has(&self, thing: &str) -> bool {
+        self.map.contains_key(&thing.to_lowercase())
+    }
+
+    fn ranked(&self, n: usize, bottom: bool) -> Vec<(String, i64)> {
+        let mut v: Vec<(String, i64)> = self.map.iter().map(|(k, val)| (k.clone(), *val)).collect();
+        v.sort_by(|a, b| b.1.cmp(&a.1).then(a.0.cmp(&b.0)));
+        if bottom {
+            v.reverse();
+        }
+        v.truncate(n);
+        v
+    }
+
+    fn rank(&self, thing: &str) -> (usize, usize) {
+        let key = thing.to_lowercase();
+        let mut v: Vec<(&String, &i64)> = self.map.iter().collect();
+        v.sort_by(|a, b| b.1.cmp(a.1).then(a.0.cmp(b.0)));
+        let rank = v
+            .iter()
+            .position(|(k, _)| **k == key)
+            .map(|i| i + 1)
+            .unwrap_or(0);
+        (rank, v.len())
+    }
 }

src/bot/modules/weather.rs

@@ -132,7 +132,12 @@ impl Store {
     }
 
     fn save(&self, path: &Path) -> io::Result<()> {
-        fs::write(path, to_json(&self.map) + "\n")
+        let body = to_json(&self.map) + "\n";
+        let mut tmp = path.to_path_buf().into_os_string();
+        tmp.push(".tmp");
+        let tmp = PathBuf::from(tmp);
+        fs::write(&tmp, body)?;
+        fs::rename(&tmp, path)
     }
 
     fn get(&self, nick: &str) -> Option<&String> {

tests/karma.rs

@@ -75,3 +75,68 @@ fn command_reports_karma() {
     assert!(out.contains("foo has 1 karma"), "{out}");
     let _ = std::fs::remove_file(&path);
 }
+
+fn reply_of(k: &mut Karma, name: &str, args: &[&str]) -> String {
+    let cmd = Command {
+        sender: "z",
+        reply_to: "#c",
+        name,
+        args,
+        prefix: ">",
+        network: "t",
+    };
+    match k.on_command(&cmd).as_slice() {
+        [Action::Reply(s)] => s.clone(),
+        _ => panic!("expected one reply"),
+    }
+}
+
+#[test]
+fn ignores_single_char_and_numeric_targets() {
+    assert_eq!(
+        parse_changes("i++ 5++ x-- ok++", "s"),
+        vec![("ok".to_string(), 1)]
+    );
+}
+
+#[test]
+fn report_shows_rank() {
+    let path = std::env::temp_dir().join("rubot-test-karma-rank.json");
+    let _ = std::fs::remove_file(&path);
+    let mut k = Karma::with_path(path.clone());
+    k.on_message(&chat("a", "alpha++"));
+    k.on_message(&chat("b", "alpha++"));
+    k.on_message(&chat("c", "alpha++ beta++"));
+    assert!(reply_of(&mut k, "karma", &["alpha"]).contains("alpha has 3 karma (#1 of 2)"));
+    assert!(reply_of(&mut k, "karma", &["beta"]).contains("beta has 1 karma (#2 of 2)"));
+    let _ = std::fs::remove_file(&path);
+}
+
+#[test]
+fn unseen_thing_reports_no_karma() {
+    let path = std::env::temp_dir().join("rubot-test-karma-unseen.json");
+    let _ = std::fs::remove_file(&path);
+    let mut k = Karma::with_path(path.clone());
+    assert!(reply_of(&mut k, "karma", &["ghost"]).contains("no karma for 'ghost' yet"));
+    let _ = std::fs::remove_file(&path);
+}
+
+#[test]
+fn leaderboard_top_and_bottom() {
+    let path = std::env::temp_dir().join("rubot-test-karma-top.json");
+    let _ = std::fs::remove_file(&path);
+    let mut k = Karma::with_path(path.clone());
+    k.on_message(&chat("a", "high++ mid++ low--"));
+    k.on_message(&chat("b", "high++"));
+    let top = reply_of(&mut k, "karma", &[]);
+    assert!(
+        top.starts_with("karma top:") && top.contains("high (2)"),
+        "{top}"
+    );
+    let bottom = reply_of(&mut k, "karmabottom", &[]);
+    assert!(
+        bottom.starts_with("karma bottom:") && bottom.contains("low (-1)"),
+        "{bottom}"
+    );
+    let _ = std::fs::remove_file(&path);
+}