From e67175157bd1c01091c41574246328879d5eccdd Mon Sep 17 00:00:00 2001 From: Steve Walsh Date: Thu, 8 Jul 2021 16:57:27 +0100 Subject: [PATCH] Fix for when after from function or change is an array --- README.md | 15 ++++- nodes/PayloadValidator.js | 19 ++++-- nodes/core/core/80-function.js | 2 +- nodes/core/logic/15-change.js | 2 +- test/unit/fixtures/data/orgEvent.js | 3 +- test/unit/test-payloadValidator.js | 90 ++++++++++++++++++++++++++--- 6 files changed, 116 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 035779ab0..474b6bb72 100644 --- a/README.md +++ b/README.md @@ -6,4 +6,17 @@ To make a change to the node-red runtime being used by K4 avalanche: 3. PR into this branch 4. Merge on approval 5. Manually bump the package version -6. Manually publish to NPM with `npm publish` \ No newline at end of file +6. Manually publish to NPM with `npm publish` - Request creds from ops for this + + +# CHANGE-LOG + + +## 0.18.7-patch-9 +2021-07-09 +- Bug fix for when retuning a array out of a function node + + +## 0.18.7-patch-8 +2021-07-08 +- Added support to log out when organization variable was changed within function or change node \ No newline at end of file diff --git a/nodes/PayloadValidator.js b/nodes/PayloadValidator.js index 05d15c4f6..273ee1d40 100644 --- a/nodes/PayloadValidator.js +++ b/nodes/PayloadValidator.js @@ -8,7 +8,7 @@ const variablesToCheck = [ ]; module.exports = class PayloadValidator { - constructor(_before) { + constructor(_before, id) { try { const before = clone(_before); const { @@ -20,6 +20,9 @@ module.exports = class PayloadValidator { organization, region } + }, + event: { + workers: [{ id: workerId }] } } = before; this.before = before; @@ -28,9 +31,11 @@ module.exports = class PayloadValidator { this.conversationId = conversationId; this.organization = organization; this.region = region; + this.workerId = workerId.split(':::')[0]; + this.nodeId = id.split(`${organization}-${this.workerId}-`)[1]; this.isValidBefore = true; } catch (e) { - console.log('Error while instantiating class with invalid object'); + console.log('Error while instantiating class with invalid object for'); console.log(e); this.isValidBefore = false; } @@ -40,13 +45,19 @@ module.exports = class PayloadValidator { return location.split('.').reduce((p, c) => (p && p[c]) || null, object); } - verify(after) { + verify(_after) { if (this.isValidBefore) { try { + let after = _after; + if (Array.isArray(after)) { + after = after.find((msg) => !!msg); + } variablesToCheck.forEach((location) => { if (this.getValue(this.before, location) !== this.getValue(after, location)) { const details = { - message: `msg.${location} changed from "${this.getValue(this.before, location)}" to "${this.getValue(after, location)}" for bot "${this.bot}"` + message: `msg.${location} changed from "${this.getValue(this.before, location)}" to "${this.getValue(after, location)}" for bot "${this.bot}"`, + nodeId: this.nodeId, + workerId: this.workerId }; this.logger.error(details.message); this.logger.app.platform.organization({ diff --git a/nodes/core/core/80-function.js b/nodes/core/core/80-function.js index dd856ef59..0219b53e6 100644 --- a/nodes/core/core/80-function.js +++ b/nodes/core/core/80-function.js @@ -209,7 +209,7 @@ module.exports = function(RED) { try { this.on("input", function(msg) { try { - const payloadValidator = new PayloadValidator(msg) + const payloadValidator = new PayloadValidator(msg, this.id) var start = process.hrtime(); sandbox.msg = msg; const vm2Instance = new vm2.VM({ sandbox, timeout: 5000 }); diff --git a/nodes/core/logic/15-change.js b/nodes/core/logic/15-change.js index e4c2e31a7..2cdd58cb6 100644 --- a/nodes/core/logic/15-change.js +++ b/nodes/core/logic/15-change.js @@ -229,7 +229,7 @@ module.exports = function(RED) { } if (valid) { this.on('input', function(msg) { - const payloadValidator = new PayloadValidator(msg) + const payloadValidator = new PayloadValidator(msg, this.id) for (var i=0; i ({ +module.exports = (org, workerId = 'worker-id') => ({ logger: { metadata: { organization: org @@ -13,6 +13,7 @@ module.exports = (org) => ({ }, }, event: { + workers:[{id: `${workerId}:::something:::else`}], event: { organization: org, token: { diff --git a/test/unit/test-payloadValidator.js b/test/unit/test-payloadValidator.js index e15fbc81b..36d7e3f6d 100644 --- a/test/unit/test-payloadValidator.js +++ b/test/unit/test-payloadValidator.js @@ -5,13 +5,17 @@ const assert = require('assert'); describe.only('Unit: PayloadValidator', () => { it('Should not log when no changes', () => { - const beforeEvent = orgEvent('before'); - const payloadValidator = new PayloadValidator(beforeEvent); + const nodeId = 'abc-id' + const workerId = 'worker-id' + const beforeEvent = orgEvent('before', workerId); + const payloadValidator = new PayloadValidator(beforeEvent, `before-${workerId}-${nodeId}`); payloadValidator.verify(beforeEvent); }); it('Should warn when org is overwritten', () => { - const beforeEvent = orgEvent('before'); + const nodeId = 'abc-id' + const workerId = 'worker-id' + const beforeEvent = orgEvent('before', workerId); errorLogStub = sinon.stub(); appLogStub = sinon.stub(); beforeEvent.logger.error = errorLogStub; @@ -20,8 +24,8 @@ describe.only('Unit: PayloadValidator', () => { organization: appLogStub } }; - - const payloadValidator = new PayloadValidator(beforeEvent); + + const payloadValidator = new PayloadValidator(beforeEvent, `before-${workerId}-${nodeId}`); const modifiedEvent = orgEvent('after'); @@ -30,14 +34,24 @@ describe.only('Unit: PayloadValidator', () => { assert(appLogStub.callCount === 4) const [[log1], [log2], [log3], [log4]] = appLogStub.args assert(log1.details.message.includes('logger.metadata.organization')) + assert.strictEqual(log1.details.nodeId, nodeId) + assert.strictEqual(log1.details.workerId, workerId) assert(log2.details.message.includes('payload.system.organization')) + assert.strictEqual(log2.details.nodeId, nodeId) + assert.strictEqual(log2.details.workerId, workerId) assert(log3.details.message.includes('event.event.organization')) + assert.strictEqual(log3.details.nodeId, nodeId) + assert.strictEqual(log3.details.workerId, workerId) assert(log4.details.message.includes('event.event.token.contents.organization')) + assert.strictEqual(log4.details.nodeId, nodeId) + assert.strictEqual(log4.details.workerId, workerId) }); it('Should warn when org is deleted', () => { - const beforeEvent = orgEvent('before'); + const nodeId = 'abc-id' + const workerId = 'worker-id' + const beforeEvent = orgEvent('before', workerId); errorLogStub = sinon.stub(); appLogStub = sinon.stub(); beforeEvent.logger.error = errorLogStub; @@ -47,7 +61,8 @@ describe.only('Unit: PayloadValidator', () => { } }; - const payloadValidator = new PayloadValidator(beforeEvent); + const payloadValidator = new PayloadValidator(beforeEvent, `before-${workerId}-${nodeId}`); + delete beforeEvent.logger.metadata.organization; delete beforeEvent.payload.system.organization; @@ -58,11 +73,70 @@ describe.only('Unit: PayloadValidator', () => { assert(appLogStub.callCount === 4) const [[log1], [log2], [log3], [log4]] = appLogStub.args assert(log1.details.message.includes('logger.metadata.organization')) + assert.strictEqual(log1.details.nodeId, nodeId) + assert.strictEqual(log1.details.workerId, workerId) assert(log2.details.message.includes('payload.system.organization')) + assert.strictEqual(log2.details.nodeId, nodeId) + assert.strictEqual(log2.details.workerId, workerId) assert(log3.details.message.includes('event.event.organization')) + assert.strictEqual(log3.details.nodeId, nodeId) + assert.strictEqual(log3.details.workerId, workerId) assert(log4.details.message.includes('event.event.token.contents.organization')) + assert.strictEqual(log4.details.nodeId, nodeId) + assert.strictEqual(log4.details.workerId, workerId) }); + it('Should handle when after is an array', () =>{ + const nodeId = 'abc-id' + const workerId = 'worker-id' + const beforeEvent = orgEvent('before', workerId); + errorLogStub = sinon.stub(); + appLogStub = sinon.stub(); + beforeEvent.logger.error = errorLogStub; + beforeEvent.logger.app = { + platform:{ + organization: appLogStub + } + }; + + const payloadValidator = new PayloadValidator(beforeEvent, `before-${workerId}-${nodeId}`); + const modifiedEvent = orgEvent('before'); + modifiedEvent.logger.metadata.organization = 'after' + payloadValidator.verify([modifiedEvent]); + assert(errorLogStub.callCount === 1) + assert(appLogStub.callCount === 1) + const [[log1]] = appLogStub.args + assert(log1.details.message.includes('logger.metadata.organization')) + assert.strictEqual(log1.details.nodeId, nodeId) + assert.strictEqual(log1.details.workerId, workerId) + }) + + it('Should handle when after is an array in something other than first position', () =>{ + const nodeId = 'abc-id' + const workerId = 'worker-id' + const beforeEvent = orgEvent('before', workerId); + errorLogStub = sinon.stub(); + appLogStub = sinon.stub(); + beforeEvent.logger.error = errorLogStub; + beforeEvent.logger.app = { + platform:{ + organization: appLogStub + } + }; + + const payloadValidator = new PayloadValidator(beforeEvent, `before-${workerId}-${nodeId}`); + + const modifiedEvent = orgEvent('before'); + modifiedEvent.logger.metadata.organization = 'after' + payloadValidator.verify([null, null, modifiedEvent, null]); + assert(errorLogStub.callCount === 1) + assert(appLogStub.callCount === 1) + const [[log1]] = appLogStub.args + assert(log1.details.message.includes('logger.metadata.organization')) + assert.strictEqual(log1.details.nodeId, nodeId) + assert.strictEqual(log1.details.workerId, workerId) + }) + it('Should not die when error', () => { const beforeEvent = orgEvent('before'); const payloadValidator = new PayloadValidator(beforeEvent); @@ -80,4 +154,6 @@ describe.only('Unit: PayloadValidator', () => { const payloadValidator = new PayloadValidator({}); payloadValidator.verify({}); }); + + }); \ No newline at end of file