mirror of
https://github.com/kuhyx/diet-guard.git
synced 2026-11-10 19:27:14 +01:00
Make the phone publish on every tick, and fail closed on Firebase errors
Some checks failed
Some checks failed
Meals logged on the phone stopped reaching the PC, so the gate kept locking for slots that were already logged and the only way through was to spam "nothing". The PC's fetch was never broken: it was faithfully pulling a Firebase that did not contain the phone's meals. The phone had no working publish backstop on either transport: - `checkAndNotify` gated its sync on `due.isNotEmpty`. Logging every meal promptly means nothing is ever due, so the ~15-min periodic task never published. It was a notification job that happened to sync, not a sync timer. It now syncs on every tick it is asked to, then computes due slots from the merged result, so a meal pulled from another device suppresses a nag instead of triggering one. - `backgroundSyncPush` never called `initSyncDeviceId()`. A fresh isolate has its own static state, so `currentSyncDeviceId` fell back to the compile-time 'phone' constant and published to `devices/phone/` rather than the persisted uuid -- which then makes the next foreground push see `unchanged` and skip writing the uuid directory entirely. Separately, on the "Fetch from sync" path itself: `FirebaseSyncError` and `FirebaseAuthError` are siblings of `GitHubSyncError` under `RemoteSyncError`, not subclasses, so every per-request failure of the *primary* backend escaped `pull_shared_log`'s catch tuple and raised a traceback out of a helper documented to fail closed with a reason string. `ConfigError` needed the same treatment by a different route: it subclasses `Exception` directly, so it is outside `RemoteSyncError` too, and is now translated at `_client_for_run`. The PAT is no longer mandatory either. Firebase is primary, but `_read_token` ran before the client was ever built, so a Firebase-only machine could not sync at all -- the commits that fixed this touched only the Dart side. A stalled sync was invisible: the config gates returned silently and `openFirebase` swallowed every error. `dart:developer`'s `log()` does not reach logcat on a release build (verified on device), so logging alone would not have surfaced it. `SyncHealth` records each tick's outcome and the log screen shows a banner when publishing has actually stopped. Also make the real-Tk probes bypass the window manager. A tiling WM retiles a managed toplevel instead of honouring the requested geometry, so `winfo_height` reported 0/1px and the layout and focus-ring assertions failed on noise rather than on any regression -- 8 failures on the dev machine that only passed under Xvfb. `overrideredirect` makes the measurement deterministic with or without a WM; the probes still detect a genuine overflow at 1366x300. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RT8X32S9yDLW1Xf1PDq8z7
This commit is contained in:
parent
fc9822b516
commit
458f10515b
42
CLAUDE.md
42
CLAUDE.md
@ -54,16 +54,50 @@ log (`_sync_merge.merge_logs`: union by `id`, tombstone wins, legacy
|
||||
regardless of origin, rebuilds the food bank, then pushes this device's own
|
||||
merged log back up.
|
||||
|
||||
**The phone's periodic tick must sync unconditionally.** `checkAndNotify`
|
||||
(`due_slot_check.dart`) once gated its sync on `due.isNotEmpty` — "only pull
|
||||
when a slot looks overdue". That is a trap: logging every meal promptly on the
|
||||
phone means nothing is ever due, so the phone never published, and the PC's
|
||||
gate kept locking for slots that *were* already logged (the only way through
|
||||
being to spam "nothing"). It publishes first, then computes due slots from the
|
||||
merged result, so a meal pulled from another device suppresses a nag instead of
|
||||
triggering one. Do not re-gate the sync on the notification's condition.
|
||||
|
||||
**Anything running in a WorkManager background isolate must call
|
||||
`initSyncDeviceId()` itself.** A fresh isolate has its own static state, so
|
||||
without it `currentSyncDeviceId` silently falls back to the compile-time
|
||||
`'phone'`/`'desktop'` role constant and the tick publishes to
|
||||
`devices/phone/` instead of the persisted uuid. Because `sync_state` holds a
|
||||
single `pushedRev` with no notion of which identity wrote it, the next
|
||||
*foreground* push then sees `unchanged` and skips writing the uuid directory
|
||||
entirely. `background_sync_service.dart` does this alongside its other
|
||||
singleton re-inits; `background_sync_service_test.dart` guards it.
|
||||
|
||||
**Catch `RemoteSyncError`, never `GitHubSyncError`, on any sync path.**
|
||||
`FirebaseSyncError`/`FirebaseAuthError` are *siblings* of `GitHubSyncError`
|
||||
under `RemoteSyncError`, not subclasses. Catching the GitHub type alone lets
|
||||
every per-request failure of the *primary* backend escape — which made
|
||||
`pull_shared_log` (the gate's "Fetch from sync" button, documented to fail
|
||||
closed with a reason string) raise a traceback instead.
|
||||
`ConfigError` is the one to watch: it subclasses `Exception` **directly**, so
|
||||
`RemoteSyncError` does not cover it either. Anything that builds a Firebase
|
||||
client must translate it (`_client_for_run` raises `SyncError`), or it escapes
|
||||
every caller's catch tuple by a different route.
|
||||
|
||||
Re-signing on every merge (not just phone-origin entries) is the
|
||||
non-negotiable step: `_entry_is_valid()` drops any unsigned entry once a
|
||||
machine has the shared HMAC key, and the phone never holds that key, so
|
||||
skipping the re-sign would silently lose every phone-logged meal on the very
|
||||
next read.
|
||||
|
||||
Requires a one-time manual setup `install.sh` does **not** automate: create a
|
||||
fine-grained GitHub PAT scoped to `syncs`'s contents (read/write),
|
||||
then save it to `~/.config/diet_guard/sync_token`, mode 600. Until that file
|
||||
exists, every sync tick is a harmless no-op that logs `sync not configured`.
|
||||
Needs **one** backend configured, not both. Either `~/.config/crdt-sync/`
|
||||
(Firebase, now the primary) or a fine-grained GitHub PAT scoped to `syncs`'s
|
||||
contents (read/write) at `~/.config/diet_guard/sync_token`, mode 600 —
|
||||
a one-time manual setup `install.sh` does **not** automate. A
|
||||
Firebase-configured machine with no PAT syncs against Firebase alone
|
||||
(`_client_for_run`); only when *neither* exists is the tick a harmless no-op
|
||||
that logs `sync not configured`. The PAT used to be required unconditionally,
|
||||
which left a Firebase-only PC unable to sync at all.
|
||||
|
||||
**Both halves of the food bank sync.** The derived bank
|
||||
(`food_bank.json`) is still rebuilt locally from the merged log
|
||||
|
||||
@ -24,12 +24,14 @@ import 'package:diet_guard_app/services/firebase_backend.dart';
|
||||
import 'package:diet_guard_app/services/foodbank_service.dart';
|
||||
import 'package:diet_guard_app/services/github_client_factory.dart';
|
||||
import 'package:diet_guard_app/services/log_storage_service.dart';
|
||||
import 'package:diet_guard_app/services/sync_health.dart';
|
||||
import 'package:diet_guard_app/services/sync_service.dart';
|
||||
import 'package:diet_guard_app/services/sync_settings.dart';
|
||||
import 'package:diet_guard_app/ui/theme.dart';
|
||||
import 'package:diet_guard_app/widgets/autocomplete_suggestion_list.dart';
|
||||
import 'package:diet_guard_app/widgets/macro_input_row.dart';
|
||||
import 'package:diet_guard_app/widgets/slot_selector_row.dart';
|
||||
import 'package:diet_guard_app/widgets/sync_health_banner.dart';
|
||||
import 'package:diet_guard_app/widgets/today_progress_card.dart';
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:http/http.dart' as http;
|
||||
@ -62,6 +64,9 @@ class _LogMealScreenState extends State<LogMealScreen>
|
||||
/// Single-flight guard so a launch sync and a lifecycle sync never overlap.
|
||||
bool _autoSyncing = false;
|
||||
|
||||
/// Latest sync health, driving the "not syncing" banner. Null until read.
|
||||
SyncHealthStatus? _syncHealth;
|
||||
|
||||
@override
|
||||
void initState() {
|
||||
super.initState();
|
||||
@ -80,6 +85,9 @@ class _LogMealScreenState extends State<LogMealScreen>
|
||||
_selectedSlot = slotForLog(DateTime.now());
|
||||
unawaited(_refreshSlots());
|
||||
unawaited(_onDescChanged());
|
||||
// Read health before the first sync finishes, so a device that stalled in
|
||||
// a previous session says so immediately rather than only after a tick.
|
||||
unawaited(_refreshSyncHealth());
|
||||
unawaited(_autoSync());
|
||||
}
|
||||
|
||||
@ -125,18 +133,31 @@ class _LogMealScreenState extends State<LogMealScreen>
|
||||
// gating on it alone silently skips every sync on a device connected
|
||||
// only to Firebase -- and once the mirror is retired that is every
|
||||
// device. Same fix workout_app's push() already carries.
|
||||
if (!settings.isConfigured && await openFirebase() == null) return;
|
||||
if (!settings.isConfigured && await openFirebase() == null) {
|
||||
// Never a silent return: an unconfigured device looked exactly like a
|
||||
// healthy one with nothing to send, which is how meals sat unpublished
|
||||
// for days. Recorded so the banner can say so.
|
||||
log(
|
||||
'diet_guard auto-sync: no backend configured; nothing to push',
|
||||
level: 900,
|
||||
);
|
||||
await SyncHealth.recordUnconfigured();
|
||||
if (mounted) await _refreshSyncHealth();
|
||||
return;
|
||||
}
|
||||
final client = createGitHubClient(
|
||||
settings,
|
||||
httpClient: widget.httpClient,
|
||||
);
|
||||
try {
|
||||
await runSync(await syncBackend(client));
|
||||
await SyncHealth.recordSuccess();
|
||||
} finally {
|
||||
client.close();
|
||||
}
|
||||
if (!mounted) return;
|
||||
await _refreshSlots();
|
||||
await _refreshSyncHealth();
|
||||
} on Object catch (error, stackTrace) {
|
||||
// Best-effort, but never silent: this swallowed the reason a desktop
|
||||
// install could not publish at all, which looked identical to "nothing
|
||||
@ -148,11 +169,20 @@ class _LogMealScreenState extends State<LogMealScreen>
|
||||
error: error,
|
||||
stackTrace: stackTrace,
|
||||
);
|
||||
await SyncHealth.recordFailure();
|
||||
if (mounted) await _refreshSyncHealth();
|
||||
} finally {
|
||||
_autoSyncing = false;
|
||||
}
|
||||
}
|
||||
|
||||
/// Re-reads sync health so the banner reflects the tick that just ran.
|
||||
Future<void> _refreshSyncHealth() async {
|
||||
final status = await SyncHealth.read();
|
||||
if (!mounted) return;
|
||||
setState(() => _syncHealth = status);
|
||||
}
|
||||
|
||||
/// Queues the platform's offline push backstop so a meal logged while
|
||||
/// offline still uploads on reconnect. The in-process [_autoSync] covers
|
||||
/// the online case; this is the backstop (a no-op on web, which has no
|
||||
@ -359,6 +389,7 @@ class _LogMealScreenState extends State<LogMealScreen>
|
||||
child: Column(
|
||||
crossAxisAlignment: CrossAxisAlignment.start,
|
||||
children: [
|
||||
SyncHealthBanner(status: _syncHealth),
|
||||
SlotSelectorRow(
|
||||
now: DateTime.now(),
|
||||
loggedSlots: _loggedSlots,
|
||||
|
||||
@ -15,6 +15,7 @@ import 'package:diet_guard_app/services/app_settings_service.dart';
|
||||
import 'package:diet_guard_app/services/firebase_backend.dart';
|
||||
import 'package:diet_guard_app/services/github_client_factory.dart';
|
||||
import 'package:diet_guard_app/services/github_device_auth.dart';
|
||||
import 'package:diet_guard_app/services/sync_health.dart';
|
||||
import 'package:diet_guard_app/services/sync_service.dart';
|
||||
import 'package:diet_guard_app/services/sync_settings.dart';
|
||||
import 'package:diet_guard_app/ui/theme.dart';
|
||||
@ -211,8 +212,10 @@ class _SettingsScreenState extends State<SettingsScreen> {
|
||||
final client = createGitHubClient(settings, httpClient: widget.httpClient);
|
||||
try {
|
||||
await runSync(await syncBackend(client));
|
||||
await SyncHealth.recordSuccess();
|
||||
_showMessage('Connected and synced.');
|
||||
} on Exception catch (e) {
|
||||
await SyncHealth.recordFailure();
|
||||
_showMessage('Connected, but sync failed: $e');
|
||||
} finally {
|
||||
client.close();
|
||||
@ -251,8 +254,13 @@ class _SettingsScreenState extends State<SettingsScreen> {
|
||||
final client = createGitHubClient(settings, httpClient: widget.httpClient);
|
||||
try {
|
||||
await runSync(await syncBackend(client));
|
||||
// Clears any stored failure: this is the button a user reaches
|
||||
// *because* the banner told them syncing had stopped, so a successful
|
||||
// run here must dismiss the warning it caused.
|
||||
await SyncHealth.recordSuccess();
|
||||
_showMessage('Synced.');
|
||||
} on Exception catch (e) {
|
||||
await SyncHealth.recordFailure();
|
||||
_showMessage('Sync failed: $e');
|
||||
} finally {
|
||||
client.close();
|
||||
|
||||
@ -12,6 +12,8 @@ import 'package:diet_guard_app/services/firebase_backend.dart';
|
||||
import 'package:diet_guard_app/services/foodbank_service.dart';
|
||||
import 'package:diet_guard_app/services/github_client_factory.dart';
|
||||
import 'package:diet_guard_app/services/log_storage_service.dart';
|
||||
import 'package:diet_guard_app/services/sync_device_id.dart';
|
||||
import 'package:diet_guard_app/services/sync_health.dart';
|
||||
import 'package:diet_guard_app/services/sync_service.dart';
|
||||
import 'package:diet_guard_app/services/sync_settings.dart';
|
||||
import 'package:http/http.dart' as http;
|
||||
@ -34,6 +36,15 @@ const String syncPushTaskName = 'diet_guard.sync_push';
|
||||
/// budget, and applies a merge winner via `AppSettingsService.instance`,
|
||||
/// which throws if the singleton was never initialised in this isolate.
|
||||
Future<bool> backgroundSyncPush({http.Client? httpClient}) async {
|
||||
// Must run here too, and before anything stamps an HLC: a fresh isolate has
|
||||
// its own static state, so without this `currentSyncDeviceId` falls back to
|
||||
// the compile-time role constant ('phone') and this tick publishes to
|
||||
// `devices/phone/` instead of the persisted uuid. That splits the device
|
||||
// across two directories, and because `sync_state` holds a single
|
||||
// `pushedRev` with no notion of which identity wrote it, the next
|
||||
// *foreground* push then sees `unchanged` and skips writing the uuid
|
||||
// directory entirely.
|
||||
await initSyncDeviceId();
|
||||
await LogStorageService.init();
|
||||
await FoodBankService.init();
|
||||
await AppSettingsService.init();
|
||||
@ -59,11 +70,13 @@ Future<bool> backgroundSyncPush({http.Client? httpClient}) async {
|
||||
'diet_guard background sync: no backend configured; nothing to push',
|
||||
level: 900,
|
||||
);
|
||||
await SyncHealth.recordUnconfigured();
|
||||
return true; // nothing to push; don't retry
|
||||
}
|
||||
final client = createGitHubClient(settings, httpClient: httpClient);
|
||||
try {
|
||||
await runSync(await syncBackend(client));
|
||||
await SyncHealth.recordSuccess();
|
||||
return true;
|
||||
} on Exception catch (error, stackTrace) {
|
||||
// Never silent: this is the only signal that a background push failed.
|
||||
@ -73,6 +86,7 @@ Future<bool> backgroundSyncPush({http.Client? httpClient}) async {
|
||||
error: error,
|
||||
stackTrace: stackTrace,
|
||||
);
|
||||
await SyncHealth.recordFailure();
|
||||
return false; // offline / transient error -> retry with backoff
|
||||
} finally {
|
||||
client.close();
|
||||
|
||||
@ -18,12 +18,20 @@ import 'package:http/http.dart' as http;
|
||||
/// Reads the local log, computes today's due-but-unlogged slots as of
|
||||
/// [now] (defaults to the real clock), and syncs notifications to match.
|
||||
///
|
||||
/// Pulls from GitHub *only* when a slot looks due, then re-checks -- the
|
||||
/// local log alone cannot see a meal logged on the PC, so without this the
|
||||
/// phone re-fires "you haven't logged your 12:00 meal" every background tick
|
||||
/// until the app is next opened. Deliberately mirrors `_cli_gate._should_lock`
|
||||
/// on the Python side: cheap local check first, network only when it would
|
||||
/// change the answer, then decide on the merged result.
|
||||
/// Syncs on every tick it is asked to ([pullWhenDue], the WorkManager
|
||||
/// caller's default), then computes due slots from the merged result.
|
||||
///
|
||||
/// Deliberately NOT gated on "a slot looks due": for the periodic caller this
|
||||
/// is the phone's only publish path, mirroring the PC's
|
||||
/// `diet-guard-sync.timer`.
|
||||
/// Gating the sync on `due.isNotEmpty` meant that promptly logging every meal
|
||||
/// on the phone left nothing ever due, so the phone never pushed and the PC
|
||||
/// kept locking for slots that were already logged -- the exact failure this
|
||||
/// check exists to prevent. A notification job that happens to sync is not a
|
||||
/// sync timer.
|
||||
///
|
||||
/// Order matters: the sync runs *before* [missingSlots], so a meal pulled from
|
||||
/// another device suppresses a nag rather than triggering one.
|
||||
///
|
||||
/// Set [pullWhenDue] false when the caller has just written to the local log
|
||||
/// itself (the log screen). There the local copy is by definition the freshest
|
||||
@ -41,17 +49,16 @@ Future<void> checkAndNotify({
|
||||
}) async {
|
||||
await LogStorageService.init();
|
||||
final at = now ?? DateTime.now();
|
||||
var due = missingSlots(
|
||||
at,
|
||||
await LogStorageService.instance.loggedSlotsToday(),
|
||||
);
|
||||
if (pullWhenDue && due.isNotEmpty) {
|
||||
if (pullWhenDue) {
|
||||
// Reuses backgroundSyncPush's settings/isConfigured/try-catch guard rather
|
||||
// than reimplementing it: that path is already proven to work from a
|
||||
// WorkManager isolate, including the token read and singleton init.
|
||||
await backgroundSyncPush(httpClient: httpClient);
|
||||
due = missingSlots(at, await LogStorageService.instance.loggedSlotsToday());
|
||||
}
|
||||
final due = missingSlots(
|
||||
at,
|
||||
await LogStorageService.instance.loggedSlotsToday(),
|
||||
);
|
||||
await _reconcileNotifications(due);
|
||||
}
|
||||
|
||||
|
||||
@ -99,11 +99,21 @@ Future<FirebaseRestClient?> openFirebase() async {
|
||||
store: credentialStore(),
|
||||
password: account.password,
|
||||
);
|
||||
} on Object {
|
||||
} on Object catch (error, stackTrace) {
|
||||
// Unreachable keystore or a rejected sign-in must not fail the tick: the
|
||||
// app keeps syncing over GitHub, exactly as before the cutover.
|
||||
// Broader than Exception because an uninitialised platform binding
|
||||
// raises a FlutterError, which is an Error.
|
||||
//
|
||||
// Never silent, for the same reason loadAccount() above says why: a
|
||||
// signed-in-but-failing device looked exactly like an unconfigured one,
|
||||
// which is how the phone went days without publishing a single meal.
|
||||
log(
|
||||
'openFirebase failed; falling back to the GitHub mirror',
|
||||
level: 1000,
|
||||
error: error,
|
||||
stackTrace: stackTrace,
|
||||
);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
164
app/lib/services/sync_health.dart
Normal file
164
app/lib/services/sync_health.dart
Normal file
@ -0,0 +1,164 @@
|
||||
/// Whether this device is actually publishing, and when it last managed to.
|
||||
///
|
||||
/// Exists because the failure it reports was invisible for days: the phone
|
||||
/// held meals nobody else could see, the PC's gate kept locking for slots that
|
||||
/// *were* logged, and every screen looked completely normal. A sync that
|
||||
/// silently stops must not be indistinguishable from a sync with nothing to
|
||||
/// say, so the outcome of every tick is recorded here and surfaced by
|
||||
/// [SyncHealthBanner] on the log screen.
|
||||
///
|
||||
/// This is the *only* signal that reaches the user on a release build. The
|
||||
/// `log()` calls at each failure site go to `dart:developer`, which surfaces
|
||||
/// through the VM service and **not** logcat on a release APK -- verified on
|
||||
/// device: zero Dart-side lines appear under `adb logcat` even from paths that
|
||||
/// definitely ran. So the logging is for a debug session; the banner is what
|
||||
/// makes a stalled sync noticeable in real use. Do not "simplify" this away in
|
||||
/// favour of logging alone.
|
||||
///
|
||||
/// Backed by SharedPreferences rather than a [DocumentStore] because a
|
||||
/// WorkManager background isolate writes it too: prefs need no per-isolate
|
||||
/// init, and a lost health stamp is cosmetic, never user data.
|
||||
library;
|
||||
|
||||
import 'package:shared_preferences/shared_preferences.dart';
|
||||
|
||||
/// How stale the last success may get before the banner appears.
|
||||
///
|
||||
/// Comfortably longer than the ~15 min periodic tick, so one missed or
|
||||
/// battery-deferred tick stays quiet and only a real stall shows up.
|
||||
const Duration kSyncStaleAfter = Duration(hours: 6);
|
||||
|
||||
/// Why the last tick did not publish, or null when it did.
|
||||
enum SyncFailureKind {
|
||||
/// No backend at all: neither a Firebase account nor a GitHub token.
|
||||
unconfigured,
|
||||
|
||||
/// A backend is configured but the attempt threw (offline, auth, remote).
|
||||
failed,
|
||||
}
|
||||
|
||||
/// Records and reports the outcome of sync attempts.
|
||||
abstract final class SyncHealth {
|
||||
static const _kLastSuccess = 'sync.health.lastSuccessMs';
|
||||
static const _kFailureKind = 'sync.health.failureKind';
|
||||
|
||||
/// Marks a tick that actually reached the remote.
|
||||
static Future<void> recordSuccess() => _withPrefs((prefs) async {
|
||||
await prefs.setInt(_kLastSuccess, DateTime.now().millisecondsSinceEpoch);
|
||||
await prefs.remove(_kFailureKind);
|
||||
});
|
||||
|
||||
/// Marks a tick that could not publish because nothing is set up.
|
||||
static Future<void> recordUnconfigured() =>
|
||||
_recordFailure(SyncFailureKind.unconfigured);
|
||||
|
||||
/// Marks a tick that had a backend but failed to publish.
|
||||
static Future<void> recordFailure() =>
|
||||
_recordFailure(SyncFailureKind.failed);
|
||||
|
||||
static Future<void> _recordFailure(SyncFailureKind kind) =>
|
||||
_withPrefs((prefs) => prefs.setString(_kFailureKind, kind.name));
|
||||
|
||||
/// The current health, for the banner.
|
||||
///
|
||||
/// Reports "healthy" when prefs are unreachable: a banner that cannot read
|
||||
/// its own state must not accuse a working device of being stalled.
|
||||
static Future<SyncHealthStatus> read({DateTime? now}) async {
|
||||
final at = now ?? DateTime.now();
|
||||
SharedPreferences prefs;
|
||||
try {
|
||||
prefs = await SharedPreferences.getInstance();
|
||||
} on Object {
|
||||
return SyncHealthStatus(lastSuccess: null, failureKind: null, now: at);
|
||||
}
|
||||
final millis = prefs.getInt(_kLastSuccess);
|
||||
final stored = prefs.getString(_kFailureKind);
|
||||
return SyncHealthStatus(
|
||||
lastSuccess: millis == null
|
||||
? null
|
||||
: DateTime.fromMillisecondsSinceEpoch(millis),
|
||||
failureKind: SyncFailureKind.values
|
||||
.where((value) => value.name == stored)
|
||||
.firstOrNull,
|
||||
now: at,
|
||||
);
|
||||
}
|
||||
|
||||
/// Forgets all recorded health. Test-only.
|
||||
static Future<void> resetForTest() => _withPrefs((prefs) async {
|
||||
await prefs.remove(_kLastSuccess);
|
||||
await prefs.remove(_kFailureKind);
|
||||
});
|
||||
|
||||
/// Runs [action] against prefs, ignoring an unreachable platform channel.
|
||||
///
|
||||
/// Health is diagnostics about syncing, never user data, so it must never
|
||||
/// take down the caller. Under `flutter test` the shared_preferences channel
|
||||
/// is unmocked by default and throws [MissingPluginException] -- the same
|
||||
/// reason [runSync]'s callers treat that as equivalent to being offline.
|
||||
static Future<void> _withPrefs(
|
||||
Future<void> Function(SharedPreferences prefs) action,
|
||||
) async {
|
||||
try {
|
||||
await action(await SharedPreferences.getInstance());
|
||||
} on Object {
|
||||
// Losing a health stamp costs a banner, never a meal.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// A snapshot of whether this device is publishing.
|
||||
class SyncHealthStatus {
|
||||
/// Creates a snapshot evaluated as of [now].
|
||||
const SyncHealthStatus({
|
||||
required this.lastSuccess,
|
||||
required this.failureKind,
|
||||
required this.now,
|
||||
});
|
||||
|
||||
/// When this device last published successfully, or null if it never has.
|
||||
final DateTime? lastSuccess;
|
||||
|
||||
/// Why the most recent attempt did not publish, or null when it did.
|
||||
final SyncFailureKind? failureKind;
|
||||
|
||||
/// The moment this snapshot was evaluated against.
|
||||
final DateTime now;
|
||||
|
||||
/// True when the user should be told that syncing has stopped.
|
||||
///
|
||||
/// A device that has never synced is *not* flagged: a fresh install with no
|
||||
/// account yet is a normal state, and nagging there would train the banner
|
||||
/// to be ignored. Only a device that used to work, or one that explicitly
|
||||
/// failed, is worth interrupting for.
|
||||
bool get isStalled {
|
||||
if (failureKind == SyncFailureKind.failed) return true;
|
||||
final last = lastSuccess;
|
||||
if (last == null) return false;
|
||||
return now.difference(last) > kSyncStaleAfter;
|
||||
}
|
||||
|
||||
/// One line explaining the state, or null when there is nothing to say.
|
||||
String? get message {
|
||||
if (!isStalled) return null;
|
||||
if (failureKind == SyncFailureKind.unconfigured) {
|
||||
return 'Not syncing: no account connected. Meals stay on this device.';
|
||||
}
|
||||
final since = lastSuccess == null ? null : now.difference(lastSuccess!);
|
||||
// An elapsed time is only worth showing once it has actually elapsed.
|
||||
// A sync that failed moments after a success rendered "Not syncing since
|
||||
// 0h ago", which reads like a bug in the banner rather than a report
|
||||
// about the sync.
|
||||
if (since == null || since < const Duration(hours: 1)) {
|
||||
return 'Not syncing. Meals stay on this device.';
|
||||
}
|
||||
return 'Not syncing since ${_ago(since)}. Meals stay on this device.';
|
||||
}
|
||||
|
||||
static String _ago(Duration since) {
|
||||
if (since.inDays >= 1) {
|
||||
return '${since.inDays}d ago';
|
||||
}
|
||||
return '${since.inHours}h ago';
|
||||
}
|
||||
}
|
||||
57
app/lib/widgets/sync_health_banner.dart
Normal file
57
app/lib/widgets/sync_health_banner.dart
Normal file
@ -0,0 +1,57 @@
|
||||
/// The "not syncing" banner for the log screen.
|
||||
library;
|
||||
|
||||
import 'package:diet_guard_app/services/sync_health.dart';
|
||||
import 'package:diet_guard_app/ui/theme.dart';
|
||||
import 'package:flutter/material.dart';
|
||||
|
||||
/// Warns, in place, that meals logged here are not reaching other devices.
|
||||
///
|
||||
/// Renders nothing at all unless [SyncHealthStatus.isStalled], so a healthy
|
||||
/// device never pays for it. This is deliberately on the log screen rather
|
||||
/// than buried in Settings: the whole failure mode being guarded against is
|
||||
/// one that stayed invisible while the user kept logging meals that went
|
||||
/// nowhere.
|
||||
class SyncHealthBanner extends StatelessWidget {
|
||||
/// Creates a [SyncHealthBanner] for [status].
|
||||
const SyncHealthBanner({required this.status, super.key});
|
||||
|
||||
/// The health to render, or null before the first read.
|
||||
final SyncHealthStatus? status;
|
||||
|
||||
@override
|
||||
Widget build(BuildContext context) {
|
||||
final message = status?.message;
|
||||
if (message == null) return const SizedBox.shrink();
|
||||
final theme = Theme.of(context);
|
||||
// Null-safe lookup, matching slot_selector_row/day_status_calendar: a
|
||||
// widget test that pumps a bare MaterialApp has no theme extension, and
|
||||
// `!` there is a crash rather than a missing colour.
|
||||
final warning =
|
||||
(theme.extension<AppStatusColors>() ?? AppStatusColors.dark).warning;
|
||||
return Container(
|
||||
margin: const EdgeInsets.only(bottom: 12),
|
||||
padding: const EdgeInsets.symmetric(horizontal: 12, vertical: 8),
|
||||
decoration: BoxDecoration(
|
||||
// A tinted fill plus a border, no shadow: the unified design system
|
||||
// allows only one depth technique per surface, and shadows are out
|
||||
// entirely in dark UI.
|
||||
color: warning.withValues(alpha: 0.12),
|
||||
border: Border.all(color: warning.withValues(alpha: 0.5)),
|
||||
borderRadius: BorderRadius.circular(8),
|
||||
),
|
||||
child: Row(
|
||||
children: [
|
||||
Icon(Icons.cloud_off, size: 18, color: warning),
|
||||
const SizedBox(width: 8),
|
||||
Expanded(
|
||||
child: Text(
|
||||
message,
|
||||
style: theme.textTheme.bodySmall?.copyWith(color: warning),
|
||||
),
|
||||
),
|
||||
],
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
@ -2,7 +2,7 @@ name: diet_guard_app
|
||||
description: "Companion phone app for diet_guard: log meals on the go."
|
||||
publish_to: 'none'
|
||||
|
||||
version: 1.0.3+1
|
||||
version: 1.0.4+1
|
||||
|
||||
environment:
|
||||
sdk: ^3.12.2
|
||||
|
||||
@ -9,6 +9,7 @@ import 'package:diet_guard_app/services/budget_history_service.dart';
|
||||
import 'package:diet_guard_app/services/firebase_backend.dart';
|
||||
import 'package:diet_guard_app/services/foodbank_service.dart';
|
||||
import 'package:diet_guard_app/services/log_storage_service.dart';
|
||||
import 'package:diet_guard_app/services/sync_health.dart';
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter/services.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
@ -199,6 +200,40 @@ void main() {
|
||||
});
|
||||
});
|
||||
|
||||
testWidgets('Sync now clears a stored failure so the banner lifts', (
|
||||
tester,
|
||||
) async {
|
||||
// "Sync now" is the button a user reaches *because* the log screen's
|
||||
// banner told them syncing had stopped. If a successful run here left
|
||||
// the recorded failure in place, the recovery action would not dismiss
|
||||
// the warning it caused, and the banner would keep accusing a device
|
||||
// that is now publishing fine.
|
||||
await SyncHealth.recordFailure();
|
||||
expect((await SyncHealth.read()).failureKind, SyncFailureKind.failed);
|
||||
|
||||
final mock = MockClient((req) async {
|
||||
if (req.method == 'PUT') return http.Response('{}', 200);
|
||||
if (req.method == 'GET' && req.url.pathSegments.length == 3) {
|
||||
return http.Response('{}', 200);
|
||||
}
|
||||
return http.Response('', 404);
|
||||
});
|
||||
await tester.runAsync(() async {
|
||||
await tester.pumpWidget(
|
||||
MaterialApp(home: SettingsScreen(httpClient: mock)),
|
||||
);
|
||||
await settle(tester);
|
||||
|
||||
await tester.tap(find.widgetWithText(ElevatedButton, 'Sync now'));
|
||||
await settle(tester);
|
||||
|
||||
expect(find.text('Synced.'), findsOneWidget);
|
||||
final status = await SyncHealth.read();
|
||||
expect(status.failureKind, isNull);
|
||||
expect(status.isStalled, isFalse);
|
||||
});
|
||||
});
|
||||
|
||||
testWidgets('Test connection reports a network exception', (tester) async {
|
||||
final mock = MockClient((_) async => throw const FormatException('no net'));
|
||||
await tester.runAsync(() async {
|
||||
|
||||
@ -11,6 +11,7 @@ import 'package:diet_guard_app/services/budget_history_service.dart';
|
||||
import 'package:diet_guard_app/services/background_sync_service.dart';
|
||||
import 'package:diet_guard_app/services/foodbank_service.dart';
|
||||
import 'package:diet_guard_app/services/log_storage_service.dart';
|
||||
import 'package:diet_guard_app/services/sync_device_id.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:http/http.dart' as http;
|
||||
import 'package:http/testing.dart';
|
||||
@ -87,6 +88,41 @@ void main() {
|
||||
expect(puts, 5);
|
||||
});
|
||||
|
||||
test('pushes under the persisted uuid, not the legacy role id', () async {
|
||||
// Regression guard. A WorkManager background isolate has its own static state,
|
||||
// so without initSyncDeviceId() here `currentSyncDeviceId` falls back to
|
||||
// the compile-time 'phone' constant and this tick writes to
|
||||
// `devices/phone/`. That splits the device across two directories, and
|
||||
// since sync_state holds a single `pushedRev` with no notion of which
|
||||
// identity wrote it, the next *foreground* push then sees `unchanged`
|
||||
// and skips writing the uuid directory at all.
|
||||
const uuid = '77e39198-ff83-479b-a905-b0cd68b66094';
|
||||
SharedPreferences.setMockInitialValues({
|
||||
'sync.owner': 'o',
|
||||
'sync.repo': 'r',
|
||||
'crdt.nodeId': uuid,
|
||||
});
|
||||
resetSyncDeviceIdForTest();
|
||||
installFakeSecureStorage(initial: {'sync.token': 't'});
|
||||
final pushedPaths = <String>[];
|
||||
final mock = MockClient((req) async {
|
||||
if (req.method == 'PUT') {
|
||||
pushedPaths.add(req.url.path);
|
||||
return http.Response('{}', 200);
|
||||
}
|
||||
if (req.method == 'GET' && req.url.pathSegments.length == 3) {
|
||||
return http.Response('{}', 200);
|
||||
}
|
||||
return http.Response('', 404);
|
||||
});
|
||||
|
||||
await backgroundSyncPush(httpClient: mock);
|
||||
|
||||
expect(pushedPaths, isNotEmpty);
|
||||
expect(pushedPaths.every((path) => path.contains(uuid)), isTrue);
|
||||
expect(pushedPaths.any((path) => path.contains('/devices/phone/')), isFalse);
|
||||
});
|
||||
|
||||
test('reports failure (retry) when the push errors', () async {
|
||||
SharedPreferences.setMockInitialValues({
|
||||
'sync.owner': 'o',
|
||||
|
||||
@ -108,7 +108,7 @@ void main() {
|
||||
);
|
||||
});
|
||||
|
||||
test('does not pull when nothing is due', () async {
|
||||
test('still syncs when nothing is due', () async {
|
||||
await LogStorageService.instance.logMeal('breakfast', _manual, slot: 8);
|
||||
SharedPreferences.setMockInitialValues({
|
||||
'sync.owner': 'o',
|
||||
@ -123,9 +123,12 @@ void main() {
|
||||
|
||||
await checkAndNotify(now: DateTime(2026, 1, 1, 8), httpClient: mock);
|
||||
|
||||
// Every due slot was already satisfied locally, so the network is never
|
||||
// touched -- the check stays cheap on the overwhelmingly common tick.
|
||||
expect(requests, 0);
|
||||
// Regression guard. This asserted `requests == 0` -- "nothing due, so
|
||||
// never touch the network" -- which is precisely the bug: logging every
|
||||
// meal promptly on the phone left nothing ever due, so this periodic task
|
||||
// never published and the PC kept locking for slots that WERE logged.
|
||||
// This is the phone's only unconditional publish path; it must always run.
|
||||
expect(requests, greaterThan(0));
|
||||
});
|
||||
|
||||
test('pullWhenDue false skips the network even with a slot due', () async {
|
||||
|
||||
134
app/test/services/sync_health_test.dart
Normal file
134
app/test/services/sync_health_test.dart
Normal file
@ -0,0 +1,134 @@
|
||||
import 'package:diet_guard_app/services/sync_health.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:shared_preferences/shared_preferences.dart';
|
||||
|
||||
void main() {
|
||||
setUp(() => SharedPreferences.setMockInitialValues({}));
|
||||
|
||||
group('SyncHealthStatus.isStalled', () {
|
||||
test('a device that has never synced is not flagged', () {
|
||||
// A fresh install with no account yet is normal, not a fault; nagging
|
||||
// there would train the banner to be ignored.
|
||||
final fresh = SyncHealthStatus(
|
||||
lastSuccess: null,
|
||||
failureKind: null,
|
||||
now: DateTime(2026, 8, 10),
|
||||
);
|
||||
expect(fresh.isStalled, isFalse);
|
||||
expect(fresh.message, isNull);
|
||||
});
|
||||
|
||||
test('a recent success is healthy', () {
|
||||
final now = DateTime(2026, 8, 10, 12);
|
||||
final status = SyncHealthStatus(
|
||||
lastSuccess: now.subtract(const Duration(minutes: 30)),
|
||||
failureKind: null,
|
||||
now: now,
|
||||
);
|
||||
expect(status.isStalled, isFalse);
|
||||
});
|
||||
|
||||
test('a success older than the staleness window is flagged', () {
|
||||
final now = DateTime(2026, 8, 10, 12);
|
||||
final status = SyncHealthStatus(
|
||||
lastSuccess: now.subtract(kSyncStaleAfter + const Duration(hours: 1)),
|
||||
failureKind: null,
|
||||
now: now,
|
||||
);
|
||||
expect(status.isStalled, isTrue);
|
||||
expect(status.message, contains('Not syncing since'));
|
||||
});
|
||||
|
||||
test('an outright failure is flagged even when recent', () {
|
||||
final now = DateTime(2026, 8, 10, 12);
|
||||
final status = SyncHealthStatus(
|
||||
lastSuccess: now.subtract(const Duration(minutes: 1)),
|
||||
failureKind: SyncFailureKind.failed,
|
||||
now: now,
|
||||
);
|
||||
expect(status.isStalled, isTrue);
|
||||
});
|
||||
|
||||
test('an unconfigured device names the missing account', () {
|
||||
final now = DateTime(2026, 8, 10, 12);
|
||||
final status = SyncHealthStatus(
|
||||
lastSuccess: now.subtract(const Duration(days: 2)),
|
||||
failureKind: SyncFailureKind.unconfigured,
|
||||
now: now,
|
||||
);
|
||||
expect(status.message, contains('no account connected'));
|
||||
});
|
||||
|
||||
test('reports days for a long stall and hours for a short one', () {
|
||||
final now = DateTime(2026, 8, 10, 12);
|
||||
String? messageAfter(Duration since) => SyncHealthStatus(
|
||||
lastSuccess: now.subtract(since),
|
||||
failureKind: null,
|
||||
now: now,
|
||||
).message;
|
||||
|
||||
expect(messageAfter(const Duration(days: 3)), contains('3d ago'));
|
||||
expect(messageAfter(const Duration(hours: 9)), contains('9h ago'));
|
||||
});
|
||||
|
||||
test('a failure just after a success does not say "0h ago"', () {
|
||||
// Regression guard: reporting elapsed time before any has elapsed
|
||||
// rendered "Not syncing since 0h ago", which reads as a bug in the
|
||||
// banner rather than a report about the sync.
|
||||
final now = DateTime(2026, 8, 10, 12);
|
||||
final status = SyncHealthStatus(
|
||||
lastSuccess: now.subtract(const Duration(minutes: 5)),
|
||||
failureKind: SyncFailureKind.failed,
|
||||
now: now,
|
||||
);
|
||||
expect(status.isStalled, isTrue);
|
||||
expect(status.message, 'Not syncing. Meals stay on this device.');
|
||||
expect(status.message, isNot(contains('0h')));
|
||||
});
|
||||
|
||||
test('a failure with no prior success still reads sensibly', () {
|
||||
final status = SyncHealthStatus(
|
||||
lastSuccess: null,
|
||||
failureKind: SyncFailureKind.failed,
|
||||
now: DateTime(2026, 8, 10),
|
||||
);
|
||||
expect(status.message, 'Not syncing. Meals stay on this device.');
|
||||
});
|
||||
});
|
||||
|
||||
group('SyncHealth persistence', () {
|
||||
test('recordSuccess clears a previous failure', () async {
|
||||
await SyncHealth.recordFailure();
|
||||
expect((await SyncHealth.read()).failureKind, SyncFailureKind.failed);
|
||||
|
||||
await SyncHealth.recordSuccess();
|
||||
final status = await SyncHealth.read();
|
||||
expect(status.failureKind, isNull);
|
||||
expect(status.lastSuccess, isNotNull);
|
||||
expect(status.isStalled, isFalse);
|
||||
});
|
||||
|
||||
test('recordUnconfigured round-trips', () async {
|
||||
await SyncHealth.recordUnconfigured();
|
||||
expect(
|
||||
(await SyncHealth.read()).failureKind,
|
||||
SyncFailureKind.unconfigured,
|
||||
);
|
||||
});
|
||||
|
||||
test('an unrecognised stored kind reads as no failure', () async {
|
||||
SharedPreferences.setMockInitialValues({
|
||||
'sync.health.failureKind': 'not-a-kind',
|
||||
});
|
||||
expect((await SyncHealth.read()).failureKind, isNull);
|
||||
});
|
||||
|
||||
test('resetForTest forgets everything', () async {
|
||||
await SyncHealth.recordSuccess();
|
||||
await SyncHealth.resetForTest();
|
||||
final status = await SyncHealth.read();
|
||||
expect(status.lastSuccess, isNull);
|
||||
expect(status.failureKind, isNull);
|
||||
});
|
||||
});
|
||||
}
|
||||
71
app/test/widgets/sync_health_banner_test.dart
Normal file
71
app/test/widgets/sync_health_banner_test.dart
Normal file
@ -0,0 +1,71 @@
|
||||
import 'package:diet_guard_app/services/sync_health.dart';
|
||||
import 'package:diet_guard_app/widgets/sync_health_banner.dart';
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
|
||||
Future<void> pump(WidgetTester tester, SyncHealthStatus? status) =>
|
||||
tester.pumpWidget(
|
||||
MaterialApp(home: Scaffold(body: SyncHealthBanner(status: status))),
|
||||
);
|
||||
|
||||
void main() {
|
||||
testWidgets('renders nothing before health has been read', (tester) async {
|
||||
await pump(tester, null);
|
||||
expect(find.byIcon(Icons.cloud_off), findsNothing);
|
||||
});
|
||||
|
||||
testWidgets('renders nothing for a healthy device', (tester) async {
|
||||
final now = DateTime(2026, 8, 10, 12);
|
||||
await pump(
|
||||
tester,
|
||||
SyncHealthStatus(
|
||||
lastSuccess: now.subtract(const Duration(minutes: 5)),
|
||||
failureKind: null,
|
||||
now: now,
|
||||
),
|
||||
);
|
||||
expect(find.byIcon(Icons.cloud_off), findsNothing);
|
||||
});
|
||||
|
||||
testWidgets('warns when the device has stopped publishing', (tester) async {
|
||||
final now = DateTime(2026, 8, 10, 12);
|
||||
await pump(
|
||||
tester,
|
||||
SyncHealthStatus(
|
||||
lastSuccess: now.subtract(const Duration(days: 2)),
|
||||
failureKind: SyncFailureKind.failed,
|
||||
now: now,
|
||||
),
|
||||
);
|
||||
|
||||
expect(find.byIcon(Icons.cloud_off), findsOneWidget);
|
||||
expect(
|
||||
find.textContaining('Meals stay on this device'),
|
||||
findsOneWidget,
|
||||
);
|
||||
});
|
||||
|
||||
testWidgets('survives a theme with no AppStatusColors extension', (
|
||||
tester,
|
||||
) async {
|
||||
// Regression guard: a `!` on the extension lookup crashed every widget
|
||||
// test that pumped a bare MaterialApp.
|
||||
final now = DateTime(2026, 8, 10, 12);
|
||||
await tester.pumpWidget(
|
||||
MaterialApp(
|
||||
theme: ThemeData(extensions: const []),
|
||||
home: Scaffold(
|
||||
body: SyncHealthBanner(
|
||||
status: SyncHealthStatus(
|
||||
lastSuccess: now.subtract(const Duration(days: 2)),
|
||||
failureKind: SyncFailureKind.unconfigured,
|
||||
now: now,
|
||||
),
|
||||
),
|
||||
),
|
||||
),
|
||||
);
|
||||
expect(tester.takeException(), isNull);
|
||||
expect(find.byIcon(Icons.cloud_off), findsOneWidget);
|
||||
});
|
||||
}
|
||||
@ -10,7 +10,7 @@ from __future__ import annotations
|
||||
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from crdt_sync import GitHubSyncError
|
||||
from crdt_sync import RemoteSyncError
|
||||
|
||||
from diet_guard._sync import SyncError, run_sync
|
||||
|
||||
@ -48,7 +48,10 @@ def cmd_sync(emit: Callable[[str], None]) -> int:
|
||||
except SyncError as exc:
|
||||
emit(f"sync not configured: {exc}")
|
||||
return 1
|
||||
except GitHubSyncError as exc:
|
||||
except RemoteSyncError as exc:
|
||||
# The shared base, not GitHubSyncError: Firebase's errors are siblings
|
||||
# of GitHub's, so catching only the latter let the primary backend's
|
||||
# failures crash this timer-driven command with a traceback.
|
||||
emit(f"sync failed: {exc}")
|
||||
return 1
|
||||
total_entries = sum(len(entries) for entries in merged.values())
|
||||
|
||||
@ -31,6 +31,7 @@ from crdt_sync import (
|
||||
RemoteStore,
|
||||
RemoteSyncError,
|
||||
SyncState,
|
||||
firebase_client_for,
|
||||
merge_logs,
|
||||
mirror_client_for,
|
||||
revision_of,
|
||||
@ -109,6 +110,47 @@ def _remote_client(github: GitHubSyncClient) -> RemoteStore:
|
||||
return github
|
||||
|
||||
|
||||
def _client_for_run() -> RemoteStore:
|
||||
"""Return the backend for one sync run, requiring only *one* to be set up.
|
||||
|
||||
The PAT is no longer mandatory. Firebase is the primary backend now, and
|
||||
requiring a GitHub token before ever constructing it meant a
|
||||
Firebase-configured machine with no PAT could not sync at all -- the
|
||||
commits that fixed exactly this ("Auto-sync on a Firebase-only device")
|
||||
changed only the Dart side, leaving the Python half behind.
|
||||
|
||||
A missing PAT is therefore fatal only when Firebase is *also* unconfigured,
|
||||
which is the genuine "nothing is set up" case the caller must report.
|
||||
"""
|
||||
try:
|
||||
token = _read_token()
|
||||
except SyncError:
|
||||
if not CONFIG_FILE.is_file():
|
||||
raise
|
||||
# Firebase-only: there is no GitHub client to mirror to, so use the
|
||||
# primary directly rather than failing the whole run.
|
||||
#
|
||||
# ConfigError is translated rather than propagated: it is *not* a
|
||||
# RemoteSyncError (it subclasses Exception directly), so letting it out
|
||||
# would escape every caller's catch tuple and raise a traceback out of
|
||||
# the gate's fail-closed "Fetch from sync" button -- the exact failure
|
||||
# the RemoteSyncError swap was made to stop. A config file that exists
|
||||
# but is unusable is reached by precisely this branch.
|
||||
try:
|
||||
return firebase_client_for("diet_guard")
|
||||
except ConfigError as exc:
|
||||
message = f"Firebase config at {CONFIG_FILE} is unusable: {exc}"
|
||||
raise SyncError(message) from exc
|
||||
return _remote_client(
|
||||
GitHubSyncClient(
|
||||
SYNC_REPO_OWNER,
|
||||
SYNC_REPO_NAME,
|
||||
token,
|
||||
timeout_seconds=SYNC_TIMEOUT_SECONDS,
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
def _device_log_path(device_id: str) -> str:
|
||||
"""Return the repo-relative path a device's full log is pushed to."""
|
||||
return f"{_DEVICES_DIR}/{device_id}/food_log.json"
|
||||
@ -372,20 +414,13 @@ def run_sync() -> DayLog:
|
||||
The merged log as it now sits on disk locally, post re-sign.
|
||||
|
||||
Raises:
|
||||
SyncError: If the local PAT is missing or empty.
|
||||
crdt_sync.GitHubSyncError: Propagated from the GitHub client for any
|
||||
transport failure -- the caller (CLI/timer) decides how to
|
||||
SyncError: If *neither* backend is configured -- no Firebase config and
|
||||
no usable PAT.
|
||||
crdt_sync.RemoteSyncError: Propagated from whichever backend failed for
|
||||
any transport failure -- the caller (CLI/timer) decides how to
|
||||
report it.
|
||||
"""
|
||||
token = _read_token()
|
||||
client = _remote_client(
|
||||
GitHubSyncClient(
|
||||
SYNC_REPO_OWNER,
|
||||
SYNC_REPO_NAME,
|
||||
token,
|
||||
timeout_seconds=SYNC_TIMEOUT_SECONDS,
|
||||
)
|
||||
)
|
||||
client = _client_for_run()
|
||||
|
||||
identity = device_identity()
|
||||
state_store = FileSyncStateStore(SYNC_STATE_FILE)
|
||||
@ -443,14 +478,21 @@ def pull_shared_log() -> str | None:
|
||||
its own lock decision rather than failing open.
|
||||
|
||||
The three caught types are the whole realistic failure surface of a run:
|
||||
:class:`SyncError` (no/empty token), :class:`~crdt_sync.GitHubSyncError`
|
||||
(the client wraps every ``requests`` transport error in this), and
|
||||
:class:`SyncError` (no/empty token), :class:`~crdt_sync.RemoteSyncError`
|
||||
(the shared base of every backend's transport/auth failure), and
|
||||
:class:`OSError` (reading the token or writing the merged log back). A bug
|
||||
outside these is deliberately *not* swallowed -- it should surface, not be
|
||||
silently reported as a sync outage.
|
||||
|
||||
``RemoteSyncError``, **not** ``GitHubSyncError``: since Firebase became the
|
||||
primary backend, ``FirebaseSyncError``/``FirebaseAuthError`` are *siblings*
|
||||
of ``GitHubSyncError`` under ``RemoteSyncError``, not subclasses. Catching
|
||||
the GitHub type alone let every per-request Firebase failure escape, so
|
||||
this fail-closed helper raised a traceback out of the gate's "Fetch from
|
||||
sync" button instead of returning a reason and leaving the lock up.
|
||||
"""
|
||||
try:
|
||||
run_sync()
|
||||
except (SyncError, GitHubSyncError, OSError) as exc:
|
||||
except (SyncError, RemoteSyncError, OSError) as exc:
|
||||
return f"sync unavailable ({exc})"
|
||||
return None
|
||||
|
||||
@ -76,6 +76,13 @@ def probe(screen_w: int = 1366, screen_h: int = 768) -> RingReport:
|
||||
"""Build the gate and probe its keyboard behaviour."""
|
||||
root = tk.Tk()
|
||||
try:
|
||||
# Bypass the window manager -- see the same call in
|
||||
# `_gate_layout_probe.measure`. A tiling WM retiles this toplevel
|
||||
# instead of honouring the geometry, the widgets never realize at a
|
||||
# usable size, and the focus ring then walks a single stop, failing
|
||||
# every traversal assertion for reasons that have nothing to do with
|
||||
# the focus order under test.
|
||||
root.overrideredirect(boolean=True)
|
||||
root.geometry(f"{screen_w}x{screen_h}+0+0")
|
||||
notebook = ttk.Notebook(root)
|
||||
notebook.place(relx=0, rely=0, relwidth=1, relheight=1)
|
||||
|
||||
@ -281,6 +281,16 @@ def measure(
|
||||
"""
|
||||
root = tk.Tk()
|
||||
try:
|
||||
# Bypass the window manager. This measures whether the layout fits a
|
||||
# given panel, so the window must actually BE that size -- but a
|
||||
# tiling WM (i3 on the dev machine) retiles a managed toplevel to the
|
||||
# workspace instead, and `winfo_height()` then reports 0/1px. Every
|
||||
# measurement taken from that is noise, and the assertions fail on it
|
||||
# rather than on any real layout regression. `overrideredirect` takes
|
||||
# the window out of the WM's control, so the requested geometry is
|
||||
# honoured exactly -- verified 1366x768 under i3, and unchanged under
|
||||
# a bare Xvfb, which has no WM to bypass in the first place.
|
||||
root.overrideredirect(boolean=True)
|
||||
root.geometry(f"{screen_w}x{screen_h}+0+0")
|
||||
_notebook_overhead(root)
|
||||
notebook = ttk.Notebook(root)
|
||||
|
||||
@ -8,10 +8,14 @@ The GitHub layer is mocked (no network access); conftest.py's
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from crdt_sync import ConfigError, GitHubSyncError, SyncState
|
||||
from crdt_sync import (
|
||||
FirebaseAuthError,
|
||||
FirebaseSyncError,
|
||||
GitHubSyncError,
|
||||
SyncState,
|
||||
)
|
||||
import pytest
|
||||
|
||||
from diet_guard import _sync
|
||||
@ -364,6 +368,30 @@ class TestPullSharedLog:
|
||||
assert reason is not None
|
||||
assert "boom" in reason
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"error",
|
||||
[
|
||||
FirebaseAuthError("token refresh rejected"),
|
||||
FirebaseSyncError("RTDB unavailable"),
|
||||
],
|
||||
)
|
||||
def test_firebase_failure_fails_closed(self, error: Exception) -> None:
|
||||
"""A Firebase failure must return a reason, never raise.
|
||||
|
||||
Regression guard: ``FirebaseAuthError``/``FirebaseSyncError`` are
|
||||
*siblings* of ``GitHubSyncError`` under ``RemoteSyncError``, not
|
||||
subclasses. Catching only the GitHub type let every per-request
|
||||
failure of the *primary* backend escape, so the gate's "Fetch from
|
||||
sync" button raised a traceback instead of reporting a reason and
|
||||
leaving the lock up -- on the exact path this helper exists to
|
||||
fail closed.
|
||||
"""
|
||||
with patch.object(_sync, "run_sync", side_effect=error):
|
||||
reason = _sync.pull_shared_log()
|
||||
|
||||
assert reason is not None
|
||||
assert str(error) in reason
|
||||
|
||||
def test_unexpected_error_is_not_swallowed(self) -> None:
|
||||
"""A bug outside the known failure surface surfaces, not hidden."""
|
||||
with (
|
||||
@ -373,50 +401,6 @@ class TestPullSharedLog:
|
||||
_sync.pull_shared_log()
|
||||
|
||||
|
||||
class TestRemoteClient:
|
||||
"""Which backend a sync tick runs against during the cutover."""
|
||||
|
||||
def test_stays_on_github_without_firebase_config(
|
||||
self, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""An unconfigured machine must not reach the network at all."""
|
||||
monkeypatch.setattr(_sync, "CONFIG_FILE", Path("/nonexistent/firebase.json"))
|
||||
github = object()
|
||||
|
||||
assert _sync._remote_client(github) is github
|
||||
|
||||
def test_mirrors_to_github_when_configured(
|
||||
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""Configured: Firebase is primary, GitHub keeps receiving writes."""
|
||||
config = tmp_path / "firebase.json"
|
||||
config.write_text("{}", encoding="utf-8")
|
||||
monkeypatch.setattr(_sync, "CONFIG_FILE", config)
|
||||
monkeypatch.setattr(
|
||||
_sync, "mirror_client_for", lambda _app, client: ("mirror", client)
|
||||
)
|
||||
github = object()
|
||||
|
||||
assert _sync._remote_client(github) == ("mirror", github)
|
||||
|
||||
def test_falls_back_when_firebase_is_unusable(
|
||||
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""A broken Firebase must degrade to GitHub, never fail the tick."""
|
||||
config = tmp_path / "firebase.json"
|
||||
config.write_text("{}", encoding="utf-8")
|
||||
monkeypatch.setattr(_sync, "CONFIG_FILE", config)
|
||||
|
||||
def _boom(*_args: object, **_kwargs: object) -> None:
|
||||
message = "no password"
|
||||
raise ConfigError(message)
|
||||
|
||||
monkeypatch.setattr(_sync, "mirror_client_for", _boom)
|
||||
github = object()
|
||||
|
||||
assert _sync._remote_client(github) is github
|
||||
|
||||
|
||||
class TestRemoteRevisions:
|
||||
"""Reading the peers' revision map, which gates the big downloads."""
|
||||
|
||||
|
||||
117
diet_guard/tests/test_sync_backend.py
Normal file
117
diet_guard/tests/test_sync_backend.py
Normal file
@ -0,0 +1,117 @@
|
||||
"""Tests for which backend a sync tick runs against.
|
||||
|
||||
Split out of ``test_sync.py`` to keep both files under the repo's 500-line
|
||||
cap. Covers the Firebase/GitHub cutover seam (``_remote_client``) and which
|
||||
credentials a run actually requires (``_client_for_run``).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from crdt_sync import ConfigError
|
||||
import pytest
|
||||
|
||||
from diet_guard import _sync
|
||||
|
||||
|
||||
class TestRemoteClient:
|
||||
"""Which backend a sync tick runs against during the cutover."""
|
||||
|
||||
def test_stays_on_github_without_firebase_config(
|
||||
self, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""An unconfigured machine must not reach the network at all."""
|
||||
monkeypatch.setattr(_sync, "CONFIG_FILE", Path("/nonexistent/firebase.json"))
|
||||
github = object()
|
||||
|
||||
assert _sync._remote_client(github) is github
|
||||
|
||||
def test_mirrors_to_github_when_configured(
|
||||
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""Configured: Firebase is primary, GitHub keeps receiving writes."""
|
||||
config = tmp_path / "firebase.json"
|
||||
config.write_text("{}", encoding="utf-8")
|
||||
monkeypatch.setattr(_sync, "CONFIG_FILE", config)
|
||||
monkeypatch.setattr(
|
||||
_sync, "mirror_client_for", lambda _app, client: ("mirror", client)
|
||||
)
|
||||
github = object()
|
||||
|
||||
assert _sync._remote_client(github) == ("mirror", github)
|
||||
|
||||
def test_falls_back_when_firebase_is_unusable(
|
||||
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""A broken Firebase must degrade to GitHub, never fail the tick."""
|
||||
config = tmp_path / "firebase.json"
|
||||
config.write_text("{}", encoding="utf-8")
|
||||
monkeypatch.setattr(_sync, "CONFIG_FILE", config)
|
||||
|
||||
def _boom(*_args: object, **_kwargs: object) -> None:
|
||||
message = "no password"
|
||||
raise ConfigError(message)
|
||||
|
||||
monkeypatch.setattr(_sync, "mirror_client_for", _boom)
|
||||
github = object()
|
||||
|
||||
assert _sync._remote_client(github) is github
|
||||
|
||||
|
||||
class TestClientForRun:
|
||||
"""Which credentials a run actually requires."""
|
||||
|
||||
def test_syncs_without_a_pat_when_firebase_is_configured(
|
||||
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""A Firebase-only machine must sync, not fail for a missing PAT.
|
||||
|
||||
Requiring the token before ever constructing Firebase left the Python
|
||||
half unable to sync at all on a device that had moved to Firebase --
|
||||
the "Firebase-only device" fix had only ever touched the Dart side.
|
||||
"""
|
||||
config = tmp_path / "firebase.json"
|
||||
config.write_text("{}", encoding="utf-8")
|
||||
monkeypatch.setattr(_sync, "CONFIG_FILE", config)
|
||||
monkeypatch.setattr(_sync, "SYNC_TOKEN_FILE", tmp_path / "absent_sync_token")
|
||||
monkeypatch.setattr(_sync, "firebase_client_for", lambda app: ("firebase", app))
|
||||
|
||||
assert _sync._client_for_run() == ("firebase", "diet_guard")
|
||||
|
||||
def test_unusable_firebase_config_becomes_a_sync_error(
|
||||
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""An unusable Firebase config must fail *closed*, not raise.
|
||||
|
||||
``ConfigError`` subclasses ``Exception`` directly, not
|
||||
``RemoteSyncError``, so letting it out of here escapes every caller's
|
||||
catch tuple and raises a traceback out of the gate's fail-closed
|
||||
"Fetch from sync" button -- reintroducing, by a different exception,
|
||||
exactly the failure the ``RemoteSyncError`` swap was made to stop.
|
||||
This branch is reached precisely when the config file exists but is
|
||||
unusable and no PAT is present.
|
||||
"""
|
||||
config = tmp_path / "firebase.json"
|
||||
config.write_text("{}", encoding="utf-8")
|
||||
monkeypatch.setattr(_sync, "CONFIG_FILE", config)
|
||||
monkeypatch.setattr(_sync, "SYNC_TOKEN_FILE", tmp_path / "absent_token")
|
||||
|
||||
def _boom(*_args: object, **_kwargs: object) -> None:
|
||||
message = "no password"
|
||||
raise ConfigError(message)
|
||||
|
||||
monkeypatch.setattr(_sync, "firebase_client_for", _boom)
|
||||
|
||||
with pytest.raises(_sync.SyncError):
|
||||
_sync._client_for_run()
|
||||
|
||||
def test_still_fails_when_neither_backend_is_configured(
|
||||
self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""No PAT *and* no Firebase is the genuine "nothing is set up" case."""
|
||||
monkeypatch.setattr(_sync, "CONFIG_FILE", tmp_path / "absent_firebase.json")
|
||||
monkeypatch.setattr(_sync, "SYNC_TOKEN_FILE", tmp_path / "absent_sync_token")
|
||||
|
||||
with pytest.raises(_sync.SyncError):
|
||||
_sync._client_for_run()
|
||||
Loading…
Reference in New Issue
Block a user