Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove obsolete iOS sync logic #5725

Merged
merged 1 commit into from Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,11 @@
# v117.0 (In progress)

## General

### 🦊 What's Changed 🦊

- Removed obsolete sync functions that were exposed for Firefox iOS prior to the sync manager component integration ([#5725](https://github.com/mozilla/application-services/pull/5725)).

## Nimbus SDK ⛅️🔬🔭

### ✨ What's New ✨
Expand Down
4 changes: 4 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 0 additions & 28 deletions components/logins/ios/Logins/LoginsStorage.swift
Expand Up @@ -21,21 +21,6 @@ open class LoginsStorage {
store = try LoginStore(path: databasePath)
}

/// Delete all locally stored login sync metadata. It's unclear if
/// there's ever a reason for users to call this
open func reset() throws {
try queue.sync {
try self.store.reset()
}
}

/// Delete all locally stored login data.
open func wipe() throws {
try queue.sync {
try self.store.wipe()
}
}

open func wipeLocal() throws {
try queue.sync {
try self.store.wipeLocal()
Expand Down Expand Up @@ -103,19 +88,6 @@ open class LoginsStorage {
self.store.registerWithSyncManager()
}
}

open func sync(unlockInfo: SyncUnlockInfo) throws -> String {
return try queue.sync {
try self.store
.sync(
keyId: unlockInfo.kid,
accessToken: unlockInfo.fxaAccessToken,
syncKey: unlockInfo.syncKey,
tokenserverUrl: unlockInfo.tokenserverURL,
localEncryptionKey: unlockInfo.loginEncryptionKey
)
}
}
}

public func migrateLoginsFromSqlcipher(
Expand Down
3 changes: 0 additions & 3 deletions components/logins/src/logins.udl
Expand Up @@ -158,7 +158,4 @@ interface LoginStore {

[Self=ByArc]
void register_with_sync_manager();

[Throws=LoginsApiError, Self=ByArc]
string sync(string key_id, string access_token, string sync_key, string tokenserver_url, string local_encryption_key);
};
58 changes: 0 additions & 58 deletions components/logins/src/store.rs
Expand Up @@ -9,7 +9,6 @@ use crate::LoginsSyncEngine;
use parking_lot::Mutex;
use std::path::Path;
use std::sync::{Arc, Weak};
use sync15::client::{sync_multiple, MemoryCachedState, Sync15StorageClientInit};
use sync15::engine::{EngineSyncAssociation, SyncEngine, SyncEngineId};

// Our "sync manager" will use whatever is stashed here.
Expand Down Expand Up @@ -150,63 +149,6 @@ impl LoginStore {
self.db.lock().add_or_update(entry, &encdec)
}

/// A convenience wrapper around sync_multiple.
// Unfortunately, iOS still uses this until they use the sync manager
// This can almost die later - consumers should never call it (they should
// use the sync manager) and any of our examples probably can too!
// Once this dies, `mem_cached_state` can die too.
#[handle_error(Error)]
pub fn sync(
self: Arc<Self>,
key_id: String,
access_token: String,
sync_key: String,
tokenserver_url: String,
local_encryption_key: String,
) -> ApiResult<String> {
let mut engine = LoginsSyncEngine::new(Arc::clone(&self))?;
engine
.set_local_encryption_key(&local_encryption_key)
.unwrap();

// This is a bit hacky but iOS still uses sync() and we can only pass strings over ffi
// Below was ported from the "C" ffi code that does essentially the same thing
let storage_init = &Sync15StorageClientInit {
key_id,
access_token,
tokenserver_url: url::Url::parse(tokenserver_url.as_str())?,
};
let root_sync_key = &sync15::KeyBundle::from_ksync_base64(sync_key.as_str())?;

let mut disk_cached_state = engine.get_global_state()?;
let mut mem_cached_state = MemoryCachedState::default();

let mut result = sync_multiple(
&[&engine],
&mut disk_cached_state,
&mut mem_cached_state,
storage_init,
root_sync_key,
&engine.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)
engine.set_global_state(&disk_cached_state)?;

// for b/w compat reasons, we do some dances with the result.
// XXX - note that this means telemetry isn't going to be reported back
// to the app - we need to check with lockwise about whether they really
// need these failures to be reported or whether we can loosen this.
if let Err(e) = result.result {
return Err(e.into());
}
match result.engine_results.remove("passwords") {
None | Some(Ok(())) => Ok(serde_json::to_string(&result.telemetry).unwrap()),
Some(Err(e)) => Err(e.into()),
}
}

// 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
Expand Down
106 changes: 0 additions & 106 deletions components/places/ios/Places/Places.swift
Expand Up @@ -70,105 +70,6 @@ public class PlacesAPI {
}
}

/**
* Sync the bookmarks collection.
*
* - Returns: A JSON string representing a telemetry ping for this sync. The
* string contains the ping payload, and should be sent to the
* telemetry submission endpoint.
*
* - Throws:
* - `PlacesApiError.databaseInterrupted`: If a call is made to `interrupt()` on this
* object from another thread.
* - `PlacesApiError.unexpected`: When an error that has not specifically been exposed
* to Swift is encountered (for example IO errors from
* the database code, etc).
* - `PlacesApiError.panic`: If the rust code panics while completing this
* operation. (If this occurs, please let us know).
*/
open func syncBookmarks(unlockInfo: SyncUnlockInfo) throws -> String {
return try queue.sync {
try self.api.bookmarksSync(
keyId: unlockInfo.kid,
accessToken: unlockInfo.fxaAccessToken,
syncKey: unlockInfo.syncKey,
tokenserverUrl: unlockInfo.tokenserverURL
)
}
}

/**
* Sync the History collection.
*
* - Returns: A JSON string representing a telemetry ping for this sync. The
* string contains the ping payload, and should be sent to the
* telemetry submission endpoint.
*
* - Throws:
* - `PlacesApiError.databaseInterrupted`: If a call is made to `interrupt()` on this
* object from another thread.
* - `PlacesApiError.unexpected`: When an error that has not specifically been exposed
* to Swift is encountered (for example IO errors from
* the database code, etc).
* - `PlacesApiError.panic`: If the rust code panics while completing this
* operation. (If this occurs, please let us know).
*/
open func syncHistory(unlockInfo: SyncUnlockInfo) throws -> String {
return try queue.sync {
try self.api.historySync(
keyId: unlockInfo.kid,
accessToken: unlockInfo.fxaAccessToken,
syncKey: unlockInfo.syncKey,
tokenserverUrl: unlockInfo.tokenserverURL
)
}
}

/**
* Resets all sync metadata for history, including change flags,
* sync statuses, and last sync time. The next sync after reset
* will behave the same way as a first sync when connecting a new
* device.
*
* This method only needs to be called when the user disconnects
* from Sync. There are other times when Places resets sync metadata,
* but those are handled internally in the Rust code.
*
* - Throws:
* - `PlacesApiError.databaseInterrupted`: If a call is made to `interrupt()` on this
* object from another thread.
* - `PlacesApiError.unexpected`: When an error that has not specifically been exposed
* to Swift is encountered (for example IO errors from
* the database code, etc).
* - `PlacesApiError.panic`: If the rust code panics while completing this
* operation. (If this occurs, please let us know).
*/
open func resetHistorySyncMetadata() throws {
return try queue.sync {
try self.api.resetHistory()
}
}

/**
* Resets all sync metadata for bookmarks, including change flags, sync statuses, and
* last sync time. The next sync after reset will behave the same way as a first sync
* when connecting a new device.
*
* - Throws:
* - `PlacesApiError.databaseInterrupted`: If a call is made to `interrupt()` on this
* object from another thread.
* - `PlacesApiError.unexpected`: When an error that has not specifically been exposed
* to Swift is encountered (for example IO errors from
* the database code, etc).
* - `PlacesApiError.panic`: If the rust code panics while completing this
* operation. (If this occurs, please let us know).
*/
open func resetBookmarkSyncMetadata() throws {
return try queue.sync {
try self.api.bookmarksReset()
}
}

open func registerWithSyncManager() {
queue.sync {
self.api.registerWithSyncManager()
Expand Down Expand Up @@ -877,13 +778,6 @@ public class PlacesWriteConnection: PlacesReadConnection {
}
}

open func wipeLocalHistory() throws {
try queue.sync {
try self.checkApi()
try self.conn.wipeLocalHistory()
}
}

open func deleteEverythingHistory() throws {
try queue.sync {
try self.checkApi()
Expand Down
2 changes: 1 addition & 1 deletion components/sync_manager/Cargo.toml
Expand Up @@ -11,7 +11,7 @@ autofill = { path = "../autofill" }
sync15 = { path = "../sync15", features = ["sync-client"] }
places = { path = "../places" }
logins = { path = "../logins" }
tabs = { path = "../tabs", features = ["full-sync"] }
tabs = { path = "../tabs" }
thiserror = "1.0"
anyhow = "1.0"
lazy_static = "1.4"
Expand Down
13 changes: 0 additions & 13 deletions components/tabs/Cargo.toml
Expand Up @@ -6,19 +6,6 @@ authors = ["application-services@mozilla.com"]
license = "MPL-2.0"
exclude = ["/android", "/ios"]

[features]
# When used in desktop we *do not* want the full-sync implementation so desktop
# doesn't get our crypto etc.
# When used on mobile, for simplicity we *do* still expose the unused "bridged engine"
# because none of the engine implementations need the crypto.
default = []

# TODO: we've enabled the "standalone-sync" feature - see the description
# of this feature in sync15's Cargo.toml for what we should do instead.
# (The short version here is that once tabs doesn't need to expose a `sync()`
# method for iOS, we can kill the `full-sync` feature entirely)
full-sync = ["sync15/standalone-sync"]

[dependencies]
anyhow = "1.0"
error-support = { path = "../support/error" }
Expand Down
23 changes: 0 additions & 23 deletions components/tabs/ios/Tabs/Tabs.swift
Expand Up @@ -26,32 +26,9 @@ open class TabsStorage {
}
}

open func reset() throws {
try queue.sync {
try self.store.reset()
}
}

open func registerWithSyncManager() {
queue.sync {
self.store.registerWithSyncManager()
}
}

open func sync(unlockInfo: SyncUnlockInfo) throws -> String {
guard let tabsLocalId = unlockInfo.tabsLocalId else {
throw TabsApiError.UnexpectedTabsError(reason: "tabs local ID was not provided")
}

return try queue.sync {
try self.store
.sync(
keyId: unlockInfo.kid,
accessToken: unlockInfo.fxaAccessToken,
syncKey: unlockInfo.syncKey,
tokenserverUrl: unlockInfo.tokenserverURL,
localId: tabsLocalId
)
}
}
}
3 changes: 2 additions & 1 deletion components/tabs/src/sync/engine.rs
Expand Up @@ -108,7 +108,8 @@ impl RemoteTab {
// (We hope to get these 2 engines even closer in the future, but for now, we suck this up)
pub struct TabsEngine {
pub(super) store: Arc<TabsStore>,
pub(super) local_id: RwLock<String>,
// local_id is made public for use in examples/tabs-sync
pub local_id: RwLock<String>,
}

impl TabsEngine {
Expand Down