Notifications: Adds updateReadStatus to NotificationStore

Storage/Storage/Tools/StorageType+Extensions.swift

@@ -92,7 +92,14 @@ public extension StorageType {
 
     /// Retrieves the Notification.
     ///
-    public func loadNotification(noteID: Int, noteHash: Int) -> Note? {
+    public func loadNotification(noteID: Int64) -> Note? {
+        let predicate = NSPredicate(format: "noteID = %ld", noteID)
+        return firstObject(ofType: Note.self, matching: predicate)
+    }
+
+    /// Retrieves the Notification.
+    ///
+    public func loadNotification(noteID: Int64, noteHash: Int) -> Note? {
         let predicate = NSPredicate(format: "noteID = %ld AND noteHash = %ld", noteID, noteHash)
         return firstObject(ofType: Note.self, matching: predicate)
     }

Yosemite/Yosemite/Actions/NotificationAction.swift

@@ -8,4 +8,5 @@ import Networking
 public enum NotificationAction: Action {
     case synchronizeNotifications(onCompletion: (Error?) -> Void)
     case updateLastSeen(timestamp: String, onCompletion: (Error?) -> Void)
+    case updateReadStatus(noteID: Int64, read: Bool, onCompletion: (Error?) -> Void)
 }

Yosemite/Yosemite/Stores/NotificationStore.swift

@@ -30,6 +30,8 @@ public class NotificationStore: Store {
             synchronizeNotifications(onCompletion: onCompletion)
         case .updateLastSeen(let timestamp, let onCompletion):
             updateLastSeen(timestamp: timestamp, onCompletion: onCompletion)
+        case .updateReadStatus(let noteID, let read, let onCompletion):
+            updateReadStatus(noteID: noteID, read: read, onCompletion: onCompletion)
         }
     }
 }
@@ -56,6 +58,7 @@ private extension NotificationStore {
 
                 // Step 2: Update the storage notes from the fetched notes
                 self.updateLocalNotes(with: remoteNotes) {
+                    type(of: self).resetSharedDerivedStorage()
                     onCompletion(nil)
                 }
             }
@@ -70,12 +73,28 @@ private extension NotificationStore {
             onCompletion(error)
         }
     }
+
+    /// Updates the read status for the given notification ID
+    ///
+    func updateReadStatus(noteID: Int64, read: Bool, onCompletion: @escaping (Error?) -> Void) {
+        let remote = NotificationsRemote(network: network)
+        remote.updateReadStatus(String(noteID), read: read) { [weak self] (error) in
+            guard let `self` = self, error == nil else {
+                onCompletion(error)
+                return
+            }
+
+            self.updateLocalNoteReadStatus(for: noteID, read: read) {
+                onCompletion(nil)
+            }
+        }
+    }
 }
 
 
 // MARK: - Persistence
 //
-private extension NotificationStore {
+extension NotificationStore {
 
     /// Deletes the collection of local notifications that cannot be found in a given collection of
     /// remote hashes.
@@ -111,9 +130,7 @@ private extension NotificationStore {
 
         derivedStorage.perform {
             for remoteNote in remoteNotes {
-                let predicate = NSPredicate(format: "(noteID == %ld)", remoteNote.noteId)
-                let localNote = derivedStorage.firstObject(ofType: Storage.Note.self, matching: predicate) ?? derivedStorage.insertNewObject(ofType: Storage.Note.self)
-
+                let localNote = derivedStorage.loadNotification(noteID: remoteNote.noteId) ?? derivedStorage.insertNewObject(ofType: Storage.Note.self)
                 localNote.update(with: remoteNote)
             }
         }
@@ -124,6 +141,22 @@ private extension NotificationStore {
             }
         }
     }
+
+    /// Updates the read status for the given noteID in the Storage layer. If the local note to update cannot be found,
+    /// nothing is updated/created in storage.
+    ///
+    func updateLocalNoteReadStatus(for noteID: Int64, read: Bool, completion: (() -> Void)? = nil) {
+        assert(Thread.isMainThread)
+        let storage = storageManager.viewStorage
+        guard let storageNote = storage.loadNotification(noteID: noteID) else {
+            completion?()
+            return
+        }
+
+        storageNote.read = read
+        storage.saveIfNeeded()
+        completion?()
+    }
 }
 
 
@@ -139,6 +172,12 @@ extension NotificationStore {
         }
         return privateStorage
     }
+
+    /// Nukes the private Shared Derived Storage instance.
+    ///
+    static func resetSharedDerivedStorage() {
+        privateStorage = nil
+    }
 }
 
 

Yosemite/YosemiteTests/Stores/NotificationStoreTests.swift

@@ -30,7 +30,10 @@ class NotificationStoreTests: XCTestCase {
         super.setUp()
         dispatcher = Dispatcher()
         storageManager = MockupStorageManager()
-        network = MockupNetwork(useResponseQueue: true)
+        network = MockupNetwork()
+
+        // Need to nuke this in-between tests otherwise some will randomly fail
+        NotificationStore.resetSharedDerivedStorage()
     }
 
 
@@ -135,4 +138,142 @@ class NotificationStoreTests: XCTestCase {
         noteStore.onAction(action)
         wait(for: [expectation], timeout: Constants.expectationTimeout)
     }
+
+
+    // MARK: - NotificationAction.updateReadStatus
+
+    /// Verifies that NotificationAction.updateReadStatus handles a success response from the backend properly
+    ///
+    func testUpdateNotificationReadStatusReturnsSuccess() {
+        let expectation = self.expectation(description: "Update read status success response")
+        let noteStore = NotificationStore(dispatcher: dispatcher, storageManager: storageManager, network: network)
+        let originalNote = sampleNotification()
+        let expectedNote = sampleNotificationMutated()
+
+        network.simulateResponse(requestUrlSuffix: "notifications/read", filename: "generic_success")
+        let action = NotificationAction.updateReadStatus(noteID: originalNote.noteId, read: true) { [weak self] (error) in
+            XCTAssertNil(error)
+            let storageNote = self?.viewStorage.loadNotification(noteID: originalNote.noteId)
+            XCTAssertEqual(storageNote?.toReadOnly().read, expectedNote.read)
+            expectation.fulfill()
+        }
+
+        XCTAssertEqual(viewStorage.countObjects(ofType: Storage.Note.self), 0)
+        noteStore.updateLocalNotes(with: [originalNote]) { [weak self] in
+            XCTAssertEqual(self?.viewStorage.countObjects(ofType: Storage.Note.self), 1)
+            noteStore.onAction(action)
+        }
+
+        wait(for: [expectation], timeout: Constants.expectationTimeout)
+    }
+
+    /// Verifies that NotificationAction.updateReadStatus returns an error whenever there is an error response from the backend.
+    ///
+    func testUpdateNotificationReadStatusReturnsErrorUponReponseError() {
+        let expectation = self.expectation(description: "Update notification read status error response")
+        let noteStore = NotificationStore(dispatcher: dispatcher, storageManager: storageManager, network: network)
+
+        network.simulateResponse(requestUrlSuffix: "notifications/read", filename: "generic_error")
+        let action = NotificationAction.updateReadStatus(noteID: 9999, read: true) { (error) in
+            XCTAssertNotNil(error)
+            expectation.fulfill()
+        }
+
+        noteStore.onAction(action)
+        wait(for: [expectation], timeout: Constants.expectationTimeout)
+    }
+
+    /// Verifies that NotificationAction.updateReadStatus returns an error whenever there is no backend response.
+    ///
+    func testUpdateNotificationReadStatusReturnsErrorUponEmptyResponse() {
+        let expectation = self.expectation(description: "Update notification read status empty response")
+        let noteStore = NotificationStore(dispatcher: dispatcher, storageManager: storageManager, network: network)
+
+        let action = NotificationAction.updateReadStatus(noteID: 9999, read: true) { (error) in
+            XCTAssertNotNil(error)
+            expectation.fulfill()
+        }
+
+        noteStore.onAction(action)
+        wait(for: [expectation], timeout: Constants.expectationTimeout)
+    }
+
+    /// Verifies that `updateLocalNoteReadStatus` does not produce duplicate entries.
+    ///
+    func testUpdateStoredNotificationEffectivelyUpdatesPreexistantNotification() {
+        let expectation = self.expectation(description: "Update read status on existing note")
+        let noteStore = NotificationStore(dispatcher: dispatcher, storageManager: storageManager, network: network)
+        let originalNote = sampleNotification()
+        let expectedNote = sampleNotificationMutated()
+
+        XCTAssertEqual(viewStorage.countObjects(ofType: Storage.Note.self), 0)
+        noteStore.updateLocalNotes(with: [originalNote]) { [weak self] in
+            XCTAssertEqual(self?.viewStorage.countObjects(ofType: Storage.Note.self), 1)
+            noteStore.updateLocalNoteReadStatus(for: originalNote.noteId, read: true) {
+                XCTAssertEqual(self?.viewStorage.countObjects(ofType: Storage.Note.self), 1)
+                let storageNote = self?.viewStorage.loadNotification(noteID: originalNote.noteId)
+                XCTAssertEqual(storageNote?.toReadOnly().read, expectedNote.read)
+                expectation.fulfill()
+            }
+        }
+
+        wait(for: [expectation], timeout: Constants.expectationTimeout)
+    }
+
+    /// Verifies that `updateLocalNoteReadStatus` does not produce duplicate entries with an invalid notification ID.
+    ///
+    func testUpdateStoredNotificationDoesntUpdateInvalidNote() {
+        let expectation = self.expectation(description: "Update read status on invalid note")
+        let noteStore = NotificationStore(dispatcher: dispatcher, storageManager: storageManager, network: network)
+
+        XCTAssertEqual(viewStorage.countObjects(ofType: Storage.Note.self), 0)
+        noteStore.updateLocalNoteReadStatus(for: 9999, read: true) { [weak self] in
+            XCTAssertEqual(self?.viewStorage.countObjects(ofType: Storage.Note.self), 0)
+            let storageNote = self?.viewStorage.loadNotification(noteID: 9999)
+            XCTAssertNil(storageNote)
+            expectation.fulfill()
+        }
+
+        wait(for: [expectation], timeout: Constants.expectationTimeout)
+    }
+}
+
+
+// MARK: - Private Methods
+//
+private extension NotificationStoreTests {
+
+    //  MARK: - Notification Sample
+
+    func sampleNotification() -> Networking.Note {
+        return Note(noteId: 123456,
+                    hash: 11223344,
+                    read: false,
+                    icon: "https://gravatar.tld/some-hash",
+                    noticon: "\u{f408}",
+                    timestamp: "2018-10-22T18:51:33+00:00",
+                    type: "comment_like",
+                    url: "https:\\someurl.sometld",
+                    title: "3 Likes",
+                    subject: Data(),
+                    header: Data(),
+                    body: Data(),
+                    meta: Data())
+    }
+
+    func sampleNotificationMutated() -> Networking.Note {
+        return Note(noteId: 123456,
+                    hash: 11223344,
+                    read: true,
+                    icon: "https://gravatar.tld/some-hash",
+                    noticon: "\u{f408}",
+                    timestamp: "2018-10-22T18:51:33+00:00",
+                    type: "comment_like",
+                    url: "https:\\someurl.sometld",
+                    title: "3 Likes",
+                    subject: Data(),
+                    header: Data(),
+                    body: Data(),
+                    meta: Data())
+    }
 }