ensure that component keys are unique for their device

app/models/component.rb

@@ -6,6 +6,9 @@ class Component < ActiveRecord::Base
 
   validates_presence_of :device, :sensor
   validates :sensor_id, :uniqueness => { :scope => [:device_id] }
+  validates :key, :uniqueness => { :scope =>  [:device_id] }
+
+  before_validation :set_key, on: :create
 
   delegate :equation, :reverse_equation, to: :sensor
 
@@ -21,4 +24,19 @@ def normalized_value x
     reverse_equation ? eval( ['->x{',reverse_equation,'}'].join ).call(x) : x
   end
 
+  def get_unique_key(default_key, other_keys)
+    matching_keys = other_keys.select { |k| k =~ /^#{default_key}/ }
+    ix = matching_keys.length
+    ix == 0 ? default_key : "#{default_key}_#{ix}"
+  end
+
+  private
+
+  def set_key
+    if sensor && device && !key
+      default_key = sensor.default_key
+      other_component_keys = device.components.map(&:key)
+      self.key = get_unique_key(default_key, other_component_keys)
+    end
+  end
 end

app/models/concerns/data_parser/storer.rb

@@ -54,19 +54,12 @@ def timestamp_parse(timestamp)
       end
 
       def sensor_reading(device, sensor)
-        begin
-          id = Integer(sensor['id'])
-          key = device.find_sensor_key_by_id(id)
-        rescue
-          key = sensor['id']
-          id = device.find_sensor_id_by_key(key)
-        end
-        component = device.find_or_create_component_by_sensor_id(id)
+        component = device.find_or_create_component_for_sensor_reading(sensor)
         return nil if component.nil?
         value = component.normalized_value( (Float(sensor['value']) rescue sensor['value']) )
         {
-          id: id,
-          key: key,
+          id: component.sensor_id,
+          key: component.key,
           component: component,
           value: value
         }

app/models/device.rb

@@ -87,19 +87,35 @@ def self.ransackable_associations(auth_object = nil)
   end
 
   def sensor_keys
-    # will be changed when different kinds of device added
-    %w(temp bat co hum light nets no2 noise panel)
+    sensor_map.keys
   end
 
   def sensor_map
-    components.map { |c| [c.key || c.sensor.key, c.sensor.id]}.to_h
+    components.map { |c| [c.key, c.sensor.id]}.to_h
   end
 
-  def find_or_create_component_by_sensor_id sensor_id
+  def find_or_create_component_by_sensor_id(sensor_id)
     return nil if sensor_id.nil? || !Sensor.exists?(id: sensor_id)
     components.find_or_create_by(sensor_id: sensor_id)
   end
 
+  def find_or_create_component_by_sensor_key(sensor_key)
+    return nil if sensor_key.nil?
+    sensor = Sensor.find_by(default_key: sensor_key)
+    return nil if sensor.nil?
+    components.find_or_create_by(sensor_id: sensor.id)
+  end
+
+  def find_or_create_component_for_sensor_reading(reading)
+    key_or_id = reading["id"]
+    if key_or_id.is_a?(Integer) || key_or_id =~ /\d+/
+      # It's an integer and therefore a sensor id
+      find_or_create_component_by_sensor_id(key_or_id)
+    else
+      find_or_create_component_by_sensor_key(key_or_id)
+    end
+  end
+
   def find_component_by_sensor_id sensor_id
     components.where(sensor_id: sensor_id).first
   end

app/models/raw_storer.rb

@@ -29,6 +29,7 @@ def initialize data, mac, version, ip, raise_errors=false
         metric = sensor
 
         metric_id = device.find_sensor_id_by_key(metric)
+
         component = device.find_or_create_component_by_sensor_id(metric_id)
         next if component.nil?
 
@@ -53,7 +54,6 @@ def initialize data, mac, version, ip, raise_errors=false
 
       #Kairos.http_post_to("/datapoints", _data)
       Redis.current.publish('telnet_queue', _data.to_json)
-
       sensor_ids = sql_data.select {|k, v| k.is_a?(Integer) }.keys.compact.uniq
       device.update_component_timestamps(parsed_ts, sensor_ids)
 

db/migrate/20230704150532_refactor_kits.rb

@@ -30,12 +30,12 @@ def change
     change_column_null :components, :board_type, true
 
     change_table :sensors do |t|
-      t.column :key, :string, null: true
+      t.column :default_key, :string, null: true
       t.column :equation, :string, null: true
       t.column :reverse_equation, :string, null: true
     end
 
-    # Add default key to sensors:
+    # Add default key to sensors to be used when new components are created:
     puts "-- setting default keys for sensors"
 
     key_counter =  Hash.new { |h, k|
@@ -51,7 +51,7 @@ def change
 
     key_counter.each do |id, keys|
       key = keys.max_by {|k, v| v }[0]
-      execute("UPDATE sensors SET key=? WHERE id=?", [key, id])
+      execute("UPDATE sensors SET default_key=? WHERE id=?", [key, id])
     end
 
 
@@ -83,7 +83,6 @@ def change
     end
 
     # For each existing device. Look up its kit, and create a component for each of that kit's components, with reference to the device itself.
-    # Override the key if it doesn't match that on sensor.
 
     puts "-- creating device components"
 
@@ -101,19 +100,12 @@ def change
           sensor_id = sensor_row["id"]
           created_at = kit_component_row["created_at"]
           updated_at = kit_component_row["updated_at"]
-          #p [sensor_row["key"], sensor_id,  sensor_map.invert[sensor_id]]
-          if sensor_row["key"] != sensor_map.invert[sensor_id]
-            key = sensor_map.invert[sensor_id]
-          end
-          begin
-            execute("""
-              INSERT INTO components
-              (device_id, sensor_id, key, created_at, updated_at)
-              VALUES (?, ?, ?, ?, ?)
-            """, [device_id, sensor_id, key, created_at, updated_at])
-          rescue Exception => e
-            raise e
-          end
+          key = sensor_map.invert[sensor_id]
+          execute("""
+            INSERT INTO components
+            (device_id, sensor_id, key, created_at, updated_at)
+            VALUES (?, ?, ?, ?, ?)
+          """, [device_id, sensor_id, key, created_at, updated_at])
         end
       end
     end
@@ -161,7 +153,7 @@ def change
     # Set constraints on components and sensors for the new device relationshop:
     change_column_null :components, :device_id, false
     add_foreign_key :components, :devices
-
-    change_column_null :sensors, :key, null: false
+    change_column_null :components, :key, null: false
+    change_column_null :sensors, :default_key, null: false
   end
 end

db/schema.rb

@@ -238,7 +238,7 @@
     t.datetime "updated_at", null: false
     t.integer "measurement_id"
     t.uuid "uuid", default: -> { "uuid_generate_v4()" }
-    t.string "key"
+    t.string "default_key"
     t.string "equation"
     t.string "reverse_equation"
     t.index ["ancestry"], name: "index_sensors_on_ancestry"

spec/factories/sensors.rb

@@ -3,5 +3,6 @@
     name { "MiCS-2710" }
     description { "Metaloxide gas sensor" }
     unit { "KΩ" }
+    default_key { "key_#{SecureRandom.alphanumeric(4)}"}
   end
 end

spec/lib/mqtt_messages_handler_spec.rb

@@ -3,13 +3,13 @@
 RSpec.describe MqttMessagesHandler do
   let(:device) { create(:device, device_token: 'aA1234') }
   let(:orphan_device) { create(:orphan_device, device_token: 'xX9876') }
-  let(:component) { build(:component, device: device, sensor: build(:sensor, id: 1)) }
+  let(:component) { build(:component, device: device, sensor: build(:sensor, id: 1, default_key: "key1")) }
 
   let(:device_inventory) { create(:device_inventory, report: '{"random_property": "random_result"}') }
 
   before do
     device.components << component
-    create(:sensor, id: 13, key: "key13")
+    create(:sensor, id: 13, default_key: "key13")
 
 
     @data = [{
@@ -81,7 +81,7 @@
         #expect(Storer).to receive(:initialize).with('a', 'b')
         expect(Redis.current).to receive(:publish).with(
           'telnet_queue', [{
-            name: nil,
+            name: "key1",
             timestamp: 1465381800000,
             value: 21.0,
             tags: {
@@ -141,15 +141,15 @@
     it 'processes raw data' do
       expect(Redis.current).to receive(:publish).with(
         'telnet_queue', [{
-          name: nil,
+          name: "key1",
           timestamp: 1490362514000,
           value: 48.45,
           tags: {
             device_id: device.id,
             method: 'REST'
           }
         },{
-          name: nil,
+          name: "key13",
           timestamp: 1490362514000,
           value: 66.0,
           tags: {

spec/models/component_spec.rb

@@ -11,4 +11,24 @@
     expect{ create(:component, device: component.device, sensor: component.sensor) }.to raise_error(ActiveRecord::RecordInvalid)
   end
 
+  describe "creating a unique sensor key" do
+    let(:component) {
+      create(:component, device: create(:device), sensor: create(:sensor))
+    }
+
+    describe "when the given key is not in the list of existing keys" do
+      it "uses the key as is" do
+        generated_key = component.get_unique_key("key", ["other"])
+        expect(generated_key).to eq("key")
+      end
+    end
+
+    describe "when the given key is in the list of existing keys" do
+      it "adds an incremeting number to the key" do
+        generated_key = component.get_unique_key("key", ["key", "other", "key_1"])
+        expect(generated_key).to eq("key_2")
+      end
+    end
+  end
+
 end

spec/models/device_spec.rb

@@ -30,29 +30,15 @@
   end
 
   describe "#sensor_map" do
-    context "when it has a sensor with its own key" do
-      it "maps the sensor key to the sensor id" do
-        device = create(:device)
-        sensor = create(:sensor, key: "sensor_key")
-        component = create(:component, device: device, sensor: sensor)
-        expect(device.sensor_map).to eq({"sensor_key" => sensor.id})
-      end
-    end
-
-    context "when it has a sensor with an overriden key" do
-      it "maps the component key to the sensor id" do
-        device = create(:device)
-        sensor = create(:sensor, key: "sensor_key")
-        component = create(:component, device: device, sensor: sensor, key: "component_key")
-        expect(device.sensor_map).to eq({"component_key" => sensor.id})
-      end
+    it "maps the component key to the sensor id" do
+      device = create(:device)
+      sensor = create(:sensor, default_key: "sensor_key")
+      component = create(:component, device: device, sensor: sensor, key: "component_key")
+      expect(device.sensor_map).to eq({"component_key" => sensor.id})
     end
-
-
   end
 
   describe "mac_address" do
-
     it "takes mac_address from existing device on update" do
       device = FactoryBot.create(:device, mac_address: mac_address)
       new_device = FactoryBot.create(:device)
@@ -298,9 +284,10 @@
   describe "update_column_timestamps" do
 
     before do
-      @component_1 = create(:component, sensor: create(:sensor, id: 1))
-      @component_2 = create(:component, sensor: create(:sensor, id: 2))
-      @device = create(:device, components: [@component_1, @component_2])
+      @device = create(:device)
+      @component_1 = create(:component, device: @device, sensor: create(:sensor, id: 1))
+      @component_2 = create(:component, device: @device,  sensor: create(:sensor, id: 2))
+      @device.reload
       @timestamp = Time.parse("2023-10-06 06:00:00")
     end
 

spec/models/raw_storer_spec.rb

@@ -6,15 +6,15 @@ def to_ts(time)
 
 RSpec.describe RawStorer, :type => :model do
   before(:each) do
-    Sensor.create!(id: 7, name: "POM-3044P-R", description: "test", key: "noise", equation: "Mathematician.table_calibration({0=>50,2=>55,3=>57,6=>58,20=>59,40=>60,60=>61,75=>62,115=>63,150=>64,180=>65,220=>66,260=>67,300=>68,375=>69,430=>70,500=>71,575=>72,660=>73,720=>74,820=>75,900=>76,975=>77,1050=>78,1125=>79,1200=>80,1275=>81,1320=>82,1375=>83,1400=>84,1430=>85,1450=>86,1480=>87,1500=>88,1525=>89,1540=>90,1560=>91,1580=>92,1600=>93,1620=>94,1640=>95,1660=>96,1680=>97,1690=>98,1700=>99,1710=>100,1720=>101,1745=>102,1770=>103,1785=>104,1800=>105,1815=>106,1830=>107,1845=>108,1860=>109,1875=>110},x)", reverse_equation: "x")
-    Sensor.create!(id: 12, name: "HPP828E031", description: "test", key: "temp", equation: "(175.72 / 65536.0 * x) - 53", reverse_equation: "x")
-    Sensor.create!(id: 13, name: "HPP828E031", description: "test", key: "hum", equation: "(125.0 / 65536.0  * x) + 7", reverse_equation: "x")
-    Sensor.create!(id: 14, name: "BH1730FVC", description: "test", key: "light", equation: "x", reverse_equation: "x/10.0")
-    Sensor.create!(id: 15, name: "MiCS-4514", description: "test", key: "no2", equation: "x", reverse_equation: "x/1000.0")
-    Sensor.create!(id: 16, name: "MiCS-4514", description: "test", key: "co", equation: "x", reverse_equation: "x/1000.0")
-    Sensor.create!(id: 17, name: "Battery", description: "test", key: "bat", equation: "x", reverse_equation: "x/10.0")
-    Sensor.create!(id: 18, name: "Solar Panel", description: "test", key: "panel", equation: "x", reverse_equation: "x/1000.0")
-    Sensor.create!(id: 21, name: "Microchip RN-131", description: "test", key: "nets", equation: "x", reverse_equation: "x")
+    Sensor.create!(id: 7, name: "POM-3044P-R", description: "test", default_key: "noise", equation: "Mathematician.table_calibration({0=>50,2=>55,3=>57,6=>58,20=>59,40=>60,60=>61,75=>62,115=>63,150=>64,180=>65,220=>66,260=>67,300=>68,375=>69,430=>70,500=>71,575=>72,660=>73,720=>74,820=>75,900=>76,975=>77,1050=>78,1125=>79,1200=>80,1275=>81,1320=>82,1375=>83,1400=>84,1430=>85,1450=>86,1480=>87,1500=>88,1525=>89,1540=>90,1560=>91,1580=>92,1600=>93,1620=>94,1640=>95,1660=>96,1680=>97,1690=>98,1700=>99,1710=>100,1720=>101,1745=>102,1770=>103,1785=>104,1800=>105,1815=>106,1830=>107,1845=>108,1860=>109,1875=>110},x)", reverse_equation: "x")
+    Sensor.create!(id: 12, name: "HPP828E031", description: "test", default_key: "temp", equation: "(175.72 / 65536.0 * x) - 53", reverse_equation: "x")
+    Sensor.create!(id: 13, name: "HPP828E031", description: "test", default_key: "hum", equation: "(125.0 / 65536.0  * x) + 7", reverse_equation: "x")
+    Sensor.create!(id: 14, name: "BH1730FVC", description: "test", default_key: "light", equation: "x", reverse_equation: "x/10.0")
+    Sensor.create!(id: 15, name: "MiCS-4514", description: "test", default_key: "no2", equation: "x", reverse_equation: "x/1000.0")
+    Sensor.create!(id: 16, name: "MiCS-4514", description: "test", default_key: "co", equation: "x", reverse_equation: "x/1000.0")
+    Sensor.create!(id: 17, name: "Battery", description: "test", default_key: "bat", equation: "x", reverse_equation: "x/10.0")
+    Sensor.create!(id: 18, name: "Solar Panel", description: "test", default_key: "panel", equation: "x", reverse_equation: "x/1000.0")
+    Sensor.create!(id: 21, name: "Microchip RN-131", description: "test", default_key: "nets", equation: "x", reverse_equation: "x")
 
     Component.create!(id: 12, device: device, sensor: Sensor.find(12))
     Component.create!(id: 13, device: device, sensor: Sensor.find(13))
@@ -41,7 +41,7 @@ def to_ts(time)
   end
 
   it "updates component last_reading_at" do
-    includes_proxy = double({ where: double({last: device})})
+    includes_proxy = double({ where: double({last: device.reload})})
     allow(Device).to receive(:includes).and_return(includes_proxy)
 
     expect(device).to receive(:update_component_timestamps).with(
@@ -63,8 +63,8 @@ def to_ts(time)
   end
 
   it "should return a correct sensor id number" do
-    expect(device.find_sensor_id_by_key(:co)).to eq(16)
-    expect(device.find_sensor_id_by_key(:bat)).to eq(17)
+    expect(device.reload.find_sensor_id_by_key(:co)).to eq(16)
+    expect(device.reload.find_sensor_id_by_key(:bat)).to eq(17)
   end
 
   it "will be created with valid data" do

spec/services/device_archive_spec.rb

@@ -15,10 +15,10 @@ def kairos_query(key)
     create(:measurement, id: 3, name: 'noise')
     create(:measurement, id: 4, name: 'NO2')
 
-    create(:sensor, id:12, name:'HPP828E031', description: 'test', measurement_id: 1, unit: 'ºC', key: "temp", equation: '(175.72 / 65536.0 * x) - 53', reverse_equation: 'x')
-    create(:sensor, id:14, name:'BH1730FVC', description: 'test', measurement_id: 2, unit: 'KΩ', key: "light", equation: 'x', reverse_equation: 'x/10.0')
-    create(:sensor, id:7, name:'POM-3044P-R', description: 'test', measurement_id: 2, unit: 'dB', key: "noise", equation: 'Mathematician.table_calibration({0=>50,2=>55,3=>57,6=>58,20=>59,40=>60,60=>61,75=>62,115=>63,150=>64,180=>65,220=>66,260=>67,300=>68,375=>69,430=>70,500=>71,575=>72,660=>73,720=>74,820=>75,900=>76,975=>77,1050=>78,1125=>79,1200=>80,1275=>81,1320=>82,1375=>83,1400=>84,1430=>85,1450=>86,1480=>87,1500=>88,1525=>89,1540=>90,1560=>91,1580=>92,1600=>93,1620=>94,1640=>95,1660=>96,1680=>97,1690=>98,1700=>99,1710=>100,1720=>101,1745=>102,1770=>103,1785=>104,1800=>105,1815=>106,1830=>107,1845=>108,1860=>109,1875=>110},x)', reverse_equation: 'x')
-    create(:sensor, id:15, name:'MiCS-4514', description: 'test', measurement_id: 4, unit: 'kOhm', key: "no2", equation: 'x', reverse_equation: 'x/1000.0')
+    create(:sensor, id:12, name:'HPP828E031', description: 'test', measurement_id: 1, unit: 'ºC', default_key: "temp", equation: '(175.72 / 65536.0 * x) - 53', reverse_equation: 'x')
+    create(:sensor, id:14, name:'BH1730FVC', description: 'test', measurement_id: 2, unit: 'KΩ', default_key: "light", equation: 'x', reverse_equation: 'x/10.0')
+    create(:sensor, id:7, name:'POM-3044P-R', description: 'test', measurement_id: 2, unit: 'dB', default_key: "noise", equation: 'Mathematician.table_calibration({0=>50,2=>55,3=>57,6=>58,20=>59,40=>60,60=>61,75=>62,115=>63,150=>64,180=>65,220=>66,260=>67,300=>68,375=>69,430=>70,500=>71,575=>72,660=>73,720=>74,820=>75,900=>76,975=>77,1050=>78,1125=>79,1200=>80,1275=>81,1320=>82,1375=>83,1400=>84,1430=>85,1450=>86,1480=>87,1500=>88,1525=>89,1540=>90,1560=>91,1580=>92,1600=>93,1620=>94,1640=>95,1660=>96,1680=>97,1690=>98,1700=>99,1710=>100,1720=>101,1745=>102,1770=>103,1785=>104,1800=>105,1815=>106,1830=>107,1845=>108,1860=>109,1875=>110},x)', reverse_equation: 'x')
+    create(:sensor, id:15, name:'MiCS-4514', description: 'test', measurement_id: 4, unit: 'kOhm', default_key: "no2", equation: 'x', reverse_equation: 'x/1000.0')
 
     create(:component, id: 12, device: device, sensor: Sensor.find(12))
     create(:component, id: 14, device: device, sensor: Sensor.find(14))