feat: feature gate jexl

mozilla · Nov 15, 2024 · ab36d7a · ab36d7a
1 parent 06a5d15
commit ab36d7a
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 21 deletions.
diff --git a/components/remote_settings/Cargo.toml b/components/remote_settings/Cargo.toml
@@ -8,6 +8,10 @@ description = "A Remote Settings client intended for application layer platforms
 license = "MPL-2.0" 
 exclude = ["/android", "/ios"]
 
+[features]
+default = []
+jexl = ["dep:jexl-eval"]
+
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
@@ -22,7 +26,7 @@ viaduct = { path = "../viaduct" }
 url = "2.1" # mozilla-central can't yet take 2.2 (see bug 1734538)
 camino = "1.0"
 rusqlite = { workspace = true, features = ["bundled"] }
-jexl-eval = "0.3.0"
+jexl-eval = { version = "0.3.0", optional = true }
 regex = "1.9"
 anyhow = "1.0"
 firefox-versioning = { path = "../support/firefox-versioning" }

diff --git a/components/remote_settings/src/client.rs b/components/remote_settings/src/client.rs
@@ -4,9 +4,12 @@
 
 use crate::config::RemoteSettingsConfig;
 use crate::error::{Error, Result};
+#[cfg(feature = "jexl")]
 use crate::jexl_filter::JexlFilter;
 use crate::storage::Storage;
-use crate::{RemoteSettingsContext, RemoteSettingsServer, UniffiCustomTypeConverter};
+#[cfg(feature = "jexl")]
+use crate::RemoteSettingsContext;
+use crate::{RemoteSettingsServer, UniffiCustomTypeConverter};
 use parking_lot::Mutex;
 use serde::{Deserialize, Serialize};
 use std::{
@@ -27,6 +30,7 @@ const HEADER_RETRY_AFTER: &str = "Retry-After";
 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>>,
 }
@@ -40,11 +44,12 @@ impl<C: ApiClient> RemoteSettingsClient<C> {
     pub fn new_from_parts(
         collection_name: String,
         storage: Storage,
-        jexl_filter: JexlFilter,
+        #[cfg(feature = "jexl")] jexl_filter: JexlFilter,
         api_client: C,
     ) -> Self {
         Self {
             collection_name,
+            #[cfg(feature = "jexl")]
             jexl_filter,
             inner: Mutex::new(RemoteSettingsClientInner {
                 storage,
@@ -57,6 +62,7 @@ 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> {
         records
             .into_iter()
@@ -69,6 +75,11 @@ impl<C: ApiClient> RemoteSettingsClient<C> {
             .collect()
     }
 
+    #[cfg(not(feature = "jexl"))]
+    fn filter_records(&self, records: Vec<RemoteSettingsRecord>) -> Vec<RemoteSettingsRecord> {
+        records
+    }
+
     /// Get the current set of records.
     ///
     /// If records are not present in storage this will normally return None.  Use `sync_if_empty =
@@ -125,15 +136,17 @@ impl RemoteSettingsClient<ViaductApiClient> {
         server_url: Url,
         bucket_name: String,
         collection_name: String,
-        context: Option<RemoteSettingsContext>,
+        #[cfg(feature = "jexl")] context: Option<RemoteSettingsContext>,
         storage: Storage,
     ) -> Result<Self> {
         let api_client = ViaductApiClient::new(server_url, &bucket_name, &collection_name)?;
+        #[cfg(feature = "jexl")]
         let jexl_filter = JexlFilter::new(context);
 
         Ok(Self::new_from_parts(
             collection_name,
             storage,
+            #[cfg(feature = "jexl")]
             jexl_filter,
             api_client,
         ))
@@ -1510,6 +1523,7 @@ mod test {
 mod test_new_client {
     use super::*;
 
+    #[cfg(not(feature = "jexl"))]
     use serde_json::json;
 
     #[test]
@@ -1536,6 +1550,7 @@ mod test_new_client {
     }
 
     #[test]
+    #[cfg(not(feature = "jexl"))]
     fn test_get_records_none_cached() {
         let mut api_client = MockApiClient::new();
         api_client.expect_collection_url().returning(|| {
@@ -1544,19 +1559,16 @@ mod test_new_client {
         // Note, don't make any api_client.expect_*() calls, the RemoteSettingsClient should not
         // attempt to make any requests for this scenario
         let storage = Storage::new(":memory:".into()).expect("Error creating storage");
-        let rs_client = RemoteSettingsClient::new_from_parts(
-            "test-collection".into(),
-            storage,
-            JexlFilter::new(None),
-            api_client,
-        );
+        let rs_client =
+            RemoteSettingsClient::new_from_parts("test-collection".into(), storage, api_client);
         assert_eq!(
             rs_client.get_records(false).expect("Error getting records"),
             None
         );
     }
 
     #[test]
+    #[cfg(not(feature = "jexl"))]
     fn test_get_records_none_cached_sync_with_empty() {
         let mut api_client = MockApiClient::new();
         let records = vec![RemoteSettingsRecord {
@@ -1577,23 +1589,19 @@ mod test_new_client {
             }
         });
         let storage = Storage::new(":memory:".into()).expect("Error creating storage");
-        let rs_client = RemoteSettingsClient::new_from_parts(
-            "test-collection".into(),
-            storage,
-            JexlFilter::new(None),
-            api_client,
-        );
+        let rs_client =
+            RemoteSettingsClient::new_from_parts("test-collection".into(), storage, api_client);
         assert_eq!(
             rs_client.get_records(true).expect("Error getting records"),
             Some(records)
         );
     }
 }
 
+#[cfg(feature = "jexl")]
 #[cfg(test)]
-mod test_filtering_records {
+mod jexl_tests {
     use super::*;
-    use serde_json::json;
 
     #[test]
     fn test_get_records_filtered_app_version_pass() {
@@ -1603,7 +1611,7 @@ mod test_filtering_records {
             last_modified: 100,
             deleted: false,
             attachment: None,
-            fields: json!({
+            fields: serde_json::json!({
                 "filter_expression": "env.version|versionCompare(\"128.0a1\") > 0"
             })
             .as_object()
@@ -1652,7 +1660,7 @@ mod test_filtering_records {
             last_modified: 100,
             deleted: false,
             attachment: None,
-            fields: json!({
+            fields: serde_json::json!({
                 "filter_expression": "env.version|versionCompare(\"128.0a1\") > 0"
             })
             .as_object()

diff --git a/components/remote_settings/src/lib.rs b/components/remote_settings/src/lib.rs
@@ -16,6 +16,7 @@ pub mod error;
 pub mod service;
 pub mod storage;
 
+#[cfg(feature = "jexl")]
 pub(crate) mod jexl_filter;
 
 pub use client::{Attachment, RemoteSettingsRecord, RemoteSettingsResponse, RsJsonObject};
@@ -209,14 +210,15 @@ impl RemoteSettingsClient {
         base_url: Url,
         bucket_name: String,
         collection_name: String,
-        context: Option<RemoteSettingsContext>,
+        #[cfg(feature = "jexl")] context: Option<RemoteSettingsContext>,
         storage: Storage,
     ) -> Result<Self> {
         Ok(Self {
             internal: client::RemoteSettingsClient::new(
                 base_url,
                 bucket_name,
                 collection_name,
+                #[cfg(feature = "jexl")]
                 context,
                 storage,
             )?,

diff --git a/components/remote_settings/src/service.rs b/components/remote_settings/src/service.rs
@@ -56,6 +56,7 @@ impl RemoteSettingsService {
     }
 
     /// Create a new Remote Settings client
+    #[cfg(feature = "jexl")]
     pub fn make_client(
         &self,
         collection_name: String,
@@ -74,6 +75,24 @@ impl RemoteSettingsService {
         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();
+        let storage = 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(),
+            storage,
+        )?);
+        inner.clients.push(Arc::downgrade(&client));
+        Ok(client)
+    }
+
     /// Sync collections for all active clients
     pub fn sync(&self) -> Result<Vec<String>> {
         // Make sure we only sync each collection once, even if there are multiple clients