Clean up commits for sync manager work, a few things still not finish…

…ed before this is actually ready for review

components/logins/src/db.rs

@@ -922,7 +922,7 @@ impl LoginDb {
     }
 }
 
-pub(crate) struct LoginStore<'a> {
+pub struct LoginStore<'a> {
     pub db: &'a LoginDb,
     pub scope: sql_support::SqlInterruptScope,
 }

components/logins/src/engine.rs

@@ -115,6 +115,7 @@ impl PasswordEngine {
             storage_init,
             root_sync_key,
             &store.scope,
+            None,
         );
         // We always update the state - sync_multiple does the right thing
         // if it needs to be dropped (ie, they will be None or contain Nones etc)

components/logins/src/lib.rs

@@ -17,6 +17,8 @@ mod util;
 
 mod ffi;
 
+// Mostly exposed for the sync manager.
+pub use crate::db::LoginStore;
 pub use crate::engine::*;
 pub use crate::error::*;
 pub use crate::login::*;

components/places/src/api/places_api.rs

@@ -68,9 +68,9 @@ lazy_static! {
 
 static ID_COUNTER: AtomicUsize = AtomicUsize::new(0);
 
-struct SyncState {
-    mem_cached_state: Cell<MemoryCachedState>,
-    disk_cached_state: Cell<Option<String>>,
+pub struct SyncState {
+    pub mem_cached_state: Cell<MemoryCachedState>,
+    pub disk_cached_state: Cell<Option<String>>,
 }
 
 /// The entry-point to the places API. This object gives access to database
@@ -242,6 +242,7 @@ impl PlacesApi {
                     client_init,
                     key_bundle,
                     &interruptee,
+                    None,
                 )
             },
         )
@@ -264,6 +265,7 @@ impl PlacesApi {
                     client_init,
                     key_bundle,
                     &interruptee,
+                    None,
                 )
             },
         )
@@ -351,6 +353,7 @@ impl PlacesApi {
             client_init,
             key_bundle,
             &interruptee,
+            None,
         );
         // even on failure we set the persisted state - sync itself takes care
         // to ensure this has been None'd out if necessary.
@@ -382,6 +385,23 @@ impl PlacesApi {
         Ok(())
     }
 
+    pub fn reset_history(&self) -> Result<()> {
+        // Take the lock to prevent syncing while we're doing this.
+        let _guard = self.sync_state.lock().unwrap();
+        let conn = self.open_sync_connection()?;
+
+        // Somewhat ironically, we start by migrating from the legacy storage
+        // format. We *are* just going to delete it anyway, but the code is
+        // simpler if we can just reuse the existing path.
+        HistoryStore::migrate_v1_global_state(&conn)?;
+
+        // We'd rather you didn't interrupt this, but it's a required arg for
+        // HistoryStore
+        let scope = conn.begin_interrupt_scope();
+        let store = HistoryStore::new(&conn, &scope);
+        store.do_reset(&sync15::StoreSyncAssociation::Disconnected)
+    }
+
     /// Get a new interrupt handle for the sync connection.
     pub fn new_sync_conn_interrupt_handle(&self) -> Result<SqlInterruptHandle> {
         // Probably not necessary to lock here, since this should only get

components/places/src/history_sync/store.rs

@@ -89,7 +89,7 @@ impl<'a> HistoryStore<'a> {
         Ok(())
     }
 
-    fn do_reset(&self, assoc: &StoreSyncAssociation) -> Result<()> {
+    pub(crate) fn do_reset(&self, assoc: &StoreSyncAssociation) -> Result<()> {
         let tx = self.db.begin_transaction()?;
         reset_storage(self.db)?;
         self.put_meta(LAST_SYNC_META_KEY, &0)?;

components/sync15/src/lib.rs

@@ -2,7 +2,7 @@
  * 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/. */
 
-#![allow(unknown_lints)]
+#![allow(unknown_lints, clippy::implicit_hasher)]
 #![warn(rust_2018_idioms)]
 
 mod bso_record;

components/sync15/src/state.rs

@@ -87,6 +87,24 @@ pub struct GlobalState {
     pub keys: EncryptedBso,
 }
 
+impl GlobalState {
+    // TODO: this function needs to change once we understand 'accepted'.
+    pub fn update_declined_engines(&mut self, changes: &HashMap<String, bool>) {
+        let mut current_state: HashMap<String, bool> = self.global.declined.iter().map(|e| (e.to_string(), false)).collect();
+        for (e, v) in changes {
+            current_state.insert(e.to_string(), *v);
+        }
+        let new_declined = current_state.into_iter().filter_map(|(e, allow)| {
+            if allow {
+                None
+            } else {
+                Some(e)
+            }
+        }).collect::<Vec<String>>();
+        self.global.declined = new_declined;
+    }
+}
+
 /// Creates a fresh `meta/global` record, using the default engine selections,
 /// and declined engines from our PersistedGlobalState.
 fn new_global(pgs: &PersistedGlobalState) -> error::Result<MetaGlobalRecord> {

components/sync15/src/status.rs

@@ -66,6 +66,9 @@ pub struct SyncResult {
     /// The general health.
     pub service_status: ServiceStatus,
 
+    /// The set of declined engines, if we know them.
+    pub declined: Option<Vec<String>>,
+
     /// The result of the sync.
     pub result: Result<(), Error>,
 

components/sync15/src/sync_multiple.rs

@@ -70,10 +70,12 @@ pub fn sync_multiple(
     storage_init: &Sync15StorageClientInit,
     root_sync_key: &KeyBundle,
     interruptee: &impl Interruptee,
+    engines_to_state_change: Option<&HashMap<String, bool>>,
 ) -> SyncResult {
     let mut sync_result = SyncResult {
         service_status: ServiceStatus::OtherError,
         result: Ok(()),
+        declined: None,
         engine_results: HashMap::with_capacity(stores.len()),
         telemetry: telemetry::SyncTelemetryPing::new(),
     };
@@ -85,6 +87,7 @@ pub fn sync_multiple(
         root_sync_key,
         interruptee,
         &mut sync_result,
+        engines_to_state_change,
     ) {
         Ok(()) => {
             log::debug!(
@@ -114,6 +117,7 @@ fn do_sync_multiple(
     root_sync_key: &KeyBundle,
     interruptee: &impl Interruptee,
     sync_result: &mut SyncResult,
+    engines_to_state_change: Option<&HashMap<String, bool>>,
 ) -> result::Result<(), Error> {
     if interruptee.was_interrupted() {
         sync_result.service_status = ServiceStatus::Interrupted;
@@ -160,7 +164,8 @@ fn do_sync_multiple(
             }
         }
         None => {
-            log::info!("The application didn't give us persisted state - this is only expected on the very first run for a given user.");
+            log::info!("The application didn't give us persisted state - \
+                this is only expected on the very first run for a given user.");
             PersistedGlobalState::default()
         }
     };

components/sync_manager/Cargo.toml

@@ -7,16 +7,19 @@ license = "MPL-2.0"
 
 [dependencies]
 sync15 = { path = "../sync15" }
-places = { path = "../places", optional = true }
-logins = { path = "../logins", optional = true }
+places = { path = "../places" }
+logins = { path = "../logins" }
 ffi-support = { path = "../support/ffi" }
 failure = "0.1.5"
 error-support = { path = "../support/error" }
 prost = "0.5.0"
 prost-derive = "0.5.0"
 bytes = "0.4.12"
-lazy_static = "1.3.0"
-log = "0.4.7"
+lazy_static = "1.4.0"
+log = "0.4.8"
+sql-support = { path = "../support/sql" }
+url = "1.7.2"
+serde_json = "1.0.40"
 
 [build-dependencies]
 prost-build = "0.5.0"

components/sync_manager/README.md

@@ -0,0 +1,13 @@
+# Sync Manager
+
+It's a bit unfortunate this component can't just be part of `sync15`.
+Conceptually and functionally, it shares most in common with with the `sync15`
+crate, and in some cases stretches (or arguably breaks) the abstraction barrier
+that `sync15` puts up.
+
+Unfortunately, places/logins/etc depend on sync15 directly, and so to prevent a
+dependency cycle (which is disallowed by cargo), doing so would require
+implementing the manager in a fully generic manner, with no specific handling of
+underlying crates. This seems extremely difficult to me, so this is split out
+into it's own crate, which might happen to reach into the guts of `sync15` in
+some cases.

components/sync_manager/ffi/Cargo.toml

@@ -6,14 +6,14 @@ edition = "2018"
 license = "MPL-2.0"
 
 [features]
-logins = ["sync_manager/logins", "logins_ffi"]
-places = ["sync_manager/places", "places-ffi"]
+# logins = ["sync_manager/logins", "logins_ffi"]
+# places = ["sync_manager/places", "places-ffi"]
 
 [dependencies]
 sync_manager = { path = ".." }
 sync15 = { path = "../../sync15" }
 ffi-support = { path = "../../support/ffi" }
-places-ffi = { path = "../../places/ffi", optional = true }
-logins_ffi = { path = "../../logins/ffi", optional = true  }
+places-ffi = { path = "../../places/ffi" }#, optional = true }
+logins_ffi = { path = "../../logins/ffi" }#, optional = true }
 prost = "0.5.0"
 log = "0.4.7"

components/sync_manager/ffi/src/lib.rs

@@ -14,7 +14,7 @@ use sync_manager::Result as MgrResult;
 #[no_mangle]
 pub extern "C" fn sync_manager_set_places(_places_api_handle: u64, error: &mut ExternError) {
     ffi_support::call_with_result(error, || -> MgrResult<()> {
-        #[cfg(feature = "places")]
+        // #[cfg(feature = "places")]
         {
             let api = places_ffi::APIS
                 .get_u64(_places_api_handle, |api| -> Result<_, HandleError> {
@@ -23,18 +23,18 @@ pub extern "C" fn sync_manager_set_places(_places_api_handle: u64, error: &mut E
             sync_manager::set_places(api);
             Ok(())
         }
-        #[cfg(not(feature = "places"))]
-        {
-            log::error!("Sync manager not compiled with places support");
-            Err(sync_manager::ErrorKind::UnsupportedFeature("places".to_string()).into())
-        }
+        // #[cfg(not(feature = "places"))]
+        // {
+        //     log::error!("Sync manager not compiled with places support");
+        //     Err(sync_manager::ErrorKind::UnsupportedFeature("places".to_string()).into())
+        // }
     })
 }
 
 #[no_mangle]
 pub extern "C" fn sync_manager_set_logins(_logins_handle: u64, error: &mut ExternError) {
     ffi_support::call_with_result(error, || -> MgrResult<()> {
-        #[cfg(feature = "logins")]
+        // #[cfg(feature = "logins")]
         {
             let api = logins_ffi::ENGINES
                 .get_u64(_logins_handle, |api| -> Result<_, HandleError> {
@@ -43,11 +43,11 @@ pub extern "C" fn sync_manager_set_logins(_logins_handle: u64, error: &mut Exter
             sync_manager::set_logins(api);
             Ok(())
         }
-        #[cfg(not(feature = "logins"))]
-        {
-            log::error!("Sync manager not compiled with logins support");
-            Err(sync_manager::ErrorKind::UnsupportedFeature("logins".to_string()).into())
-        }
+        // #[cfg(not(feature = "logins"))]
+        // {
+        //     log::error!("Sync manager not compiled with logins support");
+        //     Err(sync_manager::ErrorKind::UnsupportedFeature("logins".to_string()).into())
+        // }
     })
 }
 

components/sync_manager/src/error.rs

@@ -15,11 +15,19 @@ pub enum ErrorKind {
     InvalidHandle(#[fail(cause)] ffi_support::HandleError),
     #[fail(display = "Protobuf decode error: {}", _0)]
     ProtobufDecodeError(#[fail(cause)] prost::DecodeError),
+    // Used for things like 'failed to decode the provided sync key because it's
+    // completely the wrong format', etc.
+    #[fail(display = "Sync error: {}", _0)]
+    Sync15Error(#[fail(cause)] sync15::Error),
+    #[fail(display = "URL parse error: {}", _0)]
+    UrlParseError(#[fail(cause)] url::ParseError),
 }
 
 error_support::define_error! {
     ErrorKind {
         (InvalidHandle, ffi_support::HandleError),
         (ProtobufDecodeError, prost::DecodeError),
+        (Sync15Error, sync15::Error),
+        (UrlParseError, url::ParseError),
     }
 }

components/sync_manager/src/lib.rs

@@ -15,26 +15,21 @@ pub mod msg_types {
     include!(concat!(env!("OUT_DIR"), "/msg_types.rs"));
 }
 
-#[cfg(feature = "logins")]
 use logins::PasswordEngine;
 use manager::SyncManager;
-#[cfg(feature = "places")]
 use places::PlacesApi;
-#[cfg(any(feature = "places", feature = "logins"))]
 use std::sync::Arc;
 use std::sync::Mutex;
 
 lazy_static::lazy_static! {
     static ref MANAGER: Mutex<SyncManager> = Mutex::new(SyncManager::new());
 }
 
-#[cfg(feature = "places")]
 pub fn set_places(places: Arc<PlacesApi>) {
     let mut manager = MANAGER.lock().unwrap();
     manager.set_places(places);
 }
 
-#[cfg(feature = "logins")]
 pub fn set_logins(places: Arc<Mutex<PasswordEngine>>) {
     let mut manager = MANAGER.lock().unwrap();
     manager.set_logins(places);
@@ -47,7 +42,5 @@ pub fn disconnect() {
 
 pub fn sync(params: msg_types::SyncParams) -> Result<msg_types::SyncResult> {
     let mut manager = MANAGER.lock().unwrap();
-    // TODO: translate the protobuf message into something nicer to work with in
-    // Rust.
     manager.sync(params)
 }