Skip to content

Commit f3508c0

Browse files
tianzhouclaude
andauthored
refactor: migrate all connection modes to multi-source architecture (#128)
* refactor: migrate all connection modes to multi-source architecture This commit removes the legacy single-DSN mode and unifies all connection methods (demo, CLI DSN, TOML) to use the multi-source architecture. All database sources are now exposed via the /api/sources endpoint. Key changes: - Demo mode now uses connectWithSources() with proper SourceConfig - Single DSN mode assigns id="default" and type from DSN protocol - Removed legacy fields: activeConnector, connected, sshTunnel, originalDSN, maxRows - Removed legacy methods: connectWithDSN(), connectWithType(), isConnected() - Updated SSH tunnel tests to use connectWithSources() - Added init_script field to SourceConfig for demo database initialization Benefits: - Single unified code path for all connection modes - All sources properly registered and queryable via API - Cleaner architecture with ~120 lines of legacy code removed - Consistent behavior across demo, CLI DSN, and TOML configurations Test coverage: 38/38 integration tests passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: apply execution options to demo mode source config Apply readonly and max_rows execution options to demoSource before calling connectWithSources(), ensuring consistency with how single DSN mode handles these options in env.ts (lines 525-528). This ensures that --readonly and --max-rows flags work correctly in demo mode, with the options properly stored in executeOptions map. Addresses review feedback from PR #128. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b61974c commit f3508c0

File tree

6 files changed

+154
-280
lines changed

6 files changed

+154
-280
lines changed

src/config/demo-loader.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,4 @@ export function getSqliteInMemorySetupSql(): string {
7474
}
7575

7676
return sql;
77-
}
77+
}

src/config/env.ts

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ export function resolveSSHConfig(): { config: SSHTunnelConfig; source: string }
472472
* Priority: TOML config (--config flag or ./dbhub.toml) > single DSN/env vars
473473
* Returns array of source configs and the source of the configuration
474474
*/
475-
export function resolveSourceConfigs(): { sources: SourceConfig[]; source: string } | null {
475+
export async function resolveSourceConfigs(): Promise<{ sources: SourceConfig[]; source: string } | null> {
476476
// 1. Try loading from TOML configuration file (highest priority for multi-source)
477477
const tomlConfig = loadTomlConfig();
478478
if (tomlConfig) {
@@ -482,10 +482,39 @@ export function resolveSourceConfigs(): { sources: SourceConfig[]; source: strin
482482
// 2. Fallback to single DSN configuration for backward compatibility
483483
const dsnResult = resolveDSN();
484484
if (dsnResult) {
485+
// Parse DSN to extract database type
486+
let dsnUrl: URL;
487+
try {
488+
dsnUrl = new URL(dsnResult.dsn);
489+
} catch (error) {
490+
throw new Error(
491+
`Invalid DSN format: ${dsnResult.dsn}. Expected format: protocol://[user[:password]@]host[:port]/database`
492+
);
493+
}
494+
495+
const protocol = dsnUrl.protocol.replace(':', '');
496+
497+
// Map protocol to database type
498+
let dbType: "postgres" | "mysql" | "mariadb" | "sqlserver" | "sqlite";
499+
if (protocol === 'postgresql' || protocol === 'postgres') {
500+
dbType = 'postgres';
501+
} else if (protocol === 'mysql') {
502+
dbType = 'mysql';
503+
} else if (protocol === 'mariadb') {
504+
dbType = 'mariadb';
505+
} else if (protocol === 'sqlserver') {
506+
dbType = 'sqlserver';
507+
} else if (protocol === 'sqlite') {
508+
dbType = 'sqlite';
509+
} else {
510+
throw new Error(`Unsupported database type in DSN: ${protocol}`);
511+
}
512+
485513
// Create a single source config from the resolved DSN
486-
// Use empty string as ID for backward compatibility (default/unnamed source)
514+
// Use "default" as ID so it appears in the API sources list
487515
const source: SourceConfig = {
488-
id: "",
516+
id: "default",
517+
type: dbType,
489518
dsn: dsnResult.dsn,
490519
};
491520

@@ -507,6 +536,12 @@ export function resolveSourceConfigs(): { sources: SourceConfig[]; source: strin
507536
source.max_rows = maxRowsResult.maxRows;
508537
}
509538

539+
// Add init script for demo mode
540+
if (dsnResult.isDemo) {
541+
const { getSqliteInMemorySetupSql } = await import('./demo-loader.js');
542+
source.init_script = getSqliteInMemorySetupSql();
543+
}
544+
510545
return {
511546
sources: [source],
512547
source: dsnResult.isDemo ? "demo mode" : dsnResult.source,

src/connectors/__tests__/postgres-ssh.integration.test.ts

Lines changed: 82 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,19 @@ import { ConnectorManager } from '../manager.js';
55
import { ConnectorRegistry } from '../interface.js';
66
import { SSHTunnel } from '../../utils/ssh-tunnel.js';
77
import type { SSHTunnelConfig } from '../../types/ssh.js';
8+
import type { SourceConfig } from '../../types/config.js';
89
import * as sshConfigParser from '../../utils/ssh-config-parser.js';
910

11+
/**
12+
* Helper function to create a SourceConfig from a DSN for testing
13+
*/
14+
function createSourceConfigFromDSN(dsn: string, sourceId: string = 'test'): SourceConfig {
15+
return {
16+
id: sourceId,
17+
dsn: dsn,
18+
};
19+
}
20+
1021
describe('PostgreSQL SSH Tunnel Simple Integration Tests', () => {
1122
let postgresContainer: StartedPostgreSqlContainer;
1223

@@ -60,161 +71,119 @@ describe('PostgreSQL SSH Tunnel Simple Integration Tests', () => {
6071

6172
// Make sure no SSH config is set
6273
delete process.env.SSH_HOST;
63-
74+
6475
const dsn = postgresContainer.getConnectionUri();
65-
66-
await manager.connectWithDSN(dsn);
67-
76+
const sourceConfig = createSourceConfigFromDSN(dsn);
77+
78+
await manager.connectWithSources([sourceConfig]);
79+
6880
// Test that connection works
6981
const connector = manager.getConnector();
7082
const result = await connector.executeSQL('SELECT 1 as test', {});
7183
expect(result.rows).toHaveLength(1);
7284
expect(result.rows[0].test).toBe(1);
73-
85+
7486
await manager.disconnect();
7587
});
7688

7789
it('should fail gracefully when SSH config is invalid', async () => {
7890
const manager = new ConnectorManager();
79-
80-
// Set invalid SSH config (missing required fields)
81-
process.env.SSH_HOST = 'example.com';
82-
// Missing SSH_USER
83-
84-
try {
85-
const dsn = postgresContainer.getConnectionUri();
86-
await expect(manager.connectWithDSN(dsn)).rejects.toThrow(/SSH tunnel configuration requires/);
87-
} finally {
88-
delete process.env.SSH_HOST;
89-
}
91+
92+
// Create source config with invalid SSH config (missing required fields)
93+
const dsn = postgresContainer.getConnectionUri();
94+
const sourceConfig = createSourceConfigFromDSN(dsn);
95+
sourceConfig.ssh_host = 'example.com';
96+
// Missing ssh_user
97+
98+
await expect(manager.connectWithSources([sourceConfig])).rejects.toThrow(/SSH tunnel requires ssh_user/);
9099
});
91100

92101
it('should validate SSH authentication method', async () => {
93102
const manager = new ConnectorManager();
94-
95-
// Set SSH config without authentication method
96-
process.env.SSH_HOST = 'example.com';
97-
process.env.SSH_USER = 'testuser';
98-
// Missing both SSH_PASSWORD and SSH_KEY
99-
100-
try {
101-
const dsn = postgresContainer.getConnectionUri();
102-
await expect(manager.connectWithDSN(dsn)).rejects.toThrow(/SSH tunnel configuration requires either/);
103-
} finally {
104-
delete process.env.SSH_HOST;
105-
delete process.env.SSH_USER;
106-
}
103+
104+
// Create source config with SSH but without authentication method
105+
const dsn = postgresContainer.getConnectionUri();
106+
const sourceConfig = createSourceConfigFromDSN(dsn);
107+
sourceConfig.ssh_host = 'example.com';
108+
sourceConfig.ssh_user = 'testuser';
109+
// Missing both ssh_password and ssh_key
110+
111+
await expect(manager.connectWithSources([sourceConfig])).rejects.toThrow(/SSH tunnel requires either ssh_password or ssh_key/);
107112
});
108113

109-
it('should handle SSH config file resolution', async () => {
114+
it('should handle SSH tunnel with source config', async () => {
110115
const manager = new ConnectorManager();
111-
112-
// Mock the SSH config parser functions
113-
const mockParseSSHConfig = vi.spyOn(sshConfigParser, 'parseSSHConfig');
114-
const mockLooksLikeSSHAlias = vi.spyOn(sshConfigParser, 'looksLikeSSHAlias');
115-
116+
116117
// Spy on the SSH tunnel establish method to verify the config values
117118
const mockSSHTunnelEstablish = vi.spyOn(SSHTunnel.prototype, 'establish');
118-
119+
119120
try {
120-
// Configure mocks to simulate SSH config file lookup with specific values
121-
mockLooksLikeSSHAlias.mockReturnValue(true);
122-
mockParseSSHConfig.mockReturnValue({
123-
host: 'bastion.example.com',
124-
username: 'sshuser',
125-
port: 2222,
126-
privateKey: '/home/user/.ssh/id_rsa'
127-
});
128-
129121
// Mock SSH tunnel establish to capture the config and prevent actual connection
130122
mockSSHTunnelEstablish.mockRejectedValue(new Error('SSH connection failed (expected in test)'));
131-
132-
// Set SSH host alias (would normally come from command line)
133-
process.env.SSH_HOST = 'mybastion';
134-
123+
135124
const dsn = postgresContainer.getConnectionUri();
136-
137-
// This should fail during SSH connection (expected), but we can verify the config parsing
138-
await expect(manager.connectWithDSN(dsn)).rejects.toThrow();
139-
140-
// Verify that SSH config parsing functions were called correctly
141-
expect(mockLooksLikeSSHAlias).toHaveBeenCalledWith('mybastion');
142-
expect(mockParseSSHConfig).toHaveBeenCalledWith('mybastion', expect.stringContaining('.ssh/config'));
143-
144-
// Verify that SSH tunnel was attempted with the correct config values from SSH config
125+
const sourceConfig = createSourceConfigFromDSN(dsn);
126+
127+
// Configure SSH tunnel via source config
128+
sourceConfig.ssh_host = 'bastion.example.com';
129+
sourceConfig.ssh_user = 'sshuser';
130+
sourceConfig.ssh_port = 2222;
131+
sourceConfig.ssh_key = '/home/user/.ssh/id_rsa';
132+
133+
// This should fail during SSH connection (expected), but we can verify the config
134+
await expect(manager.connectWithSources([sourceConfig])).rejects.toThrow();
135+
136+
// Verify that SSH tunnel was attempted with the correct config values
145137
expect(mockSSHTunnelEstablish).toHaveBeenCalledTimes(1);
146138
const sshTunnelCall = mockSSHTunnelEstablish.mock.calls[0];
147139
const [sshConfig, tunnelOptions] = sshTunnelCall;
148-
149-
// Debug: Log the actual values being passed (for verification)
150-
// SSH Config should contain the values from our mocked SSH config file
151-
// Tunnel Options should contain database connection details from the container DSN
152-
153-
// Verify SSH config values were properly resolved from the SSH config file
140+
141+
// Verify SSH config values were properly set from source config
154142
expect(sshConfig).toMatchObject({
155-
host: 'bastion.example.com', // Should use HostName from SSH config
156-
username: 'sshuser', // Should use User from SSH config
157-
port: 2222, // Should use Port from SSH config
158-
privateKey: '/home/user/.ssh/id_rsa' // Should use IdentityFile from SSH config
143+
host: 'bastion.example.com',
144+
username: 'sshuser',
145+
port: 2222,
146+
privateKey: '/home/user/.ssh/id_rsa'
159147
});
160-
148+
161149
// Verify tunnel options are correctly set up for the database connection
162-
expect(tunnelOptions).toMatchObject({
163-
targetHost: expect.any(String), // Database host from DSN
164-
targetPort: expect.any(Number) // Database port from DSN
165-
});
166-
167-
// The localPort might be undefined for dynamic allocation, so check separately if it exists
168-
if (tunnelOptions.localPort !== undefined) {
169-
expect(typeof tunnelOptions.localPort).toBe('number');
170-
}
171-
172-
// Verify that the target database details from the DSN are preserved
173150
const originalDsnUrl = new URL(dsn);
174151
expect(tunnelOptions.targetHost).toBe(originalDsnUrl.hostname);
175152
expect(tunnelOptions.targetPort).toBe(parseInt(originalDsnUrl.port));
176-
153+
177154
} finally {
178-
// Clean up
179-
delete process.env.SSH_HOST;
180-
mockParseSSHConfig.mockRestore();
181-
mockLooksLikeSSHAlias.mockRestore();
182155
mockSSHTunnelEstablish.mockRestore();
183156
}
184157
});
185158

186-
it('should skip SSH config lookup for direct hostnames', async () => {
159+
it('should handle SSH tunnel with password authentication', async () => {
187160
const manager = new ConnectorManager();
188-
189-
// Mock the SSH config parser functions
190-
const mockParseSSHConfig = vi.spyOn(sshConfigParser, 'parseSSHConfig');
191-
const mockLooksLikeSSHAlias = vi.spyOn(sshConfigParser, 'looksLikeSSHAlias');
192-
161+
162+
// Spy on the SSH tunnel establish method
163+
const mockSSHTunnelEstablish = vi.spyOn(SSHTunnel.prototype, 'establish');
164+
193165
try {
194-
// Configure mocks - direct hostname should not trigger SSH config lookup
195-
mockLooksLikeSSHAlias.mockReturnValue(false);
196-
197-
// Set a direct hostname with required SSH credentials
198-
process.env.SSH_HOST = 'ssh.example.com';
199-
process.env.SSH_USER = 'sshuser';
200-
process.env.SSH_PASSWORD = 'sshpass';
201-
166+
// Mock SSH tunnel establish to prevent actual connection
167+
mockSSHTunnelEstablish.mockRejectedValue(new Error('SSH connection failed (expected in test)'));
168+
202169
const dsn = postgresContainer.getConnectionUri();
203-
204-
// This should fail during actual SSH connection, but we can verify the parsing behavior
205-
await expect(manager.connectWithDSN(dsn)).rejects.toThrow();
206-
207-
// Verify that SSH config parsing was checked but not executed
208-
expect(mockLooksLikeSSHAlias).toHaveBeenCalledWith('ssh.example.com');
209-
expect(mockParseSSHConfig).not.toHaveBeenCalled();
210-
170+
const sourceConfig = createSourceConfigFromDSN(dsn);
171+
172+
// Configure SSH tunnel with password authentication
173+
sourceConfig.ssh_host = 'ssh.example.com';
174+
sourceConfig.ssh_user = 'sshuser';
175+
sourceConfig.ssh_password = 'sshpass';
176+
177+
// This should fail during actual SSH connection
178+
await expect(manager.connectWithSources([sourceConfig])).rejects.toThrow();
179+
180+
// Verify SSH tunnel was attempted with password auth
181+
expect(mockSSHTunnelEstablish).toHaveBeenCalledTimes(1);
182+
const [sshConfig] = mockSSHTunnelEstablish.mock.calls[0];
183+
expect(sshConfig.password).toBe('sshpass');
184+
211185
} finally {
212-
// Clean up
213-
delete process.env.SSH_HOST;
214-
delete process.env.SSH_USER;
215-
delete process.env.SSH_PASSWORD;
216-
mockParseSSHConfig.mockRestore();
217-
mockLooksLikeSSHAlias.mockRestore();
186+
mockSSHTunnelEstablish.mockRestore();
218187
}
219188
});
220189
});

0 commit comments

Comments
 (0)