Skip to content

Commit e0a0b04

Browse files
committed
fix: keep edits made during an iCloud sync and SQL file encodings on save, and move diff work off the main thread
1 parent 9fc22b3 commit e0a0b04

81 files changed

Lines changed: 3839 additions & 826 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
101101

102102
### Fixed
103103

104+
- Edits made while an iCloud sync was running reverted by that sync and never uploaded.
105+
- Saved queries unavailable until relaunch after their store failed to open once.
106+
- SQL files saved as UTF-8 whatever their encoding, and non-UTF-8 SQL files not opening from Finder or **File > Open File…**.
107+
- Unresponsive app while comparing large query plans.
108+
- Slow definition diff in Compare & Sync for large tables.
104109
- Autocomplete offering another schema's tables without their schema once that schema was completed or expanded.
105110
- Stale column and MongoDB field suggestions when a refresh ran while they were loading.
106111
- Tables in an expanded Oracle or Snowflake schema missing from Open Quickly until the next refresh.

Packages/TableProCore/Sources/TableProSyncTransport/CloudKitSyncEngine.swift

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,15 @@ public actor CloudKitSyncEngine {
8888
)
8989

9090
for plan in plans {
91-
outcome.merge(try await push(
92-
plan: plan,
93-
records: publishableRecords,
94-
deletions: publishableDeletions
95-
))
91+
do {
92+
outcome.merge(try await push(
93+
plan: plan,
94+
records: publishableRecords,
95+
deletions: publishableDeletions
96+
))
97+
} catch {
98+
throw SyncPushInterruption.after(outcome, failingWith: error)
99+
}
96100
}
97101

98102
let saved = outcome.savedRecords.count
@@ -129,7 +133,11 @@ public actor CloudKitSyncEngine {
129133

130134
var outcome = PushOutcome()
131135
for half in halves {
132-
outcome.merge(try await push(plan: half, records: records, deletions: deletions))
136+
do {
137+
outcome.merge(try await push(plan: half, records: records, deletions: deletions))
138+
} catch {
139+
throw SyncPushInterruption.after(outcome, failingWith: error)
140+
}
133141
}
134142
return outcome
135143
}

Packages/TableProCore/Sources/TableProSyncTransport/PushOutcome.swift

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ public struct PushOutcome: Sendable {
4242

4343
public var hasFailures: Bool { !failures.isEmpty }
4444

45+
public var isEmpty: Bool {
46+
savedRecords.isEmpty && deletedRecordIDs.isEmpty && failures.isEmpty
47+
}
48+
4549
public var conflicts: [CKRecord.ID: SyncItemFailure] {
4650
failures.filter(\.value.isConflict)
4751
}
@@ -90,3 +94,23 @@ public struct PushOutcome: Sendable {
9094
}
9195
}
9296
}
97+
98+
public struct SyncPushInterruption: Error, Sendable {
99+
public let completed: PushOutcome
100+
public let cause: any Error
101+
102+
public init(completed: PushOutcome, cause: any Error) {
103+
self.completed = completed
104+
self.cause = cause
105+
}
106+
107+
public static func after(_ completed: PushOutcome, failingWith error: any Error) -> any Error {
108+
if let interruption = error as? SyncPushInterruption {
109+
var merged = completed
110+
merged.merge(interruption.completed)
111+
return SyncPushInterruption(completed: merged, cause: interruption.cause)
112+
}
113+
guard !completed.isEmpty else { return error }
114+
return SyncPushInterruption(completed: completed, cause: error)
115+
}
116+
}

Packages/TableProCore/Sources/TableProSyncTransport/SyncError.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ public enum SyncError: Error, LocalizedError, Equatable, Sendable {
5050
return syncError
5151
}
5252

53+
if let interruption = error as? SyncPushInterruption {
54+
return from(interruption.cause)
55+
}
56+
5357
if let ckError = error as? CKError {
5458
switch ckError.code {
5559
case .networkUnavailable, .networkFailure:

Packages/TableProCore/Tests/TableProSyncTests/PushOutcomeTests.swift

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,4 +144,59 @@ struct PushOutcomeTests {
144144
#expect(outcome.failures[rejected] != nil)
145145
#expect(!outcome.didDelete(rejected))
146146
}
147+
148+
@Test("A push that stopped after saving some records carries those saves with the error")
149+
func interruptionKeepsWhatWasSaved() throws {
150+
var completed = PushOutcome()
151+
let saved = makeRecord("Connection_Saved")
152+
completed.recordSave(saved)
153+
154+
let error = SyncPushInterruption.after(completed, failingWith: CKError(.networkFailure))
155+
156+
let interruption = try #require(error as? SyncPushInterruption)
157+
#expect(interruption.completed.didSave(saved.recordID))
158+
#expect((interruption.cause as? CKError)?.code == .networkFailure)
159+
}
160+
161+
@Test("A push that stopped before saving anything throws its own error unchanged")
162+
func interruptionWithoutProgressIsTheRawError() {
163+
let error = SyncPushInterruption.after(PushOutcome(), failingWith: CKError(.networkFailure))
164+
165+
#expect(!(error is SyncPushInterruption))
166+
#expect((error as? CKError)?.code == .networkFailure)
167+
}
168+
169+
@Test("An interruption from a later batch keeps the saves of the batches before it")
170+
func nestedInterruptionsMerge() throws {
171+
var earlier = PushOutcome()
172+
let first = makeRecord("Connection_First")
173+
earlier.recordSave(first)
174+
var later = PushOutcome()
175+
let second = makeRecord("Connection_Second")
176+
later.recordSave(second)
177+
let inner = SyncPushInterruption(completed: later, cause: CKError(.networkFailure))
178+
179+
let error = SyncPushInterruption.after(earlier, failingWith: inner)
180+
181+
let interruption = try #require(error as? SyncPushInterruption)
182+
#expect(interruption.completed.didSave(first.recordID))
183+
#expect(interruption.completed.didSave(second.recordID))
184+
}
185+
186+
@Test("An interrupted push reports the error that stopped it")
187+
func interruptionMapsToItsCause() {
188+
let interruption = SyncPushInterruption(completed: PushOutcome(), cause: CKError(.networkFailure))
189+
190+
#expect(SyncError.from(interruption) == .networkUnavailable)
191+
}
192+
193+
@Test("An outcome with nothing saved, deleted or rejected is empty")
194+
func emptyOutcome() {
195+
var outcome = PushOutcome()
196+
#expect(outcome.isEmpty)
197+
198+
outcome.recordDeletion(recordID("Connection_Gone"))
199+
200+
#expect(!outcome.isEmpty)
201+
}
147202
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
//
2+
// StructureDefinitionDiffPresentation.swift
3+
// TablePro
4+
//
5+
6+
import Foundation
7+
8+
internal struct StructureDefinitionDiffInput: Equatable, Sendable {
9+
let sourceLines: [String]
10+
let targetLines: [String]
11+
}
12+
13+
internal struct StructureDefinitionDiffPresentation: Equatable, Sendable {
14+
let input: StructureDefinitionDiffInput
15+
let pairs: [DiffPair]
16+
17+
init(input: StructureDefinitionDiffInput) {
18+
self.input = input
19+
pairs = DiffComputer.computeSplit(before: input.targetLines, after: input.sourceLines)
20+
}
21+
22+
@concurrent
23+
static func load(_ input: StructureDefinitionDiffInput) async -> StructureDefinitionDiffPresentation {
24+
StructureDefinitionDiffPresentation(input: input)
25+
}
26+
27+
func isCurrent(for input: StructureDefinitionDiffInput) -> Bool {
28+
self.input == input
29+
}
30+
}

TablePro/Core/Services/Infrastructure/EditorTabOpener.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ internal enum EditorTabOpener {
7070
databaseName: payload.databaseName ?? browseDatabaseName,
7171
sourceFileURL: payload.sourceFileURL,
7272
sourceFileStamp: payload.sourceFileStamp,
73+
sourceFileEncoding: payload.sourceFileEncoding,
7374
claimFocus: true
7475
)
7576
case .createTable:

TablePro/Core/Services/Infrastructure/SQLFileService.swift

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,26 +20,21 @@ enum SQLFileService {
2020
return types.isEmpty ? [.plainText] : Array(types)
2121
}
2222

23-
/// Reads a SQL file from disk.
24-
static func readFile(url: URL) async throws -> String {
23+
static func writeFile(content: String, to url: URL, encoding: FileTextEncoding) async throws {
2524
try await Task.detached {
26-
try String(contentsOf: url, encoding: .utf8)
25+
try FileTextWriter.write(content, to: url, as: encoding)
2726
}.value
2827
}
2928

30-
/// Writes content to a SQL file atomically.
31-
static func writeFile(content: String, to url: URL) async throws {
32-
try await Task.detached {
33-
guard let data = content.data(using: .utf8) else {
34-
throw CocoaError(.fileWriteInapplicableStringEncoding)
35-
}
36-
try data.write(to: url, options: .atomic)
29+
static func encodingOnDisk(of url: URL) async -> FileTextEncoding? {
30+
await Task.detached {
31+
FileTextLoader.load(url)?.textEncoding
3732
}.value
3833
}
3934

4035
static func writeData(_ data: Data, to url: URL) async throws {
4136
try await Task.detached {
42-
try data.write(to: url, options: .atomic)
37+
try FileTextWriter.replaceContents(of: url, with: data, attribute: TextEncodingAttribute.read(from: url))
4338
}.value
4439
}
4540

TablePro/Core/Services/Infrastructure/TabRouter.swift

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -488,29 +488,34 @@ internal final class TabRouter {
488488
}
489489

490490
if let session = DatabaseManager.shared.lastActiveSession {
491-
let read = await Task.detached(priority: .userInitiated) { () -> (content: String, stamp: FileStamp?)? in
492-
let stamp = FileStamp.read(url)
493-
guard let content = try? String(contentsOf: url, encoding: .utf8) else { return nil }
494-
return (content, stamp)
495-
}.value
496-
guard let read else {
497-
Self.logger.error("Failed to read SQL file: \(url.lastPathComponent, privacy: .private(mask: .hash))")
498-
return
499-
}
500-
let payload = EditorTabPayload(
501-
connectionId: session.connection.id,
502-
tabType: .query,
503-
initialQuery: read.content,
504-
sourceFileURL: url,
505-
sourceFileStamp: read.stamp
506-
)
491+
let payload = try await Self.sqlFileTabPayload(for: url, connectionId: session.connection.id)
507492
WindowManager.shared.openTab(payload: payload)
508493
AppActivationPolicyController.shared.activate(ignoringOtherApps: true)
509494
} else {
510495
WelcomeRouter.shared.enqueueSQLFile(url)
511496
}
512497
}
513498

499+
internal static func sqlFileTabPayload(for url: URL, connectionId: UUID) async throws -> EditorTabPayload {
500+
let read: FileTextLoader.LoadedText
501+
do {
502+
read = try await Task.detached(priority: .userInitiated) {
503+
try FileTextLoader.read(url)
504+
}.value
505+
} catch {
506+
logger.error("Failed to read SQL file: \(url.lastPathComponent, privacy: .private(mask: .hash))")
507+
throw error
508+
}
509+
return EditorTabPayload(
510+
connectionId: connectionId,
511+
tabType: .query,
512+
initialQuery: read.content,
513+
sourceFileURL: url,
514+
sourceFileStamp: read.stamp,
515+
sourceFileEncoding: read.textEncoding
516+
)
517+
}
518+
514519
// MARK: - Helpers
515520

516521
internal func bringConnectionWindowToFront(_ connectionId: UUID) {

TablePro/Core/Services/SQL/LinkedSQLFavoriteWriter.swift

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ internal enum LinkedSQLFavoriteWriter {
1313

1414
enum WriteError: Error {
1515
case readFailed
16-
case encodingMismatch(String.Encoding)
16+
case encodingMismatch(FileTextEncoding)
1717
case writeFailed
1818
}
1919

@@ -33,12 +33,10 @@ internal enum LinkedSQLFavoriteWriter {
3333

3434
let newContent = rewrite(loaded.content, with: metadata)
3535
do {
36-
try newContent.write(to: url, atomically: true, encoding: loaded.encoding)
37-
} catch let error as NSError where
38-
error.domain == NSCocoaErrorDomain &&
39-
error.code == NSFileWriteInapplicableStringEncodingError {
40-
Self.logger.error("Encoding \(loaded.encoding.rawValue) cannot represent edited content at \(url.path, privacy: .private(mask: .hash))")
41-
throw WriteError.encodingMismatch(loaded.encoding)
36+
try FileTextWriter.write(newContent, to: url, as: loaded.textEncoding)
37+
} catch FileTextWriter.WriteError.unrepresentable(let encoding) {
38+
Self.logger.error("Encoding \(encoding.encoding.rawValue) cannot represent edited content at \(url.path, privacy: .private(mask: .hash))")
39+
throw WriteError.encodingMismatch(encoding)
4240
} catch {
4341
Self.logger.error("Failed to write metadata to \(url.path, privacy: .private(mask: .hash)): \(error.publicLogShape, privacy: .public)")
4442
throw WriteError.writeFailed

0 commit comments

Comments
 (0)