From 5253d656a0c2f089649b95595d746e06bca2c82c Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Fri, 17 Jun 2016 20:23:02 +0100 Subject: [PATCH 1/4] feat($jsonpCallbacks): new service to abstract how JSONP callbacks are handled You can now override this service if you have specific requirements about the behaviour and formatting of the JSON_CALLBACK that is sent to the server for `$http.jsonp` requests. --- angularFiles.js | 1 + src/AngularPublic.js | 2 + src/ng/jsonpCallbacks.js | 83 +++++++++++++++++++++++++++++++++++ test/ng/jsonpCallbacksSpec.js | 73 ++++++++++++++++++++++++++++++ 4 files changed, 159 insertions(+) create mode 100644 src/ng/jsonpCallbacks.js create mode 100644 test/ng/jsonpCallbacksSpec.js diff --git a/angularFiles.js b/angularFiles.js index 97a23f7810f2..0513de6742fe 100755 --- a/angularFiles.js +++ b/angularFiles.js @@ -28,6 +28,7 @@ var angularFiles = { 'src/ng/httpBackend.js', 'src/ng/interpolate.js', 'src/ng/interval.js', + 'src/ng/jsonpCallbacks.js', 'src/ng/locale.js', 'src/ng/location.js', 'src/ng/log.js', diff --git a/src/AngularPublic.js b/src/AngularPublic.js index 43fbe048c08e..777943a80260 100644 --- a/src/AngularPublic.js +++ b/src/AngularPublic.js @@ -77,6 +77,7 @@ $HttpParamSerializerJQLikeProvider, $HttpBackendProvider, $xhrFactoryProvider, + $jsonpCallbacksProvider, $LocationProvider, $LogProvider, $ParseProvider, @@ -239,6 +240,7 @@ function publishExternalAPI(angular) { $httpParamSerializerJQLike: $HttpParamSerializerJQLikeProvider, $httpBackend: $HttpBackendProvider, $xhrFactory: $xhrFactoryProvider, + $jsonpCallbacks: $jsonpCallbacksProvider, $location: $LocationProvider, $log: $LogProvider, $parse: $ParseProvider, diff --git a/src/ng/jsonpCallbacks.js b/src/ng/jsonpCallbacks.js new file mode 100644 index 000000000000..85f30afbc4b3 --- /dev/null +++ b/src/ng/jsonpCallbacks.js @@ -0,0 +1,83 @@ +'use strict'; + +/** + * @ngdoc service + * @name $jsonpCallbacks + * @description + * This service handles the lifecycle of callbacks to handle JSONP requests. + * Override this service if you wish to customise where the callbacks are stored and + * how they vary compared to the requested url. + */ +var $jsonpCallbacksProvider = function() { + this.$get = ['$window', function($window) { + var counter = 0; + $window.angular.callbacks = {}; + var callbackMap = {}; + + function createCalback(callbackId) { + var callback = function(data) { + callback.data = data; + callback.called = true; + }; + callback.id = callbackId; + return callback; + } + + return { + /** + * @ngdoc method + * @name $jsonpCallbacks#createCallback + * @param {string} url the url of the JSONP request + * @returns {string} the callback path to send to the server as part of the JSONP request + * @description + * {@link $httpBackend} calls this method to create a callback and get hold of the path to the callback + * to pass to the server, which will be used to call the callback with its payload in the JSONP response. + */ + createCallback: function(url) { + var callbackId = '_' + (counter++).toString(36); + var callbackPath = 'angular.callbacks.' + callbackId; + var callback = createCalback(callbackId); + callbackMap[callbackPath] = $window.angular.callbacks[callbackId] = callback; + return callbackPath; + }, + /** + * @ngdoc method + * @name $jsonpCallbacks#wasCalled + * @param {string} callbackPath the path to the callback that was sent in the JSONP request + * @returns {boolean} whether the callback has been called, as a result of the JSONP response + * @description + * {@link $httpBackend} calls this method to find out whether the JSONP response actually called the + * callback that was passed in the request. + */ + wasCalled: function(callbackPath) { + return callbackMap[callbackPath].called; + }, + /** + * @ngdoc method + * @name $jsonpCallbacks#getResponse + * @param {string} callbackPath the path to the callback that was sent in the JSONP request + * @returns {*} the data received from the response via the registered callback + * @description + * {@link $httpBackend} calls this method to get hold of the data that was provided to the callback + * in the JSONP response. + */ + getResponse: function(callbackPath) { + var callback = callbackMap[callbackPath]; + return callback.data; + }, + /** + * @ngdoc method + * @name $jsonpCallbacks#removeCallback + * @param {string} callbackPath the path to the callback that was sent in the JSONP request + * @description + * {@link $httpBackend} calls this method to remove the callback after the JSONP request has + * completed or timed-out. + */ + removeCallback: function(callbackPath) { + var callback = callbackMap[callbackPath]; + delete $window.angular.callbacks[callback.id]; + delete callbackMap[callbackPath]; + } + }; + }]; +}; diff --git a/test/ng/jsonpCallbacksSpec.js b/test/ng/jsonpCallbacksSpec.js new file mode 100644 index 000000000000..75dd72fbac69 --- /dev/null +++ b/test/ng/jsonpCallbacksSpec.js @@ -0,0 +1,73 @@ +'use strict'; + +describe('$jsonpCallbacks', function() { + var $mockWindow; + beforeEach(module(function($provide) { + // mock out the $window object + $provide.value('$window', { angular: {} }); + })); + + describe('createCallback(url)', function() { + + it('should return a new unique path to a callback function on each call', inject(function($jsonpCallbacks) { + var path = $jsonpCallbacks.createCallback('http://some.dummy.com/jsonp/request'); + expect(path).toEqual('angular.callbacks._0'); + + path = $jsonpCallbacks.createCallback('http://some.dummy.com/jsonp/request'); + expect(path).toEqual('angular.callbacks._1'); + + path = $jsonpCallbacks.createCallback('http://some.dummy.com/jsonp/request'); + expect(path).toEqual('angular.callbacks._2'); + + path = $jsonpCallbacks.createCallback('http://some.dummy.com/jsonp/request'); + expect(path).toEqual('angular.callbacks._3'); + })); + + it('should add a callback method to the $window.angular.callbacks collection on each call', inject(function($window, $jsonpCallbacks) { + $jsonpCallbacks.createCallback('http://some.dummy.com/jsonp/request'); + expect($window.angular.callbacks._0).toEqual(jasmine.any(Function)); + + $jsonpCallbacks.createCallback('http://some.dummy.com/jsonp/request'); + expect($window.angular.callbacks._1).toEqual(jasmine.any(Function)); + + $jsonpCallbacks.createCallback('http://some.dummy.com/jsonp/request'); + expect($window.angular.callbacks._2).toEqual(jasmine.any(Function)); + + $jsonpCallbacks.createCallback('http://some.dummy.com/jsonp/request'); + expect($window.angular.callbacks._3).toEqual(jasmine.any(Function)); + })); + }); + + + describe('wasCalled(callbackPath)', function() { + + it('should return true once the callback has been called', inject(function($window, $jsonpCallbacks) { + var path = $jsonpCallbacks.createCallback('http://some.dummy.com/jsonp/request'); + expect($jsonpCallbacks.wasCalled(path)).toBeFalsy(); + var response = {}; + $window.angular.callbacks._0(response); + expect($jsonpCallbacks.wasCalled(path)).toBeTruthy(); + })); + }); + + + describe('getResponse(callbackPath)', function() { + + it('should retrieve the data from when the callback was called', inject(function($window, $jsonpCallbacks) { + var path = $jsonpCallbacks.createCallback('http://some.dummy.com/jsonp/request'); + var response = {}; + $window.angular.callbacks._0(response); + var result = $jsonpCallbacks.getResponse(path); + expect(result).toBe(response); + })); + }); + + describe('removeCallback(calbackPath)', function() { + + it('should remove the callback', inject(function($window, $jsonpCallbacks) { + var path = $jsonpCallbacks.createCallback('http://some.dummy.com/jsonp/request'); + $jsonpCallbacks.removeCallback(path); + expect($window.angular.callbacks._0).toBeUndefined(); + })); + }); +}); From c1a335dc61c53c4a647dca8b3150bf183085bb30 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Fri, 17 Jun 2016 20:45:08 +0100 Subject: [PATCH 2/4] refact($http): use the $jsonpCallbacks service Use the built-in service to handling callbacks to `$http.jsonp` requests. Closes #3073 --- src/ng/http.js | 2 ++ src/ng/httpBackend.js | 26 +++++++++------------ test/ng/httpBackendSpec.js | 47 +++++++++++++++++++++++++++++--------- 3 files changed, 49 insertions(+), 26 deletions(-) diff --git a/src/ng/http.js b/src/ng/http.js index a1e75762a87d..7da80bccc92c 100644 --- a/src/ng/http.js +++ b/src/ng/http.js @@ -1122,6 +1122,8 @@ function $HttpProvider() { * * @description * Shortcut method to perform `JSONP` request. + * If you would like to customise where and how the callbacks are stored then try overriding + * or decorating the {@link jsonpCallbacks} service. * * @param {string} url Relative or absolute URL specifying the destination of the request. * The name of the callback should be the string `JSON_CALLBACK`. diff --git a/src/ng/httpBackend.js b/src/ng/httpBackend.js index 727d11169eb0..69bc5fd67337 100644 --- a/src/ng/httpBackend.js +++ b/src/ng/httpBackend.js @@ -32,7 +32,7 @@ function $xhrFactoryProvider() { /** * @ngdoc service * @name $httpBackend - * @requires $window + * @requires $jsonpCallbacks * @requires $document * @requires $xhrFactory * @@ -47,8 +47,8 @@ function $xhrFactoryProvider() { * $httpBackend} which can be trained with responses. */ function $HttpBackendProvider() { - this.$get = ['$browser', '$window', '$document', '$xhrFactory', function($browser, $window, $document, $xhrFactory) { - return createHttpBackend($browser, $xhrFactory, $browser.defer, $window.angular.callbacks, $document[0]); + this.$get = ['$browser', '$jsonpCallbacks', '$document', '$xhrFactory', function($browser, $jsonpCallbacks, $document, $xhrFactory) { + return createHttpBackend($browser, $xhrFactory, $browser.defer, $jsonpCallbacks, $document[0]); }]; } @@ -59,16 +59,11 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc url = url || $browser.url(); if (lowercase(method) === 'jsonp') { - var callbackId = '_' + (callbacks.counter++).toString(36); - callbacks[callbackId] = function(data) { - callbacks[callbackId].data = data; - callbacks[callbackId].called = true; - }; - - var jsonpDone = jsonpReq(url.replace('JSON_CALLBACK', 'angular.callbacks.' + callbackId), - callbackId, function(status, text) { - completeRequest(callback, status, callbacks[callbackId].data, "", text); - callbacks[callbackId] = noop; + var callbackPath = callbacks.createCallback(url); + var jsonpDone = jsonpReq(url, callbackPath, function(status, text) { + var response = (status === 200) && callbacks.getResponse(callbackPath); + completeRequest(callback, status, response, "", text); + callbacks.removeCallback(callbackPath); }); } else { @@ -170,7 +165,8 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc } }; - function jsonpReq(url, callbackId, done) { + function jsonpReq(url, callbackPath, done) { + url = url.replace('JSON_CALLBACK', callbackPath); // we can't use jQuery/jqLite here because jQuery does crazy stuff with script elements, e.g.: // - fetches local scripts via XHR and evals them // - adds and immediately removes script elements from the document @@ -188,7 +184,7 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc var text = "unknown"; if (event) { - if (event.type === "load" && !callbacks[callbackId].called) { + if (event.type === "load" && !callbacks.wasCalled(callbackPath)) { event = { type: "error" }; } text = event.type; diff --git a/test/ng/httpBackendSpec.js b/test/ng/httpBackendSpec.js index af3175acfd6d..936b616a97bc 100644 --- a/test/ng/httpBackendSpec.js +++ b/test/ng/httpBackendSpec.js @@ -3,13 +3,13 @@ describe('$httpBackend', function() { - var $backend, $browser, callbacks, + var $backend, $browser, $jsonpCallbacks, xhr, fakeDocument, callback; - beforeEach(inject(function($injector) { - callbacks = {counter: 0}; + $browser = $injector.get('$browser'); + fakeDocument = { $$scripts: [], createElement: jasmine.createSpy('createElement').and.callFake(function() { @@ -28,7 +28,27 @@ describe('$httpBackend', function() { }) } }; - $backend = createHttpBackend($browser, createMockXhr, $browser.defer, callbacks, fakeDocument); + + $jsonpCallbacks = { + createCallback: function(url) { + $jsonpCallbacks[url] = function(data) { + $jsonpCallbacks[url].called = true; + $jsonpCallbacks[url].data = data; + }; + return url; + }, + wasCalled: function(callbackPath) { + return $jsonpCallbacks[callbackPath].called; + }, + getResponse: function(callbackPath) { + return $jsonpCallbacks[callbackPath].data; + }, + removeCallback: function(callbackPath) { + delete $jsonpCallbacks[callbackPath]; + } + }; + + $backend = createHttpBackend($browser, createMockXhr, $browser.defer, $jsonpCallbacks, fakeDocument); callback = jasmine.createSpy('done'); })); @@ -235,7 +255,7 @@ describe('$httpBackend', function() { it('should call $xhrFactory with method and url', function() { var mockXhrFactory = jasmine.createSpy('mockXhrFactory').and.callFake(createMockXhr); - $backend = createHttpBackend($browser, mockXhrFactory, $browser.defer, callbacks, fakeDocument); + $backend = createHttpBackend($browser, mockXhrFactory, $browser.defer, $jsonpCallbacks, fakeDocument); $backend('GET', '/some-url', 'some-data', noop); expect(mockXhrFactory).toHaveBeenCalledWith('GET', '/some-url'); }); @@ -294,7 +314,7 @@ describe('$httpBackend', function() { describe('JSONP', function() { - var SCRIPT_URL = /([^\?]*)\?cb=angular\.callbacks\.(.*)/; + var SCRIPT_URL = /([^\?]*)\?cb=(.*)/; it('should add script tag for JSONP request', function() { @@ -310,7 +330,7 @@ describe('$httpBackend', function() { url = script.src.match(SCRIPT_URL); expect(url[1]).toBe('http://example.org/path'); - callbacks[url[2]]('some-data'); + $jsonpCallbacks[url[2]]('some-data'); browserTrigger(script, "load"); expect(callback).toHaveBeenCalledOnce(); @@ -318,6 +338,9 @@ describe('$httpBackend', function() { it('should clean up the callback and remove the script', function() { + spyOn($jsonpCallbacks, 'wasCalled').and.callThrough(); + spyOn($jsonpCallbacks, 'removeCallback').and.callThrough(); + $backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback); expect(fakeDocument.$$scripts.length).toBe(1); @@ -325,10 +348,10 @@ describe('$httpBackend', function() { var script = fakeDocument.$$scripts.shift(), callbackId = script.src.match(SCRIPT_URL)[2]; - callbacks[callbackId]('some-data'); + $jsonpCallbacks[callbackId]('some-data'); browserTrigger(script, "load"); - expect(callbacks[callbackId]).toBe(angular.noop); + expect($jsonpCallbacks.removeCallback).toHaveBeenCalledOnceWith(callbackId); expect(fakeDocument.body.removeChild).toHaveBeenCalledOnceWith(script); }); @@ -343,7 +366,9 @@ describe('$httpBackend', function() { }); - it('should abort request on timeout and replace callback with noop', function() { + it('should abort request on timeout and remove JSONP callback', function() { + spyOn($jsonpCallbacks, 'removeCallback').and.callThrough(); + callback.and.callFake(function(status, response) { expect(status).toBe(-1); }); @@ -359,7 +384,7 @@ describe('$httpBackend', function() { expect(fakeDocument.$$scripts.length).toBe(0); expect(callback).toHaveBeenCalledOnce(); - expect(callbacks[callbackId]).toBe(angular.noop); + expect($jsonpCallbacks.removeCallback).toHaveBeenCalledOnceWith(callbackId); }); From bdb210dd2040e5f550abc5f2eaca31ed965d2194 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Sat, 18 Jun 2016 17:01:21 +0100 Subject: [PATCH 3/4] feat($jsonpCallbacks): new service to abstract how JSONP callbacks are handled You can now override this service if you have specific requirements about the behaviour and formatting of the JSON_CALLBACK that is sent to the server for `$http.jsonp` requests. --- src/ng/jsonpCallbacks.js | 8 ++++---- test/ng/jsonpCallbacksSpec.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ng/jsonpCallbacks.js b/src/ng/jsonpCallbacks.js index 85f30afbc4b3..180990516308 100644 --- a/src/ng/jsonpCallbacks.js +++ b/src/ng/jsonpCallbacks.js @@ -3,6 +3,7 @@ /** * @ngdoc service * @name $jsonpCallbacks + * @requires $window * @description * This service handles the lifecycle of callbacks to handle JSONP requests. * Override this service if you wish to customise where the callbacks are stored and @@ -14,7 +15,7 @@ var $jsonpCallbacksProvider = function() { $window.angular.callbacks = {}; var callbackMap = {}; - function createCalback(callbackId) { + function createCallback(callbackId) { var callback = function(data) { callback.data = data; callback.called = true; @@ -36,7 +37,7 @@ var $jsonpCallbacksProvider = function() { createCallback: function(url) { var callbackId = '_' + (counter++).toString(36); var callbackPath = 'angular.callbacks.' + callbackId; - var callback = createCalback(callbackId); + var callback = createCallback(callbackId); callbackMap[callbackPath] = $window.angular.callbacks[callbackId] = callback; return callbackPath; }, @@ -62,8 +63,7 @@ var $jsonpCallbacksProvider = function() { * in the JSONP response. */ getResponse: function(callbackPath) { - var callback = callbackMap[callbackPath]; - return callback.data; + return callbackMap[callbackPath].data; }, /** * @ngdoc method diff --git a/test/ng/jsonpCallbacksSpec.js b/test/ng/jsonpCallbacksSpec.js index 75dd72fbac69..75d421857552 100644 --- a/test/ng/jsonpCallbacksSpec.js +++ b/test/ng/jsonpCallbacksSpec.js @@ -1,7 +1,7 @@ 'use strict'; describe('$jsonpCallbacks', function() { - var $mockWindow; + beforeEach(module(function($provide) { // mock out the $window object $provide.value('$window', { angular: {} }); From d7dfeab915f2a10befc3ae36a7e65c2169944cab Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Sat, 18 Jun 2016 17:01:29 +0100 Subject: [PATCH 4/4] refact($http): use the $jsonpCallbacks service Use the built-in service to handling callbacks to `$http.jsonp` requests. Closes #3073 --- test/ng/httpBackendSpec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/ng/httpBackendSpec.js b/test/ng/httpBackendSpec.js index 936b616a97bc..bf27cf290a02 100644 --- a/test/ng/httpBackendSpec.js +++ b/test/ng/httpBackendSpec.js @@ -338,7 +338,6 @@ describe('$httpBackend', function() { it('should clean up the callback and remove the script', function() { - spyOn($jsonpCallbacks, 'wasCalled').and.callThrough(); spyOn($jsonpCallbacks, 'removeCallback').and.callThrough(); $backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback);