feat($http): add support for aborting via timeout promises

dbinit · IgorMinar · commit 9f4f59371126 · 2013-05-20T14:15:04.000-07:00
If the timeout argument is a promise, abort the request when it is resolved. Implemented by adding support to $httpBackend service and $httpBackend mock service. This api can also be used to explicitly abort requests while keeping the communication between the deffered and promise unidirectional. Closes angular#1159
diff --git a/src/ng/http.js b/src/ng/http.js
@@ -548,7 +548,8 @@ function $HttpProvider() {
      *      GET request, otherwise if a cache instance built with
      *      {@link ng.$cacheFactory $cacheFactory}, this cache will be used for
      *      caching.
-     *    - **timeout** – `{number}` – timeout in milliseconds.
+     *    - **timeout** – `{number|Promise}` – timeout in milliseconds, or {@link ng.$q promise}
+     *      that should abort the request when resolved.
      *    - **withCredentials** - `{boolean}` - whether to to set the `withCredentials` flag on the
      *      XHR object. See {@link https://developer.mozilla.org/en/http_access_control#section_5
      *      requests with credentials} for more information.
@@ -927,7 +928,7 @@ function $HttpProvider() {
         }
 
         resolvePromise(response, status, headersString);
-        $rootScope.$apply();
+        if (!$rootScope.$$phase) $rootScope.$apply();
       }
 
 
diff --git a/src/ng/httpBackend.js b/src/ng/httpBackend.js
@@ -107,20 +107,25 @@ function createHttpBackend($browser, XHR, $browserDefer, callbacks, rawDocument,
     }
 
     if (timeout > 0) {
-      var timeoutId = $browserDefer(function() {
-        status = -1;
-        jsonpDone && jsonpDone();
-        xhr && xhr.abort();
-      }, timeout);
+      var timeoutId = $browserDefer(timeoutRequest, timeout);
+    } else if (timeout && timeout.then) {
+      timeout.then(timeoutRequest);
     }
 
 
+    function timeoutRequest() {
+      status = -1;
+      jsonpDone && jsonpDone();
+      xhr && xhr.abort();
+    }
+
     function completeRequest(callback, status, response, headersString) {
       // URL_MATCH is defined in src/service/location.js
       var protocol = (url.match(SERVER_MATCH) || ['', locationProtocol])[1];
 
-      // cancel timeout
+      // cancel timeout and subsequent timeout promise resolution
       timeoutId && $browserDefer.cancel(timeoutId);
+      jsonpDone = xhr = null;
 
       // fix status code for file protocol (it's always 0)
       status = (protocol == 'file') ? (response ? 200 : 404) : status;
diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js
@@ -937,7 +937,7 @@ function createHttpBackendMock($rootScope, $delegate, $browser) {
   }
 
   // TODO(vojta): change params to: method, url, data, headers, callback
-  function $httpBackend(method, url, data, callback, headers) {
+  function $httpBackend(method, url, data, callback, headers, timeout) {
     var xhr = new MockXhr(),
         expectation = expectations[0],
         wasExpected = false;
@@ -948,6 +948,28 @@ function createHttpBackendMock($rootScope, $delegate, $browser) {
           : angular.toJson(data);
     }
 
+    function wrapResponse(wrapped) {
+      if (!$browser && timeout && timeout.then) timeout.then(handleTimeout);
+
+      return handleResponse;
+
+      function handleResponse() {
+        var response = wrapped.response(method, url, data, headers);
+        xhr.$$respHeaders = response[2];
+        callback(response[0], response[1], xhr.getAllResponseHeaders());
+      }
+
+      function handleTimeout() {
+        for (var i = 0, ii = responses.length; i < ii; i++) {
+          if (responses[i] === handleResponse) {
+            responses.splice(i, 1);
+            callback(-1, undefined, '');
+            break;
+          }
+        }
+      }
+    }
+
     if (expectation && expectation.match(method, url)) {
       if (!expectation.matchData(data))
         throw Error('Expected ' + expectation + ' with different data\n' +
@@ -961,11 +983,7 @@ function createHttpBackendMock($rootScope, $delegate, $browser) {
       expectations.shift();
 
       if (expectation.response) {
-        responses.push(function() {
-          var response = expectation.response(method, url, data, headers);
-          xhr.$$respHeaders = response[2];
-          callback(response[0], response[1], xhr.getAllResponseHeaders());
-        });
+        responses.push(wrapResponse(expectation));
         return;
       }
       wasExpected = true;
@@ -976,13 +994,9 @@ function createHttpBackendMock($rootScope, $delegate, $browser) {
       if (definition.match(method, url, data, headers || {})) {
         if (definition.response) {
           // if $browser specified, we do auto flush all requests
-          ($browser ? $browser.defer : responsesPush)(function() {
-            var response = definition.response(method, url, data, headers);
-            xhr.$$respHeaders = response[2];
-            callback(response[0], response[1], xhr.getAllResponseHeaders());
-          });
+          ($browser ? $browser.defer : responsesPush)(wrapResponse(definition));
         } else if (definition.passThrough) {
-          $delegate(method, url, data, callback, headers);
+          $delegate(method, url, data, callback, headers, timeout);
         } else throw Error('No response defined !');
         return;
       }
diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js
@@ -85,7 +85,8 @@
  *     GET request, otherwise if a cache instance built with
  *     {@link ng.$cacheFactory $cacheFactory}, this cache will be used for
  *     caching.
- *   - **`timeout`** – `{number}` – timeout in milliseconds.
+ *   - **`timeout`** – `{number|Promise}` – timeout in milliseconds, or {@link ng.$q promise} that
+ *     should abort the request when resolved.
  *   - **`withCredentials`** - `{boolean}` - whether to to set the `withCredentials` flag on the
  *     XHR object. See {@link https://developer.mozilla.org/en/http_access_control#section_5
  *     requests with credentials} for more information.
diff --git a/test/ng/httpBackendSpec.js b/test/ng/httpBackendSpec.js
@@ -117,6 +117,44 @@ describe('$httpBackend', function() {
   });
 
 
+  it('should abort request on timeout promise resolution', inject(function($timeout) {
+    callback.andCallFake(function(status, response) {
+      expect(status).toBe(-1);
+    });
+
+    $backend('GET', '/url', null, callback, {}, $timeout(noop, 2000));
+    xhr = MockXhr.$$lastInstance;
+    spyOn(xhr, 'abort');
+
+    $timeout.flush();
+    expect(xhr.abort).toHaveBeenCalledOnce();
+
+    xhr.status = 0;
+    xhr.readyState = 4;
+    xhr.onreadystatechange();
+    expect(callback).toHaveBeenCalledOnce();
+  }));
+
+
+  it('should not abort resolved request on timeout promise resolution', inject(function($timeout) {
+    callback.andCallFake(function(status, response) {
+      expect(status).toBe(200);
+    });
+
+    $backend('GET', '/url', null, callback, {}, $timeout(noop, 2000));
+    xhr = MockXhr.$$lastInstance;
+    spyOn(xhr, 'abort');
+
+    xhr.status = 200;
+    xhr.readyState = 4;
+    xhr.onreadystatechange();
+    expect(callback).toHaveBeenCalledOnce();
+
+    $timeout.flush();
+    expect(xhr.abort).not.toHaveBeenCalled();
+  }));
+
+
   it('should cancel timeout on completion', function() {
     callback.andCallFake(function(status, response) {
       expect(status).toBe(200);
diff --git a/test/ng/httpSpec.js b/test/ng/httpSpec.js
@@ -1273,6 +1273,33 @@ describe('$http', function() {
     });
 
 
+    describe('timeout', function() {
+
+      it('should abort requests when timeout promise resolves', inject(function($q) {
+        var canceler = $q.defer();
+
+        $httpBackend.expect('GET', '/some').respond(200);
+
+        $http({method: 'GET', url: '/some', timeout: canceler.promise}).error(
+            function(data, status, headers, config) {
+              expect(data).toBeUndefined();
+              expect(status).toBe(0);
+              expect(headers()).toEqual({});
+              expect(config.url).toBe('/some');
+              callback();
+            });
+
+        $rootScope.$apply(function() {
+          canceler.resolve();
+        });
+
+        expect(callback).toHaveBeenCalled();
+        $httpBackend.verifyNoOutstandingExpectation();
+        $httpBackend.verifyNoOutstandingRequest();
+      }));
+    });
+
+
     describe('pendingRequests', function() {
 
       it('should be an array of pending requests', function() {
diff --git a/test/ngMock/angular-mocksSpec.js b/test/ngMock/angular-mocksSpec.js
@@ -798,6 +798,24 @@ describe('ngMock', function() {
     });
 
 
+    it('should abort requests when timeout promise resolves', function() {
+      hb.expect('GET', '/url1').respond(200);
+
+      var canceler, then = jasmine.createSpy('then').andCallFake(function(fn) {
+        canceler = fn;
+      });
+
+      hb('GET', '/url1', null, callback, null, {then: then});
+      expect(typeof canceler).toBe('function');
+
+      canceler();  // simulate promise resolution
+
+      expect(callback).toHaveBeenCalledWith(-1, undefined, '');
+      hb.verifyNoOutstandingExpectation();
+      hb.verifyNoOutstandingRequest();
+    });
+
+
     it('should throw an exception if no response defined', function() {
       hb.when('GET', '/test');
       expect(function() {
@@ -1006,8 +1024,8 @@ describe('ngMockE2E', function() {
         hb.when('GET', /\/passThrough\/.*/).passThrough();
         hb('GET', '/passThrough/23', null, callback);
 
-        expect(realHttpBackend).
-            toHaveBeenCalledOnceWith('GET', '/passThrough/23', null, callback, undefined);
+        expect(realHttpBackend).toHaveBeenCalledOnceWith(
+            'GET', '/passThrough/23', null, callback, undefined, undefined);
       });
     });
 
