Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ internal extension MainSplitViewController {
containerTarget: workspace.connection.flatMap {
PluginManager.shared.containerSwitchTarget(for: $0.type)
},
executionOwner: sessionState.coordinator,
execution: TabExecutionObservation(owner: sessionState.coordinator),
onNewTab: { [weak workspace] in
workspace?.sessionState?.coordinator.commandActions?.newTab()
}
Expand Down
24 changes: 24 additions & 0 deletions TablePro/Extensions/View+OnValueChange.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,30 @@ internal extension View {
) -> some View {
modifier(PairedValueChangeModifier(value: value, action: action))
}

/// The same, for a value that belongs to an object this view does not observe.
///
/// A body that reads `parent.child.value` never hears the child change: `@Published` on the
/// child publishes on the child alone, and observing the parent covers only the parent's own
/// properties. The modifier observes the object instead of the view, so the object's other
/// changes redraw nothing but the modifier.
func onValueChange<Object: ObservableObject, Value: Equatable>(
of keyPath: KeyPath<Object, Value>,
in object: Object,
_ action: @escaping (Value, Value) -> Void
) -> some View {
modifier(ObservedValueChangeModifier(object: object, keyPath: keyPath, action: action))
}
}

private struct ObservedValueChangeModifier<Object: ObservableObject, Value: Equatable>: ViewModifier {
@ObservedObject var object: Object
let keyPath: KeyPath<Object, Value>
let action: (Value, Value) -> Void

func body(content: Content) -> some View {
content.onValueChange(of: object[keyPath: keyPath], action)
}
}

private struct PairedValueChangeModifier<Value: Equatable>: ViewModifier {
Expand Down
34 changes: 34 additions & 0 deletions TablePro/Views/ConnectionForm/ConnectionFormCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ final class ConnectionFormCoordinator: ObservableObject {

@Published private var temporaryTestIds: Set<UUID> = []

private var childChangeForwarding: AnyCancellable?

let services: AppServices
var storage: ConnectionStorage { services.connectionStorage }
@Published var dismissAction: (() -> Void)?
Expand Down Expand Up @@ -137,6 +139,38 @@ final class ConnectionFormCoordinator: ObservableObject {
customization.coordinator = ref
advanced.coordinator = ref
aiRules.coordinator = ref

childChangeForwarding = forwardChildChanges()
}

/// Every pane observes this coordinator and reads its values through a child, as in
/// `coordinator.network.type`. `@Published` on a child only fires when the reference is
/// replaced, never when a value inside it changes, so without this a pane stayed as it was
/// drawn: picking Sentinel left the Redis mode picker on Standalone with the host field still
/// showing. The send is synchronous because SwiftUI needs `objectWillChange` before the value
/// lands, and `switchToLatest` moves the subscription to a replacement child.
private func forwardChildChanges() -> AnyCancellable {
Publishers.MergeMany([
Self.changes(of: $network),
Self.changes(of: $auth),
Self.changes(of: $ssh),
Self.changes(of: $remoteFile),
Self.changes(of: $cloudflareTunnel),
Self.changes(of: $cloudSQLProxy),
Self.changes(of: $socksProxy),
Self.changes(of: $tunnelCommand),
Self.changes(of: $ssl),
Self.changes(of: $customization),
Self.changes(of: $advanced),
Self.changes(of: $aiRules),
])
.sink { [weak self] in self?.objectWillChange.send() }
}

private static func changes<Child: ObservableObject>(
of child: Published<Child>.Publisher
) -> AnyPublisher<Void, Never> where Child.ObjectWillChangePublisher == ObservableObjectPublisher {
child.map(\.objectWillChange).switchToLatest().eraseToAnyPublisher()
}

/// Performs the one-time side-effecting setup: applying initial type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,19 +63,15 @@ internal final class ConnectionFormSplitViewController: NSSplitViewController {
/// The window title follows the connection's type, which the Change… button can now alter.
///
/// `NSWindow(contentViewController:)` binds the window's title to this controller's, so nothing
/// writes `window.title` directly. `withObservationTracking` fires once per change, so the
/// closure re-arms itself.
/// writes `window.title` directly. The coordinator passes on its children's changes, so its own
/// publisher covers a type change in `network`.
private func trackTitle() {
title = windowTitle
titleObservations = [
coordinator.onMainActorChange { [weak self] in
guard let self else { return }
self.title = self.windowTitle
},
coordinator.network.onMainActorChange { [weak self] in
guard let self else { return }
self.title = self.windowTitle
},
]
}

Expand Down
13 changes: 4 additions & 9 deletions TablePro/Views/Main/EditorTabStrip.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,11 @@ internal struct EditorTabStrip: View {
/// shares a title with. Resolved by the window, because a view has no business asking the
/// plugin registry what kind of container a connection has.
internal let containerTarget: ContainerSwitchTarget?
/// Which tabs are running something. Read from the coordinator rather than pushed in, because
/// `tabExecution` is a stored property of an `@Observable`, so a claim opening or settling
/// invalidates this strip the same way it invalidates the result pane. A tab that is not the
/// Which tabs are running something. Observed rather than pushed in, so a claim opening or
/// settling redraws this strip the same way it redraws the result pane. A tab that is not the
/// selected one has no status bar on screen, and its progress used to show as the window-wide
/// spinner in the centre of the toolbar.
///
/// Weak for the reason `MainWindowToolbar.coordinator` is: the coordinator leaves
/// `activeCoordinators` only on deinit, so a strong reference held by a pane that outlives the
/// workspace would keep a torn-down connection voting in every aggregate that walks it.
internal weak var executionOwner: MainContentCoordinator?
@ObservedObject internal var execution: TabExecutionObservation
internal let onNewTab: () -> Void
/// Left unset by the app, which reads the two accessibility settings instead. A test sets it,
/// because glass does not rasterise.
Expand Down Expand Up @@ -182,7 +177,7 @@ internal struct EditorTabStrip: View {
),
position: index + 1,
count: tabs.count,
isBusy: executionOwner?.tabExecution.isBusy(tab.id) ?? false,
isBusy: execution.isBusy(tab.id),
commands: interaction.commands
)
.opacity(opacity(of: tab))
Expand Down
4 changes: 2 additions & 2 deletions TablePro/Views/Main/MainContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ struct MainContentView: View {
/// fields rendering changes the row under it without moving anything `InspectorTrigger`
/// watches. Rebuilding on the switch is enough: the two renderings are never on screen
/// together, so the stale snapshot is only ever reached by switching to it.
.onChange(of: trailingPaneState.inspector.viewMode) { _ in
.onValueChange(of: \.viewMode, in: trailingPaneState.inspector) { _, _ in
updateInspectorContext()
}
/// A value window detached from a field goes on writing while the JSON rendering is the
Expand Down Expand Up @@ -415,7 +415,7 @@ struct MainContentView: View {
handleConnectionStatusChange()
}

.onValueChange(of: coordinator.windowSidebarState.selectedTables) { oldTables, newTables in
.onValueChange(of: \.selectedTables, in: coordinator.windowSidebarState) { oldTables, newTables in
guard !coordinator.isTearingDown else {
Self.lifecycleLogger.debug("[switch] windowSidebarState.selectedTables SKIPPED (tearingDown) windowId=\(windowId, privacy: .public)")
return
Expand Down
34 changes: 34 additions & 0 deletions TablePro/Views/Main/TabExecutionObservation.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//
// TabExecutionObservation.swift
// TablePro
//

import Combine
import Foundation

/// Which tabs are running something, for a view that must not keep the coordinator alive.
///
/// The coordinator leaves `activeCoordinators` only on deinit, so a strong reference held by a pane
/// that outlives the workspace keeps a torn-down connection voting in every aggregate that walks it.
/// A weak property cannot be `@ObservedObject`, and a view that reads `tabExecution` through one
/// never hears a claim open or settle: a tab that is not the selected one kept its spinner, or never
/// showed one, until something unrelated redrew the strip.
///
/// This relays `tabExecution` alone rather than the whole coordinator, which also publishes the
/// cursor on every keystroke.
@MainActor
internal final class TabExecutionObservation: ObservableObject {
private weak var owner: MainContentCoordinator?
private var subscription: AnyCancellable?

internal init(owner: MainContentCoordinator?) {
self.owner = owner
subscription = owner?.$tabExecution
.dropFirst()
.sink { [weak self] _ in self?.objectWillChange.send() }
}

internal func isBusy(_ tabId: UUID) -> Bool {
owner?.tabExecution.isBusy(tabId) ?? false
}
}
102 changes: 102 additions & 0 deletions TableProTests/Extensions/ObservedValueChangeTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
//
// ObservedValueChangeTests.swift
// TableProTests
//
// A view that observes a parent and reacts to `parent.child.value` never hears the child change,
// because the child's `@Published` values publish on the child alone. The main window reacts to
// the sidebar selection and the inspector's view mode that way, each held by a child object.
//

import AppKit
import Combine
import SwiftUI
import Testing

@testable import TablePro

@MainActor
@Suite("Observed value change")
struct ObservedValueChangeTests {
private final class Child: ObservableObject {
@Published var value = 0
}

private final class Parent: ObservableObject {
@Published var title = ""
let child = Child()
}

private final class Changes {
var pairs: [[Int]] = []
}

private struct ObservedProbe: View {
@ObservedObject var parent: Parent
let changes: Changes

var body: some View {
Color.clear.onValueChange(of: \.value, in: parent.child) { old, new in
changes.pairs.append([old, new])
}
}
}

private struct ReadThroughProbe: View {
@ObservedObject var parent: Parent
let changes: Changes

var body: some View {
Color.clear.onValueChange(of: parent.child.value) { old, new in
changes.pairs.append([old, new])
}
}
}

private func host(_ view: some View) -> NSWindow {
let window = NSWindow(
contentRect: NSRect(x: 0, y: 0, width: 40, height: 40),
styleMask: [.titled],
backing: .buffered,
defer: false
)
window.isReleasedWhenClosed = false
window.contentView = NSHostingView(rootView: view)
settle(window)
return window
}

private func settle(_ window: NSWindow) {
for _ in 0 ..< 10 {
window.contentView?.layoutSubtreeIfNeeded()
RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.02))
}
}

@Test("a change inside the child reaches the action with both values")
func childChangeReachesAction() {
let parent = Parent()
let changes = Changes()
let window = host(ObservedProbe(parent: parent, changes: changes))
defer { window.orderOut(nil) }

parent.child.value = 1
settle(window)
parent.child.value = 2
settle(window)

#expect(changes.pairs == [[0, 1], [1, 2]])
}

@Test("reading the value through the parent misses the change")
func readingThroughParentMissesTheChange() {
let parent = Parent()
let changes = Changes()
let window = host(ReadThroughProbe(parent: parent, changes: changes))
defer { window.orderOut(nil) }

parent.child.value = 1
settle(window)

#expect(changes.pairs.isEmpty)
}
}
80 changes: 80 additions & 0 deletions TableProTests/ViewModels/ConnectionFormChildObservationTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
//
// ConnectionFormChildObservationTests.swift
// TableProTests
//
// Every connection form pane observes the coordinator and reads its values through a child view
// model. A change inside a child that never reached the coordinator's publisher left the pane as
// it was drawn, which is how picking Sentinel kept the Redis mode picker on Standalone.
//

import Combine
import Foundation
import Testing

@testable import TablePro

@MainActor
@Suite("Connection form child observation")
struct ConnectionFormChildObservationTests {
private final class ChangeCounter {
private(set) var sends = 0
private var subscription: AnyCancellable?

init(_ coordinator: ConnectionFormCoordinator) {
subscription = coordinator.objectWillChange.sink { [weak self] in self?.sends += 1 }
}
}

private func children(of coordinator: ConnectionFormCoordinator) -> [(String, ObservableObjectPublisher)] {
[
("network", coordinator.network.objectWillChange),
("auth", coordinator.auth.objectWillChange),
("ssh", coordinator.ssh.objectWillChange),
("remoteFile", coordinator.remoteFile.objectWillChange),
("cloudflareTunnel", coordinator.cloudflareTunnel.objectWillChange),
("cloudSQLProxy", coordinator.cloudSQLProxy.objectWillChange),
("socksProxy", coordinator.socksProxy.objectWillChange),
("tunnelCommand", coordinator.tunnelCommand.objectWillChange),
("ssl", coordinator.ssl.objectWillChange),
("customization", coordinator.customization.objectWillChange),
("advanced", coordinator.advanced.objectWillChange),
("aiRules", coordinator.aiRules.objectWillChange),
]
}

@Test("a value changed inside a child reaches the coordinator's observers")
func childValueChangeReachesCoordinator() {
let coordinator = ConnectionFormCoordinator(connectionId: nil)
let counter = ChangeCounter(coordinator)

coordinator.network.additionalFieldValues["redisMode"] = "sentinel"

#expect(counter.sends == 1)
}

@Test("every child forwards its changes")
func everyChildForwards() {
let coordinator = ConnectionFormCoordinator(connectionId: nil)

for (name, publisher) in children(of: coordinator) {
let counter = ChangeCounter(coordinator)
publisher.send()
#expect(counter.sends == 1, "\(name) did not reach the coordinator")
}
}

@Test("a replaced child forwards, and the one it replaced goes quiet")
func replacedChildForwards() {
let coordinator = ConnectionFormCoordinator(connectionId: nil)
let replaced = coordinator.network
let replacement = NetworkPaneViewModel()
coordinator.network = replacement
let counter = ChangeCounter(coordinator)

replaced.type = .postgresql
#expect(counter.sends == 0)

replacement.type = .postgresql
#expect(counter.sends == 1)
}
}
2 changes: 1 addition & 1 deletion TableProTests/Views/Main/EditorTabStripChromeTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ struct EditorTabStripChromeTests {
tabManager: manager,
interaction: interaction,
containerTarget: nil,
executionOwner: nil,
execution: TabExecutionObservation(owner: nil),
onNewTab: {},
surfaceStyle: .solid
)
Expand Down
Loading
Loading