From bdc5c619f7e767e9a2a147b303513d827b5f95e7 Mon Sep 17 00:00:00 2001
From: Damien Elmes <gpg@ankiweb.net>
Date: Sat, 23 Jan 2021 12:43:03 +1000
Subject: [PATCH] handle decks/notetypes with a duplicate name being sent in a
 sync

Typically caused by older clients, but could happen if the user added
the same name on different devices without syncing.

Also add an inactive test that was used to try track down this issue
and might be useful in the future.
---
 rslib/src/decks/mod.rs    |  2 +-
 rslib/src/notetype/mod.rs |  8 ++++--
 rslib/src/sync/mod.rs     | 54 ++++++++++++++++++++++++++++++++++-----
 3 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/rslib/src/decks/mod.rs b/rslib/src/decks/mod.rs
index ff3c05dba..27f7d1e70 100644
--- a/rslib/src/decks/mod.rs
+++ b/rslib/src/decks/mod.rs
@@ -225,7 +225,7 @@ impl Collection {
         self.storage.update_deck(deck)
     }
 
-    fn ensure_deck_name_unique(&self, deck: &mut Deck, usn: Usn) -> Result<()> {
+    pub(crate) fn ensure_deck_name_unique(&self, deck: &mut Deck, usn: Usn) -> Result<()> {
         loop {
             match self.storage.get_deck_id(&deck.name)? {
                 Some(did) if did == deck.id => {
diff --git a/rslib/src/notetype/mod.rs b/rslib/src/notetype/mod.rs
index ebf13f180..0a3c9ccf5 100644
--- a/rslib/src/notetype/mod.rs
+++ b/rslib/src/notetype/mod.rs
@@ -399,10 +399,14 @@ impl Collection {
         self.storage.add_new_notetype(nt)
     }
 
-    fn ensure_notetype_name_unique(&self, notetype: &mut NoteType, usn: Usn) -> Result<()> {
+    pub(crate) fn ensure_notetype_name_unique(
+        &self,
+        notetype: &mut NoteType,
+        usn: Usn,
+    ) -> Result<()> {
         loop {
             match self.storage.get_notetype_id(&notetype.name)? {
-                Some(did) if did == notetype.id => {
+                Some(id) if id == notetype.id => {
                     break;
                 }
                 None => break,
diff --git a/rslib/src/sync/mod.rs b/rslib/src/sync/mod.rs
index 7bef2c751..127cdf2cf 100644
--- a/rslib/src/sync/mod.rs
+++ b/rslib/src/sync/mod.rs
@@ -822,8 +822,8 @@ impl Collection {
     //----------------------------------------------------------------
 
     fn apply_changes(&mut self, remote: UnchunkedChanges, latest_usn: Usn) -> Result<()> {
-        self.merge_notetypes(remote.notetypes)?;
-        self.merge_decks(remote.decks_and_config.decks)?;
+        self.merge_notetypes(remote.notetypes, latest_usn)?;
+        self.merge_decks(remote.decks_and_config.decks, latest_usn)?;
         self.merge_deck_config(remote.decks_and_config.config)?;
         self.merge_tags(remote.tags, latest_usn)?;
         if let Some(crt) = remote.creation_stamp {
@@ -837,9 +837,9 @@ impl Collection {
         Ok(())
     }
 
-    fn merge_notetypes(&mut self, notetypes: Vec<NoteTypeSchema11>) -> Result<()> {
+    fn merge_notetypes(&mut self, notetypes: Vec<NoteTypeSchema11>, latest_usn: Usn) -> Result<()> {
         for nt in notetypes {
-            let nt: NoteType = nt.into();
+            let mut nt: NoteType = nt.into();
             let proceed = if let Some(existing_nt) = self.storage.get_notetype(nt.id)? {
                 if existing_nt.mtime_secs <= nt.mtime_secs {
                     if (existing_nt.fields.len() != nt.fields.len())
@@ -858,6 +858,7 @@ impl Collection {
                 true
             };
             if proceed {
+                self.ensure_notetype_name_unique(&mut nt, latest_usn)?;
                 self.storage.add_or_update_notetype(&nt)?;
                 self.state.notetype_cache.remove(&nt.id);
             }
@@ -865,7 +866,7 @@ impl Collection {
         Ok(())
     }
 
-    fn merge_decks(&mut self, decks: Vec<DeckSchema11>) -> Result<()> {
+    fn merge_decks(&mut self, decks: Vec<DeckSchema11>, latest_usn: Usn) -> Result<()> {
         for deck in decks {
             let proceed = if let Some(existing_deck) = self.storage.get_deck(deck.id())? {
                 existing_deck.mtime_secs <= deck.common().mtime
@@ -873,7 +874,8 @@ impl Collection {
                 true
             };
             if proceed {
-                let deck = deck.into();
+                let mut deck = deck.into();
+                self.ensure_deck_name_unique(&mut deck, latest_usn)?;
                 self.storage.add_or_update_deck(&deck)?;
                 self.state.deck_cache.remove(&deck.id);
             }
@@ -1538,4 +1540,44 @@ mod test {
         assert!(matches!(out.required, SyncActionRequired::FullSyncRequired { .. }));
         Ok(())
     }
+
+    // Helper to reproduce issues with a copy of the client and server collections.
+    // #[test]
+    // fn repro_test() {
+    //     let mut rt = Runtime::new().unwrap();
+    //     rt.block_on(repro_test_inner()).unwrap();
+    // }
+
+    // async fn repro_test_inner() -> Result<()> {
+    //     let client_fname = "/path/to/collection1.anki2";
+    //     let server_fname = "/path/to/collection2.anki2";
+
+    //     use std::env::temp_dir;
+    //     use std::fs;
+    //     use tempfile::NamedTempFile;
+
+    //     let client_col_file = NamedTempFile::new()?;
+    //     let client_col_name = client_col_file
+    //         .path()
+    //         .file_name()
+    //         .unwrap()
+    //         .to_string_lossy();
+    //     fs::copy(client_fname, client_col_file.path())?;
+    //     let server_col_file = NamedTempFile::new()?;
+    //     let server_col_name = server_col_file
+    //         .path()
+    //         .file_name()
+    //         .unwrap()
+    //         .to_string_lossy();
+    //     fs::copy(server_fname, server_col_file.path())?;
+    //     let dir = temp_dir();
+    //     let server = Box::new(LocalServer::new(open_col(&dir, true, &server_col_name)?));
+    //     let mut client_col = open_col(&dir, false, &client_col_name)?;
+    //     NormalSyncer::new(&mut client_col, server, norm_progress)
+    //         .sync()
+    //         .await
+    //         .unwrap();
+
+    //     Ok(())
+    // }
 }