Skip to content

Commit

Permalink
fix: make the cache to respect an artifact url (#113)
Browse files Browse the repository at this point in the history
alexeykuzmin authored and MarshallOfSound committed Aug 26, 2019

Verified

This commit was signed with the committer’s verified signature.
SergeyKopienko Sergey Kopienko
1 parent 6d392f7 commit 5661538
Showing 8 changed files with 68 additions and 64 deletions.
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
@@ -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",
14 changes: 8 additions & 6 deletions src/Cache.ts
Original file line number Diff line number Diff line change
@@ -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');
24 changes: 6 additions & 18 deletions src/artifact-utils.ts
Original file line number Diff line number Diff line change
@@ -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}`;
}
15 changes: 6 additions & 9 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -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);
});
}
32 changes: 18 additions & 14 deletions test/Cache.spec.ts
Original file line number Diff line number Diff line change
@@ -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');
});
19 changes: 3 additions & 16 deletions test/artifact-utils.spec.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
2 changes: 1 addition & 1 deletion test/index.spec.ts
Original file line number Diff line number Diff line change
@@ -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');
});

24 changes: 24 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
@@ -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"

0 comments on commit 5661538

Please sign in to comment.