From 9f33d8503ed17307d86c73dc161bf4636e600744 Mon Sep 17 00:00:00 2001 From: Kieran Sedgwick Date: Mon, 19 May 2014 16:47:24 -0400 Subject: [PATCH] Swapped out XMLHttpRequest for a custom module We made a module to encapsulate the logic that chooses the nodejs or browser dependency that actually downloads a file when the module is used. --- gruntfile.js | 21 ++++++++++- package.json | 11 ++++-- src/network.js | 82 +++++++++++++++++++++++++++++++++++++++++ src/shell/shell.js | 19 +++------- tests/require-config.js | 38 +++++++++++++------ tests/spec/lib.spec.js | 44 ++++++++++++++++++++++ tests/test-manifest.js | 1 + 7 files changed, 185 insertions(+), 31 deletions(-) create mode 100644 src/network.js create mode 100644 tests/spec/lib.spec.js diff --git a/gruntfile.js b/gruntfile.js index a92b69f..b76c968 100644 --- a/gruntfile.js +++ b/gruntfile.js @@ -175,6 +175,21 @@ module.exports = function(grunt) { remote: GIT_REMOTE, branch: 'gh-pages', force: true + }, + } + }, + connect: { + server_for_node: { + options: { + port: 1234, + base: '.' + } + }, + server_for_browser: { + options: { + port: 1234, + base: '.', + keepalive: true } } } @@ -191,6 +206,7 @@ module.exports = function(grunt) { grunt.loadNpmTasks('grunt-git'); grunt.loadNpmTasks('grunt-prompt'); grunt.loadNpmTasks('grunt-shell'); + grunt.loadNpmTasks('grunt-contrib-connect'); grunt.registerTask('develop', ['clean', 'requirejs:develop']); grunt.registerTask('filer-test', ['clean', 'requirejs:test']); @@ -214,7 +230,6 @@ module.exports = function(grunt) { ' to ' + semver.inc(currentVersion, patchLevel).yellow + '?'; grunt.config('prompt.confirm.options', promptOpts); - // TODO: ADD NPM RELEASE grunt.task.run([ 'prompt:confirm', 'checkBranch', @@ -226,7 +241,9 @@ module.exports = function(grunt) { 'npm-publish' ]); }); - grunt.registerTask('test', ['check', 'filer-test', 'shell:mocha']); + grunt.registerTask('test-node', ['check', 'filer-test', 'connect:server_for_node', 'shell:mocha']); + grunt.registerTask('test-browser', ['check', 'filer-test', 'connect:server_for_browser']); + grunt.registerTask('test', ['test-node']); grunt.registerTask('default', ['test']); }; diff --git a/package.json b/package.json index 2eea605..d06dcc2 100644 --- a/package.json +++ b/package.json @@ -25,15 +25,17 @@ "url": "https://github.com/js-platform/filer.git" }, "dependencies": { - "bower": "~1.0.0" + "bower": "~1.0.0", + "request": "^2.36.0" }, "devDependencies": { + "chai": "~1.9.1", "grunt": "~0.4.0", "grunt-bump": "0.0.13", "grunt-contrib-clean": "~0.4.0", "grunt-contrib-compress": "~0.4.1", "grunt-contrib-concat": "~0.1.3", - "grunt-contrib-connect": "~0.7.1", + "grunt-contrib-connect": "^0.7.1", "grunt-contrib-jshint": "~0.7.1", "grunt-contrib-requirejs": "~0.4.0", "grunt-contrib-uglify": "~0.1.2", @@ -47,6 +49,7 @@ "semver": "^2.3.0", "requirejs": "~2.1.11", "mocha": "~1.18.2", - "chai": "~1.9.1" - } + "requirejs": "~2.1.11" + }, + "main": "dist/filer_node.js" } diff --git a/src/network.js b/src/network.js new file mode 100644 index 0000000..2a8d46a --- /dev/null +++ b/src/network.js @@ -0,0 +1,82 @@ +define(function(require) { + // Pull in node's request module if possible/needed + if (typeof XMLHttpRequest === 'undefined') { + // This is a stupid workaround for the fact that + // the r.js optimizer checks every require() call + // during optimization and throws an error if it + // can't find the module. + // + // This is only an issue with our browser build + // using `almond` (https://github.com/jrburke/almond) + // which doesn't fallback to node's require when we + // need it to. + var node_req = require; + var request = node_req('request'); + } + + function browserDownload(uri, callback) { + var query = new XMLHttpRequest(); + query.onload = function() { + var err = query.status != 200 ? { message: query.statusText, code: query.status } : null, + data = err ? null : new Uint8Array(query.response); + + callback(err, data); + }; + query.open("GET", uri); + if("withCredentials" in query) { + query.withCredentials = true; + } + + query.responseType = "arraybuffer"; + query.send(); + } + + function nodeDownload(uri, callback) { + request({ + url: uri, + method: "GET", + encoding: null + }, function(err, msg, body) { + var data = null, + arrayBuffer, + statusCode, + arrayLength = body && body.length, + error; + + msg = msg || null; + statusCode = msg && msg.statusCode; + + error = statusCode != 200 ? { message: err || 'Not found!', code: statusCode } : null; + + arrayBuffer = arrayLength && new ArrayBuffer(arrayLength); + + // Convert buffer to Uint8Array + if (arrayBuffer && (statusCode == 200)) { + data = new Uint8Array(arrayBuffer); + for (var i = 0; i < body.length; ++i) { + data[i] = body[i]; + } + } + + callback(error, data); + }); + } + + return { + download: function(uri, callback) { + if (!uri) { + throw('Uri required!'); + } + + if (!callback) { + throw('Callback required'); + } + + if (typeof XMLHttpRequest === "undefined") { + nodeDownload(uri, callback); + } else { + browserDownload(uri, callback); + } + } + }; +}); diff --git a/src/shell/shell.js b/src/shell/shell.js index 0966fd2..167b1b0 100644 --- a/src/shell/shell.js +++ b/src/shell/shell.js @@ -5,6 +5,7 @@ define(function(require) { var Errors = require('src/errors'); var Environment = require('src/shell/environment'); var async = require('async'); + var Network = require('src/network'); require('zip'); require('unzip'); @@ -447,20 +448,18 @@ define(function(require) { // Grab whatever is after the last / (assuming there is one) and // remove any non-filename type chars(i.e., : and /). Like the real // wget, we leave query string or hash portions in tact. - var path = options.filename || url.replace(/[:/]/g, '').split('/').pop(); + var path = options.filename || url.replace(/[:/]/, '').split('/').pop(); path = Path.resolve(fs.cwd, path); function onerror() { - callback(new Error('unable to get resource')); + callback(new Error('unable to get resource')); } - var request = new XMLHttpRequest(); - request.onload = function() { - if(request.status !== 200) { + Network.download('get', url, function(err, data) { + if (err || !data) { return onerror(); } - var data = new Uint8Array(request.response); fs.writeFile(path, data, function(err) { if(err) { callback(err); @@ -468,15 +467,7 @@ define(function(require) { callback(null, path); } }); - }; - request.onerror = onerror; - request.open("GET", url, true); - if("withCredentials" in request) { - request.withCredentials = true; } - - request.responseType = "arraybuffer"; - request.send(); }; Shell.prototype.unzip = function(zipfile, options, callback) { diff --git a/tests/require-config.js b/tests/require-config.js index 2415177..6fb739c 100644 --- a/tests/require-config.js +++ b/tests/require-config.js @@ -7,7 +7,8 @@ // // ?filer-dist/filer.js --> use dist/filer.js // ?filer-dist/filer.min.js --> use dist/filer.min.js -// ? --> (default) use src/filer.js with require +// ?filer-src/filer.js --> use src/filer.js with require +// ? --> (default) use dist/filer-test.js with require var filerArgs = window.filerArgs = {}; var config = (function() { var query = window.location.search.substring(1); @@ -51,24 +52,39 @@ var config = (function() { } // Support src/ filer via require + if(filerArgs['filer-src/filer.js']) { + return { + paths: { + "tests": "../tests", + "src": "../src", + "spec": "../tests/spec", + "bugs": "../tests/bugs", + "util": "../tests/lib/test-utils", + "Filer": "../src/index" + }, + baseUrl: "../lib", + optimize: "none", + shim: { + // TextEncoder and TextDecoder shims. encoding-indexes must get loaded first, + // and we use a fake one for reduced size, since we only care about utf8. + "encoding": { + deps: ["encoding-indexes-shim"] + } + } + }; + } + + // Support dist/filer-test.js return { paths: { "tests": "../tests", - "src": "../src", "spec": "../tests/spec", "bugs": "../tests/bugs", "util": "../tests/lib/test-utils", - "Filer": "../src/index" + "Filer": "../dist/filer-test" }, baseUrl: "../lib", - optimize: "none", - shim: { - // TextEncoder and TextDecoder shims. encoding-indexes must get loaded first, - // and we use a fake one for reduced size, since we only care about utf8. - "encoding": { - deps: ["encoding-indexes-shim"] - } - } + optimize: "none" }; }()); diff --git a/tests/spec/lib.spec.js b/tests/spec/lib.spec.js new file mode 100644 index 0000000..93b76ee --- /dev/null +++ b/tests/spec/lib.spec.js @@ -0,0 +1,44 @@ +define(['../../src/network'], function(network) { + + describe('Network download tool', function() { + if(typeof XMLHttpRequest === "undefined") { + it('should connect to server',function(done) { + var request = require('request'); + request({ + url: 'http://localhost:8080/package.json', + method: 'GET', + encoding: 'utf-8' + }, function(error, msg, body) { + expect(error).not.to.exist; + done(); + }); + }); + } + + it('should throw an exception when a URI is not passed', function(done) { + expect(function() { + network.download(undefined, function(error, data, statusCode) {}); + }).to.throwError; + done(); + }); + + it('should get an error when a non-existent path is specified', function(done) { + network.download('http://localhost:8080/file-non-existent', function(error, data, statusCode) { + expect(error).not.to.exist; + expect(statusCode).to.eql('404'); + expect(data).to.be.a(null); + done(); + }); + }); + + it('should download a resource from the server', function(done) { + network.download('http://localhost:8080/package.json', function(error, data, statusCode) { + expect(error).not.to.exist; + expect(statusCode).to.eql('200'); + expect(data).to.exist; + expect(data).to.have.length.above(0); + done(); + }); + }); + }); +}); diff --git a/tests/test-manifest.js b/tests/test-manifest.js index 668964b..740643b 100644 --- a/tests/test-manifest.js +++ b/tests/test-manifest.js @@ -38,6 +38,7 @@ define([ "spec/time-flags.spec", "spec/fs.watch.spec", "spec/errors.spec", + "spec/lib.spec", // Filer.FileSystem.providers.* "spec/providers/providers.spec",