From 10771a20199941b23b1ed06ba9d5f11f068275c5 Mon Sep 17 00:00:00 2001
From: Steve Walsh <stephen.walsh@servisbot.com>
Date: Tue, 13 Jul 2021 16:10:42 +0100
Subject: [PATCH] Updated logic to handle when parts of the msg object are
 changed to strings

---
 nodes/PayloadValidator.js          | 19 ++++++++
 test/unit/test-payloadValidator.js | 69 ++++++++++++++++++++++++------
 2 files changed, 75 insertions(+), 13 deletions(-)

diff --git a/nodes/PayloadValidator.js b/nodes/PayloadValidator.js
index c8619e72f..b7f6c0c89 100644
--- a/nodes/PayloadValidator.js
+++ b/nodes/PayloadValidator.js
@@ -65,6 +65,21 @@ module.exports = class PayloadValidator {
     return _.set(object, location, value);
   }
 
+  /**
+   * lodash set will always pass but might not actually set the value
+   * So here we test that it was actually set and use that to confirm we can set
+   *
+   * @param {*} object
+   * @param {*} location
+   * @returns
+   */
+  canSet(object, location) {
+    const test = 'test';
+    let clonedObject = clone(object);
+    clonedObject = this.setValue(clonedObject, location, test);
+    return test === this.getValue(clonedObject, location);
+  }
+
   check(_after) {
     let after = _after;
     try {
@@ -85,6 +100,10 @@ module.exports = class PayloadValidator {
             details,
             conversationId: this.conversationId
           });
+
+          if (!this.canSet(after, location)) {
+            throw new Error(`Cant set value as ${location} is no longer present in after`);
+          }
           after = this.setValue(after, location, beforeValue);
         }
       });
diff --git a/test/unit/test-payloadValidator.js b/test/unit/test-payloadValidator.js
index 1e046113f..66bfba502 100644
--- a/test/unit/test-payloadValidator.js
+++ b/test/unit/test-payloadValidator.js
@@ -26,9 +26,7 @@ describe('Unit: PayloadValidator', () => {
     };
 
     const payloadValidator = new PayloadValidator(beforeEvent, `before-${workerId}-${nodeId}`);
-
     const modifiedEvent = orgEvent('after');
-
     const result = payloadValidator.verify(modifiedEvent);
 
     assert(errorLogStub.callCount === 4);
@@ -54,6 +52,38 @@ describe('Unit: PayloadValidator', () => {
     assert.deepStrictEqual(result, beforeEvent);
   });
 
+  it('Should warn when org is overwritten and fail to fix it due to overwriting payload', () => {
+    const nodeId = 'abc-id';
+    const workerId = 'worker-id';
+    const beforeEvent = orgEvent('before', workerId);
+    const errorLogStub = sinon.stub();
+    const 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.payload = 'some text';
+    const result = payloadValidator.verify(modifiedEvent);
+    console.log(errorLogStub.callCount);
+    assert(errorLogStub.callCount === 1);
+    assert(appLogStub.callCount === 1);
+    const [[log]] = appLogStub.args;
+    assert(log.details.message.includes('msg.payload.system.organization'));
+    assert.strictEqual(log.details.nodeId, nodeId);
+    assert.strictEqual(log.details.workerId, workerId);
+    // deleting due to sinon funness
+    delete result.logger.error;
+    delete result.logger.app;
+    delete modifiedEvent.logger.error;
+    delete modifiedEvent.logger.app;
+    assert.deepStrictEqual(result, modifiedEvent);
+  });
+
   it('Should warn when org is deleted and fix it', () => {
     const nodeId = 'abc-id';
     const workerId = 'worker-id';
@@ -187,15 +217,6 @@ describe('Unit: PayloadValidator', () => {
     assert.deepStrictEqual(result, beforeEvent);
   });
 
-  it('Should not die when error', () => {
-    const beforeEvent = orgEvent('before');
-    const payloadValidator = new PayloadValidator(beforeEvent);
-
-    const modifiedEvent = orgEvent('after');
-
-    payloadValidator.verify(modifiedEvent);
-  });
-
   it('Should not die with initiating the class with bad object', () => {
     const payloadValidator = new PayloadValidator({});
   });
@@ -209,7 +230,7 @@ describe('Unit: PayloadValidator', () => {
     let payloadValidator;
     before(() => {
       const event = orgEvent('event');
-      payloadValidator = new PayloadValidator(event);
+      payloadValidator = new PayloadValidator(event, 'before-worker-id-nodeId');
     });
     it('Should set a root level variable', () => {
       const location = 'hello';
@@ -241,7 +262,7 @@ describe('Unit: PayloadValidator', () => {
     let payloadValidator;
     before(() => {
       const event = orgEvent('event');
-      payloadValidator = new PayloadValidator(event);
+      payloadValidator = new PayloadValidator(event, 'before-worker-id-nodeId');
     });
     it('Should set a root level variable', () => {
       const object = {};
@@ -259,4 +280,26 @@ describe('Unit: PayloadValidator', () => {
       assert.strictEqual(result.should.set.hello, value);
     });
   });
+
+  describe('Can set', () => {
+    let payloadValidator;
+    before(() => {
+      const event = orgEvent('event');
+      payloadValidator = new PayloadValidator(event, 'before-worker-id-nodeId');
+    });
+
+    it('Should get back false for canSet when object is a string', () => {
+      const object = 'some string';
+      const location = 'hello';
+      const result = payloadValidator.canSet(object, location);
+      assert.strictEqual(result, false);
+    });
+
+    it('Should get back true for canSet', () => {
+      const object = {};
+      const location = 'should.set.hello';
+      const result = payloadValidator.canSet(object, location);
+      assert.strictEqual(result, true);
+    });
+  });
 });