a0cf99e · parseable-macos

git.iko.soy/parseable-macos

Fix 6 bugs: saved-filter onChange race, JSON export column leak, live-tail poll late-write and clear repopulation, formatter locale, and stale stream selection

a0cf99eiko3 months ago

ParseableViewer/App/AppState.swift+6

compare with master

unifiedsplit

@@ -237,6 +237,12 @@ final class AppState {

237 streamLoadError = nil

238 do {

239 streams = try await client.listStreams()

240+ // Drop a selection that no longer exists server-side (e.g. the stream

241+ // was deleted from another client); otherwise the detail tabs keep

242+ // querying a missing stream and surface 404s.

243+ if let selected = selectedStream, !streams.contains(where: { $0.name == selected }) {

244+ selectedStream = nil

245+ }

240 } catch {

246 } catch {

241 let message = ParseableError.userFriendlyMessage(for: error)

247 let message = ParseableError.userFriendlyMessage(for: error)

242 streamLoadError = message

248 streamLoadError = message

ParseableViewer/ViewModels/LiveTailViewModel.swift+29−1

@@ -48,6 +48,10 @@ final class LiveTailViewModel {

48 /// (which would let the two interleave and write `lastTimestamp` /

49 /// `consecutiveErrors` out of order).

50 private var isPolling = false

51+ /// Bumped by start()/stop()/clear() so an in-flight poll whose network

52+ /// round-trip outlives one of those transitions can detect it was superseded

53+ /// and drop its results instead of repopulating a torn-down/cleared tail.

54+ private var pollGeneration = 0

52 deinit {

56 deinit {

53 timer?.invalidate()

57 timer?.invalidate()

@@ -56,6 +60,9 @@ final class LiveTailViewModel {

56 // Cached formatters — nonisolated(unsafe) so static methods can use them from Task.detached

60 // Cached formatters — nonisolated(unsafe) so static methods can use them from Task.detached

57 nonisolated(unsafe) private static let displayFormatter: DateFormatter = {

61 nonisolated(unsafe) private static let displayFormatter: DateFormatter = {

58 let f = DateFormatter()

62 let f = DateFormatter()

63+ // Fixed-format formatters must use en_US_POSIX so a non-Gregorian or

64+ // non-Latin-digit system locale doesn't render wrong numerals/era.

65+ f.locale = Locale(identifier: "en_US_POSIX")

59 f.dateFormat = "HH:mm:ss.SSS"

66 f.dateFormat = "HH:mm:ss.SSS"

60 return f

67 return f

61 }()

68 }()

@@ -168,6 +175,7 @@ final class LiveTailViewModel {

168 let storedMax = UserDefaults.standard.integer(forKey: "liveTailMaxEntries")

175 let storedMax = UserDefaults.standard.integer(forKey: "liveTailMaxEntries")

169 if storedMax > 0 { maxEntries = storedMax }

176 if storedMax > 0 { maxEntries = storedMax }

170

177

178+ pollGeneration += 1

171 isRunning = true

179 isRunning = true

172 isPaused = false

180 isPaused = false

173 errorMessage = nil

181 errorMessage = nil

@@ -195,6 +203,7 @@ final class LiveTailViewModel {

195 }

203 }

196

204

197 func stop() {

205 func stop() {

206+ pollGeneration += 1

198 timer?.invalidate()

207 timer?.invalidate()

199 timer = nil

208 timer = nil

200 isRunning = false

209 isRunning = false

@@ -206,8 +215,14 @@ final class LiveTailViewModel {

206 }

215 }

207

216

208 func clear() {

217 func clear() {

218+ pollGeneration += 1

209 entries = []

219 entries = []

210 seenFingerprints = []

220 seenFingerprints = []

221+ // Advance the look-back floor to now: without this, the next poll queries

222+ // from the old `lastTimestamp` with an empty fingerprint set and re-inserts

223+ // every just-cleared record still inside the ~90 s window, repopulating the

224+ // view the user just emptied. Matches start()'s reset.

225+ lastTimestamp = Date()

211 allKnownKeys = []

226 allKnownKeys = []

212 droppedCount = 0

227 droppedCount = 0

213 columns = []

228 columns = []

@@ -355,6 +370,7 @@ final class LiveTailViewModel {

355 guard !isPolling else { return }

370 guard !isPolling else { return }

356 isPolling = true

371 isPolling = true

357 defer { isPolling = false }

372 defer { isPolling = false }

373+ let generation = pollGeneration

358

374

359 let now = Date()

375 let now = Date()

360 // The Parseable server truncates query time-ranges to minute

376 // The Parseable server truncates query time-ranges to minute

@@ -374,6 +390,11 @@ final class LiveTailViewModel {

374 do {

390 do {

375 let records = try await client.query(sql: sql, startTime: queryStart, endTime: now)

391 let records = try await client.query(sql: sql, startTime: queryStart, endTime: now)

376

392

393+ // A stop()/clear()/start during the network round-trip supersedes this

394+ // poll; don't write stale results to a torn-down or cleared tail. Also

395+ // bail if the user paused mid-flight.

396+ guard isRunning, !isPaused, generation == pollGeneration else { return }

397+

377 // Process records off the main actor using a snapshot of seen fingerprints

398 // Process records off the main actor using a snapshot of seen fingerprints

378 let seenSnapshot = seenFingerprints

399 let seenSnapshot = seenFingerprints

379 let pollTime = now

400 let pollTime = now

@@ -395,6 +416,10 @@ final class LiveTailViewModel {

395 return results

416 return results

396 }.value

417 }.value

397

418

419+ // Re-check after the detached processing hop: a stop/clear could have

420+ // landed while we were off the main actor.

421+ guard isRunning, !isPaused, generation == pollGeneration else { return }

422+

398 // Authoritative dedup on main actor

423 // Authoritative dedup on main actor

399 var newEntries: [LiveTailEntry] = []

424 var newEntries: [LiveTailEntry] = []

400 for entry in candidateEntries {

425 for entry in candidateEntries {

@@ -451,7 +476,10 @@ final class LiveTailViewModel {

451 errorMessage = nil

476 errorMessage = nil

452 consecutiveErrors = 0

477 consecutiveErrors = 0

453 } catch {

478 } catch {

479+ // Drop the error if this poll was cancelled or superseded by a

480+ // stop/clear/start — don't bump the failure counter or surface an

481+ // error on a tail the user already tore down.

454− guard !Task.isCancelled else { return }

482+ guard !Task.isCancelled, isRunning, generation == pollGeneration else { return }

455 consecutiveErrors += 1

483 consecutiveErrors += 1

456 if consecutiveErrors >= Self.maxConsecutiveErrors {

484 if consecutiveErrors >= Self.maxConsecutiveErrors {

457 errorMessage = "\(ParseableError.userFriendlyMessage(for: error)) — stopped after \(consecutiveErrors) consecutive failures"

485 errorMessage = "\(ParseableError.userFriendlyMessage(for: error)) — stopped after \(consecutiveErrors) consecutive failures"

ParseableViewer/ViewModels/QueryViewModel.swift+12−1

@@ -497,7 +497,7 @@ final class QueryViewModel {

497 guard !results.isEmpty else { return "[]" }

498 let encoder = JSONEncoder()

499 encoder.outputFormatting = [.prettyPrinted, .sortedKeys]

500− if let data = try? encoder.encode(results),

500+ if let data = try? encoder.encode(Self.projectRecords(results, to: visibleColumns)),

501 let string = String(data: data, encoding: .utf8) {

502 return string

503 }

@@ -508,6 +508,17 @@ final class QueryViewModel {

508 Self.buildCSV(records: results, columns: visibleColumns)

509 }

510

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

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

513+ /// any thread.

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

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

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

517+ guard !columns.isEmpty else { return records }

518+ let allowed = Set(columns)

519+ return records.map { record in record.filter { allowed.contains($0.key) } }

520+ }

521+

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

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

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

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

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

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

ParseableViewer/Views/QueryView.swift+36−15

@@ -1,5 +1,15 @@

1import SwiftUI

3+/// Composite of the inputs that decide which query QueryView should run. Watched

4+/// by a single `.onChange` so a stream switch and a pending saved-filter (which

5+/// arrive in the same update) are handled together in one pass — two separate

6+/// `.onChange` handlers would race and could clobber the filter with the default

7+/// query depending on their firing order.

8+private struct QueryViewContext: Equatable {

9+ let stream: String?

10+ let pendingSQL: String?

11+}

12+

3struct QueryView: View {

13struct QueryView: View {

4 @Environment(AppState.self) private var appState

14 @Environment(AppState.self) private var appState

5 @State private var viewModel = QueryViewModel()

15 @State private var viewModel = QueryViewModel()

@@ -251,19 +261,33 @@ struct QueryView: View {

251 )

261 )

252 }

262 }

253 }

263 }

254− .onChange(of: appState.selectedStream) { oldValue, newValue in

264+ .onChange(of: QueryViewContext(stream: appState.selectedStream, pendingSQL: appState.pendingFilterSQL)) { old, new in

265+ guard let stream = new.stream else {

255− viewModel.clearResults()

266+ viewModel.clearResults()

256− if let stream = newValue {

267+ return

257− let didSetDefault = viewModel.setDefaultQuery(stream: stream, previousStream: oldValue)

268+ }

269+ let streamChanged = old.stream != new.stream

270+ if streamChanged {

271+ viewModel.clearResults()

258 Task {

272 Task {

259 await viewModel.loadSchema(client: appState.client, stream: stream)

273 await viewModel.loadSchema(client: appState.client, stream: stream)

260 }

274 }

275+ }

276+ // A pending saved filter (set together with the stream switch, or on

277+ // its own for the already-selected stream) takes priority over the

278+ // default query. Consuming it here in the same handler that owns the

279+ // stream switch avoids the previous two-handler race.

280+ if let sql = appState.pendingFilterSQL {

281+ appState.pendingFilterSQL = nil

282+ viewModel.sqlQuery = sql

283+ Task {

284+ await viewModel.executeQuery(client: appState.client, stream: stream)

285+ }

286+ } else if streamChanged {

287+ let didSetDefault = viewModel.setDefaultQuery(stream: stream, previousStream: old.stream)

261 if didSetDefault {

288 if didSetDefault {

262 Task {

289 Task {

263− await viewModel.executeQuery(

290+ await viewModel.executeQuery(client: appState.client, stream: stream)

264− client: appState.client,

265− stream: stream

266− )

267 }

291 }

268 }

292 }

269 }

293 }

@@ -324,12 +348,6 @@ struct QueryView: View {

324 )

348 )

325 }

349 }

326 }

350 }

327− .onChange(of: appState.pendingFilterSQL) { _, newSQL in

328− if let sql = newSQL {

329− viewModel.sqlQuery = sql

330− appState.pendingFilterSQL = nil

331− }

332− }

333 .alert("Export Error", isPresented: $showExportError) {

351 .alert("Export Error", isPresented: $showExportError) {

334 Button("OK") {}

352 Button("OK") {}

335 } message: {

353 } message: {

@@ -395,7 +413,10 @@ struct QueryView: View {

395 } else {

413 } else {

396 let encoder = JSONEncoder()

414 let encoder = JSONEncoder()

397 encoder.outputFormatting = [.prettyPrinted, .sortedKeys]

415 encoder.outputFormatting = [.prettyPrinted, .sortedKeys]

416+ // Project to visible columns so JSON export matches CSV and

417+ // doesn't leak hidden fields.

418+ let projected = QueryViewModel.projectRecords(results, to: columns)

398− content = (try? encoder.encode(results))

419+ content = (try? encoder.encode(projected))

399 .flatMap { String(data: $0, encoding: .utf8) }

420 .flatMap { String(data: $0, encoding: .utf8) }

400 }

421 }

401 guard let content else {

422 guard let content else {

ParseableViewerTests/QueryViewModelTests.swift+13

@@ -138,6 +138,19 @@ final class QueryViewModelTests: XCTestCase {

138 XCTAssertTrue(json.contains("\"value\""))

139 }

140

141+ func testProjectRecordsExcludesHiddenColumns() {

142+ let records: [LogRecord] = [["visible": .string("v"), "hidden": .string("secret")]]

143+ let projected = QueryViewModel.projectRecords(records, to: ["visible"])

144+ XCTAssertEqual(projected.first?.keys.sorted(), ["visible"])

145+ XCTAssertNil(projected.first?["hidden"], "Hidden column must not leak into export")

146+ }

147+

148+ func testProjectRecordsEmptyColumnsKeepsAllFields() {

149+ let records: [LogRecord] = [["a": .int(1), "b": .int(2)]]

150+ let projected = QueryViewModel.projectRecords(records, to: [])

151+ XCTAssertEqual(projected.first?.keys.sorted(), ["a", "b"])

152+ }

153+

141 // MARK: - Time range

154 // MARK: - Time range

142

155

143 func testTimeRangeOptionsAllHaveValues() {

156 func testTimeRangeOptionsAllHaveValues() {