From 651ba8839345c0aac3cbf25d1e423bad08821648 Mon Sep 17 00:00:00 2001 From: RumovZ Date: Mon, 12 Jun 2023 03:14:14 +0200 Subject: [PATCH] Automate schema 11 other duplicates clearing (#2542) * Skip linting target folder Contains build files not passing the copyright header check. * Implicitly clear duplicate keys when serializing Fixes `originalStockKind` not being cleared from `other`, as it had mistakenly been added to the field list for `NoteFieldSchema11`. --- rslib/src/deckconfig/schema11.rs | 23 ----------------------- rslib/src/decks/schema11.rs | 20 +------------------- rslib/src/notetype/schema11.rs | 18 +----------------- rslib/src/serde.rs | 19 ++++++++++++++++++- rslib/src/storage/deck/mod.rs | 6 +++--- rslib/src/storage/deckconfig/mod.rs | 2 +- rslib/src/storage/notetype/mod.rs | 6 +++--- tools/minilints/src/main.rs | 1 + 8 files changed, 28 insertions(+), 67 deletions(-) diff --git a/rslib/src/deckconfig/schema11.rs b/rslib/src/deckconfig/schema11.rs index 198970c61..1d713d1f9 100644 --- a/rslib/src/deckconfig/schema11.rs +++ b/rslib/src/deckconfig/schema11.rs @@ -48,7 +48,6 @@ pub struct DeckConfSchema11 { // 2021 scheduler options: these were not in schema 11, but we need to persist them // so the settings are not lost on upgrade/downgrade. - // NOTE: if adding new ones, make sure to update clear_other_duplicates() #[serde(default)] new_mix: i32, #[serde(default)] @@ -335,7 +334,6 @@ impl From for DeckConfSchema11 { top_other = Default::default(); } else { top_other = serde_json::from_slice(&c.inner.other).unwrap_or_default(); - clear_other_duplicates(&mut top_other); if let Some(new) = top_other.remove("new") { let val: HashMap = serde_json::from_value(new).unwrap_or_default(); new_other = val; @@ -409,27 +407,6 @@ impl From for DeckConfSchema11 { } } -fn clear_other_duplicates(top_other: &mut HashMap) { - // Older clients may have received keys from a newer client when - // syncing, which get bundled into `other`. If they then upgrade, then - // downgrade their collection to schema11, serde will serialize the - // new default keys, but then add them again from `other`, leading - // to the keys being duplicated in the resulting json - which older - // clients then can't read. So we need to strip out any new keys we - // add. - for key in &[ - "newMix", - "newPerDayMinimum", - "interdayLearningMix", - "reviewOrder", - "newSortOrder", - "newGatherPriority", - "buryInterdayLearning", - ] { - top_other.remove(*key); - } -} - #[cfg(test)] mod test { use serde::de::IntoDeserializer; diff --git a/rslib/src/decks/schema11.rs b/rslib/src/decks/schema11.rs index 01a958801..e1b87276a 100644 --- a/rslib/src/decks/schema11.rs +++ b/rslib/src/decks/schema11.rs @@ -380,12 +380,6 @@ impl From for DeckSchema11 { impl From for DeckCommonSchema11 { fn from(deck: Deck) -> Self { - let mut other: HashMap = if deck.common.other.is_empty() { - Default::default() - } else { - serde_json::from_slice(&deck.common.other).unwrap_or_default() - }; - clear_other_duplicates(&mut other); DeckCommonSchema11 { id: deck.id, mtime: deck.mtime_secs, @@ -403,23 +397,11 @@ impl From for DeckCommonSchema11 { DeckKind::Normal(n) => n.description, DeckKind::Filtered(_) => String::new(), }, - other, + other: serde_json::from_slice(&deck.common.other).unwrap_or_default(), } } } -/// See [crate::deckconfig::schema11::clear_other_duplicates()]. -fn clear_other_duplicates(other: &mut HashMap) { - for key in [ - "reviewLimit", - "newLimit", - "reviewLimitToday", - "newLimitToday", - ] { - other.remove(key); - } -} - impl From<&Deck> for DeckTodaySchema11 { fn from(deck: &Deck) -> Self { let day = deck.common.last_day_studied as i32; diff --git a/rslib/src/notetype/schema11.rs b/rslib/src/notetype/schema11.rs index 174cbd20a..ef8e68c4c 100644 --- a/rslib/src/notetype/schema11.rs +++ b/rslib/src/notetype/schema11.rs @@ -170,19 +170,6 @@ impl From for NotetypeSchema11 { } } -/// See [crate::deckconfig::schema11::clear_other_duplicates()]. -fn clear_other_field_duplicates(other: &mut HashMap) { - for key in &[ - "description", - "plainText", - "collapsed", - "excludeFromSearch", - "originalStockKind", - ] { - other.remove(*key); - } -} - impl From for CardRequirement { fn from(r: CardRequirementSchema11) -> Self { CardRequirement { @@ -225,7 +212,6 @@ pub struct NoteFieldSchema11 { // This was not in schema 11, but needs to be listed here so that the setting is not lost // on downgrade/upgrade. - // NOTE: if adding new ones, make sure to update clear_other_field_duplicates() #[serde(default, deserialize_with = "default_on_invalid")] pub(crate) description: String, @@ -285,8 +271,6 @@ impl From for NoteField { impl From for NoteFieldSchema11 { fn from(p: NoteField) -> Self { let conf = p.config; - let mut other = bytes_to_other(&conf.other); - clear_other_field_duplicates(&mut other); NoteFieldSchema11 { name: p.name, ord: p.ord.map(|o| o as u16), @@ -298,7 +282,7 @@ impl From for NoteFieldSchema11 { description: conf.description, collapsed: conf.collapsed, exclude_from_search: conf.exclude_from_search, - other, + other: bytes_to_other(&conf.other), } } } diff --git a/rslib/src/serde.rs b/rslib/src/serde.rs index b0bcfc695..50a00bd4e 100644 --- a/rslib/src/serde.rs +++ b/rslib/src/serde.rs @@ -7,7 +7,24 @@ pub(crate) use serde_aux::field_attributes::deserialize_bool_from_anything; pub(crate) use serde_aux::field_attributes::deserialize_number_from_string; use serde_json::Value; -use crate::timestamp::TimestampSecs; +use crate::prelude::*; + +/// Serializes the value to JSON, removing any duplicate keys in the process. +/// +/// This function solves a very specific problem when (de)serializing structs on +/// up-/downgrade: +/// Older clients may have received keys from a newer client when +/// syncing, which get bundled into `other`. If they then upgrade, then +/// downgrade their collection to schema11, serde will serialize the +/// new default keys, but then add them again from `other`, leading +/// to the keys being duplicated in the resulting json - which older +/// clients then can't read. So we need to strip out any new keys we +/// add. +pub(crate) fn schema11_to_string(value: impl serde::Serialize) -> Result { + serde_json::to_value(value) + .and_then(|val| serde_json::to_string(&val)) + .map_err(Into::into) +} /// Note: if you wish to cover the case where a field is missing, make sure you /// also use the `serde(default)` flag. diff --git a/rslib/src/storage/deck/mod.rs b/rslib/src/storage/deck/mod.rs index 121d32014..84e71a2b0 100644 --- a/rslib/src/storage/deck/mod.rs +++ b/rslib/src/storage/deck/mod.rs @@ -404,7 +404,7 @@ impl SqliteStorage { pub(crate) fn downgrade_decks_from_schema15(&self) -> Result<()> { let decks = self.get_all_decks_as_schema11()?; - self.set_schema11_decks(decks) + self.set_schema11_decks(&decks) } fn get_schema11_decks(&self) -> Result> { @@ -420,8 +420,8 @@ impl SqliteStorage { Ok(decks) } - pub(crate) fn set_schema11_decks(&self, decks: HashMap) -> Result<()> { - let json = serde_json::to_string(&decks)?; + pub(crate) fn set_schema11_decks(&self, decks: &HashMap) -> Result<()> { + let json = crate::serde::schema11_to_string(decks)?; self.db.execute("update col set decks = ?", [json])?; Ok(()) } diff --git a/rslib/src/storage/deckconfig/mod.rs b/rslib/src/storage/deckconfig/mod.rs index 5ea57aae3..82f84443b 100644 --- a/rslib/src/storage/deckconfig/mod.rs +++ b/rslib/src/storage/deckconfig/mod.rs @@ -254,7 +254,7 @@ impl SqliteStorage { .collect(); self.db.execute( "update col set dconf=?", - params![serde_json::to_string(&confmap)?], + params![crate::serde::schema11_to_string(confmap)?], )?; Ok(()) } diff --git a/rslib/src/storage/notetype/mod.rs b/rslib/src/storage/notetype/mod.rs index 30625eaff..5c3b5ed5b 100644 --- a/rslib/src/storage/notetype/mod.rs +++ b/rslib/src/storage/notetype/mod.rs @@ -361,7 +361,7 @@ impl SqliteStorage { pub(crate) fn downgrade_notetypes_from_schema15(&self) -> Result<()> { let nts = self.get_all_notetypes_as_schema11()?; - self.set_schema11_notetypes(nts) + self.set_schema11_notetypes(&nts) } fn get_schema11_notetypes(&self) -> Result> { @@ -379,9 +379,9 @@ impl SqliteStorage { pub(crate) fn set_schema11_notetypes( &self, - notetypes: HashMap, + notetypes: &HashMap, ) -> Result<()> { - let json = serde_json::to_string(¬etypes)?; + let json = crate::serde::schema11_to_string(notetypes)?; self.db.execute("update col set models = ?", [json])?; Ok(()) } diff --git a/tools/minilints/src/main.rs b/tools/minilints/src/main.rs index 875d3b6a9..bfd39b676 100644 --- a/tools/minilints/src/main.rs +++ b/tools/minilints/src/main.rs @@ -36,6 +36,7 @@ const IGNORED_FOLDERS: &[&str] = &[ "./qt/aqt/forms", "./tools/workspace-hack", "./qt/bundle/PyOxidizer", + "./target", ]; fn main() -> Result<()> {