Open shouldRetry in RetryPolicy (#3181)

* adding more flexibility in reusing RetryPolicy

* running swift format

* changing the concept from reusable static helper methods to overridable `RetryPolicy` instance methods

* removing `ConditionalRetrier` and disallowing override of the delay calculation

* Update doc comment, code formatting.

Co-authored-by: Jon Shier <jon@jonshier.com>

Source/RetryPolicy.swift

@@ -1,7 +1,7 @@
 //
 //  RetryPolicy.swift
 //
-//  Copyright (c) 2019 Alamofire Software Foundation (http://alamofire.org/)
+//  Copyright (c) 2019-2020 Alamofire Software Foundation (http://alamofire.org/)
 //
 //  Permission is hereby granted, free of charge, to any person obtaining a copy
 //  of this software and associated documentation files (the "Software"), to deal
@@ -309,29 +309,32 @@ open class RetryPolicy: RequestInterceptor {
                     for session: Session,
                     dueTo error: Error,
                     completion: @escaping (RetryResult) -> Void) {
-        if
-            request.retryCount < retryLimit,
-            let httpMethod = request.request?.method,
-            retryableHTTPMethods.contains(httpMethod),
-            shouldRetry(response: request.response, error: error) {
-            let timeDelay = pow(Double(exponentialBackoffBase), Double(request.retryCount)) * exponentialBackoffScale
-            completion(.retryWithDelay(timeDelay))
+        if request.retryCount < retryLimit, shouldRetry(request: request, dueTo: error) {
+            completion(.retryWithDelay(pow(Double(exponentialBackoffBase), Double(request.retryCount)) * exponentialBackoffScale))
         } else {
             completion(.doNotRetry)
         }
     }
 
-    private func shouldRetry(response: HTTPURLResponse?, error: Error) -> Bool {
-        if let statusCode = response?.statusCode, retryableHTTPStatusCodes.contains(statusCode) {
+    /// Determines whether or not to retry the provided `Request`.
+    ///
+    /// - Parameters:
+    ///     - request: `Request` that failed due to the provided `Error`.
+    ///     - error:   `Error` encountered while executing the `Request`.
+    ///
+    /// - Returns:     `Bool` determining whether or not to retry the `Request`.
+    open func shouldRetry(request: Request, dueTo error: Error) -> Bool {
+        guard let httpMethod = request.request?.method, retryableHTTPMethods.contains(httpMethod) else { return false }
+        
+        if let statusCode = request.response?.statusCode, retryableHTTPStatusCodes.contains(statusCode) {
             return true
         } else {
             let errorCode = (error as? URLError)?.code
             let afErrorCode = (error.asAFError?.underlyingError as? URLError)?.code
-            if let code = errorCode ?? afErrorCode, retryableURLErrorCodes.contains(code) {
-                return true
-            } else {
-                return false
-            }
+
+            guard let code = errorCode ?? afErrorCode else { return false }
+
+            return retryableURLErrorCodes.contains(code)
         }
     }
 }

Tests/RetryPolicyTests.swift

@@ -76,6 +76,7 @@ class BaseRetryPolicyTestCase: BaseTestCase {
     lazy var unknownError = AFError.sessionTaskFailed(error: unknown)
 
     let retryableStatusCodes: Set<Int> = [408, 500, 502, 503, 504]
+    let statusCodes = Set(100...599)
 
     let retryableErrorCodes: Set<URLError.Code> = [.backgroundSessionInUseByAnotherProcess,
                                                    .backgroundSessionWasDisconnected,
@@ -130,6 +131,22 @@ class BaseRetryPolicyTestCase: BaseTestCase {
     var errorCodes: Set<URLError.Code> {
         retryableErrorCodes.union(nonRetryableErrorCodes)
     }
+
+    // MARK: Test Helpers
+
+    func request(method: HTTPMethod = .get, statusCode: Int? = nil) -> Request {
+        var response: HTTPURLResponse?
+
+        if let statusCode = statusCode {
+            response = HTTPURLResponse(url: url, statusCode: statusCode, httpVersion: nil, headerFields: nil)
+        }
+
+        return StubRequest(url, method: method, response: response, session: session)
+    }
+
+    func urlError(with code: URLError.Code) -> URLError {
+        NSError(domain: URLError.errorDomain, code: code.rawValue, userInfo: nil) as! URLError
+    }
 }
 
 // MARK: -
@@ -207,7 +224,6 @@ final class RetryPolicyTestCase: BaseRetryPolicyTestCase {
     func testThatRetryPolicyRetriesRequestsWithRetryableStatusCodes() {
         // Given
         let retryPolicy = RetryPolicy()
-        let statusCodes = Set(100...599)
         var results: [Int: RetryResult] = [:]
 
         // When
@@ -375,22 +391,6 @@ final class RetryPolicyTestCase: BaseRetryPolicyTestCase {
             XCTAssertNil(results[4]?.error)
         }
     }
-
-    // MARK: Test Helpers
-
-    func request(method: HTTPMethod = .get, statusCode: Int? = nil) -> Request {
-        var response: HTTPURLResponse?
-
-        if let statusCode = statusCode {
-            response = HTTPURLResponse(url: url, statusCode: statusCode, httpVersion: nil, headerFields: nil)
-        }
-
-        return StubRequest(url, method: method, response: response, session: session)
-    }
-
-    func urlError(with code: URLError.Code) -> URLError {
-        NSError(domain: URLError.errorDomain, code: code.rawValue, userInfo: nil) as! URLError
-    }
 }
 
 // MARK: -