fixes and improvements related to Core Data usage #1443

CHANGELOG.md

@@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed a bug where nostr entities in URLs were treated like quoted note links.
 - Added in-app profile photo editing.
 - Changed "Name" to "Display Name" on the Edit Profile View.
+- Fixes and improvements related to Core Data usage.
 
 ### Internal Changes
 - Included the npub in the properties list sent to analytics.

Nos/Controller/PersistenceController.swift

@@ -2,24 +2,24 @@ import CoreData
 import Logger
 import Dependencies
 
-class PersistenceController {
+final class PersistenceController {
 
     @Dependency(\.currentUser) var currentUser
     @Dependency(\.crashReporting) var crashReporting
 
     /// Increment this to delete core data on update
-    static let version = 3
-    static let versionKey = "NosPersistenceControllerVersion"
+    private static let version = 3
+    private static let versionKey = "NosPersistenceControllerVersion"
 
     static var preview: PersistenceController = {
         let controller = PersistenceController(inMemory: true)
-        let viewContext = controller.container.viewContext
+        let viewContext = controller.viewContext
         return controller
     }()
 
     static var empty: PersistenceController = {
         let result = PersistenceController(inMemory: true)
-        let viewContext = result.container.viewContext
+        let viewContext = result.viewContext
         return result
     }()
 
@@ -28,22 +28,18 @@ class PersistenceController {
     }
 
     /// A context for parsing Nostr events from relays.
-    lazy var parseContext = {
-        newBackgroundContext()
-    }()
+    private(set) lazy var parseContext = newBackgroundContext()
 
     /// A context for Views to do expensive queries that we want to keep off the viewContext.
-    lazy var backgroundViewContext = {
-        self.newBackgroundContext()
-    }()
+    private(set) lazy var backgroundViewContext = newBackgroundContext()
 
     var sqliteURL: URL? {
         container.persistentStoreDescriptions.first?.url
     }
 
     private(set) var container: NSPersistentContainer
-    private var model: NSManagedObjectModel
-    private var inMemory: Bool
+    private let model: NSManagedObjectModel
+    private let inMemory: Bool
 
     init(containerName: String = "Nos", inMemory: Bool = false, erase: Bool = false) {
         self.inMemory = inMemory
@@ -53,57 +49,19 @@ class PersistenceController {
         setUp(erasingPrevious: erase)
     }
 
-    func tearDown() throws {
-        for store in container.persistentStoreCoordinator.persistentStores {
-            try container.persistentStoreCoordinator.remove(store)
-        }
-
-        try container.persistentStoreDescriptions.forEach { storeDescription in
-            try container.persistentStoreCoordinator.destroyPersistentStore(
-                at: storeDescription.url!, 
-                ofType: NSSQLiteStoreType, 
-                options: nil
-            )
-        }
-
-        viewContext.reset()
-        backgroundViewContext.reset()
-        parseContext.reset() 
-    }
-
-    func setUp(erasingPrevious: Bool) {
+    private func setUp(erasingPrevious: Bool) {
         if inMemory {
             container.persistentStoreDescriptions.first!.url = URL(fileURLWithPath: "/dev/null")
         } 
 
         loadPersistentStores(from: container, erasingPrevious: erasingPrevious)
 
-        container.viewContext.automaticallyMergesChangesFromParent = true
-        let mergeType = NSMergePolicyType.mergeByPropertyStoreTrumpMergePolicyType
-        container.viewContext.mergePolicy = NSMergePolicy(merge: mergeType)
+        viewContext.automaticallyMergesChangesFromParent = true
+        viewContext.mergePolicy = NSMergePolicy.mergeByPropertyStoreTrump
     }
 
-    #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(erasingPrevious: true)
-        viewContext.reset()
-        backgroundViewContext.reset()
-        parseContext.reset() 
-    }
-    #endif
-
     private func loadPersistentStores(from container: NSPersistentContainer, erasingPrevious: Bool) {
-        container.loadPersistentStores(completionHandler: { (storeDescription, error) in
+        container.loadPersistentStores { storeDescription, error in
 
             // Drop database if necessary
             if Self.loadVersionFromDisk() < Self.version || erasingPrevious {
@@ -124,7 +82,7 @@ class PersistenceController {
                 }
                 fatalError("Could not initialize database \(error), \(error.userInfo)")
             }
-        })
+        }
     }
 
     @MainActor
@@ -135,7 +93,7 @@ class PersistenceController {
         }
     }
 
-    static func clearCoreData(store storeURL: URL, in container: NSPersistentContainer) {
+    private static func clearCoreData(store storeURL: URL, in container: NSPersistentContainer) {
         Log.info("Dropping Core Data...")
         do {
             try container.persistentStoreCoordinator.destroyPersistentStore(at: storeURL, type: .sqlite)
@@ -144,6 +102,7 @@ class PersistenceController {
         }
     }
 
+#if DEBUG
     func loadSampleData(context: NSManagedObjectContext) async throws {
         guard let sampleFile = Bundle.current.url(forResource: "sample_data", withExtension: "json") else {
             Log.error("Error: bad sample file location")
@@ -178,22 +137,40 @@ class PersistenceController {
         }
     }
 
+    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(erasingPrevious: true)
+        viewContext.reset()
+        backgroundViewContext.reset()
+        parseContext.reset()
+    }
+#endif
+
     func newBackgroundContext() -> NSManagedObjectContext {
         let context = container.newBackgroundContext()
         context.automaticallyMergesChangesFromParent = true
         context.mergePolicy = NSMergePolicy.mergeByPropertyStoreTrump
         return context
     }
 
-    static func loadVersionFromDisk() -> Int {
+    private static func loadVersionFromDisk() -> Int {
         UserDefaults.standard.integer(forKey: Self.versionKey)
     }
 
-    static func saveVersionToDisk(_ newVersion: Int) {
+    private static func saveVersionToDisk(_ newVersion: Int) {
         UserDefaults.standard.set(newVersion, forKey: Self.versionKey)
     }
 
-    /// Cleans up uneeded entities from the database. Our local database is really just a cache, and we need to 
+    /// Cleans up unneeded entities from the database. Our local database is really just a cache, and we need to
     /// invalidate old items to keep it from growing indefinitely.
     /// 
     /// This should only be called once right at app launch.

Nos/Controller/SearchController.swift

@@ -49,9 +49,7 @@ class SearchController: ObservableObject {
     /// The timer for showing the "not finding results" view. Resets any time the query is changed.
     private var timer: Timer?
 
-    private lazy var context: NSManagedObjectContext = {
-        persistenceController.viewContext
-    }()
+    private lazy var context = persistenceController.viewContext
 
     /// The amount of time, in seconds, to remain in the `.loading` state until switching to `.stillLoading`.
     private let stillLoadingTime: TimeInterval = 10
@@ -141,7 +139,7 @@ class SearchController: ObservableObject {
         authorResults = []
     }
 
-    func note(fromPublicKey publicKeyString: String) -> Event? {
+    private func note(fromPublicKey publicKeyString: String) -> Event? {
         let strippedString = publicKeyString.trimmingCharacters(
             in: NSCharacterSet.whitespacesAndNewlines
         )

Nos/Models/CoreData/Event+CoreDataClass.swift

@@ -10,9 +10,6 @@ public class Event: NosManagedObject, VerifiableEvent {
     @Dependency(\.currentUser) @ObservationIgnored private var currentUser
 
     var pubKey: String { author?.hexadecimalPublicKey ?? "" }
-
-    static var replyNoteReferences = "kind = 1 AND ANY eventReferences.referencedEvent.identifier == %@ " +
-        "AND author.muted = false"
 
     @nonobjc public class func allEventsRequest() -> NSFetchRequest<Event> {
         let fetchRequest = NSFetchRequest<Event>(entityName: "Event")
@@ -113,7 +110,6 @@ public class Event: NosManagedObject, VerifiableEvent {
 
     @nonobjc public class func lastReceived(for user: Author) -> NSFetchRequest<Event> {
         let fetchRequest = NSFetchRequest<Event>(entityName: "Event")
-        fetchRequest.sortDescriptors = [NSSortDescriptor(keyPath: \Event.createdAt, ascending: false)]
         fetchRequest.predicate = NSPredicate(format: "author != %@", user)
         fetchRequest.fetchLimit = 1
         fetchRequest.sortDescriptors = [NSSortDescriptor(keyPath: \Event.receivedAt, ascending: false)]
@@ -128,6 +124,11 @@ public class Event: NosManagedObject, VerifiableEvent {
         guard let noteID else {
             return emptyRequest()
         }
+
+        let replyNoteReferences = "kind = 1 " +
+            "AND ANY eventReferences.referencedEvent.identifier == %@ " +
+            "AND author.muted = false"
+
         let fetchRequest = NSFetchRequest<Event>(entityName: "Event")
         fetchRequest.sortDescriptors = [NSSortDescriptor(keyPath: \Event.receivedAt, ascending: false)]
         fetchRequest.predicate = NSPredicate(
@@ -480,8 +481,8 @@ public class Event: NosManagedObject, VerifiableEvent {
 
     // MARK: - Creating
 
-    func createIfNecessary(
-        jsonEvent: JSONEvent, 
+    static func createIfNecessary(
+        jsonEvent: JSONEvent,
         relay: Relay?, 
         context: NSManagedObjectContext
     ) throws -> Event? {
@@ -562,7 +563,7 @@ public class Event: NosManagedObject, VerifiableEvent {
     }
 
     /// Populates an event stub (with only its ID set) using the data in the given JSON.
-    func hydrate(from jsonEvent: JSONEvent, relay: Relay?, in context: NSManagedObjectContext) throws {
+    private func hydrate(from jsonEvent: JSONEvent, relay: Relay?, in context: NSManagedObjectContext) throws {
         assert(isStub, "Tried to hydrate an event that isn't a stub. This is a programming error")
 
         // if this stub was created with a replaceableIdentifier and author, it won't have an identifier yet
@@ -629,7 +630,11 @@ public class Event: NosManagedObject, VerifiableEvent {
         }
     }
 
-    func hydrateContactList(from jsonEvent: JSONEvent, author newAuthor: Author, context: NSManagedObjectContext) {
+    private func hydrateContactList(
+        from jsonEvent: JSONEvent,
+        author newAuthor: Author,
+        context: NSManagedObjectContext
+    ) {
         guard createdAt! > newAuthor.lastUpdatedContactList ?? Date.distantPast else {
             return
         }
@@ -682,7 +687,7 @@ public class Event: NosManagedObject, VerifiableEvent {
         }
     }
 
-    func hydrateDefault(from jsonEvent: JSONEvent, context: NSManagedObjectContext) {
+    private func hydrateDefault(from jsonEvent: JSONEvent, context: NSManagedObjectContext) {
         let newEventReferences = NSMutableOrderedSet()
         let newAuthorReferences = NSMutableOrderedSet()
         for jsonTag in jsonEvent.tags {
@@ -706,7 +711,7 @@ public class Event: NosManagedObject, VerifiableEvent {
         authorReferences = newAuthorReferences
     }
 
-    func hydrateMetaData(from jsonEvent: JSONEvent, author newAuthor: Author, context: NSManagedObjectContext) {
+    private func hydrateMetaData(from jsonEvent: JSONEvent, author newAuthor: Author, context: NSManagedObjectContext) {
         guard createdAt! > newAuthor.lastUpdatedMetadata ?? Date.distantPast else {
             // This is old data
             return
@@ -738,7 +743,7 @@ public class Event: NosManagedObject, VerifiableEvent {
         seenOnRelays.insert(relay) 
     }
 
-    func hydrateMuteList(from jsonEvent: JSONEvent, context: NSManagedObjectContext) {
+    private func hydrateMuteList(from jsonEvent: JSONEvent, context: NSManagedObjectContext) {
         let mutedKeys = jsonEvent.tags.map { $0[1] }
 
         let request = Author.allAuthorsRequest(muted: true)
@@ -766,19 +771,15 @@ public class Event: NosManagedObject, VerifiableEvent {
     }
 
     /// Tries to parse a new event out of the given jsonEvent's `content` field.
-    @discardableResult
-    func parseContent(from jsonEvent: JSONEvent, context: NSManagedObjectContext) -> Event? {
+    private func parseContent(from jsonEvent: JSONEvent, context: NSManagedObjectContext) {
         do {
             if let contentData = jsonEvent.content.data(using: .utf8) {
                 let jsonEvent = try JSONDecoder().decode(JSONEvent.self, from: contentData)
-                return try Event().createIfNecessary(jsonEvent: jsonEvent, relay: nil, context: context)
+                _ = try Event.createIfNecessary(jsonEvent: jsonEvent, relay: nil, context: context)
             }
         } catch {
             Log.error("Could not parse content for jsonEvent: \(jsonEvent)")
-            return nil
         }
-
-        return nil
     }
 
     // MARK: - Preloading and Caching
@@ -887,9 +888,8 @@ public class Event: NosManagedObject, VerifiableEvent {
         @Dependency(\.persistenceController) var persistenceController
         let context = persistenceController.backgroundViewContext
 
-        _ = try? Event.findOrCreateStubBy(id: quotedNoteID, context: context)
-
         await context.perform {
+            _ = try? Event.findOrCreateStubBy(id: quotedNoteID, context: context)
             try? context.save()
         }
 

Nos/NosApp.swift

@@ -26,7 +26,7 @@ struct NosApp: App {
     var body: some Scene {
         WindowGroup {
             AppView()
-                .environment(\.managedObjectContext, persistenceController.container.viewContext)
+                .environment(\.managedObjectContext, persistenceController.viewContext)
                 .environmentObject(relayService)
                 .environmentObject(router)
                 .environment(appController)

Nos/Service/EventProcessor.swift

@@ -11,7 +11,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)
@@ -79,7 +79,7 @@ enum EventProcessor {
         from relay: Relay?,
         in persistenceController: PersistenceController
     ) throws -> [Event] {
-        let parseContext = persistenceController.container.viewContext
+        let parseContext = persistenceController.viewContext
         return try parse(jsonData: jsonData, from: relay, in: parseContext)
     }
 }