Update node last seen when events are received (#212)

* Update node last seen timestamps from event receive times

* run rufo

* fix tests

web/app.rb

@@ -731,6 +731,46 @@ def ensure_unknown_node(db, node_ref, fallback_num = nil, heard_time: nil)
   inserted
 end
 
+# Ensure the node's last_seen timestamp reflects the provided receive time.
+#
+# @param db [SQLite3::Database] open database handle.
+# @param node_ref [Object] raw identifier used to resolve the node.
+# @param fallback_num [Object] optional numeric identifier.
+# @param rx_time [Object] receive timestamp that should update the node.
+def touch_node_last_seen(db, node_ref, fallback_num = nil, rx_time: nil)
+  timestamp = coerce_integer(rx_time)
+  return unless timestamp
+
+  node_id = nil
+
+  parts = canonical_node_parts(node_ref, fallback_num)
+  node_id, = parts if parts
+
+  unless node_id
+    trimmed = string_or_nil(node_ref)
+    if trimmed
+      node_id = normalize_node_id(db, trimmed) || trimmed
+    elsif fallback_num
+      fallback_parts = canonical_node_parts(fallback_num, nil)
+      node_id, = fallback_parts if fallback_parts
+    end
+  end
+
+  return unless node_id
+
+  with_busy_retry do
+    db.execute <<~SQL, [timestamp, timestamp, timestamp, node_id]
+                 UPDATE nodes
+                    SET last_heard = CASE
+                      WHEN COALESCE(last_heard, 0) >= ? THEN last_heard
+                      ELSE ?
+                    END,
+                        first_heard = COALESCE(first_heard, ?)
+                  WHERE node_id = ?
+               SQL
+  end
+end
+
 # Insert or update a node row with the most recent metrics.
 #
 # @param db [SQLite3::Database] open database handle.
@@ -974,6 +1014,7 @@ def insert_position(db, payload)
   node_id = canonical if canonical
 
   ensure_unknown_node(db, node_id || node_num, node_num, heard_time: rx_time)
+  touch_node_last_seen(db, node_id || node_num, node_num, rx_time: rx_time)
 
   to_id = string_or_nil(payload["to_id"] || payload["to"])
 
@@ -1123,7 +1164,7 @@ def insert_position(db, payload)
   )
 end
 
-def update_node_from_telemetry(db, node_id, node_num, _rx_time, metrics = {})
+def update_node_from_telemetry(db, node_id, node_num, rx_time, metrics = {})
   num = coerce_integer(node_num)
   id = string_or_nil(node_id)
   if id&.start_with?("!")
@@ -1132,9 +1173,8 @@ def update_node_from_telemetry(db, node_id, node_num, _rx_time, metrics = {})
   id ||= format("!%08x", num & 0xFFFFFFFF) if num
   return unless id
 
-  now = Time.now.to_i
-
-  ensure_unknown_node(db, id, num, heard_time: now)
+  ensure_unknown_node(db, id, num, heard_time: rx_time)
+  touch_node_last_seen(db, id, num, rx_time: rx_time)
 
   battery = coerce_float(metrics[:battery_level] || metrics["battery_level"])
   voltage = coerce_float(metrics[:voltage] || metrics["voltage"])
@@ -1150,12 +1190,6 @@ def update_node_from_telemetry(db, node_id, node_num, _rx_time, metrics = {})
     params << num
   end
 
-  assignments << "last_heard = ?"
-  params << now
-
-  assignments << "first_heard = COALESCE(first_heard, ?)"
-  params << now
-
   metric_updates = {
     "battery_level" => battery,
     "voltage" => voltage,
@@ -1391,9 +1425,7 @@ def insert_message(db, m)
   encrypted = string_or_nil(m["encrypted"])
 
   ensure_unknown_node(db, from_id || raw_from_id, m["from_num"], heard_time: rx_time)
-
-  update_sender_last_heard =
-    encrypted && !encrypted.strip.empty? && from_id && rx_time
+  touch_node_last_seen(db, from_id || raw_from_id || m["from_num"], m["from_num"], rx_time: rx_time)
 
   row = [
     msg_id,
@@ -1464,18 +1496,6 @@ def insert_message(db, m)
       end
     end
   end
-
-  if update_sender_last_heard
-    with_busy_retry do
-      db.execute <<~SQL, [rx_time, rx_time, from_id, rx_time]
-                   UPDATE nodes
-                      SET last_heard = ?,
-                          first_heard = COALESCE(first_heard, ?)
-                    WHERE node_id = ?
-                      AND COALESCE(last_heard, 0) <= ?
-                 SQL
-    end
-  end
 end
 
 # Resolve a node reference to the canonical node ID when possible.

web/spec/app_spec.rb

@@ -897,14 +897,14 @@ RSpec.describe "Potato Mesh Sinatra app" do
           expect_same_value(metrics_node["channel_utilization"], payload[0]["device_metrics"]["channelUtilization"])
           expect_same_value(metrics_node["air_util_tx"], payload[0]["device_metrics"]["airUtilTx"])
           expect(metrics_node["uptime_seconds"]).to eq(payload[0]["device_metrics"]["uptimeSeconds"])
-          expect(metrics_node["last_heard"]).to eq(reference_time.to_i)
-          expect(metrics_node["first_heard"]).to eq(reference_time.to_i)
+          expect(metrics_node["last_heard"]).to eq(payload[0]["rx_time"])
+          expect(metrics_node["first_heard"]).to eq(payload[0]["rx_time"])
 
           env_node = db.get_first_row(
             "SELECT last_heard, battery_level, voltage FROM nodes WHERE node_id = ?",
             [payload[1]["node_id"]],
           )
-          expect(env_node["last_heard"]).to eq(reference_time.to_i)
+          expect(env_node["last_heard"]).to eq(payload[1]["rx_time"])
           expect(env_node["battery_level"]).to be_nil
           expect(env_node["voltage"]).to be_nil
 
@@ -914,7 +914,7 @@ RSpec.describe "Potato Mesh Sinatra app" do
           )
           expect_same_value(local_node["battery_level"], payload[2]["device_metrics"]["battery_level"])
           expect(local_node["uptime_seconds"]).to eq(payload[2]["device_metrics"]["uptime_seconds"])
-          expect(local_node["last_heard"]).to eq(reference_time.to_i)
+          expect(local_node["last_heard"]).to eq(payload[2]["rx_time"])
         end
       end
 
@@ -1107,7 +1107,7 @@ RSpec.describe "Potato Mesh Sinatra app" do
           [sender_id],
         )
 
-        expect(node_row["last_heard"]).to eq(reference_time.to_i)
+        expect(node_row["last_heard"]).to eq(payload["rx_time"])
       end
 
       get "/api/messages"
@@ -1118,6 +1118,44 @@ RSpec.describe "Potato Mesh Sinatra app" do
       expect(messages).to be_empty
     end
 
+    it "updates node last_heard for plaintext messages" do
+      node_id = "!plainmsg01"
+      initial_first = reference_time.to_i - 600
+      initial_last = reference_time.to_i - 300
+
+      with_db do |db|
+        db.execute(
+          "INSERT INTO nodes(node_id, last_heard, first_heard) VALUES (?,?,?)",
+          [node_id, initial_last, initial_first],
+        )
+      end
+
+      rx_time = reference_time.to_i - 120
+      payload = {
+        "packet_id" => 888_001,
+        "rx_time" => rx_time,
+        "rx_iso" => Time.at(rx_time).utc.iso8601,
+        "from_id" => node_id,
+        "text" => "plaintext update",
+      }
+
+      post "/api/messages", payload.to_json, auth_headers
+
+      expect(last_response).to be_ok
+      expect(JSON.parse(last_response.body)).to eq("status" => "ok")
+
+      with_db(readonly: true) do |db|
+        db.results_as_hash = true
+        row = db.get_first_row(
+          "SELECT last_heard, first_heard FROM nodes WHERE node_id = ?",
+          [node_id],
+        )
+
+        expect(row["last_heard"]).to eq(rx_time)
+        expect(row["first_heard"]).to eq(initial_first)
+      end
+    end
+
     it "stores messages containing SQL control characters without executing them" do
       payload = {
         "packet_id" => 404,
@@ -1315,6 +1353,38 @@ RSpec.describe "Potato Mesh Sinatra app" do
         node_aliases[num.to_s] ||= canonical
       end
 
+      latest_rx_by_node = {}
+      messages_fixture.each do |message|
+        rx_time = message["rx_time"]
+        next unless rx_time
+
+        canonical = nil
+        from_id = message["from_id"]
+
+        if from_id.is_a?(String)
+          trimmed = from_id.strip
+          unless trimmed.empty?
+            if trimmed.match?(/\A[0-9]+\z/)
+              canonical = node_aliases[trimmed] || trimmed
+            else
+              canonical = trimmed
+            end
+          end
+        end
+
+        canonical ||= message.dig("node", "node_id")
+
+        if canonical.nil?
+          num = message.dig("node", "num")
+          canonical = node_aliases[num.to_s] if num
+        end
+
+        next unless canonical
+
+        existing = latest_rx_by_node[canonical]
+        latest_rx_by_node[canonical] = [existing, rx_time].compact.max
+      end
+
       messages_fixture.each do |message|
         expected = message.reject { |key, _| key == "node" }
         actual_row = actual_by_id.fetch(message["id"])
@@ -1367,7 +1437,12 @@ RSpec.describe "Potato Mesh Sinatra app" do
           expect_same_value(node_actual["snr"], node_expected["snr"])
           expect_same_value(node_actual["battery_level"], node_expected["battery_level"])
           expect_same_value(node_actual["voltage"], node_expected["voltage"])
-          expect(node_actual["last_heard"]).to eq(node_expected["last_heard"])
+          expected_last_heard = node_expected["last_heard"]
+          latest_rx = latest_rx_by_node[node_expected["node_id"]]
+          if latest_rx
+            expected_last_heard = [expected_last_heard, latest_rx].compact.max
+          end
+          expect(node_actual["last_heard"]).to eq(expected_last_heard)
           expect(node_actual["first_heard"]).to eq(node_expected["first_heard"])
           expect_same_value(node_actual["latitude"], node_expected["latitude"])
           expect_same_value(node_actual["longitude"], node_expected["longitude"])