From 0afff23d49dcdd29db6a42f9b0e16afeb4b2c43d Mon Sep 17 00:00:00 2001 From: gregdyke Date: Wed, 25 May 2022 03:12:03 +0100 Subject: [PATCH] fix: json serialize lists_getIndex with json extraState (#6136) (#6170) * fix: json serialize lists_getIndex with json extraState (#6136) * address review comments * fix: move block tests to mocha/block folder --- blocks/lists.js | 33 +++++- core/serialization/blocks.js | 3 + tests/deps.js | 2 +- tests/deps.mocha.js | 7 +- tests/mocha/blocks/lists_test.js | 109 ++++++++++++++++++ .../mocha/{ => blocks}/logic_ternary_test.js | 0 tests/mocha/{ => blocks}/procedures_test.js | 0 tests/mocha/{ => blocks}/variables_test.js | 0 tests/mocha/index.html | 1 + 9 files changed, 147 insertions(+), 8 deletions(-) create mode 100644 tests/mocha/blocks/lists_test.js rename tests/mocha/{ => blocks}/logic_ternary_test.js (100%) rename tests/mocha/{ => blocks}/procedures_test.js (100%) rename tests/mocha/{ => blocks}/variables_test.js (100%) diff --git a/blocks/lists.js b/blocks/lists.js index dfff7b9f9..f320440df 100644 --- a/blocks/lists.js +++ b/blocks/lists.js @@ -13,6 +13,7 @@ goog.module('Blockly.libraryBlocks.lists'); const xmlUtils = goog.require('Blockly.utils.xml'); +const Xml = goog.require('Blockly.Xml'); const {Align} = goog.require('Blockly.Input'); /* eslint-disable-next-line no-unused-vars */ const {Block} = goog.requireType('Blockly.Block'); @@ -452,10 +453,34 @@ blocks['lists_getIndex'] = { this.updateAt_(isAt); }, - // This block does not need JSO serialization hooks (saveExtraState and - // loadExtraState) because the state of this object is already encoded in the - // dropdown values. - // XML hooks are kept for backwards compatibility. + /** + * Returns the state of this block as a JSON serializable object. + * Returns null for efficiency if no state is needed (not a statement) + * @return {?{isStatement: boolean}} The state of this block, ie whether it's + * a statement. + */ + saveExtraState: function() { + if (!this.outputConnection) { + return { + 'isStatement': true, + }; + } + return null; + }, + + /** + * Applies the given state to this block. + * @param {*} state The state to apply to this block, ie whether it's a + * statement. + */ + loadExtraState: function(state) { + if (state['isStatement']) { + this.updateStatement_(true); + } else if (typeof state === 'string') { + // backward compatible for json serialised mutations + this.domToMutation(Xml.textToDom(state)); + } + }, /** * Switch between a value block and a statement block. diff --git a/core/serialization/blocks.js b/core/serialization/blocks.js index 781dec01d..ba2b9fa84 100644 --- a/core/serialization/blocks.js +++ b/core/serialization/blocks.js @@ -441,6 +441,9 @@ const loadAttributes = function(block, state) { /** * Applies any extra state information available on the state object to the * block. + * For json serialised blocks with a loadExtraState method, if the extraState + * is an xml mutation (not an object), domToMutation will be called instead for + * backward compatibility. * @param {!Block} block The block to set the extra state of. * @param {!State} state The state object to reference. */ diff --git a/tests/deps.js b/tests/deps.js index 9d8d89a56..57e9c2cd4 100644 --- a/tests/deps.js +++ b/tests/deps.js @@ -1,6 +1,6 @@ goog.addDependency('../../blocks/blocks.js', ['Blockly.libraryBlocks'], ['Blockly.libraryBlocks.colour', 'Blockly.libraryBlocks.lists', 'Blockly.libraryBlocks.logic', 'Blockly.libraryBlocks.loops', 'Blockly.libraryBlocks.math', 'Blockly.libraryBlocks.procedures', 'Blockly.libraryBlocks.texts', 'Blockly.libraryBlocks.variables', 'Blockly.libraryBlocks.variablesDynamic'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../blocks/colour.js', ['Blockly.libraryBlocks.colour'], ['Blockly.FieldColour', 'Blockly.common'], {'lang': 'es6', 'module': 'goog'}); -goog.addDependency('../../blocks/lists.js', ['Blockly.libraryBlocks.lists'], ['Blockly.ConnectionType', 'Blockly.FieldDropdown', 'Blockly.FieldDropdown', 'Blockly.Input', 'Blockly.Msg', 'Blockly.Mutator', 'Blockly.common', 'Blockly.utils.xml'], {'lang': 'es6', 'module': 'goog'}); +goog.addDependency('../../blocks/lists.js', ['Blockly.libraryBlocks.lists'], ['Blockly.ConnectionType', 'Blockly.FieldDropdown', 'Blockly.FieldDropdown', 'Blockly.Input', 'Blockly.Msg', 'Blockly.Mutator', 'Blockly.Xml', 'Blockly.common', 'Blockly.utils.xml'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../blocks/logic.js', ['Blockly.libraryBlocks.logic'], ['Blockly.Events', 'Blockly.Extensions', 'Blockly.FieldDropdown', 'Blockly.FieldLabel', 'Blockly.Msg', 'Blockly.Mutator', 'Blockly.common', 'Blockly.utils.xml'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../blocks/loops.js', ['Blockly.libraryBlocks.loops'], ['Blockly.ContextMenu', 'Blockly.Events', 'Blockly.Extensions', 'Blockly.FieldDropdown', 'Blockly.FieldLabel', 'Blockly.FieldNumber', 'Blockly.FieldVariable', 'Blockly.Msg', 'Blockly.Variables', 'Blockly.Warning', 'Blockly.common', 'Blockly.utils.xml'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../blocks/math.js', ['Blockly.libraryBlocks.math'], ['Blockly.Extensions', 'Blockly.FieldDropdown', 'Blockly.FieldLabel', 'Blockly.FieldNumber', 'Blockly.FieldVariable', 'Blockly.common', 'Blockly.utils.xml'], {'lang': 'es6', 'module': 'goog'}); diff --git a/tests/deps.mocha.js b/tests/deps.mocha.js index ba58a1b31..c7e8c5c48 100644 --- a/tests/deps.mocha.js +++ b/tests/deps.mocha.js @@ -4,6 +4,10 @@ goog.addDependency('../../tests/mocha/block_change_event_test.js', ['Blockly.tes goog.addDependency('../../tests/mocha/block_create_event_test.js', ['Blockly.test.blockCreateEvent'], ['Blockly.Events.utils', 'Blockly.test.helpers.events', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../tests/mocha/block_json_test.js', ['Blockly.test.blockJson'], ['Blockly.Input'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../tests/mocha/block_test.js', ['Blockly.test.blocks'], ['Blockly.ConnectionType', 'Blockly.Events.utils', 'Blockly.blocks', 'Blockly.test.helpers.blockDefinitions', 'Blockly.test.helpers.setupTeardown', 'Blockly.test.helpers.warnings'], {'lang': 'es6', 'module': 'goog'}); +goog.addDependency('../../tests/mocha/blocks/lists_test.js', ['Blockly.test.lists'], ['Blockly.ConnectionType', 'Blockly.test.helpers.blockDefinitions', 'Blockly.test.helpers.serialization', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); +goog.addDependency('../../tests/mocha/blocks/logic_ternary_test.js', ['Blockly.test.logicTernary'], ['Blockly.Events.utils', 'Blockly.test.helpers.serialization', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); +goog.addDependency('../../tests/mocha/blocks/procedures_test.js', ['Blockly.test.procedures'], ['Blockly', 'Blockly.Msg', 'Blockly.test.helpers.procedures', 'Blockly.test.helpers.serialization', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); +goog.addDependency('../../tests/mocha/blocks/variables_test.js', ['Blockly.test.variables'], ['Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../tests/mocha/comment_deserialization_test.js', ['Blockly.test.commentDeserialization'], ['Blockly.test.helpers.setupTeardown', 'Blockly.test.helpers.userInput'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../tests/mocha/comment_test.js', ['Blockly.test.comments'], ['Blockly.Events.utils', 'Blockly.test.helpers.events', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../tests/mocha/connection_checker_test.js', ['Blockly.test.connectionChecker'], ['Blockly.ConnectionType', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); @@ -36,11 +40,9 @@ goog.addDependency('../../tests/mocha/jso_deserialization_test.js', ['Blockly.te goog.addDependency('../../tests/mocha/jso_serialization_test.js', ['Blockly.test.jsoSerialization'], ['Blockly.test.helpers.blockDefinitions', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es8', 'module': 'goog'}); goog.addDependency('../../tests/mocha/json_test.js', ['Blockly.test.json'], ['Blockly.test.helpers.setupTeardown', 'Blockly.test.helpers.warnings'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../tests/mocha/keydown_test.js', ['Blockly.test.keydown'], ['Blockly.test.helpers.blockDefinitions', 'Blockly.test.helpers.setupTeardown', 'Blockly.test.helpers.userInput'], {'lang': 'es6', 'module': 'goog'}); -goog.addDependency('../../tests/mocha/logic_ternary_test.js', ['Blockly.test.logicTernary'], ['Blockly.Events.utils', 'Blockly.test.helpers.serialization', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../tests/mocha/metrics_test.js', ['Blockly.test.metrics'], ['Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../tests/mocha/mutator_test.js', ['Blockly.test.mutator'], ['Blockly.test.helpers.blockDefinitions', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../tests/mocha/names_test.js', ['Blockly.test.names'], ['Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); -goog.addDependency('../../tests/mocha/procedures_test.js', ['Blockly.test.procedures'], ['Blockly', 'Blockly.Msg', 'Blockly.test.helpers.procedures', 'Blockly.test.helpers.serialization', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../tests/mocha/registry_test.js', ['Blockly.test.registry'], ['Blockly.test.helpers.setupTeardown', 'Blockly.test.helpers.warnings'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../tests/mocha/run_mocha_tests_in_browser.js', [], [], {'lang': 'es8'}); goog.addDependency('../../tests/mocha/serializer_test.js', ['Blockly.test.serialization'], ['Blockly.test.helpers.common', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); @@ -65,7 +67,6 @@ goog.addDependency('../../tests/mocha/trashcan_test.js', ['Blockly.test.trashcan goog.addDependency('../../tests/mocha/utils_test.js', ['Blockly.test.utils'], ['Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../tests/mocha/variable_map_test.js', ['Blockly.test.variableMap'], ['Blockly.test.helpers.setupTeardown', 'Blockly.test.helpers.variables'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../tests/mocha/variable_model_test.js', ['Blockly.test.variableModel'], ['Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); -goog.addDependency('../../tests/mocha/variables_test.js', ['Blockly.test.variables'], ['Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../tests/mocha/widget_div_test.js', ['Blockly.test.widgetDiv'], ['Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../tests/mocha/workspace_comment_test.js', ['Blockly.test.workspaceComment'], ['Blockly.WorkspaceComment', 'Blockly.test.helpers.setupTeardown'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../tests/mocha/workspace_svg_test.js', ['Blockly.test.workspaceSvg'], ['Blockly.Events.utils', 'Blockly.test.helpers.blockDefinitions', 'Blockly.test.helpers.events', 'Blockly.test.helpers.setupTeardown', 'Blockly.test.helpers.variables', 'Blockly.test.helpers.workspace'], {'lang': 'es6', 'module': 'goog'}); diff --git a/tests/mocha/blocks/lists_test.js b/tests/mocha/blocks/lists_test.js new file mode 100644 index 000000000..b7d516720 --- /dev/null +++ b/tests/mocha/blocks/lists_test.js @@ -0,0 +1,109 @@ +/** + * @license + * Copyright 2022 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +goog.module('Blockly.test.lists'); + +const {runSerializationTestSuite} = goog.require('Blockly.test.helpers.serialization'); +const {sharedTestSetup, sharedTestTeardown} = goog.require('Blockly.test.helpers.setupTeardown'); +const {ConnectionType} = goog.require('Blockly.ConnectionType'); +const {defineStatementBlock} = goog.require('Blockly.test.helpers.blockDefinitions'); + + +suite('Lists', function() { + setup(function() { + defineStatementBlock(); + sharedTestSetup.call(this); + this.workspace = new Blockly.Workspace(); + }); + + teardown(function() { + sharedTestTeardown.call(this); + }); + + suite('ListsGetIndex', function() { + /** + * Test cases for serialization tests. + * @type {Array} + */ + const testCases = [ + { + title: 'JSON not requiring mutations', + json: { + type: 'lists_getIndex', + id: '1', + fields: {MODE: 'GET', WHERE: 'FIRST'}, + }, + assertBlockStructure: (block) => { + chai.assert.equal(block.type, 'lists_getIndex'); + chai.assert.exists(block.outputConnection); + }, + }, + { + title: 'JSON requiring mutations', + json: { + type: 'lists_getIndex', + id: '1', + extraState: {isStatement: true}, + fields: {MODE: 'REMOVE', WHERE: 'FROM_START'}, + }, + assertBlockStructure: (block) => { + chai.assert.equal(block.type, 'lists_getIndex'); + chai.assert.isNotTrue(block.outputConnection); + chai.assert.isTrue( + block.getInput('AT').type === ConnectionType.INPUT_VALUE + ); + }, + }, + { + title: + 'JSON requiring mutations and extra state for previous connection', + json: { + type: 'statement_block', + id: '1', + next: { + block: { + type: 'lists_getIndex', + id: '2', + extraState: {isStatement: true}, + fields: {MODE: 'REMOVE', WHERE: 'FROM_START'}, + }, + }, + }, + assertBlockStructure: (block) => {}, + }, + { + title: + 'JSON requiring mutations with XML extra state', + json: { + type: 'statement_block', + id: '1', + next: { + block: { + type: 'lists_getIndex', + id: '2', + extraState: '', + fields: {MODE: 'REMOVE', WHERE: 'FROM_START'}, + }, + }, + }, + expectedJson: { + type: 'statement_block', + id: '1', + next: { + block: { + type: 'lists_getIndex', + id: '2', + extraState: {isStatement: true}, + fields: {MODE: 'REMOVE', WHERE: 'FROM_START'}, + }, + }, + }, + assertBlockStructure: (block) => {}, + }, + ]; + runSerializationTestSuite(testCases); + }); +}); diff --git a/tests/mocha/logic_ternary_test.js b/tests/mocha/blocks/logic_ternary_test.js similarity index 100% rename from tests/mocha/logic_ternary_test.js rename to tests/mocha/blocks/logic_ternary_test.js diff --git a/tests/mocha/procedures_test.js b/tests/mocha/blocks/procedures_test.js similarity index 100% rename from tests/mocha/procedures_test.js rename to tests/mocha/blocks/procedures_test.js diff --git a/tests/mocha/variables_test.js b/tests/mocha/blocks/variables_test.js similarity index 100% rename from tests/mocha/variables_test.js rename to tests/mocha/blocks/variables_test.js diff --git a/tests/mocha/index.html b/tests/mocha/index.html index 27edd9d6a..ab1bc428f 100644 --- a/tests/mocha/index.html +++ b/tests/mocha/index.html @@ -91,6 +91,7 @@ goog.require('Blockly.test.jsoSerialization'); goog.require('Blockly.test.json'); goog.require('Blockly.test.keydown'); + goog.require('Blockly.test.lists'); goog.require('Blockly.test.logicTernary'); goog.require('Blockly.test.metrics'); goog.require('Blockly.test.mutator');