From 0e2c409aeddb4f130c8934079bdc058a21412104 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 17 May 2023 12:11:03 +0100 Subject: [PATCH 1/4] test: fix flakey JWT re-auth test The test would sometimes fail because we'd end up with two re-auths instead of one due to the token expiring in the middle of the test. This change fixes the issue, by only caring about the first token regeneration. --- .../lib/test/realtime/RealtimeJWTTest.java | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/lib/src/test/java/io/ably/lib/test/realtime/RealtimeJWTTest.java b/lib/src/test/java/io/ably/lib/test/realtime/RealtimeJWTTest.java index 8e7409710..1e648aa58 100644 --- a/lib/src/test/java/io/ably/lib/test/realtime/RealtimeJWTTest.java +++ b/lib/src/test/java/io/ably/lib/test/realtime/RealtimeJWTTest.java @@ -305,7 +305,9 @@ public Object getTokenRequest(TokenParams params) throws AblyException { @Override public Object handleResponse(HttpCore.Response response, ErrorInfo error) throws AblyException { try { - callbackCalled.add(true); + synchronized (tokens) { + callbackCalled.add(true); + } resultToken[0] = new String(response.body, "UTF-8"); } catch (UnsupportedEncodingException e) { e.printStackTrace(); @@ -331,11 +333,14 @@ public void onRawConnect(String url) { } public void onRawMessageSend(ProtocolMessage message) { } @Override public void onRawMessageRecv(ProtocolMessage message) { - if (message.action == ProtocolMessage.Action.auth) { - authMessages[0] = true; + synchronized (tokens) { + if (message.action == ProtocolMessage.Action.auth) { + authMessages[0] = true; + } } } }; + final AblyRealtime ablyRealtime = new AblyRealtime(options); /* Once connected for the first time capture the assigned token and @@ -343,9 +348,9 @@ public void onRawMessageRecv(ProtocolMessage message) { ablyRealtime.connection.once(ConnectionEvent.connected, new ConnectionStateListener() { @Override public void onConnectionStateChanged(ConnectionStateChange stateChange) { - assertTrue("Callback not called the first time", callbackCalled.get(0)); - assertEquals("State is not connected", ConnectionState.connected, stateChange.current); synchronized (tokens) { + assertTrue("Callback not called the first time", callbackCalled.get(0)); + assertEquals("State is not connected", ConnectionState.connected, stateChange.current); tokens[0] = ablyRealtime.auth.getTokenDetails().token; } } @@ -365,12 +370,13 @@ public void onConnectionStateChanged(ConnectionStateChange stateChange) { ablyRealtime.connection.on(ConnectionEvent.update, new ConnectionStateListener() { @Override public void onConnectionStateChanged(ConnectionStateChange state) { - assertTrue("Callback not called the second time", callbackCalled.get(1)); - assertEquals("Callback not called 2 times", callbackCalled.size(), 2); - assertNotEquals("Token should not be the same", tokens[0], ablyRealtime.auth.getTokenDetails().token); - assertTrue("Auth protocol message has not been received", authMessages[0]); - updateEvents[0] = true; - ablyRealtime.close(); + synchronized (tokens) { + assertTrue("Callback not called the second time", callbackCalled.get(1)); + assertNotEquals("Token should not be the same", ablyRealtime.auth.getTokenDetails().token, tokens[0]); + assertTrue("Auth protocol message has not been received", authMessages[0]); + updateEvents[0] = true; + ablyRealtime.close(); + } } }); From 8e69dee3df5f7b211b32c9a7a47cf59b7ef1971b Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 17 May 2023 14:16:35 +0100 Subject: [PATCH 2/4] test: fix flakey reject_invalid_message_data test The test was failing as it was setting the log handler and level, which is static and therefore global. This logger is shared between tests that are not running in isolation, so other tests could write to the log, causing the log assertion to fail. This change fixes the fails by removing the log assertions and instead asserting on the state of the message post-encoding. Fixes #946 --- .../test/realtime/RealtimeMessageTest.java | 20 ++----------------- 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/lib/src/test/java/io/ably/lib/test/realtime/RealtimeMessageTest.java b/lib/src/test/java/io/ably/lib/test/realtime/RealtimeMessageTest.java index cf137057b..c161ce93e 100644 --- a/lib/src/test/java/io/ably/lib/test/realtime/RealtimeMessageTest.java +++ b/lib/src/test/java/io/ably/lib/test/realtime/RealtimeMessageTest.java @@ -796,34 +796,18 @@ static class MessagesEncodingDataItem { } @Test + public void reject_invalid_message_data() throws AblyException { HashMap data = new HashMap(); Message message = new Message("event", data); - Log.LogHandler originalLogHandler = Log.handler; - int originalLogLevel = Log.level; - Log.setLevel(Log.DEBUG); - final ArrayList capturedLog = new ArrayList<>(); - Log.setHandler(new Log.LogHandler() { - @Override - public void println(int severity, String tag, String msg, Throwable tr) { - capturedLog.add(new LogLine(severity, tag, msg, tr)); - } - }); - try { message.encode(null); + fail("reject_invalid_message_data: Expected AblyException to be thrown."); } catch(AblyException e) { assertEquals(null, message.encoding); assertEquals(data, message.data); - assertEquals(1, capturedLog.size()); - LogLine capturedLine = capturedLog.get(0); - assertTrue(capturedLine.tag.contains("ably")); - assertTrue(capturedLine.msg.contains("Message data must be either `byte[]`, `String` or `JSONElement`; implicit coercion of other types to String is deprecated")); } catch(Throwable t) { fail("reject_invalid_message_data: Unexpected exception"); - } finally { - Log.setHandler(originalLogHandler); - Log.setLevel(originalLogLevel); } } From ab2083f5f54067ebe249347a7aff88941a3bba38 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 17 May 2023 15:03:29 +0100 Subject: [PATCH 3/4] test: fix resend pending messages flakey test The test uses a sleep to ensure that the TTLs pass and that the connection is marked as stale. However, it does this whilst the channel is still active, and websocket ping/pongs could still be sent, therefore its possible that on the channel reconnecting, a resume will take place. This change fixes the bug by performing the wait when the channel has been disconnected (with retries suppressed) which guarantees that the connection will be stale when the test reconnects. Fixes #942 --- .../lib/test/realtime/RealtimeResumeTest.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/src/test/java/io/ably/lib/test/realtime/RealtimeResumeTest.java b/lib/src/test/java/io/ably/lib/test/realtime/RealtimeResumeTest.java index d606bb4c0..356dcf210 100644 --- a/lib/src/test/java/io/ably/lib/test/realtime/RealtimeResumeTest.java +++ b/lib/src/test/java/io/ably/lib/test/realtime/RealtimeResumeTest.java @@ -962,7 +962,7 @@ public void resume_publish_reenter_when_resume_failed() throws AblyException { options.realtimeRequestTimeout = 2000L; /* We want this greater than newTtl + newIdleInterval */ - final long waitInDisconnectedState = 3000L; + final long waitInDisconnectedState = 5000L; options.transportFactory = mockWebsocketFactory; try(AblyRealtime ably = new AblyRealtime(options)) { final long newTtl = 1000L; @@ -1017,12 +1017,6 @@ public void onConnectionStateChanged(ConnectionStateChange state) { mockWebsocketFactory.blockReceiveProcessing(message -> message.action == ProtocolMessage.Action.ack || message.action == ProtocolMessage.Action.nack); - /* Wait for the connection to go stale, then reconnect */ - try { - Thread.sleep(waitInDisconnectedState); - } catch (InterruptedException e) { - } - //enter next 3 clients for (int i = 0; i < 3; i++) { senderChannel.presence.enterClient(clients[i+3],null,presenceCompletion.add()); @@ -1045,6 +1039,13 @@ public void onConnectionStateChanged(ConnectionStateChange state) { for (int i = 0; i < 3; i++) { senderChannel.presence.enterClient(clients[i+6],null,presenceCompletion.add()); } + + /* Wait for the connection to go stale, then reconnect */ + try { + Thread.sleep(waitInDisconnectedState); + } catch (InterruptedException e) { + } + //now let's unblock the ack nacks and reconnect mockWebsocketFactory.blockReceiveProcessing(message -> false); /* Wait for the connection to go stale, then reconnect */ From 888c143466594ecb22efcbfa26105db61abd0d6e Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 17 May 2023 17:24:04 +0100 Subject: [PATCH 4/4] test: fix range comparison in test The comparison in the test breaks if the number is exactly on the lower bound. This change fixes it by making the check inclusive. Fixes #948 --- .../io/ably/lib/test/realtime/RealtimeConnectFailTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/test/java/io/ably/lib/test/realtime/RealtimeConnectFailTest.java b/lib/src/test/java/io/ably/lib/test/realtime/RealtimeConnectFailTest.java index cff0bf0aa..2fa5da2f7 100644 --- a/lib/src/test/java/io/ably/lib/test/realtime/RealtimeConnectFailTest.java +++ b/lib/src/test/java/io/ably/lib/test/realtime/RealtimeConnectFailTest.java @@ -615,9 +615,9 @@ public void onConnectionStateChanged(ConnectionStateChange state) { System.out.println("higher range: " + higherRange + " - lower range: " + lowerRange + " | checked value: " + retryTime); assertTrue("retry time higher range for count " + i + " is not in valid: " + retryTime + " expected: " + higherRange, - retryTime < higherRange); + retryTime <= higherRange); assertTrue("retry time lower range for count " + i + " is not in valid: " + retryTime + " expected: " + lowerRange, - retryTime > lowerRange); + retryTime >= lowerRange); } System.out.println("------------------------------------------------------------"); } catch (AblyException e) {