Skip to content

Commit 4550da2

Browse files
committed
feat(ads-client): instantiate nimbus client directly inside ads-client
1 parent b241e8b commit 4550da2

5 files changed

Lines changed: 119 additions & 100 deletions

File tree

components/ads-client/android/build.gradle

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ android {
3838
}
3939

4040
dependencies {
41-
api project(":nimbus")
42-
4341
implementation libs.mozilla.glean
4442

4543
testImplementation libs.mozilla.glean.forUnitTests

components/ads-client/src/client.rs

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use crate::client::ad_response::{
1111
};
1212
use crate::client::config::AdsClientConfig;
1313
use crate::error::{RecordClickError, RecordImpressionError, ReportAdError, RequestAdsError};
14-
use crate::experiments::is_http_cache_enabled;
14+
use crate::experiments::ExperimentClient;
1515
use crate::http_cache::{HttpCache, RequestCachePolicy};
1616
use crate::mars::MARSClient;
1717
use crate::telemetry::Telemetry;
@@ -35,6 +35,8 @@ where
3535
{
3636
client: MARSClient<T>,
3737
context_id_component: ContextIDComponent,
38+
#[allow(dead_code)]
39+
experiment_client: Option<ExperimentClient>,
3840
telemetry: T,
3941
}
4042

@@ -52,21 +54,31 @@ where
5254
);
5355
let telemetry = client_config.telemetry;
5456

57+
// Create ExperimentClient using the parent directory of the cache db
58+
let experiment_client = client_config
59+
.cache_config
60+
.as_ref()
61+
.and_then(|cfg| std::path::Path::new(&cfg.db_path).parent()) // Perhaps we should have a data_dir param to store all the ads-client related data
62+
.and_then(|p| p.to_str())
63+
.and_then(ExperimentClient::new);
64+
65+
let cache_enabled = experiment_client
66+
.as_ref()
67+
.map(|c| c.is_http_cache_enabled())
68+
.unwrap_or(true);
69+
5570
// Configure the cache if a path is provided.
5671
// Nimbus experiment can disable the cache entirely.
57-
if let Some(cache_cfg) = client_config.cache_config {
58-
// Check if cache is disabled by Nimbus experiment
59-
let cache_enabled = is_http_cache_enabled(client_config.nimbus.clone());
60-
61-
let default_cache_ttl = Duration::from_secs(
62-
cache_cfg
63-
.default_cache_ttl_seconds
64-
.unwrap_or(DEFAULT_TTL_SECONDS),
65-
);
66-
let max_cache_size =
67-
ByteSize::mib(cache_cfg.max_size_mib.unwrap_or(DEFAULT_MAX_CACHE_SIZE_MIB));
68-
69-
let http_cache = if cache_enabled {
72+
let http_cache = if let Some(cache_cfg) = client_config.cache_config {
73+
if cache_enabled {
74+
let default_cache_ttl = Duration::from_secs(
75+
cache_cfg
76+
.default_cache_ttl_seconds
77+
.unwrap_or(DEFAULT_TTL_SECONDS),
78+
);
79+
let max_cache_size =
80+
ByteSize::mib(cache_cfg.max_size_mib.unwrap_or(DEFAULT_MAX_CACHE_SIZE_MIB));
81+
7082
match HttpCache::builder(cache_cfg.db_path)
7183
.max_size(max_cache_size)
7284
.default_ttl(default_cache_ttl)
@@ -80,23 +92,17 @@ where
8092
}
8193
} else {
8294
None
83-
};
84-
85-
let client = MARSClient::new(client_config.environment, http_cache, telemetry.clone());
86-
let client = Self {
87-
context_id_component,
88-
client,
89-
telemetry: telemetry.clone(),
90-
};
91-
telemetry.record(&ClientOperationEvent::New);
92-
return client;
93-
}
95+
}
96+
} else {
97+
None
98+
};
9499

95-
let client = MARSClient::new(client_config.environment, None, telemetry.clone());
100+
let client = MARSClient::new(client_config.environment, http_cache, telemetry.clone());
96101
let client = Self {
97102
context_id_component,
98103
client,
99104
telemetry: telemetry.clone(),
105+
experiment_client,
100106
};
101107
telemetry.record(&ClientOperationEvent::New);
102108
client
@@ -258,6 +264,7 @@ mod tests {
258264
context_id_component,
259265
client,
260266
telemetry: MozAdsTelemetryWrapper::noop(),
267+
experiment_client: None,
261268
}
262269
}
263270

@@ -267,7 +274,6 @@ mod tests {
267274
environment: Environment::Test,
268275
cache_config: None,
269276
telemetry: MozAdsTelemetryWrapper::noop(),
270-
nimbus: None,
271277
};
272278
let client = AdsClient::new(config);
273279
let context_id = client.get_context_id().unwrap();
@@ -280,7 +286,6 @@ mod tests {
280286
environment: Environment::Test,
281287
cache_config: None,
282288
telemetry: MozAdsTelemetryWrapper::noop(),
283-
nimbus: None,
284289
};
285290
let mut client = AdsClient::new(config);
286291
let old_id = client.get_context_id().unwrap();

components/ads-client/src/client/config.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
44
*/
55

6-
use nimbus::NimbusClient;
76
use once_cell::sync::Lazy;
8-
use std::sync::Arc;
97
use url::Url;
108

119
use crate::telemetry::Telemetry;
@@ -23,7 +21,6 @@ where
2321
pub environment: Environment,
2422
pub cache_config: Option<AdsCacheConfig>,
2523
pub telemetry: T,
26-
pub nimbus: Option<Arc<NimbusClient>>,
2724
}
2825

2926
#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]

components/ads-client/src/experiments.rs

Lines changed: 86 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -5,95 +5,117 @@
55

66
//! Nimbus experiment integration for ads-client.
77
8-
use nimbus::NimbusClient;
8+
use nimbus::{AppContext, NimbusClient};
9+
use serde_json::json;
910
use std::sync::Arc;
1011

1112
const FEATURE_ID: &str = "ads-client";
1213

13-
/// Query NimbusClient to check if HTTP cache is enabled for this user.
14-
/// Returns true (cache enabled) by default if no NimbusClient or no experiment is active.
15-
pub fn is_http_cache_enabled(nimbus: Option<Arc<NimbusClient>>) -> bool {
16-
let Some(nimbus) = nimbus else {
17-
return true; // Default: cache enabled
18-
};
19-
20-
let Ok(Some(json)) = nimbus.get_feature_config_variables(FEATURE_ID.to_string()) else {
21-
return true; // Default: cache enabled
22-
};
23-
24-
serde_json::from_str::<serde_json::Value>(&json)
25-
.ok()
26-
.and_then(|v| v.get("http-cache-enabled")?.as_bool())
27-
.unwrap_or(true)
14+
struct NoopMetricsHandler;
15+
impl nimbus::metrics::MetricsHandler for NoopMetricsHandler {
16+
fn record_enrollment_statuses(&self, _: Vec<nimbus::metrics::EnrollmentStatusExtraDef>) {}
17+
fn record_feature_activation(&self, _: nimbus::metrics::FeatureExposureExtraDef) {}
18+
fn record_feature_exposure(&self, _: nimbus::metrics::FeatureExposureExtraDef) {}
19+
fn record_malformed_feature_config(&self, _: nimbus::metrics::MalformedFeatureConfigExtraDef) {}
2820
}
2921

30-
#[cfg(test)]
31-
mod tests {
32-
use super::*;
33-
use nimbus::AppContext;
34-
use serde_json::json;
35-
use uuid::Uuid;
36-
37-
struct NoopMetricsHandler;
38-
impl nimbus::metrics::MetricsHandler for NoopMetricsHandler {
39-
fn record_enrollment_statuses(&self, _: Vec<nimbus::metrics::EnrollmentStatusExtraDef>) {}
40-
fn record_feature_activation(&self, _: nimbus::metrics::FeatureExposureExtraDef) {}
41-
fn record_feature_exposure(&self, _: nimbus::metrics::FeatureExposureExtraDef) {}
42-
fn record_malformed_feature_config(
43-
&self,
44-
_: nimbus::metrics::MalformedFeatureConfigExtraDef,
45-
) {
46-
}
47-
}
22+
/// Internal experiment client that manages a NimbusClient with hardcoded experiment.
23+
pub struct ExperimentClient {
24+
nimbus: Arc<NimbusClient>,
25+
}
4826

49-
#[test]
50-
fn test_fifty_fifty_experiment() {
27+
impl ExperimentClient {
28+
/// Create a new ExperimentClient with hardcoded 50/50 experiment.
29+
pub fn new(db_path: &str) -> Option<Self> {
30+
let ctx = AppContext {
31+
app_name: "ads-client".into(),
32+
app_id: "org.mozilla.ads-client".into(),
33+
channel: "release".into(),
34+
..Default::default()
35+
};
36+
37+
// For a real implementation, the app would typically:
38+
// 1. Pass its own `AppContext` with real app info
39+
// 2. Pass a `MetricsHandler` that records to Glean
40+
// 3. Configure remote settings to fetch experiments from Mozilla's experiment server
41+
// 4. Not hardcode experiments in code
42+
let nimbus = Arc::new(
43+
NimbusClient::new(
44+
ctx,
45+
None,
46+
vec![],
47+
db_path,
48+
Box::new(NoopMetricsHandler),
49+
None,
50+
None,
51+
None,
52+
)
53+
.ok()?,
54+
);
55+
56+
nimbus.initialize().ok()?;
57+
58+
// Hardcoded 50/50 experiment for http-cache-enabled
5159
let experiment = json!({
5260
"data": [{
5361
"schemaVersion": "1.0.0",
54-
"slug": "ads-cache-test",
62+
"slug": "ads-client-cache",
5563
"featureIds": ["ads-client"],
5664
"branches": [
5765
{ "slug": "control", "ratio": 1, "feature": { "featureId": "ads-client", "value": { "http-cache-enabled": true } } },
5866
{ "slug": "treatment", "ratio": 1, "feature": { "featureId": "ads-client", "value": { "http-cache-enabled": false } } }
5967
],
60-
"bucketConfig": { "count": 10000, "start": 0, "total": 10000, "namespace": "test", "randomizationUnit": "nimbus_id" },
61-
"appName": "test-app", "appId": "org.mozilla.test", "channel": "test",
62-
"userFacingName": "Test", "userFacingDescription": "Test",
68+
"bucketConfig": { "count": 10000, "start": 0, "total": 10000, "namespace": "ads-client-cache", "randomizationUnit": "nimbus_id" },
69+
"appName": "ads-client", "appId": "org.mozilla.ads-client", "channel": "release",
70+
"userFacingName": "Ads Client Cache", "userFacingDescription": "50/50 experiment for HTTP cache",
6371
"isEnrollmentPaused": false, "proposedEnrollment": 7, "referenceBranch": "control"
6472
}]
6573
}).to_string();
6674

75+
nimbus.set_experiments_locally(experiment).ok()?;
76+
nimbus.apply_pending_experiments().ok()?;
77+
78+
Some(Self { nimbus })
79+
}
80+
81+
/// Check if HTTP cache is enabled for this user.
82+
pub fn is_http_cache_enabled(&self) -> bool {
83+
let Ok(Some(json)) = self
84+
.nimbus
85+
.get_feature_config_variables(FEATURE_ID.to_string())
86+
else {
87+
return true;
88+
};
89+
90+
serde_json::from_str::<serde_json::Value>(&json)
91+
.ok()
92+
.and_then(|v| v.get("http-cache-enabled")?.as_bool())
93+
.unwrap_or(true)
94+
}
95+
}
96+
97+
#[cfg(test)]
98+
mod tests {
99+
use super::*;
100+
101+
#[test]
102+
fn test_fifty_fifty_experiment() {
67103
let (mut enabled, mut disabled) = (0, 0);
68104
for i in 0..100 {
69105
let tmp_dir = tempfile::tempdir().unwrap();
70-
let nimbus = Arc::new(
71-
NimbusClient::new(
72-
AppContext {
73-
app_name: "test-app".into(),
74-
app_id: "org.mozilla.test".into(),
75-
channel: "test".into(),
76-
..Default::default()
77-
},
78-
None,
79-
vec![],
80-
tmp_dir.path(),
81-
Box::new(NoopMetricsHandler),
82-
None,
83-
None,
84-
None,
85-
)
86-
.unwrap(),
87-
);
88-
nimbus.initialize().unwrap();
89-
// Create a deterministic UUID from the loop index
106+
let db_path = tmp_dir.path().join("nimbus.db");
107+
let client = ExperimentClient::new(db_path.to_str().unwrap()).unwrap();
108+
109+
// Set a unique nimbus_id for each "user"
90110
let mut bytes = [0u8; 16];
91111
bytes[0] = i as u8;
92-
nimbus.set_nimbus_id(&Uuid::from_bytes(bytes)).unwrap();
93-
nimbus.set_experiments_locally(experiment.clone()).unwrap();
94-
nimbus.apply_pending_experiments().unwrap();
112+
client
113+
.nimbus
114+
.set_nimbus_id(&uuid::Uuid::from_bytes(bytes))
115+
.unwrap();
116+
client.nimbus.apply_pending_experiments().unwrap();
95117

96-
if is_http_cache_enabled(Some(nimbus.clone())) {
118+
if client.is_http_cache_enabled() {
97119
enabled += 1;
98120
} else {
99121
disabled += 1;

components/ads-client/src/ffi.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ use crate::error::ComponentError;
1616
use crate::ffi::telemetry::MozAdsTelemetryWrapper;
1717
use crate::http_cache::{CacheMode, RequestCachePolicy};
1818
use error_support::{ErrorHandling, GetErrorHandling};
19-
use nimbus::NimbusClient;
2019
use url::Url;
2120

2221
pub type AdsClientApiResult<T> = std::result::Result<T, MozAdsClientApiError>;
@@ -94,7 +93,6 @@ pub struct MozAdsClientConfig {
9493
pub environment: MozAdsEnvironment,
9594
pub cache_config: Option<MozAdsCacheConfig>,
9695
pub telemetry: Option<Arc<dyn MozAdsTelemetry>>,
97-
pub nimbus: Option<Arc<NimbusClient>>,
9896
}
9997

10098
impl From<MozAdsClientConfig> for AdsClientConfig<MozAdsTelemetryWrapper> {
@@ -107,7 +105,6 @@ impl From<MozAdsClientConfig> for AdsClientConfig<MozAdsTelemetryWrapper> {
107105
environment: config.environment.into(),
108106
cache_config: config.cache_config.map(Into::into),
109107
telemetry,
110-
nimbus: config.nimbus,
111108
}
112109
}
113110
}

0 commit comments

Comments
 (0)