From 139c9912d5a0879d1c1db912dcfee822a3f81c80 Mon Sep 17 00:00:00 2001 From: Frederick Zhang Date: Mon, 29 Jan 2024 20:50:06 +1100 Subject: [PATCH] refactor: Simplify version check in ping-pong Since I realised that I had to check bypassVersionCheck in the extension anyway, it's cleaner to just remove it from the host. --- extension/background.js | 29 +++++++++++------------------ src/main.rs | 38 +++++++++----------------------------- src/model/messaging.rs | 15 --------------- 3 files changed, 20 insertions(+), 62 deletions(-) diff --git a/extension/background.js b/extension/background.js index fc08123..c0ef9ef 100644 --- a/extension/background.js +++ b/extension/background.js @@ -125,11 +125,9 @@ async function composeActionListener(tab, info) { } async function nativeMessagingPing() { - const settings = await browser.storage.local.get(['bypassVersionCheck']) await browser.storage.local.remove(['healthy']) const request = { ping: Date.now(), - bypassVersionCheck: !!settings.bypassVersionCheck, version, } console.debug(`${manifest.short_name} sending: `, request) @@ -143,23 +141,18 @@ async function nativeMessagingListener(response) { await browser.storage.local.set({ healthy: response.ping === response.pong, }) - if (!response.compatible) { - // unfortunately we have to check the setting again on top of response.compatible. - // when this is released, we want users to see it ASAP, so while response.compatible already takes bypassVersionCheck into account, - // the extension still needs to act according to the setting even if response.compatible is missing. - const settings = await browser.storage.local.get(['bypassVersionCheck']) - if (!settings.bypassVersionCheck) { - let message = '' - if (response.hostVersion) { - message = `Extension version is ${version} while host version is ${response.hostVersion}.\n` - } - message += 'Click here to download the compatible messaging host. (Please restart Thunderbird manually afterwards.)' - createBasicNotification( - 'download-host', - `${manifest.short_name} messaging host may be incompatible!`, - message - ) + const settings = await browser.storage.local.get(['bypassVersionCheck']) + if (!response.compatible && !settings.bypassVersionCheck) { + let message = '' + if (response.hostVersion) { + message = `Extension version is ${version} while host version is ${response.hostVersion}.\n` } + message += 'Click here to download the compatible messaging host. (Please restart Thunderbird manually afterwards.)' + createBasicNotification( + 'download-host', + `${manifest.short_name} messaging host may be incompatible!`, + message + ) } } else if (response.title && response.message) { await createBasicNotification('', response.title, response.message) diff --git a/src/main.rs b/src/main.rs index c0aac2e..0057ccd 100644 --- a/src/main.rs +++ b/src/main.rs @@ -22,8 +22,7 @@ where { request.pong = request.ping; request.host_version = env!("CARGO_PKG_VERSION").to_string(); - request.compatible = request.bypass_version_check - || util::is_extension_compatible(env!("CARGO_PKG_VERSION"), &request.version); + request.compatible = util::is_extension_compatible(env!("CARGO_PKG_VERSION"), &request.version); if let Err(write_error) = T::write_message(&request) { eprintln!("ExtEditorR failed to send response to Thunderbird: {write_error}"); } @@ -235,7 +234,12 @@ mod tests { let _guard = WRITE_MESSAGE_CONTEXT_LOCK.lock().unwrap(); let ctx = MockTr::write_message_context(); ctx.expect::() - .withf(|p: &Ping| p.ping == 123456 && p.pong == 123456 && p.compatible) + .withf(|p: &Ping| { + p.ping == 123456 + && p.pong == 123456 + && !p.compatible + && p.host_version == env!("CARGO_PKG_VERSION") + }) .returning(|&_| Ok(())); handle_ping::(ping); ctx.checkpoint(); @@ -244,10 +248,7 @@ mod tests { #[test] fn ping_pong_successful_version_check_test() { let host_version = env!("CARGO_PKG_VERSION"); - let ping_json = format!( - r#"{{"ping": 123456, "bypassVersionCheck": false, "version": "{}"}}"#, - host_version - ); + let ping_json = format!(r#"{{"ping": 123456, "version": "{}"}}"#, host_version); let ping: Ping = serde_json::from_str(&ping_json).unwrap(); let _guard = WRITE_MESSAGE_CONTEXT_LOCK.lock().unwrap(); @@ -268,7 +269,7 @@ mod tests { #[test] fn ping_pong_failed_version_check_test() { let host_version = env!("CARGO_PKG_VERSION"); - let ping_json = r#"{"ping": 123456, "bypassVersionCheck": false, "version": "0.0.0.0"}"#; + let ping_json = r#"{"ping": 123456, "version": "0.0.0.0"}"#; let ping: Ping = serde_json::from_str(ping_json).unwrap(); let _guard = WRITE_MESSAGE_CONTEXT_LOCK.lock().unwrap(); @@ -286,27 +287,6 @@ mod tests { ctx.checkpoint(); } - #[test] - fn ping_pong_bypass_failed_version_check_test() { - let host_version = env!("CARGO_PKG_VERSION"); - let ping_json = r#"{"ping": 123456, "bypassVersionCheck": true, "version": "0.0.0.0"}"#; - let ping: Ping = serde_json::from_str(ping_json).unwrap(); - - let _guard = WRITE_MESSAGE_CONTEXT_LOCK.lock().unwrap(); - let ctx = MockTr::write_message_context(); - ctx.expect::() - .withf(|p: &Ping| { - let host_version = host_version.to_string(); - p.ping == 123456 - && p.pong == 123456 - && p.compatible - && p.host_version == host_version - }) - .returning(|&_| Ok(())); - handle_ping::(ping); - ctx.checkpoint(); - } - #[test] fn echo_compose_test() { let mut compose = get_blank_compose(); diff --git a/src/model/messaging.rs b/src/model/messaging.rs index 471a62a..e63ecd5 100644 --- a/src/model/messaging.rs +++ b/src/model/messaging.rs @@ -58,10 +58,6 @@ pub struct Ping { pub ping: u64, #[serde(default)] pub pong: u64, - // don't run this check as part of ping-pong if the extension is somehow older which does not - // send this attribute - #[serde(default = "default_as_true")] - pub bypass_version_check: bool, #[serde(default)] pub version: String, #[serde(default)] @@ -70,10 +66,6 @@ pub struct Ping { pub compatible: bool, } -fn default_as_true() -> bool { - true -} - #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct Configuration { @@ -1228,13 +1220,6 @@ pub mod tests { refute_contains!(output, "X-ExtEditorR-Help"); } - #[test] - fn default_ping_bypass_version_check_test() { - let ping_json = r#"{"ping": 123456}"#; - let ping: Ping = serde_json::from_str(ping_json).unwrap(); - assert!(ping.bypass_version_check); - } - fn to_eml_and_assert(compose: &Compose) -> String { let mut buf = Vec::new(); let result = compose.to_eml(&mut buf);