Skip to content

Commit

Permalink
Bug 1901805 - Starting Fakespot ingestion code
Browse files Browse the repository at this point in the history
Defined the `fakespot` feature and put most of the changes behind that
feature flag.

Updated ingest code to use 2 remote settings clients, one for fakespot
and one for everything else.  I think I like this setup going forward,
since it makes it easy for new suggestions to have their own colleciton
if we decide it makes sense.

Implemented some very simple matching logic, the next step is for the
SNG team to update this with the real matching logic.  I tried to leave
`FAKESPOT-TODO` comment breadcrumbs where they need to update the code.
  • Loading branch information
bendk committed Jun 18, 2024
1 parent b609482 commit a8c15ba
Show file tree
Hide file tree
Showing 15 changed files with 609 additions and 48 deletions.
2 changes: 2 additions & 0 deletions components/suggest/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ uniffi = { workspace = true, features = ["build"] }
[features]
# Required for the benchmarks to work, wasted bytes otherwise.
benchmark_api = ["tempfile", "viaduct-reqwest"]
# Enable fakespot suggestions. This is behind a feature flag since it's currently a WIP.
fakespot = []

[[bench]]
name = "benchmark_all"
Expand Down
4 changes: 4 additions & 0 deletions components/suggest/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,9 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

fn main() {
#[cfg(feature = "fakespot")]
uniffi::generate_scaffolding("./src/suggest-fakespot.udl").unwrap();

#[cfg(not(feature = "fakespot"))]
uniffi::generate_scaffolding("./src/suggest.udl").unwrap();
}
13 changes: 3 additions & 10 deletions components/suggest/src/benchmarks/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

use crate::{rs, Result};
use parking_lot::Mutex;
use remote_settings::{Client, RemoteSettingsConfig};
use std::collections::HashMap;

/// Remotes settings client that runs during the benchmark warm-up phase.
Expand All @@ -13,20 +12,14 @@ use std::collections::HashMap;
/// Then it can be converted into a [RemoteSettingsBenchmarkClient], which allows benchmark code to exclude the network request time.
/// [RemoteSettingsBenchmarkClient] implements [rs::Client] by getting data from a HashMap rather than hitting the network.
pub struct RemoteSettingsWarmUpClient {
client: Client,
client: rs::RemoteSettingsClient,
pub get_records_responses: Mutex<HashMap<rs::RecordRequest, Vec<rs::Record>>>,
}

impl RemoteSettingsWarmUpClient {
pub fn new() -> Self {
Self {
client: Client::new(RemoteSettingsConfig {
server: None,
server_url: None,
bucket_name: None,
collection_name: crate::rs::REMOTE_SETTINGS_COLLECTION.into(),
})
.unwrap(),
client: rs::RemoteSettingsClient::new(None, None, None).unwrap(),
get_records_responses: Mutex::new(HashMap::new()),
}
}
Expand All @@ -40,7 +33,7 @@ impl Default for RemoteSettingsWarmUpClient {

impl rs::Client for RemoteSettingsWarmUpClient {
fn get_records(&self, request: rs::RecordRequest) -> Result<Vec<rs::Record>> {
let response = <Client as rs::Client>::get_records(&self.client, request.clone())?;
let response = self.client.get_records(request.clone())?;
self.get_records_responses
.lock()
.insert(request, response.clone());
Expand Down
104 changes: 104 additions & 0 deletions components/suggest/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ use crate::{
Result, SuggestionQuery,
};

#[cfg(feature = "fakespot")]
use crate::rs::DownloadedFakespotSuggestion;

/// The metadata key whose value is a JSON string encoding a
/// `SuggestGlobalConfig`, which contains global Suggest configuration data.
pub const GLOBAL_CONFIG_META_KEY: &str = "global_config";
Expand Down Expand Up @@ -216,6 +219,8 @@ impl<'a> SuggestDao<'a> {
SuggestionProvider::Yelp => self.fetch_yelp_suggestions(query),
SuggestionProvider::Mdn => self.fetch_mdn_suggestions(query),
SuggestionProvider::Weather => self.fetch_weather_suggestions(query),
#[cfg(feature = "fakespot")]
SuggestionProvider::Fakespot => self.fetch_fakespot_suggestions(query),
}?;
acc.extend(suggestions);
Ok(acc)
Expand Down Expand Up @@ -672,6 +677,45 @@ impl<'a> SuggestDao<'a> {
Ok(suggestions)
}

#[cfg(feature = "fakespot")]
/// Fetches fakespot suggestions
pub fn fetch_fakespot_suggestions(&self, query: &SuggestionQuery) -> Result<Vec<Suggestion>> {
// FAKESPOT-TODO: The SNG will update this based on the results of their FTS experimentation
self.conn.query_rows_and_then_cached(
r#"
SELECT
s.title,
s.url,
s.score,
f.fakespot_grade,
f.product_id,
f.rating,
f.total_reviews
FROM
suggestions s
JOIN
fakespot_custom_details f
ON f.suggestion_id = s.id
WHERE
s.title LIKE '%' || ? || '%'
ORDER BY
s.score DESC
"#,
(&query.keyword,),
|row| {
Ok(Suggestion::Fakespot {
title: row.get(0)?,
url: row.get(1)?,
score: row.get(2)?,
fakespot_grade: row.get(3)?,
product_id: row.get(4)?,
rating: row.get(5)?,
total_reviews: row.get(6)?,
})
},
)
}

/// Inserts all suggestions from a downloaded AMO attachment into
/// the database.
pub fn insert_amo_suggestions(
Expand Down Expand Up @@ -878,6 +922,29 @@ impl<'a> SuggestDao<'a> {
Ok(())
}

/// Inserts all suggestions from a downloaded Fakespot attachment into the database.
#[cfg(feature = "fakespot")]
pub fn insert_fakespot_suggestions(
&mut self,
record_id: &SuggestRecordId,
suggestions: &[DownloadedFakespotSuggestion],
) -> Result<()> {
// FAKESPOT-TODO: The SNG will update this based on the results of their FTS experimentation
let mut suggestion_insert = SuggestionInsertStatement::new(self.conn)?;
let mut fakespot_insert = FakespotInsertStatement::new(self.conn)?;
for suggestion in suggestions {
let suggestion_id = suggestion_insert.execute(
record_id,
&suggestion.title,
&suggestion.url,
suggestion.score,
SuggestionProvider::Fakespot,
)?;
fakespot_insert.execute(suggestion_id, suggestion)?;
}
Ok(())
}

/// Inserts weather record data into the database.
pub fn insert_weather_data(
&mut self,
Expand Down Expand Up @@ -1288,6 +1355,43 @@ impl<'conn> MdnInsertStatement<'conn> {
}
}

#[cfg(feature = "fakespot")]
struct FakespotInsertStatement<'conn>(rusqlite::Statement<'conn>);

#[cfg(feature = "fakespot")]
impl<'conn> FakespotInsertStatement<'conn> {
fn new(conn: &'conn Connection) -> Result<Self> {
Ok(Self(conn.prepare(
"INSERT INTO fakespot_custom_details(
suggestion_id,
fakespot_grade,
product_id,
rating,
total_reviews
)
VALUES(?, ?, ?, ?, ?)
",
)?))
}

fn execute(
&mut self,
suggestion_id: i64,
fakespot: &DownloadedFakespotSuggestion,
) -> Result<()> {
self.0
.execute((
suggestion_id,
&fakespot.fakespot_grade,
&fakespot.product_id,
fakespot.rating,
fakespot.total_reviews,
))
.with_context("fakespot insert")?;
Ok(())
}
}

struct KeywordInsertStatement<'conn>(rusqlite::Statement<'conn>);

impl<'conn> KeywordInsertStatement<'conn> {
Expand Down
4 changes: 4 additions & 0 deletions components/suggest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,8 @@ pub use suggestion::{raw_suggestion_url_matches, Suggestion};
pub(crate) type Result<T> = std::result::Result<T, error::Error>;
pub type SuggestApiResult<T> = std::result::Result<T, error::SuggestApiError>;

#[cfg(not(feature = "fakespot"))]
uniffi::include_scaffolding!("suggest");

#[cfg(feature = "fakespot")]
uniffi::include_scaffolding!("suggest-fakespot");
25 changes: 25 additions & 0 deletions components/suggest/src/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ pub enum SuggestionProvider {
Mdn = 6,
Weather = 7,
AmpMobile = 8,
#[cfg(feature = "fakespot")]
Fakespot = 9,
}

impl FromSql for SuggestionProvider {
Expand All @@ -35,6 +37,7 @@ impl FromSql for SuggestionProvider {
}

impl SuggestionProvider {
#[cfg(not(feature = "fakespot"))]
pub fn all() -> [Self; 8] {
[
Self::Amp,
Expand All @@ -48,6 +51,21 @@ impl SuggestionProvider {
]
}

#[cfg(feature = "fakespot")]
pub fn all() -> [Self; 9] {
[
Self::Amp,
Self::Wikipedia,
Self::Amo,
Self::Pocket,
Self::Yelp,
Self::Mdn,
Self::Weather,
Self::AmpMobile,
Self::Fakespot,
]
}

#[inline]
pub(crate) fn from_u8(v: u8) -> Option<Self> {
match v {
Expand All @@ -58,6 +76,9 @@ impl SuggestionProvider {
5 => Some(SuggestionProvider::Yelp),
6 => Some(SuggestionProvider::Mdn),
7 => Some(SuggestionProvider::Weather),
8 => Some(SuggestionProvider::AmpMobile),
#[cfg(feature = "fakespot")]
9 => Some(SuggestionProvider::Fakespot),
_ => None,
}
}
Expand Down Expand Up @@ -105,6 +126,10 @@ impl SuggestionProvider {
SuggestRecordType::GlobalConfig,
]
}
#[cfg(feature = "fakespot")]
SuggestionProvider::Fakespot => {
vec![SuggestRecordType::Fakespot]
}
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions components/suggest/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,15 @@ impl SuggestionQuery {
}
}

#[cfg(feature = "fakespot")]
pub fn fakespot(keyword: &str) -> Self {
Self {
keyword: keyword.into(),
providers: vec![SuggestionProvider::Fakespot],
limit: None,
}
}

pub fn weather(keyword: &str) -> Self {
Self {
keyword: keyword.into(),
Expand Down
Loading

0 comments on commit a8c15ba

Please sign in to comment.