diff --git a/services/settings/Attachments.sys.mjs b/services/settings/Attachments.sys.mjs index 9047ba4ae9a7..9982c2766119 100644 --- a/services/settings/Attachments.sys.mjs +++ b/services/settings/Attachments.sys.mjs @@ -189,6 +189,9 @@ export class Downloader { return null; } + // Save attachments in bulks. + const BULK_SAVE_COUNT = 50; + const url = (await lazy.Utils.baseAttachmentsURL()) + `bundles/${this.bucketName}--${this.collectionName}.zip`; @@ -218,7 +221,13 @@ export class Downloader { const tmpZipFile = await IOUtils.getFile(tmpZipFilePath); zipReader.open(tmpZipFile); - for (const entryName of zipReader.findEntries("*.meta.json")) { + const cacheEntries = []; + const zipFiles = Array.from(zipReader.findEntries("*.meta.json")); + allSuccess = !!zipFiles.length; + + for (let i = 0; i < zipFiles.length; i++) { + const lastLoop = i == zipFiles.length - 1; + const entryName = zipFiles[i]; try { // 3. Read the meta.json entry const recordZStream = zipReader.getInputStream(entryName); @@ -245,18 +254,31 @@ export class Downloader { const fileBytes = stream.readBytes(dataLength); const blob = new Blob([fileBytes]); - // 5. Save to cache - await this.cacheImpl.set(record.id, { record, blob }); + cacheEntries.push([record.id, { record, blob }]); stream.close(); zStream.close(); } catch (ex) { lazy.console.warn( - `${this.bucketName}/${this.collectionName}: Unable to save attachment entry for ${entryName}.`, + `${this.bucketName}/${this.collectionName}: Unable to extract attachment of ${entryName}.`, ex ); allSuccess = false; } + + // 5. Save bulk to cache (last loop or reached count) + if (lastLoop || cacheEntries.length == BULK_SAVE_COUNT) { + try { + await this.cacheImpl.setMultiple(cacheEntries); + } catch (ex) { + lazy.console.warn( + `${this.bucketName}/${this.collectionName}: Unable to save attachments in cache`, + ex + ); + allSuccess = false; + } + cacheEntries.splice(0); // start new bulk. + } } } catch (ex) { lazy.console.warn( diff --git a/services/settings/Database.sys.mjs b/services/settings/Database.sys.mjs index 266a70ed4a9d..43b5b4a5d1fc 100644 --- a/services/settings/Database.sys.mjs +++ b/services/settings/Database.sys.mjs @@ -233,22 +233,34 @@ export class Database { } async saveAttachment(attachmentId, attachment) { + return await this.saveAttachments([[attachmentId, attachment]]); + } + + async saveAttachments(idsAndBlobs) { try { await executeIDB( "attachments", store => { - if (attachment) { - store.put({ cid: this.identifier, attachmentId, attachment }); - } else { - store.delete([this.identifier, attachmentId]); + for (const [attachmentId, attachment] of idsAndBlobs) { + if (attachment) { + store.put({ cid: this.identifier, attachmentId, attachment }); + } else { + store.delete([this.identifier, attachmentId]); + } } }, - { desc: "saveAttachment(" + attachmentId + ") in " + this.identifier } + { + desc: + "saveAttachments(<" + + idsAndBlobs.length + + " items>) in " + + this.identifier, + } ); } catch (e) { throw new lazy.IDBHelpers.IndexedDBError( e, - "saveAttachment()", + "saveAttachments()", this.identifier ); } diff --git a/services/settings/RemoteSettingsClient.sys.mjs b/services/settings/RemoteSettingsClient.sys.mjs index 6b7c441c8d91..f1506d11dbd2 100644 --- a/services/settings/RemoteSettingsClient.sys.mjs +++ b/services/settings/RemoteSettingsClient.sys.mjs @@ -204,6 +204,9 @@ class AttachmentDownloader extends Downloader { set: async (attachmentId, attachment) => { return this._client.db.saveAttachment(attachmentId, attachment); }, + setMultiple: async attachmentsIdsBlobs => { + return this._client.db.saveAttachments(attachmentsIdsBlobs); + }, delete: async attachmentId => { return this._client.db.saveAttachment(attachmentId, null); }, diff --git a/services/settings/test/unit/test_attachments_downloader.js b/services/settings/test/unit/test_attachments_downloader.js index 4db566e66800..66dc65c3b3b3 100644 --- a/services/settings/test/unit/test_attachments_downloader.js +++ b/services/settings/test/unit/test_attachments_downloader.js @@ -71,6 +71,9 @@ async function clear_state() { set: async (id, obj) => { downloader.cache[id] = obj; }, + setMultiple: async idsObjs => { + idsObjs.forEach(([id, obj]) => (downloader.cache[id] = obj)); + }, delete: async id => { delete downloader.cache[id]; }, @@ -100,6 +103,11 @@ async function clear_state() { response.setHeader("Content-Type", "application/json; charset=UTF-8"); response.setStatusLine(null, 200, "OK"); }); + + // For tests that use a real client and DB cache, clear the local DB too. + const client = RemoteSettings("some-collection"); + await client.db.clear(); + await client.db.pruneAttachments([]); } add_task(clear_state); @@ -852,6 +860,30 @@ add_task(async function test_cacheAll_happy_path() { ); }); +add_task(async function test_cacheAll_using_real_db() { + const client = RemoteSettings("some-collection"); + + const allSuccess = await client.attachments.cacheAll(); + + Assert.ok( + allSuccess, + "Attachments cacheAll succesfully downloaded a bundle and saved all attachments" + ); + + Assert.equal( + (await client.attachments.cacheImpl.get("2")).record.title, + "test2", + "Test record 2 meta content appears accurate." + ); + Assert.equal( + await (await client.attachments.cacheImpl.get("2")).blob.text(), + "test2\n", + "Test file 2 content is accurate." + ); +}); + +add_task(clear_state); + add_task(async function test_cacheAll_skips_with_existing_data() { downloader.cache = { 1: "1", @@ -903,3 +935,23 @@ add_task(async function test_cacheAll_failed_unzip() { }); add_task(clear_state); + +add_task(async function test_cacheAll_failed_save() { + const client = RemoteSettings("some-collection"); + + const backup = client.db.saveAttachments; + client.db.saveAttachments = () => { + throw new Error("boom"); + }; + + const allSuccess = await client.attachments.cacheAll(); + + Assert.equal( + allSuccess, + false, + "Attachments cacheAll failed to save entries in DB and returned false" + ); + client.db.saveAttachments = backup; +}); + +add_task(clear_state);