Move app_context to RemoteSettingsService

Author: bendk

components/remote_settings/src/client.rs

@@ -74,14 +74,14 @@ struct CollectionData {
 pub struct RemoteSettingsClient<C = ViaductApiClient> {
     // This is immutable, so it can be outside the mutex
     collection_name: String,
-    #[cfg(feature = "jexl")]
-    jexl_filter: JexlFilter,
     inner: Mutex<RemoteSettingsClientInner<C>>,
 }
 
 struct RemoteSettingsClientInner<C> {
     storage: Storage,
     api_client: C,
+    #[cfg(feature = "jexl")]
+    jexl_filter: JexlFilter,
 }
 
 // Add your local packaged data you want to work with here
@@ -123,11 +123,11 @@ impl<C: ApiClient> RemoteSettingsClient<C> {
     ) -> Self {
         Self {
             collection_name,
-            #[cfg(feature = "jexl")]
-            jexl_filter,
             inner: Mutex::new(RemoteSettingsClientInner {
                 storage,
                 api_client,
+                #[cfg(feature = "jexl")]
+                jexl_filter,
             }),
         }
     }
@@ -149,20 +149,28 @@ impl<C: ApiClient> RemoteSettingsClient<C> {
 
     /// Filters records based on the presence and evaluation of `filter_expression`.
     #[cfg(feature = "jexl")]
-    fn filter_records(&self, records: Vec<RemoteSettingsRecord>) -> Vec<RemoteSettingsRecord> {
+    fn filter_records(
+        &self,
+        records: Vec<RemoteSettingsRecord>,
+        inner: &RemoteSettingsClientInner<C>,
+    ) -> Vec<RemoteSettingsRecord> {
         records
             .into_iter()
             .filter(|record| match record.fields.get("filter_expression") {
                 Some(serde_json::Value::String(filter_expr)) => {
-                    self.jexl_filter.evaluate(filter_expr).unwrap_or(false)
+                    inner.jexl_filter.evaluate(filter_expr).unwrap_or(false)
                 }
                 _ => true, // Include records without a valid filter expression by default
             })
             .collect()
     }
 
     #[cfg(not(feature = "jexl"))]
-    fn filter_records(&self, records: Vec<RemoteSettingsRecord>) -> Vec<RemoteSettingsRecord> {
+    fn filter_records(
+        &self,
+        records: Vec<RemoteSettingsRecord>,
+        _inner: &RemoteSettingsClientInner<C>,
+    ) -> Vec<RemoteSettingsRecord> {
         records
     }
 
@@ -199,7 +207,7 @@ impl<C: ApiClient> RemoteSettingsClient<C> {
                     packaged_data.timestamp,
                     CollectionMetadata::default(),
                 )?;
-                return Ok(Some(self.filter_records(packaged_data.data)));
+                return Ok(Some(self.filter_records(packaged_data.data, &inner)));
             }
         }
 
@@ -210,7 +218,7 @@ impl<C: ApiClient> RemoteSettingsClient<C> {
             //
             // Note: we should return these even if it's an empty list and `sync_if_empty=true`.
             // The "if empty" part refers to the cache being empty, not the list.
-            (Some(cached_records), _) => Some(self.filter_records(cached_records)),
+            (Some(cached_records), _) => Some(self.filter_records(cached_records, &inner)),
             // Case 3: sync_if_empty=true
             (None, true) => {
                 let changeset = inner.api_client.fetch_changeset(None)?;
@@ -220,7 +228,7 @@ impl<C: ApiClient> RemoteSettingsClient<C> {
                     changeset.timestamp,
                     changeset.metadata,
                 )?;
-                Some(self.filter_records(changeset.changes))
+                Some(self.filter_records(changeset.changes, &inner))
             }
             // Case 4: Nothing to return
             (None, false) => None,

components/remote_settings/src/config.rs

@@ -10,7 +10,7 @@
 
 use url::Url;
 
-use crate::{ApiResult, Error, Result};
+use crate::{ApiResult, Error, RemoteSettingsContext, Result};
 
 /// Remote settings configuration
 ///
@@ -25,6 +25,9 @@ pub struct RemoteSettingsConfig2 {
     /// Bucket name to use, defaults to "main".  Use "main-preview" for a preview bucket
     #[uniffi(default = None)]
     pub bucket_name: Option<String>,
+    /// App context to use for JEXL filtering (when the `jexl` feature is present).
+    #[uniffi(default = None)]
+    pub app_context: Option<RemoteSettingsContext>,
 }
 
 /// Custom configuration for the client.

components/remote_settings/src/lib.rs

@@ -103,12 +103,8 @@ impl RemoteSettingsService {
 
     /// Create a new Remote Settings client
     #[handle_error(Error)]
-    pub fn make_client(
-        &self,
-        collection_name: String,
-        app_context: Option<RemoteSettingsContext>,
-    ) -> ApiResult<Arc<RemoteSettingsClient>> {
-        self.internal.make_client(collection_name, app_context)
+    pub fn make_client(&self, collection_name: String) -> ApiResult<Arc<RemoteSettingsClient>> {
+        self.internal.make_client(collection_name)
     }
 
     /// Sync collections for all active clients
@@ -212,7 +208,7 @@ impl RemoteSettingsClient {
         base_url: Url,
         bucket_name: String,
         collection_name: String,
-        #[cfg(feature = "jexl")] context: Option<RemoteSettingsContext>,
+        #[allow(unused)] context: Option<RemoteSettingsContext>,
         storage: Storage,
     ) -> Result<Self> {
         Ok(Self {

components/remote_settings/src/service.rs

@@ -25,6 +25,7 @@ struct RemoteSettingsServiceInner {
     storage_dir: Utf8PathBuf,
     base_url: Url,
     bucket_name: String,
+    app_context: Option<RemoteSettingsContext>,
     /// Weakrefs for all clients that we've created.  Note: this stores the
     /// top-level/public `RemoteSettingsClient` structs rather than `client::RemoteSettingsClient`.
     /// The reason for this is that we return Arcs to the public struct to the foreign code, so we
@@ -50,18 +51,13 @@ impl RemoteSettingsService {
                 storage_dir,
                 base_url,
                 bucket_name,
+                app_context: config.app_context,
                 clients: vec![],
             }),
         })
     }
 
-    /// Create a new Remote Settings client
-    #[cfg(feature = "jexl")]
-    pub fn make_client(
-        &self,
-        collection_name: String,
-        context: Option<RemoteSettingsContext>,
-    ) -> Result<Arc<RemoteSettingsClient>> {
+    pub fn make_client(&self, collection_name: String) -> Result<Arc<RemoteSettingsClient>> {
         let mut inner = self.inner.lock();
         // Allow using in-memory databases for testing of external crates.
         let storage = if inner.storage_dir == ":memory:" {
@@ -74,32 +70,7 @@ impl RemoteSettingsService {
             inner.base_url.clone(),
             inner.bucket_name.clone(),
             collection_name.clone(),
-            context,
-            storage,
-        )?);
-        inner.clients.push(Arc::downgrade(&client));
-        Ok(client)
-    }
-
-    #[cfg(not(feature = "jexl"))]
-    pub fn make_client(
-        &self,
-        collection_name: String,
-        #[allow(unused_variables)] context: Option<RemoteSettingsContext>,
-    ) -> Result<Arc<RemoteSettingsClient>> {
-        let mut inner = self.inner.lock();
-        // Allow using in-memory databases for testing of external crates.
-        let storage = if inner.storage_dir == ":memory:" {
-            Storage::new(Utf8PathBuf::from(
-                "file:remote-settings-{collection-name}?mode=memory&cache=shared".to_string(),
-            ))?
-        } else {
-            Storage::new(inner.storage_dir.join(format!("{collection_name}.sql")))?
-        };
-        let client = Arc::new(RemoteSettingsClient::new(
-            inner.base_url.clone(),
-            inner.bucket_name.clone(),
-            collection_name.clone(),
+            inner.app_context.clone(),
             storage,
         )?);
         inner.clients.push(Arc::downgrade(&client));

components/search/src/selector.rs

@@ -11,7 +11,7 @@ use crate::{
 };
 use error_support::handle_error;
 use parking_lot::Mutex;
-use remote_settings::{RemoteSettingsClient, RemoteSettingsContext, RemoteSettingsService};
+use remote_settings::{RemoteSettingsClient, RemoteSettingsService};
 use std::sync::Arc;
 
 #[derive(Default)]
@@ -47,11 +47,10 @@ impl SearchEngineSelector {
     pub fn use_remote_settings_server(
         self: Arc<Self>,
         service: &Arc<RemoteSettingsService>,
-        options: Option<RemoteSettingsContext>,
         #[allow(unused_variables)] apply_engine_overrides: bool,
     ) -> SearchApiResult<()> {
         self.0.lock().search_config_client =
-            Some(service.make_client("search-config-v2".to_string(), options)?);
+            Some(service.make_client("search-config-v2".to_string())?);
         Ok(())
     }
 
@@ -1631,19 +1630,14 @@ mod tests {
                 url: mockito::server_url(),
             }),
             bucket_name: Some(String::from("main")),
+            app_context: Some(RemoteSettingsContext::default()),
         };
         let service =
             Arc::new(RemoteSettingsService::new(String::from(":memory:"), config).unwrap());
 
         let selector = Arc::new(SearchEngineSelector::new());
 
-        let settings_result = Arc::clone(&selector).use_remote_settings_server(
-            &service,
-            Some(RemoteSettingsContext {
-                ..Default::default()
-            }),
-            false,
-        );
+        let settings_result = Arc::clone(&selector).use_remote_settings_server(&service, false);
         assert!(
             settings_result.is_ok(),
             "Should have set the client successfully. {:?}",

examples/remote-settings-cli/src/main.rs

@@ -116,6 +116,7 @@ fn build_service(cli: &Cli) -> Result<RemoteSettingsService> {
             RemoteSettingsServerArg::Prod => RemoteSettingsServer::Prod,
         }),
         bucket_name: cli.bucket.clone(),
+        app_context: None,
     };
     let storage_dir = cli
         .storage_dir
@@ -128,7 +129,7 @@ fn sync(service: RemoteSettingsService, collections: Vec<String>) -> Result<()>
     // Create a bunch of clients so that sync() syncs their collections
     let _clients = collections
         .into_iter()
-        .map(|collection| Ok(service.make_client(collection, None)?))
+        .map(|collection| Ok(service.make_client(collection)?))
         .collect::<Result<Vec<_>>>()?;
     service.sync()?;
     Ok(())
@@ -139,7 +140,7 @@ fn get_records(
     collection: String,
     sync_if_empty: bool,
 ) -> Result<()> {
-    let client = service.make_client(collection, None)?;
+    let client = service.make_client(collection)?;
     match client.get_records(sync_if_empty) {
         Some(records) => {
             for record in records {