Making sharing more testable

Sources/SharingGRDBCore/CloudKit/CloudContainer.swift

@@ -1,20 +1,50 @@
 #if canImport(CloudKit)
 import CloudKit
 
+@available(iOS 15, tvOS 15, macOS 12, watchOS 8, *)
 package protocol CloudContainer<Database>: AnyObject, Equatable, Hashable, Sendable {
   associatedtype Database: CloudDatabase
 
   func accountStatus() async throws -> CKAccountStatus
   var containerIdentifier: String? { get }
   var rawValue: CKContainer { get }
   var privateCloudDatabase: Database { get }
-  func accept(_ metadata: CKShare.Metadata) async throws -> CKShare
+  func accept(_ metadata: ShareMetadata) async throws -> CKShare
   static func createContainer(identifier containerIdentifier: String) -> Self
   var sharedCloudDatabase: Database { get }
-  @available(macOS 12.0, iOS 15.0, tvOS 15.0, watchOS 8.0, *)
-  func shareMetadata(for url: URL, shouldFetchRootRecord: Bool) async throws -> CKShare.Metadata
+  @available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
+  func shareMetadata(for share: CKShare, shouldFetchRootRecord: Bool) async throws -> ShareMetadata
 }
 
+@available(iOS 15, tvOS 15, macOS 12, watchOS 8, *)
+package struct ShareMetadata: Hashable {
+  package var containerIdentifier: String
+  package var hierarchicalRootRecordID: CKRecord.ID?
+  package var rootRecord: CKRecord?
+  package var share: CKShare
+  package var rawValue: CKShare.Metadata?
+  package init(rawValue: CKShare.Metadata) {
+    self.containerIdentifier = rawValue.containerIdentifier
+    self.hierarchicalRootRecordID = rawValue.hierarchicalRootRecordID
+    self.rootRecord = rawValue.rootRecord
+    self.share = rawValue.share
+    self.rawValue = rawValue
+  }
+  package init(
+    containerIdentifier: String,
+    hierarchicalRootRecordID: CKRecord.ID?,
+    rootRecord: CKRecord?,
+    share: CKShare
+  ) {
+    self.containerIdentifier = containerIdentifier
+    self.hierarchicalRootRecordID = hierarchicalRootRecordID
+    self.rootRecord = rootRecord
+    self.share = share
+    self.rawValue = nil
+  }
+}
+
+@available(iOS 15, tvOS 15, macOS 12, watchOS 8, *)
 extension CloudContainer {
   package func database(for recordID: CKRecord.ID) -> any CloudDatabase {
     recordID.zoneID.ownerName == CKCurrentUserDefaultName
@@ -23,7 +53,16 @@ extension CloudContainer {
   }
 }
 
+@available(iOS 15, tvOS 15, macOS 12, watchOS 8, *)
 extension CKContainer: CloudContainer {
+  package func accept(_ metadata: ShareMetadata) async throws -> CKShare {
+    guard let metadata = metadata.rawValue
+    else {
+      fatalError("This should never be called with 'ShareMetadata' that has a nil 'rawValue'")
+    }
+    return try await self.accept(metadata)
+  }
+
   package static func createContainer(identifier containerIdentifier: String) -> Self {
     Self(identifier: containerIdentifier)
   }
@@ -32,16 +71,16 @@ extension CKContainer: CloudContainer {
     self
   }
 
-  @available(macOS 12.0, iOS 15.0, tvOS 15.0, watchOS 8.0, *)
+  @available(macOS 12, iOS 15, tvOS 15, watchOS 8, *)
   package func shareMetadata(
-    for url: URL,
+    for share: CKShare,
     shouldFetchRootRecord: Bool = false
-  ) async throws -> CKShare.Metadata {
+  ) async throws -> ShareMetadata {
     try await withUnsafeThrowingContinuation { continuation in
-      let operation = CKFetchShareMetadataOperation(shareURLs: [url])
+      let operation = CKFetchShareMetadataOperation(shareURLs: [share.url].compactMap(\.self))
       operation.shouldFetchRootRecord = true
       operation.perShareMetadataResultBlock = { url, result in
-        continuation.resume(with: result)
+        continuation.resume(with: result.map(ShareMetadata.init(rawValue:)))
       }
       add(operation)
     }

Sources/SharingGRDBCore/CloudKit/CloudKit+StructuredQueries.swift

@@ -243,7 +243,7 @@ extension CKRecord {
         let column = column as! any WritableTableColumnExpression<T, Value>
         let didSet: Bool
         if let value = other[key] as? CKAsset {
-          didSet = setValue(value, forKey: key, at: other.encryptedValues[at: key])
+          didSet = setValue(value, forKey: key, at: other[at: key])
         } else if let value = other.encryptedValues[key] as? any EquatableCKRecordValueProtocol {
           didSet = setValue(value, forKey: key, at: other.encryptedValues[at: key])
         } else if other.encryptedValues[key] == nil {

Sources/SharingGRDBCore/CloudKit/CloudKitSharing.swift

@@ -7,6 +7,7 @@ import SwiftUI
   import UIKit
 #endif
 
+@available(iOS 15, tvOS 15, macOS 12, watchOS 8, *)
 public struct SharedRecord: Hashable, Identifiable, Sendable {
   let container: any CloudContainer
   public let share: CKShare
@@ -124,7 +125,7 @@ extension SyncEngine {
     let sharedRecord = try await existingShare ?? CKShare(
       rootRecord: rootRecord,
       shareID: CKRecord.ID(
-        recordName: UUID().uuidString,
+        recordName: "share-\(recordName)",
         zoneID: rootRecord.recordID.zoneID
       )
     )

Sources/SharingGRDBCore/CloudKit/Internal/MockCloudContainer.swift

@@ -18,6 +18,12 @@ package final class MockCloudContainer: CloudContainer, CustomDumpReflectable {
     self.containerIdentifier = containerIdentifier
     self.privateCloudDatabase = privateCloudDatabase
     self.sharedCloudDatabase = sharedCloudDatabase
+
+    guard let containerIdentifier else { return }
+    @Dependency(\.mockCloudContainers) var mockCloudContainers
+    mockCloudContainers.withValue { storage in
+      storage[containerIdentifier] = self
+    }
   }
 
   package func accountStatus() -> CKAccountStatus {
@@ -32,20 +38,47 @@ package final class MockCloudContainer: CloudContainer, CustomDumpReflectable {
     _accountStatus.withValue { $0 }
   }
 
-  package func shareMetadata(for url: URL, shouldFetchRootRecord: Bool) async throws -> CKShare.Metadata {
-    fatalError()
+  package func shareMetadata(
+    for share: CKShare,
+    shouldFetchRootRecord: Bool
+  ) async throws -> ShareMetadata {
+    let database = share.recordID.zoneID.ownerName == CKCurrentUserDefaultName
+    ? privateCloudDatabase
+    : sharedCloudDatabase
+
+    let rootRecord: CKRecord? = database.storage.withValue {
+      $0[share.recordID.zoneID]?.values.first { record in
+        record.share?.recordID == share.recordID
+      }
+    }
+
+    return ShareMetadata(
+      containerIdentifier: containerIdentifier!,
+      hierarchicalRootRecordID: rootRecord?.recordID,
+      rootRecord: shouldFetchRootRecord ? rootRecord : nil,
+      share: share
+    )
   }
 
-  package func accept(_ metadata: CKShare.Metadata) async throws -> CKShare {
-    fatalError()
+  package func accept(_ metadata: ShareMetadata) async throws -> CKShare {
+    guard let rootRecord = metadata.rootRecord
+    else {
+      fatalError("Must provide root record in mock shares during tests.")
+    }
+
+    let (saveResults, _) = try sharedCloudDatabase.modifyRecords(
+      saving: [metadata.share, rootRecord]
+    )
+    try saveResults.values.forEach { _ = try $0.get() }
+    return metadata.share
   }
 
   package static func createContainer(identifier containerIdentifier: String) -> MockCloudContainer {
     @Dependency(\.mockCloudContainers) var mockCloudContainers
     return mockCloudContainers.withValue { storage in
       let container: MockCloudContainer
       if let existingContainer = storage[containerIdentifier] {
-        container = existingContainer
+        return existingContainer
       } else {
         container = MockCloudContainer(
           accountStatus: .available,
@@ -82,7 +115,10 @@ package final class MockCloudContainer: CloudContainer, CustomDumpReflectable {
 }
 
 @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
-private enum MockCloudContainersKey: TestDependencyKey {
+private enum MockCloudContainersKey: DependencyKey {
+  static var liveValue: LockIsolated<[String: MockCloudContainer]> {
+    LockIsolated<[String: MockCloudContainer]>([:])
+  }
   static var testValue: LockIsolated<[String: MockCloudContainer]> {
     LockIsolated<[String: MockCloudContainer]>([:])
   }

Sources/SharingGRDBCore/CloudKit/Internal/MockCloudDatabase.swift

@@ -87,6 +87,22 @@ package final class MockCloudDatabase: CloudDatabase {
       switch savePolicy {
       case .ifServerRecordUnchanged:
         for recordToSave in recordsToSave {
+          if let share = recordToSave as? CKShare {
+            let isSavingRootRecord = recordsToSave.contains(where: { $0.share?.recordID == share.recordID })
+            let shareWasPreviouslySaved = storage[share.recordID.zoneID]?[share.recordID] != nil
+            guard shareWasPreviouslySaved || isSavingRootRecord
+            else {
+              reportIssue(
+                """
+                An added share is being saved without its rootRecord being saved in the same \
+                operation.
+                """
+              )
+              saveResults[recordToSave.recordID] = .failure(CKError(.invalidArguments))
+              continue
+            }
+          }
+
           guard storage[recordToSave.recordID.zoneID] != nil
           else {
             saveResults[recordToSave.recordID] = .failure(CKError(.zoneNotFound))
@@ -111,6 +127,7 @@ package final class MockCloudDatabase: CloudDatabase {
             guard let copy = recordToSave.copy() as? CKRecord
             else { fatalError("Could not copy CKRecord.") }
             copy._recordChangeTag = UUID().uuidString
+
             assets.withValue { assets in
               for key in copy.allKeys() {
                 guard let assetURL = (copy[key] as? CKAsset)?.fileURL
@@ -119,6 +136,8 @@ package final class MockCloudDatabase: CloudDatabase {
                   .load(assetURL)
               }
             }
+
+            // TODO: this should merge copy's values into storage but not sure how right now. 
             storage[recordToSave.recordID.zoneID]?[recordToSave.recordID] = copy
             saveResults[recordToSave.recordID] = .success(copy)
           }
@@ -268,7 +287,7 @@ extension MockCloudDatabase: CustomDumpReflectable {
   }
 }
 
-@available(macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0, *)
+@available(macOS 13, iOS 16, tvOS 16, watchOS 9, *)
 private func ckError(forAccountStatus accountStatus: CKAccountStatus) -> CKError {
   switch accountStatus {
   case .couldNotDetermine, .restricted, .noAccount:

Sources/SharingGRDBCore/CloudKit/Internal/MockSyncEngine.swift

@@ -33,7 +33,27 @@ package final class MockSyncEngine: SyncEngineProtocol {
   }
 
   package func fetchChanges(_ options: CKSyncEngine.FetchChangesOptions) async throws {
-    // TODO: do something here
+    let records: [CKRecord]
+    let zoneIDs: [CKRecordZone.ID]
+    switch options.scope {
+    case .all:
+      zoneIDs = Array(database.storage.keys)
+    case .allExcluding(let excludedZoneIDs):
+      zoneIDs = Array(Set(database.storage.keys).subtracting(excludedZoneIDs))
+    case .zoneIDs(let includedZoneIDs):
+      zoneIDs = includedZoneIDs
+    @unknown default:
+      fatalError()
+    }
+    records = zoneIDs.reduce(into: [CKRecord]()) { accum, zoneID in
+      accum += database.storage.withValue {
+        ($0[zoneID]?.values).map { Array($0) } ?? []
+      }
+    }
+    await delegate.handleEvent(
+      .fetchedRecordZoneChanges(modifications: records, deletions: []),
+      syncEngine: self
+    )
   }
 
   package func recordZoneChangeBatch(

Sources/SharingGRDBCore/CloudKit/SyncEngine.swift

@@ -478,22 +478,16 @@
       syncEngine?.state.add(pendingRecordZoneChanges: changes)
     }
 
-    // TODO: Possible to get test coverage on this?
     package func acceptShare(metadata: ShareMetadata) async throws {
-      guard let metadata = metadata.rawValue
-      else {
-        reportIssue("TODO")
-        return
-      }
       guard let rootRecordID = metadata.hierarchicalRootRecordID
       else {
-        reportIssue("TODO")
+        reportIssue("Attempting to share without root record information.")
         return
       }
       let container = type(of: container).createContainer(identifier: metadata.containerIdentifier)
       _ = try await container.accept(metadata)
       try await syncEngines.shared?.fetchChanges(
-        .init(
+        CKSyncEngine.FetchChangesOptions(
           scope: .zoneIDs([rootRecordID.zoneID]),
           operationGroup: nil
         )
@@ -1253,25 +1247,18 @@
     }
 
     private func cacheShare(_ share: CKShare) async throws {
-      // TODO: Instead of getting URL here we can make `shareMetadata(…)` take a share instead of a URL
-      guard let url = share.url
-      else { return }
-
       guard
-        let metadata = try? await container.shareMetadata(
-          for: url,
-          shouldFetchRootRecord: true
-        )
+        let metadata = try? await container.shareMetadata(for: share, shouldFetchRootRecord: false)
       else {
         // TODO: should we delete this record if it doesn't exist in the container?
         return
       }
 
-      guard let rootRecord = metadata.rootRecord
+      guard let rootRecordID = metadata.hierarchicalRootRecordID
       else { return }
       try await userDatabase.write { db in
         try SyncMetadata
-          .where { $0.recordName.eq(rootRecord.recordID.recordName) }
+          .where { $0.recordName.eq(rootRecordID.recordName) }
           .update { $0.share = share }
           .execute(db)
       }

Sources/SharingGRDBCore/CloudKit/SyncEngineProtocol.swift

@@ -27,23 +27,6 @@ package protocol SyncEngineProtocol<Database, State>: AnyObject, Sendable {
   ) async -> CKSyncEngine.RecordZoneChangeBatch?
 }
 
-@available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
-package struct ShareMetadata: Hashable {
-  package var containerIdentifier: String
-  package var hierarchicalRootRecordID: CKRecord.ID?
-  package var rawValue: CKShare.Metadata?
-  package init(rawValue: CKShare.Metadata) {
-    self.containerIdentifier = rawValue.containerIdentifier
-    self.hierarchicalRootRecordID = rawValue.hierarchicalRootRecordID
-    self.rawValue = rawValue
-  }
-  package init(containerIdentifier: String, hierarchicalRootRecordID: CKRecord.ID?) {
-    self.containerIdentifier = containerIdentifier
-    self.hierarchicalRootRecordID = hierarchicalRootRecordID
-    self.rawValue = nil
-  }
-}
-
 @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *)
 package protocol CKSyncEngineStateProtocol: Sendable {
   var pendingRecordZoneChanges: [CKSyncEngine.PendingRecordZoneChange] { get }

Tests/SharingGRDBTests/CloudKitTests/MockCloudDatabaseTests.swift

@@ -388,5 +388,27 @@ extension BaseCloudKitTests {
           """
       }
     }
+
+    @Test func saveShareWithoutRootRecord() async throws {
+      let record = CKRecord(recordType: "A", recordID: CKRecord.ID(recordName: "1"))
+      let share = CKShare(rootRecord: record, shareID: CKRecord.ID(recordName: "share"))
+      try withKnownIssue {
+        _ = try syncEngine.modifyRecords(scope: .private, saving: [share])
+      } matching: { issue in
+        issue.description == """
+          Issue recorded: An added share is being saved without its rootRecord being saved in the \
+          same operation.
+          """
+      }
+    }
+
+    @Test func saveShareAndRootThenSaveShareAlone() async throws {
+      let record = CKRecord(recordType: "A", recordID: CKRecord.ID(recordName: "1"))
+      let share = CKShare(rootRecord: record, shareID: CKRecord.ID(recordName: "share"))
+      _ = try syncEngine.modifyRecords(scope: .private, saving: [share, record])
+
+      let newShare = try syncEngine.private.database.record(for: CKRecord.ID(recordName: "share"))
+      _ = try syncEngine.modifyRecords(scope: .private, saving: [newShare])
+    }
   }
 }