Suggest: shorten the type names from the rs module.

Nan suggested this in the review of #6236 and I think it's a great idea.

Author: bendk

components/suggest/src/benchmarks/client.rs

@@ -2,13 +2,7 @@
  * 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::{
-    rs::{
-        SuggestRemoteSettingsClient, SuggestRemoteSettingsRecord,
-        SuggestRemoteSettingsRecordRequest,
-    },
-    Result,
-};
+use crate::{rs, Result};
 use parking_lot::Mutex;
 use remote_settings::{Client, RemoteSettingsConfig};
 use std::collections::HashMap;
@@ -17,11 +11,10 @@ use std::collections::HashMap;
 ///
 /// This should be used to run a full ingestion.
 /// Then it can be converted into a [RemoteSettingsBenchmarkClient], which allows benchmark code to exclude the network request time.
-/// [RemoteSettingsBenchmarkClient] implements [SuggestRemoteSettingsClient] by getting data from a HashMap rather than hitting the network.
+/// [RemoteSettingsBenchmarkClient] implements [rs::Client] by getting data from a HashMap rather than hitting the network.
 pub struct RemoteSettingsWarmUpClient {
     client: Client,
-    pub get_records_responses:
-        Mutex<HashMap<SuggestRemoteSettingsRecordRequest, Vec<SuggestRemoteSettingsRecord>>>,
+    pub get_records_responses: Mutex<HashMap<rs::RecordRequest, Vec<rs::Record>>>,
 }
 
 impl RemoteSettingsWarmUpClient {
@@ -45,13 +38,9 @@ impl Default for RemoteSettingsWarmUpClient {
     }
 }
 
-impl SuggestRemoteSettingsClient for RemoteSettingsWarmUpClient {
-    fn get_records(
-        &self,
-        request: SuggestRemoteSettingsRecordRequest,
-    ) -> Result<Vec<SuggestRemoteSettingsRecord>> {
-        let response =
-            <Client as SuggestRemoteSettingsClient>::get_records(&self.client, request.clone())?;
+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())?;
         self.get_records_responses
             .lock()
             .insert(request, response.clone());
@@ -61,15 +50,11 @@ impl SuggestRemoteSettingsClient for RemoteSettingsWarmUpClient {
 
 #[derive(Clone)]
 pub struct RemoteSettingsBenchmarkClient {
-    pub get_records_responses:
-        HashMap<SuggestRemoteSettingsRecordRequest, Vec<SuggestRemoteSettingsRecord>>,
+    pub get_records_responses: HashMap<rs::RecordRequest, Vec<rs::Record>>,
 }
 
-impl SuggestRemoteSettingsClient for RemoteSettingsBenchmarkClient {
-    fn get_records(
-        &self,
-        request: SuggestRemoteSettingsRecordRequest,
-    ) -> Result<Vec<SuggestRemoteSettingsRecord>> {
+impl rs::Client for RemoteSettingsBenchmarkClient {
+    fn get_records(&self, request: rs::RecordRequest) -> Result<Vec<rs::Record>> {
         Ok(self
             .get_records_responses
             .get(&request)

components/suggest/src/db.rs

@@ -22,7 +22,7 @@ use crate::{
     rs::{
         DownloadedAmoSuggestion, DownloadedAmpSuggestion, DownloadedAmpWikipediaSuggestion,
         DownloadedMdnSuggestion, DownloadedPocketSuggestion, DownloadedWeatherData,
-        DownloadedWikipediaSuggestion, SuggestRecordId, SuggestRemoteSettingsRecord,
+        DownloadedWikipediaSuggestion, Record, SuggestRecordId,
     },
     schema::{clear_database, SuggestConnectionInitializer},
     suggestion::{cook_raw_suggestion_url, AmpSuggestionType, Suggestion},
@@ -134,21 +134,13 @@ impl<'a> SuggestDao<'a> {
     //
     //  These methods combine several low-level calls into one logical operation.
 
-    pub fn handle_ingested_record(
-        &mut self,
-        last_ingest_key: &str,
-        record: &SuggestRemoteSettingsRecord,
-    ) -> Result<()> {
+    pub fn handle_ingested_record(&mut self, last_ingest_key: &str, record: &Record) -> Result<()> {
         // Advance the last fetch time, so that we can resume
         // fetching after this record if we're interrupted.
         self.put_last_ingest_if_newer(last_ingest_key, record.last_modified)
     }
 
-    pub fn handle_deleted_record(
-        &mut self,
-        last_ingest_key: &str,
-        record: &SuggestRemoteSettingsRecord,
-    ) -> Result<()> {
+    pub fn handle_deleted_record(&mut self, last_ingest_key: &str, record: &Record) -> Result<()> {
         let record_id = SuggestRecordId::from(&record.id);
         // Drop either the icon or suggestions, records only contain one or the other
         match record_id.as_icon_id() {

components/suggest/src/rs.rs

@@ -64,19 +64,13 @@ pub(crate) const DEFAULT_RECORDS_TYPES: [SuggestRecordType; 9] = [
 /// A trait for a client that downloads suggestions from Remote Settings.
 ///
 /// This trait lets tests use a mock client.
-pub(crate) trait SuggestRemoteSettingsClient {
+pub(crate) trait Client {
     /// Fetch a list of records and attachment data
-    fn get_records(
-        &self,
-        request: SuggestRemoteSettingsRecordRequest,
-    ) -> Result<Vec<SuggestRemoteSettingsRecord>>;
+    fn get_records(&self, request: RecordRequest) -> Result<Vec<Record>>;
 }
 
-impl SuggestRemoteSettingsClient for remote_settings::Client {
-    fn get_records(
-        &self,
-        request: SuggestRemoteSettingsRecordRequest,
-    ) -> Result<Vec<SuggestRemoteSettingsRecord>> {
+impl Client for remote_settings::Client {
+    fn get_records(&self, request: RecordRequest) -> Result<Vec<Record>> {
         let options = request.into();
         self.get_records_with_options(&options)?
             .records
@@ -87,21 +81,21 @@ impl SuggestRemoteSettingsClient for remote_settings::Client {
                     .as_ref()
                     .map(|a| self.get_attachment(&a.location))
                     .transpose()?;
-                Ok(SuggestRemoteSettingsRecord::new(record, attachment_data))
+                Ok(Record::new(record, attachment_data))
             })
             .collect()
     }
 }
 
 #[derive(Clone, Debug, Default, PartialEq, Eq, Hash)]
-pub struct SuggestRemoteSettingsRecordRequest {
+pub struct RecordRequest {
     pub record_type: Option<String>,
     pub last_modified: Option<u64>,
     pub limit: Option<u64>,
 }
 
-impl From<SuggestRemoteSettingsRecordRequest> for GetItemsOptions {
-    fn from(value: SuggestRemoteSettingsRecordRequest) -> Self {
+impl From<RecordRequest> for GetItemsOptions {
+    fn from(value: RecordRequest) -> Self {
         let mut options = GetItemsOptions::new();
 
         // Remote Settings returns records in descending modification order
@@ -130,7 +124,7 @@ impl From<SuggestRemoteSettingsRecordRequest> for GetItemsOptions {
 ///
 /// This is `remote_settings::RemoteSettingsRecord`, plus the downloaded attachment data.
 #[derive(Clone, Debug, Default)]
-pub struct SuggestRemoteSettingsRecord {
+pub struct Record {
     pub id: String,
     pub last_modified: u64,
     pub deleted: bool,
@@ -139,7 +133,7 @@ pub struct SuggestRemoteSettingsRecord {
     pub attachment_data: Option<Vec<u8>>,
 }
 
-impl SuggestRemoteSettingsRecord {
+impl Record {
     pub fn new(record: RemoteSettingsRecord, attachment_data: Option<Vec<u8>>) -> Self {
         Self {
             id: record.id,
@@ -634,9 +628,9 @@ mod test {
     #[test]
     fn test_remote_settings_limits() {
         fn check_limit(suggestion_limit: Option<u64>, expected_record_limit: Option<&str>) {
-            let request = SuggestRemoteSettingsRecordRequest {
+            let request = RecordRequest {
                 limit: suggestion_limit,
-                ..SuggestRemoteSettingsRecordRequest::default()
+                ..RecordRequest::default()
             };
             let options: GetItemsOptions = request.into();
             let actual_record_limit = options

components/suggest/src/store.rs

@@ -22,9 +22,8 @@ use crate::{
     error::Error,
     provider::SuggestionProvider,
     rs::{
-        SuggestAttachment, SuggestRecord, SuggestRecordId, SuggestRecordType,
-        SuggestRemoteSettingsClient, SuggestRemoteSettingsRecord,
-        SuggestRemoteSettingsRecordRequest, DEFAULT_RECORDS_TYPES, REMOTE_SETTINGS_COLLECTION,
+        Client, Record, RecordRequest, SuggestAttachment, SuggestRecord, SuggestRecordId,
+        SuggestRecordType, DEFAULT_RECORDS_TYPES, REMOTE_SETTINGS_COLLECTION,
     },
     Result, SuggestApiResult, Suggestion, SuggestionQuery,
 };
@@ -311,7 +310,7 @@ impl<S> SuggestStoreInner<S> {
 
 impl<S> SuggestStoreInner<S>
 where
-    S: SuggestRemoteSettingsClient,
+    S: Client,
 {
     pub fn ingest(&self, constraints: SuggestIngestionConstraints) -> Result<()> {
         let writer = &self.dbs()?.writer;
@@ -344,7 +343,7 @@ where
         writer: &SuggestDb,
         constraints: &SuggestIngestionConstraints,
     ) -> Result<()> {
-        let request = SuggestRemoteSettingsRecordRequest {
+        let request = RecordRequest {
             record_type: Some(ingest_record_type.to_string()),
             last_modified: writer.read(|dao| {
                 dao.get_meta::<u64>(ingest_record_type.last_ingest_meta_key().as_str())
@@ -361,7 +360,7 @@ where
         &self,
         last_ingest_key: &str,
         writer: &SuggestDb,
-        records: &[SuggestRemoteSettingsRecord],
+        records: &[Record],
     ) -> Result<()> {
         for record in records {
             let record_id = SuggestRecordId::from(&record.id);
@@ -493,7 +492,7 @@ where
         &self,
         last_ingest_key: &str,
         writer: &SuggestDb,
-        record: &SuggestRemoteSettingsRecord,
+        record: &Record,
         ingestion_handler: impl FnOnce(&mut SuggestDao<'_>, &SuggestRecordId) -> Result<()>,
     ) -> Result<()> {
         let record_id = SuggestRecordId::from(&record.id);
@@ -516,7 +515,7 @@ where
         &self,
         last_ingest_key: &str,
         writer: &SuggestDb,
-        record: &SuggestRemoteSettingsRecord,
+        record: &Record,
         ingestion_handler: impl FnOnce(&mut SuggestDao<'_>, &SuggestRecordId, &[T]) -> Result<()>,
     ) -> Result<()>
     where
@@ -549,7 +548,7 @@ where
 #[cfg(feature = "benchmark_api")]
 impl<S> SuggestStoreInner<S>
 where
-    S: SuggestRemoteSettingsClient,
+    S: Client,
 {
     pub fn into_settings_client(self) -> S {
         self.settings_client
@@ -681,7 +680,7 @@ mod tests {
     /// Creates a unique in-memory Suggest store.
     fn unique_test_store<S>(settings_client: S) -> SuggestStoreInner<S>
     where
-        S: SuggestRemoteSettingsClient,
+        S: Client,
     {
         let mut unique_suffix = [0u8; 8];
         rand::fill(&mut unique_suffix).expect("Failed to generate unique suffix for test store");
@@ -758,11 +757,8 @@ mod tests {
         }
     }
 
-    impl SuggestRemoteSettingsClient for SnapshotSettingsClient {
-        fn get_records(
-            &self,
-            _request: SuggestRemoteSettingsRecordRequest,
-        ) -> Result<Vec<SuggestRemoteSettingsRecord>> {
+    impl Client for SnapshotSettingsClient {
+        fn get_records(&self, _request: RecordRequest) -> Result<Vec<Record>> {
             let snapshot = self.snapshot.borrow();
             snapshot
                 .records
@@ -780,7 +776,7 @@ mod tests {
                         .transpose()?
                         .cloned();
 
-                    Ok(SuggestRemoteSettingsRecord::new(r.clone(), attachment))
+                    Ok(Record::new(r.clone(), attachment))
                 })
                 .collect()
         }

components/suggest/src/testing/client.rs

@@ -9,10 +9,7 @@ use serde_json::json;
 use serde_json::Value as JsonValue;
 
 use crate::{
-    rs::{
-        SuggestRemoteSettingsClient, SuggestRemoteSettingsRecord,
-        SuggestRemoteSettingsRecordRequest,
-    },
+    rs::{Client, Record, RecordRequest},
     testing::JsonExt,
     Result,
 };
@@ -22,7 +19,7 @@ use crate::{
 /// MockRemoteSettingsClient uses the builder pattern for its API: most methods input `self` and
 /// return a modified version of it.
 pub struct MockRemoteSettingsClient {
-    pub records: HashMap<String, Vec<SuggestRemoteSettingsRecord>>,
+    pub records: HashMap<String, Vec<Record>>,
     pub last_modified_timestamp: u64,
 }
 
@@ -43,7 +40,7 @@ impl MockRemoteSettingsClient {
     pub fn with_record(mut self, record_type: &str, record_id: &str, items: JsonValue) -> Self {
         let location = format!("{record_type}-{record_id}.json");
         let records = self.records.entry(record_type.to_string()).or_default();
-        records.push(SuggestRemoteSettingsRecord {
+        records.push(Record {
             id: record_id.to_string(),
             last_modified: self.last_modified_timestamp,
             deleted: false,
@@ -67,7 +64,7 @@ impl MockRemoteSettingsClient {
     /// This is used by remote settings to indicated a deleted record
     pub fn with_tombstone(mut self, record_type: &str, record_id: &str) -> Self {
         let records = self.records.entry(record_type.to_string()).or_default();
-        records.push(SuggestRemoteSettingsRecord {
+        records.push(Record {
             id: record_id.to_string(),
             last_modified: self.last_modified_timestamp,
             deleted: true,
@@ -84,7 +81,7 @@ impl MockRemoteSettingsClient {
         let record_id = format!("icon-{icon_id}");
         let location = format!("icon-{icon_id}.png");
         let records = self.records.entry("icon".to_string()).or_default();
-        records.push(SuggestRemoteSettingsRecord {
+        records.push(Record {
             id: record_id.to_string(),
             last_modified: self.last_modified_timestamp,
             deleted: false,
@@ -113,11 +110,8 @@ pub struct MockIcon {
     pub mimetype: &'static str,
 }
 
-impl SuggestRemoteSettingsClient for MockRemoteSettingsClient {
-    fn get_records(
-        &self,
-        request: SuggestRemoteSettingsRecordRequest,
-    ) -> Result<Vec<SuggestRemoteSettingsRecord>> {
+impl Client for MockRemoteSettingsClient {
+    fn get_records(&self, request: RecordRequest) -> Result<Vec<Record>> {
         let record_type = request.record_type.unwrap_or_else(|| {
             panic!("MockRemoteSettingsClient.get_records: record_type required ")
         });