diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7d32153..de54e59 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -69,7 +69,7 @@ jobs: test: name: Tests runs-on: macos-15 - timeout-minutes: 20 + timeout-minutes: 25 steps: - uses: actions/checkout@v4 @@ -106,7 +106,7 @@ jobs: done - name: Run unit tests with coverage - timeout-minutes: 10 + timeout-minutes: 15 run: | set -o pipefail xcodebuild test-without-building \ @@ -116,12 +116,19 @@ jobs: -only-testing:DittoTests \ -enableCodeCoverage YES \ -resultBundlePath TestResults.xcresult \ - 2>&1 | tee /tmp/test_output.log | tail -30 + -test-timeouts-enabled YES \ + -default-test-execution-time-allowance 60 \ + -maximum-test-execution-time-allowance 180 \ + 2>&1 | tee /tmp/test_output.log - name: Show test errors if: failure() run: | - grep -i 'error\|failed\|failure' /tmp/test_output.log | tail -40 | while IFS= read -r line; do + echo "=== Last 200 lines of test output ===" + tail -200 /tmp/test_output.log || true + echo "" + echo "=== Errors and failures ===" + grep -iE 'error|failed|failure|timed out|hung' /tmp/test_output.log | tail -40 | while IFS= read -r line; do echo "::error::TEST: $line" done @@ -149,7 +156,7 @@ jobs: ui-test: name: UI Tests runs-on: macos-15 - timeout-minutes: 20 + timeout-minutes: 30 steps: - uses: actions/checkout@v4 @@ -180,7 +187,7 @@ jobs: 2>&1 | tee /tmp/ui_build_output.log | tail -20 - name: Run UI tests - timeout-minutes: 10 + timeout-minutes: 15 run: | set -o pipefail xcodebuild test-without-building \ @@ -196,7 +203,7 @@ jobs: CODE_SIGN_IDENTITY="" \ CODE_SIGNING_REQUIRED=NO \ CODE_SIGNING_ALLOWED=NO \ - 2>&1 | tee /tmp/ui_test_output.log | tail -50 + 2>&1 | tee /tmp/ui_test_output.log - name: Show UI test output if: always() diff --git a/Ditto/DittoApp.swift b/Ditto/DittoApp.swift index 7994cdb..cc00939 100644 --- a/Ditto/DittoApp.swift +++ b/Ditto/DittoApp.swift @@ -24,7 +24,7 @@ struct DittoApp: App { do { let container = try CloudSyncManager.makeModelContainer(cloudSyncEnabled: false) - // Migrate legacy Core Data store before creating DittoStore, + // Migrate legacy NSUserDefaults-backed store before creating DittoStore, // so ensureProfileExists() finds migrated data instead of creating presets if LegacyDataMigrator.needsMigration { let migrationContext = ModelContext(container) diff --git a/Ditto/LegacyDataMigrator.swift b/Ditto/LegacyDataMigrator.swift index 7b9e2d4..30a16f5 100644 --- a/Ditto/LegacyDataMigrator.swift +++ b/Ditto/LegacyDataMigrator.swift @@ -1,177 +1,155 @@ -import CoreData import Foundation import SwiftData -/// Migrates data from the legacy Core Data store (v1/v2) to the new SwiftData store. +/// Migrates data from the legacy NSUserDefaults-backed store (v1/v2) to the new SwiftData store. /// -/// The old Core Data model used entities: Profile, Category, Ditto -/// with ordered relationships and snake_case attributes (use_count). -/// This migrator reads the old store, creates equivalent SwiftData objects, -/// and removes the old store files after successful migration. -@available(iOS, deprecated: 18.0, message: "Remove once all users have migrated from Core Data (target: v4.0)") +/// The pre-3.0 app persisted user content directly in NSUserDefaults under two keys: +/// - "categories": `[String]` — ordered list of category titles +/// - "dittos": `[String: [String]]` — category title → ordered list of ditto texts +/// +/// Some installs wrote to the shared App Group suite (once the keyboard extension shipped), +/// while earlier installs wrote to `UserDefaults.standard`. We check both, prefer whichever +/// has data, and merge if both are populated. +/// +/// IMPORTANT: We deliberately do NOT delete the legacy keys from NSUserDefaults after a +/// successful migration. Keeping the source data intact lets users roll back to an older +/// build (or re-run the migration) without data loss. +@available(iOS, deprecated: 18.0, message: "Remove once all users have migrated from NSUserDefaults (target: v4.0)") enum LegacyDataMigrator { private static let appGroupIdentifier = "group.io.kern.ditto" - private static let migrationCompleteKey = "legacyCoreDataMigrationComplete" + private static let migrationCompleteKey = "legacyUserDefaultsMigrationComplete" + + private static let legacyCategoriesKey = "categories" + private static let legacyDittosKey = "dittos" - /// Returns true if legacy Core Data files exist and haven't been migrated yet. + /// Returns true if legacy NSUserDefaults content exists and hasn't been migrated yet. static var needsMigration: Bool { guard let defaults = UserDefaults(suiteName: appGroupIdentifier) else { return false } if defaults.bool(forKey: migrationCompleteKey) { return false } - return legacyStoreURL != nil - } - - /// The URL of the legacy Core Data SQLite store, if it exists. - private static var legacyStoreURL: URL? { - guard let groupURL = FileManager.default.containerURL( - forSecurityApplicationGroupIdentifier: appGroupIdentifier - ) else { return nil } - - // Check common Core Data store filenames from the old app - let candidates = [ - groupURL.appendingPathComponent("Ditto.sqlite"), - groupURL.appendingPathComponent("ditto.sqlite") - ] - for url in candidates where FileManager.default.fileExists(atPath: url.path) { - return url - } - - // Also check the default application support directory - let appSupport = FileManager.default.urls(for: .applicationSupportDirectory, in: .userDomainMask).first - if let url = appSupport?.appendingPathComponent("Ditto.sqlite"), - FileManager.default.fileExists(atPath: url.path) { - return url - } - - return nil + return !readLegacyCategories().isEmpty } // MARK: - Migration - /// Migrates legacy Core Data content into the given SwiftData model context. + /// Migrates legacy NSUserDefaults content into the given SwiftData model context. /// Returns `true` if data was migrated, `false` if no legacy data was found. + /// + /// The legacy NSUserDefaults entries are preserved (not deleted) so the source data + /// remains available for rollback or repeated migration runs. @discardableResult static func migrateIfNeeded(into context: ModelContext) -> Bool { - guard let storeURL = legacyStoreURL else { + let legacyCategories = readLegacyCategories() + guard !legacyCategories.isEmpty else { markComplete() return false } - do { - let legacyData = try readLegacyStore(at: storeURL) - if legacyData.isEmpty { - markComplete() - cleanupLegacyFiles(at: storeURL) - return false - } + writeMigratedData(legacyCategories, into: context) - writeMigratedData(legacyData, into: context) + do { try context.save() - - markComplete() - cleanupLegacyFiles(at: storeURL) - return true } catch { - print("Legacy data migration failed: \(error)") + print("Legacy data migration save failed: \(error)") return false } + + markComplete() + return true } // MARK: - Read Legacy Store private struct LegacyCategory { let title: String - let dittos: [LegacyDitto] + let dittos: [String] } - private struct LegacyDitto { - let text: String - let useCount: Int - } - - private static func readLegacyStore(at url: URL) throws -> [LegacyCategory] { - guard let modelURL = Bundle.main.url(forResource: "Ditto", withExtension: "momd") - ?? Bundle.main.url(forResource: "Ditto", withExtension: "mom"), - let model = NSManagedObjectModel(contentsOf: modelURL) else { - print("Legacy Core Data model not found in bundle") - return [] - } - - let container = NSPersistentContainer(name: "Ditto", managedObjectModel: model) - let description = NSPersistentStoreDescription(url: url) - description.isReadOnly = true - description.shouldMigrateStoreAutomatically = true - description.shouldInferMappingModelAutomatically = true - container.persistentStoreDescriptions = [description] - - var loadError: Error? - container.loadPersistentStores { _, error in - loadError = error + /// Reads ordered legacy categories from both the App Group suite and standard defaults, + /// merging duplicates by title (App Group takes precedence; standard contributes any + /// categories or trailing dittos missing from the group store). + private static func readLegacyCategories() -> [LegacyCategory] { + let groupCategories = readLegacyCategories(from: UserDefaults(suiteName: appGroupIdentifier)) + let standardCategories = readLegacyCategories(from: .standard) + + if standardCategories.isEmpty { return groupCategories } + if groupCategories.isEmpty { return standardCategories } + + // Merge: keep order from group, then append any group-missing categories from standard. + // For shared categories, union the ditto lists while preserving group order. + var titleToIndex: [String: Int] = [:] + var merged: [LegacyCategory] = [] + for cat in groupCategories { + titleToIndex[cat.title] = merged.count + merged.append(cat) } - if let error = loadError { throw error } - - let moc = container.viewContext - - // Fetch the profile to get ordered categories - let profileRequest = NSFetchRequest(entityName: "Profile") - let profiles = try moc.fetch(profileRequest) - - guard let profile = profiles.first else { - // No profile means no data to migrate - try fetching categories directly - return try readCategoriesWithoutProfile(moc: moc) + for cat in standardCategories { + if let idx = titleToIndex[cat.title] { + var combined = merged[idx].dittos + for text in cat.dittos where !combined.contains(text) { + combined.append(text) + } + merged[idx] = LegacyCategory(title: merged[idx].title, dittos: combined) + } else { + titleToIndex[cat.title] = merged.count + merged.append(cat) + } } + return merged + } - // Core Data ordered relationship returns NSOrderedSet - guard let categoriesSet = profile.value(forKey: "categories") as? NSOrderedSet else { - return [] - } + private static func readLegacyCategories(from defaults: UserDefaults?) -> [LegacyCategory] { + guard let defaults else { return [] } + guard let titles = defaults.array(forKey: legacyCategoriesKey) as? [String], + !titles.isEmpty else { return [] } + let dittosByTitle = defaults.dictionary(forKey: legacyDittosKey) as? [String: [String]] ?? [:] - var result: [LegacyCategory] = [] - for case let categoryObj as NSManagedObject in categoriesSet { - let category = readCategory(categoryObj) - result.append(category) + return titles.map { title in + LegacyCategory(title: title, dittos: dittosByTitle[title] ?? []) } - return result } - private static func readCategoriesWithoutProfile(moc: NSManagedObjectContext) throws -> [LegacyCategory] { - let request = NSFetchRequest(entityName: "Category") - request.sortDescriptors = [NSSortDescriptor(key: "title", ascending: true)] - let categories = try moc.fetch(request) - return categories.map { readCategory($0) } - } - - private static func readCategory(_ obj: NSManagedObject) -> LegacyCategory { - let title = obj.value(forKey: "title") as? String ?? "" + // MARK: - Write Migrated Data - var dittos: [LegacyDitto] = [] - if let dittosSet = obj.value(forKey: "dittos") as? NSOrderedSet { - for case let dittoObj as NSManagedObject in dittosSet { - let text = dittoObj.value(forKey: "text") as? String ?? "" - let useCount = (dittoObj.value(forKey: "use_count") as? Int) ?? 0 - dittos.append(LegacyDitto(text: text, useCount: useCount)) - } + private static func writeMigratedData(_ categories: [LegacyCategory], into context: ModelContext) { + // Reuse an existing Profile if one was already created (e.g. by a previous + // partial run); otherwise create a new one. + let profile: Profile + let descriptor = FetchDescriptor() + if let existing = (try? context.fetch(descriptor))?.first { + profile = existing + } else { + profile = Profile() + context.insert(profile) } - return LegacyCategory(title: title, dittos: dittos) - } + // Track titles already in the profile so we don't duplicate preset categories. + let existingByTitle = Dictionary( + profile.orderedCategories.map { ($0.title, $0) }, + uniquingKeysWith: { first, _ in first } + ) + + var nextCategorySortOrder = profile.orderedCategories.count + + for legacyCat in categories { + let category: DittoCategory + if let existing = existingByTitle[legacyCat.title] { + category = existing + } else { + category = DittoCategory(title: legacyCat.title, profile: profile) + category.sortOrder = nextCategorySortOrder + nextCategorySortOrder += 1 + context.insert(category) + profile.categories?.append(category) + } - // MARK: - Write Migrated Data + let existingTexts = Set((category.dittos ?? []).map { $0.text }) + var nextDittoSortOrder = (category.dittos ?? []).count - private static func writeMigratedData(_ categories: [LegacyCategory], into context: ModelContext) { - let profile = Profile() - context.insert(profile) - - for (catIndex, legacyCat) in categories.enumerated() { - let category = DittoCategory(title: legacyCat.title, profile: profile) - category.sortOrder = catIndex - context.insert(category) - profile.categories?.append(category) - - for (dittoIndex, legacyDitto) in legacyCat.dittos.enumerated() { - let item = DittoItem(text: legacyDitto.text, category: category) - item.useCount = legacyDitto.useCount - item.sortOrder = dittoIndex + for text in legacyCat.dittos where !existingTexts.contains(text) { + let item = DittoItem(text: text, category: category) + item.sortOrder = nextDittoSortOrder + nextDittoSortOrder += 1 context.insert(item) category.dittos?.append(item) } @@ -180,16 +158,9 @@ enum LegacyDataMigrator { // MARK: - Cleanup + /// Records that migration finished. The legacy NSUserDefaults entries are intentionally + /// left in place so the source data is preserved. private static func markComplete() { UserDefaults(suiteName: appGroupIdentifier)?.set(true, forKey: migrationCompleteKey) } - - private static func cleanupLegacyFiles(at storeURL: URL) { - let fm = FileManager.default - let suffixes = ["", "-shm", "-wal", "-journal"] - for suffix in suffixes { - let url = URL(fileURLWithPath: storeURL.path + suffix) - try? fm.removeItem(at: url) - } - } } diff --git a/DittoTests/LegacyDataMigratorTests.swift b/DittoTests/LegacyDataMigratorTests.swift index e9e0d6c..a5676e9 100644 --- a/DittoTests/LegacyDataMigratorTests.swift +++ b/DittoTests/LegacyDataMigratorTests.swift @@ -6,38 +6,153 @@ import Testing @Suite("LegacyDataMigrator Tests", .serialized) struct LegacyDataMigratorTests { - @Test("Migration flag prevents repeated migration") - func migrationFlag() { - // Without a legacy store file present, needsMigration should be false - // (or true only if there's actually an old .sqlite file) - // This validates the guard logic works - let defaults = UserDefaults(suiteName: "group.io.kern.ditto") - let originalValue = defaults?.bool(forKey: "legacyCoreDataMigrationComplete") - - // Mark as complete - defaults?.set(true, forKey: "legacyCoreDataMigrationComplete") - #expect(!LegacyDataMigrator.needsMigration) + private let appGroupSuite = "group.io.kern.ditto" + private let completeKey = "legacyUserDefaultsMigrationComplete" + private let categoriesKey = "categories" + private let dittosKey = "dittos" + + private struct DefaultsSnapshot { + let complete: Bool + let categories: Any? + let dittos: Any? + let stdCategories: Any? + let stdDittos: Any? + } + + private func appGroupDefaults() -> UserDefaults? { + UserDefaults(suiteName: appGroupSuite) + } + + private func snapshot() -> DefaultsSnapshot { + let group = appGroupDefaults() + return DefaultsSnapshot( + complete: group?.bool(forKey: completeKey) ?? false, + categories: group?.object(forKey: categoriesKey), + dittos: group?.object(forKey: dittosKey), + stdCategories: UserDefaults.standard.object(forKey: categoriesKey), + stdDittos: UserDefaults.standard.object(forKey: dittosKey) + ) + } + + private func restore(_ snap: DefaultsSnapshot) { + let group = appGroupDefaults() + group?.set(snap.complete, forKey: completeKey) + group?.set(snap.categories, forKey: categoriesKey) + group?.set(snap.dittos, forKey: dittosKey) + UserDefaults.standard.set(snap.stdCategories, forKey: categoriesKey) + UserDefaults.standard.set(snap.stdDittos, forKey: dittosKey) + } - // Restore original value - if let original = originalValue, original { - defaults?.set(true, forKey: "legacyCoreDataMigrationComplete") - } else { - defaults?.removeObject(forKey: "legacyCoreDataMigrationComplete") - } + private func clearAll() { + let group = appGroupDefaults() + group?.removeObject(forKey: completeKey) + group?.removeObject(forKey: categoriesKey) + group?.removeObject(forKey: dittosKey) + UserDefaults.standard.removeObject(forKey: categoriesKey) + UserDefaults.standard.removeObject(forKey: dittosKey) } - @Test("Migration returns false when no legacy store exists") - func noLegacyStore() throws { + private func makeContext() throws -> ModelContext { let schema = Schema([Profile.self, DittoCategory.self, DittoItem.self]) let config = ModelConfiguration("Migration-\(UUID())", schema: schema, isStoredInMemoryOnly: true, cloudKitDatabase: .none) let container = try ModelContainer(for: schema, configurations: [config]) - let context = ModelContext(container) + return ModelContext(container) + } + + @Test("Migration flag prevents repeated migration") + func migrationFlag() { + let snap = snapshot() + defer { restore(snap) } + + clearAll() + appGroupDefaults()?.set(["Personal"], forKey: categoriesKey) + appGroupDefaults()?.set(["Personal": ["hello"]], forKey: dittosKey) + appGroupDefaults()?.set(true, forKey: completeKey) + + #expect(!LegacyDataMigrator.needsMigration) + } + + @Test("needsMigration is false when no legacy data exists") + func noLegacyData() { + let snap = snapshot() + defer { restore(snap) } - // Clear migration flag to allow check - let defaults = UserDefaults(suiteName: "group.io.kern.ditto") - defaults?.removeObject(forKey: "legacyCoreDataMigrationComplete") + clearAll() + #expect(!LegacyDataMigrator.needsMigration) + } + + @Test("needsMigration is true when legacy data is present") + func detectsLegacyData() { + let snap = snapshot() + defer { restore(snap) } + + clearAll() + appGroupDefaults()?.set(["Greetings"], forKey: categoriesKey) + appGroupDefaults()?.set(["Greetings": ["hi"]], forKey: dittosKey) + + #expect(LegacyDataMigrator.needsMigration) + } + + @Test("Migration imports categories and dittos preserving order") + func migratesData() throws { + let snap = snapshot() + defer { restore(snap) } + + clearAll() + let titles = ["Work", "Personal"] + let dittos: [String: [String]] = [ + "Work": ["meeting at ___", "OOO today"], + "Personal": ["on my way", "running late"] + ] + appGroupDefaults()?.set(titles, forKey: categoriesKey) + appGroupDefaults()?.set(dittos, forKey: dittosKey) + + let context = try makeContext() + let result = LegacyDataMigrator.migrateIfNeeded(into: context) + #expect(result) + let profiles = try context.fetch(FetchDescriptor()) + let profile = try #require(profiles.first) + let ordered = profile.orderedCategories + #expect(ordered.map { $0.title } == titles) + #expect(ordered[0].orderedDittos.map { $0.text } == ["meeting at ___", "OOO today"]) + #expect(ordered[1].orderedDittos.map { $0.text } == ["on my way", "running late"]) + } + + @Test("Migration does not delete legacy NSUserDefaults entries") + func preservesLegacyDefaults() throws { + let snap = snapshot() + defer { restore(snap) } + + clearAll() + let titles = ["Notes"] + let dittos: [String: [String]] = ["Notes": ["remember the milk"]] + appGroupDefaults()?.set(titles, forKey: categoriesKey) + appGroupDefaults()?.set(dittos, forKey: dittosKey) + + let context = try makeContext() + _ = LegacyDataMigrator.migrateIfNeeded(into: context) + + // Legacy entries must remain in NSUserDefaults after migration + #expect(appGroupDefaults()?.array(forKey: categoriesKey) as? [String] == titles) + #expect((appGroupDefaults()?.dictionary(forKey: dittosKey) as? [String: [String]]) == dittos) + } + + @Test("Migration reads from UserDefaults.standard when App Group is empty") + func readsFromStandardDefaults() throws { + let snap = snapshot() + defer { restore(snap) } + + clearAll() + UserDefaults.standard.set(["Old"], forKey: categoriesKey) + UserDefaults.standard.set(["Old": ["legacy ditto"]], forKey: dittosKey) + + let context = try makeContext() let result = LegacyDataMigrator.migrateIfNeeded(into: context) - #expect(!result) + #expect(result) + + let profile = try #require(try context.fetch(FetchDescriptor()).first) + #expect(profile.orderedCategories.map { $0.title } == ["Old"]) + #expect(profile.orderedCategories.first?.orderedDittos.map { $0.text } == ["legacy ditto"]) } } diff --git a/DittoUITests/ScreenshotTests.swift b/DittoUITests/ScreenshotTests.swift index c849651..25b69d5 100644 --- a/DittoUITests/ScreenshotTests.swift +++ b/DittoUITests/ScreenshotTests.swift @@ -1,5 +1,6 @@ import XCTest +@MainActor final class ScreenshotTests: XCTestCase { private var app: XCUIApplication! diff --git a/DittoUITests/SnapshotHelper.swift b/DittoUITests/SnapshotHelper.swift index 6dec130..9ccbc0c 100644 --- a/DittoUITests/SnapshotHelper.swift +++ b/DittoUITests/SnapshotHelper.swift @@ -169,7 +169,7 @@ open class Snapshot: NSObject { let screenshot = XCUIScreen.main.screenshot() #if os(iOS) && !targetEnvironment(macCatalyst) - let image = XCUIDevice.shared.orientation.isLandscape ? fixLandscapeOrientation(image: screenshot.image) : screenshot.image + let image = XCUIDevice.shared.orientation.isLandscape ? fixLandscapeOrientation(image: screenshot.image) : screenshot.image #else let image = screenshot.image #endif