From 6651984e3b456d99fad0df1deb727b7c2f62ec16 Mon Sep 17 00:00:00 2001 From: lougeniaC64 Date: Fri, 6 Sep 2024 14:17:59 -0400 Subject: [PATCH] Revert "Exposed webext-storage bridged engine logic" This reverts commit b94438a80dc0e6b9fbd521ac99b032dd1e95608b. --- CHANGELOG.md | 5 - components/webext-storage/src/db.rs | 1 - components/webext-storage/src/error.rs | 8 -- components/webext-storage/src/lib.rs | 15 --- components/webext-storage/src/store.rs | 11 +- components/webext-storage/src/sync/bridge.rs | 100 +----------------- components/webext-storage/src/sync/mod.rs | 3 +- .../webext-storage/src/webext-storage.udl | 63 ----------- 8 files changed, 13 insertions(+), 193 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 783f257a78..d5c7801b3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,11 +44,6 @@ ### Suggest - Removed the deprecated `remote_settings_config` method. No consumers were using this. -### Webext-Storage -- Exposed the bridged engine logic for use in desktop ([#6180](https://github.com/mozilla/application-services/pull/6180)). - - This updates the signature of the `bridged_engine` function technically making this a breaking change though the - references to function with the old signature are being removed in a desktop patch. - ## ✨ What's New ✨ ### Glean diff --git a/components/webext-storage/src/db.rs b/components/webext-storage/src/db.rs index e8735fbd86..e24c8ea97c 100644 --- a/components/webext-storage/src/db.rs +++ b/components/webext-storage/src/db.rs @@ -120,7 +120,6 @@ impl ThreadSafeStorageDb { Arc::clone(&self.interrupt_handle) } - #[allow(dead_code)] pub fn begin_interrupt_scope(&self) -> Result { Ok(self.interrupt_handle.begin_interrupt_scope()?) } diff --git a/components/webext-storage/src/error.rs b/components/webext-storage/src/error.rs index 7774f15dce..11ed250436 100644 --- a/components/webext-storage/src/error.rs +++ b/components/webext-storage/src/error.rs @@ -143,11 +143,3 @@ impl From for WebExtStorageApiError { } } } - -impl From for WebExtStorageApiError { - fn from(value: anyhow::Error) -> Self { - WebExtStorageApiError::UnexpectedError { - reason: value.to_string(), - } - } -} diff --git a/components/webext-storage/src/lib.rs b/components/webext-storage/src/lib.rs index 8f3da9d6b9..dd9e72006b 100644 --- a/components/webext-storage/src/lib.rs +++ b/components/webext-storage/src/lib.rs @@ -25,7 +25,6 @@ pub use api::SYNC_QUOTA_BYTES_PER_ITEM; pub use crate::error::{QuotaReason, WebExtStorageApiError}; pub use crate::store::WebExtStorageStore; -pub use crate::sync::{bridge::WebExtStorageBridgedEngine, SyncedExtensionChange}; pub use api::UsageInfo; pub use api::{StorageChanges, StorageValueChange}; @@ -43,17 +42,3 @@ impl UniffiCustomTypeConverter for JsonValue { obj.to_string() } } - -// Our UDL uses a `Guid` type. -use sync_guid::Guid; -impl UniffiCustomTypeConverter for Guid { - type Builtin = String; - - fn into_custom(val: Self::Builtin) -> uniffi::Result { - Ok(Guid::new(val.as_str())) - } - - fn from_custom(obj: Self) -> Self::Builtin { - obj.into() - } -} diff --git a/components/webext-storage/src/store.rs b/components/webext-storage/src/store.rs index ab697518fd..d9c3281dd4 100644 --- a/components/webext-storage/src/store.rs +++ b/components/webext-storage/src/store.rs @@ -29,7 +29,7 @@ use serde_json::Value as JsonValue; /// connection with our sync engines - ie, these engines also hold an Arc<> /// around the same object. pub struct WebExtStorageStore { - pub(crate) db: Arc, + db: Arc, } impl WebExtStorageStore { @@ -119,9 +119,14 @@ impl WebExtStorageStore { /// Returns the bytes in use for the specified items (which can be null, /// a string, or an array) - pub fn get_bytes_in_use(&self, ext_id: &str, keys: JsonValue) -> Result { + pub fn get_bytes_in_use(&self, ext_id: &str, keys: JsonValue) -> Result { let db = self.db.lock(); - Ok(api::get_bytes_in_use(&db, ext_id, keys)? as u64) + api::get_bytes_in_use(&db, ext_id, keys) + } + + /// Returns a bridged sync engine for Desktop for this store. + pub fn bridged_engine(&self) -> sync::BridgedEngine { + sync::BridgedEngine::new(&self.db) } /// Closes the store and its database connection. See the docs for diff --git a/components/webext-storage/src/sync/bridge.rs b/components/webext-storage/src/sync/bridge.rs index 1b7d5e92fb..f948003f7a 100644 --- a/components/webext-storage/src/sync/bridge.rs +++ b/components/webext-storage/src/sync/bridge.rs @@ -5,30 +5,18 @@ use anyhow::Result; use rusqlite::Transaction; use std::sync::{Arc, Weak}; -use sync15::bso::{IncomingBso, OutgoingBso}; -use sync15::engine::{ApplyResults, BridgedEngine as Sync15BridgedEngine}; +use sync15::bso::IncomingBso; +use sync15::engine::ApplyResults; use sync_guid::Guid as SyncGuid; use crate::db::{delete_meta, get_meta, put_meta, ThreadSafeStorageDb}; use crate::schema; use crate::sync::incoming::{apply_actions, get_incoming, plan_incoming, stage_incoming}; use crate::sync::outgoing::{get_outgoing, record_uploaded, stage_outgoing}; -use crate::WebExtStorageStore; const LAST_SYNC_META_KEY: &str = "last_sync_time"; const SYNC_ID_META_KEY: &str = "sync_id"; -impl WebExtStorageStore { - // Returns a bridged sync engine for this store. - pub fn bridged_engine(self: Arc) -> Arc { - let engine = Box::new(BridgedEngine::new(&self.db)); - let bridged_engine = WebExtStorageBridgedEngine { - bridge_impl: engine, - }; - Arc::new(bridged_engine) - } -} - /// A bridged engine implements all the methods needed to make the /// `storage.sync` store work with Desktop's Sync implementation. /// Conceptually, it's similar to `sync15::Store`, which we @@ -66,7 +54,7 @@ impl BridgedEngine { } } -impl Sync15BridgedEngine for BridgedEngine { +impl sync15::engine::BridgedEngine for BridgedEngine { fn last_sync(&self) -> Result { let shared_db = self.thread_safe_storage_db()?; let db = shared_db.lock(); @@ -194,88 +182,6 @@ impl Sync15BridgedEngine for BridgedEngine { } } -pub struct WebExtStorageBridgedEngine { - bridge_impl: Box, -} - -impl WebExtStorageBridgedEngine { - pub fn new(bridge_impl: Box) -> Self { - Self { bridge_impl } - } - - pub fn last_sync(&self) -> Result { - self.bridge_impl.last_sync() - } - - pub fn set_last_sync(&self, last_sync: i64) -> Result<()> { - self.bridge_impl.set_last_sync(last_sync) - } - - pub fn sync_id(&self) -> Result> { - self.bridge_impl.sync_id() - } - - pub fn reset_sync_id(&self) -> Result { - self.bridge_impl.reset_sync_id() - } - - pub fn ensure_current_sync_id(&self, sync_id: &str) -> Result { - self.bridge_impl.ensure_current_sync_id(sync_id) - } - - pub fn prepare_for_sync(&self, client_data: &str) -> Result<()> { - self.bridge_impl.prepare_for_sync(client_data) - } - - pub fn store_incoming(&self, incoming: Vec) -> Result<()> { - self.bridge_impl - .store_incoming(self.convert_incoming_bsos(incoming)?) - } - - pub fn apply(&self) -> Result> { - let apply_results = self.bridge_impl.apply()?; - self.convert_outgoing_bsos(apply_results.records) - } - - pub fn set_uploaded(&self, server_modified_millis: i64, guids: Vec) -> Result<()> { - self.bridge_impl - .set_uploaded(server_modified_millis, &guids) - } - - pub fn sync_started(&self) -> Result<()> { - self.bridge_impl.sync_started() - } - - pub fn sync_finished(&self) -> Result<()> { - self.bridge_impl.sync_finished() - } - - pub fn reset(&self) -> Result<()> { - self.bridge_impl.reset() - } - - pub fn wipe(&self) -> Result<()> { - self.bridge_impl.wipe() - } - - fn convert_incoming_bsos(&self, incoming: Vec) -> Result> { - let mut bsos = Vec::with_capacity(incoming.len()); - for inc in incoming { - bsos.push(serde_json::from_str::(&inc)?); - } - Ok(bsos) - } - - // Encode OutgoingBso's into JSON for UniFFI - fn convert_outgoing_bsos(&self, outgoing: Vec) -> Result> { - let mut bsos = Vec::with_capacity(outgoing.len()); - for e in outgoing { - bsos.push(serde_json::to_string(&e)?); - } - Ok(bsos) - } -} - impl From for crate::error::Error { fn from(value: anyhow::Error) -> Self { crate::error::Error::SyncError(value.to_string()) diff --git a/components/webext-storage/src/sync/mod.rs b/components/webext-storage/src/sync/mod.rs index b1e8e9d4f9..22ff912def 100644 --- a/components/webext-storage/src/sync/mod.rs +++ b/components/webext-storage/src/sync/mod.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/. */ -pub(crate) mod bridge; +mod bridge; mod incoming; mod outgoing; @@ -17,6 +17,7 @@ use serde_derive::*; use sql_support::ConnExt; use sync_guid::Guid as SyncGuid; +pub use bridge::BridgedEngine; use incoming::IncomingAction; type JsonMap = serde_json::Map; diff --git a/components/webext-storage/src/webext-storage.udl b/components/webext-storage/src/webext-storage.udl index 3578804a9b..f912861030 100644 --- a/components/webext-storage/src/webext-storage.udl +++ b/components/webext-storage/src/webext-storage.udl @@ -5,9 +5,6 @@ [Custom] typedef string JsonValue; -[Custom] -typedef string Guid; - namespace webextstorage { }; @@ -25,11 +22,6 @@ interface WebExtStorageApiError { QuotaError(QuotaReason reason); }; -dictionary SyncedExtensionChange { - string ext_id; - string changes; -}; - dictionary StorageValueChange { string key; JsonValue? old_value; @@ -40,9 +32,6 @@ dictionary StorageChanges { sequence changes; }; -// Note that the `close` function has been intentionally excluded from `WebExtStorageStore` because at present -// it is not necessary for our current use in mozilla central and converting `close` to pass a reference to -// the store resulted in errors. interface WebExtStorageStore { [Throws=WebExtStorageApiError] constructor(string path); @@ -53,61 +42,9 @@ interface WebExtStorageStore { [Throws=WebExtStorageApiError] JsonValue get([ByRef] string ext_id, JsonValue keys); - [Throws=WebExtStorageApiError] - u64 get_bytes_in_use([ByRef] string ext_id, JsonValue keys); - [Throws=WebExtStorageApiError] StorageChanges remove([ByRef] string ext_id, JsonValue keys); [Throws=WebExtStorageApiError] StorageChanges clear([ByRef] string ext_id); - - [Self=ByArc] - WebExtStorageBridgedEngine bridged_engine(); - - [Throws=WebExtStorageApiError] - sequence get_synced_changes(); -}; - -// Note the canonical docs for this are in https://github.com/mozilla/application-services/blob/main/components/sync15/src/engine/bridged_engine.rs -// NOTE: all timestamps here are milliseconds. -interface WebExtStorageBridgedEngine { - [Throws=WebExtStorageApiError] - i64 last_sync(); - - [Throws=WebExtStorageApiError] - void set_last_sync(i64 last_sync); - - [Throws=WebExtStorageApiError] - string? sync_id(); - - [Throws=WebExtStorageApiError] - string reset_sync_id(); - - [Throws=WebExtStorageApiError] - string ensure_current_sync_id([ByRef]string new_sync_id); - - [Throws=WebExtStorageApiError] - void prepare_for_sync([ByRef]string client_data); - - [Throws=WebExtStorageApiError] - void sync_started(); - - [Throws=WebExtStorageApiError] - void store_incoming(sequence incoming); - - [Throws=WebExtStorageApiError] - sequence apply(); - - [Throws=WebExtStorageApiError] - void set_uploaded(i64 server_modified_millis, sequence guids); - - [Throws=WebExtStorageApiError] - void sync_finished(); - - [Throws=WebExtStorageApiError] - void reset(); - - [Throws=WebExtStorageApiError] - void wipe(); };