From 5503c9db334e0e092ca44011a63da19536c3dffa Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 18 Dec 2025 11:39:47 -0300 Subject: [PATCH 1/9] Run sonar on merge --- .github/workflows/sonar.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml index 716383ba..6bab5832 100644 --- a/.github/workflows/sonar.yml +++ b/.github/workflows/sonar.yml @@ -5,6 +5,10 @@ on: branches: - "**" + merge: + branches: + - "**" + # Cancel in-progress runs when a new workflow with the same group is triggered concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} @@ -13,7 +17,7 @@ concurrency: jobs: build-and-collect-coverage: name: Build & Collect Coverage - runs-on: macos-latest + runs-on: macos-15-xlarge timeout-minutes: 15 steps: - name: Checkout @@ -28,7 +32,7 @@ jobs: -project Split.xcodeproj \ -scheme Split \ -testPlan "SplitiOSFull" \ - -destination 'platform=iOS Simulator,OS=18.4,name=iPhone 16 Pro Max' \ + -destination 'platform=iOS Simulator,OS=18.5,name=iPhone 16' \ -enableCodeCoverage YES \ -resultBundlePath build/Logs/Test/TestResults.xcresult \ -derivedDataPath build From 8d384c34800da9a29441e12a6c90ef05e22b2aab Mon Sep 17 00:00:00 2001 From: gthea Date: Thu, 18 Dec 2025 12:02:07 -0300 Subject: [PATCH 2/9] Persistence breaker component (#791) --- Split.xcodeproj/project.pbxproj | 17 +++- Split/Storage/PersistenceBreaker.swift | 54 +++++++++++ .../Storage/PersistenceBreakerTests.swift | 93 +++++++++++++++++++ 3 files changed, 161 insertions(+), 3 deletions(-) create mode 100644 Split/Storage/PersistenceBreaker.swift create mode 100644 SplitTests/Storage/PersistenceBreakerTests.swift diff --git a/Split.xcodeproj/project.pbxproj b/Split.xcodeproj/project.pbxproj index 0fd0b404..efe4edcd 100644 --- a/Split.xcodeproj/project.pbxproj +++ b/Split.xcodeproj/project.pbxproj @@ -539,7 +539,6 @@ 95649FBA2603F89E006D5E0C /* SplitsBgSyncWorkerTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 95649FB92603F89E006D5E0C /* SplitsBgSyncWorkerTest.swift */; }; 95649FBE260405C7006D5E0C /* MySegmentsBgSyncWorkerTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 95649FBD260405C7006D5E0C /* MySegmentsBgSyncWorkerTest.swift */; }; 9566744729F03662001B4FA5 /* DbCipher.swift in Sources */ = {isa = PBXBuildFile; fileRef = 95715A8229D23F9600A1B2F9 /* DbCipher.swift */; }; - CD311117BB394A70AD6057CD04272DF2 /* DbEncryptionManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5A0EA0213A424C098E507B2544F5CD6F /* DbEncryptionManager.swift */; }; 9566744829F03675001B4FA5 /* SplitEncryptionLevel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 95715A8029D22C1B00A1B2F9 /* SplitEncryptionLevel.swift */; }; 956816BE2836C91B00206FC5 /* UniqueKeyTracker.swift in Sources */ = {isa = PBXBuildFile; fileRef = 956816BD2836C91B00206FC5 /* UniqueKeyTracker.swift */; }; 9569E1CB2784C00B006FC7E5 /* TelemetryConfigHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9569E1CA2784C00B006FC7E5 /* TelemetryConfigHelper.swift */; }; @@ -1093,6 +1092,10 @@ C526DE4C2D9B09EB0051DAB8 /* ImpressionsPropertiesE2ETest.swift in Sources */ = {isa = PBXBuildFile; fileRef = C526DE4B2D9B09EB0051DAB8 /* ImpressionsPropertiesE2ETest.swift */; }; C52C57292EEB41350064D049 /* EncryptionKeyValidationIntegrationTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = C52C57282EEB41350064D049 /* EncryptionKeyValidationIntegrationTest.swift */; }; C52C572B2EEB41450064D049 /* EncryptionKeyValidationTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = C52C572A2EEB41450064D049 /* EncryptionKeyValidationTest.swift */; }; + C53207E72EF44A0700418BB1 /* DbEncryptionManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5A0EA0213A424C098E507B2544F5CD6F /* DbEncryptionManager.swift */; }; + C53207E92EF44A2100418BB1 /* PersistenceBreakerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C53207E82EF44A2100418BB1 /* PersistenceBreakerTests.swift */; }; + C53207EB2EF44A2F00418BB1 /* PersistenceBreaker.swift in Sources */ = {isa = PBXBuildFile; fileRef = C53207EA2EF44A2F00418BB1 /* PersistenceBreaker.swift */; }; + C53207EC2EF44A2F00418BB1 /* PersistenceBreaker.swift in Sources */ = {isa = PBXBuildFile; fileRef = C53207EA2EF44A2F00418BB1 /* PersistenceBreaker.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 */; }; @@ -1196,6 +1199,7 @@ C5E9675F2D3849BE00112DAC /* RolloutDefinitionsCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = C5E9675E2D3849BE00112DAC /* RolloutDefinitionsCache.swift */; }; C5E967602D3849BE00112DAC /* RolloutDefinitionsCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = C5E9675E2D3849BE00112DAC /* RolloutDefinitionsCache.swift */; }; C5E967622D395DAA00112DAC /* RolloutCacheManagerTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = C5E967612D395DAA00112DAC /* RolloutCacheManagerTest.swift */; }; + CD311117BB394A70AD6057CD04272DF2 /* DbEncryptionManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5A0EA0213A424C098E507B2544F5CD6F /* DbEncryptionManager.swift */; }; /* End PBXBuildFile section */ /* Begin PBXContainerItemProxy section */ @@ -1572,6 +1576,7 @@ 59FB7C34220329B900ECC96A /* SplitFactoryBuilderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SplitFactoryBuilderTests.swift; sourceTree = ""; }; 59FB7C3B2203795F00ECC96A /* LocalhostSplitsParser.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocalhostSplitsParser.swift; sourceTree = ""; }; 59FB7C3D22037B9400ECC96A /* SpaceDelimitedLocalhostSplitsParser.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SpaceDelimitedLocalhostSplitsParser.swift; sourceTree = ""; }; + 5A0EA0213A424C098E507B2544F5CD6F /* DbEncryptionManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DbEncryptionManager.swift; sourceTree = ""; }; 5B279CF82E340FB900B73A36 /* splitschanges_no_segments.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = splitschanges_no_segments.json; sourceTree = ""; }; 5B343EAC2E26E937006BEBE7 /* StorageHelper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StorageHelper.swift; sourceTree = ""; }; 5B91B8382DDE4A30000510F0 /* SplitDTOTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SplitDTOTests.swift; sourceTree = ""; }; @@ -1758,7 +1763,6 @@ 956D17D0260D266E0037F575 /* SplitsChangesCheckerTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SplitsChangesCheckerTest.swift; sourceTree = ""; }; 956D17D6260D28320037F575 /* MySegmentsChangeChecker.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MySegmentsChangeChecker.swift; sourceTree = ""; }; 95715A8029D22C1B00A1B2F9 /* SplitEncryptionLevel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SplitEncryptionLevel.swift; sourceTree = ""; }; - 5A0EA0213A424C098E507B2544F5CD6F /* DbEncryptionManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DbEncryptionManager.swift; sourceTree = ""; }; 95715A8229D23F9600A1B2F9 /* DbCipher.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DbCipher.swift; sourceTree = ""; }; 95715A8429D353C100A1B2F9 /* DbCipherTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DbCipherTest.swift; sourceTree = ""; }; 95715A9429DB0ED800A1B2F9 /* InitDbCipherTest.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = InitDbCipherTest.swift; sourceTree = ""; }; @@ -2000,6 +2004,8 @@ C526DE4B2D9B09EB0051DAB8 /* ImpressionsPropertiesE2ETest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImpressionsPropertiesE2ETest.swift; sourceTree = ""; }; C52C57282EEB41350064D049 /* EncryptionKeyValidationIntegrationTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EncryptionKeyValidationIntegrationTest.swift; sourceTree = ""; }; C52C572A2EEB41450064D049 /* EncryptionKeyValidationTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EncryptionKeyValidationTest.swift; sourceTree = ""; }; + C53207E82EF44A2100418BB1 /* PersistenceBreakerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PersistenceBreakerTests.swift; sourceTree = ""; }; + C53207EA2EF44A2F00418BB1 /* PersistenceBreaker.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PersistenceBreaker.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 = ""; }; @@ -2637,6 +2643,7 @@ 3B6DEF0720EA6AE40067435E /* Storage */ = { isa = PBXGroup; children = ( + C53207EA2EF44A2F00418BB1 /* PersistenceBreaker.swift */, C5A501C72D88A15600206F82 /* RuleBasedSegments */, 955E122F2BFBC90700AE6D10 /* HashedImpression */, 952E266E2833E4210015D633 /* UniqueKeys */, @@ -2676,6 +2683,7 @@ 5905D4E3255B2373006DA3B1 /* Storage */ = { isa = PBXGroup; children = ( + C53207E82EF44A2100418BB1 /* PersistenceBreakerTests.swift */, C52C572A2EEB41450064D049 /* EncryptionKeyValidationTest.swift */, 599407DF22403BE9003B85CC /* SplitsStorageTrafficTypesTests.swift */, 59F65F002562FF71005FE8C9 /* EventDaoTest.swift */, @@ -4416,6 +4424,7 @@ 5905D4D82555FE74006DA3B1 /* SplitDatabase.swift in Sources */, 952FA12B2A2E593900264AB5 /* SplitComponentCatalog.swift in Sources */, 5BD767402E64B6F800F9EFBD /* FallbackTreatmentsCalculator.swift in Sources */, + C53207EB2EF44A2F00418BB1 /* PersistenceBreaker.swift in Sources */, 59B2042724F4171E0092F2E9 /* SyncUpdateWorker.swift in Sources */, 955428D6256810D300331356 /* ImpressionDao.swift in Sources */, 950B7417268F4716005FD649 /* ImpressionsMode.swift in Sources */, @@ -4656,6 +4665,7 @@ 9502B7B42837E70200B779C0 /* UniqueTrackerTest.swift in Sources */, 952BF5F8278DF57D00FF6181 /* TelemetryIntegrationTest.swift in Sources */, 59F4AAA124FFC94100A1C69A /* NotificationManagerKeeperTest.swift in Sources */, + C53207E92EF44A2100418BB1 /* PersistenceBreakerTests.swift in Sources */, 955E12372BFCDEAC00AE6D10 /* HashedImpressionDaoMock.swift in Sources */, 95ABF4FC29369B73006ED016 /* TelemetrySynchronizerStub.swift in Sources */, 5BF52DF92DE4B8D400FEDAFE /* PrerequisitesMatcherTest.swift in Sources */, @@ -5181,7 +5191,8 @@ 95B02D9B28D0BDC30030EC8B /* DataResult.swift in Sources */, 958AD2132CA457C100E3DD43 /* RetryableSegmentsSyncWorker.swift in Sources */, 9566744729F03662001B4FA5 /* DbCipher.swift in Sources */, - CD311117BB394A70AD6057CD04272DF2 /* DbEncryptionManager.swift in Sources */, + C53207E72EF44A0700418BB1 /* DbEncryptionManager.swift in Sources */, + C53207EC2EF44A2F00418BB1 /* PersistenceBreaker.swift in Sources */, 95B02D9C28D0BDC30030EC8B /* ServiceEndpoints.swift in Sources */, 95B02D9D28D0BDC30030EC8B /* Endpoint.swift in Sources */, 95B02D9E28D0BDC30030EC8B /* EndpointFactory.swift in Sources */, diff --git a/Split/Storage/PersistenceBreaker.swift b/Split/Storage/PersistenceBreaker.swift new file mode 100644 index 00000000..dd58042b --- /dev/null +++ b/Split/Storage/PersistenceBreaker.swift @@ -0,0 +1,54 @@ +// +// PersistenceBreaker.swift +// Split +// +// + +import Foundation + +/// Protocol for controlling persistence behavior in response to failures. +/// +/// PersistenceBreaker centralizes "disable persistence for session" behavior. +/// After a persistence failure (e.g., CoreData save() error), storages can use +/// this to check if persistence is still allowed and to disable it on first failure. +/// +protocol PersistenceBreaker { + /// Returns true if persistence is currently enabled, false if disabled for session. + var isPersistenceEnabled: Bool { get } + + /// Disables persistence for the remainder of the session. + /// This operation is idempotent - calling multiple times has no additional effect. + func disable() +} + +/// Default implementation of PersistenceBreaker. +/// +/// Usage: +/// let breaker = DefaultPersistenceBreaker() +/// if breaker.isPersistenceEnabled { +/// // attempt persistence... +/// // on failure: breaker.disable() +/// } +class DefaultPersistenceBreaker: PersistenceBreaker { + + private let lock = NSLock() + private var _enabled = true + + var isPersistenceEnabled: Bool { + lock.lock() + defer { lock.unlock() } + return _enabled + } + + func disable() { + lock.lock() + defer { lock.unlock() } + + // Idempotent: only disable once + if _enabled { + _enabled = false + Logger.d("Targeting rules persistence disabled for session") + } + } +} + diff --git a/SplitTests/Storage/PersistenceBreakerTests.swift b/SplitTests/Storage/PersistenceBreakerTests.swift new file mode 100644 index 00000000..a8ab81f1 --- /dev/null +++ b/SplitTests/Storage/PersistenceBreakerTests.swift @@ -0,0 +1,93 @@ +// +// PersistenceBreakerTests.swift +// SplitTests +// +// + +import Foundation +import XCTest +@testable import Split + +class PersistenceBreakerTests: XCTestCase { + + var breaker: PersistenceBreaker! + + override func setUp() { + super.setUp() + } + + func testInitiallyPersistenceIsEnabled() { + breaker = DefaultPersistenceBreaker() + + XCTAssertTrue(breaker.isPersistenceEnabled, + "Persistence should be enabled initially") + } + + func testDisablingPersistenceWorks() { + breaker = DefaultPersistenceBreaker() + + breaker.disable() + + XCTAssertFalse(breaker.isPersistenceEnabled, + "Persistence should be disabled after calling disable()") + } + + func testDisableIsIdempotent() { + breaker = DefaultPersistenceBreaker() + + breaker.disable() + breaker.disable() + breaker.disable() + + XCTAssertFalse(breaker.isPersistenceEnabled, + "Multiple disable() calls should be idempotent") + } + + func testThreadSafetyOfDisable() { + breaker = DefaultPersistenceBreaker() + let expectation = self.expectation(description: "Concurrent disable calls complete") + expectation.expectedFulfillmentCount = 10 + + for _ in 0..<10 { + DispatchQueue.global().async { + self.breaker.disable() + expectation.fulfill() + } + } + + waitForExpectations(timeout: 2.0) + XCTAssertFalse(breaker.isPersistenceEnabled, + "Concurrent disable() calls should be thread-safe") + } + + func testThreadSafetyOfReads() { + breaker = DefaultPersistenceBreaker() + let expectation = self.expectation(description: "Concurrent operations complete") + expectation.expectedFulfillmentCount = 100 + + var readResults = [Bool]() + let resultsQueue = DispatchQueue(label: "resultsQueue") + + // Many threads read while one disables + for i in 0..<100 { + DispatchQueue.global().async { + if i == 50 { + // Disable in the middle + self.breaker.disable() + } + let result = self.breaker.isPersistenceEnabled + resultsQueue.sync { + readResults.append(result) + } + expectation.fulfill() + } + } + + waitForExpectations(timeout: 5.0) + XCTAssertEqual(100, readResults.count, + "All reads should complete without crashes") + XCTAssertFalse(breaker.isPersistenceEnabled, + "Final state should be disabled") + } +} + From 026b67dcfa903895ffa99067ff4508b8d9e72dbe Mon Sep 17 00:00:00 2001 From: gthea Date: Thu, 18 Dec 2025 14:57:42 -0300 Subject: [PATCH 3/9] DB transaction functionality (#792) * New methods for transactions * CoreDataHelper test * Add missing tests --- .github/workflows/sonar.yml | 5 +- Split.xcodeproj/project.pbxproj | 8 ++ Split/Storage/CoreDataHelper.swift | 18 ++++ .../Storage/GeneralInfo/GeneralInfoDao.swift | 15 ++++ Split/Storage/Splits/SplitDao.swift | 35 ++++++++ .../Fake/Storage/CoreDataHelperStub.swift | 43 ++++++++++ .../Fake/Storage/GeneralInfoDaoStub.swift | 4 + SplitTests/Fake/Storage/SplitDaoStub.swift | 8 ++ .../Fake/Storage/SplitDatabaseStub.swift | 8 +- SplitTests/Storage/CoreDataHelperTests.swift | 84 +++++++++++++++++++ SplitTests/Storage/GeneralInfoDaoTests.swift | 41 +++++++++ SplitTests/Storage/SplitDaoTest.swift | 79 +++++++++++++++++ 12 files changed, 343 insertions(+), 5 deletions(-) create mode 100644 SplitTests/Fake/Storage/CoreDataHelperStub.swift create mode 100644 SplitTests/Storage/CoreDataHelperTests.swift diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml index 6bab5832..14c7321e 100644 --- a/.github/workflows/sonar.yml +++ b/.github/workflows/sonar.yml @@ -4,10 +4,9 @@ on: pull_request: branches: - "**" - - merge: + push: branches: - - "**" + - master # Cancel in-progress runs when a new workflow with the same group is triggered concurrency: diff --git a/Split.xcodeproj/project.pbxproj b/Split.xcodeproj/project.pbxproj index efe4edcd..1adce743 100644 --- a/Split.xcodeproj/project.pbxproj +++ b/Split.xcodeproj/project.pbxproj @@ -1096,6 +1096,8 @@ C53207E92EF44A2100418BB1 /* PersistenceBreakerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C53207E82EF44A2100418BB1 /* PersistenceBreakerTests.swift */; }; C53207EB2EF44A2F00418BB1 /* PersistenceBreaker.swift in Sources */ = {isa = PBXBuildFile; fileRef = C53207EA2EF44A2F00418BB1 /* PersistenceBreaker.swift */; }; 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 */; }; 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 */; }; @@ -2006,6 +2008,8 @@ C52C572A2EEB41450064D049 /* EncryptionKeyValidationTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EncryptionKeyValidationTest.swift; sourceTree = ""; }; C53207E82EF44A2100418BB1 /* PersistenceBreakerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PersistenceBreakerTests.swift; sourceTree = ""; }; 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 = ""; }; 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 = ""; }; @@ -2683,6 +2687,7 @@ 5905D4E3255B2373006DA3B1 /* Storage */ = { isa = PBXGroup; children = ( + C53207F02EF456AF00418BB1 /* CoreDataHelperTests.swift */, C53207E82EF44A2100418BB1 /* PersistenceBreakerTests.swift */, C52C572A2EEB41450064D049 /* EncryptionKeyValidationTest.swift */, 599407DF22403BE9003B85CC /* SplitsStorageTrafficTypesTests.swift */, @@ -2727,6 +2732,7 @@ 5905D4E6255B23C8006DA3B1 /* Storage */ = { isa = PBXGroup; children = ( + C53207ED2EF452C000418BB1 /* CoreDataHelperStub.swift */, C539CAC22D88C7570050C732 /* PersistentRuleBasedSegmentsStorageStub.swift */, C539CAC32D88C7570050C732 /* RuleBasedSegmentsStorageStub.swift */, C5A501D82D88A7E900206F82 /* RuleBasedSegmentDaoStub.swift */, @@ -4631,6 +4637,7 @@ 952E26752833FF3F0015D633 /* UniqueKeyDaoStub.swift in Sources */, 59ED408F24F06EC100EF7B09 /* TimersManagerTest.swift in Sources */, 59D84BE7221AE775003DA248 /* LocalhostManagerTests.swift in Sources */, + C53207F12EF456AF00418BB1 /* CoreDataHelperTests.swift in Sources */, C539CAE22D9477770050C732 /* PropertyValidatorStub.swift in Sources */, 955B596C2816BC0C00D105CD /* MultiClientEvaluationTest.swift in Sources */, 59FB7C35220329B900ECC96A /* SplitFactoryBuilderTests.swift in Sources */, @@ -4819,6 +4826,7 @@ 5982D938219F57BE00230F44 /* FileHelper.swift in Sources */, 95B180272763DA0E002DC9DF /* HttpTelemetryConfigRecorderTest.swift in Sources */, C5977C202BF29F5B003E293A /* EqualToSemverMatcherTest.swift in Sources */, + C53207EE2EF452C000418BB1 /* CoreDataHelperStub.swift in Sources */, C5A7D5532DD672CF0081D190 /* RuleBasedSegmentsIntegrationTest.swift in Sources */, 59ED408424EAB8C900EF7B09 /* PushNotificationManagerTest.swift in Sources */, 95F7BC292C46A4C800C5F2E4 /* SecurityHelper.swift in Sources */, diff --git a/Split/Storage/CoreDataHelper.swift b/Split/Storage/CoreDataHelper.swift index d996f87e..713b8e9f 100644 --- a/Split/Storage/CoreDataHelper.swift +++ b/Split/Storage/CoreDataHelper.swift @@ -64,6 +64,24 @@ class CoreDataHelper { } } + /// Save with error handling. Throws errors to caller + /// Used for transactional operations that need to handle persistence failures + func saveWithErrorHandling() throws { + var thrownError: Error? + managedObjectContext.performAndWait { + do { + if self.managedObjectContext.hasChanges { + try self.managedObjectContext.save() + } + } catch { + thrownError = error + } + } + if let error = thrownError { + throw error + } + } + func generateId() -> String { return UUID().uuidString } diff --git a/Split/Storage/GeneralInfo/GeneralInfoDao.swift b/Split/Storage/GeneralInfo/GeneralInfoDao.swift index 9753311b..097fcd49 100644 --- a/Split/Storage/GeneralInfo/GeneralInfoDao.swift +++ b/Split/Storage/GeneralInfo/GeneralInfoDao.swift @@ -28,6 +28,10 @@ protocol GeneralInfoDao { func stringValue(info: GeneralInfo) -> String? func longValue(info: GeneralInfo) -> Int64? func delete(info: GeneralInfo) + + /// Synchronous update for use in transactions. + /// Caller must call coreDataHelper.saveWithErrorHandling() after all ops + func transactionalUpdate(info: GeneralInfo, longValue: Int64) } class CoreDataGeneralInfoDao: BaseCoreDataDao, GeneralInfoDao { @@ -86,6 +90,17 @@ class CoreDataGeneralInfoDao: BaseCoreDataDao, GeneralInfoDao { } } + /// Synchronous update that does NOT save. Caller must save. For use in transactions + func transactionalUpdate(info: GeneralInfo, longValue: Int64) { + if let obj = get(for: info) ?? coreDataHelper.create(entity: .generalInfo) as? GeneralInfoEntity { + obj.name = info.rawValue + obj.stringValue = "" + obj.longValue = longValue + obj.updatedAt = Date().unixTimestamp() + // Not saving. Caller will save the entire transaction + } + } + private func update(info: GeneralInfo, stringValue: String?, longValue: Int64?) { if let obj = get(for: info) ?? coreDataHelper.create(entity: .generalInfo) as? GeneralInfoEntity { obj.name = info.rawValue diff --git a/Split/Storage/Splits/SplitDao.swift b/Split/Storage/Splits/SplitDao.swift index cae62e01..bb2403be 100644 --- a/Split/Storage/Splits/SplitDao.swift +++ b/Split/Storage/Splits/SplitDao.swift @@ -16,6 +16,14 @@ protocol SplitDao { func delete(_ splits: [String]) func deleteAll() func syncInsertOrUpdate(split: Split) + + /// Synchronous insert/update for use in transactions + /// Caller must call coreDataHelper.saveWithErrorHandling() + func transactionalInsertOrUpdate(splits: [Split]) throws + + /// Synchronous delete for use in transactions + /// Caller must call coreDataHelper.saveWithErrorHandling() + func transactionalDelete(_ splitNames: [String]) } class CoreDataSplitDao: BaseCoreDataDao, SplitDao { @@ -112,6 +120,33 @@ class CoreDataSplitDao: BaseCoreDataDao, SplitDao { } } + /// Synchronous insert/update that does NOT save + func transactionalInsertOrUpdate(splits: [Split]) throws { + 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 { + obj.name = name + obj.body = json + obj.updatedAt = Date.now() + // Do NOT save here. Caller should save the entire transaction + } + } + } + + /// Synchronous delete that does NOT save + func transactionalDelete(_ splitNames: [String]) { + if splitNames.count == 0 { + return + } + + var names = splitNames + if let cipher = self.cipher { + names = splitNames.map { cipher.encrypt($0) ?? $0 } + } + self.coreDataHelper.delete(entity: .split, by: "name", values: names) + // Do NOT save here. Caller should save the entire transaction + } + private func insertOrUpdate(_ split: Split) { if let splitName = cipher?.encrypt(split.name) ?? split.name, let obj = self.getBy(name: splitName) ?? self.coreDataHelper.create(entity: .split) as? SplitEntity { diff --git a/SplitTests/Fake/Storage/CoreDataHelperStub.swift b/SplitTests/Fake/Storage/CoreDataHelperStub.swift new file mode 100644 index 00000000..0dd736f5 --- /dev/null +++ b/SplitTests/Fake/Storage/CoreDataHelperStub.swift @@ -0,0 +1,43 @@ +// +// CoreDataHelperStub.swift +// SplitTests +// + +import Foundation +import CoreData +@testable import Split + +class CoreDataHelperStub: CoreDataHelper { + + var shouldFailOnSave = false + var saveError: Error = NSError(domain: "TestCoreData", code: 500, userInfo: [NSLocalizedDescriptionKey: "Simulated save failure"]) + + init() { + let model = NSManagedObjectModel() + let coordinator = NSPersistentStoreCoordinator(managedObjectModel: model) + let context = NSManagedObjectContext(concurrencyType: .privateQueueConcurrencyType) + context.persistentStoreCoordinator = coordinator + + super.init(managedObjectContext: context, persistentCoordinator: coordinator) + } + + override func performAndWait(_ operation: () -> Void) { + operation() + } + + override func perform(_ operation: @escaping () -> Void) { + operation() + } + + override func save() { + // No-op for stubs + } + + override func saveWithErrorHandling() throws { + if shouldFailOnSave { + throw saveError + } + // Success + } +} + diff --git a/SplitTests/Fake/Storage/GeneralInfoDaoStub.swift b/SplitTests/Fake/Storage/GeneralInfoDaoStub.swift index 90db7dcd..2ba3611a 100644 --- a/SplitTests/Fake/Storage/GeneralInfoDaoStub.swift +++ b/SplitTests/Fake/Storage/GeneralInfoDaoStub.swift @@ -34,4 +34,8 @@ class GeneralInfoDaoStub: GeneralInfoDao { updatedString.removeValue(forKey: info.rawValue) updatedLong.removeValue(forKey: info.rawValue) } + + func transactionalUpdate(info: GeneralInfo, longValue: Int64) { + updatedLong[info.rawValue] = longValue + } } diff --git a/SplitTests/Fake/Storage/SplitDaoStub.swift b/SplitTests/Fake/Storage/SplitDaoStub.swift index 7d6bd0aa..b0c32f13 100644 --- a/SplitTests/Fake/Storage/SplitDaoStub.swift +++ b/SplitTests/Fake/Storage/SplitDaoStub.swift @@ -38,4 +38,12 @@ class SplitDaoStub: SplitDao { func deleteAll() { deleteAllCalled = true } + + func transactionalInsertOrUpdate(splits: [Split]) throws { + insertedSplits = splits + } + + func transactionalDelete(_ splitNames: [String]) { + deletedSplits = splitNames + } } diff --git a/SplitTests/Fake/Storage/SplitDatabaseStub.swift b/SplitTests/Fake/Storage/SplitDatabaseStub.swift index 6f1eab59..193e47f6 100644 --- a/SplitTests/Fake/Storage/SplitDatabaseStub.swift +++ b/SplitTests/Fake/Storage/SplitDatabaseStub.swift @@ -37,7 +37,7 @@ struct CoreDataDaoProviderMock: DaoProvider { var ruleBasedSegmentDao: RuleBasedSegmentDao = RuleBasedSegmentDaoStub() } -class SplitDatabaseStub: SplitDatabase { +class SplitDatabaseStub: SplitDatabase, TestSplitDatabase { var splitDao: SplitDao var mySegmentsDao: MySegmentsDao @@ -51,7 +51,10 @@ class SplitDatabaseStub: SplitDatabase { var uniqueKeyDao: UniqueKeyDao var ruleBasedSegmentDao: RuleBasedSegmentDao - init(daoProvider: DaoProvider) { + // TestSplitDatabase conformance + var coreDataHelper: CoreDataHelper + + init(daoProvider: DaoProvider, coreDataHelper: CoreDataHelper? = nil) { self.eventDao = daoProvider.eventDao self.impressionDao = daoProvider.impressionDao self.impressionsCountDao = daoProvider.impressionsCountDao @@ -63,5 +66,6 @@ class SplitDatabaseStub: SplitDatabase { self.uniqueKeyDao = daoProvider.uniqueKeyDao self.hashedImpressionDao = daoProvider.hashedImpressionDao self.ruleBasedSegmentDao = daoProvider.ruleBasedSegmentDao + self.coreDataHelper = coreDataHelper ?? CoreDataHelperStub() } } diff --git a/SplitTests/Storage/CoreDataHelperTests.swift b/SplitTests/Storage/CoreDataHelperTests.swift new file mode 100644 index 00000000..c2c6bae8 --- /dev/null +++ b/SplitTests/Storage/CoreDataHelperTests.swift @@ -0,0 +1,84 @@ +// +// CoreDataHelperTests.swift +// SplitTests +// + +import Foundation +import XCTest +import CoreData +@testable import Split + +class CoreDataHelperTests: XCTestCase { + + var coreDataHelper: CoreDataHelper! + + override func setUp() { + let queue = DispatchQueue(label: "coredata helper test") + coreDataHelper = IntegrationCoreDataHelper.get(databaseName: "test", dispatchQueue: queue) + } + + func testSaveWithErrorHandlingSucceedsWhenChangesExist() { + coreDataHelper.performAndWait { + if let entity = self.coreDataHelper.create(entity: .generalInfo) as? GeneralInfoEntity { + entity.name = "test_info" + entity.stringValue = "test_value" + } + } + + XCTAssertNoThrow(try coreDataHelper.saveWithErrorHandling()) + } + + func testSaveWithErrorHandlingThrowsOnValidationError() { + coreDataHelper.performAndWait { + // Create entity without required 'name' field to trigger validation error + _ = self.coreDataHelper.create(entity: .generalInfo) + } + + XCTAssertThrowsError(try coreDataHelper.saveWithErrorHandling()) { error in + let nsError = error as NSError + XCTAssertEqual(NSValidationMissingMandatoryPropertyError, nsError.code) + } + } + + func testSaveWithErrorHandlingSucceedsWhenNoChanges() { + XCTAssertNoThrow(try coreDataHelper.saveWithErrorHandling()) + } + + func testSaveWithErrorHandlingPersistsData() { + coreDataHelper.performAndWait { + if let entity = self.coreDataHelper.create(entity: .generalInfo) as? GeneralInfoEntity { + entity.name = GeneralInfo.splitsChangeNumber.rawValue + entity.longValue = 12345 + } + } + + try? coreDataHelper.saveWithErrorHandling() + + let fetched = coreDataHelper.fetch(entity: .generalInfo) + XCTAssertEqual(1, fetched.count) + + if let entity = fetched.first as? GeneralInfoEntity { + XCTAssertEqual(12345, entity.longValue) + } else { + XCTFail("Expected GeneralInfoEntity") + } + } + + func testSaveWithErrorHandlingThrowsOnInvalidContext() { + let invalidContext = NSManagedObjectContext(concurrencyType: .privateQueueConcurrencyType) + let invalidHelper = CoreDataHelper( + managedObjectContext: invalidContext, + persistentCoordinator: NSPersistentStoreCoordinator(managedObjectModel: NSManagedObjectModel()) + ) + + invalidContext.performAndWait { + let entity = NSEntityDescription() + entity.name = "TestEntity" + entity.managedObjectClassName = NSStringFromClass(NSManagedObject.self) + } + + // Context without persistent store should not throw when there are no changes + XCTAssertNoThrow(try invalidHelper.saveWithErrorHandling()) + } +} + diff --git a/SplitTests/Storage/GeneralInfoDaoTests.swift b/SplitTests/Storage/GeneralInfoDaoTests.swift index 017c8dab..a22f8662 100644 --- a/SplitTests/Storage/GeneralInfoDaoTests.swift +++ b/SplitTests/Storage/GeneralInfoDaoTests.swift @@ -63,6 +63,47 @@ class GeneralInfoDaoTest: XCTestCase { XCTAssertEqual(data, segmentsInUse) } + + func testTransactionalUpdateDoesNotSaveUntilCallerSaves() { + guard let coreDataDao = generalInfoDao as? CoreDataGeneralInfoDao else { + XCTFail("Expected CoreDataGeneralInfoDao") + return + } + + coreDataDao.coreDataHelper.performAndWait { + coreDataDao.transactionalUpdate(info: .splitsChangeNumber, longValue: 999) + } + + // Value should be in context but we need to save to persist + coreDataDao.coreDataHelper.save() + + let savedValue = generalInfoDao.longValue(info: .splitsChangeNumber) + XCTAssertEqual(999, savedValue) + } + + func testTransactionalUpdateUpdatesExistingValue() { + guard let coreDataDao = generalInfoDao as? CoreDataGeneralInfoDao else { + XCTFail("Expected CoreDataGeneralInfoDao") + return + } + + // Create initial value + generalInfoDao.update(info: .splitsUpdateTimestamp, longValue: 100) + + // Wait for async save to complete + let exp = expectation(description: "wait for save") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { exp.fulfill() } + wait(for: [exp], timeout: 1.0) + + // Transactionally update + coreDataDao.coreDataHelper.performAndWait { + coreDataDao.transactionalUpdate(info: .splitsUpdateTimestamp, longValue: 200) + } + coreDataDao.coreDataHelper.save() + + let updatedValue = generalInfoDao.longValue(info: .splitsUpdateTimestamp) + XCTAssertEqual(200, updatedValue) + } override func tearDown() { } diff --git a/SplitTests/Storage/SplitDaoTest.swift b/SplitTests/Storage/SplitDaoTest.swift index 3ca84823..9926c8e0 100644 --- a/SplitTests/Storage/SplitDaoTest.swift +++ b/SplitTests/Storage/SplitDaoTest.swift @@ -154,6 +154,85 @@ class SplitDaoTest: XCTestCase { return (name: name, body: body) } + func testTransactionalInsertOrUpdateDoesNotSaveUntilCallerSaves() { + guard let coreDataDao = splitDao as? CoreDataSplitDao else { + XCTFail("Expected CoreDataSplitDao") + return + } + + let newSplits = [newSplit(name: "transactional_split_1", trafficType: "user")] + + coreDataDao.coreDataHelper.performAndWait { + try? coreDataDao.transactionalInsertOrUpdate(splits: newSplits) + } + + // Save manually + coreDataDao.coreDataHelper.save() + + let allSplits = splitDao.getAll() + let found = allSplits.first { $0.name == "transactional_split_1" } + + XCTAssertNotNil(found) + XCTAssertEqual("user", found?.trafficTypeName) + } + + func testTransactionalInsertOrUpdateUpdatesExisting() { + guard let coreDataDao = splitDao as? CoreDataSplitDao else { + XCTFail("Expected CoreDataSplitDao") + return + } + + // feat_0 was created in setUp + let updatedSplit = newSplit(name: "feat_0", trafficType: "updated_type") + + coreDataDao.coreDataHelper.performAndWait { + try? coreDataDao.transactionalInsertOrUpdate(splits: [updatedSplit]) + } + coreDataDao.coreDataHelper.save() + + let allSplits = splitDao.getAll() + let found = allSplits.first { $0.name == "feat_0" } + + XCTAssertNotNil(found) + XCTAssertEqual("updated_type", found?.trafficTypeName) + } + + func testTransactionalDeleteDoesNotSaveUntilCallerSaves() { + guard let coreDataDao = splitDao as? CoreDataSplitDao else { + XCTFail("Expected CoreDataSplitDao") + return + } + + let beforeCount = splitDao.getAll().count + + coreDataDao.coreDataHelper.performAndWait { + coreDataDao.transactionalDelete(["feat_0", "feat_1"]) + } + coreDataDao.coreDataHelper.save() + + let afterCount = splitDao.getAll().count + + XCTAssertEqual(beforeCount - 2, afterCount) + } + + func testTransactionalDeleteWithEmptyArrayDoesNothing() { + guard let coreDataDao = splitDao as? CoreDataSplitDao else { + XCTFail("Expected CoreDataSplitDao") + return + } + + let beforeCount = splitDao.getAll().count + + coreDataDao.coreDataHelper.performAndWait { + coreDataDao.transactionalDelete([]) + } + coreDataDao.coreDataHelper.save() + + let afterCount = splitDao.getAll().count + + XCTAssertEqual(beforeCount, afterCount) + } + private func createSplits() -> [Split] { return SplitTestHelper.createSplits(namePrefix: "feat_", count: 10) } From 7cba4b5439788b827ce6ac706e290043b39c3d62 Mon Sep 17 00:00:00 2001 From: gthea Date: Thu, 18 Dec 2025 15:23:58 -0300 Subject: [PATCH 4/9] Integrate transaction & persistence breaker in SDK (#793) --- Split.xcodeproj/project.pbxproj | 8 + Split/Api/SplitDatabaseHelper.swift | 17 +- Split/Network/Sync/SyncCommons.swift | 1 + .../Splits/PersistentSplitsStorage.swift | 34 +++- Split/Storage/Splits/SplitDao.swift | 4 +- Split/Storage/Splits/SplitsStorage.swift | 33 +++- .../Storage/PersistentSplitsStorageStub.swift | 2 +- SplitTests/Fake/Storage/SplitDaoStub.swift | 2 +- SplitTests/Helpers/TestingHelper.swift | 3 +- .../PersistentSplitsStorageTests.swift | 2 +- ...stentSplitsStorageTransactionalTests.swift | 106 +++++++++++ SplitTests/Storage/SplitDaoTest.swift | 4 +- ...itsPersistenceBreakerIntegrationTest.swift | 164 ++++++++++++++++++ SplitTests/Storage/SplitsStorageTests.swift | 8 +- .../SplitsStorageTrafficTypesTests.swift | 3 +- .../FeatureFlagsSynchronizerTest.swift | 3 +- .../Streaming/ImpressionsTrackerTest.swift | 3 +- SplitTests/Streaming/SynchronizerTest.swift | 3 +- SplitTests/TreatmentManagerTest.swift | 3 +- 19 files changed, 368 insertions(+), 35 deletions(-) create mode 100644 SplitTests/Storage/PersistentSplitsStorageTransactionalTests.swift create mode 100644 SplitTests/Storage/SplitsPersistenceBreakerIntegrationTest.swift diff --git a/Split.xcodeproj/project.pbxproj b/Split.xcodeproj/project.pbxproj index 1adce743..6aab8e1a 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 48f4b5dd..ff7eb745 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 fb3313ae..e8df6c3a 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 b082b582..88a122f1 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 bb2403be..137f9b2a 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 732edbec..02a147f7 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 a26e1bf6..5c4334a7 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 b0c32f13..0078f3fb 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 c908d1cd..e377ccfc 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 48c17dbd..cc5eccb9 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 00000000..76856bb7 --- /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 9926c8e0..9ae5a324 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 00000000..c509f9f5 --- /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 508e3139..335a970d 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 39228a11..bc316e24 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 ab794172..e06d5b62 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 f4010aff..a9e54338 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 63f65ca3..2e9a8d32 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 efbf4200..2168ef70 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()) } } From 3438c1f5251c28c4f400554c4147738004e18852 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 18 Dec 2025 15:28:53 -0300 Subject: [PATCH 5/9] chore: Update version to 3.5.2-rc1 --- Split.podspec | 2 +- Split/Common/Utils/Version.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Split.podspec b/Split.podspec index 0fd2ac69..698b5d22 100644 --- a/Split.podspec +++ b/Split.podspec @@ -1,7 +1,7 @@ Pod::Spec.new do |s| s.name = 'Split' s.module_name = 'Split' - s.version = '3.5.1' + s.version = '3.5.2-rc1' s.summary = 'iOS SDK for Split' s.description = <<-DESC This SDK is designed to work with Split, the platform for controlled rollouts, serving features to your users via the Split feature flag to manage your complete customer experience. diff --git a/Split/Common/Utils/Version.swift b/Split/Common/Utils/Version.swift index 6008118e..a7dba831 100644 --- a/Split/Common/Utils/Version.swift +++ b/Split/Common/Utils/Version.swift @@ -10,7 +10,7 @@ import Foundation class Version { private static let kSdkPlatform: String = "ios" - private static let kVersion = "3.5.1" + private static let kVersion = "3.5.2-rc1" static var semantic: String { return kVersion From 4f14503400242d805e34e639add181429c465337 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 18 Dec 2025 19:33:56 -0300 Subject: [PATCH 6/9] chore: Update version to 3.5.2-rc2 --- Split.podspec | 2 +- Split/Common/Utils/Version.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Split.podspec b/Split.podspec index 698b5d22..4b7d37b5 100644 --- a/Split.podspec +++ b/Split.podspec @@ -1,7 +1,7 @@ Pod::Spec.new do |s| s.name = 'Split' s.module_name = 'Split' - s.version = '3.5.2-rc1' + s.version = '3.5.2-rc2' s.summary = 'iOS SDK for Split' s.description = <<-DESC This SDK is designed to work with Split, the platform for controlled rollouts, serving features to your users via the Split feature flag to manage your complete customer experience. diff --git a/Split/Common/Utils/Version.swift b/Split/Common/Utils/Version.swift index a7dba831..eb82f2f8 100644 --- a/Split/Common/Utils/Version.swift +++ b/Split/Common/Utils/Version.swift @@ -10,7 +10,7 @@ import Foundation class Version { private static let kSdkPlatform: String = "ios" - private static let kVersion = "3.5.2-rc1" + private static let kVersion = "3.5.2-rc2" static var semantic: String { return kVersion From 05e033ecd1904f7d8ff66deb1e0e3da33865a918 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 18 Dec 2025 19:34:39 -0300 Subject: [PATCH 7/9] Use async write in flags transaction --- Split/Storage/CoreDataHelper.swift | 8 +++++++ .../Splits/PersistentSplitsStorage.swift | 7 +++++-- .../Fake/Storage/CoreDataHelperStub.swift | 5 +++++ SplitTests/Storage/CoreDataHelperTests.swift | 21 +++++++++++++++++++ ...stentSplitsStorageTransactionalTests.swift | 15 +++++++++++++ 5 files changed, 54 insertions(+), 2 deletions(-) diff --git a/Split/Storage/CoreDataHelper.swift b/Split/Storage/CoreDataHelper.swift index 713b8e9f..f8f7aa54 100644 --- a/Split/Storage/CoreDataHelper.swift +++ b/Split/Storage/CoreDataHelper.swift @@ -122,6 +122,14 @@ class CoreDataHelper { } } + /// Roll back any unsaved changes in the managed object context. + /// Useful after a failed save(), to prevent the context from keeping invalid pending changes. + func rollback() { + managedObjectContext.performAndWait { + self.managedObjectContext.rollback() + } + } + private func delete(entity: CoreDataEntity, predicate: NSPredicate? = nil) { managedObjectContext.performAndWait { diff --git a/Split/Storage/Splits/PersistentSplitsStorage.swift b/Split/Storage/Splits/PersistentSplitsStorage.swift index 88a122f1..c0ce94d9 100644 --- a/Split/Storage/Splits/PersistentSplitsStorage.swift +++ b/Split/Storage/Splits/PersistentSplitsStorage.swift @@ -38,8 +38,9 @@ class DefaultPersistentSplitsStorage: PersistentSplitsStorage { } func update(splitChange: ProcessedSplitChange, onFailure: ((Error) -> Void)? = nil) { - // Execute transactionally: all operations must succeed or all must fail - coreDataHelper.performAndWait { [weak self] in + // This is intentionally async to avoid blocking the caller thread. + // All operations must succeed or all must fail. + coreDataHelper.perform { [weak self] in guard let self = self else { return } do { @@ -55,6 +56,8 @@ class DefaultPersistentSplitsStorage: PersistentSplitsStorage { try self.coreDataHelper.saveWithErrorHandling() } catch { Logger.e("Transactional flags update failed: \(error.localizedDescription)") + // Rollback to avoid leaving invalid pending changes in the shared context, + self.coreDataHelper.rollback() onFailure?(error) } } diff --git a/SplitTests/Fake/Storage/CoreDataHelperStub.swift b/SplitTests/Fake/Storage/CoreDataHelperStub.swift index 0dd736f5..aead6c42 100644 --- a/SplitTests/Fake/Storage/CoreDataHelperStub.swift +++ b/SplitTests/Fake/Storage/CoreDataHelperStub.swift @@ -11,6 +11,7 @@ class CoreDataHelperStub: CoreDataHelper { var shouldFailOnSave = false var saveError: Error = NSError(domain: "TestCoreData", code: 500, userInfo: [NSLocalizedDescriptionKey: "Simulated save failure"]) + var rollbackCalled = false init() { let model = NSManagedObjectModel() @@ -39,5 +40,9 @@ class CoreDataHelperStub: CoreDataHelper { } // Success } + + override func rollback() { + rollbackCalled = true + } } diff --git a/SplitTests/Storage/CoreDataHelperTests.swift b/SplitTests/Storage/CoreDataHelperTests.swift index c2c6bae8..c41c111e 100644 --- a/SplitTests/Storage/CoreDataHelperTests.swift +++ b/SplitTests/Storage/CoreDataHelperTests.swift @@ -80,5 +80,26 @@ class CoreDataHelperTests: XCTestCase { // Context without persistent store should not throw when there are no changes XCTAssertNoThrow(try invalidHelper.saveWithErrorHandling()) } + + func testRollbackClearsInvalidPendingChangesAndAllowsNextSave() { + // Create an invalid entity that will cause a validation failure on save. + coreDataHelper.performAndWait { + _ = self.coreDataHelper.create(entity: .generalInfo) + } + + XCTAssertThrowsError(try coreDataHelper.saveWithErrorHandling()) + + // Rollback should clear the invalid pending changes so future saves can succeed. + coreDataHelper.rollback() + + coreDataHelper.performAndWait { + if let entity = self.coreDataHelper.create(entity: .generalInfo) as? GeneralInfoEntity { + entity.name = "post_rollback_ok" + entity.stringValue = "value" + } + } + + XCTAssertNoThrow(try coreDataHelper.saveWithErrorHandling()) + } } diff --git a/SplitTests/Storage/PersistentSplitsStorageTransactionalTests.swift b/SplitTests/Storage/PersistentSplitsStorageTransactionalTests.swift index 76856bb7..732f625c 100644 --- a/SplitTests/Storage/PersistentSplitsStorageTransactionalTests.swift +++ b/SplitTests/Storage/PersistentSplitsStorageTransactionalTests.swift @@ -65,6 +65,21 @@ class PersistentSplitsStorageTransactionalTests: XCTestCase { XCTAssertNotNil(reportedError, "Error should be reported") } + func testRollbackIsInvokedOnSaveError() { + coreDataHelperStub.shouldFailOnSave = true + + let change = ProcessedSplitChange( + activeSplits: [createSplit(name: "split1")], + archivedSplits: [], + changeNumber: 100, + updateTimestamp: 1000 + ) + + splitsStorage.update(splitChange: change, onFailure: { _ in }) + + XCTAssertTrue(coreDataHelperStub.rollbackCalled, "Rollback should be invoked when transactional save fails") + } + func testNilFailureCallbackIsHandled() { let change = ProcessedSplitChange( activeSplits: [createSplit(name: "split1")], From 066ca1592f00eff582d25760210c4eb6a17ba8ab Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 18 Dec 2025 19:55:34 -0300 Subject: [PATCH 8/9] Fix test --- .../EncryptionKeyValidationIntegrationTest.swift | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/SplitTests/Integration/EncryptionKeyValidationIntegrationTest.swift b/SplitTests/Integration/EncryptionKeyValidationIntegrationTest.swift index e601cbe7..5360b692 100644 --- a/SplitTests/Integration/EncryptionKeyValidationIntegrationTest.swift +++ b/SplitTests/Integration/EncryptionKeyValidationIntegrationTest.swift @@ -243,14 +243,19 @@ class EncryptionKeyValidationIntegrationTest: XCTestCase { } wait(for: [checkExp], timeout: 3) - // 2. Disable encryption + // 2. Create a fresh dbHelper + dbHelper = IntegrationCoreDataHelper.get(databaseName: testDbName, + dispatchQueue: DispatchQueue.global()) + let freshGeneralInfoDao = CoreDataGeneralInfoDao(coreDataHelper: dbHelper) + + // 3. Disable encryption let factory = createFactory(encryptionEnabled: false) waitForReady(factory: factory) - // 3. Verify verifier is removed + // 4. Verify verifier is removed let verifyExp = expectation(description: "Verifier removed") DispatchQueue.global().asyncAfter(deadline: .now() + 1.0) { - XCTAssertNil(generalInfoDao.stringValue(info: .encryptionVerifier)) + XCTAssertNil(freshGeneralInfoDao.stringValue(info: .encryptionVerifier)) verifyExp.fulfill() } wait(for: [verifyExp], timeout: 3) From bb0a86e412a9c800d791403a2675bbf99ddc84d1 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Thu, 18 Dec 2025 17:06:44 -0300 Subject: [PATCH 9/9] chore: Update version to 3.5.2 and update CHANGES.txt --- CHANGES.txt | 3 +++ Split.podspec | 2 +- Split/Common/Utils/Version.swift | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index fe456e95..dd075f5e 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,6 @@ +3.5.2: (Dec 18, 2025) +- Fixed issue where targeting rules cache integrity could be lost if a SQLite write failed. + 3.5.1: (Dec 17, 2025) - Fixed issue in encryption mode where SDK cache functionality could not be recovered if the encryption key was removed. diff --git a/Split.podspec b/Split.podspec index 4b7d37b5..5c9655f2 100644 --- a/Split.podspec +++ b/Split.podspec @@ -1,7 +1,7 @@ Pod::Spec.new do |s| s.name = 'Split' s.module_name = 'Split' - s.version = '3.5.2-rc2' + s.version = '3.5.2' s.summary = 'iOS SDK for Split' s.description = <<-DESC This SDK is designed to work with Split, the platform for controlled rollouts, serving features to your users via the Split feature flag to manage your complete customer experience. diff --git a/Split/Common/Utils/Version.swift b/Split/Common/Utils/Version.swift index eb82f2f8..8b552e67 100644 --- a/Split/Common/Utils/Version.swift +++ b/Split/Common/Utils/Version.swift @@ -10,7 +10,7 @@ import Foundation class Version { private static let kSdkPlatform: String = "ios" - private static let kVersion = "3.5.2-rc2" + private static let kVersion = "3.5.2" static var semantic: String { return kVersion