From 69867a420c6ebe1ca01bb5f25db12afa695ced26 Mon Sep 17 00:00:00 2001 From: yashikakhurana Date: Mon, 17 Nov 2025 10:16:22 -0800 Subject: [PATCH 1/2] Bug 2000264: Remove passing the home directory to the Rust SDK --- .../org/mozilla/experiments/nimbus/Nimbus.kt | 1 - components/nimbus/src/nimbus.udl | 1 - components/nimbus/src/stateful/matcher.rs | 2 - .../nimbus/src/stateful/nimbus_client.rs | 43 +--------------- .../nimbus/src/tests/stateful/test_nimbus.rs | 49 +++++-------------- 5 files changed, 13 insertions(+), 83 deletions(-) diff --git a/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt b/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt index a091371e1c..0fed3ef9a0 100644 --- a/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt +++ b/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt @@ -623,7 +623,6 @@ open class Nimbus( os = "Android", osVersion = Build.VERSION.RELEASE, installationDate = packageInfo?.firstInstallTime, - homeDirectory = context.applicationInfo?.dataDir, customTargetingAttributes = appInfo.customTargetingAttributes, ) } diff --git a/components/nimbus/src/nimbus.udl b/components/nimbus/src/nimbus.udl index 2ac8b7cf9a..0b0cc1567e 100644 --- a/components/nimbus/src/nimbus.udl +++ b/components/nimbus/src/nimbus.udl @@ -44,7 +44,6 @@ dictionary AppContext { // Note that installation date is // the unix time, which is milliseconds since epoch i64? installation_date; - string? home_directory; JsonObject? custom_targeting_attributes; }; diff --git a/components/nimbus/src/stateful/matcher.rs b/components/nimbus/src/stateful/matcher.rs index 3259c366d0..d170e08295 100644 --- a/components/nimbus/src/stateful/matcher.rs +++ b/components/nimbus/src/stateful/matcher.rs @@ -33,7 +33,6 @@ use serde_json::{Map, Value}; /// - `android_sdk_version`: Android specific for targeting specific sdk versions /// - `debug_tag`: Used for debug purposes as a way to match only developer builds, etc. /// - `installation_date`: The date the application installed the app -/// - `home_directory`: The application's home directory /// - `custom_targeting_attributes`: Contains attributes specific to the application, derived by the application #[derive(Deserialize, Serialize, Debug, Clone, Default)] pub struct AppContext { @@ -51,7 +50,6 @@ pub struct AppContext { pub android_sdk_version: Option, pub debug_tag: Option, pub installation_date: Option, - pub home_directory: Option, #[serde(flatten)] pub custom_targeting_attributes: Option>, } diff --git a/components/nimbus/src/stateful/nimbus_client.rs b/components/nimbus/src/stateful/nimbus_client.rs index 7b955ba654..85edcfce85 100644 --- a/components/nimbus/src/stateful/nimbus_client.rs +++ b/components/nimbus/src/stateful/nimbus_client.rs @@ -10,7 +10,7 @@ use crate::{ EnrolledFeature, EnrollmentChangeEvent, EnrollmentChangeEventType, EnrollmentsEvolver, ExperimentEnrollment, }, - error::{info, warn, BehaviorError}, + error::{info, BehaviorError}, evaluator::{ get_calculated_attributes, is_experiment_available, CalculatedAttributes, ExperimentAvailable, TargetingAttributes, @@ -49,7 +49,7 @@ use remote_settings::RemoteSettingsService; use serde_json::Value; use std::collections::HashSet; use std::fmt::Debug; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::sync::{Arc, Mutex, MutexGuard}; use uuid::Uuid; @@ -501,17 +501,6 @@ impl NimbusClient { Ok( if let Some(installation_date) = persisted_installation_date { installation_date - } else if let Some(home_directory) = &self.app_context.home_directory { - let installation_date = match self.get_creation_date_from_path(home_directory) { - Ok(installation_date) => installation_date, - Err(e) => { - warn!("[Nimbus] Unable to get installation date from path, defaulting to today: {:?}", e); - Utc::now() - } - }; - let store = db.get_store(StoreId::Meta); - store.put(writer, DB_KEY_INSTALLATION_DATE, &installation_date)?; - installation_date } else { Utc::now() }, @@ -553,34 +542,6 @@ impl NimbusClient { ) } - #[cfg(not(test))] - fn get_creation_date_from_path>(&self, path: P) -> Result> { - info!("[Nimbus] Getting creation date from path"); - let metadata = std::fs::metadata(path)?; - let system_time_created = metadata.created()?; - let date_time_created = DateTime::::from(system_time_created); - info!( - "[Nimbus] Creation date retrieved form path successfully: {}", - date_time_created - ); - Ok(date_time_created) - } - - #[cfg(test)] - fn get_creation_date_from_path>(&self, path: P) -> Result> { - use std::io::Read; - let test_path = path.as_ref().with_file_name("test.json"); - let mut file = std::fs::File::open(test_path)?; - let mut buf = String::new(); - file.read_to_string(&mut buf)?; - - let res = match serde_json::from_str::>(&buf) { - Ok(v) => v, - Err(e) => return Err(NimbusError::JSONError("res = nimbus::stateful::nimbus_client::get_creation_date_from_path::serde_json::from_str".into(), e.to_string())) - }; - Ok(res) - } - pub fn set_experiments_locally(&self, experiments_json: String) -> Result<()> { let new_experiments = parse_experiments(&experiments_json)?; let db = self.db()?; diff --git a/components/nimbus/src/tests/stateful/test_nimbus.rs b/components/nimbus/src/tests/stateful/test_nimbus.rs index 4f4a09b665..cb9c17d1b3 100644 --- a/components/nimbus/src/tests/stateful/test_nimbus.rs +++ b/components/nimbus/src/tests/stateful/test_nimbus.rs @@ -27,9 +27,8 @@ use crate::{ use chrono::{DateTime, Duration, Utc}; use serde_json::json; use std::collections::HashMap; -use std::path::Path; +use std::str::FromStr; use std::sync::Arc; -use std::{io::Write, str::FromStr}; use tempfile::TempDir; use uuid::Uuid; @@ -132,7 +131,6 @@ fn test_installation_date() -> Result<()> { let time_stamp = three_days_ago.timestamp_millis(); let mut app_context = AppContext { installation_date: Some(time_stamp), - home_directory: Some(tmp_dir.path().to_str().unwrap().to_string()), ..Default::default() }; let client = NimbusClient::new( @@ -163,9 +161,8 @@ fn test_installation_date() -> Result<()> { store.clear(&mut writer)?; writer.commit()?; - // Step 2: We test that we will fallback to the - // filesystem, and if that fails we - // set Today's date. + // Step 2: We test that when no installation_date is provided in context + // and no persisted date exists, we set Today's date. // We recreate our client to make sure // we wipe any non-persistent memory @@ -182,9 +179,8 @@ fn test_installation_date() -> Result<()> { None, None, )?; - delete_test_creation_date(tmp_dir.path()).ok(); - // When we check the filesystem, we will fail. We haven't `set_test_creation_date` - // yet. + // Since no installation_date is in context and storage is cleared, + // we will default to today's date client.initialize()?; client.apply_pending_experiments()?; // We verify that it's today. @@ -207,15 +203,12 @@ fn test_installation_date() -> Result<()> { None, )?; client.initialize()?; - // We now store a date for days ago in our file system - // this shouldn't change the installation date for the nimbus client - // since client already persisted the date seen earlier. - let four_days_ago = Utc::now() - Duration::days(4); - set_test_creation_date(four_days_ago, tmp_dir.path())?; + // Since we already persisted the date from the previous run, + // we should still get 0 days_since_install client.apply_pending_experiments()?; let targeting_attributes = client.get_targeting_attributes(); // We will **STILL** get a 0 `days_since_install` since we persisted the value - // we got on the previous run, therefore we did not check the file system. + // we got on the previous run. assert!(matches!(targeting_attributes.days_since_install, Some(0))); // We now clear the persisted storage @@ -227,7 +220,9 @@ fn test_installation_date() -> Result<()> { store.clear(&mut writer)?; writer.commit()?; - // Step 4: We test that if the storage is clear, we will fallback to the + // Step 4: Test with 4 days since installation + let four_days_ago = Utc::now() - Duration::days(4); + app_context.installation_date = Some(four_days_ago.timestamp_millis()); let client = NimbusClient::new( app_context, Default::default(), @@ -239,8 +234,6 @@ fn test_installation_date() -> Result<()> { None, )?; client.initialize()?; - // now that the store is clear, we will fallback again to the - // file system, and retrieve the four_days_ago number we stored earlier client.apply_pending_experiments()?; let targeting_attributes = client.get_targeting_attributes(); assert!(matches!(targeting_attributes.days_since_install, Some(4))); @@ -258,7 +251,6 @@ fn test_days_since_calculation_happens_at_startup() -> Result<()> { let time_stamp = three_days_ago.timestamp_millis(); let app_context = AppContext { installation_date: Some(time_stamp), - home_directory: Some(tmp_dir.path().to_str().unwrap().to_string()), ..Default::default() }; let client = NimbusClient::new( @@ -934,25 +926,6 @@ fn event_store_on_targeting_attributes_is_updated_after_an_event_is_recorded() - Ok(()) } -fn set_test_creation_date>(date: DateTime, path: P) -> Result<()> { - use std::fs::OpenOptions; - let test_path = path.as_ref().with_file_name("test.json"); - let mut file = OpenOptions::new() - .write(true) - .create(true) - .truncate(true) - .open(test_path) - .unwrap(); - file.write_all(serde_json::to_string(&date).unwrap().as_bytes())?; - Ok(()) -} - -fn delete_test_creation_date>(path: P) -> Result<()> { - let test_path = path.as_ref().with_file_name("test.json"); - std::fs::remove_file(test_path)?; - Ok(()) -} - #[cfg(feature = "stateful")] #[test] fn test_ios_rollout() -> Result<()> { From e37eb4c5b4ecdef2e57716245d24a1d1ff472587 Mon Sep 17 00:00:00 2001 From: yashikakhurana Date: Wed, 19 Nov 2025 18:00:37 -0800 Subject: [PATCH 2/2] Bug 2000264: Add changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6ce63ddb5..7916ce873b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ [Full Changelog](In progress) +### Nimbus + +### ⚠️ Breaking Changes ⚠️ +- Removed unused `home_directory` field from AppContext. Both Kotlin and Swift sides were passing null values and it wasn't used anywhere. ([#7085](https://github.com/mozilla/application-services/pull/7085)) ([#30782](https://github.com/mozilla-mobile/firefox-ios/pull/30782)) + ### `rc_crypto` - Thread-safety improvements for PKCS-token-dependent methods by introducing a global mutex. Refactored key unpacking logic and removed redundant code;