diff --git a/src/ng/http.js b/src/ng/http.js index 351591001673..6f9d573baf13 100644 --- a/src/ng/http.js +++ b/src/ng/http.js @@ -374,8 +374,8 @@ function $HttpProvider() { **/ var interceptorFactories = this.interceptors = []; - this.$get = ['$httpBackend', '$$cookieReader', '$cacheFactory', '$rootScope', '$q', '$injector', - function($httpBackend, $$cookieReader, $cacheFactory, $rootScope, $q, $injector) { + this.$get = ['$browser', '$httpBackend', '$$cookieReader', '$cacheFactory', '$rootScope', '$q', '$injector', + function($browser, $httpBackend, $$cookieReader, $cacheFactory, $rootScope, $q, $injector) { var defaultCache = $cacheFactory('$http'); @@ -957,48 +957,28 @@ function $HttpProvider() { config.headers = mergeHeaders(requestConfig); config.method = uppercase(config.method); config.paramSerializer = isString(config.paramSerializer) ? - $injector.get(config.paramSerializer) : config.paramSerializer; + $injector.get(config.paramSerializer) : config.paramSerializer; - var serverRequest = function(config) { - var headers = config.headers; - var reqData = transformData(config.data, headersGetter(headers), undefined, config.transformRequest); - - // strip content-type if data is undefined - if (isUndefined(reqData)) { - forEach(headers, function(value, header) { - if (lowercase(header) === 'content-type') { - delete headers[header]; - } - }); - } - - if (isUndefined(config.withCredentials) && !isUndefined(defaults.withCredentials)) { - config.withCredentials = defaults.withCredentials; - } - - // send request - return sendReq(config, reqData).then(transformResponse, transformResponse); - }; + $browser.$$incOutstandingRequestCount(); - var chain = [serverRequest, undefined]; + var requestInterceptors = []; + var responseInterceptors = []; var promise = $q.when(config); // apply interceptors forEach(reversedInterceptors, function(interceptor) { if (interceptor.request || interceptor.requestError) { - chain.unshift(interceptor.request, interceptor.requestError); + requestInterceptors.unshift(interceptor.request, interceptor.requestError); } if (interceptor.response || interceptor.responseError) { - chain.push(interceptor.response, interceptor.responseError); + responseInterceptors.push(interceptor.response, interceptor.responseError); } }); - while (chain.length) { - var thenFn = chain.shift(); - var rejectFn = chain.shift(); - - promise = promise.then(thenFn, rejectFn); - } + promise = chainInterceptors(promise, requestInterceptors); + promise = promise.then(serverRequest); + promise = chainInterceptors(promise, responseInterceptors); + promise = promise.finally(completeOutstandingRequest); if (useLegacyPromise) { promise.success = function(fn) { @@ -1025,14 +1005,22 @@ function $HttpProvider() { return promise; - function transformResponse(response) { - // make a copy since the response must be cacheable - var resp = extend({}, response); - resp.data = transformData(response.data, response.headers, response.status, - config.transformResponse); - return (isSuccess(response.status)) - ? resp - : $q.reject(resp); + + function chainInterceptors(promise, interceptors) { + for (var i = 0, ii = interceptors.length; i < ii;) { + var thenFn = interceptors[i++]; + var rejectFn = interceptors[i++]; + + promise = promise.then(thenFn, rejectFn); + } + + interceptors.length = 0; + + return promise; + } + + function completeOutstandingRequest() { + $browser.$$completeOutstandingRequest(noop); } function executeHeaderFns(headers, config) { @@ -1076,6 +1064,37 @@ function $HttpProvider() { // execute if header value is a function for merged headers return executeHeaderFns(reqHeaders, shallowCopy(config)); } + + function serverRequest(config) { + var headers = config.headers; + var reqData = transformData(config.data, headersGetter(headers), undefined, config.transformRequest); + + // strip content-type if data is undefined + if (isUndefined(reqData)) { + forEach(headers, function(value, header) { + if (lowercase(header) === 'content-type') { + delete headers[header]; + } + }); + } + + if (isUndefined(config.withCredentials) && !isUndefined(defaults.withCredentials)) { + config.withCredentials = defaults.withCredentials; + } + + // send request + return sendReq(config, reqData).then(transformResponse, transformResponse); + } + + function transformResponse(response) { + // make a copy since the response must be cacheable + var resp = extend({}, response); + resp.data = transformData(response.data, response.headers, response.status, + config.transformResponse); + return (isSuccess(response.status)) + ? resp + : $q.reject(resp); + } } $http.pendingRequests = []; diff --git a/src/ng/httpBackend.js b/src/ng/httpBackend.js index d4a28d76e8f6..51c87ab57ed1 100644 --- a/src/ng/httpBackend.js +++ b/src/ng/httpBackend.js @@ -55,7 +55,6 @@ function $HttpBackendProvider() { function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDocument) { // TODO(vojta): fix the signature return function(method, url, post, callback, headers, timeout, withCredentials, responseType, eventHandlers, uploadEventHandlers) { - $browser.$$incOutstandingRequestCount(); url = url || $browser.url(); if (lowercase(method) === 'jsonp') { @@ -162,7 +161,6 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc jsonpDone = xhr = null; callback(status, response, headersString, statusText); - $browser.$$completeOutstandingRequest(noop); } }; diff --git a/test/e2e/fixtures/http/index.html b/test/e2e/fixtures/http/index.html new file mode 100644 index 000000000000..8a1551be0b19 --- /dev/null +++ b/test/e2e/fixtures/http/index.html @@ -0,0 +1,11 @@ + + + +
+

{{text}}

+
+ + + + + diff --git a/test/e2e/fixtures/http/script.js b/test/e2e/fixtures/http/script.js new file mode 100644 index 000000000000..ebd69e2d1558 --- /dev/null +++ b/test/e2e/fixtures/http/script.js @@ -0,0 +1,28 @@ +angular. + module('test', []). + config(function($httpProvider) { + $httpProvider.interceptors.push(function($q) { + return { + request: function(config) { + return $q(function(resolve) { + setTimeout(resolve, 100, config); + }); + }, + response: function(response) { + return $q(function(resolve) { + setTimeout(resolve, 100, response); + }); + } + }; + }); + }). + controller('TestController', function($cacheFactory, $http, $scope) { + var url = '/some/url'; + + var cache = $cacheFactory('test'); + cache.put(url, 'Hello, world!'); + + $http. + get(url, {cache: cache}). + then(function(response) { $scope.text = response.data; }); + }); diff --git a/test/e2e/tests/httpSpec.js b/test/e2e/tests/httpSpec.js new file mode 100644 index 000000000000..fe8c7d67afd5 --- /dev/null +++ b/test/e2e/tests/httpSpec.js @@ -0,0 +1,9 @@ +describe('$http', function() { + beforeEach(function() { + loadFixture('http'); + }); + + it('should correctly update the outstanding request count', function() { + expect(element(by.binding('text')).getText()).toBe('Hello, world!'); + }); +}); diff --git a/test/ng/httpSpec.js b/test/ng/httpSpec.js index bd44b1cbbc00..479ce359bc92 100644 --- a/test/ng/httpSpec.js +++ b/test/ng/httpSpec.js @@ -1899,6 +1899,233 @@ describe('$http', function() { }); + describe('$browser\'s outstandingRequestCount', function() { + var $http; + var $httpBackend; + var $rootScope; + var incOutstandingRequestCountSpy; + var completeOutstandingRequestSpy; + + + describe('without interceptors', function() { + beforeEach(setupServicesAndSpies); + + + it('should immediately call `$browser.$$incOutstandingRequestCount()`', function() { + expect(incOutstandingRequestCountSpy).not.toHaveBeenCalled(); + $http.get(''); + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + }); + + + it('should call `$browser.$$completeOutstandingRequest()` on success', function() { + $httpBackend.when('GET').respond(200); + + $http.get(''); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + $httpBackend.flush(); + expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce(); + }); + + + it('should call `$browser.$$completeOutstandingRequest()` on error', function() { + $httpBackend.when('GET').respond(500); + + $http.get('').catch(noop); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + $httpBackend.flush(); + expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce(); + }); + + + it('should increment/decrement `outstandingRequestCount` on error in `transformRequest`', + inject(function($exceptionHandler) { + expect(incOutstandingRequestCountSpy).not.toHaveBeenCalled(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + + $http.get('', {transformRequest: function() { throw new Error(); }}).catch(noop); + + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + + $rootScope.$digest(); + + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce(); + + expect($exceptionHandler.errors).toEqual([jasmine.any(Error)]); + $exceptionHandler.errors = []; + }) + ); + + + it('should increment/decrement `outstandingRequestCount` on error in `transformResponse`', + inject(function($exceptionHandler) { + expect(incOutstandingRequestCountSpy).not.toHaveBeenCalled(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + + $httpBackend.when('GET').respond(200); + $http.get('', {transformResponse: function() { throw new Error(); }}).catch(noop); + + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + + $httpBackend.flush(); + + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce(); + + expect($exceptionHandler.errors).toEqual([jasmine.any(Error)]); + $exceptionHandler.errors = []; + }) + ); + }); + + + describe('with interceptors', function() { + var reqInterceptorDeferred; + var resInterceptorDeferred; + var reqInterceptorFulfilled; + var resInterceptorFulfilled; + + beforeEach(module(function($httpProvider) { + reqInterceptorDeferred = null; + resInterceptorDeferred = null; + reqInterceptorFulfilled = false; + resInterceptorFulfilled = false; + + $httpProvider.interceptors.push(function($q) { + return { + request: function(config) { + return (reqInterceptorDeferred = $q.defer()). + promise. + finally(function() { reqInterceptorFulfilled = true; }). + then(valueFn(config)); + }, + response: function() { + return (resInterceptorDeferred = $q.defer()). + promise. + finally(function() { resInterceptorFulfilled = true; }); + } + }; + }); + })); + + beforeEach(setupServicesAndSpies); + + beforeEach(function() { + $httpBackend.when('GET').respond(200); + }); + + + it('should increment/decrement `outstandingRequestCount` before/after async interceptors', + function() { + expect(reqInterceptorFulfilled).toBe(false); + expect(resInterceptorFulfilled).toBe(false); + expect(incOutstandingRequestCountSpy).not.toHaveBeenCalled(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + + $http.get(''); + $rootScope.$digest(); + + expect(reqInterceptorFulfilled).toBe(false); + expect(resInterceptorFulfilled).toBe(false); + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + + reqInterceptorDeferred.resolve(); + $httpBackend.flush(); + + expect(reqInterceptorFulfilled).toBe(true); + expect(resInterceptorFulfilled).toBe(false); + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + + resInterceptorDeferred.resolve(); + $rootScope.$digest(); + + expect(reqInterceptorFulfilled).toBe(true); + expect(resInterceptorFulfilled).toBe(true); + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce(); + } + ); + + + it('should increment/decrement `outstandingRequestCount` on error in request interceptor', + function() { + expect(reqInterceptorFulfilled).toBe(false); + expect(incOutstandingRequestCountSpy).not.toHaveBeenCalled(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + + $http.get('').catch(noop); + $rootScope.$digest(); + + expect(reqInterceptorFulfilled).toBe(false); + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + + reqInterceptorDeferred.reject(); + $rootScope.$digest(); + + expect(reqInterceptorFulfilled).toBe(true); + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce(); + } + ); + + + it('should increment/decrement `outstandingRequestCount` on error in response interceptor', + function() { + expect(reqInterceptorFulfilled).toBe(false); + expect(resInterceptorFulfilled).toBe(false); + expect(incOutstandingRequestCountSpy).not.toHaveBeenCalled(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + + $http.get('').catch(noop); + $rootScope.$digest(); + + expect(reqInterceptorFulfilled).toBe(false); + expect(resInterceptorFulfilled).toBe(false); + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + + reqInterceptorDeferred.resolve(); + $httpBackend.flush(); + + expect(reqInterceptorFulfilled).toBe(true); + expect(resInterceptorFulfilled).toBe(false); + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + + resInterceptorDeferred.reject(); + $rootScope.$digest(); + + expect(reqInterceptorFulfilled).toBe(true); + expect(resInterceptorFulfilled).toBe(true); + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce(); + } + ); + }); + + + // Helpers + function setupServicesAndSpies() { + inject(function($browser, _$http_, _$httpBackend_, _$rootScope_) { + $http = _$http_; + $httpBackend = _$httpBackend_; + $rootScope = _$rootScope_; + + incOutstandingRequestCountSpy = + spyOn($browser, '$$incOutstandingRequestCount').and.callThrough(); + completeOutstandingRequestSpy = + spyOn($browser, '$$completeOutstandingRequest').and.callThrough(); + }); + } + }); + + it('should pass timeout, withCredentials and responseType', function() { var $httpBackend = jasmine.createSpy('$httpBackend');