diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index eb49dee..8e75ccd 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -102,7 +102,7 @@ repos: - pytest - requests - gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.4.0#subdirectory=gatelock - - crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.3.0#subdirectory=crdt-sync + - crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.5.1#subdirectory=crdt-sync - mcp>=2,<3 - pydantic # Test suites intentionally use patterns (protected-access, magic-value diff --git a/app/lib/screens/log_meal_screen.dart b/app/lib/screens/log_meal_screen.dart index 7b4d076..1b02a0a 100644 --- a/app/lib/screens/log_meal_screen.dart +++ b/app/lib/screens/log_meal_screen.dart @@ -19,6 +19,7 @@ import 'package:diet_guard_app/services/background_tasks.dart'; import 'package:diet_guard_app/services/budget_history_service.dart'; import 'package:diet_guard_app/services/day_status_service.dart'; import 'package:diet_guard_app/services/due_slot_check.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/github_client_factory.dart'; import 'package:diet_guard_app/services/log_storage_service.dart'; @@ -125,7 +126,7 @@ class _LogMealScreenState extends State httpClient: widget.httpClient, ); try { - await runSync(client); + await runSync(await syncBackend(client)); } finally { client.close(); } diff --git a/app/lib/screens/settings_screen.dart b/app/lib/screens/settings_screen.dart index 38974da..9e32dc2 100644 --- a/app/lib/screens/settings_screen.dart +++ b/app/lib/screens/settings_screen.dart @@ -8,8 +8,10 @@ library; import 'dart:async'; +import 'package:crdt_sync/crdt_sync.dart'; import 'package:diet_guard_app/screens/log_meal_screen.dart'; 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_service.dart'; @@ -49,6 +51,9 @@ class _SettingsScreenState extends State { final _repoController = TextEditingController(); final _tokenController = TextEditingController(); final _clientIdController = TextEditingController(); + final _firebaseEmailController = TextEditingController(); + final _firebasePasswordController = TextEditingController(); + bool _firebaseConfigured = false; bool _loading = true; bool _busy = false; String? _status; @@ -81,7 +86,15 @@ class _SettingsScreenState extends State { ? settings.token : ''; _clientIdController.text = settings.clientId; - setState(() => _loading = false); + // Reflect a previously-stored account, so a returning user sees the + // real state instead of an empty form that looks unconfigured. + final account = await loadAccount(); + if (!mounted) return; + if (account != null) _firebaseEmailController.text = account.email; + setState(() { + _firebaseConfigured = account != null; + _loading = false; + }); } @override @@ -94,6 +107,8 @@ class _SettingsScreenState extends State { _ownerController.dispose(); _repoController.dispose(); _tokenController.dispose(); + _firebaseEmailController.dispose(); + _firebasePasswordController.dispose(); _clientIdController.dispose(); super.dispose(); } @@ -194,7 +209,7 @@ class _SettingsScreenState extends State { final settings = _currentSettings(); final client = createGitHubClient(settings, httpClient: widget.httpClient); try { - await runSync(client); + await runSync(await syncBackend(client)); _showMessage('Connected and synced.'); } on Exception catch (e) { _showMessage('Connected, but sync failed: $e'); @@ -232,7 +247,7 @@ class _SettingsScreenState extends State { await settings.save(); final client = createGitHubClient(settings, httpClient: widget.httpClient); try { - await runSync(client); + await runSync(await syncBackend(client)); _showMessage('Synced.'); } on Exception catch (e) { _showMessage('Sync failed: $e'); @@ -268,6 +283,63 @@ class _SettingsScreenState extends State { bool get _isAndroid => !kIsWeb && defaultTargetPlatform == TargetPlatform.android; + /// Stores the Firebase account in the OS keystore and proves it works. + /// + /// Signing in here rather than only saving means a typo surfaces now, on + /// the screen where it can be fixed, instead of as a silently failed + /// background sync hours later. + Future _saveFirebaseAccount() async { + final email = _firebaseEmailController.text.trim(); + final password = _firebasePasswordController.text; + if (email.isEmpty || password.isEmpty) { + setState(() => _status = 'Enter the sync account email and password.'); + return; + } + setState(() { + _busy = true; + _status = null; + }); + try { + await saveAccount(FirebaseAccount(email: email, password: password)); + final client = await openFirebase(); + if (client == null) { + throw Exception('could not reach Firebase'); + } + final reachable = await client.canAccessRemote(); + client.close(); + if (!mounted) return; + setState(() { + _firebaseConfigured = reachable; + _status = reachable + ? 'Firebase connected - syncs now go there first.' + : 'Signed in, but the database refused the read.'; + }); + } on Object catch (error) { + // Broader than Exception: a missing platform binding raises an Error. + // A half-stored account is worse than none, so forget it. + await clearAccount(); + if (!mounted) return; + setState(() { + _firebaseConfigured = false; + _status = 'Firebase sign-in failed: $error'; + }); + } finally { + if (mounted) setState(() => _busy = false); + } + } + + /// Forgets the Firebase account, so sync falls back to GitHub alone. + Future _disconnectFirebase() async { + await clearAccount(); + _firebaseEmailController.clear(); + _firebasePasswordController.clear(); + if (!mounted) return; + setState(() { + _firebaseConfigured = false; + _status = 'Firebase disconnected - syncing over GitHub only.'; + }); + } + @override Widget build(BuildContext context) { if (_loading) { @@ -307,6 +379,59 @@ class _SettingsScreenState extends State { const SizedBox(height: 24), const Divider(), const SizedBox(height: 8), + Text( + 'Firebase sync', + style: Theme.of(context).textTheme.titleMedium, + ), + const SizedBox(height: 8), + Text( + _firebaseConfigured + ? 'Connected. Syncs go to Firebase first, and still ' + 'mirror to GitHub until every device has moved.' + : 'Not connected - syncing over GitHub only. Enter the ' + 'shared sync account to move this device over. The ' + 'password is kept in the device keystore, never in ' + 'the app or the repo.', + style: Theme.of(context).textTheme.bodySmall, + ), + const SizedBox(height: 12), + TextField( + controller: _firebaseEmailController, + keyboardType: TextInputType.emailAddress, + autocorrect: false, + decoration: const InputDecoration( + labelText: 'Sync account email', + ), + ), + const SizedBox(height: 8), + TextField( + controller: _firebasePasswordController, + obscureText: true, + autocorrect: false, + decoration: const InputDecoration( + labelText: 'Sync account password', + ), + ), + const SizedBox(height: 12), + Row( + children: [ + FilledButton.icon( + onPressed: _busy ? null : _saveFirebaseAccount, + icon: const Icon(Icons.cloud_done), + label: const Text('Connect Firebase'), + ), + if (_firebaseConfigured) ...[ + const SizedBox(width: 8), + TextButton( + onPressed: _busy ? null : _disconnectFirebase, + child: const Text('Disconnect'), + ), + ], + ], + ), + const SizedBox(height: 24), + const Divider(), + const SizedBox(height: 8), Text( 'Authorize in your browser — no token to paste. Syncs to ' 'kuhyx/syncs by default.', diff --git a/app/lib/services/background_sync_service.dart b/app/lib/services/background_sync_service.dart index 42f18f6..10abd11 100644 --- a/app/lib/services/background_sync_service.dart +++ b/app/lib/services/background_sync_service.dart @@ -7,6 +7,7 @@ library; import 'package:diet_guard_app/services/app_settings_service.dart'; 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/github_client_factory.dart'; import 'package:diet_guard_app/services/log_storage_service.dart'; @@ -46,7 +47,7 @@ Future backgroundSyncPush({http.Client? httpClient}) async { if (!settings.isConfigured) return true; // nothing to push; don't retry final client = createGitHubClient(settings, httpClient: httpClient); try { - await runSync(client); + await runSync(await syncBackend(client)); return true; } on Exception { return false; // offline / transient GitHub error -> retry with backoff diff --git a/app/lib/services/firebase_backend.dart b/app/lib/services/firebase_backend.dart new file mode 100644 index 0000000..433126b --- /dev/null +++ b/app/lib/services/firebase_backend.dart @@ -0,0 +1,105 @@ +/// Wiring for the Firebase backend, during and after the GitHub cutover. +/// +/// Split by what is safe to publish, because this repo is public: +/// +/// * [kProject] holds the Web API key and database URL. Both are public +/// identifiers that already ship inside the APK; the security rules, not +/// their secrecy, are what protect the data. +/// * The account email and password are entered once per device and kept in +/// the OS keystore, next to the GitHub token this app already stores there. +/// +/// Nothing here reads `~/.config/crdt-sync/` — that is the desktop/Python +/// half. On Android there is no such file. +library; + +import 'package:crdt_sync/crdt_sync.dart'; +import 'package:flutter_secure_storage/flutter_secure_storage.dart'; + +/// The shared `kuhy-syncs` project. +/// +/// `databaseUrl` is the **regional** host. The plain `*.firebaseio.com` form +/// answers 404 with a `correctUrl` body rather than an obvious error, which +/// reads like an auth failure and wastes a debugging session. +const kProject = FirebaseProject( + apiKey: 'AIzaSyCF_sA3xCMehAYXK8eND-rAygb9NXXW_8E', + databaseUrl: + 'https://kuhy-syncs-default-rtdb.europe-west1.firebasedatabase.app', +); + +/// Same options the GitHub token uses: off Android's deprecated +/// `encryptedSharedPreferences` path, and libsecret on Linux. +const _secure = FlutterSecureStorage(); + +// Everything below reaches the OS keystore through a platform channel, which +// `flutter test` has no binding for -- the same reason `main.dart` and +// `openRepository()` are excluded. The logic these wrap (parsing, the +// public/private split, sign-in) lives in `crdt_sync` and is covered there at +// 100%; what is left here is the two-line adapter. +// coverage:ignore-start + +/// The keystore-backed home for the Firebase refresh token. +SecureCredentialStore credentialStore() => SecureCredentialStore( + read: (key) => _secure.read(key: key), + write: (key, value) => _secure.write(key: key, value: value), + delete: (key) => _secure.delete(key: key), +); + +/// Reads the per-device account, or null when sync has not been set up. +Future loadAccount() async { + try { + return FirebaseAccount.tryParse( + await _secure.read(key: kFirebaseAccountKey), + ); + } on Object { + // No secret service available: behave as "not configured" rather than + // crashing the settings screen. + return null; + } +} + +/// Stores the per-device account. Keystore only — never prefs, never source. +Future saveAccount(FirebaseAccount account) => + _secure.write(key: kFirebaseAccountKey, value: account.toJsonString()); + +/// Forgets the account and any cached session. +Future clearAccount() async { + await _secure.delete(key: kFirebaseAccountKey); + await credentialStore().clear(); +} + +/// Returns a signed-in Firebase client, or null when not configured. +/// +/// Signs in with the stored password only when there is no cached refresh +/// token, so the usual path costs no authentication round trip. +Future openFirebase() async { + final account = await loadAccount(); + if (account == null) return null; + try { + return await firebaseClientFor( + config: kProject.configFor(account.email), + store: credentialStore(), + password: account.password, + ); + } on Object { + // 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. + return null; + } +} + +// coverage:ignore-end + +/// Wraps [github] so Firebase is primary and GitHub is kept as a mirror. +/// +/// Returns [github] unchanged when this device has not been set up for +/// Firebase — a normal state during the cutover, not an error, and also the +/// rollback. Reads union both, so a device that has not moved yet still +/// converges. +Future syncBackend(RemoteStore github) async { + final firebase = await openFirebase(); + return firebase == null + ? github + : MirrorStore(primary: firebase, mirror: github); +} diff --git a/app/lib/services/sync_service.dart b/app/lib/services/sync_service.dart index 4590159..7243fbc 100644 --- a/app/lib/services/sync_service.dart +++ b/app/lib/services/sync_service.dart @@ -20,6 +20,7 @@ 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_merge.dart'; +import 'package:diet_guard_app/services/sync_state_factory.dart'; const _devicesDir = 'diet-guard-sync/devices'; @@ -28,7 +29,7 @@ const _devicesDir = 'diet-guard-sync/devices'; /// Returns the merged log as it now sits on disk locally. Propagates any /// [GitHubSyncError] from the client for the caller (auto-sync / the manual /// "Sync now" action) to decide how to report. -Future runSync(GitHubClient client) async { +Future runSync(RemoteStore client, {SyncStateStore? stateStore}) async { final logService = LogStorageService.instance; final local = await logService.readLog(); @@ -41,6 +42,11 @@ Future runSync(GitHubClient client) async { decode: parseRemoteLog, filename: 'food_log.json', commitMessage: 'diet_guard_app sync', + // Without this every sync re-downloads every peer's whole food log -- + // hundreds of KB -- whether or not anything changed, and re-pushes an + // unchanged one. That is the traffic the Firebase free tier's monthly + // budget depends on not happening. + stateStore: stateStore ?? await openSyncStateStore(), ); final merged = logToDayLog(mergedLog); @@ -58,7 +64,7 @@ Future runSync(GitHubClient client) async { /// Runs after the local rebuild above, so this device's records already /// reflect the merged log; the merge then unions in whatever another device /// knows, max-count winning per food. Mirrors `_sync._sync_food_bank`. -Future _syncFoodBank(GitHubClient client) async { +Future _syncFoodBank(RemoteStore client) async { final merged = await syncLog( client: client, deviceId: syncDeviceId, @@ -78,7 +84,7 @@ Future _syncFoodBank(GitHubClient client) async { /// the food log, so unlike `food_bank.json` they need a real merge: /// last-writer-wins per food name by edit time, union across devices. Mirrors /// `_sync._sync_manual_bank`. -Future _syncManualBank(GitHubClient client) async { +Future _syncManualBank(RemoteStore client) async { final local = await FoodBankService.instance.readManualBank(); final merged = await syncLog( client: client, @@ -102,7 +108,7 @@ Future _syncManualBank(GitHubClient client) async { /// [AppSettingsService.dailyKcalGoalUpdatedAt]), so a fresh install's /// unset 2200 default can never spuriously outrank a real budget synced /// from elsewhere. -Future _syncBudget(GitHubClient client) async { +Future _syncBudget(RemoteStore client) async { final updatedAt = AppSettingsService.dailyKcalGoalUpdatedAt; final localRecord = updatedAt == null ? null diff --git a/app/lib/services/sync_state_factory.dart b/app/lib/services/sync_state_factory.dart new file mode 100644 index 0000000..528ae67 --- /dev/null +++ b/app/lib/services/sync_state_factory.dart @@ -0,0 +1,9 @@ +/// Platform entry point for the sync revision cache. +/// +/// A conditional export because `dart:io` cannot even be *imported* in a web +/// compile: Android gets a file, the Chrome-wrapper desktop build gets a +/// `SharedPreferences` entry. +library; + +export 'sync_state_factory_io.dart' + if (dart.library.js_interop) 'sync_state_factory_web.dart'; diff --git a/app/lib/services/sync_state_factory_io.dart b/app/lib/services/sync_state_factory_io.dart new file mode 100644 index 0000000..9a33dda --- /dev/null +++ b/app/lib/services/sync_state_factory_io.dart @@ -0,0 +1,37 @@ +import 'dart:io'; + +import 'package:crdt_sync/crdt_sync.dart'; +import 'package:crdt_sync/crdt_sync_io.dart'; +import 'package:path/path.dart' as p; +import 'package:path_provider/path_provider.dart'; + +/// File holding the revision cache, beside the food log it describes. +const kSyncStateFileName = 'sync_state.json'; + +/// Opens the revision cache on a `dart:io` platform (Android). +/// +/// Falls back to an in-memory cache when the platform channel is unavailable +/// (no binding under `flutter test`, or a plugin that failed to register). +/// Losing the cache costs one tick of extra traffic; failing here would take +/// sync down entirely, which is far worse than the thing being optimised. +Future openSyncStateStore() async { + try { + final dir = await getApplicationSupportDirectory(); + return openSyncStateStoreIn(dir.path); + } on Object { + // Deliberately broader than Exception: an uninitialised binding raises a + // FlutterError, which is an Error. Either way the platform is + // unavailable and an in-memory cache is the right degradation. + return InMemorySyncStateStore(); + } +} + +/// Opens the revision cache rooted at [dirPath]. +/// +/// Lives next to the log it describes and must be cleared with it: skipping +/// an unchanged peer is only sound because that peer's records are already +/// merged into the local log, so state that outlived its log would skip peers +/// whose data had been lost. +SyncStateStore openSyncStateStoreIn(String dirPath) => PersistedSyncStateStore( + FileLogPersistence(File(p.join(dirPath, kSyncStateFileName))), +); diff --git a/app/lib/services/sync_state_factory_web.dart b/app/lib/services/sync_state_factory_web.dart new file mode 100644 index 0000000..3a3abed --- /dev/null +++ b/app/lib/services/sync_state_factory_web.dart @@ -0,0 +1,27 @@ +import 'package:crdt_sync/crdt_sync.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +/// Key holding the revision cache in `SharedPreferences`. +const kSyncStateKey = 'sync.state'; + +/// Opens the revision cache on web (the Chrome-wrapper desktop build). +/// +/// `SharedPreferences` rather than IndexedDB: the payload is two short hashes +/// per peer, so a key-value entry is the whole requirement, and the inventory +/// log already owns the IndexedDB store. +Future openSyncStateStore() async => PersistedSyncStateStore( + _PrefsPersistence(await SharedPreferences.getInstance()), +); + +class _PrefsPersistence implements LogPersistence { + _PrefsPersistence(this._prefs); + + final SharedPreferences _prefs; + + @override + Future read() async => _prefs.getString(kSyncStateKey); + + @override + Future write(String text) async => + _prefs.setString(kSyncStateKey, text); +} diff --git a/app/pubspec.lock b/app/pubspec.lock index c0bec70..6617784 100644 --- a/app/pubspec.lock +++ b/app/pubspec.lock @@ -85,11 +85,11 @@ packages: dependency: "direct main" description: path: crdt_sync_dart - ref: "crdt_sync_dart-v0.3.0" - resolved-ref: "5480e723354c31e2a38dd2757d5d2c75a04e9618" + ref: "crdt_sync_dart-v0.6.0" + resolved-ref: fc99eff00599a4c3195e4c1573ba08f15909a10d url: "https://github.com/kuhyx/utils" source: git - version: "0.3.0" + version: "0.6.0" crypto: dependency: "direct main" description: diff --git a/app/pubspec.yaml b/app/pubspec.yaml index b708d3a..0697370 100644 --- a/app/pubspec.yaml +++ b/app/pubspec.yaml @@ -11,7 +11,7 @@ dependencies: crdt_sync: git: url: https://github.com/kuhyx/utils - ref: crdt_sync_dart-v0.3.0 + ref: crdt_sync_dart-v0.6.0 path: crdt_sync_dart crypto: ^3.0.6 flutter: diff --git a/app/test/services/background_sync_service_test.dart b/app/test/services/background_sync_service_test.dart index 25004c6..fb0c6aa 100644 --- a/app/test/services/background_sync_service_test.dart +++ b/app/test/services/background_sync_service_test.dart @@ -82,7 +82,9 @@ void main() { expect(ok, isTrue); // syncLog always pushes, even an empty merged result: food_log.json, // budget.json, food_bank.json, food_bank_manual.json. - expect(puts, 4); + // Four data files plus this device's revision, which is what lets a + // later tick skip an unchanged peer. + expect(puts, 5); }); test('reports failure (retry) when the push errors', () async { diff --git a/app/test/services/sync_service_test.dart b/app/test/services/sync_service_test.dart index 5b547a9..48bc749 100644 --- a/app/test/services/sync_service_test.dart +++ b/app/test/services/sync_service_test.dart @@ -145,8 +145,10 @@ void main() { expect(merged.values.expand((e) => e).length, 1); // syncLog always pushes, even an empty merged result: food_log.json, - // budget.json, food_bank.json, food_bank_manual.json. - expect(fake.puts, hasLength(4)); + // budget.json, food_bank.json, food_bank_manual.json -- plus this + // device's revision, published after the log so a peer can never cache + // "seen rev X" against a log it never received. + expect(fake.puts, hasLength(5)); }); test("skips its own device id ('phone') when listing", () async { diff --git a/app/test/services/sync_state_factory_test.dart b/app/test/services/sync_state_factory_test.dart new file mode 100644 index 0000000..7600947 --- /dev/null +++ b/app/test/services/sync_state_factory_test.dart @@ -0,0 +1,89 @@ +/// Tests for the revision cache and the backend choice. +/// +/// The cache is what keeps a sync tick from re-downloading every peer's whole +/// food log when nothing changed, so "does it survive a restart" and "does a +/// dead platform channel take sync down with it" are the behaviours worth +/// pinning. +@TestOn('vm') +library; + +import 'dart:io'; + +import 'package:crdt_sync/crdt_sync.dart'; +import 'package:diet_guard_app/services/firebase_backend.dart'; +import 'package:diet_guard_app/services/sync_state_factory_io.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:path/path.dart' as p; + +/// A do-nothing [RemoteStore], standing in for the GitHub client. +class _StubRemote implements RemoteStore { + @override + Future> listDirectory(String path) async => []; + + @override + Future getFileText(String path) async => null; + + @override + Future putFileText( + String path, + String text, { + required String message, + }) async {} + + @override + Future deleteFile(String path, {String message = ''}) async {} + + @override + Future canAccessRemote() async => true; + + @override + void close() {} +} + +void main() { + late Directory dir; + + setUp(() async { + dir = await Directory.systemTemp.createTemp('dg_state_'); + }); + + tearDown(() async { + if (dir.existsSync()) await dir.delete(recursive: true); + }); + + test('persists revisions across a restart', () async { + // A fresh store instance stands in for the next app launch: an in-memory + // cache would forget every peer and re-download all of them. + await openSyncStateStoreIn(dir.path).save( + const SyncState(pushedRev: 'mine', peerRevs: {'pc': 'theirs'}), + ); + + final reloaded = await openSyncStateStoreIn(dir.path).load(); + + expect(reloaded.pushedRev, 'mine'); + expect(reloaded.peerRevs, {'pc': 'theirs'}); + }); + + test('writes beside the log it describes', () async { + await openSyncStateStoreIn(dir.path).save(const SyncState(pushedRev: 'x')); + + expect(File(p.join(dir.path, kSyncStateFileName)).existsSync(), isTrue); + }); + + test('degrades to memory when the platform channel is unavailable', () async { + // No binding under `flutter test`. Losing the cache costs one tick of + // extra traffic; failing would take sync down entirely. + final store = await openSyncStateStore(); + + expect(store, isA()); + expect((await store.load()).pushedRev, isNull); + }); + + test('stays on GitHub when Firebase is not set up', () async { + // The pre-migration path, and the rollback. Reaching the keystore without + // a binding must read as "not configured", not as an error. + final github = _StubRemote(); + + expect(await syncBackend(github), same(github)); + }); +} diff --git a/diet_guard/_constants.py b/diet_guard/_constants.py index 16806ae..625c057 100644 --- a/diet_guard/_constants.py +++ b/diet_guard/_constants.py @@ -21,6 +21,12 @@ BUDGET_WARN_FRACTION: float = 0.80 # is deliberately NOT committed to git (unlike wake_state.json). DATA_DIR: Path = Path.home() / ".local" / "share" / "diet_guard" FOOD_LOG_FILE: Path = DATA_DIR / "food_log.json" + +# Revision cache for cross-device sync. Beside the log it describes and +# cleared with it: skipping an unchanged peer is only sound because that +# peer's records are already merged into the local log, so state that outlived +# its log would skip peers whose data had been lost. +SYNC_STATE_FILE: Path = DATA_DIR / "sync_state.json" # The user's personal "food bank": every food they have logged before, with its # full macros, keyed by name. This is the ONLY corpus the gate's autocomplete # searches -- Open Food Facts is used to *fill* a new food's macros, never to diff --git a/diet_guard/_sync.py b/diet_guard/_sync.py index 3773039..ddf6979 100644 --- a/diet_guard/_sync.py +++ b/diet_guard/_sync.py @@ -19,7 +19,21 @@ from __future__ import annotations import json import logging -from crdt_sync import GitHubSyncClient, GitHubSyncError, Log, merge_logs +from crdt_sync import ( + CONFIG_FILE, + ConfigError, + FileSyncStateStore, + FirebaseAuthError, + GitHubSyncClient, + GitHubSyncError, + Log, + RemoteStore, + RemoteSyncError, + SyncState, + merge_logs, + mirror_client_for, + revision_of, +) from diet_guard._budget import read_raw_record, write_raw_record from diet_guard._budget_history import ( @@ -31,6 +45,7 @@ from diet_guard._constants import ( SYNC_DEVICE_ID, SYNC_REPO_NAME, SYNC_REPO_OWNER, + SYNC_STATE_FILE, SYNC_TIMEOUT_SECONDS, SYNC_TOKEN_FILE, ) @@ -56,12 +71,43 @@ from diet_guard._sync_merge import ( _logger = logging.getLogger(__name__) _DEVICES_DIR = "diet-guard-sync/devices" +# One small text node per device, each written only by its owner. Deliberately +# not one shared map: a whole-map write would erase every other device's entry, +# after which those peers would look permanently unchanged and never be +# fetched again. +_REVS_DIR = "diet-guard-sync/revs" class SyncError(Exception): """Raised when a sync run cannot even start (no usable PAT).""" +def _remote_client(github: GitHubSyncClient) -> RemoteStore: + """Return the backend to sync against. + + Firebase when ``~/.config/crdt-sync/`` is set up, with GitHub kept as a + mirror so a device that has not moved yet still converges; GitHub alone + otherwise. An unconfigured machine keeps syncing exactly as before. + + Every sub-sync here (log, budget, food bank, manual bank) takes the client + as a parameter, so this one swap covers all of them. + + The config file is checked before constructing anything, so an + unconfigured machine never reaches the network -- otherwise a suite that + blocks real sockets fails here rather than in the code under test. + + Rolling back is deleting this function and passing ``github`` straight + through: no data moves either way. + """ + if not CONFIG_FILE.is_file(): + return github + try: + return mirror_client_for("diet_guard", github) + except (ConfigError, FirebaseAuthError, RemoteSyncError) as exc: + _logger.warning("Firebase unavailable, syncing via GitHub only: %s", exc) + return github + + 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" @@ -103,7 +149,30 @@ def _read_token() -> str: return token -def _pull_remote_logs(client: GitHubSyncClient) -> list[Log]: +def _remote_revisions(client: RemoteStore) -> dict[str, str]: + """Return every peer's published food-log revision, cheaply where possible. + + Degrades to an empty map -- meaning "fetch everything", the old behaviour + -- on a backend without a bulk-map read, so correctness never depends on + the optimisation being available. + """ + get_string_map = getattr(client, "get_string_map", None) + if get_string_map is None: + return {} + try: + return get_string_map(_REVS_DIR) + except (GitHubSyncError, RemoteSyncError): + # A revision map that cannot be read is not worth failing a sync + # over; without it every peer is simply fetched, as before. + return {} + + +def _pull_remote_logs( + client: RemoteStore, + remote_revs: dict[str, str], + state: SyncState, + seen_revs: dict[str, str], +) -> list[Log]: """Return every other device's last-pushed log, skipping this one. A device whose pushed file is corrupt, truncated, or otherwise @@ -111,18 +180,33 @@ def _pull_remote_logs(client: GitHubSyncClient) -> list[Log]: that has never pushed at all -- GitHub is an external system boundary, and one bad device's file must not stall merging in every other device's. + + A peer whose published revision matches the one already merged is skipped + without downloading it at all. This is the single largest traffic saving + in the fleet: these logs are hundreds of KB and this timer runs 96 times a + day, so re-reading an unchanged peer is pure waste. """ remote_logs: list[Log] = [] for device_id in client.list_directory(_DEVICES_DIR): if device_id == SYNC_DEVICE_ID: continue + remote_rev = remote_revs.get(device_id) + if remote_rev is not None and remote_rev == state.peer_revs.get(device_id): + # Already merged, and that merge is in the local log. Carry the + # revision forward so it stays skipped next tick. + seen_revs[device_id] = remote_rev + continue text = client.get_file_text(_device_log_path(device_id)) if text is None: continue try: remote_logs.append(parse_remote_log(text)) except (TypeError, KeyError, ValueError, json.JSONDecodeError): + # Deliberately not recorded as seen: a corrupt push must be + # retried next tick, not remembered as merged. _logger.warning("Unparsable log pushed by device %r, skipping", device_id) + continue + seen_revs[device_id] = remote_rev or revision_of(text) return remote_logs @@ -289,12 +373,22 @@ def run_sync() -> DayLog: report it. """ token = _read_token() - client = GitHubSyncClient( - SYNC_REPO_OWNER, SYNC_REPO_NAME, token, timeout_seconds=SYNC_TIMEOUT_SECONDS + client = _remote_client( + GitHubSyncClient( + SYNC_REPO_OWNER, + SYNC_REPO_NAME, + token, + timeout_seconds=SYNC_TIMEOUT_SECONDS, + ) ) + state_store = FileSyncStateStore(SYNC_STATE_FILE) + state = state_store.load() + remote_revs = _remote_revisions(client) + seen_revs: dict[str, str] = {} + merged = daylog_to_log(read_raw_log()) - for remote_log in _pull_remote_logs(client): + for remote_log in _pull_remote_logs(client, remote_revs, state, seen_revs): merged = merge_logs(merged, remote_log) merged_daylog = log_to_daylog(merged) @@ -313,11 +407,21 @@ def run_sync() -> DayLog: {record_id: record.to_dict() for record_id, record in push_log.items()}, indent=2, ) - client.put_file_text( - _device_log_path(SYNC_DEVICE_ID), - push_json, - message="diet_guard sync", - ) + revision = revision_of(push_json) + if revision != state.pushed_rev: + client.put_file_text( + _device_log_path(SYNC_DEVICE_ID), + push_json, + message="diet_guard sync", + ) + # Published after the log, never before: a peer that cached "seen rev + # X" against a log it never received would skip it forever. + client.put_file_text( + f"{_REVS_DIR}/{SYNC_DEVICE_ID}", + revision, + message="diet_guard sync: revision", + ) + state_store.save(SyncState(pushed_rev=revision, peer_revs=seen_revs)) return resigned diff --git a/diet_guard/tests/conftest.py b/diet_guard/tests/conftest.py index a63e37e..6d7c22b 100644 --- a/diet_guard/tests/conftest.py +++ b/diet_guard/tests/conftest.py @@ -116,6 +116,17 @@ def _isolate_state(tmp_path: Path) -> Iterator[None]: "diet_guard._sync.SYNC_TOKEN_FILE", tmp_path / "sync_token", ), + patch( + "diet_guard._sync.SYNC_STATE_FILE", + tmp_path / "sync_state.json", + ), + # `run_sync` reads this to decide whether to build a Firebase-primary + # mirror. On a developer machine the real file exists, so without this + # every sync test would sign in and push to the live database. + patch( + "diet_guard._sync.CONFIG_FILE", + tmp_path / "nonexistent-firebase.json", + ), ): yield diff --git a/diet_guard/tests/test_sync.py b/diet_guard/tests/test_sync.py index 43b4ea9..3ac9f75 100644 --- a/diet_guard/tests/test_sync.py +++ b/diet_guard/tests/test_sync.py @@ -8,8 +8,10 @@ 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 import pytest from diet_guard import _sync @@ -58,6 +60,11 @@ def _mock_client( client.list_directory.return_value = list(devices) resolved_files = files or {} client.get_file_text.side_effect = resolved_files.get + # The real GitHubSyncClient has no bulk-map read, and a MagicMock would + # otherwise auto-create one returning a Mock -- which then fails to + # serialise. Deleting it also exercises the documented degrade path: + # no revision map means every peer is fetched, exactly as before. + del client.get_string_map return client @@ -94,14 +101,20 @@ class TestRunSync: assert sum(len(entries) for entries in merged.values()) == 1 pushed = [call.args[0] for call in client.put_file_text.call_args_list] - # The log push, plus the derived bank rebuilt from that same log. + # The log push, the derived bank rebuilt from that same log, and this + # device's revision -- published *after* the log, so a peer can never + # cache "seen rev X" against a log it never received. assert pushed == [ "diet-guard-sync/devices/pc/food_bank.json", "diet-guard-sync/devices/pc/food_log.json", + "diet-guard-sync/revs/pc", ] - pushed_path = client.put_file_text.call_args.args[0] - assert pushed_path == "diet-guard-sync/devices/pc/food_log.json" - pushed_json = client.put_file_text.call_args.args[1] + log_call = next( + call + for call in client.put_file_text.call_args_list + if call.args[0].endswith("food_log.json") + ) + pushed_json = log_call.args[1] pushed = json.loads(pushed_json) (record,) = pushed.values() assert "fields" in record @@ -357,3 +370,122 @@ class TestPullSharedLog: pytest.raises(KeyError), ): _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.""" + + def test_is_empty_without_a_bulk_map_read(self) -> None: + """GitHub has none; correctness must not depend on the optimisation.""" + client = MagicMock() + del client.get_string_map + + assert _sync._remote_revisions(client) == {} + + def test_returns_the_published_revisions(self) -> None: + client = MagicMock() + client.get_string_map.return_value = {"phone": "abc"} + + assert _sync._remote_revisions(client) == {"phone": "abc"} + + def test_an_unreadable_map_degrades_to_fetching_everything(self) -> None: + """Not worth failing a sync over; every peer is simply fetched.""" + client = MagicMock() + client.get_string_map.side_effect = GitHubSyncError("boom") + + assert _sync._remote_revisions(client) == {} + + +class TestPullSkipsUnchangedPeers: + """The single largest traffic saving in the fleet.""" + + def test_skips_a_peer_whose_revision_is_unchanged(self) -> None: + client = _mock_client(devices=("phone",)) + state = SyncState(pushed_rev=None, peer_revs={"phone": "rev-1"}) + seen: dict[str, str] = {} + + logs = _sync._pull_remote_logs(client, {"phone": "rev-1"}, state, seen) + + assert logs == [] + assert seen == {"phone": "rev-1"} + client.get_file_text.assert_not_called() + + def test_downloads_a_peer_whose_revision_moved(self) -> None: + text = json.dumps({}) + client = _mock_client( + devices=("phone",), + files={"diet-guard-sync/devices/phone/food_log.json": text}, + ) + state = SyncState(pushed_rev=None, peer_revs={"phone": "rev-1"}) + seen: dict[str, str] = {} + + _sync._pull_remote_logs(client, {"phone": "rev-2"}, state, seen) + + client.get_file_text.assert_called_once() + assert seen == {"phone": "rev-2"} + + +class TestNoOpPushSuppression: + """88% of the old GitHub history was byte-identical no-op pushes.""" + + def test_a_second_unchanged_sync_pushes_no_log(self) -> None: + """The saving the free-tier budget depends on, at 96 ticks a day.""" + _write_token() + client = _mock_client() + + with patch.object(_sync, "GitHubSyncClient", return_value=client): + _sync.run_sync() + pushed_first = [ + call.args[0] for call in client.put_file_text.call_args_list + ] + client.put_file_text.reset_mock() + _sync.run_sync() + + pushed_second = [call.args[0] for call in client.put_file_text.call_args_list] + assert "diet-guard-sync/devices/pc/food_log.json" in pushed_first + assert "diet-guard-sync/devices/pc/food_log.json" not in pushed_second + assert "diet-guard-sync/revs/pc" not in pushed_second