diff --git a/web/lib/potato_mesh/application/data_processing.rb b/web/lib/potato_mesh/application/data_processing.rb index 59d8dbc..22cb39b 100644 --- a/web/lib/potato_mesh/application/data_processing.rb +++ b/web/lib/potato_mesh/application/data_processing.rb @@ -352,13 +352,20 @@ module PotatoMesh user = n["user"] || {} met = n["deviceMetrics"] || {} pos = n["position"] || {} - role = user["role"] || "CLIENT" + # nil when user info absent; COALESCE in the conflict clause preserves + # the stored role rather than overwriting with a default. + role = user["role"] lh = coerce_integer(n["lastHeard"]) pt = coerce_integer(pos["time"]) now = Time.now.to_i pt = nil if pt && pt > now lh = now if lh && lh > now - lh = pt if pt && (!lh || lh < pt) + # 0 is truthy in Ruby — `lh ||= now` won't replace it, leaving the + # 7-day list filter to evaluate `0 >= now-7days` → false (node hidden). + lh = nil if lh && lh <= 0 + # position.time = 0 means no GPS fix; skip it as a last_heard anchor + # (would re-introduce the same zero-timestamp exclusion bug for lh). + lh = pt if pt && pt > 0 && (!lh || lh < pt) lh ||= now node_num = resolve_node_num(node_id, n) @@ -425,8 +432,14 @@ module PotatoMesh position_time,location_source,precision_bits,latitude,longitude,altitude,lora_freq,modem_preset,protocol) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?) ON CONFLICT(node_id) DO UPDATE SET - num=excluded.num, short_name=excluded.short_name, long_name=#{long_name_conflict_sql}, macaddr=excluded.macaddr, - hw_model=excluded.hw_model, role=excluded.role, public_key=excluded.public_key, is_unmessagable=excluded.is_unmessagable, + num=COALESCE(excluded.num, nodes.num), + short_name=COALESCE(excluded.short_name, nodes.short_name), + long_name=#{long_name_conflict_sql}, + macaddr=COALESCE(excluded.macaddr, nodes.macaddr), + hw_model=COALESCE(excluded.hw_model, nodes.hw_model), + role=COALESCE(excluded.role, nodes.role), + public_key=COALESCE(excluded.public_key, nodes.public_key), + is_unmessagable=COALESCE(excluded.is_unmessagable, nodes.is_unmessagable), is_favorite=excluded.is_favorite, hops_away=excluded.hops_away, snr=excluded.snr, last_heard=excluded.last_heard, first_heard=COALESCE(nodes.first_heard, excluded.first_heard, excluded.last_heard), battery_level=excluded.battery_level, voltage=excluded.voltage, channel_utilization=excluded.channel_utilization, diff --git a/web/spec/data_processing_spec.rb b/web/spec/data_processing_spec.rb index 414e1e7..f9b9a79 100644 --- a/web/spec/data_processing_spec.rb +++ b/web/spec/data_processing_spec.rb @@ -136,14 +136,10 @@ RSpec.describe PotatoMesh::App::DataProcessing do end end - # --------------------------------------------------------------------------- - # insert_telemetry (telemetry_type validation path) - # --------------------------------------------------------------------------- - describe "#insert_telemetry" do + shared_context "with isolated db" do around do |example| Dir.mktmpdir("dp-spec-") do |dir| db_path = File.join(dir, "mesh.db") - RSpec::Mocks.with_temporary_scope do allow(PotatoMesh::Config).to receive(:db_path).and_return(db_path) allow(PotatoMesh::Config).to receive(:db_busy_timeout_ms).and_return(5000) @@ -164,7 +160,35 @@ RSpec.describe PotatoMesh::App::DataProcessing do db end + # Return the full node row for the canonical test node ID. + def read_node(db) + db.execute("SELECT * FROM nodes WHERE node_id = '!aabbccdd'").first + end + + # Insert the canonical test node with full user info and CLIENT_BASE role. + def seed_node(db) + dp.upsert_node(db, "!aabbccdd", { + "lastHeard" => now - 100, + "num" => 0xaabbccdd, + "user" => { + "role" => "CLIENT_BASE", + "longName" => "Real Long Name", + "shortName" => "RLN", + "macaddr" => "aa:bb:cc:dd:ee:ff", + "hwModel" => "TBEAM", + "publicKey" => "abc123", + }, + }) + end + let(:now) { Time.now.to_i } + end + + # --------------------------------------------------------------------------- + # insert_telemetry (telemetry_type validation path) + # --------------------------------------------------------------------------- + describe "#insert_telemetry" do + include_context "with isolated db" let(:valid_payload) do { @@ -209,4 +233,143 @@ RSpec.describe PotatoMesh::App::DataProcessing do expect(result).to be_nil end end + + # --------------------------------------------------------------------------- + # upsert_node — Bug 1: lastHeard = 0 must not be stored as 0 + # --------------------------------------------------------------------------- + describe "#upsert_node — last_heard zero handling" do + include_context "with isolated db" + + it "treats lastHeard=0 as absent, storing approximately now instead" do + db = open_db + dp.upsert_node(db, "!aabbccdd", { "lastHeard" => 0, "num" => 0xaabbccdd }) + lh = read_node(db)["last_heard"] + db.close + expect(lh).not_to eq(0) + expect(lh).to be_within(5).of(now) + end + + it "uses position time as last_heard fallback when lastHeard is 0" do + pt = now - 300 + db = open_db + dp.upsert_node(db, "!aabbccdd", { + "lastHeard" => 0, + "num" => 0xaabbccdd, + "position" => { "time" => pt, "latitude" => 1.0, "longitude" => 2.0 }, + }) + expect(read_node(db)["last_heard"]).to eq(pt) + db.close + end + + it "stores a positive lastHeard value as-is" do + lh = now - 120 + db = open_db + dp.upsert_node(db, "!aabbccdd", { "lastHeard" => lh, "num" => 0xaabbccdd }) + expect(read_node(db)["last_heard"]).to eq(lh) + db.close + end + + it "includes the node in query_nodes results after lastHeard=0 fix" do + db = open_db + dp.upsert_node(db, "!aabbccdd", { "lastHeard" => 0, "num" => 0xaabbccdd }) + db.close + # query_nodes applies a 7-day floor; the node must appear + expect(dp.query_nodes(100).map { |n| n["node_id"] }).to include("!aabbccdd") + end + end + + # --------------------------------------------------------------------------- + # upsert_node — Bug 2: role must not be reset by no-user packets + # --------------------------------------------------------------------------- + describe "#upsert_node — role preservation" do + include_context "with isolated db" + + it "preserves CLIENT_BASE role when a no-user packet arrives" do + db = open_db + seed_node(db) + dp.upsert_node(db, "!aabbccdd", { "lastHeard" => now, "num" => 0xaabbccdd }) + expect(read_node(db)["role"]).to eq("CLIENT_BASE") + db.close + end + + it "updates role when an explicit role is supplied" do + db = open_db + seed_node(db) + dp.upsert_node(db, "!aabbccdd", { + "lastHeard" => now, + "num" => 0xaabbccdd, + "user" => { "role" => "CLIENT", "longName" => "Real Long Name", "shortName" => "RLN" }, + }) + expect(read_node(db)["role"]).to eq("CLIENT") + db.close + end + + it "stores NULL role for new nodes without user info (display layer supplies CLIENT)" do + db = open_db + dp.upsert_node(db, "!aabbccdd", { "lastHeard" => now, "num" => 0xaabbccdd }) + # NULL is stored; query_nodes applies r["role"] ||= "CLIENT" for display + expect(read_node(db)["role"]).to be_nil + db.close + end + end + + # --------------------------------------------------------------------------- + # upsert_node — Bug 3: identity fields must not be overwritten with NULL + # --------------------------------------------------------------------------- + describe "#upsert_node — identity field preservation" do + include_context "with isolated db" + + it "preserves all identity fields when a no-user packet arrives" do + db = open_db + seed_node(db) + dp.upsert_node(db, "!aabbccdd", { "lastHeard" => now, "num" => 0xaabbccdd }) + row = read_node(db) + db.close + expect(row["short_name"]).to eq("RLN") + expect(row["long_name"]).to eq("Real Long Name") + expect(row["macaddr"]).to eq("aa:bb:cc:dd:ee:ff") + expect(row["hw_model"]).to eq("TBEAM") + expect(row["public_key"]).to eq("abc123") + end + + it "updates short_name when a real value is supplied" do + db = open_db + seed_node(db) + dp.upsert_node(db, "!aabbccdd", { + "lastHeard" => now, + "num" => 0xaabbccdd, + "user" => { "shortName" => "NEW", "longName" => "New Long Name" }, + }) + expect(read_node(db)["short_name"]).to eq("NEW") + db.close + end + + it "does not overwrite a real long_name with a generic placeholder" do + db = open_db + seed_node(db) + dp.upsert_node(db, "!aabbccdd", { + "lastHeard" => now, + "num" => 0xaabbccdd, + "user" => { "longName" => "Meshtastic CCDD", "shortName" => "RLN" }, + }) + expect(read_node(db)["long_name"]).to eq("Real Long Name") + db.close + end + + it "overwrites a generic placeholder long_name with a real long_name" do + db = open_db + dp.upsert_node(db, "!aabbccdd", { + "lastHeard" => now - 100, + "num" => 0xaabbccdd, + "user" => { "longName" => "Meshtastic CCDD", "shortName" => "CCDD" }, + }) + dp.upsert_node(db, "!aabbccdd", { + "lastHeard" => now, + "num" => 0xaabbccdd, + "user" => { "longName" => "Real Long Name", "shortName" => "RLN" }, + }) + expect(read_node(db)["long_name"]).to eq("Real Long Name") + db.close + end + end end