mozilla
/
application-services
зеркало из
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

suggest: Prepare to support other types of suggestions. 

This commit:

* Renames some of our crate-internal types and methods to clarify that
  they're specific to AMP-Wikipedia suggestions.
* Generalizes `DownloadedSuggestDataAttachment` to a `SuggestAttachment`
  type that can accommodate any suggestion.
* Removes `icon_id` from the common `suggestions` table, because not
  all suggestions that we'll support in the future have icon data. For
  example, Pocket suggestions don't have icons at all, and AMO
  suggestions have URLs instead of binary data.
This commit is contained in:
Lina Butler 2023-08-02 08:43:27 -07:00
Родитель a5a8cf90b7
Коммит 4981e78a9c
5 изменённых файлов: 126 добавлений и 95 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

2
CHANGELOG.md
Просмотреть файл

@ -5,7 +5,7 @@
## General
### 🦊 What's Changed 🦊
- Changes to Suggestion schema to accomodate custom details for providers. ([#5745](https://github.com/mozilla/application-services/pull/5745))
- Backward-incompatible changes to the Suggest database schema to accommodate custom details for providers ([#5745](https://github.com/mozilla/application-services/pull/5745)) and future suggestion types ([#5766](https://github.com/mozilla/application-services/pull/5766)). This only affects prototyping, because we aren't consuming Suggest in any of our products yet.
- The Remote Settings `Client::get_attachment()` method now returns a `Vec<u8>` instead of a Viaduct `Response` ([#5764](https://github.com/mozilla/application-services/pull/5764)). You can use the new `Client::get_attachment_raw()` method if you need the `Response`. This is a backward-incompatible change for Rust consumers only; Swift and Kotlin are unaffected.
- The Remote Settings client now parses `ETag` response headers from Remote Settings correctly ([#5764](https://github.com/mozilla/application-services/pull/5764)).

80
components/suggest/src/db.rs
Просмотреть файл

@ -16,7 +16,7 @@ use sql_support::{open_database::open_database_with_flags, ConnExt};
use crate::{
keyword::full_keyword,
rs::{DownloadedSuggestion, SuggestRecordId, SuggestionProvider},
rs::{DownloadedAmpWikipediaSuggestion, SuggestRecordId, SuggestionProvider},
schema::SuggestConnectionInitializer,
Result, Suggestion,
};
@ -122,12 +122,11 @@ impl<'a> SuggestDao<'a> {
/// Fetches suggestions that match the given keyword from the database.
pub fn fetch_by_keyword(&self, keyword: &str) -> Result<Vec<Suggestion>> {
self.conn.query_rows_and_then_cached(
"SELECT s.id, k.rank, s.title, s.url, s.provider,
(SELECT i.data FROM icons i WHERE i.id = s.icon_id) AS icon
FROM suggestions s
JOIN keywords k ON k.suggestion_id = s.id
WHERE k.keyword = :keyword
LIMIT 1",
"SELECT s.id, k.rank, s.title, s.url, s.provider
FROM suggestions s
JOIN keywords k ON k.suggestion_id = s.id
WHERE k.keyword = :keyword
LIMIT 1",
named_params! {
":keyword": keyword,
},
@ -135,7 +134,6 @@ impl<'a> SuggestDao<'a> {
let suggestion_id: i64 = row.get("id")?;
let title = row.get("title")?;
let url = row.get("url")?;
let icon = row.get("icon")?;
let provider = row.get("provider")?;
let keywords: Vec<String> = self.conn.query_rows_and_then_cached(
@ -152,12 +150,14 @@ impl<'a> SuggestDao<'a> {
match provider {
SuggestionProvider::Amp => {
self.conn.query_row_and_then(
"SELECT amp.advertiser, amp.block_id, amp.iab_category, amp.impression_url, amp.click_url
FROM amp_custom_details amp WHERE amp.suggestion_id = :suggestion_id",
"SELECT amp.advertiser, amp.block_id, amp.iab_category, amp.impression_url, amp.click_url,
(SELECT i.data FROM icons i WHERE i.id = amp.icon_id) AS icon
FROM amp_custom_details amp
WHERE amp.suggestion_id = :suggestion_id",
named_params! {
":suggestion_id": suggestion_id
},
|row|{
|row| {
let iab_category = row.get::<_, String>("iab_category")?;
let is_sponsored = !NONSPONSORED_IAB_CATEGORIES.contains(&iab_category.as_str());
Ok(Suggestion {
@ -168,7 +168,7 @@ impl<'a> SuggestDao<'a> {
title,
url,
full_keyword: full_keyword(keyword, &keywords),
icon,
icon: row.get("icon")?,
impression_url: row.get("impression_url")?,
click_url: row.get("click_url")?,
provider
@ -176,7 +176,17 @@ impl<'a> SuggestDao<'a> {
}
)
},
SuggestionProvider::Wikipedia =>{
SuggestionProvider::Wikipedia => {
let icon = self.conn.try_query_one(
"SELECT i.data
FROM icons i
JOIN wikipedia_custom_details s ON s.icon_id = i.id
WHERE s.suggestion_id = :suggestion_id",
named_params! {
":suggestion_id": suggestion_id
},
true,
)?;
Ok(Suggestion {
block_id: 0,
advertiser: "Wikipedia".to_string(),
@ -192,18 +202,16 @@ impl<'a> SuggestDao<'a> {
})
}
}
},
)
}
/// Inserts all suggestions associated with a Remote Settings record into
/// Inserts all suggestions from a downloaded AMP-Wikipedia attachment into
/// the database.
pub fn insert_suggestions(
pub fn insert_amp_wikipedia_suggestions(
&mut self,
record_id: &SuggestRecordId,
suggestions: &[DownloadedSuggestion],
suggestions: &[DownloadedAmpWikipediaSuggestion],
) -> Result<()> {
for suggestion in suggestions {
self.scope.err_if_interrupted()?;
@ -214,15 +222,13 @@ impl<'a> SuggestDao<'a> {
record_id,
provider,
title,
url,
icon_id
url
)
VALUES(
:record_id,
:provider,
:title,
:url,
:icon_id
:url
)
RETURNING id
",
@ -231,14 +237,13 @@ impl<'a> SuggestDao<'a> {
":provider": &provider,
":title": common_details.title,
":url": common_details.url,
":icon_id": common_details.icon_id
},
|row| row.get(0),
true,
)?;
match suggestion {
DownloadedSuggestion::Amp(amp) => {
DownloadedAmpWikipediaSuggestion::Amp(amp) => {
self.conn.execute(
"INSERT INTO amp_custom_details(
suggestion_id,
@ -246,7 +251,8 @@ impl<'a> SuggestDao<'a> {
block_id,
iab_category,
impression_url,
click_url
click_url,
icon_id
)
VALUES(
:suggestion_id,
@ -254,7 +260,8 @@ impl<'a> SuggestDao<'a> {
:block_id,
:iab_category,
:impression_url,
:click_url
:click_url,
:icon_id
)",
named_params! {
":suggestion_id": suggestion_id,
@ -262,12 +269,27 @@ impl<'a> SuggestDao<'a> {
":block_id": amp.block_id,
":iab_category": amp.iab_category,
":impression_url": amp.impression_url,
":click_url": amp.click_url
":click_url": amp.click_url,
":icon_id": amp.icon_id,
},
)?;
}
DownloadedAmpWikipediaSuggestion::Wikipedia(wikipedia) => {
self.conn.execute(
"INSERT INTO wikipedia_custom_details(
suggestion_id,
icon_id
)
VALUES(
:suggestion_id,
:icon_id
)",
named_params! {
":suggestion_id": suggestion_id,
":icon_id": wikipedia.icon_id,
},
)?;
}
DownloadedSuggestion::Wikipedia(_) => {}
}
for (index, keyword) in common_details.keywords.iter().enumerate() {
self.conn.execute(

106
components/suggest/src/rs.rs
Просмотреть файл

@ -1,4 +1,4 @@
use std::{borrow::Cow, ops::Deref};
use std::borrow::Cow;
use remote_settings::{GetItemsOptions, RemoteSettingsResponse};
use rusqlite::{
@ -56,34 +56,33 @@ pub(crate) enum SuggestRecord {
#[serde(rename = "icon")]
Icon,
#[serde(rename = "data")]
Data,
AmpWikipedia,
}
/// Represents either a single value, or a list of values. This is used to
/// deserialize downloaded data attachments.
/// deserialize downloaded attachments.
#[derive(Clone, Debug, Deserialize)]
#[serde(untagged)]
pub(crate) enum OneOrMany<T> {
enum OneOrMany<T> {
One(T),
Many(Vec<T>),
}
impl<T> Deref for OneOrMany<T> {
type Target = [T];
/// A downloaded Remote Settings attachment that contains suggestions.
#[derive(Clone, Debug, Deserialize)]
#[serde(transparent)]
pub(crate) struct SuggestAttachment<T>(OneOrMany<T>);
fn deref(&self) -> &Self::Target {
match self {
impl<T> SuggestAttachment<T> {
/// Returns a slice of suggestions to ingest from the downloaded attachment.
pub fn suggestions(&self) -> &[T] {
match &self.0 {
OneOrMany::One(value) => std::slice::from_ref(value),
OneOrMany::Many(values) => values,
}
}
}
/// The contents of a downloaded [`TypedSuggestRecord::Data`] attachment.
#[derive(Clone, Debug, Deserialize)]
#[serde(transparent)]
pub(crate) struct DownloadedSuggestDataAttachment(pub OneOrMany<DownloadedSuggestion>);
/// The ID of a record in the Suggest Remote Settings collection.
#[derive(Clone, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd)]
#[serde(transparent)]
@ -113,15 +112,15 @@ where
}
}
/// Fields that are common to all downloaded suggestions.
#[derive(Clone, Debug, Deserialize, Eq, PartialEq)]
pub(crate) struct DownloadedSuggestionCommonDetails {
pub keywords: Vec<String>,
pub title: String,
pub url: String,
#[serde(rename = "icon")]
pub icon_id: String,
}
/// An AMP suggestion to ingest from an AMP-Wikipedia attachment.
#[derive(Clone, Debug, Deserialize, Eq, PartialEq)]
pub(crate) struct DownloadedAmpSuggestion {
#[serde(flatten)]
@ -132,6 +131,17 @@ pub(crate) struct DownloadedAmpSuggestion {
pub iab_category: String,
pub click_url: String,
pub impression_url: String,
#[serde(rename = "icon")]
pub icon_id: String,
}
/// A Wikipedia suggestion to ingest from an AMP-Wikipedia attachment.
#[derive(Clone, Debug, Deserialize, Eq, PartialEq)]
pub(crate) struct DownloadedWikipediaSuggestion {
#[serde(flatten)]
pub common_details: DownloadedSuggestionCommonDetails,
#[serde(rename = "icon")]
pub icon_id: String,
}
/// Provider Types
@ -169,72 +179,66 @@ impl ToSql for SuggestionProvider {
}
}
/// A suggestion to ingest from a downloaded Remote Settings attachment.
/// A suggestion to ingest from an AMP-Wikipedia attachment downloaded from
/// Remote Settings.
#[derive(Clone, Debug, Eq, PartialEq)]
pub(crate) enum DownloadedSuggestion {
pub(crate) enum DownloadedAmpWikipediaSuggestion {
Amp(DownloadedAmpSuggestion),
Wikipedia(DownloadedSuggestionCommonDetails),
Wikipedia(DownloadedWikipediaSuggestion),
}
impl DownloadedSuggestion {
/// Returns the suggestion fields that are common to AMP and
/// Wikipedia suggestions.
impl DownloadedAmpWikipediaSuggestion {
/// Returns the details that are common to AMP and Wikipedia suggestions.
pub fn common_details(&self) -> &DownloadedSuggestionCommonDetails {
match self {
Self::Amp(DownloadedAmpSuggestion { common_details, .. }) => common_details,
Self::Wikipedia(common_details) => common_details,
Self::Wikipedia(DownloadedWikipediaSuggestion { common_details, .. }) => common_details,
}
}
/// Returns the provider of this suggestion.
pub fn provider(&self) -> SuggestionProvider {
match self {
DownloadedSuggestion::Amp(_) => SuggestionProvider::Amp,
DownloadedSuggestion::Wikipedia(_) => SuggestionProvider::Wikipedia,
DownloadedAmpWikipediaSuggestion::Amp(_) => SuggestionProvider::Amp,
DownloadedAmpWikipediaSuggestion::Wikipedia(_) => SuggestionProvider::Wikipedia,
}
}
}
impl<'de> Deserialize<'de> for DownloadedSuggestion {
fn deserialize<D>(deserializer: D) -> std::result::Result<DownloadedSuggestion, D::Error>
impl<'de> Deserialize<'de> for DownloadedAmpWikipediaSuggestion {
fn deserialize<D>(
deserializer: D,
) -> std::result::Result<DownloadedAmpWikipediaSuggestion, D::Error>
where
D: Deserializer<'de>,
{
// AMP and Wikipedia suggestions conform to the same JSON schema, but
// we want to represent them separately. To distinguish between the two,
// we use an "untagged" outer enum and a "tagged" inner enum.
// AMP and Wikipedia suggestions use the same schema. To separate them,
// we use a "maybe tagged" outer enum with tagged and untagged variants,
// and a "tagged" inner enum.
//
// Wikipedia suggestions always use the `"Wikipedia"` advertiser, so
// they'll deserialize successfully into the `KnownTag` variant.
// AMP suggestions will try the `KnownTag` variant first, fail, then
// try the `UnknownTag` variant and succeed.
// Wikipedia suggestions will deserialize successfully into the tagged
// variant. AMP suggestions will try the tagged variant, fail, and fall
// back to the untagged variant.
//
// This strategy is an implementation detail, so we turn the nested
// enums into a friendlier `DownloadedAmpSuggestion` enum after
// deserializing.
// This approach works around serde-rs/serde#912.
#[derive(Deserialize)]
#[serde(untagged)]
enum UntaggedDownloadedSuggestion {
KnownTag(TaggedDownloadedSuggestion),
UnknownTag(DownloadedAmpSuggestion),
enum MaybeTagged {
Tagged(Tagged),
Untagged(DownloadedAmpSuggestion),
}
#[derive(Deserialize)]
#[serde(tag = "advertiser")]
enum TaggedDownloadedSuggestion {
enum Tagged {
#[serde(rename = "Wikipedia")]
Wikipedia(DownloadedSuggestionCommonDetails),
Wikipedia(DownloadedWikipediaSuggestion),
}
Ok(
match UntaggedDownloadedSuggestion::deserialize(deserializer)? {
UntaggedDownloadedSuggestion::KnownTag(TaggedDownloadedSuggestion::Wikipedia(
common_details,
)) => Self::Wikipedia(common_details),
UntaggedDownloadedSuggestion::UnknownTag(common_details) => {
Self::Amp(common_details)
}
},
)
Ok(match MaybeTagged::deserialize(deserializer)? {
MaybeTagged::Tagged(Tagged::Wikipedia(wikipedia)) => Self::Wikipedia(wikipedia),
MaybeTagged::Untagged(amp) => Self::Amp(amp),
})
}
}

11
components/suggest/src/schema.rs
Просмотреть файл

@ -6,7 +6,7 @@
use rusqlite::{Connection, Transaction};
use sql_support::open_database::{self, ConnectionInitializer};
pub const VERSION: u32 = 2;
pub const VERSION: u32 = 3;
pub const SQL: &str = "
CREATE TABLE meta(
@ -28,8 +28,7 @@ pub const SQL: &str = "
record_id TEXT NOT NULL,
provider INTEGER NOT NULL,
title TEXT NOT NULL,
url TEXT NOT NULL,
icon_id TEXT NOT NULL
url TEXT NOT NULL
);
CREATE TABLE amp_custom_details(
@ -39,10 +38,16 @@ pub const SQL: &str = "
iab_category TEXT NOT NULL,
impression_url TEXT NOT NULL,
click_url TEXT NOT NULL,
icon_id TEXT NOT NULL,
FOREIGN KEY(suggestion_id) REFERENCES suggestions(id)
ON DELETE CASCADE
);
CREATE TABLE wikipedia_custom_details(
suggestion_id INTEGER PRIMARY KEY REFERENCES suggestions(id) ON DELETE CASCADE,
icon_id TEXT NOT NULL
);
CREATE INDEX suggestions_record_id ON suggestions(record_id);
CREATE TABLE icons(

22
components/suggest/src/store.rs
Просмотреть файл

@ -11,8 +11,8 @@ use remote_settings::{self, GetItemsOptions, RemoteSettingsConfig, SortOrder};
use crate::{
db::{ConnectionType, SuggestDb, LAST_INGEST_META_KEY},
rs::{
DownloadedSuggestDataAttachment, SuggestRecord, SuggestRecordId,
SuggestRemoteSettingsClient, REMOTE_SETTINGS_COLLECTION, SUGGESTIONS_PER_ATTACHMENT,
SuggestAttachment, SuggestRecord, SuggestRecordId, SuggestRemoteSettingsClient,
REMOTE_SETTINGS_COLLECTION, SUGGESTIONS_PER_ATTACHMENT,
},
Result, SuggestApiResult, Suggestion, SuggestionQuery,
};
@ -209,19 +209,18 @@ where
continue;
};
match fields {
SuggestRecord::Data => {
SuggestRecord::AmpWikipedia => {
let Some(attachment) = record.attachment.as_ref() else {
// A data record should always have an attachment with
// suggestions. If it doesn't, it's malformed, so skip
// to the next record.
// An AMP-Wikipedia record should always have an
// attachment with suggestions. If it doesn't, it's
// malformed, so skip to the next record.
writer.write(|dao| dao.put_meta(LAST_INGEST_META_KEY, record.last_modified))?;
continue;
};
let suggestions = serde_json::from_slice::<DownloadedSuggestDataAttachment>(
let attachment: SuggestAttachment<_> = serde_json::from_slice(
&self.settings_client.get_attachment(&attachment.location)?,
)?
.0;
)?;
writer.write(|dao| {
// Drop any suggestions that we previously ingested from
@ -231,8 +230,9 @@ where
// dropping and re-ingesting all of them.
dao.drop_suggestions(&record_id)?;
// Ingest (or re-ingest) all suggestions in the attachment.
dao.insert_suggestions(&record_id, &suggestions)?;
// Ingest (or re-ingest) all suggestions in the
// attachment.
dao.insert_amp_wikipedia_suggestions(&record_id, attachment.suggestions())?;
// Advance the last fetch time, so that we can resume
// fetching after this record if we're interrupted.