refactor(ads-client): convert MozAdsClientBuilder to fluent builder pattern with Arc-based setters (#7188)

- Restructure MozAdsClientBuilder to use Mutex<Inner> pattern following SuggestStoreBuilder
- Change build() method from consuming self to borrowing &self for uniffi Arc compatibility
- Add fluent setter methods: environment(), cache_config(), telemetry()
- Fix uniffi error where Arc<T> cannot move values out with consuming self
- Add Default impl for MozAdsClientBuilder and clippy allow for new_ret_no_self
- Update usage.md with new builder pattern examples for Swift and Kotlin
- Update integration tests to use new builder.build() pattern
- Remove unused AdsClientConfig import

Author: Almaju

components/ads-client/ARCHITECTURE.md

@@ -13,7 +13,7 @@ This component uses a clear separation between **FFI (Foreign Function Interface
 
 #### 1. Clear Public API Identification
 
-All types prefixed with `MozAds` (e.g., `MozAdsClient`, `MozAd`, `MozAdsClientConfig`) represent the **public API contract**. This makes it immediately obvious:
+All types prefixed with `MozAds` (e.g., `MozAdsClient`, `MozAd`, `MozAdsClientBuilder`) represent the **public API contract**. This makes it immediately obvious:
 
 - What types external consumers depend on
 - What changes require coordination with consumers
@@ -40,7 +40,38 @@ The separation enables future API versioning strategies:
 - Evolve the public API independently from internal implementation
 - Provide migration paths between API versions
 
-### Implementation Pattern
+### Implementation Patterns
+
+#### Builder Pattern for Client Construction
+
+`MozAdsClientBuilder` follows the fluent builder pattern compatible with UniFFI's Arc-based object model:
+
+```rust
+#[derive(uniffi::Object)]
+pub struct MozAdsClientBuilder(Mutex<MozAdsClientBuilderInner>);
+
+#[uniffi::export]
+impl MozAdsClientBuilder {
+    // Setter methods take Arc<Self> and return Arc<Self> for chaining
+    pub fn environment(self: Arc<Self>, environment: MozAdsEnvironment) -> Arc<Self> {
+        self.0.lock().environment = Some(environment);
+        self  // Returns self for method chaining
+    }
+    
+    // build() takes &self (not consuming) to work with UniFFI's Arc wrapping
+    pub fn build(&self) -> MozAdsClient { ... }
+}
+```
+
+Key design decisions:
+- **Mutex wrapper**: Enables interior mutability across FFI boundaries
+- **Arc<Self> for setters**: Required for UniFFI objects, enables method chaining
+- **&self for build()**: UniFFI wraps objects in Arc, so we can't consume self
+- **Separate inner struct**: Keeps the actual configuration fields in a non-uniffi type
+
+This pattern is consistent with other builders in the codebase (e.g., `SuggestStoreBuilder`).
+
+#### Type Conversions
 
 The conversion between FFI and business logic types is handled through `From`/`Into` trait implementations in `ffi.rs`:
 

components/ads-client/docs/usage.md

@@ -17,16 +17,35 @@ pub struct MozAdsClient {
 }
 ```
 
-#### Constructors
+#### Creating a Client
 
-```rust
-impl MozAdsClient {
-  pub fn new(client_config: Option<MozAdsClientConfig>) -> Self
-}
+Use the `MozAdsClientBuilder` to configure and create the client. The builder provides a fluent API for setting configuration options.
+
+**Swift:**
+```swift
+let client = MozAdsClientBuilder()
+    .environment(environment: .prod)
+    .cacheConfig(cacheConfig: cache)
+    .telemetry(telemetry: telemetry)
+    .build()
 ```
 
-Creates a new ads client with an optional configuration object.
-If a cache configuration is provided, the client will initialize an on-disk HTTP cache at the given path.
+**Kotlin:**
+```kotlin
+val client = MozAdsClientBuilder()
+    .environment(MozAdsEnvironment.PROD)
+    .cacheConfig(cache)
+    .telemetry(telemetry)
+    .build()
+```
+
+**Rust:**
+```rust
+let client = MozAdsClientBuilder::new()
+    .environment(MozAdsEnvironment::Prod)
+    .cache_config(cache_config)
+    .build();
+```
 
 #### Methods
 
@@ -45,34 +64,38 @@ If a cache configuration is provided, the client will initialize an on-disk HTTP
 >
 > - We recommend that this client be initialized as a singleton or something similar so that multiple instances of the client do not exist at once.
 > - Responses omit placements with no fill. Empty placements do not appear in the returned maps.
-> - The HTTP cache is internally managed. Configuration can be set with `MozAdsClientConfig`. Per-request cache settings can be set with `MozAdsRequestOptions`.
+> - The HTTP cache is internally managed. Configuration can be set with `MozAdsClientBuilder`. Per-request cache settings can be set with `MozAdsRequestOptions`.
 > - If `cache_config` is `None`, caching is disabled entirely.
 
 ---
 
-## `MozAdsClientConfig`
+## `MozAdsClientBuilder`
 
-Configuration for initializing the ads client.
+Builder for configuring and creating the ads client. Use the fluent builder pattern to set configuration options.
 
 ```rust
-pub struct MozAdsClientConfig {
-  pub environment: Environment,
-  pub cache_config: Option<MozAdsCacheConfig>,
-  pub telemetry: Option<Arc<dyn MozAdsTelemetry>>,
-}
+pub struct MozAdsClientBuilder
 ```
 
-| Field          | Type                               | Description                                                                                            |
+#### Methods
+
+- **`new() -> MozAdsClientBuilder`** - Creates a new builder with default values
+- **`environment(self, environment: MozAdsEnvironment) -> Self`** - Sets the MARS environment (Prod, Staging, or Test)
+- **`cache_config(self, cache_config: MozAdsCacheConfig) -> Self`** - Sets the cache configuration
+- **`telemetry(self, telemetry: Arc<dyn MozAdsTelemetry>) -> Self`** - Sets the telemetry implementation
+- **`build(self) -> MozAdsClient`** - Builds and returns the configured client
+
+| Configuration  | Type                               | Description                                                                                            |
 | -------------- | ---------------------------------- | ------------------------------------------------------------------------------------------------------ |
-| `environment`  | `Environment`                      | Selects which MARS environment to connect to. Unless in a dev build, this value can only ever be Prod. |
+| `environment`  | `MozAdsEnvironment`                | Selects which MARS environment to connect to. Unless in a dev build, this value can only ever be Prod. Defaults to Prod. |
 | `cache_config` | `Option<MozAdsCacheConfig>`        | Optional configuration for the internal cache.                                                         |
 | `telemetry`    | `Option<Arc<dyn MozAdsTelemetry>>` | Optional telemetry instance for recording metrics. If not provided, a no-op implementation is used.    |
 
 ---
 
 ## `MozAdsTelemetry`
 
-Telemetry interface for recording ads client metrics. You must provide an implementation of this interface to the `MozAdsClientConfig` constructor to enable telemetry collection. If no telemetry instance is provided, a no-op implementation is used and no metrics will be recorded.
+Telemetry interface for recording ads client metrics. You must provide an implementation of this interface to the `MozAdsClientBuilder` constructor to enable telemetry collection. If no telemetry instance is provided, a no-op implementation is used and no metrics will be recorded.
 
 ```rust
 pub trait MozAdsTelemetry: Send + Sync {
@@ -86,7 +109,7 @@ pub trait MozAdsTelemetry: Send + Sync {
 
 ### Implementing Telemetry
 
-To enable telemetry collection, you need to implement the `MozAdsTelemetry` interface and provide an instance to the `MozAdsClientConfig` constructor. The following examples show how to bind Glean metrics to the telemetry interface.
+To enable telemetry collection, you need to implement the `MozAdsTelemetry` interface and provide an instance to the `MozAdsClientBuilder` constructor. The following examples show how to bind Glean metrics to the telemetry interface.
 
 #### Swift Example
 
@@ -489,13 +512,12 @@ let cache = MozAdsCacheConfig(
 )
 
 let telemetry = AdsClientTelemetry()
-let clientCfg = MozAdsClientConfig(
-    environment: .prod,
-    cacheConfig: cache,
-    telemetry: telemetry
-)
 
-let client = MozAdsClient(clientConfig: clientCfg)
+let client = MozAdsClientBuilder()
+    .environment(environment: .prod)
+    .cacheConfig(cacheConfig: cache)
+    .telemetry(telemetry: telemetry)
+    .build()
 ```
 
 ```kotlin
@@ -507,13 +529,12 @@ val cache = MozAdsCacheConfig(
 )
 
 val telemetry = AdsClientTelemetry()
-val clientCfg = MozAdsClientConfig(
-    environment = MozAdsEnvironment.PROD,
-    cacheConfig = cache,
-    telemetry = telemetry
-)
 
-val client = MozAdsClient(clientCfg)
+val client = MozAdsClientBuilder()
+    .environment(MozAdsEnvironment.PROD)
+    .cacheConfig(cache)
+    .telemetry(telemetry)
+    .build()
 ```
 
 Where `db_path` represents the location of the SQLite file. This must be a file that the client has permission to write to.

components/ads-client/src/ffi.rs

@@ -12,10 +12,13 @@ use crate::client::ad_response::{
     AdCallbacks, AdImage, AdSpoc, AdTile, SpocFrequencyCaps, SpocRanking,
 };
 use crate::client::config::{AdsCacheConfig, AdsClientConfig, Environment};
+use crate::client::AdsClient;
 use crate::error::ComponentError;
 use crate::ffi::telemetry::MozAdsTelemetryWrapper;
 use crate::http_cache::{CacheMode, RequestCachePolicy};
+use crate::MozAdsClient;
 use error_support::{ErrorHandling, GetErrorHandling};
+use parking_lot::Mutex;
 use url::Url;
 
 pub type AdsClientApiResult<T> = std::result::Result<T, MozAdsClientApiError>;
@@ -88,23 +91,58 @@ pub struct MozAdsCallbacks {
     pub report: Option<Url>,
 }
 
-#[derive(Default, uniffi::Record)]
-pub struct MozAdsClientConfig {
-    pub environment: MozAdsEnvironment,
-    pub cache_config: Option<MozAdsCacheConfig>,
-    pub telemetry: Option<Arc<dyn MozAdsTelemetry>>,
+#[derive(uniffi::Object)]
+pub struct MozAdsClientBuilder(Mutex<MozAdsClientBuilderInner>);
+
+#[derive(Default)]
+struct MozAdsClientBuilderInner {
+    environment: Option<MozAdsEnvironment>,
+    cache_config: Option<MozAdsCacheConfig>,
+    telemetry: Option<Arc<dyn MozAdsTelemetry>>,
 }
 
-impl From<MozAdsClientConfig> for AdsClientConfig<MozAdsTelemetryWrapper> {
-    fn from(config: MozAdsClientConfig) -> Self {
-        let telemetry = config
-            .telemetry
-            .map(MozAdsTelemetryWrapper::new)
-            .unwrap_or_else(MozAdsTelemetryWrapper::noop);
-        Self {
-            environment: config.environment.into(),
-            cache_config: config.cache_config.map(Into::into),
-            telemetry,
+impl Default for MozAdsClientBuilder {
+    fn default() -> Self {
+        Self(Mutex::new(MozAdsClientBuilderInner::default()))
+    }
+}
+
+#[uniffi::export]
+impl MozAdsClientBuilder {
+    #[uniffi::constructor]
+    pub fn new() -> Self {
+        Self::default()
+    }
+
+    pub fn environment(self: Arc<Self>, environment: MozAdsEnvironment) -> Arc<Self> {
+        self.0.lock().environment = Some(environment);
+        self
+    }
+
+    pub fn cache_config(self: Arc<Self>, cache_config: MozAdsCacheConfig) -> Arc<Self> {
+        self.0.lock().cache_config = Some(cache_config);
+        self
+    }
+
+    pub fn telemetry(self: Arc<Self>, telemetry: Arc<dyn MozAdsTelemetry>) -> Arc<Self> {
+        self.0.lock().telemetry = Some(telemetry);
+        self
+    }
+
+    pub fn build(&self) -> MozAdsClient {
+        let inner = self.0.lock();
+        let client_config = AdsClientConfig {
+            environment: inner.environment.unwrap_or_default().into(),
+            cache_config: inner.cache_config.clone().map(Into::into),
+            telemetry: inner
+                .telemetry
+                .clone()
+                .map(MozAdsTelemetryWrapper::new)
+                .unwrap_or_else(MozAdsTelemetryWrapper::noop),
+        };
+        let client = AdsClient::new(client_config);
+        MozAdsClient {
+            inner: Mutex::new(client),
         }
     }
 }
@@ -118,7 +156,7 @@ pub enum MozAdsEnvironment {
     Test,
 }
 
-#[derive(uniffi::Record)]
+#[derive(Clone, uniffi::Record)]
 pub struct MozAdsCacheConfig {
     pub db_path: String,
     pub default_cache_ttl_seconds: Option<u64>,

components/ads-client/src/lib.rs

@@ -23,7 +23,6 @@ pub mod telemetry;
 
 pub use ffi::*;
 
-use crate::client::config::AdsClientConfig;
 use crate::ffi::telemetry::MozAdsTelemetryWrapper;
 
 #[cfg(test)]
@@ -41,18 +40,9 @@ uniffi::custom_type!(AdsClientUrl, String, {
 pub struct MozAdsClient {
     inner: Mutex<AdsClient<MozAdsTelemetryWrapper>>,
 }
+
 #[uniffi::export]
 impl MozAdsClient {
-    #[uniffi::constructor]
-    pub fn new(client_config: Option<MozAdsClientConfig>) -> Self {
-        let client_config = client_config.unwrap_or_default();
-        let client_config: AdsClientConfig<MozAdsTelemetryWrapper> = client_config.into();
-        let client = AdsClient::new(client_config);
-        Self {
-            inner: Mutex::new(client),
-        }
-    }
-
     #[handle_error(ComponentError)]
     pub fn request_image_ads(
         &self,

components/ads-client/tests/integration_test.rs

@@ -7,7 +7,7 @@ use std::time::Duration;
 
 use ads_client::{
     http_cache::{ByteSize, CacheOutcome, HttpCache, RequestCachePolicy},
-    MozAdsClient, MozAdsPlacementRequest, MozAdsPlacementRequestWithCount,
+    MozAdsClientBuilder, MozAdsPlacementRequest, MozAdsPlacementRequestWithCount,
 };
 use url::Url;
 use viaduct::Request;
@@ -17,7 +17,7 @@ use viaduct::Request;
 fn test_mock_pocket_billboard_1_placement() {
     viaduct_dev::init_backend_dev();
 
-    let client = MozAdsClient::new(None);
+    let client = MozAdsClientBuilder::new().build();
 
     let placement_request = MozAdsPlacementRequest {
         placement_id: "mock_pocket_billboard_1".to_string(),
@@ -45,7 +45,7 @@ fn test_mock_pocket_billboard_1_placement() {
 fn test_newtab_spocs_placement() {
     viaduct_dev::init_backend_dev();
 
-    let client = MozAdsClient::new(None);
+    let client = MozAdsClientBuilder::new().build();
 
     let count = 3;
     let placement_request = MozAdsPlacementRequestWithCount {
@@ -81,7 +81,7 @@ fn test_newtab_spocs_placement() {
 fn test_newtab_tile_1_placement() {
     viaduct_dev::init_backend_dev();
 
-    let client = MozAdsClient::new(None);
+    let client = MozAdsClientBuilder::new().build();
 
     let placement_request = MozAdsPlacementRequest {
         placement_id: "newtab_tile_1".to_string(),