Bug 1938922 - Implement sub-variant handling in the search engine selector.

This commit is contained in:
Mark Banner 2025-01-13 14:54:21 +00:00
Родитель c5c7bbec08
Коммит c28a94f2af
4 изменённых файлов: 461 добавлений и 61 удалений

Просмотреть файл

@ -81,6 +81,7 @@ pub(crate) struct JSONEngineBase {
/// The classification of search engine according to the main search types
/// (e.g. general, shopping, travel, dictionary). Currently, only marking as
/// a general search engine is supported.
#[serde(default)]
pub classification: SearchEngineClassification,
/// The user visible name for the search engine.
@ -182,6 +183,15 @@ pub(crate) struct JSONEngineVariant {
/// The urls for this variant.
pub urls: Option<JSONEngineUrls>,
/// This section describes subvariations of this search engine that may occur
/// depending on the user's environment. The last subvariant that matches
/// the user's environment will be applied to the engine.
///
/// Note: sub-variants are only supported in a top-level variant. You cannot
/// have nested sub-variants.
#[serde(default)]
pub sub_variants: Vec<JSONEngineVariant>,
}
/// Represents an individual engine record in the configuration.

Просмотреть файл

@ -22,17 +22,19 @@ impl From<JSONEngineUrl> for SearchEngineUrl {
}
}
impl JSONEngineUrl {
/// Merges two `JSONEngineUrl` objects, preferring the values from the
/// `preferred` object.
fn merge(original: Self, preferred: Self) -> Self {
Self {
base: preferred.base.or(original.base),
method: preferred.method.or(original.method),
params: preferred.params.or(original.params),
search_term_param_name: preferred
.search_term_param_name
.or(original.search_term_param_name),
impl SearchEngineUrl {
fn merge(&mut self, preferred: &JSONEngineUrl) {
if let Some(base) = &preferred.base {
self.base = base.clone();
}
if let Some(method) = &preferred.method {
self.method = method.as_str().to_string();
}
if let Some(params) = &preferred.params {
self.params = params.clone();
}
if let Some(search_term_param_name) = &preferred.search_term_param_name {
self.search_term_param_name = Some(search_term_param_name.clone());
}
}
}
@ -47,46 +49,49 @@ impl From<JSONEngineUrls> for SearchEngineUrls {
}
}
impl JSONEngineUrls {
fn maybe_merge_urls(
original_url: Option<JSONEngineUrl>,
preferred_url: Option<JSONEngineUrl>,
) -> Option<JSONEngineUrl> {
match (&original_url, &preferred_url) {
(Some(original), Some(preferred)) => {
Some(JSONEngineUrl::merge(original.clone(), preferred.clone()))
}
(None, Some(preferred)) => Some(preferred.clone()),
_ => original_url.clone(),
impl SearchEngineUrls {
fn merge(&mut self, preferred: &JSONEngineUrls) {
if let Some(search_url) = &preferred.search {
self.search.merge(search_url);
}
}
/// Merges two `JSONEngineUrl` objects, preferring the values from the
/// `preferred` object.
fn merge(original: Self, preferred: JSONEngineUrls) -> Self {
Self {
search: JSONEngineUrls::maybe_merge_urls(original.search, preferred.search),
suggestions: JSONEngineUrls::maybe_merge_urls(
original.suggestions,
preferred.suggestions,
),
trending: JSONEngineUrls::maybe_merge_urls(original.trending, preferred.trending),
if let Some(suggestions_url) = &preferred.suggestions {
match &mut self.suggestions {
Some(suggestion) => suggestion.merge(suggestions_url),
None => self.suggestions = Some(suggestions_url.clone().into()),
};
}
if let Some(trending_url) = &preferred.trending {
match &mut self.trending {
Some(trend) => trend.merge(trending_url),
None => self.suggestions = Some(trending_url.clone().into()),
};
}
}
}
impl SearchEngineDefinition {
fn merge_variant(&mut self, variant: &JSONEngineVariant) {
if !self.optional {
self.optional = variant.optional;
}
if let Some(partner_code) = &variant.partner_code {
self.partner_code = partner_code.clone();
}
if let Some(telemetry_suffix) = &variant.telemetry_suffix {
self.telemetry_suffix = telemetry_suffix.clone();
}
if let Some(urls) = &variant.urls {
self.urls.merge(urls);
}
}
pub(crate) fn from_configuration_details(
identifier: &str,
base: JSONEngineBase,
variant: JSONEngineVariant,
variant: &JSONEngineVariant,
sub_variant: &Option<JSONEngineVariant>,
) -> SearchEngineDefinition {
let urls: JSONEngineUrls = match variant.urls {
Some(urls) => JSONEngineUrls::merge(base.urls, urls),
None => base.urls,
};
SearchEngineDefinition {
let mut engine_definition = SearchEngineDefinition {
aliases: base.aliases.unwrap_or_default(),
charset: base.charset.unwrap_or_else(|| "UTF-8".to_string()),
classification: base.classification,
@ -94,13 +99,17 @@ impl SearchEngineDefinition {
name: base.name,
optional: variant.optional,
order_hint: None,
partner_code: variant
.partner_code
.or(base.partner_code)
.unwrap_or_default(),
telemetry_suffix: variant.telemetry_suffix,
urls: urls.into(),
partner_code: base.partner_code.unwrap_or_default(),
telemetry_suffix: String::new(),
urls: base.urls.into(),
};
engine_definition.merge_variant(variant);
if let Some(sub_variant) = sub_variant {
engine_definition.merge_variant(sub_variant);
}
engine_definition
}
}
@ -159,8 +168,23 @@ fn maybe_extract_engine_config(
.rev()
.find(|r| matches_user_environment(&r.environment, user_environment));
let mut matching_sub_variant = None;
if let Some(variant) = &matching_variant {
matching_sub_variant = variant
.sub_variants
.iter()
.rev()
.find(|r| matches_user_environment(&r.environment, user_environment))
.cloned();
}
matching_variant.map(|variant| {
SearchEngineDefinition::from_configuration_details(&identifier, base, variant)
SearchEngineDefinition::from_configuration_details(
&identifier,
base,
&variant,
&matching_sub_variant,
)
})
}
@ -274,7 +298,7 @@ mod tests {
trending: None,
},
},
JSONEngineVariant {
&JSONEngineVariant {
environment: JSONVariantEnvironment {
all_regions_and_locales: true,
..Default::default()
@ -283,7 +307,9 @@ mod tests {
partner_code: None,
telemetry_suffix: None,
urls: None,
sub_variants: vec![],
},
&None,
);
assert_eq!(
@ -297,7 +323,7 @@ mod tests {
name: "Test".to_string(),
optional: false,
order_hint: None,
telemetry_suffix: None,
telemetry_suffix: String::new(),
urls: SearchEngineUrls {
search: SearchEngineUrl {
base: "https://example.com".to_string(),
@ -357,7 +383,7 @@ mod tests {
let result = SearchEngineDefinition::from_configuration_details(
"test",
Lazy::force(&ENGINE_BASE).clone(),
JSONEngineVariant {
&JSONEngineVariant {
environment: JSONVariantEnvironment {
all_regions_and_locales: true,
..Default::default()
@ -366,7 +392,9 @@ mod tests {
partner_code: None,
telemetry_suffix: None,
urls: None,
sub_variants: vec![],
},
&None,
);
assert_eq!(
@ -380,7 +408,7 @@ mod tests {
name: "Test".to_string(),
optional: false,
order_hint: None,
telemetry_suffix: None,
telemetry_suffix: String::new(),
urls: SearchEngineUrls {
search: SearchEngineUrl {
base: "https://example.com".to_string(),
@ -422,7 +450,7 @@ mod tests {
let result = SearchEngineDefinition::from_configuration_details(
"test",
Lazy::force(&ENGINE_BASE).clone(),
JSONEngineVariant {
&JSONEngineVariant {
environment: JSONVariantEnvironment {
all_regions_and_locales: true,
..Default::default()
@ -462,7 +490,9 @@ mod tests {
search_term_param_name: Some("trend".to_string()),
}),
}),
sub_variants: vec![],
},
&None,
);
assert_eq!(
@ -476,7 +506,7 @@ mod tests {
name: "Test".to_string(),
optional: true,
order_hint: None,
telemetry_suffix: Some("star".to_string()),
telemetry_suffix: "star".to_string(),
urls: SearchEngineUrls {
search: SearchEngineUrl {
base: "https://example.com/variant".to_string(),
@ -513,6 +543,149 @@ mod tests {
)
}
#[test]
fn test_from_configuration_details_merges_sub_variants() {
let result = SearchEngineDefinition::from_configuration_details(
"test",
Lazy::force(&ENGINE_BASE).clone(),
&JSONEngineVariant {
environment: JSONVariantEnvironment {
all_regions_and_locales: true,
..Default::default()
},
optional: true,
partner_code: Some("trek".to_string()),
telemetry_suffix: Some("star".to_string()),
urls: Some(JSONEngineUrls {
search: Some(JSONEngineUrl {
base: Some("https://example.com/variant".to_string()),
method: Some(JSONEngineMethod::Get),
params: Some(vec![SearchUrlParam {
name: "variant".to_string(),
value: Some("test variant".to_string()),
experiment_config: None,
}]),
search_term_param_name: Some("ship".to_string()),
}),
suggestions: Some(JSONEngineUrl {
base: Some("https://example.com/suggestions-variant".to_string()),
method: Some(JSONEngineMethod::Get),
params: Some(vec![SearchUrlParam {
name: "suggest-variant".to_string(),
value: Some("sugg test variant".to_string()),
experiment_config: None,
}]),
search_term_param_name: Some("variant".to_string()),
}),
trending: Some(JSONEngineUrl {
base: Some("https://example.com/trending-variant".to_string()),
method: Some(JSONEngineMethod::Get),
params: Some(vec![SearchUrlParam {
name: "trend-variant".to_string(),
value: Some("trend test variant".to_string()),
experiment_config: None,
}]),
search_term_param_name: Some("trend".to_string()),
}),
}),
// This would be the list of sub-variants for this part of the
// configuration, however it is not used as the actual sub-variant
// to be merged is passed as the third argument to
// `from_configuration_details`.
sub_variants: vec![],
},
&Some(JSONEngineVariant {
environment: JSONVariantEnvironment {
all_regions_and_locales: true,
..Default::default()
},
optional: true,
partner_code: Some("trek2".to_string()),
telemetry_suffix: Some("star2".to_string()),
urls: Some(JSONEngineUrls {
search: Some(JSONEngineUrl {
base: Some("https://example.com/subvariant".to_string()),
method: Some(JSONEngineMethod::Get),
params: Some(vec![SearchUrlParam {
name: "subvariant".to_string(),
value: Some("test subvariant".to_string()),
experiment_config: None,
}]),
search_term_param_name: Some("shuttle".to_string()),
}),
suggestions: Some(JSONEngineUrl {
base: Some("https://example.com/suggestions-subvariant".to_string()),
method: Some(JSONEngineMethod::Get),
params: Some(vec![SearchUrlParam {
name: "suggest-subvariant".to_string(),
value: Some("sugg test subvariant".to_string()),
experiment_config: None,
}]),
search_term_param_name: Some("subvariant".to_string()),
}),
trending: Some(JSONEngineUrl {
base: Some("https://example.com/trending-subvariant".to_string()),
method: Some(JSONEngineMethod::Get),
params: Some(vec![SearchUrlParam {
name: "trend-subvariant".to_string(),
value: Some("trend test subvariant".to_string()),
experiment_config: None,
}]),
search_term_param_name: Some("subtrend".to_string()),
}),
}),
sub_variants: vec![],
}),
);
assert_eq!(
result,
SearchEngineDefinition {
aliases: vec!["foo".to_string(), "bar".to_string()],
charset: "ISO-8859-15".to_string(),
classification: SearchEngineClassification::Unknown,
identifier: "test".to_string(),
partner_code: "trek2".to_string(),
name: "Test".to_string(),
optional: true,
order_hint: None,
telemetry_suffix: "star2".to_string(),
urls: SearchEngineUrls {
search: SearchEngineUrl {
base: "https://example.com/subvariant".to_string(),
method: "GET".to_string(),
params: vec![SearchUrlParam {
name: "subvariant".to_string(),
value: Some("test subvariant".to_string()),
experiment_config: None,
}],
search_term_param_name: Some("shuttle".to_string()),
},
suggestions: Some(SearchEngineUrl {
base: "https://example.com/suggestions-subvariant".to_string(),
method: "GET".to_string(),
params: vec![SearchUrlParam {
name: "suggest-subvariant".to_string(),
value: Some("sugg test subvariant".to_string()),
experiment_config: None,
}],
search_term_param_name: Some("subvariant".to_string()),
}),
trending: Some(SearchEngineUrl {
base: "https://example.com/trending-subvariant".to_string(),
method: "GET".to_string(),
params: vec![SearchUrlParam {
name: "trend-subvariant".to_string(),
value: Some("trend test subvariant".to_string()),
experiment_config: None,
}],
search_term_param_name: Some("subtrend".to_string()),
})
}
}
)
}
static ENGINES_LIST: Lazy<Vec<SearchEngineDefinition>> = Lazy::new(|| {
vec![
SearchEngineDefinition {

Просмотреть файл

@ -273,7 +273,7 @@ mod tests {
"identifier": "test2",
"base": {
"name": "Test 2",
"classification": "general",
// No classification specified to test fallback.
"urls": {
"search": {
"base": "https://example.com/2",
@ -356,13 +356,13 @@ mod tests {
SearchEngineDefinition {
aliases: Vec::new(),
charset: "UTF-8".to_string(),
classification: SearchEngineClassification::General,
classification: SearchEngineClassification::Unknown,
identifier: "test2".to_string(),
name: "Test 2".to_string(),
optional: false,
order_hint: None,
partner_code: String::new(),
telemetry_suffix: None,
telemetry_suffix: String::new(),
urls: SearchEngineUrls {
search: SearchEngineUrl {
base: "https://example.com/2".to_string(),
@ -391,10 +391,10 @@ mod tests {
{
"recordType": "engine",
"identifier": "test1",
"partnerCode": "star",
"base": {
"name": "Test 1",
"classification": "general",
"partnerCode": "star",
"urls": {
"search": {
"base": "https://example.com/1",
@ -497,6 +497,7 @@ mod tests {
classification: SearchEngineClassification::General,
identifier: "test1".to_string(),
name: "Test 1".to_string(),
partner_code: "star".to_string(),
urls: SearchEngineUrls {
search: SearchEngineUrl {
base: "https://example.com/1".to_string(),
@ -538,7 +539,7 @@ mod tests {
name: "Test 2".to_string(),
optional: true,
partner_code: "ship".to_string(),
telemetry_suffix: Some("E".to_string()),
telemetry_suffix: "E".to_string(),
urls: SearchEngineUrls {
search: SearchEngineUrl {
base: "https://example.com/2".to_string(),
@ -557,6 +558,221 @@ mod tests {
)
}
#[test]
fn test_filter_engine_configuration_handles_basic_subvariants() {
let selector = Arc::new(SearchEngineSelector::new());
let config_result = Arc::clone(&selector).set_search_config(
json!({
"data": [
{
"recordType": "engine",
"identifier": "test1",
"base": {
"name": "Test 1",
"partnerCode": "star",
"urls": {
"search": {
"base": "https://example.com/1",
"method": "GET",
"searchTermParamName": "q"
},
"suggestions": {
"base": "https://example.com/suggestions",
"method": "POST",
"params": [{
"name": "type",
"value": "space",
}],
"searchTermParamName": "suggest"
},
"trending": {
"base": "https://example.com/trending",
"method": "GET",
"params": [{
"name": "area",
"experimentConfig": "area-param",
}]
}
}
},
"variants": [{
"environment": {
"allRegionsAndLocales": true
},
},
{
"environment": {
"regions": ["FR"]
},
"urls": {
"search": {
"method": "POST",
"params": [{
"name": "variant-param-name",
"value": "variant-param-value"
}]
}
},
"subVariants": [
{
"environment": {
"locales": ["fr"]
},
"partnerCode": "fr-partner-code",
"telemetrySuffix": "fr-telemetry-suffix"
},
{
"environment": {
"locales": ["en-CA"]
},
"urls": {
"search": {
"method": "GET",
"params": [{
"name": "en-ca-param-name",
"value": "en-ca-param-value"
}]
}
},
}
]
}],
},
{
"recordType": "defaultEngines",
"globalDefault": "test1"
}
]
})
.to_string(),
);
assert!(
config_result.is_ok(),
"Should have set the configuration successfully. {:?}",
config_result
);
let mut result = Arc::clone(&selector).filter_engine_configuration(SearchUserEnvironment {
region: "FR".into(),
locale: "fr".into(),
..Default::default()
});
assert!(
result.is_ok(),
"Should have filtered the configuration without error. {:?}",
result
);
assert_eq!(
result.unwrap(),
RefinedSearchConfig {
engines: vec!(SearchEngineDefinition {
charset: "UTF-8".to_string(),
identifier: "test1".to_string(),
name: "Test 1".to_string(),
partner_code: "fr-partner-code".to_string(),
telemetry_suffix: "fr-telemetry-suffix".to_string(),
urls: SearchEngineUrls {
search: SearchEngineUrl {
base: "https://example.com/1".to_string(),
method: "POST".to_string(),
params: vec![SearchUrlParam {
name: "variant-param-name".to_string(),
value: Some("variant-param-value".to_string()),
experiment_config: None
}],
search_term_param_name: Some("q".to_string())
},
suggestions: Some(SearchEngineUrl {
base: "https://example.com/suggestions".to_string(),
method: "POST".to_string(),
params: vec![SearchUrlParam {
name: "type".to_string(),
value: Some("space".to_string()),
experiment_config: None
}],
search_term_param_name: Some("suggest".to_string())
}),
trending: Some(SearchEngineUrl {
base: "https://example.com/trending".to_string(),
method: "GET".to_string(),
params: vec![SearchUrlParam {
name: "area".to_string(),
value: None,
experiment_config: Some("area-param".to_string())
}],
search_term_param_name: None
})
},
..Default::default()
}),
app_default_engine_id: Some("test1".to_string()),
app_private_default_engine_id: None
},
"Should have correctly matched and merged the fr locale sub-variant."
);
result = selector.filter_engine_configuration(SearchUserEnvironment {
region: "FR".into(),
locale: "en-CA".into(),
..Default::default()
});
assert!(
result.is_ok(),
"Should have filtered the configuration without error. {:?}",
result
);
assert_eq!(
result.unwrap(),
RefinedSearchConfig {
engines: vec!(SearchEngineDefinition {
charset: "UTF-8".to_string(),
identifier: "test1".to_string(),
name: "Test 1".to_string(),
partner_code: "star".to_string(),
urls: SearchEngineUrls {
search: SearchEngineUrl {
base: "https://example.com/1".to_string(),
method: "GET".to_string(),
params: vec![SearchUrlParam {
name: "en-ca-param-name".to_string(),
value: Some("en-ca-param-value".to_string()),
experiment_config: None
}],
search_term_param_name: Some("q".to_string())
},
suggestions: Some(SearchEngineUrl {
base: "https://example.com/suggestions".to_string(),
method: "POST".to_string(),
params: vec![SearchUrlParam {
name: "type".to_string(),
value: Some("space".to_string()),
experiment_config: None
}],
search_term_param_name: Some("suggest".to_string())
}),
trending: Some(SearchEngineUrl {
base: "https://example.com/trending".to_string(),
method: "GET".to_string(),
params: vec![SearchUrlParam {
name: "area".to_string(),
value: None,
experiment_config: Some("area-param".to_string())
}],
search_term_param_name: None
})
},
..Default::default()
}),
app_default_engine_id: Some("test1".to_string()),
app_private_default_engine_id: None
},
"Should have correctly matched and merged the en-CA locale sub-variant."
);
}
#[test]
fn test_filter_engine_configuration_handles_environments() {
let selector = Arc::new(SearchEngineSelector::new());

Просмотреть файл

@ -188,8 +188,9 @@ pub struct SearchEngineDefinition {
pub partner_code: String,
/// Optional suffix that is appended to the search engine identifier
/// following a dash, i.e. `<identifier>-<suffix>`
pub telemetry_suffix: Option<String>,
/// following a dash, i.e. `<identifier>-<suffix>`. If it is an empty string
/// no dash should be appended.
pub telemetry_suffix: String,
/// The URLs associated with the search engine.
pub urls: SearchEngineUrls,