From f4db98b7597d4b6d2ca162d8b97d98cb78e964fe Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Fri, 22 Sep 2023 16:58:31 -0400 Subject: [PATCH] feat: treat iOS patches as diffs instead of full artifacts (#88) * feat: apply iOS patches from diffs * Clean up imports * fix lint * Add iOS, tests * refactor * revert changes to shorebird_code_push * remove unused debug trait * coverage * imports --- .vscode/settings.json | 2 +- library/src/c_api.rs | 3 +- library/src/lib.rs | 4 +- library/src/{ => platform}/android.rs | 3 + library/src/platform/ios.rs | 85 +++++++++++++++++++++++++++ library/src/platform/mod.rs | 13 ++++ library/src/platform/unknown.rs | 20 +++++++ library/src/updater.rs | 53 +++-------------- 8 files changed, 134 insertions(+), 49 deletions(-) rename library/src/{ => platform}/android.rs (98%) create mode 100644 library/src/platform/ios.rs create mode 100644 library/src/platform/mod.rs create mode 100644 library/src/platform/unknown.rs diff --git a/.vscode/settings.json b/.vscode/settings.json index a068272..bf390ba 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -15,5 +15,5 @@ "reqwest", "tempdir", "vmcode" - ] + ], } \ No newline at end of file diff --git a/library/src/c_api.rs b/library/src/c_api.rs index 9282121..fd22be5 100644 --- a/library/src/c_api.rs +++ b/library/src/c_api.rs @@ -255,6 +255,7 @@ pub extern "C" fn shorebird_report_launch_success() { mod test { use super::*; use crate::network::{testing_set_network_hooks, PatchCheckResponse}; + use crate::platform; use anyhow::Ok; use serial_test::serial; use tempdir::TempDir; @@ -409,7 +410,7 @@ mod test { let options = zip::write::FileOptions::default() .compression_method(zip::CompressionMethod::Stored) .unix_permissions(0o755); - let app_path = crate::android::get_relative_lib_path("libapp.so"); + let app_path = platform::get_relative_lib_path("libapp.so"); zip.start_file(app_path.to_str().unwrap(), options).unwrap(); zip.write_all(libapp_contents).unwrap(); zip.finish().unwrap(); diff --git a/library/src/lib.rs b/library/src/lib.rs index c63d07d..41e2499 100644 --- a/library/src/lib.rs +++ b/library/src/lib.rs @@ -11,13 +11,11 @@ mod config; mod events; mod logging; mod network; +mod platform; mod updater; mod updater_lock; mod yaml; -#[cfg(any(target_os = "android", test))] -mod android; - // Take all public items from the updater namespace and make them public. pub use self::updater::*; diff --git a/library/src/android.rs b/library/src/platform/android.rs similarity index 98% rename from library/src/android.rs rename to library/src/platform/android.rs index 9a5dea4..1106169 100644 --- a/library/src/android.rs +++ b/library/src/platform/android.rs @@ -184,6 +184,9 @@ pub(crate) fn open_base_lib(apks_dir: &Path, lib_name: &str) -> anyhow::Result Result { // FIXME: This makes the assumption that the last path provided is the full // path to the libapp.so file. This is true for the current engine, but diff --git a/library/src/platform/ios.rs b/library/src/platform/ios.rs new file mode 100644 index 0000000..c261d3f --- /dev/null +++ b/library/src/platform/ios.rs @@ -0,0 +1,85 @@ +use std::{ + fs::File, + io::{Cursor, Read}, + path::{Path, PathBuf}, +}; + +use anyhow::Context; + +use crate::UpdateError; + +/// lib name is unused on iOS, it exists as a parameter here to match the signature of the +/// function on Android. +pub(crate) fn open_base_lib(app_dir: &Path, _lib_name: &str) -> anyhow::Result>> { + let mut file = + File::open(app_dir).with_context(|| format!("Failed to open iOS app_dir {:?}", app_dir))?; + let mut buffer = Vec::new(); + file.read_to_end(&mut buffer) + .with_context(|| format!("Failed to read iOS app_dir {:?}", app_dir))?; + Ok(Cursor::new(buffer)) +} + +pub fn libapp_path_from_settings(original_libapp_paths: &[String]) -> Result { + let first = original_libapp_paths + .first() + .ok_or(UpdateError::InvalidArgument( + "original_libapp_paths".to_string(), + "empty".to_string(), + )); + first.map(PathBuf::from) +} + +#[cfg(test)] +mod tests { + use std::{fs::File, path::PathBuf}; + + use tempdir::TempDir; + + use crate::UpdateError; + + use super::{libapp_path_from_settings, open_base_lib}; + + #[test] + fn opens_and_reads_app() { + let tmp_dir = TempDir::new("test").unwrap(); + let path = tmp_dir.path().join("foo.txt"); + File::create(&path).unwrap(); + let result = open_base_lib(&path.to_path_buf(), ""); + assert!(result.is_ok()); + } + + #[test] + fn returns_error_if_app_fails_to_open() { + let tmp_dir = TempDir::new("test").unwrap(); + let path = tmp_dir.path().join("foo.txt"); + let result = open_base_lib(&path.to_path_buf(), ""); + assert!(result.is_err()); + assert_eq!( + format!("{}", result.unwrap_err()), + format!("Failed to open iOS app_dir \"{}\"", path.to_str().unwrap()), + ); + } + + // TODO(bryanoltman): we don't currently test read_to_end returning an Err + // result. We should do that, but I'm not sure how. + + #[test] + fn libapp_path_from_settings_returns_first_path() { + let path1 = "some/path/1".to_string(); + let path2 = "some/path/2".to_string(); + let path3 = "some/path/3".to_string(); + let result = libapp_path_from_settings(&[path1.clone(), path2.clone(), path3.clone()]); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), PathBuf::from(path1)); + } + + #[test] + fn libapp_path_from_settings_returns_err_when_provided_slice_is_empty() { + let result = libapp_path_from_settings(&[]); + assert!(result.is_err()); + assert_eq!( + result.unwrap_err(), + UpdateError::InvalidArgument("original_libapp_paths".to_string(), "empty".to_string(),) + ); + } +} diff --git a/library/src/platform/mod.rs b/library/src/platform/mod.rs new file mode 100644 index 0000000..310ce0e --- /dev/null +++ b/library/src/platform/mod.rs @@ -0,0 +1,13 @@ +#[cfg(any(target_os = "android", test))] +pub mod android; +#[cfg(any(target_os = "ios", test))] +pub mod ios; +#[cfg(not(any(target_os = "android", target_os = "ios", test)))] +pub mod unknown; + +#[cfg(any(target_os = "android", test))] +pub use android::*; +#[cfg(target_os = "ios")] +pub use ios::*; +#[cfg(not(any(target_os = "android", target_os = "ios", test)))] +pub use unknown::*; diff --git a/library/src/platform/unknown.rs b/library/src/platform/unknown.rs new file mode 100644 index 0000000..76ea585 --- /dev/null +++ b/library/src/platform/unknown.rs @@ -0,0 +1,20 @@ +use crate::UpdateError; +use anyhow::bail; +use std::{ + io::Cursor, + path::{Path, PathBuf}, +}; + +const UNKNOWN_PLATFORM_ERR_MSG: &str = "Unknown platform"; + +pub fn open_base_lib(_app_dir: &Path, _lib_name: &str) -> anyhow::Result>> { + bail!(UNKNOWN_PLATFORM_ERR_MSG) +} + +pub fn libapp_path_from_settings( + _original_libapp_paths: &[String], +) -> Result { + Err(UpdateError::InvalidState( + UNKNOWN_PLATFORM_ERR_MSG.to_string(), + )) +} diff --git a/library/src/updater.rs b/library/src/updater.rs index 8dc1b02..0251b61 100644 --- a/library/src/updater.rs +++ b/library/src/updater.rs @@ -1,8 +1,7 @@ // This file's job is to be the Rust API for the updater. use std::fmt::{Display, Formatter}; -use std::fs; -#[cfg(any(target_os = "android", test))] +use std::fs::File; use std::io::{Read, Seek}; use std::path::{Path, PathBuf}; @@ -16,6 +15,7 @@ use crate::logging::init_logging; use crate::network::{ download_to_path, send_patch_check_request, NetworkHooks, PatchCheckResponse, }; +use crate::platform::{libapp_path_from_settings, open_base_lib}; use crate::updater_lock::{with_updater_thread_lock, UpdaterLockState}; use crate::yaml::YamlConfig; @@ -84,29 +84,12 @@ pub struct AppConfig { pub original_libapp_paths: Vec, } -// On Android we don't use a direct path to libapp.so, but rather a data dir -// and a hard-coded name for the libapp file which we look up in the -// split APKs in that datadir. On other platforms we just use a path. -#[cfg(not(any(target_os = "android", test)))] -fn libapp_path_from_settings(original_libapp_paths: &[String]) -> Result { - let first = original_libapp_paths - .first() - .ok_or(UpdateError::InvalidArgument( - "original_libapp_paths".to_string(), - "empty".to_string(), - )); - first.map(PathBuf::from) -} - /// Initialize the updater library. /// Takes a `AppConfig` struct and a yaml string. /// The yaml string is the contents of the `shorebird.yaml` file. /// The `AppConfig` struct is information about the running app and where /// the updater should keep its cache. pub fn init(app_config: AppConfig, yaml: &str) -> Result<(), UpdateError> { - #[cfg(any(target_os = "android", test))] - use crate::android::libapp_path_from_settings; - init_logging(); let config = YamlConfig::from_yaml(yaml) .map_err(|err| UpdateError::InvalidArgument("yaml".to_string(), err.to_string()))?; @@ -144,7 +127,7 @@ fn check_hash(path: &Path, expected_string: &str) -> anyhow::Result<()> { // Based on guidance from: // - let mut file = fs::File::open(path)?; + let mut file = File::open(path)?; let mut hasher = Sha256::new(); std::io::copy(&mut file, &mut hasher)?; // Check that the length from copy is the same as the file size? @@ -169,32 +152,17 @@ fn check_hash(path: &Path, expected_string: &str) -> anyhow::Result<()> { Ok(()) } -// This is just a place to put our terrible android hacks. -// And also avoid (for now) dealing with inflating patches on iOS. -#[cfg(any(target_os = "android", test))] fn prepare_for_install( - config: &UpdateConfig, + app_dir: &PathBuf, download_path: &Path, output_path: &Path, ) -> anyhow::Result<()> { // We abuse `libapp_path` to actually be the path to the data dir for now. // This is an abuse because the variable name is `libapp_path`, but // we're making it point to a the `app_data` directory instead. - let app_dir = &config.libapp_path; debug!("app_dir: {:?}", app_dir); - let base_r = crate::android::open_base_lib(app_dir, "libapp.so")?; - inflate(download_path, base_r, output_path) -} - -#[cfg(not(any(target_os = "android", test)))] -fn prepare_for_install( - _config: &UpdateConfig, - download_path: &Path, - output_path: &Path, -) -> anyhow::Result<()> { - // On iOS we don't yet support compressed patches, just copy the file. - fs::copy(download_path, output_path)?; - Ok(()) + let base_release_artifact = open_base_lib(app_dir, "libapp.so")?; + inflate(download_path, base_release_artifact, output_path) } fn copy_update_config() -> anyhow::Result { @@ -265,8 +233,7 @@ fn update_internal(_: &UpdaterLockState) -> anyhow::Result { download_to_path(&config.network_hooks, &patch.download_url, &download_path)?; let output_path = download_dir.join(format!("{}.full", patch.number)); - // Should not pass config, rather should read necessary information earlier. - prepare_for_install(&config, &download_path, &output_path)?; + prepare_for_install(&config.libapp_path, &download_path, &output_path)?; // Check the hash before moving into place. check_hash(&output_path, &patch.hash).context(format!( @@ -302,7 +269,6 @@ pub fn update() -> anyhow::Result { /// Given a path to a patch file, and a base file, apply the patch to the base /// and write the result to the output path. -#[cfg(any(target_os = "android", test))] fn inflate(patch_path: &Path, base_r: RS, output_path: &Path) -> anyhow::Result<()> where RS: Read + Seek, @@ -316,10 +282,9 @@ where // PipeReader/Writer errors instead of file open errors. debug!("Reading patch file: {:?}", patch_path); let compressed_patch_r = BufReader::new( - fs::File::open(patch_path) - .context(format!("Failed to open patch file: {:?}", patch_path))?, + File::open(patch_path).context(format!("Failed to open patch file: {:?}", patch_path))?, ); - let output_file_w = fs::File::create(output_path)?; + let output_file_w = File::create(output_path)?; // Set up a pipe to connect the writing from the decompression thread // to the reading of the decompressed patch data on this thread.