From 2cef8e48fd50ed85dfa75aa9f77021ed9b4d792f Mon Sep 17 00:00:00 2001 From: ZhuchkaTriplesix Date: Fri, 25 Sep 2026 10:13:37 +0300 Subject: [PATCH] perf(storage): resolve connection secrets lazily and handle Keychain errors gracefully (Closes #916) --- .../storage/connection_secrets_store.dart | 12 ++-- lib/core/storage/local_db.dart | 53 +++++++++----- test/core/storage/local_db_secrets_test.dart | 71 +++++++++++++++++-- test/memory_secrets_backend.dart | 13 +++- 4 files changed, 122 insertions(+), 27 deletions(-) diff --git a/lib/core/storage/connection_secrets_store.dart b/lib/core/storage/connection_secrets_store.dart index a37758e..6424e33 100644 --- a/lib/core/storage/connection_secrets_store.dart +++ b/lib/core/storage/connection_secrets_store.dart @@ -56,10 +56,14 @@ class ConnectionSecretsStore { readForConnection( int connectionId, ) async { - final password = await backend.read(_passwordKey(connectionId)); - final connectionString = - await backend.read(_connectionStringKey(connectionId)); - return (password: password, connectionString: connectionString); + try { + final password = await backend.read(_passwordKey(connectionId)); + final connectionString = + await backend.read(_connectionStringKey(connectionId)); + return (password: password, connectionString: connectionString); + } catch (_) { + return (password: null, connectionString: null); + } } static Future deleteForConnection(int connectionId) async { diff --git a/lib/core/storage/local_db.dart b/lib/core/storage/local_db.dart index 848ce11..d34225c 100644 --- a/lib/core/storage/local_db.dart +++ b/lib/core/storage/local_db.dart @@ -1,5 +1,6 @@ import 'dart:io'; +import 'package:flutter/foundation.dart'; import 'package:path/path.dart' as p; import 'package:querya_desktop/core/storage/app_data_root.dart'; import 'package:querya_desktop/core/storage/connection_secrets_store.dart'; @@ -423,10 +424,19 @@ class LocalDb { return _sqliteInt(rows.first['id']); } - Future> getConnections() async { + /// Returns all saved connections. + /// + /// By default, passwords and connection strings are NOT eagerly hydrated from + /// the platform secure store to avoid IPC bottlenecks, Keychain lockups, and + /// D-Bus timeouts during sidebar/startup population. Secrets are resolved + /// on-demand when a connection is initiated. + Future> getConnections({bool hydrateSecrets = false}) async { final db = await _open(); final rows = await db.query('connections', orderBy: 'sort_order ASC, name ASC'); + if (!hydrateSecrets) { + return rows.map((m) => ConnectionRow.fromMap(m)).toList(); + } final futures = rows.map((m) => _hydrateConnection(ConnectionRow.fromMap(m))); return Future.wait(futures); @@ -434,23 +444,30 @@ class LocalDb { static Future _hydrateConnection(ConnectionRow row) async { if (row.id == null) return row; - final secrets = await ConnectionSecretsStore.readForConnection(row.id!); - return ConnectionRow( - id: row.id, - type: row.type, - name: row.name, - host: row.host, - port: row.port, - username: row.username, - password: secrets.password ?? row.password, - databaseName: row.databaseName, - authSource: row.authSource, - useSSL: row.useSSL, - connectionString: secrets.connectionString ?? row.connectionString, - folderId: row.folderId, - sortOrder: row.sortOrder, - createdAt: row.createdAt, - ); + try { + final secrets = await ConnectionSecretsStore.readForConnection(row.id!); + return row.copyWith( + password: secrets.password ?? row.password, + connectionString: secrets.connectionString ?? row.connectionString, + ); + } catch (e) { + debugPrint('LocalDb._hydrateConnection failed for connection ${row.id}: $e'); + return row; + } + } + + /// Hydrates secrets for a single [ConnectionRow] on demand. + Future hydrateConnection(ConnectionRow row) => + _hydrateConnection(row); + + /// Retrieves a single connection by [id], optionally hydrating secrets. + Future getConnectionById(int id, {bool hydrateSecrets = false}) async { + final db = await _open(); + final rows = await db.query('connections', where: 'id = ?', whereArgs: [id]); + if (rows.isEmpty) return null; + final row = ConnectionRow.fromMap(rows.first); + if (!hydrateSecrets) return row; + return _hydrateConnection(row); } /// Inserts a row and returns the SQLite row id. diff --git a/test/core/storage/local_db_secrets_test.dart b/test/core/storage/local_db_secrets_test.dart index dacae45..1aae290 100644 --- a/test/core/storage/local_db_secrets_test.dart +++ b/test/core/storage/local_db_secrets_test.dart @@ -98,7 +98,15 @@ void main() { final list = await LocalDb.instance.getConnections(); final loaded = list.singleWhere((c) => c.id == id); - expect(loaded.password, 'redis-secret'); + // Secrets are resolved lazily on demand, not eagerly in getConnections() + expect(loaded.password, isNull); + + final hydrated = await LocalDb.instance.hydrateConnection(loaded); + expect(hydrated.password, 'redis-secret'); + + final eagerlyHydrated = (await LocalDb.instance.getConnections(hydrateSecrets: true)) + .singleWhere((c) => c.id == id); + expect(eagerlyHydrated.password, 'redis-secret'); }); test('removeConnection deletes secure-store entries', () async { @@ -146,7 +154,7 @@ void main() { ); await LocalDb.instance.updateConnection(updatedRow); - final list = await LocalDb.instance.getConnections(); + final list = await LocalDb.instance.getConnections(hydrateSecrets: true); final loaded = list.singleWhere((c) => c.id == id); expect(loaded.name, 'PG_Updated'); expect(loaded.host, 'db.example.com'); @@ -234,7 +242,7 @@ void main() { throwsA(isA()), ); - final list = await LocalDb.instance.getConnections(); + final list = await LocalDb.instance.getConnections(hydrateSecrets: true); final loaded = list.singleWhere((c) => c.id == id); expect(loaded.name, 'PG_Before'); expect(loaded.host, 'localhost'); @@ -273,10 +281,65 @@ void main() { expect(merged.host, 'db.example.com'); await LocalDb.instance.updateConnection(merged); - final loaded = (await LocalDb.instance.getConnections()) + final loaded = (await LocalDb.instance.getConnections(hydrateSecrets: true)) .singleWhere((c) => c.id == id); expect(loaded.password, 'keep-me'); expect(loaded.name, 'PG Renamed'); }); + + test('getConnections does not read secure store by default', () async { + const row = ConnectionRow( + type: 'mysql', + name: 'MySQL_Lazy', + host: 'localhost', + port: 3306, + username: 'root', + password: 'super-secret-pw', + createdAt: '2026-01-01T00:00:00Z', + ); + final id = await LocalDb.instance.addConnection(row); + + // failNextRead should NOT be triggered because getConnections() does not read secrets! + testMemorySecrets.failNextRead = StateError('should not be called'); + + final list = await LocalDb.instance.getConnections(); + final conn = list.singleWhere((c) => c.id == id); + expect(conn.name, 'MySQL_Lazy'); + expect(conn.password, isNull); + + // failNextRead is still set because read was never called + expect(testMemorySecrets.failNextRead, isNotNull); + testMemorySecrets.failNextRead = null; + }); + + test( + 'readForConnection and _hydrateConnection handle Keychain error gracefully without throwing', + () async { + const row = ConnectionRow( + type: 'mysql', + name: 'MySQL_Faulty', + host: 'localhost', + port: 3306, + username: 'root', + password: 'secret-password', + createdAt: '2026-01-01T00:00:00Z', + ); + final id = await LocalDb.instance.addConnection(row); + + testMemorySecrets.failNextRead = StateError('org.freedesktop.DBus.Error.NoReply'); + + // readForConnection should return nulls instead of throwing + final secrets = await ConnectionSecretsStore.readForConnection(id); + expect(secrets.password, isNull); + expect(secrets.connectionString, isNull); + + testMemorySecrets.failNextRead = StateError('Keychain locked'); + + // getConnections(hydrateSecrets: true) should still succeed and return the connection row + final list = await LocalDb.instance.getConnections(hydrateSecrets: true); + final conn = list.singleWhere((c) => c.id == id); + expect(conn.name, 'MySQL_Faulty'); + expect(conn.password, isNull); + }); }); } diff --git a/test/memory_secrets_backend.dart b/test/memory_secrets_backend.dart index 28efca4..5253a20 100644 --- a/test/memory_secrets_backend.dart +++ b/test/memory_secrets_backend.dart @@ -8,6 +8,9 @@ final MemorySecretsStorageBackend testMemorySecrets = class MemorySecretsStorageBackend implements SecretsStorageBackend { final Map _values = {}; + /// When non-null, the next [read] throws this error (then clears the flag). + Object? failNextRead; + /// When non-null, the next [write] throws this error (then clears the flag). Object? failNextWrite; @@ -15,7 +18,14 @@ class MemorySecretsStorageBackend implements SecretsStorageBackend { Object? failNextDelete; @override - Future read(String key) async => _values[key]; + Future read(String key) async { + final fail = failNextRead; + if (fail != null) { + failNextRead = null; + throw fail; + } + return _values[key]; + } @override Future write(String key, String? value) async { @@ -43,6 +53,7 @@ class MemorySecretsStorageBackend implements SecretsStorageBackend { void clear() { _values.clear(); + failNextRead = null; failNextWrite = null; failNextDelete = null; }