Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add connection freshness check #390

Merged
merged 26 commits into from
May 18, 2018
Merged
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
664c5e6
Add connection freshness check
funkyboy May 9, 2018
41b28a1
Test connectionId after ttl + idle interval has passed
funkyboy May 9, 2018
2b15846
Fix reconnection test after ttl
funkyboy May 9, 2018
dbeb79a
Test reconnection before ttl + idleInterval has passed
funkyboy May 9, 2018
aa24aaa
Fix detection of connection freshness
funkyboy May 9, 2018
ec4cb43
Improve connection state tests
funkyboy May 10, 2018
b16bdec
Add test for RTN15g3
funkyboy May 10, 2018
c75d55e
Fix implementation fo connection freshness
funkyboy May 10, 2018
cc38a40
Refactor stale connection check
funkyboy May 14, 2018
9efc78a
Fix test when reconnecting and connection is stale
funkyboy May 14, 2018
6a2c3e4
Fix indentation
funkyboy May 15, 2018
0a1c622
Add log message when clearing stale connection
funkyboy May 15, 2018
6892ecc
Improve after TTL test
funkyboy May 15, 2018
0a14d26
Explicitly wait for channel to attach in reconnection test
funkyboy May 15, 2018
77f3cf8
Explicitly wait for close in reconnection test
funkyboy May 15, 2018
b1b9085
Set lastActivity from SocketTransport
funkyboy May 15, 2018
3a868ee
Improve reattach test
funkyboy May 15, 2018
09d76b2
Fix suspendAll() (used for RTN15c3 and RTN15g3) to ensure previously …
paddybyers May 16, 2018
79d490b
Add test for RTN15c3
paddybyers May 16, 2018
f94e877
Merge branch 'add-rtn15c-test' into add-connection-freshness-check
funkyboy May 16, 2018
6ce56a8
Ongoing tests
funkyboy May 16, 2018
056edff
Implement RTN15g3: when suppressing resume due to a stale connection,…
paddybyers May 17, 2018
6ab31f7
Clean up log statements and clarify comments
funkyboy May 17, 2018
0521e14
Add suspended channels to RTN15g3 test
funkyboy May 17, 2018
3e41787
To kick a Travis build
funkyboy May 17, 2018
b681cd8
Use disconnectAndSuppressRetries to pause connectionManager in RTN15g…
funkyboy May 18, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import static org.junit.Assert.fail;

import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.InvocationTargetException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

Expand All @@ -30,6 +32,7 @@
import io.ably.lib.test.common.Helpers;
import io.ably.lib.test.common.ParameterizedTest;
import io.ably.lib.test.common.Helpers.ConnectionWaiter;
import io.ably.lib.test.common.Helpers.ChannelWaiter;
import io.ably.lib.transport.ConnectionManager;
import io.ably.lib.transport.Defaults;
import io.ably.lib.types.AblyException;
Expand Down Expand Up @@ -562,37 +565,49 @@ public void onConnectionStateChanged(ConnectionStateChange state) {

/* Attach to channel */
final Channel channel = ably.channels.get(channelName);
channel.on(new ChannelStateListener() {
@Override
public void onChannelStateChanged(ChannelStateChange stateChange) {
System.out.println("======================= state is " + stateChange.current);
}
});
channel.once(ChannelState.attached, new ChannelStateListener() {
@Override
public void onChannelStateChanged(ChannelStateChange stateChange) {
System.out.println("Channel attached for the first time");
assertEquals("Channel is attached", stateChange.current, ChannelState.attached);
}
});

/* attach and wait for the channel to be attached */
channel.attach();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're not explicitly awaiting the channel attached, and you cannot be sure the onChannelStateChanged handler ran with this code path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(new Helpers.ChannelWaiter(channel)).waitFor(ChannelState.attached);
ChannelWaiter channelWaiter = new Helpers.ChannelWaiter(channel);
channelWaiter.waitFor(ChannelState.attached);

ably.connection.once(ConnectionEvent.disconnected, new ConnectionStateListener() {
@Override
public void onConnectionStateChanged(ConnectionStateChange state) {
synchronized (ably.connection.connectionManager) {
try {
/* The client will try to reconnect right away after disconnection.
* We want it to stay into a disconnected state long enough
* so that the connection becomes stale.
*/
ably.connection.connectionManager.wait(waitInDisconnectedState);
} catch (InterruptedException e) {
e.printStackTrace();
}
}
}
});
/* suppress automatic retries by the connection manager */
try {
Method method = ably.connection.connectionManager.getClass().getDeclaredMethod("disconnectAndSuppressRetries");
method.setAccessible(true);
method.invoke(ably.connection.connectionManager);
} catch (NoSuchMethodException|IllegalAccessException|InvocationTargetException e) {
fail("Unexpected exception in suppressing retries");
}

ably.connection.connectionManager.requestState(ConnectionState.disconnected);
connectionWaiter.waitFor(ConnectionState.disconnected);
assertEquals("Disconnected state was not reached", ConnectionState.disconnected, ably.connection.state);
System.out.println("=============================== DISCONNECTED ");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are these for lines for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That were meant for debugging but have been deleted in this commit 6ab31f7#diff-4cfebe2518c34c9f3975cde19f3ced08L598


try { Thread.sleep(waitInDisconnectedState); } catch(InterruptedException e) {}

channel.on(ChannelEvent.update, new ChannelStateListener() {
ably.connection.connect();
connectionWaiter.waitFor(ConnectionState.connected);

/* verify a new connection was assigned */
assertNotEquals("A new connection was created", firstConnectionId, ably.connection.id);
System.out.println("=============================== connection is new " + firstConnectionId + " " + ably.connection.id);
System.out.println("channel state is " + channel.state);

channel.once(ChannelEvent.attached, new ChannelStateListener() {
@Override
public void onChannelStateChanged(ChannelStateChange stateChange) {
System.out.println("Channel reattached after a new connection has been established");
Expand All @@ -603,7 +618,10 @@ public void onChannelStateChanged(ChannelStateChange stateChange) {
ably.close();
}
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are not waiting for those to close, this entire section could easily never be run as part of this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(new Helpers.ChannelWaiter(channel)).waitFor(ChannelState.attached);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will you please sort out your IDE/editor? It's getting tiring spending time commenting on whitespace I am afraid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this is an old commit and is now fixed.

/* */
(new Helpers.ConnectionWaiter(ably.connection)).waitFor(ConnectionState.closed);

} catch (AblyException e) {
e.printStackTrace();
fail("init0: Unexpected exception instantiating library");
Expand Down