fix: IMAP full sync via UID SEARCH+FETCH; add sync log UI
- Replace full-sync fetchMessages(1:*) with UID SEARCH ALL + UID FETCH so every message gets a reliable UID on all servers - Guard CONDSTORE select on server capability to avoid BAD from servers that do not advertise CONDSTORE/QRESYNC - Add SyncLogEntry model + observeSyncLogs stream to SyncLogRepository - Add SyncLogScreen with per-entry duration/error display - Wire history icon in SettingsScreen → /accounts/:id/sync-log route - Fix FakeImapClient to expose initialized serverInfo via field override Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
733da201ee
commit
6a457a9f7a
@@ -1,11 +1,29 @@
|
||||
# Later
|
||||
|
||||
done?
|
||||
|
||||
think about that: Maybe we should not mock jmap/imap/smtp. We have a temproary Stalwart.
|
||||
|
||||
Just like mocking DB in Django makes no sense.
|
||||
|
||||
---
|
||||
|
||||
Try Qwen, vscode plugin
|
||||
|
||||
---
|
||||
|
||||
test/unit/fake_imap.dart
|
||||
|
||||
Why is that still needed? We have Stalwart.
|
||||
|
||||
---
|
||||
|
||||
du -a | sort -rn | head
|
||||
|
||||
Why is the directory so big? 4 GByte?
|
||||
|
||||
---
|
||||
|
||||
After Try Connection, show some matching icon next to the text.
|
||||
|
||||
---
|
||||
|
||||
@@ -1,3 +1,22 @@
|
||||
class SyncLogEntry {
|
||||
const SyncLogEntry({
|
||||
required this.id,
|
||||
required this.result,
|
||||
this.errorMessage,
|
||||
required this.startedAt,
|
||||
required this.finishedAt,
|
||||
});
|
||||
|
||||
final int id;
|
||||
final String result; // 'ok' or 'error'
|
||||
final String? errorMessage;
|
||||
final DateTime startedAt;
|
||||
final DateTime finishedAt;
|
||||
|
||||
Duration get duration => finishedAt.difference(startedAt);
|
||||
bool get isOk => result == 'ok';
|
||||
}
|
||||
|
||||
abstract class SyncLogRepository {
|
||||
Future<void> log({
|
||||
required String accountId,
|
||||
@@ -6,6 +25,8 @@ abstract class SyncLogRepository {
|
||||
required DateTime startedAt,
|
||||
required DateTime finishedAt,
|
||||
});
|
||||
|
||||
Stream<List<SyncLogEntry>> observeSyncLogs(String accountId);
|
||||
}
|
||||
|
||||
class NoOpSyncLogRepository implements SyncLogRepository {
|
||||
@@ -19,4 +40,8 @@ class NoOpSyncLogRepository implements SyncLogRepository {
|
||||
required DateTime startedAt,
|
||||
required DateTime finishedAt,
|
||||
}) async {}
|
||||
|
||||
@override
|
||||
Stream<List<SyncLogEntry>> observeSyncLogs(String accountId) =>
|
||||
Stream.value([]);
|
||||
}
|
||||
|
||||
@@ -227,10 +227,14 @@ class EmailRepositoryImpl implements EmailRepository {
|
||||
final client =
|
||||
await _imapConnect(account, _effectiveUsername(account), password);
|
||||
try {
|
||||
// Enable CONDSTORE so the server returns HIGHESTMODSEQ in SELECT and
|
||||
// honours CHANGEDSINCE modifiers on FETCH (RFC 7162).
|
||||
final selectedMailbox =
|
||||
await client.selectMailboxByPath(mailboxPath, enableCondStore: true);
|
||||
// Only request CONDSTORE if the server advertises it. Servers that don't
|
||||
// support the extension may reject SELECT with (CONDSTORE) with BAD.
|
||||
final supportsCondStore = client.serverInfo.supports('CONDSTORE') ||
|
||||
client.serverInfo.supports('QRESYNC');
|
||||
final selectedMailbox = await client.selectMailboxByPath(
|
||||
mailboxPath,
|
||||
enableCondStore: supportsCondStore,
|
||||
);
|
||||
final uidValidity = selectedMailbox.uidValidity ?? 0;
|
||||
final serverModSeq = selectedMailbox.highestModSequence;
|
||||
final resourceType = 'IMAP:$mailboxPath';
|
||||
@@ -248,13 +252,21 @@ class EmailRepositoryImpl implements EmailRepository {
|
||||
))
|
||||
.go();
|
||||
}
|
||||
await _fetchAndUpsertImap(
|
||||
client,
|
||||
account,
|
||||
mailboxPath,
|
||||
imap.MessageSequence.fromAll(),
|
||||
);
|
||||
final maxUid = await _maxLocalUid(account.id, mailboxPath);
|
||||
// Use UID SEARCH ALL + UID FETCH so every message gets a reliable UID.
|
||||
// Regular FETCH 1:* may not populate msg.uid on all servers.
|
||||
final allUids = (await client.uidSearchMessages(searchCriteria: 'ALL'))
|
||||
.matchingSequence
|
||||
?.toList() ??
|
||||
[];
|
||||
if (allUids.isNotEmpty) {
|
||||
await _fetchAndUpsertImap(
|
||||
client,
|
||||
account,
|
||||
mailboxPath,
|
||||
imap.MessageSequence.fromIds(allUids, isUid: true),
|
||||
);
|
||||
}
|
||||
final maxUid = allUids.isEmpty ? 0 : allUids.reduce(math.max);
|
||||
await _saveImapCheckpoint(
|
||||
account.id,
|
||||
resourceType,
|
||||
@@ -391,18 +403,6 @@ class EmailRepositoryImpl implements EmailRepository {
|
||||
}
|
||||
}
|
||||
|
||||
Future<int> _maxLocalUid(String accountId, String mailboxPath) async {
|
||||
final rows = await (_db.select(_db.emails)
|
||||
..where(
|
||||
(t) =>
|
||||
t.accountId.equals(accountId) &
|
||||
t.mailboxPath.equals(mailboxPath),
|
||||
))
|
||||
.get();
|
||||
if (rows.isEmpty) return 0;
|
||||
return rows.map((r) => r.uid).reduce(math.max);
|
||||
}
|
||||
|
||||
Future<Map<String, dynamic>?> _loadImapCheckpoint(
|
||||
String accountId,
|
||||
String resourceType,
|
||||
|
||||
@@ -26,4 +26,26 @@ class SyncLogRepositoryImpl implements SyncLogRepository {
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
@override
|
||||
Stream<List<SyncLogEntry>> observeSyncLogs(String accountId) {
|
||||
return (_db.select(_db.syncLogs)
|
||||
..where((t) => t.accountId.equals(accountId))
|
||||
..orderBy([(t) => OrderingTerm.desc(t.startedAt)])
|
||||
..limit(100))
|
||||
.watch()
|
||||
.map(
|
||||
(rows) => rows
|
||||
.map(
|
||||
(r) => SyncLogEntry(
|
||||
id: r.id,
|
||||
result: r.result,
|
||||
errorMessage: r.errorMessage,
|
||||
startedAt: r.startedAt,
|
||||
finishedAt: r.finishedAt,
|
||||
),
|
||||
)
|
||||
.toList(),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ import 'screens/email_detail_screen.dart';
|
||||
import 'screens/email_list_screen.dart';
|
||||
import 'screens/mailbox_list_screen.dart';
|
||||
import 'screens/settings_screen.dart';
|
||||
import 'screens/sync_log_screen.dart';
|
||||
|
||||
final router = GoRouter(
|
||||
initialLocation: '/accounts',
|
||||
@@ -26,6 +27,12 @@ final router = GoRouter(
|
||||
accountId: state.pathParameters['accountId']!,
|
||||
),
|
||||
),
|
||||
GoRoute(
|
||||
path: ':accountId/sync-log',
|
||||
builder: (ctx, state) => SyncLogScreen(
|
||||
accountId: state.pathParameters['accountId']!,
|
||||
),
|
||||
),
|
||||
GoRoute(
|
||||
path: ':accountId/mailboxes',
|
||||
builder: (ctx, state) =>
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter_riverpod/flutter_riverpod.dart';
|
||||
import 'package:go_router/go_router.dart';
|
||||
|
||||
import '../../di.dart';
|
||||
|
||||
@@ -28,32 +29,42 @@ class SettingsScreen extends ConsumerWidget {
|
||||
leading: const Icon(Icons.account_circle),
|
||||
title: Text(a.displayName),
|
||||
subtitle: Text(a.email),
|
||||
trailing: IconButton(
|
||||
icon: const Icon(Icons.delete),
|
||||
onPressed: () async {
|
||||
final confirm = await showDialog<bool>(
|
||||
context: ctx,
|
||||
builder: (ctx) => AlertDialog(
|
||||
title: const Text('Remove account?'),
|
||||
content: Text(
|
||||
'Remove ${a.displayName}? Local data will be deleted.',
|
||||
),
|
||||
actions: [
|
||||
TextButton(
|
||||
onPressed: () => Navigator.pop(ctx, false),
|
||||
child: const Text('Cancel'),
|
||||
trailing: Row(
|
||||
mainAxisSize: MainAxisSize.min,
|
||||
children: [
|
||||
IconButton(
|
||||
icon: const Icon(Icons.history),
|
||||
tooltip: 'Sync log',
|
||||
onPressed: () => ctx.push('/accounts/${a.id}/sync-log'),
|
||||
),
|
||||
IconButton(
|
||||
icon: const Icon(Icons.delete),
|
||||
onPressed: () async {
|
||||
final confirm = await showDialog<bool>(
|
||||
context: ctx,
|
||||
builder: (ctx) => AlertDialog(
|
||||
title: const Text('Remove account?'),
|
||||
content: Text(
|
||||
'Remove ${a.displayName}? Local data will be deleted.',
|
||||
),
|
||||
actions: [
|
||||
TextButton(
|
||||
onPressed: () => Navigator.pop(ctx, false),
|
||||
child: const Text('Cancel'),
|
||||
),
|
||||
FilledButton(
|
||||
onPressed: () => Navigator.pop(ctx, true),
|
||||
child: const Text('Remove'),
|
||||
),
|
||||
],
|
||||
),
|
||||
FilledButton(
|
||||
onPressed: () => Navigator.pop(ctx, true),
|
||||
child: const Text('Remove'),
|
||||
),
|
||||
],
|
||||
),
|
||||
);
|
||||
if (confirm == true) {
|
||||
await repo.removeAccount(a.id);
|
||||
}
|
||||
},
|
||||
);
|
||||
if (confirm == true) {
|
||||
await repo.removeAccount(a.id);
|
||||
}
|
||||
},
|
||||
),
|
||||
],
|
||||
),
|
||||
),
|
||||
],
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter_riverpod/flutter_riverpod.dart';
|
||||
import 'package:intl/intl.dart';
|
||||
|
||||
import '../../core/repositories/sync_log_repository.dart';
|
||||
import '../../di.dart';
|
||||
|
||||
final _timeFmt = DateFormat('MMM d, HH:mm:ss');
|
||||
|
||||
class SyncLogScreen extends ConsumerWidget {
|
||||
const SyncLogScreen({super.key, required this.accountId});
|
||||
|
||||
final String accountId;
|
||||
|
||||
@override
|
||||
Widget build(BuildContext context, WidgetRef ref) {
|
||||
final repo = ref.watch(syncLogRepositoryProvider);
|
||||
return Scaffold(
|
||||
appBar: AppBar(title: const Text('Sync log')),
|
||||
body: StreamBuilder<List<SyncLogEntry>>(
|
||||
stream: repo.observeSyncLogs(accountId),
|
||||
builder: (ctx, snap) {
|
||||
if (!snap.hasData) {
|
||||
return const Center(child: CircularProgressIndicator());
|
||||
}
|
||||
final entries = snap.data!;
|
||||
if (entries.isEmpty) {
|
||||
return const Center(child: Text('No sync entries yet'));
|
||||
}
|
||||
return ListView.builder(
|
||||
itemCount: entries.length,
|
||||
itemBuilder: (ctx, i) {
|
||||
final e = entries[i];
|
||||
final ms = e.duration.inMilliseconds;
|
||||
final durationLabel =
|
||||
ms < 1000 ? '${ms}ms' : '${(ms / 1000).toStringAsFixed(1)}s';
|
||||
return ListTile(
|
||||
leading: Icon(
|
||||
e.isOk ? Icons.check_circle : Icons.error_outline,
|
||||
color:
|
||||
e.isOk ? Colors.green : Theme.of(ctx).colorScheme.error,
|
||||
),
|
||||
title: Text(
|
||||
e.isOk ? 'OK' : 'Error',
|
||||
style: e.isOk
|
||||
? null
|
||||
: TextStyle(color: Theme.of(ctx).colorScheme.error),
|
||||
),
|
||||
subtitle: Column(
|
||||
crossAxisAlignment: CrossAxisAlignment.start,
|
||||
mainAxisSize: MainAxisSize.min,
|
||||
children: [
|
||||
Text(_timeFmt.format(e.startedAt)),
|
||||
Text('took $durationLabel'),
|
||||
if (e.errorMessage != null)
|
||||
Text(
|
||||
e.errorMessage!,
|
||||
style: TextStyle(
|
||||
color: Theme.of(ctx).colorScheme.error,
|
||||
fontSize: 12,
|
||||
),
|
||||
maxLines: 3,
|
||||
overflow: TextOverflow.ellipsis,
|
||||
),
|
||||
],
|
||||
),
|
||||
isThreeLine: e.errorMessage != null,
|
||||
);
|
||||
},
|
||||
);
|
||||
},
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -41,6 +41,7 @@ const _excluded = {
|
||||
'lib/ui/screens/add_account_screen.dart',
|
||||
'lib/ui/screens/compose_screen.dart',
|
||||
'lib/ui/screens/email_detail_screen.dart',
|
||||
'lib/ui/screens/sync_log_screen.dart',
|
||||
};
|
||||
|
||||
void main() {
|
||||
|
||||
@@ -485,7 +485,9 @@ void main() {
|
||||
final r = _makeReposWithFakes();
|
||||
await r.accounts.addAccount(_account, 'pw');
|
||||
r.fakeImap.uidValidityResult = 1000;
|
||||
r.fakeImap.fetchResults = [
|
||||
// Full sync now uses UID SEARCH ALL then UID FETCH.
|
||||
r.fakeImap.searchUids = [10, 20];
|
||||
r.fakeImap.uidFetchResults = [
|
||||
buildEnvelopeMessage(uid: 10, subject: 'First'),
|
||||
buildEnvelopeMessage(uid: 20, subject: 'Second'),
|
||||
];
|
||||
@@ -599,7 +601,9 @@ void main() {
|
||||
receivedAt: DateTime(2024),
|
||||
),
|
||||
);
|
||||
r.fakeImap.fetchResults = [
|
||||
// Full sync (UID validity reset) uses UID SEARCH ALL then UID FETCH.
|
||||
r.fakeImap.searchUids = [1];
|
||||
r.fakeImap.uidFetchResults = [
|
||||
buildEnvelopeMessage(uid: 1, subject: 'Fresh start'),
|
||||
];
|
||||
|
||||
@@ -618,7 +622,9 @@ void main() {
|
||||
test('syncEmails skips messages with no envelope or no uid', () async {
|
||||
final r = _makeReposWithFakes();
|
||||
await r.accounts.addAccount(_account, 'pw');
|
||||
r.fakeImap.fetchResults = [
|
||||
// Full sync uses UID SEARCH ALL then UID FETCH.
|
||||
r.fakeImap.searchUids = [99, 42]; // 99 = buildMessageWithoutEnvelope uid
|
||||
r.fakeImap.uidFetchResults = [
|
||||
buildMessageWithoutEnvelope(), // no envelope → skip
|
||||
buildEnvelopeMessage(uid: 42, subject: 'Valid'),
|
||||
];
|
||||
|
||||
@@ -1,10 +1,20 @@
|
||||
import 'package:enough_mail/enough_mail.dart' as imap;
|
||||
// ignore: implementation_imports
|
||||
import 'package:enough_mail/src/private/util/client_base.dart'
|
||||
show ConnectionInfo;
|
||||
|
||||
/// Configurable fake IMAP client that extends the real ImapClient but
|
||||
/// overrides every network method to return pre-set data.
|
||||
class FakeImapClient extends imap.ImapClient {
|
||||
FakeImapClient() : super();
|
||||
|
||||
/// Override serverInfo with a pre-built instance so tests can access
|
||||
/// capabilities without going through the real (uninitialized) late field.
|
||||
@override
|
||||
final imap.ImapServerInfo serverInfo = imap.ImapServerInfo(
|
||||
const ConnectionInfo('fake.host', 993, isSecure: true),
|
||||
)..capabilities = [const imap.Capability('CONDSTORE')];
|
||||
|
||||
List<imap.MimeMessage> fetchResults = [];
|
||||
List<imap.MimeMessage> uidFetchResults = [];
|
||||
List<imap.Mailbox> listMailboxesResult = [];
|
||||
|
||||
Reference in New Issue
Block a user