diff --git a/CHANGELOG.md b/CHANGELOG.md index e08d8e07d..5b5c818a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### Changes ### Fixes +- share stock string translations across accounts created by the same account manager #3640 ## 1.96.0 diff --git a/benches/contacts.rs b/benches/contacts.rs index 130988b8c..aef0b7e3f 100644 --- a/benches/contacts.rs +++ b/benches/contacts.rs @@ -1,6 +1,7 @@ use criterion::{black_box, criterion_group, criterion_main, Criterion}; use deltachat::contact::Contact; use deltachat::context::Context; +use deltachat::stock_str::StockStrings; use deltachat::Events; use tempfile::tempdir; @@ -8,7 +9,9 @@ async fn address_book_benchmark(n: u32, read_count: u32) { let dir = tempdir().unwrap(); let dbfile = dir.path().join("db.sqlite"); let id = 100; - let context = Context::new(&dbfile, id, Events::new()).await.unwrap(); + let context = Context::new(&dbfile, id, Events::new(), StockStrings::new()) + .await + .unwrap(); let book = (0..n) .map(|i| format!("Name {}\naddr{}@example.org\n", i, i)) diff --git a/benches/get_chat_msgs.rs b/benches/get_chat_msgs.rs index 2c81094b4..61d39e0e7 100644 --- a/benches/get_chat_msgs.rs +++ b/benches/get_chat_msgs.rs @@ -5,11 +5,14 @@ use criterion::{black_box, criterion_group, criterion_main, Criterion}; use deltachat::chat::{self, ChatId}; use deltachat::chatlist::Chatlist; use deltachat::context::Context; +use deltachat::stock_str::StockStrings; use deltachat::Events; async fn get_chat_msgs_benchmark(dbfile: &Path, chats: &[ChatId]) { let id = 100; - let context = Context::new(dbfile, id, Events::new()).await.unwrap(); + let context = Context::new(dbfile, id, Events::new(), StockStrings::new()) + .await + .unwrap(); for c in chats.iter().take(10) { black_box(chat::get_chat_msgs(&context, *c, 0).await.ok()); @@ -23,7 +26,7 @@ fn criterion_benchmark(c: &mut Criterion) { let rt = tokio::runtime::Runtime::new().unwrap(); let chats: Vec<_> = rt.block_on(async { - let context = Context::new(Path::new(&path), 100, Events::new()) + let context = Context::new(Path::new(&path), 100, Events::new(), StockStrings::new()) .await .unwrap(); let chatlist = Chatlist::try_load(&context, 0, None, None).await.unwrap(); diff --git a/benches/get_chatlist.rs b/benches/get_chatlist.rs index 25dd09cab..3b7a0ca1d 100644 --- a/benches/get_chatlist.rs +++ b/benches/get_chatlist.rs @@ -4,6 +4,7 @@ use criterion::{black_box, criterion_group, criterion_main, Criterion}; use deltachat::chatlist::Chatlist; use deltachat::context::Context; +use deltachat::stock_str::StockStrings; use deltachat::Events; async fn get_chat_list_benchmark(context: &Context) { @@ -16,7 +17,7 @@ fn criterion_benchmark(c: &mut Criterion) { if let Ok(path) = std::env::var("DELTACHAT_BENCHMARK_DATABASE") { let rt = tokio::runtime::Runtime::new().unwrap(); let context = rt.block_on(async { - Context::new(Path::new(&path), 100, Events::new()) + Context::new(Path::new(&path), 100, Events::new(), StockStrings::new()) .await .unwrap() }); diff --git a/benches/receive_emails.rs b/benches/receive_emails.rs index e51509556..1e9e5aa43 100644 --- a/benches/receive_emails.rs +++ b/benches/receive_emails.rs @@ -6,6 +6,7 @@ use deltachat::{ context::Context, imex::{imex, ImexMode}, receive_imf::receive_imf, + stock_str::StockStrings, Events, }; use tempfile::tempdir; @@ -41,7 +42,9 @@ async fn create_context() -> Context { let dir = tempdir().unwrap(); let dbfile = dir.path().join("db.sqlite"); let id = 100; - let context = Context::new(&dbfile, id, Events::new()).await.unwrap(); + let context = Context::new(&dbfile, id, Events::new(), StockStrings::new()) + .await + .unwrap(); let backup: PathBuf = std::env::current_dir() .unwrap() diff --git a/benches/search_msgs.rs b/benches/search_msgs.rs index a91219542..48333696d 100644 --- a/benches/search_msgs.rs +++ b/benches/search_msgs.rs @@ -1,11 +1,12 @@ use criterion::{black_box, criterion_group, criterion_main, Criterion}; use deltachat::context::Context; +use deltachat::stock_str::StockStrings; use deltachat::Events; use std::path::Path; async fn search_benchmark(dbfile: impl AsRef) { let id = 100; - let context = Context::new(dbfile.as_ref(), id, Events::new()) + let context = Context::new(dbfile.as_ref(), id, Events::new(), StockStrings::new()) .await .unwrap(); diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index e2e0cc143..a0c007bd5 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -38,6 +38,7 @@ use deltachat::ephemeral::Timer as EphemeralTimer; use deltachat::key::DcKey; use deltachat::message::MsgId; use deltachat::stock_str::StockMessage; +use deltachat::stock_str::StockStrings; use deltachat::webxdc::StatusUpdateSerial; use deltachat::*; use deltachat::{accounts::Accounts, log::LogExt}; @@ -98,7 +99,12 @@ pub unsafe extern "C" fn dc_context_new( let ctx = if blobdir.is_null() || *blobdir == 0 { // generate random ID as this functionality is not yet available on the C-api. let id = rand::thread_rng().gen(); - block_on(Context::new(as_path(dbfile), id, Events::new())) + block_on(Context::new( + as_path(dbfile), + id, + Events::new(), + StockStrings::new(), + )) } else { eprintln!("blobdir can not be defined explicitly anymore"); return ptr::null_mut(); @@ -122,7 +128,12 @@ pub unsafe extern "C" fn dc_context_new_closed(dbfile: *const libc::c_char) -> * } let id = rand::thread_rng().gen(); - match block_on(Context::new_closed(as_path(dbfile), id, Events::new())) { + match block_on(Context::new_closed( + as_path(dbfile), + id, + Events::new(), + StockStrings::new(), + )) { Ok(context) => Box::into_raw(Box::new(context)), Err(err) => { eprintln!("failed to create context: {:#}", err); diff --git a/examples/repl/main.rs b/examples/repl/main.rs index ffe1f20dd..46c2fb746 100644 --- a/examples/repl/main.rs +++ b/examples/repl/main.rs @@ -20,6 +20,7 @@ use deltachat::context::*; use deltachat::oauth2::*; use deltachat::qr_code_generator::get_securejoin_qr_svg; use deltachat::securejoin::*; +use deltachat::stock_str::StockStrings; use deltachat::{EventType, Events}; use log::{error, info, warn}; use rustyline::completion::{Completer, FilenameCompleter, Pair}; @@ -298,7 +299,7 @@ async fn start(args: Vec) -> Result<(), Error> { println!("Error: Bad arguments, expected [db-name]."); bail!("No db-name specified"); } - let context = Context::new(Path::new(&args[1]), 0, Events::new()).await?; + let context = Context::new(Path::new(&args[1]), 0, Events::new(), StockStrings::new()).await?; let events = context.get_event_emitter(); tokio::task::spawn(async move { diff --git a/examples/simple.rs b/examples/simple.rs index 2d9ef4b0a..d14b1ac48 100644 --- a/examples/simple.rs +++ b/examples/simple.rs @@ -6,6 +6,7 @@ use deltachat::config; use deltachat::contact::*; use deltachat::context::*; use deltachat::message::Message; +use deltachat::stock_str::StockStrings; use deltachat::{EventType, Events}; fn cb(event: EventType) { @@ -36,7 +37,7 @@ async fn main() { let dir = tempdir().unwrap(); let dbfile = dir.path().join("db.sqlite"); log::info!("creating database {:?}", dbfile); - let ctx = Context::new(&dbfile, 0, Events::new()) + let ctx = Context::new(&dbfile, 0, Events::new(), StockStrings::new()) .await .expect("Failed to create context"); let info = ctx.get_info().await; diff --git a/src/accounts.rs b/src/accounts.rs index a256c8c6d..75c7e70bf 100644 --- a/src/accounts.rs +++ b/src/accounts.rs @@ -10,6 +10,7 @@ use uuid::Uuid; use crate::context::Context; use crate::events::{Event, EventEmitter, EventType, Events}; +use crate::stock_str::StockStrings; /// Account manager, that can handle multiple accounts in a single place. #[derive(Debug)] @@ -20,6 +21,12 @@ pub struct Accounts { /// Event channel to emit account manager errors. events: Events, + + /// Stock string translations shared by all created contexts. + /// + /// This way changing a translation for one context automatically + /// changes it for all other contexts. + stockstrings: StockStrings, } impl Accounts { @@ -55,8 +62,9 @@ impl Accounts { .await .context("failed to load accounts config")?; let events = Events::new(); + let stockstrings = StockStrings::new(); let accounts = config - .load_accounts(&events) + .load_accounts(&events, &stockstrings) .await .context("failed to load accounts")?; @@ -65,6 +73,7 @@ impl Accounts { config, accounts, events, + stockstrings, }) } @@ -104,6 +113,7 @@ impl Accounts { &account_config.dbfile(), account_config.id, self.events.clone(), + self.stockstrings.clone(), ) .await?; self.accounts.insert(account_config.id, ctx); @@ -119,6 +129,7 @@ impl Accounts { &account_config.dbfile(), account_config.id, self.events.clone(), + self.stockstrings.clone(), ) .await?; self.accounts.insert(account_config.id, ctx); @@ -204,7 +215,13 @@ impl Accounts { match res { Ok(_) => { - let ctx = Context::new(&new_dbfile, account_config.id, self.events.clone()).await?; + let ctx = Context::new( + &new_dbfile, + account_config.id, + self.events.clone(), + self.stockstrings.clone(), + ) + .await?; self.accounts.insert(account_config.id, ctx); Ok(account_config.id) } @@ -339,17 +356,31 @@ impl Config { Ok(Config { file, inner }) } - pub async fn load_accounts(&self, events: &Events) -> Result> { + /// Loads all accounts defined in the configuration file. + /// + /// Created contexts share the same event channel and stock string + /// translations. + pub async fn load_accounts( + &self, + events: &Events, + stockstrings: &StockStrings, + ) -> Result> { let mut accounts = BTreeMap::new(); + for account_config in &self.inner.accounts { - let ctx = Context::new(&account_config.dbfile(), account_config.id, events.clone()) - .await - .with_context(|| { - format!( - "failed to create context from file {:?}", - account_config.dbfile() - ) - })?; + let ctx = Context::new( + &account_config.dbfile(), + account_config.id, + events.clone(), + stockstrings.clone(), + ) + .await + .with_context(|| { + format!( + "failed to create context from file {:?}", + account_config.dbfile() + ) + })?; accounts.insert(account_config.id, ctx); } @@ -446,6 +477,8 @@ impl AccountConfig { mod tests { use super::*; + use crate::stock_str::{self, StockMessage}; + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_account_new_open() { let dir = tempfile::tempdir().unwrap(); @@ -522,7 +555,7 @@ mod tests { assert_eq!(accounts.config.get_selected_account(), 0); let extern_dbfile: PathBuf = dir.path().join("other"); - let ctx = Context::new(&extern_dbfile, 0, Events::new()) + let ctx = Context::new(&extern_dbfile, 0, Events::new(), StockStrings::new()) .await .unwrap(); ctx.set_config(crate::config::Config::Addr, Some("me@mail.com")) @@ -717,4 +750,28 @@ mod tests { Ok(()) } + + /// Tests that accounts share stock string translations. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_accounts_share_translations() -> Result<()> { + let dir = tempfile::tempdir().unwrap(); + let p: PathBuf = dir.path().join("accounts"); + + let mut accounts = Accounts::new(p.clone()).await?; + accounts.add_account().await?; + accounts.add_account().await?; + + let account1 = accounts.get_account(1).context("failed to get account 1")?; + let account2 = accounts.get_account(2).context("failed to get account 2")?; + + assert_eq!(stock_str::no_messages(&account1).await, "No messages."); + assert_eq!(stock_str::no_messages(&account2).await, "No messages."); + account1 + .set_stock_translation(StockMessage::NoMessages, "foobar".to_string()) + .await?; + assert_eq!(stock_str::no_messages(&account1).await, "foobar"); + assert_eq!(stock_str::no_messages(&account2).await, "foobar"); + + Ok(()) + } } diff --git a/src/context.rs b/src/context.rs index ff257231b..520afaea7 100644 --- a/src/context.rs +++ b/src/context.rs @@ -23,6 +23,7 @@ use crate::quota::QuotaInfo; use crate::ratelimit::Ratelimit; use crate::scheduler::Scheduler; use crate::sql::Sql; +use crate::stock_str::StockStrings; use crate::tools::{duration_to_str, time}; #[derive(Clone, Debug)] @@ -51,7 +52,7 @@ pub struct InnerContext { pub(crate) oauth2_mutex: Mutex<()>, /// Mutex to prevent a race condition when a "your pw is wrong" warning is sent, resulting in multiple messeges being sent. pub(crate) wrong_pw_warning_mutex: Mutex<()>, - pub(crate) translated_stockstrings: RwLock>, + pub(crate) translated_stockstrings: StockStrings, pub(crate) events: Events, pub(crate) scheduler: RwLock>, @@ -119,8 +120,13 @@ pub fn get_info() -> BTreeMap<&'static str, String> { impl Context { /// Creates new context and opens the database. - pub async fn new(dbfile: &Path, id: u32, events: Events) -> Result { - let context = Self::new_closed(dbfile, id, events).await?; + pub async fn new( + dbfile: &Path, + id: u32, + events: Events, + stock_strings: StockStrings, + ) -> Result { + let context = Self::new_closed(dbfile, id, events, stock_strings).await?; // Open the database if is not encrypted. if context.check_passphrase("".to_string()).await? { @@ -130,7 +136,12 @@ impl Context { } /// Creates new context without opening the database. - pub async fn new_closed(dbfile: &Path, id: u32, events: Events) -> Result { + pub async fn new_closed( + dbfile: &Path, + id: u32, + events: Events, + stockstrings: StockStrings, + ) -> Result { let mut blob_fname = OsString::new(); blob_fname.push(dbfile.file_name().unwrap_or_default()); blob_fname.push("-blobs"); @@ -138,7 +149,7 @@ impl Context { if !blobdir.exists() { tokio::fs::create_dir_all(&blobdir).await?; } - let context = Context::with_blobdir(dbfile.into(), blobdir, id, events)?; + let context = Context::with_blobdir(dbfile.into(), blobdir, id, events, stockstrings)?; Ok(context) } @@ -174,6 +185,7 @@ impl Context { blobdir: PathBuf, id: u32, events: Events, + stockstrings: StockStrings, ) -> Result { ensure!( blobdir.is_dir(), @@ -190,7 +202,7 @@ impl Context { generating_key_mutex: Mutex::new(()), oauth2_mutex: Mutex::new(()), wrong_pw_warning_mutex: Mutex::new(()), - translated_stockstrings: RwLock::new(HashMap::new()), + translated_stockstrings: stockstrings, events, scheduler: RwLock::new(None), ratelimit: RwLock::new(Ratelimit::new(Duration::new(60, 0), 6.0)), // Allow to send 6 messages immediately, no more than once every 10 seconds. @@ -706,7 +718,7 @@ mod tests { let tmp = tempfile::tempdir()?; let dbfile = tmp.path().join("db.sqlite"); tokio::fs::write(&dbfile, b"123").await?; - let res = Context::new(&dbfile, 1, Events::new()).await?; + let res = Context::new(&dbfile, 1, Events::new(), StockStrings::new()).await?; // Broken database is indistinguishable from encrypted one. assert_eq!(res.is_open().await, false); @@ -852,7 +864,9 @@ mod tests { async fn test_blobdir_exists() { let tmp = tempfile::tempdir().unwrap(); let dbfile = tmp.path().join("db.sqlite"); - Context::new(&dbfile, 1, Events::new()).await.unwrap(); + Context::new(&dbfile, 1, Events::new(), StockStrings::new()) + .await + .unwrap(); let blobdir = tmp.path().join("db.sqlite-blobs"); assert!(blobdir.is_dir()); } @@ -863,7 +877,7 @@ mod tests { let dbfile = tmp.path().join("db.sqlite"); let blobdir = tmp.path().join("db.sqlite-blobs"); tokio::fs::write(&blobdir, b"123").await.unwrap(); - let res = Context::new(&dbfile, 1, Events::new()).await; + let res = Context::new(&dbfile, 1, Events::new(), StockStrings::new()).await; assert!(res.is_err()); } @@ -873,7 +887,9 @@ mod tests { let subdir = tmp.path().join("subdir"); let dbfile = subdir.join("db.sqlite"); let dbfile2 = dbfile.clone(); - Context::new(&dbfile, 1, Events::new()).await.unwrap(); + Context::new(&dbfile, 1, Events::new(), StockStrings::new()) + .await + .unwrap(); assert!(subdir.is_dir()); assert!(dbfile2.is_file()); } @@ -883,7 +899,7 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); let dbfile = tmp.path().join("db.sqlite"); let blobdir = PathBuf::new(); - let res = Context::with_blobdir(dbfile, blobdir, 1, Events::new()); + let res = Context::with_blobdir(dbfile, blobdir, 1, Events::new(), StockStrings::new()); assert!(res.is_err()); } @@ -892,7 +908,7 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); let dbfile = tmp.path().join("db.sqlite"); let blobdir = tmp.path().join("blobs"); - let res = Context::with_blobdir(dbfile, blobdir, 1, Events::new()); + let res = Context::with_blobdir(dbfile, blobdir, 1, Events::new(), StockStrings::new()); assert!(res.is_err()); } @@ -1061,7 +1077,7 @@ mod tests { let dbfile = dir.path().join("db.sqlite"); let id = 1; - let context = Context::new_closed(&dbfile, id, Events::new()) + let context = Context::new_closed(&dbfile, id, Events::new(), StockStrings::new()) .await .context("failed to create context")?; assert_eq!(context.open("foo".to_string()).await?, true); @@ -1069,7 +1085,7 @@ mod tests { drop(context); let id = 2; - let context = Context::new(&dbfile, id, Events::new()) + let context = Context::new(&dbfile, id, Events::new(), StockStrings::new()) .await .context("failed to create context")?; assert_eq!(context.is_open().await, false); diff --git a/src/stock_str.rs b/src/stock_str.rs index 2b9dc8343..6ae8a4e19 100644 --- a/src/stock_str.rs +++ b/src/stock_str.rs @@ -1,8 +1,12 @@ //! Module to work with translatable stock strings. -use anyhow::{bail, Error}; +use std::collections::HashMap; +use std::sync::Arc; + +use anyhow::{bail, Error, Result}; use strum::EnumProperty as EnumPropertyTrait; use strum_macros::EnumProperty; +use tokio::sync::RwLock; use crate::blob::BlobObject; use crate::chat::{self, Chat, ChatId, ProtectionStatus}; @@ -14,6 +18,12 @@ use crate::param::Param; use crate::tools::timestamp_to_str; use humansize::{file_size_opts, FileSize}; +#[derive(Debug, Clone)] +pub struct StockStrings { + /// Map from stock string ID to the translation. + translated_stockstrings: Arc>>, +} + /// Stock strings /// /// These identify the string to return in [Context.stock_str]. The @@ -402,15 +412,54 @@ impl StockMessage { } } +impl Default for StockStrings { + fn default() -> Self { + StockStrings::new() + } +} + +impl StockStrings { + pub fn new() -> Self { + Self { + translated_stockstrings: Arc::new(RwLock::new(Default::default())), + } + } + + async fn translated(&self, id: StockMessage) -> String { + self.translated_stockstrings + .read() + .await + .get(&(id as usize)) + .map(AsRef::as_ref) + .unwrap_or_else(|| id.fallback()) + .to_string() + } + + async fn set_stock_translation(&self, id: StockMessage, stockstring: String) -> Result<()> { + if stockstring.contains("%1") && !id.fallback().contains("%1") { + bail!( + "translation {} contains invalid %1 placeholder, default is {}", + stockstring, + id.fallback() + ); + } + if stockstring.contains("%2") && !id.fallback().contains("%2") { + bail!( + "translation {} contains invalid %2 placeholder, default is {}", + stockstring, + id.fallback() + ); + } + self.translated_stockstrings + .write() + .await + .insert(id as usize, stockstring); + Ok(()) + } +} + async fn translated(context: &Context, id: StockMessage) -> String { - context - .translated_stockstrings - .read() - .await - .get(&(id as usize)) - .map(AsRef::as_ref) - .unwrap_or_else(|| id.fallback()) - .to_string() + context.translated_stockstrings.translated(id).await } /// Helper trait only meant to be implemented for [`String`]. @@ -1205,29 +1254,10 @@ pub(crate) async fn aeap_explanation_and_link( impl Context { /// Set the stock string for the [StockMessage]. /// - pub async fn set_stock_translation( - &self, - id: StockMessage, - stockstring: String, - ) -> Result<(), Error> { - if stockstring.contains("%1") && !id.fallback().contains("%1") { - bail!( - "translation {} contains invalid %1 placeholder, default is {}", - stockstring, - id.fallback() - ); - } - if stockstring.contains("%2") && !id.fallback().contains("%2") { - bail!( - "translation {} contains invalid %2 placeholder, default is {}", - stockstring, - id.fallback() - ); - } + pub async fn set_stock_translation(&self, id: StockMessage, stockstring: String) -> Result<()> { self.translated_stockstrings - .write() - .await - .insert(id as usize, stockstring); + .set_stock_translation(id, stockstring) + .await?; Ok(()) } diff --git a/src/test_utils.rs b/src/test_utils.rs index cca04b8e6..6ab5ea19f 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -29,6 +29,7 @@ use crate::key::{self, DcKey, KeyPair, KeyPairUse}; use crate::message::{update_msg_state, Message, MessageState, MsgId, Viewtype}; use crate::mimeparser::MimeMessage; use crate::receive_imf::receive_imf; +use crate::stock_str::StockStrings; use crate::tools::EmailAddress; #[allow(non_upper_case_globals)] @@ -277,7 +278,7 @@ impl TestContext { let mut context_names = CONTEXT_NAMES.write().unwrap(); context_names.insert(id, name); } - let ctx = Context::new(&dbfile, id, Events::new()) + let ctx = Context::new(&dbfile, id, Events::new(), StockStrings::new()) .await .expect("failed to create context");