mirror of
https://github.com/mozilla/gecko-dev.git
synced 2024-11-24 13:21:05 +00:00
Bug 1634127 - Download attachments in IndexedDB by default r=necko-reviewers,robwu,dragana
This patch changes the default behaviour of `download()`.
- Previous file-based behaviour was moved to `downloadToDisk()` and `deleteFromDisk()`. Existing consumers were migrated to avoid behaviour change.
- `download()` has now `{useCache: true}` by default, option was dropped, and `deleteCached()` is now `deleteDownloaded()`
Differential Revision: https://phabricator.services.mozilla.com/D141980
This commit is contained in:
Mathieu Leplatre
parent
b017ffb1c8
commit
e5519cc11b
@ -304,7 +304,7 @@ const MessageLoaderUtils = {
|
||||
"newtab"
|
||||
);
|
||||
// Await here in order to capture the exceptions for reporting.
|
||||
await downloader.download(record.data, {
|
||||
await downloader.downloadToDisk(record.data, {
|
||||
retries: RS_DOWNLOAD_MAX_RETRIES,
|
||||
});
|
||||
RemoteL10n.reloadL10n();
|
||||
|
||||
@ -974,7 +974,7 @@ describe("ASRouter", () => {
|
||||
.stub(MessageLoaderUtils, "_getRemoteSettingsMessages")
|
||||
.resolves([{ id: "message_1" }]);
|
||||
const spy = sandbox.spy();
|
||||
global.Downloader.prototype.download = spy;
|
||||
global.Downloader.prototype.downloadToDisk = spy;
|
||||
const provider = {
|
||||
id: "cfr",
|
||||
enabled: true,
|
||||
@ -2814,7 +2814,7 @@ describe("ASRouter", () => {
|
||||
.stub(MessageLoaderUtils, "_getRemoteSettingsMessages")
|
||||
.resolves([{ id: "message_1" }]);
|
||||
spy = sandbox.spy();
|
||||
global.Downloader.prototype.download = spy;
|
||||
global.Downloader.prototype.downloadToDisk = spy;
|
||||
});
|
||||
it("should be called with the expected dir path", async () => {
|
||||
const dlSpy = sandbox.spy(global, "Downloader");
|
||||
|
||||
@ -410,7 +410,12 @@ class QuickSuggest extends EventEmitter {
|
||||
await Promise.all(
|
||||
event.data.deleted
|
||||
.filter(d => d.attachment)
|
||||
.map(entry => this._rs.attachments.delete(entry))
|
||||
.map(entry =>
|
||||
Promise.all([
|
||||
this._rs.attachments.deleteDownloaded(entry), // type: data
|
||||
this._rs.attachments.deleteFromDisk(entry), // type: icon
|
||||
])
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
@ -420,7 +425,7 @@ class QuickSuggest extends EventEmitter {
|
||||
this._rs
|
||||
.get({ filters: { type: "icon" } })
|
||||
.then(icons =>
|
||||
Promise.all(icons.map(i => this._rs.attachments.download(i)))
|
||||
Promise.all(icons.map(i => this._rs.attachments.downloadToDisk(i)))
|
||||
),
|
||||
]);
|
||||
|
||||
@ -430,9 +435,7 @@ class QuickSuggest extends EventEmitter {
|
||||
this._resultsByKeyword.clear();
|
||||
|
||||
for (let record of data) {
|
||||
let { buffer } = await this._rs.attachments.download(record, {
|
||||
useCache: true,
|
||||
});
|
||||
let { buffer } = await this._rs.attachments.download(record);
|
||||
let results = JSON.parse(new TextDecoder("utf-8").decode(buffer));
|
||||
this._addResults(results);
|
||||
}
|
||||
@ -482,7 +485,7 @@ class QuickSuggest extends EventEmitter {
|
||||
if (!record) {
|
||||
return null;
|
||||
}
|
||||
return this._rs.attachments.download(record);
|
||||
return this._rs.attachments.downloadToDisk(record);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -33,7 +33,9 @@ const PublicSuffixList = {
|
||||
.then(async records => {
|
||||
if (records.length == 1) {
|
||||
// Get the downloaded file URI (most likely to be a no-op here, since file will exist).
|
||||
const fileURI = await this.CLIENT.attachments.download(records[0]);
|
||||
const fileURI = await this.CLIENT.attachments.downloadToDisk(
|
||||
records[0]
|
||||
);
|
||||
// Send a signal so that the C++ code loads the updated list on startup.
|
||||
this.notifyUpdate(fileURI);
|
||||
}
|
||||
@ -80,7 +82,7 @@ const PublicSuffixList = {
|
||||
async onUpdate({ data: { created, updated, deleted } }) {
|
||||
// In theory, this will never happen, we will never delete the record.
|
||||
if (deleted.length == 1) {
|
||||
await this.CLIENT.attachments.delete(deleted[0]);
|
||||
await this.CLIENT.attachments.deleteFromDisk(deleted[0]);
|
||||
}
|
||||
// Handle creation and update the same way
|
||||
const changed = created.concat(updated.map(u => u.new));
|
||||
@ -94,7 +96,7 @@ const PublicSuffixList = {
|
||||
// Download the updated file.
|
||||
let fileURI;
|
||||
try {
|
||||
fileURI = await this.CLIENT.attachments.download(changed[0]);
|
||||
fileURI = await this.CLIENT.attachments.downloadToDisk(changed[0]);
|
||||
} catch (err) {
|
||||
Cu.reportError(err);
|
||||
return;
|
||||
|
||||
@ -62,26 +62,26 @@ registerCleanupFunction(() => {
|
||||
});
|
||||
|
||||
/**
|
||||
* downloadCalled is used by mockDownload() and resetMockDownload()
|
||||
* downloadToDiskCalled is used by mockDownloadToDisk() and resetMockDownloadToDisk()
|
||||
* to keep track weather CLIENT.attachments.download is called or not
|
||||
* downloadBackup will help restore CLIENT.attachments.download to original definition
|
||||
* downloadToDiskBackup will help restore CLIENT.attachments.download to original definition
|
||||
* notifyUpdateBackup will help restore PublicSuffixList.notifyUpdate to original definition
|
||||
*/
|
||||
let downloadCalled = false;
|
||||
const downloadBackup = CLIENT.attachments.download;
|
||||
let downloadToDiskCalled = false;
|
||||
const downloadToDiskBackup = CLIENT.attachments.downloadToDisk;
|
||||
|
||||
// returns a fake fileURI and sends a signal with filePath and no nsifile
|
||||
const mockDownload = () => {
|
||||
downloadCalled = false;
|
||||
CLIENT.attachments.download = async rec => {
|
||||
downloadCalled = true;
|
||||
function mockDownloadToDisk() {
|
||||
downloadToDiskCalled = false;
|
||||
CLIENT.attachments.downloadToDisk = async rec => {
|
||||
downloadToDiskCalled = true;
|
||||
return `file://${mockedFilePath}`; // Create a fake file URI
|
||||
};
|
||||
};
|
||||
}
|
||||
|
||||
// resetMockDownload() must be run at the end of the test that uses mockDownload()
|
||||
const resetMockDownload = () => {
|
||||
CLIENT.attachments.download = downloadBackup;
|
||||
// resetMockDownloadToDisk() must be run at the end of the test that uses mockDownloadToDisk()
|
||||
const resetMockDownloadToDisk = () => {
|
||||
CLIENT.attachments.downloadToDisk = downloadToDiskBackup;
|
||||
};
|
||||
|
||||
add_task(async () => {
|
||||
@ -94,7 +94,7 @@ add_task(async () => {
|
||||
attachment: {},
|
||||
});
|
||||
|
||||
mockDownload();
|
||||
mockDownloadToDisk();
|
||||
|
||||
const promiseSignal = TestUtils.topicObserved(SIGNAL);
|
||||
await PublicSuffixList.init();
|
||||
@ -106,13 +106,13 @@ add_task(async () => {
|
||||
"File path sent when record is in DB."
|
||||
);
|
||||
await CLIENT.db.clear(); // Clean up the mockDownloaded record
|
||||
resetMockDownload();
|
||||
resetMockDownloadToDisk();
|
||||
});
|
||||
|
||||
add_task(async () => {
|
||||
info("File path sent when record updated.");
|
||||
|
||||
mockDownload();
|
||||
mockDownloadToDisk();
|
||||
|
||||
const promiseSignal = TestUtils.topicObserved(SIGNAL);
|
||||
await PublicSuffixList.init();
|
||||
@ -124,53 +124,53 @@ add_task(async () => {
|
||||
mockedFilePath,
|
||||
"File path sent when record updated."
|
||||
);
|
||||
resetMockDownload();
|
||||
resetMockDownloadToDisk();
|
||||
});
|
||||
|
||||
add_task(async () => {
|
||||
info("Attachment downloaded when record created.");
|
||||
|
||||
mockDownload();
|
||||
mockDownloadToDisk();
|
||||
|
||||
await PublicSuffixList.init();
|
||||
await CLIENT.emit("sync", { data: PAYLOAD_CREATED_RECORDS });
|
||||
|
||||
Assert.equal(
|
||||
downloadCalled,
|
||||
downloadToDiskCalled,
|
||||
true,
|
||||
"Attachment downloaded when record created."
|
||||
);
|
||||
resetMockDownload();
|
||||
resetMockDownloadToDisk();
|
||||
});
|
||||
|
||||
add_task(async () => {
|
||||
info("Attachment downloaded when record updated.");
|
||||
|
||||
mockDownload();
|
||||
mockDownloadToDisk();
|
||||
|
||||
await PublicSuffixList.init();
|
||||
await CLIENT.emit("sync", { data: PAYLOAD_UPDATED_RECORDS });
|
||||
|
||||
Assert.equal(
|
||||
downloadCalled,
|
||||
downloadToDiskCalled,
|
||||
true,
|
||||
"Attachment downloaded when record updated."
|
||||
);
|
||||
resetMockDownload();
|
||||
resetMockDownloadToDisk();
|
||||
});
|
||||
|
||||
add_task(async () => {
|
||||
info("No download when more than one record is changed.");
|
||||
|
||||
mockDownload();
|
||||
mockDownloadToDisk();
|
||||
|
||||
await PublicSuffixList.init();
|
||||
await CLIENT.emit("sync", { data: PAYLOAD_UPDATED_AND_CREATED_RECORDS });
|
||||
|
||||
Assert.equal(
|
||||
downloadCalled,
|
||||
downloadToDiskCalled,
|
||||
false,
|
||||
"No download when more than one record is changed."
|
||||
);
|
||||
resetMockDownload();
|
||||
resetMockDownloadToDisk();
|
||||
});
|
||||
|
||||
@ -610,7 +610,7 @@ class CRLiteFilters {
|
||||
for (let filter of filtersToDownload) {
|
||||
try {
|
||||
// If we've already downloaded this, the backend should just grab it from its cache.
|
||||
let localURI = await this.client.attachments.download(filter);
|
||||
let localURI = await this.client.attachments.downloadToDisk(filter);
|
||||
let buffer = await (await fetch(localURI)).arrayBuffer();
|
||||
let bytes = new Uint8Array(buffer);
|
||||
log.debug(`Downloaded ${filter.details.name}: ${bytes.length} bytes`);
|
||||
|
||||
@ -116,50 +116,37 @@ class Downloader {
|
||||
* If omitted, the attachmentId option must be set.
|
||||
* @param {Object} options Some download options.
|
||||
* @param {Number} options.retries Number of times download should be retried (default: `3`)
|
||||
* @param {Number} options.checkHash Check content integrity (default: `true`)
|
||||
* @param {Boolean} options.checkHash Check content integrity (default: `true`)
|
||||
* @param {string} options.attachmentId The attachment identifier to use for
|
||||
* caching and accessing the attachment.
|
||||
* (default: record.id)
|
||||
* @param {Boolean} options.useCache Whether to use a cache to read and store
|
||||
* the attachment. (default: false)
|
||||
* (default: `record.id`)
|
||||
* @param {Boolean} options.fallbackToCache Return the cached attachment when the
|
||||
* input record cannot be fetched.
|
||||
* (default: false)
|
||||
* (default: `false`)
|
||||
* @param {Boolean} options.fallbackToDump Use the remote settings dump as a
|
||||
* potential source of the attachment.
|
||||
* (default: false)
|
||||
* (default: `false`)
|
||||
* @throws {Downloader.DownloadError} if the file could not be fetched.
|
||||
* @throws {Downloader.BadContentError} if the downloaded content integrity is not valid.
|
||||
* @returns {Object} An object with two properties:
|
||||
* buffer: ArrayBuffer with the file content.
|
||||
* record: Record associated with the bytes.
|
||||
* _source: identifies the source of the result. Used for testing.
|
||||
* `buffer` `ArrayBuffer`: the file content.
|
||||
* `record` `Object`: record associated with the attachment.
|
||||
* `_source` `String`: identifies the source of the result. Used for testing.
|
||||
*/
|
||||
async download(record, options) {
|
||||
let {
|
||||
retries,
|
||||
checkHash,
|
||||
attachmentId = record?.id,
|
||||
useCache = false,
|
||||
fallbackToCache = false,
|
||||
fallbackToDump = false,
|
||||
} = options || {};
|
||||
|
||||
if (!useCache) {
|
||||
// For backwards compatibility.
|
||||
// WARNING: Its return type is different from what's documented.
|
||||
// See downloadToDisk's documentation.
|
||||
return this.downloadToDisk(record, options);
|
||||
}
|
||||
|
||||
if (!this.cacheImpl) {
|
||||
throw new Error("useCache is true but there is no cacheImpl!");
|
||||
}
|
||||
|
||||
if (!attachmentId) {
|
||||
// Check for pre-condition. This should not happen, but it is explicitly
|
||||
// checked to avoid mixing up attachments, which could be dangerous.
|
||||
throw new Error("download() was called without attachmentId or recordID");
|
||||
throw new Error(
|
||||
"download() was called without attachmentId or `record.id`"
|
||||
);
|
||||
}
|
||||
|
||||
const dumpInfo = new LazyRecordAndBuffer(() =>
|
||||
@ -184,7 +171,7 @@ class Downloader {
|
||||
}
|
||||
|
||||
// Check if the requested attachment has already been cached.
|
||||
if (useCache && record) {
|
||||
if (record) {
|
||||
if (await cacheInfo.isMatchingRequestedRecord(record)) {
|
||||
try {
|
||||
return { ...(await cacheInfo.getResult()), _source: "cache_match" };
|
||||
@ -206,12 +193,10 @@ class Downloader {
|
||||
checkHash,
|
||||
});
|
||||
const blob = new Blob([newBuffer]);
|
||||
if (useCache) {
|
||||
// Caching is optional, don't wait for the cache before returning.
|
||||
this.cacheImpl
|
||||
.set(attachmentId, { record, blob })
|
||||
.catch(e => Cu.reportError(e));
|
||||
}
|
||||
// Store in cache but don't wait for it before returning.
|
||||
this.cacheImpl
|
||||
.set(attachmentId, { record, blob })
|
||||
.catch(e => Cu.reportError(e));
|
||||
return { buffer: newBuffer, record, _source: "remote_match" };
|
||||
} catch (e) {
|
||||
// No network, corrupted content, etc.
|
||||
@ -263,6 +248,30 @@ class Downloader {
|
||||
}
|
||||
|
||||
/**
|
||||
* Delete the record attachment downloaded locally.
|
||||
* No-op if the attachment does not exist.
|
||||
*
|
||||
* @param record A Remote Settings entry with attachment.
|
||||
* @param {Object} options Some options.
|
||||
* @param {string} options.attachmentId The attachment identifier to use for
|
||||
* accessing and deleting the attachment.
|
||||
* (default: `record.id`)
|
||||
*/
|
||||
async deleteDownloaded(record, options) {
|
||||
let { attachmentId = record?.id } = options || {};
|
||||
if (!attachmentId) {
|
||||
// Check for pre-condition. This should not happen, but it is explicitly
|
||||
// checked to avoid mixing up attachments, which could be dangerous.
|
||||
throw new Error(
|
||||
"deleteDownloaded() was called without attachmentId or `record.id`"
|
||||
);
|
||||
}
|
||||
return this.cacheImpl.delete(attachmentId);
|
||||
}
|
||||
|
||||
/**
|
||||
* @deprecated See https://bugzilla.mozilla.org/show_bug.cgi?id=1634127
|
||||
*
|
||||
* Download the record attachment into the local profile directory
|
||||
* and return a file:// URL that points to the local path.
|
||||
*
|
||||
@ -308,7 +317,7 @@ class Downloader {
|
||||
checkHash: false, // Hash will be checked on file.
|
||||
retries: 0, // Already in a retry loop.
|
||||
});
|
||||
await OS.File.writeAtomic(localFilePath, buffer, {
|
||||
await IOUtils.write(localFilePath, new Uint8Array(buffer), {
|
||||
tmpPath: `${localFilePath}.tmp`,
|
||||
});
|
||||
} catch (e) {
|
||||
@ -326,7 +335,7 @@ class Downloader {
|
||||
* @param {Object} record A Remote Settings entry with attachment.
|
||||
* @param {Object} options Some download options.
|
||||
* @param {Number} options.retries Number of times download should be retried (default: `3`)
|
||||
* @param {Number} options.checkHash Check content integrity (default: `true`)
|
||||
* @param {Boolean} options.checkHash Check content integrity (default: `true`)
|
||||
* @throws {Downloader.DownloadError} if the file could not be fetched.
|
||||
* @throws {Downloader.BadContentError} if the downloaded content integrity is not valid.
|
||||
* @returns {ArrayBuffer} the file content.
|
||||
@ -361,12 +370,18 @@ class Downloader {
|
||||
}
|
||||
|
||||
/**
|
||||
* @deprecated See https://bugzilla.mozilla.org/show_bug.cgi?id=1634127
|
||||
*
|
||||
* Delete the record attachment downloaded locally.
|
||||
* This is the counterpart of `downloadToDisk()`.
|
||||
* Use `deleteDownloaded()` if `download()` was used to retrieve
|
||||
* the attachment.
|
||||
*
|
||||
* No-op if the related file does not exist.
|
||||
*
|
||||
* @param record A Remote Settings entry with attachment.
|
||||
*/
|
||||
async delete(record) {
|
||||
async deleteFromDisk(record) {
|
||||
const {
|
||||
attachment: { filename },
|
||||
} = record;
|
||||
@ -375,14 +390,10 @@ class Downloader {
|
||||
...this.folders,
|
||||
filename
|
||||
);
|
||||
await OS.File.remove(path, { ignoreAbsent: true });
|
||||
await IOUtils.remove(path);
|
||||
await this._rmDirs();
|
||||
}
|
||||
|
||||
async deleteCached(attachmentId) {
|
||||
return this.cacheImpl.delete(attachmentId);
|
||||
}
|
||||
|
||||
async _baseAttachmentsURL() {
|
||||
if (!this._cdnURL) {
|
||||
const server = Utils.SERVER_URL;
|
||||
@ -457,7 +468,7 @@ class Downloader {
|
||||
OS.Constants.Path.localProfileDir,
|
||||
...this.folders
|
||||
);
|
||||
await OS.File.makeDir(dirPath, { from: OS.Constants.Path.localProfileDir });
|
||||
await IOUtils.makeDirectory(dirPath, { createAncestors: true });
|
||||
}
|
||||
|
||||
async _rmDirs() {
|
||||
@ -467,7 +478,7 @@ class Downloader {
|
||||
...this.folders.slice(0, i)
|
||||
);
|
||||
try {
|
||||
await OS.File.removeEmptyDir(dirPath, { ignoreAbsent: true });
|
||||
await IOUtils.remove(dirPath);
|
||||
} catch (e) {
|
||||
// This could fail if there's something in
|
||||
// the folder we're not permitted to remove.
|
||||
|
||||
@ -216,7 +216,11 @@ class AttachmentDownloader extends Downloader {
|
||||
async deleteAll() {
|
||||
let allRecords = await this._client.db.list();
|
||||
return Promise.all(
|
||||
allRecords.filter(r => !!r.attachment).map(r => this.delete(r))
|
||||
allRecords
|
||||
.filter(r => !!r.attachment)
|
||||
.map(r =>
|
||||
Promise.all([this.deleteDownloaded(r), this.deleteFromDisk(r)])
|
||||
)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@ -117,27 +117,25 @@ Remote files are not downloaded automatically. In order to keep attachments in s
|
||||
.filter(d => d.attachment);
|
||||
|
||||
// Remove local files of deleted records
|
||||
await Promise.all(toDelete.map(entry => client.attachments.delete(entry)));
|
||||
// Download attachments
|
||||
const fileURLs = await Promise.all(
|
||||
toDownload.map(entry => client.attachments.download(entry, { retries: 2 }))
|
||||
await Promise.all(
|
||||
toDelete.map(record => client.attachments.deleteDownloaded(record))
|
||||
);
|
||||
|
||||
// Open downloaded files...
|
||||
// Download new attachments
|
||||
const fileContents = await Promise.all(
|
||||
fileURLs.map(async url => {
|
||||
const r = await fetch(url);
|
||||
return r.blob();
|
||||
})
|
||||
toDownload.map(async record => {
|
||||
const { buffer } = await client.attachments.download(record);
|
||||
return buffer;
|
||||
});
|
||||
);
|
||||
});
|
||||
|
||||
The provided helper will:
|
||||
- fetch the remote binary content
|
||||
- write the file in the profile folder
|
||||
- write the file in the local IndexedDB
|
||||
- check the file size
|
||||
- check the content SHA256 hash
|
||||
- do nothing if the file is already present and sound locally.
|
||||
- do nothing if the attachment was already present and sound locally.
|
||||
|
||||
.. important::
|
||||
|
||||
@ -149,27 +147,22 @@ The provided helper will:
|
||||
|
||||
.. note::
|
||||
|
||||
The ``download()`` method does not return a file path but instead a ``file://`` URL which points to the locally-downloaded file.
|
||||
This will allow us to package attachments as part of a Firefox release (see `Bug 1542177 <https://bugzilla.mozilla.org/show_bug.cgi?id=1542177>`_)
|
||||
and return them to calling code as ``resource://`` from within a package archive.
|
||||
The ``download()`` method supports the following options:
|
||||
|
||||
- ``retries`` (default: ``3``): number of retries on network errors
|
||||
- ``fallbackToCache`` (default: ``false``): allows callers to fall back to the cached file and record, if the requested record's attachment fails to download.
|
||||
This enables callers to always have a valid pair of attachment and record,
|
||||
provided that the attachment has been retrieved at least once.
|
||||
- ``fallbackToDump`` (default: ``false``): activates a fallback to a dump that has been
|
||||
packaged with the client, when other ways to load the attachment have failed.
|
||||
See :ref:`services/packaging-attachments <services/packaging-attachments>` for more information.
|
||||
|
||||
.. note::
|
||||
|
||||
By default, the ``download()`` method is prone to leaving extraneous files in the profile directory
|
||||
(see `Bug 1634127 <https://bugzilla.mozilla.org/show_bug.cgi?id=1634127>`_).
|
||||
Pass the ``useCache`` option to use an IndexedDB-based cache, and unlock the following features:
|
||||
A ``downloadAsBytes()`` method returning an ``ArrayBuffer`` is also available, if writing the attachment locally is not necessary.
|
||||
|
||||
The ``fallbackToCache`` option allows callers to fall back to the cached file and record, if the requested record's attachment fails to download.
|
||||
This enables callers to always have a valid pair of attachment and record,
|
||||
provided that the attachment has been retrieved at least once.
|
||||
|
||||
The ``fallbackToDump`` option activates a fallback to a dump that has been
|
||||
packaged with the client, when other ways to load the attachment have failed.
|
||||
See :ref:`services/packaging-attachments <services/packaging-attachments>` for more information.
|
||||
|
||||
.. note::
|
||||
|
||||
A ``downloadAsBytes()`` method returning an ``ArrayBuffer`` is also available, if writing the attachment into the user profile is not necessary.
|
||||
Some ``downloadToDisk()`` and ``deleteFromDisk()`` methods are also available but generally discouraged, since they are prone to leaving extraneous files
|
||||
in the profile directory (see `Bug 1634127 <https://bugzilla.mozilla.org/show_bug.cgi?id=1634127>`_).
|
||||
|
||||
|
||||
.. _services/initial-data:
|
||||
|
||||
@ -85,21 +85,34 @@ function run_test() {
|
||||
|
||||
async function clear_state() {
|
||||
downloader = new Downloader("main", "some-collection");
|
||||
await downloader.delete(RECORD);
|
||||
const dummyCacheImpl = {
|
||||
get: async attachmentId => {},
|
||||
set: async (attachmentId, attachment) => {},
|
||||
delete: async attachmentId => {},
|
||||
};
|
||||
// The download() method requires a cacheImpl, but the Downloader
|
||||
// class does not have one. Define a dummy no-op one.
|
||||
Object.defineProperty(downloader, "cacheImpl", {
|
||||
value: dummyCacheImpl,
|
||||
// Writable to allow specific tests to override cacheImpl.
|
||||
writable: true,
|
||||
});
|
||||
|
||||
await downloader.deleteDownloaded(RECORD);
|
||||
}
|
||||
|
||||
add_task(clear_state);
|
||||
|
||||
add_task(async function test_download_writes_file_in_profile() {
|
||||
const fileURL = await downloader.download(RECORD);
|
||||
const fileURL = await downloader.downloadToDisk(RECORD);
|
||||
const localFilePath = pathFromURL(fileURL);
|
||||
|
||||
Assert.equal(
|
||||
fileURL,
|
||||
PROFILE_URL + "/settings/main/some-collection/test_file.pem"
|
||||
);
|
||||
Assert.ok(await OS.File.exists(localFilePath));
|
||||
const stat = await OS.File.stat(localFilePath);
|
||||
Assert.ok(await IOUtils.exists(localFilePath));
|
||||
const stat = await IOUtils.stat(localFilePath);
|
||||
Assert.equal(stat.size, 1597);
|
||||
});
|
||||
add_task(clear_state);
|
||||
@ -113,39 +126,38 @@ add_task(async function test_download_as_bytes() {
|
||||
add_task(clear_state);
|
||||
|
||||
add_task(async function test_file_is_redownloaded_if_size_does_not_match() {
|
||||
const fileURL = await downloader.download(RECORD);
|
||||
const fileURL = await downloader.downloadToDisk(RECORD);
|
||||
const localFilePath = pathFromURL(fileURL);
|
||||
await OS.File.writeAtomic(localFilePath, "bad-content", {
|
||||
encoding: "utf-8",
|
||||
});
|
||||
let stat = await OS.File.stat(localFilePath);
|
||||
await IOUtils.writeUTF8(localFilePath, "bad-content");
|
||||
let stat = await IOUtils.stat(localFilePath);
|
||||
Assert.notEqual(stat.size, 1597);
|
||||
|
||||
await downloader.download(RECORD);
|
||||
await downloader.downloadToDisk(RECORD);
|
||||
|
||||
stat = await OS.File.stat(localFilePath);
|
||||
stat = await IOUtils.stat(localFilePath);
|
||||
Assert.equal(stat.size, 1597);
|
||||
});
|
||||
add_task(clear_state);
|
||||
|
||||
add_task(async function test_file_is_redownloaded_if_corrupted() {
|
||||
const fileURL = await downloader.download(RECORD);
|
||||
const fileURL = await downloader.downloadToDisk(RECORD);
|
||||
const localFilePath = pathFromURL(fileURL);
|
||||
const byteArray = await OS.File.read(localFilePath);
|
||||
const byteArray = await IOUtils.read(localFilePath);
|
||||
byteArray[0] = 42;
|
||||
await OS.File.writeAtomic(localFilePath, byteArray);
|
||||
let content = await OS.File.read(localFilePath, { encoding: "utf-8" });
|
||||
await IOUtils.write(localFilePath, byteArray);
|
||||
let content = await IOUtils.readUTF8(localFilePath);
|
||||
Assert.notEqual(content.slice(0, 5), "-----");
|
||||
|
||||
await downloader.download(RECORD);
|
||||
await downloader.downloadToDisk(RECORD);
|
||||
|
||||
content = await OS.File.read(localFilePath, { encoding: "utf-8" });
|
||||
content = await IOUtils.readUTF8(localFilePath);
|
||||
Assert.equal(content.slice(0, 5), "-----");
|
||||
});
|
||||
add_task(clear_state);
|
||||
|
||||
add_task(async function test_download_is_retried_3_times_if_download_fails() {
|
||||
const record = {
|
||||
id: "abc",
|
||||
attachment: {
|
||||
...RECORD.attachment,
|
||||
location: "404-error.pem",
|
||||
@ -173,6 +185,7 @@ add_task(clear_state);
|
||||
|
||||
add_task(async function test_download_is_retried_3_times_if_content_fails() {
|
||||
const record = {
|
||||
id: "abc",
|
||||
attachment: {
|
||||
...RECORD.attachment,
|
||||
hash: "always-wrong",
|
||||
@ -197,38 +210,41 @@ add_task(async function test_download_is_retried_3_times_if_content_fails() {
|
||||
add_task(clear_state);
|
||||
|
||||
add_task(async function test_delete_removes_local_file() {
|
||||
const fileURL = await downloader.download(RECORD);
|
||||
const fileURL = await downloader.downloadToDisk(RECORD);
|
||||
const localFilePath = pathFromURL(fileURL);
|
||||
Assert.ok(await OS.File.exists(localFilePath));
|
||||
Assert.ok(await IOUtils.exists(localFilePath));
|
||||
|
||||
downloader.delete(RECORD);
|
||||
await downloader.deleteFromDisk(RECORD);
|
||||
|
||||
Assert.ok(!(await OS.File.exists(localFilePath)));
|
||||
Assert.ok(
|
||||
!(await OS.File.exists(
|
||||
OS.Path.join(OS.Constants.Path.localProfileDir, ...downloader.folders)
|
||||
))
|
||||
Assert.ok(!(await IOUtils.exists(localFilePath)));
|
||||
// And removes parent folders.
|
||||
const parentFolder = OS.Path.join(
|
||||
OS.Constants.Path.localProfileDir,
|
||||
...downloader.folders
|
||||
);
|
||||
Assert.ok(!(await IOUtils.exists(parentFolder)));
|
||||
});
|
||||
add_task(clear_state);
|
||||
|
||||
add_task(async function test_delete_all() {
|
||||
const client = RemoteSettings("some-collection");
|
||||
await client.db.create(RECORD);
|
||||
const fileURL = await downloader.download(RECORD);
|
||||
await downloader.download(RECORD);
|
||||
const fileURL = await downloader.downloadToDisk(RECORD);
|
||||
const localFilePath = pathFromURL(fileURL);
|
||||
Assert.ok(await OS.File.exists(localFilePath));
|
||||
Assert.ok(await IOUtils.exists(localFilePath));
|
||||
|
||||
await client.attachments.deleteAll();
|
||||
|
||||
Assert.ok(!(await OS.File.exists(localFilePath)));
|
||||
Assert.ok(!(await IOUtils.exists(localFilePath)));
|
||||
Assert.ok(!(await client.attachments.cacheImpl.get(RECORD.id)));
|
||||
});
|
||||
add_task(clear_state);
|
||||
|
||||
add_task(async function test_downloader_is_accessible_via_client() {
|
||||
const client = RemoteSettings("some-collection");
|
||||
|
||||
const fileURL = await client.attachments.download(RECORD);
|
||||
const fileURL = await client.attachments.downloadToDisk(RECORD);
|
||||
|
||||
Assert.equal(
|
||||
fileURL,
|
||||
@ -332,9 +348,7 @@ async function doTestDownloadCacheImpl({ simulateCorruption }) {
|
||||
};
|
||||
Object.defineProperty(downloader, "cacheImpl", { value: cacheImpl });
|
||||
|
||||
let downloadResult = await downloader.download(RECORD, {
|
||||
useCache: true,
|
||||
});
|
||||
let downloadResult = await downloader.download(RECORD);
|
||||
Assert.equal(downloadResult._source, "remote_match", "expected source");
|
||||
Assert.equal(downloadResult.buffer.byteLength, 1597, "expected result");
|
||||
Assert.equal(readCount, 1, "expected cache read attempts");
|
||||
@ -430,7 +444,7 @@ add_task(async function test_download_cached() {
|
||||
checkInfo(info5, "cache_fallback", "fallbackToCache ignores bad record");
|
||||
|
||||
// Bye bye cache.
|
||||
await client.attachments.deleteCached(attachmentId);
|
||||
await client.attachments.deleteDownloaded({ id: attachmentId });
|
||||
await Assert.rejects(
|
||||
downloadWithCache(null, { attachmentId, fallbackToCache: true }),
|
||||
/DownloadError: Could not download dummy filename/,
|
||||
@ -473,7 +487,6 @@ add_task(async function test_download_from_dump() {
|
||||
// If record matches, should happen before network request.
|
||||
const dump1 = await client.attachments.download(RECORD_OF_DUMP, {
|
||||
// Note: attachmentId not set, so should fall back to record.id.
|
||||
useCache: true,
|
||||
fallbackToDump: true,
|
||||
});
|
||||
checkInfo(dump1, "dump_match");
|
||||
@ -481,7 +494,6 @@ add_task(async function test_download_from_dump() {
|
||||
// If no record given, should try network first, but then fall back to dump.
|
||||
const dump2 = await client.attachments.download(null, {
|
||||
attachmentId: RECORD_OF_DUMP.id,
|
||||
useCache: true,
|
||||
fallbackToDump: true,
|
||||
});
|
||||
checkInfo(dump2, "dump_fallback");
|
||||
@ -493,7 +505,6 @@ add_task(async function test_download_from_dump() {
|
||||
});
|
||||
// The dump should take precedence over the cache.
|
||||
const dump3 = await client.attachments.download(RECORD_OF_DUMP, {
|
||||
useCache: true,
|
||||
fallbackToCache: true,
|
||||
fallbackToDump: true,
|
||||
});
|
||||
@ -503,7 +514,6 @@ add_task(async function test_download_from_dump() {
|
||||
// as a fallback (when the cache and dump are identical).
|
||||
const dump4 = await client.attachments.download(null, {
|
||||
attachmentId: RECORD_OF_DUMP.id,
|
||||
useCache: true,
|
||||
fallbackToCache: true,
|
||||
fallbackToDump: true,
|
||||
});
|
||||
@ -522,7 +532,6 @@ add_task(async function test_download_from_dump() {
|
||||
// When the record is not given, use the cache if it has a more recent record.
|
||||
const dump5 = await client.attachments.download(null, {
|
||||
attachmentId: RECORD_OF_DUMP.id,
|
||||
useCache: true,
|
||||
fallbackToCache: true,
|
||||
fallbackToDump: true,
|
||||
});
|
||||
@ -530,24 +539,21 @@ add_task(async function test_download_from_dump() {
|
||||
|
||||
// When a record is given, use whichever that has the matching last_modified.
|
||||
const dump6 = await client.attachments.download(RECORD_OF_DUMP, {
|
||||
useCache: true,
|
||||
fallbackToCache: true,
|
||||
fallbackToDump: true,
|
||||
});
|
||||
checkInfo(dump6, "dump_match");
|
||||
const dump7 = await client.attachments.download(RECORD_NEWER_THAN_DUMP, {
|
||||
useCache: true,
|
||||
fallbackToCache: true,
|
||||
fallbackToDump: true,
|
||||
});
|
||||
checkInfo(dump7, "cache_match", RECORD_NEWER_THAN_DUMP);
|
||||
|
||||
await client.attachments.deleteCached(RECORD_OF_DUMP.id);
|
||||
await client.attachments.deleteDownloaded(RECORD_OF_DUMP);
|
||||
|
||||
await Assert.rejects(
|
||||
client.attachments.download(null, {
|
||||
attachmentId: "filename-without-meta.txt",
|
||||
useCache: true,
|
||||
fallbackToDump: true,
|
||||
}),
|
||||
/DownloadError: Could not download filename-without-meta.txt/,
|
||||
@ -557,7 +563,6 @@ add_task(async function test_download_from_dump() {
|
||||
await Assert.rejects(
|
||||
client.attachments.download(null, {
|
||||
attachmentId: "filename-without-content.txt",
|
||||
useCache: true,
|
||||
fallbackToDump: true,
|
||||
}),
|
||||
/Could not download resource:\/\/rs-downloader-test\/settings\/dump-bucket\/dump-collection\/filename-without-content\.txt(?!\.meta\.json)/,
|
||||
|
||||
@ -446,7 +446,7 @@ class RegionDetector {
|
||||
const toDelete = deleted.filter(d => d.attachment);
|
||||
// Remove local files of deleted records
|
||||
await Promise.all(
|
||||
toDelete.map(entry => this._rsClient.attachments.delete(entry))
|
||||
toDelete.map(entry => this._rsClient.attachments.deleteDownloaded(entry))
|
||||
);
|
||||
await this._ensureRegionFilesDownloaded();
|
||||
}
|
||||
@ -464,10 +464,7 @@ class RegionDetector {
|
||||
log.info("_ensureRegionFilesDownloaded: Nothing to download");
|
||||
return;
|
||||
}
|
||||
let opts = { useCache: true };
|
||||
await Promise.all(
|
||||
records.map(r => this._rsClient.attachments.download(r, opts))
|
||||
);
|
||||
await Promise.all(records.map(r => this._rsClient.attachments.download(r)));
|
||||
log.info("_ensureRegionFilesDownloaded complete");
|
||||
this._regionFilesReady = true;
|
||||
}
|
||||
@ -480,9 +477,7 @@ class RegionDetector {
|
||||
*/
|
||||
async _fetchAttachment(id) {
|
||||
let record = (await this._rsClient.get({ filters: { id } })).pop();
|
||||
let { buffer } = await this._rsClient.attachments.download(record, {
|
||||
useCache: true,
|
||||
});
|
||||
let { buffer } = await this._rsClient.attachments.download(record);
|
||||
let text = new TextDecoder("utf-8").decode(buffer);
|
||||
return JSON.parse(text);
|
||||
}
|
||||
|
||||
@ -976,7 +976,6 @@ const ExtensionBlocklistMLBF = {
|
||||
_source: rsAttachmentSource,
|
||||
} = await this._client.attachments.download(record, {
|
||||
attachmentId: this.RS_ATTACHMENT_ID,
|
||||
useCache: true,
|
||||
fallbackToCache: true,
|
||||
fallbackToDump: true,
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user