It's possible to have several drafts per one chat

[WofWca]

• Operating System (Linux/Mac/Windows/iOS/Android): Linux

• Delta Chat Version: Desktop 3bd0c73bcbff864151bf780d4987c2d9330178ca

• Expected behavior:
  await exp.rpc.getDraft() should evaluate to null after await exp.rpc.removeDraft().

• Actual behavior:
  A draft is still present after await exp.rpc.removeDraft()

• Steps to reproduce the problem:

  1. Start Delta Chat desktop in dev mode with pnpm -w dev:electron.

  2. Create a chat, and learn its ID.

  3. Open the console and paste the following:

     draftNumber = 0
     chatId = 13
     await Promise.all([
         exp.rpc.miscSetDraft(window.__selectedAccountId, chatId, `test ${draftNumber++}`, null, null, null),
         exp.rpc.miscSetDraft(window.__selectedAccountId, chatId, `test ${draftNumber++}`, null, null, null),
         exp.rpc.miscSetDraft(window.__selectedAccountId, chatId, `test ${draftNumber++}`, null, null, null),
         exp.rpc.miscSetDraft(window.__selectedAccountId, chatId, `test ${draftNumber++}`, null, null, null),
         exp.rpc.miscSetDraft(window.__selectedAccountId, chatId, `test ${draftNumber++}`, null, null, null),
         exp.rpc.miscSetDraft(window.__selectedAccountId, chatId, `test ${draftNumber++}`, null, null, null),
     ]);
     await exp.rpc.removeDraft(window.__selectedAccountId, chatId)
     draft = await exp.rpc.getDraft(window.__selectedAccountId, chatId)

     You can execute

     await exp.rpc.removeDraft(window.__selectedAccountId, chatId)
     draft = await exp.rpc.getDraft(window.__selectedAccountId, chatId)

     a few more times and you'll that draft is not null until you clear all the drafts.

• Screenshots:

  https://github.com/user-attachments/assets/02ef2c06-ed75-4152-9cc2-a24cdd94c602

• Logs:

Judging from API (miscSetDraft, removeDraft), it looks like we're only supposed to have one draft.

This could be what is causing deltachat/deltachat-desktop#3586 and deltachat/deltachat-desktop#3965 (comment)

Where to start looking:

deltachat-core-rust/src/chat.rs

Lines 921 to 948 in 8a88479

	// if possible, replace existing draft and keep id
	if !msg.id.is_special() {
	if let Some(old_draft) = self.get_draft(context).await? {
	if old_draft.id == msg.id
	&& old_draft.chat_id == self
	&& old_draft.state == MessageState::OutDraft
	{
	context
	.sql
	.execute(
	"UPDATE msgs
	SET timestamp=?,type=?,txt=?,txt_normalized=?,param=?,mime_in_reply_to=?
	WHERE id=?;",
	(
	time(),
	msg.viewtype,
	&msg.text,
	message::normalize_text(&msg.text),
	msg.param.to_string(),
	msg.in_reply_to.as_deref().unwrap_or_default(),
	msg.id,
	),
	)
	.await?;
	return Ok(true);
	}
	}
	}

I tried to write a failing test, but it's actually passing:

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_only_one_draft_per_chat() -> Result<()> {
    let t = TestContext::new_alice().await;
    let chat_id = create_group_chat(&t, ProtectionStatus::Unprotected, "abc").await?;

    let mut msgs: Vec<message::Message> = (1..=1000).map(|i| {
        let mut msg = Message::new(Viewtype::Text);
        msg.set_text(i.to_string());
        return msg;
    }).collect();
    futures::future::join_all(
        msgs.iter_mut().map(|msg| chat_id.set_draft(&t, Some(msg)))
    ).await;

    assert_eq!("1000", chat_id.get_draft(&t).await?.unwrap().text);

    chat_id.set_draft(&t, None).await?;
    assert!(chat_id.get_draft(&t).await?.is_none());

    Ok(())
}

[link2xt]

Maybe increasing number of worker threads will help reproduce this, currently worker_threads = 2.
And instead of joining all futures, you need to spawn them so they are executed on another thread, then join tasks.

[link2xt]

But even from the code, checking for existing draft and setting a new one happens in different SQL transactions, that's a bug.

[link2xt]

Made a test that fails:

    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
    async fn test_only_one_draft_per_chat() -> Result<()> {
        let t = TestContext::new_alice().await;
        let chat_id = create_group_chat(&t, ProtectionStatus::Unprotected, "abc").await?;

        let mut msgs: Vec<message::Message> = (1..=1000).map(|i| {
            let mut msg = Message::new(Viewtype::Text);
            msg.set_text(i.to_string());
            return msg;
        }).collect();
        let mut tasks = Vec::new();
        for mut msg in msgs {
            let ctx = t.clone();
            let task = tokio::spawn(async move {
                let ctx = ctx;
                chat_id.set_draft(&ctx, Some(&mut msg)).await
            });
            tasks.push(task);
        }
        futures::future::join_all(tasks.into_iter()).await;

        assert!(chat_id.get_draft(&t).await?.is_some());

        chat_id.set_draft(&t, None).await?;
        assert!(chat_id.get_draft(&t).await?.is_none());

        Ok(())
    }

[link2xt]

Made a fix #6052