Skip to content

Commit f4a25eb

Browse files
committed
fix: address PR review — access control, validation, accessibility, thread safety
1 parent 58644e2 commit f4a25eb

16 files changed

Lines changed: 101 additions & 48 deletions

TablePro/ContentView.swift

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -355,14 +355,6 @@ struct ContentView: View {
355355
)
356356
}
357357

358-
private func sidebarTabBinding(for connectionId: UUID) -> Binding<SidebarTab> {
359-
let state = SharedSidebarState.forConnection(connectionId)
360-
return Binding(
361-
get: { state.selectedSidebarTab },
362-
set: { state.selectedSidebarTab = $0 }
363-
)
364-
}
365-
366358
private func sidebarSearchPrompt(for connectionId: UUID) -> String {
367359
let state = SharedSidebarState.forConnection(connectionId)
368360
switch state.selectedSidebarTab {

TablePro/Core/SSH/LibSSH2Tunnel.swift

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,7 @@ internal final class LibSSH2Tunnel: @unchecked Sendable {
3434
/// Keeps blocking work off the Swift cooperative thread pool.
3535
private static let relayQueue = DispatchQueue(
3636
label: "com.TablePro.ssh.relay",
37-
qos: .utility,
38-
attributes: .concurrent
37+
qos: .utility
3938
)
4039

4140
/// Callback invoked when the tunnel dies (keep-alive failure, etc.)
@@ -308,7 +307,10 @@ internal final class LibSSH2Tunnel: @unchecked Sendable {
308307
}
309308
}
310309

311-
relayTasks.withLock { $0.append(task) }
310+
relayTasks.withLock { tasks in
311+
tasks.removeAll { $0.isCancelled }
312+
tasks.append(task)
313+
}
312314
}
313315

314316
/// Blocking relay loop — must only be called on `relayQueue`, never the cooperative pool.

TablePro/Core/Storage/SQLFavoriteManager.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import Foundation
77
import os
88

99
/// Manages SQL favorites with notifications and sync tracking
10-
final class SQLFavoriteManager {
10+
internal final class SQLFavoriteManager {
1111
static let shared = SQLFavoriteManager()
1212
private static let logger = Logger(subsystem: "com.TablePro", category: "SQLFavoriteManager")
1313

TablePro/Core/Storage/SQLFavoriteStorage.swift

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import os
88
import SQLite3
99

1010
/// Thread-safe SQLite storage for SQL favorites with FTS5 full-text search
11-
final class SQLFavoriteStorage {
11+
internal final class SQLFavoriteStorage {
1212
static let shared = SQLFavoriteStorage()
1313
private static let logger = Logger(subsystem: "com.TablePro", category: "SQLFavoriteStorage")
1414

@@ -564,9 +564,17 @@ final class SQLFavoriteStorage {
564564
sqlite3_bind_null(moveFavStatement, 1)
565565
}
566566
sqlite3_bind_text(moveFavStatement, 2, idString, -1, SQLITE_TRANSIENT)
567-
sqlite3_step(moveFavStatement)
567+
let moveFavResult = sqlite3_step(moveFavStatement)
568+
sqlite3_finalize(moveFavStatement)
569+
if moveFavResult != SQLITE_DONE {
570+
if inTransaction { sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil) }
571+
return false
572+
}
573+
} else {
574+
sqlite3_finalize(moveFavStatement)
575+
if inTransaction { sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil) }
576+
return false
568577
}
569-
sqlite3_finalize(moveFavStatement)
570578

571579
// Move child subfolders to the parent folder
572580
let moveSubfoldersSQL = "UPDATE folders SET parent_id = ? WHERE parent_id = ?;"
@@ -578,9 +586,17 @@ final class SQLFavoriteStorage {
578586
sqlite3_bind_null(moveSubStatement, 1)
579587
}
580588
sqlite3_bind_text(moveSubStatement, 2, idString, -1, SQLITE_TRANSIENT)
581-
sqlite3_step(moveSubStatement)
589+
let moveSubResult = sqlite3_step(moveSubStatement)
590+
sqlite3_finalize(moveSubStatement)
591+
if moveSubResult != SQLITE_DONE {
592+
if inTransaction { sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil) }
593+
return false
594+
}
595+
} else {
596+
sqlite3_finalize(moveSubStatement)
597+
if inTransaction { sqlite3_exec(self.db, "ROLLBACK;", nil, nil, nil) }
598+
return false
582599
}
583-
sqlite3_finalize(moveSubStatement)
584600

585601
// Delete the folder
586602
let deleteSQL = "DELETE FROM folders WHERE id = ?;"

TablePro/Models/Query/SQLFavorite.swift

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import Foundation
77

88
/// A saved SQL query that can be quickly recalled and optionally expanded via keyword
9-
struct SQLFavorite: Identifiable, Codable, Hashable {
9+
internal struct SQLFavorite: Identifiable, Codable, Hashable {
1010
let id: UUID
1111
var name: String
1212
var query: String
@@ -25,17 +25,18 @@ struct SQLFavorite: Identifiable, Codable, Hashable {
2525
folderId: UUID? = nil,
2626
connectionId: UUID? = nil,
2727
sortOrder: Int = 0,
28-
createdAt: Date = Date(),
29-
updatedAt: Date = Date()
28+
createdAt: Date? = nil,
29+
updatedAt: Date? = nil
3030
) {
31+
let now = Date()
3132
self.id = id
3233
self.name = name
3334
self.query = query
3435
self.keyword = keyword
3536
self.folderId = folderId
3637
self.connectionId = connectionId
3738
self.sortOrder = sortOrder
38-
self.createdAt = createdAt
39-
self.updatedAt = updatedAt
39+
self.createdAt = createdAt ?? now
40+
self.updatedAt = updatedAt ?? now
4041
}
4142
}

TablePro/Models/Query/SQLFavoriteFolder.swift

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import Foundation
77

88
/// A folder for organizing SQL favorites into a hierarchy
9-
struct SQLFavoriteFolder: Identifiable, Codable, Hashable {
9+
internal struct SQLFavoriteFolder: Identifiable, Codable, Hashable {
1010
let id: UUID
1111
var name: String
1212
var parentId: UUID?
@@ -21,15 +21,16 @@ struct SQLFavoriteFolder: Identifiable, Codable, Hashable {
2121
parentId: UUID? = nil,
2222
connectionId: UUID? = nil,
2323
sortOrder: Int = 0,
24-
createdAt: Date = Date(),
25-
updatedAt: Date = Date()
24+
createdAt: Date? = nil,
25+
updatedAt: Date? = nil
2626
) {
27+
let now = Date()
2728
self.id = id
2829
self.name = name
2930
self.parentId = parentId
3031
self.connectionId = connectionId
3132
self.sortOrder = sortOrder
32-
self.createdAt = createdAt
33-
self.updatedAt = updatedAt
33+
self.createdAt = createdAt ?? now
34+
self.updatedAt = updatedAt ?? now
3435
}
3536
}

TablePro/Models/UI/SharedSidebarState.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import Foundation
1010

1111
/// Which sidebar tab is active
12-
enum SidebarTab: String, CaseIterable {
12+
internal enum SidebarTab: String, CaseIterable {
1313
case tables
1414
case favorites
1515
}

TablePro/Resources/Localizable.xcstrings

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6034,6 +6034,9 @@
60346034
},
60356035
"Delete Folder" : {
60366036

6037+
},
6038+
"Delete Folder?" : {
6039+
60376040
},
60386041
"Delete Foreign Key" : {
60396042
"extractionState" : "stale",
@@ -8387,6 +8390,9 @@
83878390
},
83888391
"Focus Border" : {
83898392

8393+
},
8394+
"Folder name" : {
8395+
83908396
},
83918397
"Font" : {
83928398
"extractionState" : "stale",
@@ -10016,6 +10022,9 @@
1001610022
},
1001710023
"Keyword:" : {
1001810024

10025+
},
10026+
"keyword: %@" : {
10027+
1001910028
},
1002010029
"Language:" : {
1002110030
"localizations" : {
@@ -10808,6 +10817,9 @@
1080810817
}
1080910818
}
1081010819
}
10820+
},
10821+
"Move to" : {
10822+
1081110823
},
1081210824
"Move Up" : {
1081310825
"extractionState" : "stale",
@@ -14928,6 +14940,9 @@
1492814940
}
1492914941
}
1493014942
}
14943+
},
14944+
"Root Level" : {
14945+
1493114946
},
1493214947
"Row %lld" : {
1493314948
"localizations" : {
@@ -16067,6 +16082,9 @@
1606716082
},
1606816083
"Settings:" : {
1606916084

16085+
},
16086+
"Shadows the SQL keyword '%@'" : {
16087+
1607016088
},
1607116089
"Share anonymous usage data" : {
1607216090
"localizations" : {
@@ -17735,6 +17753,9 @@
1773517753
}
1773617754
}
1773717755
}
17756+
},
17757+
"The folder \"%@\" will be deleted. Items inside will be moved to the parent level." : {
17758+
1773817759
},
1773917760
"The following %lld queries may permanently modify or delete data. This action cannot be undone.\n\n%@" : {
1774017761
"localizations" : {
@@ -19468,9 +19489,6 @@
1946819489
}
1946919490
}
1947019491
}
19471-
},
19472-
"Warning: this shadows the SQL keyword '%@'" : {
19473-
1947419492
},
1947519493
"Website" : {
1947619494
"localizations" : {
@@ -19553,6 +19571,9 @@
1955319571
}
1955419572
}
1955519573
}
19574+
},
19575+
"When enabled, this favorite is visible in all connections" : {
19576+
1955619577
},
1955719578
"When TablePro starts:" : {
1955819579
"localizations" : {

TablePro/ViewModels/FavoritesSidebarViewModel.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import Foundation
77
import Observation
88

99
/// Tree node for displaying favorites and folders in a hierarchy
10-
enum FavoriteTreeItem: Identifiable, Hashable {
10+
internal enum FavoriteTreeItem: Identifiable, Hashable {
1111
case folder(SQLFavoriteFolder, children: [FavoriteTreeItem])
1212
case favorite(SQLFavorite)
1313

@@ -21,7 +21,7 @@ enum FavoriteTreeItem: Identifiable, Hashable {
2121

2222
/// ViewModel for the favorites sidebar section
2323
@MainActor @Observable
24-
final class FavoritesSidebarViewModel {
24+
internal final class FavoritesSidebarViewModel {
2525
// MARK: - State
2626

2727
var treeItems: [FavoriteTreeItem] = []
@@ -86,7 +86,7 @@ final class FavoritesSidebarViewModel {
8686

8787
let levelFolders = folders
8888
.filter { $0.parentId == parentId }
89-
.sorted { $0.sortOrder < $1.sortOrder }
89+
.sorted { $0.sortOrder != $1.sortOrder ? $0.sortOrder < $1.sortOrder : $0.name.localizedStandardCompare($1.name) == .orderedAscending }
9090

9191
for folder in levelFolders {
9292
let children = buildTree(folders: folders, favorites: favorites, parentId: folder.id)
@@ -95,7 +95,7 @@ final class FavoritesSidebarViewModel {
9595

9696
let levelFavorites = favorites
9797
.filter { $0.folderId == parentId }
98-
.sorted { $0.sortOrder < $1.sortOrder }
98+
.sorted { $0.sortOrder != $1.sortOrder ? $0.sortOrder < $1.sortOrder : $0.name.localizedStandardCompare($1.name) == .orderedAscending }
9999

100100
for fav in levelFavorites {
101101
items.append(.favorite(fav))

TablePro/Views/Components/ConflictResolutionView.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ struct ConflictResolutionView: View {
140140
}
141141
case .favorite, .favoriteFolder:
142142
if let name = record["name"] as? String {
143-
fieldRow(label: "Name", value: name)
143+
fieldRow(label: String(localized: "Name"), value: name)
144144
}
145145
}
146146
}

0 commit comments

Comments
 (0)