Exporting & importing

* exporting asks before overwriting
* importing does not create duplicates

src/exporting.rs

@@ -13,26 +13,27 @@ use crate::main_window::Display;
 use crate::main_window::MainWindow;
 use crate::NAMESPACE_PREFIX;
 
-pub type AccountsImportExportResult = ::std::result::Result<(), RepositoryError>;
+pub type AccountsImportExportResult = Result<(), RepositoryError>;
 
 pub trait Exporting {
-    fn export_accounts(&self, popover: gtk::PopoverMenu, connection: Arc<Mutex<Connection>>) -> Box<dyn Fn(&gtk::Button)>;
+    fn export_accounts(&self, popover: PopoverMenu, connection: Arc<Mutex<Connection>>) -> Box<dyn Fn(&Button)>;
 
-    fn import_accounts(&self, popover: gtk::PopoverMenu, connection: Arc<Mutex<Connection>>) -> Box<dyn Fn(&gtk::Button)>;
+    fn import_accounts(&self, popover: PopoverMenu, connection: Arc<Mutex<Connection>>) -> Box<dyn Fn(&Button)>;
 
     fn popup_close(popup: gtk::Window) -> Box<dyn Fn(&[glib::Value]) -> Option<glib::Value>>;
 }
 
 impl Exporting for MainWindow {
     fn export_accounts(&self, popover: PopoverMenu, connection: Arc<Mutex<Connection>>) -> Box<dyn Fn(&Button)> {
-        Box::new(clone!(@strong self as gui  => move |_: &gtk::Button| {
+        Box::new(clone!(@strong self as gui  => move |_| {
             popover.set_visible(false);
 
             let builder = gtk::Builder::from_resource(format!("{}/{}", NAMESPACE_PREFIX, "error_popup.ui").as_str());
             get_widget!(builder, gtk::FileChooserDialog, dialog);
             get_widget!(builder, gtk::Window, error_popup);
             get_widget!(builder, gtk::Label, error_popup_body);
 
+            dialog.set_do_overwrite_confirmation(true);
             error_popup_body.set_label(&gettext("Could not export accounts!"));
 
             builder.connect_signals(clone!(@strong error_popup  => move |_, handler_name| match handler_name {
@@ -82,7 +83,7 @@ impl Exporting for MainWindow {
         }))
     }
 
-    fn import_accounts(&self, popover: gtk::PopoverMenu, connection: Arc<Mutex<Connection>>) -> Box<dyn Fn(&gtk::Button)> {
+    fn import_accounts(&self, popover: PopoverMenu, connection: Arc<Mutex<Connection>>) -> Box<dyn Fn(&Button)> {
         Box::new(clone!(@strong self as gui  => move |_b: &gtk::Button| {
             popover.set_visible(false);
 

src/helpers/backup.rs

@@ -42,9 +42,10 @@ impl Backup {
     }
 
     pub async fn restore_account_and_signal_back(path: PathBuf, connection: Arc<Mutex<Connection>>, tx: Sender<AccountsImportExportResult>) {
-        let db = Self::restore_accounts(path, connection).await;
+        let db = Self::restore_accounts(path, connection.clone()).await;
 
-        match db.and_then(|_| Paths::update_keyring_secrets()) {
+        let connection = connection.lock().unwrap();
+        match db.and_then(|_| Paths::update_keyring_secrets(&connection)) {
             Ok(_) => tx.send(Ok(())).expect("Could not send message"),
             Err(e) => tx.send(Err(e)).expect("Could not send message"),
         }

src/helpers/database.rs

@@ -2,7 +2,7 @@ use std::fmt::Debug;
 use std::str::FromStr;
 use std::string::ToString;
 
-use log::warn;
+use log::{info, warn};
 use rusqlite::types::ToSqlOutput;
 use rusqlite::{named_params, params, Connection, OpenFlags, OptionalExtension, Row, ToSql};
 use serde::{Deserialize, Serialize};
@@ -66,6 +66,7 @@ impl Database {
     }
 
     pub fn update_group(connection: &Connection, group: &AccountGroup) -> Result<()> {
+        info!("Updating group {}", group.name);
         connection
             .execute(
                 "UPDATE groups SET name = ?2, icon = ?3, url = ?4, collapsed = ?5 WHERE id = ?1",
@@ -76,6 +77,8 @@ impl Database {
     }
 
     pub fn save_group(connection: &Connection, group: &mut AccountGroup) -> Result<()> {
+        info!("Adding group {}", group.name);
+
         connection.execute(
             "INSERT INTO groups (name, icon, url, collapsed) VALUES (?1, ?2, ?3, ?4)",
             params![group.name, group.icon, group.url, group.collapsed],
@@ -127,7 +130,7 @@ impl Database {
                 .iter_mut()
                 .map(|account| {
                     account.group_id = group_id;
-                    Self::save_account(connection, account)
+                    Self::upsert_account(connection, account)
                 })
                 .collect::<Result<Vec<u32>>>()
                 .map(|_| ()),
@@ -164,7 +167,19 @@ impl Database {
         .map_err(RepositoryError::SqlError)
     }
 
+    pub fn upsert_account(connection: &Connection, account: &mut Account) -> Result<u32> {
+        match Self::get_account_by_name(connection, account.label.as_str()).unwrap() {
+            Some(a) => {
+                account.id = a.id;
+                account.secret_type = LOCAL; // so that keyring get updated too
+                Self::update_account(connection, account)
+            }
+            None => Self::save_account(connection, account),
+        }
+    }
+
     pub fn save_account(connection: &Connection, account: &mut Account) -> Result<u32> {
+        info!("Adding account {}", account.label);
         let secret = if account.secret_type == KEYRING { "" } else { account.secret.as_str() };
 
         connection
@@ -185,6 +200,7 @@ impl Database {
     }
 
     pub fn update_account(connection: &Connection, account: &mut Account) -> Result<u32> {
+        info!("Updating account [{}:{}]", account.label, account.id);
         let secret = if account.secret_type == KEYRING { "" } else { account.secret.as_str() };
 
         connection
@@ -196,7 +212,7 @@ impl Database {
             .map_err(RepositoryError::SqlError)
     }
 
-    pub fn get_account(connection: &Connection, account_id: u32) -> Result<Account> {
+    pub fn get_account(connection: &Connection, account_id: u32) -> Result<Option<Account>> {
         let mut stmt = connection.prepare("SELECT id, group_id, label, secret, secret_type FROM accounts WHERE id = ?1")?;
 
         stmt.query_row(params![account_id], |row| {
@@ -211,22 +227,39 @@ impl Database {
 
             Ok(account)
         })
+        .optional()
+        .map_err(RepositoryError::SqlError)
+    }
+
+    pub fn get_account_by_name(connection: &Connection, name: &str) -> Result<Option<Account>> {
+        let mut stmt = connection.prepare("SELECT id, group_id, label, secret, secret_type FROM accounts WHERE label = ?1")?;
+
+        stmt.query_row(params![name], |row| {
+            let group_id: u32 = row.get_unwrap(1);
+            let label: String = row.get_unwrap(2);
+            let secret: String = row.get_unwrap(3);
+            let id = row.get_unwrap(0);
+
+            let secret_type = Database::extract_secret_type(row, 4);
+
+            let account = Account::new(id, group_id, label.as_str(), secret.as_str(), secret_type);
+
+            Ok(account)
+        })
+        .optional()
         .map_err(RepositoryError::SqlError)
     }
 
     fn extract_secret_type(row: &Row, idx: usize) -> SecretType {
-        match row.get::<_, String>(idx) {
+        match row.get::<usize, String>(idx) {
             Ok(v) => match SecretType::from_str(v.as_str()) {
                 Ok(secret_type) => secret_type,
                 Err(_) => {
                     warn!("Invalid secret type [{}]", v);
                     LOCAL
                 }
             },
-            Err(e) => {
-                warn!("Invalid secret type [{:?}]", e);
-                LOCAL
-            }
+            Err(e) => panic!("Index {} is invalid. [{:?}]", idx, e),
         }
     }
 
@@ -265,7 +298,7 @@ impl Database {
 
 impl Default for SecretType {
     fn default() -> Self {
-        SecretType::KEYRING
+        KEYRING
     }
 }
 
@@ -279,6 +312,7 @@ impl ToSql for SecretType {
 #[cfg(test)]
 mod tests {
     use rusqlite::Connection;
+    use serde_json::error::Category::Data;
 
     use crate::helpers::runner;
     use crate::helpers::SecretType::LOCAL;
@@ -305,7 +339,7 @@ mod tests {
         assert!(account.group_id > 0);
         assert_eq!("label", account.label);
 
-        let account_reloaded = Database::get_account(&connection, account.id).unwrap();
+        let account_reloaded = Database::get_account(&connection, account.id).unwrap().unwrap();
 
         assert_eq!(account, account_reloaded);
 
@@ -466,13 +500,21 @@ mod tests {
 
         runner::run(&mut connection).unwrap();
 
-        let account = Account::new(0, 0, "label", "secret", LOCAL);
-        let mut account_group = AccountGroup::new(0, "group", None, None, false, vec![account]);
+        let account1 = Account::new(0, 0, "label", "secret", LOCAL);
+        let account2 = Account::new(0, 0, "label2", "secret2", LOCAL);
+        let mut account_group = AccountGroup::new(0, "group", None, None, false, vec![account1, account2]);
 
         Database::save_group_and_accounts(&connection, &mut account_group).expect("could not save");
 
         assert!(account_group.id > 0);
-        assert_eq!(1, account_group.entries.len());
+        assert_eq!(2, account_group.entries.len());
         assert!(account_group.entries.first().unwrap().id > 0);
+
+        // saving sames accounts a second time should not produce duplicates
+        Database::save_group_and_accounts(&connection, &mut account_group).expect("could not save");
+
+        let accounts = Database::get_accounts(&connection, account_group.id, None).unwrap();
+        assert_eq!(2, account_group.entries.len());
+        assert_eq!(2, accounts.len());
     }
 }

src/helpers/paths.rs

@@ -1,5 +1,6 @@
 use anyhow::Result;
-use log::debug;
+use log::{debug, info};
+use rusqlite::Connection;
 
 use crate::helpers::{Database, Keyring, RepositoryError, SecretType};
 
@@ -49,20 +50,19 @@ impl Paths {
         Ok(())
     }
 
-    pub fn update_keyring_secrets() -> Result<(), RepositoryError> {
-        let connection = Database::create_connection()?;
-
-        let accounts = Database::load_account_groups(&connection, None)?;
+    pub fn update_keyring_secrets(connection: &Connection) -> Result<(), RepositoryError> {
+        let accounts = Database::load_account_groups(connection, None)?;
 
         accounts
             .iter()
             .flat_map(|group| group.entries.iter().cloned())
             .filter(|account| account.secret_type == SecretType::LOCAL)
             .for_each(|ref mut account| {
+                info!("Adding {} to keyring", account.label);
                 Keyring::upsert(account.label.as_str(), account.id, account.secret.as_str()).unwrap();
                 account.secret = "".to_owned();
                 account.secret_type = SecretType::KEYRING;
-                Database::update_account(&connection, account).unwrap();
+                Database::update_account(connection, account).unwrap();
             });
 
         Ok(())

src/main.rs

@@ -68,7 +68,7 @@ fn main() {
             Err(e) => panic!("{:?}", e),
         }
 
-        match Paths::update_keyring_secrets() {
+        match Paths::update_keyring_secrets(&connection) {
             Ok(()) => info!("Added local accounts to keyring"),
             Err(e) => panic!("{:?}", e),
         }

src/main_window.rs

@@ -75,35 +75,33 @@ impl MainWindow {
         let accounts_window = AccountsWindow::new(builder.clone());
         let errors = ErrorsWindow::new(builder.clone());
 
-        {
-            get_widget!(builder, gtk::Button, add_group_save);
-            get_widget!(builder, gtk::Button, edit_account_save);
-
-            builder.connect_signals(clone!(@strong about_popup  => move |_, handler_name| {
-                match handler_name {
-                    // handler_name as defined in the glade file
-                    "about_popup_close" => {
-                        Box::new(clone!( @strong about_popup => move |_| {
-                            about_popup.hide();
-                            None
-                        }))
-                    }
-                    "save_group" => {
-                        Box::new(clone!( @strong add_group_save => move |_| {
-                            add_group_save.clicked();
-                            None
-                        }))
-                    }
-                    "save_account" => {
-                        Box::new(clone!( @strong edit_account_save => move |_| {
-                            edit_account_save.clicked();
-                            None
-                        }))
-                    }
-                    _ => Box::new(|_| None),
+        get_widget!(builder, gtk::Button, add_group_save);
+        get_widget!(builder, gtk::Button, edit_account_save);
+
+        builder.connect_signals(clone!(@strong about_popup  => move |_, handler_name| {
+            match handler_name {
+                // handler_name as defined in the glade file
+                "about_popup_close" => {
+                    Box::new(clone!( @strong about_popup => move |_| {
+                        about_popup.hide();
+                        None
+                    }))
                 }
-            }));
-        }
+                "save_group" => {
+                    Box::new(clone!( @strong add_group_save => move |_| {
+                        add_group_save.clicked();
+                        None
+                    }))
+                }
+                "save_account" => {
+                    Box::new(clone!( @strong edit_account_save => move |_| {
+                        edit_account_save.clicked();
+                        None
+                    }))
+                }
+                _ => Box::new(|_| None),
+            }
+        }));
 
         MainWindow {
             window: main_window,

src/ui/accounts_window.rs

@@ -342,32 +342,37 @@ impl AccountsWindow {
                     let groups = Database::load_account_groups(&connection, None).unwrap();
                     let account = Database::get_account(&connection, id).unwrap();
 
-                    edit_account.input_group.remove_all(); //re-added and refreshed just below
-
-                    edit_account.set_group_dropdown(Some(account.group_id), &groups);
-
-                    let account_id = account.id.to_string();
-                    edit_account.input_account_id.set_text(account_id.as_str());
-                    edit_account.input_name.set_text(account.label.as_str());
-
-                    edit_account.add_accounts_container_add.set_visible(false);
-                    edit_account.add_accounts_container_edit.set_visible(true);
-
-                    edit_account.add_accounts_container_edit.set_text(account.label.as_str());
-
-                    popover.hide();
-
-                    match Keyring::secret(account.id) {
-                        Ok(secret) => {
-                            let buffer = edit_account.input_secret.buffer().unwrap();
-                            buffer.set_text(secret.unwrap_or_default().as_str());
-                            gui.switch_to(Display::EditAccount);
+                    match account {
+                        Some(account) => {
+                            edit_account.input_group.remove_all(); //re-added and refreshed just below
+
+                            edit_account.set_group_dropdown(Some(account.group_id), &groups);
+
+                            let account_id = account.id.to_string();
+                            edit_account.input_account_id.set_text(account_id.as_str());
+                            edit_account.input_name.set_text(account.label.as_str());
+
+                            edit_account.add_accounts_container_add.set_visible(false);
+                            edit_account.add_accounts_container_edit.set_visible(true);
+
+                            edit_account.add_accounts_container_edit.set_text(account.label.as_str());
+
+                            popover.hide();
+
+                            match Keyring::secret(account.id) {
+                                Ok(secret) => {
+                                    let buffer = edit_account.input_secret.buffer().unwrap();
+                                    buffer.set_text(secret.unwrap_or_default().as_str());
+                                    gui.switch_to(Display::EditAccount);
+                                },
+                                Err(e) => {
+                                    gui.errors.error_display_message.set_text(format!("{:?}", e).as_str());
+                                    gui.switch_to(Display::Errors);
+                                }
+                            };
                         },
-                        Err(e) => {
-                            gui.errors.error_display_message.set_text(format!("{:?}", e).as_str());
-                            gui.switch_to(Display::Errors);
-                        }
-                    };
+                        None => panic!("Account {} not found", id)
+                    }
                 }));
             }
         }