58be682 · parseable-macos

git.iko.soy/parseable-macos

Fix 6 bugs: QueryViewModel data race, Settings sheet unreachable, dropped live-tail rebuilds, QueryResponse decoder, refresh token guard, popup anchoring

58be682iko3 months ago

ParseableViewer/Models/JSONValue.swift+14

compare with master

unifiedsplit

@@ -379,4 +379,18 @@ typealias LogRecord = [String: JSONValue]

379struct QueryResponse: Codable, Sendable {

380 let records: [LogRecord]

381 let fields: [String]?

382+

383+ enum CodingKeys: String, CodingKey {

384+ case records, fields

385+ }

386+

387+ init(from decoder: Decoder) throws {

388+ let container = try decoder.container(keyedBy: CodingKeys.self)

389+ // A wrapped response can legitimately omit `records` for a zero-row result

390+ // (e.g. `{"fields": [...]}`). Default to an empty array rather than letting

391+ // the synthesized decoder throw, which would surface as a spurious

392+ // "Unexpected query response format" error instead of an empty result set.

393+ records = try container.decodeIfPresent([LogRecord].self, forKey: .records) ?? []

394+ fields = try container.decodeIfPresent([String].self, forKey: .fields)

395+ }

382}

396}

ParseableViewer/ViewModels/QueryViewModel.swift+19−12

@@ -2,6 +2,7 @@ import Foundation

2import SwiftUI

4@Observable

5+@MainActor

5final class QueryViewModel {

6final class QueryViewModel {

6 var sqlQuery = "" {

7 var sqlQuery = "" {

7 didSet {

8 didSet {

@@ -24,14 +25,20 @@ final class QueryViewModel {

24 // re-applies the filter.

25 // re-applies the filter.

25 updateFilteredResults()

26 updateFilteredResults()

26 let snapshot = results

27 let snapshot = results

28+ // The class is @MainActor, so this Task inherits main-actor isolation —

29+ // its mutations of cachedSearchTexts/filteredResults are safely serialized

30+ // with SwiftUI reads. Only the pure (and potentially large) displayString

31+ // mapping is pushed off-main via Task.detached so it doesn't block the UI.

27− searchTextTask = Task {

32+ searchTextTask = Task { [weak self] in

33+ let texts = await Task.detached(priority: .userInitiated) {

28− let texts = snapshot.map { record in

34+ snapshot.map { record in

29− record.values.map { $0.displayString }.joined(separator: " ")

35+ record.values.map { $0.displayString }.joined(separator: " ")

30− }

36+ }

37+ }.value

31− guard !Task.isCancelled else { return }

38+ guard let self, !Task.isCancelled else { return }

32− cachedSearchTexts = texts

39+ self.cachedSearchTexts = texts

33− if !filterText.isEmpty {

40+ if !self.filterText.isEmpty {

34− updateFilteredResults()

41+ self.updateFilteredResults()

35 }

42 }

36 }

43 }

37 }

44 }

@@ -551,7 +558,7 @@ final class QueryViewModel {

551 /// Projects each record down to the given columns so JSON export, like CSV,

558 /// Projects each record down to the given columns so JSON export, like CSV,

552 /// respects hidden columns instead of leaking every field. Safe to call from

559 /// respects hidden columns instead of leaking every field. Safe to call from

553 /// any thread.

560 /// any thread.

554− static func projectRecords(_ records: [LogRecord], to columns: [String]) -> [LogRecord] {

561+ nonisolated static func projectRecords(_ records: [LogRecord], to columns: [String]) -> [LogRecord] {

555 // No column list means no restriction (columns not computed yet) — export

562 // No column list means no restriction (columns not computed yet) — export

556 // the full records rather than stripping every field to an empty object.

563 // the full records rather than stripping every field to an empty object.

557 guard !columns.isEmpty else { return records }

564 guard !columns.isEmpty else { return records }

@@ -560,7 +567,7 @@ final class QueryViewModel {

560 }

567 }

561

568

562 /// Builds a CSV string from records and columns. Safe to call from any thread.

569 /// Builds a CSV string from records and columns. Safe to call from any thread.

563− static func buildCSV(records: [LogRecord], columns: [String]) -> String {

570+ nonisolated static func buildCSV(records: [LogRecord], columns: [String]) -> String {

564 guard !records.isEmpty, !columns.isEmpty else { return "" }

571 guard !records.isEmpty, !columns.isEmpty else { return "" }

565

572

566 var csv = String()

573 var csv = String()

@@ -586,7 +593,7 @@ final class QueryViewModel {

586 return csv

593 return csv

587 }

594 }

588

595

589− private static func escapeCSV(_ value: String) -> String {

596+ nonisolated private static func escapeCSV(_ value: String) -> String {

590 // Check at the Unicode scalar level to correctly detect \r and \n

597 // Check at the Unicode scalar level to correctly detect \r and \n

591 // inside \r\n (CRLF) grapheme clusters, which Swift's String.contains

598 // inside \r\n (CRLF) grapheme clusters, which Swift's String.contains

592 // treats as a single Character that doesn't match "\r" or "\n" alone.

599 // treats as a single Character that doesn't match "\r" or "\n" alone.

@@ -731,7 +738,7 @@ final class QueryViewModel {

731 }

738 }

732

739

733 /// Escapes a SQL identifier by doubling internal double-quotes, then wrapping in double-quotes.

740 /// Escapes a SQL identifier by doubling internal double-quotes, then wrapping in double-quotes.

734− static func escapeSQLIdentifier(_ identifier: String) -> String {

741+ nonisolated static func escapeSQLIdentifier(_ identifier: String) -> String {

735 "\"\(identifier.replacingOccurrences(of: "\"", with: "\"\""))\""

742 "\"\(identifier.replacingOccurrences(of: "\"", with: "\"\""))\""

736 }

743 }

737

744

ParseableViewer/Views/LiveTailView.swift+8−3

@@ -290,6 +290,14 @@ struct LiveTailView: View {

290 // next poll.

291 rebuildSortedEntries()

292 }

293+ .onChange(of: viewModel.filteredEntriesGeneration) { _, _ in

294+ // Kept on the always-present container (not inside liveTailTable, which

295+ // is only mounted once visibleColumns is non-empty). A generation bump

296+ // that lands while the table is unmounted — e.g. the first poll arriving

297+ // before columns exist — would otherwise be dropped, leaving the new

298+ // entries unrendered until some other change happened to trigger a rebuild.

299+ rebuildSortedEntries()

300+ }

293 .onAppear {

301 .onAppear {

294 rebuildSortedEntries()

302 rebuildSortedEntries()

295 }

303 }

@@ -414,9 +422,6 @@ struct LiveTailView: View {

414 }

422 }

415 }

423 }

416 }

424 }

417− .onChange(of: viewModel.filteredEntriesGeneration) { _, _ in

418− rebuildSortedEntries()

419− }

420 .onChange(of: sortColumn) { _, _ in

425 .onChange(of: sortColumn) { _, _ in

421 rebuildSortedEntries()

426 rebuildSortedEntries()

422 }

427 }

ParseableViewer/Views/QueryView.swift+4

@@ -341,6 +341,10 @@ struct QueryView: View {

341 }

342 }

343 .onChange(of: appState.queryRefreshToken) { _, _ in

344+ // Guard on a selected stream like every sibling query trigger in this

345+ // view. Without it, a refresh (Cmd+R) with no stream selected and an

346+ // empty editor surfaces a spurious "Select a stream..." error.

347+ guard appState.selectedStream != nil else { return }

344 Task {

348 Task {

345 await viewModel.executeQuery(

349 await viewModel.executeQuery(

346 client: appState.client,

350 client: appState.client,

ParseableViewer/Views/SQLAutoComplete.swift+6−1

@@ -389,7 +389,12 @@ final class SQLCompletionPopup: NSObject, NSTableViewDataSource, NSTableViewDele

389

390 let rowCount = min(items.count, Self.maxVisibleRows)

391 let height = CGFloat(rowCount) * Self.rowHeight + 4

392+ // Anchor by the top edge (where the caret sits), matching show()/reposition().

393+ // The panel's origin is its bottom-left corner, so holding origin.y fixed

394+ // while the height changes would move the top edge — growing the list would

395+ // creep the popup upward over the caret. Pin the top edge instead.

396+ let topEdge = panel.frame.origin.y + panel.frame.height

392− let frame = NSRect(x: panel.frame.origin.x, y: panel.frame.origin.y,

397+ let frame = NSRect(x: panel.frame.origin.x, y: topEdge - height,

393 width: Self.panelWidth, height: height)

398 width: Self.panelWidth, height: height)

394 panel.setFrame(frame, display: true)

399 panel.setFrame(frame, display: true)

395 }

400 }

ParseableViewer/Views/SettingsView.swift+14−4

@@ -6,6 +6,13 @@ struct SettingsView: View {

6 @AppStorage("maxQueryResults") private var maxQueryResults = 1000

7 @AppStorage("liveTailPollInterval") private var liveTailPollInterval = 2.0

8 @AppStorage("liveTailMaxEntries") private var liveTailMaxEntries = 5000

9+ // The Settings scene is a separate window from ContentView. A .sheet presents

10+ // over the window hosting the modifier, and binding both windows to the shared

11+ // appState.showConnectionSheet would present two sheets at once. So the Settings

12+ // window drives its own connection sheet with local state, fully independent of

13+ // the main window's.

14+ @State private var showConnectionSheet = false

15+ @State private var sheetConnection: ServerConnection?

9 var body: some View {

16 var body: some View {

10 TabView {

17 TabView {

11 // General settings

18 // General settings

@@ -77,8 +84,8 @@ struct SettingsView: View {

77 }

84 }

79 Button {

86 Button {

80− appState.editingConnection = connection

87+ sheetConnection = connection

81− appState.showConnectionSheet = true

88+ showConnectionSheet = true

82 } label: {

89 } label: {

83 Image(systemName: "pencil")

90 Image(systemName: "pencil")

84 }

91 }

@@ -98,8 +105,8 @@ struct SettingsView: View {

98 HStack {

105 HStack {

99 Spacer()

106 Spacer()

100 Button("Add Connection...") {

107 Button("Add Connection...") {

101− appState.editingConnection = nil

108+ sheetConnection = nil

102− appState.showConnectionSheet = true

109+ showConnectionSheet = true

103 }

110 }

104 }

111 }

105 .padding()

112 .padding()

@@ -109,5 +116,8 @@ struct SettingsView: View {

109 }

116 }

110 }

117 }

111 .frame(width: 500, height: 400)

118 .frame(width: 500, height: 400)

119+ .sheet(isPresented: $showConnectionSheet) {

120+ ConnectionSheet(connection: sheetConnection)

121+ }

112 }

122 }

113}

123}

ParseableViewerTests/QueryViewModelTests.swift+2

@@ -1,6 +1,8 @@

1import XCTest

2@testable import ParseableViewer

4+// QueryViewModel is @MainActor, so its tests must run on the main actor too.

5+@MainActor

4final class QueryViewModelTests: XCTestCase {

6final class QueryViewModelTests: XCTestCase {

5 // MARK: - Column extraction

7 // MARK: - Column extraction