From 05b72cbee358f1490ab49f8385a766ff3173e02a Mon Sep 17 00:00:00 2001 From: Salientekill Date: Fri, 19 Jun 2026 21:50:24 +0000 Subject: [PATCH 1/4] feat(groups): backfill participant phone_number from LID-PN mapping The server often omits the phone_number attribute on nodes of LID-addressed groups, leaving GroupMetadata LID-only. Consumers that cross-reference data keyed by PN then treat current members as absent. get_participating/get_metadata now backfill each LID participant's phone_number from the persisted lid_pn_mapping the client already learned (single backend load + in-memory join, only when a LID-addressed group has a participant missing its PN). --- src/features/groups.rs | 126 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 122 insertions(+), 4 deletions(-) diff --git a/src/features/groups.rs b/src/features/groups.rs index 5ee60dcf0..1d6889e4a 100644 --- a/src/features/groups.rs +++ b/src/features/groups.rs @@ -237,6 +237,23 @@ pub struct Groups<'a> { client: &'a Client, } +/// Fills each LID participant's `phone_number` from a preloaded +/// `LID user-part → PN user-part` map. No-op outside LID-addressed groups or +/// when the participant already has a PN. +fn fill_participant_pns(meta: &mut GroupMetadata, lid_to_pn: &HashMap) { + if meta.addressing_mode != AddressingMode::Lid { + return; + } + for p in meta.participants.iter_mut() { + if p.phone_number.is_none() + && p.jid.is_lid() + && let Some(pn) = lid_to_pn.get(p.jid.user.as_str()) + { + p.phone_number = Some(Jid::pn(pn.as_str())); + } + } +} + impl<'a> Groups<'a> { pub(crate) fn new(client: &'a Client) -> Self { Self { client } @@ -348,26 +365,80 @@ impl<'a> Groups<'a> { Ok(info) } + /// Loads the full `LID user-part → PN user-part` map from the backend. + /// Empty on error/absence so the caller degrades to PN-less metadata. + async fn load_lid_pn_map(&self) -> HashMap { + match self + .client + .persistence_manager + .backend() + .get_all_lid_mappings() + .await + { + Ok(entries) => entries + .into_iter() + .map(|e| (e.lid, e.phone_number)) + .collect(), + Err(e) => { + log::warn!("load_lid_pn_map: get_all_lid_mappings failed: {e}"); + HashMap::new() + } + } + } + + /// True if any metadata needs PN backfill: a LID-addressed group with a LID + /// participant whose `phone_number` is absent (server omitted it in the stanza). + fn needs_pn_fill<'m>(metas: impl Iterator) -> bool { + metas.into_iter().any(|m| { + m.addressing_mode == AddressingMode::Lid + && m.participants + .iter() + .any(|p| p.phone_number.is_none() && p.jid.is_lid()) + }) + } + pub async fn get_participating(&self) -> Result, GroupError> { let response = self.client.execute(GroupParticipatingIq::new()).await?; - let result = response + let mut result: HashMap = response .groups .into_iter() .map(|group| { let key = group.id.clone(); - let metadata = GroupMetadata::from(group); - (key, metadata) + (key, GroupMetadata::from(group)) }) .collect(); + // The server often omits `phone_number` on `` nodes of + // LID-addressed groups, so consumers that key data by PN would treat + // current members as absent. Backfill from the persisted mapping the lib + // already learned (one load + join), instead of forcing each consumer to + // re-resolve LID→PN themselves. + if Self::needs_pn_fill(result.values()) { + let lid_to_pn = self.load_lid_pn_map().await; + if !lid_to_pn.is_empty() { + for meta in result.values_mut() { + fill_participant_pns(meta, &lid_to_pn); + } + } + } + Ok(result) } pub async fn get_metadata(&self, jid: &Jid) -> Result { // No phash is sent, so the server always returns the full group. match self.client.execute(GroupQueryIq::new(jid)).await? { - GroupInfoOutcome::Full(group) => Ok(GroupMetadata::from(*group)), + GroupInfoOutcome::Full(group) => { + let mut meta = GroupMetadata::from(*group); + if Self::needs_pn_fill(std::iter::once(&meta)) { + let lid_to_pn = self.load_lid_pn_map().await; + if !lid_to_pn.is_empty() { + fill_participant_pns(&mut meta, &lid_to_pn); + } + } + Ok(meta) + } GroupInfoOutcome::NotModified => Err(GroupError::InvalidRequest( "group query returned not-modified without a phash".into(), )), @@ -1211,6 +1282,53 @@ mod tests { assert!(!metadata.participants[0].is_super_admin()); } + #[test] + fn fill_participant_pns_backfills_lid_from_mapping() { + use std::collections::HashMap; + use wacore_binary::jid::{Jid, Server}; + + let lid = Jid::new("26263000000099", Server::Lid); + let mut meta = GroupMetadata { + id: "120399@g.us".parse().unwrap(), + participants: vec![GroupParticipant { + jid: lid, + phone_number: None, + participant_type: ParticipantType::Member, + }], + addressing_mode: AddressingMode::Lid, + ..Default::default() + }; + let map = HashMap::from([("26263000000099".to_string(), "5521900000099".to_string())]); + fill_participant_pns(&mut meta, &map); + assert_eq!( + meta.participants[0].phone_number, + Some(Jid::pn("5521900000099")), + "LID participant should receive phone_number from the mapping" + ); + } + + #[test] + fn fill_participant_pns_noop_in_pn_group() { + use std::collections::HashMap; + use wacore_binary::jid::{Jid, Server}; + + // PN-addressed group: untouched (jid already is the PN). + let pn = Jid::new("5521900000098", Server::Pn); + let mut meta = GroupMetadata { + id: "120398@g.us".parse().unwrap(), + participants: vec![GroupParticipant { + jid: pn, + phone_number: None, + participant_type: ParticipantType::Member, + }], + addressing_mode: AddressingMode::Pn, + ..Default::default() + }; + let map = HashMap::from([("5521900000098".to_string(), "5521900000098".to_string())]); + fill_participant_pns(&mut meta, &map); + assert_eq!(meta.participants[0].phone_number, None); + } + #[test] fn test_extract_invite_code() { // Pattern 3: most common From 6a70fadfa7438805797e58eb39b99da5af31244c Mon Sep 17 00:00:00 2001 From: Salientekill Date: Fri, 19 Jun 2026 22:54:13 +0000 Subject: [PATCH 2/4] refactor(groups): resolve participant PN via lid_pn_cache, not full table load Per review: instead of reloading the whole lid_pn_mapping table on every call, look each LID participant up through the client's warm in-memory cache via get_lid_pn_entry (same path create_group uses). Cheaper (only touches participants that need it) and fresher (picks up offline-learned mappings the detached DB persist may lag). Drops load_lid_pn_map/needs_pn_fill; the addressing_mode guard stays inside fill_participant_pns. Tests now go through a client with a warmed mapping. --- src/features/groups.rs | 118 ++++++++++++++--------------------------- 1 file changed, 40 insertions(+), 78 deletions(-) diff --git a/src/features/groups.rs b/src/features/groups.rs index 1d6889e4a..4dac43586 100644 --- a/src/features/groups.rs +++ b/src/features/groups.rs @@ -237,23 +237,6 @@ pub struct Groups<'a> { client: &'a Client, } -/// Fills each LID participant's `phone_number` from a preloaded -/// `LID user-part → PN user-part` map. No-op outside LID-addressed groups or -/// when the participant already has a PN. -fn fill_participant_pns(meta: &mut GroupMetadata, lid_to_pn: &HashMap) { - if meta.addressing_mode != AddressingMode::Lid { - return; - } - for p in meta.participants.iter_mut() { - if p.phone_number.is_none() - && p.jid.is_lid() - && let Some(pn) = lid_to_pn.get(p.jid.user.as_str()) - { - p.phone_number = Some(Jid::pn(pn.as_str())); - } - } -} - impl<'a> Groups<'a> { pub(crate) fn new(client: &'a Client) -> Self { Self { client } @@ -365,38 +348,26 @@ impl<'a> Groups<'a> { Ok(info) } - /// Loads the full `LID user-part → PN user-part` map from the backend. - /// Empty on error/absence so the caller degrades to PN-less metadata. - async fn load_lid_pn_map(&self) -> HashMap { - match self - .client - .persistence_manager - .backend() - .get_all_lid_mappings() - .await - { - Ok(entries) => entries - .into_iter() - .map(|e| (e.lid, e.phone_number)) - .collect(), - Err(e) => { - log::warn!("load_lid_pn_map: get_all_lid_mappings failed: {e}"); - HashMap::new() + /// Backfills each LID participant's `phone_number` from the client's LID-PN + /// cache (`get_lid_pn_entry`, same warm-cache + backend path `create_group` + /// uses). The server often omits the attribute on `` nodes of + /// LID-addressed groups, so consumers keying data by PN would treat current + /// members as absent. No-op outside LID-addressed groups or when the PN is + /// already present; unknown mappings leave the participant untouched. + async fn fill_participant_pns(&self, meta: &mut GroupMetadata) { + if meta.addressing_mode != AddressingMode::Lid { + return; + } + for p in meta.participants.iter_mut() { + if p.phone_number.is_none() + && p.jid.is_lid() + && let Ok(Some(entry)) = self.client.get_lid_pn_entry(&p.jid).await + { + p.phone_number = Some(Jid::pn(&*entry.phone_number)); } } } - /// True if any metadata needs PN backfill: a LID-addressed group with a LID - /// participant whose `phone_number` is absent (server omitted it in the stanza). - fn needs_pn_fill<'m>(metas: impl Iterator) -> bool { - metas.into_iter().any(|m| { - m.addressing_mode == AddressingMode::Lid - && m.participants - .iter() - .any(|p| p.phone_number.is_none() && p.jid.is_lid()) - }) - } - pub async fn get_participating(&self) -> Result, GroupError> { let response = self.client.execute(GroupParticipatingIq::new()).await?; @@ -409,18 +380,8 @@ impl<'a> Groups<'a> { }) .collect(); - // The server often omits `phone_number` on `` nodes of - // LID-addressed groups, so consumers that key data by PN would treat - // current members as absent. Backfill from the persisted mapping the lib - // already learned (one load + join), instead of forcing each consumer to - // re-resolve LID→PN themselves. - if Self::needs_pn_fill(result.values()) { - let lid_to_pn = self.load_lid_pn_map().await; - if !lid_to_pn.is_empty() { - for meta in result.values_mut() { - fill_participant_pns(meta, &lid_to_pn); - } - } + for meta in result.values_mut() { + self.fill_participant_pns(meta).await; } Ok(result) @@ -431,12 +392,7 @@ impl<'a> Groups<'a> { match self.client.execute(GroupQueryIq::new(jid)).await? { GroupInfoOutcome::Full(group) => { let mut meta = GroupMetadata::from(*group); - if Self::needs_pn_fill(std::iter::once(&meta)) { - let lid_to_pn = self.load_lid_pn_map().await; - if !lid_to_pn.is_empty() { - fill_participant_pns(&mut meta, &lid_to_pn); - } - } + self.fill_participant_pns(&mut meta).await; Ok(meta) } GroupInfoOutcome::NotModified => Err(GroupError::InvalidRequest( @@ -1282,50 +1238,56 @@ mod tests { assert!(!metadata.participants[0].is_super_admin()); } - #[test] - fn fill_participant_pns_backfills_lid_from_mapping() { - use std::collections::HashMap; + #[tokio::test] + async fn fill_participant_pns_backfills_from_cache() { + use crate::lid_pn_cache::{LearningSource, LidPnEntry}; use wacore_binary::jid::{Jid, Server}; - let lid = Jid::new("26263000000099", Server::Lid); + let client = crate::test_utils::create_test_client().await; + // Warm the LID-PN cache with a mapping the server didn't echo on the + // participant stanza. + let entry = LidPnEntry::new( + "26263000000099".to_string(), + "5521900000099".to_string(), + LearningSource::Usync, + ); + client.lid_pn_cache.add(&entry).await; + let mut meta = GroupMetadata { id: "120399@g.us".parse().unwrap(), participants: vec![GroupParticipant { - jid: lid, + jid: Jid::new("26263000000099", Server::Lid), phone_number: None, participant_type: ParticipantType::Member, }], addressing_mode: AddressingMode::Lid, ..Default::default() }; - let map = HashMap::from([("26263000000099".to_string(), "5521900000099".to_string())]); - fill_participant_pns(&mut meta, &map); + client.groups().fill_participant_pns(&mut meta).await; assert_eq!( meta.participants[0].phone_number, Some(Jid::pn("5521900000099")), - "LID participant should receive phone_number from the mapping" + "LID participant should receive its PN from the warm cache" ); } - #[test] - fn fill_participant_pns_noop_in_pn_group() { - use std::collections::HashMap; + #[tokio::test] + async fn fill_participant_pns_noop_in_pn_group() { use wacore_binary::jid::{Jid, Server}; + let client = crate::test_utils::create_test_client().await; // PN-addressed group: untouched (jid already is the PN). - let pn = Jid::new("5521900000098", Server::Pn); let mut meta = GroupMetadata { id: "120398@g.us".parse().unwrap(), participants: vec![GroupParticipant { - jid: pn, + jid: Jid::new("5521900000098", Server::Pn), phone_number: None, participant_type: ParticipantType::Member, }], addressing_mode: AddressingMode::Pn, ..Default::default() }; - let map = HashMap::from([("5521900000098".to_string(), "5521900000098".to_string())]); - fill_participant_pns(&mut meta, &map); + client.groups().fill_participant_pns(&mut meta).await; assert_eq!(meta.participants[0].phone_number, None); } From a58fb39e4ebc6e755715fc279ae4ded25e6d83c3 Mon Sep 17 00:00:00 2001 From: Salientekill Date: Fri, 19 Jun 2026 23:07:04 +0000 Subject: [PATCH 3/4] perf(groups): resolve participant PNs with bounded concurrent fan-out Per CodeRabbit nitpick: the per-participant get_lid_pn_entry awaits ran serially, which on a large group with a cold cache serializes the DB fallbacks. Collect the PN-less LID participants by index and resolve them with buffer_unordered(16), then apply results back. Cache hits stay cheap; cold-cache large groups no longer serialize. --- src/features/groups.rs | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/src/features/groups.rs b/src/features/groups.rs index 4dac43586..120824900 100644 --- a/src/features/groups.rs +++ b/src/features/groups.rs @@ -358,13 +358,40 @@ impl<'a> Groups<'a> { if meta.addressing_mode != AddressingMode::Lid { return; } - for p in meta.participants.iter_mut() { - if p.phone_number.is_none() - && p.jid.is_lid() - && let Ok(Some(entry)) = self.client.get_lid_pn_entry(&p.jid).await - { - p.phone_number = Some(Jid::pn(&*entry.phone_number)); - } + // Participants the server left PN-less, kept with their index. + let pending: Vec<(usize, Jid)> = meta + .participants + .iter() + .enumerate() + .filter_map(|(i, p)| { + (p.phone_number.is_none() && p.jid.is_lid()).then(|| (i, p.jid.clone())) + }) + .collect(); + if pending.is_empty() { + return; + } + + // Cache hits are in-memory, but a cold cache falls back to the DB and a + // large group would otherwise serialize those lookups — bounded fan-out. + use futures::StreamExt; + let resolved: Vec<(usize, Jid)> = futures::stream::iter(pending) + .map(|(i, jid)| async move { + let pn = self + .client + .get_lid_pn_entry(&jid) + .await + .ok() + .flatten() + .map(|e| Jid::pn(&*e.phone_number)); + (i, pn) + }) + .buffer_unordered(16) + .filter_map(|(i, pn)| async move { pn.map(|pn| (i, pn)) }) + .collect() + .await; + + for (i, pn) in resolved { + meta.participants[i].phone_number = Some(pn); } } From 2c68cc0f96240f32c500daedde6f99e67789afb5 Mon Sep 17 00:00:00 2001 From: Salientekill Date: Fri, 19 Jun 2026 23:19:24 +0000 Subject: [PATCH 4/4] style(groups): use filter+map over filter_map+bool::then (clippy) clippy::filter_map_bool_then under -D warnings. Same result, no behavior change. --- src/features/groups.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/features/groups.rs b/src/features/groups.rs index 120824900..2ef1cf2b3 100644 --- a/src/features/groups.rs +++ b/src/features/groups.rs @@ -363,9 +363,8 @@ impl<'a> Groups<'a> { .participants .iter() .enumerate() - .filter_map(|(i, p)| { - (p.phone_number.is_none() && p.jid.is_lid()).then(|| (i, p.jid.clone())) - }) + .filter(|(_, p)| p.phone_number.is_none() && p.jid.is_lid()) + .map(|(i, p)| (i, p.jid.clone())) .collect(); if pending.is_empty() { return;