From dc06872a3719bfcbe3fb66e4d6dcbc7ed47138f6 Mon Sep 17 00:00:00 2001 From: marisaleung Date: Wed, 24 May 2017 10:51:54 -0700 Subject: [PATCH] Deserialization variables at top. --- core/xml.js | 49 ++++++++++- tests/jsunit/xml_test.js | 174 ++++++++++++++++++++++++++++----------- 2 files changed, 175 insertions(+), 48 deletions(-) diff --git a/core/xml.js b/core/xml.js index da6595aa6..4e9e4b989 100644 --- a/core/xml.js +++ b/core/xml.js @@ -349,6 +349,14 @@ Blockly.Xml.domToWorkspace = function(xml, workspace) { } } else if (name == 'shadow') { goog.asserts.fail('Shadow block cannot be a top-level block.'); + } else if (name == 'variables') { + if (i == 1) { + Blockly.Xml.domToVariables(xmlChild, workspace); + } + else { + throw Error('\'variables\' tag must be the first element in the' + + 'workspace XML, but it was found in another location.'); + } } } if (!existingGroup) { @@ -470,6 +478,25 @@ Blockly.Xml.domToBlock = function(xmlBlock, workspace) { return topBlock; }; +/** + * Decode an XML list of variables and add the variables to the workspace. + * @param {!Element} xmlVariables List of XML variable elements. + * @param {!Blockly.Workspace} workspace The workspace to which the variable + * should be added. + */ +Blockly.Xml.domToVariables = function(xmlVariables, workspace) { + for (var i = 0, xmlChild; xmlChild = xmlVariables.children[i]; i++) { + var type = xmlChild.getAttribute('type'); + var id = xmlChild.getAttribute('id'); + var name = xmlChild.textContent; + + if (typeof(type) === undefined || type === null) { + throw Error('Variable with id, ' + id + ' is without a type'); + } + workspace.createVariable(name, type, id); + } +}; + /** * Decode an XML block tag and create a block (and possibly sub blocks) on the * workspace. @@ -551,12 +578,32 @@ Blockly.Xml.domToBlockHeadless_ = function(xmlBlock, workspace) { // Fall through. case 'field': var field = block.getField(name); + var text = xmlChild.textContent; + if (field instanceof Blockly.FieldVariable) { + // TODO (marisaleung): When we change setValue and getValue to + // interact with id's instead of names, update this so that we get + // the variable based on id instead of textContent. + var type = xmlChild.getAttribute('variabletype') || ''; + var variable = workspace.getVariable(text); + if (!variable) { + variable = workspace.createVariable(text, type, + xmlChild.getAttribute(id)); + } + if (typeof(type) !== undefined && type !== null) { + if (type !== variable.type) { + throw Error('Serialized variable type with id \'' + + variable.getId() + '\' had type ' + variable.type + ', and ' + + 'does not match variable field that references it: ' + + Blockly.Xml.domToText(xmlChild) + '.'); + } + } + } if (!field) { console.warn('Ignoring non-existent field ' + name + ' in block ' + prototypeName); break; } - field.setValue(xmlChild.textContent); + field.setValue(text); break; case 'value': case 'statement': diff --git a/tests/jsunit/xml_test.js b/tests/jsunit/xml_test.js index a118413ba..c7fc2b379 100644 --- a/tests/jsunit/xml_test.js +++ b/tests/jsunit/xml_test.js @@ -59,6 +59,17 @@ function xmlTest_setUp() { function xmlTest_setUpWithMockBlocks() { xmlTest_setUp(); + Blockly.defineBlocksWithJsonArray([{ + 'type': 'field_variable_test_block', + 'message0': '%1', + 'args0': [ + { + 'type': 'field_variable', + 'name': 'VAR', + 'variable': 'item' + } + ], + }]); // 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 @@ -118,6 +129,20 @@ function xmlTest_checkVariableDomValues(variableDom, type, id, text) { assertEquals(text, variableDom.textContent); } +/** + * 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 xmlTest_checkVariableValues(name, type, id) { + var variable = workspace.getVariable(name); + assertNotUndefined(variable); + assertEquals(name, variable.name); + assertEquals(type, variable.type); + assertEquals(id, variable.getId()); +} + function test_textToDom() { var dom = Blockly.Xml.textToDom(XML_TEXT); assertEquals('XML tag', 'xml', dom.nodeName); @@ -131,31 +156,119 @@ function test_domToText() { text.replace(/\s+/g, '')); } -function test_domToWorkspace() { - Blockly.Blocks.test_block = { - init: function() { - this.jsonInit({ - message0: 'test', - }); - } - }; - - workspace = new Blockly.Workspace(); +function test_domToWorkspace_BackwardCompatibility() { + // Expect that workspace still loads without serialized variables. + xmlTest_setUpWithMockBlocks(); + var mockGenUid = mockControl_.createMethodMock(Blockly.utils, 'genUid'); + mockGenUid().$returns('1'); + mockGenUid().$returns('1'); + mockGenUid().$replay(); try { var dom = Blockly.Xml.textToDom( - '' + - ' ' + + '' + + ' ' + + ' name1' + ' ' + ''); Blockly.Xml.domToWorkspace(dom, workspace); assertEquals('Block count', 1, workspace.getAllBlocks().length); + xmlTest_checkVariableValues('name1', '', '1'); } finally { - delete Blockly.Blocks.test_block; + xmlTest_tearDownWithMockBlocks(); + } +} +function test_domToWorkspace_VariablesAtTop() { + // Expect that unused variables are preserved. + xmlTest_setUpWithMockBlocks(); + try { + var dom = Blockly.Xml.textToDom( + '' + + ' ' + + ' name1' + + ' name2' + + ' name3' + + ' ' + + ' ' + + ' name3' + + ' ' + + ''); + Blockly.Xml.domToWorkspace(dom, workspace); + assertEquals('Block count', 1, workspace.getAllBlocks().length); + xmlTest_checkVariableValues('name1', 'type1', 'id1'); + xmlTest_checkVariableValues('name2', 'type2', 'id2'); + xmlTest_checkVariableValues('name3', '', 'id3'); + } finally { + xmlTest_tearDownWithMockBlocks(); + } +} + +function test_domToWorkspace_VariablesAtTop_DuplicateVariablesTag() { + // Expect thrown Error because of duplicate 'variables' tag + xmlTest_setUpWithMockBlocks(); + try { + var dom = Blockly.Xml.textToDom( + '' + + ' ' + + ' ' + + ' ' + + ' ' + + ''); + Blockly.Xml.domToWorkspace(dom, workspace); + fail(); + } + catch (e) { + // expected + } finally { + xmlTest_tearDownWithMockBlocks(); + } +} + +function test_domToWorkspace_VariablesAtTop_MissingType() { + // Expect thrown error when a variable tag is missing the type attribute. + workspace = new Blockly.Workspace(); + try { + var dom = Blockly.Xml.textToDom( + '' + + ' ' + + ' name1' + + ' ' + + ' ' + + ' name3' + + ' ' + + ''); + Blockly.Xml.domToWorkspace(dom, workspace); + fail(); + } catch (e) { + // expected + } finally { workspace.dispose(); } } +function test_domToWorkspace_VariablesAtTop_MismatchBlockType() { + // Expect thrown error when the serialized type of a variable does not match + // the type of a variable field that references it. + xmlTest_setUpWithMockBlocks(); + try { + var dom = Blockly.Xml.textToDom( + '' + + ' ' + + ' name1' + + ' ' + + ' ' + + ' name1' + + ' ' + + ''); + Blockly.Xml.domToWorkspace(dom, workspace); + fail(); + } catch (e) { + // expected + } finally { + xmlTest_tearDownWithMockBlocks(); + } +} + function test_domToPrettyText() { var dom = Blockly.Xml.textToDom(XML_TEXT); var text = Blockly.Xml.domToPrettyText(dom); @@ -195,17 +308,6 @@ function test_appendDomToWorkspace() { } function test_blockToDom_fieldToDom_trivial() { - Blockly.defineBlocksWithJsonArray([{ - 'type': 'field_variable_test_block', - 'message0': '%1', - 'args0': [ - { - 'type': 'field_variable', - 'name': 'VAR', - 'variable': 'item' - } - ], - }]); xmlTest_setUpWithMockBlocks() workspace.createVariable('name1', 'type1', 'id1'); var block = new Blockly.Block(workspace, 'field_variable_test_block'); @@ -216,17 +318,6 @@ function test_blockToDom_fieldToDom_trivial() { } function test_blockToDom_fieldToDom_defaultCase() { - Blockly.defineBlocksWithJsonArray([{ - 'type': 'field_variable_test_block', - 'message0': '%1', - 'args0': [ - { - 'type': 'field_variable', - 'name': 'VAR', - 'variable': 'item' - } - ], - }]); xmlTest_setUpWithMockBlocks() var mockGenUid = mockControl_.createMethodMock(Blockly.utils, 'genUid'); mockGenUid().$returns('1'); @@ -277,17 +368,6 @@ function test_variablesToDom_oneVariable() { } function test_variablesToDom_twoVariables_oneBlock() { - Blockly.defineBlocksWithJsonArray([{ - 'type': 'field_variable_test_block', - 'message0': '%1', - 'args0': [ - { - 'type': 'field_variable', - 'name': 'VAR', - 'variable': 'item' - } - ], - }]); xmlTest_setUpWithMockBlocks(); workspace.createVariable('name1', 'type1', 'id1'); @@ -308,6 +388,6 @@ function test_variablesToDom_noVariables() { xmlTest_setUp(); workspace.createVariable('name1'); var resultDom = Blockly.Xml.variablesToDom(workspace.getAllVariables()); - assertEquals(0, resultDom.children.length); + assertEquals(1, resultDom.children.length); xmlTest_tearDown(); }