diff --git a/Split.xcodeproj/project.pbxproj b/Split.xcodeproj/project.pbxproj index 1adce743b..6aab8e1a9 100644 --- a/Split.xcodeproj/project.pbxproj +++ b/Split.xcodeproj/project.pbxproj @@ -1098,6 +1098,8 @@ C53207EC2EF44A2F00418BB1 /* PersistenceBreaker.swift in Sources */ = {isa = PBXBuildFile; fileRef = C53207EA2EF44A2F00418BB1 /* PersistenceBreaker.swift */; }; C53207EE2EF452C000418BB1 /* CoreDataHelperStub.swift in Sources */ = {isa = PBXBuildFile; fileRef = C53207ED2EF452C000418BB1 /* CoreDataHelperStub.swift */; }; C53207F12EF456AF00418BB1 /* CoreDataHelperTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C53207F02EF456AF00418BB1 /* CoreDataHelperTests.swift */; }; + C53207F42EF45BC600418BB1 /* PersistentSplitsStorageTransactionalTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C53207F22EF45BC600418BB1 /* PersistentSplitsStorageTransactionalTests.swift */; }; + C53207F52EF45BC600418BB1 /* SplitsPersistenceBreakerIntegrationTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = C53207F32EF45BC600418BB1 /* SplitsPersistenceBreakerIntegrationTest.swift */; }; C539CAB62D88C1F10050C732 /* RuleBasedSegment.swift in Sources */ = {isa = PBXBuildFile; fileRef = C539CAB52D88C1F10050C732 /* RuleBasedSegment.swift */; }; C539CABE2D88C7410050C732 /* PersistentRuleBasedSegmentsStorage.swift in Sources */ = {isa = PBXBuildFile; fileRef = C539CABC2D88C7410050C732 /* PersistentRuleBasedSegmentsStorage.swift */; }; C539CABF2D88C7410050C732 /* RuleBasedSegmentsStorage.swift in Sources */ = {isa = PBXBuildFile; fileRef = C539CABD2D88C7410050C732 /* RuleBasedSegmentsStorage.swift */; }; @@ -2010,6 +2012,8 @@ C53207EA2EF44A2F00418BB1 /* PersistenceBreaker.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PersistenceBreaker.swift; sourceTree = ""; }; C53207ED2EF452C000418BB1 /* CoreDataHelperStub.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CoreDataHelperStub.swift; sourceTree = ""; }; C53207F02EF456AF00418BB1 /* CoreDataHelperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CoreDataHelperTests.swift; sourceTree = ""; }; + C53207F22EF45BC600418BB1 /* PersistentSplitsStorageTransactionalTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PersistentSplitsStorageTransactionalTests.swift; sourceTree = ""; }; + C53207F32EF45BC600418BB1 /* SplitsPersistenceBreakerIntegrationTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SplitsPersistenceBreakerIntegrationTest.swift; sourceTree = ""; }; C539CAB52D88C1F10050C732 /* RuleBasedSegment.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RuleBasedSegment.swift; sourceTree = ""; }; C539CABC2D88C7410050C732 /* PersistentRuleBasedSegmentsStorage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PersistentRuleBasedSegmentsStorage.swift; sourceTree = ""; }; C539CABD2D88C7410050C732 /* RuleBasedSegmentsStorage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RuleBasedSegmentsStorage.swift; sourceTree = ""; }; @@ -2687,6 +2691,8 @@ 5905D4E3255B2373006DA3B1 /* Storage */ = { isa = PBXGroup; children = ( + C53207F22EF45BC600418BB1 /* PersistentSplitsStorageTransactionalTests.swift */, + C53207F32EF45BC600418BB1 /* SplitsPersistenceBreakerIntegrationTest.swift */, C53207F02EF456AF00418BB1 /* CoreDataHelperTests.swift */, C53207E82EF44A2100418BB1 /* PersistenceBreakerTests.swift */, C52C572A2EEB41450064D049 /* EncryptionKeyValidationTest.swift */, @@ -4776,6 +4782,8 @@ 592C6AC4211B718E002D120C /* RegexTest.swift in Sources */, 9500D9922C56F9BA00383593 /* HostDomainFilterTests.swift in Sources */, 95F3F0282590D81B00084AF8 /* ImpressionsRecorderWorkerTests.swift in Sources */, + C53207F42EF45BC600418BB1 /* PersistentSplitsStorageTransactionalTests.swift in Sources */, + C53207F52EF45BC600418BB1 /* SplitsPersistenceBreakerIntegrationTest.swift in Sources */, 955E12312BFBEA8600AE6D10 /* HashedImpressionDaoTest.swift in Sources */, 95342E992A4C71300045B201 /* FeatureFlagsPayloadDecoderMock.swift in Sources */, 9577A8472683B8C800D92AE1 /* HttpImpressionsCountRecorderTests.swift in Sources */, diff --git a/Split/Api/SplitDatabaseHelper.swift b/Split/Api/SplitDatabaseHelper.swift index 48f4b5dd8..ff7eb7452 100644 --- a/Split/Api/SplitDatabaseHelper.swift +++ b/Split/Api/SplitDatabaseHelper.swift @@ -91,7 +91,13 @@ struct SplitDatabaseHelper { DefaultFlagSetsCache(setsInFilter: splitClientConfig.bySetsFilter()?.values.asSet()) let persistentSplitsStorage = DefaultPersistentSplitsStorage(database: splitDatabase) let generalInfoStorage = openGeneralInfoStorage(database: splitDatabase) - let splitsStorage = openSplitsStorage(database: splitDatabase, flagSetsCache: flagSetsCache, generalInfoStorage: generalInfoStorage) + + // Create shared persistence breaker for targeting rules (splits + RBS) + let targetingRulesPersistenceBreaker = DefaultPersistenceBreaker() + let splitsStorage = openSplitsStorage(database: splitDatabase, + flagSetsCache: flagSetsCache, + generalInfoStorage: generalInfoStorage, + persistenceBreaker: targetingRulesPersistenceBreaker) let persistentImpressionsStorage = openPersistentImpressionsStorage(database: splitDatabase) let impressionsStorage = openImpressionsStorage(persistentStorage: persistentImpressionsStorage) @@ -139,7 +145,8 @@ struct SplitDatabaseHelper { hashedImpressionsStorage: hashedImpressionsStorage, generalInfoStorage: generalInfoStorage, ruleBasedSegmentsStorage: ruleBasedSegmentsStorage, - persistentRuleBasedSegmentsStorage: persistentRuleBasedSegmentsStorage) + persistentRuleBasedSegmentsStorage: persistentRuleBasedSegmentsStorage, + targetingRulesPersistenceBreaker: targetingRulesPersistenceBreaker) } static func openDatabase(dataFolderName: String, @@ -158,9 +165,11 @@ struct SplitDatabaseHelper { } static func openSplitsStorage(database: SplitDatabase, - flagSetsCache: FlagSetsCache, generalInfoStorage: GeneralInfoStorage) -> SplitsStorage { + flagSetsCache: FlagSetsCache, generalInfoStorage: GeneralInfoStorage, + persistenceBreaker: PersistenceBreaker) -> SplitsStorage { return DefaultSplitsStorage(persistentSplitsStorage: openPersistentSplitsStorage(database: database), - flagSetsCache: flagSetsCache, GeneralInfoStorage: generalInfoStorage) + flagSetsCache: flagSetsCache, generalInfoStorage: generalInfoStorage, + persistenceBreaker: persistenceBreaker) } static func openPersistentMySegmentsStorage(database: SplitDatabase) -> PersistentMySegmentsStorage { diff --git a/Split/Network/Sync/SyncCommons.swift b/Split/Network/Sync/SyncCommons.swift index fb3313ae9..e8df6c3a0 100644 --- a/Split/Network/Sync/SyncCommons.swift +++ b/Split/Network/Sync/SyncCommons.swift @@ -26,6 +26,7 @@ struct SplitStorageContainer { let generalInfoStorage: GeneralInfoStorage let ruleBasedSegmentsStorage: RuleBasedSegmentsStorage let persistentRuleBasedSegmentsStorage: PersistentRuleBasedSegmentsStorage + let targetingRulesPersistenceBreaker: PersistenceBreaker } protocol ImpressionLogger { diff --git a/Split/Storage/Splits/PersistentSplitsStorage.swift b/Split/Storage/Splits/PersistentSplitsStorage.swift index b082b5825..88a122f12 100644 --- a/Split/Storage/Splits/PersistentSplitsStorage.swift +++ b/Split/Storage/Splits/PersistentSplitsStorage.swift @@ -9,7 +9,7 @@ import Foundation protocol PersistentSplitsStorage { - func update(splitChange: ProcessedSplitChange) + func update(splitChange: ProcessedSplitChange, onFailure: ((Error) -> Void)?) func update(split: Split) func update(bySetsFilter: SplitFilter?) func getBySetsFilter() -> SplitFilter? @@ -25,17 +25,39 @@ class DefaultPersistentSplitsStorage: PersistentSplitsStorage { private let splitDao: SplitDao private let generalInfoDao: GeneralInfoDao + private let coreDataHelper: CoreDataHelper init(database: SplitDatabase) { self.splitDao = database.splitDao self.generalInfoDao = database.generalInfoDao + if let testDb = database as? TestSplitDatabase { + self.coreDataHelper = testDb.coreDataHelper + } else { + fatalError("Database must provide CoreDataHelper for transactional operations") + } } - func update(splitChange: ProcessedSplitChange) { - splitDao.insertOrUpdate(splits: splitChange.activeSplits) - splitDao.delete(splitChange.archivedSplits.compactMap { return $0.name }) - generalInfoDao.update(info: .splitsChangeNumber, longValue: splitChange.changeNumber) - generalInfoDao.update(info: .splitsUpdateTimestamp, longValue: splitChange.updateTimestamp) + func update(splitChange: ProcessedSplitChange, onFailure: ((Error) -> Void)? = nil) { + // Execute transactionally: all operations must succeed or all must fail + coreDataHelper.performAndWait { [weak self] in + guard let self = self else { return } + + do { + // All operations within this block happen in the same CoreData context + self.splitDao.transactionalInsertOrUpdate(splits: splitChange.activeSplits) + let archivedNames = splitChange.archivedSplits.compactMap { $0.name } + self.splitDao.transactionalDelete(archivedNames) + + self.generalInfoDao.transactionalUpdate(info: .splitsChangeNumber, longValue: splitChange.changeNumber) + self.generalInfoDao.transactionalUpdate(info: .splitsUpdateTimestamp, longValue: splitChange.updateTimestamp) + + // Save everything as one transaction + try self.coreDataHelper.saveWithErrorHandling() + } catch { + Logger.e("Transactional flags update failed: \(error.localizedDescription)") + onFailure?(error) + } + } } func update(split: Split) { diff --git a/Split/Storage/Splits/SplitDao.swift b/Split/Storage/Splits/SplitDao.swift index bb2403be1..137f9b2af 100644 --- a/Split/Storage/Splits/SplitDao.swift +++ b/Split/Storage/Splits/SplitDao.swift @@ -19,7 +19,7 @@ protocol SplitDao { /// Synchronous insert/update for use in transactions /// Caller must call coreDataHelper.saveWithErrorHandling() - func transactionalInsertOrUpdate(splits: [Split]) throws + func transactionalInsertOrUpdate(splits: [Split]) /// Synchronous delete for use in transactions /// Caller must call coreDataHelper.saveWithErrorHandling() @@ -121,7 +121,7 @@ class CoreDataSplitDao: BaseCoreDataDao, SplitDao { } /// Synchronous insert/update that does NOT save - func transactionalInsertOrUpdate(splits: [Split]) throws { + func transactionalInsertOrUpdate(splits: [Split]) { let parsed = self.encoder.encode(splits) for (name, json) in parsed { if let obj = self.getBy(name: name) ?? self.coreDataHelper.create(entity: .split) as? SplitEntity { diff --git a/Split/Storage/Splits/SplitsStorage.swift b/Split/Storage/Splits/SplitsStorage.swift index 732edbec4..02a147f79 100644 --- a/Split/Storage/Splits/SplitsStorage.swift +++ b/Split/Storage/Splits/SplitsStorage.swift @@ -36,18 +36,21 @@ class DefaultSplitsStorage: SplitsStorage { private var trafficTypes: SynchronizedDictionary private let flagSetsCache: FlagSetsCache private let generalInfoStorage: GeneralInfoStorage + private let persistenceBreaker: PersistenceBreaker private(set) var changeNumber: Int64 = -1 private(set) var updateTimestamp: Int64 = -1 init(persistentSplitsStorage: PersistentSplitsStorage, flagSetsCache: FlagSetsCache, - GeneralInfoStorage: GeneralInfoStorage) { + generalInfoStorage: GeneralInfoStorage, + persistenceBreaker: PersistenceBreaker) { self.persistentStorage = persistentSplitsStorage self.inMemorySplits = SynchronizedDictionary() self.trafficTypes = SynchronizedDictionary() self.flagSetsCache = flagSetsCache - self.generalInfoStorage = GeneralInfoStorage + self.generalInfoStorage = generalInfoStorage + self.persistenceBreaker = persistenceBreaker } func loadLocal() { @@ -86,26 +89,39 @@ class DefaultSplitsStorage: SplitsStorage { func update(splitChange: ProcessedSplitChange) -> Bool { - // Process + // Process in-memory updates (always happens) let updated = processUpdated(splits: splitChange.activeSplits, active: true) let removed = processUpdated(splits: splitChange.archivedSplits, active: false) - // Update + // Update in-memory metadata (always happens) changeNumber = splitChange.changeNumber updateTimestamp = splitChange.updateTimestamp - persistentStorage.update(splitChange: splitChange) + + // Attempt persistence only if breaker allows + if persistenceBreaker.isPersistenceEnabled { + persistentStorage.update(splitChange: splitChange, onFailure: { [weak self] _ in + // On first failure, disable persistence for remainder of session + self?.persistenceBreaker.disable() + }) + } return updated || removed } func update(bySetsFilter filter: SplitFilter?) { - self.persistentStorage.update(bySetsFilter: filter) + // Only call persistence if breaker allows + if persistenceBreaker.isPersistenceEnabled { + self.persistentStorage.update(bySetsFilter: filter) + } } func updateWithoutChecks(split: Split) { if let splitName = split.name?.lowercased() { inMemorySplits.setValue(split, forKey: splitName) - persistentStorage.update(split: split) + // Only call persistence if breaker allows + if persistenceBreaker.isPersistenceEnabled { + persistentStorage.update(split: split) + } } } @@ -296,7 +312,8 @@ class BackgroundSyncSplitsStorage: SyncSplitsStorage { } func update(splitChange: ProcessedSplitChange) -> Bool { - persistentStorage.update(splitChange: splitChange) + // If persistence fails, it will be logged but won't trigger breaker + persistentStorage.update(splitChange: splitChange, onFailure: nil) return true } diff --git a/SplitTests/Fake/Storage/PersistentSplitsStorageStub.swift b/SplitTests/Fake/Storage/PersistentSplitsStorageStub.swift index a26e1bf6b..5c4334a78 100644 --- a/SplitTests/Fake/Storage/PersistentSplitsStorageStub.swift +++ b/SplitTests/Fake/Storage/PersistentSplitsStorageStub.swift @@ -43,7 +43,7 @@ class PersistentSplitsStorageStub: PersistentSplitsStorage { self.init(delegate: nil) } - func update(splitChange: ProcessedSplitChange) { + func update(splitChange: ProcessedSplitChange, onFailure: ((Error) -> Void)? = nil) { processedSplitChange = splitChange changeNumber = splitChange.changeNumber updateCalled = true diff --git a/SplitTests/Fake/Storage/SplitDaoStub.swift b/SplitTests/Fake/Storage/SplitDaoStub.swift index b0c32f131..0078f3fb9 100644 --- a/SplitTests/Fake/Storage/SplitDaoStub.swift +++ b/SplitTests/Fake/Storage/SplitDaoStub.swift @@ -39,7 +39,7 @@ class SplitDaoStub: SplitDao { deleteAllCalled = true } - func transactionalInsertOrUpdate(splits: [Split]) throws { + func transactionalInsertOrUpdate(splits: [Split]) { insertedSplits = splits } diff --git a/SplitTests/Helpers/TestingHelper.swift b/SplitTests/Helpers/TestingHelper.swift index c908d1cd7..e377ccfc4 100644 --- a/SplitTests/Helpers/TestingHelper.swift +++ b/SplitTests/Helpers/TestingHelper.swift @@ -198,7 +198,8 @@ struct TestingHelper { hashedImpressionsStorage: HashedImpressionsStorageMock(), generalInfoStorage: GeneralInfoStorageMock(), ruleBasedSegmentsStorage: RuleBasedSegmentsStorageStub(), - persistentRuleBasedSegmentsStorage: PersistentRuleBasedSegmentsStorageStub()) + persistentRuleBasedSegmentsStorage: PersistentRuleBasedSegmentsStorageStub(), + targetingRulesPersistenceBreaker: DefaultPersistenceBreaker()) } static func createApiFacade() -> SplitApiFacade { diff --git a/SplitTests/Storage/PersistentSplitsStorageTests.swift b/SplitTests/Storage/PersistentSplitsStorageTests.swift index 48c17dbda..cc5eccb98 100644 --- a/SplitTests/Storage/PersistentSplitsStorageTests.swift +++ b/SplitTests/Storage/PersistentSplitsStorageTests.swift @@ -31,7 +31,7 @@ class PersistentSplitsStorageTest: XCTestCase { let archivedSplits = [newSplit(name: "ar1", trafficType: "t2", status: .archived), newSplit(name: "ar2", trafficType: "t2", status: .archived)] let change = ProcessedSplitChange(activeSplits: activeSplits, archivedSplits: archivedSplits, changeNumber: 100, updateTimestamp: 200) - splitsStorage.update(splitChange: change) + splitsStorage.update(splitChange: change, onFailure: nil) XCTAssertEqual(3, splitDao.insertedSplits.count) XCTAssertEqual(2, splitDao.deletedSplits?.count) diff --git a/SplitTests/Storage/PersistentSplitsStorageTransactionalTests.swift b/SplitTests/Storage/PersistentSplitsStorageTransactionalTests.swift new file mode 100644 index 000000000..76856bb7f --- /dev/null +++ b/SplitTests/Storage/PersistentSplitsStorageTransactionalTests.swift @@ -0,0 +1,106 @@ +// +// PersistentSplitsStorageTransactionalTests.swift +// SplitTests +// + +import Foundation +import XCTest +@testable import Split + +class PersistentSplitsStorageTransactionalTests: XCTestCase { + + var splitsStorage: PersistentSplitsStorage! + var splitDao: SplitDaoStub! + var generalInfoDao: GeneralInfoDaoStub! + var coreDataHelperStub: CoreDataHelperStub! + + override func setUp() { + splitDao = SplitDaoStub() + generalInfoDao = GeneralInfoDaoStub() + coreDataHelperStub = CoreDataHelperStub() + var daoProvider = CoreDataDaoProviderMock() + daoProvider.splitDao = splitDao + daoProvider.generalInfoDao = generalInfoDao + splitsStorage = DefaultPersistentSplitsStorage( + database: SplitDatabaseStub(daoProvider: daoProvider, coreDataHelper: coreDataHelperStub) + ) + } + + func testSuccessDoesNotInvokeFailureCallback() { + let change = ProcessedSplitChange( + activeSplits: [createSplit(name: "split1")], + archivedSplits: [], + changeNumber: 100, + updateTimestamp: 1000 + ) + + var failureWasReported = false + + splitsStorage.update(splitChange: change, onFailure: { _ in + failureWasReported = true + }) + + XCTAssertFalse(failureWasReported, "With working stubs, no failure should occur") + } + + func testFailureCallbackIsInvokedOnSaveError() { + coreDataHelperStub.shouldFailOnSave = true + + let change = ProcessedSplitChange( + activeSplits: [createSplit(name: "split1")], + archivedSplits: [], + changeNumber: 100, + updateTimestamp: 1000 + ) + + var failureWasReported = false + var reportedError: Error? + + splitsStorage.update(splitChange: change, onFailure: { error in + failureWasReported = true + reportedError = error + }) + + XCTAssertTrue(failureWasReported, "Failure callback should be invoked on save error") + XCTAssertNotNil(reportedError, "Error should be reported") + } + + func testNilFailureCallbackIsHandled() { + let change = ProcessedSplitChange( + activeSplits: [createSplit(name: "split1")], + archivedSplits: [], + changeNumber: 100, + updateTimestamp: 1000 + ) + + splitsStorage.update(splitChange: change, onFailure: nil) + + XCTAssertEqual(1, splitDao.insertedSplits.count) + XCTAssertEqual(100, generalInfoDao.longValue(info: .splitsChangeNumber)) + } + + func testSplitsAndGeneralInfoAreUpdatedTogether() { + let activeSplits = [createSplit(name: "s1"), createSplit(name: "s2")] + let archivedSplits = [createSplit(name: "s3")] + let change = ProcessedSplitChange( + activeSplits: activeSplits, + archivedSplits: archivedSplits, + changeNumber: 200, + updateTimestamp: 2000 + ) + + splitsStorage.update(splitChange: change, onFailure: nil) + + XCTAssertEqual(2, splitDao.insertedSplits.count, "Active splits should be inserted") + XCTAssertEqual(1, splitDao.deletedSplits?.count, "Archived splits should be deleted") + XCTAssertEqual(200, generalInfoDao.longValue(info: .splitsChangeNumber), "ChangeNumber should be updated") + XCTAssertEqual(2000, generalInfoDao.longValue(info: .splitsUpdateTimestamp), "UpdateTimestamp should be updated") + } + + private func createSplit(name: String, trafficType: String = "user") -> Split { + let split = SplitTestHelper.newSplit(name: name, trafficType: trafficType) + split.status = .active + return split + } +} + diff --git a/SplitTests/Storage/SplitDaoTest.swift b/SplitTests/Storage/SplitDaoTest.swift index 9926c8e0b..9ae5a324d 100644 --- a/SplitTests/Storage/SplitDaoTest.swift +++ b/SplitTests/Storage/SplitDaoTest.swift @@ -163,7 +163,7 @@ class SplitDaoTest: XCTestCase { let newSplits = [newSplit(name: "transactional_split_1", trafficType: "user")] coreDataDao.coreDataHelper.performAndWait { - try? coreDataDao.transactionalInsertOrUpdate(splits: newSplits) + coreDataDao.transactionalInsertOrUpdate(splits: newSplits) } // Save manually @@ -186,7 +186,7 @@ class SplitDaoTest: XCTestCase { let updatedSplit = newSplit(name: "feat_0", trafficType: "updated_type") coreDataDao.coreDataHelper.performAndWait { - try? coreDataDao.transactionalInsertOrUpdate(splits: [updatedSplit]) + coreDataDao.transactionalInsertOrUpdate(splits: [updatedSplit]) } coreDataDao.coreDataHelper.save() diff --git a/SplitTests/Storage/SplitsPersistenceBreakerIntegrationTest.swift b/SplitTests/Storage/SplitsPersistenceBreakerIntegrationTest.swift new file mode 100644 index 000000000..c509f9f50 --- /dev/null +++ b/SplitTests/Storage/SplitsPersistenceBreakerIntegrationTest.swift @@ -0,0 +1,164 @@ +// +// SplitsPersistenceBreakerIntegrationTest.swift +// SplitTests +// + +import Foundation +import XCTest +@testable import Split + +class SplitsPersistenceBreakerIntegrationTest: XCTestCase { + + var splitsStorage: DefaultSplitsStorage! + var persistentStorage: FailingPersistentSplitsStorage! + var flagSetsCache: FlagSetsCacheMock! + var generalInfoStorage: GeneralInfoStorageMock! + var persistenceBreaker: DefaultPersistenceBreaker! + + override func setUp() { + super.setUp() + persistentStorage = FailingPersistentSplitsStorage() + flagSetsCache = FlagSetsCacheMock() + generalInfoStorage = GeneralInfoStorageMock() + persistenceBreaker = DefaultPersistenceBreaker() + + splitsStorage = DefaultSplitsStorage( + persistentSplitsStorage: persistentStorage, + flagSetsCache: flagSetsCache, + generalInfoStorage: generalInfoStorage, + persistenceBreaker: persistenceBreaker + ) + } + + func testFirstPersistenceFailureDisablesFurtherPersistence() { + persistentStorage.shouldFail = true + persistentStorage.failOnCallNumber = 1 + + let change1 = ProcessedSplitChange( + activeSplits: [createSplit(name: "split1")], + archivedSplits: [], + changeNumber: 100, + updateTimestamp: 1000 + ) + + let change2 = ProcessedSplitChange( + activeSplits: [createSplit(name: "split2")], + archivedSplits: [], + changeNumber: 200, + updateTimestamp: 2000 + ) + + _ = splitsStorage.update(splitChange: change1) + + XCTAssertEqual(1, persistentStorage.updateCallCount, "First update should attempt persistence") + + _ = splitsStorage.update(splitChange: change2) + + XCTAssertEqual(1, persistentStorage.updateCallCount, + "After first failure, no further persistence calls should occur") + } + + func testInMemorySplitsStillWorkAfterPersistenceDisabled() { + persistentStorage.shouldFail = true + persistentStorage.failOnCallNumber = 1 + + let change1 = ProcessedSplitChange( + activeSplits: [createSplit(name: "split1")], + archivedSplits: [], + changeNumber: 100, + updateTimestamp: 1000 + ) + + let change2 = ProcessedSplitChange( + activeSplits: [createSplit(name: "split2")], + archivedSplits: [], + changeNumber: 200, + updateTimestamp: 2000 + ) + + _ = splitsStorage.update(splitChange: change1) + _ = splitsStorage.update(splitChange: change2) + + XCTAssertNotNil(splitsStorage.get(name: "split1"), + "First split should be in memory despite persistence failure") + XCTAssertNotNil(splitsStorage.get(name: "split2"), + "Second split should be in memory (persistence skipped)") + + XCTAssertEqual(200, splitsStorage.changeNumber, + "In-memory change number should advance even when persistence disabled") + XCTAssertEqual(2000, splitsStorage.updateTimestamp, + "In-memory timestamp should advance even when persistence disabled") + } + + private func createSplit(name: String, trafficType: String = "user") -> Split { + let split = SplitTestHelper.newSplit(name: name, trafficType: trafficType) + split.status = .active + return split + } +} + +class FailingPersistentSplitsStorage: PersistentSplitsStorage { + + var shouldFail = false + var failOnCallNumber: Int = 1 + var updateCallCount = 0 + var failureReported = false + + private var snapshot = SplitsSnapshot(changeNumber: -1, splits: [], updateTimestamp: -1) + + func update(splitChange: ProcessedSplitChange, onFailure: ((Error) -> Void)? = nil) { + updateCallCount += 1 + + if shouldFail && updateCallCount == failOnCallNumber { + // Simulate a CoreData save() failure + let error = NSError(domain: "TestCoreData", code: 1001, userInfo: [NSLocalizedDescriptionKey: "Simulated CoreData save failure"]) + failureReported = true + onFailure?(error) + return + } + + // Normal success path (not failing) + snapshot = SplitsSnapshot( + changeNumber: splitChange.changeNumber, + splits: splitChange.activeSplits, + updateTimestamp: splitChange.updateTimestamp + ) + } + + func update(split: Split) { + // No-op for this test + } + + func update(bySetsFilter: SplitFilter?) { + // No-op for this test + } + + func getBySetsFilter() -> SplitFilter? { + return nil + } + + func getSplitsSnapshot() -> SplitsSnapshot { + return snapshot + } + + func getChangeNumber() -> Int64 { + return snapshot.changeNumber + } + + func getUpdateTimestamp() -> Int64 { + return snapshot.updateTimestamp + } + + func getAll() -> [Split] { + return snapshot.splits + } + + func delete(splitNames: [String]) { + // No-op for this test + } + + func clear() { + snapshot = SplitsSnapshot(changeNumber: -1, splits: [], updateTimestamp: -1) + } +} + diff --git a/SplitTests/Storage/SplitsStorageTests.swift b/SplitTests/Storage/SplitsStorageTests.swift index 508e31396..335a970d3 100644 --- a/SplitTests/Storage/SplitsStorageTests.swift +++ b/SplitTests/Storage/SplitsStorageTests.swift @@ -25,7 +25,7 @@ class SplitsStorageTest: XCTestCase { override func setUp() { persistentStorage = PersistentSplitsStorageStub() flagSetsCache = FlagSetsCacheMock() - splitsStorage = DefaultSplitsStorage(persistentSplitsStorage: persistentStorage, flagSetsCache: flagSetsCache, GeneralInfoStorage: generalInfoStorage) + splitsStorage = DefaultSplitsStorage(persistentSplitsStorage: persistentStorage, flagSetsCache: flagSetsCache, generalInfoStorage: generalInfoStorage, persistenceBreaker: DefaultPersistenceBreaker()) } func testNoLocalLoaded() { @@ -40,7 +40,7 @@ class SplitsStorageTest: XCTestCase { } func testLazyParsing() { - noLoadedStorage = DefaultSplitsStorage(persistentSplitsStorage: createPersistentStorageStub(isParsed: false), flagSetsCache: FlagSetsCacheMock(), GeneralInfoStorage: generalInfoStorage) + noLoadedStorage = DefaultSplitsStorage(persistentSplitsStorage: createPersistentStorageStub(isParsed: false), flagSetsCache: FlagSetsCacheMock(), generalInfoStorage: generalInfoStorage, persistenceBreaker: DefaultPersistenceBreaker()) noLoadedStorage?.loadLocal() @@ -278,7 +278,7 @@ class SplitsStorageTest: XCTestCase { let flagSetsCache = FlagSetsCacheMock() flagSetsCache.setsInFilter = ["set1", "set2", "set3"] - splitsStorage = DefaultSplitsStorage(persistentSplitsStorage: persistentStorage, flagSetsCache: flagSetsCache, GeneralInfoStorage: GeneralInfoStorageMock()) + splitsStorage = DefaultSplitsStorage(persistentSplitsStorage: persistentStorage, flagSetsCache: flagSetsCache, generalInfoStorage: GeneralInfoStorageMock(), persistenceBreaker: DefaultPersistenceBreaker()) persistentStorage.snapshot = getTestSnapshot(count: 3, sets: [ ["set1", "set2"], ["set1"], @@ -472,7 +472,7 @@ private class MockPersistentSplitsSegmentsStorage: PersistentSplitsStorage { self.segmentsInUse = segmentsInUse } - func update(splitChange: ProcessedSplitChange) { + func update(splitChange: ProcessedSplitChange, onFailure: ((Error) -> Void)?) { // No-op for the mock } diff --git a/SplitTests/Storage/SplitsStorageTrafficTypesTests.swift b/SplitTests/Storage/SplitsStorageTrafficTypesTests.swift index 39228a116..bc316e242 100644 --- a/SplitTests/Storage/SplitsStorageTrafficTypesTests.swift +++ b/SplitTests/Storage/SplitsStorageTrafficTypesTests.swift @@ -27,7 +27,8 @@ class SplitsStorageTrafficTypesTests: XCTestCase { flagSetsCache = FlagSetsCacheMock() persistent.snapshot = SplitsSnapshot(changeNumber: 1, splits: splits, updateTimestamp: 100) - splitsStorage = DefaultSplitsStorage(persistentSplitsStorage: persistent, flagSetsCache: flagSetsCache, GeneralInfoStorage: generalInfoStorage) + splitsStorage = DefaultSplitsStorage(persistentSplitsStorage: persistent, flagSetsCache: flagSetsCache, generalInfoStorage: generalInfoStorage, + persistenceBreaker: DefaultPersistenceBreaker()) splitsStorage.loadLocal() } diff --git a/SplitTests/Streaming/FeatureFlagsSynchronizerTest.swift b/SplitTests/Streaming/FeatureFlagsSynchronizerTest.swift index ab794172c..e06d5b629 100644 --- a/SplitTests/Streaming/FeatureFlagsSynchronizerTest.swift +++ b/SplitTests/Streaming/FeatureFlagsSynchronizerTest.swift @@ -79,7 +79,8 @@ class FeatureFlagsSynchronizerTest: XCTestCase { hashedImpressionsStorage: HashedImpressionsStorageMock(), generalInfoStorage: self.generalInfoStorage!, ruleBasedSegmentsStorage: ruleBasedSegmentsStorage, - persistentRuleBasedSegmentsStorage: PersistentRuleBasedSegmentsStorageStub()) + persistentRuleBasedSegmentsStorage: PersistentRuleBasedSegmentsStorageStub(), + targetingRulesPersistenceBreaker: DefaultPersistenceBreaker()) splitConfig = SplitClientConfig() splitConfig.syncEnabled = syncEnabled diff --git a/SplitTests/Streaming/ImpressionsTrackerTest.swift b/SplitTests/Streaming/ImpressionsTrackerTest.swift index f4010affc..a9e543387 100644 --- a/SplitTests/Streaming/ImpressionsTrackerTest.swift +++ b/SplitTests/Streaming/ImpressionsTrackerTest.swift @@ -361,7 +361,8 @@ class ImpressionsTrackerTest: XCTestCase { hashedImpressionsStorage: HashedImpressionsStorageMock(), generalInfoStorage: GeneralInfoStorageMock(), ruleBasedSegmentsStorage: RuleBasedSegmentsStorageStub(), - persistentRuleBasedSegmentsStorage: PersistentRuleBasedSegmentsStorageStub()) + persistentRuleBasedSegmentsStorage: PersistentRuleBasedSegmentsStorageStub(), + targetingRulesPersistenceBreaker: DefaultPersistenceBreaker()) let apiFacade = try! SplitApiFacade.builder() .setUserKey("userKey") diff --git a/SplitTests/Streaming/SynchronizerTest.swift b/SplitTests/Streaming/SynchronizerTest.swift index 63f65ca3b..2e9a8d326 100644 --- a/SplitTests/Streaming/SynchronizerTest.swift +++ b/SplitTests/Streaming/SynchronizerTest.swift @@ -69,7 +69,8 @@ class SynchronizerTest: XCTestCase { hashedImpressionsStorage: HashedImpressionsStorageMock(), generalInfoStorage: GeneralInfoStorageMock(), ruleBasedSegmentsStorage: RuleBasedSegmentsStorageStub(), - persistentRuleBasedSegmentsStorage: PersistentRuleBasedSegmentsStorageStub()) + persistentRuleBasedSegmentsStorage: PersistentRuleBasedSegmentsStorageStub(), + targetingRulesPersistenceBreaker: DefaultPersistenceBreaker()) splitConfig = SplitClientConfig() splitConfig.syncEnabled = syncEnabled diff --git a/SplitTests/TreatmentManagerTest.swift b/SplitTests/TreatmentManagerTest.swift index efbf4200b..2168ef702 100644 --- a/SplitTests/TreatmentManagerTest.swift +++ b/SplitTests/TreatmentManagerTest.swift @@ -73,7 +73,8 @@ class TreatmentManagerTest: XCTestCase { hashedImpressionsStorage: HashedImpressionsStorageMock(), generalInfoStorage: GeneralInfoStorageMock(), ruleBasedSegmentsStorage: RuleBasedSegmentsStorageStub(), - persistentRuleBasedSegmentsStorage: PersistentRuleBasedSegmentsStorageStub()) + persistentRuleBasedSegmentsStorage: PersistentRuleBasedSegmentsStorageStub(), + targetingRulesPersistenceBreaker: DefaultPersistenceBreaker()) } }