refactor: simplify NodeValue API to 2 methods, fix AttrParser JID bug

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("<unknown>");
+            let name = collection_node.attrs().optional_string("name");
+            let name = name.as_deref().unwrap_or("<unknown>");
             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,17 +2685,24 @@ 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()),
                 },
             ));
         }
     }
 
     pub(crate) async fn handle_iq(self: &Arc<Self>, 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<Node> {
 /// WA Web omits `type` when ACKing `<notification type="encrypt"><identity/></notification>`.
 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]

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())

src/handlers/iq.rs

@@ -24,7 +24,7 @@ impl StanzaHandler for IqHandler {
 
     async fn handle(&self, client: Arc<Client>, node: Arc<Node>, _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)

src/handlers/notification.rs

@@ -40,11 +40,12 @@ impl StanzaHandler for NotificationHandler {
 }
 
 async fn handle_notification_impl(client: &Arc<Client>, 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<Client>, 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("<unknown>");
+                    let name_cow = collection_node.attrs().optional_string("name");
+                    let name_str = name_cow.as_deref().unwrap_or("<unknown>");
                     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<Client>, 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(|| {

src/handlers/presence.rs

@@ -21,29 +21,15 @@ impl StanzaHandler for PresenceHandler {
     }
 
     async fn handle(&self, client: Arc<Client>, node: Arc<Node>, _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

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 {}. \

src/pair.rs

@@ -153,12 +153,24 @@ async fn handle_pair_success(client: &Arc<Client>, 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

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::<u8>().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);
     }
 }

src/request.rs

@@ -208,7 +208,7 @@ impl Client {
     /// This method accepts an `Arc<Node>` - 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<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 {
             // First check if there's a waiter (without cloning)
             let waiter = self.response_waiters.lock().await.remove(&id);