feat(api): .subscribe() for GraphQL #1915

ragingsquirrel3 · 2022-07-21T23:34:49Z

This PR has subscriptions for GraphQL in dart. It implements the appsync real-time client described on https://docs.aws.amazon.com/appsync/latest/devguide/aws-appsync-real-time-data.html.

It does not implement reconnect on error or backoff/retry.

It does not include increased integration tests for other auth modes.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

* feat(api): GraphQL API key auth mode * BREAKING CHANGE: GraphQL response errors now nullable

…r registers in Auth (#1851)

codecov-commenter · 2022-07-22T00:12:55Z

Codecov Report

Merging #1915 (6e384c3) into feat/api-next (3ac59f4) will increase coverage by 0.22%.
The diff coverage is 71.92%.

@@                Coverage Diff                @@
##           feat/api-next    #1915      +/-   ##
=================================================
+ Coverage          44.82%   45.05%   +0.22%     
=================================================
  Files                129      129              
  Lines               7752     7853     +101     
=================================================
+ Hits                3475     3538      +63     
- Misses              4277     4315      +38

Flag	Coverage Δ
android-unit-tests	`∅ <ø> (∅)`
flutter-unit-tests	`30.00% <71.92%> (+1.50%)`	⬆️
ios-unit-tests	`89.48% <ø> (+0.39%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...kages/api/amplify_api/lib/src/api_plugin_impl.dart	`81.73% <30.00%> (-5.51%)`	⬇️
..._api/lib/src/graphql/ws/web_socket_connection.dart	`59.77% <59.77%> (ø)`
...plify_api/lib/src/graphql/ws/web_socket_types.dart	`79.36% <79.36%> (ø)`
...phql/ws/web_socket_message_stream_transformer.dart	`90.47% <90.47%> (ø)`
..._api/lib/src/decorators/web_socket_auth_utils.dart	`100.00% <100.00%> (ø)`
.../ios/unit_tests/amplify_flutter_exampleTests.swift
...plify_flutter/example/ios/Runner/AppDelegate.swift
...ter/example/ios/unit_tests/AtomicResultTests.swift
...e/ios/unit_tests/MockAnalyticsCategoryPlugin.swift

packages/api/amplify_api/lib/src/graphql/ws/websocket_connection.dart

packages/api/amplify_api/lib/src/decorators/authorize_websocket_message.dart

packages/api/amplify_api/lib/src/graphql/ws/websocket_connection.dart

packages/api/amplify_api/lib/src/decorators/authorize_websocket_message.dart

packages/api/amplify_api/lib/src/graphql/ws/websocket_message.dart

packages/api/amplify_api/lib/src/graphql/ws/websocket_message_stream_transformer.dart

packages/api/amplify_api/lib/src/graphql/ws/websocket_message.dart

ragingsquirrel3 · 2022-07-27T22:11:37Z

I'm still working on this, especially testing, logging, error handling, many comments not addressed. However, I did refactor the auth logic to use more pure functions and added lots of comments there. Let me know if still difficult to follow.

Equartey

Nice work!

Question: Does the example app need to be updated to reflect any api changes?

packages/api/amplify_api/lib/src/graphql/ws/web_socket_connection.dart

…HTTP requests (#1913)

ragingsquirrel3 · 2022-08-10T21:15:00Z

Nice work!

Question: Does the example app need to be updated to reflect any api changes?

No API changes for subscriptions, though I did just make a change to example app with last rev so unsubscribe button works.

* feat(api): GraphQL API key auth mode * BREAKING CHANGE: GraphQL response errors now nullable

…r registers in Auth (#1851)

…HTTP requests (#1913)

dnys1 · 2022-08-16T22:53:45Z

packages/api/amplify_api/test/dart_graphql_test.dart

@@ -12,10 +12,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.

+import 'dart:async';


I guess I never noticed this - but why are the other method channel tests/infra hanging around still? Outside of the configure API, we have no use for them anymore. These tests should just replace the old subscribe tests.

I would rather do the cleanup in a single, separate PR. There was a task we mentioned in grooming for it.

packages/api/amplify_api/lib/src/graphql/ws/websocket_message_stream_transformer.dart

packages/api/amplify_api/example/lib/graphql_api_view.dart

packages/api/amplify_api/lib/src/graphql/ws/web_socket_connection.dart

Equartey

Good work on the revisions. I noticed some things that will be nice for reconnection :)

packages/api/amplify_api/lib/src/decorators/web_socket_auth_utils.dart

packages/api/amplify_api/lib/src/graphql/ws/web_socket_connection.dart

packages/api/amplify_api/lib/src/graphql/ws/web_socket_message_stream_transformer.dart

packages/api/amplify_api/lib/src/graphql/ws/web_socket_types.dart

packages/api/amplify_api/test/dart_graphql_test.dart

dnys1 · 2022-08-19T23:30:54Z

packages/api/amplify_api/test/util.dart

+
+/// Extension of [WebSocketConnection] that stores messages internally instead
+/// of sending them.
+class MockWebSocketConnection extends WebSocketConnection {


At some point we should be mocking/injecting the WebSocketChannel instead and not overriding methods since it becomes unclear what's being tested. Like here, I guess we're testing everything but connect/send - but those seem like good things to test too.

makes sense, I put a task in backlog to improve this

dnys1

This looks good for now. Once the method channel stuff is gone, I would recommend enabling lints at that point and doing the clean up for that so that future PRs meet spec. I've withheld all my formatting/style comments because it's easier if the analyzer prompts you as you go.

dnys1 · 2022-08-25T23:41:54Z

packages/api/amplify_api/lib/src/graphql/ws/web_socket_connection.dart

+  }
+
+  /// Times out the connection (usually if a keep alive has not been received in time).
+  void _timeout(Duration timeoutDuration) {


Shouldn't this method do more, like close the WS connection? I guess Elijah's PR will address that

Yeah, I think there will be more cleanup here but will leave for Elijah.

Co-authored-by: Elijah Quartey <Equartey@users.noreply.github.com>

Travis Sheppard and others added 9 commits July 21, 2022 13:58

chore!(api): migrate API category type definitions (#1640)

a68b67a

chore(api): API Native Bridge for .addPlugin() (#1756)

6f4c84f

chore(api): API Pigeon update (#1813)

7b4e0ca

feat(api): REST methods in dart with auth mode none (#1783)

26b23b6

feat!(api): GraphQL API key auth mode (#1858)

e3c9cf6

* feat(api): GraphQL API key auth mode * BREAKING CHANGE: GraphQL response errors now nullable

feat!(core,auth): auth providers definition and CognitoIamAuthProvide…

18e4272

…r registers in Auth (#1851)

feat(core,api): IAM auth mode for HTTP requests (REST and GQL) (#1893)

7a98c5a

feat(api): .subscribe() for GraphQL

5f2783b

change name

55931aa

dnys1 reviewed Jul 22, 2022

View reviewed changes

Travis Sheppard added 3 commits July 26, 2022 15:34

add some unit tests

b6ebdab

refactor a little

dcaeba1

some renames

5715cb5

Equartey reviewed Jul 28, 2022

View reviewed changes

Equartey and others added 5 commits July 29, 2022 09:31

feat(api): GraphQL Custom Request Headers (#1938)

db28cab

feat(auth,api): cognito user pools auth provider & auth mode for API …

c8297a6

…HTTP requests (#1913)

Merge branch 'feat/api-next' into feat/api-next-subscription

a62dc21

change disconnect

c464e6e

add logger

d93e00c

Travis Sheppard and others added 8 commits August 15, 2022 07:52

throw error during connection

ac5348a

expand unit tests

2bc9024

chore!(api): migrate API category type definitions (#1640)

bdf706f

chore(api): API Native Bridge for .addPlugin() (#1756)

ef1b223

chore(api): API Pigeon update (#1813)

4621a66

feat(api): REST methods in dart with auth mode none (#1783)

438c236

feat!(api): GraphQL API key auth mode (#1858)

c99ca0c

* feat(api): GraphQL API key auth mode * BREAKING CHANGE: GraphQL response errors now nullable

feat!(core,auth): auth providers definition and CognitoIamAuthProvide…

98891d7

…r registers in Auth (#1851)

feat(auth,api): cognito user pools auth provider & auth mode for API …

64a50f6

…HTTP requests (#1913)

ragingsquirrel3 force-pushed the feat/api-next branch from c8297a6 to 64a50f6 Compare August 16, 2022 17:15

Merge branch 'feat/api-next' into feat/api-next-subscription

02c51fe

ragingsquirrel3 marked this pull request as ready for review August 16, 2022 20:43

ragingsquirrel3 requested a review from a team as a code owner August 16, 2022 20:43

dnys1 reviewed Aug 16, 2022

View reviewed changes

Equartey reviewed Aug 17, 2022

View reviewed changes

packages/api/amplify_api/example/lib/graphql_api_view.dart Show resolved Hide resolved

Equartey reviewed Aug 17, 2022

View reviewed changes

packages/api/amplify_api/lib/src/graphql/ws/web_socket_connection.dart Outdated Show resolved Hide resolved

Equartey approved these changes Aug 17, 2022

View reviewed changes

correct error handling

8b125c6

dnys1 reviewed Aug 19, 2022

View reviewed changes

Travis Sheppard added 4 commits August 22, 2022 10:05

fix(auth): correct auth providers imports from rebase (#2042)

7300024

Merge branch 'feat/api-next' into feat/api-next-subscription

8eebbe4

improve test syntax

e1ac0ce

change null safety

b6358fc

ragingsquirrel3 force-pushed the feat/api-next branch from 7300024 to 3ac59f4 Compare August 23, 2022 21:04

Travis Sheppard added 3 commits August 23, 2022 14:07

Merge branch 'feat/api-next' into feat/api-next-subscription

a7a9339

undo plist change

d09da9a

make enum messageType

6e384c3

dnys1 approved these changes Aug 25, 2022

View reviewed changes

ragingsquirrel3 merged commit 4009458 into aws-amplify:feat/api-next Aug 26, 2022

ragingsquirrel3 deleted the feat/api-next-subscription branch August 26, 2022 16:13

ragingsquirrel3 pushed a commit that referenced this pull request Sep 9, 2022

feat(api): .subscribe() for GraphQL (#1915)

2e05c6f

Co-authored-by: Elijah Quartey <Equartey@users.noreply.github.com>

ragingsquirrel3 pushed a commit that referenced this pull request Sep 14, 2022

feat(api): .subscribe() for GraphQL (#1915)

678154c

Co-authored-by: Elijah Quartey <Equartey@users.noreply.github.com>

ragingsquirrel3 pushed a commit that referenced this pull request Sep 15, 2022

feat(api): .subscribe() for GraphQL (#1915)

c705795

Co-authored-by: Elijah Quartey <Equartey@users.noreply.github.com>

ragingsquirrel3 pushed a commit that referenced this pull request Sep 26, 2022

feat(api): .subscribe() for GraphQL (#1915)

0dddb4c

Co-authored-by: Elijah Quartey <Equartey@users.noreply.github.com>

ragingsquirrel3 pushed a commit that referenced this pull request Oct 19, 2022

feat(api): .subscribe() for GraphQL (#1915)

79d6526

Co-authored-by: Elijah Quartey <Equartey@users.noreply.github.com>

ragingsquirrel3 pushed a commit that referenced this pull request Nov 8, 2022

feat(api): .subscribe() for GraphQL (#1915)

93b4e80

Co-authored-by: Elijah Quartey <Equartey@users.noreply.github.com>

HuiSF pushed a commit to HuiSF/amplify-flutter that referenced this pull request Nov 10, 2022

feat(api): .subscribe() for GraphQL (aws-amplify#1915)

f292459

Co-authored-by: Elijah Quartey <Equartey@users.noreply.github.com>

ragingsquirrel3 pushed a commit that referenced this pull request Nov 10, 2022

feat(api): .subscribe() for GraphQL (#1915)

4dd0271

Co-authored-by: Elijah Quartey <Equartey@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api): .subscribe() for GraphQL #1915

feat(api): .subscribe() for GraphQL #1915

ragingsquirrel3 commented Jul 21, 2022 •

edited

Loading

codecov-commenter commented Jul 22, 2022 •

edited

Loading

ragingsquirrel3 commented Jul 27, 2022 •

edited

Loading

Equartey left a comment

ragingsquirrel3 commented Aug 10, 2022

dnys1 Aug 16, 2022

ragingsquirrel3 Aug 17, 2022 •

edited

Loading

Equartey left a comment

dnys1 Aug 19, 2022

ragingsquirrel3 Aug 25, 2022

dnys1 left a comment

dnys1 Aug 25, 2022

ragingsquirrel3 Aug 26, 2022

feat(api): .subscribe() for GraphQL #1915

feat(api): .subscribe() for GraphQL #1915

Conversation

ragingsquirrel3 commented Jul 21, 2022 • edited Loading

codecov-commenter commented Jul 22, 2022 • edited Loading

Codecov Report

ragingsquirrel3 commented Jul 27, 2022 • edited Loading

Equartey left a comment

Choose a reason for hiding this comment

ragingsquirrel3 commented Aug 10, 2022

dnys1 Aug 16, 2022

Choose a reason for hiding this comment

ragingsquirrel3 Aug 17, 2022 • edited Loading

Choose a reason for hiding this comment

Equartey left a comment

Choose a reason for hiding this comment

dnys1 Aug 19, 2022

Choose a reason for hiding this comment

ragingsquirrel3 Aug 25, 2022

Choose a reason for hiding this comment

dnys1 left a comment

Choose a reason for hiding this comment

dnys1 Aug 25, 2022

Choose a reason for hiding this comment

ragingsquirrel3 Aug 26, 2022

Choose a reason for hiding this comment

ragingsquirrel3 commented Jul 21, 2022 •

edited

Loading

codecov-commenter commented Jul 22, 2022 •

edited

Loading

ragingsquirrel3 commented Jul 27, 2022 •

edited

Loading

ragingsquirrel3 Aug 17, 2022 •

edited

Loading