max / synckit
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
6 files changed,
+280 insertions,
-13 deletions
| @@ -1,6 +1,6 @@ | |||
| 1 | 1 | [package] | |
| 2 | 2 | name = "synckit-client" | |
| 3 | - | version = "0.6.0" | |
| 3 | + | version = "0.7.0" | |
| 4 | 4 | edition = "2024" | |
| 5 | 5 | license = "LicenseRef-PolyForm-Noncommercial-1.0.0" | |
| 6 | 6 | description = "SyncKit client SDK with end-to-end encryption" |
| @@ -655,9 +655,17 @@ | |||
| 655 | 655 | the loss is at least visible. This is why `field_merge` takes the list as an | |
| 656 | 656 | argument rather than being a bare flag. | |
| 657 | 657 | - **Columns only valid together**, like a `status` and the `completed_at` derived | |
| 658 | - | from it. If one write site sets `status` without `completed_at`, a merge can take | |
| 659 | - | each from a different device and produce a pair neither one wrote. Enumerate the | |
| 660 | - | table's writers and make dependent columns move together at every site first. | |
| 658 | + | from it. Declare them with `dependent_columns(&[&["status", "completed_at"]])` | |
| 659 | + | and a contested group is taken whole from the winning side, so it stays | |
| 660 | + | coherent. | |
| 661 | + | ||
| 662 | + | This one cannot be fixed in the app, which is worth knowing because the obvious | |
| 663 | + | attempt looks like it should work. Base `{status: Pending, completed_at: null}`; | |
| 664 | + | one device starts the task, the other completes it. Only the completer moved | |
| 665 | + | `completed_at`, so it is uncontested and survives whoever wins `status`, and the | |
| 666 | + | row lands on `{status: Started, completed_at: T}` half the time. Making the | |
| 667 | + | starter write `completed_at` explicitly changes nothing: its value already | |
| 668 | + | equals the base, and a merge contests changes rather than writes. | |
| 661 | 669 | ||
| 662 | 670 | ## Blob policy | |
| 663 | 671 |
| @@ -419,6 +419,39 @@ | |||
| 419 | 419 | base: &serde_json::Value, | |
| 420 | 420 | local_hlc: &Hlc, | |
| 421 | 421 | remote_hlc: &Hlc, | |
| 422 | + | ) -> Resolution { | |
| 423 | + | resolve_field_merge_with(local, remote, base, local_hlc, remote_hlc, &[]) | |
| 424 | + | } | |
| 425 | + | ||
| 426 | + | /// [`resolve_field_merge`] with groups of columns that are only valid together. | |
| 427 | + | /// | |
| 428 | + | /// A plain field merge decides each field on its own, and for most columns that | |
| 429 | + | /// is the point. It is wrong for a column whose meaning depends on another: | |
| 430 | + | /// GoingsOn's `completed_at` is only meaningful for the `status` it was derived | |
| 431 | + | /// from, so deciding the two independently can produce a pair neither device | |
| 432 | + | /// wrote. | |
| 433 | + | /// | |
| 434 | + | /// The failure is worth spelling out, because the obvious fix does not work. | |
| 435 | + | /// Base `{status: Pending, completed_at: null}`; one device starts the task, the | |
| 436 | + | /// other completes it. `status` is contested and goes to the winner, but | |
| 437 | + | /// `completed_at` was moved by only one side, so it is *uncontested* and that | |
| 438 | + | /// side's value survives regardless. If the starter wins `status`, the row lands | |
| 439 | + | /// on `{status: Started, completed_at: T}`. Making the starter write | |
| 440 | + | /// `completed_at` explicitly changes nothing: its value equals the base either | |
| 441 | + | /// way, so it is never a change to contest. | |
| 442 | + | /// | |
| 443 | + | /// So the grouping has to be declared, and it is enforced here rather than by the | |
| 444 | + | /// caller because the winner has to be *this* merge's winner. Naming a group says: | |
| 445 | + | /// if any member is contested, every member takes the row-level winner's value, | |
| 446 | + | /// so the group lands as one device's coherent version of it. Members outside the | |
| 447 | + | /// contested group still merge field by field. | |
| 448 | + | pub fn resolve_field_merge_with( | |
| 449 | + | local: &serde_json::Value, | |
| 450 | + | remote: &serde_json::Value, | |
| 451 | + | base: &serde_json::Value, | |
| 452 | + | local_hlc: &Hlc, | |
| 453 | + | remote_hlc: &Hlc, | |
| 454 | + | dependent: &[&[&str]], | |
| 422 | 455 | ) -> Resolution { | |
| 423 | 456 | // Device-independent winner of a contested field (and of the whole entry in | |
| 424 | 457 | // the no-base fallback): identical on every device, so the merge converges. | |
| @@ -495,6 +528,38 @@ | |||
| 495 | 528 | } | |
| 496 | 529 | } | |
| 497 | 530 | ||
| 531 | + | // Dependent groups, applied last so they override whatever the field-by-field | |
| 532 | + | // pass decided for their members. A group is triggered by *any* member being | |
| 533 | + | // contested, and then the whole group is taken from the winning side, which is | |
| 534 | + | // the only way its members can be guaranteed to describe one device's state. | |
| 535 | + | let winner = if local_wins { local_obj } else { remote_obj }; | |
| 536 | + | for group in dependent { | |
| 537 | + | let triggered = group | |
| 538 | + | .iter() | |
| 539 | + | .any(|c| local_changed.contains_key(c) && remote_changed.contains_key(c)); | |
| 540 | + | if !triggered { | |
| 541 | + | continue; | |
| 542 | + | } | |
| 543 | + | tracing::debug!( | |
| 544 | + | ?group, | |
| 545 | + | local_wins, | |
| 546 | + | "dependent group contested; taking it whole" | |
| 547 | + | ); | |
| 548 | + | for col in *group { | |
| 549 | + | match winner.get(*col) { | |
| 550 | + | Some(v) => { | |
| 551 | + | result.insert((*col).to_string(), v.clone()); | |
| 552 | + | } | |
| 553 | + | // The winner does not carry the column at all, so neither can the | |
| 554 | + | // group: leaving the other side's value would rebuild the split | |
| 555 | + | // this exists to prevent. | |
| 556 | + | None => { | |
| 557 | + | result.remove(*col); | |
| 558 | + | } | |
| 559 | + | } | |
| 560 | + | } | |
| 561 | + | } | |
| 562 | + | ||
| 498 | 563 | Resolution::Merged(serde_json::Value::Object(result)) | |
| 499 | 564 | } | |
| 500 | 565 |
| @@ -91,7 +91,7 @@ | |||
| 91 | 91 | }; | |
| 92 | 92 | pub use conflict::{ | |
| 93 | 93 | CleanChanges, ConflictPair, ConflictResolver, Resolution, contested_fields, detect_conflicts, | |
| 94 | - | resolve_field_merge, resolve_lww, | |
| 94 | + | resolve_field_merge, resolve_field_merge_with, resolve_lww, | |
| 95 | 95 | }; | |
| 96 | 96 | pub use error::{Result, SyncKitError}; | |
| 97 | 97 | pub use identity::{ |
| @@ -27,7 +27,7 @@ | |||
| 27 | 27 | use super::stash; | |
| 28 | 28 | use crate::conflict::{ | |
| 29 | 29 | Resolution, change_order, contested_fields, detect_conflicts, is_clock_poisoned, | |
| 30 | - | resolve_field_merge, resolve_lww_at, | |
| 30 | + | resolve_field_merge_with, resolve_lww_at, | |
| 31 | 31 | }; | |
| 32 | 32 | use crate::error::Result; | |
| 33 | 33 | use crate::ids::DeviceId; | |
| @@ -423,7 +423,14 @@ | |||
| 423 | 423 | return None; | |
| 424 | 424 | } | |
| 425 | 425 | ||
| 426 | - | let merged = resolve_field_merge(local_data, remote_data, &base, &pair.local.hlc, &entry.hlc); | |
| 426 | + | let merged = resolve_field_merge_with( | |
| 427 | + | local_data, | |
| 428 | + | remote_data, | |
| 429 | + | &base, | |
| 430 | + | &pair.local.hlc, | |
| 431 | + | &entry.hlc, | |
| 432 | + | table.dependent_columns_groups(), | |
| 433 | + | ); | |
| 427 | 434 | ||
| 428 | 435 | // Only a field both sides moved to *different* values loses anything. Both | |
| 429 | 436 | // arriving at the same value is a contest with no loser, and stashing it | |
| @@ -1820,6 +1827,161 @@ | |||
| 1820 | 1827 | ); | |
| 1821 | 1828 | } | |
| 1822 | 1829 | ||
| 1830 | + | // ── Dependent columns ── | |
| 1831 | + | ||
| 1832 | + | /// A row whose `state` carries a `state_at` derived from it, the shape | |
| 1833 | + | /// GoingsOn's `status`/`completed_at` has. | |
| 1834 | + | fn dep_schema() -> SyncSchema { | |
| 1835 | + | SyncSchema::new(vec![ | |
| 1836 | + | SyncTable::full("job", &["id", "state", "state_at", "note"]) | |
| 1837 | + | .field_merge(&[]) | |
| 1838 | + | .dependent_columns(&[&["state", "state_at"]]), | |
| 1839 | + | ]) | |
| 1840 | + | } | |
| 1841 | + | ||
| 1842 | + | fn dep_device(n: u128) -> (Connection, DeviceId) { | |
| 1843 | + | let conn = Connection::open_in_memory().unwrap(); | |
| 1844 | + | configure_connection(&conn).unwrap(); | |
| 1845 | + | conn.execute_batch( | |
| 1846 | + | "CREATE TABLE job (id TEXT PRIMARY KEY, state TEXT, state_at TEXT, note TEXT);", | |
| 1847 | + | ) | |
| 1848 | + | .unwrap(); | |
| 1849 | + | conn.execute_batch(&dep_schema().migration_sql()).unwrap(); | |
| 1850 | + | (conn, node(n)) | |
| 1851 | + | } | |
| 1852 | + | ||
| 1853 | + | fn job_change(from: DeviceId, wall_ms: i64, data: serde_json::Value) -> PulledChange { | |
| 1854 | + | PulledChange { | |
| 1855 | + | entry: ChangeEntry { | |
| 1856 | + | table: "job".into(), | |
| 1857 | + | op: ChangeOp::Update, | |
| 1858 | + | row_id: "j1".into(), | |
| 1859 | + | timestamp: Utc::now(), | |
| 1860 | + | hlc: Hlc { | |
| 1861 | + | wall_ms, | |
| 1862 | + | counter: 0, | |
| 1863 | + | node: from, | |
| 1864 | + | }, | |
| 1865 | + | data: Some(data), | |
| 1866 | + | extra: serde_json::Map::default(), | |
| 1867 | + | }, | |
| 1868 | + | device_id: from, | |
| 1869 | + | seq: 1, | |
| 1870 | + | } | |
| 1871 | + | } | |
| 1872 | + | ||
| 1873 | + | fn job(conn: &Connection) -> (Option<String>, Option<String>, Option<String>) { | |
| 1874 | + | conn.query_row( | |
| 1875 | + | "SELECT state, state_at, note FROM job WHERE id = 'j1'", | |
| 1876 | + | [], | |
| 1877 | + | |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)), | |
| 1878 | + | ) | |
| 1879 | + | .unwrap() | |
| 1880 | + | } | |
| 1881 | + | ||
| 1882 | + | /// The failure the declaration exists for, and the reason it cannot be fixed | |
| 1883 | + | /// in the app: one device moves the row to `started` (leaving `state_at` | |
| 1884 | + | /// alone), the other to `done` (stamping it). Only the second moved | |
| 1885 | + | /// `state_at`, so a column-by-column merge treats it as uncontested and keeps | |
| 1886 | + | /// it whichever way `state` falls, producing a started job with a completion | |
| 1887 | + | /// time. The group forces both from one side. | |
| 1888 | + | #[test] | |
| 1889 | + | fn a_contested_dependent_group_is_taken_whole_from_one_side() { | |
| 1890 | + | for (local_newer, expect) in [(true, ("started", None)), (false, ("done", Some("T")))] { | |
| 1891 | + | let (mut conn, n) = dep_device(1); | |
| 1892 | + | let peer = node(2); | |
| 1893 | + | let t0 = card_t0(); | |
| 1894 | + | let s = dep_schema(); | |
| 1895 | + | ||
| 1896 | + | pull_apply_with( | |
| 1897 | + | &mut conn, | |
| 1898 | + | &s, | |
| 1899 | + | n, | |
| 1900 | + | vec![job_change( | |
| 1901 | + | peer, | |
| 1902 | + | t0, | |
| 1903 | + | serde_json::json!({"id": "j1", "state": "pending", "state_at": null, "note": "n"}), | |
| 1904 | + | )], | |
| 1905 | + | ); | |
| 1906 | + | ||
| 1907 | + | // Local moves state only; remote moves state and its derived stamp. | |
| 1908 | + | conn.execute("UPDATE job SET state = 'started' WHERE id = 'j1'", []) | |
| 1909 | + | .unwrap(); | |
| 1910 | + | let (local_ms, remote_ms) = if local_newer { | |
| 1911 | + | (t0 + 3_000, t0 + 2_000) | |
| 1912 | + | } else { | |
| 1913 | + | (t0 + 1_000, t0 + 2_000) | |
| 1914 | + | }; | |
| 1915 | + | stamp_pending(&conn, n, local_ms).unwrap(); | |
| 1916 | + | pull_apply_with( | |
| 1917 | + | &mut conn, | |
| 1918 | + | &s, | |
| 1919 | + | n, | |
| 1920 | + | vec![job_change( | |
| 1921 | + | peer, | |
| 1922 | + | remote_ms, | |
| 1923 | + | serde_json::json!({"id": "j1", "state": "done", "state_at": "T", "note": "n"}), | |
| 1924 | + | )], | |
| 1925 | + | ); | |
| 1926 | + | ||
| 1927 | + | let (state, state_at, _) = job(&conn); | |
| 1928 | + | assert_eq!( | |
| 1929 | + | (state.as_deref(), state_at.as_deref()), | |
| 1930 | + | (Some(expect.0), expect.1), | |
| 1931 | + | "the group must land as one device's version of it (local_newer = {local_newer})" | |
| 1932 | + | ); | |
| 1933 | + | } | |
| 1934 | + | } | |
| 1935 | + | ||
| 1936 | + | /// The group only fires when it is contested. A device that moves the group | |
| 1937 | + | /// while the other moves an unrelated column still gets a merge, which is the | |
| 1938 | + | /// whole reason the table opted in. | |
| 1939 | + | #[test] | |
| 1940 | + | fn an_uncontested_dependent_group_still_merges() { | |
| 1941 | + | let (mut conn, n) = dep_device(1); | |
| 1942 | + | let peer = node(2); | |
| 1943 | + | let t0 = card_t0(); | |
| 1944 | + | let s = dep_schema(); | |
| 1945 | + | ||
| 1946 | + | pull_apply_with( | |
| 1947 | + | &mut conn, | |
| 1948 | + | &s, | |
| 1949 | + | n, | |
| 1950 | + | vec![job_change( | |
| 1951 | + | peer, | |
| 1952 | + | t0, | |
| 1953 | + | serde_json::json!({"id": "j1", "state": "pending", "state_at": null, "note": "n"}), | |
| 1954 | + | )], | |
| 1955 | + | ); | |
| 1956 | + | ||
| 1957 | + | // Local edits the unrelated column; only remote touches the group. | |
| 1958 | + | conn.execute("UPDATE job SET note = 'mine' WHERE id = 'j1'", []) | |
| 1959 | + | .unwrap(); | |
| 1960 | + | stamp_pending(&conn, n, t0 + 1_000).unwrap(); | |
| 1961 | + | pull_apply_with( | |
| 1962 | + | &mut conn, | |
| 1963 | + | &s, | |
| 1964 | + | n, | |
| 1965 | + | vec![job_change( | |
| 1966 | + | peer, | |
| 1967 | + | t0 + 2_000, | |
| 1968 | + | serde_json::json!({"id": "j1", "state": "done", "state_at": "T", "note": "n"}), | |
| 1969 | + | )], | |
| 1970 | + | ); | |
| 1971 | + | ||
| 1972 | + | let (state, state_at, note) = job(&conn); | |
| 1973 | + | assert_eq!( | |
| 1974 | + | (state.as_deref(), state_at.as_deref()), | |
| 1975 | + | (Some("done"), Some("T")), | |
| 1976 | + | "an uncontested group must carry across intact" | |
| 1977 | + | ); | |
| 1978 | + | assert_eq!( | |
| 1979 | + | note.as_deref(), | |
| 1980 | + | Some("mine"), | |
| 1981 | + | "declaring a group must not cost the table its merge on other columns" | |
| 1982 | + | ); | |
| 1983 | + | } | |
| 1984 | + | ||
| 1823 | 1985 | /// The stash is bounded. Unbounded, a pathological sync loop grows it without | |
| 1824 | 1986 | /// limit. | |
| 1825 | 1987 | #[test] |
| @@ -93,6 +93,7 @@ | |||
| 93 | 93 | pub(crate) group_scope: Option<&'static str>, | |
| 94 | 94 | pub(crate) field_merge: bool, | |
| 95 | 95 | pub(crate) counters: &'static [&'static str], | |
| 96 | + | pub(crate) dependent: &'static [&'static [&'static str]], | |
| 96 | 97 | } | |
| 97 | 98 | ||
| 98 | 99 | impl SyncTable { | |
| @@ -113,6 +114,12 @@ | |||
| 113 | 114 | self.counters | |
| 114 | 115 | } | |
| 115 | 116 | ||
| 117 | + | /// Groups of columns this table declared as only valid together | |
| 118 | + | /// ([`dependent_columns`](Self::dependent_columns)). | |
| 119 | + | pub fn dependent_columns_groups(&self) -> &'static [&'static [&'static str]] { | |
| 120 | + | self.dependent | |
| 121 | + | } | |
| 122 | + | ||
| 116 | 123 | /// The local provenance column that routes this table's rows to a group | |
| 117 | 124 | /// scope, or `None` if the table is personal-only. Public so a consumer can | |
| 118 | 125 | /// assert exactly which tables are group-scoped (GoingsOn's M3 check). | |
| @@ -138,6 +145,7 @@ | |||
| 138 | 145 | group_scope: None, | |
| 139 | 146 | field_merge: false, | |
| 140 | 147 | counters: &[], | |
| 148 | + | dependent: &[], | |
| 141 | 149 | } | |
| 142 | 150 | } | |
| 143 | 151 | ||
| @@ -214,12 +222,9 @@ | |||
| 214 | 222 | /// and falls back to LWW-and-stash. Pass `&[]` only after checking there is | |
| 215 | 223 | /// no such column; the API asks because the question is easy to not ask. | |
| 216 | 224 | /// | |
| 217 | - | /// One thing merge cannot check for you: **columns that are only valid | |
| 218 | - | /// together**. If `status` and `completed_at` are written as a pair | |
| 219 | - | /// everywhere but one, a merge can take `status` from one device and | |
| 220 | - | /// `completed_at` from the other and produce a pair neither device wrote. | |
| 221 | - | /// Enumerate the table's writers before opting in, and make dependent columns | |
| 222 | - | /// move together at every site. | |
| 225 | + | /// The other thing to check is **columns that are only valid together**, | |
| 226 | + | /// which get their own declaration: see | |
| 227 | + | /// [`dependent_columns`](Self::dependent_columns). | |
| 223 | 228 | #[must_use] | |
| 224 | 229 | pub fn field_merge(mut self, counters: &'static [&'static str]) -> Self { | |
| 225 | 230 | self.field_merge = true; | |
| @@ -227,6 +232,33 @@ | |||
| 227 | 232 | self | |
| 228 | 233 | } | |
| 229 | 234 | ||
| 235 | + | /// Groups of columns that are only meaningful as a set, so a merge decides | |
| 236 | + | /// each group as a unit instead of column by column. | |
| 237 | + | /// | |
| 238 | + | /// The case this exists for is a derived column: GoingsOn's `completed_at` is | |
| 239 | + | /// the timestamp of the transition into its `status`, so a row carrying one | |
| 240 | + | /// device's `status` and another's `completed_at` describes a task neither | |
| 241 | + | /// device has. Declaring `&["status", "completed_at"]` says that if either is | |
| 242 | + | /// contested, both come from whichever side won, so the pair stays coherent. | |
| 243 | + | /// | |
| 244 | + | /// **This cannot be fixed in the app instead**, which is worth stating because | |
| 245 | + | /// the obvious attempt looks like it should work. Base | |
| 246 | + | /// `{status: Pending, completed_at: null}`; one device starts the task, the | |
| 247 | + | /// other completes it. Only the completer moved `completed_at`, so it is | |
| 248 | + | /// uncontested and survives whoever wins `status`, and the row can land on | |
| 249 | + | /// `{status: Started, completed_at: T}`. Making the starter write | |
| 250 | + | /// `completed_at` explicitly changes nothing, because its value already equals | |
| 251 | + | /// the base and a merge contests changes, not writes. The engine has to be | |
| 252 | + | /// told. | |
| 253 | + | /// | |
| 254 | + | /// Only consulted for a table that called [`field_merge`](Self::field_merge); | |
| 255 | + | /// a table resolving by plain LWW already takes every column from one side. | |
| 256 | + | #[must_use] | |
| 257 | + | pub fn dependent_columns(mut self, groups: &'static [&'static [&'static str]]) -> Self { | |
| 258 | + | self.dependent = groups; | |
| 259 | + | self | |
| 260 | + | } | |
| 261 | + | ||
| 230 | 262 | /// Values injected only on first INSERT (e.g. to satisfy a NOT NULL on a | |
| 231 | 263 | /// preserved secret column). | |
| 232 | 264 | #[must_use] |