AC-99: Remove context id provider

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 [Full Changelog](In progress)
 
+### Ads-Client
+
+- Removed the `MozAdsContextIdProvider` callback interface and the `context_id_provider()` builder method. Wiring it up from Firefox Desktop crashed off the main thread ([Bug 2062806](https://bugzilla.mozilla.org/show_bug.cgi?id=2062806)) and nothing else used it, so the ads client now always uses its embedded `ContextIDComponent`. ([AC-99](https://mozilla-hub.atlassian.net/browse/AC-99))
+
 ### Glean
 - Updated to v70.0.0 ([#7598](https://github.com/mozilla/application-services/pull/7598))
 

components/ads-client/src/client.rs

@@ -30,23 +30,13 @@ const DEFAULT_TTL_SECONDS: u64 = 300;
 const DEFAULT_MAX_CACHE_SIZE_MIB: u64 = 10;
 const DEFAULT_ROTATION_DAYS: u8 = 3;
 
-pub trait ContextIdProvider: Send + Sync {
-    fn context_id(&self) -> context_id::ApiResult<String>;
-}
-
-impl ContextIdProvider for ContextIDComponent {
-    fn context_id(&self) -> context_id::ApiResult<String> {
-        self.request(DEFAULT_ROTATION_DAYS)
-    }
-}
-
 pub struct AdsClient<T>
 where
     T: Clone + Telemetry,
 {
     ads_store: Arc<Mutex<Option<AdsStore>>>,
     client: MARSClient<T>,
-    context_id_provider: Box<dyn ContextIdProvider>,
+    context_id_component: ContextIDComponent,
     telemetry: T,
 }
 
@@ -55,14 +45,12 @@ where
     T: Clone + Telemetry,
 {
     pub fn new(client_config: AdsClientConfig<T>) -> Self {
-        let context_id_provider = client_config.context_id_provider.unwrap_or_else(|| {
-            Box::new(ContextIDComponent::new(
-                &Uuid::new_v4().to_string(),
-                0,
-                cfg!(test),
-                Box::new(DefaultContextIdCallback),
-            ))
-        });
+        let context_id_component = ContextIDComponent::new(
+            &Uuid::new_v4().to_string(),
+            0,
+            cfg!(test),
+            Box::new(DefaultContextIdCallback),
+        );
 
         let telemetry = client_config.telemetry;
         let environment = client_config.environment;
@@ -106,7 +94,7 @@ where
         telemetry.record(&ClientOperationEvent::New);
         Self {
             client,
-            context_id_provider,
+            context_id_component,
             telemetry: telemetry.clone(),
             ads_store: Arc::new(Mutex::new(ads_store)),
         }
@@ -117,7 +105,7 @@ where
     }
 
     pub fn get_context_id(&self) -> context_id::ApiResult<String> {
-        self.context_id_provider.context_id()
+        self.context_id_component.request(DEFAULT_ROTATION_DAYS)
     }
 
     pub fn record_click(&self, click_url: Url, ohttp: bool) -> Result<(), RecordClickError> {
@@ -299,7 +287,6 @@ pub enum ClientOperationEvent {
 
 #[cfg(test)]
 mod tests {
-    use std::assert_eq;
 
     use crate::{
         ads_store::builder::AdsStoreBuilder,
@@ -319,12 +306,12 @@ mod tests {
         let telemetry = client.get_telemetry();
         AdsClient {
             client,
-            context_id_provider: Box::new(ContextIDComponent::new(
+            context_id_component: ContextIDComponent::new(
                 &Uuid::new_v4().to_string(),
                 0,
                 false,
                 Box::new(DefaultContextIdCallback),
-            )),
+            ),
             telemetry,
             ads_store: Arc::new(Mutex::new(Some(
                 AdsStoreBuilder::new("test_store.db")
@@ -338,7 +325,6 @@ mod tests {
     fn test_get_context_id() {
         let config = AdsClientConfig {
             cache_config: None,
-            context_id_provider: None,
             environment: Environment::Test,
             telemetry: MozAdsTelemetryWrapper::noop(),
             store_config: None,
@@ -424,36 +410,31 @@ mod tests {
     }
 
     #[test]
-    fn test_custom_context_id_provider() {
+    fn test_context_id_is_sent_to_mars() {
         viaduct_dev::init_backend_dev();
 
-        struct FixedContextId;
-        impl ContextIdProvider for FixedContextId {
-            fn context_id(&self) -> context_id::ApiResult<String> {
-                Ok("custom-context-id-12345".to_string())
-            }
-        }
-
-        let expected_response = get_example_happy_image_response();
-        let m = mockito::mock("POST", "/ads")
-            .match_body(mockito::Matcher::PartialJsonString(
-                r#"{"context_id":"custom-context-id-12345"}"#.to_string(),
-            ))
-            .with_status(200)
-            .with_header("content-type", "application/json")
-            .with_body(serde_json::to_string(&expected_response.data).unwrap())
-            .create();
-
         let config = AdsClientConfig {
             cache_config: None,
-            context_id_provider: Some(Box::new(FixedContextId)),
             environment: Environment::Test,
             telemetry: MozAdsTelemetryWrapper::noop(),
             store_config: None,
         };
         let client = AdsClient::new(config);
 
-        assert_eq!(client.get_context_id().unwrap(), "custom-context-id-12345");
+        // The client generates its own context id, so read it back first and
+        // assert that exactly that value reaches the wire.
+        let context_id = client.get_context_id().unwrap();
+        assert!(Uuid::parse_str(&context_id).is_ok());
+
+        let expected_response = get_example_happy_image_response();
+        let m = mockito::mock("POST", "/ads")
+            .match_body(mockito::Matcher::PartialJsonString(format!(
+                r#"{{"context_id":"{context_id}"}}"#
+            )))
+            .with_status(200)
+            .with_header("content-type", "application/json")
+            .with_body(serde_json::to_string(&expected_response.data).unwrap())
+            .create();
 
         let result = client.request_image_ads(
             make_happy_placement_requests(),

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

@@ -11,7 +11,6 @@ where
     T: Telemetry,
 {
     pub cache_config: Option<AdsCacheConfig>,
-    pub context_id_provider: Option<Box<dyn super::ContextIdProvider>>,
     pub environment: Environment,
     pub store_config: Option<AdsStoreConfig>,
     pub telemetry: T,

components/ads-client/src/ffi.rs

@@ -11,7 +11,7 @@ use std::sync::Arc;
 use std::sync::Weak;
 
 use crate::client::config::{AdsCacheConfig, AdsClientConfig, AdsStoreConfig};
-use crate::client::{AdsClient, ContextIdProvider};
+use crate::client::AdsClient;
 use crate::ffi::telemetry::MozAdsTelemetryWrapper;
 use crate::http_cache::CachePolicy;
 use crate::mars::ad_request::{
@@ -30,33 +30,6 @@ use std::collections::HashMap;
 pub use error::{AdsClientApiResult, MozAdsClientApiError};
 pub use telemetry::MozAdsTelemetry;
 
-// TODO: Temporary workaround for HNT requirements — do not use for new integrations.
-// Context ID management should remain internal to the ads client and this interface should be removed.
-#[uniffi::export(with_foreign)]
-pub trait MozAdsContextIdProvider: Send + Sync {
-    fn context_id(&self) -> String;
-}
-
-struct MozAdsContextIdProviderWrapper(Arc<dyn MozAdsContextIdProvider>);
-
-impl MozAdsContextIdProviderWrapper {
-    fn new(provider: Arc<dyn MozAdsContextIdProvider>) -> Self {
-        Self(provider)
-    }
-}
-
-impl ContextIdProvider for MozAdsContextIdProviderWrapper {
-    fn context_id(&self) -> context_id::ApiResult<String> {
-        Ok(self.0.context_id())
-    }
-}
-
-impl From<MozAdsContextIdProviderWrapper> for Box<dyn ContextIdProvider> {
-    fn from(wrapper: MozAdsContextIdProviderWrapper) -> Self {
-        Box::new(wrapper)
-    }
-}
-
 #[derive(Default, uniffi::Record)]
 pub struct MozAdsRequestOptions {
     #[uniffi(default)]
@@ -108,7 +81,6 @@ pub struct MozAdsClientBuilder(Mutex<MozAdsClientBuilderInner>);
 #[derive(Default)]
 struct MozAdsClientBuilderInner {
     cache_config: Option<MozAdsCacheConfig>,
-    context_id_provider: Option<Arc<dyn MozAdsContextIdProvider>>,
     environment: Option<MozAdsEnvironment>,
     store_config: Option<MozAdsStoreConfig>,
     telemetry: Option<Arc<dyn MozAdsTelemetry>>,
@@ -136,11 +108,6 @@ impl MozAdsClientBuilder {
             .unwrap_or_else(MozAdsTelemetryWrapper::noop);
         let client_config = AdsClientConfig {
             cache_config: inner.cache_config.clone().map(Into::into),
-            context_id_provider: inner
-                .context_id_provider
-                .clone()
-                .map(MozAdsContextIdProviderWrapper::new)
-                .map(Into::into),
             environment: inner.environment.clone().unwrap_or_default().into(),
             telemetry: telemetry.clone(),
             store_config: inner.store_config.clone().map(Into::into),
@@ -163,14 +130,6 @@ impl MozAdsClientBuilder {
         self
     }
 
-    pub fn context_id_provider(
-        self: Arc<Self>,
-        provider: Arc<dyn MozAdsContextIdProvider>,
-    ) -> Arc<Self> {
-        self.0.lock().context_id_provider = Some(provider);
-        self
-    }
-
     pub fn environment(self: Arc<Self>, environment: MozAdsEnvironment) -> Arc<Self> {
         self.0.lock().environment = Some(environment);
         self