re-codec using JSON and Codegen #13

tiholic · 2020-05-23T11:32:25Z

JSON based encoding and decoding.

Encoders and Decoders will now return JSON from Ably type and return Ably type from JSON
Addressing: #12

Codegen for constants that need to be in Sync on platform side and flutter side

Generate codec types (Addressing: Feature/platform message encoding #5 (comment))
Generate method names for MethodChannel and EventChannel
Generate Properties of all transferable objects (Ex: ClientOptions.authUrl, ClientOptions.authMethod, etc)

PS
protobuf based codec didn't turnout to be a good approach as per trails and discussions.

And other approaches like flatbuffers don't have direct support for Objective-C, but support Swift which would require us to port iOS platform code to Swift.

QuintinWillison

Not quite ready to approve as there are a few things that I would like you to look at, however generally I think this is a really good PR. Nice work @tiholic! 😀

bin/README.md

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

android/src/main/java/io/ably/flutter/plugin/gen/PlatformConstants.java

bin/codegen.dart

ios/Classes/codec/AblyFlutterReader.h

ios/Classes/codec/AblyFlutterReader.m

ios/Classes/codec/AblyFlutterWriter.m

lib/src/codec.dart

[code consistency] remove space for ObjC arguments #13 (comment) Co-authored-by: Quintin <quintin@ably.io>

[cleanup] Adding NS_ASSUME_NONNULL_BEGIN and END as per the comment #13 (comment) Co-authored-by: Quintin <quintin@ably.io>

mattheworiordan

I like where this is headed!

Caveat - I haven't done a full review, so don't rely on me for this PR's technical review

#13 (comment)

tiholic

@QuintinWillison thanks for the comments. I've addressed them.

tiholic · 2020-06-04T11:25:38Z

bin/templates/platformconstants.dart.dart

+}
+''';}).join('\n')}
+''';
+}


Agree 💯

Addressed in 72743fd

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

android/src/main/java/io/ably/flutter/plugin/gen/PlatformConstants.java

…mplemented event channels

- mustache template based codegen - generates codec types and platform methods - addresses #5 (comment)

- 4 generated files: - PlatformConstants.java - AblyPlatformConstants.h - AblyPlatformConstants.m - platformconstants.dart Changes related to the same in relevant files

… update

…oid end

1. Generation format changed from mustache templates to straight forward dart string interpolation 2. generating JSON keys for encodable/decodable objects. Properties of each object are generated to `TxObjectName` 3. generated JSON keys used in encoding/decoding methods

[code consistency] remove space for ObjC arguments #13 (comment) Co-authored-by: Quintin <quintin@ably.io>

[cleanup] Adding NS_ASSUME_NONNULL_BEGIN and END as per the comment #13 (comment) Co-authored-by: Quintin <quintin@ably.io>

https://github.com/ably/ably-flutter/pull/13/files#r434375290

https://github.com/ably/ably-flutter/pull/13/files#r434377705

- remove usage of abbreviations gen -> generated https://github.com/ably/ably-flutter/pull/13/files#r434369291 - remove commented code (will be re-implemented while working on channel events) https://github.com/ably/ably-flutter/pull/13/files#r434372985 - make Java static classes final https://github.com/ably/ably-flutter/pull/13/files#r434373812

- `@import Foundation;` https://github.com/ably/ably-flutter/pull/13/files#r434376328 https://github.com/ably/ably-flutter/pull/13/files#r434376097 - static externs instead of assigned properties https://github.com/ably/ably-flutter/pull/13/files#r434381831 - missing properties in platform libraries added to issue #14 https://github.com/ably/ably-flutter/pull/13/files#r434454731

#13 (comment)

QuintinWillison

Thanks for adding commentary, @tiholic ... Now that I'm looking at this again from a bit more of a zoomed out view, plus observing that you've reduced visibility for some of the types, I think some of the names can be simplified:

_CodecEncoder rename to _Encoder
_CodecDecoder rename to _Decoder
_CodecPair rename to _Codec (by definition bidirectional, thus implicitly a pair)

adhering to #13 (review)

tiholic · 2020-06-27T05:21:15Z

@QuintinWillison renamed _CodecEncoder and _CodecDecoder, bit I'd prefer leaving the class name _CodecPair as is because the class extending StandardMessageCodec is named as Codec (ref)

2a49b96

QuintinWillison

@tiholic please merge this when you're ready to.

[code consistency] remove space for ObjC arguments #13 (comment) Co-authored-by: Quintin <quintin@ably.io>

[cleanup] Adding NS_ASSUME_NONNULL_BEGIN and END as per the comment #13 (comment) Co-authored-by: Quintin <quintin@ably.io>

#13 (comment)

tiholic requested review from QuintinWillison, mattheworiordan and paddybyers May 23, 2020 11:32

QuintinWillison mentioned this pull request Jun 3, 2020

Feature/platform message encoding #5

Merged

QuintinWillison reviewed Jun 3, 2020

View reviewed changes

tiholic added a commit that referenced this pull request Jun 4, 2020

Update ios/Classes/codec/AblyFlutterWriter.m

3dbf717

[code consistency] remove space for ObjC arguments #13 (comment) Co-authored-by: Quintin <quintin@ably.io>

tiholic added a commit that referenced this pull request Jun 4, 2020

Update ios/Classes/codec/AblyFlutterReader.m

8df3927

[cleanup] Adding NS_ASSUME_NONNULL_BEGIN and END as per the comment #13 (comment) Co-authored-by: Quintin <quintin@ably.io>

mattheworiordan approved these changes Jun 4, 2020

View reviewed changes

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

remove abbreviations jsonDict -> dictionary

fbe86ff

#13 (comment)

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

update README

b78eba3

#13 (comment)

tiholic commented Jun 5, 2020

View reviewed changes

tiholic referenced this pull request Jun 18, 2020

doc string for AblyEventStreamHandler and raise proper error on uni…

d59f89a

…mplemented event channels

tiholic mentioned this pull request Jun 18, 2020

Deviation from Ably SDK APIs #15

Closed

Base automatically changed from feature/realtime-pilot to master June 20, 2020 09:43

Rohit R. Abbadi and others added 16 commits June 20, 2020 15:22

custom Codegen for CodecTypes and PlatformMethods

97bd8ee

- mustache template based codegen - generates codec types and platform methods - addresses #5 (comment)

code generated using custom Codegen for CodecTypes and PlatformMethods

7ad4263

- 4 generated files: - PlatformConstants.java - AblyPlatformConstants.h - AblyPlatformConstants.m - platformconstants.dart Changes related to the same in relevant files

iOS | generated code usage in implementation files and DeveloperNotes…

0d1f80b

… update

android | generated code usage in implementation files

cc1b82b

Transmission using plain JSON. Coded refactored on dart, iOS and Andr…

de7f690

…oid end

cleanup unwanted codecTypes

53187a6

Update ios/Classes/codec/AblyFlutterWriter.m

cd09506

[code consistency] remove space for ObjC arguments #13 (comment) Co-authored-by: Quintin <quintin@ably.io>

Update ios/Classes/codec/AblyFlutterReader.m

aee2ca2

[cleanup] Adding NS_ASSUME_NONNULL_BEGIN and END as per the comment #13 (comment) Co-authored-by: Quintin <quintin@ably.io>

leading space consistency in commentary

bc8172c

https://github.com/ably/ably-flutter/pull/13/files#r434375290

[consistency] new line at EOF

a811d56

https://github.com/ably/ably-flutter/pull/13/files#r434377705

cleanup java genereated code

f9269cb

fixed improper indent in generated dart file

236bda2

remove abbreviations jsonDict -> dictionary

b1c2b0a

#13 (comment)

update README

5fcbad8

#13 (comment)

tiholic force-pushed the feature/codec-upgrade branch from b78eba3 to 5fcbad8 Compare June 20, 2020 09:54

documented codec.dart and make necessary declarations private

237315e

QuintinWillison suggested changes Jun 26, 2020

View reviewed changes

Rename _CodecEncoder and _CodecDecoder

2a49b96

adhering to #13 (review)

QuintinWillison approved these changes Jun 30, 2020

View reviewed changes

tiholic merged commit 20bd3b5 into master Jul 6, 2020

tiholic added a commit that referenced this pull request Jul 6, 2020

Update ios/Classes/codec/AblyFlutterWriter.m

77d3090

[code consistency] remove space for ObjC arguments #13 (comment) Co-authored-by: Quintin <quintin@ably.io>

tiholic added a commit that referenced this pull request Jul 6, 2020

Update ios/Classes/codec/AblyFlutterReader.m

f771c85

[cleanup] Adding NS_ASSUME_NONNULL_BEGIN and END as per the comment #13 (comment) Co-authored-by: Quintin <quintin@ably.io>

tiholic deleted the feature/codec-upgrade branch July 6, 2020 18:59

tiholic pushed a commit that referenced this pull request Jul 6, 2020

remove abbreviations jsonDict -> dictionary

9419876

#13 (comment)

tiholic pushed a commit that referenced this pull request Jul 6, 2020

update README

2aa4d2b

#13 (comment)

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

re-codec using JSON and Codegen #13

re-codec using JSON and Codegen #13

tiholic commented May 23, 2020 •

edited

Loading

QuintinWillison left a comment

mattheworiordan left a comment

tiholic left a comment

tiholic Jun 4, 2020

QuintinWillison left a comment

tiholic commented Jun 27, 2020

QuintinWillison left a comment

re-codec using JSON and Codegen #13

re-codec using JSON and Codegen #13

Conversation

tiholic commented May 23, 2020 • edited Loading

JSON based encoding and decoding.

Codegen for constants that need to be in Sync on platform side and flutter side

QuintinWillison left a comment

Choose a reason for hiding this comment

mattheworiordan left a comment

Choose a reason for hiding this comment

tiholic left a comment

Choose a reason for hiding this comment

tiholic Jun 4, 2020

Choose a reason for hiding this comment

QuintinWillison left a comment

Choose a reason for hiding this comment

tiholic commented Jun 27, 2020

QuintinWillison left a comment

Choose a reason for hiding this comment

tiholic commented May 23, 2020 •

edited

Loading