From 8ad48e04d6453b3824a7a5b8af28ba340dd97d1e Mon Sep 17 00:00:00 2001 From: Creasor Date: Sun, 9 Feb 2025 16:06:21 -0500 Subject: [PATCH] Added tests to verify removeListener failure due to SAM autoboxing Added two tests demonstrating that passing lambdas directly to MessageClient.addListener and CapabilityClient.addListener results in removeListener failing to remove them due to SAM autoboxing. This commit also includes temporary test dependencies required to run these tests, which should be removed before merging. --- core/connectivity/data/build.gradle.kts | 5 + .../connectivity/data/WearNodeDiscovery.kt | 3 +- .../data/messaging/WearMessagingClient.kt | 6 +- .../data/WearNodeDiscoveryTest.kt | 92 +++++++++++++++++++ .../messaging/WearMessageClientUnitTest.kt | 91 ++++++++++++++++++ 5 files changed, 192 insertions(+), 5 deletions(-) create mode 100644 core/connectivity/data/src/test/java/com/plcoding/core/connectivity/data/WearNodeDiscoveryTest.kt create mode 100644 core/connectivity/data/src/test/java/com/plcoding/core/connectivity/data/messaging/WearMessageClientUnitTest.kt diff --git a/core/connectivity/data/build.gradle.kts b/core/connectivity/data/build.gradle.kts index 720edec3..1ca6fe7b 100644 --- a/core/connectivity/data/build.gradle.kts +++ b/core/connectivity/data/build.gradle.kts @@ -16,4 +16,9 @@ dependencies { implementation(projects.core.domain) implementation(projects.core.connectivity.domain) + + // Temporary tests for 2 removeListener() bugs. + testImplementation("org.jetbrains.kotlinx:kotlinx-coroutines-test:1.7.3") + testImplementation("io.mockk:mockk:1.13.5") + testImplementation("junit:junit:4.13.2") } \ No newline at end of file diff --git a/core/connectivity/data/src/main/java/com/plcoding/core/connectivity/data/WearNodeDiscovery.kt b/core/connectivity/data/src/main/java/com/plcoding/core/connectivity/data/WearNodeDiscovery.kt index 18c8584b..1990b284 100644 --- a/core/connectivity/data/src/main/java/com/plcoding/core/connectivity/data/WearNodeDiscovery.kt +++ b/core/connectivity/data/src/main/java/com/plcoding/core/connectivity/data/WearNodeDiscovery.kt @@ -3,7 +3,6 @@ package com.plcoding.core.connectivity.data import android.content.Context import com.google.android.gms.common.api.ApiException import com.google.android.gms.wearable.CapabilityClient -import com.google.android.gms.wearable.CapabilityInfo import com.google.android.gms.wearable.Wearable import com.plcoding.core.connectivity.domain.DeviceNode import com.plcoding.core.connectivity.domain.DeviceType @@ -36,7 +35,7 @@ class WearNodeDiscovery( return@callbackFlow } - val listener: (CapabilityInfo) -> Unit = { + val listener = CapabilityClient.OnCapabilityChangedListener { trySend(it.nodes.map { it.toDeviceNode() }.toSet()) } capabilityClient.addListener(listener, remoteCapability) diff --git a/core/connectivity/data/src/main/java/com/plcoding/core/connectivity/data/messaging/WearMessagingClient.kt b/core/connectivity/data/src/main/java/com/plcoding/core/connectivity/data/messaging/WearMessagingClient.kt index 60367b79..804205d0 100644 --- a/core/connectivity/data/src/main/java/com/plcoding/core/connectivity/data/messaging/WearMessagingClient.kt +++ b/core/connectivity/data/src/main/java/com/plcoding/core/connectivity/data/messaging/WearMessagingClient.kt @@ -2,7 +2,7 @@ package com.plcoding.core.connectivity.data.messaging import android.content.Context import com.google.android.gms.common.api.ApiException -import com.google.android.gms.wearable.MessageEvent +import com.google.android.gms.wearable.MessageClient import com.google.android.gms.wearable.Wearable import com.plcoding.core.connectivity.domain.messaging.MessagingAction import com.plcoding.core.connectivity.domain.messaging.MessagingClient @@ -29,8 +29,8 @@ class WearMessagingClient( connectedNodeId = nodeId return callbackFlow { - val listener: (MessageEvent) -> Unit = { event -> - if(event.path.startsWith(BASE_PATH_MESSAGING_ACTION)) { + val listener = MessageClient.OnMessageReceivedListener { event -> + if (event.path.startsWith(BASE_PATH_MESSAGING_ACTION)) { val json = event.data.decodeToString() val action = Json.decodeFromString(json) trySend(action.toMessagingAction()) diff --git a/core/connectivity/data/src/test/java/com/plcoding/core/connectivity/data/WearNodeDiscoveryTest.kt b/core/connectivity/data/src/test/java/com/plcoding/core/connectivity/data/WearNodeDiscoveryTest.kt new file mode 100644 index 00000000..bafd61a2 --- /dev/null +++ b/core/connectivity/data/src/test/java/com/plcoding/core/connectivity/data/WearNodeDiscoveryTest.kt @@ -0,0 +1,92 @@ +package com.plcoding.core.connectivity.data + +import com.google.android.gms.tasks.Tasks +import com.google.android.gms.wearable.CapabilityClient +import com.google.android.gms.wearable.CapabilityInfo +import com.plcoding.core.connectivity.domain.DeviceNode +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test + +class WearNodeDiscoveryTest { + + private lateinit var capabilityClient: CapabilityClient + private val listeners = mutableSetOf() + + @Before + fun setup() { + capabilityClient = mockk(relaxed = true) + + every { capabilityClient.addListener(any(), any()) } answers { + val listener = firstArg() + listeners.add(listener) + Tasks.forResult(null) + } + + every { capabilityClient.removeListener(any()) } answers { + val listener = firstArg() + listeners.remove(listener) + Tasks.forResult(null) + } + } + + @Test + fun `removeListener fails when using lambda`() = runBlocking { + val receivedUpdates = mutableListOf>() + + // The listener is a Kotlin function type (CapabilityInfo) -> Unit, + // NOT an instance of CapabilityClient.OnCapabilityChangedListener. + val listenerLambda: (CapabilityInfo) -> Unit = { info -> + receivedUpdates.add(info.nodes.map { it.toDeviceNode() }.toSet()) + } + + // When passing listenerLambda to addListener(), + // Kotlin will box it into an anonymous class. + capabilityClient.addListener(listenerLambda, "runique_wear_app") + + // When calling removeListener(), it expects the exact same instance. + // However, since listenerLambda was boxed into a new anonymous class, + // removeListener does NOT remove the original listener. + capabilityClient.removeListener(listenerLambda) + + // Simulate an event - the listener should have been removed, + // but it still gets invoked. + val fakeCapabilityInfo = mockk(relaxed = true) + listeners.forEach { it.onCapabilityChanged(fakeCapabilityInfo) } + + assertEquals( + "Listener should have been removed but was still triggered.", + 0, + receivedUpdates.size + ) + } + + @Test + fun `removeListener succeeds when using explicit listener`() = runBlocking { + val receivedUpdates = mutableListOf>() + + // The listener is explicitly declared as an instance + // of OnCapabilityChangedListener. + val listener = CapabilityClient.OnCapabilityChangedListener { info -> + receivedUpdates.add(info.nodes.map { it.toDeviceNode() }.toSet()) + } + + // This ensures the exact same instance is used for + // both addListener and removeListener. + capabilityClient.addListener(listener, "runique_wear_app") + capabilityClient.removeListener(listener) + + // Simulate an event - the listener should NOT be triggered. + val fakeCapabilityInfo = mockk(relaxed = true) + listeners.forEach { it.onCapabilityChanged(fakeCapabilityInfo) } + + assertEquals( + "Listener was correctly removed and did not receive updates.", + 0, + receivedUpdates.size + ) + } +} diff --git a/core/connectivity/data/src/test/java/com/plcoding/core/connectivity/data/messaging/WearMessageClientUnitTest.kt b/core/connectivity/data/src/test/java/com/plcoding/core/connectivity/data/messaging/WearMessageClientUnitTest.kt new file mode 100644 index 00000000..64d831ec --- /dev/null +++ b/core/connectivity/data/src/test/java/com/plcoding/core/connectivity/data/messaging/WearMessageClientUnitTest.kt @@ -0,0 +1,91 @@ +package com.plcoding.core.connectivity.data.messaging + +import com.google.android.gms.tasks.Tasks +import com.google.android.gms.wearable.MessageClient +import com.google.android.gms.wearable.MessageEvent +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import kotlin.collections.forEach + +class WearMessageClientListenerTest { + + private lateinit var messageClient: MessageClient + private val listeners = mutableSetOf() + + @Before + fun setup() { + messageClient = mockk(relaxed = true) + + every { messageClient.addListener(any()) } answers { + val listener = firstArg() + listeners.add(listener) + Tasks.forResult(null) + } + + every { messageClient.removeListener(any()) } answers { + val listener = firstArg() + listeners.remove(listener) + Tasks.forResult(null) + } + } + + /** + * This test demonstrates a bug in the removeListener() method. + */ + @Test + fun `removeListener fails when using lambda`() = runBlocking { + val receivedMessages = mutableListOf() + + // The listener is a Kotlin function type (MessageEvent) -> Unit, + // NOT an instance of MessageClient.OnMessageReceivedListener. + val listenerLambda: (MessageEvent) -> Unit = { receivedMessages.add(it) } + + // When passing listenerLambda to addListener(), + // Kotlin will box it into an anonymous class. + messageClient.addListener(listenerLambda) + + // When calling removeListener(), it expects the exact same instance. + // However, since listenerLambda was boxed into a new anonymous class, + // removeListener does NOT remove the original listener. + messageClient.removeListener(listenerLambda) + + // Simulate an event - the listener should have been removed, + // but it still gets invoked. + val fakeEvent = mockk(relaxed = true) + listeners.forEach { it.onMessageReceived(fakeEvent) } + + assertEquals( + "Listener should have been removed but was still triggered.", + 0, + receivedMessages.size + ) + } + + /** + * This test demonstrates the correct way to remove a listener. + */ + @Test + fun `removeListener succeeds when using explicit listener`() = runBlocking { + val receivedMessages = mutableListOf() + + // Correct way to create a listener using the expected interface + val listener = MessageClient.OnMessageReceivedListener { receivedMessages.add(it) } + + // Add the listener + messageClient.addListener(listener) + + // Remove it properly + messageClient.removeListener(listener) + + // Simulate an event - should NOT be received + val fakeEvent = mockk(relaxed = true) + listeners.forEach { it.onMessageReceived(fakeEvent) } + + // This test passes this check + assertEquals("Listener should have been removed!", 0, receivedMessages.size) + } +} \ No newline at end of file