refactor: Use methods instead of free functions with UpdateState (#74)

library/src/cache.rs

@@ -89,70 +89,70 @@ impl UpdaterState {
         }
         self.successful_patches.push(patch.version.clone());
     }
-}
 
-pub fn load_state(cache_dir: &str) -> anyhow::Result<UpdaterState> {
-    // Load UpdaterState from disk
-    let path = Path::new(cache_dir).join("state.json");
-    let file = File::open(path)?;
-    let reader = BufReader::new(file);
-    // TODO: Now that we depend on serde_yaml for shorebird.yaml
-    // we could use yaml here instead of json.
-    let state = serde_json::from_reader(reader)?;
-    Ok(state)
-}
-
-pub fn save_state(state: &UpdaterState, cache_dir: &str) -> anyhow::Result<()> {
-    // Save UpdaterState to disk
-    std::fs::create_dir_all(cache_dir)?;
-    let path = Path::new(cache_dir).join("state.json");
-    let file = File::create(path)?;
-    let writer = BufWriter::new(file);
-    serde_json::to_writer_pretty(writer, &state)?;
-    Ok(())
-}
-
-pub fn client_id(state: &UpdaterState) -> String {
-    state.client_id.to_string()
-}
-
-pub fn current_patch(state: &UpdaterState) -> Option<PatchInfo> {
-    // If there is no state, return None.
-    if state.slots.is_empty() {
-        return None;
+    pub fn load(cache_dir: &str) -> anyhow::Result<Self> {
+        // Load UpdaterState from disk
+        let path = Path::new(cache_dir).join("state.json");
+        let file = File::open(path)?;
+        let reader = BufReader::new(file);
+        // TODO: Now that we depend on serde_yaml for shorebird.yaml
+        // we could use yaml here instead of json.
+        let state = serde_json::from_reader(reader)?;
+        Ok(state)
     }
-    let slot = &state.slots[state.current_slot_index];
-    // Otherwise return the version info from the current slot.
-    return Some(PatchInfo {
-        path: slot.path.clone(),
-        version: slot.patch_version.clone(),
-    });
-}
 
-fn unused_slot(state: &UpdaterState) -> usize {
-    // Assume we only use two slots and pick the one that's not current.
-    if state.slots.is_empty() {
+    pub fn save(&self, cache_dir: &str) -> anyhow::Result<()> {
+        // Save UpdaterState to disk
+        std::fs::create_dir_all(cache_dir)?;
+        let path = Path::new(cache_dir).join("state.json");
+        let file = File::create(path)?;
+        let writer = BufWriter::new(file);
+        serde_json::to_writer_pretty(writer, self)?;
+        Ok(())
+    }
+
+    pub fn client_id(&self) -> String {
+        self.client_id.to_string()
+    }
+
+    pub fn current_patch(&self) -> Option<PatchInfo> {
+        // If there is no state, return None.
+        if self.slots.is_empty() {
+            return None;
+        }
+        let slot = &self.slots[self.current_slot_index];
+        // Otherwise return the version info from the current slot.
+        return Some(PatchInfo {
+            path: slot.path.clone(),
+            version: slot.patch_version.clone(),
+        });
+    }
+
+    fn unused_slot(&self) -> usize {
+        // Assume we only use two slots and pick the one that's not current.
+        if self.slots.is_empty() {
+            return 0;
+        }
+        if self.current_slot_index == 0 {
+            return 1;
+        }
         return 0;
     }
-    if state.current_slot_index == 0 {
-        return 1;
-    }
-    return 0;
-}
 
-fn set_slot(state: &mut UpdaterState, index: usize, slot: Slot) {
-    if state.slots.len() < index + 1 {
-        // Make sure we're not filling with empty slots.
-        assert!(state.slots.len() == index);
-        state.slots.resize(index + 1, Slot::default());
+    fn set_slot(&mut self, index: usize, slot: Slot) {
+        if self.slots.len() < index + 1 {
+            // Make sure we're not filling with empty slots.
+            assert!(self.slots.len() == index);
+            self.slots.resize(index + 1, Slot::default());
+        }
+        // Set the given slot to the given version.
+        self.slots[index] = slot
     }
-    // Set the given slot to the given version.
-    state.slots[index] = slot
-}
 
-pub fn set_current_slot(state: &mut UpdaterState, index: usize) {
-    state.current_slot_index = index;
-    // This does not implicitly save the state, but maybe should?
+    pub fn set_current_slot(&mut self, index: usize) {
+        self.current_slot_index = index;
+        // This does not implicitly save the state, but maybe should?
+    }
 }
 
 pub fn download_into_unused_slot(
@@ -161,7 +161,7 @@ pub fn download_into_unused_slot(
     state: &mut UpdaterState,
 ) -> anyhow::Result<usize> {
     // Download the new version into the unused slot.
-    let slot_index = unused_slot(state);
+    let slot_index = state.unused_slot();
     download_into_slot(cache_dir, patch_check_response, state, slot_index)?;
     Ok(slot_index)
 }
@@ -189,15 +189,14 @@ fn download_into_slot(
     // Check the hash against the download?
 
     // Update the state to include the new version.
-    set_slot(
-        state,
+    state.set_slot(
         slot_index,
         Slot {
             path: path.to_str().unwrap().to_string(),
             patch_version: patch.version.clone(),
         },
     );
-    save_state(&state, cache_dir)?;
+    state.save(cache_dir)?;
 
     return Ok(());
 }

library/src/network.rs

@@ -1,16 +1,18 @@
 // This file's job is to deal with the update_server and network side
 // of the updater library.
 
-use std::collections::HashMap;
-use std::string::ToString;
-
 use serde::Deserialize;
+use std::collections::HashMap;
+use std::fs::File;
+use std::io::Write;
+use std::path::PathBuf;
+use std::string::ToString;
 
 use std::fs::File;
 use std::io::Write;
 use std::path::PathBuf;
 
-use crate::cache::{client_id, current_patch, UpdaterState};
+use crate::cache::UpdaterState;
 use crate::config::{current_arch, current_platform, ResolvedConfig};
 
 fn patches_check_url(base_url: &str) -> String {
@@ -35,12 +37,12 @@ pub fn send_patch_check_request(
     config: &ResolvedConfig,
     state: &UpdaterState,
 ) -> anyhow::Result<PatchCheckResponse> {
-    let patch = current_patch(state);
+    let patch = state.current_patch();
 
     // Send the request to the server.
     let client = reqwest::blocking::Client::new();
     let mut body = HashMap::new();
-    body.insert("client_id", client_id(state));
+    body.insert("client_id", state.client_id());
     body.insert("product_id", config.product_id.clone());
     body.insert("channel", config.channel.clone());
     body.insert("base_version", config.base_version.clone());

library/src/updater.rs

@@ -2,9 +2,7 @@
 
 use std::fmt::{Display, Formatter};
 
-use crate::cache::{
-    current_patch, download_into_unused_slot, load_state, save_state, set_current_slot, PatchInfo,
-};
+use crate::cache::{download_into_unused_slot, PatchInfo, UpdaterState};
 use crate::config::{set_config, with_config, ResolvedConfig};
 use crate::logging::init_logging;
 use crate::network::send_patch_check_request;
@@ -54,7 +52,7 @@ pub fn init(app_config: AppConfig, yaml: &str) {
 fn check_for_update_internal(config: &ResolvedConfig) -> bool {
     // Load UpdaterState from disk
     // If there is no state, make an empty state.
-    let state = load_state(&config.cache_dir).unwrap_or_default();
+    let state = UpdaterState::load(&config.cache_dir).unwrap_or_default();
     // Send info from app + current slot to server.
     let response_result = send_patch_check_request(&config, &state);
     match response_result {
@@ -75,7 +73,7 @@ pub fn check_for_update() -> bool {
 
 fn update_internal(config: &ResolvedConfig) -> anyhow::Result<UpdateStatus> {
     // Load the state from disk.
-    let mut state = load_state(&config.cache_dir).unwrap_or_default();
+    let mut state = UpdaterState::load(&config.cache_dir).unwrap_or_default();
     // Check for update.
     let response = send_patch_check_request(&config, &state)?;
     if !response.patch_available {
@@ -84,8 +82,8 @@ fn update_internal(config: &ResolvedConfig) -> anyhow::Result<UpdateStatus> {
     // If needed, download the new version.
     let slot = download_into_unused_slot(&config.cache_dir, &response, &mut state)?;
     // Install the new version.
-    set_current_slot(&mut state, slot);
-    save_state(&state, &config.cache_dir)?;
+    state.set_current_slot(slot);
+    state.save(&config.cache_dir)?;
     // Set the state to "restart required".
     return Ok(UpdateStatus::UpdateInstalled);
 }
@@ -93,28 +91,28 @@ fn update_internal(config: &ResolvedConfig) -> anyhow::Result<UpdateStatus> {
 /// Reads the current patch from the cache and returns it.
 pub fn active_patch() -> Option<PatchInfo> {
     return with_config(|config| {
-        let state = load_state(&config.cache_dir).unwrap_or_default();
-        return current_patch(&state);
+        let state = UpdaterState::load(&config.cache_dir).unwrap_or_default();
+        return state.current_patch();
     });
 }
 
 pub fn report_failed_launch() {
     with_config(|config| {
-        let mut state = load_state(&config.cache_dir).unwrap_or_default();
+        let mut state = UpdaterState::load(&config.cache_dir).unwrap_or_default();
 
-        let patch = current_patch(&state).unwrap();
+        let patch = state.current_patch().unwrap();
         state.mark_patch_as_bad(&patch);
-        save_state(&state, &config.cache_dir).unwrap();
+        state.save(&config.cache_dir).unwrap();
     });
 }
 
 pub fn report_successful_launch() {
     with_config(|config| {
-        let mut state = load_state(&config.cache_dir).unwrap_or_default();
+        let mut state = UpdaterState::load(&config.cache_dir).unwrap_or_default();
 
-        let patch = current_patch(&state).unwrap();
+        let patch = state.current_patch().unwrap();
         state.mark_patch_as_good(&patch);
-        save_state(&state, &config.cache_dir).unwrap();
+        state.save(&config.cache_dir).unwrap();
     });
 }