Device new string state event: better null check (#1894)

GladysAssistant · Oct 6, 2023 · f5a6a0d · f5a6a0d
1 parent 9fe0c93
commit f5a6a0d
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 11 deletions.
diff --git a/server/lib/device/device.newStateEvent.js b/server/lib/device/device.newStateEvent.js
@@ -12,12 +12,15 @@ const logger = require('../../utils/logger');
  * newStateEvent({ device_feature_external_id: 'xx', state: 12 });
  */
 async function newStateEvent(event) {
+  const deviceFeature = this.stateManager.get('deviceFeatureByExternalId', event.device_feature_external_id);
+  if (deviceFeature === null) {
+    throw new NotFoundError(`DeviceFeature ${event.device_feature_external_id} not found`);
+  }
+  const device = this.stateManager.get('deviceById', deviceFeature.device_id);
+  if (device === null) {
+    throw new NotFoundError(`Device ${deviceFeature.device_id} not found`);
+  }
   try {
-    const deviceFeature = this.stateManager.get('deviceFeatureByExternalId', event.device_feature_external_id);
-    const device = this.stateManager.get('deviceById', deviceFeature.device_id);
-    if (deviceFeature === null) {
-      throw new NotFoundError('DeviceFeature not found');
-    }
     if (event.text) {
       await this.saveStringState(device, deviceFeature, event.text);
     } else if (event.created_at) {

diff --git a/server/lib/device/device.saveStringState.js b/server/lib/device/device.saveStringState.js
@@ -17,6 +17,13 @@ async function saveStringState(device, deviceFeature, newValue) {
   logger.debug(`device.saveStringState of deviceFeature ${deviceFeature.selector}`);
 
   const now = new Date();
+
+  deviceFeature.last_value_string = newValue;
+  deviceFeature.last_value_changed = now;
+
+  // save local state in RAM
+  this.stateManager.setState('deviceFeature', deviceFeature.selector, deviceFeature);
+
   await db.DeviceFeature.update(
     {
       last_value_string: newValue,
@@ -29,12 +36,6 @@ async function saveStringState(device, deviceFeature, newValue) {
     },
   );
 
-  deviceFeature.last_value_string = newValue;
-  deviceFeature.last_value_changed = new Date();
-
-  // save local state in RAM
-  this.stateManager.setState('deviceFeature', deviceFeature.selector, deviceFeature);
-
   // send websocket event
   this.eventManager.emit(EVENTS.WEBSOCKET.SEND_ALL, {
     type: WEBSOCKET_MESSAGE_TYPES.DEVICE.NEW_STRING_STATE,

diff --git a/server/test/lib/device/device.newStateEvent.test.js b/server/test/lib/device/device.newStateEvent.test.js
@@ -1,9 +1,14 @@
+const sinon = require('sinon');
+
+const { assert } = sinon;
+
 const EventEmitter = require('events');
 const { expect } = require('chai');
 
 const Device = require('../../../lib/device');
 const StateManager = require('../../../lib/state');
 const Job = require('../../../lib/job');
+const { NotFoundError } = require('../../../utils/coreErrors');
 
 const event = new EventEmitter();
 const job = new Job(event);
@@ -28,8 +33,11 @@ describe('Device.newStateEvent', () => {
       created_at: '2019-02-12 07:49:07.556 +00:00',
       updated_at: '2019-02-12 07:49:07.556 +00:00',
     });
+    stateManager.setState('deviceById', '7f85c2f8-86cc-4600-84db-6c074dadb4e8', {});
     const device = new Device(event, {}, stateManager, {}, {}, {}, job);
     await device.newStateEvent({ device_feature_external_id: 'hue:binary:1', state: 12 });
+    const newDeviceFeature = stateManager.get('deviceFeature', 'test-device-feature');
+    expect(newDeviceFeature).to.have.property('last_value', 12);
   });
   it('should save new string state', async () => {
     const stateManager = new StateManager(event);
@@ -50,6 +58,7 @@ describe('Device.newStateEvent', () => {
       created_at: '2019-02-12 07:49:07.556 +00:00',
       updated_at: '2019-02-12 07:49:07.556 +00:00',
     });
+    stateManager.setState('deviceById', '7f85c2f8-86cc-4600-84db-6c074dadb4e8', {});
     const device = new Device(event, {}, stateManager, {}, {}, {}, job);
     await device.newStateEvent({ device_feature_external_id: 'hue:binary:1', text: 'my-text' });
     const newDeviceFeature = stateManager.get('deviceFeatureByExternalId', 'hue:binary:1');
@@ -74,6 +83,7 @@ describe('Device.newStateEvent', () => {
       created_at: '2019-02-12 07:49:07.556 +00:00',
       updated_at: '2019-02-12 07:49:07.556 +00:00',
     };
+    stateManager.setState('deviceById', '7f85c2f8-86cc-4600-84db-6c074dadb4e8', {});
     stateManager.setState('deviceFeatureByExternalId', 'hue:binary:1', currentDeviceFeature);
     stateManager.setState('deviceFeature', 'test-device-feature', currentDeviceFeature);
     const device = new Device(event, {}, stateManager, {}, {}, {}, job);
@@ -107,6 +117,7 @@ describe('Device.newStateEvent', () => {
       created_at: '2019-02-12 07:49:07.556 +00:00',
       updated_at: '2019-02-12 07:49:07.556 +00:00',
     };
+    stateManager.setState('deviceById', '7f85c2f8-86cc-4600-84db-6c074dadb4e8', {});
     stateManager.setState('deviceFeatureByExternalId', 'hue:binary:1', currentDeviceFeature);
     stateManager.setState('deviceFeature', 'test-device-feature', currentDeviceFeature);
     const device = new Device(event, {}, stateManager, {}, {}, {}, job);
@@ -121,4 +132,49 @@ describe('Device.newStateEvent', () => {
     expect(newDeviceFeature).to.have.property('last_value_changed');
     expect(newDeviceFeature.last_value_changed).to.deep.equal(dateInTheFuture);
   });
+  it('should not save state missing device feature', async () => {
+    const stateManager = new StateManager(event);
+    const device = new Device(event, {}, stateManager, {}, {}, {}, job);
+    try {
+      await device.newStateEvent({
+        device_feature_external_id: 'hue:binary:1',
+        state: 12,
+        created_at: '2019-02-12 07:49:07.556 +00:00',
+      });
+      assert.fail();
+    } catch (e) {
+      expect(e).to.be.instanceOf(NotFoundError);
+    }
+    const newDeviceFeature = stateManager.get('deviceFeatureByExternalId', 'hue:binary:1');
+    // eslint-disable-next-line no-unused-expressions
+    expect(newDeviceFeature).to.be.null;
+  });
+  it('should not save state missing device', async () => {
+    const stateManager = new StateManager(event);
+    const currentDeviceFeature = {
+      id: 'ca91dfdf-55b2-4cf8-a58b-99c0fbf6f5e4',
+      name: 'Test device feature',
+      selector: 'test-device-feature',
+      external_id: 'hue:binary:1',
+      device_id: '7f85c2f8-86cc-4600-84db-6c074dadb4e8',
+    };
+    stateManager.setState('deviceFeatureByExternalId', 'hue:binary:1', currentDeviceFeature);
+    const device = new Device(event, {}, stateManager, {}, {}, {}, job);
+    try {
+      await device.newStateEvent({
+        device_feature_external_id: 'hue:binary:1',
+        state: 12,
+        created_at: '2019-02-12 07:49:07.556 +00:00',
+      });
+      assert.fail();
+    } catch (e) {
+      expect(e).to.be.instanceOf(NotFoundError);
+    }
+    const newDeviceFeature = stateManager.get('deviceFeatureByExternalId', 'hue:binary:1');
+    expect(newDeviceFeature).not.to.have.property('last_value');
+    expect(newDeviceFeature).not.to.have.property('last_value_changed');
+    const newDevice = stateManager.get('deviceById', '7f85c2f8-86cc-4600-84db-6c074dadb4e8');
+    // eslint-disable-next-line no-unused-expressions
+    expect(newDevice).to.be.null;
+  });
 });