Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: refactors concrete implementations of the procedure data models #6575

Merged
merged 8 commits into from
Oct 25, 2022
10 changes: 10 additions & 0 deletions core/interfaces/i_parameter_model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ export interface IParameterModel {
*/
setTypes(types: string[]): this;

/**
* Returns the name of this parameter.
*/
getName(): string;

/**
* Return the types of this parameter.
*/
getTypes(): string[];

/**
* Returns the unique language-neutral ID for the parameter.
*
Expand Down
20 changes: 20 additions & 0 deletions core/interfaces/i_procedure_map.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* @license
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import {IProcedureModel} from './i_procedure_model.js';


export interface IProcedureMap extends Map<string, IProcedureModel> {
/**
* Adds the given ProcedureModel to the map of procedure models, so that
* blocks can find it.
*/
add(proc: IProcedureModel): this;


/** Returns all of the procedures stored in this map. */
getProcedures(): IProcedureModel[];
}
14 changes: 14 additions & 0 deletions core/procedures/observable_parameter_model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,20 @@ export class ObservableParameterModel implements IParameterModel {
'implement your own custom ParameterModel.');
}

/**
* Returns the name of this parameter.
*/
getName(): string {
return this.variable.name;
}

/**
* Returns the types of this parameter.
*/
getTypes(): string[] {
return [];
BeksOmega marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Returns the unique language-neutral ID for the parameter.
*
Expand Down
11 changes: 10 additions & 1 deletion core/procedures/observable_procedure_map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
import type {IProcedureModel} from '../interfaces/i_procedure_model.js';
import {triggerProceduresUpdate} from './update_procedures.js';
import type {Workspace} from '../workspace.js';
import {IProcedureMap} from '../interfaces/i_procedure_map.js';


export class ObservableProcedureMap extends Map<string, IProcedureModel> {
export class ObservableProcedureMap extends
Map<string, IProcedureModel> implements IProcedureMap {
constructor(private readonly workspace: Workspace) {
super();
}
Expand Down Expand Up @@ -52,4 +54,11 @@ export class ObservableProcedureMap extends Map<string, IProcedureModel> {
// TODO(#6526): See if this method is actually useful.
return this.set(proc.getId(), proc);
}

/**
* Returns all of the procedures stored in this map.
*/
getProcedures(): IProcedureModel[] {
return [...this.values()];
}
}
13 changes: 10 additions & 3 deletions core/procedures/observable_procedure_model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ import type {Workspace} from '../workspace.js';

export class ObservableProcedureModel implements IProcedureModel {
private id: string;
private name = '';
private name: string;
private parameters: IParameterModel[] = [];
private returnTypes: string[]|null = null;
private enabled = true;

constructor(private readonly workspace: Workspace, id?: string) {
constructor(
private readonly workspace: Workspace, name: string, id?: string) {
this.id = id ?? genUid();
this.name = name;
}

/** Sets the human-readable name of the procedure. */
Expand Down Expand Up @@ -56,8 +58,13 @@ export class ObservableProcedureModel implements IProcedureModel {
* Pass null to represent a procedure that does not return.
BeksOmega marked this conversation as resolved.
Show resolved Hide resolved
*/
setReturnTypes(types: string[]|null): this {
// TODO(#6516): Fire events.
if (types && types.length) {
throw new Error(
'The built-in ProcedureModel does not support typing. You need to ' +
'implement your own custom ProcedureModel.');
}
this.returnTypes = types;
// TODO(#65): Fire events.
BeksOmega marked this conversation as resolved.
Show resolved Hide resolved
triggerProceduresUpdate(this.workspace);
return this;
}
Expand Down
8 changes: 8 additions & 0 deletions core/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import type * as toolbox from './utils/toolbox.js';
import {VariableMap} from './variable_map.js';
import type {VariableModel} from './variable_model.js';
import type {WorkspaceComment} from './workspace_comment.js';
import {IProcedureMap} from './interfaces/i_procedure_map.js';
import {ObservableProcedureMap} from './procedures.js';


/**
Expand Down Expand Up @@ -110,6 +112,7 @@ export class Workspace implements IASTNodeLocation {
private readonly blockDB = new Map<string, Block>();
private readonly typedBlocksDB = new Map<string, Block[]>();
private variableMap: VariableMap;
private procedureMap: IProcedureMap = new ObservableProcedureMap(this);

/**
* Blocks in the flyout can refer to variables that don't exist in the main
Expand Down Expand Up @@ -829,6 +832,11 @@ export class Workspace implements IASTNodeLocation {
this.variableMap = variableMap;
}

/** Returns the map of all procedures on the workpace. */
getProcedureMap(): IProcedureMap {
return this.procedureMap;
}

/**
* Find the workspace with the specified ID.
*
Expand Down
44 changes: 27 additions & 17 deletions tests/mocha/procedure_map_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ suite('Procedure Map', function() {
setup(function() {
sharedTestSetup.call(this);
this.workspace = new Blockly.Workspace();
// this.procedureMap = this.workspace.getProcedureMap();
this.procedureMap =
new Blockly.procedures.ObservableProcedureMap(this.workspace);
this.procedureMap = this.workspace.getProcedureMap();
});

teardown(function() {
Expand All @@ -40,7 +38,8 @@ suite('Procedure Map', function() {
suite('procedure map updates', function() {
test('inserting a procedure does not trigger an update', function() {
const procedureModel =
new Blockly.procedures.ObservableProcedureModel(this.workspace);
new Blockly.procedures.ObservableProcedureModel(
this.workspace, 'test name');
this.procedureMap.set(procedureModel.getId(), procedureModel);

chai.assert.isFalse(
Expand All @@ -49,15 +48,17 @@ suite('Procedure Map', function() {

test('adding a procedure does not trigger an update', function() {
this.procedureMap.add(
new Blockly.procedures.ObservableProcedureModel(this.workspace));
new Blockly.procedures.ObservableProcedureModel(
this.workspace, 'test name'));

chai.assert.isFalse(
this.updateSpy.called, 'Expected no update to be triggered');
});

test('deleting a procedure triggers an update', function() {
const procedureModel =
new Blockly.procedures.ObservableProcedureModel(this.workspace);
new Blockly.procedures.ObservableProcedureModel(
this.workspace, 'test name');
this.procedureMap.add(procedureModel);

this.procedureMap.delete(procedureModel.getId());
Expand All @@ -70,7 +71,8 @@ suite('Procedure Map', function() {
suite('procedure model updates', function() {
test('setting the name triggers an update', function() {
const procedureModel =
new Blockly.procedures.ObservableProcedureModel(this.workspace);
new Blockly.procedures.ObservableProcedureModel(
this.workspace, 'test name');
this.procedureMap.add(procedureModel);

procedureModel.setName('new name');
Expand All @@ -81,19 +83,21 @@ suite('Procedure Map', function() {

test('setting the return type triggers an update', function() {
const procedureModel =
new Blockly.procedures.ObservableProcedureModel(this.workspace);
new Blockly.procedures.ObservableProcedureModel(
this.workspace, 'test name');
this.procedureMap.add(procedureModel);

procedureModel.setReturnTypes(['return type 1', 'return type 2']);
procedureModel.setReturnTypes([]);

chai.assert.isTrue(
this.updateSpy.calledOnce, 'Expected an update to be triggered');
});

test('removing the return type triggers an update', function() {
const procedureModel =
new Blockly.procedures.ObservableProcedureModel(this.workspace)
.setReturnTypes(['return type']);
new Blockly.procedures.ObservableProcedureModel(
this.workspace, 'test name')
.setReturnTypes([]);
this.procedureMap.add(procedureModel);
this.updateSpy.resetHistory();

Expand All @@ -105,7 +109,8 @@ suite('Procedure Map', function() {

test('disabling the procedure triggers an update', function() {
const procedureModel =
new Blockly.procedures.ObservableProcedureModel(this.workspace);
new Blockly.procedures.ObservableProcedureModel(
this.workspace, 'test name');
this.procedureMap.add(procedureModel);

procedureModel.setEnabled(false);
Expand All @@ -116,7 +121,8 @@ suite('Procedure Map', function() {

test('enabling the procedure triggers an update', function() {
const procedureModel =
new Blockly.procedures.ObservableProcedureModel(this.workspace)
new Blockly.procedures.ObservableProcedureModel(
this.workspace, 'test name')
.setEnabled(false);
this.procedureMap.add(procedureModel);
this.updateSpy.resetHistory();
Expand All @@ -129,7 +135,8 @@ suite('Procedure Map', function() {

test('inserting a parameter triggers an update', function() {
const procedureModel =
new Blockly.procedures.ObservableProcedureModel(this.workspace);
new Blockly.procedures.ObservableProcedureModel(
this.workspace, 'test name');
this.procedureMap.add(procedureModel);

procedureModel.insertParameter(
Expand All @@ -141,7 +148,8 @@ suite('Procedure Map', function() {

test('deleting a parameter triggers an update', function() {
const procedureModel =
new Blockly.procedures.ObservableProcedureModel(this.workspace)
new Blockly.procedures.ObservableProcedureModel(
this.workspace, 'test name')
.insertParameter(
new Blockly.procedures.ObservableParameterModel(
this.workspace));
Expand All @@ -161,7 +169,8 @@ suite('Procedure Map', function() {
new Blockly.procedures.ObservableParameterModel(
this.workspace, 'test1');
this.procedureMap.add(
new Blockly.procedures.ObservableProcedureModel(this.workspace)
new Blockly.procedures.ObservableProcedureModel(
this.workspace, 'test name')
.insertParameter(parameterModel));
this.updateSpy.resetHistory();

Expand All @@ -176,7 +185,8 @@ suite('Procedure Map', function() {
new Blockly.procedures.ObservableParameterModel(
this.workspace, 'test1');
this.procedureMap.add(
new Blockly.procedures.ObservableProcedureModel(this.workspace)
new Blockly.procedures.ObservableProcedureModel(
this.workspace, 'test name')
.insertParameter(parameterModel));
this.updateSpy.resetHistory();

Expand Down