FXIOS-731 ⁃ HTTP HEAD etags to cut down on data requests from Breach …

…Alerts (#7100)

* first pass

* etag & last accessed date integration

* test compatibility

* better completion handlingg

* cleanup

* review 1

* review 2

* Update BreachAlertsManager.swift

* formatting/test reformation

* fix logic errors

* Update BreachAlertsManager.swift

* fix logic

* remove mock data

Client/Frontend/Login Management/BreachAlertsClient.swift

@@ -11,7 +11,8 @@ struct BreachAlertsError: MaybeErrorType {
 }
 /// For mocking and testing BreachAlertsClient.
 protocol BreachAlertsClientProtocol {
-    func fetchData(endpoint: BreachAlertsClient.Endpoint, completion: @escaping (_ result: Maybe<Data>) -> Void)
+    func fetchEtag(endpoint: BreachAlertsClient.Endpoint, profile: Profile, completion: @escaping (_ etag: String?) -> Void)
+    func fetchData(endpoint: BreachAlertsClient.Endpoint, profile: Profile, completion: @escaping (_ result: Maybe<Data>) -> Void)
 }
 
 /// Handles all network requests for BreachAlertsManager.
@@ -20,28 +21,70 @@ public class BreachAlertsClient: BreachAlertsClientProtocol {
     public enum Endpoint: String {
         case breachedAccounts = "https://monitor.firefox.com/hibp/breaches"
     }
+    static let etagKey = "BreachAlertsDataEtag"
+    static let etagDateKey = "BreachAlertsDataDate"
 
-    /// Makes a network request to an endpoint and hands off the result to a completion handler.
-    public func fetchData(endpoint: Endpoint, completion: @escaping (_ result: Maybe<Data>) -> Void) {
-        guard let url = URL(string: endpoint.rawValue) else {
-            completion(Maybe(failure: BreachAlertsError(description: "bad endpoint URL")))
-            return
+    /// Makes a header-only request to an endpoint and hands off the endpoint's etag to a completion handler.
+    func fetchEtag(endpoint: Endpoint, profile: Profile, completion: @escaping (_ etag: String?) -> Void) {
+        guard let url = URL(string: endpoint.rawValue) else { return }
+        var request = URLRequest(url: url)
+        request.httpMethod = "HEAD"
+
+        dataTask?.cancel()
+        dataTask = URLSession.shared.dataTask(with: request) { _, response, _ in
+            guard let response = response as? HTTPURLResponse else { return }
+            guard response.statusCode < 400 else {
+                Sentry.shared.send(message: "BreachAlerts: fetchEtag: HTTP status code: \(response.statusCode)")
+                completion(nil)
+                return
+            }
+            guard let etag = response.allHeaderFields["Etag"] as Any as? String else {
+                completion(nil)
+                assert(false)
+                return
+            }
+            DispatchQueue.main.async {
+                completion(etag)
+            }
         }
+        dataTask?.resume()
+    }
+
+    /// Makes a network request to an endpoint and hands off the result to a completion handler.
+    func fetchData(endpoint: Endpoint, profile: Profile, completion: @escaping (_ result: Maybe<Data>) -> Void) {
+        guard let url = URL(string: endpoint.rawValue) else { return }
+
         dataTask?.cancel()
         dataTask = URLSession.shared.dataTask(with: url) { data, response, error in
-            guard validatedHTTPResponse(response) != nil else {
-                completion(Maybe(failure: BreachAlertsError(description: "invalid HTTP response")))
+            guard let response = response as? HTTPURLResponse else { return }
+            guard response.statusCode < 400 else {
+                Sentry.shared.send(message: "BreachAlerts: fetchData: HTTP status code: \(response.statusCode)")
                 return
             }
             if let error = error {
                 completion(Maybe(failure: BreachAlertsError(description: error.localizedDescription)))
+                Sentry.shared.send(message: "BreachAlerts: fetchData: \(error)")
                 return
             }
-            guard let data = data, !data.isEmpty else {
-                completion(Maybe(failure: BreachAlertsError(description: "empty data")))
+            guard let data = data else {
+                completion(Maybe(failure: BreachAlertsError(description: "invalid data")))
+                Sentry.shared.send(message: "BreachAlerts: fetchData: invalid data")
+                assert(false)
                 return
             }
-            completion(Maybe(success: data))
+
+            guard let etag = response.allHeaderFields["Etag"] as Any as? String else { return }
+            let date = Date.now()
+
+            if profile.prefs.stringForKey(BreachAlertsClient.etagKey) != etag {
+                profile.prefs.setString(etag, forKey: BreachAlertsClient.etagKey)
+            }
+            if profile.prefs.timestampForKey(BreachAlertsClient.etagDateKey) != date {
+                profile.prefs.setTimestamp(date, forKey: BreachAlertsClient.etagDateKey)
+            }
+            DispatchQueue.main.async {
+                completion(Maybe(success: data))
+            }
         }
         dataTask?.resume()
     }

Client/Frontend/Login Management/BreachAlertsManager.swift

@@ -30,50 +30,77 @@ final public class BreachAlertsManager {
     static let darkMode = UIColor(red: 1.00, green: 0.02, blue: 0.35, alpha: 1.00)
     static let monitorAboutUrl = URL(string: "https://monitor.firefox.com/about")
     var breaches = Set<BreachRecord>()
-    var breachAlertsClient: BreachAlertsClientProtocol
-
-    init(_ client: BreachAlertsClientProtocol = BreachAlertsClient()) {
-        self.breachAlertsClient = client
+    var client: BreachAlertsClientProtocol
+    var profile: Profile!
+    private lazy var cacheURL: URL? = {
+        guard let path = try? self.profile.files.getAndEnsureDirectory() else {
+            return nil
+        }
+        return URL(fileURLWithPath: path, isDirectory: true).appendingPathComponent("breaches.json")
+    }()
+    private let dateFormatter = DateFormatter()
+    init(_ client: BreachAlertsClientProtocol = BreachAlertsClient(), profile: Profile) {
+        self.client = client
+        self.profile = profile
     }
 
     /// Loads breaches from Monitor endpoint using BreachAlertsClient.
     ///    - Parameters:
     ///         - completion: a completion handler for the processed breaches
     func loadBreaches(completion: @escaping (Maybe<Set<BreachRecord>>) -> Void) {
-        self.breachAlertsClient.fetchData(endpoint: .breachedAccounts) { maybeData in
-            guard let data = maybeData.successValue else {
-                completion(Maybe(failure: BreachAlertsError(description: "failed to load breaches data")))
-                return
-            }
-            guard let decoded = try? JSONDecoder().decode(Set<BreachRecord>.self, from: data) else {
-                completion(Maybe(failure: BreachAlertsError(description: "JSON data decode failure")))
+        guard let cacheURL = self.cacheURL else {
+            self.fetchAndSaveBreaches(completion)
+            return
+        }
+
+        // 1. check for local breaches file
+        guard FileManager.default.fileExists(atPath: cacheURL.path) else {
+            // 1a. no local file, so fetch and save as normal and hand off
+            self.fetchAndSaveBreaches(completion)
+            return
+        }
+
+        // 1b. local file exists, so load from that
+        guard let fileData = FileManager.default.contents(atPath: cacheURL.path) else {
+            completion(Maybe(failure: BreachAlertsError(description: "failed to get data from breach.json")))
+            Sentry.shared.send(message: "BreachAlerts: failed to get data from breach.json")
+            try? FileManager.default.removeItem(at: cacheURL) // bad file, so delete it
+            self.fetchAndSaveBreaches(completion)
+            return
+        }
+
+        // 2. check the last time breach endpoint was accessed
+        guard let dateLastAccessed = profile.prefs.timestampForKey(BreachAlertsClient.etagDateKey) else {
+            profile.prefs.removeObjectForKey(BreachAlertsClient.etagDateKey) // bad key, so delete it
+            self.fetchAndSaveBreaches(completion)
+            return
+        }
+        let timeUntilNextUpdate = UInt64(60 * 60 * 24 * 3 * 1000) // 3 days in milliseconds
+        let shouldUpdateDate = dateLastAccessed + timeUntilNextUpdate
+
+        // 3. if 3 days have not passed since last update...
+        guard Date.now() >= shouldUpdateDate else {
+            // 3a. no need to refetch. decode local data and hand off
+            decodeData(data: fileData, completion)
+            return
+        }
+
+        // 3b. should update - check if the etag is different
+        client.fetchEtag(endpoint: .breachedAccounts, profile: self.profile) { etag in
+            guard let etag =  etag else {
+                self.profile.prefs.removeObjectForKey(BreachAlertsClient.etagKey) // bad key, so delete it
+                self.fetchAndSaveBreaches(completion)
                 return
             }
+            let savedEtag = self.profile.prefs.stringForKey(BreachAlertsClient.etagKey)
 
-            self.breaches = decoded
-            // remove for release
-            self.breaches.insert(BreachRecord(
-             name: "MockBreach",
-             title: "A Mock Blockbuster Record",
-             domain: "blockbuster.com",
-             breachDate: "1970-01-02",
-             description: "A mock BreachRecord for testing purposes."
-            ))
-            self.breaches.insert(BreachRecord(
-             name: "MockBreach",
-             title: "A Mock Lorem Ipsum Record",
-             domain: "lipsum.com",
-             breachDate: "1970-01-02",
-             description: "A mock BreachRecord for testing purposes."
-            ))
-            self.breaches.insert(BreachRecord(
-             name: "MockBreach",
-             title: "A Mock Swift Breach Record",
-             domain: "swift.org",
-             breachDate: "1970-01-02",
-             description: "A mock BreachRecord for testing purposes."
-            ))
-            completion(Maybe(success: self.breaches))
+            // 4. if it is, refetch the data and hand entire Set of BreachRecords off
+            if etag != savedEtag {
+                self.fetchAndSaveBreaches(completion)
+            } else {
+                self.profile.prefs.setTimestamp(Date.now(), forKey: BreachAlertsClient.etagDateKey)
+                self.decodeData(data: fileData, completion)
+            }
         }
     }
 
@@ -98,9 +125,8 @@ final public class BreachAlertsManager {
             }
             for item in potentialUserBreaches {
                 let pwLastChanged = TimeInterval(item.timePasswordChanged/1000)
-                let dateFormatter = DateFormatter()
-                dateFormatter.dateFormat = "yyyy-MM-dd"
-                guard let breachDate = dateFormatter.date(from: breach.breachDate)?.timeIntervalSince1970, pwLastChanged < breachDate else {
+                self.dateFormatter.dateFormat = "yyyy-MM-dd"
+                guard let breachDate = self.dateFormatter.date(from: breach.breachDate)?.timeIntervalSince1970, pwLastChanged < breachDate else {
                     continue
                 }
                 result.insert(item)
@@ -136,18 +162,44 @@ final public class BreachAlertsManager {
         let baseDomain = self.baseDomainForLogin(login)
         for breach in self.breaches where breach.domain == baseDomain {
             let pwLastChanged = TimeInterval(login.timePasswordChanged/1000)
-            let dateFormatter = DateFormatter()
-            dateFormatter.dateFormat = "yyyy-MM-dd"
-            guard let breachDate = dateFormatter.date(from: breach.breachDate)?.timeIntervalSince1970, pwLastChanged < breachDate else {
+            self.dateFormatter.dateFormat = "yyyy-MM-dd"
+            guard let breachDate = self.dateFormatter.date(from: breach.breachDate)?.timeIntervalSince1970, pwLastChanged < breachDate else {
                 continue
             }
             return breach
         }
         return nil
     }
 
+    // MARK: - Helper Functions
     private func baseDomainForLogin(_ login: LoginRecord) -> String {
         guard let result = login.hostname.asURL?.baseDomain else { return login.hostname }
         return result
     }
+
+    private func fetchAndSaveBreaches(_ completion: @escaping (Maybe<Set<BreachRecord>>) -> Void) {
+        guard let cacheURL = self.cacheURL else {
+            return
+        }
+        self.client.fetchData(endpoint: .breachedAccounts, profile: self.profile) { maybeData in
+            guard let fetchedData = maybeData.successValue else { return }
+            try? FileManager.default.removeItem(atPath: cacheURL.path)
+            FileManager.default.createFile(atPath: cacheURL.path, contents: fetchedData, attributes: nil)
+
+            guard let data = FileManager.default.contents(atPath: cacheURL.path) else { return }
+            self.decodeData(data: data, completion)
+        }
+    }
+
+    private func decodeData(data: Data, _ completion: @escaping (Maybe<Set<BreachRecord>>) -> Void) {
+        guard let decoded = try? JSONDecoder().decode(Set<BreachRecord>.self, from: data) else {
+            print(BreachAlertsError(description: "JSON data decode failure"))
+            assert(false)
+            return
+        }
+
+        self.breaches = decoded
+
+        completion(Maybe(success: self.breaches))
+    }
 }

Client/Frontend/Login Management/LoginListViewModel.swift

@@ -24,7 +24,9 @@ final class LoginListViewModel {
         }
     }
     fileprivate let helper = LoginListDataSourceHelper()
-    private(set) var breachAlertsManager = BreachAlertsManager()
+    private(set) lazy var breachAlertsManager: BreachAlertsManager = {
+        return BreachAlertsManager(profile: self.profile)
+    }()
     private(set) var userBreaches: Set<LoginRecord>?
     private(set) var breachIndexPath = Set<IndexPath>() {
         didSet {
@@ -61,11 +63,7 @@ final class LoginListViewModel {
     func queryLogins(_ query: String) -> Deferred<Maybe<[LoginRecord]>> {
         let deferred = Deferred<Maybe<[LoginRecord]>>()
         profile.logins.searchLoginsWithQuery(query) >>== { logins in
-            var log = logins.asArray()
-            log.append(LoginRecord(fromJSONDict: ["hostname" : "https://blockbuster.com", "timePasswordChanged": 46800000, "password": "ipsum"]))
-            log.append(LoginRecord(fromJSONDict: ["hostname" : "https://lipsum.com", "timePasswordChanged": 46800000, "username": "lorem", "password": "ipsum"]))
-            log.append(LoginRecord(fromJSONDict: ["hostname" : "https://swift.org", "timePasswordChanged": 46800000, "username": "username", "password": "ipsum"]))
-            deferred.fillIfUnfilled(Maybe(success: log))
+            deferred.fillIfUnfilled(Maybe(success: logins.asArray()))
             succeed()
         }
         return deferred
@@ -137,7 +135,7 @@ final class LoginListViewModel {
     }
 
     func setBreachAlertsManager(_ client: BreachAlertsClientProtocol) {
-        self.breachAlertsManager = BreachAlertsManager(client)
+        self.breachAlertsManager = BreachAlertsManager(client, profile: profile)
     }
 
     // MARK: - UX Constants

Client/Frontend/Settings/LoginDetailViewController.swift

@@ -149,8 +149,9 @@ extension LoginDetailViewController: UITableViewDataSource {
             }
             breachDetailView.setup(breach)
 
-            breachDetailView.learnMoreButton.addTarget(self, action: #selector(didTapBreachLearnMore), for: .touchUpInside)
-            let breachLinkGesture = UITapGestureRecognizer(target: self, action: #selector(didTapBreachLink(_:)))
+            breachDetailView.learnMoreButton.addTarget(self, action: #selector(LoginDetailViewController.didTapBreachLearnMore), for: .touchUpInside)
+            let breachLinkGesture = UITapGestureRecognizer(target: self, action: #selector(LoginDetailViewController
+                .didTapBreachLink(_:)))
             breachDetailView.goToButton.addGestureRecognizer(breachLinkGesture)
             breachCell.isAccessibilityElement = false
             breachCell.contentView.accessibilityElementsHidden = true

ClientTests/BreachAlertsTests.swift

@@ -14,7 +14,6 @@ let blockbusterBreach = BreachRecord(
  breachDate: "1970-01-02",
  description: "A mock BreachRecord for testing purposes."
 )
-// remove for official release
 let lipsumBreach = BreachRecord(
  name: "MockBreach",
  title: "A Mock Lorem Ipsum Record",
@@ -31,14 +30,19 @@ let longBreach = BreachRecord(
 )
 let unbreachedLogin = LoginRecord(fromJSONDict: ["hostname" : "http://unbreached.com", "timePasswordChanged": 1594411049000])
 let breachedLogin = LoginRecord(fromJSONDict: ["hostname" : "http://blockbuster.com", "timePasswordChanged": 46800000])
+
 class MockBreachAlertsClient: BreachAlertsClientProtocol {
-    func fetchData(endpoint: BreachAlertsClient.Endpoint, completion: @escaping (Maybe<Data>) -> Void) {
-        guard let mockData = try? JSONEncoder().encode([blockbusterBreach].self) else {
+    func fetchEtag(endpoint: BreachAlertsClient.Endpoint, profile: Client.Profile, completion: @escaping (String?) -> Void) {
+        completion("33a64df551425fcc55e4d42a148795d9f25f89d4")
+    }
+    func fetchData(endpoint: BreachAlertsClient.Endpoint, profile: Client.Profile, completion: @escaping (Maybe<Data>) -> Void) {
+        guard let mockData = try? JSONEncoder().encode([blockbusterBreach, longBreach, lipsumBreach].self) else {
             completion(Maybe(failure: BreachAlertsError(description: "failed to encode mockRecord")))
             return
         }
         completion(Maybe(success: mockData))
     }
+    var etag: String? = nil
 }
 
 class BreachAlertsTests: XCTestCase {
@@ -47,7 +51,7 @@ class BreachAlertsTests: XCTestCase {
     let breachedLoginSet = Set<LoginRecord>(arrayLiteral: breachedLogin)
 
     override func setUp() {
-        self.breachAlertsManager = BreachAlertsManager(MockBreachAlertsClient())
+        self.breachAlertsManager = BreachAlertsManager(MockBreachAlertsClient(), profile: MockProfile())
     }
 
     func testDataRequest() {

ClientTests/LoginsListViewModelTests.swift

@@ -45,19 +45,19 @@ class LoginsListViewModelTests: XCTestCase {
     func testQueryLogins() {
         let emptyQueryResult = self.viewModel.queryLogins("")
         XCTAssertTrue(emptyQueryResult.value.isSuccess)
-        XCTAssertEqual(emptyQueryResult.value.successValue?.count, 13)
+        XCTAssertEqual(emptyQueryResult.value.successValue?.count, 10)
 
         let exampleQueryResult = self.viewModel.queryLogins("example")
         XCTAssertTrue(exampleQueryResult.value.isSuccess)
-        XCTAssertEqual(exampleQueryResult.value.successValue?.count, 13)
+        XCTAssertEqual(exampleQueryResult.value.successValue?.count, 10)
 
         let threeQueryResult = self.viewModel.queryLogins("3")
         XCTAssertTrue(threeQueryResult.value.isSuccess)
-        XCTAssertEqual(threeQueryResult.value.successValue?.count, 4)
+        XCTAssertEqual(threeQueryResult.value.successValue?.count, 1)
 
         let zQueryResult = self.viewModel.queryLogins("yxz")
         XCTAssertTrue(zQueryResult.value.isSuccess)
-        XCTAssertEqual(zQueryResult.value.successValue?.count, 3)
+        XCTAssertEqual(zQueryResult.value.successValue?.count, 0)
     }
 
     func testIsDuringSearchControllerDismiss() {