From 4c003e9230e8bb45d79a51c7d1c08965cf20e73b Mon Sep 17 00:00:00 2001 From: Deepanshu Date: Tue, 18 Jul 2023 17:44:25 +0530 Subject: [PATCH] Fix force ping request logic (#72) --- .../java/com/gojek/courier/app/ui/MainActivity.kt | 2 +- .../eclipse/paho/client/mqttv3/MqttAsyncClient.java | 2 +- .../eclipse/paho/client/mqttv3/TimerPingSender.java | 2 +- .../paho/client/mqttv3/internal/ClientComms.java | 4 ++-- .../paho/client/mqttv3/internal/ClientState.java | 4 ++-- .../com/gojek/alarm/pingsender/AlarmPingSender.kt | 6 +----- .../com/gojek/timer/pingsender/TimerPingSender.kt | 6 +----- .../com/gojek/timer/pingsender/TimerPingSenderTest.kt | 11 +++++++---- .../workmanager/pingsender/WorkManagerPingSender.kt | 6 +----- .../pingsender/WorkManagerPingSenderTest.kt | 11 +++++++---- .../workmanager/pingsender/WorkManagerPingSender.kt | 6 +----- .../pingsender/WorkManagerPingSenderTest.kt | 11 +++++++---- 12 files changed, 32 insertions(+), 39 deletions(-) diff --git a/app/src/main/java/com/gojek/courier/app/ui/MainActivity.kt b/app/src/main/java/com/gojek/courier/app/ui/MainActivity.kt index 6ee9a0c4..68848d19 100644 --- a/app/src/main/java/com/gojek/courier/app/ui/MainActivity.kt +++ b/app/src/main/java/com/gojek/courier/app/ui/MainActivity.kt @@ -153,7 +153,7 @@ class MainActivity : AppCompatActivity() { incomingMessagesCleanupIntervalSecs = 10, maxInflightMessagesLimit = 1000, ), - pingSender = WorkPingSenderFactory.createMqttPingSender(applicationContext, WorkManagerPingSenderConfig()) + pingSender = WorkPingSenderFactory.createMqttPingSender(applicationContext, WorkManagerPingSenderConfig(sendForcePing = true)) ) mqttClient = MqttClientFactory.create(this, mqttConfig) mqttClient.addEventHandler(eventHandler) diff --git a/paho/src/main/java/org/eclipse/paho/client/mqttv3/MqttAsyncClient.java b/paho/src/main/java/org/eclipse/paho/client/mqttv3/MqttAsyncClient.java index 03cade6a..4fec6e40 100644 --- a/paho/src/main/java/org/eclipse/paho/client/mqttv3/MqttAsyncClient.java +++ b/paho/src/main/java/org/eclipse/paho/client/mqttv3/MqttAsyncClient.java @@ -872,7 +872,7 @@ public IMqttToken checkPing(Object userContext, IMqttActionListener callback) th // @TRACE 117=> logger.d(TAG, "checking for ping"); - token = comms.checkForActivity(); + token = comms.checkForActivity(false); // @TRACE 118=< return token; diff --git a/paho/src/main/java/org/eclipse/paho/client/mqttv3/TimerPingSender.java b/paho/src/main/java/org/eclipse/paho/client/mqttv3/TimerPingSender.java index 036693f8..8e58226c 100644 --- a/paho/src/main/java/org/eclipse/paho/client/mqttv3/TimerPingSender.java +++ b/paho/src/main/java/org/eclipse/paho/client/mqttv3/TimerPingSender.java @@ -86,7 +86,7 @@ public void run() { // @Trace 660=Check schedule at {0} logger.d(TAG, "in ping timer task run function"); - comms.checkForActivity(); + comms.checkForActivity(false); } } } diff --git a/paho/src/main/java/org/eclipse/paho/client/mqttv3/internal/ClientComms.java b/paho/src/main/java/org/eclipse/paho/client/mqttv3/internal/ClientComms.java index 8c5a3a05..4671c1d2 100644 --- a/paho/src/main/java/org/eclipse/paho/client/mqttv3/internal/ClientComms.java +++ b/paho/src/main/java/org/eclipse/paho/client/mqttv3/internal/ClientComms.java @@ -758,10 +758,10 @@ public void run() { /* * Check and send a ping if needed and check for ping timeout. Need to send a ping if nothing has been sent or received in the last keepalive interval. */ - public MqttToken checkForActivity() { + public MqttToken checkForActivity(Boolean forcePing) { MqttToken token = null; try { - token = clientState.checkForActivity(); + token = clientState.checkForActivity(forcePing); } catch (MqttException e) { handleRunException(e); } catch (Exception e) { diff --git a/paho/src/main/java/org/eclipse/paho/client/mqttv3/internal/ClientState.java b/paho/src/main/java/org/eclipse/paho/client/mqttv3/internal/ClientState.java index 3b48f81e..85f76d9f 100644 --- a/paho/src/main/java/org/eclipse/paho/client/mqttv3/internal/ClientState.java +++ b/paho/src/main/java/org/eclipse/paho/client/mqttv3/internal/ClientState.java @@ -649,7 +649,7 @@ protected void undo(MqttPublish message) throws MqttPersistenceException * * @return token of ping command, null if no ping command has been sent. */ - public MqttToken checkForActivity() throws MqttException + public MqttToken checkForActivity(Boolean forcePing) throws MqttException { final String methodName = "checkForActivity"; @@ -672,7 +672,7 @@ public MqttToken checkForActivity() throws MqttException long lastActivity = lastInboundActivity; // Is a ping required? - if (time - lastActivity + keepAliveMargin >= this.keepAlive) + if (forcePing || (time - lastActivity + keepAliveMargin >= this.keepAlive)) { // @TRACE 620=ping needed. keepAlive={0} lastOutboundActivity={1} lastInboundActivity={2} diff --git a/pingsender/alarm-pingsender/src/main/java/com/gojek/alarm/pingsender/AlarmPingSender.kt b/pingsender/alarm-pingsender/src/main/java/com/gojek/alarm/pingsender/AlarmPingSender.kt index 94c14824..86cedbd4 100644 --- a/pingsender/alarm-pingsender/src/main/java/com/gojek/alarm/pingsender/AlarmPingSender.kt +++ b/pingsender/alarm-pingsender/src/main/java/com/gojek/alarm/pingsender/AlarmPingSender.kt @@ -201,11 +201,7 @@ internal class AlarmPingSender( serverUri = comms.client.serverURI } pingSenderEvents.mqttPingInitiated(serverUri, comms.keepAlive.fromMillisToSeconds()) - val token = if (alarmPingSenderConfig.sendForcePing) { - comms.sendPingRequest() - } else { - comms.checkForActivity() - } + val token = comms.checkForActivity(alarmPingSenderConfig.sendForcePing) // No ping has been sent. if (token == null) { diff --git a/pingsender/timer-pingsender/src/main/java/com/gojek/timer/pingsender/TimerPingSender.kt b/pingsender/timer-pingsender/src/main/java/com/gojek/timer/pingsender/TimerPingSender.kt index 4eb41ab0..9a7faad5 100644 --- a/pingsender/timer-pingsender/src/main/java/com/gojek/timer/pingsender/TimerPingSender.kt +++ b/pingsender/timer-pingsender/src/main/java/com/gojek/timer/pingsender/TimerPingSender.kt @@ -72,11 +72,7 @@ internal class TimerPingSender( val serverUri = comms.client?.serverURI ?: "" val keepAliveMillis = comms.keepAlive pingSenderEvents.mqttPingInitiated(comms.client.serverURI, keepAliveMillis.fromMillisToSeconds()) - val token = if (pingSenderConfig.sendForcePing) { - comms.sendPingRequest() - } else { - comms.checkForActivity() - } + val token = comms.checkForActivity(pingSenderConfig.sendForcePing) if (token == null) { logger.d(TAG, "Mqtt Ping Token null") pingSenderEvents.pingMqttTokenNull(serverUri, keepAliveMillis.fromMillisToSeconds()) diff --git a/pingsender/timer-pingsender/src/test/java/com/gojek/timer/pingsender/TimerPingSenderTest.kt b/pingsender/timer-pingsender/src/test/java/com/gojek/timer/pingsender/TimerPingSenderTest.kt index e288e6c6..fb56b1aa 100644 --- a/pingsender/timer-pingsender/src/test/java/com/gojek/timer/pingsender/TimerPingSenderTest.kt +++ b/pingsender/timer-pingsender/src/test/java/com/gojek/timer/pingsender/TimerPingSenderTest.kt @@ -73,10 +73,11 @@ class TimerPingSenderTest { val mqttClient = mock() val testUri = "test-uri" val keepaliveMillis = 30000L + whenever(pingSenderConfig.sendForcePing).thenReturn(false) whenever(comms.client).thenReturn(mqttClient) whenever(mqttClient.serverURI).thenReturn(testUri) whenever(comms.keepAlive).thenReturn(keepaliveMillis) - whenever(comms.checkForActivity()).thenReturn(null) + whenever(comms.checkForActivity(false)).thenReturn(null) pingSender.PingTask().run() @@ -92,10 +93,11 @@ class TimerPingSenderTest { val keepaliveMillis = 30000L val startTime = TimeUnit.MILLISECONDS.toNanos(100) val endTime = TimeUnit.MILLISECONDS.toNanos(110) + whenever(pingSenderConfig.sendForcePing).thenReturn(false) whenever(comms.client).thenReturn(mqttClient) whenever(mqttClient.serverURI).thenReturn(testUri) whenever(comms.keepAlive).thenReturn(keepaliveMillis) - whenever(comms.checkForActivity()).thenReturn(mqttToken) + whenever(comms.checkForActivity(false)).thenReturn(mqttToken) whenever(clock.nanoTime()).thenReturn(startTime, endTime) pingSender.PingTask().run() @@ -120,7 +122,7 @@ class TimerPingSenderTest { whenever(comms.client).thenReturn(mqttClient) whenever(mqttClient.serverURI).thenReturn(testUri) whenever(comms.keepAlive).thenReturn(keepaliveMillis) - whenever(comms.sendPingRequest()).thenReturn(mqttToken) + whenever(comms.checkForActivity(true)).thenReturn(mqttToken) whenever(clock.nanoTime()).thenReturn(startTime, endTime) pingSender.PingTask().run() @@ -141,10 +143,11 @@ class TimerPingSenderTest { val keepaliveMillis = 30000L val startTime = TimeUnit.MILLISECONDS.toNanos(100) val endTime = TimeUnit.MILLISECONDS.toNanos(110) + whenever(pingSenderConfig.sendForcePing).thenReturn(false) whenever(comms.client).thenReturn(mqttClient) whenever(mqttClient.serverURI).thenReturn(testUri) whenever(comms.keepAlive).thenReturn(keepaliveMillis) - whenever(comms.checkForActivity()).thenReturn(mqttToken) + whenever(comms.checkForActivity(false)).thenReturn(mqttToken) whenever(clock.nanoTime()).thenReturn(startTime, endTime) pingSender.PingTask().run() diff --git a/pingsender/workmanager-2.6.0-pingsender/src/main/java/com/gojek/workmanager/pingsender/WorkManagerPingSender.kt b/pingsender/workmanager-2.6.0-pingsender/src/main/java/com/gojek/workmanager/pingsender/WorkManagerPingSender.kt index 55c9ba91..de88bf89 100644 --- a/pingsender/workmanager-2.6.0-pingsender/src/main/java/com/gojek/workmanager/pingsender/WorkManagerPingSender.kt +++ b/pingsender/workmanager-2.6.0-pingsender/src/main/java/com/gojek/workmanager/pingsender/WorkManagerPingSender.kt @@ -54,11 +54,7 @@ internal class WorkManagerPingSender( val keepAliveMillis = comms.keepAlive pingSenderEvents.mqttPingInitiated(serverUri, keepAliveMillis.fromMillisToSeconds()) - val token = if (pingSenderConfig.sendForcePing) { - comms.sendPingRequest() - } else { - comms.checkForActivity() - } + val token = comms.checkForActivity(pingSenderConfig.sendForcePing) if (token == null) { logger.d(TAG, "Mqtt Ping Token null") pingSenderEvents.pingMqttTokenNull(serverUri, keepAliveMillis.fromMillisToSeconds()) diff --git a/pingsender/workmanager-2.6.0-pingsender/src/test/java/com/gojek/workmanager/pingsender/WorkManagerPingSenderTest.kt b/pingsender/workmanager-2.6.0-pingsender/src/test/java/com/gojek/workmanager/pingsender/WorkManagerPingSenderTest.kt index 4fec4a60..1c20f064 100644 --- a/pingsender/workmanager-2.6.0-pingsender/src/test/java/com/gojek/workmanager/pingsender/WorkManagerPingSenderTest.kt +++ b/pingsender/workmanager-2.6.0-pingsender/src/test/java/com/gojek/workmanager/pingsender/WorkManagerPingSenderTest.kt @@ -61,10 +61,11 @@ class WorkManagerPingSenderTest { val mqttClient = mock() val testUri = "test-uri" val keepaliveMillis = 30000L + whenever(pingSenderConfig.sendForcePing).thenReturn(false) whenever(comms.client).thenReturn(mqttClient) whenever(mqttClient.serverURI).thenReturn(testUri) whenever(comms.keepAlive).thenReturn(keepaliveMillis) - whenever(comms.checkForActivity()).thenReturn(null) + whenever(comms.checkForActivity(false)).thenReturn(null) pingSender.sendPing { // do nothing @@ -82,10 +83,11 @@ class WorkManagerPingSenderTest { val keepaliveMillis = 30000L val startTime = TimeUnit.MILLISECONDS.toNanos(100) val endTime = TimeUnit.MILLISECONDS.toNanos(110) + whenever(pingSenderConfig.sendForcePing).thenReturn(false) whenever(comms.client).thenReturn(mqttClient) whenever(mqttClient.serverURI).thenReturn(testUri) whenever(comms.keepAlive).thenReturn(keepaliveMillis) - whenever(comms.checkForActivity()).thenReturn(mqttToken) + whenever(comms.checkForActivity(false)).thenReturn(mqttToken) whenever(clock.nanoTime()).thenReturn(startTime, endTime) var success: Boolean? = null @@ -114,7 +116,7 @@ class WorkManagerPingSenderTest { whenever(comms.client).thenReturn(mqttClient) whenever(mqttClient.serverURI).thenReturn(testUri) whenever(comms.keepAlive).thenReturn(keepaliveMillis) - whenever(comms.sendPingRequest()).thenReturn(mqttToken) + whenever(comms.checkForActivity(true)).thenReturn(mqttToken) whenever(clock.nanoTime()).thenReturn(startTime, endTime) var success: Boolean? = null @@ -139,10 +141,11 @@ class WorkManagerPingSenderTest { val keepaliveMillis = 30000L val startTime = TimeUnit.MILLISECONDS.toNanos(100) val endTime = TimeUnit.MILLISECONDS.toNanos(110) + whenever(pingSenderConfig.sendForcePing).thenReturn(false) whenever(comms.client).thenReturn(mqttClient) whenever(mqttClient.serverURI).thenReturn(testUri) whenever(comms.keepAlive).thenReturn(keepaliveMillis) - whenever(comms.checkForActivity()).thenReturn(mqttToken) + whenever(comms.checkForActivity(false)).thenReturn(mqttToken) whenever(clock.nanoTime()).thenReturn(startTime, endTime) var success: Boolean? = null diff --git a/pingsender/workmanager-pingsender/src/main/java/com/gojek/workmanager/pingsender/WorkManagerPingSender.kt b/pingsender/workmanager-pingsender/src/main/java/com/gojek/workmanager/pingsender/WorkManagerPingSender.kt index 55c9ba91..de88bf89 100644 --- a/pingsender/workmanager-pingsender/src/main/java/com/gojek/workmanager/pingsender/WorkManagerPingSender.kt +++ b/pingsender/workmanager-pingsender/src/main/java/com/gojek/workmanager/pingsender/WorkManagerPingSender.kt @@ -54,11 +54,7 @@ internal class WorkManagerPingSender( val keepAliveMillis = comms.keepAlive pingSenderEvents.mqttPingInitiated(serverUri, keepAliveMillis.fromMillisToSeconds()) - val token = if (pingSenderConfig.sendForcePing) { - comms.sendPingRequest() - } else { - comms.checkForActivity() - } + val token = comms.checkForActivity(pingSenderConfig.sendForcePing) if (token == null) { logger.d(TAG, "Mqtt Ping Token null") pingSenderEvents.pingMqttTokenNull(serverUri, keepAliveMillis.fromMillisToSeconds()) diff --git a/pingsender/workmanager-pingsender/src/test/java/com/gojek/workmanager/pingsender/WorkManagerPingSenderTest.kt b/pingsender/workmanager-pingsender/src/test/java/com/gojek/workmanager/pingsender/WorkManagerPingSenderTest.kt index 4fec4a60..1c20f064 100644 --- a/pingsender/workmanager-pingsender/src/test/java/com/gojek/workmanager/pingsender/WorkManagerPingSenderTest.kt +++ b/pingsender/workmanager-pingsender/src/test/java/com/gojek/workmanager/pingsender/WorkManagerPingSenderTest.kt @@ -61,10 +61,11 @@ class WorkManagerPingSenderTest { val mqttClient = mock() val testUri = "test-uri" val keepaliveMillis = 30000L + whenever(pingSenderConfig.sendForcePing).thenReturn(false) whenever(comms.client).thenReturn(mqttClient) whenever(mqttClient.serverURI).thenReturn(testUri) whenever(comms.keepAlive).thenReturn(keepaliveMillis) - whenever(comms.checkForActivity()).thenReturn(null) + whenever(comms.checkForActivity(false)).thenReturn(null) pingSender.sendPing { // do nothing @@ -82,10 +83,11 @@ class WorkManagerPingSenderTest { val keepaliveMillis = 30000L val startTime = TimeUnit.MILLISECONDS.toNanos(100) val endTime = TimeUnit.MILLISECONDS.toNanos(110) + whenever(pingSenderConfig.sendForcePing).thenReturn(false) whenever(comms.client).thenReturn(mqttClient) whenever(mqttClient.serverURI).thenReturn(testUri) whenever(comms.keepAlive).thenReturn(keepaliveMillis) - whenever(comms.checkForActivity()).thenReturn(mqttToken) + whenever(comms.checkForActivity(false)).thenReturn(mqttToken) whenever(clock.nanoTime()).thenReturn(startTime, endTime) var success: Boolean? = null @@ -114,7 +116,7 @@ class WorkManagerPingSenderTest { whenever(comms.client).thenReturn(mqttClient) whenever(mqttClient.serverURI).thenReturn(testUri) whenever(comms.keepAlive).thenReturn(keepaliveMillis) - whenever(comms.sendPingRequest()).thenReturn(mqttToken) + whenever(comms.checkForActivity(true)).thenReturn(mqttToken) whenever(clock.nanoTime()).thenReturn(startTime, endTime) var success: Boolean? = null @@ -139,10 +141,11 @@ class WorkManagerPingSenderTest { val keepaliveMillis = 30000L val startTime = TimeUnit.MILLISECONDS.toNanos(100) val endTime = TimeUnit.MILLISECONDS.toNanos(110) + whenever(pingSenderConfig.sendForcePing).thenReturn(false) whenever(comms.client).thenReturn(mqttClient) whenever(mqttClient.serverURI).thenReturn(testUri) whenever(comms.keepAlive).thenReturn(keepaliveMillis) - whenever(comms.checkForActivity()).thenReturn(mqttToken) + whenever(comms.checkForActivity(false)).thenReturn(mqttToken) whenever(clock.nanoTime()).thenReturn(startTime, endTime) var success: Boolean? = null