chore(api)!: Model .fromJson() Refactor #3032
Conversation
packages/amplify_core/lib/src/types/datastore/conflict_handler/datastore_conflict_handler.dart
Outdated
Show resolved
Hide resolved
...rc/main/kotlin/com/amazonaws/amplify/amplify_datastore/types/model/FlutterSerializedModel.kt
Outdated
Show resolved
Hide resolved
packages/amplify_datastore/example/integration_test/utils/setup_utils.dart
Outdated
Show resolved
Hide resolved
| @@ -5,11 +5,12 @@ library sample_app; | |||
|
|
|||
| import 'dart:async'; | |||
|
|
|||
| import 'package:amplify_core/amplify_core.dart'; | |||
There was a problem hiding this comment.
I think we should import from amplify_flutter to ensure platform stuff can be correctly configured.
There was a problem hiding this comment.
Yes, it's required. amplify_core should only be imported directly in Dart-only environments.
There was a problem hiding this comment.
Didn't mean to include this file in this PR, will be removed 👍
...rc/main/kotlin/com/amazonaws/amplify/amplify_datastore/types/model/FlutterSerializedModel.kt
Outdated
Show resolved
Hide resolved
packages/amplify_core/lib/src/types/datastore/hub/hub_event_element.dart
Outdated
Show resolved
Hide resolved
packages/amplify_core/lib/src/types/datastore/models/subscription_event.dart
Outdated
Show resolved
Hide resolved
packages/amplify_datastore/example/lib/models/BelongsToChildExplicit.dart
Outdated
Show resolved
Hide resolved
| .map((e) => Post.fromJson( | ||
| Map<String, dynamic>.from(e['serializedData']))) | ||
| _posts = json['posts'] != null | ||
| ? (json['posts']['items'] as List) |
There was a problem hiding this comment.
Is items guaranteed to be there if posts is non-null?
You could add a safeguard if not:
| ? (json['posts']['items'] as List) | |
| ? (json['posts']['items'] as List? ?? const []) |
There was a problem hiding this comment.
AppSync will pass an empty items array when posts is specified in the document. When posts is not in the document and therefore the response too, decoding returns null for posts.
| @@ -5,11 +5,12 @@ library sample_app; | |||
|
|
|||
| import 'dart:async'; | |||
|
|
|||
| import 'package:amplify_core/amplify_core.dart'; | |||
There was a problem hiding this comment.
Yes, it's required. amplify_core should only be imported directly in Dart-only environments.
packages/amplify_datastore/ios/Classes/types/hub/FlutterHubElement.swift
Outdated
Show resolved
Hide resolved
| result[key] = [ | ||
| "modelName": nextModelName, | ||
| "serializedData": try generateSerializedData( | ||
| let dataMap = try generateSerializedData( |
There was a problem hiding this comment.
Similar comment to Hui's. Instead of merging modelName into the model (which may conflict with already-present data), it may be better to return a wrapped value or use __typename instead which is part of the GraphQL spec.
There was a problem hiding this comment.
Was naming the field __modelName the resolution to this?
| @@ -15,10 +15,10 @@ import 'package:meta/meta.dart'; | |||
|
|
|||
| import 'ModelProvider.dart'; | |||
|
|
|||
| /** This is an auto generated class representing the Blog type in your schema. */ | |||
| /// This is an auto generated class representing the Blog type in your schema. | |||
There was a problem hiding this comment.
What are we testing with these "legacy" models?
There was a problem hiding this comment.
I believe this is validating older versions of codegen that we still support since not all customers update their models/backends. @ragingsquirrel3 may have more context.
Jordan-Nelson
changed the title
| @@ -180,7 +180,7 @@ name = value'''; | |||
| }); | |||
| test(r'Windows style line endings are supported.', () { | |||
| const configContents = r''' | |||
| [profile foo] | |||
| [profile foo] | |||
| @@ -75,7 +75,7 @@ struct FlutterSerializedModelData { | |||
| "postalCode": "94115" | |||
| ]), | |||
| JSONValue.object([ | |||
| "line1": "000 Somewhere close", | |||
| "line1": "111 Somewhere close", | |||
| { "field": "created", "order": "descending" } | ||
| ] | ||
| } | ||
| { |
There was a problem hiding this comment.
What are the changes in this file?
| result[key] = [ | ||
| "modelName": nextModelName, | ||
| "serializedData": try generateSerializedData( | ||
| let dataMap = try generateSerializedData( |
There was a problem hiding this comment.
Was naming the field __modelName the resolution to this?
Equartey
force-pushed
the
chore/api-from-json-refactor
branch
from
8e99a0c
to
4c0d4ae
Compare
|
closing in favor of: #4665 |
Issue #, if available:
#816
Description of changes:
Flatens the nested data structure of
serializedDataacross the method channel and the.fromJson()method found on codegen models. This PR is to be paired with changes in aws-amplify/amplify-codegen#593By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.