feat: improve internal sql interface

* refactor: move sql into own mod * refactor: resultify sql interface * fixup * start usage of sqlx * compile again * migrate exists * migrate query_row and query_row_optional * migrate table_exists * migrate with_conn * migrate rowid * migrate most of query_map * finish query_map migration * remove rusqlite and fixup examples * improve migrations and ensure closing * fix rust tests * wip * fixup rebase and get things to compile * start fixing tests * use sqlx fork * debugging * fix receive imf insert * fix message loading * fix grpid bug * exists() expects COUNT() in sql-statement * fix two more exists() statements * fix test_audit_log_view_without_daymarker * Fix clippy warnings * fix test_fetch_existing dc_get_config() must never return NULL, instead return "" * ffi: make render_info accept non-option BTreeMap * fix a typo * Restore changes from PR #2258 They have been accidentally reverted during sqlx rebasing * Remove useless cast that was not there before switch to sqlx * Split long query into several lines so rustfmt does not fail * Remove outdated comment DC_CONTACT_ID_SELF is bound into the query now. * Split long lines to make rustfmt work * Restore accidentally removed part of PR #2132 * Make rustfmt work * Use exists() instead of count() in is_group_explicitly_left * Make rustfmt work * Fix a typo ("foreing") * Fix rustfmt * Fix formatting * Replace 0x100 constant with Origin::IncomingReplyTo * Replace constant 100 with Chattype::Single * Remove commented out code * peerstate: make rustfmt work * Move the code to make msg immutable * do not re-use bind parameters sqlx does not bind the first `?` implicitly to `?1`, therefore the sql-statement fails. we could use `?1` several times, however, i did not find good documentation about that, and even if that works now, that could fail with other db or even with sqlite in newer versions. therefore, we just bind every parameter explicitly. * Do not use read only sqlite mode * Factor out database pool creation * Open reader pool in readonly mode if database is opened in readonly mode * Add a test for reopening the database * sql::table_exists: drain response stream to avoid breaking connection * Drain the stream in Sql.col_exists * sqlx: fix failing TestOnlineAccount::test_saved_mime_on_received_message (#2323) * add failing tests for save_mime_headers * fix saving mime if an encrypted message is received, and we do not have the decoded data, for whatever reason, save the raw data. * let dc_get_mime_headers return NULL on empty string * let test_save_mime_headers_*() tests return anyhow::Result<()> * make clippy happy * Remove 5 unwraps * Remove another unwrap * Replace {:?} with {:#} for error handling * Remove all dbg! statements * set max_connections=10 (default), show num_cpus in get_info() setting the number of connections to the number of cpus seems to be to low in general: - on a mac i5 from 2017, that would result in 4 connections - on an iphone7, that would result in only 2 connections the sqlx-default is 10 connections, so, this pr uses that default (plus 1 connection for the writer tag). on the iphone7, that change was directly noticable - sqlx with 2 connections feelt not much faster that rusqlite with 10 connections, however, using sqlx with the 10+1 connection was a real boost, at least at a first glance :) might be, fewer connections would be sufficient as well, however, i do not see much reasons not to go with the default here. * Disable sqlx statement cache This is a workaround for launchbadge/sqlx#1147 Draining statement results is no longer needed. * Make clippy happy * Do not pass NULL to mime_headers column nor expect it from there. Treat existing NULL as "empty string". * Increase SQL busy timeout to 100 seconds No need to abort the connection if all connections are stalled for 10 seconds. This did not happen, just to make sure each timeout is a result of an actual deadlock. * README: specify log target to get rid of sqlx query logging * Remove 3 unwrap()'s * Remove two more unwrap()'s Co-authored-by: B. Petersen <r10s@b44t.com> Co-authored-by: Hocuri <hocuri@gmx.de> Co-authored-by: link2xt <link2xt@testrun.org>
deltachat · Apr 6, 2021 · 54db9a7 · 54db9a7
1 parent 4dedc2d
commit 54db9a7
Show file tree

Hide file tree

Showing 52 changed files with 5,503 additions and 4,981 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -12,8 +12,6 @@ debug = 0
 lto = true
 
 [dependencies]
-deltachat_derive = { path = "./deltachat_derive" }
-
 libc = "0.2.51"
 pgp = { version = "0.7.0", default-features = false } 
 hex = "0.4.0"
@@ -40,9 +38,6 @@ indexmap = "1.3.0"
 kamadak-exif = "0.5"
 once_cell = "1.4.1"
 regex = "1.1.6"
-rusqlite = { version = "0.24", features = ["bundled"] }
-r2d2_sqlite = "0.17.0"
-r2d2 = "0.8.5"
 strum = "0.19.0"
 strum_macros = "0.19.0"
 backtrace = "0.3.33"
@@ -66,13 +61,17 @@ async-std-resolver = "0.19.5"
 async-tar = "0.3.0"
 uuid = { version = "0.8", features = ["serde", "v4"] }
 rust-hsluv = "0.1.4"
+sqlx = { git = "https://github.com/dignifiedquire/sqlx", branch = "fix-pool-time-out", features = ["runtime-async-std-native-tls", "sqlite"] }
+# keep in sync with sqlx 
+libsqlite3-sys = { version = "0.20.1", default-features = false, features = [ "pkg-config", "vcpkg", "bundled" ] }
 
 pretty_env_logger = { version = "0.4.0", optional = true }
 log = {version = "0.4.8", optional = true }
 rustyline = { version = "4.1.0", optional = true }
 ansi_term = { version = "0.12.1", optional = true }
 dirs = { version = "3.0.1", optional=true }
 toml = "0.5.6"
+num_cpus = "1.13.0"
 
 
 [dev-dependencies]
@@ -84,11 +83,11 @@ async-std = { version = "1.6.4", features = ["unstable", "attributes"] }
 futures-lite = "1.7.0"
 criterion = "0.3"
 ansi_term = "0.12.0"
+log = "0.4.11"
 
 [workspace]
 members = [
   "deltachat-ffi",
-  "deltachat_derive",
 ]
 
 [[example]]

diff --git a/README.md b/README.md
@@ -17,7 +17,7 @@ $ curl https://sh.rustup.rs -sSf | sh
 Compile and run Delta Chat Core command line utility, using `cargo`:
 
 ```
-$ RUST_LOG=info cargo run --example repl --features repl -- ~/deltachat-db
+$ RUST_LOG=repl=info cargo run --example repl --features repl -- ~/deltachat-db
 ```
 where ~/deltachat-db is the database file. Delta Chat will create it if it does not exist.
 
@@ -95,7 +95,7 @@ $ cargo build -p deltachat_ffi --release
 
 - `DCC_MIME_DEBUG`: if set outgoing and incoming message will be printed 
 
-- `RUST_LOG=info,async_imap=trace,async_smtp=trace`: enable IMAP and
+- `RUST_LOG=repl=info,async_imap=trace,async_smtp=trace`: enable IMAP and
 SMTP tracing in addition to info messages.
 
 ### Expensive tests

diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs
@@ -156,7 +156,14 @@ pub unsafe extern "C" fn dc_get_config(
     }
     let ctx = &*context;
     match config::Config::from_str(&to_string_lossy(key)) {
-        Ok(key) => block_on(async move { ctx.get_config(key).await.unwrap_or_default().strdup() }),
+        Ok(key) => block_on(async move {
+            ctx.get_config(key)
+                .await
+                .log_err(ctx, "Can't get config")
+                .unwrap_or_default()
+                .unwrap_or_default()
+                .strdup()
+        }),
         Err(_) => {
             warn!(ctx, "dc_get_config(): invalid key");
             "".strdup()
@@ -225,8 +232,13 @@ pub unsafe extern "C" fn dc_get_info(context: *const dc_context_t) -> *mut libc:
     }
     let ctx = &*context;
     block_on(async move {
-        let info = ctx.get_info().await;
-        render_info(info).unwrap_or_default().strdup()
+        match ctx.get_info().await {
+            Ok(info) => render_info(info).unwrap_or_default().strdup(),
+            Err(err) => {
+                warn!(ctx, "failed to get info: {}", err);
+                "".strdup()
+            }
+        }
     })
 }
 
@@ -283,7 +295,12 @@ pub unsafe extern "C" fn dc_is_configured(context: *mut dc_context_t) -> libc::c
     }
     let ctx = &*context;
 
-    block_on(async move { ctx.is_configured().await as libc::c_int })
+    block_on(async move {
+        ctx.is_configured()
+            .await
+            .log_err(ctx, "failed to get configured state")
+            .unwrap_or_default() as libc::c_int
+    })
 }
 
 #[no_mangle]
@@ -768,7 +785,12 @@ pub unsafe extern "C" fn dc_set_draft(
         Some(&mut ffi_msg.message)
     };
 
-    block_on(ChatId::new(chat_id).set_draft(&ctx, msg))
+    block_on(async move {
+        ChatId::new(chat_id)
+            .set_draft(&ctx, msg)
+            .await
+            .unwrap_or_log_default(ctx, "failed to set draft");
+    });
 }
 
 #[no_mangle]
@@ -863,6 +885,7 @@ pub unsafe extern "C" fn dc_get_chat_msgs(
         Box::into_raw(Box::new(
             chat::get_chat_msgs(&ctx, ChatId::new(chat_id), flags, marker_flag)
                 .await
+                .unwrap_or_log_default(ctx, "failed to get chat msgs")
                 .into(),
         ))
     })
@@ -876,7 +899,12 @@ pub unsafe extern "C" fn dc_get_msg_cnt(context: *mut dc_context_t, chat_id: u32
     }
     let ctx = &*context;
 
-    block_on(async move { ChatId::new(chat_id).get_msg_cnt(&ctx).await as libc::c_int })
+    block_on(async move {
+        ChatId::new(chat_id)
+            .get_msg_cnt(&ctx)
+            .await
+            .unwrap_or_log_default(ctx, "failed to get msg count") as libc::c_int
+    })
 }
 
 #[no_mangle]
@@ -890,7 +918,12 @@ pub unsafe extern "C" fn dc_get_fresh_msg_cnt(
     }
     let ctx = &*context;
 
-    block_on(async move { ChatId::new(chat_id).get_fresh_msg_cnt(&ctx).await as libc::c_int })
+    block_on(async move {
+        ChatId::new(chat_id)
+            .get_fresh_msg_cnt(&ctx)
+            .await
+            .unwrap_or_log_default(ctx, "failed to get fresh msg cnt") as libc::c_int
+    })
 }
 
 #[no_mangle]
@@ -988,6 +1021,7 @@ pub unsafe extern "C" fn dc_get_chat_media(
                 or_msg_type3,
             )
             .await
+            .unwrap_or_log_default(ctx, "Failed get_chat_media")
             .into(),
         ))
     })
@@ -1029,7 +1063,7 @@ pub unsafe extern "C" fn dc_get_next_media(
             or_msg_type3,
         )
         .await
-        .map(|msg_id| msg_id.to_u32())
+        .map(|msg_id| msg_id.map(|id| id.to_u32()).unwrap_or_default())
         .unwrap_or(0)
     })
 }
@@ -1122,7 +1156,11 @@ pub unsafe extern "C" fn dc_get_chat_contacts(
     let ctx = &*context;
 
     block_on(async move {
-        let arr = dc_array_t::from(chat::get_chat_contacts(&ctx, ChatId::new(chat_id)).await);
+        let arr = dc_array_t::from(
+            chat::get_chat_contacts(&ctx, ChatId::new(chat_id))
+                .await
+                .unwrap_or_log_default(ctx, "Failed get_chat_contacts"),
+        );
         Box::into_raw(Box::new(arr))
     })
 }
@@ -1148,6 +1186,7 @@ pub unsafe extern "C" fn dc_search_msgs(
         let arr = dc_array_t::from(
             ctx.search_msgs(chat_id, to_string_lossy(query))
                 .await
+                .unwrap_or_log_default(ctx, "Failed search_msgs")
                 .iter()
                 .map(|msg_id| msg_id.to_u32())
                 .collect::<Vec<u32>>(),
@@ -1261,7 +1300,8 @@ pub unsafe extern "C" fn dc_set_chat_name(
     chat_id: u32,
     name: *const libc::c_char,
 ) -> libc::c_int {
-    if context.is_null() || chat_id <= constants::DC_CHAT_ID_LAST_SPECIAL as u32 || name.is_null() {
+    if context.is_null() || chat_id <= constants::DC_CHAT_ID_LAST_SPECIAL.to_u32() || name.is_null()
+    {
         eprintln!("ignoring careless call to dc_set_chat_name()");
         return 0;
     }
@@ -1281,7 +1321,7 @@ pub unsafe extern "C" fn dc_set_chat_profile_image(
     chat_id: u32,
     image: *const libc::c_char,
 ) -> libc::c_int {
-    if context.is_null() || chat_id <= constants::DC_CHAT_ID_LAST_SPECIAL as u32 {
+    if context.is_null() || chat_id <= constants::DC_CHAT_ID_LAST_SPECIAL.to_u32() {
         eprintln!("ignoring careless call to dc_set_chat_profile_image()");
         return 0;
     }
@@ -1406,7 +1446,12 @@ pub unsafe extern "C" fn dc_get_msg_info(
     }
     let ctx = &*context;
 
-    block_on(message::get_msg_info(&ctx, MsgId::new(msg_id))).strdup()
+    block_on(async move {
+        message::get_msg_info(&ctx, MsgId::new(msg_id))
+            .await
+            .unwrap_or_log_default(ctx, "failed to get msg id")
+            .strdup()
+    })
 }
 
 #[no_mangle]
@@ -1420,7 +1465,9 @@ pub unsafe extern "C" fn dc_get_msg_html(
     }
     let ctx = &*context;
 
-    block_on(MsgId::new(msg_id).get_html(&ctx)).strdup()
+    block_on(MsgId::new(msg_id).get_html(&ctx))
+        .unwrap_or_log_default(ctx, "Failed get_msg_html")
+        .strdup()
 }
 
 #[no_mangle]
@@ -1435,10 +1482,13 @@ pub unsafe extern "C" fn dc_get_mime_headers(
     let ctx = &*context;
 
     block_on(async move {
-        message::get_mime_headers(&ctx, MsgId::new(msg_id))
+        let mime = message::get_mime_headers(&ctx, MsgId::new(msg_id))
             .await
-            .map(|s| s.strdup())
-            .unwrap_or_else(ptr::null_mut)
+            .unwrap_or_log_default(ctx, "failed to get mime headers");
+        if mime.is_empty() {
+            return ptr::null_mut();
+        }
+        mime.strdup()
     })
 }
 
@@ -1468,7 +1518,7 @@ pub unsafe extern "C" fn dc_forward_msgs(
     if context.is_null()
         || msg_ids.is_null()
         || msg_cnt <= 0
-        || chat_id <= constants::DC_CHAT_ID_LAST_SPECIAL as u32
+        || chat_id <= constants::DC_CHAT_ID_LAST_SPECIAL.to_u32()
     {
         eprintln!("ignoring careless call to dc_forward_msgs()");
         return;
@@ -1564,14 +1614,12 @@ pub unsafe extern "C" fn dc_lookup_contact_id_by_addr(
     }
     let ctx = &*context;
 
-    block_on(Contact::lookup_id_by_addr(
-        &ctx,
-        to_string_lossy(addr),
-        Origin::IncomingReplyTo,
-    ))
-    .ok()
-    .flatten()
-    .unwrap_or_default()
+    block_on(async move {
+        Contact::lookup_id_by_addr(&ctx, to_string_lossy(addr), Origin::IncomingReplyTo)
+            .await
+            .unwrap_or_log_default(ctx, "failed to lookup id")
+            .unwrap_or(0)
+    })
 }
 
 #[no_mangle]
@@ -1645,8 +1693,7 @@ pub unsafe extern "C" fn dc_get_blocked_cnt(context: *mut dc_context_t) -> libc:
     block_on(async move {
         Contact::get_all_blocked(&ctx)
             .await
-            .log_err(&ctx, "Can't get blocked count")
-            .unwrap_or_default()
+            .unwrap_or_log_default(ctx, "failed to get blocked count")
             .len() as libc::c_int
     })
 }
@@ -1931,7 +1978,7 @@ pub unsafe extern "C" fn dc_send_locations_to_chat(
     chat_id: u32,
     seconds: libc::c_int,
 ) {
-    if context.is_null() || chat_id <= constants::DC_CHAT_ID_LAST_SPECIAL as u32 || seconds < 0 {
+    if context.is_null() || chat_id <= constants::DC_CHAT_ID_LAST_SPECIAL.to_u32() || seconds < 0 {
         eprintln!("ignoring careless call to dc_send_locations_to_chat()");
         return;
     }
@@ -2011,7 +2058,8 @@ pub unsafe extern "C" fn dc_get_locations(
             timestamp_begin as i64,
             timestamp_end as i64,
         )
-        .await;
+        .await
+        .unwrap_or_log_default(ctx, "Failed get_locations");
         Box::into_raw(Box::new(dc_array_t::from(res)))
     })
 }
@@ -2392,8 +2440,12 @@ pub unsafe extern "C" fn dc_chat_get_profile_image(chat: *mut dc_chat_t) -> *mut
 
     block_on(async move {
         match ffi_chat.chat.get_profile_image(&ctx).await {
-            Some(p) => p.to_string_lossy().strdup(),
-            None => ptr::null_mut(),
+            Ok(Some(p)) => p.to_string_lossy().strdup(),
+            Ok(None) => ptr::null_mut(),
+            Err(err) => {
+                error!(ctx, "failed to get profile image: {:?}", err);
+                ptr::null_mut()
+            }
         }
     })
 }
@@ -2407,7 +2459,7 @@ pub unsafe extern "C" fn dc_chat_get_color(chat: *mut dc_chat_t) -> u32 {
     let ffi_chat = &*chat;
     let ctx = &*ffi_chat.context;
 
-    block_on(ffi_chat.chat.get_color(&ctx))
+    block_on(ffi_chat.chat.get_color(&ctx)).unwrap_or_log_default(ctx, "Failed get_color")
 }
 
 #[no_mangle]
@@ -3318,6 +3370,7 @@ pub unsafe extern "C" fn dc_contact_get_profile_image(
             .contact
             .get_profile_image(&ctx)
             .await
+            .unwrap_or_log_default(ctx, "failed to get profile image")
             .map(|p| p.to_string_lossy().strdup())
             .unwrap_or_else(std::ptr::null_mut)
     })

diff --git a/deltachat_derive/Cargo.toml b/deltachat_derive/Cargo.toml