From 69cee7c266f18002a193efcafb1d28392613305c Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 4 Jul 2026 15:00:20 +0700 Subject: [PATCH 1/2] fix(plugin-oracle): stop infinite loading when switching schemas (#1807) --- CHANGELOG.md | 4 + .../OracleDriverPlugin/OracleConnection.swift | 271 +++++++++++------- Plugins/OracleDriverPlugin/OraclePlugin.swift | 25 +- .../Plugins/PluginManager+Registration.swift | 3 + ...ginMetadataRegistry+RegistryDefaults.swift | 8 +- .../Query/MetadataConnectionPool.swift | 40 ++- .../Core/Services/Query/SchemaService.swift | 6 + .../Connection/ConnectionToolbarState.swift | 7 +- .../Views/Main/MainContentCoordinator.swift | 1 + TablePro/Views/Sidebar/SidebarView.swift | 4 + .../Autocomplete/SQLSchemaProviderTests.swift | 11 +- .../Core/Database/DatabaseManagerTests.swift | 61 +++- .../Plugins/ContainerEntityNameTests.swift | 15 + .../Query/MetadataConnectionPoolTests.swift | 53 ++++ .../Services/Query/SchemaServiceTests.swift | 27 ++ .../Views/SwitchContainerTests.swift | 44 +++ 16 files changed, 452 insertions(+), 128 deletions(-) create mode 100644 TableProTests/Core/Services/Query/MetadataConnectionPoolTests.swift create mode 100644 TableProTests/Views/SwitchContainerTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 065d5956a..251c568d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Switching schemas on an Oracle connection no longer hangs on an infinite loading spinner. Oracle now switches by schema like BigQuery, the sidebar lists every schema with its tables loading on expand, Oracle queries respect the query timeout setting and can be cancelled, and a schema load that fails now shows an error with a Retry button instead of spinning forever. Requires the updated Oracle plugin for the full fix. (#1807) + ## [0.55.0] - 2026-07-04 ### Added diff --git a/Plugins/OracleDriverPlugin/OracleConnection.swift b/Plugins/OracleDriverPlugin/OracleConnection.swift index ffbfd208b..41bc47273 100644 --- a/Plugins/OracleDriverPlugin/OracleConnection.swift +++ b/Plugins/OracleDriverPlugin/OracleConnection.swift @@ -29,6 +29,7 @@ struct OracleError: Error { case authVersionNotSupported case authConnectionDropped(phase: String?) case loginTimedOut + case queryTimedOut case nativeEncryptionFailed } @@ -127,6 +128,7 @@ final class OracleConnectionWrapper: @unchecked Sendable { private struct LockedState: Sendable { var isConnected = false var nioConnection: OracleNIO.OracleConnection? + var queryTimeoutSeconds = 0 } private let state = OSAllocatedUnfairLock(initialState: LockedState()) @@ -297,6 +299,8 @@ final class OracleConnectionWrapper: @unchecked Sendable { return String(localized: "This account uses a password verifier the database driver does not support.") case .loginTimedOut: return loginTimeoutMessage + case .queryTimedOut: + return queryTimeoutMessage case .nativeEncryptionFailed: return nativeEncryptionFailureMessage case .generic, .notConnected, .connectionFailed, .queryFailed, .protocolError: @@ -304,6 +308,10 @@ final class OracleConnectionWrapper: @unchecked Sendable { } } + private static let queryTimeoutMessage = String( + localized: "The query did not finish within the configured timeout, so the connection was reset. Run the query again." + ) + private func mapQueryError(_ sqlError: OracleSQLError) -> OracleError { guard Self.isChannelFatal(sqlError) else { return OracleError(message: sqlError.serverInfo?.message ?? sqlError.description) @@ -340,71 +348,71 @@ final class OracleConnectionWrapper: @unchecked Sendable { } } + func applyQueryTimeout(_ seconds: Int) { + state.withLock { $0.queryTimeoutSeconds = max(0, seconds) } + } + + func abortCurrentQuery() { + guard isConnected else { return } + osLogger.notice("Aborting Oracle query by closing the connection; the driver has no server-side cancel") + disconnect() + } + // MARK: - Query Execution - func executeQuery(_ query: String) async throws -> OracleQueryResult { - let connection = try state.withLock { current -> OracleNIO.OracleConnection in + private func requireConnection() throws -> OracleNIO.OracleConnection { + try state.withLock { current in guard let conn = current.nioConnection, current.isConnected else { throw OracleError.notConnected } return conn } + } + + /// Races the operation against the configured query timeout. On timeout the + /// connection is closed first, which fails the in-flight OracleNIO call even + /// if it ignores task cancellation, so the group can always unwind. + private func withQueryDeadline( + _ operation: @escaping @Sendable () async throws -> T + ) async throws -> T { + let timeoutSeconds = state.withLock { $0.queryTimeoutSeconds } + guard timeoutSeconds > 0 else { return try await operation() } + + return try await withThrowingTaskGroup(of: T?.self) { group in + group.addTask { try await operation() } + group.addTask { + try await Task.sleep(nanoseconds: UInt64(timeoutSeconds) * 1_000_000_000) + return nil + } + defer { group.cancelAll() } + guard let first = try await group.next(), let result = first else { + disconnect() + throw TimeoutError(seconds: Double(timeoutSeconds)) + } + return result + } + } + + private func queryTimeoutError(_ timeout: TimeoutError) -> OracleError { + osLogger.error("Oracle query timed out after \(Int(timeout.seconds), privacy: .public)s; the connection was closed to recover") + return OracleError(message: Self.queryTimeoutMessage, category: .queryTimedOut) + } + + func executeQuery(_ query: String) async throws -> OracleQueryResult { + let connection = try requireConnection() // OracleNIO does not support concurrent queries on a single connection. // Serialize all queries to prevent state-machine corruption. await queryGate.acquire() do { - let statement = OracleStatement(stringLiteral: query) - let stream = try await connection.execute(statement, logger: nioLogger) - - // Read column metadata from stream (available even with 0 rows) - var columns: [String] = [] - for col in stream.columns { - columns.append(col.name) - } - osLogger.debug("Oracle columns: \(columns.count) — \(columns.joined(separator: ", "))") - - var columnTypeNames: [String] = [] - var allRows: [[PluginCellValue]] = [] - var didReadTypes = false - var truncated = false - - for try await row in stream { - var rowValues: [PluginCellValue] = [] - for cell in row { - if !didReadTypes { - columnTypeNames.append(oracleTypeName(cell.dataType)) - } - if cell.bytes == nil { - rowValues.append(.null) - } else if cell.dataType == .raw || cell.dataType == .longRAW || cell.dataType == .blob, - let bytes = cell.bytes { - rowValues.append(.bytes(Data(bytes.readableBytesView))) - } else { - rowValues.append(PluginCellValue.fromOptional(decodeCell(cell))) - } - } - didReadTypes = true - allRows.append(rowValues) - if allRows.count >= PluginRowLimits.emergencyMax { - truncated = true - break - } - } - - if !didReadTypes { - columnTypeNames = Array(repeating: "unknown", count: columns.count) + let result = try await withQueryDeadline { [self] in + try await collectRows(query, on: connection) } - await queryGate.release() - return OracleQueryResult( - columns: columns, - columnTypeNames: columnTypeNames, - rows: allRows, - affectedRows: allRows.count, - isTruncated: truncated - ) + return result + } catch let timeout as TimeoutError { + throw queryTimeoutError(timeout) } catch let sqlError as OracleSQLError { await queryGate.release() throw mapQueryError(sqlError) @@ -420,85 +428,142 @@ final class OracleConnectionWrapper: @unchecked Sendable { } } + private func collectRows( + _ query: String, + on connection: OracleNIO.OracleConnection + ) async throws -> OracleQueryResult { + let statement = OracleStatement(stringLiteral: query) + let stream = try await connection.execute(statement, logger: nioLogger) + + // Read column metadata from stream (available even with 0 rows) + var columns: [String] = [] + for col in stream.columns { + columns.append(col.name) + } + osLogger.debug("Oracle columns (\(columns.count)): \(columns.joined(separator: ", "))") + + var columnTypeNames: [String] = [] + var allRows: [[PluginCellValue]] = [] + var didReadTypes = false + var truncated = false + + for try await row in stream { + var rowValues: [PluginCellValue] = [] + for cell in row { + if !didReadTypes { + columnTypeNames.append(oracleTypeName(cell.dataType)) + } + if cell.bytes == nil { + rowValues.append(.null) + } else if cell.dataType == .raw || cell.dataType == .longRAW || cell.dataType == .blob, + let bytes = cell.bytes { + rowValues.append(.bytes(Data(bytes.readableBytesView))) + } else { + rowValues.append(PluginCellValue.fromOptional(decodeCell(cell))) + } + } + didReadTypes = true + allRows.append(rowValues) + if allRows.count >= PluginRowLimits.emergencyMax { + truncated = true + break + } + } + + if !didReadTypes { + columnTypeNames = Array(repeating: "unknown", count: columns.count) + } + + return OracleQueryResult( + columns: columns, + columnTypeNames: columnTypeNames, + rows: allRows, + affectedRows: allRows.count, + isTruncated: truncated + ) + } + // MARK: - Streaming Query func streamQuery( _ query: String, continuation: AsyncThrowingStream.Continuation ) async throws { - let connection = try state.withLock { current -> OracleNIO.OracleConnection in - guard let conn = current.nioConnection, current.isConnected else { - throw OracleError.notConnected - } - return conn - } + let connection = try requireConnection() await queryGate.acquire() do { - let statement = OracleStatement(stringLiteral: query) - let stream = try await connection.execute(statement, logger: nioLogger) - - var columns: [String] = [] - for col in stream.columns { - columns.append(col.name) + try await withQueryDeadline { [self] in + try await streamRows(query, on: connection, continuation: continuation) } + await queryGate.release() + continuation.finish() + } catch let timeout as TimeoutError { + throw queryTimeoutError(timeout) + } catch let sqlError as OracleSQLError { + await queryGate.release() + throw mapQueryError(sqlError) + } catch is CancellationError { + await queryGate.release() + throw CancellationError() + } catch { + await queryGate.release() + throw OracleError(message: "Query execution failed: \(String(describing: error))") + } + } + + private func streamRows( + _ query: String, + on connection: OracleNIO.OracleConnection, + continuation: AsyncThrowingStream.Continuation + ) async throws { + let statement = OracleStatement(stringLiteral: query) + let stream = try await connection.execute(statement, logger: nioLogger) - var columnTypeNames: [String] = [] - var headerSent = false + var columns: [String] = [] + for col in stream.columns { + columns.append(col.name) + } - for try await row in stream { - if Task.isCancelled { - await queryGate.release() - continuation.finish(throwing: CancellationError()) - return - } + var columnTypeNames: [String] = [] + var headerSent = false - var rowValues: [PluginCellValue] = [] - for cell in row { - if !headerSent { - columnTypeNames.append(oracleTypeName(cell.dataType)) - } - if cell.bytes == nil { - rowValues.append(.null) - } else if cell.dataType == .raw || cell.dataType == .longRAW || cell.dataType == .blob, - let bytes = cell.bytes { - rowValues.append(.bytes(Data(bytes.readableBytesView))) - } else { - rowValues.append(PluginCellValue.fromOptional(decodeCell(cell))) - } - } + for try await row in stream { + try Task.checkCancellation() + var rowValues: [PluginCellValue] = [] + for cell in row { if !headerSent { - continuation.yield(.header(PluginStreamHeader( - columns: columns, - columnTypeNames: columnTypeNames - ))) - headerSent = true + columnTypeNames.append(oracleTypeName(cell.dataType)) + } + if cell.bytes == nil { + rowValues.append(.null) + } else if cell.dataType == .raw || cell.dataType == .longRAW || cell.dataType == .blob, + let bytes = cell.bytes { + rowValues.append(.bytes(Data(bytes.readableBytesView))) + } else { + rowValues.append(PluginCellValue.fromOptional(decodeCell(cell))) } - - continuation.yield(.rows([rowValues])) } if !headerSent { - columnTypeNames = Array(repeating: "unknown", count: columns.count) continuation.yield(.header(PluginStreamHeader( columns: columns, columnTypeNames: columnTypeNames ))) + headerSent = true } - await queryGate.release() - continuation.finish() - } catch let sqlError as OracleSQLError { - await queryGate.release() - throw mapQueryError(sqlError) - } catch is CancellationError { - await queryGate.release() - throw CancellationError() - } catch { - await queryGate.release() - throw OracleError(message: "Query execution failed: \(String(describing: error))") + continuation.yield(.rows([rowValues])) + } + + if !headerSent { + columnTypeNames = Array(repeating: "unknown", count: columns.count) + continuation.yield(.header(PluginStreamHeader( + columns: columns, + columnTypeNames: columnTypeNames + ))) } } diff --git a/Plugins/OracleDriverPlugin/OraclePlugin.swift b/Plugins/OracleDriverPlugin/OraclePlugin.swift index b4e8465da..41b54d51e 100644 --- a/Plugins/OracleDriverPlugin/OraclePlugin.swift +++ b/Plugins/OracleDriverPlugin/OraclePlugin.swift @@ -54,11 +54,14 @@ final class OraclePlugin: NSObject, TableProPlugin, DriverPlugin, PluginDiagnost static let supportsTriggerEditing = true static let pathFieldRole: PathFieldRole = .serviceName static let supportsForeignKeyDisable = false + static let supportsDatabaseSwitching = false static let supportsSchemaSwitching = true + static let defaultSchemaName = "" + static let containerEntityName = "Schema" static let postConnectActions: [PostConnectAction] = [.selectSchemaFromLastSession] static let brandColorHex = "#C3160B" static let systemDatabaseNames: [String] = ["SYS", "SYSTEM", "OUTLN", "DBSNMP", "APPQOSSYS", "WMSYS", "XDB"] - static let databaseGroupingStrategy: GroupingStrategy = .bySchema + static let databaseGroupingStrategy: GroupingStrategy = .hierarchicalSchema static let columnTypesByCategory: [String: [String]] = [ "Integer": ["NUMBER", "INTEGER", "INT", "SMALLINT"], "Float": ["FLOAT", "BINARY_FLOAT", "BINARY_DOUBLE", "DECIMAL", "NUMERIC", "REAL", "DOUBLE PRECISION"], @@ -201,6 +204,17 @@ final class OraclePlugin: NSObject, TableProPlugin, DriverPlugin, PluginDiagnost ], supportURL: issuesURL ) + case .queryTimedOut: + return PluginDiagnostic( + title: String(localized: "Query Timed Out"), + message: oracleError.message, + suggestedActions: [ + String(localized: "Run the query again. TablePro reconnects to the server automatically."), + String(localized: "If the query legitimately needs more time, raise the query timeout in Settings > General."), + String(localized: "If a metadata query timed out, the schema may hold a very large number of objects; try again once the server is less busy.") + ], + supportURL: issuesURL + ) case .generic, .notConnected, .connectionFailed, .queryFailed: return nil } @@ -225,6 +239,7 @@ final class OraclePluginDriver: PluginDatabaseDriver, @unchecked Sendable { .transactions, .alterTableDDL, .multiSchema, + .cancelQuery, ] } @@ -286,6 +301,14 @@ final class OraclePluginDriver: PluginDatabaseDriver, @unchecked Sendable { _ = try await execute(query: "SELECT 1 FROM DUAL") } + func cancelQuery() throws { + oracleConn?.abortCurrentQuery() + } + + func applyQueryTimeout(_ seconds: Int) async throws { + oracleConn?.applyQueryTimeout(seconds) + } + // MARK: - Transaction Management func beginTransaction() async throws { diff --git a/TablePro/Core/Plugins/PluginManager+Registration.swift b/TablePro/Core/Plugins/PluginManager+Registration.swift index 0cdb1bf24..ef383bbd7 100644 --- a/TablePro/Core/Plugins/PluginManager+Registration.swift +++ b/TablePro/Core/Plugins/PluginManager+Registration.swift @@ -38,6 +38,9 @@ extension PluginManager { from: driverType, isDownloadable: driverType.isDownloadable ) + if snapshot.schema.databaseGroupingStrategy == .hierarchicalSchema, snapshot.supportsDatabaseSwitching { + Self.logger.warning("Plugin '\(pluginId)' declares hierarchicalSchema grouping together with supportsDatabaseSwitching; schema-only engines must declare supportsDatabaseSwitching = false or the container switcher misroutes") + } PluginMetadataRegistry.shared.register(snapshot: snapshot, forTypeId: typeId, preserveIcon: true) for additionalId in driverType.additionalDatabaseTypeIds { var additionalSnapshot = snapshot diff --git a/TablePro/Core/Plugins/PluginMetadataRegistry+RegistryDefaults.swift b/TablePro/Core/Plugins/PluginMetadataRegistry+RegistryDefaults.swift index 35986e263..2ddbe7ac6 100644 --- a/TablePro/Core/Plugins/PluginMetadataRegistry+RegistryDefaults.swift +++ b/TablePro/Core/Plugins/PluginMetadataRegistry+RegistryDefaults.swift @@ -710,7 +710,7 @@ extension PluginMetadataRegistry { postConnectActions: [.selectSchemaFromLastSession], brandColorHex: "#C3160B", queryLanguageName: "SQL", editorLanguage: .sql, - connectionMode: .network, supportsDatabaseSwitching: true, + connectionMode: .network, supportsDatabaseSwitching: false, supportsColumnReorder: false, capabilities: PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: true, @@ -728,10 +728,10 @@ extension PluginMetadataRegistry { supportsOpportunisticTLS: false ), schema: PluginMetadataSnapshot.SchemaInfo( - defaultSchemaName: "public", + defaultSchemaName: "", defaultGroupName: "main", tableEntityName: "Tables", - containerEntityName: "Database", + containerEntityName: "Schema", defaultPrimaryKeyColumn: nil, immutableColumns: [], systemDatabaseNames: [ @@ -739,7 +739,7 @@ extension PluginMetadataRegistry { ], systemSchemaNames: [], fileExtensions: [], - databaseGroupingStrategy: .bySchema, + databaseGroupingStrategy: .hierarchicalSchema, structureColumnFields: [.name, .type, .nullable, .defaultValue, .autoIncrement, .comment] ), editor: PluginMetadataSnapshot.EditorConfig( diff --git a/TablePro/Core/Services/Query/MetadataConnectionPool.swift b/TablePro/Core/Services/Query/MetadataConnectionPool.swift index 8acb31bbb..45a80498c 100644 --- a/TablePro/Core/Services/Query/MetadataConnectionPool.swift +++ b/TablePro/Core/Services/Query/MetadataConnectionPool.swift @@ -4,6 +4,7 @@ // import Foundation +import TableProPluginKit @MainActor final class MetadataConnectionPool { @@ -53,7 +54,7 @@ final class MetadataConnectionPool { private var entries: [Key: Entry] = [:] private var pending: [Key: Task] = [:] private let maxPerConnection = 6 - private let connectTimeoutSeconds: UInt64 = 15 + private let operationTimeoutSeconds: Double = 15 private init() {} @@ -151,13 +152,13 @@ final class MetadataConnectionPool { awaitPlugins: true ) do { - try await connectWithTimeout(driver: driver, database: key.database) + try await Self.connect(driver, database: key.database, timeoutSeconds: operationTimeoutSeconds) try? await driver.applyQueryTimeout(AppSettingsManager.shared.general.queryTimeoutSeconds) await DatabaseManager.shared.executeStartupCommands( session.connection.startupCommands, on: driver, connectionName: session.connection.name ) if let schema = key.schema, let switchable = driver as? SchemaSwitchable { - try await switchable.switchSchema(to: schema) + try await Self.switchSchema(switchable, to: schema, timeoutSeconds: operationTimeoutSeconds) } } catch { driver.disconnect() @@ -166,18 +167,27 @@ final class MetadataConnectionPool { return Entry(driver: driver) } - private func connectWithTimeout(driver: DatabaseDriver, database: String) async throws { - let timeoutNanos = connectTimeoutSeconds * 1_000_000_000 - try await withThrowingTaskGroup(of: Void.self) { group in - group.addTask { try await driver.connect() } - group.addTask { - try await Task.sleep(nanoseconds: timeoutNanos) - throw DatabaseError.connectionFailed( - String(format: String(localized: "Connecting to '%@' timed out."), database) - ) - } - try await group.next() - group.cancelAll() + static func connect(_ driver: DatabaseDriver, database: String, timeoutSeconds: Double) async throws { + do { + try await withTimeout(seconds: timeoutSeconds) { try await driver.connect() } + } catch is TimeoutError { + throw DatabaseError.connectionFailed( + String(format: String(localized: "Connecting to '%@' timed out."), database) + ) + } + } + + static func switchSchema( + _ switchable: SchemaSwitchable, + to schema: String, + timeoutSeconds: Double + ) async throws { + do { + try await withTimeout(seconds: timeoutSeconds) { try await switchable.switchSchema(to: schema) } + } catch is TimeoutError { + throw DatabaseError.connectionFailed( + String(format: String(localized: "Switching to schema '%@' timed out."), schema) + ) } } diff --git a/TablePro/Core/Services/Query/SchemaService.swift b/TablePro/Core/Services/Query/SchemaService.swift index bdff6c584..0db39da52 100644 --- a/TablePro/Core/Services/Query/SchemaService.swift +++ b/TablePro/Core/Services/Query/SchemaService.swift @@ -214,6 +214,12 @@ final class SchemaService { await reload(connectionId: connectionId, driver: driver, connection: session.connection) } + func markLoadFailed(connectionId: UUID, message: String) { + if case .loaded = state(for: connectionId) { return } + states[connectionId] = .failed(message) + bumpGeneration(connectionId) + } + private func runLoad( connectionId: UUID, driver: DatabaseDriver, diff --git a/TablePro/Models/Connection/ConnectionToolbarState.swift b/TablePro/Models/Connection/ConnectionToolbarState.swift index 2c1983b71..43b91c0a1 100644 --- a/TablePro/Models/Connection/ConnectionToolbarState.swift +++ b/TablePro/Models/Connection/ConnectionToolbarState.swift @@ -187,9 +187,10 @@ final class ConnectionToolbarState { } /// Text shown in the toolbar's database/schema chip. For `.bySchema` engines - /// (SQL Server, PostgreSQL, Oracle, BigQuery), this is the active schema; for - /// `.byDatabase` and `.flat` engines, it is the active database. Falls back to - /// `currentDatabase` when a schema-grouped engine has not yet resolved its schema. + /// (SQL Server, PostgreSQL) and `.hierarchicalSchema` engines that switch by + /// schema (Oracle, BigQuery), this is the active schema; other engines show the + /// active database, which is also the fallback while a schema-grouped engine + /// has not yet resolved its schema. var chipText: String { switch databaseGroupingStrategy { case .bySchema: diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index a65cc3df0..787fe959f 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -615,6 +615,7 @@ final class MainContentCoordinator { } } catch { Self.logger.warning("Schema refresh failed: \(error.localizedDescription, privacy: .public)") + schemaService.markLoadFailed(connectionId: connectionId, message: error.localizedDescription) } let database = currentDatabaseOnly ? activeDatabaseName : nil await DatabaseTreeMetadataService.shared.refreshLoadedTables(connectionId: connectionId, database: database) diff --git a/TablePro/Views/Sidebar/SidebarView.swift b/TablePro/Views/Sidebar/SidebarView.swift index fd2aaf99d..f174349f3 100644 --- a/TablePro/Views/Sidebar/SidebarView.swift +++ b/TablePro/Views/Sidebar/SidebarView.swift @@ -300,6 +300,10 @@ struct SidebarView: View { .font(.caption) .foregroundStyle(.secondary) .multilineTextAlignment(.center) + Button("Retry") { + Task { await schemaService.refresh(connectionId: connectionId) } + } + .controlSize(.small) } .frame(maxWidth: .infinity, maxHeight: .infinity) .padding() diff --git a/TableProTests/Core/Autocomplete/SQLSchemaProviderTests.swift b/TableProTests/Core/Autocomplete/SQLSchemaProviderTests.swift index c974f6183..1a54049bb 100644 --- a/TableProTests/Core/Autocomplete/SQLSchemaProviderTests.swift +++ b/TableProTests/Core/Autocomplete/SQLSchemaProviderTests.swift @@ -29,12 +29,18 @@ final class MockDatabaseDriver: DatabaseDriver, SchemaSwitchable, @unchecked Sen var fetchSchemaTablesCalls: [String] = [] var applyQueryTimeoutValues: [Int] = [] var cancelQueryCallCount = 0 + var connectDelaySeconds: Double = 0 + var switchSchemaDelaySeconds: Double = 0 init(connection: DatabaseConnection = TestFixtures.makeConnection()) { self.connection = connection } - func connect() async throws {} + func connect() async throws { + guard connectDelaySeconds > 0 else { return } + try await Task.sleep(nanoseconds: UInt64(connectDelaySeconds * 1_000_000_000)) + } + func disconnect() {} func testConnection() async throws -> Bool { true } @@ -105,6 +111,9 @@ final class MockDatabaseDriver: DatabaseDriver, SchemaSwitchable, @unchecked Sen func rollbackTransaction() async throws {} func switchSchema(to schema: String) async throws { + if switchSchemaDelaySeconds > 0 { + try await Task.sleep(nanoseconds: UInt64(switchSchemaDelaySeconds * 1_000_000_000)) + } switchSchemaCallCount += 1 currentSchema = schema } diff --git a/TableProTests/Core/Database/DatabaseManagerTests.swift b/TableProTests/Core/Database/DatabaseManagerTests.swift index 32b91b3c2..565a8e6dd 100644 --- a/TableProTests/Core/Database/DatabaseManagerTests.swift +++ b/TableProTests/Core/Database/DatabaseManagerTests.swift @@ -6,8 +6,8 @@ // import Foundation -import TableProPluginKit @testable import TablePro +import TableProPluginKit import Testing @Suite("DatabaseManager Session-Scoped Accessors") @@ -68,3 +68,62 @@ struct DatabaseManagerSessionTests { #expect(DatabaseManager.shared.resolvedSchemaName(nil, for: connection.id) == nil) } } + +private class DatabaseSwitchBaseDriver { + var supportsSchemas: Bool { true } + var supportsTransactions: Bool { false } + var currentSchema: String? { nil } + var serverVersion: String? { nil } + + func connect() async throws {} + func disconnect() {} + + func execute(query: String) async throws -> PluginQueryResult { + PluginQueryResult(columns: [], columnTypeNames: [], rows: [], rowsAffected: 0, executionTime: 0) + } + + func fetchTables(schema: String?) async throws -> [PluginTableInfo] { [] } + func fetchColumns(table: String, schema: String?) async throws -> [PluginColumnInfo] { [] } + func fetchIndexes(table: String, schema: String?) async throws -> [PluginIndexInfo] { [] } + func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { [] } + func fetchTableDDL(table: String, schema: String?) async throws -> String { "" } + func fetchViewDefinition(view: String, schema: String?) async throws -> String { "" } + func fetchTableMetadata(table: String, schema: String?) async throws -> PluginTableMetadata { + PluginTableMetadata(tableName: table) + } + + func fetchDatabases() async throws -> [String] { [] } + func fetchDatabaseMetadata(_ database: String) async throws -> PluginDatabaseMetadata { + PluginDatabaseMetadata(name: database) + } +} + +private final class DatabaseSwitchingDriver: DatabaseSwitchBaseDriver, PluginDatabaseDriver { + private(set) var switchedDatabases: [String] = [] + + func switchDatabase(to database: String) async throws { + switchedDatabases.append(database) + } +} + +@Suite("DatabaseManager database switch") +@MainActor +struct DatabaseManagerDatabaseSwitchTests { + @Test("bySchema engines reset the session schema to the plugin default") + func bySchemaSwitchResetsSchemaToDefault() async throws { + let connection = TestFixtures.makeConnection(type: .mssql) + let pluginDriver = DatabaseSwitchingDriver() + let adapter = PluginDriverAdapter(connection: connection, pluginDriver: pluginDriver) + var session = ConnectionSession(connection: connection, driver: adapter) + session.currentSchema = "sales" + DatabaseManager.shared.injectSession(session, for: connection.id) + defer { DatabaseManager.shared.removeSession(for: connection.id) } + + try await DatabaseManager.shared.switchDatabase(to: "other_db", for: connection.id, persist: false) + + let updated = DatabaseManager.shared.session(for: connection.id) + #expect(pluginDriver.switchedDatabases == ["other_db"]) + #expect(updated?.currentDatabase == "other_db") + #expect(updated?.currentSchema == "dbo") + } +} diff --git a/TableProTests/Core/Plugins/ContainerEntityNameTests.swift b/TableProTests/Core/Plugins/ContainerEntityNameTests.swift index 078a737a4..d03f8e077 100644 --- a/TableProTests/Core/Plugins/ContainerEntityNameTests.swift +++ b/TableProTests/Core/Plugins/ContainerEntityNameTests.swift @@ -67,6 +67,21 @@ struct ContainerEntityNameTests { #expect(PluginManager.shared.containerSwitchTarget(for: .bigQuery) == .schema) } + @Test("Oracle switches schemas, not databases") + func oracleSwitchesSchemas() { + #expect(PluginManager.shared.containerSwitchTarget(for: .oracle) == .schema) + #expect(PluginManager.shared.supportsDatabaseSwitching(for: .oracle) == false) + #expect(PluginManager.shared.supportsSchemaSwitching(for: .oracle) == true) + } + + @Test("Oracle container is Schema with hierarchical grouping") + func oracleContainerIsSchema() { + #expect(PluginManager.shared.containerEntityName(for: .oracle) == "Schema") + #expect(PluginManager.shared.databaseGroupingStrategy(for: .oracle) == .hierarchicalSchema) + #expect(PluginManager.shared.supportsDatabaseTree(for: .oracle) == false) + #expect(snapshot(forTypeId: "Oracle")?.schema.defaultSchemaName == "") + } + @Test("Engines supporting both prefer databases") func dualModeEnginesPreferDatabases() { #expect(PluginManager.shared.containerSwitchTarget(for: .postgresql) == .database) diff --git a/TableProTests/Core/Services/Query/MetadataConnectionPoolTests.swift b/TableProTests/Core/Services/Query/MetadataConnectionPoolTests.swift new file mode 100644 index 000000000..75a125acd --- /dev/null +++ b/TableProTests/Core/Services/Query/MetadataConnectionPoolTests.swift @@ -0,0 +1,53 @@ +// +// MetadataConnectionPoolTests.swift +// TableProTests +// +// Tests for the pool's bounded connect and schema-switch steps: a driver +// that hangs must fail within the deadline instead of stalling the pool +// entry forever (#1807). +// + +import Foundation +@testable import TablePro +import Testing + +@Suite("MetadataConnectionPool timeouts") +@MainActor +struct MetadataConnectionPoolTests { + @Test("connect passes through when the driver responds in time") + func connectPassesThrough() async throws { + let driver = MockDatabaseDriver() + + try await MetadataConnectionPool.connect(driver, database: "db", timeoutSeconds: 1) + } + + @Test("connect fails with a connection error when the driver hangs") + func connectTimesOut() async { + let driver = MockDatabaseDriver() + driver.connectDelaySeconds = 5 + + await #expect(throws: DatabaseError.self) { + try await MetadataConnectionPool.connect(driver, database: "db", timeoutSeconds: 0.05) + } + } + + @Test("schema switch passes through when the driver responds in time") + func switchSchemaPassesThrough() async throws { + let driver = MockDatabaseDriver() + + try await MetadataConnectionPool.switchSchema(driver, to: "HR", timeoutSeconds: 1) + + #expect(driver.currentSchema == "HR") + } + + @Test("schema switch fails with a connection error when the driver hangs") + func switchSchemaTimesOut() async { + let driver = MockDatabaseDriver() + driver.switchSchemaDelaySeconds = 5 + + await #expect(throws: DatabaseError.self) { + try await MetadataConnectionPool.switchSchema(driver, to: "HR", timeoutSeconds: 0.05) + } + #expect(driver.currentSchema == nil) + } +} diff --git a/TableProTests/Core/Services/Query/SchemaServiceTests.swift b/TableProTests/Core/Services/Query/SchemaServiceTests.swift index e4b98e4cd..ca21f540d 100644 --- a/TableProTests/Core/Services/Query/SchemaServiceTests.swift +++ b/TableProTests/Core/Services/Query/SchemaServiceTests.swift @@ -58,4 +58,31 @@ struct SchemaServiceTests { let service = SchemaService() #expect(service.allLoadedTables(for: UUID()).isEmpty) } + + @Test("markLoadFailed surfaces a failed state for spinners to resolve") + func markLoadFailedSetsFailedState() { + let service = SchemaService() + let connectionId = UUID() + + service.markLoadFailed(connectionId: connectionId, message: "connect timed out") + + #expect(service.state(for: connectionId) == .failed("connect timed out")) + } + + @Test("markLoadFailed keeps already-loaded tables instead of replacing them") + func markLoadFailedKeepsLoadedTables() async { + let connectionId = UUID() + let driver = MockDatabaseDriver() + driver.tablesToReturn = [TableInfo(name: "orders", type: .table, rowCount: 0, schema: nil)] + let service = SchemaService() + await service.reload( + connectionId: connectionId, + driver: driver, + connection: TestFixtures.makeConnection() + ) + + service.markLoadFailed(connectionId: connectionId, message: "refresh failed") + + #expect(service.state(for: connectionId) == .loaded(driver.tablesToReturn)) + } } diff --git a/TableProTests/Views/SwitchContainerTests.swift b/TableProTests/Views/SwitchContainerTests.swift new file mode 100644 index 000000000..feeaa902e --- /dev/null +++ b/TableProTests/Views/SwitchContainerTests.swift @@ -0,0 +1,44 @@ +// +// SwitchContainerTests.swift +// TableProTests +// +// Regression coverage for #1807: Oracle declares schema-only switching, so +// the container switcher must route through switchSchema. The old routing +// went through switchDatabase, whose bySchema branch overwrote the session +// schema with a nonexistent default and hung the schema load. +// + +import Foundation +import Testing + +@testable import TablePro + +@Suite("SwitchContainer") +@MainActor +struct SwitchContainerTests { + @Test("switchContainer routes Oracle to a schema switch") + func oracleRoutesToSchemaSwitch() async { + let connection = TestFixtures.makeConnection(type: .oracle) + let driver = MockDatabaseDriver(connection: connection) + DatabaseManager.shared.injectSession( + ConnectionSession(connection: connection, driver: driver), + for: connection.id + ) + defer { DatabaseManager.shared.removeSession(for: connection.id) } + + let coordinator = MainContentCoordinator( + connection: connection, + tabManager: QueryTabManager(), + changeManager: DataChangeManager(), + toolbarState: ConnectionToolbarState() + ) + defer { coordinator.teardown() } + + await coordinator.switchContainer(to: "HR") + + #expect(driver.switchSchemaCallCount == 1) + #expect(driver.currentSchema == "HR") + #expect(coordinator.toolbarState.currentSchema == "HR") + #expect(DatabaseManager.shared.session(for: connection.id)?.currentSchema == "HR") + } +} From f37b876afd2ab8c051b34f19f133569c6daf66ae Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 4 Jul 2026 19:37:06 +0700 Subject: [PATCH 2/2] fix(plugin-oracle): release the query gate on timeout, reconnect lazily, and normalize legacy metadata (#1807) --- CHANGELOG.md | 2 +- .../AsyncTimeoutTests.swift | 38 +++++++ .../OracleDriverPlugin/OracleConnection.swift | 100 ++++++++++-------- Plugins/OracleDriverPlugin/OraclePlugin.swift | 6 +- Plugins/TableProPluginKit/AsyncTimeout.swift | 12 +++ .../Core/Plugins/PluginMetadataRegistry.swift | 52 +++++++++ .../Query/MetadataConnectionPool.swift | 48 ++++++--- .../Core/Services/Query/SchemaService.swift | 36 +++++-- .../Views/Main/MainContentCoordinator.swift | 2 + .../Autocomplete/SQLSchemaProviderTests.swift | 20 +++- .../PluginMetadataSwitchRoutingTests.swift | 51 +++++++++ .../Query/MetadataConnectionPoolTests.swift | 15 ++- .../Services/Query/SchemaServiceTests.swift | 43 ++++++++ .../Views/SwitchContainerTests.swift | 4 +- docs/customization/settings.mdx | 2 +- docs/databases/oracle.mdx | 2 +- 16 files changed, 350 insertions(+), 83 deletions(-) create mode 100644 TableProTests/Core/Plugins/PluginMetadataSwitchRoutingTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 251c568d0..0917da3f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Switching schemas on an Oracle connection no longer hangs on an infinite loading spinner. Oracle now switches by schema like BigQuery, the sidebar lists every schema with its tables loading on expand, Oracle queries respect the query timeout setting and can be cancelled, and a schema load that fails now shows an error with a Retry button instead of spinning forever. Requires the updated Oracle plugin for the full fix. (#1807) +- Switching schemas on an Oracle connection no longer hangs on an infinite loading spinner. Oracle now switches by schema like BigQuery, the sidebar lists every schema with its tables loading on expand, Oracle queries respect the query timeout setting and reconnect automatically after a timeout, and a schema load that fails shows an error with a Retry button instead of spinning forever. Works with an already-installed Oracle plugin; updating the plugin adds the query timeout enforcement. (#1807) ## [0.55.0] - 2026-07-04 diff --git a/Packages/TableProCore/Tests/TableProPluginKitTests/AsyncTimeoutTests.swift b/Packages/TableProCore/Tests/TableProPluginKitTests/AsyncTimeoutTests.swift index 7ca5759c1..6d1e1e41c 100644 --- a/Packages/TableProCore/Tests/TableProPluginKitTests/AsyncTimeoutTests.swift +++ b/Packages/TableProCore/Tests/TableProPluginKitTests/AsyncTimeoutTests.swift @@ -32,4 +32,42 @@ final class AsyncTimeoutTests: XCTestCase { XCTFail("Expected Boom, got \(error)") } } + + func testOnTimeoutUnblocksCancellationDeafOperation() async { + final class Latch: @unchecked Sendable { + var continuation: CheckedContinuation? + } + struct Closed: Error {} + let latch = Latch() + + do { + _ = try await withTimeout( + seconds: 0.05, + onTimeout: { + latch.continuation?.resume() + latch.continuation = nil + } + ) { () -> Int in + await withCheckedContinuation { latch.continuation = $0 } + throw Closed() + } + XCTFail("Expected an error") + } catch { + // TimeoutError or Closed both prove onTimeout unblocked the operation. + // Without it this test hangs: the continuation ignores task + // cancellation and the group waits for the operation child. + } + } + + func testOnTimeoutNotInvokedWhenOperationWins() async throws { + final class Flag: @unchecked Sendable { + var value = false + } + let flag = Flag() + + let value = try await withTimeout(seconds: 5, onTimeout: { flag.value = true }) { 42 } + + XCTAssertEqual(value, 42) + XCTAssertFalse(flag.value) + } } diff --git a/Plugins/OracleDriverPlugin/OracleConnection.swift b/Plugins/OracleDriverPlugin/OracleConnection.swift index 41bc47273..d1bf4df9c 100644 --- a/Plugins/OracleDriverPlugin/OracleConnection.swift +++ b/Plugins/OracleDriverPlugin/OracleConnection.swift @@ -127,8 +127,10 @@ final class OracleConnectionWrapper: @unchecked Sendable { private struct LockedState: Sendable { var isConnected = false + var hasEverConnected = false var nioConnection: OracleNIO.OracleConnection? var queryTimeoutSeconds = 0 + var sessionSchema: String? } private let state = OSAllocatedUnfairLock(initialState: LockedState()) @@ -197,6 +199,7 @@ final class OracleConnectionWrapper: @unchecked Sendable { state.withLock { current in current.nioConnection = connection current.isConnected = true + current.hasEverConnected = true } let target = useSID ? "\(self.host):\(self.port):\(identifier)" : "\(self.host):\(self.port)/\(identifier)" @@ -352,10 +355,8 @@ final class OracleConnectionWrapper: @unchecked Sendable { state.withLock { $0.queryTimeoutSeconds = max(0, seconds) } } - func abortCurrentQuery() { - guard isConnected else { return } - osLogger.notice("Aborting Oracle query by closing the connection; the driver has no server-side cancel") - disconnect() + func noteSessionSchema(_ schema: String) { + state.withLock { $0.sessionSchema = schema } } // MARK: - Query Execution @@ -369,28 +370,47 @@ final class OracleConnectionWrapper: @unchecked Sendable { } } + /// Serialized behind the query gate, so at most one reconnect runs at a time. + /// Reconnecting restores the session schema, which ALTER SESSION state does + /// not survive across connections. + private func reconnectedConnection() async throws -> OracleNIO.OracleConnection { + if let connection = state.withLock({ $0.isConnected ? $0.nioConnection : nil }) { + return connection + } + guard state.withLock({ $0.hasEverConnected }) else { + throw OracleError.notConnected + } + + osLogger.notice("Reconnecting to Oracle after the previous connection was closed") + try await connect() + let connection = try requireConnection() + if let schema = state.withLock({ $0.sessionSchema }) { + _ = try await withQueryDeadline { [self] in + try await collectRows(Self.currentSchemaStatement(schema), on: connection) + } + } + return connection + } + + private static func currentSchemaStatement(_ schema: String) -> String { + let escaped = schema.replacingOccurrences(of: "\"", with: "\"\"") + return "ALTER SESSION SET CURRENT_SCHEMA = \"\(escaped)\"" + } + /// Races the operation against the configured query timeout. On timeout the /// connection is closed first, which fails the in-flight OracleNIO call even - /// if it ignores task cancellation, so the group can always unwind. + /// if it ignores task cancellation, so the race can always unwind. private func withQueryDeadline( _ operation: @escaping @Sendable () async throws -> T ) async throws -> T { let timeoutSeconds = state.withLock { $0.queryTimeoutSeconds } guard timeoutSeconds > 0 else { return try await operation() } - return try await withThrowingTaskGroup(of: T?.self) { group in - group.addTask { try await operation() } - group.addTask { - try await Task.sleep(nanoseconds: UInt64(timeoutSeconds) * 1_000_000_000) - return nil - } - defer { group.cancelAll() } - guard let first = try await group.next(), let result = first else { - disconnect() - throw TimeoutError(seconds: Double(timeoutSeconds)) - } - return result - } + return try await withTimeout( + seconds: Double(timeoutSeconds), + onTimeout: { [self] in disconnect() }, + operation: operation + ) } private func queryTimeoutError(_ timeout: TimeoutError) -> OracleError { @@ -398,33 +418,36 @@ final class OracleConnectionWrapper: @unchecked Sendable { return OracleError(message: Self.queryTimeoutMessage, category: .queryTimedOut) } - func executeQuery(_ query: String) async throws -> OracleQueryResult { - let connection = try requireConnection() + private func mapExecutionError(_ error: Error) -> Error { + switch error { + case let timeout as TimeoutError: + return queryTimeoutError(timeout) + case let sqlError as OracleSQLError: + return mapQueryError(sqlError) + case let oracleError as OracleError: + return oracleError + case is CancellationError: + return error + default: + return OracleError(message: "Query execution failed: \(String(describing: error))") + } + } + func executeQuery(_ query: String) async throws -> OracleQueryResult { // OracleNIO does not support concurrent queries on a single connection. // Serialize all queries to prevent state-machine corruption. await queryGate.acquire() do { + let connection = try await reconnectedConnection() let result = try await withQueryDeadline { [self] in try await collectRows(query, on: connection) } await queryGate.release() return result - } catch let timeout as TimeoutError { - throw queryTimeoutError(timeout) - } catch let sqlError as OracleSQLError { - await queryGate.release() - throw mapQueryError(sqlError) - } catch let error as OracleError { - await queryGate.release() - throw error - } catch is CancellationError { - await queryGate.release() - throw CancellationError() } catch { await queryGate.release() - throw OracleError(message: "Query execution failed: \(String(describing: error))") + throw mapExecutionError(error) } } @@ -489,27 +512,18 @@ final class OracleConnectionWrapper: @unchecked Sendable { _ query: String, continuation: AsyncThrowingStream.Continuation ) async throws { - let connection = try requireConnection() - await queryGate.acquire() do { + let connection = try await reconnectedConnection() try await withQueryDeadline { [self] in try await streamRows(query, on: connection, continuation: continuation) } await queryGate.release() continuation.finish() - } catch let timeout as TimeoutError { - throw queryTimeoutError(timeout) - } catch let sqlError as OracleSQLError { - await queryGate.release() - throw mapQueryError(sqlError) - } catch is CancellationError { - await queryGate.release() - throw CancellationError() } catch { await queryGate.release() - throw OracleError(message: "Query execution failed: \(String(describing: error))") + throw mapExecutionError(error) } } diff --git a/Plugins/OracleDriverPlugin/OraclePlugin.swift b/Plugins/OracleDriverPlugin/OraclePlugin.swift index 41b54d51e..7db7c9335 100644 --- a/Plugins/OracleDriverPlugin/OraclePlugin.swift +++ b/Plugins/OracleDriverPlugin/OraclePlugin.swift @@ -239,7 +239,6 @@ final class OraclePluginDriver: PluginDatabaseDriver, @unchecked Sendable { .transactions, .alterTableDDL, .multiSchema, - .cancelQuery, ] } @@ -301,10 +300,6 @@ final class OraclePluginDriver: PluginDatabaseDriver, @unchecked Sendable { _ = try await execute(query: "SELECT 1 FROM DUAL") } - func cancelQuery() throws { - oracleConn?.abortCurrentQuery() - } - func applyQueryTimeout(_ seconds: Int) async throws { oracleConn?.applyQueryTimeout(seconds) } @@ -1149,6 +1144,7 @@ final class OraclePluginDriver: PluginDatabaseDriver, @unchecked Sendable { let escaped = schema.replacingOccurrences(of: "\"", with: "\"\"") _ = try await execute(query: "ALTER SESSION SET CURRENT_SCHEMA = \"\(escaped)\"") _currentSchema = schema + oracleConn?.noteSessionSchema(schema) } /// Oracle has no real database concept; "switch database" is a schema switch. diff --git a/Plugins/TableProPluginKit/AsyncTimeout.swift b/Plugins/TableProPluginKit/AsyncTimeout.swift index 7e4381f7f..4901e5bc5 100644 --- a/Plugins/TableProPluginKit/AsyncTimeout.swift +++ b/Plugins/TableProPluginKit/AsyncTimeout.swift @@ -11,11 +11,23 @@ public struct TimeoutError: Error, Sendable, Equatable { public func withTimeout( seconds: Double, operation: @escaping @Sendable () async throws -> T +) async throws -> T { + try await withTimeout(seconds: seconds, onTimeout: {}, operation: operation) +} + +/// The task group waits for the operation child even after the deadline fires, +/// so `onTimeout` must force the operation to complete (close a connection, +/// fail a promise) when the wrapped call does not respond to task cancellation. +public func withTimeout( + seconds: Double, + onTimeout: @escaping @Sendable () -> Void, + operation: @escaping @Sendable () async throws -> T ) async throws -> T { try await withThrowingTaskGroup(of: T.self) { group in group.addTask { try await operation() } group.addTask { try await Task.sleep(nanoseconds: UInt64(seconds * 1_000_000_000)) + onTimeout() throw TimeoutError(seconds: seconds) } defer { group.cancelAll() } diff --git a/TablePro/Core/Plugins/PluginMetadataRegistry.swift b/TablePro/Core/Plugins/PluginMetadataRegistry.swift index ce8cdf048..34d65d0d6 100644 --- a/TablePro/Core/Plugins/PluginMetadataRegistry.swift +++ b/TablePro/Core/Plugins/PluginMetadataRegistry.swift @@ -4,6 +4,7 @@ // import Foundation +import os import TableProPluginKit struct PluginMetadataSnapshot: Sendable { @@ -219,6 +220,37 @@ struct PluginMetadataSnapshot: Sendable { capabilities: capabilities, schema: schema, editor: editor, connection: connection ) } + + func withSwitchRouting(from source: PluginMetadataSnapshot) -> PluginMetadataSnapshot { + PluginMetadataSnapshot( + displayName: displayName, iconName: iconName, defaultPort: defaultPort, + requiresAuthentication: requiresAuthentication, supportsForeignKeys: supportsForeignKeys, + supportsSchemaEditing: supportsSchemaEditing, isDownloadable: isDownloadable, + primaryUrlScheme: primaryUrlScheme, parameterStyle: parameterStyle, + navigationModel: navigationModel, explainVariants: explainVariants, + pathFieldRole: pathFieldRole, supportsHealthMonitor: supportsHealthMonitor, + urlSchemes: urlSchemes, postConnectActions: postConnectActions, + brandColorHex: brandColorHex, queryLanguageName: queryLanguageName, + editorLanguage: editorLanguage, connectionMode: connectionMode, + supportsDatabaseSwitching: source.supportsDatabaseSwitching, + supportsColumnReorder: supportsColumnReorder, + capabilities: capabilities, + schema: SchemaInfo( + defaultSchemaName: source.schema.defaultSchemaName, + defaultGroupName: schema.defaultGroupName, + tableEntityName: schema.tableEntityName, + containerEntityName: source.schema.containerEntityName, + defaultPrimaryKeyColumn: schema.defaultPrimaryKeyColumn, + immutableColumns: schema.immutableColumns, + systemDatabaseNames: schema.systemDatabaseNames, + systemSchemaNames: schema.systemSchemaNames, + fileExtensions: schema.fileExtensions, + databaseGroupingStrategy: source.schema.databaseGroupingStrategy, + structureColumnFields: schema.structureColumnFields + ), + editor: editor, connection: connection + ) + } } final class PluginMetadataRegistry: @unchecked Sendable { @@ -771,6 +803,12 @@ final class PluginMetadataRegistry: @unchecked Sendable { } if let registryDefault = defaultSnapshots[typeId] { resolved = resolved.withIsDownloadable(registryDefault.isDownloadable) + if Self.declaresLegacySchemaOnlyRouting(resolved, registryDefault: registryDefault) { + Logger(subsystem: "com.TablePro", category: "PluginMetadataRegistry").notice( + "Plugin '\(typeId, privacy: .public)' declares legacy two-tier switching for a schema-only engine; applying the app's switch routing" + ) + resolved = resolved.withSwitchRouting(from: registryDefault) + } } snapshots[typeId] = resolved for scheme in resolved.urlSchemes { @@ -778,6 +816,20 @@ final class PluginMetadataRegistry: @unchecked Sendable { } } + /// A plugin built before its engine moved to schema-only switching still + /// declares database switching with bySchema grouping. The app's registry + /// default is the ground truth for routing, so its switch fields win. + static func declaresLegacySchemaOnlyRouting( + _ snapshot: PluginMetadataSnapshot, + registryDefault: PluginMetadataSnapshot + ) -> Bool { + !registryDefault.supportsDatabaseSwitching + && registryDefault.capabilities.supportsSchemaSwitching + && snapshot.supportsDatabaseSwitching + && snapshot.capabilities.supportsSchemaSwitching + && snapshot.schema.databaseGroupingStrategy == .bySchema + } + func unregister(typeId: String) { lock.lock() defer { lock.unlock() } diff --git a/TablePro/Core/Services/Query/MetadataConnectionPool.swift b/TablePro/Core/Services/Query/MetadataConnectionPool.swift index 45a80498c..3c192f060 100644 --- a/TablePro/Core/Services/Query/MetadataConnectionPool.swift +++ b/TablePro/Core/Services/Query/MetadataConnectionPool.swift @@ -157,8 +157,8 @@ final class MetadataConnectionPool { await DatabaseManager.shared.executeStartupCommands( session.connection.startupCommands, on: driver, connectionName: session.connection.name ) - if let schema = key.schema, let switchable = driver as? SchemaSwitchable { - try await Self.switchSchema(switchable, to: schema, timeoutSeconds: operationTimeoutSeconds) + if let schema = key.schema { + try await Self.switchSchema(driver, to: schema, timeoutSeconds: operationTimeoutSeconds) } } catch { driver.disconnect() @@ -168,26 +168,42 @@ final class MetadataConnectionPool { } static func connect(_ driver: DatabaseDriver, database: String, timeoutSeconds: Double) async throws { - do { - try await withTimeout(seconds: timeoutSeconds) { try await driver.connect() } - } catch is TimeoutError { - throw DatabaseError.connectionFailed( - String(format: String(localized: "Connecting to '%@' timed out."), database) - ) + try await bounded( + driver: driver, + timeoutSeconds: timeoutSeconds, + timeoutMessage: String(format: String(localized: "Connecting to '%@' timed out."), database) + ) { + try await driver.connect() + } + } + + static func switchSchema(_ driver: DatabaseDriver, to schema: String, timeoutSeconds: Double) async throws { + guard let switchable = driver as? SchemaSwitchable else { return } + try await bounded( + driver: driver, + timeoutSeconds: timeoutSeconds, + timeoutMessage: String(format: String(localized: "Switching to schema '%@' timed out."), schema) + ) { + try await switchable.switchSchema(to: schema) } } - static func switchSchema( - _ switchable: SchemaSwitchable, - to schema: String, - timeoutSeconds: Double + /// Disconnects the driver when the deadline fires so a driver call that + /// ignores task cancellation still completes and the timeout can propagate. + private static func bounded( + driver: DatabaseDriver, + timeoutSeconds: Double, + timeoutMessage: String, + _ operation: @escaping @Sendable () async throws -> Void ) async throws { do { - try await withTimeout(seconds: timeoutSeconds) { try await switchable.switchSchema(to: schema) } - } catch is TimeoutError { - throw DatabaseError.connectionFailed( - String(format: String(localized: "Switching to schema '%@' timed out."), schema) + try await withTimeout( + seconds: timeoutSeconds, + onTimeout: { driver.disconnect() }, + operation: operation ) + } catch is TimeoutError { + throw DatabaseError.connectionFailed(timeoutMessage) } } diff --git a/TablePro/Core/Services/Query/SchemaService.swift b/TablePro/Core/Services/Query/SchemaService.swift index 0db39da52..21fbe61bd 100644 --- a/TablePro/Core/Services/Query/SchemaService.swift +++ b/TablePro/Core/Services/Query/SchemaService.swift @@ -209,7 +209,13 @@ final class SchemaService { func refresh(connectionId: UUID) async { guard let session = DatabaseManager.shared.activeSessions[connectionId], - let driver = session.driver else { return } + let driver = session.driver else { + markLoadFailed( + connectionId: connectionId, + message: String(localized: "The connection is not available. Reconnect and try again.") + ) + return + } await invalidate(connectionId: connectionId) await reload(connectionId: connectionId, driver: driver, connection: session.connection) } @@ -322,18 +328,30 @@ final class SchemaService { let loadedProcedures = await proceduresTask let loadedFunctions = await functionsTask - let loadedSchemas = await Self.fetchSchemasSafely( - connectionId: connectionId, - dedup: schemasDedup, - fetch: { try await driver.fetchSchemas() } - ) - guard isCurrentLoadGeneration(generation, for: connectionId, phase: "hierarchical-loaded") else { + let loadedSchemas: [String] + do { + loadedSchemas = try await schemasDedup.execute(key: connectionId) { + try await driver.fetchSchemas() + } + } catch is CancellationError { + return + } catch { + guard isCurrentLoadGeneration(generation, for: connectionId, phase: "hierarchical-failed") else { + return + } + Self.logger.warning( + "[schema] hierarchical schema list failed connId=\(connectionId, privacy: .public) error=\(error.localizedDescription, privacy: .public)" + ) + states[connectionId] = .failed(error.localizedDescription) + bumpGeneration(connectionId) return } - if let loadedSchemas { - schemasInOrder[connectionId] = loadedSchemas + + guard isCurrentLoadGeneration(generation, for: connectionId, phase: "hierarchical-loaded") else { + return } + schemasInOrder[connectionId] = loadedSchemas procedures[connectionId] = loadedProcedures functions[connectionId] = loadedFunctions states[connectionId] = .loaded([]) diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 787fe959f..c3bef1c33 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -613,6 +613,8 @@ final class MainContentCoordinator { connection: connection ) } + } catch is CancellationError { + return } catch { Self.logger.warning("Schema refresh failed: \(error.localizedDescription, privacy: .public)") schemaService.markLoadFailed(connectionId: connectionId, message: error.localizedDescription) diff --git a/TableProTests/Core/Autocomplete/SQLSchemaProviderTests.swift b/TableProTests/Core/Autocomplete/SQLSchemaProviderTests.swift index 1a54049bb..611c2fba3 100644 --- a/TableProTests/Core/Autocomplete/SQLSchemaProviderTests.swift +++ b/TableProTests/Core/Autocomplete/SQLSchemaProviderTests.swift @@ -31,17 +31,35 @@ final class MockDatabaseDriver: DatabaseDriver, SchemaSwitchable, @unchecked Sen var cancelQueryCallCount = 0 var connectDelaySeconds: Double = 0 var switchSchemaDelaySeconds: Double = 0 + var hangsUntilDisconnect = false + var schemasToReturn: [String] = [] + var fetchSchemasError: Error? + private var hangContinuation: CheckedContinuation? init(connection: DatabaseConnection = TestFixtures.makeConnection()) { self.connection = connection } func connect() async throws { + if hangsUntilDisconnect { + await withCheckedContinuation { hangContinuation = $0 } + throw DatabaseError.notConnected + } guard connectDelaySeconds > 0 else { return } try await Task.sleep(nanoseconds: UInt64(connectDelaySeconds * 1_000_000_000)) } - func disconnect() {} + func disconnect() { + hangContinuation?.resume() + hangContinuation = nil + } + + func fetchSchemas() async throws -> [String] { + if let fetchSchemasError { + throw fetchSchemasError + } + return schemasToReturn + } func testConnection() async throws -> Bool { true } diff --git a/TableProTests/Core/Plugins/PluginMetadataSwitchRoutingTests.swift b/TableProTests/Core/Plugins/PluginMetadataSwitchRoutingTests.swift new file mode 100644 index 000000000..11d324ed4 --- /dev/null +++ b/TableProTests/Core/Plugins/PluginMetadataSwitchRoutingTests.swift @@ -0,0 +1,51 @@ +// +// PluginMetadataSwitchRoutingTests.swift +// TableProTests +// +// A plugin built before its engine moved to schema-only switching still +// declares two-tier routing; the registry must detect that and apply the +// app's switch-routing fields instead. +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +@MainActor +@Suite("Plugin metadata switch-routing normalization") +struct PluginMetadataSwitchRoutingTests { + private var oracleDefault: PluginMetadataSnapshot? { + PluginMetadataRegistry.shared.snapshot(forTypeId: DatabaseType.oracle.pluginTypeId) + } + + private var legacyTwoTier: PluginMetadataSnapshot? { + PluginMetadataRegistry.shared.snapshot(forTypeId: DatabaseType.mssql.pluginTypeId) + } + + @Test("legacy two-tier declarations on a schema-only engine are detected") + func legacyDeclarationIsDetected() throws { + let oracle = try #require(oracleDefault) + let legacy = try #require(legacyTwoTier) + + #expect(PluginMetadataRegistry.declaresLegacySchemaOnlyRouting(legacy, registryDefault: oracle)) + #expect(!PluginMetadataRegistry.declaresLegacySchemaOnlyRouting(oracle, registryDefault: oracle)) + #expect(!PluginMetadataRegistry.declaresLegacySchemaOnlyRouting(legacy, registryDefault: legacy)) + } + + @Test("withSwitchRouting carries over only the routing fields") + func switchRoutingCarriesRoutingFields() throws { + let oracle = try #require(oracleDefault) + let legacy = try #require(legacyTwoTier) + + let normalized = legacy.withSwitchRouting(from: oracle) + + #expect(normalized.supportsDatabaseSwitching == false) + #expect(normalized.schema.databaseGroupingStrategy == .hierarchicalSchema) + #expect(normalized.schema.defaultSchemaName.isEmpty) + #expect(normalized.schema.containerEntityName == "Schema") + #expect(normalized.displayName == legacy.displayName) + #expect(normalized.schema.tableEntityName == legacy.schema.tableEntityName) + #expect(normalized.capabilities.supportsSchemaSwitching == legacy.capabilities.supportsSchemaSwitching) + } +} diff --git a/TableProTests/Core/Services/Query/MetadataConnectionPoolTests.swift b/TableProTests/Core/Services/Query/MetadataConnectionPoolTests.swift index 75a125acd..7d694e6db 100644 --- a/TableProTests/Core/Services/Query/MetadataConnectionPoolTests.swift +++ b/TableProTests/Core/Services/Query/MetadataConnectionPoolTests.swift @@ -2,9 +2,8 @@ // MetadataConnectionPoolTests.swift // TableProTests // -// Tests for the pool's bounded connect and schema-switch steps: a driver -// that hangs must fail within the deadline instead of stalling the pool -// entry forever (#1807). +// Tests for the pool's bounded connect and schema-switch steps: a hanging +// driver must fail within the deadline instead of stalling the pool entry. // import Foundation @@ -31,6 +30,16 @@ struct MetadataConnectionPoolTests { } } + @Test("connect force-disconnects a driver that ignores cancellation") + func connectUnsticksCancellationDeafDriver() async { + let driver = MockDatabaseDriver() + driver.hangsUntilDisconnect = true + + await #expect(throws: DatabaseError.self) { + try await MetadataConnectionPool.connect(driver, database: "db", timeoutSeconds: 0.05) + } + } + @Test("schema switch passes through when the driver responds in time") func switchSchemaPassesThrough() async throws { let driver = MockDatabaseDriver() diff --git a/TableProTests/Core/Services/Query/SchemaServiceTests.swift b/TableProTests/Core/Services/Query/SchemaServiceTests.swift index ca21f540d..8db9ec04f 100644 --- a/TableProTests/Core/Services/Query/SchemaServiceTests.swift +++ b/TableProTests/Core/Services/Query/SchemaServiceTests.swift @@ -85,4 +85,47 @@ struct SchemaServiceTests { #expect(service.state(for: connectionId) == .loaded(driver.tablesToReturn)) } + + @Test("hierarchical load lists schemas") + func hierarchicalLoadListsSchemas() async { + let driver = MockDatabaseDriver() + driver.schemasToReturn = ["HR", "SALES"] + let connection = TestFixtures.makeConnection(type: .oracle) + let service = SchemaService() + + await service.reload(connectionId: connection.id, driver: driver, connection: connection) + + #expect(service.state(for: connection.id) == .loaded([])) + #expect(service.schemas(for: connection.id) == ["HR", "SALES"]) + } + + @Test("hierarchical schema list failure surfaces a failed state") + func hierarchicalFailureSetsFailedState() async { + let driver = MockDatabaseDriver() + driver.fetchSchemasError = DatabaseError.connectionFailed("schema list failed") + let connection = TestFixtures.makeConnection(type: .oracle) + let service = SchemaService() + + await service.reload(connectionId: connection.id, driver: driver, connection: connection) + + var isFailed = false + if case .failed = service.state(for: connection.id) { + isFailed = true + } + #expect(isFailed) + } + + @Test("refresh without a session surfaces a failed state") + func refreshWithoutSessionSetsFailedState() async { + let service = SchemaService() + let connectionId = UUID() + + await service.refresh(connectionId: connectionId) + + var isFailed = false + if case .failed = service.state(for: connectionId) { + isFailed = true + } + #expect(isFailed) + } } diff --git a/TableProTests/Views/SwitchContainerTests.swift b/TableProTests/Views/SwitchContainerTests.swift index feeaa902e..6dab18d4e 100644 --- a/TableProTests/Views/SwitchContainerTests.swift +++ b/TableProTests/Views/SwitchContainerTests.swift @@ -3,9 +3,7 @@ // TableProTests // // Regression coverage for #1807: Oracle declares schema-only switching, so -// the container switcher must route through switchSchema. The old routing -// went through switchDatabase, whose bySchema branch overwrote the session -// schema with a nonexistent default and hung the schema load. +// the container switcher must route through switchSchema. // import Foundation diff --git a/docs/customization/settings.mdx b/docs/customization/settings.mdx index f6624f3a6..0945bd011 100644 --- a/docs/customization/settings.mdx +++ b/docs/customization/settings.mdx @@ -38,7 +38,7 @@ Reopened tabs restore their SQL, cursor position, applied sort, filters, page, a Maximum seconds a query runs before cancellation. Default 60, range 0-600 (0 means no limit). -Enforced at the database level where supported: `statement_timeout` (PostgreSQL), `max_execution_time` (MySQL, ClickHouse), `max_statement_time` (MariaDB), `sqlite3_busy_timeout` (SQLite). HTTP-based drivers (ClickHouse, BigQuery, CloudflareD1, LibSQL, Etcd, DynamoDB) also bound the HTTP request timeout to the configured value plus a 30-second grace, so the server-side error fires first. Setting "No limit" raises the HTTP transport ceiling to 1 hour. Applies on new connections; change requires reconnect. +Enforced at the database level where supported: `statement_timeout` (PostgreSQL), `max_execution_time` (MySQL, ClickHouse), `max_statement_time` (MariaDB), `sqlite3_busy_timeout` (SQLite). HTTP-based drivers (ClickHouse, BigQuery, CloudflareD1, LibSQL, Etcd, DynamoDB) also bound the HTTP request timeout to the configured value plus a 30-second grace, so the server-side error fires first. Setting "No limit" raises the HTTP transport ceiling to 1 hour. Oracle enforces the timeout client-side: a query that exceeds it is stopped by closing the connection, and the next query reconnects and restores the current schema automatically. Applies on new connections; change requires reconnect. ### Software Update diff --git a/docs/databases/oracle.mdx b/docs/databases/oracle.mdx index c002363a5..aa4f9be91 100644 --- a/docs/databases/oracle.mdx +++ b/docs/databases/oracle.mdx @@ -64,7 +64,7 @@ See [Connection URL Reference](/databases/connection-urls) for all parameters. ## Features -**Schema Selection**: Each user is a schema. Switch with **⌘K**. Shows available schemas and objects. +**Schema Selection**: Each user is a schema. The sidebar lists every schema and loads a schema's tables when you expand it. Switch the active schema with **⌘K** or the toolbar schema chip. **Table Info**: Structure (columns, types, nullability, defaults), indexes (B-tree, bitmap), foreign keys, DDL via DBMS_METADATA.