From e08dd2b78c58b01026da06ba465dbc13bd196129 Mon Sep 17 00:00:00 2001 From: William Bagayoko Date: Wed, 27 Jan 2016 17:44:47 -0600 Subject: [PATCH 1/5] fix($http): properly synchronize $browser outstandingRequestCount The issue is that using $http doesn't update $browser.outstandingRequestCount synchronously so that $browser.notifyWhenNoOutstandingRequests waits accordingly. Configuring this synchronization with the promise chain updates the outstandingRequestCount correctly. Closes #13782 --- src/ng/http.js | 10 ++++++++-- src/ng/httpBackend.js | 2 -- src/ngMock/angular-mocks.js | 5 +++-- test/ng/httpSpec.js | 18 ++++++++++++++++++ 4 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/ng/http.js b/src/ng/http.js index 9f0a821f3e0f..927e8b13b3b2 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 = ['$httpBackend', '$$cookieReader', '$cacheFactory', '$rootScope', '$q', '$injector', '$browser', + function($httpBackend, $$cookieReader, $cacheFactory, $rootScope, $q, $injector, $browser) { var defaultCache = $cacheFactory('$http'); @@ -968,6 +968,8 @@ function $HttpProvider() { } }); + $browser.$$incOutstandingRequestCount(); + while (chain.length) { var thenFn = chain.shift(); var rejectFn = chain.shift(); @@ -975,6 +977,10 @@ function $HttpProvider() { promise = promise.then(thenFn, rejectFn); } + promise.finally(function() { + $browser.$$completeOutstandingRequest(noop); + }); + if (useLegacyPromise) { promise.success = function(fn) { assertArgFn(fn, 'fn'); 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/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index 3a843e2bc7ac..7ca746eb0f01 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -37,8 +37,9 @@ angular.mock.$Browser = function() { self.pollFns = []; // TODO(vojta): remove this temporary api - self.$$completeOutstandingRequest = angular.noop; - self.$$incOutstandingRequestCount = angular.noop; + self.$$outstandingRequestCount = 0; + self.$$completeOutstandingRequest = function() { self.$$outstandingRequestCount--; }; + self.$$incOutstandingRequestCount = function() { self.$$outstandingRequestCount++; }; // register url polling fn diff --git a/test/ng/httpSpec.js b/test/ng/httpSpec.js index bc27ff5b5fe6..74ec8c488cfa 100644 --- a/test/ng/httpSpec.js +++ b/test/ng/httpSpec.js @@ -1866,6 +1866,24 @@ describe('$http', function() { expect(paramSerializer({foo: 'foo', bar: ['bar', 'baz']})).toEqual('bar=bar&bar=baz&foo=foo'); }); }); + + describe('$browser outstandingRequests', function() { + var $browser; + + beforeEach(inject(['$browser', function(browser) { + $browser = browser; + }])); + + it('should update $brower outstandingRequestCount', function() { + $httpBackend.when('GET').respond(200); + + expect($browser.$$outstandingRequestCount).toBe(0); + $http({method: 'GET', url: '/some'}); + expect($browser.$$outstandingRequestCount).toBe(1); + $httpBackend.flush(); + expect($browser.$$outstandingRequestCount).toBe(0); + }); + }); }); From 28b2e0051a56d8000169ececf2202e2978c3cd04 Mon Sep 17 00:00:00 2001 From: William Bagayoko Date: Thu, 28 Jan 2016 10:32:25 -0600 Subject: [PATCH 2/5] fix($http): properly synchronize $browser outstandingRequestCount revert mock $browser changes for spy; refactor test --- src/ngMock/angular-mocks.js | 5 ++--- test/ng/httpSpec.js | 19 ++++++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index 7ca746eb0f01..3a843e2bc7ac 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -37,9 +37,8 @@ angular.mock.$Browser = function() { self.pollFns = []; // TODO(vojta): remove this temporary api - self.$$outstandingRequestCount = 0; - self.$$completeOutstandingRequest = function() { self.$$outstandingRequestCount--; }; - self.$$incOutstandingRequestCount = function() { self.$$outstandingRequestCount++; }; + self.$$completeOutstandingRequest = angular.noop; + self.$$incOutstandingRequestCount = angular.noop; // register url polling fn diff --git a/test/ng/httpSpec.js b/test/ng/httpSpec.js index 74ec8c488cfa..bcc9458ffb0a 100644 --- a/test/ng/httpSpec.js +++ b/test/ng/httpSpec.js @@ -1867,21 +1867,22 @@ describe('$http', function() { }); }); - describe('$browser outstandingRequests', function() { - var $browser; + describe('$browser\'s outstandingRequestCount', function() { + var incOutstandingRequestCountSpy, completeOutstandingRequestSpy; - beforeEach(inject(['$browser', function(browser) { - $browser = browser; - }])); + beforeEach(inject(function($browser) { + incOutstandingRequestCountSpy = spyOn($browser, '$$incOutstandingRequestCount').andCallThrough(); + completeOutstandingRequestSpy = spyOn($browser, '$$completeOutstandingRequest').andCallThrough(); + })); - it('should update $brower outstandingRequestCount', function() { + it('should update $browser outstandingRequestCount', function() { $httpBackend.when('GET').respond(200); - expect($browser.$$outstandingRequestCount).toBe(0); + expect(incOutstandingRequestCountSpy).not.toHaveBeenCalled(); $http({method: 'GET', url: '/some'}); - expect($browser.$$outstandingRequestCount).toBe(1); + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); $httpBackend.flush(); - expect($browser.$$outstandingRequestCount).toBe(0); + expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce(); }); }); }); From a40e3892225b5ed1791a77fb9bad3ab056a4ea2e Mon Sep 17 00:00:00 2001 From: William Bagayoko Date: Thu, 28 Jan 2016 16:18:21 -0600 Subject: [PATCH 3/5] fix($http): refactor $browser outstanding request api calls; add test for failure --- src/ng/http.js | 18 ++++++++++++------ test/ng/httpSpec.js | 20 ++++++++++++++++---- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/ng/http.js b/src/ng/http.js index 927e8b13b3b2..26081f8dbaad 100644 --- a/src/ng/http.js +++ b/src/ng/http.js @@ -956,7 +956,7 @@ function $HttpProvider() { }; var chain = [serverRequest, undefined]; - var promise = $q.when(config); + var promise = initiateOutstandingRequest(config); // apply interceptors forEach(reversedInterceptors, function(interceptor) { @@ -968,8 +968,6 @@ function $HttpProvider() { } }); - $browser.$$incOutstandingRequestCount(); - while (chain.length) { var thenFn = chain.shift(); var rejectFn = chain.shift(); @@ -977,9 +975,8 @@ function $HttpProvider() { promise = promise.then(thenFn, rejectFn); } - promise.finally(function() { - $browser.$$completeOutstandingRequest(noop); - }); + promise.finally(completeOutstandingRequest); + if (useLegacyPromise) { promise.success = function(fn) { @@ -1006,6 +1003,15 @@ function $HttpProvider() { return promise; + function initiateOutstandingRequest(config) { + $browser.$$incOutstandingRequestCount(); + return $q.when(config); + } + + function completeOutstandingRequest() { + $browser.$$completeOutstandingRequest(noop); + } + function transformResponse(response) { // make a copy since the response must be cacheable var resp = extend({}, response); diff --git a/test/ng/httpSpec.js b/test/ng/httpSpec.js index bcc9458ffb0a..eb2b4af4ed2c 100644 --- a/test/ng/httpSpec.js +++ b/test/ng/httpSpec.js @@ -1871,15 +1871,27 @@ describe('$http', function() { var incOutstandingRequestCountSpy, completeOutstandingRequestSpy; beforeEach(inject(function($browser) { - incOutstandingRequestCountSpy = spyOn($browser, '$$incOutstandingRequestCount').andCallThrough(); - completeOutstandingRequestSpy = spyOn($browser, '$$completeOutstandingRequest').andCallThrough(); + incOutstandingRequestCountSpy + = spyOn($browser, '$$incOutstandingRequestCount').andCallThrough(); + completeOutstandingRequestSpy + = spyOn($browser, '$$completeOutstandingRequest').andCallThrough(); })); - it('should update $browser outstandingRequestCount', function() { + it('should update $browser outstandingRequestCount on success', function() { $httpBackend.when('GET').respond(200); expect(incOutstandingRequestCountSpy).not.toHaveBeenCalled(); - $http({method: 'GET', url: '/some'}); + $http.get(''); + expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); + $httpBackend.flush(); + expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce(); + }); + + it('should update $browser outstandingRequestCount on error', function() { + $httpBackend.when('GET').respond(500); + + expect(incOutstandingRequestCountSpy).not.toHaveBeenCalled(); + $http.get(''); expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce(); $httpBackend.flush(); expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce(); From 62fa5159750b5d9c855b4bc6921ba9f9712dba0b Mon Sep 17 00:00:00 2001 From: William Bagayoko Date: Thu, 28 Jan 2016 16:28:51 -0600 Subject: [PATCH 4/5] fix($http): properly synchronize $browser outstandingRequestCount --- src/ng/http.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ng/http.js b/src/ng/http.js index 26081f8dbaad..524044f6d50e 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', '$browser', - function($httpBackend, $$cookieReader, $cacheFactory, $rootScope, $q, $injector, $browser) { + this.$get = ['$browser', '$httpBackend', '$$cookieReader', '$cacheFactory', '$rootScope', '$q', '$injector', + function($browser, $httpBackend, $$cookieReader, $cacheFactory, $rootScope, $q, $injector) { var defaultCache = $cacheFactory('$http'); @@ -977,7 +977,6 @@ function $HttpProvider() { promise.finally(completeOutstandingRequest); - if (useLegacyPromise) { promise.success = function(fn) { assertArgFn(fn, 'fn'); From e720e695f1010e6359f7af2ce5195a6bdd8bcb96 Mon Sep 17 00:00:00 2001 From: William Bagayoko Date: Mon, 1 Feb 2016 23:28:09 -0600 Subject: [PATCH 5/5] test($http): http interceptors. --- test/e2e/fixtures/http/index.html | 8 ++++++++ test/e2e/fixtures/http/script.js | 29 +++++++++++++++++++++++++++++ test/e2e/tests/httpSpec.js | 11 +++++++++++ 3 files changed, 48 insertions(+) create mode 100644 test/e2e/fixtures/http/index.html create mode 100644 test/e2e/fixtures/http/script.js create mode 100644 test/e2e/tests/httpSpec.js diff --git a/test/e2e/fixtures/http/index.html b/test/e2e/fixtures/http/index.html new file mode 100644 index 000000000000..57cd3ca795cd --- /dev/null +++ b/test/e2e/fixtures/http/index.html @@ -0,0 +1,8 @@ + + +
+

{{text}}

+
+ + + diff --git a/test/e2e/fixtures/http/script.js b/test/e2e/fixtures/http/script.js new file mode 100644 index 000000000000..db636b84734d --- /dev/null +++ b/test/e2e/fixtures/http/script.js @@ -0,0 +1,29 @@ +angular.module('test', []) + .controller('TestCtrl', function($scope, $http, $cacheFactory, $timeout) { + var cache = $cacheFactory('sites'); + var siteUrl = "http://some.site/"; + cache.put(siteUrl, "Something"); + $http.get(siteUrl, {cache: cache}).then(function(data) { + $scope.text = "Hello, world!"; + }); + }) + .config(function($httpProvider) { + $httpProvider.interceptors.push(function($q, $window) { + return { + 'request': function(config) { + return $q(function(resolve,reject) { + $window.setTimeout(function() { + resolve(config); + }, 50); + }); + }, + 'response': function(response) { + return $q(function(resolve,reject) { + $window.setTimeout(function() { + resolve(response); + }, 50); + }); + } + }; + }); + }); diff --git a/test/e2e/tests/httpSpec.js b/test/e2e/tests/httpSpec.js new file mode 100644 index 000000000000..66a0990c179f --- /dev/null +++ b/test/e2e/tests/httpSpec.js @@ -0,0 +1,11 @@ + +describe('$http', function() { + beforeEach(function() { + loadFixture("http").andWaitForAngular(); + }); + + it('should have the interpolated text', function() { + expect(element(by.binding('text')).getText()) + .toBe('Hello, world!'); + }); +});