Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,6 @@ open class Nimbus(
os = "Android",
osVersion = Build.VERSION.RELEASE,
installationDate = packageInfo?.firstInstallTime,
homeDirectory = context.applicationInfo?.dataDir,
customTargetingAttributes = appInfo.customTargetingAttributes,
)
}
Expand Down
1 change: 0 additions & 1 deletion components/nimbus/src/nimbus.udl
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be a breaking change against iOS. Please do the following:

once thats complete, you can check the "this is a breaking change" checkbox in the PR description

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay added changelog, ios PR and added ci full to trigger full integration run

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JsonObject? custom_targeting_attributes;
};

Expand Down
2 changes: 0 additions & 2 deletions components/nimbus/src/stateful/matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -51,7 +50,6 @@ pub struct AppContext {
pub android_sdk_version: Option<String>,
pub debug_tag: Option<String>,
pub installation_date: Option<i64>,
pub home_directory: Option<String>,
#[serde(flatten)]
pub custom_targeting_attributes: Option<Map<String, Value>>,
}
43 changes: 2 additions & 41 deletions components/nimbus/src/stateful/nimbus_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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()
},
Expand Down Expand Up @@ -553,34 +542,6 @@ impl NimbusClient {
)
}

#[cfg(not(test))]
fn get_creation_date_from_path<P: AsRef<Path>>(&self, path: P) -> Result<DateTime<Utc>> {
info!("[Nimbus] Getting creation date from path");
let metadata = std::fs::metadata(path)?;
let system_time_created = metadata.created()?;
let date_time_created = DateTime::<Utc>::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<P: AsRef<Path>>(&self, path: P) -> Result<DateTime<Utc>> {
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::<DateTime<Utc>>(&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()?;
Expand Down
49 changes: 11 additions & 38 deletions components/nimbus/src/tests/stateful/test_nimbus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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(),
Expand All @@ -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)));
Expand All @@ -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(
Expand Down Expand Up @@ -934,25 +926,6 @@ fn event_store_on_targeting_attributes_is_updated_after_an_event_is_recorded() -
Ok(())
}

fn set_test_creation_date<P: AsRef<Path>>(date: DateTime<Utc>, 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<P: AsRef<Path>>(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<()> {
Expand Down