[Woo POS] [Variable Products] Use pagination headers for loading prod…

…ucts (#14728)
woocommerce · Dec 19, 2024 · a157f2b · a157f2b
2 parents b3fd522 + 1fb8df0
commit a157f2b
Show file tree

Hide file tree

Showing 18 changed files with 376 additions and 93 deletions.
diff --git a/Networking/Networking/Network/AlamofireNetwork.swift b/Networking/Networking/Network/AlamofireNetwork.swift
@@ -84,6 +84,22 @@ public class AlamofireNetwork: Network {
             }
     }
 
+    public func responseDataAndHeaders(for request: URLRequestConvertible) async throws -> (Data, ResponseHeaders?) {
+        let request = requestConverter.convert(request)
+        let sessionRequest = alamofireSession.request(request)
+            .validateIfRestRequest(for: request)
+        let response = await sessionRequest.serializingData().response
+        if let error = response.networkingError {
+            throw error
+        }
+        switch response.result {
+            case .success(let data):
+                return (data, response.response?.headers.dictionary)
+            case .failure(let error):
+                throw error
+        }
+    }
+
     /// Executes the specified Network Request. Upon completion, the payload or error will be emitted to the publisher.
     /// Only one value will be emitted and the request cannot be retried.
     ///

diff --git a/Networking/Networking/Network/MockNetwork.swift b/Networking/Networking/Network/MockNetwork.swift
@@ -27,6 +27,9 @@ class MockNetwork: Network {
     ///
     var requestsForResponseData = [URLRequestConvertible]()
 
+    /// Response headers to be returned with the response data.
+    var responseHeaders: [String: String]?
+
     /// Number of notification objects in notifications-load-all.json file.
     ///
     static let notificationLoadAllJSONCount = 46
@@ -78,6 +81,20 @@ class MockNetwork: Network {
         completion(.success(data))
     }
 
+    func responseDataAndHeaders(for request: any URLRequestConvertible) async throws -> (Data, ResponseHeaders?) {
+        requestsForResponseData.append(request)
+
+        if let error = error(for: request) {
+            throw error
+        }
+
+        guard let name = filename(for: request), let data = Loader.contentsOf(name) else {
+            throw NetworkError.notFound()
+        }
+
+        return (data, responseHeaders)
+    }
+
     func responseDataPublisher(for request: URLRequestConvertible) -> AnyPublisher<Swift.Result<Data, Error>, Never> {
         requestsForResponseData.append(request)
 

diff --git a/Networking/Networking/Network/Network.swift b/Networking/Networking/Network/Network.swift
@@ -18,6 +18,7 @@ public protocol MultipartFormData {
 /// Unit Testing target, and inject mocked up responses.
 ///
 public protocol Network {
+    typealias ResponseHeaders = [String: String]
 
     var session: URLSession { get }
 
@@ -39,6 +40,8 @@ public protocol Network {
     func responseData(for request: URLRequestConvertible,
                       completion: @escaping (Swift.Result<Data, Error>) -> Void)
 
+    func responseDataAndHeaders(for request: URLRequestConvertible) async throws -> (Data, ResponseHeaders?)
+
     /// Executes the specified Network Request. Upon completion, the payload or error will be emitted to the publisher.
     ///
     /// - Parameters:

diff --git a/Networking/Networking/Network/NullNetwork.swift b/Networking/Networking/Network/NullNetwork.swift
@@ -19,6 +19,10 @@ public final class NullNetwork: Network {
 
     }
 
+    public func responseDataAndHeaders(for request: any URLRequestConvertible) async throws -> (Data, ResponseHeaders?) {
+        throw NetworkError.notFound()
+    }
+
     public func responseDataPublisher(for request: URLRequestConvertible) -> AnyPublisher<Swift.Result<Data, Error>, Never> {
         Empty<Swift.Result<Data, Error>, Never>().eraseToAnyPublisher()
     }

diff --git a/Networking/Networking/Network/WordPressOrgNetwork.swift b/Networking/Networking/Network/WordPressOrgNetwork.swift
@@ -104,6 +104,22 @@ public final class WordPressOrgNetwork: Network {
             }
     }
 
+    public func responseDataAndHeaders(for request: URLRequestConvertible) async throws -> (Data, ResponseHeaders?) {
+        let sessionRequest = alamofireSession.request(request).validate()
+        let response = await sessionRequest.serializingData().response
+        do {
+            try validateResponse(response.data)
+            switch response.result {
+                case .success(let data):
+                    return (data, response.response?.headers.dictionary)
+                case .failure(let error):
+                    throw error
+            }
+        } catch {
+            throw error
+        }
+    }
+
     /// Executes the specified Network Request. Upon completion, the payload or error will be emitted to the publisher.
     /// Only one value will be emitted and the request cannot be retried.
     ///

diff --git a/Networking/Networking/Remote/ProductsRemote.swift b/Networking/Networking/Remote/ProductsRemote.swift
@@ -204,7 +204,9 @@ public final class ProductsRemote: Remote, ProductsRemoteProtocol {
     /// - productTypes: A list of product types to be included in the results.
     /// - pageNumber: Number of page that should be retrieved.
     ///
-    public func loadProductsForPointOfSale(for siteID: Int64, productTypes: [ProductType] = [.simple], pageNumber: Int = 1) async throws -> [Product] {
+    public func loadProductsForPointOfSale(for siteID: Int64,
+                                           productTypes: [ProductType] = [.simple],
+                                           pageNumber: Int = 1) async throws -> PagedItems<Product> {
         let parameters = [
             ParameterKey.page: String(pageNumber),
             ParameterKey.perPage: POSConstants.productsPerPage,
@@ -222,7 +224,16 @@ public final class ProductsRemote: Remote, ProductsRemoteProtocol {
                                      parameters: parameters,
                                      availableAsRESTRequest: true)
         let mapper = ProductListMapper(siteID: siteID)
-        return try await enqueue(request, mapper: mapper)
+
+        let (products, responseHeaders) = try await enqueueWithResponseHeaders(request, mapper: mapper)
+
+        // Extracts the total number of pages from the response headers.
+        // Response header names are case insensitive.
+        let totalPages = responseHeaders?.first(where: { $0.key.lowercased() == Remote.PaginationHeaderKey.totalPagesCount.lowercased() })
+            .flatMap { Int($0.value) }
+        let hasMorePages = totalPages.map { pageNumber < $0 } ?? true
+
+        return .init(items: products, hasMorePages: hasMorePages)
     }
 
     /// Retrieves a specific list of `Product`s by `productID`.

diff --git a/Networking/Networking/Remote/Remote.swift b/Networking/Networking/Remote/Remote.swift
@@ -258,8 +258,33 @@ public class Remote: NSObject {
             }
         }
     }
+
+    func enqueueWithResponseHeaders<M: Mapper>(_ request: Request, mapper: M) async throws -> (data: M.Output, headers: [String: String]?) {
+        do {
+            let (data, headers) = try await network.responseDataAndHeaders(for: request)
+            let validator = request.responseDataValidator()
+            let parsedData = try validateAndParseData(data, request: request, validator: validator, mapper: mapper)
+            return (data: parsedData, headers: headers)
+        } catch {
+            handleResponseError(error: error, for: request)
+            throw mapNetworkError(error: error, for: request)
+        }
+    }
 }
 
+private extension Remote {
+    // Validation and parsing of the response data is separated so that the decoding error can be handled separately from network error.
+    func validateAndParseData<M: Mapper>(_ data: Data, request: Request, validator: ResponseDataValidator, mapper: M) throws -> M.Output {
+        do {
+            try validator.validate(data: data)
+            return try mapper.map(response: data)
+        } catch {
+            DDLogError("<> Mapping Error: \(error)")
+            handleDecodingError(error: error, for: request, entityName: "\(M.Output.self)")
+            throw error
+        }
+    }
+}
 
 // MARK: - Private Methods
 //
@@ -341,6 +366,17 @@ private extension Remote {
     }
 }
 
+/// Contains the result of a paginated request.
+public struct PagedItems<T> {
+    public let items: [T]
+    public let hasMorePages: Bool
+
+    public init(items: [T], hasMorePages: Bool) {
+        self.items = items
+        self.hasMorePages = hasMorePages
+    }
+}
+
 // MARK: - Constants!
 //
 public extension Remote {
@@ -349,6 +385,10 @@ public extension Remote {
         public static let firstPageNumber: Int = 1
     }
 
+    enum PaginationHeaderKey {
+        static let totalPagesCount = "x-wp-totalpages"
+    }
+
     enum JSONParsingErrorUserInfoKey {
         public static let path = "path"
         public static let entityName = "entity"

diff --git a/Networking/NetworkingTests/Remote/ProductsRemoteTests.swift b/Networking/NetworkingTests/Remote/ProductsRemoteTests.swift
@@ -940,9 +940,10 @@ final class ProductsRemoteTests: XCTestCase {
         // When
         network.simulateResponse(requestUrlSuffix: "products", filename: "products-load-all-type-simple")
 
-        let products = try await remote.loadProductsForPointOfSale(for: sampleSiteID)
+        let pagedProducts = try await remote.loadProductsForPointOfSale(for: sampleSiteID)
 
         // Then
+        let products = pagedProducts.items
         XCTAssertEqual(products.count, expectedProductsFromResponse)
         for product in products {
             XCTAssertEqual(try XCTUnwrap(product).productType, .simple)
@@ -970,9 +971,10 @@ final class ProductsRemoteTests: XCTestCase {
         // When
         network.simulateResponse(requestUrlSuffix: "products", filename: "products-load-all-type-simple")
 
-        let products = try await remote.loadProductsForPointOfSale(for: sampleSiteID, pageNumber: initialPageNumber)
+        let pagedProducts = try await remote.loadProductsForPointOfSale(for: sampleSiteID, pageNumber: initialPageNumber)
 
         // Then
+        let products = pagedProducts.items
         XCTAssertEqual(products.count, expectedProductsFromResponse)
         for product in products {
             XCTAssertEqual(try XCTUnwrap(product).productType, .simple)
@@ -988,10 +990,46 @@ final class ProductsRemoteTests: XCTestCase {
         // When
         network.simulateResponse(requestUrlSuffix: "products", filename: "empty-data-array")
 
-        let products = try await remote.loadProductsForPointOfSale(for: sampleSiteID, pageNumber: pageNumber)
+        let pagedProducts = try await remote.loadProductsForPointOfSale(for: sampleSiteID, pageNumber: pageNumber)
 
         // Then
-        XCTAssertEqual(products.count, expectedProductsFromResponse)
+        XCTAssertEqual(pagedProducts.items.count, expectedProductsFromResponse)
+    }
+
+    func test_loadProductsForPointOfSale_returns_hasMorePages_based_on_header_with_case_insensitive_name() async throws {
+        // Given
+        let remote = ProductsRemote(network: network)
+        network.responseHeaders = ["X-WP-TotalPages": "5"]
+        network.simulateResponse(requestUrlSuffix: "products", filename: "empty-data-array")
+
+        // When loading page 1 to 4
+        for pageNumber in 1...4 {
+            let pagedProducts = try await remote.loadProductsForPointOfSale(for: sampleSiteID, pageNumber: pageNumber)
+
+            // Then
+            XCTAssertTrue(pagedProducts.hasMorePages)
+        }
+
+        // When loading page 17
+        let pagedProducts = try await remote.loadProductsForPointOfSale(for: sampleSiteID, pageNumber: 5)
+
+        // Then
+        XCTAssertFalse(pagedProducts.hasMorePages)
+    }
+
+    func test_loadProductsForPointOfSale_returns_hasMorePages_true_when_header_is_not_set() async throws {
+        // Given
+        let remote = ProductsRemote(network: network)
+        network.responseHeaders = nil
+        network.simulateResponse(requestUrlSuffix: "products", filename: "empty-data-array")
+
+        // When loading the first 5 pages
+        for pageNumber in 1...5 {
+            let pagedProducts = try await remote.loadProductsForPointOfSale(for: sampleSiteID, pageNumber: pageNumber)
+
+            // Then
+            XCTAssertTrue(pagedProducts.hasMorePages)
+        }
     }
 }
 

diff --git a/WooCommerce/Classes/POS/Controllers/PointOfSaleItemsController.swift b/WooCommerce/Classes/POS/Controllers/PointOfSaleItemsController.swift
@@ -15,70 +15,79 @@ class PointOfSaleItemsController: PointOfSaleItemsControllerProtocol {
     private(set) var itemListStatePublisher: any Publisher<ItemListState, Never>
     private var itemListStateSubject: PassthroughSubject<ItemListState, Never> = .init()
     private var allItems: [POSItem] = []
-    private var currentPage: Int = Constants.initialPage
-    private var mightHaveMorePages: Bool = true
+    private let paginationTracker: AsyncPaginationTracker
     private let itemProvider: PointOfSaleItemServiceProtocol
 
     init(itemProvider: PointOfSaleItemServiceProtocol) {
         self.itemProvider = itemProvider
+        self.paginationTracker = .init()
         itemListStatePublisher = itemListStateSubject.eraseToAnyPublisher()
     }
 
     @MainActor
     func loadInitialItems() async {
-        mightHaveMorePages = true
         itemListStateSubject.send(.initialLoading)
-        try? await load(pageNumber: Constants.initialPage)
+        do {
+            try await paginationTracker.syncFirstPage { [weak self] pageNumber in
+                guard let self else { return true }
+                return try await fetchItems(pageNumber: pageNumber)
+            }
+            updateItemListStateAfterLoadAttempt()
+        } catch {
+            itemListStateSubject.send(.error(PointOfSaleErrorState.errorOnLoadingProducts()))
+        }
     }
 
     @MainActor
     func loadNextItems() async {
+        guard paginationTracker.hasNextPage else {
+            return
+        }
+        itemListStateSubject.send(.loading(allItems))
         do {
-            guard mightHaveMorePages else {
-                return
+            let nextPageState = try await paginationTracker.ensureNextPageIsSynced { [weak self] pageNumber in
+                guard let self else { return true }
+                return try await fetchItems(pageNumber: pageNumber)
+            }
+            switch nextPageState {
+                case .noNextPage, .synced:
+                    updateItemListStateAfterLoadAttempt()
+                case .syncing:
+                    break
             }
-            itemListStateSubject.send(.loading(allItems))
-
-            let nextPage = currentPage + 1
-            try await load(pageNumber: nextPage)
-            currentPage = nextPage
         } catch {
-            // Handle errors without incrementing currentPage.
+            // TODO: 14694 - Handle error from loading the next page, like showing an error UI at the end or as an overlay.
+            itemListStateSubject.send(.error(PointOfSaleErrorState.errorOnLoadingProducts()))
         }
     }
 
     @MainActor
     func reload() async {
         allItems.removeAll()
-        currentPage = Constants.initialPage
-        mightHaveMorePages = true
-        itemListStateSubject.send(.loading(allItems))
-        try? await load(pageNumber: currentPage)
-    }
-
-    @MainActor
-    private func load(pageNumber: Int) async throws {
         do {
-            try await fetchItems(pageNumber: pageNumber)
-            mightHaveMorePages = true
-            updateItemListStateAfterLoadAttempt()
-        } catch PointOfSaleProductServiceError.pageOutOfRange {
-            mightHaveMorePages = false
+            try await paginationTracker.resync { [weak self] pageNumber in
+                guard let self else { return true }
+                return try await fetchItems(pageNumber: pageNumber)
+            }
             updateItemListStateAfterLoadAttempt()
-            throw PointOfSaleProductServiceError.pageOutOfRange
         } catch {
+            // TODO: 14694 - Handle error from pull-to-refresh, like showing an error UI at the beginning or as an overlay.
             itemListStateSubject.send(.error(PointOfSaleErrorState.errorOnLoadingProducts()))
-            throw error
         }
     }
 
+    /// Fetches items given a page number and appends new unique items to the `allItems` array.
+    /// - Parameter pageNumber: Page number to fetch items from.
+    /// - Returns: A boolean that indicates whether there is next page for the paginated items.
     @MainActor
-    private func fetchItems(pageNumber: Int) async throws {
-        let newItems = try await itemProvider.providePointOfSaleItems(pageNumber: pageNumber)
+    private func fetchItems(pageNumber: Int) async throws -> Bool {
+        let pagedItems = try await itemProvider.providePointOfSaleItems(pageNumber: pageNumber)
+        let newItems = pagedItems.items
         let uniqueNewItems = newItems.filter { newItem in
             !allItems.contains(newItem)
         }
         allItems.append(contentsOf: uniqueNewItems)
+        return pagedItems.hasMorePages
     }
 
     private func updateItemListStateAfterLoadAttempt() {
@@ -88,8 +97,4 @@ class PointOfSaleItemsController: PointOfSaleItemsControllerProtocol {
             itemListStateSubject.send(.loaded(allItems))
         }
     }
-
-    private enum Constants {
-        static let initialPage: Int = 1
-    }
 }