From cb7c8cddfbe9067032e418c919adba0aa9cf843f Mon Sep 17 00:00:00 2001 From: Mike Dilger Date: Tue, 17 Jan 2023 14:56:36 +1300 Subject: [PATCH] Another fix for thread feed (not perfect but better) --- src/comms.rs | 2 +- src/feed.rs | 8 ++--- src/overlord/mod.rs | 77 ++++++++++++++++++++++++--------------------- src/ui/feed.rs | 31 +++++++++++------- src/ui/mod.rs | 4 +-- 5 files changed, 68 insertions(+), 54 deletions(-) diff --git a/src/comms.rs b/src/comms.rs index 200ab873..b4ae9928 100644 --- a/src/comms.rs +++ b/src/comms.rs @@ -22,7 +22,7 @@ pub enum ToOverlordMessage { PushFollow, SaveRelays, SaveSettings, - SetThreadFeed(Id), + SetThreadFeed(Id, Id), Shutdown, UnlockKey(String), UpdateMetadata(PublicKeyHex), diff --git a/src/feed.rs b/src/feed.rs index 727b45af..3737da2b 100644 --- a/src/feed.rs +++ b/src/feed.rs @@ -11,7 +11,7 @@ use tokio::task; pub enum FeedKind { General, Replies, - Thread(Id), + Thread { id: Id, referenced_by: Id }, Person(PublicKeyHex), } @@ -67,14 +67,14 @@ impl Feed { }); } - pub fn set_feed_to_thread(&self, id: Id) { - *self.current_feed_kind.write() = FeedKind::Thread(id); + pub fn set_feed_to_thread(&self, id: Id, referenced_by: Id) { + *self.current_feed_kind.write() = FeedKind::Thread { id, referenced_by }; // Parent starts with the post itself // Overlord will climb it, and recompute will climb it *self.thread_parent.write() = Some(id); let _ = GLOBALS .to_overlord - .send(ToOverlordMessage::SetThreadFeed(id)); + .send(ToOverlordMessage::SetThreadFeed(id, referenced_by)); } pub fn set_feed_to_person(&self, pubkey: PublicKeyHex) { diff --git a/src/overlord/mod.rs b/src/overlord/mod.rs index 4efe2649..95090275 100644 --- a/src/overlord/mod.rs +++ b/src/overlord/mod.rs @@ -497,8 +497,8 @@ impl Overlord { GLOBALS.settings.read().await.save().await?; tracing::debug!("Settings saved."); } - ToOverlordMessage::SetThreadFeed(id) => { - self.set_thread_feed(id).await?; + ToOverlordMessage::SetThreadFeed(id, referenced_by) => { + self.set_thread_feed(id, referenced_by).await?; } ToOverlordMessage::Shutdown => { tracing::info!("Overlord shutting down"); @@ -919,52 +919,56 @@ impl Overlord { Ok(()) } - async fn set_thread_feed(&mut self, id: Id) -> Result<(), Error> { + async fn set_thread_feed(&mut self, id: Id, referenced_by: Id) -> Result<(), Error> { // Cancel current thread subscriptions, if any let _ = self.to_minions.send(ToMinionMessage { target: "all".to_string(), payload: ToMinionPayload::UnsubscribeThreadFeed, }); + // Collect missing ancestors and relays they might be at. + // We will ask all the relays about all the ancestors, which is more than we need to + // but isn't too much to ask for. + let mut missing_ancestors: Vec = Vec::new(); + let mut relays: Vec = Vec::new(); + + // Include the relays where the referenced_by event was seen + relays.extend(DbEventSeen::get_relays_for_event(referenced_by).await?); + relays.extend(DbEventSeen::get_relays_for_event(id).await?); + if relays.is_empty() { + panic!("Our method STILL isn't good enough. We need fallback read relays. I hope this panic does not occur."); + } + // Climb the tree as high as we can, and if there are higher events, // we will ask for those in the initial subscription - let highest_parent_id = match GLOBALS.events.get_highest_local_parent(&id).await? { - Some(id) => id, - None => return Ok(()), // can't do anything - }; + if let Some(highest_parent_id) = GLOBALS.events.get_highest_local_parent(&id).await? { + GLOBALS.feed.set_thread_parent(highest_parent_id); + if let Some(highest_parent) = GLOBALS.events.get_local(highest_parent_id).await? { + for (id, opturl) in highest_parent.replies_to_ancestors() { + missing_ancestors.push(id); + if let Some(url) = opturl { + relays.push(url); + } + } + } + } else { + GLOBALS.feed.set_thread_parent(id); + missing_ancestors.push(id); + } - // Set that in the feed - GLOBALS.feed.set_thread_parent(highest_parent_id); + let missing_ancestors_hex: Vec = + missing_ancestors.iter().map(|id| (*id).into()).collect(); + tracing::debug!("Seeking ancestors {:?}", missing_ancestors_hex); - // get that highest event - let highest_parent = match GLOBALS.events.get_local(highest_parent_id).await? { - Some(event) => event, - None => return Ok(()), // can't do anything - }; - - // strictly speaking, we are only certainly missing the next parent up, we might have - // parents further above. But this isn't asking for much extra. - let mut missing_ancestors: Vec<(Id, Option)> = highest_parent.replies_to_ancestors(); - let missing_ids: Vec = missing_ancestors.iter().map(|(id, _)| *id).collect(); - let missing_ids_hex: Vec = missing_ids.iter().map(|id| (*id).into()).collect(); - tracing::debug!("Seeking ancestors {:?}", missing_ids_hex); - - // Determine which relays to subscribe on - // (everywhere the main event was seen, and all relays suggested in the 'e' tags) - let mut relay_urls = DbEventSeen::get_relays_for_event(id).await?; - let suggested_urls: Vec = missing_ancestors - .drain(..) - .filter_map(|(_, opturl)| opturl) - .collect(); - relay_urls.extend(suggested_urls); - relay_urls = relay_urls + // Clean up relays + relays = relays .drain(..) .filter(|u| u.is_valid_relay_url()) .collect(); - relay_urls.sort(); - relay_urls.dedup(); + relays.sort(); + relays.dedup(); - for url in relay_urls.iter() { + for url in relays.iter() { // Start minion if needed if !GLOBALS.relays_watching.read().await.contains(url) { self.start_minion(url.inner().to_string()).await?; @@ -973,7 +977,10 @@ impl Overlord { // Subscribe let _ = self.to_minions.send(ToMinionMessage { target: url.inner().to_string(), - payload: ToMinionPayload::SubscribeThreadFeed(id.into(), missing_ids_hex.clone()), + payload: ToMinionPayload::SubscribeThreadFeed( + id.into(), + missing_ancestors_hex.clone(), + ), }); } diff --git a/src/ui/feed.rs b/src/ui/feed.rs index 4415161e..0956b1b3 100644 --- a/src/ui/feed.rs +++ b/src/ui/feed.rs @@ -44,7 +44,7 @@ pub(super) fn update(app: &mut GossipUi, ctx: &Context, frame: &mut eframe::Fram { app.set_page(Page::Feed(FeedKind::Replies)); } - if matches!(feed_kind.clone(), FeedKind::Thread(..)) { + if matches!(feed_kind.clone(), FeedKind::Thread { .. }) { ui.separator(); ui.selectable_value(&mut app.page, Page::Feed(feed_kind.clone()), "Thread"); GLOBALS.events.clear_new(); @@ -79,7 +79,7 @@ pub(super) fn update(app: &mut GossipUi, ctx: &Context, frame: &mut eframe::Fram let feed = GLOBALS.feed.get_replies(); render_a_feed(app, ctx, frame, ui, feed, true); } - FeedKind::Thread(_id) => { + FeedKind::Thread { .. } => { if let Some(parent) = GLOBALS.feed.get_thread_parent() { render_a_feed(app, ctx, frame, ui, vec![parent], true); } @@ -416,7 +416,7 @@ fn render_post_actual( let is_main_event: bool = { let feed_kind = GLOBALS.feed.get_feed_kind(); match feed_kind { - FeedKind::Thread(id) => id == event.id, + FeedKind::Thread { id, .. } => id == event.id, _ => false, } }; @@ -467,12 +467,10 @@ fn render_post_actual( let idhex: IdHex = irt.into(); let nam = format!("replies to #{}", GossipUi::hex_id_short(&idhex)); if ui.link(&nam).clicked() { - // NOTE - We don't set the thread to the thing it replies to, - // but to the child that has the link. This is because - // the parent might not exist and that would leave us - // stranded, but we KNOW the child exists, and it's - // ancestors will render when they become available. - app.set_page(Page::Feed(FeedKind::Thread(event.id))); + app.set_page(Page::Feed(FeedKind::Thread { + id: irt, + referenced_by: event.id, + })); }; ui.reset_style(); } @@ -486,7 +484,10 @@ fn render_post_actual( ui.with_layout(Layout::right_to_left(Align::TOP), |ui| { ui.menu_button(RichText::new("≡").size(28.0), |ui| { if !is_main_event && ui.button("View Thread").clicked() { - app.set_page(Page::Feed(FeedKind::Thread(event.id))); + app.set_page(Page::Feed(FeedKind::Thread { + id: event.id, + referenced_by: event.id, + })); } if ui.button("Copy ID").clicked() { ui.output().copied_text = event.id.as_hex_string(); @@ -503,7 +504,10 @@ fn render_post_actual( if !is_main_event && ui.button("➤").on_hover_text("View Thread").clicked() { - app.set_page(Page::Feed(FeedKind::Thread(event.id))); + app.set_page(Page::Feed(FeedKind::Thread { + id: event.id, + referenced_by: event.id, + })); } ui.label( @@ -636,7 +640,10 @@ fn render_content(app: &mut GossipUi, ui: &mut Ui, tag_re: ®ex::Regex, event: let idhex: IdHex = (*id).into(); let nam = format!("#{}", GossipUi::hex_id_short(&idhex)); if ui.link(&nam).clicked() { - app.set_page(Page::Feed(FeedKind::Thread(*id))); + app.set_page(Page::Feed(FeedKind::Thread { + id: *id, + referenced_by: event.id, + })); }; } Tag::Hashtag(s) => { diff --git a/src/ui/mod.rs b/src/ui/mod.rs index c7595753..3a8ea0d9 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -222,8 +222,8 @@ impl GossipUi { GLOBALS.feed.set_feed_to_replies(); GLOBALS.events.clear_new(); } - Page::Feed(FeedKind::Thread(id)) => { - GLOBALS.feed.set_feed_to_thread(*id); + Page::Feed(FeedKind::Thread { id, referenced_by }) => { + GLOBALS.feed.set_feed_to_thread(*id, *referenced_by); } Page::Feed(FeedKind::Person(pubkey)) => { GLOBALS.feed.set_feed_to_person(pubkey.to_owned());