Fortify local notifications code

Classes/Notifications/NotificationModelController.swift

@@ -51,6 +51,9 @@ final class NotificationModelController {
         width: CGFloat,
         completion: @escaping (Result<([NotificationViewModel], Int?)>) -> Void
         ) {
+        // hack to prevent double-fetching notifications when awaking from bg fetch
+        guard UIApplication.shared.applicationState != .background else { return }
+
         let badge = githubClient.badge
         let contentSizeCategory = UIContentSizeCategory.preferred
         // TODO move handling + parsing to a single method?

Classes/Systems/AppDelegate.swift

@@ -95,13 +95,8 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
         return false
     }
 
-    private var backgroundFetchClient: GithubClient? = nil
     func application(_ application: UIApplication, performFetchWithCompletionHandler completionHandler: @escaping (UIBackgroundFetchResult) -> Void) {
-        guard let userSession = sessionManager.focusedUserSession else {
-            return
-        }
-        backgroundFetchClient = newGithubClient(userSession: userSession)
-        backgroundFetchClient?.badge.fetch(application: application, handler: completionHandler)
+        rootNavigationManager.client?.badge.fetch(application: application, handler: completionHandler)
     }
 
 }

Classes/Systems/BadgeNotifications.swift

@@ -43,9 +43,9 @@ final class BadgeNotifications {
         UNUserNotificationCenter.current().getNotificationSettings { settings in
             DispatchQueue.main.async {
                 switch settings.authorizationStatus {
-                case .notDetermined, .denied:
+                case .denied:
                     callback(.error(nil))
-                case .authorized, .provisional:
+                case .authorized, .provisional, .notDetermined:
                     callback(.success((isBadgeEnabled, isLocalNotificationEnabled)))
                 }
             }
@@ -115,17 +115,15 @@ final class BadgeNotifications {
         completion: ((Bool) -> Void)? = nil
         ) {
         localNotificationsCache.update(notifications: notifications) { [weak self] filtered in
-            let changed = notifications.count != filtered.count
-            if showAlert && changed {
+            if showAlert {
                 self?.sendLocalPush(for: filtered)
             }
-            completion?(changed)
+            completion?(filtered.count > 0)
         }
     }
 
     private func sendLocalPush(for notifications: [V3Notification]) {
         let center = UNUserNotificationCenter.current()
-
         notifications.forEach {
             guard let type = NotificationType(rawValue: $0.subject.type.rawValue),
                 let identifier = $0.subject.identifier

Classes/Systems/LocalNotificationsCache.swift

@@ -12,18 +12,20 @@ import GitHubAPI
 
 final class LocalNotificationsCache {
 
-    private let path: String = {
+    private static let sqlitePath: String = {
         let path = NSSearchPathForDirectoriesInDomains(.documentDirectory, .userDomainMask, true)[0]
         return "\(path)/read-notifications.sqlite"
     }()
+
+    private let path: String
     private lazy var queue: FMDatabaseQueue = {
         return FMDatabaseQueue(path: self.path)
     }()
-    private let defaults = UserDefaults.standard
-    private let setupKey = "com.whoisryannystrom.freetime.local-notifications.setup"
 
-    private var isFirstSetup: Bool {
-        return defaults.bool(forKey: setupKey)
+    init(
+        path: String = LocalNotificationsCache.sqlitePath
+        ) {
+        self.path = path
     }
 
     func update(
@@ -63,18 +65,21 @@ final class LocalNotificationsCache {
                     }
                 }
 
-                // add latest notification ids in the db
-                let insertSanitized = map.keys.map { _ in "(?)" }.joined(separator: ", ")
-                try db.executeUpdate(
-                    "insert into \(table) (\(apiCol)) values \(insertSanitized)",
-                    values: apiIDs
-                )
+                // only perform updates if there are new notifications
+                if map.count > 0 {
+                    // add latest notification ids in the db
+                    let insertSanitized = map.keys.map { _ in "(?)" }.joined(separator: ", ")
+                    try db.executeUpdate(
+                        "insert into \(table) (\(apiCol)) values \(insertSanitized)",
+                        values: map.keys.map { $0 }
+                    )
 
-                // cap the local database to latest 1000 notifications
-                try db.executeUpdate(
-                    "delete from \(table) where \(idCol) not in (select \(idCol) from \(table) order by \(idCol) desc limit 1000)",
-                    values: nil
-                )
+                    // cap the local database to latest 1000 notifications
+                    try db.executeUpdate(
+                        "delete from \(table) where \(idCol) not in (select \(idCol) from \(table) order by \(idCol) desc limit 1000)",
+                        values: nil
+                    )
+                }
             } catch {
                 print("failed: \(error.localizedDescription)")
             }

Classes/Systems/RootNavigationManager.swift

@@ -22,7 +22,7 @@ final class RootNavigationManager: GitHubSessionListener {
     // keep alive between switching accounts
     private var settingsRootViewController: UIViewController?
 
-    private var lastClient: GithubClient?
+    private(set) var client: GithubClient?
 
     init(
         sessionManager: GitHubSessionManager,
@@ -62,7 +62,7 @@ final class RootNavigationManager: GitHubSessionListener {
         guard let userSession = userSession else { return }
 
         let client = newGithubClient(userSession: userSession)
-        lastClient = client
+        self.client = client
 
         fetchUsernameForMigrationIfNecessary(client: client, userSession: userSession, sessionManager: sessionManager)
 

Freetime.xcodeproj/project.pbxproj

@@ -228,6 +228,7 @@
 		29764C141FDC4DB60095FF95 /* SettingsLabel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 29764C131FDC4DB60095FF95 /* SettingsLabel.swift */; };
 		2977788820B0DAD500F2AFC2 /* ViewMarkdownViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2977788720B0DAD500F2AFC2 /* ViewMarkdownViewController.swift */; };
 		2977788A20B306F200F2AFC2 /* LabelLayoutManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2977788920B306F200F2AFC2 /* LabelLayoutManager.swift */; };
+		2977D8BF215AE12D0073F737 /* LocalNotificationCacheTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2977D8BE215AE12D0073F737 /* LocalNotificationCacheTests.swift */; };
 		29792B191FFB10A3007A0C57 /* UIViewController+MessageAutocompleteControllerLayoutDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 29792B181FFB10A3007A0C57 /* UIViewController+MessageAutocompleteControllerLayoutDelegate.swift */; };
 		29792B1B1FFB21AD007A0C57 /* AutocompleteController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 29792B1A1FFB21AD007A0C57 /* AutocompleteController.swift */; };
 		29792B1D1FFB2FC6007A0C57 /* IssueManagingNavSectionController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 29792B1C1FFB2FC6007A0C57 /* IssueManagingNavSectionController.swift */; };
@@ -744,6 +745,7 @@
 		29764C131FDC4DB60095FF95 /* SettingsLabel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsLabel.swift; sourceTree = "<group>"; };
 		2977788720B0DAD500F2AFC2 /* ViewMarkdownViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ViewMarkdownViewController.swift; sourceTree = "<group>"; };
 		2977788920B306F200F2AFC2 /* LabelLayoutManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LabelLayoutManager.swift; sourceTree = "<group>"; };
+		2977D8BE215AE12D0073F737 /* LocalNotificationCacheTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocalNotificationCacheTests.swift; sourceTree = "<group>"; };
 		29792B181FFB10A3007A0C57 /* UIViewController+MessageAutocompleteControllerLayoutDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIViewController+MessageAutocompleteControllerLayoutDelegate.swift"; sourceTree = "<group>"; };
 		29792B1A1FFB21AD007A0C57 /* AutocompleteController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AutocompleteController.swift; sourceTree = "<group>"; };
 		29792B1C1FFB2FC6007A0C57 /* IssueManagingNavSectionController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IssueManagingNavSectionController.swift; sourceTree = "<group>"; };
@@ -1547,9 +1549,10 @@
 				29A476B11ED24D99005D0953 /* IssueTests.swift */,
 				29C2950D1EC7B43B00D46CD2 /* ListKitTestCase.swift */,
 				29C295091EC7AFA500D46CD2 /* ListTestKit.swift */,
-				BD89007D20B8844B0026013F /* NetworkingURLPathTests.swift */,
+				2977D8BE215AE12D0073F737 /* LocalNotificationCacheTests.swift */,
 				49AF91B0204B416500DFF325 /* MergeTests.swift */,
 				DC60C6CC1F98344B00241271 /* Mocks */,
+				BD89007D20B8844B0026013F /* NetworkingURLPathTests.swift */,
 				49D028FF1F91D90C00E39094 /* ReactionTests.swift */,
 				DC60C6CF1F9836C500241271 /* Search Tests */,
 				9870B9071FC74E300009719C /* SecretsTests.swift */,
@@ -3090,6 +3093,7 @@
 				293A45771F296B7E00DD1006 /* ListKitTestCase.swift in Sources */,
 				BD89007E20B8844B0026013F /* NetworkingURLPathTests.swift in Sources */,
 				BD3761B0209E032500401DFB /* BookmarkNavigationItemTests.swift in Sources */,
+				2977D8BF215AE12D0073F737 /* LocalNotificationCacheTests.swift in Sources */,
 				DC5C02C51F9C6E3500E80B9F /* SearchQueryTests.swift in Sources */,
 				293A457E1F296BD500DD1006 /* API.swift in Sources */,
 				49AF91B1204B416500DFF325 /* MergeTests.swift in Sources */,

FreetimeTests/LocalNotificationCacheTests.swift

@@ -0,0 +1,109 @@
+//
+//  LocalNotificationCacheTests.swift
+//  FreetimeTests
+//
+//  Created by Ryan Nystrom on 9/25/18.
+//  Copyright © 2018 Ryan Nystrom. All rights reserved.
+//
+
+import XCTest
+
+@testable import Freetime
+@testable import GitHubAPI
+
+class LocalNotificationCacheTests: XCTestCase {
+
+    func path(for test: String) -> String {
+        let clean = test.replacingOccurrences(of: "()", with: "")
+        let path = NSSearchPathForDirectoriesInDomains(.documentDirectory, .userDomainMask, true)[0]
+        return "\(path)/\(clean).sqlite"
+    }
+
+    func clear(for test: String) {
+        let path = self.path(for: test)
+        do {
+            try FileManager.default.removeItem(atPath: path)
+        } catch  {
+            print("\(error.localizedDescription)\npath: \(path)")
+        }
+    }
+
+    func test_whenNoNotifications_thatNothingReturned() {
+        clear(for: #function)
+
+        let cache = LocalNotificationsCache(path: path(for: #function))
+        var executed = false
+        cache.update(notifications: []) { results in
+            executed = true
+            XCTAssertEqual(results.count, 0)
+        }
+        XCTAssertTrue(executed)
+    }
+
+    func makeRepo() -> V3Repository {
+        return V3Repository(
+            description: nil,
+            fork: false,
+            fullName: "org/name",
+            id: 42,
+            name: "name",
+            owner: V3User(
+                avatarUrl: URL(string: "github.com")!,
+                id: 1,
+                login: "github",
+                siteAdmin: false,
+                type: .user
+            ),
+            isPrivate: false
+        )
+    }
+
+    func makeNotificaction(id: String, title: String) -> V3Notification {
+        return V3Notification(
+            id: id,
+            lastReadAt: nil,
+            reason: .assign,
+            repository: makeRepo(),
+            subject: V3NotificationSubject(title: title, type: .issue, url: nil),
+            unread: true,
+            updatedAt: Date()
+        )
+    }
+
+    func test_whenUpdating_thatReadFirstTime_thenSecondCallEmpty() {
+        clear(for: #function)
+
+        let cache = LocalNotificationsCache(path: path(for: #function))
+        let n1 = [
+            makeNotificaction(id: "123", title: "foo")
+        ]
+        var executions = 0
+
+        cache.update(notifications: n1) { results in
+            executions += 1
+            XCTAssertEqual(results.count, 1)
+        }
+
+        let n2 = [
+            makeNotificaction(id: "123", title: "foo"),
+            makeNotificaction(id: "456", title: "bar")
+        ]
+        cache.update(notifications: n2) { results in
+            executions += 1
+            XCTAssertEqual(results.count, 1)
+        }
+
+        let n3 = [
+            makeNotificaction(id: "123", title: "foo"),
+            makeNotificaction(id: "456", title: "bar")
+        ]
+
+        cache.update(notifications: n3) { results in
+            executions += 1
+            XCTAssertEqual(results.count, 0)
+        }
+
+        XCTAssertEqual(executions, 3)
+    }
+
+}