Skip to content

Commit a8c15ba

Browse files
committed
Bug 1901805 - Starting Fakespot ingestion code
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.
1 parent b609482 commit a8c15ba

File tree

15 files changed

+609
-48
lines changed

15 files changed

+609
-48
lines changed

components/suggest/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ uniffi = { workspace = true, features = ["build"] }
4141
[features]
4242
# Required for the benchmarks to work, wasted bytes otherwise.
4343
benchmark_api = ["tempfile", "viaduct-reqwest"]
44+
# Enable fakespot suggestions. This is behind a feature flag since it's currently a WIP.
45+
fakespot = []
4446

4547
[[bench]]
4648
name = "benchmark_all"

components/suggest/build.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,9 @@
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

55
fn main() {
6+
#[cfg(feature = "fakespot")]
7+
uniffi::generate_scaffolding("./src/suggest-fakespot.udl").unwrap();
8+
9+
#[cfg(not(feature = "fakespot"))]
610
uniffi::generate_scaffolding("./src/suggest.udl").unwrap();
711
}

components/suggest/src/benchmarks/client.rs

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
use crate::{rs, Result};
66
use parking_lot::Mutex;
7-
use remote_settings::{Client, RemoteSettingsConfig};
87
use std::collections::HashMap;
98

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

2019
impl RemoteSettingsWarmUpClient {
2120
pub fn new() -> Self {
2221
Self {
23-
client: Client::new(RemoteSettingsConfig {
24-
server: None,
25-
server_url: None,
26-
bucket_name: None,
27-
collection_name: crate::rs::REMOTE_SETTINGS_COLLECTION.into(),
28-
})
29-
.unwrap(),
22+
client: rs::RemoteSettingsClient::new(None, None, None).unwrap(),
3023
get_records_responses: Mutex::new(HashMap::new()),
3124
}
3225
}
@@ -40,7 +33,7 @@ impl Default for RemoteSettingsWarmUpClient {
4033

4134
impl rs::Client for RemoteSettingsWarmUpClient {
4235
fn get_records(&self, request: rs::RecordRequest) -> Result<Vec<rs::Record>> {
43-
let response = <Client as rs::Client>::get_records(&self.client, request.clone())?;
36+
let response = self.client.get_records(request.clone())?;
4437
self.get_records_responses
4538
.lock()
4639
.insert(request, response.clone());

components/suggest/src/db.rs

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ use crate::{
3030
Result, SuggestionQuery,
3131
};
3232

33+
#[cfg(feature = "fakespot")]
34+
use crate::rs::DownloadedFakespotSuggestion;
35+
3336
/// The metadata key whose value is a JSON string encoding a
3437
/// `SuggestGlobalConfig`, which contains global Suggest configuration data.
3538
pub const GLOBAL_CONFIG_META_KEY: &str = "global_config";
@@ -216,6 +219,8 @@ impl<'a> SuggestDao<'a> {
216219
SuggestionProvider::Yelp => self.fetch_yelp_suggestions(query),
217220
SuggestionProvider::Mdn => self.fetch_mdn_suggestions(query),
218221
SuggestionProvider::Weather => self.fetch_weather_suggestions(query),
222+
#[cfg(feature = "fakespot")]
223+
SuggestionProvider::Fakespot => self.fetch_fakespot_suggestions(query),
219224
}?;
220225
acc.extend(suggestions);
221226
Ok(acc)
@@ -672,6 +677,45 @@ impl<'a> SuggestDao<'a> {
672677
Ok(suggestions)
673678
}
674679

680+
#[cfg(feature = "fakespot")]
681+
/// Fetches fakespot suggestions
682+
pub fn fetch_fakespot_suggestions(&self, query: &SuggestionQuery) -> Result<Vec<Suggestion>> {
683+
// FAKESPOT-TODO: The SNG will update this based on the results of their FTS experimentation
684+
self.conn.query_rows_and_then_cached(
685+
r#"
686+
SELECT
687+
s.title,
688+
s.url,
689+
s.score,
690+
f.fakespot_grade,
691+
f.product_id,
692+
f.rating,
693+
f.total_reviews
694+
FROM
695+
suggestions s
696+
JOIN
697+
fakespot_custom_details f
698+
ON f.suggestion_id = s.id
699+
WHERE
700+
s.title LIKE '%' || ? || '%'
701+
ORDER BY
702+
s.score DESC
703+
"#,
704+
(&query.keyword,),
705+
|row| {
706+
Ok(Suggestion::Fakespot {
707+
title: row.get(0)?,
708+
url: row.get(1)?,
709+
score: row.get(2)?,
710+
fakespot_grade: row.get(3)?,
711+
product_id: row.get(4)?,
712+
rating: row.get(5)?,
713+
total_reviews: row.get(6)?,
714+
})
715+
},
716+
)
717+
}
718+
675719
/// Inserts all suggestions from a downloaded AMO attachment into
676720
/// the database.
677721
pub fn insert_amo_suggestions(
@@ -878,6 +922,29 @@ impl<'a> SuggestDao<'a> {
878922
Ok(())
879923
}
880924

925+
/// Inserts all suggestions from a downloaded Fakespot attachment into the database.
926+
#[cfg(feature = "fakespot")]
927+
pub fn insert_fakespot_suggestions(
928+
&mut self,
929+
record_id: &SuggestRecordId,
930+
suggestions: &[DownloadedFakespotSuggestion],
931+
) -> Result<()> {
932+
// FAKESPOT-TODO: The SNG will update this based on the results of their FTS experimentation
933+
let mut suggestion_insert = SuggestionInsertStatement::new(self.conn)?;
934+
let mut fakespot_insert = FakespotInsertStatement::new(self.conn)?;
935+
for suggestion in suggestions {
936+
let suggestion_id = suggestion_insert.execute(
937+
record_id,
938+
&suggestion.title,
939+
&suggestion.url,
940+
suggestion.score,
941+
SuggestionProvider::Fakespot,
942+
)?;
943+
fakespot_insert.execute(suggestion_id, suggestion)?;
944+
}
945+
Ok(())
946+
}
947+
881948
/// Inserts weather record data into the database.
882949
pub fn insert_weather_data(
883950
&mut self,
@@ -1288,6 +1355,43 @@ impl<'conn> MdnInsertStatement<'conn> {
12881355
}
12891356
}
12901357

1358+
#[cfg(feature = "fakespot")]
1359+
struct FakespotInsertStatement<'conn>(rusqlite::Statement<'conn>);
1360+
1361+
#[cfg(feature = "fakespot")]
1362+
impl<'conn> FakespotInsertStatement<'conn> {
1363+
fn new(conn: &'conn Connection) -> Result<Self> {
1364+
Ok(Self(conn.prepare(
1365+
"INSERT INTO fakespot_custom_details(
1366+
suggestion_id,
1367+
fakespot_grade,
1368+
product_id,
1369+
rating,
1370+
total_reviews
1371+
)
1372+
VALUES(?, ?, ?, ?, ?)
1373+
",
1374+
)?))
1375+
}
1376+
1377+
fn execute(
1378+
&mut self,
1379+
suggestion_id: i64,
1380+
fakespot: &DownloadedFakespotSuggestion,
1381+
) -> Result<()> {
1382+
self.0
1383+
.execute((
1384+
suggestion_id,
1385+
&fakespot.fakespot_grade,
1386+
&fakespot.product_id,
1387+
fakespot.rating,
1388+
fakespot.total_reviews,
1389+
))
1390+
.with_context("fakespot insert")?;
1391+
Ok(())
1392+
}
1393+
}
1394+
12911395
struct KeywordInsertStatement<'conn>(rusqlite::Statement<'conn>);
12921396

12931397
impl<'conn> KeywordInsertStatement<'conn> {

components/suggest/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,8 @@ pub use suggestion::{raw_suggestion_url_matches, Suggestion};
3131
pub(crate) type Result<T> = std::result::Result<T, error::Error>;
3232
pub type SuggestApiResult<T> = std::result::Result<T, error::SuggestApiError>;
3333

34+
#[cfg(not(feature = "fakespot"))]
3435
uniffi::include_scaffolding!("suggest");
36+
37+
#[cfg(feature = "fakespot")]
38+
uniffi::include_scaffolding!("suggest-fakespot");

components/suggest/src/provider.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ pub enum SuggestionProvider {
2222
Mdn = 6,
2323
Weather = 7,
2424
AmpMobile = 8,
25+
#[cfg(feature = "fakespot")]
26+
Fakespot = 9,
2527
}
2628

2729
impl FromSql for SuggestionProvider {
@@ -35,6 +37,7 @@ impl FromSql for SuggestionProvider {
3537
}
3638

3739
impl SuggestionProvider {
40+
#[cfg(not(feature = "fakespot"))]
3841
pub fn all() -> [Self; 8] {
3942
[
4043
Self::Amp,
@@ -48,6 +51,21 @@ impl SuggestionProvider {
4851
]
4952
}
5053

54+
#[cfg(feature = "fakespot")]
55+
pub fn all() -> [Self; 9] {
56+
[
57+
Self::Amp,
58+
Self::Wikipedia,
59+
Self::Amo,
60+
Self::Pocket,
61+
Self::Yelp,
62+
Self::Mdn,
63+
Self::Weather,
64+
Self::AmpMobile,
65+
Self::Fakespot,
66+
]
67+
}
68+
5169
#[inline]
5270
pub(crate) fn from_u8(v: u8) -> Option<Self> {
5371
match v {
@@ -58,6 +76,9 @@ impl SuggestionProvider {
5876
5 => Some(SuggestionProvider::Yelp),
5977
6 => Some(SuggestionProvider::Mdn),
6078
7 => Some(SuggestionProvider::Weather),
79+
8 => Some(SuggestionProvider::AmpMobile),
80+
#[cfg(feature = "fakespot")]
81+
9 => Some(SuggestionProvider::Fakespot),
6182
_ => None,
6283
}
6384
}
@@ -105,6 +126,10 @@ impl SuggestionProvider {
105126
SuggestRecordType::GlobalConfig,
106127
]
107128
}
129+
#[cfg(feature = "fakespot")]
130+
SuggestionProvider::Fakespot => {
131+
vec![SuggestRecordType::Fakespot]
132+
}
108133
}
109134
}
110135
}

components/suggest/src/query.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,15 @@ impl SuggestionQuery {
9595
}
9696
}
9797

98+
#[cfg(feature = "fakespot")]
99+
pub fn fakespot(keyword: &str) -> Self {
100+
Self {
101+
keyword: keyword.into(),
102+
providers: vec![SuggestionProvider::Fakespot],
103+
limit: None,
104+
}
105+
}
106+
98107
pub fn weather(keyword: &str) -> Self {
99108
Self {
100109
keyword: keyword.into(),

0 commit comments

Comments
 (0)