Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

feat(http): add multiple parameters serializers #7423

Closed
wants to merge 1 commit into from
Closed

feat(http): add multiple parameters serializers #7423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 77 additions & 22 deletions src/ng/http.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ function $HttpProvider() {
},

xsrfCookieName: 'XSRF-TOKEN',
xsrfHeaderName: 'X-XSRF-TOKEN'
xsrfHeaderName: 'X-XSRF-TOKEN',
paramsSerializer: 'keyValue'
};

/**
Expand All @@ -126,6 +127,62 @@ function $HttpProvider() {
*/
var interceptorFactories = this.interceptors = [];

/**
* The URL parameters serializers
*/
var paramsSerializers = this.paramsSerializers = {
keyValue: keyValueParametersSerializer,
JSON: jsonParametersSerializer,
nonShallowJSON: nonShallowJSONParametersSerializer,
_default: keyValueParametersSerializer
};

function keyValueParametersSerializer(params, callback) {
forEachSorted(params, function(value, key) {
if (value === null || isUndefined(value)) return;
if (!isArray(value)) value = [value];

forEach(value, function(v) {
if (isObject(v)) {
v = toJson(v);
}
callback(key, v);
});
});
}

function jsonParametersSerializer(params, callback) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(as far as I'm aware,) no backend expects a JSON query string, that would be crazy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it makes sense to ship more than one of these in core, we should probably just keep doing what we're already doing in core, and give people the option to extend it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, that would be less code (and I always like less code)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it makes sense to ship more than one of these in core, we should probably just keep doing what we're already doing in core, and give people the option to extend it

Mmm... not even the one that is asked at #7363 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want jQuery behaviour, you could easily just say

$httpProvider.querySerializer('jquery', jQuery.param);
$httpProvider.defaultQuerySerializer = 'jquery';

to use jQuery's style automatically --- or if you didn't want to include jQuery, you could probably come up with a variation on it

forEachSorted(params, function(value, key) {
if (value === null || isUndefined(value)) return;
if (isArray(value) || isObject(value)) {
value = toJson(value);
}
callback(key, value);
});
}

function nonShallowJSONParametersSerializer(params, callback) {
function serialize(toSerialize, prefix, topLevel) {
if (toSerialize === null || isUndefined(toSerialize)) return;
if (isArray(toSerialize)) {
forEach(toSerialize, function(value) {
serialize(value, prefix + '[]');
});
} else if (isObject(toSerialize)) {
forEachSorted(toSerialize, function(value, key) {
serialize(value, prefix +
(topLevel ? '' : '[') +
key +
(topLevel ? '' : ']'));
});
} else {
callback(prefix, toSerialize);
}
}

serialize(params, '', true);
}

this.$get = ['$httpBackend', '$browser', '$cacheFactory', '$rootScope', '$q', '$injector',
function($httpBackend, $browser, $cacheFactory, $rootScope, $q, $injector) {

Expand Down Expand Up @@ -594,7 +651,8 @@ function $HttpProvider() {
var config = {
method: 'get',
transformRequest: defaults.transformRequest,
transformResponse: defaults.transformResponse
transformResponse: defaults.transformResponse,
paramsSerializer: defaults.paramsSerializer
};
var headers = mergeHeaders(requestConfig);

Expand Down Expand Up @@ -852,7 +910,7 @@ function $HttpProvider() {
promise = deferred.promise,
cache,
cachedResp,
url = buildUrl(config.url, config.params);
url = buildUrl(config.url, config.params, config.paramsSerializer);

$http.pendingRequests.push(config);
promise.then(removePendingReq, removePendingReq);
Expand Down Expand Up @@ -939,26 +997,23 @@ function $HttpProvider() {
}


function buildUrl(url, params) {
if (!params) return url;
var parts = [];
forEachSorted(params, function(value, key) {
if (value === null || isUndefined(value)) return;
if (!isArray(value)) value = [value];
function buildUrl(url, params, paramsSerializer) {
if (!params) return url;

forEach(value, function(v) {
if (isObject(v)) {
v = toJson(v);
}
parts.push(encodeUriQuery(key) + '=' +
encodeUriQuery(v));
});
});
if(parts.length > 0) {
url += ((url.indexOf('?') == -1) ? '?' : '&') + parts.join('&');
}
return url;
}
var parts = [];
if (!isFunction(paramsSerializer)) {
paramsSerializer = paramsSerializers[paramsSerializer] ||
paramsSerializers._default;
}

paramsSerializer(params, function(key, value) {
parts.push(encodeUriQuery(key) + '=' + encodeUriQuery(value));
});
if(parts.length > 0) {
url += ((url.indexOf('?') == -1) ? '?' : '&') + parts.join('&');
}
return url;
}


}];
Expand Down
90 changes: 90 additions & 0 deletions test/ng/httpSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,12 @@ describe('$http', function() {
describe('the instance', function() {
var $httpBackend, $http, $rootScope;

beforeEach(module(function($httpProvider) {
$httpProvider.paramsSerializers.custom = function(params, callback) {
callback('foo', 'bar');
};
}));

beforeEach(inject(['$rootScope', function($rs) {
$rootScope = $rs;

Expand Down Expand Up @@ -341,6 +347,90 @@ describe('$http', function() {
$httpBackend.expect('GET', '/url').respond('');
$http({url: '/url', params: {}, method: 'GET'});
});

describe('params serialization', function() {
it('should be possible to use key-value serialization', inject(function($httpBackend, $http) {
var testCases = [
{
params: {a: 1, b: 2},
serialized: 'a=1&b=2'
},
{
params: {a: 1, b: [2, 3]},
serialized: 'a=1&b=2&b=3'
},
{
params: {a: 'abc', b: {foo: 'bar'}},
serialized: 'a=abc&b=%7B%22foo%22:%22bar%22%7D'
}
];
angular.forEach(testCases, function(testCase) {
$httpBackend.expect('GET', '/url?' + testCase.serialized).respond('');
$http({url: '/url', params: testCase.params, method: 'GET', paramsSerializer: 'keyValue'});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like the option name paramSerializer, it seems a bit verbose and awkward to use

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also want a way to specify a "default" param serializer, since most of your requests would be talking to your origin server, or at least a specific main web service

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a big fan of the paramSerializer name, I just cannot think of another name that would express exactly what it does

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can change the default parameter serializer by changing $httpProvider.defaults. paramsSerializer

});
}));

it('should be possible to use JSON serialization', inject(function($httpBackend, $http) {
var testCases = [
{
params: {a: 1, b: 2},
serialized: 'a=1&b=2'
},
{
params: {a: 1, b: [2, 3]},
serialized: 'a=1&b=%5B2,3%5D'
},
{
params: {a: 'abc', b: {foo: 'bar'}},
serialized: 'a=abc&b=%7B%22foo%22:%22bar%22%7D'
}
];
angular.forEach(testCases, function(testCase) {
$httpBackend.expect('GET', '/url?' + testCase.serialized).respond('');
$http({url: '/url', params: testCase.params, method: 'GET', paramsSerializer: 'JSON'});
});
}));

it('should be possible to use nonShallowJSON serialization', inject(function($httpBackend, $http) {
var testCases = [
{
params: {a: 1, b: 2},
serialized: 'a=1&b=2'
},
{
params: {a: 1, b: [2, 3]},
serialized: 'a=1&b%5B%5D=2&b%5B%5D=3' // a=1&b[]=2&b[]=3
},
{
params: {a: 1, b: [2, {foo: 'bar'}]},
serialized: 'a=1&b%5B%5D=2&b%5B%5D%5Bfoo%5D=bar' // a=1&b[]=2&b[][foo]=bar
},
{
params: {a: 'abc', b: {foo: 'bar', man: 'shell'}},
serialized: 'a=abc&b%5Bfoo%5D=bar&b%5Bman%5D=shell' // a=abc&b[foo]=bar&b[man]=shell
}
];
angular.forEach(testCases, function(testCase) {
$httpBackend.expect('GET', '/url?' + testCase.serialized).respond('');
$http({url: '/url', params: testCase.params, method: 'GET', paramsSerializer: 'nonShallowJSON'});
});

}));

it('should be possible to use a custom serialization', inject(function($httpBackend, $http) {
function serializer(params, callback) {
callback('foo', 'bar');
}

$httpBackend.expect('GET', '/url?foo=bar').respond('');
$http({url: '/url', params: {}, method: 'GET', paramsSerializer: serializer});
}));

it('should be possible to use a predefined custom serializer', inject(function($httpBackend, $http) {
$httpBackend.expect('GET', '/url?foo=bar').respond('');
$http({url: '/url', params: {}, method: 'GET', paramsSerializer: 'custom'});
}));
});
});


Expand Down