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;