Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Simplify version check in ping-pong #149

Merged
merged 1 commit into from
Jan 29, 2024
Merged

refactor: Simplify version check in ping-pong #149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 11 additions & 18 deletions extension/background.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
38 changes: 9 additions & 29 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
}
Expand Down Expand Up @@ -235,7 +234,12 @@ mod tests {
let _guard = WRITE_MESSAGE_CONTEXT_LOCK.lock().unwrap();
let ctx = MockTr::write_message_context();
ctx.expect::<Ping>()
.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::<MockTr>(ping);
ctx.checkpoint();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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::<Ping>()
.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::<MockTr>(ping);
ctx.checkpoint();
}

#[test]
fn echo_compose_test() {
let mut compose = get_blank_compose();
Expand Down
15 changes: 0 additions & 15 deletions src/model/messaging.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
Loading