From 4cf1cf63cda7d0f05b2cc769a1f00df06e7ca447 Mon Sep 17 00:00:00 2001 From: Dimi Date: Wed, 29 Jul 2026 17:10:03 +0800 Subject: [PATCH] Add address metadata APIs and a bridged sync engine to autofill Firefox Desktop is moving its address storage to this component and needs two things that are not currently exposed. Storage APIs for importing records already persisted elsewhere - add_address_with_meta, add_many_addresses_with_meta, update_address_with_meta and add_many_address_tombstones - taking a caller-supplied guid, timestamps and change counter. The bulk variants run each record in a savepoint so one bad record neither aborts the migration nor leaves a row behind. Carrying the change counter means update_internal_address now takes a CounterUpdate of Increment, Leave or Set(i64). A bridged sync engine, so Desktop's Sync framework can drive address sync: Store::addresses_bridged_engine() exposes the address engine the crate already has, which only implemented sync15::SyncEngine and so was unreachable from Desktop's mozIBridgedSyncEngine. Nothing on mobile calls any of this; it is additive for the Desktop migration. --- CHANGELOG.md | 2 + components/autofill/src/autofill.udl | 108 ++++ components/autofill/src/db/addresses.rs | 491 +++++++++++++++++- components/autofill/src/db/models/address.rs | 51 ++ components/autofill/src/db/store.rs | 89 +++- components/autofill/src/error.rs | 11 + components/autofill/src/lib.rs | 1 + .../autofill/src/sync/address/incoming.rs | 12 +- components/autofill/src/sync/bridge.rs | 183 +++++++ components/autofill/src/sync/engine.rs | 16 +- components/autofill/src/sync/mod.rs | 2 + 11 files changed, 952 insertions(+), 14 deletions(-) create mode 100644 components/autofill/src/sync/bridge.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index a6927bed73..15782189ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ ### Autofill - Add `Store::shutdown()`, which closes the database connection early so it happens before Firefox Desktop's late-write shutdown barrier rather than during GC. Operations after shutdown return `DatabaseClosed`. ([Bug 2050036](https://bugzilla.mozilla.org/show_bug.cgi?id=2050036)) +- Add address metadata APIs for importing records already persisted elsewhere: `add_address_with_meta`, `add_many_addresses_with_meta`, `update_address_with_meta` and `add_many_address_tombstones`, with the bulk variants isolating per-record failures. `AddressMeta` carries the guid, timestamps and `sync_change_counter`, so a record keeps whether it still has changes pending upload. +- Add `Store::addresses_bridged_engine()`, exposing the existing address sync engine through `mozIBridgedSyncEngine` so Firefox Desktop can drive address sync. # v154.0 (_2026-07-20_) diff --git a/components/autofill/src/autofill.udl b/components/autofill/src/autofill.udl index 841758bd17..a9c6e3c5d4 100644 --- a/components/autofill/src/autofill.udl +++ b/components/autofill/src/autofill.udl @@ -107,6 +107,48 @@ dictionary Passport { i64 times_used; }; +/// Metadata fields managed internally by the library: the guid, timestamps and +/// local sync state. These are automatically set on `add_address` and updated on +/// operations like `touch` and `update_address`. Not included in +/// `UpdatableAddressFields`; use `add_address_with_meta` when importing records +/// that already have metadata. +dictionary AddressMeta { + string guid; + i64 time_created; + i64? time_last_used; + i64 time_last_modified; + i64 times_used; + i64 sync_change_counter; +}; + +/// An address together with its metadata, passed to `add_address_with_meta` and +/// `update_address_with_meta` when importing a record from another store. +dictionary UpdatableAddressFieldsWithMeta { + UpdatableAddressFields fields; + AddressMeta meta; +}; + +/// A bulk insert result entry, returned per input record by `add_many_addresses_with_meta` +[Enum] +interface AddressBulkResultEntry { + Success(Address address); + Error(string message); +}; + +/// A tombstone for a record deleted locally but not yet uploaded, supplied to +/// `add_many_address_tombstones` when migrating from another store. +dictionary AddressTombstone { + string guid; + i64 time_deleted; +}; + +/// Per-record result of `add_many_address_tombstones`. +[Enum] +interface AddressBulkTombstoneResultEntry { + Success(string guid); + Error(string message); +}; + /// Metrics tracking scrubbing of credit cards that cannot be decrypted, see // `scrub_undecryptable_credit_card_data_for_remote_replacement` for more details dictionary CreditCardsDeletionMetrics { @@ -150,6 +192,19 @@ interface Store { [Throws=AutofillApiError] Address add_address(UpdatableAddressFields a); + [Throws=AutofillApiError] + Address add_address_with_meta(UpdatableAddressFieldsWithMeta entry_with_meta); + + [Throws=AutofillApiError] + sequence add_many_addresses_with_meta(sequence entries_with_meta); + + [Throws=AutofillApiError] + sequence add_many_address_tombstones(sequence tombstones); + + /// Removes every address and every address tombstone. + [Throws=AutofillApiError] + void delete_all_addresses(); + [Throws=AutofillApiError] Address get_address(string guid); @@ -162,6 +217,9 @@ interface Store { [Throws=AutofillApiError] void update_address(string guid, UpdatableAddressFields a); + [Throws=AutofillApiError] + void update_address_with_meta(UpdatableAddressFieldsWithMeta entry_with_meta); + [Throws=AutofillApiError] boolean delete_address(string guid); @@ -212,4 +270,54 @@ interface Store { [Self=ByArc] void register_with_sync_manager(); + + /// Returns a bridged sync engine for addresses, for use by Desktop's Sync + /// framework. Constructing it only assembles structs and never touches the + /// DB, so it cannot fail. + [Self=ByArc] + AddressesBridgedEngine addresses_bridged_engine(); +}; + +/// The bridged sync engine for addresses. The canonical docs are in +/// services/interfaces/mozIBridgedSyncEngine.idl. +/// NOTE: all timestamps here are milliseconds. +interface AddressesBridgedEngine { + [Throws=AutofillApiError] + i64 last_sync(); + + [Throws=AutofillApiError] + void set_last_sync(i64 last_sync); + + [Throws=AutofillApiError] + string? sync_id(); + + [Throws=AutofillApiError] + string reset_sync_id(); + + [Throws=AutofillApiError] + string ensure_current_sync_id([ByRef]string new_sync_id); + + [Throws=AutofillApiError] + void prepare_for_sync([ByRef]string client_data); + + [Throws=AutofillApiError] + void sync_started(); + + [Throws=AutofillApiError] + void store_incoming(sequence incoming_envelopes_as_json); + + [Throws=AutofillApiError] + sequence apply(); + + [Throws=AutofillApiError] + void set_uploaded(i64 new_timestamp, sequence uploaded_ids); + + [Throws=AutofillApiError] + void sync_finished(); + + [Throws=AutofillApiError] + void reset(); + + [Throws=AutofillApiError] + void wipe(); }; diff --git a/components/autofill/src/db/addresses.rs b/components/autofill/src/db/addresses.rs index 431c33d2bb..3f6b5ac72c 100644 --- a/components/autofill/src/db/addresses.rs +++ b/components/autofill/src/db/addresses.rs @@ -5,7 +5,9 @@ use crate::db::{ models::{ - address::{InternalAddress, UpdatableAddressFields}, + address::{ + AddressMeta, InternalAddress, UpdatableAddressFields, UpdatableAddressFieldsWithMeta, + }, Metadata, }, schema::{ADDRESS_COMMON_COLS, ADDRESS_COMMON_VALS}, @@ -48,6 +50,182 @@ pub(crate) fn add_address( Ok(address) } +/// Adds an address **including metadata**, taking the guid, timestamps and sync +/// change counter from the caller rather than generating them. Normally you will +/// use `add_address` instead; this is for importing records from another store +/// that already have metadata. +pub(crate) fn add_address_with_meta( + conn: &Connection, + fields: UpdatableAddressFields, + meta: AddressMeta, +) -> Result { + let tx = conn.unchecked_transaction()?; + let address = internal_address_from_meta(fields, &meta); + add_internal_address(&tx, &address)?; + tx.commit()?; + Ok(address) +} + +/// Adds multiple addresses **including metadata** within a single transaction. +/// Each record gets its own result, so a record that fails to insert is reported +/// as `Err(message)` without aborting the rest of the batch. +pub(crate) fn add_many_addresses_with_meta( + conn: &Connection, + entries: Vec, +) -> Result>> { + let tx = conn.unchecked_transaction()?; + let mut results = Vec::with_capacity(entries.len()); + for entry in entries { + let address = internal_address_from_meta(entry.fields, &entry.meta); + match with_savepoint(&tx, || add_internal_address(&tx, &address))? { + Ok(()) => results.push(Ok(address)), + Err(e) => results.push(Err(e.to_string())), + } + } + tx.commit()?; + Ok(results) +} + +/// Runs `op` in a savepoint, rolling back to it if `op` fails, so that a record +/// reported as an error by the bulk functions leaves nothing behind. The shared +/// triggers reject a guid that exists in the counterpart table with +/// `RAISE(FAIL)`, which aborts the statement but keeps the row it already +/// inserted - so without this the offending row would be committed along with +/// the rest of the batch, putting the guid in both `addresses_data` and +/// `addresses_tombstones`. +/// +/// The outer `Result` is a savepoint failure and aborts the batch; the inner one +/// is the record's own failure. +fn with_savepoint( + tx: &Transaction<'_>, + op: impl FnOnce() -> Result, +) -> Result> { + tx.execute_batch("SAVEPOINT bulk_record")?; + match op() { + Ok(value) => { + tx.execute_batch("RELEASE bulk_record")?; + Ok(Ok(value)) + } + Err(e) => { + tx.execute_batch("ROLLBACK TO bulk_record; RELEASE bulk_record")?; + Ok(Err(e)) + } + } +} + +/// Removes every address and every address tombstone, in one transaction. +/// +/// Deleting the rows alone is not enough. A delete leaves a tombstone behind for +/// any guid the sync mirror knows, and the insert trigger then rejects re-adding +/// that guid, so a wipe that kept them could not be followed by a re-import of +/// the same records. Clearing both tables is what makes the wipe repeatable. +pub(crate) fn delete_all_addresses(conn: &Connection) -> Result<()> { + let tx = conn.unchecked_transaction()?; + tx.execute("DELETE FROM addresses_data", [])?; + // After the data, so the tombstones the delete trigger just created go too. + tx.execute("DELETE FROM addresses_tombstones", [])?; + tx.commit()?; + Ok(()) +} + +/// Adds tombstones for records that were deleted locally but not yet uploaded, +/// within a single transaction and with a result per record. `time_deleted` comes +/// from the caller rather than being stamped as now, so that a deletion imported +/// from another store keeps its original time. Without the tombstone the next +/// sync has nothing to say the record was deleted and takes the server copy. +pub(crate) fn add_many_address_tombstones( + conn: &Connection, + tombstones: Vec<(String, i64)>, +) -> Result>> { + let tx = conn.unchecked_transaction()?; + let mut results = Vec::with_capacity(tombstones.len()); + for (guid, time_deleted) in tombstones { + let inserted = with_savepoint(&tx, || { + tx.execute( + "INSERT INTO addresses_tombstones (guid, time_deleted) + VALUES (:guid, :time_deleted)", + rusqlite::named_params! { + ":guid": &guid, + ":time_deleted": timestamp_from_millis(time_deleted), + }, + )?; + Ok(()) + })?; + match inserted { + Ok(()) => results.push(Ok(guid)), + Err(e) => results.push(Err(e.to_string())), + } + } + tx.commit()?; + Ok(results) +} + +/// `Timestamp` is a `u64`, so a negative millisecond value would wrap to a huge +/// one and then win every "latest wins" comparison in `Metadata::merge`. Clamp to +/// 0, which already means "unset" for these fields. The tuple constructor is used +/// rather than `Timestamp::from`, which asserts non-zero. +fn timestamp_from_millis(millis: i64) -> Timestamp { + Timestamp(millis.max(0) as u64) +} + +fn internal_address_from_meta( + fields: UpdatableAddressFields, + meta: &AddressMeta, +) -> InternalAddress { + InternalAddress { + guid: Guid::new(&meta.guid), + name: fields.name, + organization: fields.organization, + street_address: fields.street_address, + address_level3: fields.address_level3, + address_level2: fields.address_level2, + address_level1: fields.address_level1, + postal_code: fields.postal_code, + country: fields.country, + tel: fields.tel, + email: fields.email, + metadata: Metadata { + time_created: timestamp_from_millis(meta.time_created), + time_last_used: timestamp_from_millis(meta.time_last_used.unwrap_or(0)), + time_last_modified: timestamp_from_millis(meta.time_last_modified), + times_used: meta.times_used, + sync_change_counter: meta.sync_change_counter, + }, + } +} + +/// Updates an address **including metadata**, setting both its fields and its +/// timestamps and `times_used` to the supplied values. Normally you will use +/// `update_address` instead, which owns the metadata itself; this is for keeping +/// a record identical to one held in another store. Errors with `NoSuchRecord` +/// if the guid is absent. +pub(crate) fn update_address_with_meta( + conn: &Connection, + fields: UpdatableAddressFields, + meta: AddressMeta, +) -> Result<()> { + let tx = conn.unchecked_transaction()?; + + let address = internal_address_from_meta(fields, &meta); + // Checked up front because `update_internal_address` asserts on the number + // of rows changed rather than returning an error. + let exists: bool = tx.query_row( + "SELECT EXISTS(SELECT 1 FROM addresses_data WHERE guid = :guid)", + rusqlite::named_params! { ":guid": address.guid }, + |row| row.get(0), + )?; + if !exists { + return Err(Error::NoSuchRecord(address.guid.to_string())); + } + update_internal_address( + &tx, + &address, + CounterUpdate::Set(address.metadata.sync_change_counter), + )?; + tx.commit()?; + Ok(()) +} + pub(crate) fn add_internal_address(tx: &Transaction<'_>, address: &InternalAddress) -> Result<()> { tx.execute( &format!( @@ -165,17 +343,42 @@ pub(crate) fn update_address( Ok(()) } +/// How `update_internal_address` should treat the change counter. +pub(crate) enum CounterUpdate { + /// Record a local change awaiting upload. + Increment, + /// Leave the counter alone, for a change that must not be uploaded - eg one + /// applied by Sync, which is already what the server has. + Leave, + /// Replace the counter, for a record whose counter is owned by the caller. + Set(i64), +} + +impl CounterUpdate { + /// The SQL assigned to `sync_change_counter`, and the value bound to + /// `:counter` within it. `Leave` adds 0 rather than dropping `:counter` from + /// the SQL, because rusqlite rejects a named parameter the statement doesn't + /// use. + fn as_sql(&self) -> (&'static str, i64) { + match self { + Self::Increment => ("sync_change_counter + :counter", 1), + Self::Leave => ("sync_change_counter + :counter", 0), + Self::Set(counter) => (":counter", *counter), + } + } +} + /// Updates all fields including metadata - although the change counter gets -/// slightly special treatment (eg, when called by Sync we don't want the -/// change counter incremented) +/// slightly special treatment, see `CounterUpdate`. pub(crate) fn update_internal_address( tx: &Transaction<'_>, address: &InternalAddress, - flag_as_changed: bool, + counter: CounterUpdate, ) -> Result<()> { - let change_counter_increment = flag_as_changed as u32; // will be 1 or 0 + let (counter_sql, counter_value) = counter.as_sql(); let rows_changed = tx.execute( - "UPDATE addresses_data SET + &format!( + "UPDATE addresses_data SET name = :name, organization = :organization, street_address = :street_address, @@ -190,8 +393,9 @@ pub(crate) fn update_internal_address( time_last_used = :time_last_used, time_last_modified = :time_last_modified, times_used = :times_used, - sync_change_counter = sync_change_counter + :change_incr - WHERE guid = :guid", + sync_change_counter = {counter_sql} + WHERE guid = :guid" + ), rusqlite::named_params! { ":name": address.name, ":organization": address.organization, @@ -207,7 +411,7 @@ pub(crate) fn update_internal_address( ":time_last_used": address.metadata.time_last_used, ":time_last_modified": address.metadata.time_last_modified, ":times_used": address.metadata.times_used, - ":change_incr": change_counter_increment, + ":counter": counter_value, ":guid": address.guid, }, )?; @@ -513,7 +717,7 @@ mod tests { email: "".to_string(), ..Default::default() }, - false, + CounterUpdate::Leave, )?; let record_exists: bool = tx.query_row( @@ -670,4 +874,271 @@ mod tests { Ok(()) } + + fn test_fields(street_address: &str) -> UpdatableAddressFields { + UpdatableAddressFields { + name: "jane doe".to_string(), + street_address: street_address.to_string(), + address_level2: "Seattle, WA".to_string(), + country: "United States".to_string(), + ..UpdatableAddressFields::default() + } + } + + fn test_meta(guid: &str, sync_change_counter: i64) -> AddressMeta { + AddressMeta { + guid: guid.to_string(), + time_created: 1000, + time_last_used: Some(2000), + time_last_modified: 3000, + times_used: 4, + sync_change_counter, + } + } + + #[test] + fn test_address_add_with_meta() -> Result<()> { + let db = new_mem_db(); + + let saved = + add_address_with_meta(&db, test_fields("123 Main Street"), test_meta("abc", 2))?; + + // the supplied guid is used rather than a fresh one being generated. + assert_eq!(saved.guid.as_str(), "abc"); + + let retrieved = get_address(&db, &Guid::new("abc"))?; + assert_eq!(retrieved.street_address, "123 Main Street"); + assert_eq!(retrieved.metadata.time_created.as_millis(), 1000); + assert_eq!(retrieved.metadata.time_last_used.as_millis(), 2000); + assert_eq!(retrieved.metadata.time_last_modified.as_millis(), 3000); + assert_eq!(retrieved.metadata.times_used, 4); + assert_eq!(retrieved.metadata.sync_change_counter, 2); + + Ok(()) + } + + #[test] + fn test_address_add_with_meta_clamps_negative_timestamps() -> Result<()> { + let db = new_mem_db(); + + let meta = AddressMeta { + guid: "abc".to_string(), + time_created: -1, + time_last_used: Some(-1), + time_last_modified: -1, + times_used: 0, + sync_change_counter: 0, + }; + add_address_with_meta(&db, test_fields("123 Main Street"), meta)?; + + let retrieved = get_address(&db, &Guid::new("abc"))?; + assert_eq!(retrieved.metadata.time_created.as_millis(), 0); + assert_eq!(retrieved.metadata.time_last_used.as_millis(), 0); + assert_eq!(retrieved.metadata.time_last_modified.as_millis(), 0); + + Ok(()) + } + + #[test] + fn test_address_update_with_meta_keeps_supplied_counter() -> Result<()> { + let db = new_mem_db(); + + add_address_with_meta(&db, test_fields("123 Main Street"), test_meta("abc", 0))?; + + // the supplied counter must be applied, not the one already in the row. + update_address_with_meta(&db, test_fields("456 Second Avenue"), test_meta("abc", 1))?; + + let retrieved = get_address(&db, &Guid::new("abc"))?; + assert_eq!(retrieved.street_address, "456 Second Avenue"); + assert_eq!(retrieved.metadata.sync_change_counter, 1); + + // and back down again. + update_address_with_meta(&db, test_fields("456 Second Avenue"), test_meta("abc", 0))?; + assert_eq!( + get_address(&db, &Guid::new("abc"))? + .metadata + .sync_change_counter, + 0 + ); + + Ok(()) + } + + #[test] + fn test_address_update_with_meta_errors_when_missing() -> Result<()> { + let db = new_mem_db(); + + let result = + update_address_with_meta(&db, test_fields("123 Main Street"), test_meta("abc", 3)); + assert!(matches!(result, Err(Error::NoSuchRecord(guid)) if guid == "abc")); + assert!(get_address(&db, &Guid::new("abc")).is_err()); + + Ok(()) + } + + #[test] + fn test_address_add_many_with_meta_isolates_failures() -> Result<()> { + let db = new_mem_db(); + + // the second entry has an empty guid, which the `addresses_data` CHECK + // constraint rejects. The others must still be inserted. + let results = add_many_addresses_with_meta( + &db, + vec![ + UpdatableAddressFieldsWithMeta { + fields: test_fields("1 First Street"), + meta: test_meta("aaa", 1), + }, + UpdatableAddressFieldsWithMeta { + fields: test_fields("2 Second Street"), + meta: test_meta("", 1), + }, + UpdatableAddressFieldsWithMeta { + fields: test_fields("3 Third Street"), + meta: test_meta("ccc", 1), + }, + ], + )?; + + assert_eq!(results.len(), 3); + assert!(results[0].is_ok()); + assert!(results[1].is_err()); + assert!(results[2].is_ok()); + + assert_eq!(get_all_addresses(&db)?.len(), 2); + assert_eq!(get_address(&db, &Guid::new("aaa"))?.metadata.times_used, 4); + + Ok(()) + } + + #[test] + fn test_delete_all_addresses_allows_a_reimport() -> Result<()> { + let db = new_mem_db(); + + // A tombstone left by an earlier import, and a record sharing no guid + // with it. + add_many_address_tombstones(&db, vec![("gone".to_string(), 1234)])?; + let address = add_address(&db, UpdatableAddressFields::default())?; + + delete_all_addresses(&db)?; + assert_eq!(get_all_addresses(&db)?.len(), 0); + let tombstones: i64 = + db.query_row("SELECT COUNT(*) FROM addresses_tombstones", [], |row| { + row.get(0) + })?; + assert_eq!(tombstones, 0, "tombstones are cleared with the records"); + + // The point of clearing them: re-importing the same guids succeeds, + // where the insert trigger would reject a guid still tombstoned. + let results = add_many_addresses_with_meta( + &db, + vec![ + UpdatableAddressFieldsWithMeta { + fields: UpdatableAddressFields::default(), + meta: AddressMeta { + guid: address.guid.to_string(), + ..Default::default() + }, + }, + UpdatableAddressFieldsWithMeta { + fields: UpdatableAddressFields::default(), + meta: AddressMeta { + guid: "gone".to_string(), + ..Default::default() + }, + }, + ], + )?; + assert!( + results.iter().all(|r| r.is_ok()), + "a previously tombstoned guid can be re-imported: {results:?}" + ); + + Ok(()) + } + + #[test] + fn test_address_add_many_tombstones() -> Result<()> { + let db = new_mem_db(); + + let results = add_many_address_tombstones(&db, vec![("aaa".to_string(), 1234)])?; + assert_eq!(results.len(), 1); + assert!(results[0].is_ok()); + + // the supplied deletion time is used rather than being stamped as now. + let time_deleted: i64 = db.query_row( + "SELECT time_deleted FROM addresses_tombstones WHERE guid = 'aaa'", + [], + |row| row.get(0), + )?; + assert_eq!(time_deleted, 1234); + + Ok(()) + } + + #[test] + fn test_address_add_many_tombstones_rejects_live_guid() -> Result<()> { + let db = new_mem_db(); + + add_address_with_meta(&db, test_fields("123 Main Street"), test_meta("abc", 0))?; + + // a guid cannot be in both `addresses_data` and `addresses_tombstones`; + // the trigger enforcing that must not take the rest of the batch down. + let results = add_many_address_tombstones( + &db, + vec![("abc".to_string(), 1234), ("ddd".to_string(), 5678)], + )?; + + assert_eq!(results.len(), 2); + assert!(results[0].is_err()); + assert!(results[1].is_ok()); + + // the rejected tombstone must not have been committed anyway - see + // `with_savepoint`. + assert_eq!(count_tombstones(&db, "abc")?, 0); + assert!(get_address(&db, &Guid::new("abc")).is_ok()); + assert_eq!(count_tombstones(&db, "ddd")?, 1); + + Ok(()) + } + + #[test] + fn test_address_add_many_with_meta_rejects_deleted_guid() -> Result<()> { + let db = new_mem_db(); + + add_many_address_tombstones(&db, vec![("aaa".to_string(), 1234)])?; + + // the other side of the same invariant: a guid in + // `addresses_tombstones` cannot be inserted into `addresses_data`. + let results = add_many_addresses_with_meta( + &db, + vec![ + UpdatableAddressFieldsWithMeta { + fields: test_fields("1 First Street"), + meta: test_meta("aaa", 1), + }, + UpdatableAddressFieldsWithMeta { + fields: test_fields("2 Second Street"), + meta: test_meta("bbb", 1), + }, + ], + )?; + + assert_eq!(results.len(), 2); + assert!(results[0].is_err()); + assert!(results[1].is_ok()); + + assert!(get_address(&db, &Guid::new("aaa")).is_err()); + assert_eq!(get_all_addresses(&db)?.len(), 1); + + Ok(()) + } + + fn count_tombstones(conn: &Connection, guid: &str) -> Result { + Ok(conn.query_row( + "SELECT COUNT(*) FROM addresses_tombstones WHERE guid = :guid", + rusqlite::named_params! { ":guid": guid }, + |row| row.get(0), + )?) + } } diff --git a/components/autofill/src/db/models/address.rs b/components/autofill/src/db/models/address.rs index e526f278b7..fa226739c0 100644 --- a/components/autofill/src/db/models/address.rs +++ b/components/autofill/src/db/models/address.rs @@ -27,6 +27,57 @@ pub struct UpdatableAddressFields { pub email: String, } +/// Metadata fields managed internally by the library: the guid, timestamps and +/// local sync state. These are automatically set on `add_address` and updated on +/// operations like `touch` and `update_address`. Not included in +/// `UpdatableAddressFields`; use `add_address_with_meta` when importing records +/// that already have metadata. +#[derive(Debug, Clone, Default)] +pub struct AddressMeta { + pub guid: String, + pub time_created: i64, + pub time_last_used: Option, + pub time_last_modified: i64, + pub times_used: i64, + /// Local changes not yet uploaded; 0 means it matches what was last synced. + pub sync_change_counter: i64, +} + +/// A tombstone for a record deleted locally but not yet uploaded, supplied to +/// `add_many_address_tombstones` when migrating from another store. +#[derive(Debug, Clone, Default)] +pub struct AddressTombstone { + pub guid: String, + pub time_deleted: i64, +} + +/// Per-record result of `add_many_address_tombstones`. +#[derive(Debug)] +pub enum AddressBulkTombstoneResultEntry { + Success { guid: String }, + Error { message: String }, +} + +/// An address together with its metadata, passed to `add_address_with_meta` and +/// `update_address_with_meta` when importing a record from another store. +#[derive(Debug, Clone, Default)] +pub struct UpdatableAddressFieldsWithMeta { + pub fields: UpdatableAddressFields, + pub meta: AddressMeta, +} + +/// A bulk insert result entry, returned per input record by +/// `add_many_addresses_with_meta` so that one record failing does not abort the +/// batch. Note that although the success case is much larger than the error +/// case, this is negligible in real life, as we expect a very small +/// success/error ratio. +#[allow(clippy::large_enum_variant)] +#[derive(Debug)] +pub enum AddressBulkResultEntry { + Success { address: Address }, + Error { message: String }, +} + // "Address" is what we return to consumers and has most of the metadata. #[derive(Debug, Clone, Hash, PartialEq, Eq, Default)] pub struct Address { diff --git a/components/autofill/src/db/store.rs b/components/autofill/src/db/store.rs index 1ff3cbb07b..4bb7d1cd25 100644 --- a/components/autofill/src/db/store.rs +++ b/components/autofill/src/db/store.rs @@ -2,7 +2,10 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use crate::db::models::address::{Address, UpdatableAddressFields}; +use crate::db::models::address::{ + Address, AddressBulkResultEntry, AddressBulkTombstoneResultEntry, AddressTombstone, + UpdatableAddressFields, UpdatableAddressFieldsWithMeta, +}; use crate::db::models::credit_card::{CreditCard, UpdatableCreditCardFields}; use crate::db::models::passport::{Passport, UpdatablePassportFields}; use crate::db::{ @@ -133,6 +136,73 @@ impl Store { Ok(addresses::add_address(&self.lock_db()?.writer, new_address)?.into()) } + /// Adds an address **including metadata**. Normally you will use + /// `add_address` instead, and the metadata (guid, timestamps, change counter) + /// will be taken care of here. However, in some cases this method is + /// necessary, for example when migrating data from another store that + /// already contains the metadata. + #[handle_error(Error)] + pub fn add_address_with_meta( + &self, + entry_with_meta: UpdatableAddressFieldsWithMeta, + ) -> ApiResult
{ + Ok(addresses::add_address_with_meta( + &self.lock_db()?.writer, + entry_with_meta.fields, + entry_with_meta.meta, + )? + .into()) + } + + /// Adds multiple addresses **including metadata**, with a result per record. + #[handle_error(Error)] + pub fn add_many_addresses_with_meta( + &self, + entries_with_meta: Vec, + ) -> ApiResult> { + let results = + addresses::add_many_addresses_with_meta(&self.lock_db()?.writer, entries_with_meta)?; + Ok(results + .into_iter() + .map(|result| match result { + Ok(address) => AddressBulkResultEntry::Success { + address: address.into(), + }, + Err(message) => AddressBulkResultEntry::Error { message }, + }) + .collect()) + } + + /// Adds tombstones for addresses whose deletion has not yet been uploaded, + /// with a result per record. + #[handle_error(Error)] + pub fn add_many_address_tombstones( + &self, + tombstones: Vec, + ) -> ApiResult> { + let results = addresses::add_many_address_tombstones( + &self.lock_db()?.writer, + tombstones + .into_iter() + .map(|t| (t.guid, t.time_deleted)) + .collect(), + )?; + Ok(results + .into_iter() + .map(|result| match result { + Ok(guid) => AddressBulkTombstoneResultEntry::Success { guid }, + Err(message) => AddressBulkTombstoneResultEntry::Error { message }, + }) + .collect()) + } + + /// Removes every address and every address tombstone. + #[handle_error(Error)] + pub fn delete_all_addresses(&self) -> ApiResult<()> { + addresses::delete_all_addresses(&self.lock_db()?.writer)?; + Ok(()) + } + #[handle_error(Error)] pub fn get_address(&self, guid: String) -> ApiResult
{ Ok(addresses::get_address(&self.lock_db()?.writer, &Guid::new(&guid))?.into()) @@ -158,6 +228,23 @@ impl Store { addresses::update_address(&self.lock_db()?.writer, &Guid::new(&guid), &address) } + /// Updates an address **including metadata**, setting both its fields and + /// its timestamps and `times_used` to the supplied values. Normally you will + /// use `update_address` instead, which leaves `time_last_modified` to this + /// store; this is for keeping a record identical to one held elsewhere. + /// Errors with `NoSuchRecord` if the guid is absent. + #[handle_error(Error)] + pub fn update_address_with_meta( + &self, + entry_with_meta: UpdatableAddressFieldsWithMeta, + ) -> ApiResult<()> { + addresses::update_address_with_meta( + &self.lock_db()?.writer, + entry_with_meta.fields, + entry_with_meta.meta, + ) + } + #[handle_error(Error)] pub fn delete_address(&self, guid: String) -> ApiResult { addresses::delete_address(&self.lock_db()?.writer, &Guid::new(&guid)) diff --git a/components/autofill/src/error.rs b/components/autofill/src/error.rs index d45c095206..5eab83daba 100644 --- a/components/autofill/src/error.rs +++ b/components/autofill/src/error.rs @@ -31,6 +31,17 @@ pub enum AutofillApiError { UnexpectedAutofillApiError { reason: String }, } +// The `sync15` BridgedEngine traits use `anyhow::Result`, so the bridged engine +// in `sync::bridge` needs those errors mapped onto the public error type before +// UniFFI can expose its methods. +impl From for AutofillApiError { + fn from(value: anyhow::Error) -> Self { + AutofillApiError::UnexpectedAutofillApiError { + reason: value.to_string(), + } + } +} + #[derive(Debug, thiserror::Error)] pub enum Error { #[error("Error opening database: {0}")] diff --git a/components/autofill/src/lib.rs b/components/autofill/src/lib.rs index ebbe668680..5eae09c76e 100644 --- a/components/autofill/src/lib.rs +++ b/components/autofill/src/lib.rs @@ -21,6 +21,7 @@ use crate::db::models::credit_card::*; use crate::db::models::passport::*; use crate::db::store::Store; use crate::encryption::{create_autofill_key, decrypt_string, encrypt_string}; +pub use crate::sync::AddressesBridgedEngine; pub use error::{ApiResult, AutofillApiError, Error, Result}; uniffi::include_scaffolding!("autofill"); diff --git a/components/autofill/src/sync/address/incoming.rs b/components/autofill/src/sync/address/incoming.rs index 117c55eebd..c58c4b08dc 100644 --- a/components/autofill/src/sync/address/incoming.rs +++ b/components/autofill/src/sync/address/incoming.rs @@ -4,7 +4,7 @@ */ use super::AddressPayload; -use crate::db::addresses::{add_internal_address, update_internal_address}; +use crate::db::addresses::{add_internal_address, update_internal_address, CounterUpdate}; use crate::db::models::address::InternalAddress; use crate::db::schema::ADDRESS_COMMON_COLS; use crate::error::*; @@ -309,7 +309,15 @@ impl ProcessIncomingRecordImpl for IncomingAddressesImpl { new_record: Self::Record, flag_as_changed: bool, ) -> Result<()> { - update_internal_address(tx, &new_record, flag_as_changed)?; + update_internal_address( + tx, + &new_record, + if flag_as_changed { + CounterUpdate::Increment + } else { + CounterUpdate::Leave + }, + )?; Ok(()) } diff --git a/components/autofill/src/sync/bridge.rs b/components/autofill/src/sync/bridge.rs new file mode 100644 index 0000000000..88a831b8d7 --- /dev/null +++ b/components/autofill/src/sync/bridge.rs @@ -0,0 +1,183 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +use crate::db::models::address::InternalAddress; +use crate::sync::engine::ConfigSyncEngine; +use crate::Store; +use anyhow::Result; +use std::sync::Arc; +use sync15::engine::BridgedEngineAdaptor; + +impl Store { + /// Returns a bridged sync engine for addresses, for use by Desktop's Sync + /// framework. Constructing a `ConfigSyncEngine` only assembles structs and + /// never touches the DB, so this cannot fail. + pub fn addresses_bridged_engine(self: Arc) -> Arc { + let engine = crate::sync::address::create_engine(self); + Arc::new(AddressesBridgedEngine::new(Box::new( + AddressesBridgedEngineAdaptor { engine }, + ))) + } +} + +/// `ConfigSyncEngine` implements `sync15::SyncEngine`, which is what the sync +/// manager drives. Desktop instead speaks `mozIBridgedSyncEngine`, whose Rust +/// shape is `sync15::BridgedEngine`. The two differ only in that the bridge owns +/// the last-sync timestamp explicitly, so this adaptor supplies that and the +/// blanket `impl BridgedEngine for A` provides the rest. +struct AddressesBridgedEngineAdaptor { + engine: ConfigSyncEngine, +} + +impl BridgedEngineAdaptor for AddressesBridgedEngineAdaptor { + fn last_sync(&self) -> Result { + Ok(self.engine.get_last_sync_millis()?) + } + + fn set_last_sync(&self, last_sync_millis: i64) -> Result<()> { + self.engine.set_last_sync_millis(last_sync_millis)?; + Ok(()) + } + + fn engine(&self) -> &dyn sync15::engine::SyncEngine { + &self.engine + } +} + +// Generates the UniFFI-exposed `AddressesBridgedEngine`, a newtype around +// `sync15::engine::BridgedEngineWrapper`. The UDL's `set_uploaded` takes +// `sequence`, hence the `String` id type. +sync15::uniffi_bridged_engine!(AddressesBridgedEngine, String); + +#[cfg(test)] +mod tests { + use super::*; + use crate::db::models::address::UpdatableAddressFields; + use std::collections::HashMap; + + // Exercises the sync metadata the bridge owns: last_sync, sync_id and reset. + #[test] + fn test_sync_meta() { + error_support::init_for_tests(); + + let store = Arc::new(Store::new_shared_memory("addresses-bridge").unwrap()); + let bridge = store.addresses_bridged_engine(); + + // Fresh DB: never synced. + assert_eq!(bridge.last_sync().unwrap(), 0); + bridge.set_last_sync(3).unwrap(); + assert_eq!(bridge.last_sync().unwrap(), 3); + + assert!(bridge.sync_id().unwrap().is_none()); + + bridge.ensure_current_sync_id("some_guid").unwrap(); + assert_eq!(bridge.sync_id().unwrap(), Some("some_guid".to_string())); + // changing the sync ID resets the timestamp + assert_eq!(bridge.last_sync().unwrap(), 0); + bridge.set_last_sync(3).unwrap(); + + bridge.reset_sync_id().unwrap(); + assert_ne!(bridge.sync_id().unwrap(), Some("some_guid".to_string())); + assert_eq!(bridge.last_sync().unwrap(), 0); + bridge.set_last_sync(3).unwrap(); + + // `reset` clears the guid and the timestamp. + bridge.reset().unwrap(); + assert_eq!(bridge.last_sync().unwrap(), 0); + assert!(bridge.sync_id().unwrap().is_none()); + } + + // A roundtrip through the bridge's data path: stage an incoming remote + // address, apply it, and confirm the local-only address comes back out for + // upload. Unlike `test_sync_meta` this exercises the JSON (de)serialization + // of BSOs and the sync staging tables, mirroring the logins and tabs + // `test_sync_via_bridge` tests. + #[test] + fn test_sync_via_bridge() { + error_support::init_for_tests(); + + let store = Arc::new(Store::new_shared_memory("addresses-bridge-roundtrip").unwrap()); + + // A local-only address: nothing on the server knows about it yet, so it + // should be uploaded. + let local = store + .add_address(UpdatableAddressFields { + name: "Local Person".to_string(), + street_address: "1 Local Lane".to_string(), + address_level2: "Seattle, WA".to_string(), + country: "US".to_string(), + ..Default::default() + }) + .expect("should add local address"); + + let bridge = store.clone().addresses_bridged_engine(); + + // `prepare_for_sync` is what creates the sync staging tables; the client + // data it is given is unused by this engine. + bridge + .prepare_for_sync(r#"{"local_client_id":"my-client","recent_clients":{}}"#) + .expect("should prepare for sync"); + bridge.sync_started().unwrap(); + + // An incoming remote address that isn't known locally. We build the + // envelope as raw JSON, exactly as the JS bridge hands it to us. + let incoming = vec![serde_json::json!({ + "id": "remote-only-bbbb", + "modified": 0, + "payload": serde_json::json!({ + "id": "remote-only-bbbb", + "entry": { + "name": "Remote Person", + "street-address": "99 Remote Road", + "address-level2": "Portland, OR", + "country": "US", + "version": 1, + }, + }) + .to_string(), + }) + .to_string()]; + bridge + .store_incoming(incoming) + .expect("should store incoming"); + + // Applying stores the remote record locally and returns the local-only + // address for upload. + let outgoing = bridge.apply().expect("should apply"); + let changes: HashMap = outgoing + .into_iter() + .map(|s| { + let bso: serde_json::Value = serde_json::from_str(&s).unwrap(); + let payload: serde_json::Value = + serde_json::from_str(bso["payload"].as_str().unwrap()).unwrap(); + (payload["id"].as_str().unwrap().to_string(), payload) + }) + .collect(); + + // Only the local address is outgoing; the just-applied remote one is not + // re-uploaded. + assert_eq!(changes.len(), 1); + assert_eq!( + changes[&local.guid]["entry"]["street-address"], + "1 Local Lane" + ); + + // The incoming remote address was actually persisted. + let stored = store + .get_address("remote-only-bbbb".to_string()) + .expect("remote address should have been stored"); + assert_eq!(stored.street_address, "99 Remote Road"); + + // `apply` deliberately stamps last_sync with 0 - Desktop applies without + // telling us the server timestamp and sends it separately afterwards. + assert_eq!(bridge.last_sync().unwrap(), 0); + bridge.set_uploaded(1234, vec![local.guid.clone()]).unwrap(); + bridge.sync_finished().unwrap(); + assert_eq!(bridge.last_sync().unwrap(), 1234); + + // Acknowledging the upload cleared the record's change counter, so a + // subsequent sync has nothing to send. + assert!(bridge.apply().expect("should apply again").is_empty()); + } +} diff --git a/components/autofill/src/sync/engine.rs b/components/autofill/src/sync/engine.rs index 7abc981588..24b3e3fcaa 100644 --- a/components/autofill/src/sync/engine.rs +++ b/components/autofill/src/sync/engine.rs @@ -29,7 +29,8 @@ pub const GLOBAL_SYNCID_META_KEY: &str = "global_sync_id"; pub const COLLECTION_SYNCID_META_KEY: &str = "sync_id"; // A trait to abstract the broader sync processes. -pub trait SyncEngineStorageImpl { +// Send + Sync is required to use a `ConfigSyncEngine` as a `BridgedEngine`. +pub trait SyncEngineStorageImpl: Send + Sync { fn get_incoming_impl( &self, enc_key: &Option, @@ -74,6 +75,19 @@ impl ConfigSyncEngine { let key = format!("{}.{}", self.config.namespace, tail); crate::db::store::delete_meta(conn, &key) } + /// The last-sync timestamp in milliseconds, 0 if never synced. + pub(crate) fn get_last_sync_millis(&self) -> Result { + let db = self.store.lock_db()?; + Ok(self + .get_meta::(&db.writer, LAST_SYNC_META_KEY)? + .unwrap_or_default()) + } + + pub(crate) fn set_last_sync_millis(&self, millis: i64) -> Result<()> { + let db = self.store.lock_db()?; + self.put_meta(&db.writer, LAST_SYNC_META_KEY, &millis) + } + // Reset the local sync data so the next server request fetches all records. pub fn reset_local_sync_data(&self) -> Result<()> { let db = self.store.lock_db()?; diff --git a/components/autofill/src/sync/mod.rs b/components/autofill/src/sync/mod.rs index 9cc60ed47b..e19c784540 100644 --- a/components/autofill/src/sync/mod.rs +++ b/components/autofill/src/sync/mod.rs @@ -4,6 +4,8 @@ */ pub mod address; +mod bridge; +pub use bridge::AddressesBridgedEngine; mod common; pub mod credit_card; pub mod engine;