mirror of
https://github.com/jkingsman/Remote-Terminal-for-MeshCore.git
synced 2026-08-07 09:13:04 +02:00
Prevent same-second outgoing collision now that we can send faster.
Also add pending ack tracking
This commit is contained in:
@@ -387,6 +387,7 @@ class TestMessagesEndpoint:
|
||||
_patch_require_connected(mock_mc),
|
||||
patch("app.routers.messages.MessageRepository") as mock_msg_repo,
|
||||
):
|
||||
mock_msg_repo.get_by_content = AsyncMock(return_value=None)
|
||||
# Simulate duplicate - create returns None
|
||||
mock_msg_repo.create = AsyncMock(return_value=None)
|
||||
|
||||
@@ -422,6 +423,7 @@ class TestMessagesEndpoint:
|
||||
_patch_require_connected(mock_mc),
|
||||
patch("app.routers.messages.MessageRepository") as mock_msg_repo,
|
||||
):
|
||||
mock_msg_repo.get_by_content = AsyncMock(return_value=None)
|
||||
# Simulate duplicate - create returns None
|
||||
mock_msg_repo.create = AsyncMock(return_value=None)
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ import pytest
|
||||
|
||||
from app.event_handlers import (
|
||||
_active_subscriptions,
|
||||
_buffered_acks,
|
||||
_pending_acks,
|
||||
cleanup_expired_acks,
|
||||
register_event_handlers,
|
||||
@@ -26,9 +27,11 @@ from app.repository import (
|
||||
def clear_test_state():
|
||||
"""Clear pending ACKs and subscriptions before each test."""
|
||||
_pending_acks.clear()
|
||||
_buffered_acks.clear()
|
||||
_active_subscriptions.clear()
|
||||
yield
|
||||
_pending_acks.clear()
|
||||
_buffered_acks.clear()
|
||||
_active_subscriptions.clear()
|
||||
|
||||
|
||||
@@ -107,6 +110,28 @@ class TestAckTracking:
|
||||
assert len(_pending_acks) == 50
|
||||
assert all(k.startswith("valid_") for k in _pending_acks)
|
||||
|
||||
def test_track_pending_ack_consumes_buffered_ack(self):
|
||||
"""A buffered ACK should be matched immediately when the send registers later."""
|
||||
_buffered_acks["early"] = time.time()
|
||||
|
||||
matched = track_pending_ack("early", message_id=42, timeout_ms=5000)
|
||||
|
||||
assert matched is True
|
||||
assert "early" not in _buffered_acks
|
||||
assert "early" not in _pending_acks
|
||||
|
||||
def test_cleanup_removes_expired_buffered_acks(self):
|
||||
"""Buffered ACKs should expire so unmatched early ACKs do not leak forever."""
|
||||
from app.services.dm_ack_tracker import BUFFERED_ACK_TTL_SECONDS
|
||||
|
||||
_buffered_acks["stale"] = time.time() - (BUFFERED_ACK_TTL_SECONDS + 1)
|
||||
_buffered_acks["fresh"] = time.time()
|
||||
|
||||
cleanup_expired_acks()
|
||||
|
||||
assert "stale" not in _buffered_acks
|
||||
assert "fresh" in _buffered_acks
|
||||
|
||||
|
||||
class TestAckEventHandler:
|
||||
"""Test the on_ack event handler."""
|
||||
@@ -174,6 +199,7 @@ class TestAckEventHandler:
|
||||
|
||||
mock_broadcast.assert_not_called()
|
||||
assert "expected" in _pending_acks
|
||||
assert "different" in _buffered_acks
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ack_empty_code_ignored(self, test_db):
|
||||
@@ -189,6 +215,35 @@ class TestAckEventHandler:
|
||||
|
||||
mock_broadcast.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_buffered_ack_can_be_claimed_after_early_arrival(self, test_db):
|
||||
"""An ACK that arrives before registration should be recoverable."""
|
||||
from app.event_handlers import on_ack
|
||||
|
||||
msg_id = await MessageRepository.create(
|
||||
msg_type="PRIV",
|
||||
text="Hello",
|
||||
received_at=1700000000,
|
||||
conversation_key="aa" * 32,
|
||||
sender_timestamp=1700000000,
|
||||
outgoing=True,
|
||||
)
|
||||
|
||||
with patch("app.event_handlers.broadcast_event") as mock_broadcast:
|
||||
|
||||
class MockEvent:
|
||||
payload = {"code": "earlyack"}
|
||||
|
||||
await on_ack(MockEvent())
|
||||
|
||||
assert "earlyack" in _buffered_acks
|
||||
assert track_pending_ack("earlyack", message_id=msg_id, timeout_ms=10000) is True
|
||||
assert "earlyack" not in _buffered_acks
|
||||
assert "earlyack" not in _pending_acks
|
||||
ack_count, _ = await MessageRepository.get_ack_and_paths(msg_id)
|
||||
assert ack_count == 0
|
||||
mock_broadcast.assert_not_called()
|
||||
|
||||
|
||||
class TestContactMessageCLIFiltering:
|
||||
"""Test that CLI responses (txt_type=1) are filtered out."""
|
||||
|
||||
+128
-5
@@ -24,6 +24,7 @@ from app.routers.messages import (
|
||||
send_channel_message,
|
||||
send_direct_message,
|
||||
)
|
||||
from app.services import dm_ack_tracker
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
@@ -35,6 +36,8 @@ def _reset_radio_state():
|
||||
prev_connection_info = radio_manager._connection_info
|
||||
prev_slot_by_key = radio_manager._channel_slot_by_key.copy()
|
||||
prev_key_by_slot = radio_manager._channel_key_by_slot.copy()
|
||||
prev_pending_acks = dm_ack_tracker._pending_acks.copy()
|
||||
prev_buffered_acks = dm_ack_tracker._buffered_acks.copy()
|
||||
yield
|
||||
radio_manager._meshcore = prev
|
||||
radio_manager._operation_lock = prev_lock
|
||||
@@ -42,6 +45,10 @@ def _reset_radio_state():
|
||||
radio_manager._connection_info = prev_connection_info
|
||||
radio_manager._channel_slot_by_key = prev_slot_by_key
|
||||
radio_manager._channel_key_by_slot = prev_key_by_slot
|
||||
dm_ack_tracker._pending_acks.clear()
|
||||
dm_ack_tracker._pending_acks.update(prev_pending_acks)
|
||||
dm_ack_tracker._buffered_acks.clear()
|
||||
dm_ack_tracker._buffered_acks.update(prev_buffered_acks)
|
||||
|
||||
|
||||
def _make_radio_result(payload=None):
|
||||
@@ -190,6 +197,80 @@ class TestOutgoingDMBroadcast:
|
||||
assert contact_payload["out_path_len"] == 2
|
||||
assert contact_payload["out_path_hash_mode"] == 1
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_dm_same_second_duplicate_bumps_timestamp(self, test_db):
|
||||
mc = _make_mc()
|
||||
pub_key = "fa" * 32
|
||||
await _insert_contact(pub_key, "Alice")
|
||||
|
||||
now = int(time.time())
|
||||
original_id = await MessageRepository.create(
|
||||
msg_type="PRIV",
|
||||
text="hello",
|
||||
conversation_key=pub_key,
|
||||
sender_timestamp=now,
|
||||
received_at=now,
|
||||
outgoing=True,
|
||||
)
|
||||
assert original_id is not None
|
||||
|
||||
with (
|
||||
patch("app.routers.messages.require_connected", return_value=mc),
|
||||
patch.object(radio_manager, "_meshcore", mc),
|
||||
patch("app.routers.messages.broadcast_event"),
|
||||
patch("app.routers.messages.time") as mock_time,
|
||||
):
|
||||
mock_time.time.return_value = float(now)
|
||||
result = await send_direct_message(
|
||||
SendDirectMessageRequest(destination=pub_key, text="hello")
|
||||
)
|
||||
|
||||
assert result.id != original_id
|
||||
assert result.sender_timestamp == now + 1
|
||||
assert result.received_at == now
|
||||
assert mc.commands.send_msg.await_args.kwargs["timestamp"] == now + 1
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_dm_applies_buffered_ack_from_early_arrival(self, test_db):
|
||||
from app.event_handlers import on_ack
|
||||
|
||||
mc = _make_mc()
|
||||
ack_bytes = b"\xde\xad\xbe\xef"
|
||||
result = MagicMock()
|
||||
result.type = EventType.MSG_SENT
|
||||
result.payload = {
|
||||
"expected_ack": ack_bytes,
|
||||
"suggested_timeout": 8000,
|
||||
}
|
||||
mc.commands.send_msg = AsyncMock(return_value=result)
|
||||
|
||||
pub_key = "fb" * 32
|
||||
await _insert_contact(pub_key, "Alice")
|
||||
|
||||
class MockAckEvent:
|
||||
payload = {"code": "deadbeef"}
|
||||
|
||||
broadcasts = []
|
||||
|
||||
def capture_broadcast(event_type, data):
|
||||
broadcasts.append((event_type, data))
|
||||
|
||||
with (
|
||||
patch("app.event_handlers.broadcast_event", side_effect=capture_broadcast),
|
||||
patch("app.routers.messages.require_connected", return_value=mc),
|
||||
patch.object(radio_manager, "_meshcore", mc),
|
||||
patch("app.routers.messages.broadcast_event", side_effect=capture_broadcast),
|
||||
):
|
||||
await on_ack(MockAckEvent())
|
||||
message = await send_direct_message(
|
||||
SendDirectMessageRequest(destination=pub_key, text="Hello")
|
||||
)
|
||||
|
||||
ack_count, _ = await MessageRepository.get_ack_and_paths(message.id)
|
||||
assert ack_count == 1
|
||||
assert message.acked == 1
|
||||
assert any(event_type == "message_acked" for event_type, _data in broadcasts)
|
||||
|
||||
|
||||
class TestOutgoingChannelBroadcast:
|
||||
"""Test that outgoing channel messages are broadcast via broadcast_event for fanout dispatch."""
|
||||
@@ -223,6 +304,42 @@ class TestOutgoingChannelBroadcast:
|
||||
assert data["sender_name"] == "MyNode"
|
||||
assert data["channel_name"] == "#general"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_channel_same_second_duplicate_bumps_timestamp(self, test_db):
|
||||
mc = _make_mc(name="MyNode")
|
||||
chan_key = "ac" * 16
|
||||
await ChannelRepository.upsert(key=chan_key, name="#general")
|
||||
|
||||
now = int(time.time())
|
||||
original_id = await MessageRepository.create(
|
||||
msg_type="CHAN",
|
||||
text="MyNode: hello",
|
||||
conversation_key=chan_key.upper(),
|
||||
sender_timestamp=now,
|
||||
received_at=now,
|
||||
outgoing=True,
|
||||
)
|
||||
assert original_id is not None
|
||||
|
||||
with (
|
||||
patch("app.routers.messages.require_connected", return_value=mc),
|
||||
patch.object(radio_manager, "_meshcore", mc),
|
||||
patch("app.routers.messages.broadcast_event"),
|
||||
patch("app.routers.messages.time") as mock_time,
|
||||
):
|
||||
mock_time.time.return_value = float(now)
|
||||
result = await send_channel_message(
|
||||
SendChannelMessageRequest(channel_key=chan_key, text="hello")
|
||||
)
|
||||
|
||||
assert result.id != original_id
|
||||
assert result.sender_timestamp == now + 1
|
||||
assert result.received_at == now
|
||||
sent_timestamp = int.from_bytes(
|
||||
mc.commands.send_chan_msg.await_args.kwargs["timestamp"], "little"
|
||||
)
|
||||
assert sent_timestamp == now + 1
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_channel_msg_response_includes_current_ack_count(self, test_db):
|
||||
"""Send response reflects latest DB ack count at response time."""
|
||||
@@ -619,8 +736,8 @@ class TestResendChannelMessage:
|
||||
assert "restore failed" in mock_broadcast_error.call_args.args[0].lower()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resend_new_timestamp_collision_returns_original_id(self, test_db):
|
||||
"""When new-timestamp resend collides (same second), return original ID gracefully."""
|
||||
async def test_resend_new_timestamp_collision_bumps_timestamp(self, test_db):
|
||||
"""New-timestamp resend should bump the transmit timestamp instead of reusing the row."""
|
||||
mc = _make_mc(name="MyNode")
|
||||
chan_key = "dd" * 16
|
||||
await ChannelRepository.upsert(key=chan_key, name="#collision")
|
||||
@@ -642,13 +759,19 @@ class TestResendChannelMessage:
|
||||
patch("app.routers.messages.broadcast_event"),
|
||||
patch("app.routers.messages.time") as mock_time,
|
||||
):
|
||||
# Force the same second so MessageRepository.create returns None (duplicate)
|
||||
mock_time.time.return_value = float(now)
|
||||
result = await resend_channel_message(msg_id, new_timestamp=True)
|
||||
|
||||
# Should succeed gracefully, returning the original message ID
|
||||
assert result["status"] == "ok"
|
||||
assert result["message_id"] == msg_id
|
||||
assert result["message_id"] != msg_id
|
||||
resent = await MessageRepository.get_by_id(result["message_id"])
|
||||
assert resent is not None
|
||||
assert resent.sender_timestamp == now + 1
|
||||
assert resent.received_at == now
|
||||
sent_timestamp = int.from_bytes(
|
||||
mc.commands.send_chan_msg.await_args.kwargs["timestamp"], "little"
|
||||
)
|
||||
assert sent_timestamp == now + 1
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resend_non_outgoing_returns_400(self, test_db):
|
||||
|
||||
Reference in New Issue
Block a user