-
Notifications
You must be signed in to change notification settings - Fork 119
[POS FTS] 3: Add inline FTS indexing to catalog persistence #16596
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
base: pos/fts-2-search-rebuild
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -40,6 +40,11 @@ public struct POSSearchIndexBuilder { | |
| } | ||
|
|
||
| /// Returns the total count of results matching the search term. | ||
| /// - Parameters: | ||
| /// - siteID: The site ID to search within | ||
| /// - term: The search term | ||
| /// - db: The database to search in | ||
| /// - Returns: Total count of matching results | ||
| public static func searchCount(siteID: Int64, term: String, in db: Database) throws -> Int { | ||
| let ftsQuery = buildFTSQuery(from: term) | ||
| guard !ftsQuery.isEmpty else { return 0 } | ||
|
|
@@ -54,6 +59,10 @@ public struct POSSearchIndexBuilder { | |
|
|
||
| /// Checks if the FTS index needs to be rebuilt for a site. | ||
| /// Compares the index entry count against the total eligible products and variations. | ||
| /// - Parameters: | ||
| /// - siteID: The site ID to check | ||
| /// - db: The database to check in | ||
| /// - Returns: True if rebuild is needed | ||
|
Comment on lines
+62
to
+65
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. Move these to the previous PR? |
||
| public static func needsRebuild(for siteID: Int64, in db: Database) throws -> Bool { | ||
| let productCount = try PersistedProduct | ||
| .posEligibleProductsRequest(siteID: siteID) | ||
|
|
@@ -72,11 +81,81 @@ public struct POSSearchIndexBuilder { | |
| return indexCount < productCount + variationCount | ||
| } | ||
|
|
||
| // MARK: - Inline Indexing (same transaction) | ||
|
|
||
| /// Clears the FTS index for a site. | ||
| /// - Parameters: | ||
| /// - siteID: The site ID to clear | ||
| /// - db: The database (must be called within a write transaction) | ||
|
Comment on lines
+84
to
+89
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. Move these to the previous PR? |
||
| public static func clearIndex(for siteID: Int64, in db: Database) throws { | ||
| try db.execute(sql: "DELETE FROM pos_search_fts WHERE siteID = ?", arguments: [siteID]) | ||
| } | ||
|
|
||
| /// Indexes a product inline within an existing transaction. | ||
| /// - Parameters: | ||
| /// - product: The persisted product to index | ||
| /// - siteID: The site ID | ||
| /// - db: The database (must be called within a write transaction) | ||
| public static func indexProduct(_ product: PersistedProduct, siteID: Int64, in db: Database) throws { | ||
| guard ["simple", "variable"].contains(product.productTypeKey), | ||
| !product.downloadable, | ||
| !["trash", "draft", "pending"].contains(product.statusKey) else { | ||
| return | ||
| } | ||
|
|
||
| let searchableText = buildSearchableText( | ||
| name: product.name, | ||
| sku: product.sku, | ||
| globalUniqueID: product.globalUniqueID, | ||
| attributes: nil | ||
| ) | ||
|
|
||
| try db.execute( | ||
| sql: "INSERT INTO pos_search_fts (searchable_text, siteID, itemType, itemID, parentProductID) VALUES (?, ?, ?, ?, ?)", | ||
| arguments: [searchableText, siteID, POSSearchIndex.ItemType.product.rawValue, product.id, nil as Int64?] | ||
| ) | ||
| } | ||
|
|
||
| /// Indexes a variation inline within an existing transaction. | ||
| /// - Parameters: | ||
| /// - variation: The persisted variation to index | ||
| /// - parentProductName: The name of the parent product | ||
| /// - attributes: The variation's attribute options (e.g., ["Red", "Large"]) | ||
| /// - siteID: The site ID | ||
| /// - db: The database (must be called within a write transaction) | ||
| public static func indexVariation(_ variation: PersistedProductVariation, | ||
| parentProductName: String?, | ||
| attributes: [String], | ||
| siteID: Int64, | ||
| in db: Database) throws { | ||
| guard !variation.downloadable else { return } | ||
|
|
||
| let searchableText = buildSearchableText( | ||
| name: parentProductName, | ||
| sku: variation.sku, | ||
| globalUniqueID: variation.globalUniqueID, | ||
| attributes: attributes.joined(separator: " ") | ||
| ) | ||
|
|
||
| try db.execute( | ||
| sql: "INSERT INTO pos_search_fts (searchable_text, siteID, itemType, itemID, parentProductID) VALUES (?, ?, ?, ?, ?)", | ||
| arguments: [searchableText, siteID, POSSearchIndex.ItemType.variation.rawValue, variation.id, variation.productID] | ||
| ) | ||
| } | ||
|
|
||
| /// Removes a product or variation from the FTS index. | ||
| /// - Parameters: | ||
| /// - itemID: The ID of the item to remove | ||
| /// - itemType: Whether this is a product or variation | ||
| /// - siteID: The site ID | ||
| /// - db: The database (must be called within a write transaction) | ||
| public static func removeFromIndex(itemID: Int64, itemType: POSSearchIndex.ItemType, siteID: Int64, in db: Database) throws { | ||
| try db.execute( | ||
| sql: "DELETE FROM pos_search_fts WHERE siteID = ? AND itemType = ? AND itemID = ?", | ||
| arguments: [siteID, itemType.rawValue, itemID] | ||
| ) | ||
| } | ||
|
|
||
| // MARK: - Internal (for testing) | ||
|
|
||
| /// Tokenizes a search term to match FTS5 unicode61 tokenizer behavior. | ||
|
|
@@ -133,17 +212,7 @@ public struct POSSearchIndexBuilder { | |
| .fetchAll(db) | ||
|
|
||
| for product in products { | ||
| let searchableText = buildSearchableText( | ||
| name: product.name, | ||
| sku: product.sku, | ||
| globalUniqueID: product.globalUniqueID, | ||
| attributes: nil | ||
| ) | ||
|
|
||
| try db.execute( | ||
| sql: "INSERT INTO pos_search_fts (searchable_text, siteID, itemType, itemID, parentProductID) VALUES (?, ?, ?, ?, ?)", | ||
| arguments: [searchableText, siteID, POSSearchIndex.ItemType.product.rawValue, product.id, nil as Int64?] | ||
| ) | ||
| try indexProduct(product, siteID: siteID, in: db) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -161,17 +230,11 @@ public struct POSSearchIndexBuilder { | |
| .fetchAll(db) | ||
| .map(\.option) | ||
|
|
||
| let searchableText = buildSearchableText( | ||
| name: parentProduct?.name, | ||
| sku: variation.sku, | ||
| globalUniqueID: variation.globalUniqueID, | ||
| attributes: attributes.joined(separator: " ") | ||
| ) | ||
|
|
||
| try db.execute( | ||
| sql: "INSERT INTO pos_search_fts (searchable_text, siteID, itemType, itemID, parentProductID) VALUES (?, ?, ?, ?, ?)", | ||
| arguments: [searchableText, siteID, POSSearchIndex.ItemType.variation.rawValue, variation.id, variation.productID] | ||
| ) | ||
| try indexVariation(variation, | ||
| parentProductName: parentProduct?.name, | ||
| attributes: attributes, | ||
| siteID: siteID, | ||
| in: db) | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,25 +38,39 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { | |
|
|
||
| try await grdbManager.databaseConnection.write { db in | ||
| DDLogInfo("🗑️ Clearing catalog data for site \(siteID)") | ||
|
|
||
| try POSSearchIndexBuilder.clearIndex(for: siteID, in: db) | ||
|
|
||
| try PersistedSite.deleteOne(db, key: siteID) | ||
|
|
||
| let site = PersistedSite(id: siteID, lastCatalogFullSyncDate: catalog.syncDate) | ||
| try site.insert(db) | ||
|
|
||
| let productNameLookup = Dictionary(uniqueKeysWithValues: catalog.products.map { ($0.productID, $0.name) }) | ||
| let variationAttributesLookup = Dictionary(grouping: catalog.variationAttributesToPersist) { $0.productVariationID } | ||
|
|
||
| for product in catalog.productsToPersist { | ||
| try product.insert(db, onConflict: .replace) | ||
| // Filters by eligibility internally | ||
| try POSSearchIndexBuilder.indexProduct(product, siteID: siteID, in: db) | ||
| } | ||
|
|
||
| for variation in catalog.variationsToPersist { | ||
| try variation.insert(db, onConflict: .replace) | ||
| let parentName = productNameLookup[variation.productID] | ||
| let attributes = variationAttributesLookup[variation.id]?.map { $0.option } ?? [] | ||
| try POSSearchIndexBuilder.indexVariation(variation, | ||
| parentProductName: parentName, | ||
| attributes: attributes, | ||
| siteID: siteID, | ||
| in: db) | ||
| } | ||
|
|
||
| // Insert actual image data first (shared by products and variations) | ||
|
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. Retain |
||
| // Insert images | ||
| for image in catalog.imagesToPersist { | ||
| try image.insert(db, onConflict: .replace) | ||
| } | ||
|
|
||
| // Then insert join table entries | ||
|
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. Retain |
||
| for productImage in catalog.productImagesToPersist { | ||
| try productImage.insert(db, onConflict: .replace) | ||
| } | ||
|
|
@@ -65,6 +79,7 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { | |
| try variationImage.insert(db, onConflict: .replace) | ||
| } | ||
|
|
||
| // Insert attributes | ||
| for var attribute in catalog.productAttributesToPersist { | ||
| try attribute.insert(db) | ||
| } | ||
|
|
@@ -74,7 +89,7 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { | |
| } | ||
| } | ||
|
|
||
| DDLogInfo("✅ Catalog persistence complete") | ||
| DDLogInfo("✅ Catalog persistence complete with inline FTS indexing") | ||
|
|
||
| try await grdbManager.databaseConnection.read { db in | ||
| let productCount = try PersistedProduct.filter { $0.siteID == siteID }.fetchCount(db) | ||
|
|
@@ -83,10 +98,12 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { | |
| let variationCount = try PersistedProductVariation.filter { $0.siteID == siteID }.fetchCount(db) | ||
| let variationImageCount = try PersistedProductVariationImage.filter { $0.siteID == siteID }.fetchCount(db) | ||
| let variationAttributeCount = try PersistedProductVariationAttribute.filter { $0.siteID == siteID }.fetchCount(db) | ||
| let indexCount = try Int.fetchOne(db, sql: "SELECT COUNT(*) FROM pos_search_fts WHERE siteID = ?", arguments: [siteID]) ?? 0 | ||
|
|
||
| DDLogInfo("Persisted \(productCount) products, \(productImageCount) product images, " + | ||
| "\(productAttributeCount) product attributes, \(variationCount) variations, " + | ||
| "\(variationImageCount) variation images, \(variationAttributeCount) variation attributes") | ||
| "\(variationImageCount) variation images, \(variationAttributeCount) variation attributes, " + | ||
| "\(indexCount) FTS entries") | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -95,14 +112,26 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { | |
| "\(catalog.variations.count) updated variations, " + | ||
| "\(catalog.productsToRemove.count) products to remove") | ||
|
|
||
| let productNameLookup = Dictionary(uniqueKeysWithValues: catalog.products.map { ($0.productID, $0.name) }) | ||
| let variationAttributesLookup = Dictionary(grouping: catalog.variationAttributesToPersist) { $0.productVariationID } | ||
|
|
||
| try await grdbManager.databaseConnection.write { db in | ||
| for product in catalog.products { | ||
| // Remove old FTS entry before updating | ||
| try POSSearchIndexBuilder.removeFromIndex(itemID: product.productID, | ||
| itemType: .product, | ||
| siteID: siteID, | ||
| in: db) | ||
|
|
||
| // Delete attributes for updated products, the remaining set will be recreated later in the save | ||
|
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. Retain full comment lower down |
||
| try PersistedProductAttribute | ||
| .filter(PersistedProductAttribute.Columns.productID == product.productID) | ||
| .deleteAll(db) | ||
|
|
||
| try PersistedProduct(from: product).save(db) | ||
| let persistedProduct = PersistedProduct(from: product) | ||
| try persistedProduct.save(db) | ||
|
|
||
| try POSSearchIndexBuilder.indexProduct(persistedProduct, siteID: siteID, in: db) | ||
|
|
||
| // Delete variations that are no longer associated with this product | ||
| let existingVariations = try PersistedProductVariation | ||
|
|
@@ -112,26 +141,43 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { | |
|
|
||
| for variation in existingVariations { | ||
| if !product.variationIDs.contains(variation.id) { | ||
| try POSSearchIndexBuilder.removeFromIndex(itemID: variation.id, | ||
| itemType: .variation, | ||
| siteID: siteID, | ||
| in: db) | ||
| try variation.delete(db) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| for variation in catalog.variationsToPersist { | ||
| // Remove old FTS entry before updating | ||
| try POSSearchIndexBuilder.removeFromIndex(itemID: variation.id, | ||
| itemType: .variation, | ||
| siteID: siteID, | ||
| in: db) | ||
|
|
||
| // Delete attributes for updated variations, the remaining set will be recreated later in the save | ||
|
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. Retain full comment below |
||
| try PersistedProductVariationAttribute | ||
| .filter(PersistedProductVariationAttribute.Columns.productVariationID == variation.id) | ||
| .deleteAll(db) | ||
|
|
||
| try variation.save(db) | ||
|
|
||
| let parentName = productNameLookup[variation.productID] | ||
| let attributes = variationAttributesLookup[variation.id]?.map { $0.option } ?? [] | ||
| try POSSearchIndexBuilder.indexVariation(variation, | ||
| parentProductName: parentName, | ||
| attributes: attributes, | ||
| siteID: siteID, | ||
| in: db) | ||
| } | ||
|
|
||
| // Upsert actual image data (shared by products and variations) | ||
| // Upsert images | ||
| for image in catalog.imagesToPersist { | ||
| try image.save(db) | ||
| } | ||
|
|
||
| // Upsert new join table entries | ||
|
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. Retain |
||
| for image in catalog.productImagesToPersist { | ||
| try image.save(db) | ||
| } | ||
|
|
@@ -140,6 +186,7 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { | |
| try image.save(db) | ||
| } | ||
|
|
||
| // Insert attributes | ||
| for var attribute in catalog.productAttributesToPersist { | ||
| try attribute.insert(db) | ||
| } | ||
|
|
@@ -152,34 +199,32 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { | |
| try site?.updateChanges(db) { $0.lastCatalogIncrementalSyncDate = catalog.syncDate } | ||
| } | ||
|
|
||
| // Delete products hidden from POS when detected during incremental sync | ||
|
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. Retain and add FTS details |
||
| // Variations are not tracked separately, they cascade delete in GRDB when their parent product is removed, | ||
| // so there is no need to pass their IDs here. | ||
| // Delete products hidden from POS (FTS entries removed inline via deleteProducts) | ||
| if !catalog.productsToRemove.isEmpty { | ||
| try await deleteProducts(catalog.productsToRemove, variationIDs: [], siteID: siteID) | ||
| } | ||
|
|
||
| DDLogInfo("✅ Incremental catalog persistence complete") | ||
|
|
||
| try await grdbManager.databaseConnection.read { db in | ||
|
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. retain logging |
||
| let productCount = try PersistedProduct.filter { $0.siteID == siteID }.fetchCount(db) | ||
| let productImageCount = try PersistedProductImage.filter { $0.siteID == siteID }.fetchCount(db) | ||
| let productAttributeCount = try PersistedProductAttribute.filter { $0.siteID == siteID }.fetchCount(db) | ||
| let variationCount = try PersistedProductVariation.filter { $0.siteID == siteID }.fetchCount(db) | ||
| let variationImageCount = try PersistedProductVariationImage.filter { $0.siteID == siteID }.fetchCount(db) | ||
| let variationAttributeCount = try PersistedProductVariationAttribute.filter { $0.siteID == siteID }.fetchCount(db) | ||
|
|
||
| DDLogInfo("Total after incremental update: \(productCount) products, \(productImageCount) product images, " + | ||
| "\(productAttributeCount) product attributes, \(variationCount) variations, " + | ||
| "\(variationImageCount) variation images, \(variationAttributeCount) variation attributes") | ||
| } | ||
| DDLogInfo("✅ Incremental catalog persistence complete with inline FTS updates") | ||
| } | ||
|
|
||
| func deleteProducts(_ productIDs: [Int64], variationIDs: [Int64], siteID: Int64) async throws { | ||
| DDLogInfo("🗑️ Deleting \(productIDs.count) products and \(variationIDs.count) variations from catalog") | ||
|
|
||
| try await grdbManager.databaseConnection.write { db in | ||
| // Batch delete products using filter | ||
| for productID in productIDs { | ||
| try POSSearchIndexBuilder.removeFromIndex(itemID: productID, | ||
| itemType: .product, | ||
| siteID: siteID, | ||
| in: db) | ||
| } | ||
|
|
||
| for variationID in variationIDs { | ||
| try POSSearchIndexBuilder.removeFromIndex(itemID: variationID, | ||
| itemType: .variation, | ||
| siteID: siteID, | ||
| in: db) | ||
| } | ||
|
|
||
| if !productIDs.isEmpty { | ||
| let deletedProductsCount = try PersistedProduct | ||
| .filter(PersistedProduct.Columns.siteID == siteID) | ||
|
|
@@ -188,7 +233,6 @@ final class POSCatalogPersistenceService: POSCatalogPersistenceServiceProtocol { | |
| DDLogInfo("Deleted \(deletedProductsCount) products from catalog") | ||
| } | ||
|
|
||
| // Batch delete variations using filter | ||
| if !variationIDs.isEmpty { | ||
| let deletedVariationsCount = try PersistedProductVariation | ||
| .filter(PersistedProductVariation.Columns.siteID == siteID) | ||
|
|
||
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.
Move these to the previous PR?