diff --git a/crdt-sync/crdt_sync/__init__.py b/crdt-sync/crdt_sync/__init__.py index 83523e3..cda73bf 100644 --- a/crdt-sync/crdt_sync/__init__.py +++ b/crdt-sync/crdt_sync/__init__.py @@ -12,6 +12,7 @@ from crdt_sync._config import ( firebase_client_for, mirror_client_for, ) +from crdt_sync._device import DeviceIdentity, load_device_identity from crdt_sync._firebase import ( DatabaseNotFoundError, FirebaseSyncClient, @@ -49,6 +50,7 @@ __all__ = [ "ConfigError", "CredentialStore", "DatabaseNotFoundError", + "DeviceIdentity", "Field", "FileCredentialStore", "FileSyncStateStore", @@ -76,6 +78,7 @@ __all__ = [ "default_revs_path", "dump_log", "firebase_client_for", + "load_device_identity", "load_log", "merge_field", "merge_logs", diff --git a/crdt-sync/crdt_sync/_device.py b/crdt-sync/crdt_sync/_device.py new file mode 100644 index 0000000..091c511 --- /dev/null +++ b/crdt-sync/crdt_sync/_device.py @@ -0,0 +1,87 @@ +"""This device's stable sync identity. + +Every device that pushes into a namespace needs an id that is *its own*: it +is the directory segment its log is pushed under +(``//``), the revision key, and the node +component baked into every HLC stamp it writes (``--``). + +Two devices sharing an id overwrite each other's pushed file on every tick, +so the id must be per-*install*, not per-role: a fixed ``"pc"``/``"phone"`` +constant silently collides the moment a second machine takes the same role, +and a reinstall inherits the previous install's CRDT identity. + +A migration from a role constant to a persisted uuid cannot rewrite history. +Stamps already written keep the old id, and the old path keeps the log that +was pushed under it -- so a device that has just switched ids must still +recognise its *former* id as itself. That is what [legacy_id] carries, and +why [DeviceIdentity.own_ids] exists: skip-own-writes has to test membership +of that set rather than equality with one id, or the device re-downloads and +re-merges its own pre-migration history as though a peer had written it. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import TYPE_CHECKING +import uuid + +if TYPE_CHECKING: + from pathlib import Path + + +@dataclass(frozen=True) +class DeviceIdentity: + """This device's current sync id, plus the id it used to push under. + + Attributes: + device_id: The id to push, stamp and key revisions under from now on. + legacy_id: The id this device pushed under before the migration to + persisted uuids, or ``None`` once its old path has been reclaimed. + """ + + device_id: str + legacy_id: str | None = None + + @property + def own_ids(self) -> frozenset[str]: + """Every id that means "this device", for skip-own-writes checks.""" + if self.legacy_id is None: + return frozenset({self.device_id}) + return frozenset({self.device_id, self.legacy_id}) + + def is_own(self, other_device_id: str) -> bool: + """Return whether [other_device_id] is one of this device's own ids.""" + return other_device_id in self.own_ids + + +def load_device_identity(path: Path, *, legacy_id: str | None = None) -> DeviceIdentity: + """Return the id persisted at [path], creating it on first call. + + The file holds exactly one uuid4 and is written once; every later call + returns the same value, which is what makes the id stable across restarts + (and is why it must live in durable state, not a temp dir). + + Args: + path: File holding this device's uuid. Parent dirs are created. + legacy_id: The role constant this device pushed under before + migrating, recorded on the returned identity so skip-own-writes + still recognises the old path as this device's own. Pass ``None`` + once that path has been reclaimed. + + Returns: + This device's [DeviceIdentity]. + """ + try: + existing = path.read_text(encoding="utf-8").strip() + except FileNotFoundError: + existing = "" + if existing: + return DeviceIdentity(device_id=existing, legacy_id=legacy_id) + + # Deliberately not guarded: if the state directory cannot be created the + # id cannot be persisted, and returning an unpersisted one would mint a + # fresh uuid on every restart, stranding a device directory each time. + minted = str(uuid.uuid4()) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(f"{minted}\n", encoding="utf-8") + return DeviceIdentity(device_id=minted, legacy_id=legacy_id) diff --git a/crdt-sync/crdt_sync/_sync.py b/crdt-sync/crdt_sync/_sync.py index 947154a..0ae1307 100644 --- a/crdt-sync/crdt_sync/_sync.py +++ b/crdt-sync/crdt_sync/_sync.py @@ -204,7 +204,7 @@ def _remote_revs(client: RemoteStore, revs_path: str) -> dict[str, str]: def _pull_remote_logs( client: RemoteStore, - device_id: str, + own_ids: frozenset[str], path_prefix: str, filename: str, decode: Callable[[str], Log], @@ -219,12 +219,18 @@ def _pull_remote_logs( the remote is an external system boundary, and one bad device's file must not stall merging in every other device's. + ``own_ids`` is a *set* rather than a single id because a device that has + migrated from a role constant to a persisted uuid still owns the log it + pushed under the old id. Matching only the current id would pull that + file back and re-merge this device's own pre-migration history as though + a peer had written it -- idempotent, but pure wasted transfer. + Records into ``seen_revs`` which peers are now merged in, so the next tick can skip them. """ remote_logs: list[Log] = [] for other_device_id in client.list_directory(path_prefix): - if other_device_id == device_id: + if other_device_id in own_ids: continue remote_rev = remote_revs.get(other_device_id) if remote_rev is not None and remote_rev == state.peer_revs.get( @@ -265,6 +271,7 @@ def sync_log( commit_message: str = "crdt_sync: update log", state_store: SyncStateStore | None = None, revs_path: str | None = None, + legacy_device_id: str | None = None, ) -> Log: """Run one full sync tick: pull every other device's log, merge, push. @@ -295,6 +302,11 @@ def sync_log( always did: fetch everything, push unconditionally. revs_path: Where revisions live; defaults to :func:`default_revs_path`. + legacy_device_id: The id this device pushed under before migrating to + a persisted uuid. Treated as this device's own for skip-own + purposes, so its pre-migration log is not pulled back and + re-merged as a peer's. Pass ``None`` once the old path has been + reclaimed. Returns: The merged log, as pushed. @@ -311,11 +323,14 @@ def sync_log( state = state_store.load() if state_store is not None else SyncState() remote_revs = _remote_revs(client, revs) if state_store is not None else {} + own_ids = frozenset( + {device_id} if legacy_device_id is None else {device_id, legacy_device_id} + ) merged = dict(local_log) seen_revs: dict[str, str] = {} for remote_log in _pull_remote_logs( client, - device_id, + own_ids, path_prefix, filename, decode, diff --git a/crdt-sync/crdt_sync/tests/test_device.py b/crdt-sync/crdt_sync/tests/test_device.py new file mode 100644 index 0000000..e0bc7e8 --- /dev/null +++ b/crdt-sync/crdt_sync/tests/test_device.py @@ -0,0 +1,113 @@ +"""Tests for this device's persisted sync identity.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING +import uuid + +import pytest + +from crdt_sync import DeviceIdentity, load_device_identity + +if TYPE_CHECKING: + from pathlib import Path + + +def test_mints_a_uuid_on_first_call(tmp_path: Path) -> None: + """A device with no id file gets a fresh uuid4, persisted.""" + path = tmp_path / "device_id" + + identity = load_device_identity(path) + + assert uuid.UUID(identity.device_id).version == 4 + assert path.read_text(encoding="utf-8").strip() == identity.device_id + + +def test_returns_the_same_id_on_every_later_call(tmp_path: Path) -> None: + """The id is stable across restarts -- that is the whole point.""" + path = tmp_path / "device_id" + + first = load_device_identity(path) + second = load_device_identity(path) + + assert first.device_id == second.device_id + + +def test_creates_missing_parent_directories(tmp_path: Path) -> None: + """First run on a clean machine has no state dir yet.""" + path = tmp_path / "nested" / "deeper" / "device_id" + + identity = load_device_identity(path) + + assert path.read_text(encoding="utf-8").strip() == identity.device_id + + +def test_treats_a_blank_file_as_absent(tmp_path: Path) -> None: + """An empty/whitespace file (interrupted first write) re-mints.""" + path = tmp_path / "device_id" + path.write_text(" \n", encoding="utf-8") + + identity = load_device_identity(path) + + assert uuid.UUID(identity.device_id).version == 4 + + +def test_reads_an_id_that_is_not_a_uuid(tmp_path: Path) -> None: + """Whatever is on disk wins; the file is the source of truth.""" + path = tmp_path / "device_id" + path.write_text("hand-edited-id\n", encoding="utf-8") + + assert load_device_identity(path).device_id == "hand-edited-id" + + +def test_a_file_blocking_the_state_dir_surfaces_as_an_error(tmp_path: Path) -> None: + """A file where the state directory belongs is a real misconfiguration. + + The read is tolerant (it reads as "no id yet"), but the subsequent mkdir + must not be: silently continuing would hand back an id that was never + persisted, so every restart would mint a new one and strand a device + directory in the namespace each time. + """ + blocker = tmp_path / "blocker" + blocker.write_text("not a directory\n", encoding="utf-8") + + with pytest.raises((NotADirectoryError, FileExistsError)): + load_device_identity(tmp_path / "blocker" / "device_id") + + +def test_legacy_id_is_carried_onto_the_identity(tmp_path: Path) -> None: + """The pre-migration role constant travels with the new uuid.""" + identity = load_device_identity(tmp_path / "device_id", legacy_id="pc") + + assert identity.legacy_id == "pc" + + +def test_legacy_id_is_carried_when_loading_an_existing_id(tmp_path: Path) -> None: + """The load branch carries it too, not just the mint branch.""" + path = tmp_path / "device_id" + path.write_text("existing-uuid\n", encoding="utf-8") + + identity = load_device_identity(path, legacy_id="pc") + + assert (identity.device_id, identity.legacy_id) == ("existing-uuid", "pc") + + +def test_own_ids_includes_both_ids_when_migrating() -> None: + """Skip-own must match the old path as well as the new one.""" + identity = DeviceIdentity(device_id="new-uuid", legacy_id="pc") + + assert identity.own_ids == frozenset({"new-uuid", "pc"}) + + +def test_own_ids_is_just_the_uuid_once_the_old_path_is_reclaimed() -> None: + """After GC there is no legacy id left to skip.""" + assert DeviceIdentity(device_id="new-uuid").own_ids == frozenset({"new-uuid"}) + + +def test_is_own_matches_either_id() -> None: + """Both the current and the former id mean "this device".""" + identity = DeviceIdentity(device_id="new-uuid", legacy_id="pc") + + assert identity.is_own("new-uuid") + assert identity.is_own("pc") + assert not identity.is_own("phone") diff --git a/crdt-sync/crdt_sync/tests/test_sync.py b/crdt-sync/crdt_sync/tests/test_sync.py index 45fdbb4..7f6a68c 100644 --- a/crdt-sync/crdt_sync/tests/test_sync.py +++ b/crdt-sync/crdt_sync/tests/test_sync.py @@ -79,6 +79,46 @@ class TestSyncLog: client.get_file_text.assert_called_once_with("devices/phone/log.json") + def test_skips_its_legacy_device_id_as_well_as_its_current_one(self) -> None: + """A migrated device must not re-merge its own pre-migration log. + + Without this the old path looks like a peer, so the device pulls + back everything it pushed under its former id every single tick. + """ + client = _mock_client( + devices=("pc", "new-uuid", "phone"), + files={"devices/phone/log.json": "{}"}, + ) + + sync_log( + client=client, + device_id="new-uuid", + path_prefix="devices", + local_log={}, + encode=_encode, + decode=_decode, + legacy_device_id="pc", + ) + + client.get_file_text.assert_called_once_with("devices/phone/log.json") + + def test_pulls_the_old_path_when_no_legacy_id_is_declared(self) -> None: + """Sanity check the previous test: absent the legacy id, it *is* pulled.""" + client = _mock_client( + devices=("pc", "new-uuid"), files={"devices/pc/log.json": "{}"} + ) + + sync_log( + client=client, + device_id="new-uuid", + path_prefix="devices", + local_log={}, + encode=_encode, + decode=_decode, + ) + + client.get_file_text.assert_called_once_with("devices/pc/log.json") + def test_skips_a_device_with_no_pushed_file_yet(self) -> None: client = _mock_client(devices=("phone",), files={}) diff --git a/crdt-sync/pyproject.toml b/crdt-sync/pyproject.toml index 060ab75..a9184f7 100644 --- a/crdt-sync/pyproject.toml +++ b/crdt-sync/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "crdt-sync" -version = "0.5.1" +version = "0.6.0" description = "Shared CRDT merge scheme + GitHub-Contents-API sync transport for cross-device apps" requires-python = ">=3.10" dependencies = ["requests"] diff --git a/crdt_sync_dart/lib/crdt_sync.dart b/crdt_sync_dart/lib/crdt_sync.dart index b209e36..0c17979 100644 --- a/crdt_sync_dart/lib/crdt_sync.dart +++ b/crdt_sync_dart/lib/crdt_sync.dart @@ -1,6 +1,7 @@ /// Shared CRDT merge scheme + pluggable remote sync transports. library; +export 'src/device.dart'; export 'src/firebase_auth_rest.dart'; export 'src/firebase_client.dart'; export 'src/firebase_config.dart'; diff --git a/crdt_sync_dart/lib/src/device.dart b/crdt_sync_dart/lib/src/device.dart new file mode 100644 index 0000000..c1eedfe --- /dev/null +++ b/crdt_sync_dart/lib/src/device.dart @@ -0,0 +1,51 @@ +/// This device's stable sync identity. +library; + +/// This device's current sync id, plus the id it used to push under before. +/// +/// Every device that pushes into a namespace needs an id that is *its own*: +/// it is the directory segment its log is pushed under +/// (`//`), the revision key, and the node +/// component baked into every HLC stamp it writes (`--`). +/// Two devices sharing an id overwrite each other's pushed file on every +/// tick, so the id must be per-*install*, not per-role. +/// +/// A migration from a role constant (`phone`, `pc`, `desktop`) to a persisted +/// uuid cannot rewrite history: stamps already written keep the old id, and +/// the old path still holds the log pushed under it. So a device that has +/// just switched ids must still recognise its *former* id as itself -- which +/// is what [legacyId] carries, and why [ownIds] is a set. Skip-own-writes has +/// to test membership of that set rather than equality with one id, or the +/// device re-downloads and re-merges its own pre-migration history as though +/// a peer had written it. +class DeviceIdentity { + /// Creates an identity for [deviceId], optionally carrying a [legacyId]. + const DeviceIdentity({required this.deviceId, this.legacyId}); + + /// The id to push, stamp and key revisions under from now on. + final String deviceId; + + /// The id this device pushed under before migrating to a persisted uuid. + /// + /// Null once the old path has been reclaimed. + final String? legacyId; + + /// Every id that means "this device", for skip-own-writes checks. + Set get ownIds => {deviceId, ?legacyId}; + + /// Whether [otherDeviceId] is one of this device's own ids. + bool isOwn(String otherDeviceId) => ownIds.contains(otherDeviceId); + + @override + bool operator ==(Object other) => + other is DeviceIdentity && + other.deviceId == deviceId && + other.legacyId == legacyId; + + @override + int get hashCode => Object.hash(deviceId, legacyId); + + @override + String toString() => + 'DeviceIdentity(deviceId: $deviceId, legacyId: $legacyId)'; +} diff --git a/crdt_sync_dart/lib/src/sync.dart b/crdt_sync_dart/lib/src/sync.dart index bd64b79..1862ab5 100644 --- a/crdt_sync_dart/lib/src/sync.dart +++ b/crdt_sync_dart/lib/src/sync.dart @@ -155,6 +155,12 @@ String revisionOf(String encodedLog) => /// after which those peers would look permanently unchanged and never be /// fetched again. Per-device keys make that failure impossible rather than /// merely avoided. +/// +/// Pass [legacyDeviceId] on a device that has migrated from a fixed role id +/// (`phone`, `pc`, `desktop`) to a persisted uuid: the old path is then +/// treated as this device's own rather than as a peer's, so its +/// pre-migration log is not pulled back and re-merged every tick. Pass null +/// once that path has been reclaimed. Future syncLog({ required RemoteStore client, required String deviceId, @@ -166,6 +172,7 @@ Future syncLog({ String commitMessage = 'crdt_sync: update log', SyncStateStore? stateStore, String? revsPath, + String? legacyDeviceId, }) async { final revs = revsPath ?? defaultRevsPath(pathPrefix); final state = await stateStore?.load() ?? const SyncState(); @@ -173,10 +180,15 @@ Future syncLog({ ? const {} : await _remoteRevs(client, revs); + // A set rather than one id: a device that has migrated from a role + // constant to a persisted uuid still owns the log it pushed under the old + // id. Matching only the current id would pull that file back and re-merge + // this device's own pre-migration history as though a peer wrote it. + final ownIds = {deviceId, ?legacyDeviceId}; var mergedLog = Map.from(localLog); final seenRevs = {}; for (final otherDeviceId in await client.listDirectory(pathPrefix)) { - if (otherDeviceId == deviceId) continue; + if (ownIds.contains(otherDeviceId)) continue; final remoteRev = remoteRevs[otherDeviceId]; if (remoteRev != null && remoteRev == state.peerRevs[otherDeviceId]) { // Unchanged since we last merged it, and that merge is already part of diff --git a/crdt_sync_dart/pubspec.yaml b/crdt_sync_dart/pubspec.yaml index af7a17a..c7225f0 100644 --- a/crdt_sync_dart/pubspec.yaml +++ b/crdt_sync_dart/pubspec.yaml @@ -2,7 +2,7 @@ name: crdt_sync description: >- Shared CRDT merge scheme + GitHub-Contents-API sync transport for cross-device apps. Pure Dart, no Flutter dependency. -version: 0.6.0 +version: 0.7.0 publish_to: 'none' environment: diff --git a/crdt_sync_dart/test/device_test.dart b/crdt_sync_dart/test/device_test.dart new file mode 100644 index 0000000..2925275 --- /dev/null +++ b/crdt_sync_dart/test/device_test.dart @@ -0,0 +1,46 @@ +import 'package:crdt_sync/crdt_sync.dart'; +import 'package:test/test.dart'; + +void main() { + group('DeviceIdentity', () { + test('ownIds includes both ids while migrating', () { + const identity = DeviceIdentity(deviceId: 'new-uuid', legacyId: 'phone'); + + expect(identity.ownIds, {'new-uuid', 'phone'}); + }); + + test('ownIds is just the uuid once the old path is reclaimed', () { + const identity = DeviceIdentity(deviceId: 'new-uuid'); + + expect(identity.ownIds, {'new-uuid'}); + }); + + test('isOwn matches either id, and nothing else', () { + const identity = DeviceIdentity(deviceId: 'new-uuid', legacyId: 'phone'); + + expect(identity.isOwn('new-uuid'), isTrue); + expect(identity.isOwn('phone'), isTrue); + expect(identity.isOwn('pc'), isFalse); + }); + + test('value equality covers both fields', () { + const a = DeviceIdentity(deviceId: 'u', legacyId: 'phone'); + const b = DeviceIdentity(deviceId: 'u', legacyId: 'phone'); + const differentLegacy = DeviceIdentity(deviceId: 'u', legacyId: 'pc'); + const noLegacy = DeviceIdentity(deviceId: 'u'); + + expect(a, equals(b)); + expect(a.hashCode, equals(b.hashCode)); + expect(a, isNot(equals(differentLegacy))); + expect(a, isNot(equals(noLegacy))); + expect(a, isNot(equals(Object()))); + }); + + test('toString names both ids', () { + const identity = DeviceIdentity(deviceId: 'u', legacyId: 'phone'); + + expect(identity.toString(), contains('u')); + expect(identity.toString(), contains('phone')); + }); + }); +} diff --git a/crdt_sync_dart/test/sync_test.dart b/crdt_sync_dart/test/sync_test.dart index 84204d1..ec9dcf2 100644 --- a/crdt_sync_dart/test/sync_test.dart +++ b/crdt_sync_dart/test/sync_test.dart @@ -105,6 +105,58 @@ void main() { expect(putCalls, hasLength(1)); }); + test('skips its legacy device id as well as its current one', () async { + // The old path holds a record this device pushed before migrating. + // Pulling it back would re-merge its own history as a peer's. + final legacyLog = _encode({ + 'stale': Record(id: 'stale', fields: {'text': ('old', _make(1))}), + }); + final (:client, :putCalls) = _client( + contentResponses: { + 'devices': _directoryOf(['pc', 'new-uuid']), + 'devices/pc/log.json': _fileContaining(legacyLog), + }, + ); + + final merged = await syncLog( + client: client, + deviceId: 'new-uuid', + pathPrefix: 'devices', + localLog: {}, + encode: _encode, + decode: _decode, + legacyDeviceId: 'pc', + ); + + expect(merged, isEmpty); + expect(putCalls.single.path, 'devices/new-uuid/log.json'); + }); + + test('pulls the old path when no legacy id is declared', () async { + // Negative control for the test above: absent legacyDeviceId, the old + // path really is treated as a peer. + final legacyLog = _encode({ + 'stale': Record(id: 'stale', fields: {'text': ('old', _make(1))}), + }); + final (:client, :putCalls) = _client( + contentResponses: { + 'devices': _directoryOf(['pc', 'new-uuid']), + 'devices/pc/log.json': _fileContaining(legacyLog), + }, + ); + + final merged = await syncLog( + client: client, + deviceId: 'new-uuid', + pathPrefix: 'devices', + localLog: {}, + encode: _encode, + decode: _decode, + ); + + expect(merged.keys, contains('stale')); + }); + test('skips a device with no pushed file yet', () async { final (:client, :putCalls) = _client( contentResponses: {