move collection mtime bump into backend

Fixes the following issue:
- some code directly modifies the database, causing modified_in_python
to be set to true
- an undoable operation is run, which calls autosave() at the end
- autosave() notices there's an undoable operation, and commits immediately
- because modified_in_python was true, col.mtime was bumped in Python
- that invalidated the undo queue, preventing the operation from being
undone
This commit is contained in:
Damien Elmes 2021-03-13 16:27:08 +10:00
parent e364b36dc4
commit 8fc43956c2
5 changed files with 22 additions and 16 deletions

View file

@ -195,23 +195,17 @@ class Collection:
flush = setMod flush = setMod
def modified_after_begin(self) -> bool: def modified_by_backend(self) -> bool:
# Until we can move away from long-running transactions, the Python # Until we can move away from long-running transactions, the Python
# code needs to know if transaction should be committed, so we need # code needs to know if the transaction should be committed, so we need
# to check if the backend updated the modification time. # to check if the backend updated the modification time.
return self.db.last_begin_at != self.mod return self.db.last_begin_at != self.mod
def save(self, name: Optional[str] = None, trx: bool = True) -> None: def save(self, name: Optional[str] = None, trx: bool = True) -> None:
"Flush, commit DB, and take out another write lock if trx=True." "Flush, commit DB, and take out another write lock if trx=True."
# commit needed? # commit needed?
if self.db.modified_in_python or self.modified_after_begin(): if self.db.modified_in_python or self.modified_by_backend():
if self.db.modified_in_python:
self.db.execute("update col set mod = ?", intTime(1000))
self.db.modified_in_python = False self.db.modified_in_python = False
else:
# modifications made by the backend will have already bumped
# mtime
pass
self.db.commit() self.db.commit()
if trx: if trx:
self.db.begin() self.db.begin()

View file

@ -75,7 +75,7 @@ pub(super) fn db_command_bytes(col: &mut Collection, input: &[u8]) -> Result<Vec
args, args,
first_row_only, first_row_only,
} => { } => {
maybe_clear_undo(col, &sql); update_state_after_modification(col, &sql);
if first_row_only { if first_row_only {
db_query_row(&col.storage, &sql, &args)? db_query_row(&col.storage, &sql, &args)?
} else { } else {
@ -87,6 +87,10 @@ pub(super) fn db_command_bytes(col: &mut Collection, input: &[u8]) -> Result<Vec
DBResult::None DBResult::None
} }
DBRequest::Commit => { DBRequest::Commit => {
if col.state.modified_by_dbproxy {
col.storage.set_modified()?;
col.state.modified_by_dbproxy = false;
}
col.storage.commit_trx()?; col.storage.commit_trx()?;
DBResult::None DBResult::None
} }
@ -96,17 +100,17 @@ pub(super) fn db_command_bytes(col: &mut Collection, input: &[u8]) -> Result<Vec
DBResult::None DBResult::None
} }
DBRequest::ExecuteMany { sql, args } => { DBRequest::ExecuteMany { sql, args } => {
maybe_clear_undo(col, &sql); update_state_after_modification(col, &sql);
db_execute_many(&col.storage, &sql, &args)? db_execute_many(&col.storage, &sql, &args)?
} }
}; };
Ok(serde_json::to_vec(&resp)?) Ok(serde_json::to_vec(&resp)?)
} }
fn maybe_clear_undo(col: &mut Collection, sql: &str) { fn update_state_after_modification(col: &mut Collection, sql: &str) {
if !is_dql(sql) { if !is_dql(sql) {
println!("clearing undo+study due to {}", sql); println!("clearing undo+study due to {}", sql);
col.discard_undo_and_study_queues(); col.update_state_after_dbproxy_modification();
} }
} }

View file

@ -65,6 +65,9 @@ pub struct CollectionState {
pub(crate) notetype_cache: HashMap<NoteTypeID, Arc<NoteType>>, pub(crate) notetype_cache: HashMap<NoteTypeID, Arc<NoteType>>,
pub(crate) deck_cache: HashMap<DeckID, Arc<Deck>>, pub(crate) deck_cache: HashMap<DeckID, Arc<Deck>>,
pub(crate) card_queues: Option<CardQueues>, pub(crate) card_queues: Option<CardQueues>,
/// True if legacy Python code has executed SQL that has modified the
/// database, requiring modification time to be bumped.
pub(crate) modified_by_dbproxy: bool,
} }
pub struct Collection { pub struct Collection {
@ -92,7 +95,7 @@ impl Collection {
let mut res = func(self); let mut res = func(self);
if res.is_ok() { if res.is_ok() {
if let Err(e) = self.storage.mark_modified() { if let Err(e) = self.storage.set_modified() {
res = Err(e); res = Err(e);
} else if let Err(e) = self.storage.commit_rust_trx() { } else if let Err(e) = self.storage.commit_rust_trx() {
res = Err(e); res = Err(e);

View file

@ -257,7 +257,7 @@ impl SqliteStorage {
////////////////////////////////////////// //////////////////////////////////////////
pub(crate) fn mark_modified(&self) -> Result<()> { pub(crate) fn set_modified(&self) -> Result<()> {
self.set_modified_time(TimestampMillis::now()) self.set_modified_time(TimestampMillis::now())
} }

View file

@ -174,6 +174,11 @@ impl Collection {
self.clear_study_queues(); self.clear_study_queues();
} }
pub(crate) fn update_state_after_dbproxy_modification(&mut self) {
self.discard_undo_and_study_queues();
self.state.modified_by_dbproxy = true;
}
#[inline] #[inline]
pub(crate) fn save_undo(&mut self, item: impl Into<UndoableChange>) { pub(crate) fn save_undo(&mut self, item: impl Into<UndoableChange>) {
self.state.undo.save(item.into()); self.state.undo.save(item.into());