From 16fa7288f90695b2183cdf9ff763aa13c69dbcc0 Mon Sep 17 00:00:00 2001 From: Mark Banner Date: Mon, 25 Nov 2024 22:14:15 +0000 Subject: [PATCH] Bug 1930984 - Implement default search engine selection on the search engine selector. --- Cargo.lock | 1 + components/search/Cargo.toml | 3 + components/search/src/configuration_types.rs | 33 +- components/search/src/filter.rs | 602 ++++++++++++++++++- components/search/src/selector.rs | 218 ++++++- components/search/src/types.rs | 10 +- 6 files changed, 845 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dd6a152949..9835ed5a38 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4505,6 +4505,7 @@ version = "0.1.0" dependencies = [ "error-support", "firefox-versioning", + "once_cell", "parking_lot", "serde", "serde_json", diff --git a/components/search/Cargo.toml b/components/search/Cargo.toml index 4d8b4503f2..527e669c19 100644 --- a/components/search/Cargo.toml +++ b/components/search/Cargo.toml @@ -17,3 +17,6 @@ firefox-versioning = { path = "../support/firefox-versioning" } [build-dependencies] uniffi = { version = "0.28.2", features = ["build"] } + +[dev-dependencies] +once_cell = "1.18.0" diff --git a/components/search/src/configuration_types.rs b/components/search/src/configuration_types.rs index fadaf4f6d8..297a2f045f 100644 --- a/components/search/src/configuration_types.rs +++ b/components/search/src/configuration_types.rs @@ -182,12 +182,43 @@ pub(crate) struct JSONEngineRecord { pub variants: Vec, } +#[derive(Debug, Deserialize, Clone)] +#[serde(rename_all = "camelCase")] +pub(crate) struct JSONSpecificDefaultRecord { + /// The identifier of the engine that will be used as the application default + /// for the associated environment. If the entry is suffixed with a star, + /// matching is applied on a "starts with" basis. + #[serde(default)] + pub default: String, + + /// The identifier of the engine that will be used as the application default + /// in private mode for the associated environment. If the entry is suffixed + /// with a star, matching is applied on a "starts with" basis. + #[serde(default)] + pub default_private: String, + + /// The specific environment to match for this record. + pub environment: JSONVariantEnvironment, +} + /// Represents the default engines record. #[derive(Debug, Deserialize, Clone)] #[serde(rename_all = "camelCase")] pub(crate) struct JSONDefaultEnginesRecord { + /// The identifier of the engine that will be used as the application default + /// if no other engines are specified as default. pub global_default: String, - pub global_default_private: Option, + + /// The identifier of the engine that will be used as the application default + /// in private mode if no other engines are specified as default. + #[serde(default)] + pub global_default_private: String, + + /// The specific environment filters to set a different default engine. The + /// array is ordered, when multiple entries match on environments, the later + /// entry will override earlier entries. + #[serde(default)] + pub specific_defaults: Vec, } /// Represents the engine orders record. diff --git a/components/search/src/filter.rs b/components/search/src/filter.rs index aefe9b0ddc..cff832cbc8 100644 --- a/components/search/src/filter.rs +++ b/components/search/src/filter.rs @@ -6,9 +6,9 @@ use crate::environment_matching::matches_user_environment; use crate::{ - error::Error, JSONEngineBase, JSONEngineRecord, JSONEngineUrl, JSONEngineUrls, - JSONSearchConfigurationRecords, RefinedSearchConfig, SearchEngineDefinition, SearchEngineUrl, - SearchEngineUrls, SearchUserEnvironment, + error::Error, JSONDefaultEnginesRecord, JSONEngineBase, JSONEngineRecord, JSONEngineUrl, + JSONEngineUrls, JSONSearchConfigurationRecords, RefinedSearchConfig, SearchEngineDefinition, + SearchEngineUrl, SearchEngineUrls, SearchUserEnvironment, }; impl From for SearchEngineUrl { @@ -56,14 +56,14 @@ pub(crate) fn filter_engine_configuration( configuration: Vec, ) -> Result { let mut engines = Vec::new(); - let mut default_engine_id: Option = None; - let mut default_private_engine_id: Option = None; 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 mut default_engines_record = None; + for record in configuration { match record { JSONSearchConfigurationRecords::Engine(engine) => { @@ -71,8 +71,7 @@ pub(crate) fn filter_engine_configuration( engines.extend(result); } JSONSearchConfigurationRecords::DefaultEngines(default_engines) => { - default_engine_id = Some(default_engines.global_default); - default_private_engine_id.clone_from(&default_engines.global_default_private); + default_engines_record = Some(default_engines); } JSONSearchConfigurationRecords::EngineOrders(_engine_orders) => { // TODO: Implementation. @@ -83,10 +82,13 @@ pub(crate) fn filter_engine_configuration( } } + let (default_engine_id, default_private_engine_id) = + determine_default_engines(&engines, default_engines_record, &user_environment); + Ok(RefinedSearchConfig { engines, - app_default_engine_id: default_engine_id.unwrap(), - app_default_private_engine_id: default_private_engine_id, + app_default_engine_id: default_engine_id, + app_private_default_engine_id: default_private_engine_id, }) } @@ -108,11 +110,90 @@ fn maybe_extract_engine_config( .map(|_variant| SearchEngineDefinition::from_configuration_details(&identifier, base)) } +fn determine_default_engines( + engines: &[SearchEngineDefinition], + default_engines_record: Option, + user_environment: &SearchUserEnvironment, +) -> (Option, Option) { + match default_engines_record { + None => (None, None), + Some(record) => { + let mut default_engine_id = None; + let mut default_engine_private_id = None; + + let specific_default = record + .specific_defaults + .into_iter() + .rev() + .find(|r| matches_user_environment(&r.environment, user_environment)); + + if let Some(specific_default) = specific_default { + // Check the engine is present in the list of engines before + // we return it as default. + if let Some(engine_id) = find_engine_with_match(engines, specific_default.default) { + default_engine_id.replace(engine_id); + } + if let Some(private_engine_id) = + find_engine_with_match(engines, specific_default.default_private) + { + default_engine_private_id.replace(private_engine_id); + } + } + + ( + // If we haven't found a default engine in a specific default, + // then fall back to the global default engine - but only if that + // exists in the engine list. + // + // For the normal mode engine (`default_engine_id`), this would + // effectively be considered an error. However, we can't do anything + // sensible here, so we will return `None` to the application, and + // that can handle it. + default_engine_id.or_else(|| find_engine_id(engines, record.global_default)), + default_engine_private_id + .or_else(|| find_engine_id(engines, record.global_default_private)), + ) + } + } +} + +fn find_engine_id(engines: &[SearchEngineDefinition], engine_id: String) -> Option { + if engine_id.is_empty() { + return None; + } + match engines.iter().any(|e| e.identifier == engine_id) { + true => Some(engine_id.clone()), + false => None, + } +} + +fn find_engine_with_match( + engines: &[SearchEngineDefinition], + engine_id_match: String, +) -> Option { + if engine_id_match.is_empty() { + return None; + } + if let Some(match_no_star) = engine_id_match.strip_suffix('*') { + return engines + .iter() + .find(|e| e.identifier.starts_with(match_no_star)) + .map(|e| e.identifier.clone()); + } + + engines + .iter() + .find(|e| e.identifier == engine_id_match) + .map(|e| e.identifier.clone()) +} + #[cfg(test)] mod tests { use std::vec; + use super::*; use crate::*; + use once_cell::sync::Lazy; #[test] fn test_from_configuration_details_fallsback_to_defaults() { @@ -253,4 +334,507 @@ mod tests { } ) } + + static ENGINES_LIST: Lazy> = Lazy::new(|| { + vec![ + SearchEngineDefinition { + aliases: Vec::new(), + charset: String::from("UTF-8"), + classification: SearchEngineClassification::General, + identifier: "engine1".to_string(), + partner_code: String::new(), + name: "Test".to_string(), + order_hint: None, + telemetry_suffix: None, + urls: SearchEngineUrls { + search: SearchEngineUrl { + base: "https://example.com".to_string(), + method: "GET".to_string(), + params: Vec::new(), + search_term_param_name: None, + }, + suggestions: None, + trending: None, + }, + }, + SearchEngineDefinition { + aliases: Vec::new(), + charset: "UTF-8".to_string(), + classification: SearchEngineClassification::General, + identifier: "engine2".to_string(), + partner_code: String::new(), + name: "Test 2".to_string(), + order_hint: None, + telemetry_suffix: None, + urls: SearchEngineUrls { + search: SearchEngineUrl { + base: "https://example.com/2".to_string(), + method: "GET".to_string(), + params: Vec::new(), + search_term_param_name: None, + }, + suggestions: None, + trending: None, + }, + }, + SearchEngineDefinition { + aliases: Vec::new(), + charset: "UTF-8".to_string(), + classification: SearchEngineClassification::General, + identifier: "engine3".to_string(), + partner_code: String::new(), + name: "Test 3".to_string(), + order_hint: None, + telemetry_suffix: None, + urls: SearchEngineUrls { + search: SearchEngineUrl { + base: "https://example.com/3".to_string(), + method: "GET".to_string(), + params: Vec::new(), + search_term_param_name: None, + }, + suggestions: None, + trending: None, + }, + }, + SearchEngineDefinition { + aliases: Vec::new(), + charset: "UTF-8".to_string(), + classification: SearchEngineClassification::General, + identifier: "engine4wildcardmatch".to_string(), + partner_code: String::new(), + name: "Test 4".to_string(), + order_hint: None, + telemetry_suffix: None, + urls: SearchEngineUrls { + search: SearchEngineUrl { + base: "https://example.com/4".to_string(), + method: "GET".to_string(), + params: Vec::new(), + search_term_param_name: None, + }, + suggestions: None, + trending: None, + }, + }, + ] + }); + + #[test] + fn test_determine_default_engines_returns_global_default() { + let (default_engine_id, default_engine_private_id) = determine_default_engines( + &ENGINES_LIST, + Some(JSONDefaultEnginesRecord { + global_default: "engine2".to_string(), + global_default_private: String::new(), + specific_defaults: Vec::new(), + }), + &SearchUserEnvironment { + locale: "fi".into(), + region: "FR".into(), + update_channel: SearchUpdateChannel::Default, + distribution_id: String::new(), + experiment: String::new(), + app_name: SearchApplicationName::Firefox, + version: String::new(), + device_type: SearchDeviceType::None, + }, + ); + + assert_eq!( + default_engine_id.unwrap(), + "engine2", + "Should have returned the global default engine" + ); + assert!( + default_engine_private_id.is_none(), + "Should not have returned an id for the private engine" + ); + + let (default_engine_id, default_engine_private_id) = determine_default_engines( + &ENGINES_LIST, + Some(JSONDefaultEnginesRecord { + global_default: "engine2".to_string(), + global_default_private: String::new(), + specific_defaults: vec![JSONSpecificDefaultRecord { + default: "engine1".to_string(), + default_private: String::new(), + environment: JSONVariantEnvironment { + all_regions_and_locales: false, + excluded_locales: vec![], + excluded_regions: vec![], + locales: vec!["en-GB".to_string()], + regions: vec![], + distributions: vec![], + excluded_distributions: vec![], + applications: vec![], + channels: vec![], + device_type: vec![], + min_version: "".to_string(), + max_version: "".to_string(), + experiment: "".to_string(), + }, + }], + }), + &SearchUserEnvironment { + locale: "fi".into(), + region: "FR".into(), + update_channel: SearchUpdateChannel::Default, + distribution_id: String::new(), + experiment: String::new(), + app_name: SearchApplicationName::Firefox, + version: String::new(), + device_type: SearchDeviceType::None, + }, + ); + + assert_eq!( + default_engine_id.unwrap(), + "engine2", + "Should have returned the global default engine when no specific defaults environments match" + ); + assert!( + default_engine_private_id.is_none(), + "Should not have returned an id for the private engine" + ); + + let (default_engine_id, default_engine_private_id) = determine_default_engines( + &ENGINES_LIST, + Some(JSONDefaultEnginesRecord { + global_default: "engine2".to_string(), + global_default_private: String::new(), + specific_defaults: vec![JSONSpecificDefaultRecord { + default: "engine1".to_string(), + default_private: String::new(), + environment: JSONVariantEnvironment { + all_regions_and_locales: false, + excluded_locales: vec![], + excluded_regions: vec![], + locales: vec!["fi".to_string()], + regions: vec![], + distributions: vec![], + excluded_distributions: vec![], + applications: vec![], + channels: vec![], + device_type: vec![], + min_version: "".to_string(), + max_version: "".to_string(), + experiment: "".to_string(), + }, + }], + }), + &SearchUserEnvironment { + locale: "fi".into(), + region: "FR".into(), + update_channel: SearchUpdateChannel::Default, + distribution_id: String::new(), + experiment: String::new(), + app_name: SearchApplicationName::Firefox, + version: String::new(), + device_type: SearchDeviceType::None, + }, + ); + + assert_eq!( + default_engine_id.unwrap(), + "engine1", + "Should have returned the specific default when environments match" + ); + assert!( + default_engine_private_id.is_none(), + "Should not have returned an id for the private engine" + ); + + let (default_engine_id, default_engine_private_id) = determine_default_engines( + &ENGINES_LIST, + Some(JSONDefaultEnginesRecord { + global_default: "engine2".to_string(), + global_default_private: String::new(), + specific_defaults: vec![JSONSpecificDefaultRecord { + default: "engine4*".to_string(), + default_private: String::new(), + environment: JSONVariantEnvironment { + all_regions_and_locales: false, + excluded_locales: vec![], + excluded_regions: vec![], + locales: vec!["fi".to_string()], + regions: vec![], + distributions: vec![], + excluded_distributions: vec![], + applications: vec![], + channels: vec![], + device_type: vec![], + min_version: "".to_string(), + max_version: "".to_string(), + experiment: "".to_string(), + }, + }], + }), + &SearchUserEnvironment { + locale: "fi".into(), + region: "FR".into(), + update_channel: SearchUpdateChannel::Default, + distribution_id: String::new(), + experiment: String::new(), + app_name: SearchApplicationName::Firefox, + version: String::new(), + device_type: SearchDeviceType::None, + }, + ); + + assert_eq!( + default_engine_id.unwrap(), + "engine4wildcardmatch", + "Should have returned the specific default when using a wildcard match" + ); + assert!( + default_engine_private_id.is_none(), + "Should not have returned an id for the private engine" + ); + + let (default_engine_id, default_engine_private_id) = determine_default_engines( + &ENGINES_LIST, + Some(JSONDefaultEnginesRecord { + global_default: "engine2".to_string(), + global_default_private: String::new(), + specific_defaults: vec![ + JSONSpecificDefaultRecord { + default: "engine4*".to_string(), + default_private: String::new(), + environment: JSONVariantEnvironment { + all_regions_and_locales: false, + excluded_locales: vec![], + excluded_regions: vec![], + locales: vec!["fi".to_string()], + regions: vec![], + distributions: vec![], + excluded_distributions: vec![], + applications: vec![], + channels: vec![], + device_type: vec![], + min_version: "".to_string(), + max_version: "".to_string(), + experiment: "".to_string(), + }, + }, + JSONSpecificDefaultRecord { + default: "engine3".to_string(), + default_private: String::new(), + environment: JSONVariantEnvironment { + all_regions_and_locales: false, + excluded_locales: vec![], + excluded_regions: vec![], + locales: vec!["fi".to_string()], + regions: vec![], + distributions: vec![], + excluded_distributions: vec![], + applications: vec![], + channels: vec![], + device_type: vec![], + min_version: "".to_string(), + max_version: "".to_string(), + experiment: "".to_string(), + }, + }, + ], + }), + &SearchUserEnvironment { + locale: "fi".into(), + region: "FR".into(), + update_channel: SearchUpdateChannel::Default, + distribution_id: String::new(), + experiment: String::new(), + app_name: SearchApplicationName::Firefox, + version: String::new(), + device_type: SearchDeviceType::None, + }, + ); + + assert_eq!( + default_engine_id.unwrap(), + "engine3", + "Should have returned the last specific default when multiple environments match" + ); + assert!( + default_engine_private_id.is_none(), + "Should not have returned an id for the private engine" + ); + } + + #[test] + fn test_determine_default_engines_returns_global_default_private() { + let (default_engine_id, default_engine_private_id) = determine_default_engines( + &ENGINES_LIST, + Some(JSONDefaultEnginesRecord { + global_default: "engine2".to_string(), + global_default_private: "engine3".to_string(), + specific_defaults: Vec::new(), + }), + &SearchUserEnvironment { + locale: "fi".into(), + region: "FR".into(), + update_channel: SearchUpdateChannel::Default, + distribution_id: String::new(), + experiment: String::new(), + app_name: SearchApplicationName::Firefox, + version: String::new(), + device_type: SearchDeviceType::None, + }, + ); + + assert_eq!( + default_engine_id.unwrap(), + "engine2", + "Should have returned the global default engine" + ); + assert_eq!( + default_engine_private_id.unwrap(), + "engine3", + "Should have returned the global default engine for private mode" + ); + + let (default_engine_id, default_engine_private_id) = determine_default_engines( + &ENGINES_LIST, + Some(JSONDefaultEnginesRecord { + global_default: "engine2".to_string(), + global_default_private: "engine3".to_string(), + specific_defaults: vec![JSONSpecificDefaultRecord { + default: String::new(), + default_private: "engine1".to_string(), + environment: JSONVariantEnvironment { + all_regions_and_locales: false, + excluded_locales: vec![], + excluded_regions: vec![], + locales: vec!["en-GB".to_string()], + regions: vec![], + distributions: vec![], + excluded_distributions: vec![], + applications: vec![], + channels: vec![], + device_type: vec![], + min_version: "".to_string(), + max_version: "".to_string(), + experiment: "".to_string(), + }, + }], + }), + &SearchUserEnvironment { + locale: "fi".into(), + region: "FR".into(), + update_channel: SearchUpdateChannel::Default, + distribution_id: String::new(), + experiment: String::new(), + app_name: SearchApplicationName::Firefox, + version: String::new(), + device_type: SearchDeviceType::None, + }, + ); + + assert_eq!( + default_engine_id.unwrap(), + "engine2", + "Should have returned the global default engine when no specific defaults environments match" + ); + assert_eq!( + default_engine_private_id.unwrap(), + "engine3", + "Should have returned the global default engine for private mode when no specific defaults environments match" + ); + + let (default_engine_id, default_engine_private_id) = determine_default_engines( + &ENGINES_LIST, + Some(JSONDefaultEnginesRecord { + global_default: "engine2".to_string(), + global_default_private: "engine3".to_string(), + specific_defaults: vec![JSONSpecificDefaultRecord { + default: String::new(), + default_private: "engine1".to_string(), + environment: JSONVariantEnvironment { + all_regions_and_locales: false, + excluded_locales: vec![], + excluded_regions: vec![], + locales: vec!["fi".to_string()], + regions: vec![], + distributions: vec![], + excluded_distributions: vec![], + applications: vec![], + channels: vec![], + device_type: vec![], + min_version: "".to_string(), + max_version: "".to_string(), + experiment: "".to_string(), + }, + }], + }), + &SearchUserEnvironment { + locale: "fi".into(), + region: "FR".into(), + update_channel: SearchUpdateChannel::Default, + distribution_id: String::new(), + experiment: String::new(), + app_name: SearchApplicationName::Firefox, + version: String::new(), + device_type: SearchDeviceType::None, + }, + ); + + assert_eq!( + default_engine_id.unwrap(), + "engine2", + "Should have returned the global default engine when specific environments match which override the private global default (and not the global default)." + ); + assert_eq!( + default_engine_private_id.unwrap(), + "engine1", + "Should have returned the specific default engine for private mode when environments match" + ); + + let (default_engine_id, default_engine_private_id) = determine_default_engines( + &ENGINES_LIST, + Some(JSONDefaultEnginesRecord { + global_default: "engine2".to_string(), + global_default_private: String::new(), + specific_defaults: vec![JSONSpecificDefaultRecord { + default: String::new(), + default_private: "engine4*".to_string(), + environment: JSONVariantEnvironment { + all_regions_and_locales: false, + excluded_locales: vec![], + excluded_regions: vec![], + locales: vec!["fi".to_string()], + regions: vec![], + distributions: vec![], + excluded_distributions: vec![], + applications: vec![], + channels: vec![], + device_type: vec![], + min_version: "".to_string(), + max_version: "".to_string(), + experiment: "".to_string(), + }, + }], + }), + &SearchUserEnvironment { + locale: "fi".into(), + region: "FR".into(), + update_channel: SearchUpdateChannel::Default, + distribution_id: String::new(), + experiment: String::new(), + app_name: SearchApplicationName::Firefox, + version: String::new(), + device_type: SearchDeviceType::None, + }, + ); + + assert_eq!( + default_engine_id.unwrap(), + "engine2", + "Should have returned the global default engine when specific environments match which override the private global default (and not the global default)" + ); + assert_eq!( + default_engine_private_id.unwrap(), + "engine4wildcardmatch", + "Should have returned the specific default for private mode when using a wildcard match" + ); + } } diff --git a/components/search/src/selector.rs b/components/search/src/selector.rs index 886580abe0..ef0638f3a4 100644 --- a/components/search/src/selector.rs +++ b/components/search/src/selector.rs @@ -390,8 +390,8 @@ mod tests { } } ), - app_default_engine_id: "test1".to_string(), - app_default_private_engine_id: Some("test2".to_string()) + app_default_engine_id: Some("test1".to_string()), + app_private_default_engine_id: Some("test2".to_string()) } ) } @@ -518,8 +518,8 @@ mod tests { } }, ), - app_default_engine_id: "test1".to_string(), - app_default_private_engine_id: None + app_default_engine_id: Some("test1".to_string()), + app_private_default_engine_id: None }, "Should have selected test1 for all matching locales, as the environments do not match for the other two" ); @@ -584,8 +584,8 @@ mod tests { } }, ), - app_default_engine_id: "test1".to_string(), - app_default_private_engine_id: None + app_default_engine_id: Some("test1".to_string()), + app_private_default_engine_id: None }, "Should have selected test1 for all matching locales and test2 for matching Focus IOS" ); @@ -651,9 +651,211 @@ mod tests { } }, ), - app_default_engine_id: "test1".to_string(), - app_default_private_engine_id: None + app_default_engine_id: Some("test1".to_string()), + app_private_default_engine_id: None }, "Should have selected test1 for all matching locales and test3 for matching the distribution id" ); } + + #[test] + fn test_set_config_should_handle_default_engines() { + 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 + } + }], + }, + { + "recordType": "engine", + "identifier": "private-default-FR", + "base": { + "name": "Private default FR", + "classification": "general", + "urls": { + "search": { + "base": "https://example.com", + "method": "GET" + } + } + }, + "variants": [{ + "environment": { + "allRegionsAndLocales": true, + } + }], + }, + { + "recordType": "defaultEngines", + "globalDefault": "test", + "specificDefaults": [{ + "environment": { + "distributions": ["test-distro"], + }, + "default": "distro-default" + }, { + "environment": { + "regions": ["fr"] + }, + "defaultPrivate": "private-default-FR" + }] + } + ] + }) + .to_string(), + ); + assert!( + config_result.is_ok(), + "Should have set the configuration successfully. {:?}", + config_result + ); + let expected_engines = vec![ + SearchEngineDefinition { + aliases: Vec::new(), + charset: "UTF-8".to_string(), + classification: SearchEngineClassification::General, + identifier: "test".to_string(), + name: "Test".to_string(), + order_hint: None, + partner_code: String::new(), + telemetry_suffix: None, + urls: SearchEngineUrls { + search: SearchEngineUrl { + base: "https://example.com".to_string(), + method: "GET".to_string(), + params: Vec::new(), + search_term_param_name: None, + }, + suggestions: None, + trending: None, + }, + }, + SearchEngineDefinition { + aliases: Vec::new(), + charset: "UTF-8".to_string(), + classification: SearchEngineClassification::General, + identifier: "distro-default".to_string(), + name: "Distribution Default".to_string(), + order_hint: None, + partner_code: String::new(), + telemetry_suffix: None, + urls: SearchEngineUrls { + search: SearchEngineUrl { + base: "https://example.com".to_string(), + method: "GET".to_string(), + params: Vec::new(), + search_term_param_name: None, + }, + suggestions: None, + trending: None, + }, + }, + SearchEngineDefinition { + aliases: Vec::new(), + charset: "UTF-8".to_string(), + classification: SearchEngineClassification::General, + identifier: "private-default-FR".to_string(), + name: "Private default FR".to_string(), + order_hint: None, + partner_code: String::new(), + telemetry_suffix: None, + urls: SearchEngineUrls { + search: SearchEngineUrl { + base: "https://example.com".to_string(), + method: "GET".to_string(), + params: Vec::new(), + search_term_param_name: None, + }, + suggestions: None, + trending: None, + }, + }, + ]; + + let result = Arc::clone(&selector).filter_engine_configuration(SearchUserEnvironment { + locale: "fi".into(), + region: "GB".into(), + update_channel: SearchUpdateChannel::Default, + distribution_id: "test-distro".to_string(), + experiment: String::new(), + app_name: SearchApplicationName::Firefox, + version: String::new(), + device_type: SearchDeviceType::None, + }); + assert!( + result.is_ok(), + "Should have filtered the configuration without error. {:?}", + result + ); + assert_eq!( + result.unwrap(), + RefinedSearchConfig { + engines: expected_engines.clone(), + app_default_engine_id: Some("distro-default".to_string()), + app_private_default_engine_id: None + }, + "Should have selected the default engine for the matching specific default" + ); + + let result = Arc::clone(&selector).filter_engine_configuration(SearchUserEnvironment { + locale: "fi".into(), + region: "fr".into(), + update_channel: SearchUpdateChannel::Default, + distribution_id: String::new(), + experiment: String::new(), + app_name: SearchApplicationName::Firefox, + version: String::new(), + device_type: SearchDeviceType::None, + }); + assert!( + result.is_ok(), + "Should have filtered the configuration without error. {:?}", + result + ); + assert_eq!( + result.unwrap(), + RefinedSearchConfig { + engines: expected_engines, + app_default_engine_id: Some("test".to_string()), + app_private_default_engine_id: Some("private-default-FR".to_string()) + }, + "Should have selected the private default engine for the matching specific default" + ); + } } diff --git a/components/search/src/types.rs b/components/search/src/types.rs index d9e648f846..98851650b0 100644 --- a/components/search/src/types.rs +++ b/components/search/src/types.rs @@ -155,7 +155,7 @@ impl SearchEngineClassification { } /// A definition for an individual search engine to be presented to the user. -#[derive(Debug, uniffi::Record, PartialEq)] +#[derive(Debug, uniffi::Record, PartialEq, Clone)] pub struct SearchEngineDefinition { /// A list of aliases for this engine. pub aliases: Vec, @@ -217,11 +217,13 @@ pub struct RefinedSearchConfig { pub engines: Vec, /// The identifier of the engine that should be used for the application - /// default engine. - pub app_default_engine_id: String, + /// default engine. If this is undefined, an error has occurred, and the + /// application should either default to the first engine in the engines + /// list or otherwise handle appropriately. + pub app_default_engine_id: Option, /// If specified, the identifier of the engine that should be used for the /// application default engine in private browsing mode. /// Only desktop uses this currently. - pub app_default_private_engine_id: Option, + pub app_private_default_engine_id: Option, }