From c9db640643b120d8020f93a2a922b5ccc8c4345c Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Thu, 28 Jan 2016 14:38:00 +0200 Subject: [PATCH] fix($http): immediatelly increment $browser's `outstandingRequestCount` This allows protractor to more reliably detect when all outstanding requests have been completed. Fixes #13782 Closes #13862 --- src/ng/http.js | 37 ++++++--- src/ng/httpBackend.js | 2 - test/ng/httpSpec.js | 171 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 197 insertions(+), 13 deletions(-) diff --git a/src/ng/http.js b/src/ng/http.js index 8fb3ae306467..6036942a2cf9 100644 --- a/src/ng/http.js +++ b/src/ng/http.js @@ -375,8 +375,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'); @@ -955,25 +955,23 @@ function $HttpProvider() { return sendReq(config, reqData).then(transformResponse, transformResponse); }; - 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).then(serverRequest); + promise.finally(completeOutstandingRequest); + promise = chainInterceptors(promise, responseInterceptors); if (useLegacyPromise) { promise.success = function(fn) { @@ -998,6 +996,8 @@ function $HttpProvider() { promise.error = $httpMinErrLegacyFn('error'); } + $browser.$$incOutstandingRequestCount(); + return promise; function transformResponse(response) { @@ -1010,6 +1010,21 @@ function $HttpProvider() { : $q.reject(resp); } + function chainInterceptors(promise, interceptors) { + while (interceptors.length) { + var thenFn = interceptors.shift(); + var rejectFn = interceptors.shift(); + + promise = promise.then(thenFn, rejectFn); + } + + return promise; + } + + function completeOutstandingRequest() { + $browser.$$completeOutstandingRequest(noop); + } + function executeHeaderFns(headers, config) { var headerContent, processedHeaders = {}; diff --git a/src/ng/httpBackend.js b/src/ng/httpBackend.js index 0b16b34a7748..f857cefb9c23 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) { - $browser.$$incOutstandingRequestCount(); url = url || $browser.url(); if (lowercase(method) == 'jsonp') { @@ -158,7 +157,6 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc jsonpDone = xhr = null; callback(status, response, headersString, statusText); - $browser.$$completeOutstandingRequest(noop); } }; diff --git a/test/ng/httpSpec.js b/test/ng/httpSpec.js index bc27ff5b5fe6..67dba9c54bf4 100644 --- a/test/ng/httpSpec.js +++ b/test/ng/httpSpec.js @@ -1869,6 +1869,177 @@ 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()` upon response', function() { + $httpBackend.when('GET').respond(200); + + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + $http.get(''); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + $httpBackend.flush(); + expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce(); + }); + + + it('should call `$browser.$$completeOutstandingRequest()` upon error', function() { + $httpBackend.when('GET').respond(500); + + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + $http.get(''); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + $httpBackend.flush(); + expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce(); + }); + + + it('should properly increment/decrement `outstandingRequestCount` ' + + 'upon error in transformRequest', + inject(function($exceptionHandler) { + expect(incOutstandingRequestCountSpy).not.toHaveBeenCalled(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + + $http.get('', {transformRequest: function() { throw new Error(); }}); + + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + + $rootScope.$digest(); + + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce(); + + $exceptionHandler.errors = []; + }) + ); + + + it('should properly increment/decrement `outstandingRequestCount` ' + + 'upon 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(); }}); + + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + + $httpBackend.flush(); + + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce(); + + $exceptionHandler.errors = []; + }) + ); + }); + + + describe('with interceptors', function() { + var requestInterceptorCalled; + var responseInterceptorCalled; + + + beforeEach(module(function($httpProvider) { + requestInterceptorCalled = false; + responseInterceptorCalled = false; + + $httpProvider.interceptors.push(function($q) { + return { + request: function(config) { + requestInterceptorCalled = true; + return config._requestError ? $q.reject() : config; + }, + response: function() { + responseInterceptorCalled = true; + return $q.reject(); + } + }; + }); + })); + beforeEach(setupServicesAndSpies); + + + it('should properly increment/decrement `outstandingRequestCount` ' + + 'upon error in request interceptor', + function() { + expect(incOutstandingRequestCountSpy).not.toHaveBeenCalled(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + expect(requestInterceptorCalled).toBe(false); + + $http.get('', {_requestError: true}); + + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + expect(requestInterceptorCalled).toBe(false); + + $rootScope.$digest(); + + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce(); + expect(requestInterceptorCalled).toBe(true); + } + ); + + + it('should properly increment/decrement `outstandingRequestCount` ' + + 'upon error in response interceptor', + function() { + expect(incOutstandingRequestCountSpy).not.toHaveBeenCalled(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + expect(responseInterceptorCalled).toBe(false); + + $httpBackend.when('GET').respond(200); + $http.get('', {_requestError: false}); + + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).not.toHaveBeenCalled(); + expect(responseInterceptorCalled).toBe(false); + + $httpBackend.flush(); + + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce(); + expect(responseInterceptorCalled).toBe(true); + } + ); + }); + + + function setupServicesAndSpies() { + inject(function($browser, _$http_, _$httpBackend_, _$rootScope_) { + $http = _$http_; + $httpBackend = _$httpBackend_; + $rootScope = _$rootScope_; + + incOutstandingRequestCountSpy + = spyOn($browser, '$$incOutstandingRequestCount').andCallThrough(); + completeOutstandingRequestSpy + = spyOn($browser, '$$completeOutstandingRequest').andCallThrough(); + }); + } + }); + + it('should pass timeout, withCredentials and responseType', function() { var $httpBackend = jasmine.createSpy('$httpBackend');