ETagManager: refactored e-tag creation and tests (#2671)

We had lots of different parts of the code doing `request.url?.absoluteString` and also using different values for e-tags. This refactor combines all that to use a single `eTag(for:)` method. I did this as part of #2665, which is getting closed, but it will still be useful if we ever decide to change the content of e-tags.
RevenueCat · Jun 21, 2023 · 7298dfa · 7298dfa
1 parent b441846
commit 7298dfa
Show file tree

Hide file tree

Showing 4 changed files with 157 additions and 170 deletions.
diff --git a/Sources/Networking/HTTPClient/ETagManager.swift b/Sources/Networking/HTTPClient/ETagManager.swift
@@ -133,6 +133,17 @@ class ETagManager {
 
 }
 
+extension ETagManager {
+
+    // Visible for tests
+    static func cacheKey(for request: URLRequest) -> String? {
+        return request.url?.absoluteString
+    }
+
+}
+
+// MARK: - Private
+
 private extension ETagManager {
 
     func shouldUseCachedVersion(responseCode: HTTPStatusCode) -> Bool {
@@ -141,7 +152,7 @@ private extension ETagManager {
 
     func storedETagAndResponse(for request: URLRequest) -> Response? {
         return self.userDefaults.read {
-            if let cacheKey = self.eTagDefaultCacheKey(for: request),
+            if let cacheKey = Self.cacheKey(for: request),
                let value = $0.object(forKey: cacheKey),
                let data = value as? Data {
                 return try? JSONDecoder.default.decode(Response.self, jsonData: data)
@@ -172,7 +183,7 @@ private extension ETagManager {
     }
 
     func storeIfPossible(_ response: Response, for request: URLRequest) {
-        if let cacheKey = self.eTagDefaultCacheKey(for: request),
+        if let cacheKey = Self.cacheKey(for: request),
            let dataToStore = response.asData() {
             Logger.verbose(Strings.etag.storing_response(request, response))
 
@@ -182,10 +193,6 @@ private extension ETagManager {
         }
     }
 
-    func eTagDefaultCacheKey(for request: URLRequest) -> String? {
-        return request.url?.absoluteString
-    }
-
     var shouldIgnoreVerificationErrors: Bool {
         return !self.verificationMode.isEnabled
     }
@@ -274,6 +281,8 @@ private extension HTTPResponse {
     func shouldStore(ignoreVerificationErrors: Bool) -> Bool {
         return (
             self.statusCode != .notModified &&
+            // Note that we do want to store 400 responses to help the server
+            // If the request was wrong, it will also be wrong the next time.
             !self.statusCode.isServerError &&
             (ignoreVerificationErrors || self.verificationResult != .failed)
         )

diff --git a/Sources/Networking/HTTPClient/HTTPClient.swift b/Sources/Networking/HTTPClient/HTTPClient.swift
@@ -100,7 +100,6 @@ extension HTTPClient {
     enum RequestHeader: String {
 
         case authorization = "Authorization"
-        case location = "Location"
         case nonce = "X-Nonce"
         case eTag = "X-RevenueCat-ETag"
         case eTagValidationTime = "X-RC-Last-Refresh-Time"
@@ -110,6 +109,7 @@ extension HTTPClient {
     enum ResponseHeader: String {
 
         case eTag = "X-RevenueCat-ETag"
+        case location = "Location"
         case signature = "X-Signature"
         case requestDate = "X-RevenueCat-Request-Time"
         case contentType = "Content-Type"