RSC15a (#384)

Source/ARTFallback.h

@@ -7,10 +7,15 @@
 //
 
 #import <Foundation/Foundation.h>
+#import "CompatibilityMacros.h"
+
+ART_ASSUME_NONNULL_BEGIN
 
 @class ARTHttpResponse;
 @class ARTClientOptions;
 
+extern int (^ARTFallback_getRandomHostIndex)(int count);
+
 @interface ARTFallback : NSObject
 {
 
@@ -22,3 +27,5 @@
 -(NSString *) popFallbackHost;
 
 @end
+
+ART_ASSUME_NONNULL_END

Source/ARTFallback.m

@@ -13,6 +13,10 @@
 #import "ARTHttp.h"
 #import "ARTClientOptions.h"
 
+int (^ARTFallback_getRandomHostIndex)(int count) = ^int(int count) {
+    return arc4random() % count;
+};
+
 @interface ARTFallback ()
 
 @property (readwrite, strong, nonatomic) NSMutableArray * hosts;
@@ -28,7 +32,7 @@ - (id)init {
         NSMutableArray * hostArray =[[NSMutableArray alloc] initWithArray: [ARTDefault fallbackHosts]];
         size_t count = [hostArray count];
         for(int i=0; i <count; i++ ) {
-            int randomIndex = arc4random() % [hostArray count];
+            int randomIndex = ARTFallback_getRandomHostIndex((int)[hostArray count]);
             [self.hosts addObject:[hostArray objectAtIndex:randomIndex]];
             [hostArray removeObjectAtIndex:randomIndex];
         }

Source/ARTRest.m

@@ -122,10 +122,10 @@ - (void)executeRequestWithAuthentication:(NSMutableURLRequest *)request withMeth
 }
 
 - (void)executeRequest:(NSMutableURLRequest *)request completion:(void (^)(NSHTTPURLResponse *__art_nullable, NSData *__art_nullable, NSError *__art_nullable))callback {
-    return [self executeRequest:request completion:callback fallbacks:nil];
+    return [self executeRequest:request completion:callback fallbacks:nil retries:0];
 }
 
-- (void)executeRequest:(NSMutableURLRequest *)request completion:(void (^)(NSHTTPURLResponse *__art_nullable, NSData *__art_nullable, NSError *__art_nullable))callback fallbacks:(ARTFallback *)fallbacks {
+- (void)executeRequest:(NSMutableURLRequest *)request completion:(void (^)(NSHTTPURLResponse *__art_nullable, NSData *__art_nullable, NSError *__art_nullable))callback fallbacks:(ARTFallback *)fallbacks retries:(NSUInteger)retries {
     __block ARTFallback *blockFallbacks = fallbacks;
 
     [self.logger debug:__FILE__ line:__LINE__ message:@"%p executing request %@", self, request];
@@ -145,7 +145,7 @@ - (void)executeRequest:(NSMutableURLRequest *)request completion:(void (^)(NSHTT
                 error = dataError;
             }
         }
-        if ([self shouldRetryWithFallback:request response:response error:error]) {
+        if (retries < _options.httpMaxRetryCount && [self shouldRetryWithFallback:request response:response error:error]) {
             if (!blockFallbacks && [request.URL.host isEqualToString:[ARTDefault restHost]]) {
                 blockFallbacks = [[ARTFallback alloc] init];
             }
@@ -157,7 +157,7 @@ - (void)executeRequest:(NSMutableURLRequest *)request completion:(void (^)(NSHTT
                     NSURL *url = request.URL;
                     NSString *urlStr = [NSString stringWithFormat:@"%@://%@:%@%@?%@", url.scheme, host, url.port, url.path, (url.query ? url.query : @"")];
                     newRequest.URL = [NSURL URLWithString:urlStr];
-                    [self executeRequest:newRequest completion:callback fallbacks:fallbacks];
+                    [self executeRequest:newRequest completion:callback fallbacks:blockFallbacks retries:retries + 1];
                     return;
                 }
             }
@@ -273,4 +273,4 @@ - (BOOL)stats:(ARTStatsQuery *)query callback:(void (^)(__GENERIC(ARTPaginatedRe
     return self.encoders[self.defaultEncoding];
 }
 
-@end
+@end

Spec/RestClient.swift

@@ -468,11 +468,12 @@ class RestClient: QuickSpec {
                     let channel = client.channels.get("test")
 
                     var capturedURLs = [String]()
-                    testHTTPExecutor.afterRequest = { request in
+                    testHTTPExecutor.afterRequest = { request, callback in
                         capturedURLs.append(request.URL!.absoluteString)
                         if testHTTPExecutor.requests.count == 2 {
                             // Stop
                             testHTTPExecutor.http = nil
+                            callback!(nil, nil, nil)
                         }
                     }
 
@@ -490,26 +491,15 @@ class RestClient: QuickSpec {
                     expect(NSRegularExpression.match(capturedURLs[1], pattern: "//[a-e].ably-realtime.com")).to(beTrue())
                 }
 
-            }
-
-            // RSC15
-            context("Host Fallback") {
-
                 // RSC15e
                 it("every new HTTP request is first attempted to the primary host rest.ably.io") {
                     let options = ARTClientOptions(key: "xxxx:xxxx")
+                    options.httpMaxRetryCount = 1
                     let client = ARTRest(options: options)
                     client.httpExecutor = testHTTPExecutor
                     testHTTPExecutor.http = MockHTTP(network: .HostUnreachable)
                     let channel = client.channels.get("test")
 
-                    testHTTPExecutor.afterRequest = { _ in
-                        if testHTTPExecutor.requests.count == 2 {
-                            // Stop
-                            testHTTPExecutor.http = nil
-                        }
-                    }
-
                     waitUntil(timeout: testTimeout) { done in
                         channel.publish(nil, data: "nil") { _ in
                             done()
@@ -533,10 +523,85 @@ class RestClient: QuickSpec {
                     expect(NSRegularExpression.match(testHTTPExecutor.requests[2].URL!.absoluteString, pattern: "//rest.ably.io")).to(beTrue())
                 }
 
-            }
+                // RSC15a
+                context("retry hosts in random order") {
+                    let expectedHostOrder = [4, 3, 0, 2, 1]
 
-            // RSC15
-            context("Host Fallback") {
+                    let originalARTFallback_getRandomHostIndex = ARTFallback_getRandomHostIndex
+
+                    beforeEach {
+                        ARTFallback_getRandomHostIndex = {
+                            let hostIndexes = [1, 1, 0, 0, 0]
+                            var i = 0
+                            return { count in
+                                let hostIndex = hostIndexes[i]
+                                i += 1
+                                return Int32(hostIndex)
+                            }
+                        }()
+                    }
+
+                    afterEach {
+                        ARTFallback_getRandomHostIndex = originalARTFallback_getRandomHostIndex
+                    }
+
+                    it("until httpMaxRetryCount has been reached") {
+                        let options = ARTClientOptions(key: "xxxx:xxxx")
+                        let client = ARTRest(options: options)
+                        options.httpMaxRetryCount = 3
+                        client.httpExecutor = testHTTPExecutor
+                        testHTTPExecutor.http = MockHTTP(network: .HostUnreachable)
+                        testHTTPExecutor.afterRequest = { _, _ in
+                            if testHTTPExecutor.requests.count > Int(1 + options.httpMaxRetryCount) {
+                                fail("Should not retry more than \(options.httpMaxRetryCount)")
+                                testHTTPExecutor.http = nil
+                            }
+                        }
+                        let channel = client.channels.get("test")
+
+
+                        waitUntil(timeout: testTimeout) { done in
+                            channel.publish(nil, data: "nil") { _ in
+                                done()
+                            }
+                        }
+
+                        expect(testHTTPExecutor.requests).to(haveCount(Int(1 + options.httpMaxRetryCount)))
+
+                        let extractHostname = { (request: NSMutableURLRequest) in
+                            NSRegularExpression.extract(request.URL!.absoluteString, pattern: "[a-e].ably-realtime.com")
+                        }
+                        let resultFallbackHosts = testHTTPExecutor.requests.flatMap(extractHostname)
+                        let expectedFallbackHosts = Array(expectedHostOrder.map({ ARTDefault.fallbackHosts()[$0] as! String })[0..<Int(options.httpMaxRetryCount)])
+
+                        expect(resultFallbackHosts).to(equal(expectedFallbackHosts))
+                    }
+
+                    it("until all fallback hosts have been tried") {
+                        let options = ARTClientOptions(key: "xxxx:xxxx")
+                        options.httpMaxRetryCount = 10
+                        let client = ARTRest(options: options)
+                        client.httpExecutor = testHTTPExecutor
+                        testHTTPExecutor.http = MockHTTP(network: .HostUnreachable)
+                        let channel = client.channels.get("test")
+
+                        waitUntil(timeout: testTimeout) { done in
+                            channel.publish(nil, data: "nil") { _ in
+                                done()
+                            }
+                        }
+
+                        expect(testHTTPExecutor.requests).to(haveCount(ARTDefault.fallbackHosts().count + 1))
+
+                        let extractHostname = { (request: NSMutableURLRequest) in
+                            NSRegularExpression.extract(request.URL!.absoluteString, pattern: "[a-e].ably-realtime.com")
+                        }
+                        let resultFallbackHosts = testHTTPExecutor.requests.flatMap(extractHostname)
+                        let expectedFallbackHosts = expectedHostOrder.map { ARTDefault.fallbackHosts()[$0] as! String }
+
+                        expect(resultFallbackHosts).to(equal(expectedFallbackHosts))
+                    }
+                }
 
                 // RSC15d
                 context("should use an alternative host when") {
@@ -551,10 +616,11 @@ class RestClient: QuickSpec {
                             testHTTPExecutor.http = MockHTTP(network: caseTest)
                             let channel = client.channels.get("test")
 
-                            testHTTPExecutor.afterRequest = { _ in
+                            testHTTPExecutor.afterRequest = { _, callback in
                                 if testHTTPExecutor.requests.count == 2 {
                                     // Stop
                                     testHTTPExecutor.http = nil
+                                    callback!(nil, nil, nil)
                                 }
                             }
 

Spec/TestUtilities.swift

@@ -544,30 +544,31 @@ class TestProxyHTTPExecutor: NSObject, ARTHTTPExecutor {
     var requests: [NSMutableURLRequest] = []
     var responses: [NSHTTPURLResponse] = []
 
-    var beforeRequest: Optional<(NSMutableURLRequest)->()> = nil
-    var afterRequest: Optional<(NSMutableURLRequest)->()> = nil
+    var beforeRequest: Optional<(NSMutableURLRequest, ((NSHTTPURLResponse?, NSData?, NSError?) -> Void)?)->()> = nil
+    var afterRequest: Optional<(NSMutableURLRequest, ((NSHTTPURLResponse?, NSData?, NSError?) -> Void)?)->()> = nil
     var beforeProcessingDataResponse: Optional<(NSData?)->(NSData)> = nil
 
     func executeRequest(request: NSMutableURLRequest, completion callback: ((NSHTTPURLResponse?, NSData?, NSError?) -> Void)?) {
+        guard let http = self.http else {
+            return
+        }
         self.requests.append(request)
         if let performEvent = beforeRequest {
-            performEvent(request)
-        }
-        if let http = self.http {
-            http.executeRequest(request, completion: { response, data, error in
-                if let httpResponse = response {
-                    self.responses.append(httpResponse)
-                }
-                if let performEvent = self.beforeProcessingDataResponse {
-                    callback?(response, performEvent(data), error)
-                }
-                else {
-                    callback?(response, data, error)
-                }
-            })
+            performEvent(request, callback)
         }
+        http.executeRequest(request, completion: { response, data, error in
+            if let httpResponse = response {
+                self.responses.append(httpResponse)
+            }
+            if let performEvent = self.beforeProcessingDataResponse {
+                callback?(response, performEvent(data), error)
+            }
+            else {
+                callback?(response, data, error)
+            }
+        })
         if let performEvent = afterRequest {
-            performEvent(request)
+            performEvent(request, callback)
         }
     }
 
@@ -739,6 +740,19 @@ extension NSRegularExpression {
         return regex.rangeOfFirstMatchInString(value, options: [], range: range).location != NSNotFound
     }
 
+    class func extract(value: String?, pattern: String) -> String? {
+        guard let value = value else {
+            return nil
+        }
+        let options = NSRegularExpressionOptions()
+        let regex = try! NSRegularExpression(pattern: pattern, options: options)
+        let range = NSMakeRange(0, value.lengthOfBytesUsingEncoding(NSUTF8StringEncoding))
+        let result = regex.firstMatchInString(value, options: [], range: range)
+        guard let textRange = result?.rangeAtIndex(0) else { return nil }
+        let convertedRange =  value.startIndex.advancedBy(textRange.location)..<value.startIndex.advancedBy(textRange.location+textRange.length)
+        return value.substringWithRange(convertedRange)
+    }
+
 }
 
 extension String {