Merge pull request #2990 from FlowFuse/fix-2980-n+1-project-snapshots

Fix: Loading project snapshot for every device status update

forge/db/controllers/Device.js

@@ -14,16 +14,17 @@ module.exports = {
                 device.set('activeSnapshotId', null)
             }
         } else {
-            // Check the snapshot is one we recognise
+            // Update the activeSnapshotId if valid and not already set
             const snapshotId = app.db.models.ProjectSnapshot.decodeHashid(state.snapshot)
             // hashid.decode returns an array of values, not the raw value.
-            if (snapshotId?.length > 0) {
-                // check to see if snapshot still exists
-                if (await app.db.models.ProjectSnapshot.byId(state.snapshot)) {
-                    device.set('activeSnapshotId', snapshotId)
+            if (snapshotId?.length > 0 && snapshotId !== device.activeSnapshotId) {
+                // Check to see if snapshot exists
+                if (await app.db.models.ProjectSnapshot.count({ where: { id: snapshotId }, limit: 1 }) > 0) {
+                    device.set('activeSnapshotId', snapshotId[0])
                 }
             }
         }
+
         await device.save()
     },
     /**

test/lib/TestModelFactory.js

@@ -184,6 +184,7 @@ module.exports = class TestModelFactory {
             ...deviceDetails
         })
         await team.addDevice(device)
+        await device.setTeam(team)
         if (project) {
             await device.setProject(project)
         } else if (application) {

test/unit/forge/comms/devices_spec.js

@@ -43,6 +43,11 @@ describe('DeviceCommsHandler', function () {
             name: 'device1'
         }, TestObjects.ATeam)
 
+        TestObjects.applicationDevice = await app.factory.createDevice({
+            name: 'device2',
+            ownerType: 'application'
+        }, TestObjects.ATeam)
+
         TestObjects.tokens = {}
         await login('alice', 'aaPassword')
     }
@@ -253,5 +258,33 @@ describe('DeviceCommsHandler', function () {
             const payload = JSON.parse(msg.payload)
             payload.should.have.property('command', 'update')
         })
+
+        it('updates the active snapshot ID if it is found in the database', async function () {
+            TestObjects.device.Team = await TestObjects.device.getTeam() // .Team is not loaded in the tests
+
+            const knownSnapshot = await app.db.models.ProjectSnapshot.create({
+                name: 'Test Snapshot',
+                description: 'Test Description',
+                flows: {},
+                ApplicationId: TestObjects.device.ApplicationId,
+                DeviceId: TestObjects.applicationDevice.id,
+                UserId: TestObjects.alice.id
+            })
+
+            client.emit('status/device', {
+                id: TestObjects.applicationDevice.hashid,
+                status: JSON.stringify({
+                    state: 'online',
+                    snapshot: knownSnapshot.hashid
+                })
+            })
+
+            // Task happens async
+            await sleep(100)
+
+            await TestObjects.applicationDevice.reload()
+
+            TestObjects.applicationDevice.activeSnapshotId.should.equal(knownSnapshot.id)
+        })
     })
 })

test/unit/forge/db/controllers/Device_spec.js

@@ -1,5 +1,6 @@
 const sinon = require('sinon')
 const should = require('should') // eslint-disable-line
+
 const snapshotServices = require('../../../../../forge/services/snapshots.js')
 const TestModelFactory = require('../../../../lib/TestModelFactory.js')
 const setup = require('../setup')
@@ -84,6 +85,67 @@ describe('Device controller', function () {
         await app.close()
     })
 
+    describe('updateState', function () {
+        it('updates the activeSnapshot if snapshot matching hashid is found', async function () {
+            const applicationDevice = await factory.createDevice({
+                name: 'device',
+                ownerType: 'application'
+            }, TestObjects.team)
+
+            const knownSnapshot = await snapshotServices.createSnapshot(app, TestObjects.project, TestObjects.alice, {
+                name: 'instance 3 snapshot 1',
+                description: 'snapshot 1 on instance 3'
+            })
+
+            await app.db.controllers.Device.updateState(applicationDevice, {
+                snapshot: knownSnapshot.hashid
+            })
+
+            await applicationDevice.reload()
+
+            applicationDevice.activeSnapshotId.should.equal(knownSnapshot.id)
+        })
+
+        it('does not update activeSnapshot if snapshot matching hashid does not exist', async function () {
+            const applicationDevice = await factory.createDevice({
+                name: 'device',
+                ownerType: 'application'
+            }, TestObjects.team)
+
+            const oldSnapshot = await snapshotServices.createSnapshot(app, TestObjects.project, TestObjects.alice, {
+                name: 'instance 3 snapshot 1',
+                description: 'snapshot 1 on instance 3'
+            })
+
+            const oldSnapshotHashId = oldSnapshot.hashid
+
+            await oldSnapshot.destroy()
+
+            await app.db.controllers.Device.updateState(applicationDevice, {
+                snapshot: oldSnapshotHashId
+            })
+
+            await applicationDevice.reload()
+
+            should(applicationDevice.activeSnapshotId).equal(null)
+        })
+
+        it('does not update activeSnapshot if snapshot hashid is invalid', async function () {
+            const applicationDevice = await factory.createDevice({
+                name: 'device',
+                ownerType: 'application'
+            }, TestObjects.team)
+
+            await app.db.controllers.Device.updateState(applicationDevice, {
+                snapshot: 'invalid-hash-id'
+            })
+
+            await applicationDevice.reload()
+
+            should(applicationDevice.activeSnapshotId).equal(null)
+        })
+    })
+
     describe('Platform Specific Environment Variables', function () {
         it('generates env array', async function () {
             const device = {