Order Details: Load simplified objects for products (#15959)

Author: itsmeichigo

Modules/Sources/Yosemite/Internal/NSOrderedSet+Array.swift

@@ -1,7 +1,7 @@
 import Foundation
 
 extension NSOrderedSet {
-    func toArray<T>() -> [T] {
+    public func toArray<T>() -> [T] {
         guard let array = array as? [T] else {
             return []
         }

Modules/Sources/Yosemite/Model/Storage/Product+ReadOnlyConvertible.swift

@@ -2,7 +2,7 @@ import Foundation
 import Storage
 
 extension Storage.Product {
-    var imagesArray: [Storage.ProductImage] {
+    public var imagesArray: [Storage.ProductImage] {
         return images?.toArray() ?? []
     }
     var tagsArray: [Storage.ProductTag] {

Modules/Sources/Yosemite/Tools/ResultsController.swift

@@ -9,9 +9,9 @@ import CoreData
 public typealias ResultsControllerMutableType = NSManagedObject & ReadOnlyConvertible
 
 
-// MARK: - ResultsController
+// MARK: - GenericResultsController (Core Implementation)
 //
-public class ResultsController<T: ResultsControllerMutableType> {
+public class GenericResultsController<T: ResultsControllerMutableType, Output> {
 
     /// The `StorageType` used to fetch objects.
     ///
@@ -79,7 +79,7 @@ public class ResultsController<T: ResultsControllerMutableType> {
 
     /// Closure to be executed whenever an Object is updated.
     ///
-    public var onDidChangeObject: ((_ object: T.ReadOnlyType, _ indexPath: IndexPath?, _ type: ChangeType, _ newIndexPath: IndexPath?) -> Void)?
+    public var onDidChangeObject: ((_ object: Output, _ indexPath: IndexPath?, _ type: ChangeType, _ newIndexPath: IndexPath?) -> Void)?
 
     /// Closure to be executed whenever an entire Section is updated.
     ///
@@ -94,19 +94,25 @@ public class ResultsController<T: ResultsControllerMutableType> {
     ///
     private let fetchLimit: Int?
 
+    /// Transformer closure to convert T to Output type.
+    ///
+    private let transformer: (T) -> Output
+
     /// Designated Initializer.
     ///
     public init(viewStorage: StorageType,
                 sectionNameKeyPath: String? = nil,
                 matching predicate: NSPredicate? = nil,
                 fetchLimit: Int? = nil,
-                sortedBy descriptors: [NSSortDescriptor]) {
+                sortedBy descriptors: [NSSortDescriptor],
+                transformer: @escaping (T) -> Output) {
 
         self.viewStorage = viewStorage
         self.sectionNameKeyPath = sectionNameKeyPath
         self.predicate = predicate
         self.fetchLimit = fetchLimit
         self.sortDescriptors = descriptors
+        self.transformer = transformer
 
         setupResultsController()
         setupEventsForwarding()
@@ -119,13 +125,15 @@ public class ResultsController<T: ResultsControllerMutableType> {
                             sectionNameKeyPath: String? = nil,
                             matching predicate: NSPredicate? = nil,
                             fetchLimit: Int? = nil,
-                            sortedBy descriptors: [NSSortDescriptor]) {
+                            sortedBy descriptors: [NSSortDescriptor],
+                            transformer: @escaping (T) -> Output) {
 
         self.init(viewStorage: storageManager.viewStorage,
                   sectionNameKeyPath: sectionNameKeyPath,
                   matching: predicate,
                   fetchLimit: fetchLimit,
-                  sortedBy: descriptors)
+                  sortedBy: descriptors,
+                  transformer: transformer)
     }
 
 
@@ -139,14 +147,14 @@ public class ResultsController<T: ResultsControllerMutableType> {
     ///
     /// Prefer to use `safeObject(at:)` instead.
     ///
-    public func object(at indexPath: IndexPath) -> T.ReadOnlyType {
-        return controller.object(at: indexPath).toReadOnly()
+    public func object(at indexPath: IndexPath) -> Output {
+        return transformer(controller.object(at: indexPath))
     }
 
     /// Returns the fetched object at the given `indexPath`. Returns `nil` if the `indexPath`
     /// does not exist.
     ///
-    public func safeObject(at indexPath: IndexPath) -> T.ReadOnlyType? {
+    public func safeObject(at indexPath: IndexPath) -> Output? {
         guard !isEmpty else {
             return nil
         }
@@ -160,7 +168,7 @@ public class ResultsController<T: ResultsControllerMutableType> {
             return nil
         }
 
-        return controller.object(at: indexPath).toReadOnly()
+        return transformer(controller.object(at: indexPath))
     }
 
     /// Returns the Plain ObjectIndex corresponding to a given IndexPath. You can use this index to map the
@@ -194,23 +202,24 @@ public class ResultsController<T: ResultsControllerMutableType> {
     }
 
     /// Returns an array of all of the (ReadOnly) Fetched Objects.
+    /// Note: Avoid calling this in computed variables as the conversion of storage items can be costly.
     ///
-    public var fetchedObjects: [T.ReadOnlyType] {
-        let readOnlyObjects = controller.fetchedObjects?.compactMap { mutableObject in
-            mutableObject.toReadOnly()
+    public var fetchedObjects: [Output] {
+        let transformedObjects = controller.fetchedObjects?.compactMap { mutableObject in
+            transformer(mutableObject)
         }
 
-        return readOnlyObjects ?? []
+        return transformedObjects ?? []
     }
 
     /// Returns an array of SectionInfo Entitites.
     ///
     public var sections: [SectionInfo] {
-        let readOnlySections = controller.sections?.compactMap { mutableSection in
-            SectionInfo(mutableSection: mutableSection)
+        let transformedSections = controller.sections?.compactMap { mutableSection in
+            SectionInfo(mutableSection: mutableSection, transformer: transformer)
         }
 
-        return readOnlySections ?? []
+        return transformedSections ?? []
     }
 
     /// Returns an optional index path of the first matching object.
@@ -260,17 +269,17 @@ public class ResultsController<T: ResultsControllerMutableType> {
                 return
             }
 
-            let readOnlyObject = object.toReadOnly()
-            self.onDidChangeObject?(readOnlyObject, indexPath, type, newIndexPath)
+            let transformedObject = transformer(object)
+            self.onDidChangeObject?(transformedObject, indexPath, type, newIndexPath)
         }
 
         internalDelegate.onDidChangeSection = { [weak self] (mutableSection, sectionIndex, type) in
             guard let `self` = self else {
                 return
             }
 
-            let readOnlySection = SectionInfo(mutableSection: mutableSection)
-            self.onDidChangeSection?(readOnlySection, sectionIndex, type)
+            let transformedSection = SectionInfo(mutableSection: mutableSection, transformer: transformer)
+            self.onDidChangeSection?(transformedSection, sectionIndex, type)
         }
     }
 
@@ -294,7 +303,7 @@ public class ResultsController<T: ResultsControllerMutableType> {
 
 // MARK: - Nested Types
 //
-public extension ResultsController {
+public extension GenericResultsController {
 
     // MARK: - ResultsController.ChangeType
     //
@@ -322,9 +331,13 @@ public extension ResultsController {
             mutableSectionInfo.numberOfObjects
         }
 
-        /// Returns the array of (ReadOnly) objects in the section.
+        /// Transformer closure to convert objects in the section.
+        ///
+        private let transformer: (T) -> Output
+
+        /// Returns the array of transformed objects in the section.
         ///
-        private(set) public lazy var objects: [T.ReadOnlyType] = {
+        private(set) public lazy var objects: [Output] = {
             guard let objects = mutableSectionInfo.objects else {
                 return []
             }
@@ -333,13 +346,47 @@ public extension ResultsController {
                 return []
             }
 
-            return castedObjects.map { $0.toReadOnly() }
+            return castedObjects.map { transformer($0) }
         }()
 
         /// Designated Initializer
         ///
-        init(mutableSection: NSFetchedResultsSectionInfo) {
+        init(mutableSection: NSFetchedResultsSectionInfo, transformer: @escaping (T) -> Output) {
             mutableSectionInfo = mutableSection
+            self.transformer = transformer
         }
     }
 }
+
+// MARK: - ResultsController (Backward Compatible Specialization)
+//
+public class ResultsController<T: ResultsControllerMutableType>: GenericResultsController<T, T.ReadOnlyType> {
+    /// Designated Initializer.
+    ///
+    public init(viewStorage: StorageType,
+                sectionNameKeyPath: String? = nil,
+                matching predicate: NSPredicate? = nil,
+                fetchLimit: Int? = nil,
+                sortedBy descriptors: [NSSortDescriptor]) {
+        super.init(viewStorage: viewStorage,
+                   sectionNameKeyPath: sectionNameKeyPath,
+                   matching: predicate,
+                   fetchLimit: fetchLimit,
+                   sortedBy: descriptors,
+                   transformer: { $0.toReadOnly() })
+    }
+
+    /// Convenience Initializer.
+    ///
+    public convenience init(storageManager: StorageManagerType,
+                            sectionNameKeyPath: String? = nil,
+                            matching predicate: NSPredicate? = nil,
+                            fetchLimit: Int? = nil,
+                            sortedBy descriptors: [NSSortDescriptor]) {
+        self.init(viewStorage: storageManager.viewStorage,
+                  sectionNameKeyPath: sectionNameKeyPath,
+                  matching: predicate,
+                  fetchLimit: fetchLimit,
+                  sortedBy: descriptors)
+    }
+}

RELEASE-NOTES.txt

@@ -6,6 +6,7 @@
 - [*] Increased decimal sensitivity in order creation to mitigate tax rounding issues [https://github.com/woocommerce/woocommerce-ios/pull/15957]
 - [*] Fix initialization of authenticator to avoid crashes during login [https://github.com/woocommerce/woocommerce-ios/pull/15953]
 - [*] Shipping Labels: Made HS tariff number field required in customs form for EU destinations [https://github.com/woocommerce/woocommerce-ios/pull/15946]
+- [*] Order Details: Attempt to improve performance by using a simplified version of product objects. [https://github.com/woocommerce/woocommerce-ios/pull/15959]
 - [*] Order Details > Edit Shipping/Billing Address: Added map-based address lookup support for iOS 17+. [https://github.com/woocommerce/woocommerce-ios/pull/15964]
 - [*] Order Creation: Prevent subscription products to be added to an order [https://github.com/woocommerce/woocommerce-ios/pull/15960]
 - [internal] Replace COTS_DEVICE reader model name with TAP_TO_PAY_DEVICE. [https://github.com/woocommerce/woocommerce-ios/pull/15961]

WooCommerce/Classes/Analytics/WooAnalyticsEvent.swift

@@ -601,15 +601,15 @@ extension WooAnalyticsEvent {
             WooAnalyticsEvent(statName: .orderAddNew, properties: [:])
         }
 
-        static func orderProductsLoaded(order: Order, products: [Product], addOnGroups: [AddOnGroup]) -> WooAnalyticsEvent {
+        static func orderProductsLoaded(order: Order, products: [OrderDetailsProduct], addOnGroups: [AddOnGroup]) -> WooAnalyticsEvent {
             let productTypes = productTypes(order: order, products: products)
             let hasAddOns = hasAddOns(order: order, products: products, addOnGroups: addOnGroups)
             return WooAnalyticsEvent(statName: .orderProductsLoaded, properties: [Keys.orderID: order.orderID,
                                                                                   Keys.productTypes: productTypes,
                                                                                   Keys.hasAddOns: hasAddOns])
         }
 
-        private static func hasAddOns(order: Order, products: [Product], addOnGroups: [AddOnGroup]) -> Bool {
+        private static func hasAddOns(order: Order, products: [OrderDetailsProduct], addOnGroups: [AddOnGroup]) -> Bool {
             for item in order.items {
                 guard let product = products.first(where: { $0.productID == item.productID }) else {
                     continue
@@ -628,6 +628,13 @@ extension WooAnalyticsEvent {
             return false
         }
 
+        private static func productTypes(order: Order, products: [OrderDetailsProduct]) -> String {
+            let productIDs = order.items.map { $0.productID }
+            return productIDs.compactMap { productID in
+                products.first(where: { $0.productID == productID })?.productType.rawValue
+            }.uniqued().sorted().joined(separator: ",")
+        }
+
         private static func productTypes(order: Order, products: [Product]) -> String {
             let productIDs = order.items.map { $0.productID }
             return productIDs.compactMap { productID in

WooCommerce/Classes/ViewModels/Order Details/AddOns/AddOnCrossreferenceUseCase.swift

@@ -15,13 +15,13 @@ struct AddOnCrossreferenceUseCase {
 
     /// Product entity with known addOns that matches the order item.
     ///
-    private let product: Product
+    private let product: OrderDetailsProduct
 
     /// Global add-ons for the site.
     ///
     private let addOnGroups: [AddOnGroup]
 
-    init(orderItemAttributes: [OrderItemAttribute], product: Product, addOnGroups: [AddOnGroup]) {
+    init(orderItemAttributes: [OrderItemAttribute], product: OrderDetailsProduct, addOnGroups: [AddOnGroup]) {
         self.orderItemAttributes = orderItemAttributes
         self.product = product
         self.addOnGroups = addOnGroups

WooCommerce/Classes/ViewModels/Order Details/OrderDetailsDataSource.swift

@@ -149,7 +149,7 @@ final class OrderDetailsDataSource: NSObject {
 
     /// Products from an Order
     ///
-    var products: [Product] = []
+    var products: [OrderDetailsProduct] = []
 
     /// Product variations from an order
     ///
@@ -891,13 +891,14 @@ private extension OrderDetailsDataSource {
         }
 
         let imageURL: URL? = {
-            guard let imageURLString = aggregateItem.variationID != 0 ?
-                    lookUpProductVariation(productID: aggregateItem.productID, variationID: aggregateItem.variationID)?.image?.src:
-                    lookUpProduct(by: aggregateItem.productID)?.images.first?.src,
-                  let encodedImageURLString = imageURLString.addingPercentEncoding(withAllowedCharacters: .urlQueryAllowed) else {
-                return nil
+            if aggregateItem.variationID != 0 {
+                guard let imageURLString = lookUpProductVariation(productID: aggregateItem.productID, variationID: aggregateItem.variationID)?.image?.src,
+                      let encodedImageURLString = imageURLString.addingPercentEncoding(withAllowedCharacters: .urlQueryAllowed) else {
+                    return nil
+                }
+                return URL(string: encodedImageURLString)
             }
-            return URL(string: encodedImageURLString)
+            return lookUpProduct(by: aggregateItem.productID)?.imageURL
         }()
 
         let addOns: [OrderItemProductAddOn] = {
@@ -1234,7 +1235,7 @@ extension OrderDetailsDataSource {
         return currentSiteStatuses.filter({$0.status == order.status}).first
     }
 
-    func lookUpProduct(by productID: Int64) -> Product? {
+    func lookUpProduct(by productID: Int64) -> OrderDetailsProduct? {
         return products.filter({ $0.productID == productID }).first
     }