From fed6f4ab8a564562af48a997ee08df57bcf27c20 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Mon, 18 Nov 2019 14:10:29 +0100 Subject: [PATCH 1/8] do not re-add device-messages deleted by the user --- deltachat-ffi/deltachat.h | 4 +- src/chat.rs | 77 +++++++++++++++++++++------------------ src/sql.rs | 23 ++++++------ 3 files changed, 55 insertions(+), 49 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 91001b744..723bb6a43 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -1117,7 +1117,7 @@ uint32_t dc_add_device_msg (dc_context_t* context, dc_msg_t* m * Add a message only one time to the device-chat. * The device-message is defined by a name. * If a message with the same name was added before, - * the message is not added again. + * the message is not added again, even if the message was deleted in between. * Use dc_add_device_msg() to add device-messages unconditionally. * * Sends the event #DC_EVENT_MSGS_CHANGED on success. @@ -1130,7 +1130,7 @@ uint32_t dc_add_device_msg (dc_context_t* context, dc_msg_t* m * @param msg Message to be added to the device-chat. * The message appears to the user as an incoming message. * @return The ID of the added message, - * this might be the id of an older message with the same name. + * if the message was already added before or on errors, 0 is returned. */ uint32_t dc_add_device_msg_once (dc_context_t* context, const char* label, dc_msg_t* msg); diff --git a/src/chat.rs b/src/chat.rs index 98ef7755d..e20611199 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -1971,32 +1971,27 @@ fn add_device_msg_maybe_labelled( ) -> Result { let (chat_id, _blocked) = create_or_lookup_by_contact_id(context, DC_CONTACT_ID_DEVICE, Blocked::Not)?; - let rfc724_mid = dc_create_outgoing_rfc724_mid(None, "@device"); - // chat_id has an sql-index so it makes sense to add this although redundant + // if the device message is labeled and was ever added, do nothing if let Some(label) = label { - if let Ok(msg_id) = context.sql.query_row( - "SELECT id FROM msgs WHERE chat_id=? AND label=?", - params![chat_id, label], - |row| { - let msg_id: MsgId = row.get(0)?; - Ok(msg_id) - }, + if let Ok(()) = context.sql.query_row( + "SELECT label FROM devmsglabels WHERE label=?", + params![label], + |_| Ok(()), ) { - info!( - context, - "device-message {} already exist as {}", label, msg_id - ); - return Ok(msg_id); + info!(context, "device-message {} already added", label); + return Ok(MsgId::new_unset()); } } + // insert the device message + let rfc724_mid = dc_create_outgoing_rfc724_mid(None, "@device"); prepare_msg_blob(context, msg)?; unarchive(context, chat_id)?; context.sql.execute( - "INSERT INTO msgs (chat_id,from_id,to_id, timestamp,type,state, txt,param,rfc724_mid,label) \ - VALUES (?,?,?, ?,?,?, ?,?,?,?);", + "INSERT INTO msgs (chat_id,from_id,to_id, timestamp,type,state, txt,param,rfc724_mid) \ + VALUES (?,?,?, ?,?,?, ?,?,?);", params![ chat_id, DC_CONTACT_ID_DEVICE, @@ -2007,19 +2002,22 @@ fn add_device_msg_maybe_labelled( msg.text.as_ref().map_or("", String::as_str), msg.param.to_string(), rfc724_mid, - label.unwrap_or_default(), ], )?; let row_id = sql::get_rowid(context, &context.sql, "msgs", "rfc724_mid", &rfc724_mid); let msg_id = MsgId::new(row_id); - context.call_cb(Event::IncomingMsg { chat_id, msg_id }); - info!( - context, - "device-message {} added as {}", - label.unwrap_or("without label"), - msg_id - ); + + if let Some(label) = label { + context.sql.execute( + "INSERT INTO devmsglabels (label, msg_id) VALUES (?, ?);", + params![label, msg_id], + )?; + } + + if !msg_id.is_unset() { + context.call_cb(Event::IncomingMsg { chat_id, msg_id }); + } Ok(msg_id) } @@ -2175,26 +2173,27 @@ mod tests { msg1.text = Some("first message".to_string()); let msg1_id = add_device_msg_once(&t.ctx, "any-label", &mut msg1); assert!(msg1_id.is_ok()); + assert!(!msg1_id.as_ref().unwrap().is_unset()); let mut msg2 = Message::new(Viewtype::Text); msg2.text = Some("second message".to_string()); let msg2_id = add_device_msg_once(&t.ctx, "any-label", &mut msg2); assert!(msg2_id.is_ok()); - assert_eq!(msg1_id.as_ref().unwrap(), msg2_id.as_ref().unwrap()); + assert!(msg2_id.as_ref().unwrap().is_unset()); // check added message - let msg2 = message::Message::load_from_db(&t.ctx, msg2_id.unwrap()); - assert!(msg2.is_ok()); - let msg2 = msg2.unwrap(); - assert_eq!(msg1_id.unwrap(), msg2.id); - assert_eq!(msg2.text.as_ref().unwrap(), "first message"); - assert_eq!(msg2.from_id, DC_CONTACT_ID_DEVICE); - assert_eq!(msg2.to_id, DC_CONTACT_ID_SELF); - assert!(!msg2.is_info()); - assert!(!msg2.is_setupmessage()); + let msg1 = message::Message::load_from_db(&t.ctx, *msg1_id.as_ref().unwrap()); + assert!(msg1.is_ok()); + let msg1 = msg1.unwrap(); + assert_eq!(msg1_id.as_ref().unwrap(), &msg1.id); + assert_eq!(msg1.text.as_ref().unwrap(), "first message"); + assert_eq!(msg1.from_id, DC_CONTACT_ID_DEVICE); + assert_eq!(msg1.to_id, DC_CONTACT_ID_SELF); + assert!(!msg1.is_info()); + assert!(!msg1.is_setupmessage()); // check device chat - let chat_id = msg2.chat_id; + let chat_id = msg1.chat_id; assert_eq!(get_msg_cnt(&t.ctx, chat_id), 1); assert!(chat_id > DC_CHAT_ID_LAST_SPECIAL); let chat = Chat::load_from_db(&t.ctx, chat_id); @@ -2205,6 +2204,14 @@ mod tests { assert!(!chat.can_send()); assert_eq!(chat.name, t.ctx.stock_str(StockMessage::DeviceMessages)); assert!(chat.get_profile_image(&t.ctx).is_some()); + + // delete device message, make sure it is not added again + message::delete_msgs(&t.ctx, &[*msg1_id.as_ref().unwrap()]); + let msg1 = message::Message::load_from_db(&t.ctx, *msg1_id.as_ref().unwrap()); + assert!(msg1.is_err() || msg1.unwrap().chat_id == DC_CHAT_ID_TRASH); + let msg3_id = add_device_msg_once(&t.ctx, "any-label", &mut msg2); + assert!(msg3_id.is_ok()); + assert!(msg2_id.as_ref().unwrap().is_unset()); } fn chatlist_len(ctx: &Context, listflags: usize) -> usize { diff --git a/src/sql.rs b/src/sql.rs index 396b939d3..78cc2f1e4 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -810,24 +810,23 @@ fn open( )?; sql.set_raw_config_int(context, "dbversion", 55)?; } - if dbversion < 57 { - info!(context, "[migration] v57"); - // label is a unique name and is currently used for device-messages only. - // in contrast to rfc724_mid and other fields, the label is generated on the device - // and allows reliable identifications this way. + if dbversion < 59 { + info!(context, "[migration] v59"); + // records in the devmsglabels are kept when the message is deleted. + // so, msg_id may or may not exist. sql.execute( - "ALTER TABLE msgs ADD COLUMN label TEXT DEFAULT '';", - params![], + "CREATE TABLE devmsglabels (id INTEGER PRIMARY KEY AUTOINCREMENT, label TEXT, msg_id INTEGER);", + NO_PARAMS, + )?; + sql.execute( + "CREATE INDEX devmsglabels_index1 ON devmsglabels (label);", + NO_PARAMS, )?; if exists_before_update && sql.get_raw_config_int(context, "bcc_self").is_none() { sql.set_raw_config_int(context, "bcc_self", 1)?; } - sql.set_raw_config_int(context, "dbversion", 57)?; - } - if dbversion < 58 { - info!(context, "[migration] v58"); update_icons = true; - sql.set_raw_config_int(context, "dbversion", 58)?; + sql.set_raw_config_int(context, "dbversion", 59)?; } // (2) updates that require high-level objects From 035da9e5d7dfee39c61633da5c627287e2443c09 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Mon, 18 Nov 2019 16:01:15 +0100 Subject: [PATCH 2/8] add dc_skip_device_msg() and belonging tests --- deltachat-ffi/deltachat.h | 38 +++++++++++++++++++++++++++++++++ deltachat-ffi/src/lib.rs | 18 ++++++++++++++++ src/chat.rs | 45 +++++++++++++++++++++++++++++++++++---- src/sql.rs | 2 +- 4 files changed, 98 insertions(+), 5 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 723bb6a43..8fdefbea5 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -1135,6 +1135,44 @@ uint32_t dc_add_device_msg (dc_context_t* context, dc_msg_t* m uint32_t dc_add_device_msg_once (dc_context_t* context, const char* label, dc_msg_t* msg); +/** + * Skip a device-message permanently. + * Subsequent calls to dc_add_device_msg_once() with the same label + * won't add the device-message then. + * This might be handy if you want to add + * eg. different messages for first-install and updates. + * + * @memberof dc_context_t + * @param context The context as created by dc_context_new(). + * @param label A unique name for the message to skip. + * The label is typically not displayed to the user and + * must be created from the characters `A-Z`, `a-z`, `0-9`, `_` or `-`. + * If a message with that label already exist, + * nothing happens. + * @return None. + * + * Example: + * ~~~ + * dc_msg_t* welcome_msg = dc_msg_new(DC_MSG_TEXT); + * dc_msg_set_text(welcome_msg, "great that you give this app a try!"); + * + * dc_msg_t* changelog_msg = dc_msg_new(DC_MSG_TEXT); + * dc_msg_set_text(changelog_msg, "we have added 3 new emojis :)"); + * + * if (dc_add_device_msg_once(context, "welcome", welcome_msg)) { + * // do not add the changelog on a new installations - + * // not now and not when this code is executed again + * dc_skip_device_msg(context, "update-123"); + * } else { + * // welcome message was not added now, this is an oder installation, + * // add a changelog + * dc_add_device_msg_once(context, "update-123", changelog_msg); + * } + * ~~~ + */ +void dc_skip_device_msg (dc_context_t* context, const char* label); + + /** * Get draft for a chat, if any. * See dc_set_draft() for more details about drafts. diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index b7895549d..b0f469f9c 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -849,6 +849,24 @@ pub unsafe extern "C" fn dc_add_device_msg_once( .unwrap_or(0) } +#[no_mangle] +pub unsafe extern "C" fn dc_skip_device_msg( + context: *mut dc_context_t, + label: *const libc::c_char, +) { + if context.is_null() || label.is_null() { + eprintln!("ignoring careless call to dc_skip_device_msg()"); + return; + } + let ffi_context = &mut *context; + ffi_context + .with_inner(|ctx| { + chat::skip_device_msg(ctx, &to_string_lossy(label)) + .unwrap_or_log_default(ctx, "Failed to skip device message") + }) + .unwrap_or(()) +} + #[no_mangle] pub unsafe extern "C" fn dc_get_draft(context: *mut dc_context_t, chat_id: u32) -> *mut dc_msg_t { if context.is_null() { diff --git a/src/chat.rs b/src/chat.rs index e20611199..289c31540 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -1974,6 +1974,7 @@ fn add_device_msg_maybe_labelled( // if the device message is labeled and was ever added, do nothing if let Some(label) = label { + ensure!(!label.is_empty(), "cannot add empty label"); if let Ok(()) = context.sql.query_row( "SELECT label FROM devmsglabels WHERE label=?", params![label], @@ -2009,10 +2010,7 @@ fn add_device_msg_maybe_labelled( let msg_id = MsgId::new(row_id); if let Some(label) = label { - context.sql.execute( - "INSERT INTO devmsglabels (label, msg_id) VALUES (?, ?);", - params![label, msg_id], - )?; + skip_device_msg(context, label)?; } if !msg_id.is_unset() { @@ -2022,6 +2020,25 @@ fn add_device_msg_maybe_labelled( Ok(msg_id) } +pub fn skip_device_msg(context: &Context, label: &str) -> Result<(), Error> { + ensure!(!label.is_empty(), "cannot skip empty label"); + if let Ok(()) = context.sql.query_row( + "SELECT label FROM devmsglabels WHERE label=?", + params![label], + |_| Ok(()), + ) { + info!(context, "device-message {} already added", label); + return Ok(()); + } + + context.sql.execute( + "INSERT INTO devmsglabels (label) VALUES (?);", + params![label], + )?; + + Ok(()) +} + pub fn add_info_msg(context: &Context, chat_id: u32, text: impl AsRef) { let rfc724_mid = dc_create_outgoing_rfc724_mid(None, "@device"); @@ -2214,6 +2231,26 @@ mod tests { assert!(msg2_id.as_ref().unwrap().is_unset()); } + #[test] + fn test_skip_device_msg() { + let t = test_context(Some(Box::new(logging_cb))); + let res = skip_device_msg(&t.ctx, ""); + assert!(res.is_err()); + let res = skip_device_msg(&t.ctx, "some-label"); + assert!(res.is_ok()); + + let mut msg = Message::new(Viewtype::Text); + msg.text = Some("message text".to_string()); + + let msg_id = add_device_msg_once(&t.ctx, "some-label", &mut msg); + assert!(msg_id.is_ok()); + assert!(msg_id.as_ref().unwrap().is_unset()); + + let msg_id = add_device_msg_once(&t.ctx, "unused-label", &mut msg); + assert!(msg_id.is_ok()); + assert!(!msg_id.as_ref().unwrap().is_unset()); + } + fn chatlist_len(ctx: &Context, listflags: usize) -> usize { Chatlist::try_load(ctx, listflags, None, None) .unwrap() diff --git a/src/sql.rs b/src/sql.rs index 78cc2f1e4..8f70eea50 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -815,7 +815,7 @@ fn open( // records in the devmsglabels are kept when the message is deleted. // so, msg_id may or may not exist. sql.execute( - "CREATE TABLE devmsglabels (id INTEGER PRIMARY KEY AUTOINCREMENT, label TEXT, msg_id INTEGER);", + "CREATE TABLE devmsglabels (id INTEGER PRIMARY KEY AUTOINCREMENT, label TEXT, msg_id INTEGER DEFAULT 0);", NO_PARAMS, )?; sql.execute( From a4e92694ba259a0031892ffc0cbb43fd630e992a Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Mon, 18 Nov 2019 16:37:05 +0100 Subject: [PATCH 3/8] name unlabelled device-messages as such --- deltachat-ffi/deltachat.h | 84 +++++++++++++++++++-------------------- deltachat-ffi/src/lib.rs | 9 +++-- examples/repl/cmdline.rs | 2 +- src/chat.rs | 10 ++--- 4 files changed, 52 insertions(+), 53 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 8fdefbea5..bb7807d16 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -1096,9 +1096,9 @@ void dc_set_draft (dc_context_t* context, uint32_t ch * Add a message to the device-chat. * Device-messages usually contain update information * and some hints that are added during the program runs, multi-device etc. - * - * Device-messages may be added from the core, - * however, with this function, this can be done from the ui as well. + * The device-message is defined by a label; + * If a message with the same label was added or skipped before, + * the message is not added again, even if the message was deleted in between. * If needed, the device-chat is created before. * * Sends the event #DC_EVENT_MSGS_CHANGED on success. @@ -1106,50 +1106,13 @@ void dc_set_draft (dc_context_t* context, uint32_t ch * * @memberof dc_context_t * @param context The context as created by dc_context_new(). - * @param msg Message to be added to the device-chat. - * The message appears to the user as an incoming message. - * @return The ID of the added message. - */ -uint32_t dc_add_device_msg (dc_context_t* context, dc_msg_t* msg); - - -/** - * Add a message only one time to the device-chat. - * The device-message is defined by a name. - * If a message with the same name was added before, - * the message is not added again, even if the message was deleted in between. - * Use dc_add_device_msg() to add device-messages unconditionally. - * - * Sends the event #DC_EVENT_MSGS_CHANGED on success. - * - * @memberof dc_context_t - * @param context The context as created by dc_context_new(). * @param label A unique name for the message to add. * The label is typically not displayed to the user and * must be created from the characters `A-Z`, `a-z`, `0-9`, `_` or `-`. * @param msg Message to be added to the device-chat. * The message appears to the user as an incoming message. * @return The ID of the added message, - * if the message was already added before or on errors, 0 is returned. - */ -uint32_t dc_add_device_msg_once (dc_context_t* context, const char* label, dc_msg_t* msg); - - -/** - * Skip a device-message permanently. - * Subsequent calls to dc_add_device_msg_once() with the same label - * won't add the device-message then. - * This might be handy if you want to add - * eg. different messages for first-install and updates. - * - * @memberof dc_context_t - * @param context The context as created by dc_context_new(). - * @param label A unique name for the message to skip. - * The label is typically not displayed to the user and - * must be created from the characters `A-Z`, `a-z`, `0-9`, `_` or `-`. - * If a message with that label already exist, - * nothing happens. - * @return None. + * if the message was already added or skipped before or on errors, 0 is returned. * * Example: * ~~~ @@ -1170,6 +1133,41 @@ uint32_t dc_add_device_msg_once (dc_context_t* context, const char* * } * ~~~ */ +uint32_t dc_add_device_msg_once (dc_context_t* context, const char* label, dc_msg_t* msg); + + +/** + * Add a message to the device-chat unconditionally. + * As this skips the test if the message was added before, + * normally, you should prefer dc_add_device_msg_once() over this function + * + * Sends the event #DC_EVENT_MSGS_CHANGED on success. + * + * @memberof dc_context_t + * @param context The context as created by dc_context_new(). + * @param msg Message to be added to the device-chat. + * The message appears to the user as an incoming message. + * @return The ID of the added message. + */ +uint32_t dc_add_device_msg_unlabelled(dc_context_t* context, dc_msg_t* msg); + + +/** + * Skip a device-message permanently. + * Subsequent calls to dc_add_device_msg_once() with the same label + * won't add the device-message then. + * This might be handy if you want to add + * eg. different messages for first-install and updates. + * + * @memberof dc_context_t + * @param context The context as created by dc_context_new(). + * @param label A unique name for the message to skip. + * The label is typically not displayed to the user and + * must be created from the characters `A-Z`, `a-z`, `0-9`, `_` or `-`. + * If a message with that label already exist, + * nothing happens. + * @return None. + */ void dc_skip_device_msg (dc_context_t* context, const char* label); @@ -2798,9 +2796,7 @@ int dc_chat_is_self_talk (const dc_chat_t* chat); * From the ui view, device-talks are not very special, * the user can delete and forward messages, archive the chat, set notifications etc. * - * Messages may be added from the core to the device chat, - * so the chat just pops up as usual. - * However, if needed the ui can also add messages using dc_add_device_msg() + * Messages can be added to the device-talk using dc_add_device_msg_once() * * @memberof dc_chat_t * @param chat The chat object. diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index b0f469f9c..a8722ef47 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -812,16 +812,19 @@ pub unsafe extern "C" fn dc_set_draft( } #[no_mangle] -pub unsafe extern "C" fn dc_add_device_msg(context: *mut dc_context_t, msg: *mut dc_msg_t) -> u32 { +pub unsafe extern "C" fn dc_add_device_msg_unlabelled( + context: *mut dc_context_t, + msg: *mut dc_msg_t, +) -> u32 { if context.is_null() || msg.is_null() { - eprintln!("ignoring careless call to dc_add_device_msg()"); + eprintln!("ignoring careless call to dc_add_device_msg_unlabelled()"); return 0; } let ffi_context = &mut *context; let ffi_msg = &mut *msg; ffi_context .with_inner(|ctx| { - chat::add_device_msg(ctx, &mut ffi_msg.message) + chat::add_device_msg_unlabelled(ctx, &mut ffi_msg.message) .unwrap_or_log_default(ctx, "Failed to add device message") }) .map(|msg_id| msg_id.to_u32()) diff --git a/examples/repl/cmdline.rs b/examples/repl/cmdline.rs index 365853457..179f60499 100644 --- a/examples/repl/cmdline.rs +++ b/examples/repl/cmdline.rs @@ -837,7 +837,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E ); let mut msg = Message::new(Viewtype::Text); msg.set_text(Some(arg1.to_string())); - chat::add_device_msg(context, &mut msg)?; + chat::add_device_msg_unlabelled(context, &mut msg)?; } "listmedia" => { ensure!(sel_chat.is_some(), "No chat selected."); diff --git a/src/chat.rs b/src/chat.rs index 289c31540..2ae1fa16a 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -1952,7 +1952,7 @@ pub fn get_chat_id_by_grpid(context: &Context, grpid: impl AsRef) -> (u32, .unwrap_or((0, false, Blocked::Not)) } -pub fn add_device_msg(context: &Context, msg: &mut Message) -> Result { +pub fn add_device_msg_unlabelled(context: &Context, msg: &mut Message) -> Result { add_device_msg_maybe_labelled(context, None, msg) } @@ -2147,18 +2147,18 @@ mod tests { } #[test] - fn test_add_device_msg() { + fn test_add_device_msg_unlabelled() { let t = test_context(Some(Box::new(logging_cb))); // add two device-messages let mut msg1 = Message::new(Viewtype::Text); msg1.text = Some("first message".to_string()); - let msg1_id = add_device_msg(&t.ctx, &mut msg1); + let msg1_id = add_device_msg_unlabelled(&t.ctx, &mut msg1); assert!(msg1_id.is_ok()); let mut msg2 = Message::new(Viewtype::Text); msg2.text = Some("second message".to_string()); - let msg2_id = add_device_msg(&t.ctx, &mut msg2); + let msg2_id = add_device_msg_unlabelled(&t.ctx, &mut msg2); assert!(msg2_id.is_ok()); assert_ne!(msg1_id.as_ref().unwrap(), msg2_id.as_ref().unwrap()); @@ -2263,7 +2263,7 @@ mod tests { let t = dummy_context(); let mut msg = Message::new(Viewtype::Text); msg.text = Some("foo".to_string()); - let msg_id = add_device_msg(&t.ctx, &mut msg).unwrap(); + let msg_id = add_device_msg_unlabelled(&t.ctx, &mut msg).unwrap(); let chat_id1 = message::Message::load_from_db(&t.ctx, msg_id) .unwrap() .chat_id; From 1534a07deda2c36cd69923abc9698695dfc0d2a1 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Mon, 18 Nov 2019 17:14:31 +0100 Subject: [PATCH 4/8] add dc_has_device_msg() --- deltachat-ffi/deltachat.h | 15 +++++++++++++++ deltachat-ffi/src/lib.rs | 17 +++++++++++++++++ src/chat.rs | 36 ++++++++++++++++++++++++++++++------ 3 files changed, 62 insertions(+), 6 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index bb7807d16..22a544b3c 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -1171,6 +1171,21 @@ uint32_t dc_add_device_msg_unlabelled(dc_context_t* context, dc_msg_t* ms void dc_skip_device_msg (dc_context_t* context, const char* label); + +/** + * Check if a device-message was ever added or skipped. + * Device-messages can be added or skipped + * using dc_add_device_msg_once() or dc_skip_device_msg(). + * + * @memberof dc_context_t + * @param context The context as created by dc_context_new(). + * @param label Label of the message to check. + * @return 1=A message with this label was added or skipped at some point, + * 0=A message with this label was never added nor skipped. + */ +int dc_has_device_msg (dc_context_t* context, const char* label); + + /** * Get draft for a chat, if any. * See dc_set_draft() for more details about drafts. diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index a8722ef47..c0bbb5dff 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -870,6 +870,23 @@ pub unsafe extern "C" fn dc_skip_device_msg( .unwrap_or(()) } +#[no_mangle] +pub unsafe extern "C" fn dc_has_device_msg( + context: *mut dc_context_t, + label: *const libc::c_char, +) -> libc::c_int { + if context.is_null() || label.is_null() { + eprintln!("ignoring careless call to dc_has_device_msg()"); + return 0; + } + let ffi_context = &mut *context; + ffi_context + .with_inner(|ctx| { + chat::has_device_msg(ctx, &to_string_lossy(label)).unwrap_or(false) as libc::c_int + }) + .unwrap_or(0) +} + #[no_mangle] pub unsafe extern "C" fn dc_get_draft(context: *mut dc_context_t, chat_id: u32) -> *mut dc_msg_t { if context.is_null() { diff --git a/src/chat.rs b/src/chat.rs index 2ae1fa16a..6bb0b40dc 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2021,12 +2021,7 @@ fn add_device_msg_maybe_labelled( } pub fn skip_device_msg(context: &Context, label: &str) -> Result<(), Error> { - ensure!(!label.is_empty(), "cannot skip empty label"); - if let Ok(()) = context.sql.query_row( - "SELECT label FROM devmsglabels WHERE label=?", - params![label], - |_| Ok(()), - ) { + if has_device_msg(context, label)? { info!(context, "device-message {} already added", label); return Ok(()); } @@ -2039,6 +2034,19 @@ pub fn skip_device_msg(context: &Context, label: &str) -> Result<(), Error> { Ok(()) } +pub fn has_device_msg(context: &Context, label: &str) -> Result { + ensure!(!label.is_empty(), "empty label"); + if let Ok(()) = context.sql.query_row( + "SELECT label FROM devmsglabels WHERE label=?", + params![label], + |_| Ok(()), + ) { + return Ok(true); + } + + Ok(false) +} + pub fn add_info_msg(context: &Context, chat_id: u32, text: impl AsRef) { let rfc724_mid = dc_create_outgoing_rfc724_mid(None, "@device"); @@ -2251,6 +2259,22 @@ mod tests { assert!(!msg_id.as_ref().unwrap().is_unset()); } + #[test] + fn test_has_device_msg() { + let t = test_context(Some(Box::new(logging_cb))); + skip_device_msg(&t.ctx, "some-label").ok(); + assert!(has_device_msg(&t.ctx, "some-label").unwrap()); + + let mut msg = Message::new(Viewtype::Text); + msg.text = Some("message text".to_string()); + add_device_msg_once(&t.ctx, "another-label", &mut msg).ok(); + assert!(has_device_msg(&t.ctx, "another-label").unwrap()); + + assert!(!has_device_msg(&t.ctx, "unused-label").unwrap()); + + assert!(has_device_msg(&t.ctx, "").is_err()); + } + fn chatlist_len(ctx: &Context, listflags: usize) -> usize { Chatlist::try_load(ctx, listflags, None, None) .unwrap() From d3fa289f272276c1b168b5b7abe9de4619b3a0f9 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Tue, 19 Nov 2019 11:49:02 +0100 Subject: [PATCH 5/8] streamline dc_add_device_msg_once|unlabelled() to one function --- deltachat-ffi/deltachat.h | 33 +++++++++------------------------ deltachat-ffi/src/lib.rs | 32 ++++++++------------------------ examples/repl/cmdline.rs | 2 +- src/chat.rs | 34 +++++++++++----------------------- src/imex.rs | 2 +- 5 files changed, 30 insertions(+), 73 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 22a544b3c..f24de72ca 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -1096,8 +1096,8 @@ void dc_set_draft (dc_context_t* context, uint32_t ch * Add a message to the device-chat. * Device-messages usually contain update information * and some hints that are added during the program runs, multi-device etc. - * The device-message is defined by a label; - * If a message with the same label was added or skipped before, + * The device-message may be defined by a label; + * if a message with the same label was added or skipped before, * the message is not added again, even if the message was deleted in between. * If needed, the device-chat is created before. * @@ -1109,6 +1109,7 @@ void dc_set_draft (dc_context_t* context, uint32_t ch * @param label A unique name for the message to add. * The label is typically not displayed to the user and * must be created from the characters `A-Z`, `a-z`, `0-9`, `_` or `-`. + * If you pass NULL here, the message is added unconditionally. * @param msg Message to be added to the device-chat. * The message appears to the user as an incoming message. * @return The ID of the added message, @@ -1122,39 +1123,23 @@ void dc_set_draft (dc_context_t* context, uint32_t ch * dc_msg_t* changelog_msg = dc_msg_new(DC_MSG_TEXT); * dc_msg_set_text(changelog_msg, "we have added 3 new emojis :)"); * - * if (dc_add_device_msg_once(context, "welcome", welcome_msg)) { + * if (dc_add_device_msg(context, "welcome", welcome_msg)) { * // do not add the changelog on a new installations - * // not now and not when this code is executed again * dc_skip_device_msg(context, "update-123"); * } else { * // welcome message was not added now, this is an oder installation, * // add a changelog - * dc_add_device_msg_once(context, "update-123", changelog_msg); + * dc_add_device_msg(context, "update-123", changelog_msg); * } * ~~~ */ -uint32_t dc_add_device_msg_once (dc_context_t* context, const char* label, dc_msg_t* msg); - - -/** - * Add a message to the device-chat unconditionally. - * As this skips the test if the message was added before, - * normally, you should prefer dc_add_device_msg_once() over this function - * - * Sends the event #DC_EVENT_MSGS_CHANGED on success. - * - * @memberof dc_context_t - * @param context The context as created by dc_context_new(). - * @param msg Message to be added to the device-chat. - * The message appears to the user as an incoming message. - * @return The ID of the added message. - */ -uint32_t dc_add_device_msg_unlabelled(dc_context_t* context, dc_msg_t* msg); +uint32_t dc_add_device_msg (dc_context_t* context, const char* label, dc_msg_t* msg); /** * Skip a device-message permanently. - * Subsequent calls to dc_add_device_msg_once() with the same label + * Subsequent calls to dc_add_device_msg() with the same label * won't add the device-message then. * This might be handy if you want to add * eg. different messages for first-install and updates. @@ -1175,7 +1160,7 @@ void dc_skip_device_msg (dc_context_t* context, const char* /** * Check if a device-message was ever added or skipped. * Device-messages can be added or skipped - * using dc_add_device_msg_once() or dc_skip_device_msg(). + * using dc_add_device_msg() or dc_skip_device_msg(). * * @memberof dc_context_t * @param context The context as created by dc_context_new(). @@ -2811,7 +2796,7 @@ int dc_chat_is_self_talk (const dc_chat_t* chat); * From the ui view, device-talks are not very special, * the user can delete and forward messages, archive the chat, set notifications etc. * - * Messages can be added to the device-talk using dc_add_device_msg_once() + * Messages can be added to the device-talk using dc_add_device_msg() * * @memberof dc_chat_t * @param chat The chat object. diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index c0bbb5dff..37062f422 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -812,41 +812,25 @@ pub unsafe extern "C" fn dc_set_draft( } #[no_mangle] -pub unsafe extern "C" fn dc_add_device_msg_unlabelled( - context: *mut dc_context_t, - msg: *mut dc_msg_t, -) -> u32 { - if context.is_null() || msg.is_null() { - eprintln!("ignoring careless call to dc_add_device_msg_unlabelled()"); - return 0; - } - let ffi_context = &mut *context; - let ffi_msg = &mut *msg; - ffi_context - .with_inner(|ctx| { - chat::add_device_msg_unlabelled(ctx, &mut ffi_msg.message) - .unwrap_or_log_default(ctx, "Failed to add device message") - }) - .map(|msg_id| msg_id.to_u32()) - .unwrap_or(0) -} - -#[no_mangle] -pub unsafe extern "C" fn dc_add_device_msg_once( +pub unsafe extern "C" fn dc_add_device_msg( context: *mut dc_context_t, label: *const libc::c_char, msg: *mut dc_msg_t, ) -> u32 { if context.is_null() || label.is_null() || msg.is_null() { - eprintln!("ignoring careless call to dc_add_device_msg_once()"); + eprintln!("ignoring careless call to dc_add_device_msg()"); return 0; } let ffi_context = &mut *context; let ffi_msg = &mut *msg; ffi_context .with_inner(|ctx| { - chat::add_device_msg_once(ctx, &to_string_lossy(label), &mut ffi_msg.message) - .unwrap_or_log_default(ctx, "Failed to add device message once") + chat::add_device_msg( + ctx, + to_opt_string_lossy(label).as_ref().map(|x| x.as_str()), + &mut ffi_msg.message, + ) + .unwrap_or_log_default(ctx, "Failed to add device message") }) .map(|msg_id| msg_id.to_u32()) .unwrap_or(0) diff --git a/examples/repl/cmdline.rs b/examples/repl/cmdline.rs index 179f60499..09cd3c3cf 100644 --- a/examples/repl/cmdline.rs +++ b/examples/repl/cmdline.rs @@ -837,7 +837,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E ); let mut msg = Message::new(Viewtype::Text); msg.set_text(Some(arg1.to_string())); - chat::add_device_msg_unlabelled(context, &mut msg)?; + chat::add_device_msg(context, None, &mut msg)?; } "listmedia" => { ensure!(sel_chat.is_some(), "No chat selected."); diff --git a/src/chat.rs b/src/chat.rs index 6bb0b40dc..7df2bd3b1 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -1952,19 +1952,7 @@ pub fn get_chat_id_by_grpid(context: &Context, grpid: impl AsRef) -> (u32, .unwrap_or((0, false, Blocked::Not)) } -pub fn add_device_msg_unlabelled(context: &Context, msg: &mut Message) -> Result { - add_device_msg_maybe_labelled(context, None, msg) -} - -pub fn add_device_msg_once( - context: &Context, - label: &str, - msg: &mut Message, -) -> Result { - add_device_msg_maybe_labelled(context, Some(label), msg) -} - -fn add_device_msg_maybe_labelled( +pub fn add_device_msg( context: &Context, label: Option<&str>, msg: &mut Message, @@ -2161,12 +2149,12 @@ mod tests { // add two device-messages let mut msg1 = Message::new(Viewtype::Text); msg1.text = Some("first message".to_string()); - let msg1_id = add_device_msg_unlabelled(&t.ctx, &mut msg1); + let msg1_id = add_device_msg(&t.ctx, None, &mut msg1); assert!(msg1_id.is_ok()); let mut msg2 = Message::new(Viewtype::Text); msg2.text = Some("second message".to_string()); - let msg2_id = add_device_msg_unlabelled(&t.ctx, &mut msg2); + let msg2_id = add_device_msg(&t.ctx, None, &mut msg2); assert!(msg2_id.is_ok()); assert_ne!(msg1_id.as_ref().unwrap(), msg2_id.as_ref().unwrap()); @@ -2190,19 +2178,19 @@ mod tests { } #[test] - fn test_add_device_msg_once() { + fn test_add_device_msg_labelled() { let t = test_context(Some(Box::new(logging_cb))); // add two device-messages with the same label (second attempt is not added) let mut msg1 = Message::new(Viewtype::Text); msg1.text = Some("first message".to_string()); - let msg1_id = add_device_msg_once(&t.ctx, "any-label", &mut msg1); + let msg1_id = add_device_msg(&t.ctx, Some("any-label"), &mut msg1); assert!(msg1_id.is_ok()); assert!(!msg1_id.as_ref().unwrap().is_unset()); let mut msg2 = Message::new(Viewtype::Text); msg2.text = Some("second message".to_string()); - let msg2_id = add_device_msg_once(&t.ctx, "any-label", &mut msg2); + let msg2_id = add_device_msg(&t.ctx, Some("any-label"), &mut msg2); assert!(msg2_id.is_ok()); assert!(msg2_id.as_ref().unwrap().is_unset()); @@ -2234,7 +2222,7 @@ mod tests { message::delete_msgs(&t.ctx, &[*msg1_id.as_ref().unwrap()]); let msg1 = message::Message::load_from_db(&t.ctx, *msg1_id.as_ref().unwrap()); assert!(msg1.is_err() || msg1.unwrap().chat_id == DC_CHAT_ID_TRASH); - let msg3_id = add_device_msg_once(&t.ctx, "any-label", &mut msg2); + let msg3_id = add_device_msg(&t.ctx, Some("any-label"), &mut msg2); assert!(msg3_id.is_ok()); assert!(msg2_id.as_ref().unwrap().is_unset()); } @@ -2250,11 +2238,11 @@ mod tests { let mut msg = Message::new(Viewtype::Text); msg.text = Some("message text".to_string()); - let msg_id = add_device_msg_once(&t.ctx, "some-label", &mut msg); + let msg_id = add_device_msg(&t.ctx, Some("some-label"), &mut msg); assert!(msg_id.is_ok()); assert!(msg_id.as_ref().unwrap().is_unset()); - let msg_id = add_device_msg_once(&t.ctx, "unused-label", &mut msg); + let msg_id = add_device_msg(&t.ctx, Some("unused-label"), &mut msg); assert!(msg_id.is_ok()); assert!(!msg_id.as_ref().unwrap().is_unset()); } @@ -2267,7 +2255,7 @@ mod tests { let mut msg = Message::new(Viewtype::Text); msg.text = Some("message text".to_string()); - add_device_msg_once(&t.ctx, "another-label", &mut msg).ok(); + add_device_msg(&t.ctx, Some("another-label"), &mut msg).ok(); assert!(has_device_msg(&t.ctx, "another-label").unwrap()); assert!(!has_device_msg(&t.ctx, "unused-label").unwrap()); @@ -2287,7 +2275,7 @@ mod tests { let t = dummy_context(); let mut msg = Message::new(Viewtype::Text); msg.text = Some("foo".to_string()); - let msg_id = add_device_msg_unlabelled(&t.ctx, &mut msg).unwrap(); + let msg_id = add_device_msg(&t.ctx, None, &mut msg).unwrap(); let chat_id1 = message::Message::load_from_db(&t.ctx, msg_id) .unwrap() .chat_id; diff --git a/src/imex.rs b/src/imex.rs index 8248cb3a6..e16d2b839 100644 --- a/src/imex.rs +++ b/src/imex.rs @@ -247,7 +247,7 @@ fn maybe_add_bcc_self_device_msg(context: &Context) -> Result<()> { go to the settings and enable \"Send copy to self\"." .to_string(), ); - chat::add_device_msg_once(context, "bcc-self-hint", &mut msg)?; + chat::add_device_msg(context, Some("bcc-self-hint"), &mut msg)?; } Ok(()) } From 2a4c19360191ac91969eb03acf3526440fa214e3 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Tue, 19 Nov 2019 11:56:35 +0100 Subject: [PATCH 6/8] simplify check for existing device-message --- src/chat.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 7df2bd3b1..d452c2058 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -1962,12 +1962,7 @@ pub fn add_device_msg( // if the device message is labeled and was ever added, do nothing if let Some(label) = label { - ensure!(!label.is_empty(), "cannot add empty label"); - if let Ok(()) = context.sql.query_row( - "SELECT label FROM devmsglabels WHERE label=?", - params![label], - |_| Ok(()), - ) { + if has_device_msg(context, label)? { info!(context, "device-message {} already added", label); return Ok(MsgId::new_unset()); } From 700e10bc0e1e442139595c4dacf950651a7710c5 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Tue, 19 Nov 2019 13:05:01 +0100 Subject: [PATCH 7/8] use dc_add_device_msg() also to add labels-only, this is clearer as having a function with 'skip' in the name --- deltachat-ffi/deltachat.h | 37 ++++----------- deltachat-ffi/src/lib.rs | 29 ++++-------- examples/repl/cmdline.rs | 2 +- src/chat.rs | 98 ++++++++++++++++++--------------------- src/imex.rs | 2 +- 5 files changed, 65 insertions(+), 103 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index f24de72ca..e9980296c 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -1112,8 +1112,10 @@ void dc_set_draft (dc_context_t* context, uint32_t ch * If you pass NULL here, the message is added unconditionally. * @param msg Message to be added to the device-chat. * The message appears to the user as an incoming message. - * @return The ID of the added message, - * if the message was already added or skipped before or on errors, 0 is returned. + * If you pass NULL here, only the given label will be added + * and block adding messages with that label in the future. + * @return The ID of the just added message, + * if the message was already added or no message to add is given, 0 is returned. * * Example: * ~~~ @@ -1126,7 +1128,7 @@ void dc_set_draft (dc_context_t* context, uint32_t ch * if (dc_add_device_msg(context, "welcome", welcome_msg)) { * // do not add the changelog on a new installations - * // not now and not when this code is executed again - * dc_skip_device_msg(context, "update-123"); + * dc_add_device_msg(context, "update-123", NULL); * } else { * // welcome message was not added now, this is an oder installation, * // add a changelog @@ -1138,35 +1140,14 @@ uint32_t dc_add_device_msg (dc_context_t* context, const char* /** - * Skip a device-message permanently. - * Subsequent calls to dc_add_device_msg() with the same label - * won't add the device-message then. - * This might be handy if you want to add - * eg. different messages for first-install and updates. - * - * @memberof dc_context_t - * @param context The context as created by dc_context_new(). - * @param label A unique name for the message to skip. - * The label is typically not displayed to the user and - * must be created from the characters `A-Z`, `a-z`, `0-9`, `_` or `-`. - * If a message with that label already exist, - * nothing happens. - * @return None. - */ -void dc_skip_device_msg (dc_context_t* context, const char* label); - - - -/** - * Check if a device-message was ever added or skipped. - * Device-messages can be added or skipped - * using dc_add_device_msg() or dc_skip_device_msg(). + * Check if a device-message with a given label was ever added. + * Device-messages can be added dc_add_device_msg(). * * @memberof dc_context_t * @param context The context as created by dc_context_new(). * @param label Label of the message to check. - * @return 1=A message with this label was added or skipped at some point, - * 0=A message with this label was never added nor skipped. + * @return 1=A message with this label was added at some point, + * 0=A message with this label was never added. */ int dc_has_device_msg (dc_context_t* context, const char* label); diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 37062f422..b68d408e6 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -817,18 +817,23 @@ pub unsafe extern "C" fn dc_add_device_msg( label: *const libc::c_char, msg: *mut dc_msg_t, ) -> u32 { - if context.is_null() || label.is_null() || msg.is_null() { + if context.is_null() || (label.is_null() && msg.is_null()) { eprintln!("ignoring careless call to dc_add_device_msg()"); return 0; } let ffi_context = &mut *context; - let ffi_msg = &mut *msg; + let msg = if msg.is_null() { + None + } else { + let ffi_msg: &mut MessageWrapper = &mut *msg; + Some(&mut ffi_msg.message) + }; ffi_context .with_inner(|ctx| { chat::add_device_msg( ctx, to_opt_string_lossy(label).as_ref().map(|x| x.as_str()), - &mut ffi_msg.message, + msg, ) .unwrap_or_log_default(ctx, "Failed to add device message") }) @@ -836,24 +841,6 @@ pub unsafe extern "C" fn dc_add_device_msg( .unwrap_or(0) } -#[no_mangle] -pub unsafe extern "C" fn dc_skip_device_msg( - context: *mut dc_context_t, - label: *const libc::c_char, -) { - if context.is_null() || label.is_null() { - eprintln!("ignoring careless call to dc_skip_device_msg()"); - return; - } - let ffi_context = &mut *context; - ffi_context - .with_inner(|ctx| { - chat::skip_device_msg(ctx, &to_string_lossy(label)) - .unwrap_or_log_default(ctx, "Failed to skip device message") - }) - .unwrap_or(()) -} - #[no_mangle] pub unsafe extern "C" fn dc_has_device_msg( context: *mut dc_context_t, diff --git a/examples/repl/cmdline.rs b/examples/repl/cmdline.rs index 09cd3c3cf..4656b1dd3 100644 --- a/examples/repl/cmdline.rs +++ b/examples/repl/cmdline.rs @@ -837,7 +837,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E ); let mut msg = Message::new(Viewtype::Text); msg.set_text(Some(arg1.to_string())); - chat::add_device_msg(context, None, &mut msg)?; + chat::add_device_msg(context, None, Some(&mut msg))?; } "listmedia" => { ensure!(sel_chat.is_some(), "No chat selected."); diff --git a/src/chat.rs b/src/chat.rs index d452c2058..ab2fdaed4 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -1955,45 +1955,53 @@ pub fn get_chat_id_by_grpid(context: &Context, grpid: impl AsRef) -> (u32, pub fn add_device_msg( context: &Context, label: Option<&str>, - msg: &mut Message, + msg: Option<&mut Message>, ) -> Result { + ensure!( + label.is_some() || msg.is_some(), + "device-messages need label, msg or both" + ); let (chat_id, _blocked) = create_or_lookup_by_contact_id(context, DC_CONTACT_ID_DEVICE, Blocked::Not)?; + let mut msg_id = MsgId::new_unset(); - // if the device message is labeled and was ever added, do nothing if let Some(label) = label { if has_device_msg(context, label)? { info!(context, "device-message {} already added", label); - return Ok(MsgId::new_unset()); + return Ok(msg_id); } } - // insert the device message - let rfc724_mid = dc_create_outgoing_rfc724_mid(None, "@device"); - prepare_msg_blob(context, msg)?; - unarchive(context, chat_id)?; + if let Some(msg) = msg { + let rfc724_mid = dc_create_outgoing_rfc724_mid(None, "@device"); + prepare_msg_blob(context, msg)?; + unarchive(context, chat_id)?; - context.sql.execute( - "INSERT INTO msgs (chat_id,from_id,to_id, timestamp,type,state, txt,param,rfc724_mid) \ - VALUES (?,?,?, ?,?,?, ?,?,?);", - params![ - chat_id, - DC_CONTACT_ID_DEVICE, - DC_CONTACT_ID_SELF, - dc_create_smeared_timestamp(context), - msg.type_0, - MessageState::InFresh, - msg.text.as_ref().map_or("", String::as_str), - msg.param.to_string(), - rfc724_mid, - ], - )?; + context.sql.execute( + "INSERT INTO msgs (chat_id,from_id,to_id, timestamp,type,state, txt,param,rfc724_mid) \ + VALUES (?,?,?, ?,?,?, ?,?,?);", + params![ + chat_id, + DC_CONTACT_ID_DEVICE, + DC_CONTACT_ID_SELF, + dc_create_smeared_timestamp(context), + msg.type_0, + MessageState::InFresh, + msg.text.as_ref().map_or("", String::as_str), + msg.param.to_string(), + rfc724_mid, + ], + )?; - let row_id = sql::get_rowid(context, &context.sql, "msgs", "rfc724_mid", &rfc724_mid); - let msg_id = MsgId::new(row_id); + let row_id = sql::get_rowid(context, &context.sql, "msgs", "rfc724_mid", &rfc724_mid); + msg_id = MsgId::new(row_id); + } if let Some(label) = label { - skip_device_msg(context, label)?; + context.sql.execute( + "INSERT INTO devmsglabels (label) VALUES (?);", + params![label], + )?; } if !msg_id.is_unset() { @@ -2003,20 +2011,6 @@ pub fn add_device_msg( Ok(msg_id) } -pub fn skip_device_msg(context: &Context, label: &str) -> Result<(), Error> { - if has_device_msg(context, label)? { - info!(context, "device-message {} already added", label); - return Ok(()); - } - - context.sql.execute( - "INSERT INTO devmsglabels (label) VALUES (?);", - params![label], - )?; - - Ok(()) -} - pub fn has_device_msg(context: &Context, label: &str) -> Result { ensure!(!label.is_empty(), "empty label"); if let Ok(()) = context.sql.query_row( @@ -2144,12 +2138,12 @@ mod tests { // add two device-messages let mut msg1 = Message::new(Viewtype::Text); msg1.text = Some("first message".to_string()); - let msg1_id = add_device_msg(&t.ctx, None, &mut msg1); + let msg1_id = add_device_msg(&t.ctx, None, Some(&mut msg1)); assert!(msg1_id.is_ok()); let mut msg2 = Message::new(Viewtype::Text); msg2.text = Some("second message".to_string()); - let msg2_id = add_device_msg(&t.ctx, None, &mut msg2); + let msg2_id = add_device_msg(&t.ctx, None, Some(&mut msg2)); assert!(msg2_id.is_ok()); assert_ne!(msg1_id.as_ref().unwrap(), msg2_id.as_ref().unwrap()); @@ -2179,13 +2173,13 @@ mod tests { // add two device-messages with the same label (second attempt is not added) let mut msg1 = Message::new(Viewtype::Text); msg1.text = Some("first message".to_string()); - let msg1_id = add_device_msg(&t.ctx, Some("any-label"), &mut msg1); + let msg1_id = add_device_msg(&t.ctx, Some("any-label"), Some(&mut msg1)); assert!(msg1_id.is_ok()); assert!(!msg1_id.as_ref().unwrap().is_unset()); let mut msg2 = Message::new(Viewtype::Text); msg2.text = Some("second message".to_string()); - let msg2_id = add_device_msg(&t.ctx, Some("any-label"), &mut msg2); + let msg2_id = add_device_msg(&t.ctx, Some("any-label"), Some(&mut msg2)); assert!(msg2_id.is_ok()); assert!(msg2_id.as_ref().unwrap().is_unset()); @@ -2217,27 +2211,27 @@ mod tests { message::delete_msgs(&t.ctx, &[*msg1_id.as_ref().unwrap()]); let msg1 = message::Message::load_from_db(&t.ctx, *msg1_id.as_ref().unwrap()); assert!(msg1.is_err() || msg1.unwrap().chat_id == DC_CHAT_ID_TRASH); - let msg3_id = add_device_msg(&t.ctx, Some("any-label"), &mut msg2); + let msg3_id = add_device_msg(&t.ctx, Some("any-label"), Some(&mut msg2)); assert!(msg3_id.is_ok()); assert!(msg2_id.as_ref().unwrap().is_unset()); } #[test] - fn test_skip_device_msg() { + fn test_add_device_msg_label_only() { let t = test_context(Some(Box::new(logging_cb))); - let res = skip_device_msg(&t.ctx, ""); + let res = add_device_msg(&t.ctx, Some(""), None); assert!(res.is_err()); - let res = skip_device_msg(&t.ctx, "some-label"); + let res = add_device_msg(&t.ctx, Some("some-label"), None); assert!(res.is_ok()); let mut msg = Message::new(Viewtype::Text); msg.text = Some("message text".to_string()); - let msg_id = add_device_msg(&t.ctx, Some("some-label"), &mut msg); + let msg_id = add_device_msg(&t.ctx, Some("some-label"), Some(&mut msg)); assert!(msg_id.is_ok()); assert!(msg_id.as_ref().unwrap().is_unset()); - let msg_id = add_device_msg(&t.ctx, Some("unused-label"), &mut msg); + let msg_id = add_device_msg(&t.ctx, Some("unused-label"), Some(&mut msg)); assert!(msg_id.is_ok()); assert!(!msg_id.as_ref().unwrap().is_unset()); } @@ -2245,12 +2239,12 @@ mod tests { #[test] fn test_has_device_msg() { let t = test_context(Some(Box::new(logging_cb))); - skip_device_msg(&t.ctx, "some-label").ok(); + add_device_msg(&t.ctx, Some("some-label"), None).ok(); assert!(has_device_msg(&t.ctx, "some-label").unwrap()); let mut msg = Message::new(Viewtype::Text); msg.text = Some("message text".to_string()); - add_device_msg(&t.ctx, Some("another-label"), &mut msg).ok(); + add_device_msg(&t.ctx, Some("another-label"), Some(&mut msg)).ok(); assert!(has_device_msg(&t.ctx, "another-label").unwrap()); assert!(!has_device_msg(&t.ctx, "unused-label").unwrap()); @@ -2270,7 +2264,7 @@ mod tests { let t = dummy_context(); let mut msg = Message::new(Viewtype::Text); msg.text = Some("foo".to_string()); - let msg_id = add_device_msg(&t.ctx, None, &mut msg).unwrap(); + let msg_id = add_device_msg(&t.ctx, None, Some(&mut msg)).unwrap(); let chat_id1 = message::Message::load_from_db(&t.ctx, msg_id) .unwrap() .chat_id; diff --git a/src/imex.rs b/src/imex.rs index e16d2b839..756fe15b2 100644 --- a/src/imex.rs +++ b/src/imex.rs @@ -247,7 +247,7 @@ fn maybe_add_bcc_self_device_msg(context: &Context) -> Result<()> { go to the settings and enable \"Send copy to self\"." .to_string(), ); - chat::add_device_msg(context, Some("bcc-self-hint"), &mut msg)?; + chat::add_device_msg(context, Some("bcc-self-hint"), Some(&mut msg))?; } Ok(()) } From e4e6a00fe4d041c25d927a474a10d929e85d0bc8 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Tue, 19 Nov 2019 13:21:02 +0100 Subject: [PATCH 8/8] rename has_device_msg() to was_device_msg_ever_added() what describes better what the function is doing --- deltachat-ffi/deltachat.h | 2 +- deltachat-ffi/src/lib.rs | 7 ++++--- src/chat.rs | 14 +++++++------- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index e9980296c..3b405bb39 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -1149,7 +1149,7 @@ uint32_t dc_add_device_msg (dc_context_t* context, const char* * @return 1=A message with this label was added at some point, * 0=A message with this label was never added. */ -int dc_has_device_msg (dc_context_t* context, const char* label); +int dc_was_device_msg_ever_added (dc_context_t* context, const char* label); /** diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index b68d408e6..c5362761a 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -842,18 +842,19 @@ pub unsafe extern "C" fn dc_add_device_msg( } #[no_mangle] -pub unsafe extern "C" fn dc_has_device_msg( +pub unsafe extern "C" fn dc_was_device_msg_ever_added( context: *mut dc_context_t, label: *const libc::c_char, ) -> libc::c_int { if context.is_null() || label.is_null() { - eprintln!("ignoring careless call to dc_has_device_msg()"); + eprintln!("ignoring careless call to dc_was_device_msg_ever_added()"); return 0; } let ffi_context = &mut *context; ffi_context .with_inner(|ctx| { - chat::has_device_msg(ctx, &to_string_lossy(label)).unwrap_or(false) as libc::c_int + chat::was_device_msg_ever_added(ctx, &to_string_lossy(label)).unwrap_or(false) + as libc::c_int }) .unwrap_or(0) } diff --git a/src/chat.rs b/src/chat.rs index ab2fdaed4..31f54ebc2 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -1966,7 +1966,7 @@ pub fn add_device_msg( let mut msg_id = MsgId::new_unset(); if let Some(label) = label { - if has_device_msg(context, label)? { + if was_device_msg_ever_added(context, label)? { info!(context, "device-message {} already added", label); return Ok(msg_id); } @@ -2011,7 +2011,7 @@ pub fn add_device_msg( Ok(msg_id) } -pub fn has_device_msg(context: &Context, label: &str) -> Result { +pub fn was_device_msg_ever_added(context: &Context, label: &str) -> Result { ensure!(!label.is_empty(), "empty label"); if let Ok(()) = context.sql.query_row( "SELECT label FROM devmsglabels WHERE label=?", @@ -2237,19 +2237,19 @@ mod tests { } #[test] - fn test_has_device_msg() { + fn test_was_device_msg_ever_added() { let t = test_context(Some(Box::new(logging_cb))); add_device_msg(&t.ctx, Some("some-label"), None).ok(); - assert!(has_device_msg(&t.ctx, "some-label").unwrap()); + assert!(was_device_msg_ever_added(&t.ctx, "some-label").unwrap()); let mut msg = Message::new(Viewtype::Text); msg.text = Some("message text".to_string()); add_device_msg(&t.ctx, Some("another-label"), Some(&mut msg)).ok(); - assert!(has_device_msg(&t.ctx, "another-label").unwrap()); + assert!(was_device_msg_ever_added(&t.ctx, "another-label").unwrap()); - assert!(!has_device_msg(&t.ctx, "unused-label").unwrap()); + assert!(!was_device_msg_ever_added(&t.ctx, "unused-label").unwrap()); - assert!(has_device_msg(&t.ctx, "").is_err()); + assert!(was_device_msg_ever_added(&t.ctx, "").is_err()); } fn chatlist_len(ctx: &Context, listflags: usize) -> usize {