mirror of
https://github.com/kuhyx/screen-locker.git
synced 2026-11-10 17:47:23 +01:00
Push under a persisted per-install uuid, not a fixed role id
screen_locker pushed manual workouts as "pc" and the workout app as "phone". A role constant collides the moment a second machine takes the same role, and after a reinstall the new install inherits the previous one's CRDT identity. Both halves now mint a uuid once and persist it: the PC in ~/.local/share/screen_locker/.device_id, the app under SharedPreferences 'crdt.nodeId' -- the same key every sibling app uses. The old constant survives as the legacy id and is passed to sync_log / syncLog, so devices/<legacy>/ is skipped as this device's own rather than re-downloaded and re-merged as a peer's on every tick. It drops to None once that path is reclaimed. Note the *read* loop in _workout_sync deliberately still merges every device including this one: the PC needs its own manual workouts in the merged view, so "skip own" would be wrong there. Only the push path and the HLC node id move. The test fixture redirecting the new id file is added in the same change that introduces the constant -- without it the first test that syncs mints a uuid into the real state dir and this machine's live sync identity is decided by whichever test ran first. Verified: ~/.local/share/screen_locker/ held no .device_id after a full suite run. Verified against the live backend, not just mocks: ran the real push_pc_workouts, which minted 57582b3b and pushed 9 records. In the RTDB the new device's log is a strict superset of both old paths -- 0 of devices/pc/'s 10 records and 0 of devices/phone/'s 9 are missing from it. Python: 907 tests, 100% branch coverage. workout_app: 313 tests, analyzer clean. Pins bumped to crdt-sync-v0.6.0 / crdt_sync_dart-v0.7.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GZH89WEqAjuJT8WZdqc7AP
This commit is contained in:
parent
d31b488311
commit
ffc6c72df0
@ -100,7 +100,7 @@ repos:
|
||||
additional_dependencies:
|
||||
- pytest
|
||||
- gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.4.0#subdirectory=gatelock
|
||||
- crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.5.1#subdirectory=crdt-sync
|
||||
- crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.6.0#subdirectory=crdt-sync
|
||||
- mcp>=2,<3
|
||||
- pydantic
|
||||
|
||||
|
||||
@ -8,7 +8,7 @@ requires-python = ">=3.10"
|
||||
# also used by diet_guard; here for phone-workout GitHub sync (Workstream C)
|
||||
dependencies = [
|
||||
"gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.4.0#subdirectory=gatelock",
|
||||
"crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.5.1#subdirectory=crdt-sync",
|
||||
"crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.6.0#subdirectory=crdt-sync",
|
||||
]
|
||||
|
||||
[project.scripts]
|
||||
|
||||
@ -3,7 +3,7 @@
|
||||
bandit>=1.7.0
|
||||
codespell>=2.2.0
|
||||
coverage>=7.4.0
|
||||
crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.5.1#subdirectory=crdt-sync
|
||||
crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.6.0#subdirectory=crdt-sync
|
||||
gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.4.0#subdirectory=gatelock
|
||||
mcp>=2,<3
|
||||
mypy>=1.8.0
|
||||
|
||||
@ -124,7 +124,20 @@ RUNNERUP_EXPORT_DIRS: tuple[str, ...] = (
|
||||
# ---------------------------------------------------------------------------
|
||||
SYNC_REPO_OWNER: str = "kuhyx"
|
||||
SYNC_REPO_NAME: str = "syncs"
|
||||
# The phone's pre-uuid device id. Still read (the phone may not have migrated
|
||||
# yet), but no longer the only path the reader knows about -- it discovers
|
||||
# every device directory.
|
||||
SYNC_PHONE_DEVICE_ID: str = "phone"
|
||||
# The id this machine pushed manual workouts under before per-install uuids.
|
||||
# Kept so the sync layer still recognises devices/pc/ as its own and does not
|
||||
# re-merge its own history as a peer's; drop to None once that path is gone.
|
||||
SYNC_LEGACY_DEVICE_ID: str | None = "pc"
|
||||
# This machine's sync id, minted once and persisted. A fixed "pc" collides as
|
||||
# soon as a second machine takes the same role, and a reinstall would inherit
|
||||
# the previous install's CRDT identity.
|
||||
SYNC_DEVICE_ID_FILE: Path = (
|
||||
Path.home() / ".local" / "share" / "screen_locker" / ".device_id"
|
||||
)
|
||||
# A fine-grained GitHub PAT, scoped to just SYNC_REPO_NAME's contents. The
|
||||
# user creates this once via github.com and saves it here, mode 600. Never
|
||||
# committed — this path is outside the repo entirely. Unlike diet_guard,
|
||||
|
||||
22
screen_locker/_device.py
Normal file
22
screen_locker/_device.py
Normal file
@ -0,0 +1,22 @@
|
||||
"""This machine's persisted sync identity.
|
||||
|
||||
Its own module so both the manual-push writer (:mod:`screen_locker._manual_push`)
|
||||
and the workout reader (:mod:`screen_locker._workout_sync`) can resolve the id
|
||||
without importing each other.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from crdt_sync import DeviceIdentity, load_device_identity
|
||||
|
||||
from screen_locker._constants import SYNC_DEVICE_ID_FILE, SYNC_LEGACY_DEVICE_ID
|
||||
|
||||
|
||||
def device_identity() -> DeviceIdentity:
|
||||
"""Return this machine's sync identity, minting it on first use.
|
||||
|
||||
Deliberately not cached at import time: the id file is redirected
|
||||
per-test, and a module-level constant would freeze whichever value the
|
||||
first import happened to see.
|
||||
"""
|
||||
return load_device_identity(SYNC_DEVICE_ID_FILE, legacy_id=SYNC_LEGACY_DEVICE_ID)
|
||||
@ -45,6 +45,7 @@ from screen_locker._constants import (
|
||||
SYNC_TIMEOUT_SECONDS,
|
||||
SYNC_TOKEN_FILE,
|
||||
)
|
||||
from screen_locker._device import device_identity
|
||||
from screen_locker._log_io import load_workout_log
|
||||
from screen_locker._log_mixin import _derive_workout_id
|
||||
from screen_locker._sync_retry import with_sync_retry
|
||||
@ -58,7 +59,6 @@ if TYPE_CHECKING:
|
||||
|
||||
_logger = logging.getLogger(__name__)
|
||||
|
||||
_PC_DEVICE_ID = "pc"
|
||||
_PAYLOAD_FIELD = "payload"
|
||||
|
||||
|
||||
@ -127,6 +127,7 @@ def records_from_workout_log(log_file: Path) -> dict[str, Record]:
|
||||
The payload is the workout's own data plus the ``kind``/``date`` the wire
|
||||
contract requires.
|
||||
"""
|
||||
identity = device_identity()
|
||||
log: dict[str, Record] = {}
|
||||
for date, entries in load_workout_log(log_file).items():
|
||||
for entry in entries:
|
||||
@ -146,7 +147,9 @@ def records_from_workout_log(log_file: Path) -> dict[str, Record]:
|
||||
)
|
||||
continue
|
||||
payload = {**workout_data, "kind": workout_data["type"], "date": date}
|
||||
hlc = Hlc.new_tick(_PC_DEVICE_ID, wall_time_ms=_entry_wall_ms(entry, date))
|
||||
hlc = Hlc.new_tick(
|
||||
identity.device_id, wall_time_ms=_entry_wall_ms(entry, date)
|
||||
)
|
||||
log[str(record_id)] = Record(
|
||||
id=str(record_id), fields={_PAYLOAD_FIELD: (payload, hlc)}
|
||||
)
|
||||
@ -154,12 +157,13 @@ def records_from_workout_log(log_file: Path) -> dict[str, Record]:
|
||||
|
||||
|
||||
def push_pc_workouts(log_file: Path) -> PushResult:
|
||||
"""Publish every counted workout in ``log_file`` to ``devices/pc/log.json``.
|
||||
"""Publish every counted workout in ``log_file`` to this device's log.
|
||||
|
||||
Runs one full sync tick, so it also retries anything a previous push failed
|
||||
to publish. It never raises into the caller — but it is never silent either:
|
||||
the returned :class:`PushResult` and a WARNING say why a push did not happen.
|
||||
"""
|
||||
identity = device_identity()
|
||||
token = read_sync_token()
|
||||
if token is None:
|
||||
reason = f"no sync token at {SYNC_TOKEN_FILE}"
|
||||
@ -190,7 +194,8 @@ def push_pc_workouts(log_file: Path) -> PushResult:
|
||||
with_sync_retry(
|
||||
lambda: sync_log(
|
||||
client=client,
|
||||
device_id=_PC_DEVICE_ID,
|
||||
device_id=identity.device_id,
|
||||
legacy_device_id=identity.legacy_id,
|
||||
path_prefix=_DEVICES_PREFIX,
|
||||
local_log=log,
|
||||
encode=_encode_log,
|
||||
|
||||
@ -35,6 +35,21 @@ def isolate_sync_token(tmp_path: Path) -> Iterator[None]:
|
||||
yield
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def isolate_sync_device_id(tmp_path: Path) -> Iterator[None]:
|
||||
"""Redirect the persisted device id so tests never mint one for real.
|
||||
|
||||
Without this the first test that syncs writes a uuid into the real
|
||||
``~/.local/share/screen_locker/.device_id``, and this machine's live sync
|
||||
identity ends up decided by whichever test happened to run first.
|
||||
"""
|
||||
with patch(
|
||||
"screen_locker._device.SYNC_DEVICE_ID_FILE",
|
||||
tmp_path / ".device_id",
|
||||
):
|
||||
yield
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def no_sync_retry_sleep() -> Iterator[None]:
|
||||
"""Make the sync retry's backoff instant for every test.
|
||||
|
||||
@ -1,10 +1,11 @@
|
||||
import 'dart:async';
|
||||
|
||||
import 'dart:async';
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:workout_app/screens/home_screen.dart';
|
||||
import 'package:workout_app/services/backup_service.dart';
|
||||
import 'package:workout_app/services/http_server_service.dart';
|
||||
import 'package:workout_app/services/storage_service.dart';
|
||||
import 'package:workout_app/services/sync_device_id.dart';
|
||||
import 'package:workout_app/ui/theme.dart';
|
||||
|
||||
// coverage:ignore-start
|
||||
@ -13,6 +14,9 @@ import 'package:workout_app/ui/theme.dart';
|
||||
// up are unit-tested individually; the entry point itself is not.
|
||||
void main() async {
|
||||
WidgetsFlutterBinding.ensureInitialized();
|
||||
// Before anything that stamps an Hlc or syncs: until this resolves, the
|
||||
// device id falls back to the pre-migration constant.
|
||||
await initSyncDeviceId();
|
||||
await BackupService.instance.requestStoragePermission();
|
||||
await StorageService.init();
|
||||
await StorageService.instance.restoreFromBackupIfNeeded();
|
||||
|
||||
@ -0,0 +1,51 @@
|
||||
/// This install's persisted sync device id.
|
||||
library;
|
||||
|
||||
import 'package:shared_preferences/shared_preferences.dart';
|
||||
import 'package:uuid/uuid.dart';
|
||||
|
||||
/// SharedPreferences key holding this install's device id.
|
||||
///
|
||||
/// The same key the sibling apps (`todo`, `home_inventory`, `diet_guard`,
|
||||
/// `wake_alarm`) use for the same purpose.
|
||||
const kSyncDeviceIdKey = 'crdt.nodeId';
|
||||
|
||||
/// The id this install pushed under before migrating to a persisted uuid.
|
||||
///
|
||||
/// Passed to `syncLog` so the log already sitting at `devices/phone/` is
|
||||
/// treated as this device's own rather than a peer's. Set to null once that
|
||||
/// path has been reclaimed.
|
||||
const legacySyncDeviceId = 'phone';
|
||||
|
||||
String? _cached;
|
||||
|
||||
/// This install's sync device id, for HLC stamping and the pushed path.
|
||||
///
|
||||
/// Synchronous because [Hlc] stamping is; call [initSyncDeviceId] once during
|
||||
/// startup, before any sync runs. Until then this falls back to
|
||||
/// [legacySyncDeviceId] so a caller that stamps too early still gets a valid
|
||||
/// (if pre-migration) id rather than crashing.
|
||||
String get currentSyncDeviceId => _cached ?? legacySyncDeviceId;
|
||||
|
||||
/// Loads (or mints and persists) this install's device id.
|
||||
///
|
||||
/// A per-install uuid rather than the fixed `phone` constant: two installs
|
||||
/// sharing an id overwrite each other's pushed file on every tick, and a
|
||||
/// reinstall would inherit the previous install's CRDT identity.
|
||||
///
|
||||
/// Idempotent -- safe to call more than once.
|
||||
Future<String> initSyncDeviceId({SharedPreferences? prefs}) async {
|
||||
final store = prefs ?? await SharedPreferences.getInstance();
|
||||
final existing = store.getString(kSyncDeviceIdKey);
|
||||
if (existing != null && existing.isNotEmpty) {
|
||||
_cached = existing;
|
||||
return existing;
|
||||
}
|
||||
final minted = const Uuid().v4();
|
||||
await store.setString(kSyncDeviceIdKey, minted);
|
||||
_cached = minted;
|
||||
return minted;
|
||||
}
|
||||
|
||||
/// Resets the cached id. Test-only.
|
||||
void resetSyncDeviceIdForTest() => _cached = null;
|
||||
@ -1,18 +1,18 @@
|
||||
/// Pushes completed workout sessions to the shared GitHub sync repo.
|
||||
library;
|
||||
|
||||
import 'dart:convert';
|
||||
|
||||
import 'dart:convert';
|
||||
import 'package:crdt_sync/crdt_sync.dart';
|
||||
import 'package:flutter/foundation.dart';
|
||||
import 'package:http/http.dart' as http;
|
||||
import 'package:workout_app/models/manual_workout.dart';
|
||||
import 'package:workout_app/models/workout_session.dart';
|
||||
import 'package:workout_app/services/firebase_backend.dart';
|
||||
import 'package:workout_app/services/sync_device_id.dart';
|
||||
import 'package:workout_app/services/sync_settings.dart';
|
||||
import 'package:workout_app/services/sync_state_factory.dart';
|
||||
|
||||
const _deviceId = 'phone';
|
||||
const _pathPrefix = 'screen-locker-sync/devices';
|
||||
const _logFilename = 'log.json';
|
||||
|
||||
@ -95,19 +95,22 @@ class WorkoutSyncService {
|
||||
? github
|
||||
: MirrorStore(primary: firebase, mirror: github);
|
||||
try {
|
||||
const path = '$_pathPrefix/$_deviceId/$_logFilename';
|
||||
final path = '$_pathPrefix/$currentSyncDeviceId/$_logFilename';
|
||||
final existingText = await client.getFileText(path);
|
||||
final existingLog = existingText == null
|
||||
? <String, Record>{}
|
||||
: _decode(existingText);
|
||||
final record = Record(
|
||||
id: session.startTime.toIso8601String(),
|
||||
fields: {'payload': (session.toJson(), Hlc.newTick(_deviceId))},
|
||||
fields: {
|
||||
'payload': (session.toJson(), Hlc.newTick(currentSyncDeviceId)),
|
||||
},
|
||||
);
|
||||
final localLog = mergeLogs(existingLog, {record.id: record});
|
||||
await syncLog(
|
||||
client: client,
|
||||
deviceId: _deviceId,
|
||||
deviceId: currentSyncDeviceId,
|
||||
legacyDeviceId: legacySyncDeviceId,
|
||||
pathPrefix: _pathPrefix,
|
||||
localLog: localLog,
|
||||
encode: _encode,
|
||||
@ -143,7 +146,7 @@ class WorkoutSyncService {
|
||||
? github
|
||||
: MirrorStore(primary: firebase, mirror: github);
|
||||
try {
|
||||
const path = '$_pathPrefix/$_deviceId/$_logFilename';
|
||||
final path = '$_pathPrefix/$currentSyncDeviceId/$_logFilename';
|
||||
final existingText = await client.getFileText(path);
|
||||
final existingLog = existingText == null
|
||||
? <String, Record>{}
|
||||
@ -151,7 +154,8 @@ class WorkoutSyncService {
|
||||
final localLog = mergeLogs(existingLog, {record.id: record});
|
||||
await syncLog(
|
||||
client: client,
|
||||
deviceId: _deviceId,
|
||||
deviceId: currentSyncDeviceId,
|
||||
legacyDeviceId: legacySyncDeviceId,
|
||||
pathPrefix: _pathPrefix,
|
||||
localLog: localLog,
|
||||
encode: _encode,
|
||||
|
||||
@ -141,11 +141,11 @@ packages:
|
||||
dependency: "direct main"
|
||||
description:
|
||||
path: crdt_sync_dart
|
||||
ref: "crdt_sync_dart-v0.6.0"
|
||||
resolved-ref: fc99eff00599a4c3195e4c1573ba08f15909a10d
|
||||
ref: "crdt_sync_dart-v0.7.0"
|
||||
resolved-ref: "6eef99d66132e12eb5056ae8e1780b34237fa013"
|
||||
url: "https://github.com/kuhyx/utils"
|
||||
source: git
|
||||
version: "0.6.0"
|
||||
version: "0.7.0"
|
||||
crypto:
|
||||
dependency: transitive
|
||||
description:
|
||||
@ -839,13 +839,13 @@ packages:
|
||||
source: hosted
|
||||
version: "3.1.5"
|
||||
uuid:
|
||||
dependency: transitive
|
||||
dependency: "direct main"
|
||||
description:
|
||||
name: uuid
|
||||
sha256: "1fef9e8e11e2991bb773070d4656b7bd5d850967a2456cfc83cf47925ba79489"
|
||||
sha256: "9b129329f58692f6e6578329498a8fe9fbe98f090beb764ffbb8ee2eadd01dcd"
|
||||
url: "https://pub.dev"
|
||||
source: hosted
|
||||
version: "4.5.3"
|
||||
version: "4.6.0"
|
||||
vector_math:
|
||||
dependency: transitive
|
||||
description:
|
||||
|
||||
@ -12,7 +12,7 @@ dependencies:
|
||||
crdt_sync:
|
||||
git:
|
||||
url: https://github.com/kuhyx/utils
|
||||
ref: crdt_sync_dart-v0.6.0
|
||||
ref: crdt_sync_dart-v0.7.0
|
||||
path: crdt_sync_dart
|
||||
flutter:
|
||||
sdk: flutter
|
||||
@ -24,6 +24,7 @@ dependencies:
|
||||
shared_preferences: ^2.5.3
|
||||
sqflite: ^2.4.2
|
||||
url_launcher: ^6.3.2
|
||||
uuid: ^4.6.0
|
||||
vibration: ^3.1.0
|
||||
|
||||
dev_dependencies:
|
||||
|
||||
@ -0,0 +1,72 @@
|
||||
import 'package:workout_app/services/sync_device_id.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:shared_preferences/shared_preferences.dart';
|
||||
import 'package:uuid/uuid.dart';
|
||||
|
||||
void main() {
|
||||
setUp(() {
|
||||
SharedPreferences.setMockInitialValues({});
|
||||
resetSyncDeviceIdForTest();
|
||||
});
|
||||
|
||||
group('initSyncDeviceId', () {
|
||||
test('mints and persists a uuid on first run', () async {
|
||||
final minted = await initSyncDeviceId();
|
||||
|
||||
expect(Uuid.isValidUUID(fromString: minted), isTrue);
|
||||
final prefs = await SharedPreferences.getInstance();
|
||||
expect(prefs.getString(kSyncDeviceIdKey), minted);
|
||||
});
|
||||
|
||||
test('returns the same id on every later run', () async {
|
||||
final first = await initSyncDeviceId();
|
||||
resetSyncDeviceIdForTest();
|
||||
|
||||
expect(await initSyncDeviceId(), first);
|
||||
});
|
||||
|
||||
test('adopts an id already stored under the shared key', () async {
|
||||
SharedPreferences.setMockInitialValues({
|
||||
kSyncDeviceIdKey: 'pre-existing-id',
|
||||
});
|
||||
|
||||
expect(await initSyncDeviceId(), 'pre-existing-id');
|
||||
});
|
||||
|
||||
test('treats a blank stored id as absent', () async {
|
||||
SharedPreferences.setMockInitialValues({kSyncDeviceIdKey: ''});
|
||||
|
||||
expect(await initSyncDeviceId(), isNotEmpty);
|
||||
});
|
||||
|
||||
test('is idempotent', () async {
|
||||
final first = await initSyncDeviceId();
|
||||
|
||||
expect(await initSyncDeviceId(), first);
|
||||
});
|
||||
});
|
||||
|
||||
group('currentSyncDeviceId', () {
|
||||
test('falls back to the role constant before init runs', () {
|
||||
// A caller that stamps an Hlc before startup finishes must still get a
|
||||
// valid id -- the pre-migration one -- rather than crash.
|
||||
expect(currentSyncDeviceId, legacySyncDeviceId);
|
||||
});
|
||||
|
||||
test('returns the persisted uuid once init has run', () async {
|
||||
final minted = await initSyncDeviceId();
|
||||
|
||||
expect(currentSyncDeviceId, minted);
|
||||
expect(currentSyncDeviceId, isNot(legacySyncDeviceId));
|
||||
});
|
||||
});
|
||||
|
||||
group('legacySyncDeviceId', () {
|
||||
test('is the id this app shipped with before the migration', () {
|
||||
// Passed to syncLog so devices/phone/ is skipped as this device's own;
|
||||
// if this drifted, the pre-migration log would be re-merged as a
|
||||
// peer's on every tick.
|
||||
expect(legacySyncDeviceId, 'phone');
|
||||
});
|
||||
});
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user