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
errorResponseBodySerializer handles strings properly #108
Conversation
Codecov Report
@@ Coverage Diff @@
## master #108 +/- ##
============================================
- Coverage 91.19% 91.08% -0.11%
+ Complexity 2138 2137 -1
============================================
Files 150 150
Lines 6179 6182 +3
Branches 813 815 +2
============================================
- Hits 5635 5631 -4
- Misses 362 369 +7
Partials 182 182
Continue to review full report at Codecov.
|
c6b338e
to
f2ff89f
Compare
I amended my commit. Is there a way to kickoff a new coverage report? |
It should happen automatically. Looks like Travis had some connectivity issues grabbing dependencies. I retriggered the build. Hopefully when it finishes successfully it'll update the coverage. |
@@ -40,7 +40,11 @@ public static ErrorResponseBodySerializer asErrorResponseBodySerializer(ObjectMa | |||
// indicates empty response body payload, so we should return null. | |||
return null; | |||
} | |||
return objectMapper.writeValueAsString(errorResponseBody.bodyToSerialize()); | |||
if(errorResponseBody.bodyToSerialize() instanceof CharSequence) { |
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.
Pull errorResponseBody.bodyToSerialize()
into a variable so we're not calling it multiple times. Although it would be unwise to do something expensive in that method, we can't control how people will use it, so better safe than sorry.
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.
And I guess that would need to be up top, before the null checks. Hmm. Maybe:
Object bodyToSerialize = (errorResponseBody == null) ? null : errorResponseBody.bodyToSerialize();
And then the null check would just become if (bodyToSerialize == null) { ...
, and we can use bodyToSerialize
later if it passes the null check.
I have amended the commit once more with the requested changes. |
Thank you @nmyers322 ! |
fixes #107