Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion src/ax_devil_mqtt/core/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,9 @@ def __init__(
If create_publisher is False, provide a topic to subscribe to an existing publisher.
You can also inject an existing RawMqttClient or TemporaryAnalyticsMQTTPublisher for testing.
"""
topic_suffix = hashlib.sha256(analytics_data_source_key.encode()).hexdigest()[:8]
device_host = self._resolve_device_host(device_config, publisher)
hash_input = f"{analytics_data_source_key}:{device_host}"

Copilot AI Feb 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When device_host resolves to an empty string (e.g., device_config=None and no usable host on the injected publisher), the new hash_input = f"{analytics_data_source_key}:{device_host}" changes the computed suffix compared to the previous behavior (sha256(analytics_data_source_key)). This breaks determinism/backwards-compatibility for callers that previously relied on the old hash in the no-host case. Consider only adding the :device_host portion when a non-empty host is available (or otherwise keep the old hashing input when host is empty).

Suggested change
hash_input = f"{analytics_data_source_key}:{device_host}"
if device_host:
hash_input = f"{analytics_data_source_key}:{device_host}"
else:
# Preserve previous behavior when no device host is available
hash_input = analytics_data_source_key

Copilot uses AI. Check for mistakes.
topic_suffix = hashlib.sha256(hash_input.encode()).hexdigest()[:8]
self.topic: str = topic or f"ax-devil/temp/{topic_suffix}"
self._publisher: Optional[TemporaryAnalyticsMQTTPublisher] = None
self._client: RawMqttClient
Expand Down Expand Up @@ -231,6 +233,22 @@ def __init__(
broker_password=broker_password,
)

@staticmethod
def _resolve_device_host(
device_config: Optional[DeviceConfig],
publisher: Optional[TemporaryAnalyticsMQTTPublisher],
) -> str:
"""Resolve device host used in topic hashing."""
if device_config and getattr(device_config, "host", ""):
return str(device_config.host)

if publisher and getattr(publisher, "client", None):
publisher_device_config = getattr(publisher.client, "device_config", None)
if publisher_device_config and getattr(publisher_device_config, "host", ""):
return str(publisher_device_config.host)

return ""

def start(self) -> None:
"""Start listening for analytics messages."""
self._client.start()
Expand Down
74 changes: 73 additions & 1 deletion tests/test_message_processing.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,20 @@ def username_pw_set(self, username, password=None):


class DummyAnalyticsPublisher:
def __init__(self):
def __init__(self, host: str | None = None):
self.cleaned = False
if host:
self.client = type("PublisherClient", (), {"device_config": DummyDeviceConfig(host=host)})()

def cleanup(self):
self.cleaned = True


class DummyDeviceConfig:
def __init__(self, host: str):
self.host = host


def test_mqtt_client_dispatch_basic():
processed_messages = []

Expand Down Expand Up @@ -194,3 +201,68 @@ def stop(self_inner):
assert started["value"] is True
assert started["stopped"] is True
assert dummy_publisher.cleaned is True


def test_analytics_topic_hash_changes_with_device_ip():
client_a = AxisAnalyticsMqttClient(
broker_host="broker",
broker_port=1883,
device_config=DummyDeviceConfig(host="192.168.0.10"),
analytics_data_source_key="stream-key",
message_callback=lambda _: None,
create_publisher=False,
)
client_b = AxisAnalyticsMqttClient(
broker_host="broker",
broker_port=1883,
device_config=DummyDeviceConfig(host="192.168.0.11"),
analytics_data_source_key="stream-key",
message_callback=lambda _: None,
create_publisher=False,
)

assert client_a.topic != client_b.topic


def test_analytics_topic_hash_is_stable_for_same_stream_and_device_ip():
client_a = AxisAnalyticsMqttClient(
broker_host="broker",
broker_port=1883,
device_config=DummyDeviceConfig(host="192.168.0.10"),
analytics_data_source_key="stream-key",
message_callback=lambda _: None,
create_publisher=False,
)
client_b = AxisAnalyticsMqttClient(
broker_host="broker",
broker_port=1883,
device_config=DummyDeviceConfig(host="192.168.0.10"),
analytics_data_source_key="stream-key",
message_callback=lambda _: None,
create_publisher=False,
)

assert client_a.topic == client_b.topic


def test_analytics_topic_hash_uses_publisher_device_ip_when_device_config_missing():
client_a = AxisAnalyticsMqttClient(
broker_host="broker",
broker_port=1883,
device_config=None,
analytics_data_source_key="stream-key",
message_callback=lambda _: None,
create_publisher=False,
publisher=DummyAnalyticsPublisher(host="192.168.0.20"),
)
client_b = AxisAnalyticsMqttClient(
broker_host="broker",
broker_port=1883,
device_config=None,
analytics_data_source_key="stream-key",
message_callback=lambda _: None,
create_publisher=False,
publisher=DummyAnalyticsPublisher(host="192.168.0.21"),
)

assert client_a.topic != client_b.topic

Copilot AI Feb 17, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The added topic-hash tests cover (1) different device_config hosts and (2) fallback to an injected publisher host, but they don’t cover the remaining branch where both device_config and a usable publisher.client.device_config.host are missing. Adding a test for that case would protect the intended fallback behavior and (if desired) enforce backwards-compatible hashing when no host is available.

Suggested change
assert client_a.topic != client_b.topic
assert client_a.topic != client_b.topic
def test_analytics_topic_hash_is_stable_when_no_device_ip_available():
"""
When neither device_config nor a usable publisher host is available,
the topic hash should fall back to the legacy behavior and be stable
for the same stream key.
"""
client_a = AxisAnalyticsMqttClient(
broker_host="broker",
broker_port=1883,
device_config=None,
analytics_data_source_key="stream-key",
message_callback=lambda _: None,
create_publisher=False,
)
client_b = AxisAnalyticsMqttClient(
broker_host="broker",
broker_port=1883,
device_config=None,
analytics_data_source_key="stream-key",
message_callback=lambda _: None,
create_publisher=False,
)
# With no device IP available from either device_config or publisher,
# the topic hash should be deterministic and identical for the same
# stream key, preserving backwards-compatible behavior.
assert client_a.topic == client_b.topic

Copilot uses AI. Check for mistakes.