Improve hierarchy + tests

Add tests for hierarchy and few performance/memory  optimizations.
This commit is contained in:
Andrey Sobolev
2025-10-08 16:02:39 +07:00
parent 5ec9c0b066
commit f29ce90632
3 changed files with 252 additions and 24 deletions
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@hcengineering/core",
"comment": "Hierarchy improvements",
"type": "patch"
}
],
"packageName": "@hcengineering/core"
}
@@ -106,4 +106,226 @@ describe('hierarchy', () => {
spyMixinClass.mockReset()
spyMixinClass.mockRestore()
})
// Memory optimization tests - ancestors stored as array
it('getAncestors should return array directly', async () => {
const hierarchy = prepare()
const ancestors = hierarchy.getAncestors(core.class.TxCreateDoc)
// Verify it's an array
expect(Array.isArray(ancestors)).toBeTruthy()
// Verify it contains expected ancestors
expect(ancestors).toContain(core.class.TxCreateDoc)
expect(ancestors).toContain(core.class.TxCUD)
expect(ancestors).toContain(core.class.Tx)
expect(ancestors).toContain(core.class.Doc)
expect(ancestors).toContain(core.class.Obj)
// Verify order is consistent
const indexTx = ancestors.indexOf(core.class.Tx)
const indexDoc = ancestors.indexOf(core.class.Doc)
expect(indexDoc).toBeGreaterThan(indexTx)
})
it('isDerived should work with array-based ancestors', async () => {
const hierarchy = prepare()
// Test various inheritance chains
expect(hierarchy.isDerived(core.class.TxCreateDoc, core.class.Tx)).toBeTruthy()
expect(hierarchy.isDerived(core.class.TxCreateDoc, core.class.TxCUD)).toBeTruthy()
expect(hierarchy.isDerived(core.class.TxCreateDoc, core.class.Doc)).toBeTruthy()
expect(hierarchy.isDerived(core.class.TxCreateDoc, core.class.Obj)).toBeTruthy()
// Test self-derivation (class is in its own ancestors)
expect(hierarchy.isDerived(core.class.TxCreateDoc, core.class.TxCreateDoc)).toBeTruthy()
// Test non-derived classes
expect(hierarchy.isDerived(core.class.TxCreateDoc, core.class.Space)).toBeFalsy()
expect(hierarchy.isDerived(core.class.Space, core.class.Tx)).toBeFalsy()
// Test with mixins
expect(hierarchy.isDerived(test.class.TestComment, core.class.AttachedDoc)).toBeTruthy()
expect(hierarchy.isDerived(test.mixin.TaskMixinTodos, test.class.Task)).toBeTruthy()
})
it('should handle deep inheritance chains efficiently', async () => {
const hierarchy = prepare()
// TxCreateDoc has a chain: TxCreateDoc -> TxCUD -> Tx -> Doc -> Obj
const ancestors = hierarchy.getAncestors(core.class.TxCreateDoc)
expect(ancestors.length).toBeGreaterThanOrEqual(5)
// All intermediate classes should be present
expect(hierarchy.isDerived(core.class.TxCreateDoc, core.class.TxCUD)).toBeTruthy()
expect(hierarchy.isDerived(core.class.TxCUD, core.class.Tx)).toBeTruthy()
expect(hierarchy.isDerived(core.class.Tx, core.class.Doc)).toBeTruthy()
expect(hierarchy.isDerived(core.class.Doc, core.class.Obj)).toBeTruthy()
})
// Classifier properties tests - Map-based storage
it('getClassifierProp and setClassifierProp should work with Map', async () => {
const hierarchy = prepare()
// Set a property
hierarchy.setClassifierProp(core.class.Space, 'testProp', 'testValue')
// Get the property
const value = hierarchy.getClassifierProp(core.class.Space, 'testProp')
expect(value).toBe('testValue')
// Update the property
hierarchy.setClassifierProp(core.class.Space, 'testProp', 'updatedValue')
const updatedValue = hierarchy.getClassifierProp(core.class.Space, 'testProp')
expect(updatedValue).toBe('updatedValue')
})
it('should handle multiple properties per classifier', async () => {
const hierarchy = prepare()
// Set multiple properties
hierarchy.setClassifierProp(core.class.Space, 'prop1', 'value1')
hierarchy.setClassifierProp(core.class.Space, 'prop2', 'value2')
hierarchy.setClassifierProp(core.class.Space, 'prop3', 42)
hierarchy.setClassifierProp(core.class.Space, 'prop4', { nested: 'object' })
// Verify all properties are stored correctly
expect(hierarchy.getClassifierProp(core.class.Space, 'prop1')).toBe('value1')
expect(hierarchy.getClassifierProp(core.class.Space, 'prop2')).toBe('value2')
expect(hierarchy.getClassifierProp(core.class.Space, 'prop3')).toBe(42)
expect(hierarchy.getClassifierProp(core.class.Space, 'prop4')).toEqual({ nested: 'object' })
// Verify undefined for non-existent property
expect(hierarchy.getClassifierProp(core.class.Space, 'nonExistent')).toBeUndefined()
})
it('should isolate properties between different classifiers', async () => {
const hierarchy = prepare()
// Set properties on different classifiers
hierarchy.setClassifierProp(core.class.Space, 'name', 'Space')
hierarchy.setClassifierProp(core.class.Doc, 'name', 'Doc')
hierarchy.setClassifierProp(test.class.Task, 'name', 'Task')
// Verify isolation
expect(hierarchy.getClassifierProp(core.class.Space, 'name')).toBe('Space')
expect(hierarchy.getClassifierProp(core.class.Doc, 'name')).toBe('Doc')
expect(hierarchy.getClassifierProp(test.class.Task, 'name')).toBe('Task')
})
it('should handle property updates without creating new objects', async () => {
const hierarchy = prepare()
// Set initial value
hierarchy.setClassifierProp(core.class.Space, 'counter', 0)
// Update multiple times (testing that we're not creating new objects each time)
for (let i = 1; i <= 100; i++) {
hierarchy.setClassifierProp(core.class.Space, 'counter', i)
}
// Verify final value
expect(hierarchy.getClassifierProp(core.class.Space, 'counter')).toBe(100)
})
// Edge cases and integration tests
it('should handle interface implementation checks correctly', async () => {
const hierarchy = prepare()
// Task implements DummyWithState which extends WithState
expect(hierarchy.isImplements(test.class.Task, test.interface.WithState)).toBeTruthy()
expect(hierarchy.isImplements(test.class.Task, test.interface.DummyWithState)).toBeTruthy()
// TaskCheckItem directly implements WithState
expect(hierarchy.isImplements(test.class.TaskCheckItem, test.interface.WithState)).toBeTruthy()
// Negative cases
expect(hierarchy.isImplements(core.class.Space, test.interface.WithState)).toBeFalsy()
})
it('should maintain consistency after multiple hierarchy operations', async () => {
const hierarchy = prepare()
// Perform multiple operations
const ancestors1 = hierarchy.getAncestors(test.class.Task)
const isDerived1 = hierarchy.isDerived(test.class.Task, core.class.Doc)
// Set some properties
hierarchy.setClassifierProp(test.class.Task, 'test', 'value')
// Verify operations still work correctly
const ancestors2 = hierarchy.getAncestors(test.class.Task)
const isDerived2 = hierarchy.isDerived(test.class.Task, core.class.Doc)
expect(ancestors1).toEqual(ancestors2)
expect(isDerived1).toBe(isDerived2)
expect(isDerived2).toBeTruthy()
})
it('should handle getDescendants correctly', async () => {
const hierarchy = prepare()
// Get descendants of Doc (should include many classes)
const descendants = hierarchy.getDescendants(core.class.Doc)
expect(descendants).toContain(core.class.Space)
expect(descendants).toContain(core.class.Tx)
expect(descendants).toContain(test.class.Task)
expect(Array.isArray(descendants)).toBeTruthy()
})
it('should work with getBaseClass', async () => {
const hierarchy = prepare()
// Get base class of a mixin
const baseClass = hierarchy.getBaseClass(test.mixin.TaskMixinTodos)
expect(baseClass).toBe(test.class.Task)
// Get base class of a regular class (should return itself)
const baseClass2 = hierarchy.getBaseClass(test.class.Task)
expect(baseClass2).toBe(test.class.Task)
})
it('should handle getAllAttributes correctly', async () => {
const hierarchy = prepare()
// Get all attributes for a class
const attributes = hierarchy.getAllAttributes(core.class.TxCreateDoc)
// Should return a Map
expect(attributes instanceof Map).toBeTruthy()
// Test with to parameter
const attributesTo = hierarchy.getAllAttributes(core.class.TxCreateDoc, core.class.Tx)
expect(attributesTo instanceof Map).toBeTruthy()
})
it('should maintain immutability of returned ancestors array', async () => {
const hierarchy = prepare()
// Get ancestors
const ancestors = hierarchy.getAncestors(test.class.Task)
const originalLength = ancestors.length
// The returned array should be the internal array, but modifying it shouldn't break hierarchy
// (This is a trade-off for memory optimization - callers should treat it as read-only)
expect(ancestors.length).toBe(originalLength)
expect(ancestors).toContain(core.class.Doc)
})
it('should handle performance for multiple isDerived checks', async () => {
const hierarchy = prepare()
// Perform many isDerived checks to ensure array-based lookup is performant
const startTime = Date.now()
for (let i = 0; i < 1000; i++) {
hierarchy.isDerived(core.class.TxCreateDoc, core.class.Tx)
hierarchy.isDerived(test.class.Task, core.class.Doc)
hierarchy.isDerived(test.class.TaskCheckItem, core.class.AttachedDoc)
}
const endTime = Date.now()
// Should complete in reasonable time (< 100ms for 3000 checks)
expect(endTime - startTime).toBeLessThan(100)
})
})
+20 -24
View File
@@ -30,10 +30,10 @@ export class Hierarchy {
private readonly attributes = new Map<Ref<Classifier>, Map<string, AnyAttribute>>()
private readonly attributesById = new Map<Ref<AnyAttribute>, AnyAttribute>()
private readonly descendants = new Map<Ref<Classifier>, Ref<Classifier>[]>()
private readonly ancestors = new Map<Ref<Classifier>, Set<Ref<Classifier>>>()
private readonly ancestors = new Map<Ref<Classifier>, Ref<Classifier>[]>()
private readonly proxies = new Map<Ref<Mixin<Doc>>, ProxyHandler<Doc>>()
private readonly classifierProperties = new Map<Ref<Classifier>, Record<string, any>>()
private readonly classifierProperties = new Map<Ref<Classifier>, Map<string, any>>()
private createMixinProxyHandler (mixin: Ref<Mixin<Doc>>): ProxyHandler<Doc> {
const value = this.getClass(mixin)
@@ -172,7 +172,7 @@ export class Hierarchy {
if (result === undefined) {
throw new Error('ancestors not found: ' + _class)
}
return Array.from(result)
return result
}
getClass<T extends Obj = Obj>(_class: Ref<Class<T>>): Class<T> {
@@ -318,7 +318,7 @@ export class Hierarchy {
* It will iterate over parents.
*/
isDerived<T extends Obj>(_class: Ref<Class<T>>, from: Ref<Class<T>>): boolean {
return this.ancestors.get(_class)?.has(from) ?? false
return this.ancestors.get(_class)?.includes(from) ?? false
}
/**
@@ -408,29 +408,21 @@ export class Hierarchy {
private updateAncestors (_class: Ref<Classifier>, add = true): void {
const cl: Ref<Classifier>[] = [_class]
const visited = new Set<Ref<Classifier>>()
const ancestorList: Ref<Classifier>[] = []
while (cl.length > 0) {
const classifier = cl.shift() as Ref<Classifier>
if (addNew(visited, classifier)) {
const list = this.ancestors.get(_class)
if (list === undefined) {
if (add) {
this.ancestors.set(_class, new Set([classifier]))
}
} else {
if (add) {
if (!list.has(classifier)) {
list.add(classifier)
}
} else {
const pos = list.has(classifier)
if (pos) {
list.delete(classifier)
}
}
}
ancestorList.push(classifier)
cl.push(...this.ancestorsOf(classifier))
}
}
if (add) {
this.ancestors.set(_class, ancestorList)
} else {
this.ancestors.delete(_class)
}
}
/**
@@ -623,12 +615,16 @@ export class Hierarchy {
}
getClassifierProp (cl: Ref<Class<Doc>>, prop: string): any | undefined {
return this.classifierProperties.get(cl)?.[prop]
return this.classifierProperties.get(cl)?.get(prop)
}
setClassifierProp (cl: Ref<Class<Doc>>, prop: string, value: any): void {
const cur = this.classifierProperties.get(cl)
this.classifierProperties.set(cl, { ...cur, [prop]: value })
let cur = this.classifierProperties.get(cl)
if (cur === undefined) {
cur = new Map<string, any>()
this.classifierProperties.set(cl, cur)
}
cur.set(prop, value)
}
}