Make AccountHistory hold track of its own cache path

2 files changed, 54 insertions, 37 deletions

diff --git a/mullvad-daemon/src/account_history.rs b/mullvad-daemon/src/account_history.rs
index d9047ad5f2..016d2fd780 100644
--- a/mullvad-daemon/src/account_history.rs
+++ b/mullvad-daemon/src/account_history.rs
@@ -2,7 +2,7 @@ extern crate serde_json;
 
 use std::fs::File;
 use std::io;
-use std::path::{PathBuf, Path};
+use std::path::{Path, PathBuf};
 
 use mullvad_types::account::AccountToken;
 
@@ -26,40 +26,50 @@ static ACCOUNT_HISTORY_FILE: &str = "account-history.json";
 static ACCOUNT_HISTORY_LIMIT: usize = 3;
 
 #[derive(Debug, Clone, Deserialize, Serialize, Default)]
-#[serde(default)]
 pub struct AccountHistory {
     accounts: Vec<AccountToken>,
+    #[serde(skip)]
+    cache_path: PathBuf,
 }
 
 impl AccountHistory {
-    /// Loads account history from file. If no file is present it returns the defaults.
-    pub fn load(cache_dir: &Path) -> Result<AccountHistory> {
-        let history_path = cache_dir.join(ACCOUNT_HISTORY_FILE);
-        match File::open(&history_path) {
-            Ok(file) => {
+    /// Returns a new empty `AccountHistory` ready to load from, or save to, the given cache dir.
+    pub fn new(cache_dir: &Path) -> AccountHistory {
+        AccountHistory {
+            accounts: Vec::new(),
+            cache_path: cache_dir.join(ACCOUNT_HISTORY_FILE),
+        }
+    }
+
+    /// Loads account history from file. If no file is present this does nothing.
+    pub fn load(&mut self) -> Result<()> {
+        match File::open(&self.cache_path).map(io::BufReader::new) {
+            Ok(mut file) => {
                 info!(
                     "Loading account history from {}",
-                    history_path.display()
+                    &self.cache_path.display()
                 );
-                Self::parse(&mut io::BufReader::new(file))
+                self.accounts = Self::parse(&mut file)?.accounts;
+                Ok(())
             }
             Err(ref e) if e.kind() == io::ErrorKind::NotFound => {
-                info!(
-                    "No account history file at {}, using defaults",
-                    history_path.display()
-                );
-                Ok(AccountHistory::default())
+                info!("No account history file at {}", &self.cache_path.display());
+                Ok(())
             }
-            Err(e) => Err(e).chain_err(|| ErrorKind::ReadError(history_path)),
+            Err(e) => Err(e).chain_err(|| ErrorKind::ReadError(self.cache_path.clone())),
         }
     }
 
+    fn parse(file: &mut impl io::Read) -> Result<AccountHistory> {
+        serde_json::from_reader(file).chain_err(|| ErrorKind::ParseError)
+    }
+
     pub fn get_accounts(&self) -> &[AccountToken] {
         &self.accounts
     }
 
     /// Add account token to the account history removing duplicate entries
-    pub fn add_account_token(&mut self, account_token: AccountToken, cache_dir: &Path) -> Result<()> {
+    pub fn add_account_token(&mut self, account_token: AccountToken) -> Result<()> {
         self.accounts
             .retain(|existing_token| existing_token != &account_token);
         self.accounts.push(account_token);
@@ -71,27 +81,24 @@ impl AccountHistory {
                 .split_off(num_accounts - ACCOUNT_HISTORY_LIMIT);
         }
 
-        self.save(cache_dir)
+        self.save()
     }
 
     /// Remove account token from the account history
-    pub fn remove_account_token(&mut self, account_token: AccountToken, cache_dir: &Path) -> Result<()> {
+    pub fn remove_account_token(&mut self, account_token: AccountToken) -> Result<()> {
         self.accounts
             .retain(|existing_token| existing_token != &account_token);
-        self.save(cache_dir)
+        self.save()
     }
 
     /// Serializes the account history and saves it to the file it was loaded from.
-    fn save(&self, cache_dir: &Path) -> Result<()> {
-        let path = cache_dir.join(ACCOUNT_HISTORY_FILE);
+    fn save(&self) -> Result<()> {
+        debug!("Writing account history to {}", self.cache_path.display());
+        let file = File::create(&self.cache_path)
+            .map(io::BufWriter::new)
+            .chain_err(|| ErrorKind::WriteError(self.cache_path.clone()))?;
 
-        debug!("Writing account history to {}", path.display());
-        let file = File::create(&path).chain_err(|| ErrorKind::WriteError(path.clone()))?;
-
-        serde_json::to_writer_pretty(io::BufWriter::new(file), self).chain_err(|| ErrorKind::WriteError(path))
-    }
-
-    fn parse(file: &mut impl io::Read) -> Result<AccountHistory> {
-        serde_json::from_reader(file).chain_err(|| ErrorKind::ParseError)
+        serde_json::to_writer_pretty(file, self)
+            .chain_err(|| ErrorKind::WriteError(self.cache_path.clone()))
     }
 }
diff --git a/mullvad-daemon/src/management_interface.rs b/mullvad-daemon/src/management_interface.rs
index bee8f1e0ae..63fcadadd3 100644
--- a/mullvad-daemon/src/management_interface.rs
+++ b/mullvad-daemon/src/management_interface.rs
@@ -29,7 +29,7 @@ use talpid_ipc;
 use talpid_types::net::TunnelOptions;
 use uuid;
 
-use account_history::AccountHistory;
+use account_history::{AccountHistory, Error as AccountHistoryError};
 
 /// FIXME(linus): This is here just because the futures crate has deprecated it and jsonrpc_core
 /// did not introduce their own yet (https://github.com/paritytech/jsonrpc/pull/196).
@@ -291,7 +291,11 @@ struct ManagementInterface<T: From<TunnelCommand> + 'static + Send> {
 }
 
 impl<T: From<TunnelCommand> + 'static + Send> ManagementInterface<T> {
-    pub fn new(tx: IntoSender<TunnelCommand, T>, shared_secret: String, cache_dir: PathBuf) -> Self {
+    pub fn new(
+        tx: IntoSender<TunnelCommand, T>,
+        shared_secret: String,
+        cache_dir: PathBuf,
+    ) -> Self {
         ManagementInterface {
             subscriptions: Default::default(),
             tx: Mutex::new(tx),
@@ -370,6 +374,12 @@ impl<T: From<TunnelCommand> + 'static + Send> ManagementInterface<T> {
             Err(Error::invalid_request())
         }
     }
+
+    fn load_history(&self) -> Result<AccountHistory, AccountHistoryError> {
+        let mut account_history = AccountHistory::new(&self.cache_dir);
+        account_history.load()?;
+        Ok(account_history)
+    }
 }
 
 /// Evaluates a Result and early returns an error.
@@ -444,8 +454,8 @@ impl<T: From<TunnelCommand> + 'static + Send> ManagementInterfaceApi for Managem
             .and_then(|_| rx.map_err(|_| Error::internal_error()));
 
         if let Some(new_account_token) = account_token {
-            if let Err(e) = AccountHistory::load(&self.cache_dir).and_then(|mut account_history| {
-                account_history.add_account_token(new_account_token, &self.cache_dir)
+            if let Err(e) = self.load_history().and_then(|mut account_history| {
+                account_history.add_account_token(new_account_token)
             }) {
                 error!(
                     "Unable to add an account into the account history: {}",
@@ -561,8 +571,8 @@ impl<T: From<TunnelCommand> + 'static + Send> ManagementInterfaceApi for Managem
         trace!("get_account_history");
         try_future!(self.check_auth(&meta));
         Box::new(future::result(
-            AccountHistory::load(&self.cache_dir)
-                .map(|account_history| account_history.get_accounts().to_vec())
+            self.load_history()
+                .map(|history| history.get_accounts().to_vec())
                 .map_err(|error| {
                     error!("Unable to get account history: {}", error.display_chain());
                     Error::internal_error()
@@ -578,8 +588,8 @@ impl<T: From<TunnelCommand> + 'static + Send> ManagementInterfaceApi for Managem
         trace!("remove_account_from_history");
         try_future!(self.check_auth(&meta));
         Box::new(future::result(
-            AccountHistory::load(&self.cache_dir)
-                .and_then(|mut history| history.remove_account_token(account_token, &self.cache_dir))
+            self.load_history()
+                .and_then(|mut history| history.remove_account_token(account_token))
                 .map_err(|error| {
                     error!(
                         "Unable to remove account from history: {}",