Make registry case-sticky

Adding to a registry preserves case of the key.
Adding a different case of the same key is an error.
Thus any case is ok, as long as one is consistent.
This commit is contained in:
Neil Fraser
2024-03-26 09:14:48 +01:00
parent 55fe91f60a
commit 1d4eafcd3b
7 changed files with 109 additions and 62 deletions

View File

@@ -24,7 +24,6 @@ import type {ToolboxItem} from './toolbox/toolbox_item.js';
import type {IPaster} from './interfaces/i_paster.js';
import type {ICopyData, ICopyable} from './interfaces/i_copyable.js';
import type {IConnectionPreviewer} from './interfaces/i_connection_previewer.js';
import * as deprecation from './utils/deprecation.js';
/**
* A map of maps. With the keys being the type and name of the class we are
@@ -38,6 +37,15 @@ const typeMap: {
} = Object.create(null);
export const TEST_ONLY = {typeMap};
/**
* A map of maps. Record the case-insensitive version of each name.
* e.g. {'field': {'field_angle': 'fIeLd_AnGle'}}
* This enables an error to be thrown if a name of conflicting case is used.
*/
const noCaseNameMap: {
[key: string]: {[key: string]: string}
} = Object.create(null);
/**
* The string used to register the default class for a type of plugin.
*/
@@ -123,7 +131,7 @@ export function register<T>(
| AnyDuringMigration,
opt_allowOverrides?: boolean,
): void {
type = normalizeName(`${type}`);
type = normalizeType(type);
name = normalizeName(name);
if (!registryItem) {
throw Error('Can not register a null value');
@@ -132,10 +140,12 @@ export function register<T>(
// If the type registry has not been created, create it.
if (!typeRegistry) {
typeRegistry = typeMap[type] = Object.create(null);
noCaseNameMap[type] = Object.create(null);
}
// Validate that the given class has all the required properties.
validate(type, registryItem);
validateItem(type, registryItem);
validateCase(type, name);
// Don't throw an error if opt_allowOverrides is true.
// Don't throw an error if the registryItem is not changing.
@@ -147,6 +157,7 @@ export function register<T>(
throw Error(`Name "${name}" with type "${type}" already registered`);
}
typeRegistry[name] = registryItem;
noCaseNameMap[type][name.toLowerCase()] = name;
}
/**
@@ -157,16 +168,29 @@ export function register<T>(
* @param registryItem A class or object that we are checking for the required
* properties.
*/
function validate(type: string, registryItem: Function | AnyDuringMigration) {
function validateItem(type: string, registryItem: Function | AnyDuringMigration) {
switch (type) {
case String(Type.FIELD):
if (typeof registryItem.fromJson !== 'function') {
throw Error('Type "' + type + '" must have a fromJson function');
throw Error(`Type "${type}" must have a fromJson function`);
}
break;
}
}
/**
* Checks that the name hasn't been registered using a different case.
*
* @param type The type of the plugin. (e.g. Field, Renderer)
* @param name The plugin's name. (Ex. field_angle, geras)
*/
function validateCase(type: string, name: string) {
const caseName = noCaseNameMap[type]?.[name.toLowerCase()];
if (caseName && caseName !== name) {
throw Error(`Inconsistent case with name "${name}" vs "${caseName}"`);
}
}
/**
* Unregisters the registry item with the given type and name.
*
@@ -175,14 +199,16 @@ function validate(type: string, registryItem: Function | AnyDuringMigration) {
* @param name The plugin's name. (Ex. field_angle, geras)
*/
export function unregister<T>(type: string | Type<T>, name: string) {
type = normalizeName(`${type}`);
type = normalizeType(type);
name = normalizeName(name);
validateCase(type, name);
const typeRegistry = typeMap[type];
if (!typeRegistry || !typeRegistry[name]) {
console.warn(`Unable to unregister [${name}][${type}] from the registry`);
return;
}
delete typeMap[type][name];
delete typeRegistry[name];
delete noCaseNameMap[type][name.toLowerCase()];
}
/**
@@ -202,8 +228,9 @@ function getItem<T>(
name: string,
opt_throwIfMissing?: boolean,
): (new (...p1: AnyDuringMigration[]) => T) | null | AnyDuringMigration {
type = normalizeName(`${type}`);
type = normalizeType(type);
name = normalizeName(name);
validateCase(type, name);
const typeRegistry = typeMap[type];
if (!typeRegistry || !typeRegistry[name]) {
const msg = `Unable to find [${name}][${type}] in the registry.`;
@@ -228,8 +255,9 @@ function getItem<T>(
* otherwise.
*/
export function hasItem<T>(type: string | Type<T>, name: string): boolean {
type = normalizeName(`${type}`);
type = normalizeType(type);
name = normalizeName(name);
validateCase(type, name);
const typeRegistry = typeMap[type];
if (!typeRegistry) {
return false;
@@ -238,16 +266,26 @@ export function hasItem<T>(type: string | Type<T>, name: string): boolean {
}
/**
* Normalize a string to be used as a key. By default, lowercase and trimmed.
* Normalize a string to be used as a type. Trim whitespaces and lowercase.
*
* @param name The key's name. (Ex. 'FiELd_AnGlE ')
* @param name The type. (Ex. 'FiELd ')
* @returns A normalized string. (Ex. 'field')
*/
function normalizeType(type: any): string {
type = `${type}`.trim().toLowerCase();
if (!type) throw Error('Empty type');
return type;
}
/**
* Normalize a string to be used as a key. Trim whitespaces.
*
* @param name The key's name. (Ex. 'field_angle ')
* @returns A normalized string. (Ex. 'field_angle')
*/
function normalizeName(name: string): string {
// Do we need this type checking? We are using TypeScript after all.
// But there's a unit test that depends on throwing this error.
if (typeof name !== 'string') throw Error('Invalid name');
name = name.toLowerCase().trim();
name = name.trim();
if (!name) throw Error('Empty name');
return name;
}
@@ -305,9 +343,9 @@ export function getAllItems<T>(
opt_throwIfMissing?: boolean,
): {[key: string]: T | null | (new (...p1: AnyDuringMigration[]) => T)} | null {
if (typeof opt_cased !== 'undefined') {
deprecation.warn('Blockly.registry.getAllItems', 'v11', 'v12');
console.warn('Blockly.registry.getAllItems deprecated "cased" argument');
}
type = normalizeName(`${type}`);
type = normalizeType(type);
const typeRegistry = typeMap[type];
if (!typeRegistry) {
const msg = `Unable to find [${type}] in the registry.`;

View File

@@ -24,7 +24,7 @@ export function save(workspace: Workspace): {
[key: string]: AnyDuringMigration;
} {
const state = Object.create(null);
const serializerMap = registry.getAllItems(registry.Type.SERIALIZER, true);
const serializerMap = registry.getAllItems(registry.Type.SERIALIZER);
for (const key in serializerMap) {
const save = (serializerMap[key] as ISerializer)?.save(workspace);
if (save) {
@@ -47,7 +47,7 @@ export function load(
workspace: Workspace,
{recordUndo = false}: {recordUndo?: boolean} = {},
) {
const serializerMap = registry.getAllItems(registry.Type.SERIALIZER, true);
const serializerMap = registry.getAllItems(registry.Type.SERIALIZER);
if (!serializerMap) {
return;
}

View File

@@ -30,7 +30,8 @@ export class CollapsibleToolboxCategory
implements ICollapsibleToolboxItem
{
/** Name used for registering a collapsible toolbox category. */
static override registrationName = 'collapsibleCategory';
/** Must be lowercase. */
static override registrationName = 'collapsiblecategory';
/** Container for any child categories. */
protected subcategoriesDiv_: HTMLDivElement | null = null;
@@ -75,7 +76,7 @@ export class CollapsibleToolboxCategory
// Separators can exist as either a flyout item or a toolbox item so
// decide where it goes based on the type of the previous item.
if (
!registry.hasItem(registry.Type.TOOLBOX_ITEM, itemDef['kind']) ||
!registry.hasItem(registry.Type.TOOLBOX_ITEM, itemDef['kind'].toLowerCase()) ||
(itemDef['kind'].toLowerCase() ===
ToolboxSeparator.registrationName &&
prevIsFlyoutItem)
@@ -109,7 +110,7 @@ export class CollapsibleToolboxCategory
}
const ToolboxItemClass = registry.getClass(
registry.Type.TOOLBOX_ITEM,
registryName,
registryName.toLowerCase(),
);
const toolboxItem = new ToolboxItemClass!(
itemDef,

View File

@@ -25,6 +25,7 @@ import {ToolboxItem} from './toolbox_item.js';
*/
export class ToolboxSeparator extends ToolboxItem {
/** Name used for registering a toolbox separator. */
/** Must be lowercase. */
static registrationName = 'sep';
/** All the CSS class names that are used to create a separator. */

View File

@@ -366,11 +366,8 @@ export class Toolbox
*/
render(toolboxDef: toolbox.ToolboxInfo) {
this.toolboxDef_ = toolboxDef;
for (let i = 0; i < this.contents_.length; i++) {
const toolboxItem = this.contents_[i];
if (toolboxItem) {
toolboxItem.dispose();
}
for (const toolboxItem of this.contents_) {
toolboxItem?.dispose();
}
this.contents_ = [];
this.contentMap_ = Object.create(null);

View File

@@ -91,11 +91,9 @@ suite('Field Registry', function () {
type: 'FIELD_CUSTOM_TEST',
value: 'ok',
};
const field = Blockly.fieldRegistry.fromJson(json);
chai.assert.isNotNull(field);
chai.assert.equal(field.getValue(), 'ok');
chai.assert.throws(function () {
Blockly.fieldRegistry.fromJson(json);
}, 'Inconsistent case');
});
});
});

View File

@@ -55,9 +55,13 @@ suite('Registry', function () {
// Changing the case or padding doesn't help.
chai.assert.throws(function () {
Blockly.registry.register('test', ' test_NAME ', {});
}, 'already registered');
}, 'Inconsistent case');
// But it's ok if explicitly allowed with 'true'.
Blockly.registry.register('test', 'test_name', {}, true);
// But not ok if it's a different case.
chai.assert.throws(function () {
Blockly.registry.register('test', 'Test_Name', {}, true);
}, 'Inconsistent case');
});
test('Null Value', function () {
@@ -87,16 +91,20 @@ suite('Registry', function () {
});
suite('Name normalization', function () {
test('Caseless type', function () {
test('Padded', function () {
chai.assert.isTrue(
Blockly.registry.hasItem(' test ', ' test_name ')
);
});
test('Case type', function () {
chai.assert.isTrue(Blockly.registry.hasItem('TEST', 'test_name'));
});
test('Caseless name', function () {
chai.assert.isTrue(Blockly.registry.hasItem('test', 'TEST_NAME'));
});
test('Padded name', function () {
chai.assert.isTrue(Blockly.registry.hasItem(' test ', 'TEST_NAME'));
test('Case name', function () {
chai.assert.throws(function () {
Blockly.registry.hasItem('test', 'TEST_NAME');
}, 'Inconsistent case');
});
});
});
@@ -133,16 +141,20 @@ suite('Registry', function () {
});
suite('Name normalization', function () {
test('Caseless type', function () {
test('Padded', function () {
chai.assert.isNotNull(
Blockly.registry.getClass(' test ', ' test_name' )
);
});
test('Case type', function () {
chai.assert.isNotNull(Blockly.registry.getClass('TEST', 'test_name'));
});
test('Caseless name', function () {
chai.assert.isNotNull(Blockly.registry.getClass('test', 'TEST_NAME'));
});
test('Padded name', function () {
chai.assert.isTrue(Blockly.registry.hasItem(' test ', 'TEST_NAME'));
test('Case name', function () {
chai.assert.throws(function () {
Blockly.registry.getClass('test', 'TEST_NAME');
}, 'Inconsistent case');
});
});
});
@@ -179,16 +191,20 @@ suite('Registry', function () {
});
suite('Name normalization', function () {
test('Caseless type', function () {
chai.assert.isNotNull(Blockly.registry.getObject('TEST', 'test_name'));
test('Padded', function () {
chai.assert.isNotNull(
Blockly.registry.getObject(' test ', ' test_name ')
);
});
test('Caseless name', function () {
chai.assert.isNotNull(Blockly.registry.getObject('test', 'TEST_NAME'));
test('Case type', function () {
chai.assert.isTrue(Blockly.registry.hasItem('TEST', 'test_name'));
});
test('Padded name', function () {
chai.assert.isTrue(Blockly.registry.hasItem(' test ', 'TEST_NAME'));
test('Case name', function () {
chai.assert.throws(function () {
Blockly.registry.getObject('test', 'TEST_NAME');
}, 'Inconsistent case');
});
});
});
@@ -200,11 +216,15 @@ suite('Registry', function () {
});
teardown(function () {
Blockly.registry.unregister('test', 'casedname');
Blockly.registry.unregister('test', 'test_name');
Blockly.registry.unregister('test', 'casedNAME');
});
test('Has', function () {
chai.assert.isNotNull(Blockly.registry.getAllItems('test'));
chai.assert.deepEqual(Blockly.registry.getAllItems('test'), {
'test_name': {},
'casedNAME': {},
});
});
test('Does not have', function () {
@@ -222,14 +242,6 @@ suite('Registry', function () {
test('Ignore type case', function () {
chai.assert.isNotNull(Blockly.registry.getAllItems('TEST'));
});
test('Overwriting name case', function () {
Blockly.registry.register('test', 'CASEDname', {}, true);
chai.assert.deepEqual(Blockly.registry.getAllItems('test'), {
'test_name': {},
'casedname': {},
});
});
});
suite('getClassFromOptions', function () {