Update RTL2 for 0.9 #543

ricardopereira · 2016-11-28T15:28:15Z

No description provided.

mattheworiordan

@ricardopereira this looks good overall, I'm just a bit confused why you decided to change ARTRealtimeChannelAttached to ARTChannelAttached given it seems all other constants use ARTRealtime*. Personally I think Realtime is unnecessary as I assume ART stands for Ably Realtime anyway? We should perhaps discuss ASAP on Slack with others?

mattheworiordan · 2016-12-02T14:31:17Z

Source/ARTDataQuery.m

@@ -54,7 +54,7 @@ - (NSMutableArray *)asQueryItems {
    NSMutableArray *items = [super asQueryItems];
    if (self.untilAttach) {
        NSAssert(self.realtimeChannel, @"ARTRealtimeHistoryQuery used from outside ARTRealtimeChannel.history");
-        if (self.realtimeChannel.state != ARTRealtimeChannelAttached) {
+        if (self.realtimeChannel.state != ARTChannelStateAttached) {


If this has changed to ARTChannelStateAttached from ARTRealtimeChannelAttached is breaking. If you think that is necessary then please state your case. However, if you do this, then there are lots of other changes that need doing I reckon, even consider the line above which is ARTRealtimeChannel.history. Then would ARTRealtimeClosed becomes ARTConnectionClosed for example. Why did you change it, what's the thinking?

On a separate note, this change seems quite big (changing the naming everywhere) yet you have no comments in the commit as to why you did this? Use Git to tell a story if you can.

Well, I changed it because I stated from the IDL that there's the ChannelType and the ChannelState and I wanted to stay consistent with that. It seems wrong to me having an ARTChannelEvent and an ARTRelatimeChanneState.

mattheworiordan · 2016-12-02T14:37:22Z

Spec/RealtimeClientChannel.swift

+                            guard let stateChange = stateChange else {
+                                fail("ChannelStageChange is nil"); done(); return
+                            }
+                            expect(stateChange.resumed).to(beTrue())


Surprised this would pass as not sure this is supported in realtime yet? Does this test pass?

I changed it to pending on a latest commit.

mattheworiordan · 2016-12-02T14:40:08Z

Source/ARTProtocolMessage.m

+            return @"Presence"; //14
+        case ARTProtocolMessageMessage:
+            return @"Message"; //15
+        case ARTProtocolMessageSync:


Is it not worth adding Auth here now so that we don't forget, or at least a TODO?

mattheworiordan · 2016-12-02T14:40:26Z

Spec/RealtimeClientChannel.swift

@@ -284,7 +284,7 @@ class RealtimeClientChannel: QuickSpec {
                }

                // RTL2f
-                it("ChannelStateChange will contain a resumed boolean attribute with value @true@ if the bit flag RESUMED was included") {
+                pending("ChannelStateChange will contain a resumed boolean attribute with value @true@ if the bit flag RESUMED was included") {


Ah, this answers my previous question :)

mattheworiordan · 2016-12-02T16:17:34Z

Has this been updated?

ricardopereira · 2016-12-03T00:52:06Z

Not yet, unfortunately.

tcard

FWIW, I'm in favor of the rename, and of @mattheworiordan 's suggestion of renaming ARTRealtime* to ARTConnectionState* to be consistent. (Although yes, it'd be nice if the rename were been done at the end, for ease of review.)

tcard · 2016-12-05T18:49:07Z

Source/ARTRealtimeChannel.m

    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, 0), dispatch_get_main_queue(), ^{
        if (state != self.state) {
            // Already changed; do nothing.
            return;
        }
-        [self timed:[self once:^(ARTErrorInfo *errorInfo) {
+        // FIXME: should not use the global listener for internal purpose


Can you raise an issue for this so it's not forgotten?

tcard · 2016-12-05T19:00:25Z

Source/ARTTypes.h

 typedef NS_ENUM(NSUInteger, ARTChannelEvent) {
    ARTChannelEventInitialized,
    ARTChannelEventAttaching,
    ARTChannelEventAttached,
    ARTChannelEventDetaching,
    ARTChannelEventDetached,
+    ARTChannelEventSuspended,
    ARTChannelEventFailed,
    ARTChannelEventError


Error should be Updated now?

@ricardopereira this is not addressed still?

ricardopereira · 2016-12-13T10:06:17Z

Rollback: I removed all the changes related with ARTRealtimeChannelState rename.

mattheworiordan · 2016-12-13T22:08:56Z

Source/ARTTypes.m

+        case ARTChannelEventFailed:
+            return @"Failed"; //6
+        case ARTChannelEventError:
+            return @"Error"; //7


As above, this is now update no, or will you deal with this in a separate PR?

mattheworiordan · 2016-12-13T22:09:52Z

One comment, but perhaps intentional until a later PR, otherwise LGTM

- functionality hasn't been deployed

ricardopereira · 2016-12-19T14:33:48Z

@mattheworiordan @tcard Please review all those commits. Sorry but I had to rebase everything.

ricardopereira · 2016-12-20T00:09:19Z

I will merge this to help me out to avoid cherry picking all those fixes to other PR's. If there's something that doesn't fit then I'll fix it on another branch.

tcard · 2016-12-20T09:45:36Z

LGTM

@synchronized

* Start 0.9 version * TO3k7 (#513) * Start 0.9 version * Add fallbackHostsUseDefault option * Fallback: add initialiser accepting client options * TO3k7 * Add ARTFallback+Private * Default fallbackHosts as Array of Strings * fixup! TO3k7 * fixup! Add fallbackHostsUseDefault option * fixup! TO3k7 * Update RSC15a for 0.9 (#515) * Start 0.9 version * Update RSC15a: default fallback hosts * Default fallbackHosts as Array of Strings * RSA4a (#517) * Test suite: add TestProxyHTTPExecutor.simulateIncomingServerErrorOnNextRequest * RSA4a * Fix Realtime: indicate an error and not retry the request when the server responds with a token error * Update RSC15e (#514) * Remove specs RSA10c and RSA10d (#522) * RSA10l (#524) * Auth: deprecate `authorise` in favor of `authorize`. * RSA10l * Use `authorize` instead of `authorise` (close #496) * Remove `AuthOptions.force` (#527) * Remove AuthOptions.force * Fix: make a single attempt to reissue the token and resend the request * Fix RSA10a * Fix RSC9 * Remove `prepareAuthorisationHeader` access from test suite * Fix RSA10a * Update RSC15b for 0.9 (#516) * Update RSC15b * Fix: REST fallback should only apply when the default host is used * Update RSA10a for 0.9 (#520) * Update RSA10a * Fix: authorize should change auth method to Token for future requests * Update RSA10j for 0.9 (#521) * Update RSA10j * Fix: ttl when omitted should set the default value * Fix RTN15h * RTC8 (#526) * Add ARTAuthDetails * RTC8a * RTC8a1 (part 1) * RTC8a1 (part 2) * RTC8a1 (part 3) * RTC8a2 * RTC8a3 * RTC8b * RTC8b1 * RTC8c * Send AUTH protocol message on each authorize * Fix RTC8 * Test suite: `splitDone`, when a test fails, get the right location of the failure * RSA4b (#518) * RSA4b * Fix REST: should retry the request once if a token error occurs * Fix Realtime: if the token creation failed then the connection should move to the DISCONNECTED state and reports the error * RSA4c (#519) * RSA4c * Fix: 80019 and description of the underlying failure should be emitted * Fix: if connected and the client receives an ARTProtocolMessageAuth, then authorise * Fix: if authUrl or authCallback fails and is CONNECTED then the connection should remain CONNECTED - RSA4c3 * Add `artDispatchScheduled` and a way to cancel the scheduled block * Fix: authUrl/authCallback attempt times out after realtimeRequestTimeout * Test suite: reset networkConnectEvent after spec * Enhance: debug info * fixup! Fix: authUrl/authCallback attempt times out after realtimeRequestTimeout * fixup! Fix: if connected and the client receives an ARTProtocolMessageAuth, then authorise * Fix: every realtime auth attempt should check if deadline is reached * Rename createWithNSError to createFromNSError * Fix: remove Auth dependency from WebSocketTransport * Fix: should first authorize and then connect the transport * RTN22 (#537) * RTN22 * RTN22a * Fix: realtime transport can be nil * Fix: realtime should renew token by transitioning to CONNECTING * Update RTL3 for 0.9 (#544) * Update RTL3a * Update RTL3b and RTL3c * RTL3d * RTL3e * RTL14 (#550) * UPDATE event (#559) * UPDATE event (replace ERROR event) * Fix: Connection should emit an UPDATE event * Fix RTC8a1 * Fix specs and legacy tests * RTN4h * RTN4f * RTN24 * Update RTL2 for 0.9 (#543) * Add Realtime Channel Suspended state * Add ProtocolMessageActionToStr method * Add ChannelStateChange type * Update RTL2 * RTL2f: pending - functionality hasn't been deployed * Use ChannelStateChange on channel event emitter * Test suite: simulate client suspension with before suspension callback * Update tests using channel events * Fix RTL14 * Fix: Channel on suspended should transition to SUSPENDED state * Remove RTN18 * Fix: set Suspended on all channels when Connection moves to Suspended * RTL2g * Fix RTL12 * Fix RTL3d * Fix: channel should reattach when connection is Connected * Fix: should resume connection when the connection is Suspended * Fix RTN11 * Fix RTL3e * Fix RTC8a1 * Remove testSuspendingDetachesChannel * Fix RTL3d * Fix: channel is SUSPENDED then operation will result in an error * Update RTL13 for 0.9 (#549) * RTL13a * RTL13b * RTL13c * Add ClientOptions.channelRetryTimeout * RealtimeChannel reattach after timeout * Fix: if the channel receives a server initiated DETACHED message and if the channel is in the ATTACHED or SUSPENDED states, then an attempt to reattach the channel should be made immediately * Fix: move to Suspended if attach times out * Fix: if the channel receives a server initiated DETACHED message and the channel is Attaching * Remove test about #454 (replaced by RTL13) * Update RTL4f * Update RTL4 for 0.9 (#545) * RTL4i * RTL4h * Update RTL4e * Fix: attach request should be treated as though it has failed and the channel should transition to the SUSPENDED state * Fix: attach after Detaching * Fix: if it fails to detach then move back to ATTACHED * Fix RTL5f * Update RTL6c for 0.9 (#547) * Update RTL6c2 * Update RTL6c4 * RTL6c3 for Detached * Update RTL6c4 for channel Failed * Better code completion * Add ChannelStateChange.event property (#561) * Update RTN4 for 0.9 (#560) * Update RTN4e * Update RTN4f * Add ConnectionStateChange.event property * Update RTL5 for 0.9 (#546) * RTL5j * Update RTL5a * RTL5i * Fix: if channel is SUSPENDED then the detach request transitions immediately to DETACHED state * Fix: if channel is ATTACHING then do the detach operation after the completion of the attaching * Remove testSkipsFromAttachingToDetaching * Fix: presence sync can fail * Fix RTP11b * Fix RTP9e * RTP5: pending * Fix RTL6c3 * Fix: should check channel state when a queued message is processed * TR4i (#562) * ARTProtocolMessageFlag enum: use NS_OPTIONS to define a bitmask * TR4i * Fix: should indicate that the channel has been resumed or not (RESUMED flag) * Swift performance: speed up code completion (#569) * Speed up code completion - This problem is fixed on Xcode 8 but since we are still using Xcode 7 I decided to change that code. * Test suite: addMembersSequentiallyToChannel should return the realtime client * Test suite: set AsyncDefaults.Timeout with default value * Remove pending tests (#542) * Fix: RTP6c pending test * RSL1g4: remove pending * RTP2: remove pending * Fix RTC8 * Update RTP3 (#566) * Update RTP8d (#572) * RTP17 (#571) * PresenceMap: list of internal members * RTP17 * Add ARTPresenceActionToStr method * RTP17: pending * Update RTP2 for 0.9 (#563) * RTP2: remove pending * RTP2a * RTP2b * Test suite: set Nimble.AsyncDefaults.Timeout - increase the default timeout value from all async expectations * RTP2c * RTP2d * RTP2e * RTP2f * RTP2g * Test suite: ARTPresenceMessage convenience initializer * Test suite: NSDate custom operators - convenience for use of `dateByAddingTimeInterval` * Fix RTP2b1 * Fix RTP2b2 * PresenceMap: compare for newness * Fix RTP2 * Remove warnings * RTP18 (#567) * Test suite: ARTPresenceMessage convenience initializer * RTP18 * RTP18a * RTP18b * RTP18c * Update RSA9h for 0.9 (#574) * Auth: optional arguments * Update RSA9h * Update RSA8e for 0.9 (#573) * Update RSA8e * Auth: optional arguments * Comments * Update RSA10g for 0.9 (#575) * Auth: optional arguments * Update RSA10g * Auth: subsequent authorizations with stored values * Update RTP5 for 0.9 (#570) * Update RTP5 * Update RTP5a * Update RTP5b * RTP5c * Test suite: replaceAcksWithNacks - better code completion * RTP5f * PresenceMap: existing members before Sync * Fix: presence get members should not wait for sync if sync is not in progress * PresenceMap: reenter local member * Test suite: replacing acks with nacks even with Presence action * Fix: should queue messages before the attach operation * Test suite: ARTPresenceAction description * Better debug info * Fix: should continue incrementing msgSerial serially if the connection resumes successfully * Remove warnings * Test suite: timings * RTP19 (#568) * RTP19 * RTP19a * Fix: if channel resumed successfully then do not start sync * Enhance RTP5: presence get should not have Absent members * Fix race condition * Fix #583: update httpRequestTimeout and httpMaxRetryDuration * EventEmitter: use @synchronized because NSMutableArray are not thread safe * Thread safety (#586) * New EventEmitter (using NSNotificationCenter) - In most ways that matter NSNotificationCenter is thread safe. You can add/remove observers from any thread and you can post notifications from any thread. * Events for the EventEmitter * Fix: should cancel timers when connection times out * Fix: new state change can occur before receiving publishing acknowledgement * Test suite: async forced transitions * Test suite: ack order * Test suite: stop when there's no internet * Fix: instance objects released to soon * Performed a static analysis from Xcode * fixup! Test suite: ack order * Memory leak: call session invalidate to dispose of its strong reference to the delegate * fixup! Test suite: ack order * Fix RTN19a: guarantee of a new transport (check transport reference) * Fix: ACK or NACK has not yet been received for a message, the client should consider the delivery of those messages as failed * Enhance RTN14b: better timings * Fix: REST and Realtime, wait for last operation to release the object * fixup! Test suite: ack order * Fix: cancel timers when a connection gets closed * fixup! Test suite: ack order * Test suite: timings * fixup! Enhance RTN14b: better timings * Test suite: close connections * Fix: turn off immediately reachability when close occurs * Fix RTC1d: wait for host is not reachable error * fixup! Test suite: ack order * Travis update * Fix RTN19a

ricardopereira force-pushed the rtl2 branch 4 times, most recently from 34829da to 08024f9 Compare November 30, 2016 11:39

ricardopereira force-pushed the 0-9-master branch from 0857337 to 5728e01 Compare November 30, 2016 12:03

ricardopereira force-pushed the rtl2 branch 2 times, most recently from a1f5b3c to 535c474 Compare December 1, 2016 09:43

mattheworiordan requested changes Dec 2, 2016

View reviewed changes

tcard suggested changes Dec 5, 2016

View reviewed changes

ricardopereira mentioned this pull request Dec 13, 2016

Should not use the global listener for internal purpose #555

Closed

ricardopereira force-pushed the rtl2 branch from 0d8e9c4 to afe616e Compare December 13, 2016 08:46

mattheworiordan mentioned this pull request Dec 13, 2016

ART* should match IDL definition #557

Closed

4 tasks

mattheworiordan reviewed Dec 13, 2016

View reviewed changes

ricardopereira added 3 commits December 15, 2016 15:19

Add Realtime Channel Suspended state

891e8de

Add ChannelStateChange type

0c02837

Add ProtocolMessageActionToStr method

22f1b13

ricardopereira force-pushed the rtl2 branch from afe616e to 7233a8d Compare December 15, 2016 16:04

ricardopereira added 9 commits December 19, 2016 13:51

Update RTL2

c446fd9

RTL2f: pending

340e821

- functionality hasn't been deployed

Use ChannelStateChange on channel event emitter

f41e8c8

Test suite: simulate client suspension with before suspension callback

b008a3f

Update tests using channel events

d1ca520

Fix RTL14

d97ee8f

Fix: Channel on suspended should transition to SUSPENDED state

a7ad1bc

Remove RTN18

f7a2db0

Fix: set Suspended on all channels when Connection moves to Suspended

036a696

ricardopereira added 11 commits December 19, 2016 13:51

RTL2g

aa34dc0

Fix RTL12

76a7991

Fix RTL3d

5cade96

Fix: channel should reattach when connection is Connected

6389d25

Fix: should resume connection when the connection is Suspended

e8fefcd

Fix RTN11

fad4c16

Fix RTL3e

06edc0c

Fix RTC8a1

cd75e9e

Remove testSuspendingDetachesChannel

e1ab41d

Fix RTL3d

7c7c84e

Fix: channel is SUSPENDED then operation will result in an error

3e77e18

ricardopereira force-pushed the rtl2 branch from 9824d3b to 3e77e18 Compare December 19, 2016 13:52

ricardopereira merged commit 7f1309b into 0-9-master Dec 20, 2016

ricardopereira mentioned this pull request Dec 21, 2016

Update RTL12 for 0.9 #548

Closed

ricardopereira deleted the rtl2 branch December 22, 2016 09:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update RTL2 for 0.9 #543

Update RTL2 for 0.9 #543

ricardopereira commented Nov 28, 2016

mattheworiordan left a comment

mattheworiordan Dec 2, 2016

mattheworiordan Dec 2, 2016

ricardopereira Dec 3, 2016

mattheworiordan Dec 2, 2016

ricardopereira Dec 3, 2016

mattheworiordan Dec 2, 2016

ricardopereira Dec 13, 2016

mattheworiordan Dec 2, 2016

ricardopereira Dec 3, 2016

mattheworiordan commented Dec 2, 2016

ricardopereira commented Dec 3, 2016

tcard left a comment

tcard Dec 5, 2016

ricardopereira Dec 13, 2016

tcard Dec 5, 2016

mattheworiordan Dec 13, 2016

ricardopereira Dec 15, 2016

ricardopereira commented Dec 13, 2016

mattheworiordan Dec 13, 2016

ricardopereira Dec 15, 2016

mattheworiordan commented Dec 13, 2016

ricardopereira commented Dec 19, 2016

ricardopereira commented Dec 20, 2016

tcard commented Dec 20, 2016

Update RTL2 for 0.9 #543

Update RTL2 for 0.9 #543

Conversation

ricardopereira commented Nov 28, 2016

mattheworiordan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattheworiordan commented Dec 2, 2016

ricardopereira commented Dec 3, 2016

tcard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ricardopereira commented Dec 13, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattheworiordan commented Dec 13, 2016

ricardopereira commented Dec 19, 2016

ricardopereira commented Dec 20, 2016

tcard commented Dec 20, 2016