From 67c2f04e9603d6e7fca07af86f68f0be88aaa4ee Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 12 May 2015 13:41:52 -0400 Subject: [PATCH 01/22] nullglob for gen-index --- tools/gen-index | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/gen-index b/tools/gen-index index 7d78470..9c97ef8 100755 --- a/tools/gen-index +++ b/tools/gen-index @@ -19,7 +19,7 @@ # along with this program. If not, see . ## -shopt -s extglob +shopt -s extglob nullglob destpath="${1?Destination path required}" From e0f1c600ba89e4151997760ed5e4b6e9559c953d Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 18 Apr 2014 10:40:09 -0400 Subject: [PATCH 02/22] Began liberating DataApi The Data API is being heavily refactored and tested; while moving files into the Liza repository, I noticed some inefficiencies with the design. These are being heavily improved upon. --- src/dapi/DataApi.js | 51 +++++++++ src/dapi/http/HttpDataApi.js | 160 ++++++++++++++++++++++++++ src/dapi/http/HttpImpl.js | 51 +++++++++ test/dapi/http/HttpDataApiTest.js | 179 ++++++++++++++++++++++++++++++ 4 files changed, 441 insertions(+) create mode 100644 src/dapi/DataApi.js create mode 100644 src/dapi/http/HttpDataApi.js create mode 100644 src/dapi/http/HttpImpl.js create mode 100644 test/dapi/http/HttpDataApiTest.js diff --git a/src/dapi/DataApi.js b/src/dapi/DataApi.js new file mode 100644 index 0000000..b1bc59d --- /dev/null +++ b/src/dapi/DataApi.js @@ -0,0 +1,51 @@ +/** + * Generic interface for data transmission + * + * Copyright (C) 2014 LoVullo Associates, Inc. + * + * This file is part of the Liza Data Collection Framework + * + * Liza is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +var Interface = require( 'easejs' ).Interface; + + +/** + * Provies a generic interface for data transmission. The only assumption that a + * user of this API shall make is that data may be sent and received in some + * arbitrary, implementation-defined format, and that every request for data + * shall yield some sort of response via a callback. + */ +module.exports = Interface( 'DataApi', +{ + /** + * Perform an asynchronous request and invoke the callback with the reply + * + * If an implementation is synchronous, the callback must still be invoked. + * + * The data format is implementation-defined. The data parameter is + * documented as binary as it is the most permissive, but any data may be + * transferred that is supported by the protocol. + * + * The first parameter of the callback shall contain an Error in the event + * of a failure; otherwise, it shall be null. + * + * @param {string} data binary data to transmit + * @param {function(?Error,*)} callback continuation upon reply + * + * @return {DataApi} self + */ + 'public request': [ 'data', 'callback' ] +} ); diff --git a/src/dapi/http/HttpDataApi.js b/src/dapi/http/HttpDataApi.js new file mode 100644 index 0000000..fed8bb2 --- /dev/null +++ b/src/dapi/http/HttpDataApi.js @@ -0,0 +1,160 @@ +/** + * Data transmission over HTTP(S) + * + * Copyright (C) 2014 LoVullo Associates, Inc. + * + * This file is part of the Liza Data Collection Framework + * + * Liza is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +var Class = require( 'easejs' ).Class, + DataApi = require( '../DataApi' ), + HttpImpl = require( './HttpImpl' ), + + // RFC 2616 methods + rfcmethods = { + DELETE: true, + GET: true, + HEAD: true, + OPTIONS: true, + POST: true, + PUT: true, + TRACE: true + }; + + +/** + * HTTP request abstraction. Does minor validation, but delegates to a specific + * HTTP implementation for the actual request. + */ +module.exports = Class( 'HttpDataApi' ) + .implement( DataApi ) + .extend( +{ + /** + * Request URL + * @type {string} + */ + 'private _url': '', + + /** + * HTTP method + * @type {string} + */ + 'private _method': '', + + /** + * HTTP implementation to perfom request + * @type {HttpImpl} + */ + 'private _impl': null, + + + /** + * Initialize Data API with destination and HTTP implementation + * + * The supplied HTTP implementation will be used to perform the HTTP + * requests, which permits the user to use whatever implementation works + * well with their existing system. + * + * @param {string} url destination URL + * @param {string} method RFC-2616-compliant HTTP method + * @param {HttpImpl} impl HTTP implementation + * + * @throws {TypeError} when non-HttpImpl is provided + */ + __construct: function( url, method, impl ) + { + if ( !( Class.isA( HttpImpl, impl ) ) ) + { + throw TypeError( "Expected HttpImpl" ); + } + + this._url = ''+url; + this._method = this._validateMethod( method ); + this._impl = impl; + }, + + + /** + * Perform an asynchronous request and invoke the callback with the reply + * + * 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 {function(?Error,*):string} callback continuation upon reply + * + * @return {DataApi} self + * + * @throws {TypeError} on validation failure + */ + 'public request': function( data, callback ) + { + this._validateDataType( data ); + + this._impl.requestData( + this._url, this._method, data, callback + ); + + return this; + }, + + + /** + * Ensures that the provided method conforms to RFC 2616 + * + * @param {string} method HTTP method + * @return {string} provided method + * + * @throws {Error} on non-conforming method + */ + 'private _validateMethod': function( method ) + { + if ( !( rfcmethods[ method ] ) ) + { + throw Error( "Invalid RFC 2616 method: " + method ); + } + + return method; + }, + + + /** + * Validates that the provided data type is accepted by the Data API + * + * @param {*} data data to validate + * @return {undefined} + * + * @throws {TypeError} on validation failure + */ + 'private _validateDataType': function( data ) + { + var type = typeof data; + + if ( ( data === null ) + || !( ( type === 'string' ) || ( type === 'object' ) ) + ) + { + throw TypeError( + "Data must be a string of raw data or object containing " + + "key-value params" + ); + } + } +} ); diff --git a/src/dapi/http/HttpImpl.js b/src/dapi/http/HttpImpl.js new file mode 100644 index 0000000..dd214d5 --- /dev/null +++ b/src/dapi/http/HttpImpl.js @@ -0,0 +1,51 @@ +/** + * HTTP protocol implementation + * + * Copyright (C) 2014 LoVullo Associates, Inc. + * + * This file is part of the Liza Data Collection Framework + * + * Liza is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +var Interface = require( 'easejs' ).Interface; + + +/** + * HTTP protocol implementation that will perform the actual transfer. This + * abstraction allows use of whatever library the user prefers (e.g. + * XMLHttpRequest, jQuery, etc). + */ +module.exports = Interface( 'HttpImpl', +{ + /** + * Perform HTTP request + * + * If the request is synchronous, it must still return the data via the + * provided callback. The provided data is expected to be key-value if an + * object is given, otherwise a string of binary data. + * + * An implementation is not required to implement every HTTP method, + * although that is certainly preferred; a user of the API is expected to + * know when an implementation does not support a given method. + * + * @param {string} url destination URL + * @param {string} method RFC-2616-compliant HTTP method + * @param {Object|string} data request params + * @param {function(Error, Object)} callback server response callback + * + * @return {HttpImpl} self + */ + 'public requestData': [ 'url', 'method', 'data', 'callback' ] +} ); diff --git a/test/dapi/http/HttpDataApiTest.js b/test/dapi/http/HttpDataApiTest.js new file mode 100644 index 0000000..eeeb7d6 --- /dev/null +++ b/test/dapi/http/HttpDataApiTest.js @@ -0,0 +1,179 @@ +/** + * Test case for data transmission over HTTP(S) + * + * Copyright (C) 2014 LoVullo Associates, Inc. + * + * This file is part of the Liza Data Collection Framework + * + * Liza is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +var dapi = require( '../../../' ).dapi, + expect = require( 'chai' ).expect, + Class = require( 'easejs' ).Class, + Sut = dapi.http.HttpDataApi, + + dummy_url = 'http://foo', + dummy_impl = Class + .implement( dapi.http.HttpImpl ) + .extend( { requestData: function( _, __, ___, ____ ) {} } ), + + dummy_sut = Sut( dummy_url, 'GET', dummy_impl ); + + +describe( 'HttpDataApi', function() +{ + it( 'is a DataApi', function() + { + expect( Class.isA( dapi.DataApi, dummy_sut ) ).to.be.ok; + } ); + + + it( 'permits RFC 2616 HTTP methods', function() + { + var m = [ 'GET', 'POST', 'PUT', 'HEAD', 'OPTIONS', 'DELETE', 'TRACE' ]; + + m.forEach( function( method ) + { + expect( function() + { + Sut( dummy_url, method, dummy_impl ); + } ).to.not.throw( Error ); + } ); + } ); + + + it( 'does not permit non-RFC-2616 HTTP methods', function() + { + expect( function() + { + Sut( dummy_url, 'FOO', dummy_impl ); + } ).to.throw( Error, 'FOO' ); + } ); + + + it( 'rejects non-HttpImpl objects', function() + { + expect( function() + { + Sut( dummy_url, 'GET', {} ); + } ).to.throw( TypeError, 'HttpImpl' ); + } ); + + + describe( '.request', function() + { + var impl = Class( 'StubHttpImpl' ) + .implement( dapi.http.HttpImpl ) + .extend( + { + provided: [], + data: "", + err: null, + + requestData: function( url, method, data, c ) + { + this.provided = arguments; + c( this.err, this.data ); + } + } )(); + + + /** + * The actual request is performed by some underling implementation. + * This additional level of indirection allows the general concept of an + * "HTTP Data API" to vary from an underyling HTTP protocol + * implementation; they are separate concerns, although the distinction + * may seem subtle. + */ + it( 'delegates to provided HTTP implementation', function() + { + var method = 'POST', + data = {}, + c = function() {}; + + Sut( dummy_url, method, impl ).request( data, c ); + + var provided = impl.provided; + expect( provided[ 0 ] ).to.equal( dummy_url ); + expect( provided[ 1 ] ).to.equal( method ); + expect( provided[ 2 ] ).to.equal( data ); + expect( provided[ 3 ] ).to.equal( c ); + } ); + + + /** + * Method chaining + */ + it( 'returns self', function() + { + var sut = Sut( dummy_url, 'GET', impl ), + ret = sut.request( "", function() {} ); + + expect( ret ).to.equal( sut ); + } ); + + + /** + * String requests are intended to be raw messages, whereas objects are + * treated as key-value params. + */ + it( 'accepts string and object data', function() + { + expect( function() + { + Sut( dummy_url, 'GET', impl ) + .request( "", function() {} ) // string + .request( {}, function() {} ); // object + } ).to.not.throw( Error ); + } ); + + + it( 'rejects all other data types', function() + { + var sut = Sut( dummy_url, 'GET', impl ); + + [ 123, null, Infinity, undefined, NaN, function() {} ] + .forEach( function( data ) + { + expect( function() + { + sut.request( data, function() {} ); + } ).to.throw( TypeError ); + } ); + } ); + + + it( 'returns error provided by HTTP implementation', function( done ) + { + impl.err = Error( "Test impl error" ); + Sut( dummy_url, 'GET', impl ).request( "", function( err, resp ) + { + expect( err ).to.equal( impl.err ); + done(); + } ); + } ); + + + it( 'returns response provided by HTTP implementation', function( done ) + { + impl.data = {}; + Sut( dummy_url, 'GET', impl ).request( "", function( err, resp ) + { + expect( resp ).to.equal( impl.data ); + done(); + } ); + } ); + } ); +} ); From 6d0acf591664a15ddf35b083b9beec63fff0f30b Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 21 Apr 2014 16:14:28 -0400 Subject: [PATCH 03/22] Initial XhrHttpImpl implementation --- src/dapi/http/XhrHttpImpl.js | 145 ++++++++++++++++++++ test/dapi/http/HttpDataApiTest.js | 2 +- test/dapi/http/XhrHttpImplTest.js | 219 ++++++++++++++++++++++++++++++ 3 files changed, 365 insertions(+), 1 deletion(-) create mode 100644 src/dapi/http/XhrHttpImpl.js create mode 100644 test/dapi/http/XhrHttpImplTest.js diff --git a/src/dapi/http/XhrHttpImpl.js b/src/dapi/http/XhrHttpImpl.js new file mode 100644 index 0000000..ac9c6c7 --- /dev/null +++ b/src/dapi/http/XhrHttpImpl.js @@ -0,0 +1,145 @@ +/** + * XMLHttpRequest HTTP protocol implementation + * + * Copyright (C) 2014 LoVullo Associates, Inc. + * + * This file is part of the Liza Data Collection Framework + * + * Liza is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +var Class = require( 'easejs' ).Class, + HttpImpl = require( './HttpImpl' ); + + +/** + * An HTTP implementation using the standardized XMLHttpRequest prototype. + */ +module.exports = Class( 'XhrHttpImpl' ) + .implement( HttpImpl ) + .extend( +{ + /** + * XMLHttpRequest constructor + * @type {XMLHttpRequest} + * @constructor + */ + 'private _Xhr': null, + + + /** + * Initializes with constructor to the object through which XHRs will be + * made + * + * @param {Object} XMLHttpRequest ctor to object to perform requests + */ + __construct: function( XMLHttpRequest ) + { + this._Xhr = XMLHttpRequest; + }, + + + /** + * Perform HTTP request using the standard XMLHttpRequest + * + * @param {Object|string} data request params + * @param {function(Error, Object)} callback server response callback + * + * @return {HttpImpl} self + */ + 'public requestData': function( url, method, data, callback ) + { + var req = new this._Xhr(); + + try + { + this.openRequest( req, url, method ); + this.onLoad( req, function( err, resp ) + { + callback( err, resp ); + } ); + } + catch ( e ) + { + callback( e, null ); + } + + return this; + }, + + + /** + * Prepares a request to the given URL using the given HTTP method + * + * This method may be overridden by subtypes to set authentication data, + * modify headers, hook XHR callbacks, etc. + * + * Subtypes may throw exceptions; the caller of this method catches and + * properly forwards them to the callback. + * + * This method must be synchronous. + * + * @param {XMLHttpRequest} req request to prepare + * @param {string} url destination URL + * @param {string} method HTTP method + * + * @return {undefined} + */ + 'virtual protected openRequest': function( req, url, method ) + { + // alway async + req.open( method, url, true ); + }, + + + /** + * Hooks ready state change to handle data + * + * Subtypes may override this method to alter the ready state change + * actions taken (e.g. to display progress, handle errors, etc.) + * + * @param {XMLHttpRequest} req request to hook + * @param {function(string)} callback continuation to invoke with response + * + * @return {undefined} + * + * @throws {Error} if non-200 response received from server + */ + 'virtual protected onLoad': function( req, callback ) + { + req.onreadystatechange = function() + { + // ready state of 4 (DONE) indicates that the request is complete + if ( req.readyState !== 4 ) + { + return; + } + else if ( req.status !== 200 ) + { + callback( + Error( req.status + " error from server" ), + { + status: req.status, + data: req.responseText + } + ); + return; + } + + // successful + callback( null, req.responseText ); + }; + } +} ); + diff --git a/test/dapi/http/HttpDataApiTest.js b/test/dapi/http/HttpDataApiTest.js index eeeb7d6..748912e 100644 --- a/test/dapi/http/HttpDataApiTest.js +++ b/test/dapi/http/HttpDataApiTest.js @@ -27,7 +27,7 @@ var dapi = require( '../../../' ).dapi, dummy_url = 'http://foo', dummy_impl = Class .implement( dapi.http.HttpImpl ) - .extend( { requestData: function( _, __, ___, ____ ) {} } ), + .extend( { requestData: function( _, __, ___, ____ ) {} } )(), dummy_sut = Sut( dummy_url, 'GET', dummy_impl ); diff --git a/test/dapi/http/XhrHttpImplTest.js b/test/dapi/http/XhrHttpImplTest.js new file mode 100644 index 0000000..7e3efc6 --- /dev/null +++ b/test/dapi/http/XhrHttpImplTest.js @@ -0,0 +1,219 @@ +/** + * Test case for XMLHttpRequest HTTP protocol implementation + * + * Copyright (C) 2014 LoVullo Associates, Inc. + * + * This file is part of the Liza Data Collection Framework + * + * Liza is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +var dapi = require( '../../../' ).dapi, + expect = require( 'chai' ).expect, + Class = require( 'easejs' ).Class, + HttpImpl = dapi.http.HttpImpl, + Sut = dapi.http.XhrHttpImpl, + + DummyXhr = function() + { + this.open = function() + { + DummyXhr.args = arguments; + }; + }; + + +describe( 'XhrHttpImpl', function() +{ + /** + * Since ECMAScript does not have return typing, we won't know if the ctor + * actually returns an XMLHttpRequest until we try. + */ + it( 'will accept any constructor', function() + { + expect( function() + { + Sut( function() {} ); + } ).to.not.throw( Error ); + } ); + + + it( 'is an HttpImpl', function() + { + var sut = Sut( function() {} ); + expect( Class.isA( HttpImpl, sut ) ).to.be.ok; + } ); + + + describe( '.requestData', function() + { + it( 'requests a connection using the given url and method', function() + { + var url = 'http://foonugget', + method = 'GET', + sut = Sut( DummyXhr ); + + sut.requestData( url, method, {}, function() {} ); + + var args = DummyXhr.args; + expect( args[ 0 ] ).to.equal( method ); + expect( args[ 1 ] ).to.equal( url ); + expect( args[ 1 ] ).to.be.ok; // async + } ); + + + /** + * Since the request is asynchronous, we should be polite and not return + * errors in two different formats; we will catch it and instead pass it + * back via the callback. + */ + it( 'returns XHR open() errors via callback', function( done ) + { + var e = Error( "Test error" ), + Xhr = function() + { + this.open = function() + { + throw e; + }; + }; + + // should not throw an exception + expect( function() + { + // should instead provide to callback + Sut( Xhr ) + .requestData( 'http://foo', 'GET', {}, function( err, data ) + { + expect( err ).to.equal( e ); + expect( data ).to.equal( null ); + done(); + } ); + } ).to.not.throw( Error ); + } ); + + + it( 'returns XHR response via callback when no error', function( done ) + { + var retdata = "foobar", + src = "moocow", + StubXhr = createStubXhr(); + + StubXhr.prototype.responseText = retdata; + StubXhr.prototype.readyState = 4; // done + StubXhr.prototype.status = 200; // OK + + StubXhr.prototype.send = function( data ) + { + expect( data ).is.equal( src ); + StubXhr.inst.onreadystatechange(); + }; + + Sut( StubXhr ) + .requestData( 'http://bar', 'GET', src, function( err, resp ) + { + expect( err ).to.equal( null ); + expect( resp ).to.equal( retdata ); + done(); + } ); + + // invoke callback + StubXhr.inst.send( src ); + } ); + + + describe( 'if return status code is not 200', function() + { + /** + * This is the default behavior, but can be changed by overriding + * the onLoad method. + */ + it( 'returns an error to the callback', function( done ) + { + var StubXhr = createStubXhr(); + StubXhr.prototype.status = 404; + + Sut( StubXhr ) + .requestData( 'http://foo', 'GET', '', function( err, _ ) + { + expect( err ).to.be.instanceOf( Error ); + expect( err.message ).to.contain( + StubXhr.prototype.status + ); + + done(); + } ); + + StubXhr.inst.send( '' ); + } ); + + + it( 'returns response text with error code', function( done ) + { + var StubXhr = createStubXhr(), + status = 404, + reply = 'foobunny'; + + StubXhr.prototype.status = status; + StubXhr.prototype.responseText = reply; + + Sut( StubXhr ) + .requestData( 'http://foo', 'GET', '', function( _, resp ) + { + expect( resp.status ).to.equal( status ); + expect( resp.data ).to.equal( reply ); + done(); + } ); + + StubXhr.inst.send( '' ); + } ); + } ); + + + it( 'returns self', function() + { + var sut = Sut( function() {} ), + ret = sut.requestData( + 'http://foo', 'GET', {}, function() {} + ); + + expect( ret ).to.equal( sut ); + } ); + } ); +} ); + + +function createStubXhr() +{ + var StubXhr = function() + { + StubXhr.inst = this; + }; + + StubXhr.prototype = { + onreadystatechange: null, + responseText: '', + readyState: 4, // don, + status: 200, // O, + + open: function() {}, + send: function( data ) + { + this.onreadystatechange(); + } + }; + + return StubXhr; +} + From 0a07ad498287e760abc58b13d4498d6cfd0fb126 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 24 Apr 2014 14:16:12 -0400 Subject: [PATCH 04/22] Added JsonDataApi --- src/dapi/format/JsonDataApi.js | 91 ++++++++++++++++++++++ test/dapi/format/JsonDataApiTest.js | 116 ++++++++++++++++++++++++++++ 2 files changed, 207 insertions(+) create mode 100644 src/dapi/format/JsonDataApi.js create mode 100644 test/dapi/format/JsonDataApiTest.js diff --git a/src/dapi/format/JsonDataApi.js b/src/dapi/format/JsonDataApi.js new file mode 100644 index 0000000..97763b5 --- /dev/null +++ b/src/dapi/format/JsonDataApi.js @@ -0,0 +1,91 @@ +/** + * Processes DataApi return data as JSON + * + * Copyright (C) 2014 LoVullo Associates, Inc. + * + * This file is part of the Liza Data Collection Framework + * + * Liza is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +var Class = require( 'easejs' ).Class, + DataApi = require( '../DataApi' ); + + +/** + * Processes DataApi return data as JSON + */ +module.exports = Class( 'JsonDataApi' ) + .implement( DataApi ) + .extend( +{ + /** + * DataAPI to decorate + * @type {DataApi} + */ + 'private _dapi': null, + + + /** + * Decorate provided DataAPI to parse response data as JSON + * + * @param {DataApi} dapi DataApi to decorate + */ + __construct: function( dapi ) + { + if ( !( Class.isA( DataApi, dapi ) ) ) + { + throw TypeError( "Expecting DataApi" ); + } + + this._dapi = dapi; + }, + + + /** + * Proxies request to encapsulated DataApi and parses the result as JSON + * + * @param {string} data binary data to transmit + * @param {function(?Error,*)} callback continuation upon reply + * + * @return {DataApi} self + */ + 'public request': function( data, callback ) + { + this._dapi.request( data, function( err, resp ) + { + if ( err !== null ) + { + callback( err, null ); + return; + } + + try + { + var data = JSON.parse( resp ); + } + catch ( e ) + { + // parsing failed + callback( e, null ); + return; + } + + callback( null, data ); + } ); + + return this; + } +} ); + diff --git a/test/dapi/format/JsonDataApiTest.js b/test/dapi/format/JsonDataApiTest.js new file mode 100644 index 0000000..4ac363a --- /dev/null +++ b/test/dapi/format/JsonDataApiTest.js @@ -0,0 +1,116 @@ +/** + * Test case for JSON formatting of API result + * + * Copyright (C) 2014 LoVullo Associates, Inc. + * + * This file is part of the Liza Data Collection Framework + * + * Liza is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +var dapi = require( '../../../' ).dapi, + expect = require( 'chai' ).expect, + Class = require( 'easejs' ).Class, + DataApi = dapi.DataApi, + Sut = dapi.format.JsonDataApi; + + +describe( 'JsonDataApi', function() +{ + it( 'errors if not provided a DataApi', function() + { + expect( function() + { + Sut( {} ); + } ).to.throw( TypeError, 'DataApi' ); + } ); + + + it( 'accepts a DataApi', function() + { + expect( function() + { + Sut( _createStubDapi( null, '' ) ); + } ).to.not.throw( TypeError ); + } ); + + + describe( '.request', function() + { + it( 'passes data to encapsulated DataApi', function() + { + var stubs = _createStubDapi( null, '0' ), + expected = {}; + + Sut( stubs ).request( expected, function() {} ); + expect( stubs.given ).to.equal( expected ); + } ); + + + it( 'converts response to JSON', function( done ) + { + var raw = '{"foo": "bar"}'; + Sut( _createStubDapi( null, raw ) ).request( '', function( err, data ) + { + // should have been converted to JSON + expect( data ).to.deep.equal( { foo: 'bar' } ); + expect( err ).to.equal( null ); + done(); + } ); + } ); + + + it( 'returns error if JSON parsing fails', function( done ) + { + Sut( _createStubDapi( null, 'ERR' ) ) + .request( '', function( err, data ) + { + expect( err ).to.be.instanceOf( Error ); + expect( data ).to.equal( null ); + done(); + } ); + } ); + + + it( 'proxy error from encapsulated DataApi', function( done ) + { + var e = Error( 'foo' ); + + Sut( _createStubDapi( e, '0' ) ) + .request( '', function( err, data ) + { + // data should also be cleared out + expect( err ).to.equal( e ); + expect( data ).to.equal( null ); + done(); + } ); + } ); + } ); +} ); + + +function _createStubDapi( err, resp ) +{ + return Class.implement( DataApi ).extend( + { + given: null, + + request: function( data, callback ) + { + this.given = data; + callback( err, resp ); + } + } )(); +} + From 0e9c511117bb209a22e530ea1e6446cb8793a561 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 24 Apr 2014 15:59:10 -0400 Subject: [PATCH 05/22] HttpDataApi.request is now virtual Will support mixins --- src/dapi/http/HttpDataApi.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dapi/http/HttpDataApi.js b/src/dapi/http/HttpDataApi.js index fed8bb2..789bfc0 100644 --- a/src/dapi/http/HttpDataApi.js +++ b/src/dapi/http/HttpDataApi.js @@ -104,7 +104,7 @@ module.exports = Class( 'HttpDataApi' ) * * @throws {TypeError} on validation failure */ - 'public request': function( data, callback ) + 'virtual public request': function( data, callback ) { this._validateDataType( data ); From b4ef6439e9e7e655e9d72bfd76a8e071f6e68aa4 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 24 Apr 2014 15:59:31 -0400 Subject: [PATCH 06/22] JsonDataApi=>JsonResponse trait This cuts down on the code significantly and makes more sense, since we would not want to veil the API of any classes that want to use this. --- .../{JsonDataApi.js => JsonResponse.js} | 33 +++--------- ...JsonDataApiTest.js => JsonResponseTest.js} | 52 +++++++------------ 2 files changed, 24 insertions(+), 61 deletions(-) rename src/dapi/format/{JsonDataApi.js => JsonResponse.js} (70%) rename test/dapi/format/{JsonDataApiTest.js => JsonResponseTest.js} (68%) diff --git a/src/dapi/format/JsonDataApi.js b/src/dapi/format/JsonResponse.js similarity index 70% rename from src/dapi/format/JsonDataApi.js rename to src/dapi/format/JsonResponse.js index 97763b5..a9fc714 100644 --- a/src/dapi/format/JsonDataApi.js +++ b/src/dapi/format/JsonResponse.js @@ -19,51 +19,30 @@ * along with this program. If not, see . */ -var Class = require( 'easejs' ).Class, +var Trait = require( 'easejs' ).Trait, DataApi = require( '../DataApi' ); /** * Processes DataApi return data as JSON */ -module.exports = Class( 'JsonDataApi' ) +module.exports = Trait( 'JsonResponse' ) .implement( DataApi ) .extend( { /** - * DataAPI to decorate - * @type {DataApi} - */ - 'private _dapi': null, - - - /** - * Decorate provided DataAPI to parse response data as JSON + * Processes response as JSON * - * @param {DataApi} dapi DataApi to decorate - */ - __construct: function( dapi ) - { - if ( !( Class.isA( DataApi, dapi ) ) ) - { - throw TypeError( "Expecting DataApi" ); - } - - this._dapi = dapi; - }, - - - /** - * Proxies request to encapsulated DataApi and parses the result as JSON + * Will return an error if the response is not valid JSON. * * @param {string} data binary data to transmit * @param {function(?Error,*)} callback continuation upon reply * * @return {DataApi} self */ - 'public request': function( data, callback ) + 'abstract override public request': function( data, callback ) { - this._dapi.request( data, function( err, resp ) + this.__super( data, function( err, resp ) { if ( err !== null ) { diff --git a/test/dapi/format/JsonDataApiTest.js b/test/dapi/format/JsonResponseTest.js similarity index 68% rename from test/dapi/format/JsonDataApiTest.js rename to test/dapi/format/JsonResponseTest.js index 4ac363a..ede3046 100644 --- a/test/dapi/format/JsonDataApiTest.js +++ b/test/dapi/format/JsonResponseTest.js @@ -23,37 +23,19 @@ var dapi = require( '../../../' ).dapi, expect = require( 'chai' ).expect, Class = require( 'easejs' ).Class, DataApi = dapi.DataApi, - Sut = dapi.format.JsonDataApi; + Sut = dapi.format.JsonResponse; -describe( 'JsonDataApi', function() +describe( 'dapi.format.JsonRepsonse trait', function() { - it( 'errors if not provided a DataApi', function() - { - expect( function() - { - Sut( {} ); - } ).to.throw( TypeError, 'DataApi' ); - } ); - - - it( 'accepts a DataApi', function() - { - expect( function() - { - Sut( _createStubDapi( null, '' ) ); - } ).to.not.throw( TypeError ); - } ); - - describe( '.request', function() { it( 'passes data to encapsulated DataApi', function() { - var stubs = _createStubDapi( null, '0' ), + var stubs = _createStubbedDapi( null, '0' ), expected = {}; - Sut( stubs ).request( expected, function() {} ); + stubs.request( expected, function() {} ); expect( stubs.given ).to.equal( expected ); } ); @@ -61,19 +43,21 @@ describe( 'JsonDataApi', function() it( 'converts response to JSON', function( done ) { var raw = '{"foo": "bar"}'; - Sut( _createStubDapi( null, raw ) ).request( '', function( err, data ) - { - // should have been converted to JSON - expect( data ).to.deep.equal( { foo: 'bar' } ); - expect( err ).to.equal( null ); - done(); - } ); + + _createStubbedDapi( null, raw ) + .request( '', function( err, data ) + { + // should have been converted to JSON + expect( data ).to.deep.equal( { foo: 'bar' } ); + expect( err ).to.equal( null ); + done(); + } ); } ); it( 'returns error if JSON parsing fails', function( done ) { - Sut( _createStubDapi( null, 'ERR' ) ) + _createStubbedDapi( null, 'ERR' ) .request( '', function( err, data ) { expect( err ).to.be.instanceOf( Error ); @@ -87,7 +71,7 @@ describe( 'JsonDataApi', function() { var e = Error( 'foo' ); - Sut( _createStubDapi( e, '0' ) ) + _createStubbedDapi( e, '0' ) .request( '', function( err, data ) { // data should also be cleared out @@ -100,17 +84,17 @@ describe( 'JsonDataApi', function() } ); -function _createStubDapi( err, resp ) +function _createStubbedDapi( err, resp ) { return Class.implement( DataApi ).extend( { given: null, - request: function( data, callback ) + 'virtual public request': function( data, callback ) { this.given = data; callback( err, resp ); } - } )(); + } ).use( Sut )(); } From 6cc260b443aff4e1b6a2c54bbdb678da73007449 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 12 May 2015 13:53:46 -0400 Subject: [PATCH 07/22] XhrHttpImpl#isSuccessful determines HTTP response success May be overridden by subtypes. --- src/dapi/http/XhrHttpImpl.js | 22 ++++++++++++++++++++-- test/dapi/http/XhrHttpImplTest.js | 27 ++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/dapi/http/XhrHttpImpl.js b/src/dapi/http/XhrHttpImpl.js index ac9c6c7..3ecfb57 100644 --- a/src/dapi/http/XhrHttpImpl.js +++ b/src/dapi/http/XhrHttpImpl.js @@ -107,7 +107,9 @@ module.exports = Class( 'XhrHttpImpl' ) * Hooks ready state change to handle data * * Subtypes may override this method to alter the ready state change - * actions taken (e.g. to display progress, handle errors, etc.) + * actions taken (e.g. to display progress, handle errors, etc.) If + * only the HTTP status needs to be checked, subtypes may override + * success/failure determination via `#isSuccessful'. * * @param {XMLHttpRequest} req request to hook * @param {function(string)} callback continuation to invoke with response @@ -118,6 +120,8 @@ module.exports = Class( 'XhrHttpImpl' ) */ 'virtual protected onLoad': function( req, callback ) { + var _self = this; + req.onreadystatechange = function() { // ready state of 4 (DONE) indicates that the request is complete @@ -125,7 +129,7 @@ module.exports = Class( 'XhrHttpImpl' ) { return; } - else if ( req.status !== 200 ) + else if ( !( _self.isSuccessful( req.status ) ) ) { callback( Error( req.status + " error from server" ), @@ -140,6 +144,20 @@ module.exports = Class( 'XhrHttpImpl' ) // successful callback( null, req.responseText ); }; + }, + + + /** + * Determine whether the given HTTP status indicates a success or + * failure + * + * @param {number} status HTTP response status + * + * @return {bool} whether HTTP status represents a success + */ + 'virtual protected isSuccessful': function( status ) + { + return +status === 200; } } ); diff --git a/test/dapi/http/XhrHttpImplTest.js b/test/dapi/http/XhrHttpImplTest.js index 7e3efc6..05daa6a 100644 --- a/test/dapi/http/XhrHttpImplTest.js +++ b/test/dapi/http/XhrHttpImplTest.js @@ -133,7 +133,7 @@ describe( 'XhrHttpImpl', function() } ); - describe( 'if return status code is not 200', function() + describe( 'if return status code is not successful', function() { /** * This is the default behavior, but can be changed by overriding @@ -181,6 +181,31 @@ describe( 'XhrHttpImpl', function() } ); + it( 'allows overriding notion of success/failure', function( done ) + { + var chk = 12345; + + // succeed on CHK + var StubXhr = createStubXhr(); + StubXhr.prototype.status = chk; + + Sut.extend( + { + 'override protected isSuccessful': function( status ) + { + return status === chk; + }, + } )( StubXhr ) + .requestData( 'http://foo', 'GET', '', function( err, resp ) + { + expect( err ).to.equal( null ); + done(); + } ); + + StubXhr.inst.send( '' ); + } ); + + it( 'returns self', function() { var sut = Sut( function() {} ), From d4328968e89d739731e3c443a661551c117add1b Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 12 May 2015 14:06:02 -0400 Subject: [PATCH 08/22] XhrHttpImpl considers any 2xx status to be successful --- src/dapi/http/XhrHttpImpl.js | 5 ++++- test/dapi/http/XhrHttpImplTest.js | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/dapi/http/XhrHttpImpl.js b/src/dapi/http/XhrHttpImpl.js index 3ecfb57..04ac100 100644 --- a/src/dapi/http/XhrHttpImpl.js +++ b/src/dapi/http/XhrHttpImpl.js @@ -151,13 +151,16 @@ module.exports = Class( 'XhrHttpImpl' ) * Determine whether the given HTTP status indicates a success or * failure * + * The default implementation is to consider any 2xx status code to be + * successful, as indicated by RFC 2616. + * * @param {number} status HTTP response status * * @return {bool} whether HTTP status represents a success */ 'virtual protected isSuccessful': function( status ) { - return +status === 200; + return ( +status >= 200 ) && ( +status < 300 ); } } ); diff --git a/test/dapi/http/XhrHttpImplTest.js b/test/dapi/http/XhrHttpImplTest.js index 05daa6a..1256f1c 100644 --- a/test/dapi/http/XhrHttpImplTest.js +++ b/test/dapi/http/XhrHttpImplTest.js @@ -181,6 +181,22 @@ describe( 'XhrHttpImpl', function() } ); + it( 'considers any 2xx status to be successful', function( done ) + { + var StubXhr = createStubXhr(); + StubXhr.prototype.status = 250; + + Sut( StubXhr ) + .requestData( 'http://foo', 'GET', '', function( err, _ ) + { + expect( err ).to.equal( null ); + done(); + } ); + + StubXhr.inst.send( '' ); + } ); + + it( 'allows overriding notion of success/failure', function( done ) { var chk = 12345; From 8fbd4dd2209481c529f6b23f29f8a26c6d5c775b Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 12 May 2015 14:24:24 -0400 Subject: [PATCH 09/22] XhrHttpImpl#serveError --- src/dapi/http/XhrHttpImpl.js | 51 +++++++++++++++++++++++++------ test/dapi/http/XhrHttpImplTest.js | 32 +++++++++++++++++++ 2 files changed, 73 insertions(+), 10 deletions(-) diff --git a/src/dapi/http/XhrHttpImpl.js b/src/dapi/http/XhrHttpImpl.js index 04ac100..a2482ee 100644 --- a/src/dapi/http/XhrHttpImpl.js +++ b/src/dapi/http/XhrHttpImpl.js @@ -109,10 +109,15 @@ module.exports = Class( 'XhrHttpImpl' ) * Subtypes may override this method to alter the ready state change * actions taken (e.g. to display progress, handle errors, etc.) If * only the HTTP status needs to be checked, subtypes may override - * success/failure determination via `#isSuccessful'. + * success/failure determination via `#isSuccessful'. If the error + * response needs to be customized, override `#serveError'. * - * @param {XMLHttpRequest} req request to hook - * @param {function(string)} callback continuation to invoke with response + * When overriding this method, please either call the parent method or + * invoke the aforementioned two methods. + * + * @param {XMLHttpRequest} req request to hook + * @param {function(?Error,string)} callback continuation to invoke with + * response * * @return {undefined} * @@ -131,13 +136,7 @@ module.exports = Class( 'XhrHttpImpl' ) } else if ( !( _self.isSuccessful( req.status ) ) ) { - callback( - Error( req.status + " error from server" ), - { - status: req.status, - data: req.responseText - } - ); + _self.serveError( req, callback ); return; } @@ -161,6 +160,38 @@ module.exports = Class( 'XhrHttpImpl' ) 'virtual protected isSuccessful': function( status ) { return ( +status >= 200 ) && ( +status < 300 ); + }, + + + /** + * Serve an error response + * + * The default behavior is to return an Error and content containing the + * HTTP status and response text. + * + * When overriding this method, keep in mind that it should always + * return an Error for the first argument, or set it to null, indicating + * a success. + * + * This method exposes the original XMLHttpRequest used to make the + * request, so it can be used to perform additional analysis for error + * handling, or to override the error and instead return a successful + * response. + * + * @param {XMLHttpRequest} req request to hook + * @param {function(?Error,string)} callback continuation to invoke with + * response + * @return {undefined} + */ + 'virtual protected serveError': function( req, callback ) + { + callback( + Error( req.status + " error from server" ), + { + status: req.status, + data: req.responseText + } + ); } } ); diff --git a/test/dapi/http/XhrHttpImplTest.js b/test/dapi/http/XhrHttpImplTest.js index 1256f1c..eeebde3 100644 --- a/test/dapi/http/XhrHttpImplTest.js +++ b/test/dapi/http/XhrHttpImplTest.js @@ -222,6 +222,38 @@ describe( 'XhrHttpImpl', function() } ); + it( 'allows customizing error', function( done ) + { + var _self = this, + chk = {}; + + var StubXhr = createStubXhr(); + StubXhr.prototype.status = 404; + + Sut.extend( + { + 'override protected serveError': function( req, callback ) + { + var e = Error( 'foobunny' ); + e.foo = true; + + expect( req ).to.be.an.instanceOf( StubXhr ); + + callback( e, chk ); + }, + } )( StubXhr ) + .requestData( 'http://foo', 'GET', '', function( err, resp ) + { + expect( ( err || {} ).foo ).to.be.ok; + expect( resp ).to.equal( chk ); + + done(); + } ); + + StubXhr.inst.send( '' ); + } ); + + it( 'returns self', function() { var sut = Sut( function() {} ), From ca5d064455cbad912b994ff0b40fdbb9f1292439 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 19 May 2015 13:13:49 -0400 Subject: [PATCH 10/22] AutoRetry trait initial implementation --- src/dapi/AutoRetry.js | 187 +++++++++++++++++++++++++++++++++++ src/dapi/http/XhrHttpImpl.js | 2 +- test/dapi/AutoRetryTest.js | 179 +++++++++++++++++++++++++++++++++ 3 files changed, 367 insertions(+), 1 deletion(-) create mode 100644 src/dapi/AutoRetry.js create mode 100644 test/dapi/AutoRetryTest.js diff --git a/src/dapi/AutoRetry.js b/src/dapi/AutoRetry.js new file mode 100644 index 0000000..98155a5 --- /dev/null +++ b/src/dapi/AutoRetry.js @@ -0,0 +1,187 @@ +/** + * DataApi auto-retry requests on specified failure + * + * Copyright (C) 2015 LoVullo Associates, Inc. + * + * This file is part of the Liza Data Collection Framework + * + * Liza is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +var Trait = require( 'easejs' ).Trait, + DataApi = require( './DataApi' ); + + +module.exports = Trait( 'AutoRetry' ) + .implement( DataApi ) + .extend( +{ + /** + * Predicate function determining whether a retry is needed + * @var {function(?Error,*): boolean} + */ + 'private _pred': '', + + /** + * Maximum number of tries (including initial request) + * @var {number} + */ + 'private _tries': 0, + + /** + * Delay in milliseconds before making the nth request as a function + * of n + * + * @var {function(number): number} + */ + 'private _delay': null, + + + /** + * Initialize auto-retry + * + * If TRIES is negative, then requests will continue indefinitely until + * one succeeds. If TRIES is 0, then no requests will be performed. + * + * @param {function(?Error,*): boolean} pred predicate determining if + * a retry is needed + * @param {number} tries maximum number of tries, + * including the initial + * request + * @param {function(number): number} delay delay in milliseconds before + * making the nth request as + * a function of n + * + * @return {undefined} + */ + __mixin: function( pred, tries, delay ) + { + if ( typeof pred !== 'function' ) + { + throw TypeError( 'Predicate must be a function' ); + } + if ( typeof delay !== 'function' ) + { + throw TypeError( "Delay must be a function" ); + } + + this._pred = pred; + this._tries = +tries; + this._delay = delay; + }, + + + /** + * Perform an asynchronous request and invoke CALLBACK with the + * reply + * + * In the special case that the number of tries is set to zero, CALLBACK + * will be immediately invoked with a null error and result (but not + * necessarily asynchronously---that remains undefined). + * + * Otherwise, requests will continue to be re-issued until either the + * request succeeds or the number of retries are exhausted, whichever + * comes first. Once the retries are exhausted, the error and output + * data from the final request are returned. + * + * If the number of tries is negative, then requests will be performed + * indefinitely until success. + * + * TODO: A means of aborting. + * + * @param {string} input binary data to transmit + * @param {function(?Error,*)} callback continuation upon reply + * + * @return {DataApi} self + */ + 'abstract override public request': function( input, callback ) + { + this._try( input, callback, this._tries ); + + return this; + }, + + + /** + * Recursively perform request until success or try exhaustion + * + * For more information, see `#request'. + * + * @param {string} input binary data to transmit + * @param {function(?Error,*)} callback continuation upon reply + * @param {number} n number of retries remaining + * + * @return {undefined} + */ + 'private _try': function( input, callback, n ) + { + var _self = this; + + // the special case of 0 retries still invokes the callback, but has + // no data to return + if ( n === 0 ) + { + callback( null, null ); + return; + } + + this.request.super.call( this, input, function( err, output ) + { + // predicate determines whether a retry is necessary + if ( !!_self._pred( err, output ) === false ) + { + return _self._succeed( output, callback ); + } + + // note that we intentionally do not want to check <= 1, so that + // we can proceed indefinitely (JavaScript does not wrap on overflow) + if ( n === 1 ) + { + return _self._fail( err, output, callback ); + } + + _self._try( input, callback, ( n - 1 ) ); + } ); + }, + + + /** + * Produce a successful response + * + * @param {*} output output data + * @param {function(?Error,*)} callback continuation to invoke + * + * @return {undefined} + */ + 'private _succeed': function( output, callback ) + { + callback( null, output ); + }, + + + /** + * Produce a negative response + * + * @param {Error} err most recent error + * @param {*} output most recent output data + * @param {function(?Error,*)} callback continuation to invoke + * + * @return {undefined} + */ + 'private _fail': function( err, output, callback ) + { + callback( err, output ); + }, +} ); + diff --git a/src/dapi/http/XhrHttpImpl.js b/src/dapi/http/XhrHttpImpl.js index a2482ee..f0d0e08 100644 --- a/src/dapi/http/XhrHttpImpl.js +++ b/src/dapi/http/XhrHttpImpl.js @@ -1,7 +1,7 @@ /** * XMLHttpRequest HTTP protocol implementation * - * Copyright (C) 2014 LoVullo Associates, Inc. + * Copyright (C) 2015 LoVullo Associates, Inc. * * This file is part of the Liza Data Collection Framework * diff --git a/test/dapi/AutoRetryTest.js b/test/dapi/AutoRetryTest.js new file mode 100644 index 0000000..a5e6ab2 --- /dev/null +++ b/test/dapi/AutoRetryTest.js @@ -0,0 +1,179 @@ +/** + * Test case for AutoRetry + * + * Copyright (C) 2015 LoVullo Associates, Inc. + * + * This file is part of the Liza Data Collection Framework + * + * Liza is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +var dapi = require( '../../' ).dapi, + expect = require( 'chai' ).expect, + Class = require( 'easejs' ).Class, + DataApi = dapi.DataApi, + Sut = dapi.AutoRetry; + +var _void = function() {}, + _true = function() { return true; }; + + +describe( 'dapi.AutoRetry trait', function() +{ + /** + * If there are no failures, then AutoRetry has no observable effects. + */ + describe( 'when the request is successful', function() + { + it( 'makes only one request', function( done ) + { + var given = {}; + + // success (but note the number of retries presented) + var stub = _createStub( null, '' ) + .use( Sut( _void, 5, _void ) ) + (); + + stub.request( given, function() + { + expect( stub.given ).to.equal( given ); + expect( stub.requests ).to.equal( 1 ); + done(); + } ); + + } ); + + + it( 'returns the response data with no error', function( done ) + { + var chk = { foo: 'bar' }; + + // notice that we provide an error to the stub; this will ensure + // that the returned error is null even when one is provided + var stub = _createStub( {}, chk ) + .use( Sut( _void, 1, _void ) ) + (); + + stub.request( '', function( err, data ) + { + expect( err ).to.equal( null ); + expect( data ).to.equal( chk ); + done(); + } ); + } ); + } ); + + + /** + * This is when we care. + */ + describe( 'when the request fails', function() + { + it( 'will re-perform request N-1 times until failure', function( done ) + { + var n = 5; + + var stub = _createStub( {}, {} ) + .use( Sut( _true, n, _void ) ) + (); + + stub.request( {}, function( err, _ ) + { + expect( stub.requests ).to.equal( n ); + done(); + } ); + } ); + + + it( 'will return most recent error and output data', function( done ) + { + var e = Error( 'foo' ), + output = {}; + + // XXX: this does not test for most recent, because the return + // data are static for each request + var stub = _createStub( e, output ) + .use( Sut( _true, 1, _void ) ) + (); + + stub.request( {}, function( err, data ) + { + expect( err ).to.equal( e ); + expect( data ).to.equal( output ); + done(); + } ); + } ); + + + describe( 'given a negative number of tries', function() + { + it( 'will continue until a successful request', function( done ) + { + var n = 10, + pred = function( _, __ ) + { + return --n > 0; + }; + + var stub = _createStub() + .use( Sut( pred, -1, _void ) ) + (); + + stub.request( {}, function( _, __ ) + { + expect( n ).to.equal( 0 ); + done(); + } ); + } ); + } ); + } ); + + + describe( 'when the number of tries is zero', function() + { + it( 'will perform zero requests with null results', function( done ) + { + var stub = _createStub( {}, {} ) + .use( Sut( _void, 0, _void ) ) + (); + + stub.request( {}, function( err, data ) + { + expect( stub.requests ).to.equal( 0 ); + expect( err ).to.equal( null ); + expect( data ).to.equal( null ); + done(); + } ); + } ); + } ); +} ); + + + +function _createStub( err, resp ) +{ + return Class.implement( DataApi ).extend( + { + given: null, + requests: 0, + + 'virtual public request': function( data, callback ) + { + this.given = data; + this.requests++; + + callback( err, resp ); + } + } ); +} From af4a775155b373c5f55b48cf1edc1d03845bd864 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 22 May 2015 16:22:58 -0400 Subject: [PATCH 11/22] AutoRetry delay implementation --- src/dapi/AutoRetry.js | 63 +++++++++++------- src/dapi/http/XhrHttpImpl.js | 1 - test/dapi/AutoRetryTest.js | 120 +++++++++++++++++++++++++++++++++-- 3 files changed, 155 insertions(+), 29 deletions(-) diff --git a/src/dapi/AutoRetry.js b/src/dapi/AutoRetry.js index 98155a5..77048c0 100644 --- a/src/dapi/AutoRetry.js +++ b/src/dapi/AutoRetry.js @@ -40,10 +40,10 @@ module.exports = Trait( 'AutoRetry' ) 'private _tries': 0, /** - * Delay in milliseconds before making the nth request as a function - * of n + * Function to be passed a continuation to introduce a delay between + * requests * - * @var {function(number): number} + * @var {function(number,function(),function())} delay */ 'private _delay': null, @@ -52,33 +52,42 @@ module.exports = Trait( 'AutoRetry' ) * Initialize auto-retry * * If TRIES is negative, then requests will continue indefinitely until - * one succeeds. If TRIES is 0, then no requests will be performed. + * one succeeds or is aborted by DELAY. If TRIES is 0, then no requests + * will be performed. * - * @param {function(?Error,*): boolean} pred predicate determining if - * a retry is needed - * @param {number} tries maximum number of tries, - * including the initial - * request - * @param {function(number): number} delay delay in milliseconds before - * making the nth request as - * a function of n + * If DELAY is a function, then it invoked with a retry continuation + * before each retry, the number of tries remaining, and a failure + * continuation that may be used to abort the process at an arbitrary + * time. * - * @return {undefined} - */ + * @param {function(?Error,*): boolean} pred predicate determining if + * a retry is needed + * @param {number} tries maximum number of tries, + * including the initial + * request + * + * @param {?function(number,function(),function())} delay + * an optional function + * accepting a continuation + * to continue with the next + * request + * + * @return {undefined} + */ __mixin: function( pred, tries, delay ) { if ( typeof pred !== 'function' ) { throw TypeError( 'Predicate must be a function' ); } - if ( typeof delay !== 'function' ) + if ( delay && ( typeof delay !== 'function' ) ) { throw TypeError( "Delay must be a function" ); } this._pred = pred; this._tries = +tries; - this._delay = delay; + this._delay = delay || function( _, c ) { c(); }; }, @@ -96,9 +105,8 @@ module.exports = Trait( 'AutoRetry' ) * data from the final request are returned. * * If the number of tries is negative, then requests will be performed - * indefinitely until success. - * - * TODO: A means of aborting. + * indefinitely until success; the delay function (as provided via the + * constructor) may be used to abort in this case. * * @param {string} input binary data to transmit * @param {function(?Error,*)} callback continuation upon reply @@ -144,14 +152,26 @@ module.exports = Trait( 'AutoRetry' ) return _self._succeed( output, callback ); } + var fail = function() + { + _self._fail( err, output, callback ); + }; + // note that we intentionally do not want to check <= 1, so that // we can proceed indefinitely (JavaScript does not wrap on overflow) if ( n === 1 ) { - return _self._fail( err, output, callback ); + return fail(); } - _self._try( input, callback, ( n - 1 ) ); + _self._delay( + ( n - 1 ), + function() + { + _self._try( input, callback, ( n - 1 ) ); + }, + fail + ); } ); }, @@ -184,4 +204,3 @@ module.exports = Trait( 'AutoRetry' ) callback( err, output ); }, } ); - diff --git a/src/dapi/http/XhrHttpImpl.js b/src/dapi/http/XhrHttpImpl.js index f0d0e08..e6ed598 100644 --- a/src/dapi/http/XhrHttpImpl.js +++ b/src/dapi/http/XhrHttpImpl.js @@ -194,4 +194,3 @@ module.exports = Class( 'XhrHttpImpl' ) ); } } ); - diff --git a/test/dapi/AutoRetryTest.js b/test/dapi/AutoRetryTest.js index a5e6ab2..8b10529 100644 --- a/test/dapi/AutoRetryTest.js +++ b/test/dapi/AutoRetryTest.js @@ -42,7 +42,7 @@ describe( 'dapi.AutoRetry trait', function() // success (but note the number of retries presented) var stub = _createStub( null, '' ) - .use( Sut( _void, 5, _void ) ) + .use( Sut( _void, 5 ) ) (); stub.request( given, function() @@ -62,7 +62,7 @@ describe( 'dapi.AutoRetry trait', function() // notice that we provide an error to the stub; this will ensure // that the returned error is null even when one is provided var stub = _createStub( {}, chk ) - .use( Sut( _void, 1, _void ) ) + .use( Sut( _void, 1 ) ) (); stub.request( '', function( err, data ) @@ -85,7 +85,7 @@ describe( 'dapi.AutoRetry trait', function() var n = 5; var stub = _createStub( {}, {} ) - .use( Sut( _true, n, _void ) ) + .use( Sut( _true, n ) ) (); stub.request( {}, function( err, _ ) @@ -104,7 +104,7 @@ describe( 'dapi.AutoRetry trait', function() // XXX: this does not test for most recent, because the return // data are static for each request var stub = _createStub( e, output ) - .use( Sut( _true, 1, _void ) ) + .use( Sut( _true, 1 ) ) (); stub.request( {}, function( err, data ) @@ -127,7 +127,7 @@ describe( 'dapi.AutoRetry trait', function() }; var stub = _createStub() - .use( Sut( pred, -1, _void ) ) + .use( Sut( pred, -1 ) ) (); stub.request( {}, function( _, __ ) @@ -145,7 +145,7 @@ describe( 'dapi.AutoRetry trait', function() it( 'will perform zero requests with null results', function( done ) { var stub = _createStub( {}, {} ) - .use( Sut( _void, 0, _void ) ) + .use( Sut( _void, 0 ) ) (); stub.request( {}, function( err, data ) @@ -157,6 +157,114 @@ describe( 'dapi.AutoRetry trait', function() } ); } ); } ); + + + describe( 'when a delay function is provided', function() + { + it( 'will wait for continuation before retry', function( done ) + { + var waited = false; + + var wait = function( _, c ) + { + waited = true; + c(); + }; + + var stub = _createStub( {}, {} ) + .use( Sut( _true, 2, wait ) ) + (); + + stub.request( {}, function( _, __ ) + { + expect( waited ).to.equal( true ); + done(); + } ); + } ); + + + it( 'will not process if continuation is not called', function() + { + var waited = false; + var wait = function( _, c ) + { + waited = true; + /* do not invoke */ + }; + + var stub = _createStub( {}, {} ) + .use( Sut( _true, 2, wait ) ) + (); + + // this works because we know that our stub is invoked + // synchronously + stub.request( {}, function( _, __ ) + { + throw Error( "Should not have been called!" ); + } ); + + expect( waited ).to.equal( true ); + } ); + + + it( 'will call delay function until success', function() + { + var n = 5; + var wait = function( tries_left, c ) + { + n--; + + // the first argument is the number of tries left + expect( tries_left ).to.equal( n ); + c(); + }; + + var pred = function() + { + return n > 0; + }; + + var stub = _createStub( {}, {} ) + .use( Sut( pred, n, wait ) ) + (); + + // this works because we know that our stub is invoked + // synchronously + stub.request( {}, _void ); + + // the first request counts as one, which brings us down to 4, + // but the wait function has not been called at this point; so, + // we expect that it will only be called four times + expect( n ).to.equal( 1 ); + } ); + + + it( 'allows aborting via failure continuation', function( done ) + { + var err_expect = {}, + out_expect = []; + + var wait = function( _, __, abort ) + { + abort(); + }; + + // without aborting, this would never finish + var stub = _createStub( err_expect, out_expect ) + .use( Sut( _true, -1, wait ) ) + (); + + // this works because we know that our stub is invoked + // synchronously + stub.request( {}, function( err, output ) + { + expect( err ).to.equal( err_expect ); + expect( output ).to.equal( out_expect ); + + done(); + } ); + } ); + } ); } ); From 2b92a2c4475ace2be2d164b3be2e67c30881926a Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 27 May 2015 15:24:38 -0400 Subject: [PATCH 12/22] JsonResponse#request must be virtual To suppor trait stacking. --- src/dapi/format/JsonResponse.js | 4 ++-- test/dapi/format/JsonResponseTest.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dapi/format/JsonResponse.js b/src/dapi/format/JsonResponse.js index a9fc714..db357e8 100644 --- a/src/dapi/format/JsonResponse.js +++ b/src/dapi/format/JsonResponse.js @@ -1,7 +1,7 @@ /** * Processes DataApi return data as JSON * - * Copyright (C) 2014 LoVullo Associates, Inc. + * Copyright (C) 2014, 2015 LoVullo Associates, Inc. * * This file is part of the Liza Data Collection Framework * @@ -40,7 +40,7 @@ module.exports = Trait( 'JsonResponse' ) * * @return {DataApi} self */ - 'abstract override public request': function( data, callback ) + 'virtual abstract override public request': function( data, callback ) { this.__super( data, function( err, resp ) { diff --git a/test/dapi/format/JsonResponseTest.js b/test/dapi/format/JsonResponseTest.js index ede3046..2932a56 100644 --- a/test/dapi/format/JsonResponseTest.js +++ b/test/dapi/format/JsonResponseTest.js @@ -1,7 +1,7 @@ /** * Test case for JSON formatting of API result * - * Copyright (C) 2014 LoVullo Associates, Inc. + * Copyright (C) 2014, 2015 LoVullo Associates, Inc. * * This file is part of the Liza Data Collection Framework * From 76280b0cac737d66d87cf93632ea952b359f4fd7 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 27 May 2015 15:26:03 -0400 Subject: [PATCH 13/22] AutoRetry distinction between predicate truth and request failure --- src/dapi/AutoRetry.js | 74 ++++++++++++++------------------------ test/dapi/AutoRetryTest.js | 28 +++++++++------ 2 files changed, 44 insertions(+), 58 deletions(-) diff --git a/src/dapi/AutoRetry.js b/src/dapi/AutoRetry.js index 77048c0..810677c 100644 --- a/src/dapi/AutoRetry.js +++ b/src/dapi/AutoRetry.js @@ -23,6 +23,14 @@ var Trait = require( 'easejs' ).Trait, DataApi = require( './DataApi' ); +/** + * Automatically retries requests while satisfying a given predicate + * + * It is important to distinguish between the concept of a request failure + * and a retry predicate: the former represents a problem with the request, + * whereas the latter indicates that a retry should be performed, but may + * not necessarily imply a request failure. + */ module.exports = Trait( 'AutoRetry' ) .implement( DataApi ) .extend( @@ -51,9 +59,9 @@ module.exports = Trait( 'AutoRetry' ) /** * Initialize auto-retry * - * If TRIES is negative, then requests will continue indefinitely until - * one succeeds or is aborted by DELAY. If TRIES is 0, then no requests - * will be performed. + * If TRIES is negative, then requests will continue indefinitely while + * the retry predicate is true, or is aborted by DELAY. If TRIES is 0, + * then no requests will be performed. * * If DELAY is a function, then it invoked with a retry continuation * before each retry, the number of tries remaining, and a failure @@ -87,7 +95,7 @@ module.exports = Trait( 'AutoRetry' ) this._pred = pred; this._tries = +tries; - this._delay = delay || function( _, c ) { c(); }; + this._delay = delay || function( _, c, __ ) { c(); }; }, @@ -100,13 +108,13 @@ module.exports = Trait( 'AutoRetry' ) * necessarily asynchronously---that remains undefined). * * Otherwise, requests will continue to be re-issued until either the - * request succeeds or the number of retries are exhausted, whichever - * comes first. Once the retries are exhausted, the error and output - * data from the final request are returned. + * retry predicate fails or the number of retries are exhausted, + * whichever comes first. Once the retries are exhausted, the error and + * output data from the final request are returned. * * If the number of tries is negative, then requests will be performed - * indefinitely until success; the delay function (as provided via the - * constructor) may be used to abort in this case. + * indefinitely while the retry predicate is met; the delay function (as + * provided via the constructor) may be used to abort in this case. * * @param {string} input binary data to transmit * @param {function(?Error,*)} callback continuation upon reply @@ -122,7 +130,8 @@ module.exports = Trait( 'AutoRetry' ) /** - * Recursively perform request until success or try exhaustion + * Recursively perform request until retry predicate failure or try + * count exhaustion * * For more information, see `#request'. * @@ -146,22 +155,22 @@ module.exports = Trait( 'AutoRetry' ) this.request.super.call( this, input, function( err, output ) { + var complete = function() + { + callback( err, output ); + }; + // predicate determines whether a retry is necessary if ( !!_self._pred( err, output ) === false ) { - return _self._succeed( output, callback ); + return complete(); } - var fail = function() - { - _self._fail( err, output, callback ); - }; - // note that we intentionally do not want to check <= 1, so that // we can proceed indefinitely (JavaScript does not wrap on overflow) if ( n === 1 ) { - return fail(); + return complete(); } _self._delay( @@ -170,37 +179,8 @@ module.exports = Trait( 'AutoRetry' ) { _self._try( input, callback, ( n - 1 ) ); }, - fail + complete ); } ); }, - - - /** - * Produce a successful response - * - * @param {*} output output data - * @param {function(?Error,*)} callback continuation to invoke - * - * @return {undefined} - */ - 'private _succeed': function( output, callback ) - { - callback( null, output ); - }, - - - /** - * Produce a negative response - * - * @param {Error} err most recent error - * @param {*} output most recent output data - * @param {function(?Error,*)} callback continuation to invoke - * - * @return {undefined} - */ - 'private _fail': function( err, output, callback ) - { - callback( err, output ); - }, } ); diff --git a/test/dapi/AutoRetryTest.js b/test/dapi/AutoRetryTest.js index 8b10529..b9c6867 100644 --- a/test/dapi/AutoRetryTest.js +++ b/test/dapi/AutoRetryTest.js @@ -32,9 +32,9 @@ var _void = function() {}, describe( 'dapi.AutoRetry trait', function() { /** - * If there are no failures, then AutoRetry has no observable effects. + * If there are no retries, then AutoRetry has no observable effects. */ - describe( 'when the request is successful', function() + describe( 'when the request does not need retrying', function() { it( 'makes only one request', function( done ) { @@ -55,19 +55,25 @@ describe( 'dapi.AutoRetry trait', function() } ); - it( 'returns the response data with no error', function( done ) + /** + * We expect that any error will be proxied back to us; this is an + * important concept, since it allow separating the idea of a + * "retry" from that of a "failure": the latter represents a problem + * with the request, whereas the former indicates that a request + * should be performed once again. + */ + it( 'returns the response data, including any error', function( done ) { - var chk = { foo: 'bar' }; + var chk = { foo: 'bar' }, + chk_err = Error( 'foo' ); - // notice that we provide an error to the stub; this will ensure - // that the returned error is null even when one is provided - var stub = _createStub( {}, chk ) + var stub = _createStub( chk_err, chk ) .use( Sut( _void, 1 ) ) (); stub.request( '', function( err, data ) { - expect( err ).to.equal( null ); + expect( err ).to.equal( chk_err ); expect( data ).to.equal( chk ); done(); } ); @@ -78,9 +84,9 @@ describe( 'dapi.AutoRetry trait', function() /** * This is when we care. */ - describe( 'when the request fails', function() + describe( 'when the retry predicate is true', function() { - it( 'will re-perform request N-1 times until failure', function( done ) + it( 'will re-perform request N-1 times until false', function( done ) { var n = 5; @@ -207,7 +213,7 @@ describe( 'dapi.AutoRetry trait', function() } ); - it( 'will call delay function until success', function() + it( 'will call delay function until predicate falsity', function() { var n = 5; var wait = function( tries_left, c ) From 1669f9eedab2b59cf46b2dee39b3b8ee518a12e3 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 28 May 2015 09:03:39 -0400 Subject: [PATCH 14/22] XhrHttpImpl#request invokes XHR send This should have always been the case; I am not sure what I was thinking, considering that the object is encapsulated. --- src/dapi/http/XhrHttpImpl.js | 4 +++- test/dapi/http/XhrHttpImplTest.js | 15 +-------------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/dapi/http/XhrHttpImpl.js b/src/dapi/http/XhrHttpImpl.js index e6ed598..3d9a630 100644 --- a/src/dapi/http/XhrHttpImpl.js +++ b/src/dapi/http/XhrHttpImpl.js @@ -1,7 +1,7 @@ /** * XMLHttpRequest HTTP protocol implementation * - * Copyright (C) 2015 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( 'XhrHttpImpl' ) { callback( err, resp ); } ); + + req.send( data ); } catch ( e ) { diff --git a/test/dapi/http/XhrHttpImplTest.js b/test/dapi/http/XhrHttpImplTest.js index eeebde3..70e1507 100644 --- a/test/dapi/http/XhrHttpImplTest.js +++ b/test/dapi/http/XhrHttpImplTest.js @@ -1,7 +1,7 @@ /** * Test case for XMLHttpRequest HTTP protocol implementation * - * Copyright (C) 2014 LoVullo Associates, Inc. + * Copyright (C) 2014, 2015 LoVullo Associates, Inc. * * This file is part of the Liza Data Collection Framework * @@ -127,9 +127,6 @@ describe( 'XhrHttpImpl', function() expect( resp ).to.equal( retdata ); done(); } ); - - // invoke callback - StubXhr.inst.send( src ); } ); @@ -154,8 +151,6 @@ describe( 'XhrHttpImpl', function() done(); } ); - - StubXhr.inst.send( '' ); } ); @@ -175,8 +170,6 @@ describe( 'XhrHttpImpl', function() expect( resp.data ).to.equal( reply ); done(); } ); - - StubXhr.inst.send( '' ); } ); } ); @@ -192,8 +185,6 @@ describe( 'XhrHttpImpl', function() expect( err ).to.equal( null ); done(); } ); - - StubXhr.inst.send( '' ); } ); @@ -217,8 +208,6 @@ describe( 'XhrHttpImpl', function() expect( err ).to.equal( null ); done(); } ); - - StubXhr.inst.send( '' ); } ); @@ -249,8 +238,6 @@ describe( 'XhrHttpImpl', function() done(); } ); - - StubXhr.inst.send( '' ); } ); From e8344e36bb921eb766db78899d3e44044e4f9d91 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 28 May 2015 09:42:00 -0400 Subject: [PATCH 15/22] dapi.format.JsonResponse response text output value on parse failure --- src/dapi/format/JsonResponse.js | 12 ++++++++--- test/dapi/format/JsonResponseTest.js | 32 +++++++++++++++++++++------- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/src/dapi/format/JsonResponse.js b/src/dapi/format/JsonResponse.js index db357e8..d0f08fe 100644 --- a/src/dapi/format/JsonResponse.js +++ b/src/dapi/format/JsonResponse.js @@ -33,7 +33,10 @@ module.exports = Trait( 'JsonResponse' ) /** * Processes response as JSON * - * Will return an error if the response is not valid JSON. + * If the response is not valid JSON, an error will be returned. The + * output value will be an object with a single + * property---`text`---containing the response text that failed to + * parse. * * @param {string} data binary data to transmit * @param {function(?Error,*)} callback continuation upon reply @@ -56,8 +59,11 @@ module.exports = Trait( 'JsonResponse' ) } catch ( e ) { - // parsing failed - callback( e, null ); + // parsing failed; provide response text in addition to + // original data so that the caller can handle how they + // please + callback( e, { text: resp } ); + return; } diff --git a/test/dapi/format/JsonResponseTest.js b/test/dapi/format/JsonResponseTest.js index 2932a56..9211105 100644 --- a/test/dapi/format/JsonResponseTest.js +++ b/test/dapi/format/JsonResponseTest.js @@ -55,15 +55,31 @@ describe( 'dapi.format.JsonRepsonse trait', function() } ); - it( 'returns error if JSON parsing fails', function( done ) + describe( 'when JSON parsing fails', function() { - _createStubbedDapi( null, 'ERR' ) - .request( '', function( err, data ) - { - expect( err ).to.be.instanceOf( Error ); - expect( data ).to.equal( null ); - done(); - } ); + it( 'returns error', function( done ) + { + _createStubbedDapi( null, 'ERR' ) + .request( '', function( err, data ) + { + expect( err ).to.be.instanceOf( Error ); + done(); + } ); + } ); + + + it( 'provides bad text as object.text', function( done ) + { + var text = 'NOT JSON'; + + _createStubbedDapi( null, text ) + .request( '', function( err, data ) + { + expect( data ).to.be.a( 'object' ); + expect( data.text ).to.equal( text ); + done(); + } ); + } ); } ); From 57886e66d061c99673553e5d3c697f7916351a8f Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 28 May 2015 09:43:20 -0400 Subject: [PATCH 16/22] JsonResponse test ensure parse error is of type `SyntaxError` Previously `Error`. --- test/dapi/format/JsonResponseTest.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dapi/format/JsonResponseTest.js b/test/dapi/format/JsonResponseTest.js index 9211105..6360562 100644 --- a/test/dapi/format/JsonResponseTest.js +++ b/test/dapi/format/JsonResponseTest.js @@ -62,7 +62,7 @@ describe( 'dapi.format.JsonRepsonse trait', function() _createStubbedDapi( null, 'ERR' ) .request( '', function( err, data ) { - expect( err ).to.be.instanceOf( Error ); + expect( err ).to.be.instanceOf( SyntaxError ); done(); } ); } ); From 8f9b8f779fb69414fef72a93d569a250fcf5c668 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 28 May 2015 11:03:57 -0400 Subject: [PATCH 17/22] JsonResponse retain output value on request error --- src/dapi/format/JsonResponse.js | 2 +- test/dapi/format/JsonResponseTest.js | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/dapi/format/JsonResponse.js b/src/dapi/format/JsonResponse.js index d0f08fe..49ebe9e 100644 --- a/src/dapi/format/JsonResponse.js +++ b/src/dapi/format/JsonResponse.js @@ -49,7 +49,7 @@ module.exports = Trait( 'JsonResponse' ) { if ( err !== null ) { - callback( err, null ); + callback( err, resp ); return; } diff --git a/test/dapi/format/JsonResponseTest.js b/test/dapi/format/JsonResponseTest.js index 6360562..633a93f 100644 --- a/test/dapi/format/JsonResponseTest.js +++ b/test/dapi/format/JsonResponseTest.js @@ -85,14 +85,15 @@ describe( 'dapi.format.JsonRepsonse trait', function() it( 'proxy error from encapsulated DataApi', function( done ) { - var e = Error( 'foo' ); + var e = Error( 'foo' ), + chk = {}; - _createStubbedDapi( e, '0' ) + _createStubbedDapi( e, chk ) .request( '', function( err, data ) { // data should also be cleared out expect( err ).to.equal( e ); - expect( data ).to.equal( null ); + expect( data ).to.equal( chk ); done(); } ); } ); From ca68d12370e6505cd6faaccac817d06d5f095275 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 28 May 2015 12:07:28 -0400 Subject: [PATCH 18/22] XhrHttpImpl no longer modifying response text Status code included with error object --- src/dapi/http/XhrHttpImpl.js | 17 ++++++++--------- test/dapi/http/XhrHttpImplTest.js | 13 ++++++++----- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/dapi/http/XhrHttpImpl.js b/src/dapi/http/XhrHttpImpl.js index 3d9a630..ea0c914 100644 --- a/src/dapi/http/XhrHttpImpl.js +++ b/src/dapi/http/XhrHttpImpl.js @@ -168,8 +168,10 @@ module.exports = Class( 'XhrHttpImpl' ) /** * Serve an error response * - * The default behavior is to return an Error and content containing the - * HTTP status and response text. + * The default behavior is to return an Error with the status code as a + * `status` property, and the original response text as the output + * value; the philosophy here is that we should never modify the output, + * since a certain format may be expected as the result. * * When overriding this method, keep in mind that it should always * return an Error for the first argument, or set it to null, indicating @@ -187,12 +189,9 @@ module.exports = Class( 'XhrHttpImpl' ) */ 'virtual protected serveError': function( req, callback ) { - callback( - Error( req.status + " error from server" ), - { - status: req.status, - data: req.responseText - } - ); + var e = Error( req.status + " error from server" ); + e.status = req.status; + + callback( e, req.responseText ); } } ); diff --git a/test/dapi/http/XhrHttpImplTest.js b/test/dapi/http/XhrHttpImplTest.js index 70e1507..61a26fb 100644 --- a/test/dapi/http/XhrHttpImplTest.js +++ b/test/dapi/http/XhrHttpImplTest.js @@ -136,7 +136,7 @@ describe( 'XhrHttpImpl', function() * This is the default behavior, but can be changed by overriding * the onLoad method. */ - it( 'returns an error to the callback', function( done ) + it( 'returns error to callback with status code', function( done ) { var StubXhr = createStubXhr(); StubXhr.prototype.status = 404; @@ -145,29 +145,32 @@ describe( 'XhrHttpImpl', function() .requestData( 'http://foo', 'GET', '', function( err, _ ) { expect( err ).to.be.instanceOf( Error ); + expect( err.message ).to.contain( StubXhr.prototype.status ); + expect( err.status ).to.equal( + StubXhr.prototype.status + ); + done(); } ); } ); - it( 'returns response text with error code', function( done ) + it( 'returns response text as output', function( done ) { var StubXhr = createStubXhr(), status = 404, reply = 'foobunny'; - StubXhr.prototype.status = status; StubXhr.prototype.responseText = reply; Sut( StubXhr ) .requestData( 'http://foo', 'GET', '', function( _, resp ) { - expect( resp.status ).to.equal( status ); - expect( resp.data ).to.equal( reply ); + expect( resp ).to.equal( reply ); done(); } ); } ); From 881e740bb9de26e9d62b9806560c833efddf397a Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 28 May 2015 12:16:02 -0400 Subject: [PATCH 19/22] JsonResponse returns parse error or combined request/parse on failure --- src/dapi/format/JsonResponse.js | 84 +++++++++++++++++++++------- test/dapi/format/JsonResponseTest.js | 57 +++++++++++++++---- 2 files changed, 109 insertions(+), 32 deletions(-) diff --git a/src/dapi/format/JsonResponse.js b/src/dapi/format/JsonResponse.js index 49ebe9e..b88ad9a 100644 --- a/src/dapi/format/JsonResponse.js +++ b/src/dapi/format/JsonResponse.js @@ -38,6 +38,10 @@ module.exports = Trait( 'JsonResponse' ) * property---`text`---containing the response text that failed to * parse. * + * If a request error occurs in conjunction with a parse error, then + * both errors will be returned in a single error object under the + * `list` property. + * * @param {string} data binary data to transmit * @param {function(?Error,*)} callback continuation upon reply * @@ -45,32 +49,70 @@ module.exports = Trait( 'JsonResponse' ) */ 'virtual abstract override public request': function( data, callback ) { + var _self = this; + this.__super( data, function( err, resp ) { - if ( err !== null ) - { - callback( err, resp ); - return; - } - - try - { - var data = JSON.parse( resp ); - } - catch ( e ) - { - // parsing failed; provide response text in addition to - // original data so that the caller can handle how they - // please - callback( e, { text: resp } ); - - return; - } - - callback( null, data ); + _self._tryParse( err, resp, callback ); } ); return this; + }, + + + /** + * Attempt to parse SRC as JSON and invoke callback according to the + * rules of `#request` + * + * @param {?Error} err response error + * @param {string} src JSON string + * @param {function(?Error,*)} callback continuation + * + * @return {undefined} + */ + 'private _tryParse': function( err, src, callback ) + { + try + { + var data = JSON.parse( src ); + } + catch ( e ) + { + // parsing failed; provide response text in addition to + // original data so that the caller can handle how they + // please + callback( + this._getReturnError( err, e ), + { text: src } + ); + + return; + } + + callback( err, data ); + }, + + + /** + * Produce the parse error, or a combined error containing both the + * original and parse errors + * + * @param {?Error} orig response error + * @param {Error} parse parse error + * + * @return {Error} parse error or combined error + */ + 'private _getReturnError': function( orig, parse ) + { + if ( !orig ) + { + return parse; + } + + var e = Error( "Multiple errors occurred; see `list` property" ); + e.list = [ orig, parse ]; + + return e; } } ); diff --git a/test/dapi/format/JsonResponseTest.js b/test/dapi/format/JsonResponseTest.js index 633a93f..f74ea50 100644 --- a/test/dapi/format/JsonResponseTest.js +++ b/test/dapi/format/JsonResponseTest.js @@ -83,19 +83,54 @@ describe( 'dapi.format.JsonRepsonse trait', function() } ); - it( 'proxy error from encapsulated DataApi', function( done ) + describe( 'on request error from supertype', function() { - var e = Error( 'foo' ), - chk = {}; + it( 'attempts to format output as JSON', function( done ) + { + var chk = '{"foo": "bar"}'; - _createStubbedDapi( e, chk ) - .request( '', function( err, data ) - { - // data should also be cleared out - expect( err ).to.equal( e ); - expect( data ).to.equal( chk ); - done(); - } ); + _createStubbedDapi( null, chk ) + .request( '', function( _, data ) + { + expect( data ).to.be.a( 'object' ); + expect( data.foo ).to.equal( "bar" ); + done(); + } ); + } ); + + + it( 'proxies error when JSON output valid', function( done ) + { + var e = Error( 'foo' ); + + _createStubbedDapi( e, '{}' ) + .request( '', function( err, _ ) + { + expect( err ).to.equal( e ); + done(); + } ); + } ); + + + it( 'produces both errors on bad JSON output', function( done ) + { + var e = Error( 'foo' ); + + _createStubbedDapi( e, 'BAD JSON' ) + .request( '', function( err, _ ) + { + // the main error should indicate both + expect( err ).to.be.instanceOf( Error ); + + // and we should provide references to both + expect( err.list[ 0 ] ).to.equal( e ); + expect( err.list[ 1 ] ).to.be.instanceOf( + SyntaxError + ); + + done(); + } ); + } ); } ); } ); } ); From f122d8593887cb81e429f9f6eaf1e523a16811e4 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 29 May 2015 09:35:04 -0400 Subject: [PATCH 20/22] XhrHttpImpl modify URL for GET request params and encode key-value Some of this may be more appropriate to move out of XhrHttpImpl into something like HttpDataApi... --- src/dapi/http/XhrHttpImpl.js | 121 ++++++++++++++++++++- test/dapi/http/XhrHttpImplTest.js | 170 +++++++++++++++++++++++++++--- 2 files changed, 273 insertions(+), 18 deletions(-) diff --git a/src/dapi/http/XhrHttpImpl.js b/src/dapi/http/XhrHttpImpl.js index ea0c914..ea4cc9f 100644 --- a/src/dapi/http/XhrHttpImpl.js +++ b/src/dapi/http/XhrHttpImpl.js @@ -42,6 +42,8 @@ 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 ) @@ -53,14 +55,26 @@ module.exports = Class( 'XhrHttpImpl' ) /** * Perform HTTP request using the standard XMLHttpRequest * - * @param {Object|string} data request params + * 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. + * + * @param {string} url base request URL + * @param {string} method RFC-2616-compliant HTTP method + * + * @param {?Object|string=} data request params or + * post data + * * @param {function(Error, Object)} callback server response callback * * @return {HttpImpl} self */ 'public requestData': function( url, method, data, callback ) { - var req = new this._Xhr(); + var req = new this._Xhr(), + url = this._genUrl( url, method, data ); try { @@ -70,7 +84,7 @@ module.exports = Class( 'XhrHttpImpl' ) callback( err, resp ); } ); - req.send( data ); + req.send( this._getSendData( method, data ) ); } catch ( e ) { @@ -81,6 +95,107 @@ module.exports = Class( 'XhrHttpImpl' ) }, + /** + * Generate URL according to METHOD and provided DATA + * + * See `#requestData` for more information. + * + * @param {string} url base request URL + * @param {string} method RFC-2616-compliant HTTP method + * + * @param {?Object|string=} data request params or + * post data + * + * @return {string} original URL, or appended with params + */ + 'private _genUrl': function( url, method, data ) + { + if ( method !== 'GET' ) + { + 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 ) + : '' + ); + }, + + + /** + * 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 + * + * If method is GET, no data are posted + * + * @param {string} url base request URL + * @param {?Object|string=} data post data + * + * @return {string|undefined} data to post to server + */ + 'private _getSendData': function( method, data ) + { + if ( method === 'GET' ) + { + return undefined; + } + + // TODO: reject nonsense types, including arrays + switch ( typeof data ) + { + case 'object': + return this._encodeKeys( data ); + + default: + return data; + } + }, + + /** * Prepares a request to the given URL using the given HTTP method * diff --git a/test/dapi/http/XhrHttpImplTest.js b/test/dapi/http/XhrHttpImplTest.js index 61a26fb..0a6e780 100644 --- a/test/dapi/http/XhrHttpImplTest.js +++ b/test/dapi/http/XhrHttpImplTest.js @@ -31,7 +31,9 @@ var dapi = require( '../../../' ).dapi, { DummyXhr.args = arguments; }; - }; + }, + + _void = function() {}; describe( 'XhrHttpImpl', function() @@ -58,18 +60,15 @@ describe( 'XhrHttpImpl', function() describe( '.requestData', function() { - it( 'requests a connection using the given url and method', function() + it( 'requests a connection using the given method', function() { - var url = 'http://foonugget', - method = 'GET', + var method = 'GET', sut = Sut( DummyXhr ); - sut.requestData( url, method, {}, function() {} ); + sut.requestData( 'http://foo', method, {}, function() {} ); var args = DummyXhr.args; expect( args[ 0 ] ).to.equal( method ); - expect( args[ 1 ] ).to.equal( url ); - expect( args[ 1 ] ).to.be.ok; // async } ); @@ -107,21 +106,14 @@ describe( 'XhrHttpImpl', function() it( 'returns XHR response via callback when no error', function( done ) { var retdata = "foobar", - src = "moocow", StubXhr = createStubXhr(); StubXhr.prototype.responseText = retdata; StubXhr.prototype.readyState = 4; // done StubXhr.prototype.status = 200; // OK - StubXhr.prototype.send = function( data ) - { - expect( data ).is.equal( src ); - StubXhr.inst.onreadystatechange(); - }; - Sut( StubXhr ) - .requestData( 'http://bar', 'GET', src, function( err, resp ) + .requestData( 'http://bar', 'GET', {}, function( err, resp ) { expect( err ).to.equal( null ); expect( resp ).to.equal( retdata ); @@ -130,6 +122,154 @@ describe( 'XhrHttpImpl', function() } ); + describe( 'HTTP method is GET', function() + { + it( 'appends encoded non-obj data to URL', function( done ) + { + var url = 'http://bar', + src = "moocow %foocow%", + StubXhr = createStubXhr(); + + StubXhr.prototype.readyState = 4; // done + StubXhr.prototype.status = 200; // OK + + StubXhr.prototype.open = function( _, given_url ) + { + expect( given_url ).to.equal( + url + '?' + encodeURI( src ) + ); + }; + + 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( '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 ) + { + var url = 'http://bar', + 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 ); + }; + + Sut( StubXhr ) + .requestData( url, 'GET', undefined, _void ) + .requestData( url, 'GET', null, _void ) + .requestData( url, 'GET', "", _void ) + .requestData( url, 'GET', {}, done ); + } ); + + } ); + + + describe( 'HTTP method is not GET', function() + { + it( 'posts non-object data verbatim', function( done ) + { + var url = 'http://bar', + src = "moocow", + StubXhr = createStubXhr(); + + StubXhr.prototype.readyState = 4; // done + StubXhr.prototype.status = 200; // OK + + StubXhr.prototype.open = function( _, given_url ) + { + // URL should be unchanged + expect( given_url ).to.equal( url ); + }; + + StubXhr.prototype.send = function( data ) + { + expect( data ).is.equal( src ); + StubXhr.inst.onreadystatechange(); + }; + + 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 ); + } ); + } ); + + describe( 'if return status code is not successful', function() { /** From 0599acc23ac523714d2275d78fa2321e28cab8f5 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 29 May 2015 09:59:12 -0400 Subject: [PATCH 21/22] Key-value param encoding moved from XhrHttpImpl to HttpDataApi --- src/dapi/http/HttpDataApi.js | 69 +++++++++++++++++++++-- src/dapi/http/XhrHttpImpl.js | 64 ++++----------------- test/dapi/http/HttpDataApiTest.js | 39 ++++++++++++- test/dapi/http/XhrHttpImplTest.js | 94 +++++-------------------------- 4 files changed, 127 insertions(+), 139 deletions(-) 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 ); From a383a36bfc59198739d1c4b4c60db8a679f1e1d2 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Fri, 29 May 2015 12:45:02 -0400 Subject: [PATCH 22/22] HttpDataApi#request permits null data --- src/dapi/http/HttpDataApi.js | 14 ++++++++++---- test/dapi/http/HttpDataApiTest.js | 15 ++++++++++++++- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/dapi/http/HttpDataApi.js b/src/dapi/http/HttpDataApi.js index 79cf839..e2debb6 100644 --- a/src/dapi/http/HttpDataApi.js +++ b/src/dapi/http/HttpDataApi.js @@ -96,7 +96,8 @@ module.exports = Class( 'HttpDataApi' ) * 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. + * ampersands. `null` may be used to indicate that no data should be + * sent. * * 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. @@ -114,6 +115,13 @@ module.exports = Class( 'HttpDataApi' ) */ 'virtual public request': function( data, callback ) { + // null is a good indicator of "I have no intent to send any data"; + // empty strings and objects are not, since those are valid data + if ( data === null ) + { + data = ""; + } + this._validateDataType( data ); this._impl.requestData( @@ -158,9 +166,7 @@ module.exports = Class( 'HttpDataApi' ) { var type = typeof data; - if ( ( data === null ) - || !( ( type === 'string' ) || ( type === 'object' ) ) - ) + if( !( ( type === 'string' ) || ( type === 'object' ) ) ) { throw TypeError( "Data must be a string of raw data or object containing " + diff --git a/test/dapi/http/HttpDataApiTest.js b/test/dapi/http/HttpDataApiTest.js index c1a7dac..a5cdba2 100644 --- a/test/dapi/http/HttpDataApiTest.js +++ b/test/dapi/http/HttpDataApiTest.js @@ -175,11 +175,24 @@ describe( 'HttpDataApi', function() } ); + it( 'accepts null data, converting to empty string', function() + { + expect( function() + { + Sut( dummy_url, 'GET', impl ) + .request( null, function() + { + expect( impl.provided[ 2 ] ).to.equal( "" ); + } ); + } ).to.not.throw( Error ); + } ); + + it( 'rejects all other data types', function() { var sut = Sut( dummy_url, 'GET', impl ); - [ 123, null, Infinity, undefined, NaN, function() {} ] + [ 123, Infinity, undefined, NaN, function() {} ] .forEach( function( data ) { expect( function()