mirror of
https://github.com/jkingsman/Remote-Terminal-for-MeshCore.git
synced 2026-05-01 11:02:56 +02:00
Fix issue where last_seen is incremented by events that definitely shouldn't increment it. Fixes #201.
This commit is contained in:
@@ -237,7 +237,9 @@ async def on_new_contact(event: "Event") -> None:
|
||||
logger.debug("New contact: %s", public_key[:12])
|
||||
|
||||
contact_upsert = ContactUpsert.from_radio_dict(public_key.lower(), payload, on_radio=False)
|
||||
contact_upsert.last_seen = int(time.time())
|
||||
# Intentionally do not set last_seen here: NEW_CONTACT fires from the
|
||||
# radio's stored contact DB, not an RF observation. last_seen means
|
||||
# "last time we heard this pubkey on RF".
|
||||
await ContactRepository.upsert(contact_upsert)
|
||||
promoted_keys = await promote_prefix_contacts_for_contact(
|
||||
public_key=public_key,
|
||||
|
||||
@@ -94,7 +94,12 @@ class ContactRepository:
|
||||
last_advert = COALESCE(excluded.last_advert, contacts.last_advert),
|
||||
lat = COALESCE(excluded.lat, contacts.lat),
|
||||
lon = COALESCE(excluded.lon, contacts.lon),
|
||||
last_seen = excluded.last_seen,
|
||||
last_seen = CASE
|
||||
WHEN excluded.last_seen IS NULL THEN contacts.last_seen
|
||||
WHEN contacts.last_seen IS NULL THEN excluded.last_seen
|
||||
WHEN excluded.last_seen > contacts.last_seen THEN excluded.last_seen
|
||||
ELSE contacts.last_seen
|
||||
END,
|
||||
on_radio = COALESCE(excluded.on_radio, contacts.on_radio),
|
||||
last_contacted = COALESCE(excluded.last_contacted, contacts.last_contacted),
|
||||
first_seen = COALESCE(contacts.first_seen, excluded.first_seen)
|
||||
@@ -114,7 +119,7 @@ class ContactRepository:
|
||||
contact_row.last_advert,
|
||||
contact_row.lat,
|
||||
contact_row.lon,
|
||||
contact_row.last_seen if contact_row.last_seen is not None else int(time.time()),
|
||||
contact_row.last_seen,
|
||||
contact_row.on_radio,
|
||||
contact_row.last_contacted,
|
||||
contact_row.first_seen,
|
||||
@@ -339,6 +344,15 @@ class ContactRepository:
|
||||
path_hash_mode: int | None = None,
|
||||
updated_at: int | None = None,
|
||||
) -> None:
|
||||
"""Persist a learned direct route for a contact.
|
||||
|
||||
Both callers (the RF PATH packet processor and the firmware PATH_UPDATE
|
||||
event handler) are RF-backed: firmware ``onContactPathUpdated`` only
|
||||
fires from ``onContactPathRecv`` during RF PATH packet reception. So
|
||||
this method also advances ``last_seen`` monotonically. Never moves
|
||||
``last_seen`` backwards if an out-of-order arrival lands with an older
|
||||
timestamp.
|
||||
"""
|
||||
normalized_path, normalized_path_len, normalized_hash_mode = normalize_contact_route(
|
||||
path,
|
||||
path_len,
|
||||
@@ -349,13 +363,20 @@ class ContactRepository:
|
||||
"""UPDATE contacts SET direct_path = ?, direct_path_len = ?,
|
||||
direct_path_hash_mode = COALESCE(?, direct_path_hash_mode),
|
||||
direct_path_updated_at = ?,
|
||||
last_seen = ? WHERE public_key = ?""",
|
||||
last_seen = CASE
|
||||
WHEN last_seen IS NULL THEN ?
|
||||
WHEN ? > last_seen THEN ?
|
||||
ELSE last_seen
|
||||
END
|
||||
WHERE public_key = ?""",
|
||||
(
|
||||
normalized_path,
|
||||
normalized_path_len,
|
||||
normalized_hash_mode,
|
||||
ts,
|
||||
ts,
|
||||
ts,
|
||||
ts,
|
||||
public_key.lower(),
|
||||
),
|
||||
)
|
||||
@@ -444,11 +465,43 @@ class ContactRepository:
|
||||
|
||||
@staticmethod
|
||||
async def update_last_contacted(public_key: str, timestamp: int | None = None) -> None:
|
||||
"""Update the last_contacted timestamp for a contact."""
|
||||
"""Update the last_contacted timestamp for a contact.
|
||||
|
||||
``last_contacted`` tracks the most recent direct-conversation activity
|
||||
with this contact in either direction (incoming or outgoing DM). It is
|
||||
the field that powers "recent conversations" ordering on the frontend.
|
||||
|
||||
It deliberately does not touch ``last_seen``: ``last_seen`` is reserved
|
||||
for actual RF reception from the contact, and outgoing sends are not
|
||||
evidence that we heard from them. RF observations from DM ingest update
|
||||
``last_seen`` via :meth:`touch_last_seen` on incoming DMs only.
|
||||
"""
|
||||
ts = timestamp if timestamp is not None else int(time.time())
|
||||
await db.conn.execute(
|
||||
"UPDATE contacts SET last_contacted = ?, last_seen = ? WHERE public_key = ?",
|
||||
(ts, ts, public_key.lower()),
|
||||
"UPDATE contacts SET last_contacted = ? WHERE public_key = ?",
|
||||
(ts, public_key.lower()),
|
||||
)
|
||||
await db.conn.commit()
|
||||
|
||||
@staticmethod
|
||||
async def touch_last_seen(public_key: str, timestamp: int) -> None:
|
||||
"""Monotonically bump last_seen for a contact from an RF observation.
|
||||
|
||||
Never moves last_seen backwards; a no-op if the contact row does not
|
||||
exist. Use this from packet-ingest paths that have attributed a packet
|
||||
to a specific contact pubkey (advert, incoming DM, decrypted PATH, etc.).
|
||||
"""
|
||||
await db.conn.execute(
|
||||
"""
|
||||
UPDATE contacts
|
||||
SET last_seen = CASE
|
||||
WHEN last_seen IS NULL THEN ?
|
||||
WHEN ? > last_seen THEN ?
|
||||
ELSE last_seen
|
||||
END
|
||||
WHERE public_key = ?
|
||||
""",
|
||||
(timestamp, timestamp, timestamp, public_key.lower()),
|
||||
)
|
||||
await db.conn.commit()
|
||||
|
||||
|
||||
@@ -252,6 +252,11 @@ async def _store_direct_message(
|
||||
|
||||
if update_last_contacted_key:
|
||||
await contact_repository.update_last_contacted(update_last_contacted_key, received_at)
|
||||
# Incoming DMs are direct RF evidence that this contact transmitted;
|
||||
# outgoing DMs are our own send and must not bump the contact's
|
||||
# last_seen.
|
||||
if not outgoing:
|
||||
await contact_repository.touch_last_seen(update_last_contacted_key, received_at)
|
||||
|
||||
return message
|
||||
|
||||
|
||||
@@ -105,13 +105,15 @@ class TestCreateContact:
|
||||
data = response.json()
|
||||
assert data["public_key"] == KEY_A
|
||||
assert data["name"] == "NewContact"
|
||||
assert data["last_seen"] is not None
|
||||
# Manually created contacts have no RF observation yet, so last_seen
|
||||
# stays NULL until we actually hear them on the air.
|
||||
assert data["last_seen"] is None
|
||||
|
||||
# Verify in DB
|
||||
contact = await ContactRepository.get_by_key(KEY_A)
|
||||
assert contact is not None
|
||||
assert contact.name == "NewContact"
|
||||
assert data["last_seen"] == contact.last_seen
|
||||
assert contact.last_seen is None
|
||||
mock_broadcast.assert_called_once_with("contact", contact.model_dump())
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
@@ -1134,12 +1134,14 @@ class TestOnNewContact:
|
||||
|
||||
await on_new_contact(MockEvent())
|
||||
|
||||
# Verify contact was created in real DB
|
||||
# Verify contact was created in real DB. NEW_CONTACT is the radio's
|
||||
# stored contact DB, not an RF observation, so last_seen stays NULL
|
||||
# until we actually hear the contact on the air.
|
||||
contact = await ContactRepository.get_by_key("cc" * 32)
|
||||
assert contact is not None
|
||||
assert contact.name == "Charlie"
|
||||
assert contact.on_radio is False
|
||||
assert contact.last_seen == 1700000000
|
||||
assert contact.last_seen is None
|
||||
|
||||
mock_broadcast.assert_called_once()
|
||||
event_type, contact_data = mock_broadcast.call_args[0]
|
||||
|
||||
@@ -697,3 +697,126 @@ class TestContactRepositoryUpsertContracts:
|
||||
assert contact.name == "Bob"
|
||||
assert contact.type == 2
|
||||
assert contact.on_radio is True
|
||||
|
||||
|
||||
class TestContactRepositoryLastSeenSemantics:
|
||||
"""Guard the 'last_seen = last RF reception' contract.
|
||||
|
||||
Radio-driven contact-DB syncs must not clobber an earlier real RF timestamp,
|
||||
and callers that don't supply last_seen must leave the existing value alone.
|
||||
"""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_upsert_without_last_seen_preserves_existing(self, test_db):
|
||||
real_rf_observation = 1_700_000_000
|
||||
await ContactRepository.upsert(
|
||||
ContactUpsert(
|
||||
public_key="aa" * 32,
|
||||
name="Alice",
|
||||
type=1,
|
||||
last_seen=real_rf_observation,
|
||||
on_radio=False,
|
||||
)
|
||||
)
|
||||
|
||||
# A subsequent radio-sync style upsert (no last_seen supplied) must not
|
||||
# overwrite the real RF timestamp with now().
|
||||
await ContactRepository.upsert(
|
||||
ContactUpsert(public_key="aa" * 32, name="Alice", type=1, on_radio=False)
|
||||
)
|
||||
|
||||
contact = await ContactRepository.get_by_key("aa" * 32)
|
||||
assert contact is not None
|
||||
assert contact.last_seen == real_rf_observation
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_upsert_monotonically_bumps_last_seen(self, test_db):
|
||||
await ContactRepository.upsert(
|
||||
ContactUpsert(public_key="aa" * 32, last_seen=1_700_000_000, on_radio=False)
|
||||
)
|
||||
|
||||
# Newer RF observation advances last_seen.
|
||||
await ContactRepository.upsert(
|
||||
ContactUpsert(public_key="aa" * 32, last_seen=1_700_000_500, on_radio=False)
|
||||
)
|
||||
contact = await ContactRepository.get_by_key("aa" * 32)
|
||||
assert contact is not None
|
||||
assert contact.last_seen == 1_700_000_500
|
||||
|
||||
# An older timestamp (out-of-order arrival) must not move it backwards.
|
||||
await ContactRepository.upsert(
|
||||
ContactUpsert(public_key="aa" * 32, last_seen=1_699_999_000, on_radio=False)
|
||||
)
|
||||
contact = await ContactRepository.get_by_key("aa" * 32)
|
||||
assert contact is not None
|
||||
assert contact.last_seen == 1_700_000_500
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_upsert_inserts_null_last_seen_when_not_supplied(self, test_db):
|
||||
# A radio-sync-only contact (never heard on RF) should have last_seen=NULL.
|
||||
await ContactRepository.upsert(
|
||||
ContactUpsert(public_key="aa" * 32, name="Alice", type=1, on_radio=False)
|
||||
)
|
||||
|
||||
contact = await ContactRepository.get_by_key("aa" * 32)
|
||||
assert contact is not None
|
||||
assert contact.last_seen is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_touch_last_seen_bumps_monotonically(self, test_db):
|
||||
await ContactRepository.upsert(
|
||||
ContactUpsert(public_key="aa" * 32, last_seen=1_700_000_000, on_radio=False)
|
||||
)
|
||||
|
||||
await ContactRepository.touch_last_seen("aa" * 32, 1_700_000_500)
|
||||
contact = await ContactRepository.get_by_key("aa" * 32)
|
||||
assert contact is not None
|
||||
assert contact.last_seen == 1_700_000_500
|
||||
|
||||
# Older timestamps never move last_seen backwards.
|
||||
await ContactRepository.touch_last_seen("aa" * 32, 1_699_999_000)
|
||||
contact = await ContactRepository.get_by_key("aa" * 32)
|
||||
assert contact is not None
|
||||
assert contact.last_seen == 1_700_000_500
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_last_contacted_does_not_touch_last_seen(self, test_db):
|
||||
# last_contacted = we sent TO them. It must not forge RF reception.
|
||||
await ContactRepository.upsert(
|
||||
ContactUpsert(public_key="aa" * 32, last_seen=1_700_000_000, on_radio=False)
|
||||
)
|
||||
|
||||
await ContactRepository.update_last_contacted("aa" * 32, 1_700_500_000)
|
||||
|
||||
contact = await ContactRepository.get_by_key("aa" * 32)
|
||||
assert contact is not None
|
||||
assert contact.last_contacted == 1_700_500_000
|
||||
assert contact.last_seen == 1_700_000_000
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_direct_path_bumps_last_seen_monotonically(self, test_db):
|
||||
# update_direct_path is driven by RF PATH reception on both callers
|
||||
# (packet processor + firmware PATH_UPDATE, which only fires from
|
||||
# onContactPathRecv during RF reception). It should advance last_seen
|
||||
# forward-only.
|
||||
await ContactRepository.upsert(
|
||||
ContactUpsert(public_key="aa" * 32, last_seen=1_700_000_000, on_radio=False)
|
||||
)
|
||||
|
||||
await ContactRepository.update_direct_path(
|
||||
"aa" * 32, path="ab", path_len=1, path_hash_mode=0, updated_at=1_700_000_500
|
||||
)
|
||||
contact = await ContactRepository.get_by_key("aa" * 32)
|
||||
assert contact is not None
|
||||
assert contact.last_seen == 1_700_000_500
|
||||
assert contact.direct_path == "ab"
|
||||
|
||||
# Out-of-order PATH arrival with an older timestamp must not rewind.
|
||||
await ContactRepository.update_direct_path(
|
||||
"aa" * 32, path="cd", path_len=1, path_hash_mode=0, updated_at=1_699_999_000
|
||||
)
|
||||
contact = await ContactRepository.get_by_key("aa" * 32)
|
||||
assert contact is not None
|
||||
assert contact.last_seen == 1_700_000_500
|
||||
# The path itself still updates — only last_seen is monotonic-guarded.
|
||||
assert contact.direct_path == "cd"
|
||||
|
||||
Reference in New Issue
Block a user