feat: make the cache to respect an artifact url

package.json

@@ -26,6 +26,7 @@
     "env-paths": "^2.2.0",
     "fs-extra": "^8.1.0",
     "got": "^9.6.0",
+    "sanitize-filename": "^1.6.2",
     "sumchecker": "^3.0.0"
   },
   "devDependencies": {
@@ -35,6 +36,7 @@
     "@types/got": "^9.4.4",
     "@types/jest": "^24.0.13",
     "@types/node": "^12.0.2",
+    "@types/sanitize-filename": "^1.1.28",
     "gh-pages": "^2.0.1",
     "husky": "^2.3.0",
     "jest": "^24.8.0",

src/Cache.ts

@@ -2,6 +2,7 @@ import debug from 'debug';
 import envPaths from 'env-paths';
 import * as fs from 'fs-extra';
 import * as path from 'path';
+import * as sanitize from 'sanitize-filename';
 
 const d = debug('@electron/get:cache');
 
@@ -12,21 +13,22 @@ const defaultCacheRoot = envPaths('electron', {
 export class Cache {
   constructor(private cacheRoot = defaultCacheRoot) {}
 
-  private getCachePath(fileName: string): string {
-    return path.resolve(this.cacheRoot, fileName);
+  private getCachePath(url: string, fileName: string): string {
+    const sanitizedUrl = sanitize(url);
+    return path.resolve(this.cacheRoot, sanitizedUrl, fileName);
   }
 
-  public async getPathForFileInCache(fileName: string): Promise<string | null> {
-    const cachePath = this.getCachePath(fileName);
+  public async getPathForFileInCache(url: string, fileName: string): Promise<string | null> {
+    const cachePath = this.getCachePath(url, fileName);
     if (await fs.pathExists(cachePath)) {
       return cachePath;
     }
 
     return null;
   }
 
-  public async putFileInCache(currentPath: string, fileName: string): Promise<string> {
-    const cachePath = this.getCachePath(fileName);
+  public async putFileInCache(url: string, currentPath: string, fileName: string): Promise<string> {
+    const cachePath = this.getCachePath(url, fileName);
     d(`Moving ${currentPath} to ${cachePath}`);
     if (await fs.pathExists(cachePath)) {
       d('* Replacing existing file');

src/artifact-utils.ts

@@ -4,29 +4,17 @@ import { ensureIsTruthyString } from './utils';
 const BASE_URL = 'https://github.com/electron/electron/releases/download/';
 const NIGHTLY_BASE_URL = 'https://github.com/electron/nightlies/releases/download/';
 
-export enum FileNameUse {
-  LOCAL,
-  REMOTE,
-}
-
-export function getArtifactFileName(
-  details: ElectronArtifactDetails,
-  usage: FileNameUse = FileNameUse.LOCAL,
-) {
+export function getArtifactFileName(details: ElectronArtifactDetails) {
   ensureIsTruthyString(details, 'artifactName');
-  ensureIsTruthyString(details, 'version');
 
   if (details.isGeneric) {
-    // When downloading we have to use the artifact name directly as that it was is stored in the release on GitHub
-    if (usage === FileNameUse.REMOTE) {
-      return details.artifactName;
-    }
-    // When caching / using on your local disk we want the generic artifact to be versioned
-    return `${details.version}-${details.artifactName}`;
+    return details.artifactName;
   }
 
-  ensureIsTruthyString(details, 'platform');
   ensureIsTruthyString(details, 'arch');
+  ensureIsTruthyString(details, 'platform');
+  ensureIsTruthyString(details, 'version');
+
   return `${[
     details.artifactName,
     details.version,
@@ -57,7 +45,7 @@ export function getArtifactRemoteURL(details: ElectronArtifactDetails): string {
     base = mirrorVar('nightly_mirror', opts, NIGHTLY_BASE_URL);
   }
   const path = mirrorVar('customDir', opts, details.version);
-  const file = mirrorVar('customFilename', opts, getArtifactFileName(details, FileNameUse.REMOTE));
+  const file = mirrorVar('customFilename', opts, getArtifactFileName(details));
 
   return `${base}${path}/${file}`;
 }

src/index.ts

@@ -1,7 +1,7 @@
 import debug from 'debug';
 import * as path from 'path';
 
-import { getArtifactFileName, getArtifactRemoteURL, FileNameUse } from './artifact-utils';
+import { getArtifactFileName, getArtifactRemoteURL } from './artifact-utils';
 import {
   ElectronArtifactDetails,
   ElectronDownloadRequestOptions,
@@ -64,12 +64,13 @@ export async function downloadArtifact(
   artifactDetails.version = normalizeVersion(artifactDetails.version);
 
   const fileName = getArtifactFileName(artifactDetails);
+  const url = getArtifactRemoteURL(artifactDetails);
   const cache = new Cache(artifactDetails.cacheRoot);
 
   // Do not check if the file exists in the cache when force === true
   if (!artifactDetails.force) {
-    d(`Checking the cache for ${fileName}`);
-    const cachedPath = await cache.getPathForFileInCache(fileName);
+    d(`Checking the cache for ${fileName} (${url})`);
+    const cachedPath = await cache.getPathForFileInCache(url, fileName);
 
     if (cachedPath === null) {
       d('Cache miss');
@@ -93,13 +94,9 @@ export async function downloadArtifact(
   }
 
   return await withTempDirectory(async tempFolder => {
-    const tempDownloadPath = path.resolve(
-      tempFolder,
-      getArtifactFileName(artifactDetails, FileNameUse.REMOTE),
-    );
+    const tempDownloadPath = path.resolve(tempFolder, getArtifactFileName(artifactDetails));
 
     const downloader = artifactDetails.downloader || (await getDownloaderForSystem());
-    const url = getArtifactRemoteURL(artifactDetails);
     d(
       `Downloading ${url} to ${tempDownloadPath} with options: ${JSON.stringify(
         artifactDetails.downloadOptions,
@@ -127,6 +124,6 @@ export async function downloadArtifact(
       ]);
     }
 
-    return await cache.putFileInCache(tempDownloadPath, fileName);
+    return await cache.putFileInCache(url, tempDownloadPath, fileName);
   });
 }

test/Cache.spec.ts

@@ -1,13 +1,17 @@
 import * as fs from 'fs-extra';
 import * as os from 'os';
 import * as path from 'path';
+import * as sanitize from 'sanitize-filename';
 
 import { Cache } from '../src/Cache';
 
 describe('Cache', () => {
   let cacheDir: string;
   let cache: Cache;
 
+  const dummyUrl = 'dummy://';
+  const sanitizedDummyUrl = sanitize(dummyUrl);
+
   beforeEach(async () => {
     cacheDir = await fs.mkdtemp(path.resolve(os.tmpdir(), 'electron-download-spec-'));
     cache = new Cache(cacheDir);
@@ -17,45 +21,45 @@ describe('Cache', () => {
 
   describe('getPathForFileInCache()', () => {
     it('should return null for a file not in the cache', async () => {
-      expect(await cache.getPathForFileInCache('test.txt')).toBeNull();
+      expect(await cache.getPathForFileInCache(dummyUrl, 'test.txt')).toBeNull();
     });
 
     it('should return an absolute path for a file in the cache', async () => {
-      const cachePath = path.resolve(cacheDir, 'test.txt');
-      await fs.writeFile(cachePath, 'dummy data');
-      expect(await cache.getPathForFileInCache('test.txt')).toEqual(cachePath);
+      const cachePath = path.resolve(cacheDir, sanitizedDummyUrl, 'test.txt');
+      await fs.outputFile(cachePath, 'dummy data');
+      expect(await cache.getPathForFileInCache(dummyUrl, 'test.txt')).toEqual(cachePath);
     });
   });
 
   describe('putFileInCache()', () => {
     it('should throw an error if the provided file path does not exist', async () => {
       const fakePath = path.resolve(__dirname, 'fake.file');
-      await expect(cache.putFileInCache(fakePath, 'fake.file')).rejects.toHaveProperty(
+      await expect(cache.putFileInCache(dummyUrl, fakePath, 'fake.file')).rejects.toHaveProperty(
         'message',
         `ENOENT: no such file or directory, stat '${fakePath}'`,
       );
     });
 
     it('should delete the original file', async () => {
-      const originalPath = path.resolve(cacheDir, 'original.txt');
-      await fs.writeFile(originalPath, 'dummy data');
-      await cache.putFileInCache(originalPath, 'test.txt');
+      const originalPath = path.resolve(cacheDir, sanitizedDummyUrl, 'original.txt');
+      await fs.outputFile(originalPath, 'dummy data');
+      await cache.putFileInCache(dummyUrl, originalPath, 'test.txt');
       expect(await fs.pathExists(originalPath)).toEqual(false);
     });
 
     it('should create a new file in the cache with exactly the same content', async () => {
-      const originalPath = path.resolve(cacheDir, 'original.txt');
-      await fs.writeFile(originalPath, 'example content');
-      const cachePath = await cache.putFileInCache(originalPath, 'test.txt');
+      const originalPath = path.resolve(cacheDir, sanitizedDummyUrl, 'original.txt');
+      await fs.outputFile(originalPath, 'example content');
+      const cachePath = await cache.putFileInCache(dummyUrl, originalPath, 'test.txt');
       expect(cachePath.startsWith(cacheDir)).toEqual(true);
       expect(await fs.readFile(cachePath, 'utf8')).toEqual('example content');
     });
 
     it('should overwrite the file if it already exists in cache', async () => {
       const originalPath = path.resolve(cacheDir, 'original.txt');
-      await fs.writeFile(originalPath, 'example content');
-      await fs.writeFile(path.resolve(cacheDir, 'test.txt'), 'bad content');
-      const cachePath = await cache.putFileInCache(originalPath, 'test.txt');
+      await fs.outputFile(originalPath, 'example content');
+      await fs.outputFile(path.resolve(cacheDir, sanitizedDummyUrl, 'test.txt'), 'bad content');
+      const cachePath = await cache.putFileInCache(dummyUrl, originalPath, 'test.txt');
       expect(cachePath.startsWith(cacheDir)).toEqual(true);
       expect(await fs.readFile(cachePath, 'utf8')).toEqual('example content');
     });

test/artifact-utils.spec.ts

@@ -1,28 +1,15 @@
-import { getArtifactFileName, FileNameUse } from '../src/artifact-utils';
+import { getArtifactFileName } from '../src/artifact-utils';
 
 describe('artifact-utils', () => {
   describe('getArtifactFileName()', () => {
-    it('should return just the artifact name for remote generic artifacts', () => {
-      expect(
-        getArtifactFileName(
-          {
-            isGeneric: true,
-            artifactName: 'test.zip',
-            version: 'v1',
-          },
-          FileNameUse.REMOTE,
-        ),
-      ).toMatchInlineSnapshot(`"test.zip"`);
-    });
-
-    it('should return versioned artifact names for local generic artifacts', () => {
+    it('should return just the artifact name for generic artifacts', () => {
       expect(
         getArtifactFileName({
           isGeneric: true,
           artifactName: 'test.zip',
           version: 'v1',
         }),
-      ).toMatchInlineSnapshot(`"v1-test.zip"`);
+      ).toMatchInlineSnapshot(`"test.zip"`);
     });
 
     it('should return the correct hypenated artifact names for other artifacts', () => {

test/index.spec.ts

@@ -116,7 +116,7 @@ describe('Public API', () => {
         artifactName: 'electron.d.ts',
       });
       expect(await fs.pathExists(dtsPath)).toEqual(true);
-      expect(path.basename(dtsPath)).toEqual('v2.0.9-electron.d.ts');
+      expect(path.basename(dtsPath)).toEqual('electron.d.ts');
       expect(await fs.readFile(dtsPath, 'utf8')).toContain('declare namespace Electron');
     });
 

yarn.lock

@@ -624,6 +624,11 @@
   resolved "https://registry.yarnpkg.com/@types/normalize-package-data/-/normalize-package-data-2.4.0.tgz#e486d0d97396d79beedd0a6e33f4534ff6b4973e"
   integrity sha512-f5j5b/Gf71L+dbqxIpQ4Z2WlmI/mPJ0fOkGGmFgtb6sAu97EPczzbS3/tJKxmcYDj55OX6ssqwDAWOHIYDRDGA==
 
+"@types/sanitize-filename@^1.1.28":
+  version "1.1.28"
+  resolved "https://registry.yarnpkg.com/@types/sanitize-filename/-/sanitize-filename-1.1.28.tgz#baa18f5ce4330fcbb3ea7b62f65550963d9aaa07"
+  integrity sha1-uqGPXOQzD8uz6nti9lVQlj2aqgc=
+
 "@types/shelljs@^0.8.0":
   version "0.8.5"
   resolved "https://registry.yarnpkg.com/@types/shelljs/-/shelljs-0.8.5.tgz#1e507b2f6d1f893269bd3e851ec24419ef9beeea"
@@ -6106,6 +6111,13 @@ sane@^4.0.3:
     minimist "^1.1.1"
     walker "~1.0.5"
 
+sanitize-filename@^1.6.2:
+  version "1.6.2"
+  resolved "https://registry.yarnpkg.com/sanitize-filename/-/sanitize-filename-1.6.2.tgz#01b4fc8809f14e9d22761fe70380fe7f3f902185"
+  integrity sha512-cmTzND7RMxUB+f7gI+4+KAVHWEg0lfXvQJdko+FXDP5bNbGIdx4KMP5pX6lv5jfT9jSf6OBbjyxjFtZQwYA/ig==
+  dependencies:
+    truncate-utf8-bytes "^1.0.0"
+
 sax@^1.2.4:
   version "1.2.4"
   resolved "https://registry.yarnpkg.com/sax/-/sax-1.2.4.tgz#2816234e2378bddc4e5354fab5caa895df7100d9"
@@ -6889,6 +6901,13 @@ trim-right@^1.0.1:
   resolved "https://registry.yarnpkg.com/trim-right/-/trim-right-1.0.1.tgz#cb2e1203067e0c8de1f614094b9fe45704ea6003"
   integrity sha1-yy4SAwZ+DI3h9hQJS5/kVwTqYAM=
 
+truncate-utf8-bytes@^1.0.0:
+  version "1.0.2"
+  resolved "https://registry.yarnpkg.com/truncate-utf8-bytes/-/truncate-utf8-bytes-1.0.2.tgz#405923909592d56f78a5818434b0b78489ca5f2b"
+  integrity sha1-QFkjkJWS1W94pYGENLC3hInKXys=
+  dependencies:
+    utf8-byte-length "^1.0.1"
+
 ts-jest@^24.0.0:
   version "24.0.2"
   resolved "https://registry.yarnpkg.com/ts-jest/-/ts-jest-24.0.2.tgz#8dde6cece97c31c03e80e474c749753ffd27194d"
@@ -7119,6 +7138,11 @@ use@^3.1.0:
   resolved "https://registry.yarnpkg.com/use/-/use-3.1.1.tgz#d50c8cac79a19fbc20f2911f56eb973f4e10070f"
   integrity sha512-cwESVXlO3url9YWlFW/TA9cshCEhtu7IKJ/p5soJ/gGpj7vbvFrAY/eIioQ6Dw23KjZhYgiIo8HOs1nQ2vr/oQ==
 
+utf8-byte-length@^1.0.1:
+  version "1.0.4"
+  resolved "https://registry.yarnpkg.com/utf8-byte-length/-/utf8-byte-length-1.0.4.tgz#f45f150c4c66eee968186505ab93fcbb8ad6bf61"
+  integrity sha1-9F8VDExm7uloGGUFq5P8u4rWv2E=
+
 util-deprecate@^1.0.1, util-deprecate@~1.0.1:
   version "1.0.2"
   resolved "https://registry.yarnpkg.com/util-deprecate/-/util-deprecate-1.0.2.tgz#450d4dc9fa70de732762fbd2d4a28981419a0ccf"