0eb0898 · parseable-macos

git.iko.soy/parseable-macos

on master help sign in

Fix stale loading states, cached refreshes and results table state

0eb0898Claude1 week ago

ParseableViewer/Views/AlertsView.swift+12−1

unifiedsplit

@@ -5,6 +5,10 @@ struct AlertsView: View {

5 @State private var alertConfig: AlertConfig?

6 @State private var isLoading = false

7 @State private var errorMessage: String?

8+ /// Bumped by every load. SwiftUI cancels the previous `.task(id:)` load on a

9+ /// stream switch, but that load still resumes (with a cancellation error) —

10+ /// and must not touch `isLoading`, which now belongs to the newer load.

11+ @State private var loadGeneration = 0

9 var body: some View {

13 var body: some View {

10 Group {

14 Group {

@@ -77,12 +81,16 @@ struct AlertsView: View {

77 private func loadAlerts(stream: String) async {

81 private func loadAlerts(stream: String) async {

78 guard let client = appState.client else { return }

82 guard let client = appState.client else { return }

79 let connID = appState.activeConnection?.id

83 let connID = appState.activeConnection?.id

84+ loadGeneration &+= 1

85+ let generation = loadGeneration

80 isLoading = true

86 isLoading = true

81 errorMessage = nil

87 errorMessage = nil

82 alertConfig = nil

88 alertConfig = nil

84 do {

90 do {

85 let config = try await client.getAlerts(stream: stream)

91 let config = try await client.getAlerts(stream: stream)

92+ // Superseded by a newer load: leave its spinner and state alone.

93+ guard generation == loadGeneration else { return }

86 // The manual Refresh/Retry buttons spawn detached Tasks not tied to

94 // The manual Refresh/Retry buttons spawn detached Tasks not tied to

87 // `.task(id: stream)` cancellation, so a slow refresh for the old stream

95 // `.task(id: stream)` cancellation, so a slow refresh for the old stream

88 // can resolve after a switch. Drop stale results. Check the connection

96 // can resolve after a switch. Drop stale results. Check the connection

@@ -94,6 +102,7 @@ struct AlertsView: View {

94 connID == appState.activeConnection?.id else { isLoading = false; return }

102 connID == appState.activeConnection?.id else { isLoading = false; return }

95 alertConfig = config

103 alertConfig = config

96 } catch {

104 } catch {

105+ guard generation == loadGeneration else { return }

97 guard stream == appState.selectedStream,

106 guard stream == appState.selectedStream,

98 connID == appState.activeConnection?.id else { isLoading = false; return }

107 connID == appState.activeConnection?.id else { isLoading = false; return }

99 errorMessage = ParseableError.userFriendlyMessage(for: error)

108 errorMessage = ParseableError.userFriendlyMessage(for: error)

@@ -203,7 +212,9 @@ struct AlertRowView: View {

203 private var stateColor: Color {

212 private var stateColor: Color {

204 switch alert.state?.lowercased() {

213 switch alert.state?.lowercased() {

205 case "triggered": return .red

214 case "triggered": return .red

206− case "disabled": return .gray

215+ case "disabled", "silenced": return .gray

216+ // Parseable's default, healthy state (and 2.x's "resolved") — not a warning.

217+ case "not-triggered", "resolved": return .green

207 // A reported-but-unrecognized state is a genuine "needs attention" signal,

218 // A reported-but-unrecognized state is a genuine "needs attention" signal,

208 // so tint it orange; a missing state is simply unknown and stays neutral so

219 // so tint it orange; a missing state is simply unknown and stays neutral so

209 // the bell icon isn't painted as a warning for every stateless alert.

220 // the bell icon isn't painted as a warning for every stateless alert.

ParseableViewer/Views/LogTableView.swift+53−6

@@ -315,7 +315,23 @@ struct LogTableView: View {

315 }

316 .onChange(of: sortColumn) { _, _ in debouncedRebuildSort() }

317 .onChange(of: sortAscending) { _, _ in debouncedRebuildSort() }

318+ .onChange(of: selectedRecord) { _, newValue in

319+ // The owner can drop the selection on its own (QueryViewModel

320+ // clears it after every query). When the new result set equals the

321+ // old one, `onChange(of: records)` doesn't fire, so nothing would

322+ // reconcile the row highlight: the row stayed highlighted with no

323+ // detail pane, and the next click on it merely "deselected" it.

324+ if newValue == nil { selectedIndex = nil }

325+ }

318 .onAppear {

326 .onAppear {

327+ // This branch is unmounted while results are empty (e.g. across a

328+ // stream switch, which clears them first), so `onChange(of: columns)`

329+ // never sees the switch. Drop a sort column the new columns don't

330+ // have — or hide — before sorting, or rows would be ordered by an

331+ // invisible column with no header showing the sort.

332+ if let sortColumn, !columns.contains(sortColumn) {

333+ self.sortColumn = nil

334+ }

319 recomputeColumnWidths(columns: columns, records: records)

335 recomputeColumnWidths(columns: columns, records: records)

320 rebuildSort()

336 rebuildSort()

321 }

337 }

@@ -401,6 +417,9 @@ struct LogTableView: View {

401 }

417 }

402

418

403 private func rebuildSort() {

419 private func rebuildSort() {

420+ // A debounced rebuild scheduled before the table unmounted must not land

421+ // after (and overwrite) this synchronous one.

422+ sortTask?.cancel()

404 let sevCols = buildSeverityColumnSet(columns: columns)

423 let sevCols = buildSeverityColumnSet(columns: columns)

405 let enumerated = Array(records.enumerated())

424 let enumerated = Array(records.enumerated())

406 let sorted: [(offset: Int, element: LogRecord)]

425 let sorted: [(offset: Int, element: LogRecord)]

@@ -476,12 +495,28 @@ struct LogHeaderView: View {

476

495

477 @State private var draggedColumn: String?

496 @State private var draggedColumn: String?

478 @State private var dropTargetColumn: String?

497 @State private var dropTargetColumn: String?

498+ /// `.contextMenu`'s builder runs during body evaluation, so the filter menus

499+ /// compute their values while the view is being rendered. Writing an `@State`

500+ /// dictionary from there is "modifying state during view update" (undefined

501+ /// behavior, and it re-invalidates the header). The cache is therefore a plain

502+ /// reference that isn't observed; `recordsGeneration` — bumped outside body

503+ /// evaluation — is what invalidates it and triggers the re-render.

479− @State private var uniqueValuesCache: [String: [JSONValue]] = [:]

504+ @State private var uniqueValuesCache = UniqueValuesCache()

505+ @State private var recordsGeneration = 0

506+

507+ private final class UniqueValuesCache {

508+ var generation = -1

509+ var values: [String: [JSONValue]] = [:]

510+ }

480

511

481 private static let maxFilterValues = 20

512 private static let maxFilterValues = 20

482

513

483 private func uniqueValues(for column: String) -> [JSONValue] {

514 private func uniqueValues(for column: String) -> [JSONValue] {

515+ if uniqueValuesCache.generation != recordsGeneration {

516+ uniqueValuesCache.generation = recordsGeneration

517+ uniqueValuesCache.values = [:]

518+ }

484− if let cached = uniqueValuesCache[column] {

519+ if let cached = uniqueValuesCache.values[column] {

485 return cached

520 return cached

486 }

521 }

487 var seen = Set<JSONValue>()

522 var seen = Set<JSONValue>()

@@ -493,7 +528,7 @@ struct LogHeaderView: View {

493 }

528 }

494 }

529 }

495 let sorted = result.sorted()

530 let sorted = result.sorted()

496− uniqueValuesCache[column] = sorted

531+ uniqueValuesCache.values[column] = sorted

497 return sorted

532 return sorted

498 }

533 }

499

534

@@ -636,7 +671,7 @@ struct LogHeaderView: View {

636 .background(Color(nsColor: .controlBackgroundColor))

671 .background(Color(nsColor: .controlBackgroundColor))

637 .border(Color.secondary.opacity(0.2), width: 0.5)

672 .border(Color.secondary.opacity(0.2), width: 0.5)

638 .onChange(of: records) { _, _ in

673 .onChange(of: records) { _, _ in

639− uniqueValuesCache = [:]

674+ recordsGeneration &+= 1

640 }

675 }

641 }

676 }

642}

677}

@@ -669,14 +704,26 @@ private struct ColumnDropDelegate: DropDelegate {

669 }

704 }

670

705

671 func performDrop(info: DropInfo) -> Bool {

706 func performDrop(info: DropInfo) -> Bool {

672− guard let dragged = draggedColumn, dragged != column else {

707+ guard let dragged = draggedColumn, dragged != column,

708+ let provider = info.itemProviders(for: [UTType.plainText]).first else {

673 draggedColumn = nil

709 draggedColumn = nil

674 dropTargetColumn = nil

710 dropTargetColumn = nil

675 return false

711 return false

676 }

712 }

677− onMoveColumn?(dragged, column)

678 draggedColumn = nil

713 draggedColumn = nil

679 dropTargetColumn = nil

714 dropTargetColumn = nil

715+ // `draggedColumn` is only cleared by a drop on a header, so a header drag

716+ // that was cancelled (or dropped elsewhere) leaves it set, and any later

717+ // plain-text drag onto a header would move that stale column. Only move

718+ // when the dropped payload really is the column header being dragged.

719+ let target = column

720+ let move = onMoveColumn

721+ _ = provider.loadObject(ofClass: NSString.self) { object, _ in

722+ guard let payload = object as? NSString, payload as String == dragged else { return }

723+ DispatchQueue.main.async {

724+ move?(dragged, target)

725+ }

726+ }

680 return true

727 return true

681 }

728 }

682

729

ParseableViewer/Views/ServerInfoView.swift+12−2

@@ -9,6 +9,9 @@ struct ServerInfoView: View {

9 @State private var isLoading = true

10 @State private var isHealthy = false

11 @State private var errorMessage: String?

12+ /// Bumped by every load so a superseded (cancelled) load can't clear the

13+ /// newer load's spinner or overwrite its results.

14+ @State private var loadGeneration = 0

13 var body: some View {

16 var body: some View {

14 ScrollView {

17 ScrollView {

@@ -19,7 +22,7 @@ struct ServerInfoView: View {

19 .fontWeight(.semibold)

22 .fontWeight(.semibold)

20 Spacer()

23 Spacer()

21 Button {

24 Button {

22− Task { await loadInfo() }

25+ Task { await loadInfo(forceRefresh: true) }

23 } label: {

26 } label: {

24 Image(systemName: "arrow.clockwise")

27 Image(systemName: "arrow.clockwise")

25 }

28 }

@@ -153,12 +156,18 @@ struct ServerInfoView: View {

153 }

156 }

154 }

157 }

155

158

156− private func loadInfo() async {

159+ private func loadInfo(forceRefresh: Bool = false) async {

157 // Clear isLoading on the no-client path: since isLoading now starts `true`

160 // Clear isLoading on the no-client path: since isLoading now starts `true`

158 // (to avoid a red "Unhealthy" flash before the first check), an early return

161 // (to avoid a red "Unhealthy" flash before the first check), an early return

159 // without resetting it would wedge the spinner permanently.

162 // without resetting it would wedge the spinner permanently.

160 guard let client = appState.client else { isLoading = false; return }

163 guard let client = appState.client else { isLoading = false; return }

164+ // `getAbout()` is cached for 60 s; the Refresh button must bypass that.

165+ if forceRefresh {

166+ await client.invalidateCache()

167+ }

161 let connID = appState.activeConnection?.id

168 let connID = appState.activeConnection?.id

169+ loadGeneration &+= 1

170+ let generation = loadGeneration

162 isLoading = true

171 isLoading = true

163 errorMessage = nil

172 errorMessage = nil

164 // Seed from the app-wide cache so first load isn't blank, but never fall

173 // Seed from the app-wide cache so first load isn't blank, but never fall

@@ -189,6 +198,7 @@ struct ServerInfoView: View {

189 // load for the previous server can't overwrite the current one's state.

198 // load for the previous server can't overwrite the current one's state.

190 // Still clear isLoading: this task owns the spinner it set, and leaving it

199 // Still clear isLoading: this task owns the spinner it set, and leaving it

191 // true on the stale path wedges the ProgressView and disables Refresh.

200 // true on the stale path wedges the ProgressView and disables Refresh.

201+ guard generation == loadGeneration else { return }

192 guard connID == appState.activeConnection?.id else { isLoading = false; return }

202 guard connID == appState.activeConnection?.id else { isLoading = false; return }

193 isHealthy = healthy

203 isHealthy = healthy

194 about = aboutValue

204 about = aboutValue

ParseableViewer/Views/StreamDetailView.swift+18−4

@@ -8,6 +8,9 @@ struct StreamDetailView: View {

8 @State private var retention: [RetentionConfig] = []

9 @State private var isLoading = false

10 @State private var errorMessage: String?

11+ /// Bumped by every load. A superseded load (its `.task(id:)` cancelled by a

12+ /// stream switch) still resumes, and must not clear the newer load's spinner.

13+ @State private var loadGeneration = 0

12 var body: some View {

15 var body: some View {

13 Group {

16 Group {

@@ -25,7 +28,7 @@ struct StreamDetailView: View {

25 Spacer()

28 Spacer()

27 Button {

30 Button {

28− Task { await loadData(stream: stream) }

31+ Task { await loadData(stream: stream, forceRefresh: true) }

29 } label: {

32 } label: {

30 Image(systemName: "arrow.clockwise")

33 Image(systemName: "arrow.clockwise")

31 }

34 }

@@ -44,7 +47,7 @@ struct StreamDetailView: View {

44 Text(error)

47 Text(error)

45 Spacer()

48 Spacer()

46 Button("Retry") {

49 Button("Retry") {

47− Task { await loadData(stream: stream) }

50+ Task { await loadData(stream: stream, forceRefresh: true) }

48 }

51 }

49 .buttonStyle(.bordered)

52 .buttonStyle(.bordered)

50 .controlSize(.small)

53 .controlSize(.small)

@@ -174,13 +177,22 @@ struct StreamDetailView: View {

174 }

177 }

175 }

178 }

176

179

177− private func loadData(stream: String) async {

180+ private func loadData(stream: String, forceRefresh: Bool = false) async {

178 guard let client = appState.client else { return }

181 guard let client = appState.client else { return }

182+ // The client caches schema/stats/info for 60 s. That's right for the

183+ // automatic load on appear, but an explicit Refresh/Retry must hit the

184+ // server — otherwise it repaints the same cached numbers and looks like a

185+ // refresh that found nothing new.

186+ if forceRefresh {

187+ await client.invalidateCache()

188+ }

179 // Capture the connection this fetch belongs to. Two servers can expose a

189 // Capture the connection this fetch belongs to. Two servers can expose a

180 // same-named stream, so guarding on the stream name alone would let a

190 // same-named stream, so guarding on the stream name alone would let a

181 // slow fetch write the previous server's data under the new server's

191 // slow fetch write the previous server's data under the new server's

182 // stream after a server switch.

192 // stream after a server switch.

183 let connID = appState.activeConnection?.id

193 let connID = appState.activeConnection?.id

194+ loadGeneration &+= 1

195+ let generation = loadGeneration

184 isLoading = true

196 isLoading = true

185 errorMessage = nil

197 errorMessage = nil

186 // Clear previous stream's data so it isn't shown under the new header

198 // Clear previous stream's data so it isn't shown under the new header

@@ -216,7 +228,9 @@ struct StreamDetailView: View {

216 // resolve after the user switched. Drop stale results instead of writing

228 // resolve after the user switched. Drop stale results instead of writing

217 // them under the new stream's header. Also check the connection so a

229 // them under the new stream's header. Also check the connection so a

218 // same-named stream on a different server doesn't accept the old server's

230 // same-named stream on a different server doesn't accept the old server's

219− // data. Reset `isLoading` on the stale path or the spinner wedges forever.

231+ // data. Reset `isLoading` on the stale path or the spinner wedges forever —

232+ // unless a newer load has started, which owns the spinner now.

233+ guard generation == loadGeneration else { return }

220 guard stream == appState.selectedStream, connID == appState.activeConnection?.id else {

234 guard stream == appState.selectedStream, connID == appState.activeConnection?.id else {

221 isLoading = false

235 isLoading = false

222 return

236 return

ParseableViewer/Views/UsersView.swift+6

@@ -5,6 +5,9 @@ struct UsersView: View {

5 @State private var users: [UserInfo] = []

6 @State private var isLoading = false

7 @State private var errorMessage: String?

8+ /// Bumped by every load so a superseded (cancelled) load can't clear the

9+ /// newer load's spinner.

10+ @State private var loadGeneration = 0

9 var body: some View {

12 var body: some View {

10 VStack(spacing: 0) {

13 VStack(spacing: 0) {

@@ -82,6 +85,8 @@ struct UsersView: View {

82 private func loadUsers() async {

85 private func loadUsers() async {

83 guard let client = appState.client else { return }

86 guard let client = appState.client else { return }

84 let connID = appState.activeConnection?.id

87 let connID = appState.activeConnection?.id

88+ loadGeneration &+= 1

89+ let generation = loadGeneration

85 isLoading = true

90 isLoading = true

86 errorMessage = nil

91 errorMessage = nil

87 // Clear the previous server's list up front so it can't briefly show through

92 // Clear the previous server's list up front so it can't briefly show through

@@ -102,6 +107,7 @@ struct UsersView: View {

102 // Drop results if the active connection changed while awaiting. Still clear

107 // Drop results if the active connection changed while awaiting. Still clear

103 // isLoading: this task set the spinner, so leaving it true on the stale path

108 // isLoading: this task set the spinner, so leaving it true on the stale path

104 // wedges the ProgressView and disables Refresh.

109 // wedges the ProgressView and disables Refresh.

110+ guard generation == loadGeneration else { return }

105 guard connID == appState.activeConnection?.id else { isLoading = false; return }

111 guard connID == appState.activeConnection?.id else { isLoading = false; return }

106 if let loaded { users = loaded }

112 if let loaded { users = loaded }

107 errorMessage = loadError

113 errorMessage = loadError