Refactor to use string IDs for executables #262
This commit unifies the concepts of executables having same ID structure. It paves the way for more complex ID structure and using IDs in collection files as part of new ID solution (#262). Using string IDs also leads to more expressive test code. This commit also refactors the rest of the code to adopt to the changes. This commit: - Separate concerns from entities for data access (in repositories) and executables. Executables use `Identifiable` meanwhile repositories use `RepositoryEntity`. - Refactor unnecessary generic parameters for enttities and ids, enforcing string gtype everwyhere. - Changes numeric IDs to string IDs for categories to unify the retrieval and construction for executables, using pseudo-ids (their names) just like scripts. - Remove `BaseEntity` for simplicity. - Simplify usage and construction of executable objects. Move factories responsible for creation of category/scripts to domain layer. Do not longer export `CollectionCategorY` and `CollectionScript`. - Use named typed for string IDs for better differentation of different ID contexts in code.
This commit is contained in:
undergroundwires
@@ -5,7 +5,7 @@ import { CategoryReverter } from '@/presentation/components/Scripts/View/Tree/No
|
||||
import { CategoryCollectionStub } from '@tests/unit/shared/Stubs/CategoryCollectionStub';
|
||||
import { CategoryStub } from '@tests/unit/shared/Stubs/CategoryStub';
|
||||
import { ScriptStub } from '@tests/unit/shared/Stubs/ScriptStub';
|
||||
import { getCategoryNodeId, getScriptNodeId } from '@/presentation/components/Scripts/View/Tree/TreeViewAdapter/CategoryNodeMetadataConverter';
|
||||
import { getCategoryNodeId, createNodeIdForExecutable } from '@/presentation/components/Scripts/View/Tree/TreeViewAdapter/CategoryNodeMetadataConverter';
|
||||
import { type NodeMetadata, NodeType } from '@/presentation/components/Scripts/View/Tree/NodeContent/NodeMetadata';
|
||||
|
||||
describe('ReverterFactory', () => {
|
||||
@@ -24,7 +24,7 @@ describe('ReverterFactory', () => {
|
||||
it('gets ScriptReverter for script node', () => {
|
||||
// arrange
|
||||
const script = new ScriptStub('test');
|
||||
const node = getNodeContentStub(getScriptNodeId(script), NodeType.Script);
|
||||
const node = getNodeContentStub(createNodeIdForExecutable(script), NodeType.Script);
|
||||
const collection = new CategoryCollectionStub()
|
||||
.withAction(new CategoryStub(0).withScript(script));
|
||||
// act
|
||||
|
||||
@@ -2,7 +2,7 @@ import { describe, it, expect } from 'vitest';
|
||||
import { ScriptReverter } from '@/presentation/components/Scripts/View/Tree/NodeContent/Reverter/ScriptReverter';
|
||||
import { ScriptStub } from '@tests/unit/shared/Stubs/ScriptStub';
|
||||
import { SelectedScriptStub } from '@tests/unit/shared/Stubs/SelectedScriptStub';
|
||||
import { getScriptNodeId } from '@/presentation/components/Scripts/View/Tree/TreeViewAdapter/CategoryNodeMetadataConverter';
|
||||
import { createNodeIdForExecutable } from '@/presentation/components/Scripts/View/Tree/TreeViewAdapter/CategoryNodeMetadataConverter';
|
||||
import { UserSelectionStub } from '@tests/unit/shared/Stubs/UserSelectionStub';
|
||||
import type { SelectedScript } from '@/application/Context/State/Selection/Script/SelectedScript';
|
||||
import { ScriptSelectionStub } from '@tests/unit/shared/Stubs/ScriptSelectionStub';
|
||||
@@ -11,7 +11,7 @@ describe('ScriptReverter', () => {
|
||||
describe('getState', () => {
|
||||
// arrange
|
||||
const script = new ScriptStub('id');
|
||||
const nodeId = getScriptNodeId(script);
|
||||
const nodeId = createNodeIdForExecutable(script);
|
||||
const testScenarios: ReadonlyArray<{
|
||||
readonly description: string;
|
||||
readonly selectedScripts: readonly SelectedScript[];
|
||||
@@ -98,7 +98,7 @@ describe('ScriptReverter', () => {
|
||||
expectedRevert: false,
|
||||
},
|
||||
];
|
||||
const nodeId = getScriptNodeId(script);
|
||||
const nodeId = createNodeIdForExecutable(script);
|
||||
testScenarios.forEach((
|
||||
{ description, selection, expectedRevert },
|
||||
) => {
|
||||
@@ -111,7 +111,7 @@ describe('ScriptReverter', () => {
|
||||
// act
|
||||
sut.selectWithRevertState(revertState, userSelection);
|
||||
// assert
|
||||
expect(scriptSelection.isScriptSelected(script.id, expectedRevert)).to.equal(true);
|
||||
expect(scriptSelection.isScriptSelected(script.executableId, expectedRevert)).to.equal(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3,13 +3,14 @@ import { TreeNodeManager } from '@/presentation/components/Scripts/View/Tree/Tre
|
||||
import { itEachAbsentObjectValue, itEachAbsentStringValue } from '@tests/unit/shared/TestCases/AbsentTests';
|
||||
import { TreeNodeHierarchy } from '@/presentation/components/Scripts/View/Tree/TreeView/Node/Hierarchy/TreeNodeHierarchy';
|
||||
import { TreeNodeState } from '@/presentation/components/Scripts/View/Tree/TreeView/Node/State/TreeNodeState';
|
||||
import type { TreeNodeId } from '@/presentation/components/Scripts/View/Tree/TreeView/Node/TreeNode';
|
||||
|
||||
describe('TreeNodeManager', () => {
|
||||
describe('constructor', () => {
|
||||
describe('id', () => {
|
||||
it('should initialize with the provided id', () => {
|
||||
// arrange
|
||||
const expectedId = 'test-id';
|
||||
const expectedId: TreeNodeId = 'test-id';
|
||||
// act
|
||||
const node = new TreeNodeManager(expectedId);
|
||||
// assert
|
||||
@@ -18,9 +19,10 @@ describe('TreeNodeManager', () => {
|
||||
describe('should throw an error if id is not provided', () => {
|
||||
itEachAbsentStringValue((absentId) => {
|
||||
// arrange
|
||||
const id = absentId as TreeNodeId;
|
||||
const expectedError = 'missing id';
|
||||
// act
|
||||
const act = () => new TreeNodeManager(absentId);
|
||||
const act = () => new TreeNodeManager(id);
|
||||
// assert
|
||||
expect(act).to.throw(expectedError);
|
||||
}, { excludeNull: true, excludeUndefined: true });
|
||||
|
||||
@@ -5,31 +5,36 @@ import { CategoryStub } from '@tests/unit/shared/Stubs/CategoryStub';
|
||||
import { ScriptStub } from '@tests/unit/shared/Stubs/ScriptStub';
|
||||
import { CategoryCollectionStub } from '@tests/unit/shared/Stubs/CategoryCollectionStub';
|
||||
import {
|
||||
getCategoryId, getCategoryNodeId, getScriptId,
|
||||
getScriptNodeId, parseAllCategories, parseSingleCategory,
|
||||
createExecutableIdFromNodeId,
|
||||
createNodeIdForExecutable,
|
||||
parseAllCategories,
|
||||
parseSingleCategory,
|
||||
} from '@/presentation/components/Scripts/View/Tree/TreeViewAdapter/CategoryNodeMetadataConverter';
|
||||
import { ExecutableType } from '@/application/Parser/Executable/Validation/ExecutableType';
|
||||
import type { NodeMetadata } from '@/presentation/components/Scripts/View/Tree/NodeContent/NodeMetadata';
|
||||
import { expectExists } from '@tests/shared/Assertions/ExpectExists';
|
||||
import type { ExecutableId } from '@/domain/Executables/Identifiable';
|
||||
|
||||
describe('CategoryNodeMetadataConverter', () => {
|
||||
it('can convert script id and back', () => {
|
||||
// arrange
|
||||
const script = new ScriptStub('test');
|
||||
const expectedScriptId: ExecutableId = 'expected-script-id';
|
||||
const script = new ScriptStub(expectedScriptId);
|
||||
// act
|
||||
const nodeId = getScriptNodeId(script);
|
||||
const scriptId = getScriptId(nodeId);
|
||||
const nodeId = createNodeIdForExecutable(script);
|
||||
const actualScriptId = createExecutableIdFromNodeId(nodeId);
|
||||
// assert
|
||||
expect(scriptId).to.equal(script.id);
|
||||
expect(actualScriptId).to.equal(expectedScriptId);
|
||||
});
|
||||
it('can convert category id and back', () => {
|
||||
// arrange
|
||||
const category = new CategoryStub(55);
|
||||
const expectedCategoryId: ExecutableId = 'expected-category-id';
|
||||
const category = new CategoryStub(expectedCategoryId);
|
||||
// act
|
||||
const nodeId = getCategoryNodeId(category);
|
||||
const scriptId = getCategoryId(nodeId);
|
||||
const nodeId = createNodeIdForExecutable(category);
|
||||
const actualCategoryId = createExecutableIdFromNodeId(nodeId);
|
||||
// assert
|
||||
expect(scriptId).to.equal(category.id);
|
||||
expect(actualCategoryId).to.equal(expectedCategoryId);
|
||||
});
|
||||
describe('parseSingleCategory', () => {
|
||||
it('throws error if parent category cannot be retrieved', () => {
|
||||
@@ -38,32 +43,45 @@ describe('CategoryNodeMetadataConverter', () => {
|
||||
const collection = new CategoryCollectionStub();
|
||||
collection.getCategory = () => { throw new Error(expectedError); };
|
||||
// act
|
||||
const act = () => parseSingleCategory(31, collection);
|
||||
const act = () => parseSingleCategory('unimportant-id', collection);
|
||||
// assert
|
||||
expect(act).to.throw(expectedError);
|
||||
});
|
||||
it('can parse when category has sub categories', () => {
|
||||
// arrange
|
||||
const categoryId = 31;
|
||||
const firstSubCategory = new CategoryStub(11).withScriptIds('111', '112');
|
||||
const secondSubCategory = new CategoryStub(categoryId)
|
||||
.withCategory(new CategoryStub(33).withScriptIds('331', '331'))
|
||||
.withCategory(new CategoryStub(44).withScriptIds('44'));
|
||||
const collection = new CategoryCollectionStub().withAction(new CategoryStub(categoryId)
|
||||
.withCategory(firstSubCategory)
|
||||
.withCategory(secondSubCategory));
|
||||
const parentCategoryId: ExecutableId = 'parent-category';
|
||||
const firstSubcategory = new CategoryStub('subcategory-1')
|
||||
.withScriptIds('subcategory-1-script-1', 'subcategory-1-script-2');
|
||||
const secondSubCategory = new CategoryStub('subcategory-2')
|
||||
.withCategory(
|
||||
new CategoryStub('subcategory-2-subcategory-1')
|
||||
.withScriptIds('subcategory-2-subcategory-1-script-1', 'subcategory-2-subcategory-1-script-2'),
|
||||
)
|
||||
.withCategory(
|
||||
new CategoryStub('subcategory-2-subcategory-2')
|
||||
.withScriptIds('subcategory-2-subcategory-2-script-1'),
|
||||
);
|
||||
const collection = new CategoryCollectionStub().withAction(
|
||||
new CategoryStub(parentCategoryId)
|
||||
.withCategory(firstSubcategory)
|
||||
.withCategory(secondSubCategory),
|
||||
);
|
||||
// act
|
||||
const nodes = parseSingleCategory(categoryId, collection);
|
||||
const nodes = parseSingleCategory(parentCategoryId, collection);
|
||||
// assert
|
||||
expectExists(nodes);
|
||||
expect(nodes).to.have.lengthOf(2);
|
||||
expectSameCategory(nodes[0], firstSubCategory);
|
||||
expectSameCategory(nodes[0], firstSubcategory);
|
||||
expectSameCategory(nodes[1], secondSubCategory);
|
||||
});
|
||||
it('can parse when category has sub scripts', () => {
|
||||
// arrange
|
||||
const categoryId = 31;
|
||||
const scripts = [new ScriptStub('script1'), new ScriptStub('script2'), new ScriptStub('script3')];
|
||||
const categoryId: ExecutableId = 'expected-category-id';
|
||||
const scripts: readonly Script[] = [
|
||||
new ScriptStub('script1'),
|
||||
new ScriptStub('script2'),
|
||||
new ScriptStub('script3'),
|
||||
];
|
||||
const collection = new CategoryCollectionStub()
|
||||
.withAction(new CategoryStub(categoryId).withScripts(...scripts));
|
||||
// act
|
||||
@@ -79,10 +97,11 @@ describe('CategoryNodeMetadataConverter', () => {
|
||||
it('parseAllCategories parses as expected', () => {
|
||||
// arrange
|
||||
const collection = new CategoryCollectionStub()
|
||||
.withAction(new CategoryStub(0).withScriptIds('1, 2'))
|
||||
.withAction(new CategoryStub(1).withCategories(
|
||||
new CategoryStub(3).withScriptIds('3', '4'),
|
||||
new CategoryStub(4).withCategory(new CategoryStub(5).withScriptIds('6')),
|
||||
.withAction(new CategoryStub('category-1').withScriptIds('1, 2'))
|
||||
.withAction(new CategoryStub('category-2').withCategories(
|
||||
new CategoryStub('category-2-subcategory-1').withScriptIds('3', '4'),
|
||||
new CategoryStub('category-2-subcategory-1')
|
||||
.withCategory(new CategoryStub('category-2-subcategory-1-subcategory-1').withScriptIds('6')),
|
||||
));
|
||||
// act
|
||||
const nodes = parseAllCategories(collection);
|
||||
@@ -100,8 +119,8 @@ function isReversible(category: Category): boolean {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
if (category.subCategories) {
|
||||
if (category.subCategories.some((c) => !isReversible(c))) {
|
||||
if (category.subcategories) {
|
||||
if (category.subcategories.some((c) => !isReversible(c))) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
@@ -110,17 +129,17 @@ function isReversible(category: Category): boolean {
|
||||
|
||||
function expectSameCategory(node: NodeMetadata, category: Category): void {
|
||||
expect(node.type).to.equal(ExecutableType.Category, getErrorMessage('type'));
|
||||
expect(node.id).to.equal(getCategoryNodeId(category), getErrorMessage('id'));
|
||||
expect(node.id).to.equal(createNodeIdForExecutable(category), getErrorMessage('id'));
|
||||
expect(node.docs).to.equal(category.docs, getErrorMessage('docs'));
|
||||
expect(node.text).to.equal(category.name, getErrorMessage('name'));
|
||||
expect(node.isReversible).to.equal(isReversible(category), getErrorMessage('isReversible'));
|
||||
expect(node.children).to.have.lengthOf(
|
||||
category.scripts.length + category.subCategories.length,
|
||||
category.scripts.length + category.subcategories.length,
|
||||
getErrorMessage('total children'),
|
||||
);
|
||||
if (category.subCategories) {
|
||||
for (let i = 0; i < category.subCategories.length; i++) {
|
||||
expectSameCategory(node.children[i], category.subCategories[i]);
|
||||
if (category.subcategories) {
|
||||
for (let i = 0; i < category.subcategories.length; i++) {
|
||||
expectSameCategory(node.children[i], category.subcategories[i]);
|
||||
}
|
||||
}
|
||||
if (category.scripts) {
|
||||
@@ -137,7 +156,7 @@ function expectSameCategory(node: NodeMetadata, category: Category): void {
|
||||
|
||||
function expectSameScript(node: NodeMetadata, script: Script): void {
|
||||
expect(node.type).to.equal(ExecutableType.Script, getErrorMessage('type'));
|
||||
expect(node.id).to.equal(getScriptNodeId(script), getErrorMessage('id'));
|
||||
expect(node.id).to.equal(createNodeIdForExecutable(script), getErrorMessage('id'));
|
||||
expect(node.docs).to.equal(script.docs, getErrorMessage('docs'));
|
||||
expect(node.text).to.equal(script.name, getErrorMessage('name'));
|
||||
expect(node.isReversible).to.equal(script.canRevert(), getErrorMessage('canRevert'));
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { useSelectedScriptNodeIds } from '@/presentation/components/Scripts/View/Tree/TreeViewAdapter/UseSelectedScriptNodeIds';
|
||||
import { SelectedScriptStub } from '@tests/unit/shared/Stubs/SelectedScriptStub';
|
||||
import { getScriptNodeId } from '@/presentation/components/Scripts/View/Tree/TreeViewAdapter/CategoryNodeMetadataConverter';
|
||||
import { createNodeIdForExecutable } from '@/presentation/components/Scripts/View/Tree/TreeViewAdapter/CategoryNodeMetadataConverter';
|
||||
import type { Script } from '@/domain/Executables/Script/Script';
|
||||
import { UseUserSelectionStateStub } from '@tests/unit/shared/Stubs/UseUserSelectionStateStub';
|
||||
import { ScriptStub } from '@tests/unit/shared/Stubs/ScriptStub';
|
||||
import type { TreeNodeId } from '@/presentation/components/Scripts/View/Tree/TreeView/Node/TreeNode';
|
||||
import type { Executable } from '@/domain/Executables/Executable';
|
||||
|
||||
describe('useSelectedScriptNodeIds', () => {
|
||||
it('returns an empty array when no scripts are selected', () => {
|
||||
@@ -23,7 +25,7 @@ describe('useSelectedScriptNodeIds', () => {
|
||||
new SelectedScriptStub(new ScriptStub('id-1')),
|
||||
new SelectedScriptStub(new ScriptStub('id-2')),
|
||||
];
|
||||
const parsedNodeIds = new Map<Script, string>([
|
||||
const parsedNodeIds = new Map<Script, TreeNodeId>([
|
||||
[selectedScripts[0].script, 'expected-id-1'],
|
||||
[selectedScripts[1].script, 'expected-id-2'],
|
||||
]);
|
||||
@@ -47,7 +49,7 @@ describe('useSelectedScriptNodeIds', () => {
|
||||
new SelectedScriptStub(new ScriptStub('id-1')),
|
||||
new SelectedScriptStub(new ScriptStub('id-2')),
|
||||
];
|
||||
const parsedNodeIds = new Map<Script, string>([
|
||||
const parsedNodeIds = new Map<Script, TreeNodeId>([
|
||||
[changedScripts[0].script, 'expected-id-1'],
|
||||
[changedScripts[1].script, 'expected-id-2'],
|
||||
]);
|
||||
@@ -68,9 +70,9 @@ describe('useSelectedScriptNodeIds', () => {
|
||||
});
|
||||
});
|
||||
|
||||
type ScriptNodeIdParser = typeof getScriptNodeId;
|
||||
type NodeIdParser = typeof createNodeIdForExecutable;
|
||||
|
||||
function createNodeIdParserFromMap(scriptToIdMap: Map<Script, string>): ScriptNodeIdParser {
|
||||
function createNodeIdParserFromMap(scriptToIdMap: Map<Executable, TreeNodeId>): NodeIdParser {
|
||||
return (script) => {
|
||||
const expectedId = scriptToIdMap.get(script);
|
||||
if (!expectedId) {
|
||||
@@ -81,12 +83,12 @@ function createNodeIdParserFromMap(scriptToIdMap: Map<Script, string>): ScriptNo
|
||||
}
|
||||
|
||||
function runHook(scenario?: {
|
||||
readonly scriptNodeIdParser?: ScriptNodeIdParser,
|
||||
readonly scriptNodeIdParser?: NodeIdParser,
|
||||
readonly useSelectionState?: UseUserSelectionStateStub,
|
||||
}) {
|
||||
const useSelectionStateStub = scenario?.useSelectionState ?? new UseUserSelectionStateStub();
|
||||
const nodeIdParser: ScriptNodeIdParser = scenario?.scriptNodeIdParser
|
||||
?? ((script) => script.id);
|
||||
const nodeIdParser: NodeIdParser = scenario?.scriptNodeIdParser
|
||||
?? ((script) => script.executableId);
|
||||
const returnObject = useSelectedScriptNodeIds(useSelectionStateStub.get(), nodeIdParser);
|
||||
return {
|
||||
returnObject,
|
||||
|
||||
@@ -216,29 +216,29 @@ function itExpectedFilterTriggeredEvent(
|
||||
{
|
||||
description: 'returns true when category exists',
|
||||
scriptMatches: [],
|
||||
categoryMatches: [new CategoryStub(1)],
|
||||
givenNode: createNode({ id: '1', hasParent: false }),
|
||||
categoryMatches: [new CategoryStub('category-match-1')],
|
||||
givenNode: createNode({ id: 'category-match-1', hasParent: false }),
|
||||
expectedPredicateResult: true,
|
||||
},
|
||||
{
|
||||
description: 'returns true when script exists',
|
||||
scriptMatches: [new ScriptStub('a')],
|
||||
scriptMatches: [new ScriptStub('script-match-1')],
|
||||
categoryMatches: [],
|
||||
givenNode: createNode({ id: 'a', hasParent: true }),
|
||||
givenNode: createNode({ id: 'script-match-1', hasParent: true }),
|
||||
expectedPredicateResult: true,
|
||||
},
|
||||
{
|
||||
description: 'returns false when category is missing',
|
||||
scriptMatches: [new ScriptStub('b')],
|
||||
categoryMatches: [new CategoryStub(2)],
|
||||
givenNode: createNode({ id: '1', hasParent: false }),
|
||||
scriptMatches: [new ScriptStub('script-match-1')],
|
||||
categoryMatches: [new CategoryStub('category-match-1')],
|
||||
givenNode: createNode({ id: 'unrelated-node', hasParent: false }),
|
||||
expectedPredicateResult: false,
|
||||
},
|
||||
{
|
||||
description: 'finds false when script is missing',
|
||||
scriptMatches: [new ScriptStub('b')],
|
||||
categoryMatches: [new CategoryStub(1)],
|
||||
givenNode: createNode({ id: 'a', hasParent: true }),
|
||||
scriptMatches: [new ScriptStub('script-match-1')],
|
||||
categoryMatches: [new CategoryStub('category-match-1')],
|
||||
givenNode: createNode({ id: 'unrelated-node', hasParent: true }),
|
||||
expectedPredicateResult: false,
|
||||
},
|
||||
];
|
||||
@@ -261,8 +261,8 @@ function itExpectedFilterTriggeredEvent(
|
||||
expect(event.value.predicate).toBeDefined();
|
||||
const actualPredicateResult = event.value.predicate(givenNode);
|
||||
expect(actualPredicateResult).to.equal(expectedPredicateResult, formatAssertionMessage([
|
||||
`Script matches (${scriptMatches.length}): [${scriptMatches.map((s) => s.id).join(', ')}]`,
|
||||
`Category matches (${categoryMatches.length}): [${categoryMatches.map((s) => s.id).join(', ')}]`,
|
||||
`Script matches (${scriptMatches.length}): [${scriptMatches.map((s) => s.executableId).join(', ')}]`,
|
||||
`Category matches (${categoryMatches.length}): [${categoryMatches.map((s) => s.executableId).join(', ')}]`,
|
||||
`Expected node: "${givenNode.id}"`,
|
||||
]));
|
||||
});
|
||||
|
||||
@@ -7,7 +7,7 @@ import { CategoryCollectionStateStub } from '@tests/unit/shared/Stubs/CategoryCo
|
||||
import { UseCollectionStateStub } from '@tests/unit/shared/Stubs/UseCollectionStateStub';
|
||||
import { CategoryCollectionStub } from '@tests/unit/shared/Stubs/CategoryCollectionStub';
|
||||
import { CategoryStub } from '@tests/unit/shared/Stubs/CategoryStub';
|
||||
import type { ICategoryCollection } from '@/domain/ICategoryCollection';
|
||||
import type { ICategoryCollection } from '@/domain/Collection/ICategoryCollection';
|
||||
import type { NodeMetadata } from '@/presentation/components/Scripts/View/Tree/NodeContent/NodeMetadata';
|
||||
import { NodeMetadataStub } from '@tests/unit/shared/Stubs/NodeMetadataStub';
|
||||
import { convertToNodeInput } from '@/presentation/components/Scripts/View/Tree/TreeViewAdapter/TreeNodeMetadataConverter';
|
||||
|
||||
Reference in New Issue
Block a user