feat: store remote-settings attachments in local storage

components/remote_settings/src/client.rs

@@ -177,10 +177,27 @@ impl<C: ApiClient> RemoteSettingsClient<C> {
     /// Downloads an attachment from [attachment_location]. NOTE: there are no guarantees about a
     /// maximum size, so use care when fetching potentially large attachments.
     pub fn get_attachment(&self, attachment_location: &str) -> Result<Vec<u8>> {
-        self.inner
+        let mut inner = self.inner.lock();
+        let collection_url = inner.api_client.collection_url();
+
+        if let Some(attachment) = inner
+            .storage
+            .get_attachment(&collection_url, attachment_location)?
+        {
+            return Ok(attachment);
+        }
+
+        let attachment = self
+            .inner
             .lock()
             .api_client
-            .get_attachment(attachment_location)
+            .get_attachment(attachment_location)?;
+
+        inner
+            .storage
+            .set_attachment(&collection_url, attachment_location, &attachment)?;
+
+        Ok(attachment)
     }
 }
 

components/remote_settings/src/lib.rs

@@ -155,8 +155,6 @@ impl RemoteSettingsClient {
     /// this is that there is not much an application can do in this situation other than fall back
     /// to the same default handling as if records have not been synced.
     ///
-    /// TODO(Bug 1919141):
-    ///
     /// Application-services schedules regular dumps of the server data for specific collections.
     /// For these collections, `get_records` will never return None.  If you would like to add your
     /// collection to this list, please reach out to the DISCO team.
@@ -198,8 +196,8 @@ impl RemoteSettingsClient {
     ///   - This method will throw if there is a network or other error when fetching the
     ///     attachment data.
     #[handle_error(Error)]
-    pub fn get_attachment(&self, attachment_id: String) -> ApiResult<Vec<u8>> {
-        self.internal.get_attachment(&attachment_id)
+    pub fn get_attachment(&self, attachment_location: String) -> ApiResult<Vec<u8>> {
+        self.internal.get_attachment(&attachment_location)
     }
 }
 

components/remote_settings/src/storage.rs

@@ -2,7 +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::{Attachment, RemoteSettingsRecord, Result};
+use crate::{RemoteSettingsRecord, Result};
 use camino::Utf8PathBuf;
 use rusqlite::{params, Connection, OptionalExtension};
 use serde_json;
@@ -114,19 +114,14 @@ impl Storage {
         &self,
         collection_url: &str,
         attachment_id: &str,
-    ) -> Result<Option<Attachment>> {
+    ) -> Result<Option<Vec<u8>>> {
         let mut stmt = self
             .conn
             .prepare("SELECT data FROM attachments WHERE id = ? AND collection_url = ?")?;
-        let result: Option<Vec<u8>> = stmt
+
+        Ok(stmt
             .query_row((attachment_id, collection_url), |row| row.get(0))
-            .optional()?;
-        if let Some(data) = result {
-            let attachment: Attachment = serde_json::from_slice(&data)?;
-            Ok(Some(attachment))
-        } else {
-            Ok(None)
-        }
+            .optional()?)
     }
 
     /// Set the list of records stored in the database, clearing out any previously stored records
@@ -171,7 +166,7 @@ impl Storage {
         &mut self,
         collection_url: &str,
         attachment_id: &str,
-        attachment: Attachment,
+        attachment: &[u8],
     ) -> Result<()> {
         let tx = self.conn.transaction()?;
 
@@ -181,11 +176,9 @@ impl Storage {
             params![collection_url],
         )?;
 
-        let data = serde_json::to_vec(&attachment)?;
-
         tx.execute(
             "INSERT OR REPLACE INTO attachments (id, collection_url, data) VALUES (?, ?, ?)",
-            params![attachment_id, collection_url, data],
+            params![attachment_id, collection_url, attachment],
         )?;
 
         tx.commit()?;
@@ -300,20 +293,22 @@ mod tests {
         let mut storage = Storage::new(":memory:".into())?;
 
         let collection_url = "https://example.com/api";
-        let attachment_id = "attachment1";
-        let attachment = Attachment {
+        let attachment_metadata = Attachment {
             filename: "abc".to_string(),
             mimetype: "application/json".to_string(),
             location: "tmp".to_string(),
             hash: "abc123".to_string(),
             size: 1024,
         };
 
+        let attachment = &[0x18, 0x64];
+
         // Store attachment
-        storage.set_attachment(collection_url, attachment_id, attachment.clone())?;
+        storage.set_attachment(collection_url, &attachment_metadata.location, attachment)?;
 
         // Get attachment
-        let fetched_attachment = storage.get_attachment(collection_url, attachment_id)?;
+        let fetched_attachment =
+            storage.get_attachment(collection_url, &attachment_metadata.location)?;
         assert!(fetched_attachment.is_some());
         let fetched_attachment = fetched_attachment.unwrap();
         assert_eq!(fetched_attachment, attachment);
@@ -326,30 +321,42 @@ mod tests {
         let mut storage = Storage::new(":memory:".into())?;
 
         let collection_url = "https://example.com/api";
-        let attachment_id = "attachment1";
-        let attachment_1 = Attachment {
+
+        let attachment_metadata_1 = Attachment {
             filename: "abc".to_string(),
             mimetype: "application/json".to_string(),
             location: "tmp".to_string(),
             hash: "abc123".to_string(),
             size: 1024,
         };
-        let attachment_2 = Attachment {
+
+        let attachment_metadata_2 = Attachment {
             filename: "def".to_string(),
             mimetype: "application/json".to_string(),
             location: "tmp".to_string(),
             hash: "def456".to_string(),
             size: 2048,
         };
 
+        let attachment_1 = &[0x18, 0x64];
+        let attachment_2 = &[0x12, 0x48];
+
         // Store first attachment
-        storage.set_attachment(collection_url, attachment_id, attachment_1.clone())?;
+        storage.set_attachment(
+            collection_url,
+            &attachment_metadata_1.location,
+            attachment_1,
+        )?;
 
         // Replace attachment with new data
-        storage.set_attachment(collection_url, attachment_id, attachment_2.clone())?;
+        storage.set_attachment(
+            collection_url,
+            &attachment_metadata_2.location,
+            attachment_2,
+        )?;
 
         // Get attachment
-        let fetched_attachment = storage.get_attachment(collection_url, attachment_id)?;
+        let fetched_attachment = storage.get_attachment(collection_url, "tmp")?;
         assert!(fetched_attachment.is_some());
         let fetched_attachment = fetched_attachment.unwrap();
         assert_eq!(fetched_attachment, attachment_2);
@@ -363,32 +370,45 @@ mod tests {
 
         let collection_url_1 = "https://example.com/api1";
         let collection_url_2 = "https://example.com/api2";
-        let attachment_id_1 = "attachment1";
-        let attachment_id_2 = "attachment2";
-        let attachment_1 = Attachment {
+
+        let attachment_metadata_1 = Attachment {
             filename: "abc".to_string(),
             mimetype: "application/json".to_string(),
-            location: "tmp".to_string(),
+            location: "first_tmp".to_string(),
             hash: "abc123".to_string(),
             size: 1024,
         };
-        let attachment_2 = Attachment {
+
+        let attachment_metadata_2 = Attachment {
             filename: "def".to_string(),
             mimetype: "application/json".to_string(),
-            location: "tmp".to_string(),
+            location: "second_tmp".to_string(),
             hash: "def456".to_string(),
             size: 2048,
         };
 
+        let attachment_1 = &[0x18, 0x64];
+        let attachment_2 = &[0x12, 0x48];
+
         // Set attachments for two different collections
-        storage.set_attachment(collection_url_1, attachment_id_1, attachment_1.clone())?;
-        storage.set_attachment(collection_url_2, attachment_id_2, attachment_2.clone())?;
+        storage.set_attachment(
+            collection_url_1,
+            &attachment_metadata_1.location,
+            attachment_1,
+        )?;
+        storage.set_attachment(
+            collection_url_2,
+            &attachment_metadata_2.location,
+            attachment_2,
+        )?;
 
         // Verify that only the attachment for the second collection remains
-        let fetched_attachment_1 = storage.get_attachment(collection_url_1, attachment_id_1)?;
+        let fetched_attachment_1 =
+            storage.get_attachment(collection_url_1, &attachment_metadata_1.location)?;
         assert!(fetched_attachment_1.is_none());
 
-        let fetched_attachment_2 = storage.get_attachment(collection_url_2, attachment_id_2)?;
+        let fetched_attachment_2 =
+            storage.get_attachment(collection_url_2, &attachment_metadata_2.location)?;
         assert!(fetched_attachment_2.is_some());
         let fetched_attachment_2 = fetched_attachment_2.unwrap();
         assert_eq!(fetched_attachment_2, attachment_2);
@@ -430,30 +450,31 @@ mod tests {
                 id: "2".to_string(),
                 last_modified: 200,
                 deleted: false,
-                attachment: None,
+                attachment: Some(Attachment {
+                    filename: "abc".to_string(),
+                    mimetype: "application/json".to_string(),
+                    location: "tmp".to_string(),
+                    hash: "abc123".to_string(),
+                    size: 1024,
+                }),
                 fields: serde_json::json!({"key": "value2"})
                     .as_object()
                     .unwrap()
                     .clone(),
             },
         ];
-        let attachment_id = "attachment1";
-        let attachment = Attachment {
-            filename: "abc".to_string(),
-            mimetype: "application/json".to_string(),
-            location: "tmp".to_string(),
-            hash: "abc123".to_string(),
-            size: 1024,
-        };
+
+        let attachment_location = "tmp";
+        let attachment = &[0x18, 0x64];
 
         // Set records and attachment
         storage.set_records(collection_url, &records)?;
-        storage.set_attachment(collection_url, attachment_id, attachment.clone())?;
+        storage.set_attachment(collection_url, attachment_location, attachment)?;
 
         // Verify they are stored
         let fetched_records = storage.get_records(collection_url)?;
         assert!(fetched_records.is_some());
-        let fetched_attachment = storage.get_attachment(collection_url, attachment_id)?;
+        let fetched_attachment = storage.get_attachment(collection_url, attachment_location)?;
         assert!(fetched_attachment.is_some());
 
         // Empty the storage
@@ -462,7 +483,7 @@ mod tests {
         // Verify they are deleted
         let fetched_records = storage.get_records(collection_url)?;
         assert!(fetched_records.is_none());
-        let fetched_attachment = storage.get_attachment(collection_url, attachment_id)?;
+        let fetched_attachment = storage.get_attachment(collection_url, attachment_location)?;
         assert!(fetched_attachment.is_none());
 
         Ok(())