From 9211a62eab9d72cd03ea9b081b068fa0f24a5126 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 7 May 2026 12:25:08 +0000 Subject: [PATCH 1/2] Track create vs update in modifyRecords() responses (#194) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CloudKit's /records/modify endpoint does not indicate whether each operation produced a newly created record or an update, which makes detailed sync summaries ("Created: 5, Updated: 20, Failed: 0") impossible to derive from the response alone. This adds the pre-fetch + classify pattern to MistKit core so consumers no longer have to reimplement it themselves (per BushelCloud). Adds three additive types/methods, all opt-in: - OperationClassification — partitions proposed record names into creates vs updates by comparing against existing names; ships with initializers for [String], [RecordOperation], and direct construction. - BatchSyncResult — categorized response with created/updated/failed/ unclassified RecordInfo arrays plus count properties. - CloudKitService.fetchExistingRecordNames(recordType:) and modifyRecords(_:classification:atomic:) overload that pairs the pre-fetch with a tracked modify call. The existing modifyRecords(_:atomic:) signature is unchanged. --- Sources/MistKit/Service/BatchSyncResult.swift | 142 ++++++++++++++++ .../CloudKitService+Classification.swift | 115 +++++++++++++ .../Service/OperationClassification.swift | 120 ++++++++++++++ .../PublicTypes/BatchSyncResultTests.swift | 155 ++++++++++++++++++ .../OperationClassificationTests.swift | 137 ++++++++++++++++ 5 files changed, 669 insertions(+) create mode 100644 Sources/MistKit/Service/BatchSyncResult.swift create mode 100644 Sources/MistKit/Service/CloudKitService+Classification.swift create mode 100644 Sources/MistKit/Service/OperationClassification.swift create mode 100644 Tests/MistKitTests/PublicTypes/BatchSyncResultTests.swift create mode 100644 Tests/MistKitTests/PublicTypes/OperationClassificationTests.swift diff --git a/Sources/MistKit/Service/BatchSyncResult.swift b/Sources/MistKit/Service/BatchSyncResult.swift new file mode 100644 index 00000000..30728a6f --- /dev/null +++ b/Sources/MistKit/Service/BatchSyncResult.swift @@ -0,0 +1,142 @@ +// +// BatchSyncResult.swift +// MistKit +// +// Created by Leo Dion. +// Copyright © 2026 BrightDigit. +// +// Permission is hereby granted, free of charge, to any person +// obtaining a copy of this software and associated documentation +// files (the "Software"), to deal in the Software without +// restriction, including without limitation the rights to use, +// copy, modify, merge, publish, distribute, sublicense, and/or +// sell copies of the Software, and to permit persons to whom the +// Software is furnished to do so, subject to the following +// conditions: +// +// The above copyright notice and this permission notice shall be +// included in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES +// OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT +// HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, +// WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +// OTHER DEALINGS IN THE SOFTWARE. +// + +import Foundation + +/// Categorized result of a tracked `modifyRecords(_:classification:atomic:)` call. +/// +/// Returned by `CloudKitService.modifyRecords(_:classification:atomic:)`, +/// this struct partitions the records returned by CloudKit into four groups +/// based on the supplied `OperationClassification`: +/// +/// - `created`: results whose record name was classified as a create +/// - `updated`: results whose record name was classified as an update +/// - `failed`: results that came back as errors (`RecordInfo.isError == true`) +/// - `unclassified`: successful results whose record name was in neither +/// the creates nor updates sets — for example, anonymous creates where +/// CloudKit assigned the record name server-side, or records whose name +/// was not included in the classification +/// +/// Use the `*Count` properties to drive sync summaries and audit logs. +public struct BatchSyncResult: Sendable { + /// Records classified as newly created. + public let created: [RecordInfo] + + /// Records classified as updates to existing records. + public let updated: [RecordInfo] + + /// Records that came back as errors. + public let failed: [RecordInfo] + + /// Successful records that could not be classified as either a create or update. + /// + /// Typically contains anonymous creates where CloudKit assigned the record + /// name server-side, since their names won't appear in either set of the + /// supplied `OperationClassification`. + public let unclassified: [RecordInfo] + + /// Number of records classified as created. + public var createdCount: Int { created.count } + + /// Number of records classified as updated. + public var updatedCount: Int { updated.count } + + /// Number of records that returned an error. + public var failedCount: Int { failed.count } + + /// Number of successful records that could not be classified. + public var unclassifiedCount: Int { unclassified.count } + + /// Total number of records returned by CloudKit, across all categories. + public var totalCount: Int { + created.count + updated.count + failed.count + unclassified.count + } + + /// Number of records that completed successfully (created + updated + unclassified). + public var succeededCount: Int { + created.count + updated.count + unclassified.count + } + + /// Build a `BatchSyncResult` directly from category arrays. + /// + /// Prefer `init(records:classification:)` in production code; this + /// initializer is intended for tests and manual construction. + public init( + created: [RecordInfo], + updated: [RecordInfo], + failed: [RecordInfo], + unclassified: [RecordInfo] = [] + ) { + self.created = created + self.updated = updated + self.failed = failed + self.unclassified = unclassified + } + + /// Partition a flat array of `RecordInfo` results into a `BatchSyncResult` + /// using a pre-computed classification. + /// + /// Each record is sorted as follows: + /// 1. If `record.isError` is `true`, it is added to `failed`. + /// 2. Else if `record.recordName` is in `classification.creates`, it is added + /// to `created`. + /// 3. Else if `record.recordName` is in `classification.updates`, it is added + /// to `updated`. + /// 4. Otherwise it is added to `unclassified`. + /// + /// - Parameters: + /// - records: The records returned by `modifyRecords`. + /// - classification: The classification used to partition the records. + public init( + records: [RecordInfo], + classification: OperationClassification + ) { + var created: [RecordInfo] = [] + var updated: [RecordInfo] = [] + var failed: [RecordInfo] = [] + var unclassified: [RecordInfo] = [] + + for record in records { + if record.isError { + failed.append(record) + } else if classification.creates.contains(record.recordName) { + created.append(record) + } else if classification.updates.contains(record.recordName) { + updated.append(record) + } else { + unclassified.append(record) + } + } + + self.created = created + self.updated = updated + self.failed = failed + self.unclassified = unclassified + } +} diff --git a/Sources/MistKit/Service/CloudKitService+Classification.swift b/Sources/MistKit/Service/CloudKitService+Classification.swift new file mode 100644 index 00000000..4cd8be7b --- /dev/null +++ b/Sources/MistKit/Service/CloudKitService+Classification.swift @@ -0,0 +1,115 @@ +// +// CloudKitService+Classification.swift +// MistKit +// +// Created by Leo Dion. +// Copyright © 2026 BrightDigit. +// +// Permission is hereby granted, free of charge, to any person +// obtaining a copy of this software and associated documentation +// files (the "Software"), to deal in the Software without +// restriction, including without limitation the rights to use, +// copy, modify, merge, publish, distribute, sublicense, and/or +// sell copies of the Software, and to permit persons to whom the +// Software is furnished to do so, subject to the following +// conditions: +// +// The above copyright notice and this permission notice shall be +// included in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES +// OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT +// HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, +// WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +// OTHER DEALINGS IN THE SOFTWARE. +// + +import Foundation + +/// Helpers for tracking creates vs updates in `modifyRecords` responses. +/// +/// CloudKit's `/records/modify` endpoint does not include any indicator of +/// whether each operation produced a newly created record or updated an +/// existing one. The pattern in this extension implements the documented +/// pre-fetch + classify workaround: +/// +/// 1. Call `fetchExistingRecordNames(recordType:)` to discover which records +/// already exist. +/// 2. Build an `OperationClassification` from the proposed operations and the +/// existing names. +/// 3. Call `modifyRecords(_:classification:atomic:)` to perform the modify and +/// receive a `BatchSyncResult` with creates/updates/failures already +/// partitioned. +@available(macOS 11.0, iOS 14.0, tvOS 14.0, watchOS 7.0, *) +extension CloudKitService { + /// Fetch the set of record names that already exist for a record type. + /// + /// Used as the first step of the pre-fetch + classify pattern for tracking + /// creates vs updates in batch modify operations. Internally this calls + /// `queryRecords(recordType:)` and projects the results down to a + /// `Set` of record names. + /// + /// - Important: This issues a single `queryRecords` call with the maximum + /// per-request limit of 200 records. For record types with more than 200 + /// records, paginate at the call site or use a custom query. + /// + /// - Parameter recordType: The CloudKit record type to scan. + /// - Returns: Set of existing record names. + /// - Throws: `CloudKitError` if the underlying query fails. + public func fetchExistingRecordNames( + recordType: String + ) async throws(CloudKitError) -> Set { + // Pass `limit: 200` explicitly so overload resolution picks the + // typed-throws variant of `queryRecords` rather than the 1-param + // RecordManaging-conforming overload (which has untyped throws). + let records = try await queryRecords( + recordType: recordType, + limit: 200 + ) + return Set(records.map(\.recordName)) + } + + /// Modify CloudKit records and partition the response into creates, + /// updates, failures, and unclassified records. + /// + /// This overload calls `modifyRecords(_:atomic:)` internally and then + /// uses the supplied `OperationClassification` to attribute each returned + /// `RecordInfo` to a category. It does not issue any extra CloudKit + /// requests beyond the modify itself. + /// + /// ## Example + /// ```swift + /// let existing = try await service.fetchExistingRecordNames(recordType: "Article") + /// let classification = OperationClassification( + /// operations: operations, + /// existingRecordNames: existing + /// ) + /// let result = try await service.modifyRecords( + /// operations, + /// classification: classification + /// ) + /// print("Created: \(result.createdCount)") + /// print("Updated: \(result.updatedCount)") + /// print("Failed: \(result.failedCount)") + /// ``` + /// + /// - Parameters: + /// - operations: Record operations to perform. + /// - classification: Pre-computed classification of operations as creates + /// vs updates, typically from `fetchExistingRecordNames(recordType:)`. + /// - atomic: When `true`, the entire batch fails if any single operation + /// fails (default: `false`). + /// - Returns: A `BatchSyncResult` partitioning the response. + /// - Throws: `CloudKitError` if the modify request fails. + public func modifyRecords( + _ operations: [RecordOperation], + classification: OperationClassification, + atomic: Bool = false + ) async throws(CloudKitError) -> BatchSyncResult { + let records = try await modifyRecords(operations, atomic: atomic) + return BatchSyncResult(records: records, classification: classification) + } +} diff --git a/Sources/MistKit/Service/OperationClassification.swift b/Sources/MistKit/Service/OperationClassification.swift new file mode 100644 index 00000000..30080395 --- /dev/null +++ b/Sources/MistKit/Service/OperationClassification.swift @@ -0,0 +1,120 @@ +// +// OperationClassification.swift +// MistKit +// +// Created by Leo Dion. +// Copyright © 2026 BrightDigit. +// +// Permission is hereby granted, free of charge, to any person +// obtaining a copy of this software and associated documentation +// files (the "Software"), to deal in the Software without +// restriction, including without limitation the rights to use, +// copy, modify, merge, publish, distribute, sublicense, and/or +// sell copies of the Software, and to permit persons to whom the +// Software is furnished to do so, subject to the following +// conditions: +// +// The above copyright notice and this permission notice shall be +// included in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES +// OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT +// HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, +// WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +// OTHER DEALINGS IN THE SOFTWARE. +// + +import Foundation + +/// Classifies CloudKit record operations as creates or updates. +/// +/// CloudKit's `/records/modify` endpoint does not indicate in its response +/// whether each operation resulted in a newly-created record or an update to +/// an existing one. The proven workaround is to query the existing record +/// names for a record type before issuing the modify, then partition each +/// proposed operation by whether its record name was already present. +/// +/// `OperationClassification` captures the result of that partitioning so that +/// `CloudKitService.modifyRecords(_:classification:atomic:)` can attribute +/// each returned `RecordInfo` to a create or an update. +/// +/// ## Example +/// ```swift +/// let existing = try await service.fetchExistingRecordNames(recordType: "Article") +/// let classification = OperationClassification( +/// operations: operations, +/// existingRecordNames: existing +/// ) +/// let result = try await service.modifyRecords( +/// operations, +/// classification: classification +/// ) +/// print("Created: \(result.createdCount), Updated: \(result.updatedCount)") +/// ``` +public struct OperationClassification: Sendable, Equatable { + /// Record names that are expected to be created (not present in CloudKit). + public let creates: Set + + /// Record names that are expected to be updated (already present in CloudKit). + public let updates: Set + + /// Build a classification by comparing proposed record names against existing ones. + /// + /// Operations whose record name is in `existingRecordNames` are classified as + /// updates; the rest are classified as creates. Duplicate names in + /// `proposedRecordNames` are folded into the same set entry. + /// + /// - Parameters: + /// - proposedRecordNames: Record names that will be sent to CloudKit. + /// - existingRecordNames: Record names already present in CloudKit + /// (typically obtained via `fetchExistingRecordNames(recordType:)`). + public init( + proposedRecordNames: [String], + existingRecordNames: Set + ) { + var creates = Set() + var updates = Set() + + for recordName in proposedRecordNames { + if existingRecordNames.contains(recordName) { + updates.insert(recordName) + } else { + creates.insert(recordName) + } + } + + self.creates = creates + self.updates = updates + } + + /// Build a classification directly from a sequence of `RecordOperation` values. + /// + /// Operations without a `recordName` (anonymous creates where CloudKit will + /// assign the name) are skipped — they cannot be matched against existing + /// names by definition. + /// + /// - Parameters: + /// - operations: The record operations that will be sent to CloudKit. + /// - existingRecordNames: Record names already present in CloudKit. + public init( + operations: [RecordOperation], + existingRecordNames: Set + ) { + let proposedNames = operations.compactMap(\.recordName) + self.init( + proposedRecordNames: proposedNames, + existingRecordNames: existingRecordNames + ) + } + + /// Direct initializer for tests and manual construction. + /// + /// Prefer the comparison-based initializers in production code. + public init(creates: Set, updates: Set) { + self.creates = creates + self.updates = updates + } +} diff --git a/Tests/MistKitTests/PublicTypes/BatchSyncResultTests.swift b/Tests/MistKitTests/PublicTypes/BatchSyncResultTests.swift new file mode 100644 index 00000000..f853da97 --- /dev/null +++ b/Tests/MistKitTests/PublicTypes/BatchSyncResultTests.swift @@ -0,0 +1,155 @@ +// +// BatchSyncResultTests.swift +// MistKit +// +// Created by Leo Dion. +// Copyright © 2026 BrightDigit. +// +// Permission is hereby granted, free of charge, to any person +// obtaining a copy of this software and associated documentation +// files (the "Software"), to deal in the Software without +// restriction, including without limitation the rights to use, +// copy, modify, merge, publish, distribute, sublicense, and/or +// sell copies of the Software, and to permit persons to whom the +// Software is furnished to do so, subject to the following +// conditions: +// +// The above copyright notice and this permission notice shall be +// included in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES +// OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT +// HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, +// WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +// OTHER DEALINGS IN THE SOFTWARE. +// + +import Foundation +import Testing + +@testable import MistKit + +@Suite("BatchSyncResult") +internal struct BatchSyncResultTests { + // MARK: - Helpers + + internal static func makeRecord( + name: String, + type: String = "Article" + ) -> RecordInfo { + RecordInfo( + recordName: name, + recordType: type, + recordChangeTag: nil, + fields: [:] + ) + } + + /// CloudKit error responses are surfaced as RecordInfo with recordType + /// "Unknown"; the public init does not allow forging that, so use the + /// minimum of one error record by setting recordType to "Unknown". + internal static func makeErrorRecord(name: String = "Unknown") -> RecordInfo { + RecordInfo( + recordName: name, + recordType: "Unknown", + recordChangeTag: nil, + fields: [:] + ) + } + + // MARK: - Tests + + @Test("partitions records into created, updated, failed, and unclassified") + internal func partitionsRecordsIntoAllFourCategories() { + let classification = OperationClassification( + creates: ["new-1", "new-2"], + updates: ["existing-1"] + ) + let records: [RecordInfo] = [ + Self.makeRecord(name: "new-1"), + Self.makeRecord(name: "existing-1"), + Self.makeRecord(name: "new-2"), + Self.makeRecord(name: "server-assigned-name"), + Self.makeErrorRecord(), + ] + + let result = BatchSyncResult(records: records, classification: classification) + + #expect(result.createdCount == 2) + #expect(result.updatedCount == 1) + #expect(result.failedCount == 1) + #expect(result.unclassifiedCount == 1) + #expect(result.created.map(\.recordName).sorted() == ["new-1", "new-2"]) + #expect(result.updated.map(\.recordName) == ["existing-1"]) + #expect(result.unclassified.map(\.recordName) == ["server-assigned-name"]) + } + + @Test("counts add up across all categories") + internal func countsAddUpAcrossCategories() { + let classification = OperationClassification( + creates: ["a"], + updates: ["b"] + ) + let records: [RecordInfo] = [ + Self.makeRecord(name: "a"), + Self.makeRecord(name: "b"), + Self.makeRecord(name: "c"), + Self.makeErrorRecord(), + ] + + let result = BatchSyncResult(records: records, classification: classification) + + #expect(result.totalCount == 4) + #expect(result.succeededCount == 3) + #expect(result.totalCount == result.createdCount + result.updatedCount + + result.failedCount + result.unclassifiedCount) + } + + @Test("treats error records as failures regardless of classification") + internal func treatsErrorRecordsAsFailures() { + // Even if the recordName matches the creates set, the error sentinel + // (recordType == "Unknown") routes the record to the failed bucket. + let classification = OperationClassification( + creates: ["Unknown"], + updates: [] + ) + let records: [RecordInfo] = [Self.makeErrorRecord()] + + let result = BatchSyncResult(records: records, classification: classification) + + #expect(result.failedCount == 1) + #expect(result.createdCount == 0) + } + + @Test("returns empty buckets for empty inputs") + internal func returnsEmptyBucketsForEmptyInputs() { + let classification = OperationClassification(creates: [], updates: []) + let result = BatchSyncResult(records: [], classification: classification) + + #expect(result.totalCount == 0) + #expect(result.succeededCount == 0) + #expect(result.created.isEmpty) + #expect(result.updated.isEmpty) + #expect(result.failed.isEmpty) + #expect(result.unclassified.isEmpty) + } + + @Test("manual init exposes the supplied arrays directly") + internal func manualInitExposesSuppliedArrays() { + let result = BatchSyncResult( + created: [Self.makeRecord(name: "a")], + updated: [Self.makeRecord(name: "b"), Self.makeRecord(name: "c")], + failed: [Self.makeErrorRecord()] + ) + + #expect(result.createdCount == 1) + #expect(result.updatedCount == 2) + #expect(result.failedCount == 1) + #expect(result.unclassifiedCount == 0) + #expect(result.totalCount == 4) + #expect(result.succeededCount == 3) + } +} diff --git a/Tests/MistKitTests/PublicTypes/OperationClassificationTests.swift b/Tests/MistKitTests/PublicTypes/OperationClassificationTests.swift new file mode 100644 index 00000000..4d0e6aec --- /dev/null +++ b/Tests/MistKitTests/PublicTypes/OperationClassificationTests.swift @@ -0,0 +1,137 @@ +// +// OperationClassificationTests.swift +// MistKit +// +// Created by Leo Dion. +// Copyright © 2026 BrightDigit. +// +// Permission is hereby granted, free of charge, to any person +// obtaining a copy of this software and associated documentation +// files (the "Software"), to deal in the Software without +// restriction, including without limitation the rights to use, +// copy, modify, merge, publish, distribute, sublicense, and/or +// sell copies of the Software, and to permit persons to whom the +// Software is furnished to do so, subject to the following +// conditions: +// +// The above copyright notice and this permission notice shall be +// included in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES +// OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +// NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT +// HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, +// WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +// OTHER DEALINGS IN THE SOFTWARE. +// + +import Foundation +import Testing + +@testable import MistKit + +@Suite("OperationClassification") +internal struct OperationClassificationTests { + @Test("partitions proposed names against existing names") + internal func partitionsProposedNamesAgainstExistingNames() { + let classification = OperationClassification( + proposedRecordNames: ["a", "b", "c", "d"], + existingRecordNames: ["b", "d", "e"] + ) + + #expect(classification.creates == ["a", "c"]) + #expect(classification.updates == ["b", "d"]) + } + + @Test("classifies all as creates when nothing exists") + internal func classifiesAllAsCreatesWhenNothingExists() { + let classification = OperationClassification( + proposedRecordNames: ["a", "b", "c"], + existingRecordNames: [] + ) + + #expect(classification.creates == ["a", "b", "c"]) + #expect(classification.updates.isEmpty) + } + + @Test("classifies all as updates when all already exist") + internal func classifiesAllAsUpdatesWhenAllAlreadyExist() { + let classification = OperationClassification( + proposedRecordNames: ["a", "b"], + existingRecordNames: ["a", "b", "c"] + ) + + #expect(classification.creates.isEmpty) + #expect(classification.updates == ["a", "b"]) + } + + @Test("collapses duplicate proposed names into a single set entry") + internal func collapsesDuplicateProposedNames() { + let classification = OperationClassification( + proposedRecordNames: ["a", "a", "b", "b", "b"], + existingRecordNames: ["b"] + ) + + #expect(classification.creates == ["a"]) + #expect(classification.updates == ["b"]) + } + + @Test("returns empty sets for empty inputs") + internal func returnsEmptySetsForEmptyInputs() { + let classification = OperationClassification( + proposedRecordNames: [], + existingRecordNames: [] + ) + + #expect(classification.creates.isEmpty) + #expect(classification.updates.isEmpty) + } + + @Test("classifies operations directly via convenience initializer") + internal func classifiesOperationsDirectly() { + let operations: [RecordOperation] = [ + .create(recordType: "Article", recordName: "new-1", fields: [:]), + .update( + recordType: "Article", + recordName: "existing-1", + fields: [:], + recordChangeTag: nil + ), + .create(recordType: "Article", recordName: "new-2", fields: [:]), + ] + + let classification = OperationClassification( + operations: operations, + existingRecordNames: ["existing-1"] + ) + + #expect(classification.creates == ["new-1", "new-2"]) + #expect(classification.updates == ["existing-1"]) + } + + @Test("skips anonymous operations that have no record name") + internal func skipsAnonymousOperations() { + let operations: [RecordOperation] = [ + .create(recordType: "Article", recordName: nil, fields: [:]), + .create(recordType: "Article", recordName: "named", fields: [:]), + ] + + let classification = OperationClassification( + operations: operations, + existingRecordNames: [] + ) + + #expect(classification.creates == ["named"]) + #expect(classification.updates.isEmpty) + } + + @Test("equates classifications with the same contents") + internal func equatesClassificationsWithSameContents() { + let lhs = OperationClassification(creates: ["a"], updates: ["b"]) + let rhs = OperationClassification(creates: ["a"], updates: ["b"]) + + #expect(lhs == rhs) + } +} From 73cd1beb78b5f6cbabe627c80772b3c188ea5f31 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 7 May 2026 12:32:50 +0000 Subject: [PATCH 2/2] Address PR review feedback - Add optional `limit:` argument to `fetchExistingRecordNames(recordType:)` so callers can scope the prefetch when they know they need fewer than 200 names; defaults to CloudKit's per-request maximum. - Introduce `CloudKitService.maxRecordsPerRequest` (= 200) and use it instead of the bare literal so the magic number lives in one place. - Build the error `RecordInfo` in `BatchSyncResultTests` via the same `RecordInfo(from: Components.Schemas.RecordResponse())` decoding path used by `RecordInfoTests`, instead of hardcoding the "Unknown" sentinel string in the test. The "errors win over classification" test now reads the record's actual name dynamically. --- .../CloudKitService+Classification.swift | 24 ++++++++------- Sources/MistKit/Service/CloudKitService.swift | 3 ++ .../PublicTypes/BatchSyncResultTests.swift | 29 +++++++++---------- 3 files changed, 31 insertions(+), 25 deletions(-) diff --git a/Sources/MistKit/Service/CloudKitService+Classification.swift b/Sources/MistKit/Service/CloudKitService+Classification.swift index 4cd8be7b..6a532bfc 100644 --- a/Sources/MistKit/Service/CloudKitService+Classification.swift +++ b/Sources/MistKit/Service/CloudKitService+Classification.swift @@ -49,25 +49,29 @@ extension CloudKitService { /// /// Used as the first step of the pre-fetch + classify pattern for tracking /// creates vs updates in batch modify operations. Internally this calls - /// `queryRecords(recordType:)` and projects the results down to a + /// `queryRecords(recordType:limit:)` and projects the results down to a /// `Set` of record names. /// - /// - Important: This issues a single `queryRecords` call with the maximum - /// per-request limit of 200 records. For record types with more than 200 - /// records, paginate at the call site or use a custom query. + /// - Important: This issues a single `queryRecords` call. CloudKit caps a + /// single response at 200 records, so for larger record types you must + /// paginate at the call site or use a custom query. /// - /// - Parameter recordType: The CloudKit record type to scan. + /// - Parameters: + /// - recordType: The CloudKit record type to scan. + /// - limit: Optional maximum number of records to fetch (1-200). Defaults + /// to CloudKit's per-request maximum. /// - Returns: Set of existing record names. /// - Throws: `CloudKitError` if the underlying query fails. public func fetchExistingRecordNames( - recordType: String + recordType: String, + limit: Int? = nil ) async throws(CloudKitError) -> Set { - // Pass `limit: 200` explicitly so overload resolution picks the - // typed-throws variant of `queryRecords` rather than the 1-param - // RecordManaging-conforming overload (which has untyped throws). + // Pass `limit:` explicitly so overload resolution picks the typed-throws + // variant of `queryRecords` rather than the 1-param RecordManaging- + // conforming overload (which has untyped throws). let records = try await queryRecords( recordType: recordType, - limit: 200 + limit: limit ?? Self.maxRecordsPerRequest ) return Set(records.map(\.recordName)) } diff --git a/Sources/MistKit/Service/CloudKitService.swift b/Sources/MistKit/Service/CloudKitService.swift index 9ce2cc7a..af992603 100644 --- a/Sources/MistKit/Service/CloudKitService.swift +++ b/Sources/MistKit/Service/CloudKitService.swift @@ -41,6 +41,9 @@ import OpenAPIRuntime /// Service for interacting with CloudKit Web Services @available(macOS 11.0, iOS 14.0, tvOS 14.0, watchOS 7.0, *) public struct CloudKitService: Sendable { + /// CloudKit's maximum number of records returned per query/modify request. + internal static let maxRecordsPerRequest: Int = 200 + /// The CloudKit container identifier public let containerIdentifier: String /// The API token for authentication diff --git a/Tests/MistKitTests/PublicTypes/BatchSyncResultTests.swift b/Tests/MistKitTests/PublicTypes/BatchSyncResultTests.swift index f853da97..260a97bc 100644 --- a/Tests/MistKitTests/PublicTypes/BatchSyncResultTests.swift +++ b/Tests/MistKitTests/PublicTypes/BatchSyncResultTests.swift @@ -48,16 +48,12 @@ internal struct BatchSyncResultTests { ) } - /// CloudKit error responses are surfaced as RecordInfo with recordType - /// "Unknown"; the public init does not allow forging that, so use the - /// minimum of one error record by setting recordType to "Unknown". - internal static func makeErrorRecord(name: String = "Unknown") -> RecordInfo { - RecordInfo( - recordName: name, - recordType: "Unknown", - recordChangeTag: nil, - fields: [:] - ) + /// Builds an error `RecordInfo` via the same response-decoding path used in + /// production (`RecordInfo.init(from:)` with an empty `RecordResponse`), + /// rather than hardcoding the "Unknown" sentinel string in the test. + /// This matches the pattern in `RecordInfoTests.recordInfoWithUnknownRecord`. + internal static func makeErrorRecord() -> RecordInfo { + RecordInfo(from: Components.Schemas.RecordResponse()) } // MARK: - Tests @@ -110,15 +106,18 @@ internal struct BatchSyncResultTests { @Test("treats error records as failures regardless of classification") internal func treatsErrorRecordsAsFailures() { - // Even if the recordName matches the creates set, the error sentinel - // (recordType == "Unknown") routes the record to the failed bucket. + // Build a classification that *would* claim the error record as a create + // by reading its actual recordName, then verify the failure check wins. + let errorRecord = Self.makeErrorRecord() let classification = OperationClassification( - creates: ["Unknown"], + creates: [errorRecord.recordName], updates: [] ) - let records: [RecordInfo] = [Self.makeErrorRecord()] - let result = BatchSyncResult(records: records, classification: classification) + let result = BatchSyncResult( + records: [errorRecord], + classification: classification + ) #expect(result.failedCount == 1) #expect(result.createdCount == 0)