corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1761473 - Get rid of deprecated downloadToDisk() attachments method r=acottner,omc-reviewers,mviar

Browse Source 
Differential Revision: https://phabricator.services.mozilla.com/D234466
This commit is contained in:
Mathieu Leplatre
2025-05-21 14:46:24 +00:00
committed by mleplatre@mozilla.com
parent 8dd59e9654
commit 6995a90947
8 changed files with 117 additions and 247 deletions
Download Patch File Download Diff File Expand all files Collapse all files

121
services/settings/Attachments.sys.mjs
View File

@@ -499,67 +499,6 @@ export class Downloader {
async prune(excludeIds) {
return this.cacheImpl.prune(excludeIds);
}
/**
* @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.
*
* No-op if the file was already downloaded and not corrupted.
*
* @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`)
* @throws {Downloader.DownloadError} if the file could not be fetched.
* @throws {Downloader.BadContentError} if the downloaded file integrity is not valid.
* @throws {Downloader.ServerInfoError} if the server response is not valid.
* @throws {NetworkError} if fetching the attachment fails.
* @returns {String} the absolute file path to the downloaded attachment.
*/
async downloadToDisk(record, options = {}) {
const { retries = 3 } = options;
const {
attachment: { filename, size, hash },
} = record;
const localFilePath = PathUtils.join(
PathUtils.localProfileDir,
...this.folders,
filename
);
const localFileUrl = PathUtils.toFileURI(localFilePath);
await this._makeDirs();
let retried = 0;
while (true) {
if (
await lazy.RemoteSettingsWorker.checkFileHash(localFileUrl, size, hash)
) {
return localFileUrl;
}
// File does not exist or is corrupted.
if (retried > retries) {
throw new Downloader.BadContentError(localFilePath);
}
try {
// Download and write on disk.
const buffer = await this.downloadAsBytes(record, {
checkHash: false, // Hash will be checked on file.
retries: 0, // Already in a retry loop.
});
await IOUtils.write(localFilePath, new Uint8Array(buffer), {
tmpPath: `${localFilePath}.tmp`,
});
} catch (e) {
if (retried >= retries) {
throw e;
}
}
retried++;
}
}
/**
* Download the record attachment and return its content as bytes.
*
@@ -609,31 +548,6 @@ export 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 deleteFromDisk(record) {
const {
attachment: { filename },
} = record;
const path = PathUtils.join(
PathUtils.localProfileDir,
...this.folders,
filename
);
await IOUtils.remove(path);
await this._rmDirs();
}
async _fetchAttachment(url) {
const headers = new Headers();
headers.set("Accept-Encoding", "gzip");
@@ -688,25 +602,22 @@ export class Downloader {
// Separate variable to allow tests to override this.
static _RESOURCE_BASE_URL = "resource://app/defaults";
}
async _makeDirs() {
const dirPath = PathUtils.join(PathUtils.localProfileDir, ...this.folders);
await IOUtils.makeDirectory(dirPath, { createAncestors: true });
}
async _rmDirs() {
for (let i = this.folders.length; i > 0; i--) {
const dirPath = PathUtils.join(
PathUtils.localProfileDir,
...this.folders.slice(0, i)
);
try {
await IOUtils.remove(dirPath);
} catch (e) {
// This could fail if there's something in
// the folder we're not permitted to remove.
break;
}
}
/**
* A bare downloader that does not store anything in cache.
*/
export class UnstoredDownloader extends Downloader {
get cacheImpl() {
const cacheImpl = {
get: async () => {},
set: async () => {},
setMultiple: async () => {},
delete: async () => {},
prune: async () => {},
hasData: async () => false,
};
Object.defineProperty(this, "cacheImpl", { value: cacheImpl });
return cacheImpl;
}
}

6
services/settings/RemoteSettingsClient.sys.mjs
View File

@@ -257,11 +257,7 @@ class AttachmentDownloader extends Downloader {
async deleteAll() {
let allRecords = await this._client.db.list();
return Promise.all(
allRecords
.filter(r => !!r.attachment)
.map(r =>
Promise.all([this.deleteDownloaded(r), this.deleteFromDisk(r)])
)
allRecords.filter(r => !!r.attachment).map(r => this.deleteDownloaded(r))
);
}
}

4
services/settings/docs/index.rst
View File

@@ -177,10 +177,6 @@ The provided ``download`` helper will:
A ``downloadAsBytes()`` method returning an ``ArrayBuffer`` is also available, if writing the attachment locally 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:
Initial data

107
services/settings/test/unit/test_attachments_downloader.js
View File

@@ -28,14 +28,6 @@ const RECORD_OF_DUMP = {
let downloader;
let server;
function pathFromURL(url) {
const uri = Services.io.newURI(url);
const file = uri.QueryInterface(Ci.nsIFileURL).file;
return file.path;
}
const PROFILE_URL = PathUtils.toFileURI(PathUtils.localProfileDir);
add_setup(() => {
server = new HttpServer();
server.start(-1);
@@ -131,58 +123,6 @@ add_task(
);
add_task(clear_state);
add_task(async function test_download_writes_file_in_profile() {
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 IOUtils.exists(localFilePath));
const stat = await IOUtils.stat(localFilePath);
Assert.equal(stat.size, 1597);
});
add_task(clear_state);
add_task(async function test_download_as_bytes() {
const bytes = await downloader.downloadAsBytes(RECORD);
// See *.pem file in tests data.
Assert.ok(bytes.byteLength > 1500, `Wrong bytes size: ${bytes.byteLength}`);
});
add_task(clear_state);
add_task(async function test_file_is_redownloaded_if_size_does_not_match() {
const fileURL = await downloader.downloadToDisk(RECORD);
const localFilePath = pathFromURL(fileURL);
await IOUtils.writeUTF8(localFilePath, "bad-content");
let stat = await IOUtils.stat(localFilePath);
Assert.notEqual(stat.size, 1597);
await downloader.downloadToDisk(RECORD);
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.downloadToDisk(RECORD);
const localFilePath = pathFromURL(fileURL);
const byteArray = await IOUtils.read(localFilePath);
byteArray[0] = 42;
await IOUtils.write(localFilePath, byteArray);
let content = await IOUtils.readUTF8(localFilePath);
Assert.notEqual(content.slice(0, 5), "-----");
await downloader.downloadToDisk(RECORD);
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",
@@ -211,6 +151,14 @@ add_task(async function test_download_is_retried_3_times_if_download_fails() {
});
add_task(clear_state);
add_task(async function test_download_as_bytes() {
const bytes = await downloader.downloadAsBytes(RECORD);
// See *.pem file in tests data.
Assert.ok(bytes.byteLength > 1500, `Wrong bytes size: ${bytes.byteLength}`);
});
add_task(clear_state);
add_task(async function test_download_is_retried_3_times_if_content_fails() {
const record = {
id: "abc",
@@ -237,56 +185,17 @@ 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.downloadToDisk(RECORD);
const localFilePath = pathFromURL(fileURL);
Assert.ok(await IOUtils.exists(localFilePath));
await downloader.deleteFromDisk(RECORD);
Assert.ok(!(await IOUtils.exists(localFilePath)));
// And removes parent folders.
const parentFolder = PathUtils.join(
PathUtils.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);
await downloader.download(RECORD);
const fileURL = await downloader.downloadToDisk(RECORD);
const localFilePath = pathFromURL(fileURL);
Assert.ok(await IOUtils.exists(localFilePath));
await client.attachments.deleteAll();
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.downloadToDisk(RECORD);
Assert.equal(
fileURL,
[
PROFILE_URL,
"settings",
client.bucketName,
client.collectionName,
RECORD.attachment.filename,
].join("/")
);
});
add_task(clear_state);
add_task(async function test_downloader_reports_download_errors() {
await withFakeChannel("nightly", async () => {
const client = RemoteSettings("some-collection");