diff --git a/crates/radicle-tauri/src/commands/cob/issue.rs b/crates/radicle-tauri/src/commands/cob/issue.rs index 185385919..59d5e8ccb 100644 --- a/crates/radicle-tauri/src/commands/cob/issue.rs +++ b/crates/radicle-tauri/src/commands/cob/issue.rs @@ -17,8 +17,6 @@ use radicle_types::traits::issue::IssuesMut; use crate::AppState; -use either::Either; - #[tauri::command] pub fn create_issue( ctx: tauri::State, @@ -50,46 +48,10 @@ pub(crate) async fn list_issues( // None: return all issues, `skip` is ignored. take: Option, ) -> Result>, Error> { - use types::cobs::query::IssueStatus; - let profile = ctx.profile(); let aliases = profile.aliases(); - let cursor = skip.unwrap_or(0); - let issues = match status.unwrap_or_default() { - IssueStatus::All => Either::Left(issue_service.list(rid)?), - IssueStatus::Open => Either::Right(issue_service.list_by_status(rid, "open")?), - IssueStatus::Closed => Either::Right(issue_service.list_by_status(rid, "closed")?), - }; - - match take { - None => { - let content = issues - .map(|(id, issue)| types::cobs::issue::Issue::summary(&id, &issue, &aliases)) - .collect::>(); - - Ok::<_, Error>(types::cobs::PaginatedQuery { - cursor: 0, - more: false, - content, - }) - } - Some(take) => { - let mut content = issues - .skip(cursor) - .take(take + 1) - .map(|(id, issue)| types::cobs::issue::Issue::summary(&id, &issue, &aliases)) - .collect::>(); - let more = content.len() > take; - content.truncate(take); - - Ok::<_, Error>(types::cobs::PaginatedQuery { - cursor, - more, - content, - }) - } - } + Ok(issue_service.list_paginated(rid, status.unwrap_or_default(), skip, take, &aliases)?) } #[tauri::command] diff --git a/crates/radicle-types/src/domain/issue/models/issue.rs b/crates/radicle-types/src/domain/issue/models/issue.rs index 268f2707f..a3ef8b7ef 100644 --- a/crates/radicle-types/src/domain/issue/models/issue.rs +++ b/crates/radicle-types/src/domain/issue/models/issue.rs @@ -1,5 +1,21 @@ use thiserror::Error; +/// Issue state filter understood by the COB cache queries. +#[derive(Debug, Clone, Copy)] +pub enum Status { + Open, + Closed, +} + +impl Status { + pub fn as_str(&self) -> &'static str { + match self { + Self::Open => "open", + Self::Closed => "closed", + } + } +} + #[derive(Debug, Error)] pub enum ListIssuesError { #[error(transparent)] diff --git a/crates/radicle-types/src/domain/issue/service.rs b/crates/radicle-types/src/domain/issue/service.rs index 7bd3f6d6e..467705631 100644 --- a/crates/radicle-types/src/domain/issue/service.rs +++ b/crates/radicle-types/src/domain/issue/service.rs @@ -35,7 +35,7 @@ where fn list_by_status( &self, rid: identity::RepoId, - status: &'static str, + status: models::issue::Status, ) -> Result, models::issue::ListIssuesError> { self.issues.list_by_status(rid, status) } diff --git a/crates/radicle-types/src/domain/issue/traits.rs b/crates/radicle-types/src/domain/issue/traits.rs index 724789a3d..2e4efec0f 100644 --- a/crates/radicle-types/src/domain/issue/traits.rs +++ b/crates/radicle-types/src/domain/issue/traits.rs @@ -1,7 +1,10 @@ use radicle::identity; use radicle::issue::{Issue, IssueId}; +use radicle::node::AliasStore; +use crate::cobs; use crate::domain::issue::models; +use crate::domain::issue::models::issue::Status; pub trait IssueStorage { fn list( @@ -12,7 +15,7 @@ pub trait IssueStorage { fn list_by_status( &self, rid: identity::RepoId, - status: &'static str, + status: Status, ) -> Result, models::issue::ListIssuesError>; } @@ -25,6 +28,51 @@ pub trait IssueService { fn list_by_status( &self, rid: identity::RepoId, - status: &'static str, + status: Status, ) -> Result, models::issue::ListIssuesError>; + + /// One page of issue summaries, shared by all drivers so pagination + /// behaves identically everywhere. Rows are consumed lazily: with `take` + /// set, issues beyond the requested page are never deserialized. Without + /// `take` the full list is returned and `skip` is ignored. + fn list_paginated( + &self, + rid: identity::RepoId, + status: cobs::query::IssueStatus, + skip: Option, + take: Option, + aliases: &impl AliasStore, + ) -> Result>, models::issue::ListIssuesError> { + let issues: Box + '_> = match status { + cobs::query::IssueStatus::All => Box::new(self.list(rid)?), + cobs::query::IssueStatus::Open => Box::new(self.list_by_status(rid, Status::Open)?), + cobs::query::IssueStatus::Closed => Box::new(self.list_by_status(rid, Status::Closed)?), + }; + let summary = + |(id, issue): (IssueId, Issue)| cobs::issue::Issue::summary(&id, &issue, aliases); + + match take { + None => Ok(cobs::PaginatedQuery { + cursor: 0, + more: false, + content: issues.map(summary).collect::>(), + }), + Some(take) => { + let cursor = skip.unwrap_or(0); + let mut content = issues + .skip(cursor) + .take(take + 1) + .map(summary) + .collect::>(); + let more = content.len() > take; + content.truncate(take); + + Ok(cobs::PaginatedQuery { + cursor, + more, + content, + }) + } + } + } } diff --git a/crates/radicle-types/src/outbound/sqlite.rs b/crates/radicle-types/src/outbound/sqlite.rs index 02485e0f9..185048eb5 100644 --- a/crates/radicle-types/src/outbound/sqlite.rs +++ b/crates/radicle-types/src/outbound/sqlite.rs @@ -10,12 +10,13 @@ use sqlite as sql; use crate::domain::inbox::models::notification; use crate::domain::inbox::traits::InboxStorage; -use crate::domain::issue::models::issue::ListIssuesError; +use crate::domain::issue::models::issue::{ListIssuesError, Status as IssueStatus}; use crate::domain::issue::traits::IssueStorage; use crate::domain::patch::models::patch::{CountsError, ListPatchesError, PatchCounts, State}; use crate::domain::patch::traits::PatchStorage; use crate::error::Error; +#[derive(Clone)] pub struct Sqlite { pub db: Arc, } @@ -113,22 +114,39 @@ impl PatchStorage for Sqlite { } } -impl IssueStorage for Sqlite { - fn list( +impl Sqlite { + /// Issues for `rid`, newest first, optionally filtered by state. Single + /// home of the issue-listing SQL; `list`/`list_by_status` differ only in + /// the status predicate. The sort key is the root comment's timestamp + /// (the comment without a `replyTo`) — the issue's creation time — not + /// the minimum across replies, whose author-supplied clocks could + /// otherwise sink an issue below its real position. + fn issues_by( &self, rid: identity::RepoId, + status: Option, ) -> Result, ListIssuesError> { - let mut stmt = self.db.prepare( + let filter = if status.is_some() { + "AND issue->>'$.state.status' = ?2" + } else { + "" + }; + let mut stmt = self.db.prepare(format!( "SELECT id, issue, ( SELECT MIN(JSON_EXTRACT(comment.value, '$.edits[0].timestamp')) FROM JSON_EACH(JSON_EXTRACT(i.issue, '$.thread.comments')) AS comment + WHERE JSON_EXTRACT(comment.value, '$.replyTo') IS NULL ) AS created_timestamp FROM issues AS i WHERE repo = ?1 + {filter} ORDER BY created_timestamp DESC, id DESC; - ", - )?; + " + ))?; stmt.bind((1, &rid))?; + if let Some(status) = status { + stmt.bind((2, status.as_str()))?; + } Ok(stmt.into_iter().filter_map(|row| { let row = row.ok()?; let id = IssueId::from_str(row.read::<&str, _>("id")).ok()?; @@ -136,31 +154,22 @@ impl IssueStorage for Sqlite { Some((id, issue)) })) } +} + +impl IssueStorage for Sqlite { + fn list( + &self, + rid: identity::RepoId, + ) -> Result, ListIssuesError> { + self.issues_by(rid, None) + } fn list_by_status( &self, rid: identity::RepoId, - status: &'static str, + status: IssueStatus, ) -> Result, ListIssuesError> { - let mut stmt = self.db.prepare( - "SELECT id, issue, ( - SELECT MIN(JSON_EXTRACT(comment.value, '$.edits[0].timestamp')) - FROM JSON_EACH(JSON_EXTRACT(i.issue, '$.thread.comments')) AS comment - ) AS created_timestamp - FROM issues AS i - WHERE repo = ?1 - AND issue->>'$.state.status' = ?2 - ORDER BY created_timestamp DESC, id DESC; - ", - )?; - stmt.bind((1, &rid))?; - stmt.bind((2, status))?; - Ok(stmt.into_iter().filter_map(|row| { - let row = row.ok()?; - let id = IssueId::from_str(row.read::<&str, _>("id")).ok()?; - let issue = serde_json::from_str::(row.read::<&str, _>("issue")).ok()?; - Some((id, issue)) - })) + self.issues_by(rid, Some(status)) } } diff --git a/crates/test-http-api/src/api.rs b/crates/test-http-api/src/api.rs index 8cd999f99..4824abc18 100644 --- a/crates/test-http-api/src/api.rs +++ b/crates/test-http-api/src/api.rs @@ -473,50 +473,12 @@ async fn issues_handler( take, }): Json, ) -> impl IntoResponse { - use types::cobs::query::IssueStatus; - let aliases = ctx.profile.aliases(); - let cursor = skip.unwrap_or(0); + let page = ctx + .issues + .list_paginated(rid, status.unwrap_or_default(), skip, take, &aliases)?; - let issues = match status.unwrap_or_default() { - IssueStatus::All => ctx.issues.list(rid)?.collect::>(), - IssueStatus::Open => ctx.issues.list_by_status(rid, "open")?.collect::>(), - IssueStatus::Closed => ctx - .issues - .list_by_status(rid, "closed")? - .collect::>(), - }; - - match take { - None => { - let content = issues - .into_iter() - .map(|(id, issue)| issue::Issue::summary(&id, &issue, &aliases)) - .collect::>(); - - Ok::<_, Error>(Json(cobs::PaginatedQuery { - cursor: 0, - more: false, - content, - })) - } - Some(take) => { - let mut content = issues - .into_iter() - .skip(cursor) - .take(take + 1) - .map(|(id, issue)| issue::Issue::summary(&id, &issue, &aliases)) - .collect::>(); - let more = content.len() > take; - content.truncate(take); - - Ok::<_, Error>(Json(cobs::PaginatedQuery { - cursor, - more, - content, - })) - } - } + Ok::<_, Error>(Json(page)) } #[derive(Serialize, Deserialize)]