kong
kong copied to clipboard
feat (pdk) getters on service request
Summary
This PR is to add getter functions in the pdk.service.request
object: get_method
, get_scheme
, get_host
, get_port
, get_path
, get_raw_query
, get_query_arg
, get_query
, get_header
and get_headers
.
get_raw_body
and get_body
are not in scope, so far.
During the review of this PR, could you please pay some specific attention to:
- The check of the phase. I guessed that all getters should be accessible from
PHASE.request
- The getters for the host and port are based on
ngx.ctx.balancer_data
. Is it a correct assumption? (the tests tend to say ‘yes’, but I would like that you confirm this assumption)
To be honest, I made many copy/paste from pdk.request
… but hopefully there are a couple of differences ;)
Note that making this exercice on getters, I realized that I have a concern related to the setters of the Service Request (and the impact of them on the Request). I’m about to post a topic on KongNation for a discussion on that (as this is somehow out of this PR).
Full changelog
- Updated
pdk.service.request
- Implemented tests: I started by implementing tests under
t/01-pdk/06-service-request/
and I realized at the end that many test cases were not possible or meaningless to implement with this solution (contrary to the tests onpdk.request
) as it was just checking the value that was just being set through another way, without really checking that the right value was set at the right place by Kong routing mechanism itself. So I implemented another set of tests usingbusted
. Tests are located in thespec/02-integration/08-pdk/01-service-request_spec.lua
file (spec/02-integration/08-pdk/
is indeed a new directory). For this, I also implemented a new test plugin located in thespec/fixtures/custom_plugins/kong/plugins/pdk-service-request/
directory. Please, tell me if you prefer keeping only one of the test sets (I would say the one based onbusted
as this is the one with the most important test coverage), or if you want to keep both.
PDK update and tests are provided in 2 different commits.
Issues resolved
Hi all, Is there any missing information for the review of this PR ? I'm at your disposal for any missing stuff: feel free to ask :P Thanks !
Hi @pamiel,
Sorry we didn't get a chance to look at this for now. We are still busy with releasing Kong 1.0 these days. We will let get back to this when we have more time on our hands to review such large PRs. Thanks! :)
@pamiel Please re-open if this is still needed.