Pilot implementation for Realtime event based API's - ConnectionStateChange #6

tiholic · 2020-05-03T21:49:38Z

Realtime events workflow based on Streams handled for both Android and iOS.

Using StreamChannel, which is based on EventChannel

StreamsChannel (EventChannel) is setup on both Android and iOS
ConnectionStateChange is fully supported
Better way to handle codecs using CodecPair<Type>(encoder, decoder)
- Supported only on Dart and Android side.
- Objective C has separate reader and writer implementations.

Known Issues - Applies only for Event Streams:

Hot-Restart will not dispose of platform side objects
- This is a concern only for development phase
In production, the behavior is as below:
- On minimising the app (background) - instances stay intact (dart side will be perfectly connected to platform side)
- On killing the app - all instances will be created afresh and linked as expected

NOTE: A fix for the above issue is an experimental WIP, and changes related to the same is not available in the repo.

mattheworiordan

Looking good.

Any chance you could help me run this locally and test with some instructions? Perhaps in the example README?

Also, as requested in other PRs, it would be great to see the Dart IDL README include docs as you go along, in line with the other SDKs.

android/src/main/java/io/ably/flutter/plugin/AblyEventStreamHandler.java

android/src/main/java/io/ably/flutter/plugin/AblyMessageCodec.java

ios/Classes/codec/AblyFlutterReader.h

QuintinWillison

I've made a first round of comments but I need to go for a few hours so wanted to get these in front of you @tiholic so discussion can start. I'll be back later on to contribute further.

pubspec.yaml

example/lib/main.dart

android/src/main/java/io/ably/flutter/plugin/AblyMessageCodec.java

ios/Classes/codec/AblyFlutterReader.h

android/src/main/java/io/ably/flutter/plugin/AblyEventStreamHandler.java

ios/Classes/AblyFlutterStreamHandler.h

QuintinWillison · 2020-05-06T11:22:38Z

You need to rebase your commits against the target branch as there are conflicts which may be making review harder. Thanks!

tiholic · 2020-05-06T20:15:20Z

@QuintinWillison rebase complete

#6 (comment)

- _Nullable to nullable - making sure argument name placeholder and selector signature are in sync #6 (comment)

https://docs.ably.io/client-lib-development-guide/features/#RTL6i

- `.fromKey` and `.fromOptions` are removed from high level interface #2 (comment) - Getting rid of XXXBase and XXX as it is only required for ably-java since it supports plain java and android #2 (comment)

- Getting rid of XXXBase and XXX as it is only required for ably-java (since it supports plain java and android) #2 (comment)

- adding new streams_channel dependency for handling multiple streams - reverting old dev dependency versions due to pub get conflict

…mplemented event channels

#6 (comment)

- _Nullable to nullable - making sure argument name placeholder and selector signature are in sync #6 (comment)

- improvise example for nested listener creation and cancellation - A new test to understand how nested stream cancellations based on the order in which subscriptions are created

tiholic · 2020-05-16T10:11:47Z

@mattheworiordan @QuintinWillison

Updated README with "how to run example" and "API usage"

9f9a3f8

README.md

mattheworiordan

I am not clear why we have designed the API in a way that deviates so significantly from all other SDKs. See https://www.ably.io/documentation/quick-start-guide and choose different languages to see. We have very good consistency across the board, yet Dart has deviated significantly with a new plugin object, and all static object types exposed at the root level (i.e. Ably. in an IDE will surface every object type now).

Can you explain why the API is presented in this way?
What decision was made that justified deviating from the norm?

README.md

Addressing: https://github.com/ably/ably-flutter/pull/6/files/9f9a3f8df536495a6a9607eee2396c6d58e89abf#r430215959 https://github.com/ably/ably-flutter/pull/6/files/9f9a3f8df536495a6a9607eee2396c6d58e89abf#r430214818 https://github.com/ably/ably-flutter/pull/6/files/9f9a3f8df536495a6a9607eee2396c6d58e89abf#r430215097

mattheworiordan · 2020-06-10T21:54:01Z

Ok re separate PR to address comments. Looking forward to seeing an API that follows the Ably IDL.

remove env from README

Shedding light on RTE6a drawback and suggestion on overcoming that from SDK users end

QuintinWillison

I think we've deliberated over this particular PR long enough already. We understand the constraints and it delivers the functionality required in a Dart (/ Flutter) idiomatic manner. Let's get it merged and move this repository forward!

Will be addressing in a separate PR ( #15 )

#6 (comment)

- _Nullable to nullable - making sure argument name placeholder and selector signature are in sync #6 (comment)

Hoping to solve this error in workflow environment: Unhandled exception: Invalid argument (uri): Unknown package: Instance of '_SimpleUri' #0 _resolveUri.<anonymous closure> (package:dartdoc/src/generator/resource_loader.dart:34:9) #1 _RootZone.runUnary (dart:async/zone.dart:1612:54) #2 _FutureListener.handleValue (dart:async/future_impl.dart:152:18) #3 Future._propagateToListeners.handleValueCallback (dart:async/future_impl.dart:704:45) #4 Future._propagateToListeners (dart:async/future_impl.dart:733:32) #5 Future._completeWithValue (dart:async/future_impl.dart:539:5) #6 Future._asyncCompleteWithValue.<anonymous closure> (dart:async/future_impl.dart:577:7) #7 _microtaskLoop (dart:async/schedule_microtask.dart:40:21) #8 _startMicrotaskLoop (dart:async/schedule_microtask.dart:49:5) #9 _runPendingImmediateCallback (dart:isolate-patch/isolate_patch.dart:120:13) #10 _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:185:5) Error: Process completed with exit code 255.

tiholic requested review from QuintinWillison, mattheworiordan and paddybyers May 3, 2020 21:50

tiholic changed the base branch from master to feature/platform-message-encoding May 4, 2020 08:03

QuintinWillison mentioned this pull request May 4, 2020

Feature/ios rest #4

Merged

tiholic force-pushed the feature/platform-message-encoding branch from 0c862fe to c11e35d Compare May 5, 2020 17:06

mattheworiordan reviewed May 5, 2020

View reviewed changes

tiholic mentioned this pull request May 6, 2020

implement or ignore endOfStream in AblyEventStreamHandler. MainThreadEventSink #9

Open

QuintinWillison reviewed May 6, 2020

View reviewed changes

tiholic force-pushed the feature/realtime-pilot branch 2 times, most recently from 7e01bd4 to 6f8c575 Compare May 6, 2020 19:49

tiholic pushed a commit that referenced this pull request May 7, 2020

staying consistent with spacing and docstrings vs regular comments

870dbee

#6 (comment)

tiholic pushed a commit that referenced this pull request May 7, 2020

Sanitize

1542ced

- _Nullable to nullable - making sure argument name placeholder and selector signature are in sync #6 (comment)

This was referenced May 7, 2020

map/dict based approach to Encoding and Decoding channel data #12

Closed

Feature/platform message encoding #5

Merged

tiholic changed the base branch from feature/platform-message-encoding to master May 13, 2020 17:04

Rohit R. Abbadi added 12 commits May 14, 2020 02:52

ObjectiveC: using const wherever required

3455f8e

[RTL6i] spec update: Rest.publish => name and/or data are optional

6ed4eb0

https://docs.ably.io/client-lib-development-guide/features/#RTL6i

spec changes: assert based optional arguments. No more bases

d453599

- `.fromKey` and `.fromOptions` are removed from high level interface #2 (comment) - Getting rid of XXXBase and XXX as it is only required for ably-java since it supports plain java and android #2 (comment)

No more XXXBase - except AblyBase

3f99ee4

- Getting rid of XXXBase and XXX as it is only required for ably-java (since it supports plain java and android) #2 (comment)

Passing Ably(plugin) instance to PlatformObjects

e5c0baa

Passing Ably(plugin) instance to PlatformObjects

5350ef4

realtime channels made final

a1ddf71

pubspec.yaml update

e4765a0

- adding new streams_channel dependency for handling multiple streams - reverting old dev dependency versions due to pub get conflict

Adding StreamsChannel instance to ably_implementation

db8150c

realtime implementation and other spec migrations

4202ebb

android part of StreamHandler - support for multiple listeners handled

5dacd42

dependency lock files

d366382

Rohit R. Abbadi added 3 commits May 14, 2020 02:52

doc string for AblyEventStreamHandler and raise proper error on uni…

31c9a6b

…mplemented event channels

staying consistent with spacing and docstrings vs regular comments

9ed9de3

#6 (comment)

Sanitize

3027da0

- _Nullable to nullable - making sure argument name placeholder and selector signature are in sync #6 (comment)

tiholic force-pushed the feature/realtime-pilot branch from 1542ced to 3027da0 Compare May 13, 2020 21:22

Rohit R. Abbadi added 3 commits May 15, 2020 10:19

event names update at cocoa end

82d2184

RTE6a

cb8090b

- improvise example for nested listener creation and cancellation - A new test to understand how nested stream cancellations based on the order in which subscriptions are created

update README

9f9a3f8

QuintinWillison reviewed May 26, 2020

View reviewed changes

README.md Show resolved Hide resolved

QuintinWillison reviewed May 26, 2020

View reviewed changes

README.md Show resolved Hide resolved

QuintinWillison reviewed May 26, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

mattheworiordan reviewed May 26, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

mattheworiordan previously requested changes May 26, 2020

View reviewed changes

README.md Show resolved Hide resolved

README.md Show resolved Hide resolved

tiholic added 2 commits May 27, 2020 10:56

Update README.md - no env suggestions

d46b88e

Update README.md

c777214

remove env from README

This was referenced Jun 18, 2020

Deviation from Ably SDK APIs #15

Closed

Should we implement EventEmitter? #16

Closed

tiholic added 2 commits June 19, 2020 12:09

Update README

1d0f376

Shedding light on RTE6a drawback and suggestion on overcoming that from SDK users end

[housekeeping] Update README

a6151d7

QuintinWillison approved these changes Jun 19, 2020

View reviewed changes

tiholic merged commit 683a6e0 into master Jun 20, 2020

tiholic pushed a commit that referenced this pull request Jun 20, 2020

staying consistent with spacing and docstrings vs regular comments

fd5d12e

#6 (comment)

tiholic pushed a commit that referenced this pull request Jun 20, 2020

Sanitize

dcb5be7

- _Nullable to nullable - making sure argument name placeholder and selector signature are in sync #6 (comment)

tiholic deleted the feature/realtime-pilot branch June 20, 2020 09:43

1N50MN14 mentioned this pull request May 11, 2022

iOS _CastError exception #355

Closed

sync-by-unito bot mentioned this pull request Oct 26, 2022

Error with onDeactivate #294

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pilot implementation for Realtime event based API's - ConnectionStateChange #6

Pilot implementation for Realtime event based API's - ConnectionStateChange #6

tiholic commented May 3, 2020

mattheworiordan left a comment

QuintinWillison left a comment

QuintinWillison commented May 6, 2020

tiholic commented May 6, 2020

tiholic commented May 16, 2020

mattheworiordan left a comment

mattheworiordan commented Jun 10, 2020

QuintinWillison left a comment

Pilot implementation for Realtime event based API's - ConnectionStateChange #6

Pilot implementation for Realtime event based API's - ConnectionStateChange #6

Conversation

tiholic commented May 3, 2020

Realtime events workflow based on Streams handled for both Android and iOS.

Known Issues - Applies only for Event Streams:

mattheworiordan left a comment

Choose a reason for hiding this comment

QuintinWillison left a comment

Choose a reason for hiding this comment

QuintinWillison commented May 6, 2020

tiholic commented May 6, 2020

tiholic commented May 16, 2020

mattheworiordan left a comment

Choose a reason for hiding this comment

mattheworiordan commented Jun 10, 2020

QuintinWillison left a comment

Choose a reason for hiding this comment