python: use pyright

.github/workflows/ci.yml

@@ -10,26 +10,20 @@ env:
   RUSTFLAGS: -Dwarnings
    
 jobs:
-
-  fmt:
-    name: Rustfmt
+  lint:
+    name: Rustfmt and Clippy
     runs-on: ubuntu-latest
+    env:
+      RUSTUP_TOOLCHAIN: 1.67.1
     steps:
       - uses: actions/checkout@v3
-      - run: cargo fmt --all -- --check
-
-  run_clippy:
-    name: Clippy
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v3
-      - name: Install clippy
-        run: rustup toolchain install 1.67.1 --component clippy
+      - name: Install rustfmt and clippy
+        run: rustup toolchain install $RUSTUP_TOOLCHAIN --component rustfmt --component clippy
       - name: Cache rust cargo artifacts
         uses: swatinem/rust-cache@v2
+      - name: Run rustfmt
+        run: cargo fmt --all -- --check
       - name: Run clippy
-        env:
-          RUSTUP_TOOLCHAIN: 1.67.1
         run: scripts/clippy.sh
 
   docs:

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## Unreleased
+
+### Changes
+- use transaction in `Contact::add_or_lookup()` #4059
+
+### Fixes
+
+### API-Changes
+
+
 ## 1.109.0
 
 ### Changes

deltachat-ffi/src/lib.rs

@@ -2003,12 +2003,10 @@ pub unsafe extern "C" fn dc_create_contact(
     let ctx = &*context;
     let name = to_string_lossy(name);
 
-    block_on(async move {
-        Contact::create(ctx, &name, &to_string_lossy(addr))
-            .await
-            .map(|id| id.to_u32())
-            .unwrap_or(0)
-    })
+    block_on(Contact::create(ctx, &name, &to_string_lossy(addr)))
+        .log_err(ctx, "Cannot create contact")
+        .map(|id| id.to_u32())
+        .unwrap_or(0)
 }
 
 #[no_mangle]

python/pyproject.toml

@@ -55,3 +55,5 @@ line-length = 120
 
 [tool.isort]
 profile = "black"
+
+[tool.pyright]

python/src/deltachat/account.py

@@ -284,9 +284,9 @@ class Account:
         :returns: :class:`deltachat.contact.Contact` instance.
         """
         (name, addr) = self.get_contact_addr_and_name(obj, name)
-        name = as_dc_charpointer(name)
-        addr = as_dc_charpointer(addr)
-        contact_id = lib.dc_create_contact(self._dc_context, name, addr)
+        name_c = as_dc_charpointer(name)
+        addr_c = as_dc_charpointer(addr)
+        contact_id = lib.dc_create_contact(self._dc_context, name_c, addr_c)
         return Contact(self, contact_id)
 
     def get_contact(self, obj) -> Optional[Contact]:
@@ -363,12 +363,12 @@ class Account:
         :returns: list of :class:`deltachat.contact.Contact` objects.
         """
         flags = 0
-        query = as_dc_charpointer(query)
+        query_c = as_dc_charpointer(query)
         if only_verified:
             flags |= const.DC_GCL_VERIFIED_ONLY
         if with_self:
             flags |= const.DC_GCL_ADD_SELF
-        dc_array = ffi.gc(lib.dc_get_contacts(self._dc_context, flags, query), lib.dc_array_unref)
+        dc_array = ffi.gc(lib.dc_get_contacts(self._dc_context, flags, query_c), lib.dc_array_unref)
         return list(iter_array(dc_array, lambda x: Contact(self, x)))
 
     def get_fresh_messages(self) -> Generator[Message, None, None]:

python/src/deltachat/chat.py

@@ -162,8 +162,8 @@ class Chat:
         :param name: as a unicode string.
         :returns: True on success, False otherwise
         """
-        name = as_dc_charpointer(name)
-        return bool(lib.dc_set_chat_name(self.account._dc_context, self.id, name))
+        name_c = as_dc_charpointer(name)
+        return bool(lib.dc_set_chat_name(self.account._dc_context, self.id, name_c))
 
     def get_color(self):
         """return the color of the chat.
@@ -463,9 +463,10 @@ class Chat:
         :returns: None
         """
         contact = self.account.get_contact(obj)
-        ret = lib.dc_remove_contact_from_chat(self.account._dc_context, self.id, contact.id)
-        if ret != 1:
-            raise ValueError(f"could not remove contact {contact!r} from chat")
+        if contact:
+            ret = lib.dc_remove_contact_from_chat(self.account._dc_context, self.id, contact.id)
+            if ret != 1:
+                raise ValueError("could not remove contact {contact!r} from chat")
 
     def get_contacts(self):
         """get all contacts for this chat.

python/src/deltachat/direct_imap.py

@@ -147,7 +147,7 @@ class DirectImap:
                     log("Message", msg.uid, "has empty body")
                     continue
 
-                path = pathlib.Path(str(dir)).joinpath("IMAP", self.logid, imapfolder)
+                path = pathlib.Path(str(dir)).joinpath("IMAP", str(self.logid), imapfolder)
                 path.mkdir(parents=True, exist_ok=True)
                 fn = path.joinpath(str(msg.uid))
                 fn.write_bytes(body)

python/src/deltachat/events.py

@@ -111,11 +111,12 @@ class FFIEventTracker:
         while True:
             yield self.get(timeout=timeout, check_error=check_error)
 
-    def get_matching(self, event_name_regex, check_error=True, timeout=None):
+    def get_matching(self, event_name_regex, check_error=True, timeout=None) -> FFIEvent:
         rex = re.compile(f"^(?:{event_name_regex})$")
         for ev in self.iter_events(timeout=timeout, check_error=check_error):
             if rex.match(ev.name):
                 return ev
+        assert False
 
     def get_info_contains(self, regex: str) -> FFIEvent:
         rex = re.compile(regex)
@@ -123,6 +124,7 @@ class FFIEventTracker:
             ev = self.get_matching("DC_EVENT_INFO")
             if rex.search(ev.data2):
                 return ev
+        assert False
 
     def get_info_regex_groups(self, regex, check_error=True):
         rex = re.compile(regex)

python/src/deltachat/testplugin.py

@@ -250,7 +250,7 @@ def data(request):
                 fn = os.path.join(path, *bn.split("/"))
                 if os.path.exists(fn):
                     return fn
-            print(f"WARNING: path does not exist: {fn!r}")
+            print(f"WARNING: path does not exist: {request!r}")
             return None
 
         def read_path(self, bn, mode="r"):

python/tests/test_2_increation.py

@@ -1,6 +1,7 @@
 import os.path
 import shutil
 from filecmp import cmp
+from typing import Optional
 
 import pytest
 
@@ -13,7 +14,7 @@ def wait_msg_delivered(account, msg_list):
         msg_list.remove((ev.data1, ev.data2))
 
 
-def wait_msgs_changed(account, msgs_list):
+def wait_msgs_changed(account, msgs_list: list[tuple[int, Optional[int]]]):
     """wait for one or more MSGS_CHANGED events to match msgs_list contents."""
     account.log(f"waiting for msgs_list={msgs_list}")
     msgs_list = list(msgs_list)

python/tox.ini

@@ -5,10 +5,11 @@ envlist =
     lint
     mypy
     auditwheels
+    pyright
 
 [testenv]
 commands = 
-    pytest -n6 --extra-info --reruns 2 --reruns-delay 5 -v -rsXx --ignored --strict-tls {posargs: tests examples}
+    pytest -n6 --extra-info -v -rsXx --ignored --strict-tls {posargs: tests examples}
     pip wheel . -w {toxworkdir}/wheelhouse --no-deps
 setenv =
 # Avoid stack overflow when Rust core is built without optimizations.
@@ -21,11 +22,11 @@ passenv =
     RUSTC_WRAPPER
 deps = 
     pytest
-    pytest-rerunfailures
     pytest-timeout
     pytest-xdist
     pdbpp
     requests
+    pyright: pyright
 
 [testenv:.pkg]
 passenv =
@@ -69,6 +70,10 @@ deps =
 commands =
     mypy --no-incremental src/
 
+[testenv:pyright]
+commands =
+    pyright
+
 [testenv:doc]
 changedir=doc
 deps =

src/contact.rs

@@ -13,6 +13,7 @@ use async_channel::{self as channel, Receiver, Sender};
 use deltachat_derive::{FromSql, ToSql};
 use once_cell::sync::Lazy;
 use regex::Regex;
+use rusqlite::OptionalExtension;
 use serde::{Deserialize, Serialize};
 use tokio::task;
 use tokio::time::{timeout, Duration};
@@ -520,8 +521,6 @@ impl Contact {
     /// Depending on the origin, both, "row_name" and "row_authname" are updated from "name".
     ///
     /// Returns the contact_id and a `Modifier` value indicating if a modification occurred.
-    ///
-    /// Returns None if the contact with such address cannot exist.
     pub(crate) async fn add_or_lookup(
         context: &Context,
         name: &str,
@@ -566,14 +565,12 @@ impl Contact {
         );
 
         let mut update_addr = false;
-        let mut row_id = 0;
 
-        if let Some((id, row_name, row_addr, row_origin, row_authname)) = context
-            .sql
-            .query_row_optional(
-                "SELECT id, name, addr, origin, authname \
-            FROM contacts WHERE addr=? COLLATE NOCASE;",
-                paramsv![addr.to_string()],
+        let row_id = context.sql.transaction(|transaction| {
+            let row = transaction.query_row(
+                "SELECT id, name, addr, origin, authname
+                 FROM contacts WHERE addr=? COLLATE NOCASE",
+                 [addr.to_string()],
                 |row| {
                     let row_id: isize = row.get(0)?;
                     let row_name: String = row.get(1)?;
@@ -582,120 +579,130 @@ impl Contact {
                     let row_authname: String = row.get(4)?;
 
                     Ok((row_id, row_name, row_addr, row_origin, row_authname))
-                },
-            )
-            .await?
-        {
-            let update_name = manual && name != row_name;
-            let update_authname = !manual
-                && name != row_authname
-                && !name.is_empty()
-                && (origin >= row_origin
-                    || origin == Origin::IncomingUnknownFrom
-                    || row_authname.is_empty());
+                }).optional()?;
 
-            row_id = u32::try_from(id)?;
-            if origin >= row_origin && addr.as_ref() != row_addr {
-                update_addr = true;
-            }
-            if update_name || update_authname || update_addr || origin > row_origin {
-                let new_name = if update_name {
-                    name.to_string()
-                } else {
-                    row_name
-                };
+            let row_id;
+            if let Some((id, row_name, row_addr, row_origin, row_authname)) = row {
+                let update_name = manual && name != row_name;
+                let update_authname = !manual
+                    && name != row_authname
+                    && !name.is_empty()
+                    && (origin >= row_origin
+                        || origin == Origin::IncomingUnknownFrom
+                        || row_authname.is_empty());
 
-                context
-                    .sql
-                    .execute(
-                        "UPDATE contacts SET name=?, addr=?, origin=?, authname=? WHERE id=?;",
-                        paramsv![
-                            new_name,
-                            if update_addr {
-                                addr.to_string()
-                            } else {
-                                row_addr
-                            },
-                            if origin > row_origin {
-                                origin
-                            } else {
-                                row_origin
-                            },
-                            if update_authname {
-                                name.to_string()
-                            } else {
-                                row_authname
-                            },
-                            row_id
-                        ],
-                    )
-                    .await
-                    .ok();
+                row_id = u32::try_from(id)?;
+                if origin >= row_origin && addr.as_ref() != row_addr {
+                    update_addr = true;
+                }
+                if update_name || update_authname || update_addr || origin > row_origin {
+                    let new_name = if update_name {
+                        name.to_string()
+                    } else {
+                        row_name
+                    };
 
-                if update_name || update_authname {
-                    // Update the contact name also if it is used as a group name.
-                    // This is one of the few duplicated data, however, getting the chat list is easier this way.
-                    let chat_id: Option<i32> = context.sql.query_get_value(
-                        "SELECT id FROM chats WHERE type=? AND id IN(SELECT chat_id FROM chats_contacts WHERE contact_id=?)",
-                        paramsv![Chattype::Single, isize::try_from(row_id)?]
-                    ).await?;
-                    if let Some(chat_id) = chat_id {
-                        let contact = Contact::get_by_id(context, ContactId::new(row_id)).await?;
-                        let chat_name = contact.get_display_name();
-                        match context
-                            .sql
-                            .execute(
-                                "UPDATE chats SET name=?1 WHERE id=?2 AND name!=?3",
-                                paramsv![chat_name, chat_id, chat_name],
-                            )
-                            .await
-                        {
-                            Err(err) => warn!(context, "Can't update chat name: {}", err),
-                            Ok(count) => {
-                                if count > 0 {
-                                    // Chat name updated
-                                    context.emit_event(EventType::ChatModified(ChatId::new(
-                                        chat_id.try_into()?,
-                                    )));
-                                }
+                    transaction
+                        .execute(
+                            "UPDATE contacts SET name=?, addr=?, origin=?, authname=? WHERE id=?;",
+                            paramsv![
+                                new_name,
+                                if update_addr {
+                                    addr.to_string()
+                                } else {
+                                    row_addr
+                                },
+                                if origin > row_origin {
+                                    origin
+                                } else {
+                                    row_origin
+                                },
+                                if update_authname {
+                                    name.to_string()
+                                } else {
+                                    row_authname
+                                },
+                                row_id
+                            ],
+                        )?;
+
+                    if update_name || update_authname {
+                        // Update the contact name also if it is used as a group name.
+                        // This is one of the few duplicated data, however, getting the chat list is easier this way.
+                        let chat_id: Option<ChatId> = transaction.query_row(
+                            "SELECT id FROM chats WHERE type=? AND id IN(SELECT chat_id FROM chats_contacts WHERE contact_id=?)",
+                            params![Chattype::Single, isize::try_from(row_id)?],
+                            |row| {
+                                let chat_id: ChatId = row.get(0)?;
+                                Ok(chat_id)
+                            }
+                        ).optional()?;
+
+                        if let Some(chat_id) = chat_id {
+                            let contact_id = ContactId::new(row_id);
+                            let (addr, name, authname) =
+                                transaction.query_row(
+                                    "SELECT addr, name, authname
+                                     FROM contacts
+                                     WHERE id=?",
+                                     params![contact_id],
+                                |row| {
+                                    let addr: String = row.get(0)?;
+                                    let name: String = row.get(1)?;
+                                    let authname: String = row.get(2)?;
+                                    Ok((addr, name, authname))
+                                })?;
+
+                            let chat_name = if !name.is_empty() {
+                                name
+                            } else if !authname.is_empty() {
+                                authname
+                            } else {
+                                addr
+                            };
+
+                            let count = transaction.execute(
+                                    "UPDATE chats SET name=?1 WHERE id=?2 AND name!=?1",
+                                    params![chat_name, chat_id])?;
+
+                            if count > 0 {
+                                // Chat name updated
+                                context.emit_event(EventType::ChatModified(chat_id));
                             }
                         }
                     }
+                    sth_modified = Modifier::Modified;
                 }
-                sth_modified = Modifier::Modified;
-            }
-        } else {
-            let update_name = manual;
-            let update_authname = !manual;
-
-            if let Ok(new_row_id) = context
-                .sql
-                .insert(
-                    "INSERT INTO contacts (name, addr, origin, authname) VALUES(?, ?, ?, ?);",
-                    paramsv![
-                        if update_name {
-                            name.to_string()
-                        } else {
-                            "".to_string()
-                        },
-                        addr,
-                        origin,
-                        if update_authname {
-                            name.to_string()
-                        } else {
-                            "".to_string()
-                        }
-                    ],
-                )
-                .await
-            {
-                row_id = u32::try_from(new_row_id)?;
-                sth_modified = Modifier::Created;
-                info!(context, "added contact id={} addr={}", row_id, &addr);
             } else {
-                error!(context, "Cannot add contact.");
+                let update_name = manual;
+                let update_authname = !manual;
+
+                transaction
+                    .execute(
+                        "INSERT INTO contacts (name, addr, origin, authname)
+                         VALUES (?, ?, ?, ?);",
+                        params![
+                            if update_name {
+                                name.to_string()
+                            } else {
+                                "".to_string()
+                            },
+                            addr,
+                            origin,
+                            if update_authname {
+                                name.to_string()
+                            } else {
+                                "".to_string()
+                            }
+                        ],
+                    )?;
+
+                sth_modified = Modifier::Created;
+                row_id = u32::try_from(transaction.last_insert_rowid())?;
+                info!(context, "added contact id={} addr={}", row_id, &addr);
             }
-        }
+            Ok(row_id)
+        }).await?;
 
         Ok((ContactId::new(row_id), sth_modified))
     }