From c0a65af16d26eca2770f2abc8c46479086e3985b Mon Sep 17 00:00:00 2001 From: Craig Date: Tue, 22 Nov 2016 16:18:31 -0800 Subject: [PATCH] chore(downloader): replace workflow with a single request (#158) - add method to downloader to getFile - add method to file manager to use new downloader getFile - rewrite updateBinary function to use new file manager method - add a couple of unit tests around downloader getFile - use es6 promises, change travis to use node 6 minimum - remove old method to request file in file manager and downloader --- lib/cmds/update.ts | 47 ++++---- lib/files/downloader.ts | 204 ++++++++++++++------------------- lib/files/file_manager.ts | 77 +++++++------ spec/files/downloader_spec.ts | 49 ++++++++ spec/files/fileManager_spec.ts | 2 + 5 files changed, 199 insertions(+), 180 deletions(-) diff --git a/lib/cmds/update.ts b/lib/cmds/update.ts index 33122dcc..ecce1d9e 100644 --- a/lib/cmds/update.ts +++ b/lib/cmds/update.ts @@ -118,10 +118,8 @@ function update(options: Options): void { // permissions if (standalone) { let binary = binaries[StandAlone.id]; - FileManager.toDownload(binary, outputDir, proxy, ignoreSSL).then((value: boolean) => { - if (value) { - Downloader.downloadBinary(binary, outputDir, proxy, ignoreSSL); - } else { + FileManager.downloadFile(binary, outputDir, proxy, ignoreSSL).then((downloaded: boolean) => { + if (!downloaded) { logger.info( binary.name + ': file exists ' + path.resolve(outputDir, binary.filename(Config.osType(), Config.osArch()))); @@ -131,21 +129,21 @@ function update(options: Options): void { } if (chrome) { let binary = binaries[ChromeDriver.id]; - updateBinary(binary, outputDir, proxy, ignoreSSL).done(); + updateBinary(binary, outputDir, proxy, ignoreSSL); } if (gecko) { let binary = binaries[GeckoDriver.id]; - updateBinary(binary, outputDir, proxy, ignoreSSL).done(); + updateBinary(binary, outputDir, proxy, ignoreSSL); } if (ie) { let binary = binaries[IEDriver.id]; binary.arch = Config.osArch(); // Win32 or x64 - updateBinary(binary, outputDir, proxy, ignoreSSL).done(); + updateBinary(binary, outputDir, proxy, ignoreSSL); } if (ie32) { let binary = binaries[IEDriver.id]; binary.arch = 'Win32'; - updateBinary(binary, outputDir, proxy, ignoreSSL).done(); + updateBinary(binary, outputDir, proxy, ignoreSSL); } if (android) { let binary = binaries[AndroidSDK.id]; @@ -179,27 +177,24 @@ function update(options: Options): void { } } -function updateBinary( - binary: Binary, outputDir: string, proxy: string, ignoreSSL: boolean): q.Promise { - return FileManager.toDownload(binary, outputDir, proxy, ignoreSSL).then((value: boolean) => { - if (value) { - let deferred = q.defer(); - Downloader.downloadBinary( +function updateBinary(binary: Binary, outputDir: string, proxy: string, ignoreSSL: boolean) { + FileManager + .downloadFile( binary, outputDir, proxy, ignoreSSL, (binary: Binary, outputDir: string, fileName: string) => { unzip(binary, outputDir, fileName); - deferred.resolve(); - }); - return deferred.promise; - } else { - logger.info( - binary.name + ': file exists ' + - path.resolve(outputDir, binary.filename(Config.osType(), Config.osArch()))); - let fileName = binary.filename(Config.osType(), Config.osArch()); - unzip(binary, outputDir, fileName); - logger.info(binary.name + ': v' + binary.versionCustom + ' up to date'); - } - }); + }) + .then(downloaded => { + if (!downloaded) { + // The file did not have to download, we should unzip it. + logger.info( + binary.name + ': file exists ' + + path.resolve(outputDir, binary.filename(Config.osType(), Config.osArch()))); + let fileName = binary.filename(Config.osType(), Config.osArch()); + unzip(binary, outputDir, fileName); + logger.info(binary.name + ': v' + binary.versionCustom + ' up to date'); + } + }); } function unzip(binary: T, outputDir: string, fileName: string): void { diff --git a/lib/files/downloader.ts b/lib/files/downloader.ts index 1637e652..f26527b0 100644 --- a/lib/files/downloader.ts +++ b/lib/files/downloader.ts @@ -1,5 +1,4 @@ import * as fs from 'fs'; -import * as http from 'http'; import * as path from 'path'; import * as q from 'q'; import * as request from 'request'; @@ -15,32 +14,6 @@ let logger = new Logger('downloader'); * The file downloader. */ export class Downloader { - /** - * Download the binary file. - * @param binary The binary of interest. - * @param outputDir The directory where files are downloaded and stored. - * @param opt_proxy The proxy for downloading files. - * @param opt_ignoreSSL To ignore SSL. - * @param opt_callback Callback method to be executed after the file is downloaded. - */ - static downloadBinary( - binary: Binary, outputDir: string, opt_proxy?: string, opt_ignoreSSL?: boolean, - opt_callback?: Function): void { - logger.info(binary.name + ': downloading version ' + binary.version()); - var url = binary.url(Config.osType(), Config.osArch()); - if (!url) { - logger.error(binary.name + ' v' + binary.version() + ' is not available for your system.'); - return; - } - Downloader.httpGetFile_( - url, binary.filename(Config.osType(), Config.osArch()), outputDir, opt_proxy, opt_ignoreSSL, - (filePath: string) => { - if (opt_callback) { - opt_callback(binary, outputDir, filePath); - } - }); - } - /** * Resolves proxy based on values set * @param fileUrl The url to download the file. @@ -80,60 +53,29 @@ export class Downloader { return undefined; } - static httpHeadContentLength(fileUrl: string, opt_proxy?: string, opt_ignoreSSL?: boolean): - q.Promise { - let deferred = q.defer(); - if (opt_ignoreSSL) { - logger.info('ignoring SSL certificate'); - } - - let options = { - method: 'GET', - url: fileUrl, - strictSSL: !opt_ignoreSSL, - rejectUnauthorized: !opt_ignoreSSL, - proxy: Downloader.resolveProxy_(fileUrl, opt_proxy) - }; - - request(options).on('response', (response) => { - if (response.headers['Location']) { - let urlLocation = response.headers['Location']; - deferred.resolve(Downloader.httpHeadContentLength(urlLocation, opt_proxy, opt_ignoreSSL)); - } else if (response.headers['content-length']) { - let contentLength = response.headers['content-length']; - deferred.resolve(contentLength); - } - response.destroy(); - }); - return deferred.promise; - } - /** - * Ceates the GET request for the file name. - * @param fileUrl The url to download the file. - * @param fileName The name of the file to download. - * @param opt_proxy The proxy to connect to to download files. - * @param opt_ignoreSSL To ignore SSL. + * Http get the file. Check the content length of the file before writing the file. + * If the content length does not match, remove it and download the file. + * + * @param binary The binary of interest. + * @param fileName The file name. + * @param outputDir The directory where files are downloaded and stored. + * @param contentLength The content length of the existing file. + * @param opt_proxy The proxy for downloading files. + * @param opt_ignoreSSL Should the downloader ignore SSL. + * @param opt_callback Callback method to be executed after the file is downloaded. + * @returns Promise Resolves true = downloaded. Resolves false = not downloaded. + * Rejected with an error. */ - static httpGetFile_( - fileUrl: string, fileName: string, outputDir: string, opt_proxy?: string, - opt_ignoreSSL?: boolean, callback?: Function): void { - logger.info('curl -o ' + outputDir + '/' + fileName + ' ' + fileUrl); + static getFile( + binary: Binary, fileUrl: string, fileName: string, outputDir: string, contentLength: number, + opt_proxy?: string, opt_ignoreSSL?: boolean, callback?: Function): Promise { + // logger.info('curl -o ' + outputDir + '/' + fileName + ' ' + fileUrl); + // let contentLength = 0; let filePath = path.resolve(outputDir, fileName); - let file = fs.createWriteStream(filePath); - let contentLength = 0; - - interface Options { - url: string; - timeout: number; - strictSSL?: boolean; - rejectUnauthorized?: boolean; - proxy?: string; - headers?: {[key: string]: any}; - [key: string]: any; - } + let file: any; - let options: Options = { + let options: IOptions = { url: fileUrl, // default Linux can be anywhere from 20-120 seconds // increasing this arbitrarily to 4 minutes @@ -153,46 +95,72 @@ export class Downloader { } } - request(options) - .on('response', - (response) => { - if (response.statusCode !== 200) { - fs.unlinkSync(filePath); - logger.error('Error: Got code ' + response.statusCode + ' from ' + fileUrl); - } - contentLength = response.headers['content-length']; - }) - .on('error', - (error) => { - if ((error as any).code === 'ETIMEDOUT') { - logger.error('Connection timeout downloading: ' + fileUrl); - logger.error('Default timeout is 4 minutes.'); - - } else if ((error as any).connect) { - logger.error('Could not connect to the server to download: ' + fileUrl); - } - logger.error(error); - fs.unlinkSync(filePath); - }) - .pipe(file); - - file.on('close', function() { - fs.stat(filePath, function(err, stats) { - if (err) { - logger.error('Error: Got error ' + err + ' from ' + fileUrl); - return; - } - if (stats.size != contentLength) { - logger.error( - 'Error: corrupt download for ' + fileName + - '. Please re-run webdriver-manager update'); - fs.unlinkSync(filePath); - return; - } - if (callback) { - callback(filePath); - } - }); - }); + let req: request.Request = null; + let resContentLength: number; + + return new Promise((resolve, reject) => { + req = request(options); + req.on('response', response => { + if (response.statusCode === 200) { + resContentLength = +response.headers['content-length']; + if (contentLength === resContentLength) { + // if the size is the same, do not download and stop here + response.destroy(); + resolve(false); + } else { + // only pipe if the headers are different length + file = fs.createWriteStream(filePath); + req.pipe(file); + file.on('close', () => { + fs.stat(filePath, (error, stats) => { + if (error) { + (error as any).msg = 'Error: Got error ' + error + ' from ' + fileUrl; + reject(error); + } + if (stats.size != resContentLength) { + (error as any).msg = 'Error: corrupt download for ' + fileName + + '. Please re-run webdriver-manager update'; + fs.unlinkSync(filePath); + reject(error); + } + if (callback) { + callback(binary, outputDir, fileName); + } + }); + resolve(true); + }); + } + + } else { + let error = new Error(); + (error as any).msg = + 'Expected response code 200, received: ' + response.statusCode; + reject(error); + } + }); + req.on('error', error => { + if ((error as any).code === 'ETIMEDOUT') { + (error as any).msg = 'Connection timeout downloading: ' + fileUrl + + '. Default timeout is 4 minutes.'; + } else if ((error as any).connect) { + (error as any).msg = 'Could not connect to the server to download: ' + fileUrl; + } + reject(error); + }); + }) + .catch(error => { + logger.error((error as any).msg); + }); } } + +interface IOptions { + method?: string; + url: string; + timeout: number; + strictSSL?: boolean; + rejectUnauthorized?: boolean; + proxy?: string; + headers?: {[key: string]: any}; + [key: string]: any; +} diff --git a/lib/files/file_manager.ts b/lib/files/file_manager.ts index a6e123e5..17fee372 100644 --- a/lib/files/file_manager.ts +++ b/lib/files/file_manager.ts @@ -161,49 +161,54 @@ export class FileManager { } /** - * Check to see if the binary version should be downloaded. + * Try to download the binary version. * @param binary The binary of interest. * @param outputDir The directory where files are downloaded and stored. - * @returns If the file should be downloaded. + * @returns Promise resolved to true for files downloaded, resolved to false for files not + * downloaded because they exist, rejected if there is an error. */ - static toDownload( - binary: T, outputDir: string, proxy: string, ignoreSSL: boolean): q.Promise { - let osType = Config.osType(); - let osArch = Config.osArch(); - let filePath: string; - let readData: Buffer; - let deferred = q.defer(); - let downloaded: BinaryMap = FileManager.downloadedBinaries(outputDir); + static downloadFile( + binary: T, outputDir: string, opt_proxy?: string, opt_ignoreSSL?: boolean, + callback?: Function): Promise { + return new Promise((resolve, reject) => { + let filePath = path.resolve(outputDir, binary.filename(Config.osType(), Config.osArch())); + let fileUrl = binary.url(Config.osType(), Config.osArch()); + let fileName = binary.filename(Config.osType(), Config.osArch()); + let outDir = Config.getSeleniumDir(); + let downloaded: BinaryMap = FileManager.downloadedBinaries(outputDir); + let contentLength = 0; - if (downloaded[binary.id()]) { - let downloadedBinary = downloaded[binary.id()]; - let versions = downloadedBinary.versions; - let version = binary.version(); - for (let index in versions) { - let v = versions[index]; - if (v === version) { - filePath = path.resolve(outputDir, binary.filename(osType, osArch)); - readData = fs.readFileSync(filePath); + // If we have downloaded the file before, check the content length + if (downloaded[binary.id()]) { + let downloadedBinary = downloaded[binary.id()]; + let versions = downloadedBinary.versions; + let version = binary.version(); - // we have the version, verify it is the correct file size - let contentLength = - Downloader.httpHeadContentLength(binary.url(osType, osArch), proxy, ignoreSSL); - return contentLength.then((value: any): boolean => { - if (value == readData.length) { - return false; - } else { - logger.warn( - path.basename(filePath) + ' expected length ' + value + ', found ' + - readData.length); - logger.warn('removing file: ' + filePath); - return true; - } - }); + for (let index in versions) { + let v = versions[index]; + if (v === version) { + contentLength = fs.statSync(filePath).size; + Downloader + .getFile( + binary, fileUrl, fileName, outputDir, contentLength, opt_proxy, opt_ignoreSSL, + callback) + .then(downloaded => { + resolve(downloaded); + }); + } } + } else { + // We have not downloaded it before, or the version does not exist. Use the default content + // length of zero and download the file. + Downloader + .getFile( + binary, fileUrl, fileName, outputDir, contentLength, opt_proxy, opt_ignoreSSL, + callback) + .then(downloaded => { + resolve(downloaded); + }); } - } - deferred.resolve(true); - return deferred.promise; + }); } /** diff --git a/spec/files/downloader_spec.ts b/spec/files/downloader_spec.ts index 9523c36c..239bf66c 100644 --- a/spec/files/downloader_spec.ts +++ b/spec/files/downloader_spec.ts @@ -1,3 +1,7 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import * as rimraf from 'rimraf'; + import {Config} from '../../lib/config'; import {Downloader} from '../../lib/files'; @@ -73,4 +77,49 @@ describe('downloader', () => { }); }); }); + + describe('get file', () => { + let fileUrl = + 'https://selenium-release.storage.googleapis.com/3.0/selenium-server-standalone-3.0.0.jar'; + let fileName = 'foobar.jar'; + let outputDir = Config.getSeleniumDir(); + let actualContentLength = 22138949; + let contentLength: number; + + it('should download a file with mismatch content length', (done) => { + contentLength = 0; + Downloader.getFile(null, fileUrl, fileName, outputDir, contentLength) + .then(result => { + expect(result).toBeTruthy(); + let file = path.resolve(outputDir, fileName); + let stat = fs.statSync(file); + expect(stat.size).toEqual(actualContentLength); + rimraf.sync(file); + done(); + }) + .catch(error => { + console.log(error); + done.fail(); + }); + }); + + it('should not download a file if the content lengths match', (done) => { + contentLength = actualContentLength; + Downloader.getFile(null, fileUrl, fileName, outputDir, contentLength) + .then(result => { + expect(result).not.toBeTruthy(); + let file = path.resolve(outputDir, fileName); + try { + let access = fs.accessSync(file); + } catch (err) { + (err as any).code === 'ENOENT' + } + done(); + }) + .catch(error => { + console.log(error); + done.fail(); + }); + }); + }); }); diff --git a/spec/files/fileManager_spec.ts b/spec/files/fileManager_spec.ts index 1d63ede8..885b9580 100644 --- a/spec/files/fileManager_spec.ts +++ b/spec/files/fileManager_spec.ts @@ -265,4 +265,6 @@ describe('file manager', () => { } } }); + + // TODO(cnishina): download binaries for each os type / arch combination });