From 25ce08131a502f5359c4fee2e31da8f42362afc1 Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Tue, 20 Nov 2018 15:11:12 -0500 Subject: [PATCH 1/3] Add rename and delete functionality for cloud variables. Refactor function name for cloud variable creation API. --- src/engine/target.js | 11 ++++++++++- src/io/cloud.js | 25 ++++++++++++++++++++++++- test/unit/engine_target.js | 12 ++++++------ test/unit/io_cloud.js | 8 ++++---- 4 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/engine/target.js b/src/engine/target.js index 331cfa5de..14dc69c09 100644 --- a/src/engine/target.js +++ b/src/engine/target.js @@ -241,7 +241,7 @@ class Target extends EventEmitter { const newVariable = new Variable(id, name, type, false); this.variables[id] = newVariable; if (isCloud && this.isStage) { - this.runtime.ioDevices.cloud.requestCreateCloudVariable(newVariable); + this.runtime.ioDevices.cloud.requestCreateVariable(newVariable); } } } @@ -285,9 +285,14 @@ class Target extends EventEmitter { if (this.variables.hasOwnProperty(id)) { const variable = this.variables[id]; if (variable.id === id) { + const oldName = variable.name; variable.name = newName; if (this.runtime) { + if (variable.isCloud && this.isStage) { + this.runtime.ioDevices.cloud.requestRenameVariable(oldName, newName); + } + const blocks = this.runtime.monitorBlocks; blocks.changeBlock({ id: id, @@ -314,8 +319,12 @@ class Target extends EventEmitter { */ deleteVariable (id) { if (this.variables.hasOwnProperty(id)) { + const variable = this.variables[id]; delete this.variables[id]; if (this.runtime) { + if (variable.isCloud && this.isStage) { + this.runtime.ioDevices.cloud.requestDeleteVariable(variable.name); + } this.runtime.monitorBlocks.deleteBlock(id); this.runtime.requestRemoveMonitor(id); } diff --git a/src/io/cloud.js b/src/io/cloud.js index 244e914c8..f115dfae8 100644 --- a/src/io/cloud.js +++ b/src/io/cloud.js @@ -100,7 +100,7 @@ class Cloud { } } - requestCreateCloudVariable (variable) { + requestCreateVariable (variable) { if (this.runtime.canAddCloudVariable()) { if (this.provider) { this.provider.createVariable(variable.name, variable.value); @@ -123,6 +123,29 @@ class Cloud { } } + /** + * Request the cloud data provider to rename the variable with the given name + * to the given new name. Does nothing if this io device does not have a provider set. + * @param {string} oldName The name of the variable to rename + * @param {string | number} newName The new name for the variable + */ + requestRenameVariable (oldName, newName) { + if (this.provider) { + this.provider.renameVariable(oldName, newName); + } + } + + /** + * Request the cloud data provider to delete the variable with the given name + * Does nothing if this io device does not have a provider set. + * @param {string} name The name of the variable to delete + */ + requestDeleteVariable (name) { + if (this.provider) { + this.provider.deleteVariable(name); + } + } + /** * Create a cloud variable based on the message * received from the cloud provider. diff --git a/test/unit/engine_target.js b/test/unit/engine_target.js index 6f7c61660..bdf6161ac 100644 --- a/test/unit/engine_target.js +++ b/test/unit/engine_target.js @@ -71,11 +71,11 @@ test('createListVariable creates a list', t => { t.end(); }); -test('createVariable calls cloud io device\'s requestCreateCloudVariable', t => { +test('createVariable calls cloud io device\'s requestCreateVariable', t => { const runtime = new Runtime(); - // Mock the requestCreateCloudVariable function + // Mock the requestCreateVariable function let requestCreateCloudWasCalled = false; - runtime.ioDevices.cloud.requestCreateCloudVariable = () => { + runtime.ioDevices.cloud.requestCreateVariable = () => { requestCreateCloudWasCalled = true; }; @@ -97,11 +97,11 @@ test('createVariable calls cloud io device\'s requestCreateCloudVariable', t => t.end(); }); -test('createVariable does not call cloud io device\'s requestCreateCloudVariable if target is not stage', t => { +test('createVariable does not call cloud io device\'s requestCreateVariable if target is not stage', t => { const runtime = new Runtime(); - // Mock the requestCreateCloudVariable function + // Mock the requestCreateVariable function let requestCreateCloudWasCalled = false; - runtime.ioDevices.cloud.requestCreateCloudVariable = () => { + runtime.ioDevices.cloud.requestCreateVariable = () => { requestCreateCloudWasCalled = true; }; diff --git a/test/unit/io_cloud.js b/test/unit/io_cloud.js index 0991a4f3d..52473e13b 100644 --- a/test/unit/io_cloud.js +++ b/test/unit/io_cloud.js @@ -12,7 +12,7 @@ test('spec', t => { t.type(cloud.postData, 'function'); t.type(cloud.requestUpdateVariable, 'function'); t.type(cloud.updateCloudVariable, 'function'); - t.type(cloud.requestCreateCloudVariable, 'function'); + t.type(cloud.requestCreateVariable, 'function'); t.type(cloud.createCloudVariable, 'function'); t.type(cloud.setProvider, 'function'); t.type(cloud.setStage, 'function'); @@ -115,7 +115,7 @@ test('requestUpdateVariable calls provider\'s updateVariable function', t => { t.end(); }); -test('requestCreateCloudVariable calls provider\'s createVariable function', t => { +test('requestCreateVariable calls provider\'s createVariable function', t => { let createVariableCalled = false; const mockVariable = new Variable('a var id', 'my var', Variable.SCALAR_TYPE, false); let mockVarName; @@ -134,11 +134,11 @@ test('requestCreateCloudVariable calls provider\'s createVariable function', t = const runtime = new Runtime(); const cloud = new Cloud(runtime); cloud.setProvider(provider); - cloud.requestCreateCloudVariable(mockVariable); + cloud.requestCreateVariable(mockVariable); t.equals(createVariableCalled, true); t.strictEquals(mockVarName, 'my var'); t.strictEquals(mockVarValue, 0); - // Calling requestCreateCloudVariable does not set isCloud flag on variable + // Calling requestCreateVariable does not set isCloud flag on variable t.strictEquals(mockVariable.isCloud, false); t.end(); }); From abbce0020383a3f3bc6608981c4bcf8ff22ad3b6 Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Tue, 20 Nov 2018 19:56:45 -0500 Subject: [PATCH 2/3] Add unit tests for new rename and delete cloud var functionality. --- test/unit/engine_target.js | 100 +++++++++++++++++++++++++++++++++++++ test/unit/io_cloud.js | 47 +++++++++++++++++ 2 files changed, 147 insertions(+) diff --git a/test/unit/engine_target.js b/test/unit/engine_target.js index bdf6161ac..afff2c48c 100644 --- a/test/unit/engine_target.js +++ b/test/unit/engine_target.js @@ -177,6 +177,61 @@ test('renameVariable3', t => { t.end(); }); +test('renameVariable calls cloud io device\'s requestRenameVariable function', t => { + const runtime = new Runtime(); + + let requestRenameVariableWasCalled = false; + runtime.ioDevices.cloud.requestRenameVariable = () => { + requestRenameVariableWasCalled = true; + }; + + const target = new Target(runtime); + target.isStage = true; + const mockCloudVar = new Variable('foo', 'bar', Variable.SCALAR_TYPE, true); + target.variables[mockCloudVar.id] = mockCloudVar; + runtime.targets.push(target); + + target.renameVariable('foo', 'bar2'); + + const variables = target.variables; + t.equal(Object.keys(variables).length, 1); + const variable = variables[Object.keys(variables)[0]]; + t.equal(variable.id, 'foo'); + t.equal(variable.name, 'bar2'); + t.equal(variable.value, 0); + t.equal(variable.isCloud, true); + t.equal(requestRenameVariableWasCalled, true); + + t.end(); +}); + +test('renameVariable does not call cloud io device\'s requestRenameVariable function if target is not stage', t => { + const runtime = new Runtime(); + + let requestRenameVariableWasCalled = false; + runtime.ioDevices.cloud.requestRenameVariable = () => { + requestRenameVariableWasCalled = true; + }; + + const target = new Target(runtime); + const mockCloudVar = new Variable('foo', 'bar', Variable.SCALAR_TYPE, true); + target.variables[mockCloudVar.id] = mockCloudVar; + runtime.targets.push(target); + + target.renameVariable('foo', 'bar2'); + + const variables = target.variables; + t.equal(Object.keys(variables).length, 1); + const variable = variables[Object.keys(variables)[0]]; + t.equal(variable.id, 'foo'); + t.equal(variable.name, 'bar2'); + t.equal(variable.value, 0); + t.equal(variable.isCloud, true); + t.equal(requestRenameVariableWasCalled, false); + + t.end(); +}); + // Delete Variable tests. test('deleteVariable', t => { const target = new Target(); @@ -200,6 +255,51 @@ test('deleteVariable2', t => { t.end(); }); +test('deleteVariable calls cloud io device\'s requestRenameVariable function', t => { + const runtime = new Runtime(); + + let requestDeleteVariableWasCalled = false; + runtime.ioDevices.cloud.requestDeleteVariable = () => { + requestDeleteVariableWasCalled = true; + }; + + const target = new Target(runtime); + target.isStage = true; + const mockCloudVar = new Variable('foo', 'bar', Variable.SCALAR_TYPE, true); + target.variables[mockCloudVar.id] = mockCloudVar; + runtime.targets.push(target); + + target.deleteVariable('foo'); + + const variables = target.variables; + t.equal(Object.keys(variables).length, 0); + t.equal(requestDeleteVariableWasCalled, true); + + t.end(); +}); + +test('deleteVariable calls cloud io device\'s requestRenameVariable function', t => { + const runtime = new Runtime(); + + let requestDeleteVariableWasCalled = false; + runtime.ioDevices.cloud.requestDeleteVariable = () => { + requestDeleteVariableWasCalled = true; + }; + + const target = new Target(runtime); + const mockCloudVar = new Variable('foo', 'bar', Variable.SCALAR_TYPE, true); + target.variables[mockCloudVar.id] = mockCloudVar; + runtime.targets.push(target); + + target.deleteVariable('foo'); + + const variables = target.variables; + t.equal(Object.keys(variables).length, 0); + t.equal(requestDeleteVariableWasCalled, false); + + t.end(); +}); + test('duplicateVariable creates a new variable with a new ID by default', t => { const target = new Target(); target.createVariable('a var ID', 'foo', Variable.SCALAR_TYPE); diff --git a/test/unit/io_cloud.js b/test/unit/io_cloud.js index 52473e13b..3cf398dff 100644 --- a/test/unit/io_cloud.js +++ b/test/unit/io_cloud.js @@ -142,3 +142,50 @@ test('requestCreateVariable calls provider\'s createVariable function', t => { t.strictEquals(mockVariable.isCloud, false); t.end(); }); + +test('requestRenameVariable calls provider\'s renameVariable function', t => { + let renameVariableCalled = false; + let mockVarOldName; + let mockVarNewName; + const mockRenameVariable = (oldName, newName) => { + renameVariableCalled = true; + mockVarOldName = oldName; + mockVarNewName = newName; + return; + }; + + const provider = { + renameVariable: mockRenameVariable + }; + + const runtime = new Runtime(); + const cloud = new Cloud(runtime); + cloud.setProvider(provider); + cloud.requestRenameVariable('my var', 'new var name'); + t.equals(renameVariableCalled, true); + t.strictEquals(mockVarOldName, 'my var'); + t.strictEquals(mockVarNewName, 'new var name'); + t.end(); +}); + +test('requestDeleteVariable calls provider\'s deleteVariable function', t => { + let deleteVariableCalled = false; + let mockVarName; + const mockDeleteVariable = name => { + deleteVariableCalled = true; + mockVarName = name; + return; + }; + + const provider = { + deleteVariable: mockDeleteVariable + }; + + const runtime = new Runtime(); + const cloud = new Cloud(runtime); + cloud.setProvider(provider); + cloud.requestDeleteVariable('my var'); + t.equals(deleteVariableCalled, true); + t.strictEquals(mockVarName, 'my var'); + t.end(); +}); From 63c4b53b098ada3397ece1c636b490d67058feda Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Wed, 21 Nov 2018 11:45:16 -0500 Subject: [PATCH 3/3] Replace reference to deleted variable with specific variable info needed. --- src/engine/target.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/engine/target.js b/src/engine/target.js index 14dc69c09..31d56715c 100644 --- a/src/engine/target.js +++ b/src/engine/target.js @@ -319,11 +319,13 @@ class Target extends EventEmitter { */ deleteVariable (id) { if (this.variables.hasOwnProperty(id)) { - const variable = this.variables[id]; + // Get info about the variable before deleting it + const deletedVariableName = this.variables[id].name; + const deletedVariableWasCloud = this.variables[id].isCloud; delete this.variables[id]; if (this.runtime) { - if (variable.isCloud && this.isStage) { - this.runtime.ioDevices.cloud.requestDeleteVariable(variable.name); + if (deletedVariableWasCloud && this.isStage) { + this.runtime.ioDevices.cloud.requestDeleteVariable(deletedVariableName); } this.runtime.monitorBlocks.deleteBlock(id); this.runtime.requestRemoveMonitor(id);