fix duplication for testmsg cog

Author: andyshinn

.vscode/settings.json

@@ -3,5 +3,25 @@
         "tests"
     ],
     "python.testing.unittestEnabled": false,
-    "python.testing.pytestEnabled": true
+    "python.testing.pytestEnabled": true,
+    "workbench.colorCustomizations": {
+        "activityBar.activeBackground": "#1e593f",
+        "activityBar.background": "#1e593f",
+        "activityBar.foreground": "#e7e7e7",
+        "activityBar.inactiveForeground": "#e7e7e799",
+        "activityBarBadge.background": "#221030",
+        "activityBarBadge.foreground": "#e7e7e7",
+        "commandCenter.border": "#e7e7e799",
+        "sash.hoverBorder": "#1e593f",
+        "statusBar.background": "#113324",
+        "statusBar.foreground": "#e7e7e7",
+        "statusBarItem.hoverBackground": "#1e593f",
+        "statusBarItem.remoteBackground": "#113324",
+        "statusBarItem.remoteForeground": "#e7e7e7",
+        "titleBar.activeBackground": "#113324",
+        "titleBar.activeForeground": "#e7e7e7",
+        "titleBar.inactiveBackground": "#11332499",
+        "titleBar.inactiveForeground": "#e7e7e799"
+    },
+    "peacock.color": "#113324"
 }

bridger/cogs/testmsg.py

@@ -27,14 +27,15 @@
 
 
 class TestMsg(commands.GroupCog, name="testmsg"):
+    __test__ = False  # Disable pytest discovery for this cog
     queue = SimpleMemoryCache()
 
     def __init__(self, bot: commands.Bot, discord_channel_id: int, influx_reader: InfluxReader):
         self.bot = bot
         self.discord_channel_id = discord_channel_id
         self.discord_channel = None
         self.influx_reader = influx_reader
-        self.deduplicator = PacketDeduplicator(maxlen=100)
+        self.deduplicator = PacketDeduplicator(maxlen=100, use_gateway_id=True)
 
     @commands.Cog.listener(name="on_ready")
     async def on_ready(self):

bridger/deduplication.py

@@ -6,14 +6,20 @@
 
 
 class PacketDeduplicator:
-    def __init__(self, maxlen: int = 100):
+    def __init__(self, maxlen: int = 100, use_gateway_id: bool = False):
         self.message_queue = deque(maxlen=maxlen)
+        self.use_gateway_id = use_gateway_id
 
     def is_duplicate(self, service_envelope: ServiceEnvelope) -> bool:
         packet_id = service_envelope.packet.id
         gateway_id = service_envelope.gateway_id
 
-        if packet_id in self.message_queue:
+        if self.use_gateway_id:
+            key = (gateway_id, packet_id)
+        else:
+            key = packet_id
+
+        if key in self.message_queue:
             logger.bind(envelope_id=packet_id).opt(colors=True).debug(
                 f"Packet <yellow>{packet_id}</yellow> from <green>{gateway_id}</green> already in queue"
             )
@@ -23,7 +29,14 @@ def is_duplicate(self, service_envelope: ServiceEnvelope) -> bool:
 
     def mark_processed(self, service_envelope: ServiceEnvelope) -> None:
         packet_id = service_envelope.packet.id
-        self.message_queue.append(packet_id)
+        gateway_id = service_envelope.gateway_id
+
+        if self.use_gateway_id:
+            key = (gateway_id, packet_id)
+        else:
+            key = packet_id
+
+        self.message_queue.append(key)
 
     def should_process(self, service_envelope: ServiceEnvelope) -> bool:
         if self.is_duplicate(service_envelope):

tests/test_dataclasses.py

@@ -6,14 +6,14 @@
 
 
 @dataclass
-class TestNodeWithNodeId(NodeMixin):
+class NodeWithNodeId(NodeMixin):
     """Test class with node_id attribute"""
 
     node_id: int
 
 
 @dataclass
-class TestNodeWithFrom(NodeMixin):
+class NodeWithFrom(NodeMixin):
     """Test class with _from attribute (like TelemetryPoint)"""
 
     _from: int
@@ -24,38 +24,38 @@ class TestNodeMixin:
 
     def test_node_hex_id_with_bang_from_node_id(self):
         """Test hex ID conversion from node_id attribute"""
-        node = TestNodeWithNodeId(node_id=439041101)  # 0x1a2b3c4d
+        node = NodeWithNodeId(node_id=439041101)  # 0x1a2b3c4d
         assert node.node_hex_id_with_bang == "!1a2b3c4d"
         assert node.node_hex_id_without_bang == "1a2b3c4d"
         assert node.color == "2b3c4d"
 
     def test_node_hex_id_with_bang_from_from_attribute(self):
         """Test hex ID conversion from _from attribute"""
-        node = TestNodeWithFrom(_from=439041101)  # 0x1a2b3c4d
+        node = NodeWithFrom(_from=439041101)  # 0x1a2b3c4d
         assert node.node_hex_id_with_bang == "!1a2b3c4d"
         assert node.node_hex_id_without_bang == "1a2b3c4d"
         assert node.color == "2b3c4d"
 
     def test_node_hex_id_padding(self):
         """Test that hex IDs are properly zero-padded to 8 characters"""
-        node = TestNodeWithNodeId(node_id=255)  # 0xff
+        node = NodeWithNodeId(node_id=255)  # 0xff
         assert node.node_hex_id_with_bang == "!000000ff"
         assert node.node_hex_id_without_bang == "000000ff"
 
     def test_node_hex_id_large_numbers(self):
         """Test hex ID conversion with large numbers"""
-        node = TestNodeWithNodeId(node_id=4294967295)  # 0xffffffff
+        node = NodeWithNodeId(node_id=4294967295)  # 0xffffffff
         assert node.node_hex_id_with_bang == "!ffffffff"
         assert node.node_hex_id_without_bang == "ffffffff"
 
     def test_node_hex_id_no_attributes_raises_error(self):
         """Test that missing required attributes raises AttributeError"""
 
         @dataclass
-        class TestNodeEmpty(NodeMixin):
+        class NodeEmpty(NodeMixin):
             other_field: str
 
-        node = TestNodeEmpty(other_field="test")
+        node = NodeEmpty(other_field="test")
 
         with pytest.raises(AttributeError) as exc_info:
             _ = node.node_hex_id_with_bang
@@ -66,25 +66,25 @@ def test_priority_from_attribute_over_node_id(self):
         """Test that _from attribute takes priority over node_id"""
 
         @dataclass
-        class TestNodeBoth(NodeMixin):
+        class NodeBoth(NodeMixin):
             _from: int
             node_id: int
 
-        node = TestNodeBoth(_from=439041101, node_id=123456)  # Should use _from
+        node = NodeBoth(_from=439041101, node_id=123456)  # Should use _from
         assert node.node_hex_id_with_bang == "!1a2b3c4d"
 
     def test_color_property_various_scenarios(self):
         """Test color property extraction in various scenarios"""
         # Test with short hex (should take last 6 chars after padding)
-        node1 = TestNodeWithNodeId(node_id=255)  # 0x000000ff
+        node1 = NodeWithNodeId(node_id=255)  # 0x000000ff
         assert node1.color == "0000ff"
 
         # Test with full hex
-        node2 = TestNodeWithNodeId(node_id=0xFFA2B3C4)
+        node2 = NodeWithNodeId(node_id=0xFFA2B3C4)
         assert node2.color == "a2b3c4"
 
         # Test with direct node_id input
-        node3 = TestNodeWithNodeId(node_id=0x12345678)
+        node3 = NodeWithNodeId(node_id=0x12345678)
         assert node3.color == "345678"
 
 

tests/test_deduplication.py

@@ -10,6 +10,11 @@ def deduplicator():
     return PacketDeduplicator(maxlen=3)
 
 
+@pytest.fixture
+def deduplicator_with_gateway_id():
+    return PacketDeduplicator(maxlen=3, use_gateway_id=True)
+
+
 @pytest.fixture
 def service_envelope():
     envelope = MagicMock()
@@ -130,3 +135,58 @@ def test_bounded_deque_allows_reprocessing_evicted_packets(self, deduplicator):
         assert deduplicator.should_process(envelopes[0])
         # Packet 4 should still be in queue and be duplicate
         assert not deduplicator.should_process(envelopes[4])
+
+
+class TestPacketDeduplicatorWithGatewayId:
+    def test_init_with_gateway_id(self, deduplicator_with_gateway_id):
+        assert deduplicator_with_gateway_id.use_gateway_id is True
+        assert deduplicator_with_gateway_id.message_queue.maxlen == 3
+
+    def test_mark_processed_adds_to_queue_with_gateway_id(self, deduplicator_with_gateway_id, service_envelope):
+        deduplicator_with_gateway_id.mark_processed(service_envelope)
+        assert ("!1a2b3c4d", 12345) in deduplicator_with_gateway_id.message_queue
+
+    def test_is_duplicate_with_gateway_id_not_in_queue(self, deduplicator_with_gateway_id, service_envelope):
+        deduplicator_with_gateway_id.message_queue.append(("!different", 99999))
+        assert not deduplicator_with_gateway_id.is_duplicate(service_envelope)
+
+    def test_is_duplicate_with_gateway_id_in_queue(self, deduplicator_with_gateway_id, service_envelope):
+        deduplicator_with_gateway_id.message_queue.append(("!1a2b3c4d", 12345))
+        assert deduplicator_with_gateway_id.is_duplicate(service_envelope)
+
+    def test_should_process_same_packet_different_gateway_with_gateway_id(
+        self, deduplicator_with_gateway_id, service_envelope, service_envelope_different_gateway
+    ):
+        assert deduplicator_with_gateway_id.should_process(service_envelope)
+        assert deduplicator_with_gateway_id.should_process(service_envelope_different_gateway)
+
+    def test_should_process_duplicate_packet_same_gateway_with_gateway_id(
+        self, deduplicator_with_gateway_id, service_envelope
+    ):
+        deduplicator_with_gateway_id.mark_processed(service_envelope)
+        assert not deduplicator_with_gateway_id.should_process(service_envelope)
+
+    def test_bounded_deque_behavior_with_gateway_id(self, deduplicator_with_gateway_id):
+        # Fill queue to capacity (maxlen=3)
+        for i in range(3):
+            envelope = MagicMock()
+            envelope.packet.id = i
+            envelope.gateway_id = "!test"
+            deduplicator_with_gateway_id.mark_processed(envelope)
+
+        assert len(deduplicator_with_gateway_id.message_queue) == 3
+        assert ("!test", 0) in deduplicator_with_gateway_id.message_queue
+        assert ("!test", 1) in deduplicator_with_gateway_id.message_queue
+        assert ("!test", 2) in deduplicator_with_gateway_id.message_queue
+
+        # Add one more, should evict the oldest (0)
+        envelope = MagicMock()
+        envelope.packet.id = 3
+        envelope.gateway_id = "!test"
+        deduplicator_with_gateway_id.mark_processed(envelope)
+
+        assert len(deduplicator_with_gateway_id.message_queue) == 3
+        assert ("!test", 0) not in deduplicator_with_gateway_id.message_queue
+        assert ("!test", 1) in deduplicator_with_gateway_id.message_queue
+        assert ("!test", 2) in deduplicator_with_gateway_id.message_queue
+        assert ("!test", 3) in deduplicator_with_gateway_id.message_queue

tests/test_http.py

@@ -12,8 +12,8 @@ async def test_client(aiohttp_client):
         mock_instance = AsyncMock()
         mock_device.return_value = mock_instance
         app = create_app()
+        app["device"] = mock_instance  # Set state before starting client to avoid deprecation warning
         client = await aiohttp_client(app)
-        app["device"] = mock_instance
         yield client
 
 

tests/test_testmsg.py

@@ -58,19 +58,21 @@ def test_init_creates_deduplicator(self, testmsg_cog):
 
     def test_deduplicator_processes_unique_message(self, testmsg_cog, mock_service_envelope):
         assert testmsg_cog.deduplicator.should_process(mock_service_envelope)
-        assert 12345 in testmsg_cog.deduplicator.message_queue
+        assert ("!1a2b3c4d", 12345) in testmsg_cog.deduplicator.message_queue
 
-    def test_deduplicator_skips_duplicate_message(self, testmsg_cog, mock_service_envelope, mock_service_envelope_duplicate):
+    def test_deduplicator_processes_same_packet_different_gateway(
+        self, testmsg_cog, mock_service_envelope, mock_service_envelope_duplicate
+    ):
         assert testmsg_cog.deduplicator.should_process(mock_service_envelope)
-        assert not testmsg_cog.deduplicator.should_process(mock_service_envelope_duplicate)
+        assert testmsg_cog.deduplicator.should_process(mock_service_envelope_duplicate)
 
     def test_deduplicator_processes_different_messages(
         self, testmsg_cog, mock_service_envelope, mock_service_envelope_different
     ):
         assert testmsg_cog.deduplicator.should_process(mock_service_envelope)
         assert testmsg_cog.deduplicator.should_process(mock_service_envelope_different)
-        assert 12345 in testmsg_cog.deduplicator.message_queue
-        assert 67890 in testmsg_cog.deduplicator.message_queue
+        assert ("!1a2b3c4d", 12345) in testmsg_cog.deduplicator.message_queue
+        assert ("!1a2b3c4d", 67890) in testmsg_cog.deduplicator.message_queue
 
     @patch("bridger.cogs.testmsg.PBPacketProcessor")
     def test_mqtt_processing_skips_duplicate_packets(self, mock_processor_class, testmsg_cog, mock_service_envelope):
@@ -108,7 +110,7 @@ def test_mqtt_processing_handles_unique_packets(self, mock_processor_class, test
             assert processor.data.text == "!test message"
 
     def test_deduplicator_bounded_queue_behavior(self, testmsg_cog):
-        testmsg_cog.deduplicator = PacketDeduplicator(maxlen=3)
+        testmsg_cog.deduplicator = PacketDeduplicator(maxlen=3, use_gateway_id=True)
 
         for i in range(3):
             envelope = MagicMock()
@@ -117,28 +119,28 @@ def test_deduplicator_bounded_queue_behavior(self, testmsg_cog):
             testmsg_cog.deduplicator.mark_processed(envelope)
 
         assert len(testmsg_cog.deduplicator.message_queue) == 3
-        assert 0 in testmsg_cog.deduplicator.message_queue
-        assert 1 in testmsg_cog.deduplicator.message_queue
-        assert 2 in testmsg_cog.deduplicator.message_queue
+        assert ("!test", 0) in testmsg_cog.deduplicator.message_queue
+        assert ("!test", 1) in testmsg_cog.deduplicator.message_queue
+        assert ("!test", 2) in testmsg_cog.deduplicator.message_queue
 
         envelope = MagicMock()
         envelope.packet.id = 3
         envelope.gateway_id = "!test"
         testmsg_cog.deduplicator.mark_processed(envelope)
 
         assert len(testmsg_cog.deduplicator.message_queue) == 3
-        assert 0 not in testmsg_cog.deduplicator.message_queue
-        assert 1 in testmsg_cog.deduplicator.message_queue
-        assert 2 in testmsg_cog.deduplicator.message_queue
-        assert 3 in testmsg_cog.deduplicator.message_queue
+        assert ("!test", 0) not in testmsg_cog.deduplicator.message_queue
+        assert ("!test", 1) in testmsg_cog.deduplicator.message_queue
+        assert ("!test", 2) in testmsg_cog.deduplicator.message_queue
+        assert ("!test", 3) in testmsg_cog.deduplicator.message_queue
 
     @patch("bridger.deduplication.logger")
     def test_deduplicator_logs_duplicate_detection(self, mock_logger, testmsg_cog, mock_service_envelope):
         testmsg_cog.deduplicator.should_process(mock_service_envelope)
         testmsg_cog.deduplicator.is_duplicate(mock_service_envelope)
         mock_logger.bind.assert_called_with(envelope_id=12345)
 
-    def test_integration_deduplication_prevents_discord_spam(self, testmsg_cog):
+    def test_integration_deduplication_processes_different_gateways(self, testmsg_cog):
         envelope1 = MagicMock()
         envelope1.packet.id = 99999
         envelope1.gateway_id = "!gateway1"
@@ -150,5 +152,20 @@ def test_integration_deduplication_prevents_discord_spam(self, testmsg_cog):
         should_process_first = testmsg_cog.deduplicator.should_process(envelope1)
         assert should_process_first
 
+        should_process_second = testmsg_cog.deduplicator.should_process(envelope2)
+        assert should_process_second
+
+    def test_integration_deduplication_prevents_true_duplicates(self, testmsg_cog):
+        envelope1 = MagicMock()
+        envelope1.packet.id = 99999
+        envelope1.gateway_id = "!gateway1"
+
+        envelope2 = MagicMock()
+        envelope2.packet.id = 99999
+        envelope2.gateway_id = "!gateway1"
+
+        should_process_first = testmsg_cog.deduplicator.should_process(envelope1)
+        assert should_process_first
+
         should_process_second = testmsg_cog.deduplicator.should_process(envelope2)
         assert not should_process_second