From 25f4ba161d934d0d1a0672936c82750c55811aeb Mon Sep 17 00:00:00 2001 From: Horia Radu Date: Thu, 21 Jul 2016 13:06:12 +0300 Subject: [PATCH] Stringify object query params as JSON [2.x] Stringify query params which are objects in a way in which empty ararys and null values are preserved instead of removed (default querystring implementation). backport of: https://github.com/strongloop/strong-remoting/pull/325 --- lib/http-invocation.js | 13 ++- test/http-invocation.test.js | 202 ++++++++++++++++++++++++++++++----- 2 files changed, 189 insertions(+), 26 deletions(-) diff --git a/lib/http-invocation.js b/lib/http-invocation.js index 0839dcfd..f6cc97a7 100644 --- a/lib/http-invocation.js +++ b/lib/http-invocation.js @@ -126,7 +126,7 @@ HttpInvocation.prototype._processArg = function(req, verb, query, accept) { // From the query string if (val !== undefined) { query = query || {}; - query[name] = val; + query[name] = serializeQueryStringValue(val, accept); } break; case 'header': @@ -146,7 +146,7 @@ HttpInvocation.prototype._processArg = function(req, verb, query, accept) { // default to query string for GET if (val !== undefined) { query = query || {}; - query[name] = val; + query[name] = serializeQueryStringValue(val, accept); } } else { // default to storing args on the body for !GET @@ -157,6 +157,15 @@ HttpInvocation.prototype._processArg = function(req, verb, query, accept) { return query; }; +function serializeQueryStringValue(val, accept) { + if ((accept.type === 'object' || accept.type === 'string') && + typeof val === 'object') { + return JSON.stringify(val); + } else { + return val; + } +} + /** * Build args object from the http context's `req` and `res`. */ diff --git a/test/http-invocation.test.js b/test/http-invocation.test.js index 807b88d7..a7b00310 100644 --- a/test/http-invocation.test.js +++ b/test/http-invocation.test.js @@ -15,33 +15,33 @@ describe('HttpInvocation', function() { function expectNamedArgs(accepts, inputArgs, expectedNamedArgs) { var method = givenSharedStaticMethod({ - accepts: accepts + accepts: accepts, }); - var inv = new HttpInvocation(method, null, inputArgs); + var inv = givenInvocation(method, { args: inputArgs }); expect(inv.namedArgs).to.deep.equal(expectedNamedArgs); } it('should correctly name a single arg', function() { expectNamedArgs( - [{arg: 'a', type: 'number'}], + [{ arg: 'a', type: 'number' }], [1], - {a: 1} + { a: 1 } ); }); it('should correctly name multiple args', function() { expectNamedArgs( - [{arg: 'a', type: 'number'}, {arg: 'str', type: 'string'}], + [{ arg: 'a', type: 'number' }, { arg: 'str', type: 'string' }], [1, 'foo'], - {a: 1, str: 'foo'} + { a: 1, str: 'foo' } ); }); it('should correctly name multiple args when a partial set is provided', function() { expectNamedArgs( - [{arg: 'a', type: 'number'}, {arg: 'str', type: 'string'}], + [{ arg: 'a', type: 'number' }, { arg: 'str', type: 'string' }], [1], - {a: 1} + { a: 1 } ); }); @@ -49,7 +49,7 @@ describe('HttpInvocation', function() { it('should accept an acceptable argument', function() { var acceptable = HttpInvocation.isAcceptable(2, { arg: 'foo', - type: 'number' + type: 'number', }); expect(acceptable).to.equal(true); }); @@ -57,7 +57,7 @@ describe('HttpInvocation', function() { it('should always accept args when type is any', function() { var acceptable = HttpInvocation.isAcceptable(2, { arg: 'bar', - type: 'any' + type: 'any', }); expect(acceptable).to.equal(true); }); @@ -65,7 +65,7 @@ describe('HttpInvocation', function() { it('should always accept args when type is complex', function() { var acceptable = HttpInvocation.isAcceptable({}, { arg: 'bar', - type: 'MyComplexType' + type: 'MyComplexType', }); expect(acceptable).to.equal(true); }); @@ -73,7 +73,7 @@ describe('HttpInvocation', function() { it('should accept null arg when type is complex', function() { var acceptable = HttpInvocation.isAcceptable(null, { arg: 'bar', - type: 'MyComplexType' + type: 'MyComplexType', }); expect(acceptable).to.equal(true); }); @@ -93,11 +93,11 @@ describe('HttpInvocation', function() { setupReturnTypes({ arg: 'data', type: 'bar', - root: true + root: true, }, 'bar', function(data) { return data ? new TestClass(data) : data; }, { - body: { foo: 'bar' } + body: { foo: 'bar' }, }, function(err, inst) { expect(err).to.be.null; expect(inst).to.be.instanceOf(TestClass); @@ -114,14 +114,14 @@ describe('HttpInvocation', function() { setupReturnTypes({ arg: 'data', type: ['bar'], - root: true + root: true, }, 'bar', function(data) { return data ? new TestClass(data) : data; }, { body: [ { foo: 'bar' }, - { foo: 'grok' } - ] + { foo: 'grok' }, + ], }, function(err, insts) { expect(err).to.be.null; expect(insts).to.be.an('array'); @@ -139,7 +139,7 @@ describe('HttpInvocation', function() { it('should forward all error properties', function(done) { var method = givenSharedStaticMethod({}); - var inv = new HttpInvocation(method); + var inv = givenInvocation(method); var res = { statusCode: 555, body: { @@ -148,11 +148,11 @@ describe('HttpInvocation', function() { message: 'Custom error message', statusCode: 555, details: { - key: 'value' + key: 'value', }, - extra: 'extra value' - } - } + extra: 'extra value', + }, + }, }; inv.transformResponse(res, res.body, function(err) { @@ -170,10 +170,10 @@ describe('HttpInvocation', function() { it('should forward statusCode and non-object error response', function(done) { var method = givenSharedStaticMethod({}); - var inv = new HttpInvocation(method); + var inv = givenInvocation(method); var res = { statusCode: 555, - body: 'error body' + body: 'error body', }; inv.transformResponse(res, res.body, function(err) { @@ -186,6 +186,133 @@ describe('HttpInvocation', function() { }); }); }); + + describe('createRequest', function() { + it('creates a simple request', function() { + var inv = givenInvocationForEndpoint(null, []); + var expectedReq = { method: 'GET', + url: 'http://base/testModel/testMethod', + protocol: 'http:', + json: true, + }; + expect(inv.createRequest()).to.eql(expectedReq); + }); + + it('makes primitive type arguments as query params', function() { + var accepts = [ + { arg: 'a', type: 'number' }, + { arg: 'b', type: 'string' }, + ]; + var aValue = 2; + var bValue = 'foo'; + var inv = givenInvocationForEndpoint(accepts, [aValue, bValue]); + var expectedReq = { method: 'GET', + url: 'http://base/testModel/testMethod?a=2&b=foo', + protocol: 'http:', + json: true, + }; + expect(inv.createRequest()).to.eql(expectedReq); + }); + + it('makes an array argument as a query param', function() { + var accepts = [ + { arg: 'a', type: 'object' }, + ]; + var aValue = [1, 2, 3]; + var inv = givenInvocationForEndpoint(accepts, [aValue]); + var expectedReq = { method: 'GET', + url: 'http://base/testModel/testMethod?a=' + encodeURIComponent('[1,2,3]'), + protocol: 'http:', + json: true, + }; + expect(inv.createRequest()).to.eql(expectedReq); + }); + + it('keeps an empty array as a query param', function() { + var accepts = [ + { arg: 'a', type: 'object' }, + ]; + var aValue = []; + var inv = givenInvocationForEndpoint(accepts, [aValue]); + var expectedReq = { method: 'GET', + url: 'http://base/testModel/testMethod?a=' + encodeURIComponent('[]'), + protocol: 'http:', + json: true, + }; + expect(inv.createRequest()).to.eql(expectedReq); + }); + + it('keeps an empty array as a body param for a POST request', function() { + var accepts = [ + { arg: 'a', type: 'object' }, + ]; + var aValue = []; + var inv = givenInvocationForEndpoint(accepts, [aValue], 'POST'); + var expectedReq = { method: 'POST', + url: 'http://base/testModel/testMethod', + protocol: 'http:', + json: true, + body: { + a: [], + }, + }; + expect(inv.createRequest()).to.eql(expectedReq); + }); + + it('handles a loopback filter as a query param', function() { + var accepts = [ + { arg: 'filter', type: 'object' }, + ]; + var filter = { + where: { + id: { + inq: [1, 2], + }, + typeId: { + inq: [], + }, + }, + include: ['related'], + }; + var inv = givenInvocationForEndpoint(accepts, [filter]); + var expectedFilter = + '{"where":{"id":{"inq":[1,2]},"typeId":{"inq":[]}},"include":["related"]}'; + var expectedReq = { method: 'GET', + url: 'http://base/testModel/testMethod?filter=' + + encodeURIComponent(expectedFilter), + protocol: 'http:', + json: true, + }; + expect(inv.createRequest()).to.eql(expectedReq); + }); + }); + + it('handles a loopback filter as a body param for a POST request', function() { + var accepts = [ + { arg: 'filter', type: 'object' }, + ]; + var filter = { + where: { + id: { + inq: [1, 2], + }, + typeId: { + inq: [], + }, + }, + include: ['related'], + }; + var inv = givenInvocationForEndpoint(accepts, [filter], 'POST'); + var expectedReq = { method: 'POST', + url: 'http://base/testModel/testMethod', + protocol: 'http:', + json: true, + body: { + filter: filter, + }, + }; + expect(inv.createRequest()).to.eql(expectedReq); + }); }); function givenSharedStaticMethod(fn, config) { @@ -200,3 +327,30 @@ function givenSharedStaticMethod(fn, config) { extend(testClass.testMethod, config); return SharedMethod.fromFunction(fn, 'testStaticMethodName', null, true); } + +function givenInvocation(method, params) { + params = params || {}; + return new HttpInvocation(method, + params.ctorArgs, + params.args, + params.baseUrl, + params.auth); +} + +function givenInvocationForEndpoint(accepts, args, verb) { + var method = givenSharedStaticMethod({ + accepts: accepts, + }); + method.getEndpoints = function() { + return [createEndpoint({ verb: verb || 'GET' })]; + }; + return givenInvocation(method, { ctorArgs: [], args: args, baseUrl: 'http://base' }); +} + +function createEndpoint(config) { + config = config || {}; + return { + verb: config.verb || 'GET', + fullPath: config.fullPath || '/testModel/testMethod', + }; +}