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.

Sign up for GitHub

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

Jump to bottom

Remove AuthOptions.force #527

Merged
merged 6 commits into from
Nov 1, 2016
Merged

Remove AuthOptions.force #527

merged 6 commits into from
Nov 1, 2016

Conversation

ricardopereira
Copy link
Contributor

No description provided.

mattheworiordan
mattheworiordan approved these changes Oct 20, 2016
View reviewed changes
Copy link
Member

@mattheworiordan mattheworiordan left a comment

Choose a reason for hiding this comment

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

Surprised this didn't break tests, but +1. Obviously we need to just make sure we have coverage over authorize always authing

@ricardopereira
Copy link
Contributor Author

There are broken tests. I'll fix them now.

@ricardopereira
Copy link
Contributor Author

@tcard PTAL

mattheworiordan
mattheworiordan requested changes Nov 1, 2016
View reviewed changes
Copy link
Member

@mattheworiordan mattheworiordan left a comment

Choose a reason for hiding this comment

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

LGTM apart from one test that needs addressing

Spec/Auth.swift
@@ -1679,6 +1675,7 @@ class Auth : QuickSpec {
// RSA10a
it("should create a token if needed and use it") {
Copy link
Member

Choose a reason for hiding this comment

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

This description is wrong, it will always create a token?

Copy link
Member

Choose a reason for hiding this comment

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

Also, https://github.com/ably/ably-ios/pull/527/files#diff-b5d84fe9bce26902a7481c2ffec84fffR1673 should be authorize

Spec/Auth.swift
@@ -1692,7 +1689,7 @@ class Auth : QuickSpec {
guard let tokenDetails = tokenDetails else {
XCTFail("TokenDetails is nil"); done(); return
}
expect(tokenDetails.token).to(equal(options.token))
expect(tokenDetails.token).toNot(equal(options.token))
Copy link
Member

Choose a reason for hiding this comment

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

The comment above says Reuse the valid token yet that's not correct. Also, I would like to see that implicit publish operations don't now create a new token every time. Could we therefore change the logic for this test to:

  • Publish a message
  • Check that token exists
  • Publish another message
  • Check that token has not changed
  • Call authorize
  • Check that token has changed

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 6bef509 👍

@ricardopereira ricardopereira merged commit 9d1a8af into 0-9-master Nov 1, 2016
@ricardopereira ricardopereira deleted the remove-auth-force-prop branch November 1, 2016 13:02
ricardopereira added a commit that referenced this pull request Nov 30, 2016
@ricardopereira
Remove AuthOptions.force (#527)
f9e39d3 
* 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
ricardopereira added a commit that referenced this pull request Mar 23, 2017
@ricardopereira
0.9 master (#588)
85fd3b5 
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattheworiordan mattheworiordan mattheworiordan requested changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ricardopereira @mattheworiordan