DB transaction functionality

.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:

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 = "<group>"; };
 		C53207E82EF44A2100418BB1 /* PersistenceBreakerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PersistenceBreakerTests.swift; sourceTree = "<group>"; };
 		C53207EA2EF44A2F00418BB1 /* PersistenceBreaker.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PersistenceBreaker.swift; sourceTree = "<group>"; };
+		C53207ED2EF452C000418BB1 /* CoreDataHelperStub.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CoreDataHelperStub.swift; sourceTree = "<group>"; };
+		C53207F02EF456AF00418BB1 /* CoreDataHelperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CoreDataHelperTests.swift; sourceTree = "<group>"; };
 		C539CAB52D88C1F10050C732 /* RuleBasedSegment.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RuleBasedSegment.swift; sourceTree = "<group>"; };
 		C539CABC2D88C7410050C732 /* PersistentRuleBasedSegmentsStorage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PersistentRuleBasedSegmentsStorage.swift; sourceTree = "<group>"; };
 		C539CABD2D88C7410050C732 /* RuleBasedSegmentsStorage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RuleBasedSegmentsStorage.swift; sourceTree = "<group>"; };
@@ -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 */,

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
     }

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

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 {

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
+    }
+}
+

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
+    }
 }

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
+    }
 }

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()
     }
 }

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())
+    }
+}
+

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() {
     }