diff --git a/CHANGELOG.md b/CHANGELOG.md index ad21cdef70..fd0aa69ce2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # v130.0 (In progress) +### Suggest +- Added support for Fakespot suggestions. + [Full Changelog](In progress) # v129.0 (_2024-07-08_) diff --git a/components/suggest/src/db.rs b/components/suggest/src/db.rs index 8136b8c48c..b5b02291f3 100644 --- a/components/suggest/src/db.rs +++ b/components/suggest/src/db.rs @@ -690,7 +690,9 @@ impl<'a> SuggestDao<'a> { f.fakespot_grade, f.product_id, f.rating, - f.total_reviews + f.total_reviews, + i.data, + i.mimetype FROM suggestions s JOIN @@ -699,6 +701,9 @@ impl<'a> SuggestDao<'a> { JOIN fakespot_custom_details f ON f.suggestion_id = s.id + LEFT JOIN + icons i + ON i.id = f.icon_id WHERE fakespot_fts MATCH ? ORDER BY @@ -714,6 +719,8 @@ impl<'a> SuggestDao<'a> { product_id: row.get(4)?, rating: row.get(5)?, total_reviews: row.get(6)?, + icon: row.get(7)?, + icon_mimetype: row.get(8)?, }) }, ) @@ -1385,9 +1392,10 @@ impl<'conn> FakespotInsertStatement<'conn> { fakespot_grade, product_id, rating, - total_reviews + total_reviews, + icon_id ) - VALUES(?, ?, ?, ?, ?) + VALUES(?, ?, ?, ?, ?, ?) ", )?)) } @@ -1397,6 +1405,10 @@ impl<'conn> FakespotInsertStatement<'conn> { suggestion_id: i64, fakespot: &DownloadedFakespotSuggestion, ) -> Result<()> { + let icon_id = fakespot + .product_id + .split_once('-') + .map(|(vendor, _)| format!("fakespot-{vendor}")); self.0 .execute(( suggestion_id, @@ -1404,6 +1416,7 @@ impl<'conn> FakespotInsertStatement<'conn> { &fakespot.product_id, fakespot.rating, fakespot.total_reviews, + icon_id, )) .with_context("fakespot insert")?; Ok(()) diff --git a/components/suggest/src/schema.rs b/components/suggest/src/schema.rs index 919e50829f..dbae7e741d 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 = 21; +pub const VERSION: u32 = 22; /// The current Suggest database schema. pub const SQL: &str = " @@ -95,6 +95,7 @@ CREATE TABLE fakespot_custom_details( product_id TEXT NOT NULL, rating REAL NOT NULL, total_reviews INTEGER NOT NULL, + icon_id TEXT, FOREIGN KEY(suggestion_id) REFERENCES suggestions(id) ON DELETE CASCADE ); @@ -321,6 +322,30 @@ END; )?; Ok(()) } + 21 => { + // Drop and re-create the fakespot_custom_details to add the icon_id column. + tx.execute_batch( + " +DROP TABLE fakespot_custom_details; +CREATE TABLE fakespot_custom_details( + suggestion_id INTEGER PRIMARY KEY, + fakespot_grade TEXT NOT NULL, + product_id TEXT NOT NULL, + rating REAL NOT NULL, + total_reviews INTEGER NOT NULL, + icon_id TEXT, + FOREIGN KEY(suggestion_id) REFERENCES suggestions(id) ON DELETE CASCADE +); +CREATE TRIGGER fakespot_ai AFTER INSERT ON fakespot_custom_details BEGIN + INSERT INTO fakespot_fts(rowid, title) + SELECT id, title + FROM suggestions + WHERE id = new.suggestion_id; +END; + ", + )?; + Ok(()) + } _ => Err(open_database::Error::IncompatibleVersion(version)), } } diff --git a/components/suggest/src/store.rs b/components/suggest/src/store.rs index 3557839915..8ce84783ce 100644 --- a/components/suggest/src/store.rs +++ b/components/suggest/src/store.rs @@ -2129,11 +2129,15 @@ mod tests { fn query_fakespot() -> anyhow::Result<()> { before_each(); - let store = TestStore::new(MockRemoteSettingsClient::default().with_record( - "fakespot-suggestions", - "fakespot-1", - json!([snowglobe_fakespot(), simpsons_fakespot()]), - )); + let store = TestStore::new( + MockRemoteSettingsClient::default() + .with_record( + "fakespot-suggestions", + "fakespot-1", + json!([snowglobe_fakespot(), simpsons_fakespot()]), + ) + .with_icon(fakespot_amazon_icon()), + ); store.ingest(SuggestIngestionConstraints::all_providers()); assert_eq!( store.fetch_suggestions(SuggestionQuery::fakespot("globe")), @@ -2165,11 +2169,15 @@ mod tests { fn fakespot_prefix_matching() -> anyhow::Result<()> { before_each(); - let store = TestStore::new(MockRemoteSettingsClient::default().with_record( - "fakespot-suggestions", - "fakespot-1", - json!([snowglobe_fakespot(), simpsons_fakespot()]), - )); + let store = TestStore::new( + MockRemoteSettingsClient::default() + .with_record( + "fakespot-suggestions", + "fakespot-1", + json!([snowglobe_fakespot(), simpsons_fakespot()]), + ) + .with_icon(fakespot_amazon_icon()), + ); store.ingest(SuggestIngestionConstraints::all_providers()); assert_eq!( store.fetch_suggestions(SuggestionQuery::fakespot("simp")), @@ -2191,23 +2199,30 @@ mod tests { fn fakespot_updates_and_deletes() -> anyhow::Result<()> { before_each(); - let mut store = TestStore::new(MockRemoteSettingsClient::default().with_record( - "fakespot-suggestions", - "fakespot-1", - json!([snowglobe_fakespot(), simpsons_fakespot()]), - )); + let mut store = TestStore::new( + MockRemoteSettingsClient::default() + .with_record( + "fakespot-suggestions", + "fakespot-1", + json!([snowglobe_fakespot(), simpsons_fakespot()]), + ) + .with_icon(fakespot_amazon_icon()), + ); store.ingest(SuggestIngestionConstraints::all_providers()); // Update the snapshot so that: // - The Simpsons entry is deleted // - Snow globes now use sea glass instead of glitter - store.replace_client(MockRemoteSettingsClient::default().with_record( - "fakespot-suggestions", - "fakespot-1", - json!([ - snowglobe_fakespot().merge(json!({"title": "Make Your Own Sea Glass Snow Globes"})) - ]), - )); + store.replace_client( + MockRemoteSettingsClient::default() + .with_record( + "fakespot-suggestions", + "fakespot-1", + json!([snowglobe_fakespot() + .merge(json!({"title": "Make Your Own Sea Glass Snow Globes"}))]), + ) + .with_icon(fakespot_amazon_icon()), + ); store.ingest(SuggestIngestionConstraints::all_providers()); assert_eq!( diff --git a/components/suggest/src/suggest.udl b/components/suggest/src/suggest.udl index b28361abe6..f68e65104a 100644 --- a/components/suggest/src/suggest.udl +++ b/components/suggest/src/suggest.udl @@ -103,6 +103,8 @@ interface Suggestion { string title, i64 total_reviews, string url, + sequence? icon, + string? icon_mimetype, f64 score ); }; diff --git a/components/suggest/src/suggestion.rs b/components/suggest/src/suggestion.rs index b39ac9637f..1ead35f6ee 100644 --- a/components/suggest/src/suggestion.rs +++ b/components/suggest/src/suggestion.rs @@ -88,6 +88,8 @@ pub enum Suggestion { title: String, total_reviews: i64, url: String, + icon: Option>, + icon_mimetype: Option, score: f64, }, } @@ -127,7 +129,8 @@ impl Suggestion { | Self::Wikipedia { url, .. } | Self::Amo { url, .. } | Self::Yelp { url, .. } - | Self::Mdn { url, .. } => Some(url), + | Self::Mdn { url, .. } + | Self::Fakespot { url, .. } => Some(url), _ => None, } } @@ -160,6 +163,16 @@ impl Suggestion { Self::Fakespot { title, .. } => title, } } + + pub fn icon_data(&self) -> Option<&[u8]> { + match self { + Self::Amp { icon, .. } + | Self::Wikipedia { icon, .. } + | Self::Yelp { icon, .. } + | Self::Fakespot { icon, .. } => icon.as_deref(), + _ => None, + } + } } impl Eq for Suggestion {} diff --git a/components/suggest/src/testing/data.rs b/components/suggest/src/testing/data.rs index 007f893838..5bdc331886 100644 --- a/components/suggest/src/testing/data.rs +++ b/components/suggest/src/testing/data.rs @@ -473,29 +473,43 @@ pub fn snowglobe_suggestion() -> Suggestion { total_reviews: 152, url: "http://amazon.com/dp/ABC".into(), score: 0.7, + icon: Some("fakespot-icon-amazon-data".as_bytes().to_vec()), + icon_mimetype: Some("image/png".into()), } } pub fn simpsons_fakespot() -> JsonValue { json!({ "fakespot_grade": "A", - "product_id": "amazon-XYZ", + // Use a product ID that doesn't match the ingested icons to test what happens. In this + // case, icon and icon_mimetype for the returned Suggestion should both be None. + "product_id": "vendorwithouticon-XYZ", "rating": 4.9, "score": 0.9, "title": "The Simpsons: Skinner's Sense of Snow (DVD)", "total_reviews": 14000, - "url": "http://amazon.com/dp/XYZ" + "url": "http://vendorwithouticon.com/dp/XYZ" }) } pub fn simpsons_suggestion() -> Suggestion { Suggestion::Fakespot { fakespot_grade: "A".into(), - product_id: "amazon-XYZ".into(), + product_id: "vendorwithouticon-XYZ".into(), rating: 4.9, title: "The Simpsons: Skinner's Sense of Snow (DVD)".into(), total_reviews: 14000, - url: "http://amazon.com/dp/XYZ".into(), + url: "http://vendorwithouticon.com/dp/XYZ".into(), score: 0.9, + icon: None, + icon_mimetype: None, + } +} + +pub fn fakespot_amazon_icon() -> MockIcon { + MockIcon { + id: "fakespot-amazon", + data: "fakespot-icon-amazon-data", + mimetype: "image/png", } } diff --git a/examples/suggest-cli/src/main.rs b/examples/suggest-cli/src/main.rs index 7503772dfe..8ec609b8b9 100644 --- a/examples/suggest-cli/src/main.rs +++ b/examples/suggest-cli/src/main.rs @@ -107,7 +107,7 @@ fn build_store(cli: &Cli) -> Arc { fn ingest(store: &SuggestStore) { println!("Ingesting data..."); store - .ingest(SuggestIngestionConstraints::default()) + .ingest(SuggestIngestionConstraints::all_providers()) .unwrap_or_else(|e| panic!("Error in ingest: {e}")); println!("Done"); } @@ -127,11 +127,13 @@ fn query(store: &SuggestStore, provider: SuggestionProviderArg, input: String) { println!("Results:"); for suggestion in suggestions { let title = suggestion.title(); - if let Some(url) = suggestion.url() { - println!("{title} ({url})"); + let url = suggestion.url().unwrap_or("[no-url]"); + let icon = if suggestion.icon_data().is_some() { + "with icon" } else { - println!("{title}"); - } + "no icon" + }; + println!("{title} ({url}) ({icon})"); } } }