From 34e5ec560b3b38461b46bd1346b71e404f521d4a Mon Sep 17 00:00:00 2001 From: Barret Rennie Date: Mon, 16 Jan 2023 17:55:19 +0000 Subject: [PATCH] Bug 1796878 - Port osfile.jsm usage to IOUtils in services/ r=markh Differential Revision: https://phabricator.services.mozilla.com/D163410 --- services/automation/ServicesAutomation.jsm | 39 +++---- services/common/logmanager.js | 103 ++++++------------ services/settings/Attachments.jsm | 24 ++-- .../test/unit/test_attachments_downloader.js | 25 ++--- 4 files changed, 67 insertions(+), 124 deletions(-) diff --git a/services/automation/ServicesAutomation.jsm b/services/automation/ServicesAutomation.jsm index 9d455f65147d..613c3f74934a 100644 --- a/services/automation/ServicesAutomation.jsm +++ b/services/automation/ServicesAutomation.jsm @@ -42,7 +42,6 @@ XPCOMUtils.defineLazyModuleGetters(lazy, { Svc: "resource://services-sync/util.js", FxAccountsClient: "resource://gre/modules/FxAccountsClient.jsm", FxAccountsConfig: "resource://gre/modules/FxAccountsConfig.jsm", - OS: "resource://gre/modules/osfile.jsm", }); XPCOMUtils.defineLazyGetter(lazy, "fxAccounts", () => { @@ -293,10 +292,7 @@ var Authentication = { */ var Sync = { getSyncLogsDirectory() { - return lazy.OS.Path.join( - lazy.OS.Constants.Path.profileDir, - ...["weave", "logs"] - ); + return PathUtils.join(PathUtils.profileDir, "weave", "logs"); }, async init() { @@ -361,19 +357,12 @@ var Sync = { async wipeLogs() { let outputDirectory = this.getSyncLogsDirectory(); - if (!(await lazy.OS.File.exists(outputDirectory))) { + if (!(await IOUtils.exists(outputDirectory))) { return; } LOG("Wiping existing Sync logs"); try { - let iterator = new lazy.OS.File.DirectoryIterator(outputDirectory); - await iterator.forEach(async entry => { - try { - await lazy.OS.File.remove(entry.path); - } catch (error) { - LOG("wipeLogs() could not remove " + entry.path, error); - } - }); + await IOUtils.remove(outputDirectory, { recursive: true }); } catch (error) { LOG("wipeLogs() failed", error); } @@ -383,27 +372,25 @@ var Sync = { let outputDirectory = this.getSyncLogsDirectory(); let entries = []; - if (await lazy.OS.File.exists(outputDirectory)) { + if (await IOUtils.exists(outputDirectory)) { // Iterate through the directory - let iterator = new lazy.OS.File.DirectoryIterator(outputDirectory); + for (const path of await IOUtils.getChildren(outputDirectory)) { + const info = await IOUtils.stat(path); - await iterator.forEach(async entry => { - let info = await lazy.OS.File.stat(entry.path); entries.push({ - path: entry.path, - name: entry.name, - lastModificationDate: info.lastModificationDate, + path, + name: PathUtils.filename(path), + lastModified: info.lastModified, }); - }); + } + entries.sort(function(a, b) { - return b.lastModificationDate - a.lastModificationDate; + return b.lastModified - a.lastModified; }); } const promises = entries.map(async entry => { - let content = await lazy.OS.File.read(entry.path, { - encoding: "utf-8", - }); + const content = await IOUtils.readUTF8(entry.path); return { name: entry.name, content, diff --git a/services/common/logmanager.js b/services/common/logmanager.js index 7950031acfe8..98ec0945e9af 100644 --- a/services/common/logmanager.js +++ b/services/common/logmanager.js @@ -12,7 +12,11 @@ const lazy = {}; ChromeUtils.defineESModuleGetters(lazy, { FileUtils: "resource://gre/modules/FileUtils.sys.mjs", }); -ChromeUtils.defineModuleGetter(lazy, "OS", "resource://gre/modules/osfile.jsm"); +ChromeUtils.defineModuleGetter( + lazy, + "NetUtil", + "resource://gre/modules/NetUtil.jsm" +); const { Preferences } = ChromeUtils.importESModule( "resource://gre/modules/Preferences.sys.mjs" @@ -182,45 +186,26 @@ class FlushableStorageAppender extends StorageStreamAppender { * Returns a promise that is resolved on completion or rejected with an error. */ async _copyStreamToFile(inputStream, subdirArray, outputFileName, log) { - // The log data could be large, so we don't want to pass it all in a single - // message, so use BUFFER_SIZE chunks. - const BUFFER_SIZE = 8192; + let outputDirectory = PathUtils.join(PathUtils.profileDir, ...subdirArray); + await IOUtils.makeDirectory(outputDirectory); + let fullOutputFileName = PathUtils.join(outputDirectory, outputFileName); - // get a binary stream - let binaryStream = Cc["@mozilla.org/binaryinputstream;1"].createInstance( - Ci.nsIBinaryInputStream - ); - binaryStream.setInputStream(inputStream); + let outputStream = Cc[ + "@mozilla.org/network/file-output-stream;1" + ].createInstance(Ci.nsIFileOutputStream); - let outputDirectory = lazy.OS.Path.join( - lazy.OS.Constants.Path.profileDir, - ...subdirArray + outputStream.init( + new lazy.FileUtils.File(fullOutputFileName), + -1, + -1, + Ci.nsIFileOutputStream.DEFER_OPEN ); - await lazy.OS.File.makeDir(outputDirectory, { - ignoreExisting: true, - from: lazy.OS.Constants.Path.profileDir, - }); - let fullOutputFileName = lazy.OS.Path.join(outputDirectory, outputFileName); - let output = await lazy.OS.File.open(fullOutputFileName, { write: true }); - try { - while (true) { - let available = binaryStream.available(); - if (!available) { - break; - } - let chunk = binaryStream.readByteArray( - Math.min(available, BUFFER_SIZE) - ); - await output.write(new Uint8Array(chunk)); - } - } finally { - try { - binaryStream.close(); // inputStream is closed by the binaryStream - await output.close(); - } catch (ex) { - log.error("Failed to close the input stream", ex); - } - } + + await new Promise(resolve => + lazy.NetUtil.asyncCopy(inputStream, outputStream, () => resolve()) + ); + + outputStream.close(); log.trace("finished copy to", fullOutputFileName); } } @@ -420,7 +405,7 @@ LogManager.prototype = { this._log.debug("Log cleanup threshold time: " + threshold); let shouldDelete = fileInfo => { - return fileInfo.lastModificationDate.getTime() < threshold; + return fileInfo.lastModified < threshold; }; return this._deleteLogFiles(shouldDelete); }, @@ -440,46 +425,28 @@ LogManager.prototype = { "ProfD", this._logFileSubDirectoryEntries ); - let iterator = new lazy.OS.File.DirectoryIterator(logDir.path); + for (const path of await IOUtils.getChildren(logDir.path)) { + const name = PathUtils.filename(path); - await iterator.forEach(async entry => { - // Note that we don't check this.logFilePrefix is in the name - we cleanup - // all files in this directory regardless of that prefix so old logfiles - // for prefixes no longer in use are still cleaned up. See bug 1279145. - if ( - !entry.name.startsWith("error-") && - !entry.name.startsWith("success-") - ) { - return; + if (!name.startsWith("error-") && !name.startsWith("success-")) { + continue; } + try { - // need to call .stat() as the enumerator doesn't give that to us on *nix. - let info = await lazy.OS.File.stat(entry.path); + const info = await IOUtils.stat(path); if (!cbShouldDelete(info)) { - return; + continue; } - this._log.trace( - " > Cleanup removing " + - entry.name + - " (" + - info.lastModificationDate.getTime() + - ")" - ); - await lazy.OS.File.remove(entry.path); - this._log.trace("Deleted " + entry.name); + + this._log.trace(` > Cleanup removing ${name} (${info.lastModified})`); + await IOUtils.remove(path); + this._log.trace(`Deleted ${name}`); } catch (ex) { this._log.debug( - "Encountered error trying to clean up old log file " + entry.name, + `Encountered error trying to clean up old log file ${name}`, ex ); } - }); - // Wait for this to close if we need to (but it might fail if OS.File has - // shut down) - try { - await iterator.close(); - } catch (e) { - this._log.warn("Failed to close directory iterator", e); } this._cleaningUpFileLogs = false; this._log.debug("Done deleting files."); diff --git a/services/settings/Attachments.jsm b/services/settings/Attachments.jsm index f7bd2a89d0d4..d8fc0a211d61 100644 --- a/services/settings/Attachments.jsm +++ b/services/settings/Attachments.jsm @@ -12,7 +12,6 @@ XPCOMUtils.defineLazyModuleGetters(lazy, { RemoteSettingsWorker: "resource://services-settings/RemoteSettingsWorker.jsm", Utils: "resource://services-settings/Utils.jsm", }); -ChromeUtils.defineModuleGetter(lazy, "OS", "resource://gre/modules/osfile.jsm"); class DownloadError extends Error { constructor(url, resp) { @@ -313,16 +312,12 @@ class Downloader { const { attachment: { filename, size, hash }, } = record; - const localFilePath = lazy.OS.Path.join( - lazy.OS.Constants.Path.localProfileDir, + const localFilePath = PathUtils.join( + PathUtils.localProfileDir, ...this.folders, filename ); - const localFileUrl = `file://${[ - ...lazy.OS.Path.split(lazy.OS.Constants.Path.localProfileDir).components, - ...this.folders, - filename, - ].join("/")}`; + const localFileUrl = PathUtils.toFileURI(localFilePath); await this._makeDirs(); @@ -413,8 +408,8 @@ class Downloader { const { attachment: { filename }, } = record; - const path = lazy.OS.Path.join( - lazy.OS.Constants.Path.localProfileDir, + const path = PathUtils.join( + PathUtils.localProfileDir, ...this.folders, filename ); @@ -500,17 +495,14 @@ class Downloader { static _RESOURCE_BASE_URL = "resource://app/defaults"; async _makeDirs() { - const dirPath = lazy.OS.Path.join( - lazy.OS.Constants.Path.localProfileDir, - ...this.folders - ); + 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 = lazy.OS.Path.join( - lazy.OS.Constants.Path.localProfileDir, + const dirPath = PathUtils.join( + PathUtils.localProfileDir, ...this.folders.slice(0, i) ); try { diff --git a/services/settings/test/unit/test_attachments_downloader.js b/services/settings/test/unit/test_attachments_downloader.js index 3630e8ba40e3..175890e07aa5 100644 --- a/services/settings/test/unit/test_attachments_downloader.js +++ b/services/settings/test/unit/test_attachments_downloader.js @@ -12,7 +12,6 @@ const { Downloader } = ChromeUtils.import( const { TelemetryTestUtils } = ChromeUtils.importESModule( "resource://testing-common/TelemetryTestUtils.sys.mjs" ); -const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm"); const RECORD = { id: "1f3a0802-648d-11ea-bd79-876a8b69c377", @@ -46,9 +45,7 @@ function pathFromURL(url) { return file.path; } -const PROFILE_URL = - "file://" + - OS.Path.split(OS.Constants.Path.localProfileDir).components.join("/"); +const PROFILE_URL = PathUtils.toFileURI(PathUtils.localProfileDir); function run_test() { server = new HttpServer(); @@ -156,7 +153,7 @@ add_task(async function test_download_writes_file_in_profile() { Assert.equal( fileURL, - PROFILE_URL + "/settings/main/some-collection/test_file.pem" + PROFILE_URL + "settings/main/some-collection/test_file.pem" ); Assert.ok(await IOUtils.exists(localFilePath)); const stat = await IOUtils.stat(localFilePath); @@ -265,8 +262,8 @@ add_task(async function test_delete_removes_local_file() { Assert.ok(!(await IOUtils.exists(localFilePath))); // And removes parent folders. - const parentFolder = OS.Path.join( - OS.Constants.Path.localProfileDir, + const parentFolder = PathUtils.join( + PathUtils.localProfileDir, ...downloader.folders ); Assert.ok(!(await IOUtils.exists(parentFolder))); @@ -295,13 +292,13 @@ add_task(async function test_downloader_is_accessible_via_client() { Assert.equal( fileURL, - [ - PROFILE_URL, - "settings", - client.bucketName, - client.collectionName, - RECORD.attachment.filename, - ].join("/") + PROFILE_URL + + [ + "settings", + client.bucketName, + client.collectionName, + RECORD.attachment.filename, + ].join("/") ); }); add_task(clear_state);