From dee3951541cb099777be16df5243b6f5e27c4b9b Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 5 Jan 2023 23:19:17 +0000 Subject: [PATCH] feat: procedure defs deserialize models (#6706) * feat: add procedure defs deserializing models * chore: rename model to model_ * chore: cleanup * chore: fix tests failures * fix: PR comments --- blocks/procedures.js | 69 ++++- core/procedures/observable_parameter_model.ts | 4 +- tests/mocha/blocks/procedures_test.js | 250 ++++++++++++++++++ 3 files changed, 309 insertions(+), 14 deletions(-) diff --git a/blocks/procedures.js b/blocks/procedures.js index 5bc876b7f..565bdf058 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -92,6 +92,7 @@ const blocks = createBlockDefinitionsFromJsonArray([ 'procedure_def_validator_helper', 'procedure_defnoreturn_get_caller_block_mixin', 'procedure_defnoreturn_set_comment_helper', + 'procedure_def_set_no_return_helper', ], 'mutator': 'procedure_def_mutator', }, @@ -168,6 +169,7 @@ const blocks = createBlockDefinitionsFromJsonArray([ 'procedure_def_validator_helper', 'procedure_defreturn_get_caller_block_mixin', 'procedure_defreturn_set_comment_helper', + 'procedure_def_set_return_helper', ], 'mutator': 'procedure_def_mutator', }, @@ -279,10 +281,15 @@ exports.blocks = blocks; /** @this {Block} */ const procedureDefGetDefMixin = function() { const mixin = { - model: null, + model_: null, + /** + * Returns the data model for this procedure block. + * @return {!IProcedureModel} The data model for this procedure + * block. + */ getProcedureModel() { - return this.model; + return this.model_; }, /** @@ -312,9 +319,9 @@ const procedureDefGetDefMixin = function() { }, }; - mixin.model = + mixin.model_ = new ObservableProcedureModel(this.workspace, this.getFieldValue('NAME')); - this.workspace.getProcedureMap().add(mixin.model); + this.workspace.getProcedureMap().add(mixin.getProcedureModel()); this.mixin(mixin, true); }; @@ -395,8 +402,8 @@ const procedureDefUpdateShapeMixin = { * Updates the block to reflect the state of the procedure model. */ doProcedureUpdate: function() { - this.setFieldValue(this.model.getName(), 'NAME'); - this.setEnabled(this.model.getEnabled()); + this.setFieldValue(this.getProcedureModel().getName(), 'NAME'); + this.setEnabled(this.getProcedureModel().getEnabled()); this.updateParameters_(); }, @@ -405,7 +412,8 @@ const procedureDefUpdateShapeMixin = { * model. */ updateParameters_: function() { - const params = this.model.getParameters().map((p) => p.getName()); + const params = + this.getProcedureModel().getParameters().map((p) => p.getName()); const paramString = params.length ? `${Msg['PROCEDURES_BEFORE_PARAMS']} ${params.join(', ')}` : ''; @@ -551,6 +559,17 @@ const procedureDefMutator = { * @this {Block} */ domToMutation: function(xmlElement) { + for (let i = 0; i < xmlElement.childNodes.length; i++) { + const node = xmlElement.childNodes[i]; + if (node.nodeName.toLowerCase() !== 'arg') continue; + this.getProcedureModel().insertParameter( + new ObservableParameterModel( + this.workspace, node.getAttribute('name'), + node.getAttribute('varid')), + i); + } + + // TODO: Remove this data update code. this.arguments_ = []; this.argumentVarModels_ = []; for (let i = 0, childNode; (childNode = xmlElement.childNodes[i]); i++) { @@ -610,6 +629,16 @@ const procedureDefMutator = { * statements. */ loadExtraState: function(state) { + if (state['params']) { + for (let i = 0; i < state['params'].length; i++) { + const param = state['params'][i]; + this.getProcedureModel().insertParameter( + new ObservableParameterModel(this.workspace, param.name, param.id), + i); + } + } + + // TODO: Remove this data update code. this.arguments_ = []; this.argumentVarModels_ = []; if (state['params']) { @@ -694,14 +723,16 @@ const procedureDefMutator = { } this.updateParams_(); Procedures.mutateCallers(this); - for (let i = this.model.getParameters().length; i >= 0; i--) { - this.model.deleteParameter(i); + + const model = this.getProcedureModel(); + for (let i = model.getParameters().length; i >= 0; i--) { + model.deleteParameter(i); } let i = 0; paramBlock = containerBlock.getInputTargetBlock('STACK'); while (paramBlock && !paramBlock.isInsertionMarker()) { - this.model.insertParameter( + model.insertParameter( new ObservableParameterModel( this.workspace, paramBlock.getFieldValue('NAME'), paramBlock.id), i); @@ -771,8 +802,8 @@ Extensions.registerMixin( const procedureDefOnChangeMixin = { onchange: function(e) { if (e.type === Events.BLOCK_CHANGE && e.blockId === this.id && - e.element == 'disabled') { - this.model.setEnabled(!e.newValue); + e.element === 'disabled') { + this.getProcedureModel().setEnabled(!e.newValue); } }, }; @@ -843,6 +874,20 @@ Extensions.registerMixin( 'procedure_defreturn_get_caller_block_mixin', procedureDefReturnGetCallerBlockMixin); +/** @this {Block} */ +const procedureDefSetNoReturnHelper = function() { + this.getProcedureModel().setReturnTypes(null); +}; +Extensions.register( + 'procedure_def_set_no_return_helper', procedureDefSetNoReturnHelper); + +/** @this {Block} */ +const procedureDefSetReturnHelper = function() { + this.getProcedureModel().setReturnTypes([]); +}; +Extensions.register( + 'procedure_def_set_return_helper', procedureDefSetReturnHelper); + const validateProcedureParamMixin = { /** * Obtain a valid name for the procedure argument. Create a variable if diff --git a/core/procedures/observable_parameter_model.ts b/core/procedures/observable_parameter_model.ts index 8b0fd5309..91f624f95 100644 --- a/core/procedures/observable_parameter_model.ts +++ b/core/procedures/observable_parameter_model.ts @@ -24,8 +24,8 @@ export class ObservableParameterModel implements IParameterModel { constructor( private readonly workspace: Workspace, name: string, id?: string) { this.id = id ?? genUid(); - this.variable = - this.workspace.getVariable(name) ?? workspace.createVariable(name); + this.variable = this.workspace.getVariable(name) ?? + workspace.createVariable(name, '', id); } /** diff --git a/tests/mocha/blocks/procedures_test.js b/tests/mocha/blocks/procedures_test.js index a967e7e24..ae137689a 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -452,6 +452,256 @@ suite('Procedures', function() { }); }); + suite('deserializing data models', function() { + suite('return types', function() { + test('procedure defs without returns have null return types', function() { + const json = { + 'blocks': { + 'languageVersion': 0, + 'blocks': [ + { + 'type': 'procedures_defnoreturn', + 'fields': { + 'NAME': 'test name', + }, + }, + ], + }, + }; + Blockly.serialization.workspaces.load(json, this.workspace); + const procedureModel = + this.workspace.getProcedureMap().getProcedures()[0]; + + chai.assert.isNull( + procedureModel.getReturnTypes(), + 'Expected the return types to be null'); + }); + + test('procedure defs with returns have array return types', function() { + const json = { + 'blocks': { + 'languageVersion': 0, + 'blocks': [ + { + 'type': 'procedures_defreturn', + 'fields': { + 'NAME': 'test name', + }, + }, + ], + }, + }; + Blockly.serialization.workspaces.load(json, this.workspace); + const procedureModel = + this.workspace.getProcedureMap().getProcedures()[0]; + + chai.assert.isArray( + procedureModel.getReturnTypes(), + 'Expected the return types to be an array'); + }); + }); + + suite('json', function() { + test('procedure names get deserialized', function() { + const json = { + 'blocks': { + 'languageVersion': 0, + 'blocks': [ + { + 'type': 'procedures_defnoreturn', + 'fields': { + 'NAME': 'test name', + }, + }, + ], + }, + }; + Blockly.serialization.workspaces.load(json, this.workspace); + const procedureModel = + this.workspace.getProcedureMap().getProcedures()[0]; + + chai.assert.equal( + procedureModel.name, + 'test name', + 'Expected the name of the procedure model to equal the name ' + + 'being deserialized.'); + }); + + test('procedure parameter names get deserialized', function() { + const json = { + 'blocks': { + 'languageVersion': 0, + 'blocks': [ + { + 'type': 'procedures_defnoreturn', + 'fields': { + 'NAME': 'test name', + }, + 'extraState': { + 'params': [ + { + 'id': 'test id 1', + 'name': 'test name 1', + }, + { + 'id': 'test id 2', + 'name': 'test name 2', + }, + ], + }, + }, + ], + }, + }; + Blockly.serialization.workspaces.load(json, this.workspace); + const procedureModel = + this.workspace.getProcedureMap().getProcedures()[0]; + + chai.assert.equal( + procedureModel.getParameter(0).getName(), + 'test name 1', + 'Expected the name of the first parameter to equal the name ' + + 'being deserialized.'); + chai.assert.equal( + procedureModel.getParameter(1).getName(), + 'test name 2', + 'Expected the name of the second parameter to equal the name ' + + 'being deserialized.'); + }); + + test('procedure variables get matching IDs', function() { + const json = { + 'blocks': { + 'languageVersion': 0, + 'blocks': [ + { + 'type': 'procedures_defnoreturn', + 'extraState': { + 'params': [ + { + 'name': 'test param name', + 'id': 'test param id', + }, + ], + }, + 'fields': { + 'NAME': 'test proc name', + }, + }, + ], + }, + 'variables': [ + { + 'name': 'test param name', + 'id': 'test param id', + }, + ], + }; + Blockly.serialization.workspaces.load(json, this.workspace); + const procedureModel = + this.workspace.getProcedureMap().getProcedures()[0]; + + chai.assert.equal( + procedureModel.getParameter(0).getVariableModel().getId(), + 'test param id', + 'Expected the variable id to match the serialized param id'); + }); + }); + + suite('xml', function() { + test('procedure names get deserialized', function() { + const xml = Blockly.Xml.textToDom( + `` + + ` test name` + + ``); + Blockly.Xml.domToBlock(xml, this.workspace); + const procedureModel = + this.workspace.getProcedureMap().getProcedures()[0]; + + chai.assert.equal( + procedureModel.name, + 'test name', + 'Expected the name of the procedure model to equal the name ' + + 'being deserialized.'); + }); + + test('procedure parameter names get deserialized', function() { + const xml = Blockly.Xml.textToDom( + `` + + ` ` + + ` ` + + ` ` + + ` ` + + ` test name` + + ``); + Blockly.Xml.domToBlock(xml, this.workspace); + const procedureModel = + this.workspace.getProcedureMap().getProcedures()[0]; + + chai.assert.equal( + procedureModel.getParameter(0).getName(), + 'test name 1', + 'Expected the name of the first parameter to equal the name ' + + 'being deserialized.'); + chai.assert.equal( + procedureModel.getParameter(1).getName(), + 'test name 2', + 'Expected the name of the second parameter to equal the name ' + + 'being deserialized.'); + }); + + test('procedure variables get matching IDs', function() { + const json = { + 'blocks': { + 'languageVersion': 0, + 'blocks': [ + { + 'type': 'procedures_defnoreturn', + 'extraState': { + 'params': [ + { + 'name': 'test param name', + 'id': 'test param id', + }, + ], + }, + 'fields': { + 'NAME': 'test proc name', + }, + }, + ], + }, + 'variables': [ + { + 'name': 'test param name', + 'id': 'test param id', + }, + ], + }; + const xml = Blockly.Xml.textToDom( + `` + + ` ` + + ` test param name` + + ` ` + + ` ` + + ` ` + + ` ` + + ` ` + + ` test name` + + ` ` + + ``); + Blockly.Xml.domToWorkspace(xml, this.workspace); + const procedureModel = + this.workspace.getProcedureMap().getProcedures()[0]; + + chai.assert.equal( + procedureModel.getParameter(0).getVariableModel().getId(), + 'test param id', + 'Expected the variable id to match the serialized param id'); + }); + }); + }); + suite('Renaming procedures', function() { test('callers are updated to have the new name', function() { const defBlock = createProcDefBlock(this.workspace);