diff --git a/crates/codex-api/src/docs.rs b/crates/codex-api/src/docs.rs index f32e0fdb..a868a5ac 100644 --- a/crates/codex-api/src/docs.rs +++ b/crates/codex-api/src/docs.rs @@ -369,6 +369,8 @@ The following paths are exempt from rate limiting: v1::handlers::record_reading_sessions, v1::handlers::get_reading_stats, v1::handlers::get_reading_coverage, + v1::handlers::get_orphaned_reading_history, + v1::handlers::purge_orphaned_reading_history, // Reading progress endpoints v1::handlers::update_reading_progress, @@ -1046,6 +1048,8 @@ The following paths are exempt from rate limiting: v1::dto::ReadingStatsGranularity, v1::dto::ReadingStatsSort, v1::dto::ReadingCoverageDto, + v1::dto::OrphanedHistoryDto, + v1::dto::PurgedOrphanedHistoryDto, // Reading session DTOs v1::dto::RecordReadingSessionsRequest, diff --git a/crates/codex-api/src/routes/v1/dto/reading_stats.rs b/crates/codex-api/src/routes/v1/dto/reading_stats.rs index cf6143c7..1d039f7c 100644 --- a/crates/codex-api/src/routes/v1/dto/reading_stats.rs +++ b/crates/codex-api/src/routes/v1/dto/reading_stats.rs @@ -6,8 +6,8 @@ use chrono::{DateTime, Utc}; use codex_db::repositories::{ - DurationBreakdown, ReadingByDevice, ReadingByFormat, ReadingBySeries, ReadingCoverage, - ReadingPeriod, ReadingSummary, StatsGranularity, StatsSort, + DurationBreakdown, OrphanedHistory, PurgedOrphanedHistory, ReadingByDevice, ReadingByFormat, + ReadingBySeries, ReadingCoverage, ReadingPeriod, ReadingSummary, StatsGranularity, StatsSort, }; use serde::{Deserialize, Serialize}; use utoipa::{IntoParams, ToSchema}; @@ -219,9 +219,17 @@ impl From for ReadingByDeviceDto { #[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] #[serde(rename_all = "camelCase")] pub struct ReadingBySeriesDto { - pub series_id: Uuid, + /// Null on the removed-from-library row. + pub series_id: Option, + /// Null on the removed-from-library row. #[schema(example = "Berserk")] - pub series_name: String, + pub series_name: Option, + /// True on the single row that gathers reading whose book has since been + /// deleted from the server. That time still counts towards every total, + /// but which series it belonged to is no longer known. Its `books` and + /// `booksFinished` are always 0: both count distinct books, and deleted + /// books cannot be told apart. + pub removed_from_library: bool, pub duration: DurationBreakdownDto, pub pages_read: i64, pub sessions: i64, @@ -235,6 +243,7 @@ pub struct ReadingBySeriesDto { impl From for ReadingBySeriesDto { fn from(value: ReadingBySeries) -> Self { Self { + removed_from_library: value.series_id.is_none(), series_id: value.series_id, series_name: value.series_name, duration: value.duration.into(), @@ -250,8 +259,12 @@ impl From for ReadingBySeriesDto { #[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] #[serde(rename_all = "camelCase")] pub struct ReadingByFormatDto { + /// Null on the removed-from-library row. #[schema(example = "cbz")] - pub format: String, + pub format: Option, + /// True on the single row that gathers reading whose book has since been + /// deleted from the server, and whose format is therefore unknown. + pub removed_from_library: bool, pub duration: DurationBreakdownDto, pub pages_read: i64, pub sessions: i64, @@ -261,6 +274,7 @@ pub struct ReadingByFormatDto { impl From for ReadingByFormatDto { fn from(value: ReadingByFormat) -> Self { Self { + removed_from_library: value.format.is_none(), format: value.format, duration: value.duration.into(), pages_read: value.pages_read, @@ -308,3 +322,46 @@ pub struct ReadingStatsResponse { pub series: Vec, pub formats: Vec, } + +/// What purging the removed-from-library history deleted. +#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct PurgedOrphanedHistoryDto { + /// Reading sessions deleted. Their time no longer counts anywhere. + pub sessions_removed: u64, + /// Finished read-throughs deleted. + pub completions_removed: u64, +} + +impl From for PurgedOrphanedHistoryDto { + fn from(value: PurgedOrphanedHistory) -> Self { + Self { + sessions_removed: value.sessions, + completions_removed: value.completions, + } + } +} + +/// The caller's reading history whose book has since been deleted, across +/// every date. What a purge would remove. +#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct OrphanedHistoryDto { + pub duration: DurationBreakdownDto, + pub pages_read: i64, + /// Sittings, counted as the dashboard counts them. + pub sessions: i64, + /// Finished read-throughs. + pub completions: u64, +} + +impl From for OrphanedHistoryDto { + fn from(value: OrphanedHistory) -> Self { + Self { + duration: value.duration.into(), + pages_read: value.pages_read, + sessions: value.sessions, + completions: value.completions, + } + } +} diff --git a/crates/codex-api/src/routes/v1/handlers/reading_stats.rs b/crates/codex-api/src/routes/v1/handlers/reading_stats.rs index 4ac3c433..f79ff0c2 100644 --- a/crates/codex-api/src/routes/v1/handlers/reading_stats.rs +++ b/crates/codex-api/src/routes/v1/handlers/reading_stats.rs @@ -1,9 +1,10 @@ //! Reading statistics, aggregated from the session log. use super::super::dto::{ - DurationBreakdownDto, ReadingByDeviceDto, ReadingByFormatDto, ReadingBySeriesDto, - ReadingCoverageDto, ReadingPeriodDto, ReadingStatsGranularity, ReadingStatsQuery, - ReadingStatsResponse, ReadingStatsSort, ReadingSummaryDto, + DurationBreakdownDto, OrphanedHistoryDto, PurgedOrphanedHistoryDto, ReadingByDeviceDto, + ReadingByFormatDto, ReadingBySeriesDto, ReadingCoverageDto, ReadingPeriodDto, + ReadingStatsGranularity, ReadingStatsQuery, ReadingStatsResponse, ReadingStatsSort, + ReadingSummaryDto, }; use crate::{AppState, error::ApiError, extractors::AuthContext, permissions::Permission}; use axum::{ @@ -34,7 +35,11 @@ const MAX_TZ_OFFSET_MINUTES: i32 = 14 * 60; #[derive(OpenApi)] #[openapi( - paths(get_reading_stats), + paths( + get_reading_stats, + get_orphaned_reading_history, + purge_orphaned_reading_history + ), components(schemas( ReadingStatsResponse, ReadingSummaryDto, @@ -44,6 +49,8 @@ const MAX_TZ_OFFSET_MINUTES: i32 = 14 * 60; ReadingByFormatDto, DurationBreakdownDto, ReadingStatsGranularity, + OrphanedHistoryDto, + PurgedOrphanedHistoryDto, )), tags( (name = "Reading Statistics", description = "Aggregated reading time and pages") @@ -186,3 +193,70 @@ pub async fn get_reading_coverage( Ok(Json(coverage.into())) } + +/// The caller's reading history for books that no longer exist, in total +/// +/// Unwindowed: exactly what `DELETE /api/v1/reading-stats/orphaned` would +/// remove, so a client can say so before asking the reader to confirm. +#[utoipa::path( + get, + path = "/api/v1/reading-stats/orphaned", + responses( + (status = 200, description = "Totals of the detached history", body = OrphanedHistoryDto), + (status = 401, description = "Unauthorized"), + (status = 403, description = "Forbidden"), + ), + security( + ("jwt_bearer" = []), + ("api_key" = []) + ), + tag = "Reading Statistics" +)] +pub async fn get_orphaned_reading_history( + State(state): State>, + auth: AuthContext, +) -> Result, ApiError> { + auth.require_permission(&Permission::ProgressRead)?; + + let totals = ReadingStatsRepository::orphaned_totals(&state.db, auth.user_id) + .await + .map_err(|e| ApiError::Internal(format!("Failed to total orphaned history: {}", e)))?; + + Ok(Json(totals.into())) +} + +/// Delete the caller's reading history for books that no longer exist +/// +/// When a book is deleted from the server its reading sessions and finished +/// read-throughs are kept, so the time still counts towards every statistic; +/// the series and format breakdowns show it as one "removed from library" row. +/// This discards those rows for the caller, and only for the caller. +/// +/// Irreversible. Only history already detached from any book is touched; +/// attributed reading is never affected. +#[utoipa::path( + delete, + path = "/api/v1/reading-stats/orphaned", + responses( + (status = 200, description = "What was deleted", body = PurgedOrphanedHistoryDto), + (status = 401, description = "Unauthorized"), + (status = 403, description = "Forbidden"), + ), + security( + ("jwt_bearer" = []), + ("api_key" = []) + ), + tag = "Reading Statistics" +)] +pub async fn purge_orphaned_reading_history( + State(state): State>, + auth: AuthContext, +) -> Result, ApiError> { + auth.require_permission(&Permission::ProgressWrite)?; + + let purged = ReadingStatsRepository::purge_orphaned(&state.db, auth.user_id) + .await + .map_err(|e| ApiError::Internal(format!("Failed to purge orphaned history: {}", e)))?; + + Ok(Json(purged.into())) +} diff --git a/crates/codex-api/src/routes/v1/routes/books.rs b/crates/codex-api/src/routes/v1/routes/books.rs index 4f147896..b8321020 100644 --- a/crates/codex-api/src/routes/v1/routes/books.rs +++ b/crates/codex-api/src/routes/v1/routes/books.rs @@ -186,6 +186,13 @@ pub fn routes(_state: Arc) -> Router> { "/reading-stats/coverage", get(handlers::get_reading_coverage), ) + // Reading whose book has been deleted is kept until the reader says + // otherwise. + .route( + "/reading-stats/orphaned", + get(handlers::get_orphaned_reading_history) + .delete(handlers::purge_orphaned_reading_history), + ) // Mark as read/unread routes .route("/books/{book_id}/read", post(handlers::mark_book_as_read)) .route( diff --git a/crates/codex-db/src/entities/read_completions.rs b/crates/codex-db/src/entities/read_completions.rs index 596c13ff..ed3cc371 100644 --- a/crates/codex-db/src/entities/read_completions.rs +++ b/crates/codex-db/src/entities/read_completions.rs @@ -2,8 +2,9 @@ //! //! An append-only log of finished read-throughs: one row per completed pass of //! one book by one user. Rows are inserted when a book is completed and only -//! ever removed by an explicit history reset (or by a cascade when the user or -//! book is deleted). Nothing updates them. +//! ever removed by an explicit history reset or by a cascade when the user is +//! deleted. Deleting the book keeps the row and clears `book_id`. Nothing else +//! updates them. //! //! This is deliberately separate from `read_progress`, which tracks the //! *current* pass and is deleted when a book is marked unread. Keeping the @@ -21,7 +22,11 @@ pub struct Model { #[sea_orm(primary_key, auto_increment = false)] pub id: Uuid, pub user_id: Uuid, - pub book_id: Uuid, + /// The finished book. `None` once that book has been hard-deleted: the fact + /// that it was finished survives the file, only the attribution is lost. + /// Every write path supplies a book; `None` is only ever reached by the + /// foreign key's `ON DELETE SET NULL`. + pub book_id: Option, /// When this pass started, copied from the `read_progress` row that was /// current when the book completed. pub started_at: DateTime, @@ -36,7 +41,7 @@ pub enum Relation { from = "Column::BookId", to = "super::books::Column::Id", on_update = "NoAction", - on_delete = "Cascade" + on_delete = "SetNull" )] Books, #[sea_orm( diff --git a/crates/codex-db/src/entities/reading_sessions.rs b/crates/codex-db/src/entities/reading_sessions.rs index bc16f85c..831efe04 100644 --- a/crates/codex-db/src/entities/reading_sessions.rs +++ b/crates/codex-db/src/entities/reading_sessions.rs @@ -28,7 +28,12 @@ pub struct Model { #[sea_orm(primary_key, auto_increment = false)] pub id: Uuid, pub user_id: Uuid, - pub book_id: Uuid, + /// The book this reading happened in. `None` once that book has been + /// hard-deleted: the session outlives the file so the time still counts + /// towards the reader's statistics, but nothing can say which book it was. + /// Every write path supplies a book; `None` is only ever reached by the + /// foreign key's `ON DELETE SET NULL`. + pub book_id: Option, /// Stable per install. Producers with no device concept of their own /// (Komga, OPDS) derive one from the API key or user agent. pub device_id: String, @@ -146,7 +151,7 @@ pub enum Relation { from = "Column::BookId", to = "super::books::Column::Id", on_update = "NoAction", - on_delete = "Cascade" + on_delete = "SetNull" )] Books, #[sea_orm( diff --git a/crates/codex-db/src/repositories/mod.rs b/crates/codex-db/src/repositories/mod.rs index 05ff2a2f..63716941 100644 --- a/crates/codex-db/src/repositories/mod.rs +++ b/crates/codex-db/src/repositories/mod.rs @@ -97,9 +97,9 @@ pub use read_progress::ReadProgressRepository; pub use reading_sessions::{AppendOutcome, DeviceContext, NewSession, ReadingSessionRepository}; #[allow(unused_imports)] pub use reading_stats::{ - DurationBreakdown, ReadingByDevice, ReadingByFormat, ReadingBySeries, ReadingCoverage, - ReadingPeriod, ReadingStatsRepository, ReadingSummary, StatsGranularity, StatsSort, - StatsWindow, + DurationBreakdown, OrphanedHistory, PurgedOrphanedHistory, ReadingByDevice, ReadingByFormat, + ReadingBySeries, ReadingCoverage, ReadingPeriod, ReadingStatsRepository, ReadingSummary, + StatsGranularity, StatsSort, StatsWindow, }; #[allow(unused_imports)] pub use refresh_token::{NewRefreshToken, RefreshTokenRepository}; diff --git a/crates/codex-db/src/repositories/read_completions.rs b/crates/codex-db/src/repositories/read_completions.rs index 00841cd9..1e598315 100644 --- a/crates/codex-db/src/repositories/read_completions.rs +++ b/crates/codex-db/src/repositories/read_completions.rs @@ -53,7 +53,7 @@ impl ReadCompletionRepository { let row = read_completions::ActiveModel { id: Set(Uuid::new_v4()), user_id: Set(user_id), - book_id: Set(book_id), + book_id: Set(Some(book_id)), started_at: Set(started_at), completed_at: Set(completed_at), }; @@ -240,8 +240,11 @@ impl ReadCompletionRepository { .all(db) .await?; for row in rows { + let Some(book_id) = row.book_id else { + continue; + }; by_book - .entry(row.book_id) + .entry(book_id) .or_default() .push((row.started_at, row.completed_at)); } @@ -611,8 +614,8 @@ mod tests { .unwrap(); assert_eq!(entries.len(), 2, "the unrelated series must not leak in"); // Newest first. - assert_eq!(entries[0].book_id, second.id); - assert_eq!(entries[1].book_id, first.id); + assert_eq!(entries[0].book_id, Some(second.id)); + assert_eq!(entries[1].book_id, Some(first.id)); } #[tokio::test] diff --git a/crates/codex-db/src/repositories/read_progress.rs b/crates/codex-db/src/repositories/read_progress.rs index 6a51350e..5816c73e 100644 --- a/crates/codex-db/src/repositories/read_progress.rs +++ b/crates/codex-db/src/repositories/read_progress.rs @@ -1771,7 +1771,7 @@ mod tests { rows.push(reading_sessions::ActiveModel { id: Set(Uuid::new_v4()), user_id: Set(user.id), - book_id: Set(book.id), + book_id: Set(Some(book.id)), device_id: Set(format!("bench-device-{}", i % 3)), device_name: Set(None), pass: Set(pass), diff --git a/crates/codex-db/src/repositories/reading_sessions/fold.rs b/crates/codex-db/src/repositories/reading_sessions/fold.rs index ec2851b8..0dfdf9f3 100644 --- a/crates/codex-db/src/repositories/reading_sessions/fold.rs +++ b/crates/codex-db/src/repositories/reading_sessions/fold.rs @@ -274,7 +274,7 @@ mod tests { Session { id: Uuid::new_v4(), user_id: Uuid::nil(), - book_id: Uuid::nil(), + book_id: Some(Uuid::nil()), device_id: self.device.to_string(), device_name: None, pass: self.pass, diff --git a/crates/codex-db/src/repositories/reading_sessions/mod.rs b/crates/codex-db/src/repositories/reading_sessions/mod.rs index ae699791..8e21e865 100644 --- a/crates/codex-db/src/repositories/reading_sessions/mod.rs +++ b/crates/codex-db/src/repositories/reading_sessions/mod.rs @@ -455,7 +455,7 @@ impl ReadingSessionRepository { let model = reading_sessions::ActiveModel { id: Set(session.id), user_id: Set(session.user_id), - book_id: Set(session.book_id), + book_id: Set(Some(session.book_id)), device_id: Set(session.device_id), device_name: Set(session.device_name), pass: Set(pass), diff --git a/crates/codex-db/src/repositories/reading_stats.rs b/crates/codex-db/src/repositories/reading_stats.rs index 6dd0f57e..ef392d47 100644 --- a/crates/codex-db/src/repositories/reading_stats.rs +++ b/crates/codex-db/src/repositories/reading_stats.rs @@ -17,9 +17,13 @@ #![allow(dead_code)] +use crate::entities::{read_completions, reading_sessions}; use anyhow::Result; use chrono::{DateTime, Utc}; -use sea_orm::{ConnectionTrait, DatabaseBackend, FromQueryResult, Statement, Value}; +use sea_orm::{ + ColumnTrait, ConnectionTrait, DatabaseBackend, EntityTrait, FromQueryResult, PaginatorTrait, + QueryFilter, Statement, TransactionTrait, Value, +}; use serde::{Deserialize, Serialize}; use uuid::Uuid; @@ -182,10 +186,14 @@ pub struct ReadingByDevice { } /// Totals for one series. +/// +/// `series_id` and `series_name` are both `None` on the one row that gathers +/// reading whose book has since been hard-deleted. That time is real and +/// counts towards every total, but nothing says which series it came from. #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct ReadingBySeries { - pub series_id: Uuid, - pub series_name: String, + pub series_id: Option, + pub series_name: Option, pub duration: DurationBreakdown, pub pages_read: i64, pub sessions: i64, @@ -194,9 +202,12 @@ pub struct ReadingBySeries { } /// Totals for one file format. +/// +/// `format` is `None` on the one row that gathers reading whose book has since +/// been hard-deleted, for the same reason as [`ReadingBySeries`]. #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct ReadingByFormat { - pub format: String, + pub format: Option, pub duration: DurationBreakdown, pub pages_read: i64, pub sessions: i64, @@ -211,6 +222,22 @@ pub struct ReadingCoverage { pub last_read_at: Option>, } +/// The caller's reading history whose book has been hard-deleted, in total. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct OrphanedHistory { + pub duration: DurationBreakdown, + pub pages_read: i64, + pub sessions: i64, + pub completions: u64, +} + +/// What [`ReadingStatsRepository::purge_orphaned`] removed. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct PurgedOrphanedHistory { + pub sessions: u64, + pub completions: u64, +} + /// The window a query covers. #[derive(Copy, Clone, Debug)] pub struct StatsWindow { @@ -281,7 +308,18 @@ const READING_KINDS: &str = "rs.kind IN ('progress', 'completed')"; /// /// Left-joined and coalesced rather than inner-joined: a series whose metadata /// row is missing has to keep appearing in the reader's own history. -const SERIES_TITLE_SORT: &str = "COALESCE(sm.title_sort, sm.title, s.name)"; +/// +/// Sessions outlive a hard-deleted book with a null `book_id`, so the series +/// and format breakdowns `LEFT JOIN` too, and those orphans group into a single +/// row whose name is null. The engines disagree about where a null sorts (first +/// on SQLite, last on PostgreSQL), so the orphan row is placed after every +/// named one explicitly rather than by whichever backend is running. +const SERIES_TIEBREAK: &str = + "CASE WHEN s.id IS NULL THEN 1 ELSE 0 END ASC, COALESCE(sm.title_sort, sm.title, s.name)"; + +/// The format breakdown's tiebreak, with the orphan row last. `books.format` is +/// never null, so a null format means the book is gone. +const FORMAT_TIEBREAK: &str = "CASE WHEN b.format IS NULL THEN 1 ELSE 0 END ASC, b.format"; #[derive(Debug, FromQueryResult)] struct SummaryRow { @@ -319,8 +357,8 @@ struct DeviceRow { #[derive(Debug, FromQueryResult)] struct SeriesRow { - series_id: Uuid, - series_name: String, + series_id: Option, + series_name: Option, measured_ms: i64, inferred_ms: i64, pages_read: i64, @@ -331,7 +369,7 @@ struct SeriesRow { #[derive(Debug, FromQueryResult)] struct FormatRow { - format: String, + format: Option, measured_ms: i64, inferred_ms: i64, pages_read: i64, @@ -501,7 +539,7 @@ impl ReadingStatsRepository { limit: u64, ) -> Result> { let backend = db.get_database_backend(); - let order_by = sort.order_by(SERIES_TITLE_SORT); + let order_by = sort.order_by(SERIES_TIEBREAK); let sql = format!( "SELECT s.id AS series_id, \ COALESCE(sm.title, s.name) AS series_name, \ @@ -512,8 +550,8 @@ impl ReadingStatsRepository { {COMPLETIONS_SUM} AS books_finished, \ COUNT(DISTINCT rs.book_id) AS books \ FROM reading_sessions rs \ - JOIN books b ON b.id = rs.book_id \ - JOIN series s ON s.id = b.series_id \ + LEFT JOIN books b ON b.id = rs.book_id \ + LEFT JOIN series s ON s.id = b.series_id \ LEFT JOIN series_metadata sm ON sm.series_id = s.id \ WHERE rs.user_id = $1 AND {READING_KINDS} \ AND rs.client_started_at >= $2 AND rs.client_started_at < $3 \ @@ -556,7 +594,7 @@ impl ReadingStatsRepository { sort: StatsSort, ) -> Result> { let backend = db.get_database_backend(); - let order_by = sort.order_by("format"); + let order_by = sort.order_by(FORMAT_TIEBREAK); let sql = format!( "SELECT b.format AS format, \ {MEASURED_SUM} AS measured_ms, \ @@ -565,7 +603,7 @@ impl ReadingStatsRepository { COUNT(*) AS sessions, \ {COMPLETIONS_SUM} AS books_finished \ FROM reading_sessions rs \ - JOIN books b ON b.id = rs.book_id \ + LEFT JOIN books b ON b.id = rs.book_id \ WHERE rs.user_id = $1 AND {READING_KINDS} \ AND rs.client_started_at >= $2 AND rs.client_started_at < $3 \ GROUP BY b.format \ @@ -635,6 +673,100 @@ impl ReadingStatsRepository { ) } + /// Everything [`Self::purge_orphaned`] would delete for this user, across + /// their whole history rather than any window. + /// + /// A purge is not windowed, so the confirmation in front of it cannot be + /// either: the removed-from-library row on the dashboard covers only the + /// dates on screen and would understate what is about to go. + pub async fn orphaned_totals( + db: &C, + user_id: Uuid, + ) -> Result { + #[derive(Debug, FromQueryResult)] + struct OrphanRow { + measured_ms: i64, + inferred_ms: i64, + pages_read: i64, + sessions: i64, + } + + let backend = db.get_database_backend(); + // Sittings are counted the way the dashboard counts them, so the + // confirmation agrees with the row the reader clicked. A purge also + // removes orphaned `reset` rows, but those count towards nothing. + let sql = format!( + "SELECT {MEASURED_SUM} AS measured_ms, \ + {INFERRED_SUM} AS inferred_ms, \ + {PAGES_SUM} AS pages_read, \ + COUNT(*) AS sessions \ + FROM reading_sessions rs \ + WHERE rs.user_id = $1 AND rs.book_id IS NULL AND {READING_KINDS}" + ); + let row = OrphanRow::find_by_statement(Statement::from_sql_and_values( + backend, + &sql, + [Value::Uuid(Some(Box::new(user_id)))], + )) + .one(db) + .await?; + + let completions = read_completions::Entity::find() + .filter(read_completions::Column::UserId.eq(user_id)) + .filter(read_completions::Column::BookId.is_null()) + .count(db) + .await?; + + Ok(row.map_or_else( + || OrphanedHistory { + completions, + ..OrphanedHistory::default() + }, + |r| OrphanedHistory { + duration: DurationBreakdown { + measured_ms: r.measured_ms, + inferred_ms: r.inferred_ms, + }, + pages_read: r.pages_read, + sessions: r.sessions, + completions, + }, + )) + } + + /// Delete the caller's reading history whose book has been hard-deleted, + /// and nothing else. + /// + /// Those rows keep counting towards every total until the reader decides + /// otherwise; this is that decision, so it is never run on a schedule. + /// + /// Sessions and completions go together in one transaction so the two + /// logs cannot disagree about whether the history exists. + pub async fn purge_orphaned( + db: &C, + user_id: Uuid, + ) -> Result { + let txn = db.begin().await?; + let sessions = reading_sessions::Entity::delete_many() + .filter(reading_sessions::Column::UserId.eq(user_id)) + .filter(reading_sessions::Column::BookId.is_null()) + .exec(&txn) + .await? + .rows_affected; + let completions = read_completions::Entity::delete_many() + .filter(read_completions::Column::UserId.eq(user_id)) + .filter(read_completions::Column::BookId.is_null()) + .exec(&txn) + .await? + .rows_affected; + txn.commit().await?; + + Ok(PurgedOrphanedHistory { + sessions, + completions, + }) + } + /// How many session rows a user has, for judging whether retention or /// compaction is worth building. pub async fn row_count(db: &C, user_id: Uuid) -> Result { @@ -849,7 +981,7 @@ mod tests { reading_sessions::ActiveModel { id: Set(Uuid::new_v4()), user_id: Set(user_id), - book_id: Set(book_id), + book_id: Set(Some(book_id)), device_id: Set(spec.device.to_string()), device_name: Set(spec.device_name.map(str::to_string)), pass: Set(spec.pass), @@ -1553,10 +1685,10 @@ mod tests { .unwrap(); assert_eq!(series.len(), 2); - assert_eq!(series[0].series_name, "Berserk"); + assert_eq!(series[0].series_name.as_deref(), Some("Berserk")); assert_eq!(series[0].duration.measured_ms, 90 * MINUTE_MS); assert_eq!(series[0].books, 1); - assert_eq!(series[1].series_name, "Vinland Saga"); + assert_eq!(series[1].series_name.as_deref(), Some("Vinland Saga")); } /// Coverage answers "which years can this reader ask for", so it must not @@ -1826,6 +1958,7 @@ mod tests { .unwrap()[0] .series_name .clone() + .unwrap_or_default() }; assert_eq!(top(StatsSort::Time).await, "Long Sitting"); @@ -1856,7 +1989,7 @@ mod tests { .await .unwrap(); - assert_eq!(series[0].series_name, "Prison School"); + assert_eq!(series[0].series_name.as_deref(), Some("Prison School")); } /// Metadata is created alongside every series today, but a missing row must @@ -1886,7 +2019,7 @@ mod tests { .unwrap(); assert_eq!(series.len(), 1); - assert_eq!(series[0].series_name, "Berserk"); + assert_eq!(series[0].series_name.as_deref(), Some("Berserk")); } #[tokio::test] @@ -1910,8 +2043,8 @@ mod tests { .unwrap(); assert_eq!(series.len(), 2); - assert_eq!(series[0].series_name, "A"); - assert_eq!(series[1].series_name, "B"); + assert_eq!(series[0].series_name.as_deref(), Some("A")); + assert_eq!(series[1].series_name.as_deref(), Some("B")); } #[tokio::test] @@ -1942,9 +2075,9 @@ mod tests { .unwrap(); assert_eq!(formats.len(), 2); - assert_eq!(formats[0].format, "cbz"); + assert_eq!(formats[0].format.as_deref(), Some("cbz")); assert_eq!(formats[0].duration.measured_ms, 90 * MINUTE_MS); - assert_eq!(formats[1].format, "epub"); + assert_eq!(formats[1].format.as_deref(), Some("epub")); } /// Pages are summed independently of time, so a client that reports one and @@ -2177,7 +2310,7 @@ mod tests_support { reading_sessions::ActiveModel { id: Set(Uuid::new_v4()), user_id: Set(user_id), - book_id: Set(book_id), + book_id: Set(Some(book_id)), device_id: Set(device.to_string()), device_name: Set(Some(device.to_string())), pass: Set(1), diff --git a/docs/api/openapi.json b/docs/api/openapi.json index c616015f..157908ff 100644 --- a/docs/api/openapi.json +++ b/docs/api/openapi.json @@ -10434,6 +10434,76 @@ ] } }, + "/api/v1/reading-stats/orphaned": { + "get": { + "tags": [ + "Reading Statistics" + ], + "summary": "The caller's reading history for books that no longer exist, in total", + "description": "Unwindowed: exactly what `DELETE /api/v1/reading-stats/orphaned` would\nremove, so a client can say so before asking the reader to confirm.", + "operationId": "get_orphaned_reading_history", + "responses": { + "200": { + "description": "Totals of the detached history", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/OrphanedHistoryDto" + } + } + } + }, + "401": { + "description": "Unauthorized" + }, + "403": { + "description": "Forbidden" + } + }, + "security": [ + { + "jwt_bearer": [] + }, + { + "api_key": [] + } + ] + }, + "delete": { + "tags": [ + "Reading Statistics" + ], + "summary": "Delete the caller's reading history for books that no longer exist", + "description": "When a book is deleted from the server its reading sessions and finished\nread-throughs are kept, so the time still counts towards every statistic;\nthe series and format breakdowns show it as one \"removed from library\" row.\nThis discards those rows for the caller, and only for the caller.\n\nIrreversible. Only history already detached from any book is touched;\nattributed reading is never affected.", + "operationId": "purge_orphaned_reading_history", + "responses": { + "200": { + "description": "What was deleted", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PurgedOrphanedHistoryDto" + } + } + } + }, + "401": { + "description": "Unauthorized" + }, + "403": { + "description": "Forbidden" + } + }, + "security": [ + { + "jwt_bearer": [] + }, + { + "api_key": [] + } + ] + } + }, "/api/v1/readlists": { "get": { "tags": [ @@ -35590,6 +35660,36 @@ } } }, + "OrphanedHistoryDto": { + "type": "object", + "description": "The caller's reading history whose book has since been deleted, across\nevery date. What a purge would remove.", + "required": [ + "duration", + "pagesRead", + "sessions", + "completions" + ], + "properties": { + "completions": { + "type": "integer", + "format": "int64", + "description": "Finished read-throughs.", + "minimum": 0 + }, + "duration": { + "$ref": "#/components/schemas/DurationBreakdownDto" + }, + "pagesRead": { + "type": "integer", + "format": "int64" + }, + "sessions": { + "type": "integer", + "format": "int64", + "description": "Sittings, counted as the dashboard counts them." + } + } + }, "PageDto": { "type": "object", "description": "Page data transfer object", @@ -38682,6 +38782,28 @@ } } }, + "PurgedOrphanedHistoryDto": { + "type": "object", + "description": "What purging the removed-from-library history deleted.", + "required": [ + "sessionsRemoved", + "completionsRemoved" + ], + "properties": { + "completionsRemoved": { + "type": "integer", + "format": "int64", + "description": "Finished read-throughs deleted.", + "minimum": 0 + }, + "sessionsRemoved": { + "type": "integer", + "format": "int64", + "description": "Reading sessions deleted. Their time no longer counts anywhere.", + "minimum": 0 + } + } + }, "QueueHealthMetricsDto": { "type": "object", "description": "Queue health metrics", @@ -39002,7 +39124,7 @@ "type": "object", "description": "Totals for one file format.", "required": [ - "format", + "removedFromLibrary", "duration", "pagesRead", "sessions", @@ -39017,13 +39139,21 @@ "$ref": "#/components/schemas/DurationBreakdownDto" }, "format": { - "type": "string", + "type": [ + "string", + "null" + ], + "description": "Null on the removed-from-library row.", "example": "cbz" }, "pagesRead": { "type": "integer", "format": "int64" }, + "removedFromLibrary": { + "type": "boolean", + "description": "True on the single row that gathers reading whose book has since been\ndeleted from the server, and whose format is therefore unknown." + }, "sessions": { "type": "integer", "format": "int64" @@ -39034,8 +39164,7 @@ "type": "object", "description": "Totals for one series, most-read first.", "required": [ - "seriesId", - "seriesName", + "removedFromLibrary", "duration", "pagesRead", "sessions", @@ -39060,12 +39189,24 @@ "type": "integer", "format": "int64" }, + "removedFromLibrary": { + "type": "boolean", + "description": "True on the single row that gathers reading whose book has since been\ndeleted from the server. That time still counts towards every total,\nbut which series it belonged to is no longer known. Its `books` and\n`booksFinished` are always 0: both count distinct books, and deleted\nbooks cannot be told apart." + }, "seriesId": { - "type": "string", - "format": "uuid" + "type": [ + "string", + "null" + ], + "format": "uuid", + "description": "Null on the removed-from-library row." }, "seriesName": { - "type": "string", + "type": [ + "string", + "null" + ], + "description": "Null on the removed-from-library row.", "example": "Berserk" }, "sessions": { diff --git a/docs/docs/libraries.md b/docs/docs/libraries.md index ff3e2f10..f0b088d6 100644 --- a/docs/docs/libraries.md +++ b/docs/docs/libraries.md @@ -300,6 +300,12 @@ Deleted files are soft-deleted in the database: - Can be restored if file returns - Permanent deletion available via API +Permanent deletion (purging deleted books, or deleting the library) removes the +book for good, including your current progress in it. Your reading time and +finished read-throughs are kept and still count in your statistics, under a +**Removed from library** line; see +[Reading Progress](./reading-progress.md#when-a-book-is-deleted-from-the-server). + ### Moving Files If you move files: diff --git a/docs/docs/reading-progress.md b/docs/docs/reading-progress.md index ab7572b0..d0d7235e 100644 --- a/docs/docs/reading-progress.md +++ b/docs/docs/reading-progress.md @@ -114,6 +114,42 @@ whichever book is holding the count up updates the series straight away. Since the series number is the *lowest* count across its books, it only moves if that book was the one setting it. +## When a book is deleted from the server + +A scan that finds a file missing only hides the book (a soft delete), and +everything about it, progress and history included, comes back if the file +does. A **permanent** deletion is different: purging deleted books, or deleting +a whole library, removes the book's row for good. + +What happens to your reading then: + +- **Your reading time, pages and sittings are kept.** They still count towards + every total on the **Reading Statistics** page, the calendar and the + per-device breakdown. The time happened; removing the file does not undo it. +- **Which book it was is lost.** The series and format panels show that time on + one row labelled **Removed from library**, since there is no longer a series + or format to put it under. +- **Books read and books finished drop the deleted book.** Both count distinct + books, and two deleted books can no longer be told apart, so they are left + out rather than guessed at. Time, pages and sittings are unaffected. +- **Current progress is removed.** Where you were in a book that no longer + exists has nothing to resume, so your position in it goes with the book. + Your finished read-throughs are kept, detached from the book, like your + sittings. + +### Deleting the removed-from-library history + +Whenever you have any, the **Reading Statistics** page shows a **Reading of +removed books** notice above the series panel, whatever dates you are viewing. +If you would rather that reading stopped counting, use **Delete** on that +notice. The confirmation states how many sittings, how much reading time and +how many finished read-throughs will go, across **all** dates rather than only +the ones on screen, because that is what is deleted. It is permanent, affects only your own history, and never +touches reading attributed to a book that still exists. + +The same is available through the API: `GET /api/v1/reading-stats/orphaned` +returns the totals, and `DELETE /api/v1/reading-stats/orphaned` removes them. + ## What is not recorded - **Abandoned reads leave no trace.** Marking a book unread at page 50 without diff --git a/migration/src/lib.rs b/migration/src/lib.rs index c9c25b38..74eaff10 100644 --- a/migration/src/lib.rs +++ b/migration/src/lib.rs @@ -215,6 +215,7 @@ mod m20260825_000110_add_task_progress; mod m20260826_000111_normalize_library_reading_direction; mod m20260826_000112_create_user_series_reader_settings; mod m20260828_000113_relax_renumber_series_task_dedup; +mod m20260927_000114_reading_history_survives_book_delete; pub struct Migrator; @@ -414,6 +415,9 @@ impl MigratorTrait for Migrator { // already running: that pass may have read the series before the // book existed, so the request would be lost. Box::new(m20260828_000113_relax_renumber_series_task_dedup::Migration), + // Reading history keeps its rows when the book is hard-deleted; + // only the attribution is lost. + Box::new(m20260927_000114_reading_history_survives_book_delete::Migration), ] } } diff --git a/migration/src/m20260927_000114_reading_history_survives_book_delete.rs b/migration/src/m20260927_000114_reading_history_survives_book_delete.rs new file mode 100644 index 00000000..b1263fe6 --- /dev/null +++ b/migration/src/m20260927_000114_reading_history_survives_book_delete.rs @@ -0,0 +1,542 @@ +//! Let reading history outlive the books it was recorded against. +//! +//! `reading_sessions` is the only source of every reading statistic and +//! `read_completions` the only record that a book was ever finished. Both +//! cascaded on `books.id`, so purging deleted books or removing a library +//! erased hours the user really spent reading, with nothing left to rebuild +//! them from. A session records something that happened; it does not stop +//! having happened because the file was later removed. +//! +//! After this migration `book_id` is nullable on both tables and its foreign +//! key is `ON DELETE SET NULL`: the row survives and loses only its +//! attribution. `user_id` keeps `ON DELETE CASCADE`, because deleting a user +//! must still remove their history. +//! +//! # How each backend gets there +//! +//! PostgreSQL alters the column and swaps the constraint in place. +//! +//! SQLite cannot alter a column's nullability or a foreign key's action, so +//! each table is rebuilt: create a replacement, copy every row, drop the +//! original, rename the replacement, recreate the indexes. SQLite's documented +//! procedure also switches `foreign_keys` off around the swap, but that step +//! exists to stop dropping a *parent* table from cascading into its children. +//! Nothing references a session or a completion, so these tables have no +//! children and the rebuild is safe with enforcement left on. That matters: the +//! pragma is a no-op inside a transaction and is per connection in a pool, +//! which is what made the swap unreliable in +//! `m20260508_000081_add_release_sources_plugin_uuid_fk`. Here each rebuild +//! runs in one transaction and never touches the pragma. +//! +//! The copy is checked by row count before the original is dropped, and every +//! index from the create migrations is recreated with its original columns and +//! ordering; a rebuild that silently lost an index would slow the statistics +//! queries without failing anything. +//! +//! # A `book_id` index +//! +//! Neither table had an index leading with `book_id`, so every deleted book +//! made the foreign key action scan both tables in full. Deleting a library +//! deletes every book in it, one scan each. That was already true under +//! `CASCADE`; `SET NULL` additionally rewrites the rows it finds, and the +//! rebuild is the cheapest moment to add the index. +//! +//! # Rollback is lossy +//! +//! `down` restores `NOT NULL` and `CASCADE`, which orphaned rows cannot +//! satisfy, so it deletes every row whose `book_id` is null first. Those are +//! exactly the rows this migration exists to keep. + +use sea_orm::{ + ConnectionTrait, DatabaseTransaction, DbBackend, DbErr, Statement, TransactionTrait, +}; +use sea_orm_migration::prelude::*; + +#[derive(DeriveMigrationName)] +pub struct Migration; + +#[async_trait::async_trait] +impl MigrationTrait for Migration { + async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> { + migrate(manager, Direction::Up).await + } + + async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { + migrate(manager, Direction::Down).await + } +} + +#[derive(Clone, Copy, PartialEq, Eq)] +enum Direction { + /// `book_id` nullable, `ON DELETE SET NULL`. + Up, + /// `book_id` required, `ON DELETE CASCADE`, as originally created. + Down, +} + +impl Direction { + fn on_delete(self) -> ForeignKeyAction { + match self { + Self::Up => ForeignKeyAction::SetNull, + Self::Down => ForeignKeyAction::Cascade, + } + } +} + +const HISTORY_TABLES: [&str; 2] = ["reading_sessions", "read_completions"]; + +fn book_index_name(table: &str) -> String { + format!("idx_{table}_book_id") +} + +/// `down` restores `NOT NULL`, which orphaned rows cannot satisfy. +async fn delete_orphans(db: &C) -> Result<(), DbErr> { + for table in HISTORY_TABLES { + db.execute_unprepared(&format!("DELETE FROM {table} WHERE book_id IS NULL")) + .await?; + } + Ok(()) +} + +async fn migrate(manager: &SchemaManager<'_>, direction: Direction) -> Result<(), DbErr> { + let db = manager.get_connection(); + + match db.get_database_backend() { + // Already inside the migrator's transaction. + DbBackend::Postgres => { + if direction == Direction::Down { + delete_orphans(db).await?; + for table in HISTORY_TABLES { + db.execute_unprepared(&format!( + "DROP INDEX IF EXISTS {}", + book_index_name(table) + )) + .await?; + } + } + alter_postgres( + db, + "reading_sessions", + "fk_reading_sessions_book_id", + direction, + ) + .await?; + alter_postgres( + db, + "read_completions", + "fk_read_completions_book_id", + direction, + ) + .await?; + if direction == Direction::Up { + for table in HISTORY_TABLES { + db.execute_unprepared(&format!( + "CREATE INDEX {} ON {table} (book_id)", + book_index_name(table) + )) + .await?; + } + } + } + // The migrator does not wrap SQLite migrations in a transaction, so + // this one opens its own: a rollback that deleted the orphans and then + // failed the rebuild would otherwise lose them for nothing. + DbBackend::Sqlite => { + let txn = db.begin().await?; + if direction == Direction::Down { + delete_orphans(&txn).await?; + } + rebuild_sqlite(&txn, &ReadingSessionsTable, direction).await?; + rebuild_sqlite(&txn, &ReadCompletionsTable, direction).await?; + txn.commit().await?; + } + DbBackend::MySql => { + return Err(DbErr::Migration( + "MySQL is not a supported backend".to_string(), + )); + } + } + + Ok(()) +} + +async fn alter_postgres( + db: &C, + table: &str, + fk_name: &str, + direction: Direction, +) -> Result<(), DbErr> { + let (nullability, action) = match direction { + Direction::Up => ("DROP NOT NULL", "SET NULL"), + Direction::Down => ("SET NOT NULL", "CASCADE"), + }; + db.execute_unprepared(&format!( + "ALTER TABLE {table} ALTER COLUMN book_id {nullability}" + )) + .await?; + db.execute_unprepared(&format!("ALTER TABLE {table} DROP CONSTRAINT {fk_name}")) + .await?; + db.execute_unprepared(&format!( + "ALTER TABLE {table} ADD CONSTRAINT {fk_name} FOREIGN KEY (book_id) \ + REFERENCES books (id) ON DELETE {action} ON UPDATE NO ACTION" + )) + .await?; + Ok(()) +} + +/// One table's shape, as SQLite needs it to rebuild the table from scratch. +trait RebuildableTable: Sync { + fn name(&self) -> &'static str; + /// Every column, in the order the copy lists them. + fn columns(&self) -> &'static [&'static str]; + /// The full table definition under `table_name`, with `book_id` shaped by + /// `direction`. + fn create(&self, table_name: &str, direction: Direction) -> TableCreateStatement; + /// Every index the create migration made, against the final table name. + fn indexes(&self) -> Vec; +} + +async fn rebuild_sqlite( + txn: &DatabaseTransaction, + table: &dyn RebuildableTable, + direction: Direction, +) -> Result<(), DbErr> { + let backend = DbBackend::Sqlite; + let name = table.name(); + let staging = format!("{name}_rebuild"); + let columns = table.columns().join(", "); + + // A row that already references a missing user or book would fail the + // copy below with SQLite's bare "FOREIGN KEY constraint failed", and since + // migrations run at startup the server would not start. Say which table. + let existing = foreign_key_violations(txn, name).await?; + if existing > 0 { + return Err(DbErr::Migration(format!( + "{name} has {existing} rows referencing a missing user or book; \ + `PRAGMA foreign_key_check({name})` lists them. Delete them and restart." + ))); + } + + let before = count_rows(txn, name).await?; + + txn.execute(backend.build(&table.create(&staging, direction))) + .await?; + txn.execute_unprepared(&format!( + "INSERT INTO {staging} ({columns}) SELECT {columns} FROM {name}" + )) + .await?; + + let copied = count_rows(txn, &staging).await?; + if copied != before { + return Err(DbErr::Migration(format!( + "rebuilding {name} copied {copied} of {before} rows; aborting before the original is dropped" + ))); + } + + // Dropping the original drops its indexes with it, which frees their + // names for the recreation below. + txn.execute_unprepared(&format!("DROP TABLE {name}")) + .await?; + txn.execute_unprepared(&format!("ALTER TABLE {staging} RENAME TO {name}")) + .await?; + for index in table.indexes() { + txn.execute(backend.build(&index)).await?; + } + if direction == Direction::Up { + txn.execute_unprepared(&format!( + "CREATE INDEX {} ON {name} (book_id)", + book_index_name(name) + )) + .await?; + } + + let violations = foreign_key_violations(txn, name).await?; + if violations > 0 { + return Err(DbErr::Migration(format!( + "rebuilding {name} left {violations} foreign key violations" + ))); + } + + Ok(()) +} + +async fn foreign_key_violations(txn: &DatabaseTransaction, table: &str) -> Result { + Ok(txn + .query_all(Statement::from_string( + DbBackend::Sqlite, + format!("PRAGMA foreign_key_check({table})"), + )) + .await? + .len()) +} + +async fn count_rows(db: &C, table: &str) -> Result { + let row = db + .query_one(Statement::from_string( + db.get_database_backend(), + format!("SELECT COUNT(*) AS n FROM {table}"), + )) + .await? + .ok_or_else(|| DbErr::Migration(format!("COUNT(*) on {table} returned no row")))?; + row.try_get("", "n") +} + +fn book_id_column(direction: Direction) -> ColumnDef { + let mut column = ColumnDef::new(Alias::new("book_id")); + column.uuid(); + match direction { + Direction::Up => column.null(), + Direction::Down => column.not_null(), + }; + column +} + +struct ReadingSessionsTable; + +impl RebuildableTable for ReadingSessionsTable { + fn name(&self) -> &'static str { + "reading_sessions" + } + + fn columns(&self) -> &'static [&'static str] { + &[ + "id", + "user_id", + "book_id", + "device_id", + "device_name", + "pass", + "kind", + "to_page", + "to_percentage", + "r2_progression", + "active_duration_ms", + "duration_source", + "pages_read", + "client_started_at", + "client_ended_at", + "server_recorded_at", + ] + } + + // Mirrors `m20260814_000105_create_reading_sessions` column for column. + fn create(&self, table_name: &str, direction: Direction) -> TableCreateStatement { + let table = Alias::new(table_name); + Table::create() + .table(table.clone()) + .col( + ColumnDef::new(ReadingSessions::Id) + .uuid() + .not_null() + .primary_key(), + ) + .col(ColumnDef::new(ReadingSessions::UserId).uuid().not_null()) + .col(book_id_column(direction)) + .col( + ColumnDef::new(ReadingSessions::DeviceId) + .string() + .not_null(), + ) + .col(ColumnDef::new(ReadingSessions::DeviceName).string().null()) + .col( + ColumnDef::new(ReadingSessions::Pass) + .integer() + .not_null() + .default(1), + ) + .col(ColumnDef::new(ReadingSessions::Kind).string().not_null()) + .col(ColumnDef::new(ReadingSessions::ToPage).integer().null()) + .col( + ColumnDef::new(ReadingSessions::ToPercentage) + .double() + .null(), + ) + .col(ColumnDef::new(ReadingSessions::R2Progression).text().null()) + .col( + ColumnDef::new(ReadingSessions::ActiveDurationMs) + .big_integer() + .null(), + ) + .col( + ColumnDef::new(ReadingSessions::DurationSource) + .string() + .not_null() + .default("unknown"), + ) + .col(ColumnDef::new(ReadingSessions::PagesRead).integer().null()) + .col( + ColumnDef::new(ReadingSessions::ClientStartedAt) + .timestamp_with_time_zone() + .not_null(), + ) + .col( + ColumnDef::new(ReadingSessions::ClientEndedAt) + .timestamp_with_time_zone() + .not_null(), + ) + .col( + ColumnDef::new(ReadingSessions::ServerRecordedAt) + .timestamp_with_time_zone() + .not_null(), + ) + .foreign_key( + ForeignKey::create() + .name("fk_reading_sessions_user_id") + .from(table.clone(), ReadingSessions::UserId) + .to(Users::Table, Users::Id) + .on_delete(ForeignKeyAction::Cascade) + .on_update(ForeignKeyAction::NoAction), + ) + .foreign_key( + ForeignKey::create() + .name("fk_reading_sessions_book_id") + .from(table, ReadingSessions::BookId) + .to(Books::Table, Books::Id) + .on_delete(direction.on_delete()) + .on_update(ForeignKeyAction::NoAction), + ) + .to_owned() + } + + fn indexes(&self) -> Vec { + vec![ + Index::create() + .name("idx_reading_sessions_fold") + .table(ReadingSessions::Table) + .col(ReadingSessions::UserId) + .col(ReadingSessions::BookId) + .col(ReadingSessions::Pass) + .col(ReadingSessions::ClientEndedAt) + .to_owned(), + Index::create() + .name("idx_reading_sessions_stats") + .table(ReadingSessions::Table) + .col(ReadingSessions::UserId) + .col(ReadingSessions::ClientStartedAt) + .to_owned(), + Index::create() + .name("idx_reading_sessions_coalesce") + .table(ReadingSessions::Table) + .col(ReadingSessions::UserId) + .col(ReadingSessions::BookId) + .col(ReadingSessions::DeviceId) + .col(ReadingSessions::Pass) + .col((ReadingSessions::ClientEndedAt, IndexOrder::Desc)) + .to_owned(), + ] + } +} + +struct ReadCompletionsTable; + +impl RebuildableTable for ReadCompletionsTable { + fn name(&self) -> &'static str { + "read_completions" + } + + fn columns(&self) -> &'static [&'static str] { + &["id", "user_id", "book_id", "started_at", "completed_at"] + } + + // Mirrors `m20260729_000103_create_read_completions` column for column. + fn create(&self, table_name: &str, direction: Direction) -> TableCreateStatement { + let table = Alias::new(table_name); + Table::create() + .table(table.clone()) + .col( + ColumnDef::new(ReadCompletions::Id) + .uuid() + .not_null() + .primary_key(), + ) + .col(ColumnDef::new(ReadCompletions::UserId).uuid().not_null()) + .col(book_id_column(direction)) + .col( + ColumnDef::new(ReadCompletions::StartedAt) + .timestamp_with_time_zone() + .not_null(), + ) + .col( + ColumnDef::new(ReadCompletions::CompletedAt) + .timestamp_with_time_zone() + .not_null(), + ) + .foreign_key( + ForeignKey::create() + .name("fk_read_completions_user_id") + .from(table.clone(), ReadCompletions::UserId) + .to(Users::Table, Users::Id) + .on_delete(ForeignKeyAction::Cascade) + .on_update(ForeignKeyAction::NoAction), + ) + .foreign_key( + ForeignKey::create() + .name("fk_read_completions_book_id") + .from(table, ReadCompletions::BookId) + .to(Books::Table, Books::Id) + .on_delete(direction.on_delete()) + .on_update(ForeignKeyAction::NoAction), + ) + .to_owned() + } + + fn indexes(&self) -> Vec { + vec![ + Index::create() + .name("idx_read_completions_user_book") + .table(ReadCompletions::Table) + .col(ReadCompletions::UserId) + .col(ReadCompletions::BookId) + .to_owned(), + Index::create() + .name("idx_read_completions_user_date") + .table(ReadCompletions::Table) + .col(ReadCompletions::UserId) + .col((ReadCompletions::CompletedAt, IndexOrder::Desc)) + .to_owned(), + ] + } +} + +#[derive(DeriveIden)] +enum ReadingSessions { + Table, + Id, + UserId, + BookId, + DeviceId, + DeviceName, + Pass, + Kind, + ToPage, + ToPercentage, + R2Progression, + ActiveDurationMs, + DurationSource, + PagesRead, + ClientStartedAt, + ClientEndedAt, + ServerRecordedAt, +} + +#[derive(DeriveIden)] +enum ReadCompletions { + Table, + Id, + UserId, + BookId, + StartedAt, + CompletedAt, +} + +#[derive(DeriveIden)] +enum Users { + Table, + Id, +} + +#[derive(DeriveIden)] +enum Books { + Table, + Id, +} diff --git a/tests/api/reading_stats.rs b/tests/api/reading_stats.rs index 9b595784..5e7597ab 100644 --- a/tests/api/reading_stats.rs +++ b/tests/api/reading_stats.rs @@ -4,7 +4,9 @@ mod common; use chrono::{DateTime, Duration, TimeZone, Utc}; -use codex::api::routes::v1::dto::ReadingStatsResponse; +use codex::api::routes::v1::dto::{ + OrphanedHistoryDto, PurgedOrphanedHistoryDto, ReadingStatsResponse, +}; use codex::db::ScanningStrategy; use codex::db::repositories::{ BookRepository, LibraryRepository, SeriesRepository, UserRepository, @@ -191,8 +193,8 @@ async fn one_response_carries_every_breakdown() { assert_eq!(stats.formats.len(), 2); assert_eq!(stats.devices[0].device_id, "phone", "ranked by time read"); - assert_eq!(stats.series[0].series_name, "Berserk"); - assert_eq!(stats.formats[0].format, "cbz"); + assert_eq!(stats.series[0].series_name.as_deref(), Some("Berserk")); + assert_eq!(stats.formats[0].format.as_deref(), Some("cbz")); } /// A reader with no history gets an empty dashboard rather than an error. @@ -364,6 +366,7 @@ async fn the_sort_key_decides_which_series_survive_the_limit() { body.expect("expected a JSON body").series[0] .series_name .clone() + .unwrap_or_default() } }; @@ -638,3 +641,182 @@ async fn the_response_reports_the_window_it_used() { "the default window covers roughly the last ninety days" ); } + +// ============================================================================ +// Reading whose book was deleted +// ============================================================================ + +async fn hard_delete_book(db: &sea_orm::DatabaseConnection, book_id: Uuid) { + use sea_orm::EntityTrait; + codex::db::entities::books::Entity::delete_by_id(book_id) + .exec(db) + .await + .unwrap(); +} + +async fn orphaned_completions(db: &sea_orm::DatabaseConnection, user_id: Uuid) -> u64 { + use codex::db::entities::read_completions; + use sea_orm::{ColumnTrait, EntityTrait, PaginatorTrait, QueryFilter}; + read_completions::Entity::find() + .filter(read_completions::Column::UserId.eq(user_id)) + .filter(read_completions::Column::BookId.is_null()) + .count(db) + .await + .unwrap() +} + +async fn purge_orphans( + state: std::sync::Arc, + token: &str, +) -> (StatusCode, Option) { + let app = create_test_router(state).await; + let request = delete_request_with_auth("/api/v1/reading-stats/orphaned", token); + make_json_request(app, request).await +} + +/// Reading a deleted book keeps counting, and the series and format panels +/// show it as one flagged row rather than dropping it or naming a null series. +#[tokio::test] +async fn deleted_books_reading_shows_as_one_flagged_row() { + let (db, _temp_dir) = setup_test_db().await; + let kept = book_in_series(&db, "Berserk", "cbz").await; + let removed = book_in_series(&db, "Dune", "epub").await; + let state = create_test_auth_state(db.clone()).await; + let (_user_id, token) = admin_and_token(&db, &state, "reader").await; + + record_session(state.clone(), &token, kept.id, "phone", 60, at(3)).await; + record_session(state.clone(), &token, removed.id, "phone", 20, at(4)).await; + hard_delete_book(&db, removed.id).await; + + let window = format!("?from={}&to={}", q(at(1)), q(at(30))); + let (status, response) = fetch_stats(state, &token, &window).await; + let stats = response.expect("expected a JSON body"); + assert_eq!(status, StatusCode::OK); + + assert_eq!( + stats.summary.duration.total_ms, + 80 * MINUTE_MS, + "the deleted book's time still counts" + ); + + assert_eq!(stats.series.len(), 2); + assert!(!stats.series[0].removed_from_library); + assert_eq!(stats.series[0].series_name.as_deref(), Some("Berserk")); + let bucket = &stats.series[1]; + assert!(bucket.removed_from_library); + assert!(bucket.series_id.is_none()); + assert!(bucket.series_name.is_none()); + assert_eq!(bucket.duration.total_ms, 20 * MINUTE_MS); + + let formats: Vec<_> = stats + .formats + .iter() + .map(|f| (f.format.clone(), f.removed_from_library)) + .collect(); + assert_eq!( + formats, + vec![(Some("cbz".to_string()), false), (None, true)], + "the epub is gone, so its time is in the removed row, ranked like any other" + ); +} + +/// Purging removes exactly the caller's orphaned history: their attributed +/// reading stays, and another reader's orphans of the same book are untouched. +#[tokio::test] +async fn purging_orphaned_history_is_scoped_to_the_caller() { + let (db, _temp_dir) = setup_test_db().await; + let kept = book_in_series(&db, "Berserk", "cbz").await; + let removed = book_in_series(&db, "Dune", "epub").await; + let state = create_test_auth_state(db.clone()).await; + let (purger, purger_token) = admin_and_token(&db, &state, "purger").await; + let (_bystander, bystander_token) = admin_and_token(&db, &state, "bystander").await; + + record_session(state.clone(), &purger_token, kept.id, "phone", 60, at(3)).await; + record_reading( + state.clone(), + &purger_token, + removed.id, + "phone", + 20, + 10, + "completed", + at(4), + ) + .await; + record_session( + state.clone(), + &bystander_token, + removed.id, + "tablet", + 15, + at(5), + ) + .await; + hard_delete_book(&db, removed.id).await; + + let completions = orphaned_completions(&db, purger).await; + let (status, purged) = purge_orphans(state.clone(), &purger_token).await; + assert_eq!(status, StatusCode::OK); + let purged = purged.expect("expected a JSON body"); + assert_eq!(purged.sessions_removed, 1); + assert_eq!(purged.completions_removed, completions); + assert_eq!(orphaned_completions(&db, purger).await, 0); + + let window = format!("?from={}&to={}", q(at(1)), q(at(30))); + let (_, mine) = fetch_stats(state.clone(), &purger_token, &window).await; + let mine = mine.expect("expected a JSON body"); + assert_eq!(mine.summary.duration.total_ms, 60 * MINUTE_MS); + assert!(mine.series.iter().all(|s| !s.removed_from_library)); + + let (_, theirs) = fetch_stats(state.clone(), &bystander_token, &window).await; + let theirs = theirs.expect("expected a JSON body"); + assert_eq!(theirs.summary.duration.total_ms, 15 * MINUTE_MS); + assert!(theirs.series[0].removed_from_library); + + // Nothing left to purge is not an error. + let (status, again) = purge_orphans(state, &purger_token).await; + assert_eq!(status, StatusCode::OK); + let again = again.expect("expected a JSON body"); + assert_eq!((again.sessions_removed, again.completions_removed), (0, 0)); +} + +/// The confirmation before a purge has to state what will actually go, which +/// is every orphaned row regardless of the window the dashboard is showing. +#[tokio::test] +async fn orphaned_totals_ignore_the_window_and_the_other_reader() { + let (db, _temp_dir) = setup_test_db().await; + let removed = book_in_series(&db, "Dune", "epub").await; + let state = create_test_auth_state(db.clone()).await; + let (_reader, token) = admin_and_token(&db, &state, "reader").await; + let (_other, other_token) = admin_and_token(&db, &state, "other").await; + + // Two months apart, so no single dashboard window shows both. + record_session(state.clone(), &token, removed.id, "phone", 20, at(3)).await; + record_session( + state.clone(), + &token, + removed.id, + "phone", + 40, + at(3) - Duration::days(60), + ) + .await; + record_session(state.clone(), &other_token, removed.id, "tablet", 5, at(4)).await; + hard_delete_book(&db, removed.id).await; + + let app = create_test_router(state.clone()).await; + let request = get_request_with_auth("/api/v1/reading-stats/orphaned", &token); + let (status, body): (_, Option) = make_json_request(app, request).await; + assert_eq!(status, StatusCode::OK); + let body = body.expect("expected a JSON body"); + assert_eq!(body.sessions, 2); + assert_eq!(body.duration.total_ms, 60 * MINUTE_MS); + assert_eq!(body.completions, 0); + + purge_orphans(state.clone(), &token).await; + let app = create_test_router(state).await; + let request = get_request_with_auth("/api/v1/reading-stats/orphaned", &token); + let (_, body): (_, Option) = make_json_request(app, request).await; + let body = body.expect("expected a JSON body"); + assert_eq!((body.sessions, body.duration.total_ms), (0, 0)); +} diff --git a/tests/db/mod.rs b/tests/db/mod.rs index 450d841a..0cfd457f 100644 --- a/tests/db/mod.rs +++ b/tests/db/mod.rs @@ -9,6 +9,7 @@ mod entity_event_bridge; mod migrations; mod oidc_pending_state; mod postgres; +mod reading_history_retention; mod reading_sessions; mod reading_stats; mod refresh_token_repository; diff --git a/tests/db/reading_history_retention.rs b/tests/db/reading_history_retention.rs new file mode 100644 index 00000000..4cb89bb8 --- /dev/null +++ b/tests/db/reading_history_retention.rs @@ -0,0 +1,611 @@ +//! Reading history outlives the books it was recorded against. +//! +//! `reading_sessions` is the only source of every reading statistic, and +//! `read_completions` is the only record that a book was ever finished. Both +//! used to cascade on `books.id`, so purging deleted books or removing a +//! library destroyed hours the user really did spend reading, with nothing left +//! to reconstruct them from. +//! +//! A session is a record of something that happened. It does not stop having +//! happened because the file was later removed, so the row survives the delete +//! and loses only its attribution. +//! +//! The user cascade is deliberately unchanged and tested here too: deleting a +//! user must still take their history with them. + +#[path = "../common/mod.rs"] +mod common; + +use chrono::{Duration, Utc}; +use codex::db::ScanningStrategy; +use codex::db::entities::reading_sessions::SessionKind; +use codex::db::entities::{books, read_completions, reading_sessions, users}; +use codex::db::repositories::{ + BookRepository, LibraryRepository, NewSession, ReadCompletionRepository, + ReadProgressRepository, ReadingStatsRepository, SeriesRepository, StatsGranularity, StatsSort, + StatsWindow, UserRepository, +}; +use common::*; +use migration::{Migrator, MigratorTrait}; +use sea_orm::{ + ColumnTrait, ConnectionTrait, DatabaseBackend, DatabaseConnection, EntityTrait, QueryFilter, + Statement, +}; +use tempfile::TempDir; +use uuid::Uuid; + +fn unique() -> String { + Uuid::new_v4().to_string() +} + +async fn persist_user(db: &DatabaseConnection) -> Uuid { + let handle = format!("reader-{}", unique()); + let model = create_test_user(&handle, &format!("{handle}@test.test"), "hash", true); + UserRepository::create(db, &model).await.unwrap().id +} + +async fn persist_book(db: &DatabaseConnection) -> Uuid { + let library = LibraryRepository::create( + db, + "Lib", + &format!("/lib/{}", unique()), + ScanningStrategy::Default, + ) + .await + .unwrap(); + let series = SeriesRepository::create(db, library.id, "Series", None) + .await + .unwrap(); + let book = create_test_book( + series.id, + library.id, + &format!("/lib/{}.cbz", unique()), + "book", + &format!("hash_{}", unique()), + "cbz", + 200, + ); + BookRepository::create(db, &book, None).await.unwrap().id +} + +/// One measured sitting plus one banked completion, which is the pair a +/// finished read leaves behind. +async fn record_history(db: &DatabaseConnection, user: Uuid, book: Uuid) { + let now = Utc::now(); + let session = NewSession::from_client( + Uuid::new_v4(), + user, + book, + "device-1", + Some("Codex Web".to_string()), + SessionKind::Progress, + Some(900_000), + Some(24), + now - Duration::minutes(20), + now - Duration::minutes(5), + ) + .with_page(24); + + ReadProgressRepository::record_session(db, session) + .await + .unwrap(); + + ReadCompletionRepository::record(db, user, book, now - Duration::minutes(20), now) + .await + .unwrap(); +} + +async fn sessions_for_user(db: &DatabaseConnection, user: Uuid) -> Vec { + reading_sessions::Entity::find() + .filter(reading_sessions::Column::UserId.eq(user)) + .all(db) + .await + .unwrap() +} + +async fn completions_for_user(db: &DatabaseConnection, user: Uuid) -> Vec { + read_completions::Entity::find() + .filter(read_completions::Column::UserId.eq(user)) + .all(db) + .await + .unwrap() +} + +/// Hard-deleting a book keeps the history and nulls only the attribution. +/// +/// This is the whole point: `purge_deleted_in_library` and library deletion both +/// reach a real `DELETE`, and either one used to erase the statistics silently. +async fn exercise_history_survives_book_delete(db: &DatabaseConnection) { + let user = persist_user(db).await; + let book = persist_book(db).await; + record_history(db, user, book).await; + + assert_eq!( + sessions_for_user(db, user).await.len(), + 1, + "fixture should have recorded one session" + ); + assert_eq!( + completions_for_user(db, user).await.len(), + 1, + "fixture should have recorded one completion" + ); + + books::Entity::delete_by_id(book).exec(db).await.unwrap(); + + let sessions = sessions_for_user(db, user).await; + assert_eq!( + sessions.len(), + 1, + "the session must outlive the book it was recorded against" + ); + assert!( + sessions[0].book_id.is_none(), + "the surviving session must lose its attribution, not keep a dangling book id" + ); + + let completions = completions_for_user(db, user).await; + assert_eq!( + completions.len(), + 1, + "the completion must outlive the book it was recorded against" + ); + assert!( + completions[0].book_id.is_none(), + "the surviving completion must lose its attribution, not keep a dangling book id" + ); +} + +/// Deleting the user still takes their history. That cascade is unchanged, and +/// a migration that relaxed it by accident would be a privacy defect. +async fn exercise_user_delete_still_removes_history(db: &DatabaseConnection) { + let user = persist_user(db).await; + let book = persist_book(db).await; + record_history(db, user, book).await; + + users::Entity::delete_by_id(user).exec(db).await.unwrap(); + + assert!( + sessions_for_user(db, user).await.is_empty(), + "deleting a user must still remove their sessions" + ); + assert!( + completions_for_user(db, user).await.is_empty(), + "deleting a user must still remove their completions" + ); +} + +/// Everything the statistics page shows that is a plain sum over sessions. +/// +/// `books` and `books_finished` are deliberately absent: both count distinct +/// books, and a book that no longer exists cannot be told apart from another +/// deleted one, so those two figures lose the deleted book's contribution by +/// design. Time, pages and sittings are additive per row and must not move. +#[derive(Debug, PartialEq)] +struct AdditiveTotals { + summary: (i64, i64, i64, i64), + periods: Vec<(String, i64, i64, i64)>, + devices: Vec<(String, i64, i64, i64)>, + first_read_at: Option>, + last_read_at: Option>, +} + +fn whole_history() -> StatsWindow { + StatsWindow { + from: Utc::now() - Duration::days(30), + to: Utc::now() + Duration::days(1), + } +} + +async fn additive_totals(db: &DatabaseConnection, user: Uuid) -> AdditiveTotals { + let window = whole_history(); + let summary = ReadingStatsRepository::summary(db, user, window) + .await + .unwrap(); + let periods = ReadingStatsRepository::by_period(db, user, window, StatsGranularity::Day, 0) + .await + .unwrap(); + let devices = ReadingStatsRepository::by_device(db, user, window, StatsSort::Time) + .await + .unwrap(); + let coverage = ReadingStatsRepository::coverage(db, user).await.unwrap(); + + AdditiveTotals { + summary: ( + summary.duration.measured_ms, + summary.duration.inferred_ms, + summary.pages_read, + summary.sessions, + ), + periods: periods + .into_iter() + .map(|p| (p.bucket, p.duration.total_ms(), p.pages_read, p.sessions)) + .collect(), + devices: devices + .into_iter() + .map(|d| (d.device_id, d.duration.total_ms(), d.pages_read, d.sessions)) + .collect(), + first_read_at: coverage.first_read_at, + last_read_at: coverage.last_read_at, + } +} + +/// A hard delete changes no additive total, and the two breakdowns that name a +/// book's series or format carry its time in one "removed from library" row +/// instead of dropping it. +async fn exercise_statistics_keep_deleted_reading(db: &DatabaseConnection) { + let user = persist_user(db).await; + let kept = persist_book(db).await; + let removed = persist_book(db).await; + record_history(db, user, kept).await; + record_history(db, user, removed).await; + + let before = additive_totals(db, user).await; + books::Entity::delete_by_id(removed).exec(db).await.unwrap(); + let after = additive_totals(db, user).await; + + assert_eq!( + after, before, + "deleting a book must not change any time, page or sitting total" + ); + + let window = whole_history(); + let series = ReadingStatsRepository::by_series(db, user, window, StatsSort::Time, 50) + .await + .unwrap(); + assert_eq!(series.len(), 2, "one real series plus the removed bucket"); + let bucket: Vec<_> = series.iter().filter(|s| s.series_id.is_none()).collect(); + assert_eq!(bucket.len(), 1, "orphans collapse into exactly one bucket"); + assert_eq!(bucket[0].duration.measured_ms, 900_000); + assert_eq!(bucket[0].pages_read, 24); + assert_eq!(bucket[0].sessions, 1); + assert!(bucket[0].series_name.is_none()); + let series_time: i64 = series.iter().map(|s| s.duration.total_ms()).sum(); + assert_eq!( + series_time, + before.summary.0 + before.summary.1, + "the series breakdown, bucket included, must add up to the summary" + ); + + let formats = ReadingStatsRepository::by_format(db, user, window, StatsSort::Time) + .await + .unwrap(); + let cbz: Vec<_> = formats + .iter() + .filter(|f| f.format.as_deref() == Some("cbz")) + .collect(); + let orphaned: Vec<_> = formats.iter().filter(|f| f.format.is_none()).collect(); + assert_eq!(cbz.len(), 1); + assert_eq!( + cbz[0].duration.measured_ms, 900_000, + "only the kept book remains cbz" + ); + assert_eq!(orphaned.len(), 1, "the deleted book's format is unknowable"); + assert_eq!(orphaned[0].duration.measured_ms, 900_000); + + let totals = ReadingStatsRepository::orphaned_totals(db, user) + .await + .unwrap(); + assert_eq!(totals.duration.measured_ms, 900_000); + assert_eq!(totals.pages_read, 24); + assert_eq!(totals.sessions, 1); + assert_eq!(totals.completions, 1); +} + +/// Purging removes only the caller's orphans: their attributed history stays, +/// and another reader's orphans of the same book are untouched. +async fn exercise_purge_is_scoped_to_orphans_of_one_user(db: &DatabaseConnection) { + let purger = persist_user(db).await; + let bystander = persist_user(db).await; + let kept = persist_book(db).await; + let removed = persist_book(db).await; + record_history(db, purger, kept).await; + record_history(db, purger, removed).await; + record_history(db, bystander, removed).await; + books::Entity::delete_by_id(removed).exec(db).await.unwrap(); + + let purged = ReadingStatsRepository::purge_orphaned(db, purger) + .await + .unwrap(); + assert_eq!((purged.sessions, purged.completions), (1, 1)); + + let sessions = sessions_for_user(db, purger).await; + assert_eq!(sessions.len(), 1); + assert_eq!(sessions[0].book_id, Some(kept), "attributed history stays"); + assert_eq!(completions_for_user(db, purger).await.len(), 1); + + assert_eq!(sessions_for_user(db, bystander).await.len(), 1); + assert_eq!(completions_for_user(db, bystander).await.len(), 1); +} + +#[tokio::test] +async fn history_survives_book_delete_sqlite() { + let (db, _temp_dir) = setup_test_db().await; + exercise_history_survives_book_delete(&db).await; +} + +#[tokio::test] +async fn statistics_keep_deleted_reading_sqlite() { + let (db, _temp_dir) = setup_test_db().await; + exercise_statistics_keep_deleted_reading(&db).await; + exercise_purge_is_scoped_to_orphans_of_one_user(&db).await; +} + +#[tokio::test] +async fn purge_is_scoped_to_orphans_of_one_user_sqlite() { + let (db, _temp_dir) = setup_test_db().await; + exercise_purge_is_scoped_to_orphans_of_one_user(&db).await; +} + +#[tokio::test] +async fn user_delete_still_removes_history_sqlite() { + let (db, _temp_dir) = setup_test_db().await; + exercise_user_delete_still_removes_history(&db).await; +} + +/// Both of the above against PostgreSQL, sequenced in one test on purpose: +/// `setup_test_db_postgres` truncates a database shared by the whole run, so two +/// PostgreSQL tests running at once delete each other's fixtures. +/// +/// This one matters more than most: the SQLite path rebuilds the table to change +/// the constraint while PostgreSQL alters it in place, so the two engines reach +/// the same schema by different routes and only this test says they agree. +#[tokio::test] +#[ignore] // Requires PostgreSQL test database +async fn reading_history_retention_postgres() { + let Some(db) = setup_test_db_postgres().await else { + eprintln!("PostgreSQL test database not available, skipping"); + return; + }; + + exercise_history_survives_book_delete(&db).await; + exercise_user_delete_still_removes_history(&db).await; + exercise_statistics_keep_deleted_reading(&db).await; +} + +// ============================================================================ +// The SQLite table rebuild +// ============================================================================ + +const HISTORY_TABLES: [&str; 2] = ["reading_sessions", "read_completions"]; + +/// A SQLite database migrated up to, but not including, the migration that +/// relaxes the cascade: the schema exactly as the original create migrations +/// left it. +async fn sqlite_before_retention_migration() -> (Database, TempDir) { + let temp_dir = TempDir::new().unwrap(); + let db_path = temp_dir.path().join("test.db"); + let config = DatabaseConfig { + db_type: DatabaseType::SQLite, + postgres: None, + sqlite: Some(SQLiteConfig { + path: db_path.to_str().unwrap().to_string(), + pragmas: None, + ..SQLiteConfig::default() + }), + ..DatabaseConfig::default() + }; + let db = Database::new(&config).await.unwrap(); + + let migrations = Migrator::migrations(); + let target = migrations + .iter() + .position(|m| m.name().contains("reading_history_survives_book_delete")) + .expect("the retention migration should be registered"); + Migrator::up(db.sea_orm_connection(), Some(target as u32)) + .await + .unwrap(); + (db, temp_dir) +} + +async fn apply_retention_migration(db: &DatabaseConnection) { + Migrator::up(db, Some(1)).await.unwrap(); +} + +async fn strings(db: &DatabaseConnection, sql: &str) -> Vec { + db.query_all(Statement::from_string( + DatabaseBackend::Sqlite, + sql.to_string(), + )) + .await + .unwrap() + .into_iter() + .map(|row| row.try_get_by_index::(0).unwrap()) + .collect() +} + +/// Every index on the table, as the SQL that created it. Comparing the SQL +/// catches a dropped index, a lost column and a lost `DESC` alike. +async fn index_definitions(db: &DatabaseConnection, table: &str) -> Vec { + strings( + db, + &format!( + "SELECT sql FROM sqlite_master WHERE type = 'index' AND tbl_name = '{table}' \ + AND sql IS NOT NULL ORDER BY name" + ), + ) + .await +} + +/// `name type notnull default pk` per column, in column order. +async fn column_shapes(db: &DatabaseConnection, table: &str) -> Vec { + strings( + db, + &format!( + "SELECT name || ' ' || type || ' ' || \"notnull\" || ' ' || \ + COALESCE(dflt_value, '-') || ' ' || pk FROM pragma_table_info('{table}') ORDER BY cid" + ), + ) + .await +} + +/// `column -> table on_delete` per foreign key. +async fn foreign_keys(db: &DatabaseConnection, table: &str) -> Vec { + strings( + db, + &format!( + "SELECT \"from\" || ' -> ' || \"table\" || ' ' || on_delete \ + FROM pragma_foreign_key_list('{table}') ORDER BY \"from\"" + ), + ) + .await +} + +/// The rebuild keeps every row and attribution, reproduces the original +/// indexes and columns exactly, and changes only `book_id`'s nullability and +/// delete action. +#[tokio::test] +async fn sqlite_rebuild_keeps_rows_and_indexes() { + let (db, _temp_dir) = sqlite_before_retention_migration().await; + let conn = db.sea_orm_connection(); + + let user = persist_user(conn).await; + let mut books = Vec::new(); + for _ in 0..3 { + let book = persist_book(conn).await; + record_history(conn, user, book).await; + books.push(book); + } + + let mut indexes_before = Vec::new(); + let mut columns_before = Vec::new(); + for table in HISTORY_TABLES { + indexes_before.push(index_definitions(conn, table).await); + columns_before.push(column_shapes(conn, table).await); + } + assert_eq!( + indexes_before[0].len(), + 3, + "reading_sessions starts with 3 indexes" + ); + assert_eq!( + indexes_before[1].len(), + 2, + "read_completions starts with 2 indexes" + ); + + apply_retention_migration(conn).await; + + for (i, table) in HISTORY_TABLES.into_iter().enumerate() { + let book_index = format!("idx_{table}_book_id"); + let (added, kept): (Vec, Vec) = index_definitions(conn, table) + .await + .into_iter() + .partition(|sql| { + sql.contains(&format!("\"{book_index}\"")) + || sql.contains(&format!(" {book_index} ")) + }); + assert_eq!( + kept, indexes_before[i], + "{table}: the rebuild must recreate every original index exactly" + ); + assert_eq!( + added.len(), + 1, + "{table}: the rebuild adds one index on book_id for the foreign key action" + ); + + let expected_columns: Vec = columns_before[i] + .iter() + .map(|shape| { + if shape.starts_with("book_id ") { + shape.replacen(" 1 ", " 0 ", 1) + } else { + shape.clone() + } + }) + .collect(); + assert_eq!( + column_shapes(conn, table).await, + expected_columns, + "{table}: only book_id's nullability may change" + ); + + assert_eq!( + foreign_keys(conn, table).await, + vec![ + "book_id -> books SET NULL".to_string(), + "user_id -> users CASCADE".to_string(), + ], + "{table}: book deletes null the column, user deletes still cascade" + ); + } + + let sessions = sessions_for_user(conn, user).await; + let completions = completions_for_user(conn, user).await; + assert_eq!(sessions.len(), 3, "every session survives the rebuild"); + assert_eq!( + completions.len(), + 3, + "every completion survives the rebuild" + ); + for book in &books { + assert!(sessions.iter().any(|s| s.book_id == Some(*book))); + assert!(completions.iter().any(|c| c.book_id == Some(*book))); + } + + // Enforcement is on for application connections, so this proves the new + // constraint is live and not merely declared. + books::Entity::delete_by_id(books[0]) + .exec(conn) + .await + .unwrap(); + let sessions = sessions_for_user(conn, user).await; + assert_eq!(sessions.len(), 3); + assert_eq!(sessions.iter().filter(|s| s.book_id.is_none()).count(), 1); + + db.close().await; +} + +/// Rolling back restores the original constraint, which orphaned rows cannot +/// satisfy, so it discards exactly those and keeps everything attributed. +#[tokio::test] +async fn sqlite_rollback_drops_only_orphans() { + let (db, _temp_dir) = setup_test_db_wrapper().await; + let conn = db.sea_orm_connection(); + + let user = persist_user(conn).await; + let kept = persist_book(conn).await; + let removed = persist_book(conn).await; + record_history(conn, user, kept).await; + record_history(conn, user, removed).await; + books::Entity::delete_by_id(removed) + .exec(conn) + .await + .unwrap(); + + Migrator::down(conn, Some(1)).await.unwrap(); + + for table in HISTORY_TABLES { + assert!( + column_shapes(conn, table) + .await + .iter() + .any(|shape| shape.starts_with("book_id ") && shape.contains(" 1 ")), + "{table}: book_id is required again" + ); + assert!( + foreign_keys(conn, table) + .await + .contains(&"book_id -> books CASCADE".to_string()), + "{table}: book deletes cascade again" + ); + } + + let sessions = sessions_for_user(conn, user).await; + let completions = completions_for_user(conn, user).await; + assert_eq!(sessions.len(), 1, "only the orphaned session is discarded"); + assert_eq!(sessions[0].book_id, Some(kept)); + assert_eq!( + completions.len(), + 1, + "only the orphaned completion is discarded" + ); + assert_eq!(completions[0].book_id, Some(kept)); + + // And forward again, so the migration is re-runnable. + Migrator::up(conn, None).await.unwrap(); + assert_eq!(sessions_for_user(conn, user).await.len(), 1); + + db.close().await; +} diff --git a/tests/db/reading_stats.rs b/tests/db/reading_stats.rs index 4ed96b29..37ad8336 100644 --- a/tests/db/reading_stats.rs +++ b/tests/db/reading_stats.rs @@ -92,7 +92,7 @@ async fn seed( reading_sessions::ActiveModel { id: Set(Uuid::new_v4()), user_id: Set(user_id), - book_id: Set(book_id), + book_id: Set(Some(book_id)), device_id: Set(device.to_string()), device_name: Set(Some(device.to_string())), pass: Set(1), @@ -272,9 +272,9 @@ async fn exercise_reading_stats(db: &DatabaseConnection) { .await .expect("series breakdown must decode on this engine"); assert_eq!(series.len(), 2); - assert_eq!(series[0].series_name, "Berserk"); + assert_eq!(series[0].series_name.as_deref(), Some("Berserk")); assert_eq!(series[0].duration.total_ms(), 50 * MINUTE_MS); - assert_eq!(series[1].series_name, "Dune"); + assert_eq!(series[1].series_name.as_deref(), Some("Dune")); assert_eq!( series[1].books_finished, 1, "the duplicated finish collapses in the series breakdown too" @@ -284,7 +284,7 @@ async fn exercise_reading_stats(db: &DatabaseConnection) { .await .expect("format breakdown must decode on this engine"); assert_eq!(formats.len(), 2); - assert_eq!(formats[0].format, "cbz"); + assert_eq!(formats[0].format.as_deref(), Some("cbz")); // Each ranking key is a different `ORDER BY` over aggregate expressions, // and PostgreSQL is the strict one about what may appear there. Ranking by diff --git a/web/openapi.json b/web/openapi.json index c616015f..157908ff 100644 --- a/web/openapi.json +++ b/web/openapi.json @@ -10434,6 +10434,76 @@ ] } }, + "/api/v1/reading-stats/orphaned": { + "get": { + "tags": [ + "Reading Statistics" + ], + "summary": "The caller's reading history for books that no longer exist, in total", + "description": "Unwindowed: exactly what `DELETE /api/v1/reading-stats/orphaned` would\nremove, so a client can say so before asking the reader to confirm.", + "operationId": "get_orphaned_reading_history", + "responses": { + "200": { + "description": "Totals of the detached history", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/OrphanedHistoryDto" + } + } + } + }, + "401": { + "description": "Unauthorized" + }, + "403": { + "description": "Forbidden" + } + }, + "security": [ + { + "jwt_bearer": [] + }, + { + "api_key": [] + } + ] + }, + "delete": { + "tags": [ + "Reading Statistics" + ], + "summary": "Delete the caller's reading history for books that no longer exist", + "description": "When a book is deleted from the server its reading sessions and finished\nread-throughs are kept, so the time still counts towards every statistic;\nthe series and format breakdowns show it as one \"removed from library\" row.\nThis discards those rows for the caller, and only for the caller.\n\nIrreversible. Only history already detached from any book is touched;\nattributed reading is never affected.", + "operationId": "purge_orphaned_reading_history", + "responses": { + "200": { + "description": "What was deleted", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PurgedOrphanedHistoryDto" + } + } + } + }, + "401": { + "description": "Unauthorized" + }, + "403": { + "description": "Forbidden" + } + }, + "security": [ + { + "jwt_bearer": [] + }, + { + "api_key": [] + } + ] + } + }, "/api/v1/readlists": { "get": { "tags": [ @@ -35590,6 +35660,36 @@ } } }, + "OrphanedHistoryDto": { + "type": "object", + "description": "The caller's reading history whose book has since been deleted, across\nevery date. What a purge would remove.", + "required": [ + "duration", + "pagesRead", + "sessions", + "completions" + ], + "properties": { + "completions": { + "type": "integer", + "format": "int64", + "description": "Finished read-throughs.", + "minimum": 0 + }, + "duration": { + "$ref": "#/components/schemas/DurationBreakdownDto" + }, + "pagesRead": { + "type": "integer", + "format": "int64" + }, + "sessions": { + "type": "integer", + "format": "int64", + "description": "Sittings, counted as the dashboard counts them." + } + } + }, "PageDto": { "type": "object", "description": "Page data transfer object", @@ -38682,6 +38782,28 @@ } } }, + "PurgedOrphanedHistoryDto": { + "type": "object", + "description": "What purging the removed-from-library history deleted.", + "required": [ + "sessionsRemoved", + "completionsRemoved" + ], + "properties": { + "completionsRemoved": { + "type": "integer", + "format": "int64", + "description": "Finished read-throughs deleted.", + "minimum": 0 + }, + "sessionsRemoved": { + "type": "integer", + "format": "int64", + "description": "Reading sessions deleted. Their time no longer counts anywhere.", + "minimum": 0 + } + } + }, "QueueHealthMetricsDto": { "type": "object", "description": "Queue health metrics", @@ -39002,7 +39124,7 @@ "type": "object", "description": "Totals for one file format.", "required": [ - "format", + "removedFromLibrary", "duration", "pagesRead", "sessions", @@ -39017,13 +39139,21 @@ "$ref": "#/components/schemas/DurationBreakdownDto" }, "format": { - "type": "string", + "type": [ + "string", + "null" + ], + "description": "Null on the removed-from-library row.", "example": "cbz" }, "pagesRead": { "type": "integer", "format": "int64" }, + "removedFromLibrary": { + "type": "boolean", + "description": "True on the single row that gathers reading whose book has since been\ndeleted from the server, and whose format is therefore unknown." + }, "sessions": { "type": "integer", "format": "int64" @@ -39034,8 +39164,7 @@ "type": "object", "description": "Totals for one series, most-read first.", "required": [ - "seriesId", - "seriesName", + "removedFromLibrary", "duration", "pagesRead", "sessions", @@ -39060,12 +39189,24 @@ "type": "integer", "format": "int64" }, + "removedFromLibrary": { + "type": "boolean", + "description": "True on the single row that gathers reading whose book has since been\ndeleted from the server. That time still counts towards every total,\nbut which series it belonged to is no longer known. Its `books` and\n`booksFinished` are always 0: both count distinct books, and deleted\nbooks cannot be told apart." + }, "seriesId": { - "type": "string", - "format": "uuid" + "type": [ + "string", + "null" + ], + "format": "uuid", + "description": "Null on the removed-from-library row." }, "seriesName": { - "type": "string", + "type": [ + "string", + "null" + ], + "description": "Null on the removed-from-library row.", "example": "Berserk" }, "sessions": { diff --git a/web/src/api/readingStats.ts b/web/src/api/readingStats.ts index 228fa8df..5e8420a6 100644 --- a/web/src/api/readingStats.ts +++ b/web/src/api/readingStats.ts @@ -14,6 +14,9 @@ export type ReadingStatsGranularity = components["schemas"]["ReadingStatsGranularity"]; export type ReadingStatsSort = components["schemas"]["ReadingStatsSort"]; export type ReadingCoverage = components["schemas"]["ReadingCoverageDto"]; +export type OrphanedHistory = components["schemas"]["OrphanedHistoryDto"]; +export type PurgedOrphanedHistory = + components["schemas"]["PurgedOrphanedHistoryDto"]; export interface ReadingStatsParams { from?: Date; @@ -69,4 +72,22 @@ export const readingStatsApi = { const response = await api.get("/reading-stats/coverage"); return response.data; }, + + /** + * Totals of the reader's history whose book has since been deleted, across + * every date. Exactly what `purgeOrphaned` would remove, which the windowed + * "removed from library" row cannot say. + */ + orphaned: async (): Promise => { + const response = await api.get("/reading-stats/orphaned"); + return response.data; + }, + + /** Permanently delete the reader's history whose book has been deleted. */ + purgeOrphaned: async (): Promise => { + const response = await api.delete( + "/reading-stats/orphaned", + ); + return response.data; + }, }; diff --git a/web/src/components/reading/ReadingStatsPanels.test.tsx b/web/src/components/reading/ReadingStatsPanels.test.tsx index c980daa7..c71b8586 100644 --- a/web/src/components/reading/ReadingStatsPanels.test.tsx +++ b/web/src/components/reading/ReadingStatsPanels.test.tsx @@ -124,6 +124,7 @@ describe("TopSeries", () => { { seriesId: "11111111-1111-1111-1111-111111111111", seriesName: "Berserk", + removedFromLibrary: false, duration: duration(2 * HOUR), pagesRead: 120, sessions: 4, @@ -133,6 +134,7 @@ describe("TopSeries", () => { { seriesId: "22222222-2222-2222-2222-222222222222", seriesName: "Vinland Saga", + removedFromLibrary: false, duration: duration(30 * MINUTE), pagesRead: 40, sessions: 1, @@ -156,6 +158,36 @@ describe("TopSeries", () => { expect(screen.getByText("40 pages across 1 book")).toBeInTheDocument(); }); + /// Reading of deleted books still counts, but it is not a series: it must + /// not link anywhere or claim a book count it cannot know. + it("shows reading of deleted books as a labelled row, not a series", () => { + renderWithProviders( + , + ); + + const label = screen.getByText("Removed from library"); + expect(label.closest("a")).toBeNull(); + expect(screen.getByText("45m")).toBeInTheDocument(); + expect( + screen.getByText("3 sittings of books no longer on the server"), + ).toBeInTheDocument(); + expect(screen.getAllByRole("link")).toHaveLength(2); + }); + it("says so plainly when nothing was read", () => { renderWithProviders(); @@ -224,6 +256,7 @@ describe("empty rows", () => { { seriesId: "11111111-1111-1111-1111-111111111111", seriesName: "Berserk", + removedFromLibrary: false, duration: duration(2 * HOUR), pagesRead: 120, sessions: 4, @@ -232,6 +265,7 @@ describe("empty rows", () => { { seriesId: "22222222-2222-2222-2222-222222222222", seriesName: "Imported Series", + removedFromLibrary: false, duration: duration(0), pagesRead: 0, sessions: 6, @@ -252,6 +286,7 @@ describe("empty rows", () => { { seriesId: "22222222-2222-2222-2222-222222222222", seriesName: "Imported Series", + removedFromLibrary: false, duration: duration(0), pagesRead: 0, sessions: 6, @@ -300,6 +335,7 @@ describe("empty rows", () => { formats={[ { format: "cbz", + removedFromLibrary: false, duration: duration(90 * MINUTE), pagesRead: 60, sessions: 3, @@ -307,6 +343,7 @@ describe("empty rows", () => { }, { format: "pdf", + removedFromLibrary: false, duration: duration(0), pagesRead: 0, sessions: 0, @@ -321,12 +358,33 @@ describe("empty rows", () => { expect(screen.getByText("1h 30m")).toBeInTheDocument(); }); + it("names the deleted books' format row instead of printing nothing", () => { + renderWithProviders( + , + ); + + expect(screen.getByText("Removed from library")).toBeInTheDocument(); + expect(screen.getByText("20m")).toBeInTheDocument(); + }); + it("renders no format panel at all when every format is empty", () => { renderWithProviders( { { seriesId: "11111111-1111-1111-1111-111111111111", seriesName: "Berserk", + removedFromLibrary: false, duration: duration(2 * HOUR), pagesRead: 420, sessions: 4, diff --git a/web/src/components/reading/ReadingStatsPanels.tsx b/web/src/components/reading/ReadingStatsPanels.tsx index 3e0b31c4..25f7204b 100644 --- a/web/src/components/reading/ReadingStatsPanels.tsx +++ b/web/src/components/reading/ReadingStatsPanels.tsx @@ -402,11 +402,15 @@ function deviceLabel(device: ReadingByDeviceDto): string { return device.deviceName ?? device.deviceId; } +/** How the series and format panels name reading of since-deleted books. */ +const REMOVED_LABEL = "Removed from library"; + /** A labelled row with a proportional bar. Used for series, devices, formats. */ function RankedRow({ label, href, sublabel, + muted = false, measuredMs, inferredMs, value, @@ -416,6 +420,8 @@ function RankedRow({ label: string; href?: string; sublabel?: string; + /** Set for a row that is not a real item, so it cannot pass for one. */ + muted?: boolean; measuredMs: number; inferredMs: number; value: number; @@ -444,7 +450,13 @@ function RankedRow({ {label} ) : ( - + {label} )} @@ -495,19 +507,38 @@ export function TopSeries({ return ( - {read.map((s) => ( - - ))} + {read.map((s) => + s.removedFromLibrary ? ( + // Reading of books since deleted from the server. It still counts, + // but there is no series to link to or name, and its book counts + // are always zero because deleted books cannot be told apart. The + // way to discard it is the page-level notice, which is not bound to + // this panel's window or ranking. + + ) : ( + + ), + )} ); } @@ -563,10 +594,20 @@ export function FormatBreakdown({ return ( {read.map((f) => ( - - - {f.format} - + + {f.removedFromLibrary ? ( + + {REMOVED_LABEL} + + ) : ( + + {f.format} + + )} {formatMetric(rowValue(f, metric), metric)} diff --git a/web/src/components/reading/RemovedHistoryNotice.test.tsx b/web/src/components/reading/RemovedHistoryNotice.test.tsx new file mode 100644 index 00000000..856bfefd --- /dev/null +++ b/web/src/components/reading/RemovedHistoryNotice.test.tsx @@ -0,0 +1,133 @@ +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { type OrphanedHistory, readingStatsApi } from "@/api/readingStats"; +import { renderWithProviders, screen, waitFor } from "@/test/utils"; +import { RemovedHistoryNotice } from "./RemovedHistoryNotice"; + +vi.mock("@/api/readingStats", () => ({ + readingStatsApi: { + orphaned: vi.fn(), + purgeOrphaned: vi.fn(), + }, +})); + +const HOUR = 60 * 60_000; + +function totals( + sessions: number, + totalMs: number, + completions: number, +): OrphanedHistory { + return { + duration: { measuredMs: totalMs, inferredMs: 0, totalMs }, + pagesRead: 0, + sessions, + completions, + }; +} + +async function openDialog() { + const user = userEvent.setup(); + await user.click( + await screen.findByRole("button", { + name: "Delete reading of removed books", + }), + ); + return user; +} + +describe("RemovedHistoryNotice", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("renders nothing when no removed history exists", async () => { + vi.mocked(readingStatsApi.orphaned).mockResolvedValue(totals(0, 0, 0)); + renderWithProviders(); + + await waitFor(() => expect(readingStatsApi.orphaned).toHaveBeenCalled()); + expect( + screen.queryByText("Reading of removed books"), + ).not.toBeInTheDocument(); + }); + + /// Not bound to the dashboard window or the series ranking: finished reads + /// with no sittings at all would never produce a series row. + it("offers the purge for completions that have no sittings", async () => { + vi.mocked(readingStatsApi.orphaned).mockResolvedValue(totals(0, 0, 3)); + renderWithProviders(); + + expect( + await screen.findByText(/3 finished reads from books since deleted/), + ).toBeInTheDocument(); + }); + + it("states what will be deleted across all dates before asking", async () => { + vi.mocked(readingStatsApi.orphaned).mockResolvedValue( + totals(5, 2 * HOUR, 2), + ); + renderWithProviders(); + await openDialog(); + + expect( + await screen.findByText( + /permanently deletes 5 sittings \(2h of reading\) and 2 finished reads, across all dates/, + ), + ).toBeInTheDocument(); + expect(readingStatsApi.purgeOrphaned).not.toHaveBeenCalled(); + }); + + it("re-reads the totals on open and deletes only after confirming", async () => { + vi.mocked(readingStatsApi.orphaned).mockResolvedValue(totals(1, HOUR, 0)); + vi.mocked(readingStatsApi.purgeOrphaned).mockResolvedValue({ + sessionsRemoved: 1, + completionsRemoved: 0, + }); + renderWithProviders(); + const user = await openDialog(); + + await waitFor(() => + expect(readingStatsApi.orphaned).toHaveBeenCalledTimes(2), + ); + const confirm = await screen.findByRole("button", { + name: "Delete permanently", + }); + await waitFor(() => expect(confirm).toBeEnabled()); + await user.click(confirm); + + await waitFor(() => + expect(readingStatsApi.purgeOrphaned).toHaveBeenCalledTimes(1), + ); + }); + + it("will not confirm against totals it failed to refresh", async () => { + vi.mocked(readingStatsApi.orphaned) + .mockResolvedValueOnce(totals(4, HOUR, 0)) + .mockRejectedValue(new Error("offline")); + renderWithProviders(); + await openDialog(); + + expect( + await screen.findByText(/Could not total the removed history/), + ).toBeInTheDocument(); + expect(screen.queryByText(/permanently deletes/)).not.toBeInTheDocument(); + expect( + screen.getByRole("button", { name: "Delete permanently" }), + ).toBeDisabled(); + }); + + it("says so when the history is already gone by the time it opens", async () => { + vi.mocked(readingStatsApi.orphaned) + .mockResolvedValueOnce(totals(2, HOUR, 0)) + .mockResolvedValue(totals(0, 0, 0)); + renderWithProviders(); + await openDialog(); + + expect( + await screen.findByText("There is no removed history left to delete."), + ).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: "Delete permanently" }), + ).toBeDisabled(); + }); +}); diff --git a/web/src/components/reading/RemovedHistoryNotice.tsx b/web/src/components/reading/RemovedHistoryNotice.tsx new file mode 100644 index 00000000..b058e91b --- /dev/null +++ b/web/src/components/reading/RemovedHistoryNotice.tsx @@ -0,0 +1,180 @@ +/** + * Reading of books since deleted from the server, and the way to discard it. + * + * That reading keeps counting towards every statistic. This notice says so and + * lets the reader decide it should not, stating exactly what that costs first. + * + * It is driven by its own unwindowed request rather than by the "Removed from + * library" row in the series panel. That row only exists when the reading falls + * inside the dates on screen, ranks in the top few, and is non-zero for the + * chosen metric, so a reader could hold removed history with no row, and no + * control, anywhere on the page. A purge is not windowed either, so the row's + * figures would understate what is lost. + */ + +import { + Alert, + Button, + Group, + Loader, + Modal, + Stack, + Text, +} from "@mantine/core"; +import { notifications } from "@mantine/notifications"; +import { IconAlertTriangle, IconInfoCircle } from "@tabler/icons-react"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { useState } from "react"; +import { type OrphanedHistory, readingStatsApi } from "@/api/readingStats"; +import { formatDuration } from "./readingStatsFormat"; + +const ORPHANED_KEY = ["readingStats", "orphaned"] as const; + +function plural(count: number, one: string, many: string): string { + return `${count} ${count === 1 ? one : many}`; +} + +function isEmpty(totals: OrphanedHistory): boolean { + return totals.sessions === 0 && totals.completions === 0; +} + +/** What the totals amount to, in one clause. */ +function describe(totals: OrphanedHistory): string { + const parts = []; + if (totals.sessions > 0) { + parts.push( + `${plural(totals.sessions, "sitting", "sittings")} (${formatDuration(totals.duration.totalMs)} of reading)`, + ); + } + if (totals.completions > 0) { + parts.push(plural(totals.completions, "finished read", "finished reads")); + } + return parts.join(" and "); +} + +export function RemovedHistoryNotice() { + const [opened, setOpened] = useState(false); + const queryClient = useQueryClient(); + + const totals = useQuery({ + queryKey: ORPHANED_KEY, + queryFn: () => readingStatsApi.orphaned(), + staleTime: 60_000, + }); + + const purge = useMutation({ + mutationFn: () => readingStatsApi.purgeOrphaned(), + onSuccess: () => { + setOpened(false); + // Every total on the page included this history, and coverage may have + // moved too, so everything under the prefix is stale. + queryClient.invalidateQueries({ queryKey: ["readingStats"] }); + notifications.show({ + title: "Removed history deleted", + message: "Reading of books no longer on the server no longer counts.", + color: "green", + }); + }, + onError: () => { + notifications.show({ + title: "Could not delete removed history", + message: "Nothing was deleted. Try again in a moment.", + color: "red", + }); + }, + }); + + const data = totals.data; + // Stays mounted while the dialog is open, so a refetch that finds nothing + // left can say so instead of the dialog vanishing under the reader. + if (!data || (isEmpty(data) && !opened)) return null; + + const open = () => { + setOpened(true); + // The dialog's whole job is to be accurate, so it never confirms against + // figures fetched before it opened. + totals.refetch(); + }; + + // Settled means the figures on screen are the ones just fetched. + const settled = !totals.isFetching && !totals.isError; + const nothingLeft = settled && isEmpty(data); + + return ( + <> + {!isEmpty(data) && ( + } + title="Reading of removed books" + > + + + {describe(data)} from books since deleted from the server still + count in your statistics, shown as "Removed from library". + + + + + )} + setOpened(false)} + title="Delete reading of removed books?" + centered + > + + + These books have been deleted from the server. Their reading still + counts in your statistics, but which book or series it came from is + no longer known. + + {totals.isFetching && ( + + + + )} + {totals.isError && !totals.isFetching && ( + }> + Could not total the removed history, so nothing can be deleted + right now. + + )} + {settled && !nothingLeft && ( + }> + This permanently deletes {describe(data)}, across all dates, not + only the ones shown. It cannot be undone. + + )} + {nothingLeft && ( + + There is no removed history left to delete. + + )} + + + + + + + + ); +} diff --git a/web/src/mocks/handlers/readingStats.ts b/web/src/mocks/handlers/readingStats.ts index ab24ec36..9fc9e757 100644 --- a/web/src/mocks/handlers/readingStats.ts +++ b/web/src/mocks/handlers/readingStats.ts @@ -209,6 +209,22 @@ export const readingStatsHandlers = [ }); }), + // Nothing in the mock library has been deleted, so there is nothing to purge. + http.get("*/api/v1/reading-stats/orphaned", async () => { + await delay(80); + return HttpResponse.json({ + duration: { measuredMs: 0, inferredMs: 0, totalMs: 0 }, + pagesRead: 0, + sessions: 0, + completions: 0, + }); + }), + + http.delete("*/api/v1/reading-stats/orphaned", async () => { + await delay(80); + return HttpResponse.json({ sessionsRemoved: 0, completionsRemoved: 0 }); + }), + http.get("*/api/v1/reading-stats", async ({ request }) => { await delay(150); @@ -245,6 +261,7 @@ export const readingStatsHandlers = [ return { seriesId: source.id, seriesName: source.title, + removedFromLibrary: false, duration: durationOf(days), pagesRead: days.reduce((sum, d) => sum + d.pagesRead, 0), sessions: days.reduce((sum, d) => sum + d.sessions, 0), @@ -274,6 +291,7 @@ export const readingStatsHandlers = [ ] .map(([format, days]) => ({ format, + removedFromLibrary: false, duration: durationOf(days), pagesRead: days.reduce((sum, d) => sum + d.pagesRead, 0), sessions: days.reduce((sum, d) => sum + d.sessions, 0), diff --git a/web/src/pages/ReadingStats.tsx b/web/src/pages/ReadingStats.tsx index c81bdd37..e0556a74 100644 --- a/web/src/pages/ReadingStats.tsx +++ b/web/src/pages/ReadingStats.tsx @@ -34,6 +34,7 @@ import { StatTile, TopSeries, } from "@/components/reading/ReadingStatsPanels"; +import { RemovedHistoryNotice } from "@/components/reading/RemovedHistoryNotice"; import { buildCalendar, groupIntoYears, @@ -356,6 +357,8 @@ export function ReadingStats() { + + diff --git a/web/src/types/api.generated.ts b/web/src/types/api.generated.ts index 620a328c..00df781f 100644 --- a/web/src/types/api.generated.ts +++ b/web/src/types/api.generated.ts @@ -3360,6 +3360,37 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v1/reading-stats/orphaned": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** + * The caller's reading history for books that no longer exist, in total + * @description Unwindowed: exactly what `DELETE /api/v1/reading-stats/orphaned` would + * remove, so a client can say so before asking the reader to confirm. + */ + get: operations["get_orphaned_reading_history"]; + put?: never; + post?: never; + /** + * Delete the caller's reading history for books that no longer exist + * @description When a book is deleted from the server its reading sessions and finished + * read-throughs are kept, so the time still counts towards every statistic; + * the series and format breakdowns show it as one "removed from library" row. + * This discards those rows for the caller, and only for the caller. + * + * Irreversible. Only history already detached from any book is touched; + * attributed reading is never affected. + */ + delete: operations["purge_orphaned_reading_history"]; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/readlists": { parameters: { query?: never; @@ -14877,6 +14908,25 @@ export interface components { */ sizeBytes: number; }; + /** + * @description The caller's reading history whose book has since been deleted, across + * every date. What a purge would remove. + */ + OrphanedHistoryDto: { + /** + * Format: int64 + * @description Finished read-throughs. + */ + completions: number; + duration: components["schemas"]["DurationBreakdownDto"]; + /** Format: int64 */ + pagesRead: number; + /** + * Format: int64 + * @description Sittings, counted as the dashboard counts them. + */ + sessions: number; + }; /** @description Page data transfer object */ PageDto: { /** @@ -16691,6 +16741,19 @@ export interface components { */ deleted: number; }; + /** @description What purging the removed-from-library history deleted. */ + PurgedOrphanedHistoryDto: { + /** + * Format: int64 + * @description Finished read-throughs deleted. + */ + completionsRemoved: number; + /** + * Format: int64 + * @description Reading sessions deleted. Their time no longer counts anywhere. + */ + sessionsRemoved: number; + }; /** @description Queue health metrics */ QueueHealthMetricsDto: { /** @@ -16895,10 +16958,18 @@ export interface components { /** Format: int64 */ booksFinished: number; duration: components["schemas"]["DurationBreakdownDto"]; - /** @example cbz */ - format: string; + /** + * @description Null on the removed-from-library row. + * @example cbz + */ + format?: string | null; /** Format: int64 */ pagesRead: number; + /** + * @description True on the single row that gathers reading whose book has since been + * deleted from the server, and whose format is therefore unknown. + */ + removedFromLibrary: boolean; /** Format: int64 */ sessions: number; }; @@ -16918,10 +16989,24 @@ export interface components { duration: components["schemas"]["DurationBreakdownDto"]; /** Format: int64 */ pagesRead: number; - /** Format: uuid */ - seriesId: string; - /** @example Berserk */ - seriesName: string; + /** + * @description True on the single row that gathers reading whose book has since been + * deleted from the server. That time still counts towards every total, + * but which series it belonged to is no longer known. Its `books` and + * `booksFinished` are always 0: both count distinct books, and deleted + * books cannot be told apart. + */ + removedFromLibrary: boolean; + /** + * Format: uuid + * @description Null on the removed-from-library row. + */ + seriesId?: string | null; + /** + * @description Null on the removed-from-library row. + * @example Berserk + */ + seriesName?: string | null; /** Format: int64 */ sessions: number; }; @@ -29443,6 +29528,74 @@ export interface operations { }; }; }; + get_orphaned_reading_history: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Totals of the detached history */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["OrphanedHistoryDto"]; + }; + }; + /** @description Unauthorized */ + 401: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + /** @description Forbidden */ + 403: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; + purge_orphaned_reading_history: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description What was deleted */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["PurgedOrphanedHistoryDto"]; + }; + }; + /** @description Unauthorized */ + 401: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + /** @description Forbidden */ + 403: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; list_readlists: { parameters: { query?: never;