diff --git a/crates/chatd/src/adapters/discord/members.rs b/crates/chatd/src/adapters/discord/members.rs index 2ecaea1..0129f1a 100644 --- a/crates/chatd/src/adapters/discord/members.rs +++ b/crates/chatd/src/adapters/discord/members.rs @@ -300,6 +300,23 @@ impl Discord { } watched.seen = watching.clone(); } + self.place(watching); + } + + /// The screen the shell last named, placed again now that READY has said what its servers + /// and channels are: a Watch that came before READY found none of them, and the same Watch + /// again is not acted on. + pub(super) fn rewatch(&self) { + let seen = self.state.lock().unwrap().members.seen.clone(); + if *seen == Watching::default() { + // Nothing named; a Space opened without a Watch stays as it is. + return self.sync_subscription(); + } + self.place(&seen); + } + + /// Works out what `watching` puts on screen and subscribes it. + fn place(&self, watching: &Arc) { let ours = |key: &protocol::Key| key.network == Network::Discord && key.account == self.ctx.account.account; let open = watching.conversation.as_ref().filter(|key| ours(key)); let conversation = open.and_then(|key| self.ctx.known(&key.id).ok().flatten()); @@ -308,6 +325,10 @@ impl Discord { { let mut state = self.state.lock().unwrap(); let state = &mut *state; + // A newer Watch came meanwhile on the other thread; it places itself. + if !Arc::ptr_eq(&state.members.seen, watching) { + return; + } // The tree on screen is named outright; an open conversation says it too. let named = watching.space.as_ref().map(|space| ours(space).then(|| space.id.clone())); let guild = guild.filter(|guild| state.guilds.contains_key(guild)); diff --git a/crates/chatd/src/adapters/discord/ready.rs b/crates/chatd/src/adapters/discord/ready.rs index 8da5d93..d4de1e7 100644 --- a/crates/chatd/src/adapters/discord/ready.rs +++ b/crates/chatd/src/adapters/discord/ready.rs @@ -829,8 +829,9 @@ impl Discord { for (_, id) in warm { let _ = self.ctx.commands.send(Command::Warm(id)); } - // The Space on screen is subscribed again on the new session. - self.sync_subscription(); + // The Space on screen is subscribed again on the new session, placed against the + // servers just read, so a Watch sent while connecting is not left unsubscribed. + self.rewatch(); Ok(()) } diff --git a/crates/chatd/tests/discord.rs b/crates/chatd/tests/discord.rs index 7fdb491..8ee83b3 100644 --- a/crates/chatd/tests/discord.rs +++ b/crates/chatd/tests/discord.rs @@ -99,6 +99,8 @@ struct Shared { forbidden: Mutex>, /// What an identify is answered with: READY as text. ready: Mutex, + /// How long after an identify READY is held back. + ready_after: Mutex, /// Close codes the next identifies are answered with instead. closes: Mutex>, /// The page that names the web client's build: how long it takes, and its status. @@ -467,6 +469,7 @@ fn serve_gateway(shared: &Shared, stream: TcpStream, control: &Receiver while Instant::now() < until && !matches!(socket.read(), Err(tungstenite::Error::ConnectionClosed)) {} return; } + sleep(*shared.ready_after.lock().unwrap()); // Written out as text, so a READY of real size is sent as it is. let ready = shared.ready.lock().unwrap().replace("{{GATEWAY}}", gateway); let s = shared.sequence.fetch_add(1, Ordering::Relaxed) + 1; @@ -581,6 +584,7 @@ impl World { raw: Mutex::new(HashMap::new()), forbidden: Mutex::new(Vec::new()), ready: Mutex::new(READY.to_owned()), + ready_after: Mutex::new(Duration::ZERO), closes: Mutex::new(VecDeque::new()), login: Mutex::new((Duration::ZERO, 200)), logins: AtomicUsize::new(0), @@ -1458,6 +1462,30 @@ fn missing_member_pages_never_block_a_new_viewport_or_move_its_subscription() { assert_eq!(world.discord.frames_of(37).len(), before, "cached reads and listed presence must not spend subscription budget"); } +#[test] +fn member_viewport_selected_before_ready_is_subscribed_after_connect() { + let world = World::launch(&[], |shared| *shared.ready_after.lock().unwrap() = Duration::from_millis(1500)); + wait("the identify", || world.discord.frames_of(2).pop()); + // The shell moves twice while READY is on its way: the second screen is the one it shows. + let watch = |channel: &str| Watching { space: Some(world.key("4000")), conversation: Some(world.key(channel)), members: Some((0, 50)), ..Watching::default() }; + world.ask(Request::Watch(watch("4101"))); + world.ask(Request::Watch(watch("4105"))); + assert_ne!(world.link(), Link::Connected, "READY came before the Watch"); + assert!(world.discord.frames_of(37).is_empty(), "nothing can be subscribed before READY"); + wait("the account to connect", || (world.link() == Link::Connected).then_some(())); + + // The shell's next Watch is the same one, so it is READY that subscribes the window. + world.ask(Request::Watch(watch("4105"))); + let subscribed = wait("the member window", || world.discord.frames_of(37).pop().filter(|frame| + frame["d"]["subscriptions"]["4000"]["channels"]["4105"].is_array())); + assert_eq!(subscribed["d"]["subscriptions"]["4000"]["channels"], json!({"4105": [[0, 99]]})); + let earlier = world.discord.frames_of(37).iter().any(|frame| frame["d"]["subscriptions"]["4000"]["channels"]["4101"].is_array()); + assert!(!earlier, "the screen left while connecting was subscribed"); + // Until Discord answers, the list is pending rather than empty. + let pending = world.ask(Request::Members { conversation: world.key("4105"), offset: 0, limit: 50 }); + assert!(matches!(pending, Response::Members { pending: true, ref members, .. } if members.is_empty()), "{pending:?}"); +} + #[test] fn servers_channels_people_and_settings_follow_live_events() { let world = World::start();