diff --git a/components/search/src/configuration_overrides_types.rs b/components/search/src/configuration_overrides_types.rs new file mode 100644 index 0000000000..eebe4a66c9 --- /dev/null +++ b/components/search/src/configuration_overrides_types.rs @@ -0,0 +1,40 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +//! This module defines the structures that we use for serde_json to parse +//! the search configuration overrides. + +use crate::JSONEngineUrls; +use serde::Deserialize; + +/// Represents search configuration overrides record. +#[derive(Debug, Deserialize, Clone)] +#[serde(rename_all = "camelCase")] +pub struct JSONOverridesRecord { + /// This is the identifier of the search engine in search-config-v2 that this + /// record will override. It may be extended by telemetry_suffix. + pub identifier: String, + + /// The partner code for the engine or variant. This will be inserted into + /// parameters which include '{partnerCode} + pub partner_code: String, + + /// Suffix that is appended to the search engine identifier following a + /// dash, i.e. `-`. There should always be a suffix + /// supplied if the partner code is different. + pub telemetry_suffix: Option, + + /// The url used for reporting clicks. + pub click_url: String, + + /// The URLs associated with the search engine. + //pub urls: JSONOverrideEngineUrls, + pub urls: JSONEngineUrls, +} + +/// Represents the search configuration overrides as received from remote settings. +#[derive(Debug, Deserialize)] +pub(crate) struct JSONSearchConfigurationOverrides { + pub data: Vec, +} diff --git a/components/search/src/error.rs b/components/search/src/error.rs index 80d260a7a8..d1888f2865 100644 --- a/components/search/src/error.rs +++ b/components/search/src/error.rs @@ -17,6 +17,8 @@ use remote_settings::RemoteSettingsError; pub enum Error { #[error("Search configuration not specified")] SearchConfigNotSpecified, + #[error("Search configuration overrides not specified")] + SearchConfigOverridesNotSpecified, #[error("No records received from remote settings")] SearchConfigNoRecords, #[error("JSON error: {0}")] diff --git a/components/search/src/filter.rs b/components/search/src/filter.rs index b87e7cecb4..e8a15c77c1 100644 --- a/components/search/src/filter.rs +++ b/components/search/src/filter.rs @@ -4,6 +4,7 @@ //! This module defines the functions for managing the filtering of the configuration. +use crate::configuration_overrides_types::JSONOverridesRecord; use crate::environment_matching::matches_user_environment; use crate::{ error::Error, JSONDefaultEnginesRecord, JSONEngineBase, JSONEngineRecord, JSONEngineUrl, @@ -94,6 +95,17 @@ impl SearchEngineDefinition { } } + fn merge_override(&mut self, override_record: &JSONOverridesRecord) { + self.partner_code = override_record.partner_code.clone(); + self.urls.merge(&override_record.urls); + if let Some(telemetry_suffix) = &override_record.telemetry_suffix { + self.telemetry_suffix = telemetry_suffix.clone(); + } + if self.click_url.is_none() { + self.click_url = Some(override_record.click_url.clone()); + } + } + pub(crate) fn from_configuration_details( identifier: &str, base: JSONEngineBase, @@ -111,6 +123,7 @@ impl SearchEngineDefinition { partner_code: base.partner_code.unwrap_or_default(), telemetry_suffix: String::new(), urls: base.urls.into(), + click_url: None, }; engine_definition.merge_variant(variant); @@ -132,6 +145,7 @@ pub(crate) trait Filter { fn filter_records( &self, user_environment: &SearchUserEnvironment, + overrides: Option>, ) -> Result; } @@ -139,6 +153,7 @@ impl Filter for Vec { fn filter_records( &self, user_environment: &SearchUserEnvironment, + _overrides: Option>, ) -> Result { let mut engines = Vec::new(); let mut default_engines_record = None; @@ -183,6 +198,7 @@ impl Filter for Vec { fn filter_records( &self, user_environment: &SearchUserEnvironment, + overrides: Option>, ) -> Result { let mut engines = Vec::new(); let mut default_engines_record = None; @@ -191,7 +207,10 @@ impl Filter for Vec { for record in self { match record { JSONSearchConfigurationRecords::Engine(engine) => { - let result = maybe_extract_engine_config(user_environment, engine.clone()); + let mut result = maybe_extract_engine_config(user_environment, engine.clone()); + if let Some(overrides_data) = &overrides { + apply_overrides(&mut result, overrides_data); + } engines.extend(result); } JSONSearchConfigurationRecords::DefaultEngines(default_engines) => { @@ -214,16 +233,30 @@ impl Filter for Vec { } } +fn apply_overrides( + engines: &mut Option, + overrides: &[JSONOverridesRecord], +) { + for override_record in overrides { + if let Some(engine) = engines + .iter_mut() + .find(|e| e.identifier == override_record.identifier) + { + engine.merge_override(override_record); + } + } +} pub(crate) fn filter_engine_configuration_impl( user_environment: SearchUserEnvironment, configuration: &impl Filter, + overrides: Option>, ) -> Result { let mut user_environment = user_environment.clone(); user_environment.locale = user_environment.locale.to_lowercase(); user_environment.region = user_environment.region.to_lowercase(); user_environment.version = user_environment.version.to_lowercase(); - let filtered_result = configuration.filter_records(&user_environment); + let filtered_result = configuration.filter_records(&user_environment, overrides); filtered_result.map(|result| { let (default_engine_id, default_private_engine_id) = determine_default_engines( @@ -442,7 +475,8 @@ mod tests { suggestions: None, trending: None, search_form: None - } + }, + click_url: None } ) } @@ -593,7 +627,8 @@ mod tests { }], search_term_param_name: None, }), - } + }, + click_url: None } ) } @@ -719,7 +754,8 @@ mod tests { }], search_term_param_name: None, }), - } + }, + click_url: None } ) } @@ -904,7 +940,8 @@ mod tests { }], search_term_param_name: None, }), - } + }, + click_url: None } ) } diff --git a/components/search/src/lib.rs b/components/search/src/lib.rs index be6925a82d..4abee70b6b 100644 --- a/components/search/src/lib.rs +++ b/components/search/src/lib.rs @@ -2,6 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +mod configuration_overrides_types; mod configuration_types; mod environment_matching; mod error; diff --git a/components/search/src/selector.rs b/components/search/src/selector.rs index 940a63bad9..9065c85868 100644 --- a/components/search/src/selector.rs +++ b/components/search/src/selector.rs @@ -4,6 +4,7 @@ //! This module defines the main `SearchEngineSelector`. +use crate::configuration_overrides_types::JSONSearchConfigurationOverrides; use crate::filter::filter_engine_configuration_impl; use crate::{ error::Error, JSONSearchConfiguration, RefinedSearchConfig, SearchApiResult, @@ -17,6 +18,7 @@ use std::sync::Arc; #[derive(Default)] pub(crate) struct SearchEngineSelectorInner { configuration: Option, + configuration_overrides: Option, search_config_client: Option>, } @@ -68,6 +70,15 @@ impl SearchEngineSelector { Ok(()) } + #[handle_error(Error)] + pub fn set_config_overrides(self: Arc, overrides: String) -> SearchApiResult<()> { + if overrides.is_empty() { + return Err(Error::SearchConfigOverridesNotSpecified); + } + self.0.lock().configuration_overrides = serde_json::from_str(&overrides)?; + Ok(()) + } + /// Clears the search configuration from memory if it is known that it is /// not required for a time, e.g. if the configuration will only be re-filtered /// after an app/environment update. @@ -92,16 +103,21 @@ impl SearchEngineSelector { if records.is_empty() { return Err(Error::SearchConfigNoRecords); } - return filter_engine_configuration_impl(user_environment, &records); + return filter_engine_configuration_impl(user_environment, &records, None); } else { return Err(Error::SearchConfigNoRecords); } } - let data = match &self.0.lock().configuration { + let config = match &self.0.lock().configuration { None => return Err(Error::SearchConfigNotSpecified), Some(configuration) => configuration.data.clone(), }; - return filter_engine_configuration_impl(user_environment, &data); + + let config_overrides = match &self.0.lock().configuration_overrides { + None => return Err(Error::SearchConfigOverridesNotSpecified), + Some(overrides) => overrides.data.clone(), + }; + return filter_engine_configuration_impl(user_environment, &config, Some(config_overrides)); } } @@ -270,6 +286,26 @@ mod tests { fn test_filter_engine_configuration_returns_basic_engines() { let selector = Arc::new(SearchEngineSelector::new()); + let config_overrides_result = Arc::clone(&selector).set_config_overrides( + json!({ + "data": [ + { + "identifier": "overrides-engine", + "partnerCode": "test-partner", + "clickUrl": "https://example.com/click", + "telemetrySuffix": "test-suffix", + "urls": { + "search": { + "base": "https://example.com/search?q={searchTerm}", + "method": "GET", + "params": [] + } + } + } + ] + }) + .to_string(), + ); let config_result = Arc::clone(&selector).set_search_config( json!({ "data": [ @@ -356,6 +392,11 @@ mod tests { "Should have set the configuration successfully. {:?}", config_result ); + assert!( + config_overrides_result.is_ok(), + "Should have set the configuration overrides successfully. {:?}", + config_overrides_result + ); let result = selector.filter_engine_configuration(SearchUserEnvironment { ..Default::default() @@ -445,7 +486,8 @@ mod tests { suggestions: None, trending: None, search_form: None - } + }, + click_url: None, } ), app_default_engine_id: Some("test1".to_string()), @@ -458,6 +500,26 @@ mod tests { fn test_filter_engine_configuration_handles_basic_variants() { let selector = Arc::new(SearchEngineSelector::new()); + let config_overrides_result = Arc::clone(&selector).set_config_overrides( + json!({ + "data": [ + { + "identifier": "overrides-engine", + "partnerCode": "test-partner", + "clickUrl": "https://example.com/click", + "telemetrySuffix": "test-suffix", + "urls": { + "search": { + "base": "https://example.com/search?q={searchTerm}", + "method": "GET", + "params": [] + } + } + } + ] + }) + .to_string(), + ); let config_result = Arc::clone(&selector).set_search_config( json!({ "data": [ @@ -558,6 +620,11 @@ mod tests { "Should have set the configuration successfully. {:?}", config_result ); + assert!( + config_overrides_result.is_ok(), + "Should have set the configuration overrides successfully. {:?}", + config_overrides_result + ); let result = selector.filter_engine_configuration(SearchUserEnvironment { region: "FR".into(), @@ -657,6 +724,26 @@ mod tests { fn test_filter_engine_configuration_handles_basic_subvariants() { let selector = Arc::new(SearchEngineSelector::new()); + let config_overrides_result = Arc::clone(&selector).set_config_overrides( + json!({ + "data": [ + { + "identifier": "overrides-engine", + "partnerCode": "test-partner", + "clickUrl": "https://example.com/click", + "telemetrySuffix": "test-suffix", + "urls": { + "search": { + "base": "https://example.com/search?q={searchTerm}", + "method": "GET", + "params": [] + } + } + } + ] + }) + .to_string(), + ); let config_result = Arc::clone(&selector).set_search_config( json!({ "data": [ @@ -755,6 +842,11 @@ mod tests { "Should have set the configuration successfully. {:?}", config_result ); + assert!( + config_overrides_result.is_ok(), + "Should have set the configuration overrides successfully. {:?}", + config_overrides_result + ); let mut result = Arc::clone(&selector).filter_engine_configuration(SearchUserEnvironment { region: "FR".into(), @@ -908,6 +1000,26 @@ mod tests { fn test_filter_engine_configuration_handles_environments() { let selector = Arc::new(SearchEngineSelector::new()); + let config_overrides_result = Arc::clone(&selector).set_config_overrides( + json!({ + "data": [ + { + "identifier": "overrides-engine", + "partnerCode": "test-partner", + "clickUrl": "https://example.com/click", + "telemetrySuffix": "test-suffix", + "urls": { + "search": { + "base": "https://example.com/search?q={searchTerm}", + "method": "GET", + "params": [] + } + } + } + ] + }) + .to_string(), + ); let config_result = Arc::clone(&selector).set_search_config( json!({ "data": [ @@ -984,6 +1096,11 @@ mod tests { "Should have set the configuration successfully. {:?}", config_result ); + assert!( + config_overrides_result.is_ok(), + "Should have set the configuration overrides successfully. {:?}", + config_overrides_result + ); let mut result = Arc::clone(&selector).filter_engine_configuration(SearchUserEnvironment { distribution_id: String::new(), @@ -1134,6 +1251,26 @@ mod tests { fn test_set_config_should_handle_default_engines() { let selector = Arc::new(SearchEngineSelector::new()); + let config_overrides_result = Arc::clone(&selector).set_config_overrides( + json!({ + "data": [ + { + "identifier": "overrides-engine", + "partnerCode": "test-partner", + "clickUrl": "https://example.com/click", + "telemetrySuffix": "test-suffix", + "urls": { + "search": { + "base": "https://example.com/search?q={searchTerm}", + "method": "GET", + "params": [] + } + } + } + ] + }) + .to_string(), + ); let config_result = Arc::clone(&selector).set_search_config( json!({ "data": [ @@ -1218,6 +1355,11 @@ mod tests { "Should have set the configuration successfully. {:?}", config_result ); + assert!( + config_overrides_result.is_ok(), + "Should have set the configuration overrides successfully. {:?}", + config_overrides_result + ); let test_engine = SearchEngineDefinition { charset: "UTF-8".to_string(), @@ -1320,6 +1462,26 @@ mod tests { fn test_filter_engine_orders() { let selector = Arc::new(SearchEngineSelector::new()); + let config_overrides_result = Arc::clone(&selector).set_config_overrides( + json!({ + "data": [ + { + "identifier": "overrides-engine", + "partnerCode": "test-partner", + "clickUrl": "https://example.com/click", + "telemetrySuffix": "test-suffix", + "urls": { + "search": { + "base": "https://example.com/search?q={searchTerm}", + "method": "GET", + "params": [] + } + } + } + ] + }) + .to_string(), + ); let engine_order_config = Arc::clone(&selector).set_search_config( json!({ "data": [ @@ -1444,6 +1606,11 @@ mod tests { "Should have set the configuration successfully. {:?}", engine_order_config ); + assert!( + config_overrides_result.is_ok(), + "Should have set the configuration overrides successfully. {:?}", + config_overrides_result + ); fn assert_actual_engines_equals_expected( result: Result, @@ -1911,4 +2078,163 @@ mod tests { ); m.expect(1).assert(); } + + #[test] + fn test_configuration_overrides_applied() { + let selector = Arc::new(SearchEngineSelector::new()); + + let config_result = Arc::clone(&selector).set_search_config( + json!({ + "data": [ + { + "recordType": "engine", + "identifier": "test", + "base": { + "name": "Test", + "classification": "general", + "urls": { + "search": { + "base": "https://example.com", + "method": "GET", + } + } + }, + "variants": [{ + "environment": { + "allRegionsAndLocales": true + } + }], + }, + { + "recordType": "engine", + "identifier": "distro-default", + "base": { + "name": "Distribution Default", + "classification": "general", + "urls": { + "search": { + "base": "https://example.com", + "method": "GET" + } + } + }, + "variants": [{ + "environment": { + "allRegionsAndLocales": true + }, + "telemetrySuffix": "distro-telemetry-suffix", + }], + }, + ] + }) + .to_string(), + ); + assert!( + config_result.is_ok(), + "Should have set the configuration successfully. {:?}", + config_result + ); + + let test_engine = SearchEngineDefinition { + charset: "UTF-8".to_string(), + classification: SearchEngineClassification::General, + identifier: "test".to_string(), + name: "Test".to_string(), + partner_code: "override-partner-code".to_string(), + telemetry_suffix: "override-telemetry-suffix".to_string(), + click_url: Some("https://example.com/click-url".to_string()), + urls: SearchEngineUrls { + search: SearchEngineUrl { + base: "https://override.com/search".to_string(), + method: "GET".to_string(), + params: vec![SearchUrlParam { + name: "override-name".to_string(), + value: Some("override-value".to_string()), + enterprise_value: None, + experiment_config: None, + }], + search_term_param_name: None, + }, + ..Default::default() + }, + ..Default::default() + }; + let distro_default_engine = SearchEngineDefinition { + charset: "UTF-8".to_string(), + classification: SearchEngineClassification::General, + identifier: "distro-default".to_string(), + name: "Distribution Default".to_string(), + partner_code: "override-partner-code".to_string(), + telemetry_suffix: "distro-telemetry-suffix".to_string(), + click_url: Some("https://example.com/click-url".to_string()), + urls: SearchEngineUrls { + search: SearchEngineUrl { + base: "https://override.com/search".to_string(), + method: "GET".to_string(), + params: Vec::new(), + search_term_param_name: None, + }, + ..Default::default() + }, + ..Default::default() + }; + + let config_overrides_result = Arc::clone(&selector).set_config_overrides( + json!({ + "data": [ + { + "identifier": "test", + "partnerCode": "override-partner-code", + "telemetrySuffix": "override-telemetry-suffix", + "clickUrl": "https://example.com/click-url", + "urls": { + "search": { + "base": "https://override.com/search", + "params": [{ + "name": "override-name", + "value": "override-value", + }], + }, + }, + }, + { // Test partial override with some missing fields + "identifier": "distro-default", + "partnerCode": "override-partner-code", + "clickUrl": "https://example.com/click-url", + "urls": { + "search": { + "base": "https://override.com/search", + }, + }, + } + ] + }) + .to_string(), + ); + + assert!( + config_overrides_result.is_ok(), + "Should have set the configuration overrides successfully. {:?}", + config_overrides_result + ); + + let result = Arc::clone(&selector).filter_engine_configuration(SearchUserEnvironment { + ..Default::default() + }); + assert!( + result.is_ok(), + "Should have filtered the configuration without error. {:?}", + result + ); + + assert_eq!( + result.unwrap(), + RefinedSearchConfig { + engines: vec![distro_default_engine.clone(), test_engine.clone(),], + app_default_engine_id: None, + app_private_default_engine_id: None + }, + "Should have applied the overrides to the matching engine." + ); + } } diff --git a/components/search/src/types.rs b/components/search/src/types.rs index 325b804fb0..a1dfe9e8e3 100644 --- a/components/search/src/types.rs +++ b/components/search/src/types.rs @@ -209,6 +209,9 @@ pub struct SearchEngineDefinition { /// If the number is not specified, other methods of sorting may be relied /// upon (e.g. alphabetical). pub order_hint: Option, + + ///The url used for reporting clicks. + pub click_url: Option, } /// Details of the search engines to display to the user, generated as a result