Skip to content

Commit 74c3bfa

Browse files
authored
Merge pull request #259 from datlechin/fix/plugin-standardization-review-issues
fix: address review issues from plugin standardization PR
2 parents 0085ec7 + 6ca6395 commit 74c3bfa

6 files changed

Lines changed: 29 additions & 28 deletions

File tree

Plugins/MongoDBDriverPlugin/MongoDBConnection.swift

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ final class MongoDBConnection: @unchecked Sendable {
369369
projection: String? = nil,
370370
skip: Int,
371371
limit: Int
372-
) async throws -> [[String: Any]] {
372+
) async throws -> (docs: [[String: Any]], isTruncated: Bool) {
373373
#if canImport(CLibMongoc)
374374
resetCancellation()
375375
return try await pluginDispatchAsync(on: queue) { [self] in
@@ -387,7 +387,7 @@ final class MongoDBConnection: @unchecked Sendable {
387387
#endif
388388
}
389389

390-
func aggregate(database: String, collection: String, pipeline: String) async throws -> [[String: Any]] {
390+
func aggregate(database: String, collection: String, pipeline: String) async throws -> (docs: [[String: Any]], isTruncated: Bool) {
391391
#if canImport(CLibMongoc)
392392
resetCancellation()
393393
return try await pluginDispatchAsync(on: queue) { [self] in
@@ -595,7 +595,7 @@ private extension MongoDBConnection {
595595
func findSync(
596596
client: OpaquePointer, database: String, collection: String,
597597
filter: String, sort: String?, projection: String?, skip: Int, limit: Int
598-
) throws -> [[String: Any]] {
598+
) throws -> (docs: [[String: Any]], isTruncated: Bool) {
599599
try checkCancelled()
600600

601601
guard let filterBson = jsonToBson(filter) else {
@@ -640,7 +640,7 @@ private extension MongoDBConnection {
640640

641641
func aggregateSync(
642642
client: OpaquePointer, database: String, collection: String, pipeline: String
643-
) throws -> [[String: Any]] {
643+
) throws -> (docs: [[String: Any]], isTruncated: Bool) {
644644
try checkCancelled()
645645

646646
guard let pipelineBson = jsonToBson(pipeline) else {
@@ -833,14 +833,15 @@ private extension MongoDBConnection {
833833
}
834834
defer { mongoc_cursor_destroy(cursor) }
835835

836-
return try iterateCursor(cursor)
836+
return try iterateCursor(cursor).docs
837837
}
838838

839-
func iterateCursor(_ cursor: OpaquePointer) throws -> [[String: Any]] {
839+
func iterateCursor(_ cursor: OpaquePointer) throws -> (docs: [[String: Any]], isTruncated: Bool) {
840840
try checkCancelled()
841841

842842
var results: [[String: Any]] = []
843843
var docPtr: OpaquePointer?
844+
var truncated = false
844845

845846
while mongoc_cursor_next(cursor, &docPtr) {
846847
try checkCancelled()
@@ -850,6 +851,7 @@ private extension MongoDBConnection {
850851
}
851852

852853
if results.count >= PluginRowLimits.defaultMax {
854+
truncated = true
853855
logger.warning("Result set truncated at \(PluginRowLimits.defaultMax) documents")
854856
break
855857
}
@@ -859,7 +861,7 @@ private extension MongoDBConnection {
859861
if mongoc_cursor_error(cursor, &error) {
860862
throw makeError(error)
861863
}
862-
return results
864+
return (docs: results, isTruncated: truncated)
863865
}
864866
}
865867
#endif

Plugins/MongoDBDriverPlugin/MongoDBPluginDriver.swift

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ final class MongoDBPluginDriver: PluginDatabaseDriver {
123123
case .findOne:
124124
return 1
125125
case .aggregate(let collection, let pipeline):
126-
let docs = try await conn.aggregate(database: db, collection: collection, pipeline: pipeline)
127-
return docs.count
126+
let result = try await conn.aggregate(database: db, collection: collection, pipeline: pipeline)
127+
return result.docs.count
128128
case .countDocuments(let collection, let filter):
129129
let count = try await conn.countDocuments(database: db, collection: collection, filter: filter)
130130
return Int(count)
@@ -148,12 +148,12 @@ final class MongoDBPluginDriver: PluginDatabaseDriver {
148148
case .find(let collection, let filter, var options):
149149
options.skip = offset
150150
options.limit = limit
151-
let docs = try await conn.find(
151+
let result = try await conn.find(
152152
database: db, collection: collection, filter: filter,
153153
sort: options.sort, projection: options.projection,
154154
skip: offset, limit: limit
155155
)
156-
return buildPluginResult(from: docs, startTime: startTime)
156+
return buildPluginResult(from: result.docs, startTime: startTime, isTruncated: result.isTruncated)
157157
default:
158158
return try await executeOperation(operation, connection: conn, startTime: startTime)
159159
}
@@ -178,7 +178,7 @@ final class MongoDBPluginDriver: PluginDatabaseDriver {
178178
let docs = try await conn.find(
179179
database: currentDb, collection: table,
180180
filter: "{}", sort: nil, projection: nil, skip: 0, limit: 500
181-
)
181+
).docs
182182

183183
if docs.isEmpty {
184184
return [
@@ -527,30 +527,29 @@ final class MongoDBPluginDriver: PluginDatabaseDriver {
527527

528528
switch operation {
529529
case .find(let collection, let filter, let options):
530-
let docs = try await conn.find(
530+
let result = try await conn.find(
531531
database: db, collection: collection, filter: filter,
532532
sort: options.sort, projection: options.projection,
533533
skip: options.skip ?? 0, limit: options.limit ?? PluginRowLimits.defaultMax
534534
)
535-
if docs.isEmpty {
535+
if result.docs.isEmpty {
536536
return PluginQueryResult(
537537
columns: ["_id"], columnTypeNames: ["ObjectId"],
538538
rows: [], rowsAffected: 0, executionTime: Date().timeIntervalSince(startTime)
539539
)
540540
}
541-
let truncated = docs.count >= PluginRowLimits.defaultMax
542-
return buildPluginResult(from: docs, startTime: startTime, isTruncated: truncated)
541+
return buildPluginResult(from: result.docs, startTime: startTime, isTruncated: result.isTruncated)
543542

544543
case .findOne(let collection, let filter):
545-
let docs = try await conn.find(
544+
let result = try await conn.find(
546545
database: db, collection: collection, filter: filter,
547546
sort: nil, projection: nil, skip: 0, limit: 1
548547
)
549-
return buildPluginResult(from: docs, startTime: startTime)
548+
return buildPluginResult(from: result.docs, startTime: startTime)
550549

551550
case .aggregate(let collection, let pipeline):
552-
let docs = try await conn.aggregate(database: db, collection: collection, pipeline: pipeline)
553-
return buildPluginResult(from: docs, startTime: startTime)
551+
let result = try await conn.aggregate(database: db, collection: collection, pipeline: pipeline)
552+
return buildPluginResult(from: result.docs, startTime: startTime, isTruncated: result.isTruncated)
554553

555554
case .countDocuments(let collection, let filter):
556555
let count = try await conn.countDocuments(database: db, collection: collection, filter: filter)

Plugins/RedisDriverPlugin/RedisCommandParser.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ extension RedisParseError: PluginDriverError {
8888
var pluginErrorMessage: String {
8989
switch self {
9090
case .emptySyntax: return String(localized: "Empty Redis command")
91-
case .invalidArgument(let msg): return msg
92-
case .missingArgument(let msg): return msg
91+
case .invalidArgument(let msg): return String(localized: "Invalid argument: \(msg)")
92+
case .missingArgument(let msg): return String(localized: "Missing argument: \(msg)")
9393
}
9494
}
9595
}

Plugins/TableProPluginKit/PluginConcurrencySupport.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public func pluginDispatchAsync(
3232
}
3333
}
3434

35-
public func pluginDispatchAsync<T: Sendable>(
35+
public func pluginDispatchAsyncCancellable<T: Sendable>(
3636
on queue: DispatchQueue,
3737
cancellationCheck: (@Sendable () -> Bool)? = nil,
3838
execute work: @escaping @Sendable () throws -> T

Plugins/TableProPluginKit/PluginDriverError.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@ public extension PluginDriverError {
1919

2020
var errorDescription: String? {
2121
var desc = pluginErrorMessage
22-
if let code = pluginErrorCode {
22+
if let code = pluginErrorCode, code != 0 {
2323
desc = "[\(code)] \(desc)"
2424
}
2525
if let state = pluginSqlState {
2626
desc += " (SQLSTATE: \(state))"
2727
}
2828
if let detail = pluginErrorDetail, !detail.isEmpty {
29-
desc += "\n\(detail)"
29+
desc += "\nDetail: \(detail)"
3030
}
3131
return desc
3232
}

TablePro/Views/Toolbar/TableProToolbarView.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,9 @@ struct TableProToolbar: ViewModifier {
139139
: state.databaseType == .redis ? "Preview Commands (⌘⇧P)"
140140
: "Preview SQL (⌘⇧P)")
141141
.disabled(!state.hasPendingChanges || state.connectionState != .connected)
142+
.popover(isPresented: $state.showSQLReviewPopover) {
143+
SQLReviewPopover(statements: state.previewStatements, databaseType: state.databaseType)
144+
}
142145
}
143146

144147
ToolbarItem(placement: .primaryAction) {
@@ -179,9 +182,6 @@ struct TableProToolbar: ViewModifier {
179182
}
180183
}
181184
}
182-
.popover(isPresented: $state.showSQLReviewPopover) {
183-
SQLReviewPopover(statements: state.previewStatements, databaseType: state.databaseType)
184-
}
185185
.onReceive(NotificationCenter.default.publisher(for: .openConnectionSwitcher)) { _ in
186186
showConnectionSwitcher = true
187187
}

0 commit comments

Comments
 (0)