Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #999 +/- ##
============================================
+ Coverage 65.61% 70.88% +5.27%
- Complexity 3319 3933 +614
============================================
Files 355 388 +33
Lines 13865 15661 +1796
Branches 1498 1704 +206
============================================
+ Hits 9098 11102 +2004
+ Misses 3916 3580 -336
- Partials 851 979 +128 ☔ View full report in Codecov by Sentry. |
|
It looks good.
Would you add a test that verifies the status? It is a simple test but will be helpful to detect regressions in the future. |
|
@ikhoon nim, i make a new commit to apply your comments. I discovered something in the process. the values may differ! Since the |
ikhoon
left a comment
There was a problem hiding this comment.
Thanks, @chickenchickenlove! 👍👍
minwoox
left a comment
There was a problem hiding this comment.
Thanks, @chickenchickenlove ❤️
Motivation:
content-typein header, Central Dogma returns the response: {"exception":"java.lang.IllegalArgumentException","message":"No suitable request converter found for a @RequestObject 'CommitMessageDto'"}. This response is quite ambiguous, making it difficult for the client to understand the problem.Modifications:
@ConsumesJsonto each method that usesChangesRequestConverter.class.FYI
@RequestConverteris declared on method signatures, Armeria will add an object Resolver. (link)ChangesRequestConvertertries to parse body from request. Actually,ChangeRequestConverterdelegates toJacksonRequestConvertFunction.JacksonRequestConvertervalidates contents type. (here)content-typeorcontent-typeis notapplication/json,JacksonRequestConverterwill callRequestConverterFunction.fallthrough().With this flow, client will receive response
: {"exception":"java.lang.IllegalArgumentException","message":"No suitable request converter found for a @RequestObject 'CommitMessageDto'"}.If
@ConsumsJsonis delcared to each method havingChangesRequestConverter.classon their method signature, Armeria will not try to resolve request without headerContent-Type: application/json.Result:
@Consume(content-type)to all REST services class or methods #987.