diff --git a/Sources/SQLiteData/CloudKit/SyncEngine.swift b/Sources/SQLiteData/CloudKit/SyncEngine.swift index 878e832f..17b25e13 100644 --- a/Sources/SQLiteData/CloudKit/SyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/SyncEngine.swift @@ -2009,6 +2009,12 @@ else { return "" } + + // On conflict only update columns that have a value the server record. + let columnNamesForUpdate = nonPrimaryKeyChangedColumns.filter { columnName in + record[columnName] is CKAsset || record.encryptedValues.allKeys().contains(columnName) + } + var record = record let recordHasAsset = nonPrimaryKeyChangedColumns.contains { columnName in record[columnName] is CKAsset @@ -2035,26 +2041,33 @@ } .joined(separator: ", ") ) - query.append(") ON CONFLICT(\(quote: T.primaryKey.name)) DO UPDATE SET ") - query.append(" ") - query.append( - nonPrimaryKeyChangedColumns - .map { columnName in - if let asset = record[columnName] as? CKAsset { - let data = try? asset.fileURL.map { try dataManager.wrappedValue.load($0) } - if data == nil { - reportIssue("Asset data not found on disk") + + // if there are no columns + if columnNamesForUpdate.isEmpty { + query.append(") ON CONFLICT(\(quote: T.primaryKey.name)) DO NOTHING ") + } else { + query.append(") ON CONFLICT(\(quote: T.primaryKey.name)) DO UPDATE SET") + query.append(" ") + query.append( + columnNamesForUpdate + .map { columnName in + if let asset = record[columnName] as? CKAsset { + let data = try? asset.fileURL.map { try dataManager.wrappedValue.load($0) } + if data == nil { + reportIssue("Asset data not found on disk") + } + return "\(quote: columnName) = \(data?.queryFragment ?? #""excluded".\#(quote: columnName)"#)" + } else { + return """ + \(quote: columnName) = \ + \(record.encryptedValues[columnName]?.queryFragment ?? #""excluded".\#(quote: columnName)"#) + """ + } } - return "\(quote: columnName) = \(data?.queryFragment ?? #""excluded".\#(quote: columnName)"#)" - } else { - return """ - \(quote: columnName) = \ - \(record.encryptedValues[columnName]?.queryFragment ?? #""excluded".\#(quote: columnName)"#) - """ - } - } - .joined(separator: ",") - ) + .joined(separator: ", ") + ) + } + return query } } @@ -2437,6 +2450,14 @@ ) -> QueryFragment { let allColumnNames = T.TableColumns.writableColumns.map(\.name) let hasNonPrimaryKeyColumns = columnNames.contains { $0 != T.primaryKey.name } + + let columnNamesForUpdate = columnNames.filter { columnName in + if columnName == T.primaryKey.name { + return false + } + return (record[columnName] is CKAsset) || record.encryptedValues.allKeys().contains(columnName) + } + var query: QueryFragment = "INSERT INTO \(T.self) (" query.append(allColumnNames.map { "\(quote: $0)" }.joined(separator: ", ")) query.append(") VALUES (") @@ -2454,11 +2475,10 @@ .joined(separator: ", ") ) query.append(") ON CONFLICT(\(quote: T.primaryKey.name)) DO") - if hasNonPrimaryKeyColumns { + if !columnNamesForUpdate.isEmpty { query.append(" UPDATE SET ") query.append( - columnNames - .filter { $0 != T.primaryKey.name } + columnNamesForUpdate .map { """ \(quote: $0) = "excluded".\(quote: $0) diff --git a/Tests/SQLiteDataTests/Internal/Schema.swift b/Tests/SQLiteDataTests/Internal/Schema.swift index ba018d1c..3a05392f 100644 --- a/Tests/SQLiteDataTests/Internal/Schema.swift +++ b/Tests/SQLiteDataTests/Internal/Schema.swift @@ -77,7 +77,8 @@ import SQLiteData @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) func database( containerIdentifier: String, - attachMetadatabase: Bool + attachMetadatabase: Bool, + url databaseURL: URL? = nil ) throws -> DatabasePool { var configuration = Configuration() configuration.prepareDatabase { db in @@ -88,8 +89,11 @@ func database( // print($0.expandedDescription) // } } - let url = URL.temporaryDirectory.appending(path: "\(UUID().uuidString).sqlite") + let url = databaseURL ?? URL.temporaryDirectory.appending(path: "\(UUID().uuidString).sqlite") let database = try DatabasePool(path: url.path(), configuration: configuration) + guard databaseURL == nil else { + return database + } try database.write { db in try #sql( """ diff --git a/Tests/SQLiteDataTests/MigrationTests.swift b/Tests/SQLiteDataTests/MigrationTests.swift index 42e5f24b..a5db78a1 100644 --- a/Tests/SQLiteDataTests/MigrationTests.swift +++ b/Tests/SQLiteDataTests/MigrationTests.swift @@ -33,7 +33,176 @@ import Testing } } + @available(iOS 15, *) @Table private struct Model { var date: Date } + + +#if canImport(CloudKit) + @available(iOS 15, *) + @Table private struct User: Identifiable { + var id: UUID + var name: String + } + + @Table("users") private struct UpdatedUser: Identifiable { + var id: UUID + var name: String + var honorific: String? + } + + import CloudKit + import ConcurrencyExtras + import CustomDump + import InlineSnapshotTesting + import OrderedCollections + import SQLiteData + import SQLiteDataTestSupport + import SnapshotTestingCustomDump + import Testing + + @MainActor + @Suite + final class MigrationSyncEngineTests { + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + + private let _container: any Sendable + + var container: MockCloudContainer { + _container as! MockCloudContainer + } + + let testContainerIdentifier: String + let databaseURL: URL + + func userDatabase() throws -> UserDatabase { + UserDatabase( + database: try SQLiteDataTests.database( + containerIdentifier: testContainerIdentifier, + attachMetadatabase: false, + url: databaseURL + ) + ) + } + + + init() async throws { + testContainerIdentifier = "iCloud.co.pointfree.Testing.\(UUID())" + databaseURL = URL.temporaryDirectory.appending(path: "\(UUID().uuidString).sqlite") + + let privateDatabase = MockCloudDatabase(databaseScope: .private) + let sharedDatabase = MockCloudDatabase(databaseScope: .shared) + let container = MockCloudContainer( + accountStatus: _AccountStatusScope.accountStatus, + containerIdentifier: testContainerIdentifier, + privateCloudDatabase: privateDatabase, + sharedCloudDatabase: sharedDatabase + ) + _container = container + privateDatabase.set(container: container) + sharedDatabase.set(container: container) + } + + @available(iOS 15, *) + @Test func handleDataMigration() async throws { + let userDatabase = try userDatabase() + // Do first migration + try await userDatabase.userWrite { db in + try #sql( + """ + CREATE TABLE "users" ( + "id" TEXT PRIMARY KEY NOT NULL ON CONFLICT REPLACE DEFAULT (uuid()), + "name" TEXT NOT NULL + ) + """ + ) + .execute(db) + } + + var syncEngine: Optional = try await SyncEngine( + container: container, + userDatabase: userDatabase, + delegate: nil, + privateTables: User.self, + startImmediately: true + ) + + let currentUserRecordID = CKRecord.ID( + recordName: "currentUser" + ) + + await syncEngine!.handleEvent( + .accountChange(changeType: .signIn(currentUser: currentUserRecordID)), + syncEngine: syncEngine!.private + ) + await syncEngine!.handleEvent( + .accountChange(changeType: .signIn(currentUser: currentUserRecordID)), + syncEngine: syncEngine!.shared + ) + try await syncEngine!.processPendingDatabaseChanges(scope: .private) + + try await userDatabase.userWrite { db in + try User.insert { + User.Draft(name: "Bob") + User.Draft(name: "Alice") + User.Draft(name: "Alfred") + }.execute(db) + } + + // Do we need to await something here? + try await Task.sleep(for: .seconds(1)) + try await syncEngine?.processPendingRecordZoneChanges(scope: .private) + syncEngine?.stop() + syncEngine = nil + + try userDatabase.database.close() + + let newDbConnection = try self.userDatabase() + + try await newDbConnection.userWrite { db in + try #sql( + """ + ALTER TABLE "users" + ADD COLUMN "honorific" TEXT + """ + ) + .execute(db) + + let existingUsers = try UpdatedUser.all.fetchAll(db) + for user in existingUsers { + switch user.name.lowercased() { + case "bob": + try UpdatedUser.find(user.id).update { + $0.honorific = "Mr" + }.execute(db) + case "alice": + try UpdatedUser.find(user.id).update { + $0.honorific = "Ms" + }.execute(db) + default: + continue + } + } + } + + syncEngine = try await SyncEngine( + container: container, + userDatabase: newDbConnection, + delegate: nil, + privateTables: UpdatedUser.self, + startImmediately: true + ) + + let bob = try await newDbConnection.read { db in + try UpdatedUser.all.where { $0.name.eq("Bob") }.fetchOne(db) + } + #expect(bob?.honorific == "Mr") + } + + + + // Do we need to wait here... + } +#endif