chore: better handling of empty nodes

src/Ember/Client/__tests__/index.spec.ts

@@ -84,7 +84,7 @@ describe('client', () => {
 	function createQualifiedNodeResponse(
 		path: string,
 		content: EmberElement,
-		children: Collection<NumberedTreeNode<EmberElement>>
+		children: Collection<NumberedTreeNode<EmberElement>> | undefined
 	): DecodeResult<Root> {
 		const parent = new QualifiedElementImpl<EmberElement>(path, content, children)
 
@@ -95,8 +95,10 @@ describe('client', () => {
 				fixLevel(child, node)
 			}
 		}
-		for (const child of Object.values<NumberedTreeNode<EmberElement>>(children)) {
-			fixLevel(child, parent as any as NumberedTreeNode<EmberElement>)
+		if (children) {
+			for (const child of Object.values<NumberedTreeNode<EmberElement>>(children)) {
+				fixLevel(child, parent as any as NumberedTreeNode<EmberElement>)
+			}
 		}
 		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<EmberElement>(
+						'1.1.1',
+						new ParameterImpl(ParameterType.Boolean, 'On', undefined, false)
+					) as Exclude<RootElement, NumberedTreeNode<EmberElement>>,
+				},
+			})
+
+			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<EmberElement>('1.1', new EmberNodeImpl('Sums', undefined, undefined, false), {
+						1: new NumberedTreeNodeImpl(1, new EmberNodeImpl('Empty', undefined, undefined, true)),
+					}) as Exclude<RootElement, NumberedTreeNode<EmberElement>>,
+				},
+			})
+
+			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<EmberElement>('1.1.1', new EmberNodeImpl()) as Exclude<
+						RootElement,
+						NumberedTreeNode<EmberElement>
+					>,
+				},
+			})
+
+			await new Promise(setImmediate)
+
+			const res = await req.response
+			expect(res).toBeTruthy()
+		})
+	})
 })

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<EmberClientEvents> {
 				cb,
 			})
 
-		return this._sendCommand<RootElement>(node, command, ExpectResponse.Any)
+		return this._sendCommand<RootElement>(node, command, ExpectResponse.HasChildren)
 	}
 	async subscribe(
 		node: RootElement | Array<RootElement>,
@@ -513,7 +514,15 @@ export class EmberClient extends EventEmitter<EmberClientEvents> {
 			)
 			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<EmberClientEvents> {
 							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<EmberClientEvents> {
 		const changes: Array<Change> = []
 
 		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<EmberClientEvents> {
 		} else if (update.children) {
 			changes.push({ path: getPath(tree), node: tree })
 			tree.children = update.children
+			for (const c of Object.values<NumberedTreeNode<EmberElement>>(update.children)) {
+				c.parent = tree
+			}
 		}
 
 		return changes

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<RootElement, NumberedTreeNode<EmberElement>> {
 	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<EmberElement>): boolean {
+	const isNode = (node: TreeElement<EmberElement>): node is TreeElement<EmberNode> => {
+		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
+}