From 0ad0adfb75ce5a2d183f86a9636b9a85ddd3be64 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Mon, 8 Jan 2024 14:00:12 -0800 Subject: [PATCH] feat!: add serialization hooks to procedure models (#7740) * feat!: add serialization hooks to procedure models * chore: fix tests * chore: remove internal functions * fix: add state interfaces back --- core/interfaces/i_parameter_model.ts | 8 + core/interfaces/i_procedure_model.ts | 8 + core/serialization/procedures.ts | 103 +++---- tests/mocha/jso_deserialization_test.js | 347 +++--------------------- tests/mocha/jso_serialization_test.js | 123 ++------- tests/mocha/test_helpers/procedures.js | 16 ++ 6 files changed, 136 insertions(+), 469 deletions(-) diff --git a/core/interfaces/i_parameter_model.ts b/core/interfaces/i_parameter_model.ts index 1b0239703..6b351b6b3 100644 --- a/core/interfaces/i_parameter_model.ts +++ b/core/interfaces/i_parameter_model.ts @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import {ParameterState} from '../serialization/procedures'; import {IProcedureModel} from './i_procedure_model'; /** @@ -40,4 +41,11 @@ export interface IParameterModel { /** Sets the procedure model this parameter is associated with. */ setProcedureModel(model: IProcedureModel): this; + + /** + * Serializes the state of the parameter to JSON. + * + * @returns JSON serializable state of the parameter. + */ + saveState(): ParameterState; } diff --git a/core/interfaces/i_procedure_model.ts b/core/interfaces/i_procedure_model.ts index cb5fda09f..61026adae 100644 --- a/core/interfaces/i_procedure_model.ts +++ b/core/interfaces/i_procedure_model.ts @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import {State} from '../serialization/procedures.js'; import {IParameterModel} from './i_parameter_model.js'; /** @@ -60,4 +61,11 @@ export interface IProcedureModel { * disabled, all procedure caller blocks should be disabled as well. */ getEnabled(): boolean; + + /** + * Serializes the state of the procedure to JSON. + * + * @returns JSON serializable state of the procedure. + */ + saveState(): State; } diff --git a/core/serialization/procedures.ts b/core/serialization/procedures.ts index 34d381171..688904b08 100644 --- a/core/serialization/procedures.ts +++ b/core/serialization/procedures.ts @@ -10,89 +10,77 @@ import type {ISerializer} from '../interfaces/i_serializer.js'; import * as priorities from './priorities.js'; import type {Workspace} from '../workspace.js'; -/** - * Representation of a procedure data model. - */ +/** Represents the state of a procedure model. */ export interface State { - // TODO: This should also handle enabled. id: string; name: string; returnTypes: string[] | null; parameters?: ParameterState[]; + [key: string]: unknown; } -/** - * Representation of a parameter data model. - */ +/** Represents the state of a parameter model. */ export interface ParameterState { id: string; name: string; types?: string[]; + [key: string]: unknown; } /** * A newable signature for an IProcedureModel. * * Refer to - * https://www.typescriptlang.org/docs/handbook/2/generics.html#using-class-types-in-generics + * https://www.typescriptlang.org/docs/handbook/interfaces.html#difference-between-the-static-and-instance-sides-of-classes * for what is going on with this. */ -type ProcedureModelConstructor = new ( - workspace: Workspace, - name: string, - id: string, -) => ProcedureModel; +interface ProcedureModelConstructor { + new (workspace: Workspace, name: string, id: string): ProcedureModel; + + /** + * Deserializes the JSON state and returns a procedure model. + * + * @param state The state to deserialize. + * @param workspace The workspace to load the procedure model into. + * @returns The constructed procedure model. + */ + loadState(state: Object, workspace: Workspace): ProcedureModel; +} /** * A newable signature for an IParameterModel. * * Refer to - * https://www.typescriptlang.org/docs/handbook/2/generics.html#using-class-types-in-generics + * https://www.typescriptlang.org/docs/handbook/interfaces.html#difference-between-the-static-and-instance-sides-of-classes * for what is going on with this. */ -type ParameterModelConstructor = new ( - workspace: Workspace, - name: string, - id: string, -) => ParameterModel; +interface ParameterModelConstructor { + new (workspace: Workspace, name: string, id: string): ParameterModel; -/** - * Serializes the given IProcedureModel to JSON. - * - * @internal - */ -export function saveProcedure(proc: IProcedureModel): State { - const state: State = { - id: proc.getId(), - name: proc.getName(), - returnTypes: proc.getReturnTypes(), - }; - if (!proc.getParameters().length) return state; - state.parameters = proc.getParameters().map((param) => saveParameter(param)); - return state; + /** + * Deserializes the JSON state and returns a parameter model. + * + * @param state The state to deserialize. + * @param workspace The workspace to load the parameter model into. + * @returns The constructed parameter model. + */ + loadState(state: Object, workspace: Workspace): ParameterModel; } /** - * Serializes the given IParameterModel to JSON. - * - * @internal + * Serializes the given IProcedureModel to JSON. */ -export function saveParameter(param: IParameterModel): ParameterState { - const state: ParameterState = { - id: param.getId(), - name: param.getName(), - }; - if (!param.getTypes().length) return state; - state.types = param.getTypes(); +function saveProcedure(proc: IProcedureModel): State { + const state: State = proc.saveState(); + if (!proc.getParameters().length) return state; + state.parameters = proc.getParameters().map((param) => param.saveState()); return state; } /** * Deserializes the given procedure model State from JSON. - * - * @internal */ -export function loadProcedure< +function loadProcedure< ProcedureModel extends IProcedureModel, ParameterModel extends IParameterModel, >( @@ -101,36 +89,17 @@ export function loadProcedure< state: State, workspace: Workspace, ): ProcedureModel { - const proc = new procedureModelClass( - workspace, - state.name, - state.id, - ).setReturnTypes(state.returnTypes); + const proc = procedureModelClass.loadState(state, workspace); if (!state.parameters) return proc; for (const [index, param] of state.parameters.entries()) { proc.insertParameter( - loadParameter(parameterModelClass, param, workspace), + parameterModelClass.loadState(param, workspace), index, ); } return proc; } -/** - * Deserializes the given ParameterState from JSON. - * - * @internal - */ -export function loadParameter( - parameterModelClass: ParameterModelConstructor, - state: ParameterState, - workspace: Workspace, -): ParameterModel { - const model = new parameterModelClass(workspace, state.name, state.id); - if (state.types) model.setTypes(state.types); - return model; -} - /** Serializer for saving and loading procedure state. */ export class ProcedureSerializer< ProcedureModel extends IProcedureModel, diff --git a/tests/mocha/jso_deserialization_test.js b/tests/mocha/jso_deserialization_test.js index d58e208fb..7c8a06db8 100644 --- a/tests/mocha/jso_deserialization_test.js +++ b/tests/mocha/jso_deserialization_test.js @@ -11,14 +11,20 @@ import { } from './test_helpers/setup_teardown.js'; import {assertEventFired} from './test_helpers/events.js'; import * as eventUtils from '../../build/src/core/events/utils.js'; +import { + MockParameterModel, + MockProcedureModel, +} from './test_helpers/procedures.js'; suite('JSO Deserialization', function () { setup(function () { sharedTestSetup.call(this); + this.sandbox = sinon.createSandbox(); this.workspace = new Blockly.Workspace(); }); teardown(function () { + this.sandbox.restore(); sharedTestTeardown.call(this); }); @@ -785,95 +791,6 @@ suite('JSO Deserialization', function () { }); suite('Procedures', function () { - class MockProcedureModel { - constructor(workspace, name, id) { - this.id = id ?? Blockly.utils.idGenerator.genUid(); - this.name = name; - this.parameters = []; - this.returnTypes = null; - this.enabled = true; - } - - setName(name) { - this.name = name; - return this; - } - - insertParameter(parameterModel, index) { - this.parameters.splice(index, 0, parameterModel); - return this; - } - - deleteParameter(index) { - this.parameters.splice(index, 1); - return this; - } - - setReturnTypes(types) { - this.returnTypes = types; - return this; - } - - setEnabled(enabled) { - this.enabled = enabled; - return this; - } - - getId() { - return this.id; - } - - getName() { - return this.name; - } - - getParameter(index) { - return this.parameters[index]; - } - - getParameters() { - return [...this.parameters]; - } - - getReturnTypes() { - return this.returnTypes; - } - - getEnabled() { - return this.enabled; - } - } - - class MockParameterModel { - constructor(workspace, name, id) { - this.id = id ?? Blockly.utils.idGenerator.genUid(); - this.name = name; - this.types = []; - } - - setName(name) { - this.name = name; - return this; - } - - setTypes(types) { - this.types = types; - return this; - } - - getName() { - return this.name; - } - - getTypes() { - return this.types; - } - - getId() { - return this.id; - } - } - setup(function () { this.procedureSerializer = new Blockly.serialization.procedures.ProcedureSerializer( @@ -888,232 +805,46 @@ suite('JSO Deserialization', function () { this.procedureMap = null; }); - suite('invariant properties', function () { - test('the id property is assigned', function () { - const jso = { - 'id': 'test id', - 'name': 'test name', - 'returnTypes': [], - }; + test('load is called for the procedure model', function () { + const state = [ + { + 'id': 'test', + 'parameters': [], + }, + ]; + const spy = this.sandbox.spy(MockProcedureModel, 'loadState'); - this.procedureSerializer.load([jso], this.workspace); + this.procedureSerializer.load(state, this.workspace); - const procedureModel = this.procedureMap.getProcedures()[0]; - chai.assert.isNotNull( - procedureModel, - 'Expected a procedure model to exist', - ); - chai.assert.equal( - procedureModel.getId(), - 'test id', - 'Expected the procedure model ID to match the serialized ID', - ); - }); - - test('the name property is assigned', function () { - const jso = { - 'id': 'test id', - 'name': 'test name', - 'returnTypes': [], - }; - - this.procedureSerializer.load([jso], this.workspace); - - const procedureModel = this.procedureMap.getProcedures()[0]; - chai.assert.isNotNull( - procedureModel, - 'Expected a procedure model to exist', - ); - chai.assert.equal( - procedureModel.getName(), - 'test name', - 'Expected the procedure model name to match the serialized name', - ); - }); + chai.assert.isTrue( + spy.calledOnce, + 'Expected the loadState method to be called', + ); }); - suite('return types', function () { - test('if the return type property is null it is assigned', function () { - const jso = { - 'id': 'test id', - 'name': 'test name', - 'returnTypes': null, - }; + test('load is called for each parameter model', function () { + const state = [ + { + 'id': 'test', + 'parameters': [ + { + 'id': 'test1', + }, + { + 'id': 'test2', + }, + ], + }, + ]; - this.procedureSerializer.load([jso], this.workspace); + const spy = this.sandbox.spy(MockParameterModel, 'loadState'); - const procedureModel = this.procedureMap.getProcedures()[0]; - chai.assert.isNotNull( - procedureModel, - 'Expected a procedure model to exist', - ); - chai.assert.isNull( - procedureModel.getReturnTypes(), - 'Expected the procedure model types to be null', - ); - }); + this.procedureSerializer.load(state, this.workspace); - test('if the return type property is an empty array it is assigned', function () { - const jso = { - 'id': 'test id', - 'name': 'test name', - 'returnTypes': [], - }; - - this.procedureSerializer.load([jso], this.workspace); - - const procedureModel = this.procedureMap.getProcedures()[0]; - chai.assert.isNotNull( - procedureModel, - 'Expected a procedure model to exist', - ); - chai.assert.isArray( - procedureModel.getReturnTypes(), - 'Expected the procedure model types to be an array', - ); - chai.assert.isEmpty( - procedureModel.getReturnTypes(), - 'Expected the procedure model types array to be empty', - ); - }); - - test('if the return type property is a string array it is assigned', function () { - const jso = { - 'id': 'test id', - 'name': 'test name', - 'returnTypes': ['test type 1', 'test type 2'], - }; - - this.procedureSerializer.load([jso], this.workspace); - - const procedureModel = this.procedureMap.getProcedures()[0]; - chai.assert.isNotNull( - procedureModel, - 'Expected a procedure model to exist', - ); - chai.assert.isArray( - procedureModel.getReturnTypes(), - 'Expected the procedure model types to be an array', - ); - chai.assert.deepEqual( - procedureModel.getReturnTypes(), - ['test type 1', 'test type 2'], - 'Expected the procedure model types array to be match the ' + - 'serialized array', - ); - }); - }); - - suite('parameters', function () { - suite('invariant properties', function () { - test('the id property is assigned', function () { - const jso = { - 'id': 'test id', - 'name': 'test name', - 'returnTypes': [], - 'parameters': [ - { - 'id': 'test id', - 'name': 'test name', - }, - ], - }; - - this.procedureSerializer.load([jso], this.workspace); - - const parameterModel = this.procedureMap - .getProcedures()[0] - .getParameters()[0]; - chai.assert.isNotNull( - parameterModel, - 'Expected a parameter model to exist', - ); - chai.assert.equal( - parameterModel.getId(), - 'test id', - 'Expected the parameter model ID to match the serialized ID', - ); - }); - - test('the name property is assigned', function () { - const jso = { - 'id': 'test id', - 'name': 'test name', - 'returnTypes': [], - 'parameters': [ - { - 'id': 'test id', - 'name': 'test name', - }, - ], - }; - - this.procedureSerializer.load([jso], this.workspace); - - const parameterModel = this.procedureMap - .getProcedures()[0] - .getParameters()[0]; - chai.assert.isNotNull( - parameterModel, - 'Expected a parameter model to exist', - ); - chai.assert.equal( - parameterModel.getName(), - 'test name', - 'Expected the parameter model name to match the serialized name', - ); - }); - }); - - suite('types', function () { - test('if the type property does not exist, nothing is assigned', function () { - const jso = { - 'id': 'test id', - 'name': 'test name', - 'returnTypes': [], - 'parameters': [ - { - 'id': 'test id', - 'name': 'test name', - }, - ], - }; - - chai.assert.doesNotThrow(() => { - this.procedureMap.getProcedures()[0].getParameters()[0]; - }, 'Expected the deserializer to skip the non-existant type property'); - }); - - test('if the type property exists, it is assigned', function () { - const jso = { - 'id': 'test id', - 'name': 'test name', - 'returnTypes': [], - 'parameters': [ - { - 'id': 'test id', - 'name': 'test name', - 'types': ['test type 1', 'test type 2'], - }, - ], - }; - - this.procedureSerializer.load([jso], this.workspace); - - const parameterModel = this.procedureMap - .getProcedures()[0] - .getParameters()[0]; - chai.assert.isNotNull( - parameterModel, - 'Expected a parameter model to exist', - ); - chai.assert.deepEqual( - parameterModel.getTypes(), - ['test type 1', 'test type 2'], - 'Expected the parameter model types to match the serialized types', - ); - }); - }); + chai.assert.isTrue( + spy.calledTwice, + 'Expected the loadState method to be called once for each parameter', + ); }); }); }); diff --git a/tests/mocha/jso_serialization_test.js b/tests/mocha/jso_serialization_test.js index 895ff0be2..04bc65b2e 100644 --- a/tests/mocha/jso_serialization_test.js +++ b/tests/mocha/jso_serialization_test.js @@ -25,6 +25,7 @@ suite('JSO Serialization', function () { setup(function () { sharedTestSetup.call(this); this.workspace = new Blockly.Workspace(); + this.sandbox = sinon.createSandbox(); defineStackBlock(); defineRowBlock(); @@ -34,6 +35,7 @@ suite('JSO Serialization', function () { }); teardown(function () { + this.sandbox.restore(); workspaceTeardown.call(this, this.workspace); sharedTestTeardown.call(this); }); @@ -857,106 +859,39 @@ suite('JSO Serialization', function () { this.serializer = null; }); - suite('invariant properties', function () { - test('the state always has an id property', function () { - const procedureModel = new MockProcedureModel(); - this.procedureMap.add(procedureModel); - const jso = this.serializer.save(this.workspace); - const procedure = jso[0]; - assertProperty(procedure, 'id', procedureModel.getId()); - }); + test('save is called on the procedure model', function () { + const proc = new MockProcedureModel(); + this.workspace.getProcedureMap().set('test', proc); + const spy = this.sandbox.spy(proc, 'saveState'); - test('if the name has not been set, name is an empty string', function () { - const procedureModel = new MockProcedureModel(); - this.procedureMap.add(procedureModel); - const jso = this.serializer.save(this.workspace); - const procedure = jso[0]; - assertProperty(procedure, 'name', ''); - }); + this.serializer.save(this.workspace); - test('if the name has been set, name is the string', function () { - const procedureModel = new MockProcedureModel().setName('testName'); - this.procedureMap.add(procedureModel); - const jso = this.serializer.save(this.workspace); - const procedure = jso[0]; - assertProperty(procedure, 'name', 'testName'); - }); + chai.assert.isTrue( + spy.calledOnce, + 'Expected the saveState method to be called on the procedure model', + ); }); - suite('return types', function () { - test('if the procedure does not return, returnTypes is null', function () { - const procedureModel = new MockProcedureModel(); - this.procedureMap.add(procedureModel); - const jso = this.serializer.save(this.workspace); - const procedure = jso[0]; - assertProperty(procedure, 'returnTypes', null); - }); + test('save is called on each parameter model', function () { + const proc = new MockProcedureModel(); + const param1 = new MockParameterModel(); + const param2 = new MockParameterModel(); + proc.insertParameter(param1, 0); + proc.insertParameter(param2, 1); + this.workspace.getProcedureMap().set('test', proc); + const spy1 = this.sandbox.spy(param1, 'saveState'); + const spy2 = this.sandbox.spy(param2, 'saveState'); - test('if the procedure has no return type, returnTypes is an empty array', function () { - const procedureModel = new MockProcedureModel().setReturnTypes([]); - this.procedureMap.add(procedureModel); - const jso = this.serializer.save(this.workspace); - const procedure = jso[0]; - assertProperty(procedure, 'returnTypes', []); - }); + this.serializer.save(this.workspace); - test('if the procedure has return types, returnTypes is the array', function () { - const procedureModel = new MockProcedureModel().setReturnTypes([ - 'a type', - ]); - this.procedureMap.add(procedureModel); - const jso = this.serializer.save(this.workspace); - const procedure = jso[0]; - assertProperty(procedure, 'returnTypes', ['a type']); - }); - }); - - suite('parameters', function () { - suite('invariant properties', function () { - test('the state always has an id property', function () { - const parameterModel = new MockParameterModel('testparam'); - this.procedureMap.add( - new MockProcedureModel().insertParameter(parameterModel, 0), - ); - const jso = this.serializer.save(this.workspace); - const parameter = jso[0]['parameters'][0]; - assertProperty(parameter, 'id', parameterModel.getId()); - }); - - test('the state always has a name property', function () { - const parameterModel = new MockParameterModel('testparam'); - this.procedureMap.add( - new MockProcedureModel().insertParameter(parameterModel, 0), - ); - const jso = this.serializer.save(this.workspace); - const parameter = jso[0]['parameters'][0]; - assertProperty(parameter, 'name', 'testparam'); - }); - }); - - suite('types', function () { - test('if the parameter has no type, there is no type property', function () { - const parameterModel = new MockParameterModel('testparam'); - this.procedureMap.add( - new MockProcedureModel().insertParameter(parameterModel, 0), - ); - const jso = this.serializer.save(this.workspace); - const parameter = jso[0]['parameters'][0]; - assertNoProperty(parameter, 'types'); - }); - - test('if the parameter has types, types is an array', function () { - const parameterModel = new MockParameterModel('testparam').setTypes([ - 'a type', - ]); - this.procedureMap.add( - new MockProcedureModel().insertParameter(parameterModel, 0), - ); - const jso = this.serializer.save(this.workspace); - const parameter = jso[0]['parameters'][0]; - assertProperty(parameter, 'types', ['a type']); - }); - }); + chai.assert.isTrue( + spy1.calledOnce, + 'Expected the saveState method to be called on the first parameter model', + ); + chai.assert.isTrue( + spy2.calledOnce, + 'Expected the saveState method to be called on the first parameter model', + ); }); }); }); diff --git a/tests/mocha/test_helpers/procedures.js b/tests/mocha/test_helpers/procedures.js index e4ddc0e3f..4717b38c8 100644 --- a/tests/mocha/test_helpers/procedures.js +++ b/tests/mocha/test_helpers/procedures.js @@ -189,6 +189,14 @@ export class MockProcedureModel { this.enabled = true; } + static loadState(state, workspace) { + return new MockProcedureModel(); + } + + saveState() { + return {}; + } + setName(name) { this.name = name; return this; @@ -250,6 +258,14 @@ export class MockParameterModel { this.types = []; } + static loadState(state, workspace) { + return new MockParameterModel('test'); + } + + saveState() { + return {}; + } + setName(name) { this.name = name; return this;