Closed chickenchickenlove closed 3 months ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 70.88%. Comparing base (
c960313
) to head (38bd2b1
). Report is 135 commits behind head on main.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
It looks good.
Client will receive 415 response.
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.
When the Content-Type
is not configured for each client, the default Content-Type
may differ.
the values may differ!
dogma.client
don't configure Content-Type
. thus value
is null
.
curl
configure application/x-form-urlencoded
as default. thus value
is application/x-form/urlencoded
.
aiohttp.ClientSession()
in python configure text/plain
as default. thus value
is text/plain
.
Since the Dogma client
defaults to a null
Content-Type, it always will receive 400 response.
Therefore, to properly test this, I used parameterized tests to vary the Content-Type
.
Motivation:
content-type
in 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:
@ConsumesJson
to each method that usesChangesRequestConverter.class
.FYI
@RequestConverter
is declared on method signatures, Armeria will add an object Resolver. (link)ChangesRequestConverter
tries to parse body from request. Actually,ChangeRequestConverter
delegates toJacksonRequestConvertFunction
.JacksonRequestConverter
validates contents type. (here)content-type
orcontent-type
is notapplication/json
,JacksonRequestConverter
will callRequestConverterFunction.fallthrough()
.With this flow, client will receive response
: {"exception":"java.lang.IllegalArgumentException","message":"No suitable request converter found for a @RequestObject 'CommitMessageDto'"}
.If
@ConsumsJson
is delcared to each method havingChangesRequestConverter.class
on their method signature, Armeria will not try to resolve request without headerContent-Type: application/json
.Result: