refactor(ads-client): use any for simpler telemetry type

Author: Almaju

components/ads-client/src/client.rs

@@ -4,16 +4,14 @@
 */
 
 use std::collections::HashMap;
-use std::sync::Arc;
 use std::time::Duration;
 
 use crate::client::ad_response::{
     pop_request_hash_from_url, AdImage, AdResponse, AdResponseValue, AdSpoc, AdTile,
 };
 use crate::client::config::AdsClientConfig;
-use crate::client::telemetry::ClientOperationEvent;
 use crate::error::{RecordClickError, RecordImpressionError, ReportAdError, RequestAdsError};
-use crate::http_cache::{CacheOutcome, HttpCache, HttpCacheBuilderError, RequestCachePolicy};
+use crate::http_cache::{HttpCache, RequestCachePolicy};
 use crate::mars::MARSClient;
 use crate::telemetry::Telemetry;
 use ad_request::{AdPlacementRequest, AdRequest};
@@ -26,37 +24,22 @@ use crate::http_cache::{ByteSize, HttpCacheError};
 pub mod ad_request;
 pub mod ad_response;
 pub mod config;
-pub mod telemetry;
 
 const DEFAULT_TTL_SECONDS: u64 = 300;
 const DEFAULT_MAX_CACHE_SIZE_MIB: u64 = 10;
 
 pub struct AdsClient<T>
 where
-    T: Telemetry<CacheOutcome>
-        + Telemetry<ClientOperationEvent>
-        + Telemetry<HttpCacheBuilderError>
-        + Telemetry<RecordClickError>
-        + Telemetry<RecordImpressionError>
-        + Telemetry<ReportAdError>
-        + Telemetry<RequestAdsError>
-        + Telemetry<serde_json::Error>,
+    T: Clone + Telemetry,
 {
     client: MARSClient<T>,
     context_id_component: ContextIDComponent,
-    telemetry: Arc<T>,
+    telemetry: T,
 }
 
 impl<T> AdsClient<T>
 where
-    T: Telemetry<CacheOutcome>
-        + Telemetry<serde_json::Error>
-        + Telemetry<ClientOperationEvent>
-        + Telemetry<HttpCacheBuilderError>
-        + Telemetry<RecordClickError>
-        + Telemetry<RecordImpressionError>
-        + Telemetry<ReportAdError>
-        + Telemetry<RequestAdsError>,
+    T: Clone + Telemetry,
 {
     pub fn new(client_config: AdsClientConfig<T>) -> Self {
         let context_id = Uuid::new_v4().to_string();
@@ -66,6 +49,7 @@ where
             cfg!(test),
             Box::new(DefaultContextIdCallback),
         );
+        let telemetry = client_config.telemetry;
 
         // Configure the cache if a path is provided.
         // Defaults for ttl and cache size are also set if unspecified.
@@ -85,36 +69,28 @@ where
             {
                 Ok(cache) => Some(cache),
                 Err(e) => {
-                    client_config.telemetry.record(&e);
+                    telemetry.record(&e);
                     None
                 }
             };
 
-            let client = MARSClient::new(
-                client_config.environment,
-                http_cache,
-                client_config.telemetry.clone(),
-            );
+            let client = MARSClient::new(client_config.environment, http_cache, telemetry.clone());
             let client = Self {
                 context_id_component,
                 client,
-                telemetry: client_config.telemetry.clone(),
+                telemetry: telemetry.clone(),
             };
-            client.telemetry.record(&ClientOperationEvent::New);
+            telemetry.record(&ClientOperationEvent::New);
             return client;
         }
 
-        let client = MARSClient::new(
-            client_config.environment,
-            None,
-            client_config.telemetry.clone(),
-        );
+        let client = MARSClient::new(client_config.environment, None, telemetry.clone());
         let client = Self {
             context_id_component,
             client,
-            telemetry: client_config.telemetry.clone(),
+            telemetry: telemetry.clone(),
         };
-        client.telemetry.record(&ClientOperationEvent::New);
+        telemetry.record(&ClientOperationEvent::New);
         client
     }
 
@@ -238,19 +214,31 @@ where
     }
 }
 
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum ClientOperationEvent {
+    New,
+    RecordClick,
+    RecordImpression,
+    ReportAd,
+    RequestAds,
+}
+
 #[cfg(test)]
 mod tests {
     use crate::{
         client::config::Environment,
+        ffi::telemetry::MozAdsTelemetryWrapper,
         test_utils::{
             get_example_happy_image_response, get_example_happy_spoc_response,
-            get_example_happy_uatile_response, make_happy_placement_requests, PrintAdsTelemetry,
+            get_example_happy_uatile_response, make_happy_placement_requests,
         },
     };
 
     use super::*;
 
-    fn new_with_mars_client(client: MARSClient<PrintAdsTelemetry>) -> AdsClient<PrintAdsTelemetry> {
+    fn new_with_mars_client(
+        client: MARSClient<MozAdsTelemetryWrapper>,
+    ) -> AdsClient<MozAdsTelemetryWrapper> {
         let context_id_component = ContextIDComponent::new(
             &uuid::Uuid::new_v4().to_string(),
             0,
@@ -260,17 +248,16 @@ mod tests {
         AdsClient {
             context_id_component,
             client,
-            telemetry: Arc::new(PrintAdsTelemetry),
+            telemetry: MozAdsTelemetryWrapper::noop(),
         }
     }
 
     #[test]
     fn test_get_context_id() {
-        use std::sync::Arc;
         let config = AdsClientConfig {
             environment: Environment::Test,
             cache_config: None,
-            telemetry: Arc::new(PrintAdsTelemetry),
+            telemetry: MozAdsTelemetryWrapper::noop(),
         };
         let client = AdsClient::new(config);
         let context_id = client.get_context_id().unwrap();
@@ -279,11 +266,10 @@ mod tests {
 
     #[test]
     fn test_cycle_context_id() {
-        use std::sync::Arc;
         let config = AdsClientConfig {
             environment: Environment::Test,
             cache_config: None,
-            telemetry: Arc::new(PrintAdsTelemetry),
+            telemetry: MozAdsTelemetryWrapper::noop(),
         };
         let mut client = AdsClient::new(config);
         let old_id = client.get_context_id().unwrap();
@@ -304,8 +290,8 @@ mod tests {
             .with_body(serde_json::to_string(&expected_response.data).unwrap())
             .create();
 
-        let telemetry = Arc::new(PrintAdsTelemetry);
-        let mars_client = MARSClient::new(Environment::Test, None, telemetry.clone());
+        let telemetry = MozAdsTelemetryWrapper::noop();
+        let mars_client = MARSClient::new(Environment::Test, None, telemetry);
         let ads_client = new_with_mars_client(mars_client);
 
         let ad_placement_requests = make_happy_placement_requests();
@@ -326,8 +312,8 @@ mod tests {
             .with_body(serde_json::to_string(&expected_response.data).unwrap())
             .create();
 
-        let telemetry = Arc::new(PrintAdsTelemetry);
-        let mars_client = MARSClient::new(Environment::Test, None, telemetry.clone());
+        let telemetry = MozAdsTelemetryWrapper::noop();
+        let mars_client = MARSClient::new(Environment::Test, None, telemetry);
         let ads_client = new_with_mars_client(mars_client);
 
         let ad_placement_requests = make_happy_placement_requests();
@@ -348,7 +334,7 @@ mod tests {
             .with_body(serde_json::to_string(&expected_response.data).unwrap())
             .create();
 
-        let telemetry = Arc::new(PrintAdsTelemetry);
+        let telemetry = MozAdsTelemetryWrapper::noop();
         let mars_client = MARSClient::new(Environment::Test, None, telemetry.clone());
         let ads_client = new_with_mars_client(mars_client);
 
@@ -365,7 +351,7 @@ mod tests {
         let cache = HttpCache::builder("test_record_click_invalidates_cache")
             .build()
             .unwrap();
-        let telemetry = Arc::new(PrintAdsTelemetry);
+        let telemetry = MozAdsTelemetryWrapper::noop();
         let mars_client = MARSClient::new(Environment::Test, Some(cache), telemetry.clone());
         let ads_client = new_with_mars_client(mars_client);
 

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

@@ -16,7 +16,7 @@ pub struct AdResponse<A: AdResponseValue> {
 }
 
 impl<A: AdResponseValue> AdResponse<A> {
-    pub fn parse<T: Telemetry<serde_json::Error> + ?Sized>(
+    pub fn parse<T: Telemetry>(
         data: serde_json::Value,
         telemetry: &T,
     ) -> Result<AdResponse<A>, serde_json::Error> {
@@ -175,7 +175,7 @@ impl AdResponseValue for AdTile {
 
 #[cfg(test)]
 mod tests {
-    use crate::test_utils::PrintAdsTelemetry;
+    use crate::ffi::telemetry::MozAdsTelemetryWrapper;
 
     use super::*;
     use serde_json::{from_str, json};
@@ -317,7 +317,8 @@ mod tests {
             ]
         });
 
-        let parsed = AdResponse::<AdImage>::parse(raw_ad_response, &PrintAdsTelemetry).unwrap();
+        let parsed =
+            AdResponse::<AdImage>::parse(raw_ad_response, &MozAdsTelemetryWrapper::noop()).unwrap();
 
         let expected = AdResponse {
             data: HashMap::from([(
@@ -353,7 +354,8 @@ mod tests {
             "example_placement_2": []
         });
 
-        let parsed = AdResponse::<AdImage>::parse(raw_ad_response, &PrintAdsTelemetry).unwrap();
+        let parsed =
+            AdResponse::<AdImage>::parse(raw_ad_response, &MozAdsTelemetryWrapper::noop()).unwrap();
 
         let expected = AdResponse {
             data: HashMap::from([]),

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

@@ -3,14 +3,9 @@
 * file, You can obtain one at http://mozilla.org/MPL/2.0/.
 */
 
-use std::sync::Arc;
-
 use once_cell::sync::Lazy;
 use url::Url;
 
-use crate::client::telemetry::ClientOperationEvent;
-use crate::error::{RecordClickError, RecordImpressionError, ReportAdError, RequestAdsError};
-use crate::http_cache::{CacheOutcome, HttpCacheBuilderError};
 use crate::telemetry::Telemetry;
 
 static MARS_API_ENDPOINT_PROD: Lazy<Url> =
@@ -22,18 +17,11 @@ static MARS_API_ENDPOINT_STAGING: Lazy<Url> =
 
 pub struct AdsClientConfig<T>
 where
-    T: Telemetry<CacheOutcome>
-        + Telemetry<ClientOperationEvent>
-        + Telemetry<HttpCacheBuilderError>
-        + Telemetry<RecordClickError>
-        + Telemetry<RecordImpressionError>
-        + Telemetry<ReportAdError>
-        + Telemetry<RequestAdsError>
-        + Telemetry<serde_json::Error>,
+    T: Telemetry,
 {
     pub environment: Environment,
     pub cache_config: Option<AdsCacheConfig>,
-    pub telemetry: Arc<T>,
+    pub telemetry: T,
 }
 
 #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]

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

@@ -13,7 +13,7 @@ use crate::client::ad_response::{
 };
 use crate::client::config::{AdsCacheConfig, AdsClientConfig, Environment};
 use crate::error::ComponentError;
-use crate::ffi::telemetry::{MozAdsTelemetryWrapper, NoopMozAdsTelemetry};
+use crate::ffi::telemetry::MozAdsTelemetryWrapper;
 use crate::http_cache::{CacheMode, RequestCachePolicy};
 use error_support::{ErrorHandling, GetErrorHandling};
 use url::Url;
@@ -99,12 +99,8 @@ impl From<MozAdsClientConfig> for AdsClientConfig<MozAdsTelemetryWrapper> {
     fn from(config: MozAdsClientConfig) -> Self {
         let telemetry = config
             .telemetry
-            .map(|t| Arc::new(MozAdsTelemetryWrapper { inner: t }))
-            .unwrap_or_else(|| {
-                Arc::new(MozAdsTelemetryWrapper {
-                    inner: Arc::new(NoopMozAdsTelemetry),
-                })
-            });
+            .map(MozAdsTelemetryWrapper::new)
+            .unwrap_or_else(MozAdsTelemetryWrapper::noop);
         Self {
             environment: config.environment.into(),
             cache_config: config.cache_config.map(Into::into),