Skip to content

Commit

Permalink
feat: add timestamp to patch events (#179)
Browse files Browse the repository at this point in the history
* feat: add timestamp to patch events

* fix test
  • Loading branch information
bryanoltman committed Jun 20, 2024
1 parent 349f423 commit 3da6c38
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 3 deletions.
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions library/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ oslog = "0.2.0"
[dev-dependencies]
mockall = "0.12.1"
mockito = "1.2.0"
mock_instant = "0.5.1"
# Gives #[serial] attribute for locking all of our shorebird_init
# tests to a single thread so they don't conflict with each other.
serial_test = "2.0.0"
Expand Down
3 changes: 3 additions & 0 deletions library/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,7 @@ pub struct PatchEvent {

/// The release version from AndroidManifest.xml, Info.plist in the app.
pub release_version: String,

/// When this event occurred as a Unix epoch timestamp in seconds.
pub timestamp: u64,
}
1 change: 1 addition & 0 deletions library/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ mod config;
mod events;
mod logging;
mod network;
mod time;
mod updater;
mod updater_lock;
mod yaml;
Expand Down
7 changes: 5 additions & 2 deletions library/src/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ pub fn download_to_path(

#[cfg(test)]
mod tests {
use crate::network::PatchCheckResponse;
use crate::{network::PatchCheckResponse, time};

use super::{patches_events_url, PatchEvent};
use crate::events::EventType;
Expand Down Expand Up @@ -295,12 +295,13 @@ mod tests {
platform: "platform".to_string(),
release_version: "release_version".to_string(),
identifier: EventType::PatchInstallSuccess,
timestamp: 1234,
};
let request = super::CreatePatchEventRequest { event };
let json_string = serde_json::to_string(&request).unwrap();
assert_eq!(
json_string,
r#"{"event":{"app_id":"app_id","arch":"arch","type":"__patch_install__","patch_number":1,"platform":"platform","release_version":"release_version"}}"#
r#"{"event":{"app_id":"app_id","arch":"arch","type":"__patch_install__","patch_number":1,"platform":"platform","release_version":"release_version","timestamp":1234}}"#
)
}

Expand Down Expand Up @@ -375,6 +376,7 @@ mod tests {
platform: "platform".to_string(),
release_version: "release_version".to_string(),
identifier: EventType::PatchInstallSuccess,
timestamp: time::unix_timestamp(),
};
let result = super::report_event_default(
// Make the request to a non-existent URL, which will trigger the
Expand Down Expand Up @@ -402,6 +404,7 @@ mod tests {
platform: "platform".to_string(),
release_version: "release_version".to_string(),
identifier: EventType::PatchInstallSuccess,
timestamp: time::unix_timestamp(),
},
},
);
Expand Down
32 changes: 32 additions & 0 deletions library/src/time.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#[cfg(test)]
use mock_instant::global::SystemTime;

#[cfg(not(test))]
use std::time::SystemTime;

/// The number of seconds since the Unix epoch. Returns 0 if the system clock is set before the
/// Unix epoch.
pub(crate) fn unix_timestamp() -> u64 {
match SystemTime::now().duration_since(SystemTime::UNIX_EPOCH) {
Ok(n) => n.as_secs(),
Err(_) => 0,
}
}

#[cfg(test)]
mod tests {
use std::time::Duration;

use mock_instant::global::MockClock;

#[test]
fn returns_duration_since_unix_epoch() {
MockClock::set_system_time(Duration::from_secs(123));
assert_eq!(super::unix_timestamp(), 123);
}

// Ideally, we'd be able to test the case where `duration_since` returns an error, but it
// seems to only happen when system time is set before the Unix epoch, which is not possible
// with the current implementation of `MockClock` because `set_system_time` expects a duration,
// and a duration cannot be negative.
}
6 changes: 5 additions & 1 deletion library/src/updater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use crate::network::{
download_to_path, patches_check_url, send_patch_check_request, NetworkHooks, PatchCheckRequest,
PatchCheckResponse,
};
use crate::time;
use crate::updater_lock::{with_updater_thread_lock, UpdaterLockState};
use crate::yaml::YamlConfig;

Expand Down Expand Up @@ -477,6 +478,7 @@ pub fn report_launch_failure() -> anyhow::Result<()> {
patch_number: patch.number,
platform: current_platform().to_string(),
release_version: config.release_version.clone(),
timestamp: time::unix_timestamp(),
};
// Queue the failure event for later sending since right after this
// function returns the Flutter engine is likely to abort().
Expand Down Expand Up @@ -524,6 +526,7 @@ pub fn report_launch_success() -> anyhow::Result<()> {
platform: current_platform().to_string(),
release_version: config_copy.release_version.clone(),
identifier: EventType::PatchInstallSuccess,
timestamp: time::unix_timestamp(),
};
let report_result = crate::network::send_patch_event(event, &config_copy);
if let Err(err) = report_result {
Expand Down Expand Up @@ -561,7 +564,7 @@ mod tests {
use crate::{
config::{testing_reset_config, with_config},
network::{testing_set_network_hooks, NetworkHooks, PatchCheckResponse},
ExternalFileProvider,
time, ExternalFileProvider,
};

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -867,6 +870,7 @@ mod tests {
patch_number: 1,
platform: current_platform().to_string(),
release_version: config.release_version.clone(),
timestamp: time::unix_timestamp(),
};
// Queue 5 events.
assert!(state.queue_event(fail_event.clone()).is_ok());
Expand Down

0 comments on commit 3da6c38

Please sign in to comment.