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

RTN22 #537

Merged
merged 4 commits into from
Dec 14, 2016
Merged

RTN22 #537

Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion Source/ARTRealtime+Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ ART_ASSUME_NONNULL_BEGIN
@interface ARTRealtime ()

@property (readwrite, strong, nonatomic) ARTRest *rest;
@property (readonly, getter=getTransport) id<ARTRealtimeTransport> transport;
@property (readonly, getter=getTransport, art_nullable) id<ARTRealtimeTransport> transport;
@property (readonly, strong, nonatomic, art_nonnull) id<ARTReachability> reachability;
@property (readonly, getter=getLogger) ARTLog *logger;

Expand Down
12 changes: 5 additions & 7 deletions Source/ARTRealtime.m
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ - (void)transitionSideEffects:(ARTConnectionStateChange *)stateChange {
}
_transport = [[_transportClass alloc] initWithRest:self.rest options:self.options resumeKey:resumeKey connectionSerial:connectionSerial];
_transport.delegate = self;
[self transportConnect];
[self transportConnectForcingNewToken:_renewingToken];
}

if (self.connection.state != ARTRealtimeFailed && self.connection.state != ARTRealtimeClosed && self.connection.state != ARTRealtimeDisconnected) {
Expand Down Expand Up @@ -530,9 +530,10 @@ - (void)onDisconnected:(ARTProtocolMessage *)message {
[self.logger info:@"R:%p ARTRealtime disconnected", self];
ARTErrorInfo *error = message.error;
if ([self shouldRenewToken:&error]) {
[self transportReconnectWithRenewedToken];
[self transition:ARTRealtimeDisconnected withErrorInfo:error];
[self.connection setErrorReason:nil];
_renewingToken = true;
[self transition:ARTRealtimeConnecting withErrorInfo:nil];
return;
}
[self transition:ARTRealtimeDisconnected withErrorInfo:error];
Expand All @@ -558,6 +559,7 @@ - (void)onAuth {
switch (self.connection.state) {
case ARTRealtimeConnecting:
case ARTRealtimeConnected:
_resuming = true;
[self transportReconnectWithRenewedToken];
break;
default:
Expand Down Expand Up @@ -612,18 +614,14 @@ - (BOOL)shouldRenewToken:(ARTErrorInfo **)errorPtr {

- (void)transportReconnectWithHost:(NSString *)host {
[self.transport setHost:host];
[self transportConnect];
[self transportConnectForcingNewToken:false];
}

- (void)transportReconnectWithRenewedToken {
_renewingToken = true;
[self transportConnectForcingNewToken:true];
}

- (void)transportConnect {
[self transportConnectForcingNewToken:false];
}

- (void)transportConnectForcingNewToken:(BOOL)forceNewToken {
ARTClientOptions *options = [self.options copy];
if ([options isBasicAuth]) {
Expand Down
12 changes: 5 additions & 7 deletions Spec/Auth.swift
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ class Auth : QuickSpec {
// Inject AUTH
let authMessage = ARTProtocolMessage()
authMessage.action = ARTProtocolMessageAction.Auth
realtime.transport.receive(authMessage)
realtime.transport?.receive(authMessage)

expect(realtime.connection.errorReason).toEventuallyNot(beNil(), timeout: testTimeout)
guard let errorInfo = realtime.connection.errorReason else {
Expand Down Expand Up @@ -482,7 +482,7 @@ class Auth : QuickSpec {
// Inject AUTH
let authMessage = ARTProtocolMessage()
authMessage.action = ARTProtocolMessageAction.Auth
realtime.transport.receive(authMessage)
realtime.transport?.receive(authMessage)

expect(realtime.connection.errorReason).toEventuallyNot(beNil(), timeout: testTimeout)
guard let errorInfo = realtime.connection.errorReason else {
Expand Down Expand Up @@ -573,7 +573,7 @@ class Auth : QuickSpec {
// Inject AUTH
let authMessage = ARTProtocolMessage()
authMessage.action = ARTProtocolMessageAction.Auth
realtime.transport.receive(authMessage)
realtime.transport?.receive(authMessage)

expect(realtime.connection.errorReason).toEventuallyNot(beNil(), timeout: testTimeout)
guard let errorInfo = realtime.connection.errorReason else {
Expand Down Expand Up @@ -656,7 +656,7 @@ class Auth : QuickSpec {
// Inject AUTH
let authMessage = ARTProtocolMessage()
authMessage.action = ARTProtocolMessageAction.Auth
realtime.transport.receive(authMessage)
realtime.transport?.receive(authMessage)

expect(realtime.connection.errorReason).toEventuallyNot(beNil(), timeout: testTimeout)
guard let errorInfo = realtime.connection.errorReason else {
Expand Down Expand Up @@ -714,9 +714,7 @@ class Auth : QuickSpec {
options.autoConnect = false

let client = ARTRealtime(options: options)
defer {
client.close()
}
defer { client.dispose(); client.close() }
client.setTransportClass(TestProxyTransport.self)
client.connect()

Expand Down
3 changes: 1 addition & 2 deletions Spec/RealtimeClientChannel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2430,8 +2430,7 @@ class RealtimeClientChannel: QuickSpec {
protoMsg.action = .Detach
protoMsg.error = ARTErrorInfo.createWithCode(123, message: "test error")
protoMsg.channel = "test"

client.realtimeTransport(client.transport, didReceiveMessage: protoMsg)
client.transport?.receive(protoMsg)

expect(channel.state).to(equal(ARTRealtimeChannelState.Detached))
expect(channel.errorReason).to(equal(protoMsg.error))
Expand Down
137 changes: 133 additions & 4 deletions Spec/RealtimeClientConnection.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2359,7 +2359,6 @@ class RealtimeClientConnection: QuickSpec {
}

waitUntil(timeout: testTimeout) { done in
// Wait for token to expire
client.connection.once(.Connected) { stateChange in
expect(stateChange?.reason).to(beNil())
done()
Expand Down Expand Up @@ -3401,6 +3400,137 @@ class RealtimeClientConnection: QuickSpec {
}
}

// RTN22
it("Ably can request that a connected client re-authenticates by sending the client an AUTH ProtocolMessage") {
let options = AblyTests.commonAppSetup()
options.useTokenAuth = true
let client = ARTRealtime(options: options)
defer { client.dispose(); client.close() }
let channel = client.channels.get("foo")

waitUntil(timeout: testTimeout) { done in
channel.attach { error in
expect(error).to(beNil())
done()
}
}

guard let initialConnectionId = client.connection.id else {
fail("ConnectionId is nil"); return
}

guard let initialToken = client.auth.tokenDetails?.token else {
fail("Initial token is nil"); return
}

waitUntil(timeout: testTimeout) { done in
client.connection.once(.Connected) { stateChange in
Copy link
Contributor

Choose a reason for hiding this comment

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

This is racy. The expectation above can pass after this event is fired. The event listener should be set up before. In fact, you can just remove the expectation above and check that the AUTH protocol message was received inside the CONNECTED event listener.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you're right. Will fix.

expect(stateChange?.reason).to(beNil())
expect(initialToken).toNot(equal(client.auth.tokenDetails?.token))
Copy link
Member

Choose a reason for hiding this comment

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

Please check the connection was resumed successfully

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

done()
}

let authMessage = ARTProtocolMessage()
authMessage.action = .Auth
client.transport?.receive(authMessage)
}

expect(client.connection.id).to(equal(initialConnectionId))

waitUntil(timeout: testTimeout) { done in
let partialDone = AblyTests.splitDone(2, done: done)
let expectedMessage = ARTMessage(name: "ios", data: "message1")

channel.subscribe() { message in
expect(message.name).to(equal(expectedMessage.name))
expect(message.data as? String).to(equal(expectedMessage.data as? String))
partialDone()
}

let rest = ARTRest(options: AblyTests.clientOptions(key: options.key!))
rest.channels.get("foo").publish([expectedMessage]) { error in
expect(error).to(beNil())
partialDone()
}
}

channel.off()
}

// RTN22a
it("re-authenticate and resume the connection when the client is forcibly disconnected following a DISCONNECTED message containing an error code in the range 40140 <= code < 40150") {
let options = AblyTests.commonAppSetup()
options.token = getTestToken(key: options.key!, ttl: 5.0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Five seconds isn't a bit too much?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As above.

let client = ARTRealtime(options: options)
defer { client.dispose(); client.close() }
let channel = client.channels.get("foo")

waitUntil(timeout: testTimeout) { done in
channel.attach { error in
expect(error).to(beNil())
done()
}
}

guard let initialConnectionId = client.connection.id else {
fail("ConnectionId is nil"); return
}

guard let initialToken = client.auth.tokenDetails?.token else {
fail("Initial token is nil"); return
}

channel.once(.Detached) { _ in
fail("Should not detach channels")
}

var authorizeMethodCallCount = 0
let hook = client.auth.testSuite_injectIntoMethodAfter(#selector(client.auth.authorize)) {
authorizeMethodCallCount += 1
}
defer { hook.remove() }

waitUntil(timeout: testTimeout) { done in
client.connection.once(.Disconnected) { stateChange in
guard let error = stateChange?.reason else {
fail("Error is nil"); done(); return
}
expect(error.code) == 40142
done()
}
}

waitUntil(timeout: testTimeout) { done in
client.connection.once(.Connected) { stateChange in
expect(stateChange?.reason).to(beNil())
expect(initialToken).toNot(equal(client.auth.tokenDetails?.token))
done()
}
}

expect(client.connection.id).to(equal(initialConnectionId))
expect(authorizeMethodCallCount) == 1

waitUntil(timeout: testTimeout) { done in
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this whole block checking?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This block is checking if the channel was resumed successfully (recommended by @mattheworiordan).

test that uses a low value TTL for the token, attaches to a channel, then waits for the auth to complete renewing the token, and then publish over REST to that channel to make sure it's received?

Copy link
Member

Choose a reason for hiding this comment

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

It's just a fail safe to make sure that the reauth is in fact working and there is continuity over the channels

let partialDone = AblyTests.splitDone(2, done: done)
let expectedMessage = ARTMessage(name: "ios", data: "message1")

channel.subscribe() { message in
expect(message.name).to(equal(expectedMessage.name))
expect(message.data as? String).to(equal(expectedMessage.data as? String))
partialDone()
}

let rest = ARTRest(options: AblyTests.clientOptions(key: options.key!))
rest.channels.get("foo").publish([expectedMessage]) { error in
expect(error).to(beNil())
partialDone()
}
}

channel.off()
}

}

// https://github.com/ably/ably-ios/issues/454
Expand All @@ -3417,8 +3547,7 @@ class RealtimeClientConnection: QuickSpec {
let protoMsg = ARTProtocolMessage()
protoMsg.action = .Disconnect
protoMsg.error = ARTErrorInfo.createWithCode(123, message: "test error")

client.realtimeTransport(client.transport, didReceiveMessage: protoMsg)
client.transport?.receive(protoMsg)

expect(client.connection.state).to(equal(ARTRealtimeConnectionState.Disconnected))
expect(client.connection.errorReason).to(equal(protoMsg.error))
Expand Down Expand Up @@ -3651,7 +3780,7 @@ class RealtimeClientConnection: QuickSpec {

let authMessage = ARTProtocolMessage()
authMessage.action = .Auth
client.transport.receive(authMessage)
client.transport?.receive(authMessage)

client.close()

Expand Down