-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix error handling in json functions with default values #8111
fix error handling in json functions with default values #8111
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8111 +/- ##
=============================================
- Coverage 71.35% 14.15% -57.20%
+ Complexity 4302 81 -4221
=============================================
Files 1617 1572 -45
Lines 83896 82019 -1877
Branches 12543 12340 -203
=============================================
- Hits 59866 11613 -48253
- Misses 19944 69542 +49598
+ Partials 4086 864 -3222
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM otherwise
@@ -89,7 +89,7 @@ public static Object jsonPath(Object object, String jsonPath) { | |||
if (object instanceof String) { | |||
return PARSE_CONTEXT.parse((String) object).read(jsonPath, NO_PREDICATES); | |||
} | |||
return PARSE_CONTEXT.parse(object).read(jsonPath, NO_PREDICATES); | |||
return object == null ? null : PARSE_CONTEXT.parse(object).read(jsonPath, NO_PREDICATES); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Input object
should not be null
. IMO We can let it throw NPE
we caught this in our CI pipeline