From 2328d156e6ca3c7782071a5e10619d5ce7b1a078 Mon Sep 17 00:00:00 2001
From: Samuel Holland <samuel@sholland.org>
Date: Tue, 18 Nov 2025 02:32:03 -0600
Subject: [PATCH] Fix auto_map_topic_alias_send logic

Previously, it would always remove the topic name when adding the alias,
even the first time, so the mapping would not be set on the server, and
the server would reject the packet.
---
 src/mqtt/connection/core.rs          |  2 +-
 src/mqtt/packet/v5_0/publish.rs      | 39 ++++++++++++++++++++++++++++
 tests/connection-core-topic-alias.rs | 12 ++++-----
 3 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/src/mqtt/connection/core.rs b/src/mqtt/connection/core.rs
index e3d2233..934366b 100644
--- a/src/mqtt/connection/core.rs
+++ b/src/mqtt/connection/core.rs
@@ -1630,7 +1630,7 @@ where
                     } else {
                         let lru_ta = topic_alias_send.get_lru_alias();
                         topic_alias_send.insert_or_update(packet.topic_name(), lru_ta);
-                        packet = packet.remove_topic_add_topic_alias(lru_ta);
+                        packet = packet.add_topic_alias(lru_ta);
                     }
                 }
             } else if self.auto_replace_topic_alias_send {
diff --git a/src/mqtt/packet/v5_0/publish.rs b/src/mqtt/packet/v5_0/publish.rs
index 7223f68..7fafc2c 100644
--- a/src/mqtt/packet/v5_0/publish.rs
+++ b/src/mqtt/packet/v5_0/publish.rs
@@ -688,6 +688,45 @@ where
         // Set topic name to empty string
         self.topic_name_buf = MqttString::new("").unwrap();
 
+        self.add_topic_alias(topic_alias)
+    }
+
+    /// Add TopicAlias property
+    ///
+    /// This method adds a TopicAlias property to the packet properties. This
+    /// is required to associate a topic alias with a topic name on the server.
+    /// Future messages can use only the topic alias and omit the topic name.
+    /// This is useful for reducing packet size when sending multiple messages
+    /// to the same topic.
+    ///
+    /// The method removes any existing TopicAlias property before adding the new one
+    /// to ensure only one TopicAlias property exists. It automatically recalculates
+    /// the property_length and remaining_length to reflect these changes.
+    ///
+    /// # Parameters
+    ///
+    /// - `topic_alias`: The numeric topic alias to associate with the topic name
+    ///
+    /// # Returns
+    ///
+    /// The modified `GenericPublish` instance with the TopicAlias property
+    ///
+    /// # Examples
+    ///
+    /// ```ignore
+    /// use mqtt_protocol_core::mqtt;
+    ///
+    /// let publish_with_topic = mqtt::packet::v5_0::Publish::builder()
+    ///     .topic_name("sensors/temperature/room1")
+    ///     .unwrap()
+    ///     .payload(b"23.5")
+    ///     .build()
+    ///     .unwrap();
+    ///
+    /// let publish_with_alias = publish_with_topic.add_topic_alias(42);
+    /// // props() now contains TopicAlias property with value 42
+    /// ```
+    pub fn add_topic_alias(mut self, topic_alias: TopicAliasType) -> Self {
         // Add TopicAlias property to the end of properties
         let topic_alias_property =
             Property::TopicAlias(crate::mqtt::packet::TopicAlias::new(topic_alias).unwrap());
diff --git a/tests/connection-core-topic-alias.rs b/tests/connection-core-topic-alias.rs
index 1331ff6..96d0db3 100644
--- a/tests/connection-core-topic-alias.rs
+++ b/tests/connection-core-topic-alias.rs
@@ -71,8 +71,8 @@ fn auto_map_topic_alias_send() {
             ..
         } = event
         {
-            // Verify topic name is empty and topic alias is 1
-            if p.topic_name().is_empty() {
+            // Verify topic name is present and topic alias is 1
+            if p.topic_name() == "topic/a" {
                 // Check for TopicAlias property with value 1
                 for prop in p.props().iter() {
                     if let mqtt::packet::Property::TopicAlias(ta) = prop {
@@ -87,7 +87,7 @@ fn auto_map_topic_alias_send() {
     }
     assert!(
         publish_a_mapped,
-        "PUBLISH A should have empty topic name and TopicAlias=1"
+        "PUBLISH A should have valid topic name and TopicAlias=1"
     );
 
     // Send QoS0 PUBLISH B
@@ -109,8 +109,8 @@ fn auto_map_topic_alias_send() {
             ..
         } = event
         {
-            // Verify topic name is empty and topic alias is 2
-            if p.topic_name().is_empty() {
+            // Verify topic name is present and topic alias is 2
+            if p.topic_name() == "topic/b" {
                 // Check for TopicAlias property with value 2
                 for prop in p.props().iter() {
                     if let mqtt::packet::Property::TopicAlias(ta) = prop {
@@ -125,7 +125,7 @@ fn auto_map_topic_alias_send() {
     }
     assert!(
         publish_b_mapped,
-        "PUBLISH B should have empty topic name and TopicAlias=2"
+        "PUBLISH B should have valid topic name and TopicAlias=2"
     );
 
     {