Wrap the autofill implementation in an Arc<> to help the sync manager.

This patch works-around the lack of the functionlity described
in mozilla/uniffi-rs#417.

In short, it splits the store into 2 parts - the
uniffi-exposed "wrapper" and the actual implementation.
This "wrapper" wraps the implementation in an `Arc<>`, so
that it can hand a clone of this `Arc<>` to the sync manager, so
that it can construct sync engines, causing us to end up with
multiple references to the `Store` being alive but without any
being actual `&` references.

The database itself continues to be wrapped in a `Mutex<>`

It also makes some changes in the sync manager, really just
enough to convince me that it will work - there will be plenty
of other places that will need similar treatment. I chose to
use the `SyncEngine` trait exclusively there, which I think makes
sense because ideally we'd use that trait *everywhere* to avoid the
Sync Manager knowing about the implementation (but it would still
require it to link to all those crates, so that's just a
code-simplicity issue)

components/autofill/Cargo.toml

@@ -12,6 +12,7 @@ anyhow = "1.0"
 error-support = { path = "../support/error" }
 interrupt-support = { path = "../support/interrupt" }
 jwcrypto = { path = "../support/jwcrypto" }
+lazy_static = "1.4"
 log = "0.4"
 serde = "1"
 serde_derive = "1"
@@ -31,7 +32,6 @@ features = ["functions", "bundled", "serde_json", "unlock_notify"]
 
 [dev-dependencies]
 env_logger = { version = "0.7", default-features = false }
-lazy_static = "1.4"
 libsqlite3-sys = "0.20.1"
 
 [build-dependencies]

components/autofill/src/autofill.udl

@@ -86,6 +86,11 @@ enum Error {
    "CryptoError",
 };
 
+// We don't really *need* this to be `Threadsafe` because we have a mutex to
+// manage concurrent DB connections - but we might as well so our Mutexes
+// do what we thing they are doing rather than being controlled by the hidden
+// mutexes non-threadsafe uniffi gives us.
+[Threadsafe]
 interface Store {
     [Throws=Error]
     constructor(string dbpath);
@@ -125,4 +130,6 @@ interface Store {
 
     [Throws=Error]
     void touch_address(string guid);
+
+    void register_with_sync_manager();
 };

components/autofill/src/db/store.rs

@@ -11,35 +11,143 @@ use rusqlite::{
     Connection,
 };
 use sql_support::{self, ConnExt};
+use std::cell::RefCell;
 use std::path::Path;
-use std::sync::{Arc, Mutex};
+use std::sync::{Arc, Mutex, Weak};
 use sync15_traits::SyncEngine;
 use sync_guid::Guid;
 
-#[allow(dead_code)]
+// Our "sync manager" will use whatever is stashed here.
+lazy_static::lazy_static! {
+    // Mutex: just taken long enough to update the inner stuff - needed
+    //        to wrap the RefCell as they aren't `Sync`
+    // RefCell: So we can replace what it holds. Normally you'd use `get_ref()`
+    //          on the mutex and avoid the RefCell entirely, but that requires
+    //          the mutex to be declared as `mut` which is apparently
+    //          impossible in a `lazy_static`
+    // [Arc/Weak]<StoreImpl>: What the sync manager actually needs.
+    pub static ref STORE_FOR_MANAGER: Mutex<RefCell<Weak<StoreImpl>>> = Mutex::new(RefCell::new(Weak::new()));
+}
+
+// This is the type that uniffi exposes. It holds an `Arc<>` around the
+// actual implementation, because we need to hand a clone of this `Arc<>` to
+// the sync manager and to sync engines. One day
+// https://github.com/mozilla/uniffi-rs/issues/417 will give us access to the
+// `Arc<>` uniffi owns, which means we can drop this entirely (ie, `Store` and
+// `StoreImpl` could be re-unified)
+// Sadly, this is `pub` because our `autofill-utils` example uses it.
 pub struct Store {
-    db: Arc<Mutex<AutofillDb>>,
+    store_impl: Arc<StoreImpl>,
 }
 
-#[allow(dead_code)]
 impl Store {
     pub fn new(db_path: impl AsRef<Path>) -> Result<Self> {
         Ok(Self {
-            db: Arc::new(Mutex::new(AutofillDb::new(db_path)?)),
+            store_impl: Arc::new(StoreImpl::new(db_path)?),
         })
     }
 
-    /// Creates a store backed by an in-memory database.
-    #[cfg(test)]
-    pub fn new_memory(db_path: &str) -> Result<Self> {
+    pub fn add_credit_card(&self, fields: UpdatableCreditCardFields) -> Result<CreditCard> {
+        self.store_impl.add_credit_card(fields)
+    }
+
+    pub fn get_credit_card(&self, guid: String) -> Result<CreditCard> {
+        self.store_impl.get_credit_card(guid)
+    }
+
+    pub fn get_all_credit_cards(&self) -> Result<Vec<CreditCard>> {
+        self.store_impl.get_all_credit_cards()
+    }
+
+    pub fn update_credit_card(
+        &self,
+        guid: String,
+        credit_card: UpdatableCreditCardFields,
+    ) -> Result<()> {
+        self.store_impl.update_credit_card(guid, credit_card)
+    }
+
+    pub fn delete_credit_card(&self, guid: String) -> Result<bool> {
+        self.store_impl.delete_credit_card(guid)
+    }
+
+    pub fn touch_credit_card(&self, guid: String) -> Result<()> {
+        self.store_impl.touch_credit_card(guid)
+    }
+
+    pub fn add_address(&self, new_address: UpdatableAddressFields) -> Result<Address> {
+        self.store_impl.add_address(new_address)
+    }
+
+    pub fn get_address(&self, guid: String) -> Result<Address> {
+        self.store_impl.get_address(guid)
+    }
+
+    pub fn get_all_addresses(&self) -> Result<Vec<Address>> {
+        self.store_impl.get_all_addresses()
+    }
+
+    pub fn update_address(&self, guid: String, address: UpdatableAddressFields) -> Result<()> {
+        self.store_impl.update_address(guid, address)
+    }
+
+    pub fn delete_address(&self, guid: String) -> Result<bool> {
+        self.store_impl.delete_address(guid)
+    }
+
+    pub fn touch_address(&self, guid: String) -> Result<()> {
+        self.store_impl.touch_address(guid)
+    }
+
+    // This allows the embedding app to say "make this instance available to
+    // the sync manager". The implementation is more like "offer to sync mgr"
+    // (thereby avoiding us needing to link with the sync manager) but
+    // `register_with_sync_manager()` is logically what's happening so that's
+    // the name it gets.
+    pub fn register_with_sync_manager(&self) {
+        STORE_FOR_MANAGER
+            .lock()
+            .unwrap()
+            .replace(Arc::downgrade(&self.store_impl));
+    }
+
+    // These 2 are odd ones out - they don't just delegate but instead
+    // hand off the Arc.
+    // Currently the only consumer of this is our "example" (and hence why they
+    // are `pub` and not `pub(crate)`) - the sync manager duplicates it (because
+    // it doesn't have a reference to us, just to the store_impl)
+    // We could probably make the example work with the sync manager - but then
+    // our example would link with places and logins etc.
+    pub fn create_credit_cards_sync_engine(&self) -> Box<dyn SyncEngine> {
+        Box::new(crate::sync::credit_card::create_engine(
+            self.store_impl.clone(),
+        ))
+    }
+
+    pub fn create_addresses_sync_engine(&self) -> Box<dyn SyncEngine> {
+        Box::new(crate::sync::address::create_engine(self.store_impl.clone()))
+    }
+}
+
+// This is the actual implementation. All code in this crate works with this.
+// Sadly, it's forced to be `pub` because the SyncManager also uses it.
+pub struct StoreImpl {
+    pub(crate) db: Mutex<AutofillDb>,
+}
+
+impl StoreImpl {
+    pub fn new(db_path: impl AsRef<Path>) -> Result<Self> {
         Ok(Self {
-            db: Arc::new(Mutex::new(AutofillDb::new_memory(db_path)?)),
+            db: Mutex::new(AutofillDb::new(db_path)?),
         })
     }
 
+    /// Creates a store backed by an in-memory database.
     #[cfg(test)]
-    pub fn db(&self) -> Arc<Mutex<AutofillDb>> {
-        self.db.clone()
+    pub fn new_memory() -> Self {
+        Self {
+            db: Mutex::new(crate::db::test::new_mem_db()),
+        }
     }
 
     pub fn add_credit_card(&self, fields: UpdatableCreditCardFields) -> Result<CreditCard> {
@@ -108,14 +216,6 @@ impl Store {
     pub fn touch_address(&self, guid: String) -> Result<()> {
         addresses::touch(&self.db.lock().unwrap().writer, &Guid::new(&guid))
     }
-
-    pub fn create_credit_cards_sync_engine(&self) -> Box<dyn SyncEngine> {
-        Box::new(crate::sync::credit_card::create_engine(self.db.clone()))
-    }
-
-    pub fn create_addresses_sync_engine(&self) -> Box<dyn SyncEngine> {
-        Box::new(crate::sync::address::create_engine(self.db.clone()))
-    }
 }
 
 pub(crate) fn put_meta(conn: &Connection, key: &str, value: &dyn ToSql) -> Result<()> {

components/autofill/src/lib.rs

@@ -11,6 +11,9 @@ pub mod encryption;
 pub mod error;
 pub mod sync;
 
+// Re-export stuff the sync manager needs.
+pub use crate::db::store::{StoreImpl, STORE_FOR_MANAGER};
+
 // Expose stuff needed by the uniffi generated code.
 use crate::db::models::address::*;
 use crate::db::models::credit_card::*;

components/autofill/src/sync/address/mod.rs

@@ -17,18 +17,18 @@ use incoming::IncomingAddressesImpl;
 use outgoing::OutgoingAddressesImpl;
 use rusqlite::Transaction;
 use serde::{Deserialize, Serialize};
-use std::sync::{Arc, Mutex};
+use std::sync::Arc;
 use sync_guid::Guid;
 use types::Timestamp;
 
 // The engine.
-pub fn create_engine(db: Arc<Mutex<crate::db::AutofillDb>>) -> ConfigSyncEngine<InternalAddress> {
+pub fn create_engine(store: Arc<crate::StoreImpl>) -> ConfigSyncEngine<InternalAddress> {
     ConfigSyncEngine::new(
         EngineConfig {
             namespace: "addresses".to_string(),
             collection: "addresses",
         },
-        db,
+        store,
         Box::new(AddressesEngineStorageImpl {}),
     )
 }

components/autofill/src/sync/credit_card/mod.rs

@@ -19,20 +19,18 @@ use incoming::IncomingCreditCardsImpl;
 use outgoing::OutgoingCreditCardsImpl;
 use rusqlite::Transaction;
 use serde::{Deserialize, Serialize};
-use std::sync::{Arc, Mutex};
+use std::sync::Arc;
 use sync_guid::Guid;
 use types::Timestamp;
 
 // The engine.
-pub fn create_engine(
-    db: Arc<Mutex<crate::db::AutofillDb>>,
-) -> ConfigSyncEngine<InternalCreditCard> {
+pub fn create_engine(store: Arc<crate::StoreImpl>) -> ConfigSyncEngine<InternalCreditCard> {
     ConfigSyncEngine::new(
         EngineConfig {
             namespace: "credit_cards".to_string(),
             collection: "creditcards",
         },
-        db,
+        store,
         Box::new(CreditCardsEngineStorageImpl {}),
     )
 }