refactor(ads-client): housekeeping - sort fields/methods, add UniFFI defaults, split docs, add Client in HttpCache

components/ads-client/README.md

@@ -44,4 +44,10 @@ cargo test -p ads-client-integration-tests --test mars test_contract_image_stagi
 
 ## Usage
 
-Please refer to `./docs/usage.md` for information on using the component.
+Full API reference and usage guides for each supported language:
+
+- [Swift](./docs/usage-swift.md)
+- [Kotlin](./docs/usage-kotlin.md)
+- [JavaScript](./docs/usage-javascript.md)
+
+Each guide is a complete standalone document containing all type definitions, API tables, cache behavior documentation, and code examples in that language.

components/ads-client/docs/building-locally-for-android.md

@@ -189,4 +189,4 @@ If Fenix doesn't auto-publish application-services:
 - [Main Application Services Building Guide](../../../docs/building.md)
 - [Auto-Publishing Workflow Documentation](../../../docs/howtos/locally-published-components-in-fenix.md)
 - [Android FAQs](../../../docs/android-faqs.md)
-- [ads-client Usage Documentation](./usage.md)
+- [ads-client Usage Documentation (Kotlin)](./usage-kotlin.md)

components/ads-client/integration-tests/tests/http_cache.rs

@@ -6,9 +6,9 @@
 use std::hash::{Hash, Hasher};
 use std::time::Duration;
 
-use ads_client::http_cache::{ByteSize, CacheMode, CacheOutcome, HttpCache, RequestCachePolicy};
+use ads_client::http_cache::{ByteSize, CacheOutcome, CachePolicy, HttpCache};
 use mockito::mock;
-use viaduct::Request;
+use viaduct::{Client, ClientSettings, Request};
 
 /// Test-only hashable wrapper around Request.
 #[derive(Clone)]
@@ -49,6 +49,7 @@ fn test_cache_works_using_real_timeouts() {
         ],
     })));
 
+    let client = Client::new(ClientSettings::default());
     let test_ttl = 2;
 
     let _m1 = mock("POST", "/v1/ads")
@@ -61,18 +62,18 @@ fn test_cache_works_using_real_timeouts() {
     // First call: miss -> store
     let (_, outcomes) = cache
         .send_with_policy(
+            &client,
             req.clone(),
-            &RequestCachePolicy {
-                mode: CacheMode::CacheFirst,
-                ttl_seconds: Some(test_ttl),
+            &CachePolicy::CacheFirst {
+                ttl: Some(Duration::from_secs(test_ttl)),
             },
         )
         .unwrap();
     assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored));
 
     // Second call: hit (no extra HTTP due to expect(1))
     let (response, outcomes) = cache
-        .send_with_policy(req.clone(), &RequestCachePolicy::default())
+        .send_with_policy(&client, req.clone(), &CachePolicy::default())
         .unwrap();
     assert!(matches!(outcomes.last().unwrap(), CacheOutcome::Hit));
     assert_eq!(response.status, 200);
@@ -87,7 +88,7 @@ fn test_cache_works_using_real_timeouts() {
     // Third call: Miss due to timeout for the test_ttl duration
     std::thread::sleep(Duration::from_secs(test_ttl));
     let (response, outcomes) = cache
-        .send_with_policy(req, &RequestCachePolicy::default())
+        .send_with_policy(&client, req, &CachePolicy::default())
         .unwrap();
     assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored));
     assert_eq!(response.status, 200);

components/ads-client/integration-tests/tests/mars.rs

@@ -29,8 +29,8 @@ fn test_contract_image_prod() {
     let client = prod_client();
     let result = client.request_image_ads(
         vec![MozAdsPlacementRequest {
-            placement_id: "mock_billboard_1".to_string(),
             iab_content: None,
+            placement_id: "mock_billboard_1".to_string(),
         }],
         None,
     );
@@ -52,9 +52,9 @@ fn test_contract_spoc_prod() {
     let client = prod_client();
     let result = client.request_spoc_ads(
         vec![MozAdsPlacementRequestWithCount {
-            placement_id: "mock_spoc_1".to_string(),
             count: 3,
             iab_content: None,
+            placement_id: "mock_spoc_1".to_string(),
         }],
         None,
     );
@@ -73,8 +73,8 @@ fn test_contract_tile_prod() {
     let client = prod_client();
     let result = client.request_tile_ads(
         vec![MozAdsPlacementRequest {
-            placement_id: "mock_tile_1".to_string(),
             iab_content: None,
+            placement_id: "mock_tile_1".to_string(),
         }],
         None,
     );

components/ads-client/src/client.rs

@@ -26,7 +26,7 @@ impl ReportReason {
         }
     }
 }
-use crate::http_cache::{HttpCache, RequestCachePolicy};
+use crate::http_cache::{CachePolicy, HttpCache};
 use crate::mars::MARSClient;
 use crate::telemetry::Telemetry;
 use ad_request::{AdPlacementRequest, AdRequest};
@@ -51,8 +51,8 @@ where
     client: MARSClient<T>,
     context_id_component: ContextIDComponent,
     environment: Environment,
-    telemetry: T,
     rotation_days: u8,
+    telemetry: T,
 }
 
 impl<T> AdsClient<T>
@@ -96,50 +96,86 @@ where
 
             let client = MARSClient::new(http_cache, telemetry.clone());
             let client = Self {
-                environment,
-                context_id_component,
                 client,
-                telemetry: telemetry.clone(),
+                context_id_component,
+                environment,
                 rotation_days,
+                telemetry: telemetry.clone(),
             };
             telemetry.record(&ClientOperationEvent::New);
             return client;
         }
 
         let client = MARSClient::new(None, telemetry.clone());
         let client = Self {
-            environment,
-            context_id_component,
             client,
-            telemetry: telemetry.clone(),
+            context_id_component,
+            environment,
             rotation_days,
+            telemetry: telemetry.clone(),
         };
         telemetry.record(&ClientOperationEvent::New);
         client
     }
 
-    fn request_ads<A>(
-        &self,
-        ad_placement_requests: Vec<AdPlacementRequest>,
-        options: Option<RequestCachePolicy>,
-    ) -> Result<AdResponse<A>, RequestAdsError>
-    where
-        A: AdResponseValue,
-    {
-        let context_id = self.get_context_id()?;
-        let url = self.environment.into_url("ads");
-        let ad_request = AdRequest::try_new(context_id, ad_placement_requests, url)?;
-        let cache_policy = options.unwrap_or_default();
-        let (mut response, request_hash) = self.client.fetch_ads::<A>(ad_request, &cache_policy)?;
-        response.add_request_hash_to_callbacks(&request_hash);
-        response.add_placement_info_to_report_callbacks();
-        Ok(response)
+    pub fn clear_cache(&self) -> Result<(), HttpCacheError> {
+        self.client.clear_cache()
+    }
+
+    pub fn get_context_id(&self) -> context_id::ApiResult<String> {
+        self.context_id_component.request(self.rotation_days)
+    }
+
+    pub fn record_click(&self, click_url: Url) -> Result<(), RecordClickError> {
+        // TODO: Re-enable cache invalidation behind a Nimbus experiment.
+        // The mobile team has requested this be temporarily disabled.
+        // let mut click_url = click_url.clone();
+        // if let Some(request_hash) = pop_request_hash_from_url(&mut click_url) {
+        //     let _ = self.client.invalidate_cache_by_hash(&request_hash);
+        // }
+        self.client
+            .record_click(click_url)
+            .inspect_err(|e| {
+                self.telemetry.record(e);
+            })
+            .inspect(|_| {
+                self.telemetry.record(&ClientOperationEvent::RecordClick);
+            })
+    }
+
+    pub fn record_impression(&self, impression_url: Url) -> Result<(), RecordImpressionError> {
+        // TODO: Re-enable cache invalidation behind a Nimbus experiment.
+        // The mobile team has requested this be temporarily disabled.
+        // let mut impression_url = impression_url.clone();
+        // if let Some(request_hash) = pop_request_hash_from_url(&mut impression_url) {
+        //     let _ = self.client.invalidate_cache_by_hash(&request_hash);
+        // }
+        self.client
+            .record_impression(impression_url)
+            .inspect_err(|e| {
+                self.telemetry.record(e);
+            })
+            .inspect(|_| {
+                self.telemetry
+                    .record(&ClientOperationEvent::RecordImpression);
+            })
+    }
+
+    pub fn report_ad(&self, report_url: Url, reason: ReportReason) -> Result<(), ReportAdError> {
+        self.client
+            .report_ad(report_url, reason)
+            .inspect_err(|e| {
+                self.telemetry.record(e);
+            })
+            .inspect(|_| {
+                self.telemetry.record(&ClientOperationEvent::ReportAd);
+            })
     }
 
     pub fn request_image_ads(
         &self,
         ad_placement_requests: Vec<AdPlacementRequest>,
-        options: Option<RequestCachePolicy>,
+        options: Option<CachePolicy>,
     ) -> Result<HashMap<String, AdImage>, RequestAdsError> {
         let response = self
             .request_ads::<AdImage>(ad_placement_requests, options)
@@ -153,7 +189,7 @@ where
     pub fn request_spoc_ads(
         &self,
         ad_placement_requests: Vec<AdPlacementRequest>,
-        options: Option<RequestCachePolicy>,
+        options: Option<CachePolicy>,
     ) -> Result<HashMap<String, Vec<AdSpoc>>, RequestAdsError> {
         let result = self.request_ads::<AdSpoc>(ad_placement_requests, options);
         result
@@ -169,7 +205,7 @@ where
     pub fn request_tile_ads(
         &self,
         ad_placement_requests: Vec<AdPlacementRequest>,
-        options: Option<RequestCachePolicy>,
+        options: Option<CachePolicy>,
     ) -> Result<HashMap<String, AdTile>, RequestAdsError> {
         let result = self.request_ads::<AdTile>(ad_placement_requests, options);
         result
@@ -182,58 +218,22 @@ where
             })
     }
 
-    pub fn record_impression(&self, impression_url: Url) -> Result<(), RecordImpressionError> {
-        // TODO: Re-enable cache invalidation behind a Nimbus experiment.
-        // The mobile team has requested this be temporarily disabled.
-        // let mut impression_url = impression_url.clone();
-        // if let Some(request_hash) = pop_request_hash_from_url(&mut impression_url) {
-        //     let _ = self.client.invalidate_cache_by_hash(&request_hash);
-        // }
-        self.client
-            .record_impression(impression_url)
-            .inspect_err(|e| {
-                self.telemetry.record(e);
-            })
-            .inspect(|_| {
-                self.telemetry
-                    .record(&ClientOperationEvent::RecordImpression);
-            })
-    }
-
-    pub fn record_click(&self, click_url: Url) -> Result<(), RecordClickError> {
-        // TODO: Re-enable cache invalidation behind a Nimbus experiment.
-        // The mobile team has requested this be temporarily disabled.
-        // let mut click_url = click_url.clone();
-        // if let Some(request_hash) = pop_request_hash_from_url(&mut click_url) {
-        //     let _ = self.client.invalidate_cache_by_hash(&request_hash);
-        // }
-        self.client
-            .record_click(click_url)
-            .inspect_err(|e| {
-                self.telemetry.record(e);
-            })
-            .inspect(|_| {
-                self.telemetry.record(&ClientOperationEvent::RecordClick);
-            })
-    }
-
-    pub fn report_ad(&self, report_url: Url, reason: ReportReason) -> Result<(), ReportAdError> {
-        self.client
-            .report_ad(report_url, reason)
-            .inspect_err(|e| {
-                self.telemetry.record(e);
-            })
-            .inspect(|_| {
-                self.telemetry.record(&ClientOperationEvent::ReportAd);
-            })
-    }
-
-    pub fn get_context_id(&self) -> context_id::ApiResult<String> {
-        self.context_id_component.request(self.rotation_days)
-    }
-
-    pub fn clear_cache(&self) -> Result<(), HttpCacheError> {
-        self.client.clear_cache()
+    fn request_ads<A>(
+        &self,
+        ad_placement_requests: Vec<AdPlacementRequest>,
+        options: Option<CachePolicy>,
+    ) -> Result<AdResponse<A>, RequestAdsError>
+    where
+        A: AdResponseValue,
+    {
+        let context_id = self.get_context_id()?;
+        let url = self.environment.into_url("ads");
+        let ad_request = AdRequest::try_new(context_id, ad_placement_requests, url)?;
+        let cache_policy = options.unwrap_or_default();
+        let (mut response, request_hash) = self.client.fetch_ads::<A>(ad_request, cache_policy)?;
+        response.add_request_hash_to_callbacks(&request_hash);
+        response.add_placement_info_to_report_callbacks();
+        Ok(response)
     }
 }
 
@@ -268,21 +268,21 @@ mod tests {
             Box::new(DefaultContextIdCallback),
         );
         AdsClient {
-            environment: Environment::Test,
-            context_id_component,
             client,
-            telemetry: MozAdsTelemetryWrapper::noop(),
+            context_id_component,
+            environment: Environment::Test,
             rotation_days: DEFAULT_ROTATION_DAYS,
+            telemetry: MozAdsTelemetryWrapper::noop(),
         }
     }
 
     #[test]
     fn test_get_context_id() {
         let config = AdsClientConfig {
-            environment: Environment::Test,
             cache_config: None,
-            telemetry: MozAdsTelemetryWrapper::noop(),
+            environment: Environment::Test,
             rotation_days: None,
+            telemetry: MozAdsTelemetryWrapper::noop(),
         };
         let client = AdsClient::new(config);
         let context_id = client.get_context_id().unwrap();
@@ -394,7 +394,7 @@ mod tests {
         ads_client
             .request_ads::<AdImage>(
                 make_happy_placement_requests(),
-                Some(RequestCachePolicy::default()),
+                Some(CachePolicy::default()),
             )
             .unwrap();
     }