Merge pull request #19395 from wordpress-mobile/reimplement-notification-cache-observer

Reimplement invaliding `Notification` attributes cache

Author: crazytonyli

WordPress/Classes/Models/Notifications/Notification.swift

@@ -81,10 +81,32 @@ class Notification: NSManagedObject {
     ///
     fileprivate var cachedHeaderAndBodyContentGroups: [FormattableContentGroup]?
 
+    private var cachedAttributesObserver: NotificationCachedAttributesObserver?
+
     /// Array that contains the Cached Property Names
     ///
     fileprivate static let cachedAttributes = Set(arrayLiteral: "body", "header", "subject", "timestamp")
 
+    override func awakeFromFetch() {
+        super.awakeFromFetch()
+
+        if cachedAttributesObserver == nil {
+            let observer = NotificationCachedAttributesObserver()
+            for attr in Notification.cachedAttributes {
+                addObserver(observer, forKeyPath: attr, options: [.prior], context: nil)
+            }
+            cachedAttributesObserver = observer
+        }
+    }
+
+    deinit {
+        if let observer = cachedAttributesObserver {
+            for attr in Notification.cachedAttributes {
+                removeObserver(observer, forKeyPath: attr)
+            }
+        }
+    }
+
     func renderSubject() -> NSAttributedString? {
         guard let subjectContent = subjectContentGroup?.blocks.first else {
             return nil
@@ -99,26 +121,6 @@ class Notification: NSManagedObject {
         return formatter.render(content: snippetContent, with: SnippetsContentStyles())
     }
 
-    /// When needed, nukes cached attributes
-    ///
-    override func willChangeValue(forKey key: String) {
-        super.willChangeValue(forKey: key)
-
-        // Note:
-        // Cached Attributes are only consumed on the main thread, when initializing UI elements.
-        // As an optimization, we'll only reset those attributes when we're running on the main thread.
-        //
-        guard managedObjectContext?.concurrencyType == .mainQueueConcurrencyType else {
-            return
-        }
-
-        guard Swift.type(of: self).cachedAttributes.contains(key) else {
-            return
-        }
-
-        resetCachedAttributes()
-    }
-
     /// Nukes any cached values.
     ///
     func resetCachedAttributes() {
@@ -412,3 +414,25 @@ extension Notification: Notifiable {
         return notificationId
     }
 }
+
+private class NotificationCachedAttributesObserver: NSObject {
+    override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey: Any]?, context: UnsafeMutableRawPointer?) {
+        guard let keyPath, let notification = object as? Notification, Notification.cachedAttributes.contains(keyPath) else {
+            return
+        }
+
+        guard (change?[.notificationIsPriorKey] as? NSNumber)?.boolValue == true else {
+            return
+        }
+
+        // Note:
+        // Cached Attributes are only consumed on the main thread, when initializing UI elements.
+        // As an optimization, we'll only reset those attributes when we're running on the main thread.
+        //
+        guard notification.managedObjectContext?.concurrencyType == .mainQueueConcurrencyType else {
+            return
+        }
+
+        notification.resetCachedAttributes()
+    }
+}

WordPress/WordPressTest/NotificationTests.swift

@@ -229,7 +229,37 @@ class NotificationTests: CoreDataTestCase {
         XCTAssertEqual(note.headerAndBodyContentGroups.count, totalGroupsCount)
     }
 
-
+    func testNotificationCacheIsInvalidated() throws {
+        let commentNotificationId = "44444"
+        let _ = try utility.loadCommentNotification()
+        contextManager.saveContextAndWait(mainContext)
+
+        let fetchRequest = NSFetchRequest<WordPress.Notification>(entityName: WordPress.Notification.entityName())
+        fetchRequest.predicate = NSPredicate(format: "notificationId == %@", commentNotificationId)
+
+        let note = try XCTUnwrap(mainContext.fetch(fetchRequest).first)
+        XCTAssertEqual(note.timestamp, "2015-03-10T18:57:37+00:00")
+        XCTAssertEqual(note.timestampAsDate.timeIntervalSince1970, 1426013857)
+
+        note.timestamp = "2015-03-10T18:57:38+00:00"
+        XCTAssertEqual(note.timestampAsDate.timeIntervalSince1970, 1426013858)
+
+        mainContext.reset()
+
+        contextManager.performAndSave { context in
+            let notification = (try? context.fetch(fetchRequest))?.first
+            XCTAssertNotNil(notification)
+            XCTAssertEqual(notification?.timestampAsDate.timeIntervalSince1970, 1426013857)
+
+            XCTExpectFailure("""
+                This assertion failure is problematic.
+                When timestamp (or any other cached attribute) is changed, the cache still should be invalidated
+                even though the notification is associated with a non-main context.
+                """)
+            note.timestamp = "2015-03-10T18:57:38+00:00"
+            XCTAssertEqual(notification?.timestampAsDate.timeIntervalSince1970, 1426013858)
+        }
+    }
 
     // MARK: - Helpers