From 8ee703f47940d23070c649dc0409a2a9ad357d65 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Wed, 5 Jul 2023 16:49:55 +1200 Subject: [PATCH 1/2] Add warnings about incorrect `performQuery` usages --- .../Classes/Utility/CoreDataHelper.swift | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/WordPress/Classes/Utility/CoreDataHelper.swift b/WordPress/Classes/Utility/CoreDataHelper.swift index afea8bc21751..c241568d2eec 100644 --- a/WordPress/Classes/Utility/CoreDataHelper.swift +++ b/WordPress/Classes/Utility/CoreDataHelper.swift @@ -196,6 +196,8 @@ extension ContextManager.ContextManagerError: LocalizedError, CustomDebugStringC extension CoreDataStack { /// Perform a query using the `mainContext` and return the result. + /// + /// - Warning: Do not return `NSManagedObject` instances from the closure. func performQuery(_ block: @escaping (NSManagedObjectContext) -> T) -> T { var value: T! = nil self.mainContext.performAndWait { @@ -360,3 +362,69 @@ extension CoreDataStack { try ContextManager.migrateDataModelsIfNecessary(storeURL: databaseLocation, objectModel: objectModel) } } + +/// This extension declares many `performQuery` usages that may introduce Core Data concurrency issues. +/// +/// The context object used by the `performQuery` function is opaque to the caller. The caller should not assume what +/// the context object is, nor the context queue type (the main queue or a background queue). That means the caller +/// does not have enough information to guarantee safe access to the returned `NSManagedObject` instances. +/// +/// The closure passed to the `performQuery` function should use the context to query objects and return none Core Data +/// types. Here is an example of how it should be used. +/// +/// ``` +/// // Wrong: +/// let account = coreDataStack.performQuery { context in +/// return Account.lookUp(in: context) +/// } +/// let name = account.username +/// +/// // Right: +/// let name = coreDataStack.performQuery { context in +/// let account = Account.lookUp(in: context) +/// return account.username +/// } +/// ``` +extension CoreDataStack { + @available(*, deprecated, message: "Returning `NSManagedObject` instances may introduce Core Data concurrency issues.") + func performQuery(_ block: @escaping (NSManagedObjectContext) -> T) -> T where T: NSManagedObject { + mainContext.performAndWait { [mainContext] in + block(mainContext) + } + } + + @available(*, deprecated, message: "Returning `NSManagedObject` instances may introduce Core Data concurrency issues.") + func performQuery(_ block: @escaping (NSManagedObjectContext) -> T?) -> T? where T: NSManagedObject { + mainContext.performAndWait { [mainContext] in + block(mainContext) + } + } + + @available(*, deprecated, message: "Returning `NSManagedObject` instances may introduce Core Data concurrency issues.") + func performQuery(_ block: @escaping (NSManagedObjectContext) -> T) -> T where T: Sequence, T.Element: NSManagedObject { + mainContext.performAndWait { [mainContext] in + block(mainContext) + } + } + + @available(*, deprecated, message: "Returning `NSManagedObject` instances may introduce Core Data concurrency issues.") + func performQuery(_ block: @escaping (NSManagedObjectContext) -> T?) -> T? where T: Sequence, T.Element: NSManagedObject { + mainContext.performAndWait { [mainContext] in + block(mainContext) + } + } + + @available(*, deprecated, message: "Returning `NSManagedObject` instances may introduce Core Data concurrency issues.") + func performQuery(_ block: @escaping (NSManagedObjectContext) -> Result) -> Result where T: NSManagedObject, E: Error { + mainContext.performAndWait { [mainContext] in + block(mainContext) + } + } + + @available(*, deprecated, message: "Returning `NSManagedObject` instances may introduce Core Data concurrency issues.") + func performQuery(_ block: @escaping (NSManagedObjectContext) -> Result?) -> Result? where T: NSManagedObject, E: Error { + mainContext.performAndWait { [mainContext] in + block(mainContext) + } + } +} From 12554eaf066b819e0180c144fb314e6187d1eb1d Mon Sep 17 00:00:00 2001 From: Tony Li Date: Thu, 6 Jul 2023 10:50:16 +1200 Subject: [PATCH 2/2] Update a code comment Co-authored-by: Paul Von Schrottky <1898325+guarani@users.noreply.github.com> --- WordPress/Classes/Utility/CoreDataHelper.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WordPress/Classes/Utility/CoreDataHelper.swift b/WordPress/Classes/Utility/CoreDataHelper.swift index c241568d2eec..095ebcb8aa94 100644 --- a/WordPress/Classes/Utility/CoreDataHelper.swift +++ b/WordPress/Classes/Utility/CoreDataHelper.swift @@ -369,7 +369,7 @@ extension CoreDataStack { /// the context object is, nor the context queue type (the main queue or a background queue). That means the caller /// does not have enough information to guarantee safe access to the returned `NSManagedObject` instances. /// -/// The closure passed to the `performQuery` function should use the context to query objects and return none Core Data +/// The closure passed to the `performQuery` function should use the context to query objects and return non- Core Data /// types. Here is an example of how it should be used. /// /// ```