From b8936781baf2942e5d3896348a2dfa522e06b815 Mon Sep 17 00:00:00 2001 From: hubab1 <50897577+hubab1@users.noreply.github.com> Date: Sat, 24 Oct 2026 22:31:26 +0100 Subject: [PATCH 1/6] feat(persistence): Compact release databases Archive complete daily rating records in bounded compressed groups. Validate saved payloads before removing source rows, and resume from durable checkpoints after interruptions. Prune unused internal history and compact stores during coordinated release startup. Keep charts, exports, MCP reads, and corrections able to access every historical observation. --- OpenASO.xcodeproj/project.pbxproj | 28 ++ OpenASO/App/AppServices.swift | 2 +- .../AppDetail/AppDetailCSVTransfer.swift | 14 +- .../Table/KeywordRankingListLoader.swift | 21 +- .../Table/KeywordRankingListSheet.swift | 4 + .../AppDetail/Ratings/AppRatingsView.swift | 10 +- .../Ratings/RatingsDashboardSupport.swift | 4 + OpenASO/Models/OpenASOSchemaV1.swift | 7 +- OpenASO/Models/OpenASOSchemaV7.swift | 67 +++++ .../AppDetail/AppDetailRefreshService.swift | 2 +- OpenASO/Services/MCP/OpenASOMCPService.swift | 39 +-- .../Persistence/BackgroundModelStore.swift | 10 + .../Persistence/ModelContainerFactory.swift | 68 ++++- .../PersistentStoreCompactor.swift | 81 ++++++ .../Persistence/RatingHistoryStore.swift | 247 +++++++++++++++++ .../Persistence/StorageSchemaV7Migrator.swift | 246 +++++++++++++++++ .../AppStorefrontRatingService.swift | 34 +-- .../RankingRefreshCoordinator.swift | 20 +- OpenASOTests/AppServicesDependencyTests.swift | 8 +- OpenASOTests/OpenASOMCPServiceTests.swift | 17 +- OpenASOTests/PersistenceMigrationTests.swift | 9 +- .../StorageSchemaV7MigrationTests.swift | 260 ++++++++++++++++++ 22 files changed, 1079 insertions(+), 119 deletions(-) create mode 100644 OpenASO/Models/OpenASOSchemaV7.swift create mode 100644 OpenASO/Services/Persistence/PersistentStoreCompactor.swift create mode 100644 OpenASO/Services/Persistence/RatingHistoryStore.swift create mode 100644 OpenASO/Services/Persistence/StorageSchemaV7Migrator.swift create mode 100644 OpenASOTests/StorageSchemaV7MigrationTests.swift diff --git a/OpenASO.xcodeproj/project.pbxproj b/OpenASO.xcodeproj/project.pbxproj index f25e0f1..de4582f 100644 --- a/OpenASO.xcodeproj/project.pbxproj +++ b/OpenASO.xcodeproj/project.pbxproj @@ -149,12 +149,14 @@ D32A00000000000000000002 /* OpenASOSchemaV5.swift in Sources */ = {isa = PBXBuildFile; fileRef = D32A00000000000000000011 /* OpenASOSchemaV5.swift */; }; D32A00000000000000000003 /* KeywordResearchProject.swift in Sources */ = {isa = PBXBuildFile; fileRef = D32A00000000000000000012 /* KeywordResearchProject.swift */; }; D60000000000000000000001 /* OpenASOSchemaV6.swift in Sources */ = {isa = PBXBuildFile; fileRef = D60000000000000000000011 /* OpenASOSchemaV6.swift */; }; + C20E9443FB12441E82E114E8 /* OpenASOSchemaV7.swift in Sources */ = {isa = PBXBuildFile; fileRef = B96ADC93B6AE4117B2E47D73 /* OpenASOSchemaV7.swift */; }; D60000000000000000000002 /* RankingAppRevision.swift in Sources */ = {isa = PBXBuildFile; fileRef = D60000000000000000000012 /* RankingAppRevision.swift */; }; D60000000000000000000003 /* RankingCrawlRecord.swift in Sources */ = {isa = PBXBuildFile; fileRef = D60000000000000000000013 /* RankingCrawlRecord.swift */; }; D60000000000000000000004 /* RankingFact.swift in Sources */ = {isa = PBXBuildFile; fileRef = D60000000000000000000014 /* RankingFact.swift */; }; D60000000000000000000005 /* TrackedRankingCrawlLink.swift in Sources */ = {isa = PBXBuildFile; fileRef = D60000000000000000000015 /* TrackedRankingCrawlLink.swift */; }; D60000000000000000000006 /* RankingMigrationState.swift in Sources */ = {isa = PBXBuildFile; fileRef = D60000000000000000000016 /* RankingMigrationState.swift */; }; D60000000000000000000021 /* OpenASOSchemaV6.swift in Sources */ = {isa = PBXBuildFile; fileRef = D60000000000000000000011 /* OpenASOSchemaV6.swift */; }; + D2DB6E31771E4C3195A905AC /* OpenASOSchemaV7.swift in Sources */ = {isa = PBXBuildFile; fileRef = B96ADC93B6AE4117B2E47D73 /* OpenASOSchemaV7.swift */; }; D60000000000000000000022 /* RankingAppRevision.swift in Sources */ = {isa = PBXBuildFile; fileRef = D60000000000000000000012 /* RankingAppRevision.swift */; }; D60000000000000000000023 /* RankingCrawlRecord.swift in Sources */ = {isa = PBXBuildFile; fileRef = D60000000000000000000013 /* RankingCrawlRecord.swift */; }; D60000000000000000000024 /* RankingFact.swift in Sources */ = {isa = PBXBuildFile; fileRef = D60000000000000000000014 /* RankingFact.swift */; }; @@ -163,8 +165,15 @@ D60000000000000000000007 /* SearchRanking/RankingAppRevisionStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = D60000000000000000000017 /* SearchRanking/RankingAppRevisionStore.swift */; }; D60000000000000000000027 /* SearchRanking/RankingAppRevisionStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = D60000000000000000000017 /* SearchRanking/RankingAppRevisionStore.swift */; }; D60000000000000000000008 /* Persistence/RankingSchemaV6Migrator.swift in Sources */ = {isa = PBXBuildFile; fileRef = D60000000000000000000018 /* Persistence/RankingSchemaV6Migrator.swift */; }; + 49621A2D2079403C8FB09C1A /* Persistence/PersistentStoreCompactor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9AED285CFBB54DB09AC958F9 /* Persistence/PersistentStoreCompactor.swift */; }; + 0129D26AA92C4ED0B32BCDB8 /* Persistence/StorageSchemaV7Migrator.swift in Sources */ = {isa = PBXBuildFile; fileRef = EB753AAEB38F4C8DA227FFA3 /* Persistence/StorageSchemaV7Migrator.swift */; }; + CD1242318DB74C7DB224A6FF /* Persistence/RatingHistoryStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED8BE61549704D9DA2D9FC76 /* Persistence/RatingHistoryStore.swift */; }; D60000000000000000000028 /* Persistence/RankingSchemaV6Migrator.swift in Sources */ = {isa = PBXBuildFile; fileRef = D60000000000000000000018 /* Persistence/RankingSchemaV6Migrator.swift */; }; + DBF111AE76C4432B9DE83560 /* Persistence/PersistentStoreCompactor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9AED285CFBB54DB09AC958F9 /* Persistence/PersistentStoreCompactor.swift */; }; + 073892B3DFEF4C34B0EBBC39 /* Persistence/StorageSchemaV7Migrator.swift in Sources */ = {isa = PBXBuildFile; fileRef = EB753AAEB38F4C8DA227FFA3 /* Persistence/StorageSchemaV7Migrator.swift */; }; + 9532E6E87A564ABFB26C6A21 /* Persistence/RatingHistoryStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED8BE61549704D9DA2D9FC76 /* Persistence/RatingHistoryStore.swift */; }; D60000000000000000000009 /* RankingSchemaV6MigrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D60000000000000000000019 /* RankingSchemaV6MigrationTests.swift */; }; + 72678FBA68F545FCBBDFA8B3 /* StorageSchemaV7MigrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2A1112B3E0D44EE6B2271949 /* StorageSchemaV7MigrationTests.swift */; }; D32A00000000000000000004 /* KeywordResearchProject.swift in Sources */ = {isa = PBXBuildFile; fileRef = D32A00000000000000000012 /* KeywordResearchProject.swift */; }; D32A00000000000000000005 /* KeywordResearchKeyword.swift in Sources */ = {isa = PBXBuildFile; fileRef = D32A00000000000000000013 /* KeywordResearchKeyword.swift */; }; D32A00000000000000000006 /* KeywordResearchKeyword.swift in Sources */ = {isa = PBXBuildFile; fileRef = D32A00000000000000000013 /* KeywordResearchKeyword.swift */; }; @@ -475,6 +484,7 @@ D32A00000000000000000013 /* KeywordResearchKeyword.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = KeywordResearchKeyword.swift; sourceTree = ""; }; D32A00000000000000000014 /* KeywordResearchPersistenceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = KeywordResearchPersistenceTests.swift; sourceTree = ""; }; D60000000000000000000011 /* OpenASOSchemaV6.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OpenASOSchemaV6.swift; sourceTree = ""; }; + B96ADC93B6AE4117B2E47D73 /* OpenASOSchemaV7.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OpenASOSchemaV7.swift; sourceTree = ""; }; D60000000000000000000012 /* RankingAppRevision.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RankingAppRevision.swift; sourceTree = ""; }; D60000000000000000000013 /* RankingCrawlRecord.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RankingCrawlRecord.swift; sourceTree = ""; }; D60000000000000000000014 /* RankingFact.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RankingFact.swift; sourceTree = ""; }; @@ -482,7 +492,11 @@ D60000000000000000000016 /* RankingMigrationState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RankingMigrationState.swift; sourceTree = ""; }; D60000000000000000000017 /* SearchRanking/RankingAppRevisionStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SearchRanking/RankingAppRevisionStore.swift; sourceTree = ""; }; D60000000000000000000018 /* Persistence/RankingSchemaV6Migrator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Persistence/RankingSchemaV6Migrator.swift; sourceTree = ""; }; + 9AED285CFBB54DB09AC958F9 /* Persistence/PersistentStoreCompactor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Persistence/PersistentStoreCompactor.swift; sourceTree = ""; }; + EB753AAEB38F4C8DA227FFA3 /* Persistence/StorageSchemaV7Migrator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Persistence/StorageSchemaV7Migrator.swift; sourceTree = ""; }; + ED8BE61549704D9DA2D9FC76 /* Persistence/RatingHistoryStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Persistence/RatingHistoryStore.swift; sourceTree = ""; }; D60000000000000000000019 /* RankingSchemaV6MigrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RankingSchemaV6MigrationTests.swift; sourceTree = ""; }; + 2A1112B3E0D44EE6B2271949 /* StorageSchemaV7MigrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StorageSchemaV7MigrationTests.swift; sourceTree = ""; }; D32C00000000000000000011 /* Persistence/KeywordResearchProjectStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Persistence/KeywordResearchProjectStore.swift; sourceTree = ""; }; D32C00000000000000000012 /* KeywordResearchProjectStoreTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = KeywordResearchProjectStoreTests.swift; sourceTree = ""; }; D33A00000000000000000011 /* SearchRanking/KeywordResearchRankingWorkflow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SearchRanking/KeywordResearchRankingWorkflow.swift; sourceTree = ""; }; @@ -671,6 +685,7 @@ D02000000000000000000007 /* ReleasedV1StoreFixture.swift */, D77BC4C40000000000000011 /* ExactV4StoreFixtureTests.swift */, D60000000000000000000019 /* RankingSchemaV6MigrationTests.swift */, + 2A1112B3E0D44EE6B2271949 /* StorageSchemaV7MigrationTests.swift */, D32A00000000000000000014 /* KeywordResearchPersistenceTests.swift */, D32C00000000000000000012 /* KeywordResearchProjectStoreTests.swift */, D33A00000000000000000012 /* KeywordResearchRankingWorkflowTests.swift */, @@ -851,6 +866,9 @@ 26D4B2C87FDED9CFCFA50D7D /* SearchRanking/RankingRefreshCoordinator.swift */, D60000000000000000000017 /* SearchRanking/RankingAppRevisionStore.swift */, D60000000000000000000018 /* Persistence/RankingSchemaV6Migrator.swift */, + 9AED285CFBB54DB09AC958F9 /* Persistence/PersistentStoreCompactor.swift */, + EB753AAEB38F4C8DA227FFA3 /* Persistence/StorageSchemaV7Migrator.swift */, + ED8BE61549704D9DA2D9FC76 /* Persistence/RatingHistoryStore.swift */, D33A00000000000000000011 /* SearchRanking/KeywordResearchRankingWorkflow.swift */, D34A00000000000000000011 /* SearchRanking/KeywordResearchHistoryReader.swift */, A70000000000000000000002 /* RatingsReviews/ReviewLanguageDetectionService.swift */, @@ -1044,6 +1062,7 @@ D20B00000000000000000011 /* OpenASOSchemaV4.swift */, D32A00000000000000000011 /* OpenASOSchemaV5.swift */, D60000000000000000000011 /* OpenASOSchemaV6.swift */, + B96ADC93B6AE4117B2E47D73 /* OpenASOSchemaV7.swift */, D60000000000000000000012 /* RankingAppRevision.swift */, D60000000000000000000013 /* RankingCrawlRecord.swift */, D60000000000000000000014 /* RankingFact.swift */, @@ -1334,6 +1353,7 @@ D20B00000000000000000001 /* OpenASOSchemaV4.swift in Sources */, D32A00000000000000000001 /* OpenASOSchemaV5.swift in Sources */, D60000000000000000000001 /* OpenASOSchemaV6.swift in Sources */, + C20E9443FB12441E82E114E8 /* OpenASOSchemaV7.swift in Sources */, D60000000000000000000002 /* RankingAppRevision.swift in Sources */, D60000000000000000000003 /* RankingCrawlRecord.swift in Sources */, D60000000000000000000004 /* RankingFact.swift in Sources */, @@ -1341,6 +1361,9 @@ D60000000000000000000006 /* RankingMigrationState.swift in Sources */, D60000000000000000000007 /* SearchRanking/RankingAppRevisionStore.swift in Sources */, D60000000000000000000008 /* Persistence/RankingSchemaV6Migrator.swift in Sources */, + 49621A2D2079403C8FB09C1A /* Persistence/PersistentStoreCompactor.swift in Sources */, + 0129D26AA92C4ED0B32BCDB8 /* Persistence/StorageSchemaV7Migrator.swift in Sources */, + CD1242318DB74C7DB224A6FF /* Persistence/RatingHistoryStore.swift in Sources */, D32A00000000000000000003 /* KeywordResearchProject.swift in Sources */, D32A00000000000000000005 /* KeywordResearchKeyword.swift in Sources */, D20B00000000000000000003 /* EstimatedKeywordDifficultyMetric.swift in Sources */, @@ -1476,6 +1499,7 @@ D35A00000000000000000003 /* HeadlessRefreshServiceTests.swift in Sources */, D35D00000000000000000003 /* HeadlessRefreshObservationTests.swift in Sources */, D60000000000000000000009 /* RankingSchemaV6MigrationTests.swift in Sources */, + 72678FBA68F545FCBBDFA8B3 /* StorageSchemaV7MigrationTests.swift in Sources */, 0A5C00000000000000000005 /* AppStoreConnectReviewServiceTests.swift in Sources */, 0A5C00000000000000000007 /* AppStoreWebMetadataProviderTests.swift in Sources */, A19000000000000000000003 /* AppStoreWebRankingProviderTests.swift in Sources */, @@ -1526,6 +1550,7 @@ D20B00000000000000000002 /* OpenASOSchemaV4.swift in Sources */, D32A00000000000000000002 /* OpenASOSchemaV5.swift in Sources */, D60000000000000000000021 /* OpenASOSchemaV6.swift in Sources */, + D2DB6E31771E4C3195A905AC /* OpenASOSchemaV7.swift in Sources */, D60000000000000000000022 /* RankingAppRevision.swift in Sources */, D60000000000000000000023 /* RankingCrawlRecord.swift in Sources */, D60000000000000000000024 /* RankingFact.swift in Sources */, @@ -1533,6 +1558,9 @@ D60000000000000000000026 /* RankingMigrationState.swift in Sources */, D60000000000000000000027 /* SearchRanking/RankingAppRevisionStore.swift in Sources */, D60000000000000000000028 /* Persistence/RankingSchemaV6Migrator.swift in Sources */, + DBF111AE76C4432B9DE83560 /* Persistence/PersistentStoreCompactor.swift in Sources */, + 073892B3DFEF4C34B0EBBC39 /* Persistence/StorageSchemaV7Migrator.swift in Sources */, + 9532E6E87A564ABFB26C6A21 /* Persistence/RatingHistoryStore.swift in Sources */, D32A00000000000000000004 /* KeywordResearchProject.swift in Sources */, D32A00000000000000000006 /* KeywordResearchKeyword.swift in Sources */, D20B00000000000000000004 /* EstimatedKeywordDifficultyMetric.swift in Sources */, diff --git a/OpenASO/App/AppServices.swift b/OpenASO/App/AppServices.swift index 9f42824..3aaedb5 100644 --- a/OpenASO/App/AppServices.swift +++ b/OpenASO/App/AppServices.swift @@ -268,7 +268,7 @@ final class AppServices { observedAt: .now, source: .appStorePage ) - appStorefrontRatingService.persist( + try appStorefrontRatingService.persist( AppStorefrontRatingRefreshOutcome( storefront: request.storefront, result: result, diff --git a/OpenASO/Features/AppDetail/AppDetailCSVTransfer.swift b/OpenASO/Features/AppDetail/AppDetailCSVTransfer.swift index fb59437..4f6199e 100644 --- a/OpenASO/Features/AppDetail/AppDetailCSVTransfer.swift +++ b/OpenASO/Features/AppDetail/AppDetailCSVTransfer.swift @@ -50,6 +50,10 @@ struct AppDetailRatingSnapshotValue: Sendable { let observedAt: Date init(_ snapshot: AppDailyRating) { + self.init(RatingHistoryRecord(snapshot)) + } + + init(_ snapshot: RatingHistoryRecord) { self.appStoreID = snapshot.appStoreID self.storefront = snapshot.storefront self.ratingDate = snapshot.ratingDate @@ -187,17 +191,11 @@ struct AppDetailRatingsCSVExporter { }, sortBy: [SortDescriptor(\.storefront, order: .forward)] ) - var snapshotDescriptor = FetchDescriptor( - predicate: #Predicate { snapshot in - snapshot.appStoreID == targetAppStoreID - }, - sortBy: [SortDescriptor(\.observedAt, order: .reverse)] - ) - snapshotDescriptor.fetchLimit = 1_000 + let snapshots = try RatingHistoryStore.fetch(appStoreIDs: [appStoreID], in: modelContext) return AppDetailRatingsExportData( latestRatings: try modelContext.fetch(latestDescriptor).map(AppDetailRatingLatestValue.init), - ratingSnapshots: try modelContext.fetch(snapshotDescriptor).map(AppDetailRatingSnapshotValue.init) + ratingSnapshots: snapshots.prefix(1_000).map(AppDetailRatingSnapshotValue.init) ) } diff --git a/OpenASO/Features/AppDetail/Keywords/Table/KeywordRankingListLoader.swift b/OpenASO/Features/AppDetail/Keywords/Table/KeywordRankingListLoader.swift index 1317a71..e5c1d48 100644 --- a/OpenASO/Features/AppDetail/Keywords/Table/KeywordRankingListLoader.swift +++ b/OpenASO/Features/AppDetail/Keywords/Table/KeywordRankingListLoader.swift @@ -239,19 +239,14 @@ enum KeywordRankingListLoader { snapshotsByID.reserveCapacity(uniqueAppStoreIDs.count) guard !uniqueAppStoreIDs.isEmpty else { return [:] } - let descriptor = FetchDescriptor( - predicate: #Predicate { snapshot in - snapshot.storefront == targetStorefront - && uniqueAppStoreIDs.contains(snapshot.appStoreID) - && snapshot.ratingCount != nil - }, - sortBy: [ - SortDescriptor(\.appStoreID, order: .forward), - SortDescriptor(\.ratingDate, order: .reverse), - SortDescriptor(\.observedAt, order: .reverse) - ] - ) - for (index, snapshot) in try modelContext.fetch(descriptor).enumerated() { + let snapshots = try RatingHistoryStore.fetch( + appStoreIDs: uniqueAppStoreIDs, storefronts: [targetStorefront], in: modelContext + ).filter { $0.ratingCount != nil }.sorted { + if $0.appStoreID != $1.appStoreID { return $0.appStoreID < $1.appStoreID } + if $0.ratingDate != $1.ratingDate { return $0.ratingDate > $1.ratingDate } + return $0.observedAt > $1.observedAt + } + for (index, snapshot) in snapshots.enumerated() { if snapshotsByID[snapshot.appStoreID, default: []].count < ratingSnapshotLimit { snapshotsByID[snapshot.appStoreID, default: []].append( RatingSnapshotDisplayValue(snapshot) diff --git a/OpenASO/Features/AppDetail/Keywords/Table/KeywordRankingListSheet.swift b/OpenASO/Features/AppDetail/Keywords/Table/KeywordRankingListSheet.swift index 5329475..db38505 100644 --- a/OpenASO/Features/AppDetail/Keywords/Table/KeywordRankingListSheet.swift +++ b/OpenASO/Features/AppDetail/Keywords/Table/KeywordRankingListSheet.swift @@ -760,6 +760,10 @@ struct RatingSnapshotDisplayValue: Sendable { let observedAt: Date init(_ snapshot: AppDailyRating) { + self.init(RatingHistoryRecord(snapshot)) + } + + init(_ snapshot: RatingHistoryRecord) { self.appStoreID = snapshot.appStoreID self.ratingDate = snapshot.ratingDate self.ratingCount = snapshot.ratingCount diff --git a/OpenASO/Features/AppDetail/Ratings/AppRatingsView.swift b/OpenASO/Features/AppDetail/Ratings/AppRatingsView.swift index 2501c71..ac2c1d0 100644 --- a/OpenASO/Features/AppDetail/Ratings/AppRatingsView.swift +++ b/OpenASO/Features/AppDetail/Ratings/AppRatingsView.swift @@ -159,15 +159,9 @@ struct AppRatingsView: View { }, sortBy: [SortDescriptor(\.storefront, order: .forward)] ) - let snapshotDescriptor = FetchDescriptor( - predicate: #Predicate { snapshot in - snapshot.appStoreID == targetAppStoreID - }, - sortBy: [SortDescriptor(\.observedAt, order: .reverse)] - ) - let latestRatings = try modelContext.fetch(latestDescriptor).map(RatingLatestValue.init) - let ratingSnapshots = try modelContext.fetch(snapshotDescriptor).map(RatingSnapshotValue.init) + let ratingSnapshots = try RatingHistoryStore.fetch(appStoreIDs: [appStoreID], in: modelContext) + .map(RatingSnapshotValue.init) return RatingsDashboardData( latestRatings: latestRatings, diff --git a/OpenASO/Features/AppDetail/Ratings/RatingsDashboardSupport.swift b/OpenASO/Features/AppDetail/Ratings/RatingsDashboardSupport.swift index 71ec482..a62f69b 100644 --- a/OpenASO/Features/AppDetail/Ratings/RatingsDashboardSupport.swift +++ b/OpenASO/Features/AppDetail/Ratings/RatingsDashboardSupport.swift @@ -70,6 +70,10 @@ struct RatingSnapshotValue: Identifiable, Hashable, Sendable { var id: String { identityKey } init(_ snapshot: AppDailyRating) { + self.init(RatingHistoryRecord(snapshot)) + } + + init(_ snapshot: RatingHistoryRecord) { self.identityKey = snapshot.identityKey self.appStoreID = snapshot.appStoreID self.storefront = snapshot.storefront diff --git a/OpenASO/Models/OpenASOSchemaV1.swift b/OpenASO/Models/OpenASOSchemaV1.swift index 927322a..8704297 100644 --- a/OpenASO/Models/OpenASOSchemaV1.swift +++ b/OpenASO/Models/OpenASOSchemaV1.swift @@ -42,7 +42,8 @@ enum OpenASOMigrationPlan: SchemaMigrationPlan { OpenASOSchemaV3.self, OpenASOSchemaV4.self, OpenASOSchemaV5.self, - OpenASOSchemaV6.self + OpenASOSchemaV6.self, + OpenASOSchemaV7.self ] } @@ -73,6 +74,10 @@ enum OpenASOMigrationPlan: SchemaMigrationPlan { .lightweight( fromVersion: OpenASOSchemaV5.self, toVersion: OpenASOSchemaV6.self + ), + .lightweight( + fromVersion: OpenASOSchemaV6.self, + toVersion: OpenASOSchemaV7.self ) ] } diff --git a/OpenASO/Models/OpenASOSchemaV7.swift b/OpenASO/Models/OpenASOSchemaV7.swift new file mode 100644 index 0000000..0e8fe39 --- /dev/null +++ b/OpenASO/Models/OpenASOSchemaV7.swift @@ -0,0 +1,67 @@ +import Foundation +import SwiftData + +// V1–V6 remain frozen. Keep their rating entity for current writes and add +// lossless, independently readable archives for older daily observations. +enum OpenASOSchemaV7: VersionedSchema { + static var versionIdentifier: Schema.Version { Schema.Version(7, 0, 0) } + + static var models: [any PersistentModel.Type] { + OpenASOSchemaV6.models + [RatingHistoryChunk.self, StorageMigrationState.self] + } + + @Model + final class RatingHistoryChunk { + #Index([\.appStoreID], [\.appStoreID, \.storefront]) + @Attribute(.unique) var identityKey: String + var appStoreID: Int64 + var storefront: String + var year: String + var firstObservedAt: Date + var lastObservedAt: Date + var recordCount: Int + var formatVersion: Int + var uncompressedByteCount: Int + var digest: Data + var payload: Data + + init(appStoreID: Int64, storefront: String, year: String) { + self.identityKey = Self.key(appStoreID: appStoreID, storefront: storefront, year: year) + self.appStoreID = appStoreID + self.storefront = storefront + self.year = year + self.firstObservedAt = .distantFuture + self.lastObservedAt = .distantPast + self.recordCount = 0 + self.formatVersion = 1 + self.uncompressedByteCount = 0 + self.digest = Data() + self.payload = Data() + } + + static func key(appStoreID: Int64, storefront: String, year: String) -> String { + "\(appStoreID)::\(storefront)::\(year)" + } + } + + @Model + final class StorageMigrationState { + @Attribute(.unique) var migrationKey: String + var lastIdentityKey: String? + var pendingDeletionPayload: Data? + var archivedRecordCount: Int + var completed: Bool + var needsCompaction: Bool + var lastMaintenanceAt: Date? + + init() { + self.migrationKey = "storage-v7" + self.archivedRecordCount = 0 + self.completed = false + self.needsCompaction = true + } + } +} + +typealias RatingHistoryChunk = OpenASOSchemaV7.RatingHistoryChunk +typealias StorageMigrationState = OpenASOSchemaV7.StorageMigrationState diff --git a/OpenASO/Services/AppDetail/AppDetailRefreshService.swift b/OpenASO/Services/AppDetail/AppDetailRefreshService.swift index db2a8c8..c34266e 100644 --- a/OpenASO/Services/AppDetail/AppDetailRefreshService.swift +++ b/OpenASO/Services/AppDetail/AppDetailRefreshService.swift @@ -1032,7 +1032,7 @@ final class AppDetailRefreshService: Sendable { try Task.checkCancellation() let storeApp = try storeApp(for: app, in: modelContext) for outcome in outcomes { - appStorefrontRatingService.persist(outcome, for: storeApp, in: modelContext) + try appStorefrontRatingService.persist(outcome, for: storeApp, in: modelContext) } } try Task.checkCancellation() diff --git a/OpenASO/Services/MCP/OpenASOMCPService.swift b/OpenASO/Services/MCP/OpenASOMCPService.swift index 5bc41ab..fe4523f 100644 --- a/OpenASO/Services/MCP/OpenASOMCPService.swift +++ b/OpenASO/Services/MCP/OpenASOMCPService.swift @@ -1357,35 +1357,16 @@ final class OpenASOMCPService: Sendable { let maximumDate = dateTo ?? .distantFuture let storefrontValues = Array(storefronts) let filtersStorefront = !storefrontValues.isEmpty - let countPredicate = #Predicate { rating in - rating.appStoreID == appStoreID - && (!filtersStorefront || storefrontValues.contains(rating.storefront)) - && rating.observedAt >= minimumDate - && rating.observedAt <= maximumDate - } - let total = try modelContext.fetchCount( - FetchDescriptor(predicate: countPredicate) - ) - let hasCursor = historyCursor != nil - let cursorTimestamp = historyCursor?.timestamp ?? .distantFuture - let cursorTieKey = historyCursor?.tieKey ?? "" - var descriptor = FetchDescriptor( - predicate: #Predicate { rating in - rating.appStoreID == appStoreID - && (!filtersStorefront || storefrontValues.contains(rating.storefront)) - && rating.observedAt >= minimumDate - && rating.observedAt <= maximumDate - && (!hasCursor - || rating.observedAt < cursorTimestamp - || (rating.observedAt == cursorTimestamp && rating.identityKey > cursorTieKey)) - }, - sortBy: [ - SortDescriptor(\.observedAt, order: .reverse), - SortDescriptor(\.identityKey, comparator: .lexical, order: .forward), - ] + let allRatings = try RatingHistoryStore.fetch( + appStoreIDs: [appStoreID], storefronts: storefrontValues, + from: minimumDate, to: maximumDate, in: modelContext ) - descriptor.fetchLimit = page.limit + 1 - let ratingsWithLookahead = try modelContext.fetch(descriptor) + let total = allRatings.count + let ratingsWithLookahead = Array(allRatings.filter { rating in + guard let historyCursor else { return true } + return rating.observedAt < historyCursor.timestamp + || (rating.observedAt == historyCursor.timestamp && rating.identityKey > historyCursor.tieKey) + }.prefix(page.limit + 1)) let pageSlice = try Self.historyPageFromLookahead( rowsWithLookahead: ratingsWithLookahead, kind: .ratings, @@ -4363,7 +4344,7 @@ extension OpenASOMCPService { ) } - fileprivate static func ratingSnapshot(_ rating: AppDailyRating) -> OpenASOMCPRatingSnapshot { + fileprivate static func ratingSnapshot(_ rating: RatingHistoryRecord) -> OpenASOMCPRatingSnapshot { OpenASOMCPRatingSnapshot( id: rating.identityKey, identityKey: rating.identityKey, diff --git a/OpenASO/Services/Persistence/BackgroundModelStore.swift b/OpenASO/Services/Persistence/BackgroundModelStore.swift index fe36dc3..acf0a5d 100644 --- a/OpenASO/Services/Persistence/BackgroundModelStore.swift +++ b/OpenASO/Services/Persistence/BackgroundModelStore.swift @@ -1,4 +1,5 @@ import Dispatch +import Foundation import SwiftData @ModelActor @@ -56,6 +57,7 @@ private actor BackgroundModelContextWorker { ) private let modelContainer: ModelContainer private var context: ModelContext? + private var lastHistoryPruneAt: Date? nonisolated var unownedExecutor: UnownedSerialExecutor { executor.asUnownedSerialExecutor() @@ -91,6 +93,14 @@ private actor BackgroundModelContextWorker { do { let value = try operation(modelContext) try modelContext.save() + let now = Date.now + if now.timeIntervalSince(lastHistoryPruneAt ?? .distantPast) >= 3_600 { + lastHistoryPruneAt = now + do { try PersistentHistoryMaintenance.prune(in: modelContext) } + catch { + OpenASOLog.rankingMigration.error("Internal history maintenance failed: \(String(reflecting: error), privacy: .public)") + } + } return value } catch { modelContext.rollback() diff --git a/OpenASO/Services/Persistence/ModelContainerFactory.swift b/OpenASO/Services/Persistence/ModelContainerFactory.swift index 4bc8a7d..1d32eb3 100644 --- a/OpenASO/Services/Persistence/ModelContainerFactory.swift +++ b/OpenASO/Services/Persistence/ModelContainerFactory.swift @@ -1,6 +1,7 @@ import CryptoKit import Darwin import Foundation +import ObjectiveC import SwiftData enum PersistentStoreError: LocalizedError, Sendable { @@ -95,9 +96,33 @@ enum ModelContainerFactory { let schema = Self.schema let configuration = ModelConfiguration(schema: schema, url: storeURL) do { - return try withExclusiveMigrationLock(for: storeURL) { + return try withMigrationAccess(for: storeURL) { canMigrate in + guard canMigrate else { return try opener(schema, configuration) } + let prepared: ModelContainer? = try autoreleasepool { + let container = try opener(schema, configuration) + try RankingSchemaV6Migrator.migrateIfNeeded(in: container) + let context = ModelContext(container) + let prior = try context.fetch(FetchDescriptor()).first + let needsMaintenance = prior?.needsCompaction != false || (prior?.lastMaintenanceAt.map { + Date.now.timeIntervalSince($0) >= 86_400 + } ?? true) + guard needsMaintenance else { return container } + // No app consumer persists SwiftData history tokens. Research + // observations live in model entities, not this change log. + try PersistentHistoryMaintenance.prune(in: context) + return nil + } + if let prepared { return prepared } + try StorageSchemaV7Migrator.migrateIfNeeded(at: storeURL) + if try PersistentStoreCompactor.shouldCompact(at: storeURL) { + try PersistentStoreCompactor.compact(at: storeURL) + } let container = try opener(schema, configuration) - try RankingSchemaV6Migrator.migrateIfNeeded(in: container) + let context = ModelContext(container) + if let state = try context.fetch(FetchDescriptor()).first { + state.needsCompaction = false + try context.save() + } return container } } catch { @@ -126,10 +151,10 @@ enum ModelContainerFactory { ) } - private static func withExclusiveMigrationLock( + private static func withMigrationAccess( for storeURL: URL, - _ operation: () throws -> Value - ) throws -> Value { + _ operation: (Bool) throws -> ModelContainer + ) throws -> ModelContainer { let lockDirectory = FileManager.default.temporaryDirectory .appendingPathComponent("OpenASO-Migrations", isDirectory: true) try FileManager.default.createDirectory( @@ -146,14 +171,33 @@ enum ModelContainerFactory { guard descriptor >= 0 else { throw CocoaError(.fileWriteUnknown) } - defer { Darwin.close(descriptor) } - - while flock(descriptor, LOCK_EX) != 0 { - guard errno == EINTR else { - throw CocoaError(.fileLocking) + let lease = PersistentStoreLease(descriptor: descriptor) + let exclusive = flock(descriptor, LOCK_EX | LOCK_NB) == 0 + if !exclusive { + guard errno == EWOULDBLOCK || errno == EAGAIN else { throw CocoaError(.fileLocking) } + while flock(descriptor, LOCK_SH) != 0 { + guard errno == EINTR else { throw CocoaError(.fileLocking) } } + } else if FileManager.default.fileExists(atPath: storeURL.path) { + try PersistentStoreCompactor.requireNoOlderClients(at: storeURL) } - defer { flock(descriptor, LOCK_UN) } - return try operation() + let container = try operation(exclusive) + if exclusive, flock(descriptor, LOCK_SH) != 0 { throw CocoaError(.fileLocking) } + // Hold shared access for the container's lifetime. Another app, MCP, + // or refresh process can read/write, but cannot compact a live store. + let key = UnsafeRawPointer(bitPattern: UInt(bitPattern: ObjectIdentifier(PersistentStoreLease.self)))! + objc_setAssociatedObject(container, key, lease, .OBJC_ASSOCIATION_RETAIN_NONATOMIC) + return container + } +} + +private final class PersistentStoreLease: Sendable { + let descriptor: Int32 + + init(descriptor: Int32) { self.descriptor = descriptor } + + deinit { + flock(descriptor, LOCK_UN) + Darwin.close(descriptor) } } diff --git a/OpenASO/Services/Persistence/PersistentStoreCompactor.swift b/OpenASO/Services/Persistence/PersistentStoreCompactor.swift new file mode 100644 index 0000000..493d880 --- /dev/null +++ b/OpenASO/Services/Persistence/PersistentStoreCompactor.swift @@ -0,0 +1,81 @@ +import CoreData +import Foundation +import SQLite3 +import SwiftData + +enum PersistentStoreCompactor { + static func shouldCompact(at storeURL: URL) throws -> Bool { + var database: OpaquePointer? + guard sqlite3_open_v2(storeURL.path, &database, SQLITE_OPEN_READONLY, nil) == SQLITE_OK else { + if let database { sqlite3_close(database) } + throw CocoaError(.fileReadUnknown) + } + defer { sqlite3_close(database) } + func pragma(_ name: String) throws -> Int64 { + var statement: OpaquePointer? + guard sqlite3_prepare_v2(database, "PRAGMA \(name)", -1, &statement, nil) == SQLITE_OK else { + throw CocoaError(.fileReadUnknown) + } + defer { sqlite3_finalize(statement) } + guard sqlite3_step(statement) == SQLITE_ROW else { throw CocoaError(.fileReadUnknown) } + return sqlite3_column_int64(statement, 0) + } + let freePages = try pragma("freelist_count") + let pageCount = try pragma("page_count") + let freeBytes = try freePages * pragma("page_size") + // Avoid a full-file rewrite on daily launches when pages can be reused. + return freeBytes >= 64 * 1_024 * 1_024 + || (freeBytes >= 8 * 1_024 * 1_024 && freePages >= pageCount / 5) + } + + // Older releases do not take the new lifetime lease. Refuse an upgrade + // while any other process still has this exact store open. + static func requireNoOlderClients(at storeURL: URL) throws { + let process = Process() + let output = Pipe() + process.executableURL = URL(fileURLWithPath: "/usr/sbin/lsof") + process.arguments = ["-nP", "-F", "p", "--", storeURL.path] + process.standardOutput = output + process.standardError = FileHandle.nullDevice + try process.run() + let data = output.fileHandleForReading.readDataToEndOfFile() + process.waitUntilExit() + guard process.terminationStatus == 0 || process.terminationStatus == 1 else { + throw CocoaError(.fileLocking) + } + let ownPID = ProcessInfo.processInfo.processIdentifier + let otherPIDs = String(decoding: data, as: UTF8.self).split(separator: "\n") + .compactMap { $0.first == "p" ? Int32($0.dropFirst()) : nil } + .filter { $0 != ownPID } + guard otherPIDs.isEmpty else { + throw NSError(domain: "OpenASO.StorageMigration", code: 1, userInfo: [ + NSLocalizedDescriptionKey: "Quit other OpenASO, refresh, or MCP processes before upgrading this database." + ]) + } + } + + /// Use Core Data's supported SQLite maintenance option. Never edit its + /// private history tables or replace a live SQLite file behind a container. + static func compact(at storeURL: URL) throws { + try autoreleasepool { + guard let model = NSManagedObjectModel.makeManagedObjectModel( + for: OpenASOMigrationPlan.currentSchema.models + ) else { throw CocoaError(.persistentStoreInvalidType) } + let coordinator = NSPersistentStoreCoordinator(managedObjectModel: model) + let store = try coordinator.addPersistentStore( + type: .sqlite, at: storeURL, + options: [NSSQLiteManualVacuumOption: true, NSPersistentHistoryTrackingKey: true, + NSPersistentStoreTimeoutOption: 5] + ) + try coordinator.remove(store) + } + } +} + +// Research observations are ordinary models. This log is only SwiftData's +// internal change stream; no app consumer currently stores history tokens. +enum PersistentHistoryMaintenance { + static func prune(in context: ModelContext) throws { + try context.deleteHistory(HistoryDescriptor()) + } +} diff --git a/OpenASO/Services/Persistence/RatingHistoryStore.swift b/OpenASO/Services/Persistence/RatingHistoryStore.swift new file mode 100644 index 0000000..21e20a4 --- /dev/null +++ b/OpenASO/Services/Persistence/RatingHistoryStore.swift @@ -0,0 +1,247 @@ +import CryptoKit +import Foundation +import SwiftData + +/// The logical daily record is unchanged by archival. Keep the logical date +/// separate from observedAt, including imported dates and source evidence. +struct RatingHistoryRecord: Codable, Equatable, Sendable { + var identityKey: String + var appStoreID: Int64 + var storefront: String + var ratingDate: String + var ratingCount: Int? + var averageRating: Double? + var oneStarRatingCount: Int? + var twoStarRatingCount: Int? + var threeStarRatingCount: Int? + var fourStarRatingCount: Int? + var fiveStarRatingCount: Int? + var observedAt: Date + var submissionCount: Int + var winningCount: Int + var confidenceRaw: String? + var sourceRaw: String + var storeAppID: Int64? + + var source: AppStorefrontSource { AppStorefrontSource(rawValue: sourceRaw) ?? .appStorePage } + var chunkKey: String { + RatingHistoryChunk.key(appStoreID: appStoreID, storefront: storefront, year: String(ratingDate.prefix(4))) + } + + init(_ value: AppDailyRating) { + identityKey = value.identityKey + appStoreID = value.appStoreID + storefront = value.storefront + ratingDate = value.ratingDate + ratingCount = value.ratingCount + averageRating = value.averageRating + oneStarRatingCount = value.oneStarRatingCount + twoStarRatingCount = value.twoStarRatingCount + threeStarRatingCount = value.threeStarRatingCount + fourStarRatingCount = value.fourStarRatingCount + fiveStarRatingCount = value.fiveStarRatingCount + observedAt = value.observedAt + submissionCount = value.submissionCount + winningCount = value.winningCount + confidenceRaw = value.confidenceRaw + sourceRaw = value.sourceRaw + storeAppID = value.storeApp?.appStoreID + } + + // A versioned positional payload avoids repeating property names and + // binary-plist offset tables in each small archive. Nulls keep their slots. + init(from decoder: any Decoder) throws { + var fields = try decoder.unkeyedContainer() + identityKey = try fields.decode(String.self) + appStoreID = try fields.decode(Int64.self) + storefront = try fields.decode(String.self) + ratingDate = try fields.decode(String.self) + ratingCount = try fields.decode(Int?.self) + averageRating = try fields.decode(Double?.self) + oneStarRatingCount = try fields.decode(Int?.self) + twoStarRatingCount = try fields.decode(Int?.self) + threeStarRatingCount = try fields.decode(Int?.self) + fourStarRatingCount = try fields.decode(Int?.self) + fiveStarRatingCount = try fields.decode(Int?.self) + observedAt = try fields.decode(Date.self) + submissionCount = try fields.decode(Int.self) + winningCount = try fields.decode(Int.self) + confidenceRaw = try fields.decode(String?.self) + sourceRaw = try fields.decode(String.self) + storeAppID = try fields.decode(Int64?.self) + guard fields.isAtEnd else { + throw DecodingError.dataCorruptedError(in: fields, debugDescription: "Unknown rating history fields") + } + } + + func encode(to encoder: any Encoder) throws { + var fields = encoder.unkeyedContainer() + try fields.encode(identityKey) + try fields.encode(appStoreID) + try fields.encode(storefront) + try fields.encode(ratingDate) + try fields.encode(ratingCount) + try fields.encode(averageRating) + try fields.encode(oneStarRatingCount) + try fields.encode(twoStarRatingCount) + try fields.encode(threeStarRatingCount) + try fields.encode(fourStarRatingCount) + try fields.encode(fiveStarRatingCount) + try fields.encode(observedAt) + try fields.encode(submissionCount) + try fields.encode(winningCount) + try fields.encode(confidenceRaw) + try fields.encode(sourceRaw) + try fields.encode(storeAppID) + } + + func makeModel(storeApp: StoreApp?) -> AppDailyRating { + let model = AppDailyRating( + appStoreID: appStoreID, storefront: storefront, + ratingCount: ratingCount, averageRating: averageRating, + oneStarRatingCount: oneStarRatingCount, twoStarRatingCount: twoStarRatingCount, + threeStarRatingCount: threeStarRatingCount, fourStarRatingCount: fourStarRatingCount, + fiveStarRatingCount: fiveStarRatingCount, ratingDate: ratingDate, + observedAt: observedAt, submissionCount: submissionCount, + winningCount: winningCount, confidence: confidenceRaw, source: source, + storeApp: storeApp + ) + model.identityKey = identityKey + model.sourceRaw = sourceRaw + return model + } +} + +enum RatingHistoryError: LocalizedError { + case invalidArchive(String) + + var errorDescription: String? { + switch self { + case .invalidArchive(let key): "Rating history failed validation: \(key). No unvalidated source rows were removed." + } + } +} + +enum RatingHistoryStore { + static func records(in chunk: RatingHistoryChunk) throws -> [RatingHistoryRecord] { + guard chunk.formatVersion == 1, chunk.uncompressedByteCount > 0, + chunk.uncompressedByteCount <= 8 * 1_024 * 1_024 else { + throw RatingHistoryError.invalidArchive(chunk.identityKey) + } + let data = try (chunk.payload as NSData).decompressed(using: .zlib) as Data + guard data.count == chunk.uncompressedByteCount, + Data(SHA256.hash(data: data)) == chunk.digest else { + throw RatingHistoryError.invalidArchive(chunk.identityKey) + } + let values = try JSONDecoder().decode([RatingHistoryRecord].self, from: data) + guard values.count == chunk.recordCount, + Set(values.map(\.identityKey)).count == values.count, + values.allSatisfy({ $0.chunkKey == chunk.identityKey }), + values.map(\.observedAt).min() == chunk.firstObservedAt, + values.map(\.observedAt).max() == chunk.lastObservedAt else { + throw RatingHistoryError.invalidArchive(chunk.identityKey) + } + return values + } + + static func replace(_ records: [RatingHistoryRecord], in chunk: RatingHistoryChunk) throws { + let sorted = records.sorted { $0.identityKey < $1.identityKey } + guard !sorted.isEmpty, sorted.count <= 512, + Set(sorted.map(\.identityKey)).count == sorted.count, + sorted.allSatisfy({ $0.chunkKey == chunk.identityKey }) else { + throw RatingHistoryError.invalidArchive(chunk.identityKey) + } + let data = try JSONEncoder().encode(sorted) + let compressed = try (data as NSData).compressed(using: .zlib) as Data + guard try JSONDecoder().decode([RatingHistoryRecord].self, + from: (compressed as NSData).decompressed(using: .zlib) as Data) == sorted else { + throw RatingHistoryError.invalidArchive(chunk.identityKey) + } + chunk.payload = compressed + chunk.digest = Data(SHA256.hash(data: data)) + chunk.uncompressedByteCount = data.count + chunk.recordCount = sorted.count + chunk.firstObservedAt = sorted.map(\.observedAt).min()! + chunk.lastObservedAt = sorted.map(\.observedAt).max()! + } + + static func fetch( + appStoreIDs: [Int64], storefronts: [String] = [], + from: Date = .distantPast, to: Date = .distantFuture, + in context: ModelContext + ) throws -> [RatingHistoryRecord] { + guard !appStoreIDs.isEmpty else { return [] } + let filtersStorefront = !storefronts.isEmpty + let legacy = try context.fetch(FetchDescriptor(predicate: #Predicate { + appStoreIDs.contains($0.appStoreID) + && (!filtersStorefront || storefronts.contains($0.storefront)) + + })) + let chunks = try context.fetch(FetchDescriptor(predicate: #Predicate { + appStoreIDs.contains($0.appStoreID) + && (!filtersStorefront || storefronts.contains($0.storefront)) + && $0.firstObservedAt <= to && $0.lastObservedAt >= from + })) + var byKey: [String: RatingHistoryRecord] = [:] + for chunk in chunks { + for record in try records(in: chunk) where record.observedAt >= from && record.observedAt <= to { + byKey[record.identityKey] = record + } + } + for record in legacy.map(RatingHistoryRecord.init) { + if let archived = byKey[record.identityKey], archived.observedAt > record.observedAt { continue } + byKey[record.identityKey] = record + } + return byKey.values.filter { $0.observedAt >= from && $0.observedAt <= to }.sorted { + $0.observedAt == $1.observedAt ? $0.identityKey < $1.identityKey : $0.observedAt > $1.observedAt + } + } + + /// Restore an archived day before an existing write path changes it. Both + /// removal from the chunk and the new row commit with the caller's save. + static func restore(identityKey: String, in context: ModelContext) throws -> AppDailyRating? { + try restore(identityKeys: [identityKey], in: context)[identityKey] + } + + static func restore(identityKeys: [String], in context: ModelContext) throws -> [String: AppDailyRating] { + guard !identityKeys.isEmpty else { return [:] } + let existing = try context.fetch(FetchDescriptor(predicate: #Predicate { + identityKeys.contains($0.identityKey) + })) + var models = Dictionary(uniqueKeysWithValues: existing.map { ($0.identityKey, $0) }) + let missing = Set(identityKeys).subtracting(models.keys) + let keys = Array(Set(missing.compactMap { key -> String? in + let parts = key.components(separatedBy: "::") + guard parts.count == 3, let appID = Int64(parts[0]) else { return nil } + return RatingHistoryChunk.key(appStoreID: appID, storefront: parts[1], year: String(parts[2].prefix(4))) + })) + guard !keys.isEmpty else { return models } + let chunks = try context.fetch(FetchDescriptor(predicate: #Predicate { + keys.contains($0.identityKey) + })) + let decoded = try chunks.map { ($0, try records(in: $0)) } + let storeIDs = Array(Set(decoded.flatMap { $0.1 }.compactMap(\.storeAppID))) + let apps = try context.fetch(FetchDescriptor(predicate: #Predicate { storeIDs.contains($0.appStoreID) })) + let appsByID = Dictionary(uniqueKeysWithValues: apps.map { ($0.appStoreID, $0) }) + for (chunk, values) in decoded { + let restored = values.filter { missing.contains($0.identityKey) } + guard !restored.isEmpty else { continue } + for value in restored { + let model = value.makeModel(storeApp: value.storeAppID.flatMap { appsByID[$0] }) + context.insert(model) + models[value.identityKey] = model + } + let remaining = values.filter { !missing.contains($0.identityKey) } + if remaining.isEmpty { context.delete(chunk) } + else { try replace(remaining, in: chunk) } + } + return models + } + + static func remove(appStoreID: Int64, storefront: String, in context: ModelContext) throws { + let chunks = try context.fetch(FetchDescriptor(predicate: #Predicate { + $0.appStoreID == appStoreID && $0.storefront == storefront + })) + for chunk in chunks { context.delete(chunk) } + } +} diff --git a/OpenASO/Services/Persistence/StorageSchemaV7Migrator.swift b/OpenASO/Services/Persistence/StorageSchemaV7Migrator.swift new file mode 100644 index 0000000..6ed80e8 --- /dev/null +++ b/OpenASO/Services/Persistence/StorageSchemaV7Migrator.swift @@ -0,0 +1,246 @@ +import CoreData +import Foundation +import SwiftData + +/// Copy and validate persisted archives before removing source rows. The +/// cursor advances with removal, so interrupted batches can safely repeat. +enum StorageSchemaV7Migrator { + static let batchSize = 5_000 + + private struct PendingDeletion: Codable { + let keys: [String] + let chunkKeys: [String] + let nextCursor: String + } + + /// The persistent path uses the public Core Data model bridge. SwiftData's + /// object deletion walks inverse arrays for every row in a large store. + /// Core Data performs the same bounded, validated transactions without + /// materializing those SwiftData arrays. No private SQLite tables are used. + @discardableResult + static func migrateIfNeeded( + at storeURL: URL, now: Date = .now, batchLimit: Int? = nil, + beforeDeletion: (@Sendable () throws -> Void)? = nil + ) throws -> Bool { + guard let model = NSManagedObjectModel.makeManagedObjectModel(for: OpenASOMigrationPlan.currentSchema.models) else { + throw CocoaError(.persistentStoreInvalidType) + } + let coordinator = NSPersistentStoreCoordinator(managedObjectModel: model) + let store = try coordinator.addPersistentStore(type: .sqlite, at: storeURL, + options: [NSPersistentHistoryTrackingKey: true, NSSQLitePragmasOption: ["cache_size": "-65536"]]) + defer { try? coordinator.remove(store) } + var batches = 0 + while batchLimit == nil || batches < batchLimit! { + let done = try autoreleasepool { + let context = NSManagedObjectContext(concurrencyType: .privateQueueConcurrencyType) + context.persistentStoreCoordinator = coordinator + context.undoManager = nil + return try context.performAndWait { + try migratePersistentBatch(in: context, coordinator: coordinator, now: now, beforeDeletion: beforeDeletion) + } + } + if done { return true } + batches += 1 + } + return false + } + + private static func migratePersistentBatch( + in context: NSManagedObjectContext, coordinator: NSPersistentStoreCoordinator, now: Date, + beforeDeletion: (@Sendable () throws -> Void)? + ) throws -> Bool { + let stateRequest = NSFetchRequest(entityName: "StorageMigrationState") + stateRequest.predicate = NSPredicate(format: "migrationKey == %@", "storage-v7") + stateRequest.fetchLimit = 1 + let state: NSManagedObject + if let existing = try context.fetch(stateRequest).first { state = existing } + else { + state = NSEntityDescription.insertNewObject(forEntityName: "StorageMigrationState", into: context) + state.setValue("storage-v7", forKey: "migrationKey") + state.setValue(0, forKey: "archivedRecordCount") + state.setValue(false, forKey: "completed") + state.setValue(true, forKey: "needsCompaction") + try context.save() + } + if state.value(forKey: "completed") as? Bool == true { + let previous = state.value(forKey: "lastMaintenanceAt") as? Date ?? .distantPast + guard now.timeIntervalSince(previous) >= 86_400 else { return true } + state.setValue(false, forKey: "completed") + state.setValue(true, forKey: "needsCompaction") + state.setValue(nil, forKey: "lastIdentityKey") + } + if let pending = state.value(forKey: "pendingDeletionPayload") as? Data { + try finishDeletion(JSONDecoder().decode(PendingDeletion.self, from: pending), state: state, in: context) + return false + } + let cursor = state.value(forKey: "lastIdentityKey") as? String ?? "" + let request = NSFetchRequest(entityName: "AppDailyRating") + request.predicate = NSPredicate(format: "identityKey > %@", cursor) + request.sortDescriptors = [NSSortDescriptor(key: "identityKey", ascending: true)] + request.fetchLimit = batchSize + 1 + request.returnsObjectsAsFaults = false + request.relationshipKeyPathsForPrefetching = ["storeApp"] + let candidates = try context.fetch(request) + let batch = Array(candidates.prefix(batchSize)) + guard let last = batch.last else { + state.setValue(true, forKey: "completed") + state.setValue(now, forKey: "lastMaintenanceAt") + try context.save() + try context.execute(NSPersistentHistoryChangeRequest.deleteHistory(before: Date.now)) + return true + } + let nextCursor = try RatingHistoryRecord(last).identityKey + let records = try batch.map(RatingHistoryRecord.init) + let nextRecord = try candidates.dropFirst(batch.count).first.map(RatingHistoryRecord.init) + let following = Array(records.dropFirst()) + [nextRecord].compactMap { $0 } + var sourceObjects: [NSManagedObject] = [] + var originals: [RatingHistoryRecord] = [] + for (index, record) in records.enumerated() where index < following.count { + let next = following[index] + if next.appStoreID == record.appStoreID && next.storefront == record.storefront { + sourceObjects.append(batch[index]) + originals.append(record) + } + } + let groups = Dictionary(grouping: originals, by: \.chunkKey) + let keys = Array(groups.keys) + let chunkRequest = NSFetchRequest(entityName: "RatingHistoryChunk") + chunkRequest.predicate = NSPredicate(format: "identityKey IN %@", keys) + let existing = try context.fetch(chunkRequest) + var chunks = Dictionary(uniqueKeysWithValues: existing.compactMap { row -> (String, NSManagedObject)? in + (row.value(forKey: "identityKey") as? String).map { ($0, row) } + }) + for (key, values) in groups { + let first = values[0] + let archived: RatingHistoryChunk + if let existing = chunks[key] { archived = try archiveValue(existing) } + else { archived = RatingHistoryChunk(appStoreID: first.appStoreID, storefront: first.storefront, year: String(first.ratingDate.prefix(4))) } + var merged = archived.recordCount == 0 ? [:] : Dictionary(uniqueKeysWithValues: try RatingHistoryStore.records(in: archived).map { ($0.identityKey, $0) }) + for record in values { merged[record.identityKey] = record } + try RatingHistoryStore.replace(Array(merged.values), in: archived) + let row = chunks[key] ?? NSEntityDescription.insertNewObject(forEntityName: "RatingHistoryChunk", into: context) + row.setValue(archived.identityKey, forKey: "identityKey") + row.setValue(archived.appStoreID, forKey: "appStoreID") + row.setValue(archived.storefront, forKey: "storefront") + row.setValue(archived.year, forKey: "year") + row.setValue(archived.firstObservedAt, forKey: "firstObservedAt") + row.setValue(archived.lastObservedAt, forKey: "lastObservedAt") + row.setValue(archived.recordCount, forKey: "recordCount") + row.setValue(archived.formatVersion, forKey: "formatVersion") + row.setValue(archived.uncompressedByteCount, forKey: "uncompressedByteCount") + row.setValue(archived.digest, forKey: "digest") + row.setValue(archived.payload, forKey: "payload") + chunks[key] = row + } + try context.save() + let expectedRecords = originals + let verification = NSManagedObjectContext(concurrencyType: .privateQueueConcurrencyType) + verification.persistentStoreCoordinator = coordinator + try verification.performAndWait { + let request = NSFetchRequest(entityName: "RatingHistoryChunk") + request.predicate = NSPredicate(format: "identityKey IN %@", keys) + let persisted = try verification.fetch(request) + let decoded = try persisted.flatMap { try RatingHistoryStore.records(in: archiveValue($0)) } + let byKey = Dictionary(uniqueKeysWithValues: decoded.map { ($0.identityKey, $0) }) + for original in expectedRecords where byKey[original.identityKey] != original { + throw RatingHistoryError.invalidArchive(original.identityKey) + } + } + // Persist detachment and a recovery marker together. Batch deletion + // then satisfies the frozen model's inverse-relationship constraint. + // After interruption, the validated archives remain authoritative for + // their original links until the pending deletion finishes. + let pending = PendingDeletion(keys: originals.map(\.identityKey), chunkKeys: keys, nextCursor: nextCursor) + for row in sourceObjects { row.setValue(nil, forKey: "storeApp") } + state.setValue(try JSONEncoder().encode(pending), forKey: "pendingDeletionPayload") + try context.save() + try beforeDeletion?() + try finishDeletion(pending, state: state, in: context) + return false + } + + private static func finishDeletion( + _ pending: PendingDeletion, state: NSManagedObject, in context: NSManagedObjectContext + ) throws { + let request = NSFetchRequest(entityName: "RatingHistoryChunk") + request.predicate = NSPredicate(format: "identityKey IN %@", pending.chunkKeys) + let archived = try context.fetch(request).flatMap { try RatingHistoryStore.records(in: archiveValue($0)) } + let archivedKeys = Set(archived.map(\.identityKey)) + guard pending.keys.allSatisfy({ archivedKeys.contains($0) }) else { + throw RatingHistoryError.invalidArchive("Pending source deletion") + } + if !pending.keys.isEmpty { + let source = NSFetchRequest(entityName: "AppDailyRating") + source.predicate = NSPredicate(format: "identityKey IN %@", pending.keys) + try context.execute(NSBatchDeleteRequest(fetchRequest: source)) + } + let previousCount = (state.value(forKey: "archivedRecordCount") as? NSNumber)?.intValue ?? 0 + state.setValue(previousCount + pending.keys.count, forKey: "archivedRecordCount") + state.setValue(pending.nextCursor, forKey: "lastIdentityKey") + state.setValue(nil, forKey: "pendingDeletionPayload") + try context.save() + if previousCount / 100_000 != (previousCount + pending.keys.count) / 100_000 { + try context.execute(NSPersistentHistoryChangeRequest.deleteHistory(before: Date.now)) + } + } + + private static func archiveValue(_ row: NSManagedObject) throws -> RatingHistoryChunk { + guard let appID = row.value(forKey: "appStoreID") as? NSNumber, + let storefront = row.value(forKey: "storefront") as? String, + let year = row.value(forKey: "year") as? String, + let key = row.value(forKey: "identityKey") as? String, + let first = row.value(forKey: "firstObservedAt") as? Date, + let last = row.value(forKey: "lastObservedAt") as? Date, + let count = row.value(forKey: "recordCount") as? NSNumber, + let version = row.value(forKey: "formatVersion") as? NSNumber, + let size = row.value(forKey: "uncompressedByteCount") as? NSNumber, + let digest = row.value(forKey: "digest") as? Data, + let payload = row.value(forKey: "payload") as? Data else { + throw RatingHistoryError.invalidArchive("Persistent archive attributes") + } + let value = RatingHistoryChunk(appStoreID: appID.int64Value, storefront: storefront, year: year) + value.identityKey = key + value.firstObservedAt = first + value.lastObservedAt = last + value.recordCount = count.intValue + value.formatVersion = version.intValue + value.uncompressedByteCount = size.intValue + value.digest = digest + value.payload = payload + return value + } + +} + +private extension RatingHistoryRecord { + init(_ row: NSManagedObject) throws { + guard let key = row.value(forKey: "identityKey") as? String, + let appID = row.value(forKey: "appStoreID") as? NSNumber, + let store = row.value(forKey: "storefront") as? String, + let day = row.value(forKey: "ratingDate") as? String, + let observed = row.value(forKey: "observedAt") as? Date, + let submissions = row.value(forKey: "submissionCount") as? NSNumber, + let winners = row.value(forKey: "winningCount") as? NSNumber, + let source = row.value(forKey: "sourceRaw") as? String else { + throw RatingHistoryError.invalidArchive("Persistent rating attributes") + } + identityKey = key + appStoreID = appID.int64Value + storefront = store + ratingDate = day + observedAt = observed + submissionCount = submissions.intValue + winningCount = winners.intValue + sourceRaw = source + ratingCount = (row.value(forKey: "ratingCount") as? NSNumber)?.intValue + averageRating = (row.value(forKey: "averageRating") as? NSNumber)?.doubleValue + oneStarRatingCount = (row.value(forKey: "oneStarRatingCount") as? NSNumber)?.intValue + twoStarRatingCount = (row.value(forKey: "twoStarRatingCount") as? NSNumber)?.intValue + threeStarRatingCount = (row.value(forKey: "threeStarRatingCount") as? NSNumber)?.intValue + fourStarRatingCount = (row.value(forKey: "fourStarRatingCount") as? NSNumber)?.intValue + fiveStarRatingCount = (row.value(forKey: "fiveStarRatingCount") as? NSNumber)?.intValue + confidenceRaw = row.value(forKey: "confidenceRaw") as? String + let parent = row.value(forKey: "storeApp") as? NSManagedObject + storeAppID = (parent?.value(forKey: "appStoreID") as? NSNumber)?.int64Value + } +} diff --git a/OpenASO/Services/RatingsReviews/AppStorefrontRatingService.swift b/OpenASO/Services/RatingsReviews/AppStorefrontRatingService.swift index 3792022..48c41ba 100644 --- a/OpenASO/Services/RatingsReviews/AppStorefrontRatingService.swift +++ b/OpenASO/Services/RatingsReviews/AppStorefrontRatingService.swift @@ -29,10 +29,17 @@ final class AppStorefrontRatingService: Sendable { return [] } - for outcome in outcomes { - persist(outcome, for: storeApp, in: modelContext) + do { + for outcome in outcomes { + try persist(outcome, for: storeApp, in: modelContext) + } + try modelContext.save() + } catch { + modelContext.rollback() + return outcomes.map { + AppStorefrontRatingRefreshOutcome(storefront: $0.storefront, result: nil, error: OpenASOError.map(error)) + } } - try? modelContext.save() OpenASOLog.ratings.info( "Finished ratings refresh appStoreID=\(storeApp.appStoreID, privacy: .public) successes=\(outcomes.filter { $0.error == nil }.count, privacy: .public) failures=\(outcomes.filter { $0.error != nil }.count, privacy: .public)" ) @@ -167,9 +174,9 @@ final class AppStorefrontRatingService: Sendable { _ outcome: AppStorefrontRatingRefreshOutcome, for storeApp: StoreApp, in modelContext: ModelContext - ) { + ) throws { if let result = outcome.result { - upsert( + try upsert( result, ratingDate: nil, submissionCount: 1, @@ -181,7 +188,7 @@ final class AppStorefrontRatingService: Sendable { } if outcome.clearsStoredRatings { - clearRatings(appStoreID: storeApp.appStoreID, storefront: outcome.storefront, in: modelContext) + try clearRatings(appStoreID: storeApp.appStoreID, storefront: outcome.storefront, in: modelContext) } } @@ -207,7 +214,7 @@ final class AppStorefrontRatingService: Sendable { confidence: String?, for storeApp: StoreApp, in modelContext: ModelContext - ) { + ) throws { let resolvedRatingDate = ratingDate ?? LatestAppRating.ratingDateString(for: result.observedAt) let snapshotKey = AppDailyRating.makeIdentityKey( appStoreID: result.appStoreID, @@ -215,7 +222,7 @@ final class AppStorefrontRatingService: Sendable { ratingDate: resolvedRatingDate ) let snapshot: AppDailyRating - if let existing = try? fetchSnapshot(identityKey: snapshotKey, in: modelContext) { + if let existing = try fetchSnapshot(identityKey: snapshotKey, in: modelContext) { snapshot = existing guard result.observedAt >= existing.observedAt else { return @@ -319,16 +326,10 @@ final class AppStorefrontRatingService: Sendable { } private func fetchSnapshot(identityKey: String, in modelContext: ModelContext) throws -> AppDailyRating? { - let targetIdentityKey = identityKey - let descriptor = FetchDescriptor( - predicate: #Predicate { snapshot in - snapshot.identityKey == targetIdentityKey - } - ) - return try modelContext.fetch(descriptor).first + try RatingHistoryStore.restore(identityKey: identityKey, in: modelContext) } - private func clearRatings(appStoreID: Int64, storefront: String, in modelContext: ModelContext) { + private func clearRatings(appStoreID: Int64, storefront: String, in modelContext: ModelContext) throws { let targetAppStoreID = appStoreID let targetStorefront = storefront let latestDescriptor = FetchDescriptor( @@ -342,6 +343,7 @@ final class AppStorefrontRatingService: Sendable { } ) + try RatingHistoryStore.remove(appStoreID: appStoreID, storefront: storefront, in: modelContext) let latest = (try? modelContext.fetch(latestDescriptor)) ?? [] let snapshots = (try? modelContext.fetch(snapshotDescriptor)) ?? [] for value in latest { diff --git a/OpenASO/Services/SearchRanking/RankingRefreshCoordinator.swift b/OpenASO/Services/SearchRanking/RankingRefreshCoordinator.swift index b214d2e..8393136 100644 --- a/OpenASO/Services/SearchRanking/RankingRefreshCoordinator.swift +++ b/OpenASO/Services/SearchRanking/RankingRefreshCoordinator.swift @@ -1207,14 +1207,7 @@ final class RankingRefreshCoordinator: Sendable { } private func fetchRatingSnapshot(identityKey: String, in modelContext: ModelContext) throws -> AppDailyRating? { - let targetIdentityKey = identityKey - var descriptor = FetchDescriptor( - predicate: #Predicate { snapshot in - snapshot.identityKey == targetIdentityKey - } - ) - descriptor.fetchLimit = 1 - return try modelContext.fetch(descriptor).first + try RatingHistoryStore.restore(identityKey: identityKey, in: modelContext) } private func fetchLatestRatings(identityKeys: [String], in modelContext: ModelContext) throws -> [String: LatestAppRating] { @@ -1231,16 +1224,7 @@ final class RankingRefreshCoordinator: Sendable { } private func fetchRatingSnapshots(identityKeys: [String], in modelContext: ModelContext) throws -> [String: AppDailyRating] { - guard !identityKeys.isEmpty else { return [:] } - - let targetIdentityKeys = identityKeys - let descriptor = FetchDescriptor( - predicate: #Predicate { snapshot in - targetIdentityKeys.contains(snapshot.identityKey) - } - ) - - return Dictionary(uniqueKeysWithValues: try modelContext.fetch(descriptor).map { ($0.identityKey, $0) }) + try RatingHistoryStore.restore(identityKeys: identityKeys, in: modelContext) } @MainActor diff --git a/OpenASOTests/AppServicesDependencyTests.swift b/OpenASOTests/AppServicesDependencyTests.swift index 0a085f4..905920b 100644 --- a/OpenASOTests/AppServicesDependencyTests.swift +++ b/OpenASOTests/AppServicesDependencyTests.swift @@ -7,7 +7,7 @@ import Testing @MainActor struct AppServicesDependencyTests { @Test - func modelContainerFactoryUsesAppendOnlyV6MigrationPlan() throws { + func modelContainerFactoryUsesAppendOnlyV7MigrationPlan() throws { let container = try ModelContainerFactory.makeModelContainer(isStoredInMemoryOnly: true) #expect(OpenASOSchemaV1.versionIdentifier == Schema.Version(1, 0, 0)) @@ -16,16 +16,16 @@ struct AppServicesDependencyTests { #expect(OpenASOSchemaV4.versionIdentifier == Schema.Version(4, 0, 0)) #expect(OpenASOSchemaV5.versionIdentifier == Schema.Version(5, 0, 0)) #expect(OpenASOSchemaV6.versionIdentifier == Schema.Version(6, 0, 0)) - #expect(OpenASOMigrationPlan.schemas.count == 6) + #expect(OpenASOMigrationPlan.schemas.count == 7) #expect(OpenASOMigrationPlan.schemas.first?.versionIdentifier == OpenASOSchemaV1.versionIdentifier) - #expect(OpenASOMigrationPlan.schemas.last?.versionIdentifier == OpenASOSchemaV6.versionIdentifier) + #expect(OpenASOMigrationPlan.schemas.last?.versionIdentifier == OpenASOSchemaV7.versionIdentifier) #expect(OpenASOSchemaV1.models.count == 17) #expect(OpenASOSchemaV2.models.count == 18) #expect(OpenASOSchemaV3.models.count == 19) #expect(OpenASOSchemaV4.models.count == 21) #expect(OpenASOSchemaV5.models.count == 23) #expect(OpenASOSchemaV6.models.count == 28) - #expect(OpenASOMigrationPlan.stages.count == 5) + #expect(OpenASOMigrationPlan.stages.count == 6) #expect(container.migrationPlan != nil) } diff --git a/OpenASOTests/OpenASOMCPServiceTests.swift b/OpenASOTests/OpenASOMCPServiceTests.swift index 28d2450..4a0dfbc 100644 --- a/OpenASOTests/OpenASOMCPServiceTests.swift +++ b/OpenASOTests/OpenASOMCPServiceTests.swift @@ -2801,9 +2801,16 @@ struct OpenASOMCPServiceTests { #expect(!traversedKeys.contains(insertedAfterFirstPage.observationKey)) } - @Test - func ratingHistoryIsAppScopedFilteredAndKeysetPaginated() async throws { - let context = try MCPTestContext() + @Test(arguments: [false, true]) + func ratingHistoryIsAppScopedFilteredAndKeysetPaginated(archived: Bool) async throws { + let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-MCP-Archive-\(UUID())") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let url = root.appendingPathComponent("default.store") + let schema = ModelContainerFactory.schema + let container = try ModelContainer(for: schema, migrationPlan: OpenASOMigrationPlan.self, + configurations: ModelConfiguration(schema: schema, url: url)) + let context = try MCPTestContext(modelContainer: container) let target = try context.insertStoreApp(appStoreID: 123, name: "Focus Timer") let other = try context.insertStoreApp(appStoreID: 999, name: "Other Timer") let tiedGB = try context.insertRatingSnapshot( @@ -2848,6 +2855,8 @@ struct OpenASOMCPServiceTests { averageRating: 5 ) let tiedKeys = [tiedGB.identityKey, tiedUS.identityKey].sorted() + let oldUSKey = oldUS.identityKey + if archived { try StorageSchemaV7Migrator.migrateIfNeeded(at: url) } let first = try await context.service.listRatingHistory( appStoreID: 123, @@ -2875,7 +2884,7 @@ struct OpenASOMCPServiceTests { appStoreID: 123, page: OpenASOMCPPageRequest(limit: 1, cursor: secondCursor) ) - #expect(third.items.map(\.identityKey) == [oldUS.identityKey]) + #expect(third.items.map(\.identityKey) == [oldUSKey]) #expect(third.nextCursor == nil) #expect(![first, second, third].flatMap(\.items).contains { $0.identityKey == insertedAfterFirstPage.identityKey diff --git a/OpenASOTests/PersistenceMigrationTests.swift b/OpenASOTests/PersistenceMigrationTests.swift index 19dcc40..c46d933 100644 --- a/OpenASOTests/PersistenceMigrationTests.swift +++ b/OpenASOTests/PersistenceMigrationTests.swift @@ -6,7 +6,7 @@ import Testing @MainActor struct PersistenceMigrationTests { @Test - func migrationPlanAppendsThroughV6AndKeepsPriorSchemasFrozen() throws { + func migrationPlanAppendsThroughV7AndKeepsPriorSchemasFrozen() throws { let container = try ModelContainerFactory.makeModelContainer(isStoredInMemoryOnly: true) #expect(OpenASOSchemaV1.versionIdentifier == Schema.Version(1, 0, 0)) @@ -21,8 +21,9 @@ struct PersistenceMigrationTests { #expect(OpenASOSchemaV5.models.count == 23) #expect(OpenASOSchemaV6.versionIdentifier == Schema.Version(6, 0, 0)) #expect(OpenASOSchemaV6.models.count == 28) - #expect(OpenASOMigrationPlan.currentSchema.versionIdentifier == Schema.Version(6, 0, 0)) - #expect(OpenASOMigrationPlan.schemas.count == 6) + #expect(OpenASOSchemaV7.models.count == 30) + #expect(OpenASOMigrationPlan.currentSchema.versionIdentifier == Schema.Version(7, 0, 0)) + #expect(OpenASOMigrationPlan.schemas.count == 7) #expect(OpenASOMigrationPlan.schemas[0].versionIdentifier == OpenASOSchemaV1.versionIdentifier) #expect(OpenASOMigrationPlan.schemas[1].versionIdentifier == OpenASOSchemaV2.versionIdentifier) #expect(OpenASOMigrationPlan.schemas[2].versionIdentifier == OpenASOSchemaV3.versionIdentifier) @@ -33,7 +34,7 @@ struct PersistenceMigrationTests { OpenASOMigrationPlan.currentSchema.versionIdentifier == OpenASOMigrationPlan.schemas.last?.versionIdentifier ) - #expect(OpenASOMigrationPlan.stages.count == 5) + #expect(OpenASOMigrationPlan.stages.count == 6) #expect(container.migrationPlan != nil) } diff --git a/OpenASOTests/StorageSchemaV7MigrationTests.swift b/OpenASOTests/StorageSchemaV7MigrationTests.swift new file mode 100644 index 0000000..ebab01d --- /dev/null +++ b/OpenASOTests/StorageSchemaV7MigrationTests.swift @@ -0,0 +1,260 @@ +import Foundation +import SwiftData +import Testing +@testable import OpenASO + +@MainActor +struct StorageSchemaV7MigrationTests { + @Test + func archivePreservesEveryValueAndHistoricalCorrectionsCanBeWritten() throws { + try withTestStore { container, url in + let context = ModelContext(container) + let app = StoreApp(appStoreID: 42, bundleID: "example", name: "App", sellerName: nil, + iconURLString: nil, defaultPlatform: .iphone) + context.insert(app) + let old = makeRating(day: "2026-01-01", observedAt: Date(timeIntervalSince1970: 1_700_000_000), app: app) + let latest = makeRating(day: "2026-01-02", observedAt: old.observedAt.addingTimeInterval(1), app: app) + old.sourceRaw = "future-source" + context.insert(old); context.insert(latest) + try context.save() + let expected = try RatingHistoryStore.fetch(appStoreIDs: [42], in: context) + + try StorageSchemaV7Migrator.migrateIfNeeded(at: url) + let migrated = ModelContext(container) + #expect(try migrated.fetchCount(FetchDescriptor()) == 1) + #expect(try migrated.fetchCount(FetchDescriptor()) == 1) + #expect(try RatingHistoryStore.fetch(appStoreIDs: [42], in: migrated) == expected) + + let key = "42::gb::2026-01-01" + let restored = try #require(try RatingHistoryStore.restore(identityKey: key, in: migrated)) + #expect(restored.storeApp?.appStoreID == 42) + #expect(restored.sourceRaw == "future-source") + restored.ratingCount = 999 + try migrated.save() + let reread = try RatingHistoryStore.fetch(appStoreIDs: [42], in: ModelContext(container)) + #expect(reread.count == 2) + #expect(reread.first { $0.identityKey == key }?.ratingCount == 999) + } + } + + @Test + func interruptedBatchesResumeAndSecondRunDoesNotDuplicateDays() throws { + try withTestStore { container, url in + let context = ModelContext(container) + for appID in 1...60 { + for day in 1...100 { + let value = makeRating(day: String(format: "2026-%02d-%02d", 1 + (day - 1) / 28, 1 + (day - 1) % 28), + observedAt: Date(timeIntervalSince1970: Double(day)), app: nil) + value.appStoreID = Int64(appID) + value.identityKey = AppDailyRating.makeIdentityKey(appStoreID: Int64(appID), storefront: "gb", ratingDate: value.ratingDate) + context.insert(value) + } + } + try context.save() + let ids = (1...60).map(Int64.init) + let expected = try RatingHistoryStore.fetch(appStoreIDs: ids, in: context) + #expect(try !StorageSchemaV7Migrator.migrateIfNeeded(at: url, batchLimit: 1)) + #expect(try RatingHistoryStore.fetch(appStoreIDs: ids, in: ModelContext(container)) == expected) + #expect(try StorageSchemaV7Migrator.migrateIfNeeded(at: url)) + #expect(try RatingHistoryStore.fetch(appStoreIDs: ids, in: ModelContext(container)) == expected) + #expect(try StorageSchemaV7Migrator.migrateIfNeeded(at: url)) + #expect(try ModelContext(container).fetchCount(FetchDescriptor()) == 60) + #expect(try RatingHistoryStore.fetch(appStoreIDs: ids, in: ModelContext(container)) == expected) + } + } + + @Test + func corruptArchiveStopsMigrationBeforeSourceDeletion() throws { + try withTestStore { container, url in + let context = ModelContext(container) + for day in ["2026-01-01", "2026-01-02"] { + context.insert(makeRating(day: day, observedAt: .now, app: nil)) + } + let corrupt = RatingHistoryChunk(appStoreID: 42, storefront: "gb", year: "2026") + corrupt.recordCount = 1 + corrupt.payload = Data([1, 2, 3]) + context.insert(corrupt) + try context.save() + #expect(throws: (any Error).self) { try StorageSchemaV7Migrator.migrateIfNeeded(at: url) } + #expect(try ModelContext(container).fetchCount(FetchDescriptor()) == 2) + } + } + + @Test + func observationDateFiltersRemainIndependentOfLogicalRatingDate() throws { + try withTestStore { container, url in + let context = ModelContext(container) + let observed = Date(timeIntervalSince1970: 100) + context.insert(makeRating(day: "2025-12-31", observedAt: observed, app: nil)) + context.insert(makeRating(day: "2026-01-01", observedAt: observed.addingTimeInterval(100), app: nil)) + try context.save() + try StorageSchemaV7Migrator.migrateIfNeeded(at: url) + let values = try RatingHistoryStore.fetch(appStoreIDs: [42], storefronts: ["gb"], from: observed, + to: observed, in: ModelContext(container)) + #expect(values.map(\.ratingDate) == ["2025-12-31"]) + } + } + + @Test + func releasedV6StoreCompactsAutomaticallyAndReopens() throws { + let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-V7-\(UUID())") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let url = root.appendingPathComponent("default.store") + try autoreleasepool { + let schema = Schema(versionedSchema: OpenASOSchemaV6.self) + let container = try ModelContainer(for: schema, configurations: ModelConfiguration(schema: schema, url: url)) + let context = ModelContext(container) + context.insert(makeRating(day: "2026-01-01", observedAt: .now, app: nil)) + context.insert(makeRating(day: "2026-01-02", observedAt: .now, app: nil)) + try context.save() + } + try autoreleasepool { + let container = try ModelContainerFactory.makePersistentModelContainer(at: url) + let context = ModelContext(container) + #expect(try RatingHistoryStore.fetch(appStoreIDs: [42], in: context).count == 2) + let state = try #require(try context.fetch(FetchDescriptor()).first) + #expect(state.completed) + #expect(!state.needsCompaction) + } + try autoreleasepool { + let container = try ModelContainerFactory.makePersistentModelContainer(at: url) + #expect(try RatingHistoryStore.fetch(appStoreIDs: [42], in: ModelContext(container)).count == 2) + } + } + + @Test + func anotherLiveContainerDefersPhysicalMaintenance() throws { + let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-V7-Lease-\(UUID())") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let url = root.appendingPathComponent("default.store") + try autoreleasepool { + let first = try ModelContainerFactory.makePersistentModelContainer(at: url) + let context = ModelContext(first) + let state = try #require(try context.fetch(FetchDescriptor()).first) + state.lastMaintenanceAt = .distantPast + try context.save() + let second = try ModelContainerFactory.makePersistentModelContainer(at: url) + let otherState = try #require(try ModelContext(second).fetch(FetchDescriptor()).first) + #expect(otherState.lastMaintenanceAt == .distantPast) + } + } + + @Test + func newerLiveCopySupersedesAnArchivedDayBeforeDateFiltering() throws { + try withTestStore { container, url in + let context = ModelContext(container) + let old = makeRating(day: "2026-01-01", observedAt: Date(timeIntervalSince1970: 100), app: nil) + let chunk = RatingHistoryChunk(appStoreID: 42, storefront: "gb", year: "2026") + try RatingHistoryStore.replace([RatingHistoryRecord(old)], in: chunk) + context.insert(chunk) + old.observedAt = Date(timeIntervalSince1970: 200) + context.insert(old) + try context.save() + let values = try RatingHistoryStore.fetch(appStoreIDs: [42], + from: Date(timeIntervalSince1970: 50), to: Date(timeIntervalSince1970: 150), in: context) + #expect(values.isEmpty) + } + } + + @Test + func preLeaseClientPreventsAnUpgrade() throws { + let url = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-LegacyLease-\(UUID())") + try Data(repeating: 7, count: 256 * 1_024).write(to: url) + defer { try? FileManager.default.removeItem(at: url) } + let process = Process() + let output = Pipe() + process.executableURL = URL(fileURLWithPath: "/bin/cat") + process.arguments = [url.path] + process.standardOutput = output + process.standardError = FileHandle.nullDevice + try process.run() + defer { + process.terminate() + process.waitUntilExit() + } + _ = output.fileHandleForReading.readData(ofLength: 1) + #expect(throws: (any Error).self) { try PersistentStoreCompactor.requireNoOlderClients(at: url) } + } + + @Test + func releasedV6ClientCannotDowngradeAnArchivedStore() throws { + let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-V7-Downgrade-\(UUID())") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let url = root.appendingPathComponent("default.store") + try autoreleasepool { + let container = try ModelContainerFactory.makePersistentModelContainer(at: url) + let context = ModelContext(container) + context.insert(makeRating(day: "2026-01-01", observedAt: .now, app: nil)) + context.insert(makeRating(day: "2026-01-02", observedAt: .now, app: nil)) + try context.save() + // Run a later maintenance pass on this private test store. + try StorageSchemaV7Migrator.migrateIfNeeded(at: url, now: .now.addingTimeInterval(86_401)) + } + #expect(throws: (any Error).self) { + try autoreleasepool { + let schema = Schema(versionedSchema: OpenASOSchemaV6.self) + _ = try ModelContainer(for: schema, migrationPlan: ReleasedV6StoragePlan.self, + configurations: ModelConfiguration(schema: schema, url: url)) + } + } + try autoreleasepool { + let container = try ModelContainerFactory.makePersistentModelContainer(at: url) + #expect(try RatingHistoryStore.fetch(appStoreIDs: [42], in: ModelContext(container)).count == 2) + } + } + + @Test + func interruptedAfterDetachmentResumesWithoutLosingOriginalLinks() throws { + try withTestStore { container, url in + let context = ModelContext(container) + let app = StoreApp(appStoreID: 42, bundleID: "example", name: "App", sellerName: nil, + iconURLString: nil, defaultPlatform: .iphone) + context.insert(app) + context.insert(makeRating(day: "2026-01-01", observedAt: .now, app: app)) + context.insert(makeRating(day: "2026-01-02", observedAt: .now, app: app)) + try context.save() + let expected = try RatingHistoryStore.fetch(appStoreIDs: [42], in: context) + #expect(throws: InjectedStorageInterruption.self) { + try StorageSchemaV7Migrator.migrateIfNeeded(at: url, beforeDeletion: { + throw InjectedStorageInterruption.stop + }) + } + let state = try #require(try ModelContext(container).fetch(FetchDescriptor()).first) + #expect(state.pendingDeletionPayload != nil) + try StorageSchemaV7Migrator.migrateIfNeeded(at: url) + #expect(try RatingHistoryStore.fetch(appStoreIDs: [42], in: ModelContext(container)) == expected) + #expect(try ModelContext(container).fetchCount(FetchDescriptor()) == 1) + } + } + + private func withTestStore(_ body: (ModelContainer, URL) throws -> Void) throws { + let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-Archive-\(UUID())") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + try autoreleasepool { + let schema = ModelContainerFactory.schema + let url = root.appendingPathComponent("default.store") + let container = try ModelContainer(for: schema, migrationPlan: OpenASOMigrationPlan.self, + configurations: ModelConfiguration(schema: schema, url: url)) + try body(container, url) + } + } + + private func makeRating(day: String, observedAt: Date, app: StoreApp?) -> AppDailyRating { + AppDailyRating(appStoreID: 42, storefront: "gb", ratingCount: 123, averageRating: 4.625, + oneStarRatingCount: 1, twoStarRatingCount: 2, threeStarRatingCount: 3, + fourStarRatingCount: 4, fiveStarRatingCount: 113, ratingDate: day, + observedAt: observedAt, submissionCount: 5, winningCount: 4, + confidence: "evidence", source: .iTunesSearch, storeApp: app) + } +} + +private enum ReleasedV6StoragePlan: SchemaMigrationPlan { + static var schemas: [any VersionedSchema.Type] { Array(OpenASOMigrationPlan.schemas.prefix(6)) } + static var stages: [MigrationStage] { Array(OpenASOMigrationPlan.stages.prefix(5)) } +} + +private enum InjectedStorageInterruption: Error { case stop } From 20f8ca45bea1ae2af36edb6a12c0ec898a2241fb Mon Sep 17 00:00:00 2001 From: hubab1 <50897577+hubab1@users.noreply.github.com> Date: Sat, 24 Oct 2026 23:34:58 +0100 Subject: [PATCH 2/6] perf(persistence): Optimize release compaction Repack large stores with 16 KiB pages and prefetch live rating links. Prepare persistence off the UI thread and show startup progress. Verify that history survives compaction and WAL reopening. --- OpenASO/App/OpenASOApp.swift | 40 ++++++++++++++-- .../PersistentStoreCompactor.swift | 27 +++++++++-- .../Persistence/RatingHistoryStore.swift | 7 +-- .../StorageSchemaV7MigrationTests.swift | 46 +++++++++++++++++++ 4 files changed, 109 insertions(+), 11 deletions(-) diff --git a/OpenASO/App/OpenASOApp.swift b/OpenASO/App/OpenASOApp.swift index b6749fe..8d3056e 100644 --- a/OpenASO/App/OpenASOApp.swift +++ b/OpenASO/App/OpenASOApp.swift @@ -7,7 +7,8 @@ struct OpenASOApp: App { @State private var updaterController = SparkleUpdaterController() @State private var launchAlert: AppLaunchAlertContext? - private let startupState: OpenASOStartupState + @State private var startupState: OpenASOStartupState = .preparing + @State private var isPreparingStartup = false init() { let executionMode = OpenASOExecutionMode( @@ -21,20 +22,42 @@ struct OpenASOApp: App { Self.runBackgroundRefreshAndExit() } - let startupState = Self.makeStartupState() if executionMode == .mcpStdio { - switch startupState { + switch Self.makeStartupState() { case .ready(_, let services): Self.runMCPStdioAndExit(serverProvider: services.mcpServerProvider) case .storeUnavailable(let error): Self.exitMCPStdio(with: error.diagnosticReport) + case .preparing: + preconditionFailure("Synchronous startup cannot return a preparing state.") } } - self.startupState = startupState _launchAlert = State(initialValue: nil) } + private func prepareStartupIfNeeded() async { + guard case .preparing = startupState, !isPreparingStartup else { return } + isPreparingStartup = true + defer { isPreparingStartup = false } + let result = await Task.detached(priority: .userInitiated) { + () -> Result in + do { + return .success(try ModelContainerFactory.makeModelContainer(isStoredInMemoryOnly: false)) + } catch let error as PersistentStoreError { + return .failure(error) + } catch { + return .failure(.unexpected(diagnosticDescription: String(reflecting: error))) + } + }.value + switch result { + case .success(let container): + startupState = .ready(modelContainer: container, services: AppServices.appLaunch(modelContainer: container)) + case .failure(let error): + startupState = .storeUnavailable(error) + } + } + private static func makeStartupState() -> OpenASOStartupState { do { let modelContainer = try ModelContainerFactory.makeModelContainer(isStoredInMemoryOnly: false) @@ -84,6 +107,10 @@ struct OpenASOApp: App { var body: some Scene { WindowGroup("OpenASO") { switch startupState { + case .preparing: + ProgressView("Preparing your workspace…") + .frame(idealWidth: 1000, idealHeight: 760) + .task { await prepareStartupIfNeeded() } case .ready(let modelContainer, let services): RootView() .environment(services) @@ -123,6 +150,10 @@ struct OpenASOApp: App { Settings { switch startupState { + case .preparing: + ProgressView("Preparing your workspace…") + .frame(idealWidth: 680, idealHeight: 480) + .task { await prepareStartupIfNeeded() } case .ready(let modelContainer, let services): SettingsView() .environment(services) @@ -167,6 +198,7 @@ struct OpenASOApp: App { } private enum OpenASOStartupState { + case preparing case ready(modelContainer: ModelContainer, services: AppServices) case storeUnavailable(PersistentStoreError) } diff --git a/OpenASO/Services/Persistence/PersistentStoreCompactor.swift b/OpenASO/Services/Persistence/PersistentStoreCompactor.swift index 493d880..73eb1a8 100644 --- a/OpenASO/Services/Persistence/PersistentStoreCompactor.swift +++ b/OpenASO/Services/Persistence/PersistentStoreCompactor.swift @@ -22,9 +22,11 @@ enum PersistentStoreCompactor { } let freePages = try pragma("freelist_count") let pageCount = try pragma("page_count") - let freeBytes = try freePages * pragma("page_size") + let pageSize = try pragma("page_size") + let freeBytes = freePages * pageSize // Avoid a full-file rewrite on daily launches when pages can be reused. - return freeBytes >= 64 * 1_024 * 1_024 + return (pageSize != 16_384 && pageCount * pageSize >= 256 * 1_024 * 1_024) + || freeBytes >= 64 * 1_024 * 1_024 || (freeBytes >= 8 * 1_024 * 1_024 && freePages >= pageCount / 5) } @@ -57,6 +59,22 @@ enum PersistentStoreCompactor { /// Use Core Data's supported SQLite maintenance option. Never edit its /// private history tables or replace a live SQLite file behind a container. static func compact(at storeURL: URL) throws { + // SQLite cannot change page size in WAL mode. All clients are closed + // under the migration lease; checkpoint before the framework rebuild. + // SwiftData restores WAL when the completed store is reopened. + var database: OpaquePointer? + guard sqlite3_open_v2(storeURL.path, &database, SQLITE_OPEN_READWRITE, nil) == SQLITE_OK else { + if let database { sqlite3_close(database) } + throw CocoaError(.fileWriteUnknown) + } + do { + defer { sqlite3_close(database) } + sqlite3_busy_timeout(database, 5_000) + guard sqlite3_exec(database, "PRAGMA wal_checkpoint(TRUNCATE)", nil, nil, nil) == SQLITE_OK, + sqlite3_exec(database, "PRAGMA journal_mode=DELETE", nil, nil, nil) == SQLITE_OK else { + throw CocoaError(.fileLocking) + } + } try autoreleasepool { guard let model = NSManagedObjectModel.makeManagedObjectModel( for: OpenASOMigrationPlan.currentSchema.models @@ -64,8 +82,9 @@ enum PersistentStoreCompactor { let coordinator = NSPersistentStoreCoordinator(managedObjectModel: model) let store = try coordinator.addPersistentStore( type: .sqlite, at: storeURL, - options: [NSSQLiteManualVacuumOption: true, NSPersistentHistoryTrackingKey: true, - NSPersistentStoreTimeoutOption: 5] + options: [NSSQLiteManualVacuumOption: true, NSPersistentHistoryTrackingKey: false, + NSPersistentStoreTimeoutOption: 5, + NSSQLitePragmasOption: ["journal_mode": "DELETE", "page_size": "16384"]] ) try coordinator.remove(store) } diff --git a/OpenASO/Services/Persistence/RatingHistoryStore.swift b/OpenASO/Services/Persistence/RatingHistoryStore.swift index 21e20a4..805cd86 100644 --- a/OpenASO/Services/Persistence/RatingHistoryStore.swift +++ b/OpenASO/Services/Persistence/RatingHistoryStore.swift @@ -172,11 +172,12 @@ enum RatingHistoryStore { ) throws -> [RatingHistoryRecord] { guard !appStoreIDs.isEmpty else { return [] } let filtersStorefront = !storefronts.isEmpty - let legacy = try context.fetch(FetchDescriptor(predicate: #Predicate { + var legacyDescriptor = FetchDescriptor(predicate: #Predicate { appStoreIDs.contains($0.appStoreID) && (!filtersStorefront || storefronts.contains($0.storefront)) - - })) + }) + legacyDescriptor.relationshipKeyPathsForPrefetching = [\.storeApp] + let legacy = try context.fetch(legacyDescriptor) let chunks = try context.fetch(FetchDescriptor(predicate: #Predicate { appStoreIDs.contains($0.appStoreID) && (!filtersStorefront || storefronts.contains($0.storefront)) diff --git a/OpenASOTests/StorageSchemaV7MigrationTests.swift b/OpenASOTests/StorageSchemaV7MigrationTests.swift index ebab01d..65c3111 100644 --- a/OpenASOTests/StorageSchemaV7MigrationTests.swift +++ b/OpenASOTests/StorageSchemaV7MigrationTests.swift @@ -1,4 +1,5 @@ import Foundation +import SQLite3 import SwiftData import Testing @testable import OpenASO @@ -230,6 +231,51 @@ struct StorageSchemaV7MigrationTests { } } + @Test + func compactedPageLayoutReopensWithUnchangedHistoryAndWAL() throws { + let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-PageLayout-\(UUID())") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let url = root.appendingPathComponent("default.store") + let expected = try autoreleasepool { + let schema = ModelContainerFactory.schema + let container = try ModelContainer(for: schema, migrationPlan: OpenASOMigrationPlan.self, + configurations: ModelConfiguration(schema: schema, url: url)) + let context = ModelContext(container) + context.insert(makeRating(day: "2026-01-01", observedAt: .now, app: nil)) + context.insert(makeRating(day: "2026-01-02", observedAt: .now, app: nil)) + try context.save() + try PersistentHistoryMaintenance.prune(in: context) + return try RatingHistoryStore.fetch(appStoreIDs: [42], in: context) + } + try PersistentStoreCompactor.compact(at: url) + #expect(try databasePragma("page_size", at: url) == "16384") + let actual = try autoreleasepool { + let container = try ModelContainerFactory.makePersistentModelContainer(at: url) + return try RatingHistoryStore.fetch(appStoreIDs: [42], in: ModelContext(container)) + } + #expect(actual == expected) + #expect(try databasePragma("page_size", at: url) == "16384") + #expect(try databasePragma("journal_mode", at: url) == "wal") + } + + private func databasePragma(_ name: String, at url: URL) throws -> String { + var database: OpaquePointer? + guard sqlite3_open_v2(url.path, &database, SQLITE_OPEN_READONLY, nil) == SQLITE_OK else { + throw CocoaError(.fileReadUnknown) + } + defer { sqlite3_close(database) } + var statement: OpaquePointer? + guard sqlite3_prepare_v2(database, "PRAGMA " + name, -1, &statement, nil) == SQLITE_OK else { + throw CocoaError(.fileReadUnknown) + } + defer { sqlite3_finalize(statement) } + guard sqlite3_step(statement) == SQLITE_ROW, let value = sqlite3_column_text(statement, 0) else { + throw CocoaError(.fileReadUnknown) + } + return String(cString: value) + } + private func withTestStore(_ body: (ModelContainer, URL) throws -> Void) throws { let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-Archive-\(UUID())") try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) From 94bd6690c5be436361c8ca2074f3ee09c9181515 Mon Sep 17 00:00:00 2001 From: hubab1 <50897577+hubab1@users.noreply.github.com> Date: Sun, 25 Oct 2026 10:07:52 +0000 Subject: [PATCH 3/6] fix(persistence): Compact history during ingestion Archive prior days during ingestion and update older days in place. Keep the existing-data migration complete after its first successful run. Serialize worker transactions to preserve concurrent archive edits. --- OpenASO/Models/OpenASOSchemaV7.swift | 1 + .../Persistence/BackgroundModelStore.swift | 9 +- .../Persistence/ModelContainerFactory.swift | 39 ++- .../Persistence/RatingHistoryStore.swift | 257 +++++++++++++-- .../Persistence/StorageSchemaV7Migrator.swift | 8 +- .../AppStorefrontRatingService.swift | 134 +------- .../RankingRefreshCoordinator.swift | 207 +----------- .../AppStorefrontRatingServiceTests.swift | 1 + .../RankingRefreshCoordinatorTests.swift | 37 +++ .../StorageSchemaV7MigrationTests.swift | 295 +++++++++++++++++- 10 files changed, 605 insertions(+), 383 deletions(-) diff --git a/OpenASO/Models/OpenASOSchemaV7.swift b/OpenASO/Models/OpenASOSchemaV7.swift index 0e8fe39..5ffb55d 100644 --- a/OpenASO/Models/OpenASOSchemaV7.swift +++ b/OpenASO/Models/OpenASOSchemaV7.swift @@ -52,6 +52,7 @@ enum OpenASOSchemaV7: VersionedSchema { var archivedRecordCount: Int var completed: Bool var needsCompaction: Bool + // Completion timestamp retained by the one-time upgrade; never a scan trigger. var lastMaintenanceAt: Date? init() { diff --git a/OpenASO/Services/Persistence/BackgroundModelStore.swift b/OpenASO/Services/Persistence/BackgroundModelStore.swift index acf0a5d..ec4441c 100644 --- a/OpenASO/Services/Persistence/BackgroundModelStore.swift +++ b/OpenASO/Services/Persistence/BackgroundModelStore.swift @@ -89,8 +89,10 @@ private actor BackgroundModelContextWorker { func write( _ operation: @Sendable (ModelContext) throws -> Value ) throws -> Value { - let modelContext = prepareModelContext() - do { + // The dedicated queue may wait for another process's bounded save; + // never keep a cached context across independent write transactions. + defer { context = nil } + return try ModelContainerFactory.withWriteTransaction(in: modelContainer) { modelContext in let value = try operation(modelContext) try modelContext.save() let now = Date.now @@ -102,9 +104,6 @@ private actor BackgroundModelContextWorker { } } return value - } catch { - modelContext.rollback() - throw error } } diff --git a/OpenASO/Services/Persistence/ModelContainerFactory.swift b/OpenASO/Services/Persistence/ModelContainerFactory.swift index 1d32eb3..7f69ce1 100644 --- a/OpenASO/Services/Persistence/ModelContainerFactory.swift +++ b/OpenASO/Services/Persistence/ModelContainerFactory.swift @@ -103,10 +103,8 @@ enum ModelContainerFactory { try RankingSchemaV6Migrator.migrateIfNeeded(in: container) let context = ModelContext(container) let prior = try context.fetch(FetchDescriptor()).first - let needsMaintenance = prior?.needsCompaction != false || (prior?.lastMaintenanceAt.map { - Date.now.timeIntervalSince($0) >= 86_400 - } ?? true) - guard needsMaintenance else { return container } + let needsUpgrade = prior?.completed != true || prior?.needsCompaction != false + guard needsUpgrade else { return container } // No app consumer persists SwiftData history tokens. Research // observations live in model entities, not this change log. try PersistentHistoryMaintenance.prune(in: context) @@ -133,6 +131,39 @@ enum ModelContainerFactory { } } + /// Serialize complete worker transactions across app, MCP, and refresh + /// processes. Fresh contexts prevent cached chunks from overwriting a + /// different writer's committed days. No network await occurs under this lock. + static func withWriteTransaction( + in container: ModelContainer, _ operation: (ModelContext) throws -> Value + ) throws -> Value { + let configuration = container.configurations.first! + let identity = configuration.isStoredInMemoryOnly + ? "memory:\(ObjectIdentifier(container))" + : configuration.url.standardizedFileURL.path + let digest = SHA256.hash(data: Data(identity.utf8)).map { String(format: "%02x", $0) }.joined() + let directory = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-Writes", isDirectory: true) + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + let url = directory.appendingPathComponent("\(digest).lock") + let descriptor = url.path.withCString { Darwin.open($0, O_CREAT | O_RDWR | O_CLOEXEC, S_IRUSR | S_IWUSR) } + guard descriptor >= 0 else { throw CocoaError(.fileWriteUnknown) } + defer { Darwin.close(descriptor) } + while flock(descriptor, LOCK_EX) != 0 { + guard errno == EINTR else { throw CocoaError(.fileLocking) } + } + defer { flock(descriptor, LOCK_UN) } + let context = ModelContext(container) + context.autosaveEnabled = false + do { + let result = try operation(context) + if context.hasChanges { try context.save() } + return result + } catch { + context.rollback() + throw error + } + } + private static func storeURL(namespace: AppNamespace) throws -> URL { let storeDirectoryURL = try namespace.applicationSupportDirectoryURL() .appendingPathComponent("SwiftData", isDirectory: true) diff --git a/OpenASO/Services/Persistence/RatingHistoryStore.swift b/OpenASO/Services/Persistence/RatingHistoryStore.swift index 805cd86..62fe16f 100644 --- a/OpenASO/Services/Persistence/RatingHistoryStore.swift +++ b/OpenASO/Services/Persistence/RatingHistoryStore.swift @@ -48,6 +48,74 @@ struct RatingHistoryRecord: Codable, Equatable, Sendable { storeAppID = value.storeApp?.appStoreID } + init(_ result: AppStorefrontRatingResult, ratingDate: String?, submissionCount: Int, + winningCount: Int, confidence: String?) { + appStoreID = result.appStoreID + storefront = result.storefront.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() + self.ratingDate = ratingDate ?? LatestAppRating.ratingDateString(for: result.observedAt) + identityKey = AppDailyRating.makeIdentityKey(appStoreID: appStoreID, storefront: storefront, ratingDate: self.ratingDate) + ratingCount = result.ratingCount + averageRating = result.averageRating + oneStarRatingCount = result.ratingCounts?.oneStar + twoStarRatingCount = result.ratingCounts?.twoStar + threeStarRatingCount = result.ratingCounts?.threeStar + fourStarRatingCount = result.ratingCounts?.fourStar + fiveStarRatingCount = result.ratingCounts?.fiveStar + observedAt = result.observedAt + self.submissionCount = submissionCount + self.winningCount = winningCount + confidenceRaw = confidence + sourceRaw = result.source.rawValue + storeAppID = result.appStoreID + } + + var scopeKey: String { LatestAppRating.makeIdentityKey(appStoreID: appStoreID, storefront: storefront) } + + mutating func retainStarCounts(from previous: RatingHistoryRecord) { + oneStarRatingCount = previous.oneStarRatingCount + twoStarRatingCount = previous.twoStarRatingCount + threeStarRatingCount = previous.threeStarRatingCount + fourStarRatingCount = previous.fourStarRatingCount + fiveStarRatingCount = previous.fiveStarRatingCount + } + + func apply(to model: AppDailyRating, storeApp: StoreApp?) { + if model.identityKey != identityKey { model.identityKey = identityKey } + if model.appStoreID != appStoreID { model.appStoreID = appStoreID } + if model.storefront != storefront { model.storefront = storefront } + if model.ratingCount != ratingCount { model.ratingCount = ratingCount } + if model.averageRating != averageRating { model.averageRating = averageRating } + if model.oneStarRatingCount != oneStarRatingCount { model.oneStarRatingCount = oneStarRatingCount } + if model.twoStarRatingCount != twoStarRatingCount { model.twoStarRatingCount = twoStarRatingCount } + if model.threeStarRatingCount != threeStarRatingCount { model.threeStarRatingCount = threeStarRatingCount } + if model.fourStarRatingCount != fourStarRatingCount { model.fourStarRatingCount = fourStarRatingCount } + if model.fiveStarRatingCount != fiveStarRatingCount { model.fiveStarRatingCount = fiveStarRatingCount } + if model.ratingDate != ratingDate { model.ratingDate = ratingDate } + if model.observedAt != observedAt { model.observedAt = observedAt } + if model.submissionCount != submissionCount { model.submissionCount = submissionCount } + if model.winningCount != winningCount { model.winningCount = winningCount } + if model.confidenceRaw != confidenceRaw { model.confidenceRaw = confidenceRaw } + if model.sourceRaw != sourceRaw { model.sourceRaw = sourceRaw } + if model.storeApp !== storeApp { model.storeApp = storeApp } + } + + func apply(to model: LatestAppRating, storeApp: StoreApp?) { + if model.ratingCount != ratingCount { model.ratingCount = ratingCount } + if model.averageRating != averageRating { model.averageRating = averageRating } + if model.oneStarRatingCount != oneStarRatingCount { model.oneStarRatingCount = oneStarRatingCount } + if model.twoStarRatingCount != twoStarRatingCount { model.twoStarRatingCount = twoStarRatingCount } + if model.threeStarRatingCount != threeStarRatingCount { model.threeStarRatingCount = threeStarRatingCount } + if model.fourStarRatingCount != fourStarRatingCount { model.fourStarRatingCount = fourStarRatingCount } + if model.fiveStarRatingCount != fiveStarRatingCount { model.fiveStarRatingCount = fiveStarRatingCount } + if model.ratingDate != ratingDate { model.ratingDate = ratingDate } + if model.observedAt != observedAt { model.observedAt = observedAt } + if model.submissionCount != submissionCount { model.submissionCount = submissionCount } + if model.winningCount != winningCount { model.winningCount = winningCount } + if model.confidenceRaw != confidenceRaw { model.confidenceRaw = confidenceRaw } + if model.sourceRaw != sourceRaw { model.sourceRaw = sourceRaw } + if model.storeApp !== storeApp { model.storeApp = storeApp } + } + // A versioned positional payload avoids repeating property names and // binary-plist offset tables in each small archive. Nulls keep their slots. init(from decoder: any Decoder) throws { @@ -114,9 +182,11 @@ struct RatingHistoryRecord: Codable, Equatable, Sendable { enum RatingHistoryError: LocalizedError { case invalidArchive(String) + case schemaUpgradeRequired var errorDescription: String? { switch self { + case .schemaUpgradeRequired: "Upgrade this database before writing compressed rating history." case .invalidArchive(let key): "Rating history failed validation: \(key). No unvalidated source rows were removed." } } @@ -152,6 +222,9 @@ enum RatingHistoryStore { throw RatingHistoryError.invalidArchive(chunk.identityKey) } let data = try JSONEncoder().encode(sorted) + guard data.count <= 8 * 1_024 * 1_024 else { + throw RatingHistoryError.invalidArchive(chunk.identityKey) + } let compressed = try (data as NSData).compressed(using: .zlib) as Data guard try JSONDecoder().decode([RatingHistoryRecord].self, from: (compressed as NSData).decompressed(using: .zlib) as Data) == sorted else { @@ -198,45 +271,161 @@ enum RatingHistoryStore { } } - /// Restore an archived day before an existing write path changes it. Both - /// removal from the chunk and the new row commit with the caller's save. - static func restore(identityKey: String, in context: ModelContext) throws -> AppDailyRating? { - try restore(identityKeys: [identityKey], in: context)[identityKey] + struct Observation: Sendable { + let record: RatingHistoryRecord + let replacesStarCounts: Bool + + init(_ result: AppStorefrontRatingResult, ratingDate: String? = nil, + submissionCount: Int = 1, winningCount: Int = 1, confidence: String? = "single_source") { + record = RatingHistoryRecord(result, ratingDate: ratingDate, submissionCount: submissionCount, + winningCount: winningCount, confidence: confidence) + replacesStarCounts = result.ratingCounts != nil + } } - static func restore(identityKeys: [String], in context: ModelContext) throws -> [String: AppDailyRating] { - guard !identityKeys.isEmpty else { return [:] } - let existing = try context.fetch(FetchDescriptor(predicate: #Predicate { - identityKeys.contains($0.identityKey) - })) - var models = Dictionary(uniqueKeysWithValues: existing.map { ($0.identityKey, $0) }) - let missing = Set(identityKeys).subtracting(models.keys) - let keys = Array(Set(missing.compactMap { key -> String? in - let parts = key.components(separatedBy: "::") - guard parts.count == 3, let appID = Int64(parts[0]) else { return nil } - return RatingHistoryChunk.key(appStoreID: appID, storefront: parts[1], year: String(parts[2].prefix(4))) - })) - guard !keys.isEmpty else { return models } - let chunks = try context.fetch(FetchDescriptor(predicate: #Predicate { - keys.contains($0.identityKey) + /// Mutate only the caller's transaction. One live logical day per scope; + /// rollover and historical corrections update only the affected archives. + /// The caller commits or rolls back these changes with its other work. + /// Shared stores use BackgroundModelStore.write for transaction-wide access. + static func ingest( + _ observations: [Observation], storeApps: [Int64: StoreApp], + acceptsEqualTimestamps: Bool = true, in context: ModelContext + ) throws { + guard !observations.isEmpty else { return } + guard context.container.schema.entities.contains(where: { $0.name == "RatingHistoryChunk" }) else { + throw RatingHistoryError.schemaUpgradeRequired + } + let scopes = Dictionary(grouping: observations, by: { $0.record.scopeKey }) + let appIDs = Array(Set(observations.map { $0.record.appStoreID })) + let storefronts = Array(Set(observations.map { $0.record.storefront })) + var descriptor = FetchDescriptor(predicate: #Predicate { + appIDs.contains($0.appStoreID) && storefronts.contains($0.storefront) + }) + descriptor.relationshipKeyPathsForPrefetching = [\.storeApp] + let live = try context.fetch(descriptor).filter { scopes[LatestAppRating.makeIdentityKey( + appStoreID: $0.appStoreID, storefront: $0.storefront)] != nil } + let liveByScope = Dictionary(grouping: live, by: { + LatestAppRating.makeIdentityKey(appStoreID: $0.appStoreID, storefront: $0.storefront) + }) + let chunkKeys = Array(Set(observations.map { $0.record.chunkKey } + live.map { RatingHistoryRecord($0).chunkKey })) + let archives = try ArchiveEdits(keys: chunkKeys, in: context) + let latestValues = try context.fetch(FetchDescriptor(predicate: #Predicate { + appIDs.contains($0.appStoreID) && storefronts.contains($0.storefront) })) - let decoded = try chunks.map { ($0, try records(in: $0)) } - let storeIDs = Array(Set(decoded.flatMap { $0.1 }.compactMap(\.storeAppID))) - let apps = try context.fetch(FetchDescriptor(predicate: #Predicate { storeIDs.contains($0.appStoreID) })) - let appsByID = Dictionary(uniqueKeysWithValues: apps.map { ($0.appStoreID, $0) }) - for (chunk, values) in decoded { - let restored = values.filter { missing.contains($0.identityKey) } - guard !restored.isEmpty else { continue } - for value in restored { - let model = value.makeModel(storeApp: value.storeAppID.flatMap { appsByID[$0] }) - context.insert(model) - models[value.identityKey] = model + let latestByScope = Dictionary(uniqueKeysWithValues: latestValues.map { ($0.identityKey, $0) }) + var hotUpdates: [(RatingHistoryRecord, AppDailyRating?)] = [] + var obsolete: [AppDailyRating] = [] + var latestUpdates: [String: RatingHistoryRecord] = [:] + + for (scope, incoming) in scopes { + let rows = liveByScope[scope] ?? [] + let hotRow = rows.max { $0.ratingDate < $1.ratingDate } + var hot = hotRow.map(RatingHistoryRecord.init) + // Covers raw legacy rows in a caller's existing transaction without + // scanning any app/storefront outside this ingestion batch. + for row in rows where row !== hotRow { + try archives.merge(RatingHistoryRecord(row)) + obsolete.append(row) + } + for observation in incoming { + var record = observation.record + let previous = hot?.identityKey == record.identityKey ? hot : try archives.record(record) + if let previous { + guard record.observedAt > previous.observedAt + || (acceptsEqualTimestamps && record.observedAt == previous.observedAt) else { continue } + if !observation.replacesStarCounts { record.retainStarCounts(from: previous) } + } + if let previous = hot, record.ratingDate > previous.ratingDate { + try archives.merge(previous) + } + if hot == nil || record.ratingDate >= hot!.ratingDate { + hot = record + } else { + try archives.merge(record, replacesEqual: acceptsEqualTimestamps) + } + let latest = latestByScope[scope] + let previousDate = latestUpdates[scope]?.observedAt ?? latest?.observedAt + if let previousDate, record.observedAt < previousDate + || (!acceptsEqualTimestamps && record.observedAt == previousDate) { continue } + var latestRecord = observation.record + if !observation.replacesStarCounts { + if let previous = latestUpdates[scope] { latestRecord.retainStarCounts(from: previous) } + else if let latest { + latestRecord.oneStarRatingCount = latest.oneStarRatingCount + latestRecord.twoStarRatingCount = latest.twoStarRatingCount + latestRecord.threeStarRatingCount = latest.threeStarRatingCount + latestRecord.fourStarRatingCount = latest.fourStarRatingCount + latestRecord.fiveStarRatingCount = latest.fiveStarRatingCount + } + } + latestUpdates[scope] = latestRecord + } + if let hot { hotUpdates.append((hot, hotRow)) } + } + // Validate every compressed update before changing or removing live rows. + try archives.apply(in: context) + for (record, row) in hotUpdates { + let app = record.storeAppID.flatMap { storeApps[$0] } ?? row?.storeApp + if let row { record.apply(to: row, storeApp: app) } + else { context.insert(record.makeModel(storeApp: app)) } + } + for row in obsolete { context.delete(row) } + for (scope, record) in latestUpdates { + let app = record.storeAppID.flatMap { storeApps[$0] } + let latest = latestByScope[scope] ?? LatestAppRating( + appStoreID: record.appStoreID, storefront: record.storefront, + ratingCount: record.ratingCount, averageRating: record.averageRating, storeApp: app + ) + if latest.modelContext == nil { context.insert(latest) } + record.apply(to: latest, storeApp: app) + } + } + + private final class ArchiveEdits { + private let chunks: [String: RatingHistoryChunk] + private var decoded: [String: [String: RatingHistoryRecord]] = [:] + private var changed = Set() + + init(keys: [String], in context: ModelContext) throws { + chunks = Dictionary(uniqueKeysWithValues: try context.fetch(FetchDescriptor( + predicate: #Predicate { keys.contains($0.identityKey) } + )).map { ($0.identityKey, $0) }) + } + + private func values(for key: String) throws -> [String: RatingHistoryRecord] { + if let values = decoded[key] { return values } + let values = try chunks[key].map { chunk in + Dictionary(uniqueKeysWithValues: try records(in: chunk).map { ($0.identityKey, $0) }) + } ?? [:] + decoded[key] = values + return values + } + + func record(_ record: RatingHistoryRecord) throws -> RatingHistoryRecord? { + try values(for: record.chunkKey)[record.identityKey] + } + + func merge(_ record: RatingHistoryRecord, replacesEqual: Bool = true) throws { + var values = try values(for: record.chunkKey) + if let previous = values[record.identityKey] { + guard record != previous, record.observedAt > previous.observedAt + || (replacesEqual && record.observedAt == previous.observedAt) else { return } + } + values[record.identityKey] = record + decoded[record.chunkKey] = values + changed.insert(record.chunkKey) + } + + func apply(in context: ModelContext) throws { + for key in changed.sorted() { + let values = Array(decoded[key]!.values) + let first = values[0] + let chunk = chunks[key] ?? RatingHistoryChunk(appStoreID: first.appStoreID, + storefront: first.storefront, year: String(first.ratingDate.prefix(4))) + try replace(values, in: chunk) + if chunk.modelContext == nil { context.insert(chunk) } } - let remaining = values.filter { !missing.contains($0.identityKey) } - if remaining.isEmpty { context.delete(chunk) } - else { try replace(remaining, in: chunk) } } - return models } static func remove(appStoreID: Int64, storefront: String, in context: ModelContext) throws { diff --git a/OpenASO/Services/Persistence/StorageSchemaV7Migrator.swift b/OpenASO/Services/Persistence/StorageSchemaV7Migrator.swift index 6ed80e8..c1cd9f5 100644 --- a/OpenASO/Services/Persistence/StorageSchemaV7Migrator.swift +++ b/OpenASO/Services/Persistence/StorageSchemaV7Migrator.swift @@ -62,13 +62,7 @@ enum StorageSchemaV7Migrator { state.setValue(true, forKey: "needsCompaction") try context.save() } - if state.value(forKey: "completed") as? Bool == true { - let previous = state.value(forKey: "lastMaintenanceAt") as? Date ?? .distantPast - guard now.timeIntervalSince(previous) >= 86_400 else { return true } - state.setValue(false, forKey: "completed") - state.setValue(true, forKey: "needsCompaction") - state.setValue(nil, forKey: "lastIdentityKey") - } + if state.value(forKey: "completed") as? Bool == true { return true } if let pending = state.value(forKey: "pendingDeletionPayload") as? Data { try finishDeletion(JSONDecoder().decode(PendingDeletion.self, from: pending), state: state, in: context) return false diff --git a/OpenASO/Services/RatingsReviews/AppStorefrontRatingService.swift b/OpenASO/Services/RatingsReviews/AppStorefrontRatingService.swift index 48c41ba..629037c 100644 --- a/OpenASO/Services/RatingsReviews/AppStorefrontRatingService.swift +++ b/OpenASO/Services/RatingsReviews/AppStorefrontRatingService.swift @@ -176,15 +176,8 @@ final class AppStorefrontRatingService: Sendable { in modelContext: ModelContext ) throws { if let result = outcome.result { - try upsert( - result, - ratingDate: nil, - submissionCount: 1, - winningCount: 1, - confidence: "single_source", - for: storeApp, - in: modelContext - ) + try RatingHistoryStore.ingest([.init(result)], + storeApps: [storeApp.appStoreID: storeApp], in: modelContext) } if outcome.clearsStoredRatings { @@ -206,129 +199,6 @@ final class AppStorefrontRatingService: Sendable { return Array(Set(normalizedStorefronts)).sorted() } - private func upsert( - _ result: AppStorefrontRatingResult, - ratingDate: String?, - submissionCount: Int, - winningCount: Int, - confidence: String?, - for storeApp: StoreApp, - in modelContext: ModelContext - ) throws { - let resolvedRatingDate = ratingDate ?? LatestAppRating.ratingDateString(for: result.observedAt) - let snapshotKey = AppDailyRating.makeIdentityKey( - appStoreID: result.appStoreID, - storefront: result.storefront, - ratingDate: resolvedRatingDate - ) - let snapshot: AppDailyRating - if let existing = try fetchSnapshot(identityKey: snapshotKey, in: modelContext) { - snapshot = existing - guard result.observedAt >= existing.observedAt else { - return - } - } else { - snapshot = AppDailyRating( - appStoreID: result.appStoreID, - storefront: result.storefront, - ratingCount: result.ratingCount, - averageRating: result.averageRating, - oneStarRatingCount: result.ratingCounts?.oneStar, - twoStarRatingCount: result.ratingCounts?.twoStar, - threeStarRatingCount: result.ratingCounts?.threeStar, - fourStarRatingCount: result.ratingCounts?.fourStar, - fiveStarRatingCount: result.ratingCounts?.fiveStar, - ratingDate: resolvedRatingDate, - observedAt: result.observedAt, - submissionCount: submissionCount, - winningCount: winningCount, - confidence: confidence, - source: result.source, - storeApp: storeApp - ) - modelContext.insert(snapshot) - } - snapshot.ratingCount = result.ratingCount - snapshot.averageRating = result.averageRating - if let ratingCounts = result.ratingCounts { - snapshot.oneStarRatingCount = ratingCounts.oneStar - snapshot.twoStarRatingCount = ratingCounts.twoStar - snapshot.threeStarRatingCount = ratingCounts.threeStar - snapshot.fourStarRatingCount = ratingCounts.fourStar - snapshot.fiveStarRatingCount = ratingCounts.fiveStar - } - snapshot.ratingDate = resolvedRatingDate - snapshot.observedAt = result.observedAt - snapshot.submissionCount = submissionCount - snapshot.winningCount = winningCount - snapshot.confidenceRaw = confidence - snapshot.source = result.source - snapshot.storeApp = storeApp - - let identityKey = LatestAppRating.makeIdentityKey( - appStoreID: result.appStoreID, - storefront: result.storefront - ) - let latest: LatestAppRating - if let existing = try? fetchLatest(identityKey: identityKey, in: modelContext) { - latest = existing - guard result.observedAt >= existing.observedAt else { - return - } - } else { - latest = LatestAppRating( - appStoreID: result.appStoreID, - storefront: result.storefront, - ratingCount: result.ratingCount, - averageRating: result.averageRating, - oneStarRatingCount: result.ratingCounts?.oneStar, - twoStarRatingCount: result.ratingCounts?.twoStar, - threeStarRatingCount: result.ratingCounts?.threeStar, - fourStarRatingCount: result.ratingCounts?.fourStar, - fiveStarRatingCount: result.ratingCounts?.fiveStar, - ratingDate: resolvedRatingDate, - observedAt: result.observedAt, - submissionCount: submissionCount, - winningCount: winningCount, - confidence: confidence, - source: result.source, - storeApp: storeApp - ) - modelContext.insert(latest) - } - - latest.ratingCount = result.ratingCount - latest.averageRating = result.averageRating - if let ratingCounts = result.ratingCounts { - latest.oneStarRatingCount = ratingCounts.oneStar - latest.twoStarRatingCount = ratingCounts.twoStar - latest.threeStarRatingCount = ratingCounts.threeStar - latest.fourStarRatingCount = ratingCounts.fourStar - latest.fiveStarRatingCount = ratingCounts.fiveStar - } - latest.ratingDate = resolvedRatingDate - latest.observedAt = result.observedAt - latest.submissionCount = submissionCount - latest.winningCount = winningCount - latest.confidenceRaw = confidence - latest.source = result.source - latest.storeApp = storeApp - } - - private func fetchLatest(identityKey: String, in modelContext: ModelContext) throws -> LatestAppRating? { - let targetIdentityKey = identityKey - let descriptor = FetchDescriptor( - predicate: #Predicate { latest in - latest.identityKey == targetIdentityKey - } - ) - return try modelContext.fetch(descriptor).first - } - - private func fetchSnapshot(identityKey: String, in modelContext: ModelContext) throws -> AppDailyRating? { - try RatingHistoryStore.restore(identityKey: identityKey, in: modelContext) - } - private func clearRatings(appStoreID: Int64, storefront: String, in modelContext: ModelContext) throws { let targetAppStoreID = appStoreID let targetStorefront = storefront diff --git a/OpenASO/Services/SearchRanking/RankingRefreshCoordinator.swift b/OpenASO/Services/SearchRanking/RankingRefreshCoordinator.swift index 8393136..953d3a8 100644 --- a/OpenASO/Services/SearchRanking/RankingRefreshCoordinator.swift +++ b/OpenASO/Services/SearchRanking/RankingRefreshCoordinator.swift @@ -680,12 +680,8 @@ final class RankingRefreshCoordinator: Sendable { storefrontCode: pageResult.request.storefront, in: modelContext ) - var ratingCache = try makeRatingPageCache( - items: catalogItems, - storefront: pageResult.request.storefront, - observedAt: pageResult.searchedAt, - in: modelContext - ) + var ratingObservations: [RatingHistoryStore.Observation] = [] + var ratingApps: [Int64: StoreApp] = [:] let currentObservationKey = observation.observationKey let existingObservationItems = try modelContext.fetch( FetchDescriptor( @@ -714,15 +710,14 @@ final class RankingRefreshCoordinator: Sendable { in: modelContext, cache: &catalogCache ) - upsertStorefrontRating( - from: item, - storefront: pageResult.request.storefront, - observedAt: pageResult.searchedAt, - source: storefrontRatingSource(for: pageResult.page.source), - storeApp: storeApp, - in: modelContext, - cache: &ratingCache - ) + if !observation.storefront.isEmpty && (item.ratingCount != nil || item.averageRating != nil) { + ratingObservations.append(.init(.init( + appStoreID: item.appStoreID, storefront: pageResult.request.storefront, + ratingCount: item.ratingCount, averageRating: item.averageRating, ratingCounts: nil, + observedAt: pageResult.searchedAt, source: storefrontRatingSource(for: pageResult.page.source) + ))) + ratingApps[item.appStoreID] = storeApp + } } let revisionKey = RankingAppRevisionPayload(item).revisionKey guard let revision = revisionsByKey[revisionKey] else { @@ -736,6 +731,8 @@ final class RankingRefreshCoordinator: Sendable { itemsByAppStoreID: &observationItemsByAppStoreID ) } + try RatingHistoryStore.ingest(ratingObservations, storeApps: ratingApps, + acceptsEqualTimestamps: false, in: modelContext) pruneObservationItems( Array(observationItemsByAppStoreID.values), keeping: pageResult.page.items.map(\.appStoreID), @@ -1038,154 +1035,6 @@ final class RankingRefreshCoordinator: Sendable { .max { $0.observedAt < $1.observedAt } } - private struct RatingPageCache { - var latestByIdentityKey: [String: LatestAppRating] - var snapshotsByIdentityKey: [String: AppDailyRating] - } - - private func makeRatingPageCache( - items: [SearchRankingItem], - storefront: String, - observedAt: Date, - in modelContext: ModelContext - ) throws -> RatingPageCache { - let normalizedStorefront = storefront.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() - guard !normalizedStorefront.isEmpty else { - return RatingPageCache(latestByIdentityKey: [:], snapshotsByIdentityKey: [:]) - } - - let ratingDate = LatestAppRating.ratingDateString(for: observedAt) - let appStoreIDs = Set(items.lazy.filter { - $0.ratingCount != nil || $0.averageRating != nil - }.map(\.appStoreID)) - let latestKeys = appStoreIDs.map { - LatestAppRating.makeIdentityKey(appStoreID: $0, storefront: normalizedStorefront) - } - let snapshotKeys = appStoreIDs.map { - AppDailyRating.makeIdentityKey(appStoreID: $0, storefront: normalizedStorefront, ratingDate: ratingDate) - } - - return RatingPageCache( - latestByIdentityKey: try fetchLatestRatings(identityKeys: latestKeys, in: modelContext), - snapshotsByIdentityKey: try fetchRatingSnapshots(identityKeys: snapshotKeys, in: modelContext) - ) - } - - private func upsertStorefrontRating( - from item: SearchRankingItem, - storefront: String, - observedAt: Date, - source: AppStorefrontSource, - storeApp: StoreApp, - in modelContext: ModelContext, - cache: inout RatingPageCache - ) { - guard item.ratingCount != nil || item.averageRating != nil else { return } - - let normalizedStorefront = storefront.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() - guard !normalizedStorefront.isEmpty else { return } - - let ratingDate = LatestAppRating.ratingDateString(for: observedAt) - let snapshotKey = AppDailyRating.makeIdentityKey( - appStoreID: item.appStoreID, - storefront: normalizedStorefront, - ratingDate: ratingDate - ) - let existingSnapshot = cache.snapshotsByIdentityKey[snapshotKey] - let isNewSnapshot = existingSnapshot == nil - let snapshot = existingSnapshot ?? AppDailyRating( - appStoreID: item.appStoreID, - storefront: normalizedStorefront, - ratingCount: item.ratingCount, - averageRating: item.averageRating, - ratingDate: ratingDate, - observedAt: observedAt, - submissionCount: 1, - winningCount: 1, - confidence: "single_source", - source: source, - storeApp: storeApp - ) - if !isNewSnapshot, observedAt <= snapshot.observedAt { - return - } - if isNewSnapshot { - modelContext.insert(snapshot) - cache.snapshotsByIdentityKey[snapshotKey] = snapshot - } - if snapshot.storeApp !== storeApp { - snapshot.storeApp = storeApp - } - let snapshotChanged = isNewSnapshot - || snapshot.observedAt != observedAt - || snapshot.ratingCount != item.ratingCount - || snapshot.averageRating != item.averageRating - || snapshot.ratingDate != ratingDate - || snapshot.submissionCount != 1 - || snapshot.winningCount != 1 - || snapshot.confidenceRaw != "single_source" - || snapshot.source != source - if snapshotChanged { - snapshot.ratingCount = item.ratingCount - snapshot.averageRating = item.averageRating - snapshot.ratingDate = ratingDate - snapshot.observedAt = observedAt - snapshot.submissionCount = 1 - snapshot.winningCount = 1 - snapshot.confidenceRaw = "single_source" - snapshot.source = source - } - - let latestKey = LatestAppRating.makeIdentityKey( - appStoreID: item.appStoreID, - storefront: normalizedStorefront - ) - let existingLatest = cache.latestByIdentityKey[latestKey] - let isNewLatest = existingLatest == nil - let latest = existingLatest ?? LatestAppRating( - appStoreID: item.appStoreID, - storefront: normalizedStorefront, - ratingCount: item.ratingCount, - averageRating: item.averageRating, - ratingDate: ratingDate, - observedAt: observedAt, - submissionCount: 1, - winningCount: 1, - confidence: "single_source", - source: source, - storeApp: storeApp - ) - if !isNewLatest, observedAt <= latest.observedAt { - return - } - if isNewLatest { - modelContext.insert(latest) - cache.latestByIdentityKey[latestKey] = latest - } - if latest.storeApp !== storeApp { - latest.storeApp = storeApp - } - let latestChanged = isNewLatest - || latest.observedAt != observedAt - || latest.ratingCount != item.ratingCount - || latest.averageRating != item.averageRating - || latest.ratingDate != ratingDate - || latest.submissionCount != 1 - || latest.winningCount != 1 - || latest.confidenceRaw != "single_source" - || latest.source != source - if latestChanged { - latest.ratingCount = item.ratingCount - latest.averageRating = item.averageRating - latest.ratingDate = ratingDate - latest.observedAt = observedAt - latest.submissionCount = 1 - latest.winningCount = 1 - latest.confidenceRaw = "single_source" - latest.source = source - } - } - private func storefrontRatingSource(for rankingSource: RankingSource) -> AppStorefrontSource { switch rankingSource { case .appStoreWeb: @@ -1195,38 +1044,6 @@ final class RankingRefreshCoordinator: Sendable { } } - private func fetchLatestRating(identityKey: String, in modelContext: ModelContext) throws -> LatestAppRating? { - let targetIdentityKey = identityKey - var descriptor = FetchDescriptor( - predicate: #Predicate { latest in - latest.identityKey == targetIdentityKey - } - ) - descriptor.fetchLimit = 1 - return try modelContext.fetch(descriptor).first - } - - private func fetchRatingSnapshot(identityKey: String, in modelContext: ModelContext) throws -> AppDailyRating? { - try RatingHistoryStore.restore(identityKey: identityKey, in: modelContext) - } - - private func fetchLatestRatings(identityKeys: [String], in modelContext: ModelContext) throws -> [String: LatestAppRating] { - guard !identityKeys.isEmpty else { return [:] } - - let targetIdentityKeys = identityKeys - let descriptor = FetchDescriptor( - predicate: #Predicate { latest in - targetIdentityKeys.contains(latest.identityKey) - } - ) - - return Dictionary(uniqueKeysWithValues: try modelContext.fetch(descriptor).map { ($0.identityKey, $0) }) - } - - private func fetchRatingSnapshots(identityKeys: [String], in modelContext: ModelContext) throws -> [String: AppDailyRating] { - try RatingHistoryStore.restore(identityKeys: identityKeys, in: modelContext) - } - @MainActor func refresh( tracks: [TrackedAppKeyword], diff --git a/OpenASOTests/AppStorefrontRatingServiceTests.swift b/OpenASOTests/AppStorefrontRatingServiceTests.swift index 74ced4e..0957497 100644 --- a/OpenASOTests/AppStorefrontRatingServiceTests.swift +++ b/OpenASOTests/AppStorefrontRatingServiceTests.swift @@ -654,6 +654,7 @@ private func makeRatingContainer() throws -> ModelContainer { AppKeywordStats.self, LatestAppRating.self, AppDailyRating.self, + RatingHistoryChunk.self, AppStorefrontReview.self, StoreApp.self, AppStorefrontMetadata.self, diff --git a/OpenASOTests/RankingRefreshCoordinatorTests.swift b/OpenASOTests/RankingRefreshCoordinatorTests.swift index 2c5cc7f..8b81d42 100644 --- a/OpenASOTests/RankingRefreshCoordinatorTests.swift +++ b/OpenASOTests/RankingRefreshCoordinatorTests.swift @@ -7,6 +7,42 @@ import Testing @MainActor struct RankingRefreshCoordinatorTests { + @Test + func rankingIngestionArchivesOnRolloverAndUpdatesOlderDaysInPlace() throws { + let container = try makeInMemoryContainer() + let context = ModelContext(container) + let tracked = TrackedApp(appStoreID: 100, bundleID: "example", name: "Target", + sellerName: nil, defaultPlatform: .iphone) + let track = try makeTrackedAppKeyword(term: "notes", platform: .iphone, trackedApp: tracked, in: context) + context.insert(tracked) + context.insert(track) + try context.save() + let coordinator = RankingRefreshCoordinator( + rankingProvider: StubRankingProvider(page: SearchRankingPage(items: [], source: .appStoreWeb)), + appCatalogService: AppCatalogService(appResolver: StubAppResolver()) + ) + let start = try Date("2026-12-31T15:00:00Z", strategy: .iso8601) + for (offset, seconds, count) in [(0, 0, 100), (1, 0, 200), (2, 0, 200), (0, 3_600, 150)] { + let date = start.addingTimeInterval(Double(offset * 86_400 + seconds)) + _ = try coordinator.persistRankingPage(.init( + request: RankingRefreshRequest(track: track), + page: SearchRankingPage(items: [.init(position: 1, appStoreID: 200, + bundleID: "competitor", name: "Competitor", sellerName: nil, + ratingCount: count, averageRating: 4.5)], source: .appStoreWeb), + searchedAt: date, observedHour: nil, submissionCount: 1, winningCount: 1, confidence: "single_source" + ), in: context, scheduleMetadataEnrichment: false) + } + let history = try RatingHistoryStore.fetch(appStoreIDs: [200], in: context) + #expect(history.count == 3) + #expect(history.first { $0.ratingDate == "2026-12-31" }?.ratingCount == 150) + #expect(try context.fetchCount(FetchDescriptor()) == 1) + #expect(try context.fetch(FetchDescriptor()).first?.ratingDate == "2027-01-02") + #expect(try context.fetch(FetchDescriptor()).first?.ratingCount == 200) + let chunks = try context.fetch(FetchDescriptor()) + #expect(Set(chunks.map(\.year)) == ["2026", "2027"]) + #expect(chunks.reduce(0) { $0 + $1.recordCount } == 2) + } + @Test func rankingRequestGroupsNormalizeEquivalentQueriesAndPreserveFanOutMetadata() throws { let requests = [ @@ -3004,6 +3040,7 @@ private func makeInMemoryContainer() throws -> ModelContainer { AppKeywordStats.self, LatestAppRating.self, AppDailyRating.self, + RatingHistoryChunk.self, AppStorefrontReview.self, StoreApp.self, AppStorefrontMetadata.self, diff --git a/OpenASOTests/StorageSchemaV7MigrationTests.swift b/OpenASOTests/StorageSchemaV7MigrationTests.swift index 65c3111..f992dd6 100644 --- a/OpenASOTests/StorageSchemaV7MigrationTests.swift +++ b/OpenASOTests/StorageSchemaV7MigrationTests.swift @@ -27,10 +27,14 @@ struct StorageSchemaV7MigrationTests { #expect(try RatingHistoryStore.fetch(appStoreIDs: [42], in: migrated) == expected) let key = "42::gb::2026-01-01" - let restored = try #require(try RatingHistoryStore.restore(identityKey: key, in: migrated)) - #expect(restored.storeApp?.appStoreID == 42) - #expect(restored.sourceRaw == "future-source") - restored.ratingCount = 999 + let archived = try #require(expected.first { $0.identityKey == key }) + #expect(archived.storeAppID == 42) + #expect(archived.sourceRaw == "future-source") + try RatingHistoryStore.ingest([.init(.init( + appStoreID: 42, storefront: "gb", ratingCount: 999, averageRating: archived.averageRating, + ratingCounts: nil, observedAt: archived.observedAt.addingTimeInterval(1), source: .iTunesSearch + ), ratingDate: archived.ratingDate, submissionCount: 7, winningCount: 6, confidence: "correction")], + storeApps: [42: try #require(migrated.fetch(FetchDescriptor()).first)], in: migrated) try migrated.save() let reread = try RatingHistoryStore.fetch(appStoreIDs: [42], in: ModelContext(container)) #expect(reread.count == 2) @@ -191,8 +195,16 @@ struct StorageSchemaV7MigrationTests { context.insert(makeRating(day: "2026-01-01", observedAt: .now, app: nil)) context.insert(makeRating(day: "2026-01-02", observedAt: .now, app: nil)) try context.save() - // Run a later maintenance pass on this private test store. - try StorageSchemaV7Migrator.migrateIfNeeded(at: url, now: .now.addingTimeInterval(86_401)) + // A completed upgrade never reruns. New observations archive on ingestion. + let service = AppStorefrontRatingService(httpClient: PreviewHTTPClient()) + let app = StoreApp(appStoreID: 84, bundleID: nil, name: "New app", sellerName: nil, + iconURLString: nil, defaultPlatform: .iphone) + context.insert(app) + let start = try Date("2026-02-01T15:00:00Z", strategy: .iso8601) + try ingestRating(service, app: app, observedAt: start, count: 100, in: context) + try ingestRating(service, app: app, observedAt: start.addingTimeInterval(86_400), count: 101, in: context) + try context.save() + #expect(try context.fetchCount(FetchDescriptor()) == 1) } #expect(throws: (any Error).self) { try autoreleasepool { @@ -276,6 +288,277 @@ struct StorageSchemaV7MigrationTests { return String(cString: value) } + @Test + func ingestionKeepsOneLiveDayAndCorrectsArchivesWithoutMaintenance() throws { + let container = try ModelContainerFactory.makeModelContainer(isStoredInMemoryOnly: true) + let context = ModelContext(container) + let app = StoreApp(appStoreID: 42, bundleID: "example", name: "App", sellerName: nil, + iconURLString: nil, defaultPlatform: .iphone) + context.insert(app) + let service = AppStorefrontRatingService(httpClient: PreviewHTTPClient()) + let start = try Date("2026-01-01T15:00:00Z", strategy: .iso8601) + for offset in 0..<3 { + try ingestRating(service, app: app, observedAt: start.addingTimeInterval(Double(offset) * 86_400), + count: offset == 0 ? 100 : 200, in: context) + try context.save() + } + let expected = try RatingHistoryStore.fetch(appStoreIDs: [42], in: context) + #expect(expected.count == 3) // Equal values on different days remain distinct history. + #expect(try context.fetchCount(FetchDescriptor()) == 1) + let chunk = try #require(try context.fetch(FetchDescriptor()).first) + #expect(chunk.recordCount == 2) + #expect(expected.first { $0.ratingDate == "2026-01-01" }?.fiveStarRatingCount == 90) + + try ingestRating(service, app: app, observedAt: start.addingTimeInterval(3_600), count: 150, in: context) + try context.save() + let corrected = try RatingHistoryStore.fetch(appStoreIDs: [42], in: context) + #expect(corrected.count == 3) + #expect(corrected.first { $0.ratingDate == "2026-01-01" }?.ratingCount == 150) + #expect(try context.fetchCount(FetchDescriptor()) == 1) + #expect(chunk.recordCount == 2) + #expect(try context.fetch(FetchDescriptor()).first?.ratingCount == 200) + let digest = chunk.digest + try ingestRating(service, app: app, observedAt: start, count: 99, in: context) + #expect(!context.hasChanges) + #expect(chunk.digest == digest) + try ingestRating(service, app: app, observedAt: start.addingTimeInterval(3_600), count: 150, in: context) + #expect(!context.hasChanges) + #expect(chunk.digest == digest) + } + + @Test + func backfillAndYearRolloverStayCompactAndRollbackRetainsHistory() throws { + let container = try ModelContainerFactory.makeModelContainer(isStoredInMemoryOnly: true) + let context = ModelContext(container) + let app = StoreApp(appStoreID: 42, bundleID: nil, name: "App", sellerName: nil, + iconURLString: nil, defaultPlatform: .iphone) + context.insert(app) + let service = AppStorefrontRatingService(httpClient: PreviewHTTPClient()) + let start = try Date("2026-12-31T15:00:00Z", strategy: .iso8601) + for offset in [0, -1, 1, 2] { + try ingestRating(service, app: app, observedAt: start.addingTimeInterval(Double(offset) * 86_400), + count: 100 + offset, in: context) + } + try context.save() + let before = try RatingHistoryStore.fetch(appStoreIDs: [42], in: context) + #expect(before.count == 4) + #expect(try context.fetchCount(FetchDescriptor()) == 1) + let chunks = try context.fetch(FetchDescriptor()) + #expect(Set(chunks.map(\.year)) == ["2026", "2027"]) + #expect(chunks.reduce(0) { $0 + $1.recordCount } == 3) + try ingestRating(service, app: app, observedAt: start.addingTimeInterval(3 * 86_400), count: 999, in: context) + context.rollback() + #expect(try RatingHistoryStore.fetch(appStoreIDs: [42], in: context) == before) + #expect(try context.fetchCount(FetchDescriptor()) == 1) + } + + @Test + func completedMigrationNeverRestartsAnArchivalScan() throws { + try withTestStore { container, url in + let context = ModelContext(container) + context.insert(makeRating(day: "2026-01-01", observedAt: .now, app: nil)) + context.insert(makeRating(day: "2026-01-02", observedAt: .now, app: nil)) + try context.save() + try StorageSchemaV7Migrator.migrateIfNeeded(at: url) + let migrated = ModelContext(container) + let state = try #require(try migrated.fetch(FetchDescriptor()).first) + let completion = state.lastMaintenanceAt + let cursor = state.lastIdentityKey + let count = state.archivedRecordCount + // A deliberately raw insert detects accidental rescanning, independently of ingestion. + migrated.insert(makeRating(day: "2026-01-03", observedAt: .now, app: nil)) + try migrated.save() + try StorageSchemaV7Migrator.migrateIfNeeded(at: url, now: .now.addingTimeInterval(366 * 86_400)) + let reread = ModelContext(container) + let completed = try #require(try reread.fetch(FetchDescriptor()).first) + #expect(completed.completed) + #expect(completed.lastMaintenanceAt == completion) + #expect(completed.lastIdentityKey == cursor) + #expect(completed.archivedRecordCount == count) + #expect(try reread.fetchCount(FetchDescriptor()) == 2) + } + } + + @Test + func ingestionPreservesLongHistoriesAndStorefrontIsolation() throws { + let container = try ModelContainerFactory.makeModelContainer(isStoredInMemoryOnly: true) + let context = ModelContext(container) + let app = StoreApp(appStoreID: 42, bundleID: nil, name: "App", sellerName: nil, + iconURLString: nil, defaultPlatform: .iphone) + context.insert(app) + let start = try Date("2024-01-01T15:00:00Z", strategy: .iso8601) + var observations: [RatingHistoryStore.Observation] = [] + for day in 0..<800 { + for storefront in ["gb", "us"] { + observations.append(.init(.init(appStoreID: 42, storefront: storefront, + ratingCount: day, averageRating: 4.5, ratingCounts: nil, + observedAt: start.addingTimeInterval(Double(day) * 86_400), source: .iTunesSearch))) + } + } + try RatingHistoryStore.ingest(observations, storeApps: [42: app], in: context) + try context.save() + let records = try RatingHistoryStore.fetch(appStoreIDs: [42], in: context) + #expect(records.count == 1_600) + #expect(try context.fetchCount(FetchDescriptor()) == 2) + #expect(try context.fetch(FetchDescriptor()).reduce(0) { $0 + $1.recordCount } == 1_598) + #expect(Set(records.filter { $0.storefront == "gb" }.compactMap(\.ratingCount)) == Set(0..<800)) + #expect(records.allSatisfy { $0.storeAppID == 42 && $0.sourceRaw == AppStorefrontSource.iTunesSearch.rawValue }) + } + + @Test + func corruptArchiveRejectsRolloverBeforeReplacingLiveHistory() throws { + let container = try ModelContainerFactory.makeModelContainer(isStoredInMemoryOnly: true) + let context = ModelContext(container) + let app = StoreApp(appStoreID: 42, bundleID: nil, name: "App", sellerName: nil, + iconURLString: nil, defaultPlatform: .iphone) + context.insert(app) + let service = AppStorefrontRatingService(httpClient: PreviewHTTPClient()) + let start = try Date("2026-01-01T15:00:00Z", strategy: .iso8601) + try ingestRating(service, app: app, observedAt: start, count: 100, in: context) + let corrupt = RatingHistoryChunk(appStoreID: 42, storefront: "gb", year: "2026") + corrupt.recordCount = 1 + corrupt.payload = Data([1, 2, 3]) + context.insert(corrupt) + try context.save() + #expect(throws: (any Error).self) { + try ingestRating(service, app: app, observedAt: start.addingTimeInterval(86_400), count: 200, in: context) + } + context.rollback() + let live = try #require(try context.fetch(FetchDescriptor()).first) + #expect(live.ratingDate == "2026-01-01") + #expect(live.ratingCount == 100) + #expect(try context.fetchCount(FetchDescriptor()) == 1) + #expect(try context.fetch(FetchDescriptor()).first?.observedAt == start) + } + + @Test + func reopeningCompletedUpgradeDoesNotScanEvenWithAnOldCompletionDate() throws { + let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-NoRescan-\(UUID())") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let url = root.appendingPathComponent("default.store") + try autoreleasepool { + let container = try ModelContainerFactory.makePersistentModelContainer(at: url) + let context = ModelContext(container) + let state = try #require(try context.fetch(FetchDescriptor()).first) + state.lastMaintenanceAt = .distantPast + for day in ["2026-01-01", "2026-01-02"] { + context.insert(makeRating(day: day, observedAt: .now, app: nil)) + } + try context.save() + } + try autoreleasepool { + let container = try ModelContainerFactory.makePersistentModelContainer(at: url) + let context = ModelContext(container) + let state = try #require(try context.fetch(FetchDescriptor()).first) + #expect(state.lastMaintenanceAt == .distantPast) + #expect(state.completed && !state.needsCompaction) + #expect(try context.fetchCount(FetchDescriptor()) == 0) + #expect(try context.fetchCount(FetchDescriptor()) == 2) + } + } + + @Test + func ingestionRejectsAnUnupgradedSchemaBeforeChangingRows() throws { + let schema = Schema(versionedSchema: OpenASOSchemaV6.self) + let container = try ModelContainer(for: schema, + configurations: ModelConfiguration(schema: schema, isStoredInMemoryOnly: true)) + let context = ModelContext(container) + let app = StoreApp(appStoreID: 42, bundleID: nil, name: "App", sellerName: nil, + iconURLString: nil, defaultPlatform: .iphone) + context.insert(app) + let original = makeRating(day: "2026-01-01", observedAt: .now, app: app) + context.insert(original) + try context.save() + let before = RatingHistoryRecord(original) + let service = AppStorefrontRatingService(httpClient: PreviewHTTPClient()) + let nextDay = try Date("2026-01-02T15:00:00Z", strategy: .iso8601) + #expect(throws: RatingHistoryError.self) { + try ingestRating(service, app: app, observedAt: nextDay, count: 200, in: context) + } + #expect(!context.hasChanges) + #expect(RatingHistoryRecord(original) == before) + } + + @Test + func independentWorkersPreserveOverlappingHistoricalCorrectionsAndRollover() async throws { + let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-WriterRace-\(UUID())") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let url = root.appendingPathComponent("default.store") + let container = try ModelContainerFactory.makePersistentModelContainer(at: url) + let otherContainer = try ModelContainerFactory.makePersistentModelContainer(at: url) + let seed = ModelContext(container) + let app = StoreApp(appStoreID: 42, bundleID: nil, name: "App", sellerName: nil, + iconURLString: nil, defaultPlatform: .iphone) + seed.insert(app) + let service = AppStorefrontRatingService(httpClient: PreviewHTTPClient()) + let start = try Date("2026-01-01T15:00:00Z", strategy: .iso8601) + for day in 0..<3 { + try ingestRating(service, app: app, observedAt: start.addingTimeInterval(Double(day) * 86_400), + count: 100 + day, in: seed) + } + try seed.save() + let first = BackgroundModelStore(modelContainer: container) + let second = BackgroundModelStore(modelContainer: otherContainer) + for _ in 0..<3 { + async let firstWrite: Void = first.write { context in + let app = try #require(try context.fetch(FetchDescriptor()).first) + try RatingHistoryStore.ingest([.init(.init(appStoreID: 42, storefront: "gb", + ratingCount: 111, averageRating: 4.5, ratingCounts: nil, + observedAt: start.addingTimeInterval(3_600), source: .iTunesSearch))], + storeApps: [42: app], in: context) + } + async let secondWrite: Void = second.write { context in + let app = try #require(try context.fetch(FetchDescriptor()).first) + try RatingHistoryStore.ingest([ + .init(.init(appStoreID: 42, storefront: "gb", ratingCount: 222, averageRating: 4.5, + ratingCounts: nil, observedAt: start.addingTimeInterval(86_400 + 3_600), source: .iTunesSearch)), + .init(.init(appStoreID: 42, storefront: "gb", ratingCount: 104, averageRating: 4.5, + ratingCounts: nil, observedAt: start.addingTimeInterval(3 * 86_400), source: .iTunesSearch)) + ], storeApps: [42: app], in: context) + } + _ = try await (firstWrite, secondWrite) + } + let history = try RatingHistoryStore.fetch(appStoreIDs: [42], in: ModelContext(container)) + #expect(history.count == 4) + #expect(history.first { $0.ratingDate == "2026-01-01" }?.ratingCount == 111) + #expect(history.first { $0.ratingDate == "2026-01-02" }?.ratingCount == 222) + #expect(history.first { $0.ratingDate == "2026-01-03" }?.ratingCount == 102) + #expect(try ModelContext(container).fetchCount(FetchDescriptor()) == 1) + } + + @Test + func explicitClearRemovesLiveAndArchivedRatingsTogether() throws { + let container = try ModelContainerFactory.makeModelContainer(isStoredInMemoryOnly: true) + let context = ModelContext(container) + let app = StoreApp(appStoreID: 42, bundleID: nil, name: "App", sellerName: nil, + iconURLString: nil, defaultPlatform: .iphone) + context.insert(app) + let service = AppStorefrontRatingService(httpClient: PreviewHTTPClient()) + let start = try Date("2026-01-01T15:00:00Z", strategy: .iso8601) + for day in 0..<3 { + try ingestRating(service, app: app, observedAt: start.addingTimeInterval(Double(day) * 86_400), + count: 100 + day, in: context) + } + try context.save() + try service.persist(.init(storefront: "gb", result: nil, error: nil, clearsStoredRatings: true), + for: app, in: context) + try context.save() + #expect(try RatingHistoryStore.fetch(appStoreIDs: [42], in: context).isEmpty) + #expect(try context.fetchCount(FetchDescriptor()) == 0) + #expect(try context.fetchCount(FetchDescriptor()) == 0) + #expect(try context.fetchCount(FetchDescriptor()) == 0) + } + + private func ingestRating(_ service: AppStorefrontRatingService, app: StoreApp, observedAt: Date, + count: Int, in context: ModelContext) throws { + try service.persist(.init(storefront: "gb", result: .init( + appStoreID: app.appStoreID, storefront: "gb", ratingCount: count, averageRating: 4.5, + ratingCounts: .init(oneStar: 1, twoStar: 2, threeStar: 3, fourStar: 4, fiveStar: 90), + observedAt: observedAt, source: .iTunesSearch), error: nil), for: app, in: context) + } + private func withTestStore(_ body: (ModelContainer, URL) throws -> Void) throws { let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-Archive-\(UUID())") try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) From cd5f7a5e8892eb53933f7131a5ccf3704975d158 Mon Sep 17 00:00:00 2001 From: hubab1 <50897577+hubab1@users.noreply.github.com> Date: Sun, 25 Oct 2026 10:34:52 +0000 Subject: [PATCH 4/6] fix(persistence): Preserve history during recovery Reject shared clients until migration finishes and validate every saved archive before replacing it. Defer disk-full compaction without hiding corruption errors. Keep the pending flag so a later launch can retry safely. --- .../Persistence/ModelContainerFactory.swift | 33 +++- .../PersistentStoreCompactor.swift | 27 +++ .../Persistence/StorageSchemaV7Migrator.swift | 13 +- .../StorageSchemaV7MigrationTests.swift | 163 ++++++++++++++++++ 4 files changed, 227 insertions(+), 9 deletions(-) diff --git a/OpenASO/Services/Persistence/ModelContainerFactory.swift b/OpenASO/Services/Persistence/ModelContainerFactory.swift index 7f69ce1..d1ce460 100644 --- a/OpenASO/Services/Persistence/ModelContainerFactory.swift +++ b/OpenASO/Services/Persistence/ModelContainerFactory.swift @@ -91,13 +91,25 @@ enum ModelContainerFactory { static func makePersistentModelContainer( at storeURL: URL, - opener: ContainerOpener = openModelContainer + opener: ContainerOpener = openModelContainer, + compactStore: (URL) throws -> Void = PersistentStoreCompactor.compactIfNeeded ) throws -> ModelContainer { let schema = Self.schema let configuration = ModelConfiguration(schema: schema, url: storeURL) do { return try withMigrationAccess(for: storeURL) { canMigrate in - guard canMigrate else { return try opener(schema, configuration) } + guard canMigrate else { + let container = try opener(schema, configuration) + let state = try ModelContext(container).fetch(FetchDescriptor()).first + // A failed migration owner also releases its lock. Shared + // access alone does not prove that the upgrade finished. + guard state?.completed == true, state?.pendingDeletionPayload == nil else { + throw NSError(domain: "OpenASO.StorageMigration", code: 2, userInfo: [ + NSLocalizedDescriptionKey: "The database upgrade is unfinished. Quit other OpenASO, refresh, or MCP processes and reopen to resume it." + ]) + } + return container + } let prepared: ModelContainer? = try autoreleasepool { let container = try opener(schema, configuration) try RankingSchemaV6Migrator.migrateIfNeeded(in: container) @@ -112,14 +124,23 @@ enum ModelContainerFactory { } if let prepared { return prepared } try StorageSchemaV7Migrator.migrateIfNeeded(at: storeURL) - if try PersistentStoreCompactor.shouldCompact(at: storeURL) { - try PersistentStoreCompactor.compact(at: storeURL) + var compactionFinished = false + do { + try compactStore(storeURL) + compactionFinished = true + } catch { + guard PersistentStoreCompactor.canRetryLater(error) else { throw error } + // Logical migration has already verified all research data. + // Reopening still has to succeed before returning a usable store. + OpenASOLog.rankingMigration.error("Optional storage compaction deferred: \(String(reflecting: error), privacy: .public)") } let container = try opener(schema, configuration) let context = ModelContext(container) if let state = try context.fetch(FetchDescriptor()).first { - state.needsCompaction = false - try context.save() + if compactionFinished { + state.needsCompaction = false + try context.save() + } } return container } diff --git a/OpenASO/Services/Persistence/PersistentStoreCompactor.swift b/OpenASO/Services/Persistence/PersistentStoreCompactor.swift index 73eb1a8..d763445 100644 --- a/OpenASO/Services/Persistence/PersistentStoreCompactor.swift +++ b/OpenASO/Services/Persistence/PersistentStoreCompactor.swift @@ -4,6 +4,33 @@ import SQLite3 import SwiftData enum PersistentStoreCompactor { + static func compactIfNeeded(at storeURL: URL) throws { + if try shouldCompact(at: storeURL) { try compact(at: storeURL) } + } + + /// Only storage-capacity/access failures are optional. Archive validation, + /// schema incompatibility, and corruption errors must still stop startup. + static func canRetryLater(_ error: any Error) -> Bool { + let error = error as NSError + if error.domain == NSCocoaErrorDomain { + if error.code == CocoaError.fileReadCorruptFile.rawValue { return false } + if [CocoaError.fileWriteOutOfSpace.rawValue, CocoaError.fileWriteNoPermission.rawValue, + CocoaError.fileLocking.rawValue].contains(error.code) { return true } + } + if error.domain == "NSSQLiteErrorDomain" { + // Extended SQLite errors keep the primary code in the low byte. + return [SQLITE_FULL, SQLITE_BUSY, SQLITE_LOCKED, SQLITE_READONLY, SQLITE_PERM] + .contains(Int32(error.code & 0xff)) + } + if let underlying = error.userInfo[NSUnderlyingErrorKey] as? NSError { + return canRetryLater(underlying) + } + if let details = error.userInfo[NSDetailedErrorsKey] as? [NSError], !details.isEmpty { + return details.allSatisfy(canRetryLater) + } + return false + } + static func shouldCompact(at storeURL: URL) throws -> Bool { var database: OpaquePointer? guard sqlite3_open_v2(storeURL.path, &database, SQLITE_OPEN_READONLY, nil) == SQLITE_OK else { diff --git a/OpenASO/Services/Persistence/StorageSchemaV7Migrator.swift b/OpenASO/Services/Persistence/StorageSchemaV7Migrator.swift index c1cd9f5..613fad7 100644 --- a/OpenASO/Services/Persistence/StorageSchemaV7Migrator.swift +++ b/OpenASO/Services/Persistence/StorageSchemaV7Migrator.swift @@ -107,9 +107,16 @@ enum StorageSchemaV7Migrator { for (key, values) in groups { let first = values[0] let archived: RatingHistoryChunk - if let existing = chunks[key] { archived = try archiveValue(existing) } - else { archived = RatingHistoryChunk(appStoreID: first.appStoreID, storefront: first.storefront, year: String(first.ratingDate.prefix(4))) } - var merged = archived.recordCount == 0 ? [:] : Dictionary(uniqueKeysWithValues: try RatingHistoryStore.records(in: archived).map { ($0.identityKey, $0) }) + var merged: [String: RatingHistoryRecord] + if let existing = chunks[key] { + archived = try archiveValue(existing) + // Only a new, unsaved archive may be empty. A stored zero count + // must fail validation rather than discard earlier batches. + merged = Dictionary(uniqueKeysWithValues: try RatingHistoryStore.records(in: archived).map { ($0.identityKey, $0) }) + } else { + archived = RatingHistoryChunk(appStoreID: first.appStoreID, storefront: first.storefront, year: String(first.ratingDate.prefix(4))) + merged = [:] + } for record in values { merged[record.identityKey] = record } try RatingHistoryStore.replace(Array(merged.values), in: archived) let row = chunks[key] ?? NSEntityDescription.insertNewObject(forEntityName: "RatingHistoryChunk", into: context) diff --git a/OpenASOTests/StorageSchemaV7MigrationTests.swift b/OpenASOTests/StorageSchemaV7MigrationTests.swift index f992dd6..a8cad0c 100644 --- a/OpenASOTests/StorageSchemaV7MigrationTests.swift +++ b/OpenASOTests/StorageSchemaV7MigrationTests.swift @@ -551,6 +551,169 @@ struct StorageSchemaV7MigrationTests { #expect(try context.fetchCount(FetchDescriptor()) == 0) } + @Test + func migrationRejectsZeroCountExistingArchiveInsteadOfReplacingItsHistory() throws { + try withTestStore { container, url in + let context = ModelContext(container) + let prior = makeRating(day: "2026-01-01", observedAt: Date(timeIntervalSince1970: 100), app: nil) + let chunk = RatingHistoryChunk(appStoreID: 42, storefront: "gb", year: "2026") + try RatingHistoryStore.replace([RatingHistoryRecord(prior)], in: chunk) + let payload = chunk.payload + chunk.recordCount = 0 // Corrupt metadata must never mean an existing archive is empty. + context.insert(chunk) + context.insert(makeRating(day: "2026-01-02", observedAt: .now, app: nil)) + context.insert(makeRating(day: "2026-01-03", observedAt: .now, app: nil)) + try context.save() + #expect(throws: (any Error).self) { try StorageSchemaV7Migrator.migrateIfNeeded(at: url) } + let reread = ModelContext(container) + #expect(try reread.fetchCount(FetchDescriptor()) == 2) + #expect(try reread.fetch(FetchDescriptor()).first?.payload == payload) + } + } + + @Test + func waitingClientRejectsAnUnfinishedUpgradeAndLaterResumePreservesHistory() throws { + let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-UnfinishedOwner-\(UUID())") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let url = root.appendingPathComponent("default.store") + let expected = try autoreleasepool { + let owner = try ModelContainerFactory.makePersistentModelContainer(at: url) + let context = ModelContext(owner) + let state = try #require(try context.fetch(FetchDescriptor()).first) + state.completed = false + state.needsCompaction = true + let app = StoreApp(appStoreID: 42, bundleID: nil, name: "App", sellerName: nil, + iconURLString: nil, defaultPlatform: .iphone) + context.insert(app) + context.insert(makeRating(day: "2026-01-01", observedAt: .now, app: app)) + context.insert(makeRating(day: "2026-01-02", observedAt: .now, app: app)) + try context.save() + let expected = try RatingHistoryStore.fetch(appStoreIDs: [42], in: context) + #expect(throws: InjectedStorageInterruption.self) { + try StorageSchemaV7Migrator.migrateIfNeeded(at: url, beforeDeletion: { + throw InjectedStorageInterruption.stop + }) + } + #expect(try ModelContext(owner).fetch(FetchDescriptor()).first?.pendingDeletionPayload != nil) + // Shared access must not turn an unfinished migration into a ready app. + #expect(throws: PersistentStoreError.self) { + _ = try ModelContainerFactory.makePersistentModelContainer(at: url) + } + return expected + } + let reopened = try ModelContainerFactory.makePersistentModelContainer(at: url) + let context = ModelContext(reopened) + #expect(try RatingHistoryStore.fetch(appStoreIDs: [42], in: context) == expected) + #expect(try context.fetch(FetchDescriptor()).first?.completed == true) + #expect(try context.fetchCount(FetchDescriptor()) == 1) + } + + @Test(arguments: [false, true]) + func diskFullDuringOptionalCompactionStillOpensVerifiedHistoryAndRetriesLater(wrappedSQLiteError: Bool) throws { + let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-CompactionDiskFull-\(UUID())") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let url = root.appendingPathComponent("default.store") + let expected = try autoreleasepool { + let schema = Schema(versionedSchema: OpenASOSchemaV6.self) + let container = try ModelContainer(for: schema, configurations: ModelConfiguration(schema: schema, url: url)) + let context = ModelContext(container) + let records = [makeRating(day: "2026-01-01", observedAt: .now, app: nil), + makeRating(day: "2026-01-02", observedAt: .now, app: nil)] + for record in records { context.insert(record) } + try context.save() + return records.map(RatingHistoryRecord.init).sorted { $0.identityKey < $1.identityKey } + } + let completion = try autoreleasepool { + let container = try ModelContainerFactory.makePersistentModelContainer(at: url, compactStore: { _ in + if wrappedSQLiteError { + throw NSError(domain: NSCocoaErrorDomain, code: 134070, userInfo: [ + NSUnderlyingErrorKey: NSError(domain: "NSSQLiteErrorDomain", code: Int(SQLITE_FULL)) + ]) + } + throw CocoaError(.fileWriteOutOfSpace) + }) + let context = ModelContext(container) + #expect(try RatingHistoryStore.fetch(appStoreIDs: [42], in: context).sorted { $0.identityKey < $1.identityKey } == expected) + let state = try #require(try context.fetch(FetchDescriptor()).first) + #expect(state.completed && state.needsCompaction) + return state.lastMaintenanceAt + } + let reopened = try ModelContainerFactory.makePersistentModelContainer(at: url) + let context = ModelContext(reopened) + let state = try #require(try context.fetch(FetchDescriptor()).first) + #expect(state.completed && !state.needsCompaction) + #expect(state.lastMaintenanceAt == completion) + #expect(try RatingHistoryStore.fetch(appStoreIDs: [42], in: context).sorted { $0.identityKey < $1.identityKey } == expected) + } + + @Test + func longLivedReaderSeesAnotherWorkersHistoricalCorrectionAndRollover() async throws { + let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-ReaderUpdates-\(UUID())") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let url = root.appendingPathComponent("default.store") + let firstContainer = try ModelContainerFactory.makePersistentModelContainer(at: url) + let secondContainer = try ModelContainerFactory.makePersistentModelContainer(at: url) + let seed = ModelContext(firstContainer) + let app = StoreApp(appStoreID: 42, bundleID: nil, name: "App", sellerName: nil, + iconURLString: nil, defaultPlatform: .iphone) + seed.insert(app) + let service = AppStorefrontRatingService(httpClient: PreviewHTTPClient()) + let start = try Date("2026-01-01T15:00:00Z", strategy: .iso8601) + for day in 0..<3 { + try ingestRating(service, app: app, observedAt: start.addingTimeInterval(Double(day) * 86_400), + count: 100 + day, in: seed) + } + try seed.save() + let reader = BackgroundModelStore(modelContainer: firstContainer) + let writer = BackgroundModelStore(modelContainer: secondContainer) + let before = try await reader.read { try RatingHistoryStore.fetch(appStoreIDs: [42], in: $0) } + #expect(before.count == 3) + try await writer.write { context in + let app = try #require(try context.fetch(FetchDescriptor()).first) + try RatingHistoryStore.ingest([ + .init(.init(appStoreID: 42, storefront: "gb", ratingCount: 111, averageRating: 4.5, + ratingCounts: nil, observedAt: start.addingTimeInterval(3_600), source: .iTunesSearch)), + .init(.init(appStoreID: 42, storefront: "gb", ratingCount: 103, averageRating: 4.5, + ratingCounts: nil, observedAt: start.addingTimeInterval(3 * 86_400), source: .iTunesSearch)) + ], storeApps: [42: app], in: context) + } + let after = try await reader.read { try RatingHistoryStore.fetch(appStoreIDs: [42], in: $0) } + #expect(after.count == 4) + #expect(after.first { $0.ratingDate == "2026-01-01" }?.ratingCount == 111) + #expect(after.first { $0.ratingDate == "2026-01-04" }?.ratingCount == 103) + #expect(after.allSatisfy { $0.storeAppID == 42 }) + } + + @Test + func optionalCompactionNeverSuppressesCorruptionErrors() throws { + let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-CompactionCorruption-\(UUID())") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let url = root.appendingPathComponent("default.store") + try autoreleasepool { + let schema = Schema(versionedSchema: OpenASOSchemaV6.self) + let container = try ModelContainer(for: schema, configurations: ModelConfiguration(schema: schema, url: url)) + let context = ModelContext(container) + context.insert(makeRating(day: "2026-01-01", observedAt: .now, app: nil)) + context.insert(makeRating(day: "2026-01-02", observedAt: .now, app: nil)) + try context.save() + } + #expect(throws: PersistentStoreError.self) { + _ = try ModelContainerFactory.makePersistentModelContainer(at: url, compactStore: { _ in + throw CocoaError(.fileReadCorruptFile) + }) + } + let schema = ModelContainerFactory.schema + let container = try ModelContainer(for: schema, configurations: ModelConfiguration(schema: schema, url: url)) + let context = ModelContext(container) + #expect(try RatingHistoryStore.fetch(appStoreIDs: [42], in: context).count == 2) + let state = try #require(try context.fetch(FetchDescriptor()).first) + #expect(state.completed && state.needsCompaction) + } + private func ingestRating(_ service: AppStorefrontRatingService, app: StoreApp, observedAt: Date, count: Int, in context: ModelContext) throws { try service.persist(.init(storefront: "gb", result: .init( From 9d1fce7de7f1d323472705f00d457c07eef4e251 Mon Sep 17 00:00:00 2001 From: hubab1 <50897577+hubab1@users.noreply.github.com> Date: Sun, 25 Oct 2026 11:02:02 +0000 Subject: [PATCH 5/6] fix(persistence): Handle native disk-full errors Recognize SQLite error codes carried in Core Data error metadata. Keep corruption fatal and preserve history during a compaction retry. --- .../Persistence/PersistentStoreCompactor.swift | 8 ++++++-- .../StorageSchemaV7MigrationTests.swift | 18 +++++++++++++----- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/OpenASO/Services/Persistence/PersistentStoreCompactor.swift b/OpenASO/Services/Persistence/PersistentStoreCompactor.swift index d763445..7272087 100644 --- a/OpenASO/Services/Persistence/PersistentStoreCompactor.swift +++ b/OpenASO/Services/Persistence/PersistentStoreCompactor.swift @@ -17,10 +17,14 @@ enum PersistentStoreCompactor { if [CocoaError.fileWriteOutOfSpace.rawValue, CocoaError.fileWriteNoPermission.rawValue, CocoaError.fileLocking.rawValue].contains(error.code) { return true } } - if error.domain == "NSSQLiteErrorDomain" { + // Core Data may attach the SQLite code directly to a Cocoa error + // instead of wrapping an underlying NSSQLiteErrorDomain error. + let sqliteCode = error.domain == "NSSQLiteErrorDomain" + ? error.code : (error.userInfo["NSSQLiteErrorDomain"] as? NSNumber)?.intValue + if let sqliteCode { // Extended SQLite errors keep the primary code in the low byte. return [SQLITE_FULL, SQLITE_BUSY, SQLITE_LOCKED, SQLITE_READONLY, SQLITE_PERM] - .contains(Int32(error.code & 0xff)) + .contains(Int32(sqliteCode & 0xff)) } if let underlying = error.userInfo[NSUnderlyingErrorKey] as? NSError { return canRetryLater(underlying) diff --git a/OpenASOTests/StorageSchemaV7MigrationTests.swift b/OpenASOTests/StorageSchemaV7MigrationTests.swift index a8cad0c..1bbac2b 100644 --- a/OpenASOTests/StorageSchemaV7MigrationTests.swift +++ b/OpenASOTests/StorageSchemaV7MigrationTests.swift @@ -609,8 +609,8 @@ struct StorageSchemaV7MigrationTests { #expect(try context.fetchCount(FetchDescriptor()) == 1) } - @Test(arguments: [false, true]) - func diskFullDuringOptionalCompactionStillOpensVerifiedHistoryAndRetriesLater(wrappedSQLiteError: Bool) throws { + @Test(arguments: ["cocoa", "underlyingSQLite", "metadataSQLite"]) + func diskFullDuringOptionalCompactionStillOpensVerifiedHistoryAndRetriesLater(errorRepresentation: String) throws { let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-CompactionDiskFull-\(UUID())") try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) defer { try? FileManager.default.removeItem(at: root) } @@ -627,7 +627,11 @@ struct StorageSchemaV7MigrationTests { } let completion = try autoreleasepool { let container = try ModelContainerFactory.makePersistentModelContainer(at: url, compactStore: { _ in - if wrappedSQLiteError { + if errorRepresentation == "metadataSQLite" { + throw NSError(domain: NSCocoaErrorDomain, code: CocoaError.fileReadUnknown.rawValue, + userInfo: ["NSSQLiteErrorDomain": Int(SQLITE_FULL)]) + } + if errorRepresentation == "underlyingSQLite" { throw NSError(domain: NSCocoaErrorDomain, code: 134070, userInfo: [ NSUnderlyingErrorKey: NSError(domain: "NSSQLiteErrorDomain", code: Int(SQLITE_FULL)) ]) @@ -687,8 +691,8 @@ struct StorageSchemaV7MigrationTests { #expect(after.allSatisfy { $0.storeAppID == 42 }) } - @Test - func optionalCompactionNeverSuppressesCorruptionErrors() throws { + @Test(arguments: [false, true]) + func optionalCompactionNeverSuppressesCorruptionErrors(sqliteMetadata: Bool) throws { let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-CompactionCorruption-\(UUID())") try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) defer { try? FileManager.default.removeItem(at: root) } @@ -703,6 +707,10 @@ struct StorageSchemaV7MigrationTests { } #expect(throws: PersistentStoreError.self) { _ = try ModelContainerFactory.makePersistentModelContainer(at: url, compactStore: { _ in + if sqliteMetadata { + throw NSError(domain: NSCocoaErrorDomain, code: CocoaError.fileReadUnknown.rawValue, + userInfo: ["NSSQLiteErrorDomain": Int(SQLITE_CORRUPT)]) + } throw CocoaError(.fileReadCorruptFile) }) } From 00b0fdfdb3587dd7b2430b018c37418a25720078 Mon Sep 17 00:00:00 2001 From: hubab1 <50897577+hubab1@users.noreply.github.com> Date: Sun, 25 Oct 2026 14:11:51 +0000 Subject: [PATCH 6/6] fix(persistence): Preserve compact rating history Keep past ratings when providers report missing or mismatched storefronts. Make explicit clearing atomic across live rows, archives, and summaries. Read only required history chunks under a shared writer lease. Defer unused archive fetches and expose migration progress and read errors. --- OpenASO.xcodeproj/project.pbxproj | 12 + OpenASO/App/OpenASOApp.swift | 39 +++- .../AppDetail/AppDetailCSVTransfer.swift | 4 +- .../Table/KeywordRankingListLoader.swift | 7 +- .../AppDetail/Ratings/AppRatingsView.swift | 42 +++- OpenASO/Services/MCP/OpenASOMCPService.swift | 16 +- .../Persistence/ModelContainerFactory.swift | 81 +++++-- .../RatingHistoryStore+Reading.swift | 168 ++++++++++++++ .../Persistence/RatingHistoryStore.swift | 34 ++- .../StoragePreparationProgress.swift | 10 + .../Persistence/StorageSchemaV7Migrator.swift | 57 ++++- .../AppStorefrontRatingService.swift | 38 +--- .../AppStorefrontRatingTypes.swift | 5 +- .../AppStorefrontRatingServiceTests.swift | 29 ++- .../StorageSchemaV7MigrationTests.swift | 209 +++++++++++++++++- 15 files changed, 639 insertions(+), 112 deletions(-) create mode 100644 OpenASO/Services/Persistence/RatingHistoryStore+Reading.swift create mode 100644 OpenASO/Services/Persistence/StoragePreparationProgress.swift diff --git a/OpenASO.xcodeproj/project.pbxproj b/OpenASO.xcodeproj/project.pbxproj index de4582f..fa3f345 100644 --- a/OpenASO.xcodeproj/project.pbxproj +++ b/OpenASO.xcodeproj/project.pbxproj @@ -349,6 +349,10 @@ F008A0000000000000000001 /* Networking/ProviderRequestGate.swift in Sources */ = {isa = PBXBuildFile; fileRef = F008A0000000000000000011 /* Networking/ProviderRequestGate.swift */; }; F008A0000000000000000002 /* ProviderRequestGateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F008A0000000000000000012 /* ProviderRequestGateTests.swift */; }; F008B0000000000000000001 /* Networking/ProviderHTTPClientPipeline.swift in Sources */ = {isa = PBXBuildFile; fileRef = F008B0000000000000000011 /* Networking/ProviderHTTPClientPipeline.swift */; }; + 40A7F458B27249B89EDDCC41 /* Persistence/RatingHistoryStore+Reading.swift in Sources */ = {isa = PBXBuildFile; fileRef = E182C2554EA7417792642340 /* Persistence/RatingHistoryStore+Reading.swift */; }; + D45DEBF8843440B384689D6A /* Persistence/RatingHistoryStore+Reading.swift in Sources */ = {isa = PBXBuildFile; fileRef = E182C2554EA7417792642340 /* Persistence/RatingHistoryStore+Reading.swift */; }; + 04FE3BB52BA4436A8E8A7B36 /* Persistence/StoragePreparationProgress.swift in Sources */ = {isa = PBXBuildFile; fileRef = 382A12B0A246434D826A43F9 /* Persistence/StoragePreparationProgress.swift */; }; + 195FABB04F3A4F39925CFA43 /* Persistence/StoragePreparationProgress.swift in Sources */ = {isa = PBXBuildFile; fileRef = 382A12B0A246434D826A43F9 /* Persistence/StoragePreparationProgress.swift */; }; /* End PBXBuildFile section */ /* Begin PBXContainerItemProxy section */ @@ -637,6 +641,8 @@ F008A0000000000000000011 /* Networking/ProviderRequestGate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Networking/ProviderRequestGate.swift; sourceTree = ""; }; F008A0000000000000000012 /* ProviderRequestGateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProviderRequestGateTests.swift; sourceTree = ""; }; F008B0000000000000000011 /* Networking/ProviderHTTPClientPipeline.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Networking/ProviderHTTPClientPipeline.swift; sourceTree = ""; }; + E182C2554EA7417792642340 /* Persistence/RatingHistoryStore+Reading.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Persistence/RatingHistoryStore+Reading.swift"; sourceTree = ""; }; + 382A12B0A246434D826A43F9 /* Persistence/StoragePreparationProgress.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Persistence/StoragePreparationProgress.swift"; sourceTree = ""; }; /* End PBXFileReference section */ /* Begin PBXFrameworksBuildPhase section */ @@ -869,6 +875,8 @@ 9AED285CFBB54DB09AC958F9 /* Persistence/PersistentStoreCompactor.swift */, EB753AAEB38F4C8DA227FFA3 /* Persistence/StorageSchemaV7Migrator.swift */, ED8BE61549704D9DA2D9FC76 /* Persistence/RatingHistoryStore.swift */, + 382A12B0A246434D826A43F9 /* Persistence/StoragePreparationProgress.swift */, + E182C2554EA7417792642340 /* Persistence/RatingHistoryStore+Reading.swift */, D33A00000000000000000011 /* SearchRanking/KeywordResearchRankingWorkflow.swift */, D34A00000000000000000011 /* SearchRanking/KeywordResearchHistoryReader.swift */, A70000000000000000000002 /* RatingsReviews/ReviewLanguageDetectionService.swift */, @@ -1364,6 +1372,8 @@ 49621A2D2079403C8FB09C1A /* Persistence/PersistentStoreCompactor.swift in Sources */, 0129D26AA92C4ED0B32BCDB8 /* Persistence/StorageSchemaV7Migrator.swift in Sources */, CD1242318DB74C7DB224A6FF /* Persistence/RatingHistoryStore.swift in Sources */, + 04FE3BB52BA4436A8E8A7B36 /* Persistence/StoragePreparationProgress.swift in Sources */, + 40A7F458B27249B89EDDCC41 /* Persistence/RatingHistoryStore+Reading.swift in Sources */, D32A00000000000000000003 /* KeywordResearchProject.swift in Sources */, D32A00000000000000000005 /* KeywordResearchKeyword.swift in Sources */, D20B00000000000000000003 /* EstimatedKeywordDifficultyMetric.swift in Sources */, @@ -1561,6 +1571,8 @@ DBF111AE76C4432B9DE83560 /* Persistence/PersistentStoreCompactor.swift in Sources */, 073892B3DFEF4C34B0EBBC39 /* Persistence/StorageSchemaV7Migrator.swift in Sources */, 9532E6E87A564ABFB26C6A21 /* Persistence/RatingHistoryStore.swift in Sources */, + 195FABB04F3A4F39925CFA43 /* Persistence/StoragePreparationProgress.swift in Sources */, + D45DEBF8843440B384689D6A /* Persistence/RatingHistoryStore+Reading.swift in Sources */, D32A00000000000000000004 /* KeywordResearchProject.swift in Sources */, D32A00000000000000000006 /* KeywordResearchKeyword.swift in Sources */, D20B00000000000000000004 /* EstimatedKeywordDifficultyMetric.swift in Sources */, diff --git a/OpenASO/App/OpenASOApp.swift b/OpenASO/App/OpenASOApp.swift index 8d3056e..d6aaab5 100644 --- a/OpenASO/App/OpenASOApp.swift +++ b/OpenASO/App/OpenASOApp.swift @@ -9,6 +9,7 @@ struct OpenASOApp: App { @State private var startupState: OpenASOStartupState = .preparing @State private var isPreparingStartup = false + @State private var preparationProgress = StoragePreparationProgress.opening init() { let executionMode = OpenASOExecutionMode( @@ -40,16 +41,21 @@ struct OpenASOApp: App { guard case .preparing = startupState, !isPreparingStartup else { return } isPreparingStartup = true defer { isPreparingStartup = false } + let (updates, continuation) = AsyncStream.makeStream(bufferingPolicy: .bufferingNewest(1)) + async let progressUpdates: Void = observePreparationProgress(updates) let result = await Task.detached(priority: .userInitiated) { () -> Result in + defer { continuation.finish() } do { - return .success(try ModelContainerFactory.makeModelContainer(isStoredInMemoryOnly: false)) + return .success(try ModelContainerFactory.makeModelContainer(isStoredInMemoryOnly: false, + progress: { continuation.yield($0) })) } catch let error as PersistentStoreError { return .failure(error) } catch { return .failure(.unexpected(diagnosticDescription: String(reflecting: error))) } }.value + await progressUpdates switch result { case .success(let container): startupState = .ready(modelContainer: container, services: AppServices.appLaunch(modelContainer: container)) @@ -58,6 +64,10 @@ struct OpenASOApp: App { } } + private func observePreparationProgress(_ updates: AsyncStream) async { + for await progress in updates { preparationProgress = progress } + } + private static func makeStartupState() -> OpenASOStartupState { do { let modelContainer = try ModelContainerFactory.makeModelContainer(isStoredInMemoryOnly: false) @@ -108,7 +118,7 @@ struct OpenASOApp: App { WindowGroup("OpenASO") { switch startupState { case .preparing: - ProgressView("Preparing your workspace…") + StoragePreparationView(progress: preparationProgress) .frame(idealWidth: 1000, idealHeight: 760) .task { await prepareStartupIfNeeded() } case .ready(let modelContainer, let services): @@ -151,7 +161,7 @@ struct OpenASOApp: App { Settings { switch startupState { case .preparing: - ProgressView("Preparing your workspace…") + StoragePreparationView(progress: preparationProgress) .frame(idealWidth: 680, idealHeight: 480) .task { await prepareStartupIfNeeded() } case .ready(let modelContainer, let services): @@ -208,3 +218,26 @@ private struct AppLaunchAlertContext: Identifiable { let title: String let message: String } + +private struct StoragePreparationView: View { + let progress: StoragePreparationProgress + + var body: some View { + VStack(spacing: 12) { + switch progress { + case .archiving(let completed, let total): + ProgressView("Compressing rating history…", value: Double(completed), total: Double(max(total, 1))) + Text("\(completed.formatted()) of \(total.formatted()) rating records processed") + .font(.caption).foregroundStyle(.secondary) + case .compacting: + ProgressView("Reclaiming database space…") + case .waitingForClients: + ProgressView("Waiting for another OpenASO process to finish preparing the database…") + case .opening, .ready: + ProgressView("Opening your workspace…") + } + } + .padding(32) + .frame(maxWidth: 440) + } +} diff --git a/OpenASO/Features/AppDetail/AppDetailCSVTransfer.swift b/OpenASO/Features/AppDetail/AppDetailCSVTransfer.swift index 4f6199e..b7f9d8c 100644 --- a/OpenASO/Features/AppDetail/AppDetailCSVTransfer.swift +++ b/OpenASO/Features/AppDetail/AppDetailCSVTransfer.swift @@ -191,11 +191,11 @@ struct AppDetailRatingsCSVExporter { }, sortBy: [SortDescriptor(\.storefront, order: .forward)] ) - let snapshots = try RatingHistoryStore.fetch(appStoreIDs: [appStoreID], in: modelContext) + let snapshots = try RatingHistoryStore.recent(appStoreIDs: [appStoreID], limitPerApp: 1_000, in: modelContext) return AppDetailRatingsExportData( latestRatings: try modelContext.fetch(latestDescriptor).map(AppDetailRatingLatestValue.init), - ratingSnapshots: snapshots.prefix(1_000).map(AppDetailRatingSnapshotValue.init) + ratingSnapshots: snapshots.map(AppDetailRatingSnapshotValue.init) ) } diff --git a/OpenASO/Features/AppDetail/Keywords/Table/KeywordRankingListLoader.swift b/OpenASO/Features/AppDetail/Keywords/Table/KeywordRankingListLoader.swift index e5c1d48..07cef06 100644 --- a/OpenASO/Features/AppDetail/Keywords/Table/KeywordRankingListLoader.swift +++ b/OpenASO/Features/AppDetail/Keywords/Table/KeywordRankingListLoader.swift @@ -239,9 +239,10 @@ enum KeywordRankingListLoader { snapshotsByID.reserveCapacity(uniqueAppStoreIDs.count) guard !uniqueAppStoreIDs.isEmpty else { return [:] } - let snapshots = try RatingHistoryStore.fetch( - appStoreIDs: uniqueAppStoreIDs, storefronts: [targetStorefront], in: modelContext - ).filter { $0.ratingCount != nil }.sorted { + let snapshots = try RatingHistoryStore.recent( + appStoreIDs: uniqueAppStoreIDs, storefronts: [targetStorefront], limitPerApp: ratingSnapshotLimit, + order: .ratingDate, requiresRatingCount: true, in: modelContext + ).sorted { if $0.appStoreID != $1.appStoreID { return $0.appStoreID < $1.appStoreID } if $0.ratingDate != $1.ratingDate { return $0.ratingDate > $1.ratingDate } return $0.observedAt > $1.observedAt diff --git a/OpenASO/Features/AppDetail/Ratings/AppRatingsView.swift b/OpenASO/Features/AppDetail/Ratings/AppRatingsView.swift index ac2c1d0..18b3bc0 100644 --- a/OpenASO/Features/AppDetail/Ratings/AppRatingsView.swift +++ b/OpenASO/Features/AppDetail/Ratings/AppRatingsView.swift @@ -15,6 +15,9 @@ struct AppRatingsView: View { @State private var latestRatings: [RatingLatestValue] = [] @State private var ratingSnapshots: [RatingSnapshotValue] = [] @State private var isLoadingRatings = false + @State private var activeRatingsLoad: UUID? + @State private var loadedRatingsAppID: Int64? + @State private var ratingsLoadError: String? @State private var appStoreConnectStatus = RatingsAppStoreConnectStatus.notConnected @State private var metric = RatingsMetric.ratingCount @@ -64,11 +67,26 @@ struct AppRatingsView: View { ) .frame(minWidth: 300, maxWidth: .infinity, maxHeight: .infinity) } + .safeAreaInset(edge: .top) { + if let ratingsLoadError { + HStack(alignment: .top) { + VStack(alignment: .leading, spacing: 4) { + Label("Rating history couldn’t be loaded", systemImage: "exclamationmark.triangle") + Text(ratingsLoadError).font(.caption).foregroundStyle(.secondary) + } + Spacer() + Button("Retry") { Task { await loadRatings() } } + .disabled(isLoadingRatings) + } + .padding() + .background(.bar) + } + } .task { loadStorefrontDefinitionsIfNeeded() } .task(id: ratingsReloadID) { - await loadRatings(reset: true) + await loadRatings() } .task(id: appStoreConnectReloadID) { await updateAppStoreConnectStatus() @@ -102,15 +120,19 @@ struct AppRatingsView: View { } } - private func loadRatings(reset: Bool) async { - guard let backgroundModelStore = services.backgroundModelStore, !isLoadingRatings else { - return - } - + private func loadRatings() async { + guard let backgroundModelStore = services.backgroundModelStore else { return } + let loadID = UUID() + activeRatingsLoad = loadID isLoadingRatings = true - defer { isLoadingRatings = false } + ratingsLoadError = nil + defer { if activeRatingsLoad == loadID { isLoadingRatings = false } } let targetAppStoreID = appStoreID + if loadedRatingsAppID != targetAppStoreID { + latestRatings = [] + ratingSnapshots = [] + } do { let data = try await backgroundModelStore.read { modelContext in try Self.fetchRatingsDashboardData( @@ -119,11 +141,13 @@ struct AppRatingsView: View { ) } + guard !Task.isCancelled, activeRatingsLoad == loadID, appStoreID == targetAppStoreID else { return } latestRatings = data.latestRatings ratingSnapshots = data.ratingSnapshots.sorted { $0.observedAt < $1.observedAt } + loadedRatingsAppID = targetAppStoreID } catch { - latestRatings = reset ? [] : latestRatings - ratingSnapshots = reset ? [] : ratingSnapshots + guard !Task.isCancelled, activeRatingsLoad == loadID, appStoreID == targetAppStoreID else { return } + ratingsLoadError = error.localizedDescription } } diff --git a/OpenASO/Services/MCP/OpenASOMCPService.swift b/OpenASO/Services/MCP/OpenASOMCPService.swift index fe4523f..2d9d18c 100644 --- a/OpenASO/Services/MCP/OpenASOMCPService.swift +++ b/OpenASO/Services/MCP/OpenASOMCPService.swift @@ -1356,17 +1356,13 @@ final class OpenASOMCPService: Sendable { let minimumDate = dateFrom ?? .distantPast let maximumDate = dateTo ?? .distantFuture let storefrontValues = Array(storefronts) - let filtersStorefront = !storefrontValues.isEmpty - let allRatings = try RatingHistoryStore.fetch( - appStoreIDs: [appStoreID], storefronts: storefrontValues, - from: minimumDate, to: maximumDate, in: modelContext + let cursor = historyCursor.map { RatingHistoryStore.Cursor(observedAt: $0.timestamp, identityKey: $0.tieKey) } + let history = try RatingHistoryStore.page( + appStoreID: appStoreID, storefronts: storefrontValues, + from: minimumDate, to: maximumDate, after: cursor, limit: page.limit + 1, in: modelContext ) - let total = allRatings.count - let ratingsWithLookahead = Array(allRatings.filter { rating in - guard let historyCursor else { return true } - return rating.observedAt < historyCursor.timestamp - || (rating.observedAt == historyCursor.timestamp && rating.identityKey > historyCursor.tieKey) - }.prefix(page.limit + 1)) + let total = history.total + let ratingsWithLookahead = history.records let pageSlice = try Self.historyPageFromLookahead( rowsWithLookahead: ratingsWithLookahead, kind: .ratings, diff --git a/OpenASO/Services/Persistence/ModelContainerFactory.swift b/OpenASO/Services/Persistence/ModelContainerFactory.swift index d1ce460..bc6ab86 100644 --- a/OpenASO/Services/Persistence/ModelContainerFactory.swift +++ b/OpenASO/Services/Persistence/ModelContainerFactory.swift @@ -3,6 +3,7 @@ import Darwin import Foundation import ObjectiveC import SwiftData +import Synchronization enum PersistentStoreError: LocalizedError, Sendable { case locationUnavailable(diagnosticDescription: String) @@ -54,6 +55,8 @@ enum PersistentStoreError: LocalizedError, Sendable { } enum ModelContainerFactory { + private static let writingContexts = Mutex>([]) + typealias ContainerOpener = (Schema, ModelConfiguration) throws -> ModelContainer static var schema: Schema { @@ -63,7 +66,8 @@ enum ModelContainerFactory { static func makeModelContainer( isStoredInMemoryOnly: Bool, namespace: AppNamespace = .current, - opener: ContainerOpener = openModelContainer + opener: ContainerOpener = openModelContainer, + progress: (@Sendable (StoragePreparationProgress) -> Void)? = nil ) throws -> ModelContainer { let schema = Self.schema if isStoredInMemoryOnly { @@ -71,7 +75,10 @@ enum ModelContainerFactory { schema: schema, isStoredInMemoryOnly: true ) - return try opener(schema, configuration) + progress?(.opening) + let container = try opener(schema, configuration) + progress?(.ready) + return container } let persistentStoreURL: URL @@ -85,19 +92,21 @@ enum ModelContainerFactory { return try makePersistentModelContainer( at: persistentStoreURL, - opener: opener + opener: opener, progress: progress ) } static func makePersistentModelContainer( at storeURL: URL, opener: ContainerOpener = openModelContainer, - compactStore: (URL) throws -> Void = PersistentStoreCompactor.compactIfNeeded + compactStore: (URL) throws -> Void = PersistentStoreCompactor.compactIfNeeded, + progress: (@Sendable (StoragePreparationProgress) -> Void)? = nil ) throws -> ModelContainer { let schema = Self.schema let configuration = ModelConfiguration(schema: schema, url: storeURL) do { - return try withMigrationAccess(for: storeURL) { canMigrate in + progress?(.opening) + return try withMigrationAccess(for: storeURL, progress: progress) { canMigrate in guard canMigrate else { let container = try opener(schema, configuration) let state = try ModelContext(container).fetch(FetchDescriptor()).first @@ -108,6 +117,7 @@ enum ModelContainerFactory { NSLocalizedDescriptionKey: "The database upgrade is unfinished. Quit other OpenASO, refresh, or MCP processes and reopen to resume it." ]) } + progress?(.ready) return container } let prepared: ModelContainer? = try autoreleasepool { @@ -122,8 +132,12 @@ enum ModelContainerFactory { try PersistentHistoryMaintenance.prune(in: context) return nil } - if let prepared { return prepared } - try StorageSchemaV7Migrator.migrateIfNeeded(at: storeURL) + if let prepared { + progress?(.ready) + return prepared + } + try StorageSchemaV7Migrator.migrateIfNeeded(at: storeURL, progress: progress) + progress?(.compacting) var compactionFinished = false do { try compactStore(storeURL) @@ -142,6 +156,7 @@ enum ModelContainerFactory { try context.save() } } + progress?(.ready) return container } } catch { @@ -157,6 +172,43 @@ enum ModelContainerFactory { /// different writer's committed days. No network await occurs under this lock. static func withWriteTransaction( in container: ModelContainer, _ operation: (ModelContext) throws -> Value + ) throws -> Value { + try withTransactionAccess(in: container, mode: LOCK_EX) { + let context = ModelContext(container) + context.autosaveEnabled = false + let identity = ObjectIdentifier(context) + writingContexts.withLock { _ = $0.insert(identity) } + defer { writingContexts.withLock { _ = $0.remove(identity) } } + do { + let result = try operation(context) + if context.hasChanges { try context.save() } + return result + } catch { + context.rollback() + throw error + } + } + } + + /// A bounded history query reads live rows, chunk metadata, and payloads + /// under one shared lease. Fresh contexts avoid mixing cached generations. + /// Reads inside an existing write transaction reuse its exclusive lease. + static func withReadTransaction( + in context: ModelContext, _ operation: (ModelContext) throws -> Value + ) throws -> Value { + if writingContexts.withLock({ $0.contains(ObjectIdentifier(context)) }) { + return try operation(context) + } + return try withTransactionAccess(in: context.container, mode: LOCK_SH) { + if context.hasChanges { return try operation(context) } + let fresh = ModelContext(context.container) + fresh.autosaveEnabled = false + return try operation(fresh) + } + } + + private static func withTransactionAccess( + in container: ModelContainer, mode: Int32, _ operation: () throws -> Value ) throws -> Value { let configuration = container.configurations.first! let identity = configuration.isStoredInMemoryOnly @@ -169,20 +221,11 @@ enum ModelContainerFactory { let descriptor = url.path.withCString { Darwin.open($0, O_CREAT | O_RDWR | O_CLOEXEC, S_IRUSR | S_IWUSR) } guard descriptor >= 0 else { throw CocoaError(.fileWriteUnknown) } defer { Darwin.close(descriptor) } - while flock(descriptor, LOCK_EX) != 0 { + while flock(descriptor, mode) != 0 { guard errno == EINTR else { throw CocoaError(.fileLocking) } } defer { flock(descriptor, LOCK_UN) } - let context = ModelContext(container) - context.autosaveEnabled = false - do { - let result = try operation(context) - if context.hasChanges { try context.save() } - return result - } catch { - context.rollback() - throw error - } + return try operation() } private static func storeURL(namespace: AppNamespace) throws -> URL { @@ -205,6 +248,7 @@ enum ModelContainerFactory { private static func withMigrationAccess( for storeURL: URL, + progress: (@Sendable (StoragePreparationProgress) -> Void)?, _ operation: (Bool) throws -> ModelContainer ) throws -> ModelContainer { let lockDirectory = FileManager.default.temporaryDirectory @@ -227,6 +271,7 @@ enum ModelContainerFactory { let exclusive = flock(descriptor, LOCK_EX | LOCK_NB) == 0 if !exclusive { guard errno == EWOULDBLOCK || errno == EAGAIN else { throw CocoaError(.fileLocking) } + progress?(.waitingForClients) while flock(descriptor, LOCK_SH) != 0 { guard errno == EINTR else { throw CocoaError(.fileLocking) } } diff --git a/OpenASO/Services/Persistence/RatingHistoryStore+Reading.swift b/OpenASO/Services/Persistence/RatingHistoryStore+Reading.swift new file mode 100644 index 0000000..f27c464 --- /dev/null +++ b/OpenASO/Services/Persistence/RatingHistoryStore+Reading.swift @@ -0,0 +1,168 @@ +import Foundation +import SwiftData + +extension RatingHistoryStore { + struct Cursor: Sendable { + let observedAt: Date + let identityKey: String + } + + struct Page: Sendable { + let records: [RatingHistoryRecord] + let total: Int + } + + enum RecentOrder: Sendable { + case observationDate + case ratingDate + + func precedes(_ lhs: RatingHistoryRecord, _ rhs: RatingHistoryRecord) -> Bool { + if self == .ratingDate, lhs.ratingDate != rhs.ratingDate { return lhs.ratingDate > rhs.ratingDate } + return lhs.observedAt == rhs.observedAt + ? lhs.identityKey < rhs.identityKey : lhs.observedAt > rhs.observedAt + } + } + + /// Counts complete chunks from metadata and only decodes payloads that can + /// contribute to this page. Date ranges cutting through chunks require + /// decoding those boundary chunks to retain exact totals. + static func page(appStoreID: Int64, storefronts: [String] = [], + from: Date = .distantPast, to: Date = .distantFuture, + after cursor: Cursor? = nil, limit: Int, in context: ModelContext) throws -> Page { + try ModelContainerFactory.withReadTransaction(in: context) { context in + precondition(limit > 0) + let reader = try BoundedReader(appStoreIDs: [appStoreID], storefronts: storefronts, + from: from, to: to, in: context) + guard reader.isCompact else { + let all = try fetch(appStoreIDs: [appStoreID], storefronts: storefronts, from: from, to: to, in: context) + return Page(records: Array(all.filter { isAfter($0, cursor: cursor) }.prefix(limit)), total: all.count) + } + var selection = Selection(limit: limit, order: .observationDate, cursor: cursor, from: from, to: to) + selection.add(reader.live) + var total = reader.live.filter { $0.observedAt >= from && $0.observedAt <= to }.count + var complete: [RatingHistoryChunk] = [] + for chunk in reader.chunks { + if chunk.firstObservedAt >= from && chunk.lastObservedAt <= to { + total += chunk.recordCount + complete.append(chunk) + } else { + let values = try records(in: chunk) + total += values.filter { $0.observedAt >= from && $0.observedAt <= to }.count + selection.add(values) + } + } + try selection.read(complete) + return Page(records: selection.values, total: total) + } + } + + /// Returns at most limitPerApp records for each app, retaining the requested + /// logical-date or observation-time ordering without hydrating lifetime history. + static func recent(appStoreIDs: [Int64], storefronts: [String] = [], limitPerApp: Int, + order: RecentOrder = .observationDate, requiresRatingCount: Bool = false, + in context: ModelContext) throws -> [RatingHistoryRecord] { + try ModelContainerFactory.withReadTransaction(in: context) { context in + guard !appStoreIDs.isEmpty, limitPerApp > 0 else { return [] } + let reader = try BoundedReader(appStoreIDs: appStoreIDs, storefronts: storefronts, in: context) + guard reader.isCompact else { + let all = try fetch(appStoreIDs: appStoreIDs, storefronts: storefronts, in: context) + .filter { !requiresRatingCount || $0.ratingCount != nil } + return Dictionary(grouping: all, by: \.appStoreID).values.flatMap { + $0.sorted(by: order.precedes).prefix(limitPerApp) + }.sorted(by: order.precedes) + } + let live = Dictionary(grouping: reader.live, by: \.appStoreID) + let chunks = Dictionary(grouping: reader.chunks, by: \.appStoreID) + var result: [RatingHistoryRecord] = [] + for appID in Set(appStoreIDs).sorted() { + var selection = Selection(limit: limitPerApp, order: order, requiresRatingCount: requiresRatingCount) + selection.add(live[appID] ?? []) + try selection.read(chunks[appID] ?? []) + result.append(contentsOf: selection.values) + } + return result.sorted(by: order.precedes) + } + } + + private static func isAfter(_ value: RatingHistoryRecord, cursor: Cursor?) -> Bool { + guard let cursor else { return true } + return value.observedAt < cursor.observedAt + || (value.observedAt == cursor.observedAt && value.identityKey > cursor.identityKey) + } + + private struct BoundedReader { + let live: [RatingHistoryRecord] + let chunks: [RatingHistoryChunk] + let isCompact: Bool + + init(appStoreIDs: [Int64], storefronts: [String], from: Date = .distantPast, + to: Date = .distantFuture, in context: ModelContext) throws { + let filtersStorefront = !storefronts.isEmpty + var raw = FetchDescriptor(predicate: #Predicate { + appStoreIDs.contains($0.appStoreID) && (!filtersStorefront || storefronts.contains($0.storefront)) + }) + raw.relationshipKeyPathsForPrefetching = [\.storeApp] + live = try context.fetch(raw).map(RatingHistoryRecord.init) + var descriptor = FetchDescriptor(predicate: #Predicate { + appStoreIDs.contains($0.appStoreID) && (!filtersStorefront || storefronts.contains($0.storefront)) + && $0.firstObservedAt <= to && $0.lastObservedAt >= from + }) + // Accessing payload/digest later faults them only for selected chunks. + descriptor.propertiesToFetch = [\.identityKey, \.appStoreID, \.storefront, \.year, + \.firstObservedAt, \.lastObservedAt, \.recordCount, \.formatVersion, \.uncompressedByteCount] + chunks = try context.fetch(descriptor) + for chunk in chunks { + guard chunk.recordCount > 0, chunk.recordCount <= 512, + chunk.formatVersion == 1, chunk.uncompressedByteCount > 0, + chunk.uncompressedByteCount <= 8 * 1_024 * 1_024 else { + throw RatingHistoryError.invalidArchive(chunk.identityKey) + } + } + var stateRequest = FetchDescriptor(predicate: #Predicate { $0.migrationKey == "storage-v7" }) + stateRequest.fetchLimit = 1 + let state = try context.fetch(stateRequest).first + // A completed upgrade plus compact ingestion guarantees disjoint + // archives and one hot day per scope. Legacy/partial fixtures use the + // full reader's overlap reconciliation instead of metadata counts. + isCompact = !context.hasChanges && (live.isEmpty || (state?.completed == true && state?.pendingDeletionPayload == nil + && Dictionary(grouping: live, by: \.scopeKey).values.allSatisfy { $0.count == 1 })) + } + } + + private struct Selection { + let limit: Int + let order: RecentOrder + var requiresRatingCount = false + var cursor: Cursor? = nil + var from: Date = .distantPast + var to: Date = .distantFuture + private(set) var values: [RatingHistoryRecord] = [] + + mutating func add(_ records: [RatingHistoryRecord]) { + var byKey = Dictionary(uniqueKeysWithValues: values.map { ($0.identityKey, $0) }) + for record in records where record.observedAt >= from && record.observedAt <= to + && (!requiresRatingCount || record.ratingCount != nil) && isAfter(record, cursor: cursor) { + if let prior = byKey[record.identityKey], prior.observedAt > record.observedAt { continue } + byKey[record.identityKey] = record + } + values = Array(byKey.values.sorted(by: order.precedes).prefix(limit)) + } + + mutating func read(_ chunks: [RatingHistoryChunk]) throws { + let ordered = chunks.sorted { + if order == .ratingDate, $0.year != $1.year { return $0.year > $1.year } + return $0.lastObservedAt > $1.lastObservedAt + } + for chunk in ordered { + try Task.checkCancellation() + if let cursor, chunk.firstObservedAt > cursor.observedAt { continue } + if values.count == limit, let cutoff = values.last { + if order == .ratingDate { + if chunk.year < String(cutoff.ratingDate.prefix(4)) { continue } + } else if chunk.lastObservedAt < cutoff.observedAt { continue } + } + add(try records(in: chunk)) + } + } + } +} diff --git a/OpenASO/Services/Persistence/RatingHistoryStore.swift b/OpenASO/Services/Persistence/RatingHistoryStore.swift index 62fe16f..fb75f58 100644 --- a/OpenASO/Services/Persistence/RatingHistoryStore.swift +++ b/OpenASO/Services/Persistence/RatingHistoryStore.swift @@ -308,7 +308,7 @@ enum RatingHistoryStore { LatestAppRating.makeIdentityKey(appStoreID: $0.appStoreID, storefront: $0.storefront) }) let chunkKeys = Array(Set(observations.map { $0.record.chunkKey } + live.map { RatingHistoryRecord($0).chunkKey })) - let archives = try ArchiveEdits(keys: chunkKeys, in: context) + let archives = ArchiveEdits(keys: chunkKeys, in: context) let latestValues = try context.fetch(FetchDescriptor(predicate: #Predicate { appIDs.contains($0.appStoreID) && storefronts.contains($0.storefront) })) @@ -382,19 +382,26 @@ enum RatingHistoryStore { } private final class ArchiveEdits { - private let chunks: [String: RatingHistoryChunk] + private var chunks: [String: RatingHistoryChunk]? + private let keys: [String] + private let context: ModelContext private var decoded: [String: [String: RatingHistoryRecord]] = [:] private var changed = Set() - init(keys: [String], in context: ModelContext) throws { - chunks = Dictionary(uniqueKeysWithValues: try context.fetch(FetchDescriptor( - predicate: #Predicate { keys.contains($0.identityKey) } - )).map { ($0.identityKey, $0) }) + init(keys: [String], in context: ModelContext) { + self.keys = keys + self.context = context } private func values(for key: String) throws -> [String: RatingHistoryRecord] { if let values = decoded[key] { return values } - let values = try chunks[key].map { chunk in + if chunks == nil { + let requestedKeys = keys + chunks = Dictionary(uniqueKeysWithValues: try context.fetch(FetchDescriptor( + predicate: #Predicate { requestedKeys.contains($0.identityKey) } + )).map { ($0.identityKey, $0) }) + } + let values = try chunks?[key].map { chunk in Dictionary(uniqueKeysWithValues: try records(in: chunk).map { ($0.identityKey, $0) }) } ?? [:] decoded[key] = values @@ -420,7 +427,7 @@ enum RatingHistoryStore { for key in changed.sorted() { let values = Array(decoded[key]!.values) let first = values[0] - let chunk = chunks[key] ?? RatingHistoryChunk(appStoreID: first.appStoreID, + let chunk = chunks?[key] ?? RatingHistoryChunk(appStoreID: first.appStoreID, storefront: first.storefront, year: String(first.ratingDate.prefix(4))) try replace(values, in: chunk) if chunk.modelContext == nil { context.insert(chunk) } @@ -428,10 +435,21 @@ enum RatingHistoryStore { } } + /// Explicit scope clearing only; provider unavailability must preserve history. static func remove(appStoreID: Int64, storefront: String, in context: ModelContext) throws { let chunks = try context.fetch(FetchDescriptor(predicate: #Predicate { $0.appStoreID == appStoreID && $0.storefront == storefront })) + let live = try context.fetch(FetchDescriptor(predicate: #Predicate { + $0.appStoreID == appStoreID && $0.storefront == storefront + })) + let latest = try context.fetch(FetchDescriptor(predicate: #Predicate { + $0.appStoreID == appStoreID && $0.storefront == storefront + })) + // Fetch every representation before staging changes. The caller owns + // the save/rollback, just as with ingestion. for chunk in chunks { context.delete(chunk) } + for value in live { context.delete(value) } + for value in latest { context.delete(value) } } } diff --git a/OpenASO/Services/Persistence/StoragePreparationProgress.swift b/OpenASO/Services/Persistence/StoragePreparationProgress.swift new file mode 100644 index 0000000..8aac4ba --- /dev/null +++ b/OpenASO/Services/Persistence/StoragePreparationProgress.swift @@ -0,0 +1,10 @@ +import Foundation + +/// Transient startup status; never persisted and never starts maintenance work. +enum StoragePreparationProgress: Equatable, Sendable { + case opening + case waitingForClients + case archiving(completed: Int, total: Int) + case compacting + case ready +} diff --git a/OpenASO/Services/Persistence/StorageSchemaV7Migrator.swift b/OpenASO/Services/Persistence/StorageSchemaV7Migrator.swift index 613fad7..79377bd 100644 --- a/OpenASO/Services/Persistence/StorageSchemaV7Migrator.swift +++ b/OpenASO/Services/Persistence/StorageSchemaV7Migrator.swift @@ -20,7 +20,8 @@ enum StorageSchemaV7Migrator { @discardableResult static func migrateIfNeeded( at storeURL: URL, now: Date = .now, batchLimit: Int? = nil, - beforeDeletion: (@Sendable () throws -> Void)? = nil + beforeDeletion: (@Sendable () throws -> Void)? = nil, + progress: (@Sendable (StoragePreparationProgress) -> Void)? = nil ) throws -> Bool { guard let model = NSManagedObjectModel.makeManagedObjectModel(for: OpenASOMigrationPlan.currentSchema.models) else { throw CocoaError(.persistentStoreInvalidType) @@ -29,17 +30,43 @@ enum StorageSchemaV7Migrator { let store = try coordinator.addPersistentStore(type: .sqlite, at: storeURL, options: [NSPersistentHistoryTrackingKey: true, NSSQLitePragmasOption: ["cache_size": "-65536"]]) defer { try? coordinator.remove(store) } + var total: Int? + if progress != nil { + let context = NSManagedObjectContext(concurrencyType: .privateQueueConcurrencyType) + context.persistentStoreCoordinator = coordinator + total = try context.performAndWait { + let request = NSFetchRequest(entityName: "StorageMigrationState") + request.predicate = NSPredicate(format: "migrationKey == %@", "storage-v7") + request.fetchLimit = 1 + let state = try context.fetch(request).first + guard state?.value(forKey: "completed") as? Bool != true else { return nil } + let cursor = state?.value(forKey: "lastIdentityKey") as? String ?? "" + let remaining = NSFetchRequest(entityName: "AppDailyRating") + remaining.predicate = NSPredicate(format: "identityKey > %@", cursor) + return try context.count(for: remaining) + } + // A completed migration must not count or scan old source history. + guard let total else { return true } + progress?(.archiving(completed: 0, total: total)) + } + var completed = 0 var batches = 0 while batchLimit == nil || batches < batchLimit! { - let done = try autoreleasepool { + let processed = try autoreleasepool { let context = NSManagedObjectContext(concurrencyType: .privateQueueConcurrencyType) context.persistentStoreCoordinator = coordinator context.undoManager = nil return try context.performAndWait { - try migratePersistentBatch(in: context, coordinator: coordinator, now: now, beforeDeletion: beforeDeletion) + try migratePersistentBatch(in: context, coordinator: coordinator, now: now, beforeDeletion: beforeDeletion, + reportsProgress: progress != nil) } } - if done { return true } + guard let processed else { + if let total { progress?(.archiving(completed: total, total: total)) } + return true + } + completed += processed + if let total { progress?(.archiving(completed: min(completed, total), total: total)) } batches += 1 } return false @@ -47,8 +74,8 @@ enum StorageSchemaV7Migrator { private static func migratePersistentBatch( in context: NSManagedObjectContext, coordinator: NSPersistentStoreCoordinator, now: Date, - beforeDeletion: (@Sendable () throws -> Void)? - ) throws -> Bool { + beforeDeletion: (@Sendable () throws -> Void)?, reportsProgress: Bool + ) throws -> Int? { let stateRequest = NSFetchRequest(entityName: "StorageMigrationState") stateRequest.predicate = NSPredicate(format: "migrationKey == %@", "storage-v7") stateRequest.fetchLimit = 1 @@ -62,10 +89,18 @@ enum StorageSchemaV7Migrator { state.setValue(true, forKey: "needsCompaction") try context.save() } - if state.value(forKey: "completed") as? Bool == true { return true } + if state.value(forKey: "completed") as? Bool == true { return nil } if let pending = state.value(forKey: "pendingDeletionPayload") as? Data { - try finishDeletion(JSONDecoder().decode(PendingDeletion.self, from: pending), state: state, in: context) - return false + let pending = try JSONDecoder().decode(PendingDeletion.self, from: pending) + var processed = 0 + if reportsProgress { + let request = NSFetchRequest(entityName: "AppDailyRating") + let cursor = state.value(forKey: "lastIdentityKey") as? String ?? "" + request.predicate = NSPredicate(format: "identityKey > %@ AND identityKey <= %@", cursor, pending.nextCursor) + processed = try context.count(for: request) + } + try finishDeletion(pending, state: state, in: context) + return processed } let cursor = state.value(forKey: "lastIdentityKey") as? String ?? "" let request = NSFetchRequest(entityName: "AppDailyRating") @@ -81,7 +116,7 @@ enum StorageSchemaV7Migrator { state.setValue(now, forKey: "lastMaintenanceAt") try context.save() try context.execute(NSPersistentHistoryChangeRequest.deleteHistory(before: Date.now)) - return true + return nil } let nextCursor = try RatingHistoryRecord(last).identityKey let records = try batch.map(RatingHistoryRecord.init) @@ -157,7 +192,7 @@ enum StorageSchemaV7Migrator { try context.save() try beforeDeletion?() try finishDeletion(pending, state: state, in: context) - return false + return batch.count } private static func finishDeletion( diff --git a/OpenASO/Services/RatingsReviews/AppStorefrontRatingService.swift b/OpenASO/Services/RatingsReviews/AppStorefrontRatingService.swift index 629037c..e0fb8df 100644 --- a/OpenASO/Services/RatingsReviews/AppStorefrontRatingService.swift +++ b/OpenASO/Services/RatingsReviews/AppStorefrontRatingService.swift @@ -139,8 +139,7 @@ final class AppStorefrontRatingService: Sendable { storefront: storefront, result: nil, error: nil, - unavailabilityReason: unavailable.localizedDescription, - clearsStoredRatings: true + unavailabilityReason: unavailable.localizedDescription ) } catch let mismatch as AppStorefrontRatingStorefrontMismatch { try Task.checkCancellation() @@ -151,8 +150,7 @@ final class AppStorefrontRatingService: Sendable { return AppStorefrontRatingRefreshOutcome( storefront: storefront, result: nil, - error: mappedError, - clearsStoredRatings: true + error: mappedError ) } catch { if Task.isCancelled { @@ -179,10 +177,6 @@ final class AppStorefrontRatingService: Sendable { try RatingHistoryStore.ingest([.init(result)], storeApps: [storeApp.appStoreID: storeApp], in: modelContext) } - - if outcome.clearsStoredRatings { - try clearRatings(appStoreID: storeApp.appStoreID, storefront: outcome.storefront, in: modelContext) - } } func fetchRatings(appStoreID: Int64, storefront: String) async throws -> AppStorefrontRatingResult { @@ -198,34 +192,6 @@ final class AppStorefrontRatingService: Sendable { .filter { !$0.isEmpty } return Array(Set(normalizedStorefronts)).sorted() } - - private func clearRatings(appStoreID: Int64, storefront: String, in modelContext: ModelContext) throws { - let targetAppStoreID = appStoreID - let targetStorefront = storefront - let latestDescriptor = FetchDescriptor( - predicate: #Predicate { latest in - latest.appStoreID == targetAppStoreID && latest.storefront == targetStorefront - } - ) - let snapshotDescriptor = FetchDescriptor( - predicate: #Predicate { snapshot in - snapshot.appStoreID == targetAppStoreID && snapshot.storefront == targetStorefront - } - ) - - try RatingHistoryStore.remove(appStoreID: appStoreID, storefront: storefront, in: modelContext) - let latest = (try? modelContext.fetch(latestDescriptor)) ?? [] - let snapshots = (try? modelContext.fetch(snapshotDescriptor)) ?? [] - for value in latest { - modelContext.delete(value) - } - for value in snapshots { - modelContext.delete(value) - } - OpenASOLog.ratings.info( - "Cleared ratings storefront=\(storefront, privacy: .public) appStoreID=\(appStoreID, privacy: .public) latest=\(latest.count, privacy: .public) snapshots=\(snapshots.count, privacy: .public)" - ) - } } private struct AppStorefrontRatingFetcher: Sendable { diff --git a/OpenASO/Services/RatingsReviews/AppStorefrontRatingTypes.swift b/OpenASO/Services/RatingsReviews/AppStorefrontRatingTypes.swift index b2c4209..b405b20 100644 --- a/OpenASO/Services/RatingsReviews/AppStorefrontRatingTypes.swift +++ b/OpenASO/Services/RatingsReviews/AppStorefrontRatingTypes.swift @@ -25,20 +25,17 @@ struct AppStorefrontRatingRefreshOutcome: Sendable { let result: AppStorefrontRatingResult? let error: OpenASOError? let unavailabilityReason: String? - let clearsStoredRatings: Bool init( storefront: String, result: AppStorefrontRatingResult?, error: OpenASOError?, - unavailabilityReason: String? = nil, - clearsStoredRatings: Bool = false + unavailabilityReason: String? = nil ) { self.storefront = storefront self.result = result self.error = error self.unavailabilityReason = unavailabilityReason - self.clearsStoredRatings = clearsStoredRatings } } diff --git a/OpenASOTests/AppStorefrontRatingServiceTests.swift b/OpenASOTests/AppStorefrontRatingServiceTests.swift index 0957497..55bad13 100644 --- a/OpenASOTests/AppStorefrontRatingServiceTests.swift +++ b/OpenASOTests/AppStorefrontRatingServiceTests.swift @@ -250,7 +250,7 @@ struct AppStorefrontRatingServiceTests { } @Test - func refreshRatingsTreatsLookupMissAsUnavailableStorefrontNotFailure() async throws { + func refreshRatingsPreservesHistoryWhenLookupBecomesUnavailable() async throws { let container = try makeRatingContainer() let modelContext = ModelContext(container) let storeApp = StoreApp( @@ -276,7 +276,9 @@ struct AppStorefrontRatingServiceTests { averageRating: 4.1, storeApp: storeApp )) + try addArchivedRating(for: storeApp, storefront: "ru", in: modelContext) try modelContext.save() + let expectedHistory = try RatingHistoryStore.fetch(appStoreIDs: [storeApp.appStoreID], in: modelContext) var requestedURLs: [String] = [] let progressRecorder = RatingProgressRecorder() @@ -311,8 +313,9 @@ struct AppStorefrontRatingServiceTests { #expect(outcomes.first?.error == nil) #expect(outcomes.first?.unavailabilityReason == "App 6480417616 is not available in RU.") #expect(await progressRecorder.failureCounts == [0, 0]) - #expect(try modelContext.fetch(FetchDescriptor()).isEmpty) - #expect(try modelContext.fetch(FetchDescriptor()).isEmpty) + #expect(try modelContext.fetchCount(FetchDescriptor()) == 1) + #expect(try RatingHistoryStore.fetch(appStoreIDs: [storeApp.appStoreID], in: modelContext) == expectedHistory) + #expect(try modelContext.fetchCount(FetchDescriptor()) == 1) } @Test @@ -524,7 +527,7 @@ struct AppStorefrontRatingServiceTests { } @Test - func refreshRatingsRejectsRedirectedStorefrontAndClearsStaleRows() async throws { + func refreshRatingsRejectsRedirectedStorefrontAndPreservesHistory() async throws { let container = try makeRatingContainer() let modelContext = ModelContext(container) let storeApp = StoreApp( @@ -550,7 +553,9 @@ struct AppStorefrontRatingServiceTests { averageRating: 4.7, storeApp: storeApp )) + try addArchivedRating(for: storeApp, storefront: "ad", in: modelContext) try modelContext.save() + let expectedHistory = try RatingHistoryStore.fetch(appStoreIDs: [storeApp.appStoreID], in: modelContext) var requestURLs: [String] = [] let client = MockHTTPClient { request in @@ -602,8 +607,20 @@ struct AppStorefrontRatingServiceTests { "https://itunes.apple.com/lookup?id=6608976383&country=ad", "https://apps.apple.com/ad/app/id6608976383?l=en-US" ]) - #expect(try modelContext.fetch(FetchDescriptor()).isEmpty) - #expect(try modelContext.fetch(FetchDescriptor()).isEmpty) + #expect(try modelContext.fetchCount(FetchDescriptor()) == 1) + #expect(try RatingHistoryStore.fetch(appStoreIDs: [storeApp.appStoreID], in: modelContext) == expectedHistory) + #expect(try modelContext.fetchCount(FetchDescriptor()) == 1) + } + + private func addArchivedRating(for app: StoreApp, storefront: String, in context: ModelContext) throws { + let older = AppDailyRating(appStoreID: app.appStoreID, storefront: storefront, + ratingCount: 7, averageRating: 4.0, ratingDate: "2020-01-01", + observedAt: Date(timeIntervalSince1970: 1_577_836_800), storeApp: nil) + let chunk = RatingHistoryChunk(appStoreID: app.appStoreID, storefront: storefront, year: "2020") + var record = RatingHistoryRecord(older) + record.storeAppID = app.appStoreID + try RatingHistoryStore.replace([record], in: chunk) + context.insert(chunk) } private static func reviewFeedJSON(entries: [String]) -> String { diff --git a/OpenASOTests/StorageSchemaV7MigrationTests.swift b/OpenASOTests/StorageSchemaV7MigrationTests.swift index 1bbac2b..5200ad6 100644 --- a/OpenASOTests/StorageSchemaV7MigrationTests.swift +++ b/OpenASOTests/StorageSchemaV7MigrationTests.swift @@ -1,6 +1,7 @@ import Foundation import SQLite3 import SwiftData +import Synchronization import Testing @testable import OpenASO @@ -542,8 +543,7 @@ struct StorageSchemaV7MigrationTests { count: 100 + day, in: context) } try context.save() - try service.persist(.init(storefront: "gb", result: nil, error: nil, clearsStoredRatings: true), - for: app, in: context) + try RatingHistoryStore.remove(appStoreID: app.appStoreID, storefront: "gb", in: context) try context.save() #expect(try RatingHistoryStore.fetch(appStoreIDs: [42], in: context).isEmpty) #expect(try context.fetchCount(FetchDescriptor()) == 0) @@ -722,6 +722,211 @@ struct StorageSchemaV7MigrationTests { #expect(state.completed && state.needsCompaction) } + @Test + func boundedPagesRetainExactTotalsDatesTiesAndCorrections() throws { + try withTestStore { container, url in + try seedBoundedHistory(in: container, at: url) + let context = ModelContext(container) + let all = try RatingHistoryStore.fetch(appStoreIDs: [42], in: context) + var cursor: RatingHistoryStore.Cursor? + var collected: [RatingHistoryRecord] = [] + for _ in 0..<100 { + let page = try RatingHistoryStore.page(appStoreID: 42, after: cursor, limit: 7, in: context) + #expect(page.total == all.count) + collected.append(contentsOf: page.records) + guard let last = page.records.last else { break } + cursor = .init(observedAt: last.observedAt, identityKey: last.identityKey) + } + #expect(collected == all) + let from = try Date("2025-01-12T00:00:00Z", strategy: .iso8601) + let to = try Date("2026-01-10T00:00:00Z", strategy: .iso8601) + let expected = try RatingHistoryStore.fetch(appStoreIDs: [42], storefronts: ["gb"], from: from, to: to, in: context) + let page = try RatingHistoryStore.page(appStoreID: 42, storefronts: ["gb"], from: from, to: to, limit: 9, in: context) + #expect(page.total == expected.count) + #expect(page.records == Array(expected.prefix(9))) + #expect(all.first?.ratingDate == "2023-01-01") + } + } + + @Test + func boundedRecentReadsRetainPerAppLogicalDatesAndObservationDates() throws { + try withTestStore { container, url in + try seedBoundedHistory(in: container, at: url) + let context = ModelContext(container) + let all = try RatingHistoryStore.fetch(appStoreIDs: [42, 43], storefronts: ["gb"], in: context) + let recent = try RatingHistoryStore.recent(appStoreIDs: [42, 43], storefronts: ["gb"], + limitPerApp: 31, order: .ratingDate, requiresRatingCount: true, in: context) + for appID: Int64 in [42, 43] { + let expected = all.filter { $0.appStoreID == appID && $0.ratingCount != nil }.sorted { + $0.ratingDate == $1.ratingDate ? $0.observedAt > $1.observedAt : $0.ratingDate > $1.ratingDate + } + #expect(recent.filter { $0.appStoreID == appID } == Array(expected.prefix(31))) + } + let export = try RatingHistoryStore.recent(appStoreIDs: [42], storefronts: ["gb"], limitPerApp: 9, in: context) + #expect(export == Array(all.filter { $0.appStoreID == 42 }.prefix(9))) + } + } + + @Test + func boundedReadsSkipUnneededPayloadsAndStillValidateRequestedHistory() throws { + try withTestStore { container, _ in + let context = ModelContext(container) + for year in [2023, 2026] { + let chunk = RatingHistoryChunk(appStoreID: 42, storefront: "gb", year: String(year)) + let rows = try (1...28).map { day in + let date = String(format: "%d-01-%02d", year, day) + return RatingHistoryRecord(makeRating(day: date, + observedAt: try Date(date + "T00:00:00Z", strategy: .iso8601), app: nil)) + } + try RatingHistoryStore.replace(rows, in: chunk) + if year == 2023 { chunk.payload = Data([1, 2, 3]) } + context.insert(chunk) + } + try context.save() + let fresh = ModelContext(container) + let page = try RatingHistoryStore.page(appStoreID: 42, limit: 5, in: fresh) + #expect(page.records.count == 5 && page.total == 56) + #expect(page.records.allSatisfy { $0.ratingDate.hasPrefix("2026") }) + #expect(try RatingHistoryStore.recent(appStoreIDs: [42], limitPerApp: 5, order: .ratingDate, in: fresh).count == 5) + #expect(throws: (any Error).self) { + try RatingHistoryStore.page(appStoreID: 42, + after: .init(observedAt: Date("2025-01-01T00:00:00Z", strategy: .iso8601), identityKey: ""), limit: 5, in: fresh) + } + } + } + + @Test + func boundedReaderReconcilesLegacyOverlapBeforeApplyingLimits() throws { + try withTestStore { container, _ in + let context = ModelContext(container) + let old = makeRating(day: "2026-01-01", observedAt: Date(timeIntervalSince1970: 10), app: nil) + let chunk = RatingHistoryChunk(appStoreID: 42, storefront: "gb", year: "2026") + try RatingHistoryStore.replace([RatingHistoryRecord(old)], in: chunk) + context.insert(chunk) + context.insert(makeRating(day: old.ratingDate, observedAt: Date(timeIntervalSince1970: 20), app: nil)) + try context.save() + let page = try RatingHistoryStore.page(appStoreID: 42, limit: 5, in: context) + #expect(page.total == 1 && page.records.first?.observedAt == Date(timeIntervalSince1970: 20)) + } + } + + @Test + func migrationReportsCommittedProgressAndCompletedReopeningDoesNotArchive() throws { + try withTestStore { container, url in + let context = ModelContext(container) + for day in ["2026-01-01", "2026-01-02", "2026-01-03"] { + context.insert(makeRating(day: day, observedAt: .now, app: nil)) + } + try context.save() + let events = Mutex<[StoragePreparationProgress]>([]) + try StorageSchemaV7Migrator.migrateIfNeeded(at: url, progress: { event in events.withLock { $0.append(event) } }) + let values = events.withLock { $0 } + #expect(values.first == .archiving(completed: 0, total: 3)) + #expect(values.last == .archiving(completed: 3, total: 3)) + events.withLock { $0.removeAll() } + try StorageSchemaV7Migrator.migrateIfNeeded(at: url, progress: { event in events.withLock { $0.append(event) } }) + #expect(events.withLock { $0.isEmpty }) + } + } + + @Test + func explicitClearRollsBackLiveArchivedAndLatestTogether() async throws { + let container = try ModelContainerFactory.makeModelContainer(isStoredInMemoryOnly: true) + let context = ModelContext(container) + let service = AppStorefrontRatingService(httpClient: PreviewHTTPClient()) + let app = StoreApp(appStoreID: 42, bundleID: nil, name: "App", sellerName: nil, iconURLString: nil, defaultPlatform: .iphone) + context.insert(app) + let start = try Date("2026-01-01T15:00:00Z", strategy: .iso8601) + for day in 0..<3 { try ingestRating(service, app: app, observedAt: start.addingTimeInterval(Double(day) * 86_400), count: day, in: context) } + try context.save() + let expected = try RatingHistoryStore.fetch(appStoreIDs: [42], in: context) + let worker = BackgroundModelStore(modelContainer: container) + await #expect(throws: InjectedStorageInterruption.self) { + try await worker.write { context in + try RatingHistoryStore.remove(appStoreID: 42, storefront: "gb", in: context) + throw InjectedStorageInterruption.stop + } + } + #expect(try RatingHistoryStore.fetch(appStoreIDs: [42], in: ModelContext(container)) == expected) + #expect(try ModelContext(container).fetchCount(FetchDescriptor()) == 1) + } + + @Test + func boundedPagesRemainCoherentDuringConcurrentRollover() async throws { + let root = FileManager.default.temporaryDirectory.appendingPathComponent("OpenASO-PageRace-\(UUID())") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + let url = root.appendingPathComponent("default.store") + let container = try ModelContainerFactory.makePersistentModelContainer(at: url) + let writer = BackgroundModelStore(modelContainer: container) + let start = Date(timeIntervalSince1970: 1_735_689_600) + try await writer.write { context in + try RatingHistoryStore.ingest((0..<200).map { day in + .init(.init(appStoreID: 42, storefront: "gb", ratingCount: day, averageRating: 4.5, + ratingCounts: nil, observedAt: start.addingTimeInterval(Double(day) * 86_400), source: .iTunesSearch)) + }, storeApps: [:], in: context) + } + let readers = try (0..<2).map { _ in + BackgroundModelStore(modelContainer: try ModelContainerFactory.makePersistentModelContainer(at: url)) + } + try await withThrowingTaskGroup(of: Void.self) { group in + group.addTask { + for day in 200..<250 { + try await writer.write { context in + try RatingHistoryStore.ingest([.init(.init(appStoreID: 42, storefront: "gb", ratingCount: day, + averageRating: 4.5, ratingCounts: nil, observedAt: start.addingTimeInterval(Double(day) * 86_400), + source: .iTunesSearch))], storeApps: [:], in: context) + } + } + } + for reader in readers { + group.addTask { + for _ in 0..<100 { + let page = try await reader.read { try RatingHistoryStore.page(appStoreID: 42, limit: 1000, in: $0) } + #expect(page.records.count == page.total) + #expect((200...250).contains(page.total)) + } + } + } + try await group.waitForAll() + } + } + + @Test + func boundedReadsWithinWriteTransactionsSeePendingHistory() async throws { + let container = try ModelContainerFactory.makeModelContainer(isStoredInMemoryOnly: true) + let writer = BackgroundModelStore(modelContainer: container) + let page = try await writer.write { context in + try RatingHistoryStore.ingest((0..<2).map { day in + .init(.init(appStoreID: 42, storefront: "gb", ratingCount: day, averageRating: 4.5, + ratingCounts: nil, observedAt: Date(timeIntervalSince1970: 1_735_689_600 + Double(day) * 86_400), + source: .iTunesSearch)) + }, storeApps: [:], in: context) + return try RatingHistoryStore.page(appStoreID: 42, limit: 10, in: context) + } + #expect(page.total == 2 && page.records.count == 2) + } + + private func seedBoundedHistory(in container: ModelContainer, at url: URL) throws { + let context = ModelContext(container) + for appID: Int64 in [42, 43] { + for storefront in ["gb", "us"] { + for year in [2023, 2025, 2026] { + for day in 1...28 { + let date = String(format: "%d-01-%02d", year, day) + let observed = try Date(date + "T00:00:00Z", strategy: .iso8601) + let value = AppDailyRating(appStoreID: appID, storefront: storefront, + ratingCount: day.isMultiple(of: 7) ? nil : day, averageRating: 4.5, + ratingDate: date, observedAt: year == 2023 && day == 1 ? observed.addingTimeInterval(126_230_400) : observed) + context.insert(value) + } + } + } + } + try context.save() + try StorageSchemaV7Migrator.migrateIfNeeded(at: url) + } + private func ingestRating(_ service: AppStorefrontRatingService, app: StoreApp, observedAt: Date, count: Int, in context: ModelContext) throws { try service.persist(.init(storefront: "gb", result: .init(