Add JsonKey.required, plus other things#221
Merged
Merged
Conversation
Put logic in one place to make future additions easier
jakemac53
reviewed
Jun 6, 2018
| `MissingRequiredKeysException` that is thrown when `required` fields don't | ||
| have corresponding keys in a source JSON map. | ||
|
|
||
| * Added a new `$` helper function and deprecate an old one. Upgrading to the |
| /// | ||
| /// Should not be used directly. | ||
| @Deprecated('Code generated with the latest `json_serializable` will use ' | ||
| '`\$checkKeys` instead. This function will be removed in the next majoy ' |
| } | ||
|
|
||
| /// Helper function used in generated code when | ||
| /// `JsonSerializable.disallowUnrecognizedKeys` is `true`. |
Collaborator
There was a problem hiding this comment.
Or if any fields have JsonKey.required
| /// Should not be used directly. | ||
| void $checkKeys(Map map, | ||
| {Iterable<String> allowedKeys, List<String> requiredKeys}) { | ||
| if (map == null) return; |
Collaborator
There was a problem hiding this comment.
Should this still happen if requiredKeys is not empty?
Collaborator
Author
There was a problem hiding this comment.
ack – although look at my fix
| var invalidKeys = map.keys.where((k) => !allowedKeys.contains(k)); | ||
| if (invalidKeys.isNotEmpty) { | ||
| throw new UnrecognizedKeysException( | ||
| new List<String>.from(invalidKeys), map, allowedKeys.toList()); |
Collaborator
There was a problem hiding this comment.
lets move this toList up to the original assignment so its consistent
| * Added support for `JsonKey.required`. | ||
| * When `true`, generated code throws a `MissingRequiredKeysException` if | ||
| the key does not exist in the JSON map used to populate the annotated field. | ||
| * Will be captured captured and wrapped in a `CheckedFromJsonException` if |
| if (!field.isPublic) { | ||
| unavailableReasons[field.name] = 'It is assigned to a private field.'; | ||
| } else if (jsonKeyFor(field).ignore == true) { | ||
| } else if (jsonKeyFor(field).ignore) { |
Collaborator
There was a problem hiding this comment.
hmm so its important now that anybody using the newest json_serializable changes there json_annotation dependency as well to have the minimum bound of 0.2.8... but there is no way to really communicate that.
I am not sure what the right solution is here?
Will allow for other checks to be added
jakemac53
approved these changes
Jun 6, 2018
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.