diff --git a/src/dapi/http/HttpDataApi.js b/src/dapi/http/HttpDataApi.js index 789bfc0..79cf839 100644 --- a/src/dapi/http/HttpDataApi.js +++ b/src/dapi/http/HttpDataApi.js @@ -1,7 +1,7 @@ /** * Data transmission over HTTP(S) * - * Copyright (C) 2014 LoVullo Associates, Inc. + * Copyright (C) 2014, 2015 LoVullo Associates, Inc. * * This file is part of the Liza Data Collection Framework * @@ -69,6 +69,8 @@ module.exports = Class( 'HttpDataApi' ) * requests, which permits the user to use whatever implementation works * well with their existing system. * + * TODO: Accept URI encoder. + * * @param {string} url destination URL * @param {string} method RFC-2616-compliant HTTP method * @param {HttpImpl} impl HTTP implementation @@ -91,13 +93,19 @@ module.exports = Class( 'HttpDataApi' ) /** * Perform an asynchronous request and invoke the callback with the reply * + * DATA must be either a string or an object; the latter is treated as a + * key-value parameter list, which will have each key and value + * individually URI-encoded and converted into a string, delimited by + * ampersands. + * * In the event of an error, the first parameter is the error; otherwise, it * is null. The return data shall not be used in the event of an error. * * The return value shall be a raw string; conversion to other formats must * be handled by a wrapper. * - * @param {string} data binary data to transmit + * @param {?Object|string} data request params or post data + * * @param {function(?Error,*):string} callback continuation upon reply * * @return {DataApi} self @@ -109,7 +117,10 @@ module.exports = Class( 'HttpDataApi' ) this._validateDataType( data ); this._impl.requestData( - this._url, this._method, data, callback + this._url, + this._method, + this._encodeData( data ), + callback ); return this; @@ -156,5 +167,55 @@ module.exports = Class( 'HttpDataApi' ) "key-value params" ); } - } + }, + + + /** + * If the data are an object, it's converted to an encoded key-value + * URI; otherwise, the original string datum is returned. + * + * @param {?Object|string=} data raw data or key-value + * + * @return {string} encoded data + */ + 'private _encodeData': function( data ) + { + if ( typeof data !== 'object' ) + { + return ''+data; + } + + return this._encodeKeys( data ); + }, + + + /** + * Generate params for URI from key-value DATA + * + * @param {Object} data key-value request params + * + * @return {string} generated URI, or empty if no keys + */ + 'private _encodeKeys': function( obj ) + { + var uri = ''; + + // ES3 support + for ( var key in obj ) + { + if ( !Object.prototype.hasOwnProperty.call( obj, key ) ) + { + continue; + } + + uri += ( uri ) + ? '&' + : ''; + + uri += encodeURIComponent( key ) + '=' + + encodeURIComponent( obj[ key ] ); + } + + return uri; + }, } ); diff --git a/src/dapi/http/XhrHttpImpl.js b/src/dapi/http/XhrHttpImpl.js index ea4cc9f..622b5c7 100644 --- a/src/dapi/http/XhrHttpImpl.js +++ b/src/dapi/http/XhrHttpImpl.js @@ -42,8 +42,6 @@ module.exports = Class( 'XhrHttpImpl' ) * Initializes with constructor to the object through which XHRs will be * made * - * TODO: Accept URI encoders - * * @param {Object} XMLHttpRequest ctor to object to perform requests */ __construct: function( XMLHttpRequest ) @@ -58,14 +56,13 @@ module.exports = Class( 'XhrHttpImpl' ) * If METHOD is `"GET"`, the data will be appended to the URL; * otherwise, the URL remains unchanged. * - * If DATA is an object, its keys will be encoded and added to the URL - * an in undefined order. + * No additional encoding is preformed on DATA; that is assumed to have + * already been performed. * * @param {string} url base request URL * @param {string} method RFC-2616-compliant HTTP method * - * @param {?Object|string=} data request params or - * post data + * @param {string} data request data * * @param {function(Error, Object)} callback server response callback * @@ -73,6 +70,13 @@ module.exports = Class( 'XhrHttpImpl' ) */ 'public requestData': function( url, method, data, callback ) { + if ( typeof data !== 'string' ) + { + throw TypeError( + "Request data must be a string; " + typeof data + " given" + ); + } + var req = new this._Xhr(), url = this._genUrl( url, method, data ); @@ -115,58 +119,14 @@ module.exports = Class( 'XhrHttpImpl' ) return url; } - var encoded; - - // TODO: reject nonsense types, including arrays - switch ( typeof data ) - { - case 'object': - encoded = this._encodeKeys( data ); - break; - - default: - encoded = encodeURI( data ); - break; - } - return url + - ( ( encoded ) - ? ( '?' + encoded ) + ( ( data ) + ? ( '?' + data ) : '' ); }, - /** - * Generate params for URI from key-value DATA - * - * @param {?Object|string=} data key-value request params - * - * @return {string} generated URI, or empty if no keys - */ - 'private _encodeKeys': function( obj ) - { - var uri = ''; - - // ES3 support - for ( var key in obj ) - { - if ( !Object.prototype.hasOwnProperty.call( obj, key ) ) - { - continue; - } - - uri += ( uri ) - ? '&' - : ''; - - uri += key + '=' + encodeURIComponent( obj[ key ] ); - } - - return uri; - }, - - /** * Determine what DATA to post to the server * diff --git a/test/dapi/http/HttpDataApiTest.js b/test/dapi/http/HttpDataApiTest.js index 748912e..c1a7dac 100644 --- a/test/dapi/http/HttpDataApiTest.js +++ b/test/dapi/http/HttpDataApiTest.js @@ -1,7 +1,7 @@ /** * Test case for data transmission over HTTP(S) * - * Copyright (C) 2014 LoVullo Associates, Inc. + * Copyright (C) 2014, 2015 LoVullo Associates, Inc. * * This file is part of the Liza Data Collection Framework * @@ -100,7 +100,7 @@ describe( 'HttpDataApi', function() it( 'delegates to provided HTTP implementation', function() { var method = 'POST', - data = {}, + data = "ribbit", c = function() {}; Sut( dummy_url, method, impl ).request( data, c ); @@ -113,6 +113,41 @@ describe( 'HttpDataApi', function() } ); + /** + * It's nice to do this for the HttpImpl so that they don't have to + * worry about the proper way to handle it, or duplicate the logic. + */ + describe( 'given key-value data', function() + { + it( 'converts data into encoded string', function() + { + var method = 'POST', + data = { foo: "bar=baz", '&bar': "moo%cow" }, + c = function() {}; + + Sut( dummy_url, method, impl ).request( data, c ); + + expect( impl.provided[ 2 ] ).to.equal( + 'foo=' + encodeURIComponent( data.foo ) + + '&' + encodeURIComponent( '&bar' ) + '=' + + encodeURIComponent( data[ '&bar' ] ) + ); + } ); + + + it( 'with no keys, results in empty string', function() + { + var method = 'POST', + data = {}, + c = function() {}; + + Sut( dummy_url, method, impl ).request( data, c ); + + expect( impl.provided[ 2 ] ).to.equal( "" ); + } ); + } ); + + /** * Method chaining */ diff --git a/test/dapi/http/XhrHttpImplTest.js b/test/dapi/http/XhrHttpImplTest.js index 0a6e780..ffa5ca2 100644 --- a/test/dapi/http/XhrHttpImplTest.js +++ b/test/dapi/http/XhrHttpImplTest.js @@ -31,9 +31,7 @@ var dapi = require( '../../../' ).dapi, { DummyXhr.args = arguments; }; - }, - - _void = function() {}; + }; describe( 'XhrHttpImpl', function() @@ -65,7 +63,7 @@ describe( 'XhrHttpImpl', function() var method = 'GET', sut = Sut( DummyXhr ); - sut.requestData( 'http://foo', method, {}, function() {} ); + sut.requestData( 'http://foo', method, "", function() {} ); var args = DummyXhr.args; expect( args[ 0 ] ).to.equal( method ); @@ -93,7 +91,7 @@ describe( 'XhrHttpImpl', function() { // should instead provide to callback Sut( Xhr ) - .requestData( 'http://foo', 'GET', {}, function( err, data ) + .requestData( 'http://foo', 'GET', "", function( err, data ) { expect( err ).to.equal( e ); expect( data ).to.equal( null ); @@ -113,7 +111,7 @@ describe( 'XhrHttpImpl', function() StubXhr.prototype.status = 200; // OK Sut( StubXhr ) - .requestData( 'http://bar', 'GET', {}, function( err, resp ) + .requestData( 'http://bar', 'GET', "", function( err, resp ) { expect( err ).to.equal( null ); expect( resp ).to.equal( retdata ); @@ -124,10 +122,10 @@ describe( 'XhrHttpImpl', function() describe( 'HTTP method is GET', function() { - it( 'appends encoded non-obj data to URL', function( done ) + it( 'appends data to URL', function( done ) { var url = 'http://bar', - src = "moocow %foocow%", + src = "moocow%foocow%", StubXhr = createStubXhr(); StubXhr.prototype.readyState = 4; // done @@ -135,8 +133,10 @@ describe( 'XhrHttpImpl', function() StubXhr.prototype.open = function( _, given_url ) { + // no additional encoding should be performed; it's + // assumed to have already been done expect( given_url ).to.equal( - url + '?' + encodeURI( src ) + url + '?' + src ); }; @@ -152,39 +152,7 @@ describe( 'XhrHttpImpl', function() } ); - it( 'appends encoded key-val data to URL', function( done ) - { - var url = 'http://bar', - src = { foo: "bar=baz", bar: "moo%cow" }, - StubXhr = createStubXhr(); - - StubXhr.prototype.readyState = 4; // done - StubXhr.prototype.status = 200; // OK - - StubXhr.prototype.open = function( _, given_url ) - { - // XXX: the docblock for requestData says "undefined - // order", but fundamentally we need to pass in our own - // encoder - expect( given_url ).to.equal( - url + '?foo=' + encodeURIComponent( src.foo ) + - '&bar=' + encodeURIComponent( src.bar ) - ); - }; - - StubXhr.prototype.send = function( data ) - { - // no posting on GET - expect( data ).is.equal( undefined ); - StubXhr.inst.onreadystatechange(); - }; - - Sut( StubXhr ) - .requestData( url, 'GET', src, done ); - } ); - - - it( 'leaves URL unaltered with empty data', function( done ) + it( 'leaves URL unaltered when data is empty', function( done ) { var url = 'http://bar', StubXhr = createStubXhr(); @@ -199,10 +167,7 @@ describe( 'XhrHttpImpl', function() }; Sut( StubXhr ) - .requestData( url, 'GET', undefined, _void ) - .requestData( url, 'GET', null, _void ) - .requestData( url, 'GET', "", _void ) - .requestData( url, 'GET', {}, done ); + .requestData( url, 'GET', "", done ); } ); } ); @@ -210,7 +175,7 @@ describe( 'XhrHttpImpl', function() describe( 'HTTP method is not GET', function() { - it( 'posts non-object data verbatim', function( done ) + it( 'sends data verbatim', function( done ) { var url = 'http://bar', src = "moocow", @@ -234,39 +199,6 @@ describe( 'XhrHttpImpl', function() Sut( StubXhr ) .requestData( url, 'POST', src, done ); } ); - - - it( 'encodes key-value data', function( done ) - { - var url = 'http://bar', - src = { foo: "bar=baz", bar: "moo%cow" }, - StubXhr = createStubXhr(); - - StubXhr.prototype.readyState = 4; // done - StubXhr.prototype.status = 200; // OK - - StubXhr.prototype.open = function( _, given_url ) - { - // unaltered - expect( given_url ).to.equal( url ); - }; - - StubXhr.prototype.send = function( data ) - { - // XXX: the docblock for requestData says "undefined - // order", but fundamentally we need to pass in our own - // encoder - expect( data ).to.equal( - 'foo=' + encodeURIComponent( src.foo ) + - '&bar=' + encodeURIComponent( src.bar ) - ); - - StubXhr.inst.onreadystatechange(); - }; - - Sut( StubXhr ) - .requestData( url, 'POST', src, done ); - } ); } ); @@ -388,7 +320,7 @@ describe( 'XhrHttpImpl', function() { var sut = Sut( function() {} ), ret = sut.requestData( - 'http://foo', 'GET', {}, function() {} + 'http://foo', 'GET', "", function() {} ); expect( ret ).to.equal( sut );