From de0be44d61a5ea07c4b862855dd7542bc767ff4c Mon Sep 17 00:00:00 2001 From: Mint de Wit Date: Fri, 20 Sep 2024 10:39:24 +0200 Subject: [PATCH] chore: better handling of empty nodes --- src/Ember/Client/__tests__/index.spec.ts | 152 +++++++++++++++++++++-- src/Ember/Client/index.ts | 24 +++- src/Ember/Lib/util.ts | 29 +++++ 3 files changed, 193 insertions(+), 12 deletions(-) diff --git a/src/Ember/Client/__tests__/index.spec.ts b/src/Ember/Client/__tests__/index.spec.ts index 02ad8b2..acc15f2 100644 --- a/src/Ember/Client/__tests__/index.spec.ts +++ b/src/Ember/Client/__tests__/index.spec.ts @@ -84,7 +84,7 @@ describe('client', () => { function createQualifiedNodeResponse( path: string, content: EmberElement, - children: Collection> + children: Collection> | undefined ): DecodeResult { const parent = new QualifiedElementImpl(path, content, children) @@ -95,8 +95,10 @@ describe('client', () => { fixLevel(child, node) } } - for (const child of Object.values>(children)) { - fixLevel(child, parent as any as NumberedTreeNode) + if (children) { + for (const child of Object.values>(children)) { + fixLevel(child, parent as any as NumberedTreeNode) + } } return { value: { @@ -169,11 +171,22 @@ describe('client', () => { await new Promise(setImmediate) + // lookup on the parameter + expect(onSocketWrite).toHaveBeenCalledTimes(3) + socket.mockData({ + value: { + 1: new QualifiedElementImpl( + '1.1.1', + new ParameterImpl(ParameterType.Boolean, 'On', undefined, false) + ) as Exclude>, + }, + }) + + await new Promise(setImmediate) + const res = await getByPathPromise expect(res).toBeTruthy() - expect(res).toMatchObject( - new NumberedTreeNodeImpl(1, new ParameterImpl(ParameterType.Boolean, 'On', undefined, false)) - ) + expect(res?.contents).toMatchObject(new ParameterImpl(ParameterType.Boolean, 'On', undefined, false)) }) }) @@ -229,15 +242,34 @@ describe('client', () => { await new Promise(setImmediate) - // Final lookup + // Final tree node expect(onSocketWrite).toHaveBeenCalledTimes(6) socket.mockData( createQualifiedNodeResponse('1.1.1', new EmberNodeImpl('MAIN', undefined, undefined, false), { 1: new NumberedTreeNodeImpl(1, new ParameterImpl(ParameterType.Boolean, 'On', undefined, false)), - 2: new NumberedTreeNodeImpl(1, new ParameterImpl(ParameterType.Boolean, 'Second', undefined, false)), + 2: new NumberedTreeNodeImpl(2, new ParameterImpl(ParameterType.Boolean, 'Second', undefined, false)), }) ) + await new Promise(setImmediate) + + // last call to the parameters just in case + expect(onSocketWrite).toHaveBeenCalledTimes(8) + socket.mockData( + createQualifiedNodeResponse( + '1.1.1.1', + new ParameterImpl(ParameterType.Boolean, 'On', undefined, false), + undefined + ) + ) + socket.mockData( + createQualifiedNodeResponse( + '1.1.1.2', + new ParameterImpl(ParameterType.Boolean, 'Second', undefined, false), + undefined + ) + ) + // Both completed successfully const res = await getByPathPromise expect(res).toBeTruthy() @@ -246,4 +278,108 @@ describe('client', () => { expect(res2).toBeTruthy() }) }) + + it('getElementByPath empty node in the root', async () => { + await runWithConnection(async (client, socket) => { + // Do initial load + const getRootDirReq = await client.getDirectory(client.tree) + getRootDirReq.response?.catch(() => null) // Ensure uncaught response is ok + expect(onSocketWrite).toHaveBeenCalledTimes(1) + onSocketWrite.mockClear() + + // Mock a valid response + socket.mockData({ + value: { + 1: new NumberedTreeNodeImpl(1, new EmberNodeImpl('Ruby', undefined, undefined, true)), + }, + }) + await getRootDirReq.response + + // Request the empty node + const req = await client.getDirectory(client.tree[1]) + + // Returns empty node + expect(onSocketWrite).toHaveBeenCalledTimes(1) + socket.mockData({ + value: { + 1: new NumberedTreeNodeImpl(1, new EmberNodeImpl()), + }, + }) + + await new Promise(setImmediate) + + const res = await req.response + expect(res).toBeTruthy() + }) + }) + + it('getElementByPath empty node in the tree', async () => { + await runWithConnection(async (client, socket) => { + // Do initial load + const getRootDirReq = await client.getDirectory(client.tree) + getRootDirReq.response?.catch(() => null) // Ensure uncaught response is ok + expect(onSocketWrite).toHaveBeenCalledTimes(1) + onSocketWrite.mockClear() + + // Mock a valid response + socket.mockData({ + value: { + 1: new NumberedTreeNodeImpl(1, new EmberNodeImpl('Ruby', undefined, undefined, true)), + }, + }) + await getRootDirReq.response + + // Run the tree + const getByPathPromise = client.getElementByPath('Ruby.Sums.Empty') + + // First lookup + expect(onSocketWrite).toHaveBeenCalledTimes(1) + socket.mockData({ + value: { + 1: new NumberedTreeNodeImpl(1, new EmberNodeImpl('Ruby', undefined, undefined, true), { + 1: new NumberedTreeNodeImpl(1, new EmberNodeImpl('Sums', undefined, undefined, true)), + }), + }, + }) + + await new Promise(setImmediate) + + // Second lookup + expect(onSocketWrite).toHaveBeenCalledTimes(2) + socket.mockData({ + value: { + 1: new QualifiedElementImpl('1.1', new EmberNodeImpl('Sums', undefined, undefined, false), { + 1: new NumberedTreeNodeImpl(1, new EmberNodeImpl('Empty', undefined, undefined, true)), + }) as Exclude>, + }, + }) + + await new Promise(setImmediate) + + const getByPathRes = await getByPathPromise + expect(getByPathRes).toBeTruthy() + + const node = client.tree[1].children?.[1].children?.[1] + if (!node) throw new Error('Empty res') // really just a typeguard + + // Request the empty node + const req = await client.getDirectory(node) + + // lookup on the empty node + expect(onSocketWrite).toHaveBeenCalledTimes(3) + socket.mockData({ + value: { + 1: new QualifiedElementImpl('1.1.1', new EmberNodeImpl()) as Exclude< + RootElement, + NumberedTreeNode + >, + }, + }) + + await new Promise(setImmediate) + + const res = await req.response + expect(res).toBeTruthy() + }) + }) }) diff --git a/src/Ember/Client/index.ts b/src/Ember/Client/index.ts index 34536d2..460475f 100644 --- a/src/Ember/Client/index.ts +++ b/src/Ember/Client/index.ts @@ -29,7 +29,7 @@ import { Connection, ConnectionDisposition, ConnectionOperation } from '../../mo import { EmberNode } from '../../model/EmberNode' import { EventEmitter } from 'eventemitter3' import { S101Client } from '../Socket' -import { getPath, assertQualifiedEmberNode, insertCommand, updateProps } from '../Lib/util' +import { getPath, assertQualifiedEmberNode, insertCommand, updateProps, isEmptyNode } from '../Lib/util' import { berEncode } from '../..' import { NumberedTreeNodeImpl } from '../../model/Tree' import { EmberFunction } from '../../model/EmberFunction' @@ -70,6 +70,7 @@ export interface Subscription { export interface Change { path: string | undefined node: RootElement + emptyNode?: boolean } export enum ConnectionStatus { @@ -209,7 +210,7 @@ export class EmberClient extends EventEmitter { cb, }) - return this._sendCommand(node, command, ExpectResponse.Any) + return this._sendCommand(node, command, ExpectResponse.HasChildren) } async subscribe( node: RootElement | Array, @@ -513,7 +514,15 @@ export class EmberClient extends EventEmitter { ) for (const req of reqs) { // Don't complete the response, if the call was expecting the children to be loaded - if (req.nodeResponse === ExpectResponse.HasChildren && !change.node.children) continue + if (req.nodeResponse === ExpectResponse.HasChildren && !change.node.children) { + if (change.node.contents.type === ElementType.Parameter) { + // can't have children, therefore don't continue + } else if (change.emptyNode) { + // update comes from an empty node, so we can't continue anyway + } else { + continue + } + } if (req.cb) req.cb(change.node) if (req.resolve) { @@ -603,6 +612,9 @@ export class EmberClient extends EventEmitter { this.tree[rootElement.number] = rootElement changes.push({ path: undefined, node: rootElement }) } + } else if (isEmptyNode(rootElement)) { + // empty node on the root of the tree must mean we have done a getDir on that specific node + changes.push({ path: rootElement.number + '', node: rootElement, emptyNode: true }) } else { // this must have been something on the root of the tree (like GetDirectory) this.tree[rootElement.number] = rootElement @@ -619,7 +631,8 @@ export class EmberClient extends EventEmitter { const changes: Array = [] if (update.contents.type === tree.contents.type) { - changes.push({ path: getPath(tree), node: tree }) + changes.push({ path: getPath(tree), node: tree, emptyNode: isEmptyNode(update) }) + // changes.push({ path: getPath(tree), node: tree }) switch (tree.contents.type) { case ElementType.Node: this._updateEmberNode(update.contents as EmberNode, tree.contents) @@ -642,6 +655,9 @@ export class EmberClient extends EventEmitter { } else if (update.children) { changes.push({ path: getPath(tree), node: tree }) tree.children = update.children + for (const c of Object.values>(update.children)) { + c.parent = tree + } } return changes diff --git a/src/Ember/Lib/util.ts b/src/Ember/Lib/util.ts index a2233c7..ba571e6 100644 --- a/src/Ember/Lib/util.ts +++ b/src/Ember/Lib/util.ts @@ -2,6 +2,7 @@ import { QualifiedElement, NumberedTreeNode, RootElement } from '../../types/typ import { EmberElement, ElementType } from '../../model/EmberElement' import { Command } from '../../model/Command' import { QualifiedElementImpl, NumberedTreeNodeImpl, TreeElement } from '../../model/Tree' +import { EmberNode } from '../../model' export function assertQualifiedEmberNode(node: RootElement): Exclude> { if ('path' in node) { @@ -76,3 +77,31 @@ export function normalizeError(e: unknown): Error { return new Error(typeof e === 'string' ? e : (e as any)?.toString()) } + +export function isEmptyNode(node: TreeElement): boolean { + const isNode = (node: TreeElement): node is TreeElement => { + return node.contents.type === ElementType.Node + } + + if (!isNode(node)) { + return false + } + + if (node.children) { + return false + } + + if ( + node.contents.description ?? + node.contents.identifier ?? + node.contents.isOnline ?? + node.contents.isRoot ?? + node.contents.schemaIdentifiers ?? + node.contents.templateReference + ) { + return false + } + + // node is a node, node has no children, node has no properties set => node must be empty + return true +}