Skip to content

Commit c1a335d

Browse files
refact($http): use the $jsonpCallbacks service
Use the built-in service to handling callbacks to `$http.jsonp` requests. Closes angular#3073
1 parent 5253d65 commit c1a335d

File tree

3 files changed

+49
-26
lines changed

3 files changed

+49
-26
lines changed

src/ng/http.js

+2
Original file line numberDiff line numberDiff line change
@@ -1122,6 +1122,8 @@ function $HttpProvider() {
11221122
*
11231123
* @description
11241124
* Shortcut method to perform `JSONP` request.
1125+
* If you would like to customise where and how the callbacks are stored then try overriding
1126+
* or decorating the {@link jsonpCallbacks} service.
11251127
*
11261128
* @param {string} url Relative or absolute URL specifying the destination of the request.
11271129
* The name of the callback should be the string `JSON_CALLBACK`.

src/ng/httpBackend.js

+11-15
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ function $xhrFactoryProvider() {
3232
/**
3333
* @ngdoc service
3434
* @name $httpBackend
35-
* @requires $window
35+
* @requires $jsonpCallbacks
3636
* @requires $document
3737
* @requires $xhrFactory
3838
*
@@ -47,8 +47,8 @@ function $xhrFactoryProvider() {
4747
* $httpBackend} which can be trained with responses.
4848
*/
4949
function $HttpBackendProvider() {
50-
this.$get = ['$browser', '$window', '$document', '$xhrFactory', function($browser, $window, $document, $xhrFactory) {
51-
return createHttpBackend($browser, $xhrFactory, $browser.defer, $window.angular.callbacks, $document[0]);
50+
this.$get = ['$browser', '$jsonpCallbacks', '$document', '$xhrFactory', function($browser, $jsonpCallbacks, $document, $xhrFactory) {
51+
return createHttpBackend($browser, $xhrFactory, $browser.defer, $jsonpCallbacks, $document[0]);
5252
}];
5353
}
5454

@@ -59,16 +59,11 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
5959
url = url || $browser.url();
6060

6161
if (lowercase(method) === 'jsonp') {
62-
var callbackId = '_' + (callbacks.counter++).toString(36);
63-
callbacks[callbackId] = function(data) {
64-
callbacks[callbackId].data = data;
65-
callbacks[callbackId].called = true;
66-
};
67-
68-
var jsonpDone = jsonpReq(url.replace('JSON_CALLBACK', 'angular.callbacks.' + callbackId),
69-
callbackId, function(status, text) {
70-
completeRequest(callback, status, callbacks[callbackId].data, "", text);
71-
callbacks[callbackId] = noop;
62+
var callbackPath = callbacks.createCallback(url);
63+
var jsonpDone = jsonpReq(url, callbackPath, function(status, text) {
64+
var response = (status === 200) && callbacks.getResponse(callbackPath);
65+
completeRequest(callback, status, response, "", text);
66+
callbacks.removeCallback(callbackPath);
7267
});
7368
} else {
7469

@@ -170,7 +165,8 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
170165
}
171166
};
172167

173-
function jsonpReq(url, callbackId, done) {
168+
function jsonpReq(url, callbackPath, done) {
169+
url = url.replace('JSON_CALLBACK', callbackPath);
174170
// we can't use jQuery/jqLite here because jQuery does crazy stuff with script elements, e.g.:
175171
// - fetches local scripts via XHR and evals them
176172
// - adds and immediately removes script elements from the document
@@ -188,7 +184,7 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
188184
var text = "unknown";
189185

190186
if (event) {
191-
if (event.type === "load" && !callbacks[callbackId].called) {
187+
if (event.type === "load" && !callbacks.wasCalled(callbackPath)) {
192188
event = { type: "error" };
193189
}
194190
text = event.type;

test/ng/httpBackendSpec.js

+36-11
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33

44
describe('$httpBackend', function() {
55

6-
var $backend, $browser, callbacks,
6+
var $backend, $browser, $jsonpCallbacks,
77
xhr, fakeDocument, callback;
88

9-
109
beforeEach(inject(function($injector) {
11-
callbacks = {counter: 0};
10+
1211
$browser = $injector.get('$browser');
12+
1313
fakeDocument = {
1414
$$scripts: [],
1515
createElement: jasmine.createSpy('createElement').and.callFake(function() {
@@ -28,7 +28,27 @@ describe('$httpBackend', function() {
2828
})
2929
}
3030
};
31-
$backend = createHttpBackend($browser, createMockXhr, $browser.defer, callbacks, fakeDocument);
31+
32+
$jsonpCallbacks = {
33+
createCallback: function(url) {
34+
$jsonpCallbacks[url] = function(data) {
35+
$jsonpCallbacks[url].called = true;
36+
$jsonpCallbacks[url].data = data;
37+
};
38+
return url;
39+
},
40+
wasCalled: function(callbackPath) {
41+
return $jsonpCallbacks[callbackPath].called;
42+
},
43+
getResponse: function(callbackPath) {
44+
return $jsonpCallbacks[callbackPath].data;
45+
},
46+
removeCallback: function(callbackPath) {
47+
delete $jsonpCallbacks[callbackPath];
48+
}
49+
};
50+
51+
$backend = createHttpBackend($browser, createMockXhr, $browser.defer, $jsonpCallbacks, fakeDocument);
3252
callback = jasmine.createSpy('done');
3353
}));
3454

@@ -235,7 +255,7 @@ describe('$httpBackend', function() {
235255

236256
it('should call $xhrFactory with method and url', function() {
237257
var mockXhrFactory = jasmine.createSpy('mockXhrFactory').and.callFake(createMockXhr);
238-
$backend = createHttpBackend($browser, mockXhrFactory, $browser.defer, callbacks, fakeDocument);
258+
$backend = createHttpBackend($browser, mockXhrFactory, $browser.defer, $jsonpCallbacks, fakeDocument);
239259
$backend('GET', '/some-url', 'some-data', noop);
240260
expect(mockXhrFactory).toHaveBeenCalledWith('GET', '/some-url');
241261
});
@@ -294,7 +314,7 @@ describe('$httpBackend', function() {
294314

295315
describe('JSONP', function() {
296316

297-
var SCRIPT_URL = /([^\?]*)\?cb=angular\.callbacks\.(.*)/;
317+
var SCRIPT_URL = /([^\?]*)\?cb=(.*)/;
298318

299319

300320
it('should add script tag for JSONP request', function() {
@@ -310,25 +330,28 @@ describe('$httpBackend', function() {
310330
url = script.src.match(SCRIPT_URL);
311331

312332
expect(url[1]).toBe('http://example.org/path');
313-
callbacks[url[2]]('some-data');
333+
$jsonpCallbacks[url[2]]('some-data');
314334
browserTrigger(script, "load");
315335

316336
expect(callback).toHaveBeenCalledOnce();
317337
});
318338

319339

320340
it('should clean up the callback and remove the script', function() {
341+
spyOn($jsonpCallbacks, 'wasCalled').and.callThrough();
342+
spyOn($jsonpCallbacks, 'removeCallback').and.callThrough();
343+
321344
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback);
322345
expect(fakeDocument.$$scripts.length).toBe(1);
323346

324347

325348
var script = fakeDocument.$$scripts.shift(),
326349
callbackId = script.src.match(SCRIPT_URL)[2];
327350

328-
callbacks[callbackId]('some-data');
351+
$jsonpCallbacks[callbackId]('some-data');
329352
browserTrigger(script, "load");
330353

331-
expect(callbacks[callbackId]).toBe(angular.noop);
354+
expect($jsonpCallbacks.removeCallback).toHaveBeenCalledOnceWith(callbackId);
332355
expect(fakeDocument.body.removeChild).toHaveBeenCalledOnceWith(script);
333356
});
334357

@@ -343,7 +366,9 @@ describe('$httpBackend', function() {
343366
});
344367

345368

346-
it('should abort request on timeout and replace callback with noop', function() {
369+
it('should abort request on timeout and remove JSONP callback', function() {
370+
spyOn($jsonpCallbacks, 'removeCallback').and.callThrough();
371+
347372
callback.and.callFake(function(status, response) {
348373
expect(status).toBe(-1);
349374
});
@@ -359,7 +384,7 @@ describe('$httpBackend', function() {
359384
expect(fakeDocument.$$scripts.length).toBe(0);
360385
expect(callback).toHaveBeenCalledOnce();
361386

362-
expect(callbacks[callbackId]).toBe(angular.noop);
387+
expect($jsonpCallbacks.removeCallback).toHaveBeenCalledOnceWith(callbackId);
363388
});
364389

365390

0 commit comments

Comments
 (0)