diff --git a/src/client.rs b/src/client.rs index d2436974b..86d307a98 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1225,8 +1225,7 @@ impl Client { let count: usize = preview .attrs .get("count") - .and_then(|v| v.as_str()) - .and_then(|s| s.parse().ok()) + .and_then(|v| v.as_str().parse().ok()) .unwrap_or(0); if count == 0 { @@ -1308,10 +1307,8 @@ impl Client { && let Some(sync_node) = node.get_optional_child("sync") && let Some(collection_node) = sync_node.get_optional_child("collection") { - let name = collection_node - .attrs() - .optional_string("name") - .unwrap_or(""); + let name = collection_node.attrs().optional_string("name"); + let name = name.as_deref().unwrap_or(""); debug!(target: "Client/Recv", "Received app state sync response for '{name}' (hiding content)."); } else { debug!(target: "Client/Recv","{}", DisplayableNode(&node)); @@ -1336,9 +1333,9 @@ impl Client { } if node.tag.as_ref() == "iq" - && let Some(id) = node.attrs.get("id").and_then(|v| v.as_str()) + && let Some(id) = node.attrs.get("id").map(|v| v.as_str()) { - let has_waiter = self.response_waiters.lock().await.contains_key(id); + let has_waiter = self.response_waiters.lock().await.contains_key(id.as_ref()); if has_waiter && self.handle_iq_response(Arc::clone(&node)).await { return; } @@ -1822,7 +1819,7 @@ impl Client { /// If an ack with an ID that matches a pending task in `response_waiters`, /// the task is resolved and the function returns `true`. Otherwise, returns `false`. pub(crate) async fn handle_ack_response(&self, node: Node) -> bool { - let id_opt = node.attrs.get("id").map(|v| v.to_string_value()); + let id_opt = node.attrs.get("id").map(|v| v.as_str().into_owned()); if let Some(id) = id_opt && let Some(waiter) = self.response_waiters.lock().await.remove(&id) { @@ -2499,10 +2496,17 @@ impl Client { self.is_logged_in.store(false, Ordering::Relaxed); let mut attrs = node.attrs(); - let code = attrs.optional_string("code").unwrap_or(""); + let code_cow = attrs.optional_string("code"); + let code = code_cow.as_deref().unwrap_or(""); let conflict_type = node .get_optional_child("conflict") - .map(|n| n.attrs().optional_string("type").unwrap_or("").to_string()) + .map(|n| { + n.attrs() + .optional_string("type") + .as_deref() + .unwrap_or("") + .to_string() + }) .unwrap_or_default(); if !conflict_type.is_empty() { @@ -2681,7 +2685,11 @@ impl Client { self.core.event_bus.dispatch(&Event::ConnectFailure( crate::types::events::ConnectFailure { reason, - message: attrs.optional_string("message").unwrap_or("").to_string(), + message: attrs + .optional_string("message") + .as_deref() + .unwrap_or("") + .to_string(), raw: Some(node.clone()), }, )); @@ -2689,9 +2697,12 @@ impl Client { } pub(crate) async fn handle_iq(self: &Arc, node: &wacore_binary::node::Node) -> bool { - if let Some("get") = node.attrs.get("type").and_then(|s| s.as_str()) + if node.attrs.get("type").is_some_and(|s| s == "get") && (node.get_optional_child("ping").is_some() - || node.attrs.get("xmlns").and_then(|s| s.as_str()) == Some("urn:xmpp:ping")) + || node + .attrs + .get("xmlns") + .is_some_and(|s| s == "urn:xmpp:ping")) { info!("Received ping, sending pong."); let mut parser = node.attrs(); @@ -3143,7 +3154,7 @@ fn build_ack_node(node: &Node, own_device_pn: Option<&Jid>) -> Option { /// WA Web omits `type` when ACKing ``. fn is_encrypt_identity_notification(node: &Node) -> bool { node.tag == "notification" - && node.attrs.get("type").and_then(|v| v.as_str()) == Some("encrypt") + && node.attrs.get("type").is_some_and(|v| v == "encrypt") && node.get_optional_child("identity").is_some() } @@ -3281,9 +3292,11 @@ mod tests { // 4. Await the receiver with a timeout match tokio::time::timeout(Duration::from_secs(1), rx).await { Ok(Ok(response_node)) => { - assert_eq!( - response_node.attrs.get("id").and_then(|v| v.as_str()), - Some(test_id.as_str()), + assert!( + response_node + .attrs + .get("id") + .is_some_and(|v| v == test_id.as_str()), "Response node should have correct ID" ); } @@ -4310,10 +4323,7 @@ mod tests { // Convert to node let node = session.into_node(); assert_eq!(node.tag, "unified_session"); - assert_eq!( - node.attrs.get("id").and_then(|v| v.as_str()), - Some("123456789") - ); + assert!(node.attrs.get("id").is_some_and(|v| v == "123456789")); // Create an IB stanza let stanza = IbStanza::unified_session(UnifiedSession::new("987654321")); @@ -4325,9 +4335,11 @@ mod tests { let children = ib_node.children().expect("IB stanza should have children"); assert_eq!(children.len(), 1); assert_eq!(children[0].tag, "unified_session"); - assert_eq!( - children[0].attrs.get("id").and_then(|v| v.as_str()), - Some("987654321") + assert!( + children[0] + .attrs + .get("id") + .is_some_and(|v| v == "987654321") ); info!("✅ test_unified_session_protocol_node passed"); @@ -4684,19 +4696,12 @@ mod tests { #[test] fn test_build_pong_with_id() { let pong = build_pong("s.whatsapp.net".to_string(), Some("ping-123")); - assert_eq!( - pong.attrs.get("id").and_then(|v| v.as_str()), - Some("ping-123"), + assert!( + pong.attrs.get("id").is_some_and(|v| v == "ping-123"), "pong should include id when server ping has one" ); - assert_eq!( - pong.attrs.get("type").and_then(|v| v.as_str()), - Some("result") - ); - assert_eq!( - pong.attrs.get("to").and_then(|v| v.as_str()), - Some("s.whatsapp.net") - ); + assert!(pong.attrs.get("type").is_some_and(|v| v == "result")); + assert!(pong.attrs.get("to").is_some_and(|v| v == "s.whatsapp.net")); } #[test] @@ -4706,10 +4711,7 @@ mod tests { !pong.attrs.contains_key("id"), "pong should NOT include id when server ping has none" ); - assert_eq!( - pong.attrs.get("type").and_then(|v| v.as_str()), - Some("result") - ); + assert!(pong.attrs.get("type").is_some_and(|v| v == "result")); } #[test] diff --git a/src/features/presence.rs b/src/features/presence.rs index dc3e9ed70..c748b130c 100644 --- a/src/features/presence.rs +++ b/src/features/presence.rs @@ -97,7 +97,8 @@ impl<'a> Presence<'a> { presence_type, node.attrs .get("name") - .and_then(|s| s.as_str()) + .map(|s| s.as_str()) + .as_deref() .unwrap_or("") ); @@ -445,10 +446,7 @@ mod tests { let node = client.presence().build_unsubscription_node(&jid); assert_eq!(node.tag, "presence"); - assert_eq!( - node.attrs.get("type").and_then(|v| v.as_str()), - Some("unsubscribe") - ); + assert!(node.attrs.get("type").is_some_and(|v| v == "unsubscribe")); assert_eq!( node.attrs.get("to").map(ToString::to_string), Some(jid.to_string()) diff --git a/src/handlers/iq.rs b/src/handlers/iq.rs index 0afb70c65..bb4f0c2c1 100644 --- a/src/handlers/iq.rs +++ b/src/handlers/iq.rs @@ -24,7 +24,7 @@ impl StanzaHandler for IqHandler { async fn handle(&self, client: Arc, node: Arc, _cancelled: &mut bool) -> bool { if !client.handle_iq(&node).await { - if node.attrs.get("type").and_then(|s| s.as_str()) == Some("result") { + if node.attrs.get("type").is_some_and(|s| s == "result") { debug!( "Received late IQ response (waiter already removed): {}", DisplayableNode(&node) diff --git a/src/handlers/notification.rs b/src/handlers/notification.rs index 591b59429..d47595fcc 100644 --- a/src/handlers/notification.rs +++ b/src/handlers/notification.rs @@ -40,11 +40,12 @@ impl StanzaHandler for NotificationHandler { } async fn handle_notification_impl(client: &Arc, node: &Node) { - let notification_type = node.attrs().optional_string("type").unwrap_or_default(); + let notification_type = node.attrs().optional_string("type"); + let notification_type = notification_type.as_deref().unwrap_or_default(); match notification_type { "encrypt" => { - if node.attrs().optional_string("from") == Some(SERVER_JID) { + if node.attrs.get("from").is_some_and(|v| v == SERVER_JID) { // Dispatch based on first child tag, matching WA Web's handleEncryptNotification. // "count" → handlePreKeyLow, "digest" → handleDigestKey let first_child_tag = node @@ -74,10 +75,8 @@ async fn handle_notification_impl(client: &Arc, node: &Node) { let mut collections = Vec::new(); if let Some(children) = node.children() { for collection_node in children.iter().filter(|c| c.tag == "collection") { - let name_str = collection_node - .attrs() - .optional_string("name") - .unwrap_or(""); + let name_cow = collection_node.attrs().optional_string("name"); + let name_str = name_cow.as_deref().unwrap_or(""); let server_version = collection_node.attrs().optional_u64("version").unwrap_or(0); debug!( @@ -461,7 +460,7 @@ async fn handle_account_sync_devices(client: &Arc, node: &Node, devices_ let dhash = devices_node .attrs() .optional_string("dhash") - .map(String::from); + .map(|s| s.into_owned()); // Get timestamp from notification let timestamp = node.attrs().optional_u64("t").unwrap_or_else(|| { diff --git a/src/handlers/presence.rs b/src/handlers/presence.rs index 37149fca9..063016f07 100644 --- a/src/handlers/presence.rs +++ b/src/handlers/presence.rs @@ -21,29 +21,15 @@ impl StanzaHandler for PresenceHandler { } async fn handle(&self, client: Arc, node: Arc, _cancelled: &mut bool) -> bool { - let from = match node.attrs.get("from").map(|v| v.to_string()) { - Some(f) => f, + let from_jid = match node.attrs.get("from").and_then(|v| v.to_jid()) { + Some(jid) => jid, None => { - debug!(target: "PresenceHandler", "Presence stanza missing 'from' attribute"); + debug!(target: "PresenceHandler", "Presence stanza missing or invalid 'from' attribute"); return true; } }; - let from_jid = match from.parse() { - Ok(jid) => jid, - Err(e) => { - debug!(target: "PresenceHandler", "Failed to parse presence 'from' JID: {}", e); - return true; - } - }; - - let presence_type = node - .attrs - .get("type") - .map(|v| v.to_string()) - .unwrap_or_default(); - - let unavailable = presence_type == "unavailable"; + let unavailable = node.attrs.get("type").is_some_and(|v| v == "unavailable"); // Parse last_seen from 'last' attribute if present let last_seen = node @@ -54,8 +40,8 @@ impl StanzaHandler for PresenceHandler { debug!( target: "PresenceHandler", - "Received presence from {}: type={}, unavailable={}", - from, presence_type, unavailable + "Received presence from {}: unavailable={}", + from_jid, unavailable ); client diff --git a/src/message.rs b/src/message.rs index 85c612b10..4e695c385 100644 --- a/src/message.rs +++ b/src/message.rs @@ -435,7 +435,11 @@ impl Client { max_sender_retry_count = max_sender_retry_count.max(sender_count); // Parse decrypt-fail attribute (WA Web: e.maybeAttrString("decrypt-fail") === "hide") - if enc_node.attrs().optional_string("decrypt-fail") == Some("hide") { + if enc_node + .attrs + .get("decrypt-fail") + .is_some_and(|v| v == "hide") + { has_hide_fail = true; } @@ -479,8 +483,7 @@ impl Client { && !group_content_enc_nodes.is_empty() && all_enc_nodes .first() - .and_then(|n| n.attrs().optional_string("type")) - == Some("skmsg") + .is_some_and(|n| n.attrs.get("type").is_some_and(|v| v == "skmsg")) { log::error!( "[msg:{}] Protocol violation: skmsg is first in multi-enc message from {}. \ diff --git a/src/pair.rs b/src/pair.rs index aaa619d53..4e54283dc 100644 --- a/src/pair.rs +++ b/src/pair.rs @@ -153,12 +153,24 @@ async fn handle_pair_success(client: &Arc, request_node: &Node, success_ let business_name = success_node .get_optional_child_by_tag(&["biz"]) - .map(|n| n.attrs().optional_string("name").unwrap_or("").to_string()) + .map(|n| { + n.attrs() + .optional_string("name") + .as_deref() + .unwrap_or("") + .to_string() + }) .unwrap_or_default(); let platform = success_node .get_optional_child_by_tag(&["platform"]) - .map(|n| n.attrs().optional_string("name").unwrap_or("").to_string()) + .map(|n| { + n.attrs() + .optional_string("name") + .as_deref() + .unwrap_or("") + .to_string() + }) .unwrap_or_default(); // For jid and lid, parse them together to handle errors correctly diff --git a/src/receipt.rs b/src/receipt.rs index eadf80223..b88db86d6 100644 --- a/src/receipt.rs +++ b/src/receipt.rs @@ -36,7 +36,8 @@ impl Client { return; } }; - let receipt_type_str = attrs.optional_string("type").unwrap_or("delivery"); + let receipt_type_cow = attrs.optional_string("type"); + let receipt_type_str = receipt_type_cow.as_deref().unwrap_or("delivery"); let participant = attrs.optional_jid("participant"); let receipt_type = ReceiptType::from(receipt_type_str.to_string()); @@ -94,9 +95,15 @@ impl Client { log::debug!( "Received enc_rekey_retry receipt for call-id={} from {} \ (call-creator={}, count={}). VoIP not implemented, forwarding as event.", - attrs.optional_string("call-id").unwrap_or_default(), + attrs + .optional_string("call-id") + .as_deref() + .unwrap_or_default(), from, - attrs.optional_string("call-creator").unwrap_or_default(), + attrs + .optional_string("call-creator") + .as_deref() + .unwrap_or_default(), attrs .optional_string("count") .and_then(|s| s.parse::().ok()) @@ -625,7 +632,7 @@ mod tests { "'to' attr should be JID-typed, got: {:?}", to_attr ); - assert_eq!(to_attr.as_jid().unwrap(), &chat_jid); + assert_eq!(to_attr.to_jid().unwrap(), chat_jid); // "participant" must also be JID-typed let participant_attr = node @@ -637,6 +644,6 @@ mod tests { "'participant' attr should be JID-typed, got: {:?}", participant_attr ); - assert_eq!(participant_attr.as_jid().unwrap(), &sender_jid); + assert_eq!(participant_attr.to_jid().unwrap(), sender_jid); } } diff --git a/src/request.rs b/src/request.rs index 8e2c977bc..0460e07ea 100644 --- a/src/request.rs +++ b/src/request.rs @@ -208,7 +208,7 @@ impl Client { /// This method accepts an `Arc` - if there's a waiter, we clone the Arc (cheap) /// and unwrap it if we're the only holder, otherwise clone the inner Node. pub(crate) async fn handle_iq_response(&self, node: Arc) -> bool { - let id_opt = node.attrs.get("id").map(|v| v.to_string_value()); + let id_opt = node.attrs.get("id").map(|v| v.as_str().into_owned()); if let Some(id) = id_opt { // First check if there's a waiter (without cloning) let waiter = self.response_waiters.lock().await.remove(&id); diff --git a/src/retry.rs b/src/retry.rs index 266092057..b38801d2f 100644 --- a/src/retry.rs +++ b/src/retry.rs @@ -1114,7 +1114,7 @@ mod tests { // Verify top-level receipt attributes assert_eq!( - receipt_node.attrs().optional_string("type"), + receipt_node.attrs().optional_string("type").as_deref(), Some("enc_rekey_retry"), "receipt type must be enc_rekey_retry" ); @@ -1126,7 +1126,7 @@ mod tests { "receipt 'to' must be peer JID" ); assert_eq!( - receipt_node.attrs().optional_string("id"), + receipt_node.attrs().optional_string("id").as_deref(), Some("3EB0AABBCCDD") ); @@ -1139,7 +1139,7 @@ mod tests { .get_optional_child("enc_rekey") .expect(" child must exist"); assert_eq!( - enc_rekey.attrs().optional_string("call-id"), + enc_rekey.attrs().optional_string("call-id").as_deref(), Some("CALL-ABC-123") ); assert!( @@ -1149,7 +1149,10 @@ mod tests { .is_some_and(|v| *v == "5511888888888@s.whatsapp.net"), "enc_rekey 'call-creator' must be creator JID" ); - assert_eq!(enc_rekey.attrs().optional_string("count"), Some("2")); + assert_eq!( + enc_rekey.attrs().optional_string("count").as_deref(), + Some("2") + ); // Verify child let registration = receipt_node diff --git a/src/spam_report.rs b/src/spam_report.rs index f8ade2be5..b95dbe223 100644 --- a/src/spam_report.rs +++ b/src/spam_report.rs @@ -68,13 +68,17 @@ mod tests { let node = build_spam_list_node(&request); assert_eq!(node.tag, "spam_list"); - assert_eq!(node.attrs().string("spam_flow"), "MessageMenu"); + assert!( + node.attrs + .get("spam_flow") + .is_some_and(|v| v == "MessageMenu") + ); let message = node .get_optional_child_by_tag(&["message"]) .expect("spam_list node should have message child"); - assert_eq!(message.attrs().string("id"), "TEST123"); - assert_eq!(message.attrs().string("t"), "1234567890"); + assert!(message.attrs.get("id").is_some_and(|v| v == "TEST123")); + assert!(message.attrs.get("t").is_some_and(|v| v == "1234567890")); } #[test] @@ -97,8 +101,8 @@ mod tests { .get_optional_child_by_tag(&["raw"]) .expect("message node should have raw child"); - assert_eq!(raw.attrs().string("v"), "3"); - assert_eq!(raw.attrs().string("mediatype"), "image"); + assert!(raw.attrs.get("v").is_some_and(|v| v == "3")); + assert!(raw.attrs.get("mediatype").is_some_and(|v| v == "image")); } #[test] @@ -115,8 +119,16 @@ mod tests { let node = build_spam_list_node(&request); - assert_eq!(node.attrs().string("spam_flow"), "GroupInfoReport"); - assert_eq!(node.attrs().string("jid"), "120363025918861132@g.us"); - assert_eq!(node.attrs().string("subject"), "Test Group"); + assert!( + node.attrs + .get("spam_flow") + .is_some_and(|v| v == "GroupInfoReport") + ); + assert!( + node.attrs + .get("jid") + .is_some_and(|v| v == "120363025918861132@g.us") + ); + assert!(node.attrs.get("subject").is_some_and(|v| v == "Test Group")); } } diff --git a/src/types/enc_handler.rs b/src/types/enc_handler.rs index 52fe4434d..26a53d64b 100644 --- a/src/types/enc_handler.rs +++ b/src/types/enc_handler.rs @@ -54,6 +54,7 @@ mod tests { let enc_type = enc_node .attrs() .optional_string("type") + .as_deref() .unwrap_or("unknown") .to_string(); self.calls.lock().await.push(enc_type); diff --git a/src/unified_session.rs b/src/unified_session.rs index f207109f2..129979442 100644 --- a/src/unified_session.rs +++ b/src/unified_session.rs @@ -43,8 +43,8 @@ impl UnifiedSessionManager { /// Update server time offset from node's `t` attribute (Unix timestamp in seconds). pub fn update_server_time_offset(&self, node: &Node) { - if let Some(t_str) = node.attrs.get("t").and_then(|v| v.as_str()) - && let Ok(server_time) = t_str.parse::() + if let Some(t_val) = node.attrs.get("t").map(|v| v.as_str()) + && let Ok(server_time) = t_val.parse::() && server_time > 0 { let local_time = chrono::Utc::now().timestamp(); @@ -62,8 +62,8 @@ impl UnifiedSessionManager { /// This gives a more accurate clock skew estimate by assuming the server /// timestamp corresponds to the midpoint of the round trip. pub fn update_server_time_offset_with_rtt(&self, node: &Node, start_time_ms: i64, rtt_ms: i64) { - if let Some(t_str) = node.attrs.get("t").and_then(|v| v.as_str()) - && let Ok(server_time) = t_str.parse::() + if let Some(t_val) = node.attrs.get("t").map(|v| v.as_str()) + && let Ok(server_time) = t_val.parse::() && server_time > 0 { let midpoint_s = (start_time_ms + rtt_ms / 2) / 1000; diff --git a/tests/e2e/tests/groups.rs b/tests/e2e/tests/groups.rs index 9c09383b8..1a25dabc9 100644 --- a/tests/e2e/tests/groups.rs +++ b/tests/e2e/tests/groups.rs @@ -36,7 +36,7 @@ async fn wait_for_group_notification( ) -> anyhow::Result { client .wait_for_event(timeout_secs, |e| { - matches!(e, Event::Notification(node) if node.attrs().optional_string("type") == Some("w:gp2")) + matches!(e, Event::Notification(node) if node.attrs.get("type").is_some_and(|v| v == "w:gp2")) }) .await } diff --git a/tests/e2e/tests/memory_soak.rs b/tests/e2e/tests/memory_soak.rs index 6ec610f6c..e187be3d4 100644 --- a/tests/e2e/tests/memory_soak.rs +++ b/tests/e2e/tests/memory_soak.rs @@ -454,7 +454,7 @@ async fn test_heavy_group_soak() -> anyhow::Result<()> { for client in [&mut client_b, &mut client_c] { client .wait_for_event(15, |e| { - matches!(e, Event::Notification(node) if node.attrs().optional_string("type") == Some("w:gp2")) + matches!(e, Event::Notification(node) if node.attrs.get("type").is_some_and(|v| v == "w:gp2")) }) .await?; } @@ -474,7 +474,7 @@ async fn test_heavy_group_soak() -> anyhow::Result<()> { client_b .wait_for_event(15, |e| { - matches!(e, Event::Notification(node) if node.attrs().optional_string("type") == Some("w:gp2")) + matches!(e, Event::Notification(node) if node.attrs.get("type").is_some_and(|v| v == "w:gp2")) }) .await?; @@ -599,7 +599,7 @@ async fn test_heavy_mixed_soak() -> anyhow::Result<()> { for client in [&mut client_b, &mut client_c] { client .wait_for_event(15, |e| { - matches!(e, Event::Notification(node) if node.attrs().optional_string("type") == Some("w:gp2")) + matches!(e, Event::Notification(node) if node.attrs.get("type").is_some_and(|v| v == "w:gp2")) }) .await?; } diff --git a/tests/e2e/tests/offline_groups.rs b/tests/e2e/tests/offline_groups.rs index 6f60304f6..0294a2c6d 100644 --- a/tests/e2e/tests/offline_groups.rs +++ b/tests/e2e/tests/offline_groups.rs @@ -50,7 +50,7 @@ async fn test_offline_group_notification() -> anyhow::Result<()> { // Wait for B to get the create notification (confirms group is set up) let _notif_b = client_b .wait_for_event(10, |e| { - matches!(e, Event::Notification(node) if node.attrs().optional_string("type") == Some("w:gp2")) + matches!(e, Event::Notification(node) if node.attrs.get("type").is_some_and(|v| v == "w:gp2")) }) .await?; info!("B received group create notification"); @@ -76,7 +76,7 @@ async fn test_offline_group_notification() -> anyhow::Result<()> { // B (online) should get the notification immediately let _notif_b2 = client_b .wait_for_event(10, |e| { - matches!(e, Event::Notification(node) if node.attrs().optional_string("type") == Some("w:gp2")) + matches!(e, Event::Notification(node) if node.attrs.get("type").is_some_and(|v| v == "w:gp2")) }) .await?; info!("B received add notification (online)"); @@ -84,14 +84,17 @@ async fn test_offline_group_notification() -> anyhow::Result<()> { // Step 4: C should receive the notification after reconnecting (from offline queue) let notif_c = client_c .wait_for_event(30, |e| { - matches!(e, Event::Notification(node) if node.attrs().optional_string("type") == Some("w:gp2")) + matches!(e, Event::Notification(node) if node.attrs.get("type").is_some_and(|v| v == "w:gp2")) }) .await?; if let Event::Notification(node) = notif_c { info!( "C received offline group notification: type={}", - node.attrs().optional_string("type").unwrap_or("?") + node.attrs() + .optional_string("type") + .as_deref() + .unwrap_or("?") ); } else { panic!("Expected Notification event for C"); @@ -144,13 +147,13 @@ async fn test_mixed_offline_event_ordering() -> anyhow::Result<()> { // Wait for C to receive create notification let _notif = client_c .wait_for_event(10, |e| { - matches!(e, Event::Notification(node) if node.attrs().optional_string("type") == Some("w:gp2")) + matches!(e, Event::Notification(node) if node.attrs.get("type").is_some_and(|v| v == "w:gp2")) }) .await?; // Also consume B's notification let _notif_b = client_b .wait_for_event(10, |e| { - matches!(e, Event::Notification(node) if node.attrs().optional_string("type") == Some("w:gp2")) + matches!(e, Event::Notification(node) if node.attrs.get("type").is_some_and(|v| v == "w:gp2")) }) .await?; @@ -197,7 +200,7 @@ async fn test_mixed_offline_event_ordering() -> anyhow::Result<()> { // B receives the add notification let _notif_b2 = client_b .wait_for_event(10, |e| { - matches!(e, Event::Notification(node) if node.attrs().optional_string("type") == Some("w:gp2")) + matches!(e, Event::Notification(node) if node.attrs.get("type").is_some_and(|v| v == "w:gp2")) }) .await?; @@ -227,7 +230,7 @@ async fn test_mixed_offline_event_ordering() -> anyhow::Result<()> { let result = client_c .wait_for_event(10, |e| { matches!(e, Event::Message(msg, _) if msg.conversation.is_some()) - || matches!(e, Event::Notification(node) if node.attrs().optional_string("type") == Some("w:gp2")) + || matches!(e, Event::Notification(node) if node.attrs.get("type").is_some_and(|v| v == "w:gp2")) }) .await; diff --git a/tests/e2e/tests/receipts.rs b/tests/e2e/tests/receipts.rs index da06a84fb..05f0ed2fc 100644 --- a/tests/e2e/tests/receipts.rs +++ b/tests/e2e/tests/receipts.rs @@ -439,7 +439,7 @@ async fn test_group_delivery_receipt() -> anyhow::Result<()> { // Wait for B to receive group create notification client_b .wait_for_event(10, |e| { - matches!(e, Event::Notification(node) if node.attrs().optional_string("type") == Some("w:gp2")) + matches!(e, Event::Notification(node) if node.attrs.get("type").is_some_and(|v| v == "w:gp2")) }) .await?; diff --git a/wacore/appstate/src/patch_decode.rs b/wacore/appstate/src/patch_decode.rs index a8424a8b6..cd10438b7 100644 --- a/wacore/appstate/src/patch_decode.rs +++ b/wacore/appstate/src/patch_decode.rs @@ -119,7 +119,7 @@ fn parse_single_collection(collection: &Node) -> Result { // Check for per-collection error (WA Web: `3JJWKHeu5-P.js:54222-54254`) let col_type = ag.optional_string("type"); - let error = parse_collection_error(collection, col_type); + let error = parse_collection_error(collection, col_type.as_deref()); ag.finish()?; @@ -174,12 +174,13 @@ fn parse_collection_error( // error if the child is missing/malformed. let (code, text) = if let Some(error_node) = collection.get_optional_child("error") { let mut error_attrs = error_node.attrs(); - let code_str = error_attrs.optional_string("code").unwrap_or("0"); + let code_str = error_attrs.optional_string("code"); let text = error_attrs .optional_string("text") + .as_deref() .unwrap_or("") .to_string(); - let code: u16 = code_str.parse().unwrap_or(0); + let code: u16 = code_str.as_deref().unwrap_or("0").parse().unwrap_or(0); (code, text) } else { (0u16, "missing child".to_string()) diff --git a/wacore/binary/src/attrs.rs b/wacore/binary/src/attrs.rs index ee36c51ce..aacb92033 100644 --- a/wacore/binary/src/attrs.rs +++ b/wacore/binary/src/attrs.rs @@ -51,38 +51,22 @@ impl<'a> AttrParserRef<'a> { val } - /// Get string from the value. - /// For JID values, this returns None - use optional_jid instead. - pub fn optional_string(&mut self, key: &str) -> Option<&'a str> { - self.get_raw(key, false).and_then(|v| v.as_str()) + /// Get string from the value. Works for both String and JID variants. + /// - String variant: Cow::Borrowed — zero copy + /// - JID variant: Cow::Owned — allocates only when needed + pub fn optional_string(&mut self, key: &str) -> Option> { + self.get_raw(key, false).map(|v| v.to_string_cow()) } /// Get a required string attribute, returning an error if missing. /// /// Prefer this over `string()` for required attributes as it makes /// the error explicit rather than silently defaulting to empty string. - pub fn required_string(&mut self, key: &str) -> Result<&'a str> { + pub fn required_string(&mut self, key: &str) -> Result> { self.optional_string(key) .ok_or_else(|| BinaryError::MissingAttr(key.to_string())) } - /// Get string, defaulting to empty string if missing. - /// - /// # Deprecation - /// - /// This method silently defaults to an empty string when the attribute is missing. - /// Use `optional_string()` with explicit error handling or `required_string()` - /// to avoid silent failures. - #[deprecated( - since = "0.2.0", - note = "Use optional_string() with explicit handling or required_string() instead" - )] - pub fn string(&mut self, key: &str) -> String { - self.get_raw(key, true) - .map(|v| v.to_string_cow().into_owned()) - .unwrap_or_default() - } - /// Get JID from the value. /// If the value is a JidRef, returns it directly without parsing (zero allocation). /// If the value is a string, parses it as a JID. @@ -218,36 +202,22 @@ impl<'a> AttrParser<'a> { } // --- String --- - pub fn optional_string(&mut self, key: &str) -> Option<&'a str> { - self.get_raw(key, false).and_then(|v| v.as_str()) + /// Get string from the value. Works for both String and JID variants. + /// - String variant: Cow::Borrowed — zero copy + /// - JID variant: Cow::Owned — allocates only when needed + pub fn optional_string(&mut self, key: &str) -> Option> { + self.get_raw(key, false).map(|v| v.as_str()) } /// Get a required string attribute, returning an error if missing. /// /// Prefer this over `string()` for required attributes as it makes /// the error explicit rather than silently defaulting to empty string. - pub fn required_string(&mut self, key: &str) -> Result<&'a str> { + pub fn required_string(&mut self, key: &str) -> Result> { self.optional_string(key) .ok_or_else(|| BinaryError::MissingAttr(key.to_string())) } - /// Get string, defaulting to empty string if missing. - /// - /// # Deprecation - /// - /// This method silently defaults to an empty string when the attribute is missing. - /// Use `optional_string()` with explicit error handling or `required_string()` - /// to avoid silent failures. - #[deprecated( - since = "0.2.0", - note = "Use optional_string() with explicit handling or required_string() instead" - )] - pub fn string(&mut self, key: &str) -> String { - self.get_raw(key, true) - .map(|v| v.to_string_value()) - .unwrap_or_default() - } - // --- JID --- /// Get JID from the value. /// If the value is a JID variant, returns it directly without parsing (zero allocation clone). diff --git a/wacore/binary/src/node.rs b/wacore/binary/src/node.rs index da151680c..eb079af58 100644 --- a/wacore/binary/src/node.rs +++ b/wacore/binary/src/node.rs @@ -37,21 +37,14 @@ impl Default for NodeValue { } impl NodeValue { - /// Get the value as a string slice, if it's a string variant. - #[inline] - pub fn as_str(&self) -> Option<&str> { - match self { - NodeValue::String(s) => Some(s.as_ref()), - NodeValue::Jid(_) => None, - } - } - - /// Get the value as a Jid reference, if it's a JID variant. + /// String view of the value. Works for both variants. + /// - String variant: Cow::Borrowed(&str) — zero copy + /// - Jid variant: Cow::Owned(formatted) — allocates only when needed #[inline] - pub fn as_jid(&self) -> Option<&Jid> { + pub fn as_str(&self) -> Cow<'_, str> { match self { - NodeValue::Jid(j) => Some(j), - NodeValue::String(_) => None, + NodeValue::String(s) => Cow::Borrowed(s.as_str()), + NodeValue::Jid(j) => Cow::Owned(j.to_string()), } } @@ -63,15 +56,6 @@ impl NodeValue { NodeValue::String(s) => s.parse().ok(), } } - - /// Convert to a string, formatting the JID if necessary. - #[inline] - pub fn to_string_value(&self) -> String { - match self { - NodeValue::String(s) => s.clone(), - NodeValue::Jid(j) => j.to_string(), - } - } } use std::fmt; diff --git a/wacore/derive/src/lib.rs b/wacore/derive/src/lib.rs index bdb8c63e0..ac64c62cf 100644 --- a/wacore/derive/src/lib.rs +++ b/wacore/derive/src/lib.rs @@ -221,14 +221,14 @@ pub fn derive_protocol_node(input: TokenStream) -> TokenStream { (AttrType::StringEnum, false, Some(default)) => { quote! { #field_ident: ::wacore::protocol::parse_string_enum( - node.attrs().optional_string(#attr_name).unwrap_or(#default) + node.attrs().optional_string(#attr_name).as_deref().unwrap_or(#default) )? } } (AttrType::StringEnum, false, None) => { quote! { #field_ident: ::wacore::protocol::parse_string_enum( - node.attrs().optional_string(#attr_name) + &node.attrs().optional_string(#attr_name) .ok_or_else(|| ::anyhow::anyhow!("missing required attribute '{}'", #attr_name))? )? } @@ -236,7 +236,7 @@ pub fn derive_protocol_node(input: TokenStream) -> TokenStream { (AttrType::StringEnum, true, _) => { quote! { #field_ident: node.attrs().optional_string(#attr_name) - .map(|s| ::wacore::protocol::parse_string_enum(s)) + .map(|s| ::wacore::protocol::parse_string_enum(&s)) .transpose()? } } diff --git a/wacore/src/ib.rs b/wacore/src/ib.rs index 6c7679a79..e726af46a 100644 --- a/wacore/src/ib.rs +++ b/wacore/src/ib.rs @@ -105,10 +105,7 @@ mod tests { let node = session.into_node(); assert_eq!(node.tag, "unified_session"); - assert_eq!( - node.attrs.get("id").and_then(|v| v.as_str()), - Some("123456789") - ); + assert!(node.attrs.get("id").is_some_and(|v| v == "123456789")); } #[test] @@ -130,9 +127,11 @@ mod tests { let children = node.children().unwrap(); assert_eq!(children.len(), 1); assert_eq!(children[0].tag, "unified_session"); - assert_eq!( - children[0].attrs.get("id").and_then(|v| v.as_str()), - Some("123456789") + assert!( + children[0] + .attrs + .get("id") + .is_some_and(|v| v == "123456789") ); } diff --git a/wacore/src/iq/blocklist.rs b/wacore/src/iq/blocklist.rs index ccd56a1c4..eb290158b 100644 --- a/wacore/src/iq/blocklist.rs +++ b/wacore/src/iq/blocklist.rs @@ -189,8 +189,12 @@ mod tests { let node = request.into_node(); assert_eq!(node.tag, "item"); - assert_eq!(node.attrs().string("action"), "block"); - assert_eq!(node.attrs().string("jid"), "1234567890@s.whatsapp.net"); + assert!(node.attrs.get("action").is_some_and(|v| v == "block")); + assert!( + node.attrs + .get("jid") + .is_some_and(|v| v == "1234567890@s.whatsapp.net") + ); } #[test] @@ -203,8 +207,12 @@ mod tests { let node = entry.into_node(); assert_eq!(node.tag, "item"); - assert_eq!(node.attrs().string("jid"), "1234567890@s.whatsapp.net"); - assert_eq!(node.attrs().string("t"), "1234567890"); + assert!( + node.attrs + .get("jid") + .is_some_and(|v| v == "1234567890@s.whatsapp.net") + ); + assert!(node.attrs.get("t").is_some_and(|v| v == "1234567890")); } #[test] diff --git a/wacore/src/iq/chatstate.rs b/wacore/src/iq/chatstate.rs index d06fe095d..244d9f40d 100644 --- a/wacore/src/iq/chatstate.rs +++ b/wacore/src/iq/chatstate.rs @@ -79,7 +79,7 @@ impl ReceivedChatState { match child.tag.as_ref() { "composing" => { // Check for media="audio" to distinguish recording from typing - if child.attrs().optional_string("media") == Some("audio") { + if child.attrs.get("media").is_some_and(|v| v == "audio") { Self::RecordingAudio } else { Self::Typing diff --git a/wacore/src/iq/contacts.rs b/wacore/src/iq/contacts.rs index 10715b4e8..9616c2226 100644 --- a/wacore/src/iq/contacts.rs +++ b/wacore/src/iq/contacts.rs @@ -136,15 +136,14 @@ impl IqSpec for ProfilePictureSpec { // Check for error response if let Some(error_node) = picture_node.get_optional_child("error") { - let code = error_node.attrs().optional_string("code").unwrap_or("0"); - if code == "404" || code == "401" { + let code = error_node.attrs().optional_string("code"); + let code_str = code.as_deref().unwrap_or("0"); + if code_str == "404" || code_str == "401" { return Ok(None); } - let text = error_node - .attrs() - .optional_string("text") - .unwrap_or("unknown error"); - return Err(anyhow!("Profile picture error {}: {}", code, text)); + let text = error_node.attrs().optional_string("text"); + let text_str = text.as_deref().unwrap_or("unknown error"); + return Err(anyhow!("Profile picture error {}: {}", code_str, text_str)); } let id = match picture_node.attrs().optional_string("id") { @@ -320,10 +319,7 @@ mod tests { if let Some(NodeContent::Nodes(nodes)) = &iq.content { assert_eq!(nodes[0].tag, "picture"); - assert_eq!( - nodes[0].attrs.get("type").and_then(|s| s.as_str()), - Some("preview") - ); + assert!(nodes[0].attrs.get("type").is_some_and(|s| s == "preview")); } } @@ -336,10 +332,7 @@ mod tests { let iq = spec.build_iq(); if let Some(NodeContent::Nodes(nodes)) = &iq.content { - assert_eq!( - nodes[0].attrs.get("type").and_then(|s| s.as_str()), - Some("image") - ); + assert!(nodes[0].attrs.get("type").is_some_and(|s| s == "image")); } } @@ -450,10 +443,7 @@ mod tests { if let Some(NodeContent::Nodes(nodes)) = &iq.content { let picture = &nodes[0]; assert_eq!(picture.tag, "picture"); - assert_eq!( - picture.attrs.get("type").and_then(|v| v.as_str()), - Some("image") - ); + assert!(picture.attrs.get("type").is_some_and(|v| v == "image")); match &picture.content { Some(NodeContent::Bytes(data)) => { assert_eq!(data, &[0xFF, 0xD8, 0xFF]); diff --git a/wacore/src/iq/dirty.rs b/wacore/src/iq/dirty.rs index 82fe65b95..3a96cac4b 100644 --- a/wacore/src/iq/dirty.rs +++ b/wacore/src/iq/dirty.rs @@ -112,9 +112,11 @@ mod tests { if let Some(NodeContent::Nodes(nodes)) = &iq.content { assert_eq!(nodes.len(), 1); assert_eq!(nodes[0].tag, "clean"); - assert_eq!( - nodes[0].attrs.get("type").and_then(|v| v.as_str()), - Some("account_sync") + assert!( + nodes[0] + .attrs + .get("type") + .is_some_and(|v| v == "account_sync") ); assert!(nodes[0].attrs.get("timestamp").is_none()); } else { @@ -129,13 +131,12 @@ mod tests { if let Some(NodeContent::Nodes(nodes)) = &iq.content { assert_eq!(nodes.len(), 1); - assert_eq!( - nodes[0].attrs.get("type").and_then(|v| v.as_str()), - Some("groups") - ); - assert_eq!( - nodes[0].attrs.get("timestamp").and_then(|v| v.as_str()), - Some("1234567890") + assert!(nodes[0].attrs.get("type").is_some_and(|v| v == "groups")); + assert!( + nodes[0] + .attrs + .get("timestamp") + .is_some_and(|v| v == "1234567890") ); } else { panic!("Expected NodeContent::Nodes"); @@ -165,18 +166,19 @@ mod tests { if let Some(NodeContent::Nodes(nodes)) = &iq.content { assert_eq!(nodes.len(), 2); - assert_eq!( - nodes[0].attrs.get("type").and_then(|v| v.as_str()), - Some("account_sync") + assert!( + nodes[0] + .attrs + .get("type") + .is_some_and(|v| v == "account_sync") ); assert!(nodes[0].attrs.get("timestamp").is_none()); - assert_eq!( - nodes[1].attrs.get("type").and_then(|v| v.as_str()), - Some("groups") - ); - assert_eq!( - nodes[1].attrs.get("timestamp").and_then(|v| v.as_str()), - Some("9876543210") + assert!(nodes[1].attrs.get("type").is_some_and(|v| v == "groups")); + assert!( + nodes[1] + .attrs + .get("timestamp") + .is_some_and(|v| v == "9876543210") ); } else { panic!("Expected NodeContent::Nodes"); diff --git a/wacore/src/iq/groups.rs b/wacore/src/iq/groups.rs index 08399f332..c599a6028 100644 --- a/wacore/src/iq/groups.rs +++ b/wacore/src/iq/groups.rs @@ -341,7 +341,10 @@ impl ProtocolNode for GroupParticipantResponse { .ok_or_else(|| anyhow!("participant missing required 'jid' attribute"))?; let phone_number = node.attrs().optional_jid("phone_number"); // Default to Member for unknown participant types to avoid failing the whole group parse - let participant_type = ParticipantType::try_from(node.attrs().optional_string("type")) + let participant_type = node + .attrs() + .optional_string("type") + .and_then(|s| ParticipantType::try_from(s.as_ref()).ok()) .unwrap_or(ParticipantType::Member); Ok(Self { @@ -479,11 +482,17 @@ impl ProtocolNode for GroupInfoResponse { Jid::group(id_str) }; - let subject = - GroupSubject::new_unchecked(optional_attr(node, "subject").unwrap_or_default()); + let subject = GroupSubject::new_unchecked( + optional_attr(node, "subject") + .as_deref() + .unwrap_or_default(), + ); - let addressing_mode = - AddressingMode::try_from(optional_attr(node, "addressing_mode").unwrap_or("pn"))?; + let addressing_mode = AddressingMode::try_from( + optional_attr(node, "addressing_mode") + .as_deref() + .unwrap_or("pn"), + )?; let participants = collect_children::(node, "participant")?; @@ -1416,7 +1425,7 @@ mod tests { let node = build_create_group_node(&options); assert_eq!(node.tag, "create"); assert_eq!( - node.attrs().optional_string("subject"), + node.attrs().optional_string("subject").as_deref(), Some("Test Subject") ); @@ -1454,7 +1463,10 @@ mod tests { // id is random hex, just check it exists and is 8 chars let id = desc_node.attrs().optional_string("id").unwrap(); assert_eq!(id.len(), 8); - assert_eq!(desc_node.attrs().optional_string("prev"), Some("AABBCCDD")); + assert_eq!( + desc_node.attrs().optional_string("prev").as_deref(), + Some("AABBCCDD") + ); // Should have a child assert!(desc_node.get_children_by_tag("body").next().is_some()); } else { @@ -1471,8 +1483,14 @@ mod tests { if let Some(NodeContent::Nodes(nodes)) = &iq.content { let desc_node = &nodes[0]; assert_eq!(desc_node.tag, "description"); - assert_eq!(desc_node.attrs().optional_string("delete"), Some("true")); - assert_eq!(desc_node.attrs().optional_string("prev"), Some("PREV1234")); + assert_eq!( + desc_node.attrs().optional_string("delete").as_deref(), + Some("true") + ); + assert_eq!( + desc_node.attrs().optional_string("prev").as_deref(), + Some("PREV1234") + ); // id should still be present assert!(desc_node.attrs().optional_string("id").is_some()); } else { @@ -1629,7 +1647,7 @@ mod tests { if let Some(NodeContent::Nodes(nodes)) = &iq.content { assert_eq!(nodes[0].tag, "ephemeral"); assert_eq!( - nodes[0].attrs().optional_string("expiration"), + nodes[0].attrs().optional_string("expiration").as_deref(), Some("86400") ); } else { @@ -1654,7 +1672,7 @@ mod tests { if let Some(NodeContent::Nodes(nodes)) = &iq.content { assert_eq!(nodes[0].tag, "membership_approval_mode"); let join = nodes[0].get_children_by_tag("group_join").next().unwrap(); - assert_eq!(join.attrs().optional_string("state"), Some("on")); + assert!(join.attrs.get("state").is_some_and(|v| v == "on")); } else { panic!("expected nodes content"); } diff --git a/wacore/src/iq/mediaconn.rs b/wacore/src/iq/mediaconn.rs index 22144c639..b92144386 100644 --- a/wacore/src/iq/mediaconn.rs +++ b/wacore/src/iq/mediaconn.rs @@ -189,17 +189,24 @@ impl ProtocolNode for MediaConnHostExtended { .to_string(); let host_type = attrs .optional_string("type") + .as_deref() .unwrap_or("primary") .to_string(); Ok(Self { hostname, host_type, - fallback_hostname: attrs.optional_string("fallback_hostname").map(String::from), - ip4: attrs.optional_string("ip4").map(String::from), - ip6: attrs.optional_string("ip6").map(String::from), - fallback_ip4: attrs.optional_string("fallback_ip4").map(String::from), - fallback_ip6: attrs.optional_string("fallback_ip6").map(String::from), + fallback_hostname: attrs + .optional_string("fallback_hostname") + .map(|s| s.into_owned()), + ip4: attrs.optional_string("ip4").map(|s| s.into_owned()), + ip6: attrs.optional_string("ip6").map(|s| s.into_owned()), + fallback_ip4: attrs + .optional_string("fallback_ip4") + .map(|s| s.into_owned()), + fallback_ip6: attrs + .optional_string("fallback_ip6") + .map(|s| s.into_owned()), upload: node.get_optional_child("upload").is_some(), download: node.get_optional_child("download").is_some(), download_categories: node @@ -295,7 +302,7 @@ impl ProtocolNode for MediaConnResponseExtended { let ttl = attrs.optional_u64("ttl").unwrap_or(0); let auth_ttl = attrs.optional_u64("auth_ttl"); let max_buckets = attrs.optional_u64("max_buckets"); - let ip_token = attrs.optional_string("ip_token").map(String::from); + let ip_token = attrs.optional_string("ip_token").map(|s| s.into_owned()); let set_ip_token = attrs.optional_u64("set_ip_token"); let mut hosts = Vec::new(); diff --git a/wacore/src/iq/mex.rs b/wacore/src/iq/mex.rs index 29d922262..2f9b257ec 100644 --- a/wacore/src/iq/mex.rs +++ b/wacore/src/iq/mex.rs @@ -188,9 +188,11 @@ mod tests { if let Some(NodeContent::Nodes(nodes)) = &iq.content { assert_eq!(nodes.len(), 1); assert_eq!(nodes[0].tag, "query"); - assert_eq!( - nodes[0].attrs.get("query_id").and_then(|s| s.as_str()), - Some("29829202653362039") + assert!( + nodes[0] + .attrs + .get("query_id") + .is_some_and(|s| s == "29829202653362039") ); } else { panic!("Expected NodeContent::Nodes"); diff --git a/wacore/src/iq/node.rs b/wacore/src/iq/node.rs index f4faf77a2..6d3261049 100644 --- a/wacore/src/iq/node.rs +++ b/wacore/src/iq/node.rs @@ -3,6 +3,8 @@ //! These functions provide a consistent way to extract required and optional //! children/attributes from protocol nodes with clear error messages. +use std::borrow::Cow; + use crate::protocol::ProtocolNode; use anyhow::anyhow; use wacore_binary::jid::Jid; @@ -26,12 +28,12 @@ pub fn optional_child<'a>(node: &'a Node, tag: &str) -> Option<&'a Node> { pub fn required_attr(node: &Node, key: &str) -> Result { node.attrs .get(key) - .map(|v| v.to_string_value()) + .map(|v| v.to_string()) .ok_or_else(|| anyhow!("missing required attribute {key}")) } /// Get an optional string attribute. -pub fn optional_attr<'a>(node: &'a Node, key: &str) -> Option<&'a str> { +pub fn optional_attr<'a>(node: &'a Node, key: &str) -> Option> { node.attrs().optional_string(key) } diff --git a/wacore/src/iq/prekeys.rs b/wacore/src/iq/prekeys.rs index d026e83e5..8b369370f 100644 --- a/wacore/src/iq/prekeys.rs +++ b/wacore/src/iq/prekeys.rs @@ -109,7 +109,8 @@ impl IqSpec for PreKeyCountSpec { // Server may return without value attribute when count is 0, // or return an unparseable value. Default to 0 in these cases. - let count_str = count_node.attrs().optional_string("value").unwrap_or("0"); + let count_str = count_node.attrs().optional_string("value"); + let count_str = count_str.as_deref().unwrap_or("0"); let count = count_str.parse::().unwrap_or(0); Ok(PreKeyCountResponse { count }) @@ -1010,7 +1011,10 @@ mod tests { assert_eq!(node.tag, "user"); assert_eq!(node.attrs().optional_jid("jid"), Some(jid)); - assert_eq!(node.attrs().optional_string("type"), Some("result")); + assert_eq!( + node.attrs().optional_string("type").as_deref(), + Some("result") + ); // Verify children count (registration, type, identity, skey, key, device-identity) if let Some(children) = node.children() { diff --git a/wacore/src/iq/privacy.rs b/wacore/src/iq/privacy.rs index f2b425621..196ee8827 100644 --- a/wacore/src/iq/privacy.rs +++ b/wacore/src/iq/privacy.rs @@ -148,8 +148,8 @@ impl IqSpec for PrivacySettingsSpec { .ok_or_else(|| anyhow::anyhow!("missing value in category"))?; settings.push(PrivacySetting { - category: PrivacyCategory::from(name), - value: PrivacyValue::from(value), + category: PrivacyCategory::from(name.as_ref()), + value: PrivacyValue::from(value.as_ref()), }); } diff --git a/wacore/src/iq/props.rs b/wacore/src/iq/props.rs index ef8ec8b22..b166984bc 100644 --- a/wacore/src/iq/props.rs +++ b/wacore/src/iq/props.rs @@ -242,8 +242,8 @@ impl crate::protocol::ProtocolNode for PropsResponse { return Err(anyhow::anyhow!("expected , got <{}>", node.tag)); } - let ab_key = optional_attr(node, "ab_key").map(str::to_string); - let hash = optional_attr(node, "hash").map(str::to_string); + let ab_key = optional_attr(node, "ab_key").map(|s| s.into_owned()); + let hash = optional_attr(node, "hash").map(|s| s.into_owned()); let refresh = optional_attr(node, "refresh").and_then(|s| s.parse().ok()); let refresh_id = optional_attr(node, "refresh_id").and_then(|s| s.parse().ok()); let delta_update = optional_attr(node, "delta_update") @@ -343,10 +343,7 @@ mod tests { if let Some(NodeContent::Nodes(nodes)) = &iq.content { assert_eq!(nodes.len(), 1); assert_eq!(nodes[0].tag, "props"); - assert_eq!( - nodes[0].attrs.get("protocol").and_then(|v| v.as_str()), - Some("1") - ); + assert!(nodes[0].attrs.get("protocol").is_some_and(|v| v == "1")); assert!(nodes[0].attrs.get("hash").is_none()); assert!(nodes[0].attrs.get("refresh_id").is_none()); } else { @@ -360,10 +357,7 @@ mod tests { let iq = spec.build_iq(); if let Some(NodeContent::Nodes(nodes)) = &iq.content { - assert_eq!( - nodes[0].attrs.get("hash").and_then(|v| v.as_str()), - Some("abc123") - ); + assert!(nodes[0].attrs.get("hash").is_some_and(|v| v == "abc123")); assert!(nodes[0].attrs.get("refresh_id").is_none()); } else { panic!("Expected NodeContent::Nodes"); @@ -377,10 +371,7 @@ mod tests { if let Some(NodeContent::Nodes(nodes)) = &iq.content { assert!(nodes[0].attrs.get("hash").is_none()); - assert_eq!( - nodes[0].attrs.get("refresh_id").and_then(|v| v.as_str()), - Some("42") - ); + assert!(nodes[0].attrs.get("refresh_id").is_some_and(|v| v == "42")); } else { panic!("Expected NodeContent::Nodes"); } diff --git a/wacore/src/iq/spam_report.rs b/wacore/src/iq/spam_report.rs index 2d6baaff8..0e1c60423 100644 --- a/wacore/src/iq/spam_report.rs +++ b/wacore/src/iq/spam_report.rs @@ -90,9 +90,11 @@ mod tests { if let Some(NodeContent::Nodes(nodes)) = &iq.content { assert_eq!(nodes.len(), 1); assert_eq!(nodes[0].tag, "spam_list"); - assert_eq!( - nodes[0].attrs.get("spam_flow").and_then(|s| s.as_str()), - Some("MessageMenu") + assert!( + nodes[0] + .attrs + .get("spam_flow") + .is_some_and(|s| s == "MessageMenu") ); } else { panic!("Expected NodeContent::Nodes"); diff --git a/wacore/src/iq/tctoken.rs b/wacore/src/iq/tctoken.rs index ec1f27aa7..afa3fece2 100644 --- a/wacore/src/iq/tctoken.rs +++ b/wacore/src/iq/tctoken.rs @@ -221,7 +221,8 @@ pub fn parse_privacy_token_notification( let mut tokens = Vec::new(); for token_node in tokens_node.get_children_by_tag("token") { - let token_type = optional_attr(token_node, "type").unwrap_or(""); + let token_type = optional_attr(token_node, "type"); + let token_type = token_type.as_deref().unwrap_or(""); if token_type != "trusted_contact" { continue; } @@ -473,7 +474,10 @@ mod tests { fn test_build_tc_token_node_with_timestamp() { let node = build_tc_token_node_with_timestamp(&[0x01], 1707000000); assert_eq!(node.tag, "tctoken"); - assert_eq!(node.attrs().optional_string("t"), Some("1707000000")); + assert_eq!( + node.attrs().optional_string("t").as_deref(), + Some("1707000000") + ); } #[test] diff --git a/wacore/src/iq/usync.rs b/wacore/src/iq/usync.rs index f840b1d12..968d066fe 100644 --- a/wacore/src/iq/usync.rs +++ b/wacore/src/iq/usync.rs @@ -119,7 +119,7 @@ fn parse_user_common_fields(user_node: &Node) -> Option { let contact_node = user_node.get_optional_child("contact"); let is_registered = contact_node - .map(|c| c.attrs().optional_string("type") == Some("in")) + .map(|c| c.attrs.get("type").is_some_and(|v| v == "in")) .unwrap_or(false); let lid = user_node.get_optional_child("lid").and_then(|lid_node| { @@ -274,7 +274,7 @@ impl IqSpec for IsOnWhatsAppSpec { { let contact_node = user_node.get_optional_child("contact"); let is_registered = contact_node - .map(|c| c.attrs().optional_string("type") == Some("in")) + .map(|c| c.attrs.get("type").is_some_and(|v| v == "in")) .unwrap_or(false); results.push(IsOnWhatsAppResult { jid, is_registered }); @@ -652,17 +652,13 @@ mod tests { assert_eq!(nodes.len(), 1); let usync = &nodes[0]; assert_eq!(usync.tag, "usync"); - assert_eq!( - usync.attrs.get("sid").and_then(|s| s.as_str()), - Some("test-sid") - ); - assert_eq!( - usync.attrs.get("mode").and_then(|s| s.as_str()), - Some("query") - ); - assert_eq!( - usync.attrs.get("context").and_then(|s| s.as_str()), - Some("interactive") + assert!(usync.attrs.get("sid").is_some_and(|s| s == "test-sid")); + assert!(usync.attrs.get("mode").is_some_and(|s| s == "query")); + assert!( + usync + .attrs + .get("context") + .is_some_and(|s| s == "interactive") ); } else { panic!("Expected NodeContent::Nodes"); @@ -777,13 +773,12 @@ mod tests { if let Some(NodeContent::Nodes(nodes)) = &iq.content { let usync = &nodes[0]; - assert_eq!( - usync.attrs.get("mode").and_then(|s| s.as_str()), - Some("full") - ); - assert_eq!( - usync.attrs.get("context").and_then(|s| s.as_str()), - Some("background") + assert!(usync.attrs.get("mode").is_some_and(|s| s == "full")); + assert!( + usync + .attrs + .get("context") + .is_some_and(|s| s == "background") ); } else { panic!("Expected NodeContent::Nodes"); @@ -865,25 +860,13 @@ mod tests { if let Some(NodeContent::Nodes(nodes)) = &iq.content { let usync = &nodes[0]; - assert_eq!( - usync.attrs.get("sid").and_then(|s| s.as_str()), - Some("test-sid") - ); - assert_eq!( - usync.attrs.get("mode").and_then(|s| s.as_str()), - Some("query") - ); - assert_eq!( - usync.attrs.get("context").and_then(|s| s.as_str()), - Some("message") - ); + assert!(usync.attrs.get("sid").is_some_and(|s| s == "test-sid")); + assert!(usync.attrs.get("mode").is_some_and(|s| s == "query")); + assert!(usync.attrs.get("context").is_some_and(|s| s == "message")); let query = usync.get_optional_child("query").unwrap(); let devices = query.get_optional_child("devices").unwrap(); - assert_eq!( - devices.attrs.get("version").and_then(|s| s.as_str()), - Some("2") - ); + assert!(devices.attrs.get("version").is_some_and(|s| s == "2")); } else { panic!("Expected NodeContent::Nodes"); } diff --git a/wacore/src/pair.rs b/wacore/src/pair.rs index 1fee94ba0..78738ca37 100644 --- a/wacore/src/pair.rs +++ b/wacore/src/pair.rs @@ -71,8 +71,8 @@ impl PairUtils { Some( NodeBuilder::new("iq") .attrs([ - ("to", to.to_string_value()), - ("id", id.to_string_value()), + ("to", to.to_string()), + ("id", id.to_string()), ("type", "result".to_string()), ]) .build(), diff --git a/wacore/src/reporting_token.rs b/wacore/src/reporting_token.rs index 57f8a375d..dc20ce994 100644 --- a/wacore/src/reporting_token.rs +++ b/wacore/src/reporting_token.rs @@ -875,7 +875,7 @@ mod tests { let token_node = node.get_children_by_tag("reporting_token").next().unwrap(); - assert_eq!(token_node.attrs().string("v"), "2"); + assert!(token_node.attrs.get("v").is_some_and(|v| v == "2")); // CRITICAL: Verify the token content is BINARY BYTES, not a hex string. // WhatsApp expects raw bytes in the reporting_token node content. diff --git a/wacore/src/request.rs b/wacore/src/request.rs index ecec0871f..9179df488 100644 --- a/wacore/src/request.rs +++ b/wacore/src/request.rs @@ -194,7 +194,11 @@ impl RequestUtils { if let Some(error_node) = error_child { let mut parser = wacore_binary::attrs::AttrParser::new(error_node); let code = parser.optional_u64("code").unwrap_or(0) as u16; - let text = parser.optional_string("text").unwrap_or("").to_string(); + let text = parser + .optional_string("text") + .as_deref() + .unwrap_or("") + .to_string(); return Box::new(Err(IqError::ServerError { code, text })); } return Box::new(Err(IqError::ServerError { diff --git a/wacore/src/stanza/business.rs b/wacore/src/stanza/business.rs index 8cb41fc3f..c5cf90ab4 100644 --- a/wacore/src/stanza/business.rs +++ b/wacore/src/stanza/business.rs @@ -55,17 +55,23 @@ impl VerifiedName { let name = node .attrs() .optional_string("name") + .map(|s| s.into_owned()) .or_else(|| { node.get_optional_child_by_tag(&["name"]) .and_then(|n| match &n.content { - Some(NodeContent::String(s)) => Some(s.as_str()), + Some(NodeContent::String(s)) => Some(s.clone()), _ => None, }) - }) - .map(String::from); + }); - let serial = node.attrs().optional_string("serial").map(String::from); - let issuer = node.attrs().optional_string("issuer").map(String::from); + let serial = node + .attrs() + .optional_string("serial") + .map(|s| s.into_owned()); + let issuer = node + .attrs() + .optional_string("issuer") + .map(|s| s.into_owned()); let certificate = match &node.content { Some(NodeContent::Bytes(b)) => Some(b.clone()), _ => None, @@ -117,7 +123,7 @@ impl BusinessNotification { if node.tag != "notification" { return Err(anyhow!("expected , got <{}>", node.tag)); } - if optional_attr(node, "type") != Some("business") { + if !node.attrs.get("type").is_some_and(|v| v == "business") { return Err(anyhow!("expected type='business'")); } @@ -127,7 +133,7 @@ impl BusinessNotification { .ok_or_else(|| anyhow!("notification missing required 'from' attribute"))?; let stanza_id = optional_attr(node, "id") - .map(String::from) + .map(|s| s.into_owned()) .unwrap_or_default(); let timestamp = match node.attrs().optional_u64("t") { diff --git a/wacore/src/stanza/devices.rs b/wacore/src/stanza/devices.rs index aa10df13e..288482cd7 100644 --- a/wacore/src/stanza/devices.rs +++ b/wacore/src/stanza/devices.rs @@ -307,7 +307,7 @@ impl DeviceNotification { if node.tag != "notification" { return Err(anyhow!("expected , got <{}>", node.tag)); } - if optional_attr(node, "type") != Some("devices") { + if !node.attrs.get("type").is_some_and(|v| v == "devices") { return Err(anyhow!("expected type='devices'")); } @@ -317,7 +317,7 @@ impl DeviceNotification { .ok_or_else(|| anyhow!("notification missing required 'from' attribute"))?; let lid_user = node.attrs().optional_jid("lid"); let stanza_id = optional_attr(node, "id") - .map(String::from) + .map(|s| s.into_owned()) .unwrap_or_default(); // Parse timestamp with checked conversion diff --git a/wacore/src/stanza/groups.rs b/wacore/src/stanza/groups.rs index 97cc02da2..5287ced56 100644 --- a/wacore/src/stanza/groups.rs +++ b/wacore/src/stanza/groups.rs @@ -208,8 +208,7 @@ impl GroupNotification { let is_lid_addressing_mode = node .attrs() .optional_string("addressing_mode") - .map(|s| s == "lid") - .unwrap_or(false); + .is_some_and(|s| s == "lid"); let actions = node .children() @@ -233,11 +232,17 @@ fn parse_action(node: &Node) -> Option { // Participant management "add" => GroupNotificationAction::Add { participants: parse_participants(node), - reason: node.attrs().optional_string("reason").map(str::to_string), + reason: node + .attrs() + .optional_string("reason") + .map(|s| s.into_owned()), }, "remove" => GroupNotificationAction::Remove { participants: parse_participants(node), - reason: node.attrs().optional_string("reason").map(str::to_string), + reason: node + .attrs() + .optional_string("reason") + .map(|s| s.into_owned()), }, "promote" => GroupNotificationAction::Promote { participants: parse_participants(node), @@ -254,6 +259,7 @@ fn parse_action(node: &Node) -> Option { subject: node .attrs() .optional_string("subject") + .as_deref() .unwrap_or_default() .to_string(), subject_owner: node.attrs().optional_jid("s_o"), @@ -263,6 +269,7 @@ fn parse_action(node: &Node) -> Option { let id = node .attrs() .optional_string("id") + .as_deref() .unwrap_or_default() .to_string(); let description = if node.get_optional_child("delete").is_some() { @@ -285,7 +292,7 @@ fn parse_action(node: &Node) -> Option { threshold: node .attrs() .optional_string("threshold") - .map(str::to_string), + .map(|s| s.into_owned()), }, "unlocked" => GroupNotificationAction::Unlocked, "announcement" => GroupNotificationAction::Announce, @@ -302,8 +309,7 @@ fn parse_action(node: &Node) -> Option { let enabled = node .get_optional_child("group_join") .and_then(|gj| gj.attrs().optional_string("state")) - .map(|s| s == "on") - .unwrap_or(false); + .is_some_and(|s| s == "on"); GroupNotificationAction::MembershipApprovalMode { enabled } } "member_add_mode" => { @@ -322,6 +328,7 @@ fn parse_action(node: &Node) -> Option { code: node .attrs() .optional_string("code") + .as_deref() .unwrap_or_default() .to_string(), }, @@ -331,6 +338,7 @@ fn parse_action(node: &Node) -> Option { lock_type: node .attrs() .optional_string("type") + .as_deref() .unwrap_or_default() .to_string(), }, @@ -339,7 +347,10 @@ fn parse_action(node: &Node) -> Option { // Group lifecycle "create" => GroupNotificationAction::Create { raw: node.clone() }, "delete" => GroupNotificationAction::Delete { - reason: node.attrs().optional_string("reason").map(str::to_string), + reason: node + .attrs() + .optional_string("reason") + .map(|s| s.into_owned()), }, // Community linking @@ -347,6 +358,7 @@ fn parse_action(node: &Node) -> Option { link_type: node .attrs() .optional_string("link_type") + .as_deref() .unwrap_or_default() .to_string(), raw: node.clone(), @@ -355,12 +367,13 @@ fn parse_action(node: &Node) -> Option { unlink_type: node .attrs() .optional_string("unlink_type") + .as_deref() .unwrap_or_default() .to_string(), unlink_reason: node .attrs() .optional_string("unlink_reason") - .map(str::to_string), + .map(|s| s.into_owned()), raw: node.clone(), }, diff --git a/wacore/src/types/spam_report.rs b/wacore/src/types/spam_report.rs index e69cec2e5..5206bb3bf 100644 --- a/wacore/src/types/spam_report.rs +++ b/wacore/src/types/spam_report.rs @@ -150,13 +150,17 @@ mod tests { let node = build_spam_list_node(&request); assert_eq!(node.tag, "spam_list"); - assert_eq!(node.attrs().string("spam_flow"), "MessageMenu"); + assert!( + node.attrs + .get("spam_flow") + .is_some_and(|v| v == "MessageMenu") + ); let message = node .get_optional_child_by_tag(&["message"]) .expect("test node child should exist"); - assert_eq!(message.attrs().string("id"), "TEST123"); - assert_eq!(message.attrs().string("t"), "1234567890"); + assert!(message.attrs.get("id").is_some_and(|v| v == "TEST123")); + assert!(message.attrs.get("t").is_some_and(|v| v == "1234567890")); } #[test] @@ -179,8 +183,8 @@ mod tests { .get_optional_child_by_tag(&["raw"]) .expect("test node child should exist"); - assert_eq!(raw.attrs().string("v"), "3"); - assert_eq!(raw.attrs().string("mediatype"), "image"); + assert!(raw.attrs.get("v").is_some_and(|v| v == "3")); + assert!(raw.attrs.get("mediatype").is_some_and(|v| v == "image")); } #[test] @@ -197,8 +201,16 @@ mod tests { let node = build_spam_list_node(&request); - assert_eq!(node.attrs().string("spam_flow"), "GroupInfoReport"); - assert_eq!(node.attrs().string("jid"), "120363025918861132@g.us"); - assert_eq!(node.attrs().string("subject"), "Test Group"); + assert!( + node.attrs + .get("spam_flow") + .is_some_and(|v| v == "GroupInfoReport") + ); + assert!( + node.attrs + .get("jid") + .is_some_and(|v| v == "120363025918861132@g.us") + ); + assert!(node.attrs.get("subject").is_some_and(|v| v == "Test Group")); } } diff --git a/wacore/tests/binary_protocol_test.rs b/wacore/tests/binary_protocol_test.rs index ce8e32a88..2f6ca6779 100644 --- a/wacore/tests/binary_protocol_test.rs +++ b/wacore/tests/binary_protocol_test.rs @@ -40,8 +40,8 @@ fn test_attr_parser_ref_zero_copy_access() { let node_ref = unmarshal_ref(&marshaled_with_flag[1..]).expect("unmarshal_ref failed"); let mut parser = node_ref.attr_parser(); - assert_eq!(parser.string("xmlns"), "test"); - assert_eq!(parser.optional_string("type"), Some("result")); + assert_eq!(parser.optional_string("xmlns").as_deref(), Some("test")); + assert_eq!(parser.optional_string("type").as_deref(), Some("result")); assert!(parser.ok()); parser .finish()