From 5df6263ce4b386ff6632a96af9d3b5fa89fe51f3 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Wed, 13 Dec 2023 18:37:26 -0800 Subject: [PATCH 01/10] feat: dontCache option --- src/index.ts | 161 +++++++++++++++++++++++++++------------------ src/types.ts | 7 ++ src/utils.ts | 22 +++++-- test/index.spec.ts | 76 +++++++++++++++++++++ test/utils.spec.ts | 20 ++++-- 5 files changed, 212 insertions(+), 74 deletions(-) diff --git a/src/index.ts b/src/index.ts index ff2323a16..24c780f68 100644 --- a/src/index.ts +++ b/src/index.ts @@ -20,7 +20,7 @@ import { getNodeArch, ensureIsTruthyString, isOfficialLinuxIA32Download, - withTempDirectory, + mkdtemp, } from './utils'; export { getHostArch } from './utils'; @@ -42,58 +42,73 @@ async function validateArtifact( downloadedAssetPath: string, _downloadArtifact: ArtifactDownloader, ) { - return await withTempDirectoryIn(artifactDetails.tempDirectory, async tempFolder => { - // Don't try to verify the hash of the hash file itself - // and for older versions that don't have a SHASUMS256.txt - if ( - !artifactDetails.artifactName.startsWith('SHASUMS256') && - !artifactDetails.unsafelyDisableChecksums && - semver.gte(artifactDetails.version, '1.3.2') - ) { - let shasumPath: string; - const checksums = artifactDetails.checksums; - if (checksums) { - shasumPath = path.resolve(tempFolder, 'SHASUMS256.txt'); - const fileNames: string[] = Object.keys(checksums); - if (fileNames.length === 0) { - throw new Error( - 'Provided "checksums" object is empty, cannot generate a valid SHASUMS256.txt', - ); + return await withTempDirectoryIn( + artifactDetails.tempDirectory, + async tempFolder => { + // Don't try to verify the hash of the hash file itself + // and for older versions that don't have a SHASUMS256.txt + if ( + !artifactDetails.artifactName.startsWith('SHASUMS256') && + !artifactDetails.unsafelyDisableChecksums && + semver.gte(artifactDetails.version, '1.3.2') + ) { + let shasumPath: string; + const checksums = artifactDetails.checksums; + if (checksums) { + shasumPath = path.resolve(tempFolder, 'SHASUMS256.txt'); + const fileNames: string[] = Object.keys(checksums); + if (fileNames.length === 0) { + throw new Error( + 'Provided "checksums" object is empty, cannot generate a valid SHASUMS256.txt', + ); + } + const generatedChecksums = fileNames + .map(fileName => `${checksums[fileName]} *${fileName}`) + .join('\n'); + await fs.writeFile(shasumPath, generatedChecksums); + } else { + shasumPath = await _downloadArtifact({ + isGeneric: true, + version: artifactDetails.version, + artifactName: 'SHASUMS256.txt', + force: artifactDetails.force, + downloadOptions: artifactDetails.downloadOptions, + cacheRoot: artifactDetails.cacheRoot, + downloader: artifactDetails.downloader, + mirrorOptions: artifactDetails.mirrorOptions, + dontCache: artifactDetails.dontCache, + }); } - const generatedChecksums = fileNames - .map(fileName => `${checksums[fileName]} *${fileName}`) - .join('\n'); - await fs.writeFile(shasumPath, generatedChecksums); - } else { - shasumPath = await _downloadArtifact({ - isGeneric: true, - version: artifactDetails.version, - artifactName: 'SHASUMS256.txt', - force: artifactDetails.force, - downloadOptions: artifactDetails.downloadOptions, - cacheRoot: artifactDetails.cacheRoot, - downloader: artifactDetails.downloader, - mirrorOptions: artifactDetails.mirrorOptions, - }); - } - // For versions 1.3.2 - 1.3.4, need to overwrite the `defaultTextEncoding` option: - // https://github.com/electron/electron/pull/6676#discussion_r75332120 - if (semver.satisfies(artifactDetails.version, '1.3.2 - 1.3.4')) { - const validatorOptions: sumchecker.ChecksumOptions = {}; - validatorOptions.defaultTextEncoding = 'binary'; - const checker = new sumchecker.ChecksumValidator('sha256', shasumPath, validatorOptions); - await checker.validate( - path.dirname(downloadedAssetPath), - path.basename(downloadedAssetPath), - ); - } else { - await sumchecker('sha256', shasumPath, path.dirname(downloadedAssetPath), [ - path.basename(downloadedAssetPath), - ]); + try { + // For versions 1.3.2 - 1.3.4, need to overwrite the `defaultTextEncoding` option: + // https://github.com/electron/electron/pull/6676#discussion_r75332120 + if (semver.satisfies(artifactDetails.version, '1.3.2 - 1.3.4')) { + const validatorOptions: sumchecker.ChecksumOptions = {}; + validatorOptions.defaultTextEncoding = 'binary'; + const checker = new sumchecker.ChecksumValidator( + 'sha256', + shasumPath, + validatorOptions, + ); + await checker.validate( + path.dirname(downloadedAssetPath), + path.basename(downloadedAssetPath), + ); + } else { + await sumchecker('sha256', shasumPath, path.dirname(downloadedAssetPath), [ + path.basename(downloadedAssetPath), + ]); + } + } finally { + if (artifactDetails.dontCache) { + await fs.remove(path.dirname(shasumPath)); + } + } } - } - }); + }, + artifactDetails.dontCache !== true, + ); } /** @@ -137,11 +152,21 @@ export async function downloadArtifact( d('Cache miss'); } else { d('Cache hit'); + let artifactPath = cachedPath; + if (artifactDetails.dontCache) { + // Copy out of cache into temporary directory if dontCache + const tempDir = await mkdtemp(artifactDetails.tempDirectory); + artifactPath = path.resolve(tempDir, fileName); + await fs.copyFile(cachedPath, artifactPath); + } try { - await validateArtifact(artifactDetails, cachedPath, downloadArtifact); + await validateArtifact(artifactDetails, artifactPath, downloadArtifact); - return cachedPath; + return artifactPath; } catch (err) { + if (artifactDetails.dontCache) { + await fs.remove(path.dirname(artifactPath)); + } d("Artifact in cache didn't match checksums", err); d('falling back to re-download'); } @@ -161,21 +186,29 @@ export async function downloadArtifact( console.warn('For more info: https://electronjs.org/blog/linux-32bit-support'); } - return await withTempDirectoryIn(artifactDetails.tempDirectory, async tempFolder => { - const tempDownloadPath = path.resolve(tempFolder, getArtifactFileName(artifactDetails)); + return await withTempDirectoryIn( + artifactDetails.tempDirectory, + async tempFolder => { + const tempDownloadPath = path.resolve(tempFolder, getArtifactFileName(artifactDetails)); - const downloader = artifactDetails.downloader || (await getDownloaderForSystem()); - d( - `Downloading ${url} to ${tempDownloadPath} with options: ${JSON.stringify( - artifactDetails.downloadOptions, - )}`, - ); - await downloader.download(url, tempDownloadPath, artifactDetails.downloadOptions); + const downloader = artifactDetails.downloader || (await getDownloaderForSystem()); + d( + `Downloading ${url} to ${tempDownloadPath} with options: ${JSON.stringify( + artifactDetails.downloadOptions, + )}`, + ); + await downloader.download(url, tempDownloadPath, artifactDetails.downloadOptions); - await validateArtifact(artifactDetails, tempDownloadPath, downloadArtifact); + await validateArtifact(artifactDetails, tempDownloadPath, downloadArtifact); - return await cache.putFileInCache(url, tempDownloadPath, fileName); - }); + if (artifactDetails.dontCache) { + return tempDownloadPath; + } else { + return await cache.putFileInCache(url, tempDownloadPath, fileName); + } + }, + artifactDetails.dontCache !== true, + ); } /** diff --git a/src/types.ts b/src/types.ts index 0d98db32c..d4cc8ccd0 100644 --- a/src/types.ts +++ b/src/types.ts @@ -106,6 +106,13 @@ export interface ElectronDownloadRequestOptions { * It is used before artifacts are put into cache. */ tempDirectory?: string; + /** + * When set to `true`, do not put the downloaded artifact into the cache, leaving if + * in the temporary directory. You need to clean up the temporary directory yourself. + * + * Defaults to `false`. + */ + dontCache?: boolean; } export type ElectronPlatformArtifactDetails = { diff --git a/src/utils.ts b/src/utils.ts index a98472b3d..0a018c988 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -16,17 +16,29 @@ async function useAndRemoveDirectory( return result; } +export async function mkdtemp(parentDirectory: string = os.tmpdir()): Promise { + const tempDirectoryPrefix = 'electron-download-'; + return await fs.mkdtemp(path.resolve(parentDirectory, tempDirectoryPrefix)); +} + export async function withTempDirectoryIn( parentDirectory: string = os.tmpdir(), fn: (directory: string) => Promise, + removeAfter?: boolean, ): Promise { - const tempDirectoryPrefix = 'electron-download-'; - const tempDirectory = await fs.mkdtemp(path.resolve(parentDirectory, tempDirectoryPrefix)); - return useAndRemoveDirectory(tempDirectory, fn); + const tempDirectory = await mkdtemp(parentDirectory); + if (removeAfter === undefined ? true : removeAfter) { + return useAndRemoveDirectory(tempDirectory, fn); + } else { + return fn(tempDirectory); + } } -export async function withTempDirectory(fn: (directory: string) => Promise): Promise { - return withTempDirectoryIn(undefined, fn); +export async function withTempDirectory( + fn: (directory: string) => Promise, + removeAfter?: boolean, +): Promise { + return withTempDirectoryIn(undefined, fn, removeAfter); } export function normalizeVersion(version: string): string { diff --git a/test/index.spec.ts b/test/index.spec.ts index 58ed8c534..2f59a99bd 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -31,6 +31,8 @@ describe('Public API', () => { expect(typeof zipPath).toEqual('string'); expect(await fs.pathExists(zipPath)).toEqual(true); expect(path.extname(zipPath)).toEqual('.zip'); + expect(path.dirname(zipPath).startsWith(cacheRoot)).toEqual(true); + expect(fs.readdirSync(cacheRoot).length).toBeGreaterThan(0); }); it('should return a valid path to a downloaded zip file for nightly releases', async () => { @@ -129,6 +131,35 @@ describe('Public API', () => { expect(path.extname(zipPath)).toEqual('.zip'); process.env.ELECTRON_CUSTOM_VERSION = ''; }); + + it('should not put artifact in cache when dontCache=true', async () => { + const zipPath = await download('2.0.10', { + cacheRoot, + downloader, + dontCache: true, + }); + expect(typeof zipPath).toEqual('string'); + expect(await fs.pathExists(zipPath)).toEqual(true); + expect(path.extname(zipPath)).toEqual('.zip'); + expect(path.dirname(zipPath).startsWith(cacheRoot)).toEqual(false); + expect(fs.readdirSync(cacheRoot).length).toEqual(0); + }); + + it('should use cache hits when dontCache=true', async () => { + const zipPath = await download('2.0.9', { + cacheRoot, + downloader, + }); + await fs.writeFile(zipPath, 'cached content'); + const zipPath2 = await download('2.0.9', { + cacheRoot, + downloader, + dontCache: true, + }); + expect(zipPath2).not.toEqual(zipPath); + expect(path.dirname(zipPath2).startsWith(cacheRoot)).toEqual(false); + expect(await fs.readFile(zipPath2, 'utf8')).toEqual('cached content'); + }); }); describe('downloadArtifact()', () => { @@ -143,6 +174,8 @@ describe('Public API', () => { expect(await fs.pathExists(dtsPath)).toEqual(true); expect(path.basename(dtsPath)).toEqual('electron.d.ts'); expect(await fs.readFile(dtsPath, 'utf8')).toContain('declare namespace Electron'); + expect(path.dirname(dtsPath).startsWith(cacheRoot)).toEqual(true); + expect(fs.readdirSync(cacheRoot).length).toBeGreaterThan(0); }); it('should work with the default platform/arch', async () => { @@ -231,6 +264,49 @@ describe('Public API', () => { expect(path.basename(zipPath)).toMatchInlineSnapshot(`"electron-v2.0.10-darwin-x64.zip"`); }); + it('should not put artifact in cache when dontCache=true', async () => { + const driverPath = await downloadArtifact({ + cacheRoot, + downloader, + version: '2.0.9', + artifactName: 'chromedriver', + platform: 'darwin', + arch: 'x64', + dontCache: true, + }); + expect(await fs.pathExists(driverPath)).toEqual(true); + expect(path.basename(driverPath)).toMatchInlineSnapshot( + `"chromedriver-v2.0.9-darwin-x64.zip"`, + ); + expect(path.extname(driverPath)).toEqual('.zip'); + expect(path.dirname(driverPath).startsWith(cacheRoot)).toEqual(false); + expect(fs.readdirSync(cacheRoot).length).toEqual(0); + }); + + it('should use cache hits when dontCache=true', async () => { + const driverPath = await downloadArtifact({ + cacheRoot, + downloader, + version: '2.0.9', + artifactName: 'chromedriver', + platform: 'darwin', + arch: 'x64', + }); + await fs.writeFile(driverPath, 'cached content'); + const driverPath2 = await downloadArtifact({ + cacheRoot, + downloader, + version: '2.0.9', + artifactName: 'chromedriver', + platform: 'darwin', + arch: 'x64', + dontCache: true, + }); + expect(driverPath2).not.toEqual(driverPath); + expect(path.dirname(driverPath2).startsWith(cacheRoot)).toEqual(false); + expect(await fs.readFile(driverPath2, 'utf8')).toEqual('cached content'); + }); + describe('sumchecker', () => { beforeEach(() => { jest.clearAllMocks(); diff --git a/test/utils.spec.ts b/test/utils.spec.ts index 8fddc0533..3b0a1cf7e 100644 --- a/test/utils.spec.ts +++ b/test/utils.spec.ts @@ -47,13 +47,11 @@ describe('utils', () => { }); it('should delete the directory when the function terminates', async () => { - let mDir: string | undefined = undefined; - await withTempDirectory(async dir => { - mDir = dir; + const mDir = await withTempDirectory(async dir => { + return dir; }); expect(mDir).not.toBeUndefined(); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - expect(await fs.pathExists(mDir!)).toEqual(false); + expect(await fs.pathExists(mDir)).toEqual(false); }); it('should delete the directory and reject correctly even if the function throws', async () => { @@ -68,6 +66,18 @@ describe('utils', () => { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion expect(await fs.pathExists(mDir!)).toEqual(false); }); + + it('should not delete the directory if removeAfter is false', async () => { + const mDir = await withTempDirectory(async dir => { + return dir; + }, false); + expect(mDir).not.toBeUndefined(); + try { + expect(await fs.pathExists(mDir)).toEqual(true); + } finally { + await fs.remove(mDir); + } + }); }); describe('getHostArch()', () => { From e9fe425ca58cab6ebf7eb67115b716bf667ae1fa Mon Sep 17 00:00:00 2001 From: David Sanders Date: Thu, 18 Jan 2024 10:13:11 -0800 Subject: [PATCH 02/10] chore: fix typo in comment --- src/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types.ts b/src/types.ts index d4cc8ccd0..6afa06e0e 100644 --- a/src/types.ts +++ b/src/types.ts @@ -107,7 +107,7 @@ export interface ElectronDownloadRequestOptions { */ tempDirectory?: string; /** - * When set to `true`, do not put the downloaded artifact into the cache, leaving if + * When set to `true`, do not put the downloaded artifact into the cache, leaving it * in the temporary directory. You need to clean up the temporary directory yourself. * * Defaults to `false`. From c6f673e01ecf19fff90b1935588a75b9a691e5d1 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Fri, 12 Jul 2024 10:57:38 -0400 Subject: [PATCH 03/10] update to use enum --- src/types.ts | 30 ++++++++++++++++++++++++++---- src/utils.ts | 41 +++++++++++++++++++++++++++++++++++++++++ test/index.spec.ts | 18 +++++++++--------- 3 files changed, 76 insertions(+), 13 deletions(-) diff --git a/src/types.ts b/src/types.ts index d060855c0..6ffee8a93 100644 --- a/src/types.ts +++ b/src/types.ts @@ -82,6 +82,28 @@ export interface ElectronDownloadRequest { artifactName: string; } +export enum ElectronDownloadCacheMode { + /** + * Reads from the cache if present + * Writes to the cache after fetch if not present + */ + ReadWrite, + /** + * Reads from the cache if present + * Will **not** write back to the cache after fetching missing artifact + */ + ReadOnly, + /** + * Skips reading from the cache + * Will write back into the cache, overwriting anything currently in the cache after fetch + */ + WriteOnly, + /** + * Bypasses the cache completely, neither reads from nor writes to the cache + */ + Bypass, +} + /** * @category Download Electron */ @@ -90,6 +112,7 @@ export interface ElectronDownloadRequestOptions { * Whether to download an artifact regardless of whether it's in the cache directory. * * @defaultValue `false` + * @deprecated This option is depracated and directly maps to `cacheMode: ElectronDownloadCacheMode.WriteOnly` */ force?: boolean; /** @@ -149,12 +172,11 @@ export interface ElectronDownloadRequestOptions { */ tempDirectory?: string; /** - * When set to `true`, do not put the downloaded artifact into the cache, leaving it - * in the temporary directory. You need to clean up the temporary directory yourself. + * Controls the cache read and write behavior. * - * Defaults to `false`. + * Defaults to `ElectronDownloadCacheMode.ReadWrite`. */ - dontCache?: boolean; + cacheMode?: ElectronDownloadCacheMode; } /** diff --git a/src/utils.ts b/src/utils.ts index 04cdabdd3..751560e0f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -2,6 +2,11 @@ import * as childProcess from 'child_process'; import * as fs from 'fs-extra'; import * as os from 'os'; import * as path from 'path'; +import { + ElectronDownloadCacheMode, + ElectronGenericArtifactDetails, + ElectronPlatformArtifactDetailsWithDefaults, +} from './types'; async function useAndRemoveDirectory( directory: string, @@ -134,3 +139,39 @@ export function setEnv(key: string, value: string | undefined): void { process.env[key] = value; } } + +export function effectiveCacheMode( + artifactDetails: ElectronPlatformArtifactDetailsWithDefaults | ElectronGenericArtifactDetails, +): ElectronDownloadCacheMode { + if (artifactDetails.force) { + if (artifactDetails.cacheMode) { + throw new Error( + 'Setting both "force" and "cacheMode" is not supported, please exclusively use "cacheMode"', + ); + } + return ElectronDownloadCacheMode.ReadWrite; + } + + return artifactDetails.cacheMode || ElectronDownloadCacheMode.ReadWrite; +} + +export function shouldTryReadCache(cacheMode: ElectronDownloadCacheMode): boolean { + return ( + cacheMode === ElectronDownloadCacheMode.ReadOnly || + cacheMode === ElectronDownloadCacheMode.ReadWrite + ); +} + +export function shouldWriteCache(cacheMode: ElectronDownloadCacheMode): boolean { + return ( + cacheMode === ElectronDownloadCacheMode.WriteOnly || + cacheMode === ElectronDownloadCacheMode.ReadWrite + ); +} + +export function doesCallerOwnTemporaryOutput(cacheMode: ElectronDownloadCacheMode): boolean { + return ( + cacheMode === ElectronDownloadCacheMode.Bypass || + cacheMode === ElectronDownloadCacheMode.ReadOnly + ); +} diff --git a/test/index.spec.ts b/test/index.spec.ts index 2f59a99bd..732756d7c 100644 --- a/test/index.spec.ts +++ b/test/index.spec.ts @@ -5,7 +5,7 @@ import * as path from 'path'; import { FixtureDownloader } from './FixtureDownloader'; import { download, downloadArtifact } from '../src'; -import { DownloadOptions } from '../src/types'; +import { DownloadOptions, ElectronDownloadCacheMode } from '../src/types'; import * as sumchecker from 'sumchecker'; jest.mock('sumchecker'); @@ -132,11 +132,11 @@ describe('Public API', () => { process.env.ELECTRON_CUSTOM_VERSION = ''; }); - it('should not put artifact in cache when dontCache=true', async () => { + it('should not put artifact in cache when cacheMode=ReadOnly', async () => { const zipPath = await download('2.0.10', { cacheRoot, downloader, - dontCache: true, + cacheMode: ElectronDownloadCacheMode.ReadOnly, }); expect(typeof zipPath).toEqual('string'); expect(await fs.pathExists(zipPath)).toEqual(true); @@ -145,7 +145,7 @@ describe('Public API', () => { expect(fs.readdirSync(cacheRoot).length).toEqual(0); }); - it('should use cache hits when dontCache=true', async () => { + it('should use cache hits when cacheMode=ReadOnly', async () => { const zipPath = await download('2.0.9', { cacheRoot, downloader, @@ -154,7 +154,7 @@ describe('Public API', () => { const zipPath2 = await download('2.0.9', { cacheRoot, downloader, - dontCache: true, + cacheMode: ElectronDownloadCacheMode.ReadOnly, }); expect(zipPath2).not.toEqual(zipPath); expect(path.dirname(zipPath2).startsWith(cacheRoot)).toEqual(false); @@ -264,7 +264,7 @@ describe('Public API', () => { expect(path.basename(zipPath)).toMatchInlineSnapshot(`"electron-v2.0.10-darwin-x64.zip"`); }); - it('should not put artifact in cache when dontCache=true', async () => { + it('should not put artifact in cache when cacheMode=ReadOnly', async () => { const driverPath = await downloadArtifact({ cacheRoot, downloader, @@ -272,7 +272,7 @@ describe('Public API', () => { artifactName: 'chromedriver', platform: 'darwin', arch: 'x64', - dontCache: true, + cacheMode: ElectronDownloadCacheMode.ReadOnly, }); expect(await fs.pathExists(driverPath)).toEqual(true); expect(path.basename(driverPath)).toMatchInlineSnapshot( @@ -283,7 +283,7 @@ describe('Public API', () => { expect(fs.readdirSync(cacheRoot).length).toEqual(0); }); - it('should use cache hits when dontCache=true', async () => { + it('should use cache hits when cacheMode=ReadOnly', async () => { const driverPath = await downloadArtifact({ cacheRoot, downloader, @@ -300,7 +300,7 @@ describe('Public API', () => { artifactName: 'chromedriver', platform: 'darwin', arch: 'x64', - dontCache: true, + cacheMode: ElectronDownloadCacheMode.ReadOnly, }); expect(driverPath2).not.toEqual(driverPath); expect(path.dirname(driverPath2).startsWith(cacheRoot)).toEqual(false); From 92948eb30564ae649b0a3cf17b0539d361ae4c1a Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Fri, 12 Jul 2024 11:01:45 -0400 Subject: [PATCH 04/10] refactor: use clean up enum instead of magic optional bool --- src/index.ts | 7 +++++-- src/utils.ts | 13 +++++++++---- test/GotDownloader.network.spec.ts | 10 +++++----- test/utils.spec.ts | 13 +++++++------ 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/index.ts b/src/index.ts index 4ccc46ff1..58edc409f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -26,6 +26,7 @@ import { doesCallerOwnTemporaryOutput, effectiveCacheMode, shouldTryReadCache, + TempDirCleanUpMode, } from './utils'; export { getHostArch } from './utils'; @@ -113,7 +114,9 @@ async function validateArtifact( } } }, - !doesCallerOwnTemporaryOutput(effectiveCacheMode(artifactDetails)), + doesCallerOwnTemporaryOutput(effectiveCacheMode(artifactDetails)) + ? TempDirCleanUpMode.ORPHAN + : TempDirCleanUpMode.CLEAN, ); } @@ -221,7 +224,7 @@ export async function downloadArtifact( return await cache.putFileInCache(url, tempDownloadPath, fileName); } }, - !doesCallerOwnTemporaryOutput(cacheMode), + doesCallerOwnTemporaryOutput(cacheMode) ? TempDirCleanUpMode.ORPHAN : TempDirCleanUpMode.CLEAN, ); } diff --git a/src/utils.ts b/src/utils.ts index 751560e0f..f379047af 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -26,13 +26,18 @@ export async function mkdtemp(parentDirectory: string = os.tmpdir()): Promise( parentDirectory: string = os.tmpdir(), fn: (directory: string) => Promise, - removeAfter?: boolean, + cleanUp: TempDirCleanUpMode, ): Promise { const tempDirectory = await mkdtemp(parentDirectory); - if (removeAfter === undefined ? true : removeAfter) { + if (cleanUp === TempDirCleanUpMode.CLEAN) { return useAndRemoveDirectory(tempDirectory, fn); } else { return fn(tempDirectory); @@ -41,9 +46,9 @@ export async function withTempDirectoryIn( export async function withTempDirectory( fn: (directory: string) => Promise, - removeAfter?: boolean, + cleanUp: TempDirCleanUpMode, ): Promise { - return withTempDirectoryIn(undefined, fn, removeAfter); + return withTempDirectoryIn(undefined, fn, cleanUp); } export function normalizeVersion(version: string): string { diff --git a/test/GotDownloader.network.spec.ts b/test/GotDownloader.network.spec.ts index 62add393e..e2a7e3dd3 100644 --- a/test/GotDownloader.network.spec.ts +++ b/test/GotDownloader.network.spec.ts @@ -2,7 +2,7 @@ import * as fs from 'fs-extra'; import * as path from 'path'; import { GotDownloader } from '../src/GotDownloader'; -import { withTempDirectory } from '../src/utils'; +import { TempDirCleanUpMode, withTempDirectory } from '../src/utils'; import { EventEmitter } from 'events'; describe('GotDownloader', () => { @@ -26,7 +26,7 @@ describe('GotDownloader', () => { expect(await fs.pathExists(testFile)).toEqual(true); expect(await fs.readFile(testFile, 'utf8')).toMatchSnapshot(); expect(progressCallbackCalled).toEqual(true); - }); + }, TempDirCleanUpMode.CLEAN); }); it('should throw an error if the file does not exist', async function() { @@ -36,7 +36,7 @@ describe('GotDownloader', () => { const url = 'https://github.com/electron/electron/releases/download/v2.0.18/bad.file'; const snapshot = `[HTTPError: Response code 404 (Not Found) for ${url}]`; await expect(downloader.download(url, testFile)).rejects.toMatchInlineSnapshot(snapshot); - }); + }, TempDirCleanUpMode.CLEAN); }); it('should throw an error if the file write stream fails', async () => { @@ -55,7 +55,7 @@ describe('GotDownloader', () => { testFile, ), ).rejects.toMatchInlineSnapshot(`"bad write error thing"`); - }); + }, TempDirCleanUpMode.CLEAN); }); it('should download to a deep uncreated path', async () => { @@ -71,7 +71,7 @@ describe('GotDownloader', () => { ).resolves.toBeUndefined(); expect(await fs.pathExists(testFile)).toEqual(true); expect(await fs.readFile(testFile, 'utf8')).toMatchSnapshot(); - }); + }, TempDirCleanUpMode.CLEAN); }); }); }); diff --git a/test/utils.spec.ts b/test/utils.spec.ts index 3b0a1cf7e..73dd02bba 100644 --- a/test/utils.spec.ts +++ b/test/utils.spec.ts @@ -9,6 +9,7 @@ import { isOfficialLinuxIA32Download, getEnv, setEnv, + TempDirCleanUpMode, } from '../src/utils'; describe('utils', () => { @@ -39,17 +40,17 @@ describe('utils', () => { await withTempDirectory(async dir => { expect(await fs.pathExists(dir)).toEqual(true); expect(await fs.readdir(dir)).toEqual([]); - }); + }, TempDirCleanUpMode.CLEAN); }); it('should return the value the function returns', async () => { - expect(await withTempDirectory(async () => 1234)).toEqual(1234); + expect(await withTempDirectory(async () => 1234, TempDirCleanUpMode.CLEAN)).toEqual(1234); }); it('should delete the directory when the function terminates', async () => { const mDir = await withTempDirectory(async dir => { return dir; - }); + }, TempDirCleanUpMode.CLEAN); expect(mDir).not.toBeUndefined(); expect(await fs.pathExists(mDir)).toEqual(false); }); @@ -60,17 +61,17 @@ describe('utils', () => { withTempDirectory(async dir => { mDir = dir; throw 'my error'; - }), + }, TempDirCleanUpMode.CLEAN), ).rejects.toEqual('my error'); expect(mDir).not.toBeUndefined(); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion expect(await fs.pathExists(mDir!)).toEqual(false); }); - it('should not delete the directory if removeAfter is false', async () => { + it('should not delete the directory if told to orphan the temp dir', async () => { const mDir = await withTempDirectory(async dir => { return dir; - }, false); + }, TempDirCleanUpMode.ORPHAN); expect(mDir).not.toBeUndefined(); try { expect(await fs.pathExists(mDir)).toEqual(true); From 9f3f6571367fd554c637bb440cb40334028110b5 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Fri, 12 Jul 2024 11:05:50 -0400 Subject: [PATCH 05/10] clean up docs --- src/index.ts | 8 ++++---- src/types.ts | 8 ++++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/index.ts b/src/index.ts index 58edc409f..23686955d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -204,17 +204,17 @@ export async function downloadArtifact( } return await withTempDirectoryIn( - artifactDetails.tempDirectory, + details.tempDirectory, async tempFolder => { const tempDownloadPath = path.resolve(tempFolder, getArtifactFileName(details)); - const downloader = artifactDetails.downloader || (await getDownloaderForSystem()); + const downloader = details.downloader || (await getDownloaderForSystem()); d( `Downloading ${url} to ${tempDownloadPath} with options: ${JSON.stringify( - artifactDetails.downloadOptions, + details.downloadOptions, )}`, ); - await downloader.download(url, tempDownloadPath, artifactDetails.downloadOptions); + await downloader.download(url, tempDownloadPath, details.downloadOptions); await validateArtifact(details, tempDownloadPath, downloadArtifact); diff --git a/src/types.ts b/src/types.ts index 6ffee8a93..07fd631fc 100644 --- a/src/types.ts +++ b/src/types.ts @@ -174,6 +174,14 @@ export interface ElectronDownloadRequestOptions { /** * Controls the cache read and write behavior. * + * When set to either `ReadOnly` or `Bypass` the caller is responsible + * for cleaning up the returned file path once they are done using + * it. E.g. `fs.remove(path.dirname(pathFromElectronGet))` + * + * When set to either `WriteOnly` or `ReadWrite` (the default) the caller + * should not move or delete the file path that is returned as the path + * points directly to the disk cache. + * * Defaults to `ElectronDownloadCacheMode.ReadWrite`. */ cacheMode?: ElectronDownloadCacheMode; From 355fd37efc114b4e4c07f1483e979fd91ececdc0 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Fri, 12 Jul 2024 11:07:40 -0400 Subject: [PATCH 06/10] fix: map force to WriteOnly --- src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils.ts b/src/utils.ts index f379047af..284b6c958 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -154,7 +154,7 @@ export function effectiveCacheMode( 'Setting both "force" and "cacheMode" is not supported, please exclusively use "cacheMode"', ); } - return ElectronDownloadCacheMode.ReadWrite; + return ElectronDownloadCacheMode.WriteOnly; } return artifactDetails.cacheMode || ElectronDownloadCacheMode.ReadWrite; From 0ee63838efbd1263d6105cb971c7195e5a04adde Mon Sep 17 00:00:00 2001 From: David Sanders Date: Fri, 12 Jul 2024 18:50:28 -0700 Subject: [PATCH 07/10] chore: fix typo in comment --- src/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types.ts b/src/types.ts index 07fd631fc..7a4f7d02a 100644 --- a/src/types.ts +++ b/src/types.ts @@ -112,7 +112,7 @@ export interface ElectronDownloadRequestOptions { * Whether to download an artifact regardless of whether it's in the cache directory. * * @defaultValue `false` - * @deprecated This option is depracated and directly maps to `cacheMode: ElectronDownloadCacheMode.WriteOnly` + * @deprecated This option is deprecated and directly maps to `cacheMode: ElectronDownloadCacheMode.WriteOnly` */ force?: boolean; /** From 1f3aef6fca7ab3dffc63d066f63e39fd9bc537af Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Sat, 13 Jul 2024 11:37:13 -0700 Subject: [PATCH 08/10] Update src/types.ts Co-authored-by: Erick Zhao --- src/types.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/types.ts b/src/types.ts index 7a4f7d02a..5ccea45ae 100644 --- a/src/types.ts +++ b/src/types.ts @@ -174,17 +174,20 @@ export interface ElectronDownloadRequestOptions { /** * Controls the cache read and write behavior. * - * When set to either `ReadOnly` or `Bypass` the caller is responsible - * for cleaning up the returned file path once they are done using - * it. E.g. `fs.remove(path.dirname(pathFromElectronGet))` + * When set to either {@link ElectronDownloadCacheMode.ReadOnly | ReadOnly} or + * {@link ElectronDownloadCacheMode.Bypass | Bypass}, the caller is responsible + * for cleaning up the returned file path once they are done using it + * (e.g. via `fs.remove(path.dirname(pathFromElectronGet))`). * - * When set to either `WriteOnly` or `ReadWrite` (the default) the caller + * When set to either {@link ElectronDownloadCacheMode.WriteOnly | WriteOnly} or + * {@link ElectronDownloadCacheMode.ReadWrite | ReadWrite} (the default), the caller * should not move or delete the file path that is returned as the path * points directly to the disk cache. * - * Defaults to `ElectronDownloadCacheMode.ReadWrite`. + * This option cannot be used in conjunction with {@link ElectronDownloadRequestOptions.force}. + * + * @defaultValue {@link ElectronDownloadCacheMode.ReadWrite} */ - cacheMode?: ElectronDownloadCacheMode; } /** From f3e2ef521c17b12bd124d5c59e1cc1e39ce5d943 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Sat, 13 Jul 2024 11:37:19 -0700 Subject: [PATCH 09/10] Update src/types.ts Co-authored-by: Erick Zhao --- src/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types.ts b/src/types.ts index 5ccea45ae..a093dff6c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -112,7 +112,7 @@ export interface ElectronDownloadRequestOptions { * Whether to download an artifact regardless of whether it's in the cache directory. * * @defaultValue `false` - * @deprecated This option is deprecated and directly maps to `cacheMode: ElectronDownloadCacheMode.WriteOnly` + * @deprecated This option is deprecated and directly maps to {@link cacheMode | `cacheMode: ElectronDownloadCacheMode.WriteOnly`} */ force?: boolean; /** From 641415fba70d98aa28d025d5c9b201544c7aac2e Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Sun, 14 Jul 2024 11:14:30 -0700 Subject: [PATCH 10/10] i am a silly goose --- src/types.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/types.ts b/src/types.ts index a093dff6c..c753e1885 100644 --- a/src/types.ts +++ b/src/types.ts @@ -188,6 +188,7 @@ export interface ElectronDownloadRequestOptions { * * @defaultValue {@link ElectronDownloadCacheMode.ReadWrite} */ + cacheMode?: ElectronDownloadCacheMode; } /**