Synchronize creation of Events and Authors

CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed incorrect ellipsis applied to long notes.
 - Changed note rendering to retain more newlines. 
 - Show reposts in stories.
+- Fixed a bug where notes, reposts, and author profiles could fail to load.
 - Show truncated URLs in notes instead of hiding them completely.
 
 ## [0.1 (86)] - 2023-10-25Z

Nos/Models/Author+CoreDataClass.swift

@@ -11,6 +11,10 @@ import CoreData
 import Dependencies
 import Logger
 
+enum AuthorError: Error {
+    case coreData
+}
+
 @objc(Author)
 public class Author: NosManagedObject {
 
@@ -100,13 +104,29 @@ public class Author: NosManagedObject {
 
     @discardableResult
     class func findOrCreate(by pubKey: HexadecimalString, context: NSManagedObjectContext) throws -> Author {
+        @Dependency(\.persistenceController) var persistenceController
+        @Dependency(\.crashReporting) var crashReporting
+
         if let author = try? Author.find(by: pubKey, context: context) {
             return author
         } else {
-            let author = Author(context: context)
-            author.hexadecimalPublicKey = pubKey
-            author.muted = false
-            return author
+            /// Always create authors in the creationContext first to make sure we never end up with two identical
+            /// Authors in different contexts with the same objectID, because this messes up SwiftUI's observation
+            /// of changes.
+            let creationContext = persistenceController.creationContext
+            let objectID = try creationContext.performAndWait {
+                let author = Author(context: creationContext)
+                author.hexadecimalPublicKey = pubKey
+                author.muted = false
+                try creationContext.save()
+                return author.objectID
+            }
+            guard let fetchedAuthor = context.object(with: objectID) as? Author else {
+                let error = AuthorError.coreData
+                crashReporting.report(error)
+                throw error
+            }
+            return fetchedAuthor
         }
     }
 
@@ -213,7 +233,7 @@ public class Author: NosManagedObject {
         let fetchRequest = NSFetchRequest<Author>(entityName: "Author")
         fetchRequest.sortDescriptors = [NSSortDescriptor(keyPath: \Author.lastUpdatedContactList, ascending: false)]
         fetchRequest.predicate = NSPredicate(
-            format: "hexadecimalPublicKey IN %@.follows.destination.hexadecimalPublicKey",
+            format: "ANY followers.source = %@",
             author
         )
         return fetchRequest

Nos/Models/Event+CoreDataClass.swift

@@ -23,6 +23,7 @@ enum EventError: Error {
     case invalidETag([String])
     case invalidSignature(Event)
     case expiredEvent
+    case coreData
 
     var description: String? {
         switch self {
@@ -37,7 +38,7 @@ enum EventError: Error {
         case .expiredEvent:
             return "This event has expired"
         default:
-            return ""
+            return "An unkown error occurred."
         }
 	}
 }
@@ -75,6 +76,7 @@ extension FetchedResults where Element == Event {
 public class Event: NosManagedObject {
 
     @Dependency(\.currentUser) private var currentUser
+    @Dependency(\.persistenceController) private var persistenceController
 
     static var replyNoteReferences = "kind = 1 AND ANY eventReferences.referencedEvent.identifier == %@ " +
         "AND author.muted = false"
@@ -466,7 +468,7 @@ public class Event: NosManagedObject {
 
     // MARK: - Creating
 
-    class func createIfNecessary(
+    func createIfNecessary(
         jsonEvent: JSONEvent, 
         relay: Relay?, 
         context: NSManagedObjectContext
@@ -481,28 +483,65 @@ public class Event: NosManagedObject {
                 try existingEvent.hydrate(from: jsonEvent, relay: relay, in: context)
             }
             return existingEvent
+        } else {
+            @Dependency(\.crashReporting) var crashReporting
+            @Dependency(\.persistenceController) var persistenceController
+
+            /// Always create events in the creationContext first to make sure we never end up with two identical
+            /// Events in different contexts with the same objectID, because this messes up SwiftUI's observation
+            /// of changes.
+            let creationContext = persistenceController.creationContext
+            let objectID = try creationContext.performAndWait {
+                let event = Event(context: creationContext)
+                event.identifier = jsonEvent.id
+                try creationContext.save()
+                return event.objectID
+            }
+            guard let fetchedEvent = context.object(with: objectID) as? Event else {
+                let error = EventError.coreData
+                crashReporting.report(error)
+                throw error
+            }
+
+            fetchedEvent.receivedAt = .now
+            try fetchedEvent.hydrate(from: jsonEvent, relay: relay, in: context)
+            return fetchedEvent
         }
-
-        return try Event(context: context, jsonEvent: jsonEvent, relay: relay)
     }
 
-    class func findOrCreateStubBy(id: String, context: NSManagedObjectContext) throws -> Event {
+    /// Fetches the event with the given ID out of the database, and otherwise creates a stubbed Event.
+    /// A stubbed event only has an `identifier` - we know an event with this identifier exists but we don't
+    /// have its content or tags yet.
+    ///  
+    /// - Parameters:
+    ///   - id: The hexadecimal Nostr ID of the event.
+    /// - Returns: The Event model with the given ID.
+    class func findOrCreateStubBy(id: HexadecimalString, context: NSManagedObjectContext) throws -> Event {
         if let existingEvent = try context.fetch(Event.event(by: id)).first {
             return existingEvent
         } else {
-            let event = Event(context: context)
-            event.identifier = id
-            return event
+            @Dependency(\.crashReporting) var crashReporting
+            @Dependency(\.persistenceController) var persistenceController
+
+            /// Always create events in the creationContext first to make sure we never end up with two identical
+            /// Events in different contexts with the same objectID, because this messes up SwiftUI's observation
+            /// of changes.
+            let creationContext = persistenceController.creationContext
+            let objectID = try creationContext.performAndWait {
+                let event = Event(context: creationContext)
+                event.identifier = id
+                try creationContext.save()
+                return event.objectID
+            }
+            guard let fetchedEvent = context.object(with: objectID) as? Event else {
+                let error = EventError.coreData
+                crashReporting.report(error)
+                throw error
+            }
+            return fetchedEvent
         }
     }
 
-    convenience init(context: NSManagedObjectContext, jsonEvent: JSONEvent, relay: Relay?) throws {
-        self.init(context: context)
-        identifier = jsonEvent.id
-        receivedAt = .now
-        try hydrate(from: jsonEvent, relay: relay, in: context)
-    }
-
     func deleteEvents(identifiers: [String], context: NSManagedObjectContext) async {
         print("Deleting: \(identifiers)")
         let deleteRequest = Event.deletePostsRequest(for: identifiers)

Nos/Models/Persistence.swift

@@ -33,18 +33,34 @@ class PersistenceController {
         container.viewContext
     }
 
-    lazy var backgroundViewContext = {
+    /// A context to synchronize creation of Events and Authors so we don't end up with duplicates.
+    lazy var creationContext = {
         newBackgroundContext()
     }()
 
-    var container: NSPersistentContainer
+    /// A context for parsing Nostr events from relays.
+    lazy var parseContext = {
+        self.newBackgroundContext()
+    }()
+
+    /// A context for Views to do expensive queries that we want to keep off the viewContext.
+    lazy var backgroundViewContext = {
+        self.newBackgroundContext()
+    }()
+
+    private(set) var container: NSPersistentContainer
+    private var model: NSManagedObjectModel
+    private var inMemory: Bool
 
-    init(inMemory: Bool = false) {
+    init(containerName: String = "Nos", inMemory: Bool = false) {
+        self.inMemory = inMemory
         let modelURL = Bundle.current.url(forResource: "Nos", withExtension: "momd")!
-        container = NSPersistentContainer(
-            name: "Nos",
-            managedObjectModel: NSManagedObjectModel(contentsOf: modelURL)!
-        )
+        model = NSManagedObjectModel(contentsOf: modelURL)!
+        container = NSPersistentContainer(name: containerName, managedObjectModel: model)
+        setUp()
+    }
+
+    func setUp() {
         if inMemory {
             container.persistentStoreDescriptions.first!.url = URL(fileURLWithPath: "/dev/null")
         }
@@ -56,6 +72,26 @@ class PersistenceController {
         container.viewContext.mergePolicy = NSMergePolicy(merge: mergeType)
     }
 
+    #if DEBUG
+    func resetForTesting() {
+        container = NSPersistentContainer(name: "Nos", managedObjectModel: model)
+        if !inMemory {
+            container.loadPersistentStores(completionHandler: { (storeDescription, _) in
+                guard let storeURL = storeDescription.url else {
+                    Log.error("Could not get store URL")
+                    return
+                }
+                Self.clearCoreData(store: storeURL, in: self.container)
+            })
+        }
+        setUp()
+        viewContext.reset()
+        creationContext = newBackgroundContext()
+        backgroundViewContext = newBackgroundContext()
+        parseContext = newBackgroundContext()
+    }
+    #endif
+
     private func loadPersistentStores(from container: NSPersistentContainer) {
         container.loadPersistentStores(completionHandler: { (storeDescription, error) in
 
@@ -168,7 +204,7 @@ class PersistenceController {
 
         cleanupTask = Task {
             defer { self.cleanupTask = nil }
-            let context = backgroundViewContext
+            let context = parseContext
             let startTime = Date.now
             Log.info("Starting Core Data cleanup...")
 

Nos/Router.swift

@@ -9,6 +9,7 @@ import SwiftUI
 import Combine
 import CoreData
 import Logger
+import Dependencies
 
 // Manages the app's navigation state.
 @MainActor class Router: ObservableObject {
@@ -19,6 +20,7 @@ import Logger
     @Published var profilePath = NavigationPath()
     @Published var sideMenuPath = NavigationPath()
     @Published var selectedTab = AppDestination.home
+    @Dependency(\.persistenceController) private var persistenceController
 
     var currentPath: Binding<NavigationPath> {
         if sideMenuOpened {
@@ -101,9 +103,9 @@ extension Router {
         // the hex format pubkey of the mentioned author
         do {
             if link.hasPrefix("@") {
-                push(try Author.findOrCreate(by: identifier, context: context))
+                push(try Author.findOrCreate(by: identifier, context: persistenceController.parseContext))
             } else if link.hasPrefix("%") {
-                push(try Event.findOrCreateStubBy(id: identifier, context: context))
+                push(try Event.findOrCreateStubBy(id: identifier, context: persistenceController.parseContext))
             } else if url.scheme == "http" || url.scheme == "https" {
                 push(url)
             } else {

Nos/Service/DependencyInjection.swift

@@ -92,7 +92,7 @@ fileprivate enum PushNotificationServiceKey: DependencyKey {
 
 fileprivate enum PersistenceControllerKey: DependencyKey {
     static let liveValue = PersistenceController()
-    static let testValue = PersistenceController(inMemory: true)
+    static var testValue = PersistenceController(inMemory: true)
     static let previewValue = PersistenceController(inMemory: true)
 }
 

Nos/Service/EventProcessor.swift

@@ -27,7 +27,7 @@ enum EventProcessor {
         in parseContext: NSManagedObjectContext,
         skipVerification: Bool = false
     ) throws -> Event? {
-        if let event = try Event.createIfNecessary(jsonEvent: jsonEvent, relay: relay, context: parseContext) {
+        if let event = try Event().createIfNecessary(jsonEvent: jsonEvent, relay: relay, context: parseContext) {
             relay.unwrap {
                 do {
                     try event.trackDelete(on: $0, context: parseContext)

Nos/Service/PushNotificationService.swift

@@ -217,7 +217,7 @@ import Combine
         if let viewModel {
             // Leave an hour of margin on the notificationcutoff to allow for events arriving slightly out of order.
             notificationCutoff = viewModel.date.addingTimeInterval(-60 * 60)
-            await viewModel.loadContent(in: self.modelContext)
+            await viewModel.loadContent(in: self.persistenceController.parseContext)
 
             do {
                 try await UNUserNotificationCenter.current().add(viewModel.notificationCenterRequest)

Nos/Service/RelayService.swift

@@ -34,7 +34,7 @@ final class RelayService: ObservableObject {
         self.subscriptions = RelaySubscriptionManager()
         @Dependency(\.persistenceController) var persistenceController
         self.backgroundContext = persistenceController.newBackgroundContext()
-        self.parseContext = persistenceController.newBackgroundContext()
+        self.parseContext = persistenceController.parseContext
         parseContext.mergePolicy = NSMergePolicy.mergeByPropertyObjectTrump
 
         self.eventProcessingLoop = Task(priority: .userInitiated) { [weak self] in

Nos/Service/SocialGraphCache.swift

@@ -41,7 +41,6 @@ import Logger
 
     private var twoHopReferences: [HexadecimalString: Int]
 
-    // swiftlint:disable function_body_length
     init(userKey: HexadecimalString?, context: NSManagedObjectContext) {
         self.userKey = userKey
         self.context = context
@@ -53,42 +52,36 @@ import Logger
             super.init()
             return
         }
+
+        super.init()
 
-        let user: Author
         do {
-            user = try context.performAndWait {
-                let author = try Author.findOrCreate(by: userKey, context: context)
-                try? context.saveIfNeeded()
-                return author
+            try context.performAndWait {
+                let user = try Author.findOrCreate(by: userKey, context: context)
+                followedKeys.append(userKey)
+                userWatcher = NSFetchedResultsController(
+                    fetchRequest: Author.request(by: userKey),
+                    managedObjectContext: context,
+                    sectionNameKeyPath: nil,
+                    cacheName: "SocialGraphCache.userWatcher"
+                )
+                oneHopWatcher = NSFetchedResultsController(
+                    fetchRequest: Author.oneHopRequest(for: user),
+                    managedObjectContext: context,
+                    sectionNameKeyPath: nil,
+                    cacheName: "SocialGraphCache.oneHopWatcher"
+                )
             }
         } catch {
             Log.error(error.localizedDescription)
-            super.init()
-            return
         }
 
-        super.init()
-
-        followedKeys.append(userKey)
-        userWatcher = NSFetchedResultsController(
-            fetchRequest: Author.request(by: userKey),
-            managedObjectContext: context,
-            sectionNameKeyPath: nil,
-            cacheName: "SocialGraphCache.userWatcher"
-        )
-        oneHopWatcher = NSFetchedResultsController(
-            fetchRequest: Author.oneHopRequest(for: user),
-            managedObjectContext: context,
-            sectionNameKeyPath: nil,
-            cacheName: "SocialGraphCache.oneHopWatcher"
-        )
-
         userWatcher?.delegate = self
         oneHopWatcher?.delegate = self
         do {
-            try context.performAndWait {
-                try self.userWatcher?.performFetch()
-                try self.oneHopWatcher?.performFetch()
+            try self.userWatcher?.performFetch()
+            try self.oneHopWatcher?.performFetch()
+            context.performAndWait {
                 self.oneHopWatcher?.fetchedObjects?.forEach { author in
                     guard let followedKey = author.hexadecimalPublicKey else {
                         return
@@ -102,7 +95,6 @@ import Logger
             return
         }
     }
-    // swiftlint:enable function_body_length
 
     // MARK: - Processing Changes