From d903b5e86ba247dbecd31be16b4513eaa94ee889 Mon Sep 17 00:00:00 2001 From: marisaleung Date: Tue, 25 Apr 2017 18:11:21 -0700 Subject: [PATCH] VariableMap and functions added. --- core/field_variable.js | 2 +- core/variables.js | 23 +- core/workspace.js | 324 ++++++++++---- core/xml.js | 2 +- generators/dart.js | 2 +- generators/javascript.js | 2 +- generators/php/procedures.js | 3 +- generators/python.js | 2 +- generators/python/procedures.js | 3 +- msg/messages.js | 2 + tests/jsunit/workspace_test.js | 723 +++++++++++++++++++++++++++++++- 11 files changed, 982 insertions(+), 106 deletions(-) diff --git a/core/field_variable.js b/core/field_variable.js index 553d2a554..d02dd2a7f 100644 --- a/core/field_variable.js +++ b/core/field_variable.js @@ -137,7 +137,7 @@ Blockly.FieldVariable.dropdownCreate = function() { if (this.sourceBlock_ && this.sourceBlock_.workspace) { // Get a copy of the list, so that adding rename and new variable options // doesn't modify the workspace's list. - var variableList = this.sourceBlock_.workspace.variableList.slice(0); + var variableList = this.sourceBlock_.workspace.getVariablesOfType(''); } else { var variableList = []; } diff --git a/core/variables.js b/core/variables.js index e1312864b..528dc19b2 100644 --- a/core/variables.js +++ b/core/variables.js @@ -79,7 +79,7 @@ Blockly.Variables.allUsedVariables = function(root) { * Find all variables that the user has created through the workspace or * toolbox. For use by generators. * @param {!Blockly.Workspace} root The workspace to inspect. - * @return {!Array.} Array of variable names. + * @return {!Array.} Array of variable models. */ Blockly.Variables.allVariables = function(root) { if (root instanceof Blockly.Block) { @@ -87,8 +87,9 @@ Blockly.Variables.allVariables = function(root) { console.warn('Deprecated call to Blockly.Variables.allVariables ' + 'with a block instead of a workspace. You may want ' + 'Blockly.Variables.allUsedVariables'); + return {}; } - return root.variableList; + return root.getAllVariables(); }; /** @@ -97,7 +98,7 @@ Blockly.Variables.allVariables = function(root) { * @return {!Array.} Array of XML block elements. */ Blockly.Variables.flyoutCategory = function(workspace) { - var variableList = workspace.variableList; + var variableList = workspace.getVariablesOfType(''); variableList.sort(goog.string.caseInsensitiveCompare); var xmlList = []; @@ -162,7 +163,7 @@ Blockly.Variables.flyoutCategory = function(workspace) { * @return {string} New variable name. */ Blockly.Variables.generateUniqueName = function(workspace) { - var variableList = workspace.variableList; + var variableList = workspace.getAllVariables(); var newName = ''; if (variableList.length) { var nameSuffix = 1; @@ -172,7 +173,7 @@ Blockly.Variables.generateUniqueName = function(workspace) { while (!newName) { var inUse = false; for (var i = 0; i < variableList.length; i++) { - if (variableList[i].toLowerCase() == potName) { + if (variableList[i].name.toLowerCase() == potName) { // This potential name is already used. inUse = true; break; @@ -215,13 +216,21 @@ Blockly.Variables.createVariable = function(workspace, opt_callback) { Blockly.Variables.promptName(Blockly.Msg.NEW_VARIABLE_TITLE, defaultName, function(text) { if (text) { - if (workspace.variableIndexOf(text) != -1) { + if (workspace.getVariable(text)) { Blockly.alert(Blockly.Msg.VARIABLE_ALREADY_EXISTS.replace('%1', text.toLowerCase()), function() { promptAndCheckWithAlert(text); // Recurse }); - } else { + } + else if (!Blockly.Procedures.isLegalName_(text, workspace)) { + Blockly.alert(Blockly.Msg.PROCEDURE_ALREADY_EXISTS.replace('%1', + text.toLowerCase()), + function() { + promptAndCheckWithAlert(text); // Recurse + }); + } + else { workspace.createVariable(text); if (opt_callback) { opt_callback(text); diff --git a/core/workspace.js b/core/workspace.js index d5c355d3f..dac7346f0 100644 --- a/core/workspace.js +++ b/core/workspace.js @@ -74,12 +74,15 @@ Blockly.Workspace = function(opt_options) { * @private */ this.blockDB_ = Object.create(null); - /* - * @type {!Array.} - * A list of all of the named variables in the workspace, including variables + + /** + * @type {!Object>} + * A map from variable type to list of variable names. The lists contain all + * of the named variables in the workspace, including variables * that are not currently in use. + * @private */ - this.variableList = []; + this.variableMap_ = Object.create(null); }; /** @@ -115,19 +118,20 @@ Blockly.Workspace.SCAN_ANGLE = 3; /** * Add a block to the list of top blocks. - * @param {!Blockly.Block} block Block to remove. + * @param {!Blockly.Block} block Block to add. */ Blockly.Workspace.prototype.addTopBlock = function(block) { this.topBlocks_.push(block); - if (this.isFlyout) { - // This is for the (unlikely) case where you have a variable in a block in - // an always-open flyout. It needs to be possible to edit the block in the - // flyout, so the contents of the dropdown need to be correct. - var variables = Blockly.Variables.allUsedVariables(block); - for (var i = 0; i < variables.length; i++) { - if (this.variableList.indexOf(variables[i]) == -1) { - this.variableList.push(variables[i]); - } + if (!this.isFlyout) { + return; + } + // This is for the (unlikely) case where you have a variable in a block in + // an always-open flyout. It needs to be possible to edit the block in the + // flyout, so the contents of the dropdown need to be correct. + var variableNames = Blockly.Variables.allUsedVariables(block); + for (var i = 0, name; name = variableNames[i]; i++) { + if (!this.getVariable(name)) { + this.createVariable(name); } } }; @@ -191,84 +195,176 @@ Blockly.Workspace.prototype.clear = function() { if (!existingGroup) { Blockly.Events.setGroup(false); } - - this.variableList.length = 0; + this.variableMap_ = Object.create(null); }; /** - * Walk the workspace and update the list of variables to only contain ones in + * Walk the workspace and update the map of variables to only contain ones in * use on the workspace. Use when loading new workspaces from disk. - * @param {boolean} clearList True if the old variable list should be cleared. + * @param {boolean} clear True if the old variable map should be cleared. */ -Blockly.Workspace.prototype.updateVariableList = function(clearList) { +Blockly.Workspace.prototype.updateVariableStore = function(clear) { // TODO: Sort - if (!this.isFlyout) { - // Update the list in place so that the flyout's references stay correct. - if (clearList) { - this.variableList.length = 0; + if (this.isFlyout) { + return; + } + var variableNames = Blockly.Variables.allUsedVariables(this); + var varList = []; + for (var i = 0, name; name = variableNames[i]; i++) { + // Get variable model with the used variable name. + var tempVar = this.getVariable(name); + if (tempVar) { + varList.push({'name': tempVar.name, 'type': tempVar.type, + 'id': tempVar.getId()}); } - var allVariables = Blockly.Variables.allUsedVariables(this); - for (var i = 0; i < allVariables.length; i++) { - this.createVariable(allVariables[i]); + else { + varList.push({'name': name, 'type': null, 'id': null}); + // TODO(marisaleung): Use variable.type and variable.getId() once variable + // instances are storing more than just name. + } + } + if (clear) { + this.variableMap_ = Object.create(null); + } + // Update the list in place so that the flyout's references stay correct. + for (var i = 0, varDict; varDict = varList[i]; i++) { + if (!this.getVariable(varDict.name)) { + this.createVariable(varDict.name, varDict.type, varDict.id); } } }; /** - * Rename a variable by updating its name in the variable list. + * Rename a variable by updating its name in the variable map. Identify the + * variable to rename with the given variable. * TODO: #468 - * @param {string} oldName Variable to rename. + * @param {?Blockly.VariableModel} variable Variable to rename. * @param {string} newName New variable name. */ -Blockly.Workspace.prototype.renameVariable = function(oldName, newName) { - // Find the old name in the list. - var variableIndex = this.variableIndexOf(oldName); - var newVariableIndex = this.variableIndexOf(newName); +Blockly.Workspace.prototype.renameVariableInternal_ = function(variable, newName) { + var newVariable = this.getVariable(newName); + var oldCase; + var variableIndex = -1; + var newVariableIndex = -1; + var type; - // We might be renaming to an existing name but with different case. If so, - // we will also update all of the blocks using the new name to have the - // correct case. - if (newVariableIndex != -1 && - this.variableList[newVariableIndex] != newName) { - var oldCase = this.variableList[newVariableIndex]; + // If they are different types, throw an error. + if (variable && newVariable && variable.type != newVariable.type) { + throw Error('Variable "' + oldName + '" is type "' + variable.type + + '" and variable "' + newName + '" is type "' + newVariable.type + + '". Both must be the same type.'); + } + if (variable || newVariable) { + type = (variable || newVariable).type; + } + else { + type = ''; + } + + var variableList = this.getVariablesOfType(type); + if (variable) { + variableIndex = variableList.indexOf(variable); + } + if (newVariable){ + newVariableIndex = variableList.indexOf(newVariable); + } + + // Find if newVariable case is different. + if (newVariableIndex != -1 && newVariable.name != newName) { + oldCase = newVariable.name; } Blockly.Events.setGroup(true); var blocks = this.getAllBlocks(); - // Iterate through every block. + // Iterate through every block and update name. for (var i = 0; i < blocks.length; i++) { - blocks[i].renameVar(oldName, newName); + blocks[i].renameVar(variable.name, newName); if (oldCase) { blocks[i].renameVar(oldCase, newName); } } Blockly.Events.setGroup(false); - - if (variableIndex == newVariableIndex || + if (variableIndex == -1 && newVariableIndex == -1) { + this.createVariable(newName, ''); + console.log('Tried to rename an non-existent variable.'); + } + else if (variableIndex == newVariableIndex || variableIndex != -1 && newVariableIndex == -1) { // Only changing case, or renaming to a completely novel name. - this.variableList[variableIndex] = newName; + this.variableMap_[type][variableIndex].name = newName; } else if (variableIndex != -1 && newVariableIndex != -1) { // Renaming one existing variable to another existing variable. // The case might have changed, so we update the destination ID. - this.variableList[newVariableIndex] = newName; - this.variableList.splice(variableIndex, 1); - } else { - this.variableList.push(newName); - console.log('Tried to rename an non-existent variable.'); + this.variableMap_[type][newVariableIndex].name = newName; + this.variableMap_[type].splice(variableIndex, 1); } }; + /** - * Create a variable with the given name. + * Rename a variable by updating its name in the variable map. Identify the + * variable to rename with the given name. * TODO: #468 - * @param {string} name The new variable's name. + * @param {string} oldName Variable to rename. + * @param {string} newName New variable name. */ -Blockly.Workspace.prototype.createVariable = function(name) { - var index = this.variableIndexOf(name); - if (index == -1) { - this.variableList.push(name); +Blockly.Workspace.prototype.renameVariable = function(oldName, newName) { + // Warning: Prefer to use renameVariableById. + var variable = this.getVariable(oldName); + this.renameVariableInternal_(variable, newName); +}; + +/** + * Rename a variable by updating its name in the variable map. Identify the + * variable to rename with the given id. + * @param {string} id Id of the variable to rename. + * @param {string} newName New variable name. + */ +Blockly.Workspace.prototype.renameVariableById = function(id, newName) { + var variable = this.getVariableById(id); + this.renameVariableInternal_(variable, newName); +} + +/** + * Create a variable with a given name, optional type, and optional id. + * @param {!string} name The name of the variable. This must be unique across + * variables and procedures. + * @param {?string} opt_type The type of the variable like 'int' or 'string'. + * Does not need to be unique. Field_variable can filter variables based on + * their type. This will default to '' which is a specific type. + * @param {?string} opt_id The unique id of the variable. This will default to + * a UUID. + */ +Blockly.Workspace.prototype.createVariable = function(name, opt_type, opt_id) { + var variable = this.getVariable(name); + if (variable) { + if (opt_type && variable.type != opt_type) { + throw Error('Variable "' + name + '" is already in use and its type is "' + + variable.type + '" which conflicts with the passed in ' + + 'type, "' + opt_type + '".'); + } + if (opt_id && variable.getId() != opt_id) { + throw Error('Variable "' + name + '" is already in use and its id is "' + + variable.getId() + '" which conflicts with the passed in ' + + 'id, "' + opt_id + '".'); + } + return; + } + if (opt_id && this.getVariableById(opt_id)) { + throw Error('Variable id, "' + opt_id + '", is already in use.'); + } + opt_id = opt_id || Blockly.utils.genUid(); + opt_type = opt_type || ''; + + variable = new Blockly.VariableModel(name, opt_type, opt_id); + // If opt_type is not a key, create a new list. + if (!this.variableMap_[opt_type]) { + this.variableMap_[opt_type] = [variable]; + } + // Else append the variable to the preexisting list. + else { + this.variableMap_[opt_type].push(variable); } }; @@ -297,15 +393,11 @@ Blockly.Workspace.prototype.getVariableUses = function(name) { }; /** - * Delete a variables and all of its uses from this workspace. May prompt the - * user for confirmation. + * Delete a variable by the passed in name and all of its uses from this + * workspace. May prompt the user for confirmation. * @param {string} name Name of variable to delete. */ Blockly.Workspace.prototype.deleteVariable = function(name) { - var variableIndex = this.variableIndexOf(name); - if (variableIndex == -1) { - return; - } // Check whether this variable is a function parameter before deleting. var uses = this.getVariableUses(name); for (var i = 0, block; block = uses[i]; i++) { @@ -321,6 +413,7 @@ Blockly.Workspace.prototype.deleteVariable = function(name) { } var workspace = this; + var variable = workspace.getVariable(name); if (uses.length > 1) { // Confirm before deleting multiple blocks. Blockly.confirm( @@ -328,29 +421,49 @@ Blockly.Workspace.prototype.deleteVariable = function(name) { replace('%2', name), function(ok) { if (ok) { - workspace.deleteVariableInternal_(name); + workspace.deleteVariableInternal_(variable); } }); } else { // No confirmation necessary for a single block. - this.deleteVariableInternal_(name); + this.deleteVariableInternal_(variable); + } +}; + +/** + * Delete a variables by the passed in id and all of its uses from this + * workspace. May prompt the user for confirmation. + * @param {string} id Id of variable to delete. + */ +Blockly.Workspace.prototype.deleteVariableById = function(id) { + var variable = this.getVariableById(id); + if (variable) { + this.deleteVariableInternal_(variable); } }; /** * Deletes a variable and all of its uses from this workspace without asking the * user for confirmation. + * @param {Blockly.VariableModel} variable Variable to delete. * @private */ -Blockly.Workspace.prototype.deleteVariableInternal_ = function(name) { - var uses = this.getVariableUses(name); - var variableIndex = this.variableIndexOf(name); +Blockly.Workspace.prototype.deleteVariableInternal_ = function(variable) { + var uses = this.getVariableUses(variable.name); Blockly.Events.setGroup(true); for (var i = 0; i < uses.length; i++) { uses[i].dispose(true, false); } Blockly.Events.setGroup(false); - this.variableList.splice(variableIndex, 1); + + var type = variable.type; + for (var i = 0, tempVar; tempVar = this.variableMap_[type][i]; i++) { + if (Blockly.Names.equals(tempVar.name, variable.name)) { + delete this.variableMap_[type][i]; + this.variableMap_[type].splice(i, 1); + return; + } + } }; /** @@ -359,14 +472,48 @@ Blockly.Workspace.prototype.deleteVariableInternal_ = function(name) { * @param {string} name The name to check for. * @return {number} The index of the name in the variable list, or -1 if it is * not present. + * @deprecated April 2017 */ Blockly.Workspace.prototype.variableIndexOf = function(name) { - for (var i = 0, varname; varname = this.variableList[i]; i++) { - if (Blockly.Names.equals(varname, name)) { - return i; + console.warn( + 'Deprecated call to Blockly.Workspace.prototype.variableIndexOf'); + return -1; +}; + +/** + * Find the variable by the given name and return it. Return null if it is not + * found. + * @param {!string} name The name to check for. + * @return {?Blockly.VariableModel} the variable with the given name. + */ +Blockly.Workspace.prototype.getVariable = function(name) { + var keys = Object.keys(this.variableMap_); + for (var i = 0; i < keys.length; i++ ) { + var key = keys[i]; + for (var j = 0, variable; variable = this.variableMap_[key][j]; j++) { + if (Blockly.Names.equals(variable.name, name)) { + return variable; + } } } - return -1; + return null; +}; + +/** + * Find the variable by the given id and return it. Return null if it is not + * found. + * @param {!string} id The id to check for. + * @return {?Blockly.VariableModel} The variable with the given id. + */ +Blockly.Workspace.prototype.getVariableById = function(id) { + for (var key of Object.keys(this.variableMap_)) { + for (var variable of this.variableMap_[key]) { + if (variable.getId() == id) { + return variable; + } + } + } + return null; }; /** @@ -504,6 +651,43 @@ Blockly.Workspace.prototype.allInputsFilled = function(opt_shadowBlocksAreFilled return true; }; +/** + * Find the variable with the specified type. If type is null, return list of + * variables with empty string type. + * @param {?string} type Type of the variables to find. + * @return {Array.} The sought after variables of the + * passed in type. An empty array if none are found. + */ +Blockly.Workspace.prototype.getVariablesOfType = function(type) { + type = type || ''; + var variable_list = this.variableMap_[type]; + if (variable_list) { + return variable_list; + } + return []; +}; + +/** + * Return all variable types. + * @return {!Array.} List of variable types. + */ +Blockly.Workspace.prototype.getVariableTypes = function() { + return Object.keys(this.variableMap_); +}; + +/** + * Return all variables of all types. + * @return {!Array.} List of variable models. + */ +Blockly.Workspace.prototype.getAllVariables = function() { + var all_variables = []; + var keys = Object.keys(this.variableMap_); + for (var i = 0; i < keys.length; i++ ) { + all_variables = all_variables.concat(this.variableMap_[keys[i]]); + } + return all_variables; +}; + /** * Database of all workspaces. * @private diff --git a/core/xml.js b/core/xml.js index 8506ca51d..8f0c2738c 100644 --- a/core/xml.js +++ b/core/xml.js @@ -331,7 +331,7 @@ Blockly.Xml.domToWorkspace = function(xml, workspace) { } Blockly.Field.stopCache(); - workspace.updateVariableList(false); + workspace.updateVariableStore(false); // Re-enable workspace resizing. if (workspace.setResizesEnabled) { workspace.setResizesEnabled(true); diff --git a/generators/dart.js b/generators/dart.js index 094e772a9..295d9002a 100644 --- a/generators/dart.js +++ b/generators/dart.js @@ -103,7 +103,7 @@ Blockly.Dart.init = function(workspace) { } var defvars = []; - var variables = workspace.variableList; + var variables = workspace.getAllVariables(); if (variables.length) { for (var i = 0; i < variables.length; i++) { defvars[i] = Blockly.Dart.variableDB_.getName(variables[i], diff --git a/generators/javascript.js b/generators/javascript.js index 2090f436e..b641d2dce 100644 --- a/generators/javascript.js +++ b/generators/javascript.js @@ -153,7 +153,7 @@ Blockly.JavaScript.init = function(workspace) { } var defvars = []; - var variables = workspace.variableList; + var variables = workspace.getAllVariables(); if (variables.length) { for (var i = 0; i < variables.length; i++) { defvars[i] = Blockly.JavaScript.variableDB_.getName(variables[i], diff --git a/generators/php/procedures.js b/generators/php/procedures.js index 537b97364..ad2cc1849 100644 --- a/generators/php/procedures.js +++ b/generators/php/procedures.js @@ -33,7 +33,8 @@ Blockly.PHP['procedures_defreturn'] = function(block) { // First, add a 'global' statement for every variable that is not shadowed by // a local parameter. var globals = []; - for (var i = 0, varName; varName = block.workspace.variableList[i]; i++) { + var variables = workspace.getAllVariables(); + for (var i = 0, varName; varName = variables[i]; i++) { if (block.arguments_.indexOf(varName) == -1) { globals.push(Blockly.PHP.variableDB_.getName(varName, Blockly.Variables.NAME_TYPE)); diff --git a/generators/python.js b/generators/python.js index 82cc9bbc4..3babbde6a 100644 --- a/generators/python.js +++ b/generators/python.js @@ -161,7 +161,7 @@ Blockly.Python.init = function(workspace) { } var defvars = []; - var variables = workspace.variableList; + var variables = workspace.getAllVariables(); for (var i = 0; i < variables.length; i++) { defvars[i] = Blockly.Python.variableDB_.getName(variables[i], Blockly.Variables.NAME_TYPE) + ' = None'; diff --git a/generators/python/procedures.js b/generators/python/procedures.js index 6af144819..94804fd47 100644 --- a/generators/python/procedures.js +++ b/generators/python/procedures.js @@ -34,7 +34,8 @@ Blockly.Python['procedures_defreturn'] = function(block) { // First, add a 'global' statement for every variable that is not shadowed by // a local parameter. var globals = []; - for (var i = 0, varName; varName = block.workspace.variableList[i]; i++) { + var variables = workspace.getAllVariables(); + for (var i = 0, varName; varName = variables[i]; i++) { if (block.arguments_.indexOf(varName) == -1) { globals.push(Blockly.Python.variableDB_.getName(varName, Blockly.Variables.NAME_TYPE)); diff --git a/msg/messages.js b/msg/messages.js index 627292a38..6bed0634d 100644 --- a/msg/messages.js +++ b/msg/messages.js @@ -125,6 +125,8 @@ Blockly.Msg.NEW_VARIABLE = 'Create variable...'; Blockly.Msg.NEW_VARIABLE_TITLE = 'New variable name:'; /// alert - Tells the user that the name they entered is already in use. Blockly.Msg.VARIABLE_ALREADY_EXISTS = 'A variable named "%1" already exists.' +/// alert - Tells the user that the name they entered is already in use for a procedure. +Blockly.Msg.PROCEDURE_ALREADY_EXISTS = 'A procedure named "%1" already exists.' // Variable deletion. /// confirm - Ask the user to confirm their deletion of multiple uses of a variable. diff --git a/tests/jsunit/workspace_test.js b/tests/jsunit/workspace_test.js index 5c4bed429..379ea36ec 100644 --- a/tests/jsunit/workspace_test.js +++ b/tests/jsunit/workspace_test.js @@ -19,8 +19,119 @@ */ 'use strict'; +goog.require('goog.testing'); +goog.require('goog.testing.MockControl'); + +var workspace; +var mockControl_; +var saved_msg = Blockly.Msg.DELETE_VARIABLE; +Blockly.defineBlocksWithJsonArray([{ + "type": "get_var_block", + "message0": "%1", + "args0": [ + { + "type": "field_variable", + "name": "VAR", + } + ] +}]); + +function workspaceTest_setUp() { + workspace = new Blockly.Workspace(); + mockControl_ = new goog.testing.MockControl(); +} + +function workspaceTest_setUpWithMockBlocks() { + workspaceTest_setUp(); + // Need to define this because field_variable's dropdownCreate() calls replace + // on undefined value, Blockly.Msg.DELETE_VARIABLE. To fix this, define + // Blockly.Msg.DELETE_VARIABLE as %1 so the replace function finds the %1 it + // expects. + Blockly.Msg.DELETE_VARIABLE = '%1'; +} + +function workspaceTest_tearDown() { + mockControl_.$tearDown(); + workspace.dispose(); +} + +function workspaceTest_tearDownWithMockBlocks() { + workspaceTest_tearDown(); + Blockly.Msg.DELETE_VARIABLE = saved_msg; +} + +/** + * Create a test get_var_block. + * @param {?string} variable The string to put into the variable field. + * @return {!Blockly.Block} The created block. + */ +function createMockBlock(variable) { + var block = new Blockly.Block(workspace, 'get_var_block'); + block.inputList[0].fieldRow[0].setValue(variable); + return block; +} + +/** + * Check that two arrays have the same content. + * @param {!Array.} array1 The first array. + * @param {!Array.} array2 The second array. + */ +function isEqualArrays(array1, array2) { + assertEquals(array1.length, array2.length); + for (var i = 0; i < array1.length; i++) { + assertEquals(array1[i], array2[i]); + } +} + +/** + * Check if a variable with the given values exists. + * @param {!string} name The expected name of the variable. + * @param {!string} type The expected type of the variable. + * @param {!string} id The expected id of the variable. + */ +function checkVariableValues(name, type, id) { + var variable = workspace.getVariable(name); + assertNotUndefined(variable); + assertEquals(name, variable.name); + assertEquals(type, variable.type); + assertEquals(id, variable.getId()); +} + +/** + * Create a variable with the specified parameters and return it. + * @param {!string} name The name of the variable. + * @param {!string} opt_type The type of the variable. + * @param {!string} opt_id The id of the variable. + * @return {!Blockly.VariableModel} The created variable. + */ +function createAndGetVariable(name, opt_type, opt_id) { + workspace.createVariable(name, opt_type, opt_id); + return workspace.getVariable(name); +} + +/** + * Creates a controlled MethodMock. Set the expected return values. Set the + * method to replay. + * @param {!Object} scope The scope of the method to be mocked out. + * @param {!string} funcName The name of the function we're going to mock. + * @param {Object} parameters The parameters to call the mock with. + * @param {!Object} return_value The value to return when called. + * @return {!goog.testing.MockInterface} The mocked method. + */ +function setUpMockMethod(scope, funcName, parameters, return_value) { + var mockMethod = mockControl_.createMethodMock(scope, funcName); + if (parameters) { + mockMethod(parameters).$returns(return_value); + } + else { + mockMethod().$returns(return_value); + } + mockMethod.$replay(); + return mockMethod; +} + function test_emptyWorkspace() { - var workspace = new Blockly.Workspace(); + workspaceTest_setUp(); try { assertEquals('Empty workspace (1).', 0, workspace.getTopBlocks(true).length); assertEquals('Empty workspace (2).', 0, workspace.getTopBlocks(false).length); @@ -29,20 +140,20 @@ function test_emptyWorkspace() { assertEquals('Empty workspace (4).', 0, workspace.getTopBlocks(true).length); assertEquals('Empty workspace (5).', 0, workspace.getTopBlocks(false).length); assertEquals('Empty workspace (6).', 0, workspace.getAllBlocks().length); - } finally { - workspace.dispose(); + } + finally { + workspaceTest_tearDown(); } } function test_flatWorkspace() { - var workspace = new Blockly.Workspace(); - var blockA, blockB; + workspaceTest_setUp(); try { - blockA = workspace.newBlock(''); + var blockA = workspace.newBlock(''); assertEquals('One block workspace (1).', 1, workspace.getTopBlocks(true).length); assertEquals('One block workspace (2).', 1, workspace.getTopBlocks(false).length); assertEquals('One block workspace (3).', 1, workspace.getAllBlocks().length); - blockB = workspace.newBlock(''); + var blockB = workspace.newBlock(''); assertEquals('Two block workspace (1).', 2, workspace.getTopBlocks(true).length); assertEquals('Two block workspace (2).', 2, workspace.getTopBlocks(false).length); assertEquals('Two block workspace (3).', 2, workspace.getAllBlocks().length); @@ -55,17 +166,15 @@ function test_flatWorkspace() { assertEquals('Cleared workspace (2).', 0, workspace.getTopBlocks(false).length); assertEquals('Cleared workspace (3).', 0, workspace.getAllBlocks().length); } finally { - blockB && blockB.dispose(); - blockA && blockA.dispose(); - workspace.dispose(); + workspaceTest_tearDown(); } } function test_maxBlocksWorkspace() { - var workspace = new Blockly.Workspace(); - var blockA = workspace.newBlock(''); - var blockB = workspace.newBlock(''); + workspaceTest_setUp(); try { + var blockA = workspace.newBlock(''); + var blockB = workspace.newBlock(''); assertEquals('Infinite capacity.', Infinity, workspace.remainingCapacity()); workspace.options.maxBlocks = 3; assertEquals('Three capacity.', 1, workspace.remainingCapacity()); @@ -78,9 +187,7 @@ function test_maxBlocksWorkspace() { workspace.clear(); assertEquals('Cleared capacity.', 0, workspace.remainingCapacity()); } finally { - blockB.dispose(); - blockA.dispose(); - workspace.dispose(); + workspaceTest_tearDown(); } } @@ -106,10 +213,10 @@ function test_getWorkspaceById() { } function test_getBlockById() { - var workspace = new Blockly.Workspace(); - var blockA = workspace.newBlock(''); - var blockB = workspace.newBlock(''); + workspaceTest_setUp(); try { + var blockA = workspace.newBlock(''); + var blockB = workspace.newBlock(''); assertEquals('Find blockA.', blockA, workspace.getBlockById(blockA.id)); assertEquals('Find blockB.', blockB, workspace.getBlockById(blockB.id)); assertEquals('No block found.', null, @@ -120,8 +227,580 @@ function test_getBlockById() { workspace.clear(); assertEquals('Can\'t find blockB.', null, workspace.getBlockById(blockB.id)); } finally { - blockB.dispose(); - blockA.dispose(); - workspace.dispose(); + workspaceTest_tearDown(); } } + +function test_getVariable_Trivial() { + workspaceTest_setUp(); + var var_1 = this.createAndGetVariable('name1', 'type1', 'id1'); + var var_2 = this.createAndGetVariable('name2', 'type1', 'id2'); + var var_3 = this.createAndGetVariable('name3', 'type2', 'id3'); + var result_1 = workspace.getVariable('name1'); + var result_2 = workspace.getVariable('name2'); + var result_3 = workspace.getVariable('name3'); + + assertEquals(var_1, result_1); + assertEquals(var_2, result_2); + assertEquals(var_3, result_3); + workspaceTest_tearDown(); +} + +function test_getVariable_NotFound() { + workspaceTest_setUp(); + var result = workspace.getVariable('name1'); + assertNull(result); + workspaceTest_tearDown(); +} + +function test_getVariableById_Trivial() { + workspaceTest_setUp(); + var var_1 = this.createAndGetVariable('name1', 'type1', 'id1'); + var var_2 = this.createAndGetVariable('name2', 'type1', 'id2'); + var var_3 = this.createAndGetVariable('name3', 'type2', 'id3'); + var result_1 = workspace.getVariableById('id1'); + var result_2 = workspace.getVariableById('id2'); + var result_3 = workspace.getVariableById('id3'); + + assertEquals(var_1, result_1); + assertEquals(var_2, result_2); + assertEquals(var_3, result_3); + workspaceTest_tearDown(); +} + +function test_getVariableById_NotFound() { + workspaceTest_setUp(); + var result = workspace.getVariableById('id1'); + assertNull(result); + workspaceTest_tearDown(); +} + +function test_createVariable_Trivial() { + workspaceTest_setUp(); + workspace.createVariable('name1', 'type1', 'id1'); + checkVariableValues('name1', 'type1', 'id1') + workspaceTest_tearDown(); +} + +function test_createVariable_AlreadyExists() { + // Expect that when the variable already exists, the variableMap_ is unchanged. + workspaceTest_setUp(); + var var_1 = this.createAndGetVariable('name1', 'type1', 'id1'); + // Assert there is only one variable in the workspace. + var keys = Object.keys(workspace.variableMap_); + assertEquals(1, keys.length); + assertEquals(1, workspace.variableMap_[keys[0]].length); + + workspace.createVariable('name1'); + checkVariableValues('name1', 'type1', 'id1'); + // Check that the size of the variableMap_ did not change. + assertEquals(1, keys.length); + var varMapLength = workspace.variableMap_[keys[0]].length; + assertEquals(1, varMapLength); + workspaceTest_tearDown(); +} + +function test_createVariable_NullAndUndefinedType() { + workspaceTest_setUp(); + workspace.createVariable('name1', null, 'id1'); + workspace.createVariable('name2', undefined, 'id2'); + + checkVariableValues('name1', '', 'id1'); + checkVariableValues('name2', '', 'id2'); + workspaceTest_tearDown(); +} + +function test_createVariable_NullId() { + workspaceTest_setUp(); + var mockGenUid = setUpMockMethod(Blockly.utils, 'genUid', null, '1'); + try { + workspace.createVariable('name1', 'type1', null); + mockGenUid.$verify(); + checkVariableValues('name1', 'type1', '1'); + } + finally { + workspaceTest_tearDown(); + } +} + +function test_createVariable_UndefinedId() { + workspaceTest_setUp(); + var mockGenUid = setUpMockMethod(Blockly.utils, 'genUid', null, '1'); + try { + workspace.createVariable('name1', 'type1', undefined); + mockGenUid.$verify(); + checkVariableValues('name1', 'type1', '1'); + } + finally { + workspaceTest_tearDown(); + } +} + +function test_createVariable_IdAlreadyExists() { + workspaceTest_setUp(); + workspace.createVariable('name1', 'type1', 'id1'); + try { + workspace.createVariable('name2', 'type2', 'id1'); + fail(); + } catch (e) { + // expected + } + workspaceTest_tearDown(); +} + +function test_createVariable_MismatchedIdAndType() { + workspaceTest_setUp(); + workspace.createVariable('name1', 'type1', 'id1'); + try { + workspace.createVariable('name1', 'type2', 'id1'); + fail(); + } catch (e) { + // expected + } + try { + workspace.createVariable('name1', 'type1', 'id2'); + fail(); + } catch (e) { + // expected + } + workspaceTest_tearDown(); +} + +function test_createVariable_TwoSameTypes() { + workspaceTest_setUp(); + workspace.createVariable('name1', 'type1', 'id1'); + workspace.createVariable('name2', 'type1', 'id2'); + + checkVariableValues('name1', 'type1', 'id1'); + checkVariableValues('name2', 'type1', 'id2'); + workspaceTest_tearDown(); +} + +function test_deleteVariable_InternalTrivial() { + workspaceTest_setUpWithMockBlocks() + var var_1 = createAndGetVariable('name1', 'type1', 'id1'); + workspace.createVariable('name2', 'type2', 'id2'); + createMockBlock('name1'); + createMockBlock('name1'); + createMockBlock('name2'); + + workspace.deleteVariableInternal_(var_1); + var variable = workspace.getVariable('name1'); + var block_var_name = workspace.topBlocks_[0].getVars()[0]; + assertNull(variable); + checkVariableValues('name2', 'type2', 'id2'); + assertEquals('name2', block_var_name); + workspaceTest_tearDownWithMockBlocks(); +} + +// TODO(marisaleung): Test the alert for deleting a variable that is a procedure. + +function test_updateVariableStore_TrivialNoClear() { + workspaceTest_setUp(); + workspace.createVariable('name1', 'type1', 'id1'); + workspace.createVariable('name2', 'type2', 'id2'); + var mockAllUsedVariables = setUpMockMethod(Blockly.Variables, + 'allUsedVariables', workspace, ['name1', 'name2']); + + try { + workspace.updateVariableStore(); + mockAllUsedVariables.$verify(); + checkVariableValues('name1', 'type1', 'id1'); + checkVariableValues('name2', 'type2', 'id2'); + } + finally { + workspaceTest_tearDown(); + } +} + +function test_updateVariableStore_NameNotInvariableMap_NoClear() { + workspaceTest_setUp(); + setUpMockMethod(Blockly.Variables, 'allUsedVariables', workspace, ['name1']); + setUpMockMethod(Blockly.utils, 'genUid', null, '1'); + + try { + workspace.updateVariableStore(); + mockControl_.$verifyAll(); + checkVariableValues('name1', '', '1'); + } + finally { + workspaceTest_tearDown(); + } +} + +function test_updateVariableStore_ClearAndAllInUse() { + workspaceTest_setUp(); + workspace.createVariable('name1', 'type1', 'id1'); + workspace.createVariable('name2', 'type2', 'id2'); + var mockAllUsedVariables = setUpMockMethod(Blockly.Variables, + 'allUsedVariables', workspace, ['name1', 'name2']); + + try { + workspace.updateVariableStore(true); + mockAllUsedVariables.$verify(); + checkVariableValues('name1', 'type1', 'id1'); + checkVariableValues('name2', 'type2', 'id2'); + } + finally { + workspaceTest_tearDown(); + } +} + +function test_updateVariableStore_ClearAndOneInUse() { + workspaceTest_setUp(); + workspace.createVariable('name1', 'type1', 'id1'); + workspace.createVariable('name2', 'type2', 'id2'); + var mockAllUsedVariables = setUpMockMethod(Blockly.Variables, + 'allUsedVariables', workspace, ['name1']); + + try { + workspace.updateVariableStore(true); + mockAllUsedVariables.$verify(); + checkVariableValues('name1', 'type1', 'id1'); + var variabe = workspace.getVariable('name2'); + assertNull(variable); + } + finally { + workspaceTest_tearDown(); + } +} + +function test_addTopBlock_TrivialFlyoutIsTrue() { + workspaceTest_setUpWithMockBlocks() + workspace.isFlyout = true; + var block = createMockBlock(); + workspace.removeTopBlock(block); + setUpMockMethod(Blockly.Variables, 'allUsedVariables', block, ['name1']); + setUpMockMethod(Blockly.utils, 'genUid', null, '1'); + + try { + workspace.addTopBlock(block); + mockControl_.$verifyAll(); + checkVariableValues('name1', '', '1'); + } + finally { + workspaceTest_tearDownWithMockBlocks(); + } +} + +function test_clear_Trivial() { + workspaceTest_setUp(); + workspace.createVariable('name1', 'type1', 'id1'); + workspace.createVariable('name2', 'type2', 'id2'); + var mockSetGroup = mockControl_.createMethodMock(Blockly.Events, 'setGroup'); + mockSetGroup(true); + mockSetGroup(false); + mockSetGroup.$replay(); + + try { + workspace.clear(); + mockControl_.$verifyAll(); + var topBlocks_length = workspace.topBlocks_.length; + var varMapLength = Object.keys(workspace.variableMap_).length; + assertEquals(0, topBlocks_length); + assertEquals(0, varMapLength); + } + finally { + workspaceTest_tearDown(); + } +} + +function test_clear_NoVariables() { + workspaceTest_setUp(); + var mockSetGroup = mockControl_.createMethodMock(Blockly.Events, 'setGroup'); + mockSetGroup(true); + mockSetGroup(false); + mockSetGroup.$replay(); + + try { + workspace.clear(); + mockSetGroup.$verify(); + var topBlocks_length = workspace.topBlocks_.length; + var varMapLength = Object.keys(workspace.variableMap_).length; + assertEquals(0, topBlocks_length); + assertEquals(0, varMapLength); + } + finally { + workspaceTest_tearDown(); + } +} + +function test_renameVariable_NoBlocks() { + // Expect 'renameVariable' to create new variable with newName. + workspaceTest_setUp(); + var oldName = 'name1'; + var newName = 'name2'; + var mockSetGroup = mockControl_.createMethodMock(Blockly.Events, 'setGroup'); + var mockGenUid = mockControl_.createMethodMock(Blockly.utils, 'genUid'); + // Mocked setGroup to ensure only one call to the mocked genUid. + mockSetGroup(true); + mockSetGroup(false); + mockGenUid().$returns('1'); + mockControl_.$replayAll(); + + try { + workspace.renameVariable(oldName, newName); + mockControl_.$verifyAll(); + checkVariableValues('name2', '', '1'); + var variable = workspace.getVariable(oldName); + assertNull(variable); + } + finally { + workspaceTest_tearDown(); + } +} + +function test_renameVariable_SameNameNoBlocks() { + // Expect 'renameVariable' to create new variable with newName. + workspaceTest_setUpWithMockBlocks() + var name = 'name1'; + workspace.createVariable(name, 'type1', 'id1'); + + workspace.renameVariable(name, name); + checkVariableValues(name, 'type1', 'id1'); + workspaceTest_tearDownWithMockBlocks(); +} + +function test_renameVariable_OnlyOldNameBlockExists() { + // Expect 'renameVariable' to change oldName variable name to newName. + workspaceTest_setUpWithMockBlocks() + var oldName = 'name1'; + var newName = 'name2'; + workspace.createVariable(oldName, 'type1', 'id1'); + createMockBlock(oldName); + + workspace.renameVariable(oldName, newName); + checkVariableValues(newName, 'type1', 'id1'); + var variable = workspace.getVariable(oldName); + var block_var_name = workspace.topBlocks_[0].getVars()[0]; + assertNull(variable); + assertEquals(newName, block_var_name); + workspaceTest_tearDownWithMockBlocks(); +} + +function test_renameVariable_TwoVariablesSameType() { + // Expect 'renameVariable' to change oldName variable name to newName. + // Expect oldName block name to change to newName + workspaceTest_setUpWithMockBlocks() + var oldName = 'name1'; + var newName = 'name2'; + workspace.createVariable(oldName, 'type1', 'id1'); + workspace.createVariable(newName, 'type1', 'id2'); + createMockBlock(oldName); + createMockBlock(newName); + + workspace.renameVariable(oldName, newName); + checkVariableValues(newName, 'type1', 'id2'); + var variable = workspace.getVariable(oldName); + var block_var_name_1 = workspace.topBlocks_[0].getVars()[0]; + var block_var_name_2 = workspace.topBlocks_[1].getVars()[0]; + assertNull(variable); + assertEquals(newName, block_var_name_1); + assertEquals(newName, block_var_name_2); + workspaceTest_tearDownWithMockBlocks(); +} + +function test_renameVariable_TwoVariablesDifferentType() { + // Expect triggered error because of different types + workspaceTest_setUpWithMockBlocks() + var oldName = 'name1'; + var newName = 'name2'; + workspace.createVariable(oldName, 'type1', 'id1'); + workspace.createVariable(newName, 'type2', 'id2'); + createMockBlock(oldName); + createMockBlock(newName); + + try { + workspace.renameVariable(oldName, newName); + fail(); + } catch (e) { + // expected + } + checkVariableValues(oldName, 'type1', 'id1'); + checkVariableValues(newName, 'type2', 'id2'); + var block_var_name_1 = workspace.topBlocks_[0].getVars()[0]; + var block_var_name_2 = workspace.topBlocks_[1].getVars()[0]; + assertEquals(oldName, block_var_name_1); + assertEquals(newName, block_var_name_2); + workspaceTest_tearDownWithMockBlocks(); +} + +function test_renameVariable_OldCase() { + // Expect triggered error because of different types + workspaceTest_setUpWithMockBlocks(); + var oldCase = 'Name1'; + var newName = 'name1'; + workspace.createVariable(oldCase, 'type1', 'id1'); + createMockBlock(oldCase); + + workspace.renameVariable(oldCase, newName); + checkVariableValues(newName, 'type1', 'id1'); + var result_oldCase = workspace.getVariable(oldCase).name + assertNotEquals(oldCase, result_oldCase); + workspaceTest_tearDownWithMockBlocks(); +} + +function test_renameVariable_TwoVariablesAndOldCase() { + // Expect triggered error because of different types + workspaceTest_setUpWithMockBlocks() + var oldName = 'name1'; + var oldCase = 'Name2'; + var newName = 'name2'; + workspace.createVariable(oldName, 'type1', 'id1'); + workspace.createVariable(oldCase, 'type1', 'id2'); + createMockBlock(oldName); + createMockBlock(oldCase); + + workspace.renameVariable(oldName, newName); + + checkVariableValues(newName, 'type1', 'id2'); + var variable = workspace.getVariable(oldName); + var result_oldCase = workspace.getVariable(oldCase).name; + var block_var_name_1 = workspace.topBlocks_[0].getVars()[0]; + var block_var_name_2 = workspace.topBlocks_[1].getVars()[0]; + assertNull(variable); + assertNotEquals(oldCase, result_oldCase); + assertEquals(newName, block_var_name_1); + assertEquals(newName, block_var_name_2); + workspaceTest_tearDownWithMockBlocks(); +} + +// Extra testing not required for renameVariableById. It calls renameVariable +// and that has extensive testing. +function test_renameVariableById_TwoVariablesSameType() { + // Expect 'renameVariableById' to change oldName variable name to newName. + // Expect oldName block name to change to newName + workspaceTest_setUpWithMockBlocks() + var oldName = 'name1'; + var newName = 'name2'; + workspace.createVariable(oldName, 'type1', 'id1'); + workspace.createVariable(newName, 'type1', 'id2'); + createMockBlock(oldName); + createMockBlock(newName); + + workspace.renameVariableById('id1', newName); + checkVariableValues(newName, 'type1', 'id2'); + var variable = workspace.getVariable(oldName) + var block_var_name_1 = workspace.topBlocks_[0].getVars()[0]; + var block_var_name_2 = workspace.topBlocks_[1].getVars()[0]; + assertNull(variable); + assertEquals(newName, block_var_name_1); + assertEquals(newName, block_var_name_2); + workspaceTest_tearDownWithMockBlocks(); +} + +function test_deleteVariable_Trivial() { + workspaceTest_setUpWithMockBlocks() + workspace.createVariable('name1', 'type1', 'id1'); + workspace.createVariable('name2', 'type1', 'id2'); + createMockBlock('name1'); + createMockBlock('name2'); + + workspace.deleteVariable('name1'); + checkVariableValues('name2', 'type1', 'id2'); + var variable = workspace.getVariable('name1'); + var block_var_name = workspace.topBlocks_[0].getVars()[0]; + assertNull(variable); + assertEquals('name2', block_var_name); + workspaceTest_tearDownWithMockBlocks(); +} + +function test_deleteVariableById_Trivial() { + workspaceTest_setUpWithMockBlocks() + workspace.createVariable('name1', 'type1', 'id1'); + workspace.createVariable('name2', 'type1', 'id2'); + createMockBlock('name1'); + createMockBlock('name2'); + + workspace.deleteVariableById('id1'); + checkVariableValues('name2', 'type1', 'id2'); + var variable = workspace.getVariable('name1'); + var block_var_name = workspace.topBlocks_[0].getVars()[0]; + assertNull(variable); + assertEquals('name2', block_var_name); + workspaceTest_tearDownWithMockBlocks(); +} + +function test_getVariablesOfType_Trivial() { + workspaceTest_setUp(); + var var_1 = this.createAndGetVariable('name1', 'type1', 'id1'); + var var_2 = this.createAndGetVariable('name2', 'type1', 'id2'); + workspace.createVariable('name3', 'type2', 'id3'); + workspace.createVariable('name4', 'type3', 'id4'); + var result_array_1 = workspace.getVariablesOfType('type1'); + var result_array_2 = workspace.getVariablesOfType('type5'); + this.isEqualArrays([var_1, var_2], result_array_1); + this.isEqualArrays([], result_array_2); + workspaceTest_tearDown(); +} + +function test_getVariablesOfType_Null() { + workspaceTest_setUp(); + var var_1 = this.createAndGetVariable('name1', '', 'id1'); + var var_2 = this.createAndGetVariable('name2', '', 'id2'); + var var_3 = this.createAndGetVariable('name3', '', 'id3'); + workspace.createVariable('name4', 'type1', 'id4'); + var result_array = workspace.getVariablesOfType(null); + this.isEqualArrays([var_1, var_2, var_3], result_array); + workspaceTest_tearDown(); +} + +function test_getVariablesOfType_EmptyString() { + workspaceTest_setUp(); + var var_1 = this.createAndGetVariable('name1', null, 'id1'); + var var_2 = this.createAndGetVariable('name2', null, 'id2'); + var result_array = workspace.getVariablesOfType(''); + this.isEqualArrays([var_1, var_2], result_array); + workspaceTest_tearDown(); +} + +function test_getVariablesOfType_Deleted() { + workspaceTest_setUp(); + workspace.createVariable('name1', null, 'id1'); + workspace.deleteVariable('name1'); + var result_array = workspace.getVariablesOfType(''); + this.isEqualArrays([], result_array); + workspaceTest_tearDown(); +} + +function test_getVariablesOfType_DoesNotExist() { + workspaceTest_setUp(); + var result_array = workspace.getVariablesOfType('type1'); + this.isEqualArrays([], result_array); + workspaceTest_tearDown(); +} + +function test_getVariableTypes_Trivial() { + workspaceTest_setUp(); + workspace.createVariable('name1', 'type1', 'id1'); + workspace.createVariable('name2', 'type1', 'id2'); + workspace.createVariable('name3', 'type2', 'id3'); + workspace.createVariable('name4', 'type3', 'id4'); + var result_array = workspace.getVariableTypes(); + this.isEqualArrays(['type1', 'type2', 'type3'], result_array); + workspaceTest_tearDown(); +} + +function test_getVariableTypes_None() { + workspaceTest_setUp(); + var result_array = workspace.getVariableTypes(); + this.isEqualArrays([], result_array); + workspaceTest_tearDown(); +} + +function test_getAllVariables_Trivial() { + workspaceTest_setUp(); + var var_1 = this.createAndGetVariable('name1', 'type1', 'id1'); + var var_2 = this.createAndGetVariable('name2', 'type1', 'id2'); + var var_3 = this.createAndGetVariable('name3', 'type2', 'id3'); + var result_array = workspace.getAllVariables(); + this.isEqualArrays([var_1, var_2, var_3], result_array); + workspaceTest_tearDown(); +} + +function test_getAllVariables_None() { + workspaceTest_setUp(); + var result_array = workspace.getAllVariables(); + this.isEqualArrays([], result_array); + workspaceTest_tearDown(); +}