diff --git a/components/suggest/src/benchmarks/query.rs b/components/suggest/src/benchmarks/query.rs index 5495dbbcaf..30e41dc677 100644 --- a/components/suggest/src/benchmarks/query.rs +++ b/components/suggest/src/benchmarks/query.rs @@ -40,7 +40,7 @@ impl BenchmarkWithInput for QueryBenchmark { InputType(SuggestionQuery { providers: vec![self.provider], keyword: self.query.to_string(), - limit: None, + ..SuggestionQuery::default() }) } diff --git a/components/suggest/src/db.rs b/components/suggest/src/db.rs index fa9a25f800..047e7cd138 100644 --- a/components/suggest/src/db.rs +++ b/components/suggest/src/db.rs @@ -13,7 +13,7 @@ use rusqlite::{ types::{FromSql, ToSql}, Connection, OpenFlags, OptionalExtension, }; -use sql_support::{open_database::open_database_with_flags, ConnExt}; +use sql_support::{open_database::open_database_with_flags, repeat_sql_vars, ConnExt}; use crate::{ config::{SuggestGlobalConfig, SuggestProviderConfig}, @@ -24,8 +24,9 @@ use crate::{ provider::SuggestionProvider, rs::{ DownloadedAmoSuggestion, DownloadedAmpSuggestion, DownloadedAmpWikipediaSuggestion, - DownloadedFakespotSuggestion, DownloadedMdnSuggestion, DownloadedPocketSuggestion, - DownloadedWeatherData, DownloadedWikipediaSuggestion, Record, SuggestRecordId, + DownloadedExposureSuggestion, DownloadedFakespotSuggestion, DownloadedMdnSuggestion, + DownloadedPocketSuggestion, DownloadedWeatherData, DownloadedWikipediaSuggestion, Record, + SuggestRecordId, }, schema::{clear_database, SuggestConnectionInitializer}, suggestion::{cook_raw_suggestion_url, AmpSuggestionType, Suggestion}, @@ -805,6 +806,67 @@ impl<'a> SuggestDao<'a> { ) } + /// Fetches exposure suggestions + pub fn fetch_exposure_suggestions(&self, query: &SuggestionQuery) -> Result> { + // A single exposure suggestion can be spread across multiple remote + // settings records, for example if it has very many keywords. On ingest + // we will insert one row in `exposure_custom_details` and one row in + // `suggestions` per record, but that's only an implementation detail. + // Logically, and for consumers, there's only ever at most one exposure + // suggestion with a given exposure suggestion type. + // + // Why do insertions this way? It's how other suggestions work, and it + // lets us perform relational operations on suggestions, records, and + // keywords. For example, when a record is deleted we can look up its ID + // in `suggestions`, join the keywords table on the suggestion ID, and + // delete the keywords that were added by that record. + + let Some(suggestion_types) = query + .provider_constraints + .as_ref() + .and_then(|c| c.exposure_suggestion_types.as_ref()) + else { + return Ok(vec![]); + }; + + let keyword = query.keyword.to_lowercase(); + let params = rusqlite::params_from_iter( + std::iter::once(&SuggestionProvider::Exposure as &dyn ToSql) + .chain(std::iter::once(&keyword as &dyn ToSql)) + .chain(suggestion_types.iter().map(|t| t as &dyn ToSql)), + ); + self.conn.query_rows_and_then_cached( + &format!( + r#" + SELECT DISTINCT + d.type + FROM + suggestions s + JOIN + exposure_custom_details d + ON d.suggestion_id = s.id + JOIN + keywords k + ON k.suggestion_id = s.id + WHERE + s.provider = ? + AND k.keyword = ? + AND d.type IN ({}) + ORDER BY + d.type + "#, + repeat_sql_vars(suggestion_types.len()) + ), + params, + |row| -> Result { + Ok(Suggestion::Exposure { + suggestion_type: row.get("type")?, + score: 1.0, + }) + }, + ) + } + /// Inserts all suggestions from a downloaded AMO attachment into /// the database. pub fn insert_amo_suggestions( @@ -1058,6 +1120,39 @@ impl<'a> SuggestDao<'a> { Ok(()) } + /// Inserts exposure suggestion records data into the database. + pub fn insert_exposure_suggestions( + &mut self, + record_id: &SuggestRecordId, + suggestion_type: &str, + suggestions: &[DownloadedExposureSuggestion], + ) -> Result<()> { + // `suggestion.keywords()` can yield duplicates for exposure + // suggestions, so ignore failures on insert in the uniqueness + // constraint on `(suggestion_id, keyword)`. + let mut keyword_insert = KeywordInsertStatement::new_with_or_ignore(self.conn)?; + let mut suggestion_insert = SuggestionInsertStatement::new(self.conn)?; + let mut exposure_insert = ExposureInsertStatement::new(self.conn)?; + for suggestion in suggestions { + self.scope.err_if_interrupted()?; + let suggestion_id = suggestion_insert.execute( + record_id, + "", // title, not used by exposure suggestions + "", // url, not used by exposure suggestions + DEFAULT_SUGGESTION_SCORE, + SuggestionProvider::Exposure, + )?; + exposure_insert.execute(suggestion_id, suggestion_type)?; + + // Exposure suggestions don't use `rank` but `(suggestion_id, rank)` + // must be unique since there's an index on that tuple. + for (rank, keyword) in suggestion.keywords().enumerate() { + keyword_insert.execute(suggestion_id, &keyword, None, rank)?; + } + } + Ok(()) + } + /// Inserts or replaces an icon for a suggestion into the database. pub fn put_icon(&mut self, icon_id: &str, data: &[u8], mimetype: &str) -> Result<()> { self.conn.execute( @@ -1498,6 +1593,28 @@ impl<'conn> FakespotInsertStatement<'conn> { } } +struct ExposureInsertStatement<'conn>(rusqlite::Statement<'conn>); + +impl<'conn> ExposureInsertStatement<'conn> { + fn new(conn: &'conn Connection) -> Result { + Ok(Self(conn.prepare( + "INSERT INTO exposure_custom_details( + suggestion_id, + type + ) + VALUES(?, ?) + ", + )?)) + } + + fn execute(&mut self, suggestion_id: i64, suggestion_type: &str) -> Result<()> { + self.0 + .execute((suggestion_id, suggestion_type)) + .with_context("exposure insert")?; + Ok(()) + } +} + struct KeywordInsertStatement<'conn>(rusqlite::Statement<'conn>); impl<'conn> KeywordInsertStatement<'conn> { @@ -1514,6 +1631,19 @@ impl<'conn> KeywordInsertStatement<'conn> { )?)) } + fn new_with_or_ignore(conn: &'conn Connection) -> Result { + Ok(Self(conn.prepare( + "INSERT OR IGNORE INTO keywords( + suggestion_id, + keyword, + full_keyword_id, + rank + ) + VALUES(?, ?, ?, ?) + ", + )?)) + } + fn execute( &mut self, suggestion_id: i64, diff --git a/components/suggest/src/lib.rs b/components/suggest/src/lib.rs index 3d4ac50824..0b47925143 100644 --- a/components/suggest/src/lib.rs +++ b/components/suggest/src/lib.rs @@ -26,7 +26,7 @@ mod yelp; pub use config::{SuggestGlobalConfig, SuggestProviderConfig}; pub use error::SuggestApiError; pub use metrics::{LabeledTimingSample, SuggestIngestionMetrics}; -pub use provider::SuggestionProvider; +pub use provider::{SuggestionProvider, SuggestionProviderConstraints}; pub use query::{QueryWithMetricsResult, SuggestionQuery}; pub use store::{InterruptKind, SuggestIngestionConstraints, SuggestStore, SuggestStoreBuilder}; pub use suggestion::{raw_suggestion_url_matches, Suggestion}; diff --git a/components/suggest/src/provider.rs b/components/suggest/src/provider.rs index bfa1a3ce07..cfcdb4fb13 100644 --- a/components/suggest/src/provider.rs +++ b/components/suggest/src/provider.rs @@ -12,6 +12,17 @@ use rusqlite::{ use crate::rs::SuggestRecordType; +/// Record types from these providers will be ingested when consumers do not +/// specify providers in `SuggestIngestionConstraints`. +pub(crate) const DEFAULT_INGEST_PROVIDERS: [SuggestionProvider; 6] = [ + SuggestionProvider::Amp, + SuggestionProvider::Wikipedia, + SuggestionProvider::Amo, + SuggestionProvider::Yelp, + SuggestionProvider::Mdn, + SuggestionProvider::AmpMobile, +]; + /// A provider is a source of search suggestions. #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] #[repr(u8)] @@ -25,6 +36,7 @@ pub enum SuggestionProvider { Weather = 7, AmpMobile = 8, Fakespot = 9, + Exposure = 10, } impl fmt::Display for SuggestionProvider { @@ -39,6 +51,7 @@ impl fmt::Display for SuggestionProvider { Self::Weather => write!(f, "weather"), Self::AmpMobile => write!(f, "ampmobile"), Self::Fakespot => write!(f, "fakespot"), + Self::Exposure => write!(f, "exposure"), } } } @@ -54,7 +67,7 @@ impl FromSql for SuggestionProvider { } impl SuggestionProvider { - pub fn all() -> [Self; 9] { + pub fn all() -> [Self; 10] { [ Self::Amp, Self::Wikipedia, @@ -65,36 +78,39 @@ impl SuggestionProvider { Self::Weather, Self::AmpMobile, Self::Fakespot, + Self::Exposure, ] } #[inline] pub(crate) fn from_u8(v: u8) -> Option { match v { - 1 => Some(SuggestionProvider::Amp), - 2 => Some(SuggestionProvider::Wikipedia), - 3 => Some(SuggestionProvider::Amo), - 4 => Some(SuggestionProvider::Pocket), - 5 => Some(SuggestionProvider::Yelp), - 6 => Some(SuggestionProvider::Mdn), - 7 => Some(SuggestionProvider::Weather), - 8 => Some(SuggestionProvider::AmpMobile), - 9 => Some(SuggestionProvider::Fakespot), + 1 => Some(Self::Amp), + 2 => Some(Self::Wikipedia), + 3 => Some(Self::Amo), + 4 => Some(Self::Pocket), + 5 => Some(Self::Yelp), + 6 => Some(Self::Mdn), + 7 => Some(Self::Weather), + 8 => Some(Self::AmpMobile), + 9 => Some(Self::Fakespot), + 10 => Some(Self::Exposure), _ => None, } } pub(crate) fn record_type(&self) -> SuggestRecordType { match self { - SuggestionProvider::Amp => SuggestRecordType::AmpWikipedia, - SuggestionProvider::Wikipedia => SuggestRecordType::AmpWikipedia, - SuggestionProvider::Amo => SuggestRecordType::Amo, - SuggestionProvider::Pocket => SuggestRecordType::Pocket, - SuggestionProvider::Yelp => SuggestRecordType::Yelp, - SuggestionProvider::Mdn => SuggestRecordType::Mdn, - SuggestionProvider::Weather => SuggestRecordType::Weather, - SuggestionProvider::AmpMobile => SuggestRecordType::AmpMobile, - SuggestionProvider::Fakespot => SuggestRecordType::Fakespot, + Self::Amp => SuggestRecordType::AmpWikipedia, + Self::Wikipedia => SuggestRecordType::AmpWikipedia, + Self::Amo => SuggestRecordType::Amo, + Self::Pocket => SuggestRecordType::Pocket, + Self::Yelp => SuggestRecordType::Yelp, + Self::Mdn => SuggestRecordType::Mdn, + Self::Weather => SuggestRecordType::Weather, + Self::AmpMobile => SuggestRecordType::AmpMobile, + Self::Fakespot => SuggestRecordType::Fakespot, + Self::Exposure => SuggestRecordType::Exposure, } } } @@ -104,3 +120,8 @@ impl ToSql for SuggestionProvider { Ok(ToSqlOutput::from(*self as u8)) } } + +#[derive(Clone, Default, Debug)] +pub struct SuggestionProviderConstraints { + pub exposure_suggestion_types: Option>, +} diff --git a/components/suggest/src/query.rs b/components/suggest/src/query.rs index 4b4c2c45f3..249ca12514 100644 --- a/components/suggest/src/query.rs +++ b/components/suggest/src/query.rs @@ -2,13 +2,14 @@ * 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/. */ -use crate::{LabeledTimingSample, Suggestion, SuggestionProvider}; +use crate::{LabeledTimingSample, Suggestion, SuggestionProvider, SuggestionProviderConstraints}; /// A query for suggestions to show in the address bar. #[derive(Clone, Debug, Default)] pub struct SuggestionQuery { pub keyword: String, pub providers: Vec, + pub provider_constraints: Option, pub limit: Option, } @@ -24,7 +25,7 @@ impl SuggestionQuery { Self { keyword: keyword.to_string(), providers: Vec::from(SuggestionProvider::all()), - limit: None, + ..Self::default() } } @@ -32,7 +33,7 @@ impl SuggestionQuery { Self { keyword: keyword.to_string(), providers, - limit: None, + ..Self::default() } } @@ -50,7 +51,7 @@ impl SuggestionQuery { Self { keyword: keyword.into(), providers: vec![SuggestionProvider::Amp], - limit: None, + ..Self::default() } } @@ -58,7 +59,7 @@ impl SuggestionQuery { Self { keyword: keyword.into(), providers: vec![SuggestionProvider::Wikipedia], - limit: None, + ..Self::default() } } @@ -66,7 +67,7 @@ impl SuggestionQuery { Self { keyword: keyword.into(), providers: vec![SuggestionProvider::AmpMobile], - limit: None, + ..Self::default() } } @@ -74,7 +75,7 @@ impl SuggestionQuery { Self { keyword: keyword.into(), providers: vec![SuggestionProvider::Amo], - limit: None, + ..Self::default() } } @@ -82,7 +83,7 @@ impl SuggestionQuery { Self { keyword: keyword.into(), providers: vec![SuggestionProvider::Pocket], - limit: None, + ..Self::default() } } @@ -90,7 +91,7 @@ impl SuggestionQuery { Self { keyword: keyword.into(), providers: vec![SuggestionProvider::Yelp], - limit: None, + ..Self::default() } } @@ -98,7 +99,7 @@ impl SuggestionQuery { Self { keyword: keyword.into(), providers: vec![SuggestionProvider::Mdn], - limit: None, + ..Self::default() } } @@ -106,7 +107,7 @@ impl SuggestionQuery { Self { keyword: keyword.into(), providers: vec![SuggestionProvider::Fakespot], - limit: None, + ..Self::default() } } @@ -114,7 +115,20 @@ impl SuggestionQuery { Self { keyword: keyword.into(), providers: vec![SuggestionProvider::Weather], - limit: None, + ..Self::default() + } + } + + pub fn exposure(keyword: &str, suggestion_types: &[&str]) -> Self { + Self { + keyword: keyword.into(), + providers: vec![SuggestionProvider::Exposure], + provider_constraints: Some(SuggestionProviderConstraints { + exposure_suggestion_types: Some( + suggestion_types.iter().map(|s| s.to_string()).collect(), + ), + }), + ..Self::default() } } diff --git a/components/suggest/src/rs.rs b/components/suggest/src/rs.rs index db50b63338..e22f7eab99 100644 --- a/components/suggest/src/rs.rs +++ b/components/suggest/src/rs.rs @@ -38,22 +38,6 @@ use serde::{Deserialize, Deserializer}; use crate::{db::SuggestDao, error::Error, provider::SuggestionProvider, Result}; -/// A list of default record types to download if nothing is specified. -/// This defaults to all record types available as-of Fx128. -/// Consumers should specify provider types in `SuggestIngestionConstraints` if they want a -/// different set. -pub(crate) const DEFAULT_RECORDS_TYPES: [SuggestRecordType; 9] = [ - SuggestRecordType::Icon, - SuggestRecordType::AmpWikipedia, - SuggestRecordType::Amo, - SuggestRecordType::Pocket, - SuggestRecordType::Yelp, - SuggestRecordType::Mdn, - SuggestRecordType::Weather, - SuggestRecordType::GlobalConfig, - SuggestRecordType::AmpMobile, -]; - #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum Collection { Quicksuggest, @@ -194,8 +178,8 @@ impl Record { /// A record in the Suggest Remote Settings collection. /// -/// Except for the type, Suggest records don't carry additional fields. All -/// suggestions are stored in each record's attachment. +/// Most Suggest records don't carry inline fields except for `type`. +/// Suggestions themselves are typically stored in each record's attachment. #[derive(Clone, Debug, Deserialize)] #[serde(tag = "type")] pub(crate) enum SuggestRecord { @@ -219,6 +203,8 @@ pub(crate) enum SuggestRecord { AmpMobile, #[serde(rename = "fakespot-suggestions")] Fakespot, + #[serde(rename = "exposure-suggestions")] + Exposure(DownloadedExposureRecord), } /// Enum for the different record types that can be consumed. @@ -236,6 +222,7 @@ pub enum SuggestRecordType { GlobalConfig, AmpMobile, Fakespot, + Exposure, } impl From<&SuggestRecord> for SuggestRecordType { @@ -251,6 +238,7 @@ impl From<&SuggestRecord> for SuggestRecordType { SuggestRecord::GlobalConfig(_) => Self::GlobalConfig, SuggestRecord::AmpMobile => Self::AmpMobile, SuggestRecord::Fakespot => Self::Fakespot, + SuggestRecord::Exposure(_) => Self::Exposure, } } } @@ -278,6 +266,7 @@ impl SuggestRecordType { Self::GlobalConfig, Self::AmpMobile, Self::Fakespot, + Self::Exposure, ] } @@ -293,6 +282,7 @@ impl SuggestRecordType { Self::GlobalConfig => "configuration", Self::AmpMobile => "amp-mobile-suggestions", Self::Fakespot => "fakespot-suggestions", + Self::Exposure => "exposure-suggestions", } } @@ -558,6 +548,86 @@ pub(crate) struct DownloadedFakespotSuggestion { pub url: String, } +/// An exposure suggestion record's inline data +#[derive(Clone, Debug, Deserialize)] +pub(crate) struct DownloadedExposureRecord { + pub suggestion_type: String, +} + +/// An exposure suggestion to ingest from an attachment +#[derive(Clone, Debug, Deserialize)] +pub(crate) struct DownloadedExposureSuggestion { + keywords: Vec>, +} + +impl DownloadedExposureSuggestion { + /// Iterate over all keywords for this suggestion. Iteration may contain + /// duplicate keywords depending on the structure of the data, so do not + /// assume keywords are unique. Duplicates are not filtered out because + /// doing so would require O(number of keywords) space, and the number of + /// keywords can be very large. If you are inserting into the store, rely on + /// uniqueness constraints and use `INSERT OR IGNORE`. + pub fn keywords(&self) -> impl Iterator + '_ { + self.keywords.iter().flat_map(|e| e.keywords()) + } +} + +/// A single full keyword or a `(prefix, suffixes)` tuple representing multiple +/// prefix keywords. Prefix keywords are enumerated by appending to `prefix` +/// each possible prefix of each suffix, including the full suffix. The prefix +/// is also enumerated by itself. Examples: +/// +/// `FullOrPrefixKeywords::Full("some full keyword")` +/// => "some full keyword" +/// +/// `FullOrPrefixKeywords::Prefix(("sug", vec!["gest", "arplum"]))` +/// => "sug" +/// "sugg" +/// "sugge" +/// "sugges" +/// "suggest" +/// "suga" +/// "sugar" +/// "sugarp" +/// "sugarpl" +/// "sugarplu" +/// "sugarplum" +#[derive(Clone, Debug, Deserialize)] +#[serde(untagged)] +enum FullOrPrefixKeywords { + Full(T), + Prefix((T, Vec)), +} + +impl From for FullOrPrefixKeywords { + fn from(full_keyword: T) -> Self { + Self::Full(full_keyword) + } +} + +impl From<(T, Vec)> for FullOrPrefixKeywords { + fn from(prefix_suffixes: (T, Vec)) -> Self { + Self::Prefix(prefix_suffixes) + } +} + +impl FullOrPrefixKeywords { + pub fn keywords(&self) -> Box + '_> { + match self { + FullOrPrefixKeywords::Full(kw) => Box::new(std::iter::once(kw.to_owned())), + FullOrPrefixKeywords::Prefix((prefix, suffixes)) => Box::new( + std::iter::once(prefix.to_owned()).chain(suffixes.iter().flat_map(|suffix| { + let mut kw = prefix.clone(); + suffix.chars().map(move |c| { + kw.push(c); + kw.clone() + }) + })), + ), + } + } +} + /// Weather data to ingest from a weather record #[derive(Clone, Debug, Deserialize)] pub(crate) struct DownloadedWeatherData { @@ -706,4 +776,124 @@ mod test { ], ); } + + fn full_or_prefix_keywords_to_owned( + kws: Vec>, + ) -> Vec> { + kws.iter() + .map(|val| match val { + FullOrPrefixKeywords::Full(s) => FullOrPrefixKeywords::Full(s.to_string()), + FullOrPrefixKeywords::Prefix((prefix, suffixes)) => FullOrPrefixKeywords::Prefix(( + prefix.to_string(), + suffixes.iter().map(|s| s.to_string()).collect(), + )), + }) + .collect() + } + + #[test] + fn test_exposure_keywords() { + let suggestion = DownloadedExposureSuggestion { + keywords: full_or_prefix_keywords_to_owned(vec![ + "no suffixes".into(), + ("empty suffixes", vec![]).into(), + ("empty string suffix", vec![""]).into(), + ("choco", vec!["", "bo", "late"]).into(), + "duplicate 1".into(), + "duplicate 1".into(), + ("dup", vec!["licate 1", "licate 2"]).into(), + ("dup", vec!["lo", "licate 2", "licate 3"]).into(), + ("duplic", vec!["ate 3", "ar", "ate 4"]).into(), + ("du", vec!["plicate 4", "plicate 5", "nk"]).into(), + ]), + }; + + assert_eq!( + Vec::from_iter(suggestion.keywords()), + vec![ + "no suffixes", + "empty suffixes", + "empty string suffix", + "choco", + "chocob", + "chocobo", + "chocol", + "chocola", + "chocolat", + "chocolate", + "duplicate 1", + "duplicate 1", + "dup", + "dupl", + "dupli", + "duplic", + "duplica", + "duplicat", + "duplicate", + "duplicate ", + "duplicate 1", + "dupl", + "dupli", + "duplic", + "duplica", + "duplicat", + "duplicate", + "duplicate ", + "duplicate 2", + "dup", + "dupl", + "duplo", + "dupl", + "dupli", + "duplic", + "duplica", + "duplicat", + "duplicate", + "duplicate ", + "duplicate 2", + "dupl", + "dupli", + "duplic", + "duplica", + "duplicat", + "duplicate", + "duplicate ", + "duplicate 3", + "duplic", + "duplica", + "duplicat", + "duplicate", + "duplicate ", + "duplicate 3", + "duplica", + "duplicar", + "duplica", + "duplicat", + "duplicate", + "duplicate ", + "duplicate 4", + "du", + "dup", + "dupl", + "dupli", + "duplic", + "duplica", + "duplicat", + "duplicate", + "duplicate ", + "duplicate 4", + "dup", + "dupl", + "dupli", + "duplic", + "duplica", + "duplicat", + "duplicate", + "duplicate ", + "duplicate 5", + "dun", + "dunk", + ], + ); + } } diff --git a/components/suggest/src/schema.rs b/components/suggest/src/schema.rs index ae13883119..0f71eb6151 100644 --- a/components/suggest/src/schema.rs +++ b/components/suggest/src/schema.rs @@ -19,7 +19,7 @@ use sql_support::{ /// [`SuggestConnectionInitializer::upgrade_from`]. /// a. If suggestions should be re-ingested after the migration, call `clear_database()` inside /// the migration. -pub const VERSION: u32 = 25; +pub const VERSION: u32 = 26; /// The current Suggest database schema. pub const SQL: &str = " @@ -171,6 +171,13 @@ CREATE TABLE mdn_custom_details( FOREIGN KEY(suggestion_id) REFERENCES suggestions(id) ON DELETE CASCADE ); +CREATE TABLE exposure_custom_details( + suggestion_id INTEGER PRIMARY KEY, + type TEXT NOT NULL, + FOREIGN KEY(suggestion_id) REFERENCES suggestions(id) ON DELETE CASCADE +); +CREATE INDEX exposure_custom_details_type ON exposure_custom_details(type); + CREATE TABLE dismissed_suggestions ( url TEXT PRIMARY KEY ) WITHOUT ROWID; @@ -423,6 +430,20 @@ CREATE TABLE ingested_records( )?; Ok(()) } + 25 => { + // Create the exposure suggestions table and index. + tx.execute_batch( + " +CREATE TABLE exposure_custom_details( + suggestion_id INTEGER PRIMARY KEY, + type TEXT NOT NULL, + FOREIGN KEY(suggestion_id) REFERENCES suggestions(id) ON DELETE CASCADE +); +CREATE INDEX exposure_custom_details_type ON exposure_custom_details(type); + ", + )?; + Ok(()) + } _ => Err(open_database::Error::IncompatibleVersion(version)), } } diff --git a/components/suggest/src/store.rs b/components/suggest/src/store.rs index 08f6a8ef03..db6fe81a0b 100644 --- a/components/suggest/src/store.rs +++ b/components/suggest/src/store.rs @@ -21,10 +21,10 @@ use crate::{ db::{ConnectionType, IngestedRecord, Sqlite3Extension, SuggestDao, SuggestDb}, error::Error, metrics::{DownloadTimer, SuggestIngestionMetrics, SuggestQueryMetrics}, - provider::SuggestionProvider, + provider::{SuggestionProvider, SuggestionProviderConstraints, DEFAULT_INGEST_PROVIDERS}, rs::{ Client, Collection, Record, RemoteSettingsClient, SuggestAttachment, SuggestRecord, - SuggestRecordId, SuggestRecordType, DEFAULT_RECORDS_TYPES, + SuggestRecordId, SuggestRecordType, }, suggestion::AmpSuggestionType, QueryWithMetricsResult, Result, SuggestApiResult, Suggestion, SuggestionQuery, @@ -255,6 +255,7 @@ impl SuggestStore { #[derive(Clone, Default, Debug)] pub struct SuggestIngestionConstraints { pub providers: Option>, + pub provider_constraints: Option, /// Only run ingestion if the table `suggestions` is empty pub empty_only: bool, } @@ -272,6 +273,7 @@ impl SuggestIngestionConstraints { SuggestionProvider::Weather, SuggestionProvider::AmpMobile, SuggestionProvider::Fakespot, + SuggestionProvider::Exposure, ]), ..Self::default() } @@ -336,6 +338,7 @@ impl SuggestStoreInner { SuggestionProvider::Mdn => dao.fetch_mdn_suggestions(&query), SuggestionProvider::Weather => dao.fetch_weather_suggestions(&query), SuggestionProvider::Fakespot => dao.fetch_fakespot_suggestions(&query), + SuggestionProvider::Exposure => dao.fetch_exposure_suggestions(&query), }) })?; suggestions.extend(new_suggestions); @@ -423,24 +426,28 @@ where return Ok(metrics); } - // Figure out which record types we're ingesting - let ingest_record_types = if let Some(rt) = &constraints.providers { - rt.iter() - .map(|x| x.record_type()) - // Always ingest these types - .chain([SuggestRecordType::Icon, SuggestRecordType::GlobalConfig]) - .collect::>() - } else { - DEFAULT_RECORDS_TYPES.into_iter().collect() - }; + // Figure out which record types we're ingesting and group them by + // collection. A record type may be used by multiple providers, but we + // want to ingest each one at most once. + let mut record_types_by_collection = HashMap::>::new(); + for p in constraints + .providers + .as_ref() + .unwrap_or(&DEFAULT_INGEST_PROVIDERS.to_vec()) + .iter() + { + record_types_by_collection + .entry(p.record_type().collection()) + .or_default() + .insert(p.record_type()); + } - // Group record types by collection - let mut record_types_by_collection = HashMap::>::new(); - for record_type in ingest_record_types { + // Always ingest these record types. + for rt in [SuggestRecordType::Icon, SuggestRecordType::GlobalConfig] { record_types_by_collection - .entry(record_type.collection()) + .entry(rt.collection()) .or_default() - .push(record_type); + .insert(rt); } // Create a single write scope for all DB operations @@ -458,7 +465,7 @@ where // For each record type in that collection, calculate the changes and pass them to // [Self::ingest_records] for record_type in record_types { - breadcrumb!("Ingesting {record_type}"); + breadcrumb!("Ingesting record_type: {record_type}"); metrics.measure_ingest(record_type.to_string(), |download_timer| { let changes = RecordChanges::new( records.iter().filter(|r| r.record_type() == record_type), @@ -467,8 +474,9 @@ where && i.collection == collection.name() }), ); - write_scope - .write(|dao| self.ingest_records(dao, collection, changes, download_timer)) + write_scope.write(|dao| { + self.ingest_records(dao, collection, changes, &constraints, download_timer) + }) })?; write_scope.err_if_interrupted()?; } @@ -483,23 +491,24 @@ where dao: &mut SuggestDao, collection: Collection, changes: RecordChanges<'_>, + constraints: &SuggestIngestionConstraints, download_timer: &mut DownloadTimer, ) -> Result<()> { for record in &changes.new { - log::trace!("Ingesting: {}", record.id.as_str()); - self.ingest_record(dao, record, download_timer)?; + log::trace!("Ingesting record ID: {}", record.id.as_str()); + self.ingest_record(dao, record, constraints, download_timer)?; } for record in &changes.updated { // Drop any data that we previously ingested from this record. // Suggestions in particular don't have a stable identifier, and // determining which suggestions in the record actually changed is // more complicated than dropping and re-ingesting all of them. - log::trace!("Reingesting: {}", record.id.as_str()); + log::trace!("Reingesting record ID: {}", record.id.as_str()); dao.delete_record_data(&record.id)?; - self.ingest_record(dao, record, download_timer)?; + self.ingest_record(dao, record, constraints, download_timer)?; } for record in &changes.deleted { - log::trace!("Deleting: {:?}", record.id); + log::trace!("Deleting record ID: {:?}", record.id); dao.delete_record_data(&record.id)?; } dao.update_ingested_records( @@ -515,6 +524,7 @@ where &self, dao: &mut SuggestDao, record: &Record, + constraints: &SuggestIngestionConstraints, download_timer: &mut DownloadTimer, ) -> Result<()> { match &record.payload { @@ -605,6 +615,30 @@ where }, )?; } + SuggestRecord::Exposure(r) => { + // Ingest this record's attachment if its suggestion type + // matches a type in the constraints. + if let Some(suggestion_types) = constraints + .provider_constraints + .as_ref() + .and_then(|c| c.exposure_suggestion_types.as_ref()) + { + if suggestion_types.iter().any(|t| *t == r.suggestion_type) { + self.ingest_attachment( + dao, + record, + download_timer, + |dao, record_id, suggestions| { + dao.insert_exposure_suggestions( + record_id, + &r.suggestion_type, + suggestions, + ) + }, + )?; + } + } + } } Ok(()) } @@ -707,7 +741,13 @@ where ); writer .write(|dao| { - self.ingest_records(dao, ingest_record_type.collection(), changes, &mut timer) + self.ingest_records( + dao, + ingest_record_type.collection(), + changes, + &SuggestIngestionConstraints::default(), + &mut timer, + ) }) .unwrap(); } @@ -1962,9 +2002,11 @@ mod tests { "weather", "weather-1", json!({ - "min_keyword_length": 3, - "keywords": ["ab", "xyz", "weather"], - "score": "0.24" + "weather": { + "min_keyword_length": 3, + "keywords": ["ab", "xyz", "weather"], + "score": "0.24" + }, }), )); store.ingest(SuggestIngestionConstraints::all_providers()); @@ -2066,7 +2108,9 @@ mod tests { "configuration", "configuration-1", json!({ - "show_less_frequently_cap": 3, + "configuration": { + "show_less_frequently_cap": 3, + }, }), )); store.ingest(SuggestIngestionConstraints::all_providers()); @@ -2119,9 +2163,11 @@ mod tests { "weather", "weather-1", json!({ - "min_keyword_length": 3, - "keywords": ["weather"], - "score": "0.24" + "weather": { + "min_keyword_length": 3, + "keywords": ["weather"], + "score": "0.24" + }, }), )); store.ingest(SuggestIngestionConstraints::all_providers()); @@ -2410,4 +2456,464 @@ mod tests { ); Ok(()) } + + #[test] + fn exposure_basic() -> anyhow::Result<()> { + before_each(); + + let store = TestStore::new( + MockRemoteSettingsClient::default() + .with_full_record( + "exposure-suggestions", + "exposure-0", + Some(json!({ + "suggestion_type": "aaa", + })), + Some(json!({ + "keywords": [ + "aaa keyword", + "both keyword", + ["common prefix", [" aaa"]], + ["choco", ["bo", "late"]], + ["dup", ["licate 1", "licate 2"]], + ], + })), + ) + .with_full_record( + "exposure-suggestions", + "exposure-1", + Some(json!({ + "suggestion_type": "bbb", + })), + Some(json!({ + "keywords": [ + "bbb keyword", + "both keyword", + ["common prefix", [" bbb"]], + ], + })), + ), + ); + store.ingest(SuggestIngestionConstraints { + providers: Some(vec![SuggestionProvider::Exposure]), + provider_constraints: Some(SuggestionProviderConstraints { + exposure_suggestion_types: Some(vec!["aaa".to_string(), "bbb".to_string()]), + }), + ..SuggestIngestionConstraints::all_providers() + }); + + let no_matches = vec!["aaa", "both", "common prefi", "choc", "chocolate extra"]; + for query in &no_matches { + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["aaa"])), + vec![], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["bbb"])), + vec![], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["aaa", "bbb"])), + vec![], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["aaa", "zzz"])), + vec![], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["zzz"])), + vec![], + ); + } + + let aaa_only_matches = vec![ + "aaa keyword", + "common prefix a", + "common prefix aa", + "common prefix aaa", + "choco", + "chocob", + "chocobo", + "chocol", + "chocolate", + "dup", + "dupl", + "duplicate", + "duplicate ", + "duplicate 1", + "duplicate 2", + ]; + for query in &aaa_only_matches { + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["aaa"])), + vec![Suggestion::Exposure { + suggestion_type: "aaa".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["aaa", "bbb"])), + vec![Suggestion::Exposure { + suggestion_type: "aaa".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["bbb", "aaa"])), + vec![Suggestion::Exposure { + suggestion_type: "aaa".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["aaa", "zzz"])), + vec![Suggestion::Exposure { + suggestion_type: "aaa".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["zzz", "aaa"])), + vec![Suggestion::Exposure { + suggestion_type: "aaa".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["bbb"])), + vec![], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["zzz"])), + vec![], + ); + } + + let bbb_only_matches = vec![ + "bbb keyword", + "common prefix b", + "common prefix bb", + "common prefix bbb", + ]; + for query in &bbb_only_matches { + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["bbb"])), + vec![Suggestion::Exposure { + suggestion_type: "bbb".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["bbb", "aaa"])), + vec![Suggestion::Exposure { + suggestion_type: "bbb".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["aaa", "bbb"])), + vec![Suggestion::Exposure { + suggestion_type: "bbb".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["bbb", "zzz"])), + vec![Suggestion::Exposure { + suggestion_type: "bbb".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["zzz", "bbb"])), + vec![Suggestion::Exposure { + suggestion_type: "bbb".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["aaa"])), + vec![], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["zzz"])), + vec![], + ); + } + + let both_matches = vec!["both keyword", "common prefix", "common prefix "]; + for query in &both_matches { + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["aaa"])), + vec![Suggestion::Exposure { + suggestion_type: "aaa".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["bbb"])), + vec![Suggestion::Exposure { + suggestion_type: "bbb".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["aaa", "bbb"])), + vec![ + Suggestion::Exposure { + suggestion_type: "aaa".into(), + score: 1.0, + }, + Suggestion::Exposure { + suggestion_type: "bbb".into(), + score: 1.0, + }, + ], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["bbb", "aaa"])), + vec![ + Suggestion::Exposure { + suggestion_type: "aaa".into(), + score: 1.0, + }, + Suggestion::Exposure { + suggestion_type: "bbb".into(), + score: 1.0, + }, + ], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["aaa", "zzz"])), + vec![Suggestion::Exposure { + suggestion_type: "aaa".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["zzz", "aaa"])), + vec![Suggestion::Exposure { + suggestion_type: "aaa".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["bbb", "zzz"])), + vec![Suggestion::Exposure { + suggestion_type: "bbb".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["zzz", "bbb"])), + vec![Suggestion::Exposure { + suggestion_type: "bbb".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["aaa", "zzz", "bbb"])), + vec![ + Suggestion::Exposure { + suggestion_type: "aaa".into(), + score: 1.0, + }, + Suggestion::Exposure { + suggestion_type: "bbb".into(), + score: 1.0, + }, + ], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["zzz"])), + vec![], + ); + } + + Ok(()) + } + + #[test] + fn exposure_spread_across_multiple_records() -> anyhow::Result<()> { + before_each(); + + let mut store = TestStore::new( + MockRemoteSettingsClient::default() + .with_full_record( + "exposure-suggestions", + "exposure-0", + Some(json!({ + "suggestion_type": "aaa", + })), + Some(json!({ + "keywords": [ + "record 0 keyword", + ["sug", ["gest"]], + ], + })), + ) + .with_full_record( + "exposure-suggestions", + "exposure-1", + Some(json!({ + "suggestion_type": "aaa", + })), + Some(json!({ + "keywords": [ + "record 1 keyword", + ["sug", ["arplum"]], + ], + })), + ), + ); + store.ingest(SuggestIngestionConstraints { + providers: Some(vec![SuggestionProvider::Exposure]), + provider_constraints: Some(SuggestionProviderConstraints { + exposure_suggestion_types: Some(vec!["aaa".to_string()]), + }), + ..SuggestIngestionConstraints::all_providers() + }); + + let matches = vec![ + "record 0 keyword", + "sug", + "sugg", + "sugge", + "sugges", + "suggest", + "record 1 keyword", + "suga", + "sugar", + "sugarp", + "sugarpl", + "sugarplu", + "sugarplum", + ]; + for query in &matches { + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["aaa"])), + vec![Suggestion::Exposure { + suggestion_type: "aaa".into(), + score: 1.0, + }], + ); + } + + // Delete the first record. + store + .client_mut() + .delete_record(Collection::Quicksuggest.name(), "exposure-0"); + store.ingest(SuggestIngestionConstraints { + providers: Some(vec![SuggestionProvider::Exposure]), + provider_constraints: Some(SuggestionProviderConstraints { + exposure_suggestion_types: Some(vec!["aaa".to_string()]), + }), + ..SuggestIngestionConstraints::all_providers() + }); + + // Keywords from the second record should still return the suggestion. + let record_1_matches = vec![ + "record 1 keyword", + "sug", + "suga", + "sugar", + "sugarp", + "sugarpl", + "sugarplu", + "sugarplum", + ]; + for query in &record_1_matches { + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["aaa"])), + vec![Suggestion::Exposure { + suggestion_type: "aaa".into(), + score: 1.0, + }], + ); + } + + // Keywords from the first record should not return the suggestion. + let record_0_matches = vec!["record 0 keyword", "sugg", "sugge", "sugges", "suggest"]; + for query in &record_0_matches { + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure(query, &["exposure-test"])), + vec![] + ); + } + + Ok(()) + } + + #[test] + fn exposure_not_ingested() -> anyhow::Result<()> { + before_each(); + + // Create suggestions with types "aaa" and "bbb". + let store = TestStore::new( + MockRemoteSettingsClient::default() + .with_full_record( + "exposure-suggestions", + "exposure-0", + Some(json!({ + "suggestion_type": "aaa", + })), + Some(json!({ + "keywords": ["aaa keyword"], + })), + ) + .with_full_record( + "exposure-suggestions", + "exposure-1", + Some(json!({ + "suggestion_type": "bbb", + })), + Some(json!({ + "keywords": ["bbb keyword"], + })), + ), + ); + + // Ingest only the "bbb" suggestion. + store.ingest(SuggestIngestionConstraints { + providers: Some(vec![SuggestionProvider::Exposure]), + provider_constraints: Some(SuggestionProviderConstraints { + exposure_suggestion_types: Some(vec!["bbb".to_string()]), + }), + ..SuggestIngestionConstraints::all_providers() + }); + + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure("aaa keyword", &["aaa"])), + vec![], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure("aaa keyword", &["bbb"])), + vec![], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure("aaa keyword", &["aaa", "bbb"])), + vec![], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure("bbb keyword", &["aaa"])), + vec![], + ); + + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure("bbb keyword", &["bbb"])), + vec![Suggestion::Exposure { + suggestion_type: "bbb".into(), + score: 1.0, + }], + ); + assert_eq!( + store.fetch_suggestions(SuggestionQuery::exposure("bbb keyword", &["aaa", "bbb"])), + vec![Suggestion::Exposure { + suggestion_type: "bbb".into(), + score: 1.0, + }], + ); + + Ok(()) + } } diff --git a/components/suggest/src/suggest.udl b/components/suggest/src/suggest.udl index b14b23b967..988a6b5eac 100644 --- a/components/suggest/src/suggest.udl +++ b/components/suggest/src/suggest.udl @@ -35,6 +35,7 @@ enum SuggestionProvider { "Weather", "AmpMobile", "Fakespot", + "Exposure", }; [Enum] @@ -107,16 +108,22 @@ interface Suggestion { string? icon_mimetype, f64 score ); + Exposure( + string suggestion_type, + f64 score + ); }; dictionary SuggestionQuery { string keyword; sequence providers; + SuggestionProviderConstraints? provider_constraints = null; i32? limit = null; }; dictionary SuggestIngestionConstraints { sequence? providers = null; + SuggestionProviderConstraints? provider_constraints = null; // Only ingest if the table `suggestions` is empty. // // This is indented to handle periodic updates. Consumers can schedule an ingest with @@ -127,6 +134,15 @@ dictionary SuggestIngestionConstraints { boolean empty_only = false; }; +// Some providers manage multiple suggestion subtypes. Queries, ingests, and +// other operations on those providers must be constrained to a desired subtype. +dictionary SuggestionProviderConstraints { + // `Exposure` provider - For each desired exposure suggestion type, this + // should contain the value of the `suggestion_type` field of its remote + // settings record(s). + sequence? exposure_suggestion_types = null; +}; + dictionary SuggestIngestionMetrics { // Samples for the `suggest.ingestion_time` metric sequence ingestion_times; diff --git a/components/suggest/src/suggestion.rs b/components/suggest/src/suggestion.rs index 6fbb836b54..24936b505b 100644 --- a/components/suggest/src/suggestion.rs +++ b/components/suggest/src/suggestion.rs @@ -92,6 +92,10 @@ pub enum Suggestion { icon_mimetype: Option, score: f64, }, + Exposure { + suggestion_type: String, + score: f64, + }, } impl PartialOrd for Suggestion { @@ -102,22 +106,9 @@ impl PartialOrd for Suggestion { impl Ord for Suggestion { fn cmp(&self, other: &Self) -> std::cmp::Ordering { - let a_score = match self { - Suggestion::Amp { score, .. } - | Suggestion::Pocket { score, .. } - | Suggestion::Amo { score, .. } => score, - Suggestion::Fakespot { score, .. } => score, - _ => &DEFAULT_SUGGESTION_SCORE, - }; - let b_score = match other { - Suggestion::Amp { score, .. } - | Suggestion::Pocket { score, .. } - | Suggestion::Amo { score, .. } => score, - Suggestion::Fakespot { score, .. } => score, - _ => &DEFAULT_SUGGESTION_SCORE, - }; - b_score - .partial_cmp(a_score) + other + .score() + .partial_cmp(&self.score()) .unwrap_or(std::cmp::Ordering::Equal) } } @@ -160,9 +151,9 @@ impl Suggestion { | Self::Wikipedia { title, .. } | Self::Amo { title, .. } | Self::Yelp { title, .. } - | Self::Mdn { title, .. } => title, - Self::Weather { .. } => "weather", - Self::Fakespot { title, .. } => title, + | Self::Mdn { title, .. } + | Self::Fakespot { title, .. } => title, + _ => "untitled", } } @@ -175,6 +166,20 @@ impl Suggestion { _ => None, } } + + pub fn score(&self) -> f64 { + match self { + Self::Amp { score, .. } + | Self::Pocket { score, .. } + | Self::Amo { score, .. } + | Self::Yelp { score, .. } + | Self::Mdn { score, .. } + | Self::Weather { score, .. } + | Self::Fakespot { score, .. } + | Self::Exposure { score, .. } => *score, + Self::Wikipedia { .. } => DEFAULT_SUGGESTION_SCORE, + } + } } #[cfg(test)] diff --git a/components/suggest/src/testing/client.rs b/components/suggest/src/testing/client.rs index e07454f45b..2d8034d283 100644 --- a/components/suggest/src/testing/client.rs +++ b/components/suggest/src/testing/client.rs @@ -12,6 +12,7 @@ use crate::{ db::SuggestDao, error::Error, rs::{Client, Collection, Record, SuggestRecordId, SuggestRecordType}, + testing::JsonExt, Result, }; @@ -71,6 +72,17 @@ impl MockRemoteSettingsClient { self } + pub fn with_full_record( + mut self, + record_type: &str, + record_id: &str, + inline_data: Option, + items: Option, + ) -> Self { + self.add_full_record(record_type, record_id, inline_data, items); + self + } + // Non-Consuming Builder API, this is best for updating an existing client /// Add a record to the mock data @@ -83,25 +95,7 @@ impl MockRemoteSettingsClient { record_id: &str, items: JsonValue, ) -> &mut Self { - let location = format!("{record_type}-{record_id}.json"); - self.records.push(Record { - id: SuggestRecordId::new(record_id.to_string()), - collection: record_type_for_str(record_type).collection(), - last_modified: self.last_modified_timestamp, - attachment: Some(Attachment { - filename: location.clone(), - mimetype: "application/json".into(), - hash: "".into(), - size: 0, - location: location.clone(), - }), - payload: serde_json::from_value(json!({"type": record_type})).unwrap(), - }); - self.attachments.insert( - location, - serde_json::to_vec(&items).expect("error serializing attachment data"), - ); - self + self.add_full_record(record_type, record_id, None, Some(items)) } /// Add a record for an icon to the mock data @@ -133,14 +127,7 @@ impl MockRemoteSettingsClient { record_type: &str, record_id: &str, ) -> &mut Self { - self.records.push(Record { - id: SuggestRecordId::new(record_id.to_string()), - collection: record_type_for_str(record_type).collection(), - last_modified: self.last_modified_timestamp, - attachment: None, - payload: serde_json::from_value(json!({"type": record_type})).unwrap(), - }); - self + self.add_full_record(record_type, record_id, None, None) } /// Add a record to the mock data, with data stored inline rather than in an attachment @@ -153,17 +140,44 @@ impl MockRemoteSettingsClient { record_id: &str, inline_data: JsonValue, ) -> &mut Self { + self.add_full_record(record_type, record_id, Some(inline_data), None) + } + + /// Add a record with optional extra fields stored inline and attachment + /// items + pub fn add_full_record( + &mut self, + record_type: &str, + record_id: &str, + inline_data: Option, + items: Option, + ) -> &mut Self { + let location = format!("{record_type}-{record_id}.json"); self.records.push(Record { id: SuggestRecordId::new(record_id.to_string()), collection: record_type_for_str(record_type).collection(), last_modified: self.last_modified_timestamp, - payload: serde_json::from_value(json!({ - "type": record_type, - record_type: inline_data, - })) + payload: serde_json::from_value( + json!({ + "type": record_type, + }) + .merge(inline_data.unwrap_or(json!({}))), + ) .unwrap(), - attachment: None, + attachment: items.as_ref().map(|_| Attachment { + filename: location.clone(), + mimetype: "application/json".into(), + hash: "".into(), + size: 0, + location: location.clone(), + }), }); + if let Some(i) = items { + self.attachments.insert( + location, + serde_json::to_vec(&i).expect("error serializing attachment data"), + ); + } self } diff --git a/components/suggest/src/testing/mod.rs b/components/suggest/src/testing/mod.rs index 0a228b5bf0..7a788f91bd 100644 --- a/components/suggest/src/testing/mod.rs +++ b/components/suggest/src/testing/mod.rs @@ -43,6 +43,7 @@ impl Suggestion { Self::Weather { score, .. } => score, Self::Wikipedia { .. } => panic!("with_score not valid for wikipedia suggestions"), Self::Fakespot { score, .. } => score, + Self::Exposure { score, .. } => score, }; *current_score = score; self diff --git a/examples/suggest-cli/src/main.rs b/examples/suggest-cli/src/main.rs index dec11601fa..e1374bc4ff 100644 --- a/examples/suggest-cli/src/main.rs +++ b/examples/suggest-cli/src/main.rs @@ -186,7 +186,7 @@ fn query( None => SuggestionProvider::all().to_vec(), }, keyword: input, - limit: None, + ..SuggestionQuery::default() }; let mut results = store .query_with_metrics(query)