From 09f6fa9795ba9484f511a8bb4e29d03b864ef2b6 Mon Sep 17 00:00:00 2001 From: Ben Butterworth <24711048+ben-xD@users.noreply.github.com> Date: Thu, 18 Nov 2021 08:34:15 +0000 Subject: [PATCH 1/7] Add defaults to ChannelOptions --- .../authentication/src/client_options.dart | 28 ++++----- lib/src/platform/src/codec.dart | 61 ++++--------------- lib/src/platform/src/realtime/realtime.dart | 2 +- 3 files changed, 28 insertions(+), 63 deletions(-) diff --git a/lib/src/authentication/src/client_options.dart b/lib/src/authentication/src/client_options.dart index eba848bb8..bb69c2e67 100644 --- a/lib/src/authentication/src/client_options.dart +++ b/lib/src/authentication/src/client_options.dart @@ -32,7 +32,7 @@ class ClientOptions extends AuthOptions { /// /// Use constants from [LogLevel] to pass arguments /// https://docs.ably.com/client-lib-development-guide/features/#TO3b - int? logLevel; + int logLevel = LogLevel.warn; /// for development environments only /// @@ -53,7 +53,7 @@ class ClientOptions extends AuthOptions { /// /// By default, a TLS connection is used to connect to Ably /// https://docs.ably.com/client-lib-development-guide/features/#TO3d - bool? tls; + bool tls = true; /// for development environments only /// @@ -63,29 +63,29 @@ class ClientOptions extends AuthOptions { /// Automatically connect to Ably when client is instantiated. /// /// This is true by default. If false, will wait for an explicit - /// [ConnectionInterface]#connect to be called before connecting + /// [Connection.connect] to be called before connecting /// https://docs.ably.com/client-lib-development-guide/features/#RTC1b - bool? autoConnect; + bool autoConnect = true; /// Decides whether to use MsgPack binary encoding or JSON encoding. /// /// Defaults to true. If false, JSON encoding is used for REST and Realtime /// operations, instead of the default binary msgpack encoding. /// https://docs.ably.com/client-lib-development-guide/features/#TO3f - bool? useBinaryProtocol; + bool useBinaryProtocol = true; /// When true, messages will be queued whilst the connection is disconnected. /// /// True by default. /// https://docs.ably.com/client-lib-development-guide/features/#TO3g - bool? queueMessages; + bool queueMessages = true; /// When true, messages published on channels by this client will be /// echoed back to this client. /// /// This is true by default. /// https://docs.ably.com/client-lib-development-guide/features/#TO3h - bool? echoMessages; + bool echoMessages = true; /// Can be used to explicitly recover a connection. /// @@ -122,7 +122,7 @@ class ClientOptions extends AuthOptions { /// /// default 15,000 (15 seconds) /// https://docs.ably.com/client-lib-development-guide/features/#TO3l1 - int? disconnectedRetryTimeout; + int disconnectedRetryTimeout = 15000; /// When the connection enters the [ConnectionState.suspended] state, /// after this delay in milliseconds, if the state is still @@ -131,7 +131,7 @@ class ClientOptions extends AuthOptions { /// /// default 30,000 (30 seconds) /// https://docs.ably.com/client-lib-development-guide/features/#TO3l2 - int? suspendedRetryTimeout; + int suspendedRetryTimeout = 30000; /// https://docs.ably.com/client-lib-development-guide/features/#TO3n bool? idempotentRestPublishing; @@ -147,20 +147,20 @@ class ClientOptions extends AuthOptions { /// /// default 4,000 (4s) /// https://docs.ably.com/client-lib-development-guide/features/#RTC1f - int? httpOpenTimeout; + int httpOpenTimeout = 4000; /// Timeout for any single HTTP request and response /// /// default 10,000 (10s) /// https://docs.ably.com/client-lib-development-guide/features/#TO3l4 - int? httpRequestTimeout; + int httpRequestTimeout = 10000; /// Max number of fallback hosts to use as a fallback when an HTTP request /// to the primary host is unreachable or indicates that it is unserviceable /// /// default 3 /// https://docs.ably.com/client-lib-development-guide/features/#TO3l5 - int? httpMaxRetryCount; + int httpMaxRetryCount = 3; /// When a realtime client library is establishing a connection with Ably, /// or sending a HEARTBEAT, CONNECT, ATTACH, DETACH or CLOSE ProtocolMessage @@ -179,7 +179,7 @@ class ClientOptions extends AuthOptions { /// /// default 600000 (10 minutes) /// https://docs.ably.com/client-lib-development-guide/features/#TO3l10 - int? fallbackRetryTimeout; + int fallbackRetryTimeout = 600000; /// When a channel becomes [ChannelState.suspended] following a server /// initiated [ChannelState.detached], after this delay in milliseconds, @@ -189,7 +189,7 @@ class ClientOptions extends AuthOptions { /// /// default 15,000 (15s) /// https://docs.ably.com/client-lib-development-guide/features/#TO3l7 - int? channelRetryTimeout; + int channelRetryTimeout = 15000; // TODO(tiholic) unimplemented: // diff --git a/lib/src/platform/src/codec.dart b/lib/src/platform/src/codec.dart index 238deaaa3..6e0cad3e7 100644 --- a/lib/src/platform/src/codec.dart +++ b/lib/src/platform/src/codec.dart @@ -573,15 +573,9 @@ class Codec extends StandardMessageCodec { jsonMap, TxClientOptions.clientId, ) - ..logLevel = _readFromJson( - jsonMap, - TxClientOptions.logLevel, - ) + ..logLevel = jsonMap[TxClientOptions.logLevel] as int //TODO handle logHandler - ..tls = _readFromJson( - jsonMap, - TxClientOptions.tls, - ) + ..tls = jsonMap[TxClientOptions.tls] as bool ..restHost = _readFromJson( jsonMap, TxClientOptions.restHost, @@ -598,22 +592,10 @@ class Codec extends StandardMessageCodec { jsonMap, TxClientOptions.tlsPort, ) - ..autoConnect = _readFromJson( - jsonMap, - TxClientOptions.autoConnect, - ) - ..useBinaryProtocol = _readFromJson( - jsonMap, - TxClientOptions.useBinaryProtocol, - ) - ..queueMessages = _readFromJson( - jsonMap, - TxClientOptions.queueMessages, - ) - ..echoMessages = _readFromJson( - jsonMap, - TxClientOptions.echoMessages, - ) + ..autoConnect = jsonMap[TxClientOptions.autoConnect] as bool + ..useBinaryProtocol = jsonMap[TxClientOptions.useBinaryProtocol] as bool + ..queueMessages = jsonMap[TxClientOptions.queueMessages] as bool + ..echoMessages = jsonMap[TxClientOptions.echoMessages] as bool ..recover = _readFromJson( jsonMap, TxClientOptions.recover, @@ -626,22 +608,11 @@ class Codec extends StandardMessageCodec { jsonMap, TxClientOptions.idempotentRestPublishing, ) - ..httpOpenTimeout = _readFromJson( - jsonMap, - TxClientOptions.httpOpenTimeout, - ) - ..httpRequestTimeout = _readFromJson( - jsonMap, - TxClientOptions.httpRequestTimeout, - ) - ..httpMaxRetryCount = _readFromJson( - jsonMap, - TxClientOptions.httpMaxRetryCount, - ) - ..realtimeRequestTimeout = _readFromJson( - jsonMap, - TxClientOptions.realtimeRequestTimeout, - ) + ..httpOpenTimeout = jsonMap[TxClientOptions.httpOpenTimeout] as int + ..httpRequestTimeout = jsonMap[TxClientOptions.httpRequestTimeout] as int + ..httpMaxRetryCount = jsonMap[TxClientOptions.httpMaxRetryCount] as int + ..realtimeRequestTimeout = + jsonMap[TxClientOptions.realtimeRequestTimeout] as int ..fallbackHosts = _readFromJson>( jsonMap, TxClientOptions.fallbackHosts, @@ -650,16 +621,10 @@ class Codec extends StandardMessageCodec { jsonMap, TxClientOptions.fallbackHostsUseDefault, ) - ..fallbackRetryTimeout = _readFromJson( - jsonMap, - TxClientOptions.fallbackRetryTimeout, - ) + ..fallbackRetryTimeout = jsonMap[TxClientOptions.fallbackRetryTimeout] as int ..defaultTokenParams = (tokenParams == null) ? null : _decodeTokenParams(tokenParams) - ..channelRetryTimeout = _readFromJson( - jsonMap, - TxClientOptions.channelRetryTimeout, - ) + ..channelRetryTimeout = jsonMap[TxClientOptions.channelRetryTimeout] as int ..transportParams = _readFromJson>( jsonMap, TxClientOptions.transportParams, diff --git a/lib/src/platform/src/realtime/realtime.dart b/lib/src/platform/src/realtime/realtime.dart index cfd1b062c..351194f2a 100644 --- a/lib/src/platform/src/realtime/realtime.dart +++ b/lib/src/platform/src/realtime/realtime.dart @@ -37,7 +37,7 @@ class Realtime extends PlatformObject { ); _realtimeInstances[handle] = this; - if (io.Platform.isAndroid && options.autoConnect != false) { + if (io.Platform.isAndroid && !options.autoConnect) { // On Android, clientOptions.autoConnect is set to `false` to prevent // the authCallback being called before we get the realtime handle. // If this happens, we won't be able to identify which realtime client From 07d35a33d1796d0111743d691e5b30be866bb82b Mon Sep 17 00:00:00 2001 From: Ben Butterworth <24711048+ben-xD@users.noreply.github.com> Date: Thu, 18 Nov 2021 10:26:05 +0000 Subject: [PATCH 2/7] Run `flutter format .` --- lib/src/platform/src/codec.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/src/platform/src/codec.dart b/lib/src/platform/src/codec.dart index 6e0cad3e7..7ba69eb31 100644 --- a/lib/src/platform/src/codec.dart +++ b/lib/src/platform/src/codec.dart @@ -621,10 +621,12 @@ class Codec extends StandardMessageCodec { jsonMap, TxClientOptions.fallbackHostsUseDefault, ) - ..fallbackRetryTimeout = jsonMap[TxClientOptions.fallbackRetryTimeout] as int + ..fallbackRetryTimeout = + jsonMap[TxClientOptions.fallbackRetryTimeout] as int ..defaultTokenParams = (tokenParams == null) ? null : _decodeTokenParams(tokenParams) - ..channelRetryTimeout = jsonMap[TxClientOptions.channelRetryTimeout] as int + ..channelRetryTimeout = + jsonMap[TxClientOptions.channelRetryTimeout] as int ..transportParams = _readFromJson>( jsonMap, TxClientOptions.transportParams, From 54fcf8e89d5e1504c9d9996bf4879f600b2b08a8 Mon Sep 17 00:00:00 2001 From: Ben Butterworth <24711048+ben-xD@users.noreply.github.com> Date: Thu, 18 Nov 2021 10:35:42 +0000 Subject: [PATCH 3/7] Fix boolean check --- lib/src/platform/src/realtime/realtime.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/platform/src/realtime/realtime.dart b/lib/src/platform/src/realtime/realtime.dart index 351194f2a..07ea86135 100644 --- a/lib/src/platform/src/realtime/realtime.dart +++ b/lib/src/platform/src/realtime/realtime.dart @@ -37,7 +37,7 @@ class Realtime extends PlatformObject { ); _realtimeInstances[handle] = this; - if (io.Platform.isAndroid && !options.autoConnect) { + if (io.Platform.isAndroid && options.autoConnect) { // On Android, clientOptions.autoConnect is set to `false` to prevent // the authCallback being called before we get the realtime handle. // If this happens, we won't be able to identify which realtime client From a933b3116b4eae353363ac5868385d25aa732d26 Mon Sep 17 00:00:00 2001 From: Ben Butterworth <24711048+ben-xD@users.noreply.github.com> Date: Thu, 18 Nov 2021 10:42:32 +0000 Subject: [PATCH 4/7] Remove unnecessary constructor logic --- lib/src/authentication/src/client_options.dart | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/src/authentication/src/client_options.dart b/lib/src/authentication/src/client_options.dart index bb69c2e67..77004e19e 100644 --- a/lib/src/authentication/src/client_options.dart +++ b/lib/src/authentication/src/client_options.dart @@ -5,16 +5,12 @@ import 'package:ably_flutter/ably_flutter.dart'; /// https://docs.ably.com/client-lib-development-guide/features/#TO1 class ClientOptions extends AuthOptions { /// initializes [ClientOptions] with log level set to info - ClientOptions() { - logLevel = LogLevel.info; - } + ClientOptions(); /// initializes [ClientOptions] with a key and log level set to info /// /// See [AuthOptions.fromKey] for more details - ClientOptions.fromKey(String key) : super.fromKey(key) { - logLevel = LogLevel.info; - } + ClientOptions.fromKey(String key) : super.fromKey(key); /// Optional clientId that can be used to specify the identity for this client /// @@ -32,7 +28,7 @@ class ClientOptions extends AuthOptions { /// /// Use constants from [LogLevel] to pass arguments /// https://docs.ably.com/client-lib-development-guide/features/#TO3b - int logLevel = LogLevel.warn; + int logLevel = LogLevel.info; /// for development environments only /// From b0799d3e0859200c91a244ff8e9a8d5b0f5d5cfe Mon Sep 17 00:00:00 2001 From: Ben Butterworth <24711048+ben-xD@users.noreply.github.com> Date: Thu, 18 Nov 2021 10:43:45 +0000 Subject: [PATCH 5/7] Add documentation to ClientOptions constructor --- lib/src/authentication/src/client_options.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/authentication/src/client_options.dart b/lib/src/authentication/src/client_options.dart index 77004e19e..f06c05886 100644 --- a/lib/src/authentication/src/client_options.dart +++ b/lib/src/authentication/src/client_options.dart @@ -4,7 +4,7 @@ import 'package:ably_flutter/ably_flutter.dart'; /// /// https://docs.ably.com/client-lib-development-guide/features/#TO1 class ClientOptions extends AuthOptions { - /// initializes [ClientOptions] with log level set to info + /// Set fields on [ClientOptions] to configure it. ClientOptions(); /// initializes [ClientOptions] with a key and log level set to info From 30f92750458b397320b03379b98f6aa3f5b8fb45 Mon Sep 17 00:00:00 2001 From: Ben Butterworth <24711048+ben-xD@users.noreply.github.com> Date: Thu, 18 Nov 2021 10:52:27 +0000 Subject: [PATCH 6/7] Check realtimeRequestTimeout is non null before writing --- lib/src/authentication/src/client_options.dart | 2 +- lib/src/platform/src/codec.dart | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/src/authentication/src/client_options.dart b/lib/src/authentication/src/client_options.dart index f06c05886..3b22f4521 100644 --- a/lib/src/authentication/src/client_options.dart +++ b/lib/src/authentication/src/client_options.dart @@ -166,7 +166,7 @@ class ClientOptions extends AuthOptions { /// /// default 10s /// https://docs.ably.com/client-lib-development-guide/features/#DF1b - int? realtimeRequestTimeout; + int realtimeRequestTimeout = 10; /// After a failed request to the default endpoint, /// followed by a successful request to a fallback endpoint, diff --git a/lib/src/platform/src/codec.dart b/lib/src/platform/src/codec.dart index 7ba69eb31..33418d814 100644 --- a/lib/src/platform/src/codec.dart +++ b/lib/src/platform/src/codec.dart @@ -535,7 +535,7 @@ class Codec extends StandardMessageCodec { jsonMap, TxClientOptions.defaultTokenParams, )); - return ClientOptions() + final clientOptions = ClientOptions() // AuthOptions (super class of ClientOptions) ..authUrl = _readFromJson( jsonMap, @@ -611,8 +611,6 @@ class Codec extends StandardMessageCodec { ..httpOpenTimeout = jsonMap[TxClientOptions.httpOpenTimeout] as int ..httpRequestTimeout = jsonMap[TxClientOptions.httpRequestTimeout] as int ..httpMaxRetryCount = jsonMap[TxClientOptions.httpMaxRetryCount] as int - ..realtimeRequestTimeout = - jsonMap[TxClientOptions.realtimeRequestTimeout] as int ..fallbackHosts = _readFromJson>( jsonMap, TxClientOptions.fallbackHosts, @@ -631,6 +629,12 @@ class Codec extends StandardMessageCodec { jsonMap, TxClientOptions.transportParams, ); + + if (jsonMap[TxClientOptions.realtimeRequestTimeout] != null) { + clientOptions.realtimeRequestTimeout = + jsonMap[TxClientOptions.realtimeRequestTimeout] as int; + } + return clientOptions; } /// Decodes value [jsonMap] to [TokenDetails] From 9dd2770a264bd360a2a130e0a25ae5b0ddb9a1aa Mon Sep 17 00:00:00 2001 From: Ben Butterworth <24711048+ben-xD@users.noreply.github.com> Date: Mon, 22 Nov 2021 17:09:45 +0000 Subject: [PATCH 7/7] Make `realtimeRequestTimeout` optional, since it is meaningless for Rest clients and would be confusing --- lib/src/authentication/src/client_options.dart | 2 +- lib/src/platform/src/codec.dart | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/src/authentication/src/client_options.dart b/lib/src/authentication/src/client_options.dart index 3b22f4521..f06c05886 100644 --- a/lib/src/authentication/src/client_options.dart +++ b/lib/src/authentication/src/client_options.dart @@ -166,7 +166,7 @@ class ClientOptions extends AuthOptions { /// /// default 10s /// https://docs.ably.com/client-lib-development-guide/features/#DF1b - int realtimeRequestTimeout = 10; + int? realtimeRequestTimeout; /// After a failed request to the default endpoint, /// followed by a successful request to a fallback endpoint, diff --git a/lib/src/platform/src/codec.dart b/lib/src/platform/src/codec.dart index 33418d814..be258fce6 100644 --- a/lib/src/platform/src/codec.dart +++ b/lib/src/platform/src/codec.dart @@ -608,6 +608,8 @@ class Codec extends StandardMessageCodec { jsonMap, TxClientOptions.idempotentRestPublishing, ) + ..realtimeRequestTimeout = + jsonMap[TxClientOptions.realtimeRequestTimeout] as int? ..httpOpenTimeout = jsonMap[TxClientOptions.httpOpenTimeout] as int ..httpRequestTimeout = jsonMap[TxClientOptions.httpRequestTimeout] as int ..httpMaxRetryCount = jsonMap[TxClientOptions.httpMaxRetryCount] as int @@ -629,11 +631,6 @@ class Codec extends StandardMessageCodec { jsonMap, TxClientOptions.transportParams, ); - - if (jsonMap[TxClientOptions.realtimeRequestTimeout] != null) { - clientOptions.realtimeRequestTimeout = - jsonMap[TxClientOptions.realtimeRequestTimeout] as int; - } return clientOptions; }