From c7691fbebe62752bcc10b68875f8f56bb2b3c3bb Mon Sep 17 00:00:00 2001 From: link2xt Date: Fri, 25 Nov 2022 22:22:29 +0000 Subject: [PATCH] Use UPSERT when saving peerstates This way there is no need to distinguish between creating and updating peerstate. --- src/decrypt.rs | 4 +-- src/mimeparser.rs | 6 ++-- src/peerstate.rs | 72 +++++++++++++++++++++------------------------- src/qr.rs | 2 +- src/receive_imf.rs | 2 +- src/securejoin.rs | 7 ++--- src/sql.rs | 2 +- src/tests/aeap.rs | 2 +- 8 files changed, 43 insertions(+), 54 deletions(-) diff --git a/src/decrypt.rs b/src/decrypt.rs index 4f76a2786..8c57cdd36 100644 --- a/src/decrypt.rs +++ b/src/decrypt.rs @@ -306,7 +306,7 @@ pub(crate) async fn get_autocrypt_peerstate( if addr_cmp(&peerstate.addr, from) { if allow_change { peerstate.apply_header(header, message_time); - peerstate.save_to_db(&context.sql, false).await?; + peerstate.save_to_db(&context.sql).await?; } else { info!( context, @@ -322,7 +322,7 @@ pub(crate) async fn get_autocrypt_peerstate( // to the database. } else { let p = Peerstate::from_header(header, message_time); - p.save_to_db(&context.sql, true).await?; + p.save_to_db(&context.sql).await?; peerstate = Some(p); } } else { diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 04a4b4d52..2e811ebb9 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -310,7 +310,7 @@ impl MimeMessage { // && decryption_info.dkim_results.allow_keychange { peerstate.degrade_encryption(message_time); - peerstate.save_to_db(&context.sql, false).await?; + peerstate.save_to_db(&context.sql).await?; } } (Ok(mail), HashSet::new(), false) @@ -1586,11 +1586,11 @@ async fn update_gossip_peerstates( let peerstate; if let Some(mut p) = Peerstate::from_addr(context, &header.addr).await? { p.apply_gossip(&header, message_time); - p.save_to_db(&context.sql, false).await?; + p.save_to_db(&context.sql).await?; peerstate = p; } else { let p = Peerstate::from_gossip(&header, message_time); - p.save_to_db(&context.sql, true).await?; + p.save_to_db(&context.sql).await?; peerstate = p; }; peerstate diff --git a/src/peerstate.rs b/src/peerstate.rs index 09453cac7..11db2c467 100644 --- a/src/peerstate.rs +++ b/src/peerstate.rs @@ -410,37 +410,34 @@ impl Peerstate { } } - pub async fn save_to_db(&self, sql: &Sql, create: bool) -> Result<()> { - if self.to_save || create { + pub async fn save_to_db(&self, sql: &Sql) -> Result<()> { + if self.to_save { sql.execute( - if create { - "INSERT INTO acpeerstates ( \ - last_seen, \ - last_seen_autocrypt, \ - prefer_encrypted, \ - public_key, \ - gossip_timestamp, \ - gossip_key, \ - public_key_fingerprint, \ - gossip_key_fingerprint, \ - verified_key, \ - verified_key_fingerprint, \ - addr \ - ) VALUES(?,?,?,?,?,?,?,?,?,?,?)" - } else { - "UPDATE acpeerstates \ - SET last_seen=?, \ - last_seen_autocrypt=?, \ - prefer_encrypted=?, \ - public_key=?, \ - gossip_timestamp=?, \ - gossip_key=?, \ - public_key_fingerprint=?, \ - gossip_key_fingerprint=?, \ - verified_key=?, \ - verified_key_fingerprint=? \ - WHERE addr=?" - }, + "INSERT INTO acpeerstates ( + last_seen, + last_seen_autocrypt, + prefer_encrypted, + public_key, + gossip_timestamp, + gossip_key, + public_key_fingerprint, + gossip_key_fingerprint, + verified_key, + verified_key_fingerprint, + addr) + VALUES (?,?,?,?,?,?,?,?,?,?,?) + ON CONFLICT (addr) + DO UPDATE SET + last_seen = excluded.last_seen, + last_seen_autocrypt = excluded.last_seen_autocrypt, + prefer_encrypted = excluded.prefer_encrypted, + public_key = excluded.public_key, + gossip_timestamp = excluded.gossip_timestamp, + gossip_key = excluded.gossip_key, + public_key_fingerprint = excluded.public_key_fingerprint, + gossip_key_fingerprint = excluded.gossip_key_fingerprint, + verified_key = excluded.verified_key, + verified_key_fingerprint = excluded.verified_key_fingerprint", paramsv![ self.last_seen, self.last_seen_autocrypt, @@ -630,12 +627,7 @@ pub async fn maybe_do_aeap_transition( peerstate.apply_header(header, info.message_time); peerstate.to_save = true; - // We don't know whether a peerstate with this address already existed, or a - // new one should be created, so just try both create=false and create=true, - // and if this fails, create=true, one will succeed (this is a very cold path, - // so performance doesn't really matter). - peerstate.save_to_db(&context.sql, true).await?; - peerstate.save_to_db(&context.sql, false).await?; + peerstate.save_to_db(&context.sql).await?; } } @@ -704,7 +696,7 @@ mod tests { }; assert!( - peerstate.save_to_db(&ctx.ctx.sql, true).await.is_ok(), + peerstate.save_to_db(&ctx.ctx.sql).await.is_ok(), "failed to save to db" ); @@ -746,11 +738,11 @@ mod tests { }; assert!( - peerstate.save_to_db(&ctx.ctx.sql, true).await.is_ok(), + peerstate.save_to_db(&ctx.ctx.sql).await.is_ok(), "failed to save" ); assert!( - peerstate.save_to_db(&ctx.ctx.sql, true).await.is_ok(), + peerstate.save_to_db(&ctx.ctx.sql).await.is_ok(), "double-call with create failed" ); } @@ -779,7 +771,7 @@ mod tests { }; assert!( - peerstate.save_to_db(&ctx.ctx.sql, true).await.is_ok(), + peerstate.save_to_db(&ctx.ctx.sql).await.is_ok(), "failed to save" ); diff --git a/src/qr.rs b/src/qr.rs index 2b6a7954a..05003e7a9 100644 --- a/src/qr.rs +++ b/src/qr.rs @@ -897,7 +897,7 @@ mod tests { fingerprint_changed: false, }; assert!( - peerstate.save_to_db(&ctx.ctx.sql, true).await.is_ok(), + peerstate.save_to_db(&ctx.ctx.sql).await.is_ok(), "failed to save peerstate" ); diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 13241072d..6fbfdddf0 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -2129,7 +2129,7 @@ async fn check_verified_properties( &fp, PeerstateVerifiedStatus::BidirectVerified, ); - peerstate.save_to_db(&context.sql, false).await?; + peerstate.save_to_db(&context.sql).await?; is_verified = true; } } diff --git a/src/securejoin.rs b/src/securejoin.rs index 69ffc0a15..6f977e85a 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -641,10 +641,7 @@ async fn mark_peer_as_verified(context: &Context, fingerprint: &Fingerprint) -> ) { peerstate.prefer_encrypt = EncryptPreference::Mutual; peerstate.to_save = true; - peerstate - .save_to_db(&context.sql, false) - .await - .unwrap_or_default(); + peerstate.save_to_db(&context.sql).await.unwrap_or_default(); return Ok(()); } } @@ -935,7 +932,7 @@ mod tests { to_save: true, fingerprint_changed: false, }; - peerstate.save_to_db(&bob.ctx.sql, true).await?; + peerstate.save_to_db(&bob.ctx.sql).await?; // Step 1: Generate QR-code, ChatId(0) indicates setup-contact let qr = get_securejoin_qr(&alice.ctx, None).await?; diff --git a/src/sql.rs b/src/sql.rs index ce31b5995..3c2d1418d 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -280,7 +280,7 @@ impl Sql { for addr in &addrs { if let Some(ref mut peerstate) = Peerstate::from_addr(context, addr).await? { peerstate.recalc_fingerprint(); - peerstate.save_to_db(self, false).await?; + peerstate.save_to_db(self).await?; } } } diff --git a/src/tests/aeap.rs b/src/tests/aeap.rs index 0f52d4a7e..90c05f715 100644 --- a/src/tests/aeap.rs +++ b/src/tests/aeap.rs @@ -343,7 +343,7 @@ async fn mark_as_verified(this: &TestContext, other: &TestContext) { peerstate.verified_key_fingerprint = peerstate.public_key_fingerprint.clone(); peerstate.to_save = true; - peerstate.save_to_db(&this.sql, false).await.unwrap(); + peerstate.save_to_db(&this.sql).await.unwrap(); } async fn get_last_info_msg(t: &TestContext, chat_id: ChatId) -> Option {