From c0e3b64fb3dbaad7ebe803c09fd31b2ad578ca31 Mon Sep 17 00:00:00 2001 From: Sashank Bhamidi Date: Tue, 20 Oct 2026 16:16:09 +0530 Subject: [PATCH 1/3] chore: add the changelog fragment for pushes and conflict resolution --- news/9.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/9.feature.rst diff --git a/news/9.feature.rst b/news/9.feature.rst new file mode 100644 index 0000000..cdcefc3 --- /dev/null +++ b/news/9.feature.rst @@ -0,0 +1 @@ +Added pushes and conflict resolution. A ``Session`` lists the edits ready to push, claims each one so no other process sends it, and records how the push ended; the next complete ingest settles a push whose answer never arrived. ``Replica.resolve`` and ``Replica.resolve_conflicts`` resolve conflicts with your edit, the server's side, merged content, or a delete. From 73cfb13a8ad120afb126787d7f5a2809c6cb8938 Mon Sep 17 00:00:00 2001 From: Sashank Bhamidi Date: Wed, 21 Oct 2026 01:10:09 +0530 Subject: [PATCH 2/3] feat: add pushes and conflict resolution --- docs/contribute.rst | 4 +- docs/explanation/design.rst | 14 +- docs/reference/glossary.rst | 5 +- docs/reference/objects.rst | 14 +- docs/reference/replica.rst | 8 +- src/calendaring_sync/__init__.py | 34 + src/calendaring_sync/_classify.py | 31 +- src/calendaring_sync/_text.py | 20 + src/calendaring_sync/error.py | 26 +- src/calendaring_sync/objects/_data.py | 29 + src/calendaring_sync/objects/change.py | 10 +- src/calendaring_sync/objects/conflict.py | 19 +- src/calendaring_sync/objects/feed.py | 6 + src/calendaring_sync/objects/push.py | 86 ++ src/calendaring_sync/objects/record.py | 2 +- src/calendaring_sync/objects/resolution.py | 83 ++ src/calendaring_sync/replica.py | 813 +++++++++++++++-- src/calendaring_sync/store/base.py | 57 +- src/calendaring_sync/store/memory.py | 10 +- src/calendaring_sync/store/sqlite.py | 16 +- src/calendaring_sync/tests/benchmark.py | 69 +- .../tests/test_classify_unit.py | 24 +- src/calendaring_sync/tests/test_sync_unit.py | 860 ++++++++++++++++-- 23 files changed, 1973 insertions(+), 267 deletions(-) create mode 100644 src/calendaring_sync/objects/_data.py create mode 100644 src/calendaring_sync/objects/push.py create mode 100644 src/calendaring_sync/objects/resolution.py diff --git a/docs/contribute.rst b/docs/contribute.rst index 41f03dd..e511ce8 100644 --- a/docs/contribute.rst +++ b/docs/contribute.rst @@ -33,13 +33,13 @@ Tests live inside the package, in ``src/calendaring_sync/tests/``. ``test_classi Checking performance ==================== -If your change touches ingesting, local edits or a store, run the benchmarks on ``main`` and on your branch, on the same machine, and compare: +If your change touches ingesting, local edits, pushes or a store, run the benchmarks on ``main`` and on your branch, on the same machine, and compare: .. code-block:: bash python -m calendaring_sync.tests.benchmark -It times a full listing of a large scope, a second listing over local edits and conflicts, and a run of single edits, each on a fresh SQLite store. Times differ between machines, so only the comparison means anything. Explain any slowdown beyond run-to-run noise in your pull request. ``--size`` and ``--runs`` change the scope's size and the number of runs. +It times a full listing of a large scope, a second listing over local edits and conflicts, a run of single edits, and pushes of those edits, each on a fresh SQLite store. Times differ between machines, so only the comparison means anything. Explain any slowdown beyond run-to-run noise in your pull request. ``--size`` and ``--runs`` change the scope's size and the number of runs. Previewing docs =============== diff --git a/docs/explanation/design.rst b/docs/explanation/design.rst index 2b4e7a2..6dc04de 100644 --- a/docs/explanation/design.rst +++ b/docs/explanation/design.rst @@ -122,7 +122,7 @@ A session is ``ingesting`` while pages remain and ``ingested`` once the token is note right of ingesting: no pushes while pages remain note right of pushing: no ingests until it ends -Before a push goes out, the core marks it :term:`in flight` in the store, in one step that fails if another process got there first, so two processes can't send the same edit. Each push has one of five outcomes: +Before a push goes out, the adapter :term:`claims ` it: the core marks it :term:`in flight` in the store, in one step that fails if another process got there first, so two processes can't send the same edit. Each push has one of five outcomes: - applied: the edit becomes the synced content - rejected: the server refused the write because something changed since your last sync, and the edit stays pending @@ -130,14 +130,14 @@ Before a push goes out, the core marks it :term:`in flight` in the store, in one - refused because another object holds its :term:`logical key`, which becomes a conflict - unknown: the request may or may not have reached the server -If the process stops while a push is in flight, the push is unknown the next time you open the store. The next complete :term:`ingest` settles each unknown push: +An unknown push stays in flight, and so does one whose process stopped before the answer arrived. The next complete :term:`ingest` of its scope, in any process, settles it: - a create that comes back under the id it chose reached the server, and the core stores the server's copy; where the server picks the id, a create that comes back with the logical key it was sent with counts the same, but only if the adapter says that key is unique - an update that comes back with your fingerprint reached the server, and one that comes back with another fingerprint is a conflict, since someone else changed the object too - a delete that comes back removed reached the server - a push the change set doesn't mention didn't reach the server, and its edit is pending again -Treating an unknown push as failed could create the same object twice. +Treating an unknown push as failed could create the same object twice. An outcome that arrives after an ingest settled its push still counts while the record holds the same edit, and changes nothing once the edit has changed or the server's side decided it. Two processes syncing one scope at the same moment can therefore send one edit twice: the second write is rejected where the adapter guards it with the object's version, and nothing is lost either way. Why the server's answer to a write isn't trusted ================================================ @@ -191,7 +191,7 @@ The table lists every transition with its conditions, including the ones the dia - Pending * - pending - Edit back to the synced content, or discarded - - Synced + - Synced; a create the server never had is deleted from the store instead * - pending - Remote change or removal ingested - Conflict, unless both sides arrived at the same content or both deleted @@ -218,7 +218,7 @@ The table lists every transition with its conditions, including the ones the dia - Conflict with the new remote side; synced if it now equals your edit, pending again if the server is back at the synced content, or removed if both sides deleted the object * - conflict - Resolved - - Synced, pending, or removed, depending on the resolution + - Synced, pending, or removed, depending on the resolution; a create the server refused is deleted from the store when you discard it or push it as an update to the object holding its logical key * - removed - Remote change: the server holds an object at that id again - Synced with the new content, as the same record @@ -268,9 +268,9 @@ Each :term:`conflict record` has one of five kinds: Later remote changes update the record's remote side. The conflict resolves itself when the server reaches your edit, goes back to the synced content, or removes an object you deleted too. -The core stores conflicts instead of raising them: an exception for one object would abort every other change in the sync, and a :term:`resolver` called during a sync would run again on every retry. You resolve a record with your edit, the server's version, merged content, or a delete, and the core pushes the result as a change to the server's current version. You can't push a create the server refused for its UID again unchanged, so you can discard it, push it as an update to the object that holds the UID, or replace it with merged content, possibly with a new UID. +The core stores conflicts instead of raising them: an exception for one object would abort every other change in the sync, and a :term:`resolver` called during a sync would run again on every retry. You resolve a record with your edit (``KeepEdit``), the server's version (``TakeServer``), merged content (``Merge``), or a delete (``Delete``), and the core pushes the result as a change to the server's current version. You can't push a create the server refused for its UID again unchanged, so you can discard it (``TakeServer`` or ``Delete``), push it as an update to the object that holds the UID (``UpdateHolder``), or replace it with merged content, possibly with a new UID (``Merge``). -``resolve_conflicts(resolver)`` passes each open record to your resolver, outside any sync. Each resolution is its own transaction: ``None`` leaves a record open, and an exception stops the loop and keeps the resolutions before it. +``resolve(record, resolution)`` resolves one record you read, and refuses if its conflict changed since, so a decision is never applied to news you haven't seen. ``resolve_conflicts(resolver)`` passes each open record to your resolver, outside any sync, and resolves each answer the same way. Each resolution is its own transaction: ``None`` leaves a record open, a record that changed while your resolver ran waits for the next call, and an exception stops the loop and keeps the resolutions before it. A removal on the server against a local edit is always a conflict, never resolved automatically, because that would delete your edit without asking. diff --git a/docs/reference/glossary.rst b/docs/reference/glossary.rst index 4edf003..1fd9d2f 100644 --- a/docs/reference/glossary.rst +++ b/docs/reference/glossary.rst @@ -25,6 +25,9 @@ These are the sync terms calendaring-sync's pages use. :doc:`../explanation/desi checkpoint The sync token a :term:`truncated page` ends at. The next request continues from it. + claim + To mark a :term:`pending edit` :term:`in flight` just before pushing it, in one step that fails if another process got there first. + conflict An object where your :term:`pending edit` and the server disagree: both sides changed it, one side deleted it while the other edited it, or the server refused your create because another object holds its :term:`UID`. calendaring-sync reports it and leaves the decision to you. @@ -56,7 +59,7 @@ These are the sync terms calendaring-sync's pages use. :doc:`../explanation/desi A :term:`change set` that lists every object in a scope, not only the ones that changed. in flight - The state of a :term:`push` the core has marked as sent, before the adapter reports how it ended. + The state of a :term:`push` that was :term:`claimed `, until the adapter reports how it ended or the next complete :term:`ingest` settles it. incremental change set A :term:`change set` that holds only what changed since the scope's :term:`sync token`. diff --git a/docs/reference/objects.rst b/docs/reference/objects.rst index 176ab83..1c911ab 100644 --- a/docs/reference/objects.rst +++ b/docs/reference/objects.rst @@ -1,13 +1,13 @@ .. SPDX-FileCopyrightText: 2026 calendaring-sync contributors .. SPDX-License-Identifier: AGPL-3.0-or-later -:mod:`objects`: Change sets, records, edits, conflicts, scopes, and feed entries -================================================================================ +:mod:`objects`: Change sets, records, edits, conflicts, push outcomes, resolutions, scopes, and feed entries +============================================================================================================ .. meta:: - :description: The data objects calendaring-sync takes and returns: change sets and their changes, records with their local edits and conflicts, scopes, feed entries, and dropped scopes. + :description: The data objects calendaring-sync takes and returns: change sets and their changes, records with their local edits and conflicts, push outcomes, resolutions, scopes, feed entries, and dropped scopes. -The data an :term:`adapter` hands the core, and what the core returns. +The data an :term:`adapter` hands the core, such as change sets and how each :term:`push` ended, the resolutions your application chooses for :term:`conflicts `, and what the core returns. .. automodule:: calendaring_sync.objects.change_set :members: @@ -24,6 +24,12 @@ The data an :term:`adapter` hands the core, and what the core returns. .. automodule:: calendaring_sync.objects.conflict :members: +.. automodule:: calendaring_sync.objects.push + :members: + +.. automodule:: calendaring_sync.objects.resolution + :members: + .. automodule:: calendaring_sync.objects.scope :members: diff --git a/docs/reference/replica.rst b/docs/reference/replica.rst index ef83857..194b2d9 100644 --- a/docs/reference/replica.rst +++ b/docs/reference/replica.rst @@ -1,13 +1,13 @@ .. SPDX-FileCopyrightText: 2026 calendaring-sync contributors .. SPDX-License-Identifier: AGPL-3.0-or-later -:mod:`replica`: Sessions, ingesting, local edits, and the change feed -===================================================================== +:mod:`replica`: Sessions, ingesting, local edits, pushing, resolving conflicts, and the change feed +=================================================================================================== .. meta:: - :description: Replica and Session, where calendaring-sync applies change sets, records local edits, stores conflicts, and serves the change feed. + :description: Replica and Session, where calendaring-sync applies change sets, records local edits, pushes them, stores and resolves conflicts, and serves the change feed. -:class:`~calendaring_sync.replica.Replica` is the entry point. Record your :term:`local edits ` as you make them. To sync, open a :term:`session` for a :term:`scope` and :term:`ingest` the :term:`change sets ` an :term:`adapter` fetched. Then read the :term:`change feed` to learn what changed and which :term:`conflicts ` came up. +:class:`~calendaring_sync.replica.Replica` is the entry point. Record your :term:`local edits ` as you make them. To sync, open a :term:`session` for a :term:`scope` and :term:`ingest` the :term:`change sets ` an :term:`adapter` fetched. After the last page, the adapter pushes your edits in the same session: it :term:`claims ` each one, sends it, and settles it with how the :term:`push` ended. Then read the :term:`change feed` to learn what changed and which :term:`conflicts ` came up, and resolve them with :meth:`~.Replica.resolve` or :meth:`~.Replica.resolve_conflicts`. .. automodule:: calendaring_sync.replica :members: diff --git a/src/calendaring_sync/__init__.py b/src/calendaring_sync/__init__.py index cf273ba..42151c3 100644 --- a/src/calendaring_sync/__init__.py +++ b/src/calendaring_sync/__init__.py @@ -15,11 +15,13 @@ DuplicateObjectError, EditRefusedError, OpenConflictError, + PushInFlightError, RecordValueError, RemovedRecordError, ScopeMismatchError, SessionError, StaleChangeSetError, + StaleConflictError, StoreBusyError, StoreError, StoreVersionError, @@ -28,6 +30,7 @@ TransactionRequiredError, TrimmedFeedError, UnexpectedStoreError, + UnknownHolderError, UnknownRecordError, UnknownScopeError, ) @@ -37,7 +40,23 @@ from calendaring_sync.objects.dropped_scope import DroppedScope from calendaring_sync.objects.edit import EditKind, LocalEdit from calendaring_sync.objects.feed import FeedEntry, FeedEntryKind +from calendaring_sync.objects.push import ( + Applied, + Gone, + LogicalKeyTaken, + PushOutcome, + Rejected, + Unknown, +) from calendaring_sync.objects.record import Record, RecordState +from calendaring_sync.objects.resolution import ( + Delete, + KeepEdit, + Merge, + Resolution, + TakeServer, + UpdateHolder, +) from calendaring_sync.objects.scope import Scope from calendaring_sync.replica import Replica, Session, SessionState from calendaring_sync.store.base import StateStore @@ -45,27 +64,37 @@ from calendaring_sync.store.sqlite import SQLiteStore __all__ = [ + "Applied", "CalendaringSyncError", "Change", "ChangeSet", "Changed", "Conflict", "ConflictKind", + "Delete", "DroppedScope", "DuplicateObjectError", "EditKind", "EditRefusedError", "FeedEntry", "FeedEntryKind", + "Gone", + "KeepEdit", "LocalEdit", + "LogicalKeyTaken", "MemoryStore", + "Merge", "OpenConflictError", + "PushInFlightError", + "PushOutcome", "Record", "RecordState", "RecordValueError", + "Rejected", "Removed", "RemovedRecordError", "Replica", + "Resolution", "SQLiteStore", "Scope", "ScopeMismatchError", @@ -73,17 +102,22 @@ "SessionError", "SessionState", "StaleChangeSetError", + "StaleConflictError", "StateStore", "StoreBusyError", "StoreError", "StoreVersionError", + "TakeServer", "TransactionError", "TransactionPermissionError", "TransactionRequiredError", "TrimmedFeedError", "Unchanged", "UnexpectedStoreError", + "Unknown", + "UnknownHolderError", "UnknownRecordError", "UnknownScopeError", + "UpdateHolder", "__version__", ] diff --git a/src/calendaring_sync/_classify.py b/src/calendaring_sync/_classify.py index 070fdf7..25c8458 100644 --- a/src/calendaring_sync/_classify.py +++ b/src/calendaring_sync/_classify.py @@ -7,15 +7,12 @@ from enum import StrEnum +from calendaring_sync.objects._data import REMOTE, data from calendaring_sync.objects.change import Changed, Removed from calendaring_sync.objects.conflict import Conflict, ConflictKind from calendaring_sync.objects.edit import EditKind, LocalEdit from calendaring_sync.objects.record import Record -#: What "the same data" compares, in every copy of an object. A conflict -#: holds the server's copy under the same names with ``remote_`` in front. -_DATA_FIELDS = ("fingerprint", "content", "containers", "logical_key") - #: A copy of an object: the synced copy, a server's change, or a local edit. Copy = Record | Changed | LocalEdit @@ -39,10 +36,6 @@ class ChangeClass(StrEnum): CONFLICT = "conflict" -def _data(copy: object, prefix: str = "") -> tuple[object, ...]: - return tuple(getattr(copy, prefix + name) for name in _DATA_FIELDS) - - def _same(a: tuple[object, ...], b: tuple[object, ...]) -> bool: """Whether two copies' data match. @@ -59,18 +52,29 @@ def _same(a: tuple[object, ...], b: tuple[object, ...]) -> bool: def same_data(a: Copy, b: Copy) -> bool: """Whether two copies of an object hold the same data.""" - return _same(_data(a), _data(b)) + return _same(data(a), data(b)) def same_remote(conflict: Conflict, change: Changed | Removed) -> bool: """Whether the server reports again what an open conflict already holds.""" if conflict.remote_content is None or isinstance(change, Removed): return conflict.remote_content is None and isinstance(change, Removed) - return _same(_data(conflict, "remote_"), _data(change)) + return _same(data(conflict, REMOTE), data(change)) + + +def same_conflict(a: Conflict, b: Conflict) -> bool: + """Whether two copies of a conflict say the same thing, whatever the server's version.""" + if (a.kind, a.holder) != (b.kind, b.holder): + return False + return _same(data(a, REMOTE), data(b, REMOTE)) def classify( - synced: Record | None, remote: Changed | Removed | None, local: LocalEdit | None + synced: Record | None, + remote: Changed | Removed | None, + local: LocalEdit | None, + *, + in_flight: bool = False, ) -> tuple[ChangeClass, ConflictKind | None]: """Classify one object from its synced side, the server's report and the local edit. @@ -85,6 +89,9 @@ def classify( remote: The server's new copy, ``Removed`` if the server removed it, or ``None`` if the server reported nothing new. local: The record's :term:`pending edit`, or ``None``. + in_flight: Whether ``local`` was pushed and the push's outcome is + unknown. A create that then comes back at its own id is your + own write, whatever the server made of its content. Returns: The class, and the conflict's kind when the class is ``CONFLICT``. @@ -125,6 +132,8 @@ def classify( case EditKind.DELETE, Removed(): # Both sides deleted the object. return ChangeClass.DELETED_REMOTE, None + case EditKind.CREATE, Changed() if in_flight: + return ChangeClass.UNCHANGED, None case EditKind.CREATE, Changed(): return ChangeClass.CONFLICT, ConflictKind.BOTH_CREATED case _: diff --git a/src/calendaring_sync/_text.py b/src/calendaring_sync/_text.py index b210e77..90e55e9 100644 --- a/src/calendaring_sync/_text.py +++ b/src/calendaring_sync/_text.py @@ -44,3 +44,23 @@ def check_content(content: object) -> None: """ if not isinstance(content, bytes): raise TypeError(f"content must be bytes, not {type(content).__name__}") + + +def check_data( + content: object, + fingerprint: str | None, + containers: frozenset[str], + logical_key: str | None, + **values: str | None, +) -> None: + """Check a copy of an object: its content with :func:`check_content`, and its strings. + + Args: + content: The copy's content. + fingerprint: Its fingerprint. + containers: Its containers. + logical_key: Its logical key. + values: Any other strings that come with it, by name. + """ + check_content(content) + check_text(fingerprint=fingerprint, containers=containers, logical_key=logical_key, **values) diff --git a/src/calendaring_sync/error.py b/src/calendaring_sync/error.py index 1577a10..585a05e 100644 --- a/src/calendaring_sync/error.py +++ b/src/calendaring_sync/error.py @@ -59,11 +59,15 @@ class RecordValueError(CalendaringSyncError, ValueError): class EditRefusedError(CalendaringSyncError): """The :term:`record` is in a state that takes no :term:`local edit`. - Raised as itself for a record whose :term:`push` is :term:`in flight`. - Its subclasses cover a record in :term:`conflict` and a removed one. + Its subclasses say which: a :term:`push` :term:`in flight`, an open + :term:`conflict`, or a record the server removed. """ +class PushInFlightError(EditRefusedError): + """The :term:`record`'s :term:`push` is :term:`in flight`; edit it once the push has ended.""" + + class OpenConflictError(EditRefusedError): """The :term:`record` holds an open :term:`conflict`; resolve it before editing.""" @@ -72,6 +76,24 @@ class RemovedRecordError(EditRefusedError): """The :term:`record` was removed from the server, so there's nothing to edit.""" +class StaleConflictError(CalendaringSyncError): + """The :term:`conflict` changed since your application read it, so a resolution isn't applied. + + The record ended, or its edit or the server's side moved on. Read it + again and decide on what it holds now. + """ + + +class UnknownHolderError(CalendaringSyncError): + """No single record is known to hold what a refused create was refused for. + + The server named no record that's still there, and none or several hold + the create's :term:`logical key`, or it was sent without one. So the + create can't be pushed as a change to one of them. Resolve it another + way, or sync first. + """ + + class ScopeMismatchError(CalendaringSyncError): """A :term:`session` was opened with a media type the :term:`scope` doesn't have. diff --git a/src/calendaring_sync/objects/_data.py b/src/calendaring_sync/objects/_data.py new file mode 100644 index 0000000..b3b278d --- /dev/null +++ b/src/calendaring_sync/objects/_data.py @@ -0,0 +1,29 @@ +# SPDX-FileCopyrightText: 2026 calendaring-sync contributors +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""An object's data: what every copy of it holds, whoever made the copy.""" + +from __future__ import annotations + +#: The fields that hold an object's data, in every copy of it: a record's +#: synced side, a change from the server, a local edit, and a conflict's +#: server side. Two copies hold the same data when these match, and a record +#: synced to a copy takes these from it. +DATA_FIELDS = ("fingerprint", "content", "containers", "logical_key") + +#: What a conflict puts in front of each field name for the server's side. +REMOTE = "remote_" + +#: A copy's data: its fingerprint, content, containers and logical key. +Data = tuple[str | None, bytes | None, frozenset[str], str | None] + +#: The data of a copy that isn't there, such as an object the server removed. +NO_DATA: Data = (None, None, frozenset(), None) + + +def data(copy: object, prefix: str = "") -> Data: + """Return a copy's data, one value for each name in :data:`DATA_FIELDS`.""" + fingerprint, content, containers, logical_key = ( + getattr(copy, prefix + name) for name in DATA_FIELDS + ) + return fingerprint, content, containers, logical_key diff --git a/src/calendaring_sync/objects/change.py b/src/calendaring_sync/objects/change.py index d57f22a..1ec7b0c 100644 --- a/src/calendaring_sync/objects/change.py +++ b/src/calendaring_sync/objects/change.py @@ -7,7 +7,7 @@ from dataclasses import dataclass -from calendaring_sync._text import check_content, check_text +from calendaring_sync._text import check_data @dataclass(frozen=True) @@ -46,12 +46,8 @@ class Changed: logical_key: str | None = None def __post_init__(self) -> None: - check_content(self.content) - check_text( - version=self.version, - fingerprint=self.fingerprint, - containers=self.containers, - logical_key=self.logical_key, + check_data( + self.content, self.fingerprint, self.containers, self.logical_key, version=self.version ) diff --git a/src/calendaring_sync/objects/conflict.py b/src/calendaring_sync/objects/conflict.py index 8fc21df..46fe863 100644 --- a/src/calendaring_sync/objects/conflict.py +++ b/src/calendaring_sync/objects/conflict.py @@ -1,7 +1,7 @@ # SPDX-FileCopyrightText: 2026 calendaring-sync contributors # SPDX-License-Identifier: AGPL-3.0-or-later -"""The server's side of a :term:`conflict`, kept on its :term:`record`.""" +"""The server's side of a :term:`conflict`, kept on its :term:`record` until it's resolved.""" from __future__ import annotations @@ -10,6 +10,7 @@ from enum import StrEnum from typing import Self +from calendaring_sync.objects._data import NO_DATA, data from calendaring_sync.objects.change import Changed, Removed @@ -59,6 +60,9 @@ class Conflict: remote_logical_key: The :term:`logical key` of the server's copy, if it has one. opened_at: When the conflict was first stored. + holder: For a create refused because another object holds its + :term:`logical key`, that object's :term:`object id` when the + server named it; ``None`` otherwise. """ kind: ConflictKind @@ -68,6 +72,7 @@ class Conflict: remote_containers: frozenset[str] remote_logical_key: str | None opened_at: datetime + holder: str | None = None @classmethod def from_change( @@ -75,13 +80,5 @@ def from_change( ) -> Self: """Return a conflict whose server side is what a change reported.""" if isinstance(change, Removed): - return cls(kind, None, None, None, frozenset(), None, opened_at) - return cls( - kind, - change.version, - change.fingerprint, - change.content, - change.containers, - change.logical_key, - opened_at, - ) + return cls(kind, None, *NO_DATA, opened_at) + return cls(kind, change.version, *data(change), opened_at) diff --git a/src/calendaring_sync/objects/feed.py b/src/calendaring_sync/objects/feed.py index 9d04474..a178355 100644 --- a/src/calendaring_sync/objects/feed.py +++ b/src/calendaring_sync/objects/feed.py @@ -26,6 +26,12 @@ class FeedEntryKind(StrEnum): CONFLICT_UPDATED = "conflict_updated" #: An open conflict ended, and the record holds what it ended with. CONFLICT_RESOLVED = "conflict_resolved" + #: A :term:`push` reached the server: the record holds your edit as + #: synced, or is removed for a delete. + PUSHED = "pushed" + #: A :term:`push` didn't go through, and the edit is pending again for + #: a later sync. + PUSH_FAILED = "push_failed" @dataclass(frozen=True) diff --git a/src/calendaring_sync/objects/push.py b/src/calendaring_sync/objects/push.py new file mode 100644 index 0000000..94479a2 --- /dev/null +++ b/src/calendaring_sync/objects/push.py @@ -0,0 +1,86 @@ +# SPDX-FileCopyrightText: 2026 calendaring-sync contributors +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""How a :term:`push` ended, as the :term:`adapter` reports it.""" + +from __future__ import annotations + +from dataclasses import dataclass + +from calendaring_sync._text import check_text + + +@dataclass(frozen=True) +class Applied: + """The server applied the edit. + + Attributes: + object_id: The :term:`object id` the server gave the object, for a + create whose id the server picks, or when the server moved the + object to another id. ``None`` keeps the record's id. + + Raises: + ValueError: If the id holds a :ref:`lone surrogate `. + + Examples: + An :term:`adapter` reports a create the server stored under an id it + picked: + + >>> from calendaring_sync import Applied + >>> Applied("event-17").object_id + 'event-17' + """ + + object_id: str | None = None + + def __post_init__(self) -> None: + check_text(object_id=self.object_id) + + +@dataclass(frozen=True) +class Rejected: + """The server refused the write because something changed since your last sync. + + The edit stays pending. The next :term:`ingest` makes it a + :term:`conflict` only if this object changed, and otherwise it's pushed + again. + """ + + +@dataclass(frozen=True) +class Gone: + """The object no longer exists on the server. + + A delete is then done; a change becomes a :term:`conflict`. + """ + + +@dataclass(frozen=True) +class LogicalKeyTaken: + """The server refused a create because another object holds its :term:`logical key`. + + Attributes: + holder: The :term:`object id` of the object that holds the key, when + the server names it; ``None`` when it doesn't. + + Raises: + ValueError: If the id holds a :ref:`lone surrogate `. + """ + + holder: str | None = None + + def __post_init__(self) -> None: + check_text(holder=self.holder) + + +@dataclass(frozen=True) +class Unknown: + """The request may or may not have reached the server. + + The push stays :term:`in flight`, and the next complete :term:`ingest` + settles it from what the server then holds. + """ + + +#: Any one of the five. +PushOutcome = Applied | Rejected | Gone | LogicalKeyTaken | Unknown diff --git a/src/calendaring_sync/objects/record.py b/src/calendaring_sync/objects/record.py index e43fdd6..f40ebf0 100644 --- a/src/calendaring_sync/objects/record.py +++ b/src/calendaring_sync/objects/record.py @@ -20,7 +20,7 @@ class RecordState(StrEnum): SYNCED = "synced" #: Holds a :term:`pending edit`. PENDING = "pending" - #: Its :term:`push` was sent, and how it ended isn't known yet. + #: Its :term:`push` was :term:`claimed `, and how it ended isn't known yet. IN_FLIGHT = "in_flight" #: Holds an open :term:`conflict`. CONFLICT = "conflict" diff --git a/src/calendaring_sync/objects/resolution.py b/src/calendaring_sync/objects/resolution.py new file mode 100644 index 0000000..c394fbd --- /dev/null +++ b/src/calendaring_sync/objects/resolution.py @@ -0,0 +1,83 @@ +# SPDX-FileCopyrightText: 2026 calendaring-sync contributors +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""How your application resolves a :term:`conflict`.""" + +from __future__ import annotations + +from dataclasses import KW_ONLY, dataclass + +from calendaring_sync._text import check_data + + +@dataclass(frozen=True) +class TakeServer: + """Keep the server's side and drop your edit. + + For a create the server refused because another object holds its + :term:`logical key`, the server has nothing of yours, so the create is + discarded. + """ + + +@dataclass(frozen=True) +class KeepEdit: + """Push your edit as it is, over the server's current copy. + + Where the server removed the object, a change is pushed as a create. + """ + + +@dataclass(frozen=True) +class Merge: + """Push new content you made from both sides, over the server's current copy. + + Build ``fingerprint``, ``containers`` and ``logical_key`` with the + :term:`adapter`'s function for them, as for + :meth:`~calendaring_sync.replica.Replica.change`. + + Attributes: + content: The merged :term:`content`. + fingerprint: Its :term:`fingerprint`. + containers: The :term:`containers ` it belongs to. + logical_key: Its :term:`logical key`, if it has one. + + Raises: + TypeError: If the content isn't bytes. + ValueError: If a string holds a :ref:`lone surrogate `. + + Examples: + Your application keeps what both sides changed in one copy: + + >>> from calendaring_sync import Merge + >>> merged = Merge(b"Team meeting, room 3, moved", fingerprint="fp-merged") + >>> merged.containers, merged.logical_key + (frozenset(), None) + """ + + content: bytes + _: KW_ONLY + fingerprint: str + containers: frozenset[str] = frozenset() + logical_key: str | None = None + + def __post_init__(self) -> None: + check_data(self.content, self.fingerprint, self.containers, self.logical_key) + + +@dataclass(frozen=True) +class Delete: + """Delete the object, whatever the server's side holds.""" + + +@dataclass(frozen=True) +class UpdateHolder: + """Push a create the server refused as a change to the object that holds its logical key. + + Only for a create refused because another object holds its + :term:`logical key`. + """ + + +#: Any one of the five. +Resolution = TakeServer | KeepEdit | Merge | Delete | UpdateHolder diff --git a/src/calendaring_sync/replica.py b/src/calendaring_sync/replica.py index 4b1a802..d60e43a 100644 --- a/src/calendaring_sync/replica.py +++ b/src/calendaring_sync/replica.py @@ -1,7 +1,7 @@ # SPDX-FileCopyrightText: 2026 calendaring-sync contributors # SPDX-License-Identifier: AGPL-3.0-or-later -"""The core's entry point: sessions, ingests, local edits, and the :term:`change feed`. +"""The core's entry point: sessions, pushes, local edits, conflicts, and the :term:`change feed`. Every sync rule lives here, on top of any :class:`~calendaring_sync.store.base.StateStore`. @@ -15,34 +15,65 @@ from datetime import UTC, datetime from enum import StrEnum from threading import Lock -from typing import TypeVar - -from calendaring_sync._classify import ChangeClass, classify, same_data, same_remote -from calendaring_sync._text import check_content, check_text +from typing import TypeVar, cast + +from calendaring_sync._classify import ( + ChangeClass, + classify, + same_conflict, + same_data, + same_remote, +) +from calendaring_sync._text import check_data, check_text from calendaring_sync.constants import FEED_PAGE_SIZE from calendaring_sync.error import ( - EditRefusedError, + DuplicateObjectError, OpenConflictError, + PushInFlightError, RemovedRecordError, ScopeMismatchError, SessionError, StaleChangeSetError, + StaleConflictError, TrimmedFeedError, + UnknownHolderError, UnknownRecordError, UnknownScopeError, ) +from calendaring_sync.objects._data import REMOTE, data from calendaring_sync.objects.change import Change, Changed, Removed, Unchanged from calendaring_sync.objects.change_set import ChangeSet -from calendaring_sync.objects.conflict import Conflict +from calendaring_sync.objects.conflict import Conflict, ConflictKind from calendaring_sync.objects.dropped_scope import DroppedScope from calendaring_sync.objects.edit import EditKind, LocalEdit from calendaring_sync.objects.feed import FeedEntry, FeedEntryKind +from calendaring_sync.objects.push import ( + Applied, + Gone, + LogicalKeyTaken, + PushOutcome, + Rejected, + Unknown, +) from calendaring_sync.objects.record import Record, RecordState +from calendaring_sync.objects.resolution import ( + Delete, + KeepEdit, + Merge, + Resolution, + TakeServer, + UpdateHolder, +) from calendaring_sync.objects.scope import Scope from calendaring_sync.store.base import StateStore _T = TypeVar("_T") +# The record states the core reads one at a time. +_PENDING = frozenset({RecordState.PENDING}) +_IN_FLIGHT = frozenset({RecordState.IN_FLIGHT}) +_CONFLICT = frozenset({RecordState.CONFLICT}) + def _utc_now() -> datetime: return datetime.now(UTC) @@ -62,6 +93,153 @@ def _without_fingerprints(record: Record) -> Record: ) +def _new_record(scope_id: str, object_id: str | None) -> Record: + """A record the store doesn't hold yet, with nothing synced; the caller fills it in.""" + return Record( + local_key=0, + scope=scope_id, + object_id=object_id, + state=RecordState.SYNCED, + version=None, + fingerprint=None, + content=None, + containers=frozenset(), + logical_key=None, + synced_at=None, + last_entry=0, + ) + + +def _with_copy( + record: Record, copy: object, version: str | None, now: datetime, *, prefix: str = "" +) -> Record: + """The record whose synced side is a copy of its object, at ``version``. + + ``copy`` is a change from the server, your edit once it's applied, or a + conflict's server side, whose fields carry the :data:`REMOTE` prefix. + """ + fingerprint, content, containers, logical_key = data(copy, prefix) + return replace( + record, + version=version, + fingerprint=fingerprint, + content=content, + containers=containers, + logical_key=logical_key, + synced_at=now, + ) + + +def _synced( + record: Record, copy: object, version: str | None, now: datetime, *, prefix: str = "" +) -> Record: + """The record synced to a copy of its object, with no edit or conflict left.""" + return replace( + _with_copy(record, copy, version, now, prefix=prefix), + state=RecordState.SYNCED, + edit=None, + conflict=None, + ) + + +def _removed(record: Record, now: datetime) -> Record: + """The record once its object is gone from the server, with nothing left to push.""" + return replace(record, state=RecordState.REMOVED, edit=None, conflict=None, synced_at=now) + + +def _check_resolution(record: Record, resolution: Resolution) -> None: + """Raise ``ValueError`` unless ``record`` is in conflict and ``resolution`` fits its kind.""" + if record.state is not RecordState.CONFLICT or record.conflict is None: + raise ValueError("resolve() takes a record in conflict") + refused_create = record.conflict.kind is ConflictKind.LOGICAL_KEY_TAKEN + if isinstance(resolution, KeepEdit) and refused_create: + raise ValueError("A create refused for its logical key can't be pushed again unchanged") + if isinstance(resolution, UpdateHolder) and not refused_create: + raise ValueError("Only a create refused for its logical key goes to the key's holder") + + +def _same_decision(stored: Record | None, record: Record) -> bool: + """Whether ``stored`` still holds the conflict a decision was made on from ``record``.""" + return ( + stored is not None + and stored.conflict is not None + and record.conflict is not None + and stored.edit == record.edit + and same_conflict(stored.conflict, record.conflict) + ) + + +def _resolved(record: Record, resolution: Resolution, now: datetime) -> Record | None: + """The record a resolution leaves, or ``None`` when it leaves nothing to keep. + + The synced side becomes the server's current copy, so whatever is pushed + next goes out against the server's current version. + """ + conflict, edit = cast(Conflict, record.conflict), cast(LocalEdit, record.edit) + on_server = conflict.remote_content is not None + if not on_server and isinstance(resolution, (TakeServer, Delete)): + # The server doesn't have the object: a create it never had leaves nothing. + return None if record.is_create else _removed(record, now) + base = _synced(record, conflict, conflict.remote_version, now, prefix=REMOTE) + # Over a copy the server has, an edit is a change; with none, a create. + kind = EditKind.CHANGE if on_server else EditKind.CREATE + match resolution: + case TakeServer(): + return base + case KeepEdit(): + return _edited(base, edit if edit.kind is EditKind.DELETE else replace(edit, kind=kind)) + case Merge(): + merged = LocalEdit( + kind, + resolution.content, + resolution.fingerprint, + resolution.containers, + resolution.logical_key, + now, + ) + return _edited(base, merged) + # Delete, the one resolution left. + return _edited(base, LocalEdit(EditKind.DELETE, None, None, frozenset(), None, now)) + + +def _edited(record: Record, edit: LocalEdit) -> Record: + """The record holding ``edit``, or synced if the edit brings it back to its synced copy.""" + if edit.kind is EditKind.CHANGE and record.content is not None and same_data(edit, record): + return replace(record, state=RecordState.SYNCED, edit=None) + return replace(record, state=RecordState.PENDING, edit=edit) + + +def _changed_to(record: Record, edit: LocalEdit) -> Record: + """The record after a change to ``edit``'s data; a create stays a create.""" + kind = EditKind.CREATE if record.is_create else EditKind.CHANGE + return _edited(record, replace(edit, kind=kind)) + + +def _waits_for_version(record: Record, *, guarded: bool) -> bool: + """Whether a pending edit waits for an ingest, because its push would go out unguarded.""" + return guarded and record.version is None and not record.is_create + + +def _check_outcome(scope_id: str, claimed: Record, outcome: PushOutcome) -> None: + """Raise ``ValueError`` unless the outcome is one a push of the claimed edit can have.""" + if ( + claimed.scope != scope_id + or claimed.state is not RecordState.IN_FLIGHT + or claimed.edit is None + ): + raise ValueError("settle() takes a record this session's claim() returned") + kind = claimed.edit.kind + match outcome: + case Gone() if kind is EditKind.CREATE: + raise ValueError("A create can't be gone: the server never had the object") + case LogicalKeyTaken() if kind is not EditKind.CREATE: + raise ValueError(f"Only a create can be refused for its logical key, not a {kind}") + case Applied(object_id=str()) if kind is EditKind.DELETE: + raise ValueError("An applied delete has no object id to report") + case Applied(object_id=None) if kind is EditKind.CREATE and claimed.object_id is None: + raise ValueError("An applied create needs the object id the server gave it") + + class SessionState(StrEnum): """Where a :term:`session` is.""" @@ -71,14 +249,45 @@ class SessionState(StrEnum): INGESTING = "ingesting" #: The last page was ingested, and the token is final. INGESTED = "ingested" + #: A :term:`push` was claimed; no more pages can be ingested. + PUSHING = "pushing" class Session: - """One sync of one :term:`scope`, opened with :meth:`Replica.session`.""" + """One sync of one :term:`scope`, opened with :meth:`Replica.session`. - def __init__(self, replica: Replica, scope_id: str) -> None: + A session :term:`ingests ` every page first, then the + :term:`adapter` pushes your :term:`pending edits `: for each + record :meth:`to_push` lists, it :meth:`claims ` it, sends it, and + :meth:`settles ` it with how the push ended. + + Examples: + The adapter pushes an edit after ingesting what changed on the server, + here nothing: + + >>> from calendaring_sync import Applied + >>> key = replica.find_record("work", "a").local_key + >>> replica.change(key, b"Team meeting, moved", fingerprint="fp-a2") + >>> with replica.session( + ... "work", fingerprint_scheme="sha256", media_type="text/plain" + ... ) as session: + ... session.ingest(ChangeSet("work", "t1", "t2", {})) + ... for record in session.to_push(): + ... claimed = session.claim(record.local_key) + ... # The adapter sends claimed.edit to the server here. + ... session.settle(claimed, Applied()) + True + >>> replica.record(key).state, replica.record(key).content + (, b'Team meeting, moved') + """ + + def __init__( + self, replica: Replica, scope_id: str, *, guarded_by_version: bool, unique_logical_key: bool + ) -> None: self._replica = replica self._scope_id = scope_id + self._guarded_by_version = guarded_by_version + self._unique_logical_key = unique_logical_key self._state = SessionState.IDLE self._listing: bool | None = None self._mentioned: set[str] = set() @@ -104,8 +313,10 @@ def ingest(self, change_set: ChangeSet) -> None: reported. A :term:`truncated page` of an :term:`incremental change set` moves the token to its :term:`checkpoint`. A :term:`full listing` moves the token only on its last page, which also removes every - record the listing never mentioned. If anything fails, nothing - changes. + record the listing never mentioned. A complete change set, or the + last page, also settles every :term:`push` still :term:`in flight` + from what the server reported, and one it doesn't mention is + pending again. If anything fails, nothing changes. Raises: TransactionError: If a store transaction is already open in this thread. @@ -115,8 +326,8 @@ def ingest(self, change_set: ChangeSet) -> None: session goes back to idle, so the adapter can fetch again and ingest from the start in the same session. SessionError: If the session has ended, already ingested its last - page, or the page is a listing page in an incremental session - or the other way round. + page, has started pushing, or the page is a listing page in an + incremental session or the other way round. ValueError: If the change set is for another scope, an incremental change set arrives for a scope with no token, or an ``Unchanged`` entry is outside a listing or names a record @@ -130,11 +341,16 @@ def ingest(self, change_set: ChangeSet) -> None: raise SessionError("The session has ended") if self._state is SessionState.INGESTED: raise SessionError("The session already ingested its last page") + if self._state is SessionState.PUSHING: + raise SessionError("The session already pushed, and a change set can't come after that") if self._listing is not None and self._listing != change_set.full_listing: raise SessionError("One session's pages are all listing pages or all incremental") try: self._mentioned = self._replica._ingest( - change_set, first_page=self._state is SessionState.IDLE, mentioned=self._mentioned + change_set, + first_page=self._state is SessionState.IDLE, + mentioned=self._mentioned, + unique_logical_key=self._unique_logical_key, ) except StaleChangeSetError: self._restart() @@ -142,6 +358,137 @@ def ingest(self, change_set: ChangeSet) -> None: self._listing = change_set.full_listing self._state = SessionState.INGESTING if change_set.truncated else SessionState.INGESTED + def to_push(self) -> list[Record]: + """Return the :term:`pending edits ` ready to :term:`push`, by local key. + + With ``guarded_by_version``, a change or delete of a record with no + :term:`version` waits for an ingest that gives it one, and isn't + listed. Runs in a read transaction of its own. + + Raises: + TransactionError: If a store transaction is already open in this thread. + SessionError: If the session hasn't ingested its last page, or has ended. + + Examples: + >>> key = replica.find_record("work", "a").local_key + >>> replica.change(key, b"Team meeting, moved", fingerprint="fp-a2") + >>> with replica.session( + ... "work", fingerprint_scheme="sha256", media_type="text/plain" + ... ) as session: + ... session.ingest(ChangeSet("work", "t1", "t2", {})) + ... [record.local_key for record in session.to_push()] == [key] + True + """ + self._check_ready() + return self._replica._to_push(self._scope_id, guarded=self._guarded_by_version) + + def claim(self, local_key: int, *, object_id: str | None = None) -> Record | None: + """Mark a :term:`pending edit` :term:`in flight`, so no other process pushes it. + + Send the record this returns, then report how the push ended with + :meth:`settle`. Runs in a write transaction of its own; when two + processes claim one record, only one gets it. + + Args: + local_key: The record to push. + object_id: For a create the server takes ids for, the id it's + about to be sent under. It's stored with the claim, so the + create is recognized by it even if the answer never arrives. + An id a removed record holds passes to this one. + + Returns: + The claimed record, or ``None`` when it no longer holds a + pending edit to push: another process claimed it, the edit was + discarded or resolved, or it waits for a version. + + Raises: + TransactionError: If a store transaction is already open in this thread. + SessionError: If the session hasn't ingested its last page, or has ended. + DuplicateObjectError: If a record that isn't removed holds ``object_id``. + ValueError: If the record belongs to another scope, ``object_id`` + is given for a record that has one, or a string holds a + :ref:`lone surrogate `. + + Examples: + A second claim of the same record gets nothing: + + >>> key = replica.find_record("work", "a").local_key + >>> replica.change(key, b"Team meeting, moved", fingerprint="fp-a2") + >>> with replica.session( + ... "work", fingerprint_scheme="sha256", media_type="text/plain" + ... ) as session: + ... session.ingest(ChangeSet("work", "t1", "t2", {})) + ... claimed = session.claim(key) + ... claimed.state, session.claim(key) + (, None) + """ + check_text(object_id=object_id) + self._check_ready() + claimed = self._replica._claim( + self._scope_id, local_key, object_id, guarded=self._guarded_by_version + ) + if claimed is not None: + self._state = SessionState.PUSHING + return claimed + + def settle(self, claimed: Record, outcome: PushOutcome) -> bool: + """Record how the :term:`push` of a claimed record ended. + + Runs in a write transaction of its own, and writes a :term:`change + feed` entry for every outcome but ``Unknown``. If another process + already settled this push, for example by ingesting while it was + in flight, the outcome only counts while the record still holds the + same edit. + + Args: + claimed: The record :meth:`claim` returned. + outcome: How the push ended. + + Returns: + ``True`` if the store now agrees with the outcome, ``False`` if + the push had already been settled another way and nothing was + written. + + Raises: + TransactionError: If a store transaction is already open in this thread. + SessionError: If the session isn't pushing, or has ended. + DuplicateObjectError: If ``Applied`` names an id a record that + isn't removed holds. + ValueError: If ``claimed`` isn't a claimed record of this scope, + or the outcome is one no push of its edit can have: ``Gone`` + for a create, ``LogicalKeyTaken`` for anything but a create, + ``Applied`` with an id for a delete, or ``Applied`` without + one for a create that has none. + + Examples: + The server refused the write, so the edit is pending again: + + >>> from calendaring_sync import Rejected + >>> key = replica.find_record("work", "a").local_key + >>> replica.change(key, b"Team meeting, moved", fingerprint="fp-a2") + >>> with replica.session( + ... "work", fingerprint_scheme="sha256", media_type="text/plain" + ... ) as session: + ... session.ingest(ChangeSet("work", "t1", "t2", {})) + ... session.settle(session.claim(key), Rejected()) + True + >>> replica.record(key).state + + """ + _check_outcome(self._scope_id, claimed, outcome) + if self._closed: + raise SessionError("The session has ended") + if self._state is not SessionState.PUSHING: + raise SessionError("Only a session that claimed a push can settle one") + return self._replica._settle(claimed, outcome) + + def _check_ready(self) -> None: + """Raise unless the session may push: its last page is ingested and it hasn't ended.""" + if self._closed: + raise SessionError("The session has ended") + if self._state not in (SessionState.INGESTED, SessionState.PUSHING): + raise SessionError("A session pushes only after it ingested its last page") + def _restart(self) -> None: """Go back to idle, dropping a half-done listing.""" self._state = SessionState.IDLE @@ -211,7 +558,13 @@ def __init__(self, store: StateStore, *, clock: Callable[[], datetime] | None = @contextmanager def session( - self, scope_id: str, *, fingerprint_scheme: str, media_type: str + self, + scope_id: str, + *, + fingerprint_scheme: str, + media_type: str, + guarded_by_version: bool = False, + unique_logical_key: bool = False, ) -> Iterator[Session]: """Open a :term:`session` for one scope, for the length of the ``with`` block. @@ -228,6 +581,14 @@ def session( scope_id: The scope's id, chosen by the :term:`adapter`. fingerprint_scheme: The name of the adapter's fingerprint scheme. media_type: The media type of the scope's :term:`content`. + guarded_by_version: Whether the adapter's pushes only apply if + the object still has the :term:`version` they were based on. + A change or delete then waits for an ingest whenever its + record has no version, so it never goes out unguarded. + unique_logical_key: Whether no two objects in the scope can share + a :term:`logical key`. A create whose answer never arrived is + then recognized by its key at the next ingest, which a create + the server picks the id for needs. Raises: TransactionError: If a store transaction is already open in this thread. @@ -245,7 +606,12 @@ def session( b'Team meeting, moved' """ check_text(scope_id=scope_id, fingerprint_scheme=fingerprint_scheme, media_type=media_type) - session = Session(self, scope_id) + session = Session( + self, + scope_id, + guarded_by_version=guarded_by_version, + unique_logical_key=unique_logical_key, + ) with self._sessions_lock: if scope_id in self._sessions: raise SessionError(f"A session for {scope_id!r} is already open") @@ -277,7 +643,14 @@ def _open_scope(self, scope_id: str, fingerprint_scheme: str, media_type: str) - for record in self._store.scope_records(scope_id): self._store.update_record(_without_fingerprints(record)) - def _ingest(self, change_set: ChangeSet, *, first_page: bool, mentioned: set[str]) -> set[str]: + def _ingest( + self, + change_set: ChangeSet, + *, + first_page: bool, + mentioned: set[str], + unique_logical_key: bool, + ) -> set[str]: """Apply one change set in one transaction; return the listing's mentioned ids after it.""" now = self._clock() mentioned = set(mentioned) if not first_page else set() @@ -292,10 +665,11 @@ def _ingest(self, change_set: ChangeSet, *, first_page: bool, mentioned: set[str raise ValueError(f"Scope {scope.id!r} has no token, so it needs a full listing") if change_set.full_listing: scope = self._check_listing_page(scope, change_set, first_page) + claimed = self._claimed_creates(scope.id) if unique_logical_key else {} for object_id, change in change_set.changes.items(): if not change_set.full_listing and isinstance(change, Unchanged): raise ValueError(f"Unchanged object {object_id!r} outside a full listing") - self._apply(scope.id, object_id, change, now) + self._apply(scope.id, object_id, change, now, claimed) if change_set.full_listing: mentioned.add(object_id) if change_set.full_listing and not change_set.truncated: @@ -306,6 +680,11 @@ def _ingest(self, change_set: ChangeSet, *, first_page: bool, mentioned: set[str elif not change_set.full_listing: scope = replace(scope, token=change_set.token) if not change_set.truncated: + # The change set is complete: a push it didn't settle didn't land. + for record in self._store.scope_records(scope.id, states=_IN_FLIGHT): + self._write( + replace(record, state=RecordState.PENDING), FeedEntryKind.PUSH_FAILED, now + ) scope = replace(scope, last_sync_at=now) self._store.put_scope(scope) return mentioned @@ -321,8 +700,20 @@ def _check_listing_page(self, scope: Scope, change_set: ChangeSet, first_page: b ) return scope - def _apply(self, scope_id: str, object_id: str, change: Change, now: datetime) -> None: - """Apply one object's change to the store, classified against its local edit.""" + def _apply( + self, + scope_id: str, + object_id: str, + change: Change, + now: datetime, + claimed: dict[str, Record], + ) -> None: + """Apply one object's change to the store, classified against its local edit. + + ``claimed`` maps a logical key to the one create in flight without + an object id that holds it; a new object with that key is taken as + that create, and leaves the map. + """ record = self._store.find_record(scope_id, object_id) if isinstance(change, Unchanged): if record is None or record.server_version is None: @@ -331,9 +722,9 @@ def _apply(self, scope_id: str, object_id: str, change: Change, now: datetime) - "but the store has no version for it" ) return - if record is not None and record.state is RecordState.IN_FLIGHT: - # How an ingest settles a push in flight is decided with pushes. - return + if record is None and isinstance(change, Changed) and change.logical_key in claimed: + record = claimed.pop(change.logical_key) + in_flight = record is not None and record.state is RecordState.IN_FLIGHT live = None if record is None or record.state is RecordState.REMOVED else record if live is not None and live.conflict is not None and same_remote(live.conflict, change): if isinstance(change, Changed): @@ -342,10 +733,16 @@ def _apply(self, scope_id: str, object_id: str, change: Change, now: datetime) - return edit = None if live is None else live.edit synced = live if live is not None and live.content is not None else None - change_class, kind = classify(synced, change, edit) + change_class, kind = classify(synced, change, edit, in_flight=in_flight) was_conflict = live is not None and live.conflict is not None - # A record leaving a conflict is reported as the conflict ending. - settled = FeedEntryKind.CONFLICT_RESOLVED if was_conflict else None + # A record leaving a conflict is reported as the conflict ending, and + # one whose push was in flight as the push reaching the server. + if was_conflict: + settled: FeedEntryKind | None = FeedEntryKind.CONFLICT_RESOLVED + elif in_flight: + settled = FeedEntryKind.PUSHED + else: + settled = None match change_class: case ChangeClass.CONFLICT if kind is not None and live is not None: opened_at = now if live.conflict is None else live.conflict.opened_at @@ -366,30 +763,21 @@ def _apply(self, scope_id: str, object_id: str, change: Change, now: datetime) - # Both sides arrived at the same content, so the edit is done. self._store_server_copy(record, scope_id, object_id, change, now, settled) case ChangeClass.DELETED_REMOTE if live is not None: - self._write( - replace( - live, state=RecordState.REMOVED, edit=None, conflict=None, synced_at=now - ), - settled or FeedEntryKind.REMOVED, - now, - ) + self._write(_removed(live, now), settled or FeedEntryKind.REMOVED, now) case ChangeClass.UNCHANGED | ChangeClass.LOCAL_ONLY | ChangeClass.DELETED_LOCAL if ( - live is not None and (isinstance(change, Changed) or was_conflict) + live is not None and (isinstance(change, Changed) or was_conflict or in_flight) ): # The server's copy holds the synced data, so only its version and - # content move; a conflict this settles leaves the edit to be pushed. + # content move; a conflict this settles leaves the edit to be pushed, + # and so does a push in flight that hasn't landed. if isinstance(change, Changed): - live = replace( - live, - version=change.version, - fingerprint=change.fingerprint, - content=change.content, - synced_at=now, - ) - if was_conflict: + live = _with_copy(live, change, change.version, now) + if was_conflict or in_flight: self._write( replace(live, state=RecordState.PENDING, conflict=None), - FeedEntryKind.CONFLICT_RESOLVED, + FeedEntryKind.CONFLICT_RESOLVED + if was_conflict + else FeedEntryKind.PUSH_FAILED, now, ) else: @@ -405,28 +793,30 @@ def _store_server_copy( settled: FeedEntryKind | None, ) -> None: """Store the server's copy as the record's synced copy, with the feed entry about it.""" - synced = Record( - local_key=0 if record is None else record.local_key, - scope=scope_id, - object_id=object_id, - state=RecordState.SYNCED, - version=change.version, - fingerprint=change.fingerprint, - content=change.content, - containers=change.containers, - logical_key=change.logical_key, - synced_at=now, - last_entry=0 if record is None else record.last_entry, - ) + known = _new_record(scope_id, object_id) if record is None else record + synced = replace(_synced(known, change, change.version, now), object_id=object_id) if record is None: synced = replace(synced, local_key=self._store.insert_record(synced)) self._write(synced, settled or FeedEntryKind.CHANGED, now) + def _claimed_creates(self, scope_id: str) -> dict[str, Record]: + """Map each logical key to the one create in flight without an object id that holds it. + + A key two such creates hold is left out: neither can be told apart + from the other by it. + """ + found: dict[str, list[Record]] = {} + for record in self._store.scope_records(scope_id, states=_IN_FLIGHT): + key = record.edit.logical_key if record.edit else None + if record.is_create and record.object_id is None and key is not None: + found.setdefault(key, []).append(record) + return {key: records[0] for key, records in found.items() if len(records) == 1} + def _remove_unmentioned(self, scope_id: str, mentioned: set[str], now: datetime) -> None: """Remove every record with an object id the finished listing never mentioned.""" for record in self._store.scope_records(scope_id): if record.object_id is not None and record.object_id not in mentioned: - self._apply(scope_id, record.object_id, Removed(), now) + self._apply(scope_id, record.object_id, Removed(), now, {}) def _write(self, record: Record, kind: FeedEntryKind, at: datetime) -> None: """Store a record together with the feed entry about it.""" @@ -590,35 +980,19 @@ def create( >>> replica.record(key).edit.content b'Lunch' """ - check_content(content) - check_text( - scope_id=scope_id, - fingerprint=fingerprint, - containers=containers, - logical_key=logical_key, - object_id=object_id, + check_data( + content, fingerprint, containers, logical_key, scope_id=scope_id, object_id=object_id ) with self._store.transaction(): self._existing_scope(scope_id) edit = LocalEdit( EditKind.CREATE, content, fingerprint, containers, logical_key, self._clock() ) - created = Record( - local_key=0, - scope=scope_id, - object_id=object_id, - state=RecordState.PENDING, - version=None, - fingerprint=None, - content=None, - containers=frozenset(), - logical_key=None, - synced_at=None, - last_entry=0, - edit=edit, + created = replace( + _new_record(scope_id, object_id), state=RecordState.PENDING, edit=edit ) - removed = None if object_id is None else self._store.find_record(scope_id, object_id) - if removed is None or removed.state is not RecordState.REMOVED: + removed = None if object_id is None else self._removed_holder(scope_id, object_id) + if removed is None: return self._store.insert_record(created) self._store.update_record( replace( @@ -653,7 +1027,7 @@ def change( UnknownRecordError: If there's no record with this local key. OpenConflictError: If the record holds an open :term:`conflict`. RemovedRecordError: If the record was removed from the server. - EditRefusedError: If the record's :term:`push` is :term:`in flight`. + PushInFlightError: If the record's :term:`push` is :term:`in flight`. TypeError: If the content isn't bytes. ValueError: If a string holds a :ref:`lone surrogate `. @@ -669,22 +1043,12 @@ def change( >>> replica.record(key).state """ - check_content(content) - check_text(fingerprint=fingerprint, containers=containers, logical_key=logical_key) + check_data(content, fingerprint, containers, logical_key) with self._editing(local_key) as record: - created = record.is_create edit = LocalEdit( - EditKind.CREATE if created else EditKind.CHANGE, - content, - fingerprint, - containers, - logical_key, - self._clock(), + EditKind.CHANGE, content, fingerprint, containers, logical_key, self._clock() ) - if not created and same_data(edit, record): - self._drop_edit(record) - else: - self._store.update_record(replace(record, state=RecordState.PENDING, edit=edit)) + self._store.update_record(_changed_to(record, edit)) def delete(self, local_key: int) -> None: """Record the deletion of an object. @@ -699,7 +1063,7 @@ def delete(self, local_key: int) -> None: UnknownRecordError: If there's no record with this local key. OpenConflictError: If the record holds an open :term:`conflict`. RemovedRecordError: If the record was removed from the server. - EditRefusedError: If the record's :term:`push` is :term:`in flight`. + PushInFlightError: If the record's :term:`push` is :term:`in flight`. Examples: >>> key = replica.find_record("work", "a").local_key @@ -727,7 +1091,7 @@ def discard(self, local_key: int) -> None: UnknownRecordError: If there's no record with this local key. OpenConflictError: If the record holds an open :term:`conflict`. RemovedRecordError: If the record was removed from the server. - EditRefusedError: If the record's :term:`push` is :term:`in flight`. + PushInFlightError: If the record's :term:`push` is :term:`in flight`. Examples: >>> key = replica.find_record("work", "a").local_key @@ -740,6 +1104,109 @@ def discard(self, local_key: int) -> None: if record.edit is not None: self._drop_edit(record) + def _to_push(self, scope_id: str, *, guarded: bool) -> list[Record]: + with self._store.transaction(write=False): + pending = self._store.scope_records(scope_id, states=_PENDING) + return [r for r in pending if not _waits_for_version(r, guarded=guarded)] + + def _claim( + self, scope_id: str, local_key: int, object_id: str | None, *, guarded: bool + ) -> Record | None: + with self._store.transaction(): + record = self._store.get_record(local_key) + if record is None: + return None + if record.scope != scope_id: + raise ValueError(f"Record {local_key} belongs to scope {record.scope!r}") + if object_id is not None and record.object_id is not None: + raise ValueError(f"Record {local_key} already has object id {record.object_id!r}") + if record.state is not RecordState.PENDING or _waits_for_version( + record, guarded=guarded + ): + return None + if object_id is not None: + self._free_object_id(scope_id, object_id, record.local_key) + claimed = replace( + record, state=RecordState.IN_FLIGHT, object_id=object_id or record.object_id + ) + self._store.update_record(claimed) + return claimed + + def _settle(self, claimed: Record, outcome: PushOutcome) -> bool: + now = self._clock() + with self._store.transaction(): + record = self._store.get_record(claimed.local_key) + if ( + record is None + or record.edit != claimed.edit + or record.state not in (RecordState.IN_FLIGHT, RecordState.PENDING) + ): + return False + pending_again = record.state is RecordState.PENDING + # A record in flight or pending always holds an edit; the store checks it. + edit = cast(LocalEdit, record.edit) + match outcome: + case Rejected() | Unknown() if pending_again: + pass + case Rejected(): + self._write( + replace(record, state=RecordState.PENDING), FeedEntryKind.PUSH_FAILED, now + ) + case Unknown(): + pass + case Applied() | Gone() if edit.kind is EditKind.DELETE: + self._write(_removed(record, now), FeedEntryKind.PUSHED, now) + case Applied(object_id=renamed): + if renamed is not None and renamed != record.object_id: + self._free_object_id(record.scope, renamed, record.local_key) + # The server may store the edit rewritten, so no version is kept. + pushed = replace( + _synced(record, edit, None, now), object_id=renamed or record.object_id + ) + self._write(pushed, FeedEntryKind.PUSHED, now) + case Gone(): + self._open_conflict(record, ConflictKind.REMOVED_REMOTELY, None, now) + case _: + # LogicalKeyTaken, the one outcome left. + self._open_conflict(record, ConflictKind.LOGICAL_KEY_TAKEN, outcome.holder, now) + return True + + def _open_conflict( + self, record: Record, kind: ConflictKind, holder: str | None, now: datetime + ) -> None: + """Store a conflict whose server has no copy of the object, from a push's outcome.""" + conflict = replace(Conflict.from_change(kind, Removed(), now), holder=holder) + self._write( + replace(record, state=RecordState.CONFLICT, conflict=conflict), + FeedEntryKind.CONFLICT_OPENED, + now, + ) + + def _free_object_id(self, scope_id: str, object_id: str, owner: int) -> None: + """Make ``object_id`` free for record ``owner``: a removed record holding it is deleted. + + Raises: + DuplicateObjectError: If a record that isn't removed holds it. + """ + removed = self._removed_holder(scope_id, object_id, owner) + if removed is not None: + self._store.delete_record(removed.local_key) + + def _removed_holder( + self, scope_id: str, object_id: str, owner: int | None = None + ) -> Record | None: + """The removed record that hands ``object_id`` over, or ``None`` if no other holds it. + + Raises: + DuplicateObjectError: If a record that isn't removed holds it. + """ + holder = self._store.find_record(scope_id, object_id) + if holder is None or holder.local_key == owner: + return None + if holder.state is not RecordState.REMOVED: + raise DuplicateObjectError(f"Scope {scope_id!r} already has object {object_id!r}") + return holder + def _drop_edit(self, record: Record) -> None: """Leave the record as the server has it: synced, or gone if the server never had it.""" if record.is_create: @@ -768,7 +1235,7 @@ def _editable(self, local_key: int) -> Record: if record.state is RecordState.REMOVED: raise RemovedRecordError(f"Record {local_key} was removed from the server") if record.state is RecordState.IN_FLIGHT: - raise EditRefusedError(f"Record {local_key} has a push in flight") + raise PushInFlightError(f"Record {local_key} has a push in flight") return record def records(self, scope_id: str) -> list[Record]: @@ -865,6 +1332,158 @@ def drop_scope(self, scope_id: str) -> DroppedScope: conflicts=tuple(r for r in records if r.state is RecordState.CONFLICT), ) + def resolve(self, record: Record, resolution: Resolution) -> None: + """Resolve one :term:`conflict`, as your application decided from ``record``. + + Anything but the server's side becomes a :term:`pending edit` over the + server's current copy, so the next sync pushes it against the server's + current version. Writes a ``CONFLICT_RESOLVED`` entry to the + :term:`change feed`. Runs in a write transaction of its own. + + Args: + record: The record in conflict, as your application read it. + resolution: What to do with it. + + Raises: + TransactionError: If a store transaction is already open in this thread. + StaleConflictError: If the conflict changed since ``record`` was + read: it ended, or its edit or the server's side moved on. A + move of the server's version alone doesn't count. + UnknownHolderError: For ``UpdateHolder``, if the server named no + record that's still there, and no single record holds the + refused create's logical key, or it was sent without one. + OpenConflictError: For ``UpdateHolder``, if the holder is in conflict. + PushInFlightError: For ``UpdateHolder``, if the holder's push is in flight. + ValueError: If ``record`` isn't in conflict, or the resolution + doesn't fit its kind: ``KeepEdit`` for a create the server + refused for its logical key, or ``UpdateHolder`` for any other. + + Examples: + >>> from calendaring_sync import KeepEdit + >>> key = replica.find_record("work", "a").local_key + >>> replica.change(key, b"Team meeting, room 2", fingerprint="fp-mine") + >>> theirs = {"a": Changed("v2", "fp-theirs", b"Team meeting, room 3")} + >>> with replica.session( + ... "work", fingerprint_scheme="sha256", media_type="text/plain" + ... ) as session: + ... session.ingest(ChangeSet("work", "t1", "t2", theirs)) + >>> record = replica.record(key) + >>> record.conflict.kind + + >>> replica.resolve(record, KeepEdit()) + >>> replica.record(key).state, replica.record(key).version + (, 'v2') + """ + _check_resolution(record, resolution) + now = self._clock() + with self._store.transaction(): + stored = self._store.get_record(record.local_key) + if not _same_decision(stored, record): + raise StaleConflictError(f"Record {record.local_key}'s conflict changed") + stored = cast(Record, stored) + if isinstance(resolution, UpdateHolder): + holder = self._editable(self._holder(stored).local_key) + edit = cast(LocalEdit, stored.edit) + self._store.update_record(_changed_to(holder, replace(edit, recorded_at=now))) + resolved = None + else: + resolved = _resolved(stored, resolution, now) + if resolved is None: + self._store.append_entry( + stored.scope, + FeedEntryKind.CONFLICT_RESOLVED, + stored.local_key, + stored.object_id, + now, + ) + self._store.delete_record(stored.local_key) + else: + self._write(resolved, FeedEntryKind.CONFLICT_RESOLVED, now) + + def resolve_conflicts(self, resolver: Callable[[Record], Resolution | None]) -> int: + """Pass every open :term:`conflict` to ``resolver``, and resolve each as it answers. + + The conflicts are read in one read transaction, then ``resolver`` runs + outside any transaction, so it can take its time, and each answer is + resolved with :meth:`resolve` in a write transaction of its own. + ``None`` leaves a record open. A record that changed after it was read + is skipped, and comes back on the next call. An exception from + ``resolver`` or :meth:`resolve` stops the loop and keeps every + resolution before it. + + Args: + resolver: Your function. It gets each record in conflict, from every + scope, and returns a resolution or ``None``. + + Returns: + How many conflicts were resolved. + + Raises: + TransactionError: If a store transaction is already open in this thread. + + Examples: + >>> from calendaring_sync import TakeServer + >>> key = replica.find_record("work", "a").local_key + >>> replica.change(key, b"Team meeting, room 2", fingerprint="fp-mine") + >>> theirs = {"a": Changed("v2", "fp-theirs", b"Team meeting, room 3")} + >>> with replica.session( + ... "work", fingerprint_scheme="sha256", media_type="text/plain" + ... ) as session: + ... session.ingest(ChangeSet("work", "t1", "t2", theirs)) + >>> replica.resolve_conflicts(lambda record: TakeServer()) + 1 + >>> replica.record(key).content + b'Team meeting, room 3' + """ + with self._store.transaction(write=False): + open_conflicts = [ + record + for scope in self._store.scopes() + for record in self._store.scope_records(scope.id, states=_CONFLICT) + ] + resolved = 0 + for record in open_conflicts: + resolution = resolver(record) + if resolution is None: + continue + try: + self.resolve(record, resolution) + except StaleConflictError: + continue + resolved += 1 + return resolved + + def _holder(self, record: Record) -> Record: + """The one live record holding a refused create's logical key. + + The id the server named comes first; without one, or before that + object is synced, the key decides. A create sent without a key has + only the id to go by. + + Raises: + UnknownHolderError: If no single record holds it. + """ + conflict, edit = cast(Conflict, record.conflict), cast(LocalEdit, record.edit) + if conflict.holder is not None: + named = self._store.find_record(record.scope, conflict.holder) + if named is not None and named.state is not RecordState.REMOVED: + return named + keyed = ( + [] + if edit.logical_key is None + else self._store.find_by_logical_key(record.scope, edit.logical_key) + ) + holders = [ + other + for other in keyed + if other.local_key != record.local_key and other.state is not RecordState.REMOVED + ] + if len(holders) != 1: + raise UnknownHolderError( + f"{len(holders)} records hold logical key {edit.logical_key!r}, not one" + ) + return holders[0] + def feed_head(self) -> int: """Return the number of the last :term:`change feed` entry written, ``0`` for none. diff --git a/src/calendaring_sync/store/base.py b/src/calendaring_sync/store/base.py index 878fe0a..5e80887 100644 --- a/src/calendaring_sync/store/base.py +++ b/src/calendaring_sync/store/base.py @@ -31,6 +31,7 @@ UnknownRecordError, UnknownScopeError, ) +from calendaring_sync.objects._data import NO_DATA, REMOTE, data from calendaring_sync.objects.conflict import ConflictKind from calendaring_sync.objects.edit import EditKind from calendaring_sync.objects.feed import FeedEntry, FeedEntryKind @@ -40,20 +41,17 @@ # The states in which a record holds a pending edit. _EDIT_STATES = frozenset({RecordState.PENDING, RecordState.IN_FLIGHT, RecordState.CONFLICT}) -# The edit each kind of conflict is about, and whether the server still has -# the object. What the server's side of a refused create holds is decided -# with pushes, so that kind allows either. -_CONFLICT_SIDES: dict[ConflictKind, tuple[EditKind, bool | None]] = { +# The edit each kind of conflict is about, and whether the conflict holds a +# copy of the server's object. A refused create holds none: the server sent +# nothing back but the refusal. +_CONFLICT_SIDES: dict[ConflictKind, tuple[EditKind, bool]] = { ConflictKind.BOTH_CHANGED: (EditKind.CHANGE, True), ConflictKind.REMOVED_REMOTELY: (EditKind.CHANGE, False), ConflictKind.DELETED_LOCALLY: (EditKind.DELETE, True), ConflictKind.BOTH_CREATED: (EditKind.CREATE, True), - ConflictKind.LOGICAL_KEY_TAKEN: (EditKind.CREATE, None), + ConflictKind.LOGICAL_KEY_TAKEN: (EditKind.CREATE, False), } -# Content, fingerprint, containers and logical key of a copy that isn't there. -_NO_COPY: tuple[None, None, frozenset[str], None] = (None, None, frozenset(), None) - def _check_record(record: Record) -> None: """Raise :class:`RecordValueError` if the record holds what its state can't.""" @@ -71,8 +69,7 @@ def _check_record(record: Record) -> None: raise RecordValueError("A record has no synced content exactly when its edit is a create") # A delete holds no copy of the object; a change or a create holds one. if edit is not None: - edit_copy = (edit.content, edit.fingerprint, edit.containers, edit.logical_key) - if edit.kind is EditKind.DELETE and edit_copy != _NO_COPY: + if edit.kind is EditKind.DELETE and data(edit) != NO_DATA: raise RecordValueError("A delete edit can't hold a copy of the object") if edit.kind is not EditKind.DELETE and edit.content is None: raise RecordValueError(f"A {edit.kind} edit needs the object's content") @@ -83,18 +80,15 @@ def _check_record(record: Record) -> None: if edit is None or edit.kind is not edit_kind: raise RecordValueError(f"A {conflict.kind} conflict needs a {edit_kind} edit") # The server's side holds a copy exactly when the server still has the object. - remote_copy = ( - conflict.remote_content, - conflict.remote_fingerprint, - conflict.remote_containers, - conflict.remote_logical_key, - ) - removed = remote_copy == _NO_COPY and conflict.remote_version is None + removed = data(conflict, REMOTE) == NO_DATA and conflict.remote_version is None if conflict.remote_content is None and not removed: raise RecordValueError("A conflict without the server's content can't hold the rest of it") - if server_has_it is not None and server_has_it == removed: + if server_has_it == removed: need = "needs" if server_has_it else "can't hold" raise RecordValueError(f"A {conflict.kind} conflict {need} the server's copy") + # Only a refused create names the object holding its logical key. + if conflict.holder is not None and conflict.kind is not ConflictKind.LOGICAL_KEY_TAKEN: + raise RecordValueError(f"A {conflict.kind} conflict can't name a holder") class StateStore(ABC): @@ -262,9 +256,10 @@ def insert_record(self, record: Record) -> int: in conflict, the server's side of a conflict exactly when it's in conflict, and no synced content exactly when its edit is a create. An edit holds a copy of the object unless - it's a delete, and a conflict's edit and server's side match - its kind. The record is checked first, so this is raised - outside a transaction too. + it's a delete, a conflict's edit and server's side match its + kind, and only a refused create's conflict names a holder. + The record is checked first, so this is raised outside a + transaction too. TransactionRequiredError: If no transaction is open in this thread. TransactionPermissionError: If the transaction only reads. UnknownScopeError: If the record's scope doesn't exist. @@ -364,16 +359,26 @@ def find_by_logical_key(self, scope_id: str, logical_key: str) -> list[Record]: with self._operation(write=False): return self._find_by_logical_key(scope_id, logical_key) - def scope_records(self, scope_id: str) -> list[Record]: - """Return every record in the scope, ordered by local key. + def scope_records( + self, scope_id: str, *, states: frozenset[RecordState] | None = None + ) -> list[Record]: + """Return the scope's records, ordered by local key. Runs inside any transaction. + Args: + scope_id: The scope to read. + states: Only the records in one of these states; ``None`` for + every record. An empty set matches none, without asking the + store, since not every database takes an empty list. + Raises: TransactionRequiredError: If no transaction is open in this thread. """ with self._operation(write=False): - return self._scope_records(scope_id) + if states is not None and not states: + return [] + return self._scope_records(scope_id, states) def removed_records(self, through: int) -> list[Record]: """Return the removed records no feed entry after ``through`` is about. @@ -519,7 +524,9 @@ def _find_record(self, scope_id: str, object_id: str) -> Record | None: ... def _find_by_logical_key(self, scope_id: str, logical_key: str) -> list[Record]: ... @abstractmethod - def _scope_records(self, scope_id: str) -> list[Record]: ... + def _scope_records( + self, scope_id: str, states: frozenset[RecordState] | None + ) -> list[Record]: ... @abstractmethod def _removed_records(self, through: int) -> list[Record]: ... diff --git a/src/calendaring_sync/store/memory.py b/src/calendaring_sync/store/memory.py index b96a312..f1143d6 100644 --- a/src/calendaring_sync/store/memory.py +++ b/src/calendaring_sync/store/memory.py @@ -97,12 +97,16 @@ def _find_record(self, scope_id: str, object_id: str) -> Record | None: def _find_by_logical_key(self, scope_id: str, logical_key: str) -> list[Record]: return [ r - for r in self._scope_records(scope_id) + for r in self._scope_records(scope_id, None) if logical_key in (r.logical_key, r.edit and r.edit.logical_key) ] - def _scope_records(self, scope_id: str) -> list[Record]: - return [r for r in self._ordered_records() if r.scope == scope_id] + def _scope_records(self, scope_id: str, states: frozenset[RecordState] | None) -> list[Record]: + return [ + r + for r in self._ordered_records() + if r.scope == scope_id and (states is None or r.state in states) + ] def _removed_records(self, through: int) -> list[Record]: return [ diff --git a/src/calendaring_sync/store/sqlite.py b/src/calendaring_sync/store/sqlite.py index 5270b10..69aea37 100644 --- a/src/calendaring_sync/store/sqlite.py +++ b/src/calendaring_sync/store/sqlite.py @@ -257,10 +257,12 @@ def _marks(columns: tuple[_Column, ...]) -> str: conflict_remote_content BLOB, conflict_remote_containers TEXT, conflict_remote_logical_key TEXT, - conflict_opened_at TEXT + conflict_opened_at TEXT, + conflict_holder TEXT )""", f"""CREATE UNIQUE INDEX {_RECORDS}_by_object_id ON {_RECORDS} (scope, object_id)""", f"""CREATE INDEX {_RECORDS}_by_state ON {_RECORDS} (state, last_entry)""", + f"""CREATE INDEX {_RECORDS}_by_scope_state ON {_RECORDS} (scope, state)""", f"""CREATE INDEX {_RECORDS}_by_logical_key ON {_RECORDS} (scope, logical_key)""", f"""CREATE INDEX {_RECORDS}_by_edit_logical_key ON {_RECORDS} (scope, edit_logical_key)""", f"""CREATE TABLE {_FEED} ( @@ -454,8 +456,16 @@ def _find_by_logical_key(self, scope_id: str, logical_key: str) -> list[Record]: (scope_id, logical_key, scope_id, logical_key), ) - def _scope_records(self, scope_id: str) -> list[Record]: - return self._select(Record, _RECORDS, "WHERE scope = ? ORDER BY local_key", (scope_id,)) + def _scope_records(self, scope_id: str, states: frozenset[RecordState] | None) -> list[Record]: + if states is None: + return self._select(Record, _RECORDS, "WHERE scope = ? ORDER BY local_key", (scope_id,)) + marks = ", ".join("?" for _ in states) + return self._select( + Record, + _RECORDS, + f"WHERE scope = ? AND state IN ({marks}) ORDER BY local_key", + (scope_id, *states), + ) def _removed_records(self, through: int) -> list[Record]: return self._select( diff --git a/src/calendaring_sync/tests/benchmark.py b/src/calendaring_sync/tests/benchmark.py index fa72578..ac89d3a 100644 --- a/src/calendaring_sync/tests/benchmark.py +++ b/src/calendaring_sync/tests/benchmark.py @@ -2,7 +2,7 @@ # SPDX-License-Identifier: AGPL-3.0-or-later """ -Benchmarks for the work a sync does most: full listings and local edits. +Benchmarks for the work a sync does most: full listings, local edits and pushes. Run ``python -m calendaring_sync.tests.benchmark`` on ``main`` and on your branch, on the same machine, and compare the two. Times differ between @@ -22,10 +22,19 @@ import time from collections import Counter from collections.abc import Callable, Iterator -from contextlib import contextmanager +from contextlib import AbstractContextManager, contextmanager from pathlib import Path -from calendaring_sync import Changed, ChangeSet, RecordState, Replica, SQLiteStore +from calendaring_sync import ( + Applied, + Changed, + ChangeSet, + FeedEntryKind, + RecordState, + Replica, + Session, + SQLiteStore, +) #: The number of objects in the scope: a large calendar. DEFAULT_SIZE = 20_000 @@ -66,16 +75,25 @@ def _listing(size: int, since: str | None, token: str, changed: int = 0) -> Chan return ChangeSet(_SCOPE, since, token, objects, full_listing=True) +def _session(replica: Replica) -> AbstractContextManager[Session]: + """A session for the benchmark's scope.""" + return replica.session(_SCOPE, fingerprint_scheme=_SCHEME, media_type=_MEDIA) + + def _ingest(replica: Replica, change_set: ChangeSet) -> float: """Ingest a change set in one session, and return how long it took.""" started = time.perf_counter() - with replica.session(_SCOPE, fingerprint_scheme=_SCHEME, media_type=_MEDIA) as session: + with _session(replica) as session: session.ingest(change_set) return time.perf_counter() - started -def _edit(replica: Replica, size: int) -> float: - """Change one in ``EDITED_SHARE`` objects, one edit at a time, and return how long it took.""" +def _list_and_edit(replica: Replica, size: int) -> float: + """List the scope, then change one in ``EDITED_SHARE`` objects one edit at a time. + + Returns how long the edits took. + """ + _ingest(replica, _listing(size, None, _FIRST)) keys = [record.local_key for record in replica.records(_SCOPE)[: size // EDITED_SHARE]] started = time.perf_counter() for key in keys: @@ -83,14 +101,29 @@ def _edit(replica: Replica, size: int) -> float: return time.perf_counter() - started +def _push_edits(replica: Replica) -> float: + """Ingest an empty change set, then push every edit, and return how long it took. + + Each push is claimed and settled as applied in its own transaction, as an + adapter does around each request. + """ + started = time.perf_counter() + with _session(replica) as session: + session.ingest(ChangeSet(_SCOPE, _FIRST, _SECOND, {})) + for record in session.to_push(): + claimed = session.claim(record.local_key) + assert claimed is not None + session.settle(claimed, Applied()) + return time.perf_counter() - started + + def _relist_over_edits(replica: Replica, size: int) -> float: """List the scope, edit some objects, then list it again with new versions; time the second. Most objects come back with only a new version. The edited ones the server also changed become conflicts. """ - _ingest(replica, _listing(size, None, _FIRST)) - _edit(replica, size) + _list_and_edit(replica, size) conflicted = size // EDITED_SHARE // CONFLICTED_SHARE return _ingest(replica, _listing(size, _FIRST, _SECOND, changed=conflicted)) @@ -110,8 +143,14 @@ def listing_over_edits(size: int) -> float: def single_edits(size: int) -> float: """Local edits to one in ``EDITED_SHARE`` of ``size`` objects, each its own transaction.""" with _replica() as replica: - _ingest(replica, _listing(size, None, _FIRST)) - return _edit(replica, size) + return _list_and_edit(replica, size) + + +def pushes(size: int) -> float: + """Pushing local edits to one in ``EDITED_SHARE`` of ``size`` objects, each applied.""" + with _replica() as replica: + _list_and_edit(replica, size) + return _push_edits(replica) def states_over_edits(size: int) -> Counter[RecordState]: @@ -121,11 +160,21 @@ def states_over_edits(size: int) -> Counter[RecordState]: return Counter(record.state for record in replica.records(_SCOPE)) +def entries_after_pushes(size: int) -> Counter[FeedEntryKind]: + """The change feed entries ``pushes`` writes, to check what it measures.""" + with _replica() as replica: + _list_and_edit(replica, size) + head = replica.feed_head() + _push_edits(replica) + return Counter(entry.kind for entry in replica.read_feed(head)) + + #: Each scenario by name. Each takes the scope's size and returns the seconds it measured. SCENARIOS: dict[str, Callable[[int], float]] = { "full listing": full_listing, "listing over edits and conflicts": listing_over_edits, f"single edits to one in {EDITED_SHARE} objects": single_edits, + f"pushes of one in {EDITED_SHARE} objects": pushes, } diff --git a/src/calendaring_sync/tests/test_classify_unit.py b/src/calendaring_sync/tests/test_classify_unit.py index b0a12c6..dbcad0c 100644 --- a/src/calendaring_sync/tests/test_classify_unit.py +++ b/src/calendaring_sync/tests/test_classify_unit.py @@ -84,24 +84,36 @@ ("none", "fx", "create"): (_CONFLICT, ConflictKind.BOTH_CREATED), } +# A push in flight whose outcome never arrived classifies the same, except +# that a create coming back at its own id is your own write, whatever the +# server made of its content. +_IN_FLIGHT = _EXPECTED | { + ("none", "fa", "create"): (ChangeClass.UNCHANGED, None), + ("none", "fx", "create"): (ChangeClass.UNCHANGED, None), +} + _EVERY = list(product(_SYNCED, _REMOTES, _LOCALS)) _IMPOSSIBLE = [sides for sides in _EVERY if sides not in _EXPECTED] -def _classify(synced: str, remote: str, local: str) -> tuple[ChangeClass, ConflictKind | None]: - return classify(_SYNCED[synced], _REMOTES[remote], _LOCALS[local]) +def _classify( + synced: str, remote: str, local: str, *, in_flight: bool = False +) -> tuple[ChangeClass, ConflictKind | None]: + return classify(_SYNCED[synced], _REMOTES[remote], _LOCALS[local], in_flight=in_flight) +@pytest.mark.parametrize("in_flight, expected", [(False, _EXPECTED), (True, _IN_FLIGHT)]) @pytest.mark.parametrize("sides", _EXPECTED, ids="-".join) -def test_every_possible_combination(sides): - assert _classify(*sides) == _EXPECTED[sides] +def test_every_possible_combination(sides, in_flight, expected): + assert _classify(*sides, in_flight=in_flight) == expected[sides] +@pytest.mark.parametrize("in_flight", [False, True]) @pytest.mark.parametrize("sides", _IMPOSSIBLE, ids="-".join) -def test_combinations_no_object_can_have_raise(sides): +def test_combinations_no_object_can_have_raise(sides, in_flight): synced, _, local = sides with pytest.raises(ValueError): - _classify(*sides) + _classify(*sides, in_flight=in_flight) # Only these: nothing at all, a create of an object with a synced copy, # or a change or delete of one without. assert sides == ("none", "none", "none") or (local == "create") == (synced != "none") diff --git a/src/calendaring_sync/tests/test_sync_unit.py b/src/calendaring_sync/tests/test_sync_unit.py index 435834a..8d5c26b 100644 --- a/src/calendaring_sync/tests/test_sync_unit.py +++ b/src/calendaring_sync/tests/test_sync_unit.py @@ -16,33 +16,44 @@ import runpy import sqlite3 import sys +from collections import Counter from collections.abc import Callable, Iterator, Mapping from concurrent.futures import ThreadPoolExecutor from contextlib import AbstractContextManager, ExitStack, contextmanager, suppress from dataclasses import dataclass, replace from datetime import UTC, datetime, timedelta +from functools import partial from pathlib import Path +from typing import cast from unittest.mock import patch import pytest import calendaring_sync from calendaring_sync import ( + Applied, Change, Changed, ChangeSet, Conflict, ConflictKind, + Delete, DuplicateObjectError, EditKind, - EditRefusedError, FeedEntryKind, + Gone, + KeepEdit, LocalEdit, + LogicalKeyTaken, MemoryStore, + Merge, OpenConflictError, + PushInFlightError, + PushOutcome, Record, RecordState, RecordValueError, + Rejected, Removed, RemovedRecordError, Replica, @@ -53,18 +64,23 @@ SessionState, SQLiteStore, StaleChangeSetError, + StaleConflictError, StateStore, StoreBusyError, StoreError, StoreVersionError, + TakeServer, TransactionError, TransactionPermissionError, TransactionRequiredError, TrimmedFeedError, Unchanged, UnexpectedStoreError, + Unknown, + UnknownHolderError, UnknownRecordError, UnknownScopeError, + UpdateHolder, ) from calendaring_sync.constants import FEED_PAGE_SIZE, ISSUES_URL, SCHEMA_VERSION from calendaring_sync.store.sqlite import _FEED, _META, _RECORDS, _reader @@ -161,9 +177,21 @@ def _page( def _open_session( - replica: Replica, scope: str = _SCOPE, *, scheme: str = _SCHEME, media: str = _MEDIA + replica: Replica, + scope: str = _SCOPE, + *, + scheme: str = _SCHEME, + media: str = _MEDIA, + guarded_by_version: bool = False, + unique_logical_key: bool = False, ) -> AbstractContextManager[Session]: - return replica.session(scope, fingerprint_scheme=scheme, media_type=media) + return replica.session( + scope, + fingerprint_scheme=scheme, + media_type=media, + guarded_by_version=guarded_by_version, + unique_logical_key=unique_logical_key, + ) def _enter(context: AbstractContextManager[object]) -> None: @@ -173,7 +201,8 @@ def _enter(context: AbstractContextManager[object]) -> None: def _sync(replica: Replica, *pages: ChangeSet) -> SessionState: - with _open_session(replica) as session: + """Ingest change sets in one session, opened for their scope; return where it ended.""" + with _open_session(replica, pages[0].scope) as session: for change_set in pages: session.ingest(change_set) return session.state @@ -275,6 +304,13 @@ def _local_record(state: RecordState, object_id: str | None = None, local_key: i "part of a removed server copy": replace( _CONFLICTED, conflict=replace(_GONE, remote_version="v2") ), + "a refused create holding the server's copy": replace( + _CONFLICTED, + content=None, + edit=replace(_EDIT, kind=EditKind.CREATE), + conflict=replace(_CONFLICT, kind=ConflictKind.LOGICAL_KEY_TAKEN), + ), + "a holder on another kind": replace(_CONFLICTED, conflict=replace(_CONFLICT, holder="b")), } @@ -336,6 +372,26 @@ def _change( ) +def _create( + replica: Replica, + fingerprint: str, + *, + scope: str = _SCOPE, + containers: frozenset[str] = frozenset(), + logical_key: str | None = None, + object_id: str | None = None, +) -> int: + """Record a create holding the content a test gives ``fingerprint``; return its local key.""" + return replica.create( + scope, + _content(fingerprint), + fingerprint=fingerprint, + containers=containers, + logical_key=logical_key, + object_id=object_id, + ) + + def _step(replica: Replica, local_key: int, step: str) -> None: """Record one edit written as ``change ``, ``delete`` or ``discard``.""" action, *fingerprint = step.split() @@ -345,14 +401,17 @@ def _step(replica: Replica, local_key: int, step: str) -> None: getattr(replica, action)(local_key) -def _edit_locally(replica: Replica, local: str | None, object_id: str = "a") -> tuple[int, str]: +def _edit_locally( + replica: Replica, local: str | None, object_id: str = "a", *, logical_key: str | None = None +) -> tuple[int, str]: """Give a seeded scope one local edit; return its record's key and object id. - A create gets the id ``n``; every other edit goes to ``object_id``, a - seeded object, and a change makes it ``fl``. + A create gets the id ``n`` and ``logical_key``; every other edit goes to + ``object_id``, a seeded object, and a change makes it ``fl``. """ if local == "create": - return replica.create(_SCOPE, _content("fl"), fingerprint="fl", object_id="n"), "n" + created = _create(replica, "fl", logical_key=logical_key, object_id="n") + return created, "n" key = _key(replica, object_id) if local is not None: _step(replica, key, "change fl" if local == "change" else local) @@ -375,13 +434,6 @@ def _summary(record: Record | None) -> tuple[object, ...] | None: ) -def _in_conflict(replica: Replica, object_id: str, since: str = "T1", token: str = "T2") -> int: - """Change a seeded object locally while the server changes it too; return its local key.""" - key, _ = _edit_locally(replica, "change", object_id) - _sync(replica, _page(since, token, {object_id: _changed("fx", "v2")})) - return key - - def _edited(replica: Replica) -> Replica: """A seeded scope at token T2 where a holds a pending change and b an open conflict.""" _seeded(replica) @@ -396,6 +448,96 @@ def _a_removed(replica: Replica) -> Replica: return replica +@contextmanager +def _pushing( + replica: Replica, since: str = "T1", token: str = "T2", *, guarded_by_version: bool = False +) -> Iterator[Session]: + """A session that ingested a change set with nothing in it, so it may push.""" + with _open_session(replica, guarded_by_version=guarded_by_version) as session: + session.ingest(_page(since, token, {})) + yield session + + +def _claim(session: Session, local_key: int) -> Record: + """Claim a record the test knows is pending.""" + claimed = session.claim(local_key) + assert claimed is not None + return claimed + + +def _push( + replica: Replica, + local_key: int, + outcome: PushOutcome | None, + since: str = "T1", + token: str = "T2", +) -> None: + """Push one record in a session of its own. + + ``None`` leaves the push in flight, as a process that stops before the + server answers does. + """ + with _pushing(replica, since, token) as session: + claimed = _claim(session, local_key) + if outcome is not None: + assert session.settle(claimed, outcome) + + +def _refused_create( + replica: Replica, + holder: str | None, + since: str = "T1", + token: str = "T2", + *, + logical_key: str | None = "u", +) -> int: + """Push a create with id n that the server refuses for its logical key; return its local key.""" + key, _ = _edit_locally(replica, "create", logical_key=logical_key) + _push(replica, key, LogicalKeyTaken(holder), since, token) + return key + + +# The local edit and the server's report that open each kind of conflict an +# ingest stores. +_OPENED_BY = { + ConflictKind.BOTH_CHANGED: ("change", "fx"), + ConflictKind.REMOVED_REMOTELY: ("change", "removed"), + ConflictKind.DELETED_LOCALLY: ("delete", "fx"), + ConflictKind.BOTH_CREATED: ("create", "fx"), +} + + +def _in_conflict( + replica: Replica, + object_id: str = "a", + since: str = "T1", + token: str = "T2", + *, + kind: ConflictKind = ConflictKind.BOTH_CHANGED, +) -> int: + """Open a conflict of ``kind`` in a seeded scope; return its local key. + + A conflict about a create is at the id n, and a refused create names b + as the object holding its logical key u. + """ + if kind is ConflictKind.LOGICAL_KEY_TAKEN: + return _refused_create(replica, "b", since, token) + local, remote = _OPENED_BY[kind] + key, object_id = _edit_locally(replica, local, object_id) + _sync(replica, _page(since, token, {object_id: _remote(remote)})) + return key + + +def _two_conflicts(replica: Replica) -> tuple[int, int]: + """Seed a scope that ends at T3, where a and b both changed on both sides; return their keys.""" + return _in_conflict(_seeded(replica)), _in_conflict(replica, "b", "T2", "T3") + + +def _seeded_edit(replica: Replica, local: str | None) -> tuple[int, str]: + """Seed a scope and give it one local edit, as ``_edit_locally`` does.""" + return _edit_locally(_seeded(replica), local) + + def _assert_refused( replica: Replica, store: StateStore, @@ -423,6 +565,10 @@ def _assert_refused( assert _snapshot(replica, store) == before +# Examples that continue from the replica the Replica class example builds. +_CONTINUING = ("calendaring_sync.replica.Replica.", "calendaring_sync.replica.Session") + + def _docstring_examples() -> list[doctest.DocTest]: """Every docstring example in the package; a replica method's start from the class's.""" finder = doctest.DocTestFinder() @@ -435,7 +581,7 @@ def _docstring_examples() -> list[doctest.DocTest]: ] start = next(t for t in tests if t.name == "calendaring_sync.replica.Replica").examples for test in tests: - if test.name.startswith("calendaring_sync.replica.Replica."): + if test.name.startswith(_CONTINUING): test.examples = start + test.examples return tests @@ -460,6 +606,7 @@ def test_benchmarks_run_and_measure_what_they_say(self, monkeypatch, capsys): RecordState.PENDING: edited - conflicted, RecordState.CONFLICT: conflicted, } + assert benchmark.entries_after_pushes(size) == {FeedEntryKind.PUSHED: edited} def test_version_is_set(self): assert isinstance(calendaring_sync.__version__, str) @@ -692,9 +839,9 @@ def test_a_listing_meets_pending_edits(self, replica): _change(replica, _key(replica, "a"), "fl") replica.delete(_key(replica, "b")) creates = [ - replica.create(_SCOPE, _content("fn"), fingerprint="fn", object_id="n"), - replica.create(_SCOPE, _content("n1"), fingerprint="n1"), - replica.create(_SCOPE, _content("n2"), fingerprint="n2"), + _create(replica, "fn", object_id="n"), + _create(replica, "n1"), + _create(replica, "n2"), ] _sync(replica, _page("T1", "L", {}, full=True)) assert [_summary(replica.find_record(_SCOPE, o)) for o in ("a", "b", "c")] == [ @@ -887,14 +1034,13 @@ def test_unknown_scope(self, replica, store): replica.drop_scope, lambda scope: replica.find_record(scope, "a"), lambda scope: replica.find_by_logical_key(scope, "u"), - lambda scope: replica.create(scope, _content("fn"), fingerprint="fn"), + lambda scope: _create(replica, "fn", scope=scope), ): _assert_refused(replica, store, lambda call=call: call("nope"), UnknownScopeError) def test_two_scopes_stay_independent(self, replica): _a_removed(replica) - with _open_session(replica, _OTHER) as session: - session.ingest(_page(None, "O1", {"a": _changed("other-a")}, full=True, scope=_OTHER)) + _sync(replica, _page(None, "O1", {"a": _changed("other-a")}, full=True, scope=_OTHER)) assert [s.id for s in replica.scopes()] == [_SCOPE, _OTHER] assert replica.records(_OTHER)[0].state is RecordState.SYNCED assert replica.scope(_OTHER).token == "O1" @@ -903,7 +1049,7 @@ def test_two_scopes_stay_independent(self, replica): def test_drop_scope(self, replica, store): _edited(replica) - created = replica.create(_SCOPE, _content("fn"), fingerprint="fn") + created = _create(replica, "fn") with store.transaction(): in_flight = store.insert_record(_local_record(RecordState.IN_FLIGHT, object_id="f")) pending, head = [_key(replica), created, in_flight], replica.feed_head() @@ -1000,6 +1146,112 @@ def _conflicted(edit: EditKind, kind: ConflictKind) -> tuple[object, ...]: ("change", "fx", "fx", _conflicted(EditKind.CHANGE, ConflictKind.BOTH_CHANGED), []), ] +# How a push ended, recorded with settle(): the local edit, the outcome, +# then what the record ends up as and the feed entries settling wrote. +_OUTCOMES = [ + ("change", Applied(), _SYNCED, [FeedEntryKind.PUSHED]), + ("create", Applied(), _SYNCED, [FeedEntryKind.PUSHED]), + ("delete", Applied(), _REMOVED, [FeedEntryKind.PUSHED]), + ("delete", Gone(), _REMOVED, [FeedEntryKind.PUSHED]), + ("change", Rejected(), _pending(EditKind.CHANGE), [FeedEntryKind.PUSH_FAILED]), + ("delete", Rejected(), _pending(EditKind.DELETE), [FeedEntryKind.PUSH_FAILED]), + ("create", Rejected(), _pending(EditKind.CREATE), [FeedEntryKind.PUSH_FAILED]), + ( + "change", + Gone(), + _conflicted(EditKind.CHANGE, ConflictKind.REMOVED_REMOTELY), + [FeedEntryKind.CONFLICT_OPENED], + ), + ( + "create", + LogicalKeyTaken("b"), + _conflicted(EditKind.CREATE, ConflictKind.LOGICAL_KEY_TAKEN), + [FeedEntryKind.CONFLICT_OPENED], + ), + ( + "create", + LogicalKeyTaken(), + _conflicted(EditKind.CREATE, ConflictKind.LOGICAL_KEY_TAKEN), + [FeedEntryKind.CONFLICT_OPENED], + ), + ("change", Unknown(), (RecordState.IN_FLIGHT, EditKind.CHANGE, None), []), +] + +# A push whose outcome never arrived, met by the next complete ingest: the +# local edit, what the server reported, then what the record ends up as and +# the feed entries the ingest wrote. +_LEFT_IN_FLIGHT = [ + ("change", None, _pending(EditKind.CHANGE), [FeedEntryKind.PUSH_FAILED]), + ("change", "fa", _pending(EditKind.CHANGE), [FeedEntryKind.PUSH_FAILED]), + ("change", "fl", _SYNCED, [FeedEntryKind.PUSHED]), + ( + "change", + "fx", + _conflicted(EditKind.CHANGE, ConflictKind.BOTH_CHANGED), + [FeedEntryKind.CONFLICT_OPENED], + ), + ( + "change", + "removed", + _conflicted(EditKind.CHANGE, ConflictKind.REMOVED_REMOTELY), + [FeedEntryKind.CONFLICT_OPENED], + ), + ("delete", None, _pending(EditKind.DELETE), [FeedEntryKind.PUSH_FAILED]), + ("delete", "fa", _pending(EditKind.DELETE), [FeedEntryKind.PUSH_FAILED]), + ( + "delete", + "fx", + _conflicted(EditKind.DELETE, ConflictKind.DELETED_LOCALLY), + [FeedEntryKind.CONFLICT_OPENED], + ), + ("delete", "removed", _REMOVED, [FeedEntryKind.PUSHED]), + ("create", None, _pending(EditKind.CREATE), [FeedEntryKind.PUSH_FAILED]), + ("create", "removed", _pending(EditKind.CREATE), [FeedEntryKind.PUSH_FAILED]), + ("create", "fl", _SYNCED, [FeedEntryKind.PUSHED]), + # The server rewrote what it stored, and it's still your own write. + ("create", "fx", _SYNCED, [FeedEntryKind.PUSHED]), +] + +# An outcome arriving after another process's ingest already settled the +# push: what that ingest saw (None for nothing, "edited" for nothing and +# then a new local edit), the outcome, then what settle() returns, what the +# record ends up as and the feed entries settling wrote. +_SETTLED_LATE = [ + (None, Applied(), True, _SYNCED, [FeedEntryKind.PUSHED]), + (None, Rejected(), True, _pending(EditKind.CHANGE), []), + (None, Unknown(), True, _pending(EditKind.CHANGE), []), + ("edited", Applied(), False, _pending(EditKind.CHANGE), []), + ("fl", Applied(), False, _SYNCED, []), + ("fx", Rejected(), False, _conflicted(EditKind.CHANGE, ConflictKind.BOTH_CHANGED), []), +] + +_MERGED = Merge(_content("fm"), fingerprint="fm") + +# Each conflict resolved each way: the conflict's kind, the resolution, then +# what the record ends up as, or None once it's gone. +_RESOLVED = [ + (ConflictKind.BOTH_CHANGED, TakeServer(), _SYNCED), + (ConflictKind.BOTH_CHANGED, KeepEdit(), _pending(EditKind.CHANGE)), + (ConflictKind.BOTH_CHANGED, _MERGED, _pending(EditKind.CHANGE)), + (ConflictKind.BOTH_CHANGED, Delete(), _pending(EditKind.DELETE)), + (ConflictKind.REMOVED_REMOTELY, TakeServer(), _REMOVED), + (ConflictKind.REMOVED_REMOTELY, KeepEdit(), _pending(EditKind.CREATE)), + (ConflictKind.REMOVED_REMOTELY, _MERGED, _pending(EditKind.CREATE)), + (ConflictKind.REMOVED_REMOTELY, Delete(), _REMOVED), + (ConflictKind.DELETED_LOCALLY, TakeServer(), _SYNCED), + (ConflictKind.DELETED_LOCALLY, KeepEdit(), _pending(EditKind.DELETE)), + (ConflictKind.DELETED_LOCALLY, _MERGED, _pending(EditKind.CHANGE)), + (ConflictKind.DELETED_LOCALLY, Delete(), _pending(EditKind.DELETE)), + (ConflictKind.BOTH_CREATED, TakeServer(), _SYNCED), + (ConflictKind.BOTH_CREATED, KeepEdit(), _pending(EditKind.CHANGE)), + (ConflictKind.BOTH_CREATED, _MERGED, _pending(EditKind.CHANGE)), + (ConflictKind.BOTH_CREATED, Delete(), _pending(EditKind.DELETE)), + (ConflictKind.LOGICAL_KEY_TAKEN, TakeServer(), None), + (ConflictKind.LOGICAL_KEY_TAKEN, _MERGED, _pending(EditKind.CREATE)), + (ConflictKind.LOGICAL_KEY_TAKEN, Delete(), None), + (ConflictKind.LOGICAL_KEY_TAKEN, UpdateHolder(), None), +] + # Local edits in a row: where the record starts (synced as fa, or a # create), the edits, then what the record ends up as, or None once it's gone. _EDITED = [ @@ -1041,12 +1293,14 @@ def _conflicted(edit: EditKind, kind: ConflictKind) -> tuple[object, ...]: # A string no store can keep: a lone surrogate. _BAD_TEXT = "a\ud800b" +# Content that isn't bytes, typed as bytes so the calls refusing it type-check. +_NOT_BYTES = cast(bytes, "text of fa") + class TestLocalEdits: @pytest.mark.parametrize("start, steps, expected", _EDITED) def test_edits_collapse(self, replica, start, steps, expected): - _seeded(replica) - key, _ = _edit_locally(replica, "create" if start == "create" else None) + key, _ = _seeded_edit(replica, "create" if start == "create" else None) head = replica.feed_head() for step in steps: _step(replica, key, step) @@ -1058,10 +1312,9 @@ def test_edits_collapse(self, replica, start, steps, expected): def test_create_keeps_what_it_was_given(self, replica, clock): _seeded(replica) - key = replica.create( - _SCOPE, - _content("fn"), - fingerprint="fn", + key = _create( + replica, + "fn", containers=frozenset({"x", "y"}), logical_key="u", object_id="n", @@ -1077,11 +1330,11 @@ def test_a_create_at_an_id_in_use(self, replica, store): _assert_refused( replica, store, - lambda: replica.create(_SCOPE, _content("fn"), fingerprint="fn", object_id="b"), + lambda: _create(replica, "fn", object_id="b"), DuplicateObjectError, ) removed = replica.find_record(_SCOPE, "a") - key = replica.create(_SCOPE, _content("fn"), fingerprint="fn", object_id="a") + key = _create(replica, "fn", object_id="a") record = replica.record(key) assert key == removed.local_key assert _summary(record) == _pending(EditKind.CREATE) @@ -1100,7 +1353,7 @@ def test_records_that_take_no_edit(self, replica, store, step): refused = { _key(replica, "a"): RemovedRecordError, _key(replica, "b"): OpenConflictError, - in_flight: EditRefusedError, + in_flight: PushInFlightError, 999: UnknownRecordError, } for key, error in refused.items(): @@ -1119,8 +1372,7 @@ def test_an_edit_from_one_handle_meets_an_ingest_from_another(self, make_store, class TestConflicts: @pytest.mark.parametrize("local, remote, expected, feed", _CLASSIFIED) def test_ingest_classifies_against_the_local_edit(self, replica, local, remote, expected, feed): - _seeded(replica) - key, object_id = _edit_locally(replica, local) + key, object_id = _seeded_edit(replica, local) head = replica.feed_head() _sync(replica, _page("T1", "T2", {} if remote is None else {object_id: _remote(remote)})) assert _summary(replica.record(key)) == expected @@ -1130,8 +1382,7 @@ def test_ingest_classifies_against_the_local_edit(self, replica, local, remote, def test_an_open_conflict_meets_the_next_report( self, replica, local, opening, following, expected, feed ): - _seeded(replica) - key, object_id = _edit_locally(replica, local) + key, object_id = _seeded_edit(replica, local) _sync(replica, _page("T1", "T2", {object_id: _remote(opening)})) opened_at = replica.record(key).conflict.opened_at head = replica.feed_head() @@ -1143,18 +1394,8 @@ def test_an_open_conflict_meets_the_next_report( assert record.conflict.opened_at == opened_at assert record.conflict.remote_version == (None if following == "removed" else "v3") - def test_an_ingest_leaves_a_push_in_flight_alone(self, replica, store): - _seeded(replica) - with store.transaction(): - key = store.insert_record(_local_record(RecordState.IN_FLIGHT, object_id="f")) - before, head = replica.record(key), replica.feed_head() - _sync(replica, _page("T1", "T2", {"f": _changed("fx", "v2")})) - _sync(replica, _page("T2", "T3", {"f": Removed()})) - assert (replica.record(key), replica.feed_head()) == (before, head) - def test_a_version_only_change_moves_the_version_under_an_edit(self, replica): - _seeded(replica) - key, _ = _edit_locally(replica, "change") + key, _ = _seeded_edit(replica, "change") _sync(replica, _page("T1", "T2", {"a": _changed("fa", "v2", content=b"rewritten")})) record = replica.record(key) assert (record.state, record.version, record.content) == ( @@ -1169,8 +1410,7 @@ def test_a_version_only_change_moves_the_version_under_an_edit(self, replica): [("fx", ConflictKind.BOTH_CHANGED, "v2"), ("removed", ConflictKind.REMOVED_REMOTELY, None)], ) def test_an_open_conflict_waits_for_news(self, replica, opening, kind, known): - _seeded(replica) - key, _ = _edit_locally(replica, "change") + key, _ = _seeded_edit(replica, "change") _sync(replica, _page("T1", "T2", {"a": _remote(opening)})) # The server's version of the conflicted copy, none once it removed it. assert replica.known_versions(_SCOPE).get("a") == known @@ -1211,6 +1451,447 @@ def test_a_scheme_change_makes_edits_compare_by_content(self, replica): assert _feed(replica, head) == [] +class TestPushes: + def test_to_push_lists_pending_edits_by_local_key(self, replica): + _edited(replica) + created = _create(replica, "fn") + with _pushing(replica, "T2", "T3") as session: + assert [r.local_key for r in session.to_push()] == [_key(replica), created] + + def test_a_guarded_push_waits_for_a_version(self, replica): + _seeded(replica) + key = _key(replica) + _change(replica, key, "fl") + _push(replica, key, Applied()) + _change(replica, key, "fy") + created = _create(replica, "fn") + with _pushing(replica, "T2", "T3") as session: + assert [r.local_key for r in session.to_push()] == [key, created] + # The applied push left no version, so a guarded push of the next + # edit would go out unguarded; it waits for an ingest that has one. + with _pushing(replica, "T3", "T4", guarded_by_version=True) as session: + assert [r.local_key for r in session.to_push()] == [created] + assert session.claim(key) is None + _sync(replica, _page("T4", "T5", {"a": _changed("fl", "v5")})) + with _pushing(replica, "T5", "T6", guarded_by_version=True) as session: + assert [r.local_key for r in session.to_push()] == [key, created] + assert _claim(session, key).version == "v5" + + @pytest.mark.parametrize("local, outcome, expected, feed", _OUTCOMES) + def test_settle_records_how_a_push_ended(self, replica, local, outcome, expected, feed): + key, _ = _seeded_edit(replica, local) + head = replica.feed_head() + _push(replica, key, outcome) + record = replica.record(key) + assert _summary(record) == expected + assert [kind for kind, _ in _feed(replica, head)] == feed + if expected == _SYNCED: + # The server may store the edit rewritten, so it gives no version to keep. + assert (record.version, record.fingerprint, record.content) == ( + None, + "fl", + _content("fl"), + ) + if isinstance(outcome, LogicalKeyTaken): + assert record.conflict.holder == outcome.holder + + @pytest.mark.parametrize( + "local, outcome, message", + [ + ("create", Gone(), "can't be gone"), + ("change", LogicalKeyTaken(), "Only a create"), + ("delete", LogicalKeyTaken("b"), "Only a create"), + ("delete", Applied("x"), "no object id"), + ("create without an id", Applied(), "needs the object id"), + ("not claimed", Applied(), "takes a record"), + ], + ) + def test_an_outcome_no_push_can_have_is_refused(self, replica, store, local, outcome, message): + _seeded(replica) + if local == "create without an id": + key = _create(replica, "fl") + else: + key, _ = _edit_locally(replica, "change" if local == "not claimed" else local) + with _pushing(replica) as session: + claimed = _claim(session, key) + if local == "not claimed": + claimed = replace(claimed, state=RecordState.PENDING) + _assert_refused( + replica, + store, + lambda: session.settle(claimed, outcome), + ValueError, + message, + before_a_transaction=True, + ) + + def test_pushing_follows_the_session_order(self, replica, store): + key, _ = _seeded_edit(replica, "change") + + def refused(call: Callable[[], object], match: str) -> None: + _assert_refused(replica, store, call, SessionError, match, before_a_transaction=True) + + with _open_session(replica) as session: + for page in (_page("T1", "C", {}, truncated=True), _page("C", "T2", {})): + refused(session.to_push, "after it ingested its last page") + refused(lambda: session.claim(key), "after it ingested its last page") + session.ingest(page) + unclaimed = replace(replica.record(key), state=RecordState.IN_FLIGHT) + refused(lambda: session.settle(unclaimed, Applied()), "Only a session that claimed") + claimed = _claim(session, key) + refused(lambda: session.ingest(_page("T2", "T3", {})), "already pushed") + refused(session.to_push, "has ended") + refused(lambda: session.claim(key), "has ended") + refused(lambda: session.settle(claimed, Applied()), "has ended") + + def test_claim_passes_over_what_has_nothing_to_push(self, replica): + _edited(replica) + a, b = _key(replica, "a"), _key(replica, "b") + with _pushing(replica, "T2", "T3") as session: + assert session.state is SessionState.INGESTED + assert _claim(session, a).state is RecordState.IN_FLIGHT + assert session.state is SessionState.PUSHING + assert [session.claim(key) for key in (a, b, 999)] == [None, None, None] + + def test_claim_refuses_what_it_cant_push(self, replica, store): + key, _ = _seeded_edit(replica, "change") + _sync(replica, _page(None, "O1", {}, full=True, scope=_OTHER)) + elsewhere = _create(replica, "fn", scope=_OTHER) + with _pushing(replica) as session: + for call, match in [ + (lambda: session.claim(elsewhere), "belongs to scope"), + (lambda: session.claim(key, object_id="x"), "already has object id"), + ]: + _assert_refused(replica, store, call, ValueError, match) + _assert_refused( + replica, + store, + lambda: session.claim(key, object_id=_BAD_TEXT), + ValueError, + "^object_id ", + before_a_transaction=True, + ) + + @pytest.mark.parametrize("at_claim", [True, False], ids=["claimed", "applied"]) + @pytest.mark.parametrize("object_id", ["x", "a", "b"]) + def test_a_create_takes_the_id_it_goes_out_under(self, replica, store, at_claim, object_id): + _a_removed(replica) + key = _create(replica, "fn") + with _pushing(replica, "T2", "T3") as session: + if at_claim: + give = partial(session.claim, key, object_id=object_id) + else: + give = partial(session.settle, _claim(session, key), Applied(object_id)) + if object_id == "b": + _assert_refused(replica, store, give, DuplicateObjectError) + return + give() + # A removed record holding the id hands it over. + assert replica.find_record(_SCOPE, object_id).local_key == key + # The server's copy at that id is the create, whether it's in flight or applied. + _sync(replica, _page("T3", "T4", {object_id: _changed("fx", "v4")})) + assert _summary(replica.record(key)) == _SYNCED + + def test_each_push_is_claimed_and_settled_once(self, make_store, clock): + first = _seeded(Replica(make_store(), clock=clock)) + keys = [_edit_locally(first, "change", object_id)[0] for object_id in "ab"] + keys += [_create(first, f"n{i}", object_id=f"n{i}") for i in range(6)] + head = first.feed_head() + handles = [Replica(make_store(), clock=clock) for _ in range(8)] + + def push_all(session: Session) -> list[int]: + pushed = [] + for key in keys: + claimed = session.claim(key) + if claimed is not None: + assert session.settle(claimed, Applied()) + pushed.append(key) + return pushed + + with ExitStack() as stack: + sessions = [stack.enter_context(_open_session(handle)) for handle in handles] + # Every handle ingests first, since an ingest settles pushes in flight. + for n, session in enumerate(sessions, 1): + session.ingest(_page(f"T{n}", f"T{n + 1}", {})) + with ThreadPoolExecutor(len(sessions)) as pool: + pushed = [ + key for keys_pushed in pool.map(push_all, sessions) for key in keys_pushed + ] + assert sorted(pushed) == sorted(keys) + assert {_summary(first.record(key)) for key in keys} == {_SYNCED} + entries = first.read_feed(head) + assert Counter(e.local_key for e in entries) == Counter(keys) + assert {e.kind for e in entries} == {FeedEntryKind.PUSHED} + + @pytest.mark.parametrize("following, outcome, settled, expected, feed", _SETTLED_LATE) + def test_a_late_outcome_counts_only_for_the_same_edit( + self, make_store, clock, following, outcome, settled, expected, feed + ): + pusher = _seeded(Replica(make_store(), clock=clock)) + syncer = Replica(make_store(), clock=clock) + key, _ = _edit_locally(pusher, "change") + with _pushing(pusher) as session: + claimed = _claim(session, key) + # Another process syncs while the server's answer is on its way. + reported = {} if following in (None, "edited") else {"a": _remote(following, "v3")} + _sync(syncer, _page("T2", "T3", reported)) + if following == "edited": + _change(syncer, key, "fy") + head = pusher.feed_head() + assert session.settle(claimed, outcome) is settled + assert _summary(pusher.record(key)) == expected + assert [kind for kind, _ in _feed(pusher, head)] == feed + + @pytest.mark.parametrize("local, remote, expected, feed", _LEFT_IN_FLIGHT) + def test_the_next_complete_ingest_settles_a_push_left_in_flight( + self, replica, local, remote, expected, feed + ): + key, object_id = _seeded_edit(replica, local) + _push(replica, key, None) + head = replica.feed_head() + reported = {} if remote is None else {object_id: _remote(remote, "v3")} + # A unique key changes nothing for a push that has an id. + with _open_session(replica, unique_logical_key=True) as session: + session.ingest(_page("T2", "C", {}, truncated=True)) + assert _summary(replica.record(key)) == (RecordState.IN_FLIGHT, EditKind(local), None) + session.ingest(_page("C", "T3", reported)) + assert _summary(replica.record(key)) == expected + assert [kind for kind, _ in _feed(replica, head)] == feed + + def test_a_listing_settles_pushes_it_never_mentions(self, replica): + _seeded(replica) + keys = { + "change": _edit_locally(replica, "change")[0], + "delete": _edit_locally(replica, "delete", "b")[0], + "create": _edit_locally(replica, "create")[0], + "create without an id": _create(replica, "fm"), + } + with _pushing(replica) as session: + for key in keys.values(): + session.claim(key) + head = replica.feed_head() + _sync(replica, _page("T2", "L", {}, full=True)) + assert {name: _summary(replica.record(key)) for name, key in keys.items()} == { + "change": _conflicted(EditKind.CHANGE, ConflictKind.REMOVED_REMOTELY), + "delete": _REMOVED, + "create": _pending(EditKind.CREATE), + "create without an id": _pending(EditKind.CREATE), + } + assert [kind for kind, _ in _feed(replica, head)] == [ + FeedEntryKind.CONFLICT_OPENED, + FeedEntryKind.PUSHED, + FeedEntryKind.PUSH_FAILED, + FeedEntryKind.PUSH_FAILED, + ] + + @pytest.mark.parametrize("unique, creates", [(True, 1), (False, 1), (True, 2)]) + def test_a_create_without_an_id_is_known_by_a_unique_key(self, replica, unique, creates): + _seeded(replica) + keys = [_create(replica, "fl", logical_key="u") for _ in range(creates)] + with _pushing(replica) as session: + for key in keys: + session.claim(key) + head = replica.feed_head() + with _open_session(replica, unique_logical_key=unique) as session: + session.ingest(_page("T2", "T3", {"new": _changed("fx", "v3", logical_key="u")})) + new = replica.find_record(_SCOPE, "new") + feed = [kind for kind, _ in _feed(replica, head)] + # Only a key that one create holds, in a scope where keys are unique, + # tells which create the new object is. + if unique and creates == 1: + assert (new.local_key, _summary(new)) == (keys[0], _SYNCED) + assert feed == [FeedEntryKind.PUSHED] + else: + assert new.local_key not in keys + assert [_summary(replica.record(key)) for key in keys] == [ + _pending(EditKind.CREATE) + ] * creates + assert feed == [FeedEntryKind.CHANGED] + [FeedEntryKind.PUSH_FAILED] * creates + + @pytest.mark.parametrize( + "changes, expected", + [ + ({}, _pending(EditKind.CHANGE)), + ({"b": _changed("fx", "v3")}, _conflicted(EditKind.CHANGE, ConflictKind.BOTH_CHANGED)), + ], + ) + def test_a_scope_wide_rejection_is_pushed_again(self, replica, changes, expected): + _seeded(replica) + keys = [_edit_locally(replica, "change", object_id)[0] for object_id in "ab"] + with _pushing(replica) as session: + for key in keys: + session.settle(_claim(session, key), Rejected()) + # Only an object the server changed since is a conflict; the rest go out again. + with _open_session(replica) as session: + session.ingest(_page("T2", "T3", changes)) + to_push = [r.local_key for r in session.to_push()] + assert [_summary(replica.record(key)) for key in keys] == [ + _pending(EditKind.CHANGE), + expected, + ] + assert to_push == [key for key in keys if replica.record(key).state is RecordState.PENDING] + + +class TestResolutions: + @pytest.mark.parametrize("kind, resolution, expected", _RESOLVED) + def test_each_resolution_of_each_conflict(self, replica, kind, resolution, expected): + key = _in_conflict(_seeded(replica), kind=kind) + conflict = replica.record(key).conflict + head = replica.feed_head() + replica.resolve(replica.record(key), resolution) + record = replica.record(key) + assert _summary(record) == expected + assert [kind for kind, _ in _feed(replica, head)] == [FeedEntryKind.CONFLICT_RESOLVED] + if record is not None and record.state is not RecordState.REMOVED: + # Whatever is pushed next goes out against the server's current copy. + assert (record.version, record.content) == ( + conflict.remote_version, + conflict.remote_content, + ) + if isinstance(resolution, Merge): + assert record is not None + assert (record.edit and record.edit.content) == resolution.content + + @pytest.mark.parametrize( + "kind, resolution, message", + [ + *((kind, UpdateHolder(), "^Only a create refused") for kind in _OPENED_BY), + (ConflictKind.LOGICAL_KEY_TAKEN, KeepEdit(), "can't be pushed again unchanged$"), + (None, TakeServer(), "takes a record in conflict$"), + ], + ) + def test_a_resolution_that_does_not_fit_is_refused( + self, replica, store, kind, resolution, message + ): + _seeded(replica) + key = _key(replica) if kind is None else _in_conflict(replica, kind=kind) + record = replica.record(key) + _assert_refused( + replica, + store, + lambda: replica.resolve(record, resolution), + ValueError, + message, + before_a_transaction=True, + ) + + def test_a_merge_the_server_already_holds_is_synced(self, replica): + key = _in_conflict(_seeded(replica)) + replica.resolve(replica.record(key), Merge(_content("fx"), fingerprint="fx")) + assert _summary(replica.record(key)) == _SYNCED + + @pytest.mark.parametrize("following", ["fx", "fy", "removed", "fl"]) + def test_a_conflict_that_moved_on_is_stale(self, replica, store, following): + key = _in_conflict(_seeded(replica)) + read = replica.record(key) + _sync(replica, _page("T2", "T3", {"a": _remote(following, "v3")})) + if following == "fx": + # Only the version moved, and the edit goes out against the new one. + replica.resolve(read, KeepEdit()) + record = replica.record(key) + assert (record.state, record.version) == (RecordState.PENDING, "v3") + else: + _assert_refused( + replica, store, lambda: replica.resolve(read, KeepEdit()), StaleConflictError + ) + + @pytest.mark.parametrize( + "holder, keyed, logical_key", + [ + ("b", [], "u"), + ("b", [], None), + (None, ["b"], "u"), + ("z", ["b"], "u"), + (None, [], "u"), + (None, ["a", "b"], "u"), + # Only b has no key, which isn't the key the create had: it had none. + (None, [], None), + ], + ) + def test_a_refused_create_goes_to_the_key_holder( + self, replica, store, holder, keyed, logical_key + ): + _seeded(replica) + key = _refused_create(replica, holder, logical_key=logical_key) + changes: dict[str, Change] = {"a": Removed()} if logical_key is None else {} + changes |= {o: _changed(f"f{o}", "v2", logical_key="u") for o in keyed} + _sync(replica, _page("T2", "T3", changes)) + refused = replica.record(key) + if holder is None and len(keyed) != 1: + _assert_refused( + replica, store, lambda: replica.resolve(refused, UpdateHolder()), UnknownHolderError + ) + return + replica.resolve(refused, UpdateHolder()) + assert replica.record(key) is None + b = replica.find_record(_SCOPE, "b") + assert _summary(b) == _pending(EditKind.CHANGE) + assert (b.edit.content, b.edit.logical_key) == (_content("fl"), logical_key) + + @pytest.mark.parametrize( + "holding, error", [("conflict", OpenConflictError), ("in flight", PushInFlightError)] + ) + def test_a_holder_that_takes_no_edit(self, replica, store, holding, error): + key = _refused_create(_seeded(replica), "b") + if holding == "conflict": + _in_conflict(replica, "b", "T2", "T3") + else: + _push(replica, _edit_locally(replica, "change", "b")[0], None, "T2", "T3") + refused = replica.record(key) + _assert_refused(replica, store, lambda: replica.resolve(refused, UpdateHolder()), error) + + def test_resolve_conflicts_resolves_each_answer(self, replica): + a, b = _two_conflicts(replica) + # A conflict in another scope comes after them. + _sync(replica, _page(None, "O1", {"a": _changed("fa")}, full=True, scope=_OTHER)) + other = replica.find_record(_OTHER, "a").local_key + _change(replica, other, "fl") + _sync(replica, _page("O1", "O2", {"a": _changed("fx", "v2")}, scope=_OTHER)) + seen = [] + + def resolver(record: Record) -> TakeServer | None: + seen.append(record.local_key) + return None if record.local_key == a else TakeServer() + + assert replica.resolve_conflicts(resolver) == 2 + assert seen == [a, b, other] + assert [_summary(replica.record(key)) for key in (a, b, other)] == [ + _conflicted(EditKind.CHANGE, ConflictKind.BOTH_CHANGED), + _SYNCED, + _SYNCED, + ] + assert replica.resolve_conflicts(lambda record: None) == 0 + + def test_resolve_conflicts_skips_a_conflict_that_moved_on(self, replica): + a, b = _two_conflicts(replica) + + def resolver(record: Record) -> TakeServer: + if record.local_key == a: + # A sync brings news about b while the application decides about a. + _sync(replica, _page("T3", "T4", {"b": _changed("fy", "v4")})) + return TakeServer() + + assert replica.resolve_conflicts(resolver) == 1 + assert replica.record(b).state is RecordState.CONFLICT + assert replica.resolve_conflicts(lambda record: TakeServer()) == 1 + assert replica.record(b).content == _content("fy") + + def test_an_error_from_the_resolver_keeps_earlier_resolutions(self, replica): + a, b = _two_conflicts(replica) + + def resolver(record: Record) -> TakeServer: + if record.local_key == b: + raise RuntimeError("the application failed") + return TakeServer() + + with pytest.raises(RuntimeError, match="the application failed"): + replica.resolve_conflicts(resolver) + assert [replica.record(key).state for key in (a, b)] == [ + RecordState.SYNCED, + RecordState.CONFLICT, + ] + + class TestLocalCopy: def test_find_record(self, replica): _a_removed(replica) @@ -1221,7 +1902,7 @@ def test_find_record(self, replica): def test_find_by_logical_key(self, replica): objects = {"a": _changed("fa", logical_key="u1"), "b": _changed("fb", logical_key="u2")} _sync(replica, _page(None, "T1", objects, full=True)) - created = replica.create(_SCOPE, _content("fn"), fingerprint="fn", logical_key="u1") + created = _create(replica, "fn", logical_key="u1") _sync(replica, _page("T1", "T2", {"a": Removed()})) found = replica.find_by_logical_key(_SCOPE, "u1") assert [r.local_key for r in found] == [_key(replica), created] @@ -1238,12 +1919,23 @@ def test_content_and_strings_round_trip_exactly(self, replica, content, text): assert replica.record(created).edit.content == content def test_invalid_text_and_content_are_refused(self, replica, store): - with pytest.raises(ValueError, match="^fingerprint holds a lone surrogate at 1"): - Changed("v1", _BAD_TEXT, b"c") - with pytest.raises(TypeError, match="^content must be bytes, not str"): - Changed("v1", "fa", "text of fa") # type: ignore[arg-type] - with pytest.raises(ValueError, match="^object_ids "): - _page(None, "T1", {_BAD_TEXT: Removed()}) + # The values an adapter or the application builds check themselves. + built: list[tuple[Callable[[], object], type[Exception], str]] = [ + ( + lambda: Changed("v1", _BAD_TEXT, b"c"), + ValueError, + "^fingerprint holds a lone surrogate at 1", + ), + (lambda: Changed("v1", "fa", _NOT_BYTES), TypeError, "^content must be bytes, not str"), + (lambda: _page(None, "T1", {_BAD_TEXT: Removed()}), ValueError, "^object_ids "), + (lambda: Applied(_BAD_TEXT), ValueError, "^object_id "), + (lambda: LogicalKeyTaken(_BAD_TEXT), ValueError, "^holder "), + (lambda: Merge(b"c", fingerprint=_BAD_TEXT), ValueError, "^fingerprint "), + (lambda: Merge(_NOT_BYTES, fingerprint="f"), TypeError, "^content "), + ] + for build, error, match in built: + with pytest.raises(error, match=match): + build() _seeded(replica) key = _key(replica) refused: list[tuple[Callable[[], object], type[Exception], str]] = [ @@ -1260,8 +1952,8 @@ def test_invalid_text_and_content_are_refused(self, replica, store): ValueError, "^logical_key ", ), - (lambda: replica.create(_SCOPE, "c", fingerprint="f"), TypeError, "^content "), # type: ignore[arg-type] - (lambda: replica.change(key, "c", fingerprint="f"), TypeError, "^content "), # type: ignore[arg-type] + (lambda: replica.create(_SCOPE, _NOT_BYTES, fingerprint="f"), TypeError, "^content "), + (lambda: replica.change(key, _NOT_BYTES, fingerprint="f"), TypeError, "^content "), ] for call, error, match in refused: _assert_refused(replica, store, call, error, match, before_a_transaction=True) @@ -1373,14 +2065,13 @@ def test_threads_share_one_store(self, replica): def write(n: int) -> None: scope = f"scope-{n}" objects = {f"o{i}": _changed(f"f{n}-{i}") for i in range(20)} - with _open_session(replica, scope) as session: - session.ingest(_page(None, "T1", objects, full=True, scope=scope)) + _sync(replica, _page(None, "T1", objects, full=True, scope=scope)) if n % 2: replica.drop_scope(scope) def edit(n: int) -> None: for i in range(3): - key = replica.create(_SCOPE, _content(f"n{n}-{i}"), fingerprint=f"n{n}-{i}") + key = _create(replica, f"n{n}-{i}") _change(replica, key, f"m{n}-{i}") def read(_: int) -> None: @@ -1425,6 +2116,27 @@ def test_object_ids_are_unique_per_scope(self, replica, store): with store.transaction(), pytest.raises(DuplicateObjectError): store.update_record(_local_record(b.state, object_id="a", local_key=b.local_key)) + @pytest.mark.parametrize( + "states, expected", + [ + (None, ["a", "b"]), + (frozenset(), []), + (frozenset({RecordState.PENDING}), ["a"]), + (frozenset({RecordState.PENDING, RecordState.CONFLICT}), ["a", "b"]), + (frozenset({RecordState.SYNCED, RecordState.REMOVED}), []), + ], + ) + def test_scope_records_by_state(self, replica, store, states, expected): + _edited(replica) + with ( + store.transaction(write=False), + patch.object(store, "_scope_records", wraps=store._scope_records) as hook, + ): + records = store.scope_records(_SCOPE, states=states) + assert [r.object_id for r in records] == expected + # An empty set is answered by the base class, so no store sees one. + assert hook.called == (states != frozenset()) + @pytest.mark.parametrize("record", _BAD_RECORDS.values(), ids=_BAD_RECORDS) def test_records_holding_what_their_state_cant_are_refused(self, store, record): with _write_with_scope(store): @@ -1438,14 +2150,13 @@ def test_records_holding_what_their_state_cant_are_refused(self, store, record): with store.transaction(write=False): assert store.scope_records(_SCOPE) == [_local_record(RecordState.SYNCED, local_key=key)] - @pytest.mark.parametrize("conflict", [_CONFLICT, _GONE], ids=["kept", "removed"]) - def test_a_refused_create_may_hold_either_server_side(self, store, conflict): - edit = replace(_EDIT, kind=EditKind.CREATE) + @pytest.mark.parametrize("holder", [None, "b"]) + def test_a_refused_create_keeps_its_holder(self, store, holder): record = replace( - _local_record(RecordState.CONFLICT), + _CONFLICTED, content=None, - edit=edit, - conflict=replace(conflict, kind=ConflictKind.LOGICAL_KEY_TAKEN), + edit=replace(_EDIT, kind=EditKind.CREATE), + conflict=replace(_GONE, kind=ConflictKind.LOGICAL_KEY_TAKEN, holder=holder), ) with _write_with_scope(store): key = store.insert_record(record) @@ -1616,10 +2327,9 @@ def _close(self) -> None: def test_state_survives_reopening(self, db_path, clock): with SQLiteStore(db_path) as store: replica = _edited(Replica(store, clock=clock)) - replica.create( - _SCOPE, - _content("fn"), - fingerprint="fn", + created = _create( + replica, + "fn", containers=frozenset({"x", "y"}), logical_key="u", ) @@ -1629,11 +2339,15 @@ def test_state_survives_reopening(self, db_path, clock): _sync(replica, _page("T2", "T3", changes)) replica.delete(_key(replica, "c")) _sync(replica, _page("T3", "T4", {"d": Removed()})) + # A create refused with the object holding its key, and a push in flight. + with _pushing(replica, "T4", "T5") as session: + session.settle(_claim(session, created), LogicalKeyTaken("b")) + session.claim(_key(replica)) expected = _snapshot(replica, store) assert [_summary(r) for r in replica.records(_SCOPE)] == [ - _pending(EditKind.CHANGE), + (RecordState.IN_FLIGHT, EditKind.CHANGE, None), _conflicted(EditKind.CHANGE, ConflictKind.REMOVED_REMOTELY), - _pending(EditKind.CREATE), + _conflicted(EditKind.CREATE, ConflictKind.LOGICAL_KEY_TAKEN), _pending(EditKind.DELETE), _REMOVED, ] From 3166a94e89d53e2772ed76e263f62527f20fc424 Mon Sep 17 00:00:00 2001 From: Sashank Bhamidi Date: Wed, 21 Oct 2026 15:25:49 +0530 Subject: [PATCH 3/3] fix: keep local edits, removed records and claims safe while pushing --- docs/explanation/design.rst | 6 +- src/calendaring_sync/__init__.py | 2 + src/calendaring_sync/_classify.py | 7 -- src/calendaring_sync/error.py | 7 ++ src/calendaring_sync/objects/feed.py | 3 +- src/calendaring_sync/objects/record.py | 4 + src/calendaring_sync/replica.py | 68 ++++++++++----- src/calendaring_sync/store/sqlite.py | 3 +- .../tests/test_classify_unit.py | 24 ++---- src/calendaring_sync/tests/test_sync_unit.py | 84 ++++++++++++++----- 10 files changed, 135 insertions(+), 73 deletions(-) diff --git a/docs/explanation/design.rst b/docs/explanation/design.rst index 6dc04de..be1e512 100644 --- a/docs/explanation/design.rst +++ b/docs/explanation/design.rst @@ -132,12 +132,12 @@ Before a push goes out, the adapter :term:`claims ` it: the core marks it An unknown push stays in flight, and so does one whose process stopped before the answer arrived. The next complete :term:`ingest` of its scope, in any process, settles it: -- a create that comes back under the id it chose reached the server, and the core stores the server's copy; where the server picks the id, a create that comes back with the logical key it was sent with counts the same, but only if the adapter says that key is unique +- a create that comes back with what you sent, under the id it chose, reached the server; where the server picks the id, a new object with the same logical key and what you sent counts the same, but only if the adapter says that key is unique. A different object at the id the create chose is a conflict, and one that only shares its logical key is another object, so the create goes out again - an update that comes back with your fingerprint reached the server, and one that comes back with another fingerprint is a conflict, since someone else changed the object too - a delete that comes back removed reached the server - a push the change set doesn't mention didn't reach the server, and its edit is pending again -Treating an unknown push as failed could create the same object twice. An outcome that arrives after an ingest settled its push still counts while the record holds the same edit, and changes nothing once the edit has changed or the server's side decided it. Two processes syncing one scope at the same moment can therefore send one edit twice: the second write is rejected where the adapter guards it with the object's version, and nothing is lost either way. +Treating an unknown push as failed could create the same object twice. An outcome that arrives after an ingest settled its push still counts while the record holds the same edit and no one has claimed it again, and changes nothing otherwise. Two processes syncing one scope at the same moment can therefore send one edit twice. A second change or delete is rejected where the adapter guards it with the object's version. A create whose id the server picks has no such guard, so the server can end up with the object twice, unless the adapter says logical keys are unique: then the server refuses the second create for its key, which becomes a conflict. Nothing you edited is lost either way. Why the server's answer to a write isn't trusted ================================================ @@ -270,7 +270,7 @@ Later remote changes update the record's remote side. The conflict resolves itse The core stores conflicts instead of raising them: an exception for one object would abort every other change in the sync, and a :term:`resolver` called during a sync would run again on every retry. You resolve a record with your edit (``KeepEdit``), the server's version (``TakeServer``), merged content (``Merge``), or a delete (``Delete``), and the core pushes the result as a change to the server's current version. You can't push a create the server refused for its UID again unchanged, so you can discard it (``TakeServer`` or ``Delete``), push it as an update to the object that holds the UID (``UpdateHolder``), or replace it with merged content, possibly with a new UID (``Merge``). -``resolve(record, resolution)`` resolves one record you read, and refuses if its conflict changed since, so a decision is never applied to news you haven't seen. ``resolve_conflicts(resolver)`` passes each open record to your resolver, outside any sync, and resolves each answer the same way. Each resolution is its own transaction: ``None`` leaves a record open, a record that changed while your resolver ran waits for the next call, and an exception stops the loop and keeps the resolutions before it. +``resolve(record, resolution)`` resolves one record you read, and refuses if its conflict changed since, so a decision is never applied to news you haven't seen. ``resolve_conflicts(resolver)`` passes each open record to your resolver, never during an ingest, and resolves each answer the same way. Each resolution is its own transaction: ``None`` leaves a record open, a record that changed while your resolver ran waits for the next call, and an exception stops the loop and keeps the resolutions before it. A removal on the server against a local edit is always a conflict, never resolved automatically, because that would delete your edit without asking. diff --git a/src/calendaring_sync/__init__.py b/src/calendaring_sync/__init__.py index 42151c3..4d86ea9 100644 --- a/src/calendaring_sync/__init__.py +++ b/src/calendaring_sync/__init__.py @@ -15,6 +15,7 @@ DuplicateObjectError, EditRefusedError, OpenConflictError, + PendingEditError, PushInFlightError, RecordValueError, RemovedRecordError, @@ -85,6 +86,7 @@ "MemoryStore", "Merge", "OpenConflictError", + "PendingEditError", "PushInFlightError", "PushOutcome", "Record", diff --git a/src/calendaring_sync/_classify.py b/src/calendaring_sync/_classify.py index 25c8458..9720ab7 100644 --- a/src/calendaring_sync/_classify.py +++ b/src/calendaring_sync/_classify.py @@ -73,8 +73,6 @@ def classify( synced: Record | None, remote: Changed | Removed | None, local: LocalEdit | None, - *, - in_flight: bool = False, ) -> tuple[ChangeClass, ConflictKind | None]: """Classify one object from its synced side, the server's report and the local edit. @@ -89,9 +87,6 @@ def classify( remote: The server's new copy, ``Removed`` if the server removed it, or ``None`` if the server reported nothing new. local: The record's :term:`pending edit`, or ``None``. - in_flight: Whether ``local`` was pushed and the push's outcome is - unknown. A create that then comes back at its own id is your - own write, whatever the server made of its content. Returns: The class, and the conflict's kind when the class is ``CONFLICT``. @@ -132,8 +127,6 @@ def classify( case EditKind.DELETE, Removed(): # Both sides deleted the object. return ChangeClass.DELETED_REMOTE, None - case EditKind.CREATE, Changed() if in_flight: - return ChangeClass.UNCHANGED, None case EditKind.CREATE, Changed(): return ChangeClass.CONFLICT, ConflictKind.BOTH_CREATED case _: diff --git a/src/calendaring_sync/error.py b/src/calendaring_sync/error.py index 585a05e..3a7c8b6 100644 --- a/src/calendaring_sync/error.py +++ b/src/calendaring_sync/error.py @@ -94,6 +94,13 @@ class UnknownHolderError(CalendaringSyncError): """ +class PendingEditError(CalendaringSyncError): + """The :term:`record` holds a :term:`pending edit` of its own, which this would replace. + + Push or discard that edit first, then try again. + """ + + class ScopeMismatchError(CalendaringSyncError): """A :term:`session` was opened with a media type the :term:`scope` doesn't have. diff --git a/src/calendaring_sync/objects/feed.py b/src/calendaring_sync/objects/feed.py index a178355..e3104c0 100644 --- a/src/calendaring_sync/objects/feed.py +++ b/src/calendaring_sync/objects/feed.py @@ -45,7 +45,8 @@ class FeedEntry: kind: What happened. local_key: The :term:`local key` of the record the entry is about, ``None`` for a dropped scope. The record may since have been - :term:`purged `. + :term:`purged `, or deleted if it was a create the server + never had. object_id: The record's :term:`object id` when the entry was written, ``None`` for a dropped scope. at: When the entry was written. diff --git a/src/calendaring_sync/objects/record.py b/src/calendaring_sync/objects/record.py index f40ebf0..f7e300f 100644 --- a/src/calendaring_sync/objects/record.py +++ b/src/calendaring_sync/objects/record.py @@ -67,6 +67,9 @@ class Record: flight or in conflict, and ``None`` otherwise. conflict: The server's side of an open :term:`conflict`, held while the record is in conflict, and ``None`` otherwise. + claims: How many times a :term:`push` of the record was + :term:`claimed `. How a push ended only counts while no + later claim has been made. """ local_key: int @@ -82,6 +85,7 @@ class Record: last_entry: int edit: LocalEdit | None = None conflict: Conflict | None = None + claims: int = 0 @property def is_create(self) -> bool: diff --git a/src/calendaring_sync/replica.py b/src/calendaring_sync/replica.py index d60e43a..30c26a4 100644 --- a/src/calendaring_sync/replica.py +++ b/src/calendaring_sync/replica.py @@ -29,6 +29,7 @@ from calendaring_sync.error import ( DuplicateObjectError, OpenConflictError, + PendingEditError, PushInFlightError, RemovedRecordError, ScopeMismatchError, @@ -220,10 +221,10 @@ def _waits_for_version(record: Record, *, guarded: bool) -> bool: return guarded and record.version is None and not record.is_create -def _check_outcome(scope_id: str, claimed: Record, outcome: PushOutcome) -> None: +def _check_outcome(claimed_keys: set[int], claimed: Record, outcome: PushOutcome) -> None: """Raise ``ValueError`` unless the outcome is one a push of the claimed edit can have.""" if ( - claimed.scope != scope_id + claimed.local_key not in claimed_keys or claimed.state is not RecordState.IN_FLIGHT or claimed.edit is None ): @@ -292,6 +293,8 @@ def __init__( self._listing: bool | None = None self._mentioned: set[str] = set() self._closed = False + # The local keys this session claimed, the only ones it settles. + self._claimed: set[int] = set() @property def scope_id(self) -> str: @@ -429,6 +432,7 @@ def claim(self, local_key: int, *, object_id: str | None = None) -> Record | Non ) if claimed is not None: self._state = SessionState.PUSHING + self._claimed.add(local_key) return claimed def settle(self, claimed: Record, outcome: PushOutcome) -> bool: @@ -438,7 +442,7 @@ def settle(self, claimed: Record, outcome: PushOutcome) -> bool: feed` entry for every outcome but ``Unknown``. If another process already settled this push, for example by ingesting while it was in flight, the outcome only counts while the record still holds the - same edit. + same edit and no one has claimed it again. Args: claimed: The record :meth:`claim` returned. @@ -451,14 +455,14 @@ def settle(self, claimed: Record, outcome: PushOutcome) -> bool: Raises: TransactionError: If a store transaction is already open in this thread. - SessionError: If the session isn't pushing, or has ended. + SessionError: If the session has ended. DuplicateObjectError: If ``Applied`` names an id a record that isn't removed holds. - ValueError: If ``claimed`` isn't a claimed record of this scope, - or the outcome is one no push of its edit can have: ``Gone`` - for a create, ``LogicalKeyTaken`` for anything but a create, - ``Applied`` with an id for a delete, or ``Applied`` without - one for a create that has none. + ValueError: If ``claimed`` isn't a record this session's + :meth:`claim` returned, or the outcome is one no push of its + edit can have: ``Gone`` for a create, ``LogicalKeyTaken`` for + anything but a create, ``Applied`` with an id for a delete, or + ``Applied`` without one for a create that has none. Examples: The server refused the write, so the edit is pending again: @@ -475,11 +479,9 @@ def settle(self, claimed: Record, outcome: PushOutcome) -> bool: >>> replica.record(key).state """ - _check_outcome(self._scope_id, claimed, outcome) + _check_outcome(self._claimed, claimed, outcome) if self._closed: raise SessionError("The session has ended") - if self._state is not SessionState.PUSHING: - raise SessionError("Only a session that claimed a push can settle one") return self._replica._settle(claimed, outcome) def _check_ready(self) -> None: @@ -587,8 +589,8 @@ def session( record has no version, so it never goes out unguarded. unique_logical_key: Whether no two objects in the scope can share a :term:`logical key`. A create whose answer never arrived is - then recognized by its key at the next ingest, which a create - the server picks the id for needs. + then recognized at the next ingest by its key and the data it + was sent with, which a create the server picks the id for needs. Raises: TransactionError: If a store transaction is already open in this thread. @@ -722,8 +724,12 @@ def _apply( "but the store has no version for it" ) return - if record is None and isinstance(change, Changed) and change.logical_key in claimed: - record = claimed.pop(change.logical_key) + if record is None and isinstance(change, Changed) and change.logical_key is not None: + # A new object with the key is the create only if it holds what was sent; + # anything else is someone else's object, and the create goes out again. + create = claimed.get(change.logical_key) + if create is not None and create.edit is not None and same_data(change, create.edit): + record = claimed.pop(change.logical_key) in_flight = record is not None and record.state is RecordState.IN_FLIGHT live = None if record is None or record.state is RecordState.REMOVED else record if live is not None and live.conflict is not None and same_remote(live.conflict, change): @@ -733,7 +739,7 @@ def _apply( return edit = None if live is None else live.edit synced = live if live is not None and live.content is not None else None - change_class, kind = classify(synced, change, edit, in_flight=in_flight) + change_class, kind = classify(synced, change, edit) was_conflict = live is not None and live.conflict is not None # A record leaving a conflict is reported as the conflict ending, and # one whose push was in flight as the push reaching the server. @@ -1127,7 +1133,10 @@ def _claim( if object_id is not None: self._free_object_id(scope_id, object_id, record.local_key) claimed = replace( - record, state=RecordState.IN_FLIGHT, object_id=object_id or record.object_id + record, + state=RecordState.IN_FLIGHT, + object_id=object_id or record.object_id, + claims=record.claims + 1, ) self._store.update_record(claimed) return claimed @@ -1136,9 +1145,12 @@ def _settle(self, claimed: Record, outcome: PushOutcome) -> bool: now = self._clock() with self._store.transaction(): record = self._store.get_record(claimed.local_key) + # Only the latest claim of the same edit counts: another process may + # have settled this push, and claimed the edit again since. if ( record is None or record.edit != claimed.edit + or record.claims != claimed.claims or record.state not in (RecordState.IN_FLIGHT, RecordState.PENDING) ): return False @@ -1183,14 +1195,17 @@ def _open_conflict( ) def _free_object_id(self, scope_id: str, object_id: str, owner: int) -> None: - """Make ``object_id`` free for record ``owner``: a removed record holding it is deleted. + """Make ``object_id`` free for record ``owner``. + + A removed record holding it gives the id up but stays, as a + :term:`change feed` reader may still read it, until it's purged. Raises: DuplicateObjectError: If a record that isn't removed holds it. """ removed = self._removed_holder(scope_id, object_id, owner) if removed is not None: - self._store.delete_record(removed.local_key) + self._store.update_record(replace(removed, object_id=None)) def _removed_holder( self, scope_id: str, object_id: str, owner: int | None = None @@ -1354,6 +1369,8 @@ def resolve(self, record: Record, resolution: Resolution) -> None: refused create's logical key, or it was sent without one. OpenConflictError: For ``UpdateHolder``, if the holder is in conflict. PushInFlightError: For ``UpdateHolder``, if the holder's push is in flight. + PendingEditError: For ``UpdateHolder``, if the holder holds a + pending edit of its own, which this would replace. ValueError: If ``record`` isn't in conflict, or the resolution doesn't fit its kind: ``KeepEdit`` for a create the server refused for its logical key, or ``UpdateHolder`` for any other. @@ -1383,6 +1400,10 @@ def resolve(self, record: Record, resolution: Resolution) -> None: stored = cast(Record, stored) if isinstance(resolution, UpdateHolder): holder = self._editable(self._holder(stored).local_key) + if holder.edit is not None: + raise PendingEditError( + f"Record {holder.local_key} holds a pending edit of its own" + ) edit = cast(LocalEdit, stored.edit) self._store.update_record(_changed_to(holder, replace(edit, recorded_at=now))) resolved = None @@ -1457,8 +1478,9 @@ def _holder(self, record: Record) -> Record: """The one live record holding a refused create's logical key. The id the server named comes first; without one, or before that - object is synced, the key decides. A create sent without a key has - only the id to go by. + object is synced, the key of a synced copy decides, since the server + refused the create for an object it has. A create sent without a key + has only the id to go by. Raises: UnknownHolderError: If no single record holds it. @@ -1476,7 +1498,7 @@ def _holder(self, record: Record) -> Record: holders = [ other for other in keyed - if other.local_key != record.local_key and other.state is not RecordState.REMOVED + if other.logical_key == edit.logical_key and other.state is not RecordState.REMOVED ] if len(holders) != 1: raise UnknownHolderError( diff --git a/src/calendaring_sync/store/sqlite.py b/src/calendaring_sync/store/sqlite.py index 69aea37..0c65ffc 100644 --- a/src/calendaring_sync/store/sqlite.py +++ b/src/calendaring_sync/store/sqlite.py @@ -258,7 +258,8 @@ def _marks(columns: tuple[_Column, ...]) -> str: conflict_remote_containers TEXT, conflict_remote_logical_key TEXT, conflict_opened_at TEXT, - conflict_holder TEXT + conflict_holder TEXT, + claims INTEGER NOT NULL )""", f"""CREATE UNIQUE INDEX {_RECORDS}_by_object_id ON {_RECORDS} (scope, object_id)""", f"""CREATE INDEX {_RECORDS}_by_state ON {_RECORDS} (state, last_entry)""", diff --git a/src/calendaring_sync/tests/test_classify_unit.py b/src/calendaring_sync/tests/test_classify_unit.py index dbcad0c..b0a12c6 100644 --- a/src/calendaring_sync/tests/test_classify_unit.py +++ b/src/calendaring_sync/tests/test_classify_unit.py @@ -84,36 +84,24 @@ ("none", "fx", "create"): (_CONFLICT, ConflictKind.BOTH_CREATED), } -# A push in flight whose outcome never arrived classifies the same, except -# that a create coming back at its own id is your own write, whatever the -# server made of its content. -_IN_FLIGHT = _EXPECTED | { - ("none", "fa", "create"): (ChangeClass.UNCHANGED, None), - ("none", "fx", "create"): (ChangeClass.UNCHANGED, None), -} - _EVERY = list(product(_SYNCED, _REMOTES, _LOCALS)) _IMPOSSIBLE = [sides for sides in _EVERY if sides not in _EXPECTED] -def _classify( - synced: str, remote: str, local: str, *, in_flight: bool = False -) -> tuple[ChangeClass, ConflictKind | None]: - return classify(_SYNCED[synced], _REMOTES[remote], _LOCALS[local], in_flight=in_flight) +def _classify(synced: str, remote: str, local: str) -> tuple[ChangeClass, ConflictKind | None]: + return classify(_SYNCED[synced], _REMOTES[remote], _LOCALS[local]) -@pytest.mark.parametrize("in_flight, expected", [(False, _EXPECTED), (True, _IN_FLIGHT)]) @pytest.mark.parametrize("sides", _EXPECTED, ids="-".join) -def test_every_possible_combination(sides, in_flight, expected): - assert _classify(*sides, in_flight=in_flight) == expected[sides] +def test_every_possible_combination(sides): + assert _classify(*sides) == _EXPECTED[sides] -@pytest.mark.parametrize("in_flight", [False, True]) @pytest.mark.parametrize("sides", _IMPOSSIBLE, ids="-".join) -def test_combinations_no_object_can_have_raise(sides, in_flight): +def test_combinations_no_object_can_have_raise(sides): synced, _, local = sides with pytest.raises(ValueError): - _classify(*sides, in_flight=in_flight) + _classify(*sides) # Only these: nothing at all, a create of an object with a synced copy, # or a change or delete of one without. assert sides == ("none", "none", "none") or (local == "create") == (synced != "none") diff --git a/src/calendaring_sync/tests/test_sync_unit.py b/src/calendaring_sync/tests/test_sync_unit.py index 8d5c26b..1e9060f 100644 --- a/src/calendaring_sync/tests/test_sync_unit.py +++ b/src/calendaring_sync/tests/test_sync_unit.py @@ -48,6 +48,7 @@ MemoryStore, Merge, OpenConflictError, + PendingEditError, PushInFlightError, PushOutcome, Record, @@ -1208,14 +1209,20 @@ def _conflicted(edit: EditKind, kind: ConflictKind) -> tuple[object, ...]: ("create", None, _pending(EditKind.CREATE), [FeedEntryKind.PUSH_FAILED]), ("create", "removed", _pending(EditKind.CREATE), [FeedEntryKind.PUSH_FAILED]), ("create", "fl", _SYNCED, [FeedEntryKind.PUSHED]), - # The server rewrote what it stored, and it's still your own write. - ("create", "fx", _SYNCED, [FeedEntryKind.PUSHED]), + # Another object at the create's id isn't the create. + ( + "create", + "fx", + _conflicted(EditKind.CREATE, ConflictKind.BOTH_CREATED), + [FeedEntryKind.CONFLICT_OPENED], + ), ] # An outcome arriving after another process's ingest already settled the # push: what that ingest saw (None for nothing, "edited" for nothing and -# then a new local edit), the outcome, then what settle() returns, what the -# record ends up as and the feed entries settling wrote. +# then a new local edit, "claimed again" for nothing and then a claim of its +# own), the outcome, then what settle() returns, what the record ends up as +# and the feed entries settling wrote. _SETTLED_LATE = [ (None, Applied(), True, _SYNCED, [FeedEntryKind.PUSHED]), (None, Rejected(), True, _pending(EditKind.CHANGE), []), @@ -1223,8 +1230,15 @@ def _conflicted(edit: EditKind, kind: ConflictKind) -> tuple[object, ...]: ("edited", Applied(), False, _pending(EditKind.CHANGE), []), ("fl", Applied(), False, _SYNCED, []), ("fx", Rejected(), False, _conflicted(EditKind.CHANGE, ConflictKind.BOTH_CHANGED), []), + ("claimed again", Rejected(), False, (RecordState.IN_FLIGHT, EditKind.CHANGE, None), []), ] +# Records settle() refuses because this session didn't claim them so. +_UNCLAIMED = ("not claimed", "another record") + +# The rows of _SETTLED_LATE where the server reported nothing new. +_NOTHING_NEW = (None, "edited", "claimed again") + _MERGED = Merge(_content("fm"), fingerprint="fm") # Each conflict resolved each way: the conflict's kind, the resolution, then @@ -1504,18 +1518,22 @@ def test_settle_records_how_a_push_ended(self, replica, local, outcome, expected ("delete", Applied("x"), "no object id"), ("create without an id", Applied(), "needs the object id"), ("not claimed", Applied(), "takes a record"), + ("another record", Applied(), "takes a record"), ], ) def test_an_outcome_no_push_can_have_is_refused(self, replica, store, local, outcome, message): + """Including a record this session didn't claim, or not as it claimed it.""" _seeded(replica) if local == "create without an id": key = _create(replica, "fl") else: - key, _ = _edit_locally(replica, "change" if local == "not claimed" else local) + key, _ = _edit_locally(replica, "change" if local in _UNCLAIMED else local) with _pushing(replica) as session: claimed = _claim(session, key) if local == "not claimed": claimed = replace(claimed, state=RecordState.PENDING) + if local == "another record": + claimed = replace(claimed, local_key=_key(replica, "b")) _assert_refused( replica, store, @@ -1536,8 +1554,6 @@ def refused(call: Callable[[], object], match: str) -> None: refused(session.to_push, "after it ingested its last page") refused(lambda: session.claim(key), "after it ingested its last page") session.ingest(page) - unclaimed = replace(replica.record(key), state=RecordState.IN_FLIGHT) - refused(lambda: session.settle(unclaimed, Applied()), "Only a session that claimed") claimed = _claim(session, key) refused(lambda: session.ingest(_page("T2", "T3", {})), "already pushed") refused(session.to_push, "has ended") @@ -1576,6 +1592,7 @@ def test_claim_refuses_what_it_cant_push(self, replica, store): @pytest.mark.parametrize("object_id", ["x", "a", "b"]) def test_a_create_takes_the_id_it_goes_out_under(self, replica, store, at_claim, object_id): _a_removed(replica) + removed = replica.find_record(_SCOPE, "a") key = _create(replica, "fn") with _pushing(replica, "T2", "T3") as session: if at_claim: @@ -1586,10 +1603,13 @@ def test_a_create_takes_the_id_it_goes_out_under(self, replica, store, at_claim, _assert_refused(replica, store, give, DuplicateObjectError) return give() - # A removed record holding the id hands it over. assert replica.find_record(_SCOPE, object_id).local_key == key - # The server's copy at that id is the create, whether it's in flight or applied. - _sync(replica, _page("T3", "T4", {object_id: _changed("fx", "v4")})) + # A removed record holding the id gives it up, and stays until it's purged. + kept = replace(removed, object_id=None) if object_id == "a" else removed + assert replica.record(removed.local_key) == kept + assert replica.purge(replica.feed_head()) == 1 + # What was sent, at that id, is the create, whether it's in flight or applied. + _sync(replica, _page("T3", "T4", {object_id: _changed("fn", "v4")})) assert _summary(replica.record(key)) == _SYNCED def test_each_push_is_claimed_and_settled_once(self, make_store, clock): @@ -1633,10 +1653,12 @@ def test_a_late_outcome_counts_only_for_the_same_edit( with _pushing(pusher) as session: claimed = _claim(session, key) # Another process syncs while the server's answer is on its way. - reported = {} if following in (None, "edited") else {"a": _remote(following, "v3")} + reported = {} if following in _NOTHING_NEW else {"a": _remote(following, "v3")} _sync(syncer, _page("T2", "T3", reported)) if following == "edited": _change(syncer, key, "fy") + if following == "claimed again": + _push(syncer, key, None, "T3", "T4") head = pusher.feed_head() assert session.settle(claimed, outcome) is settled assert _summary(pusher.record(key)) == expected @@ -1684,8 +1706,13 @@ def test_a_listing_settles_pushes_it_never_mentions(self, replica): FeedEntryKind.PUSH_FAILED, ] - @pytest.mark.parametrize("unique, creates", [(True, 1), (False, 1), (True, 2)]) - def test_a_create_without_an_id_is_known_by_a_unique_key(self, replica, unique, creates): + @pytest.mark.parametrize( + "unique, creates, fingerprint", + [(True, 1, "fl"), (True, 1, "fx"), (False, 1, "fl"), (True, 2, "fl")], + ) + def test_a_create_without_an_id_is_known_by_a_unique_key( + self, replica, unique, creates, fingerprint + ): _seeded(replica) keys = [_create(replica, "fl", logical_key="u") for _ in range(creates)] with _pushing(replica) as session: @@ -1693,12 +1720,13 @@ def test_a_create_without_an_id_is_known_by_a_unique_key(self, replica, unique, session.claim(key) head = replica.feed_head() with _open_session(replica, unique_logical_key=unique) as session: - session.ingest(_page("T2", "T3", {"new": _changed("fx", "v3", logical_key="u")})) + session.ingest(_page("T2", "T3", {"new": _changed(fingerprint, "v3", logical_key="u")})) new = replica.find_record(_SCOPE, "new") feed = [kind for kind, _ in _feed(replica, head)] # Only a key that one create holds, in a scope where keys are unique, - # tells which create the new object is. - if unique and creates == 1: + # tells which create the new object is, and only if it holds what was + # sent; otherwise it's someone else's object, and the create goes out again. + if unique and creates == 1 and fingerprint == "fl": assert (new.local_key, _summary(new)) == (keys[0], _SYNCED) assert feed == [FeedEntryKind.PUSHED] else: @@ -1804,7 +1832,7 @@ def test_a_conflict_that_moved_on_is_stale(self, replica, store, following): ("z", ["b"], "u"), (None, [], "u"), (None, ["a", "b"], "u"), - # Only b has no key, which isn't the key the create had: it had none. + # The create had no key, so only an object the server named can hold it. (None, [], None), ], ) @@ -1813,7 +1841,14 @@ def test_a_refused_create_goes_to_the_key_holder( ): _seeded(replica) key = _refused_create(replica, holder, logical_key=logical_key) - changes: dict[str, Change] = {"a": Removed()} if logical_key is None else {} + if logical_key is None: + # Without a key, nothing but the holder the server named is left to pick. + changes: dict[str, Change] = {o: Removed() for o in "ab" if o != holder} + unsent = None + else: + # A create not sent yet holds the key too, but the server doesn't have it. + changes = {} + unsent = _create(replica, "fo", logical_key=logical_key) changes |= {o: _changed(f"f{o}", "v2", logical_key="u") for o in keyed} _sync(replica, _page("T2", "T3", changes)) refused = replica.record(key) @@ -1824,19 +1859,28 @@ def test_a_refused_create_goes_to_the_key_holder( return replica.resolve(refused, UpdateHolder()) assert replica.record(key) is None + if unsent is not None: + assert replica.record(unsent).edit.content == _content("fo") b = replica.find_record(_SCOPE, "b") assert _summary(b) == _pending(EditKind.CHANGE) assert (b.edit.content, b.edit.logical_key) == (_content("fl"), logical_key) @pytest.mark.parametrize( - "holding, error", [("conflict", OpenConflictError), ("in flight", PushInFlightError)] + "holding, error", + [ + ("conflict", OpenConflictError), + ("in flight", PushInFlightError), + ("an edit", PendingEditError), + ], ) def test_a_holder_that_takes_no_edit(self, replica, store, holding, error): key = _refused_create(_seeded(replica), "b") if holding == "conflict": _in_conflict(replica, "b", "T2", "T3") - else: + elif holding == "in flight": _push(replica, _edit_locally(replica, "change", "b")[0], None, "T2", "T3") + else: + _edit_locally(replica, "change", "b") refused = replica.record(key) _assert_refused(replica, store, lambda: replica.resolve(refused, UpdateHolder()), error)