Improve hierarchy + tests

Add tests for hierarchy and few performance/memory  optimizations.

Author: haiodo

common/changes/@hcengineering/core/main_2025-10-08-09-02.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@hcengineering/core",
+      "comment": "Hierarchy improvements",
+      "type": "patch"
+    }
+  ],
+  "packageName": "@hcengineering/core"
+}

packages/core/src/__tests__/hierarchy.test.ts

@@ -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)
+  })
 })

packages/core/src/hierarchy.ts

@@ -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)
   }
 }