diff --git a/Classes/Notifications/NotificationModelController.swift b/Classes/Notifications/NotificationModelController.swift index d21143525..b581b951e 100644 --- a/Classes/Notifications/NotificationModelController.swift +++ b/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? diff --git a/Classes/Systems/AppDelegate.swift b/Classes/Systems/AppDelegate.swift index 9ef8b65c9..e5166e4e6 100644 --- a/Classes/Systems/AppDelegate.swift +++ b/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) } } diff --git a/Classes/Systems/BadgeNotifications.swift b/Classes/Systems/BadgeNotifications.swift index b221a7747..b6cb00e74 100644 --- a/Classes/Systems/BadgeNotifications.swift +++ b/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 diff --git a/Classes/Systems/LocalNotificationsCache.swift b/Classes/Systems/LocalNotificationsCache.swift index 1aafed927..83124ed31 100644 --- a/Classes/Systems/LocalNotificationsCache.swift +++ b/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)") } diff --git a/Classes/Systems/RootNavigationManager.swift b/Classes/Systems/RootNavigationManager.swift index c668afa21..4f761d3ef 100644 --- a/Classes/Systems/RootNavigationManager.swift +++ b/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) diff --git a/Freetime.xcodeproj/project.pbxproj b/Freetime.xcodeproj/project.pbxproj index 71b636583..888c83a14 100644 --- a/Freetime.xcodeproj/project.pbxproj +++ b/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 = ""; }; 2977788720B0DAD500F2AFC2 /* ViewMarkdownViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ViewMarkdownViewController.swift; sourceTree = ""; }; 2977788920B306F200F2AFC2 /* LabelLayoutManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LabelLayoutManager.swift; sourceTree = ""; }; + 2977D8BE215AE12D0073F737 /* LocalNotificationCacheTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocalNotificationCacheTests.swift; sourceTree = ""; }; 29792B181FFB10A3007A0C57 /* UIViewController+MessageAutocompleteControllerLayoutDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIViewController+MessageAutocompleteControllerLayoutDelegate.swift"; sourceTree = ""; }; 29792B1A1FFB21AD007A0C57 /* AutocompleteController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AutocompleteController.swift; sourceTree = ""; }; 29792B1C1FFB2FC6007A0C57 /* IssueManagingNavSectionController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IssueManagingNavSectionController.swift; sourceTree = ""; }; @@ -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 */, diff --git a/FreetimeTests/LocalNotificationCacheTests.swift b/FreetimeTests/LocalNotificationCacheTests.swift new file mode 100644 index 000000000..a20e51d84 --- /dev/null +++ b/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) + } + +}