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