From e8c0b903a83d3e9157ab9c5c4f25583ba6b11b12 Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Thu, 6 Feb 2025 10:01:23 -0800 Subject: [PATCH 1/4] drive-by compiler warning fixes --- crates/notedeck_chrome/src/notedeck.rs | 2 +- crates/notedeck_columns/src/timeline/route.rs | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/notedeck_chrome/src/notedeck.rs b/crates/notedeck_chrome/src/notedeck.rs index 0ca84366..d9b3894c 100644 --- a/crates/notedeck_chrome/src/notedeck.rs +++ b/crates/notedeck_chrome/src/notedeck.rs @@ -194,7 +194,7 @@ mod tests { .timeline_id() .unwrap(); - let timelines = app.timeline_cache.timelines.len() == 2; + assert_eq!(app.timeline_cache.timelines.len(), 2); assert!(app.timeline_cache.timelines.get(&tl1).is_some()); assert!(app.timeline_cache.timelines.get(&tl2).is_some()); diff --git a/crates/notedeck_columns/src/timeline/route.rs b/crates/notedeck_columns/src/timeline/route.rs index ecbb595b..76bd13c0 100644 --- a/crates/notedeck_columns/src/timeline/route.rs +++ b/crates/notedeck_columns/src/timeline/route.rs @@ -141,7 +141,7 @@ pub fn render_profile_route( #[cfg(test)] mod tests { use enostr::NoteId; - use tokenator::{TokenParser, TokenSerializable, TokenWriter}; + use tokenator::{TokenParser, TokenWriter}; use crate::timeline::{ThreadSelection, TimelineKind}; use enostr::Pubkey; @@ -149,8 +149,6 @@ mod tests { #[test] fn test_timeline_route_serialize() { - use super::TimelineKind; - let note_id_hex = "1c54e5b0c386425f7e017d9e068ddef8962eb2ce1bb08ed27e24b93411c12e60"; let note_id = NoteId::from_hex(note_id_hex).unwrap(); let data_str = format!("thread:{}", note_id_hex); From 2fcfed4dd5cbbad940bc007a6a24bec4511f4cf7 Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Thu, 23 Jan 2025 13:20:05 -0800 Subject: [PATCH 2/4] improve debug logging, comments, and variable names for clarity --- crates/notedeck/src/accounts.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/crates/notedeck/src/accounts.rs b/crates/notedeck/src/accounts.rs index efd7636d..818874e0 100644 --- a/crates/notedeck/src/accounts.rs +++ b/crates/notedeck/src/accounts.rs @@ -481,7 +481,8 @@ impl Accounts { } } - // Returns added and removed accounts + // Return accounts which have no account_data yet (added) and accounts + // which have still data but are no longer in our account list (removed). fn delta_accounts(&self) -> (Vec<[u8; 32]>, Vec<[u8; 32]>) { let mut added = Vec::new(); for pubkey in self.accounts.iter().map(|a| a.pubkey.bytes()) { @@ -552,8 +553,9 @@ impl Accounts { wakeup: impl Fn() + Send + Sync + Clone + 'static, ) { debug!( - "updating relay configuration for currently selected account {:?}", + "updating relay configuration for currently selected {:?}", self.currently_selected_account + .map(|i| hex::encode(self.accounts.get(i).unwrap().pubkey.bytes())) ); // If forced relays are set use them only @@ -605,7 +607,7 @@ impl Accounts { // make sure it is fast when idle // On the initial update the relays need config even if nothing changes below - let mut relays_changed = self.needs_relay_config; + let mut need_reconfig = self.needs_relay_config; let ctx2 = ctx.clone(); let wakeup = move || { @@ -616,18 +618,18 @@ impl Accounts { let (added, removed) = self.delta_accounts(); for pk in added { self.handle_added_account(ndb, pool, &pk); - relays_changed = true; + need_reconfig = true; } for pk in removed { self.handle_removed_account(&pk); - relays_changed = true; + need_reconfig = true; } // Did any accounts receive updates (ie NIP-65 relay lists) - relays_changed = self.poll_for_updates(ndb) || relays_changed; + need_reconfig = self.poll_for_updates(ndb) || need_reconfig; // If needed, update the relay configuration - if relays_changed { + if need_reconfig { self.update_relay_configuration(pool, wakeup); self.needs_relay_config = false; } From 9ce4c2891e23ce77bd8835d57eb53d4a818a57f8 Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Thu, 23 Jan 2025 15:58:11 -0800 Subject: [PATCH 3/4] explicitly activate and deactivate account relay list subs - ensures only one active at a time - stops leaking relay list subs --- crates/notedeck/src/accounts.rs | 104 +++++++++++++++++++++++--------- crates/notedeck/src/app.rs | 2 +- 2 files changed, 77 insertions(+), 29 deletions(-) diff --git a/crates/notedeck/src/accounts.rs b/crates/notedeck/src/accounts.rs index 818874e0..a44f0caa 100644 --- a/crates/notedeck/src/accounts.rs +++ b/crates/notedeck/src/accounts.rs @@ -37,7 +37,7 @@ pub enum AccountsAction { pub struct AccountRelayData { filter: Filter, - subid: String, + subid: Option, sub: Option, local: BTreeSet, // used locally but not advertised advertised: BTreeSet, // advertised via NIP-65 @@ -56,7 +56,7 @@ pub struct AddAccountAction { } impl AccountRelayData { - pub fn new(ndb: &Ndb, pool: &mut RelayPool, pubkey: &[u8; 32]) -> Self { + pub fn new(ndb: &Ndb, pubkey: &[u8; 32]) -> Self { // Construct a filter for the user's NIP-65 relay list let filter = Filter::new() .authors([pubkey]) @@ -64,11 +64,6 @@ impl AccountRelayData { .limit(1) .build(); - // Local ndb subscription - let ndbsub = ndb - .subscribe(&[filter.clone()]) - .expect("ndb relay list subscription"); - // Query the ndb immediately to see if the user list is already there let txn = Transaction::new(ndb).expect("transaction"); let lim = filter.limit().unwrap_or(crate::filter::default_limit()) as i32; @@ -85,21 +80,51 @@ impl AccountRelayData { relays ); - // Id for future remote relay subscriptions - let subid = Uuid::new_v4().to_string(); - - // Add remote subscription to existing relays - pool.subscribe(subid.clone(), vec![filter.clone()]); - AccountRelayData { filter, - subid, - sub: Some(ndbsub), + subid: None, + sub: None, local: BTreeSet::new(), advertised: relays.into_iter().collect(), } } + // make this account the current selected account + pub fn activate(&mut self, ndb: &Ndb, pool: &mut RelayPool) { + debug!("activating relay sub {}", self.filter.json().unwrap()); + assert_eq!(self.subid, None, "subid already exists"); + assert_eq!(self.sub, None, "sub already exists"); + + // local subscription + let sub = ndb + .subscribe(&[self.filter.clone()]) + .expect("ndb relay list subscription"); + + // remote subscription + let subid = Uuid::new_v4().to_string(); + pool.subscribe(subid.clone(), vec![self.filter.clone()]); + + self.sub = Some(sub); + self.subid = Some(subid); + } + + // this account is no longer the selected account + pub fn deactivate(&mut self, ndb: &mut Ndb, pool: &mut RelayPool) { + debug!("deactivating relay sub {}", self.filter.json().unwrap()); + assert_ne!(self.subid, None, "subid doesn't exist"); + assert_ne!(self.sub, None, "sub doesn't exist"); + + // remote subscription + pool.unsubscribe(self.subid.as_ref().unwrap().clone()); + + // local subscription + ndb.unsubscribe(self.sub.unwrap()) + .expect("ndb relay list unsubscribe"); + + self.sub = None; + self.subid = None; + } + // standardize the format (ie, trailing slashes) to avoid dups pub fn canonicalize_url(url: &str) -> String { match Url::parse(url) { @@ -432,13 +457,12 @@ impl Accounts { } } - pub fn get_selected_account_data(&self) -> Option<&AccountData> { - if let Some(account) = self.get_selected_account() { - if let Some(account_data) = self.account_data.get(account.pubkey.bytes()) { - return Some(account_data); - } - } - None + pub fn get_selected_account_data(&mut self) -> Option<&mut AccountData> { + let account_pubkey = { + let account = self.get_selected_account()?; + *account.pubkey.bytes() + }; + self.account_data.get_mut(&account_pubkey) } pub fn select_account(&mut self, index: usize) { @@ -470,10 +494,13 @@ impl Accounts { pub fn send_initial_filters(&mut self, pool: &mut RelayPool, relay_url: &str) { for data in self.account_data.values() { - pool.send_to( - &ClientMessage::req(data.relay.subid.clone(), vec![data.relay.filter.clone()]), - relay_url, - ); + // send the active account's relay list subscription + if let Some(relay_subid) = &data.relay.subid { + pool.send_to( + &ClientMessage::req(relay_subid.clone(), vec![data.relay.filter.clone()]), + relay_url, + ); + } pool.send_to( &ClientMessage::req(data.muted.subid.clone(), vec![data.muted.filter.clone()]), relay_url, @@ -504,7 +531,7 @@ impl Accounts { // Create the user account data let new_account_data = AccountData { - relay: AccountRelayData::new(ndb, pool, pubkey), + relay: AccountRelayData::new(ndb, pubkey), muted: AccountMutedData::new(ndb, pool, pubkey), }; self.account_data.insert(*pubkey, new_account_data); @@ -602,7 +629,7 @@ impl Accounts { debug!("current relays: {:?}", pool.urls()); } - pub fn update(&mut self, ndb: &Ndb, pool: &mut RelayPool, ctx: &egui::Context) { + pub fn update(&mut self, ndb: &mut Ndb, pool: &mut RelayPool, ctx: &egui::Context) { // IMPORTANT - This function is called in the UI update loop, // make sure it is fast when idle @@ -614,6 +641,19 @@ impl Accounts { ctx2.request_repaint(); }; + // Do we need to deactivate any existing account subs? + for (ndx, account) in self.accounts.iter().enumerate() { + if Some(ndx) != self.currently_selected_account { + // this account is not currently selected + if let Some(data) = self.account_data.get_mut(account.pubkey.bytes()) { + if data.relay.sub.is_some() { + // this account has subs, deactivate them + data.relay.deactivate(ndb, pool); + } + } + } + } + // Were any accounts added or removed? let (added, removed) = self.delta_accounts(); for pk in added { @@ -633,6 +673,14 @@ impl Accounts { self.update_relay_configuration(pool, wakeup); self.needs_relay_config = false; } + + // Do we need to activate account subs? + if let Some(data) = self.get_selected_account_data() { + if data.relay.sub.is_none() { + // the currently selected account doesn't have subs, activate them + data.relay.activate(ndb, pool); + } + } } pub fn get_full<'a>(&'a self, pubkey: &[u8; 32]) -> Option> { diff --git a/crates/notedeck/src/app.rs b/crates/notedeck/src/app.rs index c4169fce..4f34ba35 100644 --- a/crates/notedeck/src/app.rs +++ b/crates/notedeck/src/app.rs @@ -69,7 +69,7 @@ impl eframe::App for Notedeck { puffin::GlobalProfiler::lock().new_frame(); // handle account updates - self.accounts.update(&self.ndb, &mut self.pool, ctx); + self.accounts.update(&mut self.ndb, &mut self.pool, ctx); main_panel(&ctx.style(), crate::ui::is_narrow(ctx)).show(ctx, |ui| { // render app From 38da6c9eafbfb0093475cf1fdc91b6751dbe47f8 Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Thu, 30 Jan 2025 16:55:51 -0800 Subject: [PATCH 4/4] explicitly activate and deactivate account muted list subs This is the same treatment to muted as applied to relay lists in #678 --- crates/notedeck/src/accounts.rs | 84 +++++++++++++++++++++++---------- 1 file changed, 60 insertions(+), 24 deletions(-) diff --git a/crates/notedeck/src/accounts.rs b/crates/notedeck/src/accounts.rs index a44f0caa..dae29a80 100644 --- a/crates/notedeck/src/accounts.rs +++ b/crates/notedeck/src/accounts.rs @@ -187,13 +187,13 @@ impl AccountRelayData { pub struct AccountMutedData { filter: Filter, - subid: String, + subid: Option, sub: Option, muted: Arc, } impl AccountMutedData { - pub fn new(ndb: &Ndb, pool: &mut RelayPool, pubkey: &[u8; 32]) -> Self { + pub fn new(ndb: &Ndb, pubkey: &[u8; 32]) -> Self { // Construct a filter for the user's NIP-51 muted list let filter = Filter::new() .authors([pubkey]) @@ -201,11 +201,6 @@ impl AccountMutedData { .limit(1) .build(); - // Local ndb subscription - let ndbsub = ndb - .subscribe(&[filter.clone()]) - .expect("ndb muted subscription"); - // Query the ndb immediately to see if the user's muted list is already there let txn = Transaction::new(ndb).expect("transaction"); let lim = filter.limit().unwrap_or(crate::filter::default_limit()) as i32; @@ -218,20 +213,50 @@ impl AccountMutedData { let muted = Self::harvest_nip51_muted(ndb, &txn, &nks); debug!("pubkey {}: initial muted {:?}", hex::encode(pubkey), muted); - // Id for future remote relay subscriptions - let subid = Uuid::new_v4().to_string(); - - // Add remote subscription to existing relays - pool.subscribe(subid.clone(), vec![filter.clone()]); - AccountMutedData { filter, - subid, - sub: Some(ndbsub), + subid: None, + sub: None, muted: Arc::new(muted), } } + // make this account the current selected account + pub fn activate(&mut self, ndb: &Ndb, pool: &mut RelayPool) { + debug!("activating muted sub {}", self.filter.json().unwrap()); + assert_eq!(self.subid, None, "subid already exists"); + assert_eq!(self.sub, None, "sub already exists"); + + // local subscription + let sub = ndb + .subscribe(&[self.filter.clone()]) + .expect("ndb muted subscription"); + + // remote subscription + let subid = Uuid::new_v4().to_string(); + pool.subscribe(subid.clone(), vec![self.filter.clone()]); + + self.sub = Some(sub); + self.subid = Some(subid); + } + + // this account is no longer the selected account + pub fn deactivate(&mut self, ndb: &mut Ndb, pool: &mut RelayPool) { + debug!("deactivating muted sub {}", self.filter.json().unwrap()); + assert_ne!(self.subid, None, "subid doesn't exist"); + assert_ne!(self.sub, None, "sub doesn't exist"); + + // remote subscription + pool.unsubscribe(self.subid.as_ref().unwrap().clone()); + + // local subscription + ndb.unsubscribe(self.sub.unwrap()) + .expect("ndb muted unsubscribe"); + + self.sub = None; + self.subid = None; + } + fn harvest_nip51_muted(ndb: &Ndb, txn: &Transaction, nks: &[NoteKey]) -> Muted { let mut muted = Muted::default(); for nk in nks.iter() { @@ -501,10 +526,13 @@ impl Accounts { relay_url, ); } - pool.send_to( - &ClientMessage::req(data.muted.subid.clone(), vec![data.muted.filter.clone()]), - relay_url, - ); + // send the active account's muted subscription + if let Some(muted_subid) = &data.muted.subid { + pool.send_to( + &ClientMessage::req(muted_subid.clone(), vec![data.muted.filter.clone()]), + relay_url, + ); + } } } @@ -526,13 +554,13 @@ impl Accounts { (added, removed) } - fn handle_added_account(&mut self, ndb: &Ndb, pool: &mut RelayPool, pubkey: &[u8; 32]) { + fn handle_added_account(&mut self, ndb: &Ndb, pubkey: &[u8; 32]) { debug!("handle_added_account {}", hex::encode(pubkey)); // Create the user account data let new_account_data = AccountData { relay: AccountRelayData::new(ndb, pubkey), - muted: AccountMutedData::new(ndb, pool, pubkey), + muted: AccountMutedData::new(ndb, pubkey), }; self.account_data.insert(*pubkey, new_account_data); } @@ -647,9 +675,13 @@ impl Accounts { // this account is not currently selected if let Some(data) = self.account_data.get_mut(account.pubkey.bytes()) { if data.relay.sub.is_some() { - // this account has subs, deactivate them + // this account has relay subs, deactivate them data.relay.deactivate(ndb, pool); } + if data.muted.sub.is_some() { + // this account has muted subs, deactivate them + data.muted.deactivate(ndb, pool); + } } } } @@ -657,7 +689,7 @@ impl Accounts { // Were any accounts added or removed? let (added, removed) = self.delta_accounts(); for pk in added { - self.handle_added_account(ndb, pool, &pk); + self.handle_added_account(ndb, &pk); need_reconfig = true; } for pk in removed { @@ -677,9 +709,13 @@ impl Accounts { // Do we need to activate account subs? if let Some(data) = self.get_selected_account_data() { if data.relay.sub.is_none() { - // the currently selected account doesn't have subs, activate them + // the currently selected account doesn't have relay subs, activate them data.relay.activate(ndb, pool); } + if data.muted.sub.is_none() { + // the currently selected account doesn't have muted subs, activate them + data.muted.activate(ndb, pool); + } } }