Add credit card bulk-import APIs for store migration

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## ✨ What's Changed ✨
 
+### Autofill
+
+- Added credit card equivalents of the address bulk-import API, for applications migrating a credit card collection into this store: `add_credit_card_with_meta()`, `add_many_credit_cards_with_meta()`, `update_credit_card_with_meta()`, `add_many_credit_card_tombstones()` and `delete_all_credit_cards()`. These take the guid, timestamps and sync change counter from the caller, so a migrated record keeps the identity it already had. `cc_number_enc` is stored exactly as supplied and is not checked against the store's key, matching `add_credit_card()`. ([bug 2068982](https://bugzilla.mozilla.org/show_bug.cgi?id=2068982))
+- Timestamps outside the range a JS `Date` can represent are now reported as 0 wherever they enter the store: on the metadata an application supplies to the bulk-import APIs, on every read out of the local database, and on incoming sync payloads. This matches the treatment logins received in [bug 2066257](https://bugzilla.mozilla.org/show_bug.cgi?id=2066257) and covers addresses, credit cards and passports. ([bug 2068982](https://bugzilla.mozilla.org/show_bug.cgi?id=2068982))
+
 ### Logins
 
 - Timestamps outside the range a JS `Date` can represent are now reported as 0 wherever they enter or leave the store: on the metadata an application supplies to `add_with_meta()`, on every read out of the local database, and on incoming sync payloads, similar to what Desktop does. ([bug 2066257](https://bugzilla.mozilla.org/show_bug.cgi?id=2066257))

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_credit_card` and
+/// updated on operations like `touch` and `update_credit_card`. Not included in
+/// `UpdatableCreditCardFields`; use `add_credit_card_with_meta` when importing
+/// records that already have metadata.
+dictionary CreditCardMeta {
+    string guid;
+    i64 time_created;
+    i64? time_last_used;
+    i64 time_last_modified;
+    i64 times_used;
+    i64 sync_change_counter;
+};
+
+/// A credit card together with its metadata, passed to `add_credit_card_with_meta`
+/// and `update_credit_card_with_meta` when importing a record from another store.
+dictionary UpdatableCreditCardFieldsWithMeta {
+    UpdatableCreditCardFields fields;
+    CreditCardMeta meta;
+};
+
+/// A bulk insert result entry, returned per input record by `add_many_credit_cards_with_meta`
+[Enum]
+interface CreditCardBulkResultEntry {
+    Success(CreditCard credit_card);
+    Error(string message);
+};
+
+/// A tombstone for a record deleted locally but not yet uploaded, supplied to
+/// `add_many_credit_card_tombstones` when migrating from another store.
+dictionary CreditCardTombstone {
+    string guid;
+    i64 time_deleted;
+};
+
+/// Per-record result of `add_many_credit_card_tombstones`.
+[Enum]
+interface CreditCardBulkTombstoneResultEntry {
+    Success(string guid);
+    Error(string message);
+};
+
 /// 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
@@ -189,6 +231,27 @@ interface Store {
     [Throws=AutofillApiError]
     void touch_credit_card(string guid);
 
+    [Throws=AutofillApiError]
+    CreditCard add_credit_card_with_meta(UpdatableCreditCardFieldsWithMeta entry_with_meta);
+
+    [Throws=AutofillApiError]
+    sequence<CreditCardBulkResultEntry> add_many_credit_cards_with_meta(sequence<UpdatableCreditCardFieldsWithMeta> entries_with_meta);
+
+    [Throws=AutofillApiError]
+    sequence<CreditCardBulkTombstoneResultEntry> add_many_credit_card_tombstones(sequence<CreditCardTombstone> tombstones);
+
+    /// Removes every credit card and every credit card tombstone.
+    ///
+    /// A migration primitive: it leaves the sync mirror intact and produces no
+    /// tombstones, so the deletions are never uploaded and a synced profile gets
+    /// the records back on the next sync. Use `delete_credit_card` to delete on the
+    /// user's behalf.
+    [Throws=AutofillApiError]
+    void delete_all_credit_cards();
+
+    [Throws=AutofillApiError]
+    void update_credit_card_with_meta(UpdatableCreditCardFieldsWithMeta entry_with_meta);
+
     [Throws=AutofillApiError]
     Address add_address(UpdatableAddressFields a);
 
@@ -202,6 +265,11 @@ interface Store {
     sequence<AddressBulkTombstoneResultEntry> add_many_address_tombstones(sequence<AddressTombstone> tombstones);
 
     /// Removes every address and every address tombstone.
+    ///
+    /// A migration primitive: it leaves the sync mirror intact and produces no
+    /// tombstones, so the deletions are never uploaded and a synced profile gets
+    /// the records back on the next sync. Use `delete_address` to delete on the
+    /// user's behalf.
     [Throws=AutofillApiError]
     void delete_all_addresses();
 

components/autofill/src/db/addresses.rs

@@ -12,6 +12,7 @@ use crate::db::{
     },
     schema::{ADDRESS_COMMON_COLS, ADDRESS_COMMON_VALS},
 };
+use crate::db::{timestamp_from_millis, with_savepoint, CounterUpdate};
 use crate::error::*;
 
 use rusqlite::{Connection, Transaction};
@@ -86,33 +87,6 @@ pub(crate) fn add_many_addresses_with_meta(
     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<T>(
-    tx: &Transaction<'_>,
-    op: impl FnOnce() -> Result<T>,
-) -> Result<std::result::Result<T, Error>> {
-    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
@@ -160,14 +134,6 @@ pub(crate) fn add_many_address_tombstones(
     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,
@@ -345,31 +311,6 @@ 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, see `CounterUpdate`.
 pub(crate) fn update_internal_address(
@@ -950,20 +891,56 @@ mod tests {
     }
 
     #[test]
-    fn test_address_add_with_meta_clamps_negative_timestamps() -> Result<()> {
+    fn test_address_add_with_meta_sanitizes_out_of_range_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)?;
+        // Negative, and the value from bug 2066257 - a negative microsecond
+        // timestamp that a JS consumer already reinterpreted as a u64 and
+        // divided by 1000, so it reaches us as a huge positive number. Both are
+        // "we don't know when", and a `.max(0)` would only catch the first.
+        for (guid, out_of_range) in [("abc", -1), ("def", 18446744071857664)] {
+            let meta = AddressMeta {
+                guid: guid.to_string(),
+                time_created: out_of_range,
+                time_last_used: Some(out_of_range),
+                time_last_modified: out_of_range,
+                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(guid))?;
+            assert_eq!(
+                retrieved.metadata.time_created.as_millis(),
+                0,
+                "{out_of_range} survived"
+            );
+            assert_eq!(retrieved.metadata.time_last_used.as_millis(), 0);
+            assert_eq!(retrieved.metadata.time_last_modified.as_millis(), 0);
+        }
 
-        let retrieved = get_address(&db, &Guid::new("abc"))?;
+        Ok(())
+    }
+
+    /// Surface 2: a value already on disk, put there before the import path
+    /// sanitized anything. Reading it must repair rather than propagate it.
+    #[test]
+    fn test_address_from_row_sanitizes_corrupt_timestamps() -> Result<()> {
+        let db = new_mem_db();
+
+        let address = add_address(&db, test_fields("123 Main Street"))?;
+        db.execute(
+            // Three shapes that are not representable dates: the u64-reinterpreted
+            // value from bug 2066257, a raw negative, and MAX_DATE_MS + 1.
+            "UPDATE addresses_data
+             SET time_created = 18446744071857664,
+                 time_last_used = -1,
+                 time_last_modified = 8640000000000001
+             WHERE guid = :guid",
+            rusqlite::named_params! { ":guid": address.guid },
+        )?;
+
+        let retrieved = get_address(&db, &address.guid)?;
         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);