add $browser.defer and $defer service and fix async xhr cache issue

- Closes #152 ($resource().query() sometimes calls callback before
  returning, and it shouldn't)
- add $browser.defer method
- add $defer service
- integrate $browser.defer with outstandingRequests counter in $browser
- fix all old tests that relied on buggy behavior
This commit is contained in:
Igor Minar 2010-12-04 23:49:26 -08:00
parent 58d0e8945d
commit 011fa39c2a
8 changed files with 205 additions and 28 deletions

View file

@ -15,7 +15,8 @@ angularService('$browser', function($log){
jqLite(window.document), jqLite(window.document),
jqLite(window.document.getElementsByTagName('head')[0]), jqLite(window.document.getElementsByTagName('head')[0]),
XHR, XHR,
$log); $log,
window.setTimeout);
browserSingleton.startPoller(50, function(delay, fn){setTimeout(delay,fn);}); browserSingleton.startPoller(50, function(delay, fn){setTimeout(delay,fn);});
browserSingleton.bind(); browserSingleton.bind();
} }

View file

@ -8,7 +8,7 @@ var XHR = window.XMLHttpRequest || function () {
throw new Error("This browser does not support XMLHttpRequest."); throw new Error("This browser does not support XMLHttpRequest.");
}; };
function Browser(location, document, head, XHR, $log) { function Browser(location, document, head, XHR, $log, setTimeout) {
var self = this; var self = this;
self.isMock = false; self.isMock = false;
@ -19,6 +19,28 @@ function Browser(location, document, head, XHR, $log) {
var outstandingRequestCount = 0; var outstandingRequestCount = 0;
var outstandingRequestCallbacks = []; var outstandingRequestCallbacks = [];
/**
* Executes the `fn` function (supports currying) and decrements the `outstandingRequestCallbacks`
* counter. If the counter reaches 0, all the `outstandingRequestCallbacks` are executed.
*/
function completeOutstandingRequest(fn) {
try {
fn.apply(null, slice.call(arguments, 1));
} finally {
outstandingRequestCount--;
if (outstandingRequestCount === 0) {
while(outstandingRequestCallbacks.length) {
try {
outstandingRequestCallbacks.pop()();
} catch (e) {
$log.error(e);
}
}
}
}
}
/** /**
* @workInProgress * @workInProgress
* @ngdoc method * @ngdoc method
@ -58,19 +80,7 @@ function Browser(location, document, head, XHR, $log) {
outstandingRequestCount ++; outstandingRequestCount ++;
xhr.onreadystatechange = function() { xhr.onreadystatechange = function() {
if (xhr.readyState == 4) { if (xhr.readyState == 4) {
try { completeOutstandingRequest(callback, xhr.status || 200, xhr.responseText);
callback(xhr.status || 200, xhr.responseText);
} finally {
outstandingRequestCount--;
if (outstandingRequestCount === 0) {
while(outstandingRequestCallbacks.length) {
try {
outstandingRequestCallbacks.pop()();
} catch (e) {
}
}
}
}
} }
}; };
xhr.send(post || ''); xhr.send(post || '');
@ -250,6 +260,27 @@ function Browser(location, document, head, XHR, $log) {
} }
}; };
/**
* @workInProgress
* @ngdoc
* @name angular.service.$browser#defer
* @methodOf angular.service.$browser
*
* @description
* Executes a fn asynchroniously via `setTimeout(fn, 0)`.
*
* Unlike when calling `setTimeout` directly, in test this function is mocked and instead of using
* `setTimeout` in tests, the fns are queued in an array, which can be programaticaly flushed via
* `$browser.defer.flush()`.
*
* @param {function()} fn A function, who's execution should be defered.
*/
self.defer = function(fn) {
outstandingRequestCount++;
setTimeout(function() { completeOutstandingRequest(fn); }, 0);
};
////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////
// Misc API // Misc API
////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////

View file

@ -685,7 +685,7 @@ angularServiceInject('$route', function(location) {
* @ngdoc service * @ngdoc service
* @name angular.service.$xhr * @name angular.service.$xhr
* @requires $browser * @requires $browser
* @requires $error * @requires $xhr.error
* @requires $log * @requires $log
* *
* @description * @description
@ -801,6 +801,36 @@ angularServiceInject('$xhr.bulk', function($xhr, $error, $log){
return bulkXHR; return bulkXHR;
}, ['$xhr', '$xhr.error', '$log']); }, ['$xhr', '$xhr.error', '$log']);
/**
* @workInProgress
* @ngdoc service
* @name angular.service.$defer
* @requires $browser
* @requires $log
*
* @description
* Delegates to {@link angular.service.$browser.defer $browser.defer}, but wraps the `fn` function
* into a try/catch block and delegates any exceptions to
* {@link angular.services.$exceptionHandler $exceptionHandler} service.
*
* In tests you can use `$browser.defer.flush()` to flush the queue of deferred functions.
*
* @param {function()} fn A function, who's execution should be deferred.
*/
angularServiceInject('$defer', function($browser, $exceptionHandler) {
return function(fn) {
$browser.defer(function() {
try {
fn();
} catch(e) {
$exceptionHandler(e);
}
});
};
}, ['$browser', '$exceptionHandler']);
/** /**
* @workInProgress * @workInProgress
* @ngdoc service * @ngdoc service
@ -811,7 +841,7 @@ angularServiceInject('$xhr.bulk', function($xhr, $error, $log){
* *
* @example * @example
*/ */
angularServiceInject('$xhr.cache', function($xhr){ angularServiceInject('$xhr.cache', function($xhr, $defer){
var inflight = {}, self = this; var inflight = {}, self = this;
function cache(method, url, post, callback, verifyCache){ function cache(method, url, post, callback, verifyCache){
if (isFunction(post)) { if (isFunction(post)) {
@ -819,9 +849,9 @@ angularServiceInject('$xhr.cache', function($xhr){
post = _null; post = _null;
} }
if (method == 'GET') { if (method == 'GET') {
var data; var data, dataCached;
if (data = cache.data[url]) { if (dataCached = cache.data[url]) {
callback(200, copy(data.value)); $defer(function() { callback(200, copy(dataCached.value)); });
if (!verifyCache) if (!verifyCache)
return; return;
} }
@ -853,7 +883,7 @@ angularServiceInject('$xhr.cache', function($xhr){
cache.data = {}; cache.data = {};
cache.delegate = $xhr; cache.delegate = $xhr;
return cache; return cache;
}, ['$xhr.bulk']); }, ['$xhr.bulk', '$defer']);
/** /**

View file

@ -1,8 +1,21 @@
describe('browser', function(){ describe('browser', function(){
var browser, location, head, xhr; var browser, location, head, xhr, setTimeoutQueue;
function fakeSetTimeout(fn) {
setTimeoutQueue.push(fn);
}
fakeSetTimeout.flush = function() {
foreach(setTimeoutQueue, function(fn) {
fn();
});
};
beforeEach(function(){ beforeEach(function(){
setTimeoutQueue = [];
location = {href:"http://server", hash:""}; location = {href:"http://server", hash:""};
head = { head = {
scripts: [], scripts: [],
@ -14,7 +27,7 @@ describe('browser', function(){
this.open = noop; this.open = noop;
this.setRequestHeader = noop; this.setRequestHeader = noop;
this.send = noop; this.send = noop;
}); }, undefined, fakeSetTimeout);
}); });
it('should contain cookie cruncher', function() { it('should contain cookie cruncher', function() {
@ -59,6 +72,28 @@ describe('browser', function(){
}); });
describe('defer', function() {
it('should execute fn asynchroniously via setTimeout', function() {
var counter = 0;
browser.defer(function() {counter++;});
expect(counter).toBe(0);
fakeSetTimeout.flush();
expect(counter).toBe(1);
});
it('should update outstandingRequests counter', function() {
var callback = jasmine.createSpy('callback');
browser.defer(callback);
expect(callback).not.wasCalled();
fakeSetTimeout.flush();
expect(callback).wasCalled();
});
});
describe('cookies', function() { describe('cookies', function() {
function deleteAllCookies() { function deleteAllCookies() {

View file

@ -184,6 +184,8 @@ describe("resource", function() {
$browser.xhr.expectGET('/Person/123').respond('[\n{\nname:\n"rob"\n}\n]'); $browser.xhr.expectGET('/Person/123').respond('[\n{\nname:\n"rob"\n}\n]');
var person2 = Person.query({id:123}); var person2 = Person.query({id:123});
$browser.defer.flush();
expect(person2[0].name).toEqual('misko'); expect(person2[0].name).toEqual('misko');
var person2Cache = person2; var person2Cache = person2;
$browser.xhr.flush(); $browser.xhr.flush();

10
test/angular-mocks.js vendored
View file

@ -113,6 +113,15 @@ function MockBrowser() {
self.cookieHash = {}; self.cookieHash = {};
self.lastCookieHash = {}; self.lastCookieHash = {};
self.deferredFns = [];
self.defer = function(fn) {
self.deferredFns.push(fn);
};
self.defer.flush = function() {
while (self.deferredFns.length) self.deferredFns.shift()();
};
} }
MockBrowser.prototype = { MockBrowser.prototype = {
@ -156,7 +165,6 @@ MockBrowser.prototype = {
return this.cookieHash; return this.cookieHash;
} }
} }
}; };
angular.service('$browser', function(){ angular.service('$browser', function(){

View file

@ -329,6 +329,47 @@ describe("service", function(){
}); });
}); });
describe('$defer', function() {
var $defer, $exceptionHandler;
beforeEach(function(){
scope = createScope({}, angularService, {
'$exceptionHandler': jasmine.createSpy('$exceptionHandler')
});
$browser = scope.$inject('$browser');
$defer = scope.$inject('$defer');
$exceptionHandler = scope.$inject('$exceptionHandler');
});
it('should delegate functions to $browser.defer', function() {
var counter = 0;
$defer(function() { counter++; });
expect(counter).toBe(0);
$browser.defer.flush();
expect(counter).toBe(1);
$browser.defer.flush(); //does nothing
expect(counter).toBe(1);
expect($exceptionHandler).not.toHaveBeenCalled();
});
it('should delegate exception to the $exceptionHandler service', function() {
$defer(function() { throw "Test Error"; });
expect($exceptionHandler).not.toHaveBeenCalled();
$browser.defer.flush();
expect($exceptionHandler).toHaveBeenCalledWith("Test Error");
});
});
describe('$xhr', function(){ describe('$xhr', function(){
var log; var log;
function callback(code, response) { function callback(code, response) {
@ -426,12 +467,15 @@ describe("service", function(){
$browserXhr.expectGET('/url').respond('first'); $browserXhr.expectGET('/url').respond('first');
cache('GET', '/url', null, callback); cache('GET', '/url', null, callback);
$browserXhr.flush(); $browserXhr.flush();
$browserXhr.expectGET('/url').respond('ERROR'); $browserXhr.expectGET('/url').respond('ERROR');
cache('GET', '/url', null, callback); cache('GET', '/url', null, callback);
$browser.defer.flush();
$browserXhr.flush(); $browserXhr.flush();
expect(log).toEqual('"first";"first";'); expect(log).toEqual('"first";"first";');
cache('GET', '/url', null, callback, false); cache('GET', '/url', null, callback, false);
$browserXhr.flush(); $browser.defer.flush();
expect(log).toEqual('"first";"first";"first";'); expect(log).toEqual('"first";"first";"first";');
}); });
@ -439,9 +483,12 @@ describe("service", function(){
$browserXhr.expectGET('/url').respond('first'); $browserXhr.expectGET('/url').respond('first');
cache('GET', '/url', null, callback, true); cache('GET', '/url', null, callback, true);
$browserXhr.flush(); $browserXhr.flush();
$browserXhr.expectGET('/url').respond('ERROR'); $browserXhr.expectGET('/url').respond('ERROR');
cache('GET', '/url', null, callback, true); cache('GET', '/url', null, callback, true);
$browser.defer.flush();
expect(log).toEqual('"first";"first";'); expect(log).toEqual('"first";"first";');
$browserXhr.flush(); $browserXhr.flush();
expect(log).toEqual('"first";"first";"ERROR";'); expect(log).toEqual('"first";"first";"ERROR";');
}); });
@ -449,8 +496,11 @@ describe("service", function(){
it('should serve requests from cache', function(){ it('should serve requests from cache', function(){
cache.data.url = {value:'123'}; cache.data.url = {value:'123'};
cache('GET', 'url', null, callback); cache('GET', 'url', null, callback);
$browser.defer.flush();
expect(log).toEqual('"123";'); expect(log).toEqual('"123";');
cache('GET', 'url', null, callback, false); cache('GET', 'url', null, callback, false);
$browser.defer.flush();
expect(log).toEqual('"123";"123";'); expect(log).toEqual('"123";"123";');
}); });
@ -478,6 +528,21 @@ describe("service", function(){
cache('POST', 'abc', {}); cache('POST', 'abc', {});
expect(cache.data.url).toBeUndefined(); expect(cache.data.url).toBeUndefined();
}); });
it('should call callback asynchronously for both cache hit and cache miss', function() {
$browserXhr.expectGET('/url').respond('+');
cache('GET', '/url', null, callback);
expect(log).toEqual(''); //callback hasn't executed
$browserXhr.flush();
expect(log).toEqual('"+";'); //callback has executed
cache('GET', '/url', null, callback);
expect(log).toEqual('"+";'); //callback hasn't executed
$browser.defer.flush();
expect(log).toEqual('"+";"+";'); //callback has executed
});
}); });
}); });

View file

@ -530,6 +530,7 @@ describe("widget", function(){
scope.url = 'myUrl'; scope.url = 'myUrl';
scope.$inject('$xhr.cache').data.myUrl = {value:'{{name}}'}; scope.$inject('$xhr.cache').data.myUrl = {value:'{{name}}'};
scope.$init(); scope.$init();
scope.$inject('$browser').defer.flush();
expect(element.text()).toEqual('misko'); expect(element.text()).toEqual('misko');
dealoc(scope); dealoc(scope);
}); });
@ -542,6 +543,7 @@ describe("widget", function(){
scope.url = 'myUrl'; scope.url = 'myUrl';
scope.$inject('$xhr.cache').data.myUrl = {value:'{{name}}'}; scope.$inject('$xhr.cache').data.myUrl = {value:'{{name}}'};
scope.$init(); scope.$init();
scope.$inject('$browser').defer.flush();
expect(element.text()).toEqual('igor'); expect(element.text()).toEqual('igor');
@ -558,9 +560,11 @@ describe("widget", function(){
scope.url = 'myUrl'; scope.url = 'myUrl';
scope.$inject('$xhr.cache').data.myUrl = {value:'{{c=c+1}}'}; scope.$inject('$xhr.cache').data.myUrl = {value:'{{c=c+1}}'};
scope.$init(); scope.$init();
// This should not be 4, but to fix this properly scope.$inject('$browser').defer.flush();
// we need to have real events on the scopes.
expect(element.text()).toEqual('4'); // this one should really be just '1', but due to lack of real events things are not working
// properly. see discussion at: http://is.gd/ighKk
expect(element.text()).toEqual('2');
dealoc(scope); dealoc(scope);
}); });
@ -573,6 +577,7 @@ describe("widget", function(){
scope.url = 'myUrl'; scope.url = 'myUrl';
scope.$inject('$xhr.cache').data.myUrl = {value:'my partial'}; scope.$inject('$xhr.cache').data.myUrl = {value:'my partial'};
scope.$init(); scope.$init();
scope.$inject('$browser').defer.flush();
expect(element.text()).toEqual('my partial'); expect(element.text()).toEqual('my partial');
expect(scope.loaded).toBe(true); expect(scope.loaded).toBe(true);
dealoc(scope); dealoc(scope);