chore: refactor backend around domain types (#26)

Reviewed-on: #26

crates/signed_core/src/inbox.rs

@@ -4,41 +4,29 @@ use std::time::Duration;
 use nostr::prelude::*;
 use serde::{Deserialize, Serialize};
 
-use crate::{RepoAddr, activity_subject};
+use crate::{GitEvent, RepoAddr};
 
-/// Window before `now` that an advanced cutoff retreats to.
 const ADVANCE_WINDOW: Duration = Duration::from_secs(3 * 24 * 60 * 60);
-
-/// Window before `now` that a mark-all cutoff retreats to.
 const MARK_ALL_WINDOW: Duration = Duration::from_secs(10 * 24 * 60 * 60);
 
-/// A thread of notification and own-activity events sharing one root.
 #[derive(Debug, Clone)]
 pub struct InboxItem {
-    /// The root issue, patch or pull request the events belong to.
     pub root: EventId,
-    /// The root event itself, when it is known locally.
     pub root_event: Option<Event>,
-    /// Repository the root belongs to, from the root's `a` tag.
     pub address: Option<RepoAddr>,
-    /// Notification events directed at the user, newest first.
     pub events: Vec<Event>,
-    /// The user's own events in the thread, newest first.
     pub own_events: Vec<Event>,
-    /// Unread event ids, oldest first.
     pub unread_ids: Vec<EventId>,
-    /// Whether every notification event in the thread is archived.
     pub archived: bool,
 }
 
 impl InboxItem {
-    /// Title of the thread, read from its root issue/patch/PR when known.
     pub fn title(&self) -> String {
         self.root_event
             .as_ref()
             .or_else(|| self.own_events.first())
             .or_else(|| self.events.first())
-            .map(activity_subject)
+            .map(|event| event.activity_subject())
             .unwrap_or_else(|| "Untitled".to_string())
     }
 
@@ -50,7 +38,6 @@ impl InboxItem {
             .map(|event| event.kind)
     }
 
-    /// Timestamp of the newest event in the thread.
     pub fn latest_activity(&self) -> Timestamp {
         self.root_event
             .as_ref()
@@ -62,7 +49,6 @@ impl InboxItem {
             .unwrap_or_default()
     }
 
-    /// Up to `limit` events of the thread, oldest first.
     pub fn timeline(&self, limit: usize) -> Vec<Event> {
         let mut seen: HashSet<EventId> = HashSet::new();
         let mut events: Vec<Event> = Vec::new();
@@ -92,7 +78,6 @@ impl InboxItem {
         events
     }
 
-    /// Whether the thread has an unread event still visible in the inbox.
     pub fn is_unread(&self) -> bool {
         !self.archived && !self.unread_ids.is_empty()
     }
@@ -112,44 +97,144 @@ impl InboxItem {
     }
 }
 
-/// Root issue, patch or pull request of a notification event.
-///
-/// Returns `None` when the event is not git-related, or when its root is a
-/// coordinate rather than an event.
-///
-/// - issue (1621) / PR (1618): itself
-/// - patch (1617): its `e` parent patch, else itself
-/// - NIP-22 comment (1111): uppercase `E` root pointer
-/// - PR update (1619): uppercase `E`
-/// - statuses (1630-1633): NIP-10 root `e`
-pub fn notification_root<L>(event: &Event, lookup: &L) -> Option<EventId>
+pub struct ThreadResolver<'a, L: ?Sized> {
+    lookup: &'a L,
+}
+
+impl<'a, L> ThreadResolver<'a, L>
 where
-    L: Fn(EventId) -> Option<Event>,
+    L: Fn(EventId) -> Option<Event> + ?Sized,
 {
-    match event.kind {
-        Kind::GitIssue | Kind::GitPullRequest => Some(event.id),
-        Kind::GitPatch => Some(match first_e_id(event) {
-            Some(parent) => resolve_thread_root(parent, lookup),
-            None => event.id,
-        }),
-        Kind::Comment => match nip22::extract_root(event) {
-            Some(CommentTarget::Event { id, .. }) => Some(resolve_thread_root(id, lookup)),
+    pub fn new(lookup: &'a L) -> Self {
+        Self { lookup }
+    }
+
+    // Kind → root mapping:
+    // - issue (1621) / PR (1618): itself
+    // - patch (1617): its `e` parent patch, else itself
+    // - NIP-22 comment (1111): uppercase `E` root pointer
+    // - PR update (1619): uppercase `E`
+    // - statuses (1630-1633): NIP-10 root `e`
+    // Returns `None` when the event is not git-related, or when its root is a
+    // coordinate rather than an event.
+    pub fn notification_root(&self, event: &Event) -> Option<EventId> {
+        match event.kind {
+            Kind::GitIssue | Kind::GitPullRequest => Some(event.id),
+            Kind::GitPatch => Some(match self.first_e_id(event) {
+                Some(parent) => self.resolve_thread_root(parent),
+                None => event.id,
+            }),
+            Kind::Comment => match nip22::extract_root(event) {
+                Some(CommentTarget::Event { id, .. }) => Some(self.resolve_thread_root(id)),
+                _ => None,
+            },
+            Kind::GitPullRequestUpdate => self
+                .first_uppercase_e_id(event)
+                .map(|root| self.resolve_thread_root(root)),
+            Kind::GitStatusOpen
+            | Kind::GitStatusApplied
+            | Kind::GitStatusClosed
+            | Kind::GitStatusDraft => self
+                .nip10_root_id(event)
+                .map(|root| self.resolve_thread_root(root)),
             _ => None,
-        },
-        Kind::GitPullRequestUpdate => {
-            first_uppercase_e_id(event).map(|root| resolve_thread_root(root, lookup))
         }
-        Kind::GitStatusOpen
-        | Kind::GitStatusApplied
-        | Kind::GitStatusClosed
-        | Kind::GitStatusDraft => {
-            nip10_root_id(event).map(|root| resolve_thread_root(root, lookup))
+    }
+
+    // Follow NIP-10/NIP-22 parent pointers until a root item is reached.
+    pub fn resolve_thread_root(&self, id: EventId) -> EventId {
+        let mut seen = HashSet::new();
+        let mut root = id;
+
+        loop {
+            if !seen.insert(root) {
+                return id;
+            }
+
+            let Some(event) = (self.lookup)(root) else {
+                return root;
+            };
+
+            if matches!(event.kind, Kind::GitIssue | Kind::GitPullRequest) {
+                return root;
+            }
+
+            match self.parent_id(&event) {
+                Some(parent) => root = parent,
+                None => return root,
+            }
         }
-        _ => None,
+    }
+
+    // Mirrors gitworkshop's `getParentId`.
+    fn parent_id(&self, event: &Event) -> Option<EventId> {
+        for marker in ["reply", "root"] {
+            if let Some(id) = event
+                .tags
+                .iter()
+                .find_map(|tag| self.e_tag_with_marker(tag, marker))
+            {
+                return Some(id);
+            }
+        }
+
+        if let Some(id) = event.tags.iter().find_map(|tag| {
+            if tag.kind() != "e" {
+                return None;
+            }
+
+            let slice = tag.as_slice();
+            let is_mention = slice.len() == 4 && slice[3] == "mention";
+
+            if is_mention {
+                return None;
+            }
+
+            tag.content()
+                .and_then(|content| EventId::from_hex(content).ok())
+        }) {
+            return Some(id);
+        }
+
+        self.first_uppercase_e_id(event)
+    }
+
+    fn nip10_root_id(&self, event: &Event) -> Option<EventId> {
+        event
+            .tags
+            .iter()
+            .find_map(|tag| self.e_tag_with_marker(tag, "root"))
+            .or_else(|| self.first_e_id(event))
+    }
+
+    fn first_e_id(&self, event: &Event) -> Option<EventId> {
+        self.first_tag_id(event, "e")
+    }
+
+    fn first_uppercase_e_id(&self, event: &Event) -> Option<EventId> {
+        self.first_tag_id(event, "E")
+    }
+
+    fn first_tag_id(&self, event: &Event, name: &str) -> Option<EventId> {
+        event.tags.iter().find_map(|tag| {
+            if tag.kind() != name {
+                return None;
+            }
+            tag.content()
+                .and_then(|content| EventId::from_hex(content).ok())
+        })
+    }
+
+    fn e_tag_with_marker(&self, tag: &Tag, marker: &str) -> Option<EventId> {
+        let slice = tag.as_slice();
+        if tag.kind() != "e" || slice.len() != 4 || slice[3] != marker {
+            return None;
+        }
+        tag.content()
+            .and_then(|content| EventId::from_hex(content).ok())
     }
 }
 
-/// Group notification events and the user's own events into one item per thread.
 pub fn group<E, O, L>(
     events: E,
     own: O,
@@ -162,12 +247,13 @@ where
     O: IntoIterator<Item = Event>,
     L: Fn(EventId) -> Option<Event>,
 {
+    let resolver = ThreadResolver::new(lookup);
     let mut groups: HashMap<EventId, Vec<Event>> = HashMap::new();
     for event in events {
         if event.pubkey == me {
             continue;
         }
-        let Some(root) = notification_root(&event, lookup) else {
+        let Some(root) = resolver.notification_root(&event) else {
             continue;
         };
         groups.entry(root).or_default().push(event);
@@ -175,7 +261,7 @@ where
 
     let mut own_groups: HashMap<EventId, Vec<Event>> = HashMap::new();
     for event in own {
-        let root = notification_root(&event, lookup).unwrap_or(event.id);
+        let root = resolver.notification_root(&event).unwrap_or(event.id);
         own_groups.entry(root).or_default().push(event);
     }
 
@@ -188,8 +274,8 @@ where
         .map(|root| {
             let mut events = groups.remove(&root).unwrap_or_default();
             let mut own_events = own_groups.remove(&root).unwrap_or_default();
-            sort_newest_first(&mut events);
-            sort_newest_first(&mut own_events);
+            utils::sort_newest_first(&mut events);
+            utils::sort_newest_first(&mut own_events);
 
             let root_event = lookup(root);
 
@@ -197,7 +283,8 @@ where
                 root,
                 address: root_event
                     .as_ref()
-                    .and_then(|event| event.tags.coordinates().next()),
+                    .and_then(|event| event.tags.coordinates().next())
+                    .map(RepoAddr::from),
                 root_event,
                 events,
                 own_events,
@@ -218,16 +305,8 @@ where
     items
 }
 
-/// Sort thread events newest first, ties broken by id.
-fn sort_newest_first(events: &mut [Event]) {
-    events.sort_by(|a, b| {
-        b.created_at
-            .cmp(&a.created_at)
-            .then_with(|| b.id.to_hex().cmp(&a.id.to_hex()))
-    });
-}
-
-/// Read and archive state of the inbox, a high-water-mark model.
+// High-water-mark model: events at or before the cutoff
+// are covered without an entry in the id set.
 #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
 pub struct InboxReadState {
     #[serde(default)]
@@ -241,31 +320,26 @@ pub struct InboxReadState {
 }
 
 impl InboxReadState {
-    /// Whether `event` is at or before the read cutoff, or marked read.
     pub fn is_read(&self, event: &Event) -> bool {
         event.created_at <= self.read_before || self.read_ids.contains(&event.id)
     }
 
-    /// Whether `event` is at or before the archived cutoff, or marked archived.
     pub fn is_archived(&self, event: &Event) -> bool {
         event.created_at <= self.archived_before || self.archived_ids.contains(&event.id)
     }
 
-    /// Mark one event read. Events at or before the cutoff are already read.
     pub fn mark_read(&mut self, event: &Event) {
         if event.created_at > self.read_before {
             self.read_ids.insert(event.id);
         }
     }
 
-    /// Mark one event archived. Events at or before the cutoff are already archived.
     pub fn mark_archived(&mut self, event: &Event) {
         if event.created_at > self.archived_before {
             self.archived_ids.insert(event.id);
         }
     }
 
-    /// Mark every non-self event read, anchoring the cutoff ten days back.
     pub fn mark_all_read(&mut self, all: &[Event], me: PublicKey, now: Timestamp) {
         let cutoff = now - MARK_ALL_WINDOW;
         self.read_before = cutoff;
@@ -276,15 +350,14 @@ impl InboxReadState {
             .collect();
     }
 
-    /// Advance the read cutoff to the newest point that keeps unread events
-    /// unread, then prune the id set.
+    // Advance the cutoff to the newest point that keeps unread events unread,
+    // then prune the id set.
     pub fn advance_read(&mut self, all: &[Event], me: PublicKey, now: Timestamp) {
         let cutoff = advance_cutoff(all, me, now, self.read_before, |event| self.is_read(event));
         self.read_before = cutoff;
         prune_ids(&mut self.read_ids, all, cutoff);
     }
 
-    /// Advance the archived cutoff, mirroring [`Self::advance_read`].
     pub fn advance_archived(&mut self, all: &[Event], me: PublicKey, now: Timestamp) {
         let cutoff = advance_cutoff(all, me, now, self.archived_before, |event| {
             self.is_archived(event)
@@ -294,7 +367,6 @@ impl InboxReadState {
     }
 }
 
-/// Newest cutoff that keeps unread events unread, never earlier than `current`.
 fn advance_cutoff<M>(
     all: &[Event],
     me: PublicKey,
@@ -321,7 +393,6 @@ where
     candidate.max(current)
 }
 
-/// Drop ids whose event is unknown or now covered by the cutoff.
 fn prune_ids(ids: &mut HashSet<EventId>, all: &[Event], cutoff: Timestamp) {
     let created_at: HashMap<EventId, Timestamp> = all
         .iter()
@@ -330,103 +401,6 @@ fn prune_ids(ids: &mut HashSet<EventId>, all: &[Event], cutoff: Timestamp) {
     ids.retain(|id| created_at.get(id).is_some_and(|at| *at >= cutoff));
 }
 
-/// Follow NIP-10/NIP-22 parent pointers until a root item is reached.
-fn resolve_thread_root(id: EventId, lookup: &impl Fn(EventId) -> Option<Event>) -> EventId {
-    let mut seen = HashSet::new();
-    let mut root = id;
-
-    loop {
-        if !seen.insert(root) {
-            return id;
-        }
-
-        let Some(event) = lookup(root) else {
-            return root;
-        };
-
-        if matches!(event.kind, Kind::GitIssue | Kind::GitPullRequest) {
-            return root;
-        }
-
-        match parent_id(&event) {
-            Some(parent) => root = parent,
-            None => return root,
-        }
-    }
-}
-
-/// Parent of a thread event, mirroring gitworkshop's `getParentId`.
-fn parent_id(event: &Event) -> Option<EventId> {
-    for marker in ["reply", "root"] {
-        if let Some(id) = event
-            .tags
-            .iter()
-            .find_map(|tag| e_tag_with_marker(tag, marker))
-        {
-            return Some(id);
-        }
-    }
-
-    if let Some(id) = event.tags.iter().find_map(|tag| {
-        if tag.kind() != "e" {
-            return None;
-        }
-
-        let slice = tag.as_slice();
-        let is_mention = slice.len() == 4 && slice[3] == "mention";
-
-        if is_mention {
-            return None;
-        }
-
-        tag.content()
-            .and_then(|content| EventId::from_hex(content).ok())
-    }) {
-        return Some(id);
-    }
-
-    first_uppercase_e_id(event)
-}
-
-/// NIP-10 root of an event: the `e` tag marked `root`, else the first `e` tag.
-fn nip10_root_id(event: &Event) -> Option<EventId> {
-    event
-        .tags
-        .iter()
-        .find_map(|tag| e_tag_with_marker(tag, "root"))
-        .or_else(|| first_e_id(event))
-}
-
-/// First `e` tag id, in document order.
-fn first_e_id(event: &Event) -> Option<EventId> {
-    first_tag_id(event, "e")
-}
-
-/// First uppercase `E` tag id, in document order.
-fn first_uppercase_e_id(event: &Event) -> Option<EventId> {
-    first_tag_id(event, "E")
-}
-
-fn first_tag_id(event: &Event, name: &str) -> Option<EventId> {
-    event.tags.iter().find_map(|tag| {
-        if tag.kind() != name {
-            return None;
-        }
-        tag.content()
-            .and_then(|content| EventId::from_hex(content).ok())
-    })
-}
-
-/// Event id from a four-element `e` tag carrying `marker`.
-fn e_tag_with_marker(tag: &Tag, marker: &str) -> Option<EventId> {
-    let slice = tag.as_slice();
-    if tag.kind() != "e" || slice.len() != 4 || slice[3] != marker {
-        return None;
-    }
-    tag.content()
-        .and_then(|content| EventId::from_hex(content).ok())
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -489,7 +463,7 @@ mod tests {
         );
         let events = [issue.clone(), comment.clone()];
         assert_eq!(
-            notification_root(&comment, &lookup(&events)),
+            ThreadResolver::new(&lookup(&events)).notification_root(&comment),
             Some(issue.id)
         );
     }
@@ -500,7 +474,7 @@ mod tests {
         let child_patch = signed(&keys(1), Kind::GitPatch, vec![e_tag(&root_patch)], 200);
         let events = [root_patch.clone(), child_patch.clone()];
         assert_eq!(
-            notification_root(&child_patch, &lookup(&events)),
+            ThreadResolver::new(&lookup(&events)).notification_root(&child_patch),
             Some(root_patch.id)
         );
     }
@@ -515,7 +489,10 @@ mod tests {
             200,
         );
         let events = [issue.clone(), status.clone()];
-        assert_eq!(notification_root(&status, &lookup(&events)), Some(issue.id));
+        assert_eq!(
+            ThreadResolver::new(&lookup(&events)).notification_root(&status),
+            Some(issue.id)
+        );
     }
 
     #[test]
@@ -524,7 +501,10 @@ mod tests {
         let reply = signed(&keys(2), Kind::Comment, vec![uppercase_e_tag(&issue)], 200);
         let nested = signed(&keys(3), Kind::Comment, vec![uppercase_e_tag(&reply)], 300);
         let events = [issue.clone(), reply, nested.clone()];
-        assert_eq!(notification_root(&nested, &lookup(&events)), Some(issue.id));
+        assert_eq!(
+            ThreadResolver::new(&lookup(&events)).notification_root(&nested),
+            Some(issue.id)
+        );
     }
 
     #[test]
@@ -568,7 +548,6 @@ mod tests {
         assert_eq!(items[0].kind(), Some(Kind::GitIssue));
         assert_eq!(items[0].title(), "Add retry logic");
         assert_eq!(items[0].events, vec![reply.clone()]);
-        // The own events are kept apart from the notifications, newest first.
         assert_eq!(items[0].own_events, vec![mine.clone(), issue.clone()]);
         assert_eq!(
             items[0]
@@ -579,94 +558,4 @@ mod tests {
             vec![issue.id, mine.id, reply.id]
         );
     }
-
-    #[test]
-    fn mark_all_read_marks_known_recent_events() {
-        let me = keys(1);
-        let now = Timestamp::from_secs(1_000_000_000);
-        let recent = issue(&keys(2), now.as_secs() - 1000);
-        let old = issue(&keys(2), now.as_secs() - 5 * 24 * 60 * 60);
-        let ancient = issue(&keys(2), now.as_secs() - 20 * 24 * 60 * 60);
-        let mine = issue(&keys(1), now.as_secs() - 100);
-
-        let mut state = InboxReadState::default();
-        state.mark_all_read(
-            &[recent.clone(), old.clone(), ancient.clone(), mine.clone()],
-            me.public_key(),
-            now,
-        );
-
-        assert_eq!(state.read_before, now - MARK_ALL_WINDOW);
-        assert_eq!(state.read_ids, HashSet::from([recent.id, old.id]));
-        assert!(state.is_read(&recent));
-        assert!(state.is_read(&ancient));
-        assert!(!state.is_read(&mine));
-    }
-
-    #[test]
-    fn advance_read_never_moves_the_cutoff_backwards() {
-        let me = keys(1);
-        let unread = issue(&keys(2), 1_000);
-        let all = [unread];
-        let now = Timestamp::from_secs(1_000_000_000);
-
-        let mut state = InboxReadState {
-            read_before: Timestamp::from_secs(999_999_999),
-            ..Default::default()
-        };
-        state.advance_read(&all, me.public_key(), now);
-
-        assert_eq!(state.read_before, Timestamp::from_secs(999_999_999));
-    }
-
-    #[test]
-    fn advance_read_moves_before_the_oldest_unread_and_prunes_ids() {
-        let me = keys(1);
-        let now = Timestamp::from_secs(1_000_000_000);
-        let five_days = 5 * 24 * 60 * 60;
-        let old_unread = issue(&keys(2), now.as_secs() - five_days);
-        // Read ids that fall before and after the new cutoff.
-        let stale = signed(
-            &keys(2),
-            Kind::GitIssue,
-            Vec::new(),
-            now.as_secs() - five_days - 1000,
-        );
-        let fresh = signed(
-            &keys(2),
-            Kind::GitIssue,
-            Vec::new(),
-            now.as_secs() - 100_000,
-        );
-
-        let mut state = InboxReadState {
-            read_ids: HashSet::from([stale.id, fresh.id]),
-            ..Default::default()
-        };
-        state.advance_read(
-            &[old_unread.clone(), stale.clone(), fresh.clone()],
-            me.public_key(),
-            now,
-        );
-
-        assert_eq!(state.read_before, old_unread.created_at - 1);
-        assert_eq!(state.read_ids, HashSet::from([fresh.id]));
-    }
-
-    #[test]
-    fn mark_archived_skips_events_at_or_before_the_cutoff() {
-        let now = Timestamp::from_secs(1_000_000_000);
-        let event = issue(&keys(2), now.as_secs() - 1000);
-
-        let mut state = InboxReadState {
-            archived_before: now,
-            ..Default::default()
-        };
-        state.mark_archived(&event);
-        assert!(state.archived_ids.is_empty());
-
-        let mut state = InboxReadState::default();
-        state.mark_archived(&event);
-        assert_eq!(state.archived_ids, HashSet::from([event.id]));
-    }
 }