Skip to content

Commit

Permalink
Extend listener removal in BusHelper (#37)
Browse files Browse the repository at this point in the history
* Extend listener removal in BusHelper

- Adds a clean-up function `removeListeners` that in addition to
  the already existing PropertiesChanged removal on itself
  (in `Device.disconnect`) also removes PropertiesChanged
  event listeners on the _propsProxy object.
- Adds use of this clean-up function in `Device.disconnect`.

* Fix re-use after listener removal in BusHelper

* Add test cases for BusHelper.removeListeners
  • Loading branch information
tuxedoxt authored Feb 19, 2024
1 parent 425958d commit 6611591
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 1 deletion.
8 changes: 8 additions & 0 deletions src/BusHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ class BusHelper extends EventEmitter {
return this._ifaceProxy[methodName](...args)
}

removeListeners () {
this.removeAllListeners('PropertiesChanged')
if (this._propsProxy !== null) {
this._propsProxy.removeAllListeners('PropertiesChanged')
this._ready = false
}
}

static buildChildren (path, nodes) {
if (path === '/') path = ''
const children = new Set()
Expand Down
2 changes: 1 addition & 1 deletion src/Device.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class Device extends EventEmitter {
*/
async disconnect () {
await this.helper.callMethod('Disconnect')
this.helper.removeAllListeners('PropertiesChanged') // might be improved
this.helper.removeListeners()
}

/**
Expand Down
28 changes: 28 additions & 0 deletions test/BusHelper.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,31 @@ test('propsEvents', async () => {
await helper.set('VirtualProperty', buildTypedValue('string', 'bar'))
await expect(res).resolves.toMatchObject({ VirtualProperty: { signature: 's', value: 'bar' } })
})

test('removeListeners', async () => {
const helper = new BusHelper(dbus, TEST_NAME, TEST_OBJECT, TEST_IFACE, { useProps: true, usePropsEvents: true })

const dummyCb = () => {}

// Init with listener on helper (directly attached dummyCb) and _propsProxy (through method call triggered _prepare)
helper.on('PropertiesChanged', dummyCb)
await helper.callMethod('Echo', 'ping')
expect(helper.listenerCount('PropertiesChanged')).toBeGreaterThan(0)
expect(helper._propsProxy.listenerCount('PropertiesChanged')).toBeGreaterThan(0)

// Test remove
helper.removeListeners()
expect(helper.listenerCount('PropertiesChanged')).toBe(0)
expect(helper._propsProxy.listenerCount('PropertiesChanged')).toBe(0)

// Test reuse after remove (same initialization as before)
helper.on('PropertiesChanged', dummyCb)
await helper.callMethod('Echo', 'ping')
expect(helper.listenerCount('PropertiesChanged')).toBeGreaterThan(0)
expect(helper._propsProxy.listenerCount('PropertiesChanged')).toBeGreaterThan(0)

// Remove second time
helper.removeListeners()
expect(helper.listenerCount('PropertiesChanged')).toBe(0)
expect(helper._propsProxy.listenerCount('PropertiesChanged')).toBe(0)
})
1 change: 1 addition & 0 deletions test/Device.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ jest.doMock('../src/BusHelper', () => {
this.waitPropChange = jest.fn()
this.children = jest.fn()
this.callMethod = jest.fn()
this.removeListeners = jest.fn()
}
}
})
Expand Down

0 comments on commit 6611591

Please sign in to comment.