Fix sensor state translations (#16393)

* Fix sensor state translations

* Fix up lint/style

* Set state_index_id

* Apply fixes from StyleCI

* Wrong call

* just use a loop

* Wrong id column

* Missing fillable

* Handle sensors missing state translations

* Before making a state index

* Can't map to a state index if it doesn't exist

* Apply fixes from StyleCI

* ies5000 overflowing tinyint

* Accept state translations directly add that in the translation

* handle duplicate state names, but with different case (skip no way to work there)

* Apply fixes from StyleCI

* Fix type stuffs

---------

Co-authored-by: Tony Murray <murrant@users.noreply.github.com>

LibreNMS/DB/SyncsModels.php

@@ -26,7 +26,6 @@
 namespace LibreNMS\DB;
 
 use App\Models\Device;
-use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Relations\HasManyThrough;
 use Illuminate\Support\Collection;
 use LibreNMS\Interfaces\Models\Keyable;
@@ -37,15 +36,15 @@ trait SyncsModels
      * Sync several models for a device's relationship
      * Model must implement \LibreNMS\Interfaces\Models\Keyable interface
      *
-     * @param  \App\Models\Device  $device
+     * @param  \Illuminate\Database\Eloquent\Model  $parentModel
      * @param  string  $relationship
      * @param  \Illuminate\Support\Collection<Keyable>  $models  \LibreNMS\Interfaces\Models\Keyable
      * @return \Illuminate\Support\Collection
      */
-    protected function syncModels($device, $relationship, $models, $existing = null): Collection
+    protected function syncModels($parentModel, $relationship, $models, $existing = null): Collection
     {
         $models = $models->keyBy->getCompositeKey();
-        $existing = ($existing ?? $device->$relationship)->groupBy->getCompositeKey();
+        $existing = ($existing ?? $parentModel->$relationship)->groupBy->getCompositeKey();
 
         foreach ($existing as $exist_key => $existing_rows) {
             if ($models->offsetExists($exist_key)) {
@@ -67,12 +66,12 @@ trait SyncsModels
         }
 
         $new = $models->diffKeys($existing);
-        if (is_a($device->$relationship(), HasManyThrough::class)) {
+        if (is_a($parentModel->$relationship(), HasManyThrough::class)) {
             // if this is a distant relation, the models need the intermediate relationship set
             // just save assuming things are correct
             $new->each->save();
         } else {
-            $device->$relationship()->saveMany($new);
+            $parentModel->$relationship()->saveMany($new);
         }
 
         return $existing->map->first()->merge($new);

app/Discovery/Sensor.php

@@ -26,10 +26,16 @@
 namespace App\Discovery;
 
 use App\Models\Device;
+use App\Models\Eventlog;
+use App\Models\SensorToStateIndex;
+use App\Models\StateIndex;
+use App\Models\StateTranslation;
+use Illuminate\Database\UniqueConstraintViolationException;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Log;
 use LibreNMS\Config;
 use LibreNMS\DB\SyncsModels;
+use LibreNMS\Enum\Severity;
 
 class Sensor
 {
@@ -40,6 +46,8 @@ class Sensor
     private array $discovered = [];
     private string $relationship = 'sensors';
     private Device $device;
+    /** @var array<string, Collection<StateTranslation>> */
+    private array $states = [];
 
     public function __construct(Device $device)
     {
@@ -65,6 +73,18 @@ class Sensor
         return $this;
     }
 
+    /**
+     * @param  string  $stateName
+     * @param  StateTranslation[]|Collection<StateTranslation>  $states
+     * @return $this
+     */
+    public function withStateTranslations(string $stateName, array|Collection $states): static
+    {
+        $this->states[$stateName] = new Collection($states);
+
+        return $this;
+    }
+
     public function isDiscovered(string $type): bool
     {
         return $this->discovered[$type] ?? false;
@@ -78,6 +98,8 @@ class Sensor
             $synced = $this->syncModelsByGroup($this->device, 'sensors', $this->getModels(), $params);
             $this->discovered[$type] = true;
 
+            $this->syncStates($synced);
+
             return $synced;
         }
 
@@ -107,4 +129,61 @@ class Sensor
 
         return false;
     }
+
+    private function syncStates(Collection $sensors): void
+    {
+        $stateSensors = $sensors->where('sensor_class', 'state');
+
+        if ($stateSensors->isEmpty()) {
+            return;
+        }
+
+        $usedStates = $stateSensors->pluck('sensor_type');
+        $existingStateIndexes = StateIndex::whereIn('state_name', $usedStates)->get()->keyBy('state_name');
+
+        foreach ($usedStates as $stateName) {
+            // make sure the state translations were given for this state name
+            if (! isset($this->states[$stateName])) {
+                Log::error("Non existent state name ($stateName) set by sensor: " . $stateSensors->where('sensor_type', $stateName)->first()?->sensor_descr);
+
+                continue;
+            }
+
+            $stateIndex = $existingStateIndexes->get($stateName);
+
+            // create new state indexes
+            if ($stateIndex == null) {
+                try {
+                    $stateIndex = StateIndex::create(['state_name' => $stateName]);
+                    $existingStateIndexes->put($stateName, $stateIndex);
+                } catch (UniqueConstraintViolationException) {
+                    Eventlog::log("Duplicate state name $stateName (with case mismatch)", $this->device, 'sensor', Severity::Error, $stateSensors->where('sensor_type', $stateName)->first()?->sensor_id);
+
+                    continue;
+                }
+            }
+
+            // set state_index_id
+            $stateTranslations = $this->states[$stateName];
+            foreach ($stateTranslations as $translation) {
+                $translation->state_index_id = $stateIndex->state_index_id;
+            }
+
+            // sync the translations to make sure they are up to date
+            $this->syncModels($stateIndex, 'translations', $stateTranslations);
+        }
+
+        // update sensor to state indexes
+        foreach ($stateSensors as $stateSensor) {
+            $state_index_id = $existingStateIndexes->get($stateSensor->sensor_type)?->state_index_id;
+
+            // only map if sensor gave a valid state name
+            if ($state_index_id) {
+                SensorToStateIndex::updateOrCreate(
+                    ['sensor_id' => $stateSensor->sensor_id],
+                    ['state_index_id' => $state_index_id],
+                );
+            }
+        }
+    }
 }

app/Models/Sensor.php

@@ -4,6 +4,7 @@ namespace App\Models;
 
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Relations\BelongsToMany;
+use Illuminate\Database\Eloquent\Relations\HasOneThrough;
 use Illuminate\Database\Eloquent\Relations\MorphMany;
 use LibreNMS\Interfaces\Models\Keyable;
 
@@ -129,9 +130,14 @@ class Sensor extends DeviceRelatedModel implements Keyable
         return $this->morphMany(Eventlog::class, 'events', 'type', 'reference');
     }
 
+    public function stateIndex(): HasOneThrough
+    {
+        return $this->hasOneThrough(StateIndex::class, SensorToStateIndex::class, 'sensor_id', 'state_index_id', 'sensor_id', 'state_index_id');
+    }
+
     public function translations(): BelongsToMany
     {
-        return $this->belongsToMany(StateTranslation::class, 'sensors_to_state_indexes', 'sensor_id', 'state_index_id');
+        return $this->belongsToMany(StateTranslation::class, 'sensors_to_state_indexes', 'sensor_id', 'state_index_id', 'sensor_id', 'state_index_id');
     }
 
     public function getCompositeKey(): string

app/Models/SensorToStateIndex.php

@@ -0,0 +1,24 @@
+<?php
+
+namespace App\Models;
+
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\HasOne;
+
+class SensorToStateIndex extends Model
+{
+    protected $table = 'sensors_to_state_indexes';
+    protected $primaryKey = 'sensors_to_state_translations_id';
+    public $timestamps = false;
+    protected $fillable = ['sensor_id', 'state_index_id'];
+
+    public function sensor(): HasOne
+    {
+        return $this->hasOne(Sensor::class, 'sensor_id', 'sensor_id');
+    }
+
+    public function stateIndex(): HasOne
+    {
+        return $this->hasOne(StateIndex::class, 'state_index_id', 'state_index_id');
+    }
+}

app/Models/StateIndex.php

@@ -0,0 +1,28 @@
+<?php
+
+namespace App\Models;
+
+use Illuminate\Database\Eloquent\Factories\HasFactory;
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\HasMany;
+use Illuminate\Database\Eloquent\Relations\HasManyThrough;
+
+class StateIndex extends Model
+{
+    use HasFactory;
+
+    public $timestamps = false;
+    protected $table = 'state_indexes';
+    protected $fillable = ['state_name'];
+    protected $primaryKey = 'state_index_id';
+
+    public function sensors(): HasManyThrough
+    {
+        return $this->hasManyThrough(Sensor::class, SensorToStateIndex::class, 'state_index_id', 'sensor_id', 'state_index_id', 'sensor_id');
+    }
+
+    public function translations(): HasMany
+    {
+        return $this->hasMany(StateTranslation::class, 'state_index_id', 'state_index_id');
+    }
+}

app/Models/StateTranslation.php

@@ -26,9 +26,28 @@
 namespace App\Models;
 
 use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Relations\BelongsTo;
+use LibreNMS\Interfaces\Models\Keyable;
 
-class StateTranslation extends Model
+class StateTranslation extends Model implements Keyable
 {
-    public $timestamps = false;
-    protected $primaryKey = 'state_index_id';
+    const CREATED_AT = null;
+    const UPDATED_AT = 'state_lastupdated';
+    protected $primaryKey = 'state_translation_id';
+    protected $fillable = [
+        'state_descr',
+        'state_draw_graph',
+        'state_value',
+        'state_generic_value',
+    ];
+
+    public function stateIndex(): BelongsTo
+    {
+        return $this->belongsTo(StateIndex::class, 'state_index_id', 'state_index_id');
+    }
+
+    public function getCompositeKey()
+    {
+        return $this->state_value;
+    }
 }

includes/definitions/discovery/ies5000.yaml

@@ -105,7 +105,7 @@ modules:
                         - { value: 4096, generic: 2, graph: 0, descr: macSpoof }
                         - { value: 8192, generic: 2, graph: 0, descr: cpuHigh }
                         - { value: 16384, generic: 2, graph: 0, descr: memoryUsageHigh }
-                        - { value: 32768, generic: 2, graph: 0, descr: packetBufferUsageHigh }
+#                        - { value: 32768, generic: 2, graph: 0, descr: packetBufferUsageHigh }  # state value larger than smallint 32767
                 -
                     oid: ledAlarmStatus
                     value: ledAlarmStatus

includes/functions.php

@@ -8,6 +8,7 @@
  * @copyright  (C) 2006 - 2012 Adam Armstrong
  */
 
+use App\Models\StateTranslation;
 use Illuminate\Support\Str;
 use LibreNMS\Config;
 use LibreNMS\Enum\Severity;
@@ -493,106 +494,23 @@ function dnslookup($device, $type = false, $return = false)
  *
  * @param  string  $state_name  the unique name for this state translation
  * @param  array  $states  array of states, each must contain keys: descr, graph, value, generic
- * @return int|null
+ * @return void
  */
-function create_state_index($state_name, $states = [])
+function create_state_index($state_name, $states = []): void
 {
-    $state_index_id = dbFetchCell('SELECT `state_index_id` FROM state_indexes WHERE state_name = ? LIMIT 1', [$state_name]);
-    if (! is_numeric($state_index_id)) {
-        $state_index_id = dbInsert(['state_name' => $state_name], 'state_indexes');
-
-        // legacy code, return index so states are created
-        if (empty($states)) {
-            return $state_index_id;
-        }
-    }
-
-    // check or synchronize states
-    if (empty($states)) {
-        $translations = dbFetchRows('SELECT * FROM `state_translations` WHERE `state_index_id` = ?', [$state_index_id]);
-        if (count($translations) == 0) {
-            // If we don't have any translations something has gone wrong so return the state_index_id so they get created.
-            return $state_index_id;
-        }
-    } else {
-        sync_sensor_states($state_index_id, $states);
-    }
-
-    return null;
-}
-
-/**
- * Synchronize the sensor state translations with the database
- *
- * @param  int  $state_index_id  index of the state
- * @param  array  $states  array of states, each must contain keys: descr, graph, value, generic
- */
-function sync_sensor_states($state_index_id, $states)
-{
-    $new_translations = array_reduce($states, function ($array, $state) use ($state_index_id) {
-        $array[$state['value']] = [
-            'state_index_id' => $state_index_id,
+    app('sensor-discovery')->withStateTranslations($state_name, array_map(function ($state) {
+        return new StateTranslation([
             'state_descr' => $state['descr'],
             'state_draw_graph' => $state['graph'],
             'state_value' => $state['value'],
             'state_generic_value' => $state['generic'],
-        ];
-
-        return $array;
-    }, []);
-
-    $existing_translations = dbFetchRows(
-        'SELECT `state_index_id`,`state_descr`,`state_draw_graph`,`state_value`,`state_generic_value` FROM `state_translations` WHERE `state_index_id`=?',
-        [$state_index_id]
-    );
-
-    foreach ($existing_translations as $translation) {
-        $value = $translation['state_value'];
-        if (isset($new_translations[$value])) {
-            if ($new_translations[$value] != $translation) {
-                dbUpdate(
-                    $new_translations[$value],
-                    'state_translations',
-                    '`state_index_id`=? AND `state_value`=?',
-                    [$state_index_id, $value]
-                );
-            }
-
-            // this translation is synchronized, it doesn't need to be inserted
-            unset($new_translations[$value]);
-        } else {
-            dbDelete('state_translations', '`state_index_id`=? AND `state_value`=?', [$state_index_id, $value]);
-        }
-    }
-
-    // insert any new translations
-    dbBulkInsert($new_translations, 'state_translations');
+        ]);
+    }, $states));
 }
 
 function create_sensor_to_state_index($device, $state_name, $index)
 {
-    $sensor_entry = dbFetchRow('SELECT sensor_id FROM `sensors` WHERE `sensor_class` = ? AND `device_id` = ? AND `sensor_type` = ? AND `sensor_index` = ?', [
-        'state',
-        $device['device_id'],
-        $state_name,
-        $index,
-    ]);
-    $state_indexes_entry = dbFetchRow('SELECT state_index_id FROM `state_indexes` WHERE `state_name` = ?', [
-        $state_name,
-    ]);
-    if (! empty($sensor_entry['sensor_id']) && ! empty($state_indexes_entry['state_index_id'])) {
-        $insert = [
-            'sensor_id' => $sensor_entry['sensor_id'],
-            'state_index_id' => $state_indexes_entry['state_index_id'],
-        ];
-        foreach ($insert as $key => $val_check) {
-            if (! isset($val_check)) {
-                unset($insert[$key]);
-            }
-        }
-
-        dbInsert($insert, 'sensors_to_state_indexes');
-    }
+    // no op
 }
 
 function delta_to_bits($delta, $period)