From 7ca9e950dc0b754a5abe5919fbc8da1b00027a36 Mon Sep 17 00:00:00 2001 From: "Claude (Modal)" Date: Wed, 23 Sep 2026 05:59:04 +0000 Subject: [PATCH] fix(libcalibre): bump books.last_modified when custom values change set_custom_value and set_book_read_state wrote directly to the custom column tables without touching books.last_modified, unlike update_book, identifiers, and asset edits. Calibre relies on last_modified for OPF backups and change detection, so edits through these two paths were invisible to it. Both now compare the stored value before and after the write and only bump last_modified when it actually changed, matching Calibre's _update_last_modified(dirtied). Fixes #157. Co-Authored-By: Claude Opus 5.5 Co-Authored-By: Claude Sonnet 5 --- crates/libcalibre/src/library.rs | 29 ++++-- crates/libcalibre/tests/last_modified_test.rs | 91 +++++++++++++++++++ 2 files changed, 113 insertions(+), 7 deletions(-) create mode 100644 crates/libcalibre/tests/last_modified_test.rs diff --git a/crates/libcalibre/src/library.rs b/crates/libcalibre/src/library.rs index 87a680d7..ddd29d2c 100644 --- a/crates/libcalibre/src/library.rs +++ b/crates/libcalibre/src/library.rs @@ -776,7 +776,8 @@ impl Library { value: Option, ) -> Result<(), CalibreError> { let column = custom_columns::get_column(&mut self.conn, column_id)?; - custom_columns::set_value(&mut self.conn, &column, book_id, value) + self.conn + .transaction(|conn| set_custom_value_and_touch(conn, &column, book_id, value)) } /// One column's values for many books at once. Books with no stored @@ -826,12 +827,9 @@ impl Library { is_read: bool, ) -> Result<(), CalibreError> { let column = self.get_or_create_read_state_column()?; - custom_columns::set_value( - &mut self.conn, - &column, - book_id, - Some(CustomValue::Bool(is_read)), - ) + self.conn.transaction(|conn| { + set_custom_value_and_touch(conn, &column, book_id, Some(CustomValue::Bool(is_read))) + }) } pub fn batch_get_read_states( @@ -1005,6 +1003,23 @@ impl Library { } } +/// Compares the stored value before and after the write, so no-op edits and +/// values `set_value` normalises to what's already stored leave +/// `last_modified` alone, as Calibre does. +fn set_custom_value_and_touch( + conn: &mut SqliteConnection, + column: &CustomColumn, + book_id: BookId, + value: Option, +) -> Result<(), CalibreError> { + let before = custom_columns::get_value(conn, column, book_id)?; + custom_columns::set_value(conn, column, book_id, value)?; + if custom_columns::get_value(conn, column, book_id)? != before { + book_queries::touch(conn, book_id)?; + } + Ok(()) +} + // ============================================================================= // Metadata OPF generation (ported from calibre_client.rs) // ============================================================================= diff --git a/crates/libcalibre/tests/last_modified_test.rs b/crates/libcalibre/tests/last_modified_test.rs new file mode 100644 index 00000000..690403a9 --- /dev/null +++ b/crates/libcalibre/tests/last_modified_test.rs @@ -0,0 +1,91 @@ +// Regression tests for https://github.com/everydaythingssoftware/citadel/issues/157: +// metadata edits must bump `books.last_modified` only when a value actually changes, +// matching Calibre's `Cache.set_field()` -> `_update_last_modified(dirtied)`. +mod common; + +use common::{setup_with_library, standard_test_book}; +use libcalibre::{BookId, CustomColumnKind, CustomColumnSpec, CustomValue, Library}; + +fn add_book(lib: &mut Library) -> BookId { + lib.add_book(standard_test_book()).unwrap().id +} + +fn last_modified(lib: &mut Library, book: BookId) -> chrono::NaiveDateTime { + lib.get_book(book).unwrap().updated_at +} + +fn add_bool_column(lib: &mut Library) -> i32 { + lib.create_custom_column(CustomColumnSpec { + label: "finished".to_string(), + name: "Finished".to_string(), + kind: CustomColumnKind::Bool, + is_multiple: false, + enum_values: vec![], + display: None, + }) + .unwrap() + .id +} + +#[test] +fn test_set_custom_value_touches_last_modified() { + let (_temp, mut lib) = setup_with_library(); + let book = add_book(&mut lib); + let col = add_bool_column(&mut lib); + + let before = last_modified(&mut lib, book); + + lib.set_custom_value(book, col, Some(CustomValue::Bool(true))) + .unwrap(); + + let after = last_modified(&mut lib, book); + assert!( + after > before, + "last_modified must advance after a custom-value edit (before={before:?}, after={after:?})" + ); +} + +#[test] +fn test_set_book_read_state_touches_last_modified() { + let (_temp, mut lib) = setup_with_library(); + let book = add_book(&mut lib); + + let before = last_modified(&mut lib, book); + + lib.set_book_read_state(book, true).unwrap(); + + let after = last_modified(&mut lib, book); + assert!( + after > before, + "last_modified must advance after a read-state edit (before={before:?}, after={after:?})" + ); +} + +#[test] +fn test_unchanged_custom_value_leaves_last_modified() { + let (_temp, mut lib) = setup_with_library(); + let book = add_book(&mut lib); + let col = add_bool_column(&mut lib); + lib.set_custom_value(book, col, Some(CustomValue::Bool(true))) + .unwrap(); + + let before = last_modified(&mut lib, book); + + lib.set_custom_value(book, col, Some(CustomValue::Bool(true))) + .unwrap(); + + assert_eq!(last_modified(&mut lib, book), before); +} + +#[test] +fn test_unchanged_read_state_leaves_last_modified() { + let (_temp, mut lib) = setup_with_library(); + let book = add_book(&mut lib); + lib.set_book_read_state(book, true).unwrap(); + + let before = last_modified(&mut lib, book); + + lib.set_book_read_state(book, true).unwrap(); + + assert_eq!(last_modified(&mut lib, book), before); +}