-
Notifications
You must be signed in to change notification settings - Fork 121
[Local Catalog] Add GRDB data source for item list #16231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
fd80f54
764f97c
a84aebc
f346d98
2e56a86
95f3ddc
16f9345
dce795b
8c63b39
5354d0a
794dca7
96c7d30
b832cd6
728ac9a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,232 @@ | ||
| // periphery:ignore:all | ||
| import Foundation | ||
| import GRDB | ||
| import Combine | ||
| import Observation | ||
| import Storage | ||
| import WooFoundation | ||
|
|
||
| /// Observable data source for GRDB-based POS items using ValueObservation | ||
| /// Provides automatic SwiftUI updates when database changes occur | ||
| @Observable | ||
| public final class GRDBObservableDataSource: POSObservableDataSourceProtocol { | ||
| // MARK: - Observable Properties | ||
|
|
||
| public private(set) var productItems: [POSItem] = [] | ||
| public private(set) var variationItems: [POSItem] = [] | ||
| public private(set) var isLoadingProducts: Bool = false | ||
| public private(set) var isLoadingVariations: Bool = false | ||
| public private(set) var error: Error? = nil | ||
|
|
||
| public var hasMoreProducts: Bool { | ||
| productItems.count >= (pageSize * currentProductPage) && totalProductCount > productItems.count | ||
| } | ||
|
|
||
| public var hasMoreVariations: Bool { | ||
| variationItems.count >= (pageSize * currentVariationPage) && totalVariationCount > variationItems.count | ||
| } | ||
|
|
||
| // MARK: - Private Properties | ||
|
|
||
| private let siteID: Int64 | ||
| private let grdbManager: GRDBManagerProtocol | ||
| private let itemMapper: PointOfSaleItemMapperProtocol | ||
| private let pageSize: Int | ||
|
|
||
| private var currentProductPage: Int = 1 | ||
| private var currentVariationPage: Int = 1 | ||
| private var currentParentProduct: POSVariableParentProduct? | ||
| private var totalProductCount: Int = 0 | ||
| private var totalVariationCount: Int = 0 | ||
|
|
||
| // ValueObservation subscriptions | ||
| private var productObservationCancellable: AnyCancellable? | ||
| private var variationObservationCancellable: AnyCancellable? | ||
| private var statisticsObservationCancellable: AnyCancellable? | ||
|
|
||
| // MARK: - Initialization | ||
|
|
||
| public init(siteID: Int64, | ||
| grdbManager: GRDBManagerProtocol, | ||
| currencySettings: CurrencySettings, | ||
| itemMapper: PointOfSaleItemMapperProtocol? = nil, | ||
| pageSize: Int = 20) { | ||
| self.siteID = siteID | ||
| self.grdbManager = grdbManager | ||
| self.itemMapper = itemMapper ?? PointOfSaleItemMapper(currencySettings: currencySettings) | ||
| self.pageSize = pageSize | ||
|
|
||
| setupStatisticsObservation() | ||
| } | ||
|
|
||
| deinit { | ||
| productObservationCancellable?.cancel() | ||
| variationObservationCancellable?.cancel() | ||
| statisticsObservationCancellable?.cancel() | ||
| } | ||
|
|
||
| // MARK: - POSObservableDataSourceProtocol | ||
|
|
||
| public func loadProducts() { | ||
| currentProductPage = 1 | ||
| isLoadingProducts = true | ||
| setupProductObservation() | ||
| } | ||
|
|
||
| public func loadMoreProducts() { | ||
| guard hasMoreProducts && !isLoadingProducts else { return } | ||
|
|
||
| isLoadingProducts = true | ||
| currentProductPage += 1 | ||
| setupProductObservation() | ||
| } | ||
|
|
||
| public func loadVariations(for parentProduct: POSVariableParentProduct) { | ||
| guard currentParentProduct?.productID != parentProduct.productID else { | ||
| return // Same parent - idempotent | ||
| } | ||
|
|
||
| currentParentProduct = parentProduct | ||
| currentVariationPage = 1 | ||
| isLoadingVariations = true | ||
| variationItems = [] | ||
|
|
||
| setupVariationObservation(parentProduct: parentProduct) | ||
| } | ||
|
|
||
| public func loadMoreVariations() { | ||
| guard let parentProduct = currentParentProduct, | ||
| hasMoreVariations && !isLoadingVariations else { return } | ||
|
|
||
| isLoadingVariations = true | ||
| currentVariationPage += 1 | ||
| setupVariationObservation(parentProduct: parentProduct) | ||
| } | ||
|
|
||
| public func refresh() { | ||
| // No-op: database observation automatically updates when data changes during incremental sync | ||
| } | ||
|
|
||
| // MARK: - ValueObservation Setup | ||
|
|
||
| private func setupProductObservation() { | ||
| let currentPage = currentProductPage | ||
| let observation = ValueObservation | ||
| .tracking { [weak self] database -> [POSProduct] in | ||
| guard let self else { return [] } | ||
|
|
||
| let persistedProducts = try PersistedProduct | ||
| .posProductsRequest(siteID: siteID) | ||
| .limit(pageSize * currentPage) | ||
| .fetchAll(database) | ||
|
|
||
| return try persistedProducts.map { persistedProduct in | ||
| let images = try persistedProduct.request(for: PersistedProduct.images).fetchAll(database) | ||
| let attributes = try persistedProduct.request(for: PersistedProduct.attributes).fetchAll(database) | ||
|
|
||
| return persistedProduct.toPOSProduct( | ||
| images: images.map { $0.toProductImage() }, | ||
| attributes: attributes.map { $0.toProductAttribute(siteID: persistedProduct.siteID) } | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| productObservationCancellable = observation | ||
| .publisher(in: grdbManager.databaseConnection) | ||
| .receive(on: DispatchQueue.main) | ||
| .sink( | ||
| receiveCompletion: { [weak self] completion in | ||
| if case .failure(let error) = completion { | ||
| self?.error = error | ||
| self?.isLoadingProducts = false | ||
| } | ||
| }, | ||
| receiveValue: { [weak self] observedProducts in | ||
| guard let self else { return } | ||
| let posItems = itemMapper.mapProductsToPOSItems(products: observedProducts) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mentioned the performance with a lot of items which may be connected to mapping, could we do the mapping outside the main thread and only receive on main thread? .publisher(in: grdbManager.databaseConnection)
.map { [itemMapper] observedProducts in
// Mapping happens on background Combine scheduler
itemMapper.mapProductsToPOSItems(products: observedProducts)
}
.receive(on: DispatchQueue.main)We could also leave such performance considerations for the future. I'm sure there are even more things that we could do if we reach use cases with extremely large catalogs.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I thought about this approach after I finished up last week, and it should work. But I previously had the mapping happen off the main thread (with different syntax) and it didn't make a noticable performance difference, so I think it's fine to leave for now. |
||
| productItems = posItems | ||
| error = nil | ||
| isLoadingProducts = false | ||
| } | ||
| ) | ||
| } | ||
|
|
||
| private func setupVariationObservation(parentProduct: POSVariableParentProduct) { | ||
| variationObservationCancellable?.cancel() | ||
|
|
||
| let currentPage = currentVariationPage | ||
| let observation = ValueObservation | ||
| .tracking { [weak self] database -> [POSProductVariation] in | ||
| guard let self else { return [] } | ||
|
|
||
| let persistedVariations = try PersistedProductVariation | ||
| .posVariationsRequest(siteID: self.siteID, parentProductID: parentProduct.productID) | ||
| .limit(self.pageSize * currentPage) | ||
| .fetchAll(database) | ||
|
|
||
| return try persistedVariations.map { persistedVariation in | ||
| let attributes = try persistedVariation.request(for: PersistedProductVariation.attributes).fetchAll(database) | ||
| let image = try persistedVariation.request(for: PersistedProductVariation.image).fetchOne(database) | ||
|
|
||
| return persistedVariation.toPOSProductVariation( | ||
| attributes: attributes.map { $0.toProductVariationAttribute() }, | ||
| image: image?.toProductImage() | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| variationObservationCancellable = observation | ||
| .publisher(in: grdbManager.databaseConnection) | ||
| .receive(on: DispatchQueue.main) | ||
| .sink( | ||
| receiveCompletion: { [weak self] completion in | ||
| if case .failure(let error) = completion { | ||
| self?.error = error | ||
| self?.isLoadingVariations = false | ||
| } | ||
| }, | ||
| receiveValue: { [weak self] observedVariations in | ||
| guard let self else { return } | ||
| let posItems = itemMapper.mapVariationsToPOSItems( | ||
| variations: observedVariations, | ||
| parentProduct: parentProduct | ||
| ) | ||
| variationItems = posItems | ||
| error = nil | ||
| isLoadingVariations = false | ||
| } | ||
| ) | ||
| } | ||
|
|
||
| private func setupStatisticsObservation() { | ||
| let observation = ValueObservation | ||
| .tracking { [weak self] database in | ||
| guard let self else { return (0, 0) } | ||
|
|
||
| let productCount = try PersistedProduct | ||
| .posProductsRequest(siteID: siteID) | ||
| .fetchCount(database) | ||
|
|
||
| let variationCount = try PersistedProductVariation | ||
| .filter(PersistedProductVariation.Columns.siteID == siteID) | ||
|
||
| .fetchCount(database) | ||
|
|
||
| return (productCount, variationCount) | ||
| } | ||
|
|
||
| statisticsObservationCancellable = observation | ||
| .publisher(in: grdbManager.databaseConnection) | ||
| .receive(on: DispatchQueue.main) | ||
| .sink( | ||
| receiveCompletion: { completion in | ||
| if case .failure = completion { | ||
| // Silently ignore - statistics are not critical | ||
| } | ||
| }, | ||
| receiveValue: { [weak self] (productCount, variationCount) in | ||
| self?.totalProductCount = productCount | ||
| self?.totalVariationCount = variationCount | ||
| } | ||
| ) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| // periphery:ignore:all | ||
| import Foundation | ||
|
|
||
| /// Protocol for observable data sources that provide POS items with automatic updates | ||
| public protocol POSObservableDataSourceProtocol { | ||
| /// Current products mapped to POSItems | ||
| var productItems: [POSItem] { get } | ||
|
|
||
| /// Current variations for the selected parent product mapped to POSItems | ||
| var variationItems: [POSItem] { get } | ||
|
|
||
| /// Loading state for products | ||
| var isLoadingProducts: Bool { get } | ||
|
|
||
| /// Loading state for variations | ||
| var isLoadingVariations: Bool { get } | ||
|
|
||
| /// Whether more products are available to load | ||
| var hasMoreProducts: Bool { get } | ||
|
|
||
| /// Whether more variations are available for current parent | ||
| var hasMoreVariations: Bool { get } | ||
|
|
||
| /// Current error, if any | ||
| var error: Error? { get } | ||
|
|
||
| /// Loads the first page of products | ||
| func loadProducts() | ||
|
|
||
| /// Loads the next page of products | ||
| func loadMoreProducts() | ||
|
|
||
| /// Loads variations for a specific parent product | ||
| func loadVariations(for parentProduct: POSVariableParentProduct) | ||
|
|
||
| /// Loads more variations for the current parent product | ||
| func loadMoreVariations() | ||
|
|
||
| /// Refreshes all data | ||
| /// Note: For GRDB implementations, this is a no-op as the database observation | ||
| /// automatically updates when data changes during incremental sync | ||
| func refresh() | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| import Foundation | ||
| import Observation | ||
| import Yosemite | ||
|
|
||
| /// Mock implementation for testing and development | ||
| @Observable | ||
| final class MockPOSObservableDataSource: POSObservableDataSourceProtocol { | ||
| var productItems: [POSItem] = [] | ||
| var variationItems: [POSItem] = [] | ||
| var isLoadingProducts: Bool = false | ||
| var isLoadingVariations: Bool = false | ||
| var hasMoreProducts: Bool = false | ||
| var hasMoreVariations: Bool = false | ||
| var error: Error? = nil | ||
|
|
||
| init() {} | ||
|
|
||
| func loadProducts() { | ||
| // Tests set properties directly - no async behavior needed | ||
| } | ||
|
|
||
| func loadMoreProducts() { | ||
| // Tests set properties directly - no async behavior needed | ||
| } | ||
|
|
||
| func loadVariations(for parentProduct: POSVariableParentProduct) { | ||
| // Tests set properties directly - no async behavior needed | ||
| } | ||
|
|
||
| func loadMoreVariations() { | ||
| // Tests set properties directly - no async behavior needed | ||
| } | ||
|
|
||
| func refresh() { | ||
| // No-op for mock | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For each product, we request images and attributes. Is this an effective way?
Could we flatten it all by:
Map into POSProduct with attributes and images.
Or I see there also
.includingmethod which maybe could be used.Same for variations
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improved in b832cd6, 2x faster comparing test speed with 1000s of products.