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

@@ -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,string>|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,string>|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,string>|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,string>|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
      *

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',
+            var method = 'GET',
-                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,13 +106,125 @@ 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
 
+            Sut( StubXhr )
+                .requestData( 'http://bar', 'GET', {}, function( err, resp )
+                {
+                    expect( err ).to.equal( null );
+                    expect( resp ).to.equal( retdata );
+                    done();
+                } );
+        } );
+
+
+        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 );
@@ -121,11 +232,40 @@ describe( 'XhrHttpImpl', function()
                 };
 
                 Sut( StubXhr )
-                .requestData( 'http://bar', 'GET', src, function( err, resp )
+                    .requestData( url, 'POST', src, done );
+            } );
+
+
+            it( 'encodes key-value data', function( done )
             {
-                    expect( err ).to.equal( null );
+                var url     = 'http://bar',
-                    expect( resp ).to.equal( retdata );
+                    src     = { foo: "bar=baz", bar: "moo%cow" },
-                    done();
+                    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 );
             } );
         } );