From 6611591c13ab7f2ddbed3992420f60a7625da01b Mon Sep 17 00:00:00 2001
From: tuxedoxt <48201871+tuxedoxt@users.noreply.github.com>
Date: Mon, 19 Feb 2024 18:57:30 +0100
Subject: [PATCH] Extend listener removal in BusHelper (#37)

* 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
---
 src/BusHelper.js       |  8 ++++++++
 src/Device.js          |  2 +-
 test/BusHelper.spec.js | 28 ++++++++++++++++++++++++++++
 test/Device.spec.js    |  1 +
 4 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/src/BusHelper.js b/src/BusHelper.js
index ae87ea6..f29da07 100644
--- a/src/BusHelper.js
+++ b/src/BusHelper.js
@@ -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()
diff --git a/src/Device.js b/src/Device.js
index 9142620..23154c6 100644
--- a/src/Device.js
+++ b/src/Device.js
@@ -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()
   }
 
   /**
diff --git a/test/BusHelper.spec.js b/test/BusHelper.spec.js
index 8e594f3..fbf0da9 100644
--- a/test/BusHelper.spec.js
+++ b/test/BusHelper.spec.js
@@ -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)
+})
diff --git a/test/Device.spec.js b/test/Device.spec.js
index 0cd342e..0679e03 100644
--- a/test/Device.spec.js
+++ b/test/Device.spec.js
@@ -13,6 +13,7 @@ jest.doMock('../src/BusHelper', () => {
       this.waitPropChange = jest.fn()
       this.children = jest.fn()
       this.callMethod = jest.fn()
+      this.removeListeners = jest.fn()
     }
   }
 })