Fixed http service to work correctly with cache

Author: pavelgj

lib/http.dart

@@ -1,16 +1,28 @@
 part of angular;
 
 class Http {
+  Map<String, async.Future<String>> _pendingRequests = <String, async.Future<String>>{};
+  
   async.Future<String> getString(String url, {bool withCredentials, void onProgress(dom.ProgressEvent e), Cache cache}) {
+    // We return a pending request only if caching is enabled.
+    if (cache != null && _pendingRequests.containsKey(url)) {
+      return _pendingRequests[url];
+    }
     var cachedValue = cache != null ? cache.get(url) : null;
     if (cachedValue != null) {
       return new async.Future.value(cachedValue);
     }
-    return dom.HttpRequest.getString(url, withCredentials: withCredentials, onProgress: onProgress).then((value) {
+    var result = dom.HttpRequest.getString(url, withCredentials: withCredentials, onProgress: onProgress).then((value) {
       if (cache != null) {
         cache.put(url, value);
       }
+      _pendingRequests.remove(url);
       return value;
+    }, onError: (error) {
+      _pendingRequests.remove(url);
+      throw error;
     });
+    _pendingRequests[url] = result;
+    return result;
   }
 }

test/_http.dart

@@ -6,24 +6,29 @@ import 'dart:html';
 import 'package:angular/angular.dart';
 
 class MockHttp extends Http {
-  Map<String, String> gets = {};
+  Map<String, MockHttpData> gets = {};
   List futures = [];
 
-  expectGET(url, content) {
-    gets[url] = content;
+  expectGET(String url, String content, {int times: 1}) {
+    gets[url] = new MockHttpData(content, times);
   }
 
-  flush() => gets.length == 0 ? Future.wait(futures) :
+  flush() => Future.wait(futures);
+  
+  assertAllGetsCalled() {
+    if (gets.length != 0) {
       throw "Expected GETs not called $gets";
+    }
+  }
 
   Future<String> getString(String url, {bool withCredentials, void onProgress(ProgressEvent e), Cache cache}) {
-    var cachedValue = cache != null ? cache.get(url) : null;
-    if (cachedValue != null) {
-      return new Future.value(cachedValue);
+    if (!gets.containsKey(url)) throw "Unexpected URL $url $gets";
+    var data = gets[url];
+    data.times--;
+    if (data.times <= 0) {
+      gets.remove(url);
     }
-
-    if (!gets.containsKey(url)) throw "Unexpected URL $url";
-    var expectedValue = gets.remove(url);
+    var expectedValue = data.value;
     if (cache != null) {
       cache.put(url, expectedValue);
     }
@@ -33,4 +38,12 @@ class MockHttp extends Http {
   }
 }
 
+class MockHttpData {
+  String value;
+  int times;
+  MockHttpData(this.value, this.times);
+  
+  toString() => value;
+}
+
 main() {}

test/_http_spec.dart

@@ -14,6 +14,27 @@ main() {
         expect(data).toEqual('response');
       }));
     });
+    
+    it('should replay an http request which is expected multiple times', () {
+      http.expectGET('request', 'response', times: 2);
+      http.getString('request').then(expectAsync1((data) {
+        expect(http.gets.length).toEqual(1);
+        expect(data).toEqual('response');
+        http.getString('request').then(expectAsync1((data) {
+          expect(http.gets.length).toEqual(0);
+          expect(data).toEqual('response');
+        }));
+      }));
+    });
+    
+    it('should throw an exeception on assertAllGetsCalled when not all expected GETs were called', () {
+      http.expectGET('request', 'response', times: 2);
+      http.getString('request').then(expectAsync1((data) {
+        expect(() {
+          http.assertAllGetsCalled();
+        }).toThrow('Expected GETs not called {request: response}');
+      }));
+    });
 
     it('should cache results', inject((CacheFactory $cacheFactory) {
       http.expectGET('request', 'response');
@@ -25,18 +46,6 @@ main() {
       }));
     }));
 
-    it('should return cached results', inject((CacheFactory $cacheFactory) {
-      http.expectGET('request', 'response');
-      Cache cache = $cacheFactory('test');
-      http.getString('request', cache: cache).then(expectAsync1((data) {
-        expect(data).toEqual('response');
-
-        http.getString('request', cache: cache).then(expectAsync1((data) {
-          expect(data).toEqual('response');
-        }));
-      }));
-    }));
-
     it('should barf on an unseen request', () {
       expect(() {
         http.getString('unknown');
@@ -47,6 +56,7 @@ main() {
       http.expectGET('request', 'response');
       expect(() {
         http.flush();
+        http.assertAllGetsCalled();
       }).toThrow('Expected GETs not called {request: response}');
     });
   });

test/templateurl_spec.dart

@@ -44,6 +44,10 @@ main() {
       module.directive(OnlyCssComponent);
       module.directive(InlineWithCssComponent);
     }));
+    
+    afterEach(inject((MockHttp $http) {
+      $http.assertAllGetsCalled();
+    }));
 
     it('should replace element with template from url', inject((MockHttp $http, Compiler $compile, Scope $rootScope,  Log log, Injector injector) {
       $http.expectGET('simple.html', '<div log="SIMPLE">Simple!</div>');
@@ -59,7 +63,7 @@ main() {
     }));
 
     it('should load template from URL once', inject((MockHttp $http, Compiler $compile, Scope $rootScope,  Log log, Injector injector) {
-      $http.expectGET('simple.html', '<div log="SIMPLE">Simple!</div>');
+      $http.expectGET('simple.html', '<div log="SIMPLE">Simple!</div>', times: 2);
 
       var element = $('<div><simple-url log>ignore</simple-url><simple-url log>ignore</simple-url><div>');
       $compile(element)(injector, element);