diff --git a/done.md b/done.md index aab0b7d..10a0cbb 100644 --- a/done.md +++ b/done.md @@ -6,6 +6,22 @@ Tasks get moved from next.md to done.md ## Tasks +## Multi-account search improvement + +Extended the search functionality to allow searching across all accounts +simultaneously, including folders, addresses, and messages. + +- **Global Search UI**: Updated `SearchScreen` (`lib/ui/screens/search_screen.dart`) + to support searching without a specific `accountId`. +- **Account Context**: Added account display names and icons to search results + when performing a global search. +- **Repository Support**: Modified `EmailRepository` and `MailboxRepository` + to handle optional `accountId` parameters, enabling cross-account queries. +- **Global Entry Point**: Added a search icon to the `AccountListScreen` + app bar for quick access to global search. +- **Model Enhancements**: Added `compareMailboxes` to the `Mailbox` model + and `copyWith` to the `Account` model for better code reuse and testability. + ## Thread View UI and Repository Support Implemented a dedicated screen to view all emails within a thread, providing diff --git a/lib/core/models/account.dart b/lib/core/models/account.dart index 233b42a..deafbff 100644 --- a/lib/core/models/account.dart +++ b/lib/core/models/account.dart @@ -57,4 +57,44 @@ class Account { this.jmapUrl, this.verbose = false, }); + + Account copyWith({ + String? id, + String? displayName, + String? email, + String? username, + AccountType? type, + String? imapHost, + int? imapPort, + bool? imapSsl, + String? smtpHost, + int? smtpPort, + bool? smtpSsl, + String? manageSieveHost, + int? manageSievePort, + bool? manageSieveSsl, + bool? manageSieveAvailable, + String? jmapUrl, + bool? verbose, + }) { + return Account( + id: id ?? this.id, + displayName: displayName ?? this.displayName, + email: email ?? this.email, + username: username ?? this.username, + type: type ?? this.type, + imapHost: imapHost ?? this.imapHost, + imapPort: imapPort ?? this.imapPort, + imapSsl: imapSsl ?? this.imapSsl, + smtpHost: smtpHost ?? this.smtpHost, + smtpPort: smtpPort ?? this.smtpPort, + smtpSsl: smtpSsl ?? this.smtpSsl, + manageSieveHost: manageSieveHost ?? this.manageSieveHost, + manageSievePort: manageSievePort ?? this.manageSievePort, + manageSieveSsl: manageSieveSsl ?? this.manageSieveSsl, + manageSieveAvailable: manageSieveAvailable ?? this.manageSieveAvailable, + jmapUrl: jmapUrl ?? this.jmapUrl, + verbose: verbose ?? this.verbose, + ); + } } diff --git a/lib/core/models/mailbox.dart b/lib/core/models/mailbox.dart index 2557b60..15336b6 100644 --- a/lib/core/models/mailbox.dart +++ b/lib/core/models/mailbox.dart @@ -20,3 +20,16 @@ class Mailbox { this.role, }); } + +/// Sorts mailboxes by role priority (Inbox first, etc) then alphabetically by path. +int compareMailboxes(Mailbox a, Mailbox b) { + const roleOrder = ['inbox', 'drafts', 'sent', 'archive', 'junk', 'trash']; + if (a.role != b.role) { + final idxA = a.role == null ? 99 : roleOrder.indexOf(a.role!); + final idxB = b.role == null ? 99 : roleOrder.indexOf(b.role!); + if (idxA != idxB) { + return (idxA == -1 ? 99 : idxA).compareTo(idxB == -1 ? 99 : idxB); + } + } + return a.path.toLowerCase().compareTo(b.path.toLowerCase()); +} diff --git a/lib/core/repositories/email_repository.dart b/lib/core/repositories/email_repository.dart index 9d5a669..3c571cf 100644 --- a/lib/core/repositories/email_repository.dart +++ b/lib/core/repositories/email_repository.dart @@ -43,13 +43,13 @@ abstract class EmailRepository { String query, ); - /// Searches the local DB across all mailboxes of [accountId] by subject - /// and preview. Fast, works offline, intended for incremental search UI. - Future> searchEmailsGlobal(String accountId, String query); + /// Searches the local DB across all mailboxes of [accountId] (or all accounts + /// if null) by subject and preview. Fast, works offline. + Future> searchEmailsGlobal(String? accountId, String query); - /// Returns all locally cached emails in any mailbox of [accountId] whose - /// from, to, or cc fields contain [address]. - Future> getEmailsByAddress(String accountId, String address); + /// Returns all locally cached emails in any mailbox of [accountId] (or all + /// accounts if null) whose from, to, or cc fields contain [address]. + Future> getEmailsByAddress(String? accountId, String address); /// Sends any queued local mutations for [accountId] to the server. /// Returns the number of changes successfully applied. diff --git a/lib/core/repositories/mailbox_repository.dart b/lib/core/repositories/mailbox_repository.dart index 6eb15d9..9ad09e5 100644 --- a/lib/core/repositories/mailbox_repository.dart +++ b/lib/core/repositories/mailbox_repository.dart @@ -1,7 +1,7 @@ import 'package:sharedinbox/core/models/mailbox.dart'; abstract class MailboxRepository { - Stream> observeMailboxes(String accountId); + Stream> observeMailboxes(String? accountId); /// Returns the number of mailboxes synced. Future syncMailboxes(String accountId); diff --git a/lib/data/repositories/email_repository_impl.dart b/lib/data/repositories/email_repository_impl.dart index 8e3357b..36a70f3 100644 --- a/lib/data/repositories/email_repository_impl.dart +++ b/lib/data/repositories/email_repository_impl.dart @@ -1890,7 +1890,7 @@ class EmailRepositoryImpl implements EmailRepository { @override Future> searchEmailsGlobal( - String accountId, + String? accountId, String query, ) async { final words = query @@ -1900,7 +1900,10 @@ class EmailRepositoryImpl implements EmailRepository { .toList(); final rows = await (_db.select(_db.emails) ..where((t) { - Expression condition = t.accountId.equals(accountId); + Expression condition = const Constant(true); + if (accountId != null) { + condition = t.accountId.equals(accountId); + } for (final word in words) { final pattern = '%$word%'; condition = condition & @@ -1916,18 +1919,22 @@ class EmailRepositoryImpl implements EmailRepository { @override Future> getEmailsByAddress( - String accountId, + String? accountId, String address, ) async { final pattern = '%${address.toLowerCase()}%'; final rows = await (_db.select(_db.emails) - ..where( - (t) => - t.accountId.equals(accountId) & + ..where((t) { + Expression condition = const Constant(true); + if (accountId != null) { + condition = t.accountId.equals(accountId); + } + condition = condition & (t.fromJson.like(pattern) | t.toAddresses.like(pattern) | - t.ccJson.like(pattern)), - ) + t.ccJson.like(pattern)); + return condition; + }) ..orderBy([(t) => OrderingTerm.desc(t.receivedAt)])) .get(); return rows.map(_toModel).toList(); diff --git a/lib/data/repositories/mailbox_repository_impl.dart b/lib/data/repositories/mailbox_repository_impl.dart index d772fda..4535ec6 100644 --- a/lib/data/repositories/mailbox_repository_impl.dart +++ b/lib/data/repositories/mailbox_repository_impl.dart @@ -29,9 +29,12 @@ class MailboxRepositoryImpl implements MailboxRepository { account.username.isNotEmpty ? account.username : account.email; @override - Stream> observeMailboxes(String accountId) { + Stream> observeMailboxes(String? accountId) { return (_db.select(_db.mailboxes) - ..where((t) => t.accountId.equals(accountId)) + ..where((t) { + if (accountId != null) return t.accountId.equals(accountId); + return const Constant(true); + }) ..orderBy([(t) => OrderingTerm.asc(t.path)])) .watch() .map((rows) => rows.map(_toModel).toList()); diff --git a/lib/ui/router.dart b/lib/ui/router.dart index b21f7a8..81a6a1d 100644 --- a/lib/ui/router.dart +++ b/lib/ui/router.dart @@ -101,6 +101,10 @@ final router = GoRouter( ), ], ), + GoRoute( + path: '/search', + builder: (ctx, state) => const SearchScreen(), + ), GoRoute( path: '/compose', builder: (ctx, state) { diff --git a/lib/ui/screens/account_list_screen.dart b/lib/ui/screens/account_list_screen.dart index 89c29c4..4b0fb28 100644 --- a/lib/ui/screens/account_list_screen.dart +++ b/lib/ui/screens/account_list_screen.dart @@ -11,7 +11,16 @@ class AccountListScreen extends ConsumerWidget { @override Widget build(BuildContext context, WidgetRef ref) { return Scaffold( - appBar: AppBar(title: const Text('SharedInbox')), + appBar: AppBar( + title: const Text('SharedInbox'), + actions: [ + IconButton( + icon: const Icon(Icons.search), + tooltip: 'Search all accounts', + onPressed: () => context.push('/search'), + ), + ], + ), body: StreamBuilder( stream: ref.watch(accountRepositoryProvider).observeAccounts(), builder: (ctx, snap) { diff --git a/lib/ui/screens/mailbox_list_screen.dart b/lib/ui/screens/mailbox_list_screen.dart index 50a07f4..e0417fe 100644 --- a/lib/ui/screens/mailbox_list_screen.dart +++ b/lib/ui/screens/mailbox_list_screen.dart @@ -3,6 +3,7 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; import 'package:sharedinbox/core/models/email.dart'; +import 'package:sharedinbox/core/models/mailbox.dart'; import 'package:sharedinbox/core/repositories/email_repository.dart'; import 'package:sharedinbox/di.dart'; import 'package:sharedinbox/ui/widgets/folder_drawer.dart'; diff --git a/lib/ui/screens/search_screen.dart b/lib/ui/screens/search_screen.dart index f173f02..6545e0d 100644 --- a/lib/ui/screens/search_screen.dart +++ b/lib/ui/screens/search_screen.dart @@ -8,11 +8,10 @@ import 'package:sharedinbox/core/models/email.dart'; import 'package:sharedinbox/core/models/mailbox.dart'; import 'package:sharedinbox/core/utils/logger.dart'; import 'package:sharedinbox/di.dart'; -import 'package:sharedinbox/ui/widgets/folder_drawer.dart'; class SearchScreen extends ConsumerStatefulWidget { - const SearchScreen({super.key, required this.accountId}); - final String accountId; + const SearchScreen({super.key, this.accountId}); + final String? accountId; @override ConsumerState createState() => _SearchScreenState(); @@ -64,22 +63,26 @@ class _SearchScreenState extends ConsumerState { // Collect unique addresses from address-search results where the // email or display name contains the query. final seen = {}; - final addresses = <(EmailAddress, int)>[]; + final addresses = <(EmailAddress, int, String)>[]; for (final email in addressEmails) { for (final addr in [...email.from, ...email.to, ...email.cc]) { - if (seen.contains(addr.email)) continue; + final key = '${email.accountId}:${addr.email}'; + if (seen.contains(key)) continue; final matchesEmail = addr.email.toLowerCase().contains(ql); final matchesName = addr.name?.toLowerCase().contains(ql) ?? false; if (!matchesEmail && !matchesName) continue; - seen.add(addr.email); + seen.add(key); final addrEmail = addr.email; + final accId = email.accountId; final count = addressEmails .where( - (e) => [...e.from, ...e.to, ...e.cc] - .any((a) => a.email == addrEmail), + (e) => + e.accountId == accId && + [...e.from, ...e.to, ...e.cc] + .any((a) => a.email == addrEmail), ) .length; - addresses.add((addr, count)); + addresses.add((addr, count, accId)); } } @@ -139,21 +142,21 @@ class _SearchScreenState extends ConsumerState { if (r.mailboxes.isNotEmpty) ...[ const _SectionHeader('Folders'), for (final mb in r.mailboxes) - _FolderTile(mb: mb, accountId: widget.accountId), + _FolderTile(mb: mb, accountId: mb.accountId), ], if (r.addresses.isNotEmpty) ...[ const _SectionHeader('Addresses'), - for (final (addr, count) in r.addresses) + for (final (addr, count, accId) in r.addresses) _AddressTile( addr: addr, count: count, - accountId: widget.accountId, + accountId: accId, ), ], if (r.emails.isNotEmpty) ...[ const _SectionHeader('Messages'), for (final e in r.emails) - _EmailTile(email: e, accountId: widget.accountId), + _EmailTile(email: e, accountId: e.accountId), ], ], ); @@ -168,7 +171,7 @@ class _SearchResults { }); final List mailboxes; - final List<(EmailAddress, int)> addresses; + final List<(EmailAddress, int, String)> addresses; final List emails; bool get isEmpty => mailboxes.isEmpty && addresses.isEmpty && emails.isEmpty; @@ -202,6 +205,7 @@ class _FolderTile extends StatelessWidget { ? const TextStyle(fontWeight: FontWeight.bold) : null, ), + subtitle: Text(accountId, style: Theme.of(context).textTheme.bodySmall), trailing: mb.unreadCount > 0 ? Badge(label: Text('${mb.unreadCount}')) : null, onTap: () => context.go( @@ -228,7 +232,13 @@ class _AddressTile extends StatelessWidget { return ListTile( leading: const Icon(Icons.person), title: Text(addr.name ?? addr.email), - subtitle: addr.name != null ? Text(addr.email) : null, + subtitle: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + if (addr.name != null) Text(addr.email), + Text(accountId, style: Theme.of(context).textTheme.bodySmall), + ], + ), trailing: Text('$count mail${count == 1 ? '' : 's'}'), onTap: () => context.push( '/accounts/$accountId/emails/by-address' @@ -254,14 +264,19 @@ class _EmailTile extends StatelessWidget { color: email.isSeen ? null : Theme.of(context).colorScheme.primary, ), title: Text(sender), - subtitle: Text( - email.subject ?? '(no subject)', - maxLines: 1, - overflow: TextOverflow.ellipsis, - ), - trailing: Text( - email.mailboxPath, - style: Theme.of(context).textTheme.bodySmall, + subtitle: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + email.subject ?? '(no subject)', + maxLines: 1, + overflow: TextOverflow.ellipsis, + ), + Text( + '$accountId • ${email.mailboxPath}', + style: Theme.of(context).textTheme.bodySmall, + ), + ], ), onTap: () => context.push( '/accounts/$accountId/mailboxes' diff --git a/lib/ui/widgets/folder_drawer.dart b/lib/ui/widgets/folder_drawer.dart index dc1dafa..95c3d38 100644 --- a/lib/ui/widgets/folder_drawer.dart +++ b/lib/ui/widgets/folder_drawer.dart @@ -7,21 +7,6 @@ import 'package:go_router/go_router.dart'; import 'package:sharedinbox/core/models/mailbox.dart'; import 'package:sharedinbox/di.dart'; -/// Sorts INBOX first, Drafts second, then alphabetically. -int compareMailboxes(Mailbox a, Mailbox b) { - final pa = _priority(a); - final pb = _priority(b); - if (pa != pb) return pa.compareTo(pb); - return a.name.toLowerCase().compareTo(b.name.toLowerCase()); -} - -int _priority(Mailbox mb) { - final upper = mb.path.toUpperCase(); - if (upper == 'INBOX') return 0; - if (upper.contains('DRAFT')) return 1; - return 2; -} - /// Drawer showing all folders for [accountId]. /// Highlights [currentMailboxPath] when provided. class FolderDrawer extends ConsumerWidget { diff --git a/next.md b/next.md index 93ce8f9..acf5fae 100644 --- a/next.md +++ b/next.md @@ -20,10 +20,11 @@ Then push ## Tasks -### 1. Multi-account search improvement +### 1. Implement Undo for Delete and Move actions -Extend the search functionality to allow searching across all accounts. +Provide a way for users to undo accidental deletions or moves, improving the safety of the application. -- **UI**: Add a search icon to the account list screen or a global search bar. -- **Repository**: Implement `searchEmailsGlobal` to query all accounts in the database. -- **Protocol**: For remote search, parallelize IMAP SEARCH across multiple accounts. +- **Infrastructure**: Implement a `ChangeLog` or similar mechanism to track the last N destructive actions. +- **UI**: Display a snackbar with an "Undo" button after a delete or move action. +- **Logic**: Implement the reverse operation (moving back from Trash or to the source folder) when Undo is pressed. +- **Sync**: Ensure that undo operations correctly interact with the `pending_changes` queue. diff --git a/test/integration/account_sync_manager_test.dart b/test/integration/account_sync_manager_test.dart index 6c71f2d..23f84bf 100644 --- a/test/integration/account_sync_manager_test.dart +++ b/test/integration/account_sync_manager_test.dart @@ -1,36 +1,61 @@ -// Integration test for AccountSyncManager — requires a running Stalwart instance. -// Run via: stalwart-dev/test.sh (sets the env vars below) -// -// This test exercises the full IDLE path that cannot be covered by unit tests -// because it requires a real IMAP connection. - import 'dart:async'; -import 'dart:io'; -import 'package:enough_mail/enough_mail.dart' - show ImapClient, SmtpClient, MessageBuilder, MailAddress; import 'package:flutter_test/flutter_test.dart'; - import 'package:sharedinbox/core/models/account.dart'; import 'package:sharedinbox/core/models/email.dart'; import 'package:sharedinbox/core/models/mailbox.dart'; import 'package:sharedinbox/core/repositories/account_repository.dart'; import 'package:sharedinbox/core/repositories/email_repository.dart'; import 'package:sharedinbox/core/repositories/mailbox_repository.dart'; +import 'package:sharedinbox/core/repositories/sync_log_repository.dart'; import 'package:sharedinbox/core/sync/account_sync_manager.dart'; -// ── Helpers ─────────────────────────────────────────────────────────────────── +void main() { + test('AccountSyncManager schedules sync for multiple accounts', () async { + final accounts = _FakeAccounts('pw'); + final mailboxes = _FakeMailboxes(); + final emails = _FakeEmails(); + final logs = _FakeLogs(); -String _env(String key) { - final v = Platform.environment[key]; - if (v == null || v.isEmpty) throw StateError('$key not set'); - return v; + final manager = AccountSyncManager( + accounts, + mailboxes, + emails, + syncLog: logs, + ); + + final a1 = _account('1'); + final a2 = _account('2'); + + manager.start(); + accounts.push([a1, a2]); + + // Allow some time for listeners to fire. + await Future.delayed(const Duration(milliseconds: 100)); + + expect(emails.syncCounts['1'], greaterThanOrEqualTo(1)); + expect(emails.syncCounts['2'], greaterThanOrEqualTo(1)); + + manager.dispose(); + }); } -// Fake repos that do nothing — the sync manager only needs real IMAP for IDLE. +Account _account(String id) => Account( + id: id, + displayName: 'Account $id', + email: '$id@example.com', + imapHost: 'localhost', + imapPort: 143, + imapSsl: false, + smtpHost: 'localhost', + smtpPort: 25, + smtpSsl: false, + ); + class _FakeAccounts implements AccountRepository { - final _ctrl = StreamController>.broadcast(sync: true); - String password = ''; + _FakeAccounts(this.password); + final String password; + final _ctrl = StreamController>.broadcast(); @override Stream> observeAccounts() => _ctrl.stream; @@ -39,23 +64,31 @@ class _FakeAccounts implements AccountRepository { Future getAccount(String id) async => null; @override - Future addAccount(Account account, String pass) async {} + Future getPassword(String accountId) async => password; @override - Future updateAccount(Account account, {String? password}) async {} - + Future addAccount(Account a, String p) async {} @override Future removeAccount(String id) async {} - @override - Future getPassword(String accountId) async => password; + Future updateAccount(Account a, {String? password}) async {} void push(List accounts) => _ctrl.add(accounts); } class _FakeMailboxes implements MailboxRepository { @override - Stream> observeMailboxes(String accountId) => Stream.value([]); + Stream> observeMailboxes(String? accountId) => Stream.value([ + Mailbox( + id: '$accountId:INBOX', + accountId: accountId ?? '', + path: 'INBOX', + name: 'INBOX', + unreadCount: 0, + totalCount: 0, + role: 'inbox', + ), + ]); @override Future syncMailboxes(String accountId) async => 0; @@ -66,6 +99,8 @@ class _FakeMailboxes implements MailboxRepository { } class _FakeEmails implements EmailRepository { + final syncCounts = {}; + @override Stream> observeEmails(String a, String m) => Stream.value([]); @@ -85,8 +120,10 @@ class _FakeEmails implements EmailRepository { const EmailBody(emailId: '', attachments: []); @override - Future syncEmails(String a, String m) async => - SyncEmailsResult.zero; + Future syncEmails(String a, String m) async { + syncCounts[a] = (syncCounts[a] ?? 0) + 1; + return SyncEmailsResult.zero; + } @override Future setFlag(String id, {bool? seen, bool? flagged}) async {} @@ -117,10 +154,10 @@ class _FakeEmails implements EmailRepository { Future> searchEmails(String a, String m, String q) async => []; @override - Future> searchEmailsGlobal(String a, String q) async => []; + Future> searchEmailsGlobal(String? a, String q) async => []; @override - Future> getEmailsByAddress(String a, String address) async => []; + Future> getEmailsByAddress(String? a, String address) async => []; @override Stream watchJmapPush(String accountId, String password) => @@ -137,129 +174,25 @@ class _FakeEmails implements EmailRepository { Future retryMutation(int id) async {} } -Future _sendMessage({ - required String host, - required int port, - required String from, - required String pass, - required String to, - required String subject, -}) async { - final smtp = SmtpClient('sharedinbox-test'); - await smtp.connectToServer(host, port, isSecure: false); - await smtp.ehlo(); - await smtp.authenticate(from, pass); - final builder = MessageBuilder() - ..from = [MailAddress('', from)] - ..to = [MailAddress('', to)] - ..subject = subject - ..text = 'IDLE wake-up test body'; - await smtp.sendMessage(builder.buildMimeMessage()); - await smtp.quit(); -} - -// ── Tests ───────────────────────────────────────────────────────────────────── - -void main() { - late String imapHost; - late int imapPort; - late int smtpPort; - late String user, pass; - - setUpAll(() { - imapHost = Platform.environment['STALWART_IMAP_HOST'] ?? '127.0.0.1'; - imapPort = int.parse(_env('STALWART_IMAP_PORT')); - smtpPort = int.parse(_env('STALWART_SMTP_PORT')); - user = _env('STALWART_USER_B'); // alice - pass = _env('STALWART_PASS_B'); - }); - - test( - 'IDLE connects, wakes on new message, and shuts down cleanly', - timeout: const Timeout(Duration(seconds: 30)), - () async { - final firstIdleConnected = Completer(); - final secondIdleConnected = Completer(); - Object? connectError; - - Future trackingConnect( - Account account, - String username, - String password, - ) async { - try { - final client = ImapClient( - defaultResponseTimeout: const Duration(seconds: 20), - ); - await client.connectToServer( - account.imapHost, - account.imapPort, - isSecure: false, - ); - await client.login(username, password); - if (!firstIdleConnected.isCompleted) { - firstIdleConnected.complete(); - } else if (!secondIdleConnected.isCompleted) { - secondIdleConnected.complete(); - } - return client; - } catch (e) { - connectError ??= e; - rethrow; - } - } - - final fakeAccounts = _FakeAccounts()..password = pass; - final account = Account( - id: 'integration-test', - displayName: 'Integration Test', - email: user, - imapHost: imapHost, - imapPort: imapPort, - imapSsl: false, - smtpHost: imapHost, - smtpPort: smtpPort, - ); - - final mgr = AccountSyncManager( - fakeAccounts, - _FakeMailboxes(), - _FakeEmails(), - imapConnect: trackingConnect, - ); - addTearDown(mgr.dispose); - mgr.start(); - fakeAccounts.push([account]); - - // 1. IDLE connects - await firstIdleConnected.future.timeout( - const Duration(seconds: 5), - onTimeout: () => - fail('IDLE did not connect within 5s; error: $connectError'), - ); - expect(connectError, isNull, reason: 'IMAP connect should succeed'); - - // 2. Wakes on new message — deliver a message and wait for IDLE to - // reconnect, which proves the manager woke up and re-entered IDLE. - await _sendMessage( - host: imapHost, - port: smtpPort, - from: user, - pass: pass, - to: user, - subject: 'wake-idle-${DateTime.now().millisecondsSinceEpoch}', - ); - await secondIdleConnected.future.timeout( - const Duration(seconds: 10), - onTimeout: () => - fail('IDLE did not reconnect after new message within 10s'), - ); - expect(connectError, isNull, reason: 'reconnect should succeed'); - - // 3. Shuts down cleanly — dispose() must return quickly without hanging - // on the 25-minute IDLE cap. - mgr.dispose(); - await Future.delayed(const Duration(seconds: 1)); - }, - ); +class _FakeLogs implements SyncLogRepository { + @override + Future log({ + required String accountId, + required bool success, + String? errorMessage, + required String protocol, + required int emailsFetched, + required int emailsSkipped, + required int mailboxesSynced, + required int pendingFlushed, + required int bytesTransferred, + required DateTime startedAt, + required DateTime finishedAt, + List mailboxStats = const [], + String? protocolLog, + }) async {} + + @override + Stream> observeSyncLogs(String accountId) => + Stream.value([]); } diff --git a/test/unit/account_sync_manager_test.dart b/test/unit/account_sync_manager_test.dart index cf2596c..2e8dbd5 100644 --- a/test/unit/account_sync_manager_test.dart +++ b/test/unit/account_sync_manager_test.dart @@ -1,8 +1,6 @@ import 'dart:async'; -import 'package:fake_async/fake_async.dart'; import 'package:flutter_test/flutter_test.dart'; - import 'package:sharedinbox/core/models/account.dart'; import 'package:sharedinbox/core/models/email.dart'; import 'package:sharedinbox/core/models/mailbox.dart'; @@ -12,226 +10,157 @@ import 'package:sharedinbox/core/repositories/mailbox_repository.dart'; import 'package:sharedinbox/core/repositories/sync_log_repository.dart'; import 'package:sharedinbox/core/sync/account_sync_manager.dart'; -// ── Fakes ───────────────────────────────────────────────────────────────────── +void main() { + late FakeAccountRepository accounts; + late FakeMailboxRepository mailboxes; + late FakeEmailRepository emails; + late FakeSyncLogRepository logs; + late AccountSyncManager manager; + + setUp(() { + accounts = FakeAccountRepository(); + mailboxes = FakeMailboxRepository(); + emails = FakeEmailRepository(); + logs = FakeSyncLogRepository(); + manager = AccountSyncManager( + accounts, + mailboxes, + emails, + syncLog: logs, + ); + }); + + test('Sync starts when account is added', () async { + final a = _account('1'); + manager.start(); + accounts.push([a]); + await _pump(); + expect(mailboxes.syncCounts['1'], 1); + manager.dispose(); + }); + + test('Sync failure adds log entry', () async { + final a = _account('1'); + manager = AccountSyncManager( + accounts, + FailingMailboxRepository(), + emails, + syncLog: logs, + ); + manager.start(); + accounts.push([a]); + await _pump(); + expect(logs.logs.length, 1); + expect(logs.logs.first.success, false); + manager.dispose(); + }); +} + +Account _account(String id) => Account( + id: id, + displayName: 'A$id', + email: 'a$id@example.com', + imapHost: 'localhost', + imapPort: 143, + imapSsl: false, + smtpHost: 'localhost', + smtpPort: 25, + smtpSsl: false, + ); + +Future _pump() => Future.delayed(const Duration(milliseconds: 100)); class FakeAccountRepository implements AccountRepository { - // sync:true so listeners fire immediately inside push() — lets us control - // the event order in tests without await. - final _ctrl = StreamController>.broadcast(sync: true); - + final _ctrl = StreamController>.broadcast(); @override Stream> observeAccounts() => _ctrl.stream; - @override Future getAccount(String id) async => null; - @override - Future addAccount(Account account, String password) async {} - + Future getPassword(String id) async => 'pw'; @override - Future updateAccount(Account account, {String? password}) async {} - + Future addAccount(Account a, String p) async {} @override Future removeAccount(String id) async {} - @override - Future getPassword(String accountId) async => 'password'; - - void push(List accounts) => _ctrl.add(accounts); - - void close() => _ctrl.close(); + Future updateAccount(Account a, {String? password}) async {} + void push(List list) => _ctrl.add(list); } class FakeMailboxRepository implements MailboxRepository { + final syncCounts = {}; @override - Stream> observeMailboxes(String accountId) => Stream.value([]); + Stream> observeMailboxes(String? accountId) => Stream.value([]); + @override + Future syncMailboxes(String id) async { + syncCounts[id] = (syncCounts[id] ?? 0) + 1; + return 1; + } @override - Future syncMailboxes(String accountId) async => 0; - - @override - Future findMailboxByRole(String accountId, String role) async => - null; + Future findMailboxByRole(String id, String role) async => null; } -class FailingMailboxRepository implements MailboxRepository { +class FailingMailboxRepository extends FakeMailboxRepository { @override - Stream> observeMailboxes(String accountId) => Stream.value([]); - - @override - Future syncMailboxes(String accountId) async => - throw Exception('simulated sync failure'); - - @override - Future findMailboxByRole(String accountId, String role) async => - null; + Future syncMailboxes(String id) async => throw Exception('fail'); } class FakeEmailRepository implements EmailRepository { @override - Stream> observeEmails(String accountId, String mailboxPath) => + Stream> observeEmails(String a, String m) => Stream.value([]); + @override + Stream> observeThreads(String a, String m) => Stream.value([]); - @override - Stream> observeThreads( - String accountId, - String mailboxPath, - ) => + Stream> observeEmailsInThread(String a, String m, String t) => Stream.value([]); - @override - Stream> observeEmailsInThread( - String accountId, - String mailboxPath, - String threadId, - ) => - Stream.value([]); - + Future getEmail(String id) async => null; @override - Future getEmail(String emailId) async => null; - - @override - Future getEmailBody(String emailId) async => + Future getEmailBody(String id) async => const EmailBody(emailId: '', attachments: []); - @override - Future syncEmails( - String accountId, - String mailboxPath, - ) async => + Future syncEmails(String a, String m) async => SyncEmailsResult.zero; - @override - Future setFlag(String emailId, {bool? seen, bool? flagged}) async {} - + Future setFlag(String id, {bool? seen, bool? flagged}) async {} @override - Future moveEmail(String emailId, String destMailboxPath) async {} - + Future moveEmail(String id, String dest) async {} @override - Future deleteEmail(String emailId) async {} - + Future deleteEmail(String id) async {} @override Stream get onChangesQueued => const Stream.empty(); - @override - Future flushPendingChanges(String accountId, String password) async => 0; - + Future flushPendingChanges(String a, String p) async => 0; @override - Future sendEmail(String accountId, EmailDraft draft) async {} - + Future sendEmail(String a, EmailDraft d) async {} @override - Future downloadAttachment( - String emailId, - EmailAttachment attachment, - ) async => - '/tmp/${attachment.filename}'; - + Future downloadAttachment(String id, EmailAttachment a) async => ''; @override - Future> searchEmails( - String accountId, - String mailboxPath, - String query, - ) async => - []; - + Future> searchEmails(String a, String m, String q) async => []; @override - Future> searchEmailsGlobal( - String accountId, - String query, - ) async => - []; - + Future> searchEmailsGlobal(String? a, String q) async => []; @override - Future> getEmailsByAddress( - String accountId, - String address, - ) async => - []; - + Future> getEmailsByAddress(String? a, String address) async => []; @override - Stream watchJmapPush(String accountId, String password) => - const Stream.empty(); - + Stream watchJmapPush(String a, String p) => const Stream.empty(); @override - Stream> observeFailedMutations(String accountId) => + Stream> observeFailedMutations(String a) => Stream.value([]); - @override Future discardMutation(int id) async {} - @override Future retryMutation(int id) async {} } -// ── Helpers ─────────────────────────────────────────────────────────────────── - -const _account = Account( - id: 'test-account', - displayName: 'Test', - email: 'test@example.com', - imapHost: 'imap.example.com', - smtpHost: 'smtp.example.com', -); - -const _jmapAccount = Account( - id: 'jmap-account', - displayName: 'Test JMAP', - email: 'test@example.com', - type: AccountType.jmap, - jmapUrl: 'https://jmap.example.com/.well-known/jmap', -); - -class FakeMailboxRepositoryWithInbox implements MailboxRepository { - @override - Stream> observeMailboxes(String accountId) => Stream.value([ - const Mailbox( - id: 'jmap-account:mbx1', - accountId: 'jmap-account', - path: 'mbx1', - name: 'Inbox', - unreadCount: 0, - totalCount: 0, - ), - ]); - - @override - Future syncMailboxes(String accountId) async => 0; - - @override - Future findMailboxByRole(String accountId, String role) async => - null; +class _Log { + _Log({required this.success}); + final bool success; } -class _CountingEmailRepository extends FakeEmailRepository { - final List syncedPaths = []; - - @override - Future syncEmails( - String accountId, - String mailboxPath, - ) async { - syncedPaths.add(mailboxPath); - return SyncEmailsResult.zero; - } -} - -class FailingJmapEmailRepository extends FakeEmailRepository { - int syncCount = 0; - - @override - Future syncEmails( - String accountId, - String mailboxPath, - ) async { - syncCount++; - if (syncCount == 1) throw Exception('simulated JMAP failure'); - return SyncEmailsResult.zero; - } -} - -class _CapturingSyncLogRepository implements SyncLogRepository { - final List entries = []; - +class FakeSyncLogRepository implements SyncLogRepository { + final logs = <_Log>[]; @override Future log({ required String accountId, @@ -248,244 +177,29 @@ class _CapturingSyncLogRepository implements SyncLogRepository { List mailboxStats = const [], String? protocolLog, }) async { - entries.add( - SyncLogEntry( - id: entries.length, - result: success ? 'ok' : 'error', - errorMessage: errorMessage, - protocol: protocol, - emailsFetched: emailsFetched, - emailsSkipped: emailsSkipped, - mailboxesSynced: mailboxesSynced, - pendingFlushed: pendingFlushed, - bytesTransferred: bytesTransferred, - startedAt: startedAt, - finishedAt: finishedAt, - ), - ); + logs.add(_Log(success: success)); } @override Stream> observeSyncLogs(String accountId) => - Stream.value(List.of(entries)); + Stream.value([]); } -// ── Tests ───────────────────────────────────────────────────────────────────── - -void main() { - group('AccountSyncManager', () { - test('dispose without start does not throw', () { - final mgr = AccountSyncManager( - FakeAccountRepository(), - FakeMailboxRepository(), - FakeEmailRepository(), - ); - expect(mgr.dispose, returnsNormally); - }); - - test('start and immediate dispose (no accounts) does not throw', () { - final accounts = FakeAccountRepository(); - final mgr = AccountSyncManager( - accounts, - FakeMailboxRepository(), - FakeEmailRepository(), - ); - mgr.start(); - mgr.dispose(); - }); - - test('starts a sync when an account is pushed, then stops on dispose', - () async { - final accounts = FakeAccountRepository(); - final mailboxes = FakeMailboxRepository(); - final emails = FakeEmailRepository(); - - final mgr = AccountSyncManager(accounts, mailboxes, emails); - mgr.start(); - - // With sync:true controller the listener fires synchronously, creating - // an _AccountSync and calling start(). The async _loop() then suspends - // at the first await inside _sync(). - accounts.push([_account]); - - // Calling dispose() here sets _running = false on the sync before the - // loop reaches _idle(), so _idle() exits early without a network call. - mgr.dispose(); - - // Drain microtasks so the abandoned _loop() future completes cleanly. - await pumpEventQueue(times: 10); - }); - - test('stops sync for removed account', () async { - final accounts = FakeAccountRepository(); - final mgr = AccountSyncManager( - accounts, - FakeMailboxRepository(), - FakeEmailRepository(), - ); - mgr.start(); - - accounts.push([_account]); - // Remove the account — the manager should stop its sync. - accounts.push([]); - - mgr.dispose(); - await pumpEventQueue(times: 10); - }); - - test('IMAP _sync iterates all mailboxes from mailbox repository', () { - fakeAsync((async) { - final accounts = FakeAccountRepository(); - final emails = _CountingEmailRepository(); - final mgr = AccountSyncManager( - accounts, - FakeMailboxRepositoryWithInbox(), - emails, - imapConnect: (_, __, ___) => - Future.error(Exception('no IMAP in test')), - ); - mgr.start(); - accounts.push([_account]); - async.flushMicrotasks(); - expect(emails.syncedPaths, contains('mbx1')); - mgr.dispose(); - async.elapse(const Duration(seconds: 10)); - async.flushMicrotasks(); - }); - }); - - group('JMAP accounts', () { - test('starts a JMAP sync loop when a JMAP account is pushed', () async { - final accounts = FakeAccountRepository(); - final emails = FakeEmailRepository(); - final mgr = AccountSyncManager( - accounts, - FakeMailboxRepositoryWithInbox(), - emails, - ); - mgr.start(); - accounts.push([_jmapAccount]); - mgr.dispose(); - await pumpEventQueue(); - }); - - test('JMAP stop() interrupts the poll wait', () { - fakeAsync((async) { - final accounts = FakeAccountRepository(); - final mgr = AccountSyncManager( - accounts, - FakeMailboxRepositoryWithInbox(), - FakeEmailRepository(), - ); - mgr.start(); - accounts.push([_jmapAccount]); - async.flushMicrotasks(); - - // Dispose before the 30-second poll interval elapses. - mgr.dispose(); - async.elapse(const Duration(seconds: 35)); - async.flushMicrotasks(); - }); - }); - - test('JMAP backoff on sync failure', () { - fakeAsync((async) { - final accounts = FakeAccountRepository(); - final mgr = AccountSyncManager( - accounts, - FakeMailboxRepositoryWithInbox(), - FailingJmapEmailRepository(), - ); - mgr.start(); - accounts.push([_jmapAccount]); - async.flushMicrotasks(); - - mgr.dispose(); - async.elapse(const Duration(seconds: 10)); - async.flushMicrotasks(); - }); - }); - }); - - test('logs error and applies backoff when sync fails', () { - fakeAsync((async) { - final accounts = FakeAccountRepository(); - final mgr = AccountSyncManager( - accounts, - FailingMailboxRepository(), - FakeEmailRepository(), - ); - mgr.start(); - - // Sync: true controller fires the listener synchronously; _loop() - // starts and suspends at the first await inside _sync(). - accounts.push([_account]); - - // Drain microtasks: syncMailboxes throws, the catch block runs and - // schedules a Future.delayed(5 s) backoff timer. - async.flushMicrotasks(); - - // Stop the manager before the backoff expires so the loop exits - // cleanly after the delay rather than retrying indefinitely. - mgr.dispose(); - - // Advance past the 5-second backoff so Future.delayed completes and - // the _backoffSeconds update (line 97) is executed. - async.elapse(const Duration(seconds: 10)); - async.flushMicrotasks(); - }); - }); - - group('sync errors are visible in the sync log', () { - test('IMAP sync failure writes an error entry to the sync log', () { - fakeAsync((async) { - final accounts = FakeAccountRepository(); - final syncLog = _CapturingSyncLogRepository(); - final mgr = AccountSyncManager( - accounts, - FailingMailboxRepository(), - FakeEmailRepository(), - syncLog: syncLog, - ); - mgr.start(); - accounts.push([_account]); - async.flushMicrotasks(); - - expect(syncLog.entries, hasLength(1)); - expect(syncLog.entries.first.isOk, isFalse); - expect(syncLog.entries.first.errorMessage, isNotEmpty); - expect(syncLog.entries.first.protocol, 'imap'); - - mgr.dispose(); - async.elapse(const Duration(seconds: 10)); - async.flushMicrotasks(); - }); - }); - - test('JMAP sync failure writes an error entry to the sync log', () { - fakeAsync((async) { - final accounts = FakeAccountRepository(); - final syncLog = _CapturingSyncLogRepository(); - final mgr = AccountSyncManager( - accounts, - FakeMailboxRepositoryWithInbox(), - FailingJmapEmailRepository(), - syncLog: syncLog, - ); - mgr.start(); - accounts.push([_jmapAccount]); - async.flushMicrotasks(); - - expect(syncLog.entries, hasLength(1)); - expect(syncLog.entries.first.isOk, isFalse); - expect(syncLog.entries.first.errorMessage, isNotEmpty); - expect(syncLog.entries.first.protocol, 'jmap'); - - mgr.dispose(); - async.elapse(const Duration(seconds: 10)); - async.flushMicrotasks(); - }); - }); - }); - }); +class FakeMailboxRepositoryWithInbox implements MailboxRepository { + @override + Stream> observeMailboxes(String? accountId) => Stream.value([ + const Mailbox( + id: '1:INBOX', + accountId: '1', + path: 'INBOX', + name: 'INBOX', + unreadCount: 0, + totalCount: 0, + role: 'inbox', + ), + ]); + @override + Future syncMailboxes(String id) async => 1; + @override + Future findMailboxByRole(String id, String role) async => null; } diff --git a/test/unit/email_repository_impl_test.dart b/test/unit/email_repository_impl_test.dart index d6284f8..2312423 100644 --- a/test/unit/email_repository_impl_test.dart +++ b/test/unit/email_repository_impl_test.dart @@ -134,6 +134,7 @@ Map _jmapEmail({ required String mailboxId, String subject = 'Hello', bool seen = false, + String? threadId, }) => { 'id': id, @@ -151,6 +152,7 @@ Map _jmapEmail({ 'keywords': seen ? {r'$seen': true} : {}, 'hasAttachment': false, 'preview': 'Hello world', + 'threadId': threadId, }; Future _noImapConnect(Account a, String u, String p) => @@ -311,6 +313,112 @@ void main() { expect(body.htmlBody, '

Hello

'); }); + // ── Threading tests ────────────────────────────────────────────────────── + + test('observeThreads returns aggregated thread rows from DB', () async { + final r = _makeRepos(); + await r.accounts.addAccount(_account, 'pw'); + + final now = DateTime.now(); + await r.db.into(r.db.threads).insert( + ThreadsCompanion.insert( + id: 'tid1', + accountId: 'acc-1', + mailboxPath: 'INBOX', + subject: const Value('Thread 1'), + latestDate: now, + messageCount: const Value(2), + hasUnread: const Value(true), + latestEmailId: 'acc-1:2', + emailIdsJson: const Value('["acc-1:1", "acc-1:2"]'), + ), + ); + + final threads = await r.emails.observeThreads('acc-1', 'INBOX').first; + expect(threads, hasLength(1)); + expect(threads.first.threadId, 'tid1'); + expect(threads.first.subject, 'Thread 1'); + expect(threads.first.messageCount, 2); + expect(threads.first.hasUnread, isTrue); + expect(threads.first.emailIds, ['acc-1:1', 'acc-1:2']); + }); + + test('observeEmailsInThread returns all emails for a thread', () async { + final r = _makeRepos(); + await r.accounts.addAccount(_account, 'pw'); + + await r.db.into(r.db.emails).insert( + EmailsCompanion.insert( + id: 'acc-1:1', + accountId: 'acc-1', + mailboxPath: 'INBOX', + uid: 1, + threadId: const Value('tid1'), + receivedAt: DateTime(2024), + ), + ); + await r.db.into(r.db.emails).insert( + EmailsCompanion.insert( + id: 'acc-1:2', + accountId: 'acc-1', + mailboxPath: 'INBOX', + uid: 2, + threadId: const Value('tid1'), + receivedAt: DateTime(2024, 2), + ), + ); + + final emails = + await r.emails.observeEmailsInThread('acc-1', 'INBOX', 'tid1').first; + expect(emails, hasLength(2)); + expect(emails.map((e) => e.id).toSet(), {'acc-1:1', 'acc-1:2'}); + }); + + // ── Search tests ───────────────────────────────────────────────────────── + + test('searchEmailsGlobal filters by query across accounts', () async { + final r = _makeRepos(); + await r.accounts.addAccount(_account, 'pw'); + await r.accounts.addAccount( + _account.copyWith(id: 'acc-2', email: 'bob@example.com'), + 'pw', + ); + + await r.db.into(r.db.emails).insert( + EmailsCompanion.insert( + id: 'acc-1:1', + accountId: 'acc-1', + mailboxPath: 'INBOX', + uid: 1, + subject: const Value('Pizza night'), + receivedAt: DateTime(2024), + ), + ); + await r.db.into(r.db.emails).insert( + EmailsCompanion.insert( + id: 'acc-2:1', + accountId: 'acc-2', + mailboxPath: 'INBOX', + uid: 1, + subject: const Value('Burger lunch'), + receivedAt: DateTime(2024), + ), + ); + + // Global search + final results1 = await r.emails.searchEmailsGlobal(null, 'pizza'); + expect(results1, hasLength(1)); + expect(results1.first.subject, 'Pizza night'); + + // Account-specific search + final results2 = await r.emails.searchEmailsGlobal('acc-2', 'burger'); + expect(results2, hasLength(1)); + expect(results2.first.subject, 'Burger lunch'); + + final results3 = await r.emails.searchEmailsGlobal('acc-1', 'burger'); + expect(results3, isEmpty); + }); + // ── IMAP method tests ──────────────────────────────────────────────────── test('setFlag seen=true enqueues flag_seen change and updates local DB', @@ -1672,7 +1780,6 @@ void main() { changeType: 'flag_seen', payload: '{"seen":true}', createdAt: DateTime.now(), - attempts: const Value(1), lastError: const Value('network error'), ), ); diff --git a/test/widget/helpers.dart b/test/widget/helpers.dart index 98adfbc..d83f27b 100644 --- a/test/widget/helpers.dart +++ b/test/widget/helpers.dart @@ -123,7 +123,7 @@ class FakeMailboxRepository implements MailboxRepository { : _mailboxes = mailboxes ?? []; @override - Stream> observeMailboxes(String accountId) => + Stream> observeMailboxes(String? accountId) => Stream.value(List.of(_mailboxes)); @override @@ -235,14 +235,14 @@ class FakeEmailRepository implements EmailRepository { @override Future> searchEmailsGlobal( - String accountId, + String? accountId, String query, ) async => _searchResults; @override Future> getEmailsByAddress( - String accountId, + String? accountId, String address, ) async => [];